Skip to content

Commit 202b7b7

Browse files
committed
fix(intent-bridge): validate signed decision enum (#319)
Rebased onto upstream 760cc3f after the CHANGELOG-only conflict identified by maintainers. Signed-off-by: Altru.dev <altrudevelop@gmail.com>
1 parent 760cc3f commit 202b7b7

3 files changed

Lines changed: 33 additions & 1 deletion

File tree

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ Format: [Semantic Versioning](https://semver.org/). Spec versions follow `MAJOR.
1313

1414
### Fixed
1515

16+
- **`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.
17+
1618
- **`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.
1719

1820
- **`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.

‎src/agentrust_trace/intent_bridge.py‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ class IntentBridgeError(ValueError):
2727

2828

2929
class AuthorizationDenied(IntentBridgeError):
30-
"""The signed decision is not an authorization to execute."""
30+
"""The signed decision is the literal valid `deny`, not authorization to execute."""
3131

3232

3333
class AuthorizationMismatch(IntentBridgeError):
@@ -95,6 +95,14 @@ def _nonempty_string(value: Any, field: str) -> str:
9595
return value
9696

9797

98+
99+
def _decision(value: Any) -> str:
100+
"""Return a valid authorization decision or refuse a malformed value."""
101+
if not isinstance(value, str) or value not in {"allow", "deny"}:
102+
raise IntentBridgeError('authorization.decision must be "allow" or "deny"')
103+
return value
104+
105+
98106
def _unique_nonempty_strings(value: Any, field: str) -> list[str]:
99107
if (
100108
not isinstance(value, list)
@@ -148,6 +156,7 @@ def verify_bridge(
148156
raise IntentBridgeError(f"authorization is missing fields: {sorted(missing)}")
149157
for field in ("authorization_id", "authorizer", "authorizer_key_id"):
150158
_nonempty_string(authorization[field], f"authorization.{field}")
159+
_decision(authorization["decision"])
151160

152161
# Hoisted out of the try below. Inside it, an authorization JCS cannot serialize
153162
# was reported as "the signature is invalid", which is a different fact and sends

‎tests/test_intent_bridge.py‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,27 @@ def test_a_malformed_base64_signature_raises_intentbridgeerror_not_valueerror(
104104
)
105105

106106

107+
108+
@pytest.mark.parametrize("bad_decision", [True, 1, None, "", "reject"])
109+
def test_malformed_signed_decision_is_not_classified_as_denial(bad_decision) -> None:
110+
"""Only literal `deny` is AuthorizationDenied.
111+
112+
Malformed values are producer errors, not policy decisions.
113+
"""
114+
bridge, key, declaration, intent, args, tool_call, transcript = _fixture()
115+
malformed = copy.deepcopy(bridge)
116+
malformed["authorization"]["decision"] = bad_decision
117+
malformed = sign_bridge(malformed["authorization"], key)
118+
119+
with pytest.raises(IntentBridgeError, match="decision must be") as excinfo:
120+
verify_bridge(
121+
malformed, {**key_to_jwk(key), "kid": "key-7"}, declaration=declaration,
122+
pic_intent_digest=intent, pic_args_digest=args,
123+
tool_call=tool_call, transcript=transcript, now=150,
124+
)
125+
assert not isinstance(excinfo.value, AuthorizationDenied)
126+
127+
107128
def test_deny_and_scope_fail_closed() -> None:
108129
bridge, key, declaration, intent, args, tool_call, transcript = _fixture()
109130
denied = copy.deepcopy(bridge)

0 commit comments

Comments
 (0)