Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions code/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@
"react-textarea-autosize": "^8.3.0",
"react-transition-group": "^4.4.5",
"require-from-string": "^2.0.2",
"resolve": "^1.22.11",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Good: stable, well-known module resolution library added.

The "resolve" package is a mature, well-established module resolution utility. The caret version constraint (^1.22.11) is sensible, allowing patch and minor updates while maintaining API stability.

Verify that the version is current and free from known vulnerabilities, and confirm it's actually used in the new utilities (e.g., the resolveImport function mentioned in the PR).


🌐 Web query:

What is the latest version of resolve npm package and are there any known security vulnerabilities?

💡 Result:

Latest published version: 1.22.10 (published ~9 months ago). [1]

Known security status: there are no high‑profile npm security advisories explicitly for the resolve package in the npm advisory database; however third‑party scanners (e.g., ReversingLabs / Snyk feed) flag some indirect/patch‑mandated issues tied to versions/dependencies. I recommend running npm audit / Snyk scan on your project to see any dependency paths and remediations. [1][2]

Sources:

  • npm package page for resolve (version and publish info). [1]
  • ReversingLabs/Security scan summary showing flagged issues for resolve. [2]

Correct the resolve version: 1.22.11 does not exist as a published release.

The latest published version of resolve is 1.22.10, but the PR specifies ^1.22.11. Update line 317 to use "resolve": "^1.22.10" instead.

No high-profile npm security advisories exist for resolve, though third-party scanners flag some indirect/patch-mandated issues. Run npm audit after correcting the version to verify dependency security.

🤖 Prompt for AI Agents
In code/core/package.json at line 317 the dependency version for resolve is
incorrect (references non-existent 1.22.11); change the entry to reference the
latest published 1.22.10 (e.g. "resolve": "^1.22.10") and then run npm install
and npm audit to verify dependencies and security.

"resolve.exports": "^2.0.3",
"sirv": "^2.0.4",
"slash": "^5.0.0",
Expand Down
1 change: 1 addition & 0 deletions code/core/src/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export * from './js-package-manager';
export * from './utils/scan-and-transform-files';
export * from './utils/transform-imports';
export * from '../shared/utils/module';
export * from './utils/utils';

export { versions };

Expand Down
48 changes: 44 additions & 4 deletions code/core/src/common/utils/interpret-files.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,58 @@
import { existsSync } from 'node:fs';
import { extname } from 'node:path';

import resolve from 'resolve';

export const supportedExtensions = [
'.js',
'.mjs',
'.cjs',
'.jsx',
'.ts',
'.jsx',
'.tsx',
'.mjs',
'.mts',
'.mtsx',
'.cjs',
'.cts',
'.tsx',
'.ctsx',
] as const;

export function getInterpretedFile(pathToFile: string) {
return supportedExtensions
.map((ext) => (pathToFile.endsWith(ext) ? pathToFile : `${pathToFile}${ext}`))
.find((candidate) => existsSync(candidate));
}

export function resolveImport(id: string, options: resolve.SyncOpts): string {
const mergedOptions: resolve.SyncOpts = {
extensions: supportedExtensions,
packageFilter(pkg) {
// Prefer 'module' over 'main' if available
if (pkg.module) {
pkg.main = pkg.module;
}
return pkg;
},
...options,
};

try {
return resolve.sync(id, { ...mergedOptions });
} catch (error) {
const ext = extname(id);

// if we try to import a JavaScript file it might be that we are actually pointing to
// a TypeScript file. This can happen in ES modules as TypeScript requires to import other
// TypeScript files with .js extensions
// https://www.typescriptlang.org/docs/handbook/esm-node.html#type-in-packagejson-and-new-extensions
const newId = ['.js', '.mjs', '.cjs'].includes(ext)
? `${id.slice(0, -2)}ts`
: ext === '.jsx'
? `${id.slice(0, -3)}tsx`
: null;

if (!newId) {
throw error;
}
return resolve.sync(newId, { ...mergedOptions, extensions: [] });
}
}
26 changes: 26 additions & 0 deletions code/core/src/common/utils/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Object.groupBy polyfill
export const groupBy = <K extends PropertyKey, T>(
items: T[],
keySelector: (item: T, index: number) => K
) => {
return items.reduce<Record<K, T[]>>(
(acc, item, index) => {
const key = keySelector(item, index);
acc[key] ??= [];
acc[key].push(item);
return acc;
},
{} as Record<K, T[]>
);
};

