Skip to content

feat: verify_record consumes the revocation bundle and reports its check - #271

Merged
imran-siddique merged 2 commits into
agentrust-io:mainfrom
opento-suggestions:feat/revocation-consumer
Sep 2, 2026
Merged

feat: verify_record consumes the revocation bundle and reports its check#271
imran-siddique merged 2 commits into
agentrust-io:mainfrom
opento-suggestions:feat/revocation-consumer

Conversation

@opento-suggestions

@opento-suggestions opento-suggestions commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

What this changes

spec/trace-v0.2.md section 3.2.3 publishes TraceRevocationBundle/1.0 and states what a verifier reports under it:

An expired bundle is not a pass. A verifier whose newest bundle is older than the profile's maximum age MUST report the record as unverified for revocation rather than as verified, and a verifier with no bundle at all MUST report that it performed no revocation check. Neither may be reported as an affirming appraisal.

The bundle format merged with #187 and nothing consumed it. #246 records the consequence: verify_record() returned None whether 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, and issued_at as the only field an age can be measured from.

verify_record() gains revocation_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) and now, and returns a VerificationResult. Its revocation field carries outcome, one of section 3.2.3's three states in the section's own words; cause, when a supplied bundle could not ground verified; and evidence, 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 verified with source: "store" and no horizon, since a store has none; and now governs 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.py runs 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 to schema/. docs/verification.md is 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. No appraisal.status value is named anywhere in the module.

Closes #246.

Type of change

  • Editorial (typo, link fix, clarification)
  • Non-breaking spec change
  • Breaking spec change
  • Schema change
  • Example addition
  • Reference implementation: verify_record() returns a value where it returned None. Callers ignoring the return are unaffected; no test in the tree asserts is None on it. CHANGELOG.md entry 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

  • DCO sign-off on all commits (git commit -s)
  • CHANGELOG.md updated
  • Breaking changes marked in spec text (no spec text changes)
  • Backward compatibility statement (not a breaking spec change; the API note above is the compatibility statement for the package)

Amendment, 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.

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>
@opento-suggestions
opento-suggestions requested a review from a team as a code owner September 1, 2026 16:14

@lywinged lywinged left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
@opento-suggestions

Copy link
Copy Markdown
Contributor Author

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 docs/verification.md now state the ordering and the reason beside it. Credit is in the trailer.

On the store-grade verified: kept as is for now, with the evidence field as the discriminator, because a fourth outcome value would name something section 3.2.3 does not, and the vocabulary across the four threads is being settled on #190. Happy to revisit once it is.

@lywinged lywinged left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 imran-siddique left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

verify_record reports a pass when no revocation check was performed, where §3.2.3 requires it to report that none was performed

3 participants