From 975412ff92091c584486d3d251cbc8f00e2d6c08 Mon Sep 17 00:00:00 2001 From: Andrei Hasna Date: Wed, 15 Jul 2026 23:34:13 +0300 Subject: [PATCH 1/8] fix: make secret exposure scans fail closed --- README.md | 58 ++++++++++++++-- src/cli/commands/llm.ts | 11 +++ src/cli/commands/scan.ts | 5 ++ src/cli/commands/secrets.test.ts | 67 +++++++++++++++++++ src/cli/commands/secrets.ts | 55 ++++++++++----- src/db/findings.test.ts | 44 ++++++++++-- src/db/findings.ts | 57 +++++++++------- src/lib/finding-safety.test.ts | 59 ++++++++++++++++ src/lib/finding-safety.ts | 71 ++++++++++++++++++++ src/lib/secret-exposure.test.ts | 24 +++++++ src/lib/secret-exposure.ts | 9 +-- src/llm/analyzer.ts | 2 + src/llm/credential-boundary.test.ts | 51 ++++++++++++++ src/llm/explainer.ts | 2 + src/llm/fixer.ts | 2 + src/llm/triager.ts | 2 + src/mcp/tools/findings.ts | 17 ++++- src/mcp/tools/output-safety.test.ts | 100 ++++++++++++++++++++++++++++ src/mcp/tools/scan.ts | 33 +++++---- src/reporters/json.test.ts | 19 +++--- src/reporters/json.ts | 6 +- src/reporters/sarif.test.ts | 15 +++++ src/reporters/sarif.ts | 4 +- src/reporters/terminal.test.ts | 44 ++++++++++++ src/reporters/terminal.ts | 6 +- src/scanners/git-history.ts | 2 +- src/scanners/secrets.test.ts | 5 +- src/scanners/secrets.ts | 14 ++-- src/server/serve.ts | 12 ++++ 29 files changed, 704 insertions(+), 92 deletions(-) create mode 100644 src/cli/commands/secrets.test.ts create mode 100644 src/lib/finding-safety.test.ts create mode 100644 src/lib/finding-safety.ts create mode 100644 src/llm/credential-boundary.test.ts create mode 100644 src/mcp/tools/output-safety.test.ts create mode 100644 src/reporters/terminal.test.ts diff --git a/README.md b/README.md index 4ee109d..dfb467d 100644 --- a/README.md +++ b/README.md @@ -19,9 +19,12 @@ bun install -g @hasna/shield # Scan your repo for security issues shield scan . -# Focused secret-exposure scan (repo files, git history, processes, tmux) +# Focused secret-exposure scan (safe default: repository files only) shield secrets . +# Explicit historical scan (still redacted in terminal/JSON/SARIF output) +shield secrets . --git-history + # Check if a package is compromised (axios/litellm/Trivy supply chain attacks) shield check-package axios 1.14.1 shield check-package litellm 1.82.8 --ecosystem pypi @@ -127,7 +130,7 @@ API endpoints: ``` shield scan [path] Run shield scan -shield secrets [options] [path] Focused secret-exposure scan (files + live context) +shield secrets [options] [path] Focused secret-exposure scan (file-only by default) shield findings List findings shield explain AI explanation for a finding shield fix AI-suggested fix @@ -151,21 +154,62 @@ Stored in `~/.hasna/security/` (override with `SECURITY_DB` env var). ## Secret Exposure Workflow -`shield secrets` combines four sources: +`shield secrets` scans repository files by default. The following additional +sources exist, but each requires an explicit opt-in because it crosses a wider +data boundary: - repository files such as `.env` files and config files -- git history across all branches -- running process environments -- tmux pane/session metadata plus recent pane history +- `--git-history` scans git history across all branches +- `--processes` inspects running process command/environment snapshots +- `--tmux` inspects tmux pane/session metadata plus recent pane history + +Secret and credential findings never emit raw code snippets. Terminal, JSON, +and SARIF reporters retain the rule, location, severity, and fingerprint while +replacing sensitive snippets and analysis text with `[REDACTED]`. Credential +findings are also excluded from LLM explanation, triage, analysis, and fix +context so source lines cannot cross a model boundary. Secret-scan error output +also withholds underlying exception text because parser or provider errors can +contain scanned source context. Useful flags: ```bash -shield secrets . --repo-only +# Safe file-only modes (the default, plus an explicit fail-closed form) +shield secrets . +shield secrets . --files-only --json + +# Historical source: explicit opt-in +shield secrets . --git-history --json + +# Live sources: sensitive explicit opt-in; never use in routine CI +shield secrets . --processes +shield secrets . --tmux + +# --repo-only blocks live sources; history still requires --git-history +shield secrets . --repo-only --git-history shield secrets . --json shield secrets . --severity high --fail-on medium + +# Package/archive-only validation does not inspect ambient processes or tmux +shield fleet-package ./package.tgz --json ``` +### Migration warning for 0.1.25 and earlier + +Versions through 0.1.25 enabled git-history, process, and tmux sources by +default. Structured output could therefore include credential-bearing source +context. Upgrade before using `shield secrets` in an agent, CI job, log +collector, or transcript-producing tool. Until the fixed version is installed, +use `shield secrets . --repo-only --no-git-history --no-processes --no-tmux` +or use the `secrets scan workspace` and `shield fleet-package` file/archive +paths. If an older structured scan ran in a credential-bearing environment, +treat the visible credential identifiers as exposed, preserve values out of +incident channels, and follow the owning vault/provider rotation runbook. +Existing database rows are sanitized when read but are not destructively +rewritten by this update; purge or migration of historical local state requires +separate incident-owner authorization. Credential-finding fingerprints may +change once newly scanned records use the redacted persistence form. + ## Storage Shield stores local state in `~/.hasna/security/` by default. Set diff --git a/src/cli/commands/llm.ts b/src/cli/commands/llm.ts index f061037..cac8b4e 100644 --- a/src/cli/commands/llm.ts +++ b/src/cli/commands/llm.ts @@ -6,6 +6,7 @@ import { explainFinding as llmExplainFinding, suggestFix as llmSuggestFix, } from "../../llm/index.js"; +import { isCredentialFinding } from "../../lib/finding-safety.js"; import { getCodeContext } from "../helpers.js"; export function registerLLMCommands(program: Command): void { @@ -27,6 +28,11 @@ export function registerLLMCommands(program: Command): void { process.exit(1); } + if (isCredentialFinding(finding)) { + console.error(chalk.red("\n LLM features are disabled for credential findings.\n")); + process.exit(1); + } + if (finding.llm_explanation) { console.log(chalk.bold("\n Explanation (cached):\n")); console.log(` ${finding.llm_explanation}\n`); @@ -64,6 +70,11 @@ export function registerLLMCommands(program: Command): void { process.exit(1); } + if (isCredentialFinding(finding)) { + console.error(chalk.red("\n LLM features are disabled for credential findings.\n")); + process.exit(1); + } + if (finding.llm_fix) { console.log(chalk.bold("\n Suggested Fix (cached):\n")); console.log(finding.llm_fix); diff --git a/src/cli/commands/scan.ts b/src/cli/commands/scan.ts index ea64c1b..aad9780 100644 --- a/src/cli/commands/scan.ts +++ b/src/cli/commands/scan.ts @@ -10,6 +10,7 @@ import { runScanner, getScanner } from "../../scanners/index.js"; import { isLLMAvailable, analyzeFinding as llmAnalyzeFinding } from "../../llm/index.js"; import { getReporter } from "../../reporters/index.js"; import { loadConfig } from "../../lib/index.js"; +import { isCredentialFinding } from "../../lib/finding-safety.js"; import { parseFormat, parseSeverity, resolveScannerTypes, filterBySeverity, ensureProject, getCodeContext, @@ -89,6 +90,10 @@ export function registerScanCommand(program: Command): void { const batch = storedFindings.slice(i, i + BATCH_SIZE); await Promise.allSettled( batch.map(async (finding) => { + if (isCredentialFinding(finding)) { + analyzed++; + return; + } const codeContext = getCodeContext(resolve(scanPath, finding.file), finding.line); const analysis = await llmAnalyzeFinding(finding, codeContext); if (analysis) finding.llm_exploitability = analysis.exploitability; diff --git a/src/cli/commands/secrets.test.ts b/src/cli/commands/secrets.test.ts new file mode 100644 index 0000000..44af72c --- /dev/null +++ b/src/cli/commands/secrets.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, test } from "bun:test"; +import { Command } from "commander"; +import { registerSecretsCommand, resolveSecretExposureSources } from "./secrets.js"; + +describe("secret exposure source flags", () => { + test("defaults every non-file source off", () => { + expect(resolveSecretExposureSources({})).toEqual({ + include_git_history: false, + include_processes: false, + include_tmux: false, + }); + }); + + test("requires a separate explicit opt-in for each source", () => { + expect(resolveSecretExposureSources({ gitHistory: true })).toEqual({ + include_git_history: true, + include_processes: false, + include_tmux: false, + }); + expect(resolveSecretExposureSources({ processes: true, tmux: true })).toEqual({ + include_git_history: false, + include_processes: true, + include_tmux: true, + }); + }); + + test("files-only wins over every ambient or historical opt-in", () => { + expect(resolveSecretExposureSources({ + filesOnly: true, + gitHistory: true, + processes: true, + tmux: true, + })).toEqual({ + include_git_history: false, + include_processes: false, + include_tmux: false, + }); + }); + + test("repo-only blocks live sources while allowing explicit git history", () => { + expect(resolveSecretExposureSources({ + repoOnly: true, + gitHistory: true, + processes: true, + tmux: true, + })).toEqual({ + include_git_history: true, + include_processes: false, + include_tmux: false, + }); + }); + + test("Commander help keeps compatibility flags while positive source flags default off", () => { + const program = new Command(); + registerSecretsCommand(program); + const command = program.commands.find((candidate) => candidate.name() === "secrets"); + + expect(command).toBeDefined(); + expect(command?.options.find((option) => option.long === "--git-history")?.defaultValue).toBe(false); + expect(command?.options.find((option) => option.long === "--processes")?.defaultValue).toBe(false); + expect(command?.options.find((option) => option.long === "--tmux")?.defaultValue).toBe(false); + expect(command?.options.some((option) => option.long === "--no-git-history")).toBe(true); + expect(command?.options.some((option) => option.long === "--no-processes")).toBe(true); + expect(command?.options.some((option) => option.long === "--no-tmux")).toBe(true); + expect(command?.helpInformation()).toContain("file-only"); + }); +}); diff --git a/src/cli/commands/secrets.ts b/src/cli/commands/secrets.ts index 2b95092..c5d6999 100644 --- a/src/cli/commands/secrets.ts +++ b/src/cli/commands/secrets.ts @@ -12,6 +12,29 @@ import { parseSeverity } from "../helpers.js"; type SecretCommandFormat = "terminal" | "json"; +export interface SecretExposureSourceFlags { + filesOnly?: boolean; + repoOnly?: boolean; + gitHistory?: boolean; + processes?: boolean; + tmux?: boolean; +} + +export function resolveSecretExposureSources(options: SecretExposureSourceFlags): { + include_git_history: boolean; + include_processes: boolean; + include_tmux: boolean; +} { + const filesOnly = options.filesOnly === true; + const liveSourcesAllowed = !filesOnly && options.repoOnly !== true; + + return { + include_git_history: !filesOnly && options.gitHistory === true, + include_processes: liveSourcesAllowed && options.processes === true, + include_tmux: liveSourcesAllowed && options.tmux === true, + }; +} + function parseSecretCommandFormat(value: string): SecretCommandFormat { const normalized = value.toLowerCase(); if (normalized === "terminal" || normalized === "json") return normalized; @@ -69,16 +92,20 @@ function printTerminalSummary( export function registerSecretsCommand(program: Command): void { program .command("secrets") - .description("Scan repo files, git history, running processes, and tmux panes for exposed secrets") + .description("Scan repository files for exposed secrets; ambient and historical sources require explicit opt-in") .argument("[path]", "Path to scan", ".") .option("--format ", "Output format (terminal/json)", "terminal") .option("-j, --json", "Shortcut for --format json") .option("--severity ", "Minimum severity threshold to display", "info") .option("--fail-on ", "Exit non-zero when findings meet or exceed this severity", "high") - .option("--no-git-history", "Skip git history scanning") - .option("--no-processes", "Skip running process environment scanning") - .option("--no-tmux", "Skip tmux metadata/history scanning") - .option("--repo-only", "Only scan repository files and git history") + .option("--git-history", "Also scan git history (explicit opt-in)", false) + .option("--processes", "Also inspect running process command/environment snapshots (sensitive explicit opt-in)", false) + .option("--tmux", "Also inspect tmux metadata/history (sensitive explicit opt-in)", false) + .option("--no-git-history", "Compatibility flag; git history is disabled by default") + .option("--no-processes", "Compatibility flag; process inspection is disabled by default") + .option("--no-tmux", "Compatibility flag; tmux inspection is disabled by default") + .option("--files-only", "Force file-only scanning even when ambient-source flags are present") + .option("--repo-only", "Disable live process and tmux sources; git history still requires --git-history") .action(async (pathArg: string, options) => { const scanPath = resolve(pathArg); if (!existsSync(scanPath)) { @@ -90,22 +117,19 @@ export function registerSecretsCommand(program: Command): void { const format = options.json ? "json" : parseSecretCommandFormat(options.format); const severityThreshold = parseSeverity(options.severity); const failThreshold = parseSeverity(options.failOn); - const includeProcesses = options.repoOnly ? false : options.processes; - const includeTmux = options.repoOnly ? false : options.tmux; + const sources = resolveSecretExposureSources(options); const result = await scanSecretExposure({ path: scanPath, - include_git_history: options.gitHistory, - include_processes: includeProcesses, - include_tmux: includeTmux, + ...sources, }); const filtered = filterSecretExposureBySeverity(result.findings, severityThreshold); const enabledSources = [ "files", - options.gitHistory ? "git-history" : null, - includeProcesses ? "processes" : null, - includeTmux ? "tmux" : null, + sources.include_git_history ? "git-history" : null, + sources.include_processes ? "processes" : null, + sources.include_tmux ? "tmux" : null, ].filter(Boolean) as string[]; if (format === "json") { @@ -125,9 +149,8 @@ export function registerSecretsCommand(program: Command): void { if (result.findings.some((finding) => SEVERITY_ORDER[finding.severity] <= failOrder)) { process.exit(1); } - } catch (error) { - const errMsg = error instanceof Error ? error.message : String(error); - console.error(chalk.red(`\n Secret exposure scan failed: ${errMsg}\n`)); + } catch { + console.error(chalk.red("\n Secret exposure scan failed. Details were withheld to protect scanned source context.\n")); process.exit(1); } }); diff --git a/src/db/findings.test.ts b/src/db/findings.test.ts index d8df8a1..4c532ca 100644 --- a/src/db/findings.test.ts +++ b/src/db/findings.test.ts @@ -83,14 +83,15 @@ describe("findings", () => { expect(f1.fingerprint).not.toBe(f2.fingerprint); }); - test("createFinding stores optional fields (column, end_line, code_snippet)", () => { + test("createFinding stores optional location fields but redacts secret snippets", () => { const finding = createFinding( scanId, makeInput({ column: 10, end_line: 45, code_snippet: "let x = 1;" }), ); expect(finding.column).toBe(10); expect(finding.end_line).toBe(45); - expect(finding.code_snippet).toBe("let x = 1;"); + expect(finding.code_snippet).toBe("[REDACTED]"); + expect(finding.message).toContain("Potential credential exposure"); }); test("createFinding defaults optional fields to null", () => { @@ -108,6 +109,22 @@ describe("findings", () => { expect(fetched!.suppressed).toBe(false); }); + test("getFinding redacts legacy credential-bearing rows on read", () => { + const syntheticSecret = "ghp_" + "SYNTHETICONLYABCDEFGHIJKLMNOPQRSTUVWXYZ12"; + const created = createFinding(scanId, makeInput()); + const db = getCurrentTestDb(); + db.prepare("UPDATE findings SET message = ?, code_snippet = ? WHERE id = ?").run( + `GitHub token detected: ${syntheticSecret}`, + `GITHUB_TOKEN=${syntheticSecret}`, + created.id, + ); + + const fetched = getFinding(created.id); + expect(JSON.stringify(fetched)).not.toContain(syntheticSecret); + expect(fetched?.message).toContain("Potential credential exposure"); + expect(fetched?.code_snippet).toBe("[REDACTED]"); + }); + test("getFinding returns null for unknown id", () => { expect(getFinding("nonexistent")).toBeNull(); }); @@ -193,11 +210,15 @@ describe("findings", () => { const updated = getFinding(finding.id); expect(updated!.suppressed).toBe(true); - expect(updated!.suppressed_reason).toBe("Known false positive"); + expect(updated!.suppressed_reason).toBe("[REDACTED]"); }); test("updateFinding updates LLM fields", () => { - const finding = createFinding(scanId, makeInput()); + const finding = createFinding(scanId, makeInput({ + rule_id: "code-rule", + scanner_type: ScannerType.Code, + message: "Unsafe code path", + })); updateFinding(finding.id, { llm_explanation: "This is a test explanation", llm_fix: "Use env vars instead", @@ -210,6 +231,21 @@ describe("findings", () => { expect(updated!.llm_exploitability).toBe(0.8); }); + test("updateFinding cannot persist analysis text for credential findings", () => { + const syntheticSecret = "ghp_" + "SYNTHETICONLYABCDEFGHIJKLMNOPQRSTUVWXYZ12"; + const finding = createFinding(scanId, makeInput()); + updateFinding(finding.id, { + llm_explanation: `Credential ${syntheticSecret} is exposed`, + llm_fix: `Remove ${syntheticSecret}`, + llm_exploitability: 0.8, + }); + + const updated = getFinding(finding.id); + expect(updated?.llm_explanation).toBe("[REDACTED]"); + expect(updated?.llm_fix).toBe("[REDACTED]"); + expect(JSON.stringify(updated)).not.toContain(syntheticSecret); + }); + test("countFindings counts all findings", () => { createFinding(scanId, makeInput()); createFinding(scanId, makeInput({ file: "b.ts" })); diff --git a/src/db/findings.ts b/src/db/findings.ts index 72ff429..9302fc1 100644 --- a/src/db/findings.ts +++ b/src/db/findings.ts @@ -3,6 +3,12 @@ import { createHash } from "crypto"; import { getDb } from "./database.js"; import type { Finding, FindingInput, SecurityScore } from "../types/index.js"; import { Severity, type ScannerType } from "../types/index.js"; +import { + isCredentialFinding, + REDACTED_FINDING_TEXT, + sanitizeFindingForOutput, + sanitizeFindingForPersistence, +} from "../lib/finding-safety.js"; interface FindingRow { id: string; @@ -26,12 +32,12 @@ interface FindingRow { } function rowToFinding(row: FindingRow): Finding { - return { + return sanitizeFindingForOutput({ ...row, scanner_type: row.scanner_type as ScannerType, severity: row.severity as Severity, suppressed: row.suppressed === 1, - }; + }); } function generateFingerprint(rule_id: string, file: string, line: number, message: string): string { @@ -43,9 +49,10 @@ function generateFingerprint(rule_id: string, file: string, line: number, messag export function createFinding(scan_id: string, input: FindingInput): Finding { const db = getDb(); + const safeInput = sanitizeFindingForPersistence(input); const id = crypto.randomUUID(); const now = new Date().toISOString(); - const fingerprint = generateFingerprint(input.rule_id, input.file, input.line, input.message); + const fingerprint = generateFingerprint(safeInput.rule_id, safeInput.file, safeInput.line, safeInput.message); const stmt = db.prepare( `INSERT INTO findings (id, scan_id, rule_id, scanner_type, severity, file, line, "column", end_line, message, code_snippet, fingerprint, suppressed, created_at) @@ -54,15 +61,15 @@ export function createFinding(scan_id: string, input: FindingInput): Finding { stmt.run( id, scan_id, - input.rule_id, - input.scanner_type, - input.severity, - input.file, - input.line, - input.column ?? null, - input.end_line ?? null, - input.message, - input.code_snippet ?? null, + safeInput.rule_id, + safeInput.scanner_type, + safeInput.severity, + safeInput.file, + safeInput.line, + safeInput.column ?? null, + safeInput.end_line ?? null, + safeInput.message, + safeInput.code_snippet ?? null, fingerprint, now ); @@ -70,15 +77,15 @@ export function createFinding(scan_id: string, input: FindingInput): Finding { return { id, scan_id, - rule_id: input.rule_id, - scanner_type: input.scanner_type, - severity: input.severity, - file: input.file, - line: input.line, - column: input.column ?? null, - end_line: input.end_line ?? null, - message: input.message, - code_snippet: input.code_snippet ?? null, + rule_id: safeInput.rule_id, + scanner_type: safeInput.scanner_type, + severity: safeInput.severity, + file: safeInput.file, + line: safeInput.line, + column: safeInput.column ?? null, + end_line: safeInput.end_line ?? null, + message: safeInput.message, + code_snippet: safeInput.code_snippet ?? null, fingerprint, suppressed: false, suppressed_reason: null, @@ -149,6 +156,8 @@ export function updateFinding( updates: Partial> ): void { const db = getDb(); + const existing = getFinding(id); + const sensitive = existing ? isCredentialFinding(existing) : false; const sets: string[] = []; const params: unknown[] = []; @@ -158,15 +167,15 @@ export function updateFinding( } if (updates.suppressed_reason !== undefined) { sets.push("suppressed_reason = ?"); - params.push(updates.suppressed_reason); + params.push(sensitive && updates.suppressed_reason != null ? REDACTED_FINDING_TEXT : updates.suppressed_reason); } if (updates.llm_explanation !== undefined) { sets.push("llm_explanation = ?"); - params.push(updates.llm_explanation); + params.push(sensitive && updates.llm_explanation != null ? REDACTED_FINDING_TEXT : updates.llm_explanation); } if (updates.llm_fix !== undefined) { sets.push("llm_fix = ?"); - params.push(updates.llm_fix); + params.push(sensitive && updates.llm_fix != null ? REDACTED_FINDING_TEXT : updates.llm_fix); } if (updates.llm_exploitability !== undefined) { sets.push("llm_exploitability = ?"); diff --git a/src/lib/finding-safety.test.ts b/src/lib/finding-safety.test.ts new file mode 100644 index 0000000..e7b56f6 --- /dev/null +++ b/src/lib/finding-safety.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, test } from "bun:test"; +import { ScannerType, Severity, type FindingInput } from "../types/index.js"; +import { + isCredentialFinding, + sanitizeFindingForOutput, + sanitizeFindingForPersistence, +} from "./finding-safety.js"; + +function finding(overrides: Partial = {}): FindingInput { + return { + rule_id: "code-rule", + scanner_type: ScannerType.Code, + severity: Severity.High, + file: "src/app.ts", + line: 1, + message: "Unsafe code path", + ...overrides, + }; +} + +describe("finding safety", () => { + test("classifies credential findings by scanner and semantic rule name", () => { + expect(isCredentialFinding(finding({ scanner_type: ScannerType.Secrets }))).toBe(true); + expect(isCredentialFinding(finding({ rule_id: "hardcoded-password" }))).toBe(true); + expect(isCredentialFinding(finding())).toBe(false); + }); + + test("redacts credential persistence and every output snippet", () => { + const syntheticSecret = "ghp_" + "SYNTHETICONLYABCDEFGHIJKLMNOPQRSTUVWXYZ12"; + const credential = finding({ + scanner_type: ScannerType.Secrets, + rule_id: "github-token", + message: `GitHub token detected: ${syntheticSecret}`, + code_snippet: `GITHUB_TOKEN=${syntheticSecret}`, + }); + + const persisted = sanitizeFindingForPersistence(credential); + const output = sanitizeFindingForOutput(credential); + + expect(JSON.stringify(persisted)).not.toContain(syntheticSecret); + expect(JSON.stringify(output)).not.toContain(syntheticSecret); + expect(persisted.code_snippet).toBe("[REDACTED]"); + expect(output.message).toContain("Potential credential exposure"); + }); + + test("caps and control-cleans non-sensitive output fields", () => { + const output = sanitizeFindingForOutput(finding({ + file: `src/${"a".repeat(700)}\nsecret.ts`, + message: `Unsafe\u0000code ${"x".repeat(700)}`, + code_snippet: "raw source text", + })); + + expect(output.file.length).toBeLessThanOrEqual(512); + expect(output.message.length).toBeLessThanOrEqual(512); + expect(output.file).not.toContain("\n"); + expect(output.message).not.toContain("\u0000"); + expect(output.code_snippet).toBe("[REDACTED]"); + }); +}); diff --git a/src/lib/finding-safety.ts b/src/lib/finding-safety.ts new file mode 100644 index 0000000..6963cac --- /dev/null +++ b/src/lib/finding-safety.ts @@ -0,0 +1,71 @@ +import { ScannerType, type Finding, type FindingInput } from "../types/index.js"; + +export const REDACTED_FINDING_TEXT = "[REDACTED]"; + +const MAX_LOCATION_LENGTH = 512; +const MAX_MESSAGE_LENGTH = 512; +const MAX_RULE_ID_LENGTH = 128; + +type FindingLike = FindingInput | Finding; + +function boundedSingleLine(value: string, maxLength: number): string { + const normalized = value.replace(/[\u0000-\u001f\u007f]/g, " ").replace(/\s+/g, " ").trim(); + if (normalized.length <= maxLength) return normalized; + return `${normalized.slice(0, Math.max(0, maxLength - 1))}…`; +} + +function safeRuleId(value: string): string { + const normalized = value.replace(/[^A-Za-z0-9._-]/g, "-"); + return boundedSingleLine(normalized || "credential", MAX_RULE_ID_LENGTH); +} + +export function isCredentialFinding(finding: Pick): boolean { + if (finding.scanner_type === ScannerType.Secrets || finding.scanner_type === ScannerType.GitHistory) { + return true; + } + + return /(?:secret|credential|password|passphrase|private[-_ ]?key|api[-_ ]?key|access[-_ ]?key|token|bearer|high[-_ ]?entropy)/i.test( + `${finding.rule_id} ${finding.message}`, + ); +} + +export function sanitizeFindingForPersistence(finding: T): T { + if (!isCredentialFinding(finding)) return finding; + + return { + ...finding, + file: boundedSingleLine(finding.file, MAX_LOCATION_LENGTH), + message: `Potential credential exposure detected (${safeRuleId(finding.rule_id)})`, + ...(finding.code_snippet != null ? { code_snippet: REDACTED_FINDING_TEXT } : {}), + } as T; +} + +export function sanitizeFindingForOutput(finding: T): T { + const sensitive = isCredentialFinding(finding); + const result = { + ...finding, + file: boundedSingleLine(finding.file, MAX_LOCATION_LENGTH), + message: sensitive + ? `Potential credential exposure detected (${safeRuleId(finding.rule_id)})` + : boundedSingleLine(finding.message, MAX_MESSAGE_LENGTH), + ...(finding.code_snippet != null ? { code_snippet: REDACTED_FINDING_TEXT } : {}), + } as T; + + if ("llm_explanation" in result && result.llm_explanation != null) { + result.llm_explanation = sensitive + ? REDACTED_FINDING_TEXT + : boundedSingleLine(result.llm_explanation, MAX_MESSAGE_LENGTH); + } + if ("llm_fix" in result && result.llm_fix != null) { + result.llm_fix = sensitive + ? REDACTED_FINDING_TEXT + : boundedSingleLine(result.llm_fix, MAX_MESSAGE_LENGTH); + } + if ("suppressed_reason" in result && result.suppressed_reason != null) { + result.suppressed_reason = sensitive + ? REDACTED_FINDING_TEXT + : boundedSingleLine(result.suppressed_reason, MAX_MESSAGE_LENGTH); + } + + return result; +} diff --git a/src/lib/secret-exposure.test.ts b/src/lib/secret-exposure.test.ts index aefe4b8..893966d 100644 --- a/src/lib/secret-exposure.test.ts +++ b/src/lib/secret-exposure.test.ts @@ -40,18 +40,38 @@ describe("secret exposure", () => { const result = await scanSecretExposure({ path: tempDir, + include_git_history: true, include_processes: false, include_tmux: false, }); expect(result.findings.some((finding) => finding.file === ".env")).toBe(true); expect(result.findings.some((finding) => finding.scanner_type === ScannerType.GitHistory)).toBe(true); + expect(JSON.stringify(result.findings)).not.toContain(githubToken); const summary = summarizeSecretExposure(result.findings); expect(summary.total).toBe(result.findings.length); expect(summary.critical).toBeGreaterThan(0); }); + test("scanSecretExposure defaults to repository files without invoking ambient sources", async () => { + const githubToken = "ghp_" + "SYNTHETICONLYABCDEFGHIJKLMNOPQRSTUVWXYZ12"; + writeFileSync(join(tempDir, ".env"), `CURRENT_TOKEN=${githubToken}\n`, "utf-8"); + + const calls: Array<{ command: string; args: string[] }> = []; + const runner: CommandRunner = (command, args) => { + calls.push({ command, args }); + return `123 GITHUB_TOKEN=${githubToken} node server.js\n`; + }; + + const result = await scanSecretExposure({ path: tempDir }, runner); + + expect(calls).toEqual([]); + expect(result.findings.length).toBeGreaterThan(0); + expect(result.findings.every((finding) => !finding.file.startsWith("process:") && !finding.file.startsWith("tmux:"))).toBe(true); + expect(JSON.stringify(result.findings)).not.toContain(githubToken); + }); + test("scanRunningProcesses inspects process environment snapshots", () => { const githubToken = "ghp_" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890ab"; const runner: CommandRunner = (_command, _args) => `123 USER=me GITHUB_TOKEN=${githubToken} node server.js\n`; @@ -60,6 +80,8 @@ describe("secret exposure", () => { expect(findings.length).toBeGreaterThan(0); expect(findings[0].file).toBe("process:123"); expect(findings[0].message).toContain("running process 123"); + expect(JSON.stringify(findings)).not.toContain(githubToken); + expect(findings[0].code_snippet).toBe("[REDACTED]"); }); test("scanTmuxPanes inspects pane metadata and history", () => { @@ -79,5 +101,7 @@ describe("secret exposure", () => { expect(findings.length).toBeGreaterThan(0); expect(findings[0].file).toBe("tmux:workspace:0.0:meta"); expect(findings[0].message).toContain("tmux pane metadata workspace:0.0"); + expect(JSON.stringify(findings)).not.toContain(awsKey); + expect(findings[0].code_snippet).toBe("[REDACTED]"); }); }); diff --git a/src/lib/secret-exposure.ts b/src/lib/secret-exposure.ts index d5e8c04..0fb7de7 100644 --- a/src/lib/secret-exposure.ts +++ b/src/lib/secret-exposure.ts @@ -4,6 +4,7 @@ import { resolve } from "path"; import { gitHistoryScanner } from "../scanners/git-history.js"; import { scanFile, secretsScanner } from "../scanners/secrets.js"; import { SEVERITY_ORDER, Severity, type FindingInput } from "../types/index.js"; +import { sanitizeFindingForOutput } from "./finding-safety.js"; type RunnerOptions = { cwd?: string; @@ -259,19 +260,19 @@ export async function scanSecretExposure( })), ); - if (options.include_git_history ?? true) { + if (options.include_git_history === true) { findings.push(...(await gitHistoryScanner.scan(scanPath))); } - if (options.include_processes ?? true) { + if (options.include_processes === true) { findings.push(...scanRunningProcesses(runner)); } - if (options.include_tmux ?? true) { + if (options.include_tmux === true) { findings.push(...scanTmuxPanes(runner)); } - const deduped = dedupeFindings(findings); + const deduped = dedupeFindings(findings).map(sanitizeFindingForOutput); return { path: scanPath, findings: deduped, diff --git a/src/llm/analyzer.ts b/src/llm/analyzer.ts index b72e2ff..1dac056 100644 --- a/src/llm/analyzer.ts +++ b/src/llm/analyzer.ts @@ -1,4 +1,5 @@ import type { Finding } from "../types/index.js"; +import { isCredentialFinding } from "../lib/finding-safety.js"; import { chat } from "./client.js"; import { ANALYZER_PROMPT } from "./prompts.js"; @@ -15,6 +16,7 @@ export async function analyzeFinding( is_true_positive: boolean; confidence: number; } | null> { + if (isCredentialFinding(finding)) return null; const cacheKey = finding.fingerprint; if (cache.has(cacheKey)) return cache.get(cacheKey)!; diff --git a/src/llm/credential-boundary.test.ts b/src/llm/credential-boundary.test.ts new file mode 100644 index 0000000..4e32789 --- /dev/null +++ b/src/llm/credential-boundary.test.ts @@ -0,0 +1,51 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { ScannerType, Severity, type Finding } from "../types/index.js"; +import { analyzeFinding } from "./analyzer.js"; +import { explainFinding } from "./explainer.js"; +import { suggestFix } from "./fixer.js"; +import { triageFinding } from "./triager.js"; + +const originalFetch = globalThis.fetch; + +afterEach(() => { + globalThis.fetch = originalFetch; +}); + +describe("credential finding LLM boundary", () => { + test("never sends credential findings or their context to an LLM", async () => { + const syntheticSecret = "ghp_" + "SYNTHETICONLYABCDEFGHIJKLMNOPQRSTUVWXYZ12"; + let fetchCalls = 0; + globalThis.fetch = (async () => { + fetchCalls++; + throw new Error("network must not be reached"); + }) as unknown as typeof fetch; + + const credentialFinding: Finding = { + id: "finding-1", + scan_id: "scan-1", + rule_id: "github-token", + scanner_type: ScannerType.Secrets, + severity: Severity.Critical, + file: "synthetic.env", + line: 1, + column: 1, + end_line: null, + message: "Potential credential exposure detected (github-token)", + code_snippet: "[REDACTED]", + fingerprint: "synthetic-fingerprint", + suppressed: false, + suppressed_reason: null, + llm_explanation: null, + llm_fix: null, + llm_exploitability: null, + created_at: "2026-07-15T00:00:00.000Z", + }; + const context = `GITHUB_TOKEN=${syntheticSecret}`; + + expect(await analyzeFinding(credentialFinding, context)).toBeNull(); + expect(await explainFinding(credentialFinding, context)).toBeNull(); + expect(await suggestFix(credentialFinding, context)).toBeNull(); + expect(await triageFinding(credentialFinding, context)).toBeNull(); + expect(fetchCalls).toBe(0); + }); +}); diff --git a/src/llm/explainer.ts b/src/llm/explainer.ts index b9ef0b0..0e5f5ad 100644 --- a/src/llm/explainer.ts +++ b/src/llm/explainer.ts @@ -1,4 +1,5 @@ import type { Finding } from "../types/index.js"; +import { isCredentialFinding } from "../lib/finding-safety.js"; import { chat } from "./client.js"; import { EXPLAINER_PROMPT } from "./prompts.js"; @@ -8,6 +9,7 @@ export async function explainFinding( finding: Finding, codeContext: string, ): Promise { + if (isCredentialFinding(finding)) return null; const cacheKey = finding.fingerprint; if (cache.has(cacheKey)) return cache.get(cacheKey)!; diff --git a/src/llm/fixer.ts b/src/llm/fixer.ts index 6e1071c..1b0e0dc 100644 --- a/src/llm/fixer.ts +++ b/src/llm/fixer.ts @@ -1,4 +1,5 @@ import type { Finding } from "../types/index.js"; +import { isCredentialFinding } from "../lib/finding-safety.js"; import { chat } from "./client.js"; import { FIXER_PROMPT } from "./prompts.js"; @@ -8,6 +9,7 @@ export async function suggestFix( finding: Finding, codeContext: string, ): Promise { + if (isCredentialFinding(finding)) return null; const cacheKey = finding.fingerprint; if (cache.has(cacheKey)) return cache.get(cacheKey)!; diff --git a/src/llm/triager.ts b/src/llm/triager.ts index 2838a55..6431be7 100644 --- a/src/llm/triager.ts +++ b/src/llm/triager.ts @@ -1,4 +1,5 @@ import { type Finding, Severity } from "../types/index.js"; +import { isCredentialFinding } from "../lib/finding-safety.js"; import { chat } from "./client.js"; import { TRIAGER_PROMPT } from "./prompts.js"; @@ -16,6 +17,7 @@ export async function triageFinding( finding: Finding, codeContext: string, ): Promise<{ severity: Severity; reasoning: string } | null> { + if (isCredentialFinding(finding)) return null; const cacheKey = finding.fingerprint; if (cache.has(cacheKey)) return cache.get(cacheKey)!; diff --git a/src/mcp/tools/findings.ts b/src/mcp/tools/findings.ts index a544867..54552cd 100644 --- a/src/mcp/tools/findings.ts +++ b/src/mcp/tools/findings.ts @@ -13,6 +13,7 @@ import { triageFinding as llmTriage, isLLMAvailable, } from "../../llm/index.js"; +import { isCredentialFinding } from "../../lib/finding-safety.js"; import { Severity, ScannerType } from "../../types/index.js"; type JsonResult = { content: Array<{ type: "text"; text: string }> }; @@ -74,6 +75,9 @@ export function registerFindingTools( try { const finding = getFinding(id); if (!finding) return jsonResult({ error: "Finding not found" }); + if (isCredentialFinding(finding)) { + return jsonResult({ error: "LLM features are disabled for credential findings" }); + } if (finding.llm_explanation) return jsonResult({ finding_id: id, explanation: finding.llm_explanation }); if (!isLLMAvailable()) return jsonResult({ error: "LLM not available. Set CEREBRAS_API_KEY." }); @@ -96,6 +100,9 @@ export function registerFindingTools( try { const finding = getFinding(id); if (!finding) return jsonResult({ error: "Finding not found" }); + if (isCredentialFinding(finding)) { + return jsonResult({ error: "LLM features are disabled for credential findings" }); + } if (finding.llm_fix) return jsonResult({ finding_id: id, fix: finding.llm_fix }); if (!isLLMAvailable()) return jsonResult({ error: "LLM not available. Set CEREBRAS_API_KEY." }); @@ -122,7 +129,12 @@ export function registerFindingTools( const finding = getFinding(id); if (!finding) return jsonResult({ error: "Finding not found" }); suppressFinding(id, reason); - return jsonResult({ finding_id: id, suppressed: true, reason }); + const updated = getFinding(id); + return jsonResult({ + finding_id: id, + suppressed: true, + reason: updated?.suppressed_reason ?? null, + }); } catch (error) { return jsonResult({ error: String(error) }); } @@ -164,6 +176,9 @@ export function registerFindingTools( try { const finding = getFinding(id); if (!finding) return jsonResult({ error: "Finding not found" }); + if (isCredentialFinding(finding)) { + return jsonResult({ error: "LLM features are disabled for credential findings" }); + } if (!isLLMAvailable()) return jsonResult({ error: "LLM not available. Set CEREBRAS_API_KEY." }); const context = getCodeContext(finding.file, finding.line); diff --git a/src/mcp/tools/output-safety.test.ts b/src/mcp/tools/output-safety.test.ts new file mode 100644 index 0000000..a7924aa --- /dev/null +++ b/src/mcp/tools/output-safety.test.ts @@ -0,0 +1,100 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; +import { createFinding } from "../../db/findings.js"; +import { createProject } from "../../db/projects.js"; +import { createScan } from "../../db/scans.js"; +import { getCurrentTestDb, setupTestDb } from "../../db/test-helpers.js"; +import { ScannerType, Severity } from "../../types/index.js"; +import { registerFindingTools } from "./findings.js"; +import { registerScanTools } from "./scan.js"; + +type ToolHandler = (args: Record) => Promise; + +function captureTools(register: (server: McpServer) => void): Map { + const handlers = new Map(); + const server = { + tool(name: string, _description: string, _schema: unknown, handler: ToolHandler) { + handlers.set(name, handler); + }, + } as unknown as McpServer; + register(server); + return handlers; +} + +const jsonResult = (data: unknown) => ({ + content: [{ type: "text" as const, text: JSON.stringify(data) }], +}); + +describe("MCP credential output safety", () => { + let cleanup: () => void; + let findingId: string; + + beforeEach(() => { + cleanup = setupTestDb(); + const project = createProject("mcp-output-safety", "/tmp/mcp-output-safety"); + const db = getCurrentTestDb(); + db.prepare( + `INSERT INTO rules (id, name, description, scanner_type, severity, enabled, builtin, metadata, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, 1, 1, '{}', datetime('now'), datetime('now'))`, + ).run("github-token", "GitHub Token", "Synthetic test rule", "secrets", "critical"); + const scan = createScan(project.id, [ScannerType.Secrets]); + findingId = createFinding(scan.id, { + rule_id: "github-token", + scanner_type: ScannerType.Secrets, + severity: Severity.Critical, + file: "synthetic.env", + line: 1, + message: "Synthetic credential finding", + code_snippet: "[REDACTED]", + }).id; + }); + + afterEach(() => cleanup()); + + test("suppression responses never echo a caller-provided credential reason", async () => { + const syntheticSecret = "ghp_" + "SYNTHETICONLYABCDEFGHIJKLMNOPQRSTUVWXYZ12"; + const tools = captureTools((server) => { + registerFindingTools(server, jsonResult, () => { + throw new Error("credential context must not be read"); + }); + }); + + const result = await tools.get("suppress_finding")?.({ + id: findingId, + reason: `contains ${syntheticSecret}`, + }); + const output = JSON.stringify(result); + + expect(output).not.toContain(syntheticSecret); + expect(output).toContain("[REDACTED]"); + }); + + test("credential LLM tools short-circuit before source context is read", async () => { + let contextReads = 0; + const tools = captureTools((server) => { + registerFindingTools(server, jsonResult, () => { + contextReads++; + return "synthetic context"; + }); + }); + + for (const toolName of ["explain_finding", "suggest_fix", "triage_finding"]) { + const result = await tools.get(toolName)?.({ id: findingId }); + expect(JSON.stringify(result)).toContain("disabled for credential findings"); + } + expect(contextReads).toBe(0); + }); + + test("secret scan failures withhold arbitrary exception context", async () => { + const tools = captureTools((server) => { + registerScanTools(server, jsonResult, () => ""); + }); + const result = await tools.get("scan_secret_exposure")?.({ + path: "/definitely/missing/shield-output-safety-path", + }); + const output = JSON.stringify(result); + + expect(output).toContain("Details were withheld"); + expect(output).not.toContain("/definitely/missing/shield-output-safety-path"); + }); +}); diff --git a/src/mcp/tools/scan.ts b/src/mcp/tools/scan.ts index 3cba900..bc93351 100644 --- a/src/mcp/tools/scan.ts +++ b/src/mcp/tools/scan.ts @@ -16,6 +16,7 @@ import { runAllScanners, runScanner } from "../../scanners/index.js"; import { analyzeFinding as llmAnalyze, isLLMAvailable } from "../../llm/index.js"; import { ScannerType, ScanStatus, Severity } from "../../types/index.js"; import type { FindingInput } from "../../types/index.js"; +import { sanitizeFindingForOutput } from "../../lib/finding-safety.js"; import { scanSecretExposure, filterSecretExposureBySeverity, @@ -101,8 +102,8 @@ export function registerScanTools( llm_analysis: llm_analyze ? "running in background (5 concurrent)" : "not requested", by_severity: { critical: score.critical, high: score.high, medium: score.medium, low: score.low, info: score.info }, }); - } catch (error) { - return jsonResult({ error: String(error) }); + } catch { + return jsonResult({ error: "Repository scan failed. Details were withheld to protect scanned source context." }); } }, ); @@ -123,9 +124,13 @@ export function registerScanTools( const fileFindings = findings.filter( (f) => f.file === absPath || f.file === filePath || f.file.endsWith(filePath), ); - return jsonResult({ file: absPath, findings: fileFindings, count: fileFindings.length }); - } catch (error) { - return jsonResult({ error: String(error) }); + return jsonResult({ + file: absPath, + findings: fileFindings.map(sanitizeFindingForOutput), + count: fileFindings.length, + }); + } catch { + return jsonResult({ error: "File scan failed. Details were withheld to protect scanned source context." }); } }, ); @@ -133,12 +138,12 @@ export function registerScanTools( // 3. scan_secret_exposure server.tool( "scan_secret_exposure", - "Scan repo files, git history, running processes, and tmux panes for exposed secrets", + "Scan repository files for exposed secrets; git history, processes, and tmux require explicit opt-in", { path: z.string().describe("Path to the repository or directory to scan"), - include_git_history: z.boolean().optional().describe("Whether to include git history scanning"), - include_processes: z.boolean().optional().describe("Whether to include running process environment scanning"), - include_tmux: z.boolean().optional().describe("Whether to include tmux metadata/history scanning"), + include_git_history: z.boolean().optional().describe("Explicitly opt in to git history scanning (default false)"), + include_processes: z.boolean().optional().describe("Explicitly opt in to sensitive running process command/environment inspection (default false)"), + include_tmux: z.boolean().optional().describe("Explicitly opt in to sensitive tmux metadata/history inspection (default false)"), severity: z.string().optional().describe("Minimum severity threshold (critical/high/medium/low/info)"), }, async ({ path, include_git_history, include_processes, include_tmux, severity }) => { @@ -156,9 +161,9 @@ export function registerScanTools( const result = await scanSecretExposure({ path: resolve(path), - include_git_history: include_git_history ?? true, - include_processes: include_processes ?? true, - include_tmux: include_tmux ?? true, + include_git_history: include_git_history === true, + include_processes: include_processes === true, + include_tmux: include_tmux === true, }); const findings = filterSecretExposureBySeverity(result.findings, parsedSeverity); @@ -169,8 +174,8 @@ export function registerScanTools( findings, count: findings.length, }); - } catch (error) { - return jsonResult({ error: String(error) }); + } catch { + return jsonResult({ error: "Secret exposure scan failed. Details were withheld to protect scanned source context." }); } }, ); diff --git a/src/reporters/json.test.ts b/src/reporters/json.test.ts index fb82f83..83f2009 100644 --- a/src/reporters/json.test.ts +++ b/src/reporters/json.test.ts @@ -119,22 +119,25 @@ describe("JSON reporter", () => { expect(parsed.summary.score).toBe(100); }); - test("preserves all finding fields", () => { + test("redacts code snippets and sensitive analysis fields", () => { + const syntheticSecret = "ghp_" + "SYNTHETICONLYABCDEFGHIJKLMNOPQRSTUVWXYZ12"; const finding = makeFinding({ column: 10, end_line: 45, - code_snippet: "const key = 'secret';", - llm_explanation: "This is a hardcoded key", - llm_fix: "Use env vars", + code_snippet: `const key = '${syntheticSecret}';`, + llm_explanation: `This is a hardcoded key: ${syntheticSecret}`, + llm_fix: `Remove ${syntheticSecret}`, llm_exploitability: 0.9, }); - const parsed = JSON.parse(reportFindings([finding])); + const output = reportFindings([finding]); + const parsed = JSON.parse(output); const f = parsed.findings[0]; expect(f.column).toBe(10); expect(f.end_line).toBe(45); - expect(f.code_snippet).toBe("const key = 'secret';"); - expect(f.llm_explanation).toBe("This is a hardcoded key"); - expect(f.llm_fix).toBe("Use env vars"); + expect(f.code_snippet).toBe("[REDACTED]"); + expect(f.llm_explanation).toBe("[REDACTED]"); + expect(f.llm_fix).toBe("[REDACTED]"); expect(f.llm_exploitability).toBe(0.9); + expect(output).not.toContain(syntheticSecret); }); }); diff --git a/src/reporters/json.ts b/src/reporters/json.ts index be8c7e5..dc9bff2 100644 --- a/src/reporters/json.ts +++ b/src/reporters/json.ts @@ -1,5 +1,6 @@ import type { Finding, Scan, SecurityScore } from "../types/index.js"; import { Severity } from "../types/index.js"; +import { sanitizeFindingForOutput } from "../lib/finding-safety.js"; function computeScore(findings: Finding[]): SecurityScore { const active = findings.filter((f) => !f.suppressed); @@ -48,10 +49,11 @@ function computeScore(findings: Finding[]): SecurityScore { } export function reportFindings(findings: Finding[], scan?: Scan): string { - const summary = computeScore(findings); + const safeFindings = findings.map(sanitizeFindingForOutput); + const summary = computeScore(safeFindings); const report = { scan: scan ?? null, - findings, + findings: safeFindings, summary, }; return JSON.stringify(report, null, 2); diff --git a/src/reporters/sarif.test.ts b/src/reporters/sarif.test.ts index 3aebfd4..ddbe265 100644 --- a/src/reporters/sarif.test.ts +++ b/src/reporters/sarif.test.ts @@ -157,4 +157,19 @@ describe("SARIF reporter", () => { expect(parsed.runs[0].results).toEqual([]); expect(parsed.runs[0].tool.driver.rules).toEqual([]); }); + + test("does not serialize a credential value embedded in a secret finding", () => { + const syntheticSecret = "ghp_" + "SYNTHETICONLYABCDEFGHIJKLMNOPQRSTUVWXYZ12"; + const output = reportFindings([ + makeFinding({ + scanner_type: ScannerType.Secrets, + rule_id: "github-token", + message: `GitHub token detected: ${syntheticSecret}`, + code_snippet: `GITHUB_TOKEN=${syntheticSecret}`, + }), + ]); + + expect(output).not.toContain(syntheticSecret); + expect(JSON.parse(output).runs[0].results[0].message.text).toContain("Potential credential exposure"); + }); }); diff --git a/src/reporters/sarif.ts b/src/reporters/sarif.ts index bc21fff..09eb2b1 100644 --- a/src/reporters/sarif.ts +++ b/src/reporters/sarif.ts @@ -1,5 +1,6 @@ import type { Finding, Scan } from "../types/index.js"; import { Severity } from "../types/index.js"; +import { sanitizeFindingForOutput } from "../lib/finding-safety.js"; const SEVERITY_TO_LEVEL: Record = { [Severity.Critical]: "error", @@ -36,7 +37,8 @@ export function reportFindings(findings: Finding[], scan?: Scan): string { const rulesMap = new Map(); const results: SarifResult[] = []; - for (const finding of findings) { + for (const rawFinding of findings) { + const finding = sanitizeFindingForOutput(rawFinding); if (!rulesMap.has(finding.rule_id)) { rulesMap.set(finding.rule_id, { id: finding.rule_id, diff --git a/src/reporters/terminal.test.ts b/src/reporters/terminal.test.ts new file mode 100644 index 0000000..7f3fac5 --- /dev/null +++ b/src/reporters/terminal.test.ts @@ -0,0 +1,44 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { ScannerType, Severity, type Finding } from "../types/index.js"; +import { reportFindings } from "./terminal.js"; + +const originalLog = console.log; + +afterEach(() => { + console.log = originalLog; +}); + +describe("terminal reporter safety", () => { + test("never prints secret finding snippets or analysis text", () => { + const syntheticSecret = "ghp_" + "SYNTHETICONLYABCDEFGHIJKLMNOPQRSTUVWXYZ12"; + const output: string[] = []; + console.log = (...args: unknown[]) => output.push(args.map(String).join(" ")); + + const finding: Finding = { + id: "finding-1", + scan_id: "scan-1", + rule_id: "github-token", + scanner_type: ScannerType.Secrets, + severity: Severity.Critical, + file: "synthetic.env", + line: 1, + column: 1, + end_line: null, + message: `GitHub token detected: ${syntheticSecret}`, + code_snippet: `GITHUB_TOKEN=${syntheticSecret}`, + fingerprint: "synthetic-fingerprint", + suppressed: false, + suppressed_reason: null, + llm_explanation: `Credential ${syntheticSecret} is exposed`, + llm_fix: null, + llm_exploitability: null, + created_at: "2026-07-15T00:00:00.000Z", + }; + + reportFindings([finding]); + + const rendered = output.join("\n"); + expect(rendered).not.toContain(syntheticSecret); + expect(rendered).toContain("[REDACTED]"); + }); +}); diff --git a/src/reporters/terminal.ts b/src/reporters/terminal.ts index 668cfa9..d9ff7e4 100644 --- a/src/reporters/terminal.ts +++ b/src/reporters/terminal.ts @@ -5,6 +5,7 @@ import { Severity, SEVERITY_ORDER, } from "../types/index.js"; +import { sanitizeFindingForOutput } from "../lib/finding-safety.js"; const SEVERITY_BADGE: Record string> = { [Severity.Critical]: (t) => chalk.bgRed.white.bold(` ${t} `), @@ -67,7 +68,8 @@ export function reportFindings(findings: Finding[]): void { return; } - const sorted = [...findings].sort( + const safeFindings = findings.map(sanitizeFindingForOutput); + const sorted = [...safeFindings].sort( (a, b) => SEVERITY_ORDER[a.severity] - SEVERITY_ORDER[b.severity], ); @@ -97,7 +99,7 @@ export function reportFindings(findings: Finding[]): void { } // Summary table - const score = computeScore(findings); + const score = computeScore(safeFindings); console.log(chalk.gray("\n " + "─".repeat(70))); console.log(chalk.bold("\n Summary")); console.log( diff --git a/src/scanners/git-history.ts b/src/scanners/git-history.ts index 07eebb2..4d0bb5b 100644 --- a/src/scanners/git-history.ts +++ b/src/scanners/git-history.ts @@ -100,7 +100,7 @@ function scanDiffForSecrets(entries: GitDiffEntry[]): FindingInput[] { file: addedLine.file, line: addedLine.line, message: `${sp.name} found in git history (commit ${entry.commitHash.substring(0, 8)} by ${entry.author} on ${entry.date})`, - code_snippet: addedLine.text, + code_snippet: "[REDACTED]", }); } } diff --git a/src/scanners/secrets.test.ts b/src/scanners/secrets.test.ts index 3244638..8455ed1 100644 --- a/src/scanners/secrets.test.ts +++ b/src/scanners/secrets.test.ts @@ -379,12 +379,13 @@ function greet() { return "hi"; } expect(awsFinding!.column).toBeGreaterThan(0); }); - test("includes code snippet in findings", () => { + test("redacts code snippets in secret findings", () => { const content = "line 1\nline 2\nconst key = \"AKIAIOSFODNN7EXAMPLE\";\nline 4"; const findings = scanFile("test.ts", content); const awsFinding = findings.find((f) => f.rule_id === "aws-access-key"); expect(awsFinding!.code_snippet).toBeDefined(); - expect(awsFinding!.code_snippet).toContain("AKIAIOSFODNN7EXAMPLE"); + expect(awsFinding!.code_snippet).toBe("[REDACTED]"); + expect(JSON.stringify(awsFinding)).not.toContain("AKIAIOSFODNN7EXAMPLE"); }); }); diff --git a/src/scanners/secrets.ts b/src/scanners/secrets.ts index 4722e68..b95ebd3 100644 --- a/src/scanners/secrets.ts +++ b/src/scanners/secrets.ts @@ -83,6 +83,8 @@ export function getCodeSnippet(content: string, line: number, context: number = .join("\n"); } +const REDACTED_CODE_SNIPPET = "[REDACTED]"; + const SECURITY_IGNORE = "security-ignore"; const SLASH_COMMENT_EXTENSIONS = new Set([ ".c", @@ -695,7 +697,7 @@ function detectUnquotedEnvApiKeys( line, column: match.index + 1, message: "Generic API Key detected", - code_snippet: getCodeSnippet(content, line), + code_snippet: REDACTED_CODE_SNIPPET, }); } @@ -723,8 +725,8 @@ function detectHighEntropyStrings( severity: Severity.Medium, file: filePath, line, - message: `High-entropy hex string detected (possible secret): ${token.substring(0, 16)}...`, - code_snippet: getCodeSnippet(content, line), + message: "High-entropy hex string detected (possible secret)", + code_snippet: REDACTED_CODE_SNIPPET, }); } } @@ -741,8 +743,8 @@ function detectHighEntropyStrings( severity: Severity.Medium, file: filePath, line, - message: `High-entropy base64 string detected (possible secret): ${token.substring(0, 16)}...`, - code_snippet: getCodeSnippet(content, line), + message: "High-entropy base64 string detected (possible secret)", + code_snippet: REDACTED_CODE_SNIPPET, }); } } @@ -790,7 +792,7 @@ export function scanFile(filePath: string, content: string): FindingInput[] { line: lineNum, column: match.index + 1, message: `${sp.name} detected`, - code_snippet: getCodeSnippet(content, lineNum), + code_snippet: REDACTED_CODE_SNIPPET, }); } } diff --git a/src/server/serve.ts b/src/server/serve.ts index 2f68879..33f2fea 100644 --- a/src/server/serve.ts +++ b/src/server/serve.ts @@ -45,6 +45,7 @@ import { analyzeFinding as llmAnalyze, isLLMAvailable, } from "../llm/index.js"; +import { isCredentialFinding } from "../lib/finding-safety.js"; import { listAdvisories, getAdvisory, @@ -185,6 +186,7 @@ export function startServer(port: number) { const batch = findings.slice(i, i + BATCH_SIZE); await Promise.allSettled( batch.map(async (finding) => { + if (isCredentialFinding(finding)) return; const context = getCodeContext(finding.file, finding.line); if (context) { const analysis = await llmAnalyze(finding, context); @@ -327,6 +329,11 @@ export function startServer(port: number) { return; } + if (isCredentialFinding(finding)) { + res.status(409).json({ error: "LLM features are disabled for credential findings" }); + return; + } + if (!isLLMAvailable()) { res.status(503).json({ error: "LLM not available. Set CEREBRAS_API_KEY." }); return; @@ -367,6 +374,11 @@ export function startServer(port: number) { return; } + if (isCredentialFinding(finding)) { + res.status(409).json({ error: "LLM features are disabled for credential findings" }); + return; + } + if (!isLLMAvailable()) { res.status(503).json({ error: "LLM not available. Set CEREBRAS_API_KEY." }); return; From 18d1741971a5f7f2c9d7c5f07df83dedff9a0a2f Mon Sep 17 00:00:00 2001 From: Andrei Hasna Date: Thu, 16 Jul 2026 00:13:22 +0300 Subject: [PATCH 2/8] fix: enforce scan source and output boundaries --- README.md | 31 ++++-- sdk/src/client.test.ts | 35 +++++++ sdk/src/client.ts | 7 +- sdk/tsconfig.json | 2 +- src/cli/commands/scan-boundary.test.ts | 72 ++++++++++++++ src/cli/commands/scan.ts | 37 +++++--- src/cli/commands/secrets.ts | 2 +- src/cli/helpers.test.ts | 27 +++++- src/cli/helpers.ts | 14 ++- src/db/findings.test.ts | 25 ++++- src/db/findings.ts | 75 ++++++++++++++- src/db/llm-cache.test.ts | 38 ++++++++ src/db/llm-cache.ts | 44 +++++++-- src/db/projects.test.ts | 19 ++++ src/db/projects.ts | 39 ++++++-- src/lib/finding-safety.test.ts | 26 +++++ src/lib/finding-safety.ts | 126 +++++++++++++++++++++---- src/lib/secret-exposure.test.ts | 17 +++- src/lib/secret-exposure.ts | 8 +- src/llm/analyzer.ts | 14 +-- src/llm/client.ts | 24 ++++- src/llm/credential-boundary.test.ts | 13 +++ src/llm/explainer.ts | 19 ++-- src/llm/fixer.ts | 19 ++-- src/llm/triager.ts | 16 ++-- src/mcp/tools/output-safety.test.ts | 18 ++++ src/mcp/tools/scan.ts | 30 +++--- src/reporters/json.test.ts | 13 +++ src/scanners/index.ts | 45 ++++++--- src/scanners/ioc.ts | 11 ++- src/scanners/lockfile.ts | 8 +- src/scanners/secrets.ts | 23 ++++- src/scanners/source-boundary.test.ts | 29 ++++++ src/server/scan-boundary.test.ts | 74 +++++++++++++++ src/server/serve.ts | 36 ++++--- src/types/index.ts | 22 ++++- 36 files changed, 894 insertions(+), 164 deletions(-) create mode 100644 sdk/src/client.test.ts create mode 100644 src/cli/commands/scan-boundary.test.ts create mode 100644 src/db/llm-cache.test.ts create mode 100644 src/scanners/source-boundary.test.ts create mode 100644 src/server/scan-boundary.test.ts diff --git a/README.md b/README.md index 3e92972..aceaa3a 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,10 @@ bun install -g @hasna/shield # Scan your repo for security issues shield scan . +# Wider sources are separate, per-invocation opt-ins +shield scan . --git-history +shield scan . --system + # Focused secret-exposure scan (safe default: repository files only) shield secrets . @@ -54,8 +58,8 @@ shield init --install-pre-push | `git-history` | Secrets committed in git history | | `config` | Insecure CORS, debug mode, missing security headers | | `ai-safety` | Prompt injection, PII exposure, unsafe tool use | -| `ioc` | Supply chain attack indicators (C2 domains, RAT artifacts, malicious packages) | -| `lockfile` | Compromised locked versions, unpinned ranges during attack windows | +| `ioc` | In-tree C2/malicious-package indicators; host RAT/Python paths require `--system` | +| `lockfile` | Compromised locked versions and unpinned ranges; history requires `--git-history` | | `supply-chain` | Typosquatting, postinstall exploits, GitHub Actions tag hijacking | ## Supply Chain Attack Detection @@ -129,6 +133,12 @@ API endpoints: - `GET /api/findings` — query scan findings - `POST /api/scans` — trigger a new scan +CLI, library, SDK, MCP, REST, and dashboard-triggered aggregate scans inspect +only the requested filesystem tree by default. REST/SDK/MCP callers must send +`include_git_history: true` or `include_system: true` for the corresponding +wider source. Merely listing `git-history` in a REST/MCP scanner array does not +authorize history access. + ## All CLI Commands ``` @@ -200,18 +210,21 @@ shield fleet-package ./package.tgz --json ### Migration warning for 0.1.25 and earlier -Versions through 0.1.25 enabled git-history, process, and tmux sources by -default. Structured output could therefore include credential-bearing source -context. Upgrade before using `shield secrets` in an agent, CI job, log -collector, or transcript-producing tool. Until the fixed version is installed, +Versions through 0.1.25 allowed aggregate and focused paths to cross historical +or live-machine boundaries without a consistent per-invocation opt-in. +Structured output could therefore include credential-bearing source context. +Upgrade before using Shield in an agent, CI job, log collector, or +transcript-producing tool. Until the fixed version is installed, use `shield secrets . --repo-only --no-git-history --no-processes --no-tmux` or use the `secrets scan workspace` and `shield fleet-package` file/archive paths. If an older structured scan ran in a credential-bearing environment, treat the visible credential identifiers as exposed, preserve values out of incident channels, and follow the owning vault/provider rotation runbook. -Existing database rows are sanitized when read but are not destructively -rewritten by this update; purge or migration of historical local state requires -separate incident-owner authorization. Credential-finding fingerprints may +Existing finding rows are sanitized on read and the sanitized fields are then +written back when the local database is writable. Stable non-sensitive hashes +retain correlation without retaining the credential-bearing location or rule +identifier. A read-only database still receives sanitized API/MCP/reporter +output, but cannot be rewritten in place. Credential-finding fingerprints may change once newly scanned records use the redacted persistence form. For publishable OSS packages, see diff --git a/sdk/src/client.test.ts b/sdk/src/client.test.ts new file mode 100644 index 0000000..dd1edaa --- /dev/null +++ b/sdk/src/client.test.ts @@ -0,0 +1,35 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { OpenSecurityClient } from "./client.js"; + +const originalFetch = globalThis.fetch; + +afterEach(() => { + globalThis.fetch = originalFetch; +}); + +describe("OpenSecurityClient scan source boundary", () => { + test("omits sensitive-source opt-ins by default and forwards explicit choices", async () => { + const bodies: Array> = []; + globalThis.fetch = (async (_input: string | URL | Request, init?: RequestInit) => { + bodies.push(JSON.parse(String(init?.body))); + return new Response(JSON.stringify({ id: "scan", scanner_types: [] }), { + status: 200, + headers: { "content-type": "application/json" }, + }); + }) as unknown as typeof fetch; + + const client = new OpenSecurityClient("http://127.0.0.1:1"); + await client.triggerScan("/synthetic/repo"); + await client.triggerScan("/synthetic/repo", { + include_git_history: true, + include_system: true, + }); + + expect(bodies[0]).toEqual({ path: "/synthetic/repo" }); + expect(bodies[1]).toEqual({ + path: "/synthetic/repo", + include_git_history: true, + include_system: true, + }); + }); +}); diff --git a/sdk/src/client.ts b/sdk/src/client.ts index 3e211dd..547cf45 100644 --- a/sdk/src/client.ts +++ b/sdk/src/client.ts @@ -36,7 +36,12 @@ export class OpenSecurityClient { async triggerScan( path: string, - options?: { scanners?: string[]; llm_analyze?: boolean }, + options?: { + scanners?: string[]; + include_git_history?: boolean; + include_system?: boolean; + llm_analyze?: boolean; + }, ): Promise { return this.request("/api/scans", { method: "POST", diff --git a/sdk/tsconfig.json b/sdk/tsconfig.json index 72f9fb6..245fc0d 100644 --- a/sdk/tsconfig.json +++ b/sdk/tsconfig.json @@ -18,5 +18,5 @@ "noFallthroughCasesInSwitch": true }, "include": ["src"], - "exclude": ["node_modules", "dist"] + "exclude": ["node_modules", "dist", "src/**/*.test.ts"] } diff --git a/src/cli/commands/scan-boundary.test.ts b/src/cli/commands/scan-boundary.test.ts new file mode 100644 index 0000000..0399d65 --- /dev/null +++ b/src/cli/commands/scan-boundary.test.ts @@ -0,0 +1,72 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { execFileSync, spawnSync } from "child_process"; +import { mkdtempSync, rmSync, symlinkSync, writeFileSync } from "fs"; +import { tmpdir } from "os"; +import { join } from "path"; + +describe("CLI scan source and error boundaries", () => { + let tempDir: string; + let repoDir: string; + let env: NodeJS.ProcessEnv; + + beforeEach(() => { + tempDir = mkdtempSync(join(tmpdir(), "shield-cli-boundary-")); + repoDir = join(tempDir, "repo"); + execFileSync("mkdir", ["-p", repoDir]); + env = { + ...process.env, + HOME: tempDir, + USERPROFILE: tempDir, + SECURITY_DB: join(tempDir, "shield.db"), + CEREBRAS_API_KEY: "", + }; + }); + + afterEach(() => rmSync(tempDir, { recursive: true, force: true })); + + test("ordinary scan omits history until the current command explicitly opts in", () => { + const syntheticSecret = "ghp_" + "SYNTHETICONLYABCDEFGHIJKLMNOPQRSTUVWXYZ12"; + execFileSync("git", ["init", "-q"], { cwd: repoDir }); + execFileSync("git", ["config", "user.email", "synthetic@example.invalid"], { cwd: repoDir }); + execFileSync("git", ["config", "user.name", "Synthetic Test"], { cwd: repoDir }); + writeFileSync(join(repoDir, "history.txt"), `TOKEN=${syntheticSecret}\n`, "utf-8"); + execFileSync("git", ["add", "history.txt"], { cwd: repoDir }); + execFileSync("git", ["commit", "-qm", "synthetic secret"], { cwd: repoDir }); + writeFileSync(join(repoDir, "history.txt"), "safe=true\n", "utf-8"); + execFileSync("git", ["add", "history.txt"], { cwd: repoDir }); + execFileSync("git", ["commit", "-qm", "remove synthetic secret"], { cwd: repoDir }); + + const normal = spawnSync("bun", ["run", "src/cli/index.tsx", "scan", repoDir, "--format", "json"], { + cwd: process.cwd(), env, encoding: "utf-8", + }); + expect(normal.status).toBe(0); + expect(normal.stderr).not.toContain("git-history"); + expect(normal.stdout).not.toContain(syntheticSecret); + + const optedIn = spawnSync("bun", ["run", "src/cli/index.tsx", "scan", repoDir, "--format", "json", "--git-history"], { + cwd: process.cwd(), env, encoding: "utf-8", + }); + expect(optedIn.stderr).toContain("git-history"); + expect(`${optedIn.stdout}${optedIn.stderr}`).not.toContain(syntheticSecret); + }); + + test("files-only command scans regular files and withholds failing paths", () => { + const syntheticSecret = "ghp_" + "SYNTHETICONLYABCDEFGHIJKLMNOPQRSTUVWXYZ12"; + const file = join(tempDir, "synthetic.env"); + writeFileSync(file, `TOKEN=${syntheticSecret}\n`, "utf-8"); + const regular = spawnSync("bun", ["run", "src/cli/index.tsx", "secrets", file, "--files-only", "--json"], { + cwd: process.cwd(), env, encoding: "utf-8", + }); + expect(regular.status).toBe(1); + expect(regular.stdout).toContain('"total"'); + expect(regular.stdout).not.toContain(syntheticSecret); + + const loop = join(tempDir, syntheticSecret); + symlinkSync(loop, loop); + const failed = spawnSync("bun", ["run", "src/cli/index.tsx", "secrets", loop, "--files-only", "--json"], { + cwd: process.cwd(), env, encoding: "utf-8", + }); + expect(failed.status).toBe(1); + expect(`${failed.stdout}${failed.stderr}`).not.toContain(syntheticSecret); + }); +}); diff --git a/src/cli/commands/scan.ts b/src/cli/commands/scan.ts index aad9780..5a91cdc 100644 --- a/src/cli/commands/scan.ts +++ b/src/cli/commands/scan.ts @@ -10,7 +10,7 @@ import { runScanner, getScanner } from "../../scanners/index.js"; import { isLLMAvailable, analyzeFinding as llmAnalyzeFinding } from "../../llm/index.js"; import { getReporter } from "../../reporters/index.js"; import { loadConfig } from "../../lib/index.js"; -import { isCredentialFinding } from "../../lib/finding-safety.js"; +import { isCredentialFinding, sanitizeLocationForOutput } from "../../lib/finding-safety.js"; import { parseFormat, parseSeverity, resolveScannerTypes, filterBySeverity, ensureProject, getCodeContext, @@ -23,6 +23,10 @@ export function registerScanCommand(program: Command): void { .argument("[path]", "Path to scan", ".") .option("--quick", "Quick scan (secrets + dependencies only)") .option("--scanner ", "Run specific scanner only") + .option("--git-history", "Explicitly include the sensitive git-history scanner", false) + .option("--no-git-history", "Compatibility flag; git history is disabled by default") + .option("--system", "Explicitly include host/system IOC locations outside the requested path", false) + .option("--no-system", "Compatibility flag; host/system inspection is disabled by default") .option("--format ", "Output format (terminal/json/sarif)", "terminal") .option("--severity ", "Minimum severity threshold", "info") .option("--llm", "Enable LLM analysis of findings") @@ -30,7 +34,7 @@ export function registerScanCommand(program: Command): void { .action(async (path: string, options) => { const scanPath = resolve(path); if (!existsSync(scanPath)) { - console.error(chalk.red(`Path does not exist: ${scanPath}`)); + console.error(chalk.red("Requested scan path does not exist")); process.exit(1); } @@ -38,7 +42,12 @@ export function registerScanCommand(program: Command): void { const config = loadConfig(scanPath); const format = parseFormat(options.format); const severityThreshold = parseSeverity(options.severity); - const scannerTypes = resolveScannerTypes(options.scanner, options.quick, config); + const scannerTypes = resolveScannerTypes( + options.scanner, + options.quick, + config, + options.gitHistory === true, + ); const useLLM = options.llm || config.llm_analyze; getDb(); @@ -52,7 +61,7 @@ export function registerScanCommand(program: Command): void { ? (msg: string) => process.stderr.write(msg + "\n") : console.log; - log(chalk.bold(`\n Scanning ${chalk.cyan(scanPath)}...`)); + log(chalk.bold(`\n Scanning ${chalk.cyan(sanitizeLocationForOutput(scanPath))}...`)); log(chalk.gray(` Scanners: ${scannerTypes.join(", ")}`)); const startTime = Date.now(); @@ -61,19 +70,22 @@ export function registerScanCommand(program: Command): void { if (scannerTypes.length === 1) { findingInputs = await runScanner(scannerTypes[0], scanPath, { ignore_patterns: config.ignore_patterns, + include_git_history: options.gitHistory === true, + include_system: options.system === true, }); } else { - const results = await Promise.allSettled( + const results = await Promise.all( scannerTypes.map((type) => { const scanner = getScanner(type); - if (!scanner) return Promise.resolve([]); - return scanner.scan(scanPath, { ignore_patterns: config.ignore_patterns }); + if (!scanner) throw new Error(`Scanner not found: ${type}`); + return scanner.scan(scanPath, { + ignore_patterns: config.ignore_patterns, + include_git_history: options.gitHistory === true, + include_system: options.system === true, + }); }), ); - for (const result of results) { - if (result.status === "fulfilled") findingInputs.push(...result.value); - else console.error(chalk.yellow(` Warning: scanner failed - ${result.reason}`)); - } + findingInputs = results.flat(); } const storedFindings: Finding[] = []; @@ -120,8 +132,7 @@ export function registerScanCommand(program: Command): void { process.exit(1); } } catch (error) { - const errMsg = error instanceof Error ? error.message : String(error); - console.error(chalk.red(`\n Scan failed: ${errMsg}\n`)); + console.error(chalk.red("\n Scan failed. Details were withheld to protect scanned source context.\n")); process.exit(1); } }); diff --git a/src/cli/commands/secrets.ts b/src/cli/commands/secrets.ts index 4648e64..36bef2f 100644 --- a/src/cli/commands/secrets.ts +++ b/src/cli/commands/secrets.ts @@ -122,7 +122,7 @@ export function registerSecretsCommand(program: Command): void { .action(async (pathArg: string, options) => { const scanPath = resolve(pathArg); if (!existsSync(scanPath)) { - console.error(chalk.red(`\n Path does not exist: ${scanPath}\n`)); + console.error(chalk.red("\n Requested scan path does not exist.\n")); process.exit(1); } diff --git a/src/cli/helpers.test.ts b/src/cli/helpers.test.ts index 97ff7ea..642f3a2 100644 --- a/src/cli/helpers.test.ts +++ b/src/cli/helpers.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "bun:test"; -import { ReportFormat, ScannerType, Severity } from "../types/index.js"; -import { parseFormat, parseScannerType, parseSeverity } from "./helpers.js"; +import { DEFAULT_CONFIG, ReportFormat, ScannerType, Severity } from "../types/index.js"; +import { parseFormat, parseScannerType, parseSeverity, resolveScannerTypes } from "./helpers.js"; describe("parseSeverity", () => { it("parses valid severities case-insensitively", () => { @@ -34,3 +34,26 @@ describe("parseScannerType", () => { expect(() => parseScannerType("foo")).toThrow("Invalid scanner"); }); }); + +describe("resolveScannerTypes source boundary", () => { + it("filters legacy config git history unless the current invocation opts in", () => { + const legacyConfig = { + ...DEFAULT_CONFIG, + enabled_scanners: [...DEFAULT_CONFIG.enabled_scanners, ScannerType.GitHistory], + }; + expect(resolveScannerTypes(undefined, false, legacyConfig)).not.toContain(ScannerType.GitHistory); + expect(resolveScannerTypes(undefined, false, legacyConfig, true)).toContain(ScannerType.GitHistory); + }); + + it("treats naming git-history with --scanner as an explicit opt-in", () => { + expect(resolveScannerTypes("git-history", false, DEFAULT_CONFIG)).toEqual([ScannerType.GitHistory]); + }); + + it("honors an explicit history opt-in alongside quick or named scanners", () => { + expect(resolveScannerTypes(undefined, true, DEFAULT_CONFIG, true)).toContain(ScannerType.GitHistory); + expect(resolveScannerTypes("code", false, DEFAULT_CONFIG, true)).toEqual([ + ScannerType.Code, + ScannerType.GitHistory, + ]); + }); +}); diff --git a/src/cli/helpers.ts b/src/cli/helpers.ts index 40bc83e..32c4125 100644 --- a/src/cli/helpers.ts +++ b/src/cli/helpers.ts @@ -52,12 +52,20 @@ export function resolveScannerTypes( scannerArg: string | undefined, quick: boolean, config: ConfigFile, + includeGitHistory = false, ): ScannerType[] { + let scannerTypes: ScannerType[]; if (scannerArg) { - return [parseScannerType(scannerArg)]; + scannerTypes = [parseScannerType(scannerArg)]; + } else if (quick) { + scannerTypes = [ScannerType.Secrets, ScannerType.Dependencies]; + } else { + scannerTypes = config.enabled_scanners.filter((type) => type !== ScannerType.GitHistory); } - if (quick) return [ScannerType.Secrets, ScannerType.Dependencies]; - return config.enabled_scanners; + if (includeGitHistory && !scannerTypes.includes(ScannerType.GitHistory)) { + scannerTypes.push(ScannerType.GitHistory); + } + return scannerTypes; } export function parseSeverity(level: string): Severity { diff --git a/src/db/findings.test.ts b/src/db/findings.test.ts index 4c532ca..72fa167 100644 --- a/src/db/findings.test.ts +++ b/src/db/findings.test.ts @@ -101,6 +101,20 @@ describe("findings", () => { expect(finding.code_snippet).toBeNull(); }); + test("createFinding replaces credential-bearing rule and path fields before SQLite", () => { + const syntheticSecret = "sk_test_" + "SYNTHETICONLY0123456789"; + const finding = createFinding(scanId, makeInput({ + rule_id: `unsafe-${syntheticSecret}`, + scanner_type: ScannerType.Code, + file: `src/${syntheticSecret}/app.ts`, + message: "Unsafe code path", + })); + const raw = getCurrentTestDb().prepare("SELECT rule_id, file, message FROM findings WHERE id = ?").get(finding.id); + expect(JSON.stringify(raw)).not.toContain(syntheticSecret); + expect(finding.rule_id).toContain("REDACTED-RULE"); + expect(finding.file).toContain("REDACTED-LOCATION"); + }); + test("getFinding retrieves a finding by id", () => { const created = createFinding(scanId, makeInput()); const fetched = getFinding(created.id); @@ -113,9 +127,16 @@ describe("findings", () => { const syntheticSecret = "ghp_" + "SYNTHETICONLYABCDEFGHIJKLMNOPQRSTUVWXYZ12"; const created = createFinding(scanId, makeInput()); const db = getCurrentTestDb(); - db.prepare("UPDATE findings SET message = ?, code_snippet = ? WHERE id = ?").run( + db.prepare( + `INSERT INTO rules (id, name, description, scanner_type, severity, enabled, builtin, metadata, created_at, updated_at) + VALUES (?, 'Legacy synthetic rule', '', 'secrets', 'high', 1, 0, '{}', datetime('now'), datetime('now'))`, + ).run(`legacy-${syntheticSecret}`); + db.prepare("UPDATE findings SET rule_id = ?, file = ?, message = ?, code_snippet = ?, llm_explanation = ? WHERE id = ?").run( + `legacy-${syntheticSecret}`, + `legacy/${syntheticSecret}/config.ts`, `GitHub token detected: ${syntheticSecret}`, `GITHUB_TOKEN=${syntheticSecret}`, + `Adjacent context ${syntheticSecret}`, created.id, ); @@ -123,6 +144,8 @@ describe("findings", () => { expect(JSON.stringify(fetched)).not.toContain(syntheticSecret); expect(fetched?.message).toContain("Potential credential exposure"); expect(fetched?.code_snippet).toBe("[REDACTED]"); + const raw = db.prepare("SELECT rule_id, file, message, code_snippet, llm_explanation FROM findings WHERE id = ?").get(created.id); + expect(JSON.stringify(raw)).not.toContain(syntheticSecret); }); test("getFinding returns null for unknown id", () => { diff --git a/src/db/findings.ts b/src/db/findings.ts index 9302fc1..831747c 100644 --- a/src/db/findings.ts +++ b/src/db/findings.ts @@ -8,6 +8,7 @@ import { REDACTED_FINDING_TEXT, sanitizeFindingForOutput, sanitizeFindingForPersistence, + sanitizeTextForBoundary, } from "../lib/finding-safety.js"; interface FindingRow { @@ -32,12 +33,54 @@ interface FindingRow { } function rowToFinding(row: FindingRow): Finding { - return sanitizeFindingForOutput({ + const safe = sanitizeFindingForOutput({ ...row, scanner_type: row.scanner_type as ScannerType, severity: row.severity as Severity, suppressed: row.suppressed === 1, }); + // Opportunistically scrub legacy rows so direct database consumers after + // first read cannot recover fields written by older unsafe versions. + if ( + safe.rule_id !== row.rule_id || + safe.file !== row.file || + safe.message !== row.message || + safe.code_snippet !== row.code_snippet || + safe.suppressed_reason !== row.suppressed_reason || + safe.llm_explanation !== row.llm_explanation || + safe.llm_fix !== row.llm_fix + ) { + try { + const db = getDb(); + if (safe.rule_id !== row.rule_id) { + db.prepare( + `INSERT OR IGNORE INTO rules (id, name, description, scanner_type, severity, enabled, builtin, metadata, created_at, updated_at) + VALUES (?, 'Redacted legacy finding rule', 'Credential-bearing legacy rule identifier was replaced', ?, ?, 1, 0, '{}', datetime('now'), datetime('now'))`, + ).run(safe.rule_id, safe.scanner_type, safe.severity); + } + db.prepare( + `UPDATE findings SET rule_id = ?, file = ?, message = ?, code_snippet = ?, suppressed_reason = ?, llm_explanation = ?, llm_fix = ? WHERE id = ?`, + ).run( + safe.rule_id, + safe.file, + safe.message, + safe.code_snippet, + safe.suppressed_reason, + safe.llm_explanation, + safe.llm_fix, + row.id, + ); + if (safe.rule_id !== row.rule_id) { + db.prepare( + `DELETE FROM rules WHERE id = ? AND NOT EXISTS (SELECT 1 FROM findings WHERE rule_id = ?)`, + ).run(row.rule_id, row.rule_id); + } + } catch { + // Output remains sanitized even when a legacy/read-only database cannot + // be rewritten in place. + } + } + return safe; } function generateFingerprint(rule_id: string, file: string, line: number, message: string): string { @@ -50,6 +93,12 @@ function generateFingerprint(rule_id: string, file: string, line: number, messag export function createFinding(scan_id: string, input: FindingInput): Finding { const db = getDb(); const safeInput = sanitizeFindingForPersistence(input); + if (safeInput.rule_id !== input.rule_id) { + db.prepare( + `INSERT OR IGNORE INTO rules (id, name, description, scanner_type, severity, enabled, builtin, metadata, created_at, updated_at) + VALUES (?, 'Redacted finding rule', 'Credential-bearing rule identifier was replaced before persistence', ?, ?, 1, 0, '{}', datetime('now'), datetime('now'))`, + ).run(safeInput.rule_id, safeInput.scanner_type, safeInput.severity); + } const id = crypto.randomUUID(); const now = new Date().toISOString(); const fingerprint = generateFingerprint(safeInput.rule_id, safeInput.file, safeInput.line, safeInput.message); @@ -167,15 +216,33 @@ export function updateFinding( } if (updates.suppressed_reason !== undefined) { sets.push("suppressed_reason = ?"); - params.push(sensitive && updates.suppressed_reason != null ? REDACTED_FINDING_TEXT : updates.suppressed_reason); + params.push( + updates.suppressed_reason == null + ? updates.suppressed_reason + : sensitive + ? REDACTED_FINDING_TEXT + : sanitizeTextForBoundary(updates.suppressed_reason), + ); } if (updates.llm_explanation !== undefined) { sets.push("llm_explanation = ?"); - params.push(sensitive && updates.llm_explanation != null ? REDACTED_FINDING_TEXT : updates.llm_explanation); + params.push( + updates.llm_explanation == null + ? updates.llm_explanation + : sensitive + ? REDACTED_FINDING_TEXT + : sanitizeTextForBoundary(updates.llm_explanation), + ); } if (updates.llm_fix !== undefined) { sets.push("llm_fix = ?"); - params.push(sensitive && updates.llm_fix != null ? REDACTED_FINDING_TEXT : updates.llm_fix); + params.push( + updates.llm_fix == null + ? updates.llm_fix + : sensitive + ? REDACTED_FINDING_TEXT + : sanitizeTextForBoundary(updates.llm_fix), + ); } if (updates.llm_exploitability !== undefined) { sets.push("llm_exploitability = ?"); diff --git a/src/db/llm-cache.test.ts b/src/db/llm-cache.test.ts new file mode 100644 index 0000000..45422ad --- /dev/null +++ b/src/db/llm-cache.test.ts @@ -0,0 +1,38 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { getCurrentTestDb, setupTestDb } from "./test-helpers.js"; +import { cacheAnalysis, getCachedAnalysis } from "./llm-cache.js"; + +describe("LLM cache boundary", () => { + let cleanup: () => void; + + beforeEach(() => { + cleanup = setupTestDb(); + }); + + afterEach(() => cleanup()); + + test("sanitizes nested result, identifiers, and model before SQLite", () => { + const syntheticSecret = "ghp_" + "SYNTHETICONLYABCDEFGHIJKLMNOPQRSTUVWXYZ12"; + cacheAnalysis( + syntheticSecret, + `analysis-${syntheticSecret}`, + { explanation: `Adjacent ${syntheticSecret}`, nested: { value: syntheticSecret } }, + `model-${syntheticSecret}`, + 1, + ); + const raw = getCurrentTestDb().prepare("SELECT * FROM llm_cache").get(); + expect(JSON.stringify(raw)).not.toContain(syntheticSecret); + expect(JSON.stringify(getCachedAnalysis(syntheticSecret, `analysis-${syntheticSecret}`))).not.toContain(syntheticSecret); + }); + + test("sanitizes and rewrites legacy cache results on read", () => { + const syntheticSecret = "sk_test_" + "SYNTHETICONLY0123456789"; + const db = getCurrentTestDb(); + db.prepare( + `INSERT INTO llm_cache (id, finding_fingerprint, analysis_type, result, model, tokens_used, created_at) + VALUES ('legacy', 'safe-fingerprint', 'explain', ?, 'synthetic-model', 1, datetime('now'))`, + ).run(JSON.stringify({ text: `Adjacent ${syntheticSecret}` })); + expect(JSON.stringify(getCachedAnalysis("safe-fingerprint", "explain"))).not.toContain(syntheticSecret); + expect(JSON.stringify(db.prepare("SELECT result FROM llm_cache WHERE id = 'legacy'").get())).not.toContain(syntheticSecret); + }); +}); diff --git a/src/db/llm-cache.ts b/src/db/llm-cache.ts index b9ba3c1..998e1e5 100644 --- a/src/db/llm-cache.ts +++ b/src/db/llm-cache.ts @@ -1,17 +1,43 @@ import crypto from "crypto"; import { getDb } from "./database.js"; +import { sanitizeTextForBoundary, sanitizeValueForBoundary } from "../lib/finding-safety.js"; export function getCachedAnalysis( fingerprint: string, analysis_type: string ): Record | null { const db = getDb(); + const safeFingerprint = sanitizeTextForBoundary(fingerprint, 256); + const safeAnalysisType = sanitizeTextForBoundary(analysis_type, 128); const stmt = db.prepare( - `SELECT result FROM llm_cache WHERE finding_fingerprint = ? AND analysis_type = ?` + `SELECT id, result, finding_fingerprint, analysis_type FROM llm_cache + WHERE (finding_fingerprint = ? AND analysis_type = ?) + OR (finding_fingerprint = ? AND analysis_type = ?) + LIMIT 1` ); - const row = stmt.get(fingerprint, analysis_type) as { result: string } | undefined; + const row = stmt.get(safeFingerprint, safeAnalysisType, fingerprint, analysis_type) as { + id: string; + result: string; + finding_fingerprint: string; + analysis_type: string; + } | undefined; if (!row) return null; - return JSON.parse(row.result) as Record; + const safeResult = sanitizeValueForBoundary(JSON.parse(row.result) as Record); + const safeResultJson = JSON.stringify(safeResult); + if ( + row.result !== safeResultJson || + row.finding_fingerprint !== safeFingerprint || + row.analysis_type !== safeAnalysisType + ) { + try { + db.prepare( + "UPDATE llm_cache SET finding_fingerprint = ?, analysis_type = ?, result = ? WHERE id = ?", + ).run(safeFingerprint, safeAnalysisType, safeResultJson, row.id); + } catch { + // Return remains sanitized when legacy/read-only cache rows cannot change. + } + } + return safeResult; } export function cacheAnalysis( @@ -23,7 +49,10 @@ export function cacheAnalysis( ): void { const db = getDb(); const now = new Date().toISOString(); - const resultJson = JSON.stringify(result); + const safeFingerprint = sanitizeTextForBoundary(fingerprint, 256); + const safeAnalysisType = sanitizeTextForBoundary(analysis_type, 128); + const resultJson = JSON.stringify(sanitizeValueForBoundary(result)); + const safeModel = sanitizeTextForBoundary(model, 256); const stmt = db.prepare( `INSERT INTO llm_cache (id, finding_fingerprint, analysis_type, result, model, tokens_used, created_at) @@ -34,14 +63,15 @@ export function cacheAnalysis( tokens_used = excluded.tokens_used, created_at = excluded.created_at` ); - stmt.run(crypto.randomUUID(), fingerprint, analysis_type, resultJson, model, tokens_used, now); + stmt.run(crypto.randomUUID(), safeFingerprint, safeAnalysisType, resultJson, safeModel, tokens_used, now); } export function invalidateCache(fingerprint?: string): void { const db = getDb(); if (fingerprint) { - const stmt = db.prepare(`DELETE FROM llm_cache WHERE finding_fingerprint = ?`); - stmt.run(fingerprint); + const safeFingerprint = sanitizeTextForBoundary(fingerprint, 256); + const stmt = db.prepare(`DELETE FROM llm_cache WHERE finding_fingerprint = ? OR finding_fingerprint = ?`); + stmt.run(safeFingerprint, fingerprint); } else { db.prepare(`DELETE FROM llm_cache`).run(); } diff --git a/src/db/projects.test.ts b/src/db/projects.test.ts index e26ea70..30187f6 100644 --- a/src/db/projects.test.ts +++ b/src/db/projects.test.ts @@ -49,6 +49,25 @@ describe("projects", () => { expect(result).toBeNull(); }); + test("credential-bearing project paths are replaced before persistence and on legacy readback", () => { + const syntheticSecret = "ghp_" + "SYNTHETICONLYABCDEFGHIJKLMNOPQRSTUVWXYZ12"; + const rawPath = `/tmp/${syntheticSecret}/repo`; + const created = createProject(`project-${syntheticSecret}`, rawPath); + expect(JSON.stringify(created)).not.toContain(syntheticSecret); + expect(getProjectByPath(rawPath)?.id).toBe(created.id); + + const db = getCurrentTestDb(); + db.prepare("UPDATE projects SET name = ?, path = ? WHERE id = ?").run( + `legacy-${syntheticSecret}`, + rawPath, + created.id, + ); + const fetched = getProject(created.id); + expect(JSON.stringify(fetched)).not.toContain(syntheticSecret); + const raw = db.prepare("SELECT name, path FROM projects WHERE id = ?").get(created.id); + expect(JSON.stringify(raw)).not.toContain(syntheticSecret); + }); + test("listProjects returns all projects", () => { createProject("project-a", "/path/a"); createProject("project-b", "/path/b"); diff --git a/src/db/projects.ts b/src/db/projects.ts index 7447bc9..041ecf7 100644 --- a/src/db/projects.ts +++ b/src/db/projects.ts @@ -1,38 +1,63 @@ import crypto from "crypto"; import { getDb } from "./database.js"; import type { Project } from "../types/index.js"; +import { sanitizeLocationForOutput, sanitizeTextForBoundary } from "../lib/finding-safety.js"; + +function rowToProject(row: Project): Project { + const safe = { + ...row, + name: sanitizeTextForBoundary(row.name, 256), + path: sanitizeLocationForOutput(row.path), + }; + if (safe.name !== row.name || safe.path !== row.path) { + try { + getDb().prepare("UPDATE projects SET name = ?, path = ?, updated_at = ? WHERE id = ?").run( + safe.name, + safe.path, + new Date().toISOString(), + row.id, + ); + } catch { + // Read results remain sanitized when a legacy database is read-only. + } + } + return safe; +} export function createProject(name: string, path: string): Project { const db = getDb(); const id = crypto.randomUUID(); const now = new Date().toISOString(); + const safeName = sanitizeTextForBoundary(name, 256); + const safePath = sanitizeLocationForOutput(path); const stmt = db.prepare( `INSERT INTO projects (id, name, path, created_at, updated_at) VALUES (?, ?, ?, ?, ?)` ); - stmt.run(id, name, path, now, now); + stmt.run(id, safeName, safePath, now, now); - return { id, name, path, created_at: now, updated_at: now }; + return { id, name: safeName, path: safePath, created_at: now, updated_at: now }; } export function getProject(id: string): Project | null { const db = getDb(); const stmt = db.prepare(`SELECT * FROM projects WHERE id = ?`); const row = stmt.get(id) as Project | undefined; - return row ?? null; + return row ? rowToProject(row) : null; } export function getProjectByPath(path: string): Project | null { const db = getDb(); - const stmt = db.prepare(`SELECT * FROM projects WHERE path = ?`); - const row = stmt.get(path) as Project | undefined; - return row ?? null; + const safePath = sanitizeLocationForOutput(path); + const stmt = db.prepare(`SELECT * FROM projects WHERE path = ? OR path = ? LIMIT 1`); + const row = stmt.get(safePath, path) as Project | undefined; + return row ? rowToProject(row) : null; } export function listProjects(): Project[] { const db = getDb(); const stmt = db.prepare(`SELECT * FROM projects ORDER BY created_at DESC`); - return stmt.all() as Project[]; + return (stmt.all() as Project[]).map(rowToProject); } export function deleteProject(id: string): void { diff --git a/src/lib/finding-safety.test.ts b/src/lib/finding-safety.test.ts index e7b56f6..52c64e6 100644 --- a/src/lib/finding-safety.test.ts +++ b/src/lib/finding-safety.test.ts @@ -4,6 +4,7 @@ import { isCredentialFinding, sanitizeFindingForOutput, sanitizeFindingForPersistence, + sanitizeTextForBoundary, } from "./finding-safety.js"; function finding(overrides: Partial = {}): FindingInput { @@ -56,4 +57,29 @@ describe("finding safety", () => { expect(output.message).not.toContain("\u0000"); expect(output.code_snippet).toBe("[REDACTED]"); }); + + test("redacts credential values in non-credential context and metadata", () => { + const syntheticSecret = "sk_test_" + "SYNTHETICONLY0123456789"; + const input = finding({ + rule_id: `unsafe-${syntheticSecret}`, + file: `src/${syntheticSecret}/app.ts`, + message: `Unsafe code next to api_key=${syntheticSecret}`, + }); + const persisted = sanitizeFindingForPersistence(input); + const output = sanitizeFindingForOutput(input); + + for (const serialized of [JSON.stringify(persisted), JSON.stringify(output)]) { + expect(serialized).not.toContain(syntheticSecret); + expect(serialized).toContain("REDACTED"); + } + expect(persisted.rule_id).toMatch(/^\[REDACTED-RULE:[a-f0-9]{12}\]$/); + expect(persisted.file).toMatch(/^\[REDACTED-LOCATION:[a-f0-9]{12}\]$/); + }); + + test("sanitizes arbitrary adjacent context independent of finding classification", () => { + const syntheticSecret = "ghp_" + "SYNTHETICONLYABCDEFGHIJKLMNOPQRSTUVWXYZ12"; + const sanitized = sanitizeTextForBoundary(`ordinary config issue; adjacent=${syntheticSecret}`, 12_000); + expect(sanitized).not.toContain(syntheticSecret); + expect(sanitized).toContain("[REDACTED]"); + }); }); diff --git a/src/lib/finding-safety.ts b/src/lib/finding-safety.ts index 6963cac..e30240e 100644 --- a/src/lib/finding-safety.ts +++ b/src/lib/finding-safety.ts @@ -1,3 +1,4 @@ +import { createHash } from "crypto"; import { ScannerType, type Finding, type FindingInput } from "../types/index.js"; export const REDACTED_FINDING_TEXT = "[REDACTED]"; @@ -8,15 +9,102 @@ const MAX_RULE_ID_LENGTH = 128; type FindingLike = FindingInput | Finding; +// Deliberately conservative credential-value patterns. These run at trust +// boundaries in addition to the scanner's richer detection rules, so adjacent +// secrets cannot ride along with an otherwise non-credential finding. +const CREDENTIAL_PATTERNS: RegExp[] = [ + /-----BEGIN (?:[A-Z0-9 ]+ )?PRIVATE KEY-----[\s\S]*?-----END (?:[A-Z0-9 ]+ )?PRIVATE KEY-----/gi, + /\bgh[opusr]_[A-Za-z0-9]{20,}\b/g, + /\bgithub_pat_[A-Za-z0-9_]{20,}\b/g, + /\bAKIA[0-9A-Z]{16}\b/g, + /\bASIA[0-9A-Z]{16}\b/g, + /(?:aws_secret_access_key|aws_secret_key|secret_access_key)\s*[=:]\s*["']?[A-Za-z0-9/+=]{40}["']?/gi, + /\bxox[baprs]-[A-Za-z0-9-]{10,}\b/g, + /\bsk_(?:live|test)_[A-Za-z0-9]{12,}\b/gi, + /\bpk_live_[A-Za-z0-9]{24,}\b/gi, + /\bsk-(?:live|test|proj)-[A-Za-z0-9_-]{12,}\b/gi, + /\beyJ[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\b/g, + /\bBearer\s+[A-Za-z0-9._~+\/-]{12,}={0,2}\b/gi, + /\b(?:api[_-]?key|access[_-]?key|secret(?:[_-]?key)?|token|password|passwd|passphrase|credential)\s*[:=]\s*["']?[^\s"'`,;]{8,}["']?/gi, + /\b(?:https?|ssh):\/\/[^\s/:@]+:[^\s/@]+@[^\s]+/gi, + /\b(?:postgres(?:ql)?|mysql|mongodb(?:\+srv)?):\/\/[^\s"']+/gi, +]; + function boundedSingleLine(value: string, maxLength: number): string { const normalized = value.replace(/[\u0000-\u001f\u007f]/g, " ").replace(/\s+/g, " ").trim(); if (normalized.length <= maxLength) return normalized; return `${normalized.slice(0, Math.max(0, maxLength - 1))}…`; } +function stableRedaction(value: string, kind: string): string { + const correlation = createHash("sha256").update(value).digest("hex").slice(0, 12); + return `[REDACTED-${kind}:${correlation}]`; +} + +export function containsCredentialLikeText(value: string | null | undefined): boolean { + if (!value) return false; + if (CREDENTIAL_PATTERNS.some((pattern) => { + pattern.lastIndex = 0; + return pattern.test(value); + })) return true; + + // Refuse high-entropy opaque tokens even when they do not match a named + // provider format. This mirrors the scanner's last-resort secret heuristic. + for (const candidate of value.match(/[A-Za-z0-9+/=_-]{20,}/g) ?? []) { + const frequencies = new Map(); + for (const character of candidate) { + frequencies.set(character, (frequencies.get(character) ?? 0) + 1); + } + let entropy = 0; + for (const count of frequencies.values()) { + const probability = count / candidate.length; + entropy -= probability * Math.log2(probability); + } + if (entropy > 5) return true; + } + return false; +} + +/** Redact credential values from arbitrary untrusted text before a boundary. */ +export function sanitizeTextForBoundary( + value: string, + maxLength = MAX_MESSAGE_LENGTH, +): string { + if (containsCredentialLikeText(value)) { + return `${REDACTED_FINDING_TEXT} ${stableRedaction(value, "TEXT")}`; + } + let sanitized = value; + for (const pattern of CREDENTIAL_PATTERNS) { + pattern.lastIndex = 0; + sanitized = sanitized.replace(pattern, REDACTED_FINDING_TEXT); + } + return boundedSingleLine(sanitized, maxLength); +} + +/** Recursively sanitize JSON-compatible data before persistence or output. */ +export function sanitizeValueForBoundary(value: T): T { + if (typeof value === "string") return sanitizeTextForBoundary(value, 12_000) as T; + if (Array.isArray(value)) return value.map(sanitizeValueForBoundary) as T; + if (value !== null && typeof value === "object") { + const result: Record = {}; + for (const [key, entry] of Object.entries(value as Record)) { + result[sanitizeTextForBoundary(key, 256)] = sanitizeValueForBoundary(entry); + } + return result as T; + } + return value; +} + +export function sanitizeLocationForOutput(value: string): string { + return containsCredentialLikeText(value) + ? stableRedaction(value, "LOCATION") + : boundedSingleLine(value, MAX_LOCATION_LENGTH); +} + function safeRuleId(value: string): string { + if (containsCredentialLikeText(value)) return stableRedaction(value, "RULE"); const normalized = value.replace(/[^A-Za-z0-9._-]/g, "-"); - return boundedSingleLine(normalized || "credential", MAX_RULE_ID_LENGTH); + return boundedSingleLine(normalized || "finding", MAX_RULE_ID_LENGTH); } export function isCredentialFinding(finding: Pick): boolean { @@ -29,43 +117,41 @@ export function isCredentialFinding(finding: Pick(finding: T): T { - if (!isCredentialFinding(finding)) return finding; - - return { - ...finding, - file: boundedSingleLine(finding.file, MAX_LOCATION_LENGTH), - message: `Potential credential exposure detected (${safeRuleId(finding.rule_id)})`, - ...(finding.code_snippet != null ? { code_snippet: REDACTED_FINDING_TEXT } : {}), - } as T; -} - -export function sanitizeFindingForOutput(finding: T): T { +function sanitizeFinding(finding: T): T { const sensitive = isCredentialFinding(finding); + const ruleId = safeRuleId(finding.rule_id); const result = { ...finding, - file: boundedSingleLine(finding.file, MAX_LOCATION_LENGTH), + rule_id: ruleId, + file: sanitizeLocationForOutput(finding.file), message: sensitive - ? `Potential credential exposure detected (${safeRuleId(finding.rule_id)})` - : boundedSingleLine(finding.message, MAX_MESSAGE_LENGTH), + ? `Potential credential exposure detected (${ruleId})` + : sanitizeTextForBoundary(finding.message, MAX_MESSAGE_LENGTH), ...(finding.code_snippet != null ? { code_snippet: REDACTED_FINDING_TEXT } : {}), } as T; if ("llm_explanation" in result && result.llm_explanation != null) { result.llm_explanation = sensitive ? REDACTED_FINDING_TEXT - : boundedSingleLine(result.llm_explanation, MAX_MESSAGE_LENGTH); + : sanitizeTextForBoundary(result.llm_explanation, MAX_MESSAGE_LENGTH); } if ("llm_fix" in result && result.llm_fix != null) { result.llm_fix = sensitive ? REDACTED_FINDING_TEXT - : boundedSingleLine(result.llm_fix, MAX_MESSAGE_LENGTH); + : sanitizeTextForBoundary(result.llm_fix, MAX_MESSAGE_LENGTH); } if ("suppressed_reason" in result && result.suppressed_reason != null) { result.suppressed_reason = sensitive ? REDACTED_FINDING_TEXT - : boundedSingleLine(result.suppressed_reason, MAX_MESSAGE_LENGTH); + : sanitizeTextForBoundary(result.suppressed_reason, MAX_MESSAGE_LENGTH); } - return result; } + +export function sanitizeFindingForPersistence(finding: T): T { + return sanitizeFinding(finding); +} + +export function sanitizeFindingForOutput(finding: T): T { + return sanitizeFinding(finding); +} diff --git a/src/lib/secret-exposure.test.ts b/src/lib/secret-exposure.test.ts index 893966d..86d3089 100644 --- a/src/lib/secret-exposure.test.ts +++ b/src/lib/secret-exposure.test.ts @@ -1,6 +1,6 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { execFileSync } from "child_process"; -import { mkdtempSync, rmSync, writeFileSync } from "fs"; +import { mkdtempSync, rmSync, symlinkSync, writeFileSync } from "fs"; import { tmpdir } from "os"; import { join } from "path"; import { @@ -72,6 +72,21 @@ describe("secret exposure", () => { expect(JSON.stringify(result.findings)).not.toContain(githubToken); }); + test("scans a regular-file target instead of reporting a clean directory result", async () => { + const syntheticSecret = "ghp_" + "SYNTHETICONLYABCDEFGHIJKLMNOPQRSTUVWXYZ12"; + const file = join(tempDir, syntheticSecret); + writeFileSync(file, `TOKEN=${syntheticSecret}\n`, "utf-8"); + const result = await scanSecretExposure({ path: file }); + expect(result.findings.length).toBeGreaterThan(0); + expect(JSON.stringify(result)).not.toContain(syntheticSecret); + }); + + test("fails closed on stat/traversal errors", async () => { + const loop = join(tempDir, "loop"); + symlinkSync(loop, loop); + await expect(scanSecretExposure({ path: loop })).rejects.toThrow("Symbolic links"); + }); + test("scanRunningProcesses inspects process environment snapshots", () => { const githubToken = "ghp_" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890ab"; const runner: CommandRunner = (_command, _args) => `123 USER=me GITHUB_TOKEN=${githubToken} node server.js\n`; diff --git a/src/lib/secret-exposure.ts b/src/lib/secret-exposure.ts index 0fb7de7..68b950b 100644 --- a/src/lib/secret-exposure.ts +++ b/src/lib/secret-exposure.ts @@ -1,10 +1,9 @@ import { execFileSync } from "child_process"; -import { existsSync } from "fs"; import { resolve } from "path"; import { gitHistoryScanner } from "../scanners/git-history.js"; import { scanFile, secretsScanner } from "../scanners/secrets.js"; import { SEVERITY_ORDER, Severity, type FindingInput } from "../types/index.js"; -import { sanitizeFindingForOutput } from "./finding-safety.js"; +import { sanitizeFindingForOutput, sanitizeLocationForOutput } from "./finding-safety.js"; type RunnerOptions = { cwd?: string; @@ -249,9 +248,6 @@ export async function scanSecretExposure( runner: CommandRunner = defaultRunner, ): Promise { const scanPath = resolve(options.path); - if (!existsSync(scanPath)) { - throw new Error(`Path does not exist: ${scanPath}`); - } const findings: FindingInput[] = []; findings.push( @@ -274,7 +270,7 @@ export async function scanSecretExposure( const deduped = dedupeFindings(findings).map(sanitizeFindingForOutput); return { - path: scanPath, + path: sanitizeLocationForOutput(scanPath), findings: deduped, summary: summarizeSecretExposure(deduped), }; diff --git a/src/llm/analyzer.ts b/src/llm/analyzer.ts index 1dac056..0f2bb5c 100644 --- a/src/llm/analyzer.ts +++ b/src/llm/analyzer.ts @@ -1,5 +1,5 @@ import type { Finding } from "../types/index.js"; -import { isCredentialFinding } from "../lib/finding-safety.js"; +import { isCredentialFinding, sanitizeFindingForOutput, sanitizeTextForBoundary } from "../lib/finding-safety.js"; import { chat } from "./client.js"; import { ANALYZER_PROMPT } from "./prompts.js"; @@ -17,18 +17,20 @@ export async function analyzeFinding( confidence: number; } | null> { if (isCredentialFinding(finding)) return null; + const safeFinding = sanitizeFindingForOutput(finding); + const safeContext = sanitizeTextForBoundary(codeContext, 12_000); const cacheKey = finding.fingerprint; if (cache.has(cacheKey)) return cache.get(cacheKey)!; const userMessage = `Finding: -- Rule: ${finding.rule_id} -- Severity: ${finding.severity} -- File: ${finding.file}:${finding.line} -- Message: ${finding.message} +- Rule: ${safeFinding.rule_id} +- Severity: ${safeFinding.severity} +- File: ${safeFinding.file}:${safeFinding.line} +- Message: ${safeFinding.message} Code context: \`\`\` -${codeContext} +${safeContext} \`\`\``; const response = await chat([ diff --git a/src/llm/client.ts b/src/llm/client.ts index e6643ef..df8027a 100644 --- a/src/llm/client.ts +++ b/src/llm/client.ts @@ -1,15 +1,22 @@ import OpenAI from "openai"; +import { sanitizeTextForBoundary } from "../lib/finding-safety.js"; let _client: OpenAI | null = null; +let _clientApiKey: string | null = null; export function getLLMClient(): OpenAI | null { - if (_client) return _client; const apiKey = process.env.CEREBRAS_API_KEY; - if (!apiKey) return null; + if (!apiKey) { + _client = null; + _clientApiKey = null; + return null; + } + if (_client && _clientApiKey === apiKey) return _client; _client = new OpenAI({ baseURL: "https://api.cerebras.ai/v1", apiKey, }); + _clientApiKey = apiKey; return _client; } @@ -17,6 +24,14 @@ export function getModel(): string { return process.env.CEREBRAS_MODEL || "llama-4-scout-17b-16e-instruct"; } +export function sanitizeMessagesForProvider( + messages: OpenAI.Chat.ChatCompletionMessageParam[], +): OpenAI.Chat.ChatCompletionMessageParam[] { + return JSON.parse(JSON.stringify(messages), (_key, value) => + typeof value === "string" ? sanitizeTextForBoundary(value, 12_000) : value, + ) as OpenAI.Chat.ChatCompletionMessageParam[]; +} + export async function chat( messages: OpenAI.Chat.ChatCompletionMessageParam[], options?: { temperature?: number; max_tokens?: number }, @@ -29,11 +44,12 @@ export async function chat( try { const response = await client.chat.completions.create({ model: getModel(), - messages, + messages: sanitizeMessagesForProvider(messages), temperature: options?.temperature ?? 0.2, max_tokens: options?.max_tokens ?? 2048, }); - return response.choices[0]?.message?.content ?? null; + const content = response.choices[0]?.message?.content; + return content == null ? null : sanitizeTextForBoundary(content, 12_000); } catch (error) { if (attempt === maxAttempts) return null; await new Promise((resolve) => diff --git a/src/llm/credential-boundary.test.ts b/src/llm/credential-boundary.test.ts index 4e32789..9d031b5 100644 --- a/src/llm/credential-boundary.test.ts +++ b/src/llm/credential-boundary.test.ts @@ -4,11 +4,15 @@ import { analyzeFinding } from "./analyzer.js"; import { explainFinding } from "./explainer.js"; import { suggestFix } from "./fixer.js"; import { triageFinding } from "./triager.js"; +import { sanitizeMessagesForProvider } from "./client.js"; const originalFetch = globalThis.fetch; +const originalApiKey = process.env.CEREBRAS_API_KEY; afterEach(() => { globalThis.fetch = originalFetch; + if (originalApiKey === undefined) delete process.env.CEREBRAS_API_KEY; + else process.env.CEREBRAS_API_KEY = originalApiKey; }); describe("credential finding LLM boundary", () => { @@ -48,4 +52,13 @@ describe("credential finding LLM boundary", () => { expect(await triageFinding(credentialFinding, context)).toBeNull(); expect(fetchCalls).toBe(0); }); + + test("redacts adjacent credentials in the final provider payload", () => { + const syntheticSecret = "ghp_" + "SYNTHETICONLYABCDEFGHIJKLMNOPQRSTUVWXYZ12"; + const payload = JSON.stringify(sanitizeMessagesForProvider([ + { role: "user", content: `ordinary config issue\nGITHUB_TOKEN=${syntheticSecret}` }, + ])); + expect(payload).not.toContain(syntheticSecret); + expect(payload).toContain("[REDACTED]"); + }); }); diff --git a/src/llm/explainer.ts b/src/llm/explainer.ts index 0e5f5ad..c0d3db6 100644 --- a/src/llm/explainer.ts +++ b/src/llm/explainer.ts @@ -1,5 +1,5 @@ import type { Finding } from "../types/index.js"; -import { isCredentialFinding } from "../lib/finding-safety.js"; +import { isCredentialFinding, sanitizeFindingForOutput, sanitizeTextForBoundary } from "../lib/finding-safety.js"; import { chat } from "./client.js"; import { EXPLAINER_PROMPT } from "./prompts.js"; @@ -10,18 +10,20 @@ export async function explainFinding( codeContext: string, ): Promise { if (isCredentialFinding(finding)) return null; + const safeFinding = sanitizeFindingForOutput(finding); + const safeContext = sanitizeTextForBoundary(codeContext, 12_000); const cacheKey = finding.fingerprint; if (cache.has(cacheKey)) return cache.get(cacheKey)!; const userMessage = `Vulnerability: -- Rule: ${finding.rule_id} -- Severity: ${finding.severity} -- File: ${finding.file}:${finding.line} -- Message: ${finding.message} +- Rule: ${safeFinding.rule_id} +- Severity: ${safeFinding.severity} +- File: ${safeFinding.file}:${safeFinding.line} +- Message: ${safeFinding.message} Code context: \`\`\` -${codeContext} +${safeContext} \`\`\``; const response = await chat([ @@ -31,6 +33,7 @@ ${codeContext} if (!response) return null; - cache.set(cacheKey, response); - return response; + const safeResponse = sanitizeTextForBoundary(response); + cache.set(cacheKey, safeResponse); + return safeResponse; } diff --git a/src/llm/fixer.ts b/src/llm/fixer.ts index 1b0e0dc..9314fe9 100644 --- a/src/llm/fixer.ts +++ b/src/llm/fixer.ts @@ -1,5 +1,5 @@ import type { Finding } from "../types/index.js"; -import { isCredentialFinding } from "../lib/finding-safety.js"; +import { isCredentialFinding, sanitizeFindingForOutput, sanitizeTextForBoundary } from "../lib/finding-safety.js"; import { chat } from "./client.js"; import { FIXER_PROMPT } from "./prompts.js"; @@ -10,18 +10,20 @@ export async function suggestFix( codeContext: string, ): Promise { if (isCredentialFinding(finding)) return null; + const safeFinding = sanitizeFindingForOutput(finding); + const safeContext = sanitizeTextForBoundary(codeContext, 12_000); const cacheKey = finding.fingerprint; if (cache.has(cacheKey)) return cache.get(cacheKey)!; const userMessage = `Vulnerability to fix: -- Rule: ${finding.rule_id} -- Severity: ${finding.severity} -- File: ${finding.file}:${finding.line} -- Message: ${finding.message} +- Rule: ${safeFinding.rule_id} +- Severity: ${safeFinding.severity} +- File: ${safeFinding.file}:${safeFinding.line} +- Message: ${safeFinding.message} Current code: \`\`\` -${codeContext} +${safeContext} \`\`\``; const response = await chat([ @@ -31,6 +33,7 @@ ${codeContext} if (!response) return null; - cache.set(cacheKey, response); - return response; + const safeResponse = sanitizeTextForBoundary(response); + cache.set(cacheKey, safeResponse); + return safeResponse; } diff --git a/src/llm/triager.ts b/src/llm/triager.ts index 6431be7..55622cb 100644 --- a/src/llm/triager.ts +++ b/src/llm/triager.ts @@ -1,5 +1,5 @@ import { type Finding, Severity } from "../types/index.js"; -import { isCredentialFinding } from "../lib/finding-safety.js"; +import { isCredentialFinding, sanitizeFindingForOutput, sanitizeTextForBoundary } from "../lib/finding-safety.js"; import { chat } from "./client.js"; import { TRIAGER_PROMPT } from "./prompts.js"; @@ -18,18 +18,20 @@ export async function triageFinding( codeContext: string, ): Promise<{ severity: Severity; reasoning: string } | null> { if (isCredentialFinding(finding)) return null; + const safeFinding = sanitizeFindingForOutput(finding); + const safeContext = sanitizeTextForBoundary(codeContext, 12_000); const cacheKey = finding.fingerprint; if (cache.has(cacheKey)) return cache.get(cacheKey)!; const userMessage = `Finding to triage: -- Rule: ${finding.rule_id} -- Current severity: ${finding.severity} -- File: ${finding.file}:${finding.line} -- Message: ${finding.message} +- Rule: ${safeFinding.rule_id} +- Current severity: ${safeFinding.severity} +- File: ${safeFinding.file}:${safeFinding.line} +- Message: ${safeFinding.message} Code context: \`\`\` -${codeContext} +${safeContext} \`\`\``; const response = await chat([ @@ -50,7 +52,7 @@ ${codeContext} SEVERITY_MAP[parsed.severity?.toLowerCase()] ?? Severity.Medium; const result = { severity, - reasoning: parsed.reasoning || "No reasoning provided", + reasoning: sanitizeTextForBoundary(parsed.reasoning || "No reasoning provided"), }; cache.set(cacheKey, result); return result; diff --git a/src/mcp/tools/output-safety.test.ts b/src/mcp/tools/output-safety.test.ts index a7924aa..ab1f05d 100644 --- a/src/mcp/tools/output-safety.test.ts +++ b/src/mcp/tools/output-safety.test.ts @@ -1,4 +1,7 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, rmSync, writeFileSync } from "fs"; +import { tmpdir } from "os"; +import { join } from "path"; import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { createFinding } from "../../db/findings.js"; import { createProject } from "../../db/projects.js"; @@ -97,4 +100,19 @@ describe("MCP credential output safety", () => { expect(output).toContain("Details were withheld"); expect(output).not.toContain("/definitely/missing/shield-output-safety-path"); }); + + test("scan_repo defaults to file-only scanner types", async () => { + const dir = mkdtempSync(join(tmpdir(), "shield-mcp-files-only-")); + writeFileSync(join(dir, "index.ts"), "export const safe = true;\n", "utf-8"); + try { + const tools = captureTools((server) => registerScanTools(server, jsonResult, () => "")); + const result = await tools.get("scan_repo")?.({ path: dir }); + const envelope = result as { content: Array<{ text: string }> }; + const payload = JSON.parse(envelope.content[0].text); + expect(payload.scan.scanner_types).not.toContain(ScannerType.GitHistory); + expect(payload.scan.scanner_types).toContain(ScannerType.Code); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); }); diff --git a/src/mcp/tools/scan.ts b/src/mcp/tools/scan.ts index bc93351..bb58e17 100644 --- a/src/mcp/tools/scan.ts +++ b/src/mcp/tools/scan.ts @@ -12,11 +12,11 @@ import { updateFinding, getSecurityScore, } from "../../db/index.js"; -import { runAllScanners, runScanner } from "../../scanners/index.js"; +import { resolvePublicScannerTypes, runAllScanners, runScanner } from "../../scanners/index.js"; import { analyzeFinding as llmAnalyze, isLLMAvailable } from "../../llm/index.js"; import { ScannerType, ScanStatus, Severity } from "../../types/index.js"; import type { FindingInput } from "../../types/index.js"; -import { sanitizeFindingForOutput } from "../../lib/finding-safety.js"; +import { sanitizeFindingForOutput, sanitizeLocationForOutput } from "../../lib/finding-safety.js"; import { scanSecretExposure, filterSecretExposureBySeverity, @@ -36,10 +36,12 @@ export function registerScanTools( "Run a full security scan on a repository path", { path: z.string().describe("Path to the repository to scan"), - scanners: z.array(z.string()).optional().describe("Scanner types to run (defaults to all)"), + scanners: z.array(z.string()).optional().describe("Scanner types to run (defaults to file-only scanners; git-history also requires include_git_history=true)"), + include_git_history: z.boolean().optional().describe("Explicitly opt in to git history (default false)"), + include_system: z.boolean().optional().describe("Explicitly opt in to host/system IOC locations outside path (default false)"), llm_analyze: z.boolean().optional().describe("Whether to run LLM analysis on findings"), }, - async ({ path, scanners, llm_analyze }) => { + async ({ path, scanners, include_git_history, include_system, llm_analyze }) => { try { const scanPath = resolve(path); @@ -49,23 +51,23 @@ export function registerScanTools( project = createProject(name, scanPath); } - const scannerTypes: ScannerType[] = scanners - ? scanners.filter((s) => Object.values(ScannerType).includes(s as ScannerType)) as ScannerType[] - : Object.values(ScannerType); + const scannerTypes = resolvePublicScannerTypes(scanners, include_git_history === true); + const runOptions = { + include_git_history: include_git_history === true, + include_system: include_system === true, + }; const scan = createScan(project.id, scannerTypes); updateScanStatus(scan.id, ScanStatus.Running); let findingInputs: FindingInput[]; if (scanners && scanners.length > 0) { - const results = await Promise.allSettled( - scannerTypes.map((t) => runScanner(t, scanPath)), + const results = await Promise.all( + scannerTypes.map((t) => runScanner(t, scanPath, runOptions)), ); - findingInputs = results - .filter((r) => r.status === "fulfilled") - .flatMap((r) => (r as PromiseFulfilledResult).value); + findingInputs = results.flat(); } else { - findingInputs = await runAllScanners(scanPath); + findingInputs = await runAllScanners(scanPath, runOptions); } const findings = findingInputs.map((input) => createFinding(scan.id, input)); @@ -125,7 +127,7 @@ export function registerScanTools( (f) => f.file === absPath || f.file === filePath || f.file.endsWith(filePath), ); return jsonResult({ - file: absPath, + file: sanitizeLocationForOutput(absPath), findings: fileFindings.map(sanitizeFindingForOutput), count: fileFindings.length, }); diff --git a/src/reporters/json.test.ts b/src/reporters/json.test.ts index 83f2009..a83aead 100644 --- a/src/reporters/json.test.ts +++ b/src/reporters/json.test.ts @@ -140,4 +140,17 @@ describe("JSON reporter", () => { expect(f.llm_exploitability).toBe(0.9); expect(output).not.toContain(syntheticSecret); }); + + test("redacts credential-bearing rule and path metadata for non-secret findings", () => { + const syntheticSecret = "sk_test_" + "SYNTHETICONLY0123456789"; + const output = reportFindings([makeFinding({ + scanner_type: ScannerType.Code, + rule_id: `rule-${syntheticSecret}`, + file: `src/${syntheticSecret}/app.ts`, + message: "Unsafe code path", + })]); + expect(output).not.toContain(syntheticSecret); + expect(output).toContain("REDACTED-RULE"); + expect(output).toContain("REDACTED-LOCATION"); + }); }); diff --git a/src/scanners/index.ts b/src/scanners/index.ts index 7ebeaef..21775d0 100644 --- a/src/scanners/index.ts +++ b/src/scanners/index.ts @@ -3,6 +3,7 @@ import { type FindingInput, type ScannerRunOptions, ScannerType, + DEFAULT_FILE_SCANNERS, } from "../types/index.js"; import { secretsScanner } from "./secrets.js"; import { dependenciesScanner } from "./dependencies.js"; @@ -30,24 +31,40 @@ export function listScanners(): Scanner[] { return Array.from(scannerRegistry.values()); } +/** Resolve an API/MCP scanner request. Git history is filtered unless the + * request carries the dedicated per-invocation opt-in. Unknown scanners fail. */ +export function resolvePublicScannerTypes( + requested?: readonly string[], + includeGitHistory = false, +): ScannerType[] { + const source = !requested || requested.length === 0 ? DEFAULT_FILE_SCANNERS : requested; + const resolved = source.map((value) => { + if (!Object.values(ScannerType).includes(value as ScannerType) || !getScanner(value as ScannerType)) { + throw new Error("Unknown or unavailable scanner requested"); + } + return value as ScannerType; + }).filter((value) => value !== ScannerType.GitHistory || includeGitHistory); + if (includeGitHistory) resolved.push(ScannerType.GitHistory); + return [...new Set(resolved)]; +} + export async function runAllScanners( scanPath: string, options?: ScannerRunOptions, ): Promise { - const findings: FindingInput[] = []; - const scanners = listScanners(); - - const results = await Promise.allSettled( - scanners.map((scanner) => scanner.scan(scanPath, options)), - ); - - for (const result of results) { - if (result.status === "fulfilled") { - findings.push(...result.value); - } - } - - return findings; + // This public convenience API is intentionally file-only. Sensitive source + // scanners (currently git history) remain available through runScanner, + // where naming the scanner is the explicit opt-in. + const scannerTypes = options?.include_git_history + ? [...DEFAULT_FILE_SCANNERS, ScannerType.GitHistory] + : DEFAULT_FILE_SCANNERS; + const scanners = scannerTypes.map((type) => { + const scanner = getScanner(type); + if (!scanner) throw new Error("Default scanner unavailable"); + return scanner; + }); + const results = await Promise.all(scanners.map((scanner) => scanner.scan(scanPath, options))); + return results.flat(); } export async function runScanner( diff --git a/src/scanners/ioc.ts b/src/scanners/ioc.ts index 545ed74..4639225 100644 --- a/src/scanners/ioc.ts +++ b/src/scanners/ioc.ts @@ -554,11 +554,12 @@ export const iocScanner: Scanner = { // 2. Scan source code for C2 domains/IPs findings.push(...scanForC2Indicators(scanPath, ignorePatterns)); - // 3. Check for RAT artifacts on disk - findings.push(...checkRATArtifacts()); - - // 4. Check for malicious .pth files (Python) - findings.push(...checkPthFiles()); + // Host-wide locations and Python site discovery cross the requested tree + // boundary, so they require an explicit per-invocation opt-in. + if (options?.include_system === true) { + findings.push(...checkRATArtifacts()); + findings.push(...checkPthFiles()); + } // 5. Check postinstall scripts in node_modules findings.push(...checkPostinstallScripts(scanPath)); diff --git a/src/scanners/lockfile.ts b/src/scanners/lockfile.ts index 7a08062..0997488 100644 --- a/src/scanners/lockfile.ts +++ b/src/scanners/lockfile.ts @@ -424,7 +424,7 @@ export const lockfileScanner: Scanner = { "unpinned ranges on critical packages, lockfile changes during attack windows, " + "install/revert patterns, and missing lockfiles", - async scan(scanPath: string, _options?: ScannerRunOptions): Promise { + async scan(scanPath: string, options?: ScannerRunOptions): Promise { ensureSeeded(); const findings: FindingInput[] = []; @@ -459,8 +459,10 @@ export const lockfileScanner: Scanner = { } catch {} } - // 4. Git history analysis - findings.push(...checkLockfileGitHistory(scanPath)); + // 4. Git history analysis is a separate sensitive source. + if (options?.include_git_history === true) { + findings.push(...checkLockfileGitHistory(scanPath)); + } return findings; }, diff --git a/src/scanners/secrets.ts b/src/scanners/secrets.ts index b95ebd3..d783005 100644 --- a/src/scanners/secrets.ts +++ b/src/scanners/secrets.ts @@ -39,7 +39,7 @@ export function walkDirectory( try { entries = fs.readdirSync(currentDir, { withFileTypes: true }); } catch { - return; + throw new Error("Unable to traverse the requested scan target"); } for (const entry of entries) { @@ -61,6 +61,8 @@ export function walkDirectory( if (isBinaryFile(fullPath)) continue; if (fileFilter && !fileFilter(fullPath)) continue; results.push(fullPath); + } else if (entry.isSymbolicLink()) { + throw new Error("Symbolic links are not included in a verified file-only scan"); } } } @@ -811,16 +813,29 @@ export const secretsScanner: Scanner = { async scan(scanPath: string, options?: ScannerRunOptions): Promise { const ignorePatterns = options?.ignore_patterns ?? DEFAULT_CONFIG.ignore_patterns; - const files = walkDirectory(scanPath, ignorePatterns); + let stat: fs.Stats; + try { + stat = fs.lstatSync(scanPath); + } catch { + throw new Error("Unable to stat the requested scan target"); + } + if (stat.isSymbolicLink()) { + throw new Error("Symbolic links are not included in a verified file-only scan"); + } + const files = stat.isFile() + ? isBinaryFile(scanPath) ? [] : [scanPath] + : stat.isDirectory() + ? walkDirectory(scanPath, ignorePatterns) + : (() => { throw new Error("Requested scan target is not a regular file or directory"); })(); const findings: FindingInput[] = []; for (const file of files) { try { const content = fs.readFileSync(file, "utf-8"); - const relativePath = path.relative(scanPath, file); + const relativePath = stat.isFile() ? path.basename(file) : path.relative(scanPath, file); findings.push(...scanFile(relativePath, content)); } catch { - // Skip unreadable files + throw new Error("Unable to read every requested scan file"); } } diff --git a/src/scanners/source-boundary.test.ts b/src/scanners/source-boundary.test.ts new file mode 100644 index 0000000..67051cf --- /dev/null +++ b/src/scanners/source-boundary.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, test } from "bun:test"; +import { DEFAULT_CONFIG, DEFAULT_FILE_SCANNERS, ScannerType } from "../types/index.js"; +import { getScanner, resolvePublicScannerTypes } from "./index.js"; + +describe("public scanner source boundary", () => { + test("library and config defaults contain only registered file scanners", () => { + expect(DEFAULT_CONFIG.enabled_scanners).toEqual(DEFAULT_FILE_SCANNERS); + expect(DEFAULT_FILE_SCANNERS).not.toContain(ScannerType.GitHistory); + expect(DEFAULT_FILE_SCANNERS.every((type) => getScanner(type) !== undefined)).toBe(true); + }); + + test("API and MCP request resolution is file-only by default", () => { + expect(resolvePublicScannerTypes()).toEqual(DEFAULT_FILE_SCANNERS); + expect(resolvePublicScannerTypes([])).toEqual(DEFAULT_FILE_SCANNERS); + expect(resolvePublicScannerTypes()).not.toContain(ScannerType.GitHistory); + }); + + test("git history requires the dedicated opt-in and unknown scanners fail closed", () => { + expect(resolvePublicScannerTypes([ScannerType.Code, ScannerType.GitHistory])).toEqual([ + ScannerType.Code, + ]); + expect(resolvePublicScannerTypes([ScannerType.Code, ScannerType.GitHistory], true)).toEqual([ + ScannerType.Code, + ScannerType.GitHistory, + ]); + expect(() => resolvePublicScannerTypes(["unknown"])).toThrow("Unknown or unavailable"); + expect(() => resolvePublicScannerTypes([ScannerType.CiCd])).toThrow("Unknown or unavailable"); + }); +}); diff --git a/src/server/scan-boundary.test.ts b/src/server/scan-boundary.test.ts new file mode 100644 index 0000000..fea9baf --- /dev/null +++ b/src/server/scan-boundary.test.ts @@ -0,0 +1,74 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { spawn, type ChildProcess } from "child_process"; +import { mkdtempSync, rmSync } from "fs"; +import { createServer } from "net"; +import { tmpdir } from "os"; +import { join } from "path"; +import { ScannerType } from "../types/index.js"; + +async function availablePort(): Promise { + return await new Promise((resolve, reject) => { + const server = createServer(); + server.once("error", reject); + server.listen(0, "127.0.0.1", () => { + const address = server.address(); + if (!address || typeof address === "string") return reject(new Error("No TCP port allocated")); + server.close((error) => error ? reject(error) : resolve(address.port)); + }); + }); +} + +describe("REST scan source boundary", () => { + let tempDir: string; + let child: ChildProcess | null; + + beforeEach(() => { + tempDir = mkdtempSync(join(tmpdir(), "shield-rest-boundary-")); + child = null; + }); + + afterEach(() => { + child?.kill("SIGTERM"); + rmSync(tempDir, { recursive: true, force: true }); + }); + + test("POST /api/scans defaults to file-only scanner types", async () => { + const port = await availablePort(); + child = spawn("bun", ["run", "src/server/index.ts"], { + cwd: process.cwd(), + env: { + ...process.env, + PORT: String(port), + HOME: tempDir, + USERPROFILE: tempDir, + SECURITY_DB: join(tempDir, "shield.db"), + CEREBRAS_API_KEY: "", + }, + stdio: ["ignore", "pipe", "pipe"], + }); + + await new Promise((resolve, reject) => { + const timeout = setTimeout(() => reject(new Error("REST test server did not start")), 5_000); + child!.once("exit", (code) => { + clearTimeout(timeout); + reject(new Error(`REST test server exited early (${code})`)); + }); + child!.stdout!.on("data", (chunk) => { + if (String(chunk).includes("security dashboard")) { + clearTimeout(timeout); + resolve(); + } + }); + }); + + const response = await fetch(`http://127.0.0.1:${port}/api/scans`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ path: tempDir }), + }); + expect(response.status).toBe(202); + const scan = await response.json() as { scanner_types: ScannerType[] }; + expect(scan.scanner_types).not.toContain(ScannerType.GitHistory); + expect(scan.scanner_types).toContain(ScannerType.Code); + }); +}); diff --git a/src/server/serve.ts b/src/server/serve.ts index 33f2fea..de5701e 100644 --- a/src/server/serve.ts +++ b/src/server/serve.ts @@ -38,6 +38,7 @@ import { import { runAllScanners, runScanner, + resolvePublicScannerTypes, } from "../scanners/index.js"; import { explainFinding as llmExplain, @@ -130,11 +131,15 @@ export function startServer(port: number) { // POST /api/scans — trigger new scan app.post("/api/scans", async (req: Request, res: Response) => { try { - const { path: scanPath, scanners, llm_analyze } = req.body; + const { path: scanPath, scanners, include_git_history, include_system, llm_analyze } = req.body; if (!scanPath) { res.status(400).json({ error: "path is required" }); return; } + if (scanners !== undefined && !Array.isArray(scanners)) { + res.status(400).json({ error: "scanners must be an array" }); + return; + } const absPath = resolve(scanPath); @@ -146,9 +151,14 @@ export function startServer(port: number) { } // Determine scanner types - const scannerTypes: ScannerType[] = scanners - ? (scanners as string[]).filter((s: string) => Object.values(ScannerType).includes(s as ScannerType)) as ScannerType[] - : Object.values(ScannerType); + const scannerTypes = resolvePublicScannerTypes( + Array.isArray(scanners) ? scanners.map(String) : undefined, + include_git_history === true, + ); + const runOptions = { + include_git_history: include_git_history === true, + include_system: include_system === true, + }; // Create scan record const scan = createScan(project.id, scannerTypes); @@ -162,14 +172,12 @@ export function startServer(port: number) { try { let findingInputs: FindingInput[]; if (scanners && scanners.length > 0) { - const results = await Promise.allSettled( - scannerTypes.map((t) => runScanner(t, absPath)), + const results = await Promise.all( + scannerTypes.map((t) => runScanner(t, absPath, runOptions)), ); - findingInputs = results - .filter((r) => r.status === "fulfilled") - .flatMap((r) => (r as PromiseFulfilledResult).value); + findingInputs = results.flat(); } else { - findingInputs = await runAllScanners(absPath); + findingInputs = await runAllScanners(absPath, runOptions); } // Store findings @@ -201,12 +209,12 @@ export function startServer(port: number) { } })().catch(() => {}); } - } catch (error) { - updateScanStatus(scan.id, ScanStatus.Failed, undefined, String(error)); + } catch { + updateScanStatus(scan.id, ScanStatus.Failed, undefined, "Scanner execution failed; details withheld"); } })(); - } catch (error) { - res.status(500).json({ error: String(error) }); + } catch { + res.status(500).json({ error: "Repository scan failed; details withheld" }); } }); diff --git a/src/types/index.ts b/src/types/index.ts index cc6afb0..8601a99 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -150,6 +150,10 @@ export interface Scanner { export interface ScannerRunOptions { ignore_patterns?: string[]; rules?: Rule[]; + /** Explicit per-invocation opt-in to repository history. */ + include_git_history?: boolean; + /** Explicit per-invocation opt-in to host/system locations outside scanPath. */ + include_system?: boolean; } export interface FindingInput { @@ -175,8 +179,24 @@ export interface ConfigFile { llm_analyze: boolean; } +/** + * Scanners that only inspect the requested filesystem tree. Historical and + * live-machine sources must never be added here: callers must opt in to those + * sources explicitly for each invocation. + */ +export const DEFAULT_FILE_SCANNERS: ScannerType[] = [ + ScannerType.Secrets, + ScannerType.Dependencies, + ScannerType.Code, + ScannerType.Config, + ScannerType.AiSafety, + ScannerType.SupplyChain, + ScannerType.IOC, + ScannerType.Lockfile, +]; + export const DEFAULT_CONFIG: ConfigFile = { - enabled_scanners: Object.values(ScannerType), + enabled_scanners: [...DEFAULT_FILE_SCANNERS], severity_threshold: Severity.Info, output_format: ReportFormat.Terminal, ignore_patterns: ["node_modules", ".git", "dist", "build", "vendor", "__pycache__", "*.test.ts", "*.test.js", "*.test.tsx", "*.test.jsx", "*.spec.ts", "*.spec.js", "__tests__", "test/fixtures", "tests/fixtures"], From 264e400190ae80fbedd710a99e37e14cf4cb691d Mon Sep 17 00:00:00 2001 From: Andrei Hasna Date: Thu, 16 Jul 2026 00:40:50 +0300 Subject: [PATCH 3/8] fix(security): unify credential boundary recognition --- sdk/src/client.test.ts | 65 +++++++ src/lib/credential-invariant.test.ts | 223 ++++++++++++++++++++++++ src/lib/credential-recognition.ts | 252 +++++++++++++++++++++++++++ src/lib/finding-safety.ts | 55 +----- src/mcp/build-server.ts | 45 +++-- src/mcp/http.test.ts | 30 ++++ src/scanners/secrets.ts | 208 ++-------------------- src/server/scan-boundary.test.ts | 46 ++++- src/server/serve.ts | 11 +- 9 files changed, 677 insertions(+), 258 deletions(-) create mode 100644 src/lib/credential-invariant.test.ts create mode 100644 src/lib/credential-recognition.ts diff --git a/sdk/src/client.test.ts b/sdk/src/client.test.ts index dd1edaa..fd847f8 100644 --- a/sdk/src/client.test.ts +++ b/sdk/src/client.test.ts @@ -1,12 +1,35 @@ import { afterEach, describe, expect, test } from "bun:test"; +import { spawn, type ChildProcess } from "child_process"; +import { mkdirSync, mkdtempSync, rmSync } from "fs"; +import { createServer } from "net"; +import { tmpdir } from "os"; +import { join, resolve } from "path"; import { OpenSecurityClient } from "./client.js"; const originalFetch = globalThis.fetch; +let child: ChildProcess | undefined; +let tempDir: string | undefined; afterEach(() => { globalThis.fetch = originalFetch; + child?.kill("SIGTERM"); + child = undefined; + if (tempDir) rmSync(tempDir, { recursive: true, force: true }); + tempDir = undefined; }); +async function availablePort(): Promise { + return await new Promise((resolve, reject) => { + const server = createServer(); + server.once("error", reject); + server.listen(0, "127.0.0.1", () => { + const address = server.address(); + if (!address || typeof address === "string") return reject(new Error("No TCP port allocated")); + server.close((error) => error ? reject(error) : resolve(address.port)); + }); + }); +} + describe("OpenSecurityClient scan source boundary", () => { test("omits sensitive-source opt-ins by default and forwards explicit choices", async () => { const bodies: Array> = []; @@ -32,4 +55,46 @@ describe("OpenSecurityClient scan source boundary", () => { include_system: true, }); }); + + test("does not expose scanner-recognized values returned through the SDK", async () => { + globalThis.fetch = originalFetch; + tempDir = mkdtempSync(join(tmpdir(), "shield-sdk-boundary-")); + const synthetic = `gh${"r"}_${"A_".repeat(18)}`; + const projectDir = join(tempDir, synthetic); + mkdirSync(projectDir); + const port = await availablePort(); + child = spawn("bun", ["run", "src/server/index.ts"], { + cwd: resolve(import.meta.dir, "../.."), + env: { + ...process.env, + PORT: String(port), + HOME: tempDir, + USERPROFILE: tempDir, + SECURITY_DB: join(tempDir, "shield.db"), + CEREBRAS_API_KEY: "", + }, + stdio: ["ignore", "pipe", "pipe"], + }); + await new Promise((resolve, reject) => { + const timeout = setTimeout(() => reject(new Error("REST test server did not start")), 5_000); + child!.once("exit", (code) => { + clearTimeout(timeout); + reject(new Error(`REST test server exited early (${code})`)); + }); + child!.stdout!.on("data", (chunk) => { + if (String(chunk).includes("security dashboard")) { + clearTimeout(timeout); + resolve(); + } + }); + }); + + const client = new OpenSecurityClient(`http://127.0.0.1:${port}`); + const created = await client.createProject(`project-${synthetic}`, projectDir); + const listed = await client.listProjects(); + for (const output of [JSON.stringify(created), JSON.stringify(listed)]) { + expect(output).not.toContain(synthetic); + expect(output).toContain("REDACTED"); + } + }); }); diff --git a/src/lib/credential-invariant.test.ts b/src/lib/credential-invariant.test.ts new file mode 100644 index 0000000..06e5cfb --- /dev/null +++ b/src/lib/credential-invariant.test.ts @@ -0,0 +1,223 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { createFinding, getFinding, updateFinding } from "../db/findings.js"; +import { createProject } from "../db/projects.js"; +import { createRule } from "../db/rules.js"; +import { createScan } from "../db/scans.js"; +import { getCurrentTestDb, setupTestDb } from "../db/test-helpers.js"; +import { sanitizeMessagesForProvider } from "../llm/client.js"; +import { reportFindings as reportJson } from "../reporters/json.js"; +import { reportFindings as reportSarif } from "../reporters/sarif.js"; +import { reportFindings as reportTerminal } from "../reporters/terminal.js"; +import { scanFile } from "../scanners/secrets.js"; +import { ScannerType, Severity, type Finding } from "../types/index.js"; +import { recognizeCredentialText } from "./credential-recognition.js"; +import { + containsCredentialLikeText, + sanitizeFindingForOutput, + sanitizeFindingForPersistence, + sanitizeLocationForOutput, + sanitizeRuleIdForOutput, + sanitizeTextForBoundary, + sanitizeValueForBoundary, +} from "./finding-safety.js"; + +function syntheticScannerCorpus(): string[] { + const github = (kind: string, pairs: number) => `gh${kind}_${"A_".repeat(pairs)}`; + const values = new Set([ + `AK${"IA"}${"AB12".repeat(4)}`, + `AS${"IA"}${"CD34".repeat(4)}`, + `aws_secret_access_key=${"Ab1/".repeat(10)}`, + github("p", 18), + github("o", 18), + github("s", 18), + github("r", 18), + `github_${"pat"}_${"B_".repeat(11)}`, + `sk_${"live"}_${"Ab1C".repeat(6)}`, + `pk_${"live"}_${"Cd2E".repeat(6)}`, + `api_key="${"Ab1_".repeat(4)}"`, + `-----BEGIN ${"OPEN"}SSH PRIVATE KEY-----`, + `${"eyJ"}${"Ab1Cd2Ef3G"}.${"Hi4Jk5Lm6N"}.${"Op7Qr8St9U"}`, + `${"xox"}b-${"Ab1-".repeat(6)}Z`, + `${"xox"}p-${"Cd2-".repeat(6)}Y`, + `${"xox"}s-${"Ef3-".repeat(6)}X`, + `${"post"}gresql://synthetic:only@example.invalid/db`, + `${"my"}sql://synthetic:only@example.invalid/db`, + `${"mongo"}db+srv://synthetic:only@example.invalid/db`, + `api_key=${"Q_2r".repeat(4)}`, + "AbCdEfGhIjKlMnOpQrStUvWxYz0123456789+/", + ]); + + for (const kind of ["p", "o", "s", "r"]) { + for (const length of [36, 37, 64]) { + for (const alphabet of ["A", "a", "0", "_", "Aa0_"]) { + values.add(`gh${kind}_${alphabet.repeat(Math.ceil(length / alphabet.length)).slice(0, length)}`); + } + } + } + for (const length of [22, 23, 64]) { + for (const alphabet of ["B", "b", "1", "_", "Bb1_"]) { + values.add(`github_${"pat"}_${alphabet.repeat(Math.ceil(length / alphabet.length)).slice(0, length)}`); + } + } + for (const name of ["api_key", "apikey", "API-KEY"]) { + for (const delimiter of ["=", ":"]) { + for (const quote of ["'", '"']) { + values.add(`${name}${delimiter}${quote}${"Aa1_".repeat(4)}${quote}`); + } + } + } + return [...values]; +} + +function findingWith(value: string): Finding { + return { + id: "finding-safe", + scan_id: "scan-safe", + rule_id: `rule-${value}`, + scanner_type: ScannerType.Code, + severity: Severity.High, + file: `src/${value}/app.ts`, + line: 1, + column: 1, + end_line: null, + message: `Adjacent value: ${value}`, + code_snippet: `const adjacent = ${JSON.stringify(value)}`, + fingerprint: "safe-fingerprint", + suppressed: true, + suppressed_reason: `Reason ${value}`, + llm_explanation: `Analysis ${value}`, + llm_fix: `Fix ${value}`, + llm_exploitability: 0.5, + created_at: "2026-07-15T00:00:00.000Z", + }; +} + +const originalLog = console.log; +let cleanupDb: (() => void) | undefined; + +afterEach(() => { + console.log = originalLog; + cleanupDb?.(); + cleanupDb = undefined; +}); + +describe("scanner-to-boundary credential invariant", () => { + test("every synthetic scanner recognition is absent from every shared boundary", () => { + for (const value of syntheticScannerCorpus()) { + expect(scanFile(".env", value).length, value).toBeGreaterThan(0); + expect(recognizeCredentialText(value, { envLike: true }).length, value).toBeGreaterThan(0); + expect(containsCredentialLikeText(value), value).toBe(true); + + const finding = findingWith(value); + const rendered: string[] = []; + console.log = (...args: unknown[]) => rendered.push(args.map(String).join(" ")); + reportTerminal([finding]); + + const boundaryOutputs = [ + sanitizeTextForBoundary(`prefix ${value} suffix`, 12_000), + sanitizeLocationForOutput(`/tmp/${value}/target`), + sanitizeRuleIdForOutput(`rule-${value}`), + JSON.stringify(sanitizeValueForBoundary({ [value]: { nested: value } })), + JSON.stringify(sanitizeFindingForPersistence(finding)), + JSON.stringify(sanitizeFindingForOutput(finding)), + reportJson([finding]), + reportSarif([finding]), + rendered.join("\n"), + JSON.stringify(sanitizeMessagesForProvider([{ role: "user", content: value }])), + ]; + + for (const output of boundaryOutputs) { + expect(output, value).not.toContain(value); + expect(output, value).toContain("REDACTED"); + } + } + }); + + test("every synthetic scanner recognition is absent from create, update, legacy read, and raw SQLite", () => { + cleanupDb = setupTestDb(); + const rule = createRule({ + name: "synthetic-boundary-rule", + description: "Synthetic invariant fixture", + scanner_type: ScannerType.Code, + severity: Severity.High, + pattern: null, + enabled: true, + builtin: false, + metadata: {}, + }); + const db = getCurrentTestDb(); + + for (const [index, value] of syntheticScannerCorpus().entries()) { + const project = createProject(`project ${value}`, `/tmp/${value}/project`); + const scan = createScan(project.id, [ScannerType.Code]); + const created = createFinding(scan.id, { + rule_id: rule.id, + scanner_type: ScannerType.Code, + severity: Severity.High, + file: `src/${value}/app.ts`, + line: 1, + message: `Adjacent ${value}`, + code_snippet: value, + }); + updateFinding(created.id, { + suppressed: true, + suppressed_reason: value, + llm_explanation: value, + llm_fix: value, + }); + + const rawCreated = JSON.stringify(db.prepare( + "SELECT rule_id, file, message, code_snippet, suppressed_reason, llm_explanation, llm_fix FROM findings WHERE id = ?", + ).get(created.id)); + const rawProject = JSON.stringify(db.prepare( + "SELECT name, path FROM projects WHERE id = ?", + ).get(project.id)); + expect(rawCreated, value).not.toContain(value); + expect(rawProject, value).not.toContain(value); + expect(JSON.stringify(getFinding(created.id)), value).not.toContain(value); + + const legacyId = `legacy-${index}`; + db.prepare( + `INSERT INTO findings + (id, scan_id, rule_id, scanner_type, severity, file, line, message, code_snippet, fingerprint, suppressed, suppressed_reason, llm_explanation, llm_fix, created_at) + VALUES (?, ?, ?, ?, ?, ?, 1, ?, ?, ?, 1, ?, ?, ?, ?)`, + ).run( + legacyId, + scan.id, + rule.id, + ScannerType.Code, + Severity.High, + value, + value, + value, + `legacy-fingerprint-${index}`, + value, + value, + value, + "2026-07-15T00:00:00.000Z", + ); + expect(JSON.stringify(getFinding(legacyId)), value).not.toContain(value); + expect(JSON.stringify(db.prepare( + "SELECT file, message, code_snippet, suppressed_reason, llm_explanation, llm_fix FROM findings WHERE id = ?", + ).get(legacyId)), value).not.toContain(value); + } + }); + + test("safe noncredentials remain unchanged", () => { + const safeValues = [ + "src/app.ts", + "rule.api-v2", + "ordinary config issue", + "api_key=short", + "github_issue_123", + "A".repeat(64), + "database migration documentation", + ]; + for (const value of safeValues) { + expect(scanFile("safe.txt", value), value).toEqual([]); + expect(recognizeCredentialText(value, { boundary: true }), value).toEqual([]); + expect(sanitizeTextForBoundary(value, 12_000), value).toBe(value); + expect(sanitizeLocationForOutput(value), value).toBe(value); + } + }); +}); diff --git a/src/lib/credential-recognition.ts b/src/lib/credential-recognition.ts new file mode 100644 index 0000000..5604c30 --- /dev/null +++ b/src/lib/credential-recognition.ts @@ -0,0 +1,252 @@ +import { Severity } from "../types/index.js"; + +export interface CredentialPattern { + id: string; + name: string; + pattern: RegExp; + severity: Severity; +} + +export interface CredentialRecognition { + index: number; + length: number; + rule: CredentialPattern; +} + +interface CredentialPatternDefinition { + flags: string; + id: string; + name: string; + severity: Severity; + source: string; +} + +// These definitions are the sole named-credential vocabulary for both the +// secrets scanner and every persistence/output/provider boundary. Keep the +// definitions data-only so each consumer gets a fresh RegExp and cannot leak +// lastIndex state into another scan. +const SCANNER_PATTERN_DEFINITIONS: CredentialPatternDefinition[] = [ + { + id: "aws-access-key", + name: "AWS Access Key", + source: String.raw`\b(?:AKIA|ASIA)[0-9A-Z]{16}\b`, + flags: "g", + severity: Severity.Critical, + }, + { + id: "aws-secret-key", + name: "AWS Secret Key", + source: String.raw`(?:aws_secret_access_key|aws_secret_key|secret_access_key)\s*[=:]\s*['"]?([A-Za-z0-9/+=]{40})['"]?`, + flags: "gi", + severity: Severity.Critical, + }, + { + id: "github-token", + name: "GitHub Token", + source: String.raw`\b(?:ghp_[A-Za-z0-9_]{36,}|gho_[A-Za-z0-9_]{36,}|ghs_[A-Za-z0-9_]{36,}|ghr_[A-Za-z0-9_]{36,}|github_pat_[A-Za-z0-9_]{22,})\b`, + flags: "g", + severity: Severity.Critical, + }, + { + id: "stripe-secret-key", + name: "Stripe Secret Key", + source: String.raw`\bsk_live_[A-Za-z0-9]{24,}\b`, + flags: "g", + severity: Severity.Critical, + }, + { + id: "stripe-publishable-key", + name: "Stripe Publishable Key", + source: String.raw`\bpk_live_[A-Za-z0-9]{24,}\b`, + flags: "g", + severity: Severity.Medium, + }, + { + id: "generic-api-key", + name: "Generic API Key", + source: String.raw`(?:api_key|apikey|api[-_]?key)\s*[=:]\s*['"]([A-Za-z0-9_\-]{16,})['"]`, + flags: "gi", + severity: Severity.High, + }, + { + id: "private-key", + name: "Private Key", + source: String.raw`-----BEGIN\s+(?:RSA|DSA|EC|PGP|OPENSSH)?\s*PRIVATE KEY-----`, + flags: "g", + severity: Severity.Critical, + }, + { + id: "jwt-token", + name: "JWT Token", + source: String.raw`\beyJ[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}\b`, + flags: "g", + severity: Severity.High, + }, + { + id: "slack-token", + name: "Slack Token", + source: String.raw`\bxox[bps]-[A-Za-z0-9\-]{24,}\b`, + flags: "g", + severity: Severity.Critical, + }, + { + id: "database-url", + name: "Database URL", + source: String.raw`\b(?:postgres(?:ql)?` + String.raw`://[^\s'"]+|mysql` + + String.raw`://[^\s'"]+|mongodb(?:\+srv)?` + String.raw`://[^\s'"]+)`, + flags: "gi", + severity: Severity.High, + }, +]; + +// Boundary-only formats are deliberately conservative additions. They do not +// expand ordinary file-scan findings, but they do make the shared recognizer a +// superset at every output boundary. +const BOUNDARY_ONLY_PATTERN_DEFINITIONS: CredentialPatternDefinition[] = [ + { + id: "stripe-test-key", + name: "Stripe Test Key", + source: String.raw`\bsk_test_[A-Za-z0-9]{12,}\b`, + flags: "gi", + severity: Severity.High, + }, + { + id: "provider-api-key", + name: "Provider API Key", + source: String.raw`\bsk-(?:live|test|proj)-[A-Za-z0-9_-]{12,}\b`, + flags: "gi", + severity: Severity.High, + }, + { + id: "bearer-token", + name: "Bearer Token", + source: String.raw`\bBearer\s+[A-Za-z0-9._~+/-]{12,}={0,2}\b`, + flags: "gi", + severity: Severity.High, + }, + { + id: "generic-credential-assignment", + name: "Generic Credential Assignment", + source: String.raw`\b(?:api[_-]?key|access[_-]?key|secret(?:[_-]?key)?|token|password|passwd|passphrase|credential)\s*[:=]\s*["']?[^\s"'\x60,;]{8,}["']?`, + flags: "gi", + severity: Severity.High, + }, + { + id: "authenticated-url", + name: "Authenticated URL", + source: String.raw`\b(?:https?|ssh)://[^\s/:@]+:[^\s/@]+@[^\s]+`, + flags: "gi", + severity: Severity.High, + }, +]; + +const ENV_API_KEY_DEFINITION: CredentialPatternDefinition = { + id: "generic-api-key", + name: "Generic API Key", + source: String.raw`(?:api_key|apikey|api[-_]?key)\s*=\s*([A-Za-z0-9_\-]{16,})(?=\s|$|[;,#])`, + flags: "gi", + severity: Severity.High, +}; + +const HIGH_ENTROPY_HEX_DEFINITION: CredentialPatternDefinition = { + id: "high-entropy-hex", + name: "High-entropy hex string", + source: String.raw`\b[0-9a-fA-F]{16,}\b`, + flags: "g", + severity: Severity.Medium, +}; + +const HIGH_ENTROPY_BASE64_DEFINITION: CredentialPatternDefinition = { + id: "high-entropy-base64", + name: "High-entropy base64 string", + source: String.raw`\b[A-Za-z0-9+/=]{20,}\b`, + flags: "g", + severity: Severity.Medium, +}; + +function materialize(definition: CredentialPatternDefinition): CredentialPattern { + return { + id: definition.id, + name: definition.name, + pattern: new RegExp(definition.source, definition.flags), + severity: definition.severity, + }; +} + +export const SECRET_PATTERNS: CredentialPattern[] = SCANNER_PATTERN_DEFINITIONS.map(materialize); + +export function shannonEntropy(value: string): number { + if (value.length === 0) return 0; + const frequencies = new Map(); + for (const character of value) { + frequencies.set(character, (frequencies.get(character) ?? 0) + 1); + } + let entropy = 0; + for (const count of frequencies.values()) { + const probability = count / value.length; + entropy -= probability * Math.log2(probability); + } + return entropy; +} + +function collectPatternMatches( + value: string, + definitions: CredentialPatternDefinition[], +): CredentialRecognition[] { + const recognitions: CredentialRecognition[] = []; + for (const definition of definitions) { + const rule = materialize(definition); + let match: RegExpExecArray | null; + while ((match = rule.pattern.exec(value)) !== null) { + recognitions.push({ index: match.index, length: match[0].length, rule }); + if (match[0].length === 0) rule.pattern.lastIndex++; + } + } + return recognitions; +} + +function collectEntropyMatches(value: string): CredentialRecognition[] { + const recognitions: CredentialRecognition[] = []; + for (const [definition, threshold] of [ + [HIGH_ENTROPY_HEX_DEFINITION, 4.5], + [HIGH_ENTROPY_BASE64_DEFINITION, 5.0], + ] as const) { + const rule = materialize(definition); + let match: RegExpExecArray | null; + while ((match = rule.pattern.exec(value)) !== null) { + if (shannonEntropy(match[0]) > threshold) { + recognitions.push({ index: match.index, length: match[0].length, rule }); + } + if (match[0].length === 0) rule.pattern.lastIndex++; + } + } + return recognitions; +} + +export interface CredentialRecognitionOptions { + boundary?: boolean; + envLike?: boolean; +} + +/** + * Canonical credential recognizer used by both the scanner and every trust + * boundary. A boundary is a strict superset of scanner recognition. + */ +export function recognizeCredentialText( + value: string, + options: CredentialRecognitionOptions = {}, +): CredentialRecognition[] { + const definitions = options.boundary + ? [...SCANNER_PATTERN_DEFINITIONS, ...BOUNDARY_ONLY_PATTERN_DEFINITIONS] + : SCANNER_PATTERN_DEFINITIONS; + const recognitions = collectPatternMatches(value, definitions); + if (options.envLike || options.boundary) { + recognitions.push(...collectPatternMatches(value, [ENV_API_KEY_DEFINITION])); + } + recognitions.push(...collectEntropyMatches(value)); + return recognitions.sort((left, right) => left.index - right.index || right.length - left.length); +} + +export function containsRecognizedCredential(value: string | null | undefined): boolean { + return Boolean(value && recognizeCredentialText(value, { boundary: true }).length > 0); +} diff --git a/src/lib/finding-safety.ts b/src/lib/finding-safety.ts index e30240e..0df1a9d 100644 --- a/src/lib/finding-safety.ts +++ b/src/lib/finding-safety.ts @@ -1,5 +1,6 @@ import { createHash } from "crypto"; import { ScannerType, type Finding, type FindingInput } from "../types/index.js"; +import { containsRecognizedCredential } from "./credential-recognition.js"; export const REDACTED_FINDING_TEXT = "[REDACTED]"; @@ -9,27 +10,6 @@ const MAX_RULE_ID_LENGTH = 128; type FindingLike = FindingInput | Finding; -// Deliberately conservative credential-value patterns. These run at trust -// boundaries in addition to the scanner's richer detection rules, so adjacent -// secrets cannot ride along with an otherwise non-credential finding. -const CREDENTIAL_PATTERNS: RegExp[] = [ - /-----BEGIN (?:[A-Z0-9 ]+ )?PRIVATE KEY-----[\s\S]*?-----END (?:[A-Z0-9 ]+ )?PRIVATE KEY-----/gi, - /\bgh[opusr]_[A-Za-z0-9]{20,}\b/g, - /\bgithub_pat_[A-Za-z0-9_]{20,}\b/g, - /\bAKIA[0-9A-Z]{16}\b/g, - /\bASIA[0-9A-Z]{16}\b/g, - /(?:aws_secret_access_key|aws_secret_key|secret_access_key)\s*[=:]\s*["']?[A-Za-z0-9/+=]{40}["']?/gi, - /\bxox[baprs]-[A-Za-z0-9-]{10,}\b/g, - /\bsk_(?:live|test)_[A-Za-z0-9]{12,}\b/gi, - /\bpk_live_[A-Za-z0-9]{24,}\b/gi, - /\bsk-(?:live|test|proj)-[A-Za-z0-9_-]{12,}\b/gi, - /\beyJ[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\b/g, - /\bBearer\s+[A-Za-z0-9._~+\/-]{12,}={0,2}\b/gi, - /\b(?:api[_-]?key|access[_-]?key|secret(?:[_-]?key)?|token|password|passwd|passphrase|credential)\s*[:=]\s*["']?[^\s"'`,;]{8,}["']?/gi, - /\b(?:https?|ssh):\/\/[^\s/:@]+:[^\s/@]+@[^\s]+/gi, - /\b(?:postgres(?:ql)?|mysql|mongodb(?:\+srv)?):\/\/[^\s"']+/gi, -]; - function boundedSingleLine(value: string, maxLength: number): string { const normalized = value.replace(/[\u0000-\u001f\u007f]/g, " ").replace(/\s+/g, " ").trim(); if (normalized.length <= maxLength) return normalized; @@ -42,27 +22,7 @@ function stableRedaction(value: string, kind: string): string { } export function containsCredentialLikeText(value: string | null | undefined): boolean { - if (!value) return false; - if (CREDENTIAL_PATTERNS.some((pattern) => { - pattern.lastIndex = 0; - return pattern.test(value); - })) return true; - - // Refuse high-entropy opaque tokens even when they do not match a named - // provider format. This mirrors the scanner's last-resort secret heuristic. - for (const candidate of value.match(/[A-Za-z0-9+/=_-]{20,}/g) ?? []) { - const frequencies = new Map(); - for (const character of candidate) { - frequencies.set(character, (frequencies.get(character) ?? 0) + 1); - } - let entropy = 0; - for (const count of frequencies.values()) { - const probability = count / candidate.length; - entropy -= probability * Math.log2(probability); - } - if (entropy > 5) return true; - } - return false; + return containsRecognizedCredential(value); } /** Redact credential values from arbitrary untrusted text before a boundary. */ @@ -73,12 +33,7 @@ export function sanitizeTextForBoundary( if (containsCredentialLikeText(value)) { return `${REDACTED_FINDING_TEXT} ${stableRedaction(value, "TEXT")}`; } - let sanitized = value; - for (const pattern of CREDENTIAL_PATTERNS) { - pattern.lastIndex = 0; - sanitized = sanitized.replace(pattern, REDACTED_FINDING_TEXT); - } - return boundedSingleLine(sanitized, maxLength); + return boundedSingleLine(value, maxLength); } /** Recursively sanitize JSON-compatible data before persistence or output. */ @@ -101,7 +56,7 @@ export function sanitizeLocationForOutput(value: string): string { : boundedSingleLine(value, MAX_LOCATION_LENGTH); } -function safeRuleId(value: string): string { +export function sanitizeRuleIdForOutput(value: string): string { if (containsCredentialLikeText(value)) return stableRedaction(value, "RULE"); const normalized = value.replace(/[^A-Za-z0-9._-]/g, "-"); return boundedSingleLine(normalized || "finding", MAX_RULE_ID_LENGTH); @@ -119,7 +74,7 @@ export function isCredentialFinding(finding: Pick(finding: T): T { const sensitive = isCredentialFinding(finding); - const ruleId = safeRuleId(finding.rule_id); + const ruleId = sanitizeRuleIdForOutput(finding.rule_id); const result = { ...finding, rule_id: ruleId, diff --git a/src/mcp/build-server.ts b/src/mcp/build-server.ts index d0aa2ab..13c7f1d 100644 --- a/src/mcp/build-server.ts +++ b/src/mcp/build-server.ts @@ -6,6 +6,7 @@ import { getDb } from "../db/database.js"; import { seedBuiltinRules } from "../db/index.js"; import { seedAdvisories } from "../data/advisories.js"; import { PACKAGE_VERSION } from "../lib/version.js"; +import { sanitizeTextForBoundary, sanitizeValueForBoundary } from "../lib/finding-safety.js"; import { registerScanTools } from "./tools/scan.js"; import { registerFindingTools } from "./tools/findings.js"; @@ -27,7 +28,12 @@ function ensureSeeded(): void { } function jsonResult(data: unknown): { content: Array<{ type: "text"; text: string }> } { - return { content: [{ type: "text" as const, text: JSON.stringify(data, null, 2) }] }; + return { + content: [{ + type: "text" as const, + text: JSON.stringify(sanitizeValueForBoundary(data), null, 2), + }], + }; } function getCodeContext(filePath: string, line: number, contextLines = 10): string { @@ -60,12 +66,16 @@ export function createMcpServer(): McpServer { const existing = [..._agentReg.values()].find((x) => x.name === a.name); if (existing) { existing.last_seen_at = new Date().toISOString(); - return { content: [{ type: "text" as const, text: JSON.stringify(existing) }] }; + return jsonResult(existing); } const id = Math.random().toString(36).slice(2, 10); - const ag = { id, name: a.name, last_seen_at: new Date().toISOString() }; + const ag = { + id, + name: sanitizeTextForBoundary(a.name, 256), + last_seen_at: new Date().toISOString(), + }; _agentReg.set(id, ag); - return { content: [{ type: "text" as const, text: JSON.stringify(ag) }] }; + return jsonResult(ag); }, ); @@ -75,9 +85,9 @@ export function createMcpServer(): McpServer { { agent_id: z.string() }, async (a: { agent_id: string }) => { const ag = _agentReg.get(a.agent_id); - if (!ag) return { content: [{ type: "text" as const, text: `Agent not found: ${a.agent_id}` }], isError: true }; + if (!ag) return { ...jsonResult({ error: `Agent not found: ${a.agent_id}` }), isError: true }; ag.last_seen_at = new Date().toISOString(); - return { content: [{ type: "text" as const, text: JSON.stringify({ id: ag.id, name: ag.name, last_seen_at: ag.last_seen_at }) }] }; + return jsonResult({ id: ag.id, name: ag.name, last_seen_at: ag.last_seen_at }); }, ); @@ -87,9 +97,12 @@ export function createMcpServer(): McpServer { { agent_id: z.string(), project_id: z.string().nullable().optional() }, async (a: { agent_id: string; project_id?: string | null }) => { const ag = _agentReg.get(a.agent_id); - if (!ag) return { content: [{ type: "text" as const, text: `Agent not found: ${a.agent_id}` }], isError: true }; - (ag as { project_id?: string }).project_id = a.project_id ?? undefined; - return { content: [{ type: "text" as const, text: a.project_id ? `Focus: ${a.project_id}` : "Focus cleared" }] }; + if (!ag) return { ...jsonResult({ error: `Agent not found: ${a.agent_id}` }), isError: true }; + const safeProjectId = a.project_id == null + ? undefined + : sanitizeTextForBoundary(a.project_id, 256); + (ag as { project_id?: string }).project_id = safeProjectId; + return jsonResult({ focus: safeProjectId ?? null }); }, ); @@ -99,8 +112,8 @@ export function createMcpServer(): McpServer { {}, async () => { const agents = [..._agentReg.values()]; - if (agents.length === 0) return { content: [{ type: "text" as const, text: "No agents registered." }] }; - return { content: [{ type: "text" as const, text: JSON.stringify(agents, null, 2) }] }; + if (agents.length === 0) return jsonResult({ agents: [] }); + return jsonResult({ agents }); }, ); @@ -111,12 +124,16 @@ export function createMcpServer(): McpServer { async (params: { message: string; email?: string; category?: string }) => { try { const db = getDb(); + const safeMessage = sanitizeTextForBoundary(params.message, 12_000); + const safeEmail = params.email == null + ? null + : sanitizeTextForBoundary(params.email, 320); db.prepare("INSERT INTO feedback (message, email, category, version) VALUES (?, ?, ?, ?)").run( - params.message, params.email || null, params.category || "general", PACKAGE_VERSION, + safeMessage, safeEmail, params.category || "general", PACKAGE_VERSION, ); - return { content: [{ type: "text" as const, text: "Feedback saved. Thank you!" }] }; + return jsonResult({ status: "Feedback saved" }); } catch (e) { - return { content: [{ type: "text" as const, text: String(e) }], isError: true }; + return { ...jsonResult({ error: String(e) }), isError: true }; } }, ); diff --git a/src/mcp/http.test.ts b/src/mcp/http.test.ts index 4ca5906..ca84f09 100644 --- a/src/mcp/http.test.ts +++ b/src/mcp/http.test.ts @@ -84,6 +84,36 @@ describe("startMcpHttpServer", () => { await client.close(); }); + it("redacts scanner-recognized values from successful and error MCP responses", async () => { + const synthetic = `gh${"o"}_${"A_".repeat(18)}`; + httpServer = await startMcpHttpServer({ + port: 0, + healthName: "security", + createServer: createMcpServer, + }); + const port = getListeningPort(httpServer); + const transport = new StreamableHTTPClientTransport(new URL(`http://127.0.0.1:${port}/mcp`)); + const client = new Client({ name: "test", version: "1.0.0" }); + await client.connect(transport); + + const registered = await client.callTool({ + name: "register_agent", + arguments: { name: `agent-${synthetic}` }, + }); + const missing = await client.callTool({ + name: "heartbeat", + arguments: { agent_id: synthetic }, + }); + const listed = await client.callTool({ name: "list_agents", arguments: {} }); + + for (const result of [registered, missing, listed]) { + const output = JSON.stringify(result); + expect(output).not.toContain(synthetic); + expect(output).toContain("REDACTED"); + } + await client.close(); + }); + it("serves multiple concurrent clients from one process", async () => { httpServer = await startMcpHttpServer({ port: 0, diff --git a/src/scanners/secrets.ts b/src/scanners/secrets.ts index d783005..7a9ac74 100644 --- a/src/scanners/secrets.ts +++ b/src/scanners/secrets.ts @@ -5,9 +5,12 @@ import { type FindingInput, type ScannerRunOptions, ScannerType, - Severity, DEFAULT_CONFIG, } from "../types/index.js"; +import { recognizeCredentialText } from "../lib/credential-recognition.js"; + +export { SECRET_PATTERNS, shannonEntropy } from "../lib/credential-recognition.js"; +export type { CredentialPattern as SecretPattern } from "../lib/credential-recognition.js"; // --- Shared utilities --- @@ -582,178 +585,6 @@ export function isFindingSuppressedBySecurityIgnore( // --- Secret patterns --- -export interface SecretPattern { - id: string; - name: string; - pattern: RegExp; - severity: Severity; -} - -export const SECRET_PATTERNS: SecretPattern[] = [ - { - id: "aws-access-key", - name: "AWS Access Key", - pattern: /\bAKIA[0-9A-Z]{16}\b/g, - severity: Severity.Critical, - }, - { - id: "aws-secret-key", - name: "AWS Secret Key", - pattern: /(?:aws_secret_access_key|aws_secret_key|secret_access_key)\s*[=:]\s*['"]?([A-Za-z0-9/+=]{40})['"]?/gi, - severity: Severity.Critical, - }, - { - id: "github-token", - name: "GitHub Token", - pattern: /\b(ghp_[A-Za-z0-9_]{36,}|gho_[A-Za-z0-9_]{36,}|ghs_[A-Za-z0-9_]{36,}|ghr_[A-Za-z0-9_]{36,}|github_pat_[A-Za-z0-9_]{22,})\b/g, - severity: Severity.Critical, - }, - { - id: "stripe-secret-key", - name: "Stripe Secret Key", - pattern: /\b(sk_live_[A-Za-z0-9]{24,})\b/g, - severity: Severity.Critical, - }, - { - id: "stripe-publishable-key", - name: "Stripe Publishable Key", - pattern: /\b(pk_live_[A-Za-z0-9]{24,})\b/g, - severity: Severity.Medium, - }, - { - id: "generic-api-key", - name: "Generic API Key", - pattern: /(?:api_key|apikey|api[-_]?key)\s*[=:]\s*['"]([A-Za-z0-9_\-]{16,})['"]/gi, - severity: Severity.High, - }, - { - id: "private-key", - name: "Private Key", - pattern: /-----BEGIN\s+(?:RSA|DSA|EC|PGP|OPENSSH)?\s*PRIVATE KEY-----/g, - severity: Severity.Critical, - }, - { - id: "jwt-token", - name: "JWT Token", - pattern: /\beyJ[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}\b/g, - severity: Severity.High, - }, - { - id: "slack-token", - name: "Slack Token", - pattern: /\b(xoxb-[A-Za-z0-9\-]{24,}|xoxp-[A-Za-z0-9\-]{24,}|xoxs-[A-Za-z0-9\-]{24,})\b/g, - severity: Severity.Critical, - }, - { - id: "database-url", - name: "Database URL", - pattern: /\b(postgres(?:ql)?:\/\/[^\s'"]+|mysql:\/\/[^\s'"]+|mongodb(?:\+srv)?:\/\/[^\s'"]+)/gi, - severity: Severity.High, - }, -]; - -// --- Shannon entropy --- - -export function shannonEntropy(str: string): number { - if (str.length === 0) return 0; - - const freq: Record = {}; - for (const ch of str) { - freq[ch] = (freq[ch] || 0) + 1; - } - - let entropy = 0; - const len = str.length; - for (const count of Object.values(freq)) { - const p = count / len; - if (p > 0) { - entropy -= p * Math.log2(p); - } - } - return entropy; -} - -const HEX_RE = /\b[0-9a-fA-F]{16,}\b/g; -const BASE64_RE = /\b[A-Za-z0-9+/=]{20,}\b/g; -const UNQUOTED_ENV_API_KEY_RE = /(?:api_key|apikey|api[-_]?key)\s*=\s*([A-Za-z0-9_\-]{16,})(?=\s|$|[;,#])/gi; - -function detectUnquotedEnvApiKeys( - content: string, - filePath: string, - line: number, - lineText: string, - securityIgnore: SecurityIgnoreLineScan, -): FindingInput[] { - if (!isEnvLikeFile(filePath)) return []; - - const findings: FindingInput[] = []; - let match: RegExpExecArray | null; - UNQUOTED_ENV_API_KEY_RE.lastIndex = 0; - while ((match = UNQUOTED_ENV_API_KEY_RE.exec(lineText)) !== null) { - if (isFindingSuppressedBySecurityIgnore(securityIgnore, match.index)) continue; - findings.push({ - rule_id: "generic-api-key", - scanner_type: ScannerType.Secrets, - severity: Severity.High, - file: filePath, - line, - column: match.index + 1, - message: "Generic API Key detected", - code_snippet: REDACTED_CODE_SNIPPET, - }); - } - - return findings; -} - -function detectHighEntropyStrings( - content: string, - filePath: string, - line: number, - lineText: string, - securityIgnore: SecurityIgnoreLineScan, -): FindingInput[] { - const findings: FindingInput[] = []; - - let hexMatch: RegExpExecArray | null; - HEX_RE.lastIndex = 0; - while ((hexMatch = HEX_RE.exec(lineText)) !== null) { - const token = hexMatch[0]; - if (isFindingSuppressedBySecurityIgnore(securityIgnore, hexMatch.index)) continue; - if (shannonEntropy(token) > 4.5) { - findings.push({ - rule_id: "high-entropy-hex", - scanner_type: ScannerType.Secrets, - severity: Severity.Medium, - file: filePath, - line, - message: "High-entropy hex string detected (possible secret)", - code_snippet: REDACTED_CODE_SNIPPET, - }); - } - } - - let b64Match: RegExpExecArray | null; - BASE64_RE.lastIndex = 0; - while ((b64Match = BASE64_RE.exec(lineText)) !== null) { - const token = b64Match[0]; - if (isFindingSuppressedBySecurityIgnore(securityIgnore, b64Match.index)) continue; - if (shannonEntropy(token) > 5.0) { - findings.push({ - rule_id: "high-entropy-base64", - scanner_type: ScannerType.Secrets, - severity: Severity.Medium, - file: filePath, - line, - message: "High-entropy base64 string detected (possible secret)", - code_snippet: REDACTED_CODE_SNIPPET, - }); - } - } - - return findings; -} - // --- Scanner --- export function scanFile(filePath: string, content: string): FindingInput[] { @@ -781,26 +612,19 @@ export function scanFile(filePath: string, content: string): FindingInput[] { blockComment = securityIgnore.finalBlockComment; blockCommentHasSecurityIgnore = securityIgnore.finalBlockCommentHasSecurityIgnore; - for (const sp of SECRET_PATTERNS) { - sp.pattern.lastIndex = 0; - let match: RegExpExecArray | null; - while ((match = sp.pattern.exec(lineText)) !== null) { - if (isFindingSuppressedBySecurityIgnore(securityIgnore, match.index)) continue; - findings.push({ - rule_id: sp.id, - scanner_type: ScannerType.Secrets, - severity: sp.severity, - file: filePath, - line: lineNum, - column: match.index + 1, - message: `${sp.name} detected`, - code_snippet: REDACTED_CODE_SNIPPET, - }); - } + for (const recognition of recognizeCredentialText(lineText, { envLike: isEnvLikeFile(filePath) })) { + if (isFindingSuppressedBySecurityIgnore(securityIgnore, recognition.index)) continue; + findings.push({ + rule_id: recognition.rule.id, + scanner_type: ScannerType.Secrets, + severity: recognition.rule.severity, + file: filePath, + line: lineNum, + column: recognition.index + 1, + message: `${recognition.rule.name} detected`, + code_snippet: REDACTED_CODE_SNIPPET, + }); } - - findings.push(...detectUnquotedEnvApiKeys(content, filePath, lineNum, lineText, securityIgnore)); - findings.push(...detectHighEntropyStrings(content, filePath, lineNum, lineText, securityIgnore)); } return findings; diff --git a/src/server/scan-boundary.test.ts b/src/server/scan-boundary.test.ts index fea9baf..249dfa2 100644 --- a/src/server/scan-boundary.test.ts +++ b/src/server/scan-boundary.test.ts @@ -1,6 +1,6 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { spawn, type ChildProcess } from "child_process"; -import { mkdtempSync, rmSync } from "fs"; +import { mkdirSync, mkdtempSync, rmSync } from "fs"; import { createServer } from "net"; import { tmpdir } from "os"; import { join } from "path"; @@ -71,4 +71,48 @@ describe("REST scan source boundary", () => { expect(scan.scanner_types).not.toContain(ScannerType.GitHistory); expect(scan.scanner_types).toContain(ScannerType.Code); }); + + test("REST project responses redact scanner-recognized paths", async () => { + const synthetic = `gh${"p"}_${"A_".repeat(18)}`; + const projectDir = join(tempDir, synthetic); + mkdirSync(projectDir); + const port = await availablePort(); + child = spawn("bun", ["run", "src/server/index.ts"], { + cwd: process.cwd(), + env: { + ...process.env, + PORT: String(port), + HOME: tempDir, + USERPROFILE: tempDir, + SECURITY_DB: join(tempDir, "shield.db"), + CEREBRAS_API_KEY: "", + }, + stdio: ["ignore", "pipe", "pipe"], + }); + await new Promise((resolve, reject) => { + const timeout = setTimeout(() => reject(new Error("REST test server did not start")), 5_000); + child!.once("exit", (code) => { + clearTimeout(timeout); + reject(new Error(`REST test server exited early (${code})`)); + }); + child!.stdout!.on("data", (chunk) => { + if (String(chunk).includes("security dashboard")) { + clearTimeout(timeout); + resolve(); + } + }); + }); + + const created = await fetch(`http://127.0.0.1:${port}/api/projects`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ name: `project-${synthetic}`, path: projectDir }), + }).then((response) => response.json()); + const listed = await fetch(`http://127.0.0.1:${port}/api/projects`) + .then((response) => response.json()); + for (const output of [JSON.stringify(created), JSON.stringify(listed)]) { + expect(output).not.toContain(synthetic); + expect(output).toContain("REDACTED"); + } + }); }); diff --git a/src/server/serve.ts b/src/server/serve.ts index de5701e..d98aa3c 100644 --- a/src/server/serve.ts +++ b/src/server/serve.ts @@ -46,7 +46,7 @@ import { analyzeFinding as llmAnalyze, isLLMAvailable, } from "../llm/index.js"; -import { isCredentialFinding } from "../lib/finding-safety.js"; +import { isCredentialFinding, sanitizeValueForBoundary } from "../lib/finding-safety.js"; import { listAdvisories, getAdvisory, @@ -80,6 +80,15 @@ function getCodeContext(filePath: string, line: number, contextLines = 10): stri export function startServer(port: number) { const app = express(); + + // REST is a trust boundary. Sanitize every response body, including legacy + // rows and exception messages, even when an individual route forgets to do + // so. Safe non-credential values are preserved byte-for-byte. + app.use((_req: Request, res: Response, next: NextFunction) => { + const sendJson = res.json.bind(res); + res.json = ((body: unknown) => sendJson(sanitizeValueForBoundary(body))) as Response["json"]; + next(); + }); app.use(express.json({ limit: "10mb" })); // CORS — restrict to configured origins (defaults to localhost) From 463c2b5fcdbea9b9fd2a2229977235fd463e166e Mon Sep 17 00:00:00 2001 From: Andrei Hasna Date: Thu, 16 Jul 2026 01:03:14 +0300 Subject: [PATCH 4/8] fix: close residual output safety gaps --- sdk/src/client.test.ts | 15 +++++ src/db/findings.ts | 6 +- src/db/scans.test.ts | 37 +++++++++++ src/db/scans.ts | 17 ++++- src/lib/credential-invariant.test.ts | 98 ++++++++++++++++++++++++---- src/lib/credential-recognition.ts | 41 ++++++++++-- src/lib/finding-safety.test.ts | 34 +++++++++- src/lib/finding-safety.ts | 48 +++++++++++++- src/mcp/tools/output-safety.test.ts | 20 +++++- src/reporters/json.test.ts | 55 ++++++++++++++++ src/reporters/json.ts | 10 ++- src/reporters/sarif.test.ts | 24 +++++++ src/reporters/sarif.ts | 17 +++-- 13 files changed, 387 insertions(+), 35 deletions(-) diff --git a/sdk/src/client.test.ts b/sdk/src/client.test.ts index fd847f8..28be803 100644 --- a/sdk/src/client.test.ts +++ b/sdk/src/client.test.ts @@ -5,6 +5,7 @@ import { createServer } from "net"; import { tmpdir } from "os"; import { join, resolve } from "path"; import { OpenSecurityClient } from "./client.js"; +import { Database } from "bun:sqlite"; const originalFetch = globalThis.fetch; let child: ChildProcess | undefined; @@ -96,5 +97,19 @@ describe("OpenSecurityClient scan source boundary", () => { expect(output).not.toContain(synthetic); expect(output).toContain("REDACTED"); } + + const scan = await client.triggerScan(tempDir); + const db = new Database(join(tempDir, "shield.db")); + try { + db.prepare("UPDATE scans SET error = ? WHERE id = ?").run(synthetic, scan.id); + } finally { + db.close(); + } + const fetchedScan = await client.getScan(scan.id); + const listedScans = await client.listScans(); + for (const output of [JSON.stringify(fetchedScan), JSON.stringify(listedScans)]) { + expect(output).not.toContain(synthetic); + expect(output).toContain("REDACTED"); + } }); }); diff --git a/src/db/findings.ts b/src/db/findings.ts index 831747c..bc18bd8 100644 --- a/src/db/findings.ts +++ b/src/db/findings.ts @@ -43,6 +43,8 @@ function rowToFinding(row: FindingRow): Finding { // first read cannot recover fields written by older unsafe versions. if ( safe.rule_id !== row.rule_id || + safe.fingerprint !== row.fingerprint || + safe.created_at !== row.created_at || safe.file !== row.file || safe.message !== row.message || safe.code_snippet !== row.code_snippet || @@ -59,15 +61,17 @@ function rowToFinding(row: FindingRow): Finding { ).run(safe.rule_id, safe.scanner_type, safe.severity); } db.prepare( - `UPDATE findings SET rule_id = ?, file = ?, message = ?, code_snippet = ?, suppressed_reason = ?, llm_explanation = ?, llm_fix = ? WHERE id = ?`, + `UPDATE findings SET rule_id = ?, file = ?, message = ?, code_snippet = ?, fingerprint = ?, suppressed_reason = ?, llm_explanation = ?, llm_fix = ?, created_at = ? WHERE id = ?`, ).run( safe.rule_id, safe.file, safe.message, safe.code_snippet, + safe.fingerprint, safe.suppressed_reason, safe.llm_explanation, safe.llm_fix, + safe.created_at, row.id, ); if (safe.rule_id !== row.rule_id) { diff --git a/src/db/scans.test.ts b/src/db/scans.test.ts index ee4a283..fc312be 100644 --- a/src/db/scans.test.ts +++ b/src/db/scans.test.ts @@ -108,6 +108,43 @@ describe("scans", () => { expect(updated!.error).toBe("Something went wrong"); }); + test("sanitizes scanner-recognized errors before persistence", () => { + const syntheticCredential = `gh${"p"}_${"A_".repeat(18)}`; + const scan = createScan(projectId, [ScannerType.Secrets]); + + updateScanStatus( + scan.id, + ScanStatus.Failed, + undefined, + `Synthetic scanner failure ${syntheticCredential}`, + ); + + const raw = getCurrentTestDb() + .prepare("SELECT error FROM scans WHERE id = ?") + .get(scan.id) as { error: string }; + expect(raw.error).not.toContain(syntheticCredential); + expect(raw.error).toContain("REDACTED"); + expect(JSON.stringify(getScan(scan.id))).not.toContain(syntheticCredential); + }); + + test("sanitizes and opportunistically scrubs legacy scan errors on read", () => { + const syntheticCredential = `gh${"o"}_${"B_".repeat(18)}`; + const scan = createScan(projectId, [ScannerType.Code]); + const db = getCurrentTestDb(); + db.prepare("UPDATE scans SET error = ? WHERE id = ?").run( + `Legacy scanner failure ${syntheticCredential}`, + scan.id, + ); + + const fetched = getScan(scan.id); + expect(JSON.stringify(fetched)).not.toContain(syntheticCredential); + const raw = db.prepare("SELECT error FROM scans WHERE id = ?").get(scan.id) as { + error: string; + }; + expect(raw.error).not.toContain(syntheticCredential); + expect(raw.error).toContain("REDACTED"); + }); + test("completeScan sets completed status, timestamp, and duration", () => { const scan = createScan(projectId, [ScannerType.Secrets]); completeScan(scan.id, 10); diff --git a/src/db/scans.ts b/src/db/scans.ts index 0970d1a..4b4eba8 100644 --- a/src/db/scans.ts +++ b/src/db/scans.ts @@ -2,6 +2,7 @@ import crypto from "crypto"; import { getDb } from "./database.js"; import type { Scan } from "../types/index.js"; import { ScanStatus, type ScannerType } from "../types/index.js"; +import { sanitizeScanForOutput, sanitizeTextForBoundary } from "../lib/finding-safety.js"; interface ScanRow { id: string; @@ -17,11 +18,20 @@ interface ScanRow { } function rowToScan(row: ScanRow): Scan { - return { + const safe = sanitizeScanForOutput({ ...row, status: row.status as ScanStatus, scanner_types: JSON.parse(row.scanner_types) as ScannerType[], - }; + }); + if (safe.error !== row.error) { + try { + getDb().prepare("UPDATE scans SET error = ? WHERE id = ?").run(safe.error, row.id); + } catch { + // Output remains sanitized when a legacy/read-only database cannot be + // rewritten in place. + } + } + return safe; } export function createScan(project_id: string, scanner_types: ScannerType[]): Scan { @@ -80,7 +90,8 @@ export function updateScanStatus( `UPDATE scans SET status = ?, findings_count = COALESCE(?, findings_count), error = COALESCE(?, error) WHERE id = ?` ); - stmt.run(status, findings_count ?? null, error ?? null, id); + const safeError = error == null ? null : sanitizeTextForBoundary(error); + stmt.run(status, findings_count ?? null, safeError, id); } export function completeScan(id: string, findings_count: number): void { diff --git a/src/lib/credential-invariant.test.ts b/src/lib/credential-invariant.test.ts index 06e5cfb..ee973c4 100644 --- a/src/lib/credential-invariant.test.ts +++ b/src/lib/credential-invariant.test.ts @@ -2,21 +2,22 @@ import { afterEach, describe, expect, test } from "bun:test"; import { createFinding, getFinding, updateFinding } from "../db/findings.js"; import { createProject } from "../db/projects.js"; import { createRule } from "../db/rules.js"; -import { createScan } from "../db/scans.js"; +import { createScan, getScan, updateScanStatus } from "../db/scans.js"; import { getCurrentTestDb, setupTestDb } from "../db/test-helpers.js"; import { sanitizeMessagesForProvider } from "../llm/client.js"; import { reportFindings as reportJson } from "../reporters/json.js"; import { reportFindings as reportSarif } from "../reporters/sarif.js"; import { reportFindings as reportTerminal } from "../reporters/terminal.js"; import { scanFile } from "../scanners/secrets.js"; -import { ScannerType, Severity, type Finding } from "../types/index.js"; -import { recognizeCredentialText } from "./credential-recognition.js"; +import { ScanStatus, ScannerType, Severity, type Finding, type Scan } from "../types/index.js"; +import { recognizeCredentialText, shannonEntropy } from "./credential-recognition.js"; import { containsCredentialLikeText, sanitizeFindingForOutput, sanitizeFindingForPersistence, sanitizeLocationForOutput, sanitizeRuleIdForOutput, + sanitizeScanForOutput, sanitizeTextForBoundary, sanitizeValueForBoundary, } from "./finding-safety.js"; @@ -71,8 +72,8 @@ function syntheticScannerCorpus(): string[] { function findingWith(value: string): Finding { return { - id: "finding-safe", - scan_id: "scan-safe", + id: value, + scan_id: value, rule_id: `rule-${value}`, scanner_type: ScannerType.Code, severity: Severity.High, @@ -82,13 +83,13 @@ function findingWith(value: string): Finding { end_line: null, message: `Adjacent value: ${value}`, code_snippet: `const adjacent = ${JSON.stringify(value)}`, - fingerprint: "safe-fingerprint", + fingerprint: value, suppressed: true, suppressed_reason: `Reason ${value}`, llm_explanation: `Analysis ${value}`, llm_fix: `Fix ${value}`, llm_exploitability: 0.5, - created_at: "2026-07-15T00:00:00.000Z", + created_at: value, }; } @@ -109,6 +110,18 @@ describe("scanner-to-boundary credential invariant", () => { expect(containsCredentialLikeText(value), value).toBe(true); const finding = findingWith(value); + const scan: Scan = { + id: value, + project_id: value, + status: value as ScanStatus, + scanner_types: [value as ScannerType], + findings_count: 1, + started_at: value, + completed_at: value, + duration_ms: 1, + error: value, + created_at: value, + }; const rendered: string[] = []; console.log = (...args: unknown[]) => rendered.push(args.map(String).join(" ")); reportTerminal([finding]); @@ -120,8 +133,9 @@ describe("scanner-to-boundary credential invariant", () => { JSON.stringify(sanitizeValueForBoundary({ [value]: { nested: value } })), JSON.stringify(sanitizeFindingForPersistence(finding)), JSON.stringify(sanitizeFindingForOutput(finding)), - reportJson([finding]), - reportSarif([finding]), + JSON.stringify(sanitizeScanForOutput(scan)), + reportJson([finding], scan), + reportSarif([finding], scan), rendered.join("\n"), JSON.stringify(sanitizeMessagesForProvider([{ role: "user", content: value }])), ]; @@ -165,6 +179,7 @@ describe("scanner-to-boundary credential invariant", () => { llm_explanation: value, llm_fix: value, }); + updateScanStatus(scan.id, ScanStatus.Failed, undefined, value); const rawCreated = JSON.stringify(db.prepare( "SELECT rule_id, file, message, code_snippet, suppressed_reason, llm_explanation, llm_fix FROM findings WHERE id = ?", @@ -172,8 +187,13 @@ describe("scanner-to-boundary credential invariant", () => { const rawProject = JSON.stringify(db.prepare( "SELECT name, path FROM projects WHERE id = ?", ).get(project.id)); + const rawScan = JSON.stringify(db.prepare( + "SELECT error FROM scans WHERE id = ?", + ).get(scan.id)); expect(rawCreated, value).not.toContain(value); expect(rawProject, value).not.toContain(value); + expect(rawScan, value).not.toContain(value); + expect(JSON.stringify(getScan(scan.id)), value).not.toContain(value); expect(JSON.stringify(getFinding(created.id)), value).not.toContain(value); const legacyId = `legacy-${index}`; @@ -190,15 +210,15 @@ describe("scanner-to-boundary credential invariant", () => { value, value, value, - `legacy-fingerprint-${index}`, value, value, value, - "2026-07-15T00:00:00.000Z", + value, + value, ); expect(JSON.stringify(getFinding(legacyId)), value).not.toContain(value); expect(JSON.stringify(db.prepare( - "SELECT file, message, code_snippet, suppressed_reason, llm_explanation, llm_fix FROM findings WHERE id = ?", + "SELECT file, message, code_snippet, fingerprint, suppressed_reason, llm_explanation, llm_fix, created_at FROM findings WHERE id = ?", ).get(legacyId)), value).not.toContain(value); } }); @@ -220,4 +240,58 @@ describe("scanner-to-boundary credential invariant", () => { expect(sanitizeLocationForOutput(value), value).toBe(value); } }); + + test("high-entropy hexadecimal recognition has a reachable normalized boundary", () => { + const balancedHex = "0123456789abcdef".repeat(8); + expect(shannonEntropy(balancedHex)).toBeCloseTo(4, 10); + expect( + recognizeCredentialText(balancedHex).some(({ rule }) => rule.id === "high-entropy-hex"), + ).toBe(true); + expect(containsCredentialLikeText(balancedHex)).toBe(true); + expect(sanitizeTextForBoundary(balancedHex)).not.toContain(balancedHex); + }); + + test("high-entropy hexadecimal recognition rejects short and structured low-entropy controls", () => { + const safeHexValues = [ + "0123456789abcdef", + "a".repeat(128), + "deadbeef".repeat(16), + "00112233".repeat(16), + ]; + for (const value of safeHexValues) { + expect( + recognizeCredentialText(value).some(({ rule }) => rule.id === "high-entropy-hex"), + value.length.toString(), + ).toBe(false); + } + }); + + test("only exempts 40-character hex in pinned GitHub Action syntax", () => { + const revision = "0123456789abcdef".repeat(3).slice(0, 40); + expect( + recognizeCredentialText(revision).some(({ rule }) => rule.id === "high-entropy-hex"), + ).toBe(true); + const actionPin = `- uses: synthetic/action@${revision}`; + expect(scanFile(".github/workflows/ci.yml", actionPin)).toEqual([]); + expect(recognizeCredentialText(actionPin)).toEqual([]); + expect(sanitizeTextForBoundary(actionPin, 12_000)).toBe(actionPin); + }); + + test("high-entropy hexadecimal property corpus keeps positives and false positives separated", () => { + const alphabet = "0123456789abcdef"; + for (let offset = 0; offset < alphabet.length; offset++) { + const rotated = `${alphabet.slice(offset)}${alphabet.slice(0, offset)}`.repeat(4); + expect( + recognizeCredentialText(rotated).some(({ rule }) => rule.id === "high-entropy-hex"), + ).toBe(true); + } + for (let index = 0; index < 256; index++) { + const left = (index % 16).toString(16); + const right = ((index + 1) % 16).toString(16); + const structured = `${left.repeat(32)}${right.repeat(32)}`; + expect( + recognizeCredentialText(structured).some(({ rule }) => rule.id === "high-entropy-hex"), + ).toBe(false); + } + }); }); diff --git a/src/lib/credential-recognition.ts b/src/lib/credential-recognition.ts index 5604c30..161507e 100644 --- a/src/lib/credential-recognition.ts +++ b/src/lib/credential-recognition.ts @@ -151,7 +151,9 @@ const ENV_API_KEY_DEFINITION: CredentialPatternDefinition = { const HIGH_ENTROPY_HEX_DEFINITION: CredentialPatternDefinition = { id: "high-entropy-hex", name: "High-entropy hex string", - source: String.raw`\b[0-9a-fA-F]{16,}\b`, + // Thirty-two characters avoids treating short hexadecimal identifiers as + // credentials while still covering 128-bit and larger secret material. + source: String.raw`\b[0-9a-fA-F]{32,}\b`, flags: "g", severity: Severity.Medium, }; @@ -205,16 +207,45 @@ function collectPatternMatches( return recognitions; } +function isPinnedGitHubActionRevision( + value: string, + index: number, + match: string, +): boolean { + if (match.length !== 40) return false; + const before = value.slice(0, index).trim(); + const after = value.slice(index + match.length); + return /^-?\s*uses:\s*["']?[A-Za-z0-9_.-]+\/[A-Za-z0-9_./-]+@$/.test(before) + && /^["']?\s*(?:#.*)?$/.test(after); +} + function collectEntropyMatches(value: string): CredentialRecognition[] { const recognitions: CredentialRecognition[] = []; - for (const [definition, threshold] of [ - [HIGH_ENTROPY_HEX_DEFINITION, 4.5], - [HIGH_ENTROPY_BASE64_DEFINITION, 5.0], + // Normalize against each alphabet's theoretical maximum. Hex tops out at + // exactly 4 bits/character, so a raw threshold above 4 is unreachable. + for (const { definition, alphabetSize, normalizedThreshold } of [ + { + definition: HIGH_ENTROPY_HEX_DEFINITION, + alphabetSize: 16, + normalizedThreshold: 0.875, // 3.5 / log2(16) + }, + { + definition: HIGH_ENTROPY_BASE64_DEFINITION, + alphabetSize: 65, + normalizedThreshold: 5.0 / Math.log2(65), + }, ] as const) { const rule = materialize(definition); let match: RegExpExecArray | null; while ((match = rule.pattern.exec(value)) !== null) { - if (shannonEntropy(match[0]) > threshold) { + if ( + definition.id === HIGH_ENTROPY_HEX_DEFINITION.id + && isPinnedGitHubActionRevision(value, match.index, match[0]) + ) { + continue; + } + const normalizedEntropy = shannonEntropy(match[0]) / Math.log2(alphabetSize); + if (normalizedEntropy > normalizedThreshold) { recognitions.push({ index: match.index, length: match[0].length, rule }); } if (match[0].length === 0) rule.pattern.lastIndex++; diff --git a/src/lib/finding-safety.test.ts b/src/lib/finding-safety.test.ts index 52c64e6..d6f57c1 100644 --- a/src/lib/finding-safety.test.ts +++ b/src/lib/finding-safety.test.ts @@ -1,9 +1,10 @@ import { describe, expect, test } from "bun:test"; -import { ScannerType, Severity, type FindingInput } from "../types/index.js"; +import { ScanStatus, ScannerType, Severity, type Finding, type FindingInput, type Scan } from "../types/index.js"; import { isCredentialFinding, sanitizeFindingForOutput, sanitizeFindingForPersistence, + sanitizeScanForOutput, sanitizeTextForBoundary, } from "./finding-safety.js"; @@ -82,4 +83,35 @@ describe("finding safety", () => { expect(sanitized).not.toContain(syntheticSecret); expect(sanitized).toContain("[REDACTED]"); }); + + test("sanitizes every exported finding and scan string recursively", () => { + const syntheticCredential = `gh${"o"}_${"G_".repeat(18)}`; + const unsafeFinding = { + ...finding(), + id: syntheticCredential, + scan_id: syntheticCredential, + fingerprint: syntheticCredential, + suppressed: false, + suppressed_reason: syntheticCredential, + llm_explanation: syntheticCredential, + llm_fix: syntheticCredential, + llm_exploitability: null, + created_at: syntheticCredential, + } as Finding; + const unsafeScan: Scan = { + id: syntheticCredential, + project_id: syntheticCredential, + status: syntheticCredential as ScanStatus, + scanner_types: [syntheticCredential as ScannerType], + findings_count: 1, + started_at: syntheticCredential, + completed_at: syntheticCredential, + duration_ms: 1, + error: syntheticCredential, + created_at: syntheticCredential, + }; + + expect(JSON.stringify(sanitizeFindingForOutput(unsafeFinding))).not.toContain(syntheticCredential); + expect(JSON.stringify(sanitizeScanForOutput(unsafeScan))).not.toContain(syntheticCredential); + }); }); diff --git a/src/lib/finding-safety.ts b/src/lib/finding-safety.ts index 0df1a9d..00cd99d 100644 --- a/src/lib/finding-safety.ts +++ b/src/lib/finding-safety.ts @@ -1,5 +1,5 @@ import { createHash } from "crypto"; -import { ScannerType, type Finding, type FindingInput } from "../types/index.js"; +import { ScannerType, type Finding, type FindingInput, type Scan } from "../types/index.js"; import { containsRecognizedCredential } from "./credential-recognition.js"; export const REDACTED_FINDING_TEXT = "[REDACTED]"; @@ -7,6 +7,7 @@ export const REDACTED_FINDING_TEXT = "[REDACTED]"; const MAX_LOCATION_LENGTH = 512; const MAX_MESSAGE_LENGTH = 512; const MAX_RULE_ID_LENGTH = 128; +const MAX_IDENTIFIER_LENGTH = 256; type FindingLike = FindingInput | Finding; @@ -21,6 +22,17 @@ function stableRedaction(value: string, kind: string): string { return `[REDACTED-${kind}:${correlation}]`; } +/** Preserve correlation without retaining a credential-bearing identifier. */ +export function sanitizeIdentifierForOutput(value: string, kind = "ID"): string { + return containsCredentialLikeText(value) + ? stableRedaction(value, kind.replace(/[^A-Z0-9_-]/gi, "-").toUpperCase()) + : boundedSingleLine(value, MAX_IDENTIFIER_LENGTH); +} + +export function sanitizeFingerprintForOutput(value: string): string { + return sanitizeIdentifierForOutput(value, "FINGERPRINT"); +} + export function containsCredentialLikeText(value: string | null | undefined): boolean { return containsRecognizedCredential(value); } @@ -77,12 +89,24 @@ function sanitizeFinding(finding: T): T { const ruleId = sanitizeRuleIdForOutput(finding.rule_id); const result = { ...finding, + ...("id" in finding ? { id: sanitizeIdentifierForOutput(finding.id, "ID") } : {}), + ...("scan_id" in finding + ? { scan_id: sanitizeIdentifierForOutput(finding.scan_id, "SCAN-ID") } + : {}), rule_id: ruleId, + scanner_type: sanitizeTextForBoundary(String(finding.scanner_type), 128), + severity: sanitizeTextForBoundary(String(finding.severity), 128), file: sanitizeLocationForOutput(finding.file), message: sensitive ? `Potential credential exposure detected (${ruleId})` : sanitizeTextForBoundary(finding.message, MAX_MESSAGE_LENGTH), ...(finding.code_snippet != null ? { code_snippet: REDACTED_FINDING_TEXT } : {}), + ...("fingerprint" in finding + ? { fingerprint: sanitizeFingerprintForOutput(finding.fingerprint) } + : {}), + ...("created_at" in finding + ? { created_at: sanitizeTextForBoundary(finding.created_at, 128) } + : {}), } as T; if ("llm_explanation" in result && result.llm_explanation != null) { @@ -100,7 +124,9 @@ function sanitizeFinding(finding: T): T { ? REDACTED_FINDING_TEXT : sanitizeTextForBoundary(result.suppressed_reason, MAX_MESSAGE_LENGTH); } - return result; + // This final recursive pass is intentional: a newly added string field must + // be safe by default until it receives a more specific correlation policy. + return sanitizeValueForBoundary(result); } export function sanitizeFindingForPersistence(finding: T): T { @@ -110,3 +136,21 @@ export function sanitizeFindingForPersistence(finding: T) export function sanitizeFindingForOutput(finding: T): T { return sanitizeFinding(finding); } + +export function sanitizeScanForOutput(scan: Scan): Scan { + const safe = { + ...scan, + id: sanitizeIdentifierForOutput(scan.id, "SCAN-ID"), + project_id: sanitizeIdentifierForOutput(scan.project_id, "PROJECT-ID"), + status: sanitizeTextForBoundary(String(scan.status), 128), + scanner_types: scan.scanner_types.map((scannerType) => + sanitizeTextForBoundary(String(scannerType), 128)), + started_at: sanitizeTextForBoundary(scan.started_at, 128), + completed_at: scan.completed_at == null + ? null + : sanitizeTextForBoundary(scan.completed_at, 128), + error: scan.error == null ? null : sanitizeTextForBoundary(scan.error, MAX_MESSAGE_LENGTH), + created_at: sanitizeTextForBoundary(scan.created_at, 128), + } as Scan; + return sanitizeValueForBoundary(safe); +} diff --git a/src/mcp/tools/output-safety.test.ts b/src/mcp/tools/output-safety.test.ts index ab1f05d..f8c723d 100644 --- a/src/mcp/tools/output-safety.test.ts +++ b/src/mcp/tools/output-safety.test.ts @@ -5,11 +5,12 @@ import { join } from "path"; import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { createFinding } from "../../db/findings.js"; import { createProject } from "../../db/projects.js"; -import { createScan } from "../../db/scans.js"; +import { createScan, updateScanStatus } from "../../db/scans.js"; import { getCurrentTestDb, setupTestDb } from "../../db/test-helpers.js"; -import { ScannerType, Severity } from "../../types/index.js"; +import { ScanStatus, ScannerType, Severity } from "../../types/index.js"; import { registerFindingTools } from "./findings.js"; import { registerScanTools } from "./scan.js"; +import { registerRulesPoliciesTools } from "./rules-policies.js"; type ToolHandler = (args: Record) => Promise; @@ -115,4 +116,19 @@ describe("MCP credential output safety", () => { rmSync(dir, { recursive: true, force: true }); } }); + + test("scan history tools never expose scanner-recognized error metadata", async () => { + const syntheticCredential = `gh${"s"}_${"H_".repeat(18)}`; + const project = createProject("mcp-scan-error", "/tmp/mcp-scan-error"); + const scan = createScan(project.id, [ScannerType.Code]); + updateScanStatus(scan.id, ScanStatus.Failed, undefined, syntheticCredential); + const tools = captureTools((server) => registerRulesPoliciesTools(server, jsonResult)); + + const listed = await tools.get("list_scans")?.({ limit: 50 }); + const fetched = await tools.get("get_scan")?.({ id: scan.id }); + for (const output of [JSON.stringify(listed), JSON.stringify(fetched)]) { + expect(output).not.toContain(syntheticCredential); + expect(output).toContain("REDACTED"); + } + }); }); diff --git a/src/reporters/json.test.ts b/src/reporters/json.test.ts index a83aead..ae33e27 100644 --- a/src/reporters/json.test.ts +++ b/src/reporters/json.test.ts @@ -153,4 +153,59 @@ describe("JSON reporter", () => { expect(output).toContain("REDACTED-RULE"); expect(output).toContain("REDACTED-LOCATION"); }); + + test("sanitizes every string-bearing finding and scan field", () => { + const syntheticCredential = `gh${"s"}_${"C_".repeat(18)}`; + const finding = makeFinding({ + id: syntheticCredential, + scan_id: syntheticCredential, + rule_id: syntheticCredential, + scanner_type: syntheticCredential as ScannerType, + severity: syntheticCredential as Severity, + file: syntheticCredential, + message: syntheticCredential, + code_snippet: syntheticCredential, + fingerprint: syntheticCredential, + suppressed_reason: syntheticCredential, + llm_explanation: syntheticCredential, + llm_fix: syntheticCredential, + created_at: syntheticCredential, + }); + const scan: Scan = { + ...mockScan, + id: syntheticCredential, + project_id: syntheticCredential, + status: syntheticCredential as ScanStatus, + scanner_types: [syntheticCredential as ScannerType], + started_at: syntheticCredential, + completed_at: syntheticCredential, + error: syntheticCredential, + created_at: syntheticCredential, + }; + + const output = reportFindings([finding], scan); + expect(output).not.toContain(syntheticCredential); + expect(output).toContain("REDACTED"); + }); + + test("uses stable opaque correlations for credential-bearing identifiers", () => { + const firstCredential = `gh${"r"}_${"D_".repeat(18)}`; + const secondCredential = `gh${"r"}_${"E_".repeat(18)}`; + const first = JSON.parse(reportFindings([ + makeFinding({ id: firstCredential, fingerprint: firstCredential }), + ])).findings[0]; + const repeated = JSON.parse(reportFindings([ + makeFinding({ id: firstCredential, fingerprint: firstCredential }), + ])).findings[0]; + const distinct = JSON.parse(reportFindings([ + makeFinding({ id: secondCredential, fingerprint: secondCredential }), + ])).findings[0]; + + expect(first.id).toBe(repeated.id); + expect(first.fingerprint).toBe(repeated.fingerprint); + expect(first.id).not.toBe(distinct.id); + expect(first.fingerprint).not.toBe(distinct.fingerprint); + expect(first.id).toMatch(/^\[REDACTED-ID:[a-f0-9]{12}\]$/); + expect(first.fingerprint).toMatch(/^\[REDACTED-FINGERPRINT:[a-f0-9]{12}\]$/); + }); }); diff --git a/src/reporters/json.ts b/src/reporters/json.ts index dc9bff2..ef205fe 100644 --- a/src/reporters/json.ts +++ b/src/reporters/json.ts @@ -1,6 +1,10 @@ import type { Finding, Scan, SecurityScore } from "../types/index.js"; import { Severity } from "../types/index.js"; -import { sanitizeFindingForOutput } from "../lib/finding-safety.js"; +import { + sanitizeFindingForOutput, + sanitizeScanForOutput, + sanitizeValueForBoundary, +} from "../lib/finding-safety.js"; function computeScore(findings: Finding[]): SecurityScore { const active = findings.filter((f) => !f.suppressed); @@ -52,9 +56,9 @@ export function reportFindings(findings: Finding[], scan?: Scan): string { const safeFindings = findings.map(sanitizeFindingForOutput); const summary = computeScore(safeFindings); const report = { - scan: scan ?? null, + scan: scan ? sanitizeScanForOutput(scan) : null, findings: safeFindings, summary, }; - return JSON.stringify(report, null, 2); + return JSON.stringify(sanitizeValueForBoundary(report), null, 2); } diff --git a/src/reporters/sarif.test.ts b/src/reporters/sarif.test.ts index ddbe265..5e86845 100644 --- a/src/reporters/sarif.test.ts +++ b/src/reporters/sarif.test.ts @@ -172,4 +172,28 @@ describe("SARIF reporter", () => { expect(output).not.toContain(syntheticSecret); expect(JSON.parse(output).runs[0].results[0].message.text).toContain("Potential credential exposure"); }); + + test("sanitizes credential-bearing fingerprints and invocation metadata", () => { + const syntheticCredential = `gh${"p"}_${"F_".repeat(18)}`; + const scan: Scan = { + ...mockScan, + status: syntheticCredential as ScanStatus, + started_at: syntheticCredential, + completed_at: syntheticCredential, + error: syntheticCredential, + }; + const output = reportFindings([ + makeFinding({ + id: syntheticCredential, + scan_id: syntheticCredential, + fingerprint: syntheticCredential, + created_at: syntheticCredential, + }), + ], scan); + + expect(output).not.toContain(syntheticCredential); + const fingerprint = JSON.parse(output).runs[0].results[0] + .fingerprints["security/fingerprint"]; + expect(fingerprint).toMatch(/^\[REDACTED-FINGERPRINT:[a-f0-9]{12}\]$/); + }); }); diff --git a/src/reporters/sarif.ts b/src/reporters/sarif.ts index 09eb2b1..fe2a604 100644 --- a/src/reporters/sarif.ts +++ b/src/reporters/sarif.ts @@ -1,6 +1,10 @@ import type { Finding, Scan } from "../types/index.js"; import { Severity } from "../types/index.js"; -import { sanitizeFindingForOutput } from "../lib/finding-safety.js"; +import { + sanitizeFindingForOutput, + sanitizeScanForOutput, + sanitizeValueForBoundary, +} from "../lib/finding-safety.js"; const SEVERITY_TO_LEVEL: Record = { [Severity.Critical]: "error", @@ -34,6 +38,7 @@ interface SarifResult { } export function reportFindings(findings: Finding[], scan?: Scan): string { + const safeScan = scan ? sanitizeScanForOutput(scan) : undefined; const rulesMap = new Map(); const results: SarifResult[] = []; @@ -88,12 +93,12 @@ export function reportFindings(findings: Finding[], scan?: Scan): string { }, }, results, - ...(scan && { + ...(safeScan && { invocations: [ { - executionSuccessful: scan.status === "completed", - startTimeUtc: scan.started_at, - endTimeUtc: scan.completed_at ?? undefined, + executionSuccessful: safeScan.status === "completed", + startTimeUtc: safeScan.started_at, + endTimeUtc: safeScan.completed_at ?? undefined, }, ], }), @@ -101,5 +106,5 @@ export function reportFindings(findings: Finding[], scan?: Scan): string { ], }; - return JSON.stringify(sarif, null, 2); + return JSON.stringify(sanitizeValueForBoundary(sarif), null, 2); } From 6a5516c13964dc2f2166a44f7d55d79c2de265f9 Mon Sep 17 00:00:00 2001 From: Andrei Hasna Date: Thu, 16 Jul 2026 01:35:24 +0300 Subject: [PATCH 5/8] fix: enforce durable credential scrub boundaries --- src/db/database.ts | 2 + src/db/findings.ts | 79 +++---- src/db/legacy-credential-scrub.test.ts | 181 ++++++++++++++++ src/db/legacy-credential-scrub.ts | 288 +++++++++++++++++++++++++ src/db/projects.ts | 52 +++-- src/db/scans.ts | 60 ++++-- src/lib/credential-invariant.test.ts | 134 +++++++++++- src/lib/credential-recognition.ts | 109 ++++++++-- src/lib/finding-safety.ts | 20 ++ src/scanners/secrets.test.ts | 16 ++ src/scanners/secrets.ts | 25 ++- 11 files changed, 856 insertions(+), 110 deletions(-) create mode 100644 src/db/legacy-credential-scrub.test.ts create mode 100644 src/db/legacy-credential-scrub.ts diff --git a/src/db/database.ts b/src/db/database.ts index 2477ca3..e4879cc 100644 --- a/src/db/database.ts +++ b/src/db/database.ts @@ -2,6 +2,7 @@ import { Database } from "bun:sqlite"; import { copyFileSync, existsSync, mkdirSync } from "fs"; import { dirname, join } from "path"; import { homedir } from "os"; +import { scrubLegacyCredentialRows } from "./legacy-credential-scrub.js"; let _db: Database | null = null; @@ -73,6 +74,7 @@ export function getDb(): Database { _db.exec("PRAGMA foreign_keys = ON"); _db.exec("PRAGMA busy_timeout = 5000"); runMigrations(_db); + scrubLegacyCredentialRows(_db); if (!_initialized) { _initialized = true; for (const cb of _postInitCallbacks) cb(); diff --git a/src/db/findings.ts b/src/db/findings.ts index bc18bd8..786091c 100644 --- a/src/db/findings.ts +++ b/src/db/findings.ts @@ -10,6 +10,11 @@ import { sanitizeFindingForPersistence, sanitizeTextForBoundary, } from "../lib/finding-safety.js"; +import { + legacyRowContainsCredential, + scrubLegacyCredentialRows, +} from "./legacy-credential-scrub.js"; +import { getScan } from "./scans.js"; interface FindingRow { id: string; @@ -33,58 +38,12 @@ interface FindingRow { } function rowToFinding(row: FindingRow): Finding { - const safe = sanitizeFindingForOutput({ + return sanitizeFindingForOutput({ ...row, scanner_type: row.scanner_type as ScannerType, severity: row.severity as Severity, suppressed: row.suppressed === 1, }); - // Opportunistically scrub legacy rows so direct database consumers after - // first read cannot recover fields written by older unsafe versions. - if ( - safe.rule_id !== row.rule_id || - safe.fingerprint !== row.fingerprint || - safe.created_at !== row.created_at || - safe.file !== row.file || - safe.message !== row.message || - safe.code_snippet !== row.code_snippet || - safe.suppressed_reason !== row.suppressed_reason || - safe.llm_explanation !== row.llm_explanation || - safe.llm_fix !== row.llm_fix - ) { - try { - const db = getDb(); - if (safe.rule_id !== row.rule_id) { - db.prepare( - `INSERT OR IGNORE INTO rules (id, name, description, scanner_type, severity, enabled, builtin, metadata, created_at, updated_at) - VALUES (?, 'Redacted legacy finding rule', 'Credential-bearing legacy rule identifier was replaced', ?, ?, 1, 0, '{}', datetime('now'), datetime('now'))`, - ).run(safe.rule_id, safe.scanner_type, safe.severity); - } - db.prepare( - `UPDATE findings SET rule_id = ?, file = ?, message = ?, code_snippet = ?, fingerprint = ?, suppressed_reason = ?, llm_explanation = ?, llm_fix = ?, created_at = ? WHERE id = ?`, - ).run( - safe.rule_id, - safe.file, - safe.message, - safe.code_snippet, - safe.fingerprint, - safe.suppressed_reason, - safe.llm_explanation, - safe.llm_fix, - safe.created_at, - row.id, - ); - if (safe.rule_id !== row.rule_id) { - db.prepare( - `DELETE FROM rules WHERE id = ? AND NOT EXISTS (SELECT 1 FROM findings WHERE rule_id = ?)`, - ).run(row.rule_id, row.rule_id); - } - } catch { - // Output remains sanitized even when a legacy/read-only database cannot - // be rewritten in place. - } - } - return safe; } function generateFingerprint(rule_id: string, file: string, line: number, message: string): string { @@ -96,6 +55,8 @@ function generateFingerprint(rule_id: string, file: string, line: number, messag export function createFinding(scan_id: string, input: FindingInput): Finding { const db = getDb(); + const scan = getScan(scan_id); + const targetScanId = scan?.id ?? scan_id; const safeInput = sanitizeFindingForPersistence(input); if (safeInput.rule_id !== input.rule_id) { db.prepare( @@ -113,7 +74,7 @@ export function createFinding(scan_id: string, input: FindingInput): Finding { ); stmt.run( id, - scan_id, + targetScanId, safeInput.rule_id, safeInput.scanner_type, safeInput.severity, @@ -129,7 +90,7 @@ export function createFinding(scan_id: string, input: FindingInput): Finding { return { id, - scan_id, + scan_id: targetScanId, rule_id: safeInput.rule_id, scanner_type: safeInput.scanner_type, severity: safeInput.severity, @@ -152,7 +113,11 @@ export function createFinding(scan_id: string, input: FindingInput): Finding { export function getFinding(id: string): Finding | null { const db = getDb(); const stmt = db.prepare(`SELECT * FROM findings WHERE id = ?`); - const row = stmt.get(id) as FindingRow | undefined; + let row = stmt.get(id) as FindingRow | undefined; + if (row && legacyRowContainsCredential(row as unknown as Record)) { + const result = scrubLegacyCredentialRows(db); + row = stmt.get(result.findingIds.get(id) ?? id) as FindingRow | undefined; + } return row ? rowToFinding(row) : null; } @@ -201,7 +166,16 @@ export function listFindings(options: ListFindingsOptions = {}): Finding[] { ); params.push(limit, offset); - return (stmt.all(...(params as any[])) as FindingRow[]).map(rowToFinding); + let rows = stmt.all(...(params as any[])) as FindingRow[]; + if (rows.some((row) => legacyRowContainsCredential(row as unknown as Record))) { + const result = scrubLegacyCredentialRows(db); + rows = rows.flatMap((row) => { + const migrated = db.prepare("SELECT * FROM findings WHERE id = ?") + .get(result.findingIds.get(row.id) ?? row.id) as FindingRow | undefined; + return migrated ? [migrated] : []; + }); + } + return rows.map(rowToFinding); } export function updateFinding( @@ -210,6 +184,7 @@ export function updateFinding( ): void { const db = getDb(); const existing = getFinding(id); + const targetId = existing?.id ?? id; const sensitive = existing ? isCredentialFinding(existing) : false; const sets: string[] = []; const params: unknown[] = []; @@ -255,7 +230,7 @@ export function updateFinding( if (sets.length === 0) return; - params.push(id); + params.push(targetId); const stmt = db.prepare(`UPDATE findings SET ${sets.join(", ")} WHERE id = ?`); stmt.run(...(params as any[])); } diff --git a/src/db/legacy-credential-scrub.test.ts b/src/db/legacy-credential-scrub.test.ts new file mode 100644 index 0000000..8eebbcb --- /dev/null +++ b/src/db/legacy-credential-scrub.test.ts @@ -0,0 +1,181 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { Database } from "bun:sqlite"; +import { mkdtempSync, rmSync } from "fs"; +import { tmpdir } from "os"; +import { join } from "path"; +import { scrubLegacyCredentialRows } from "./legacy-credential-scrub.js"; +import { opaqueIdentifierForStorage } from "../lib/finding-safety.js"; + +const tempDirs: string[] = []; + +afterEach(() => { + for (const directory of tempDirs.splice(0)) rmSync(directory, { recursive: true, force: true }); +}); + +function createFixtureDb(): { db: Database; marker: string; path: string } { + const directory = mkdtempSync(join(tmpdir(), "shield-legacy-scrub-")); + tempDirs.push(directory); + const path = join(directory, "shield.db"); + const db = new Database(path); + db.exec("PRAGMA journal_mode = WAL; PRAGMA foreign_keys = ON; PRAGMA busy_timeout = 5000"); + db.exec(` + CREATE TABLE projects ( + id TEXT PRIMARY KEY, name TEXT NOT NULL, path TEXT NOT NULL, + created_at TEXT NOT NULL, updated_at TEXT NOT NULL + ); + CREATE TABLE scans ( + id TEXT PRIMARY KEY, project_id TEXT NOT NULL REFERENCES projects(id) ON DELETE CASCADE, + status TEXT NOT NULL, scanner_types TEXT NOT NULL, findings_count INTEGER NOT NULL, + started_at TEXT NOT NULL, completed_at TEXT, duration_ms INTEGER, error TEXT, created_at TEXT NOT NULL + ); + CREATE TABLE rules ( + id TEXT PRIMARY KEY, name TEXT NOT NULL, description TEXT NOT NULL, scanner_type TEXT NOT NULL, + severity TEXT NOT NULL, pattern TEXT, enabled INTEGER NOT NULL, builtin INTEGER NOT NULL, + metadata TEXT NOT NULL, created_at TEXT NOT NULL, updated_at TEXT NOT NULL + ); + CREATE TABLE findings ( + id TEXT PRIMARY KEY, scan_id TEXT NOT NULL REFERENCES scans(id) ON DELETE CASCADE, + rule_id TEXT NOT NULL REFERENCES rules(id), scanner_type TEXT NOT NULL, severity TEXT NOT NULL, + file TEXT NOT NULL, line INTEGER NOT NULL, "column" INTEGER, end_line INTEGER, message TEXT NOT NULL, + code_snippet TEXT, fingerprint TEXT NOT NULL, suppressed INTEGER NOT NULL, suppressed_reason TEXT, + llm_explanation TEXT, llm_fix TEXT, llm_exploitability REAL, created_at TEXT NOT NULL + ); + CREATE TABLE baselines (id TEXT PRIMARY KEY, finding_fingerprint TEXT NOT NULL); + CREATE TABLE llm_cache (id TEXT PRIMARY KEY, finding_fingerprint TEXT NOT NULL); + `); + const marker = `gh${"o"}_${"Rollback_A4_".repeat(4)}`; + db.prepare("INSERT INTO projects VALUES (?, ?, ?, ?, ?)").run(marker, marker, marker, marker, marker); + db.prepare("INSERT INTO scans VALUES (?, ?, ?, ?, 1, ?, ?, 1, ?, ?)") + .run(marker, marker, marker, JSON.stringify([marker]), marker, marker, marker, marker); + db.prepare("INSERT INTO rules VALUES (?, ?, ?, ?, ?, ?, 1, 0, ?, ?, ?)") + .run(marker, marker, marker, marker, marker, marker, JSON.stringify({ marker }), marker, marker); + db.prepare( + `INSERT INTO findings VALUES + (?, ?, ?, ?, ?, ?, 1, NULL, NULL, ?, ?, ?, 1, ?, ?, ?, 0.5, ?)`, + ).run( + marker, + marker, + marker, + marker, + marker, + marker, + marker, + marker, + marker, + marker, + marker, + marker, + marker, + ); + db.prepare("INSERT INTO baselines VALUES ('baseline', ?)").run(marker); + db.prepare("INSERT INTO llm_cache VALUES ('cache', ?)").run(marker); + return { db, marker, path }; +} + +function rawDatabase(db: Database): string { + return JSON.stringify(["projects", "scans", "rules", "findings", "baselines", "llm_cache"] + .map((table) => db.prepare(`SELECT * FROM ${table}`).all())); +} + +describe("legacy credential graph scrub", () => { + test("serializes simultaneous scrub attempts from separate processes", async () => { + const { db, marker, path } = createFixtureDb(); + const moduleUrl = new URL("./legacy-credential-scrub.ts", import.meta.url).href; + const program = ` + import { Database } from "bun:sqlite"; + import { scrubLegacyCredentialRows } from ${JSON.stringify(moduleUrl)}; + const db = new Database(${JSON.stringify(path)}); + db.exec("PRAGMA foreign_keys = ON; PRAGMA busy_timeout = 5000"); + scrubLegacyCredentialRows(db); + db.close(); + `; + const spawn = () => Bun.spawn({ + cmd: [process.execPath, "-e", program], + env: { PATH: process.env.PATH ?? "" }, + stderr: "ignore", + stdout: "ignore", + }); + try { + const first = spawn(); + const second = spawn(); + expect(await first.exited).toBe(0); + expect(await second.exited).toBe(0); + expect(rawDatabase(db)).not.toContain(marker); + expect(db.prepare("PRAGMA foreign_key_check").all()).toEqual([]); + } finally { + db.close(); + } + }); + + test("is idempotent across independent SQLite handles", () => { + const { db, marker, path } = createFixtureDb(); + const second = new Database(path); + second.exec("PRAGMA foreign_keys = ON; PRAGMA busy_timeout = 5000"); + try { + const firstResult = scrubLegacyCredentialRows(db); + const secondResult = scrubLegacyCredentialRows(second); + expect(firstResult.scanIds.get(marker)).toBeDefined(); + expect(secondResult.scanIds.size).toBe(0); + expect(rawDatabase(db)).not.toContain(marker); + expect(rawDatabase(second)).not.toContain(marker); + expect(db.prepare("PRAGMA foreign_key_check").all()).toEqual([]); + expect(second.prepare("PRAGMA foreign_key_check").all()).toEqual([]); + } finally { + second.close(); + db.close(); + } + }); + + test("rolls back every parent and child mutation on a write failure", () => { + const { db, marker } = createFixtureDb(); + try { + db.exec(` + CREATE TRIGGER reject_legacy_finding_update + BEFORE UPDATE ON findings + BEGIN + SELECT RAISE(ABORT, 'synthetic write rejection'); + END; + `); + expect(() => scrubLegacyCredentialRows(db)).toThrow( + "Unable to durably sanitize legacy credential data", + ); + expect(rawDatabase(db)).toContain(marker); + expect((db.prepare("SELECT COUNT(*) AS count FROM projects").get() as { count: number }).count).toBe(1); + expect((db.prepare("SELECT COUNT(*) AS count FROM scans").get() as { count: number }).count).toBe(1); + expect((db.prepare("SELECT COUNT(*) AS count FROM rules").get() as { count: number }).count).toBe(1); + expect((db.prepare("SELECT COUNT(*) AS count FROM findings").get() as { count: number }).count).toBe(1); + expect(db.prepare("PRAGMA foreign_key_check").all()).toEqual([]); + + db.exec("DROP TRIGGER reject_legacy_finding_update"); + scrubLegacyCredentialRows(db); + expect(rawDatabase(db)).not.toContain(marker); + expect(db.prepare("PRAGMA foreign_key_check").all()).toEqual([]); + } finally { + db.close(); + } + }); + + test("keeps collision candidates distinct and stable", () => { + const { db, marker } = createFixtureDb(); + try { + const occupiedScanId = opaqueIdentifierForStorage(marker, "SCAN-ID"); + db.prepare("INSERT INTO projects VALUES ('collision-project', 'safe', '/safe', 'now', 'now')").run(); + db.prepare("INSERT INTO scans VALUES (?, 'collision-project', 'completed', '[]', 0, 'now', NULL, 1, NULL, 'now')") + .run(occupiedScanId); + const result = scrubLegacyCredentialRows(db); + const ids = [ + result.projectIds.get(marker), + result.scanIds.get(marker), + result.ruleIds.get(marker), + result.findingIds.get(marker), + ]; + expect(new Set(ids).size).toBe(4); + expect(ids.every((id) => id?.startsWith("[REDACTED-"))).toBe(true); + expect(result.scanIds.get(marker)).not.toBe(occupiedScanId); + expect((db.prepare("SELECT COUNT(*) AS count FROM scans WHERE id = ?").get(occupiedScanId) as { count: number }).count) + .toBe(1); + } finally { + db.close(); + } + }); +}); diff --git a/src/db/legacy-credential-scrub.ts b/src/db/legacy-credential-scrub.ts new file mode 100644 index 0000000..9996880 --- /dev/null +++ b/src/db/legacy-credential-scrub.ts @@ -0,0 +1,288 @@ +import type { Database } from "bun:sqlite"; +import type { Finding, Scan } from "../types/index.js"; +import { ScanStatus, ScannerType, Severity } from "../types/index.js"; +import { + containsCredentialLikeText, + opaqueIdentifierForStorage, + sanitizeFindingForOutput, + sanitizeLocationForOutput, + sanitizeScanForOutput, + sanitizeTextForBoundary, + sanitizeValueForBoundary, +} from "../lib/finding-safety.js"; + +type RawRow = Record; +type IdTable = "findings" | "projects" | "rules" | "scans"; + +export interface LegacyCredentialScrubResult { + findingIds: Map; + projectIds: Map; + ruleIds: Map; + scanIds: Map; +} + +export function legacyRowContainsCredential(row: RawRow): boolean { + return Object.values(row).some((value) => + typeof value === "string" && containsCredentialLikeText(value)); +} + +function emptyResult(): LegacyCredentialScrubResult { + return { + findingIds: new Map(), + projectIds: new Map(), + ruleIds: new Map(), + scanIds: new Map(), + }; +} + +function parseStringArray(value: string): string[] { + try { + const parsed = JSON.parse(value); + if (Array.isArray(parsed)) return parsed.map(String); + } catch { + // The invalid legacy payload is replaced below rather than surfaced. + } + return [sanitizeTextForBoundary(value, 12_000)]; +} + +function sanitizeJsonText(value: string): string { + try { + return JSON.stringify(sanitizeValueForBoundary(JSON.parse(value))); + } catch { + return JSON.stringify(sanitizeTextForBoundary(value, 12_000)); + } +} + +function buildIdMap( + db: Database, + table: IdTable, + kind: string, + rows: RawRow[], +): Map { + const map = new Map(); + const reserved = new Set( + (db.prepare(`SELECT id FROM ${table}`).all() as Array<{ id: string }>).map(({ id }) => id), + ); + for (const row of rows) { + const oldId = String(row.id); + if (!containsCredentialLikeText(oldId)) { + map.set(oldId, oldId); + continue; + } + let attempt = 0; + let candidate = opaqueIdentifierForStorage(oldId, kind, attempt); + while (reserved.has(candidate) && candidate !== oldId) { + candidate = opaqueIdentifierForStorage(oldId, kind, ++attempt); + } + reserved.add(candidate); + map.set(oldId, candidate); + } + return map; +} + +function hasAnyUnsafeRows(db: Database): boolean { + for (const table of ["projects", "scans", "rules", "findings"] as const) { + const rows = db.prepare(`SELECT * FROM ${table}`).all() as RawRow[]; + if (rows.some(legacyRowContainsCredential)) return true; + } + return false; +} + +/** + * Atomically replace every credential-bearing string in the connected + * Project/Scan/Rule/Finding graph. Parent rows are inserted before foreign + * keys move and deleted only after all children point at durable opaque IDs. + */ +export function scrubLegacyCredentialRows(db: Database): LegacyCredentialScrubResult { + if (!hasAnyUnsafeRows(db)) return emptyResult(); + + const transaction = db.transaction((): LegacyCredentialScrubResult => { + const projects = db.prepare("SELECT * FROM projects").all() as RawRow[]; + const scans = db.prepare("SELECT * FROM scans").all() as RawRow[]; + const rules = db.prepare("SELECT * FROM rules").all() as RawRow[]; + const findings = db.prepare("SELECT * FROM findings").all() as RawRow[]; + const projectIds = buildIdMap(db, "projects", "PROJECT-ID", projects); + const scanIds = buildIdMap(db, "scans", "SCAN-ID", scans); + const ruleIds = buildIdMap(db, "rules", "RULE-ID", rules); + const findingIds = buildIdMap(db, "findings", "FINDING-ID", findings); + + for (const row of projects) { + const oldId = String(row.id); + const id = projectIds.get(oldId)!; + const values = [ + id, + sanitizeTextForBoundary(String(row.name), 256), + sanitizeLocationForOutput(String(row.path)), + sanitizeTextForBoundary(String(row.created_at), 128), + sanitizeTextForBoundary(String(row.updated_at), 128), + ]; + if (id === oldId) { + db.prepare( + "UPDATE projects SET name = ?, path = ?, created_at = ?, updated_at = ? WHERE id = ?", + ).run(values[1], values[2], values[3], values[4], oldId); + } else { + db.prepare( + "INSERT INTO projects (id, name, path, created_at, updated_at) VALUES (?, ?, ?, ?, ?)", + ).run(...values); + } + } + + for (const row of scans) { + const oldId = String(row.id); + const id = scanIds.get(oldId)!; + const rawScan: Scan = { + id, + project_id: projectIds.get(String(row.project_id)) ?? String(row.project_id), + status: String(row.status) as ScanStatus, + scanner_types: parseStringArray(String(row.scanner_types)) as ScannerType[], + findings_count: Number(row.findings_count), + started_at: String(row.started_at), + completed_at: row.completed_at == null ? null : String(row.completed_at), + duration_ms: row.duration_ms == null ? null : Number(row.duration_ms), + error: row.error == null ? null : String(row.error), + created_at: String(row.created_at), + }; + const safe = sanitizeScanForOutput(rawScan); + const values = [ + id, + rawScan.project_id, + safe.status, + JSON.stringify(safe.scanner_types), + safe.findings_count, + safe.started_at, + safe.completed_at, + safe.duration_ms, + safe.error, + safe.created_at, + ]; + if (id === oldId) { + db.prepare( + `UPDATE scans SET project_id = ?, status = ?, scanner_types = ?, findings_count = ?, + started_at = ?, completed_at = ?, duration_ms = ?, error = ?, created_at = ? WHERE id = ?`, + ).run(...values.slice(1), oldId); + } else { + db.prepare( + `INSERT INTO scans + (id, project_id, status, scanner_types, findings_count, started_at, completed_at, duration_ms, error, created_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + ).run(...values); + } + } + + for (const row of rules) { + const oldId = String(row.id); + const id = ruleIds.get(oldId)!; + const values = [ + id, + sanitizeTextForBoundary(String(row.name), 256), + sanitizeTextForBoundary(String(row.description), 512), + sanitizeTextForBoundary(String(row.scanner_type), 128), + sanitizeTextForBoundary(String(row.severity), 128), + row.pattern == null ? null : sanitizeTextForBoundary(String(row.pattern), 12_000), + Number(row.enabled), + Number(row.builtin), + sanitizeJsonText(String(row.metadata)), + sanitizeTextForBoundary(String(row.created_at), 128), + sanitizeTextForBoundary(String(row.updated_at), 128), + ]; + if (id === oldId) { + db.prepare( + `UPDATE rules SET name = ?, description = ?, scanner_type = ?, severity = ?, pattern = ?, + enabled = ?, builtin = ?, metadata = ?, created_at = ?, updated_at = ? WHERE id = ?`, + ).run(...values.slice(1), oldId); + } else { + db.prepare( + `INSERT INTO rules + (id, name, description, scanner_type, severity, pattern, enabled, builtin, metadata, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + ).run(...values); + } + } + + const fingerprintChanges = new Map(); + for (const row of findings) { + const oldId = String(row.id); + const id = findingIds.get(oldId)!; + const raw: Finding = { + id, + scan_id: scanIds.get(String(row.scan_id)) ?? String(row.scan_id), + rule_id: ruleIds.get(String(row.rule_id)) ?? String(row.rule_id), + scanner_type: String(row.scanner_type) as ScannerType, + severity: String(row.severity) as Severity, + file: String(row.file), + line: Number(row.line), + column: row.column == null ? null : Number(row.column), + end_line: row.end_line == null ? null : Number(row.end_line), + message: String(row.message), + code_snippet: row.code_snippet == null ? null : String(row.code_snippet), + fingerprint: String(row.fingerprint), + suppressed: Number(row.suppressed) === 1, + suppressed_reason: row.suppressed_reason == null ? null : String(row.suppressed_reason), + llm_explanation: row.llm_explanation == null ? null : String(row.llm_explanation), + llm_fix: row.llm_fix == null ? null : String(row.llm_fix), + llm_exploitability: row.llm_exploitability == null ? null : Number(row.llm_exploitability), + created_at: String(row.created_at), + }; + const safe = sanitizeFindingForOutput(raw); + fingerprintChanges.set(String(row.fingerprint), safe.fingerprint); + db.prepare( + `UPDATE findings SET id = ?, scan_id = ?, rule_id = ?, scanner_type = ?, severity = ?, file = ?, + line = ?, "column" = ?, end_line = ?, message = ?, code_snippet = ?, fingerprint = ?, suppressed = ?, + suppressed_reason = ?, llm_explanation = ?, llm_fix = ?, llm_exploitability = ?, created_at = ? + WHERE id = ?`, + ).run( + id, + raw.scan_id, + raw.rule_id, + safe.scanner_type, + safe.severity, + safe.file, + safe.line, + safe.column, + safe.end_line, + safe.message, + safe.code_snippet, + safe.fingerprint, + safe.suppressed ? 1 : 0, + safe.suppressed_reason, + safe.llm_explanation, + safe.llm_fix, + safe.llm_exploitability, + safe.created_at, + oldId, + ); + } + + for (const [oldFingerprint, safeFingerprint] of fingerprintChanges) { + if (oldFingerprint === safeFingerprint) continue; + db.prepare("UPDATE baselines SET finding_fingerprint = ? WHERE finding_fingerprint = ?") + .run(safeFingerprint, oldFingerprint); + db.prepare("UPDATE llm_cache SET finding_fingerprint = ? WHERE finding_fingerprint = ?") + .run(safeFingerprint, oldFingerprint); + } + + for (const [oldId, id] of scanIds) { + if (oldId !== id) db.prepare("DELETE FROM scans WHERE id = ?").run(oldId); + } + for (const [oldId, id] of ruleIds) { + if (oldId !== id) db.prepare("DELETE FROM rules WHERE id = ?").run(oldId); + } + for (const [oldId, id] of projectIds) { + if (oldId !== id) db.prepare("DELETE FROM projects WHERE id = ?").run(oldId); + } + + if ((db.prepare("PRAGMA foreign_key_check").all() as unknown[]).length > 0) { + throw new Error("legacy credential scrub violated referential integrity"); + } + return { findingIds, projectIds, ruleIds, scanIds }; + }); + + try { + const immediate = transaction as typeof transaction & { immediate?: () => LegacyCredentialScrubResult }; + return typeof immediate.immediate === "function" ? immediate.immediate() : transaction(); + } catch { + // Fail closed and never surface SQLite diagnostics that may repeat a + // credential-bearing legacy identifier. + throw new Error("Unable to durably sanitize legacy credential data"); + } +} diff --git a/src/db/projects.ts b/src/db/projects.ts index 041ecf7..e64c05c 100644 --- a/src/db/projects.ts +++ b/src/db/projects.ts @@ -1,26 +1,25 @@ import crypto from "crypto"; import { getDb } from "./database.js"; import type { Project } from "../types/index.js"; -import { sanitizeLocationForOutput, sanitizeTextForBoundary } from "../lib/finding-safety.js"; +import { + sanitizeIdentifierForOutput, + sanitizeLocationForOutput, + sanitizeTextForBoundary, +} from "../lib/finding-safety.js"; +import { + legacyRowContainsCredential, + scrubLegacyCredentialRows, +} from "./legacy-credential-scrub.js"; function rowToProject(row: Project): Project { const safe = { ...row, + id: sanitizeIdentifierForOutput(row.id, "PROJECT-ID"), name: sanitizeTextForBoundary(row.name, 256), path: sanitizeLocationForOutput(row.path), + created_at: sanitizeTextForBoundary(row.created_at, 128), + updated_at: sanitizeTextForBoundary(row.updated_at, 128), }; - if (safe.name !== row.name || safe.path !== row.path) { - try { - getDb().prepare("UPDATE projects SET name = ?, path = ?, updated_at = ? WHERE id = ?").run( - safe.name, - safe.path, - new Date().toISOString(), - row.id, - ); - } catch { - // Read results remain sanitized when a legacy database is read-only. - } - } return safe; } @@ -42,7 +41,11 @@ export function createProject(name: string, path: string): Project { export function getProject(id: string): Project | null { const db = getDb(); const stmt = db.prepare(`SELECT * FROM projects WHERE id = ?`); - const row = stmt.get(id) as Project | undefined; + let row = stmt.get(id) as Project | undefined; + if (row && legacyRowContainsCredential(row as unknown as Record)) { + const result = scrubLegacyCredentialRows(db); + row = stmt.get(result.projectIds.get(id) ?? id) as Project | undefined; + } return row ? rowToProject(row) : null; } @@ -50,18 +53,33 @@ export function getProjectByPath(path: string): Project | null { const db = getDb(); const safePath = sanitizeLocationForOutput(path); const stmt = db.prepare(`SELECT * FROM projects WHERE path = ? OR path = ? LIMIT 1`); - const row = stmt.get(safePath, path) as Project | undefined; + let row = stmt.get(safePath, path) as Project | undefined; + if (row && legacyRowContainsCredential(row as unknown as Record)) { + const result = scrubLegacyCredentialRows(db); + row = db.prepare("SELECT * FROM projects WHERE id = ?") + .get(result.projectIds.get(row.id) ?? row.id) as Project | undefined; + } return row ? rowToProject(row) : null; } export function listProjects(): Project[] { const db = getDb(); const stmt = db.prepare(`SELECT * FROM projects ORDER BY created_at DESC`); - return (stmt.all() as Project[]).map(rowToProject); + let rows = stmt.all() as Project[]; + if (rows.some((row) => legacyRowContainsCredential(row as unknown as Record))) { + const result = scrubLegacyCredentialRows(db); + rows = rows.flatMap((row) => { + const migrated = db.prepare("SELECT * FROM projects WHERE id = ?") + .get(result.projectIds.get(row.id) ?? row.id) as Project | undefined; + return migrated ? [migrated] : []; + }); + } + return rows.map(rowToProject); } export function deleteProject(id: string): void { const db = getDb(); + const project = getProject(id); const stmt = db.prepare(`DELETE FROM projects WHERE id = ?`); - stmt.run(id); + stmt.run(project?.id ?? id); } diff --git a/src/db/scans.ts b/src/db/scans.ts index 4b4eba8..1b6406d 100644 --- a/src/db/scans.ts +++ b/src/db/scans.ts @@ -3,6 +3,11 @@ import { getDb } from "./database.js"; import type { Scan } from "../types/index.js"; import { ScanStatus, type ScannerType } from "../types/index.js"; import { sanitizeScanForOutput, sanitizeTextForBoundary } from "../lib/finding-safety.js"; +import { + legacyRowContainsCredential, + scrubLegacyCredentialRows, +} from "./legacy-credential-scrub.js"; +import { getProject } from "./projects.js"; interface ScanRow { id: string; @@ -18,39 +23,34 @@ interface ScanRow { } function rowToScan(row: ScanRow): Scan { - const safe = sanitizeScanForOutput({ + return sanitizeScanForOutput({ ...row, status: row.status as ScanStatus, scanner_types: JSON.parse(row.scanner_types) as ScannerType[], }); - if (safe.error !== row.error) { - try { - getDb().prepare("UPDATE scans SET error = ? WHERE id = ?").run(safe.error, row.id); - } catch { - // Output remains sanitized when a legacy/read-only database cannot be - // rewritten in place. - } - } - return safe; } export function createScan(project_id: string, scanner_types: ScannerType[]): Scan { const db = getDb(); + const project = getProject(project_id); + const targetProjectId = project?.id ?? project_id; const id = crypto.randomUUID(); const now = new Date().toISOString(); - const scannerTypesJson = JSON.stringify(scanner_types); + const safeScannerTypes = scanner_types.map((scannerType) => + sanitizeTextForBoundary(String(scannerType), 128) as ScannerType); + const scannerTypesJson = JSON.stringify(safeScannerTypes); const stmt = db.prepare( `INSERT INTO scans (id, project_id, status, scanner_types, findings_count, started_at, created_at) VALUES (?, ?, ?, ?, 0, ?, ?)` ); - stmt.run(id, project_id, ScanStatus.Pending, scannerTypesJson, now, now); + stmt.run(id, targetProjectId, ScanStatus.Pending, scannerTypesJson, now, now); return { id, - project_id, + project_id: targetProjectId, status: ScanStatus.Pending, - scanner_types, + scanner_types: safeScannerTypes, findings_count: 0, started_at: now, completed_at: null, @@ -63,20 +63,35 @@ export function createScan(project_id: string, scanner_types: ScannerType[]): Sc export function getScan(id: string): Scan | null { const db = getDb(); const stmt = db.prepare(`SELECT * FROM scans WHERE id = ?`); - const row = stmt.get(id) as ScanRow | undefined; + let row = stmt.get(id) as ScanRow | undefined; + if (row && legacyRowContainsCredential(row as unknown as Record)) { + const result = scrubLegacyCredentialRows(db); + row = stmt.get(result.scanIds.get(id) ?? id) as ScanRow | undefined; + } return row ? rowToScan(row) : null; } export function listScans(project_id?: string, limit: number = 50): Scan[] { const db = getDb(); + const migrateRows = (rows: ScanRow[]): ScanRow[] => { + if (!rows.some((row) => legacyRowContainsCredential(row as unknown as Record))) { + return rows; + } + const result = scrubLegacyCredentialRows(db); + return rows.flatMap((row) => { + const migrated = db.prepare("SELECT * FROM scans WHERE id = ?") + .get(result.scanIds.get(row.id) ?? row.id) as ScanRow | undefined; + return migrated ? [migrated] : []; + }); + }; if (project_id) { const stmt = db.prepare( `SELECT * FROM scans WHERE project_id = ? ORDER BY created_at DESC LIMIT ?` ); - return (stmt.all(project_id, limit) as ScanRow[]).map(rowToScan); + return migrateRows(stmt.all(project_id, limit) as ScanRow[]).map(rowToScan); } const stmt = db.prepare(`SELECT * FROM scans ORDER BY created_at DESC LIMIT ?`); - return (stmt.all(limit) as ScanRow[]).map(rowToScan); + return migrateRows(stmt.all(limit) as ScanRow[]).map(rowToScan); } export function updateScanStatus( @@ -86,12 +101,15 @@ export function updateScanStatus( error?: string ): void { const db = getDb(); + const scan = getScan(id); + const targetId = scan?.id ?? id; const stmt = db.prepare( `UPDATE scans SET status = ?, findings_count = COALESCE(?, findings_count), error = COALESCE(?, error) WHERE id = ?` ); const safeError = error == null ? null : sanitizeTextForBoundary(error); - stmt.run(status, findings_count ?? null, safeError, id); + const safeStatus = sanitizeTextForBoundary(String(status), 128) as ScanStatus; + stmt.run(safeStatus, findings_count ?? null, safeError, targetId); } export function completeScan(id: string, findings_count: number): void { @@ -99,6 +117,7 @@ export function completeScan(id: string, findings_count: number): void { const now = new Date().toISOString(); const scan = getScan(id); + const targetId = scan?.id ?? id; const duration_ms = scan ? new Date(now).getTime() - new Date(scan.started_at).getTime() : null; @@ -106,11 +125,12 @@ export function completeScan(id: string, findings_count: number): void { const stmt = db.prepare( `UPDATE scans SET status = ?, findings_count = ?, completed_at = ?, duration_ms = ? WHERE id = ?` ); - stmt.run(ScanStatus.Completed, findings_count, now, duration_ms, id); + stmt.run(ScanStatus.Completed, findings_count, now, duration_ms, targetId); } export function deleteScan(id: string): void { const db = getDb(); + const scan = getScan(id); const stmt = db.prepare(`DELETE FROM scans WHERE id = ?`); - stmt.run(id); + stmt.run(scan?.id ?? id); } diff --git a/src/lib/credential-invariant.test.ts b/src/lib/credential-invariant.test.ts index ee973c4..2ad867c 100644 --- a/src/lib/credential-invariant.test.ts +++ b/src/lib/credential-invariant.test.ts @@ -196,6 +196,11 @@ describe("scanner-to-boundary credential invariant", () => { expect(JSON.stringify(getScan(scan.id)), value).not.toContain(value); expect(JSON.stringify(getFinding(created.id)), value).not.toContain(value); + const runtimeScan = createScan(project.id, [value as ScannerType]); + updateScanStatus(runtimeScan.id, value as ScanStatus, undefined, value); + expect(JSON.stringify(db.prepare("SELECT * FROM scans WHERE id = ?").get(runtimeScan.id)), value) + .not.toContain(value); + const legacyId = `legacy-${index}`; db.prepare( `INSERT INTO findings @@ -266,15 +271,136 @@ describe("scanner-to-boundary credential invariant", () => { } }); - test("only exempts 40-character hex in pinned GitHub Action syntax", () => { + test("only the scanner exempts an exact pinned action in a trusted workflow file", () => { const revision = "0123456789abcdef".repeat(3).slice(0, 40); expect( recognizeCredentialText(revision).some(({ rule }) => rule.id === "high-entropy-hex"), ).toBe(true); const actionPin = `- uses: synthetic/action@${revision}`; - expect(scanFile(".github/workflows/ci.yml", actionPin)).toEqual([]); - expect(recognizeCredentialText(actionPin)).toEqual([]); - expect(sanitizeTextForBoundary(actionPin, 12_000)).toBe(actionPin); + // The public line scanner does not trust a caller-claimed filename. The + // filesystem scanner's verified-path exception is exercised separately. + expect(scanFile(".github/workflows/ci.yml", actionPin).length).toBeGreaterThan(0); + expect(scanFile( + ".github/workflows/ci.yaml", + `uses: synthetic/.github/workflows/reuse.yml@${revision}`, + ).length).toBeGreaterThan(0); + + for (const untrustedPath of [ + ".env", + "config.json", + "src/data.yml", + ".github/workflows/nested/ci.yml", + ".github/actions/local/action.yml", + ]) { + expect(scanFile(untrustedPath, actionPin).length, untrustedPath).toBeGreaterThan(0); + } + + expect(recognizeCredentialText(actionPin).length).toBeGreaterThan(0); + expect(sanitizeTextForBoundary(actionPin, 12_000)).not.toContain(revision); + + const finding = findingWith(actionPin); + const scan: Scan = { + id: actionPin, + project_id: actionPin, + status: ScanStatus.Failed, + scanner_types: [ScannerType.Code], + findings_count: 1, + started_at: actionPin, + completed_at: actionPin, + duration_ms: 1, + error: actionPin, + created_at: actionPin, + }; + for (const output of [ + JSON.stringify(sanitizeValueForBoundary({ arbitrary: actionPin })), + reportJson([finding], scan), + reportSarif([finding], scan), + JSON.stringify(sanitizeMessagesForProvider([{ role: "user", content: actionPin }])), + ]) { + expect(output).not.toContain(revision); + expect(output).toContain("REDACTED"); + } + }); + + test("recognizes canonical padded and unpadded base64 tokens for common byte lengths", () => { + for (const byteLength of [16, 24, 32]) { + const bytes = Uint8Array.from({ length: byteLength }, (_, index) => index); + const padded = Buffer.from(bytes).toString("base64"); + const unpadded = padded.replace(/=+$/, ""); + for (const value of new Set([padded, unpadded])) { + expect( + recognizeCredentialText(value).some(({ rule }) => rule.id === "high-entropy-base64"), + `${byteLength}:${value.length}`, + ).toBe(true); + expect(containsCredentialLikeText(value)).toBe(true); + expect(sanitizeTextForBoundary(value)).not.toContain(value); + expect(sanitizeLocationForOutput(`/tmp/${value}/artifact`)).not.toContain(value); + } + } + }); + + test("base64 entropy boundary rejects a bounded structured-safe corpus", () => { + const structured = new Set(); + structured.add("isPlausibleBase64Token"); + structured.add("normalizedSampleEntropy"); + structured.add("allowPinnedGitHubActionRevision"); + for (let index = 0; index < 2_048; index++) { + structured.add(`component${index % 10}`.repeat(4)); + structured.add(`${"A".repeat(8)}${String(index).padStart(8, "0")}${"b".repeat(16)}`); + structured.add(`${(index % 16).toString(16).repeat(16)}${((index + 1) % 16).toString(16).repeat(16)}`); + } + for (const value of structured) { + expect( + recognizeCredentialText(value).some(({ rule }) => rule.id === "high-entropy-base64"), + value, + ).toBe(false); + } + }); + + test("legacy Scan and Finding rows are durably scrubbed as a transaction", () => { + cleanupDb = setupTestDb(); + const db = getCurrentTestDb(); + const marker = `gh${"p"}_${"Legacy_A4_".repeat(4)}`; + + db.prepare( + `INSERT INTO projects (id, name, path, created_at, updated_at) + VALUES (?, ?, ?, ?, ?)`, + ).run(marker, marker, marker, marker, marker); + db.prepare( + `INSERT INTO scans + (id, project_id, status, scanner_types, findings_count, started_at, completed_at, duration_ms, error, created_at) + VALUES (?, ?, ?, ?, 1, ?, ?, 1, ?, ?)`, + ).run(marker, marker, marker, JSON.stringify([marker]), marker, marker, marker, marker); + db.prepare( + `INSERT INTO rules + (id, name, description, scanner_type, severity, pattern, enabled, builtin, metadata, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, 1, 0, ?, ?, ?)`, + ).run(marker, marker, marker, marker, marker, marker, JSON.stringify({ marker }), marker, marker); + db.prepare( + `INSERT INTO findings + (id, scan_id, rule_id, scanner_type, severity, file, line, message, code_snippet, fingerprint, + suppressed, suppressed_reason, llm_explanation, llm_fix, created_at) + VALUES (?, ?, ?, ?, ?, ?, 1, ?, ?, ?, 1, ?, ?, ?, ?)`, + ).run(marker, marker, marker, marker, marker, marker, marker, marker, marker, marker, marker, marker, marker); + + const safeScan = getScan(marker); + const durableFindingId = (db.prepare("SELECT id FROM findings LIMIT 1").get() as { id: string }).id; + const safeFinding = getFinding(durableFindingId); + expect(JSON.stringify(safeScan)).not.toContain(marker); + expect(JSON.stringify(safeFinding)).not.toContain(marker); + expect(safeScan?.id).not.toBe(marker); + expect(safeFinding?.id).not.toBe(marker); + + for (const table of ["projects", "scans", "rules", "findings"]) { + expect(JSON.stringify(db.prepare(`SELECT * FROM ${table}`).all()), table).not.toContain(marker); + } + expect(db.prepare("PRAGMA foreign_key_check").all()).toEqual([]); + + // The durable identifiers are queryable and a second read is a no-op. + expect(getScan(safeScan!.id)).toEqual(safeScan); + expect(getFinding(safeFinding!.id)).toEqual(safeFinding); + expect(JSON.stringify(db.prepare("SELECT * FROM scans").all())).not.toContain(marker); + expect(JSON.stringify(db.prepare("SELECT * FROM findings").all())).not.toContain(marker); }); test("high-entropy hexadecimal property corpus keeps positives and false positives separated", () => { diff --git a/src/lib/credential-recognition.ts b/src/lib/credential-recognition.ts index 161507e..cb4f201 100644 --- a/src/lib/credential-recognition.ts +++ b/src/lib/credential-recognition.ts @@ -207,22 +207,61 @@ function collectPatternMatches( return recognitions; } -function isPinnedGitHubActionRevision( +function isExactPinnedGitHubActionRevision( value: string, index: number, match: string, ): boolean { if (match.length !== 40) return false; - const before = value.slice(0, index).trim(); - const after = value.slice(index + match.length); - return /^-?\s*uses:\s*["']?[A-Za-z0-9_.-]+\/[A-Za-z0-9_./-]+@$/.test(before) - && /^["']?\s*(?:#.*)?$/.test(after); + const unquoted = /^\s*(?:-\s+)?uses:\s*[A-Za-z0-9_.-]+\/[A-Za-z0-9_./-]+@([0-9a-fA-F]{40})\s*(?:#.*)?$/; + const quoted = /^\s*(?:-\s+)?uses:\s*(["'])[A-Za-z0-9_.-]+\/[A-Za-z0-9_./-]+@([0-9a-fA-F]{40})\1\s*(?:#.*)?$/; + const parsed = unquoted.exec(value); + const revision = parsed?.[1] ?? quoted.exec(value)?.[2]; + return revision === match && value.indexOf(match) === index; } -function collectEntropyMatches(value: string): CredentialRecognition[] { +function isPlausibleBase64Token(value: string): boolean { + const unpadded = value.replace(/=+$/, ""); + const paddingLength = value.length - unpadded.length; + if (paddingLength > 2 || unpadded.length < 22) return false; + if (paddingLength > 0 && value.length % 4 !== 0) return false; + if (paddingLength === 0 && unpadded.length % 4 === 1) return false; + + // Random binary tokens almost always span at least three Base64 character + // classes. Requiring that diversity sharply bounds false positives from + // prose, identifiers, repeated fixtures, and numeric values at short sample + // lengths without requiring '+' or '/' to be present. + const uppercase = [...unpadded].filter((character) => /[A-Z]/.test(character)).length; + const lowercase = [...unpadded].filter((character) => /[a-z]/.test(character)).length; + const hasNonLetter = /[0-9+/]/.test(unpadded); + const letterCount = uppercase + lowercase; + // CamelCase source identifiers can satisfy a naive three-class check only + // because their names contain "64". Random Base64 has balanced letter case; + // requiring the minority case to make up 18% of letters rejects those + // identifiers while retaining >98% of random 16-byte and >99.7% of random + // 24/32-byte samples before the independent entropy bound. + const minorityCaseRatio = letterCount === 0 + ? 0 + : Math.min(uppercase, lowercase) / letterCount; + return hasNonLetter && minorityCaseRatio >= 0.18; +} + +function normalizedSampleEntropy(value: string, alphabetSize: number): number { + const sample = value.replace(/=+$/, ""); + const reachableAlphabetSize = Math.min(alphabetSize, sample.length); + if (reachableAlphabetSize <= 1) return 0; + return shannonEntropy(sample) / Math.log2(reachableAlphabetSize); +} + +function collectEntropyMatches( + value: string, + allowPinnedGitHubActionRevision = false, + boundary = false, +): CredentialRecognition[] { const recognitions: CredentialRecognition[] = []; - // Normalize against each alphabet's theoretical maximum. Hex tops out at - // exactly 4 bits/character, so a raw threshold above 4 is unreachable. + // Normalize against the maximum empirical entropy reachable by this sample, + // not merely the alphabet maximum. A 22-character Base64 token cannot + // empirically exceed log2(22), while a hexadecimal alphabet tops out at 4. for (const { definition, alphabetSize, normalizedThreshold } of [ { definition: HIGH_ENTROPY_HEX_DEFINITION, @@ -231,8 +270,11 @@ function collectEntropyMatches(value: string): CredentialRecognition[] { }, { definition: HIGH_ENTROPY_BASE64_DEFINITION, - alphabetSize: 65, - normalizedThreshold: 5.0 / Math.log2(65), + alphabetSize: 64, + // Monte Carlo bounds for uniformly random 16/24/32-byte tokens place + // more than 99.9% above this length-aware threshold. Structured-safe + // controls are also required to span three character classes. + normalizedThreshold: 0.8, }, ] as const) { const rule = materialize(definition); @@ -240,12 +282,45 @@ function collectEntropyMatches(value: string): CredentialRecognition[] { while ((match = rule.pattern.exec(value)) !== null) { if ( definition.id === HIGH_ENTROPY_HEX_DEFINITION.id - && isPinnedGitHubActionRevision(value, match.index, match[0]) + && allowPinnedGitHubActionRevision + && isExactPinnedGitHubActionRevision(value, match.index, match[0]) + ) { + continue; + } + const plausibleBase64 = + definition.id !== HIGH_ENTROPY_BASE64_DEFINITION.id + || isPlausibleBase64Token(match[0]); + if ( + definition.id === HIGH_ENTROPY_BASE64_DEFINITION.id + && !plausibleBase64 ) { + // A credential embedded in a path or identifier can be swallowed by + // the broad Base64 alphabet (notably '/'). Boundary recognition must + // remain a strict scanner superset, so inspect canonical common-token + // windows inside the greedy candidate without changing scanner noise. + if (boundary) { + for (const windowLength of [44, 43, 32, 24, 22]) { + if (windowLength > match[0].length) continue; + for (let offset = 0; offset <= match[0].length - windowLength; offset++) { + const window = match[0].slice(offset, offset + windowLength); + if ( + isPlausibleBase64Token(window) + && normalizedSampleEntropy(window, alphabetSize) >= normalizedThreshold + ) { + recognitions.push({ + index: match.index + offset, + length: windowLength, + rule, + }); + offset = match[0].length; + } + } + } + } continue; } - const normalizedEntropy = shannonEntropy(match[0]) / Math.log2(alphabetSize); - if (normalizedEntropy > normalizedThreshold) { + const normalizedEntropy = normalizedSampleEntropy(match[0], alphabetSize); + if (normalizedEntropy >= normalizedThreshold) { recognitions.push({ index: match.index, length: match[0].length, rule }); } if (match[0].length === 0) rule.pattern.lastIndex++; @@ -257,6 +332,8 @@ function collectEntropyMatches(value: string): CredentialRecognition[] { export interface CredentialRecognitionOptions { boundary?: boolean; envLike?: boolean; + /** Scanner-only exception after the source path is verified as a workflow. */ + trustedGitHubWorkflowFile?: boolean; } /** @@ -274,7 +351,11 @@ export function recognizeCredentialText( if (options.envLike || options.boundary) { recognitions.push(...collectPatternMatches(value, [ENV_API_KEY_DEFINITION])); } - recognitions.push(...collectEntropyMatches(value)); + recognitions.push(...collectEntropyMatches( + value, + options.trustedGitHubWorkflowFile === true && options.boundary !== true, + options.boundary === true, + )); return recognitions.sort((left, right) => left.index - right.index || right.length - left.length); } diff --git a/src/lib/finding-safety.ts b/src/lib/finding-safety.ts index 00cd99d..266dd8b 100644 --- a/src/lib/finding-safety.ts +++ b/src/lib/finding-safety.ts @@ -22,6 +22,26 @@ function stableRedaction(value: string, kind: string): string { return `[REDACTED-${kind}:${correlation}]`; } +/** + * Produce a durable opaque key for replacing a credential-bearing database + * identifier. Callers verify uniqueness against the destination table and + * deterministically retry on the theoretical 48-bit correlation collision. + * Keeping the visible correlation short also prevents the opaque replacement + * itself from being classified as a high-entropy credential. + */ +export function opaqueIdentifierForStorage( + value: string, + kind: string, + attempt = 0, +): string { + const safeKind = kind.replace(/[^A-Z0-9_-]/gi, "-").toUpperCase(); + const correlation = createHash("sha256") + .update(`${safeKind}\0${value}\0${attempt}`) + .digest("hex") + .slice(0, 12); + return `[REDACTED-${safeKind}:${correlation}]`; +} + /** Preserve correlation without retaining a credential-bearing identifier. */ export function sanitizeIdentifierForOutput(value: string, kind = "ID"): string { return containsCredentialLikeText(value) diff --git a/src/scanners/secrets.test.ts b/src/scanners/secrets.test.ts index 8455ed1..1cf83f0 100644 --- a/src/scanners/secrets.test.ts +++ b/src/scanners/secrets.test.ts @@ -27,6 +27,22 @@ describe("secrets scanner", () => { // --- scanFile unit tests --- describe("scanFile", () => { + test("verifies the real workflow path before exempting an exact action pin", async () => { + const revision = "0123456789abcdef".repeat(3).slice(0, 40); + const workflowDirectory = join(tempDir, ".github", "workflows"); + mkdirSync(workflowDirectory, { recursive: true }); + const workflow = join(workflowDirectory, "ci.yml"); + const ordinary = join(tempDir, "config.yml"); + const content = `- uses: synthetic/action@${revision}`; + writeFileSync(workflow, content); + writeFileSync(ordinary, content); + + expect(await secretsScanner.scan(workflow)).toEqual([]); + expect((await secretsScanner.scan(ordinary)).some( + (finding) => finding.rule_id === "high-entropy-hex", + )).toBe(true); + }); + test("detects AWS access key", () => { const content = 'const key = "AKIAIOSFODNN7EXAMPLE";'; const findings = scanFile("test.ts", content); diff --git a/src/scanners/secrets.ts b/src/scanners/secrets.ts index 7a9ac74..ff07c8d 100644 --- a/src/scanners/secrets.ts +++ b/src/scanners/secrets.ts @@ -242,6 +242,17 @@ function isEnvLikeFile(filePath: string): boolean { return base === ".env" || base.startsWith(".env.") || base.endsWith(".env"); } +function isTrustedGitHubWorkflowFile(filePath: string): boolean { + const normalized = filePath.replace(/\\/g, "/").toLowerCase(); + const marker = "/.github/workflows/"; + const relative = normalized.startsWith(".github/workflows/") + ? normalized.slice(".github/workflows/".length) + : normalized.includes(marker) + ? normalized.slice(normalized.lastIndexOf(marker) + marker.length) + : ""; + return relative.length > 0 && !relative.includes("/") && /\.ya?ml$/.test(relative); +} + function getCommentSyntax(filePath?: string): CommentSyntax { if (!filePath) { return NO_COMMENT_SYNTAX; @@ -587,7 +598,11 @@ export function isFindingSuppressedBySecurityIgnore( // --- Scanner --- -export function scanFile(filePath: string, content: string): FindingInput[] { +export function scanFile( + filePath: string, + content: string, + verifiedSourcePath?: string, +): FindingInput[] { const findings: FindingInput[] = []; const lines = content.split("\n"); const securityIgnoreBlockRanges = collectSecurityIgnoreBlockRanges(content, filePath); @@ -612,7 +627,11 @@ export function scanFile(filePath: string, content: string): FindingInput[] { blockComment = securityIgnore.finalBlockComment; blockCommentHasSecurityIgnore = securityIgnore.finalBlockCommentHasSecurityIgnore; - for (const recognition of recognizeCredentialText(lineText, { envLike: isEnvLikeFile(filePath) })) { + for (const recognition of recognizeCredentialText(lineText, { + envLike: isEnvLikeFile(verifiedSourcePath ?? filePath), + trustedGitHubWorkflowFile: + verifiedSourcePath != null && isTrustedGitHubWorkflowFile(verifiedSourcePath), + })) { if (isFindingSuppressedBySecurityIgnore(securityIgnore, recognition.index)) continue; findings.push({ rule_id: recognition.rule.id, @@ -657,7 +676,7 @@ export const secretsScanner: Scanner = { try { const content = fs.readFileSync(file, "utf-8"); const relativePath = stat.isFile() ? path.basename(file) : path.relative(scanPath, file); - findings.push(...scanFile(relativePath, content)); + findings.push(...scanFile(relativePath, content, file)); } catch { throw new Error("Unable to read every requested scan file"); } From 35e7bee4067ad338b99d2f4b55ac3e585c92c90a Mon Sep 17 00:00:00 2001 From: Andrei Hasna Date: Thu, 16 Jul 2026 14:02:55 +0300 Subject: [PATCH 6/8] fix: scrub baseline and cache persistence --- src/db/baselines.test.ts | 73 +++++++++++++ src/db/baselines.ts | 57 ++++++++-- src/db/legacy-credential-scrub.test.ts | 141 +++++++++++++++++++++++- src/db/legacy-credential-scrub.ts | 145 +++++++++++++++++++++++-- src/db/llm-cache.test.ts | 101 +++++++++++++++++ src/db/llm-cache.ts | 71 ++++++++---- src/mcp/tools/output-safety.test.ts | 23 ++++ 7 files changed, 561 insertions(+), 50 deletions(-) create mode 100644 src/db/baselines.test.ts diff --git a/src/db/baselines.test.ts b/src/db/baselines.test.ts new file mode 100644 index 0000000..ed5289c --- /dev/null +++ b/src/db/baselines.test.ts @@ -0,0 +1,73 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { + createBaseline, + deleteBaseline, + isBaselined, + listBaselines, +} from "./baselines.js"; +import { getCurrentTestDb, setupTestDb } from "./test-helpers.js"; +import { opaqueIdentifierForStorage } from "../lib/finding-safety.js"; + +describe("baseline credential boundary", () => { + let cleanup: () => void; + + beforeEach(() => { + cleanup = setupTestDb(); + }); + + afterEach(() => cleanup()); + + test("sanitizes every caller-controlled string before persistence", () => { + const marker = `gh${"p"}_${"Baseline_New_Write_".repeat(3)}`; + const baseline = createBaseline( + marker, + `requested because the synthetic marker is ${marker}`, + `agent-${marker}`, + ); + + expect(JSON.stringify(baseline)).not.toContain(marker); + expect(isBaselined(marker)).toBe(true); + expect(JSON.stringify(listBaselines())).not.toContain(marker); + expect(JSON.stringify(getCurrentTestDb().prepare("SELECT * FROM baselines").all())) + .not.toContain(marker); + }); + + test("listBaselines durably scrubs production-shaped legacy rows", () => { + const marker = `sk_test_${"BaselineLegacy9876543210".repeat(2)}`; + const db = getCurrentTestDb(); + db.prepare( + `INSERT INTO baselines (id, finding_fingerprint, reason, created_by, created_at) + VALUES (?, ?, ?, ?, ?)`, + ).run(marker, marker, `reason=${marker}`, `creator-${marker}`, marker); + + expect(JSON.stringify(listBaselines())).not.toContain(marker); + expect(JSON.stringify(db.prepare("SELECT * FROM baselines").all())).not.toContain(marker); + + const [baseline] = listBaselines(); + expect(isBaselined(baseline.finding_fingerprint)).toBe(true); + deleteBaseline(baseline.id); + expect(listBaselines()).toEqual([]); + }); + + test("raw legacy IDs cannot delete an unrelated collision occupant", () => { + const marker = `gh${"o"}_${"BaselineDeleteCollision_".repeat(3)}`; + const occupiedId = opaqueIdentifierForStorage(marker, "BASELINE-ID"); + const db = getCurrentTestDb(); + db.prepare( + `INSERT INTO baselines (id, finding_fingerprint, reason, created_by, created_at) + VALUES (?, 'occupied-fingerprint', 'safe', 'safe', 'now')`, + ).run(occupiedId); + db.prepare( + `INSERT INTO baselines (id, finding_fingerprint, reason, created_by, created_at) + VALUES (?, 'legacy-fingerprint', 'safe', 'safe', 'now')`, + ).run(marker); + + const scrubbed = listBaselines(); + expect(scrubbed).toHaveLength(2); + deleteBaseline(marker); + expect(listBaselines()).toHaveLength(2); + + for (const baseline of scrubbed) deleteBaseline(baseline.id); + expect(listBaselines()).toEqual([]); + }); +}); diff --git a/src/db/baselines.ts b/src/db/baselines.ts index ca26c2a..cd8c3b0 100644 --- a/src/db/baselines.ts +++ b/src/db/baselines.ts @@ -1,6 +1,25 @@ import crypto from "crypto"; import { getDb } from "./database.js"; import type { Baseline } from "../types/index.js"; +import { + sanitizeFingerprintForOutput, + sanitizeIdentifierForOutput, + sanitizeTextForBoundary, +} from "../lib/finding-safety.js"; +import { + legacyRowContainsCredential, + scrubLegacyCredentialRows, +} from "./legacy-credential-scrub.js"; + +function sanitizeBaseline(row: Baseline): Baseline { + return { + id: sanitizeIdentifierForOutput(row.id, "BASELINE-ID"), + finding_fingerprint: sanitizeFingerprintForOutput(row.finding_fingerprint), + reason: sanitizeTextForBoundary(row.reason), + created_by: sanitizeTextForBoundary(row.created_by, 256), + created_at: sanitizeTextForBoundary(row.created_at, 128), + }; +} export function createBaseline( fingerprint: string, @@ -10,34 +29,50 @@ export function createBaseline( const db = getDb(); const id = crypto.randomUUID(); const now = new Date().toISOString(); + const baseline = sanitizeBaseline({ + id, + finding_fingerprint: fingerprint, + reason, + created_by, + created_at: now, + }); const stmt = db.prepare( `INSERT INTO baselines (id, finding_fingerprint, reason, created_by, created_at) VALUES (?, ?, ?, ?, ?)` ); - stmt.run(id, fingerprint, reason, created_by, now); + stmt.run( + baseline.id, + baseline.finding_fingerprint, + baseline.reason, + baseline.created_by, + baseline.created_at, + ); - return { - id, - finding_fingerprint: fingerprint, - reason, - created_by, - created_at: now, - }; + return baseline; } export function listBaselines(): Baseline[] { const db = getDb(); const stmt = db.prepare(`SELECT * FROM baselines ORDER BY created_at DESC`); - return stmt.all() as Baseline[]; + let rows = stmt.all() as Baseline[]; + if (rows.some((row) => legacyRowContainsCredential( + row as unknown as Record, + ))) { + scrubLegacyCredentialRows(db); + rows = stmt.all() as Baseline[]; + } + return rows.map(sanitizeBaseline); } export function isBaselined(fingerprint: string): boolean { const db = getDb(); + const safeFingerprint = sanitizeFingerprintForOutput(fingerprint); const stmt = db.prepare( - `SELECT COUNT(*) as count FROM baselines WHERE finding_fingerprint = ?` + `SELECT COUNT(*) as count FROM baselines + WHERE finding_fingerprint = ? OR finding_fingerprint = ?` ); - const row = stmt.get(fingerprint) as { count: number }; + const row = stmt.get(safeFingerprint, fingerprint) as { count: number }; return row.count > 0; } diff --git a/src/db/legacy-credential-scrub.test.ts b/src/db/legacy-credential-scrub.test.ts index 8eebbcb..5fa4794 100644 --- a/src/db/legacy-credential-scrub.test.ts +++ b/src/db/legacy-credential-scrub.test.ts @@ -4,7 +4,11 @@ import { mkdtempSync, rmSync } from "fs"; import { tmpdir } from "os"; import { join } from "path"; import { scrubLegacyCredentialRows } from "./legacy-credential-scrub.js"; -import { opaqueIdentifierForStorage } from "../lib/finding-safety.js"; +import { + opaqueIdentifierForStorage, + sanitizeFingerprintForOutput, + sanitizeTextForBoundary, +} from "../lib/finding-safety.js"; const tempDirs: string[] = []; @@ -40,8 +44,16 @@ function createFixtureDb(): { db: Database; marker: string; path: string } { code_snippet TEXT, fingerprint TEXT NOT NULL, suppressed INTEGER NOT NULL, suppressed_reason TEXT, llm_explanation TEXT, llm_fix TEXT, llm_exploitability REAL, created_at TEXT NOT NULL ); - CREATE TABLE baselines (id TEXT PRIMARY KEY, finding_fingerprint TEXT NOT NULL); - CREATE TABLE llm_cache (id TEXT PRIMARY KEY, finding_fingerprint TEXT NOT NULL); + CREATE TABLE baselines ( + id TEXT PRIMARY KEY, finding_fingerprint TEXT NOT NULL, reason TEXT NOT NULL DEFAULT '', + created_by TEXT NOT NULL DEFAULT 'system', created_at TEXT NOT NULL DEFAULT (datetime('now')) + ); + CREATE TABLE llm_cache ( + id TEXT PRIMARY KEY, finding_fingerprint TEXT NOT NULL, analysis_type TEXT NOT NULL, + result TEXT NOT NULL, model TEXT NOT NULL, tokens_used INTEGER NOT NULL DEFAULT 0, + created_at TEXT NOT NULL DEFAULT (datetime('now')), + UNIQUE(finding_fingerprint, analysis_type) + ); `); const marker = `gh${"o"}_${"Rollback_A4_".repeat(4)}`; db.prepare("INSERT INTO projects VALUES (?, ?, ?, ?, ?)").run(marker, marker, marker, marker, marker); @@ -67,8 +79,10 @@ function createFixtureDb(): { db: Database; marker: string; path: string } { marker, marker, ); - db.prepare("INSERT INTO baselines VALUES ('baseline', ?)").run(marker); - db.prepare("INSERT INTO llm_cache VALUES ('cache', ?)").run(marker); + db.prepare("INSERT INTO baselines VALUES (?, ?, ?, ?, ?)") + .run(marker, marker, marker, marker, marker); + db.prepare("INSERT INTO llm_cache VALUES (?, ?, ?, ?, ?, 1, ?)") + .run(marker, marker, marker, JSON.stringify({ marker }), marker, marker); return { db, marker, path }; } @@ -77,6 +91,38 @@ function rawDatabase(db: Database): string { .map((table) => db.prepare(`SELECT * FROM ${table}`).all())); } +function expectProductionMetadataClean(db: Database, marker: string): void { + const baseline = db.prepare("SELECT * FROM baselines ORDER BY id LIMIT 1").get() as Record< + string, + unknown + >; + const cache = db.prepare("SELECT * FROM llm_cache ORDER BY id LIMIT 1").get() as Record< + string, + unknown + >; + expect(Object.keys(baseline).sort()).toEqual([ + "created_at", + "created_by", + "finding_fingerprint", + "id", + "reason", + ]); + expect(Object.keys(cache).sort()).toEqual([ + "analysis_type", + "created_at", + "finding_fingerprint", + "id", + "model", + "result", + "tokens_used", + ]); + for (const row of [baseline, cache]) { + for (const value of Object.values(row)) { + if (typeof value === "string") expect(value).not.toContain(marker); + } + } +} + describe("legacy credential graph scrub", () => { test("serializes simultaneous scrub attempts from separate processes", async () => { const { db, marker, path } = createFixtureDb(); @@ -118,6 +164,7 @@ describe("legacy credential graph scrub", () => { expect(secondResult.scanIds.size).toBe(0); expect(rawDatabase(db)).not.toContain(marker); expect(rawDatabase(second)).not.toContain(marker); + expectProductionMetadataClean(db, marker); expect(db.prepare("PRAGMA foreign_key_check").all()).toEqual([]); expect(second.prepare("PRAGMA foreign_key_check").all()).toEqual([]); } finally { @@ -129,6 +176,7 @@ describe("legacy credential graph scrub", () => { test("rolls back every parent and child mutation on a write failure", () => { const { db, marker } = createFixtureDb(); try { + const before = rawDatabase(db); db.exec(` CREATE TRIGGER reject_legacy_finding_update BEFORE UPDATE ON findings @@ -139,6 +187,7 @@ describe("legacy credential graph scrub", () => { expect(() => scrubLegacyCredentialRows(db)).toThrow( "Unable to durably sanitize legacy credential data", ); + expect(rawDatabase(db)).toBe(before); expect(rawDatabase(db)).toContain(marker); expect((db.prepare("SELECT COUNT(*) AS count FROM projects").get() as { count: number }).count).toBe(1); expect((db.prepare("SELECT COUNT(*) AS count FROM scans").get() as { count: number }).count).toBe(1); @@ -159,23 +208,103 @@ describe("legacy credential graph scrub", () => { const { db, marker } = createFixtureDb(); try { const occupiedScanId = opaqueIdentifierForStorage(marker, "SCAN-ID"); + const occupiedBaselineId = opaqueIdentifierForStorage(marker, "BASELINE-ID"); + const occupiedCacheId = opaqueIdentifierForStorage(marker, "LLM-CACHE-ID"); db.prepare("INSERT INTO projects VALUES ('collision-project', 'safe', '/safe', 'now', 'now')").run(); db.prepare("INSERT INTO scans VALUES (?, 'collision-project', 'completed', '[]', 0, 'now', NULL, 1, NULL, 'now')") .run(occupiedScanId); + db.prepare("INSERT INTO baselines VALUES (?, 'safe-fingerprint', 'safe', 'safe', 'now')") + .run(occupiedBaselineId); + db.prepare("INSERT INTO llm_cache VALUES (?, 'safe-fingerprint', 'safe', '{}', 'safe', 0, 'now')") + .run(occupiedCacheId); const result = scrubLegacyCredentialRows(db); const ids = [ result.projectIds.get(marker), result.scanIds.get(marker), result.ruleIds.get(marker), result.findingIds.get(marker), + result.baselineIds.get(marker), + result.llmCacheIds.get(marker), ]; - expect(new Set(ids).size).toBe(4); + expect(new Set(ids).size).toBe(6); expect(ids.every((id) => id?.startsWith("[REDACTED-"))).toBe(true); expect(result.scanIds.get(marker)).not.toBe(occupiedScanId); + expect(result.baselineIds.get(marker)).not.toBe(occupiedBaselineId); + expect(result.llmCacheIds.get(marker)).not.toBe(occupiedCacheId); expect((db.prepare("SELECT COUNT(*) AS count FROM scans WHERE id = ?").get(occupiedScanId) as { count: number }).count) .toBe(1); } finally { db.close(); } }); + + test("scrubs baseline/cache-only exposure and is idempotent", () => { + const { db, marker } = createFixtureDb(); + try { + db.exec("DELETE FROM findings; DELETE FROM scans; DELETE FROM rules; DELETE FROM projects"); + const first = scrubLegacyCredentialRows(db); + const second = scrubLegacyCredentialRows(db); + + expect(first.baselineIds.get(marker)).toBeDefined(); + expect(first.llmCacheIds.get(marker)).toBeDefined(); + expect(second.baselineIds.size).toBe(0); + expect(second.llmCacheIds.size).toBe(0); + expect(rawDatabase(db)).not.toContain(marker); + expect(db.prepare("PRAGMA foreign_key_check").all()).toEqual([]); + } finally { + db.close(); + } + }); + + test("rolls back baseline and cache mutations when either durable write fails", () => { + const { db, marker } = createFixtureDb(); + try { + db.exec("DELETE FROM findings; DELETE FROM scans; DELETE FROM rules; DELETE FROM projects"); + const before = rawDatabase(db); + db.exec(` + CREATE TRIGGER reject_cache_scrub + BEFORE UPDATE ON llm_cache + BEGIN + SELECT RAISE(ABORT, 'synthetic cache write rejection'); + END; + `); + + expect(() => scrubLegacyCredentialRows(db)).toThrow( + "Unable to durably sanitize legacy credential data", + ); + expect(rawDatabase(db)).toBe(before); + expect(rawDatabase(db)).toContain(marker); + + db.exec("DROP TRIGGER reject_cache_scrub"); + scrubLegacyCredentialRows(db); + expect(rawDatabase(db)).not.toContain(marker); + expect(db.prepare("PRAGMA foreign_key_check").all()).toEqual([]); + } finally { + db.close(); + } + }); + + test("preserves cache rows whose sanitized lookup keys collide", () => { + const { db, marker } = createFixtureDb(); + try { + const safeFingerprint = sanitizeFingerprintForOutput(marker); + const safeAnalysisType = sanitizeTextForBoundary(marker, 128); + db.prepare("INSERT INTO llm_cache VALUES ('safe-cache', ?, ?, '{}', 'safe', 0, 'now')") + .run(safeFingerprint, safeAnalysisType); + + expect(() => scrubLegacyCredentialRows(db)).not.toThrow(); + expect(rawDatabase(db)).not.toContain(marker); + expect( + (db.prepare( + "SELECT COUNT(*) AS count FROM llm_cache WHERE finding_fingerprint = ? AND analysis_type = ?", + ).get(safeFingerprint, safeAnalysisType) as { count: number }).count, + ).toBe(1); + expect( + (db.prepare("SELECT COUNT(*) AS count FROM llm_cache").get() as { count: number }).count, + ).toBe(2); + expect(db.prepare("PRAGMA foreign_key_check").all()).toEqual([]); + } finally { + db.close(); + } + }); }); diff --git a/src/db/legacy-credential-scrub.ts b/src/db/legacy-credential-scrub.ts index 9996880..54f62b0 100644 --- a/src/db/legacy-credential-scrub.ts +++ b/src/db/legacy-credential-scrub.ts @@ -5,6 +5,7 @@ import { containsCredentialLikeText, opaqueIdentifierForStorage, sanitizeFindingForOutput, + sanitizeFingerprintForOutput, sanitizeLocationForOutput, sanitizeScanForOutput, sanitizeTextForBoundary, @@ -12,10 +13,12 @@ import { } from "../lib/finding-safety.js"; type RawRow = Record; -type IdTable = "findings" | "projects" | "rules" | "scans"; +type IdTable = "baselines" | "findings" | "llm_cache" | "projects" | "rules" | "scans"; export interface LegacyCredentialScrubResult { + baselineIds: Map; findingIds: Map; + llmCacheIds: Map; projectIds: Map; ruleIds: Map; scanIds: Map; @@ -28,7 +31,9 @@ export function legacyRowContainsCredential(row: RawRow): boolean { function emptyResult(): LegacyCredentialScrubResult { return { + baselineIds: new Map(), findingIds: new Map(), + llmCacheIds: new Map(), projectIds: new Map(), ruleIds: new Map(), scanIds: new Map(), @@ -53,6 +58,20 @@ function sanitizeJsonText(value: string): string { } } +export function sanitizeCachedResultText(value: string): string { + try { + const parsed = sanitizeValueForBoundary(JSON.parse(value)); + if (parsed !== null && typeof parsed === "object" && !Array.isArray(parsed)) { + return JSON.stringify(parsed); + } + return JSON.stringify({ legacy_cache_value: parsed }); + } catch { + return JSON.stringify({ + legacy_cache_value: sanitizeTextForBoundary(value, 12_000), + }); + } +} + function buildIdMap( db: Database, table: IdTable, @@ -81,7 +100,14 @@ function buildIdMap( } function hasAnyUnsafeRows(db: Database): boolean { - for (const table of ["projects", "scans", "rules", "findings"] as const) { + for (const table of [ + "projects", + "scans", + "rules", + "findings", + "baselines", + "llm_cache", + ] as const) { const rows = db.prepare(`SELECT * FROM ${table}`).all() as RawRow[]; if (rows.some(legacyRowContainsCredential)) return true; } @@ -101,10 +127,14 @@ export function scrubLegacyCredentialRows(db: Database): LegacyCredentialScrubRe const scans = db.prepare("SELECT * FROM scans").all() as RawRow[]; const rules = db.prepare("SELECT * FROM rules").all() as RawRow[]; const findings = db.prepare("SELECT * FROM findings").all() as RawRow[]; + const baselines = db.prepare("SELECT * FROM baselines").all() as RawRow[]; + const llmCache = db.prepare("SELECT * FROM llm_cache").all() as RawRow[]; const projectIds = buildIdMap(db, "projects", "PROJECT-ID", projects); const scanIds = buildIdMap(db, "scans", "SCAN-ID", scans); const ruleIds = buildIdMap(db, "rules", "RULE-ID", rules); const findingIds = buildIdMap(db, "findings", "FINDING-ID", findings); + const baselineIds = buildIdMap(db, "baselines", "BASELINE-ID", baselines); + const llmCacheIds = buildIdMap(db, "llm_cache", "LLM-CACHE-ID", llmCache); for (const row of projects) { const oldId = String(row.id); @@ -253,12 +283,106 @@ export function scrubLegacyCredentialRows(db: Database): LegacyCredentialScrubRe ); } - for (const [oldFingerprint, safeFingerprint] of fingerprintChanges) { - if (oldFingerprint === safeFingerprint) continue; - db.prepare("UPDATE baselines SET finding_fingerprint = ? WHERE finding_fingerprint = ?") - .run(safeFingerprint, oldFingerprint); - db.prepare("UPDATE llm_cache SET finding_fingerprint = ? WHERE finding_fingerprint = ?") - .run(safeFingerprint, oldFingerprint); + for (const row of baselines) { + const oldId = String(row.id); + const id = baselineIds.get(oldId)!; + const oldFingerprint = String(row.finding_fingerprint); + const safeFingerprint = fingerprintChanges.get(oldFingerprint) + ?? sanitizeFingerprintForOutput(oldFingerprint); + db.prepare( + `UPDATE baselines SET id = ?, finding_fingerprint = ?, reason = ?, created_by = ?, created_at = ? + WHERE id = ?`, + ).run( + id, + safeFingerprint, + sanitizeTextForBoundary(String(row.reason), 512), + sanitizeTextForBoundary(String(row.created_by), 256), + sanitizeTextForBoundary(String(row.created_at), 128), + oldId, + ); + } + + const cachePlans = llmCache.map((row, index) => { + const oldId = String(row.id); + const oldFingerprint = String(row.finding_fingerprint); + const baseSafeAnalysisType = sanitizeTextForBoundary(String(row.analysis_type), 128); + return { + index, + row, + oldId, + id: llmCacheIds.get(oldId)!, + oldFingerprint, + safeFingerprint: fingerprintChanges.get(oldFingerprint) + ?? sanitizeFingerprintForOutput(oldFingerprint), + oldAnalysisType: String(row.analysis_type), + baseSafeAnalysisType, + safeAnalysisType: baseSafeAnalysisType, + }; + }); + const reservedLookupKeys = new Set( + cachePlans.map((plan) => JSON.stringify([plan.oldFingerprint, plan.oldAnalysisType])), + ); + const canonicalLookupOwners = new Map(); + for (const [index, plan] of cachePlans.entries()) { + const safeKey = JSON.stringify([plan.safeFingerprint, plan.safeAnalysisType]); + const currentKey = JSON.stringify([plan.oldFingerprint, plan.oldAnalysisType]); + const existing = canonicalLookupOwners.get(safeKey); + if (existing === undefined || (currentKey === safeKey && JSON.stringify([ + cachePlans[existing].oldFingerprint, + cachePlans[existing].oldAnalysisType, + ]) !== safeKey)) { + canonicalLookupOwners.set(safeKey, index); + } + } + for (const safeKey of canonicalLookupOwners.keys()) reservedLookupKeys.add(safeKey); + for (const [index, plan] of cachePlans.entries()) { + const safeKey = JSON.stringify([plan.safeFingerprint, plan.safeAnalysisType]); + if (canonicalLookupOwners.get(safeKey) === index) continue; + let attempt = 0; + let collisionSafeType = opaqueIdentifierForStorage( + `${plan.oldFingerprint}\0${plan.oldAnalysisType}\0${plan.oldId}`, + "ANALYSIS-TYPE", + attempt, + ); + let collisionKey = JSON.stringify([plan.safeFingerprint, collisionSafeType]); + while (reservedLookupKeys.has(collisionKey)) { + collisionSafeType = opaqueIdentifierForStorage( + `${plan.oldFingerprint}\0${plan.oldAnalysisType}\0${plan.oldId}`, + "ANALYSIS-TYPE", + ++attempt, + ); + collisionKey = JSON.stringify([plan.safeFingerprint, collisionSafeType]); + } + plan.safeAnalysisType = collisionSafeType; + reservedLookupKeys.add(collisionKey); + } + + // Move colliding legacy keys away from every canonical target first. This + // preserves every cache row without violating the production unique index. + for (const plan of cachePlans) { + const canonicalKey = JSON.stringify([ + plan.safeFingerprint, + plan.baseSafeAnalysisType, + ]); + if (canonicalLookupOwners.get(canonicalKey) === plan.index) continue; + db.prepare("UPDATE llm_cache SET analysis_type = ? WHERE id = ?") + .run(plan.safeAnalysisType, plan.oldId); + } + + for (const plan of cachePlans) { + const { row, oldId, id, safeFingerprint, safeAnalysisType } = plan; + db.prepare( + `UPDATE llm_cache SET id = ?, finding_fingerprint = ?, analysis_type = ?, result = ?, + model = ?, created_at = ? WHERE id = ?`, + ).run( + id, + safeFingerprint, + safeAnalysisType, + sanitizeCachedResultText(String(row.result)), + sanitizeTextForBoundary(String(row.model), 256), + sanitizeTextForBoundary(String(row.created_at), 128), + oldId, + ); } for (const [oldId, id] of scanIds) { @@ -274,7 +398,10 @@ export function scrubLegacyCredentialRows(db: Database): LegacyCredentialScrubRe if ((db.prepare("PRAGMA foreign_key_check").all() as unknown[]).length > 0) { throw new Error("legacy credential scrub violated referential integrity"); } - return { findingIds, projectIds, ruleIds, scanIds }; + if (hasAnyUnsafeRows(db)) { + throw new Error("legacy credential scrub left unsafe rows"); + } + return { baselineIds, findingIds, llmCacheIds, projectIds, ruleIds, scanIds }; }); try { diff --git a/src/db/llm-cache.test.ts b/src/db/llm-cache.test.ts index 45422ad..3c7c906 100644 --- a/src/db/llm-cache.test.ts +++ b/src/db/llm-cache.test.ts @@ -1,6 +1,10 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { getCurrentTestDb, setupTestDb } from "./test-helpers.js"; import { cacheAnalysis, getCachedAnalysis } from "./llm-cache.js"; +import { + sanitizeFingerprintForOutput, + sanitizeTextForBoundary, +} from "../lib/finding-safety.js"; describe("LLM cache boundary", () => { let cleanup: () => void; @@ -35,4 +39,101 @@ describe("LLM cache boundary", () => { expect(JSON.stringify(getCachedAnalysis("safe-fingerprint", "explain"))).not.toContain(syntheticSecret); expect(JSON.stringify(db.prepare("SELECT result FROM llm_cache WHERE id = 'legacy'").get())).not.toContain(syntheticSecret); }); + + test("durably scrubs every production cache string on legacy read", () => { + const marker = `gh${"o"}_${"CacheLegacyMarker_".repeat(3)}`; + const db = getCurrentTestDb(); + db.prepare( + `INSERT INTO llm_cache + (id, finding_fingerprint, analysis_type, result, model, tokens_used, created_at) + VALUES (?, ?, ?, ?, ?, 1, ?)`, + ).run( + marker, + marker, + `analysis-${marker}`, + JSON.stringify({ text: marker, nested: { marker } }), + `model-${marker}`, + marker, + ); + + expect(JSON.stringify(getCachedAnalysis(marker, `analysis-${marker}`))).not.toContain(marker); + expect(JSON.stringify(db.prepare("SELECT * FROM llm_cache").all())).not.toContain(marker); + }); + + test("converts invalid legacy result text to a deterministic safe JSON object", () => { + const marker = `gh${"s"}_${"InvalidCacheResult_".repeat(3)}`; + const db = getCurrentTestDb(); + db.prepare( + `INSERT INTO llm_cache + (id, finding_fingerprint, analysis_type, result, model, tokens_used, created_at) + VALUES ('invalid-result', 'safe-fingerprint', 'explain', ?, 'safe-model', 1, 'now')`, + ).run(`not-json:${marker}`); + + const result = getCachedAnalysis("safe-fingerprint", "explain"); + expect(result).toEqual({ + legacy_cache_value: expect.stringContaining("[REDACTED]"), + }); + const raw = db.prepare("SELECT result FROM llm_cache WHERE id = 'invalid-result'").get() as { + result: string; + }; + expect(() => JSON.parse(raw.result)).not.toThrow(); + expect(raw.result).not.toContain(marker); + }); + + test("repairs invalid non-credential cache result text on the selected row", () => { + const db = getCurrentTestDb(); + db.prepare( + `INSERT INTO llm_cache + (id, finding_fingerprint, analysis_type, result, model, tokens_used, created_at) + VALUES ('invalid-safe-result', 'safe-fingerprint', 'triage', 'not valid json', 'safe', 0, 'now')`, + ).run(); + + expect(getCachedAnalysis("safe-fingerprint", "triage")).toEqual({ + legacy_cache_value: "not valid json", + }); + const raw = db.prepare("SELECT result FROM llm_cache WHERE id = 'invalid-safe-result'").get() as { + result: string; + }; + expect(JSON.parse(raw.result)).toEqual({ legacy_cache_value: "not valid json" }); + }); + + test("does not run the global scrub for an already-safe cache hit", () => { + const marker = `gh${"r"}_${"UnrelatedBaseline_".repeat(3)}`; + const db = getCurrentTestDb(); + db.prepare( + `INSERT INTO baselines (id, finding_fingerprint, reason, created_by, created_at) + VALUES ('unsafe-unrelated', 'safe-fingerprint', ?, 'safe', 'now')`, + ).run(marker); + db.prepare( + `INSERT INTO llm_cache + (id, finding_fingerprint, analysis_type, result, model, tokens_used, created_at) + VALUES ('safe-cache', 'safe-fingerprint', 'explain', '{"ok":true}', 'safe', 0, 'now')`, + ).run(); + + expect(getCachedAnalysis("safe-fingerprint", "explain")).toEqual({ ok: true }); + expect(JSON.stringify(db.prepare("SELECT * FROM baselines").all())).toContain(marker); + }); + + test("scrubs an exact raw legacy key even when its safe key already exists", () => { + const marker = `gh${"p"}_${"CacheLookupCollision_".repeat(3)}`; + const analysisType = `analysis-${marker}`; + const safeFingerprint = sanitizeFingerprintForOutput(marker); + const safeAnalysisType = sanitizeTextForBoundary(analysisType, 128); + const db = getCurrentTestDb(); + db.prepare( + `INSERT INTO llm_cache + (id, finding_fingerprint, analysis_type, result, model, tokens_used, created_at) + VALUES ('canonical', ?, ?, '{"owner":"canonical"}', 'safe', 0, 'now')`, + ).run(safeFingerprint, safeAnalysisType); + db.prepare( + `INSERT INTO llm_cache + (id, finding_fingerprint, analysis_type, result, model, tokens_used, created_at) + VALUES ('raw-legacy', ?, ?, '{"owner":"legacy"}', 'safe', 0, 'now')`, + ).run(marker, analysisType); + + expect(getCachedAnalysis(marker, analysisType)).toEqual({ owner: "canonical" }); + const rows = db.prepare("SELECT * FROM llm_cache").all(); + expect(rows).toHaveLength(2); + expect(JSON.stringify(rows)).not.toContain(marker); + }); }); diff --git a/src/db/llm-cache.ts b/src/db/llm-cache.ts index 998e1e5..b5082a5 100644 --- a/src/db/llm-cache.ts +++ b/src/db/llm-cache.ts @@ -1,40 +1,63 @@ import crypto from "crypto"; import { getDb } from "./database.js"; -import { sanitizeTextForBoundary, sanitizeValueForBoundary } from "../lib/finding-safety.js"; +import { + sanitizeFingerprintForOutput, + sanitizeTextForBoundary, + sanitizeValueForBoundary, +} from "../lib/finding-safety.js"; +import { + legacyRowContainsCredential, + sanitizeCachedResultText, + scrubLegacyCredentialRows, +} from "./legacy-credential-scrub.js"; + +interface CacheRow extends Record { + id: string; + result: string; + finding_fingerprint: string; + analysis_type: string; +} export function getCachedAnalysis( fingerprint: string, analysis_type: string ): Record | null { const db = getDb(); - const safeFingerprint = sanitizeTextForBoundary(fingerprint, 256); + const safeFingerprint = sanitizeFingerprintForOutput(fingerprint); const safeAnalysisType = sanitizeTextForBoundary(analysis_type, 128); - const stmt = db.prepare( - `SELECT id, result, finding_fingerprint, analysis_type FROM llm_cache - WHERE (finding_fingerprint = ? AND analysis_type = ?) - OR (finding_fingerprint = ? AND analysis_type = ?) - LIMIT 1` + const rawStmt = db.prepare( + `SELECT * FROM llm_cache WHERE finding_fingerprint = ? AND analysis_type = ? LIMIT 1`, ); - const row = stmt.get(safeFingerprint, safeAnalysisType, fingerprint, analysis_type) as { - id: string; - result: string; - finding_fingerprint: string; - analysis_type: string; - } | undefined; + const safeStmt = db.prepare( + `SELECT * FROM llm_cache WHERE finding_fingerprint = ? AND analysis_type = ? LIMIT 1`, + ); + const readRawRow = () => rawStmt.get(fingerprint, analysis_type) as CacheRow | undefined; + const readSafeRow = () => safeStmt.get( + safeFingerprint, + safeAnalysisType, + ) as CacheRow | undefined; + let row = readRawRow(); + if (row && legacyRowContainsCredential(row)) { + scrubLegacyCredentialRows(db); + row = readSafeRow(); + } else if (!row) { + row = readSafeRow(); + } + if (row && legacyRowContainsCredential(row)) { + scrubLegacyCredentialRows(db); + row = readSafeRow(); + } if (!row) return null; - const safeResult = sanitizeValueForBoundary(JSON.parse(row.result) as Record); - const safeResultJson = JSON.stringify(safeResult); + const safeResultJson = sanitizeCachedResultText(row.result); + const safeResult = JSON.parse(safeResultJson) as Record; if ( - row.result !== safeResultJson || - row.finding_fingerprint !== safeFingerprint || - row.analysis_type !== safeAnalysisType + row.result !== safeResultJson ) { try { - db.prepare( - "UPDATE llm_cache SET finding_fingerprint = ?, analysis_type = ?, result = ? WHERE id = ?", - ).run(safeFingerprint, safeAnalysisType, safeResultJson, row.id); + db.prepare("UPDATE llm_cache SET result = ? WHERE id = ?") + .run(safeResultJson, row.id); } catch { - // Return remains sanitized when legacy/read-only cache rows cannot change. + throw new Error("Unable to durably sanitize legacy LLM cache data"); } } return safeResult; @@ -49,7 +72,7 @@ export function cacheAnalysis( ): void { const db = getDb(); const now = new Date().toISOString(); - const safeFingerprint = sanitizeTextForBoundary(fingerprint, 256); + const safeFingerprint = sanitizeFingerprintForOutput(fingerprint); const safeAnalysisType = sanitizeTextForBoundary(analysis_type, 128); const resultJson = JSON.stringify(sanitizeValueForBoundary(result)); const safeModel = sanitizeTextForBoundary(model, 256); @@ -69,7 +92,7 @@ export function cacheAnalysis( export function invalidateCache(fingerprint?: string): void { const db = getDb(); if (fingerprint) { - const safeFingerprint = sanitizeTextForBoundary(fingerprint, 256); + const safeFingerprint = sanitizeFingerprintForOutput(fingerprint); const stmt = db.prepare(`DELETE FROM llm_cache WHERE finding_fingerprint = ? OR finding_fingerprint = ?`); stmt.run(safeFingerprint, fingerprint); } else { diff --git a/src/mcp/tools/output-safety.test.ts b/src/mcp/tools/output-safety.test.ts index f8c723d..d88dd7a 100644 --- a/src/mcp/tools/output-safety.test.ts +++ b/src/mcp/tools/output-safety.test.ts @@ -73,6 +73,29 @@ describe("MCP credential output safety", () => { expect(output).toContain("[REDACTED]"); }); + test("baseline_findings never persists an MCP-provided credential reason", async () => { + const syntheticSecret = "ghp_" + "SYNTHETICONLYABCDEFGHIJKLMNOPQRSTUVWXYZ12"; + const db = getCurrentTestDb(); + const scanId = (db.prepare("SELECT scan_id FROM findings WHERE id = ?").get(findingId) as { + scan_id: string; + }).scan_id; + const tools = captureTools((server) => { + registerFindingTools(server, jsonResult, () => { + throw new Error("baseline must not read source context"); + }); + }); + + const result = await tools.get("baseline_findings")?.({ + scan_id: scanId, + reason: `accept because the synthetic marker is ${syntheticSecret}`, + }); + + expect(JSON.stringify(result)).not.toContain(syntheticSecret); + expect(JSON.stringify(db.prepare("SELECT * FROM baselines").all())).not.toContain( + syntheticSecret, + ); + }); + test("credential LLM tools short-circuit before source context is read", async () => { let contextReads = 0; const tools = captureTools((server) => { From fafc3ad948b738f9da88618c670ec7d5537ee357 Mon Sep 17 00:00:00 2001 From: Andrei Hasna Date: Thu, 16 Jul 2026 14:35:27 +0300 Subject: [PATCH 7/8] fix: fail closed during database initialization --- src/db/database.test.ts | 232 +++++++++++++++++++++++++++++++++++++++- src/db/database.ts | 68 ++++++++++-- 2 files changed, 288 insertions(+), 12 deletions(-) diff --git a/src/db/database.test.ts b/src/db/database.test.ts index 7de2041..a251b48 100644 --- a/src/db/database.test.ts +++ b/src/db/database.test.ts @@ -1,5 +1,20 @@ import { describe, expect, test } from "bun:test"; -import { getTestDb } from "./database.js"; +import { Database } from "bun:sqlite"; +import { mkdtempSync, rmSync } from "fs"; +import { tmpdir } from "os"; +import { join } from "path"; +import { closeDb, getDb, getTestDb } from "./database.js"; + +const SAFE_INIT_ERROR = "Unable to initialize Shield database safely"; + +function captureErrorMessage(fn: () => unknown): string { + try { + fn(); + } catch (error) { + return error instanceof Error ? error.message : String(error); + } + throw new Error("Expected operation to throw"); +} function tableNames(db: ReturnType): string[] { return db @@ -64,4 +79,219 @@ describe("database", () => { db.close(); }); + + test("does not reuse a partially initialized connection after credential scrub failure", () => { + const originalSecurityDb = process.env.SECURITY_DB; + const directory = mkdtempSync(join(tmpdir(), "shield-init-fail-closed-")); + const path = join(directory, "shield.db"); + const marker = `gh${"o"}_${"InitFailClosed_".repeat(3)}`; + + try { + closeDb(); + process.env.SECURITY_DB = path; + + // Establish the current schema, then add a legacy row whose scrub is + // forced to fail during the next singleton initialization. + getDb(); + closeDb(); + const fixture = new Database(path); + fixture.prepare( + "INSERT INTO projects (id, name, path, created_at, updated_at) VALUES ('project', ?, '/safe', 'now', 'now')", + ).run(marker); + fixture.exec(` + CREATE TRIGGER reject_legacy_project_update + BEFORE UPDATE ON projects + BEGIN + SELECT RAISE(ABORT, 'synthetic write rejection'); + END; + `); + fixture.close(); + + const message = captureErrorMessage(() => getDb()); + expect(message).toBe(SAFE_INIT_ERROR); + expect(message).not.toContain(marker); + const retryMessage = captureErrorMessage(() => getDb()); + expect(retryMessage).toBe(SAFE_INIT_ERROR); + expect(retryMessage).not.toContain(marker); + + // Remove only the injected failure. A subsequent getDb() must open a + // fresh connection and retry the scrub before exposing the database. + const repaired = new Database(path); + repaired.exec("DROP TRIGGER reject_legacy_project_update"); + repaired.close(); + + const recovered = getDb(); + expect(JSON.stringify(recovered.prepare("SELECT * FROM projects").all())).not.toContain(marker); + expect(recovered.prepare("PRAGMA foreign_key_check").all()).toEqual([]); + } finally { + closeDb(); + if (originalSecurityDb === undefined) delete process.env.SECURITY_DB; + else process.env.SECURITY_DB = originalSecurityDb; + rmSync(directory, { recursive: true, force: true }); + } + }); + + test("sanitizes constructor and migration initialization errors", () => { + const originalSecurityDb = process.env.SECURITY_DB; + const directory = mkdtempSync(join(tmpdir(), "shield-init-errors-")); + const malformedPath = join(directory, "malformed.db"); + + try { + closeDb(); + process.env.SECURITY_DB = directory; + const constructorMessage = captureErrorMessage(() => getDb()); + expect(constructorMessage).toBe(SAFE_INIT_ERROR); + expect(constructorMessage).not.toContain(directory); + + const malformed = new Database(malformedPath); + malformed.exec("CREATE TABLE _migrations (id INTEGER PRIMARY KEY)"); + malformed.close(); + + process.env.SECURITY_DB = malformedPath; + const migrationMessage = captureErrorMessage(() => getDb()); + expect(migrationMessage).toBe(SAFE_INIT_ERROR); + expect(migrationMessage).not.toContain("_migrations"); + expect(migrationMessage).not.toContain(malformedPath); + } finally { + closeDb(); + if (originalSecurityDb === undefined) delete process.env.SECURITY_DB; + else process.env.SECURITY_DB = originalSecurityDb; + rmSync(directory, { recursive: true, force: true }); + } + }); + + test("closes and retries a fresh connection after a post-init callback failure", () => { + const directory = mkdtempSync(join(tmpdir(), "shield-callback-fail-closed-")); + const path = join(directory, "shield.db"); + const marker = `sk_${"CallbackFailure_".repeat(3)}`; + const moduleUrl = new URL("./database.ts", import.meta.url).href; + const program = ` + import { closeDb, getDb, onDbInit } from ${JSON.stringify(moduleUrl)}; + process.env.SECURITY_DB = ${JSON.stringify(path)}; + const marker = ${JSON.stringify(marker)}; + let attempts = 0; + let failedHandle; + onDbInit(() => { + attempts++; + const current = getDb(); + if (attempts === 1) { + failedHandle = current; + throw new Error(marker); + } + if (current === failedHandle) throw new Error("callback reused failed handle"); + }); + + let firstMessage = ""; + try { getDb(); } catch (error) { + firstMessage = error instanceof Error ? error.message : String(error); + } + if (firstMessage !== ${JSON.stringify(SAFE_INIT_ERROR)} || firstMessage.includes(marker)) process.exit(11); + if (!failedHandle) process.exit(12); + try { + failedHandle.exec("SELECT 1"); + process.exit(13); + } catch {} + + const recovered = getDb(); + if (attempts !== 2 || recovered === failedHandle) process.exit(14); + if ((recovered.prepare("SELECT 1 AS value").get()).value !== 1) process.exit(15); + + let immediateAttempts = 0; + let immediateFailedHandle; + let immediateMessage = ""; + try { + onDbInit(() => { + immediateAttempts++; + const current = getDb(); + if (immediateAttempts === 1) { + immediateFailedHandle = current; + throw new Error(marker); + } + if (current === immediateFailedHandle) throw new Error("immediate callback reused failed handle"); + }); + } catch (error) { + immediateMessage = error instanceof Error ? error.message : String(error); + } + if (immediateMessage !== ${JSON.stringify(SAFE_INIT_ERROR)} || immediateMessage.includes(marker)) process.exit(16); + if (!immediateFailedHandle) process.exit(17); + try { + immediateFailedHandle.exec("SELECT 1"); + process.exit(18); + } catch {} + + const recoveredAgain = getDb(); + if (immediateAttempts !== 2 || recoveredAgain === immediateFailedHandle) process.exit(19); + if ((recoveredAgain.prepare("SELECT 1 AS value").get()).value !== 1) process.exit(20); + closeDb(); + `; + + try { + const child = Bun.spawnSync({ + cmd: [process.execPath, "-e", program], + env: { SECURITY_DB: path }, + stderr: "pipe", + stdout: "pipe", + }); + const output = `${child.stdout.toString()}${child.stderr.toString()}`; + expect(output).not.toContain(marker); + expect(child.exitCode).toBe(0); + } finally { + rmSync(directory, { recursive: true, force: true }); + } + }); + + test("closes a recursively published replacement when the outer callback initialization fails", () => { + const directory = mkdtempSync(join(tmpdir(), "shield-callback-replacement-")); + const path = join(directory, "shield.db"); + const marker = `sk_${"ReplacementFailure_".repeat(3)}`; + const moduleUrl = new URL("./database.ts", import.meta.url).href; + const program = ` + import { closeDb, getDb, onDbInit } from ${JSON.stringify(moduleUrl)}; + process.env.SECURITY_DB = ${JSON.stringify(path)}; + const marker = ${JSON.stringify(marker)}; + let attempts = 0; + let originalHandle; + let replacementHandle; + onDbInit(() => { + attempts++; + const current = getDb(); + if (attempts !== 1) return; + originalHandle = current; + closeDb(); + replacementHandle = getDb(); + }); + + let outerMessage = ""; + try { getDb(); } catch (error) { + outerMessage = error instanceof Error ? error.message : String(error); + } + if (outerMessage !== ${JSON.stringify(SAFE_INIT_ERROR)} || outerMessage.includes(marker)) process.exit(21); + if (!originalHandle || !replacementHandle || originalHandle === replacementHandle) process.exit(22); + for (const failed of [originalHandle, replacementHandle]) { + try { + failed.exec("SELECT 1"); + process.exit(23); + } catch {} + } + + const recovered = getDb(); + if (attempts !== 3 || recovered === originalHandle || recovered === replacementHandle) process.exit(24); + if ((recovered.prepare("SELECT 1 AS value").get()).value !== 1) process.exit(25); + closeDb(); + `; + + try { + const child = Bun.spawnSync({ + cmd: [process.execPath, "-e", program], + env: { SECURITY_DB: path }, + stderr: "pipe", + stdout: "pipe", + }); + const output = `${child.stdout.toString()}${child.stderr.toString()}`; + expect(output).not.toContain(marker); + expect(child.exitCode).toBe(0); + } finally { + rmSync(directory, { recursive: true, force: true }); + } + }); }); diff --git a/src/db/database.ts b/src/db/database.ts index e4879cc..be79e9c 100644 --- a/src/db/database.ts +++ b/src/db/database.ts @@ -5,6 +5,7 @@ import { homedir } from "os"; import { scrubLegacyCredentialRows } from "./legacy-credential-scrub.js"; let _db: Database | null = null; +const DATABASE_INIT_ERROR = "Unable to initialize Shield database safely"; function homeDir(): string { return process.env.HOME || process.env.USERPROFILE || homedir(); @@ -49,9 +50,40 @@ function getDbPath(): string { let _postInitCallbacks: Array<() => void> = []; let _initialized = false; +function failDatabaseInitialization(db: Database | null): never { + // A callback may have closed the original candidate and recursively + // published a replacement. Detach first, then close every distinct handle + // involved in the failed initialization so no live singleton can escape. + const published = _db; + _db = null; + _initialized = false; + const failedHandles = new Set(); + if (db) failedHandles.add(db); + if (published) failedHandles.add(published); + for (const handle of failedHandles) { + try { handle.close(); } catch {} + } + // Do not repeat paths, legacy values, callback text, or SQLite diagnostics. + throw new Error(DATABASE_INIT_ERROR); +} + export function onDbInit(cb: () => void): void { _postInitCallbacks.push(cb); - if (_initialized) cb(); + if (!_initialized) return; + const db = _db; + if (!db) { + // closeDb() intentionally releases the singleton. Defer the newly + // registered callback until the next successful initialization. + _initialized = false; + return; + } + try { + cb(); + if (_db !== db) throw new Error("database connection changed during initialization"); + db.exec("SELECT 1"); + } catch { + failDatabaseInitialization(db); + } } export function getDb(): Database { @@ -69,17 +101,31 @@ export function getDb(): Database { if (_db) return _db; const dbPath = getDbPath(); mkdirSync(dirname(dbPath), { recursive: true }); - _db = new Database(dbPath); - _db.exec("PRAGMA journal_mode = WAL"); - _db.exec("PRAGMA foreign_keys = ON"); - _db.exec("PRAGMA busy_timeout = 5000"); - runMigrations(_db); - scrubLegacyCredentialRows(_db); - if (!_initialized) { - _initialized = true; - for (const cb of _postInitCallbacks) cb(); + let db: Database | null = null; + try { + db = new Database(dbPath); + db.exec("PRAGMA journal_mode = WAL"); + db.exec("PRAGMA foreign_keys = ON"); + db.exec("PRAGMA busy_timeout = 5000"); + runMigrations(db); + scrubLegacyCredentialRows(db); + // Publish before callbacks so a callback can safely call getDb() without + // recursively opening another connection. Initialization is not marked + // complete until every callback returns and the candidate remains live. + _db = db; + if (!_initialized) { + for (const cb of _postInitCallbacks) cb(); + if (_db !== db) throw new Error("database connection changed during initialization"); + db.exec("SELECT 1"); + _initialized = true; + } + return db; + } catch { + // Never publish or retain a connection whose constructor, migrations, + // credential scrub, or post-init callbacks did not finish. A later call + // must reconnect and retry every boundary before state becomes observable. + failDatabaseInitialization(db); } - return _db; } export function closeDb(): void { From 6bece3a6749c759b2c158e62b3955de2c0a77c08 Mon Sep 17 00:00:00 2001 From: Andrei Hasna Date: Thu, 16 Jul 2026 15:00:37 +0300 Subject: [PATCH 8/8] fix: sanitize database path preparation failures --- src/db/database.test.ts | 88 ++++++++++++++++++++++++++++++++++++++- src/db/database.ts | 10 ++--- src/db/hasna-home.test.ts | 2 +- 3 files changed, 93 insertions(+), 7 deletions(-) diff --git a/src/db/database.test.ts b/src/db/database.test.ts index a251b48..311b1d7 100644 --- a/src/db/database.test.ts +++ b/src/db/database.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from "bun:test"; import { Database } from "bun:sqlite"; -import { mkdtempSync, rmSync } from "fs"; +import { mkdtempSync, rmSync, writeFileSync } from "fs"; import { tmpdir } from "os"; import { join } from "path"; import { closeDb, getDb, getTestDb } from "./database.js"; @@ -160,6 +160,92 @@ describe("database", () => { } }); + test("sanitizes storage-mode resolution failures and recovers on retry", () => { + const directory = mkdtempSync(join(tmpdir(), "shield-storage-mode-init-")); + const path = join(directory, "shield.db"); + const marker = `invalid-mode-${"synthetic-marker-".repeat(3)}`; + const moduleUrl = new URL("./database.ts", import.meta.url).href; + const program = ` + import { closeDb, getDb } from ${JSON.stringify(moduleUrl)}; + const marker = ${JSON.stringify(marker)}; + process.env.SECURITY_DB = ${JSON.stringify(path)}; + process.env.HASNA_SHIELD_STORAGE_MODE = marker; + + const capture = () => { + try { getDb(); } catch (error) { + return error instanceof Error ? error.message : String(error); + } + return "did not fail"; + }; + for (const message of [capture(), capture()]) { + if (message !== ${JSON.stringify(SAFE_INIT_ERROR)} || message.includes(marker)) process.exit(31); + } + + process.env.HASNA_SHIELD_STORAGE_MODE = "local"; + const recovered = getDb(); + if ((recovered.prepare("SELECT 1 AS value").get()).value !== 1) process.exit(32); + closeDb(); + `; + + try { + const child = Bun.spawnSync({ + cmd: [process.execPath, "-e", program], + env: { ...process.env, SECURITY_DB: path }, + stderr: "pipe", + stdout: "pipe", + }); + expect(`${child.stdout.toString()}${child.stderr.toString()}`).not.toContain(marker); + expect(child.exitCode).toBe(0); + } finally { + rmSync(directory, { recursive: true, force: true }); + } + }); + + test("sanitizes path preparation failures and recovers after the parent is repaired", () => { + const directory = mkdtempSync(join(tmpdir(), "shield-path-preparation-init-")); + const blockedParent = join(directory, "blocked-parent"); + const path = join(blockedParent, "shield.db"); + writeFileSync(blockedParent, "not a directory", "utf-8"); + const moduleUrl = new URL("./database.ts", import.meta.url).href; + const program = ` + import { mkdirSync, rmSync } from "fs"; + import { closeDb, getDb } from ${JSON.stringify(moduleUrl)}; + const unsafePath = ${JSON.stringify(path)}; + process.env.SECURITY_DB = unsafePath; + process.env.HASNA_SHIELD_STORAGE_MODE = "local"; + + const capture = () => { + try { getDb(); } catch (error) { + return error instanceof Error ? error.message : String(error); + } + return "did not fail"; + }; + for (const message of [capture(), capture()]) { + if (message !== ${JSON.stringify(SAFE_INIT_ERROR)} || message.includes(unsafePath)) process.exit(41); + } + + rmSync(${JSON.stringify(blockedParent)}); + mkdirSync(${JSON.stringify(blockedParent)}); + const recovered = getDb(); + if ((recovered.prepare("SELECT 1 AS value").get()).value !== 1) process.exit(42); + closeDb(); + `; + + try { + const child = Bun.spawnSync({ + cmd: [process.execPath, "-e", program], + env: { ...process.env, SECURITY_DB: path }, + stderr: "pipe", + stdout: "pipe", + }); + const output = `${child.stdout.toString()}${child.stderr.toString()}`; + expect(output).not.toContain(path); + expect(child.exitCode).toBe(0); + } finally { + rmSync(directory, { recursive: true, force: true }); + } + }); + test("closes and retries a fresh connection after a post-init callback failure", () => { const directory = mkdtempSync(join(tmpdir(), "shield-callback-fail-closed-")); const path = join(directory, "shield.db"); diff --git a/src/db/database.ts b/src/db/database.ts index be79e9c..c817f15 100644 --- a/src/db/database.ts +++ b/src/db/database.ts @@ -99,10 +99,10 @@ export function getDb(): Database { } } if (_db) return _db; - const dbPath = getDbPath(); - mkdirSync(dirname(dbPath), { recursive: true }); let db: Database | null = null; try { + const dbPath = getDbPath(); + mkdirSync(dirname(dbPath), { recursive: true }); db = new Database(dbPath); db.exec("PRAGMA journal_mode = WAL"); db.exec("PRAGMA foreign_keys = ON"); @@ -121,9 +121,9 @@ export function getDb(): Database { } return db; } catch { - // Never publish or retain a connection whose constructor, migrations, - // credential scrub, or post-init callbacks did not finish. A later call - // must reconnect and retry every boundary before state becomes observable. + // Never publish or retain a connection whose path preparation, constructor, + // migrations, credential scrub, or post-init callbacks did not finish. A + // later call must retry every boundary before state becomes observable. failDatabaseInitialization(db); } } diff --git a/src/db/hasna-home.test.ts b/src/db/hasna-home.test.ts index 20901e7..fbb824e 100644 --- a/src/db/hasna-home.test.ts +++ b/src/db/hasna-home.test.ts @@ -107,7 +107,7 @@ describe("hasna home database", () => { delete process.env.HASNA_SECURITY_STORAGE_MODE; delete process.env.SECURITY_DB; closeDb(); - expect(() => getDb()).toThrow(/Only local SQLite storage is supported/); + expect(() => getDb()).toThrow("Unable to initialize Shield database safely"); } finally { closeDb(); if (originalShieldMode === undefined) delete process.env.HASNA_SHIELD_STORAGE_MODE;