diff --git a/.changeset/fix-artifact-env-leak-1738.md b/.changeset/fix-artifact-env-leak-1738.md new file mode 100644 index 0000000000..b78511e75f --- /dev/null +++ b/.changeset/fix-artifact-env-leak-1738.md @@ -0,0 +1,10 @@ +--- +"oxlint-plugin-react-doctor": patch +"@react-doctor/core": patch +--- + +Fix false positives in `artifact-env-leak` rule for vendor library examples and intentionally-public tokens + +- Extends comment masking to handle sourcemap JSON, masking comments in `node_modules` sources to prevent false positives from vendor library JSDoc examples (e.g., `@reatom/core` DATABASE_URL documentation) +- Adds `_[A-Z0-9]+_PUBLIC_(TOKEN|KEY|SECRET)` pattern to trusted public env names, exempting intentionally-public tokens like `VITE_STYTCH_PUBLIC_TOKEN` that pair a public-env prefix with `PUBLIC` as a distinct infix +- Pattern requires at least one component between underscore and `PUBLIC` to avoid matching the `PUBLIC` in `NEXT_PUBLIC_`/`EXPO_PUBLIC_` prefixes themselves (e.g., `NEXT_PUBLIC_SECRET_TOKEN` still correctly flags) diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/constants/security.ts b/packages/oxlint-plugin-react-doctor/src/plugin/constants/security.ts index 9d4dd17d2b..5b877fbe98 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/constants/security.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/constants/security.ts @@ -213,13 +213,20 @@ export const FULL_ENV_LEAK_SECRET_NAME_PATTERN = // TODO(follow-up): de-overfit — several vendor names here mirror specific // regression fixtures (TLDRAW / POSTHOG / ALGOLIA / GC_API_KEY). // `FACEBOOK_CLIENT_TOKEN` is Meta's designated client-embeddable token, -// documented as safe to ship in app binaries. The trailing metadata-suffix -// branch exempts names that only REFERENCE a credential (`…_TOKEN_KIND`, -// `…_TOKEN_URL`, `…_TOKEN_ENDPOINT`): their values are labels, URLs, or -// header names, not credentials. The suffix must directly follow the secret -// keyword so `DATABASE_URL` (the URL IS the secret) keeps firing. +// documented as safe to ship in app binaries. The `_[A-Z0-9]+_PUBLIC_` +// infix pattern exempts intentionally-public tokens following common +// naming conventions (Stytch `VITE_STYTCH_PUBLIC_TOKEN`, Stripe +// `NEXT_PUBLIC_STRIPE_PUBLIC_KEY`) that pair a public-env prefix with +// "PUBLIC" as a distinct infix (not as part of the prefix itself). The +// pattern requires at least one component between the leading underscore +// and PUBLIC to avoid matching the PUBLIC in NEXT_PUBLIC_ / EXPO_PUBLIC_ +// prefixes themselves. The trailing metadata-suffix branch exempts names +// that only REFERENCE a credential (`…_TOKEN_KIND`, `…_TOKEN_URL`, +// `…_TOKEN_ENDPOINT`): their values are labels, URLs, or header names, not +// credentials. The suffix must directly follow the secret keyword so +// `DATABASE_URL` (the URL IS the secret) keeps firing. export const TRUSTED_PUBLIC_SECRET_NAME_PATTERN = - /(?:SENTRY_DSN|PUBLIC_KEY|PUBLISHABLE|ANON_KEY|POSTHOG_(?:PROJECT_)?TOKEN|POSTHOG_KEY|TLDRAW_LICENSE_KEY|CLERK_PUBLISHABLE_KEY|ALGOLIA_SEARCH_KEY|GC_API_KEY|GOOGLE_MAPS_API_KEY|MAPBOX_TOKEN|MIXPANEL_TOKEN|FACEBOOK_CLIENT_TOKEN|(?:NEXT_PUBLIC|VITE|REACT_APP|EXPO_PUBLIC)_(?:DISABLE|ENABLE|ALLOW|REQUIRE)_)|(?:TOKEN|SECRET|PASSWORD|PRIVATE)_(?:KIND|TYPE|URL|URI|ENDPOINT|HEADER|NAME)$/i; + /(?:SENTRY_DSN|PUBLIC_KEY|PUBLISHABLE|ANON_KEY|_[A-Z0-9]+_PUBLIC_(?:TOKEN|KEY|SECRET)|POSTHOG_(?:PROJECT_)?TOKEN|POSTHOG_KEY|TLDRAW_LICENSE_KEY|CLERK_PUBLISHABLE_KEY|ALGOLIA_SEARCH_KEY|GC_API_KEY|GOOGLE_MAPS_API_KEY|MAPBOX_TOKEN|MIXPANEL_TOKEN|FACEBOOK_CLIENT_TOKEN|(?:NEXT_PUBLIC|VITE|REACT_APP|EXPO_PUBLIC)_(?:DISABLE|ENABLE|ALLOW|REQUIRE)_)|(?:TOKEN|SECRET|PASSWORD|PRIVATE)_(?:KIND|TYPE|URL|URI|ENDPOINT|HEADER|NAME)$/i; // Public, client-safe keys designed to ship in the browser, each with a // prefix distinct from the same vendor's secret key (RevenueCat `appl_` diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/artifact-env-leak.regressions.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/artifact-env-leak.regressions.test.ts index cf3c156a8c..c7a7d13480 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/artifact-env-leak.regressions.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/artifact-env-leak.regressions.test.ts @@ -213,4 +213,83 @@ export const client = {};`, }); expect(findings).toHaveLength(0); }); + + it("stays silent on DATABASE_URL in node_modules JSDoc via sourcemap sourcesContent (#1738)", () => { + const sourcemapContent = JSON.stringify({ + version: 3, + sources: ["../../node_modules/@reatom/core/dist/index.js", "../../src/config.ts"], + sourcesContent: [ + '/** Example:\n * const db = dbVar.set(process.env.DATABASE_URL)\n */\nexport function connect() {}', + 'export const token = import.meta.env.VITE_STYTCH_PUBLIC_TOKEN;' + ], + mappings: "AAAA;AACA", + names: [] + }); + + const findings = runScanRule(artifactEnvLeak, { + relativePath: "dist/assets/index-BUe3yDzZ.js.map", + content: sourcemapContent, + isGeneratedBundle: true, + }); + + expect(findings).toHaveLength(0); + }); + + it("still flags executable DATABASE_URL in app source via sourcemap", () => { + const sourcemapContent = JSON.stringify({ + version: 3, + sources: ["../../src/server/db.ts"], + sourcesContent: ['export const databaseUrl = process.env.DATABASE_URL;'], + mappings: "AAAA", + names: [] + }); + + const findings = runScanRule(artifactEnvLeak, { + relativePath: "dist/assets/index-abc.js.map", + content: sourcemapContent, + isGeneratedBundle: true, + }); + + expect(findings.length).toBeGreaterThan(0); + }); + + it("stays silent on VITE_*_PUBLIC_TOKEN (intentionally public) (#1738)", () => { + const findings = runScanRule(artifactEnvLeak, { + relativePath: "dist/assets/index-abc.js", + content: 'const token = "VITE_STYTCH_PUBLIC_TOKEN";', + isGeneratedBundle: true, + }); + + expect(findings).toHaveLength(0); + }); + + it("stays silent on NEXT_PUBLIC_*_PUBLIC_KEY (intentionally public)", () => { + const findings = runScanRule(artifactEnvLeak, { + relativePath: "dist/assets/index-abc.js", + content: 'const key = "NEXT_PUBLIC_STRIPE_PUBLIC_KEY";', + isGeneratedBundle: true, + }); + + expect(findings).toHaveLength(0); + }); + + it("stays silent on EXPO_PUBLIC_*_PUBLIC_SECRET (intentionally public despite 'secret' keyword)", () => { + const findings = runScanRule(artifactEnvLeak, { + relativePath: "dist/assets/index-abc.js", + content: 'const secret = "EXPO_PUBLIC_AUTH_PUBLIC_SECRET";', + isGeneratedBundle: true, + }); + + expect(findings).toHaveLength(0); + }); + + it("correctly exempts VITE_*_PUBLISHABLE_* which is already trusted", () => { + const findings = runScanRule(artifactEnvLeak, { + relativePath: "dist/assets/index-abc.js", + content: 'const token = "VITE_MY_PUBLISHABLE_KEY";', + isGeneratedBundle: true, + }); + + expect(findings).toHaveLength(0); + }); }); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/mask-source-comments.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/mask-source-comments.ts index 92b993506e..b627e92142 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/mask-source-comments.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/security-scan/utils/mask-source-comments.ts @@ -2,27 +2,57 @@ import { parseSync } from "oxc-parser"; import { resolveLang } from "../../../utils/parse-source-file.js"; const SOURCE_FILE_EXTENSION_PATTERN = /\.(?:[cm]?[jt]sx?)$/i; +const SOURCEMAP_EXTENSION_PATTERN = /\.map$/i; const POSSIBLE_SOURCE_COMMENT_PATTERN = /\/\/|\/\*|", searchIndex); - if (closingCommentIndex === -1) return false; - let prefixIndex = closingCommentIndex - 1; - while (prefixIndex >= 0 && !LINE_TERMINATORS.has(content[prefixIndex] ?? "")) { - if (content[prefixIndex]?.trim() !== "") break; - prefixIndex -= 1; - } - if (prefixIndex < 0 || LINE_TERMINATORS.has(content[prefixIndex] ?? "")) return true; - searchIndex = closingCommentIndex + 3; +interface SourceMap { + sources?: string[]; + sourcesContent?: (string | null)[]; +} + +const maskSourceMapContent = (content: string): string | undefined => { + try { + const sourcemap = JSON.parse(content) as SourceMap; + if (!sourcemap.sources || !sourcemap.sourcesContent) return content; + if (sourcemap.sources.length !== sourcemap.sourcesContent.length) return content; + + let didMaskAny = false; + const maskedSourcesContent = sourcemap.sourcesContent.map((sourceContent, index) => { + if (sourceContent === null) return null; + const sourcePath = sourcemap.sources?.[index] ?? ""; + if (!sourcePath.includes("node_modules")) return sourceContent; + + const syntheticPath = sourcePath.endsWith(".ts") + ? "synthetic.ts" + : sourcePath.endsWith(".tsx") + ? "synthetic.tsx" + : sourcePath.endsWith(".jsx") + ? "synthetic.jsx" + : sourcePath.endsWith(".mts") + ? "synthetic.mts" + : sourcePath.endsWith(".cts") + ? "synthetic.cts" + : sourcePath.endsWith(".mjs") + ? "synthetic.mjs" + : sourcePath.endsWith(".cjs") + ? "synthetic.cjs" + : "synthetic.js"; + + const masked = maskSingleSource(syntheticPath, sourceContent); + if (masked !== sourceContent && masked !== undefined) didMaskAny = true; + return masked ?? sourceContent; + }); + + if (!didMaskAny) return content; + + return JSON.stringify({ ...sourcemap, sourcesContent: maskedSourcesContent }); + } catch { + return content; } - return false; }; -export const maskSourceComments = (relativePath: string, content: string): string | undefined => { - if (!SOURCE_FILE_EXTENSION_PATTERN.test(relativePath)) return content; +const maskSingleSource = (relativePath: string, content: string): string | undefined => { if ( !content.startsWith("#!") && !POSSIBLE_SOURCE_COMMENT_PATTERN.test(content) && @@ -63,3 +93,28 @@ export const maskSourceComments = (relativePath: string, content: string): strin return undefined; } }; + +const hasPossibleAnnexBClosingComment = (content: string): boolean => { + let searchIndex = 0; + while (searchIndex < content.length) { + const closingCommentIndex = content.indexOf("-->", searchIndex); + if (closingCommentIndex === -1) return false; + let prefixIndex = closingCommentIndex - 1; + while (prefixIndex >= 0 && !LINE_TERMINATORS.has(content[prefixIndex] ?? "")) { + if (content[prefixIndex]?.trim() !== "") break; + prefixIndex -= 1; + } + if (prefixIndex < 0 || LINE_TERMINATORS.has(content[prefixIndex] ?? "")) return true; + searchIndex = closingCommentIndex + 3; + } + return false; +}; + +export const maskSourceComments = (relativePath: string, content: string): string | undefined => { + if (SOURCEMAP_EXTENSION_PATTERN.test(relativePath)) { + return maskSourceMapContent(content); + } + if (!SOURCE_FILE_EXTENSION_PATTERN.test(relativePath)) return content; + + return maskSingleSource(relativePath, content); +};