Skip to content
Draft
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
10 changes: 10 additions & 0 deletions .changeset/fix-artifact-env-leak-1738.md
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -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_`
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
import { describe, expect, it } from "vite-plus/test";
import { runScanRule } from "../../../test-utils/run-scan-rule.js";
import { artifactEnvLeak } from "./artifact-env-leak.js";
Expand Down Expand Up @@ -213,4 +213,83 @@
});
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);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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 = /\/\/|\/\*|<!--/;
const LINE_TERMINATORS = new Set(["\r", "\n", "\u2028", "\u2029"]);

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;
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) &&
Expand Down Expand Up @@ -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);
};
Loading