Slice 13: pair a witnessed Run with its observed capture file, via a PII vault - #666
Merged
Merged
Conversation
The observed capture path is personal data by construction: 2-BM's directory layout embeds a surname and proposal number (tomoscan_2bm.py's DetectorTopDir composition). Events are immutable/INSERT-only, so it can never live in an event payload. This adds a dedicated mutable side table, mirroring the shipped actor_profile / ProfileStore pattern: run_capture_path (RLS+FORCE, no event ever references its content) plus CapturePathStore / PostgresCapturePathStore / InMemoryCapturePathStore. Bumps EXPECTED_SCHEMA_VERSION to the new newest migration.
RunWitnessRecorder retains the latest full_file_name reading per capture code, each with its own substrate observed_at. At a capture's terminal, the retained value is attached only if its observed_at is at or after that capture's own BEGUN time (never CORA's local clock); absent, stale, or empty-string readings are rejected and nothing is recorded. The write happens after the outcome has already committed, so a vault-write failure is logged (error class only, never the raw exception text or the path) and never unwinds the Run's own state. capture_path_recording_enabled is a fifth independent kill switch, boot-gated exactly like the existing four: refuses to start if set without run_witness_recording_enabled (no promoted Run's terminal to attach an observed path to without it). capture_path_store is built inside wire_run(deps), mirroring feed_heartbeat_store: BC-specific stores stay BC-internal, never promoted to Kernel fields (this concept has exactly one BC, unlike the genuinely cross-BC ProfileStore). extract_capture_code (folding the capture-code external_ref off the FOLDED aggregate state) is promoted out of a private duplicate in _run_witness.py to cora.run.aggregates.run.state, so it can become the single source of truth shared with the read-model work landing next.
capture_code folds onto proj_run_summary from RunStarted.external_refs (already carried there; no new event field, no new subscription). list_runs surfaces it as a plain non-PII field on RunSummaryItem. observed_capture_path resolves from the run_capture_path vault inside get_run's own Handler (RunView, mirroring get_actor's ActorView): None when there's no capture_code (a Conducted run, not applicable), the tombstone when a capture code exists but the vault has no row yet (never observed, or rejected by the dual-clock guard), the real path otherwise -- this is the authorized surface an operator reads to find the file for ingest_scan's producing_run_id. Deliberately NOT resolved in list_runs: that handler is one shared instance read by every internal composition-root caller (rebuild_open_captures, the supervisor and initiator watchdogs) under one coarse, cursor-paginated ListRuns grant, unscoped per row (BOLA deferred until ReBAC per list_query.py). get_run has no internal caller today, so it avoids that exposure outright, mirroring why list_actors never touches ProfileStore while get_actor does.
The preflight tool's redaction was exact-role-match only: a config typo on the role key (e.g. "full_filename") would print the raw observed path. Adds defense-in-depth on top of the role check: any string value that looks like a filesystem path (starts with "/") is redacted regardless of which role or dict (watch or baseline PVs) it came through, so a misconfiguration can't silently bypass redaction. kind and length still render for the real role.
AST-walks every class in cora/run/aggregates/run/events.py (not just Run-prefixed ones -- CautionAcknowledgement, DecisionDebriefRequested, and HoldClaimReleased are real Run-stream events without that prefix) against a PII deny-list covering this slice's own fields (observed_path, capture_path, full_file_name) plus the User* PVs a later slice already names as blocked. A regression here usually means someone tried to carry the resolved path (or a raw substrate PV) onto an event for convenience; it belongs in the run_capture_path vault via CapturePathStore instead.
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
2bmSP2:HDF1:FullFileName_RBV(the areaDetector file plugin's own readback, written at file OPEN) instead of tomoscan's ownFullFileNamePV, which upstream'send_scan()writes four statements after CORA's terminal trigger and would return the previous scan's filename.run_capture_pathPII vault (RLS+FORCE), mirroring the shippedactor_profile/ProfileStorepattern.capture_code(already onRunStarted.external_refs) folds ontoproj_run_summaryand surfaces vialist_runs.observed_capture_pathresolves from the vault insideget_run's own handler (RunView, mirroringget_actor'sActorView) — deliberately not inlist_runs, which is one shared handler instance read by internal composition-root callers under one coarse, unscoped-per-row grant.capture_watch_preflightredacts the newfull_file_namerole (kind + length only), with defense-in-depth redaction for any path-shaped value regardless of role-key match.capture_path_recording_enabled, boot-gated like the existing four.Runevent payload ever carries a PII-shaped field.Test plan
make test-noio— 47926 passed, 630 skippedmake test-db— 1203 passed, 1 skippedmake lint(ruff check + format --check, full tree)make typecheck(pyright, 0 errors)tach check— all modules validated🤖 Generated with Claude Code