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
120 changes: 119 additions & 1 deletion code/core/src/telemetry/sanitize.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
import { describe, expect, it, vi } from 'vitest';
import os from 'node:os';

import { afterEach, describe, expect, it, vi } from 'vitest';

import { cleanPaths, sanitizeError } from './sanitize';

afterEach(() => {
vi.restoreAllMocks();
});

describe(`Errors Helpers`, () => {
describe(`sanitizeError`, () => {
it(`Sanitizes ansi codes in error`, () => {
Expand Down Expand Up @@ -120,5 +126,117 @@ describe(`Errors Helpers`, () => {
mockCwd.mockRestore();
}
);

describe(`package manager caches and home paths`, () => {
it(`should sanitize pnpm store under a different cwd`, () => {
const input = `/home/sandbox/project/node_modules/.pnpm/@storybook+addon-docs@10.0.2_@types+react@19.2.2_esbuild@0.25.10_rollup@4.31.0_storyboo_7cb8a1f4d4ca81d0abdb0f0cfacb0423/node_modules/@storybook/addon-docs`;
const separator = `/`;
const homedir = `/home/sandbox`;
const cwd = `/var/build`;
const retainedSegment = `@storybook/addon-docs`;
const forbidden = `sandbox`;

vi.spyOn(process, `cwd`).mockImplementation(() => cwd);
vi.spyOn(os, `homedir`).mockImplementation(() => homedir);

const sanitized = cleanPaths(input, separator);
expect(sanitized).toMatchInlineSnapshot(
`"$SNIP/project/node_modules/.pnpm/@storybook+addon-docs@10.0.2_@types+react@19.2.2_esbuild@0.25.10_rollup@4.31.0_storyboo_7cb8a1f4d4ca81d0abdb0f0cfacb0423/node_modules/@storybook/addon-docs"`
);

expect(sanitized).toContain(`$SNIP`);
expect(sanitized).toContain(retainedSegment);
expect(sanitized.toLowerCase()).not.toContain(forbidden.toLowerCase());
expect(sanitized.toLowerCase()).not.toContain(homedir.toLowerCase());
});

it(`should sanitize yarn berry cache in home`, () => {
const input = `/home/foo/.yarn/berry/cache/@storybook-addon-interactions-npm-7.6.1-9e0ac1ff40-10.zip/node_modules/@storybook/addon-interactions`;
const separator = `/`;
const homedir = `/home/foo`;
const cwd = `/workspace`;
const retainedSegment = `@storybook/addon-interactions`;
const forbidden = `foo`;

vi.spyOn(process, `cwd`).mockImplementation(() => cwd);
vi.spyOn(os, `homedir`).mockImplementation(() => homedir);

const sanitized = cleanPaths(input, separator);

expect(sanitized).toMatchInlineSnapshot(
`"$SNIP/.yarn/berry/cache/@storybook-addon-interactions-npm-7.6.1-9e0ac1ff40-10.zip/node_modules/@storybook/addon-interactions"`
);
expect(sanitized).toContain(`$SNIP`);
expect(sanitized).toContain(retainedSegment);
expect(sanitized.toLowerCase()).not.toContain(forbidden.toLowerCase());
expect(sanitized.toLowerCase()).not.toContain(homedir.toLowerCase());
});

it(`should sanitize node_modules path outside current cwd`, () => {
const input = `/Users/foo/project/node_modules/@storybook/addon-links/dist/cjs/index.js`;
const separator = `/`;
const homedir = `/Users/foo`;
const cwd = `/tmp/storybook`;
const retainedSegment = `@storybook/addon-links`;
const forbidden = `foo`;

vi.spyOn(process, `cwd`).mockImplementation(() => cwd);
vi.spyOn(os, `homedir`).mockImplementation(() => homedir);

const sanitized = cleanPaths(input, separator);

expect(sanitized).toMatchInlineSnapshot(
`"$SNIP/project/node_modules/@storybook/addon-links/dist/cjs/index.js"`
);
expect(sanitized).toContain(`$SNIP`);
expect(sanitized).toContain(retainedSegment);
expect(sanitized.toLowerCase()).not.toContain(forbidden.toLowerCase());
expect(sanitized.toLowerCase()).not.toContain(homedir.toLowerCase());
});

it(`should sanitize windows yarn berry cache with backslashes`, () => {
const input = `C:\\Users\\Foo\\AppData\\Local\\Yarn\\Berry\\cache\\@storybook-addon-measure-npm-7.6.21-61d2a610cb-10.zip\\node_modules\\@storybook\\addon-measure`;
const separator = `\\`;
const homedir = `C:\\Users\\Foo`;
const cwd = `C:\\build`;
const retainedSegment = `@storybook\\addon-measure`;
const forbidden = `Foo`;

vi.spyOn(process, `cwd`).mockImplementation(() => cwd);
vi.spyOn(os, `homedir`).mockImplementation(() => homedir);

const sanitized = cleanPaths(input, separator);

expect(sanitized).toMatchInlineSnapshot(
`"$SNIP\\AppData\\Local\\Yarn\\Berry\\cache\\@storybook-addon-measure-npm-7.6.21-61d2a610cb-10.zip\\node_modules\\@storybook\\addon-measure"`
);
expect(sanitized).toContain(`$SNIP`);
expect(sanitized).toContain(retainedSegment);
expect(sanitized.toLowerCase()).not.toContain(forbidden.toLowerCase());
expect(sanitized.toLowerCase()).not.toContain(homedir.toLowerCase());
});

it(`should sanitize windows path using forward slashes`, () => {
const input = `/C:/Users/Foo/OneDrive%20-%20DFe%20Project/Desktop/library/node_modules/@storybook/addon-coverage`;
const separator = `/`;
const homedir = `C:/Users/Foo`;
const cwd = `C:/build`;
const retainedSegment = `@storybook/addon-coverage`;
const forbidden = `Foo`;

vi.spyOn(process, `cwd`).mockImplementation(() => cwd);
vi.spyOn(os, `homedir`).mockImplementation(() => homedir);

const sanitized = cleanPaths(input, separator);

expect(sanitized).toMatchInlineSnapshot(
`"/$SNIP/OneDrive%20-%20DFe%20Project/Desktop/library/node_modules/@storybook/addon-coverage"`
);
expect(sanitized).toContain(`$SNIP`);
expect(sanitized).toContain(retainedSegment);
expect(sanitized.toLowerCase()).not.toContain(forbidden.toLowerCase());
expect(sanitized.toLowerCase()).not.toContain(homedir.toLowerCase());
});
});
});
});
49 changes: 37 additions & 12 deletions code/core/src/telemetry/sanitize.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import os from 'node:os';
import path from 'node:path';

export interface IErrorWithStdErrAndStdOut {
Expand All @@ -15,24 +16,47 @@ export function removeAnsiEscapeCodes(input = ''): string {
return input.replace(/\u001B\[[0-9;]*m/g, '');
}

/**
* Removes all user-specific file system paths from the input string, replacing them with "$SNIP".
* This helps sanitize sensitive user information from output (such as error messages or logs). e.g.
* `/Users/username/storybook-app/src/pages/index.js` -> `$SNIP/src/pages/index.js`
*/
export function cleanPaths(str: string, separator: string = path.sep): string {
if (!str) {
return str;
}

const stack = process.cwd().split(separator);
// Generate target strings to sanitize using both cwd and home dir
const separators = Array.from(new Set([separator, `/`, `\\`]));
const basePaths = [process.cwd(), os.homedir()].filter(Boolean);
const targets = basePaths.flatMap((basePath) =>
separators.map((sep) => ({
separator: sep,
normalizedPath: basePath.split(/[\\/]/).join(sep),
}))
);

while (stack.length > 1) {
const currentPath = stack.join(separator);
const currentRegex = new RegExp(regexpEscape(currentPath), `gi`);
str = str.replace(currentRegex, `$SNIP`);
// For each target paths, generalize up its parent directories
// and sanitize all such occurrences from the string.
targets.forEach(({ separator: sep, normalizedPath }) => {
// Split normalized path into its segments and iterate up the hierarchy.
const stack = normalizedPath.split(sep);
while (stack.length > 1) {
const currentPath = stack.join(sep);

const currentPath2 = stack.join(separator + separator);
const currentRegex2 = new RegExp(regexpEscape(currentPath2), `gi`);
str = str.replace(currentRegex2, `$SNIP`);
// Replace all case-insensitive occurrences of this path with "$SNIP".
const currentRegex = new RegExp(regexpEscape(currentPath), `gi`);
str = str.replace(currentRegex, `$SNIP`);

// Also handle the Windows case of doubled separators (e.g., "//", "\\"),
const doubledSeparatorPath = stack.join(sep + sep);
const doubledSeparatorRegex = new RegExp(regexpEscape(doubledSeparatorPath), `gi`);
str = str.replace(doubledSeparatorRegex, `$SNIP`);

stack.pop();
}
});
Comment thread
yannbf marked this conversation as resolved.

stack.pop();
}
return str;
}

Expand All @@ -51,7 +75,8 @@ export function sanitizeError(error: Error, pathSeparator: string = path.sep) {
const errorString = cleanPaths(JSON.stringify(error), pathSeparator);

return JSON.parse(errorString);
} catch (err: any) {
return `Sanitization error: ${err?.message}`;
} catch (err: unknown) {
const message = err instanceof Error ? err.message : String(err);
return `Sanitization error: ${message}`;
}
}
80 changes: 76 additions & 4 deletions code/core/src/telemetry/storybook-metadata.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,8 @@ describe('storybook-metadata', () => {
'@storybook/addon-knobs',
'@storybook/addon-ends-with-js',
'@storybook/addon-postcss',
'../local-addon',
'../../',
Comment thread
ndelangen marked this conversation as resolved.
'CUSTOM:local-addon',
'CUSTOM:..',
]);
});

Expand All @@ -162,7 +162,7 @@ describe('storybook-metadata', () => {
cwdSpy = vi.spyOn(process, `cwd`).mockReturnValueOnce(cwdMockPath);

expect(sanitizeAddonName(`${cwdMockPath}\\local-addon\\themes.js`)).toEqual(
'$SNIP\\local-addon\\themes'
'CUSTOM:local-addon'
);
});

Expand All @@ -173,9 +173,81 @@ describe('storybook-metadata', () => {
cwdSpy = vi.spyOn(process, `cwd`).mockReturnValue(cwdMockPath);

expect(sanitizeAddonName(`${cwdMockPath}/local-addon/themes.js`)).toEqual(
'$SNIP/local-addon/themes'
'CUSTOM:local-addon'
);
});

describe('normalizes path-like addon definitions', () => {
it('node_modules and package-manager cache or pnp like paths', () => {
const snippedAddonNames = [
'$SNIP/node_modules/@storybook/addon-docs',
'$SNIP\\node_modules\\@storybook\\addon-docs',
'$SNIP/node_modules/.pnpm/@storybook+addon-a11y@10.0.8_storybook@10.0.8_@testing-library+dom@10.4.0_prettier@3.7._4a81ac32f3a0cc0e4b95fdb0fa907a4f/node_modules/@storybook/addon-a11y',
'$SNIP\\node_modules\\.pnpm\\@storybook+addon-onboarding_2f3ab626f8b157d8b10cc93b0c4f7171\\node_modules\\@storybook\\addon-onboarding',
'$SNIP/common/temp/node_modules/.pnpm/@storybook+addon-essentials@8.5.8_storybook@8.5.8/node_modules/@storybook/addon-essentials',
'$SNIP/.yarn/__virtual__/@storybook-addon-jest-virtual-ceb31c55cc/0/cache/@storybook-addon-jest-npm-9.1.12-adf55af7e8-904699d820.zip/node_modules/@storybook/addon-jest',
'$SNIP\\.yarn\\__virtual__\\@storybook-addon-actions-virtual-ecf55a46d7\\4\\Users\\Foo\\AppData\\Local\\Yarn\\Berry\\cache\\@storybook-addon-actions-npm-8.6.7-0b3fcdd2b2-10.zip\\node_modules\\@storybook\\addon-actions',
'$SNIP/.yarn/cache/@storybook-addon-webpack5-compiler-babel-npm-4.0.0-7fea55bdc6-abe15a1cd3.zip/node_modules/@storybook/addon-webpack5-compiler-babel',
].map(sanitizeAddonName);

expect(snippedAddonNames).toEqual([
'@storybook/addon-docs',
'@storybook/addon-docs',
'@storybook/addon-a11y',
'@storybook/addon-onboarding',
'@storybook/addon-essentials',
'@storybook/addon-jest',
'@storybook/addon-actions',
'@storybook/addon-webpack5-compiler-babel',
]);

const filePathAddonNames = [
'/tmp/yarn_node_modules/7/1fb08592505aa9425e2d6d3ffdc05d84087e575a/yarn_install_node_modules/js/packages/config-storybook/node_modules/@storybook/addon-links/dist/cjs/index.js',
'/node_modules/@chromatic-com/storybook',
'../../../@storybook/addon-links',
'/C:/Users/foo/OneDrive%20-%20BAR%20BAZ/Desktop/project/node_modules/@storybook/addon-coverage',
'C:\\Users\\Foo\\AppData\\Local\\Yarn\\Berry\\cache\\@storybook-addon-measure-npm-7.6.21-61d2a610cb-10.zip\\node_modules\\@storybook\\addon-measure',
'/home/foo/project/node_modules/.pnpm/@storybook+addon-docs@10.0.2_@types+react@19.2.2_esbuild@0.25.10_rollup@4.31.0_storyboo_7cb8a1f4d4ca81d0abdb0f0cfacb0423/node_modules/@storybook/addon-docs',
'/home/foo/.yarn/berry/cache/@storybook-addon-interactions-npm-7.6.1-9e0ac1ff40-10.zip/node_modules/@storybook/addon-interactions',
'/Users/foo/project/node_modules/@storybook/addon-links/dist/cjs/index.js',
].map(sanitizeAddonName);

expect(filePathAddonNames).toEqual([
'@storybook/addon-links',
'@chromatic-com/storybook',
'@storybook/addon-links',
'@storybook/addon-coverage',
'@storybook/addon-measure',
'@storybook/addon-docs',
'@storybook/addon-interactions',
'@storybook/addon-links',
]);
});

it('custom, local, or non-node_modules paths', () => {
const addonNames = [
'file://$SNIP/.storybook/redesign-addon',
'$SNIP/addons/airtracker',
'$SNIP/preset/index',
'$SNIP/src',
'../../../tools/storybook/src/plugins/storybook-translations',
'./html-addon',
'..',
'file:///D:/foo/templates/.storybook/addons/some-addon',
].map(sanitizeAddonName);

expect(addonNames).toEqual([
'CUSTOM:redesign-addon',
'CUSTOM:airtracker',
'CUSTOM:preset',
'CUSTOM:src',
'CUSTOM:storybook-translations',
'CUSTOM:html-addon',
'CUSTOM:..',
'CUSTOM:some-addon',
]);
});
});
});

