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/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/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. 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" ],