History

fix(local): stop the database being readable by other accounts on the machine (#1594)

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.

Sergio Fernández committed 2 months ago

Commit b4add2d1 parent 6a0b3992 files · +137 −0Browse files

Unified · Split

2 files
  1. apps/local/src/db/db-file-permissions.test.ts +99 −0
  2. apps/local/src/db/libsql.ts +38 −0
apps/local/src/db/db-file-permissions.test.ts+99−0
@@ -0,0 +1,99 @@
1+// The local database sits in the same directory as `auth.json` and
2+// `server-connections.json`, both of which are deliberately 0600. SQLite
3+// creates its own files with the process umask instead, so on a default macOS
4+// or Linux install the database and its WAL sidecars land at 0644 — readable
5+// by every other account on the machine.
6+//
7+// These drive the real `openLocalLibsql` against a real file, because the
8+// property under test is what the filesystem ends up holding, not what the
9+// code appears to ask for.
10+
11+import { chmodSync, existsSync, mkdtempSync, readdirSync, rmSync, statSync } from "node:fs";
12+import { tmpdir } from "node:os";
13+import { join } from "node:path";
14+import type { Client } from "@libsql/client";
15+import { createClient } from "@libsql/client";
16+import { afterEach, describe, expect, it } from "@effect/vitest";
17+
18+import { openLocalLibsql } from "./libsql";
19+
20+const dirs: string[] = [];
21+const clients: Client[] = [];
22+
23+afterEach(() => {
24+ for (const client of clients.splice(0)) client.close();
25+ for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true });
26+});
27+
28+/** POSIX permission bits of `path`, or null when it does not exist. */
29+const modeOf = (path: string): number | null =>
30+ existsSync(path) ? statSync(path).mode & 0o777 : null;
31+
32+const tempDir = (label: string): string => {
33+ const dir = mkdtempSync(join(tmpdir(), `executor-db-perms-${label}-`));
34+ dirs.push(dir);
35+ return dir;
36+};
37+
38+/** Every file in `dir` that any account other than the owner can read. */
39+const groupOrWorldReadable = (dir: string): readonly string[] =>
40+ readdirSync(dir).filter((name) => ((modeOf(join(dir, name)) ?? 0) & 0o044) !== 0);
41+
42+describe("local database file permissions", () => {
43+ it("leaves nothing in the data directory readable by other accounts", async () => {
44+ const dir = tempDir("create");
45+ const path = join(dir, "data.db");
46+
47+ const client = await openLocalLibsql(path);
48+ clients.push(client);
49+ // Force a write so WAL materializes its sidecars.
50+ await client.execute("CREATE TABLE probe (id INTEGER PRIMARY KEY)");
51+
52+ expect(modeOf(path)).toBe(0o600);
53+ // Asserting over the whole directory rather than a fixed list of suffixes:
54+ // it does not depend on which sidecars SQLite happened to create, and it
55+ // names the offenders when it fails.
56+ expect(groupOrWorldReadable(dir)).toEqual([]);
57+ });
58+
59+ it("tightens a database left world-readable by an earlier version", async () => {
60+ // The upgrade path is the one that matters: a `mode` at creation cannot
61+ // help a file that already exists, which is why the fix chmods on open.
62+ const dir = tempDir("upgrade");
63+ const path = join(dir, "data.db");
64+
65+ const seed = createClient({ url: `file:${path}` });
66+ await seed.execute("CREATE TABLE probe (id INTEGER PRIMARY KEY)");
67+ seed.close();
68+ // Put the file into the exact state a pre-fix install leaves behind.
69+ chmodSync(path, 0o644);
70+ expect(modeOf(path)).toBe(0o644);
71+
72+ const client = await openLocalLibsql(path);
73+ clients.push(client);
74+
75+ expect(modeOf(path)).toBe(0o600);
76+ expect(groupOrWorldReadable(dir)).toEqual([]);
77+ });
78+
79+ it("POSITIVE CONTROL: a raw libSQL open leaves the database world-readable", async () => {
80+ // Proves the assertions above can fail. Without this, a chmod that
81+ // silently stopped running would still leave them green on a machine with
82+ // a strict umask — this is the check that the check works.
83+ const dir = tempDir("control");
84+ const path = join(dir, "data.db");
85+
86+ const client = createClient({ url: `file:${path}` });
87+ await client.execute("CREATE TABLE probe (id INTEGER PRIMARY KEY)");
88+ client.close();
89+
90+ expect(modeOf(path)).not.toBe(0o600);
91+ expect(groupOrWorldReadable(dir)).toContain("data.db");
92+ });
93+
94+ it("does not throw for an in-memory database", async () => {
95+ const client = await openLocalLibsql(":memory:");
96+ clients.push(client);
97+ await expect(client.execute("SELECT 1")).resolves.toBeDefined();
98+ });
99+});
apps/local/src/db/libsql.ts+38−0
@@ -1,4 +1,5 @@
11 import { createClient, type Client, type InArgs, type ResultSet } from "@libsql/client";
2+import { chmodSync } from "node:fs";
23 import { resolve } from "node:path";
34
45 // ---------------------------------------------------------------------------
@@ -16,6 +17,41 @@ import { resolve } from "node:path"; · 11 unmodified lines
1617 const toLibsqlFileUrl = (path: string): string =>
1718 path === ":memory:" ? path : `file:${resolve(path)}`;
1819
20+/** The database and the two sidecars WAL mode creates beside it. */
21+const DB_FILE_SUFFIXES = ["", "-wal", "-shm"] as const;
22+
23+/**
24+ * Restrict the database and its WAL sidecars to owner-only.
25+ *
26+ * SQLite creates these files with the process umask, which on a default macOS
27+ * or Linux install means 0644. The secret files next to them are deliberately
28+ * 0600 (`auth.json`, `server-connections.json`), and the database is not the
29+ * less sensitive of the two: it holds live `oauth_session.pkce_verifier`
30+ * values, health-check response samples, artifact previews, and — for stdio
31+ * MCP integrations created before the auth-method revamp — plaintext secret
32+ * env in `integration.config`.
33+ *
34+ * Called on open rather than on create, so a database written by an earlier
35+ * version is tightened too. That mirrors the `mode` + `chmod` pair in
36+ * `writeToken`, and for the same reason: `mode` only applies at creation.
37+ *
38+ * Best-effort per file. A filesystem with no POSIX modes, or a read-only
39+ * mount, must not stop the server booting over a permission it cannot set.
40+ */
41+const restrictDbFilePermissions = (path: string): void => {
42+ if (path === ":memory:") return;
43+ const base = resolve(path);
44+ for (const suffix of DB_FILE_SUFFIXES) {
45+ // 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
46+ try {
47+ chmodSync(`${base}${suffix}`, 0o600);
48+ } catch {
49+ // Sidecars only exist once WAL has been used, and not every filesystem
50+ // supports chmod. Either way the open proceeds.
51+ }
52+ }
53+};
54+
1955 /**
2056 * Open a libSQL client for a local on-disk DB and apply the per-connection
2157 * PRAGMAs (foreign_keys + WAL). Used for the long-lived FumaDB handle.
@@ -26,6 +62,8 @@ export const openLocalLibsql = async (path: string): Promise<Client> => { · 4 unmodified lines
2662 // first enabling. Re-apply both since libSQL gives no shared handle.
2763 await client.execute("PRAGMA foreign_keys = ON");
2864 await client.execute("PRAGMA journal_mode = WAL");
65+ // After the WAL pragma, so `-wal`/`-shm` exist to be tightened.
66+ restrictDbFilePermissions(path);
2967 // busy_timeout is per-connection (default 0 = fail immediately on a lock).
3068 // Under the supervised-daemon model a single process owns this file, but a
3169 // second OS process can still transiently hold the write lock (e.g. a CLI