mirror of https://github.com/garrytan/gstack.git
fix(security): terminal-agent uses the shared PTY cookie parser
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 <noreply@anthropic.com>
This commit is contained in:
parent
5df153d0b1
commit
7c309f7817
|
|
@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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');
|
||||
});
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue