fix(websearch): reject non-positive WEB_CUSTOM env overrides (#2124)

The custom web-search provider read WEB_CUSTOM_TIMEOUT_SEC and
WEB_CUSTOM_MAX_BODY_KB with Number(x) || DEFAULT. That idiom only
rescues 0 and NaN — a negative or Infinity value is truthy and passes
straight through. A negative WEB_CUSTOM_MAX_BODY_KB makes the
"body exceeds N bytes" check reject every POST search, and a negative
WEB_CUSTOM_TIMEOUT_SEC drives an immediate abort on every request.

Route both through readPositiveEnvNumber, which falls back to the
default for missing, empty, non-finite, or non-positive input.

Author: 0xfandom

src/tools/WebSearchTool/providers/custom.test.ts

@@ -3,7 +3,12 @@ import {
   acquireSharedMutationLock,
   releaseSharedMutationLock,
 } from '../../../test/sharedMutationLock.js'
-import { extractHits, customProvider, isPrivateHostname } from './custom.js'
+import {
+  extractHits,
+  customProvider,
+  isPrivateHostname,
+  readPositiveEnvNumber,
+} from './custom.js'
 
 async function importFreshCustomProvider() {
   const stamp = `${Date.now()}-${Math.random()}`
@@ -420,3 +425,33 @@ describe('isPrivateHostname — IPv6', () => {
     expect(isPrivateHostname('not:an:ipv6')).toBe(false)
   })
 })
+
+// ---------------------------------------------------------------------------
+// readPositiveEnvNumber — WEB_CUSTOM_TIMEOUT_SEC / WEB_CUSTOM_MAX_BODY_KB
+// ---------------------------------------------------------------------------
+
+describe('readPositiveEnvNumber', () => {
+  test('parses a valid positive override', () => {
+    expect(readPositiveEnvNumber('45', 120)).toBe(45)
+    expect(readPositiveEnvNumber('0.5', 120)).toBe(0.5)
+  })
+
+  test('falls back for missing / empty / non-numeric input', () => {
+    expect(readPositiveEnvNumber(undefined, 120)).toBe(120)
+    expect(readPositiveEnvNumber('', 120)).toBe(120)
+    expect(readPositiveEnvNumber('fast', 120)).toBe(120)
+  })
+
+  test('falls back for zero and negative values instead of passing them through', () => {
+    // The old `Number(x) || DEFAULT` idiom rescued 0 but let negatives past —
+    // a negative timeout aborts every request and a negative body cap makes the
+    // size guard reject every POST.
+    expect(readPositiveEnvNumber('0', 120)).toBe(120)
+    expect(readPositiveEnvNumber('-1', 120)).toBe(120)
+    expect(readPositiveEnvNumber('-9999', 300)).toBe(300)
+  })
+
+  test('falls back for non-finite values (Infinity)', () => {
+    expect(readPositiveEnvNumber('1e999', 120)).toBe(120)
+  })
+})

src/tools/WebSearchTool/providers/custom.ts

@@ -142,6 +142,24 @@ const DEFAULT_MAX_BODY_KB = 300
 /** Default request timeout in seconds. */
 const DEFAULT_TIMEOUT_SECONDS = 120
 
+/**
+ * Read a positive numeric env override, falling back to `fallback` for
+ * missing, empty, non-finite, or non-positive values.
+ *
+ * `Number(raw) || fallback` alone only rescues 0 and NaN — a negative or
+ * Infinity value is truthy and passes straight through. That silently breaks
+ * the size/timeout guards downstream: a negative WEB_CUSTOM_MAX_BODY_KB makes
+ * the "body exceeds N bytes" check fire for every POST, and a negative
+ * WEB_CUSTOM_TIMEOUT_SEC aborts every request immediately.
+ */
+export function readPositiveEnvNumber(
+  raw: string | undefined,
+  fallback: number,
+): number {
+  const parsed = Number(raw)
+  return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback
+}
+
 /** Header names that are always allowed (case-insensitive). */
 const SAFE_HEADER_NAMES = new Set([
   'accept',
@@ -515,7 +533,7 @@ function buildRequest(query: string) {
     const bodyTemplate = process.env.WEB_BODY_TEMPLATE
     if (bodyTemplate) {
       const body = bodyTemplate.replace(/\{query\}/g, query)
-      const maxBodyBytes = (Number(process.env.WEB_CUSTOM_MAX_BODY_KB) || DEFAULT_MAX_BODY_KB) * 1024
+      const maxBodyBytes = readPositiveEnvNumber(process.env.WEB_CUSTOM_MAX_BODY_KB, DEFAULT_MAX_BODY_KB) * 1024
       if (Buffer.byteLength(body) > maxBodyBytes) {
         throw new Error(
           `POST body exceeds ${maxBodyBytes} bytes. ` +
@@ -582,7 +600,7 @@ export function extractHits(raw: any, jsonPath?: string): SearchHit[] {
 // ---------------------------------------------------------------------------
 
 async function fetchWithRetry(url: string, init: RequestInit, signal?: AbortSignal): Promise<any> {
-  const timeoutSec = Number(process.env.WEB_CUSTOM_TIMEOUT_SEC) || DEFAULT_TIMEOUT_SECONDS
+  const timeoutSec = readPositiveEnvNumber(process.env.WEB_CUSTOM_TIMEOUT_SEC, DEFAULT_TIMEOUT_SECONDS)
   const timeoutMs = timeoutSec * 1000
   let lastErr: Error | undefined
   let lastStatus: number | undefined