feat(audit): stamp the request id the audit table has always had a column for - #240
Open
LKSNDRTMLKV wants to merge 1 commit into
Open
feat(audit): stamp the request id the audit table has always had a column for#240LKSNDRTMLKV wants to merge 1 commit into
LKSNDRTMLKV wants to merge 1 commit into
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 9 |
| Duplication | -1 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
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.
odal.passport_audit.request_idhas existed since migration0005and nothinghas ever written to it. Every row on disk has
NULLthere. This wires it.Why the id is not threaded through the service layer
PassportAuditEntryis built in eleven places — create, publish, the fourlifecycle transitions, EOL, the three transfer legs, the credentialed read.
Stamping at each of them is eleven identical lines and a twelfth call site
added later that silently writes
NULL.They all funnel through
AuditRepository::append, so the id is stamped by adecorator on the port (
dpp_vault::infra::request_stamped_audit) wired in bothcomposition roots. No service signature moves, and a new call site cannot
forget.
The id reaches the decorator through a task-local scoped by the existing
inject_request_idmiddleware. That is the only ambient value in the codebaseand it stays that way deliberately: a request id is diagnostic, so a caller
that gets
Noneloses a support handle and nothing else. Nothing decides onit. The documented limitation — a task-local does not cross
tokio::spawn— isasserted in a test rather than only described.
Why it is not in the chain hash
Two reasons, either sufficient.
The column is thirty migrations old and every existing row has
NULL. Foldingit into
chain_hashwould change the hash input for entries whoseentry_hashis already committed, and
verify_audit_chainwould report every existingchain as tampered.
And it is not a claim about the passport. The chain makes a state change
tamper-evident; a correlation handle for a support conversation describes the
transport that carried it. Signing it would assert something the trail does not
mean.
chain_hash_matches_its_persisted_golden_valuenow setsrequest_idtoSomeand asserts the same golden hash — set it to
Noneand the test passeswhile proving nothing.
skip_serializing_ifis load-bearing for the same class of reason: an entryread back with
NULLserialises byte-identically to one written before thefield existed, so no already-signed evidence dossier's content hash moves.
Asserted in
an_unstamped_entry_does_not_serialise_the_field.Checks
fmt-check,clippy -D warnings, all 85 test binaries,openapi-check(bundles regenerated, Redocly clean), and the nine script gates —
debug,subjects,mod-rs,spec-version,grants,migrations,outbound,harness,contract-fixture— all pass.The OpenAPI contract gate is what forced
api/to be updated: the exhaustivefixture failed to compile until it set the field, then failed until the schema
documented it. Working as designed.
Not in this PR
Cargo.lockonmainis patch-polluted — the ninedpp-*core entries carryno
source/checksum, so they were committed with a local[patch.crates-io]active. Building without the patch rewrites 24 lines. CIdoes not build
--locked, so nothing catches it. Left out of this diff; worthits own issue.
Groundwork for #145 — the audit trail can now answer "did my first attempt
land?" for the passport routes, which is diagnosis, not idempotency.