From fd6fa0daf997458ced72058589d9aafd6b82631e Mon Sep 17 00:00:00 2001 From: anycodes <21079031+anycodes@users.noreply.github.com> Date: Fri, 23 Oct 2026 09:06:59 +0000 Subject: [PATCH 1/4] fix: bind application HTTP requests to sandbox endpoints --- .agents/README.md | 2 + .../verify/2026-10-23-bound-http-client.md | 18 ++++++ .../2026-10-23-bound-http-client.md | 24 ++++++++ CHANGELOG.md | 2 + README.md | 15 +++++ README.zh-CN.md | 11 ++++ src/server-client.ts | 53 ++++++++++++++--- test/server-client.test.mjs | 59 +++++++++++++++++++ 8 files changed, 175 insertions(+), 9 deletions(-) create mode 100644 .agents/evidence/verify/2026-10-23-bound-http-client.md create mode 100644 .agents/notes/proposed/architecture/2026-10-23-bound-http-client.md diff --git a/.agents/README.md b/.agents/README.md index 2f0886c..b52aca5 100644 --- a/.agents/README.md +++ b/.agents/README.md @@ -27,3 +27,5 @@ state. A local/runtime result is not cloud acceptance or package publication. - [Current configuration contract](notes/implemented/architecture/2026-10-22-remove-legacy-compatibility.md) · [verification](evidence/verify/2026-10-22-remove-legacy-compatibility.md) - [Compatibility cleanup remote acceptance](evidence/verify/2026-10-22-remove-legacy-acceptance.md) + +- [Bound HTTP client](notes/proposed/architecture/2026-10-23-bound-http-client.md) · [verification](evidence/verify/2026-10-23-bound-http-client.md) diff --git a/.agents/evidence/verify/2026-10-23-bound-http-client.md b/.agents/evidence/verify/2026-10-23-bound-http-client.md new file mode 100644 index 0000000..08eeb87 --- /dev/null +++ b/.agents/evidence/verify/2026-10-23-bound-http-client.md @@ -0,0 +1,18 @@ +# Bound HTTP client verification + +Date: 2026-10-23 +Result: PASS (local scope) + +Scope: src/server-client.ts, test/server-client.test.mjs, bilingual README and changelog on feat/template-workflows-20261023, based on 513a711fb839ae4df179a03e0cd74b4cf4836669. +Environment: Node.js 22.22.2, npm lockfile installation, Python 3.11 with the current task's editable in-sandbox server installed. + +- PASS: npm ci. +- PASS: npm run typecheck. +- PASS: npm test, all 15 configured tests, including the real Python local server loop and four new HTTP binding regressions. +- PASS: npm pack --dry-run; built ESM, TypeScript declarations, configuration contract, both README files and license/notice included. +- PASS: node examples/agent-loop.mjs; real local health, shell stdout 4, file round trip answer=4 and server SIGTERM cleanup. +- The first suite invocation lacked the installed Python interpreter on PATH and failed its two real-runtime tests. Re-running with that installed interpreter passed both. The HTTP regressions passed in both runs. +- No cloud sandbox, external resource or deployment was created. Loopback servers stopped and example process exited. These checks do not establish cloud provider acceptance or package publication. +- Remote Node20/22 CI and required PR review remain separate delivery gates. + +Issue: https://github.com/Alibaba-Agent-Sandbox/nodejs-sdk/issues/9. diff --git a/.agents/notes/proposed/architecture/2026-10-23-bound-http-client.md b/.agents/notes/proposed/architecture/2026-10-23-bound-http-client.md new file mode 100644 index 0000000..65cc845 --- /dev/null +++ b/.agents/notes/proposed/architecture/2026-10-23-bound-http-client.md @@ -0,0 +1,24 @@ +# Decision: Bind HTTP requests to one application endpoint + +Status: proposed + +## Problem +Resolving relative operations against a signed gateway URL discarded its query and could misplace the proxy prefix. Default fetch redirects could send application credentials to a different sandbox. Diagnostics and object inspection exposed URL query credentials. + +## Decision +Preserve the complete prefix and exact original query bytes, append only new parameters and reject parameter replacement. Refuse every redirect, URL userinfo, fragments, non-HTTP URLs, prefix escapes and authority/transport header overrides. Keep the private endpoint separate from the query-free public baseUrl. Separate gatewayHeaders from the application's token and other headers; reject name collisions instead of silently replacing credentials. + +## API Design +ServerClientOptions adds gatewayHeaders. token remains the application's bearer token. baseUrl reports the query-free origin and path. Failed connections omit underlying error text that may contain signed URLs. The HTTP-only public SDK scope remains unchanged. Per-operation signed paths require the provider's operation resolver; this prefix client does not infer such signatures. + +## Alternatives considered +Origin-only redirect restrictions still allow credentials to reach another sandbox on a shared gateway. Reconstructing all query parameters changes signature bytes. Silent authentication retries or token precedence obscure credential ownership. + +## Dependencies +The in-sandbox HTTP server is owned by https://github.com/Alibaba-Agent-Sandbox/python-sdk. The project workflow design is tracked in https://github.com/Alibaba-Agent-Sandbox/nodejs-sdk/issues/9. + +## Test Strategy +Use real loopback HTTP servers to check wire paths/query bytes, credential separation and single-request redirect refusal. Reject malformed endpoints, path traversal, overlapping headers and query replacement before I/O. Run the full configured suite, typecheck, package validation and actual local Python server example. + +## Acceptance criteria +All configured Node tests and type checks pass. Proxy paths and signature bytes survive each operation. Redirects and authority overrides cannot move credentials. No cloud or interpreter API is introduced; verification distinguishes local runtime behavior from cloud acceptance. diff --git a/CHANGELOG.md b/CHANGELOG.md index 12cda4c..a637f0e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ ### Fixed +- Preserve sandbox proxy prefixes and signed query bytes; reject redirects, URL userinfo, escaping paths and credential/header collisions. Separate gateway headers from the application token and omit signed query credentials from diagnostics. + - Preserve caller Python source configuration in the local runtime example; document usable source-package installation and validate real runtime behavior on Node20/22 CI. Clarify configuration-name exports and retry compatibility. - Match the Python configuration schema by leaving the installable `e2b` catalog provider id unreserved while preserving all built-in/core reservations. diff --git a/README.md b/README.md index eec3f29..c5d5d26 100644 --- a/README.md +++ b/README.md @@ -123,3 +123,18 @@ readEnv("sandbox_api_key", process.env); Apache-2.0. See [LICENSE](LICENSE) and [NOTICE](NOTICE). Configuration storage uses one `storage` descriptor. / 配置存储统一使用 `storage` 字段。 + +For a provider port gateway, pass the exact sandbox/port URL including its proxy +prefix and query. The client preserves existing query bytes and appends operation +parameters without replacing gateway parameters. All redirects are rejected, +including redirects to another sandbox on the same origin. URL userinfo and +request paths that escape the bound prefix are rejected. + +Use `token` for the application service token and `gatewayHeaders` for the +provider gateway's headers. `headers` contains other application headers. Header +names must not overlap between groups; an existing `Authorization` header cannot +be combined with `token`. Authority and transport headers cannot be overridden. +The public `baseUrl` and connection errors omit query credentials. Never pass a +provider control-plane API key as the application token. This client accepts +prefix-bound HTTP URLs; operation-specific signed path bindings must use the provider +adapter's operation transport. This prefix client does not accept those bindings. diff --git a/README.zh-CN.md b/README.zh-CN.md index c9c2a47..c06718c 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -122,3 +122,14 @@ readEnv("sandbox_api_key", process.env); Apache-2.0。见 [LICENSE](LICENSE) 和 [NOTICE](NOTICE)。 Configuration storage uses one `storage` descriptor. / 配置存储统一使用 `storage` 字段。 + +通过提供方端口网关访问时，应传入包含完整代理前缀、沙箱和端口标识及查询参数 +的 URL。客户端保留原查询参数的字节，仅追加操作参数，禁止覆盖网关参数。 +所有重定向均会拒绝，包括同一域名下跳转到另一沙箱；URL 用户信息及越出绑定 +前缀的操作路径也会拒绝。 + +`token` 用于应用服务令牌，`gatewayHeaders` 用于平台网关请求头，`headers` +用于其他应用请求头。各组请求头名称不能重叠；已有 `Authorization` 请求头时 +不能再传入 `token`。不能覆盖 Host 等寻址及传输请求头。公开 `baseUrl` 和连接 +错误省略查询凭据。不要把平台控制面 API Key 作为应用令牌传入。本客户端使用 +绑定代理前缀的 HTTP URL；按操作签名的路径绑定需要使用提供方适配器的逐操作传输接口，不能传入本前缀客户端。 diff --git a/src/server-client.ts b/src/server-client.ts index ea5f349..5b8fb19 100644 --- a/src/server-client.ts +++ b/src/server-client.ts @@ -12,6 +12,8 @@ export interface ServerClientOptions { token?: string | null; /** Extra headers on every request. */ headers?: Readonly>; + /** Gateway authentication is separate from the application's bearer token. */ + gatewayHeaders?: Readonly>; /** Default per-request timeout in seconds. */ timeout?: number; } @@ -83,16 +85,35 @@ export class ServerClient { readonly files: ServerFilesApi; readonly process: ServerProcessApi; + #endpoint: URL; #token: string | undefined; #headers: Record; #closed = false; #controllers = new Set(); constructor(baseUrl: string, options: ServerClientOptions = {}) { - this.baseUrl = baseUrl.replace(/\/+$/, ""); + let endpoint: URL; + try { endpoint = new URL(baseUrl); } catch { throw new TypeError("Invalid sandbox server URL"); } + if (!["http:", "https:"].includes(endpoint.protocol) || endpoint.username || endpoint.password || endpoint.hash) { + throw new TypeError("Sandbox server URL requires HTTP(S), without userinfo or fragment"); + } + this.#endpoint = endpoint; + // Keep signed query credentials out of object inspection and connection errors. + this.baseUrl = endpoint.origin + endpoint.pathname.replace(/\/+$/, ""); this.timeout = options.timeout ?? 30; this.#token = options.token ?? undefined; - this.#headers = { ...(options.headers ?? {}) }; + const headers = new Headers(); + for (const group of [options.headers ?? {}, options.gatewayHeaders ?? {}]) { + for (const [name, value] of Object.entries(group)) { + if (["host", ":authority", "connection", "transfer-encoding", "content-length", "proxy-authorization"].includes(name.toLowerCase())) { + throw new TypeError("Authority and transport headers cannot be overridden"); + } + if (headers.has(name)) throw new TypeError("Application and gateway headers must not overlap"); + headers.set(name, value); + } + } + if (this.#token && headers.has("authorization")) throw new TypeError("Application token conflicts with an existing authentication header"); + this.#headers = Object.fromEntries(headers); this.files = new ServerFilesApi(this); this.process = new ServerProcessApi(this); } @@ -134,11 +155,22 @@ export class ServerClient { if (this.#closed) { throw new ConnectionError(`ServerClient is closed (${this.baseUrl})`); } - const url = new URL(path.replace(/^\//, ""), `${this.baseUrl}/`); + let segments: string[]; + try { segments = path.replace(/^\//, "").split("/").map(decodeURIComponent); } + catch { throw new TypeError("Invalid sandbox operation path"); } + if (!path.startsWith("/") || path.startsWith("//") || /[?#\\]/.test(path) || segments.some(segment => !segment || segment === "." || segment === ".." || /[\\/]/.test(segment))) { + throw new TypeError("Sandbox operation path must stay within the bound server prefix"); + } + const url = new URL(this.#endpoint); + url.pathname = url.pathname.replace(/\/+$/, "") + path; if (options.params) { + const appended = new URLSearchParams(); for (const [key, value] of Object.entries(options.params)) { - url.searchParams.set(key, String(value)); + if (url.searchParams.has(key)) throw new TypeError("Operation parameters must not replace gateway query parameters"); + appended.append(key, String(value)); } + const suffix = appended.toString(); + if (suffix) url.search += (url.search ? "&" : "?") + suffix; } const headers = this.authHeaders(); let body: string | undefined; @@ -155,7 +187,7 @@ export class ServerClient { ]); let response: Response; try { - response = await fetch(url, { method, headers, body, signal }); + response = await fetch(url, { method, headers, body, signal, redirect: "manual" }); } catch (err) { if (this.#closed || controller.signal.aborted) { throw new ConnectionError(`ServerClient is closed (${this.baseUrl})`); @@ -166,14 +198,17 @@ export class ServerClient { `Timed out talking to the sandbox server at ${this.baseUrl}`, ); } - const detail = err instanceof Error ? err.message : String(err); - throw new ConnectionError( - `Could not reach the sandbox server at ${this.baseUrl}: ${detail}`, - ); + throw new ConnectionError(`Could not reach the sandbox server at ${this.baseUrl}`); } finally { this.#controllers.delete(controller); } + if (response.status >= 300 && response.status < 400) { + await response.body?.cancel(); + throw new ServerAPIError("Sandbox server redirects are not allowed", { + statusCode: response.status, errorType: "BindingError", endpoint, + }); + } const text = await response.text(); let parsed: unknown = null; if (text) { diff --git a/test/server-client.test.mjs b/test/server-client.test.mjs index f237193..4f73c06 100644 --- a/test/server-client.test.mjs +++ b/test/server-client.test.mjs @@ -146,3 +146,62 @@ test("HTTP errors raise ServerAPIError and a closed port is unhealthy", async () await closed.close(); await assert.rejects(() => closed.process.shell("true"), ConnectionError); }); + +test("bound proxy prefix and signed query bytes survive operation joins", async () => { + const hits = []; + const server = await listen(async (req, res) => { + hits.push({ url: req.url, app: req.headers.authorization, gateway: req.headers['x-gateway-token'] }); + res.end(JSON.stringify({ status: 'ok', content_base64: '' })); + }); + const client = new ServerClient(`http://127.0.0.1:${server.port}/proxy/sandbox-123/9000/?sig=a%2Fb%20c`, { + token: 'application-token', gatewayHeaders: { 'X-Gateway-Token': 'gateway-token' }, + }); + try { + assert.equal(await client.health(), true); + await client.files.download('/tmp/a b'); + assert.equal(hits[0].url, '/proxy/sandbox-123/9000/health?sig=a%2Fb%20c'); + assert.equal(hits[1].url, '/proxy/sandbox-123/9000/files/download-stream?sig=a%2Fb%20c&path=%2Ftmp%2Fa+b'); + assert.equal(hits[0].app, 'Bearer application-token'); + assert.equal(hits[0].gateway, 'gateway-token'); + assert.ok(!client.baseUrl.includes('sig=')); + await assert.rejects(client.send('x', 'GET', '/health', { params: { sig: 'replacement' } }), TypeError); + } finally { await client.close(); await server.close(); } +}); + +test("userinfo, authority overrides and overlapping credentials are rejected before a request", () => { + for (const url of ['http://user:secret@localhost:9000', 'file:///tmp/server', 'http://localhost:9000/#fragment']) { + assert.throws(() => new ServerClient(url), TypeError); + } + for (const name of ['Host', ':authority', 'Proxy-Authorization', 'Content-Length']) { + assert.throws(() => new ServerClient('http://localhost:9000', { headers: { [name]: 'bad' } }), TypeError); + } + assert.throws(() => new ServerClient('http://localhost:9000', { token: 'app', gatewayHeaders: { authorization: 'gateway' } }), TypeError); + assert.throws(() => new ServerClient('http://localhost:9000', { headers: { 'X-Token': 'app' }, gatewayHeaders: { 'x-token': 'gateway' } }), TypeError); +}); + +test("redirects cannot move even same-origin credentials to a different sandbox", async () => { + const hits = []; + const server = await listen(async (req, res) => { + hits.push(req.url); + res.statusCode = 307; + res.setHeader('location', '/proxy/another-sandbox/9000/health?stolen=1'); + res.end(); + }); + const client = new ServerClient(`http://127.0.0.1:${server.port}/proxy/owned/9000?sig=secret`, { token: 'app' }); + try { + await assert.rejects(client.send('health', 'GET', '/health'), error => error instanceof ServerAPIError && error.statusCode === 307 && !String(error).includes('secret')); + assert.equal(hits.length, 1); + for (const path of ['//foreign.test/x', '/../other', '/%2e%2e/other', '/health?query', '/a%2fb', '/a\\b']) { + await assert.rejects(client.send('x', 'GET', path), TypeError); + } + assert.equal(hits.length, 1); + } finally { await client.close(); await server.close(); } +}); + +test("connection errors and object inspection do not disclose signed URL queries", async () => { + const { inspect } = await import('node:util'); + const client = new ServerClient('http://127.0.0.1:1/proxy/owned?token=signed-secret'); + assert.ok(!inspect(client).includes('signed-secret')); + await assert.rejects(client.send('health', 'GET', '/health'), error => error instanceof ConnectionError && !String(error).includes('signed-secret')); + await client.close(); +}); From f67462b08b456a579259a46ca0c3cc7634064cd6 Mon Sep 17 00:00:00 2001 From: anycodes <21079031+anycodes@users.noreply.github.com> Date: Fri, 23 Oct 2026 09:12:06 +0000 Subject: [PATCH 2/4] fix: reject forwarded authority overrides in HTTP bindings --- src/server-client.ts | 4 ++-- test/server-client.test.mjs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/server-client.ts b/src/server-client.ts index 5b8fb19..8f312f1 100644 --- a/src/server-client.ts +++ b/src/server-client.ts @@ -105,7 +105,7 @@ export class ServerClient { const headers = new Headers(); for (const group of [options.headers ?? {}, options.gatewayHeaders ?? {}]) { for (const [name, value] of Object.entries(group)) { - if (["host", ":authority", "connection", "transfer-encoding", "content-length", "proxy-authorization"].includes(name.toLowerCase())) { + if (["host", ":authority", "forwarded", "x-forwarded-host", "x-forwarded-proto", "x-forwarded-port", "connection", "transfer-encoding", "content-length", "proxy-authorization"].includes(name.toLowerCase())) { throw new TypeError("Authority and transport headers cannot be overridden"); } if (headers.has(name)) throw new TypeError("Application and gateway headers must not overlap"); @@ -175,7 +175,7 @@ export class ServerClient { const headers = this.authHeaders(); let body: string | undefined; if (options.jsonBody !== undefined) { - headers["Content-Type"] = headers["Content-Type"] ?? "application/json"; + headers["content-type"] = headers["content-type"] ?? "application/json"; body = JSON.stringify(options.jsonBody); } const timeoutSeconds = options.timeout ?? this.timeout; diff --git a/test/server-client.test.mjs b/test/server-client.test.mjs index 4f73c06..fd287f7 100644 --- a/test/server-client.test.mjs +++ b/test/server-client.test.mjs @@ -172,7 +172,7 @@ test("userinfo, authority overrides and overlapping credentials are rejected bef for (const url of ['http://user:secret@localhost:9000', 'file:///tmp/server', 'http://localhost:9000/#fragment']) { assert.throws(() => new ServerClient(url), TypeError); } - for (const name of ['Host', ':authority', 'Proxy-Authorization', 'Content-Length']) { + for (const name of ['Host', ':authority', 'Forwarded', 'X-Forwarded-Host', 'X-Forwarded-Proto', 'X-Forwarded-Port', 'Proxy-Authorization', 'Content-Length']) { assert.throws(() => new ServerClient('http://localhost:9000', { headers: { [name]: 'bad' } }), TypeError); } assert.throws(() => new ServerClient('http://localhost:9000', { token: 'app', gatewayHeaders: { authorization: 'gateway' } }), TypeError); From f89eac77743fb8a129691b459a74ae5b9d781a46 Mon Sep 17 00:00:00 2001 From: anycodes <21079031+anycodes@users.noreply.github.com> Date: Fri, 23 Oct 2026 09:54:31 +0000 Subject: [PATCH 3/4] fix: reject encoded controls before sandbox URL normalization --- .agents/README.md | 2 + .../2026-10-23-bound-http-control-review.md | 16 ++++++ .../2026-10-23-bound-http-client.md | 2 + CHANGELOG.md | 2 + README.md | 5 +- README.zh-CN.md | 3 +- src/server-client.ts | 55 ++++++++++++++++--- test/server-client.test.mjs | 40 ++++++++++++++ 8 files changed, 116 insertions(+), 9 deletions(-) create mode 100644 .agents/evidence/verify/2026-10-23-bound-http-control-review.md diff --git a/.agents/README.md b/.agents/README.md index b52aca5..f8d107f 100644 --- a/.agents/README.md +++ b/.agents/README.md @@ -29,3 +29,5 @@ state. A local/runtime result is not cloud acceptance or package publication. - [Compatibility cleanup remote acceptance](evidence/verify/2026-10-22-remove-legacy-acceptance.md) - [Bound HTTP client](notes/proposed/architecture/2026-10-23-bound-http-client.md) · [verification](evidence/verify/2026-10-23-bound-http-client.md) + +- [HTTP binding control-character review](evidence/verify/2026-10-23-bound-http-control-review.md) diff --git a/.agents/evidence/verify/2026-10-23-bound-http-control-review.md b/.agents/evidence/verify/2026-10-23-bound-http-control-review.md new file mode 100644 index 0000000..ae4e655 --- /dev/null +++ b/.agents/evidence/verify/2026-10-23-bound-http-control-review.md @@ -0,0 +1,16 @@ +# HTTP binding control-character adversarial review + +Date: 2026-10-23 +Result: PASS (local scope) + +Scope: raw/recursively encoded URL and operation validation in src/server-client.ts, two new regressions and associated documentation on the task branch after f67462b. +Environment: Node.js 22.22.2, existing locked installation, Python 3.11 with the current task server installed for the configured real-runtime tests. + +The independent review found that WHATWG pathname assignment removes TAB/CR/LF before normalizing dot segments: a path containing TAB followed by two dots could escape a sandbox proxy prefix and carry its token/signature to another sandbox. Validation now runs before URL parsing/assignment, recursively rejects C0/DEL, dot traversal and encoded separators, and refuses recursively equivalent or duplicated gateway query keys. + +- PASS: npm test, all 17 tests, including the actual Python runtime loop and real loopback gateway regression. Attack paths cause zero HTTP requests; the positive health request reaches only the owned sandbox with unchanged signature and expected application token. +- PASS: npm run typecheck. +- PASS: git diff --check. +- No cloud sandbox or external resource was created. Loopback servers and clients closed. This is local transport validation, not cloud acceptance or remote CI/review. + +Refs https://github.com/Alibaba-Agent-Sandbox/nodejs-sdk/pull/10 and https://github.com/Alibaba-Agent-Sandbox/nodejs-sdk/issues/9. The earlier PR CI result does not cover this new head; rerun Node20/22 CI and complete independent review before merge. diff --git a/.agents/notes/proposed/architecture/2026-10-23-bound-http-client.md b/.agents/notes/proposed/architecture/2026-10-23-bound-http-client.md index 65cc845..f2e429d 100644 --- a/.agents/notes/proposed/architecture/2026-10-23-bound-http-client.md +++ b/.agents/notes/proposed/architecture/2026-10-23-bound-http-client.md @@ -8,6 +8,8 @@ Resolving relative operations against a signed gateway URL discarded its query a ## Decision Preserve the complete prefix and exact original query bytes, append only new parameters and reject parameter replacement. Refuse every redirect, URL userinfo, fragments, non-HTTP URLs, prefix escapes and authority/transport header overrides. Keep the private endpoint separate from the query-free public baseUrl. Separate gatewayHeaders from the application's token and other headers; reject name collisions instead of silently replacing credentials. +Validation runs on raw base URLs and operation paths before WHATWG URL parsing or pathname assignment. Bounded recursive decoding rejects C0/DEL, encoded separators and traversal at every layer; query keys cannot collide after recursive decoding. This prevents URL normalization from silently moving a credential-bearing request into another sandbox. + ## API Design ServerClientOptions adds gatewayHeaders. token remains the application's bearer token. baseUrl reports the query-free origin and path. Failed connections omit underlying error text that may contain signed URLs. The HTTP-only public SDK scope remains unchanged. Per-operation signed paths require the provider's operation resolver; this prefix client does not infer such signatures. diff --git a/CHANGELOG.md b/CHANGELOG.md index a637f0e..9968fe4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ ### Fixed +- Reject control characters before URL normalization and recursively encoded namespace escapes/query-key collisions before an authenticated request can reach another sandbox. + - Preserve sandbox proxy prefixes and signed query bytes; reject redirects, URL userinfo, escaping paths and credential/header collisions. Separate gateway headers from the application token and omit signed query credentials from diagnostics. - Preserve caller Python source configuration in the local runtime example; document usable source-package installation and validate real runtime behavior on Node20/22 CI. Clarify configuration-name exports and retry compatibility. diff --git a/README.md b/README.md index c5d5d26..2c6fced 100644 --- a/README.md +++ b/README.md @@ -128,7 +128,10 @@ For a provider port gateway, pass the exact sandbox/port URL including its proxy prefix and query. The client preserves existing query bytes and appends operation parameters without replacing gateway parameters. All redirects are rejected, including redirects to another sandbox on the same origin. URL userinfo and -request paths that escape the bound prefix are rejected. +request paths that escape the bound prefix are rejected. Raw or recursively +encoded control characters, path separators and dot traversal are validated +before URL normalization. Duplicate or recursively equivalent gateway query +keys cannot be introduced by the base URL or an operation. Use `token` for the application service token and `gatewayHeaders` for the provider gateway's headers. `headers` contains other application headers. Header diff --git a/README.zh-CN.md b/README.zh-CN.md index c06718c..8f06688 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -126,7 +126,8 @@ Configuration storage uses one `storage` descriptor. / 配置存储统一使用 通过提供方端口网关访问时，应传入包含完整代理前缀、沙箱和端口标识及查询参数 的 URL。客户端保留原查询参数的字节，仅追加操作参数，禁止覆盖网关参数。 所有重定向均会拒绝，包括同一域名下跳转到另一沙箱；URL 用户信息及越出绑定 -前缀的操作路径也会拒绝。 +前缀的操作路径也会拒绝。URL 归一化之前即检查原始或多层编码的控制字符、 +路径分隔符与点路径；基址和操作都不能引入重复或多层编码后等价的网关查询键。 `token` 用于应用服务令牌，`gatewayHeaders` 用于平台网关请求头，`headers` 用于其他应用请求头。各组请求头名称不能重叠；已有 `Authorization` 请求头时 diff --git a/src/server-client.ts b/src/server-client.ts index 8f312f1..4852230 100644 --- a/src/server-client.ts +++ b/src/server-client.ts @@ -50,6 +50,44 @@ function isRecord(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); } +function decodedBindingValue(value: string): string { + for (let depth = 0; depth < 8; depth += 1) { + if (/[\u0000-\u001f\u007f]/.test(value)) throw new TypeError("Control characters are not allowed in sandbox bindings"); + let decoded: string; + try { decoded = decodeURIComponent(value); } + catch { throw new TypeError("Invalid sandbox binding encoding"); } + if (decoded === value) return value; + value = decoded; + } + throw new TypeError("Sandbox binding is excessively encoded"); +} + +function validateBindingPath(path: string, operation: boolean): void { + // Validate before WHATWG URL parsing/setters can remove TAB/CR/LF or dots. + decodedBindingValue(path); + const segments = path.split("/"); + if ((operation && (!path.startsWith("/") || path.startsWith("//"))) || /[?#\\]/.test(path)) { + throw new TypeError("Sandbox operation path must stay within the bound server prefix"); + } + for (let index = 0; index < segments.length; index += 1) { + const segment = decodedBindingValue(segments[index]); + if (segment === "." || segment === ".." || /[\\/]/.test(segment) || + (operation && index > 0 && !segment)) { + throw new TypeError("Sandbox operation path must stay within the bound server prefix"); + } + } +} + +function queryBindingKeys(url: URL): Set { + const keys = new Set(); + for (const key of url.searchParams.keys()) { + const decoded = decodedBindingValue(key); + if (keys.has(decoded)) throw new TypeError("Duplicate sandbox gateway query parameters are not allowed"); + keys.add(decoded); + } + return keys; +} + function errorFromResponse(status: number, body: unknown, endpoint: string): ServerAPIError { const data = isRecord(body) ? body : {}; const fromBody = data.error; @@ -92,11 +130,16 @@ export class ServerClient { #controllers = new Set(); constructor(baseUrl: string, options: ServerClientOptions = {}) { + decodedBindingValue(baseUrl); + const rawPath = /^https?:\/\/[^/?#]+([^?#]*)/i.exec(baseUrl)?.[1]; + if (rawPath === undefined || /\\/.test(baseUrl)) throw new TypeError("Invalid sandbox server URL"); + validateBindingPath(rawPath, false); let endpoint: URL; try { endpoint = new URL(baseUrl); } catch { throw new TypeError("Invalid sandbox server URL"); } if (!["http:", "https:"].includes(endpoint.protocol) || endpoint.username || endpoint.password || endpoint.hash) { throw new TypeError("Sandbox server URL requires HTTP(S), without userinfo or fragment"); } + queryBindingKeys(endpoint); this.#endpoint = endpoint; // Keep signed query credentials out of object inspection and connection errors. this.baseUrl = endpoint.origin + endpoint.pathname.replace(/\/+$/, ""); @@ -155,18 +198,16 @@ export class ServerClient { if (this.#closed) { throw new ConnectionError(`ServerClient is closed (${this.baseUrl})`); } - let segments: string[]; - try { segments = path.replace(/^\//, "").split("/").map(decodeURIComponent); } - catch { throw new TypeError("Invalid sandbox operation path"); } - if (!path.startsWith("/") || path.startsWith("//") || /[?#\\]/.test(path) || segments.some(segment => !segment || segment === "." || segment === ".." || /[\\/]/.test(segment))) { - throw new TypeError("Sandbox operation path must stay within the bound server prefix"); - } + validateBindingPath(path, true); const url = new URL(this.#endpoint); url.pathname = url.pathname.replace(/\/+$/, "") + path; if (options.params) { const appended = new URLSearchParams(); + const keys = queryBindingKeys(url); for (const [key, value] of Object.entries(options.params)) { - if (url.searchParams.has(key)) throw new TypeError("Operation parameters must not replace gateway query parameters"); + const decoded = decodedBindingValue(key); + if (keys.has(decoded)) throw new TypeError("Operation parameters must not replace gateway query parameters"); + keys.add(decoded); appended.append(key, String(value)); } const suffix = appended.toString(); diff --git a/test/server-client.test.mjs b/test/server-client.test.mjs index fd287f7..3dab750 100644 --- a/test/server-client.test.mjs +++ b/test/server-client.test.mjs @@ -205,3 +205,43 @@ test("connection errors and object inspection do not disclose signed URL queries await assert.rejects(client.send('health', 'GET', '/health'), error => error instanceof ConnectionError && !String(error).includes('signed-secret')); await client.close(); }); + +test("control characters and recursively encoded paths cannot escape a real sandbox gateway", async () => { + const hits = []; + const server = await listen(async (req, res) => { + hits.push({ url: req.url, token: req.headers.authorization }); + res.end(JSON.stringify({ status: 'ok' })); + }); + const prefix = `http://127.0.0.1:${server.port}/proxy/owned/9000`; + const client = new ServerClient(prefix + '?sig=fixture', { token: 'application-token' }); + try { + const attacks = [ + '/\t../\t../victim/9000/health', '/\r../\n../victim/9000/health', + '/%09../%09../victim/9000/health', '/%2509../%2509../victim/9000/health', + '/%252e%252e/%252e%252e/victim/9000/health', '/a%252fb', '/a%255cb', + '/\x00health', '/%00health', '/%2500health', '/\x7fhealth', '/%257fhealth', + ]; + for (const path of attacks) { + await assert.rejects(client.send('health', 'GET', path), TypeError, JSON.stringify(path)); + } + assert.deepEqual(hits, []); + assert.equal(await client.health(), true); + assert.deepEqual(hits, [{ url: '/proxy/owned/9000/health?sig=fixture', token: 'Bearer application-token' }]); + for (const path of attacks) assert.throws(() => new ServerClient(prefix + path + '?sig=fixture'), TypeError); + assert.deepEqual(hits.map(hit => hit.url), ['/proxy/owned/9000/health?sig=fixture']); + } finally { await client.close(); await server.close(); } +}); + +test("raw URL controls and recursively equivalent gateway query keys fail before requests", async () => { + for (const url of [ + 'https://gate\tway.invalid/proxy/owned', 'https://gateway.invalid/proxy/owned\n', + 'https://gateway.invalid/proxy/%2500owned', 'https://gateway.invalid/proxy/%257fowned', + 'https://gateway.invalid/proxy/owned?sig=one&sig=two', + 'https://gateway.invalid/proxy/owned?sig=one&%2573ig=two', + ]) assert.throws(() => new ServerClient(url), TypeError); + const client = new ServerClient('https://gateway.invalid/proxy/owned?sig=fixture'); + try { + await assert.rejects(client.send('health', 'GET', '/health', { params: { '%73ig': 'override' } }), TypeError); + await assert.rejects(client.send('health', 'GET', '/health', { params: { path: 'one', '%70ath': 'two' } }), TypeError); + } finally { await client.close(); } +}); From 8f4b5d46b715576e6f8a53eadb87193023079626 Mon Sep 17 00:00:00 2001 From: anycodes <21079031+anycodes@users.noreply.github.com> Date: Fri, 23 Oct 2026 09:56:05 +0000 Subject: [PATCH 4/4] fix: preserve opaque signed query values during binding checks --- .../verify/2026-10-23-bound-http-control-review.md | 5 +++-- src/server-client.ts | 4 +++- test/server-client.test.mjs | 13 +++++++++++++ 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/.agents/evidence/verify/2026-10-23-bound-http-control-review.md b/.agents/evidence/verify/2026-10-23-bound-http-control-review.md index ae4e655..f33471f 100644 --- a/.agents/evidence/verify/2026-10-23-bound-http-control-review.md +++ b/.agents/evidence/verify/2026-10-23-bound-http-control-review.md @@ -3,12 +3,13 @@ Date: 2026-10-23 Result: PASS (local scope) -Scope: raw/recursively encoded URL and operation validation in src/server-client.ts, two new regressions and associated documentation on the task branch after f67462b. +Scope: raw/recursively encoded URL and operation validation in src/server-client.ts, three new regressions and associated documentation on the task branch after f67462b. Environment: Node.js 22.22.2, existing locked installation, Python 3.11 with the current task server installed for the configured real-runtime tests. The independent review found that WHATWG pathname assignment removes TAB/CR/LF before normalizing dot segments: a path containing TAB followed by two dots could escape a sandbox proxy prefix and carry its token/signature to another sandbox. Validation now runs before URL parsing/assignment, recursively rejects C0/DEL, dot traversal and encoded separators, and refuses recursively equivalent or duplicated gateway query keys. -- PASS: npm test, all 17 tests, including the actual Python runtime loop and real loopback gateway regression. Attack paths cause zero HTTP requests; the positive health request reaches only the owned sandbox with unchanged signature and expected application token. +- PASS: npm test, all 18 tests, including the actual Python runtime loop and real loopback gateway regression. Attack paths cause zero HTTP requests; the positive health request reaches only the owned sandbox with unchanged signature and expected application token. +- PASS: an independent review confirmed the original control-character bypass is blocked and identified an opaque-query overvalidation regression. The follow-up preserves query values such as `sig=100%25&opaque=%2525` byte-for-byte; only path/key semantics are recursively decoded. A real HTTP positive regression passed. - PASS: npm run typecheck. - PASS: git diff --check. - No cloud sandbox or external resource was created. Loopback servers and clients closed. This is local transport validation, not cloud acceptance or remote CI/review. diff --git a/src/server-client.ts b/src/server-client.ts index 4852230..333c90f 100644 --- a/src/server-client.ts +++ b/src/server-client.ts @@ -130,7 +130,9 @@ export class ServerClient { #controllers = new Set(); constructor(baseUrl: string, options: ServerClientOptions = {}) { - decodedBindingValue(baseUrl); + // Query values are opaque signatures: validate raw controls here, and + // recursively inspect only the path and query keys below. + if (/[\u0000-\u001f\u007f]/.test(baseUrl)) throw new TypeError("Control characters are not allowed in sandbox bindings"); const rawPath = /^https?:\/\/[^/?#]+([^?#]*)/i.exec(baseUrl)?.[1]; if (rawPath === undefined || /\\/.test(baseUrl)) throw new TypeError("Invalid sandbox server URL"); validateBindingPath(rawPath, false); diff --git a/test/server-client.test.mjs b/test/server-client.test.mjs index 3dab750..1eaf01b 100644 --- a/test/server-client.test.mjs +++ b/test/server-client.test.mjs @@ -245,3 +245,16 @@ test("raw URL controls and recursively equivalent gateway query keys fail before await assert.rejects(client.send('health', 'GET', '/health', { params: { path: 'one', '%70ath': 'two' } }), TypeError); } finally { await client.close(); } }); + +test("opaque signed query values containing literal percent bytes stay unchanged", async () => { + const hits = []; + const server = await listen(async (req, res) => { + hits.push(req.url); + res.end(JSON.stringify({ status: 'ok' })); + }); + const client = new ServerClient(`http://127.0.0.1:${server.port}/proxy/owned/9000?sig=100%25&opaque=%2525`, { token: 'application-token' }); + try { + assert.equal(await client.health(), true); + assert.deepEqual(hits, ['/proxy/owned/9000/health?sig=100%25&opaque=%2525']); + } finally { await client.close(); await server.close(); } +});