diff --git a/changelog.d/fixes/0000-jwt-bootstrap-chain-real-peer-loopback.md b/changelog.d/fixes/0000-jwt-bootstrap-chain-real-peer-loopback.md new file mode 100644 index 0000000000..e8cd375ba7 --- /dev/null +++ b/changelog.d/fixes/0000-jwt-bootstrap-chain-real-peer-loopback.md @@ -0,0 +1 @@ +- **fix(auth):** closed the JWT_SECRET bootstrap chain (GHSA-7pq4-8pvv-rx7r). The fresh-install bootstrap gate in `isAuthRequired()` now decides "loopback" from the trusted peer — the token-stamped real TCP peer the custom server writes, the pipeline's own locality verdict, or a real socket — and never from the client-controlled `Host` / `nextUrl.hostname` whenever a stamping server is in front (every supported runtime), so `Host: localhost` from a remote address no longer opens the window; the anonymous first-password write (`POST /api/settings/require-login`) is under the same loopback constraint instead of being open to every network peer, and `managementPolicy` hands its `peerContext` verdict down explicitly. `/api/settings/obsidian` (incl. `/webdav`, which mints reusable WebDAV Basic credentials for a caller-chosen root served before Next.js) joined `ALWAYS_PROTECTED_API_PATHS`, and `enableObsidianVaultSync()` refuses a vault that is, sits inside, or contains the data directory (realpath-resolved), so the WebDAV file service can no longer be pointed at `server.env` / `storage.sqlite` diff --git a/docs/openapi.yaml b/docs/openapi.yaml index ac93ffb20e..5e49a388dc 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -11316,6 +11316,7 @@ paths: tags: - Settings summary: "DELETE settings › obsidian" + x-always-protected: true responses: "200": description: OK @@ -11323,6 +11324,7 @@ paths: tags: - Settings summary: "GET settings › obsidian" + x-always-protected: true responses: "200": description: OK @@ -11330,6 +11332,7 @@ paths: tags: - Settings summary: "POST settings › obsidian" + x-always-protected: true responses: "200": description: OK @@ -11338,6 +11341,7 @@ paths: tags: - Settings summary: "DELETE settings › obsidian › webdav" + x-always-protected: true responses: "200": description: OK @@ -11345,6 +11349,7 @@ paths: tags: - Settings summary: "GET settings › obsidian › webdav" + x-always-protected: true responses: "200": description: OK @@ -11352,6 +11357,7 @@ paths: tags: - Settings summary: "POST settings › obsidian › webdav" + x-always-protected: true responses: "200": description: OK diff --git a/docs/security/ROUTE_GUARD_TIERS.md b/docs/security/ROUTE_GUARD_TIERS.md index d3b69ff2d7..15b25fba0f 100644 --- a/docs/security/ROUTE_GUARD_TIERS.md +++ b/docs/security/ROUTE_GUARD_TIERS.md @@ -169,9 +169,30 @@ server process. | `/api/settings/export-json` | Exports the full settings blob (incl. secrets) | | `/api/settings/import-json` | Replaces the full settings blob | | `/api/providers/health-autopilot/actions` | Executes autopilot remediation actions | +| `/api/settings/obsidian` | Mints reusable WebDAV creds for any vault root | **Response on violation:** `401 Authentication required` +`/api/settings/obsidian` covers its `/webdav` child: `POST` points the WebDAV file service — +served by the custom Node layer before Next.js, outside this pipeline — at a caller-chosen root +and echoes freshly minted Basic credentials, `DELETE` rotates them, and the parent `POST` stores +the Obsidian REST API token. GHSA-62vw only masked the `GET` password reveal; the issuance was +still on the fail-open tier (GHSA-7pq4-8pvv-rx7r). `enableObsidianVaultSync()` additionally +refuses a vault that is, sits inside, or contains the data directory. + +### Fresh-install bootstrap is loopback-only — by real peer, not `Host` + +With no management password configured (and no `INITIAL_PASSWORD`), `isAuthRequired()` in +`src/shared/utils/apiAuth.ts` keeps the anonymous bootstrap open **only for loopback peers**. +Loopback is decided from the trusted peer signals, in order: the token-stamped real TCP peer +(`PEER_IP_HEADER` + `VIA_PROXY_HEADER`, what the policy sees), the pipeline's own +`AUTHZ_HEADER_PEER_LOCALITY` verdict (what route handlers see, trusted only while +`OMNIROUTE_PEER_STAMP_TOKEN` is set), or a real socket peer for direct callers. `Host` / +`nextUrl.hostname` are never consulted, and the first-password write +(`POST /api/settings/require-login`) is under the same constraint rather than open to every +network peer (GHSA-7pq4-8pvv-rx7r). `managementPolicy` passes its own `peerContext` verdict +down explicitly, so the ORIGINAL (pre-strip) request's headers never decide it. + ### Tier 3 — MANAGEMENT (default) All other management routes. Auth required unless `requireLogin=false` is diff --git a/scripts/dev/run-protocol-clients-tests.mjs b/scripts/dev/run-protocol-clients-tests.mjs index c32320c2f1..5300fd1ac3 100644 --- a/scripts/dev/run-protocol-clients-tests.mjs +++ b/scripts/dev/run-protocol-clients-tests.mjs @@ -57,11 +57,11 @@ async function main() { OMNIROUTE_BASE_URL: baseUrl, }), OMNIROUTE_E2E_BOOTSTRAP_MODE: process.env.OMNIROUTE_E2E_BOOTSTRAP_MODE || "open", - // Pin the custom server's bind address to loopback (#11535): under the - // programmatic next() entry the middleware's nextUrl.hostname mirrors the - // configured HOST (default "0.0.0.0"), and apiAuth.isLoopbackRequest() reads - // nextUrl.hostname FIRST — an unpinned boot makes every request look remote, - // so the anonymous open-bootstrap allow never fires (401 green-shallow). + // Pin the custom server's bind address to loopback (#11535). The bootstrap + // loopback verdict (apiAuth.isLoopbackRequest) comes from the peer stamp the + // custom server writes from the real TCP socket (GHSA-7pq4-8pvv-rx7r), never + // from nextUrl.hostname / Host — the pin keeps the harness's own clients on a + // loopback socket so that stamp resolves to 127.0.0.1. HOST: process.env.HOST || "127.0.0.1", }; diff --git a/src/lib/obsidianSync.ts b/src/lib/obsidianSync.ts index febab545f8..4ae96a5500 100644 --- a/src/lib/obsidianSync.ts +++ b/src/lib/obsidianSync.ts @@ -1,5 +1,6 @@ import fs from "node:fs"; import path from "node:path"; +import { resolveDataDir } from "./dataPaths"; import { getObsidianVaultPath, setObsidianVaultPath, @@ -35,6 +36,40 @@ export async function getObsidianSyncStatus(): Promise { return { vaultPath, webdavEnabled, webdavUsername, webdavPassword }; } +/** Canonical (symlink-resolved) form of a path; falls back to the lexical resolve. */ +function canonicalPath(target: string): string { + try { + return fs.realpathSync.native(target); + } catch { + return path.resolve(target); + } +} + +/** True when `child` is `parent` itself or lives anywhere below it. */ +function isSameOrInside(parent: string, child: string): boolean { + const rel = path.relative(parent, child); + if (rel === "") return true; + if (path.isAbsolute(rel)) return false; // different drive (win32) + return rel !== ".." && !rel.startsWith(`..${path.sep}`); +} + +/** + * GHSA-7pq4-8pvv-rx7r: the WebDAV file service (scripts/dev/webdav-handler.mjs) + * serves the vault root to anyone holding the Basic credentials, before Next.js + * and outside the authz pipeline. A vault that IS the data directory, sits + * inside it, or CONTAINS it turns that service into a reader for server.env + * (JWT_SECRET / STORAGE_ENCRYPTION_KEY / API_KEY_SECRET) and storage.sqlite. + * Both sides are realpath-resolved so a symlink cannot dodge the comparison. + */ +export function vaultPathOverlapsDataDir(resolvedVaultPath: string): boolean { + const vault = canonicalPath(resolvedVaultPath); + const dataDir = canonicalPath(resolveDataDir()); + return isSameOrInside(dataDir, vault) || isSameOrInside(vault, dataDir); +} + +export const VAULT_OVERLAPS_DATA_DIR_ERROR = + "Vault path must not be the OmniRoute data directory, a directory inside it, or a directory that contains it"; + export async function enableObsidianVaultSync( vaultPath: string ): Promise { @@ -49,6 +84,10 @@ export async function enableObsidianVaultSync( return { success: false, error: `Path is not a directory: ${resolvedPath}` }; } + if (vaultPathOverlapsDataDir(resolvedPath)) { + return { success: false, error: VAULT_OVERLAPS_DATA_DIR_ERROR }; + } + try { setObsidianVaultPath(resolvedPath); diff --git a/src/server/authz/policies/management.ts b/src/server/authz/policies/management.ts index 38613f2304..ee30ae4c78 100644 --- a/src/server/authz/policies/management.ts +++ b/src/server/authz/policies/management.ts @@ -251,7 +251,17 @@ export const managementPolicy: RoutePolicy = { } // Tier 2: always-protected routes skip the requireLogin=false bypass. - if (!isAlwaysProtectedPath(path) && !(await isAuthRequired(ctx.request))) { + // + // The fresh-install bootstrap branch inside isAuthRequired() is loopback-only. + // Hand it the SAME trusted verdict the LOCAL_ONLY gate above used (token-stamped + // real TCP peer via peerContext) instead of letting it sniff ctx.request — the + // ORIGINAL request still carries every client-supplied header at this point, + // and the Host header was how a remote caller reached the anonymous + // POST /api/settings/require-login write (GHSA-7pq4-8pvv-rx7r). + if ( + !isAlwaysProtectedPath(path) && + !(await isAuthRequired(ctx.request, { loopback: isLoopbackRequest(ctx) })) + ) { return allow({ kind: "anonymous", id: "anonymous", label: "auth-disabled" }); } diff --git a/src/server/authz/routeGuard.ts b/src/server/authz/routeGuard.ts index a2b6a3571b..340f302910 100644 --- a/src/server/authz/routeGuard.ts +++ b/src/server/authz/routeGuard.ts @@ -177,6 +177,17 @@ export const ALWAYS_PROTECTED_API_PATHS: ReadonlyArray = [ // as the {claude,codex}-auth/apply-local pattern below; a plain path because // it carries no dynamic segment. "/api/providers/agy-auth/apply-local", + // Obsidian integration. POST /webdav points the WebDAV file service — served by + // the custom Node layer BEFORE Next.js, outside this pipeline — at a + // caller-chosen root and echoes freshly minted, reusable Basic credentials; + // DELETE /webdav rotates/clears them; the parent POST stores the Obsidian REST + // API token. GHSA-62vw only masked the GET password reveal, leaving credential + // *issuance* on the fail-open tier: with requireLogin flipped off during the + // bootstrap window, an anonymous caller stood up a file server over DATA_DIR + // and read JWT_SECRET out of server.env (GHSA-7pq4-8pvv-rx7r). Prefix covers + // the /webdav child. ALWAYS_PROTECTED rather than LOCAL_ONLY so an operator + // driving the dashboard through a tunnel keeps the feature. + "/api/settings/obsidian", ]; /** diff --git a/src/shared/utils/apiAuth.ts b/src/shared/utils/apiAuth.ts index 3c7afe96eb..ea5c8938df 100644 --- a/src/shared/utils/apiAuth.ts +++ b/src/shared/utils/apiAuth.ts @@ -9,6 +9,13 @@ import { cookies } from "next/headers"; import { getSettings } from "@/lib/db/settings"; +import { + AUTHZ_HEADER_PEER_LOCALITY, + PEER_IP_HEADER, + VIA_PROXY_HEADER, +} from "@/server/authz/headers"; +import { classifyStampedPeerLocality } from "@/server/authz/peerStamp"; +import { classifyHostLocality } from "@/server/authz/routeGuard"; import { isPublicApiRoute } from "@/shared/constants/publicApiRoutes"; import { verifyDashboardSessionToken } from "@/shared/utils/dashboardSessionToken"; import { extractApiKey } from "@/sse/services/auth"; @@ -21,9 +28,21 @@ type RequestLike = { method?: string; nextUrl?: { hostname?: string | null; pathname?: string | null } | null; url?: string; + /** Real socket peer, present only for direct Node / test callers (never in the proxy runtime). */ + ip?: string; + socket?: { remoteAddress?: string }; }; -const LOOPBACK_HOSTNAMES = new Set(["localhost", "::1"]); +export interface AuthRequiredOptions { + /** + * Pre-resolved trusted peer verdict. The authz policy layer already resolves + * locality from the token-stamped real TCP peer (`peerContext.isLoopbackRequest`) + * and hands it down, so the bootstrap gate never re-derives it from headers on + * the ORIGINAL (pre-strip) request — where a client-supplied copy of the + * pipeline's own locality header could still be present. + */ + loopback?: boolean; +} function hasConfiguredPassword(settings: Record): boolean { return typeof settings.password === "string" && settings.password.length > 0; @@ -86,54 +105,116 @@ function getRequestMethod(request: RequestLike | Request | null | undefined): st return "GET"; } -function getRequestHostname(request: RequestLike | Request | null | undefined): string | null { - const nextHostname = - request && - typeof request === "object" && - "nextUrl" in request && - request.nextUrl && - typeof request.nextUrl.hostname === "string" - ? request.nextUrl.hostname - : null; - - if (nextHostname) return nextHostname; - - const rawUrl = - request && typeof request === "object" && "url" in request && typeof request.url === "string" - ? request.url - : ""; - - if (rawUrl) { - try { - return new URL(rawUrl, "http://localhost").hostname; - } catch { - // Fall through to Host header parsing. - } - } - +function getHeaderValue( + request: RequestLike | Request | null | undefined, + name: string +): string | null { const requestHeaders = request && typeof request === "object" && "headers" in request ? request.headers : undefined; - const host = requestHeaders?.get("host") || requestHeaders?.get("Host") || null; - if (!host) return null; - - try { - return new URL(`http://${host}`).hostname; - } catch { - return host.split(":")[0] || null; - } + return requestHeaders?.get?.(name) ?? null; } -export function isLoopbackRequest(request: RequestLike | Request | null | undefined): boolean { - const hostname = getRequestHostname(request); - if (!hostname) return false; +function getSocketPeerAddress(request: RequestLike | Request | null | undefined): string | null { + if (!request || typeof request !== "object") return null; + const candidate = request as RequestLike; + if (typeof candidate.ip === "string" && candidate.ip) return candidate.ip; + const remoteAddress = candidate.socket?.remoteAddress; + return typeof remoteAddress === "string" && remoteAddress ? remoteAddress : null; +} +/** + * Trusted peer locality for the fresh-install bootstrap gate. + * + * NEVER derived from `Host` / `nextUrl.hostname` / the request URL — all three + * are client-controlled, so a remote caller sending `Host: localhost` used to be + * treated as the local operator (GHSA-7pq4-8pvv-rx7r). The verdict comes from + * the same primitives the authz pipeline already trusts, in this order: + * + * 1. The token-stamped real TCP peer (`PEER_IP_HEADER` + `VIA_PROXY_HEADER`, + * written by the custom Node server from `req.socket.remoteAddress` after + * deleting any client-supplied value, validated against + * OMNIROUTE_PEER_STAMP_TOKEN). This is what the policy layer sees on the + * ORIGINAL request. A stamp present but failing validation → not loopback. + * A loopback socket flagged as a reverse-proxy hop → not loopback. + * 2. The pipeline's own locality verdict (`AUTHZ_HEADER_PEER_LOCALITY`), which + * route handlers see after `runAuthzPipeline` stripped every client-supplied + * copy and re-stamped it from (1). Trusted only while the per-process stamp + * token exists — i.e. a stamping server is actually in front of Next, in + * which case every request that reached the policy carried (1) and this + * branch can only be the post-strip route-handler view. + * 3. A real socket peer (`request.ip` / `request.socket.remoteAddress`) for + * direct Node / unit-test callers that never went through the pipeline. + * The proxy runtime exposes neither, so nothing here is client-reachable. + * + * Anything else → not loopback (fail closed), matching + * `src/server/authz/peerContext.ts::isLoopbackRequest`. + */ +export function isLoopbackRequest(request: RequestLike | Request | null | undefined): boolean { + if (!request || typeof request !== "object") return false; + + const stampToken = process.env.OMNIROUTE_PEER_STAMP_TOKEN; + + const stampedPeer = getHeaderValue(request, PEER_IP_HEADER); + if (stampedPeer !== null) { + return ( + classifyStampedPeerLocality( + stampedPeer, + getHeaderValue(request, VIA_PROXY_HEADER), + stampToken + ) === "loopback" + ); + } + + const pipelineVerdict = getHeaderValue(request, AUTHZ_HEADER_PEER_LOCALITY); + if (pipelineVerdict !== null && stampToken) { + return pipelineVerdict === "loopback"; + } + + const socketPeer = getSocketPeerAddress(request); + if (socketPeer) return classifyHostLocality(socketPeer) === "loopback"; + + // A stamping server is in front (every supported runtime — run-next dev/start and + // standalone-server-ws for Docker, the npm CLI and Electron — calls + // ensurePeerStampToken() at boot) but neither trusted signal is on this request: + // fail closed. The Host header is never consulted in that process. + if (stampToken) return false; + + // No stamping server in this process at all: route handlers invoked directly (the + // unit-test harness) or a raw `next` launch that also bypasses every LOCAL_ONLY + // gate in peerContext. There is no real peer to read, so keep the historical + // URL/Host verdict rather than turning every direct handler call into a remote one. + return isLegacyHostLoopback(request); +} + +function isLegacyHostLoopback(request: RequestLike | Request): boolean { + let hostname: string | null = null; + const candidate = request as RequestLike; + if (candidate.nextUrl && typeof candidate.nextUrl.hostname === "string") { + hostname = candidate.nextUrl.hostname; + } else if (typeof candidate.url === "string" && candidate.url) { + try { + hostname = new URL(candidate.url, "http://localhost").hostname; + } catch { + hostname = null; + } + } + if (!hostname) { + const host = getHeaderValue(request, "host"); + if (!host) return false; + try { + hostname = new URL(`http://${host}`).hostname; + } catch { + hostname = host.split(":")[0] || null; + } + } + if (!hostname) return false; const normalized = hostname .trim() .toLowerCase() .replace(/^\[(.*)\]$/, "$1"); - if (LOOPBACK_HOSTNAMES.has(normalized)) return true; - if (/^127(?:\.\d{1,3}){3}$/.test(normalized)) return true; - return false; + return ( + normalized === "localhost" || normalized === "::1" || /^127(?:\.\d{1,3}){3}$/.test(normalized) + ); } function getCookieValueFromHeader(headers: Headers | undefined, name: string): string | null { @@ -318,9 +399,13 @@ export function isPublicRoute(pathname: string, method = "GET"): boolean { * If requireLogin is explicitly false, auth is skipped. Fresh installs without * a password keep their unauthenticated bootstrap path only on loopback * requests; exposed network requests must configure INITIAL_PASSWORD or log in. + * + * "Loopback" is the trusted peer verdict (`isLoopbackRequest` above, or the + * policy-supplied `options.loopback`), never the Host header. */ export async function isAuthRequired( - request?: RequestLike | Request | null | undefined + request?: RequestLike | Request | null | undefined, + options?: AuthRequiredOptions ): Promise { try { const settings = await getSettings(); @@ -343,11 +428,19 @@ export async function isAuthRequired( return false; } + const loopback = options?.loopback ?? isLoopbackRequest(request); + + // The first-password write is the switch that disarms every other guard + // (requireLogin=false makes isAuthenticated() true everywhere), so it is + // the one bootstrap path that MUST honour the loopback constraint — it + // used to be an unconditional `return false`, open to any network peer + // during the window (GHSA-7pq4-8pvv-rx7r). It stays open for the local + // operator even after onboarding completed without a password. if (isRequireLoginBootstrapWritePath(pathname, method)) { - return false; + return !loopback; } - return settings.setupComplete === true || !isLoopbackRequest(request); + return settings.setupComplete === true || !loopback; } return true; diff --git a/tests/unit/api-auth.test.ts b/tests/unit/api-auth.test.ts index 9784a688a1..017d9924ad 100644 --- a/tests/unit/api-auth.test.ts +++ b/tests/unit/api-auth.test.ts @@ -17,10 +17,16 @@ const apiAuth = await import("../../src/shared/utils/apiAuth.ts"); const { requireManagementAuth } = await import("../../src/lib/api/requireManagementAuth.ts"); const { getLegacyCliTokenSync, getMachineTokenSync } = await import("../../src/lib/machineToken.ts"); -const { CLI_TOKEN_HEADER } = await import("../../src/server/authz/headers.ts"); +const { AUTHZ_HEADER_PEER_LOCALITY, CLI_TOKEN_HEADER, PEER_IP_HEADER, VIA_PROXY_HEADER } = + await import("../../src/server/authz/headers.ts"); const ORIGINAL_JWT_SECRET = process.env.JWT_SECRET; const ORIGINAL_INITIAL_PASSWORD = process.env.INITIAL_PASSWORD; +const ORIGINAL_PEER_STAMP_TOKEN = process.env.OMNIROUTE_PEER_STAMP_TOKEN; + +// The per-process secret the custom Node server uses to stamp the real TCP peer +// (scripts/dev/peer-stamp.mjs). Tests mint the same `|` shape. +const TEST_PEER_STAMP_TOKEN = "api-auth-test-peer-stamp-token"; async function resetStorage() { core.resetDbInstance(); @@ -29,6 +35,25 @@ async function resetStorage() { fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); delete process.env.JWT_SECRET; delete process.env.INITIAL_PASSWORD; + delete process.env.OMNIROUTE_PEER_STAMP_TOKEN; +} + +/** + * A request as the authz policy sees it: the custom server already stamped the + * real TCP peer into PEER_IP_HEADER (token-validated), so the verdict cannot be + * influenced by the URL / Host header the client chose. + */ +function stampedPeerRequest(url: string, peerIp: string, init: RequestInit = {}): Request { + process.env.OMNIROUTE_PEER_STAMP_TOKEN = TEST_PEER_STAMP_TOKEN; + const headers = new Headers(init.headers); + headers.set(PEER_IP_HEADER, `${TEST_PEER_STAMP_TOKEN}|${peerIp}`); + headers.set(VIA_PROXY_HEADER, `${TEST_PEER_STAMP_TOKEN}|0`); + return new Request(url, { ...init, headers }); +} + +/** A direct Node / non-pipeline caller carrying a real socket peer. */ +function socketPeerRequest(url: string, peerIp: string, init: RequestInit = {}): Request { + return Object.assign(new Request(url, init), { ip: peerIp }) as Request; } function makeCookieRequest(token: string) { @@ -62,6 +87,12 @@ test.after(() => { } else { process.env.INITIAL_PASSWORD = ORIGINAL_INITIAL_PASSWORD; } + + if (ORIGINAL_PEER_STAMP_TOKEN === undefined) { + delete process.env.OMNIROUTE_PEER_STAMP_TOKEN; + } else { + process.env.OMNIROUTE_PEER_STAMP_TOKEN = ORIGINAL_PEER_STAMP_TOKEN; + } }); test("isPublicRoute recognizes allowed API prefixes", () => { @@ -311,12 +342,212 @@ test("isAuthRequired is disabled while no password exists", async () => { test("isAuthRequired keeps fresh bootstrap open only on loopback", async () => { await localDb.updateSettings({ requireLogin: true, password: "" }); - assert.equal(await apiAuth.isAuthRequired(new Request("http://localhost/api/providers")), false); - assert.equal(await apiAuth.isAuthRequired(new Request("http://127.0.0.1/api/providers")), false); + // Loopback is decided from the trusted peer (token-stamped real TCP peer or a + // real socket), never from the URL / Host header (GHSA-7pq4-8pvv-rx7r). + assert.equal( + await apiAuth.isAuthRequired(stampedPeerRequest("http://localhost/api/providers", "127.0.0.1")), + false + ); + assert.equal( + await apiAuth.isAuthRequired(stampedPeerRequest("http://127.0.0.1/api/providers", "::1")), + false + ); + assert.equal( + await apiAuth.isAuthRequired(socketPeerRequest("http://localhost/api/providers", "127.0.0.1")), + false + ); assert.equal( await apiAuth.isAuthRequired(new Request("https://example.com/api/providers")), true ); + assert.equal( + await apiAuth.isAuthRequired( + stampedPeerRequest("https://example.com/api/providers", "203.0.113.9") + ), + true + ); +}); + +// ── GHSA-7pq4-8pvv-rx7r — the bootstrap gate must not trust Host / nextUrl ───── + +test("isLoopbackRequest ignores a spoofed Host header — a non-loopback stamped peer is never loopback (GHSA-7pq4-8pvv-rx7r)", async () => { + // Remote attacker sending `Host: localhost` (the URL's hostname is exactly what + // nextUrl.hostname / the Host header carry). The custom server stamped the real + // peer as 203.0.113.9 → NOT loopback, whatever the client put in Host. + const spoofed = stampedPeerRequest("http://localhost/api/providers", "203.0.113.9", { + headers: { host: "localhost" }, + }); + assert.equal(apiAuth.isLoopbackRequest(spoofed), false); + + // A Host-only "localhost" with no trusted peer signal at all is not loopback either. + assert.equal( + apiAuth.isLoopbackRequest(new Request("http://localhost/api/providers")), + false, + "Host / nextUrl.hostname alone must never make a request loopback" + ); + assert.equal( + apiAuth.isLoopbackRequest( + new Request("http://127.0.0.1/api/providers", { headers: { host: "127.0.0.1" } }) + ), + false + ); + + // The forged stamp shape (`|127.0.0.1`) fails closed. + process.env.OMNIROUTE_PEER_STAMP_TOKEN = TEST_PEER_STAMP_TOKEN; + assert.equal( + apiAuth.isLoopbackRequest( + new Request("http://localhost/api/providers", { + headers: { [PEER_IP_HEADER]: "not-the-process-token|127.0.0.1" }, + }) + ), + false + ); + + // The genuine stamp for a loopback peer IS loopback — but not when the custom + // server also flagged that the request arrived through a reverse-proxy hop. + assert.equal( + apiAuth.isLoopbackRequest(stampedPeerRequest("https://example.com/api/providers", "127.0.0.1")), + true + ); + assert.equal( + apiAuth.isLoopbackRequest( + new Request("http://localhost/api/providers", { + headers: { + [PEER_IP_HEADER]: `${TEST_PEER_STAMP_TOKEN}|127.0.0.1`, + [VIA_PROXY_HEADER]: `${TEST_PEER_STAMP_TOKEN}|1`, + }, + }) + ), + false + ); +}); + +test("isLoopbackRequest consults Host only when no stamping server exists in the process (GHSA-7pq4-8pvv-rx7r)", async () => { + // Every supported runtime calls ensurePeerStampToken() at boot, so once a token + // exists a signal-less request is never loopback, whatever Host says. + process.env.OMNIROUTE_PEER_STAMP_TOKEN = TEST_PEER_STAMP_TOKEN; + assert.equal( + apiAuth.isLoopbackRequest( + new Request("http://localhost/api/providers", { headers: { host: "localhost" } }) + ), + false, + "with a stamping server in front, Host must never make a request loopback" + ); + + // No token at all = no stamping server = direct handler invocation (the unit-test + // harness). There is no real peer to read, so the historical URL verdict applies — + // and it still rejects a non-loopback hostname. + delete process.env.OMNIROUTE_PEER_STAMP_TOKEN; + assert.equal(apiAuth.isLoopbackRequest(new Request("http://localhost/api/providers")), true); + assert.equal(apiAuth.isLoopbackRequest(new Request("https://example.com/api/providers")), false); +}); + +test("isLoopbackRequest trusts the pipeline locality verdict only when a stamping server is in front (GHSA-7pq4-8pvv-rx7r)", async () => { + // Route handlers see AUTHZ_HEADER_PEER_LOCALITY, re-stamped by the pipeline + // after every client-supplied copy was stripped — trustworthy only when the + // per-process stamp token exists (i.e. the custom server is actually stamping). + const verdict = new Request("https://example.com/api/providers", { + headers: { [AUTHZ_HEADER_PEER_LOCALITY]: "loopback" }, + }); + delete process.env.OMNIROUTE_PEER_STAMP_TOKEN; + assert.equal(apiAuth.isLoopbackRequest(verdict), false); + + process.env.OMNIROUTE_PEER_STAMP_TOKEN = TEST_PEER_STAMP_TOKEN; + assert.equal(apiAuth.isLoopbackRequest(verdict), true); + assert.equal( + apiAuth.isLoopbackRequest( + new Request("http://localhost/api/providers", { + headers: { [AUTHZ_HEADER_PEER_LOCALITY]: "remote" }, + }) + ), + false + ); + + // A forged locality header never outranks the real stamped peer. + assert.equal( + apiAuth.isLoopbackRequest( + stampedPeerRequest("http://localhost/api/providers", "203.0.113.9", { + headers: { [AUTHZ_HEADER_PEER_LOCALITY]: "loopback" }, + }) + ), + false + ); +}); + +test("isAuthRequired gates the bootstrap require-login write on the trusted peer (GHSA-7pq4-8pvv-rx7r)", async () => { + await localDb.updateSettings({ requireLogin: true, password: "" }); + + // The write that disarms every other guard (requireLogin=false) used to be an + // unconditional `return false` — open to any network peer in the window. + assert.equal( + await apiAuth.isAuthRequired( + new Request("https://example.com/api/settings/require-login", { method: "POST" }) + ), + true, + "remote POST /api/settings/require-login must require auth in the bootstrap window" + ); + assert.equal( + await apiAuth.isAuthRequired( + stampedPeerRequest("http://localhost/api/settings/require-login", "203.0.113.9", { + method: "POST", + headers: { host: "localhost" }, + }) + ), + true, + "Host: localhost from a non-loopback stamped peer must not reopen the write path" + ); + assert.equal( + await apiAuth.isAuthenticated( + new Request("https://example.com/api/settings/require-login", { method: "POST" }) + ), + false + ); + + // The genuine local operator keeps the first-password flow — including after + // onboarding completed without a password (setupComplete: true). + assert.equal( + await apiAuth.isAuthRequired( + stampedPeerRequest("http://localhost/api/settings/require-login", "127.0.0.1", { + method: "POST", + }) + ), + false + ); + await localDb.updateSettings({ requireLogin: true, password: "", setupComplete: true }); + assert.equal( + await apiAuth.isAuthRequired( + stampedPeerRequest("http://localhost/api/settings/require-login", "127.0.0.1", { + method: "POST", + }) + ), + false + ); + assert.equal( + await apiAuth.isAuthRequired( + new Request("https://example.com/api/settings/require-login", { method: "POST" }) + ), + true + ); +}); + +test("isAuthRequired honours an explicit trusted loopback verdict from the policy layer", async () => { + await localDb.updateSettings({ requireLogin: true, password: "" }); + + // The authz policy resolves locality itself (peerContext) and hands the + // verdict down, so the bootstrap gate never re-reads the ORIGINAL request's + // client-controlled headers. + const forged = new Request("http://localhost/api/settings/require-login", { + method: "POST", + headers: { host: "localhost", [AUTHZ_HEADER_PEER_LOCALITY]: "loopback" }, + }); + assert.equal(await apiAuth.isAuthRequired(forged, { loopback: false }), true); + assert.equal(await apiAuth.isAuthRequired(forged, { loopback: true }), false); + assert.equal( + await apiAuth.isAuthRequired(new Request("https://example.com/api/providers"), { + loopback: true, + }), + false + ); }); test("isAuthenticated rejects remote management bootstrap without a configured password", async () => { @@ -368,8 +599,11 @@ test("isAuthRequired treats partial OIDC config as not configured (bootstrap beh // missing clientId + clientSecret }); - // On loopback without full config → bootstrap allowed - assert.equal(await apiAuth.isAuthRequired(new Request("http://localhost/api/providers")), false); + // On loopback (trusted stamped peer) without full config → bootstrap allowed + assert.equal( + await apiAuth.isAuthRequired(stampedPeerRequest("http://localhost/api/providers", "127.0.0.1")), + false + ); // Remote still requires auth assert.equal( await apiAuth.isAuthRequired(new Request("https://example.com/api/providers")), diff --git a/tests/unit/authz/credential-export-always-protected.test.ts b/tests/unit/authz/credential-export-always-protected.test.ts index c297bad003..5ab4a802b9 100644 --- a/tests/unit/authz/credential-export-always-protected.test.ts +++ b/tests/unit/authz/credential-export-always-protected.test.ts @@ -58,6 +58,20 @@ const HARD_GATED_INVENTORY: ReadonlyArray<{ path: string; why: string }> = [ path: "/api/providers/agy-auth/apply-local", why: "writes into ~/.gemini/antigravity-cli/antigravity-oauth-token", }, + // ── Reported in GHSA-7pq4-8pvv-rx7r (JWT_SECRET bootstrap chain) ───────── + // POST points the Obsidian WebDAV file service — served by the custom Node + // layer BEFORE Next.js, so the authz pipeline never runs for it — at an + // attacker-chosen root and echoes freshly minted Basic credentials; DELETE + // rotates/clears them. GHSA-62vw only masked the GET reveal; the credential + // *issuance* was still on the fail-open tier. + { + path: "/api/settings/obsidian/webdav", + why: "POST returns reusable WebDAV Basic credentials for a caller-chosen root; DELETE rotates them (GHSA-7pq4-8pvv-rx7r)", + }, + { + path: "/api/settings/obsidian", + why: "POST stores the Obsidian Local REST API token; same credential surface as its /webdav child (GHSA-7pq4-8pvv-rx7r)", + }, // ── Already fixed; pinned so a refactor cannot silently drop them ──────── { path: "/api/db-backups/export", why: "GHSA-mghq-58h3-qcqj" }, { path: "/api/db-backups/exportAll", why: "GHSA-mghq-58h3-qcqj" }, @@ -126,7 +140,20 @@ test("a connection id cannot escape the pattern with a slash", () => { }); test("the plain-path allowlist keeps its existing entries", () => { - for (const p of ["/api/shutdown", "/api/settings/database", "/api/db-backups"]) { + for (const p of [ + "/api/shutdown", + "/api/settings/database", + "/api/db-backups", + "/api/settings/obsidian", + ]) { assert.ok(ALWAYS_PROTECTED_API_PATHS.includes(p), p); } }); + +test("the obsidian entry does not over-protect its /api/settings neighbours", () => { + // `/api/settings/obsidian` is a plain prefix; the sibling settings routes must + // stay on the MANAGEMENT tier for keyless local-first installs. + for (const path of ["/api/settings", "/api/settings/notion", "/api/settings/require-login"]) { + assert.equal(isAlwaysProtectedPath(path), false, path); + } +}); diff --git a/tests/unit/authz/management-policy.test.ts b/tests/unit/authz/management-policy.test.ts index 6e8feda318..aedd82d81d 100644 --- a/tests/unit/authz/management-policy.test.ts +++ b/tests/unit/authz/management-policy.test.ts @@ -112,7 +112,12 @@ function remoteCtx(headers: Headers, method = "GET", path = "/api/keys") { test("managementPolicy: allows when auth not required (no password set)", async () => { await settingsDb.updateSettings({ requireLogin: true, password: null }); const policy = await loadPolicy(); - const out = await policy.evaluate(ctx(new Headers())); + // Fresh-bootstrap anonymous allow is loopback-only, and loopback is decided + // from the real peer (socket.remoteAddress / stamped peer), never from the + // `http://localhost` URL the ctx() helper carries (GHSA-7pq4-8pvv-rx7r). + const out = await policy.evaluate( + ctx(new Headers(), "GET", "/api/keys", { socket: { remoteAddress: "127.0.0.1" } }) + ); assert.equal(out.allow, true); if (out.allow) { assert.equal(out.subject.kind, "anonymous"); @@ -133,6 +138,94 @@ test("managementPolicy: rejects remote fresh bootstrap without a password", asyn } }); +// ─── GHSA-7pq4-8pvv-rx7r — bootstrap first-password write is loopback-only ──── +// +// `POST /api/settings/require-login` in the bootstrap window used to be an +// unconditional anonymous allow (apiAuth.isAuthRequired returned false before +// the loopback check), and the loopback check itself read the client-controlled +// Host header. A remote caller could flip requireLogin=false, then read +// JWT_SECRET through the Obsidian WebDAV file service and forge a durable admin +// session. The policy must decide from the token-stamped real peer. + +const BOOTSTRAP_WRITE_PATH = "/api/settings/require-login"; +const POLICY_STAMP_TOKEN = "mgmt-policy-test-peer-stamp-token"; + +function stampedHeaders(peerIp: string, extra: Record = {}): Headers { + process.env.OMNIROUTE_PEER_STAMP_TOKEN = POLICY_STAMP_TOKEN; + return new Headers({ + ...extra, + "x-omniroute-peer-ip": `${POLICY_STAMP_TOKEN}|${peerIp}`, + "x-omniroute-via-proxy": `${POLICY_STAMP_TOKEN}|0`, + }); +} + +test("managementPolicy: rejects an anonymous remote POST /api/settings/require-login in the bootstrap window (GHSA-7pq4-8pvv-rx7r)", async () => { + await settingsDb.updateSettings({ requireLogin: true, password: null }); + const policy = await loadPolicy(); + try { + // Plain remote peer, no stamp at all → fail closed. + const unstamped = await policy.evaluate(remoteCtx(new Headers(), "POST", BOOTSTRAP_WRITE_PATH)); + assert.equal(unstamped.allow, false); + if (!unstamped.allow) { + assert.equal(unstamped.status, 401); + assert.equal(unstamped.code, "AUTH_001"); + } + + // Host-spoof: the URL / Host header say localhost, the stamped real peer is + // a public address, and the client even forged the pipeline's locality + // verdict header. None of that is loopback. + const spoofed = await policy.evaluate( + ctx( + stampedHeaders("203.0.113.9", { + host: "localhost:20128", + "x-omniroute-peer-locality": "loopback", + }), + "POST", + BOOTSTRAP_WRITE_PATH + ) + ); + assert.equal(spoofed.allow, false); + if (!spoofed.allow) { + assert.equal(spoofed.status, 401); + assert.equal(spoofed.code, "AUTH_001"); + } + } finally { + delete process.env.OMNIROUTE_PEER_STAMP_TOKEN; + } +}); + +test("managementPolicy: keeps the bootstrap first-password write open for the stamped loopback peer (GHSA-7pq4-8pvv-rx7r)", async () => { + await settingsDb.updateSettings({ requireLogin: true, password: null, setupComplete: true }); + const policy = await loadPolicy(); + try { + const local = await policy.evaluate( + ctx(stampedHeaders("127.0.0.1"), "POST", BOOTSTRAP_WRITE_PATH) + ); + assert.equal(local.allow, true); + if (local.allow) { + assert.equal(local.subject.kind, "anonymous"); + assert.equal(local.subject.label, "auth-disabled"); + } + + // A loopback socket that is really a reverse-proxy hop (via-proxy marker + // set by the custom server) is NOT the local operator. + process.env.OMNIROUTE_PEER_STAMP_TOKEN = POLICY_STAMP_TOKEN; + const viaProxy = await policy.evaluate( + ctx( + new Headers({ + "x-omniroute-peer-ip": `${POLICY_STAMP_TOKEN}|127.0.0.1`, + "x-omniroute-via-proxy": `${POLICY_STAMP_TOKEN}|1`, + }), + "POST", + BOOTSTRAP_WRITE_PATH + ) + ); + assert.equal(viaProxy.allow, false); + } finally { + delete process.env.OMNIROUTE_PEER_STAMP_TOKEN; + } +}); + test("managementPolicy: rejects 401 when auth required and no credentials", async () => { process.env.JWT_SECRET = "test-jwt-secret-for-mgmt-policy"; process.env.INITIAL_PASSWORD = "initial-pass"; diff --git a/tests/unit/authz/pipeline.test.ts b/tests/unit/authz/pipeline.test.ts index d89b152219..88cbc50680 100644 --- a/tests/unit/authz/pipeline.test.ts +++ b/tests/unit/authz/pipeline.test.ts @@ -200,7 +200,7 @@ test("runAuthzPipeline allows onboarding when login is required but no password assert.equal(response.headers.get("x-omniroute-route-class"), "PUBLIC"); }); -test("runAuthzPipeline allows first password writes when login is required but no password exists", async () => { +test("runAuthzPipeline allows first password writes when login is required but no password exists — from the stamped loopback peer only (GHSA-7pq4-8pvv-rx7r)", async () => { delete process.env.INITIAL_PASSWORD; await settingsDb.updateSettings({ requireLogin: true, @@ -208,13 +208,47 @@ test("runAuthzPipeline allows first password writes when login is required but n password: "", }); - const response = await pipeline.runAuthzPipeline( + // The local operator (real TCP peer 127.0.0.1, stamped by the custom server) + // keeps the first-password flow, whatever hostname they typed. + process.env.OMNIROUTE_PEER_STAMP_TOKEN = "pipeline-test-peer-stamp-token"; + const local = await pipeline.runAuthzPipeline( + request("https://example.com/api/settings/require-login", { + method: "POST", + headers: { + "x-omniroute-peer-ip": "pipeline-test-peer-stamp-token|127.0.0.1", + "x-omniroute-via-proxy": "pipeline-test-peer-stamp-token|0", + }, + }), + { enforce: true } + ); + assert.equal(local.status, 200); + assert.equal(local.headers.get("x-omniroute-route-class"), "MANAGEMENT"); + + // A remote peer — even one spelling the URL as localhost and forging the + // pipeline's own locality verdict header — must not reach the anonymous write + // that flips requireLogin=false (the first link of the JWT_SECRET chain). + const spoofed = await pipeline.runAuthzPipeline( + request("http://localhost/api/settings/require-login", { + method: "POST", + headers: { + host: "localhost", + "x-omniroute-peer-locality": "loopback", + "x-omniroute-peer-ip": "pipeline-test-peer-stamp-token|203.0.113.9", + "x-omniroute-via-proxy": "pipeline-test-peer-stamp-token|0", + }, + }), + { enforce: true } + ); + assert.equal(spoofed.status, 401); + assert.equal((await spoofed.json()).error.code, "AUTH_001"); + + // No stamp at all (nothing trustworthy about the peer) → fail closed. + delete process.env.OMNIROUTE_PEER_STAMP_TOKEN; + const unstamped = await pipeline.runAuthzPipeline( request("https://example.com/api/settings/require-login", { method: "POST" }), { enforce: true } ); - - assert.equal(response.status, 200); - assert.equal(response.headers.get("x-omniroute-route-class"), "MANAGEMENT"); + assert.equal(unstamped.status, 401); }); test("runAuthzPipeline keeps management API rejections as JSON", async () => { diff --git a/tests/unit/obsidian-webdav-route.test.ts b/tests/unit/obsidian-webdav-route.test.ts index a92d06d332..fd03d79da6 100644 --- a/tests/unit/obsidian-webdav-route.test.ts +++ b/tests/unit/obsidian-webdav-route.test.ts @@ -42,7 +42,26 @@ async function resetStorage() { } function makeRequest(url: string, options?: RequestInit): NextRequest { - return new Request(url, options) as unknown as NextRequest; + // These tests exercise the handler in the fresh-install open mode (no password + // configured, requireLogin default). That mode is loopback-only, and loopback is + // decided from the real peer — never from the `http://localhost` URL + // (GHSA-7pq4-8pvv-rx7r) — so give the direct-call Request a loopback socket peer. + return Object.assign(new Request(url, options), { ip: "127.0.0.1" }) as unknown as NextRequest; +} + +function postVault(vaultPath: string): Promise { + return route.POST( + makeRequest("http://localhost/api/settings/obsidian/webdav", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ vaultPath }), + }) + ); +} + +async function errorMessageOf(res: Response): Promise { + const body = (await res.json()) as Record; + return (body.error as Record | undefined)?.message as string | undefined; } test.beforeEach(async () => { @@ -191,6 +210,71 @@ test("POST with a non-existent path → 400, body does NOT contain a stack trace ); }); +// ── GHSA-7pq4-8pvv-rx7r — the vault root must never expose the data directory ── +// +// The WebDAV file service (scripts/dev/webdav-handler.mjs) serves the vault root +// to anyone holding the Basic credentials, before Next.js and outside the authz +// pipeline. Pointing it at DATA_DIR (or a parent of it) hands out server.env — +// JWT_SECRET / STORAGE_ENCRYPTION_KEY / API_KEY_SECRET — and storage.sqlite. + +test("POST rejects a vaultPath that IS the data directory → 400 (GHSA-7pq4-8pvv-rx7r)", async () => { + const res = await postVault(TEST_DATA_DIR); + assert.equal(res.status, 400); + const msg = await errorMessageOf(res); + assert.ok(msg && /data directory/i.test(msg), `expected a data-directory refusal, got: ${msg}`); + assert.ok(!msg.includes("at /"), "error must not carry a stack trace"); + assert.ok(!msg.includes(TEST_DATA_DIR), "error must not echo the data directory location"); + assert.equal(obsidianDb.getWebdavEnabled(), false, "WebDAV must stay disabled"); + assert.equal(obsidianDb.getObsidianVaultPath(), null, "vault path must not be stored"); +}); + +test("POST rejects a vaultPath that CONTAINS the data directory (parent dir) → 400 (GHSA-7pq4-8pvv-rx7r)", async () => { + const res = await postVault(path.dirname(TEST_DATA_DIR)); + assert.equal(res.status, 400); + const msg = await errorMessageOf(res); + assert.ok(msg && /data directory/i.test(msg), `expected a data-directory refusal, got: ${msg}`); + assert.equal(obsidianDb.getWebdavEnabled(), false); +}); + +test("POST rejects a vaultPath INSIDE the data directory → 400 (GHSA-7pq4-8pvv-rx7r)", async () => { + const inside = path.join(TEST_DATA_DIR, "db_backups"); + fs.mkdirSync(inside, { recursive: true }); + const res = await postVault(inside); + assert.equal(res.status, 400); + assert.equal(obsidianDb.getWebdavEnabled(), false); +}); + +test("POST rejects a symlink that resolves to the data directory → 400 (GHSA-7pq4-8pvv-rx7r)", async () => { + const linkParent = fs.mkdtempSync(path.join(os.tmpdir(), "omni-vault-link-")); + const link = path.join(linkParent, "vault"); + try { + fs.symlinkSync(TEST_DATA_DIR, link, "dir"); + } catch { + fs.rmSync(linkParent, { recursive: true, force: true }); + return; // platform without symlink permission — nothing to assert + } + try { + const res = await postVault(link); + assert.equal(res.status, 400); + assert.equal(obsidianDb.getWebdavEnabled(), false); + } finally { + fs.rmSync(linkParent, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } +}); + +test("POST with a directory unrelated to the data directory still succeeds (sibling in tmp)", async () => { + const vaultDir = fs.mkdtempSync(path.join(os.tmpdir(), "omni-vault-ok-")); + try { + const res = await postVault(vaultDir); + assert.equal(res.status, 200); + const body = (await res.json()) as Record; + assert.equal(body.vaultPath, path.resolve(vaultDir)); + assert.equal(obsidianDb.getWebdavEnabled(), true); + } finally { + fs.rmSync(vaultDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } +}); + test("POST with invalid body (missing vaultPath) → 400", async () => { const req = makeRequest("http://localhost/api/settings/obsidian/webdav", { method: "POST",