Repository navigation
Align expectations for Guardian reference implementations (#252) - #256
Merged
rocklambros merged 8 commits intoOct 22, 2026
Conversation
…ations The directory had no top-level document. The new README says what the directory holds, then states what we expect from each maintained Guardian reference implementation. Four open pull requests add or rework a Guardian, and each read the one-sentence porting issues differently. The expectations let every implementation be reviewed against the same list. The expectations are a working aid for review. They are not part of the specification and they are not normative. Part of GenAI-Security-Project#252 Signed-off-by: Evgeny Kokuykin <artmaro@gmail.com>
artmaro
requested review from
GangGreenTemperTatum,
afogel,
bar-capsule,
fewdisc,
mamicidal,
rocklambros and
sclintonowasp
as code owners
October 11, 2026 20:34
github-actions
Bot
added
the
status:needs-triage
This comment has been minimized.
This comment has been minimized.
…n bytes The introduction now says only what the directory is. The table of contents and the status line are gone, because both would go stale as implementations land. Key material is at least 32 bytes, not 32 characters. The specification and every implementation count bytes. Part of GenAI-Security-Project#252 Signed-off-by: Evgeny Kokuykin <artmaro@gmail.com>
acs-board-reconciler
Bot
moved this from Needs triage
to In progress
in ACS Project Tracker
rocklambros
removed
the
status:needs-triage
rocklambros
previously requested changes
Oct 12, 2026
rocklambros
left a comment
rocklambros
left a comment
Collaborator
There was a problem hiding this comment.
@artmaro, a few fixes before we merge:
- Lines 49 and 51 don't match the TypeScript Guardian (PR #229). It sets
key_idto the session id, notdefault, and it base64-decodes the secret instead of using the raw string. A Go or Rust Guardian built to this README won't verify against the TypeScript one. Please change both lines to match what PR #229 does. - Line 23 says we maintain a contract test suite, but we don't have one yet. Please change it to "will maintain". Line 79 depends on the same suite, so please reword it too.
- Line 35 should name the runtime:
agent_control_specification, at the version inagt.lock. AGT's Go module is a different policy engine, so "the AGT runtime" is ambiguous for Go. - Lines 69-75 read as rules the TypeScript Guardian already meets, but its logs aren't bounded yet. Please mark them as targets.
On your questions: I'd list only TypeScript as maintained for now, and add the others as they merge. Yes to the AGT rule and to moving the shims.
- Name the policy runtime, agent_control_specification, at the version in agt.lock, and say that AGT's Go module does not qualify. - Set key_id to the session_id and base64-decode the key material, as the TypeScript Guardian in GenAI-Security-Project#229 does. - List only TypeScript as maintained. Other languages are added as they merge. - Say that the contract test suite will exist, and make verification depend on it. - Mark bounded processing and bounded logging as targets the TypeScript Guardian does not meet yet. - Add message signing to the architectural boundaries. Part of GenAI-Security-Project#252 Signed-off-by: Evgeny Kokuykin <artmaro@gmail.com>
Collaborator
Author
|
Thanks for reviewing, @rocklambros. All points are addressed in the latest commit:
|
Collaborator
|
@artmaro, thanks for turning these around. One correction before this goes further, and it's on me for not flagging it sooner.
So could you make that line say |
President
added a commit
to disciplinedware/agent-control-standard
that referenced
this pull request
Oct 15, 2026
The embedded schemas follow integration after GenAI-Security-Project#63, GenAI-Security-Project#258 and GenAI-Security-Project#261. The parity recording stores the git object IDs of the deciding TypeScript paths instead of a commit, so a squash merge no longer makes TestRecordingIsCurrent fail. The paths include agt.lock and the agt-bridge package.json, which pin the AGT commit and SDK. The recorder and the test read them from one list; the test fails when the recording names other paths or was made from another corpus, and the recorder refuses uncommitted changes to them. A test checks that the Go workflow runs on every deciding path. agt/README.md tells a TypeScript contributor when to re-record. A test requires agt-native.lock and agt.lock to pin the same AGT repo and commit. The native build fails when AGT at that commit changes ffi.rs, which agteval/acs.h is written from, or fetches another opa than agt-native.lock pins. Under the agt evaluator, policy.opa_timeout must be shorter than protocol.decision_timeout; guardian.yaml ships 4s. Otherwise a slow opa lets the agent stop waiting before AGT's deny arrives (GenAI-Security-Project#269). make load measures either evaluator under concurrent clients, selected by EVALUATOR like the other targets. Under agt each decision runs one opa process; on a 20-core host decisions stop rising near 16 concurrent ones, and at 256 they reach the 5s timeout. guardian.yaml now ships limits.max_engine_calls 32, and docs/load.md records the results. The load run found that an engine call gave its slot back only after its answer was sent, so a client's next step could be refused engine_saturated with no call running; callEngine now releases the slot first, also after a panic. When the host adapter's audit log cannot be written, the record goes to stderr as one JSON line with the final disposition, and the refusal's reasoning says whether the record went to stderr or nowhere. The audit file's close error now counts. The Codex and OpenCode hooks pass their own stderr. The §6.3 row says that ALLOW at a hook that permits no DENY is an interpretation (GenAI-Security-Project#270), and the docs say only the adapter library reaches those hooks. Dependabot proposes Go updates, one pull request per dependency across both modules. The Go workflow runs weekly, so govulncheck sees advisories for the toolchain that Dependabot does not move, triggers on the agt-bridge package.json, and no longer fetches full history. A test checks that the method table permits DENY exactly where hooks.md does, and fails on a Decision line it cannot classify. The coverage floor counts every package except commands, examples and test helpers. The Guardian meets the expectations GenAI-Security-Project#256 sets for reference implementations, mapped to tests in docs/conformance.md. audit.max_file_bytes and audit.max_files rotate each audit file; guardian.yaml keeps at most 1 GiB per log; max_file_bytes needs max_files of at least 1, so a rotation never deletes the live log. make load can set the rotation and fails if the log drops, splits or reorders a record or exceeds its bound; docs/load.md records a run and the cost of long sessions. storage.provider names the session store. The HMAC secret file holds base64 of at least 32 bytes. key_id is a configured label with no default, an unknown key_id is refused (GenAI-Security-Project#118, GenAI-Security-Project#272), and the hooks and the conformance harness take it as a flag. A toolCallResult whose request_id_ref matches no request is recorded (GenAI-Security-Project#272). Decisions from other pull requests that the Go code contradicted: - A shell command's URL ends only at ASCII whitespace; U+FEFF or U+00A0 in the userinfo no longer hides the host curl reaches (GenAI-Security-Project#268). - Versions are compatible when they share X.Y under 0.x; the handshake negotiates only such a version, and a step must match it (GenAI-Security-Project#196, GenAI-Security-Project#255). - A policy target that is not a string is denied evaluation_failed; the TypeScript fix is in GenAI-Security-Project#229, so two recorded cases are listed as ahead of the recording until it is re-recorded. - The host adapter records every decision failure under either posture, with its class, handshake failures included (GenAI-Security-Project#37). - The host adapter denies under any posture a signed error other than -32603, an unverified answer and a malformed answer; the docs mark this as an interpretation of §6.4 ahead of GenAI-Security-Project#32, which is open. The version rule is marked the same way against §3. - The audit directory is 0700; a symlinked audit path or an audit file open to group or others is refused (GenAI-Security-Project#71). Claude-Session: https://claude.ai/code/session_01DFdYnoBitHhPcUSq7wMWs1 Signed-off-by: Konstantin Trunin <konstantin@trunin.com>
Collaborator
|
@artmaro, one more thing on the signing section, and it folds into the Here's the exact wording from §10 so you can match it in one pass ( The bit I don't think anyone's caught: line 49 describes the HKDF derivation but never says what Honestly, I'd just link §10 for this whole block and list only what a reviewer checks on the PR, rather than restate the crypto here. We watched the restatement drift once already. If it says "sign and verify per §10" it can't fall out of sync the next time §10 moves. The HKDF params and the base64 decode are both fine, so nothing to change there. |
Review on this pull request asked to link section 10 rather than restate the HMAC key derivation and the meaning of key_id, so the README cannot drift when section 10 changes. GenAI-Security-Project#272 adds those rules to section 10. The section keeps only what section 10 does not define: how key material is encoded and read, that it stays out of code and logs, and that each implementation documents how to configure and replace it. Part of GenAI-Security-Project#252 Signed-off-by: Evgeny Kokuykin <artmaro@gmail.com>
…rchitecture-concept Signed-off-by: Evgeny Kokuykin <artmaro@gmail.com>
Collaborator
Author
|
Thank you, @rocklambros, agreed that's a better shape for this section. The signing section now links §10 instead of restating it, so the HKDF clause and the We can merge this PR after #272. Please take a final look. Thanks |
This was referenced Oct 21, 2026
This was referenced Oct 21, 2026
Open
Signed-off-by: rocklambros <rock@rockcyber.com>
…w's open points Every expectation the TypeScript Guardian does not meet yet is now marked Not met yet, with the issue that tracks it, not only the two bounded sections. A gap is listed only with an issue, so it stays public and tracked, and the README says which wording blocks a merge. Signing names the two section 10 paragraphs it relies on and lists the key_id checks a reviewer makes, the response's key_id included. The base64 convention says the decoded bytes are the secret and the HKDF input, and which base64 is meant. The runtime is named as agt.lock records it, the cross-language pin is agt_ref, and another evaluator is allowed only as a non-default that names itself on every decision. Replay protection is a blocking expectation, as section 10.3 makes it a MUST. Bounded processing answers a breach with a deny, so it never falls into the fail-open posture, and the log cap never evicts audit records. The host-adapter location, the contract suite and the maintained-set rules point at their issues, and verification until the suite exists matches the root README. Signed-off-by: rocklambros <rock@rockcyber.com>
Collaborator
|
@artmaro, thanks for linking §10. That was the right call. I ran a code review and a premortem on the README. They found a few more gaps, so I pushed the fixes to your branch myself to save us both another round. It's all in e9e3744. Could you review it and tell me if anything reads wrong to you? What changed:
I also updated the PR description. It now says |
Collaborator
Author
|
Thanks, @rocklambros, for the review and for pushing the fixes! The changes look good to me. One change before merge: I'd suggest keeping the AGT rule to the native runtime only. One rule is easier to apply the same way across every port, and it keeps it clear that a reference implementation runs AGT. Once this merges, I'll ask #169 to remove its Go evaluator and keep only native AGT. I hope that makes sense. On bounded logging: without a cap the audit log grows without limit, and that isn't something we'd run in production. We might want to keep a configurable size cap on all logs, as part of the reference implementation's configuration. In practice, archived logs would likely move to S3, a telemetry backend, or another archive, while the active log file rotates. I'm not insisting on this though and I'm fine with leaving the current wording as is. |
…sing audit records As artmaro asked: AGT's runtime is the only evaluator a maintained implementation ships, so every port is reviewed against one rule. A test may still compare AGT with another evaluator, as long as that one never answers a request. Every log, the audit log included, now has a size cap and rotates, with the rotated file free to move to an archive. Only diagnostic logs may drop entries. Audit records and the session chain are never dropped, and a Guardian that cannot rotate or archive them answers deny. Signed-off-by: rocklambros <rock@rockcyber.com>
rocklambros
merged commit Oct 22, 2026
fd6ff38
into
GenAI-Security-Project:integration
5 checks passed
github-project-automation
Bot
moved this from In progress
to Done
in ACS Project Tracker
jrcrittenden
added a commit
to jrcrittenden/ACS
that referenced
this pull request
Oct 22, 2026
Brings in GenAI-Security-Project#272 and GenAI-Security-Project#256. Both sides are kept in every conflict: - hashEntry keeps the section 11 JCS content hash and adds findings and request_id_ref to the content only when present, so an entry without findings hashes as before and one with them is committed to them. - dispatch and evaluateStep take both this branch's replay store, negotiated sessions, registry and keys and GenAI-Security-Project#272's envelope log and findings. The replay check still refuses a repeat before examineStep, so request_id_duplicate fires only for a repeat it missed. - governStep keeps the re-handshake retry and adds GenAI-Security-Project#272's one-time retry on REPLAY_DETECTED, signed like any other request. - GenAI-Security-Project#272's request_id_ref tests handshake first, and its duplicate tests stand in a replay store that lets the repeat through; a new test pins that a repeat inside the window gets -32005 and no finding. Signed-off-by: Jason Crittenden <jrcrittenden@gmail.com>
9 of 15 tasks
President
added a commit
to disciplinedware/agent-control-standard
that referenced
this pull request
Oct 25, 2026
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>
President
added a commit
to disciplinedware/agent-control-standard
that referenced
this pull request
Oct 25, 2026
The GenAI-Security-Project#256 README expects the configuration to map each key_id to one secret and accept at least two at once, so a secret is replaced without an outage. The library already took several keys; the command took one. Add security.hmac_previous_keys, a list of key_id and secret_file pairs. The Guardian verifies a request under any configured key and answers under the same key, as section 10 requires. An unknown key_id is still refused with SIGNATURE_INVALID. docs/configuration.md gives the steps to replace a secret. TestServesThePreviousHMACKey runs the command with a current and a previous key and checks that both are answered; it fails when the previous keys do not reach the signer. Claude-Session: https://claude.ai/code/session_01SzmLMNjG3KSaCFfg8aKB11 Signed-off-by: Konstantin Trunin <konstantin@trunin.com>
President
added a commit
to disciplinedware/agent-control-standard
that referenced
this pull request
Oct 25, 2026
The GenAI-Security-Project#256 README expects the default time limit to be below the timeout_config the Guardian declares. The Guardian declared decision_timeout and gave the engine the same time, counted from the engine call, so a step denied for time reached the agent after it stopped waiting, and the agent applied on_decision_failure. Count the deadline from the request's arrival and end it protocol.answer_margin before the declared timeout. Reading the body, the wait for the session lock and the session's load run under this Guardian's own deadline, since a session's terms are known only once it is loaded; the rest of the step, the store and the engine included, runs under the session's negotiated timeout. Recording the decision gets half the margin, past the decision's deadline but not past a lost session lock, a shutdown or the caller's deadline; signing and sending keep the other half. The margin scales with a session's negotiated timeout. guardian.yaml ships 500ms. policy.opa_timeout must stay below decision_timeout minus the margin, which keeps the GenAI-Security-Project#269 rule, and server.read_timeout at least that, since the body's read ends there. An engine that never answers, a store that stops answering, a step that waits too long for its session's lock, a late decision whose recording stalls, and a stalled store under a session negotiated with a shorter timeout all get an answer in time (TestDenialArrivesBeforeTheDeclaredTimeout, TestSlowStoreAnswersBeforeTheDeclaredTimeout, TestLockWaitAnswersBeforeTheDeclaredTimeout, TestSlowRecordingAnswersInTime, TestRecoveredSessionAnswersInItsOwnTime), and a session negotiated with a longer timeout keeps it (TestRecoveredSessionKeepsItsLongerTimeout); each fails without the change. A lost lock and a shutdown after the decision's deadline, and the caller's deadline, still stop the recording (TestLostLockStopsRecording, TestShutdownStopsRecording, TestCallerDeadlineStopsRecording). docs/conformance.md cites GenAI-Security-Project#324 for the body size case: the Guardian cannot sign a deny without the key_id and session_id inside the body it does not read. Claude-Session: https://claude.ai/code/session_01SzmLMNjG3KSaCFfg8aKB11 Signed-off-by: Konstantin Trunin <konstantin@trunin.com>
President
added a commit
to disciplinedware/agent-control-standard
that referenced
this pull request
Oct 25, 2026
The GenAI-Security-Project#256 README expects the audit events §6.4, §6.5 and §9.2 require, and the §8 session chain, to be never dropped: if the Guardian cannot keep them, it answers deny. §6.4's record is the Observed Agent's, and the host adapter already denies when it cannot write it; the §8 chain lives in the store, and a step whose entry is not appended is already denied, or allowed with the reason at a hook that permits no DENY. The Guardian's own events, those of §6.5 and §9.2 and the unresolved request_id_ref hooks.md requires at toolCallResult, were written in the background and dropped when the queue was full or the disk failed, and rotation deleted the oldest file. Add AuditLog.RequiredEvent, which returns once the event is written or its context ends. modify_unsupported, which names the rewrite not delivered, ask_substituted and request_id_ref_unresolved go through it; when it fails, the step is answered DENY with the substitute's reason code, which §6.5 and §9.2 require, and audit_unavailable, a reason code of this port. An embedder that implements AuditLog adds the method. A nil AuditLog records nothing, so such a step is denied. At postCompact, where §6.5 permits no DENY, the step is answered ALLOW with modify_unsupported and audit_unavailable, and the failure is reported to the audit log and its error callback. JSONLAuditLog writes a required event in queue order and reports the write by the step's deadline; a write still pending then also reaches the error callback and Close. An event whose waiter gave up before the writer took it is not written; a write already under way may still land, followed by the Guardian's failure record for the same request. Each file gets its own size and file count: audit.max_envelope_file_bytes and audit.max_envelope_files replace audit.max_file_bytes and audit.max_files, and audit.max_event_file_bytes and audit.max_event_files are new. The envelope file deletes its oldest file. The event file deletes none: while audit.max_event_files remain, each event is dropped and counted and a required one fails, until the deployment archives them; each new fill reports once. guardian.yaml keeps at most 1 GiB of envelopes and 2.25 GiB of events. NewJSONLAuditLog takes one JSONLRotation per file and refuses two paths that rotate onto each other. Tests cover a failing, a panicking and a missing log for each of the three events, a stalled, a queued-out and a failed write, an abandoned event, retention across rotations, a full and a refilled event file and overlapping paths; each fails without its fix. Claude-Session: https://claude.ai/code/session_01SzmLMNjG3KSaCFfg8aKB11 Signed-off-by: Konstantin Trunin <konstantin@trunin.com>
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
The README says what we expect from each maintained Guardian reference implementation. The expectations cover architecture, message signing, logging, concurrency, use of the AGT runtime, and verification. Each one the TypeScript Guardian does not meet yet is marked Not met yet, with the issue that tracks it.
Depends on PR #272. The signing section links §10's "HMAC key derivation" and "
key_idfor HMAC" paragraphs, and those land with PR #272. Merge this after it.Which issue does this implement
Part of #252
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
What the README decides
agt_refinagt/agt.lock. A port may ship another evaluator only if AGT is the default and is what its tests run against, the other is never an automatic fallback, and each decision names its evaluator inmetadata.evaluator.key_idcomes from configuration with no default.I tested this
uv run pytest -vpasses on my machineuv run mkdocs build --strictpasses on my machineChecklist
git commit -s(required by the DCO)Security