Skip to content

[#988] Reverse-proxy dashboard token allowlist (v2.5.1 hotfix)#989

Merged
realproject7 merged 2 commits into
mainfrom
task/988-proxy-token-allowlist
Jul 6, 2026
Merged

[#988] Reverse-proxy dashboard token allowlist (v2.5.1 hotfix)#989
realproject7 merged 2 commits into
mainfrom
task/988-proxy-token-allowlist

Conversation

@realproject7

Copy link
Copy Markdown
Owner

Closes #988

Summary

P0 hotfix for v2.5.1 — a #968 regression locked the operator's nginx-proxied VPS dashboard out of every terminal.

The VPS dashboard is served by nginx (p7.quadwork.xyz127.0.0.1:8400, proxy_set_header Host $host) behind HTTP Basic Auth. #968's /api/session-token guard (isLocalTokenRequest) requires socket-IP and Host and Origin to all be loopback. Through the proxy the forwarded Host/Origin is the public domain, so the endpoint 403s → the dashboard can't fetch the session token → every terminal WS closes 1006.

Fix

Add an opt-in operator allowlist config.trusted_dashboard_hosts (e.g. ["p7.quadwork.xyz"]). New isTrustedProxyRequest(req) returns true only when:

  1. the socket is loopback (the request genuinely arrived via the on-box reverse proxy), and
  2. the forwarded Host and (when the browser sends one) Origin hostnames are both in the allowlist.

/api/session-token and the WS upgrade (isAllowedWsOrigin) now accept such requests. The allowlist config is read once at boot (like port/session_token).

  • Default (no allowlist configured): exactly [#S2] WS/PTY auth + Origin allowlist (tailnet keystroke-injection) #968 behavior, byte-for-byte unchangedisTrustedProxyRequest short-circuits to false when the set is empty.
  • Server-side only; the existing frontend (src/lib/sessionToken.ts) already fetches /api/session-token relative to the page origin and attaches the token to the WS, so once the endpoint returns 200 the proxied path works with no frontend change.

Security invariants (not weakened)

  • A foreign/un-allowlisted Host or Origin (DNS-rebind page, untrusted proxy, tailnet name) is never in the allowlist → still 403 on /api/session-token and on the WS upgrade. DNS-rebinding protection intact.
  • Both Host and Origin must be allowlisted — a partial spoof (only one trusted) is rejected.
  • The allowlist trusts the proxy to have already authenticated the user; the docs state the proxy must be authenticated (nginx Basic Auth) and that QuadWork must only receive proxied traffic on loopback. It is not itself an auth layer.

EPIC Alignment

Ref EPIC #967 (Phase 3 hardening). This restores the #968 (EPIC #967 Phase 1) WS/PTF auth surface for the operator's authenticated reverse-proxy deployment without relaxing its DNS-rebinding guarantees — the allowlist is a scoped, opt-in exception layered on top of the existing loopback gate, not a replacement for it.

Self-Verification

Automated — server/proxyDashboardAllowlist.test.js (new, 13 assertions, auto-discovered by run-tests.js) boots the REAL server as a subprocess on a throwaway port with a temp HOME whose config.json sets trusted_dashboard_hosts: ["p7.quadwork.xyz"], then drives the actual server.on("upgrade") handler + /api/session-token over the wire:

(a) GET /api/session-token via trusted proxy Host+Origin → 200
(a) trusted proxy receives the real session token
(a) GET /api/session-token via trusted Host:port, no Origin → 200
(c) GET /api/session-token from loopback (local dashboard) → 200, unchanged
(c) loopback dashboard gets the same token
(b) GET /api/session-token with a foreign Host (not allowlisted) → 403
(b) GET /api/session-token with a foreign Origin (not allowlisted) → 403
(d) trusted Host but foreign Origin → 403 (both must be allowlisted)
(d) foreign Host but trusted Origin → 403
(a) WS via trusted proxy Origin+Host + token → opens
(a) WS via trusted proxy but no token → 401
(b) WS with a foreign Origin (not allowlisted) → 403, not opened
(d) WS with trusted Host but foreign Origin → 403, not opened
13 passed
  • (a) trusted host in allowlist → token 200 + WS opens
  • (b) foreign host not in allowlist → token 403 + WS 403
  • (c) no allowlist → loopback-only unchanged ✅ — covered by the untouched server/wsPtyAuth.test.js (boots with an empty config; 19 passed, still green).

Full suite green: npm test61 passed, 0 failed, 2 skipped (the 2 skips are pre-existing Jest-style, out of #836 scope).

Reverse-proxy path — live-verified by booting the real server on a throwaway port (8477, temp HOME, allowlist configured) and curling the exact headers nginx forwards:

1) Host: p7.quadwork.xyz + Origin: https://p7.quadwork.xyz  → 200   (proxied dashboard fetches token)
2) Host: evil.example (not allowlisted)                     → 403   (DNS-rebind / untrusted still blocked)
3) Host: localhost:8477 (plain loopback)                    → 200   (local dashboard unaffected)
4) Host: p7.quadwork.xyz + Origin: https://evil.example     → 403   (partial spoof rejected)

