Transaction-Science/open-standards
Watch
1
0
Fork
You've already forked open-standards
 0
Code Issues Pull requests Projects Releases 15 Packages Wiki Activity Actions

op-screening: the audit log can outlive the process that wrote it #295

Merged
dcharlot merged 1 commit from fix/openpay-audit-log-survives-restart into main 2026-10-17 16:02:58 -04:00
Conversation 0 Commits 1 Files changed 2 +264 -29
dcharlot commented 2026-10-17 09:37:56 -04:00
Owner
Copy link

First of the "state claimed durable that dies with the process" family.

The defect

op-screening's crate header claims every Screener::screen call is recorded into a chained, signed audit log

"so that regulators can inspect the screening history of any payment attempt after the fact."

The log could not do that. Its entire public surface was new, verifying_key, entries, record, verify — no save, no load, and no way to supply an existing key. The only constructor mints a fresh keypair from OsRng, and every call site in the repo is AuditLog::new().

So the trail died with the process, twice over:

  1. The rows were gone. No persistence at all.
  2. The key was gone — the worse half. On the next run new() mints a new keypair, so the verifying key a regulator ingested out-of-band no longer matches anything, and rows an operator had dumped to disk by hand could never be verified by any process: verify only ever checked against the key the running log happened to own.

"After the fact" was the one time the log could not be read.

The fix

The same crate already persists its other long-lived state — SanctionsIndex::save_snapshot / load_snapshot, CBOR via ciborium — so this follows that precedent rather than inventing one:

  • AuditLog::with_signing_key signs with a key the operator already holds, instead of rotating it on every start.
  • save_entries / load_entries persist the rows. The signing key is deliberately not written: a private key does not belong in a file beside the rows it signed, and this crate ships compliance/hsm-kms-guidance.md saying where it does belong.
  • resume(signing_key, entries) continues the chain onto its real head, so the next row's prev_hash and seq follow the pre-restart rows rather than starting over. It verifies them first: resuming onto rows this key did not write would build a log that fails verification later, at the regulator, over a chain nobody can repair — so it fails here instead.
  • verify_entries(entries, verifying_key) is a free function — the regulator's path: rows plus the published public key, with no signing key and no live log. AuditLog::verify delegates to it, so the two invariants have one implementation rather than two that can drift.

Additive throughout: new, Default and every existing call site are unchanged.

Tests

  • a_restart_continues_the_chain — two rows written, saved, loaded, resumed, a third appended; the whole chain verifies and seq continues at 2 rather than restarting at 0.
  • a_regulator_verifies_dumped_rows_with_only_the_public_key — verification from the rows and the published key alone, and both invariants still bite on the detached rows: an edited row breaks its chain hash at index 0, a removed row breaks the chain.
  • resuming_with_the_wrong_key_is_refused.
  • a_fresh_log_cannot_verify_another_runs_rows — pins the limit of new() that motivated all of it, so it reads as a known constraint rather than something a regulator discovers.

Falsification

mutation result
A1 resume skips verifying the rows it resumes onto only resuming_with_the_wrong_key_is_refused FAILED
A2 save_entries persists an empty vec only the two persistence tests FAILED
A3 verify_entries always returns Ok 5 FAILED — including the pre-existing tamper_breaks_chain and tamper_signature_only_breaks

A3 is the one worth noting: moving verify()'s body into the free verify_entries is a refactor of security-relevant code, and A3 shows the two pre-existing tamper tests still depend on it — so the behaviour they pin was preserved, not re-implemented beside them.

Verification

On the build host (aarch64), the CI-equivalent gate for openpay:

  • cargo check --workspace --all-targets — exit 0
  • cargo test --workspace — exit 0, 0 failed suites, 163 passing suites
  • cargo test -p op-screening — 45 + 4 pass, including the pre-existing audit_log_records_every_call_and_verifies integration test

One thing I did not change

AuditLog::new() still exists and still mints a fresh keypair. It is the right constructor for a one-shot process and for the ~10 test call sites, and removing it would be a breaking change that buys nothing the new constructors do not already offer. The limit is now documented on it and pinned by a test instead.

🤖 Generated with Claude Code

