Repository navigation
Record a request the Guardian cannot vouch for apart from the steps, never under its claimed ids - #309
Open
rocklambros wants to merge 3 commits into
Open
Record a request the Guardian cannot vouch for apart from the steps, never under its claimed ids#309rocklambros wants to merge 3 commits into
rocklambros wants to merge 3 commits into
Conversation
PR #273 adds a rule to Trace Events: a Guardian MUST NOT emit acs.decision or acs.decision.failure under the pair of a request it rejects before trusting it, because such a request can carry any session and request id, a genuine request's included. The reference Guardian broke it. It opened the step span from the raw envelope before validating it, so an envelope that failed the schema with a forged request_id exported an acs.decision under that id. handleAcsRequest now validates once, before the span opens, and passes the result to dispatch, which no longer validates on its own. Only a request the Guardian trusts gets a span. The mapping requires the pair on every step span, so a span without it was not an option. The rejection stays visible in the envelope log. The exporter's start takes trusted as a required argument, so a caller that forgets it fails to compile. The sample trace drops its error step, which was an invalid envelope and now produces no span, and the docs that described it say so. Signed-off-by: rocklambros <rock@rockcyber.com>
Dropping a rejected request's span kept its ids off the genuine pair but hid the attack, and Bar ruled on issue #283 to keep it under separate names. A request the Guardian cannot vouch for is now a guardian.rejected_request span. It has no acs.session.id, acs.request_id, acs.decision or acs.decision.failure. The ids it claimed are under guardian.rejected.claimed_*, with the reason and any error code. That covers an envelope that fails validation, a request the Guardian could not check at all (guardian_fault, which before this went dark), and a step answered with SIGNATURE_INVALID, REPLAY_DETECTED or TIMESTAMP_OUT_OF_WINDOW. The pair now goes on a span only when it ends as a trusted step, so a check that runs inside dispatch after the span opened, as PR #229's do, cannot put a rejected request's ids back on a step span. The span's clock starts when the request arrives, so the time spent validating it counts toward a late decision. The sample trace holds a forged request that reuses the allowed call's ids and checks that the allowed call keeps the only span under its pair. The README says plainly that this Guardian verifies no signature, replay or timestamp yet, so a well-formed forgery is still trusted. Signed-off-by: rocklambros <rock@rockcyber.com>
A late rejection renamed the span but kept every step attribute it opened with, so a rejected toolCallResult still named the genuine call through acs.request_id_ref. Step attributes are now held back with the pair and go on only when the span ends as a trusted step. dispatch now returns its trust verdict beside the response, and end() takes it, so a check inside dispatch that answers with an honoured deny is recorded as a rejected request, not only one that answers with an error code. The code map stays as the backstop, now built from named constants. A Guardian fault keeps its guardian_error failure event, on a rejected span that carries no pair. A schema-invalid envelope is recorded whatever its method. The span's start time is the request's arrival, so its duration matches the elapsed time the failure event reports. The README and the trace docs say what a rejected span carries in two short paragraphs, and the replay sentence covers every pre-trust rejection. Signed-off-by: rocklambros <rock@rockcyber.com>
rocklambros
requested review from
GangGreenTemperTatum,
afogel,
artmaro,
bar-capsule,
fewdisc,
mamicidal,
sclintonowasp and
stefanoamorelli
as code owners
October 20, 2026 22:13
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
PR #273 adds a rule to Trace Events: a Guardian MUST NOT emit
acs.decisionoracs.decision.failureunder the pair of a request it rejects before trusting it. Our reference Guardian broke that rule. An envelope that failed the schema, carrying a forgedrequest_id, got anacs.decisionspan under thatrequest_id. This fixes the reference Guardian, following @bar-capsule's ruling on issue #283: keep the rejected request visible, under separate names, never under the genuine request's ids.guardian.rejected_requestspan. It carries noacs.session.id,acs.request_id, step attribute, oracs.decisionevent. The ids it claimed go underguardian.rejected.claimed_session_idandguardian.rejected.claimed_request_id, withguardian.rejected.reasonand the error code.guardian_fault, which still records itsguardian_error), and a step answered withSIGNATURE_INVALID,REPLAY_DETECTEDorTIMESTAMP_OUT_OF_WINDOW.dispatchnow returns its trust verdict next to the response. So a check that runs after the span opens can't leak a rejected request's ids onto a step span, whether it answers with an error or with a deny. That's where PR fix(agt): implement ACS-Core wire safeguards #229's checks will run.extend_opentelemetry.mdsay what a rejected span carries. The known limits say plainly that this Guardian checks nothing but the schema yet, so a well-formed forgery or replay is still recorded as a step until PR fix(agt): implement ACS-Core wire safeguards #229 lands.@sabasiddique1, this implements item 6 of issue #283, so you can skip that item. Items 1 to 5 and 7 are still yours.
Merge order: this is safe to merge before PR #273. Before that rule exists, the only effect is that a rejected request moves off the step spans. Please don't promote it to
mainwithout PR #273, or the other way round.Which issue does this implement
Part of #283
Refs #37
meaning, so it needs no issue
Base branch
integration, because this touches the specification, schemas, a referenceimplementation, an adapter, tests, or CI
main, because every changed path is on the documentation lane allowlist inCONTRIBUTING.md
Type of change
Specification changes
No normative text changes.
docs/spec/trace/extend_opentelemetry.mdchanges only where it describes what the reference Guardian exports.Breaking for implementers? No ACS change. Anyone reading the reference Guardian's trace sees a schema-invalid envelope as a
guardian.rejected_requestspan. Before, it was agen_ai.tool.callwith anenvelope_invaliddeny.I tested this
uv run pytest -vpasses on my machineuv run mkdocs build --strictpasses on my machineFrom
reference-implementations/agt/,bun run typecheckis clean andbun testpasses (1245). I checked the new tests by breaking each behavior on purpose. With trust always true, a late rejection ignored, the pair attached early,dispatch's verdict dropped, the fault event removed, or the arrival time ignored, the matching tests fail. Before writing this I ran a six-perspective premortem and a code review, and their findings are fixed here.Checklist
git commit -s(required by the DCO)Security
Without this, anyone who can reach the Guardian's port could write a decision into the trace under a live session's ids by sending a malformed envelope with those ids. A reader joining the Guardian's and the host's streams by pair would then see a step that never happened. A well-formed forgery or replay still gets through until PR #229 adds signature, replay and timestamp checks. The README says so.