diff --git a/docs/specs/security-local.md b/docs/specs/security-local.md index 481608288..1fac8c03e 100644 --- a/docs/specs/security-local.md +++ b/docs/specs/security-local.md @@ -124,11 +124,13 @@ embedder, and the browser checks the whole chain (rationale). - **FAIL IF** a request bearing a *foreign* `Origin` refreshes a grant's idle timer: a grant holds a live upstream binding, and a stranger polling it keeps a closed pane's binding open. An *absent* `Origin` must keep refreshing it — that is what a live frame's own navigations and sub-resources send. - **FAIL IF** the stream relay's grant stops being single-use, TTL-bounded, and pinned to one target port, or if it begins rewriting `Origin` rather than dropping it. It needs no `Host` check while the token holds (rationale). - **FAIL IF** the browser-dev bridge drops any of its four gates — the per-run token, the loopback `Host` check, the `application/json` content-type required of every non-GET, and the exact-origin `access-control-allow-origin`. The first three live together in the gate that runs before routing, so a route that never reads a body is covered by all of them. It is dev-only and ships in nothing, but it dispatches `pty_spawn` with caller-supplied `shell`, `args`, `cwd` and `env` — arbitrary command execution on a maintainer or CI-agent machine (`docs/specs/security-ci.md` -> "Automated Maintainer (tend)"). The content-type rule is a security control, not tidiness (rationale). +- **FAIL IF** the browser-dev Vite server permits cross-origin reads of token-bearing modules or disables its DNS-rebinding Host check. Pinned by `standalone/scripts/dev-agent-browser.test.mjs` (rationale). **Cookie-authenticated iframe pages are unsupported.** Header stripping does not isolate `document.cookie`: proxied scripts still share the loopback hostname's non-HttpOnly cookies across grant ports. This remains a browser-pane isolation gap (rationale). Source of truth: the shared rule and predicates — `isLoopbackHost`, `isOwnOrigin`, -`isForeignOrigin` — in `lib/src/host/loopback-guard.ts`. +`isForeignOrigin` — in `lib/src/host/loopback-guard.ts`; +`startDevVite` in `standalone/scripts/dev-run.mjs`. ## Persisted state diff --git a/docs/specs/security-local.rationale.md b/docs/specs/security-local.rationale.md index 9fd66ea8f..4922fb22c 100644 --- a/docs/specs/security-local.rationale.md +++ b/docs/specs/security-local.rationale.md @@ -130,6 +130,12 @@ is `pty_spawn` with caller-supplied `shell`, `args`, `cwd` and `env`. Why proxy cookies are stripped in both directions. [RFC 6265 §8.5](https://www.rfc-editor.org/rfc/rfc6265#section-8.5) scopes cookies by host, not port. An inbound cookie can therefore belong to another local service, including an HttpOnly credential, rather than the fixed upstream. Forwarding it leaks that credential; forwarding an upstream Set-Cookie lets even a remote HTTP target overwrite loopback cookies. The WebSocket handshake is HTTP too, including a refused upgrade. Parsing that handshake before piping bytes closes the same boundary without filtering WebSocket payloads. +The Vite listener serves modules containing the browser-dev bridge token. Vite's +default CORS policy allows other localhost origins to read those modules +(measured with Vite 8.3.0, 2026-09). Disabling CORS closes that read; the Host +check separately blocks DNS rebinding, where the browser sees a same-origin +request and CORS does not apply. + What header stripping cannot protect. A proxied script runs on `127.0.0.1` and can still read or write non-HttpOnly cookies through `document.cookie`, subject to browser partitioning. The per-grant port isolates origins, not cookie storage. Full isolation needs a separate browser storage context or host namespace; cookie-backed login in the iframe renderer cannot be preserved safely by forwarding ambient cookies. ## Persisted state diff --git a/scripts/loopback-lint-selftest.mjs b/scripts/loopback-lint-selftest.mjs index f6f1ffa37..b5bf21061 100644 --- a/scripts/loopback-lint-selftest.mjs +++ b/scripts/loopback-lint-selftest.mjs @@ -62,6 +62,14 @@ const FIXTURES = [ ['ws, explicit loopback host', "\nexport function __selftest() { return new WebSocket.Server({ host: '127.0.0.1' }); }\n"], ['ws, port only', '\nexport function __selftest() { return new WebSocketServer({ port: 9999 }); }\n'], ['ws, port only', '\nexport function __selftest() { return new WebSocket.Server({ port: 9999 }); }\n'], + ['vite, server.host', "\nexport const __selftest = { server: { host: '127.0.0.1', strictPort: true } };\n"], + // A nested `server` key above `host` — the shape a real `vite.config.ts` has + // and the one a first-brace-terminated scan misses. + ['vite, server.host', "\nexport const __selftest = { server: { fs: { allow: ['.'] }, host: '127.0.0.1' } };\n"], + // `proxy` nests a target object per route — two levels, the deepest the form + // reaches. A route option that nests again (`headers`, `configure`) is past + // the ceiling `scripts/loopback-lint.mjs` states. + ['vite, server.host', "\nexport const __selftest = { server: { proxy: { '/api': { target: 'http://up' } }, host: '127.0.0.1' } };\n"], ]; const selftest = makeSelftest('loopback-lint.mjs', '.loopback-selftest.bak'); diff --git a/scripts/loopback-lint.mjs b/scripts/loopback-lint.mjs index 85b048dfd..a5d0ef9e8 100644 --- a/scripts/loopback-lint.mjs +++ b/scripts/loopback-lint.mjs @@ -29,6 +29,12 @@ * - It knows the bind forms listed at BIND_FORMS and no others. A library * nobody has added yet spells its bind some way this file has never seen, * so adding a server dependency means adding its spelling here. + * - The `server`-block form scans past nested objects two levels deep, which + * reaches `fs`, `hmr`, `headers`, `watch` and a `proxy` route object. It + * stops there because no depth is the last one: a `proxy` route's own + * options nest again (`headers`, `cookieDomainRewrite`), and `configure` + * takes a function whose body carries braces of its own. A `host` written + * below one of those is a miss, and the audit is what covers it. * - Outside `ws`, it matches only an explicit loopback host. A listener that * binds every interface (`.listen(port)` with no host) is a different and * larger problem, and `relay/` does it deliberately from config, so @@ -69,6 +75,15 @@ const ROOT = fileURLToPath(new URL('..', import.meta.url)); * review; forgetting the guard entirely does not. */ const ALLOWED = { + 'standalone/scripts/dev-run.mjs': + 'The Vite dev server owns its own request path, so neither guard module can ' + + 'run on it. What stands in for them is pinned at the bind: cors: false, ' + + 'because the modules Vite serves carry the browser-dev bridge token and ' + + 'Vite\'s default admits every http://localhost:* origin to read them; and ' + + 'allowedHosts: [], the Host check that makes DNS rebinding fail. Dev-only ' + + 'and unbundled — it ships in nothing. Both controls are checked by ' + + 'standalone/scripts/dev-agent-browser.test.mjs; see ' + + 'standalone/scripts/dev-host-guard.mjs for the bridge beside it.', 'vscode-ext/src/agent-browser-host.ts': 'The stream relay authenticates with a single-use 64-hex token (60s TTL, ' + 'pinned to one target port) and drops Origin rather than rewriting it, so ' @@ -97,6 +112,14 @@ const LOOPBACK = "['\"](?:127\\.0\\.0\\.1|localhost)['\"]"; // branch that can rot alone, which is how `WebSocket\.Relay` sat here matching // nothing while the `WebSocketServer` branch beside it kept the lint green. const WS_NEW = '\\bnew\\s+WebSocket\\.?Server\\(\\s*\\{[^}]*?'; +// Keys of one options object, allowing nested objects up to two levels deep +// between the opening brace and the key being looked for. `[^}]*?` stops at the +// first `}`, so a form that uses it only matches while its key precedes every +// nested object — fine for a call's flat options, wrong for a `vite.config.ts` +// `server` block, where a nested key above `host` is the common shape. Two +// levels is where this stops, not where `server` keys stop — the header states +// what that leaves out. +const NESTED_KEYS = '(?:[^{}]|\\{(?:[^{}]|\\{[^{}]*\\})*\\})*?'; /** * Every bind form `LISTEN_RE` looks for, one entry per alternative — the @@ -111,6 +134,16 @@ const BIND_FORMS = [ { label: '@hono/node-server', re: `\\bserve\\(\\s*\\{[^}]*?hostname\\s*:\\s*${LOOPBACK}` }, { label: 'ws, explicit loopback host', re: `${WS_NEW}host\\s*:\\s*${LOOPBACK}` }, { label: 'ws, port only', re: `${WS_NEW}port\\s*:` }, + // Vite binds from config rather than from a call argument: `createServer({ + // server: { host } })` then an argument-less `listen()`, so neither `.listen` + // form can see it. Matched on the `server` block rather than on `createServer` + // because the same block is what a `vite.config.ts` — or Vitest, or + // Storybook's builder — passes to the same server. `NESTED_KEYS`, not + // `[^}]*?`, because `fs`, `hmr`, `headers` and `watch` (one level) and a + // `proxy` route object (two) are ordinary `server` keys, and any of them + // written above `host` would otherwise end the scan. A route's own nested + // options go deeper than the scan does — see this file's header. + { label: 'vite, server.host', re: `\\bserver\\s*:\\s*\\{${NESTED_KEYS}host\\s*:\\s*${LOOPBACK}` }, ]; const LISTEN_RE = new RegExp(BIND_FORMS.map((form) => form.re).join('|'), 'gs'); diff --git a/standalone/scripts/dev-agent-browser.test.mjs b/standalone/scripts/dev-agent-browser.test.mjs index c305dc0b6..f64074e9d 100644 --- a/standalone/scripts/dev-agent-browser.test.mjs +++ b/standalone/scripts/dev-agent-browser.test.mjs @@ -4,6 +4,7 @@ import { copyFile, mkdir, readFile, rm, writeFile } from 'node:fs/promises'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { spawn } from 'node:child_process'; +import { get } from 'node:http'; import { setTimeout as delay } from 'node:timers/promises'; import { sessionForKey } from 'dor-lib-common/agent-browser'; import { cleanEnv, devWorkspace, runner, writeShims } from './dev-fixture.mjs'; @@ -104,6 +105,23 @@ test('parallel worktrees own ports, browser identities and bridges; stopping one for (const [run, dir, other] of [[one, a.root, two], [two, b.root, one]]) { const js = await (await fetch(`${run.app}/app.js`)).text(); assert.ok(js.includes(`${run.bridge}/?t=${run.token}`)); + // `cors: false` in dev-run.mjs, pinned here because nothing else would + // notice its removal: these modules carry the bridge token, and Vite's + // default answers every http://localhost:* origin with an acao of its own, + // which is a read of the token by any other page in the developer's browser. + const foreign = await fetch(`${run.app}/app.js`, { headers: { origin: 'http://localhost:31337' } }); + assert.equal(foreign.status, 200); + assert.equal(foreign.headers.get('access-control-allow-origin'), null); + // DNS rebinding looks same-origin to a browser; the Host check must refuse it. + const reboundStatus = await new Promise((resolve, reject) => { + get(`${run.app}/app.js`, { + headers: { host: 'evil.example' }, signal: AbortSignal.timeout(5000), + }, response => { + response.resume(); + resolve(response.statusCode); + }).on('error', reject); + }); + assert.equal(reboundStatus, 403); // HMR must share this listener, even with a Tauri-specific host inherited. await new Promise((resolve, reject) => { const ws = new WebSocket(run.app.replace('http:', 'ws:'), 'vite-ping'); diff --git a/standalone/scripts/dev-run.mjs b/standalone/scripts/dev-run.mjs index 29f9bf7e3..39d7de1a9 100644 --- a/standalone/scripts/dev-run.mjs +++ b/standalone/scripts/dev-run.mjs @@ -31,6 +31,19 @@ export async function startDevVite(define) { host: '127.0.0.1', port: Number(process.env.DORMOUSE_BROWSER_DEV_VITE_PORT || 0), strictPort: true, + // Vite owns this listener's request path, so the bridge's guard module + // cannot run on it; these two stand in for it, pinned here rather than + // inherited. `cors: false` because Vite's default admits every + // `http://localhost:*` origin, and the modules served here carry the + // browser-dev bridge token (`VITE_DORMOUSE_BROWSER_DEV_HOST`, baked in by + // `dev-agent-browser.mjs`) — a page in the developer's own browser must + // not be able to read it. Nothing reads this server cross-origin: the app + // page is served from it, and the bridge is a separate origin sending its + // own headers. `allowedHosts: []` is Vite's own default, restated so a + // widening is a visible diff — it is the anti-DNS-rebind Host check the + // bridge makes for itself. + cors: false, + allowedHosts: [], // Share Vite's listener, including when TAURI_DEV_HOST is inherited. hmr: { host: 'localhost', port: 0, protocol: 'ws' }, },