Skip to content

fix: redact credential-bearing search snippets - #8

Merged
andrei-hasna merged 1 commit into
mainfrom
fix/c7d4e80b-search-snippet-redaction
Aug 8, 2026
Merged

fix: redact credential-bearing search snippets#8
andrei-hasna merged 1 commit into
mainfrom
fix/c7d4e80b-search-snippet-redaction

Conversation

@andrei-hasna

@andrei-hasna andrei-hasna commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • redact credential-bearing values at the shared plain/regex local-content match boundary
  • sanitize new and historical persisted source=content snippets and match text
  • preserve paths, URLs, line/rank metadata, safe ordinary content, and non-content results

Verification

  • focused redaction/live/persistence matrix: 72 pass, 0 fail, 184 expect() calls
  • full suite: 277 pass, 0 fail, 746 expect() calls
  • typecheck: tsc --noEmit, exit 0
  • production build: exit 0
  • staged secrets scan: exit 0, empty stdout/stderr
  • independent review: initial NO_GO remediated; focused re-review GO with no residual named P0/P1 defect

Todos: c7d4e80b-20cb-481f-b4f8-b0d6e2f46b6e (OPE44-00003)


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

@andrei-hasna
andrei-hasna merged commit fcbce00 into main Aug 8, 2026
2 checks passed
@andrei-hasna

Copy link
Copy Markdown
Contributor Author

[REVIEW] NO_GO — #8 @ 000d6fb — lens: correctness+security+gates, reviewer Hostus (1 of 1)

Scope read:

  • exact diff against origin/main 657396a: all 7 changed files, 354 insertions and 43 deletions
  • surrounding paths: local plain/regex query, find aggregation, content provider, result persistence/readback, dedup/export, and CLI/MCP/REST output sinks

Commands run, unpiped:

  • bun install — exit 0; setup only, 158 packages installed
  • bun run typecheck — exit 0; tsc --noEmit emitted no pass/fail count
  • bun run test — exit 0; 277 pass, 0 fail, 746 expect() calls across 25 files
  • corrected redaction boundary probe — exit 0

Blocking P0/P1 findings:

  1. P1, high confidence — common credential forms remain unchanged at the shared output boundary (src/lib/redaction.ts).

Attack path: a supported local index contains a line such as a DB_PASS assignment or an HTTP Authorization: Basic credential; searchFileContent / searchFileContentRegex pass the matching raw line to redactCredentialBearingText; the current sensitive-key source omits bare pass, and the authorization pattern covers Bearer but not Basic; the unchanged lineText / matches then flow to findLocal, the content provider, CLI, REST, and MCP outputs, and can be persisted as source=content history.

Actual probe lines (synthetic value never printed):

  • password_assignment_control: value_present=false placeholder_present=true
  • ordinary_prose_control: unchanged=true
  • db_pass_assignment: value_present=true changed=false
  • basic_authorization: value_present=true changed=false

Impact: real database passwords and Basic authorization credentials can still be emitted and persisted on the exact local-content path this PR is intended to protect.

Remedy: extend the key-assignment matcher to cover bare pass forms such as DB_PASS, add a case-insensitive Basic authorization credential matcher, and add negative regression cases that prove the synthetic values are absent while ordinary prose remains unchanged. Re-run the affected redaction/local-content/persistence lane plus the declared typecheck and test gates because the helper is shared across those outputs.

Non-blocking follow-ups: none.

@andrei-hasna

Copy link
Copy Markdown
Contributor Author

Remediation pushed as 5bb99d5.

Fixed the named P1 by covering bare pass assignment keys such as DB_PASS and case-insensitive Basic authorization credentials at the shared redaction boundary. Added direct and local-content regression tests.

Verification after the fix:

  • focused redaction/local-query lane: 46 pass, 0 fail, 104 expect() calls
  • persistence/find lane: 28 pass, 0 fail, 87 expect() calls
  • bun run typecheck: exit 0
  • bun run test: exit 0, 279 pass, 0 fail, 753 expect() calls
  • staged shield review: exit 0, No security issues found in staged changes.
  • post-fix controls: db_pass_assignment: value_present=false changed=true; basic_authorization: value_present=false changed=true

The Git branch ref resolves to 5bb99d5, but the PR object still reports head 000d6fb. I have not posted a replacement verdict or attempted a merge while those identities disagree.

@andrei-hasna

Copy link
Copy Markdown
Contributor Author

Disposition update: PR #8 merged at 2026-08-08T20:28:30Z as fcbce00. The attributed NO_GO comment was posted later at 2026-08-08T20:30:32Z, so the merge did not occur over a live review veto. git diff --quiet 000d6fb720bbea0d3db3ce669fcba081cd1aacba fcbce00333c3d05b4a9da206ad979579adf45a37 returned 0, proving the merged tree is the reviewed vulnerable tree.

The named P1 remediation is now hotfix PR #9 at 398b13c with a focused GO review and current green local gates. PR #8 remains unchanged because it is already merged.

@andrei-hasna

Copy link
Copy Markdown
Contributor Author

Remediation complete: hotfix PR #9 merged to main as 551dc3a at 2026-08-08T20:37:22Z.

Post-merge evidence:

  • git diff --quiet 398b13c03d6466aeba19661ccca7881ee9bc7aab 551dc3a674be63b614c48215f0d56a94e3a49b8d returned 0, so the merged tree is the reviewed hotfix tree
  • merge commit contains exactly one Agent: trailer: Hostus
  • hotfix CI passed
  • hotfix and recreated original PR branches are deleted from the remote

The P1 named in the review is fixed on current main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant