fix(schema): the subject pattern was a prefix test where the model requires an identity - #248
Conversation
|
❔ Contributor Check: UNKNOWN
Automated check by AgenTrust Contributor Check. |
…quires an identity `schema/trace-claim.json` held `subject` to `^(spiffe://|did:)`, which constrains only a leading `spiffe://` or `did:`. `models.py` requires `^(spiffe://[^/]+/.+|did:[a-z0-9]+:.+)$`. So `spiffe://bernstein.run`, a trust domain naming no workload, passed the published artifact a producer in any language validates against and was refused by `model_validate`. The schema is tightened to the model rather than the model loosened. A bare prefix is not a constraint on an identity, and the model's rule is DID Core section 3.1, whose ABNF fixes `method-char` to `%x61-7A / DIGIT`, so `did:X:abc` is not a conformant DID rather than one this package happens to refuse. The method-specific identifier keeps its case: `did:key:z6Mk...` is unaffected. The regex is hoisted to `_SUBJECT_RE` beside `_DIGEST_RE` and `_DURATION_RE`, the two patterns already shared verbatim between the model and the schema, and both schema copies now state the rule in prose instead of leaving it to be read off a regex. `tests/test_the_schema_and_the_models_agree.py` could not see this. Its matrix has 24 values and none of them starts with `spiffe://` or `did:`, so every one failed the prefix test too and the two validators agreed by both rejecting. Splitting them needs a value that passes the prefix and fails the shape. Four are added; with the schema reverted they fail, in both directions, naming all four. Signed-off-by: Yi Lu <louie.lunz@gmail.com>
ebdfcbb to
1f7665e
Compare
imran-siddique
left a comment
There was a problem hiding this comment.
This does the fix and then answers three questions nobody asked, each of which is worth more than the fix.
Why the parity test could not see it. This is the finding:
subject's pattern was a prefix test, so every value in its matrix failed it too and the two validators agreed by both rejecting.
A differential test that compares two validators and reports agreement is only as good as the inputs that separate them. Both surfaces rejecting the same wrong values reads identically to both surfaces being correct. Adding four values that pass the prefix and fail the shape is what turns it from a test that could not fail into one that could.
All ten patterns audited, one pair differing. That is the part that changes what I know rather than what I think. #247 was opened on the premise that these surfaces drift and nothing checks them; you have now established the schema-versus-model half is clean apart from this one, which narrows that issue considerably. I will record it there.
The bisect. 84df8ef, the pre-launch hardening in #49 on 2026-06-18, tightened the model and left the schema on the old string. Naming the commit matters because it dates the exposure: any producer validating against the published schema since June could emit a record the reference implementation refuses, and now we know for how long rather than guessing.
The rest is right too. Tightening the schema to the model rather than loosening the model is the ruling; hoisting to _SUBJECT_RE beside _DIGEST_RE and _DURATION_RE puts it with the two patterns already shared verbatim, so the next drift has a place to not happen; and correcting docs/schema.md catches the third surface, which I had not thought to ask for.
The DID Core citation carried through accurately: method-char = %x61-7A / DIGIT, so did:X:abc is not a conformant DID rather than one this package happens to refuse. Your note that the method-specific identifier is deliberately not constrained, so did:key:z6Mk... keeps its case, is the right boundary and worth having in the comment.
All five checks green. Merging, and this closes #244.
Closes #244.
What this changes
schema/trace-claim.jsonheldsubjectto"^(spiffe://|did:)", a test on a leading spiffe:// or did:.src/agentrust_trace/models.pyrequires^(spiffe://[^/]+/.+|did:[a-z0-9]+:.+)$. A record could therefore be conformant to the published artifact a producer in any language validates against, and refused bymodel_validate.Tightened to the model, per the ruling on #244, in both byte-identical schema copies. The regex is hoisted to
_SUBJECT_RE, beside_DIGEST_REand_DURATION_RE, which are the two patterns already shared verbatim between the model and the schema. Both copies now state the rule in prose rather than leaving it to be inferred from a regex.schema/trace-claim.json^(spiffe://|did:)^(spiffe://[^/]+/.+|did:[a-z0-9]+:.+)$src/agentrust_trace/schema/trace-v0.2.jsonmodels.py^(spiffe://[^/]+/.+|did:[a-z0-9]+:.+)$_SUBJECT_REdocs/schema.mdspiffe://) or DID (did:)"The two schema files are byte-identical before and after.
The two surfaces parted at a specific commit:
84df8ef, "security: pre-launch hardening" (#49), 2026-06-18, changed the model's pattern from^(spiffe://|did:)to the current one and left the schema on the old string. The hardening was applied to one surface of the four.docs/schema.mddescribed the prefix reading throughout and is corrected here as well, since leaving a third surface stating the loose rule would reproduce the defect one door down.Why the schema moves and not the model
did:X:abcis not a DID that this package happens to refuse. DID Core §3.1 fixes the method name in its ABNF:%x61-7Aisatoz, and the specification requires every DID to conform to the DID Syntax ABNF.[a-z0-9]+matches that exactly. The method-specific identifier is not constrained, sodid:key:z6MkhaXgBZDvotDkL5257faiztiGiC2QtKLGpbnnEGta2doKkeeps its case.Twelve shapes, checked against the tightened pattern, no mismatches:
spiffe://example.org/workloadspiffe://example.orgspiffe://ex.org/ns/prod/sa/paymentsspiffe://example.org/did:key:z6Mk...did:X:abcdid:web:example.com%3A8080:pathdid:example:did:ethr:0xAbC123urn:uuid:1234did:3:abcWhy the differential test did not catch it
tests/test_the_schema_and_the_models_agree.py, added in #236, mutates every field of a valid record across a 24 value matrix and compares the two validators. It is green onmainand it could not have seen this.None of the 24 values starts with
spiffe://ordid:. All 13 of the string values fail the old prefix test as well as the model's shape, so both validators rejected and agreed. Splitting them needs a value that passes the prefix and fails the shape, and the matrix had no such value for any field.Four are added:
Measured, on this branch:
main, shipped matrixmain, four values addedsubject,schema=True model=Falseschema=False model=TrueThe guard fires in both directions and on nothing else: adding the four values to the matrix produces no new disagreement on any of the other 27 paths, so
DECLARED_DIVERGENCESneeds no new row.This was the only pair of patterns that differed
Resolving the model's aliases (
DigestStr, andretention's_DURATION_RE) and comparing all ten schema patterns against their model counterparts:Nine of ten were already identical strings.
subjectwas the only one, which is worth stating on #247: this class of divergence has a denominator, and after this change it is empty.src/agentrust_trace/schema/trace-v0.1.jsoncarries the looser^spiffe://and is left alone. There is no v0.1 model class, so there is no second surface to diverge from.There are exactly three schema files in the repository and no fourth copy: the two v0.2 ones, both changed here, and v0.1.
One observation for #247, not carried in this PR
The mechanism for holding a docs table to the schema already exists here.
tests/test_build_provenance_depth_doc.pyparses thebuild_provenancetable out ofdocs/schema.mdand checks it against the schema, and it is applied to that one table. The top-level field table, the one that carried the stalesubjectdescription for 73 days, is held by nothing. Generalising that test to the top-level table is small and would have caught this surface before a producer did. I have left it out rather than widen a schema fix, but it is ready if it is wanted here or on #247.Verification
Four CI steps, on a fresh clone of
25013c5with this patch applied:mainis also 1015 passed, 1 skipped. The count does not move because the four values enter an existing sweep rather than adding cases; what changes is what the sweep can see.Every committed Trust Record still validates: 77 records found across the repository, 74 are records rather than schema fragments, and all 74 pass both validators. 69 of them carry a signature. The 11 distinct
subjectvalues in committed JSON all satisfy the tightened pattern, so nothing in the corpus had to change.Test-merged against every open PR whose diff overlaps this one:
CHANGELOG.mdonly, the adjacent-insert kindCHANGELOG.mdonlyCHANGELOG.mdonlyNo conflict in the schema or in any source file, including #234, which edits the same file at
runtime.platform.Type of change
Tightening a published pattern rejects records that previously validated. Nothing in this repository is affected, and the records it now rejects are ones
models.pyalready refused, so no record that round-tripped through this package is invalidated. A producer validating against the schema alone and emitting a bare trust domain is affected, which is the defect rather than a cost of the fix.Spec section
None.
spec/trace-v0.2.mddoes not state asubjectshape; the schema description andmodels.pyare the two surfaces that do, and this change makes them agree. Whether the rule should also be normative is #247's question, not this PR's.Checklist
git commit -s)CHANGELOG.mdupdated (for any normative change)<!-- CHANGED: #NNN: description -->in spec text