describe('computeStorybookMetadata', () => {
Expand Down
52 changes: 48 additions & 4 deletions code/core/src/telemetry/storybook-metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,56 @@ export const metaFrameworks = {
} as Record<string, string>;

export const sanitizeAddonName = (name: string) => {
return cleanPaths(name)
const normalized = name.replace(/\\/g, '/');

let candidate: string = normalized;

if (normalized.includes('/node_modules/')) {
// common case for package manager cache/pnp mode so we take the segment after node_modules
candidate = normalized.split('/node_modules/').pop() ?? normalized;
}

const cleaned = cleanPaths(candidate)
.replace(/^file:\/\//i, '')
.replace(/\/+$/, '')
.replace(/\/dist\/.*/, '')
.replace(/\.[mc]?[tj]?s[x]?$/, '')
.replace(/\/register$/, '')
.replace(/\/manager$/, '')
.replace(/\/preset$/, '');
.replace(/\/(register|manager|preset|index)$/, '')
.replace(/\$SNIP?/g, '');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Regex likely matches unintended pattern.

The regex \$SNIP? makes the P optional, so it matches both $SNI and $SNIP. Based on the cleanPaths function in sanitize.ts which produces $SNIP markers, this should likely be /\$SNIP/g without the ?.

Proposed fix
-    .replace(/\$SNIP?/g, '');
+    .replace(/\$SNIP/g, '');
📝 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
.replace(/\$SNIP?/g, '');
.replace(/\$SNIP/g, '');
🤖 Prompt for AI Agents
In `@code/core/src/telemetry/storybook-metadata.ts` at line 62, The replace call
in storybook-metadata.ts is using the regex `\$SNIP?` which makes the "P"
optional and can match unintended `$SNI`; update the replacement to target the
exact marker produced by sanitize.ts's cleanPaths (i.e., `$SNIP`) by changing
the regex used in the .replace invocation in storybook-metadata.ts so it matches
the full `$SNIP` token only (use a regex that matches the literal `$SNIP`
globally).


let prefix = '';
if (
cleaned.startsWith('file') ||
cleaned.startsWith('.') ||
cleaned.startsWith('/') ||
cleaned.includes(':')
) {
prefix = 'CUSTOM:';
}

const scopedMatches = cleaned.match(/@[^/]+\/[^/]+/g);
if (scopedMatches?.length) {
return scopedMatches.at(-1) as string;
}

const parts = cleaned.split('/').filter(Boolean);
const addonLike = [...parts]
.reverse()
.find((part) => part.includes('addon-') || part.includes('-addon'));

if (addonLike) {
return `${prefix}${addonLike}`;
}

if (parts.length >= 2 && parts[parts.length - 2].startsWith('@')) {
return `${prefix}${parts[parts.length - 2]}/${parts[parts.length - 1]}`;
}

if (parts.length) {
return `${prefix}${parts[parts.length - 1]}`;
}

return `${prefix}${candidate}`;
};

// Analyze a combination of information from main.js and package.json
Expand Down
Loading