fix(web): retain safe Brave failure details (#3429)

Co-authored-by: Justin Helmer <justin@coreplane.ai>

Author: justinhelmer

docs/reference/specs/web-tools.md

@@ -10,7 +10,7 @@ Agents can read a web page the user links and search the web for current informa
 1. **URL reading is provider-agnostic and in-process** (Option B): `web_fetch` fetches an http(s) URL and returns its text (HTML is stripped to readable text), running in the bot process via an injected `web` capability — no Executor, no workspace. A no-repo agent can use it.
 2. **web_fetch is SSRF-hardened, connect-time**: only `http`/`https` schemes; literal internal addresses are refused synchronously — including every IPv6 form of an internal address (loopback, `10/8`, `172.16/12`, `192.168/16`, link-local `169.254/16` incl. cloud metadata, ULA, and IPv4-mapped/-compat/NAT64 IPv6 like `::ffff:169.254.169.254`, expanded fully rather than string-matched) — and internal hostnames (`localhost`, `*.local`, `*.internal`). For hostnames, the production fetch uses an undici dispatcher whose `lookup` resolves and refuses internal IPs **at connect time and connects to exactly that address** — so the validated IP is the connected IP (no DNS-rebinding TOCTOU). Redirect targets are re-validated on every hop (sync guard) and re-connect through the same dispatcher. Responses are size-capped (~1 MB) and time-bounded (~12 s); blocks/timeouts/HTTP errors return a clear message, never a crash.
    - **Binary links reach the model as vision/document input (M1b)**: when the response `content-type` (parameters stripped, case-folded) is `image/jpeg|png|gif|webp` or `application/pdf`, `web_fetch` returns a parts list — a text header plus an `image` / `document` content part (base64) — instead of text, so the model *sees* the picture or reads the PDF. This is the second producer of image/document parts after Slack attachments ([slack-channel.md](slack-channel.md)), and the reason `tool_result.content` is `string | ToolResultPart[]` (`src/core/chatMessage.ts`). Per-file caps match the attachment path (5 MiB image, 10 MiB PDF); an over-cap binary is refused with a message (never truncated — a cut image is garbage), and other `image/*` types (e.g. SVG) are named as unsupported rather than sent to a model that would reject them. Text URLs are unchanged. The parts reach the model through the relay ([harness-pi.md](harness-pi.md) item 7): an image as pi's image block inside the tool result, a document as its descriptor (name, type, size), pi's own adapters carrying them to the provider. Run-visibility summaries render binary parts as `[image image/png, N bytes]` — the base64 never enters the stream.
-3. **web_search is a seam with ≥2 implementations**: `WebSearch` has a real `BraveWebSearch` adapter (keyed by `BRAVE_SEARCH_API_KEY`) and a `NullWebSearch`. `makeWebCapability` selects Brave when the key is present, Null otherwise. With no key, `web_search` returns a clear "not configured" message (and notes URL reading still works) — it never breaks the [run](../vocabulary.md#run).
+3. **web_search is a seam with ≥2 implementations**: `WebSearch` has a real `BraveWebSearch` adapter (keyed by `BRAVE_SEARCH_API_KEY`) and a `NullWebSearch`. `makeWebCapability` selects Brave when the key is present, Null otherwise. With no key, `web_search` returns a clear "not configured" message (and notes URL reading still works) — it never breaks the [run](../vocabulary.md#run). A failed Brave response keeps its observed HTTP status, validated `ErrorResponse` code and request ID, bounded rate-limit window values, and one redacted, capped detail line in the tool result. An unparseable body keeps status without inventing provider fields. A successful response with a null or missing web section yields no results; other invalid web shapes yield a decode failure. A transport failure has no HTTP status or remote message. Neither failure retries the request or exposes the search URL, key, raw body or headers. The adapter follows [Brave's error shape](https://api-dashboard.search.brave.com/api-reference/web/search/post) and [rate-limit headers](https://api-dashboard.search.brave.com/documentation/guides/rate-limiting).
 4. **Enablement by toolset**: `web_fetch` (URL reading) is in the `full` (coding), `readonly` (review), `assistant` (general) and `explore` toolsets — broadly available; `web_search` is gated to the `web` toolset, held by the research-capable agent, and the `explore` toolset, whose long investigation reads the web beside the repository it runs ([agent-explore.md](agent-explore.md) item 2). `general` reads a linked URL but does not search ([agent-general.md](agent-general.md) item 1).
 5. **A dedicated `research` agent** (`toolset: "web"`, `machine: "none"`) answers research/URL questions with search + fetch and no workspace; since [github-tools.md](github-tools.md) item 6 its toolset also holds the GitHub READ tools, so a question about one of our repos is answered from the repo over the App credential (a github.com URL of ours is read with `github_file`/`github_tree`, not `web_fetch`), and its prompt forbids concluding a repo is inaccessible from a public-web 404. `general` directs web-research asks to `agent:research`.
 6. **The capability is injected, not global**: the dispatcher builds `web` from `process.env` per run; when absent (e.g., a run that didn't inject it), the tools report themselves unavailable rather than throwing.
@@ -34,6 +34,6 @@ Agents can read a web page the user links and search the web for current informa
 | Live: `agent:research` describes an image link / summarizes a PDF link end-to-end | `[agent]` In Slack: `@switchboard agent:research what does https://<public>.png show?` → the answer describes the picture; `… summarize https://<public>.pdf` → the answer cites the PDF's content. |
 | Timeout/HTTP-error handled gracefully | `[unit]` `::web_fetch tool::reports a timeout gracefully`, `::reports non-2xx status` |
 | web_search formats seam results; missing key degrades gracefully | `[unit]` `::web_search tool::formats results from the search seam`, `::degrades gracefully when search is not configured`, `::handles empty query and missing capability`, `::reports a generic search failure` |
-| Brave adapter calls the API correctly and parses results; ≥2 impls selectable | `[unit]` `::BraveWebSearch adapter::calls the Brave API with the key header and parses results`, `::throws on non-200`; `::makeWebCapability::selects Brave when a key is present, Null otherwise`, `::NullWebSearch throws WebSearchUnavailableError` |
+| Brave adapter calls the API correctly and parses results; null web results are empty; a failed response keeps safe observed details and a failed transport stays status-free; ≥2 impls selectable | `[unit]` `src/tools/web.test.ts::BraveWebSearch adapter::calls the Brave API with the key header and parses results`, `::treats a null Brave web section as no results`, `::throws on non-200`, `::keeps a Brave 429 code, request ID, rate limits and redacted detail on the tool result`, `::reports Brave transport failure without inventing an HTTP response or exposing the request URL`, `::uses the observed status and omits invalid Brave fields while capping the detail`, `::keeps HTTP status when Brave sends an unparseable error body`, `::reports a malformed successful Brave response as a decode failure`; `::makeWebCapability::selects Brave when a key is present, Null otherwise`, `::NullWebSearch throws WebSearchUnavailableError` |
 | Enablement: web_fetch broad (incl. `assistant`), web_search gated to the `web` and `explore` toolsets; general holds `assistant` without web_search or bash | `[unit]` `src/tools/web.test.ts::toolset + agent wiring::*` |
 | Live: `agent:research` answers a question / summarizes a URL end-to-end | `[agent]` In Slack: `@switchboard agent:research <a question needing current sources>` → the answer cites sources found via `web_search`; `… summarize <public URL>` → the answer reflects that page's content. The `web_search` half requires `BRAVE_SEARCH_API_KEY` to be provisioned on the deployment; without it the run must still answer, saying search is not configured. |

src/tools/web.test.ts

@@ -522,12 +522,88 @@ describe("BraveWebSearch adapter", () => {
     expect(results).toEqual([{ title: "Cat", url: "https://cats", snippet: "meow" }]);
   });
 
+  it("treats a null Brave web section as no results", async () => {
+    const search = new BraveWebSearch(new Secret("key123", "BRAVE_SEARCH_API_KEY"), async () =>
+      Response.json({ type: "search", web: null }),
+    );
+    const out = await webSearchTool.run({ query: "cats" }, ctxWith({ search }));
+    expect(out).toBe('No results for "cats".');
+  });
+
   it("throws on non-200", async () => {
     const fetchSpy = vi.fn<FetchLike>(async () => fakeResponse({ status: 429 }));
     await expect(new BraveWebSearch(new Secret("k", "BRAVE_SEARCH_API_KEY"), fetchSpy).search("x")).rejects.toThrow(
       /429/,
     );
   });
+
+  it("keeps a Brave 429 code, request ID, rate limits and redacted detail on the tool result", async () => {
+    const fetchSpy = vi.fn<FetchLike>(async () =>
+      Response.json(
+        {
+          type: "ErrorResponse",
+          error: {
+            id: "request-123",
+            status: 429,
+            code: "RATE_LIMITED",
+            detail:
+              "Try https://private.example/path?token=secret or ghp_123456789012345678901234567890 for cats\nPRIVATE_SECOND_LINE",
+          },
+        },
+        {
+          status: 429,
+          headers: { "x-ratelimit-remaining": "0, 1000", "x-ratelimit-reset": "1, 1419704" },
+        },
+      ),
+    );
+    const search = new BraveWebSearch(new Secret("key123", "BRAVE_SEARCH_API_KEY"), fetchSpy);
+    const out = await webSearchTool.run({ query: "cats" }, ctxWith({ search }));
+
+    expect(fetchSpy).toHaveBeenCalledOnce();
+    expect(out).toBe(
+      "web_search failed: Brave search HTTP 429; code RATE_LIMITED; request request-123; remaining 0,1000; reset 1,1419704s; detail Try «redacted-url» or «redacted-github-token» for «redacted-query»",
+    );
+    expect(out).not.toMatch(/private\.example|PRIVATE_SECOND_LINE|ghp_1234|key123/);
+  });
+
+  it("reports Brave transport failure without inventing an HTTP response or exposing the request URL", async () => {
+    const search = new BraveWebSearch(new Secret("key123", "BRAVE_SEARCH_API_KEY"), async () => {
+      throw new Error("GET https://api.search.brave.com/search?q=PRIVATE_QUERY failed");
+    });
+    const out = await webSearchTool.run({ query: "PRIVATE_QUERY" }, ctxWith({ search }));
+    expect(out).toBe("web_search failed: Brave search transport failed before a response");
+  });
+
+  it("uses the observed status and omits invalid Brave fields while capping the detail", async () => {
+    const search = new BraveWebSearch(new Secret("key123", "BRAVE_SEARCH_API_KEY"), async () =>
+      Response.json(
+        {
+          type: "ErrorResponse",
+          error: { id: "https://private.example/id", status: 401, code: "BAD;TOKEN", detail: "A".repeat(300) },
+        },
+        { status: 429, headers: { "x-ratelimit-remaining": "0, PRIVATE", "x-ratelimit-reset": "-1" } },
+      ),
+    );
+    const out = await webSearchTool.run({ query: "cats" }, ctxWith({ search }));
+    expect(out).toBe(`web_search failed: Brave search HTTP 429; detail ${"A".repeat(200)}…`);
+  });
+
+  it("keeps HTTP status when Brave sends an unparseable error body", async () => {
+    const search = new BraveWebSearch(
+      new Secret("key123", "BRAVE_SEARCH_API_KEY"),
+      async () => new Response("PRIVATE_HTML_BODY", { status: 403 }),
+    );
+    const out = await webSearchTool.run({ query: "cats" }, ctxWith({ search }));
+    expect(out).toBe("web_search failed: Brave search HTTP 403");
+  });
+
+  it("reports a malformed successful Brave response as a decode failure", async () => {
+    const search = new BraveWebSearch(new Secret("key123", "BRAVE_SEARCH_API_KEY"), async () =>
+      Response.json({ web: { results: "PRIVATE_BAD_ROWS" } }),
+    );
+    const out = await webSearchTool.run({ query: "cats" }, ctxWith({ search }));
+    expect(out).toBe("web_search failed: Brave search HTTP 200 response was invalid");
+  });
 });
 
 describe("toolset + agent wiring", () => {

src/tools/web.ts

@@ -1,6 +1,7 @@
 import { lookup as dnsLookup } from "node:dns/promises";
 import { Agent, fetch as undiciFetch } from "undici";
 import { observeHttpRequest } from "../core/trace/httpTransport.js";
+import { oneLine, redactAndCap } from "../core/redact.js";
 import type { Secret, Secrets } from "../secrets.js";
 import type { RunnableTool } from "./runnableTool.js";
 
@@ -43,6 +44,97 @@ export class NullWebSearch implements WebSearch {
   }
 }
 
+type BraveFailure = {
+  kind: "http" | "transport" | "timeout" | "decode";
+  status?: number;
+  code?: string;
+  requestId?: string;
+  detail?: string;
+  remaining?: readonly number[];
+  resetSeconds?: readonly number[];
+};
+
+class BraveSearchError extends Error {
+  constructor(
+    readonly facts: Readonly<BraveFailure>,
+    cause?: unknown,
+  ) {
+    const message =
+      facts.kind === "http"
+        ? [
+            `Brave search HTTP ${facts.status}`,
+            ...(facts.code ? [`code ${facts.code}`] : []),
+            ...(facts.requestId ? [`request ${facts.requestId}`] : []),
+            ...(facts.remaining ? [`remaining ${facts.remaining.join(",")}`] : []),
+            ...(facts.resetSeconds ? [`reset ${facts.resetSeconds.join(",")}s`] : []),
+            ...(facts.detail ? [`detail ${facts.detail}`] : []),
+          ].join("; ")
+        : facts.kind === "decode"
+          ? `Brave search HTTP ${facts.status} response was invalid`
+          : facts.kind === "timeout"
+            ? "Brave search timed out before a response"
+            : `Brave search ${facts.kind} failed before a response`;
+    super(message, { cause });
+    this.name = "BraveSearchError";
+  }
+}
+
+function rateWindowFacts(value: string | null): readonly number[] | undefined {
+  if (!value || value.length > 64 || !/^(?:0|[1-9][0-9]{0,9})(?:,\s*(?:0|[1-9][0-9]{0,9})){0,3}$/.test(value))
+    return undefined;
+  const numbers = value.split(",").map((part) => Number(part.trim()));
+  return numbers.every((number) => number <= 1_000_000_000) ? numbers : undefined;
+}
+
+function braveDetail(value: unknown, query: string, apiKey: string): string | undefined {
+  if (typeof value !== "string") return undefined;
+  const first = oneLine(value);
+  if (!first) return undefined;
+  const text = first
+    .replaceAll(apiKey, "«redacted»")
+    .replace(/\b[a-z][a-z0-9+.-]*:\/\/\S+/gi, "«redacted-url»")
+    .replace(/\b[A-Z0-9._%+-]+@[A-Z0-9.-]+\.[A-Z]{2,}\b/gi, "«redacted-email»");
+  return redactAndCap(query.length >= 4 ? text.replaceAll(query, "«redacted-query»") : text, 200);
+}
+
+async function braveFailure(res: Response, query: string, apiKey: string): Promise<BraveSearchError> {
+  let body: Awaited<ReturnType<typeof readCapped>>;
+  try {
+    body = await readCapped(res, 8_192);
+  } catch (cause) {
+    return new BraveSearchError({ kind: "decode", status: res.status }, cause);
+  }
+  let remote: Record<string, unknown> | undefined;
+  if (!body.truncated) {
+    try {
+      const parsed: unknown = JSON.parse(body.bytes.toString("utf8"));
+      if (
+        parsed &&
+        typeof parsed === "object" &&
+        !Array.isArray(parsed) &&
+        "type" in parsed &&
+        parsed.type === "ErrorResponse"
+      ) {
+        const error = (parsed as { error?: unknown }).error;
+        if (error && typeof error === "object" && !Array.isArray(error)) remote = error as Record<string, unknown>;
+      }
+    } catch {
+      // The HTTP failure still has a status; unparseable body text is not a provider message.
+    }
+  }
+  const code = typeof remote?.code === "string" && /^[A-Z][A-Z_]{0,39}$/.test(remote.code) ? remote.code : undefined;
+  const requestId = typeof remote?.id === "string" && /^[A-Za-z0-9-]{1,64}$/.test(remote.id) ? remote.id : undefined;
+  return new BraveSearchError({
+    kind: "http",
+    status: res.status,
+    ...(code ? { code } : {}),
+    ...(requestId ? { requestId } : {}),
+    ...(remote ? { detail: braveDetail(remote.detail, query, apiKey) } : {}),
+    remaining: rateWindowFacts(res.headers.get("x-ratelimit-remaining")),
+    resetSeconds: rateWindowFacts(res.headers.get("x-ratelimit-reset")),
+  });
+}
+
 /** Real adapter: Brave Search API. Coded now; stays keyless until the operator
  *  provisions BRAVE_SEARCH_API_KEY (a separate, deliberate decision). */
 export class BraveWebSearch implements WebSearch {
@@ -54,13 +146,30 @@ export class BraveWebSearch implements WebSearch {
   async search(query: string, opts?: { count?: number }): Promise<WebSearchResult[]> {
     const count = Math.min(Math.max(Math.trunc(opts?.count ?? 5), 1), 10);
     const url = `https://api.search.brave.com/res/v1/web/search?q=${encodeURIComponent(query)}&count=${count}`;
-    const res = await this.fetchImpl(url, {
-      headers: { Accept: "application/json", "X-Subscription-Token": this.apiKey.reveal() },
-      signal: AbortSignal.timeout(SEARCH_TIMEOUT_MS),
-    });
-    if (!res.ok) throw new Error(`brave search returned HTTP ${res.status}`);
-    const data = (await res.json()) as { web?: { results?: Array<Record<string, unknown>> } };
-    const results = data?.web?.results ?? [];
+    let res: Response;
+    try {
+      res = await this.fetchImpl(url, {
+        headers: { Accept: "application/json", "X-Subscription-Token": this.apiKey.reveal() },
+        signal: AbortSignal.timeout(SEARCH_TIMEOUT_MS),
+      });
+    } catch (cause) {
+      const timedOut = cause instanceof Error && (cause.name === "TimeoutError" || cause.name === "AbortError");
+      throw new BraveSearchError({ kind: timedOut ? "timeout" : "transport" }, cause);
+    }
+    if (!res.ok) throw await braveFailure(res, query, this.apiKey.reveal());
+    let results: Array<Record<string, unknown>>;
+    try {
+      const data: unknown = await res.json();
+      if (!data || typeof data !== "object" || Array.isArray(data)) throw new TypeError("invalid Brave response");
+      const web = (data as Record<string, unknown>).web;
+      if (web != null && (typeof web !== "object" || Array.isArray(web))) throw new TypeError("invalid Brave response");
+      const rows = web == null ? [] : ((web as Record<string, unknown>).results ?? []);
+      if (!Array.isArray(rows) || rows.some((row) => !row || typeof row !== "object" || Array.isArray(row)))
+        throw new TypeError("invalid Brave response");
+      results = rows as Array<Record<string, unknown>>;
+    } catch (cause) {
+      throw new BraveSearchError({ kind: "decode", status: res.status }, cause);
+    }
     return results.slice(0, count).map((r) => ({
       title: String(r.title ?? ""),
       url: String(r.url ?? ""),