Fix secret scan output safety

[andrei-hasna]

Summary

• make repository files the only default source for shield secrets and MCP secret scans
• require explicit opt-in for git history, process snapshots, and tmux context
• redact credential findings before persistence, DB readback, CLI/JSON/SARIF/MCP/API output, and exclude them before LLM context reads
• add migration/upgrade guidance without rotating credentials or destructively rewriting historical state

Incident boundary

This mitigates the output-safety defect tracked in the internal Shield incident. It does not merge, publish, install, rotate credentials, delete transcripts, or claim the incident is resolved.

Verification

• bun test — 345 pass, 0 fail, 730 assertions
• bun run typecheck
• bun run build
• bun run no-cloud:source
• bun run no-cloud:pack
• package archive shield fleet-package scan — 0 findings
• git diff --cached --check
• file-only workspace secret scan (redacted; only pre-existing synthetic scanner fixtures)

Recorded residuals

• bun audit reports pre-existing transitive advisories: 6 high, 23 moderate, 1 low
• bun run lint is unavailable because the repository currently lacks the configured Biome binary/dependency

Independent adversarial exact-head review and hosted CI are required before any merge or release.

[andrei-hasna]

Adversarial defensive review A — BLOCK

Exact immutable head reviewed: 3efc963a3765dcc0efc119dd537eaee9c7a71f40.

Blocking findings

1. P1 — default comprehensive CLI/library/MCP/API scans still opt into git history. DEFAULT_CONFIG.enabled_scanners is Object.values(ScannerType) (src/types/index.ts:178-180), runAllScanners executes every registered scanner and registers gitHistoryScanner (src/scanners/index.ts:33-50,67-75), and MCP scan_repo defaults to all scanner types / runAllScanners (src/mcp/tools/scan.ts:52-69). The normal CLI resolves that default config (src/cli/commands/scan.ts:38-42), and REST follows the same all-scanner path. A synthetic introspection probe confirmed both the default config and public all-scanner registry include git history. Thus only the focused shield secrets path is opt-in; ordinary default scans still cross the historical boundary without explicit authorization.

2. P1 — a non-credential finding can send adjacent credential text to the LLM and persist it in SQLite. Classification checks only finding metadata, then the analyzers interpolate the supplied raw context (src/llm/analyzer.ts:19-37; same pattern in explainer/fixer/triager). CLI, MCP, and REST read surrounding source for any finding that is not classified as a credential. A synthetic config finding with an adjacent fake credential was classified non-sensitive, retained that fake value in the persistence form, and a mocked provider capture confirmed the fake value was present in the outbound request body. No network or real credential was used. The boundary must inspect/redact or refuse the actual context, not only the finding metadata.

3. P1 — credential redaction leaves attacker-controlled location/rule metadata credential-bearing. sanitizeFindingForPersistence and sanitizeFindingForOutput preserve raw rule_id and preserve file after only length/control normalization; the replacement message interpolates the normalized but unredacted rule ID (src/lib/finding-safety.ts:17-20,32-51). A synthetic secret scanner finding whose filename contained a fake credential retained that value in both persistence and output. A synthetic rule ID behaved the same. This reaches DB read/write, JSON/SARIF/terminal, API, and MCP through the central sanitizer and also affects legacy rows. Redact sensitive values across all serialized fields while preserving stable non-sensitive classification metadata separately.

4. P1 — file-only scanning is still fail-open on traversal/read failures. Directory traversal returns silently on readdirSync failure and per-file reads are silently skipped (src/scanners/secrets.ts:37-43,817-824). scanSecretExposure checks only existence and then returns a clean result (src/lib/secret-exposure.ts:247-280). A synthetic fake-secret regular-file target invoked with --files-only --json returned exit 0 and total: 0 instead of failing. Unreadable directories/files can likewise be omitted and reported clean. Validate the target type and propagate or aggregate traversal/read failures so a clean result proves the requested file set was actually scanned.

Positive evidence

