feat(privacy): rescan at-rest LCM store bytes under the current detector - #522
Conversation
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b463c63d61
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| FactRescanDispositionV1::Redact(patch) => { | ||
| redacted_facts = redacted_facts.saturating_add(1); | ||
| operations.push(ProjectMemoryCurationOperation::Update { |
There was a problem hiding this comment.
When a legacy fact contains a detector hit, this ordinary curation update only appends a correction assertion. The original plaintext remains in memory_v2_assertion_payloads.payload_json and content and in its FTS backing because assertion payloads are immutable, so the served-content test can pass while the credential is still present at rest. This remediation needs a storage-level purge or rewrite rather than only a projection update.
Useful? React with 👍 / 👎.
| .apply_project_memory_curation( | ||
| operations, | ||
| remediation_confidence()?, |
There was a problem hiding this comment.
If the current detector finds more than 256 dirty facts, all hits are accumulated and submitted here as one batch, but ProjectMemoryFactCurationBatchV1::new rejects more than MAX_PROJECT_MEMORY_CURATION_OPERATIONS (256). A store with 257 hits therefore commits no remediation at all, and the daemon merely logs the failure while every secret remains served; process findings through authority-supported bounded commits instead.
AGENTS.md reference: AGENTS.md:L141-L143
Useful? React with 👍 / 👎.
| "/api/multi-root/collection", | ||
| get(multi_root_api::resolve_collection), | ||
| ) | ||
| .route( | ||
| "/api/native-integration/status", |
There was a problem hiding this comment.
These routes are also reached through /api/projects/{project_id}/…, but a non-active selected state reuses the active state's application_invocation_executor, whose production InProcessDaemonInvocationExecutor is pinned to the active project_path. Neither the multi-root request nor native-status dispatch supplies the selected project, so requesting project B can return project A's collection or transaction status under B's URL, or falsely report B's data missing.
Useful? React with 👍 / 👎.
| let remaining = maximum.saturating_sub(merged.len()); | ||
| if remaining == 0 { | ||
| break; | ||
| } | ||
| merged.extend(root.poll_status(remaining).into_iter().take(remaining)); |
There was a problem hiding this comment.
In a federated workspace, once the first root has at least the 16 retained statuses requested by each LSP flush, it consumes the entire maximum and this loop breaks before polling any later root. Since the first root returns the same newest statuses on every poll even after session deduplication, transactions from later workspace roots are permanently never notified; collect candidates fairly from every root before applying the global limit.
Useful? React with 👍 / 👎.
| pub(super) notified: | ||
| BTreeMap<NativeIntegrationTransactionId, NativeIntegrationStatusProjectionV1>, |
There was a problem hiding this comment.
Federated sessions merge independent per-project transaction stores, but this map keys only on the caller-supplied transaction ID. If two authorized roots both contain transaction X, each poll returns two different projections under the same key; processing A replaces B and processing B replaces A, so both notifications are emitted again on every flush. Include the owning project/root identity in the deduplication key.
Useful? React with 👍 / 👎.
| let result = crate::application_surface::resolve_dashboard_application_surface( | ||
| crate::application_surface::ApplicationSurfaceOperation::NativeIntegrationStatus, | ||
| control.request_id(), |
There was a problem hiding this comment.
For a dashboard request with a short deadline or a cancelled/disconnected client, only the request ID is forwarded here. resolve_dashboard_application_surface creates a fresh cancellation signal and default deadline, so the in-process daemon read can outlive the admitted HTTP operation instead of observing control.deadline() and control.cancellation() as the adjacent multi-root adapter does.
Useful? React with 👍 / 👎.
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
The remediation commit no longer re-reads the row it just upserted to guard payload deletion: an external row remediates only when its body changed and payload refs are content-addressed, so the replacement ref can never equal the replaced one. The dirty probe compares at-rest bytes directly (a finding that changes no byte changes no at-rest state), parses stored provider metadata once per row instead of twice, and the projection-twin resync runs unconditionally since an absent twin matches no rows. AlreadyCurrent no longer carries the detector revision no caller reads, and the watermark key is shared with the test instead of duplicated. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
80479c1 to
d22241a
Compare
327011a
into
codex/tracedecay-total-redesign-plan
…-batch-evidence-d808 fix(privacy): close the remaining Codex P1 gaps after #522 merged
2 participants
Fills the remaining Plan 18 B5 gap on top of #516. Stacked on
cursor/mount-privacy-multiroot-native-511e(#516); once that PR merges, this diff reduces to the two commits above its head. No version bump.What #516 already mounted (not duplicated)
#516 owns the daemon at-rest rescan entry point: project-open spawns bounded background remediation (
src/daemon/privacy_remediation.rs) that re-runs the current detector over served project-memory facts. This PR extends that same owner instead of adding a second entry point, and uses the same Plan 18 sanitizer (sanitize_lcm_payload_textand the canonical LCM ingest staging/commit path) instead of inventing a new one.The gap: LCM raw-message bytes at rest
The audit's B5 evidence (
crates/tracedecay-sessions/src/runtime/lcm/raw.rs"only protects new ingest") is about the LCM raw-message store: rows persisted under older detector rules keep serving and keep their bytes at rest until something re-evaluates them. Nothing did.New owner:
RegisteredGlobalDb::lcm_privacy_rescan_raw_messagescrates/tracedecay-global-db/src/registered_lcm_privacy.rs:lcm_protect_session_raw_messagespass (the second phase of the observation-rebuild design), enumerated per owning session — no duplicate protect path.contentand whole-message external payload file bytes — is re-evaluated under the current detector. Provider metadata is probed with the exact ingest transformation (raw::provider_metadata_requires_resanitization).stage_raw_message_with_payload_tracked+commit_staged_raw_message), so redaction, externalization, quarantine, fresh deterministic receipts, and FTS triggers all follow ingest semantics; thesession_messagesprojection twin is resynchronized from the re-ingest's projection output.delete_external_payload_in_transactionand drains after commit, so superseded secret bytes leave the disk.lcm_gc_meta, keyed by the new effective detector revisionlcm_payload_detector_revision()(pinned sanitizer contract + digest of the vendored gitleaks catalogue and local supplement). The sweep runs once per rule refresh, not on every project open; an interrupted pass leaves the watermark unset and reruns idempotently.SanitizationRefused— never a silent skip — and rows whose external payload bytes are already offloaded/collected are counted truthfully asunavailable_payload_rows.Daemon mount:
spawn_at_rest_privacy_remediation(the renamed #516 spawn) now runs the memory-fact rescan, then the LCM store rescan, in the same bounded background task after fail-closed admission.Test
crates/tracedecay-global-db/src/tests/lcm_privacy_rescan.rsseeds, against a registered fixture store: a cleanly ingested row, a receipt-bound legacy inline row holding a currently-detectable credential (with a guard assertion that the fixture is actually dirty), a legacy external row whose payload file holds the credential, and an unreceipted projection-landed row. It proves exact receipt counts (4 scanned / 2 clean / 2 remediated / 1 protected), that the credential is gone from every at-rest surface (raw columns, projection twin, FTS-backing text, payload directory, and the deleted replaced payload file), that remediated and protected rows still serve through the verified raw-read authority with fresh receipts and retained provider metadata, that the watermark answers a second request without scanning, and that a forced repeat pass (watermark cleared) finds the store clean and settles nothing.Verification
cargo check --all-featuresclean from the repo root; clippy clean on touched crates (one pre-existing warning in untouchedtracedecay-rusqlite-runtime).tracedecay-runtime-coreprivacy suite (104) green;tracedecay-sessionsLCM suite (108) green; the feat: mount privacy remediation, multi-root dashboard scope, native-integration notifications #516 daemon memory-fact rescan test re-run green on this branch.tracedecay-global-dbfull lib suite: 366 passed, 12 failed — the identical 12 daemon-scope/authority tests fail on the pristine feat: mount privacy remediation, multi-root dashboard scope, native-integration notifications #516 head in a clean worktree on this VM (overlay-filesystem environment), so they are pre-existing and unrelated.