diff --git a/CHANGELOG.md b/CHANGELOG.md index 994826aa..108ae733 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ Format: [Semantic Versioning](https://semver.org/). Spec versions follow `MAJOR. ### Fixed +- **`provenance.build_record()` now validates an explicit `issued_at` before any timestamp conversion and holds it to the JCS safe-integer range.** The producer previously called `int()` before `_check_structure()`, so malformed values were silently rewritten before the strict guard saw them: `True` became `1`, `False` became `0`, `1.9` became `1`, `"123"` became `123`, and `-0.5` became `0`. Non-convertible values such as `[1]` and `"abc"` leaked raw `TypeError`/`ValueError`, while `2**60` passed the structural guard and failed later in JCS signing. Explicit values now reach the shared validator unchanged; only the internally generated `time.time()` default is converted to integer seconds. The validator also applies the same JCS safe-integer ceiling as the repository's other signed integer surfaces. No wire-format, signature, freshness, revocation, catalog, or consumer-verification semantics change. + - **`TraceSandboxAdapter`'s documentation claimed a guarantee it does not provide: that a caller cannot claim hardware it does not have.** `SandboxAttestation` validates shape only -- `platform` against the enum on `RuntimeInfo`, `measurement` against the `sha256:`/`sha384:` digest pattern -- and has never checked a quote, a signature, or a nonce. `_runtime()` then copies `platform` and `measurement` from the attestation into the record unchanged. Nothing stops the same process that constructs a `SandboxAttestation` from inventing both values, e.g. `SandboxAttestation(platform="amd-sev-snp", measurement="sha256:" + "0" * 64)`, and `build_trust_record()` accepts it, `TrustRecord.model_validate()` accepts the result, and `sign_record()` signs it -- producing a Level 1-shaped record with no hardware evidence behind it. The module docstring's "**It will not let a caller claim hardware it does not have**" and its closing claim that "a record that says `tpm2` therefore carries a measurement that something other than this process produced" were both false as written: they described appraisal this code does not perform. This isn't a gap unique to the sandbox adapter -- `docs/trust-levels.md`'s Level 1 section already states the same boundary for the format generally ("Merely changing `runtime.platform`, copying a nonzero digest, or setting `appraisal.status="affirming"` does not establish that evidence" and "`agentrust_trace.verify_record` does not itself appraise hardware quotes") -- but `sandbox.py`'s docstring and `docs/integration/sandbox-runtime.md` asserted the opposite for this adapter specifically, which is what made it a documentation defect rather than a restatement of a known limitation. Fabricating an attestation was never a bypass of anything this adapter checks; the check that was missing had never existed and was never implemented, only claimed. Both docs are corrected to say what is actually enforced (accepted-platform and digest-shape validation) and to state plainly that verifying genuine evidence from the named platform, before constructing a `SandboxAttestation`, is the caller's responsibility -- consistent with how every other Level 1 producer in this codebase is documented. The same unconditional phrasing also remained in `build_trust_record()`'s docstring and in the integration guide's "Adding a root of trust" opening line and Levels table; corrected there too, with the guide's table gaining an explicit assurance column so record shape and verified hardware assurance are no longer collapsed together. No runtime behavior changes: `SandboxAttestation` and `TraceSandboxAdapter` accept exactly the input they always accepted. A new regression test, `test_a_fabricated_but_well_shaped_attestation_is_accepted_verbatim`, pins the actual contract so it cannot silently drift toward either a false sense of verification or an undocumented new rejection. - **`cnf.jwk` accepted an RSA confirmation key carrying no key material.** The schema says "Keys must carry actual key material" and enforced it for `OKP` and `EC` only, so a `cnf.jwk` of `{"kty": "RSA"}` with no `n` and no `e` validated, and the record then failed inside the verifier, where `sign.jwk_thumbprint` reports the missing thumbprint member. Nothing was accepted that should have been refused, since every path downstream fails closed. What was wrong is which instrument spoke: the schema is the artifact an implementation in any language validates against, and it was not the thing that told the producer the key was unusable. `RSA` now requires `n` and `e`, which states what the description already claimed and refuses nothing that verifies. A `kty` enum is deliberately not added, because section 3.2.1 states signing algorithms per envelope context and fixes no set for the embedded-signature form of section 3.2.2, so narrowing `kty` here would add a constraint the specification does not make. `models.JWK`, which is exported and is what a Python caller reaches, carried the same `OKP`/`EC`-only table and is corrected with it; `n` and `e` are declared members there too, so a non-string modulus is refused rather than stored as an untyped extra. A parametrized test now checks the schema and the model against each other on every case, since a key one takes and the other refuses fails somewhere the producer did not choose. Both copies of the schema move together, and a test asserts they are the same bytes. diff --git a/src/agentrust_trace/provenance.py b/src/agentrust_trace/provenance.py index 08fe9a32..a353f940 100644 --- a/src/agentrust_trace/provenance.py +++ b/src/agentrust_trace/provenance.py @@ -21,6 +21,7 @@ from typing import Any from agentrust_trace.sign import ( + JCS_SAFE_INTEGER, RevocationStore, _b64url_decode, _canonical_bytes, @@ -198,11 +199,19 @@ 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 JCS safe-integer limit used by every signed integer surface: + # a value outside it has no portable canonical form to sign. + 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." ) @@ -235,7 +244,10 @@ def build_record( "a record needs artifact identity, endpoint identity, or both. One with " "neither identifies nothing." ) - stamped_at = int(issued_at if issued_at is not None else time.time()) + # Only the internally generated default needs conversion from time.time(). + # Explicit caller values must reach _check_structure unchanged; coercing them + # first would turn booleans, floats and numeric strings into valid-looking ints. + stamped_at = int(time.time()) if issued_at is None else issued_at _check_structure( kind=kind, artifact=artifact, diff --git a/tests/test_provenance.py b/tests/test_provenance.py index 95f9320a..68471eeb 100644 --- a/tests/test_provenance.py +++ b/tests/test_provenance.py @@ -131,6 +131,28 @@ def test_output_schema_does_not_change_the_hash() -> None: # --- building: refuse records that cannot mean anything -------------------- + +@pytest.mark.parametrize( + "bad_issued_at", + [True, False, 1.9, "123", -0.5, [1], "abc", 2**60], +) +def test_build_record_refuses_malformed_issued_at_before_coercion(bad_issued_at) -> None: + """Explicit timestamps are validated as supplied, never normalized into another value.""" + with pytest.raises(ProvenanceError, match="issued_at"): + _record(issued_at=bad_issued_at) + + +def test_build_record_preserves_an_explicit_valid_issued_at() -> None: + assert _record(issued_at=123)["issued_at"] == 123 + + +def test_build_record_converts_only_the_internal_time_default(monkeypatch) -> None: + monkeypatch.setattr(time, "time", lambda: 123.9) + record = _record() + assert record["issued_at"] == 123 + assert isinstance(record["issued_at"], int) + + def test_identity_with_neither_artifact_nor_endpoint_is_refused() -> None: with pytest.raises(ProvenanceError, match="identifies nothing"): build_record(kind="publisher-asserted", publisher="did:web:x", tools=TOOLS)