• Exact head and hosted CI verified; CI run 29448979260 is green.
• bun test: 355 pass, 0 fail, 756 expectations.
• bun run typecheck: pass.
• bun run check:secrets: pass, 0 OSS-policy violations.
• bun run no-cloud:source: pass.
• bun run no-cloud:pack: pass, 66 packed files.
• npm pack --dry-run --json: pass; package shape inspected.
• git diff --check origin/main...HEAD: pass.
• Main OSS secret-policy merge was included and its check remains green.
• Focused source resolution is safe: defaults off, each positive opt-in works, --files-only wins over opt-ins, and --repo-only blocks live sources.

Release residuals

• The package remains 0.1.25, which is already the registry version and is described by the new migration warning as historically unsafe; a later release step must bump/version this fix before publish.
• Hosted CI does not execute build/no-cloud pack gates, although both passed locally at this exact head.
• Pre-existing audit advisories remain outside this diff.

Verdict: BLOCK at exact SHA 3efc963a3765dcc0efc119dd537eaee9c7a71f40. Re-review a new immutable head after the four P1 paths have regression coverage.

[andrei-hasna]

P1 remediation ready for exact-head re-review

New immutable head: 18d1741971a5f7f2c9d7c5f07df83dedff9a0a2f

The four blocking paths from review A are covered:

1. Default CLI/library/MCP/REST aggregate scans are file-only; history and host/system sources require dedicated per-invocation opt-ins.
2. Arbitrary adjacent context and provider/cache payloads are sanitized independently of finding classification.
3. Credential-bearing rule IDs, locations, project metadata, persisted findings, legacy rows, and serialized output are replaced with stable opaque redactions.
4. Regular-file focused scans work and stat/traversal/read/symlink failures fail closed without echoing unsafe path/error data.

Additional ambient reads found during remediation were gated too: IOC host/Python discovery and lockfile history inspection.

Exact-head evidence:

• Hosted CI run 29451168359: success
• bun test --parallel=1: 376 pass, 0 fail, 816 expectations
• bun run typecheck: pass
• bun run build: pass (library, declarations, CLI, MCP, server, SDK, dashboard)
• bun run check:secrets: pass, 0 violations
• bun run no-cloud:source: pass
• git diff --cached --check / git diff --check: pass before commit

All regression probes use synthetic fixtures. No release, publish, install, deploy, merge, credential access, or live-source scan was performed. Package version remains 0.1.25; release/versioning remains a separate blocked step.

[andrei-hasna]

Adversarial defensive re-review A2 — BLOCK

Exact immutable head reviewed: 18d1741971a5f7f2c9d7c5f07df83dedff9a0a2f.
Hosted CI: 29451168359, success at that exact head.

Blocking finding

1. P1, high confidence — scanner/sanitizer credential vocabularies disagree, reopening every repaired output boundary.

   Location: src/scanners/secrets.ts:592-608 accepts GitHub token bodies containing underscores, while src/lib/finding-safety.ts:15-31 uses a separate narrower GitHub pattern that excludes underscores for ghp_/gho_/ghs_/ghr_; the entropy fallback at lines 51-64 does not cover low-entropy credential-shaped values.

   Attack path: a scanner-recognized credential-shaped value with an underscore-rich body appears in adjacent context, a file/project path, a rule ID, suppression/analysis text, or a non-secret finding → containsCredentialLikeText returns false → sanitizeTextForBoundary, sanitizeLocationForOutput, and safeRuleId preserve it → it reaches SQLite, JSON/SARIF/terminal, REST/SDK/MCP, and the final LLM provider request.

   Independent synthetic evidence: both tested ghp_ and gho_ shapes were positively detected by scanFile while the boundary classifier returned false. A single synthetic marker was then observed unchanged in:

   • project path plus finding create/read/raw SQLite fields;
   • suppression reason and LLM analysis/fix persistence;
   • JSON, SARIF, and terminal reporters;
   • REST and SDK finding responses;
   • MCP suppress_finding response;
   • an intercepted final provider request generated by chat.

   No real credential, environment enumeration, live process/tmux inspection, or external model request was used.

   Impact: a value Shield itself classifies as a credential can still be persisted, logged, returned to agents/API callers, or sent to an LLM. This directly reopens prior P1s 2 and 3.

   Required fix: use one canonical credential-recognition/redaction primitive shared by scanners and all persistence/output/provider boundaries, or propagate explicit finding taint so boundary safety cannot drift from scanner patterns. Add a property/corpus invariant: every value recognized by any credential scanner must be removed by text, location, rule-ID, nested-value, persistence, reporter, MCP/REST/SDK, suppression, and provider-payload sanitizers. Include low-entropy and delimiter/alphabet edge cases, not only realistic high-entropy examples.