docs/troubleshooting.md updated with the nginx Basic-Auth + trusted_dashboard_hosts setup (incl. the required Host/Origin/Upgrade proxy headers) and a prominent note that the proxy must be authenticated.

cc @re1 @re2 — please review on current @Head.

The #968 loopback-only gate on /api/session-token 403s a dashboard served
through an on-box authenticated reverse proxy (nginx forwards a public
Host/Origin), so the terminal WS never gets a token and closes 1006.

Add opt-in config.trusted_dashboard_hosts: when the socket is loopback AND
the forwarded Host/Origin are both in the allowlist, /api/session-token and
the WS upgrade accept the request. Empty/unset (default) → exactly #968's
loopback-only behavior, no change. A foreign/un-allowlisted Host or Origin
still gets 403 — DNS-rebinding protection intact.

Tests (server/proxyDashboardAllowlist.test.js) cover: trusted host → token
200 + WS opens; foreign host → token 403 + WS 403; loopback unchanged;
partial spoof (only Host or only Origin trusted) → 403. wsPtyAuth.test.js
(no-allowlist case) still passes. docs/troubleshooting.md documents the
authenticated-nginx + allowlist setup.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@project7-interns project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Epic Alignment: FAIL

#988 requires an untrusted/foreign Host or Origin to still get 403 on both /api/session-token and WS; the token endpoint does, but the WS origin check still accepts matched non-allowlisted Host+Origin.

Checked (evidence)

  • Structural gate: PR body contains filled ## EPIC Alignment and ## Self-Verification sections; no UI fidelity table required.
  • Ticket/epic context: gh api repos/realproject7/quadwork/issues/988 + issues/967 → contract is no DNS-rebinding weakening and no regression of working deployments.
  • Live diff reviewed: gh pr diff 989 at head ccffe682e3c4a2b7cd0b8adec4a1c83ea84c397c.
  • Riskiest part of this diff: server/index.js WS/PTY auth path; not acceptable because a non-allowlisted same-host WS still passes origin validation.
  • Kill-list: scanned all items — hit listed in Findings.
  • CI: gh pr checks 989test pass 54s.

Findings

  • [high] WS still accepts a foreign same-host origin outside trusted_dashboard_hosts
    • File: server/index.js:209
    • Why it fails: after isTrustedProxyRequest(req) returns false, the legacy u.host === req.headers.host fallback still allows Host: evil.example + Origin: https://evil.example through isAllowedWsOrigin; with a valid session token the upgrade proceeds past the 403 gate at server/index.js:1700. That contradicts #988's required invariant and the PR's security claim that unallowlisted Host/Origin still get WS 403. The new test covers foreign Origin with loopback Host at server/proxyDashboardAllowlist.test.js:158, but not the matched foreign Host+Origin case.
    • Do instead: make /ws/terminal accept only loopback origins or isTrustedProxyRequest(req) for this path, or otherwise gate the same-host fallback behind an explicit trusted config; add a regression test for Host: evil.example + Origin: https://evil.example + valid token expecting WS 403.

Decision

Requesting changes because the central security invariant for this hotfix is not actually enforced on the WS upgrade path. If retaining the same-host token path is intentional, escalate #988 to @Head because the ticket acceptance criteria and PR security section need to be changed before approval.

@realproject7

Copy link
Copy Markdown
Owner Author

APPROVE at ccffe682e3c4a2b7cd0b8adec4a1c83ea84c397c@re2

Core-risky #968 WS/PTY auth path reviewed with the security invariants front of mind. The change is opt-in and strictly additive: a foreign Host/Origin still 403s on both /api/session-token and the WS upgrade, and the default (no allowlist) is byte-for-byte the #968 loopback-only gate.

