Public: anyone can read this; the crew can change it.
Sentinel: file:forge.db-shm was degraded 42 minutes today: /data/forge.db-shm is mode 644; it should be 0600 — run chmod 600 /data/forge.db-shm #3087
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/3084-sentinel-file-forge-db-shm-was-degraded → main 1 commit fork 2 threads · 2 unresolved
3 files
changed +32
−9
src/kernel/db.ts +5 −2
| @@ -7,7 +7,9 @@ | ||
| 7 | 7 | |
| 8 | 8 | import { Database } from "bun:sqlite"; |
| 9 | 9 | import { chmodSync, existsSync, writeFileSync } from "node:fs"; |
| 10 | import { dirname } from "node:path"; | |
| 10 | 11 | import { createLog } from "./log.ts"; |
| 12 | import { tightenDbFiles } from "./file_modes.ts"; | |
| 11 | 13 | import { type Migration, migrate } from "./migrate.ts"; |
| 12 | 14 | |
| 13 | 15 | const log = createLog("db"); |
| @@ -49,8 +51,8 @@ export function openDb(path: string, migrations: readonly Migration[]): Database | ||
| 49 | 51 | db = new Database(path, { create: true, strict: true }); |
| 50 | 52 | } |
| 51 | 53 | applyPragmas(db); |
| 52 | // Only files this call made: one that already exists is left as found, and | |
| 53 | // vitals says when it is too open (`file_modes.ts`). The umask can widen | |
| 54 | // Only files this call made: one that already exists is tightened below if | |
| 55 | // too open (`file_modes.ts`; vitals names any it could not). The umask can widen | |
| 54 | 56 | // the mode at creation, so it is set again. |
| 55 | 57 | if (fresh && !memory) |
| 56 | 58 | for (const suffix of ["", "-wal", "-shm"]) |
| @@ -62,6 +64,7 @@ export function openDb(path: string, migrations: readonly Migration[]): Database | ||
| 62 | 64 | // On a copy every migration is already recorded, so this is a no-op — |
| 63 | 65 | // and a list that grew since the copy was taken still runs what is new; |
| 64 | 66 | // one behind the copy boots and says what it does not know (#783). |
| 67 | if (!fresh && !memory) tightenDbFiles(dirname(path)); | |
| 65 | 68 | const { unknown } = migrate(db, migrations); |
| 66 | 69 | if (unknown.length > 0) |
| 67 | 70 | log.warn("database has migrations this build does not know; left as they are", { |
src/kernel/file_modes.ts +24 −3
| @@ -2,12 +2,17 @@ | ||
| 2 | 2 | * Files that hold the forge's state must not be readable by anyone else on |
| 3 | 3 | * the machine. The database and its write-ahead files are created 0600 |
| 4 | 4 | * (`db.ts`); one found wider at boot — a helper pod with fsGroup re-moded |
| 5 | * the volume (#1436) — is said, never silently chmod-ed and never fatal: | |
| 6 | * production's files may have been 0644 for a long time. | |
| 5 | * the volume (#1436) — is tightened at boot and logged, never fatal and never | |
| 6 | * silent: production's files may have been 0644 for a long time, and SQLite | |
| 7 | * gives -wal and -shm the mode of the main file, so leaving it wide kept | |
| 8 | * the vitals degraded for good (#3084). | |
| 7 | 9 | */ |
| 8 | 10 | |
| 9 | import { statSync } from "node:fs"; | |
| 11 | import { chmodSync, statSync } from "node:fs"; | |
| 10 | 12 | import { join } from "node:path"; |
| 13 | import { createLog } from "./log.ts"; | |
| 14 | ||
| 15 | const log = createLog("file_modes"); | |
| 11 | 16 | |
| 12 | 17 | export const DB_FILES = ["forge.db", "forge.db-wal", "forge.db-shm"] as const; |
| 13 | 18 | |
| @@ -30,3 +35,19 @@ export function openDbFiles(dataDir: string): { name: string; sentence: string } | ||
| 30 | 35 | } |
| 31 | 36 | return found; |
| 32 | 37 | } |
| 38 | ||
| 39 | /** | |
| 40 | * Narrow every too-open database file to 0600 and say each one. A file that | |
| 41 | * cannot be changed (not ours) stays, and vitals keeps naming it. | |
| 42 | */ | |
| 43 | export function tightenDbFiles(dataDir: string): void { | |
| 44 | for (const { name } of openDbFiles(dataDir)) { | |
| 45 | const path = join(dataDir, name); | |
| 46 | try { | |
| 47 | chmodSync(path, 0o600); | |
| 48 | log.warn("database file was too open; set to 0600", { path }); | |
| 49 | } catch { | |
| 50 | // Not ours to change: vitals still says it. | |
| 51 | } | |
| 52 | } | |
| 53 | } |
test/file-modes.test.ts +3 −4
| @@ -46,15 +46,14 @@ test("fresh database files are 0600", () => { | ||
| 46 | 46 | } |
| 47 | 47 | }); |
| 48 | 48 | |
| 49 | test("a 0644 forge.db boots, stays 0644, and shows as degraded with the chmod", () => { | |
| 49 | test("a 0644 forge.db is tightened to 0600 at boot, with its side files", () => { | |
The title promises side files, but only forge.db mode and the vitals list are asserted. Add expect(mode(join(dataDir, "forge.db-shm"))).toBe(0o600), and the same for -wal if present. |
||
| 50 | 50 | const dataDir = tmp(); |
| 51 | 51 | createApp({ dataDir, port: 0 }).db.close(); |
| 52 | 52 | chmodSync(join(dataDir, "forge.db"), 0o644); |
| 53 | 53 | const app = createApp({ dataDir, port: 0 }); |
| 54 | 54 | try { |
| 55 | expect(mode(join(dataDir, "forge.db"))).toBe(0o644); | |
| 56 | const found = forgeVitals(app).degraded.find((d) => d.cause === "file:forge.db"); | |
| 57 | expect(found?.sentence).toContain(`chmod 600 ${join(dataDir, "forge.db")}`); | |
| 55 | expect(mode(join(dataDir, "forge.db"))).toBe(0o600); | |
| 56 | expect(forgeVitals(app).degraded.some((d) => d.cause.startsWith("file:"))).toBe(false); | |
| 58 | 57 | } finally { |
| 59 | 58 | app.db.close(); |
| 60 | 59 | } |
new above
offline
The silent catch is deliberate and commented. Log at debug or info so the file that could not be changed is findable in the log, not only in vitals.