Repaired paths that did pass

• Ordinary CLI, legacy-config CLI, DEFAULT_CONFIG, direct runAllScanners, REST, SDK, and MCP are file-only by default; history/system access requires an explicit current invocation.
• Direct synthetic runAllScanners found 0 history findings by default and found the history finding only with include_git_history: true.
• Regular-file scanning works; missing/stat/traversal/read/symlink failures fail closed/nonzero and withhold unsafe path/error context.
• Credential-classified findings short-circuit LLM context reads.
• Help and README accurately describe source opt-ins and compatibility behavior.

Exact-head verification

• bun run test: 376 pass, 0 fail, 816 expectations.
• Focused cross-boundary suite: 90 pass, 0 fail, 251 expectations.
• bun run typecheck: pass.
• bun run build: pass.
• bun run check:secrets: pass, 0 policy violations.
• bun run no-cloud:source: pass.
• bun run no-cloud:pack: pass, 66 packed files.
• npm pack --dry-run --json: pass, 66 entries.
• git diff --check origin/main...HEAD: pass.
• Exact hosted CI is green.

Residual release notes

• Package version remains 0.1.25, equal to the current registry version; a fixed release needs a new version.
• bun audit reports 30 dependency advisories (6 high, 23 moderate, 1 low). These were not manually attack-path-triaged in this focused review and are not the basis of this verdict, but they remain release follow-up.

Verdict: BLOCK at exact SHA 18d1741971a5f7f2c9d7c5f07df83dedff9a0a2f. Re-review a new immutable head after the shared-recognition invariant and cross-surface regression corpus land.

[andrei-hasna]

Reviewer-A2 P1 remediation ready for exact-head re-review

New immutable head: 264e400190ae80fbedd710a99e37e14cf4cb691d

• src/lib/credential-recognition.ts is now the single named/env/entropy recognizer used by both the secrets scanner and every finding-safety boundary. The boundary is a strict superset, so scanner-recognized values cannot drift past redaction.
• Recursive REST and production MCP response boundaries sanitize success and error payloads; direct MCP agent/focus/feedback paths sanitize before persistence/output.
• The synthetic property corpus covers 100+ scanner-recognized combinations, including all GitHub prefixes, upper/lower/digit/underscore bodies at minimum and longer lengths, quoted/unquoted assignment cases, provider formats, and entropy. It checks text/location/rule/nested values, finding create/update/legacy read/raw SQLite, JSON/SARIF/terminal, final provider payload, live REST+SDK, and live MCP success/error. Safe noncredentials remain unchanged.

Exact-head evidence:

• Hosted CI 29452800265: success
• bun test --parallel=1: 382 pass, 0 fail, 3,882 expectations
• Corpus suite: 3 pass, 3,052 expectations
• bun run typecheck: pass
• bun run build: pass
• bun run check:secrets: 0 policy violations
• bun run no-cloud:source: pass
• bun run no-cloud:pack: pass, 67 packed files
• npm and Bun dry-run package construction: pass, 67 files
• scrubbed-environment --files-only --json: files only, 0 findings
• staged diff and secret gates: pass

bun audit still reports pre-existing transitive advisories; package version remains 0.1.25, already present on the registry. Those remain separate release blockers. No merge, publish, install, deploy, credential access/rotation, ambient source scan, or transcript mutation was performed.

[andrei-hasna]

Adversarial defensive review A3 — BLOCK

Exact immutable head reviewed: 264e400.
Hosted CI: https://github.com/hasna/shield/actions/runs/29452800265, success at that exact head.

Blocking findings

