diff --git a/.agents/README.md b/.agents/README.md index 2f0886c..f8d107f 100644 --- a/.agents/README.md +++ b/.agents/README.md @@ -27,3 +27,7 @@ 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) + +- [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-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/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..f33471f --- /dev/null +++ b/.agents/evidence/verify/2026-10-23-bound-http-control-review.md @@ -0,0 +1,17 @@ +# 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, 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 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. + +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 new file mode 100644 index 0000000..f2e429d --- /dev/null +++ b/.agents/notes/proposed/architecture/2026-10-23-bound-http-client.md @@ -0,0 +1,26 @@ +# 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. + +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. + +## 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..9968fe4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ ### 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. - 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..2c6fced 100644 --- a/README.md +++ b/README.md @@ -123,3 +123,21 @@ 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. 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 +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..8f06688 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -122,3 +122,15 @@ readEnv("sandbox_api_key", process.env); Apache-2.0。见 [LICENSE](LICENSE) 和 [NOTICE](NOTICE)。 Configuration storage uses one `storage` descriptor. / 配置存储统一使用 `storage` 字段。 + +通过提供方端口网关访问时，应传入包含完整代理前缀、沙箱和端口标识及查询参数 +的 URL。客户端保留原查询参数的字节，仅追加操作参数，禁止覆盖网关参数。 +所有重定向均会拒绝，包括同一域名下跳转到另一沙箱；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..333c90f 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; } @@ -48,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; @@ -83,16 +123,42 @@ 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(/\/+$/, ""); + // 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); + 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(/\/+$/, ""); 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", "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"); + 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,16 +200,25 @@ export class ServerClient { if (this.#closed) { throw new ConnectionError(`ServerClient is closed (${this.baseUrl})`); } - const url = new URL(path.replace(/^\//, ""), `${this.baseUrl}/`); + 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)) { - url.searchParams.set(key, String(value)); + 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(); + if (suffix) url.search += (url.search ? "&" : "?") + suffix; } 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; @@ -155,7 +230,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 +241,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..1eaf01b 100644 --- a/test/server-client.test.mjs +++ b/test/server-client.test.mjs @@ -146,3 +146,115 @@ 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', '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); + 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(); +}); + +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(); } +}); + +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(); } +});