fix: address codex review round 1

- Re-validate redirect targets through validateFetchUrl before following, so a same-origin Location carrying credentials (or a non-http(s)/over-long URL) cannot bypass the transport hygiene a direct request enforces. - Treat only DROPPED bytes as truncation: a body exactly at maxResponseBytes is no longer falsely flagged truncated (which emitted a spurious footer). - Honor the declared response charset: parse the Content-Type charset and decode with it (rejecting unsupported labels as WEB_UNSUPPORTED_CONTENT_TYPE) instead of always assuming UTF-8 and returning replacement characters. - Catalog the web seam vocabulary in docs/core-data-structures/web.md with type-equiv blocks + manifest entries, per the core-data-structures rule.
2026-08-15 21:04:50 +00:00 · 2026-06-25 15:28:15 +08:00
parent d01f5f73b7
commit 567519184b
7 changed files with 218 additions and 11 deletions
--- a/docs/core-data-structures/core.md
+++ b/docs/core-data-structures/core.md
@@ -22,6 +22,7 @@ Everything else is documented on a **sub-page**, not here. The rule that draws t
 | [bash.md](bash.md) | the bash executor seam: `BashExecRequest`/`Spec`, `BashRunResult`, background `BashTask`s |
 | [compaction.md](compaction.md) | the compaction seam: the `compact/*` session events, `CompactionResult`, the `CompactService` interface |
 | [subagent.md](subagent.md) | the subagent seam: the named-provider registry, `SubagentStartRequest`/`Result`/`Run`, the start-time-vs-runtime capability split |