1. P1, high confidence — scan error and reporter metadata bypass the canonical boundary.

   Location: src/db/scans.ts:19-24 and 72-83 return and persist scan.error unchanged; src/reporters/json.ts:51-59 serializes the supplied Scan unchanged. sanitizeFindingForOutput also leaves finding id, scan_id, fingerprint, and created_at unchanged, and src/reporters/sarif.ts:68-70 emits fingerprint directly.

   Attack path: a scanner or legacy caller supplies an error containing scanner-recognized credential text → exported updateScanStatus persists it raw → exported getScan returns it raw → the public JSON reporter emits it unchanged. Independently, a credential-bearing Finding identifier or fingerprint supplied to the public reporter crosses JSON and SARIF unchanged.

   Synthetic evidence: with no environment inspection or live credential, one generated scanner-recognized marker survived updateScanStatus, raw SQLite, getScan, and jsonReport. A public-reporter probe also observed the marker in JSON finding id/fingerprint and scan error, and in SARIF fingerprint. Only boolean leak results were printed.

   Impact: the prior incident invariant remains incomplete for scan failures, legacy rows, direct library consumers, and reporter inputs. A credential in diagnostic error text can still be persisted and serialized.

   Required fix: sanitize scan error before persistence and on legacy readback, and recursively sanitize the complete Scan/Finding values at every public reporter boundary. Preserve stable correlation with opaque hashes. Add create/update/legacy/raw DB and JSON/SARIF regressions for error and every serialized string field.

2. P1, high confidence — the canonical high-entropy hex detector is unreachable.

   Location: src/lib/credential-recognition.ts:151-157 defines a hexadecimal alphabet; lines 208-219 require Shannon entropy greater than 4.5 bits per character.

   Invariant failure: a 16-symbol hexadecimal alphabet has a theoretical maximum entropy of log2(16) = 4 bits per character, so no value can satisfy the greater-than-4.5 condition. A balanced 128-character synthetic hex probe measured entropy 4 and produced no high-entropy-hex recognition.

   Impact: one advertised canonical credential class never detects anything. The claimed all-class corpus therefore cannot cover this class, and high-entropy hexadecimal secrets are neither found nor boundary-redacted by that detector.

   Required fix: choose a defensible reachable threshold and length policy, then add positive and negative property tests around the boundary. Re-run the all-class scanner-to-boundary invariant.

Positive evidence

• Independent 72-value corpus covered 13 named/env/base64 classes: AWS access and secret assignments; every GitHub prefix; Stripe secret/publishable; quoted and env API assignments; private-key headers; JWT; Slack prefixes; database URL variants; base64 entropy. Zero leaks occurred across shared text, location, rule, nested value, finding persistence/output, JSON, SARIF, terminal, and provider-message sanitizers.
• 24 safe noncredentials stayed unchanged. Same-value correlation was stable and distinct inputs had distinct correlations.
• Ordinary CLI/library/MCP/REST defaults and explicit source options remain covered by the passing suite. File-only regular-file, stat, traversal, read, and symlink failure regressions pass.
• bun test --parallel=1: 382 pass, 0 fail, 3,882 expectations.
• typecheck and full library/types/CLI/MCP/server/SDK/dashboard build: pass.
• check:secrets: 0 violations; no-cloud source and 67-file packed artifact: pass.
• npm and Bun dry-run package construction: pass, 67 files.
• git diff --check and clean immutable worktree: pass.

Residual release blockers

• bun audit reports 30 advisories: 6 high, 23 moderate, 1 low. They were not the basis of this verdict.
• Package version remains 0.1.25, equal to the current registry version. A fixed release requires a new version.

No edits, merge, publish, install, deploy, rotation, ambient-source scan, or transcript mutation was performed.

Verdict: BLOCK at exact SHA 264e400. Re-review a new immutable head after both P1 paths have regressions.

[andrei-hasna]

Reviewer-A3 P1 remediation ready for exact-head re-review

New immutable head: 463c2b5fcdbea9b9fd2a2229977235fd463e166e

