feat: bind PIC authorization to TRACE evidence - #180
Conversation
Signed-off-by: Imran Siddique <imran.siddique@opaque.co>
|
CI is green: 478 tests passed locally (1 skipped), hosted Python 3.11/3.12, analysis, and CodeQL all pass. The bridge is intentionally optional and fail-closed, with schema, threat-boundary documentation, and focused negative tests. Requesting one maintainer review; I cannot self-approve. |
lywinged
left a comment
There was a problem hiding this comment.
Ran it before reading: 478 passed, 1 skipped, matching your evidence block. Then probed the refusal paths rather than reading them: key-id mismatch, tampered before.tool_call, out-of-scope impact, each PIC digest separately, and not-yet-valid all refuse correctly, with distinct messages. I couldn't get past any check I tried — the security core is sound.
Reading the PR against #179's own deliverables list, two gaps and one question.
Deliverable 4 — "Deterministic test vectors (positive and negative cases)" — isn't in the PR. The six cases (in five functions) are in-code, and the fixture key is Ed25519PrivateKey.generate() — fresh every run, so nothing about the suite is deterministic or portable. The corpus shape this repo already uses fits exactly: committed vectors with expected outcomes beside them, a generator, and a key whose private half is committed (the acta corpus and agent-manifest's suite are the two running precedents; #178 is open on making that the corpus-wide bar). Happy to build these — it's the same discipline as the receipt vectors.
The question: is transcript.after deliberately unbound?
verify_bridge(..., transcript={"before": {"tool_call": tool_call}, "after": {}}, now=150)
# → verifiesbefore is bound to the executed call; after only has to be a dict, so transcript_required: true is satisfied by an empty object. #179's pre-agreed constraint says "complete before/after verification", the summary says "transcript before/after evidence", the doc says it binds "its execution evidence" — three texts say complete, the code accepts {}. If the bridge deliberately can't know a result's shape, the three sentences should narrow to what the code does; otherwise after needs a binding. Your call which.
Coverage, against the machinery already running here. intent_bridge.py carries twenty-eight raise statements against six test cases. For the receipt rules, test_vector_completeness.py enforces two load-bearing fixtures per rule and defect-independence between them ("#124's criterion, executed"), and test_margins_have_not_thinned puts the floor plainly: two. The refusals I'd pin first: authorizer_key_id ≠ trusted kid (refuses correctly, no test, and it is the headline claim of the trust-boundary doc); tool_call_digest mismatch (the scope test changes the tool name, a different rule — a same-name call with different arguments is what reaches the digest comparison, and nothing does today); not-yet-valid (a different branch from expired, only expired is covered); the two PIC digests as two defects; PIC profile mismatch; declaration_digest; impact outside scope; transcript.before mismatch; non-boolean transcript_required.
The new schema is read by nothing. grep -rn pic-trace-bridge tests/ src/ returns one hit — the profile constant. No test loads schema/pic-trace-bridge-v1.json, no implementation references it, and the only validation it received is the meta-validation in your evidence block, which the suite doesn't run. So nothing would notice it diverging from the verifier — it pins signature to 86 base64url characters, for instance, while the code accepts anything that decodes. The repo already has both halves of the fix: per-schema positive/negative vectors (test_delegation_passes_json_schema / test_delegation_bad_digest_fails_json_schema) and the comparison guard test_packaged_schema_matches_the_normative_schema, which landed in #136 after two schema files drifted in three places while each suite exercised only one of them.
Small consistency note. now has no bool guard while authorized_at and expires_at in the same function do; now=True currently fails closed as instant 1, by accident rather than by rule.
Worth cross-linking to #66. This bridge is the first executable instance of the approval-shaped binding that thread is converging on: a validity window, per-action scope, digest binding to the specific call, fail-closed. The two gaps that thread has already named — an authorization withdrawn before execution, and authority-epoch staleness — are exactly what a v2 of this profile would add, and the candidate fixtures being drafted there could target this artifact rather than a hypothetical one.
One tiny thing while reading the trailer: the sign-off is imran@agentrust.com — MAINTAINERS.md has opaque.co and the org domain is agentrust-io.com. If the third domain is yours, ignore this; in a provenance project the DCO identity seemed worth a glance.
Approving, with the auto-merge in mind, and with the ledger explicit: the PR says "Implements", not "Closes", so #179 stays open — deliverable 4 (deterministic vectors, positive and negative) still lives there, and I'll take it as follow-up unless you'd rather keep it. The transcript.after question resolves either way after merge — a binding or three narrower sentences, your pick — and the schema round-trip rides along with the vectors. The design does what the threat-boundary doc claims where I could test it; nothing I found weakens the core.
Summary
nnImplements #179 as an optional PIC-CJSON/1.0 to TRACE authorization bridge.nn- Adds versioned bridge schema and informative threat-boundary documentation.n- Signs the complete authorization without embedding trust anchors.n- Binds authorizer_key_id to the trusted JWK kid.n- Preserves PIC intent/args digests and adds JCS declaration/tool-call bindings.n- Enforces authorization scope, validity, transcript before/after evidence, and fail-closed errors.n- Adds six focused positive/negative security tests.nn## Evidencenn- python -m pytest -q --basetemp=.pytest-tmp: 478 passed, 1 skippedn- ruff check src/agentrust_trace/intent_bridge.py tests/test_intent_bridge.py: passedn- JSON Schema Draft 2020-12 meta-validation: passedn- git diff --check: passednnSigned-off-by: Imran Siddique imran@agentrust.com