From 7ff8da56dfb8e6ad0130e9dcbdb55f034c3ff530 Mon Sep 17 00:00:00 2001 From: creatixchu Date: Tue, 28 Jul 2026 16:49:52 +0800 Subject: [PATCH] fix(connection): require trustedHosts entries in canonical authority form A dangling colon (harness.internal:) or zero-padded port parses cleanly while WHATWG silently rewrites it, turning an intended exact-port grant into an any-port grant. Replace the character blacklist with a round-trip check: an entry must read back from parsing exactly as written (case aside), refusing the whole rewrite class at load. --- docs/config-catalog.md | 2 +- packages/client/connection/README.i18n.yaml | 4 +- packages/client/connection/README.md | 2 +- packages/client/connection/README.zh.md | 2 +- .../connection/src/api-request-trust.ts | 41 +++++++++++-------- packages/client/connection/src/index.ts | 2 +- .../tests/api-request-trust.spec.ts | 6 +++ 7 files changed, 35 insertions(+), 24 deletions(-) diff --git a/docs/config-catalog.md b/docs/config-catalog.md index 5a7f63c85f..878de38a19 100644 --- a/docs/config-catalog.md +++ b/docs/config-catalog.md @@ -283,7 +283,7 @@ export interface ConnectionConfig { * browser request whose Host is neither loopback nor listed here, so a * non-loopback (`0.0.0.0`) deployment must declare the names it is reached * by (the dsh CLI derives the machine's LAN IP literals itself). An entry - * that is not a bare authority fails the plugin load. + * that is not a bare, canonical authority fails the plugin load. */ trustedHosts?: string[] } diff --git a/packages/client/connection/README.i18n.yaml b/packages/client/connection/README.i18n.yaml index f0775848d3..c390223071 100644 --- a/packages/client/connection/README.i18n.yaml +++ b/packages/client/connection/README.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write packages/client/connection/README.md -README.md: 591e8361c1d28fab909bfe4a4f176fa1887edd93 -README.zh.md: bd772b2ab0f36abc8dbce35d30f55b40e53d0756 +README.md: 7e437e8fd81d1d57ead5ead64d2049111ad163dc +README.zh.md: 3615d5ea1be44e6da8e41fa17f645eb414d1ef3e diff --git a/packages/client/connection/README.md b/packages/client/connection/README.md index 591e8361c1..7e437e8fd8 100644 --- a/packages/client/connection/README.md +++ b/packages/client/connection/README.md @@ -6,7 +6,7 @@ Wire consumer layer: the client plugin's apply mounts `ctx.connection` (shared a ## /api browser-trust fence -The node half guards every request under `/api` before bridging (`src/api-request-trust.ts`). Requests without browser markers (no `Origin`, no `sec-fetch-site` — curl, tests, native clients) pass on any Host: without a browser there is no confused deputy, and such a sender forges every header anyway. For browser requests, the `Host` header must be a loopback authority or match a `trustedHosts` entry — exact on `host:port` entries, any port on port-less entries, both sides compared through WHATWG normalization (DNS-rebinding defense); an attached `Origin` must equal that authority, and an explicit `sec-fetch-site: cross-site` marker is refused. A `trustedHosts` entry that is not a bare `host[:port]` authority fails the plugin load loudly — WHATWG parsing would otherwise quietly authorize the hostname inside a typo like `harness.internal/path`. Failures answer plain 403 before any RPC dispatch. A non-loopback (`--host 0.0.0.0`) deployment therefore needs its serving authorities trusted: the dsh CLI derives the machine's LAN IP literals itself and its `--trusted-host` flag declares named ones, so `trustedHosts` in cordis.yml is for compositions the CLI does not boot. The fence is deliberately not an authentication layer — reachability policy stays with the webserver binding, and auth remains deferred work. Decision record: [the api browser-trust boundary Agent Note](../../../.agents/notes/implemented/architecture/2026-07-28-api-browser-trust-boundary.md). +The node half guards every request under `/api` before bridging (`src/api-request-trust.ts`). Requests without browser markers (no `Origin`, no `sec-fetch-site` — curl, tests, native clients) pass on any Host: without a browser there is no confused deputy, and such a sender forges every header anyway. For browser requests, the `Host` header must be a loopback authority or match a `trustedHosts` entry — exact on `host:port` entries, any port on port-less entries, both sides compared through WHATWG normalization (DNS-rebinding defense); an attached `Origin` must equal that authority, and an explicit `sec-fetch-site: cross-site` marker is refused. A `trustedHosts` entry that is not a bare, canonical `host[:port]` authority — one WHATWG parsing reads back exactly as written — fails the plugin load loudly: parsing would otherwise quietly authorize the hostname inside `harness.internal/path`, or broaden a dangling-colon or zero-padded port to an any-port grant. Failures answer plain 403 before any RPC dispatch. A non-loopback (`--host 0.0.0.0`) deployment therefore needs its serving authorities trusted: the dsh CLI derives the machine's LAN IP literals itself and its `--trusted-host` flag declares named ones, so `trustedHosts` in cordis.yml is for compositions the CLI does not boot. The fence is deliberately not an authentication layer — reachability policy stays with the webserver binding, and auth remains deferred work. Decision record: [the api browser-trust boundary Agent Note](../../../.agents/notes/implemented/architecture/2026-07-28-api-browser-trust-boundary.md). ## Keyless fixture diff --git a/packages/client/connection/README.zh.md b/packages/client/connection/README.zh.md index bd772b2ab0..3615d5ea1b 100644 --- a/packages/client/connection/README.zh.md +++ b/packages/client/connection/README.zh.md @@ -6,7 +6,7 @@ ## /api 浏览器信任栅栏 -node 半侧在桥接前守卫 `/api` 下的每个请求（`src/api-request-trust.ts`）。不带浏览器标记的请求（无 `Origin`、无 `sec-fetch-site`——curl、测试、原生客户端）在任何 Host 上都放行：没有浏览器就不存在"混淆代理人"，且这类发送方本就可以伪造任何请求头。对浏览器请求，`Host` 头必须是回环地址权威，或与某个 `trustedHosts` 条目匹配——带端口的 `host:port` 条目精确匹配，不带端口的条目匹配任意端口，两侧均经 WHATWG 归一化后比较（DNS rebinding 防御）；若带有 `Origin` 则必须与该权威完全一致；显式的 `sec-fetch-site: cross-site` 标记一律拒绝。不是纯 `host[:port]` 权威的 `trustedHosts` 条目会让插件加载大声失败——否则 WHATWG 解析会悄悄授权 `harness.internal/path` 这类笔误里的 hostname。失败在任何 RPC 分发之前以纯 403 应答。因此非回环（`--host 0.0.0.0`）部署需要让自己的服务权威被信任：dsh CLI 会自行推导本机的 LAN IP 字面量，其 `--trusted-host` flag 用于声明具名权威，所以 cordis.yml 中的 `trustedHosts` 面向 CLI 不参与引导的组合。这道栅栏刻意不承担认证职责——可达性策略归 webserver 绑定配置，认证仍是延期工作。决策记录：[api 浏览器信任边界 Agent Note](../../../.agents/notes/implemented/architecture/2026-07-28-api-browser-trust-boundary.md)。 +node 半侧在桥接前守卫 `/api` 下的每个请求（`src/api-request-trust.ts`）。不带浏览器标记的请求（无 `Origin`、无 `sec-fetch-site`——curl、测试、原生客户端）在任何 Host 上都放行：没有浏览器就不存在"混淆代理人"，且这类发送方本就可以伪造任何请求头。对浏览器请求，`Host` 头必须是回环地址权威，或与某个 `trustedHosts` 条目匹配——带端口的 `host:port` 条目精确匹配，不带端口的条目匹配任意端口，两侧均经 WHATWG 归一化后比较（DNS rebinding 防御）；若带有 `Origin` 则必须与该权威完全一致；显式的 `sec-fetch-site: cross-site` 标记一律拒绝。不是纯的、规范形 `host[:port]` 权威的 `trustedHosts` 条目——即 WHATWG 解析读回后与原文不完全一致的——会让插件加载大声失败：否则解析会悄悄授权 `harness.internal/path` 这类笔误里的 hostname，或把悬空冒号、补零端口放大成任意端口授权。失败在任何 RPC 分发之前以纯 403 应答。因此非回环（`--host 0.0.0.0`）部署需要让自己的服务权威被信任：dsh CLI 会自行推导本机的 LAN IP 字面量，其 `--trusted-host` flag 用于声明具名权威，所以 cordis.yml 中的 `trustedHosts` 面向 CLI 不参与引导的组合。这道栅栏刻意不承担认证职责——可达性策略归 webserver 绑定配置，认证仍是延期工作。决策记录：[api 浏览器信任边界 Agent Note](../../../.agents/notes/implemented/architecture/2026-07-28-api-browser-trust-boundary.md)。 ## 无密钥 fixture diff --git a/packages/client/connection/src/api-request-trust.ts b/packages/client/connection/src/api-request-trust.ts index ad2519d90b..57a8cb179c 100644 --- a/packages/client/connection/src/api-request-trust.ts +++ b/packages/client/connection/src/api-request-trust.ts @@ -42,30 +42,35 @@ function parseAuthority(authority: string): URL | undefined { /** * Assert one configured `trustedHosts` entry is a bare authority (`host` or - * `host:port`) and nothing else. WHATWG parsing would quietly read a hostname - * out of `harness.internal/path` or `user@harness.internal` — a typo must fail - * the load loudly instead of authorizing its hostname or being ignored until - * requests 403. The character test refuses every URL part beyond the authority - * (path, backslash path, query, fragment, userinfo) and all whitespace, which - * WHATWG trimming would otherwise strip silently; IPv6 brackets use none of - * them. + * `host:port`) in canonical form: it must survive WHATWG parsing unchanged + * (case aside). Anything parsing would silently rewrite is refused as a typo + * that must fail the load loudly instead of being ignored until requests 403 + * or quietly changing the grant: URL parts beyond the authority + * (`harness.internal/path`, `user@harness.internal` — which would authorize + * the embedded hostname), stripped whitespace, a dangling colon or + * zero-padded port (which would broaden an intended exact-port grant to every + * port), and non-canonical host spellings (`0x7f.0.0.1`, percent-encoding, + * unbracketed IPv6; IDN hosts are declared in punycode, the form the wire + * carries). * @param entry - the configured value, verbatim. */ export function assertTrustedAuthority(entry: string): void { - if (parseAuthority(entry) !== undefined && !/[/\\?#@\s]/.test(entry)) return + const entryUrl = parseAuthority(entry) + if (entryUrl !== undefined && canonicalAuthority(entry, entryUrl) === entry.toLowerCase()) return throw new Error(`client-connection: trustedHosts entry ${JSON.stringify(entry)} is not a bare host[:port] authority`) } /** - * Whether the parsed authority carries an explicit port: judged from URL - * parses under both special schemes (their default ports differ, so `:80` and - * `:443` still count as explicit), never from the raw string, where WHATWG - * trimming of stray whitespace would misread `host:port ` as port-less and - * broaden an exact-port grant to every port. + * Canonical form of a parsed authority: `hostname` when no port was written, + * else `hostname:port`. The port is judged from URL parses under both special + * schemes (their default ports differ, so `:80` and `:443` still count as + * explicit), never from the raw string, where WHATWG trimming would misread + * shapes like `host:port ` as port-less. */ -function hasExplicitPort(entry: string, entryUrl: URL): boolean { +function canonicalAuthority(entry: string, entryUrl: URL): string { // An authority that parsed under http cannot fail under https. - return entryUrl.port !== '' || new URL(`https://${entry}`).port !== '' + const port = entryUrl.port !== '' ? entryUrl.port : new URL(`https://${entry}`).port + return port === '' ? entryUrl.hostname : `${entryUrl.hostname}:${port}` } /** @@ -79,9 +84,9 @@ function isTrustedAuthority(hostUrl: URL, trustedHosts: readonly string[]): bool return trustedHosts.some((entry) => { const entryUrl = parseAuthority(entry) if (entryUrl === undefined) return false - return hasExplicitPort(entry, entryUrl) - ? entryUrl.host === hostUrl.host - : entryUrl.hostname === hostUrl.hostname + return canonicalAuthority(entry, entryUrl) === entryUrl.hostname + ? entryUrl.hostname === hostUrl.hostname + : entryUrl.host === hostUrl.host }) } diff --git a/packages/client/connection/src/index.ts b/packages/client/connection/src/index.ts index f37a64fb21..a649afda1a 100644 --- a/packages/client/connection/src/index.ts +++ b/packages/client/connection/src/index.ts @@ -24,7 +24,7 @@ export interface ConnectionConfig { * browser request whose Host is neither loopback nor listed here, so a * non-loopback (`0.0.0.0`) deployment must declare the names it is reached * by (the dsh CLI derives the machine's LAN IP literals itself). An entry - * that is not a bare authority fails the plugin load. + * that is not a bare, canonical authority fails the plugin load. */ trustedHosts?: string[] } diff --git a/packages/client/connection/tests/api-request-trust.spec.ts b/packages/client/connection/tests/api-request-trust.spec.ts index e3f1c91caf..2c608cc988 100644 --- a/packages/client/connection/tests/api-request-trust.spec.ts +++ b/packages/client/connection/tests/api-request-trust.spec.ts @@ -79,6 +79,12 @@ describe('isTrustedApiRequest', () => { for (const entry of ['harness.internal:3080 ', ' harness.internal', 'harness.internal:30\t80']) { expect(() => { assertTrustedAuthority(entry) }).toThrow(/not a bare host\[:port\] authority/) } + // WHATWG parsing would silently rewrite these — a dangling colon or + // zero-padded port would broaden an intended exact-port grant to every + // port, and non-canonical host spellings would not read back as written. + for (const entry of ['harness.internal:', '[::1]:', 'harness.internal:0080', '0x7f.0.0.1', '[0:0:0:0:0:0:0:1]']) { + expect(() => { assertTrustedAuthority(entry) }).toThrow(/not a bare host\[:port\] authority/) + } }) it('never lets stray whitespace broaden an exact-port entry to every port', () => {