Address Ariel's review of the trace exporter

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
 

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` 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.
 
 ## Files
@@ -167,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 |
 

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<string, unknown>;
-
-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) {

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";

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<string, string | undefined>): 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, string | undefined>): 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.`
+  );
+}

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 "./loopback-proxy.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