VM-gate Part A: durable audit record grammar - #10
Merged
Merged
Conversation
Enrich the committed outcome with the durable fields the broker's RECORD_SCHEMA
carries, so the (soon) mutation-capable public API writes a complete, replayable
audit record rather than the minimal placeholder .outcome_record() emitted before.
Observer (R/verify.R):
- .observe() reads the committed preview's resolved records into the record's
`observed` object (the schema's only T_OBJECT field): per package:arch
{status, version} for txn/configure, {selection} for hold. Absent -> not
installed / "".
- .freeze_reader() caches installed()/selections() once so the verdict and the
observed snapshot come from ONE post-commit read -- no verify/observe TOCTOU,
one read not two.
- .state_changed(before, after) is a real observed diff, NA when either side is
unavailable (D7 = S-B: never inferred from effect_issued, which only means the
effect was issued, not that on-disk state moved).
Pre-commit snapshot (R/commit.R .commit_session):
- step 3.5 snapshots `before` via .observe() prior to authorization spend;
- step 6 .capture_post() takes the frozen post-read, captures verified +
verify_detail (unchanged 5b semantics) AND observed / observed_failed /
state_changed, never raising.
authorized_via provenance (R/polkit.R):
- .authorized_via() decides "pkexec" / "autonomous" / "pkcheck" AT the
authorization site (section 2.5); .authorize() now returns list(decision, via)
so the class is recorded from complete state, never reconstructed downstream.
Record grammar (R/commit.R):
- .PKGOPS_RECORD_FIELDS pins the broker's 16-field allow-list + types;
.validate_record() rejects any non-allow-list field, broker-reserved key, or
wrong type before a record is ever built (hermetic mirror of the broker guard).
- .outcome_record() maps the outcome onto the allow-list with exact [[ ]] access,
OMITTING NA/NULL optionals (absent == null at the broker); verified -> changed
only when the post-state was read, else changed is omitted (unknown, never a
false "did not change").
Observation is success-path only, matching approved 5b scope; the failure-path
`observed` shape stays deferred. No new dependency: the pkgstate reader stays
behind the injectable seam introduced in 5a (pkgstate already in Imports).
648 tinytest green.
Two review blockers on Part A: 1. .observe_hold() recorded only the FIRST selection row for an unqualified multi-arch target (row$selection[1L]). A hold record carries no architecture, so `nginx` can match nginx:amd64 + nginx:i386; collapsing to one row meant a change confined to the second arch left the pre/post `observed` identical and `state_changed` read FALSE, even though .verify_hold (which checks every row) correctly failed. Now every matched row is recorded under its own package:arch key, radix-ordered on architecture (locale-independent for a stable diff); a no-match target keeps its identity (bare package, or package:arch if named) with an NA selection. Regression test: only the second architecture changes -> state_changed TRUE. 2. apt.update writes no observed/changed/state_changed. update reads no per-package or index state, so .observe returns NULL (read_failed FALSE) and .state_changed returns NA -> all three omitted. Confirms the field is never a fabricated "did change". Test asserts an update record carries none of the three (observed_failed stays a real FALSE). The contract doc is corrected in lockstep (runix docs/pkgops-vm-gate-plan.md, PR #75): section 2.4 pins one package:arch entry per matched hold row, and the update rows/section 1.4 state the three-field omission explicitly. 660 tinytest.
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.
VM-gate increment — Part A (durable audit record grammar)
Draft, HELD. This is the code half of the VM-gated increment from
runix/docs/pkgops-vm-gate-plan.md(approved D1–D7). It merges before the VMproof (D3); the exported-API PR #9 stays draft until Part B (the disposable-VM
proof) is green. No VM has been provisioned — this PR is hermetic code + tests only.
What it does
Enriches the committed outcome with the fields the broker's
RECORD_SCHEMArecords, so the (soon) mutation-capable public API writes a complete, replayable
audit record instead of the earlier minimal placeholder
.outcome_record().R/verify.R)..observe()reads the committed preview's resolvedrecords into the record's
observedobject — the schema's only object field:{status, version}perpackage:archfor transaction/configure,{selection}for hold; absent → not-installed /
""..freeze_reader()caches the read so theverdict (
.verify()) and the observed snapshot come from one post-commit read.state_changedis an observed diff (D7 = S-B). A pre-commit.observe()snapshot (
.commit_sessionstep 3.5) is compared with the post-commit state;state_changedisNA(omitted) whenever either side is unavailable. It isnever inferred from
effect_issued— that means the effect was issued, notthat on-disk state moved.
authorized_viaprovenance (R/polkit.R, plan §2.5). Decided at theauthorization site as
"pkexec"/"autonomous"/"pkcheck";.authorize()now returns
list(decision, via)so the class is recorded from complete state,never reconstructed downstream.
R/commit.R)..outcome_record()maps the outcomeonto the broker's 16-field allow-list, omitting
NA/NULLoptionals (absent ==null at the broker);
verified→changedonly when the post-state was read,else
changedis omitted (unknown, never a false "did not change")..validate_record()mirrors the broker guard: rejects any non-allow-list field,broker-reserved key, or wrong type before a record is built.
Two things to flag for review
the
ok/no_opreturn path). The failure-pathobservedshape from plan §2.4 isdeliberately deferred — not implemented here.
.freeze_reader()makes the verdict and theobserved snapshot share one
installed()/selections()read, closing averify/observe TOCTOU and halving the post-commit reads. The verdict semantics
from 5a/5b are unchanged.
Scope / safety
(
pkgstatealready inImports).locally. That the local record survives the real broker is exactly what Part B
proves — the R adapter rejects reserved keys but does not itself enforce the
positive allow-list, so a hermetic pass is necessary but not sufficient.
R CMD check: 0 errors, 0 notes, 1 warning (New submission).