From ad88cd23aafb781e28dcbb72961322235dd09b3b Mon Sep 17 00:00:00 2001 From: rocklambros Date: Fri, 16 Oct 2026 11:04:52 -0600 Subject: [PATCH 01/10] Read the proxy environment in main.ts, not in the trace exporter keepLoopbackOffProxy and loopbackProxyWarning now take the environment as a parameter, and createOtelTraceExporter calls neither. main.ts prints the warning and applies the exemption where it reads ACS_OTEL_ENDPOINT, before the Guardian starts, so the exporter no longer reads or writes process.env and no longer prints outside onError. The in-process routing test passed only when an earlier test had left 127.0.0.1 in NO_PROXY. It now runs in a child process, like the existing routing test, and is gated to Bun 1.3.14 and later. Refs #274 Signed-off-by: rocklambros --- .../agt/packages/guardian/README.md | 2 + .../agt/packages/guardian/src/main.ts | 18 +++ .../guardian/src/otel-trace-exporter.ts | 35 +++-- .../agt/packages/guardian/test/main.test.ts | 87 +++++++++++++ .../guardian/test/otel-trace-exporter.test.ts | 122 ++++++++++++------ 5 files changed, 209 insertions(+), 55 deletions(-) create mode 100644 reference-implementations/agt/packages/guardian/test/main.test.ts diff --git a/reference-implementations/agt/packages/guardian/README.md b/reference-implementations/agt/packages/guardian/README.md index 3a59cebe..7c200b74 100644 --- a/reference-implementations/agt/packages/guardian/README.md +++ b/reference-implementations/agt/packages/guardian/README.md @@ -149,6 +149,8 @@ await guardian.close(); `StartGuardianOptions` also accepts `hostname`, `envelopeLogPath`, `sessionContextLog`, `otelTraceFile`, `otelTraceEndpoint` and `onDecisionFailure`, which mirror the environment variables above, and five options for tests: `mappingPath`, `annotator`, `bridge`, `sessionContextStore` and `traceExporter`. Each is documented where it is declared, in `src/server.ts`. +`otelTraceEndpoint` leaves the proxy variables alone. `src/main.ts` adds a loopback endpoint to `NO_PROXY` and `no_proxy` and prints the proxy warning. An embedder that wants the same calls `loopbackProxyWarning(endpoint, process.env)` and `keepLoopbackOffProxy(endpoint, process.env)` from `src/otel-trace-exporter.ts` before it starts the Guardian. + The package has two entry points. `guardian` exports the governance verbs listed in `src/index.ts`. `guardian/deployment` exports `createDeploymentBridge`, the one function that builds the deployment's bridge with the egress annotator wired in. The conformance harness imports the second so it measures the bridge the Guardian ships, not a replica. ## Files diff --git a/reference-implementations/agt/packages/guardian/src/main.ts b/reference-implementations/agt/packages/guardian/src/main.ts index 08cea99a..736df9ca 100644 --- a/reference-implementations/agt/packages/guardian/src/main.ts +++ b/reference-implementations/agt/packages/guardian/src/main.ts @@ -19,6 +19,7 @@ * surface and server.ts reads none itself. */ import { buildServerHello } from "./handshake.ts"; +import { keepLoopbackOffProxy, loopbackProxyWarning } from "./otel-trace-exporter.ts"; import { startGuardian } from "./server.ts"; const DEFAULT_PORT = 8787; @@ -35,6 +36,23 @@ const sessionContextLog = process.env.ACS_SESSION_CONTEXT_LOG ?? DEFAULT_SESSION // passed in, like ACS_ENVELOPE_LOG: server.ts reads no env itself. const otelTraceFile = process.env.ACS_OTEL_FILE; const otelTraceEndpoint = process.env.ACS_OTEL_ENDPOINT; +// The proxy variables are environment too, so they are read and edited here +// and never by the exporter. The warning judges them as the operator set +// them, before the edit: Bun 1.3.11 ignores a runtime edit, and there the +// warning is all the operator gets. An endpoint that is not a URL is left to +// the exporter, which reports it and disables that output without stopping +// the Guardian. +if (otelTraceEndpoint !== undefined) { + try { + const warning = loopbackProxyWarning(otelTraceEndpoint, process.env); + if (warning !== undefined) { + console.error(warning); + } + keepLoopbackOffProxy(otelTraceEndpoint, process.env); + } catch { + // Not a URL. createOtelTraceExporter reports it below. + } +} // Read and validate the posture BEFORE starting the server. Not the audit // sink's path: that file is the host's, written by the hook, not by diff --git a/reference-implementations/agt/packages/guardian/src/otel-trace-exporter.ts b/reference-implementations/agt/packages/guardian/src/otel-trace-exporter.ts index e315db71..d6fd18f9 100644 --- a/reference-implementations/agt/packages/guardian/src/otel-trace-exporter.ts +++ b/reference-implementations/agt/packages/guardian/src/otel-trace-exporter.ts @@ -613,24 +613,26 @@ const splitProxyList = (value: string): string[] => * version this edit has no effect, and loopbackProxyWarning below is what * tells the operator. * - * The change is process-wide and permanent: it is written to process.env and - * never undone, so anything else in this process (a host adapter embedded - * with the Guardian) and any child process it spawns inherits it. That is - * acceptable because only loopback hosts are ever added, and traffic to this - * machine should not use a proxy for any of them. + * The edit is made to the `env` it is given and never undone. main.ts + * passes process.env, so there the change is process-wide and permanent: + * anything else in that process (a host adapter embedded with the Guardian) + * and any child process it spawns inherits it. That is acceptable because + * only loopback hosts are ever added, and traffic to this machine should not + * use a proxy for any of them. The exporter never calls this itself, so an + * embedder that builds the exporter decides whether its environment changes. */ -export function keepLoopbackOffProxy(endpoint: string): void { +export function keepLoopbackOffProxy(endpoint: string, env: Record): void { const additions = loopbackProxyEntries(endpoint); if (additions.length === 0) { return; } const keys = ["NO_PROXY", "no_proxy"] as const; - const setKeys = keys.filter((key) => (process.env[key] ?? "") !== ""); + const setKeys = keys.filter((key) => (env[key] ?? "") !== ""); for (const key of setKeys.length === 0 ? keys : setKeys) { - const entries = splitProxyList(process.env[key] ?? ""); + const entries = splitProxyList(env[key] ?? ""); const missing = additions.filter((entry) => !entries.includes(entry)); if (missing.length > 0) { - process.env[key] = [...entries, ...missing].join(","); + env[key] = [...entries, ...missing].join(","); } } } @@ -638,12 +640,12 @@ export function keepLoopbackOffProxy(endpoint: string): void { /** * A warning when a loopback endpoint would reach an HTTP proxy, judged by * the variables as they stand: a proxy is set, and the list Bun reads (a - * non-empty no_proxy, otherwise NO_PROXY) does not exempt the host. Called - * before keepLoopbackOffProxy, so on Bun 1.3.11, which ignores the runtime - * edit, the operator still learns that spans may go through the proxy and - * what to set before starting the Guardian. + * non-empty no_proxy, otherwise NO_PROXY) does not exempt the host. main.ts + * calls it before keepLoopbackOffProxy, so on Bun 1.3.11, which ignores the + * runtime edit, the operator still learns that spans may go through the + * proxy and what to set before starting the Guardian. */ -export function loopbackProxyWarning(endpoint: string, env: Record = process.env): string | undefined { +export function loopbackProxyWarning(endpoint: string, env: Record): string | undefined { const additions = loopbackProxyEntries(endpoint); const proxy = env.http_proxy || env.HTTP_PROXY; if (additions.length === 0 || !proxy) { @@ -793,11 +795,6 @@ export function createOtelTraceExporter({ // the SDK under any set here and cannot be refused, so the README lists // them as honored. construct(endpoint, () => { - const warning = loopbackProxyWarning(endpoint); - if (warning !== undefined) { - console.error(warning); - } - keepLoopbackOffProxy(endpoint); return new OTLPTraceExporter({ url: endpoint, timeoutMillis: ENDPOINT_TIMEOUT_MS, compression: "none" as NonNullable[0]>["compression"] }); }); } diff --git a/reference-implementations/agt/packages/guardian/test/main.test.ts b/reference-implementations/agt/packages/guardian/test/main.test.ts new file mode 100644 index 00000000..cdc87916 --- /dev/null +++ b/reference-implementations/agt/packages/guardian/test/main.test.ts @@ -0,0 +1,87 @@ +/** + * main.ts is the composition root, so it is the one place that reads the + * proxy variables and applies the loopback exemption (Ariel's review on + * PR #286). These tests run it as the operator does, as a process, because + * the wiring under test is the entrypoint itself. + */ +import { afterEach, describe, expect, it } from "bun:test"; +import { existsSync, mkdtempSync, rmdirSync } from "node:fs"; +import { createServer } from "node:net"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const GUARDIAN = fileURLToPath(new URL("../src/main.ts", import.meta.url)); +/** Absolute, because the process runs from a scratch cwd rather than from + * the repo root, the same reason test/readme-captures.test.ts gives. */ +const MANIFEST = fileURLToPath(new URL("../../../policy/manifest.yaml", import.meta.url)); + +const dirs: string[] = []; +function scratch(): string { + const dir = mkdtempSync(join(tmpdir(), "acs-main-")); + dirs.push(dir); + return dir; +} + +/** The Guardian mkdirs `.acs/` at startup and writes nothing into it until + * an envelope arrives. Nothing posts here, so an empty `.acs/` is the only + * thing a run can leave, and rmdirSync fails loudly on anything else. */ +afterEach(() => { + for (const dir of dirs.splice(0)) { + const acs = join(dir, ".acs"); + if (existsSync(acs)) { + rmdirSync(acs); + } + rmdirSync(dir); + } +}); + +/** A port nothing listens on once this returns. No span is sent in these + * tests, so the endpoint only has to be a well-formed loopback URL. */ +async function freePort(): Promise { + const server = createServer(); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const { port } = server.address() as { port: number }; + await new Promise((resolve) => server.close(() => resolve())); + return port; +} + +describe("main.ts applies the loopback proxy exemption", () => { + it("warns on stderr at startup when a proxy is set and loopback is not exempt", async () => { + const endpoint = `http://127.0.0.1:${await freePort()}/v1/traces`; + // process.execPath, not "bun" off PATH, so the entrypoint runs on the + // same Bun version as the suite. The environment is built up rather than + // inherited, so a NO_PROXY in the developer's shell cannot hide the + // warning. `timeout` is the runtime's own watchdog, shorter than bun + // test's five-second cap, so a missing warning fails on the assertion. + const proc = Bun.spawn([process.execPath, GUARDIAN], { + cwd: scratch(), + env: { + PATH: process.env.PATH ?? "", + HOME: process.env.HOME ?? "", + NO_COLOR: "1", + ACS_GUARDIAN_PORT: "0", + ACS_MANIFEST_PATH: MANIFEST, + ACS_OTEL_ENDPOINT: endpoint, + HTTP_PROXY: "http://127.0.0.1:9/", + }, + stdout: "ignore", + stderr: "pipe", + timeout: 3_000, + }); + const decoder = new TextDecoder(); + let stderr = ""; + for await (const chunk of proc.stderr) { + stderr += decoder.decode(chunk, { stream: true }); + if (stderr.includes("did not exempt 127.0.0.1")) { + proc.kill(); + } + } + proc.kill(); + await proc.exited; + expect(stderr).toContain( + `otel trace export to ${endpoint}: an HTTP proxy is set and NO_PROXY/no_proxy did not exempt 127.0.0.1, ` + + "so on some Bun versions these spans go through the proxy. Start the Guardian with 127.0.0.1 in both NO_PROXY and no_proxy.", + ); + }); +}); diff --git a/reference-implementations/agt/packages/guardian/test/otel-trace-exporter.test.ts b/reference-implementations/agt/packages/guardian/test/otel-trace-exporter.test.ts index 2c5f05b6..9a6995ba 100644 --- a/reference-implementations/agt/packages/guardian/test/otel-trace-exporter.test.ts +++ b/reference-implementations/agt/packages/guardian/test/otel-trace-exporter.test.ts @@ -1378,28 +1378,78 @@ describe("the Guardian's exporter does not depend on its environment", () => { } }, 10_000); - it("sends to a loopback endpoint directly when HTTP_PROXY is set", async () => { - // A proxy that only counts connections and never answers. - let proxyConnections = 0; - const proxy = createServer((socket) => { - proxyConnections += 1; - socket.destroy(); - }); - await new Promise((resolve) => proxy.listen(0, "127.0.0.1", resolve)); - const proxyUrl = `http://127.0.0.1:${(proxy.address() as { port: number }).port}`; - setEnv({ HTTP_PROXY: proxyUrl, http_proxy: proxyUrl }); + // The exemption is main.ts's to apply (Ariel's review on PR #286), so this + // test applies it the way main.ts does, before the exporter is built. It + // runs in a fresh child for the reason the routing test further down + // gives: Bun latches proxy variables across edits within a process. In + // process, this test passed only when an earlier test had already put + // 127.0.0.1 in NO_PROXY, and failed when run alone. Gated to Bun 1.3.14 + // and later, which honor a runtime edit; 1.3.11 reads the variables only + // at startup, so main.ts's warning is what an operator gets there. + it.skipIf(!Bun.semver.satisfies(Bun.version, ">=1.3.14"))( + "sends to a loopback endpoint directly when HTTP_PROXY is set and the exemption is applied", + async () => { + // A proxy that only counts connections and never answers. + let proxyConnections = 0; + const proxy = createServer((socket) => { + proxyConnections += 1; + socket.destroy(); + }); + await new Promise((resolve) => proxy.listen(0, "127.0.0.1", resolve)); + const proxyUrl = `http://127.0.0.1:${(proxy.address() as { port: number }).port}`; + const source = new URL("../src/otel-trace-exporter.ts", import.meta.url).pathname; + const script = ` + const { createOtelTraceExporter, keepLoopbackOffProxy } = await import(${JSON.stringify(source)}); + let collected = 0; + const collector = Bun.serve({ + port: 0, + hostname: "127.0.0.1", + fetch() { + collected += 1; + return new Response("{}", { headers: { "content-type": "application/json" } }); + }, + }); + const endpoint = "http://127.0.0.1:" + collector.port + "/v1/traces"; + keepLoopbackOffProxy(endpoint, process.env); + const exporter = createOtelTraceExporter({ endpoint, methodsEvaluated: ["steps/toolCallRequest"], onError: () => {} }); + exporter.end(exporter.start(${JSON.stringify(envelope(1))}, "steps/toolCallRequest"), ${JSON.stringify(allow(1))}); + await exporter.shutdown(); + console.log(JSON.stringify({ collected })); + collector.stop(true); + process.exit(0); + `; + try { + // Async spawn, so the proxy listener on this event loop can count + // the child's connections. + const child = Bun.spawn([process.execPath, "-e", script], { + env: { PATH: process.env.PATH ?? "", HOME: process.env.HOME ?? "", HTTP_PROXY: proxyUrl, http_proxy: proxyUrl }, + stdout: "pipe", + }); + const output = await new Response(child.stdout).text(); + await child.exited; + const result = JSON.parse(output.trim().split("\n").pop()!); + expect({ proxyConnections, collected: result.collected }).toEqual({ proxyConnections: 0, collected: 1 }); + } finally { + proxy.close(); + } + }, + 10_000, + ); + + // Ariel's review on PR #286: the composition root owns the environment. + // main.ts applies the loopback exemption, so an embedder that builds the + // exporter itself decides whether its own environment changes. + it("leaves NO_PROXY and no_proxy as they were when built for a loopback endpoint", async () => { unsetEnv("NO_PROXY", "no_proxy"); - const collector = startCollector(); - try { - const exporter = createOtelTraceExporter({ endpoint: collector.url, methodsEvaluated: METHODS_EVALUATED }); - exporter.end(exporter.start(envelope(1), "steps/toolCallRequest"), allow(1)); - await exporter.shutdown(); - expect({ proxyConnections, collected: collector.received.length }).toEqual({ proxyConnections: 0, collected: 1 }); - } finally { - collector.server.stop(true); - proxy.close(); - } - }, 10_000); + const unset = createOtelTraceExporter({ endpoint: "http://127.0.0.1:4318/v1/traces", methodsEvaluated: METHODS_EVALUATED, onError: () => {} }); + await unset.shutdown(); + expect({ NO_PROXY: "NO_PROXY" in process.env, no_proxy: "no_proxy" in process.env }).toEqual({ NO_PROXY: false, no_proxy: false }); + + setEnv({ NO_PROXY: "corp.internal", no_proxy: "corp.internal" }); + const set = createOtelTraceExporter({ endpoint: "http://127.0.0.1:4318/v1/traces", methodsEvaluated: METHODS_EVALUATED, onError: () => {} }); + await set.shutdown(); + expect([process.env.NO_PROXY, process.env.no_proxy]).toEqual(["corp.internal", "corp.internal"]); + }); }); describe("keepLoopbackOffProxy", () => { @@ -1422,13 +1472,13 @@ describe("keepLoopbackOffProxy", () => { it("adds localhost for a localhost endpoint", () => { reset(undefined); - keepLoopbackOffProxy("http://localhost:4318/v1/traces"); + keepLoopbackOffProxy("http://localhost:4318/v1/traces", process.env); expect([process.env.NO_PROXY, process.env.no_proxy]).toEqual(["localhost", "localhost"]); }); it("adds ::1 both bare and bracketed for an IPv6 loopback endpoint, because Bun matches only [::1]", () => { reset(undefined); - keepLoopbackOffProxy("http://[::1]:4318/v1/traces"); + keepLoopbackOffProxy("http://[::1]:4318/v1/traces", process.env); expect([process.env.NO_PROXY, process.env.no_proxy]).toEqual(["::1,[::1]", "::1,[::1]"]); }); @@ -1438,7 +1488,7 @@ describe("keepLoopbackOffProxy", () => { it("appends to NO_PROXY alone when only NO_PROXY is set, and does not create no_proxy", () => { reset(undefined); process.env.NO_PROXY = "corp.internal"; - keepLoopbackOffProxy("http://127.0.0.1:4318/v1/traces"); + keepLoopbackOffProxy("http://127.0.0.1:4318/v1/traces", process.env); expect(process.env.NO_PROXY).toBe("corp.internal,127.0.0.1"); expect("no_proxy" in process.env).toBe(false); }); @@ -1446,7 +1496,7 @@ describe("keepLoopbackOffProxy", () => { it("appends to no_proxy alone when only no_proxy is set, and does not create NO_PROXY", () => { reset(undefined); process.env.no_proxy = "corp.internal"; - keepLoopbackOffProxy("http://127.0.0.1:4318/v1/traces"); + keepLoopbackOffProxy("http://127.0.0.1:4318/v1/traces", process.env); expect(process.env.no_proxy).toBe("corp.internal,127.0.0.1"); expect("NO_PROXY" in process.env).toBe(false); }); @@ -1455,7 +1505,7 @@ describe("keepLoopbackOffProxy", () => { reset(undefined); process.env.NO_PROXY = "corp.internal"; process.env.no_proxy = ""; - keepLoopbackOffProxy("http://127.0.0.1:4318/v1/traces"); + keepLoopbackOffProxy("http://127.0.0.1:4318/v1/traces", process.env); expect([process.env.NO_PROXY, process.env.no_proxy]).toEqual(["corp.internal,127.0.0.1", ""]); }); @@ -1464,7 +1514,7 @@ describe("keepLoopbackOffProxy", () => { reset(undefined); process.env.NO_PROXY = "corp.internal"; process.env.no_proxy = blank; - keepLoopbackOffProxy("http://127.0.0.1:4318/v1/traces"); + keepLoopbackOffProxy("http://127.0.0.1:4318/v1/traces", process.env); expect({ blank, NO_PROXY: process.env.NO_PROXY, no_proxy: process.env.no_proxy }).toEqual({ blank, NO_PROXY: "corp.internal,127.0.0.1", @@ -1475,35 +1525,35 @@ describe("keepLoopbackOffProxy", () => { it("sets both when neither is set, and appends once to each when both are set", () => { reset(undefined); - keepLoopbackOffProxy("http://127.0.0.1:4318/v1/traces"); + keepLoopbackOffProxy("http://127.0.0.1:4318/v1/traces", process.env); expect([process.env.NO_PROXY, process.env.no_proxy]).toEqual(["127.0.0.1", "127.0.0.1"]); process.env.NO_PROXY = "a.internal"; process.env.no_proxy = "b.internal"; - keepLoopbackOffProxy("http://127.0.0.1:4318/v1/traces"); - keepLoopbackOffProxy("http://127.0.0.1:4318/v1/traces"); + keepLoopbackOffProxy("http://127.0.0.1:4318/v1/traces", process.env); + keepLoopbackOffProxy("http://127.0.0.1:4318/v1/traces", process.env); expect([process.env.NO_PROXY, process.env.no_proxy]).toEqual(["a.internal,127.0.0.1", "b.internal,127.0.0.1"]); }); it("does not duplicate the entry when called twice", () => { reset(undefined); - keepLoopbackOffProxy("http://127.0.0.1:4318/v1/traces"); - keepLoopbackOffProxy("http://127.0.0.1:4318/v1/traces"); + keepLoopbackOffProxy("http://127.0.0.1:4318/v1/traces", process.env); + keepLoopbackOffProxy("http://127.0.0.1:4318/v1/traces", process.env); expect([process.env.NO_PROXY, process.env.no_proxy]).toEqual(["127.0.0.1", "127.0.0.1"]); }); it("preserves existing entries", () => { reset("example.internal"); - keepLoopbackOffProxy("http://localhost:4318/v1/traces"); + keepLoopbackOffProxy("http://localhost:4318/v1/traces", process.env); expect([process.env.NO_PROXY, process.env.no_proxy]).toEqual(["example.internal,localhost", "example.internal,localhost"]); }); it("leaves both variables exactly as they were for a non-loopback endpoint", () => { reset(undefined); - keepLoopbackOffProxy("http://collector.example.com:4318/v1/traces"); + keepLoopbackOffProxy("http://collector.example.com:4318/v1/traces", process.env); expect("NO_PROXY" in process.env).toBe(false); expect("no_proxy" in process.env).toBe(false); reset("example.internal"); - keepLoopbackOffProxy("http://collector.example.com:4318/v1/traces"); + keepLoopbackOffProxy("http://collector.example.com:4318/v1/traces", process.env); expect([process.env.NO_PROXY, process.env.no_proxy]).toEqual(["example.internal", "example.internal"]); }); }); @@ -1568,7 +1618,7 @@ describe("keepLoopbackOffProxy, routed through a real proxy in a child process", } }; const guard = await go("127.0.0.1"); - keepLoopbackOffProxy("http://127.0.0.1:" + target.port + "/v1/traces"); + keepLoopbackOffProxy("http://127.0.0.1:" + target.port + "/v1/traces", process.env); const loopback = await go("127.0.0.1"); const existing = await go("localhost"); console.log(JSON.stringify({ guard, loopback, existing })); From e675d688d57ac9279229c3b61e1930d8526252e9 Mon Sep 17 00:00:00 2001 From: rocklambros Date: Fri, 16 Oct 2026 11:07:14 -0600 Subject: [PATCH 02/10] Apply the bounded drain in close() whether trace export is on or off close() no longer special-cases NULL_OTEL_TRACE_EXPORTER. Its shutdown resolves at once, so a Guardian with export off now gets the same drain, bounded by the negotiated timeout. On Bun 1.3.11 the old path waited on stop(true) alone and hung while a bridge had not answered. The new test timed out there before this change and passes on both versions after. Refs #274 Signed-off-by: rocklambros --- .../guardian/src/otel-trace-exporter.ts | 2 +- .../agt/packages/guardian/src/server.ts | 24 ++++++-------- .../guardian/test/otel-trace-exporter.test.ts | 33 +++++++++++++++++++ 3 files changed, 44 insertions(+), 15 deletions(-) diff --git a/reference-implementations/agt/packages/guardian/src/otel-trace-exporter.ts b/reference-implementations/agt/packages/guardian/src/otel-trace-exporter.ts index d6fd18f9..93bf6064 100644 --- a/reference-implementations/agt/packages/guardian/src/otel-trace-exporter.ts +++ b/reference-implementations/agt/packages/guardian/src/otel-trace-exporter.ts @@ -474,7 +474,7 @@ export type OtelTraceExporter = { /** The exporter a Guardian gets when neither output is configured: the * composition root disabling trace export gets a value of the same type, - * not a special case. */ + * not a special case. server.ts's close() treats it like any other. */ export const NULL_OTEL_TRACE_EXPORTER: OtelTraceExporter = { start(): null { return null; diff --git a/reference-implementations/agt/packages/guardian/src/server.ts b/reference-implementations/agt/packages/guardian/src/server.ts index a20d10a8..922c11bd 100644 --- a/reference-implementations/agt/packages/guardian/src/server.ts +++ b/reference-implementations/agt/packages/guardian/src/server.ts @@ -471,20 +471,16 @@ export async function startGuardian({ return { url: `http://localhost:${server.port}${ACS_PATH}`, async close() { - if (traceExporter === NULL_OTEL_TRACE_EXPORTER) { - // No exporter, nothing to close behind the drain: the close every - // Guardian had before trace export existed. - await server.stop(true); - return; - } - // Bun.serve's stop(true) closes every connection at once but resolves - // only when every in-flight handler has returned -- and a handler whose - // bridge never answers never returns. The drain is therefore bounded by - // the negotiated timeout: past it, no host is still waiting for the - // answer. The timer is unref'd so a prompt stop does not leave it - // holding the process open. The exporter shuts down after the drain, - // so a step still open then is closed as the failure it is rather - // than lost. + // The same close whether trace export is on or off: + // NULL_OTEL_TRACE_EXPORTER's shutdown resolves at once, so with export + // off the drain is all that remains. Bun.serve's stop(true) closes + // every connection at once, and on Bun 1.3.11 it resolves only when + // every in-flight handler has returned. A handler whose bridge never + // answers never returns, so the drain is bounded by the negotiated + // timeout: past it, no host is still waiting for the answer. The timer + // is unref'd so a prompt stop does not leave it holding the process + // open. The exporter shuts down after the drain, so a step still open + // then is closed as the failure it is rather than lost. const deadline = new Promise((resolve) => { setTimeout(resolve, hello.timeout_config.default_ms).unref(); }); diff --git a/reference-implementations/agt/packages/guardian/test/otel-trace-exporter.test.ts b/reference-implementations/agt/packages/guardian/test/otel-trace-exporter.test.ts index 9a6995ba..552555c1 100644 --- a/reference-implementations/agt/packages/guardian/test/otel-trace-exporter.test.ts +++ b/reference-implementations/agt/packages/guardian/test/otel-trace-exporter.test.ts @@ -950,6 +950,39 @@ describe("Guardian trace export wiring", () => { } }, 15_000); + // Ariel's review on PR #286: close() applies the same bounded drain with + // export off as with it on, since the null exporter's shutdown resolves at + // once. Before, a Guardian with export off waited on stop(true) alone. On + // Bun 1.3.11 that waits for every in-flight handler, and a handler whose + // bridge never answers never returns, so close() hung. Bun 1.3.14 resolves + // stop(true) at once, and there this test passes either way. + it("closes in bounded time with export off while a step is still in flight", async () => { + let reached: () => void = () => {}; + const reachedBridge = new Promise((resolve) => { + reached = resolve; + }); + const silent: PolicyBridge = { + evaluate() { + reached(); + return new Promise(() => {}); + }, + }; + const guardian = await startGuardian({ ...baseOptions, bridge: silent }); + const pending = postRaw(guardian.url, toolCallEnvelope("ls -la", { id: 1 })).then( + () => "answered", + () => "connection closed", + ); + await reachedBridge; + const closeStarted = performance.now(); + await guardian.close(); + const closeMs = performance.now() - closeStarted; + // The request is never decided. It ends because the server went away. + expect(await Promise.race([pending, Bun.sleep(2000).then(() => "still open")])).toBe("connection closed"); + // At most the ServerHello's default timeout, 5 s: past it, no host is + // still waiting for the answer. + expect(closeMs).toBeLessThan(buildServerHello().timeout_config.default_ms + 1000); + }, 15_000); + // server.ts builds the exporter from the ServerHello, so the posture on a // failure event is the one this Guardian declared to the host. An envelope // with neither an id nor a request_id earns a deny that cannot be From 43ee9fd501df09b8acaaf886ed326b611f1e3d5d Mon Sep 17 00:00:00 2001 From: rocklambros Date: Fri, 16 Oct 2026 11:09:25 -0600 Subject: [PATCH 03/10] Split the trace exporter into one file per job otel-trace-exporter.ts held four jobs in 1,022 lines. The mapping loader, the attribute source tables and the decision failure event move to otel-mapping.ts, the OTLP/JSON file writer to otlp-file-exporter.ts, and the proxy helpers to loopback-proxy.ts. otel-trace-exporter.ts keeps the assembly and re-exports every name it exported before, so no importer has to change. Code and comments move as they were. The only edits are the export keywords the assembly needs, the module headers, and two comments that pointed at code now in another file. Refs #274 Signed-off-by: rocklambros --- .../agt/packages/guardian/README.md | 7 +- .../packages/guardian/src/loopback-proxy.ts | 93 +++ .../agt/packages/guardian/src/main.ts | 2 +- .../agt/packages/guardian/src/otel-mapping.ts | 366 ++++++++++++ .../guardian/src/otel-trace-exporter.ts | 559 ++---------------- .../guardian/src/otlp-file-exporter.ts | 69 +++ 6 files changed, 586 insertions(+), 510 deletions(-) create mode 100644 reference-implementations/agt/packages/guardian/src/loopback-proxy.ts create mode 100644 reference-implementations/agt/packages/guardian/src/otel-mapping.ts create mode 100644 reference-implementations/agt/packages/guardian/src/otlp-file-exporter.ts diff --git a/reference-implementations/agt/packages/guardian/README.md b/reference-implementations/agt/packages/guardian/README.md index 7c200b74..8c1fcd40 100644 --- a/reference-implementations/agt/packages/guardian/README.md +++ b/reference-implementations/agt/packages/guardian/README.md @@ -149,7 +149,7 @@ await guardian.close(); `StartGuardianOptions` also accepts `hostname`, `envelopeLogPath`, `sessionContextLog`, `otelTraceFile`, `otelTraceEndpoint` and `onDecisionFailure`, which mirror the environment variables above, and five options for tests: `mappingPath`, `annotator`, `bridge`, `sessionContextStore` and `traceExporter`. Each is documented where it is declared, in `src/server.ts`. -`otelTraceEndpoint` leaves the proxy variables alone. `src/main.ts` adds a loopback endpoint to `NO_PROXY` and `no_proxy` and prints the proxy warning. An embedder that wants the same calls `loopbackProxyWarning(endpoint, process.env)` and `keepLoopbackOffProxy(endpoint, process.env)` from `src/otel-trace-exporter.ts` before it starts the Guardian. +`otelTraceEndpoint` leaves the proxy variables alone. `src/main.ts` adds a loopback endpoint to `NO_PROXY` and `no_proxy` and prints the proxy warning. An embedder that wants the same calls `loopbackProxyWarning(endpoint, process.env)` and `keepLoopbackOffProxy(endpoint, process.env)` from `src/loopback-proxy.ts` before it starts the Guardian. The package has two entry points. `guardian` exports the governance verbs listed in `src/index.ts`. `guardian/deployment` exports `createDeploymentBridge`, the one function that builds the deployment's bridge with the egress annotator wired in. The conformance harness imports the second so it measures the bridge the Guardian ships, not a replica. @@ -169,7 +169,10 @@ The package has two entry points. `guardian` exports the governance verbs listed | `src/deny-on-invalid-envelope.ts` | Turns a schema failure on a `steps/*` method into an honoured `deny` | | `src/session-context-store.ts`, `src/session-context.ts`, `src/ifc-labels.ts` | The per-session hash chain, provenance, and information-flow labels | | `src/envelope-log-sink.ts` | The envelope log writer | -| `src/otel-trace-exporter.ts` | The OpenTelemetry span exporter and its OTLP/JSON file writer | +| `src/otel-trace-exporter.ts` | The OpenTelemetry span exporter, assembled from the three files below | +| `src/otel-mapping.ts` | Reads `otel-mapping.json`, and says which envelope field feeds each span attribute | +| `src/otlp-file-exporter.ts` | The OTLP/JSON file writer | +| `src/loopback-proxy.ts` | Keeps a loopback trace endpoint off an HTTP proxy, applied by `src/main.ts` | | `src/audit-to-otlp.ts` | Converts the host adapter's audit log into the same OTLP/JSON file | | `src/acs-result.ts` | The decision result type | diff --git a/reference-implementations/agt/packages/guardian/src/loopback-proxy.ts b/reference-implementations/agt/packages/guardian/src/loopback-proxy.ts new file mode 100644 index 00000000..2ac225c7 --- /dev/null +++ b/reference-implementations/agt/packages/guardian/src/loopback-proxy.ts @@ -0,0 +1,93 @@ +/** + * Keeps a loopback trace endpoint off an HTTP proxy. Both functions take the + * environment as a parameter: main.ts, the composition root, passes + * process.env, and the exporter itself never reads or writes it. + */ + +/** The NO_PROXY entries that exempt a loopback endpoint's host, or none + * when the endpoint is not loopback. IPv6 gets both forms: Bun matches only + * `[::1]`, and other clients expect the bare `::1`. */ +function loopbackProxyEntries(endpoint: string): string[] { + const host = new URL(endpoint).hostname.replace(/^\[|\]$/g, ""); + if (host === "127.0.0.1" || host === "localhost") { + return [host]; + } + return host === "::1" ? ["::1", "[::1]"] : []; +} + +const splitProxyList = (value: string): string[] => + value + .split(",") + .map((entry) => entry.trim()) + .filter((entry) => entry !== ""); + +/** + * Adds a loopback endpoint's host to NO_PROXY and no_proxy, so an + * HTTP_PROXY in the Guardian's environment cannot route spans meant for this + * machine through a proxy: request content (reasoning text, provenance ids) + * would reach the proxy, and the collector would never see it. Bun's HTTP + * client honors these variables per request, and an explicit agent does not + * opt out, so the environment is the only lever. A non-loopback endpoint is + * left to the operator's proxy settings. + * + * Only the variables the user set are touched. Bun 1.3.14 reads a non-empty + * no_proxy in preference to NO_PROXY, so creating the one the user did not + * set would switch off every exemption in the one they did. "Set" follows + * Bun's own rule: any value but the empty string counts, and an empty one + * makes Bun fall back to the other. A set variable that holds no entries + * (a stray comma, say) is what Bun reads, so it is filled with the host. + * When neither is set, both get the host, and nothing is lost. + * + * Bun 1.3.11 reads these variables only when the process starts, so on that + * version this edit has no effect, and loopbackProxyWarning below is what + * tells the operator. + * + * The edit is made to the `env` it is given and never undone. main.ts + * passes process.env, so there the change is process-wide and permanent: + * anything else in that process (a host adapter embedded with the Guardian) + * and any child process it spawns inherits it. That is acceptable because + * only loopback hosts are ever added, and traffic to this machine should not + * use a proxy for any of them. The exporter never calls this itself, so an + * embedder that builds the exporter decides whether its environment changes. + */ +export function keepLoopbackOffProxy(endpoint: string, env: Record): void { + const additions = loopbackProxyEntries(endpoint); + if (additions.length === 0) { + return; + } + const keys = ["NO_PROXY", "no_proxy"] as const; + const setKeys = keys.filter((key) => (env[key] ?? "") !== ""); + for (const key of setKeys.length === 0 ? keys : setKeys) { + const entries = splitProxyList(env[key] ?? ""); + const missing = additions.filter((entry) => !entries.includes(entry)); + if (missing.length > 0) { + env[key] = [...entries, ...missing].join(","); + } + } +} + +/** + * A warning when a loopback endpoint would reach an HTTP proxy, judged by + * the variables as they stand: a proxy is set, and the list Bun reads (a + * non-empty no_proxy, otherwise NO_PROXY) does not exempt the host. main.ts + * calls it before keepLoopbackOffProxy, so on Bun 1.3.11, which ignores the + * runtime edit, the operator still learns that spans may go through the + * proxy and what to set before starting the Guardian. + */ +export function loopbackProxyWarning(endpoint: string, env: Record): string | undefined { + const additions = loopbackProxyEntries(endpoint); + const proxy = env.http_proxy || env.HTTP_PROXY; + if (additions.length === 0 || !proxy) { + return undefined; + } + const list = splitProxyList(env.no_proxy ? env.no_proxy : (env.NO_PROXY ?? "")); + // The bracketed form is the one Bun matches for IPv6. + const needed = additions[additions.length - 1]!; + if (list.includes("*") || list.includes(needed)) { + return undefined; + } + return ( + `otel trace export to ${endpoint}: an HTTP proxy is set and NO_PROXY/no_proxy did not exempt ${needed}, ` + + `so on some Bun versions these spans go through the proxy. Start the Guardian with ${needed} in both NO_PROXY and no_proxy.` + ); +} diff --git a/reference-implementations/agt/packages/guardian/src/main.ts b/reference-implementations/agt/packages/guardian/src/main.ts index 736df9ca..302c81b2 100644 --- a/reference-implementations/agt/packages/guardian/src/main.ts +++ b/reference-implementations/agt/packages/guardian/src/main.ts @@ -19,7 +19,7 @@ * surface and server.ts reads none itself. */ import { buildServerHello } from "./handshake.ts"; -import { keepLoopbackOffProxy, loopbackProxyWarning } from "./otel-trace-exporter.ts"; +import { keepLoopbackOffProxy, loopbackProxyWarning } from "./loopback-proxy.ts"; import { startGuardian } from "./server.ts"; const DEFAULT_PORT = 8787; diff --git a/reference-implementations/agt/packages/guardian/src/otel-mapping.ts b/reference-implementations/agt/packages/guardian/src/otel-mapping.ts new file mode 100644 index 00000000..472015e0 --- /dev/null +++ b/reference-implementations/agt/packages/guardian/src/otel-mapping.ts @@ -0,0 +1,366 @@ +/** + * The trace's vocabulary: what `specification/v0.1.0/trace/otel-mapping.json` + * says, the `acs.decision.failure` event that is not in the mapping yet, and + * the source tables that say where each attribute's value comes from. + * Shared by the two writers of the same file shape: the Guardian's exporter + * (otel-trace-exporter.ts) and the audit-log converter (audit-to-otlp.ts). + * + * What the mapping does not say is which envelope field feeds which + * attribute. That is the one piece of knowledge this module adds, in the + * four source tables below, written once by hand. An attribute the mapping + * names and no table knows is not emitted, and nothing is invented to fill a + * slot the wire did not carry -- a required attribute the envelope lacks is + * simply absent, which is itself a fact worth seeing. + */ +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import type { Attributes, AttributeValue } from "@opentelemetry/api"; + +/** The most Provenance objects one step span describes. The attribute + * length limit in otel-trace-exporter.ts bounds each string, not how many a + * column holds. Both columns are cut at the same index, so the i-th element + * of each still describes the same object. */ +const PROVENANCE_COLUMN_LIMIT = 64; + +/** JSON-RPC REPLAY_DETECTED. The pair on a replayed request belongs to the + * earlier request, so #228 forbids a failure event for it: one would mark + * that earlier, legitimate decision as not enforced. */ +export const REPLAY_DETECTED_CODE = -32005; + +// The repository's own mapping, five directories up from this file -- the +// same relationship SCHEMA_ROOT in validate-envelope.ts relies on. +const OTEL_MAPPING_PATH = fileURLToPath( + new URL("../../../../../specification/v0.1.0/trace/otel-mapping.json", import.meta.url), +); + +/** + * `acs.decision.failure` is not in otel-mapping.json yet; #228 adds it next + * to `decision_event`. Until then its names live here, in one place, the way + * DECISION_DETAIL_SOURCES holds the decision extras: the event name, its + * attribute keys, and the values the decision on #37 fixed for the class, + * the posture and the reporter. When the mapping gains the event, read the + * name from it the way `decision_event` is read. + */ +export const DECISION_FAILURE_EVENT = "acs.decision.failure"; + +/** The span status for a step answered with an error instead of a decision. + * One sentence for both writers, the Guardian here and the audit converter, + * so a query on status text finds both sides of the same failure. */ +export const GUARDIAN_ERROR_STATUS = "The Guardian returned an error instead of a decision."; + +/** The event's attribute keys: the three required ones first, then the + * ones carried when known. */ +export const FAILURE_ATTRIBUTES = { + class: "acs.failure.class", + posture: "acs.failure.posture", + reportedBy: "acs.failure.reported_by", + detail: "acs.failure.detail", + errorCode: "acs.failure.error_code", + timeoutMs: "acs.failure.timeout_ms", + elapsedMs: "acs.failure.elapsed_ms", + payloadBytes: "acs.failure.payload_bytes", +} as const; + +export type FailureClass = "timeout" | "unreachable" | "malformed_response" | "guardian_error" | "observed_agent_error" | "other"; +export type FailurePosture = "fail_open" | "fail_closed"; +export type FailureReporter = "guardian" | "observed_agent"; + +/** One decision failure, as the event records it. `detail` is required + * when the class is `other`; the rest travel when known. */ +export type DecisionFailure = { + class: FailureClass; + posture: FailurePosture; + reportedBy: FailureReporter; + detail?: string; + errorCode?: number; + timeoutMs?: number; + elapsedMs?: number; + payloadBytes?: number; +}; + +/** The event's attributes: the required three always, the others when + * present. One function for both reporters, so the Guardian's exporter and the + * audit-log converter (audit-to-otlp.ts) build the same event. */ +export function decisionFailureAttributes(failure: DecisionFailure): Attributes { + const attributes: Attributes = { + [FAILURE_ATTRIBUTES.class]: failure.class, + [FAILURE_ATTRIBUTES.posture]: failure.posture, + [FAILURE_ATTRIBUTES.reportedBy]: failure.reportedBy, + }; + if (failure.detail !== undefined) attributes[FAILURE_ATTRIBUTES.detail] = failure.detail; + if (failure.errorCode !== undefined) attributes[FAILURE_ATTRIBUTES.errorCode] = failure.errorCode; + if (failure.timeoutMs !== undefined) attributes[FAILURE_ATTRIBUTES.timeoutMs] = failure.timeoutMs; + if (failure.elapsedMs !== undefined) attributes[FAILURE_ATTRIBUTES.elapsedMs] = failure.elapsedMs; + if (failure.payloadBytes !== undefined) attributes[FAILURE_ATTRIBUTES.payloadBytes] = failure.payloadBytes; + return attributes; +} + +/** One `step_to_span` entry. `conditional_attributes` is the list #227 + * adds, for attributes the wire carries only sometimes (`acs.capability` + * among them); read the same way as the other two. */ +type StepSpanEntry = { + span_name: string; + required_attributes: string[]; + conditional_attributes?: string[]; + optional_attributes?: string[]; +}; + +/** + * Methods this Guardian evaluates that have no `step_to_span` entry in + * otel-mapping.json yet, and why. A method here gets no span. The test + * fails for an evaluated method that is neither mapped nor listed here, so + * a new method cannot quietly go untraced: it is either mapped or named + * with its reason. + */ +export const METHODS_WITHOUT_SPAN: Readonly> = {}; + +/** otel-mapping.json is a JSON Schema whose data lives inside `default` + * keys -- the same three places `packages/conformance/src/trace-pillar.ts` + * reads. This type names only those. */ +type OtelMappingSchema = { + properties: { + step_to_span: { default: Record }; + decision_event: { + properties: { + event_name: { const: string }; + required_attributes: { default: string[] }; + conditional_attributes: { default: string[] }; + }; + }; + provenance_attributes: { + properties: { + required: { default: string[] }; + optional: { default: string[] }; + }; + }; + }; +}; + +export type OtelMapping = { + stepToSpan: Record; + decisionEventName: string; + /** The event's required attributes first, then its conditional ones. Both + * are read the same way -- emitted when a value is present -- so one list + * serves. */ + decisionAttributes: string[]; + /** Required first, then optional -- carried on a step span whose payload + * has a Provenance object attached, as the mapping's own text says. */ + provenanceAttributes: string[]; +}; + +export function loadOtelMapping(path: string = OTEL_MAPPING_PATH): OtelMapping { + const schema = JSON.parse(readFileSync(path, "utf8")) as OtelMappingSchema; + const decision = schema.properties.decision_event.properties; + const provenance = schema.properties.provenance_attributes.properties; + return { + stepToSpan: schema.properties.step_to_span.default, + decisionEventName: decision.event_name.const, + decisionAttributes: [...decision.required_attributes.default, ...decision.conditional_attributes.default], + provenanceAttributes: [...provenance.required.default, ...provenance.optional.default], + }; +} + +type JsonObject = Record; + +export function asObject(value: unknown): JsonObject | undefined { + return typeof value === "object" && value !== null && !Array.isArray(value) ? (value as JsonObject) : undefined; +} + +function asString(value: unknown): string | undefined { + return typeof value === "string" ? value : undefined; +} + +export function asNumber(value: unknown): number | undefined { + return typeof value === "number" && Number.isFinite(value) ? value : undefined; +} + +/** A non-empty array of strings, or nothing. An empty array is carried by + * the wire (`policy_references: []` on an envelope-invalid deny) but says + * nothing a reader can use, so it is not emitted. */ +function asStrings(value: unknown): string[] | undefined { + if (!Array.isArray(value) || value.length === 0) return undefined; + return value.every((item) => typeof item === "string") ? (value as string[]) : undefined; +} + +/** + * One column of `policy_references`, in the wire's own order: the same + * length as `policy_references`, with `""` where a reference lacks the + * field. That is the rule agreed on #228, so a reader lines the three + * columns up by position and never loses the values the other references + * did carry. Absent only when there are no references at all. + */ +function policyColumn(references: unknown, field: string): string[] | undefined { + if (!Array.isArray(references) || references.length === 0) return undefined; + return references.map((reference) => asString(asObject(reference)?.[field]) ?? ""); +} + +/** The parts of a request envelope the source tables read. Read off the + * parsed JSON rather than the validated envelope, because the span starts + * before validation runs and an envelope that fails the schema is still a + * step this session took. */ +export type RequestParts = { params: JsonObject; metadata: JsonObject; payload: JsonObject; tool: JsonObject }; + +export function requestParts(envelope: unknown): RequestParts { + const params = asObject(asObject(envelope)?.params) ?? {}; + const metadata = asObject(params.metadata) ?? {}; + const payload = asObject(params.payload) ?? {}; + const tool = asObject(payload.tool) ?? {}; + return { params, metadata, payload, tool }; +} + +/** The decision half of a success response: the `result` and its optional + * `metadata`. `undefined` for a JSON-RPC error response, which carries no + * decision. */ +export type ResultParts = { result: JsonObject; metadata: JsonObject }; + +export function resultParts(response: unknown): ResultParts | undefined { + const result = asObject(asObject(response)?.result); + if (result === undefined) return undefined; + return { result, metadata: asObject(result.metadata) ?? {} }; +} + +export type Source = (parts: Parts) => AttributeValue | undefined; + +/** + * Attribute name -> where it is read from on the request envelope, for every + * attribute `step_to_span` can name on a span this Guardian emits. The + * mapping decides WHICH of these a span carries; this table only says HOW + * each one is read. Paths follow the hook payload schemas + * (`hooks/tool-call-request.json`, `hooks/tool-call-result.json`) and + * `request-envelope.json`'s `AcsParams.metadata`. + */ +export const STEP_ATTRIBUTE_SOURCES: Readonly>> = { + "acs.session.id": (r) => asString(r.metadata.session_id), + "acs.agent.id": (r) => asString(r.metadata.agent_id), + "gen_ai.tool.name": (r) => asString(r.tool.name), + "acs.tool.provider": (r) => asString(r.tool.provider), + "acs.tool.version": (r) => asString(r.tool.version), + "acs.capability": (r) => asString(r.payload.capability), + "acs.operation": (r) => asString(r.payload.operation), + "acs.exit_status": (r) => asString(r.payload.exit_status), + "acs.duration_ms": (r) => asNumber(r.payload.duration_ms), + "acs.request_id_ref": (r) => asString(r.payload.request_id_ref), +}; + +/** + * Carried on every span, whatever the mapping lists for it, so a reader can + * rebuild a session and pair each span with its two envelope-log lines. + * `acs.session.id` is the mapping's own name for the session. + * `acs.request_id` is this module's name for `params.request_id`, the ACS + * correlation id the response echoes; the mapping has no attribute for the + * step's own request id (its `acs.request_id_ref` names the request a + * RESULT refers back to, which is a different fact). + */ +export const CORRELATION_ATTRIBUTE_SOURCES: Record> = { + "acs.session.id": (r) => asString(r.metadata.session_id), + "acs.request_id": (r) => asString(r.params.request_id), +}; + +/** + * The `acs.decision` event's attributes, for every name the mapping's + * `decision_event` lists. Paths follow `response-envelope.json`'s + * `AcsResult`: `decision` and `reasoning` are direct members; the other four + * sit under `metadata`. `acs.evaluator` is read off the wire like the rest + * and never filled in here: the value this Guardian would send is + * `deterministic`, but it has to come from the Guardian's response (#229 + * puts it there), or the trace says something the wire never said and a + * reader cannot tell the two apart. + */ +export const DECISION_ATTRIBUTE_SOURCES: Readonly>> = { + "acs.decision": (d) => asString(d.result.decision), + "acs.evaluator": (d) => asString(d.metadata.evaluator), + "acs.reasoning": (d) => asString(d.result.reasoning), + "acs.confidence": (d) => asNumber(d.metadata.confidence), + "acs.evaluator_version": (d) => asString(d.metadata.evaluator_version), + "acs.model_id": (d) => asString(d.metadata.model_id), + // The decision facts #228 adds to the mapping. Each is emitted only when + // the response carries it; `acs.chain_hash` in particular appears only once + // a Guardian puts the chain head on the wire. + "acs.reason_codes": (d) => asStrings(d.result.reason_codes), + "acs.policy.ids": (d) => policyColumn(d.result.policy_references, "policy_id"), + "acs.policy.versions": (d) => policyColumn(d.result.policy_references, "policy_version"), + "acs.policy.rule_ids": (d) => policyColumn(d.result.policy_references, "rule_id"), + "acs.cited_provenance_ids": (d) => asStrings(d.result.cited_provenance_ids), + "acs.chain_hash": (d) => asString(d.result.chain_hash), +}; + +/** + * The decision facts emitted whether or not the mapping lists them yet. + * Before #228 the mapping does not name them; after it, it does. Reading + * this list beside the mapping's own makes the exporter correct under both, + * so this PR and #228 can land in either order. Once #228 is in, the mapping + * alone covers them and this list can go. + */ +export const DECISION_DETAIL_NAMES = [ + "acs.reason_codes", + "acs.policy.ids", + "acs.policy.versions", + "acs.policy.rule_ids", + "acs.cited_provenance_ids", + "acs.chain_hash", +] as const; + +/** + * Attribute name -> which field of a Provenance object (`provenance.json`) + * it reads. Unlike the tables above, each of these becomes a COLUMN: one + * array per attribute, one element per Provenance object the payload + * carries, in wire order. A span has one value per attribute key, and a + * payload can carry several Provenance objects -- one per argument -- so an + * array is the only shape that keeps every object visible; the i-th element + * of each column describes the i-th object, and `acs.provenance.origin` is + * always complete because provenance.json requires `origin`. + * + * `acs.provenance.lineage_depth` is in the mapping's optional list and has + * no entry here. provenance.json defines no lineage_depth field, so there is + * nothing on the wire to read it from, and deriving one (from the length of + * `derived_from`, say) would be a value nobody sent. + */ +export const PROVENANCE_ATTRIBUTE_SOURCES: Record string | undefined> = { + "acs.provenance.origin": (p) => asString(p.origin), + "acs.provenance.source_id": (p) => asString(p.source_id), +}; + +/** The Provenance objects a payload carries, in wire order: one per + * argument that has one, in the order the parsed object holds them, then one + * per output that has one, in array order. "Parsed" order rather than the + * bytes' order because `JSON.parse` has already hoisted integer-like + * argument names ahead of the rest (envelope-log-sink.ts notes `{"0": ...}` + * is a real shape), so this is the order every reader of the envelope log + * sees too. The two never meet on one payload -- the request gate carries + * `arguments`, the result gate `outputs` -- and an argument or output + * without one contributes nothing. */ +export function provenanceObjects(payload: JsonObject): JsonObject[] { + const carriers: unknown[] = [ + ...Object.values(asObject(payload.arguments) ?? {}), + ...(Array.isArray(payload.outputs) ? payload.outputs : []), + ]; + const found: JsonObject[] = []; + for (const carrier of carriers) { + if (found.length === PROVENANCE_COLUMN_LIMIT) { + break; + } + const provenance = asObject(asObject(carrier)?.provenance); + if (provenance !== undefined) { + found.push(provenance); + } + } + return found; +} + +/** One column over every Provenance object, in order, padded with `""` + * where an object lacks the field -- the same rule policyColumn applies, for + * the same reason: every column is the length of `acs.provenance.origin`, so + * the i-th element of each describes the i-th object, and a `source_id` one + * argument carries is never hidden by another argument that has none. */ +export function provenanceColumn(objects: JsonObject[], read: (provenance: JsonObject) => string | undefined): string[] { + return objects.map((object) => read(object) ?? ""); +} + +export function collect(into: Attributes, names: Iterable, sources: Record>, parts: Parts): void { + for (const name of names) { + const value = sources[name]?.(parts); + if (value !== undefined) { + into[name] = value; + } + } +} diff --git a/reference-implementations/agt/packages/guardian/src/otel-trace-exporter.ts b/reference-implementations/agt/packages/guardian/src/otel-trace-exporter.ts index 93bf6064..645fd2b8 100644 --- a/reference-implementations/agt/packages/guardian/src/otel-trace-exporter.ts +++ b/reference-implementations/agt/packages/guardian/src/otel-trace-exporter.ts @@ -6,12 +6,11 @@ * from `specification/v0.1.0/trace/otel-mapping.json`, read once at * construction. A rename in that mapping needs no change here. * - * What the mapping does not say is which envelope field feeds which - * attribute. That is the one piece of knowledge this module adds, in the - * four source tables below, written once by hand. An attribute the mapping - * names and no table knows is not emitted, and nothing is invented to fill a - * slot the wire did not carry -- a required attribute the envelope lacks is - * simply absent, which is itself a fact worth seeing. + * This file assembles the exporter. Three neighbors hold the rest: + * otel-mapping.ts reads the mapping and says which envelope field feeds + * which attribute, otlp-file-exporter.ts writes the OTLP/JSON file, and + * loopback-proxy.ts holds the proxy helpers main.ts applies before the + * Guardian starts. * * Span lifetime follows the envelope log. `start` is called where the * request is recorded and hands back the span; `end` is called with that @@ -46,16 +45,57 @@ * happen off the request's own turn. Observability degrades; governance * does not. */ -import { mkdirSync, readFileSync } from "node:fs"; -import { appendFile } from "node:fs/promises"; -import { dirname } from "node:path"; -import { fileURLToPath } from "node:url"; -import { SpanStatusCode, type Attributes, type AttributeValue, type Span, type Tracer } from "@opentelemetry/api"; +import { SpanStatusCode, type Attributes, type Span, type Tracer } from "@opentelemetry/api"; import { ExportResultCode } from "@opentelemetry/core"; import { OTLPTraceExporter } from "@opentelemetry/exporter-trace-otlp-http"; -import { JsonTraceSerializer } from "@opentelemetry/otlp-transformer"; import { resourceFromAttributes } from "@opentelemetry/resources"; import { AlwaysOnSampler, BasicTracerProvider, BatchSpanProcessor, type SpanExporter, type SpanLimits } from "@opentelemetry/sdk-trace-base"; +import { + asNumber, + asObject, + collect, + CORRELATION_ATTRIBUTE_SOURCES, + DECISION_ATTRIBUTE_SOURCES, + DECISION_DETAIL_NAMES, + DECISION_FAILURE_EVENT, + decisionFailureAttributes, + GUARDIAN_ERROR_STATUS, + loadOtelMapping, + PROVENANCE_ATTRIBUTE_SOURCES, + provenanceColumn, + provenanceObjects, + REPLAY_DETECTED_CODE, + requestParts, + resultParts, + STEP_ATTRIBUTE_SOURCES, + type DecisionFailure, + type FailureClass, + type OtelMapping, +} from "./otel-mapping.ts"; +import { createFileSpanExporter } from "./otlp-file-exporter.ts"; + +// Every name this module exported before the split, so existing importers +// keep working. +export { + DECISION_ATTRIBUTE_SOURCES, + DECISION_FAILURE_EVENT, + decisionFailureAttributes, + FAILURE_ATTRIBUTES, + GUARDIAN_ERROR_STATUS, + loadOtelMapping, + METHODS_WITHOUT_SPAN, + STEP_ATTRIBUTE_SOURCES, + type DecisionFailure, + type FailureClass, + type FailurePosture, + type FailureReporter, + type OtelMapping, + type RequestParts, + type ResultParts, + type Source, +} from "./otel-mapping.ts"; +export { createFileSpanExporter } from "./otlp-file-exporter.ts"; +export { keepLoopbackOffProxy, loopbackProxyWarning } from "./loopback-proxy.ts"; /** How long the OTLP/HTTP exporter keeps retrying one batch against an * endpoint that does not answer before it reports the failure. */ @@ -74,17 +114,6 @@ const EXPORT_TIMEOUT_MS = ENDPOINT_TIMEOUT_MS + 1000; * this is cut to it. */ const ATTRIBUTE_VALUE_LENGTH_LIMIT = 4096; -/** The most Provenance objects one step span describes. The length limit - * above bounds each string, not how many a column holds. Both columns are - * cut at the same index, so the i-th element of each still describes the - * same object. */ -const PROVENANCE_COLUMN_LIMIT = 64; - -/** JSON-RPC REPLAY_DETECTED. The pair on a replayed request belongs to the - * earlier request, so #228 forbids a failure event for it: one would mark - * that earlier, legitimate decision as not enforced. */ -const REPLAY_DETECTED_CODE = -32005; - /** The `service.name` every span carries. The literal key rather than the * semantic-conventions package: one constant does not earn a dependency. */ export const OTEL_SERVICE_NAME = "acs-reference-guardian"; @@ -102,344 +131,6 @@ export const SPAN_LIMITS: Readonly = Object.freeze({ attributePerLinkCountLimit: 128, }); -// The repository's own mapping, five directories up from this file -- the -// same relationship SCHEMA_ROOT in validate-envelope.ts relies on. -const OTEL_MAPPING_PATH = fileURLToPath( - new URL("../../../../../specification/v0.1.0/trace/otel-mapping.json", import.meta.url), -); - -/** - * `acs.decision.failure` is not in otel-mapping.json yet; #228 adds it next - * to `decision_event`. Until then its names live here, in one place, the way - * DECISION_DETAIL_SOURCES holds the decision extras: the event name, its - * attribute keys, and the values the decision on #37 fixed for the class, - * the posture and the reporter. When the mapping gains the event, read the - * name from it the way `decision_event` is read. - */ -export const DECISION_FAILURE_EVENT = "acs.decision.failure"; - -/** The span status for a step answered with an error instead of a decision. - * One sentence for both writers, the Guardian here and the audit converter, - * so a query on status text finds both sides of the same failure. */ -export const GUARDIAN_ERROR_STATUS = "The Guardian returned an error instead of a decision."; - -/** The event's attribute keys: the three required ones first, then the - * ones carried when known. */ -export const FAILURE_ATTRIBUTES = { - class: "acs.failure.class", - posture: "acs.failure.posture", - reportedBy: "acs.failure.reported_by", - detail: "acs.failure.detail", - errorCode: "acs.failure.error_code", - timeoutMs: "acs.failure.timeout_ms", - elapsedMs: "acs.failure.elapsed_ms", - payloadBytes: "acs.failure.payload_bytes", -} as const; - -export type FailureClass = "timeout" | "unreachable" | "malformed_response" | "guardian_error" | "observed_agent_error" | "other"; -export type FailurePosture = "fail_open" | "fail_closed"; -export type FailureReporter = "guardian" | "observed_agent"; - -/** One decision failure, as the event records it. `detail` is required - * when the class is `other`; the rest travel when known. */ -export type DecisionFailure = { - class: FailureClass; - posture: FailurePosture; - reportedBy: FailureReporter; - detail?: string; - errorCode?: number; - timeoutMs?: number; - elapsedMs?: number; - payloadBytes?: number; -}; - -/** The event's attributes: the required three always, the others when - * present. One function for both reporters, so the exporter below and the - * audit-log converter (audit-to-otlp.ts) build the same event. */ -export function decisionFailureAttributes(failure: DecisionFailure): Attributes { - const attributes: Attributes = { - [FAILURE_ATTRIBUTES.class]: failure.class, - [FAILURE_ATTRIBUTES.posture]: failure.posture, - [FAILURE_ATTRIBUTES.reportedBy]: failure.reportedBy, - }; - if (failure.detail !== undefined) attributes[FAILURE_ATTRIBUTES.detail] = failure.detail; - if (failure.errorCode !== undefined) attributes[FAILURE_ATTRIBUTES.errorCode] = failure.errorCode; - if (failure.timeoutMs !== undefined) attributes[FAILURE_ATTRIBUTES.timeoutMs] = failure.timeoutMs; - if (failure.elapsedMs !== undefined) attributes[FAILURE_ATTRIBUTES.elapsedMs] = failure.elapsedMs; - if (failure.payloadBytes !== undefined) attributes[FAILURE_ATTRIBUTES.payloadBytes] = failure.payloadBytes; - return attributes; -} - -/** One `step_to_span` entry. `conditional_attributes` is the list #227 - * adds, for attributes the wire carries only sometimes (`acs.capability` - * among them); read the same way as the other two. */ -type StepSpanEntry = { - span_name: string; - required_attributes: string[]; - conditional_attributes?: string[]; - optional_attributes?: string[]; -}; - -/** - * Methods this Guardian evaluates that have no `step_to_span` entry in - * otel-mapping.json yet, and why. A method here gets no span. The test - * fails for an evaluated method that is neither mapped nor listed here, so - * a new method cannot quietly go untraced: it is either mapped or named - * with its reason. - */ -export const METHODS_WITHOUT_SPAN: Readonly> = {}; - -/** otel-mapping.json is a JSON Schema whose data lives inside `default` - * keys -- the same three places `packages/conformance/src/trace-pillar.ts` - * reads. This type names only those. */ -type OtelMappingSchema = { - properties: { - step_to_span: { default: Record }; - decision_event: { - properties: { - event_name: { const: string }; - required_attributes: { default: string[] }; - conditional_attributes: { default: string[] }; - }; - }; - provenance_attributes: { - properties: { - required: { default: string[] }; - optional: { default: string[] }; - }; - }; - }; -}; - -export type OtelMapping = { - stepToSpan: Record; - decisionEventName: string; - /** The event's required attributes first, then its conditional ones. Both - * are read the same way -- emitted when a value is present -- so one list - * serves. */ - decisionAttributes: string[]; - /** Required first, then optional -- carried on a step span whose payload - * has a Provenance object attached, as the mapping's own text says. */ - provenanceAttributes: string[]; -}; - -export function loadOtelMapping(path: string = OTEL_MAPPING_PATH): OtelMapping { - const schema = JSON.parse(readFileSync(path, "utf8")) as OtelMappingSchema; - const decision = schema.properties.decision_event.properties; - const provenance = schema.properties.provenance_attributes.properties; - return { - stepToSpan: schema.properties.step_to_span.default, - decisionEventName: decision.event_name.const, - decisionAttributes: [...decision.required_attributes.default, ...decision.conditional_attributes.default], - provenanceAttributes: [...provenance.required.default, ...provenance.optional.default], - }; -} - -type JsonObject = Record; - -function asObject(value: unknown): JsonObject | undefined { - return typeof value === "object" && value !== null && !Array.isArray(value) ? (value as JsonObject) : undefined; -} - -function asString(value: unknown): string | undefined { - return typeof value === "string" ? value : undefined; -} - -function asNumber(value: unknown): number | undefined { - return typeof value === "number" && Number.isFinite(value) ? value : undefined; -} - -/** A non-empty array of strings, or nothing. An empty array is carried by - * the wire (`policy_references: []` on an envelope-invalid deny) but says - * nothing a reader can use, so it is not emitted. */ -function asStrings(value: unknown): string[] | undefined { - if (!Array.isArray(value) || value.length === 0) return undefined; - return value.every((item) => typeof item === "string") ? (value as string[]) : undefined; -} - -/** - * One column of `policy_references`, in the wire's own order: the same - * length as `policy_references`, with `""` where a reference lacks the - * field. That is the rule agreed on #228, so a reader lines the three - * columns up by position and never loses the values the other references - * did carry. Absent only when there are no references at all. - */ -function policyColumn(references: unknown, field: string): string[] | undefined { - if (!Array.isArray(references) || references.length === 0) return undefined; - return references.map((reference) => asString(asObject(reference)?.[field]) ?? ""); -} - -/** The parts of a request envelope the source tables read. Read off the - * parsed JSON rather than the validated envelope, because the span starts - * before validation runs and an envelope that fails the schema is still a - * step this session took. */ -export type RequestParts = { params: JsonObject; metadata: JsonObject; payload: JsonObject; tool: JsonObject }; - -function requestParts(envelope: unknown): RequestParts { - const params = asObject(asObject(envelope)?.params) ?? {}; - const metadata = asObject(params.metadata) ?? {}; - const payload = asObject(params.payload) ?? {}; - const tool = asObject(payload.tool) ?? {}; - return { params, metadata, payload, tool }; -} - -/** The decision half of a success response: the `result` and its optional - * `metadata`. `undefined` for a JSON-RPC error response, which carries no - * decision. */ -export type ResultParts = { result: JsonObject; metadata: JsonObject }; - -function resultParts(response: unknown): ResultParts | undefined { - const result = asObject(asObject(response)?.result); - if (result === undefined) return undefined; - return { result, metadata: asObject(result.metadata) ?? {} }; -} - -export type Source = (parts: Parts) => AttributeValue | undefined; - -/** - * Attribute name -> where it is read from on the request envelope, for every - * attribute `step_to_span` can name on a span this Guardian emits. The - * mapping decides WHICH of these a span carries; this table only says HOW - * each one is read. Paths follow the hook payload schemas - * (`hooks/tool-call-request.json`, `hooks/tool-call-result.json`) and - * `request-envelope.json`'s `AcsParams.metadata`. - */ -export const STEP_ATTRIBUTE_SOURCES: Readonly>> = { - "acs.session.id": (r) => asString(r.metadata.session_id), - "acs.agent.id": (r) => asString(r.metadata.agent_id), - "gen_ai.tool.name": (r) => asString(r.tool.name), - "acs.tool.provider": (r) => asString(r.tool.provider), - "acs.tool.version": (r) => asString(r.tool.version), - "acs.capability": (r) => asString(r.payload.capability), - "acs.operation": (r) => asString(r.payload.operation), - "acs.exit_status": (r) => asString(r.payload.exit_status), - "acs.duration_ms": (r) => asNumber(r.payload.duration_ms), - "acs.request_id_ref": (r) => asString(r.payload.request_id_ref), -}; - -/** - * Carried on every span, whatever the mapping lists for it, so a reader can - * rebuild a session and pair each span with its two envelope-log lines. - * `acs.session.id` is the mapping's own name for the session. - * `acs.request_id` is this module's name for `params.request_id`, the ACS - * correlation id the response echoes; the mapping has no attribute for the - * step's own request id (its `acs.request_id_ref` names the request a - * RESULT refers back to, which is a different fact). - */ -const CORRELATION_ATTRIBUTE_SOURCES: Record> = { - "acs.session.id": (r) => asString(r.metadata.session_id), - "acs.request_id": (r) => asString(r.params.request_id), -}; - -/** - * The `acs.decision` event's attributes, for every name the mapping's - * `decision_event` lists. Paths follow `response-envelope.json`'s - * `AcsResult`: `decision` and `reasoning` are direct members; the other four - * sit under `metadata`. `acs.evaluator` is read off the wire like the rest - * and never filled in here: the value this Guardian would send is - * `deterministic`, but it has to come from the Guardian's response (#229 - * puts it there), or the trace says something the wire never said and a - * reader cannot tell the two apart. - */ -export const DECISION_ATTRIBUTE_SOURCES: Readonly>> = { - "acs.decision": (d) => asString(d.result.decision), - "acs.evaluator": (d) => asString(d.metadata.evaluator), - "acs.reasoning": (d) => asString(d.result.reasoning), - "acs.confidence": (d) => asNumber(d.metadata.confidence), - "acs.evaluator_version": (d) => asString(d.metadata.evaluator_version), - "acs.model_id": (d) => asString(d.metadata.model_id), - // The decision facts #228 adds to the mapping. Each is emitted only when - // the response carries it; `acs.chain_hash` in particular appears only once - // a Guardian puts the chain head on the wire. - "acs.reason_codes": (d) => asStrings(d.result.reason_codes), - "acs.policy.ids": (d) => policyColumn(d.result.policy_references, "policy_id"), - "acs.policy.versions": (d) => policyColumn(d.result.policy_references, "policy_version"), - "acs.policy.rule_ids": (d) => policyColumn(d.result.policy_references, "rule_id"), - "acs.cited_provenance_ids": (d) => asStrings(d.result.cited_provenance_ids), - "acs.chain_hash": (d) => asString(d.result.chain_hash), -}; - -/** - * The decision facts emitted whether or not the mapping lists them yet. - * Before #228 the mapping does not name them; after it, it does. Reading - * this list beside the mapping's own makes the exporter correct under both, - * so this PR and #228 can land in either order. Once #228 is in, the mapping - * alone covers them and this list can go. - */ -const DECISION_DETAIL_NAMES = [ - "acs.reason_codes", - "acs.policy.ids", - "acs.policy.versions", - "acs.policy.rule_ids", - "acs.cited_provenance_ids", - "acs.chain_hash", -] as const; - -/** - * Attribute name -> which field of a Provenance object (`provenance.json`) - * it reads. Unlike the tables above, each of these becomes a COLUMN: one - * array per attribute, one element per Provenance object the payload - * carries, in wire order. A span has one value per attribute key, and a - * payload can carry several Provenance objects -- one per argument -- so an - * array is the only shape that keeps every object visible; the i-th element - * of each column describes the i-th object, and `acs.provenance.origin` is - * always complete because provenance.json requires `origin`. - * - * `acs.provenance.lineage_depth` is in the mapping's optional list and has - * no entry here. provenance.json defines no lineage_depth field, so there is - * nothing on the wire to read it from, and deriving one (from the length of - * `derived_from`, say) would be a value nobody sent. - */ -const PROVENANCE_ATTRIBUTE_SOURCES: Record string | undefined> = { - "acs.provenance.origin": (p) => asString(p.origin), - "acs.provenance.source_id": (p) => asString(p.source_id), -}; - -/** The Provenance objects a payload carries, in wire order: one per - * argument that has one, in the order the parsed object holds them, then one - * per output that has one, in array order. "Parsed" order rather than the - * bytes' order because `JSON.parse` has already hoisted integer-like - * argument names ahead of the rest (envelope-log-sink.ts notes `{"0": ...}` - * is a real shape), so this is the order every reader of the envelope log - * sees too. The two never meet on one payload -- the request gate carries - * `arguments`, the result gate `outputs` -- and an argument or output - * without one contributes nothing. */ -function provenanceObjects(payload: JsonObject): JsonObject[] { - const carriers: unknown[] = [ - ...Object.values(asObject(payload.arguments) ?? {}), - ...(Array.isArray(payload.outputs) ? payload.outputs : []), - ]; - const found: JsonObject[] = []; - for (const carrier of carriers) { - if (found.length === PROVENANCE_COLUMN_LIMIT) { - break; - } - const provenance = asObject(asObject(carrier)?.provenance); - if (provenance !== undefined) { - found.push(provenance); - } - } - return found; -} - -/** One column over every Provenance object, in order, padded with `""` - * where an object lacks the field -- the same rule policyColumn applies, for - * the same reason: every column is the length of `acs.provenance.origin`, so - * the i-th element of each describes the i-th object, and a `source_id` one - * argument carries is never hidden by another argument that has none. */ -function provenanceColumn(objects: JsonObject[], read: (provenance: JsonObject) => string | undefined): string[] { - return objects.map((object) => read(object) ?? ""); -} - -function collect(into: Attributes, names: Iterable, sources: Record>, parts: Parts): void { - for (const name of names) { - const value = sources[name]?.(parts); - if (value !== undefined) { - into[name] = value; - } - } -} - /** * The span this module started for one step, handed to the caller so the * response can close the same span and no other. Opaque: a caller holds it @@ -517,152 +208,6 @@ export type CreateOtelTraceExporterOptions = { onError?: (error: unknown) => void; }; -function asError(error: unknown): Error { - return error instanceof Error ? error : new Error(String(error)); -} - -/** - * Writes OTLP/JSON, one `ExportTraceServiceRequest` per line -- the shape - * the OpenTelemetry Collector's file exporter writes -- serialised by the - * official JsonTraceSerializer rather than by hand. - * - * The append is asynchronous: this exporter runs on the same event loop the - * Guardian answers requests on, and a synchronous write would hold every - * in-flight decision for the duration of the disk I/O. Writes are chained on - * one promise so lines land in export order even when two batches are in - * flight, and `forceFlush` and `shutdown` await the chain so a close never - * leaves a batch unwritten. The chain itself never rejects: one failed write - * is reported through its own batch's callback and cannot poison every write - * after it. - * - * 0o700 on the directory and 0o600 on the file: the file holds reasoning - * text and provenance source ids, so other local users should not read it - * (#71 is the same problem in the envelope log). The mode applies when the - * exporter creates the path; an existing file keeps whatever it had. - */ -export function createFileSpanExporter(path: string): SpanExporter { - mkdirSync(dirname(path), { recursive: true, mode: 0o700 }); - let chain: Promise = Promise.resolve(); - return { - export(spans, resultCallback): void { - let line: string; - try { - const bytes = JsonTraceSerializer.serializeRequest(spans); - if (bytes === undefined) { - throw new Error("JsonTraceSerializer produced no request"); - } - line = `${new TextDecoder().decode(bytes)}\n`; - } catch (error) { - resultCallback({ code: ExportResultCode.FAILED, error: asError(error) }); - return; - } - const write = chain.then(() => appendFile(path, line, { mode: 0o600 })); - chain = write.then( - () => undefined, - () => undefined, - ); - write.then( - () => resultCallback({ code: ExportResultCode.SUCCESS }), - (error: unknown) => resultCallback({ code: ExportResultCode.FAILED, error: asError(error) }), - ); - }, - async shutdown(): Promise { - await chain; - }, - async forceFlush(): Promise { - await chain; - }, - }; -} - -/** The NO_PROXY entries that exempt a loopback endpoint's host, or none - * when the endpoint is not loopback. IPv6 gets both forms: Bun matches only - * `[::1]`, and other clients expect the bare `::1`. */ -function loopbackProxyEntries(endpoint: string): string[] { - const host = new URL(endpoint).hostname.replace(/^\[|\]$/g, ""); - if (host === "127.0.0.1" || host === "localhost") { - return [host]; - } - return host === "::1" ? ["::1", "[::1]"] : []; -} - -const splitProxyList = (value: string): string[] => - value - .split(",") - .map((entry) => entry.trim()) - .filter((entry) => entry !== ""); - -/** - * Adds a loopback endpoint's host to NO_PROXY and no_proxy, so an - * HTTP_PROXY in the Guardian's environment cannot route spans meant for this - * machine through a proxy: request content (reasoning text, provenance ids) - * would reach the proxy, and the collector would never see it. Bun's HTTP - * client honors these variables per request, and an explicit agent does not - * opt out, so the environment is the only lever. A non-loopback endpoint is - * left to the operator's proxy settings. - * - * Only the variables the user set are touched. Bun 1.3.14 reads a non-empty - * no_proxy in preference to NO_PROXY, so creating the one the user did not - * set would switch off every exemption in the one they did. "Set" follows - * Bun's own rule: any value but the empty string counts, and an empty one - * makes Bun fall back to the other. A set variable that holds no entries - * (a stray comma, say) is what Bun reads, so it is filled with the host. - * When neither is set, both get the host, and nothing is lost. - * - * Bun 1.3.11 reads these variables only when the process starts, so on that - * version this edit has no effect, and loopbackProxyWarning below is what - * tells the operator. - * - * The edit is made to the `env` it is given and never undone. main.ts - * passes process.env, so there the change is process-wide and permanent: - * anything else in that process (a host adapter embedded with the Guardian) - * and any child process it spawns inherits it. That is acceptable because - * only loopback hosts are ever added, and traffic to this machine should not - * use a proxy for any of them. The exporter never calls this itself, so an - * embedder that builds the exporter decides whether its environment changes. - */ -export function keepLoopbackOffProxy(endpoint: string, env: Record): void { - const additions = loopbackProxyEntries(endpoint); - if (additions.length === 0) { - return; - } - const keys = ["NO_PROXY", "no_proxy"] as const; - const setKeys = keys.filter((key) => (env[key] ?? "") !== ""); - for (const key of setKeys.length === 0 ? keys : setKeys) { - const entries = splitProxyList(env[key] ?? ""); - const missing = additions.filter((entry) => !entries.includes(entry)); - if (missing.length > 0) { - env[key] = [...entries, ...missing].join(","); - } - } -} - -/** - * A warning when a loopback endpoint would reach an HTTP proxy, judged by - * the variables as they stand: a proxy is set, and the list Bun reads (a - * non-empty no_proxy, otherwise NO_PROXY) does not exempt the host. main.ts - * calls it before keepLoopbackOffProxy, so on Bun 1.3.11, which ignores the - * runtime edit, the operator still learns that spans may go through the - * proxy and what to set before starting the Guardian. - */ -export function loopbackProxyWarning(endpoint: string, env: Record): string | undefined { - const additions = loopbackProxyEntries(endpoint); - const proxy = env.http_proxy || env.HTTP_PROXY; - if (additions.length === 0 || !proxy) { - return undefined; - } - const list = splitProxyList(env.no_proxy ? env.no_proxy : (env.NO_PROXY ?? "")); - // The bracketed form is the one Bun matches for IPv6. - const needed = additions[additions.length - 1]!; - if (list.includes("*") || list.includes(needed)) { - return undefined; - } - return ( - `otel trace export to ${endpoint}: an HTTP proxy is set and NO_PROXY/no_proxy did not exempt ${needed}, ` + - `so on some Bun versions these spans go through the proxy. Start the Guardian with ${needed} in both NO_PROXY and no_proxy.` - ); -} - /** One place spans go, with the name its failure is reported under: the * file's path, the endpoint's URL, or "span exporter" for an injected one. */ type Output = { label: string; exporter: SpanExporter }; diff --git a/reference-implementations/agt/packages/guardian/src/otlp-file-exporter.ts b/reference-implementations/agt/packages/guardian/src/otlp-file-exporter.ts new file mode 100644 index 00000000..e65af21e --- /dev/null +++ b/reference-implementations/agt/packages/guardian/src/otlp-file-exporter.ts @@ -0,0 +1,69 @@ +/** + * The OTLP/JSON file output, the one span exporter this package writes + * itself. The Guardian's exporter and the audit-log converter both use it, + * so the two files share one shape. + */ +import { mkdirSync } from "node:fs"; +import { appendFile } from "node:fs/promises"; +import { dirname } from "node:path"; +import { ExportResultCode } from "@opentelemetry/core"; +import { JsonTraceSerializer } from "@opentelemetry/otlp-transformer"; +import type { SpanExporter } from "@opentelemetry/sdk-trace-base"; + +function asError(error: unknown): Error { + return error instanceof Error ? error : new Error(String(error)); +} + +/** + * Writes OTLP/JSON, one `ExportTraceServiceRequest` per line -- the shape + * the OpenTelemetry Collector's file exporter writes -- serialised by the + * official JsonTraceSerializer rather than by hand. + * + * The append is asynchronous: this exporter runs on the same event loop the + * Guardian answers requests on, and a synchronous write would hold every + * in-flight decision for the duration of the disk I/O. Writes are chained on + * one promise so lines land in export order even when two batches are in + * flight, and `forceFlush` and `shutdown` await the chain so a close never + * leaves a batch unwritten. The chain itself never rejects: one failed write + * is reported through its own batch's callback and cannot poison every write + * after it. + * + * 0o700 on the directory and 0o600 on the file: the file holds reasoning + * text and provenance source ids, so other local users should not read it + * (#71 is the same problem in the envelope log). The mode applies when the + * exporter creates the path; an existing file keeps whatever it had. + */ +export function createFileSpanExporter(path: string): SpanExporter { + mkdirSync(dirname(path), { recursive: true, mode: 0o700 }); + let chain: Promise = Promise.resolve(); + return { + export(spans, resultCallback): void { + let line: string; + try { + const bytes = JsonTraceSerializer.serializeRequest(spans); + if (bytes === undefined) { + throw new Error("JsonTraceSerializer produced no request"); + } + line = `${new TextDecoder().decode(bytes)}\n`; + } catch (error) { + resultCallback({ code: ExportResultCode.FAILED, error: asError(error) }); + return; + } + const write = chain.then(() => appendFile(path, line, { mode: 0o600 })); + chain = write.then( + () => undefined, + () => undefined, + ); + write.then( + () => resultCallback({ code: ExportResultCode.SUCCESS }), + (error: unknown) => resultCallback({ code: ExportResultCode.FAILED, error: asError(error) }), + ); + }, + async shutdown(): Promise { + await chain; + }, + async forceFlush(): Promise { + await chain; + }, + }; +} From acc5ab3f2d621aa12ab9c2db7accf37cd668dfe6 Mon Sep 17 00:00:00 2001 From: rocklambros Date: Fri, 16 Oct 2026 11:09:52 -0600 Subject: [PATCH 04/10] Define REPLAY_DETECTED_CODE and the object reader once audit-to-otlp.ts kept its own REPLAY_DETECTED_CODE and a readObject identical to the exporter's asObject. Both now live in otel-mapping.ts, with the JsonObject type they share, and the converter imports them. The copied sessionUuid stays, since importing it would be a cycle. Refs #274 Signed-off-by: rocklambros --- .../packages/guardian/src/audit-to-otlp.ts | 20 ++++++++----------- .../agt/packages/guardian/src/otel-mapping.ts | 2 +- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/reference-implementations/agt/packages/guardian/src/audit-to-otlp.ts b/reference-implementations/agt/packages/guardian/src/audit-to-otlp.ts index 33f96935..7dd92b10 100644 --- a/reference-implementations/agt/packages/guardian/src/audit-to-otlp.ts +++ b/reference-implementations/agt/packages/guardian/src/audit-to-otlp.ts @@ -53,14 +53,17 @@ import { type SpanExporter, } from "@opentelemetry/sdk-trace-base"; import { - createFileSpanExporter, + asObject, DECISION_FAILURE_EVENT, decisionFailureAttributes, GUARDIAN_ERROR_STATUS, loadOtelMapping, - SPAN_LIMITS, + REPLAY_DETECTED_CODE, type FailureClass, -} from "./otel-trace-exporter.ts"; + type JsonObject, +} from "./otel-mapping.ts"; +import { SPAN_LIMITS } from "./otel-trace-exporter.ts"; +import { createFileSpanExporter } from "./otlp-file-exporter.ts"; /** * The host's `failure.kind` (packages/host-adapter/src/failure-kinds.ts) @@ -110,7 +113,6 @@ const GENERIC_DETAIL = "The host recorded a failure this converter does not clas * (packages/host-adapter/src/failure-posture.ts). Only the integer is * taken; nothing else in the message is copied. */ const ERROR_CODE_IN_MESSAGE = /^guardian (?:refused the envelope with|returned) error (-?\d+):/; -const REPLAY_DETECTED_CODE = -32005; /** * The host adapter's `toSessionUuid` (packages/host-adapter/src/build-envelope.ts), @@ -154,12 +156,6 @@ export type ConversionSummary = { skippedUnreadable: number; }; -type JsonObject = Record; - -function readObject(value: unknown): JsonObject | undefined { - return typeof value === "object" && value !== null && !Array.isArray(value) ? (value as JsonObject) : undefined; -} - function readString(value: unknown): string | undefined { return typeof value === "string" ? value : undefined; } @@ -246,7 +242,7 @@ export async function convertAuditLog(auditPath: string, otelPath: string, mappi if (line.trim() === "") continue; let entry: JsonObject | undefined; try { - entry = readObject(JSON.parse(line)); + entry = asObject(JSON.parse(line)); } catch { entry = undefined; } @@ -267,7 +263,7 @@ export async function convertAuditLog(auditPath: string, otelPath: string, mappi summary.skippedUnreadable += 1; continue; } - const failure = readObject(entry.failure) ?? {}; + const failure = asObject(entry.failure) ?? {}; const kind = readString(failure.kind); const method = readString(entry.method); if (method === undefined) { diff --git a/reference-implementations/agt/packages/guardian/src/otel-mapping.ts b/reference-implementations/agt/packages/guardian/src/otel-mapping.ts index 472015e0..3da8fdba 100644 --- a/reference-implementations/agt/packages/guardian/src/otel-mapping.ts +++ b/reference-implementations/agt/packages/guardian/src/otel-mapping.ts @@ -160,7 +160,7 @@ export function loadOtelMapping(path: string = OTEL_MAPPING_PATH): OtelMapping { }; } -type JsonObject = Record; +export type JsonObject = Record; export function asObject(value: unknown): JsonObject | undefined { return typeof value === "object" && value !== null && !Array.isArray(value) ? (value as JsonObject) : undefined; From 76eb7772d64e1c4e4fda3d50ecb24b389e55a5f2 Mon Sep 17 00:00:00 2001 From: rocklambros Date: Fri, 16 Oct 2026 11:10:08 -0600 Subject: [PATCH 05/10] Name the OTLP compression type once The endpoint construction assembled the SDK's compression type inline on a line past 200 characters. OtlpCompression names it once, with a comment saying why the enum is not imported. Refs #274 Signed-off-by: rocklambros --- .../agt/packages/guardian/src/otel-trace-exporter.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/reference-implementations/agt/packages/guardian/src/otel-trace-exporter.ts b/reference-implementations/agt/packages/guardian/src/otel-trace-exporter.ts index 645fd2b8..c46e81f2 100644 --- a/reference-implementations/agt/packages/guardian/src/otel-trace-exporter.ts +++ b/reference-implementations/agt/packages/guardian/src/otel-trace-exporter.ts @@ -97,6 +97,9 @@ export { export { createFileSpanExporter } from "./otlp-file-exporter.ts"; export { keepLoopbackOffProxy, loopbackProxyWarning } from "./loopback-proxy.ts"; +/** The SDK's compression enum, which this package does not import directly. */ +type OtlpCompression = NonNullable[0]>["compression"]; + /** How long the OTLP/HTTP exporter keeps retrying one batch against an * endpoint that does not answer before it reports the failure. */ const ENDPOINT_TIMEOUT_MS = 2000; @@ -339,9 +342,10 @@ export function createOtelTraceExporter({ // gzip the body. Headers from OTEL_EXPORTER_OTLP_*HEADERS are merged by // the SDK under any set here and cannot be refused, so the README lists // them as honored. - construct(endpoint, () => { - return new OTLPTraceExporter({ url: endpoint, timeoutMillis: ENDPOINT_TIMEOUT_MS, compression: "none" as NonNullable[0]>["compression"] }); - }); + construct( + endpoint, + () => new OTLPTraceExporter({ url: endpoint, timeoutMillis: ENDPOINT_TIMEOUT_MS, compression: "none" as OtlpCompression }), + ); } if (spanExporter !== undefined) { construct("span exporter", () => spanExporter); From 2c7913d8f5784733d2dde52af4308bce8b7c2de2 Mon Sep 17 00:00:00 2001 From: rocklambros Date: Fri, 16 Oct 2026 11:10:20 -0600 Subject: [PATCH 06/10] Note the review changes in the 0.1.4 changelog Refs #274 Signed-off-by: rocklambros --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8ca0ffe..d342a1bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ This release changes ACS-Trace for producers and consumers. The specification ve - `otel-collector.yaml` stays on loopback, keeps its file across restarts, writes owner-only, and pins its container image by digest (PR #277, issue #276). - `OTEL_TRACES_*`, `OTEL_SPAN_*`, `OTEL_ATTRIBUTE_*`, `OTEL_BSP_*` and OTLP compression variables no longer change what the Guardian or the converter exports. `OTEL_EXPORTER_OTLP_*HEADERS` still apply (PR #277). - A loopback trace endpoint is added to whichever of `NO_PROXY` and `no_proxy` is set, keeping the user's own entries, and the Guardian warns at startup when a proxy is set and loopback isn't exempt. The walkthrough says to stop the Guardian and the collector before deleting `.acs` ([PR #279](https://github.com/GenAI-Security-Project/agent-control-standard/pull/279), issue #274). +- The Guardian's `main.ts`, not the trace exporter, now reads the proxy environment and applies the loopback exemption, and `close()` applies the same bounded drain whether trace export is on or off (Ariel's review on PR #286). ### Removed From be3362e1ce754039e41feef4b70fc393d6cc660a Mon Sep 17 00:00:00 2001 From: rocklambros Date: Fri, 16 Oct 2026 11:18:51 -0600 Subject: [PATCH 07/10] Prove main.ts keeps a loopback trace endpoint off the proxy The existing main.ts test checked only the warning, so removing the keepLoopbackOffProxy call left it green. The new test runs main.ts with HTTP_PROXY and a user NO_PROXY, sends one step, stops the Guardian with SIGTERM, and checks that spans reached a stub collector and nothing reached the proxy. It fails with the call removed and is gated to Bun 1.3.14 and later. The envelope helpers move to test/helpers so both test files can use them. Refs #274 Signed-off-by: rocklambros --- .../guardian/test/helpers/envelopes.ts | 35 +++++++ .../agt/packages/guardian/test/main.test.ts | 99 ++++++++++++++++++- .../guardian/test/otel-trace-exporter.test.ts | 32 +----- 3 files changed, 131 insertions(+), 35 deletions(-) create mode 100644 reference-implementations/agt/packages/guardian/test/helpers/envelopes.ts diff --git a/reference-implementations/agt/packages/guardian/test/helpers/envelopes.ts b/reference-implementations/agt/packages/guardian/test/helpers/envelopes.ts new file mode 100644 index 00000000..b216e3ca --- /dev/null +++ b/reference-implementations/agt/packages/guardian/test/helpers/envelopes.ts @@ -0,0 +1,35 @@ +/** + * Request envelopes for the guardian package's tests. Shared because both + * the exporter tests and main.test.ts post a real `steps/toolCallRequest`. + */ + +export function makeEnvelope( + method: string, + payload: Record, + overrides: { id?: number | null; requestId?: string; sessionId?: string } = {}, +): Record { + const { id = 1, requestId = crypto.randomUUID(), sessionId = crypto.randomUUID() } = overrides; + return { + jsonrpc: "2.0", + method, + ...(id === null ? {} : { id }), + params: { + acs_version: "0.1.0", + request_id: requestId, + timestamp: new Date().toISOString(), + metadata: { agent_id: "agent-1", session_id: sessionId }, + payload, + }, + }; +} + +/** Carries `capability`, which hooks/tool-call-request.json declares + * optional and the mapping requires on the span, so a span built from this + * envelope has every required attribute the mapping names. */ +export function toolCallEnvelope(command: string, overrides: { id?: number | null; requestId?: string; sessionId?: string } = {}) { + return makeEnvelope( + "steps/toolCallRequest", + { tool: { name: "run_shell" }, capability: "process.exec", arguments: { command: { value: command } } }, + overrides, + ); +} diff --git a/reference-implementations/agt/packages/guardian/test/main.test.ts b/reference-implementations/agt/packages/guardian/test/main.test.ts index cdc87916..b4d2f5ac 100644 --- a/reference-implementations/agt/packages/guardian/test/main.test.ts +++ b/reference-implementations/agt/packages/guardian/test/main.test.ts @@ -5,11 +5,12 @@ * the wiring under test is the entrypoint itself. */ import { afterEach, describe, expect, it } from "bun:test"; -import { existsSync, mkdtempSync, rmdirSync } from "node:fs"; +import { existsSync, mkdtempSync, rmdirSync, unlinkSync } from "node:fs"; import { createServer } from "node:net"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { fileURLToPath } from "node:url"; +import { toolCallEnvelope } from "./helpers/envelopes.ts"; const GUARDIAN = fileURLToPath(new URL("../src/main.ts", import.meta.url)); /** Absolute, because the process runs from a scratch cwd rather than from @@ -23,11 +24,18 @@ function scratch(): string { return dir; } -/** The Guardian mkdirs `.acs/` at startup and writes nothing into it until - * an envelope arrives. Nothing posts here, so an empty `.acs/` is the only - * thing a run can leave, and rmdirSync fails loudly on anything else. */ +/** What a run can leave in its scratch cwd: the `.acs/` directory the + * Guardian makes at startup, and the two default logs once an envelope + * arrives. Each is removed by name, never recursively, so rmdirSync fails + * loudly on anything else. */ +const LEFT_BEHIND = [".acs/envelopes.jsonl", ".acs/session-context.jsonl"]; afterEach(() => { for (const dir of dirs.splice(0)) { + for (const file of LEFT_BEHIND) { + if (existsSync(join(dir, file))) { + unlinkSync(join(dir, file)); + } + } const acs = join(dir, ".acs"); if (existsSync(acs)) { rmdirSync(acs); @@ -84,4 +92,87 @@ describe("main.ts applies the loopback proxy exemption", () => { "so on some Bun versions these spans go through the proxy. Start the Guardian with 127.0.0.1 in both NO_PROXY and no_proxy.", ); }); + + // The warning above proves main.ts reads the variables. This proves it + // edits them: spans reach the collector and not the proxy. Gated to Bun + // 1.3.14 and later, because 1.3.11 reads the proxy variables only at + // startup and ignores the edit. + it.skipIf(!Bun.semver.satisfies(Bun.version, ">=1.3.14"))( + "sends spans to a loopback collector directly when NO_PROXY holds only the user's own entries", + async () => { + // A proxy that only counts connections and never answers. + let proxyConnections = 0; + const proxy = createServer((socket) => { + proxyConnections += 1; + socket.destroy(); + }); + await new Promise((resolve) => proxy.listen(0, "127.0.0.1", resolve)); + const proxyUrl = `http://127.0.0.1:${(proxy.address() as { port: number }).port}`; + let exports = 0; + const collector = Bun.serve({ + port: 0, + hostname: "127.0.0.1", + fetch() { + exports += 1; + return new Response("{}", { headers: { "content-type": "application/json" } }); + }, + }); + // Async spawn, not spawnSync: the proxy and the collector run on this + // process's event loop and could not answer the child while blocked. + // `timeout` kills a Guardian that never exits, so the test fails on + // the assertions rather than hanging. + const proc = Bun.spawn([process.execPath, GUARDIAN], { + cwd: scratch(), + env: { + PATH: process.env.PATH ?? "", + HOME: process.env.HOME ?? "", + NO_COLOR: "1", + ACS_GUARDIAN_PORT: "0", + ACS_MANIFEST_PATH: MANIFEST, + ACS_OTEL_ENDPOINT: `http://127.0.0.1:${collector.port}/v1/traces`, + HTTP_PROXY: proxyUrl, + http_proxy: proxyUrl, + // A user's own exemption, which the edit must keep. no_proxy is + // left unset, so the edit goes to NO_PROXY alone. + NO_PROXY: "corp.internal", + }, + stdout: "pipe", + stderr: "ignore", + timeout: 12_000, + }); + try { + // main.ts prints "OTel traces:" in the same turn that installs its + // SIGTERM handler, so once that line is in, a SIGTERM flushes. + const reader = proc.stdout.getReader(); + const decoder = new TextDecoder(); + let stdout = ""; + while (!stdout.includes("OTel traces:")) { + const { value, done } = await reader.read(); + if (done) { + break; + } + stdout += decoder.decode(value, { stream: true }); + } + reader.releaseLock(); + const url = /Guardian listening at (\S+)/.exec(stdout)?.[1]; + expect(url).toBeDefined(); + const response = await fetch(url!, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(toolCallEnvelope("ls -la")), + }); + expect(response.ok).toBe(true); + await response.json(); + proc.kill("SIGTERM"); + await proc.exited; + expect({ proxyConnections, exportsReceived: exports > 0 }).toEqual({ proxyConnections: 0, exportsReceived: true }); + } finally { + proc.kill(); + await proc.exited; + collector.stop(true); + proxy.close(); + } + }, + 15_000, + ); }); diff --git a/reference-implementations/agt/packages/guardian/test/otel-trace-exporter.test.ts b/reference-implementations/agt/packages/guardian/test/otel-trace-exporter.test.ts index 552555c1..0d69ca9d 100644 --- a/reference-implementations/agt/packages/guardian/test/otel-trace-exporter.test.ts +++ b/reference-implementations/agt/packages/guardian/test/otel-trace-exporter.test.ts @@ -9,6 +9,7 @@ import type { PolicyBridge } from "agt-bridge"; import { startGuardian, type GuardianSnapshot } from "../src/index.ts"; import { FAILURE_DETAIL_BY_KIND } from "../src/audit-to-otlp.ts"; import { buildServerHello } from "../src/handshake.ts"; +import { makeEnvelope, toolCallEnvelope } from "./helpers/envelopes.ts"; import { createFileSpanExporter, keepLoopbackOffProxy, @@ -33,37 +34,6 @@ const MAPPING = loadOtelMapping(); const METHODS_EVALUATED = buildServerHello().methods_evaluated; const TOOL_CALL_SPAN = MAPPING.stepToSpan["steps/toolCallRequest"]!; -function makeEnvelope( - method: string, - payload: Record, - overrides: { id?: number | null; requestId?: string; sessionId?: string } = {}, -): Record { - const { id = 1, requestId = crypto.randomUUID(), sessionId = crypto.randomUUID() } = overrides; - return { - jsonrpc: "2.0", - method, - ...(id === null ? {} : { id }), - params: { - acs_version: "0.1.0", - request_id: requestId, - timestamp: new Date().toISOString(), - metadata: { agent_id: "agent-1", session_id: sessionId }, - payload, - }, - }; -} - -/** Carries `capability`, which hooks/tool-call-request.json declares - * optional and the mapping requires on the span -- so a span built from this - * envelope has every required attribute the mapping names. */ -function toolCallEnvelope(command: string, overrides: { id?: number | null; requestId?: string; sessionId?: string } = {}) { - return makeEnvelope( - "steps/toolCallRequest", - { tool: { name: "run_shell" }, capability: "process.exec", arguments: { command: { value: command } } }, - overrides, - ); -} - /** A Provenance object as provenance.json defines it: `provenance_id` and * `origin` required, everything else optional. */ function provenance(origin: string, extra: Record = {}): Record { From 04c95e69c8ad4e3adfd175aa93a0717f93d93e4c Mon Sep 17 00:00:00 2001 From: rocklambros Date: Fri, 16 Oct 2026 11:19:05 -0600 Subject: [PATCH 08/10] Export the loopback proxy helpers from the guardian package The README told embedders to call keepLoopbackOffProxy and loopbackProxyWarning from src/loopback-proxy.ts, a path the package does not export. index.ts now re-exports both, and the README names the package import. Refs #274 Signed-off-by: rocklambros --- reference-implementations/agt/packages/guardian/README.md | 2 +- reference-implementations/agt/packages/guardian/src/index.ts | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/reference-implementations/agt/packages/guardian/README.md b/reference-implementations/agt/packages/guardian/README.md index 8c1fcd40..4c604f7f 100644 --- a/reference-implementations/agt/packages/guardian/README.md +++ b/reference-implementations/agt/packages/guardian/README.md @@ -149,7 +149,7 @@ await guardian.close(); `StartGuardianOptions` also accepts `hostname`, `envelopeLogPath`, `sessionContextLog`, `otelTraceFile`, `otelTraceEndpoint` and `onDecisionFailure`, which mirror the environment variables above, and five options for tests: `mappingPath`, `annotator`, `bridge`, `sessionContextStore` and `traceExporter`. Each is documented where it is declared, in `src/server.ts`. -`otelTraceEndpoint` leaves the proxy variables alone. `src/main.ts` adds a loopback endpoint to `NO_PROXY` and `no_proxy` and prints the proxy warning. An embedder that wants the same calls `loopbackProxyWarning(endpoint, process.env)` and `keepLoopbackOffProxy(endpoint, process.env)` from `src/loopback-proxy.ts` before it starts the Guardian. +`otelTraceEndpoint` leaves the proxy variables alone. `src/main.ts` prints the proxy warning and adds a loopback endpoint's host to whichever of `NO_PROXY` and `no_proxy` is set, or to both when neither is. An embedder that wants the same imports `loopbackProxyWarning` and `keepLoopbackOffProxy` from `guardian` and calls each with `(endpoint, process.env)` before it starts the Guardian. The package has two entry points. `guardian` exports the governance verbs listed in `src/index.ts`. `guardian/deployment` exports `createDeploymentBridge`, the one function that builds the deployment's bridge with the egress annotator wired in. The conformance harness imports the second so it measures the bridge the Guardian ships, not a replica. diff --git a/reference-implementations/agt/packages/guardian/src/index.ts b/reference-implementations/agt/packages/guardian/src/index.ts index f1bc099e..56257536 100644 --- a/reference-implementations/agt/packages/guardian/src/index.ts +++ b/reference-implementations/agt/packages/guardian/src/index.ts @@ -74,3 +74,7 @@ export { type SessionProvenance, } from "./session-context-store.ts"; export { persistIfcLabels, supplySourceLabels } from "./ifc-labels.ts"; +// Not governance verbs, but startGuardian's otelTraceEndpoint leaves the +// proxy variables alone, and an embedder that wants main.ts's loopback +// exemption needs these two to apply it. +export { keepLoopbackOffProxy, loopbackProxyWarning } from "./loopback-proxy.ts"; From 9564387ea77e756ebb5d7d6ce8e7be23bedb03d8 Mon Sep 17 00:00:00 2001 From: rocklambros Date: Fri, 16 Oct 2026 11:19:49 -0600 Subject: [PATCH 09/10] Say what close() does with trace export off The exporter header said a Guardian with export off behaves exactly as before. close() now applies the bounded drain in that case too, so the header says that instead. Refs #274 Signed-off-by: rocklambros --- .../agt/packages/guardian/src/otel-trace-exporter.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/reference-implementations/agt/packages/guardian/src/otel-trace-exporter.ts b/reference-implementations/agt/packages/guardian/src/otel-trace-exporter.ts index c46e81f2..68cc61e3 100644 --- a/reference-implementations/agt/packages/guardian/src/otel-trace-exporter.ts +++ b/reference-implementations/agt/packages/guardian/src/otel-trace-exporter.ts @@ -31,7 +31,9 @@ * those into the same event with `reported_by` set to the observed agent. * * Off by default. With no file and no endpoint configured, startGuardian is - * handed NULL_OTEL_TRACE_EXPORTER and behaves exactly as before. + * handed NULL_OTEL_TRACE_EXPORTER, which emits nothing. The Guardian's + * decisions are the same as with export on, and so is the bounded drain in + * close(). * * Total by construction, as envelope-log-sink.ts is. Every entry point is * wrapped, and nothing propagates into or delays the decision path. Each From d5b1acd58def0374a962d3cc0d4bda36334608c8 Mon Sep 17 00:00:00 2001 From: rocklambros Date: Fri, 16 Oct 2026 11:19:49 -0600 Subject: [PATCH 10/10] Use plain punctuation in the trace exporter's comments Replaces each " -- " in the comments of otel-mapping.ts, otlp-file-exporter.ts and otel-trace-exporter.ts with a period, comma, colon or parentheses. Comments only. loopback-proxy.ts had none. Refs #274 Signed-off-by: rocklambros --- .../agt/packages/guardian/src/otel-mapping.ts | 18 +++++++++--------- .../guardian/src/otel-trace-exporter.ts | 8 ++++---- .../guardian/src/otlp-file-exporter.ts | 4 ++-- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/reference-implementations/agt/packages/guardian/src/otel-mapping.ts b/reference-implementations/agt/packages/guardian/src/otel-mapping.ts index 3da8fdba..05d9443b 100644 --- a/reference-implementations/agt/packages/guardian/src/otel-mapping.ts +++ b/reference-implementations/agt/packages/guardian/src/otel-mapping.ts @@ -9,7 +9,7 @@ * attribute. That is the one piece of knowledge this module adds, in the * four source tables below, written once by hand. An attribute the mapping * names and no table knows is not emitted, and nothing is invented to fill a - * slot the wire did not carry -- a required attribute the envelope lacks is + * slot the wire did not carry. A required attribute the envelope lacks is * simply absent, which is itself a fact worth seeing. */ import { readFileSync } from "node:fs"; @@ -27,7 +27,7 @@ const PROVENANCE_COLUMN_LIMIT = 64; * that earlier, legitimate decision as not enforced. */ export const REPLAY_DETECTED_CODE = -32005; -// The repository's own mapping, five directories up from this file -- the +// The repository's own mapping, five directories up from this file, the // same relationship SCHEMA_ROOT in validate-envelope.ts relies on. const OTEL_MAPPING_PATH = fileURLToPath( new URL("../../../../../specification/v0.1.0/trace/otel-mapping.json", import.meta.url), @@ -115,7 +115,7 @@ type StepSpanEntry = { export const METHODS_WITHOUT_SPAN: Readonly> = {}; /** otel-mapping.json is a JSON Schema whose data lives inside `default` - * keys -- the same three places `packages/conformance/src/trace-pillar.ts` + * keys, the same three places `packages/conformance/src/trace-pillar.ts` * reads. This type names only those. */ type OtelMappingSchema = { properties: { @@ -140,10 +140,10 @@ export type OtelMapping = { stepToSpan: Record; decisionEventName: string; /** The event's required attributes first, then its conditional ones. Both - * are read the same way -- emitted when a value is present -- so one list + * are read the same way (emitted when a value is present), so one list * serves. */ decisionAttributes: string[]; - /** Required first, then optional -- carried on a step span whose payload + /** Required first, then optional. Carried on a step span whose payload * has a Provenance object attached, as the mapping's own text says. */ provenanceAttributes: string[]; }; @@ -305,7 +305,7 @@ export const DECISION_DETAIL_NAMES = [ * it reads. Unlike the tables above, each of these becomes a COLUMN: one * array per attribute, one element per Provenance object the payload * carries, in wire order. A span has one value per attribute key, and a - * payload can carry several Provenance objects -- one per argument -- so an + * payload can carry several Provenance objects (one per argument), so an * array is the only shape that keeps every object visible; the i-th element * of each column describes the i-th object, and `acs.provenance.origin` is * always complete because provenance.json requires `origin`. @@ -326,8 +326,8 @@ export const PROVENANCE_ATTRIBUTE_SOURCES: Record