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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ Format: [Semantic Versioning](https://semver.org/). Spec versions follow `MAJOR.

### Fixed

- **`TraceAGTAdapter.build_trust_record()` stamped `appraisal.status` as `affirming` on every record it produced, with no way to change it.** `appraisal.status` is a verifier-owned field: section 3.3.1 says a verifier MUST record the depth it actually checked and MUST set the status to `contraindicated` when evidence fails, and `models.Appraisal` carries the same point in a comment, "What this verifier ran, not what the issuer claimed." The adapter set it at record-construction time, before signing and before any verifier existed, and `__init__` had no parameter to override it. The result signed and verified, so a consumer reading the field to find out whether anybody checked was told yes by a record nobody had appraised. `TraceSandboxAdapter` already had this right, with `appraisal_status` defaulting to `"none"` and tests pinning both the default and the override; `TraceAGTAdapter` now matches it, and the defaulting-to-`affirming` line was the only remaining hardcoded status in the package. **This changes the content of records this adapter emits**: an unappraised record now says `none` where it used to say `affirming`, which is the correct direction and is what a caller who really did appraise must now declare with `appraisal_status="affirming"`. Two documents described the old behaviour and are corrected with it: `docs/integration/agt.md` said the adapter "populates an `affirming` appraisal without independently evaluating the session", and `docs/tutorials/agt-adapter.md` carried a post-hoc `record["appraisal"]["status"] = "none"` line, which is to say the gap was known well enough to be worked around in a tutorial rather than fixed in the adapter. That line is gone because the default now does it. Reported by @Yatsuiii in #331, who found it by comparing the two adapters.

- **`content_marking.verify_assertion()` established that the duplicated binding fields agreed, not that they existed.** `record.get("subject") != data.get("subject")` and the same line for `eat_profile` compare two reads, and two absences compare equal. A peer-produced assertion omitting `data.subject`, paired with a hash-matching record that also omitted `subject`, agreed by mutual absence and `verify_assertion` returned the parsed record as a successful binding. `spec/content-marking-v1.md` section 2 marks both fields required and section 6 says a conforming consumer checks both against the fetched record, so this layer has to establish its own required shape: the function performs only the binding check and returns before any Trust Record signature or schema verification, and a caller is allowed to run it on its own. Presence is now checked for each field on both sides. An assertion missing one is `ContentMarkingError`, because a malformed assertion is the caller's own input and `RecordMismatch` would point the reader at the server serving the URL, the same reasoning `test_an_int_no_longer_reports_a_record_mismatch` already pins. A record missing one is `RecordMismatch`, because it matched the declared hash and that URL really is serving something that is not a conformant record. Two present values that disagree are unchanged. The only behaviour change for input that was already refused is the class on an assertion-side omission, from `RecordMismatch` to its `ContentMarkingError` parent, which no caller catching the documented contract loses. Regression coverage carries all six cases from the reproduction, including the two single-side controls that make the hole precisely mutual absence rather than something wider, and a complete-pair control. Reported by @altrudev in #326, reproduced independently by @lywinged with the six-case matrix and the check against #325's head.

- **`provenance.build_record()` coerced an explicitly supplied `issued_at` before the validator that exists to inspect it ever ran.** `stamped_at = int(issued_at if issued_at is not None else time.time())` handed the converted value to `_check_structure()`, whose guard carries the comment "bool is an int subclass, and True would otherwise pass as a timestamp". The order defeated that guard: `True` arrived as `1`, `False` as `0`, `1.9` as `1`, `"123"` as `123`, and `-0.5` as `0`, so every one of them satisfied the non-negative-integer test and was written into the record. The last case is the diagnostic one, since a negative non-integer became an accepted non-negative timestamp. The same line leaked two exception classes the module does not document: `issued_at=[1]` left `build_record` as a `TypeError` and `issued_at="abc"` as a `ValueError`, where every other public function in the module is held to `ProvenanceError`. An explicitly supplied value now reaches `_check_structure()` untouched and only an omitted one is stamped with `int(time.time())`, which puts `isinstance` in front of the conversion and closes both at once. This is distinct from #142 and #146: those moved the structural rules into the shared helper, and the helper was always strict. The caller path defeated it by normalizing first. Nothing that used to produce a valid record stops doing so; a valid integer is carried through unchanged and an omitted value is still stamped. `tests/test_public_functions_raise_what_they_document.py` listed `provenance.build_record` under `NO_ARGUMENT_TO_SWEEP` because it has no positional argument, which is why the junk matrix never reached it, so the function is now wired into that sweep with `issued_at` as the varied argument and a witness that pins the sweep actually arrives. Reported by @altrudev in #320, with the two undocumented exception classes and the reason the sweep never saw them found by @lywinged.
Expand Down
2 changes: 1 addition & 1 deletion docs/integration/agt.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ A record under the superseded v0.1 EAT profile is not accepted by the current v0
| UTF-8 chain-tip string | SHA-256 software commitment in `runtime.measurement` |
| Deployment metadata | Model, classification, and build-provenance declarations |

The adapter uses RFC 8785 for transcript canonicalization. It currently accepts a transparency string but does not submit to a registry, and populates an `affirming` appraisal without independently evaluating the session. A producer must accurately set those fields before signing; the tutorial shows how to avoid claiming an appraisal or anchor for synthetic input.
The adapter uses RFC 8785 for transcript canonicalization. It currently accepts a transparency string but does not submit to a registry. `appraisal.status` defaults to `none`, because the adapter does not evaluate the session and `appraisal.status` is the verifier's field (spec section 3.3.1); pass `appraisal_status` only when an appraisal actually happened. A producer must accurately set those fields before signing; the tutorial shows how to avoid claiming an anchor for synthetic input.

## Assurance

Expand Down
6 changes: 3 additions & 3 deletions docs/tutorials/agt-adapter.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ adapter = TraceAGTAdapter(
record = adapter.build_trust_record(session)
# The adapter currently requires a URI argument but performs no registration.
record.pop("transparency")
# Synthetic input has not been appraised; do not keep the adapter's default verdict.
record["appraisal"]["status"] = "none"
# `appraisal.status` defaults to "none", which is correct here: synthetic input has not
# been appraised. Pass appraisal_status only when an appraisal actually happened.
key = generate_key()
trusted_key = key.public_key()
signed = sign_record(record, key)
Expand All @@ -46,7 +46,7 @@ The nonzero build digest is illustrative metadata, not verified build provenance

Supply the exact policy bytes used for the session, audit entries as plain dictionaries, the session's chain tip, and its authenticated identity. The adapter hashes the audit list with RFC 8785 and the chain-tip string as UTF-8. Its default call count is the list length; supply `call_count` only when your producing profile defines a different count.

The adapter records the configured enforcement mode; it does not enforce that mode or prove the policy was evaluated. Likewise, its default `affirming` appraisal is not an independent assessment. Set the record's claims to the checks actually performed before signing.
The adapter records the configured enforcement mode; it does not enforce that mode or prove the policy was evaluated. `appraisal.status` defaults to `none` for the same reason: building a record does not appraise it, and the field is the verifier's (spec section 3.3.1). Set the record's claims to the checks actually performed before signing.

## Verify and extend

Expand Down
14 changes: 12 additions & 2 deletions src/agentrust_trace/adapters/agt.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import hashlib
import time
from dataclasses import dataclass, field
from typing import Any
from typing import Any, Literal

import rfc8785
from agentrust_trace.models import (
Expand Down Expand Up @@ -95,10 +95,19 @@ def __init__(
build_provenance_builder: str | None = None,
build_provenance_uri: str | None = None,
transparency: str,
appraisal_status: Literal["affirming", "warning", "contraindicated", "none"] = "none",
appraisal_verifier: str = "https://agentrust-io.com/verify",
appraisal_policy_ref: str | None = None,
enforcement_mode: str = "enforce",
) -> None:
"""
Args:
appraisal_status: Defaults to ``"none"``. ``appraisal.status`` is a
verifier-owned field (spec section 3.3.1): a record is not appraised by
being built, and stamping ``affirming`` on an unappraised record puts a
verdict in the field a consumer reads to find out whether anybody
checked. Set this only when an appraisal actually happened.
"""
self._model = ModelInfo(
provider=model_provider,
model_id=model_id,
Expand All @@ -112,6 +121,7 @@ def __init__(
provenance_uri=build_provenance_uri,
)
self._transparency = transparency
self._appraisal_status = appraisal_status
self._appraisal_verifier = appraisal_verifier
self._appraisal_policy_ref = appraisal_policy_ref
self._enforcement_mode = enforcement_mode
Expand Down Expand Up @@ -160,7 +170,7 @@ def build_trust_record(self, session: AGTSessionResult) -> dict[str, Any]:
).model_dump(exclude_none=True),
"build_provenance": self._build_provenance.model_dump(exclude_none=True),
"appraisal": Appraisal(
status="affirming",
status=self._appraisal_status,
verifier=self._appraisal_verifier,
policy_ref=self._appraisal_policy_ref,
timestamp=session.iat,
Expand Down
33 changes: 33 additions & 0 deletions tests/test_agt_adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -253,3 +253,36 @@ def test_different_chain_tips_produce_different_measurements() -> None:
m1 = adapter.build_trust_record(s1)["runtime"]["measurement"]
m2 = adapter.build_trust_record(s2)["runtime"]["measurement"]
assert m1 != m2


# ---------------------------------------------------------------------------
# appraisal.status is verifier-owned (#331)
# ---------------------------------------------------------------------------

def test_appraisal_status_defaults_to_none() -> None:
"""`appraisal.status` was hardcoded to `affirming` with no way to change it, so every
record this adapter produced claimed an appraisal that had not happened. Spec section
3.3.1 makes the field the verifier's: building a record is not appraising it."""
record = _make_adapter().build_trust_record(_make_session())
assert record["appraisal"]["status"] == "none"


def test_appraisal_status_is_configurable_when_one_actually_happened() -> None:
record = _make_adapter(appraisal_status="affirming").build_trust_record(_make_session())
assert record["appraisal"]["status"] == "affirming"


@pytest.mark.parametrize("status", ["affirming", "warning", "contraindicated", "none"])
def test_every_appraisal_status_the_model_allows_reaches_the_record(status: str) -> None:
record = _make_adapter(appraisal_status=status).build_trust_record(_make_session())
assert record["appraisal"]["status"] == status
TrustRecord.model_validate(record)


def test_an_unappraised_record_still_signs_and_verifies() -> None:
"""The default must not cost a caller a valid record; `none` is a legitimate value."""
record = _make_adapter().build_trust_record(_make_session())
key = generate_key()
signed = sign_record(record, key)
assert verify_record(signed, public_key_or_jwk=key_to_jwk(key)) is not None
assert signed["appraisal"]["status"] == "none"