From b241a79ee572d1b9c4991a9358bf215e359ad195 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Fri, 14 Aug 2026 15:59:02 -0700 Subject: [PATCH] fix(security): remove deleted /sidebar-chat endpoint from tunnel allowlist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TUNNEL_PATHS is the audited tunnel attack surface — its own comment says every addition widens it. '/sidebar-chat' stayed in the set after the endpoint was deleted with the chat-queue path, meaning any future route matching that path would have been silently tunnel-exposed. The set is now exactly the pair ceremony (/connect) and the scoped command endpoint (/command), and the dual-listener closed-set pin enforces that. Also repairs a pre-existing red pin in dual-listener.test.ts: v1.63.0.0 made the tunnel allowlist args-aware (canDispatchOverTunnel gained a second param) without updating the test — red on main since then, invisible because the free suite had no CI job. Co-Authored-By: Claude Fable 5 --- browse/src/server.ts | 1 - browse/test/dual-listener.test.ts | 12 +++++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/browse/src/server.ts b/browse/src/server.ts index fdbe15e78..fe99cee65 100644 --- a/browse/src/server.ts +++ b/browse/src/server.ts @@ -303,7 +303,6 @@ export function resolveConfigFromEnv(): Omit([ '/connect', '/command', - '/sidebar-chat', ]); /** diff --git a/browse/test/dual-listener.test.ts b/browse/test/dual-listener.test.ts index 9520fb13f..41df9e9c1 100644 --- a/browse/test/dual-listener.test.ts +++ b/browse/test/dual-listener.test.ts @@ -48,9 +48,12 @@ describe('Dual-listener surface types', () => { }); describe('Tunnel path allowlist', () => { - test('TUNNEL_PATHS is a closed set containing exactly /connect, /command, /sidebar-chat', () => { + test('TUNNEL_PATHS is a closed set containing exactly /connect, /command', () => { + // /sidebar-chat sat in this set long after the endpoint was deleted with + // the chat-queue path — a stale entry in the audited tunnel attack + // surface. The set is exactly the pair ceremony + command endpoint. const paths = extractSetContents(SERVER_SRC, 'TUNNEL_PATHS'); - expect(paths).toEqual(new Set(['/connect', '/command', '/sidebar-chat'])); + expect(paths).toEqual(new Set(['/connect', '/command'])); }); test('TUNNEL_PATHS does NOT contain bootstrap or admin paths', () => { @@ -220,7 +223,10 @@ describe('/command tunnel command allowlist', () => { 'return handleCommand(body, tokenInfo)' ); expect(commandBlock).toContain("surface === 'tunnel'"); - expect(commandBlock).toContain('canDispatchOverTunnel(body?.command)'); + // v1.63.0.0 made the allowlist args-aware (canDispatchOverTunnel gained a + // second param for --out denial); this pin was stale from then until the + // free suite got a CI job. + expect(commandBlock).toContain('canDispatchOverTunnel(body?.command, body?.args)'); expect(commandBlock).toContain('disallowed_command'); expect(commandBlock).toContain('is not allowed over the tunnel surface'); expect(commandBlock).toContain('status: 403');