• Scan.error is sanitized before SQLite update and on legacy readback; the read path opportunistically scrubs unsafe legacy raw rows.
• Every string-bearing Scan/Finding field is recursively sanitized at public boundaries. Credential-bearing IDs and fingerprints use stable, distinct opaque correlations; JSON/SARIF, REST/SDK, MCP, terminal and final provider boundaries have synthetic regressions.
• Legacy finding fingerprints and timestamps are scrubbed on read.
• High-entropy hex uses per-alphabet normalized entropy (hex maximum 4 bits/character). Positive, negative and property tests cover the threshold. A narrow exact GitHub Action pin exception prevents source-control SHA false positives while the same 40-character hex remains recognized outside action syntax.
• The scanner-to-boundary corpus covers 100+ scanner-recognized shapes, all model string fields, DB create/update/legacy/raw paths, reporters and transport boundaries.

Exact-head evidence:

• Hosted CI 29454083586: success
• bun test --parallel=1: 393 pass, 0 fail, 4,624 expectations
• bun run typecheck: pass
• Full library/types/CLI/MCP/server/SDK/dashboard build: pass
• bun run check:secrets: 0 policy violations
• no-cloud source and 67-file packed artifact: pass
• npm and Bun dry-run package construction: pass, 67 files
• scrubbed-environment file-only repository scan: files only, 0 findings
• staged patch scan at fail-on info: 0 findings
• staged diff, commit trailer and branch hygiene checks: pass

bun audit remains at the pre-existing 30 dependency advisories; package version remains 0.1.25, which already exists on the registry. These remain separate release blockers.

No merge, publish, install, deploy, credential access/rotation, ambient source scan, or transcript mutation was performed.

[andrei-hasna]

Adversarial defensive review A4 — BLOCK

Exact immutable head reviewed: 463c2b5fcdbea9b9fd2a2229977235fd463e166e.
Hosted CI: 29454083586, success at that exact head.

Blocking findings

1. P1, high confidence — the GitHub Actions SHA exception is a generic sanitizer bypass, not a trusted workflow-context exception.

   isPinnedGitHubActionRevision recognizes only surrounding text (src/lib/credential-recognition.ts:210-220) and is called by the canonical scanner and every boundary (:222-253). The scanner passes the line and only an env-like boolean, not a trusted GitHub-workflow path (src/scanners/secrets.ts:590-615). Therefore the same high-entropy 40-character hexadecimal value that is recognized in isolation is ignored whenever untrusted text is exactly shaped as uses: owner/repo@<value>, even in .env, scan errors, finding metadata, MCP/REST responses, or provider messages.

   A synthetic probe confirmed: the isolated value was recognized; the exact action-shaped line in .env produced zero findings; generic boundary sanitization left it unchanged. The same value survived all five independently exercised surfaces: recursive value sanitization, JSON, SARIF, and final provider payload included. No real value, environment inspection, ambient-source scan, or external provider request was used.

   Required fix: make the exception an explicit scanner-only option enabled only for trusted .github/workflows/*.yml|yaml paths and exact uses: syntax. Generic persistence/output/provider recognition must never apply this exception. Add negative tests for the same syntax in non-workflow files and every generic boundary.

2. P1, high confidence — high-entropy base64 remains mathematically unreachable for common 128–192-bit token lengths.

   The regex accepts 20+ characters (src/lib/credential-recognition.ts:161-167), but the normalized comparison at :232-249 is algebraically equivalent to requiring Shannon entropy strictly greater than 5 bits/character. A string of length 32 has a theoretical empirical maximum of log2(32) = 5, so even a maximally diverse 32-character synthetic base64 token was not recognized. Shorter 128-bit/192-bit base64 encodings cannot pass either; padding further reduces the matched sample's entropy.

   Independent evidence: the 32-character control measured entropy exactly 5 and high-entropy-base64 recognition returned false. The new property tests cover hex but contain no corresponding base64 positive/negative boundary (src/lib/credential-invariant.test.ts:244-296).

   Required fix: use a length-aware/alphabet-aware defensible threshold and add property tests for padded/unpadded base64 positives and structured negatives across relevant 128-, 192-, and 256-bit lengths, then re-run the scanner-to-boundary invariant.

3. P1, high confidence — legacy credential-bearing identifiers remain recoverable from raw SQLite after the advertised scrub-on-read.

   Finding output redacts id/scan_id, but the opportunistic update omits both fields as well as scanner_type and severity (src/db/findings.ts:35-76). Scan output redacts every string field, but the readback rewrite updates only error (src/db/scans.ts:20-34). The invariant test queries only selected columns and gives every legacy row a safe ID/scan ID (src/lib/credential-invariant.test.ts:184-222), so it does not prove the complete raw-row claim.

   A synthetic in-memory legacy-row probe placed one scanner-recognized marker in Scan/Finding identifiers and other string fields, called the exported read APIs, and re-queried SQLite. Returned Scan/Finding objects contained no marker, but both raw Scan and raw Finding rows still contained it. This contradicts the code comment that direct DB consumers cannot recover unsafe legacy fields after first read.

   Required fix: durably scrub complete legacy rows with collision-safe stable opaque identifiers in a transaction/migration that preserves primary/foreign-key integrity. Add full-row raw-SQL assertions for every Scan/Finding string field, including ID and FK fields, after create/update/legacy read.

Positive exact-head evidence

• bun test --parallel=1: 393 pass, 0 fail, 4,624 expectations.
• bun run typecheck: pass.
• Full library/types/CLI/MCP/server/SDK/dashboard build: pass.
• bun run check:secrets: pass, 0 policy violations.
• bun run no-cloud:source: pass.
• bun run no-cloud:pack: pass, 67 packed files.
• npm and Bun dry-run package construction: pass, 67 files.
• git diff --check origin/main...HEAD: pass; worktree clean; local, remote, PR, and CI head all exact.
• The repaired scan-error, fingerprint, timestamp, JSON/SARIF, and stable correlation behavior passed the existing suite and focused independent output probes when inputs did not hit the context-free action exception.

Residual release blockers

• bun audit reports the pre-existing 30 advisories: 6 high, 23 moderate, 1 low.
• Package version remains 0.1.25, equal to the registry version; a fixed release requires a new version.
• Hosted CI runs test/typecheck/secret checks but not the complete build/no-cloud packed-artifact gates; those passed locally at this head.

No edits, merge, publish, install, deploy, rotation, real-credential access, ambient-source scan, or transcript mutation was performed.

Verdict: BLOCK at exact SHA 463c2b5fcdbea9b9fd2a2229977235fd463e166e. Re-review a new immutable head after all three P1 paths and regressions land.

[andrei-hasna]

A4 remediation is pushed at exact head 6a5516c13964dc2f2166a44f7d55d79c2de265f9.

Implemented:

• GitHub Action SHA exemptions are scanner-only and require a verified direct .github/workflows/*.yml|yaml source plus exact parsed uses: syntax; generic recognition and boundary sanitization remain fail-closed.
• Length-aware normalized Base64 entropy covers common padded and unpadded 16/24/32-byte tokens, with deterministic positive cases and a 2,051-item safe corpus at zero false positives.
• Startup/read-path transactional legacy scrubbing now covers Project, Scan, Rule, and Finding strings plus credential-bearing IDs/FKs, with collision retry, concurrent/idempotent behavior, rollback, and foreign-key integrity tests.

Verification:

• Hosted CI run 29455923832: success at this exact head.
• Full local suite: 401 pass, 0 fail, 6,881 expectations.
• bun run typecheck: pass.
• bun run build: pass.
• bun run check:secrets: pass.
• bun run no-cloud:source: pass.
• bun run no-cloud:pack: pass.
• npm pack --dry-run --json: pass.
• bun pm pack --dry-run: pass.
• Scrubbed-env files-only scan: 0 findings.
• Zero-context staged patch scan: 0 findings.
• git diff --cached --check: pass before commit.

Known unchanged release blockers: bun audit reports 30 dependency vulnerabilities (6 high, 23 moderate, 1 low), and package version 0.1.25 already exists in the registry. This comment is a handoff only; no merge, publish, install, deploy, or rotation was performed. Two independent exact-head adversarial review rounds are still required.

[andrei-hasna]

Adversarial defensive review A5 — PASS

Exact immutable head reviewed: 6a5516c13964dc2f2166a44f7d55d79c2de265f9
Hosted CI: 29455923832 completed SUCCESS at the same SHA.
Mode: read-only review; no source edit, merge, publish, install, deploy, credential access, rotation, ambient-source scan, or transcript mutation.

Blocking findings

No P0/P1 findings in the requested A4 remediation scope.

The previously blocking A4 paths are accepted at this head:

• GitHub Actions 40-hex exception is scanner-only and requires a verified direct .github/workflows/*.yml|yaml source plus exact parsed uses: syntax; generic persistence/output/provider boundaries remain fail-closed.
• High-entropy Base64 recognition is length-aware and reaches common 16/24/32-byte padded and unpadded token lengths while keeping the bounded structured-safe corpus negative.
• Legacy credential-bearing Project, Scan, Rule, Finding, baseline, and LLM-cache fingerprint data is durably scrubbed in a transaction with foreign-key integrity, idempotency, concurrency, and rollback coverage.

Independent evidence

• Source/provenance: local detached checkout at 6a5516c; PR head and hosted CI head match; merge state CLEAN; git diff --check origin/main...HEAD passed.
• Focused invariant/legacy tests: bun test src/lib/credential-invariant.test.ts src/db/legacy-credential-scrub.test.ts --parallel=1 passed 14/14 with 6,023 expectations.
• Focused boundary/DB/reporter/MCP tests: passed 81/81 with 216 expectations.
• Full test suite: bun test --parallel=1 passed 401/401 with 6,881 expectations.
• bun run typecheck: pass.
• bun run build: pass across library, types, CLI, MCP, server, SDK, and dashboard. Dashboard has only the existing Vite chunk-size warning.
• bun run check:secrets: pass, 0 OSS-policy violations.
• bun run no-cloud:source: pass.
• bun run no-cloud:pack: pass, 68 packed files.
• npm pack --dry-run --json: pass, 68 files; prepack build and no-cloud artifact scan passed.
• bun pm pack --dry-run: pass, 68 files.
• Independent synthetic probes passed: real workflow action pin exempted only when scanning the verified workflow file; the same line in .env and generic boundaries is redacted/detected; deterministic 16/24/32-byte Base64 positives are recognized and redacted; safe structured controls remain negative; full raw SQLite rows across Projects/Scans/Rules/Findings/baselines/llm_cache contain no synthetic marker after scrub and PRAGMA foreign_key_check is clean.

Residual release blockers / follow-ups

• bun audit still reports 30 pre-existing advisories: 6 high, 23 moderate, 1 low. These were not introduced by this remediation and were not the basis of this acceptance, but they remain release triage work.
• Package version remains 0.1.25, which already exists on the registry; any fixed publication needs a new version and release gate.
• Hosted CI currently proves test/typecheck/secret checks; the full build/no-cloud/pack gates passed locally in this review.

Verdict: PASS for exact SHA 6a5516c13964dc2f2166a44f7d55d79c2de265f9.

[andrei-hasna]

Adversarial defensive review B5 — BLOCK

Exact immutable head reviewed: 6a5516c13964dc2f2166a44f7d55d79c2de265f9.
Hosted CI: 29455923832 completed SUCCESS at the same SHA.
Mode: read-only review plus synthetic/file/archive-only probes; no source edit, merge, publish, install, deploy, credential access/rotation, ambient-source scan, or transcript mutation.

Blocking finding

1. P1, high confidence — production-shaped baseline and LLM-cache legacy rows remain raw-recoverable after the durable scrub.

   Locations:

   • src/db/legacy-credential-scrub.ts:258-261 rewrites only baselines.finding_fingerprint and llm_cache.finding_fingerprint when a finding fingerprint changes.
   • src/db/legacy-credential-scrub.ts:83-91 checks only projects, scans, rules, and findings for unsafe rows, so cache/baseline-only legacy exposure does not trigger the scrub at all.
   • src/db/legacy-credential-scrub.test.ts:43-44 uses simplified two-column baselines and llm_cache fixtures, omitting the production string columns that can retain credentials.
   • src/db/baselines.ts:5-27 persists finding_fingerprint, reason, and created_by without the central sanitizer; MCP baseline_findings passes user-controlled reason into that path (src/mcp/tools/findings.ts:146-160).

   Attack path: a legacy database or MCP baseline request contains a credential-shaped value in baseline/cache metadata or cached analysis result → scrubLegacyCredentialRows is invoked by scan/finding read → Projects/Scans/Rules/Findings and fingerprint references are cleaned, but production baseline/cache string fields remain unchanged in SQLite → direct SQLite consumers, backups, artifacts, or later unsanitized baseline reads can recover the value.

   Independent synthetic evidence: using a production-shaped in-memory SQLite schema and a synthetic scanner-recognized marker, I inserted marker-bearing IDs/FKs and string fields across Projects, Scans, Rules, Findings, Baselines, and llm_cache, then called the exported scrubLegacyCredentialRows(db) and queried raw SQLite without printing the marker. Results:

   • Projects/Scans/Rules/Findings: clean.
   • PRAGMA foreign_key_check: clean.
   • second scrub pass: idempotent.
   • retained marker fields: baselines.id, baselines.reason, baselines.created_by, baselines.created_at, llm_cache.id, llm_cache.analysis_type, llm_cache.result, llm_cache.model, llm_cache.created_at.

   A narrower production-like probe also showed llm_cache.result remains raw-recoverable even when every core Project/Scan/Rule/Finding string is scrubbed. No real credential or environment data was used.

   Impact: the advertised durable raw-SQLite scrub boundary is incomplete. This reopens the same class of persistence/output safety incident for baseline and cache tables, and the baseline creation API can create new unsanitized persistence from MCP-controlled reason text.

   Required fix: extend the durable scrub to production-shaped baselines and llm_cache rows, including IDs and every string column (reason, created_by, analysis_type, result, model, timestamps, etc.), not just fingerprint references. Make hasAnyUnsafeRows cover those tables. Sanitize createBaseline inputs and listBaselines outputs/readback. Add production-schema raw-SQL assertions for every string column in Projects/Scans/Rules/Findings/Baselines/LLM cache, plus cache/baseline-only unsafe-row triggers, idempotency, and FK integrity.

Passing evidence for the A4-focused fixes

• PR head, local checkout, and CI head all match 6a5516c13964dc2f2166a44f7d55d79c2de265f9; merge state is CLEAN.
• CI run 29455923832 succeeded: checkout pinned by SHA, setup-bun pinned by SHA, bun install --frozen-lockfile --ignore-scripts, bun run check:secrets, bun run typecheck, and bun run test all passed.
• No-install focused run: bun --no-install test src/lib/credential-invariant.test.ts src/db/legacy-credential-scrub.test.ts --parallel=1 executed the legacy scrub process/idempotency/rollback/collision tests successfully, then stopped when the broader invariant file imported missing openai (no local dependency install was performed).
• Dependency-free synthetic probes passed:
  • isolated 40-hex recognized;
  • trusted .github/workflows/*.yml exact uses: pin exempted only for scanner context;
  • the same action-shaped line in .env is detected;
  • generic boundary sanitization redacts the workflow-shaped hex;
  • 16/24/32-byte Base64 tokens are recognized and redacted.
• Static inspection confirmed public default scans are file-only: DEFAULT_CONFIG.enabled_scanners uses DEFAULT_FILE_SCANNERS; runAllScanners, REST, SDK, and MCP require per-invocation include_git_history; file traversal/read/stat/symlink errors fail closed in the secrets scanner.
• Reporter/REST/MCP/provider boundaries generally route through sanitizeFindingForOutput, sanitizeScanForOutput, or recursive sanitizeValueForBoundary; MCP LLM analysis reaches analyzeFinding, which rejects credential findings and sanitizes provider messages.

Residual release blockers / follow-ups

• Package version remains 0.1.25, and npm view @hasna/shield version currently returns 0.1.25; publication of a fixed artifact requires a version bump.
• Hosted CI does not run full build/no-cloud/pack gates. I did not run prepack locally because package.json build scripts include a dashboard bun install, and this review scope forbids installs.
• Static npm pack --dry-run --json --ignore-scripts at this source state includes only LICENSE, README.md, bin/*, and package.json; the real release artifact depends on prepack/build evidence.
• Pre-existing dependency advisories remain release triage work; I did not perform a full dependency attack-path review in this B5 pass.

Verdict: BLOCK at exact SHA 6a5516c13964dc2f2166a44f7d55d79c2de265f9. Re-review a new immutable head after baseline and LLM-cache durable scrubbing plus regressions land.