diff --git a/CHANGELOG.md b/CHANGELOG.md index 994826aa..45b5b79b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ Format: [Semantic Versioning](https://semver.org/). Spec versions follow `MAJOR. ### Fixed +- **`intent_bridge.verify_bridge()` now refuses malformed signed decision values instead of classifying every non-`allow` value as a policy denial.** The bridge schema permits only the literal strings `"allow"` and `"deny"`, while the runtime previously used `decision != "allow"` as its branch, so re-signed values such as `true`, `1`, `null`, `""`, and `"reject"` all surfaced as `AuthorizationDenied`. That conflated malformed producer output with a legitimate signed denial. The verifier now establishes the enum explicitly before allow/deny semantics; only the literal valid `"deny"` reaches `AuthorizationDenied`, while malformed values raise `IntentBridgeError`. The signature, trust-key, scope, digest, transcript, and wire-format rules are unchanged. + - **`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/intent_bridge.py b/src/agentrust_trace/intent_bridge.py index bbb96f6f..66c7fbfe 100644 --- a/src/agentrust_trace/intent_bridge.py +++ b/src/agentrust_trace/intent_bridge.py @@ -27,7 +27,7 @@ class IntentBridgeError(ValueError): class AuthorizationDenied(IntentBridgeError): - """The signed decision is not an authorization to execute.""" + """The signed decision is the literal valid `deny`, not authorization to execute.""" class AuthorizationMismatch(IntentBridgeError): @@ -95,6 +95,14 @@ def _nonempty_string(value: Any, field: str) -> str: return value + +def _decision(value: Any) -> str: + """Return a valid authorization decision or refuse a malformed value.""" + if not isinstance(value, str) or value not in {"allow", "deny"}: + raise IntentBridgeError('authorization.decision must be "allow" or "deny"') + return value + + def _unique_nonempty_strings(value: Any, field: str) -> list[str]: if ( not isinstance(value, list) @@ -148,6 +156,7 @@ def verify_bridge( raise IntentBridgeError(f"authorization is missing fields: {sorted(missing)}") for field in ("authorization_id", "authorizer", "authorizer_key_id"): _nonempty_string(authorization[field], f"authorization.{field}") + _decision(authorization["decision"]) # 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 diff --git a/tests/test_intent_bridge.py b/tests/test_intent_bridge.py index dae5d5bc..9193ec23 100644 --- a/tests/test_intent_bridge.py +++ b/tests/test_intent_bridge.py @@ -104,6 +104,27 @@ def test_a_malformed_base64_signature_raises_intentbridgeerror_not_valueerror( ) + +@pytest.mark.parametrize("bad_decision", [True, 1, None, "", "reject"]) +def test_malformed_signed_decision_is_not_classified_as_denial(bad_decision) -> None: + """Only literal `deny` is AuthorizationDenied. + + Malformed values are producer errors, not policy decisions. + """ + bridge, key, declaration, intent, args, tool_call, transcript = _fixture() + malformed = copy.deepcopy(bridge) + malformed["authorization"]["decision"] = bad_decision + malformed = sign_bridge(malformed["authorization"], key) + + with pytest.raises(IntentBridgeError, match="decision must be") as excinfo: + verify_bridge( + malformed, {**key_to_jwk(key), "kid": "key-7"}, declaration=declaration, + pic_intent_digest=intent, pic_args_digest=args, + tool_call=tool_call, transcript=transcript, now=150, + ) + assert not isinstance(excinfo.value, AuthorizationDenied) + + def test_deny_and_scope_fail_closed() -> None: bridge, key, declaration, intent, args, tool_call, transcript = _fixture() denied = copy.deepcopy(bridge)