From 45c03127ce2f7282a0367f2c506acf9351087bb8 Mon Sep 17 00:00:00 2001 From: GeiserX <9169332+GeiserX@users.noreply.github.com> Date: Sat, 15 Aug 2026 16:12:14 +0200 Subject: [PATCH] fix(local): stop the database being readable by other accounts on the machine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit auth.json and server-connections.json are written 0600. The database beside them is created by SQLite with the process umask, so on a default macOS or Linux install it lands at 0644, along with its -wal and -shm sidecars. It is not the less sensitive of the two. It holds live PKCE verifiers, health-check response samples, artifact previews, and — for stdio MCP integrations created before the auth-method revamp — plaintext secret env. chmod on open rather than on create, so a database written by an earlier version is tightened too. Best-effort per file: a filesystem without POSIX modes must not stop the server booting over a permission it cannot set. --- apps/local/src/db/db-file-permissions.test.ts | 99 +++++++++++++++++++ apps/local/src/db/libsql.ts | 38 +++++++ 2 files changed, 137 insertions(+) create mode 100644 apps/local/src/db/db-file-permissions.test.ts diff --git a/apps/local/src/db/db-file-permissions.test.ts b/apps/local/src/db/db-file-permissions.test.ts new file mode 100644 index 0000000000..2b560d7fc9 --- /dev/null +++ b/apps/local/src/db/db-file-permissions.test.ts @@ -0,0 +1,99 @@ +// The local database sits in the same directory as `auth.json` and +// `server-connections.json`, both of which are deliberately 0600. SQLite +// creates its own files with the process umask instead, so on a default macOS +// or Linux install the database and its WAL sidecars land at 0644 — readable +// by every other account on the machine. +// +// These drive the real `openLocalLibsql` against a real file, because the +// property under test is what the filesystem ends up holding, not what the +// code appears to ask for. + +import { chmodSync, existsSync, mkdtempSync, readdirSync, rmSync, statSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import type { Client } from "@libsql/client"; +import { createClient } from "@libsql/client"; +import { afterEach, describe, expect, it } from "@effect/vitest"; + +import { openLocalLibsql } from "./libsql"; + +const dirs: string[] = []; +const clients: Client[] = []; + +afterEach(() => { + for (const client of clients.splice(0)) client.close(); + for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true }); +}); + +/** POSIX permission bits of `path`, or null when it does not exist. */ +const modeOf = (path: string): number | null => + existsSync(path) ? statSync(path).mode & 0o777 : null; + +const tempDir = (label: string): string => { + const dir = mkdtempSync(join(tmpdir(), `executor-db-perms-${label}-`)); + dirs.push(dir); + return dir; +}; + +/** Every file in `dir` that any account other than the owner can read. */ +const groupOrWorldReadable = (dir: string): readonly string[] => + readdirSync(dir).filter((name) => ((modeOf(join(dir, name)) ?? 0) & 0o044) !== 0); + +describe("local database file permissions", () => { + it("leaves nothing in the data directory readable by other accounts", async () => { + const dir = tempDir("create"); + const path = join(dir, "data.db"); + + const client = await openLocalLibsql(path); + clients.push(client); + // Force a write so WAL materializes its sidecars. + await client.execute("CREATE TABLE probe (id INTEGER PRIMARY KEY)"); + + expect(modeOf(path)).toBe(0o600); + // Asserting over the whole directory rather than a fixed list of suffixes: + // it does not depend on which sidecars SQLite happened to create, and it + // names the offenders when it fails. + expect(groupOrWorldReadable(dir)).toEqual([]); + }); + + it("tightens a database left world-readable by an earlier version", async () => { + // The upgrade path is the one that matters: a `mode` at creation cannot + // help a file that already exists, which is why the fix chmods on open. + const dir = tempDir("upgrade"); + const path = join(dir, "data.db"); + + const seed = createClient({ url: `file:${path}` }); + await seed.execute("CREATE TABLE probe (id INTEGER PRIMARY KEY)"); + seed.close(); + // Put the file into the exact state a pre-fix install leaves behind. + chmodSync(path, 0o644); + expect(modeOf(path)).toBe(0o644); + + const client = await openLocalLibsql(path); + clients.push(client); + + expect(modeOf(path)).toBe(0o600); + expect(groupOrWorldReadable(dir)).toEqual([]); + }); + + it("POSITIVE CONTROL: a raw libSQL open leaves the database world-readable", async () => { + // Proves the assertions above can fail. Without this, a chmod that + // silently stopped running would still leave them green on a machine with + // a strict umask — this is the check that the check works. + const dir = tempDir("control"); + const path = join(dir, "data.db"); + + const client = createClient({ url: `file:${path}` }); + await client.execute("CREATE TABLE probe (id INTEGER PRIMARY KEY)"); + client.close(); + + expect(modeOf(path)).not.toBe(0o600); + expect(groupOrWorldReadable(dir)).toContain("data.db"); + }); + + it("does not throw for an in-memory database", async () => { + const client = await openLocalLibsql(":memory:"); + clients.push(client); + await expect(client.execute("SELECT 1")).resolves.toBeDefined(); + }); +}); diff --git a/apps/local/src/db/libsql.ts b/apps/local/src/db/libsql.ts index 63844bffd0..a8552c199c 100644 --- a/apps/local/src/db/libsql.ts +++ b/apps/local/src/db/libsql.ts @@ -1,4 +1,5 @@ import { createClient, type Client, type InArgs, type ResultSet } from "@libsql/client"; +import { chmodSync } from "node:fs"; import { resolve } from "node:path"; // --------------------------------------------------------------------------- @@ -16,6 +17,41 @@ import { resolve } from "node:path"; const toLibsqlFileUrl = (path: string): string => path === ":memory:" ? path : `file:${resolve(path)}`; +/** The database and the two sidecars WAL mode creates beside it. */ +const DB_FILE_SUFFIXES = ["", "-wal", "-shm"] as const; + +/** + * Restrict the database and its WAL sidecars to owner-only. + * + * SQLite creates these files with the process umask, which on a default macOS + * or Linux install means 0644. The secret files next to them are deliberately + * 0600 (`auth.json`, `server-connections.json`), and the database is not the + * less sensitive of the two: it holds live `oauth_session.pkce_verifier` + * values, health-check response samples, artifact previews, and — for stdio + * MCP integrations created before the auth-method revamp — plaintext secret + * env in `integration.config`. + * + * Called on open rather than on create, so a database written by an earlier + * version is tightened too. That mirrors the `mode` + `chmod` pair in + * `writeToken`, and for the same reason: `mode` only applies at creation. + * + * Best-effort per file. A filesystem with no POSIX modes, or a read-only + * mount, must not stop the server booting over a permission it cannot set. + */ +const restrictDbFilePermissions = (path: string): void => { + if (path === ":memory:") return; + const base = resolve(path); + for (const suffix of DB_FILE_SUFFIXES) { + // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: chmod on a file that may not exist yet or a filesystem without POSIX modes; hardening must never block boot + try { + chmodSync(`${base}${suffix}`, 0o600); + } catch { + // Sidecars only exist once WAL has been used, and not every filesystem + // supports chmod. Either way the open proceeds. + } + } +}; + /** * Open a libSQL client for a local on-disk DB and apply the per-connection * PRAGMAs (foreign_keys + WAL). Used for the long-lived FumaDB handle. @@ -26,6 +62,8 @@ export const openLocalLibsql = async (path: string): Promise => { // first enabling. Re-apply both since libSQL gives no shared handle. await client.execute("PRAGMA foreign_keys = ON"); await client.execute("PRAGMA journal_mode = WAL"); + // After the WAL pragma, so `-wal`/`-shm` exist to be tightened. + restrictDbFilePermissions(path); // busy_timeout is per-connection (default 0 = fail immediately on a lock). // Under the supervised-daemon model a single process owns this file, but a // second OS process can still transiently hold the write lock (e.g. a CLI