fix(browse): drop manual Sec-WebSocket-Protocol echo in terminal-agent /ws

Bun >= 1.3 auto-echoes the first offered subprotocol in server.upgrade(),
so the agent's explicit echo header produced a duplicate
Sec-WebSocket-Protocol response header. Strict WebSocket clients
(Chromium, python websockets) reject that handshake per RFC 6455, which
left the sidebar Terminal pane permanently disconnected: /pty-session
minted fine, but new WebSocket(url, ['gstack-pty.' + token]) never
completed.

Fix verified end-to-end on Bun 1.3.6 against a live agent: single
protocol header in the 101, handshake accepted, claude PTY spawns and
responds in the sidebar.

Adds a raw-socket regression test that counts Sec-WebSocket-Protocol
header lines in the upgrade response (Headers.get normalizes duplicates
away, so fetch-level assertions can't see this class of bug).

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Minseo Lee 2026-08-11 16:39:04 +09:00
parent 94993f7401
commit 93f08f2a34
2 changed files with 44 additions and 7 deletions

View File

@ -598,12 +598,10 @@ function buildServer() {
// first that matches a known token.
const protoHeader = req.headers.get('sec-websocket-protocol') || '';
let token: string | null = null;
let acceptedProtocol: string | null = null;
for (const raw of protoHeader.split(',').map(s => s.trim()).filter(Boolean)) {
const candidate = raw.startsWith('gstack-pty.') ? raw.slice('gstack-pty.'.length) : raw;
if (validTokens.has(candidate)) {
token = candidate;
acceptedProtocol = raw;
break;
}
}
@ -632,13 +630,13 @@ function buildServer() {
// sessionsById so /internal/restart and (Commit 3) re-attach
// lookups can find it.
const sessionId = validTokens.get(token) ?? null;
// No explicit Sec-WebSocket-Protocol echo: Bun >= 1.3 auto-echoes the
// first offered protocol in the 101 response, so setting the header
// here produced a DUPLICATE header — strict clients (Chromium, python
// websockets) reject the handshake per RFC 6455 and the sidebar
// terminal could never connect. Verified on Bun 1.3.6.
const upgraded = server.upgrade(req, {
data: { cookie: token, sessionId },
// Echo the protocol back so the browser accepts the upgrade.
// Required when the client sends Sec-WebSocket-Protocol — the
// server MUST select one of the offered protocols, otherwise
// the browser closes the connection immediately.
...(acceptedProtocol ? { headers: { 'Sec-WebSocket-Protocol': acceptedProtocol } } : {}),
});
return upgraded ? undefined : new Response('upgrade failed', { status: 500 });
}

View File

@ -227,6 +227,45 @@ describe('terminal-agent: PTY round-trip via real WebSocket (Cookie auth)', () =
expect(resp.headers.get('sec-websocket-protocol')).toBe(`gstack-pty.${token}`);
});
test('upgrade response contains exactly ONE Sec-WebSocket-Protocol header', async () => {
// RFC 6455: the server MUST select at most one subprotocol. Bun >= 1.3
// auto-echoes the first offered protocol in server.upgrade(), so a
// manual echo on top of that produced TWO Sec-WebSocket-Protocol
// headers — and strict clients (Chromium, python websockets) reject the
// handshake, leaving the sidebar terminal permanently disconnected.
//
// Headers.get() normalizes duplicates away, so this test handshakes
// over a raw socket and counts header lines in the response head.
const token = 'dup-proto-token-must-be-at-least-seventeen-chars';
await grantToken(token);
const head = await new Promise<string>((resolve, reject) => {
const req =
'GET /ws HTTP/1.1\r\n' +
`Host: 127.0.0.1:${agentPort}\r\n` +
'Connection: Upgrade\r\n' +
'Upgrade: websocket\r\n' +
'Sec-WebSocket-Version: 13\r\n' +
'Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\n' +
`Sec-WebSocket-Protocol: gstack-pty.${token}\r\n` +
'Origin: chrome-extension://test-extension-id\r\n' +
'\r\n';
let buf = '';
const socket = require('net').connect(agentPort, '127.0.0.1', () => socket.write(req));
socket.setTimeout(5000, () => { socket.destroy(); reject(new Error('handshake timeout')); });
socket.on('data', (chunk: Buffer) => {
buf += chunk.toString('utf8');
const end = buf.indexOf('\r\n\r\n');
if (end !== -1) { socket.destroy(); resolve(buf.slice(0, end)); }
});
socket.on('error', reject);
});
expect(head).toContain('101');
const protoLines = head.split('\r\n').filter(l => l.toLowerCase().startsWith('sec-websocket-protocol:'));
expect(protoLines).toEqual([`Sec-WebSocket-Protocol: gstack-pty.${token}`]);
});
test('Sec-WebSocket-Protocol auth: rejects unknown token even with valid Origin', async () => {
const resp = await fetch(`http://127.0.0.1:${agentPort}/ws`, {
headers: {