feat: verify_record consumes the revocation bundle and reports its check - #271
Conversation
Section 3.2.3 publishes TraceRevocationBundle/1.0 and nothing read it: valid_until appeared zero times under src/. The section names three states a verifier must distinguish and forbids reporting any of them as an affirming appraisal. verify_record now returns a VerificationResult whose revocation field carries one of them as a value, in the section's own words: verified, unverified_for_revocation, or no_check_performed, with the cause and the evidence a second run needs to reach the same outcome from retained facts. Previously the function returned None and a caller could not tell a verified key from one nobody checked (agentrust-io#246). New parameters: revocation_bundle, trusted_bundle_keys, max_bundle_age_seconds, and now. Two bounds govern bundle age, the issuer's valid_until and the caller's maximum measured from issued_at, and the tighter governs: a deployment can be stricter than an issuer and is never forced looser (agentrust-io#190). An expired outcome names which bound tripped. Bounds are inclusive on the valid side. now pins one clock for both the record freshness check and the bundle age check; it defaults to the wall clock and a vector supplies it. A bundle that is malformed, signed by a key the caller does not trust for bundles, signed with an algorithm this build cannot verify, or issued in the future yields unverified_for_revocation with the cause named. It does not raise: inability to check is not evidence of a defect. A statement on the bundle's log naming the trusted key raises, under the section 3.2.3 fallback the existing store already implements. A store that answers "not listed" reports verified with source "store" and no horizon. Neither a bundle nor a store reports no_check_performed. Schema validation resolves the statement $ref from copies packaged with the module through a referencing.Registry, never over the network; a test validates with sockets refused and fails legibly if that changes. referencing is declared because it is imported. examples/revocation-bundle/ carries 25 generated vectors: the four rows of the truth table over the two bounds, each boundary with a one-second margin vector, the two inclusive boundaries, and every non-verified state twice. The generator writes LF bytes explicitly so the set does not depend on the platform's text-mode convention. The test file implements the five candidate staleness rules and two grace-period shortcuts as stubs; over the nine age vectors only tighter-governs survives. The set is registered with the adequacy harness and both packaged schemas are classified and held byte-identical to schema/. Not done here, and stated in the docstring rather than implied: entry-ID scoped revocation (no entry ID reaches verify_record), and per-statement signature verification against the section 3.2.1 hierarchy. Which appraisal.status value an unresolved check carries stays open on agentrust-io#190. Closes agentrust-io#246. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: opento-suggestions <opentosuggestionsofficial@gmail.com>
lywinged
left a comment
There was a problem hiding this comment.
Not approving yet, on one thing in the ordering inside check_bundle. Everything else here is
in good shape and I say what I checked at the end.
A bundle that names this key as compromised is ignored once it is stale. 3e returns on the
age bounds before 3f scans the statements, so an authenticated bundle carrying a statement for
the trusted key stops being read the moment it ages out. Reproduced on the PR's own vector 11,
with nothing switched off: a three-day record tolerance and a one-day bundle tolerance, which is
the ordinary posture for archived records checked against a current bundle.
bundle fresh REJECTED signing key is revoked: statement on log ...
bundle 10s past valid_until ACCEPTED outcome=unverified_for_revocation cause=bundle_expired
Same bundle, same signature, same statement, ten seconds apart, and the record is inside its own
freshness bound in both runs. The signature was already
verified at 3c, so the statement is authenticated evidence, not untrusted input.
The asymmetry is what decides it for me. Acting on a stale statement risks rejecting a key
whose revocation was later withdrawn, which is visible and recoverable; ignoring one risks
accepting a compromised key in the single check whose purpose is catching exactly that, which is
neither. Section 3.2.3 also gives a statement no expiry of its own: it fixes a boundary in log
order through last_valid_entry_id, and the section describes no way to withdraw one, so within
the model a statement does not stop being true when its carrier ages out. The schema draws the
same line: valid_until is a field of the bundle, and none of the statement's nine fields is an
expiry, so expiry is a property of the carrier and not of the statement it carries.
Your 3e comment says the bundle
is evidence only while both bounds hold, which is right about its silence, since the freshness
bound is what makes an absent statement mean anything. It is not needed to make a present one
mean something.
Moving the 3f scan above 3e is the whole change. I ran it: the stale case rejects, and all
46 tests still pass, because the six expiry vectors carry no statements at all and fall
through to the age rule unchanged.
I do not think this is a mistake so much as an unstated interaction: §3.2.3 mandates the outcome
for a stale bundle and says nothing about one that carries a positive hit, so the implementation
has to choose and the ordering chose. Either resolution is fine by me as long as it is chosen
rather than inherited, and if you decide the current order is right I would want the reason in
the docstring next to 3e. That is the only thing standing between this and an approval.
On the design choice you flagged: a store answering "not listed" reports outcome="verified"
with no horizon. My worry is that a caller switching on outcome alone cannot tell bundle-grade
assurance from store-grade, since only the evidence distinguishes them, and §3.2.3's "verified"
is written about a bundle valid at a stated time. Not a blocker, and I do not have a better
value to offer from the three the section names.
What I checked, since a review of a module like this is worth little unspecified: the staleness
rule is load-bearing (replacing tighter-governs with issuer-only fails four tests, and the
one-second vector 05 is what catches it); I7 and I9 both fail when violated, so the network
and appraisal.status guards have teeth; all six Cause values are exercised by vectors; no
test in the tree asserts is None on verify_record, so the return-type change is safe as
claimed; both packaged revocation schemas are byte-identical to their sources and pinned by a
test; and no path reaches verified without the signature verifying first. Suite is 1140 passed
1 skipped, ruff and mypy clean, and it merges cleanly onto current main.
check_bundle applied the two age bounds before scanning the bundle's statements, so a bundle carrying a statement that names the trusted key stopped being read the moment it aged out: the same bundle, signature and statement rejected the record ten seconds before valid_until and reported unverified_for_revocation ten seconds after, on either bound. The bounds govern what the bundle's silence is worth. An absent statement means "none known as of issued_at", which is informative only while issued_at is recent. A present statement was authenticated with the bundle's signature at step 3c, section 3.2.3 gives it no expiry and no withdrawal, and the schema puts valid_until on the bundle rather than on the statement. Acting on a stale statement risks rejecting a key whose revocation was later withdrawn, which is visible and recoverable; ignoring one risks accepting a compromised key in the one check built to catch that. The statement scan now runs directly after signature verification, ahead of both the future-dated check and the age check. Three vectors carry the same statement as vector 11 inside bundles that are stale by the issuer bound, stale by the deployment bound, and dated in the future. All three reject; their statement-free counterparts (07, 05, 24) still report unverified, so the ordering is what separates them and reversing it turns the three red. The six existing expiry vectors carry no statements and are unchanged. The docstrings and docs/verification.md state the ordering and the reason, since section 3.2.3 mandates the outcome for a stale bundle and says nothing about one carrying a positive hit; the implementation has to choose, and this chooses rather than inherits. Raised in review of agentrust-io#271 by lywinged, who supplied the argument, the reordering, and the run. Co-authored-by: LouieLuNZ <48041247+lywinged@users.noreply.github.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: opento-suggestions <opentosuggestionsofficial@gmail.com>
|
You are right, and the distinction is the one that matters: the bounds say what the bundle's silence is worth, and a present statement needs no clock to mean what it says. The ordering was inherited from the shape of the procedure rather than chosen, which is exactly what you named. b32e4a1 moves the statement scan to directly after signature verification, ahead of both the future-dated check and the age check. Three new vectors carry vector 11's statement inside bundles stale by each bound and dated in the future; all three reject, and their statement-free counterparts still report unverified, so reversing the order turns the three red. The docstrings and On the store-grade |
lywinged
left a comment
There was a problem hiding this comment.
Approved, on b32e4a1.
The scan is at 3d now, ahead of both time checks, and the docstring carries the reason rather
than the fact alone. The two things that make the ordering right are both in it: section 3.2.3
gives a statement no expiry and no withdrawal, and the schema puts valid_until on the bundle
rather than on the statement. That is the argument, not a restatement of the change.
I checked it on my own reproduction rather than only on your vectors. Same bundle, same statement
naming the trusted key, four values of now: fresh, deployment bound tripped by one second,
issuer bound tripped by one second, and one set far enough back that the bundle sits in the
future beyond the skew. On bfcc76e one of the four rejects. On b32e4a1 all four do.
Your own description of the guard is understated. Reversing the order does not turn three red, it
turns four: the three vectors plus
test_I11_an_authenticated_statement_is_read_before_either_time_check, which pins the invariant
rather than the cases. Someone reading only the vectors would miss that the property is held
separately from the examples of it.
What I looked hardest at is the risk this change actually carries, which runs the other way:
whether moving the scan earlier lets a stale bundle still reach verified for a key it says
nothing about. It does not, because 3f still runs when no statement matches, and the vectors
are built so that this is checkable rather than asserted. 26, 27 and 28 differ from 06/07, 04/05
and 23/24 by exactly one thing, the statement, and the statement-free half still reports
unverified_for_revocation with the bound named. The pairing is what makes the claim
inspectable.
One thing you fixed that I had not raised: docs/verification.md listed the causes that yield
unverified_for_revocation and did not include expiry under either bound, which was already
wrong before this commit. It is in the list now.
On the store-grade verified, keep it as you have. A fourth outcome value would name a state
section 3.2.3 does not have, and inventing vocabulary ahead of the spec is the thing this module
is otherwise careful not to do. The evidence field discriminates it today and #190 is where the
rest belongs.
What I ran on this head: the suite is 1144 passed 1 skipped, and
tests/test_revocation_bundle.py is 50 passed. ruff and mypy are clean. Re-running
gen_revocation_vectors.py leaves the tree byte-identical, so the three new vectors are
generated rather than hand-written beside a generator that no longer produces them. It merges
onto main at ab3f06d with no conflict.
imran-siddique
left a comment
There was a problem hiding this comment.
Merging, and closing #246 with it.
All four calls from #190 are implemented as ruled, not approximated. Tighter-governs precedence. The third state as a first-class value in the result rather than an out-parameter or a separate entry point. Vectors A, B and D all present, and you added one-second boundary variants of each, which is the version that catches an off-by-one in a comparison operator. issued_at as the only field an age can be measured from, with the reasoning I gave: not because the spec says so, but because it is the only candidate the schema offers.
Carrying section 3.2.3's three states in the section's own words is the detail that makes this durable. A result vocabulary paraphrased from a specification drifts from it; one quoted from it cannot.
The amendment is the most important thing in this PR.
Review found that the statement scan ran behind the age checks, so an authenticated statement naming the key was ignored once its bundle aged out.
That is a fail-open on revocation, which is the one direction that must never happen: a verifier holding a signed statement that names the compromised key, ignoring it because the bundle carrying it is stale, and reporting the record as fine. Reading statements directly after signature verification and ahead of both time checks is right, and pinning it with vectors 26, 27 and 28 rather than a comment is what stops it returning.
26-stale-by-issuer-statement-still-rejects and 27-stale-by-deployment-statement-still-rejects are the two names I would look for, and they are both there.
On the API change. Stating it plainly in the PR body, with the reason it is safe (callers ignoring the return are unaffected, no test asserts is None) and a CHANGELOG entry, is the right way to ship a return-type change. Saying the description says 25 vectors while the set is now 28, rather than quietly editing the number, is the same instinct.
What I verified and what I did not. I checked the four rulings are present, the vector names cover the discrimination set and both absence cases, the amendment's fix is described at the right layer, lywinged's approval, and that five of six checks are green with only a stale gate entry alongside a passing one. I did not run the 28 vectors or line-read the 4,069 lines. I am merging on the generator, the peer approval, and the fact that the one defect most likely to be fatal was found in review and pinned.
@lywinged, the approval carried weight here. You built the discrimination table this consumer is tested against and then reviewed the consumer; that is the closest thing to independent verification this repository has.
@opento-suggestions, thank you for taking the consumer lane and for the amendment note. Recording that the description and the vector count had diverged, in the body, rather than silently reconciling them, is why I could review the amendment separately from the original.
What this changes
spec/trace-v0.2.mdsection 3.2.3 publishesTraceRevocationBundle/1.0and states what a verifier reports under it:The bundle format merged with #187 and nothing consumed it. #246 records the consequence:
verify_record()returnedNonewhether a revocation check ran or not. This PR is the consumer lane from #190, built against the calls settled there on 2026-08-30: tighter-governs precedence, the third state as a first-class value in the result, vectors A, B and D all kept, andissued_atas the only field an age can be measured from.verify_record()gainsrevocation_bundle,trusted_bundle_keys,max_bundle_age_seconds(86400 is section 3.2.2's default, applied to bundles by 3.2.3's "same maximum-age model" sentence; 3.2.3 names no bundle default of its own) andnow, and returns aVerificationResult. Itsrevocationfield carriesoutcome, one of section 3.2.3's three states in the section's own words;cause, when a supplied bundle could not groundverified; andevidence, the retained facts a second run needs. The outcome is a value in the result rather than an exception or a separate entry point, which is the shape chosen on #190; a caller who discards the result has the old fail-open behaviour, and a test pins that silence as a fact rather than leaving it as a warning.Two design choices this PR makes that the thread did not settle, offered for review rather than assumed: a store that answers "not listed" reports
verifiedwithsource: "store"and no horizon, since a store has none; andnowgoverns both the record freshness check and the bundle age check, one clock per call, which is why the freshness code moved by a few lines (adjacent to #239, different lines).examples/revocation-bundle/carries 25 generated vectors with a README.tests/test_revocation_bundle.pyruns them, executes the invariants the module docstring states, implements the five candidate staleness rules and two grace-period shortcuts as stubs (over the nine age vectors, only tighter-governs survives), validates a bundle with sockets refused, and holds the packaged schema copies byte-identical toschema/.docs/verification.mdis corrected where it described the old return.Not in this PR, stated in the module docstring: entry-ID scoped revocation, per-statement signature verification against the section 3.2.1 hierarchy, and a row in
verification-outcome-statements.md, which can follow once the vocabulary across the four threads named on #190 is settled. Noappraisal.statusvalue is named anywhere in the module.Closes #246.
Type of change
verify_record()returns a value where it returnedNone. Callers ignoring the return are unaffected; no test in the tree assertsis Noneon it.CHANGELOG.mdentry added.Spec section
Section 3.2.3 (implemented; no normative text changes). Section 3.2.2 for the maximum-age model the default is drawn from.
Checklist
git commit -s)CHANGELOG.mdupdatedAmendment, 2026-09-01. Review found that the statement scan ran behind the age checks, so an authenticated statement naming the key was ignored once its bundle aged out. b32e4a1 reads statements directly after signature verification, ahead of both time checks, and adds three vectors (26, 27, 28) pinning that; the set is 28 vectors where this description says 25. The result type, the two design choices offered for review, and the stated limitations are unchanged.