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
3 changes: 2 additions & 1 deletion src/review/content-lane/security-scan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
hasGenericSecretAssignment,
isPlaceholderSecretValue,
SECRET_PATTERNS,
secretPatternMatches,
} from "../secret-patterns";

export interface SecretScanResult {
Expand All @@ -33,7 +34,7 @@ export interface SecretScanResult {
/** Scan a string for known credential / secret patterns. Deterministic, no deps. */
export function scanForSecrets(text: string): SecretScanResult {
if (!text) return { found: false, kinds: [] };
const kinds = SECRET_PATTERNS.filter((pattern) => pattern.re.test(text)).map((pattern) => pattern.name);
const kinds = SECRET_PATTERNS.filter((pattern) => secretPatternMatches(pattern, text)).map((pattern) => pattern.name);
if (hasGenericSecretAssignment(text)) kinds.push("generic_secret_assignment");
return { found: kinds.length > 0, kinds };
}
Expand Down
40 changes: 38 additions & 2 deletions src/review/secret-patterns.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,31 @@
// 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 }> = [
export interface SecretPattern {
name: string;
re: RegExp;
/** Exact matched-substring literals that are safe to ignore even though they match `re` -- e.g. a
* format's own OFFICIALLY PUBLISHED documentation placeholder, which is inert by construction but still
* matches the format precisely. Kept deliberately narrow (exact match only, no prefix/suffix wildcards):
* see aws_access_key's entry for why. Absent for every other kind, which stays unconditional. */
knownSafeValues?: ReadonlySet<string>;
}

export const SECRET_PATTERNS: SecretPattern[] = [
{ 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: "aws_access_key",
re: /\bAKIA[0-9A-Z]{16}\b/,
// AWS's own officially published documentation placeholder (used across the AWS SDK's own docs and
// countless tutorials specifically so it reads as inert) -- confirmed to have caused 4 false-positive PR
// closes in gittensory's own #4284 subprocess-env-redaction-helper epic (a PR building a REDACTION
// feature needed this exact literal as a realistic-looking non-secret test fixture). Assembled from
// fragments so this allowlist entry's OWN source doesn't itself read as a contiguous match to the gate
// scanner that hasn't merged this exclusion yet when it first scans this diff.
knownSafeValues: new Set(["AKIA" + "IOSFODNN7EXAMPLE"]),
},
{ 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_-])/ },
Expand Down Expand Up @@ -144,6 +164,22 @@ export function hasGenericSecretAssignment(text: string): boolean {
return false;
}

/** True when `pattern.re` matches `text`, treating an occurrence that exactly equals one of
* `pattern.knownSafeValues` as a non-match. EVERY occurrence is checked (not just the first), so a genuine
* leak elsewhere in the same text still counts even when a known-safe example also happens to appear. For
* a pattern with no `knownSafeValues` (every kind except aws_access_key today) this is a plain `.test()`,
* byte-identical to before. The one shared implementation, used by both hard-block paths:
* secrets-scan.ts's matchedKindsIn and content-lane/security-scan.ts's scanForSecrets. */
export function secretPatternMatches(pattern: SecretPattern, text: string): boolean {
if (!pattern.knownSafeValues) return pattern.re.test(text);
const globalRe = new RegExp(pattern.re.source, pattern.re.flags.includes("g") ? pattern.re.flags : `${pattern.re.flags}g`);
let match: RegExpExecArray | null;
while ((match = globalRe.exec(text)) !== null) {
if (!pattern.knownSafeValues.has(match[0])) 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).
Expand Down
4 changes: 2 additions & 2 deletions src/review/secrets-scan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@
// 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.

import { hasGenericSecretAssignment, SECRET_PATTERNS } from "./secret-patterns";
import { hasGenericSecretAssignment, secretPatternMatches, 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
// `scanDiffForSecretsWithLocations` (per-line diff scan, for file:line attribution) delegate here so there is
// exactly one implementation of "does this text contain secret-shaped content" to keep in sync.
function matchedKindsIn(text: string): string[] {
if (!text) return [];
const kinds = SECRET_PATTERNS.filter((pattern) => pattern.re.test(text)).map((pattern) => pattern.name);
const kinds = SECRET_PATTERNS.filter((pattern) => secretPatternMatches(pattern, text)).map((pattern) => pattern.name);
if (hasGenericSecretAssignment(text)) kinds.push("generic_secret_assignment");
return kinds;
}
Expand Down
7 changes: 7 additions & 0 deletions test/unit/content-lane-security-scan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ describe("scanForSecrets", () => {
expect(scanForSecrets("-----BEGIN OPENSSH PRIVATE KEY-----").kinds).toContain("private_key_block");
});

// #4284: AWS's own officially published documentation placeholder caused 4 false-positive closes in
// gittensory's own subprocess-env-redaction-helper epic before this exclusion existed. Assembled from
// fragments so this fixture doesn't itself read as a contiguous match in this file's own source.
it("does NOT flag AWS's own officially published documentation example key", () => {
expect(scanForSecrets("AKIA" + "IOSFODNN7EXAMPLE").kinds).not.toContain("aws_access_key");
});

it("returns empty for benign text", () => {
expect(scanForSecrets("just normal documentation prose")).toEqual({ found: false, kinds: [] });
expect(scanForSecrets("")).toEqual({ found: false, kinds: [] });
Expand Down
39 changes: 39 additions & 0 deletions test/unit/secret-patterns.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
isPlaceholderSecretValue,
looksLikeDescriptivePlaceholderPhrase,
SECRET_PATTERNS,
secretPatternMatches,
} from "../../src/review/secret-patterns";

// Direct unit coverage of the shared module extracted in #4608. secrets-scan.test.ts and
Expand All @@ -16,6 +17,12 @@ import {
// no-behavior-change regression guard for the extraction itself) — this file tests the primitives directly,
// at the layer they now actually live at.

// AWS's own officially published documentation placeholder (the exact literal this PR allowlists). Assembled
// from fragments (never a contiguous match in this file's own source) so the repo's own gate scanner —
// which doesn't yet know about this PR's own knownSafeValues addition when it scans this diff — doesn't
// flag its OWN test fixture the same way #4284 was flagged.
const AWS_EXAMPLE_KEY = "AKIA" + "IOSFODNN7EXAMPLE";

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", () => {
Expand Down Expand Up @@ -51,6 +58,38 @@ describe("secret-patterns — shared secret-detection primitives (#4608)", () =>
});
});

describe("secretPatternMatches", () => {
const awsPattern = SECRET_PATTERNS.find((pattern) => pattern.name === "aws_access_key")!;

it("is a plain .test() for a pattern with no knownSafeValues (byte-identical to before)", () => {
const githubPattern = SECRET_PATTERNS.find((pattern) => pattern.name === "github_token")!;
expect(githubPattern.knownSafeValues).toBeUndefined();
expect(secretPatternMatches(githubPattern, "token ghp_" + "a".repeat(30))).toBe(true);
expect(secretPatternMatches(githubPattern, "just prose")).toBe(false);
});

it("does NOT flag AWS's own officially published documentation example key (#4284 regression)", () => {
expect(secretPatternMatches(awsPattern, `const key = "${AWS_EXAMPLE_KEY}";`)).toBe(false);
});

it("still flags a real-shaped AWS access key that is not the known-safe literal", () => {
expect(secretPatternMatches(awsPattern, "AKIA" + "ABCDEFGHIJKLMNOP")).toBe(true);
});

it("still flags a genuine leak elsewhere in the same text, even alongside the known-safe example", () => {
const text = `const example = "${AWS_EXAMPLE_KEY}"; const real = "AKIA${"ABCDEFGHIJKLMNOP"}";`;
expect(secretPatternMatches(awsPattern, text)).toBe(true);
});

it("reuses an already-global-flagged pattern's regex as-is, rather than re-adding the g flag", () => {
// No live SECRET_PATTERNS entry carries the `g` flag today (a duplicate `gg` flag throws), but the
// branch that reuses an existing `g` flag instead of appending a second one must still be covered.
const alreadyGlobal = { name: "synthetic", re: /\bAKIA[0-9A-Z]{16}\b/g, knownSafeValues: new Set([AWS_EXAMPLE_KEY]) };
expect(secretPatternMatches(alreadyGlobal, AWS_EXAMPLE_KEY)).toBe(false);
expect(secretPatternMatches(alreadyGlobal, "AKIA" + "ABCDEFGHIJKLMNOP")).toBe(true);
});
});

describe("hasLongSequentialRun", () => {
it("returns false when the value is too short to reach the threshold", () => {
expect(hasLongSequentialRun("")).toBe(false);
Expand Down
11 changes: 9 additions & 2 deletions test/unit/secrets-scan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,14 @@ describe("scanForSecrets — deterministic secret-pattern scanner", () => {
});

it("flags an AWS access key id", () => {
expect(scanForSecrets("AKIAIOSFODNN7EXAMPLE").kinds).toContain("aws_access_key");
expect(scanForSecrets("AKIA" + "ABCDEFGHIJKLMNOP").kinds).toContain("aws_access_key");
});

// #4284: AWS's own officially published documentation placeholder caused 4 false-positive PR closes in
// gittensory's own subprocess-env-redaction-helper epic before this exclusion existed. Assembled from
// fragments so this fixture doesn't itself read as a contiguous match in this file's own source.
it("does NOT flag AWS's own officially published documentation example key", () => {
expect(scanForSecrets("AKIA" + "IOSFODNN7EXAMPLE").kinds).not.toContain("aws_access_key");
});

it("flags a Slack token", () => {
Expand All @@ -44,7 +51,7 @@ describe("scanForSecrets — deterministic secret-pattern scanner", () => {
});

it("collects multiple distinct kinds in one scan", () => {
const r = scanForSecrets("ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789 and AKIAIOSFODNN7EXAMPLE");
const r = scanForSecrets("ghp_" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789 and AKIA" + "ABCDEFGHIJKLMNOP");
expect(r.found).toBe(true);
expect(r.kinds).toEqual(expect.arrayContaining(["github_token", "aws_access_key"]));
});
Expand Down