Checked (evidence)

  • No trust proxyreq.ip unspoofable. grep "trust proxy\|X-Forwarded" in server/index.js = none. So isTrustedProxyRequest's req.ip || req.socket.remoteAddress (index.js:161) is the real socket address; a remote attacker cannot forge loopback via X-Forwarded-For. Off-box direct hit with Host: p7.quadwork.xyz is rejected at if (!isLocalhost(ip)) return false; (index.js:162).
  • Opt-in, no default behavior change. if (TRUSTED_DASHBOARD_HOSTS.size === 0) return false; (index.js:160) short-circuits when unset. wsPtyAuth.test.js (empty config) still green: 19/19.
  • Both Host AND Origin must be allowlisted; partial spoof rejected. index.js:163-171 requires Host in set, and Origin (when sent) in set. Verified live: trusted Host + foreign Origin → 403; foreign Host + trusted Origin → 403.
  • Host-header parse resists bypass tricks. hostnameOfHostHeader via new URL(\http://${host}`)(index.js:133-136):p7.quadwork.xyz@evil.comevil.com(not allowlisted), suffixp7.quadwork.xyz.evil.com` ≠ allowlisted entry. No substring/suffix match.
  • WS still gated on origin + token. isAllowedWsOrigin adds isTrustedProxyRequest(req) only after the loopback-origin check (index.js:206-208); upgrade handler still enforces tokenMatches (index.js:1705). Verified: trusted proxy + no token → 401; trusted origin + token → opens.
  • Tests run at PR head (ccffe682) in a tree with deps: node server/proxyDashboardAllowlist.test.js → 13/13 passed; node server/wsPtyAuth.test.js → 19/19 passed.
  • Docs: docs/troubleshooting.md states the proxy must be authenticated and that the allowlist is not itself an auth layer — matches the security intent.

No blocking issues. Nice work isolating the exception behind a loopback-socket + dual-allowlist gate.

REQUEST CHANGES: isAllowedWsOrigin's #968 same-host fallback
(u.host === req.headers.host) fired even for a LOOPBACK socket carrying a
non-loopback, non-allowlisted Host+Origin — so a forged/foreign matched pair
(untrusted on-box proxy, DNS-rebinding page) with a valid token slipped
through, violating #988's "foreign Host/Origin → WS 403" invariant.

Gate the fallback on a genuine DIRECT (non-loopback) connection: a loopback
socket with a non-loopback Host means the request arrived via an on-box
reverse proxy, which must be explicitly allowlisted (isTrustedProxyRequest,
checked first). Preserves #968 local + direct tailnet/LAN paths and strengthens
DNS-rebinding protection at the WS layer.

Adds the requested regression test: matched foreign Host+Origin + valid token
→ WS 403 (not opened). Full suite still 61 passed, 0 failed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@realproject7

Copy link
Copy Markdown
Owner Author

APPROVE at 777d9857a1a0a4c8be6ddc0d3c8167f6a2234d2d@re2 (re-review; supersedes my APPROVE at ccffe68)

@re1's REQUEST CHANGES was correct and I'd missed it: the #968 same-host fallback (u.host === req.headers.host) fired for a loopback socket carrying a matched foreign Host+Origin (DNS-rebound page / untrusted on-box proxy), so a valid token would open the WS — violating #988's invariant. The fix closes it cleanly.

Checked (evidence)

  • Fix gates the fallback on a genuine direct connection. server/index.js:218-219 now return !isLocalhost(ip) && !!req.headers.host && u.host === req.headers.host;. A loopback socket can only reach a non-loopback origin via isTrustedProxyRequest (checked just above, index.js:210) — the explicit allowlist. Matches re1's finding exactly.
  • Hole closed. New regression proxyDashboardAllowlist.test.js: matched foreign Host: evil.example + Origin: http://evil.example + valid token → WS 403, not opened. Ran at head 777d985: 14/14 passed.
  • No regression to legit paths. Local dashboard origin still hits the early loopback-origin return (index.js:207); trusted proxy via isTrustedProxyRequest; direct tailnet/LAN (non-loopback socket, Origin host == Host) still accepted by the guarded fallback. wsPtyAuth.test.js 19/19 green (no default behavior change).
  • Token endpoint untouchedisLocalTokenRequest/isTrustedProxyRequest unchanged since ccffe68; the delta is WS-origin-only.

Prior-SHA note: my earlier APPROVE at ccffe68 is stale (post-approval push). This verdict pins 777d985. No blocking issues.

@project7-interns project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Epic Alignment: PASS

Delta ccffe682e3c4a2b7cd0b8adec4a1c83ea84c397c..777d9857a1a0a4c8be6ddc0d3c8167f6a2234d2d now enforces #988's Host/Origin 403 invariant while preserving the #967 WS auth contract.

Checked (evidence)

  • Re-review delta: gh api repos/realproject7/quadwork/compare/ccffe682e3c4a2b7cd0b8adec4a1c83ea84c397c...777d985 → changed only server/index.js and server/proxyDashboardAllowlist.test.js.
  • Prior finding resolved: loopback reverse-proxy/same-host fallback is now gated by !isLocalhost(ip) at server/index.js:218, after allowlisted proxy handling.
  • Regression coverage: matched foreign Host: evil.example + Origin: http://evil.example + valid token expects WS 403 at server/proxyDashboardAllowlist.test.js:170.
  • Riskiest part of this diff: preserving direct tailnet/LAN same-host WS access while rejecting loopback proxy spoofing; acceptable because the fallback is now non-loopback only.
  • Kill-list: scanned new ranges — clean.
  • CI: gh pr checks 989test pass 48s.

Findings

None.

Decision

The requested security fix is in place and covered by the exact regression test. Approving PR #989 at 777d9857a1a0a4c8be6ddc0d3c8167f6a2234d2d.

@realproject7 realproject7 merged commit 7bf9fc1 into main Jul 6, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[#968 regression] Reverse-proxy dashboard locked out of terminal WS — /api/session-token 403s the proxied Host

2 participants