Indirect locators, so the record carries no personal names - #689
Merged
Conversation
The 2-BM pilot's scan-file directories embed a real person's surname
(2026-08-Haridy-1015116, DMagic's {yyyy-mm}-{PIlastname}-{GUP#}
convention). CaptureScanIngestor (slice 17, merged, switch still off)
writes that path directly onto DatasetRegistered.uri /
DistributionRegistered.uri, immutable INSERT-only events with no
erasure path. The disposition table already drops uri from the
published record and has no other person-name field anywhere, so
this would introduce the record's only personal name, and do it
automatically for every run once the switch goes on.
CaptureScanIngestor now mints an indirect cora-capture-path://
locator instead: host and tier stay visible (real, non-personal
provenance), the personal segment is replaced with a run-<uuid>
token resolved through run_capture_path (the same erasable vault
capture_path.py already keeps observed_path in). ingest_scan's
handler resolves this back to the real path once, before the reader
and checksum computer ever see it; the event still records the
indirect form. The manual POST route and MCP tool are unaffected,
since resolution is a pass-through for every other scheme.
Naming review caught a lock-time mistake before it shipped: the
working name cora-vault collided with this codebase's OTHER,
unrelated PII vault (actor_profile), which would have been permanent
once written to an event. Renamed to cora-capture-path, matching the
already-established capture_path/CapturePathStore/run_capture_path
family instead of inventing a second one.
Gate review (4-agent panel, security specialist declared for the
same reason as slice 17) found one real P0: Unreadable.reason and
Unreachable.error_detail are sourced from bare os.stat/h5py error
text that embeds whatever path the reader actually opened, and
InvalidScanFileError carries that text verbatim into an HTTP 400.
For a resolved indirect locator this would echo the real,
personal-data path back to a caller who submitted a reference they
had no prior right to -- the exact disclosure this slice exists to
prevent, via a different door. Fixed: the handler tracks whether
resolution substituted a different string and redacts the detail
text only in that case; a direct file:// caller, who supplied the
path themselves, sees unchanged behavior.
Also fixed: two P1s narrowing the CapturePathStore dependency to a
get-only CapturePathLookup Protocol per this codebase's
port-shaped-by-consumer convention, and correcting a false "same
pool" claim in a docstring for the in-memory deployment branch; two
P1 test gaps (a "genuine independent check" test that never actually
exercised drift, and an unreached branch in resolve's segment
parsing); and a bounded, documented-not-code-fixed P1 covering a
caller who holds IngestScan but not read access to Run capture
paths crafting a locator that names a different run's bytes.
Two of the plan's originally-listed fix sites (launch_argv, the
compute port) turned out to need no change: nothing today wires a
Dataset/Distribution URI into a launch spec automatically, and the
existing allowlist would refuse it loudly if that ever changed. A
third (record_attestation) is deliberately NOT fixed here, correcting
the plan: SshPosixChecksumComputer already deferred building an
SSH-capable ChecksumVerifier "per the rule-of-three" in slice 17, and
attestation over an SSH-sourced Distribution was unverifiable before
this slice regardless of locator scheme.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
xmap
force-pushed
the
worktree-indirect-locators-slice18
branch
from
August 19, 2026 22:11
1e0d739 to
f605250
Compare
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.
Why
2-BM's scan-file directories embed a real person's surname (
2026-08-Haridy-1015116, DMagic's{yyyy-mm}-{PIlastname}-{GUP#}convention).CaptureScanIngestor(slice 17, merged, switch still off) writes that path directly ontoDatasetRegistered.uri/DistributionRegistered.uri, immutable, INSERT-only events with no erasure path.The disposition table already drops
urifrom the published record and has no other person-name field anywhere in this codebase's event schema. This would introduce the record's only personal name, automatically, for every run, the moment the switch goes on.What changed
CaptureScanIngestornow mints an indirectcora-capture-path://locator instead of a real path:Host and tier stay visible (real, non-personal provenance, useful to a reader). The personal segment is replaced with a
run-<uuid>token resolved throughrun_capture_path— the same erasable vaultcapture_path.pyalready keepsobserved_pathin, for the identical reason.ingest_scan's handler resolves this back to the real path once, before the reader/checksum computer ever see it. The event still records the indirect form. The manual POST route and MCP tool are unaffected — resolution is a pass-through for every other scheme.Naming caught before it locked
Gate review's naming pass (auto-triggered per this repo's convention on new names) found the working name
cora-vaultcollided with this codebase's other, unrelated PII vault (actor_profile). Since a scheme string here is permanent once written to an event, this was fixed before commit: renamed tocora-capture-path, matching the already-establishedcapture_path/CapturePathStore/run_capture_pathfamily instead of inventing a second one.Gate review
4-agent panel (architecture / test-coverage / cross-BC consistency / security specialist). One real P0:
Unreadable.reason/Unreachable.error_detailare sourced from bareos.stat/h5py error text that embeds whatever path the reader actually opened, andInvalidScanFileErrorcarries that text verbatim into an HTTP 400. For a resolved indirect locator this would echo the real, personal-data path back to a caller who submitted a reference they had no prior right to — the exact disclosure this slice exists to prevent, via a different door. Fixed: the handler tracks whether resolution substituted a different string and redacts the detail text only in that case; a directfile://caller (who supplied the path themselves) sees unchanged behavior.Also fixed: two P1s narrowing
CapturePathStoreto a get-onlyCapturePathLookupProtocol (this codebase's port-shaped-by-consumer convention) and correcting a false "same pool" docstring claim for the in-memory deployment branch; two P1 test gaps (a "genuine independent check" test that never actually exercised drift between two reads, and an unreached branch in the locator parser); and one bounded, documented-not-code-fixed P1: a caller holdingIngestScanbut not read access to Run capture paths could craft a locator naming a different run's bytes. Fixing that structurally would couple this Data BC module to a specificcora.api-owned agent identity — a worse cross-layer dependency than the risk it closes — so it's recorded as a deployment-authz responsibility in the module's own docstring instead.Scope corrections from the plan
Two originally-planned fix sites (
launch_argv, the compute port) needed no change: nothing today wires a Dataset/Distribution URI into a launch spec automatically, and the existing allowlist would refuse it loudly if that ever changed. A third (record_attestation) is deliberately not fixed:SshPosixChecksumComputeralready deferred building an SSH-capableChecksumVerifier"per the rule-of-three" in slice 17, and attestation over an SSH-sourced Distribution was unverifiable before this slice regardless of locator scheme — building that verifier now would be exactly the mistake the deferral warns against.Risk
Inert on merge, same as slice 17:
capture_scan_ingestor_enabledstill defaults off. 31,181 architecture tests, 13,847 unit tests, 1,261 integration tests, pyright/ruff/tach all clean.🤖 Generated with Claude Code