From 7c309f7817d0556ec69ec5d731ce53d53c70b268 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Fri, 14 Aug 2026 19:30:50 -0700 Subject: [PATCH] fix(security): terminal-agent uses the shared PTY cookie parser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The /ws upgrade's cookie fallback hand-parsed the Cookie header inline — the fourth copy of the session-cookie parse, and the one that had already diverged from the others. Parsing now goes through extractPtyCookie; validation deliberately stays against the agent's own in-process validTokens map (the server's registry lives in a different process). The ws-handler pin test now pins the shared-parser call instead of the raw cookie-name literal. Co-Authored-By: Claude Fable 5 --- browse/src/terminal-agent.ts | 17 +++++++---------- browse/test/terminal-agent.test.ts | 6 ++++-- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/browse/src/terminal-agent.ts b/browse/src/terminal-agent.ts index b0862b209..4814c7508 100644 --- a/browse/src/terminal-agent.ts +++ b/browse/src/terminal-agent.ts @@ -26,6 +26,7 @@ import * as crypto from 'crypto'; import { writeSecureFile, mkdirSecure } from './file-permissions'; import { safeUnlink } from './error-handling'; import { writeAgentRecord, clearAgentRecord } from './terminal-agent-control'; +import { extractPtyCookie } from './pty-session-cookie'; const STATE_FILE = process.env.BROWSE_STATE_FILE || path.join(process.env.HOME || '/tmp', '.gstack', 'browse.json'); const PORT_FILE = path.join(path.dirname(STATE_FILE), 'terminal-port'); @@ -609,17 +610,13 @@ function buildServer() { } // Fallback: Cookie gstack_pty (legacy / non-browser callers). + // Parsing is shared with the server via extractPtyCookie; VALIDATION + // deliberately stays against the agent's own validTokens map — the + // server's registry lives in a different process. if (!token) { - const cookieHeader = req.headers.get('cookie') || ''; - for (const part of cookieHeader.split(';')) { - const [name, ...rest] = part.trim().split('='); - if (name === 'gstack_pty') { - const candidate = rest.join('=') || null; - if (candidate && validTokens.has(candidate)) { - token = candidate; - } - break; - } + const candidate = extractPtyCookie(req); + if (candidate && validTokens.has(candidate)) { + token = candidate; } } diff --git a/browse/test/terminal-agent.test.ts b/browse/test/terminal-agent.test.ts index 7793587c3..8dd54e200 100644 --- a/browse/test/terminal-agent.test.ts +++ b/browse/test/terminal-agent.test.ts @@ -121,9 +121,11 @@ describe('Source-level guard: terminal-agent', () => { test('validates the session token against an in-memory token set', () => { const wsHandler = AGENT_SRC.slice(AGENT_SRC.indexOf("if (url.pathname === '/ws')")); // Two transports: Sec-WebSocket-Protocol (preferred for browsers) and - // Cookie gstack_pty (fallback). Both verify against validTokens. + // the gstack_pty cookie fallback — parsing shared via extractPtyCookie + // (the hand-rolled parse here had drifted from the server's), validation + // still against the agent's own validTokens map. expect(wsHandler).toContain('sec-websocket-protocol'); - expect(wsHandler).toContain('gstack_pty'); + expect(wsHandler).toContain('extractPtyCookie'); expect(wsHandler).toContain('validTokens.has'); });