Repository navigation
feat(admission): enforce record exclusions across indexing and recall - #180
HengYangDS wants to merge 36 commits into
Conversation
|
The upstream CI run for this fork PR is currently |
Survey of the sync/index/summarize pipeline for the inline secret redaction fork. Picks the archive write as the choke point (option b) and records the three supporting changes it needs: indexer parses the archive, summarizer resume/fork disabled under redaction, staging exports redacted at write. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0141gGbp343kbYP6imaJaji2
…bed/summarize Adds an inline, rules-driven redaction stage at the one point every harness passes through: the copy into the conversation archive (Phase 0 option b). Secrets become typed tokens ([REDACTED:<ruleId>]); every downstream sink reads the archive, so SQLite text, tool_calls, embeddings, summaries and show/read only see redacted text. - src/redaction.ts: engine, rules loading/validation, JSONL-preserving line redaction, streaming copyFileRedacted. Findings are rule IDs and counts only. Idempotent: tokens never re-match. - src/redaction-rules.ts: bundled defaults ported from gitleaks for this stack (Azure client secret / storage key / SAS / connection strings, private keys, JWT, provider keys, url creds, key=value assignments); git SHA + GUID allowlist; entropy fallback off by default. - sync.ts / indexer.ts: archive copy goes through the redactor; indexer now parses the archive instead of the source. - summarizer: allowResume option; callers disable Claude resume and Codex fork under redaction, since both read the unredacted source transcript. - opencode / legacy Cursor staging exports are redacted at write time. - Fail closed: strict mode (default) aborts sync/index/import when rules fail to load. Env: EPISODIC_MEMORY_REDACTION, _RULES, _STRICT. - `episodic-memory redact --rewrite [--dry-run]` backfills the existing archive, staging dirs and index in place (re-embeds changed rows, deletes summaries built from unredacted text); also --stdin and --print-default-rules. - Tests: per-rule positives, allowlist negatives, idempotency, config, JSONL structure, end-to-end pipeline (archive/SQLite/embedding input/ summarizer input/logs), fail-closed, rewrite. Fake secrets are built at runtime; no literal in the repo matches a rule. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0141gGbp343kbYP6imaJaji2
… are caught
The first cut only redacted values whose own shape matched a rule, plus an
unquoted key=value rule that required a digit. Common .NET/Azure leaks all
slipped through: appsettings.json fields, web.config <add key value/>,
publish-profile userPWD, `az webapp config appsettings list` and
`az keyvault secret show` output, C# `ClientSecret = "..."`, and structured
MCP results (parsed JSON lost field names; keys were never checked).
Text rules (key context, no digit requirement, placeholder-aware):
- azure-keyvault-secret, name-value-secret (either order),
xml-appsettings-secret (either order), xml-secret-element,
xml-secret-attribute, quoted-secret-assignment.
- These opt out of the SHA/GUID allowlist: a GUID in a password slot
(legacy create-for-rbac) is a secret.
Structured JSON (secretFields, on by default, configurable keyPattern):
- a string whose field name ends in a secret keyword is redacted whole;
- {name, value} pairs with a secret-looking name and Key Vault bundles
have their value redacted;
- keys that are themselves secrets are renamed to their token.
Harness fields (thinking signature, usage, token_count, apiKeySource) are
untouched.
Engine: secretGroup may list several groups (first that matched);
per-rule useAllowlist. connection-string-secret no longer allows spaces
around '=', fixing a C# false positive (options.Password = configuration[...]).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0141gGbp343kbYP6imaJaji2
repairIndex disables summarizer resume when redaction is enabled, while the indexer disables it only when a redactor loaded. They differ only when non-strict rules fail to load, and there verify is the stricter one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
copyFileRedacted opened its output with the 0666 & umask default, so a 0600 transcript became world-readable in the archive, and redact --rewrite widened existing archive files the same way. copyFileSync, used before redaction, kept the source mode. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
indexUnprocessed used to re-copy every indexed transcript and parse the source. Parsing the redacted archive instead made it depend on copyIfNewer's mtime check, which skips an append that lands within the mtime granularity, so those exchanges were never indexed. copyIfNewer takes a force flag, and indexUnprocessed sets it once a file is indexed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The rule's trailing lookahead excluded '.', which is also in the secret alphabet, so a 40-char secret followed by a period could match neither with nor without it and was left in clear text. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ADO.NET keywords are case-insensitive, but the rule only matched the listed capitalization, so `password=` and `PASSWORD=` values were kept. A letters-only value isn't caught by secret-assignment either, which needs a digit. A path after `Pwd=` is still left alone: that's the shell PWD variable. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
applyRule skipped any match that overlapped a token, so in partly redacted text (an older rule set, before redact --rewrite) `Password=<secret>[REDACTED:jwt]` kept the secret. The existing tokens are now kept and each stretch between them with a letter or digit is replaced. A stretch already fully covered yields no change, so a second pass is still a no-op. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The field-name rule skipped any value containing a token, so in
{"password": "<word> <JWT>"} the JWT rule's token kept the field from
being replaced and the word stayed. Only values made up entirely of
tokens are skipped now.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
repairIndex checked only whether redaction was enabled, so with a corrupt rules file in strict mode it still deleted, re-indexed and summarized, while sync and index abort. It now loads the redactor first, which throws in strict mode, and derives allowResume from the result as the indexer does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`redact --rewrite --dry-run` opened the index with initDatabase(), which runs schema migrations and switches to WAL, so a dry run changed the database it promised not to touch (and created one if missing). A dry run now opens the existing index read-only, or skips the index if there is none. `--report` (with --dry-run) lists each value the rewrite would redact: file and line or index row, the rule, the value's shape (length, character classes, Shannon entropy) and the redacted text around it. The value itself is never printed. It uses an optional onMatch callback on RedactionContext, so it runs the real redaction path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The report matched values to tokens by rule and order, skipping as many tokens as the original line already had. A new hit before an existing token of the same rule took that token's context. A secret field redacted whole after a text rule hit part of it lost the text rule's hit, and its shape described the partly redacted value. onMatch may now return the token to write. In report mode each hit gets a numbered one ([REDACTED:<rule>--hit<n>]), so it is found exactly; unnumbered tokens were already there. A field value is restored from the numbered tokens inside it, and hits it swallowed are reported at its token. rewriteArchive also refuses report without dryRun: after a real rewrite the files would have nothing left to report. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Markdown like `Password=`, `Pwd=` matched with "`," as the value, so docs and chat about connection strings were mangled and inflated the counts (found dogfooding on a real archive). A real value always has a letter or digit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
docs/REDACTION.md now covers where redaction happens (with links to the choke points), the three detection layers, the guarantees, the backfill, configuration and limits, including how this differs from NER. It picks up behavior added since the first draft: index repair failing closed, partial tokens, whole-field replacement, kept file modes, and the connection-string exclusions. PHASE0-FINDINGS.md described the build process rather than the result, so its one lasting point (why the archive write is the choke point) moved into the guide and source comments now point there. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
The current CI table supersedes this earlier approval snapshot. It names the six current heads and distinguishes #184’s expired run. Each PR description retains its local validation and integration boundaries. |
|
Follow-up on the new revision a8eadb5: The source/product-boundary review found one concrete isolated-summary shutdown gap: after a successful result, protocol error or timeout, a Codex child that ignored SIGTERM could leave the parent waiting indefinitely for close. The minimal repair adds an isolated-only 1024 ms TERM-to-KILL grace, cancels that timer on close, and keeps temporary-directory cleanup close-bound. It does not change the normal summary route, session-ID forks, provider/model selection, or record-admission defaults. Three actual TERM-resistant child regressions failed on the submitted predecessor and pass on this revision. Native Node 24.19.0 acceptance on macOS arm64 includes build/typecheck, 41 focused tests, eight generated-dist routing tests, and 451/451 final tests across 67 files with zero skips. The unrelated MCP bundle is preserved byte-for-byte. This is local evidence, not a hosted Node 22/24, Windows, real-provider, or descendant-held-stdio claim. Exact revised-head Actions run 38096647228 is still action_required and has not executed. The existing approval request remains sufficient; this comment supplies the new defect, fix and verification facts for review rather than another unchanged approval ping. I will address actionable review/CI findings and maintain the contribution through merge and relevant stable-release adoption. |
A retained search row whose transcript has since been removed made the whole request fail with "Conversation changed during record admission". Codex replaces archived rollouts with compressed .jsonl.zst files, so a transcript indexed before compression disappears and every query that matched one of its rows failed. A removed transcript has nothing left to verify or expose, so search now withholds its rows and continues. Symlink substitution, indexed-range rewrites and other admission failures still fail closed.
|
Follow-up on a24fde1: Search admission failed the whole request with A removed transcript has nothing left to verify or expose, so search now withholds those rows and continues. Symlink substitution, indexed-range rewrites and other admission failures still fail closed. Three regression cases cover the text, vector and both modes; they fail before the change and pass after it. The full local suite passes: 67 files, 454 tests. As in the earlier fix commits, only |
|
Please approve these CI runs so the fixes can be reviewed on their current heads.
All five are waiting for approval; no hosted tests have run. #184 needs a fresh run: its earlier approval request expired after 30 days, before any job started. Local validation is recorded on each PR. #180 remains Draft at signed |
a24fde1 to
c7cca0f
Compare
Preserve the independent provider-boundary history and tests while retaining the admitted-text, sessionless Codex extension. Explicitly disabled resume also remains isolated for Claude summaries.
Preserve the redaction contributor history and tests. Apply exclusions before redaction, parse the derived archive at every index entry point, and keep transcript-only summaries on their original provider.
Opt-in record admission keeps excluded or rejected records out of archives, indexes, embeddings, summaries and recall. Safe neighboring records remain usable; source transcripts and physical line numbers are preserved.
The branch integrates #177, #178 and #187 with their original commits and tests. One archive writer applies record exclusions and screening, then redacts admitted lines. All three index entry points parse that archive. Changed redaction rules refresh it even when the source mtime is unchanged. Strict rule-loading failures stop ingestion or repair before data writes.
Filtered summaries cannot resume the original session. Codex uses an isolated ephemeral Codex thread; Claude uses its transcript-text route. Codex failures never switch providers. Existing rewrite/backfill and staging redaction come from #187 rather than a second implementation.
Review scope
This remains Draft. The shared pipeline is implemented and packaged acceptance passes; the redaction default carried from #187 still needs a maintainer decision. Both real Codex and Claude transcript-only summaries pass; production-scale recall cost remains open. Claude retains its user-level native credential helper and endpoint through the SDK settings option, while hooks, plugins, project settings and MCP stay excluded. The synthetic large-index measurements below make the current scan cost explicit. #109's summary coverage and retry budgets, #190's compressed ingestion, and #193's wider rewrite reconciliation remain separate.
Validation
Head:
aa29032ef859c66eaf6bb3b5a95c5dc4a0880dae; base:7e06519357777badd7a115d2014a7ef845904310.npm test -- --maxWorkers=2). Two new shared-pipeline regressions fail before the integration and pass afterward. The default worker run hit existing test-hook timeouts; the bounded two-worker run uses an explicitly owned native temporary root outside the ESM repository.apiKeyHelperand endpoint. Two authentication regressions fail before the fix and pass afterward; explicit endpoint precedence and settings isolation remain covered. The initial auth failure and a later HTTP 400 are retained as failed evidence; service reliability is not claimed. A small native title artifact from the synthetic test was retired through Claude purge after its exact dry-run, preserving all preexisting project/transcript counts.afterAll. Two actual child-run regressions reproduce leaks before the fix, then pass for successful and failed suites while preserving a neighboring directory.Earlier packaged MCP and child-shutdown evidence remains in the shutdown follow-up and removed-transcript regressions. Hosted platform checks await the existing workflow approval; merge, release and installed adoption are not claimed.