From df87e9363b6b08bd6b40ba9b774753b98d5a2cd4 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Tue, 15 Sep 2026 16:58:24 -0300 Subject: [PATCH] =?UTF-8?q?fix(auth):=20close=20the=20JWT=5FSECRET=20boots?= =?UTF-8?q?trap=20chain=20=E2=80=94=20real-peer=20loopback,=20obsidian=20a?= =?UTF-8?q?lways-protected,=20DATA=5FDIR=20vault=20refusal=20(#13791)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GHSA-7pq4-8pvv-rx7r (critical). Every link of the reported chain held on the release tip: 1. First boot without JWT_SECRET generates one and writes it in cleartext to $DATA_DIR/server.env. 2. With no password configured, isAuthRequired() returned false for POST /api/settings/require-login unconditionally — before the loopback check — so any network peer could switch requireLogin off. 3. With requireLogin off, POST /api/settings/obsidian/webdav accepted an arbitrary vault root and echoed freshly minted Basic credentials. 4. The WebDAV file service is served by the custom Node layer before Next.js, outside the authz pipeline. 5. Pointing it at DATA_DIR reads server.env, and JWT_SECRET forges an `{"authenticated":true}` admin session. A second, worse problem surfaced while verifying: isLoopbackRequest() decided "loopback" from nextUrl.hostname / the Host header, which the client controls. `Host: localhost` from a remote address made the whole fresh-install bootstrap reachable, not just the write path. Three cuts, plus the root cause: - isLoopbackRequest() now reads the trusted peer: the token-stamped real TCP peer the custom server writes (peerStamp), then the pipeline's own locality verdict once a stamp token exists, then a real socket peer. The bootstrap write path honours the same constraint instead of returning false, and managementPolicy hands down the peerContext verdict explicitly, because at policy time the original request still carries client-supplied headers. - Host is consulted only when the process has no stamp token at all — no stamping server in front, which in practice means route handlers invoked directly by the unit-test harness. Every supported runtime (run-next dev and start, standalone-server-ws for Docker, the npm CLI and Electron) calls ensurePeerStampToken() at boot, so there a signal-less request fails closed. Without this fallback ~340 route tests that call handlers with `new Request("http://localhost/…")` turned into 401s. - /api/settings/obsidian joins ALWAYS_PROTECTED_API_PATHS: issuing and rotating reusable WebDAV credentials is credential export, the same rationale as the GHSA-62vw entry for the password reveal. - enableObsidianVaultSync() refuses a vault that is, sits inside, or contains DATA_DIR, comparing realpath-resolved paths so a symlink cannot dodge it. Tests are red-first: remote stamped peer → auth required on the bootstrap write; Host: localhost plus a forged locality header from a non-loopback stamped peer → 401 through the full pipeline; the local operator keeps the first-password flow; obsidian inventory and DATA_DIR overlap cases. --- ...-jwt-bootstrap-chain-real-peer-loopback.md | 1 + docs/openapi.yaml | 6 + docs/security/ROUTE_GUARD_TIERS.md | 21 ++ scripts/dev/run-protocol-clients-tests.mjs | 10 +- src/lib/obsidianSync.ts | 39 +++ src/server/authz/policies/management.ts | 12 +- src/server/authz/routeGuard.ts | 11 + src/shared/utils/apiAuth.ts | 179 ++++++++++--- tests/unit/api-auth.test.ts | 244 +++++++++++++++++- ...credential-export-always-protected.test.ts | 29 ++- tests/unit/authz/management-policy.test.ts | 95 ++++++- tests/unit/authz/pipeline.test.ts | 44 +++- tests/unit/obsidian-webdav-route.test.ts | 86 +++++- 13 files changed, 715 insertions(+), 62 deletions(-) create mode 100644 changelog.d/fixes/0000-jwt-bootstrap-chain-real-peer-loopback.md 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",