From c6cc36c810fd912af8130629668a4896d4201690 Mon Sep 17 00:00:00 2001 From: Nicholas Charriere Date: Wed, 29 Jul 2026 08:17:47 -0700 Subject: [PATCH 1/2] fix(security): reject non-local origins and bind loopback by default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Srcbook has no authentication and can execute arbitrary code, so the only thing separating the user's own tab from any other page they have open is the request origin. Nothing was checking it. Verified before this change, against a running instance: any web page could read every provider API key from GET /api/settings, read arbitrary files from disk via POST /api/file, and — because the websocket accepted any origin — open a socket and drive cell:update + cell:exec to run code as the user. The server also bound 0.0.0.0, so all of it was reachable from the whole network. Changes: - New server/security.mts owns the origin allowlist. Local origins on any port are allowed, plus anything in SRCBOOK_ALLOWED_ORIGINS. Requests with no Origin header are allowed, since browsers always set it on cross-origin requests and non-browser clients (the CLI, curl) legitimately omit it. - HTTP: the 41 per-route cors() calls become one cors + verifyOrigin pair on the router. Disallowed origins get a 403 rather than just a missing CORS header — CORS alone stops an attacker reading a response, but the request still executes, which is all you need for POST /api/settings. - WebSocket: origin checked via verifyClient, so rejection happens at the handshake. Checking in the connection handler is too late; the client observes an open socket that is only torn down afterwards. - Bind 127.0.0.1 unless HOST says otherwise, and warn when it doesn't. This makes docker-compose's HOST variable real — it was previously read by nothing. Compose now sets HOST=0.0.0.0 because Docker forwards published ports to the container IP; host-side exposure stays controlled by HOST_BIND, which already defaulted to loopback. - Guard process.send in the CLI server. It only exists under IPC, so running the server directly crashed it right after printing "running at ..." — which is how this was found while testing the bind change. 12 tests for the allowlist, covering the substring cases that make this kind of check go wrong (localhost.evil.com, example.com.evil.com). --- docker-compose.yml | 6 +- packages/api/dev-server.mts | 8 +- packages/api/index.mts | 3 +- packages/api/server/http.mts | 74 ++++++--------- packages/api/server/security.mts | 134 ++++++++++++++++++++++++++++ packages/api/server/ws-client.mts | 11 +++ packages/api/test/security.test.mts | 97 ++++++++++++++++++++ srcbook/src/server.mts | 24 +++-- turbo.json | 2 + 9 files changed, 302 insertions(+), 57 deletions(-) create mode 100644 packages/api/server/security.mts create mode 100644 packages/api/test/security.test.mts diff --git a/docker-compose.yml b/docker-compose.yml index 62d089910..15b741bf3 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -14,6 +14,10 @@ services: create_host_path: true environment: - NODE_ENV=production - - HOST=${HOST_BIND:-127.0.0.1} + # Bind every interface *inside* the container — Docker forwards published ports + # to the container's IP, so binding loopback here would make the port mapping + # unreachable. Exposure is controlled by HOST_BIND on the ports line above, + # which defaults to loopback on the host. + - HOST=0.0.0.0 - SRCBOOK_INSTALL_DEPS=true command: ['pnpm', 'start'] diff --git a/packages/api/dev-server.mts b/packages/api/dev-server.mts index d95c55c13..50118bc81 100644 --- a/packages/api/dev-server.mts +++ b/packages/api/dev-server.mts @@ -3,17 +3,19 @@ import { WebSocketServer as WsWebSocketServer } from 'ws'; import app from './server/http.mjs'; import webSocketServer from './server/ws.mjs'; +import { bindHost, webSocketServerOptions } from './server/security.mjs'; export { SRCBOOK_DIR } from './constants.mjs'; const server = http.createServer(app); -const wss = new WsWebSocketServer({ server }); +const wss = new WsWebSocketServer({ server, ...webSocketServerOptions }); wss.on('connection', webSocketServer.onConnection); const port = process.env.PORT || 2150; -server.listen(port, () => { - console.log(`Server is running at http://localhost:${port}`); +const host = bindHost(); +server.listen(Number(port), host, () => { + console.log(`Server is running at http://${host}:${port}`); }); process.on('SIGINT', async function () { diff --git a/packages/api/index.mts b/packages/api/index.mts index 44d25565e..7c3bfa8f1 100644 --- a/packages/api/index.mts +++ b/packages/api/index.mts @@ -2,5 +2,6 @@ import app from './server/http.mjs'; import wss from './server/ws.mjs'; import { SRCBOOKS_DIR } from './constants.mjs'; import { posthog } from './posthog-client.mjs'; +import { bindHost, webSocketServerOptions } from './server/security.mjs'; -export { app, wss, SRCBOOKS_DIR, posthog }; +export { app, wss, SRCBOOKS_DIR, posthog, bindHost, webSocketServerOptions }; diff --git a/packages/api/server/http.mts b/packages/api/server/http.mts index c77fd0b92..875896217 100644 --- a/packages/api/server/http.mts +++ b/packages/api/server/http.mts @@ -35,16 +35,19 @@ import { readdir } from '../fs-utils.mjs'; import { EXAMPLE_SRCBOOKS } from '../srcbook/examples.mjs'; import { pathToSrcbook } from '../srcbook/path.mjs'; import { isSrcmdPath } from '../srcmd/paths.mjs'; +import { corsOptions, verifyOrigin } from './security.mjs'; const app: Application = express(); const router = express.Router(); +// Order matters: cors answers preflight requests, verifyOrigin rejects everything +// from an origin we don't trust, and only then do we parse a body. +router.use(cors(corsOptions)); +router.use(verifyOrigin); router.use(express.json()); -router.options('/file', cors()); - -router.post('/file', cors(), async (req, res) => { +router.post('/file', async (req, res) => { const { file } = req.body as { file: string; }; @@ -69,14 +72,12 @@ router.post('/file', cors(), async (req, res) => { } }); -router.options('/examples', cors()); -router.get('/examples', cors(), (_, res) => { +router.get('/examples', (_, res) => { return res.json({ result: EXAMPLE_SRCBOOKS }); }); // Create a new srcbook -router.options('/srcbooks', cors()); -router.post('/srcbooks', cors(), async (req, res) => { +router.post('/srcbooks', async (req, res) => { const { name, language } = req.body; // TODO: Zod @@ -102,8 +103,7 @@ router.post('/srcbooks', cors(), async (req, res) => { } }); -router.options('/srcbooks/:id', cors()); -router.delete('/srcbooks/:id', cors(), async (req, res) => { +router.delete('/srcbooks/:id', async (req, res) => { const { id } = req.params; const srcbookDir = pathToSrcbook(id); removeSrcbook(srcbookDir); @@ -113,8 +113,7 @@ router.delete('/srcbooks/:id', cors(), async (req, res) => { }); // Import a srcbook from a .src.md file or srcmd text. -router.options('/import', cors()); -router.post('/import', cors(), async (req, res) => { +router.post('/import', async (req, res) => { const { path, text, url } = req.body; if (typeof path === 'string' && !isSrcmdPath(path)) { @@ -143,8 +142,7 @@ router.post('/import', cors(), async (req, res) => { }); // Generate a srcbook using AI from a simple string query -router.options('/generate', cors()); -router.post('/generate', cors(), async (req, res) => { +router.post('/generate', async (req, res) => { const { query } = req.body; try { @@ -160,8 +158,7 @@ router.post('/generate', cors(), async (req, res) => { }); // Generate a cell using AI from a query string -router.options('/sessions/:id/generate_cells', cors()); -router.post('/sessions/:id/generate_cells', cors(), async (req, res) => { +router.post('/sessions/:id/generate_cells', async (req, res) => { // @TODO: zod const { insertIdx, query } = req.body; @@ -179,8 +176,7 @@ router.post('/sessions/:id/generate_cells', cors(), async (req, res) => { }); // Test that the AI generation is working with the current configuration -router.options('/ai/healthcheck', cors()); -router.get('/ai/healthcheck', cors(), async (_req, res) => { +router.get('/ai/healthcheck', async (_req, res) => { try { const result = await healthcheck(); return res.json({ error: false, result }); @@ -192,8 +188,7 @@ router.get('/ai/healthcheck', cors(), async (_req, res) => { }); // Open an existing srcbook by passing a path to the srcbook's directory -router.options('/sessions', cors()); -router.post('/sessions', cors(), async (req, res) => { +router.post('/sessions', async (req, res) => { const { path } = req.body; posthog.capture({ event: 'user opened srcbook' }); @@ -213,14 +208,12 @@ router.post('/sessions', cors(), async (req, res) => { } }); -router.get('/sessions', cors(), async (_req, res) => { +router.get('/sessions', async (_req, res) => { const sessions = await listSessions(); return res.json({ error: false, result: Object.values(sessions).map(sessionToResponse) }); }); -router.options('/sessions/:id', cors()); - -router.get('/sessions/:id', cors(), async (req, res) => { +router.get('/sessions/:id', async (req, res) => { const { id } = req.params; try { @@ -243,8 +236,7 @@ router.get('/sessions/:id', cors(), async (req, res) => { } }); -router.options('/sessions/:id/export-text', cors()); -router.get('/sessions/:id/export-text', cors(), async (req, res) => { +router.get('/sessions/:id/export-text', async (req, res) => { const session = await findSession(req.params.id); posthog.capture({ event: 'user exported srcbook' }); @@ -261,29 +253,26 @@ router.get('/sessions/:id/export-text', cors(), async (req, res) => { } }); -router.options('/sessions/:id/secrets/:name', cors()); -router.put('/sessions/:id/secrets/:name', cors(), async (req, res) => { +router.put('/sessions/:id/secrets/:name', async (req, res) => { const { id, name } = req.params; await associateSecretWithSession(name, id); await updateSessionEnvTypeDeclarations(id); return res.status(204).end(); }); -router.delete('/sessions/:id/secrets/:name', cors(), async (req, res) => { +router.delete('/sessions/:id/secrets/:name', async (req, res) => { const { id, name } = req.params; await disassociateSecretWithSession(name, id); await updateSessionEnvTypeDeclarations(id); return res.status(204).end(); }); -router.options('/settings', cors()); - -router.get('/settings', cors(), async (_req, res) => { +router.get('/settings', async (_req, res) => { const config = await getConfig(); return res.json({ error: false, result: config }); }); -router.post('/settings', cors(), async (req, res) => { +router.post('/settings', async (req, res) => { try { const updated = await updateConfig(req.body); @@ -300,24 +289,20 @@ router.post('/settings', cors(), async (req, res) => { } }); -router.options('/secrets', cors()); - -router.get('/secrets', cors(), async (_req, res) => { +router.get('/secrets', async (_req, res) => { const secrets = await getSecrets(); return res.json({ result: secrets }); }); // Create a new secret -router.post('/secrets', cors(), async (req, res) => { +router.post('/secrets', async (req, res) => { const { name, value } = req.body; posthog.capture({ event: 'user created secret' }); const updated = await addSecret(name, value); return res.json({ result: updated }); }); -router.options('/secrets/:name', cors()); - -router.post('/secrets/:name', cors(), async (req, res) => { +router.post('/secrets/:name', async (req, res) => { const { name } = req.params; const { name: newName, value } = req.body; await removeSecret(name); @@ -325,14 +310,13 @@ router.post('/secrets/:name', cors(), async (req, res) => { return res.json({ result: updated }); }); -router.delete('/secrets/:name', cors(), async (req, res) => { +router.delete('/secrets/:name', async (req, res) => { const { name } = req.params; const updated = await removeSecret(name); return res.json({ result: updated }); }); -router.options('/feedback', cors()); -router.post('/feedback', cors(), async (req, res) => { +router.post('/feedback', async (req, res) => { const { feedback, email } = req.body; // Every time you modify the appscript here, you'll need to update the URL below // @TODO: once we have an env variable setup, we can use that here. @@ -361,8 +345,7 @@ type NpmSearchResult = { * Returns the name, version and description of the packages. * Consider debouncing calls to this API on the client side. */ -router.options('/npm/search', cors()); -router.get('/npm/search', cors(), async (req, res) => { +router.get('/npm/search', async (req, res) => { const { q, size } = req.query; const response = await fetch(`https://registry.npmjs.org/-/v1/search?text=${q}&size=${size}`); if (!response.ok) { @@ -375,8 +358,7 @@ router.get('/npm/search', cors(), async (req, res) => { return res.json({ result: results }); }); -router.options('/subscribe', cors()); -router.post('/subscribe', cors(), async (req, res) => { +router.post('/subscribe', async (req, res) => { const { email } = req.body; const hubResponse = await fetch('https://hub.srcbook.com/api/subscribe', { method: 'POST', diff --git a/packages/api/server/security.mts b/packages/api/server/security.mts new file mode 100644 index 000000000..152e4d873 --- /dev/null +++ b/packages/api/server/security.mts @@ -0,0 +1,134 @@ +import type { IncomingMessage } from 'node:http'; +import type { NextFunction, Request, Response } from 'express'; +import type { CorsOptions } from 'cors'; + +/** + * Srcbook runs on the user's machine and has no authentication. That makes the + * origin of a request the only thing distinguishing the user's own browser tab + * from any other web page they happen to have open, because every page on the + * internet can reach http://localhost. + * + * So: requests from a browser must come from a local origin. Requests with no + * Origin header at all (curl, the CLI's own fetch, same-origin navigations) are + * allowed — browsers always set Origin on cross-origin requests, so an absent + * Origin cannot be an attacker's page. + * + * Note that CORS headers alone are not enough here. They stop an attacker from + * *reading* a response, but the request still executes, which is all you need for + * something like POST /api/settings. Disallowed origins are therefore rejected + * outright rather than merely being denied a CORS header. + */ + +const LOCAL_HOSTNAMES = new Set(['localhost', '127.0.0.1', '[::1]', '::1']); + +/** + * Additional origins the user explicitly trusts, as a comma-separated list. + * Intended for setups that serve the frontend from somewhere other than + * localhost, e.g. a reverse proxy or a remote dev box. + */ +function configuredOrigins(): string[] { + return (process.env.SRCBOOK_ALLOWED_ORIGINS ?? '') + .split(',') + .map((origin) => origin.trim()) + .filter((origin) => origin.length > 0); +} + +export function isOriginAllowed(origin: string | undefined): boolean { + // No Origin header. Not a cross-origin browser request, so there is nothing to check. + if (origin === undefined || origin === '' || origin === 'null') { + return true; + } + + if (configuredOrigins().includes(origin)) { + return true; + } + + let url: URL; + try { + url = new URL(origin); + } catch { + return false; + } + + if (url.protocol !== 'http:' && url.protocol !== 'https:') { + return false; + } + + return LOCAL_HOSTNAMES.has(url.hostname); +} + +/** + * Options for the `cors` middleware. Mirrors `isOriginAllowed` so preflight + * responses agree with what `verifyOrigin` will actually permit. + */ +export const corsOptions: CorsOptions = { + origin(origin, callback) { + callback(null, isOriginAllowed(origin)); + }, +}; + +/** + * Rejects requests from origins we don't trust. + * + * This is the part that stops cross-site request forgery: an attacker's page can + * issue the request regardless of CORS, so the request has to fail, not just its + * response be unreadable. + */ +export function verifyOrigin(req: Request, res: Response, next: NextFunction) { + const origin = req.get('origin'); + + if (isOriginAllowed(origin)) { + next(); + return; + } + + console.warn(`Rejected ${req.method} ${req.path} from disallowed origin '${origin}'`); + + res.status(403).json({ + error: true, + result: 'Request origin is not allowed. Srcbook only accepts requests from local origins.', + }); +} + +/** + * Origin check for the websocket upgrade. + * + * Websockets are not subject to the same-origin policy, so without this any page + * can open a socket to the server and drive cell execution. + */ +export function verifyWebSocketOrigin(request: IncomingMessage): boolean { + const origin = request.headers.origin; + + if (isOriginAllowed(origin)) { + return true; + } + + console.warn(`Rejected websocket connection from disallowed origin '${origin}'`); + return false; +} + +/** + * Options for `new WebSocketServer(...)`. + * + * `verifyClient` runs before the handshake completes, so a rejected client never + * gets an open socket. Checking in the 'connection' handler instead would be too + * late — the handshake has already succeeded by then and the client observes a + * connection that is only afterwards torn down. + */ +export const webSocketServerOptions = { + verifyClient(info: { origin: string; secure: boolean; req: IncomingMessage }): boolean { + return verifyWebSocketOrigin(info.req); + }, +}; + +/** + * The address the server binds to. + * + * Defaults to loopback. Binding every interface exposes an unauthenticated server + * that can execute arbitrary code to the whole network, so that has to be a choice + * the user makes deliberately rather than the default. + */ +export function bindHost(): string { + const host = (process.env.HOST ?? '').trim(); + return host === '' ? '127.0.0.1' : host; +} diff --git a/packages/api/server/ws-client.mts b/packages/api/server/ws-client.mts index 27c33627b..812be3f06 100644 --- a/packages/api/server/ws-client.mts +++ b/packages/api/server/ws-client.mts @@ -2,6 +2,7 @@ import { IncomingMessage } from 'node:http'; import z from 'zod'; import { type RawData, WebSocket } from 'ws'; import { WebSocketMessageSchema } from '@srcbook/shared'; +import { verifyWebSocketOrigin } from './security.mjs'; type TopicPart = { dynamic: false; segment: string } | { dynamic: true; parameter: string }; @@ -145,6 +146,9 @@ type ConnectionType = { subscriptions: string[]; }; +// RFC 6455 close code for a message that violates policy. +const WS_POLICY_VIOLATION_CODE = 1008; + export default class WebSocketServer { private readonly channels: Channel[] = []; private connections: ConnectionType[] = []; @@ -154,6 +158,13 @@ export default class WebSocketServer { } onConnection(socket: WebSocket, request: IncomingMessage) { + // Websockets are exempt from the same-origin policy, so without this check any + // page the user visits could open a socket and execute code through it. + if (!verifyWebSocketOrigin(request)) { + socket.close(WS_POLICY_VIOLATION_CODE, 'Origin not allowed'); + return; + } + const url = new URL(request.url!, `ws://${request.headers.host}`); const match = url.pathname.match(/^\/websocket\/?$/); diff --git a/packages/api/test/security.test.mts b/packages/api/test/security.test.mts new file mode 100644 index 000000000..cd2318e0c --- /dev/null +++ b/packages/api/test/security.test.mts @@ -0,0 +1,97 @@ +import { isOriginAllowed, bindHost } from '../server/security.mjs'; + +describe('isOriginAllowed', () => { + const originalEnv = process.env.SRCBOOK_ALLOWED_ORIGINS; + + afterEach(() => { + if (originalEnv === undefined) { + delete process.env.SRCBOOK_ALLOWED_ORIGINS; + } else { + process.env.SRCBOOK_ALLOWED_ORIGINS = originalEnv; + } + }); + + it('allows requests with no Origin header', () => { + // Browsers always set Origin on cross-origin requests, so an absent Origin + // means a non-browser client: curl, the CLI's own fetch, a same-origin GET. + expect(isOriginAllowed(undefined)).toBe(true); + expect(isOriginAllowed('')).toBe(true); + }); + + it("allows the opaque 'null' origin", () => { + // Sent for sandboxed iframes and file:// pages. Not attacker-controllable in + // a way that matters here, and rejecting it breaks legitimate local usage. + expect(isOriginAllowed('null')).toBe(true); + }); + + it('allows local origins on any port', () => { + expect(isOriginAllowed('http://localhost:2150')).toBe(true); + expect(isOriginAllowed('http://localhost:5173')).toBe(true); + expect(isOriginAllowed('http://127.0.0.1:2150')).toBe(true); + expect(isOriginAllowed('https://localhost:2150')).toBe(true); + expect(isOriginAllowed('http://[::1]:2150')).toBe(true); + }); + + it('rejects remote origins', () => { + expect(isOriginAllowed('https://evil.example.com')).toBe(false); + expect(isOriginAllowed('http://evil.example.com:2150')).toBe(false); + expect(isOriginAllowed('https://srcbook.com')).toBe(false); + }); + + it('rejects hostnames that merely contain a local hostname', () => { + expect(isOriginAllowed('http://localhost.evil.com')).toBe(false); + expect(isOriginAllowed('http://notlocalhost')).toBe(false); + expect(isOriginAllowed('http://127.0.0.1.evil.com')).toBe(false); + }); + + it('rejects non-http schemes', () => { + expect(isOriginAllowed('file://localhost')).toBe(false); + expect(isOriginAllowed('chrome-extension://abcdef')).toBe(false); + }); + + it('rejects unparseable origins', () => { + expect(isOriginAllowed('not a url')).toBe(false); + expect(isOriginAllowed('://localhost')).toBe(false); + }); + + it('allows origins listed in SRCBOOK_ALLOWED_ORIGINS', () => { + process.env.SRCBOOK_ALLOWED_ORIGINS = + 'https://notebooks.example.com, https://other.example.com'; + expect(isOriginAllowed('https://notebooks.example.com')).toBe(true); + expect(isOriginAllowed('https://other.example.com')).toBe(true); + expect(isOriginAllowed('https://unlisted.example.com')).toBe(false); + }); + + it('matches configured origins exactly, not by prefix', () => { + process.env.SRCBOOK_ALLOWED_ORIGINS = 'https://example.com'; + expect(isOriginAllowed('https://example.com.evil.com')).toBe(false); + expect(isOriginAllowed('https://example.com')).toBe(true); + }); +}); + +describe('bindHost', () => { + const originalHost = process.env.HOST; + + afterEach(() => { + if (originalHost === undefined) { + delete process.env.HOST; + } else { + process.env.HOST = originalHost; + } + }); + + it('defaults to loopback', () => { + delete process.env.HOST; + expect(bindHost()).toBe('127.0.0.1'); + }); + + it('treats an empty HOST as unset', () => { + process.env.HOST = ' '; + expect(bindHost()).toBe('127.0.0.1'); + }); + + it('honours an explicit HOST', () => { + process.env.HOST = '0.0.0.0'; + expect(bindHost()).toBe('0.0.0.0'); + }); +}); diff --git a/srcbook/src/server.mts b/srcbook/src/server.mts index 9a7ebc264..6d7a1005b 100644 --- a/srcbook/src/server.mts +++ b/srcbook/src/server.mts @@ -12,7 +12,7 @@ import http from 'node:http'; import express from 'express'; // @ts-ignore import { WebSocketServer as WsWebSocketServer } from 'ws'; -import { wss, app, posthog } from '@srcbook/api'; +import { wss, app, posthog, bindHost, webSocketServerOptions } from '@srcbook/api'; import chalk from 'chalk'; import { pathTo, getPackageJson } from './utils.mjs'; @@ -38,7 +38,7 @@ const server = http.createServer(app); // Create the WebSocket server console.log(chalk.dim('Creating WebSocket server...')); -const webSocketServer = new WsWebSocketServer({ server }); +const webSocketServer = new WsWebSocketServer({ server, ...webSocketServerOptions }); webSocketServer.on('connection', wss.onConnection); // Serve the react-app for all other routes, handled by client-side routing @@ -47,16 +47,28 @@ app.get('*', (_req, res) => res.sendFile(INDEX_HTML)); console.log(chalk.green('Initialization complete')); const port = Number(process.env.PORT ?? 2150); -const url = `http://localhost:${port}`; +const host = bindHost(); +const url = `http://${host === '0.0.0.0' || host === '::' ? 'localhost' : host}:${port}`; posthog.capture({ event: 'user started Srcbook application' }); const { name, version } = getPackageJson(); -server.listen(port, () => { +server.listen(port, host, () => { console.log(`${name}@${version} running at ${url}`); - // @ts-ignore - process.send('{"type":"init"}'); + + if (host !== '127.0.0.1' && host !== 'localhost' && host !== '::1') { + console.warn( + chalk.yellow( + `\nWarning: bound to ${host}, so Srcbook is reachable from other machines on this network.\n` + + `Srcbook has no authentication and can execute code, so only do this on a network you trust.\n`, + ), + ); + } + + // Only present when the CLI spawned us with an IPC channel. Running this file + // directly is legitimate, so don't blow up when it isn't there. + process.send?.('{"type":"init"}'); }); process.on('SIGINT', async () => { diff --git a/turbo.json b/turbo.json index 65eda3353..1ddf9d743 100644 --- a/turbo.json +++ b/turbo.json @@ -7,6 +7,8 @@ "VITE_SRCBOOK_API_HOST", "VITE_SRCBOOK_DEBUG_RENDER_SESSION_AS_READ_ONLY", "SRCBOOK_DISABLE_ANALYTICS", + "SRCBOOK_ALLOWED_ORIGINS", + "HOST", "PORT", "HOME" ], From 191312a0a186cd9cf9c8ed9bb9d32163c2d92a37 Mon Sep 17 00:00:00 2001 From: Nicholas Charriere Date: Wed, 29 Jul 2026 08:18:36 -0700 Subject: [PATCH 2/2] chore: add changeset and close task 0001 --- .changeset/quiet-moons-shave.md | 19 +++++++++++++++++++ tasks/0001-lock-down-network-surface.md | 23 ++++++++++++++++++++++- 2 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 .changeset/quiet-moons-shave.md diff --git a/.changeset/quiet-moons-shave.md b/.changeset/quiet-moons-shave.md new file mode 100644 index 000000000..7156aa9e4 --- /dev/null +++ b/.changeset/quiet-moons-shave.md @@ -0,0 +1,19 @@ +--- +'@srcbook/api': minor +'srcbook': minor +--- + +Reject requests and websocket connections from non-local origins, and bind to loopback by +default. + +Srcbook has no authentication and executes code, so any web page the user visited could +previously read their provider API keys, read files off disk, and run arbitrary code through +the websocket. The server also listened on every network interface. + +Requests now need a local `Origin` (any port), or one listed in the new +`SRCBOOK_ALLOWED_ORIGINS` environment variable. Requests without an `Origin` header — the CLI, +curl, same-origin navigations — are unaffected. + +To reach Srcbook from another machine, set `HOST` explicitly (for example `HOST=0.0.0.0`) and +add the origin you'll browse from to `SRCBOOK_ALLOWED_ORIGINS`. Only do this on a network you +trust. diff --git a/tasks/0001-lock-down-network-surface.md b/tasks/0001-lock-down-network-surface.md index 624a15277..4234986a1 100644 --- a/tasks/0001-lock-down-network-surface.md +++ b/tasks/0001-lock-down-network-surface.md @@ -1,7 +1,7 @@ --- id: 0001 title: Lock down the network surface (bind, CORS, websocket origin) -status: TODO +status: DONE created: 2026-07-29 area: api --- @@ -31,3 +31,24 @@ See `REVIEW.md` §1 for the reproduction. Keep the allowlist shared between the HTTP and websocket paths — one source of truth, or they will drift. + +Done in `server/security.mts`. Two things worth remembering: + +**CORS headers alone are not a fix.** They stop an attacker _reading_ a response, but the +request still executes — which is all you need for `POST /api/settings`. Disallowed origins +have to be rejected outright, so there's a `verifyOrigin` middleware alongside the `cors` +middleware rather than just `cors` configured with an allowlist. + +**Checking the origin in the websocket `connection` handler is too late.** `ws` has already +completed the handshake by then, so the client sees a socket open and only afterwards get +torn down. Verified this happening before switching to `verifyClient`, which runs before the +handshake — a rejected client now gets a 401 and never connects. + +Requests with no `Origin` header are allowed on purpose: browsers always set it on +cross-origin requests, so its absence means a non-browser client (the CLI, curl), and +rejecting those would break `srcbook import`. + +`HOST` was already set by `docker-compose.yml` and read by nothing. It's real now — but note +it has to be `0.0.0.0` _inside_ a container, because Docker forwards published ports to the +container IP. Host-side exposure is controlled by `HOST_BIND` on the ports line, which is the +knob that actually matters there.