fix: nine defects in the reference implementation, found by a package wide sweep

.gitignore

@@ -18,3 +18,22 @@ htmlcov/
 .venv/
 venv/
 uv.lock
+
+# What the documentation tells a reader to write into their working tree.
+#
+# docs/quickstart.md writes an unencrypted Ed25519 private key to trace-key.pem, under
+# a comment reading "keep secure, never commit or log". The comment was the whole of the
+# enforcement: a reader following the quickstart inside a clone was one `git add -A`
+# away from committing their signing key. tests/test_the_docs_do_not_leave_a_key_behind.py
+# recovers these paths from the documentation, so a doc that starts writing somewhere
+# new fails rather than quietly widening the gap.
+session.trace.json
+trace-key.pem
+trace-key.pem.pub
+
+# No key material of any kind, whatever it is called. The repository tracks none today
+# and there is no case for the first one arriving unnoticed.
+*.pem
+*.key
+*.p8
+*.pfx

CHANGELOG.md

@@ -13,6 +13,28 @@ Format: [Semantic Versioning](https://semver.org/). Spec versions follow `MAJOR.
 
 ### Fixed
 
+- **The exported `SCHEMA` was the live object the validator reads.** `_schema()` is `lru_cache`d and `_validator()` is built over whatever it returns, so the name exposed "for downstream tooling that needs the raw dict" and the validator's schema were one object: lowering `SCHEMA["properties"]["iat"]["minimum"]` made a record dated 1970 valid to `validate_json()`, to `iter_errors()`, and to the structural gate inside `sign.verify_record()`, for every later call in the process. Nothing about the call site looks wrong, since adapting the raw dict is the use the comment invites. It is a deep copy now; a shallow one would leave the nested `properties` dicts shared and the same edit would still land.
+
+- **All eight `"format": "uri"` declarations in the schema were inert, and `"not a uri"` validated.** `jsonschema` treats `format` as an annotation rather than an assertion unless a checker for that format is installed, and `FormatChecker().checkers` does not carry `uri` without an optional dependency the project did not declare. The wiring in `validate.py` was correct and the behaviour was a no-op, which is the shape that survives review. `rfc3986-validator` is now a dependency, chosen over `jsonschema[format]` because that pulls `rfc3987`, which is GPLv3, into an Apache-2.0 package's dependency tree. Turning the constraint on changes nothing about the existing corpus: the suite passed unchanged before any new test was added.
+
+- **`TrustRecord.model_validate(record).model_dump()` returned a record this package's own validator rejects and whose signature no longer verifies.** Pydantic serializes every unset optional as an explicit `null`, the schema types no named field as nullable, and the added members change the RFC 8785 canonical bytes the signature is taken over. This is the round trip `sign_record()`'s own docstring points a caller at, so a caller who validated the model and then wrote it out wrote a broken record, with neither check running at the moment the damage is done. Absent optionals are omitted now and the round trip is exact identity. Only declared fields are dropped: `JWK` sets `extra="allow"` and the schema permits a null among those members, so a null inside `cnf.jwk` is data.
+
+- **The models accepted booleans where JSON says integer, and read them as numbers.** `isinstance(True, int)` is a Python fact and not a JSON one: JSON Schema's `"type": "integer"` does not match `true`, so `schema/trace-claim.json` rejects `{"slsa_level": true}` and `models.BuildProvenance` accepted it and coerced it to `1`, making the record a claim of SLSA build level 1 assembled out of a boolean. `tool_transcript.call_count` did the same, and `appraisal.timestamp` read `true` as 1 January 1970. `iat` and `origin.ingested_at` did not have the hole and were safe by accident rather than by design, their lower bound sitting above 1 so the coerced value failed the range check afterwards; all five carry an explicit guard now. Found by mutating every field of a valid record and comparing the two validators, which had never been compared. The differential is committed, and the disagreements it does not fix are declared with the reason.
+
+- **Six public functions raised exceptions no module documents, and the sweep that finds them is committed.** `key_to_jwk()` and `load_key()` read `.public_key()` and `.encode()` off their argument before establishing its type; `key_to_jwk` now names the public-key case separately, because that is the plausible mistake for a function whose name reads as "turn a key into a JWK" and whose result is the public JWK. `sign_record()` in both modules unpacked `{**record}` before checking it was a mapping. `anchor_bytes()` refused the two value classes registry-anchor-v1 section 1 excludes and handed everything else to `json.dumps`, so a type JSON cannot serialize came back as a message about a serializer. `intent_bridge` let `rfc8785` errors out as themselves: those are `ValueError` subclasses, which satisfies `sign`'s contract but not this module's, since a `CanonicalizationError` is not an `IntentBridgeError`. `verify_bridge()` did not leak but misattributed, canonicalizing inside the `try` that reports the signature invalid. The sweep walks the package rather than listing functions, and a coverage test fails until every discovered function is either swept or declared unsweepable.
+
+- **`docs/quickstart.md` writes an unencrypted private key into the reader's working tree and `.gitignore` did not cover it.** The block writes `trace-key.pem` under the comment "keep secure, never commit or log", and that comment was the whole of the enforcement: a reader following the quickstart inside a clone was one `git add -A` away from committing their signing key. The three paths the documentation writes are ignored now, along with `*.pem`, `*.key`, `*.p8` and `*.pfx`, since the repository tracks no key material today. The accompanying test recovers the written paths from the documentation rather than listing them, so a doc that starts writing somewhere new fails rather than widening the gap quietly.
+
+- **The revocation check no longer fails open when the store answers with a non-bool.** `RevocationStore` is `Container[str] | Callable[[str], bool]`, and the callable's return value was read by truthiness, so `None`, `""`, `0` and `[]` all read as "not revoked" and let the key through, while the string `"no"` read as revoked. `None` is the case that matters: it is what a CRL, status or SCITT lookup returns when its author handled the 200 and forgot every other response, which is exactly the outage the existing `except` clause was written to survive. That clause already treats a store that raises as a rejection, on the stated grounds that an unavailable source is not evidence a key is unrevoked; a store answering `None` supplied no more evidence and was being believed. `provenance.verify_record()` imports the same function and makes the same claim in its own docstring, so one fix closes both entry points. The membership branch is untouched, because `in` yields a real bool whatever `__contains__` returns.
+
+- **`provenance.verify_record()`, `provenance.check_tool_catalog()` and `content_marking.verify_assertion()` now hold their externally supplied argument to the type they document.** Each read that argument before establishing its shape. Measured across a twelve-value junk matrix, the two provenance functions leaked eleven `AttributeError`s apiece; the twelfth value is `{}`, which is an object and so reached the refusal each function documents, which is not the `ProvenanceError` `verify_record` documents. `content_marking.verify_assertion()` was worse than a crash rather than merely undocumented: it did not check that `record_bytes` were bytes, and `bytes(5)` is five zero bytes, so an int was hashed, failed to match, and the caller was told the record at the URL had changed, which is a specific and false accusation about somebody else's server. All three now raise the error their module documents, naming the type received.
+
+### Fixed
+
+- **`jwk_thumbprint()` and `verify_record()` now refuse a non-object argument with the error they document.** Both read a member off the argument before establishing its shape, so a string, a number, `None`, a list or a bool raised `AttributeError`, which is not the `ValueError` `verify_record`'s docstring names for every rejection other than a bad signature, and is not caught by a caller written against that contract. Neither argument is one the caller has already established: a JWK reaches `jwk_thumbprint` from a peer, a key document or a record's own `cnf`, and the record handed to `verify_record` is by definition not yet known to be an object. Both now raise `ValueError` naming the type received. 21 tests: removing the two guards fails 20 of them, and the twenty-first is the control that has to keep passing.
+
+### Fixed
+
 - **`provenance.tool_catalog_hash()` now refuses a malformed `tools` list instead of crashing.** Reached primarily through `check_tool_catalog(record, tools)`, the function the module's own docstring calls "the step that catches a live attack," because `tools` there is what the MCP server actually returned, i.e. the untrusted party this check exists to catch. The function iterated `tools` and called `.get(...)` on each entry with no check that `tools` was a list or that its entries were objects, so a malformed response (an entry that is a string, `None`, a number, or `tools` itself not being a list) raised an unhandled `AttributeError` or `TypeError` instead of the documented `ProvenanceError`. A server that is misbehaving maliciously or just buggily and is exactly the source `tools` has no reason to trust its shape. Fixed with an explicit `isinstance` check on `tools` and on each of its entries, naming the offending index.
 
 - **`content_marking.build_assertion()` and `content_marking.verify_assertion()` now refuse record bytes that aren't a JSON object, instead of crashing.** Both functions called `.get(...)` on the result of `json.loads(record_bytes)` without checking it was a dict first. Valid JSON is not always an object that is an array, a string, a number, `null`, and a bool are all valid top-level JSON  and `record_bytes` is exactly the kind of externally-sourced input this is likely to happen to: `build_assertion()` takes whatever bytes a caller hands it, and `verify_assertion()`'s docstring is explicit that its `record_bytes` are "the record bytes actually retrieved from its URL," i.e. a network response the caller does not control. Either function raised an unhandled `AttributeError` instead of the documented `ContentMarkingError`. `verify_assertion()` had a second, related gap: its second parse of `record_bytes` (after the hash check) was not wrapped in the try/except its first parse-adjacent check uses, so genuinely malformed (non-JSON) bytes that happened to hash-match raised `json.JSONDecodeError` instead of `ContentMarkingError` too. Both functions now check `isinstance(record, dict)` after parsing, and `verify_assertion()`'s second parse now catches `ValueError` the same way its hash-computation path already implicitly required valid bytes to reach.

pyproject.toml

@@ -24,6 +24,13 @@ classifiers = [
 dependencies = [
     "pydantic>=2.0",
     "jsonschema>=4.20",
+    # `jsonschema` treats `format` as an annotation unless a checker for that format is
+    # installed. Without this, all eight `"format": "uri"` declarations in
+    # schema/trace-claim.json are inert: `validate_json` builds a `FormatChecker`, the
+    # code reads as though URIs are checked, and "not a uri" validates. The non-GPL
+    # validator is used deliberately: `jsonschema[format]` pulls `rfc3987`, which is
+    # GPLv3, into an Apache-2.0 package's dependency tree.
+    "rfc3986-validator>=0.1.1",
     "cryptography>=42.0",
     "rfc8785>=0.1.2",
 ]

src/agentrust_trace/content_marking.py

@@ -150,6 +150,16 @@ def verify_assertion(assertion: dict[str, Any], record_bytes: bytes) -> dict[str
     if not _DIGEST_RE.match(str(expected or "")):
         raise ContentMarkingError(f"record.hash {expected!r} is not a sha256:/sha384: digest")
 
+    if not isinstance(record_bytes, bytes | bytearray) or not record_bytes:
+        raise ContentMarkingError(
+            f"record_bytes must be the bytes retrieved from {ref['url']}, got "
+            f"{type(record_bytes).__name__}. `build_assertion` already refuses this and "
+            "the reason it matters more here is that `bytes(5)` is five zero bytes: an "
+            "int would be hashed, would not match, and the caller would be told the "
+            "record at the URL had changed, which is a specific accusation about "
+            "somebody else's server and would be false."
+        )
+
     actual = _digest(bytes(record_bytes), alg)
     if actual != expected:
         raise RecordMismatch(

src/agentrust_trace/intent_bridge.py

@@ -8,6 +8,7 @@
 from hmac import compare_digest
 from typing import Any
 
+import rfc8785
 from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PrivateKey
 
 from agentrust_trace.sign import _b64url_decode, _canonical_bytes, _pubkey_from_jwk
@@ -33,17 +34,40 @@ class AuthorizationMismatch(IntentBridgeError):
     """Execution evidence does not match the signed authorization."""
 
 
+def _jcs(value: dict[str, Any], what: str) -> bytes:
+    """Canonical bytes for *value*, as ``IntentBridgeError`` when there are none.
+
+    ``_canonical_bytes`` is ``rfc8785.dumps`` and raises by design: a value JCS has
+    no form for has no canonical bytes to return. Its errors are ``ValueError``
+    subclasses, which satisfies ``sign``'s documented contract but not this
+    module's: ``rfc8785.CanonicalizationError`` is not an ``IntentBridgeError``, so
+    a caller written against this module's own exception does not catch it.
+
+    Four of the five values that trip it are ordinary JSON that ``json.loads``
+    accepts, an integer outside the JCS safe range, a non-finite float, and a lone
+    surrogate among them, so an authorization assembled from a parsed document
+    reaches this.
+    """
+    try:
+        return _canonical_bytes(value)
+    except rfc8785.CanonicalizationError as exc:
+        raise IntentBridgeError(
+            f"{what} has no RFC 8785 canonical form, so it cannot be digested or "
+            f"signed: {exc}"
+        ) from exc
+
+
 def digest_jcs(value: dict[str, Any]) -> str:
     """Return the SHA-256 digest of an RFC 8785 canonical JSON object."""
     if not isinstance(value, dict):
         raise IntentBridgeError("a digest input must be a JSON object")
-    return f"sha256:{hashlib.sha256(_canonical_bytes(value)).hexdigest()}"
+    return f"sha256:{hashlib.sha256(_jcs(value, 'the digest input')).hexdigest()}"
 
 
 def sign_bridge(authorization: dict[str, Any], key: Ed25519PrivateKey) -> dict[str, Any]:
     """Sign the complete authorization; key material is deliberately not embedded."""
     artifact = {"profile": BRIDGE_PROFILE, "authorization": authorization}
-    signature = base64.urlsafe_b64encode(key.sign(_canonical_bytes(artifact))).rstrip(b"=")
+    signature = base64.urlsafe_b64encode(key.sign(_jcs(artifact, "the authorization"))).rstrip(b"=")
     return {**artifact, "signature": signature.decode("ascii")}
 
 
@@ -114,10 +138,14 @@ def verify_bridge(
     for field in ("authorization_id", "authorizer", "authorizer_key_id"):
         _nonempty_string(authorization[field], f"authorization.{field}")
 
+    # Hoisted out of the try below. Inside it, an authorization JCS cannot serialize
+    # was reported as "the signature is invalid", which is a different fact and sends
+    # the reader to look at a key. There are no bytes for a signature to be checked
+    # against, so nothing has been learned about the signature at all.
+    body = _jcs({"profile": BRIDGE_PROFILE, "authorization": authorization},
+                "the authorization")
     try:
-        _pubkey_from_jwk(trusted_authorizer_jwk).verify(
-            signature, _canonical_bytes({"profile": BRIDGE_PROFILE, "authorization": authorization})
-        )
+        _pubkey_from_jwk(trusted_authorizer_jwk).verify(signature, body)
     except Exception as exc:
         raise IntentBridgeError("authorization signature is invalid") from exc
     trusted_kid = trusted_authorizer_jwk.get("kid")