First of the "state claimed durable that dies with the process" family. ## The defect `op-screening`'s crate header claims every `Screener::screen` call is recorded into a chained, signed audit log > *"so that regulators can inspect the screening history of any payment attempt **after the fact**."* The log could not do that. Its entire public surface was `new`, `verifying_key`, `entries`, `record`, `verify` — **no save, no load, and no way to supply an existing key.** The only constructor mints a fresh keypair from `OsRng`, and every call site in the repo is `AuditLog::new()`. So the trail died with the process, twice over: 1. **The rows were gone.** No persistence at all. 2. **The key was gone** — the worse half. On the next run `new()` mints a new keypair, so the verifying key a regulator ingested out-of-band no longer matches anything, and rows an operator had dumped to disk by hand could never be verified by *any* process: `verify` only ever checked against the key the running log happened to own. "After the fact" was the one time the log could not be read. ## The fix The same crate already persists its other long-lived state — `SanctionsIndex::save_snapshot` / `load_snapshot`, CBOR via ciborium — so this follows that precedent rather than inventing one: - **`AuditLog::with_signing_key`** signs with a key the operator already holds, instead of rotating it on every start. - **`save_entries` / `load_entries`** persist the rows. The signing key is deliberately **not** written: a private key does not belong in a file beside the rows it signed, and this crate ships `compliance/hsm-kms-guidance.md` saying where it does belong. - **`resume(signing_key, entries)`** continues the chain onto its real head, so the next row's `prev_hash` and `seq` follow the pre-restart rows rather than starting over. It **verifies them first**: resuming onto rows this key did not write would build a log that fails verification later, at the regulator, over a chain nobody can repair — so it fails here instead. - **`verify_entries(entries, verifying_key)`** is a free function — the regulator's path: rows plus the published public key, with no signing key and no live log. `AuditLog::verify` delegates to it, so the two invariants have one implementation rather than two that can drift. **Additive throughout**: `new`, `Default` and every existing call site are unchanged. ## Tests - `a_restart_continues_the_chain` — two rows written, saved, loaded, resumed, a third appended; the whole chain verifies and `seq` continues at 2 rather than restarting at 0. - `a_regulator_verifies_dumped_rows_with_only_the_public_key` — verification from the rows and the published key alone, and both invariants still bite on the *detached* rows: an edited row breaks its chain hash at index 0, a removed row breaks the chain. - `resuming_with_the_wrong_key_is_refused`. - `a_fresh_log_cannot_verify_another_runs_rows` — pins the limit of `new()` that motivated all of it, so it reads as a known constraint rather than something a regulator discovers. ### Falsification | mutation | result | |---|---| | **A1** `resume` skips verifying the rows it resumes onto | **only** `resuming_with_the_wrong_key_is_refused` FAILED | | **A2** `save_entries` persists an empty vec | **only** the two persistence tests FAILED | | **A3** `verify_entries` always returns `Ok` | **5** FAILED — including the **pre-existing** `tamper_breaks_chain` and `tamper_signature_only_breaks` | A3 is the one worth noting: moving `verify()`'s body into the free `verify_entries` is a refactor of security-relevant code, and A3 shows the two pre-existing tamper tests still depend on it — so the behaviour they pin was preserved, not re-implemented beside them. ## Verification On the build host (aarch64), the CI-equivalent gate for `openpay`: - `cargo check --workspace --all-targets` — exit **0** - `cargo test --workspace` — exit **0**, **0** failed suites, **163** passing suites - `cargo test -p op-screening` — 45 + 4 pass, including the pre-existing `audit_log_records_every_call_and_verifies` integration test ## One thing I did not change `AuditLog::new()` still exists and still mints a fresh keypair. It is the right constructor for a one-shot process and for the ~10 test call sites, and removing it would be a breaking change that buys nothing the new constructors do not already offer. The limit is now documented on it and pinned by a test instead. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
op-screening: the audit log can outlive the process that wrote it
All checks were successful
standards workspaces build + test / which workspaces this change touches (pull_request) Successful in 13s
Details
standards workspaces build + test / macOS energy backends (pull_request) Has been skipped
Details
repository checks / lockfiles, tests in CI, workflow hygiene, spec and section references, public text (pull_request) Successful in 1m59s
Details
standards workspaces build + test / openpay (openpay, openpay, linux-arm64) (pull_request) Successful in 5m24s
Details
209e39c62d 
The crate header claims every `Screener::screen` call is recorded into a
chained, signed audit log "so that regulators can inspect the screening
history of any payment attempt after the fact". The log could not do
that. Its whole public surface was `new`, `verifying_key`, `entries`,
`record` and `verify`: no save, no load, and no way to supply an
existing key. The only constructor mints a fresh keypair from `OsRng`,
and every call site in the repo is `AuditLog::new()`.

So the trail died with the process, twice over. The rows were gone, and
the key was gone — which is the worse half: on the next run `new()`
mints a new keypair, so the verifying key a regulator ingested
out-of-band no longer matches anything, and rows an operator had
dumped to disk by hand could never be verified by any process, because
`verify` only ever checked against the key the running log happened to
own. "After the fact" was the one time the log could not be read.

The same crate already persists its other long-lived state —
`SanctionsIndex::save_snapshot` / `load_snapshot`, CBOR via ciborium —
so this follows that precedent rather than inventing one:

- `AuditLog::with_signing_key` signs with a key the operator already
  holds, instead of rotating it on every start.
- `save_entries` / `load_entries` persist the rows. The signing key is
  deliberately **not** written: a private key does not belong in a file
  beside the rows it signed, and this crate ships
  `compliance/hsm-kms-guidance.md` saying where it does belong.
- `resume(signing_key, entries)` continues the chain onto its real head,
  so the next row's `prev_hash` and `seq` follow the pre-restart rows
  rather than starting over. It verifies them first: resuming onto rows
  this key did not write would build a log that fails verification
  later, at the regulator, over a chain nobody can repair — so it fails
  here instead.
- `verify_entries(entries, verifying_key)` is a free function, which is
  the regulator's path: rows plus the published public key, with no
  signing key and no live log. `AuditLog::verify` delegates to it, so
  the two invariants have one implementation rather than two that can
  drift.

Additive throughout — `new`, `Default` and every existing call site are
unchanged.

Tests: a restart writes two rows, loads them back, resumes and appends a
third, and the whole chain verifies with `seq` continuing at 2; a
regulator verifies dumped rows from the public key alone, and both
invariants still bite on the detached rows (an edited row breaks its
chain hash at index 0, a removed row breaks the chain); resuming under
the wrong key is refused; and `a_fresh_log_cannot_verify_another_runs_rows`
pins the limit of `new()` that motivated all of it.

Co-Authored-By: Claude Opus 5 <[email protected]>
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
Transaction-Science/open-standards!295
No description provided.