diff --git a/scripts/check-engine-parity.ts b/scripts/check-engine-parity.ts index f4b75f764c..8f095ce8c5 100644 --- a/scripts/check-engine-parity.ts +++ b/scripts/check-engine-parity.ts @@ -89,18 +89,62 @@ export const SHARES_MEANINGFUL_FILE_MARKERS = Object.freeze([ "diffFilePriority(path) < 4", ] as const); +/** `review-enrichment/src/analyzers/secret-scan.ts` (REES) is a genuinely separate, deliberately WIDER + * advisory copy (deploys standalone on Railway with its own tsconfig/build/test pipeline — see that file's + * own header) of the shared hard-block primitives now in `src/review/secret-patterns.ts` (#4608). Unlike + * the other named pairs above, REES is NOT meant to converge toward byte-identical — a full-file comparison + * would immediately false-fail on REES's 80+ extra rules — so this pair's markers cover only the narrow, + * explicitly shared subset (the isPlaceholderSecretValue algorithm + the kind names both sides agree on). */ +export const SECRET_DETECTION_TWIN_PAIR: NamedTwinPair = Object.freeze({ + area: "secret-detection", + hostRelative: "src/review/secret-patterns.ts", + engineRelative: "review-enrichment/src/analyzers/secret-scan.ts", + hostFileName: "secret-patterns.ts", + engineFileName: "secret-scan.ts", +}); + +// isPlaceholderSecretValue's signature + full body (one marker per line, so a merely-reformatted-but- +// equivalent body doesn't false-fail) plus the HARD_SECRET_KINDS name literals that are EXACT string matches +// against REES's `kind` values today. `private_key_block`/`aws_access_key` are deliberately EXCLUDED: REES +// names the same two concepts `private_key`/`aws_access_key_id` (a pre-existing, out-of-scope naming +// divergence) — including them here would false-fail this check on the very PR that introduces it. +export const SECRET_DETECTION_MARKERS = Object.freeze([ + "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);", + '"github_token"', + '"github_pat"', + '"slack_token"', + '"google_api_key"', + '"gitlab_token"', + '"npm_token"', + '"stripe_secret_key"', + '"sendgrid_key"', + '"huggingface_token"', + '"voyage_api_key"', + '"firecrawl_api_key"', + '"jwt"', + '"generic_secret_assignment"', +] as const); + /** Every explicitly named twin pair, checked for core-marker presence in `runEngineParityChecks` — the * same escape hatch #4518 built for `GATE_DECISION_TWIN_PAIR`, generalized (#4605) so a function-level or * nested-directory duplicate can be added here without inventing a new mechanism. `GATE_DECISION_TWIN_PAIR` * additionally gets the co-edit-or-version-bump enforcement (`checkGateDecisionVersionBump`) since its two * sides are deliberately maintained as structurally divergent implementations; the other pairs here are * meant to stay much closer to byte-identical, so presence-check plus a content marker on the specific - * historically-drifted value (see `DIFF_FILE_PRIORITY_MARKERS`) is the proportionate guard for now. */ + * historically-drifted value (see `DIFF_FILE_PRIORITY_MARKERS`) is the proportionate guard for now — + * `SECRET_DETECTION_TWIN_PAIR` is the exception (see its own doc comment): its two sides are expected to + * diverge everywhere EXCEPT the explicitly shared marker subset. */ export const NAMED_TWIN_PAIRS: ReadonlyArray<{ pair: NamedTwinPair; markers: readonly string[] }> = Object.freeze([ { pair: GATE_DECISION_TWIN_PAIR, markers: GATE_DECISION_CORE_MARKERS }, { pair: SAFE_URL_TWIN_PAIR, markers: SAFE_URL_MARKERS }, { pair: DIFF_FILE_PRIORITY_TWIN_PAIR, markers: DIFF_FILE_PRIORITY_MARKERS }, { pair: SHARES_MEANINGFUL_FILE_TWIN_PAIR, markers: SHARES_MEANINGFUL_FILE_MARKERS }, + { pair: SECRET_DETECTION_TWIN_PAIR, markers: SECRET_DETECTION_MARKERS }, ]); const ENGINE_SRC_ROOT = "packages/gittensory-engine/src"; const HOST_SRC_ROOT = "src"; diff --git a/src/review/content-lane/security-scan.ts b/src/review/content-lane/security-scan.ts index 5c9a5ea020..152253aa61 100644 --- a/src/review/content-lane/security-scan.ts +++ b/src/review/content-lane/security-scan.ts @@ -1,8 +1,8 @@ // Deterministic security/abuse scan for content submissions (content-lane primitive). // // SELF-CONTAINED NATIVE PORT (reviewbot→gittensory convergence). Byte-faithful to reviewbot's -// src/agents/awesome-claude/security-scan.ts + the shared core/secrets-scan.ts (inlined here so the -// module is self-contained). PURE — data in, data out, no I/O. +// src/agents/awesome-claude/security-scan.ts + the shared core/secrets-scan.ts. PURE — data in, data out, +// no I/O. // // Design principle (learned via adversarial review): the gate AUTO-CLOSES at high confidence with NO // human queue, so a false-positive close PERMANENTLY rejects a legitimate submission — the worst @@ -11,104 +11,19 @@ // (pipe-to-shell installers, prompt-injection prose, "exfil-looking" code) is indistinguishable at // the regex level from legitimate documentation or defensive-security tooling — so it routes to // MANUAL (a human decides), never an auto-close. - -// ── Inlined secret-pattern scanner (reviewbot core/secrets-scan.ts) ─────────────────────────── -const SECRET_PATTERNS: Array<{ name: string; re: RegExp }> = [ - { name: "github_token", re: /\bgh[pousr]_[A-Za-z0-9]{20,}\b/ }, - { name: "github_pat", re: /\bgithub_pat_[A-Za-z0-9_]{20,}\b/ }, - { name: "private_key_block", re: /-----BEGIN(?: RSA| EC| OPENSSH| PGP| DSA)? PRIVATE KEY-----/ }, - { name: "aws_access_key", re: /\bAKIA[0-9A-Z]{16}\b/ }, - { name: "slack_token", re: /\bxox[baprs]-[A-Za-z0-9-]{10,}\b/ }, - { name: "google_api_key", re: /\bAIza[0-9A-Za-z_-]{35}\b/ }, - { name: "gitlab_token", re: /\bglpat-[0-9A-Za-z_-]{20}(?![0-9A-Za-z_-])/ }, - { name: "npm_token", re: /\bnpm_[A-Za-z0-9]{36}\b/ }, - // Stripe live secret / restricted keys: `sk_live_` / `rk_live_` + >=24 base62. - { name: "stripe_secret_key", re: /\b(?:sk|rk)_live_[0-9A-Za-z]{24,}\b/ }, - // SendGrid API key: `SG.` + 22-char id + `.` + 43-char secret (base64url). - { 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 }, -]; - -// Deliberately NOT in SECRET_PATTERNS above: unlike the format-specific patterns (a real GitHub token/AWS key -// ALWAYS matches its exact character format, so a bare .test() is precise enough), a keyword-plus-quoted-value -// SHAPE also matches plenty of non-secrets -- a Zod schema field (`password: z.string()`), a TypeScript type -// declaration, or a placeholder value ("xxx", "your-api-key-here", ""). Captured so each match's -// VALUE can be checked against isPlaceholderSecretValue before counting as a hit; the value itself is never -// returned from this module (only the kind name), preserving the existing never-echo-the-secret guarantee. -const GENERIC_SECRET_ASSIGNMENT_PATTERN = - /(?:api[_-]?key|secret|token|password|passwd|access[_-]?key|client[_-]?secret)["']?\s*[:=]\s*["']([A-Za-z0-9+/=_-]{16,})["']/gi; - -const PLACEHOLDER_VALUE_PATTERN = /placeholder|change[_-]?me|your[_-]|<[^>]*>|\bexample\b|redacted|dummy|\bsample\b|\btodo\b|\bfixme\b|\binsert\b|replace[_-]?me|\bfake\b/i; - -// A string with NO repeated characters (e.g. "abcdefghijklmnop123") has HIGH Shannon entropy by raw -// character-frequency counting, but is obviously not a real secret -- entropy alone only measures frequency, -// not ORDER, so a keyboard-sequential/alphabetical run slips past a pure distinct-character-count check. Detect -// the longest run of consecutive ascending or descending character codes (e.g. "abcdefg" or "9876543") and -// treat a long one as a human-constructed test value, not a randomly generated credential -- real API -// keys/tokens essentially never contain a 6+ character monotonic run. -const MIN_SEQUENTIAL_RUN_LENGTH = 6; -function hasLongSequentialRun(value: string): boolean { - let ascendingRun = 1; - let descendingRun = 1; - for (let i = 1; i < value.length; i += 1) { - const diff = value.charCodeAt(i) - value.charCodeAt(i - 1); - ascendingRun = diff === 1 ? ascendingRun + 1 : 1; - descendingRun = diff === -1 ? descendingRun + 1 : 1; - if (ascendingRun >= MIN_SEQUENTIAL_RUN_LENGTH || descendingRun >= MIN_SEQUENTIAL_RUN_LENGTH) return true; - } - 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]+)*$/; - -// #4579-followup (metagraphed/gittensory#4524 "token = default-session-token"/"beta-session-token", -// awesome-claude#4758 "embedded_secret: unsafe_install_or_secret" -- both confirmed live, no real secret -// present): a value whose OWN last hyphen/underscore-separated segment is itself one of the same secret-shaped -// trigger words reads as a NAME for a concept ("this is a kind of token/secret"), not an opaque credential -- -// a real generated token/key value never ends by literally restating what kind of thing it is. Deliberately -// NARROWER than "any multi-segment lowercase phrase": a Diceware-style passphrase like -// "alpha-bravo-charlie-delta" doesn't end in a trigger word, so it still correctly flags -- only values that -// self-identify as a token/secret/key/password NAME are excluded. -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 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); -} - -function hasGenericSecretAssignment(text: string): boolean { - // No zero-length-match / lastIndex-stall guard needed: the pattern's captured value alone requires 16+ - // characters, so every match is well over 16 characters long and lastIndex always advances past match.index. - GENERIC_SECRET_ASSIGNMENT_PATTERN.lastIndex = 0; - let match: RegExpExecArray | null; - while ((match = GENERIC_SECRET_ASSIGNMENT_PATTERN.exec(text)) !== null) { - // The pattern's sole capturing group is mandatory (not `?`/`*`-wrapped), so it is always present - // whenever the overall match succeeds -- non-null by construction, not a runtime branch. - if (!isPlaceholderSecretValue(match[1]!)) return true; - } - return false; -} +// +// #4608: the format-specific patterns + placeholder-value heuristics used to be inlined here (a second, +// independent copy of src/review/secrets-scan.ts's primitives). Both files live under src/, same build, +// same deploy — no deploy-independence reason to hand-duplicate them; that duplication already caused two +// independent, currently-live drifts (see #4587/#4604). Now imported from the shared ../secret-patterns. + +import { + GENERIC_SECRET_ASSIGNMENT_PATTERN, + HARD_SECRET_KINDS, + hasGenericSecretAssignment, + isPlaceholderSecretValue, + SECRET_PATTERNS, +} from "../secret-patterns"; export interface SecretScanResult { found: boolean; @@ -135,31 +50,6 @@ export interface SecurityFinding { // for the pipe-to-shell install check (manual-flag) and the first-party grounding relaxation. export const EXECUTABLE_CATEGORIES = new Set(["skills", "agents", "commands", "hooks", "mcp", "statuslines"]); -// Concrete credential formats only — NOT the weak heuristics (seed phrase / hot|coldkey) that would -// 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. 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", - "private_key_block", - "aws_access_key", - "slack_token", - "google_api_key", - "gitlab_token", - "npm_token", - "stripe_secret_key", - "sendgrid_key", - "huggingface_token", - "voyage_api_key", - "firecrawl_api_key", - "jwt", - "generic_secret_assignment", -]); - // A literal pipe-to-shell install. Common in legitimate installers (uv/rustup/deno/nvm), so this is a // MANUAL flag for a human, never an auto-close. const PIPED_INSTALL_RE = /\b(?:curl|wget)\b[^\n|]*\|\s*(?:sudo\s+)?(?:sh|bash|zsh|fish|python3?|node)\b/i; diff --git a/src/review/safety.ts b/src/review/safety.ts index 4be608fe91..34964cfca7 100644 --- a/src/review/safety.ts +++ b/src/review/safety.ts @@ -7,6 +7,7 @@ import type { AdvisoryFinding } from "../types"; import { neutralizePromptInjection, safeReviewTitle } from "./prompt-injection"; +import { HARD_SECRET_KINDS } from "./secret-patterns"; import { scanDiffForSecretsWithLocations } from "./secrets-scan"; // Concrete credential formats only — NOT the weak heuristics (`seed_or_mnemonic` / `bittensor_key`) that @@ -14,31 +15,11 @@ import { scanDiffForSecretsWithLocations } from "./secrets-scan"; // "mnemonic" in a .toml, .github/workflows/**, or wrangler/workers config is NOT a leaked credential, but it // matches those two patterns — on these Bittensor repos that wrongly hard-blocked owner config/workflow PRs // (RC6: #1505/#1495/#1485). A real-format token IS a leak regardless of the file it lives in, so we keep the -// concrete formats as hard blockers and ignore only the ambiguous heuristics. This mirrors the same gate the -// content lane already uses (src/review/content-lane/security-scan.ts). -// -// #2553: widened to match review-enrichment/src/analyzers/secret-scan.ts's richer, higher-recall rule set. -// google_api_key/jwt are as format-precise as the original five (near-zero false-positive risk). -// generic_secret_assignment is the one keyword-shaped pattern here — secrets-scan.ts already excludes -// placeholder/type-declaration/schema-shaped matches (see isPlaceholderSecretValue there) before this kind -// is ever produced, so it is safe to treat as an unconditional hard blocker like the rest. -const HARD_SECRET_KINDS = new Set([ - "github_token", - "github_pat", - "private_key_block", - "aws_access_key", - "slack_token", - "google_api_key", - "gitlab_token", - "npm_token", - "stripe_secret_key", - "sendgrid_key", - "huggingface_token", - "voyage_api_key", - "firecrawl_api_key", - "jwt", - "generic_secret_assignment", -]); +// concrete formats as hard blockers and ignore only the ambiguous heuristics. HARD_SECRET_KINDS is shared +// (#4608) with the same gate the content lane uses (src/review/content-lane/security-scan.ts) via +// ./secret-patterns — google_api_key/jwt/generic_secret_assignment (#2553) and voyage_api_key/ +// firecrawl_api_key (#4604) are as format-precise as the original five, so all are safe unconditional hard +// blockers; see that module's header for the full reasoning. /** True when the safety scan is enabled. Flag-OFF (default) → every helper below is a no-op pass-through. */ export function isSafetyEnabled(env: { diff --git a/src/review/secret-patterns.ts b/src/review/secret-patterns.ts new file mode 100644 index 0000000000..7b1005d5e7 --- /dev/null +++ b/src/review/secret-patterns.ts @@ -0,0 +1,143 @@ +// Shared secret-detection primitives (#4608). Deterministic, no deps. +// +// Extracted out of src/review/secrets-scan.ts (PR-diff hard-block, via src/review/safety.ts) and +// src/review/content-lane/security-scan.ts (content-lane hard-block, for awesome-claude/metagraphed +// submissions) — both live under src/, ship in the same build/deploy, and had no deploy-independence reason +// to be hand-duplicated. That duplication already caused two independent, currently-live drifts (missing +// mock carve-out + missing voyage/firecrawl kinds, see #4604) despite a same-day commit (3307ae097, #4587) +// editing both copies for one change — there was no automated pairing between the two files. +// +// review-enrichment/src/analyzers/secret-scan.ts (REES) is deliberately NOT imported here and stays a +// genuinely separate, wider copy: REES deploys standalone on Railway with its own tsconfig/build/test +// pipeline, so importing across that boundary would break its independence (the same reasoning +// secrets-scan.ts's own header documents for staying self-contained relative to reviewbot). REES's +// isPlaceholderSecretValue body and the kind names it shares with HARD_SECRET_KINDS below are instead +// drift-checked mechanically — see scripts/check-engine-parity.ts's SECRET_DETECTION_TWIN_PAIR. + +export const SECRET_PATTERNS: Array<{ name: string; re: RegExp }> = [ + { name: "github_token", re: /\bgh[pousr]_[A-Za-z0-9]{20,}\b/ }, + { name: "github_pat", re: /\bgithub_pat_[A-Za-z0-9_]{20,}\b/ }, + { name: "private_key_block", re: /-----BEGIN(?: RSA| EC| OPENSSH| PGP| DSA)? PRIVATE KEY-----/ }, + { name: "aws_access_key", re: /\bAKIA[0-9A-Z]{16}\b/ }, + { name: "slack_token", re: /\bxox[baprs]-[A-Za-z0-9-]{10,}\b/ }, + { name: "google_api_key", re: /\bAIza[0-9A-Za-z_-]{35}\b/ }, + { name: "gitlab_token", re: /\bglpat-[0-9A-Za-z_-]{20}(?![0-9A-Za-z_-])/ }, + { name: "npm_token", re: /\bnpm_[A-Za-z0-9]{36}\b/ }, + // Stripe live secret / restricted keys: `sk_live_` / `rk_live_` + >=24 base62. + { name: "stripe_secret_key", re: /\b(?:sk|rk)_live_[0-9A-Za-z]{24,}\b/ }, + // SendGrid API key: `SG.` + 22-char id + `.` + 43-char secret (base64url). + { 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 }, +]; + +// Deliberately NOT in SECRET_PATTERNS above: unlike the format-specific patterns (a real GitHub token/AWS key +// ALWAYS matches its exact character format, so a bare .test() is precise enough), a keyword-plus-quoted-value +// SHAPE also matches plenty of non-secrets -- a Zod schema field (`password: z.string()`), a TypeScript type +// declaration, or a placeholder value ("xxx", "your-api-key-here", ""). The value is captured (group +// 1) so it can be checked against isPlaceholderSecretValue before counting as a hit; the value itself is never +// returned from this module (only the kind name), preserving the never-echo-the-secret guarantee. +export const GENERIC_SECRET_ASSIGNMENT_PATTERN = + /(?:api[_-]?key|secret|token|password|passwd|access[_-]?key|client[_-]?secret)["']?\s*[:=]\s*["']([A-Za-z0-9+/=_-]{16,})["']/gi; + +const PLACEHOLDER_VALUE_PATTERN = /placeholder|change[_-]?me|your[_-]|<[^>]*>|\bexample\b|redacted|dummy|\bsample\b|\btodo\b|\bfixme\b|\binsert\b|replace[_-]?me|\bfake\b/i; + +// #2553 gate review finding: a string with NO repeated characters (e.g. "abcdefghijklmnop123") has HIGH +// Shannon entropy by raw character-frequency counting, but is obviously not a real secret -- entropy alone +// only measures frequency, not ORDER, so a keyboard-sequential/alphabetical run slips past a pure distinct- +// character-count check. Detect the longest run of consecutive ascending or descending character codes (e.g. +// "abcdefg" or "9876543") and treat a long one as a human-constructed test value, not a randomly generated +// credential -- real API keys/tokens essentially never contain a 6+ character monotonic run. +const MIN_SEQUENTIAL_RUN_LENGTH = 6; +export function hasLongSequentialRun(value: string): boolean { + let ascendingRun = 1; + let descendingRun = 1; + for (let i = 1; i < value.length; i += 1) { + const diff = value.charCodeAt(i) - value.charCodeAt(i - 1); + ascendingRun = diff === 1 ? ascendingRun + 1 : 1; + descendingRun = diff === -1 ? descendingRun + 1 : 1; + if (ascendingRun >= MIN_SEQUENTIAL_RUN_LENGTH || descendingRun >= MIN_SEQUENTIAL_RUN_LENGTH) return true; + } + 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]+)*$/; + +// #4579-followup (metagraphed/gittensory#4524 "token = default-session-token"/"beta-session-token", +// awesome-claude#4758 "embedded_secret: unsafe_install_or_secret" -- both confirmed live, no real secret +// present): a value whose OWN last hyphen/underscore-separated segment is itself one of the same secret-shaped +// trigger words reads as a NAME for a concept ("this is a kind of token/secret"), not an opaque credential -- +// a real generated token/key value never ends by literally restating what kind of thing it is. Deliberately +// NARROWER than "any multi-segment lowercase phrase": a Diceware-style passphrase like +// "alpha-bravo-charlie-delta" doesn't end in a trigger word, so it still correctly flags (regression guard +// below) -- only values that self-identify as a token/secret/key/password NAME are excluded. +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"). Mirrored (drift-checked, not imported) in + * review-enrichment/src/analyzers/secret-scan.ts — see this file's header. */ +export 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); +} + +/** True when `text` contains a keyword-plus-quoted-value assignment (see GENERIC_SECRET_ASSIGNMENT_PATTERN) + * whose value clears isPlaceholderSecretValue. The one shared implementation of "does this text contain a + * generic secret assignment", used by both secrets-scan.ts's matchedKindsIn and + * content-lane/security-scan.ts's scanForSecrets. */ +export function hasGenericSecretAssignment(text: string): boolean { + // No zero-length-match / lastIndex-stall guard needed: the pattern's captured value alone requires 16+ + // characters, so every match is well over 16 characters long and lastIndex always advances past match.index. + GENERIC_SECRET_ASSIGNMENT_PATTERN.lastIndex = 0; + let match: RegExpExecArray | null; + while ((match = GENERIC_SECRET_ASSIGNMENT_PATTERN.exec(text)) !== null) { + // The captured value group is mandatory (not `?`/`*`-wrapped), so it is always present whenever the + // overall match succeeds -- non-null by construction, not a runtime branch. + if (!isPlaceholderSecretValue(match[1]!)) return true; + } + return false; +} + +// Concrete credential formats only -- NOT the weak heuristics (seed_or_mnemonic / bittensor_key) that would +// false-positive on legitimate Bittensor content (a `coldkey:` / `hotkey =` line or the word "mnemonic" in a +// .toml, .github/workflows/**, or wrangler/workers config is not a leaked credential; RC6: #1505/#1495/#1485). +// #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 above) before the kind is ever produced, so all three are safe unconditional hard +// blockers. voyage_api_key/firecrawl_api_key (#4604) are equally format-precise. Shared by both hard-block +// paths: src/review/safety.ts's secretLeakFinding (PR-diff) and +// src/review/content-lane/security-scan.ts's firstSecretLine/scanLinkedBodiesForSecrets (content-lane). +export const HARD_SECRET_KINDS = new Set([ + "github_token", + "github_pat", + "private_key_block", + "aws_access_key", + "slack_token", + "google_api_key", + "gitlab_token", + "npm_token", + "stripe_secret_key", + "sendgrid_key", + "huggingface_token", + "voyage_api_key", + "firecrawl_api_key", + "jwt", + "generic_secret_assignment", +]); diff --git a/src/review/secrets-scan.ts b/src/review/secrets-scan.ts index c1af35e412..56ac17521a 100644 --- a/src/review/secrets-scan.ts +++ b/src/review/secrets-scan.ts @@ -1,113 +1,20 @@ // Reusable secret-pattern scanner (the `secretsScan` capability). Deterministic, no deps. // Callers run scanForSecrets() on submitted diff/text; a hit typically forces a close/manual verdict. // -// SELF-CONTAINED NATIVE PORT (reviewbot→gittensory convergence): every type + pattern this module needs is -// defined HERE. No imports from reviewbot. The logic is byte-faithful to the reviewbot source -// (src/core/secrets-scan.ts); there are no stricter-tsconfig deltas — the module is already total. +// SELF-CONTAINED NATIVE PORT (reviewbot→gittensory convergence): byte-faithful to the reviewbot source +// (src/core/secrets-scan.ts); there are no stricter-tsconfig deltas — the module is already total. No +// imports from reviewbot. // -// #2553: widened to match review-enrichment/src/analyzers/secret-scan.ts's richer, higher-recall rule set -// (google_api_key, jwt, generic_secret_assignment) so the deterministic hard blocker (safety.ts's -// HARD_SECRET_KINDS) catches the same patterns REES's advisory-only enrichment brief already does. Kept as a -// second, independent copy here rather than a cross-package import: review-enrichment deploys standalone on -// Railway with its own tsconfig/build/test pipeline (see review-enrichment/package.json), so importing across -// that boundary would break its independence — the same reasoning this file's own header already documents -// for staying self-contained relative to reviewbot. +// #4608: the format-specific patterns + placeholder-value heuristics are shared with +// content-lane/security-scan.ts via ./secret-patterns (both live under src/, same build, same deploy — no +// deploy-independence reason to hand-duplicate them; that duplication already caused two independent, +// currently-live drifts, see #4587/#4604). review-enrichment/src/analyzers/secret-scan.ts (REES) stays a +// genuinely separate, deliberately wider copy — not imported here — because REES deploys standalone on +// Railway with its own tsconfig/build/test pipeline; the same reasoning this file used to document for +// itself before the #4608 extraction. See ./secret-patterns's header and +// scripts/check-engine-parity.ts's SECRET_DETECTION_TWIN_PAIR for how REES's copy is kept from drifting. -const SECRET_PATTERNS: Array<{ name: string; re: RegExp }> = [ - { name: "github_token", re: /\bgh[pousr]_[A-Za-z0-9]{20,}\b/ }, - { name: "github_pat", re: /\bgithub_pat_[A-Za-z0-9_]{20,}\b/ }, - { name: "private_key_block", re: /-----BEGIN(?: RSA| EC| OPENSSH| PGP| DSA)? PRIVATE KEY-----/ }, - { name: "aws_access_key", re: /\bAKIA[0-9A-Z]{16}\b/ }, - { name: "slack_token", re: /\bxox[baprs]-[A-Za-z0-9-]{10,}\b/ }, - { name: "google_api_key", re: /\bAIza[0-9A-Za-z_-]{35}\b/ }, - { name: "gitlab_token", re: /\bglpat-[0-9A-Za-z_-]{20}(?![0-9A-Za-z_-])/ }, - { name: "npm_token", re: /\bnpm_[A-Za-z0-9]{36}\b/ }, - // Stripe live secret / restricted keys: `sk_live_` / `rk_live_` + >=24 base62. - { name: "stripe_secret_key", re: /\b(?:sk|rk)_live_[0-9A-Za-z]{24,}\b/ }, - // SendGrid API key: `SG.` + 22-char id + `.` + 43-char secret (base64url). - { 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 }, -]; - -// Deliberately NOT in SECRET_PATTERNS above: unlike the format-specific patterns (a real GitHub token/AWS key -// ALWAYS matches its exact character format, so a bare .test() is precise enough), a keyword-plus-quoted-value -// SHAPE also matches plenty of non-secrets -- a Zod schema field (`password: z.string()`), a TypeScript type -// declaration, or a placeholder value ("xxx", "your-api-key-here", ""). Captured so each match's -// VALUE can be checked against isPlaceholderSecretValue before counting as a hit; the value itself is never -// returned from this module (only the kind name), preserving the existing never-echo-the-secret guarantee. -const GENERIC_SECRET_ASSIGNMENT_PATTERN = - /((?:api[_-]?key|secret|token|password|passwd|access[_-]?key|client[_-]?secret))["']?\s*[:=]\s*["']([A-Za-z0-9+/=_-]{16,})["']/gi; - -const PLACEHOLDER_VALUE_PATTERN = /placeholder|change[_-]?me|your[_-]|<[^>]*>|\bexample\b|redacted|dummy|\bsample\b|\btodo\b|\bfixme\b|\binsert\b|replace[_-]?me|\bfake\b/i; - -// #2553 gate review finding: a string with NO repeated characters (e.g. "abcdefghijklmnop123") has HIGH -// Shannon entropy by raw character-frequency counting, but is obviously not a real secret -- entropy alone -// only measures frequency, not ORDER, so a keyboard-sequential/alphabetical run slips past a pure distinct- -// character-count check. Detect the longest run of consecutive ascending or descending character codes (e.g. -// "abcdefg" or "9876543") and treat a long one as a human-constructed test value, not a randomly generated -// credential -- real API keys/tokens essentially never contain a 6+ character monotonic run. -const MIN_SEQUENTIAL_RUN_LENGTH = 6; -function hasLongSequentialRun(value: string): boolean { - let ascendingRun = 1; - let descendingRun = 1; - for (let i = 1; i < value.length; i += 1) { - const diff = value.charCodeAt(i) - value.charCodeAt(i - 1); - ascendingRun = diff === 1 ? ascendingRun + 1 : 1; - descendingRun = diff === -1 ? descendingRun + 1 : 1; - if (ascendingRun >= MIN_SEQUENTIAL_RUN_LENGTH || descendingRun >= MIN_SEQUENTIAL_RUN_LENGTH) return true; - } - 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]+)*$/; - -// #4579-followup (metagraphed/gittensory#4524 "token = default-session-token"/"beta-session-token", -// awesome-claude#4758 "embedded_secret: unsafe_install_or_secret" -- both confirmed live, no real secret -// present): a value whose OWN last hyphen/underscore-separated segment is itself one of the same secret-shaped -// trigger words reads as a NAME for a concept ("this is a kind of token/secret"), not an opaque credential -- -// a real generated token/key value never ends by literally restating what kind of thing it is. Deliberately -// NARROWER than "any multi-segment lowercase phrase": a Diceware-style passphrase like -// "alpha-bravo-charlie-delta" doesn't end in a trigger word, so it still correctly flags (regression guard -// below) -- only values that self-identify as a token/secret/key/password NAME are excluded. -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"). */ -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); -} - -function hasGenericSecretAssignment(text: string): boolean { - // No zero-length-match / lastIndex-stall guard needed: the pattern's captured value alone requires 16+ - // characters, so every match is well over 16 characters long and lastIndex always advances past match.index. - GENERIC_SECRET_ASSIGNMENT_PATTERN.lastIndex = 0; - let match: RegExpExecArray | null; - while ((match = GENERIC_SECRET_ASSIGNMENT_PATTERN.exec(text)) !== null) { - // The key and value groups are mandatory (not `?`/`*`-wrapped), so both are always present - // whenever the overall match succeeds -- non-null by construction, not runtime branches. - if (!isPlaceholderSecretValue(match[2]!)) return true; - } - return false; -} +import { hasGenericSecretAssignment, SECRET_PATTERNS } from "./secret-patterns"; // #3041: the one place the pattern list (format-specific SECRET_PATTERNS + the generic keyword-assignment // heuristic) is applied to a string. Both `scanForSecrets` (whole-text scan) and diff --git a/test/unit/check-engine-parity-script.test.ts b/test/unit/check-engine-parity-script.test.ts index e14742c353..6660e82ed0 100644 --- a/test/unit/check-engine-parity-script.test.ts +++ b/test/unit/check-engine-parity-script.test.ts @@ -31,6 +31,8 @@ import { runEngineParityMain, SAFE_URL_MARKERS, SAFE_URL_TWIN_PAIR, + SECRET_DETECTION_MARKERS, + SECRET_DETECTION_TWIN_PAIR, SHARES_MEANINGFUL_FILE_MARKERS, SHARES_MEANINGFUL_FILE_TWIN_PAIR, } from "../../scripts/check-engine-parity"; @@ -242,13 +244,14 @@ describe("check-engine-parity script", () => { }); describe("named twin-pair coverage (#4605)", () => { - it("registers the gate-decision, safe-url, diff-file-priority, and shares-meaningful-file pairs", () => { + it("registers the gate-decision, safe-url, diff-file-priority, shares-meaningful-file, and secret-detection pairs", () => { const areas = NAMED_TWIN_PAIRS.map(({ pair }) => pair.area); expect(areas).toEqual([ "gate-decision", "content-lane", "diff-file-priority", "shares-meaningful-file", + "secret-detection", ]); }); @@ -262,7 +265,7 @@ describe("check-engine-parity script", () => { expect(scanned.some((discovered) => discovered.fileName === "safe-url.ts")).toBe(false); }); - it("passes marker presence for all four named pairs against the real repo (regression guard)", () => { + it("passes marker presence for all five named pairs against the real repo (regression guard)", () => { for (const { pair, markers } of NAMED_TWIN_PAIRS) { const result = checkGateDecisionTwinPresence({ root: process.cwd(), pair, markers }); expect(result.failures).toEqual([]); @@ -307,7 +310,7 @@ describe("check-engine-parity script", () => { ); }); - it("includes all four named pairs in runEngineParityChecks pairsChecked", () => { + it("includes all five named pairs in runEngineParityChecks pairsChecked", () => { const result = runEngineParityChecks({ root: process.cwd() }); const checkedAreas = result.pairsChecked.map((pair) => pair.area); for (const { pair } of NAMED_TWIN_PAIRS) { @@ -316,6 +319,77 @@ describe("check-engine-parity script", () => { }); }); + describe("secret-detection twin coverage (#4608)", () => { + it("pairs src/review/secret-patterns.ts with REES's genuinely-separate, wider copy", () => { + expect(SECRET_DETECTION_TWIN_PAIR.hostRelative).toBe("src/review/secret-patterns.ts"); + expect(SECRET_DETECTION_TWIN_PAIR.engineRelative).toBe( + "review-enrichment/src/analyzers/secret-scan.ts", + ); + // Not discoverable by the generic src/{review,settings,signals} <-> packages/gittensory-engine scan: + // REES lives under review-enrichment/, a different root entirely. + const scanned = discoverEngineParityPairs({ root: process.cwd() }); + expect(scanned.some((discovered) => discovered.fileName === "secret-patterns.ts")).toBe(false); + }); + + it("does not include the two kind names known to be named differently on REES's side (would false-fail)", () => { + // REES calls these `private_key` / `aws_access_key_id` rather than + // `private_key_block` / `aws_access_key` -- a pre-existing, out-of-scope naming divergence. Asserting + // their ABSENCE here documents the deliberate omission and guards against someone "completing the + // set" and reintroducing a false-fail. + expect(SECRET_DETECTION_MARKERS).not.toContain('"private_key_block"'); + expect(SECRET_DETECTION_MARKERS).not.toContain('"aws_access_key"'); + }); + + it("fails presence when the shared isPlaceholderSecretValue algorithm drifts on one side", () => { + // Reproduces the drift class #4608 exists to catch: one side silently drops a placeholder-detection + // exclusion rule (here, the mock-fixture carve-out) while the other keeps it. Both bodies start from + // the FULL marker set (join, not a hand-picked subset) so only the deliberately dropped line + // produces a failure -- a partial fixture would false-report every marker it happens to omit too. + const droppedMarker = "if (LOWERCASE_HYPHENATED_MOCK_FIXTURE_PATTERN.test(value)) return true;"; + const hostBody = SECRET_DETECTION_MARKERS.join("\n"); + const driftedEngineBody = SECRET_DETECTION_MARKERS.filter((marker) => marker !== droppedMarker).join("\n"); + const result = checkGateDecisionTwinPresence({ + root: "/fake", + readFile: (_root, relativePath) => { + if (relativePath === SECRET_DETECTION_TWIN_PAIR.hostRelative) return hostBody; + if (relativePath === SECRET_DETECTION_TWIN_PAIR.engineRelative) return driftedEngineBody; + throw new Error(`unexpected read: ${relativePath}`); + }, + pair: SECRET_DETECTION_TWIN_PAIR, + markers: SECRET_DETECTION_MARKERS, + }); + expect(result.failures).toHaveLength(1); + expect(result.failures[0]).toContain(SECRET_DETECTION_TWIN_PAIR.engineRelative); + expect(result.failures[0]).toContain(JSON.stringify(droppedMarker)); + }); + + it("fails presence when a shared kind name is removed from REES's rule set", () => { + const body = [ + "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);", + ].join("\n"); + const hostKinds = SECRET_DETECTION_MARKERS.filter((marker) => marker.startsWith('"')).join("\n"); + const engineKindsMissingVoyage = hostKinds.replace('"voyage_api_key"\n', ""); + const result = checkGateDecisionTwinPresence({ + root: "/fake", + readFile: (_root, relativePath) => { + if (relativePath === SECRET_DETECTION_TWIN_PAIR.hostRelative) return `${body}\n${hostKinds}`; + if (relativePath === SECRET_DETECTION_TWIN_PAIR.engineRelative) return `${body}\n${engineKindsMissingVoyage}`; + throw new Error(`unexpected read: ${relativePath}`); + }, + pair: SECRET_DETECTION_TWIN_PAIR, + markers: SECRET_DETECTION_MARKERS, + }); + expect(result.failures).toHaveLength(1); + expect(result.failures[0]).toContain(SECRET_DETECTION_TWIN_PAIR.engineRelative); + expect(result.failures[0]).toContain(JSON.stringify('"voyage_api_key"')); + }); + }); + describe("engine version skew", () => { it("classifies equal, behind, and ahead boundary cases", () => { expect(compareSemver("0.2.0", "0.2.0")).toBe(0); diff --git a/test/unit/secret-patterns.test.ts b/test/unit/secret-patterns.test.ts new file mode 100644 index 0000000000..26175db99a --- /dev/null +++ b/test/unit/secret-patterns.test.ts @@ -0,0 +1,147 @@ +import { describe, expect, it } from "vitest"; +import { + GENERIC_SECRET_ASSIGNMENT_PATTERN, + HARD_SECRET_KINDS, + hasGenericSecretAssignment, + hasLongSequentialRun, + isPlaceholderSecretValue, + SECRET_PATTERNS, +} from "../../src/review/secret-patterns"; + +// Direct unit coverage of the shared module extracted in #4608. secrets-scan.test.ts and +// content-lane-security-scan.test.ts already exercise these primitives exhaustively THROUGH their two +// callers' public scanForSecrets()/scanSubmissionContent() surfaces (kept there, unmodified, as the +// no-behavior-change regression guard for the extraction itself) — this file tests the primitives directly, +// at the layer they now actually live at. + +describe("secret-patterns — shared secret-detection primitives (#4608)", () => { + describe("SECRET_PATTERNS / HARD_SECRET_KINDS", () => { + it("SECRET_PATTERNS is a non-empty array of uniquely named patterns", () => { + expect(SECRET_PATTERNS.length).toBeGreaterThan(0); + const names = SECRET_PATTERNS.map((pattern) => pattern.name); + expect(new Set(names).size).toBe(names.length); + }); + + it("HARD_SECRET_KINDS excludes the weak seed-phrase/bittensor-key heuristics", () => { + expect(HARD_SECRET_KINDS.has("seed_or_mnemonic")).toBe(false); + expect(HARD_SECRET_KINDS.has("bittensor_key")).toBe(false); + expect(HARD_SECRET_KINDS.has("generic_secret_assignment")).toBe(true); + }); + + it("every non-generic HARD_SECRET_KINDS entry is a real SECRET_PATTERNS name", () => { + const patternNames = new Set(SECRET_PATTERNS.map((pattern) => pattern.name)); + for (const kind of HARD_SECRET_KINDS) { + if (kind === "generic_secret_assignment") continue; + expect(patternNames.has(kind)).toBe(true); + } + }); + }); + + describe("hasLongSequentialRun", () => { + it("returns false when the value is too short to reach the threshold", () => { + expect(hasLongSequentialRun("")).toBe(false); + expect(hasLongSequentialRun("a")).toBe(false); + expect(hasLongSequentialRun("ab1")).toBe(false); + }); + + it("detects an ascending monotonic run right at the 6-char threshold, not one short of it", () => { + expect(hasLongSequentialRun("abcdef")).toBe(true); + expect(hasLongSequentialRun("abcde")).toBe(false); + }); + + it("detects a descending monotonic run right at the 6-char threshold, not one short of it", () => { + expect(hasLongSequentialRun("fedcba")).toBe(true); + expect(hasLongSequentialRun("fedcb")).toBe(false); + }); + + it("resets the run counter when the sequence breaks, but still catches a later run", () => { + expect(hasLongSequentialRun("abcXdefghi")).toBe(true); // "defghi" tail is a fresh 6-run + expect(hasLongSequentialRun("acegikmoqs")).toBe(false); // constant +2 stride, never +1/-1 + }); + + it("does not mistake a high-entropy, non-monotonic credential-shaped value for a sequential run", () => { + expect(hasLongSequentialRun("aK9xQ2mZw7Ln4Rv8Pt3Bh6")).toBe(false); + }); + }); + + describe("isPlaceholderSecretValue", () => { + it("flags a known placeholder phrase", () => { + expect(isPlaceholderSecretValue("your-api-key-placeholder")).toBe(true); + }); + + it("flags a value built from at most 2 distinct characters", () => { + expect(isPlaceholderSecretValue("xxxxxxxxxxxxxxxxxxxx")).toBe(true); + expect(isPlaceholderSecretValue("----------------")).toBe(true); + }); + + it("flags a lowercase-hyphenated mock fixture name", () => { + expect(isPlaceholderSecretValue("mock-response-value")).toBe(true); + expect(isPlaceholderSecretValue("some-mock-secret-value")).toBe(true); + }); + + it("does NOT flag a mixed-case/digit-bearing mock-tokenized value (still a plausible credential)", () => { + expect(isPlaceholderSecretValue("mock-aK9xQ2mZw7Ln4Rv8Pt3Bh6")).toBe(false); + }); + + it("flags a lowercase identifier whose own last segment self-names as a secret kind", () => { + expect(isPlaceholderSecretValue("default-session-token")).toBe(true); + expect(isPlaceholderSecretValue("unsafe_install_or_secret")).toBe(true); + }); + + it("does NOT flag a self-naming-suffix-shaped value once digits/mixed case break the ALL-lowercase check", () => { + expect(isPlaceholderSecretValue("session2024-token")).toBe(false); + }); + + it("does NOT flag a multi-segment lowercase passphrase that does not self-name as a secret kind", () => { + expect(isPlaceholderSecretValue("alpha-bravo-charlie-delta")).toBe(false); + }); + + it("flags a long monotonic character-code run (ascending or descending)", () => { + expect(isPlaceholderSecretValue("abcdefghijklmnop123")).toBe(true); + expect(isPlaceholderSecretValue("zyxwvutsrqponmlkj987")).toBe(true); + }); + + it("does NOT flag a genuinely high-entropy credential-shaped value", () => { + expect(isPlaceholderSecretValue("aK9xQ2mZw7Ln4Rv8Pt3Bh6")).toBe(false); + }); + }); + + describe("GENERIC_SECRET_ASSIGNMENT_PATTERN", () => { + it("captures the value directly in group 1 (no wrapping keyword group)", () => { + GENERIC_SECRET_ASSIGNMENT_PATTERN.lastIndex = 0; + const match = GENERIC_SECRET_ASSIGNMENT_PATTERN.exec('token = "aK9xQ2mZw7Ln4Rv8Pt3Bh6"'); + expect(match?.[1]).toBe("aK9xQ2mZw7Ln4Rv8Pt3Bh6"); + }); + }); + + describe("hasGenericSecretAssignment", () => { + it("returns true for a keyword-plus-quoted-value assignment with a high-entropy value", () => { + expect(hasGenericSecretAssignment('secret = "aK9xQ2mZw7Ln4Rv8Pt3Bh6"')).toBe(true); + }); + + it("returns false for benign text with no assignment shape at all", () => { + expect(hasGenericSecretAssignment("just a normal sentence")).toBe(false); + }); + + it("returns false when the only candidate value is a placeholder (loop exhausts with no hit)", () => { + expect(hasGenericSecretAssignment('token = "your-api-key-placeholder"')).toBe(false); + }); + + it("finds a match anywhere in a longer text, not just at the start", () => { + expect(hasGenericSecretAssignment('benign prose first.\nsecret = "aK9xQ2mZw7Ln4Rv8Pt3Bh6"')).toBe(true); + }); + + it("resets the shared regex's lastIndex on every call, so a prior call cannot corrupt the next scan", () => { + // GENERIC_SECRET_ASSIGNMENT_PATTERN is a module-level /g regex; a call that returns true early leaves + // lastIndex at the END of that match. This first call's match runs to the end of a 33-char string. + const first = 'secret = "aK9xQ2mZw7Ln4Rv8Pt3Bh6"'; + expect(first).toHaveLength(33); + expect(hasGenericSecretAssignment(first)).toBe(true); + // Without the explicit `lastIndex = 0` reset at the top of hasGenericSecretAssignment, this second, + // SHORTER (31-char) string would be scanned starting past its own end and wrongly report no match. + const second = 'token = "zQ8wN2pL6vX4mK9jH3fR7"'; + expect(second).toHaveLength(31); + expect(hasGenericSecretAssignment(second)).toBe(true); + }); + }); +});