From 02069e4a2c7de9e627ac63f2e191b4250edf7a81 Mon Sep 17 00:00:00 2001 From: Saeed Al Mansouri Date: Tue, 24 Mar 2026 22:55:50 +0400 Subject: [PATCH 1/3] fix(daemon): add token-based authentication for local connections The existing CSRF protections (Origin check, X-OpenCLI header) block browser-based attacks but don't protect against local process impersonation. Any process on the machine that knows the port and static header value can connect to the daemon and access all browser sessions. This adds a random shared secret (stored at ~/.opencli/token) that the daemon requires on all HTTP and WebSocket connections: - Token is generated on first daemon start (32 random bytes, hex-encoded) - File is created with mode 0o600 (owner-only read/write) - HTTP requests must include x-opencli-token header - WebSocket connections pass the token via query parameter or Sec-WebSocket-Protocol header - CLI and discover code updated to read and send the token This closes the local privilege escalation gap identified in #395. Closes #395 --- src/browser/daemon-client.ts | 14 +++++++-- src/browser/discover.ts | 8 ++++-- src/daemon.ts | 34 ++++++++++++++++++---- src/token.ts | 56 ++++++++++++++++++++++++++++++++++++ 4 files changed, 101 insertions(+), 11 deletions(-) create mode 100644 src/token.ts diff --git a/src/browser/daemon-client.ts b/src/browser/daemon-client.ts index 73d0f979..3e126ca3 100644 --- a/src/browser/daemon-client.ts +++ b/src/browser/daemon-client.ts @@ -5,11 +5,19 @@ */ import { DEFAULT_DAEMON_PORT } from '../constants.js'; +import { readToken, TOKEN_HEADER } from '../token.js'; import type { BrowserSessionInfo } from '../types.js'; const DAEMON_PORT = parseInt(process.env.OPENCLI_DAEMON_PORT ?? String(DEFAULT_DAEMON_PORT), 10); const DAEMON_URL = `http://127.0.0.1:${DAEMON_PORT}`; +function authHeaders(): Record { + const headers: Record = { 'X-OpenCLI': '1' }; + const token = readToken(); + if (token) headers[TOKEN_HEADER] = token; + return headers; +} + let _idCounter = 0; function generateId(): string { @@ -46,7 +54,7 @@ export async function isDaemonRunning(): Promise { const controller = new AbortController(); const timer = setTimeout(() => controller.abort(), 2000); const res = await fetch(`${DAEMON_URL}/status`, { - headers: { 'X-OpenCLI': '1' }, + headers: authHeaders(), signal: controller.signal, }); clearTimeout(timer); @@ -64,7 +72,7 @@ export async function isExtensionConnected(): Promise { const controller = new AbortController(); const timer = setTimeout(() => controller.abort(), 2000); const res = await fetch(`${DAEMON_URL}/status`, { - headers: { 'X-OpenCLI': '1' }, + headers: authHeaders(), signal: controller.signal, }); clearTimeout(timer); @@ -97,7 +105,7 @@ export async function sendCommand( const res = await fetch(`${DAEMON_URL}/command`, { method: 'POST', - headers: { 'Content-Type': 'application/json', 'X-OpenCLI': '1' }, + headers: { 'Content-Type': 'application/json', ...authHeaders() }, body: JSON.stringify(command), signal: controller.signal, }); diff --git a/src/browser/discover.ts b/src/browser/discover.ts index a73cd959..78af94d7 100644 --- a/src/browser/discover.ts +++ b/src/browser/discover.ts @@ -6,6 +6,7 @@ */ import { DEFAULT_DAEMON_PORT } from '../constants.js'; +import { readToken, TOKEN_HEADER } from '../token.js'; import { isDaemonRunning } from './daemon-client.js'; export { isDaemonRunning }; @@ -19,9 +20,10 @@ export async function checkDaemonStatus(): Promise<{ }> { try { const port = parseInt(process.env.OPENCLI_DAEMON_PORT ?? String(DEFAULT_DAEMON_PORT), 10); - const res = await fetch(`http://127.0.0.1:${port}/status`, { - headers: { 'X-OpenCLI': '1' }, - }); + const headers: Record = { 'X-OpenCLI': '1' }; + const token = readToken(); + if (token) headers[TOKEN_HEADER] = token; + const res = await fetch(`http://127.0.0.1:${port}/status`, { headers }); const data = await res.json() as { ok: boolean; extensionConnected: boolean }; return { running: true, extensionConnected: data.extensionConnected }; } catch { diff --git a/src/daemon.ts b/src/daemon.ts index c39e006c..38a1d9ad 100644 --- a/src/daemon.ts +++ b/src/daemon.ts @@ -12,6 +12,7 @@ * 3. No CORS headers — responses never include Access-Control-Allow-Origin * 4. Body size limit — 1 MB max to prevent OOM * 5. WebSocket verifyClient — reject upgrade before connection is established + * 6. Token auth — random secret in ~/.opencli/token required on all connections * * Lifecycle: * - Auto-spawned by opencli on first browser command @@ -22,8 +23,10 @@ import { createServer, type IncomingMessage, type ServerResponse } from 'node:http'; import { WebSocketServer, WebSocket, type RawData } from 'ws'; import { DEFAULT_DAEMON_PORT } from './constants.js'; +import { getOrCreateToken, TOKEN_HEADER } from './token.js'; const PORT = parseInt(process.env.OPENCLI_DAEMON_PORT ?? String(DEFAULT_DAEMON_PORT), 10); +const DAEMON_TOKEN = getOrCreateToken(); const IDLE_TIMEOUT = 5 * 60 * 1000; // 5 minutes // ─── State ─────────────────────────────────────────────────────────── @@ -110,6 +113,15 @@ async function handleRequest(req: IncomingMessage, res: ServerResponse): Promise return; } + // Token auth — require the shared secret from ~/.opencli/token. + // This blocks local processes that don't have filesystem access to the + // token file, even if they know the port and X-OpenCLI header. + const clientToken = req.headers[TOKEN_HEADER] as string | undefined; + if (clientToken !== DAEMON_TOKEN) { + jsonResponse(res, 401, { ok: false, error: 'Unauthorized: invalid or missing token' }); + return; + } + const url = req.url ?? '/'; const pathname = url.split('?')[0]; @@ -184,12 +196,24 @@ const wss = new WebSocketServer({ server: httpServer, path: '/ext', verifyClient: ({ req }: { req: IncomingMessage }) => { - // Block browser-originated WebSocket connections. Browsers don't - // enforce CORS on WebSocket, so a malicious webpage could connect to - // ws://localhost:19825/ext and impersonate the Extension. Real Chrome - // Extensions send origin chrome-extension://. + // 1. Block browser-originated WebSocket connections. Browsers don't + // enforce CORS on WebSocket, so a malicious webpage could connect to + // ws://localhost:19825/ext and impersonate the Extension. Real Chrome + // Extensions send origin chrome-extension://. const origin = req.headers['origin'] as string | undefined; - return !origin || origin.startsWith('chrome-extension://'); + if (origin && !origin.startsWith('chrome-extension://')) return false; + + // 2. Token auth — require the shared secret on WebSocket connections too. + // The token is passed via the Sec-WebSocket-Protocol header or a query + // parameter, since custom headers aren't available in the WebSocket + // constructor. + const url = new URL(req.url ?? '/', `http://localhost:${PORT}`); + const tokenFromQuery = url.searchParams.get('token'); + const tokenFromProtocol = req.headers['sec-websocket-protocol'] as string | undefined; + const clientToken = tokenFromQuery ?? tokenFromProtocol; + if (clientToken !== DAEMON_TOKEN) return false; + + return true; }, }); diff --git a/src/token.ts b/src/token.ts new file mode 100644 index 00000000..dded6acc --- /dev/null +++ b/src/token.ts @@ -0,0 +1,56 @@ +/** + * Daemon authentication token — shared secret between CLI, daemon, and extension. + * + * On first run, a random token is generated and stored at ~/.opencli/token. + * The daemon requires this token on all HTTP and WebSocket connections. + * The CLI and extension read the file to authenticate. + */ + +import { randomBytes } from 'node:crypto'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as os from 'node:os'; + +const TOKEN_DIR = path.join(os.homedir(), '.opencli'); +const TOKEN_PATH = path.join(TOKEN_DIR, 'token'); +const TOKEN_LENGTH = 32; // 32 random bytes → 64-char hex string + +/** + * Get the current daemon token, creating one if it doesn't exist. + * Returns the hex-encoded token string. + */ +export function getOrCreateToken(): string { + // If token file exists and is non-empty, return it + try { + const existing = fs.readFileSync(TOKEN_PATH, 'utf-8').trim(); + if (existing.length >= 32) return existing; + } catch { + // File doesn't exist or can't be read — create a new one + } + + const token = randomBytes(TOKEN_LENGTH).toString('hex'); + + // Ensure directory exists + fs.mkdirSync(TOKEN_DIR, { recursive: true }); + + // Write token with restrictive permissions (owner-only read/write) + fs.writeFileSync(TOKEN_PATH, token, { mode: 0o600 }); + + return token; +} + +/** + * Read the existing token. Returns null if no token file exists. + * Used by clients that should not create a token (only the daemon creates it). + */ +export function readToken(): string | null { + try { + const token = fs.readFileSync(TOKEN_PATH, 'utf-8').trim(); + return token.length >= 32 ? token : null; + } catch { + return null; + } +} + +/** Header name used to pass the token */ +export const TOKEN_HEADER = 'x-opencli-token'; From ffbfbbdc4070797ea0b1a50f34c948b6916bf83f Mon Sep 17 00:00:00 2001 From: Saeed Al Mansouri Date: Tue, 24 Mar 2026 23:26:17 +0400 Subject: [PATCH 2/3] =?UTF-8?q?fix(daemon):=20harden=20token=20auth=20?= =?UTF-8?q?=E2=80=94=20timing-safe=20comparison,=20atomic=20creation,=20Wi?= =?UTF-8?q?ndows=20ACLs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review feedback on the token authentication implementation: - Use crypto.timingSafeEqual() instead of === for token comparison, preventing timing side-channel attacks on the shared secret - Use O_EXCL flag for atomic token file creation, preventing race conditions when multiple daemon processes start simultaneously - Add icacls enforcement on Windows where Unix mode 0o600 is ignored, restricting token file to current user only - Validate token format with regex (exactly 64 hex chars) to detect corrupted files instead of accepting any 32+ char string - Add rotateToken() for token invalidation if the secret leaks - Add verifyToken() export for constant-time comparison - Export authHeaders() from daemon-client and reuse in discover.ts to eliminate duplicate auth header construction - Improve error messages when token directory/file can't be created --- src/browser/daemon-client.ts | 2 +- src/browser/discover.ts | 10 ++-- src/daemon.ts | 6 +-- src/token.ts | 93 +++++++++++++++++++++++++++++++----- 4 files changed, 89 insertions(+), 22 deletions(-) diff --git a/src/browser/daemon-client.ts b/src/browser/daemon-client.ts index 3e126ca3..30b05c16 100644 --- a/src/browser/daemon-client.ts +++ b/src/browser/daemon-client.ts @@ -11,7 +11,7 @@ import type { BrowserSessionInfo } from '../types.js'; const DAEMON_PORT = parseInt(process.env.OPENCLI_DAEMON_PORT ?? String(DEFAULT_DAEMON_PORT), 10); const DAEMON_URL = `http://127.0.0.1:${DAEMON_PORT}`; -function authHeaders(): Record { +export function authHeaders(): Record { const headers: Record = { 'X-OpenCLI': '1' }; const token = readToken(); if (token) headers[TOKEN_HEADER] = token; diff --git a/src/browser/discover.ts b/src/browser/discover.ts index 78af94d7..19ab1002 100644 --- a/src/browser/discover.ts +++ b/src/browser/discover.ts @@ -6,8 +6,7 @@ */ import { DEFAULT_DAEMON_PORT } from '../constants.js'; -import { readToken, TOKEN_HEADER } from '../token.js'; -import { isDaemonRunning } from './daemon-client.js'; +import { isDaemonRunning, authHeaders } from './daemon-client.js'; export { isDaemonRunning }; @@ -20,10 +19,9 @@ export async function checkDaemonStatus(): Promise<{ }> { try { const port = parseInt(process.env.OPENCLI_DAEMON_PORT ?? String(DEFAULT_DAEMON_PORT), 10); - const headers: Record = { 'X-OpenCLI': '1' }; - const token = readToken(); - if (token) headers[TOKEN_HEADER] = token; - const res = await fetch(`http://127.0.0.1:${port}/status`, { headers }); + const res = await fetch(`http://127.0.0.1:${port}/status`, { + headers: authHeaders(), + }); const data = await res.json() as { ok: boolean; extensionConnected: boolean }; return { running: true, extensionConnected: data.extensionConnected }; } catch { diff --git a/src/daemon.ts b/src/daemon.ts index 38a1d9ad..e25229b7 100644 --- a/src/daemon.ts +++ b/src/daemon.ts @@ -23,7 +23,7 @@ import { createServer, type IncomingMessage, type ServerResponse } from 'node:http'; import { WebSocketServer, WebSocket, type RawData } from 'ws'; import { DEFAULT_DAEMON_PORT } from './constants.js'; -import { getOrCreateToken, TOKEN_HEADER } from './token.js'; +import { getOrCreateToken, verifyToken, TOKEN_HEADER } from './token.js'; const PORT = parseInt(process.env.OPENCLI_DAEMON_PORT ?? String(DEFAULT_DAEMON_PORT), 10); const DAEMON_TOKEN = getOrCreateToken(); @@ -117,7 +117,7 @@ async function handleRequest(req: IncomingMessage, res: ServerResponse): Promise // This blocks local processes that don't have filesystem access to the // token file, even if they know the port and X-OpenCLI header. const clientToken = req.headers[TOKEN_HEADER] as string | undefined; - if (clientToken !== DAEMON_TOKEN) { + if (!verifyToken(clientToken, DAEMON_TOKEN)) { jsonResponse(res, 401, { ok: false, error: 'Unauthorized: invalid or missing token' }); return; } @@ -211,7 +211,7 @@ const wss = new WebSocketServer({ const tokenFromQuery = url.searchParams.get('token'); const tokenFromProtocol = req.headers['sec-websocket-protocol'] as string | undefined; const clientToken = tokenFromQuery ?? tokenFromProtocol; - if (clientToken !== DAEMON_TOKEN) return false; + if (!verifyToken(clientToken, DAEMON_TOKEN)) return false; return true; }, diff --git a/src/token.ts b/src/token.ts index dded6acc..9d122855 100644 --- a/src/token.ts +++ b/src/token.ts @@ -6,7 +6,8 @@ * The CLI and extension read the file to authenticate. */ -import { randomBytes } from 'node:crypto'; +import { randomBytes, timingSafeEqual } from 'node:crypto'; +import { execSync } from 'node:child_process'; import * as fs from 'node:fs'; import * as path from 'node:path'; import * as os from 'node:os'; @@ -14,43 +15,111 @@ import * as os from 'node:os'; const TOKEN_DIR = path.join(os.homedir(), '.opencli'); const TOKEN_PATH = path.join(TOKEN_DIR, 'token'); const TOKEN_LENGTH = 32; // 32 random bytes → 64-char hex string +const TOKEN_REGEX = /^[0-9a-f]{64}$/; // exactly 64 hex chars + +/** + * Constant-time token comparison to prevent timing attacks. + * Returns false if either value is missing or they differ in length. + */ +export function verifyToken(clientToken: string | undefined | null, serverToken: string): boolean { + if (!clientToken) return false; + const a = Buffer.from(clientToken, 'utf-8'); + const b = Buffer.from(serverToken, 'utf-8'); + if (a.length !== b.length) return false; + return timingSafeEqual(a, b); +} + +/** + * Restrict file/directory to current user only on Windows. + * On Unix, mode 0o600/0o700 set during creation is sufficient. + */ +function restrictPermissions(filePath: string): void { + if (process.platform !== 'win32') return; + try { + execSync(`icacls "${filePath}" /inheritance:r /grant:r "%USERNAME%:F"`, { + stdio: 'ignore', + windowsHide: true, + }); + } catch { + console.error(`[token] Warning: could not restrict permissions on ${filePath}`); + } +} /** * Get the current daemon token, creating one if it doesn't exist. - * Returns the hex-encoded token string. + * Uses O_EXCL for atomic creation to prevent race conditions when + * multiple daemon processes start simultaneously. */ export function getOrCreateToken(): string { - // If token file exists and is non-empty, return it + // If token file exists and is valid, return it try { const existing = fs.readFileSync(TOKEN_PATH, 'utf-8').trim(); - if (existing.length >= 32) return existing; + if (TOKEN_REGEX.test(existing)) return existing; + // File exists but is corrupted — will be recreated below + console.error('[token] Token file corrupted, regenerating'); } catch { // File doesn't exist or can't be read — create a new one } const token = randomBytes(TOKEN_LENGTH).toString('hex'); - // Ensure directory exists - fs.mkdirSync(TOKEN_DIR, { recursive: true }); - - // Write token with restrictive permissions (owner-only read/write) - fs.writeFileSync(TOKEN_PATH, token, { mode: 0o600 }); + // Ensure directory exists with restrictive permissions + try { + fs.mkdirSync(TOKEN_DIR, { recursive: true, mode: 0o700 }); + restrictPermissions(TOKEN_DIR); + } catch (err) { + throw new Error( + `Cannot create token directory ${TOKEN_DIR}: ${(err as Error).message}. ` + + `Ensure the home directory is writable.`, + ); + } - return token; + try { + // O_CREAT | O_EXCL | O_WRONLY — fails atomically if file already exists + const fd = fs.openSync(TOKEN_PATH, fs.constants.O_CREAT | fs.constants.O_EXCL | fs.constants.O_WRONLY, 0o600); + fs.writeSync(fd, token); + fs.closeSync(fd); + restrictPermissions(TOKEN_PATH); + return token; + } catch (err: unknown) { + if ((err as NodeJS.ErrnoException).code === 'EEXIST') { + // Another process won the race — read their token + const existing = fs.readFileSync(TOKEN_PATH, 'utf-8').trim(); + if (TOKEN_REGEX.test(existing)) return existing; + } + throw new Error( + `Cannot write token file ${TOKEN_PATH}: ${(err as Error).message}. ` + + `Ensure ${TOKEN_DIR} is writable.`, + ); + } } /** - * Read the existing token. Returns null if no token file exists. + * Read the existing token. Returns null if no token file exists or is invalid. * Used by clients that should not create a token (only the daemon creates it). */ export function readToken(): string | null { try { const token = fs.readFileSync(TOKEN_PATH, 'utf-8').trim(); - return token.length >= 32 ? token : null; + return TOKEN_REGEX.test(token) ? token : null; } catch { return null; } } +/** + * Generate a new token, replacing the existing one. + * Running daemons must be restarted to pick up the new token. + */ +export function rotateToken(): string { + const token = randomBytes(TOKEN_LENGTH).toString('hex'); + fs.mkdirSync(TOKEN_DIR, { recursive: true, mode: 0o700 }); + const tmpPath = TOKEN_PATH + '.tmp'; + fs.writeFileSync(tmpPath, token, { mode: 0o600 }); + fs.renameSync(tmpPath, TOKEN_PATH); + restrictPermissions(TOKEN_PATH); + return token; +} + /** Header name used to pass the token */ export const TOKEN_HEADER = 'x-opencli-token'; From 85e78336a09a1f9aad5bdcb490b42b758d2fcdbd Mon Sep 17 00:00:00 2001 From: Saeed Al Mansouri Date: Wed, 25 Mar 2026 00:27:43 +0400 Subject: [PATCH 3/3] fix(token): handle corrupted token file by unlinking before O_EXCL create When the token file exists but contains invalid data (corrupted), the O_EXCL flag on the new file creation fails with EEXIST since the corrupt file is still on disk. Fix: unlink the corrupt file before attempting to create a new one. Found during UAT on Windows. --- src/token.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/token.ts b/src/token.ts index 9d122855..085802c0 100644 --- a/src/token.ts +++ b/src/token.ts @@ -55,8 +55,9 @@ export function getOrCreateToken(): string { try { const existing = fs.readFileSync(TOKEN_PATH, 'utf-8').trim(); if (TOKEN_REGEX.test(existing)) return existing; - // File exists but is corrupted — will be recreated below + // File exists but is corrupted — remove it so O_EXCL create succeeds console.error('[token] Token file corrupted, regenerating'); + try { fs.unlinkSync(TOKEN_PATH); } catch { /* already gone */ } } catch { // File doesn't exist or can't be read — create a new one }