Repository navigation
Record every decision failure and count them at session end - #273
Conversation
|
This pull request's description says it closes #37, which is on the project roadmap. When it merges, GitHub closes that issue and the roadmap counts it as delivered. Please confirm the change delivers the whole issue before merging. |
…n issue #37 signature_invalid keeps a tampered response apart from organic failures. handshake_failure counts a session that started without a completed handshake, and its event goes on the acs.session span, not a step span. PR #273's sessionEnd count keys on this set of eight classes. Signed-off-by: rocklambros <rock@rockcyber.com>
|
@bar-capsule, both classes from your issue #37 answer are in PR #275 now ( |
Section 6.4 now requires an audit record for every decision failure under either posture, kept in the deployment's own audit log and emitted on Trace as acs.decision.failure when the deployment claims ACS-Trace. Section 4.1 surfaces a session started unguarded as the same event with class handshake_failure. sessionEnd.summary gains decision_failures, a per-class count the Observed Agent SHOULD report, and a session that ends without it reads as unmeasurable. conformance.md and the hooks.md sessionEnd summary follow, and a test keeps the counted classes equal to the acs.failure.class values in the OTel mapping. Signed-off-by: Bar Kaduri <bar@capsule.security>
…ke_failure its posture A Guardian emits no acs.decision.failure or acs.decision event under the pair of a request it rejects before trusting it: an envelope that fails validation, a failed signature, a replay, or a timestamp outside the window. Such a request can carry a genuine request's pair. handshake_failure now covers the refuse startup posture too, and its acs.failure.posture comes from the startup posture, since on_decision_failure is never negotiated. Section 4.1 records an unguarded session with posture fail_open. Signed-off-by: Bar Kaduri <bar@capsule.security>
2d48e5d to
dffca95
Compare
|
@rocklambros @afogel ready for review - the rest of #37 on top of #275, plus the two gaps from the #275 review: requests the Guardian rejects stay off the genuine request's IDs, and handshake_failure takes its posture from the startup posture. Small spec PR, all checks green. |
rocklambros
left a comment
There was a problem hiding this comment.
@bar-capsule, thanks, this reads well, and both gaps from the PR #275 review are in. I merged it into current integration locally. It merges cleanly, pytest passes and mkdocs build --strict is clean.
Two small asks, both one-line changes in files outside your diff that still state the old rule:
specification/v0.1.0/handshake.json:131, theon_decision_failuredescription, says: "Every fail-open proceed MUST be recorded as an audit event (Specification 6.4)." Since §6.4 now says every decision failure, could it read: "Every decision failure MUST be recorded as an audit event, fail-open or fail-closed (Specification 6.4)."docs/identity/overview.md:124says the Observed Agent "MUST record every fail-open proceed as an audit event". The same change works there: "MUST record every decision failure as an audit event".
One more thing, so you know it's covered: the new rule in events.md (no acs.decision under the pair of a request the Guardian rejects before trusting it) is something our reference Guardian breaks today. I sent it a request that fails schema validation with a forged request_id, and it exported an acs.decision span under that request_id. That's on the reference implementation, not this PR. I'm opening a small Guardian fix to land alongside it, so nothing to do here for that.
…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>
Signed-off-by: rocklambros <rock@rockcyber.com>
…tity overview Both still said only a fail-open proceed is recorded, which section 6.4 no longer says after this PR. They now match: every decision failure is recorded, fail-open or fail-closed. Signed-off-by: rocklambros <rock@rockcyber.com>
Addressed in 6e1ec9e: handshake.json and the identity overview now say every decision failure is recorded.
61993cb
into
GenAI-Security-Project:integration
PR GenAI-Security-Project#273 widened section 6.4 to record every decision failure and added sessionEnd's summary.decision_failures, and it closed issue GenAI-Security-Project#37. The template now asks for decision failures by class and drops the issue GenAI-Security-Project#37 caveat. PR GenAI-Security-Project#272 made section 8.6 say a signed HMAC decision is not attributable to the Guardian against the Observed Agent, and the trust-topology block now says so. Signed-off-by: rocklambros <rock@rockcyber.com>
Remove agtbridge/goeval, the port's Go reimplementation of AGT's policy runtime. The README merged in GenAI-Security-Project#256 requires maintained implementations to ship only AGT's runtime as their evaluator. AGT is now the only evaluator. Remove OPA from go.mod. The README allows a second evaluator in tests as a drift check; do not keep goeval for that, because it would have to follow every AGT update. A build without the agteval tag still compiles, and the command then refuses to start. Leave policy.evaluator, EVALUATOR and the agtbridge.Evaluator interface in place, with "agt" as the only value, so another evaluator is one new case. The bridge tests also use the interface to plug in fake evaluators. Switch the bridge tests that used the Go evaluator to AGT. They used it because it was fast and needed no Rust. AGT already ran all 49 recorded sessions before this change. Drop TestNewRefusesAnUnsupportedManifest: it tested which manifest features the Go evaluator refused. Drop the load measurements that only the Go evaluator produced. Regenerate the spec's JSON schema copies and re-record the TypeScript Guardian's answers. No recorded decision changed. Add a signer test against the example key in spec section 10. GenAI-Security-Project#272 defines the HKDF key-derivation parameters there. The Go signer already used exactly those parameters. In docs/conformance.md, mark three expectations of the GenAI-Security-Project#256 README as not met yet: - The command accepts one HMAC key. The README requires at least two keys at once to support key rotation. - A full audit queue, or a write failure, loses audit events the spec requires while decisions continue. The README requires keeping them and denying instead. - An oversized or unreadable request body gets an error, and the engine's deadline equals the declared timeout. The README requires a deny and a lower deadline. Mark the interim verification as met: make check runs the TypeScript conformance probes against this port. GenAI-Security-Project#273 needs no change. TestDecisionFailureRecordsItsClass and TestHandshakeFailureRecordsItsClass in internal/hostadapter already cover recording every decision failure. Verified locally: make check passes, including all 10 external wire checks. The container image and other CI platforms were not tested locally. CI runs those checks. Claude-Session: https://claude.ai/code/session_01SzmLMNjG3KSaCFfg8aKB11 Signed-off-by: Konstantin Trunin <konstantin@trunin.com>
What changed
The rest of the #37 decision, the parts that live outside the Trace event #275 added. §6.4 now requires an audit record for every decision failure under either posture: in the deployment's own audit log, and as an
acs.decision.failureevent on Trace when the deployment claims ACS-Trace. §4.1 surfaces a session started unguarded as the same event with classhandshake_failure.sessionEnd.summarygainsdecision_failures, a count per failure class that the Observed Agent SHOULD report, and a session that ends without it reads as unmeasurable.It also closes two gaps from the #275 review. A request the Guardian rejects before trusting it can carry a genuine request's pair, so it produces no failure or decision event under that pair; a deployment that records rejected requests uses other attribute names, as decided on #283. And
handshake_failuregets a posture: it comes from the startup posture, sinceon_decision_failureis never negotiated.Which issue does this implement
Closes #37
Refs #283, which decided how the reference Guardian records requests it can't vouch for
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 Discussion was opened. The decisions are on #37 and #283.
docs/spec/instrument/specification.mdsessionEndcount is a SHOULD. §4.1: an unguarded session becomesacs.decision.failurewith classhandshake_failureand posturefail_openon the session span.specification/v0.1.0/hooks/session-end.jsonsummary.decision_failures: an object keyed by failure class, each value an integer of 0 or more. The keys are limited to the eightacs.failure.classvalues.specification/v0.1.0/trace/otel-mapping.jsonSIGNATURE_INVALID,REPLAY_DETECTED,TIMESTAMP_OUT_OF_WINDOW).handshake_failurecovers both startup postures, and itsacs.failure.posturecomes from the startup posture.docs/spec/trace/events.mddocs/spec/conformance.mddocs/spec/instrument/hooks.mdsessionEndpayload line names the real fields (reason, notsession_reason, plus the missingabandonedvalue) and the new count.tests/test_decision_failure_classes.pysession-end.jsoncounts differ from theacs.failure.classvalues inotel-mapping.json.Breaking for implementers? The schema change is additive:
decision_failuresis optional. Two rules tighten. §6.4 previously required a record only for a step that proceeded without a decision, and now a fail-closed decision failure needs one too. And a Guardian that used to record a rejected replay, or any other untrusted request, under its claimed pair now records it under other names or not at all.I tested this
uv run pytest -vpasses on my machineuv run mkdocs build --strictpasses on my machinepytest reports 919 passed and 1 skipped (a case-insensitive filesystem skip). From
reference-implementations/agt/,bun run typecheckis clean andbun testreports 1234 passed, 1 skipped, 0 failed.session-end.jsonwas checked with jsonschema (Draft 2020-12): a count for known classes, a zero, and a payload with no summary are valid; an unknown class, a negative count and a non-integer are rejected.Merging
Signed-off-byline.Checklist
git commit -s(required by the DCO)Security
It adds detection rather than surface: a blocked handshake now produces an event and a count, and a forged or replayed request can no longer make a genuine decision read as unenforced.