Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,21 @@

### Fixed

- **[SECURITY][SDK]** `_strict_schema_violations()` tolerated a missing
top-level `issuer` claim for **any** manifest version, not just v0.1. The
exception was added for legacy v0.1 records, which predate the `issuer`
field (CHANGELOG: "legacy v0.1 issuer omission remains compatible"), but
the filter never checked which version was being verified. The v0.2 spec
makes `issuer` REQUIRED, and it is not decorative:
`_signature_key_issuer_mismatch()` uses it to authorize the signing key.
As a result, a v0.2 manifest with `issuer` stripped out but otherwise
intact and carrying a valid COSE signature could pass schema validation
and reach `VALID`, silently dropping that authorization boundary for any
manifest an attacker (already in possession of a valid signing key) chose
to omit it from. The exception is now scoped to `version == "0.1"`; a
v0.2 manifest missing `issuer` fails closed with a `MISMATCH` schema
violation, same as any other missing required v0.2 claim.

- **[SDK]** `verify_delegation_chain()` no longer verifies a delegation chain
as `VALID` when a hop's `scope_grant.constraints` is non-empty. Constraints
are Cedar statements (spec 3.4.1); this package has no Cedar parser or
Expand Down
22 changes: 17 additions & 5 deletions python/src/agent_manifest/_verify.py
Original file line number Diff line number Diff line change
Expand Up @@ -484,10 +484,19 @@ def _strict_schema_violations(manifest: dict[str, Any]) -> list[tuple[str, str]]
"""Run the manifest through the Pydantic schema and return fail-closed errors.

Returns a list of (location, message) tuples for every validation error,
except legacy omissions inside individual artifact-binding objects. The
verifier historically appraises those incomplete bindings as ``NOT_BOUND``;
that compatibility does not extend to the required top-level claims that
define the manifest's identity, authority, validity, and signed contents.
except legacy omissions inside individual artifact-binding objects, and a
missing top-level ``issuer`` on a v0.1 manifest specifically. The verifier
historically appraises incomplete artifact bindings as ``NOT_BOUND``; that
compatibility does not extend to the other required top-level claims that
define the manifest's identity, authority, validity, and signed contents.

The ``issuer`` exception is scoped to v0.1 only. ``issuer`` did not exist
on v0.1 manifests, so a v0.1 record omitting it is unremarkable and stays
compatible (CHANGELOG: "legacy v0.1 issuer omission remains compatible").
The v0.2 spec makes ``issuer`` REQUIRED, and it is a meaningful security
property there - ``_signature_key_issuer_mismatch`` uses it for key
authorization - so a v0.2 manifest missing it is a genuine schema
violation and must fail closed like any other missing required claim.
"""
from pydantic import ValidationError

Expand All @@ -500,14 +509,17 @@ def _strict_schema_violations(manifest: dict[str, Any]) -> list[tuple[str, str]]
if not manifest.get("delegation_chain"):
manifest = {k: v for k, v in manifest.items() if k != "delegation_chain"}

is_legacy_v01 = manifest.get("version") == "0.1"

try:
Manifest.model_validate(manifest)
except ValidationError as exc:
violations: list[tuple[str, str]] = []
for err in exc.errors():
loc_parts = err.get("loc", ())
if err.get("type") == "missing" and (
len(loc_parts) > 1 or loc_parts == ("issuer",)
len(loc_parts) > 1
or (loc_parts == ("issuer",) and is_legacy_v01)
):
continue
loc = ".".join(str(p) for p in loc_parts)
Expand Down
50 changes: 50 additions & 0 deletions python/tests/test_cose.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ def base_manifest(**overrides):
"manifest_id": "018f4a3b-2c1d-7e5f-a8b9-0d1e2f3a4b5c",
"agent_id": "spiffe://trust.example/agent/kyc/prod",
"version": "0.2",
"issuer": "spiffe://trust.example/issuer/default",
"issued_at": NOW.isoformat().replace("+00:00", "Z"),
"expires_at": FUTURE,
"crypto_profile": "standard",
Expand Down Expand Up @@ -870,6 +871,55 @@ def test_a_bare_v02_dict_has_no_signature():
assert result.result == OverallResult.SIGNATURE_MISSING


# ---------------------------------------------------------------------------
# A signed v0.2 manifest missing the REQUIRED `issuer` claim must fail closed.
#
# `_strict_schema_violations` tolerates a missing `issuer` for legacy v0.1
# records (CHANGELOG: "legacy v0.1 issuer omission remains compatible"),
# because `issuer` did not exist on v0.1 manifests. That exception used to
# apply unconditionally, with no check on which version was being verified,
# so a v0.2 manifest - where the spec makes `issuer` REQUIRED - could omit it
# entirely, carry a perfectly valid COSE signature, and still come back
# VALID. `issuer` is not decorative: `_signature_key_issuer_mismatch` uses it
# to authorize the signing key, so treating it as optional on v0.2 quietly
# disables that authorization boundary whenever a manifest leaves it out.
# ---------------------------------------------------------------------------


def test_v02_manifest_missing_issuer_is_not_valid():
manifest = base_manifest()
del manifest["issuer"]
result = verify_manifest(sign_cose_sign1(manifest, KP), base_context(), store())
assert result.result != OverallResult.VALID
assert result.result == OverallResult.MISMATCH
assert any(d.field == "schema:issuer" for d in result.mismatch_details)


def test_v02_manifest_missing_issuer_is_not_rescued_by_key_authorization():
"""The bug is not neutralized just because trusted_key_issuers is unset.

An unconfigured `trusted_key_issuers` skips `_signature_key_issuer_mismatch`
entirely (see its early return), so that check alone never catches this.
The schema gate has to be the one that fails closed here.
"""
manifest = base_manifest()
del manifest["issuer"]
ctx = base_context(trusted_key_issuers={})
result = verify_manifest(sign_cose_sign1(manifest, KP), ctx, store())
assert result.result == OverallResult.MISMATCH


def test_v01_manifest_missing_issuer_still_verifies():
"""The legacy compatibility this exception exists for is unchanged."""
from agent_manifest._signing import Ed25519Signer

manifest = base_manifest(version="0.1")
del manifest["issuer"]
manifest["signature"] = Ed25519Signer(KP).sign(manifest)
result = verify_manifest(manifest, base_context(), store())
assert result.result == OverallResult.VALID


def test_engine_binds_attestation_to_the_payload_hash():
"""The binding is checked, and it is still not hardware evidence.

Expand Down
Loading