From 0da7bcacfab2d9b2b379665a2acc695dbdcb29e8 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Fri, 10 Jul 2026 01:13:01 -0700 Subject: [PATCH] fix(review): port secret-scan mock carve-out and voyage/firecrawl kinds (#4604) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit content-lane/security-scan.ts had drifted from its two siblings (secrets-scan.ts, review-enrichment's secret-scan.ts): it was missing the LOWERCASE_HYPHENATED_MOCK_FIXTURE_PATTERN placeholder carve-out added by #3866, so a fixture value like `token: "mock-response-value"` would auto-close a legitimate content-lane submission with no human queue to catch the false positive. It was also missing the voyage_api_key and firecrawl_api_key patterns added to secrets-scan.ts by #3980, so a real Voyage/Firecrawl key embedded in a content submission produced no finding at all. Ports both fixes verbatim into the content-lane copy and adds both kinds to its HARD_SECRET_KINDS set for auto-close parity with the PR-diff gate. No shared-module extraction here — that is tracked separately in #4608. --- src/review/content-lane/security-scan.ts | 23 +++++++-- test/unit/content-lane-security-scan.test.ts | 51 ++++++++++++++++++++ 2 files changed, 69 insertions(+), 5 deletions(-) diff --git a/src/review/content-lane/security-scan.ts b/src/review/content-lane/security-scan.ts index cf42231439..5c9a5ea020 100644 --- a/src/review/content-lane/security-scan.ts +++ b/src/review/content-lane/security-scan.ts @@ -28,6 +28,10 @@ const SECRET_PATTERNS: Array<{ name: string; re: RegExp }> = [ { name: "sendgrid_key", re: /\bSG\.[A-Za-z0-9_-]{22}\.[A-Za-z0-9_-]{43}(?![A-Za-z0-9_-])/ }, // Hugging Face user access token: `hf_` + 34 base62 chars. { name: "huggingface_token", re: /\bhf_[A-Za-z0-9]{34}\b/ }, + // Voyage AI API key: `pa-` (platform) or `al-` (MongoDB Atlas) + base62 body. + { name: "voyage_api_key", re: /\b(?:pa|al)-[A-Za-z0-9]{20,}(?![A-Za-z0-9_-])/ }, + // Firecrawl API key: `fc-` + base62 body (alnum only; reject hyphen-continued identifiers). + { name: "firecrawl_api_key", re: /\bfc-[A-Za-z0-9]{16,}(?![A-Za-z0-9_-])/ }, { name: "jwt", re: /\beyJ[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}\b/ }, { name: "seed_or_mnemonic", re: /\b(?:seed phrase|mnemonic)\b/i }, { name: "bittensor_key", re: /\b(?:hot|cold)key\b\s*[:=]/i }, @@ -63,6 +67,10 @@ function hasLongSequentialRun(value: string): boolean { return false; } +// Lowercase hyphenated mock names are fixtures; mixed-case/digit-bearing values containing "mock" remain +// plausible credentials and must still be reported by the generic assignment scanner. +const LOWERCASE_HYPHENATED_MOCK_FIXTURE_PATTERN = /^(?:[a-z]+-)*mock(?:-[a-z]+)*$/; + // All-lowercase-letters value check, shared by the self-naming-suffix exclusion below. const ALL_LOWERCASE_SEGMENTS_PATTERN = /^[a-z]+(?:[-_][a-z]+)*$/; @@ -77,12 +85,14 @@ const ALL_LOWERCASE_SEGMENTS_PATTERN = /^[a-z]+(?:[-_][a-z]+)*$/; const SELF_NAMING_FIXTURE_SUFFIX_PATTERN = /[-_](?:token|secret|key|password|passwd)$/i; /** True for an obvious non-secret filler value: a known placeholder phrase, a string built from at most 2 - * distinct characters (e.g. "xxxxxxxxxxxxxxxx", "----------------"), a long monotonic character-code run - * (e.g. "abcdefghijklmnop123"), or a lowercase identifier whose own last segment self-names as a secret kind - * (e.g. "default-session-token", "unsafe_install_or_secret") — real high-entropy secrets never look like any of these. */ + * distinct characters (e.g. "xxxxxxxxxxxxxxxx", "----------------"), a lowercase-hyphenated mock/fixture name + * (e.g. "mock-response-value"), a long monotonic character-code run (e.g. "abcdefghijklmnop123"), or a + * lowercase identifier whose own last segment self-names as a secret kind (e.g. "default-session-token", + * "unsafe_install_or_secret") — real high-entropy secrets never look like any of these. */ function isPlaceholderSecretValue(value: string): boolean { if (PLACEHOLDER_VALUE_PATTERN.test(value)) return true; if (new Set(value.toLowerCase()).size <= 2) return true; + if (LOWERCASE_HYPHENATED_MOCK_FIXTURE_PATTERN.test(value)) return true; if (ALL_LOWERCASE_SEGMENTS_PATTERN.test(value) && SELF_NAMING_FIXTURE_SUFFIX_PATTERN.test(value)) return true; return hasLongSequentialRun(value); } @@ -129,8 +139,9 @@ export const EXECUTABLE_CATEGORIES = new Set(["skills", "agents", "commands", "h // false-positive on legitimate Bittensor content. #2553: google_api_key/jwt are as format-precise as the // original five (near-zero false-positive risk), and generic_secret_assignment already excludes // placeholder/type-declaration/schema-shaped matches (see isPlaceholderSecretValue) before the kind is ever -// produced, so all three are safe unconditional hard blockers — keeping this gate in parity with the PR-diff -// gate it mirrors (safety.ts's HARD_SECRET_KINDS / secrets-scan.ts). +// produced, so all three are safe unconditional hard blockers. voyage_api_key/firecrawl_api_key (#4604) are +// equally format-precise — keeping this gate in parity with the PR-diff gate it mirrors (safety.ts's +// HARD_SECRET_KINDS / secrets-scan.ts). const HARD_SECRET_KINDS = new Set([ "github_token", "github_pat", @@ -143,6 +154,8 @@ const HARD_SECRET_KINDS = new Set([ "stripe_secret_key", "sendgrid_key", "huggingface_token", + "voyage_api_key", + "firecrawl_api_key", "jwt", "generic_secret_assignment", ]); diff --git a/test/unit/content-lane-security-scan.test.ts b/test/unit/content-lane-security-scan.test.ts index ed35713b52..9925686f34 100644 --- a/test/unit/content-lane-security-scan.test.ts +++ b/test/unit/content-lane-security-scan.test.ts @@ -57,6 +57,28 @@ describe("scanForSecrets", () => { expect(scanForSecrets(`sg = "SG.${"a".repeat(22)}.${"b".repeat(42)}-"`).kinds).toContain("sendgrid_key"); }); + // #4604: content-lane was missing these two kinds entirely (present in secrets-scan.ts since #3980), + // so a real Voyage/Firecrawl key embedded in a content submission produced no finding at all. + it("flags Voyage AI API keys (#4604)", () => { + expect(scanForSecrets("pa-" + "aK9xQ2mZw7Ln4Rv8Pt3B").kinds).toContain("voyage_api_key"); + expect(scanForSecrets("al-" + "mN4pL8sT2vW6xY0A1qZ5").kinds).toContain("voyage_api_key"); + }); + + it("does not flag Voyage AI-shaped values below the length floor or with identifier continuation (#4604)", () => { + expect(scanForSecrets("pa-" + "a".repeat(19)).kinds).not.toContain("voyage_api_key"); + expect(scanForSecrets("pa-" + "a".repeat(20) + "-suffix").kinds).not.toContain("voyage_api_key"); + expect(scanForSecrets("al-" + "b".repeat(20) + "_suffix").kinds).not.toContain("voyage_api_key"); + }); + + it("flags a Firecrawl API key (#4604)", () => { + expect(scanForSecrets("fc-" + "aK9xQ2mZw7Ln4Rv8").kinds).toContain("firecrawl_api_key"); + }); + + it("does not flag Firecrawl-shaped values below the length floor or with identifier continuation (#4604)", () => { + expect(scanForSecrets("fc-" + "c".repeat(15)).kinds).not.toContain("firecrawl_api_key"); + expect(scanForSecrets("fc-" + "c".repeat(16) + "-suffix").kinds).not.toContain("firecrawl_api_key"); + }); + it("flags a generic secret/password/token assignment with a high-entropy value", () => { expect(scanForSecrets(`secret = "${GENERIC_VALUE}"`).kinds).toContain("generic_secret_assignment"); expect(scanForSecrets(`api_key: '${GENERIC_VALUE}'`).kinds).toContain("generic_secret_assignment"); @@ -80,6 +102,23 @@ describe("scanForSecrets", () => { expect(scanForSecrets("password: z.string()").kinds).not.toContain("generic_secret_assignment"); }); + // #4604: content-lane was missing the LOWERCASE_HYPHENATED_MOCK_FIXTURE_PATTERN carve-out that + // secrets-scan.ts already had (since #3866) — a lowercase-hyphenated "mock" fixture value would + // auto-close a legitimate content-lane submission with no human queue to catch the false positive. + it.each([ + ["mock-response-value", 'token: "mock-response-value"'], + ["prefixed mock fixture", 'secret: "some-mock-secret-value"'], + ])("does NOT flag a lowercase-hyphenated mock fixture value: %s (#4604)", (_name, snippet) => { + expect(scanForSecrets(snippet).kinds).not.toContain("generic_secret_assignment"); + }); + + it.each([ + ["mock prefix with mixed-case suffix", 'password = "mock-aK9xQ2mZw7Ln4Rv8Pt3Bh6"'], + ["embedded mock with mixed-case suffix", 'secret = "prod-mock-aK9xQ2mZw7Ln4Rv8Pt3Bh6"'], + ])("still flags mock-tokenized generic credentials unless they are lowercase fixtures: %s (#4604)", (_name, snippet) => { + expect(scanForSecrets(snippet).kinds).toContain("generic_secret_assignment"); + }); + // #4579-followup: confirmed live false positives (awesome-claude#4758 "embedded_secret: // unsafe_install_or_secret"; the same self-naming shape as metagraphed/gittensory#4524's // "token: default-session-token") -- neither is a real secret, both are enum/fixture NAMES whose own last @@ -183,6 +222,15 @@ describe("scanSubmissionContent", () => { } }); + it("hard-closes on a Voyage or Firecrawl API key (#4604)", () => { + for (const line of ["config: pa-" + "aK9xQ2mZw7Ln4Rv8Pt3B", "key: fc-" + "aK9xQ2mZw7Ln4Rv8"]) { + const finding = scanSubmissionContent({ content: `intro line\n${line}`, category: "skills" }); + expect(finding?.verdict, line).toBe("close"); + expect(finding?.reasonCode, line).toBe("embedded_secret"); + expect(finding?.summary, line).toContain("line 2"); + } + }); + it("hard-closes on a MULTILINE generic secret assignment whose value wraps to the next line (auto-close parity)", () => { // generic_secret_assignment is the one HARD kind whose keyword-to-value span can wrap. scanForSecrets over the // whole blob catches it; scanSubmissionContent must too, or a wrapped secret bypasses the auto-close gate the @@ -228,8 +276,11 @@ describe("secret-scan parity with the PR-diff gate (secrets-scan.ts)", () => { ["stripe_secret_key", "sk_live_" + "a".repeat(24)], ["sendgrid_key", "SG." + "a".repeat(22) + "." + "b".repeat(43)], ["huggingface_token", "hf_" + "a".repeat(34)], + ["voyage_api_key", "pa-" + "aK9xQ2mZw7Ln4Rv8Pt3B"], + ["firecrawl_api_key", "fc-" + "aK9xQ2mZw7Ln4Rv8"], ["jwt", jwt], ["generic_secret_assignment", `secret = "${GENERIC_VALUE}"`], + ["lowercase-hyphenated mock fixture (not flagged on either side)", 'token: "mock-response-value"'], ["benign prose", "just normal documentation prose"], ])("detects the same kinds as the PR-diff gate for %s", (_name, input) => { expect([...scanForSecrets(input).kinds].sort()).toEqual([...prDiffScanForSecrets(input).kinds].sort());