// This invariant allows for lazy evaluation of the message, which we need to avoid excessive computation.
export function invariant(
condition: unknown,
message?: string | (() => string)
): asserts condition {
if (condition) {
return;
}
throw new Error((typeof message === 'function' ? message() : message) ?? 'Invariant failed');
}
7 changes: 6 additions & 1 deletion code/core/src/core-server/build-static.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,14 @@ import {
import { logger } from 'storybook/internal/node-logger';
import { getPrecedingUpgrade, telemetry } from 'storybook/internal/telemetry';
import type { BuilderOptions, CLIOptions, LoadOptions, Options } from 'storybook/internal/types';
import { type ComponentManifestGenerator } from 'storybook/internal/types';
import { type ComponentManifestGenerator, type ComponentsManifest } from 'storybook/internal/types';

import { global } from '@storybook/global';

import picocolors from 'picocolors';

import { resolvePackageDir } from '../shared/utils/module';
import { renderManifestComponentsPage } from './manifest';
import { StoryIndexGenerator } from './utils/StoryIndexGenerator';
import { buildOrThrow } from './utils/build-or-throw';
import { copyAllStaticFilesRelativeToMain } from './utils/copy-all-static-files';
Expand Down Expand Up @@ -180,6 +181,10 @@ export async function buildStaticStandalone(options: BuildStaticStandaloneOption
join(options.outputDir, 'manifests', 'components.json'),
JSON.stringify(manifests)
);
await writeFile(
join(options.outputDir, 'manifests', 'components.html'),
renderManifestComponentsPage(manifests)
);
} catch (e) {
logger.error('Failed to generate manifests/components.json');
logger.error(e instanceof Error ? e : String(e));
Expand Down
31 changes: 30 additions & 1 deletion code/core/src/core-server/dev-server.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import { logConfig } from 'storybook/internal/common';
import { logger } from 'storybook/internal/node-logger';
import { MissingBuilderError } from 'storybook/internal/server-errors';
import type { Options } from 'storybook/internal/types';
import type { ComponentsManifest, Options } from 'storybook/internal/types';
import { type ComponentManifestGenerator } from 'storybook/internal/types';

import compression from '@polka/compression';
import polka from 'polka';
import invariant from 'tiny-invariant';

import { telemetry } from '../telemetry';
import { renderManifestComponentsPage } from './manifest';
import { type StoryIndexGenerator } from './utils/StoryIndexGenerator';
import { doTelemetry } from './utils/doTelemetry';
import { getManagerBuilder, getPreviewBuilder } from './utils/get-builders';
Expand Down Expand Up @@ -165,6 +166,34 @@ export async function storybookDevServer(options: Options) {
return;
}
});

app.get('/manifests/components.html', async (req, res) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be nice to also support '/manifests/components', without the html extension. just call get() two times with the same handler.

try {
const componentManifestGenerator: ComponentManifestGenerator = await options.presets.apply(
'experimental_componentManifestGenerator'
);
const indexGenerator = await initializedStoryIndexGenerator;

if (!componentManifestGenerator || !indexGenerator) {
res.statusCode = 400;
res.setHeader('Content-Type', 'text/html; charset=utf-8');
res.end(`<pre>No component manifest generator configured.</pre>`);
return;
}

const manifest = (await componentManifestGenerator(
indexGenerator as unknown as import('storybook/internal/core-server').StoryIndexGenerator
)) as ComponentsManifest;

res.setHeader('Content-Type', 'text/html; charset=utf-8');
res.end(renderManifestComponentsPage(manifest));
} catch (e) {
// logger?.error?.(e instanceof Error ? e : String(e));
res.statusCode = 500;
res.setHeader('Content-Type', 'text/html; charset=utf-8');
res.end(`<pre>${e instanceof Error ? e.toString() : String(e)}</pre>`);
}
Comment on lines +191 to +195
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Escape HTML in error responses to avoid XSS; log the error.

Interpolating raw error text into HTML is unsafe. Also, keep server logs. Two options:

  • Safer HTML: escape the string and keep text/html.
  • Or return text/plain without HTML.

Suggested patch (HTML + escape + logging):

-      } catch (e) {
-        // logger?.error?.(e instanceof Error ? e : String(e));
-        res.statusCode = 500;
-        res.setHeader('Content-Type', 'text/html; charset=utf-8');
-        res.end(`<pre>${e instanceof Error ? e.toString() : String(e)}</pre>`);
-      }
+      } catch (e) {
+        logger.error(e instanceof Error ? e : String(e));
+        const esc = (s: unknown) =>
+          String(s ?? '').replace(/[&<>"']/g, (c) => ({'&':'&amp;','<':'&lt;','>':'&gt;','"':'&quot;',"'":'&#39;'}[c] as string));
+        res.statusCode = 500;
+        res.setHeader('Content-Type', 'text/html; charset=utf-8');
+        const msg = e instanceof Error ? e.toString() : String(e);
+        res.end(`<pre><code>${esc(msg)}</code></pre>`);
+      }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// logger?.error?.(e instanceof Error ? e : String(e));
res.statusCode = 500;
res.setHeader('Content-Type', 'text/html; charset=utf-8');
res.end(`<pre>${e instanceof Error ? e.toString() : String(e)}</pre>`);
}
} catch (e) {
logger.error(e instanceof Error ? e : String(e));
const esc = (s: unknown) =>
String(s ?? '').replace(/[&<>"']/g, (c) => ({'&':'&amp;','<':'&lt;','>':'&gt;','"':'&quot;',"'":'&#39;'}[c] as string));
res.statusCode = 500;
res.setHeader('Content-Type', 'text/html; charset=utf-8');
const msg = e instanceof Error ? e.toString() : String(e);
res.end(`<pre><code>${esc(msg)}</code></pre>`);
}
🤖 Prompt for AI Agents
In code/core/src/core-server/dev-server.ts around lines 191 to 195, the error
response currently interpolates raw error text into HTML and doesn't log the
error; replace that with a safe approach: log the error via the server logger
(e.g., logger?.error(e)) and then either (a) escape the error string for HTML
(implement or use an escape utility to replace &,<,>,",') and return as
text/html with the escaped content, or (b) simpler—set Content-Type to
text/plain; charset=utf-8 and return the raw string there; ensure res.statusCode
remains 500 and use e instanceof Error ? e.toString() : String(e) as the source
for logging/escaping.

});
}
// Now the preview has successfully started, we can count this as a 'dev' event.
doTelemetry(app, core, initializedStoryIndexGenerator as Promise<StoryIndexGenerator>, options);
Expand Down
Loading