Skip to content

fix(daemon): treat Sec-Fetch-Site as a hint, not an authorization signal - #7081

Open
mrzhangkris wants to merge 1 commit into
nexu-io:mainfrom
mrzhangkris:fix/7041-sec-fetch-site-not-authorization
Open

fix(daemon): treat Sec-Fetch-Site as a hint, not an authorization signal#7081
mrzhangkris wants to merge 1 commit into
nexu-io:mainfrom
mrzhangkris:fix/7041-sec-fetch-site-not-authorization

Conversation

@mrzhangkris

Copy link
Copy Markdown
Contributor

Fixes #7041

Why

I hit this while auditing the daemon's local origin guard. The pain: Sec-Fetch-Site is set by user agents and JavaScript cannot modify it, but any non-browser HTTP client (curl, scripts) can forge it trivially. The current code treats Sec-Fetch-Site: same-origin as sufficient authorization for a no-Origin request, widening the host check to the full OD_ALLOWED_ORIGINS allow-list on the non-loopback path — so a forged header reaches guarded endpoints (/api/app-config, MCP oauth/install routes, /api/dir-exists, …) that should require a real browser same-origin context or bearer auth.

What users will see

No UI change. Security behavior: a no-Origin request whose Host only matches a hostname entry in OD_ALLOWED_ORIGINS is now rejected even when it carries Sec-Fetch-Site: same-origin. Reverse-proxy deployments that rely on that combination must send an Origin header that matches the allow-list (or use bearer auth) — this is the fail-closed direction already confirmed by the maintainer in #7041.

Surface area

  • UI
  • Keyboard shortcut
  • CLI / env var
  • API / contract
  • Extension point
  • i18n keys
  • New top-level dependency
  • Default behavior change — no-Origin requests to non-loopback hosts listed in OD_ALLOWED_ORIGINS are now rejected unless they carry a matching Origin; Sec-Fetch-Site no longer substitutes.
  • None

Screenshots

N/A (security guard change, no UI surface).

Bug fix verification

  • Test path that reproduces the bug: apps/daemon/tests/origin-validation.test.tsisLocalSameOrigin: Sec-Fetch-Site is not an authorization signal (issue #7041) → "rejects a no-Origin request even when Host matches OD_ALLOWED_ORIGINS and Sec-Fetch-Site is same-origin (forged header cannot authorize)".
  • Did the test go red on main and green on this branch? Yes. On main the new assertion fails (expected true to be false — the forged header passes); on this branch the full file is green (61/61).
  • pnpm --filter @open-design/daemon typecheck passes.

Non-browser HTTP clients can forge Sec-Fetch-Site trivially, so a
no-Origin request carrying 'Sec-Fetch-Site: same-origin' must not widen
the host check to the full OD_ALLOWED_ORIGINS allow-list on the
non-loopback path. Only a loopback/private-LAN host (or an explicitly
configured IP-literal origin) may omit the Origin header; reverse-proxy
deployments with a hostname in OD_ALLOWED_ORIGINS must send Origin or
use bearer auth instead.

Fixes nexu-io#7041
@lefarcen

Copy link
Copy Markdown
Contributor

Thanks @mrzhangkris — this is a clear security-hardening direction, and the red→green bug repro in the body makes the intent easy to follow.

One small PR-body ask before review routing: could you add a short ## Validation section summarizing what you ran? The bug-fix verification is already helpful; this repo's template also looks for the explicit validation section.

@lefarcen
lefarcen requested a review from PerishCode August 18, 2026 13:39
@lefarcen lefarcen added size/S PR changes 20-100 lines risk/high High risk: apps/desktop, daemon, auth, migration, workflows, package deps type/bugfix Bug fix labels Aug 18, 2026
@lefarcen lefarcen added the skip-validation Maintainer override: bot will not auto-add needs-validation on this PR. label Aug 18, 2026
@lefarcen

Copy link
Copy Markdown
Contributor

Heads-up: #7082 from the same author also changes apps/daemon/src/origin-validation.ts and apps/daemon/tests/origin-validation.test.ts with the same #7041 hardening. If those daemon changes are meant to move together, worth consolidating so review only has to happen in one place.

@PerishCode PerishCode left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@mrzhangkris This cleanly removes the forgeable Sec-Fetch-Site authorization path while preserving the existing loopback/private-LAN and explicit-Origin behavior. I reviewed both changed ranges, the surrounding daemon authorization flow, the linked security issue, and the focused red-to-green regression coverage; the relevant daemon CI shard is passing. Nice focused hardening fix—thank you for making the trust boundary and deployment impact explicit.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk/high High risk: apps/desktop, daemon, auth, migration, workflows, package deps size/S PR changes 20-100 lines skip-validation Maintainer override: bot will not auto-add needs-validation on this PR. type/bugfix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Same-origin guard trusts forgeable Sec-Fetch-Site header (fail-open for non-browser clients)

3 participants