diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ef241dd1e..594da396f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -238,6 +238,14 @@ jobs: working-directory: packages/core run: pnpm build + # codev-types is a runtime dependency of codev (server-utils imports its + # wire constants at module load), so the install-verification install must + # include its tarball too — else `afx --help` crashes at boot with + # "Cannot find module '@cluesmith/codev-types'". + - name: Build types package + working-directory: packages/types + run: pnpm build + - name: Build package working-directory: packages/codev run: pnpm build @@ -250,11 +258,15 @@ jobs: working-directory: packages/sdk run: pnpm pack + - name: Pack types tarball + working-directory: packages/types + run: pnpm pack + - name: Pack tarball working-directory: packages/codev run: pnpm pack - name: Verify install from tarball working-directory: packages/codev - run: node scripts/verify-install.mjs cluesmith-codev-*.tgz ../core/cluesmith-codev-core-*.tgz ../sdk/cluesmith-codev-sdk-*.tgz + run: node scripts/verify-install.mjs cluesmith-codev-*.tgz ../core/cluesmith-codev-core-*.tgz ../sdk/cluesmith-codev-sdk-*.tgz ../types/cluesmith-codev-types-*.tgz diff --git a/.gitignore b/.gitignore index cbedf550a..02738e963 100644 --- a/.gitignore +++ b/.gitignore @@ -48,6 +48,11 @@ test-results/ packages/codev/dashboard-dist/ apps/web/dist/ +# three.js vendored at build time from the `three` devDependency (copy-three.mjs); +# copied into templates/vendor/ and shipped in the npm tarball, not committed. +packages/codev/templates/vendor/three.module.js +packages/codev/templates/vendor/three-*.js + # Checklister runtime state (per-project JSON files) codev/checklists/*.json diff --git a/apps/vscode/src/__tests__/terminal-adapter.test.ts b/apps/vscode/src/__tests__/terminal-adapter.test.ts index ee485ddc4..c4d3e0db2 100644 --- a/apps/vscode/src/__tests__/terminal-adapter.test.ts +++ b/apps/vscode/src/__tests__/terminal-adapter.test.ts @@ -41,7 +41,7 @@ vi.mock('ws', () => { closed = false; sent: unknown[] = []; private handlers: Record void>> = {}; - constructor(public url: string) { FakeWebSocket.instances.push(this); } + constructor(public url: string, public protocols?: string | string[]) { FakeWebSocket.instances.push(this); } on(event: string, cb: (...args: unknown[]) => void): this { (this.handlers[event] ||= []).push(cb); return this; @@ -64,7 +64,12 @@ vi.mock('@cluesmith/codev-sdk/escape-buffer', () => ({ }, })); -vi.mock('@cluesmith/codev-types', () => ({ FRAME_CONTROL: 0x00, FRAME_DATA: 0x01 })); +vi.mock('@cluesmith/codev-types', () => ({ + FRAME_CONTROL: 0x00, + FRAME_DATA: 0x01, + terminalWsProtocols: (key: string | null | undefined) => + (key ? ['codev.tower.v1', `codev-key.${key}`] : undefined), +})); const FRAME_CONTROL = 0x00; const FRAME_DATA = 0x01; @@ -127,12 +132,33 @@ function makeAdapter() { return { pty, writes }; } +/** Build an adapter with an explicit auth key (default makeAdapter uses null). */ +function makeAdapterWithKey(authKey: string | null) { + const pty = new (CodevPseudoterminal as unknown as new ( + url: string, authKey: string | null, ch: unknown, + ) => { open(d: unknown): void }) ('ws://localhost:4100/x', authKey, fakeOutputChannel()); + pty.open(undefined); + return WebSocket.instances[WebSocket.instances.length - 1] as unknown as { protocols?: string | string[] }; +} + beforeEach(() => { vi.useFakeTimers(); WebSocket.instances.length = 0; hoisted.escapeBufferCount = 0; }); +describe('WS subprotocol auth (advisory GHSA-xvjp-7748-v88v)', () => { + it('offers the marker + codev-key token when an auth key is present', () => { + const sock = makeAdapterWithKey('SECRETKEY'); + expect(sock.protocols).toEqual(['codev.tower.v1', 'codev-key.SECRETKEY']); + }); + + it('offers no subprotocol when there is no auth key', () => { + const sock = makeAdapterWithKey(null); + expect(sock.protocols).toBeUndefined(); + }); +}); + describe('PIR #936 — adapter-owned reconnect loop', () => { it('emits one backed-off notice per close, capping the delay at 30s', () => { const { writes } = makeAdapter(); diff --git a/apps/vscode/src/commands/tunnel.ts b/apps/vscode/src/commands/tunnel.ts index 993e9a875..1a56cf701 100644 --- a/apps/vscode/src/commands/tunnel.ts +++ b/apps/vscode/src/commands/tunnel.ts @@ -31,7 +31,7 @@ export async function disconnectTunnel(connectionManager: ConnectionManager): Pr }, 'Disconnect', ); - if (choice !== 'Disconnect') return; + if (choice !== 'Disconnect') {return;} await client.signalTunnel('disconnect'); vscode.window.showInformationMessage('Codev: Tower deregistered from Codev Cloud'); diff --git a/apps/vscode/src/connection-manager.ts b/apps/vscode/src/connection-manager.ts index 537b47a25..4b8f3aabf 100644 --- a/apps/vscode/src/connection-manager.ts +++ b/apps/vscode/src/connection-manager.ts @@ -237,6 +237,7 @@ export class ConnectionManager { this.scheduleReconnect(); } }, + () => this.auth.getKeySync(), ); this.sse.onEvent((type, data) => { this.sseEventEmitter.fire({ type, data }); diff --git a/apps/vscode/src/sse-client.ts b/apps/vscode/src/sse-client.ts index faab88919..91c508419 100644 --- a/apps/vscode/src/sse-client.ts +++ b/apps/vscode/src/sse-client.ts @@ -1,4 +1,5 @@ import * as vscode from 'vscode'; +import { TOWER_KEY_HEADER } from '@cluesmith/codev-types'; export type SSEListener = (eventType: string, data: string) => void; @@ -19,6 +20,7 @@ export class SSEClient { private baseUrl: string, private outputChannel: vscode.OutputChannel, private onDisconnect: () => void, + private getAuthKey: () => string | null = () => null, ) {} /** @@ -61,9 +63,12 @@ export class SSEClient { private async startSSE(url: string): Promise { try { - const response = await fetch(url, { - headers: { 'Accept': 'text/event-stream' }, - }); + // Request authentication (advisory GHSA-xvjp-7748-v88v): /api/events is a + // key-required route; SSE via fetch can carry the codev-tower-key header. + const headers: Record = { 'Accept': 'text/event-stream' }; + const key = this.getAuthKey(); + if (key) { headers[TOWER_KEY_HEADER] = key; } + const response = await fetch(url, { headers }); if (!response.ok || !response.body) { this.log('WARN', `SSE connection failed: ${response.status}`); diff --git a/apps/vscode/src/terminal-adapter.ts b/apps/vscode/src/terminal-adapter.ts index 5fd02a0ba..9447a8798 100644 --- a/apps/vscode/src/terminal-adapter.ts +++ b/apps/vscode/src/terminal-adapter.ts @@ -1,6 +1,6 @@ import * as vscode from 'vscode'; import WebSocket from 'ws'; -import { FRAME_CONTROL, FRAME_DATA, type ControlMessage } from '@cluesmith/codev-types'; +import { FRAME_CONTROL, FRAME_DATA, terminalWsProtocols, type ControlMessage } from '@cluesmith/codev-types'; import { EscapeBuffer } from '@cluesmith/codev-sdk/escape-buffer'; import { BackoffController, classifyUpgradeError } from '@cluesmith/codev-sdk/reconnect-policy'; @@ -180,7 +180,9 @@ export class CodevPseudoterminal implements vscode.Pseudoterminal { const url = this.connectUrl(); this.log('INFO', `Connecting to ${url}`); - const socket = new WebSocket(url); + // Request authentication (advisory GHSA-xvjp-7748-v88v): the shared key + // travels as a Sec-WebSocket-Protocol subprotocol, validated at the upgrade. + const socket = new WebSocket(url, terminalWsProtocols(this.authKey)); this.ws = socket; this.ws.binaryType = 'arraybuffer'; @@ -194,10 +196,9 @@ export class CodevPseudoterminal implements vscode.Pseudoterminal { // Wipe any in-progress retry notice before replayed buffer / normal // output resumes, so it doesn't orphan in scrollback (#1001). this.clearReconnectNotice(); - // Send auth via control message (not query param) - if (this.authKey) { - this.sendControl({ type: 'ping', payload: { auth: this.authKey } }); - } + // Auth is now carried by the Sec-WebSocket-Protocol subprotocol and + // validated at the upgrade (advisory GHSA-xvjp-7748-v88v), so no in-band + // auth frame is sent here. // Sync Tower's PTY to the dimensions VSCode reported. Without this, // the PTY stays at node-pty's 80×24 default until a manual resize, // which makes Claude Code's TUI render its input box mid-screen and diff --git a/apps/web/__tests__/TabBar.test.tsx b/apps/web/__tests__/TabBar.test.tsx index 95a98bc17..165005dab 100644 --- a/apps/web/__tests__/TabBar.test.tsx +++ b/apps/web/__tests__/TabBar.test.tsx @@ -4,6 +4,7 @@ import { TabBar, TAB_ICONS } from '../src/components/TabBar.js'; import type { Tab } from '../src/hooks/useTabs.js'; vi.mock('../src/lib/api.js', () => ({ + getWebKey: () => null, deleteTab: vi.fn(() => Promise.resolve()), })); diff --git a/apps/web/__tests__/Terminal.clipboard.test.tsx b/apps/web/__tests__/Terminal.clipboard.test.tsx index 23ebc49a5..637bdc7bc 100644 --- a/apps/web/__tests__/Terminal.clipboard.test.tsx +++ b/apps/web/__tests__/Terminal.clipboard.test.tsx @@ -59,6 +59,7 @@ vi.mock('@xterm/addon-web-links', () => ({ const mockUploadPasteImage = vi.fn(); vi.mock('../src/lib/api.js', () => ({ uploadPasteImage: (...args: unknown[]) => mockUploadPasteImage(...args), + getWebKey: () => null, })); // Mock WebSocket as a class diff --git a/apps/web/__tests__/analytics.test.tsx b/apps/web/__tests__/analytics.test.tsx index 65fa36182..b7d5c426b 100644 --- a/apps/web/__tests__/analytics.test.tsx +++ b/apps/web/__tests__/analytics.test.tsx @@ -15,6 +15,7 @@ import type { AnalyticsResponse } from '../src/lib/api.js'; const mockFetchAnalytics = vi.fn<(range: string, refresh?: boolean) => Promise>(); vi.mock('../src/lib/api.js', () => ({ + getWebKey: () => null, fetchAnalytics: (...args: unknown[]) => mockFetchAnalytics(...(args as [string, boolean?])), })); diff --git a/apps/web/__tests__/useOverview.stability.test.ts b/apps/web/__tests__/useOverview.stability.test.ts index ad1f5d1af..b19a12ccb 100644 --- a/apps/web/__tests__/useOverview.stability.test.ts +++ b/apps/web/__tests__/useOverview.stability.test.ts @@ -31,6 +31,7 @@ const mockFetchOverview = vi.fn<() => Promise>(); const mockRefreshOverview = vi.fn<() => Promise>(); vi.mock('../src/lib/api.js', () => ({ + getWebKey: () => null, fetchOverview: (...args: unknown[]) => mockFetchOverview(...(args as [])), refreshOverview: (...args: unknown[]) => mockRefreshOverview(...(args as [])), getSSEEventsUrl: () => 'http://localhost:0/api/events', diff --git a/apps/web/__tests__/useSSE.reconnect.test.ts b/apps/web/__tests__/useSSE.reconnect.test.ts index d333b0e4f..a809e85c1 100644 --- a/apps/web/__tests__/useSSE.reconnect.test.ts +++ b/apps/web/__tests__/useSSE.reconnect.test.ts @@ -4,30 +4,44 @@ * Verifies that when the SSE connection receives a message (e.g. after Tower * restarts and sends a "connected" event), the polling hooks immediately * re-fetch data instead of waiting for the next poll interval. + * + * useSSE streams via fetch + ReadableStream (not EventSource) so it can send the + * codev-tower-key header (advisory GHSA-xvjp-7748-v88v). This test mocks fetch to + * return a controllable stream and drives SSE frames into it. */ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { renderHook, act } from '@testing-library/react'; import type { DashboardState, OverviewData } from '../src/lib/api.js'; -// Capture EventSource instances so we can simulate SSE messages -let eventSourceInstances: Array<{ onmessage: ((ev: MessageEvent) => void) | null; close: () => void }> = []; - -class MockEventSource { - static readonly CONNECTING = 0; - static readonly OPEN = 1; - static readonly CLOSED = 2; - onmessage: ((ev: MessageEvent) => void) | null = null; - onerror: ((ev: Event) => void) | null = null; - onopen: ((ev: Event) => void) | null = null; - readyState = 1; - close = vi.fn(() => { this.readyState = MockEventSource.CLOSED; }); - constructor(_url: string) { - eventSourceInstances.push(this); - } +// Each fetch() call to the SSE endpoint creates a connection we can push frames +// into and observe aborting (the fetch-stream analogue of an EventSource). +interface MockSSEConnection { + controller: ReadableStreamDefaultController | null; + aborted: boolean; + abort: ReturnType; } - -// Override the EventSource stub from setup.ts with our instrumented mock -(globalThis as Record).EventSource = MockEventSource; +let connections: MockSSEConnection[] = []; +const encoder = new TextEncoder(); + +const mockFetch = vi.fn((_url: string, opts?: { signal?: AbortSignal }) => { + const conn: MockSSEConnection = { controller: null, aborted: false, abort: vi.fn() }; + connections.push(conn); + const body = new ReadableStream({ + start(controller) { + conn.controller = controller; + const signal = opts?.signal; + if (signal) { + signal.addEventListener('abort', () => { + conn.aborted = true; + conn.abort(); + try { controller.error(new DOMException('aborted', 'AbortError')); } catch { /* already closed */ } + }); + } + }, + }); + return Promise.resolve({ ok: true, body } as unknown as Response); +}); +(globalThis as Record).fetch = mockFetch; // Mock api module const mockFetchState = vi.fn<() => Promise>(); @@ -39,6 +53,7 @@ vi.mock('../src/lib/api.js', () => ({ fetchOverview: (...args: unknown[]) => mockFetchOverview(...(args as [])), refreshOverview: (...args: unknown[]) => mockRefreshOverview(...(args as [])), getSSEEventsUrl: () => 'http://localhost:0/api/events', + getWebKey: () => null, })); const MOCK_STATE: DashboardState = { @@ -56,18 +71,28 @@ const MOCK_OVERVIEW: OverviewData = { architects: [], }; +/** Push an SSE `data:` frame into every open connection's stream. */ function simulateSSEMessage(data: Record = { type: 'connected' }): void { - for (const es of eventSourceInstances) { - if (es.onmessage) { - es.onmessage(new MessageEvent('message', { data: JSON.stringify(data) })); + const frame = encoder.encode(`data: ${JSON.stringify(data)}\n\n`); + for (const conn of connections) { + if (conn.controller && !conn.aborted) { + conn.controller.enqueue(frame); } } } +/** Simulate the server closing the stream (→ useSSE schedules a reconnect). */ +function endStream(conn: MockSSEConnection): void { + if (conn.controller && !conn.aborted) { + try { conn.controller.close(); } catch { /* already closed */ } + } +} + describe('SSE reconnect triggers immediate refresh (bugfix #472)', () => { beforeEach(() => { vi.useFakeTimers({ shouldAdvanceTime: true }); - eventSourceInstances = []; + connections = []; + mockFetch.mockClear(); mockFetchState.mockReset().mockResolvedValue(MOCK_STATE); mockFetchOverview.mockReset().mockResolvedValue(MOCK_OVERVIEW); mockRefreshOverview.mockReset().mockResolvedValue(undefined); @@ -77,7 +102,7 @@ describe('SSE reconnect triggers immediate refresh (bugfix #472)', () => { afterEach(() => { vi.useRealTimers(); - // Reset module registry so singleton EventSource is cleaned up between tests + // Reset module registry so the singleton connection is cleaned up between tests vi.resetModules(); }); @@ -131,10 +156,12 @@ describe('SSE reconnect triggers immediate refresh (bugfix #472)', () => { const listener = vi.fn(); const { unmount } = renderHook(() => useSSE(listener)); - // Record baseline — prior tests may have leaked instances via module resets - const baseCount = eventSourceInstances.length; + await act(async () => { await vi.advanceTimersByTimeAsync(10); }); + + // Record baseline — prior tests may have leaked connections via module resets + const baseCount = connections.length; expect(baseCount).toBeGreaterThanOrEqual(1); - const currentES = eventSourceInstances[baseCount - 1]; + const currentConn = connections[baseCount - 1]; // Hide the tab Object.defineProperty(document, 'hidden', { value: true, configurable: true }); @@ -142,17 +169,18 @@ describe('SSE reconnect triggers immediate refresh (bugfix #472)', () => { document.dispatchEvent(new Event('visibilitychange')); }); - // SSE should be closed - expect(currentES.close).toHaveBeenCalled(); + // SSE should be aborted (the fetch-stream analogue of EventSource.close) + expect(currentConn.abort).toHaveBeenCalled(); // Show the tab again Object.defineProperty(document, 'hidden', { value: false, configurable: true }); - act(() => { + await act(async () => { document.dispatchEvent(new Event('visibilitychange')); + await vi.advanceTimersByTimeAsync(10); }); - // Should have reconnected (at least one new EventSource instance) - expect(eventSourceInstances.length).toBeGreaterThan(baseCount); + // Should have reconnected (at least one new connection) + expect(connections.length).toBeGreaterThan(baseCount); // Listener should have been notified on re-visible (to refresh stale data) expect(listener).toHaveBeenCalled(); @@ -164,49 +192,45 @@ describe('SSE reconnect triggers immediate refresh (bugfix #472)', () => { Object.defineProperty(document, 'hidden', { value: true, configurable: true }); const { useSSE } = await import('../src/hooks/useSSE.js'); const listener = vi.fn(); - const baseCount = eventSourceInstances.length; + const baseCount = connections.length; const { unmount } = renderHook(() => useSSE(listener)); - // Should NOT have connected (no new instances beyond baseline) - expect(eventSourceInstances.length).toBe(baseCount); + await act(async () => { await vi.advanceTimersByTimeAsync(10); }); + + // Should NOT have connected (no new connections beyond baseline) + expect(connections.length).toBe(baseCount); // Make visible — should connect now Object.defineProperty(document, 'hidden', { value: false, configurable: true }); - act(() => { + await act(async () => { document.dispatchEvent(new Event('visibilitychange')); + await vi.advanceTimersByTimeAsync(10); }); - expect(eventSourceInstances.length).toBeGreaterThan(baseCount); + expect(connections.length).toBeGreaterThan(baseCount); unmount(); }); - it('schedules reconnect when EventSource enters CLOSED state (Bugfix #1124)', async () => { + it('schedules reconnect when the stream ends (Bugfix #1124)', async () => { const { useSSE } = await import('../src/hooks/useSSE.js'); const listener = vi.fn(); const { unmount } = renderHook(() => useSSE(listener)); - const baseCount = eventSourceInstances.length; - expect(baseCount).toBeGreaterThanOrEqual(1); - const currentES = eventSourceInstances[baseCount - 1] as MockEventSource; - - // Simulate a 503 rejection: EventSource transitions to CLOSED - currentES.readyState = MockEventSource.CLOSED; - act(() => { - if (currentES.onerror) { - currentES.onerror(new Event('error')); - } - }); + await act(async () => { await vi.advanceTimersByTimeAsync(10); }); - // Should have disconnected the dead EventSource - expect(currentES.close).toHaveBeenCalled(); + const baseCount = connections.length; + expect(baseCount).toBeGreaterThanOrEqual(1); + const currentConn = connections[baseCount - 1]; - // Advance past the jittered reconnect window (max 5s) + // Simulate the server dropping the stream (non-200 / restart / capacity). await act(async () => { + endStream(currentConn); + // Advance past the jittered reconnect window (max 5s) await vi.advanceTimersByTimeAsync(6000); }); - // Should have reconnected (new EventSource instance) - expect(eventSourceInstances.length).toBeGreaterThan(baseCount); + // Should have reconnected (new connection) + expect(connections.length).toBeGreaterThan(baseCount); unmount(); }); diff --git a/apps/web/src/components/Terminal.tsx b/apps/web/src/components/Terminal.tsx index 165f885ed..1bc6b1cbe 100644 --- a/apps/web/src/components/Terminal.tsx +++ b/apps/web/src/components/Terminal.tsx @@ -9,10 +9,11 @@ import { FilePathLinkProvider, FilePathDecorationManager } from '../lib/filePath import { VirtualKeyboard, type ModifierState } from './VirtualKeyboard.js'; import { useMediaQuery } from '../hooks/useMediaQuery.js'; import { MOBILE_BREAKPOINT } from '../lib/constants.js'; -import { uploadPasteImage } from '../lib/api.js'; +import { uploadPasteImage, getWebKey } from '../lib/api.js'; import { ScrollController } from '../lib/scrollController.js'; import { EscapeBuffer } from '../lib/escapeBuffer.js'; import { BackoffController, classifyUpgradeError } from '@cluesmith/codev-sdk/reconnect-policy'; +import { terminalWsProtocols } from '@cluesmith/codev-types'; /** * Floating controls overlay for terminal windows — refresh (re-fit + resize) @@ -445,7 +446,11 @@ export function Terminal({ wsPath, onFileOpen, persistent, toolbarExtra, onPerma /** Create a WebSocket connection, optionally resuming from a sequence number. */ const connect = (resumeSeq?: number) => { const wsUrl = resumeSeq !== undefined ? `${wsBase}?resume=${resumeSeq}` : wsBase; - const ws = new WebSocket(wsUrl); + // Request authentication (advisory GHSA-xvjp-7748-v88v): browsers cannot + // set headers on a WebSocket, so the shared key travels as a subprotocol + // (validated at the upgrade), alongside the marker protocol Tower echoes. + const protocols = terminalWsProtocols(getWebKey()); + const ws = protocols ? new WebSocket(wsUrl, protocols) : new WebSocket(wsUrl); ws.binaryType = 'arraybuffer'; wsRef.current = ws; diff --git a/apps/web/src/hooks/useSSE.ts b/apps/web/src/hooks/useSSE.ts index c821dc543..63bb63999 100644 --- a/apps/web/src/hooks/useSSE.ts +++ b/apps/web/src/hooks/useSSE.ts @@ -1,25 +1,29 @@ import { useEffect } from 'react'; -import { getSSEEventsUrl } from '../lib/api.js'; +import { TOWER_KEY_HEADER } from '@cluesmith/codev-types'; +import { getSSEEventsUrl, getWebKey } from '../lib/api.js'; type Listener = () => void; -// Singleton EventSource shared across all hooks in this tab. +// Singleton SSE connection shared across all hooks in this tab. // -// WHY a singleton: Browsers enforce a 6-connection-per-origin limit for -// HTTP/1.1. Each EventSource holds one persistent connection open. Without -// sharing, every hook that calls useSSE() would open its own connection, -// quickly exhausting the limit (ERR_INSUFFICIENT_RESOURCES) and blocking -// other requests (fetch, WebSocket upgrades, etc.). +// WHY fetch+ReadableStream instead of EventSource: the browser `EventSource` +// cannot set request headers, so it cannot carry the `codev-tower-key` header the +// Tower API now requires (advisory GHSA-xvjp-7748-v88v). A `fetch` streamed +// through a `ReadableStream` sends the header and parses the same `data: {...}` +// SSE wire format. // -// VISIBILITY: When the tab is hidden, the SSE connection is closed to free -// the connection slot. With 6+ workspace tabs open, all slots would be -// consumed by SSE, blocking fetches and WebSocket upgrades entirely. -// On tab re-focus, we reconnect and fire a refresh so the UI catches up. +// WHY a singleton: browsers enforce a 6-connection-per-origin limit for +// HTTP/1.1. Each stream holds one persistent connection; without sharing, every +// hook that calls useSSE() would open its own, exhausting the limit and blocking +// other requests (fetch, WebSocket upgrades). // -// NOTE: Each browser tab gets its own module scope, so each open dashboard -// tab will have one independent EventSource connection. -let eventSource: EventSource | null = null; +// VISIBILITY: when the tab is hidden the connection is aborted to free the slot; +// on re-focus it reconnects and fires a refresh so the UI catches up. +// +// NOTE: each browser tab gets its own module scope, so each open dashboard tab +// has one independent connection. const listeners = new Set(); +let controller: AbortController | null = null; let visibilityListenerInstalled = false; let reconnectTimer: ReturnType | null = null; @@ -28,19 +32,54 @@ function notify(): void { } function connect(): void { - if (eventSource || typeof EventSource === 'undefined') return; + if (controller || typeof fetch === 'undefined') return; if (typeof document !== 'undefined' && document.hidden) return; - eventSource = new EventSource(getSSEEventsUrl()); - eventSource.onmessage = () => notify(); - eventSource.onerror = () => { - // Bugfix #1124: EventSource auto-reconnects after a successful 200 stream - // drops, but transitions to CLOSED (readyState === 2) on a non-200 - // response (e.g. 503 at capacity). Schedule a manual retry with jitter. - if (eventSource && eventSource.readyState === EventSource.CLOSED) { - disconnect(); - scheduleReconnect(); + const ctrl = new AbortController(); + controller = ctrl; + streamEvents(ctrl); +} + +async function streamEvents(ctrl: AbortController): Promise { + const headers: Record = {}; + const key = getWebKey(); + if (key) headers[TOWER_KEY_HEADER] = key; + + try { + const response = await fetch(getSSEEventsUrl(), { headers, signal: ctrl.signal }); + if (!response.ok || !response.body) { + // Non-200 (e.g. 401 without a key, or 503 at capacity) does not stream — + // schedule a manual retry with jitter. + if (controller === ctrl) { + disconnect(); + scheduleReconnect(); + } + return; + } + + const reader = response.body.getReader(); + const decoder = new TextDecoder(); + let buffer = ''; + + while (true) { + const { done, value } = await reader.read(); + if (done) break; + buffer += decoder.decode(value, { stream: true }); + // SSE frames are separated by a blank line; keep any partial trailing frame. + const frames = buffer.split('\n\n'); + buffer = frames.pop() || ''; + for (const frame of frames) { + if (/^data:/m.test(frame)) notify(); + } } - }; + } catch { + // Aborted (disconnect) or a network error — fall through to reconnect below. + } + + // The stream ended or errored; if this is still the live connection, retry. + if (controller === ctrl) { + disconnect(); + scheduleReconnect(); + } } function scheduleReconnect(): void { @@ -57,9 +96,9 @@ function disconnect(): void { clearTimeout(reconnectTimer); reconnectTimer = null; } - if (eventSource) { - eventSource.close(); - eventSource = null; + if (controller) { + controller.abort(); + controller = null; } } @@ -68,7 +107,7 @@ function handleVisibilityChange(): void { disconnect(); } else if (listeners.size > 0) { connect(); - // Notify listeners so the UI refreshes after being backgrounded + // Notify listeners so the UI refreshes after being backgrounded. notify(); } } @@ -81,9 +120,9 @@ function installVisibilityListener(): void { /** * Subscribe to SSE events from Tower. The callback fires on every SSE message - * (including the initial "connected" event sent after reconnection). - * Uses a shared EventSource singleton — multiple hooks share one connection. - * Automatically disconnects when the tab is hidden and reconnects on focus. + * (including the initial "connected" event sent after reconnection). Multiple + * hooks share one streamed connection. Automatically disconnects when the tab + * is hidden and reconnects on focus. */ export function useSSE(onEvent: Listener): void { useEffect(() => { diff --git a/apps/web/src/lib/api.ts b/apps/web/src/lib/api.ts index bca840cac..7a2787997 100644 --- a/apps/web/src/lib/api.ts +++ b/apps/web/src/lib/api.ts @@ -1,3 +1,4 @@ +import { TOWER_KEY_HEADER } from '@cluesmith/codev-types'; import { getApiBase } from './constants.js'; // Shared types from @cluesmith/codev-types @@ -27,10 +28,28 @@ function apiUrl(endpoint: string): string { return base + clean; } +/** + * Resolve the shared local key (advisory GHSA-xvjp-7748-v88v). Prefer the value + * Tower injects same-origin at serve time (`window.__CODEV_TOWER_KEY__`) so a + * direct navigation to a workspace URL works without first visiting the Tower + * shell; fall back to a previously stored value. The injected value is persisted + * so later same-origin requests keep working. + */ +export function getWebKey(): string | null { + const injected = (window as unknown as { __CODEV_TOWER_KEY__?: string }).__CODEV_TOWER_KEY__; + if (injected) { + try { localStorage.setItem('codev-tower-key', injected); } catch { /* storage may be unavailable */ } + return injected; + } + return localStorage.getItem('codev-tower-key'); +} + function getAuthHeaders(): Record { - const token = localStorage.getItem('codev-web-key'); + const token = getWebKey(); if (token) { - return { Authorization: `Bearer ${token}` }; + // Request authentication (advisory GHSA-xvjp-7748-v88v): Tower reads the + // shared local key from the codev-tower-key header. + return { [TOWER_KEY_HEADER]: token }; } return {}; } diff --git a/codev/plans/secfix-1-tower-auth-hardening.md b/codev/plans/secfix-1-tower-auth-hardening.md new file mode 100644 index 000000000..0be958d8f --- /dev/null +++ b/codev/plans/secfix-1-tower-auth-hardening.md @@ -0,0 +1,272 @@ +# PIR Plan: Enforce request authentication on the Tower local API + +Private security lane (advisory GHSA-xvjp-7748-v88v). This plan is a **hardening plan**: it +describes the authentication controls to add, not any attack. Per the privacy constraint, no +exploit mechanics, attack chain, or scenario prose appears in this file, the commits, the PR, +the review, or the thread log. All framing is "enforce request authentication on the Tower API". + +## Understanding + +Tower runs a local HTTP + WebSocket API (default bind `127.0.0.1:4100`) that reaches privileged +local operations (spawning PTYs, driving terminals, adding review comments, approving gates). +Today that API performs **no server-side request authentication**: + +- **HTTP:** the single front-door gate `isRequestAllowed(req)` returns `true` unconditionally + (`packages/codev/src/agent-farm/utils/server-utils.ts:80`), even though it is correctly wired + as the one choke point every route passes through (`tower-routes.ts:233` inside `handleRequest`). + Clients already transmit a key, but no server route reads or validates it. +- **WebSocket:** the upgrade handler (`packages/codev/src/agent-farm/servers/tower-websocket.ts`, + `setupUpgradeHandler`) validates only that the target session exists, never a key. The + browser-vs-Node discriminator `rejectUnknownSession` keys on the `Origin` header + (`tower-websocket.ts:173-187`). +- **CORS:** `handleRequest` reflects any `https://` origin and allows `Content-Type, Authorization` + (`tower-routes.ts:239-249`). + +A shared local key already exists: `~/.agent-farm/local-key`, generated by +`ensureLocalKey()` and read by `readLocalKey()` in `@cluesmith/codev-core/auth` +(`packages/core/src/auth.ts`). Clients source their key from it. The goal is to make the server +**enforce** that key on every privileged HTTP route and on the terminal WebSocket upgrade, fail +closed, and tighten CORS to an allowlist — while keeping the five API clients working. + +This is a **regression fix**: `isRequestAllowed` began as a real Host/Origin guard and was later +reduced to `return true`; CORS was later widened to reflect any HTTPS origin. This plan restores +and strengthens enforcement, now keyed on the shared local key rather than only Host/Origin. + +**Server/client isolation invariant (#1189):** enforcement is entirely server-side in +`@cluesmith/codev-core` / `packages/codev` (Tower). Clients only *transport* the key; they never +gain enforcement logic. `codev-types` stays contract-only. + +**Timing:** must land before the v3.3.0 (Luxor) release publishes the sdk publicly. + +## Proposed Change + +**Five core layers** (the advisory's layers 1–5), mapped to its remediation plan. The advisory's +6th layer (key rotation) is **out of scope for this fix** — see "Deferred follow-up" below. Each +layer is a git commit within this one PR (PIR = one PR, three human gates: plan-approval → +dev-approval → pr). + +### Design decisions (LOCKED) + +These three calls are settled for implementation (not open for the reviewer to redecide; details +in the layers below): + +1. **WebSocket key transport = `Sec-WebSocket-Protocol` (subprotocol).** Chosen over a query + param, which would leak the key into server logs, referrers, and history. The server validates + the subprotocol at `handleUpgrade` and **echoes the selected subprotocol** back so strict `ws` + clients accept the handshake. Applies to `/ws/terminal/:id`, `/workspace/:path/ws/terminal/:id`, + and `/ws/messages`. +2. **CORS = fixed origin allowlist.** Replace reflect-any-`https://` with: `http://localhost:` + and `http://127.0.0.1:` for any port (loopback), **plus** the single configured tunnel + origin when a tunnel is active (from Tower's tunnel config), and nothing else. No wildcard, no + scheme-only reflection. CORS is defense-in-depth; the Layer 1 key check is the actual control. +3. **Constant-time compare = `crypto.timingSafeEqual`, length-guarded.** Compare presented vs. + expected key as UTF-8 buffers: first compare lengths (unequal ⇒ immediate mismatch, since + `timingSafeEqual` throws on unequal-length buffers), then `timingSafeEqual` on equal-length + buffers. Wrapped in one small server-side helper. No plain `===`, no reusable helper exists in + non-test source today. + +### Layer 1 — HTTP key enforcement (server-side) + +- In `server-utils.ts`, replace the unconditional `isRequestAllowed` with real enforcement: + read the expected key (see "Key handling" below), read the presented key from the request, and + **constant-time compare**. Missing/mismatched ⇒ reject with **401** (change the `handleRequest` + reject status from 403 to 401 at `tower-routes.ts:233-236`, matching the advisory and the + existing `authFetch` 401 handling in tower.html). +- **Presented-key header:** accept `codev-web-key` (what the sdk `TowerClient` sends — + `tower-client.ts:319-325`). tower.html currently sends `Authorization: Bearer ` instead + (`tower.html:993-996`); Layer 4 switches it to `codev-web-key` so the server reads exactly one + header. (Decision: standardize on `codev-web-key`; do **not** teach the server two header names.) +- **Public-route allowlist.** A small explicit allowlist stays keyless; everything else requires + the key. Proposed allowlist (to confirm at review): + - `GET /health` (uptime ping, pre-auth) + - `GET /api/version` (VS Code preflight probe, documented keyless at `tower-routes.ts:509`) + - `GET /` and `GET /index.html` (serve the dashboard shell so the page can then present its key) + - React dashboard static assets (JS/CSS/index) — served before the page has a key + - Every other route (`/api/terminals`, `/api/send`, `/api/command`, canvas relay, workspace + APIs, etc.) requires the key. + The allowlist is evaluated in `handleRequest` by pathname+method (the choke point already has + the parsed `url`), so `isRequestAllowed`/the new key check applies to the complement. Getting + this list wrong either breaks pre-auth pings/dashboard load or leaves a privileged route open — + so it is enumerated explicitly and covered by tests (Layer 4). +- **Fail closed.** If the expected key cannot be read (absent file, unreadable), reject — never + fall back to keyless access. + +### Layer 2 — WebSocket key enforcement (server-side) + +- Add a browser-compatible key transport via **`Sec-WebSocket-Protocol`** (subprotocol), not a + query param (avoids URL/referrer/log leakage). Validate it **at the upgrade** in + `setupUpgradeHandler` (`tower-websocket.ts:200`) for the terminal routes (`/ws/terminal/:id` and + `/workspace/:path/ws/terminal/:id`) and for `/ws/messages`. Authenticating at the handshake (not + post-open) means the upgrade is rejected before any PTY attach — strictly stronger than today's + VS Code in-band control-frame auth (`apps/vscode/src/terminal-adapter.ts:198-199`), which this + replaces. +- **Echo the selected subprotocol** back in the handshake (pass the chosen protocol to + `wss.handleUpgrade`/the accept), or strict `ws` clients error out. +- **Fail-closed discriminator.** `rejectUnknownSession` degrades a browser arriving without + `Origin` to the Node path today (`tower-websocket.ts:179-186`); once a key is required, that + degradation must not become an auth bypass. Enforce the key **before** the session-existence + branch so a missing/invalid key is rejected regardless of `Origin`. +- Reject a missing/invalid WS key by closing the upgrade cleanly (Node path: HTTP `401` at the + upgrade stage; browser path: accept-then-close with an app-range close code, consistent with the + existing `WS_CLOSE_SESSION_UNKNOWN` pattern) — a clean signal, not a silent hang. +- **WS credential surface is only two clients** (see client map): the browser dashboard + (`apps/web/src/components/Terminal.tsx:448`, currently sends nothing) and the VS Code node-`ws` + client (`apps/vscode/src/terminal-adapter.ts:183`, currently in-band post-open). tower.html, the + sdk `TowerClient`, and the Stream Deck plugin open no Tower terminal WebSocket. + +### Layer 3 — CORS hardening (server-side) + +- Replace reflect-any-`https://` (`tower-routes.ts:241-247`) with an **allowlist**: localhost/ + 127.0.0.1 origins on any port, plus the configured tunnel origin (if any). +- Add `codev-web-key` to `Access-Control-Allow-Headers` (browser clients cannot send a custom + header cross-origin otherwise); drop `Authorization` once tower.html stops using it (Layer 4). +- **CORS is defense-in-depth, not the control.** A "simple" request triggers no preflight, so + Layer 1's key check must reject independently of CORS. Tests assert an unauthenticated simple + request is still 401. + +### Layer 4 — Rollout across all five clients + tests + +Confirm each of the five clients sends the key on both HTTP and (where applicable) WS, and gets a +clean 401 (not a hang) on failure. Per the client map: + +1. **sdk `TowerClient`** (`packages/sdk/src/tower-client.ts`) — already sends `codev-web-key` on + fetch/binary/SSE (`:319-326, :731-735, :945-952, :1085-1089`). Opens **no** WebSocket (only + builds the URL via `getTerminalWsUrl` `:906-908`). No change needed beyond confirming behavior. +2. **VS Code extension** (`apps/vscode/`) — HTTP via the sdk `TowerClient`, key injected via + `getAuthKey` (`connection-manager.ts:73-76`, `tower-starter.ts:122`; key from + `auth-wrapper.ts` SecretStorage → `readLocalKey`). WS uses node `ws` + (`terminal-adapter.ts:183`) and today sends the credential **in-band post-open** as a + `ping`/`auth` control frame (`terminal-adapter.ts:198-199`). **Change:** send the key as the + `Sec-WebSocket-Protocol` subprotocol at connect (ws options arg), and remove the in-band auth + frame once the server enforces at the handshake. +3. **Web dashboard React SPA** (`apps/web/`, built to `dashboard-dist`) — raw `fetch` in + `apps/web/src/lib/api.ts` currently sends `Authorization: Bearer` (`getAuthHeaders` `:30-36`) + from `localStorage['codev-web-key']`. **Change:** send the `codev-web-key` header instead. Its + terminal WS (`apps/web/src/components/Terminal.tsx:448`, browser `WebSocket`) sends nothing + today — **add the subprotocol** (second `WebSocket` arg); key from localStorage. +4. **tower.html** (`packages/codev/templates/tower.html`) — standalone launcher page; opens **no** + terminal WS. HTTP via `authFetch` sends `Authorization: Bearer` (`:993-1010`). **Change:** send + the `codev-web-key` header. **Key delivery:** it is served *by* Tower and currently reads the + key from localStorage with no serve-time injection. Proposed: **same-origin injection at serve + time** in `handleDashboard` (`tower-routes.ts:2363-2378`) — Tower writes the current key into + the page only for same-origin `127.0.0.1`/localhost requests, which is what protects it. Make + the same-origin condition explicit and tested. (Confirm at review; alternative is a one-time + login field.) +5. **Stream Deck plugin** (`apps/streamdeck/`) — HTTP via the sdk `TowerClient` + (`plugin.ts:31-32`, `getAuthKey: readLocalKey`), already sends `codev-web-key`. Opens **no** + Tower terminal WS. No change needed beyond confirming behavior. + +**Tests:** +- Flip the four mocks that hard-code `isRequestAllowed: () => true` to exercise real enforcement: + `tower-routes.test.ts:138`, `inbox-routes.test.ts:63`, `spec-761-api-state.test.ts:87`, + `tower-cron-routes.test.ts:76`. (`tower-routes.test.ts:275-277` already tests the 403→now-401 path.) +- **Negative-path:** no key ⇒ 401 on every privileged HTTP route *and* on the WS upgrade; + wrong-length key does not throw (constant-time compare is length-guarded); no-preflight "simple" + request is still 401. +- **Positive-path:** allowlisted public routes still work keyless; a valid key passes on HTTP and + WS; the WS handshake echoes the subprotocol. + +### Layer 5 — BRIDGE_MODE + +- Make key enforcement **mandatory** when `BRIDGE_MODE=1` (non-localhost bind; + `tower-server.ts:113-114`): fail closed at boot if no key file exists. +- Document that on a non-localhost bind the shared key travels in cleartext unless TLS terminates + at the tunnel/proxy — require TLS termination for bridge deployments (doc + note; no plaintext + key on an untrusted network). + +### Deferred follow-up (OUT OF SCOPE) — Key rotation + +The advisory's 6th layer (a documented way to regenerate/rotate `~/.agent-farm/local-key` and have +all clients pick up the new value) is **explicitly out of scope for this fix** and will **not** be +implemented here. Noted as a deferred follow-up so it is not lost; the architect will file it +separately. This PR does not add rotation tooling. + +## Key handling (cross-cutting) + +- **Expected key:** Tower ensures the key once at server boot via `ensureLocalKey()` (idempotent, + `packages/core/src/auth.ts`), caching the value in memory for O(1) per-request comparison. The + enforcement read fails closed if the value is unavailable. (Picking up a rotated key is the + deferred follow-up above; this fix does not implement it.) +- **Constant-time compare:** use `crypto.timingSafeEqual`, **length-guarded** first + (`timingSafeEqual` throws on unequal buffer lengths) — compare lengths, and only then the bytes, + so a wrong-length key is a clean mismatch, not a crash. Add a tiny shared helper (no reusable one + exists in non-test source today). This lives server-side only. + +## Files to Change + +Server (enforcement — `@cluesmith/codev-core` / `packages/codev`): +- `packages/codev/src/agent-farm/utils/server-utils.ts:80` — real `isRequestAllowed` key check + + length-guarded constant-time compare helper. +- `packages/codev/src/agent-farm/servers/tower-routes.ts:232-249` — public-route allowlist, 401 on + missing/invalid key, CORS allowlist, `codev-web-key` in allowed headers. +- `packages/codev/src/agent-farm/servers/tower-routes.ts:2363` (`handleDashboard`) — same-origin + key injection for tower.html at serve time. +- `packages/codev/src/agent-farm/servers/tower-websocket.ts:173-187, 200-309` — WS key validation + via `Sec-WebSocket-Protocol`, echo subprotocol, fail-closed discriminator. +- `packages/codev/src/agent-farm/servers/tower-server.ts:113-114` — ensure key at boot; BRIDGE_MODE + mandatory-enforcement + fail-closed-if-no-key. + +Clients (transport only): +- `apps/vscode/src/terminal-adapter.ts:183,198-199` — WS `Sec-WebSocket-Protocol` at connect; + remove the in-band auth control frame. HTTP already sends `codev-web-key` (no change). +- `apps/web/src/lib/api.ts:30-36` — switch HTTP `Authorization: Bearer` → `codev-web-key` header. +- `apps/web/src/components/Terminal.tsx:412-448` — add the WS subprotocol (second `WebSocket` arg). +- `packages/codev/templates/tower.html:993-1016` — switch HTTP to `codev-web-key`; consume the + same-origin-injected key. (No terminal WS on this page.) +- No change to `packages/sdk/src/tower-client.ts` or `apps/streamdeck/` (already send + `codev-web-key` on HTTP; neither opens a Tower terminal WS) — confirm only. + +Tests: +- `packages/codev/src/agent-farm/__tests__/tower-routes.test.ts`, `inbox-routes.test.ts`, + `spec-761-api-state.test.ts`, `tower-cron-routes.test.ts` — flip the `isRequestAllowed` mocks + + add negative/positive-path cases; new WS-upgrade auth test. + +Docs: +- BRIDGE_MODE TLS requirement (location TBD — likely the tunnel/bridge doc). + +## Risks & Alternatives Considered + +- **Risk — public-route allowlist wrong.** Too tight breaks the pre-auth `/health`/`/version` + pings and the dashboard shell load; too loose leaves a privileged route open. Mitigation: + explicit enumerated allowlist + positive test that public routes work keyless and negative test + that every privileged route is 401 without a key. +- **Risk — WS handshake breakage.** Not echoing the subprotocol breaks strict `ws` clients; + Node-terminal 404-string clients (#936) must keep working. Mitigation: echo the selected + subprotocol; keep the Node reject path's HTTP wording unchanged; test both shapes. +- **Risk — tower.html key delivery.** Embedding a static secret in served HTML would leak it. + Mitigation: same-origin serve-time injection, gated on localhost/127.0.0.1 origin, tested; the + same-origin condition is what protects it. +- **Risk — `timingSafeEqual` length crash.** Mitigation: length-guard before compare; test a + wrong-length key. +- **Risk — fail-open on missing key file.** Mitigation: fail closed everywhere; boot-time ensure. +- **Alternative — accept both `codev-web-key` and `Authorization: Bearer` server-side.** Rejected: + one canonical header is simpler and less error-prone; migrate tower.html instead. +- **Alternative — WS key via query param.** Rejected: leaks via logs/referrer; subprotocol is the + browser-safe channel. +- **Alternative — per-route auth decorators.** Rejected: the single `handleRequest` choke point + already covers all HTTP routes; a central allowlist is simpler and less error-prone than a sweep. + +## Test Plan + +- **Unit (server):** no key ⇒ 401 on a representative privileged HTTP route and on the WS upgrade; + valid key ⇒ pass on both; allowlisted public route works keyless; wrong-length key ⇒ clean 401 + (no throw); no-preflight simple request ⇒ 401; CORS reflects only allowlisted origins; WS + handshake echoes the subprotocol. Run from the worktree: `pnpm -C packages/codev test` (+ sdk). +- **Manual (dev-approval gate, running worktree):** + - Start Tower from the worktree; open the dashboard (`http://127.0.0.1:/`) — it loads, + authenticates via the injected/stored key, terminals attach over WS. + - VS Code extension against this Tower — terminals, gate approval, comments all still work. + - A request with no/wrong key ⇒ clean 401 (observe in devtools/network), not a hang. + - `tower.html` served page — key obtained same-origin, WS terminal attaches. + - (If reachable) Stream Deck plugin still drives Tower. +- **BRIDGE_MODE:** with `BRIDGE_MODE=1` and no key file ⇒ Tower fails closed at boot; with a key ⇒ + enforcement on; confirm the TLS-required note is documented. + +## Open Questions for the Reviewer + +1. Confirm the **public-route allowlist** (esp. whether `/` + static dashboard assets stay keyless, + and any other pre-auth route beyond `/health` and `/api/version`). +2. Confirm **tower.html key delivery** = same-origin serve-time injection (vs a login field). +3. **CVSS/disclosure** framing is the owner's call and out of scope for this code lane. +4. **Key rotation is out of scope** for this PR (deferred follow-up, per architect direction); this + plan does not implement it. diff --git a/codev/projects/builder-task-nhnj-task-NHnJ/status.yaml b/codev/projects/builder-task-nhnj-task-NHnJ/status.yaml new file mode 100644 index 000000000..0326c4a2d --- /dev/null +++ b/codev/projects/builder-task-nhnj-task-NHnJ/status.yaml @@ -0,0 +1,18 @@ +id: builder-task-nhnj +title: task-NHnJ +protocol: pir +phase: plan +plan_phases: [] +current_plan_phase: null +gates: + plan-approval: + status: pending + dev-approval: + status: pending + pr: + status: pending +iteration: 1 +build_complete: false +history: [] +started_at: '2026-08-12T04:01:13.603Z' +updated_at: '2026-08-12T04:01:13.603Z' diff --git a/codev/projects/secfix-1-tower-auth-hardening/status.yaml b/codev/projects/secfix-1-tower-auth-hardening/status.yaml new file mode 100644 index 000000000..3d1486dcb --- /dev/null +++ b/codev/projects/secfix-1-tower-auth-hardening/status.yaml @@ -0,0 +1,29 @@ +id: secfix-1 +title: tower-auth-hardening +protocol: pir +phase: review +plan_phases: [] +current_plan_phase: null +gates: + plan-approval: + status: approved + requested_at: '2026-08-12T04:33:55.262Z' + approved_at: '2026-08-12T04:44:20.897Z' + dev-approval: + status: approved + requested_at: '2026-08-12T06:18:24.688Z' + approved_at: '2026-08-12T08:51:55.685Z' + pr: + status: pending + requested_at: '2026-08-12T09:11:25.150Z' +iteration: 1 +build_complete: true +history: [] +started_at: '2026-08-12T04:32:39.515Z' +updated_at: '2026-08-12T09:11:25.152Z' +pr_history: + - phase: review + pr_number: 1421 + branch: builder/task-NHnJ + created_at: '2026-08-12T08:59:12.554Z' +pr_ready_for_human: true diff --git a/codev/resources/arch.md b/codev/resources/arch.md index d77607763..0fedb228e 100644 --- a/codev/resources/arch.md +++ b/codev/resources/arch.md @@ -107,6 +107,8 @@ tail -f ~/.agent-farm/tower.log 8. **Consultation Requirements**: External AI consultation (Gemini, Codex) is mandatory at SPIR checkpoints unless explicitly disabled. +9. **Tower API Authentication**: Tower's local HTTP + WebSocket API enforces request authentication (advisory GHSA-xvjp-7748-v88v). Every route outside the narrow public-route allowlist (`isPublicRoute` in `agent-farm/utils/server-utils.ts`) requires the shared local key (`~/.agent-farm/local-key`), sent as the `codev-tower-key` HTTP header or a `Sec-WebSocket-Protocol` subprotocol, and fails closed with 401 (the server also accepts the legacy `codev-web-key` header for one release). Any new Tower route must decide public-vs-keyed — a wrong allowlist entry either breaks a pre-auth path (health/version probes, the served HTML shells + static assets) or exposes a data route. The key is delivered to browser shells via same-origin serve-time injection; those shell responses omit `Access-Control-Allow-Origin` so the injected key is not cross-origin readable. + ## Agent Farm Internals This section provides comprehensive documentation of how the Agent Farm (`afx`) system works internally. Agent Farm is the most complex component of Codev, enabling parallel AI-assisted development through the architect-builder pattern. diff --git a/codev/resources/commands/agent-farm.md b/codev/resources/commands/agent-farm.md index 4ad0cfe8e..61714fa5b 100644 --- a/codev/resources/commands/agent-farm.md +++ b/codev/resources/commands/agent-farm.md @@ -897,6 +897,9 @@ afx tower start [options] **Environment Variables:** - `BRIDGE_MODE=1` — Enable non-localhost binding (required). Without this flag, Tower only binds to `127.0.0.1`. - `BRIDGE_TOWER_HOST` — Bind address when bridge mode is enabled (default: `127.0.0.1`). Only consulted when `BRIDGE_MODE=1`. Set to `0.0.0.0` for all network interfaces. Accepts IP literals only (no hostnames). Note: `BRIDGE_TOWER_HOST` has no effect unless `BRIDGE_MODE=1`. +- `CODEV_TOWER_ALLOWED_ORIGINS` — Comma-separated list of extra origins (e.g. `https://tunnel.example.com`) that Tower's request-authentication layer accepts for **both** the `Host` guard and CORS. Loopback (`localhost`/`127.0.0.1`/`::1`) is always allowed, and under `BRIDGE_MODE` any IP-literal `Host` is accepted (a LAN client reaches Tower by IP). Set this only when clients reach Tower by a **hostname** (a tunnel/proxy domain, a custom `.local` name); otherwise those requests are rejected with `401` and a `disallowed Host` log line. DNS names not on this list stay rejected even under `BRIDGE_MODE` (the DNS-rebinding guard). + +**Authentication & `BRIDGE_MODE` (advisory GHSA-xvjp-7748-v88v):** Tower's local API enforces request authentication with a shared key (`~/.agent-farm/local-key`). Under `BRIDGE_MODE`, enforcement is **mandatory** — Tower refuses to start on a network-reachable bind if the key cannot be created. Because a non-localhost bind serves plain HTTP, **the shared key travels in cleartext on the wire unless TLS terminates at your tunnel/proxy** — always front a bridge-mode Tower with TLS (e.g. the tunnel's HTTPS endpoint), never expose plain `http://:4100` on an untrusted network. #### afx tower stop diff --git a/codev/resources/lessons-learned.md b/codev/resources/lessons-learned.md index 49ac02506..315574557 100644 --- a/codev/resources/lessons-learned.md +++ b/codev/resources/lessons-learned.md @@ -63,6 +63,8 @@ Generalizable wisdom extracted from review documents, ordered by impact. Updated - [From 0097] `writeFileSync` with `{ mode: 0o600 }` only applies mode on file creation -- pre-existing files keep their old permissions. Always follow with `chmodSync(path, 0o600)` to enforce permissions regardless. - [From 0099] Always use `path.sep` in path security checks. The `startsWith(projectPath)` vulnerability allows sibling directory traversal. Always use `startsWith(base + path.sep)` or `path.relative()` -- never bare `startsWith(base)`. - [From 0099] Use collision-resistant IDs by default. Using `Date.now()` for IDs is a known anti-pattern when multiple operations can occur in the same millisecond. Use `crypto.randomUUID()` or a counter -- never timestamp-only. +- [secfix-1] An auth gate's public-route allowlist must include the tooling's own readiness/uptime probes. Adding key enforcement to a route that `afx tower start` polls for readiness (`/api/status`) made the probe 401 forever, so startup never detected "ready" and the launcher killed a healthy Tower after its 30s timeout — a self-inflicted boot failure, invisible to build+tests. Either keep such probes on the public allowlist (e.g. `/health`) or have the (trusted, local) probe authenticate. +- [secfix-1] In a key-bearing page, ANY XSS is credential theft: once the shared key is injected into a document, an XSS there reads the key and yields full API access, so the "no XSS" bar on those specific pages is load-bearing, not cosmetic. Sweep every sink where attacker-influenceable input (filenames, paths, query params, file-derived server values) reaches the key-bearing document's HTML/JS and encode at the sink; a media route that can only be loaded via a `src` attribute (no header possible) must be re-plumbed to an authenticated blob fetch rather than left keyless. ## Architecture @@ -183,6 +185,7 @@ Generalizable wisdom extracted from review documents, ordered by impact. Updated - [From 818] An acceptance criterion of "rule structurally identical to X" is a written-rule trap when the rule lives as duplicated prose in two views. Two copies drift even with diligence; the only durable enforcement is one shared function both views import. Extract when the second consumer lands — not before (no abstraction without users) and not later (drift starts on day one). - [From 1107] To place an *interactive* React widget (text input, buttons) inside an `innerHTML`-managed body, don't hand-build DOM there — inject an empty placeholder node in an effect and `createPortal` the React component into it. React owns the widget's state/focus/keyboard, while it still sits in normal document flow. Make the placeholder-injection effect idempotent (reuse a correctly-placed node; bail when `previousElementSibling` already matches the anchor) or the `setState`-on-inject loops; an `html` rebuild disconnects the node, which the same guard detects and re-creates. This is the read-while-write composer (#1107) but applies to any overlay/widget over imperatively-rendered content. - [From #1338] Retiring an entry from a shared resolver/registry must **fail closed at every resolution path**, not just delete the entry: a pure delete makes the explicit-name path throw a generic "unknown" error (no migration guidance) and the auto-detect path *silently* fall back to the default provider (here, the claude harness) — a dangerous mis-injection, not a visible failure. Keep the retired name in the detector and add a retirement sentinel checked BEFORE both exits so every path yields the same specific message; then grep every caller — spawn preflight, launch, and especially the reconnect/clean-exit relaunch paths that mint fresh sessions — because those are exactly the ones an "it's unreachable" analysis misses (three of them surfaced only under adversarial review here). +- [secfix-1] Adding the first **runtime (value)** import of a workspace package that was previously only **type-imported** (`import type`, erased at build) requires moving that dep from `devDependencies` to `dependencies` in the importing package. A devDependency is not installed for a published/deployed consumer, so the packaged build throws `Cannot find module` at **module load** — before any logger initializes, so it surfaces as a silent boot crash (e.g. a 30s startup timeout with zero log lines), not an obvious error. It is invisible to build+test because the monorepo symlinks everything at dev time. Verify the **packaged** artifact: `pnpm --filter deploy --prod --legacy ` (or a throwaway-prefix `npm install` of the tarballs) resolves only real `dependencies`, mimicking a published install — then load the entry module from it. Also update any hand-rolled local-install/packaging script that packs a *hand-picked subset* of workspace packages: it must now pack the newly-runtime dep too, or the install can't satisfy it. ## Process diff --git a/codev/reviews/secfix-1-tower-auth-hardening.md b/codev/reviews/secfix-1-tower-auth-hardening.md new file mode 100644 index 000000000..392dbe867 --- /dev/null +++ b/codev/reviews/secfix-1-tower-auth-hardening.md @@ -0,0 +1,199 @@ +# PIR Review: Enforce request authentication on the Tower local API + +Private security lane (advisory GHSA-xvjp-7748-v88v). This retrospective, the PR body, and every +committed artifact are deliberately **mechanics-free**: they describe the hardening, never any +exploit mechanics or attack scenario, and refer to the advisory by id only. + +No linked GitHub issue — this was an ad-hoc security task; the advisory is the source of record. + +## Summary + +Tower's local HTTP + WebSocket API performed no server-side request authentication. This PR makes +every non-public route require the shared local key (`~/.agent-farm/local-key`), fail closed, and +tightens the surrounding controls, then rolls the key transport across all clients. It implements +the advisory's five-layer remediation: HTTP key enforcement at the single request choke point, +WebSocket key enforcement at the upgrade, a fixed CORS origin allowlist, cross-client key +transport, and mandatory enforcement under `BRIDGE_MODE`. Key rotation (the advisory's 6th layer) +is a deferred follow-up. + +## Files Changed + +(Excludes the vendored three.js assets — see the Architecture note.) + +- `packages/types/src/websocket.ts` (+36), `packages/types/src/index.ts` (+5) — shared wire + contracts (header name, WS subprotocols, the terminal-WS subprotocol builder). +- `packages/codev/src/agent-farm/utils/server-utils.ts` (+280) — the auth helpers: HTTP key check, + WS key check, public-route allowlist, constant-time compare, CORS + Host allowlists, cached key. +- `packages/codev/src/agent-farm/servers/tower-routes.ts` (+163) — front-door ordering (preflight + before the key check, 401), CORS allowlist, same-origin key injection into served HTML shells, + annotator sink encoding, rejected-request logging. +- `packages/codev/src/agent-farm/servers/tower-websocket.ts` (+37) — upgrade-time key gate + (Origin-independent), clean reject shapes. +- `packages/codev/src/agent-farm/servers/tower-server.ts` (+30) — ensure the key at boot; + mandatory enforcement + fail-closed-if-no-key under `BRIDGE_MODE`; subprotocol echo. +- `packages/codev/src/agent-farm/lib/reconnect-backoff.ts` (+11) — an app-range WS close code. +- `packages/codev/src/agent-farm/lib/tunnel-client.ts` (+10) — tunnel WS upgrade sends a loopback + `Host` so it passes the Host guard. +- `packages/codev/src/agent-farm/commands/tower.ts` (+12) — the `afx tower start` readiness probe + authenticates (the probe polls a now-keyed route). +- `packages/core/src/auth.ts` (+11) — repair an existing key file's permissions to `0600` on read. +- Clients: `apps/vscode/src/{terminal-adapter,sse-client,connection-manager}.ts`, + `apps/web/src/{lib/api,hooks/useSSE,components/Terminal}.ts`, + `packages/codev/templates/{tower,open,3d-viewer}.html` — send the key on HTTP/SSE/WS; browser + pages consume the same-origin-injected key; the annotator loads media as authenticated blobs. +- `packages/codev/package.json` (+`codev-types` as a runtime dep), `scripts/local-install.sh` + (pack + install `codev-types`), `pnpm-lock.yaml` — packaging fixes (see Lessons). +- Tests: new `request-auth.test.ts` (+260) plus updates to `tower-routes`, `tower-websocket`, + `inbox-routes`, `spec-761-api-state`, `tower-cron-routes`, sdk `tower-client`, and the apps/web + suites. + +Plus 5 vendored three.js files (`packages/codev/templates/vendor/three*.js`, ~58k lines) — a +one-time local vendoring so no remote CDN code runs in a key-bearing page. + +## Commits + +`git log main..HEAD --oneline` (implementation commits; porch scaffolding omitted): + +- `e6181262f` types: add request-authentication wire contracts +- `37814ee5f` server: enforce request authentication on the Tower API +- `3a882b303` clients: transport the shared key on HTTP and WebSocket +- `b0d711f24` tests: request-authentication enforcement +- `40a080954` types: centralize the terminal WS subprotocol builder +- `5f4cef9dc` server: same-origin key delivery, Host guard, CORS shell isolation +- `f98f42e56` clients: web dashboard + annotator key transport +- `9761070b7` tests: Host guard + annotator allowlist coverage +- `19ae5013a` server: bridge/tunnel Host handling, injection safety, diagnostics +- `c7ebde935` vscode: authenticate the SSE client +- `0220b46a9` annotator: vendor three.js locally; template hardening +- `34f259a76` tests: bridge Host, no-slash workspace, SSE-stream + mock updates +- `32dc43411` harden: narrow bridge Host to IP-literals; encode annotator XSS sinks +- `f0f4afe2a` fix: make @cluesmith/codev-types a runtime dependency of codev +- `e8795de95` fix(local-install): pack + install @cluesmith/codev-types +- `0ff32b63d` fix: authenticate the Tower startup readiness probe +- `11978b516` rename the auth header codev-web-key -> codev-tower-key (dual-accept) +- `f36a134ec` chore(vscode): satisfy eslint curly rule in tunnel.ts + +## Test Results + +- `pnpm --filter @cluesmith/codev build`: ✓ pass +- Full codev suite: ✓ 4884 passed, 48 skipped, 0 failed (incl. the new `request-auth` suite + + the dual-accept test) +- `@cluesmith/codev-sdk`: ✓ 98 passed (incl. the import-boundary tests) +- `apps/web`: ✓ 335 passed; `apps/vscode`: ✓ 794 passed +- Typechecks: `apps/web` `tsc -b` ✓, `apps/vscode` main tsconfig ✓ +- Packaging verified via `pnpm deploy --prod` and a throwaway-prefix `npm install` of all four + tarballs (types/core/sdk/codev) — the boot module loads and resolves `codev-types`. +- Manual: the human approved the `dev-approval` gate after running the branch (Tower boot + + cross-client paths). + +## Architecture Updates + +Routed one system-shape invariant into COLD `codev/resources/arch.md` § **Invariants & Constraints** +(invariant #9): Tower's local API now **enforces** request authentication — non-public routes +require the shared key and fail closed, and any new Tower route must decide public-vs-keyed via the +`isPublicRoute` allowlist. Mechanics-free wording (advisory by id). Routed cold rather than into the +hot always-injected tier to avoid churning that capped file in a security PR; a future MAINTAIN pass +can promote it if the hot tier warrants it. (`arch.md`/`lessons-learned.md` are our user-evolved +instance docs, not `codev-skeleton` framework files, so no skeleton mirror applies.) + +## Lessons Learned Updates + +Routed three entries into COLD `codev/resources/lessons-learned.md`: + +1. § Architecture — **the first runtime (value) import of a previously type-only workspace package + must move that dep `devDependencies` → `dependencies`**, or the published/deployed build crashes + at module load (`Cannot find module`), invisible to build+test (the monorepo symlinks + everything). Verify the packaged artifact with `pnpm deploy --prod` / a throwaway-prefix install; + also update any local-install script that packs a hand-picked subset of workspace packages. +2. § Security — **an auth gate's public-route allowlist must include the tooling's own + readiness/uptime probes**, or startup detection breaks (a keyed `/api/status` 401'd the + `afx tower start` readiness probe, so a healthy Tower was killed by its own launcher). +3. § Security — **in a key-bearing page, any XSS is credential theft**: once the key is injected, + an XSS there reads it and yields full API access, so every attacker-influenceable sink in those + pages must be encoded and any `src`-loaded media route re-plumbed to an authenticated blob fetch. + +## Things to Look At During PR Review + +Security-sensitive spots worth focused attention: + +- **Public-route allowlist** (`isPublicRoute`) — the one place a wrong entry either blocks a + pre-auth path or exposes a data route. Note the GET-only rule, the `/workspace//` static + carve-out that excludes `api/`/`ws/`/`file`, and the annotator shell+vendor carve-out that keeps + every data/media sub-route keyed. +- **Dual-accept header** — the server accepts the new `codev-tower-key` and the legacy + `codev-web-key` for one release, so a not-yet-updated VS Code / Stream Deck keeps working; + there's a `# drop the fallback next release` follow-up. +- **Host guard + `BRIDGE_MODE`** — strict loopback for the localhost bind; in bridge mode it also + accepts IP-literal Hosts but still rejects hostNAMEs (the rebinding guard stays on). Confirm the + relaxation never weakens the (separate, still-mandatory) key check. +- **Same-origin key delivery** — the key is injected into served HTML shells and those responses + strip `Access-Control-Allow-Origin`, so a cross-origin page cannot read the injected key; the key + is validated as hex before embedding. `window.__CODEV_TOWER_KEY__`. +- **WebSocket gate** — validated at the handshake before any session lookup, Origin-independent; + the server echoes only the non-secret marker subprotocol, never the key token. +- **Annotator** — media (image/video/pdf/model) is fetched as authenticated blobs / via + `setRequestHeader`; attacker-influenceable values interpolated into the key-bearing shell are + encoded at their sink; three.js is vendored locally so no remote code runs in that page. +- **Constant-time compare** — length-guarded before `timingSafeEqual`; fail-closed when the key is + unavailable. + +### 3-Way Consultation Dispositions (single advisory pass) + +Verdicts: **Gemini APPROVE**, **Claude COMMENT**, **Codex REQUEST_CHANGES**. PIR runs one advisory +pass and will not re-review, so each finding is dispositioned here for the `pr`-gate reviewer: + +- **Missing BRIDGE_MODE/TLS + `CODEV_TOWER_ALLOWED_ORIGINS` docs** (Codex + Claude) — **Fixed.** + Documented in `codev/resources/commands/agent-farm.md` under `afx tower start`: the mandatory + bridge-mode auth, the cleartext-key/TLS-termination requirement, and the new + `CODEV_TOWER_ALLOWED_ORIGINS` knob (Host + CORS allowlist for hostname clients). +- **WS marker-echo + vscode-subprotocol test gaps** (Codex) — **Fixed.** Extracted the echo rule to + a testable `selectWsSubprotocol` (asserts it echoes the marker and never the key token) and added + a vscode test asserting the WS is opened with `[marker, codev-key.]` (and none without a key). +- **Header renamed `codev-web-key` → `codev-tower-key` + dual-accept, vs the plan's "one header"** + (Codex) — **Reasoned deviation, not a defect.** This was an explicit post-plan decision by the + human reviewer at the dev-approval gate: the Stream Deck plugin lives out-of-tree and bundles an + older sdk, so a hard cutover would break it; the server dual-accepts the legacy header for one + release. Disclosed above; Claude concurred the justification is sound. +- **CORS uses `CODEV_TOWER_ALLOWED_ORIGINS` instead of "the single tunnel origin from Tower config"** + (Codex) — **Reasoned substitution, now documented.** The tunnel subsystem exposes no clean + synchronous origin; the env var is an **exact-match** allowlist (not a wildcard), empty/secure by + default. CORS is defense-in-depth; the key check is the control. (Doc gap fixed above.) +- **`BRIDGE_MODE` doesn't fail when the key file is absent (it creates it)** (Codex) — **By design, + not a hole.** Tower owns key generation, so bridge mode always boots *with* enforced auth (a + random key) rather than refusing to start; the fail-closed path triggers when the key genuinely + cannot be obtained (unwritable `~/.agent-farm` → `getExpectedKey()` returns null → boot exits). + The fail-closed auth behavior on a null key is unit-tested (`getExpectedKey` + `isRequestAllowed`); + the boot-time `process.exit` itself runs at module load and isn't unit-tested. +- **`escapeHtml` renders `'`/`"` literally for quoted paths in ` (<); integer-validate ?line. ALL fixed. + * Client-side raw-HTML sinks in key-bearing pages (tower.html 12x innerHTML, + open.html markdown/HTML-preview/code-grid/annotations): VERIFIED already safe + - tower.html uses escapeHtml on every field; markdown via DOMPurify.sanitize; + HTML preview via iframe sandbox="allow-scripts" (no allow-same-origin -> opaque + origin, can't read parent key); code grid via Prism.highlight (escapes); + annotations via escapeHtml; line-numbers numeric. + * React SPA (apps/web): no innerHTML/dangerouslySetInnerHTML - React auto-escapes. + Sweep complete: no key-theft XSS path remains in any key-bearing page. +- pagehide bfcache guard (skip revoke when persisted). +Deferred (architect-agreed fast-follow): SSE-401 key-rotation recovery (rotation +deferred anyway); 401 WARN log spam on exposed Tower. + +Verify (iteration 4): codev build exit 0; full codev suite 4883 pass / 48 skip / +0 fail; request-auth 31 pass. + +## Reaching dev-approval gate (2026-08-12) + +All 5 core layers implemented + hardened across 4 iterations (3 cmap rounds). +Final state green: codev build 0; full suite 4883/0-fail; sdk 98; vscode 794; +apps/web 335; apps/web+vscode(main) tsc 0. Signalling porch done -> dev-approval. +Human live-verify needed for: dashboard load + WS terminal attach (direct +/workspace entry), VS Code terminals/gate/comments + SSE, tower.html, annotator +(text/image/video/pdf via authenticated blobs + vendored 3D viewer), LAN/bridge +access, tunnel terminals, and a no/wrong-key 401 path. + + +## dev-approval — BOOT REGRESSION found + fixed (2026-08-12) + +Human tried the build via local-install: Tower failed to start ("failed to +respond within 30000ms", zero log lines = crash at module load before the +logger). Diagnosed from tower.log + reproduced against the built/deployed tree. + +ROOT CAUSE (a ship-blocker my change introduced): `@cluesmith/codev-types` was a +**devDependency** of packages/codev — fine while codev only `import type`'d it +(erased at build). This lane added the FIRST runtime (value) import of it in the +server (WEB_KEY_HEADER / WS_* wire constants), so the installed/published build +tries to load codev-types at boot, but devDeps aren't installed for a consumer +artifact -> `Cannot find module '@cluesmith/codev-types'` -> boot crash. Would +have broken the published v3.3.0 release too, not just local-install. + +FIX: moved @cluesmith/codev-types from devDependencies to dependencies in +packages/codev/package.json (+ pnpm-lock.yaml). Proven with `pnpm deploy --prod +--legacy` (resolves only real deps, like a published install): codev-types/core/ +sdk now present and the deployed server-utils.js loads without the crash. + +LESSON (build+test did NOT catch this — monorepo symlinks resolve everything; +only the packaged artifact crashes). Saved to memory. Verified all runtime +@cluesmith value-imports in packages/codev are now in dependencies (core, sdk, +types). Commit f0f4afe2a. + +## dev-approval — startup readiness probe fix + header rename (2026-08-12) + +Two more issues surfaced by running the real build (build+test can't catch either): +1. BOOT: `afx tower start`'s readiness probe polls GET /api/status (commands/ + tower.ts) to detect "ready", but this change keyed /api/status -> probe 401'd + -> launcher never saw ready -> 30s timeout killed a healthy Tower. Fix: + authenticate the probe (send the key). /api/status keeps its boot-gated + readiness semantics. Commit 0ff32b63d. + +Header rename (user request): standardized codev-web-key -> codev-tower-key +(matches the already-'tower'-named WS subprotocol). Renamed the header, the +localStorage key, and the injected window global (__CODEV_TOWER_KEY__). Server +DUAL-ACCEPTS the legacy codev-web-key for one release so a lagging separately- +installed VS Code / Stream Deck (bundled old sdk) keeps working; added a +dual-accept test. Verified pre-change (main): server never read the header +(client-only aspiration), only the sdk-based clients sent it -> the rename is +server-safe, residual concern is lagging VS Code/Stream Deck (covered by +dual-accept). Wire name in codev-types (TOWER_KEY_HEADER + LEGACY_WEB_KEY_HEADER); +disk key file + WS subprotocol unchanged. Follow-up: drop the fallback next release. +Commit 11978b516. All suites green (codev 4884, sdk 98, web 335, vscode 794). + +## pr gate — CI fixes + three.js vendoring moved to build-time (2026-08-12) + +Two CI jobs were red (both direct consequences of enforcing request auth; neither +is catchable by in-package build+test because they only exercise the packaged / +served artifact): +1. Package Install Verification ran `afx --help` against an install missing + @cluesmith/codev-types (server-utils imports its wire constants at module + load), so the CLI crashed at boot. Fix: the workflow now builds + packs the + types tarball and passes it to verify-install (mirrors the local-install.sh + fix). Verified locally by packing all four tarballs and running verify-install: + codev/afx/porch/consult all respond. +2. Tower Integration Tests hit the now-authenticated Tower with keyless fetch / + WebSocket calls -> 401. Fix: an e2e-only global fetch wrapper + (vitest-e2e-setup.ts) injects the codev-tower-key header for loopback Tower + requests, and a towerWsProtocols() helper carries the key on the four terminal/ + message sockets. Two affected e2e files pass (17/17). All 7 checks green. + +three.js vendoring: reduced the PR from +60k to a normal-sized diff. The 3D viewer +is a key-bearing page (holds the injected Tower key to fetch the keyed api/model +route), so it must run zero remote code -> three.js is served same-origin from +vendor/ rather than a CDN. But the ~58k library lines don't need to live in git: +added `three` as an exact-pinned devDependency and a build step (scripts/ +copy-three.mjs) that regenerates the five vendor files at build time. Output is +byte-identical to what was committed (proven: git diff empty after regenerate), +so behaviour is unchanged; the importmap and file names are untouched. The files +are .gitignored and ship in the npm tarball via the `files` allowlist, exactly +like skeleton/ and dashboard-dist/ (dry-run confirms all 5 present in the tarball). diff --git a/packages/codev/package.json b/packages/codev/package.json index e9d4b5b6c..7a2950e14 100644 --- a/packages/codev/package.json +++ b/packages/codev/package.json @@ -22,8 +22,9 @@ ], "scripts": { "clean": "rm -rf dist", - "build": "pnpm --filter \"@cluesmith/codev^...\" build && pnpm clean && tsc && pnpm copy-dashboard && pnpm copy-skeleton", + "build": "pnpm --filter \"@cluesmith/codev^...\" build && pnpm clean && tsc && pnpm copy-dashboard && pnpm copy-skeleton && pnpm copy-three", "copy-dashboard": "rm -rf dashboard-dist && cp -r ../../apps/web/dist dashboard-dist", + "copy-three": "node ./scripts/copy-three.mjs", "dev:dashboard": "cd ../../apps/web && pnpm dev", "copy-skeleton": "rm -rf skeleton && cp -r ../../codev-skeleton skeleton", "dev": "tsx src/cli.ts", @@ -37,9 +38,10 @@ "prepublishOnly": "pnpm build" }, "dependencies": { + "@anthropic-ai/claude-agent-sdk": "^0.2.41", "@cluesmith/codev-core": "workspace:*", "@cluesmith/codev-sdk": "workspace:*", - "@anthropic-ai/claude-agent-sdk": "^0.2.41", + "@cluesmith/codev-types": "workspace:*", "@google/genai": "^1.0.0", "@openai/codex-sdk": "^0.146.0", "@xterm/addon-serialize": "^0.14.0", @@ -56,16 +58,16 @@ }, "devDependencies": { "@cluesmith/codev-web": "workspace:*", - "@cluesmith/codev-types": "workspace:*", "@playwright/test": "^1.58.0", - "@xterm/xterm": "^5.5.0", "@types/better-sqlite3": "^7.6.13", "@types/js-yaml": "^4.0.9", "@types/node": "^22.10.1", "@types/ws": "^8.18.1", "@vitest/coverage-v8": "^4.0.18", + "@xterm/xterm": "^5.5.0", "jsdom": "^28.1.0", "playwright": "^1.58.0", + "three": "0.160.0", "tsx": "^4.19.2", "typescript": "catalog:", "vitest": "^4.0.15" diff --git a/packages/codev/scripts/copy-three.mjs b/packages/codev/scripts/copy-three.mjs new file mode 100644 index 000000000..42cdd7778 --- /dev/null +++ b/packages/codev/scripts/copy-three.mjs @@ -0,0 +1,81 @@ +#!/usr/bin/env node + +/** + * Build step: vendor three.js into templates/vendor/ from the `three` + * devDependency, instead of committing ~58k lines of library source. + * + * Why vendored at all: the 3D model viewer (templates/3d-viewer.html) is a + * key-bearing page — it holds the injected Tower key to fetch the keyed + * `api/model` route. A key-bearing page must run zero remote code, so three.js + * is served same-origin from vendor/ rather than a CDN (advisory + * GHSA-xvjp-7748-v88v). This script keeps that property while moving the bytes + * out of git: the copied files are .gitignored and regenerated at build time, + * and ship in the npm tarball via the package's `files` allowlist (the same + * mechanism skeleton/ and dashboard-dist/ already rely on). + * + * Output is byte-identical to three@0.160.0's published files, except one line + * in 3MFLoader: its `../libs/fflate.module.js` relative import is rewritten to + * the bare `fflate` specifier so it resolves through the page's importmap + * (which maps `fflate` -> vendor/three-fflate.module.js). Do not edit the + * generated files — edit this script or bump the `three` devDependency. + */ + +import { createRequire } from 'node:module'; +import { dirname, join, resolve } from 'node:path'; +import { existsSync, readFileSync, copyFileSync, writeFileSync } from 'node:fs'; + +const require = createRequire(import.meta.url); + +// Resolve the `three` package root by walking up from its resolved entry point +// to the directory whose package.json is name === "three". (three's exports map +// blocks a direct require.resolve('three/package.json'), so we resolve the entry +// and climb.) +function findThreeRoot() { + let dir = dirname(require.resolve('three')); + for (let i = 0; i < 8; i++) { + const pkg = join(dir, 'package.json'); + if (existsSync(pkg)) { + try { + if (JSON.parse(readFileSync(pkg, 'utf8')).name === 'three') return dir; + } catch { + /* keep climbing */ + } + } + const parent = dirname(dir); + if (parent === dir) break; + dir = parent; + } + throw new Error('Could not locate the `three` package root — is it installed?'); +} + +const threeRoot = findThreeRoot(); +const vendorDir = resolve(import.meta.dirname, '..', 'templates', 'vendor'); + +// source (relative to three package root) -> dest flat filename in vendor/ +const VERBATIM = [ + ['build/three.module.js', 'three.module.js'], + ['examples/jsm/loaders/STLLoader.js', 'three-STLLoader.js'], + ['examples/jsm/controls/TrackballControls.js', 'three-TrackballControls.js'], + ['examples/jsm/libs/fflate.module.js', 'three-fflate.module.js'], +]; + +for (const [src, dest] of VERBATIM) { + copyFileSync(join(threeRoot, src), join(vendorDir, dest)); +} + +// 3MFLoader: rewrite the one relative fflate import to the importmap specifier. +const mfSrc = readFileSync(join(threeRoot, 'examples/jsm/loaders/3MFLoader.js'), 'utf8'); +const mfRewritten = mfSrc.replace( + "import * as fflate from '../libs/fflate.module.js';", + "import * as fflate from 'fflate';", +); +if (mfRewritten === mfSrc) { + throw new Error( + "copy-three: expected fflate import to rewrite in 3MFLoader.js but no match was found — " + + 'the `three` version may have changed its import path.', + ); +} +writeFileSync(join(vendorDir, 'three-3MFLoader.js'), mfRewritten); + +const version = JSON.parse(readFileSync(join(threeRoot, 'package.json'), 'utf8')).version; +console.log(`copy-three: vendored three@${version} (5 files) into templates/vendor/`); diff --git a/packages/codev/src/agent-farm/__tests__/helpers/tower-test-utils.ts b/packages/codev/src/agent-farm/__tests__/helpers/tower-test-utils.ts index d6b28d85c..db349635d 100644 --- a/packages/codev/src/agent-farm/__tests__/helpers/tower-test-utils.ts +++ b/packages/codev/src/agent-farm/__tests__/helpers/tower-test-utils.ts @@ -9,9 +9,21 @@ import { resolve } from 'node:path'; import { mkdtempSync, rmSync, mkdirSync, writeFileSync } from 'node:fs'; import { tmpdir, homedir } from 'node:os'; import net from 'node:net'; +import { ensureLocalKey } from '@cluesmith/codev-core/auth'; +import { terminalWsProtocols } from '@cluesmith/codev-types'; const TOWER_START_TIMEOUT = 15_000; +/** + * WebSocket subprotocols carrying the shared local key, for authenticated + * terminal/message sockets against the test Tower. Tower enforces request + * authentication (advisory GHSA-xvjp-7748-v88v); a keyless upgrade is rejected + * at the handshake. HTTP calls are keyed centrally in vitest-e2e-setup.ts. + */ +export function towerWsProtocols(): string[] | undefined { + return terminalWsProtocols(ensureLocalKey()); +} + // Path to compiled tower-server.js (4 levels up from helpers/ to packages/codev/) const TOWER_SERVER_PATH = resolve( import.meta.dirname, diff --git a/packages/codev/src/agent-farm/__tests__/inbox-routes.test.ts b/packages/codev/src/agent-farm/__tests__/inbox-routes.test.ts index 7ee276a2e..cfbead7e9 100644 --- a/packages/codev/src/agent-farm/__tests__/inbox-routes.test.ts +++ b/packages/codev/src/agent-farm/__tests__/inbox-routes.test.ts @@ -58,7 +58,8 @@ vi.mock('../utils/message-format.js', () => ({ formatArchitectMessage: vi.fn((msg: string) => msg), formatBuilderMessage: vi.fn((id: string, msg: string) => `[${id}] ${msg}`), })); -vi.mock('../utils/server-utils.js', () => ({ +vi.mock('../utils/server-utils.js', async (importActual) => ({ + ...(await importActual()), parseJsonBody: vi.fn(async () => ({})), isRequestAllowed: vi.fn(() => true), })); diff --git a/packages/codev/src/agent-farm/__tests__/request-auth.test.ts b/packages/codev/src/agent-farm/__tests__/request-auth.test.ts new file mode 100644 index 000000000..086e430ed --- /dev/null +++ b/packages/codev/src/agent-farm/__tests__/request-auth.test.ts @@ -0,0 +1,278 @@ +/** + * Request-authentication enforcement (advisory GHSA-xvjp-7748-v88v). + * + * Exercises the REAL server-side auth helpers (no isRequestAllowed stub): the + * public-route allowlist, constant-time key comparison, CORS origin allowlist, + * and the HTTP + WebSocket key checks. The expected key is controlled by mocking + * codev-core's ensureLocalKey. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type * as http from 'node:http'; +import { WS_MARKER_PROTOCOL, WS_KEY_PROTOCOL_PREFIX } from '@cluesmith/codev-types'; + +const TEST_KEY = 'a'.repeat(64); + +vi.mock('@cluesmith/codev-core/auth', () => ({ + ensureLocalKey: vi.fn(() => TEST_KEY), + readLocalKey: vi.fn(() => TEST_KEY), +})); + +import { + isPublicRoute, + keysMatch, + isAllowedOrigin, + isAllowedHost, + isRequestAllowed, + isWebSocketAllowed, + selectWsSubprotocol, + getExpectedKey, + resetExpectedKeyCache, +} from '../utils/server-utils.js'; +import { ensureLocalKey } from '@cluesmith/codev-core/auth'; + +function req(method: string, url: string, headers: Record = {}): http.IncomingMessage { + // Default to a loopback Host so the Host guard passes unless a test overrides it. + return { method, url, headers: { host: 'localhost:4100', ...headers } } as unknown as http.IncomingMessage; +} + +beforeEach(() => { + resetExpectedKeyCache(); + const mock = ensureLocalKey as unknown as ReturnType; + mock.mockReset(); + mock.mockReturnValue(TEST_KEY); + delete process.env.CODEV_TOWER_ALLOWED_ORIGINS; +}); + +describe('isPublicRoute', () => { + it('allows pre-auth probes and the dashboard shell (GET only)', () => { + expect(isPublicRoute('GET', '/health')).toBe(true); + expect(isPublicRoute('GET', '/api/version')).toBe(true); + expect(isPublicRoute('GET', '/')).toBe(true); + expect(isPublicRoute('GET', '/index.html')).toBe(true); + }); + + it('allows React SPA static assets under /workspace//', () => { + expect(isPublicRoute('GET', '/workspace/ENC/')).toBe(true); + expect(isPublicRoute('GET', '/workspace/ENC')).toBe(true); // bare, no trailing slash + expect(isPublicRoute('GET', '/workspace/ENC/assets/app.js')).toBe(true); + expect(isPublicRoute('GET', '/workspace/ENC/index.html')).toBe(true); + }); + + it('requires the key for workspace api / ws / file routes', () => { + expect(isPublicRoute('GET', '/workspace/ENC/api/state')).toBe(false); + expect(isPublicRoute('GET', '/workspace/ENC/ws/terminal/x')).toBe(false); + expect(isPublicRoute('GET', '/workspace/ENC/file')).toBe(false); + }); + + it('requires the key for top-level api routes and all mutations', () => { + expect(isPublicRoute('GET', '/api/terminals')).toBe(false); + expect(isPublicRoute('GET', '/api/overview')).toBe(false); + expect(isPublicRoute('POST', '/health')).toBe(false); + expect(isPublicRoute('POST', '/')).toBe(false); + expect(isPublicRoute('DELETE', '/workspace/ENC/assets/app.js')).toBe(false); + }); + + it('makes only the annotator shell + vendor public; its data/media routes stay keyed', () => { + // Shell (iframe navigation) and vendor libs (` can never become stored XSS in a shell. + // A malformed key yields no injection (clients then fail closed with 401). + const injection = key && /^[0-9a-f]{64}$/.test(key) + ? `` + : ''; + if (html.includes('')) { + return html.replace('', injection); + } + if (injection && html.includes('')) { + return html.replace('', `${injection}`); + } + return html; +} + +/** + * Send a key-injected HTML shell. Strips the CORS `Access-Control-Allow-Origin` + * header set by the front door: this response body carries the key, and the + * shell is only ever loaded by a same-origin navigation, so it must never be + * readable by a cross-origin `fetch` (advisory GHSA-xvjp-7748-v88v). Same-origin + * reads are unaffected; the key check still guards the actual API routes. + */ +function sendKeyInjectedHtml(res: http.ServerResponse, template: string): void { + res.removeHeader('Access-Control-Allow-Origin'); + res.removeHeader('Vary'); + res.writeHead(200, { 'Content-Type': 'text/html; charset=utf-8' }); + res.end(injectWebKey(template)); +} + +/** + * Serve the React dashboard's index.html with the key injected (SPA shell + SPA + * client-side-routing fallback). Returns false if the file cannot be read. + */ +function serveDashboardIndex(dashboardPath: string, res: http.ServerResponse): boolean { + try { + const html = fs.readFileSync(path.join(dashboardPath, 'index.html'), 'utf-8'); + sendKeyInjectedHtml(res, html); + return true; + } catch { + return false; + } +} + // ============================================================================ // Workspace-scoped route handler // ============================================================================ @@ -2449,23 +2515,24 @@ async function handleWorkspaceRoutes( // 3. React dashboard is available // 4. Workspace doesn't need to be running for static files if (!isApiCall && !isWsPath && ctx.hasReactDashboard) { - // Determine which static file to serve - let staticPath: string; - if (!subPath || subPath === '' || subPath === 'index.html') { - staticPath = path.join(ctx.reactDashboardPath, 'index.html'); + // The SPA shell (index.html) is served with the shared key injected same-origin + // (advisory GHSA-xvjp-7748-v88v) so a direct navigation to a workspace URL can + // authenticate without first visiting the Tower root. Other static assets + // (JS/CSS/images) carry no secret and stream as-is. + const isIndex = !subPath || subPath === '' || subPath === 'index.html'; + if (isIndex) { + if (serveDashboardIndex(ctx.reactDashboardPath, res)) { + return; + } } else { - // Check if it's a static asset - staticPath = path.join(ctx.reactDashboardPath, subPath); - } - - // Try to serve the static file - if (serveStaticFile(staticPath, res)) { - return; + const staticPath = path.join(ctx.reactDashboardPath, subPath); + if (serveStaticFile(staticPath, res)) { + return; + } } - // SPA fallback: serve index.html for client-side routing - const indexPath = path.join(ctx.reactDashboardPath, 'index.html'); - if (serveStaticFile(indexPath, res)) { + // SPA fallback: serve the (key-injected) index.html for client-side routing. + if (serveDashboardIndex(ctx.reactDashboardPath, res)) { return; } } @@ -3529,15 +3596,23 @@ function handleWorkspaceAnnotate( const fileName = path.basename(filePath); const fileSize = fs.statSync(filePath).size; + // The shell carries the injected key (advisory GHSA-xvjp-7748-v88v), so a + // maliciously-named file must not become XSS (which would read the key). + // HTML-escape values that land in markup/attributes (safe in the JS string + // contexts too — it blocks `<`, `"`, `'`), and for the JSON-in-` can't break out. + const safeFileName = escapeHtml(fileName); + const safeFilePath = escapeHtml(filePath); + const filePathJson = JSON.stringify(filePath).replace(/r.text()).then(init);`; + initScript = `fetch('file',{headers:authHeaders()}).then(r=>r.text()).then(init);`; } html = html.replace('// FILE_CONTENT will be injected by the server', initScript); } - // Handle ?line= query param for scroll-to-line + // Handle ?line= query param for scroll-to-line. Validate as a bare integer + // before interpolating into the script — untrusted query input must never + // reach the key-bearing shell's markup unescaped. const lineParam = url.searchParams.get('line'); - if (lineParam) { + if (lineParam && /^\d+$/.test(lineParam)) { const scrollScript = ``; html = html.replace('', `${scrollScript}`); } - res.writeHead(200, { 'Content-Type': 'text/html; charset=utf-8' }); - res.end(html); + // Same-origin key injection so the annotator's fetches can authenticate + // (advisory GHSA-xvjp-7748-v88v). The shell is public (iframe navigation); + // its data/media sub-routes stay keyed. + sendKeyInjectedHtml(res, html); } catch (err) { res.writeHead(500, { 'Content-Type': 'text/plain' }); res.end(`Failed to serve annotator: ${(err as Error).message}`); diff --git a/packages/codev/src/agent-farm/servers/tower-server.ts b/packages/codev/src/agent-farm/servers/tower-server.ts index 3d31eab92..c70289253 100644 --- a/packages/codev/src/agent-farm/servers/tower-server.ts +++ b/packages/codev/src/agent-farm/servers/tower-server.ts @@ -63,7 +63,7 @@ import { setCodevConfigNotifier, stopAllCodevConfigWatchers } from './codev-conf import { getGlobalDb } from '../db/index.js'; import { runBootConsolidation } from '../db/consolidate.js'; import { DEFAULT_TOWER_PORT, AGENT_FARM_DIR } from '../lib/tower-client.js'; -import { validateHost } from '../utils/server-utils.js'; +import { validateHost, getExpectedKey, selectWsSubprotocol } from '../utils/server-utils.js'; import { version } from '../../version.js'; const __filename = fileURLToPath(import.meta.url); @@ -115,6 +115,15 @@ const bindHost = bridgeMode ? validateHost(process.env.BRIDGE_TOWER_HOST || '127.0.0.1') : '127.0.0.1'; +// Request authentication (advisory GHSA-xvjp-7748-v88v): ensure the shared local +// key exists at boot so HTTP/WS enforcement has an expected value to compare +// against. getExpectedKey() issues the key if missing (Tower owns generation) +// and returns null only if it cannot be created (e.g. an unwritable +// ~/.agent-farm). Enforcement fails closed in that case; under BRIDGE_MODE the +// bind is non-localhost, so refuse to start a network-reachable Tower with no +// request authentication at all. +const expectedKeyAtBoot = getExpectedKey(); + // Logging utility function log(level: 'INFO' | 'ERROR' | 'WARN', message: string): void { const timestamp = new Date().toISOString(); @@ -454,7 +463,13 @@ const server = http.createServer(async (req, res) => { // Bridge mode enables non-localhost binding when BRIDGE_MODE=1 is set. server.listen(port, bindHost, () => { if (bridgeMode) { - log('WARN', `Bridge mode is ENABLED — Tower is listening on ${bindHost} network interfaces.`); + if (!expectedKeyAtBoot) { + log('ERROR', 'BRIDGE_MODE requires the shared local key at ~/.agent-farm/local-key, which could not be created. Refusing to start a network-reachable Tower without request authentication.'); + process.exit(1); + } + log('WARN', `Bridge mode is ENABLED — Tower is listening on ${bindHost} network interfaces. Request authentication is enforced, but the shared key travels in cleartext over plain HTTP — terminate TLS at the tunnel/proxy so the key is not exposed on the wire.`); + } else if (!expectedKeyAtBoot) { + log('WARN', 'Shared local key could not be created at ~/.agent-farm/local-key; Tower will reject authenticated requests (fail closed).'); } // Display localhost in URLs for local UX even when bound to all interfaces. const displayHost = bindHost === '0.0.0.0' ? 'localhost' : bindHost; @@ -739,7 +754,14 @@ async function bootSequence(): Promise { } // Initialize terminal WebSocket server (Phase 2 - Spec 0090) -terminalWss = new WebSocketServer({ noServer: true }); +// handleProtocols echoes the non-secret marker subprotocol back on the +// handshake (advisory GHSA-xvjp-7748-v88v Layer 2) so strict `ws` clients that +// offer it accept the connection; the `codev-key.` token the client also +// offers is validated at the upgrade and never echoed. +terminalWss = new WebSocketServer({ + noServer: true, + handleProtocols: (protocols: Set) => selectWsSubprotocol(protocols), +}); // Spec 0105 Phase 5: WebSocket upgrade handler extracted to tower-websocket.ts setupUpgradeHandler(server, terminalWss, port); diff --git a/packages/codev/src/agent-farm/servers/tower-websocket.ts b/packages/codev/src/agent-farm/servers/tower-websocket.ts index 60593c67d..472d001ca 100644 --- a/packages/codev/src/agent-farm/servers/tower-websocket.ts +++ b/packages/codev/src/agent-farm/servers/tower-websocket.ts @@ -9,7 +9,8 @@ import http from 'node:http'; import type net from 'node:net'; import { WebSocketServer, WebSocket } from 'ws'; -import { WS_CLOSE_SESSION_UNKNOWN } from '../lib/reconnect-backoff.js'; +import { WS_CLOSE_SESSION_UNKNOWN, WS_CLOSE_UNAUTHORIZED } from '../lib/reconnect-backoff.js'; +import { isWebSocketAllowed } from '../utils/server-utils.js'; import { encodeData, encodeControl, decodeFrame } from '../../terminal/ws-protocol.js'; import type { PtySession } from '../../terminal/pty-session.js'; import { attachWithReplay } from '../../terminal/attach-replay.js'; @@ -186,6 +187,31 @@ function rejectUnknownSession( socket.destroy(); } +/** + * Reject an upgrade that failed request authentication (advisory + * GHSA-xvjp-7748-v88v). Mirrors {@link rejectUnknownSession}'s two client + * shapes: a browser (has `Origin`, can't read a failed upgrade's HTTP status) + * gets an accepted-then-closed handshake with {@link WS_CLOSE_UNAUTHORIZED}; + * a Node `ws` client gets the HTTP-stage `401`. This runs BEFORE any session + * lookup and is independent of `Origin`, so a missing `Origin` cannot degrade + * into an auth bypass. + */ +function rejectUnauthorized( + req: http.IncomingMessage, + socket: net.Socket, + head: Buffer, + wss: WebSocketServer, +): void { + if (req.headers.origin) { + wss.handleUpgrade(req, socket, head, (ws) => { + ws.close(WS_CLOSE_UNAUTHORIZED, 'unauthorized'); + }); + return; + } + socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n'); + socket.destroy(); +} + /** * Set up the WebSocket upgrade handler on the HTTP server. * Parses upgrade requests and routes them to the appropriate terminal session: @@ -200,6 +226,15 @@ export function setupUpgradeHandler( server.on('upgrade', async (req: http.IncomingMessage, socket: net.Socket, head: Buffer) => { const reqUrl = new URL(req.url || '/', `http://localhost:${port}`); + // Request authentication (advisory GHSA-xvjp-7748-v88v): validate the key at + // the handshake, before any session lookup or PTY attach, for every WS route. + // Independent of the Origin header so a missing Origin cannot bypass auth. + if (!isWebSocketAllowed(req)) { + logTerminal('WARN', `WS upgrade 401 ${reqUrl.pathname} — disallowed Host or missing/invalid key`); + rejectUnauthorized(req, socket, head, wss); + return; + } + // Phase 2: Handle /ws/terminal/:id routes directly const terminalMatch = reqUrl.pathname.match(/^\/ws\/terminal\/([^/]+)$/); if (terminalMatch) { diff --git a/packages/codev/src/agent-farm/utils/server-utils.ts b/packages/codev/src/agent-farm/utils/server-utils.ts index 1a1b45ec5..369567c4a 100644 --- a/packages/codev/src/agent-farm/utils/server-utils.ts +++ b/packages/codev/src/agent-farm/utils/server-utils.ts @@ -5,6 +5,9 @@ */ import type * as http from 'node:http'; +import { timingSafeEqual } from 'node:crypto'; +import { ensureLocalKey } from '@cluesmith/codev-core/auth'; +import { TOWER_KEY_HEADER, LEGACY_WEB_KEY_HEADER, WS_MARKER_PROTOCOL, WS_KEY_PROTOCOL_PREFIX } from '@cluesmith/codev-types'; /** * HTML-escape a string to prevent XSS @@ -71,14 +74,292 @@ export function parseJsonBody(req: http.IncomingMessage, maxSize = 1024 * 1024): }); } +// ============================================================================ +// Request authentication (advisory GHSA-xvjp-7748-v88v) +// ============================================================================ +// +// Tower's local HTTP + WebSocket API reaches privileged local operations, so +// every request that is not on the narrow public-route allowlist must present +// the shared local key (`~/.agent-farm/local-key`) in the `codev-tower-key` +// header. Enforcement is server-side only (server/client isolation, #1189): +// clients merely transport the key. + +// Wire-contract names (header + WS subprotocols) live in `@cluesmith/codev-types` +// so the server and every client share one source of truth. + +/** + * Cached expected key. `undefined` = not yet loaded; `null` = load failed + * (fail closed — reject every authenticated request). Tower owns generation, + * so under normal operation the key file exists after boot. + */ +let cachedExpectedKey: string | null | undefined; + +/** + * The expected local key, cached after first read. Issues the key if missing + * (Tower is the owner). Returns null and stays fail-closed if the key cannot be + * read or created (e.g. an unwritable `~/.agent-farm`). + */ +export function getExpectedKey(): string | null { + if (cachedExpectedKey === undefined) { + try { + cachedExpectedKey = ensureLocalKey() || null; + } catch { + cachedExpectedKey = null; + } + } + return cachedExpectedKey; +} + +/** + * Reset the cached key. Test-only seam; also lets a future rotation path force + * a re-read. Not wired to any runtime rotation in this change. + */ +export function resetExpectedKeyCache(): void { + cachedExpectedKey = undefined; +} + +/** + * Constant-time key comparison. `timingSafeEqual` throws on unequal-length + * buffers, so length is checked first (a length mismatch is an immediate, + * non-secret reject). + */ +export function keysMatch(presented: string, expected: string): boolean { + const a = Buffer.from(presented, 'utf8'); + const b = Buffer.from(expected, 'utf8'); + if (a.length !== b.length) return false; + return timingSafeEqual(a, b); +} + +/** + * Routes intentionally reachable without the key. Kept deliberately narrow: + * pre-auth liveness/version probes, the Tower launcher shell, and the React + * dashboard's static assets (the page loads keyless, then authenticates its + * own API/WebSocket calls with the key). Everything else requires the key. + * + * The privileged workspace `file` reader and every `api/` or `ws/` subpath are + * explicitly excluded so a static-asset carve-out never exposes a data route. + */ +export function isPublicRoute(method: string, pathname: string): boolean { + if (method !== 'GET') return false; + + if (pathname === '/health') return true; + if (pathname === '/api/version') return true; + if (pathname === '/' || pathname === '/index.html') return true; + + // React SPA served under /workspace//... — static assets only. The + // trailing subpath is optional: bare /workspace/ serves the SPA shell, + // same as /workspace// (handleWorkspaceRoutes treats them identically). + const workspaceMatch = pathname.match(/^\/workspace\/[^/]+(?:\/(.*))?$/); + if (workspaceMatch) { + const subPath = workspaceMatch[1] || ''; + + // Annotator: its HTML shell and vendor libraries are loaded by iframe + // navigation and @@ -234,6 +240,24 @@ import { ThreeMFLoader } from 'three/addons/loaders/3MFLoader.js'; import { TrackballControls } from 'three/addons/controls/TrackballControls.js'; + // Request authentication (advisory GHSA-xvjp-7748-v88v): Tower injects the + // shared local key into this page same-origin; model/mtime requests send it + // via the codev-tower-key header. Three.js loaders carry it through + // loader.setRequestHeader(authHeaders()). + function getWebKey() { + if (window.__CODEV_TOWER_KEY__) { + try { localStorage.setItem('codev-tower-key', window.__CODEV_TOWER_KEY__); } catch (e) {} + return window.__CODEV_TOWER_KEY__; + } + return localStorage.getItem('codev-tower-key'); + } + function authHeaders(extra) { + const key = getWebKey(); + const headers = extra ? Object.assign({}, extra) : {}; + if (key) headers['codev-tower-key'] = key; + return headers; + } + // Configuration (injected by server with proper escaping) const FILE_PATH = {{FILE_PATH_JSON}}; // JSON-encoded by server const FILE_NAME = '{{FILE}}'; @@ -370,6 +394,7 @@ function loadSTL() { const loader = new STLLoader(); + loader.setRequestHeader(authHeaders()); loader.load( 'api/model', @@ -412,6 +437,7 @@ function load3MF() { const loader = new ThreeMFLoader(); + loader.setRequestHeader(authHeaders()); loader.load( 'api/model', @@ -715,7 +741,7 @@ async function checkForChanges() { try { - const res = await fetch('api/mtime'); + const res = await fetch('api/mtime', { headers: authHeaders() }); if (res.ok) { const data = await res.json(); if (lastMtime === null) { @@ -769,6 +795,7 @@ function reloadSTL() { const loader = new STLLoader(); + loader.setRequestHeader(authHeaders()); loader.load('api/model?t=' + Date.now(), (geometry) => { // Center geometry in XY geometry.computeBoundingBox(); @@ -811,6 +838,7 @@ function reload3MF() { const loader = new ThreeMFLoader(); + loader.setRequestHeader(authHeaders()); loader.load('api/model?t=' + Date.now(), (group) => { // 3MF uses Z-up natively, which matches our coordinate system diff --git a/packages/codev/templates/open.html b/packages/codev/templates/open.html index 167535f0d..bdd14a50f 100644 --- a/packages/codev/templates/open.html +++ b/packages/codev/templates/open.html @@ -569,6 +569,51 @@

Review Comments