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 diff --git a/reference-implementations/agt/packages/guardian/README.md b/reference-implementations/agt/packages/guardian/README.md index 3a59cebe..4c604f7f 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` 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 | 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/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"; 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 08cea99a..302c81b2 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 "./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 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..05d9443b --- /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], + }; +} + +export 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, which is 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 e315db71..b28bb85b 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 @@ -32,30 +31,76 @@ * 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 * output fails on its own: the first failed or throwing export disables * that output alone, reports once under its label, and reaches the * processor as a success, so a collector that is down, restarting or - * answering 4xx cannot reject a flush and take the file down with it -- the + * answering 4xx cannot reject a flush and take the file down with it. The * lab reads the file. A broken mapping or a bug inside start/end still * disables everything, once. Ended spans go to one BatchSpanProcessor per * output and the file is appended asynchronously, so serialisation and I/O * 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"; + +/** 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. */ @@ -74,17 +119,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 +136,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 @@ -459,7 +155,7 @@ export type StepSpan = { * points: `start` where the request is recorded, `end` where the response * is, with the handle `start` returned. */ export type OtelTraceExporter = { - /** Null when this request produces no span -- `end` then does nothing. + /** Null when this request produces no span, and `end` then does nothing. * `payloadBytes` is the request body's size, for the failure event. */ start(envelope: unknown, method: string | null, payloadBytes?: number): StepSpan | null; end(stepSpan: StepSpan | null, response: unknown): void; @@ -474,7 +170,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; @@ -492,7 +188,7 @@ export type CreateOtelTraceExporterOptions = { /** A span exporter used beside (or instead of) the two above. Exists so * tests can read spans back in-process; not meant for production use. */ spanExporter?: SpanExporter; - /** The methods this Guardian dispatches -- the ServerHello's own list. + /** The methods this Guardian dispatches: the ServerHello's own list. * A method outside it produces no span even when the mapping names one. */ methodsEvaluated: readonly string[]; /** The ServerHello's `timeout_config`: how long the host waits for a @@ -517,150 +213,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 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. - */ -export function keepLoopbackOffProxy(endpoint: string): 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] ?? "") !== ""); - for (const key of setKeys.length === 0 ? keys : setKeys) { - const entries = splitProxyList(process.env[key] ?? ""); - const missing = additions.filter((entry) => !entries.includes(entry)); - if (missing.length > 0) { - process.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. 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. - */ -export function loopbackProxyWarning(endpoint: string, env: Record = process.env): 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 }; @@ -792,14 +344,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, () => { - 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"] }); - }); + construct( + endpoint, + () => new OTLPTraceExporter({ url: endpoint, timeoutMillis: ENDPOINT_TIMEOUT_MS, compression: "none" as OtlpCompression }), + ); } if (spanExporter !== undefined) { construct("span exporter", () => spanExporter); @@ -995,7 +543,7 @@ export function createOtelTraceExporter({ } } openSpans.clear(); - // Flushes every processor, then releases them -- bounded, because a + // Flushes every processor, then releases them. Bounded, because a // stalled output would otherwise hold the process open. The SDK's own // export timeout covers a batch in flight; this covers an exporter // whose shutdown never returns. 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..2d524082 --- /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; + }, + }; +} 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/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 new file mode 100644 index 00000000..b4d2f5ac --- /dev/null +++ b/reference-implementations/agt/packages/guardian/test/main.test.ts @@ -0,0 +1,178 @@ +/** + * 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, 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 + * 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; +} + +/** 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); + } + 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.", + ); + }); + + // 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 2c5f05b6..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 { @@ -950,6 +920,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 @@ -1378,28 +1381,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 +1475,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 +1491,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 +1499,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 +1508,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 +1517,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 +1528,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 +1621,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 }));