Skip to content

Record a request the Guardian cannot vouch for apart from the steps, never under its claimed ids - #309

Merged
bar-capsule merged 1 commit into
integrationfrom
refimpl/273-untrusted-trace-pair
Oct 28, 2026
Merged

bar-capsule merged 1 commit into
integrationfrom
refimpl/273-untrusted-trace-pair

Conversation

@rocklambros

Copy link
Copy Markdown
Collaborator

What changed

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. Our reference Guardian broke that rule. An envelope that failed the schema, carrying a forged request_id, got an acs.decision span under that request_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.

  • Rejected requests get their own span. A request the Guardian can't vouch for becomes a guardian.rejected_request span. It carries no acs.session.id, acs.request_id, step attribute, or acs.decision event. The ids it claimed go under guardian.rejected.claimed_session_id and guardian.rejected.claimed_request_id, with guardian.rejected.reason and the error code.
  • What counts as rejected: an envelope that fails validation, a request the Guardian couldn't check at all (guardian_fault, which still records its guardian_error), and a step answered with SIGNATURE_INVALID, REPLAY_DETECTED or TIMESTAMP_OUT_OF_WINDOW.
  • The pair is added last. A trusted step's pair and step attributes go on only when its span ends. dispatch now 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.
  • Sample trace: it now includes a forged request that reuses the allowed call's ids. A test checks that the allowed call keeps the only span under its pair.
  • Docs: the READMEs and extend_opentelemetry.md say 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 main without PR #273, or the other way round.

Which issue does this implement

Part of #283
Refs #37

  • This is an editorial correction (typo, grammar, link, formatting) with no change in
    meaning, so it needs no issue

Base branch

  • integration, because this touches the specification, schemas, a reference
    implementation, an adapter, tests, or CI
  • main, because every changed path is on the documentation lane allowlist in
    CONTRIBUTING.md

Type of change

  • Specification change (schema, hooks, events, AgBOM)
  • Reference implementation or adapter
  • Documentation
  • Tooling or CI
  • Governance (licensing, security policy, contributor docs)

Specification changes

No normative text changes. docs/spec/trace/extend_opentelemetry.md changes 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_request span. Before, it was a gen_ai.tool.call with an envelope_invalid deny.

I tested this

  • I synced my branch with the base branch before opening this
  • uv run pytest -v passes on my machine
  • uv run mkdocs build --strict passes on my machine

From reference-implementations/agt/, bun run typecheck is clean and bun test passes (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

  • Commits are signed off with git commit -s (required by the DCO)
  • Prose follows STYLE.md
  • No secrets, tokens, or internal URLs in the diff

Security

  • This change has no security impact

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.

…never under its claimed ids

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. The reference Guardian broke it: an envelope that failed the
schema, carrying a forged request_id, got an acs.decision span under that
request_id.

Following Bar's ruling on issue #283, a request the Guardian cannot vouch
for is now a guardian.rejected_request span, with the ids it claimed under
guardian.rejected.* and no ACS pair, step attribute or decision event. That
covers an envelope that fails validation, a Guardian fault, and a step
answered with SIGNATURE_INVALID, REPLAY_DETECTED or TIMESTAMP_OUT_OF_WINDOW.
handleAcsRequest validates once before the span opens. dispatch returns its
trust verdict beside the response, and a step span gets its pair and step
attributes only when it ends as a trusted step.

Rebuilt on integration after PR #272: dispatch also takes the envelope log,
and the step findings PR #272 added still reach the chain entry.

Signed-off-by: rocklambros <rock@rockcyber.com>
@rocklambros
rocklambros force-pushed the refimpl/273-untrusted-trace-pair branch from 2016401 to 5af6ffb Compare October 22, 2026 10:03

@bar-capsule bar-capsule left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, approved. Matches what we agreed on #283. I also ran it merged with today's integration (with #273's rule in), all green.
Once this is in, I'll open the promotion to main.

@bar-capsule
bar-capsule merged commit 599b1a3 into integration Oct 28, 2026
9 checks passed
@github-project-automation github-project-automation Bot moved this from In progress to Done in ACS Project Tracker Oct 28, 2026
jrcrittenden added a commit to jrcrittenden/ACS that referenced this pull request Oct 28, 2026
Brings in GenAI-Security-Project#309, GenAI-Security-Project#273, GenAI-Security-Project#168 and GenAI-Security-Project#323. GenAI-Security-Project#309 moves a request the Guardian
rejects before trusting it off the step spans, and leaves dispatch a
`rejection` to report for this PR's checks. This merge wires them in:

- dispatch keeps this PR's checks and returns them as Dispatched. A
  SIGNATURE_INVALID, TIMESTAMP_OUT_OF_WINDOW or REPLAY_DETECTED refusal
  reports signature_invalid, timestamp_out_of_window or replay_detected,
  and a payload that fails validation reports envelope_invalid, as it
  did in GenAI-Security-Project#309.
- validateForDispatch checks the envelope's shape only. The method
  payload is still validated inside dispatch after the signature, so an
  unsigned request with a malformed payload is refused as
  unauthenticated, not answered with a trusted deny.
- A method with no step span (handshake/hello, system/ping) opens no
  span, so a rejection dispatch reports for one is recorded as a
  rejected request when the response is sent.
- The "until PR GenAI-Security-Project#229" notes from GenAI-Security-Project#309 are gone, the sample trace is
  regenerated, and the denied span is copied into the OpenTelemetry
  page. GenAI-Security-Project#309's "no span for handshake/hello" test sends a schema-valid
  ClientHello, because an invalid one is now a rejected request.
- New end-to-end tests: a replayed request, an unsigned request and a
  forged hello are each recorded apart, and the genuine request keeps
  the only span under its ids.

Signed-off-by: Jason Crittenden <jrcrittenden@gmail.com>
esherman-credo added a commit to esherman-credo/agent-control-standard that referenced this pull request Oct 29, 2026
Brings in GenAI-Security-Project#309, which changed the exporter's start() to take whether the
Guardian trusted the request. The session and message tests now pass TRUSTED,
the sample is regenerated, and the sample test and the two sample
descriptions count the session and message spans beside GenAI-Security-Project#309's rejected
request.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Eli Sherman <esherman@credo.ai>
President added a commit to disciplinedware/agent-control-standard that referenced this pull request Oct 30, 2026
GenAI-Security-Project#309 changed packages/guardian/src. Only its tree ID changes; all 49
answers stay the same.

Claude-Session: https://claude.ai/code/session_01Rhwvb5CFUg1pyMvKKnbHXt
Signed-off-by: Konstantin Trunin <konstantin@trunin.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants