Teach the loopback lint Vite's bind form and pin the dev server's origins - #671
Conversation
…gins
`standalone/scripts/dev-run.mjs` binds Vite to 127.0.0.1 through
`createServer({ server: { host } })` and an argument-less `listen()`, a
spelling no BIND_FORMS alternative matched, so the one listener that serves
the browser-dev bridge token appeared in neither the lint's inventory nor its
allowlist.
Add the form with a self-test fixture, and pin what stands in for a guard
Vite's request path cannot run: `cors: false`, because Vite's default answers
every `http://localhost:*` origin with a matching ACAO on modules carrying
`VITE_DORMOUSE_BROWSER_DEV_HOST`, and `allowedHosts: []` restated so a
widening of the anti-rebind Host check is a visible diff.
Refs #598
Deploying mouseterm with
|
| Latest commit: |
3c90b9f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://150aabbd.mouseterm.pages.dev |
| Branch Preview URL: | https://fix-loopback-lint-vite-bind.mouseterm.pages.dev |
dormouse-bot
left a comment
dormouse-bot
left a comment
There was a problem hiding this comment.
Feedback on work in progress — this is a draft, so it's a COMMENT, not a merge verdict. Mark it ready when you want the full review.
Two things, one of them load-bearing.
Nothing pins cors: false. The ALLOWED exemption says the two config keys "stand in for" the guard modules, but the lint only checks that the file matches a bind form and appears in ALLOWED — deleting cors: false from dev-run.mjs leaves pnpm test fully green, with the allowlist reason still describing a control that is gone. Every other exemption in this repo carries a mechanical check: the sibling lints' self-tests exist because, per AGENTS.md, "A rule added to one of these lints without its self-test case is not enforced — it is a claim that something is checked." The same argument applies to a control an allowlist entry claims. The cheap place to pin it is a test that already exists: in standalone/scripts/dev-agent-browser.test.mjs, the loop body that fetches /app.js from the run's app origin and asserts the bridge token is baked into it already has a live Vite server and is fetching the exact resource cors: false protects. One more request to that URL with a foreign Origin header, asserting no access-control-allow-origin comes back, turns the PR's measured table into a check. As it stands the probe is in the PR description and nowhere in CI.
The new bind form stops at the first closing brace. [^}]*? cannot cross a }, so the alternative matches a server block only while host precedes every nested object in it. Measured against the three shapes:
server block |
current | with the suggested change |
|---|---|---|
{ host: '127.0.0.1', port: 0 } |
match | match |
{ fs: { allow: ['.'] }, host: '127.0.0.1' } |
no match | match |
{ hmr: { protocol: 'ws', port: 0 }, host: '127.0.0.1' } |
no match | match |
fs, hmr, proxy, headers and watch are all ordinary server keys, so rows 2 and 3 are the common shape of a real vite.config.ts — and dev-run.mjs itself now holds hmr: { host: 'localhost', port: 0, protocol: 'ws' } in that block, one key reorder away from row 3. For dev-run.mjs the stale-allowlist check turns that into a red build, so the tracked file is safe; a new Vite config with a nested key above host is the silent case, which is the same failure mode that put this PR here. The new selftest fixture puts host first, so nothing exercises the shape that misses.
The inline suggestion permits one level of nesting. I ran both patterns over git ls-files: identical match sets today (dev-run.mjs and the selftest fixture, nothing else), including no new hit on standalone/vite.config.ts's server: { host: host || false, … } or website/vite.config.ts's server: { host: true }. Worth a second selftest fixture in the brace-before-host shape if you take it — the coverage check is keyed on labels, so a second fixture under the same label rides along without complaint.
Separately, the PR description's open question about whether the CORS pin deserves its own FAIL IF in docs/specs/security-local.md -> "Loopback Listeners" reads the right way to me as written — but the test above is worth having regardless of how that lands, since a FAIL IF is audited probabilistically and the test is not.
…th a test
Two gaps in the review of this PR's own change.
The `vite, server.host` form used `[^}]*?`, which cannot cross a `}`, so it
matched a `server` block only while `host` preceded every nested object in it.
`fs`, `hmr`, `proxy`, `headers` and `watch` are ordinary `server` keys, so the
shape it missed is the common shape of a real `vite.config.ts` — and `dev-run.mjs`
carries `hmr: { … }` in that block, one key reorder away from it. `ONE_NESTED`
permits one level of nesting; the new selftest fixture is the brace-before-`host`
shape, and narrowing the form back to `[^}]*?` makes the selftest go red.
Nothing pinned `cors: false`. The ALLOWED entry named it as a control standing in
for the guard modules, but deleting the line left `pnpm test` green with the
stated reason describing a control that was gone. `dev-agent-browser.test.mjs`
already fetches `/app.js` from the run's live Vite server to assert the bridge
token is baked into it; one more request to that URL with a foreign `Origin`
asserts no `access-control-allow-origin` comes back. Vite 8.3.0's default is
`cors: { origin: /^https?:\/\/(?:(?:[^:]+\.)?localhost|127\.0\.0\.1|\[::1\])(?::\d+)?$/ }`,
so without the pin that assertion sees `http://localhost:31337` echoed back.
dormouse-bot
left a comment
dormouse-bot
left a comment
There was a problem hiding this comment.
The ready-for-review head closed both draft findings. I checked the cors: false
pin isn't vacuous: Node's fetch forwards a caller-set origin rather than
dropping it the way a browser would, so that assertion reads Vite's response and
not an absent request header. http.get forwards an overridden host the same
way, so the new rebind assertion is real too.
One gap left. The comment on the new form names proxy among the server keys
ONE_NESTED was widened to clear, but proxy's idiomatic form nests a target
object per route — two levels — and the pattern reaches one:
server block |
ONE_NESTED |
|---|---|
{ fs: { allow: ['.'] }, host: '127.0.0.1' } |
match |
{ proxy: { '/api': 'http://up' }, host: '127.0.0.1' } |
match |
{ proxy: { '/api': { target: 'http://up' } }, host: '127.0.0.1' } |
no match |
A vite.config.ts that proxies an upstream and binds loopback is exactly the
silent miss this PR exists to close, and the comment tells the next editor that
key is covered. Allowing a second level takes row 3, still stops at the block's
own closing brace (three levels stays a miss), and leaves the match set over
git ls-files unchanged — dev-run.mjs and the selftest fixtures, 29ms across
941 files. The suggestions below carry it; I'm pushing them plus a proxy
fixture under the same label, since there is no separate author here to apply
them.
`proxy` is the one ordinary `server` key whose idiomatic form nests two objects deep — a target object per route — so a `vite.config.ts` that proxied an upstream and bound loopback slipped past the one-level scan while the comment named `proxy` as covered. Allowing a second level takes that shape, still stops at the block's own closing brace, and leaves the match set over `git ls-files` unchanged. A third fixture under the same label pins it: narrowing back to one level makes the selftest go red.
dormouse-bot
left a comment
dormouse-bot
left a comment
There was a problem hiding this comment.
proxy is reachable now and the new fixture pins it, but the comments that
arrived with it state a depth bound Vite's own API exceeds.
NESTED_KEYS says "Two levels is as deep as a server key goes", and the
fixture calls the two-level proxy shape "the deepest shape this form reaches".
A proxy route object nests again: headers and cookieDomainRewrite are
plain objects, and configure takes a function whose body carries braces of its
own. Measured against the pattern as pushed:
server block |
matches |
|---|---|
{ proxy: { '/api': { target: 'http://up' } }, host: '127.0.0.1' } |
yes |
{ proxy: { '/api': { target: 'http://up', headers: { 'X-Foo': 'bar' } } }, host: '127.0.0.1' } |
no |
{ proxy: { '/api': { target: 'http://up', configure: (p, o) => { … } } }, host: '127.0.0.1' } |
no |
configure is why a third level is not the answer: a function body carries
unbounded braces, so no depth a regex can spell is the last one, and another
round of widening buys one shape while leaving the claim just as wrong. The
defect is the claim, not the depth — a next editor reads "as deep as a server
key goes" and takes proxy-with-headers for covered, which is the same silent
miss this PR exists to close.
Pushed the correction: the depth stays at two, and the bound moves into the
header's "What it deliberately does NOT do" list, beside the runtime-host
ceiling already named there as the audit's job. Comments only — no pattern
change, so the match set and the 12 load-bearing self-checks are unchanged.
…denying it `NESTED_KEYS` claimed two levels was as deep as a `server` key goes. A `proxy` route object nests again — `headers` and `cookieDomainRewrite` are plain objects, and `configure` takes a function whose body carries braces — so proxy-with-headers above `host` is a miss the comment called covered. No depth a regex can spell is the last one, so the depth stays at two and the bound moves into the header's "What it deliberately does NOT do" list, beside the runtime-host ceiling already named there as the audit's job. Comments only: the match set and the 12 load-bearing self-checks are unchanged.
2 participants
The loopback lint missed Vite's
server.hostbind form, leaving the browser-dev Vite listener outside its inventory. Vite's default CORS policy also allowed other localhost origins to read modules containing the browser-dev bridge token.This change recognizes explicit loopback
server.hostvalues, including when options objects up to two levels deep precedehost(such as proxy target objects), and adds matching self-test fixtures. The Vite listener is allowlisted with its actual controls:cors: falseprevents cross-origin module reads, andallowedHosts: []retains Vite's DNS-rebinding Host check.The existing integration test now verifies that a foreign localhost Origin receives no CORS permission and a hostile Host receives 403.
security-local.mdrecords this boundary, with its evidence in the paired rationale.Validation:
pnpm lint:specspassed.pnpm lint:loopbackpassed, including 12 load-bearing self-checks.cors: falseor changingallowedHoststotruefails the integration test at the expected assertion.The token leak did not by itself bypass the bridge's separate JSON-content-type and exact-origin CORS gates. This change closes the read and gives it regression coverage.
Refs #598.