+| [web.md](web.md) | the web access seam: `WebSearchRequest`/`Result`, `WebFetchRequest`/`Result`, `WebFetchBody`, provider/capability status, `WebErrorCode` |

 > Type definitions on this page are pasted **verbatim** from source and drift-checked by `pnpm run verify-type-equiv` (see [development.md](../development.md#documenting-types-verbatim-ts-type-equiv)). Inline JSDoc is omitted for readability; follow the source link for the full contracts.

--- a/docs/core-data-structures/web.md
+++ b/docs/core-data-structures/web.md
@@ -0,0 +1,119 @@
+# Web Access
+
+The web access seam — a [capability seam](../rfc/implemented/architecture/2026-06-24-web-capability-seam.md) that spans **two capabilities** (search and fetch) on one `ctx.web` service, split across packages: interface ([dsh-web](../../packages/web/web), `ctx.web` + the provider registries), implementations ([dsh-web-search-exa](../../packages/web/web-search-exa), [dsh-web-search-perplexity](../../packages/web/web-search-perplexity), [dsh-web-fetch-local](../../packages/web/web-fetch-local)), and consumer ([dsh-tool-web](../../packages/web/tool-web), the `web_search`/`web_fetch` tool schemas). Web is **one optional capability**, not part of the agent-loop spine — so its vocabulary lives here, not in [core.md](core.md). A search-provider swap does not change how the model asks for a query, and a fetch-implementation swap does not change how the model asks for a URL.
+
+Source: [`packages/web/web/src/types.ts`](../../packages/web/web/src/types.ts)
+
+## Why one seam for two capabilities
+
+Search and fetch share no request schema and no business logic, but they are deliberately one `ctx.web` middle layer: one provider-selection policy owner, one abort/error vocabulary, one product-facing "how this harness reaches the web" config surface. The cost is the parallel `searchX`/`fetchX` method pairs on the service; that parallelism is intentional, not a missed extraction. Providers register **capabilities** (a `WebSearchProvider` or `WebFetchProvider`), not tools; the model-facing names, schemas, prompt guidance, and presentation all live in the single `dsh-tool-web` consumer.
+
+## Search request and result
+
+The model-facing tool argument is just a `query`; `maxResults` is a consumer-owned bound (`dsh-tool-web`'s `WEB_SEARCH_MAX_RESULTS`, default `8`) passed through the seam and enforced on the way back — if a provider over-returns, the seam truncates `sources[]` and sets `truncated`.
+
+```ts type-equiv
+interface WebSearchRequest {
+  readonly query: string
+  /**
+   * Upper bound on returned sources; the seam truncates to it. Omitted = no
+   * bound. `dsh-tool-web` always sets it.
+   */
+  readonly maxResults?: number
+}
+```
+
+```ts type-equiv
+interface WebSearchResult {
+  readonly providerId: string
+  readonly query: string
+  readonly content?: string
+  readonly sources: readonly WebSearchSource[]
+  readonly truncated: boolean
+}
+```
+
+`content` is optional provider-generated answer text (Exa returns none; Perplexity returns a generated answer). `sources[]` is the portable citation surface. A source always has a `url`; `title`/`snippet`/`publishedAt` are optional because not every provider returns them — Perplexity citations may be URL-only, and forcing adapters to invent the rest would make the seam lie. `dsh-tool-web` renders `title ?? hostname(url)`.
+
+```ts type-equiv
+interface WebSearchSource {
+  readonly url: string
+  readonly title?: string
+  readonly snippet?: string
+  readonly publishedAt?: string
+}
+```
+
+## Fetch request and result
+
+```ts type-equiv
+interface WebFetchRequest {
+  readonly url: string
+  readonly timeoutMs?: number
+}
+```
+
+HTTP status is part of the fetched resource state, not automatically a failure: a successful network fetch of a `404`/`500` returns a `WebFetchResult` with the status code and a bounded decoded body. `url` is the final URL after allowed redirects. `WebError` is reserved for failures to safely retrieve or represent the resource.
+
+```ts type-equiv
+interface WebFetchResult {
+  readonly providerId: string
+  readonly url: string
+  readonly statusCode: number
+  readonly body: WebFetchBody
+  readonly truncated: boolean
+}
+```
+
+`WebFetchBody` is a **closed** discriminated union owned by `dsh-web` (not a merge-extensible map): the provider decodes the kind and `dsh-tool-web` renders it, so a new kind is a coordinated change across known packages, not a plugin extension. Consumers `switch` on `kind` ending in `default: assertNever(...)`, so adding a kind breaks compilation at every consumer until handled. Each arm stays its own object literal even where fields coincide today, leaving room for arm-specific fields later (a future `pdf` body's `pageCount`).
+
+```ts type-equiv
+type WebFetchBody =
+  | { readonly kind: 'html'; readonly content: string }
+  | { readonly kind: 'text'; readonly content: string }
+```
+
+## Provider and capability status
+
+A provider's `status()` is a cheap LOCAL check (credential presence, parseable config) and **must not make network calls**. It is an input to selection, not a health system.
+
+```ts type-equiv
+type WebProviderStatus =
+  | { readonly available: true }
+  | { readonly available: false; readonly reason: 'missing-credential' | 'misconfigured' }
+```
+
+The service aggregates provider status into a `WebCapabilityStatus`: whether the capability has a selected usable provider, or the broad category in which selection fails. It carries the winning `providerId` on the available branch but NOT the per-reason payload (the missing id, the ambiguous set) — that branchable detail lives in the thrown `WebError`, the surface callers route on, so the same fact never gets two homes that can disagree.
+
+```ts type-equiv
+type WebCapabilityStatus =
+  | { readonly available: true; readonly providerId: string }
+  | { readonly available: false; readonly reason: 'none' | 'configured-missing' | 'configured-unavailable' | 'ambiguous' }
+```
+
+Selection never depends on registration, config, or HMR order: a capability has an explicit provider id (config `searchProvider`/`fetchProvider`, or the matching env var feeding the same field), or auto-selects when exactly one usable provider is registered; multiple usable providers with no configured id is `ambiguous`, not first-wins.
+
+## Errors
+
+`WebError extends HarnessError` ([core.md](core.md) error taxonomy) with a stable `WebErrorCode`. `WEB_DUPLICATE_PROVIDER` is a registration-time programming error (the analogue of `LlmService`'s `DUPLICATE_ADAPTER`); the `WEB_PROVIDER_*` selection codes and the fetch transport codes are execution outcomes. `WEB_PROVIDER_ERROR` is the catch-all for a provider's own failure surfaced through the seam, including network/transport failure (DNS, connection refused, TLS).
+
+```ts type-equiv
+type WebErrorCode =
+  | 'WEB_PROVIDER_UNAVAILABLE'
+  | 'WEB_PROVIDER_CONFIGURED_MISSING'
+  | 'WEB_PROVIDER_CONFIGURED_UNAVAILABLE'
+  | 'WEB_PROVIDER_AMBIGUOUS'
+  | 'WEB_DUPLICATE_PROVIDER'
+  | 'WEB_INVALID_URL'
+  | 'WEB_BLOCKED_URL'
+  | 'WEB_REDIRECT_BLOCKED'
+  | 'WEB_FETCH_TOO_LARGE'
+  | 'WEB_FETCH_TIMEOUT'
+  | 'WEB_ABORTED'
+  | 'WEB_UNSUPPORTED_CONTENT_TYPE'
+  | 'WEB_PROVIDER_ERROR'
+```
+
+## The service
+
+`WebService` (`ctx.web`, defined in [`packages/web/web/src/index.ts`](../../packages/web/web/src/index.ts)) is a provider registry plus a provider-selecting execution surface, close to `LlmService`'s shape: `registerSearchProvider`/`registerFetchProvider` (duplicate ids throw `WEB_DUPLICATE_PROVIDER`, return disposers, emit `web/providers-change`), `searchStatus`/`fetchStatus` (derived, never stored), and `search`/`fetch` (resolve the provider at call time, throw a structured `WebError` when the capability cannot run). Providers issue requests with the platform-native `fetch` (Node 24), mirroring `dsh-llm-deepseek`; the `dsh-web-fetch-local` provider owns safe retrieval (http/https-only, credential rejection, byte/char/timeout/redirect caps, same-origin-only redirects with per-hop re-validation, charset decoding) while `dsh-tool-web` owns presentation (HTML→markdown). SSRF / private-network blocking is deferred (see the RFC) — until it lands, `web_fetch` must not be enabled where it can reach sensitive internal targets.
--- a/packages/web/web-fetch-local/src/index.ts
+++ b/packages/web/web-fetch-local/src/index.ts
@@ -18,7 +18,7 @@ export {
  LocalFetchProvider,
 } from './provider.ts'
 export type { LocalFetchLimits } from './provider.ts'
-export { classifyContentType, isSameOrigin, validateFetchUrl } from './policy.ts'
+export { classifyContentType, decoderForCharset, isSameOrigin, parseCharset, validateFetchUrl } from './policy.ts'
 export type { FetchableKind } from './policy.ts'

 /** Default `User-Agent`: an explicit product agent, never a browser disguise. */
--- a/packages/web/web-fetch-local/src/policy.ts
+++ b/packages/web/web-fetch-local/src/policy.ts
@@ -57,3 +57,29 @@ export function classifyContentType(contentType: string | null): FetchableKind |
  if (mime === 'application/json' || mime === 'application/xml' || mime.endsWith('+json') || mime.endsWith('+xml')) return 'text'
  return undefined
 }
+
+/**
+ * Extract the `charset` parameter from a response `Content-Type`, lower-cased,
+ * or `undefined` when absent. The provider feeds this label to `TextDecoder`
+ * so a non-UTF-8 response is decoded with its declared encoding rather than
+ * silently mangled into replacement characters.
+ */
+export function parseCharset(contentType: string | null): string | undefined {
+  const match = /;\s*charset\s*=\s*"?([^";]+)"?/i.exec(contentType ?? '')
+  return match?.[1]?.trim().toLowerCase()
+}
+
+/**
+ * Build a `TextDecoder` for the declared charset, falling back to UTF-8 when
+ * none is declared. Throws {@link WebError} `WEB_UNSUPPORTED_CONTENT_TYPE` when
+ * the label is present but not a charset `TextDecoder` recognizes — better to
+ * fail loudly than return mojibake.
+ */
+export function decoderForCharset(charset: string | undefined): TextDecoder {
+  if (charset === undefined) return new TextDecoder('utf-8')
+  try {
+    return new TextDecoder(charset)
+  } catch (error: unknown) {
+    throw new WebError(`unsupported charset "${charset}"`, 'WEB_UNSUPPORTED_CONTENT_TYPE', { cause: error })
+  }
+}
--- a/packages/web/web-fetch-local/src/provider.ts
+++ b/packages/web/web-fetch-local/src/provider.ts
@@ -21,7 +21,7 @@

 import { WebError } from '@deepseek-ai/dsh-web'
 import type { WebFetchBody, WebFetchProvider, WebFetchRequest, WebFetchResult, WebProviderStatus } from '@deepseek-ai/dsh-web'
-import { classifyContentType, isSameOrigin, validateFetchUrl } from './policy.ts'
+import { classifyContentType, decoderForCharset, isSameOrigin, parseCharset, validateFetchUrl } from './policy.ts'

 /** Resolved provider limits (the plugin's schemastery Config supplies defaults). */
 export interface LocalFetchLimits {
@@ -92,14 +92,18 @@ export class LocalFetchProvider implements WebFetchProvider {
          throw new WebError(`redirect response (HTTP ${response.status}) without a Location header`, 'WEB_PROVIDER_ERROR')
        }
        const target = resolveRedirect(location, currentUrl)
-        if (!isSameOrigin(target, currentUrl)) {
+        // Re-validate the target against the same transport hygiene a direct
+        // request gets: a redirect must not be a back door to a credentialed,
+        // non-http(s), or over-long URL that validateFetchUrl would reject.
+        const validatedTarget = validateFetchUrl(target.toString(), this.limits.maxUrlLength)
+        if (!isSameOrigin(validatedTarget, currentUrl)) {
          throw new WebError(
-            `cross-origin redirect to ${target.origin} is not followed automatically; retry against that URL directly`,
+            `cross-origin redirect to ${validatedTarget.origin} is not followed automatically; retry against that URL directly`,
            'WEB_REDIRECT_BLOCKED',
          )
        }
        await response.body?.cancel()
-        currentUrl = target
+        currentUrl = validatedTarget
        continue
      }

@@ -124,14 +128,18 @@ export class LocalFetchProvider implements WebFetchProvider {

  /** Read, byte-cap, classify, and decode the final response body. */
  private async readBody(response: Response, finalUrl: URL): Promise<WebFetchResult> {
-    const kind = classifyContentType(response.headers.get('content-type'))
+    const contentType = response.headers.get('content-type')
+    const kind = classifyContentType(contentType)
    if (kind === undefined) {
      await response.body?.cancel()
-      throw new WebError(`unsupported content type "${response.headers.get('content-type') ?? 'unknown'}"`, 'WEB_UNSUPPORTED_CONTENT_TYPE')
+      throw new WebError(`unsupported content type "${contentType ?? 'unknown'}"`, 'WEB_UNSUPPORTED_CONTENT_TYPE')
    }

+    // Resolve the decoder BEFORE reading the body so an unsupported charset
+    // fails without consuming the stream.
+    const decoder = decoderForCharset(parseCharset(contentType))
    const { bytes, truncatedByBytes } = await this.readCapped(response)
-    const decoded = new TextDecoder('utf-8').decode(bytes)
+    const decoded = decoder.decode(bytes)
    const truncatedByChars = decoded.length > this.limits.maxBodyChars
    const content = truncatedByChars ? decoded.slice(0, this.limits.maxBodyChars) : decoded
    const body: WebFetchBody = kind === 'html' ? { kind: 'html', content } : { kind: 'text', content }
@@ -173,7 +181,10 @@ export class LocalFetchProvider implements WebFetchProvider {
        const { done, value } = await reader.read()
        if (done) break
        const remaining = this.limits.maxResponseBytes - total
-        if (value.byteLength >= remaining) {
+        // Only DROPPED bytes count as truncation: a chunk that exactly fills the
+        // remaining capacity keeps all its bytes and we read on to observe EOF,
+        // so an exactly-at-cap body is not falsely flagged truncated.
+        if (value.byteLength > remaining) {
          chunks.push(value.subarray(0, remaining))
          total += remaining
          truncatedByBytes = true
--- a/packages/web/web-fetch-local/tests/fetch-local.spec.ts
+++ b/packages/web/web-fetch-local/tests/fetch-local.spec.ts
@@ -3,7 +3,7 @@ import { createServer, type IncomingMessage, type Server, type ServerResponse }
 import { AddressInfo } from 'node:net'
 import { Context } from 'cordis'
 import WebService from '@deepseek-ai/dsh-web'
-import { LocalFetchProvider, LOCAL_FETCH_PROVIDER_ID, classifyContentType, isSameOrigin, validateFetchUrl } from '@deepseek-ai/dsh-web-fetch-local'
+import { LocalFetchProvider, LOCAL_FETCH_PROVIDER_ID, classifyContentType, decoderForCharset, isSameOrigin, parseCharset, validateFetchUrl } from '@deepseek-ai/dsh-web-fetch-local'
 import type { LocalFetchLimits } from '@deepseek-ai/dsh-web-fetch-local'
 import * as fetchPlugin from '@deepseek-ai/dsh-web-fetch-local'

@@ -62,6 +62,19 @@ describe('policy helpers', () => {
    expect(isSameOrigin(new URL('https://a.com'), new URL('https://b.com'))).toBe(false)
    expect(isSameOrigin(new URL('http://a.com'), new URL('https://a.com'))).toBe(false)
  })
+
+  it('parses the charset parameter', () => {
+    expect(parseCharset('text/html; charset=UTF-8')).toBe('utf-8')
+    expect(parseCharset('text/plain; charset="iso-8859-1"')).toBe('iso-8859-1')
+    expect(parseCharset('text/plain')).toBeUndefined()
+    expect(parseCharset(null)).toBeUndefined()
+  })
+
+  it('builds a decoder for a charset and defaults to UTF-8', () => {
+    expect(decoderForCharset(undefined).encoding).toBe('utf-8')
+    expect(decoderForCharset('iso-8859-1').encoding).toBe('windows-1252')
+    expect(() => decoderForCharset('not-a-charset')).toThrow(expect.objectContaining({ code: 'WEB_UNSUPPORTED_CONTENT_TYPE' }))
+  })
 })

 describe('LocalFetchProvider success', () => {
@@ -109,6 +122,13 @@ describe('LocalFetchProvider caps', () => {
    expect(result.truncated).toBe(true)
  })

+  it('does not flag a body that exactly fills the byte cap as truncated', async () => {
+    handler = (_req, res) => { res.writeHead(200, { 'content-type': 'text/plain' }); res.end('abcd') }
+    const result = await provider({ maxResponseBytes: 4 }).fetch({ url: base })
+    expect(result.body.content).toBe('abcd')
+    expect(result.truncated).toBe(false)
+  })
+
  it('truncates a decoded body past the character cap', async () => {
    handler = (_req, res) => { res.writeHead(200, { 'content-type': 'text/plain' }); res.end('abcdefghij') }
    const result = await provider({ maxBodyChars: 3 }).fetch({ url: base })
@@ -133,6 +153,19 @@ describe('LocalFetchProvider caps', () => {
    const result = await provider().fetch({ url: base })
    expect(result.body.content).toBe('sized')
  })
+
+  it('decodes a non-UTF-8 declared charset', async () => {
+    // 0xE9 is "é" in ISO-8859-1; decoded as UTF-8 it would be a replacement char.
+    handler = (_req, res) => { res.writeHead(200, { 'content-type': 'text/plain; charset=iso-8859-1' }); res.end(Buffer.from([0x63, 0x61, 0x66, 0xE9])) }
+    const result = await provider().fetch({ url: base })
+    expect(result.body.content).toBe('café')
+  })
+
+  it('rejects an unsupported declared charset', async () => {
+    handler = (_req, res) => { res.writeHead(200, { 'content-type': 'text/plain; charset=not-a-charset' }); res.end('x') }
+    await expect(provider().fetch({ url: base }))
+      .rejects.toThrow(expect.objectContaining({ code: 'WEB_UNSUPPORTED_CONTENT_TYPE' }))
+  })
 })

 describe('LocalFetchProvider redirects', () => {
@@ -152,6 +185,13 @@ describe('LocalFetchProvider redirects', () => {
      .rejects.toThrow(expect.objectContaining({ code: 'WEB_REDIRECT_BLOCKED' }))
  })

+  it('re-validates a redirect target, rejecting same-origin credentials in the Location', async () => {
+    const { port } = server.address() as AddressInfo
+    handler = (_req, res) => { res.writeHead(302, { location: `http://user:pass@127.0.0.1:${port}/` }); res.end() }
+    await expect(provider().fetch({ url: base }))
+      .rejects.toThrow(expect.objectContaining({ code: 'WEB_BLOCKED_URL' }))
+  })
+
  it('rejects exceeding the redirect hop cap', async () => {
    handler = (req, res) => {
      const n = Number(new URL(req.url ?? '/', base).searchParams.get('n') ?? '0')
--- a/scripts/type-equiv.manifest.json
+++ b/scripts/type-equiv.manifest.json
@@ -48,6 +48,16 @@
    { "doc": "docs/core-data-structures/subagent.md", "symbol": "SubagentResult", "source": "packages/subagent/subagent/src/types.ts" },
    { "doc": "docs/core-data-structures/subagent.md", "symbol": "SubagentStopReasonMap", "source": "packages/subagent/subagent/src/types.ts" },
    { "doc": "docs/core-data-structures/subagent.md", "symbol": "SubagentRun", "source": "packages/subagent/subagent/src/types.ts" },
-    { "doc": "docs/core-data-structures/subagent.md", "symbol": "SubagentProvider", "source": "packages/subagent/subagent/src/types.ts" }
+    { "doc": "docs/core-data-structures/subagent.md", "symbol": "SubagentProvider", "source": "packages/subagent/subagent/src/types.ts" },
+
+    { "doc": "docs/core-data-structures/web.md", "symbol": "WebSearchRequest", "source": "packages/web/web/src/types.ts" },
+    { "doc": "docs/core-data-structures/web.md", "symbol": "WebSearchResult", "source": "packages/web/web/src/types.ts" },
+    { "doc": "docs/core-data-structures/web.md", "symbol": "WebSearchSource", "source": "packages/web/web/src/types.ts" },
+    { "doc": "docs/core-data-structures/web.md", "symbol": "WebFetchRequest", "source": "packages/web/web/src/types.ts" },
+    { "doc": "docs/core-data-structures/web.md", "symbol": "WebFetchResult", "source": "packages/web/web/src/types.ts" },
+    { "doc": "docs/core-data-structures/web.md", "symbol": "WebFetchBody", "source": "packages/web/web/src/types.ts" },
+    { "doc": "docs/core-data-structures/web.md", "symbol": "WebProviderStatus", "source": "packages/web/web/src/types.ts" },
+    { "doc": "docs/core-data-structures/web.md", "symbol": "WebCapabilityStatus", "source": "packages/web/web/src/types.ts" },
+    { "doc": "docs/core-data-structures/web.md", "symbol": "WebErrorCode", "source": "packages/web/web/src/types.ts" }
  ]
 }