feat(errors): expose sanitized upstream error details in client responses (#1718)

docs/security/ERROR_SANITIZATION.md

@@ -132,6 +132,27 @@ When adding a new route or executor, copy the assertion pattern from this file.
 - The `pino` redaction config (`src/lib/log/redaction.ts` — if present) handles structured log redaction separately. This doc covers only the response-message surface.
 - Upstream-header denylist (`src/shared/constants/upstreamHeaders.ts`) covers header leakage — keep both files aligned when adding a new exfiltration concern.
 
+## Upstream details passthrough
+
+`buildErrorBody` accepts an optional third argument `upstreamDetails` (raw
+parsed body from the upstream provider). When provided, it is sanitized by
+`sanitizeUpstreamDetails` before inclusion in the response as `upstream_details`.
+
+Sanitization rules applied to `upstreamDetails`:
+1. String leaves: run through `sanitizeErrorMessage` (strips stacks + absolute paths).
+2. Key blocklist: keys matching `/stack|trace|path|file|cwd|dir|password|secret|token|key/i`
+   are removed.
+3. Depth cap: nesting beyond 4 levels is replaced with the string `"[truncated]"`.
+4. Arrays are capped at 32 elements.
+
+Only the seven upstream-error `createErrorResult` call sites in `chatCore.ts` pass
+`upstreamErrorBody`. Internal OmniRoute errors (SSE parse failures, empty content,
+guardrail blocks) do not include `upstream_details`.
+
+Do NOT pass raw `err.stack`, `err.message`, or any string from a runtime exception to
+`upstreamDetails`. Those must still go through `errorResponse` / `buildErrorBody(code, msg)`
+without an upstream body.
+
 ## Known CodeQL limitation: custom sanitizers not recognized
 
 The CodeQL query [`js/stack-trace-exposure`](https://codeql.github.com/codeql-query-help/javascript/js-stack-trace-exposure/) uses a fixed allowlist of sanitizer patterns (e.g. inline `.split("\n")[0]`, `String#replace` with specific regex shapes, access to `.message` on `Error`). It does **not** recognize indirection through a custom helper like our `sanitizeErrorMessage()`.

open-sse/handlers/chatCore.ts

@@ -4124,7 +4124,8 @@ export async function handleChatCore({
           errMsg,
           retryAfterMs,
           upstreamErrorCode,
-          upstreamErrorType
+          upstreamErrorType,
+          upstreamErrorBody
         );
       }
     }

tests/unit/error-message-sanitization.test.ts

@@ -240,6 +240,104 @@ test("buildErrorBody never exposes stack traces in its message", async () => {
   assert.ok(!body.error.message.includes("at /opt"));
 });
 
+// ── sanitizeUpstreamDetails ──────────────────────────────────────────────────
+
+test("sanitizeUpstreamDetails — basic pass-through for safe fields", async () => {
+  const { sanitizeUpstreamDetails } = await import("../../open-sse/utils/error.ts");
+  const input = { error: { message: "context_length_exceeded", type: "invalid_request_error" } };
+  const out = sanitizeUpstreamDetails(input) as any;
+  assert.equal(out.error.message, "context_length_exceeded");
+  assert.equal(out.error.type, "invalid_request_error");
+});
+
+test("sanitizeUpstreamDetails — sanitizes string values (absolute path)", async () => {
+  const { sanitizeUpstreamDetails } = await import("../../open-sse/utils/error.ts");
+  const input = { error: { message: "bad input at /srv/app/src/lib/db.ts:42" } };
+  const out = sanitizeUpstreamDetails(input) as any;
+  assert.ok(!out.error.message.includes("/srv/app/src/lib/db.ts"), "absolute path must be stripped");
+  assert.ok(out.error.message.includes("<path>"), "path placeholder must be present");
+});
+
+test("sanitizeUpstreamDetails — removes blocked keys (stack, apiKey)", async () => {
+  const { sanitizeUpstreamDetails } = await import("../../open-sse/utils/error.ts");
+  const input = { error: { message: "oops" }, stack: "Error\n    at foo.ts:1", apiKey: "sk-secret" };
+  const out = sanitizeUpstreamDetails(input) as any;
+  assert.ok(!("stack" in out), "stack key must be removed");
+  assert.ok(!("apiKey" in out), "apiKey key must be removed");
+  assert.equal(out.error.message, "oops");
+});
+
+test("sanitizeUpstreamDetails — depth cap replaces nested value at depth > 4", async () => {
+  const { sanitizeUpstreamDetails } = await import("../../open-sse/utils/error.ts");
+  // Build depth-6 nesting: a.b.c.d.e.f = "leaf"
+  const input = { a: { b: { c: { d: { e: { f: "leaf" } } } } } };
+  const out = sanitizeUpstreamDetails(input) as any;
+  // depth 0:a, 1:b, 2:c, 3:d, 4:e → e is at depth 4, f would be depth 5 → truncated
+  assert.equal(out.a.b.c.d.e, "[truncated]");
+});
+
+// ── buildErrorBody with upstreamDetails ──────────────────────────────────────
+
+test("buildErrorBody — without upstream details omits upstream_details field", async () => {
+  const { buildErrorBody } = await import("../../open-sse/utils/error.ts");
+  const body = buildErrorBody(400, "bad request");
+  assert.ok(!("upstream_details" in body), "upstream_details must be absent when not provided");
+});
+
+test("buildErrorBody — with safe upstream details embeds upstream_details", async () => {
+  const { buildErrorBody } = await import("../../open-sse/utils/error.ts");
+  const body = buildErrorBody(400, "bad request", { error: { message: "context_length_exceeded" } });
+  assert.ok("upstream_details" in body, "upstream_details must be present");
+  assert.equal((body.upstream_details as any).error.message, "context_length_exceeded");
+});
+
+test("buildErrorBody — upstream details with stack key are stripped", async () => {
+  const { buildErrorBody } = await import("../../open-sse/utils/error.ts");
+  const body = buildErrorBody(500, "err", { stack: "Error\n    at foo.ts:1", code: "internal" });
+  assert.ok("upstream_details" in body, "upstream_details must be present");
+  assert.ok(!("stack" in (body.upstream_details as any)), "stack must be stripped from upstream_details");
+  assert.equal((body.upstream_details as any).code, "internal");
+});
+
+// ── createErrorResult with upstreamDetails ───────────────────────────────────
+
+test("createErrorResult — response body includes upstream_details when provided", async () => {
+  const { createErrorResult } = await import("../../open-sse/utils/error.ts");
+  const result = createErrorResult(
+    400,
+    "context too long",
+    null,
+    "context_length_exceeded",
+    "invalid_request_error",
+    { error: { message: "context_length_exceeded" } }
+  );
+  const body = (await result.response.clone().json()) as any;
+  assert.ok("upstream_details" in body, "upstream_details must be in response body");
+  assert.equal(body.upstream_details.error.message, "context_length_exceeded");
+});
+
+test("createErrorResult — response body excludes upstream_details when not provided", async () => {
+  const { createErrorResult } = await import("../../open-sse/utils/error.ts");
+  const result = createErrorResult(400, "bad request", null, "bad_request");
+  const body = (await result.response.clone().json()) as any;
+  assert.ok(!("upstream_details" in body), "upstream_details must be absent when not provided");
+});
+
+test("regression: upstream_details never contains stack trace text", async () => {
+  const { createErrorResult } = await import("../../open-sse/utils/error.ts");
+  const upstream = { error: { message: "err" }, stack: "Error\n    at /abs/path.ts:1:2" };
+  const result = createErrorResult(500, "upstream err", null, undefined, undefined, upstream);
+  const body = (await result.response.clone().json()) as any;
+  const serialized = JSON.stringify(body);
+  assert.ok(
+    !serialized.includes("at /abs/path.ts"),
+    "stack trace path must not appear in response body"
+  );
+  assert.ok(!("stack" in (body.upstream_details || {})), "stack key must not be present");
+});
+
+// ── existing tests continue ──────────────────────────────────────────────────
+
 test("GET /token-health response never leaks stack frames or absolute paths", async () => {
   const tokenHealthRoute = await import("../../src/app/api/token-health/route.ts");
   const res = await tokenHealthRoute.GET();