diff --git a/packages/gittensory-engine/src/index.ts b/packages/gittensory-engine/src/index.ts index 4adbe14d99..43b5f0cf32 100644 --- a/packages/gittensory-engine/src/index.ts +++ b/packages/gittensory-engine/src/index.ts @@ -49,6 +49,7 @@ export { computeLaneFit, type GoalModelInput, } from "./goal-model.js"; +export { classifyContributorFit, type ContributorFit, type ContributorFitCheck, diff --git a/packages/gittensory-engine/src/opportunity-freshness.ts b/packages/gittensory-engine/src/opportunity-freshness.ts index 943ee78f49..d23ef0ff02 100644 --- a/packages/gittensory-engine/src/opportunity-freshness.ts +++ b/packages/gittensory-engine/src/opportunity-freshness.ts @@ -26,10 +26,16 @@ function pickTimestamp(issue: FreshnessIssue): string | null { return null; } +// No usable timestamp survived pickTimestamp's updatedAt->createdAt fallback -- an unknown age must never +// register as "just updated" (age 0, the freshest possible score). Floor it to a large sentinel so +// computeOpportunityFreshness's exponential decay clamps straight to the 0.05 floor, matching how a genuinely +// stale issue scores, not a fresh one. +const UNKNOWN_AGE_DAYS = 9999; + function issueAgeDays(value: string | null, nowMs: number): number { - if (!value) return 0; + if (!value) return UNKNOWN_AGE_DAYS; const parsed = Date.parse(value); - if (!Number.isFinite(parsed)) return 0; + if (!Number.isFinite(parsed)) return UNKNOWN_AGE_DAYS; return Math.floor((nowMs - parsed) / 86_400_000); } diff --git a/src/review/safety.ts b/src/review/safety.ts index 9faf479b4a..995eb252e8 100644 --- a/src/review/safety.ts +++ b/src/review/safety.ts @@ -7,7 +7,7 @@ import type { AdvisoryFinding } from "../types"; import { neutralizePromptInjection, safeReviewTitle } from "./prompt-injection"; -import { scanForSecrets } from "./secrets-scan"; +import { scanPrDiffForSecretKinds } from "./secrets-scan"; // Concrete credential formats only — NOT the weak heuristics (`seed_or_mnemonic` / `bittensor_key`) that // false-positive on legitimate config/workflow content. A `coldkey:` / `hotkey =` line or the word @@ -98,21 +98,10 @@ export function secretLeakFinding(diff: string): AdvisoryFinding | null { // secret-shaped string (e.g. deleting/defanging a test fixture, or rotating a credential out). Added/renamed // file paths are also committed PR state, but buildSecretScanDiff carries them only in `### path (status)` // headers, so keep those metadata lines while still dropping modified/removed headers and `+++` patch headers. - const added = diff - .split("\n") - .filter( - (line) => - (line.startsWith("+") && !line.startsWith("+++")) || - /^### .+ \((?:added|renamed)\) /.test(line), - ) - .join("\n"); - // Only CONCRETE credential formats hard-block. The raw scanner also returns the weak `seed_or_mnemonic` / - // `bittensor_key` heuristics, which false-positive on `coldkey:` / `hotkey =` / "mnemonic" lines in - // legitimate config/workflow files (RC6); those are filtered out here so they never produce a `secret_leak` - // blocker. A real token (github_token, aws_access_key, …) still blocks regardless of which file it is in. - const kinds = scanForSecrets(added).kinds.filter((kind) => - HARD_SECRET_KINDS.has(kind), - ); + // scanPrDiffForSecretKinds walks the diff line-by-line (with a bounded cross-line literal join on consecutive + // added lines, #2454) instead of joining all `+` lines into one blob — that join would miss a credential + // split across adjacent assignments and would also ignore hunk/context boundaries the gate must respect. + const kinds = scanPrDiffForSecretKinds(diff).filter((kind) => HARD_SECRET_KINDS.has(kind)); if (kinds.length === 0) return null; return { code: "secret_leak", diff --git a/src/review/secrets-scan.ts b/src/review/secrets-scan.ts index 5603481805..6bc3d120e6 100644 --- a/src/review/secrets-scan.ts +++ b/src/review/secrets-scan.ts @@ -90,3 +90,107 @@ export function scanForSecrets(text: string): SecretScanResult { if (hasGenericSecretAssignment(text)) kinds.push("generic_secret_assignment"); return { found: kinds.length > 0, kinds }; } + +/** Extract quoted string-literal inner text from one source line (for cross-line join below). */ +function extractStringLiteralContents(line: string): string[] { + const literals: string[] = []; + const re = /"(?:[^"\\]|\\.)*"|'(?:[^'\\]|\\.)*'|`(?:[^`\\]|\\.)*`/g; + let match: RegExpExecArray | null; + while ((match = re.exec(line)) !== null) literals.push(match[0].slice(1, -1)); + return literals; +} + +function formatSecretKindsFromText(text: string): string[] { + const kinds = SECRET_PATTERNS.filter((pattern) => pattern.re.test(text)).map((pattern) => pattern.name); + if (hasGenericSecretAssignment(text)) kinds.push("generic_secret_assignment"); + return kinds; +} + +/** True for unified-diff file headers (`+++ b/path`, `--- a/path`), not added content like `+++const`. + * Mirrors review-enrichment/src/analyzers/diff-lines.ts `isDiffFileHeaderLine`. */ +function isUnifiedDiffFileHeaderLine(line: string): boolean { + return /^(?:\+\+\+|---) (?:[ab]\/|\/dev\/null)/.test(line); +} + +/** Scan a PR diff for secret kinds introduced on added lines (and added/renamed file headers). Per-line regex + * first; then a bounded cross-line join of consecutive added lines' adjacent string literals (#2454) so a + * credential split across `const a = "AKIA…"; const b = "REST";` still trips the unconditional gate. Context, + * removed, hunk, and file-section boundaries reset both the literal-join window and generic-assignment runs. */ +export function scanPrDiffForSecretKinds(diff: string): string[] { + const found = new Set(); + let inFileSection = false; + let inHunk = false; + let previousLiterals: string[] = []; + let addedRun: string[] = []; + + const resetJoinState = (): void => { + previousLiterals = []; + addedRun = []; + }; + + const noteGenericFromAddedRun = (): void => { + if (found.has("generic_secret_assignment") || addedRun.length === 0) return; + if (hasGenericSecretAssignment(addedRun.join("\n"))) { + found.add("generic_secret_assignment"); + } + }; + + for (const line of diff.split("\n")) { + if (line === "") { + inFileSection = false; + inHunk = false; + resetJoinState(); + continue; + } + if (/^### .+ \(.+\) /.test(line)) { + inFileSection = true; + inHunk = false; + resetJoinState(); + if (/^### .+ \((?:added|renamed)\) /.test(line)) { + for (const kind of formatSecretKindsFromText(line)) found.add(kind); + } + continue; + } + if (line.startsWith("@@")) { + inHunk = true; + resetJoinState(); + continue; + } + if (line.startsWith("+")) { + if (!inFileSection) continue; + // Skip pre-hunk file headers only; inside a hunk `+++…` is added content, not a header. + if (!inHunk && isUnifiedDiffFileHeaderLine(line)) continue; + const content = line.slice(1); + addedRun.push(content); + let matched = false; + for (const kind of formatSecretKindsFromText(content)) { + found.add(kind); + matched = true; + } + noteGenericFromAddedRun(); + const currentLiterals = extractStringLiteralContents(content); + const lastPrevious = previousLiterals.at(-1); + const firstCurrent = currentLiterals[0]; + if (!matched && lastPrevious !== undefined && firstCurrent !== undefined) { + const joined = lastPrevious + firstCurrent; + for (const pattern of SECRET_PATTERNS) { + if (pattern.re.test(joined)) { + found.add(pattern.name); + break; + } + } + } + previousLiterals = currentLiterals; + continue; + } + if (line.startsWith("-")) { + resetJoinState(); + continue; + } + if (inHunk && line.startsWith(" ")) { + resetJoinState(); + } + } + + return [...found]; +} diff --git a/test/unit/safety-wiring.test.ts b/test/unit/safety-wiring.test.ts index 750fb39d6c..7728c16ec4 100644 --- a/test/unit/safety-wiring.test.ts +++ b/test/unit/safety-wiring.test.ts @@ -435,4 +435,17 @@ describe("secretLeakFinding scans only ADDED lines", () => { const diff = `### fixtures/${fakeToken}.txt (removed) +0/-1\n@@\n-const unrelated = 1;`; expect(secretLeakFinding(diff)).toBeNull(); }); + + it("flags a credential split across consecutive added lines (#2454)", () => { + const awsKeyFragmentA = "AKIA" + "IOSFODNN7"; + const awsKeyFragmentB = "EXAMPLE"; + const diff = [ + "### src/config.ts (modified) +2/-0", + "@@ -1,0 +1,2 @@", + `+const part1 = "${awsKeyFragmentA}";`, + `+const part2 = "${awsKeyFragmentB}";`, + ].join("\n"); + expect(secretLeakFinding(diff)?.code).toBe("secret_leak"); + expect(secretLeakFinding(diff)?.title).toContain("aws_access_key"); + }); }); diff --git a/test/unit/secrets-scan.test.ts b/test/unit/secrets-scan.test.ts index 04f30230a6..3270034372 100644 --- a/test/unit/secrets-scan.test.ts +++ b/test/unit/secrets-scan.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { scanForSecrets } from "../../src/review/secrets-scan"; +import { scanForSecrets, scanPrDiffForSecretKinds } from "../../src/review/secrets-scan"; describe("scanForSecrets — deterministic secret-pattern scanner", () => { it("returns no findings for empty / benign text", () => { @@ -107,3 +107,209 @@ describe("scanForSecrets — deterministic secret-pattern scanner", () => { expect(scanForSecrets('token = "short12345"').kinds).not.toContain("generic_secret_assignment"); }); }); + +describe("scanPrDiffForSecretKinds — cross-line split credentials (#2454)", () => { + const awsKeyFragmentA = "AKIA" + "IOSFODNN7"; + const awsKeyFragmentB = "EXAMPLE"; + + it("catches an AWS key split across two adjacent added lines via string literals", () => { + const diff = [ + "### src/config.ts (modified) +2/-0", + "@@ -1,0 +1,2 @@", + `+const part1 = "${awsKeyFragmentA}";`, + `+const part2 = "${awsKeyFragmentB}";`, + ].join("\n"); + expect(scanPrDiffForSecretKinds(diff)).toContain("aws_access_key"); + }); + + it("does not join literals across a context line inside the hunk", () => { + const diff = [ + "### src/config.ts (modified) +2/-0", + "@@ -1,1 +1,3 @@", + `+const part1 = "${awsKeyFragmentA}";`, + ' const unrelated = "context line";', + `+const part2 = "${awsKeyFragmentB}";`, + ].join("\n"); + expect(scanPrDiffForSecretKinds(diff)).not.toContain("aws_access_key"); + }); + + it("does not join literals across a hunk boundary", () => { + const diff = [ + "### src/config.ts (modified) +2/-0", + "@@ -1,0 +1,1 @@", + `+const part1 = "${awsKeyFragmentA}";`, + "@@ -10,0 +11,1 @@", + `+const part2 = "${awsKeyFragmentB}";`, + ].join("\n"); + expect(scanPrDiffForSecretKinds(diff)).not.toContain("aws_access_key"); + }); + + it("does not join unrelated short literals into a false positive", () => { + const diff = [ + "### src/app.ts (modified) +2/-0", + "@@ -1,0 +1,2 @@", + '+const a = "hello";', + '+const b = "world";', + ].join("\n"); + expect(scanPrDiffForSecretKinds(diff)).toEqual([]); + }); + + it("still flags a single-line secret on an added line", () => { + const fakeToken = "ghp_" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; + const diff = `### src/config.ts (modified) +1/-0\n@@\n+const token = "${fakeToken}";`; + expect(scanPrDiffForSecretKinds(diff)).toContain("github_token"); + }); + + it("does not flag secrets on removed or context lines", () => { + const fakeToken = "ghp_" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; + const removed = `### src/config.ts (modified) +0/-1\n@@\n-const token = "${fakeToken}";`; + const context = `### src/config.ts (modified) +1/-0\n@@\n const token = "${fakeToken}";\n+const unrelated = 1;`; + expect(scanPrDiffForSecretKinds(removed)).toEqual([]); + expect(scanPrDiffForSecretKinds(context)).toEqual([]); + }); + + it("scans patch-less synthetic + lines without a hunk header", () => { + const fakeToken = "ghp_" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; + const diff = `### secrets.env (modified) +1/-0\n+const token = "${fakeToken}";`; + expect(scanPrDiffForSecretKinds(diff)).toContain("github_token"); + }); + + it("flags secrets embedded in added/renamed file path headers", () => { + const fakeToken = "ghp_" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; + const added = `### fixtures/${fakeToken}.txt (added) +1/-0\n+clean content`; + const renamed = `### fixtures/${fakeToken}.txt (renamed) +0/-0\n+clean content`; + expect(scanPrDiffForSecretKinds(added)).toContain("github_token"); + expect(scanPrDiffForSecretKinds(renamed)).toContain("github_token"); + }); + + it("ignores orphan + lines that appear before any file header", () => { + const fakeToken = "ghp_" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; + expect(scanPrDiffForSecretKinds(`+const token = "${fakeToken}";`)).toEqual([]); + }); + + it("resets cross-line join state across blank lines between file sections", () => { + const fakeToken = "ghp_" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; + const diff = [ + "### src/a.ts (modified) +1/-0", + `+const part1 = "${awsKeyFragmentA}";`, + "", + "### src/b.ts (modified) +1/-0", + `+const part2 = "${awsKeyFragmentB}";`, + ].join("\n"); + expect(scanPrDiffForSecretKinds(diff)).not.toContain("aws_access_key"); + expect( + scanPrDiffForSecretKinds( + [ + "### src/b.ts (modified) +1/-0", + `+const token = "${fakeToken}";`, + ].join("\n"), + ), + ).toContain("github_token"); + }); + + it("resets cross-line join state when a removed line breaks the added-line run", () => { + const diff = [ + "### src/config.ts (modified) +2/-0", + "@@ -1,0 +1,2 @@", + `+const part1 = "${awsKeyFragmentA}";`, + "-const removed = 1;", + `+const part2 = "${awsKeyFragmentB}";`, + ].join("\n"); + expect(scanPrDiffForSecretKinds(diff)).not.toContain("aws_access_key"); + }); + + it("recovers generic_secret_assignment when keyword and value span adjacent added lines", () => { + const fakeSecret = "sk_live_" + "aK9xQ2mZw7Ln4Rv8Pt3Bh6"; + const diff = [ + "### src/config.ts (modified) +2/-0", + "@@", + "+client_secret =", + `+"${fakeSecret}"`, + ].join("\n"); + expect(scanPrDiffForSecretKinds(diff)).toContain("generic_secret_assignment"); + }); + + it("does not join generic keyword and value across context, removed, hunk, or file boundaries", () => { + const fakeSecret = "sk_live_" + "aK9xQ2mZw7Ln4Rv8Pt3Bh6"; + + const contextSplit = [ + "### src/config.ts (modified) +2/-0", + "@@", + "+client_secret =", + " const unrelated = 1;", + `+"${fakeSecret}"`, + ].join("\n"); + expect(scanPrDiffForSecretKinds(contextSplit)).not.toContain("generic_secret_assignment"); + + const removedSplit = [ + "### src/config.ts (modified) +2/-0", + "@@", + "+client_secret =", + "-const gone = 1;", + `+"${fakeSecret}"`, + ].join("\n"); + expect(scanPrDiffForSecretKinds(removedSplit)).not.toContain("generic_secret_assignment"); + + const hunkSplit = [ + "### src/config.ts (modified) +2/-0", + "@@ -1,0 +1,1 @@", + "+client_secret =", + "@@ -10,0 +11,1 @@", + `+"${fakeSecret}"`, + ].join("\n"); + expect(scanPrDiffForSecretKinds(hunkSplit)).not.toContain("generic_secret_assignment"); + + const fileSplit = [ + "### src/a.ts (modified) +1/-0", + "@@", + "+client_secret =", + "", + "### src/b.ts (modified) +1/-0", + "@@", + `+"${fakeSecret}"`, + ].join("\n"); + expect(scanPrDiffForSecretKinds(fileSplit)).not.toContain("generic_secret_assignment"); + }); + + it("does not double-join when the first added line already matches on its own", () => { + const fakeAwsKey = awsKeyFragmentA + awsKeyFragmentB; + const diff = [ + "### src/config.ts (modified) +2/-0", + "@@", + `+const key = "${fakeAwsKey}";`, + `+const tail = "${awsKeyFragmentB}";`, + ].join("\n"); + expect(scanPrDiffForSecretKinds(diff)).toEqual(["aws_access_key"]); + }); + + it("scans in-hunk added content that begins with ++ (rendered +++…)", () => { + const fakeToken = "ghp_" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; + const diff = [ + "### src/config.ts (modified) +1/-0", + "@@ -1,0 +1,1 @@", + `+++const token = "${fakeToken}";`, + ].join("\n"); + expect(scanPrDiffForSecretKinds(diff)).toContain("github_token"); + }); + + it("skips unified-diff file headers before the first hunk", () => { + const fakeToken = "ghp_" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; + const diff = [ + "### src/config.ts (modified) +1/-0", + "+++ b/src/config.ts", + "--- a/src/config.ts", + "@@ -1,0 +1,1 @@", + "+const ok = 1;", + ].join("\n"); + expect(scanPrDiffForSecretKinds(diff)).toEqual([]); + expect( + scanPrDiffForSecretKinds( + [ + "### src/config.ts (modified) +1/-0", + "+++ b/src/config.ts", + `+const token = "${fakeToken}";`, + ].join("\n"), + ), + ).toContain("github_token"); + }); +});