Repository navigation
fix(agt): implement ACS-Core wire safeguards - #229
jrcrittenden wants to merge 11 commits into
Conversation
1f40023 to
e523b2d
Compare
afogel
left a comment
There was a problem hiding this comment.
great first pass! a few requests to improve code quality/reduce footguns, and a few items that might impact correctness down the line
| const requestTimestampMs = Date.parse(envelope.params.timestamp); | ||
| const drift = Math.abs(nowMs - requestTimestampMs); | ||
| if (drift > SKEW_WINDOW_MS) { |
There was a problem hiding this comment.
Should fix: a leap-second timestamp gets past the skew window (§10.3).
The schema's date-time format accepts 23:59:60, but Date.parse returns NaN for it, and NaN > SKEW_WINDOW_MS is false. I reproduced it against a live Guardian with a signed request:
"2020-01-01T00:00:00Z" → -32006
"2020-12-31T23:59:60Z" → allow
Only a key holder can produce such a request, since the timestamp is signed. But the NaN expiry is also never pruned from the replay store, so each one stays there for good.
| const requestTimestampMs = Date.parse(envelope.params.timestamp); | |
| const drift = Math.abs(nowMs - requestTimestampMs); | |
| if (drift > SKEW_WINDOW_MS) { | |
| const requestTimestampMs = Date.parse(envelope.params.timestamp); | |
| const drift = Math.abs(nowMs - requestTimestampMs); | |
| if (!Number.isFinite(drift) || drift > SKEW_WINDOW_MS) { |
Why this helps: §10.3's MUST holds for every timestamp the schema accepts, and the replay store can't hold entries that never expire.
There was a problem hiding this comment.
Fixed in beee5b5. Non-finite drift is now rejected with -32006, so a NaN expiry never reaches the replay store. The skew test covers 2020-12-31T23:59:60Z.
Reply drafted with Claude Code.
| | Handshake, `handshake/hello` | Served. The Guardian validates ClientHello, rejects unsupported versions (`-32001`), narrows `methods_evaluated` to the intersection with the client's `methods_implemented`, and negotiates `profiles_accepted`. | | ||
| | Request and response envelopes, JSON-RPC 2.0 | Implemented. Inbound envelopes are validated against the schemas. Outbound responses are checked and the check is logged, never thrown. | | ||
| | Hook taxonomy, minimum of six hooks | Two of nineteen `steps/*` methods are evaluated: `steps/toolCallRequest` and `steps/toolCallResult`. The other four minimum hooks are not evaluated. | | ||
| | Five dispositions | `allow`, `deny` and `modify` are complete. `ask` is sent without `ask_details`, so it fails the response schema. `defer` is never produced. | | ||
| | SessionContext with published `chain_hash` | The Guardian keeps a hash chain per session and writes it to a log. It does not put `chain_hash` on any response. | | ||
| | Replay protection | Not implemented. No timestamp window check. No duplicate `request_id` check. | | ||
| | Baseline HMAC-SHA256 signature | Not implemented. Nothing signs or verifies an envelope. | | ||
| | Hook taxonomy, minimum of six hooks | All six ACS-Core hooks are dispatched: `steps/toolCallRequest` and `steps/toolCallResult` are policy-evaluated; `steps/sessionStart`, `steps/sessionEnd`, `steps/userMessage` and `steps/agentResponse` are accepted with allow-by-default and recorded in the session chain. | |
There was a problem hiding this comment.
Should fix: these two rows state more than the spec supports.
- Hook minimum.
conformance.mdlistssubagentStartfor clients that can start subagents (Claude Code and OpenCode both can), andagentTriggeras an alternative touserMessage. Both return-32011here. I testedsubagentStart. - Handshake. Negotiation is correct, but nothing enforces it. A step sent with no handshake gets
allow, and so does a method the handshake didn't negotiate, where §17.1 calls for-32003 CAPABILITY_NOT_NEGOTIATED. The PR body lists this as follow-up work. The README should say so too.
| | Handshake, `handshake/hello` | Served. The Guardian validates ClientHello, rejects unsupported versions (`-32001`), narrows `methods_evaluated` to the intersection with the client's `methods_implemented`, and negotiates `profiles_accepted`. | | |
| | Request and response envelopes, JSON-RPC 2.0 | Implemented. Inbound envelopes are validated against the schemas. Outbound responses are checked and the check is logged, never thrown. | | |
| | Hook taxonomy, minimum of six hooks | Two of nineteen `steps/*` methods are evaluated: `steps/toolCallRequest` and `steps/toolCallResult`. The other four minimum hooks are not evaluated. | | |
| | Five dispositions | `allow`, `deny` and `modify` are complete. `ask` is sent without `ask_details`, so it fails the response schema. `defer` is never produced. | | |
| | SessionContext with published `chain_hash` | The Guardian keeps a hash chain per session and writes it to a log. It does not put `chain_hash` on any response. | | |
| | Replay protection | Not implemented. No timestamp window check. No duplicate `request_id` check. | | |
| | Baseline HMAC-SHA256 signature | Not implemented. Nothing signs or verifies an envelope. | | |
| | Hook taxonomy, minimum of six hooks | All six ACS-Core hooks are dispatched: `steps/toolCallRequest` and `steps/toolCallResult` are policy-evaluated; `steps/sessionStart`, `steps/sessionEnd`, `steps/userMessage` and `steps/agentResponse` are accepted with allow-by-default and recorded in the session chain. | | |
| | Handshake, `handshake/hello` | Served, not yet enforced. The Guardian validates ClientHello, rejects unsupported versions (`-32001`), narrows `methods_evaluated` to the intersection with the client's `methods_implemented`, and negotiates `profiles_accepted`. Open Core gap: later steps are not checked against the negotiated session, so a step with no prior handshake, or a method the handshake did not negotiate, is evaluated rather than refused with `-32003`. | | |
| | Request and response envelopes, JSON-RPC 2.0 | Implemented. Inbound envelopes are validated against the schemas. Outbound responses are checked and the check is logged, never thrown. | | |
| | Hook taxonomy, minimum hook set | Partial. `steps/toolCallRequest` and `steps/toolCallResult` are policy-evaluated; `steps/sessionStart`, `steps/sessionEnd`, `steps/userMessage` and `steps/agentResponse` are accepted with allow-by-default and recorded in the session chain. Open Core gap: `steps/subagentStart` (required for subagent-capable clients) and `steps/agentTrigger` (the alternative to `userMessage`) return `-32011`. | |
Why this helps: the acs-core claim is already described as qualified. This makes the qualification match the spec.
There was a problem hiding this comment.
Applied your wording in beee5b5. The handshake row says "served, not yet enforced" (no -32003), and the hook-set row says "Partial" (subagentStart and agentTrigger return -32011).
Reply drafted with Claude Code.
There was a problem hiding this comment.
Update: dd7e3b4 enforces the handshake, so the row now reads "Implemented". Hook traffic without a hello, or for a method the hello did not declare, is refused with -32003.
Reply drafted with Claude Code.
| input.tool_name, | ||
| ]); | ||
| return createHash("sha256").update(canonical).digest("hex"); | ||
| const content = { |
There was a problem hiding this comment.
Suggestion: add request_hash so the chain commits to what each step contained (context-entry.json, RECOMMENDED).
This projection now matches §8.2 exactly. Nice: the old array-based hash didn't, and the issue never flagged it. But it drops tool_name, which the old hash included, and doesn't add request_hash. So the chain records which step ran, not what it asked for. A rewritten argument or tool name leaves every published chain_hash unchanged.
context-entry.json defines the field as the lowercase hex SHA-256 of the JCS-canonicalized request params:
type SessionStep = {
readonly method: string;
readonly request_id: string;
readonly tool_name: string;
readonly request_hash: string; // sha256(JCS(params)), computed once at the call site
};
const content = {
entry_id: `${input.session_id}:${input.seq}`,
step_id: input.request_id,
step_type: input.method,
timestamp: input.recorded_at,
request_hash: input.request_hash,
};Each appendContextEntry call site already holds envelope.params, so this is one extra field on the way in.
Why this helps: the published head becomes evidence of content, not just order. That's what §8.6's tamper-evidence argument is about.
There was a problem hiding this comment.
Added in beee5b5. request_hash = sha256(JCS(params)) is in the committed content at all three append sites. The Inspector's log type and line check now include it, and a test confirms that changing request_hash changes the entry hash.
Reply drafted with Claude Code.
|
|
||
| const guardianUrl = process.env.ACS_GUARDIAN_URL ?? DEFAULT_GUARDIAN_URL; | ||
| const guardian = createGuardianClient(guardianUrl); | ||
| const hmacSecret = process.env.ACS_HMAC_SECRET |
There was a problem hiding this comment.
Suggestion: read ACS_HMAC_SECRET once, through one validator.
The three places that read it disagree:
| Where | Empty string | Not valid base64 | Under 32 bytes |
|---|---|---|---|
guardian/src/main.ts:47 |
error | decoded to garbage | error |
hosts/claude-code/acs-hook.ts:536 (here) |
silently unsigned | decoded to garbage | accepted |
hosts/opencode/acs-plugin.ts:1400 |
silently unsigned | decoded to garbage | accepted |
Buffer.from(s, "base64") never throws, so a typo produces a wrong key and an unexplained SIGNATURE_INVALID.
Let's read the environment once at each entry point into a typed config, and pass that down. buildServerHello(env?) already follows this pattern.
export type Env = Readonly<Record<string, string | undefined>>;
export class ConfigError extends Error {}
const BASE64 = /^[A-Za-z0-9+/]+={0,2}$/;
const MIN_HMAC_KEY_BYTES = 32;
export function decodeHmacSecret(raw: string | undefined): Buffer | undefined {
if (raw === undefined || raw === "") return undefined;
if (raw.length % 4 !== 0 || !BASE64.test(raw)) throw new ConfigError("ACS_HMAC_SECRET is not valid base64");
const key = Buffer.from(raw, "base64");
if (key.length < MIN_HMAC_KEY_BYTES) throw new ConfigError(`ACS_HMAC_SECRET must decode to at least ${MIN_HMAC_KEY_BYTES} bytes`);
return key;
}
export type HostConfig = {
readonly guardianUrl: URL;
readonly hmacSecret: Buffer | undefined;
readonly auditLogPath: string;
};
export function readHostConfig(env: Env = process.env): HostConfig { /* … */ }The host can't import from the Guardian package (invariants.test.ts). A small shared wire package holding decodeHmacSecret, JCS and HKDF would satisfy that rule and remove the duplicated JCS and HKDF code too. Two related points: a deployment that requires signing should fail loudly when the host has no key, rather than run unsigned. And the Python adapters in #22 also accept ACS_HMAC_SECRET_FILE, which keeps the secret out of child processes' environments, so offering it here keeps the two consistent.
Why this helps:
- Correctness: one validation rule everywhere.
- Maintenance: tests pass a plain object instead of changing
process.env. - Readability: every setting the host reads is visible in one place.
There was a problem hiding this comment.
Agreed on all of this. We're deferring the acs-wire package, the authenticator object, reading the environment through one validator, and single-sourced constants to a follow-up PR, tracked in #230, so this one stays reviewable.
Reply drafted with Claude Code.
| export function buildServerHello(env?: { ACS_ON_DECISION_FAILURE?: string }): ServerHello { | ||
| /** §10.3: the window this Guardian enforces, declared in the ServerHello so | ||
| * the client knows the bound. Matches SKEW_WINDOW_MS in server.ts. */ | ||
| const SKEW_WINDOW_MS = 300_000; |
There was a problem hiding this comment.
Suggestion: give each constant one source, split by who owns it.
This is the same problem as the environment variables: one value defined in several places, kept in sync only by comments.
| Value | Defined in |
|---|---|
Skew window 300_000 |
Here, and server.ts:125. Only the comment "Matches SKEW_WINDOW_MS in server.ts" keeps them equal, and the ServerHello must declare the exact window the gate enforces. |
ACS version "0.1.0" |
NEGOTIATED_VERSION here, but the version check uses the literal (server.ts:940, :942). Also in deny-on-invalid-envelope.ts, build-envelope.ts and the host's handshake.ts. |
Timeout 5000 |
Here and in host-adapter/src/failure-posture.ts:73 |
| Error codes | Some are named (SIGNATURE_INVALID_CODE), but -32006, -32005, -32001 and -32003 are inline literals in server.ts. The host repeats the set in REFUSAL_RPC_CODES. |
"HMAC-SHA256" |
7 literals across 4 files |
Minimum key length 32 |
main.ts:54 and server.ts:484 |
Signature type { algorithm; value; key_id } |
Declared inline 5 times (acs-result.ts, validate-envelope.ts, server.ts, guardian-client.ts, sign-envelope.ts) |
There are two kinds of value here, and they belong in two places:
- Fixed by the spec (version, §17.1 codes, algorithm names, HKDF output length, the signature type): one
protocol.tsin a shared wire package that both sides import. The host'sREFUSAL_RPC_CODESthen becomes a set built from named codes, not a copied list.export const ACS_VERSION = "0.1.0"; export const SIGNATURE_ALGORITHM = "HMAC-SHA256"; export const AcsError = { SessionRefused: -32000, UnsupportedVersion: -32001, ProvenanceRequired: -32002, CapabilityNotNegotiated: -32003, SignatureInvalid: -32004, ReplayDetected: -32005, TimestampOutOfWindow: -32006, ChainMismatch: -32007, } as const; export type AcsSignature = { readonly algorithm: typeof SIGNATURE_ALGORITHM; readonly value: string; readonly key_id: string };
- Chosen by the deployment (skew window, timeouts, replay-store limits, minimum key length): fields of the Guardian's config, read once alongside the environment variables (see my comment on
acs-hook.ts:536).buildServerHelloand the timestamp gate then both readconfig.skewWindowMs, so they can't drift apart.
Why this helps:
- Correctness: the ServerHello always declares what the gates enforce.
- Maintenance: a spec change to a code or the version is a one-line edit.
- Readability:
AcsError.TimestampOutOfWindowsays what-32006means.
There was a problem hiding this comment.
Agreed on all of this. We're deferring the acs-wire package, the authenticator object, reading the environment through one validator, and single-sourced constants to a follow-up PR, tracked in #230, so this one stays reviewable.
Reply drafted with Claude Code.
| if ( | ||
| hmacSecret && | ||
| extractMethod(raw) !== "system/ping" && | ||
| requestHasValidSignature(raw, Buffer.from(hmacSecret, "base64")) |
There was a problem hiding this comment.
Suggestion: put HMAC behind one authenticator object, the same way this codebase already handles sinks and stores.
Why an object fits what this PR adds
- §10 makes HMAC-SHA256 the baseline of a registry of algorithms. ACS-Crypto adds ECDSA, RSA-PSS and ML-DSA, and keys can come from a TLS exporter instead of a shared secret. Signing is the part of this code the spec already expects to vary, so it belongs behind an interface from the start. The next algorithm then means adding a factory, not editing
dispatch. - §10 also requires both sides to compute the same canonical input. That argues for one implementation, not one copy per package.
- It matches the house style:
create*()factories that return a role (createGuardianClient,createAuditSink,createReplayProtectionStore), and null objects for disabled features.NULL_ENVELOPE_LOG_SINKsays it directly: "a composition root disabling observability gets a value of the sameEnvelopeLogSinktype, not a special case." HMAC is currently the one special case.
What's duplicated or unnecessary today
| What | Where |
|---|---|
| JCS, implemented twice | verify-signature.ts:9, host-adapter/src/sign-envelope.ts:11 |
| HKDF key derivation, implemented twice | verify-signature.ts:22, sign-envelope.ts:21 |
| Request verification, run twice per request | server.ts:864–886, and again in requestHasValidSignature (:1270) |
Choosing the response's result vs error to sign, implemented twice |
verify-signature.ts:41, sign-envelope.ts:76 |
| Decoding the base64 secret, five times | main.ts:53, server.ts:484, :658, :662, :880 |
| The 32-byte key-length check, twice | main.ts:54, server.ts:484 |
if (hmacSecret) branches, threaded through startGuardian → handleAcsRequest → dispatch |
server.ts, 16 mentions |
extractSessionId |
:1252. Only exists so the signing step can rediscover a session id that verification had already found. |
Proposed: a small packages/acs-wire workspace package with no dependencies beyond node:crypto, imported by both guardian and host-adapter. The rule that the host never imports the Guardian still holds.
packages/acs-wire/src/
protocol.ts ACS_VERSION, AcsError codes, AcsSignature, AcsSignatureAlgorithm
jcs.ts canonicalize(): RFC 8785, throws on undefined and non-finite numbers
authenticator.ts createHmacAuthenticator(), NULL_AUTHENTICATOR, the two role types
export type Verification =
| { readonly ok: true; readonly sessionId: string }
| { readonly ok: false; readonly reason: string };
/** The Guardian's role. */
export interface RequestVerifier {
readonly algorithms: readonly AcsSignatureAlgorithm[]; // ServerHello signature_algorithms_supported
verifyRequest(request: JsonObject): Verification;
signResponse<R extends JsonObject>(response: R, sessionId: string): R; // returns a signed copy
}
/** The host's role. */
export interface RequestSigner {
readonly algorithms: readonly AcsSignatureAlgorithm[]; // empty means: claim no acs-core
signRequest<T extends SignableRequest>(request: T): T & SignedRequest;
verifyResponse(response: JsonObject, sessionId: string): Verification;
}
/** The secret goes in here and nowhere else. Strict base64, minimum length, per-session HKDF. */
export declare function createHmacAuthenticator(ikm: Buffer): RequestVerifier & RequestSigner;
/** Signing disabled: same type, not a special case. Compatibility tests only. */
export declare const NULL_AUTHENTICATOR: RequestVerifier & RequestSigner;In the Guardian:
// main.ts: the only place that knows about the environment variable
await startGuardian({ ...options, authenticator: createHmacAuthenticator(config.hmacSecret) });
// dispatch: verify once, and return the result with the response (null for ping, per §13)
const auth = authenticator.verifyRequest(raw);
if (!auth.ok) return { response: errorResponse(rpcId, AcsError.SignatureInvalid, "SIGNATURE_INVALID", { reason: auth.reason }), auth };
// handleAcsRequest: only authenticated requests get signed responses, as now
const { response, auth } = await dispatch(/* … */);
const outgoing = auth?.ok ? authenticator.signResponse(response, auth.sessionId) : response;That removes the inline checks at :864–886, requestHasValidSignature, extractSessionId, all five decodes, and every if (hmacSecret). buildServerHello reads authenticator.algorithms. The host side is in my comment on govern-step.ts:665.
Why this helps:
- Encapsulation: one closure holds the secret, and each side depends on a role, not on raw key bytes.
- Maintenance: adding an algorithm, or fixing canonicalization, is a change in one place.
- Review: "what is authenticated, and how" has one implementation with its own tests.
There was a problem hiding this comment.
Agreed on all of this. We're deferring the acs-wire package, the authenticator object, reading the environment through one validator, and single-sourced constants to a follow-up PR, tracked in #230, so this one stays reviewable.
Reply drafted with Claude Code.
| let envelope: AcsRequestEnvelope; | ||
| try { | ||
| envelope = buildEnvelope(hookEventName, payload, hookmap); | ||
| if (hmacSecret) { |
There was a problem hiding this comment.
Suggestion: let GuardianClient own signing as well as verifying. This is the host half of my comment on server.ts:658.
Right now GuardianClient.post verifies responses, but governStep (here) and negotiateSessionConfig (handshake.ts:249) sign requests. So the secret is passed into five places, and any new caller of post has to remember to sign, or the Guardian refuses it.
// acs-hook.ts and acs-plugin.ts: construct once, at the composition root
const authenticator = config.hmacSecret ? createHmacAuthenticator(config.hmacSecret) : NULL_AUTHENTICATOR;
const guardian = createGuardianClient(config.guardianUrl, { authenticator });
// guardian-client.ts post(): sign on the way out, verify on the way back (ping exempt, per §13)
const outgoing = envelope.method === "system/ping" ? envelope : authenticator.signRequest(envelope);
// …send, then authenticator.verifyResponse(response, sessionId)Remove:
governStep'shmacSecretoption and the signing call (:171–173,:440,:665–667).negotiateSessionConfig'shmacSecretoption (handshake.ts:83,:249–251).profiles_supportedreadsguardian.algorithms.length > 0instead (:208). The check that the Guardian acceptedacs-corestays where it is, because it's about negotiation, not cryptography.- OpenCode's
Deployment.hmacSecretand its two pass-throughs (acs-plugin.ts:1185,:1327,:1354,:1412). sign-envelope.ts, which moves intoacs-wire.
Also, the doc comment at :171 says "Base64-encoded IKM," but the type is a decoded Buffer. Moving the option out of governStep removes the mismatch.
Why this helps:
- Encapsulation: only the composition root knows a secret exists.
governStepgoes back to governing steps. - Correctness: signing and verifying use the same object, so they can't use different keys, and no caller can forget to sign.
There was a problem hiding this comment.
Agreed on all of this. We're deferring the acs-wire package, the authenticator object, reading the environment through one validator, and single-sourced constants to a follow-up PR, tracked in #230, so this one stays reviewable.
Reply drafted with Claude Code.
51ffd7d to
beee5b5
Compare
|
Thanks, @jrcrittenden: I ran the issue's wire probes, split into 20 MUST requests, and none pass at the base (770f1e0) while 17 pass at this head (beee5b5). Three remain. The Guardian accepts a replay after a restart, allows a call outside the negotiated methods_implemented where spec section 17.1 lists -32003, and answers a step from a session that never sent handshake/hello, which spec section 4 rules out and your follow-up list omits. I opened probityai/agent-evidence-vectors#74 with them as an acceptance test, pinned in CI at both commits. |
|
Thanks, @astrogilda. Running these as an acceptance test in probityai/agent-evidence-vectors#74 is very useful. Two of the three are fixed in dd7e3b4:
Replay after a restart is still open. The replay store is in memory, so it does not survive a restart. The README now lists this as an Open Core gap, and it is tracked for follow-up. Reply drafted with Claude Code. |
|
Potential conformance issue: replay-cache capacity exhaustion uses -32005 REPLAY_DETECTED The replay-protection store distinguishes three outcomes: "fresh", "replay", and "capacity". However, server.ts maps both non-fresh outcomes to JSON-RPC error code -32005: if (replayStatus !== "fresh") {
return errorResponse(
rpcId,
-32005,
replayStatus === "replay"
? "REPLAY_DETECTED"
: "REPLAY_PROTECTION_CAPACITY",
);
}This means a previously unseen request can receive: The concern is that ACS §10.3/§17.1 defines -32005 as REPLAY_DETECTED for duplicate request_id/nonce detection. Replay-cache capacity exhaustion is a different condition: the request may be completely new and has not necessarily been seen before. So I don't think the fail-closed behavior itself is the issue. The concern is specifically how the "capacity" result is translated into the ACS wire error.
If the latter is intentional, it would be useful for the specification to state that explicitly so implementations and clients do not interpret -32005 exclusively as duplicate request_id/nonce detection. |
|
Hi @jrcrittenden, I have two small asks here and one question. They come from reviewing #244, the OpenTelemetry trace exporter, which will meet this PR in 1. Please put 2. Heads-up: "protocols/MCP/tools/call": "no step_to_span entry in otel-mapping.json yet",If you land first, #244 adds it. Either way the gap stays visible instead of silent. #244 also no longer edits 3. A question on the signing block in |
A full replay-protection cache answered a brand-new request with -32005 REPLAY_DETECTED. §17.1 recovers from -32005 by regenerating request_id, which cannot succeed while the cache is full. Return -32000 SESSION_REFUSED with data.reason "replay_protection_capacity" instead: it is in the §17.1 registry, so every ACS client fails closed on it. startGuardian takes an optional replayProtectionStore so a server test can fill the cache. Reported by @shivang-sec on GenAI-Security-Project#229. Signed-off-by: Jason Crittenden <jrcrittenden@gmail.com>
|
Thanks, @shivang-sec, this is a real conformance bug. A full replay cache is not a replay. The §17.1 recovery for Fixed in a4efc16. A full cache now returns The drawback is that §17.1 says not to retry |
|
Thanks, @rocklambros. All three are addressed in 6e6942d. 1. 2. 3. The signing block. Yes, it could throw, and not only on responses the Guardian built. The request half runs before any signature is verified, on unauthenticated input. Two inputs reproduced an HTML 500: a The block is now wrapped ( Envelope log: this block sits between |
The HMAC signing block in handleAcsRequest ran outside any try, on unauthenticated input. A session_id over 1024 bytes makes HKDF throw, and a deeply nested body overflows the stack in JCS canonicalization. Either escaped handleAcsRequest as an HTML 500, which a host reads as a hook that never fired. The block is now wrapped. A request that throws there cannot carry a valid signature, so its response goes out unsigned, like any other unauthenticated request's, and the failure is reported on stderr. The envelope log records the same response it did before, minus the signature it could never have had. Every AcsResult now carries metadata.evaluator "deterministic", so the trace exporter in GenAI-Security-Project#244 reads the evaluator from the wire instead of assuming it. Both raised by @rocklambros on GenAI-Security-Project#229. Signed-off-by: Jason Crittenden <jrcrittenden@gmail.com>
- 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>
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>
1d6b224 to
1d28f6a
Compare
A full replay-protection cache answered a brand-new request with -32005 REPLAY_DETECTED. §17.1 recovers from -32005 by regenerating request_id, which cannot succeed while the cache is full. Return -32000 SESSION_REFUSED with data.reason "replay_protection_capacity" instead: it is in the §17.1 registry, so every ACS client fails closed on it. startGuardian takes an optional replayProtectionStore so a server test can fill the cache. Reported by @shivang-sec on GenAI-Security-Project#229. Signed-off-by: Jason Crittenden <jrcrittenden@gmail.com>
The HMAC signing block in handleAcsRequest ran outside any try, on unauthenticated input. A session_id over 1024 bytes makes HKDF throw, and a deeply nested body overflows the stack in JCS canonicalization. Either escaped handleAcsRequest as an HTML 500, which a host reads as a hook that never fired. The block is now wrapped. A request that throws there cannot carry a valid signature, so its response goes out unsigned, like any other unauthenticated request's, and the failure is reported on stderr. The envelope log records the same response it did before, minus the signature it could never have had. Every AcsResult now carries metadata.evaluator "deterministic", so the trace exporter in GenAI-Security-Project#244 reads the evaluator from the wire instead of assuming it. Both raised by @rocklambros on GenAI-Security-Project#229. Signed-off-by: Jason Crittenden <jrcrittenden@gmail.com>
Request-signature verification canonicalises the unauthenticated body before computing the HMAC. A signed body nested past the stack made that throw, and the throw fell through dispatch to the outer net, which answers -32020. That code is outside the section 17.1 registry, so a client branching on registered codes could fail open on it. verifyHmacSha256 now treats an input it cannot canonicalise as an invalid signature, so the request gets -32004 SIGNATURE_INVALID like any other unauthenticated one. The signing-boundary test now asserts the answer for each of its two inputs. Raised by @astrogilda on GenAI-Security-Project#229. Signed-off-by: Jason Crittenden <jrcrittenden@gmail.com>
…he MCP method GenAI-Security-Project#244's wiring tests and the sample-trace generator posted steps with no handshake/hello, which GenAI-Security-Project#229's Guardian refuses with -32003. postRaw and the generator now send one first; the hello gets no span. protocols/MCP/tools/call is evaluated but otel-mapping.json maps only steps/* and agbom/* methods, so it is named in METHODS_WITHOUT_SPAN with that reason until the spec gives it a step_to_span entry. The decision event now carries acs.evaluator, and the response carries a chain_hash that covers each entry's recorded time. The two tests that compare a Guardian with export on and off give both the same clock, and the sample comparison sets the hash's value aside with the timestamps. The committed sample is regenerated. Signed-off-by: Jason Crittenden <jrcrittenden@gmail.com>
Signed-off-by: Jason Crittenden <jrcrittenden@gmail.com>
- Reject non-finite timestamp drift so a leap-second timestamp (23:59:60, which date-time accepts and Date.parse returns NaN for) no longer passes the §10.3 skew window or lands in the replay store with a NaN expiry. - Commit each ContextEntry to its request content via request_hash, the lowercase-hex SHA-256 of the JCS-canonicalized params (context-entry.json, RECOMMENDED). - README: mark the handshake as not yet enforced (-32003) and the hook set as partial (subagentStart and agentTrigger return -32011). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Jason Crittenden <jrcrittenden@gmail.com>
- Guardian: record each session's ClientHello methods_implemented and refuse with -32003 CAPABILITY_NOT_NEGOTIATED any step from a session that never sent handshake/hello, or for a method it did not declare (§4, §17.1). system/ping stays exempt. The check runs before the replay store remembers the request_id, so a retry after re-handshaking is not a false replay. The record is bounded (LRU, 4096 sessions). - Hosts: on -32003, governStep repeats the handshake once and retries with a fresh request, so a Guardian restart does not deny every step until the session ends. Only a decision replaces the refusal; a failed re-handshake or an undelivered retry keeps the -32003. - Conformance probes and tests now handshake before posting steps. - README: handshake row is implemented; replay protection names the remaining gap that the in-memory store does not survive a restart. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Jason Crittenden <jrcrittenden@gmail.com>
A full replay-protection cache answered a brand-new request with -32005 REPLAY_DETECTED. §17.1 recovers from -32005 by regenerating request_id, which cannot succeed while the cache is full. Return -32000 SESSION_REFUSED with data.reason "replay_protection_capacity" instead: it is in the §17.1 registry, so every ACS client fails closed on it. startGuardian takes an optional replayProtectionStore so a server test can fill the cache. Reported by @shivang-sec on GenAI-Security-Project#229. Signed-off-by: Jason Crittenden <jrcrittenden@gmail.com>
The HMAC signing block in handleAcsRequest ran outside any try, on unauthenticated input. A session_id over 1024 bytes makes HKDF throw, and a deeply nested body overflows the stack in JCS canonicalization. Either escaped handleAcsRequest as an HTML 500, which a host reads as a hook that never fired. The block is now wrapped. A request that throws there cannot carry a valid signature, so its response goes out unsigned, like any other unauthenticated request's, and the failure is reported on stderr. The envelope log records the same response it did before, minus the signature it could never have had. Every AcsResult now carries metadata.evaluator "deterministic", so the trace exporter in GenAI-Security-Project#244 reads the evaluator from the wire instead of assuming it. Both raised by @rocklambros on GenAI-Security-Project#229. Signed-off-by: Jason Crittenden <jrcrittenden@gmail.com>
Request-signature verification canonicalises the unauthenticated body before computing the HMAC. A signed body nested past the stack made that throw, and the throw fell through dispatch to the outer net, which answers -32020. That code is outside the section 17.1 registry, so a client branching on registered codes could fail open on it. verifyHmacSha256 now treats an input it cannot canonicalise as an invalid signature, so the request gets -32004 SIGNATURE_INVALID like any other unauthenticated one. The signing-boundary test now asserts the answer for each of its two inputs. Raised by @astrogilda on GenAI-Security-Project#229. Signed-off-by: Jason Crittenden <jrcrittenden@gmail.com>
With HMAC on, two host paths turned a deny into an allow. The client threw on any unverifiable response before reading its code, so a refusal the Guardian cannot sign (oversize body, -32700, a bad request signature) became an "unknown" failure and the proceed posture ran the step. A refusal-coded error with no result is now passed through unverified: it can only fail the step closed. The refusal codes move to failure-kinds.ts so the client can read them without an import cycle. A request too deep to canonicalise threw inside request building and the posture answered it. Signing is now its own "sign" stage whose failure, kind "unsignable", blocks regardless of posture. Signed-off-by: Jason Crittenden <jrcrittenden@gmail.com>
A protocols/MCP/tools/call payload carrying both tool/arguments and
params was evaluated on the former while a forwarding server runs the
latter. Both shapes together are now refused as envelope_invalid, and
{value} is unwrapped only in the ACS shape.
The stock rules skip a policy target that is not a string, so an object
or array command was allowed on any tool-call path. The snapshot
assembler now throws on one, which denies with evaluation_failed.
Signed-off-by: Jason Crittenden <jrcrittenden@gmail.com>
The host listed the refusal codes it knew, so a code added later, such as the retryable overload code Discussion GenAI-Security-Project#257 proposes, was classified error_without_decision and the proceed posture ran the step. The host now treats -32700 and the whole -32000..-32099 range as a refusal, both when it accepts an unsigned refusal and when it classifies one. Codes outside the range, like -32601 or -32603, still keep the posture. Signed-off-by: Jason Crittenden <jrcrittenden@gmail.com>
…he MCP method handshake/hello, which GenAI-Security-Project#229's Guardian refuses with -32003. postRaw and the generator now send one first; the hello gets no span. protocols/MCP/tools/call is evaluated but otel-mapping.json maps only steps/* and agbom/* methods, so it is named in METHODS_WITHOUT_SPAN with that reason until the spec gives it a step_to_span entry. The decision event now carries acs.evaluator, and the response carries a chain_hash that covers each entry's recorded time. The two tests that compare a Guardian with export on and off give both the same clock, and the sample comparison sets the hash's value aside with the timestamps. The committed sample is regenerated. Signed-off-by: Jason Crittenden <jrcrittenden@gmail.com>
1d28f6a to
d1f8838
Compare
…example The sample trace was regenerated under GenAI-Security-Project#229's Guardian, whose decision event now carries acs.evaluator and acs.chain_hash. The example on the page is checked against the sample, so it is copied again, and the sentence that lists the event's contents names both. Signed-off-by: Jason Crittenden <jrcrittenden@gmail.com>
|
@rocklambros rebased onto |
Next StepsNone of these blocks this PR. Each is a follow-up once it lands. 1. Stop the sample trace from changing on every regenerationThe sample's decision events now carry The follow-up needs:
2. Give
|
What changed
Implements the concrete D1-D10 wire probes from #184 across the AGT reference Guardian and the shipped Claude Code and OpenCode adapters. This adds authenticated request and response envelopes, replay and timestamp checks, published chain heads,
system/ping, handshake negotiation fields, the six-hook minimum, wrappedtools/call, explicittool_unregistereddecisions, adversarial coverage, and reproducible test instructions.This is intentionally a partial response. Follow-up work remains for replay persistence across Guardian restarts and chain continuity when the bounded session-context store evicts a session (#233).
Envelope log changes
This PR changes
packages/guardian/src/server.tsaround the envelope-log calls inhandleAcsRequest. TheenvelopeLog.write(...)calls are unchanged, but what they record changes:dispatchand the finalenvelopeLog.write("response", ...). WhenhmacSecretis set and the request is validly signed, the logged response carries itssignatureobject.-32003), a replay (-32005), and timestamp skew (-32006) are logged as error responses paired with their requests.handshake/hellobefore any step, so the log includes hello lines. A host that gets-32003re-handshakes once and resends, which adds a second hello and a second copy of the step.Which issue does this implement
Part of #184
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
I tested this
uv run pytest -vpasses on my machineuv run mkdocs build --strictpasses on my machineAdditional AGT checks:
bun run typecheckpasses.trashexecutable not being installed in this environment (bun: command not found: trash).Checklist
git commit -s(required by the DCO)Security
This change rejects unsigned, incorrectly signed, stale, and replayed envelopes; authenticates non-ping results and errors; and adds adversarial coverage for response substitution and replay-cache saturation. It responds to the already-public conformance report in #184 and does not disclose a separate unreported vulnerability.