test: sweep every keyword argument of every public function, and close what it found

src/agentrust_trace/content_marking.py

@@ -116,6 +116,11 @@ def build_assertion(
     re-serialized dict is a hash of something nobody will ever fetch: key order,
     separators and escaping all change the bytes without changing the record, and
     the verifier hashes what the server sends.
+
+    Raises ``ContentMarkingError`` for a *url* that is empty, an *alg* that is not a
+    digest algorithm name, and an *anchor* that is not a non-empty string. A non-string
+    or empty anchor used to be dropped without a word, so the caller got an assertion
+    with no anchor and no error.
     """
     if not isinstance(record_bytes, bytes | bytearray) or not record_bytes:
         raise ContentMarkingError(
@@ -125,6 +130,17 @@ def build_assertion(
     if not url:
         raise ContentMarkingError("url is required: an assertion with no reference binds nothing")
     url = _record_url(url)
+    if not isinstance(alg, str):
+        raise ContentMarkingError(
+            f"alg must be a digest algorithm name, got {type(alg).__name__}; use sha256 or sha384"
+        )
+    if anchor is not None and (not isinstance(anchor, str) or not anchor):
+        # `if anchor:` below used to drop a non-string or empty anchor on the floor, so a
+        # caller who passed one got an assertion with no anchor and no error. Refuse it.
+        raise ContentMarkingError(
+            "anchor must be a non-empty registry entry URI string or None, got "
+            f"{anchor!r}"
+        )
 
     import json
 

src/agentrust_trace/intent_bridge.py

@@ -65,7 +65,17 @@ def digest_jcs(value: dict[str, Any]) -> str:
 
 
 def sign_bridge(authorization: dict[str, Any], key: Ed25519PrivateKey) -> dict[str, Any]:
-    """Sign the complete authorization; key material is deliberately not embedded."""
+    """Sign the complete authorization; key material is deliberately not embedded.
+
+    Raises ``IntentBridgeError`` for a *key* that is not an ``Ed25519PrivateKey``: the
+    bridge profile fixes the algorithm, and the package's two other signers hold their
+    key to the same type through ``key_to_jwk``.
+    """
+    if not isinstance(key, Ed25519PrivateKey):
+        raise IntentBridgeError(
+            f"key must be an Ed25519PrivateKey, got {type(key).__name__}. The bridge "
+            "profile fixes the algorithm, so there is no other key this can sign with."
+        )
     artifact = {"profile": BRIDGE_PROFILE, "authorization": authorization}
     signature = base64.urlsafe_b64encode(key.sign(_jcs(artifact, "the authorization"))).rstrip(b"=")
     return {**artifact, "signature": signature.decode("ascii")}

src/agentrust_trace/provenance.py

@@ -20,7 +20,10 @@
 import time
 from typing import Any
 
+import rfc8785
+
 from agentrust_trace.sign import (
+    JCS_SAFE_INTEGER,
     RevocationStore,
     _b64url_decode,
     _canonical_bytes,
@@ -188,6 +191,12 @@ def _check_structure(
                 "endpoint.spki_sha256 must be a sha256: digest of the Subject Public Key "
                 "Info. A URL on its own is not an identity."
             )
+    if attestation is not None and not isinstance(attestation, dict):
+        raise ProvenanceError(
+            f"attestation must be an object or null, got {type(attestation).__name__}. "
+            "spec/server-provenance-v1.md: the evidence in the shape TRACE v0.2 section 3.1 "
+            "runtime uses, or null."
+        )
     if kind == "tee-attested" and not attestation:
         raise ProvenanceError(
             "kind='tee-attested' without attestation evidence is the claim without the "
@@ -198,11 +207,21 @@ def _check_structure(
             f"kind={kind!r} carries attestation evidence. Evidence that is present but "
             "not claimed invites a consumer to read it as an attestation that was made."
         )
-    # bool is an int subclass, and True would otherwise pass as a timestamp.
-    if not isinstance(issued_at, int) or isinstance(issued_at, bool) or issued_at < 0:
+    # bool is an int subclass, and True would otherwise pass as a timestamp. The upper
+    # bound is the same JCS safe-integer limit #219 applies to every other signed integer
+    # in this package: above it there is no portable canonical form, so the producer would
+    # accept a timestamp it cannot sign and the caller would meet `rfc8785`'s
+    # `IntegerDomainError` instead of the class this module documents.
+    if (
+        not isinstance(issued_at, int)
+        or isinstance(issued_at, bool)
+        or issued_at < 0
+        or issued_at > JCS_SAFE_INTEGER
+    ):
         raise ProvenanceError(
-            "issued_at must be a non-negative integer Unix timestamp. A record with no "
-            "issue time cannot be aged, so a consumer has no way to reject a stale one."
+            "issued_at must be a non-negative integer Unix timestamp within the JCS "
+            "safe-integer range. A record with no usable issue time cannot be aged, so "
+            "a consumer has no way to reject a stale one."
         )
 
 
@@ -225,7 +244,7 @@ def build_record(
     """
     if kind not in KINDS:
         raise ProvenanceError(f"kind {kind!r} is not one of {', '.join(KINDS)}")
-    if not _PUBLISHER_RE.match(publisher or ""):
+    if not isinstance(publisher, str) or not _PUBLISHER_RE.match(publisher):
         raise ProvenanceError(
             f"publisher {publisher!r} must be a DID or SPIFFE URI. A display name is not "
             "resolvable and a verifier cannot check one."
@@ -272,13 +291,29 @@ def sign_record(record: dict[str, Any], key: Any) -> dict[str, Any]:
     Raises ``ProvenanceError`` for a *record* that is not a JSON object. ``{**record}``
     reads it before its shape is established, so a non-mapping raised a bare
     ``TypeError`` about dict unpacking, which is not this module's documented refusal.
+    Also raises ``ProvenanceError`` for a *key* that is not an Ed25519 private key, and
+    for a record with no RFC 8785 canonical form, such as an integer outside the JCS
+    safe range; both used to escape as the underlying library's ``ValueError``.
     """
     if not isinstance(record, dict):
         raise ProvenanceError(
             f"record must be a JSON object, got {type(record).__name__}"
         )
-    payload = {**record, "cnf": {"jwk": key_to_jwk(key)}}
-    body = _canonical_bytes({k: v for k, v in payload.items() if k != "signature"})
+    try:
+        jwk = key_to_jwk(key)
+    except ValueError as exc:
+        raise ProvenanceError(f"key must be an Ed25519 private key: {exc}") from exc
+    payload = {**record, "cnf": {"jwk": jwk}}
+    try:
+        body = _canonical_bytes({k: v for k, v in payload.items() if k != "signature"})
+    except rfc8785.CanonicalizationError as exc:
+        # `_canonical_bytes` is `rfc8785.dumps` and raises its own errors for a value JCS
+        # has no form for, including an integer outside the safe domain. Those are
+        # `ValueError`s, not this module's, so a caller catching `ProvenanceError` saw a
+        # crash. Same shape as the wrap `intent_bridge._jcs` already carries.
+        raise ProvenanceError(
+            f"record has no RFC 8785 canonical form, so it cannot be signed: {exc}"
+        ) from exc
     import base64
 
     sig = base64.urlsafe_b64encode(key.sign(body)).rstrip(b"=").decode()
@@ -410,7 +445,19 @@ def verify_record(
         )
 
     pub = _pubkey_from_jwk(trusted_jwk)
-    body = _canonical_bytes({k: v for k, v in record.items() if k != "signature"})
+    try:
+        body = _canonical_bytes({k: v for k, v in record.items() if k != "signature"})
+    except rfc8785.CanonicalizationError as exc:
+        # The other half of the wrap `sign_record` carries. The record here is the
+        # untrusted document, so it can hold a value JCS has no form for wherever the
+        # structural checks above do not type the field: an integer outside the safe
+        # range under `tools`, say. `rfc8785`'s errors are its own `ValueError`s, not
+        # this module's, so a caller written against `ProvenanceError` saw a crash
+        # where every other malformed record gives a refusal.
+        raise ProvenanceError(
+            f"record has no RFC 8785 canonical form, so its signature cannot be "
+            f"checked: {exc}"
+        ) from exc
     # `signature` reaches this function from whatever the caller is verifying, the
     # same untrusted document nothing above this line has vouched for either: a
     # non-string here (an int, a list of chars, a nested object) previously hit

src/agentrust_trace/revocation.py

@@ -181,6 +181,30 @@ def _trusted_bundle_key(
     return None
 
 
+def _sequence_of(value: Any, name: str, element: type) -> list[Any]:
+    """Materialise a caller-supplied iterable, refusing the shapes that iterate wrongly.
+
+    A ``str`` iterates as characters, so ``trusted_key_identifiers="sha256:..."`` used to
+    become a list of one-character identifiers that matched no statement, and the check
+    reported ``verified`` for a key it never looked up. A ``dict`` iterates as its keys,
+    so a single JWK passed where a list of them was meant became a list of field names.
+    Neither is an iterable of *element* and both are refused here, with the documented
+    error, rather than turned into a result.
+    """
+    if isinstance(value, (str, bytes, bytearray, dict)) or not hasattr(value, "__iter__"):
+        raise ValueError(
+            f"{name} must be an iterable of {element.__name__} values, got "
+            f"{type(value).__name__}"
+        )
+    items = list(value)
+    bad = [type(v).__name__ for v in items if not isinstance(v, element)]
+    if bad:
+        raise ValueError(
+            f"{name} must contain only {element.__name__} values, found {sorted(set(bad))}"
+        )
+    return items
+
+
 def check_bundle(
     bundle: dict[str, Any],
     *,
@@ -204,13 +228,17 @@ def check_bundle(
 
     Raises ``ValueError`` when a statement on the bundle's log names the trusted
     key. That is evidence failing rather than evidence absent, and it fails closed
-    like the ``revocation`` store does.
+    like the ``revocation`` store does. Also raises ``ValueError`` for the caller's
+    own arguments when they are not what they say: ``trusted_key_identifiers`` must
+    be an iterable of strings and ``trusted_bundle_keys`` an iterable of JWK
+    objects, and a bare string or a single object is refused rather than iterated
+    as characters or field names.
     """
     _check_seconds("now", now)
     _check_seconds("max_bundle_age_seconds", max_bundle_age_seconds)
     _check_seconds("max_future_skew_seconds", max_future_skew_seconds)
-    trusted_ids = list(trusted_key_identifiers)
-    trusted_bundle_keys = list(trusted_bundle_keys)
+    trusted_ids = _sequence_of(trusted_key_identifiers, "trusted_key_identifiers", str)
+    trusted_bundle_keys = _sequence_of(trusted_bundle_keys, "trusted_bundle_keys", dict)
 
     # 3a. Shape, against the packaged schema pair. The first error by path, so the
     # evidence points at one place rather than listing the file.

src/agentrust_trace/sign.py

@@ -509,7 +509,11 @@ def verify_record(
         signed by a key not in ``trusted_bundle_keys``, signed with an algorithm
         this build cannot verify, dated in the future, or expired under either
         bound yields ``unverified_for_revocation`` with the cause named; it does
-        not raise, because inability to check is not evidence of a defect. A
+        not raise, because inability to check is not evidence of a defect. The
+        argument itself is held to its shape: ``trusted_bundle_keys`` must be an
+        iterable of JWK objects or ``None``, and a string, a single object or
+        another non-iterable raises ``ValueError`` rather than being read as no
+        keys. A
         statement on the bundle's log naming the trusted key raises ``ValueError``:
         no inclusion entry ID reaches this function, so 3.2.3's fallback applies
         and every record the key signed is rejected.
@@ -652,7 +656,7 @@ def verify_record(
         revocation_check = check_bundle(
             revocation_bundle,
             trusted_key_identifiers=_key_identifiers(trusted_jwk),
-            trusted_bundle_keys=trusted_bundle_keys or (),
+            trusted_bundle_keys=() if trusted_bundle_keys is None else trusted_bundle_keys,
             now=verification_time,
             max_bundle_age_seconds=max_bundle_age_seconds,
             max_future_skew_seconds=max_future_skew_seconds,

tests/test_provenance.py

@@ -12,6 +12,7 @@
 import time
 
 import pytest
+import rfc8785
 
 from agentrust_trace.provenance import (
     FORMAT,
@@ -23,7 +24,13 @@
     tool_catalog_hash,
     verify_record,
 )
-from agentrust_trace.sign import _canonical_bytes, generate_key, jwk_thumbprint, key_to_jwk
+from agentrust_trace.sign import (
+    JCS_SAFE_INTEGER,
+    _canonical_bytes,
+    generate_key,
+    jwk_thumbprint,
+    key_to_jwk,
+)
 
 DIGEST = "sha256:" + "a" * 64
 OTHER_DIGEST = "sha256:" + "b" * 64
@@ -797,6 +804,52 @@ def test_an_unconvertible_issued_at_raises_what_the_module_documents(
         _record(issued_at=supplied)
 
 
+# The other half of #320, which #334 did not carry: the guard had no upper bound, so a
+# producer accepted a timestamp it could not sign. The value is a well-formed non-negative
+# integer, so none of the tests above reaches it; `int(2**60)` is `2**60`.
+@pytest.mark.parametrize(
+    ("supplied", "accepted"),
+    [
+        (JCS_SAFE_INTEGER - 1, True),
+        (JCS_SAFE_INTEGER, True),
+        (JCS_SAFE_INTEGER + 1, False),
+        (2**60, False),
+    ],
+    ids=["below", "at", "above", "far-above"],
+)
+def test_the_issued_at_bound_sits_where_the_canonicalizer_stops(
+    supplied: int, accepted: bool
+) -> None:
+    """The boundary is asserted against `rfc8785` rather than against a number written
+    twice: the guard and the canonicalizer have to agree about which integers exist, or
+    `build_record` emits records `sign_record` refuses."""
+    if accepted:
+        assert _record(issued_at=supplied)["issued_at"] == supplied
+        rfc8785.dumps({"issued_at": supplied})
+        return
+    with pytest.raises(ProvenanceError, match="issued_at"):
+        _record(issued_at=supplied)
+    with pytest.raises(rfc8785.IntegerDomainError):
+        rfc8785.dumps({"issued_at": supplied})
+
+
+def test_an_out_of_range_issued_at_no_longer_leaves_the_verifier_as_rfc8785s_error() -> None:
+    """`_check_structure` is shared, so the bound reaches `verify_record` as well.
+
+    Under the default freshness policy such a record was already refused, as dated in the
+    future, so nothing that verified before is refused now. The case that changes is a
+    caller who widens `max_future_skew_seconds` past the gap: the structural check ran
+    with the value in hand, the canonicalizer met it first, and `rfc8785`'s
+    `IntegerDomainError` left a function documented to raise `ProvenanceError`.
+    """
+    key = generate_key()
+    record = _record()
+    record["issued_at"] = 2**60
+    signed = dict(record, signature="AA", cnf={"jwk": key_to_jwk(key)})
+    with pytest.raises(ProvenanceError, match="issued_at"):
+        verify_record(signed, key_to_jwk(key), max_future_skew_seconds=10**19)
+
+
 def test_a_valid_issued_at_is_still_carried_through_unchanged() -> None:
     assert _record(issued_at=1_754_000_000)["issued_at"] == 1_754_000_000