From 660b055126e77625798d644ea325c224ba2e38fb Mon Sep 17 00:00:00 2001 From: Raghuvar Karthik Durga <177662415+drk-ram@users.noreply.github.com> Date: Thu, 29 Oct 2026 09:43:32 -0500 Subject: [PATCH] fix(guardian): enforce ACS version negotiation and ClientHello transport selection Validate 0.1.x ACS version compatibility on handshake/hello and step requests (returning -32001 UNSUPPORTED_VERSION when no compatible version exists) and inspect ClientHello transports_supported so selected_transport is never returned for an unadvertised transport (returning -32003 CAPABILITY_NOT_NEGOTIATED when http is excluded). Fixes #139 Fixes #165 Signed-off-by: Raghuvar Karthik Durga <177662415+drk-ram@users.noreply.github.com> --- .../agt/packages/guardian/src/handshake.ts | 87 +++++++++++++++++- .../agt/packages/guardian/src/server.ts | 34 ++++++- .../packages/guardian/test/handshake.test.ts | 91 ++++++++++++++++++- .../agt/packages/guardian/test/server.test.ts | 37 ++++++++ 4 files changed, 241 insertions(+), 8 deletions(-) diff --git a/reference-implementations/agt/packages/guardian/src/handshake.ts b/reference-implementations/agt/packages/guardian/src/handshake.ts index 96b50a87..c9a8a8af 100644 --- a/reference-implementations/agt/packages/guardian/src/handshake.ts +++ b/reference-implementations/agt/packages/guardian/src/handshake.ts @@ -36,8 +36,93 @@ export type ServerHello = { on_decision_failure: "proceed" | "deny"; }; +export const UNSUPPORTED_VERSION_CODE = -32001; +export const CAPABILITY_NOT_NEGOTIATED_CODE = -32003; + /** The ACS spec version every schema and mapping in this repo is pinned to. */ -const NEGOTIATED_VERSION = "0.1.0"; +export const NEGOTIATED_VERSION = "0.1.0"; +const SUPPORTED_TRANSPORT = "http" as const; + +function parseSemver(version: unknown): { major: number; minor: number; patch: number } | undefined { + if (typeof version !== "string") return undefined; + const match = /^(\d+)\.(\d+)\.(\d+)$/.exec(version); + if (!match) return undefined; + return { major: Number(match[1]), minor: Number(match[2]), patch: Number(match[3]) }; +} + +/** + * Under 0.x a breaking change is a minor bump, so a peer accepts the same + * minor with any patch and refuses a different minor; from 1.0 on, the rule is + * the same major (#196). Derived from the Guardian's own version rather than a + * hard-coded minor so a spec bump (e.g. 0.2.0) updates the check in one place. + */ +export function isSupportedAcsVersion(version: unknown, guardianVersion: string = NEGOTIATED_VERSION): version is string { + const candidate = parseSemver(version); + const own = parseSemver(guardianVersion); + if (candidate === undefined || own === undefined) return false; + return own.major === 0 + ? candidate.major === 0 && candidate.minor === own.minor + : candidate.major === own.major; +} + +export type HandshakeNegotiationResult = + | { ok: true; serverHello: ServerHello } + | { ok: false; code: number; message: string }; + +export function negotiateHandshake( + envelopeParams: { acs_version: string; payload?: Record }, + env?: { ACS_ON_DECISION_FAILURE?: string }, + guardianVersion: string = NEGOTIATED_VERSION, +): HandshakeNegotiationResult { + if (!isSupportedAcsVersion(envelopeParams.acs_version, guardianVersion)) { + return { + ok: false, + code: UNSUPPORTED_VERSION_CODE, + message: `UNSUPPORTED_VERSION: unsupported acs_version ${JSON.stringify(envelopeParams.acs_version)}`, + }; + } + const payload = envelopeParams.payload ?? {}; + if ("acs_version" in payload && !isSupportedAcsVersion(payload.acs_version, guardianVersion)) { + return { + ok: false, + code: UNSUPPORTED_VERSION_CODE, + message: `UNSUPPORTED_VERSION: unsupported client acs_version ${JSON.stringify(payload.acs_version)}`, + }; + } + let selectedVersion = + typeof payload.acs_version === "string" ? payload.acs_version : envelopeParams.acs_version; + if ("acs_versions_supported" in payload) { + const versions = payload.acs_versions_supported; + const match = Array.isArray(versions) + ? versions.find((v): v is string => isSupportedAcsVersion(v, guardianVersion)) + : undefined; + if (match === undefined) { + return { + ok: false, + code: UNSUPPORTED_VERSION_CODE, + message: `UNSUPPORTED_VERSION: no compatible version in acs_versions_supported ${JSON.stringify(versions)}`, + }; + } + selectedVersion = match; + } + if ("transports_supported" in payload) { + const transports = payload.transports_supported; + if (!Array.isArray(transports) || !transports.includes(SUPPORTED_TRANSPORT)) { + return { + ok: false, + code: CAPABILITY_NOT_NEGOTIATED_CODE, + message: `CAPABILITY_NOT_NEGOTIATED: Guardian supports transport "${SUPPORTED_TRANSPORT}", client advertised ${JSON.stringify(transports)}`, + }; + } + } + return { + ok: true, + serverHello: { + ...buildServerHello(env), + negotiated_version: selectedVersion, + }, + }; +} /** * The methods this Guardian actually dispatches -- exactly the two gated diff --git a/reference-implementations/agt/packages/guardian/src/server.ts b/reference-implementations/agt/packages/guardian/src/server.ts index 8a38eed4..0cf5570d 100644 --- a/reference-implementations/agt/packages/guardian/src/server.ts +++ b/reference-implementations/agt/packages/guardian/src/server.ts @@ -93,7 +93,13 @@ import { type ToolCallResultEnvelope, } from "./validate-envelope.ts"; import { checkResponse } from "./check-response.ts"; -import { buildServerHello, type ServerHello } from "./handshake.ts"; +import { + buildServerHello, + NEGOTIATED_VERSION, + negotiateHandshake, + UNSUPPORTED_VERSION_CODE, + type ServerHello, +} from "./handshake.ts"; import { createEnvelopeLogSink, NULL_ENVELOPE_LOG_SINK, type EnvelopeLogSink } from "./envelope-log-sink.ts"; import { createOtelTraceExporter, NULL_OTEL_TRACE_EXPORTER, type OtelTraceExporter } from "./otel-trace-exporter.ts"; import type { RejectionReason } from "./otel-mapping.ts"; @@ -459,6 +465,8 @@ export async function startGuardian({ }) : NULL_OTEL_TRACE_EXPORTER); + const negotiatedVersions = new Map(); + const server = Bun.serve({ hostname: hostname ?? LOOPBACK_ONLY, port, @@ -467,7 +475,7 @@ export async function startGuardian({ if (req.method !== "POST" || pathname !== ACS_PATH) { return new Response("Not Found", { status: 404 }); } - const response = await handleAcsRequest(req, bridge, mapping, envelopeLog, onDecisionFailure, sessionContextStore, traceExporter); + const response = await handleAcsRequest(req, bridge, mapping, envelopeLog, onDecisionFailure, sessionContextStore, traceExporter, negotiatedVersions); return Response.json(response); }, }); @@ -537,6 +545,7 @@ async function handleAcsRequest( onDecisionFailure: "proceed" | "deny" | undefined, sessionContextStore: SessionContextStore, traceExporter: OtelTraceExporter, + negotiatedVersions: Map, ): Promise { const body = await readCappedBody(req); if (body.withinLimit === false) { @@ -595,7 +604,7 @@ async function handleAcsRequest( let response: JsonRpcSuccess | JsonRpcFailure; let rejection: RejectionReason | undefined; try { - ({ response, rejection } = await dispatch(raw, validation, bridge, mapping, onDecisionFailure, sessionContextStore, envelopeLog)); + ({ response, rejection } = await dispatch(raw, validation, bridge, mapping, onDecisionFailure, sessionContextStore, envelopeLog, negotiatedVersions)); } catch (error) { // The outer net. Deliberately a bare JSON-RPC error, not an ACS `deny`: // this route is `dispatch` rethrowing entirely past denyOnInvalidEnvelope @@ -873,6 +882,7 @@ async function dispatch( onDecisionFailure: "proceed" | "deny" | undefined, sessionContextStore: SessionContextStore, envelopeLog: EnvelopeLogSink, + negotiatedVersions: Map, ): Promise { const rpcId = extractId(raw); @@ -901,7 +911,23 @@ async function dispatch( if (envelope.method === HANDSHAKE_METHOD) { const env = onDecisionFailure === undefined ? undefined : { ACS_ON_DECISION_FAILURE: onDecisionFailure }; - return { response: successResponse(envelope.id, buildServerHello(env)) }; + const negotiated = negotiateHandshake(envelope.params, env); + if (!negotiated.ok) { + return { response: errorResponse(envelope.id, negotiated.code, negotiated.message) }; + } + negotiatedVersions.set(envelope.params.metadata.session_id, negotiated.serverHello.negotiated_version); + return { response: successResponse(envelope.id, negotiated.serverHello) }; + } + + const expectedVersion = negotiatedVersions.get(envelope.params.metadata.session_id) ?? NEGOTIATED_VERSION; + if (envelope.params.acs_version !== expectedVersion) { + return { + response: errorResponse( + envelope.id, + UNSUPPORTED_VERSION_CODE, + `UNSUPPORTED_VERSION: step acs_version ${JSON.stringify(envelope.params.acs_version)} does not match negotiated_version ${JSON.stringify(expectedVersion)}`, + ), + }; } // Two gated branches, one per ACS method this Guardian assembles a snapshot diff --git a/reference-implementations/agt/packages/guardian/test/handshake.test.ts b/reference-implementations/agt/packages/guardian/test/handshake.test.ts index f20173e6..c3938533 100644 --- a/reference-implementations/agt/packages/guardian/test/handshake.test.ts +++ b/reference-implementations/agt/packages/guardian/test/handshake.test.ts @@ -1,8 +1,14 @@ import { describe, expect, it } from "bun:test"; import { fileURLToPath } from "node:url"; -import { buildServerHello } from "../src/handshake.ts"; +import { + buildServerHello, + CAPABILITY_NOT_NEGOTIATED_CODE, + isSupportedAcsVersion, + negotiateHandshake, + UNSUPPORTED_VERSION_CODE, +} from "../src/handshake.ts"; -const MODULE = fileURLToPath(new URL("../src/handshake.ts", import.meta.url)); +const MODULE = new URL("../src/handshake.ts", import.meta.url).href; /** * Calls `buildServerHello()` with NO argument at all -- the shipped call @@ -26,7 +32,7 @@ async function respondInSubprocess( env.ACS_ON_DECISION_FAILURE = posture; } const proc = Bun.spawn( - ["bun", "-e", `import {buildServerHello} from ${JSON.stringify(MODULE)};` + + [process.execPath, "-e", `import {buildServerHello} from ${JSON.stringify(MODULE)};` + "process.stdout.write(JSON.stringify(buildServerHello()));"], { env: env as Record, stdout: "pipe", stderr: "pipe" }, ); @@ -110,3 +116,82 @@ describe("buildServerHello — the zero-argument path the Guardian actually call expect(stderr).toContain("dney"); }); }); + +describe("negotiateHandshake — version compatibility (#139, #196) and transport selection (#165)", () => { + it("negotiates the compatible version from the client's acs_versions_supported list (e.g. 0.1.1 -> 0.1.1)", () => { + const patchNegotiated = negotiateHandshake({ + acs_version: "0.1.1", + payload: { acs_versions_supported: ["0.1.1"], transports_supported: ["http"] }, + }); + expect(patchNegotiated.ok).toBe(true); + if (patchNegotiated.ok) { + expect(patchNegotiated.serverHello.negotiated_version).toBe("0.1.1"); + } + + const fallbackList = negotiateHandshake({ + acs_version: "0.1.4", + payload: { acs_versions_supported: ["0.2.0", "0.1.4"], transports_supported: ["http"] }, + }); + expect(fallbackList.ok).toBe(true); + if (fallbackList.ok) { + expect(fallbackList.serverHello.negotiated_version).toBe("0.1.4"); + } + }); + + it("derives the accepted minor from the Guardian's own version (0.1.x vs 0.2.x vs 1.x)", () => { + expect(isSupportedAcsVersion("0.1.0", "0.1.0")).toBe(true); + expect(isSupportedAcsVersion("0.1.4", "0.1.0")).toBe(true); + expect(isSupportedAcsVersion("0.2.0", "0.1.0")).toBe(false); + expect(isSupportedAcsVersion("0.2.3", "0.2.0")).toBe(true); + expect(isSupportedAcsVersion("0.1.4", "0.2.0")).toBe(false); + expect(isSupportedAcsVersion("1.2.0", "1.0.0")).toBe(true); + expect(isSupportedAcsVersion("2.0.0", "1.0.0")).toBe(false); + }); + + it("rejects 0.2.0 or 1.0.0 with -32001 UNSUPPORTED_VERSION (#139, #196)", () => { + for (const badVersion of ["0.2.0", "1.0.0"]) { + expect(negotiateHandshake({ acs_version: badVersion, payload: {} })).toEqual({ + ok: false, + code: UNSUPPORTED_VERSION_CODE, + message: expect.stringContaining("UNSUPPORTED_VERSION"), + }); + } + expect( + negotiateHandshake({ + acs_version: "0.1.0", + payload: { acs_versions_supported: ["0.2.0", "1.0.0", "2.0.0"] }, + }), + ).toEqual({ + ok: false, + code: UNSUPPORTED_VERSION_CODE, + message: expect.stringContaining("UNSUPPORTED_VERSION"), + }); + }); + + it("rejects ClientHello transports_supported that exclude http with -32003 CAPABILITY_NOT_NEGOTIATED", () => { + for (const transports of [["stdio"], ["https"], ["stdio", "https"]]) { + expect( + negotiateHandshake({ + acs_version: "0.1.0", + payload: { transports_supported: transports }, + }), + ).toEqual({ + ok: false, + code: CAPABILITY_NOT_NEGOTIATED_CODE, + message: expect.stringContaining("CAPABILITY_NOT_NEGOTIATED"), + }); + } + }); + + it("selects http when ClientHello advertises http in transports_supported", () => { + const res = negotiateHandshake({ + acs_version: "0.1.0", + payload: { acs_versions_supported: ["0.1.0"], transports_supported: ["stdio", "http"] }, + }); + expect(res.ok).toBe(true); + if (res.ok) { + expect(res.serverHello.selected_transport).toBe("http"); + expect(res.serverHello.negotiated_version).toBe("0.1.0"); + } + }); +}); diff --git a/reference-implementations/agt/packages/guardian/test/server.test.ts b/reference-implementations/agt/packages/guardian/test/server.test.ts index 60f0be35..02018b4e 100644 --- a/reference-implementations/agt/packages/guardian/test/server.test.ts +++ b/reference-implementations/agt/packages/guardian/test/server.test.ts @@ -196,6 +196,43 @@ describe("startGuardian POST /acs", () => { expect(serverHello.on_decision_failure).toBe("proceed"); }); + it("negotiates the client's offered 0.1.x version per session, requires every step's acs_version to match negotiated_version (#139, #196), and rejects unsupported transports with -32003 (#165)", async () => { + const minorMismatchHello = await postAcs(url, makeEnvelope("handshake/hello", { acs_versions_supported: ["0.2.0"] }, { id: 42 })); + expect(minorMismatchHello.result).toBeUndefined(); + expect(minorMismatchHello.error?.code).toBe(-32001); + + const badHello = await postAcs(url, makeEnvelope("handshake/hello", { acs_versions_supported: ["1.0.0"] }, { id: 43 })); + expect(badHello.result).toBeUndefined(); + expect(badHello.error?.code).toBe(-32001); + + // Session that offers only 0.1.1 negotiates 0.1.1; steps on that session must carry 0.1.1 + const sessionId = crypto.randomUUID(); + const hello011 = makeEnvelope("handshake/hello", { acs_versions_supported: ["0.1.1"], transports_supported: ["http"] }, { id: 44, sessionId }); + (hello011.params as Record).acs_version = "0.1.1"; + const hello011Res = await postAcs(url, hello011); + expect(hello011Res.error).toBeUndefined(); + expect((hello011Res.result as Record)?.negotiated_version).toBe("0.1.1"); + + const matchingStep = toolCallEnvelope("ls -la", { id: 45, sessionId }); + (matchingStep.params as Record).acs_version = "0.1.1"; + const matchingStepRes = await postAcs(url, matchingStep); + expect(matchingStepRes.error).toBeUndefined(); + expect((matchingStepRes.result as Record)?.decision).toBe("allow"); + + // A step on the 0.1.1 session carrying 0.1.0 or 0.1.4 is refused with -32001 + for (const wrongStepVersion of ["0.1.0", "0.1.4", "0.2.0"]) { + const wrongStep = toolCallEnvelope("ls -la", { id: 46, sessionId }); + (wrongStep.params as Record).acs_version = wrongStepVersion; + const wrongStepRes = await postAcs(url, wrongStep); + expect(wrongStepRes.result).toBeUndefined(); + expect(wrongStepRes.error?.code).toBe(-32001); + } + + const stdioOnly = await postAcs(url, makeEnvelope("handshake/hello", { transports_supported: ["stdio"] }, { id: 47 })); + expect(stdioOnly.result).toBeUndefined(); + expect(stdioOnly.error?.code).toBe(-32003); + }); + it("steps/toolCallRequest carrying rm -rf / denies, with non-empty reasoning and reason_codes, echoing request_id", async () => { const requestId = crypto.randomUUID(); const response = await postAcs(url, toolCallEnvelope("rm -rf /", { requestId }));