Skip to content

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
integrationfrom
refimpl/273-untrusted-trace-pair
Open

rocklambros wants to merge 3 commits 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.

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>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

1 participant