From f063fc0c08cdd9188a46e7d250b2723278515a4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Thu, 13 Aug 2026 00:15:25 +0000 Subject: [PATCH] remove origin validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ALLOW_ANY_ORIGIN, ALLOW_ANY_ORIGIN_MUSIC, and everything they gated. The flag defaulted to ON, so none of it ran on a real install — what comes out is documented defence in depth that was already switched off. The file said so itself: "Both flags and their call sites come out once the tailnet is the perimeter." Origin was never authentication here in any case. An app's `officer://` origin is chosen by the client, forgeable outside a browser, and extractable from a shipped binary. Gone: the two flags, isOriginAllowed, isOriginCheckDisabled, isMusicOriginExempt, originValidationMiddleware, ORIGIN_RULES and the whole OFFICER__ORIGIN scheme, PUBLIC_URL's origin/host derivation, and origin-validation.test.ts, which existed only to pin them. CORS now echoes whatever Origin it is given, which is what every install already did. What SURVIVES is the reason this needed care. origin-validation.ts held two unrelated things, and the second was the global authorization gate — a valid non-owner token reaches only what its role grants, deliberately NOT under the flag because it is account-based rather than origin-based. Its own comment called it "the airtight half". Deleting the file wholesale would have deleted authorization. So it moves to _middlewares/capability-gate.ts as capabilityGateMiddleware, with the name matching what it does: nothing in it reads an Origin header any more. hono.ts mounts it in the same position, ahead of every router. origin-middleware.ts stays and is untouched — it extracts the Origin for six auth handlers that log it, and for passkeys. Extraction, not validation. Also updates every claim that rested on the old model: CLAUDE.md's security section and repo map, docs/secret-store.md, docs/mobile-api-keys.md, and five messages in machine-setup's Tailscale section which told the owner to set ALLOW_ANY_ORIGIN=false when declining a tailnet. That advice is now impossible to follow, and the honest version is different: with no tailnet the token is the whole lock, so put a proxy in front and restrict who can reach it. Not typechecked (empty node_modules, frozen installs). Every changed file parses; the setup section was run and writes four variables now. Co-Authored-By: Claude Opus 5 (1M context) --- .env.example | 5 - CLAUDE.md | 20 +- docs/mobile-api-keys.md | 3 +- docs/secret-store.md | 3 +- scripts/setup/machine-setup/lib/tailscale.sh | 36 ++- scripts/setup/machine-setup/machine-setup.sh | 4 +- scripts/setup/officer-setup.sh | 16 -- scripts/setup/officer-setup/lib/env.sh | 10 - src/servers/_middlewares/capability-gate.ts | 51 ++++ src/servers/_middlewares/index.ts | 2 +- .../_middlewares/origin-validation.test.ts | 37 --- src/servers/_middlewares/origin-validation.ts | 220 ------------------ src/servers/_middlewares/user-middleware.ts | 14 +- src/servers/api/auth/auth.ts | 6 +- src/servers/api/auth/signin.ts | 11 +- src/servers/api/users/capabilities-routes.ts | 2 +- src/servers/api/users/users-router.ts | 4 +- src/servers/api/vault/router.ts | 1 - src/servers/api/vault/websocket.ts | 14 +- src/servers/auth-token.ts | 2 +- src/servers/hono.ts | 24 +- src/workspaces/hooks/src/useCapabilities.ts | 2 +- 22 files changed, 117 insertions(+), 370 deletions(-) create mode 100644 src/servers/_middlewares/capability-gate.ts delete mode 100644 src/servers/_middlewares/origin-validation.test.ts delete mode 100644 src/servers/_middlewares/origin-validation.ts diff --git a/.env.example b/.env.example index 0eec5b6a..65cffff3 100644 --- a/.env.example +++ b/.env.example @@ -30,11 +30,6 @@ VAULT_STORE_KEY="" # only loads this file, so `bun dev` against a production .env runs fully hardened. # PUBLIC_BUILD_ENV=dev -# Origin checking is OFF unless this is explicitly "false" — an inversion of the usual rule, and one -# that is only defensible when the tailnet is the perimeter. On a machine with no tailnet, set it to -# false. Written explicitly rather than left to the default so the choice is visible. -ALLOW_ANY_ORIGIN=true - # DATA_PATH, OFFICER_ITEMS_DIR and HOME_DIR were here until 2026-08-12 and are no longer read. # The install root is derived as the parent of the working directory (src/servers/data-path.ts), so # data/, capabilities/ and dockers/ follow from it; the owner's home comes from the OS. Three values diff --git a/CLAUDE.md b/CLAUDE.md index 745a4cbd..9d423ff6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -72,7 +72,7 @@ src/ │ └── landing/ # marketing landing page ├── servers/ │ ├── hono.ts # router composition; everything under /api -│ ├── _middlewares/ # auth, body parsing, origin validation, rate limiting +│ ├── _middlewares/ # auth, body parsing, the capability gate, rate limiting │ ├── api// # one folder per feature, each exporting a router │ ├── channels/ # send-claude-code / send-opencode — how /chat drives an agent turn │ ├── queue/ # background job engine @@ -142,14 +142,14 @@ exceed Postgres's 63-character identifier limit: name it explicitly. See `src/da ## Security Model - `IS_DEV_BUILD` (`src/servers/build-env.ts`) is true **only** when `PUBLIC_BUILD_ENV` is explicitly - `dev`/`development`. Everything else, including unset, is hardened. Origin validation, rate - limiting and password rules all key off it — they fail closed. -- Allowed origins come from `PUBLIC_URL`. Officer always sits behind an HTTPS reverse proxy, so the - forwarded `Host` must equal `PUBLIC_URL`'s authority exactly. -- **`ALLOW_ANY_ORIGIN` defaults to ON** — origin checking is off unless the var is explicitly `false`. - A deliberate inversion of the usual rule, safe only because the perimeter is the tailnet and a valid - token is still required on every protected route. It is defence in depth that is currently switched - off, not the lock. + `dev`/`development`. Everything else, including unset, is hardened. Rate limiting and password + rules key off it — they fail closed. +- **There is no origin checking.** It was removed on 2026-08-13, along with `ALLOW_ANY_ORIGIN` and + `ALLOW_ANY_ORIGIN_MUSIC`. The flag defaulted to ON, so origin validation ran on no real install — + what came out was documented defence in depth that was already switched off. Origin was never + authentication here anyway: an app's `officer://` origin is chosen by the client, forgeable + outside a browser, and extractable from a shipped binary. The perimeter is the tailnet, and the lock + is a valid token on every protected route plus the capability gate below. - JWTs are 30-day, blacklisted on signout, and invalidated by a password change (`passwordChangedAt`). **The role is deliberately not a claim** — every authorization decision re-reads `users.role` from Postgres, so a grant or a revoke takes effect on the next request rather than at next sign-in. @@ -159,7 +159,7 @@ exceed Postgres's 63-character identifier limit: name it explicitly. See `src/da ### Capabilities — read this before mounting a router Authorization is one system, and it is not in `userMiddleware` (which only answers "is this token -valid"). It is `originScopeMiddleware` → `capabilities/authorize.ts`, mounted globally in `hono.ts` +valid"). It is `_middlewares/capability-gate.ts` → `capabilities/authorize.ts`, mounted globally in `hono.ts` ahead of everything, and it re-verifies the token itself so it covers routes that never mount `userMiddleware`. diff --git a/docs/mobile-api-keys.md b/docs/mobile-api-keys.md index 399563b3..ed17484f 100644 --- a/docs/mobile-api-keys.md +++ b/docs/mobile-api-keys.md @@ -213,7 +213,8 @@ both, so the endpoint cannot be used to discover whether an id exists. ## Things that will surprise you -- **No `Origin` header is needed today.** `ALLOW_ANY_ORIGIN` defaults to on, so origin checking is off +- **No `Origin` header is needed.** Origin checking was removed entirely on 2026-08-13; before that it + was off by default and the apps work sending none — which is what they do. Nothing here changes that. If it is ever switched off, every app breaks at once and will need its `OFFICER__ORIGIN` value compiled in; that is a separate conversation, not part of this work. diff --git a/docs/secret-store.md b/docs/secret-store.md index cd1b9c26..4c84a243 100644 --- a/docs/secret-store.md +++ b/docs/secret-store.md @@ -147,7 +147,8 @@ The core is what `ecosystem.light.config.cjs` runs today — `officer`, `officer `officer-agent`, `officer-opencode`, `officer-pty` — **plus `officer-headscale`**. Headscale is core for a stated reason rather than by preference: `CLAUDE.md` says the tailnet *is* the -perimeter, and that `ALLOW_ANY_ORIGIN` defaulting on is only defensible because of it. A security model +perimeter — origin checking was removed on 2026-08-13 precisely because the tailnet is what stands in +its place, so the tailnet is now load-bearing rather than one layer of two. A security model that rests on the tailnet cannot treat administering the tailnet as an optional extra. Vaultwarden and the wallet are not load-bearing that way — nothing else stops working without them — so they become plugins. diff --git a/scripts/setup/machine-setup/lib/tailscale.sh b/scripts/setup/machine-setup/lib/tailscale.sh index e1fca3f9..e2264ea2 100644 --- a/scripts/setup/machine-setup/lib/tailscale.sh +++ b/scripts/setup/machine-setup/lib/tailscale.sh @@ -14,11 +14,10 @@ # # ── Why it matters to Officer specifically ── # -# The platform's CLAUDE.md is explicit: the perimeter IS the tailnet. -# ALLOW_ANY_ORIGIN defaults ON, and that is only defensible because the machine is -# not reachable from the open internet in the first place — a valid token plus the -# tailnet is the lock. An Officer install with no tailnet is an Officer install -# with one fewer layer than it was designed around. +# The platform's CLAUDE.md is explicit: the perimeter IS the tailnet. Origin +# checking was removed outright on 2026-08-13 because the tailnet stands in its +# place, so a valid token plus the tailnet IS the lock — not one layer of two. +# An Officer install with no tailnet is missing the half the design assumes. # # ── Why the original hung ── # @@ -46,10 +45,9 @@ tailscale_help() { echo " port forwarding, no exposed ports, no holes in the firewall." echo "" echo " For Officer it is not a convenience. The platform is built assuming" - echo " the tailnet IS the perimeter — ALLOW_ANY_ORIGIN defaults on, and" - echo " that is only defensible because the machine is not reachable from" - echo " outside in the first place. A valid token plus the tailnet is the" - echo " lock; without the tailnet it is one layer short of its design." + echo " the tailnet IS the perimeter, and there is no origin checking behind" + echo " it — a valid token plus the tailnet is the whole lock. Without the" + echo " tailnet you are running with half of it missing." echo "" echo " It is installed at this point in the run, before anything that can" echo " lock you out of the machine, so there is always a second way in." @@ -162,9 +160,8 @@ tailscale_networks_help() { echo "" echo " FOR OFFICER" echo " Whichever you pick, the tailnet is what Officer treats as its" - echo " perimeter. ALLOW_ANY_ORIGIN defaults on, and that is only" - echo " defensible because the machine is not reachable from the open" - echo " internet in the first place. Installed at this point in the run," + echo " perimeter, and it is not one layer of two — there is no origin" + echo " checking behind it. Installed at this point in the run," echo " before anything that can lock you out, so there is always a second" echo " way in." echo "" @@ -219,15 +216,14 @@ tailscale_none_warning() { echo " · This machine will be found. Anything listening on a public" echo " address is scanned within minutes and attacked continuously." echo "" - echo " For Officer specifically, one setting stops being safe:" + echo " For Officer specifically, this removes a layer that cannot be put" + echo " back from a setting:" echo "" - echo " ALLOW_ANY_ORIGIN defaults ON, which means origin checking is off" - echo " unless it is explicitly set to false. That default is deliberate" - echo " and it is only defensible because the tailnet is the perimeter." - echo " With no tailnet you must set ALLOW_ANY_ORIGIN=false and put an" - echo " HTTPS reverse proxy in front of the platform, or it is running" - echo " with a check disabled that was disabled on the assumption you" - echo " are making false." + echo " There is no origin checking in the platform. It was removed" + echo " because the tailnet is the perimeter, so a valid token plus the" + echo " tailnet is the entire lock. With no tailnet, the token is the" + echo " only thing left. Put an HTTPS reverse proxy in front of the" + echo " platform and restrict who can reach it at the network layer." } tailscale_needs_logout() { diff --git a/scripts/setup/machine-setup/machine-setup.sh b/scripts/setup/machine-setup/machine-setup.sh index b6f6be45..4f2f5d70 100755 --- a/scripts/setup/machine-setup/machine-setup.sh +++ b/scripts/setup/machine-setup/machine-setup.sh @@ -491,8 +491,8 @@ if ! skip; then if [[ "$TS_PLANE" == "none" ]]; then warn "no private network — Tailscale is installed but not connected" echo " Connect it later with: sudo tailscale up" - echo " Remember ALLOW_ANY_ORIGIN=false and an HTTPS proxy in front of Officer." - SUMMARY+=("Tailscale: NOT connected by choice — no private network, ALLOW_ANY_ORIGIN must be set false") + echo " Remember an HTTPS proxy in front of Officer, and restrict who can reach it." + SUMMARY+=("Tailscale: NOT connected by choice — no private network, so the token is the only lock") TS_CONNECT=false fi diff --git a/scripts/setup/officer-setup.sh b/scripts/setup/officer-setup.sh index 745a29de..e3939eb9 100755 --- a/scripts/setup/officer-setup.sh +++ b/scripts/setup/officer-setup.sh @@ -474,25 +474,9 @@ if ! skip; then ask_required ENV_PORT "Port Officer listens on" "${ENV_PORT:-9000}" ENV_BROWSER_RELAY_PORT="${ENV_BROWSER_RELAY_PORT:-18792}" - # ── origin checking, decided by the machine rather than by a default ── - # - # ALLOW_ANY_ORIGIN defaults to ON inside the platform, which CLAUDE.md says is - # only defensible because the tailnet is the perimeter. So the value is written - # explicitly here, from whether this machine actually has one. - if tailnet_present; then - ENV_ALLOW_ANY_ORIGIN="true" - ORIGIN_WHY="tailscale0 is up, so the tailnet is the perimeter" - else - ENV_ALLOW_ANY_ORIGIN="false" - ORIGIN_WHY="no tailnet on this machine, so origin checking is left ON" - fi - echo "" - echo " ALLOW_ANY_ORIGIN=${ENV_ALLOW_ANY_ORIGIN} — ${ORIGIN_WHY}" - echo "" echo " to write:" echo " PORT=${ENV_PORT} BROWSER_RELAY_PORT=${ENV_BROWSER_RELAY_PORT}" - echo " ALLOW_ANY_ORIGIN=${ENV_ALLOW_ANY_ORIGIN}" echo " POSTGRES_URL=${POSTGRES_URL%%:*}://…" echo "" echo " the install root is not written here — the platform derives it as the" diff --git a/scripts/setup/officer-setup/lib/env.sh b/scripts/setup/officer-setup/lib/env.sh index 441b957a..66309808 100644 --- a/scripts/setup/officer-setup/lib/env.sh +++ b/scripts/setup/officer-setup/lib/env.sh @@ -44,12 +44,6 @@ env_get() { }' "$(env_file)" } -# Origin checking is OFF unless this is explicitly false — CLAUDE.md is explicit -# that the inversion is deliberate and is only defensible because the tailnet is -# the perimeter. With no tailnet there is no perimeter, so the default stops -# being defensible and the value has to be written the other way. -tailnet_present() { ip link show tailscale0 &>/dev/null; } - write_env() { local dest dest="$(env_file)" @@ -76,10 +70,6 @@ BROWSER_RELAY_PORT="${ENV_BROWSER_RELAY_PORT}" POSTGRES_URL="${POSTGRES_URL}" -# Origin checking. Off by default in the platform, which is only safe behind a -# tailnet; written explicitly here so the machine's actual situation decides it. -ALLOW_ANY_ORIGIN="${ENV_ALLOW_ANY_ORIGIN}" - DISCORD_BUG_REPORT_WEBHOOK="${ENV_DISCORD_WEBHOOK}" ENVF diff --git a/src/servers/_middlewares/capability-gate.ts b/src/servers/_middlewares/capability-gate.ts new file mode 100644 index 00000000..e5748ff9 --- /dev/null +++ b/src/servers/_middlewares/capability-gate.ts @@ -0,0 +1,51 @@ +import type { MiddlewareHandler } from 'hono'; +import * as errors from '../custom-errors'; +import { resolveAuthToken } from '../auth-token'; +import { isSuperAdmin } from '../super-admin'; +import { isApiRequestAllowed } from '../capabilities/authorize'; +import { isExemptApiPath } from '../capabilities/totality'; + +// The global authorization gate: a valid NON-owner token may reach only what its ROLE has been granted. +// +// Mounted in hono.ts ahead of every router, and it re-verifies the token itself rather than trusting an +// earlier middleware — so it covers routes that never mount `userMiddleware`, which is most of the reason +// it exists. See auth-token.ts: the caller resolved here must be the same caller `userMiddleware` would +// resolve, or the two doors disagree about who someone is. +// +// A missing or invalid token passes straight through. Signin needs that, and `userMiddleware` rejects bad +// tokens on the protected routes. Only a VALID non-owner token is constrained here. +// +// ── What this file used to be ── +// +// This was `originScopeMiddleware`, the second half of `origin-validation.ts`, which also enforced +// per-origin path scoping: an app shipping a custom-scheme origin (`officer://` via +// OFFICER__ORIGIN) could reach only `/api/auth` plus its own feature. +// +// All of that is gone as of 2026-08-13, along with ALLOW_ANY_ORIGIN and ALLOW_ANY_ORIGIN_MUSIC. Origin +// was never authentication here — the custom-scheme origin is chosen by the client, forgeable outside a +// browser, and extractable from a shipped app binary — and the flag that disabled it defaulted to ON, so +// on every real install none of it ran. It was documented as defence in depth that was switched off. +// +// This half was deliberately NOT under that flag, because it is account-based rather than origin-based. +// Its old comment called it "the airtight half", and separating the two is why the name changed: nothing +// in here looks at an Origin header any more. +export const capabilityGateMiddleware: MiddlewareHandler = async function (ctx, next) { + const path = ctx.req.path; + + const authorization = ctx.req.header('authorization'); + const token = authorization ? authorization.split(' ')[1] : ctx.req.query('token') || undefined; + const payload = token ? await resolveAuthToken(token) : null; + const isOwner = payload ? await isSuperAdmin(payload) : false; + + // Exempt paths (signin, the public pages, the Bitwarden door) are skipped because they are served above + // the account gate — the same list the boot check uses, deliberately. + // + // Non-/api paths are not ours: the DAV sync door authenticates with its own app password and carries no + // platform account, so there is nothing here to resolve. + if (payload && !isOwner && path.startsWith('/api') && !isExemptApiPath(path)) { + const { allowed, reason } = await isApiRequestAllowed(payload.id, ctx.req.method, path); + if (!allowed) throw errors.FORBIDDEN(reason ?? 'Not permitted for this account'); + } + + return next(); +}; diff --git a/src/servers/_middlewares/index.ts b/src/servers/_middlewares/index.ts index be6dbbc4..bdc6fd55 100644 --- a/src/servers/_middlewares/index.ts +++ b/src/servers/_middlewares/index.ts @@ -1,7 +1,7 @@ export * from './body-parser'; export * from './user-middleware'; export * from './origin-middleware'; -export * from './origin-validation'; +export * from './capability-gate'; export * from './rate-limiter'; export * from './known-users'; export * from './auth-audit'; diff --git a/src/servers/_middlewares/origin-validation.test.ts b/src/servers/_middlewares/origin-validation.test.ts deleted file mode 100644 index 52d8b94a..00000000 --- a/src/servers/_middlewares/origin-validation.test.ts +++ /dev/null @@ -1,37 +0,0 @@ -import { test, expect } from 'bun:test'; - -// origin-validation reads PUBLIC_URL / PUBLIC_BUILD_ENV at module load, so set them before importing. -process.env.PUBLIC_URL = 'https://officer.example.com'; -process.env.PUBLIC_BUILD_ENV = 'production'; -// Bun loads the host's .env into tests, so an operational kill switch left on there would silently turn -// these assertions into no-ops — which is exactly what ALLOW_ANY_ORIGIN=true did. Pin the escape hatches -// off: this file's whole job is asserting that the checks reject things. -process.env.ALLOW_ANY_ORIGIN = 'false'; -process.env.ALLOW_ANY_ORIGIN_MUSIC = 'false'; - -const { isOriginAllowed } = await import('./origin-validation'); - -test('accepts the configured origin', () => { - expect(isOriginAllowed('https://officer.example.com', 'officer.example.com')).toBe(true); -}); - -test('accepts the Host forwarded by the reverse proxy when there is no Origin', () => { - expect(isOriginAllowed(undefined, 'officer.example.com')).toBe(true); -}); - -test('rejects a foreign Origin', () => { - expect(isOriginAllowed('https://evil.com', 'officer.example.com')).toBe(false); - expect(isOriginAllowed('https://officer.example.com.evil.com', 'officer.example.com')).toBe(false); -}); - -// Regression: the Host branch used `configuredOrigin.endsWith(host)`, so any suffix of the origin — -// down to a bare TLD — authenticated as the real host. -test('rejects Hosts that are merely suffixes of the configured origin', () => { - for (const host of ['com', 'example.com', 'r.example.com', 'ficer.example.com', 'evil.com']) { - expect(isOriginAllowed(undefined, host)).toBe(false); - } -}); - -test('rejects a request carrying neither Origin nor Host', () => { - expect(isOriginAllowed(undefined, undefined)).toBe(false); -}); diff --git a/src/servers/_middlewares/origin-validation.ts b/src/servers/_middlewares/origin-validation.ts deleted file mode 100644 index 73857b31..00000000 --- a/src/servers/_middlewares/origin-validation.ts +++ /dev/null @@ -1,220 +0,0 @@ -import type { MiddlewareHandler } from 'hono'; -import * as errors from '../custom-errors'; -import { IS_DEV_BUILD } from '../build-env'; -import { resolveAuthToken } from '../auth-token'; -import { isSuperAdmin } from '../super-admin'; -import { isApiRequestAllowed } from '../capabilities/authorize'; -import { isExemptApiPath } from '../capabilities/totality'; - -const { PUBLIC_URL } = process.env; - -// The allowed production web origin comes from PUBLIC_URL in .env (e.g. https://officer.pastilhas.dev), -// not a hardcoded domain. -const PUBLIC_ORIGIN = (() => { - try { - return PUBLIC_URL ? new URL(PUBLIC_URL).origin : undefined; - } catch { - return undefined; - } -})(); - -const WEB_ORIGINS: string[] = PUBLIC_ORIGIN ? [PUBLIC_ORIGIN] : []; - -// Host authorities (`example.com`, or `example.com:8080` off the default port) for the same origins. -// Officer always sits behind an HTTPS reverse proxy, so the proxy's `Host` header is expected to -// match PUBLIC_URL's authority exactly. -const WEB_HOSTS: string[] = WEB_ORIGINS.map((o) => new URL(o).host); - -const CHROME_EXTENSIONS: string[] = [ - // 'chrome-extension://' -]; - -// Every `OFFICER__ORIGIN` in the environment is an allowed app origin — each app ships a -// custom-scheme origin with an embedded token (`officer://`), set on the host, never in the repo. -// Adding an app is adding an env var; no code change, which is the point. The slug is what sits between -// the two fixed words: OFFICER_MUSIC_ORIGIN → MUSIC. -const APP_ORIGIN_VAR = /^OFFICER_([A-Z0-9_]+)_ORIGIN$/; - -// Apps that are the whole platform rather than one feature of it, so they get NO path scoping — the main -// Officer app needs every prefix the web app needs. -// -// These used to be `superAdminOnly`, which refused a non-owner outright, here and at signin. That was -// correct while single-user was the invariant and the only non-owner accounts were music-app accounts: -// there was no way to express "this person may use the platform, but only these parts of it", so the -// honest answer was to keep them out of it entirely. -// -// Capabilities express exactly that, per feature, at both doors. So the blunt version is gone — a member -// signs into the web app and sees what their role was granted. Keeping both would mean a member who has -// been granted Gitea still cannot reach the page, which is not a second layer of defence, just a bug. -const PLATFORM_APPS = new Set(['APP']); - -// Where an app's API surface isn't `/api/`. Only exceptions belong here. -const APP_SCOPE_OVERRIDES: Record = { - // OffTail signs in and mints a Headscale pre-auth key. The VPN's own traffic goes straight to - // Headscale and never through /api, so this is its entire platform surface. - TAIL: ['/api/vpn'], -}; - -type AppOrigin = { slug: string; origin: string }; - -const APP_ORIGIN_LIST: AppOrigin[] = Object.entries(process.env).flatMap(([key, value]) => { - const slug = key.match(APP_ORIGIN_VAR)?.[1]; - return slug && value ? [{ slug, origin: value }] : []; -}); - -const APP_ORIGINS: string[] = APP_ORIGIN_LIST.map((a) => a.origin); - -// The account backstop used to live here as two hand-written lists: NON_OWNER_PATHS, confining every -// non-owner to '/api/auth' + '/api/music', and NON_OWNER_WS_PROVIDERS doing the same for sockets. Both -// are gone, replaced by the capability registry (src/servers/capabilities/). -// -// They were not wrong, they were unscalable in one specific way: a hardcoded allow-list answers "which -// paths" but never "why", so onboarding anyone who needed anything other than music meant editing an -// array in a middleware file and hoping the socket half got edited too. The registry makes the two doors -// read the same declaration, and the boot-time totality check makes a THIRD door impossible to add -// without noticing. See capabilities/totality.ts for the incident that motivated it. - -function pathAllowed(path: string, prefixes: string[]): boolean { - return prefixes.some((prefix) => path === prefix || path.startsWith(`${prefix}/`)); -} - -// Per-origin access rules, enforced globally by originScopeMiddleware (mounted in hono.ts): -// - paths: this Origin may reach ONLY these path prefixes; anything else is 403. -// Origins with no rule keep full access. Rules no-op for env values that are unset. -// -// App rules are derived, not written: an app may reach /api/auth (it has to sign in) plus the one -// feature it is named for — OFFICER_VAULT_ORIGIN gets /api/auth + /api/vault. -type OriginRule = { paths?: string[] }; -const ORIGIN_RULES: Record = {}; - -for (const { slug, origin } of APP_ORIGIN_LIST) { - ORIGIN_RULES[origin] = PLATFORM_APPS.has(slug) - ? {} // no path scoping; the capability backstop is what limits a platform app's caller - : { paths: ['/api/auth', ...(APP_SCOPE_OVERRIDES[slug] ?? [`/api/${slug.toLowerCase()}`])] }; -} - -// Last, so the web origin wins if an app ever declares the same one. No path scoping, for the same reason -// as PLATFORM_APPS above: the web app is the whole platform, and what its caller may reach is decided by -// their capabilities rather than by their Origin. -if (PUBLIC_ORIGIN) ORIGIN_RULES[PUBLIC_ORIGIN] = {}; - -// ── TEMPORARY: origin checking switched off ── -// ALLOW_ANY_ORIGIN=true accepts every Origin, everywhere, and skips the per-origin path scoping. -// ALLOW_ANY_ORIGIN_MUSIC=true is the narrower version, /api/music only — which is not enough on its own, -// because an app has to reach /api/auth to sign in before it ever calls its own feature. -// -// What still holds with these on: every protected route requires a valid token (userMiddleware), and the -// capability backstop below still confines a non-owner account to what its ROLE has been granted, -// whatever Origin it claims — that one is deliberately NOT disabled, since it is account-based, not -// origin-based. -// -// What is lost: defence in depth, not the lock. Origin was never authentication here — `officer://` -// is chosen by the client, forgeable outside a browser, and extractable from a shipped app binary. -// Both flags and their call sites come out once the tailnet is the perimeter. -// Defaults to ON — origin checking is off unless ALLOW_ANY_ORIGIN is explicitly 'false'. That is a -// deliberate inversion of the usual fail-closed rule, and it is safe only because of where this runs: the -// perimeter is the tailnet, devices are admitted by hand, and a valid token is still required on every -// protected route. Set ALLOW_ANY_ORIGIN=false to put the checks back. -const ALLOW_ANY_ORIGIN = (process.env.ALLOW_ANY_ORIGIN ?? 'true') !== 'false'; -const ALLOW_ANY_ORIGIN_MUSIC = process.env.ALLOW_ANY_ORIGIN_MUSIC === 'true'; - -export function isOriginCheckDisabled(path?: string): boolean { - if (ALLOW_ANY_ORIGIN) return true; - return ALLOW_ANY_ORIGIN_MUSIC && !!path && pathAllowed(path, ['/api/music']); -} - -/** @deprecated use isOriginCheckDisabled — kept so existing call sites read the same. */ -export const isMusicOriginExempt = isOriginCheckDisabled; - -export function isOriginAllowed(origin: string | undefined, host?: string): boolean { - if (IS_DEV_BUILD) return true; - if (ALLOW_ANY_ORIGIN) return true; - - if (origin) { - if (origin.startsWith('chrome-extension://')) { - return CHROME_EXTENSIONS.includes(origin); - } - - if (APP_ORIGINS.includes(origin)) { - return true; - } - - return WEB_ORIGINS.includes(origin); - } - - if (host) { - return WEB_HOSTS.includes(host); - } - - return false; -} - -export const originValidationMiddleware: MiddlewareHandler = function (ctx, next) { - const origin = ctx.get('origin') as string | undefined; - const host = ctx.req.header('host'); - if (!isOriginAllowed(origin, host)) { - throw errors.FORBIDDEN('Invalid origin'); - } - return next(); -}; - -function resolveOrigin(headerOrigin: string | undefined, referer: string | undefined): string | undefined { - if (headerOrigin) return headerOrigin; - if (referer) { - try { - return new URL(referer).origin; - } catch { - return undefined; - } - } - return undefined; -} - -// Global gate applied to every request (mounted in hono.ts). Reads the Origin header directly (not -// ctx 'origin') so it covers the whole /api tree — the public /api/auth and the protected /api/music -// alike — regardless of which routers mount originMiddleware. Two layers: -// 1. Capability backstop (origin-INDEPENDENT): a valid NON-owner token may reach only what its ROLE -// has been granted, no matter the origin. This is the airtight rule — it holds even if a client -// omits or forges the Origin header. It replaced a hardcoded "/api/auth + /api/music" list on -// 2026-08-07; that list was why a Member could not reach /api/gitea and no UI could change it. -// 2. Per-origin rules (ORIGIN_RULES): path scoping for the single-feature apps. Redundant with the -// backstop for the account dimension, but blocks unknown-path access from an app origin. -// A missing/invalid token passes both layers (signin needs it; userMiddleware rejects bad tokens on -// protected routes). Only a VALID non-owner token is constrained. -export const originScopeMiddleware: MiddlewareHandler = async function (ctx, next) { - const path = ctx.req.path; - const origin = resolveOrigin(ctx.req.header('origin'), ctx.req.header('referer')); - - // Resolve the caller once (if any); a missing/invalid/revoked credential stays null. This goes through - // the shared resolver rather than verifying a JWT here, so an API key is the same caller at this door as - // it is at userMiddleware — the two must never disagree about who someone is. See auth-token.ts. - const authorization = ctx.req.header('authorization'); - const token = authorization ? authorization.split(' ')[1] : ctx.req.query('token') || undefined; - const payload = token ? await resolveAuthToken(token) : null; - const isOwner = payload ? await isSuperAdmin(payload) : false; - - // 1. Capability backstop — origin-INDEPENDENT, and the airtight half of this middleware. A valid - // non-owner token may reach only what its ROLE has been granted, whatever Origin it claims and whether - // or not it sends one. Exempt paths (signin, the public pages, the Bitwarden door) are skipped because - // they are served above the account gate — the same list the boot check uses, deliberately. - // - // Non-/api paths are not ours: the DAV sync door authenticates with its own app password and carries no - // platform account, so there is nothing here to resolve. - if (payload && !isOwner && path.startsWith('/api') && !isExemptApiPath(path)) { - const { allowed, reason } = await isApiRequestAllowed(payload.id, ctx.req.method, path); - if (!allowed) throw errors.FORBIDDEN(reason ?? 'Not permitted for this account'); - } - - // 2. Per-origin rules. Skipped entirely while origin checking is off (the account backstop above is - // account-based, not origin-based, so it deliberately still applies). - if (isOriginCheckDisabled(path)) return next(); - - const rule = origin ? ORIGIN_RULES[origin] : undefined; - if (rule) { - if (rule.paths && !pathAllowed(path, rule.paths)) { - throw errors.FORBIDDEN('Origin not permitted for this resource'); - } - } - - return next(); -}; diff --git a/src/servers/_middlewares/user-middleware.ts b/src/servers/_middlewares/user-middleware.ts index 287c7208..1d4cc494 100644 --- a/src/servers/_middlewares/user-middleware.ts +++ b/src/servers/_middlewares/user-middleware.ts @@ -1,7 +1,6 @@ import type { MiddlewareHandler } from 'hono'; import { resolveAuthToken } from '@@/auth-token'; import * as errors from '@@/custom-errors'; -import { isOriginAllowed, isMusicOriginExempt } from './origin-validation'; import { isLockdown, noteBlocked } from '../api/auth/panic'; import { getUserById, isTokenBlacklisted } from 'officerdb'; @@ -24,16 +23,9 @@ export const userMiddleware: MiddlewareHandler = async function (ctx, next) { } if (!token) throw errors.UNAUTHORIZED(); - // Validate origin for authenticated routes. - // TEMPORARY, opt-in: ALLOW_ANY_ORIGIN_MUSIC=true drops the Origin check for /api/music only, so a - // client that can't present the app's custom-scheme origin can still reach the music API. Auth is - // untouched — a valid token is still required, and the non-owner account backstop in - // originScopeMiddleware still applies. Delete this and the env var once the tailnet is the perimeter. - const origin = ctx.get('origin') as string | undefined; - const host = ctx.req.header('host'); - if (!isMusicOriginExempt(ctx.req.path) && !isOriginAllowed(origin, host)) { - throw errors.FORBIDDEN('Invalid origin'); - } + // There was an Origin check here until 2026-08-13. It is gone with the rest of origin validation — + // it had defaulted to off, so it ran on no real install. A valid token is required below, and the + // capability gate in hono.ts confines a non-owner to what their role grants. try { // Both credentials resolve here — see auth-token.ts. Everything below applies to a session JWT only: diff --git a/src/servers/api/auth/auth.ts b/src/servers/api/auth/auth.ts index 3fd141a4..8375e8f0 100644 --- a/src/servers/api/auth/auth.ts +++ b/src/servers/api/auth/auth.ts @@ -2,7 +2,6 @@ import { createRouter } from '../../create-router'; import { authAudit, originMiddleware, - originValidationMiddleware, userMiddleware, bodyParser, signinRateLimiter, @@ -25,10 +24,11 @@ export const authRouter = createRouter(); authRouter.use(bodyParser()); // After the body parser (it reads the claimed identity out of the body) and before everything else, so -// that a probe rejected by origin validation or the rate limiter is recorded too. Observes only. +// that a probe rejected by the rate limiter is recorded too. Observes only. authRouter.use(authAudit); +// Extracts the Origin (or derives it from Referer) for the handlers that log it. Not a check — origin +// validation was removed on 2026-08-13. authRouter.use(originMiddleware); -authRouter.use(originValidationMiddleware); authRouter.use('/', async (ctx) => ctx.json({ officerAuthServer: 'ok' })); diff --git a/src/servers/api/auth/signin.ts b/src/servers/api/auth/signin.ts index eb3ecdec..9099f36e 100755 --- a/src/servers/api/auth/signin.ts +++ b/src/servers/api/auth/signin.ts @@ -10,11 +10,12 @@ const TEST_USERS: number[] = []; export const signinHandler: Handler = async function (ctx) { const { email, password } = ctx.get('body'); // A caller that sends neither Origin nor Referer — a server-to-server client, curl — leaves this - // undefined. It used to be unreachable: originValidationMiddleware rejected those requests before this - // handler ran, so the value was always a string by the time anything touched it. That is no longer - // guaranteed (ALLOW_ANY_ORIGIN lets them through), and `undefined` reached both a SQL parameter and - // `.startsWith`. Normalising to '' keeps every downstream use honest: no passkey is registered against - // the empty origin, so an origin-less caller falls through to password auth, which is what it wants. + // undefined. It was once unreachable: origin validation rejected those requests before this handler + // ran, so the value was always a string by the time anything touched it. Then the flag that disabled + // that check defaulted to on, and `undefined` reached both a SQL parameter and `.startsWith`. Origin + // validation is gone entirely as of 2026-08-13, so undefined is now the ordinary case rather than the + // edge one. Normalising to '' keeps every downstream use honest: no passkey is registered against the + // empty origin, so an origin-less caller falls through to password auth, which is what it wants. const origin = (ctx.get('origin') as string | undefined) ?? ''; // Panic lockdown active → refuse all logins (looks like a normal failed login). diff --git a/src/servers/api/users/capabilities-routes.ts b/src/servers/api/users/capabilities-routes.ts index aff7d331..8734a3e5 100644 --- a/src/servers/api/users/capabilities-routes.ts +++ b/src/servers/api/users/capabilities-routes.ts @@ -13,7 +13,7 @@ import { capabilityAvailability } from '../../app-store/availability'; // `/user/capabilities` answers "what may I do" for the caller, and every account may ask. The dock, the // app registry and the route guards all read it, so it is the frontend's whole view of the permission // model — and it must never be the frontend's ENFORCEMENT of it. Hiding a dock icon is a courtesy; the -// 403 in origin-validation is the lock. +// 403 from the capability gate is the lock. // // Everything else here is owner-only and edits the policy itself. diff --git a/src/servers/api/users/users-router.ts b/src/servers/api/users/users-router.ts index e52e1684..5cda5ad9 100644 --- a/src/servers/api/users/users-router.ts +++ b/src/servers/api/users/users-router.ts @@ -15,8 +15,8 @@ usersRouter.use(originMiddleware); // Self-update. Any signed-in account may change its own name, username and avatar. usersRouter.put('/', updateUserHandler); -// Everything below manages OTHER accounts and is the owner's alone. The global capability backstop in -// originScopeMiddleware already refuses a non-owner here — `user-admin` is `kind: 'admin'`, so it is +// Everything below manages OTHER accounts and is the owner's alone. The global capability gate in +// hono.ts already refuses a non-owner here — `user-admin` is `kind: 'admin'`, so it is // not grantable — but that router-level rule cannot see the one exception beside it: `PUT /` is // declared `selfService` so every account can edit its own profile. This gate is what keeps that // exception from widening to the routes below it, and it is a second lock rather than a restatement. diff --git a/src/servers/api/vault/router.ts b/src/servers/api/vault/router.ts index ded3b1d8..1d899705 100644 --- a/src/servers/api/vault/router.ts +++ b/src/servers/api/vault/router.ts @@ -15,7 +15,6 @@ import { getVaultTokens, setVaultTokens, getVaultUnlockKey, setVaultUnlockKey } // • serves the native broker/unlock-key endpoints, // • proxies the rest to the officer-vault sidecar, swapping the incoming platform JWT for the stored // Vaultwarden token. Bodies are never parsed/decrypted; only the Authorization header is rewritten. -// The origin scoping (OFFICER_VAULT_ORIGIN → /api/vault) is enforced globally by originScopeMiddleware. export const vaultRouter = createRouter(); diff --git a/src/servers/api/vault/websocket.ts b/src/servers/api/vault/websocket.ts index d16065c6..6a39a801 100644 --- a/src/servers/api/vault/websocket.ts +++ b/src/servers/api/vault/websocket.ts @@ -2,7 +2,6 @@ import type { ServerWebSocket } from 'bun'; import { resolveAuthToken } from '../../auth-token'; import { isTokenBlacklisted } from 'officerdb'; import { isSuperAdmin } from '../../super-admin'; -import { isOriginAllowed } from '../../_middlewares'; import { getVaultServerWsUrl } from './sidecar-server'; import { getValidAccessToken } from './token-store'; @@ -140,14 +139,13 @@ export const vaultWebsocket = { const PREFIX = '/api/vault'; -// Serve-level upgrade for /api/vault/notifications/* WebSockets. Origin-gated; the platform JWT rides the -// query (?access_token= for SignalR, or ?token=). The session is validated in `open` (deferred). The -// device never sends a Vaultwarden token — we inject the stored one upstream. +// Serve-level upgrade for /api/vault/notifications/* WebSockets. The platform JWT rides the query +// (?access_token= for SignalR, or ?token=) and the session is validated in `open` (deferred). The device +// never sends a Vaultwarden token — we inject the stored one upstream. +// +// There was an isOriginAllowed gate here until 2026-08-13, removed with the rest of origin validation. +// It had defaulted to allow-everything, so it refused nothing on a real install. export function upgradeVaultWs(req: Request, server: any): Response | undefined { - const origin = req.headers.get('origin') ?? undefined; - const host = req.headers.get('host') ?? undefined; - if (!isOriginAllowed(origin, host)) return new Response('Forbidden', { status: 403 }); - const url = new URL(req.url); const platformToken = url.searchParams.get('access_token') || url.searchParams.get('token') || ''; if (!platformToken) return new Response('Unauthorized', { status: 401 }); diff --git a/src/servers/auth-token.ts b/src/servers/auth-token.ts index 08d11dc5..0e62be03 100644 --- a/src/servers/auth-token.ts +++ b/src/servers/auth-token.ts @@ -9,7 +9,7 @@ import { verify } from './jwt'; // ── One resolver, two doors ── // // Identity is decided in TWO independent middlewares: `userMiddleware`, which every protected router -// mounts, and `originScopeMiddleware`, which runs globally in hono.ts and re-verifies the token itself +// mounts, and `capabilityGateMiddleware`, which runs globally in hono.ts and re-verifies the token itself // because it must also cover routes that never mount `userMiddleware`. They have to agree about who a // caller is, and the way they stop agreeing is somebody teaching one of them a credential format the // other has never heard of — the second door would then see an unrecognisable token, resolve nobody, and diff --git a/src/servers/hono.ts b/src/servers/hono.ts index 8066a6a1..de1f833d 100644 --- a/src/servers/hono.ts +++ b/src/servers/hono.ts @@ -57,8 +57,7 @@ import { agentStatusRouter } from './api/agent-status/router'; import { chatRouter } from './api/chat/chat'; import { pipelineJobsRouter } from './api/tasks/pipeline-jobs-routes'; import { CustomError } from './custom-errors'; -import { userMiddleware, bodyParser, isOriginAllowed, originScopeMiddleware } from './_middlewares'; -import { isMusicOriginExempt } from './_middlewares/origin-validation'; +import { userMiddleware, bodyParser, capabilityGateMiddleware } from './_middlewares'; export { Hono }; export { createRouter }; @@ -66,14 +65,12 @@ export type { HonoVariables }; export const honoServer = new Hono<{ Variables: HonoVariables }>(); +// Origin checking was removed on 2026-08-13, so CORS echoes back whatever Origin it is given. That is +// not a loosening: the check it replaced defaulted to off, so this is what every real install already +// did. The perimeter is the tailnet and the lock is a valid token on every protected route, plus the +// capability gate below. const corsMiddleware = cors({ - origin: (origin, c) => { - const host = c.req.header('host'); - // TEMPORARY: see isMusicOriginExempt — echoes any Origin back for /api/music when enabled, so a - // browser client is not blocked by CORS after userMiddleware has already let it through. - if (isMusicOriginExempt(c.req.path)) return origin ?? '*'; - return isOriginAllowed(origin, host) ? origin : ''; - }, + origin: (origin) => origin ?? '*', allowMethods: ['GET', 'POST', 'PUT', 'PATCH', 'DELETE'], allowHeaders: ['Content-Type', 'Authorization'], }); @@ -92,17 +89,16 @@ const isDavPath = (path: string) => honoServer.use((ctx, next) => (isDavPath(ctx.req.path) ? next() : corsMiddleware(ctx, next))); -// Scoped-origin gate: restrict app origins (e.g. the music app) to their allowed path prefixes -// (/api/auth + /api/music). No-ops for the main web origin and while OFFICER_MUSIC_ORIGIN is unset. -honoServer.use(originScopeMiddleware); +// The authorization gate: a valid non-owner token reaches only what its role grants. Ahead of every +// router, and it re-verifies the token itself so it covers routes that never mount userMiddleware. +honoServer.use(capabilityGateMiddleware); honoServer.get('/api', (ctx) => ctx.json({ officerAPI: 'ok' })); honoServer.route('/api/auth', authRouter); honoServer.route('/api/landing-page-data', landingPageDataRouter); honoServer.route('/api/waitlist', waitlistRouter); // Vaultwarden reverse-proxy — mounted TOP-LEVEL (not under protectedRouter): the Bitwarden client -// carries its own bearer token, not a platform session JWT, so userMiddleware would 401 it. Origin -// gating still applies via originScopeMiddleware above (OFFICER_VAULT_ORIGIN → /api/vault). The +// carries its own bearer token, not a platform session JWT, so userMiddleware would 401 it. The // notifications WebSocket is upgraded at the serve level (server.tsx). honoServer.route('/api/vault', vaultRouter); diff --git a/src/workspaces/hooks/src/useCapabilities.ts b/src/workspaces/hooks/src/useCapabilities.ts index bde0c05f..ba78a645 100644 --- a/src/workspaces/hooks/src/useCapabilities.ts +++ b/src/workspaces/hooks/src/useCapabilities.ts @@ -6,7 +6,7 @@ import { useClient } from './useClient'; // // THIS IS NOT ACCESS CONTROL. Every answer here is a courtesy: it stops the app offering a member a // Terminal icon that would 403, and stops a screen mounting a panel whose every request will fail. The -// lock is server-side, in origin-validation's capability backstop and the websocket gate — both of which +// lock is server-side, in the capability gate and the websocket gate — both of which // hold regardless of what this hook returns, including when it returns nothing because the request failed. // // Which is why the failure mode below is deliberately generous rather than restrictive: if this request