Public: anyone can read this; the crew can change it.
SSH host keys and the database files are created 0600, and a too-open one is named on boot #3062
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/3025-ssh-host-keys-and-the-database-files-are → main 2 commits fork 2 threads · 2 unresolved
7 files
changed +135
−6
docs/THREATS.md +4
| @@ -42,6 +42,10 @@ Wants: to act as the person or agent it was issued to, for as long as it works. | ||
| 42 | 42 | - Stored provider tokens (origins) are sealed under a key in `secret.key`, never |
| 43 | 43 | returned by any route. |
| 44 | 44 | Test: `test/origins-security-auth.test.ts` |
| 45 | - The SSH host key and the database files are created 0600; a host key group- | |
| 46 | or world-readable is refused, and a database file found too open is a | |
| 47 | degraded finding naming the chmod, never a silent change and never a stop. | |
| 48 | Test: `test/file-modes.test.ts` | |
| 45 | 49 | - Revoking every token of one account at once, and telling its owner a token was |
| 46 | 50 | used from somewhere new. |
| 47 | 51 | Test: none yet |
src/kernel/db.ts +12 −2
| @@ -6,7 +6,7 @@ | ||
| 6 | 6 | */ |
| 7 | 7 | |
| 8 | 8 | import { Database } from "bun:sqlite"; |
| 9 | import { existsSync, writeFileSync } from "node:fs"; | |
| 9 | import { chmodSync, existsSync, writeFileSync } from "node:fs"; | |
| 10 | 10 | import { createLog } from "./log.ts"; |
| 11 | 11 | import { type Migration, migrate } from "./migrate.ts"; |
| 12 | 12 | |
| @@ -45,10 +45,20 @@ export function openDb(path: string, migrations: readonly Migration[]): Database | ||
| 45 | 45 | if (image !== undefined && memory) { |
| 46 | 46 | db = Database.deserialize(rollbackJournal(image), { strict: true }); |
| 47 | 47 | } else { |
| 48 | if (image !== undefined) writeFileSync(path, image, { flag: "wx" }); | |
| 48 | if (image !== undefined) writeFileSync(path, image, { flag: "wx", mode: 0o600 }); | |
| 49 | 49 | db = new Database(path, { create: true, strict: true }); |
| 50 | 50 | } |
| 51 | 51 | 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 | // the mode at creation, so it is set again. | |
| 55 | if (fresh && !memory) | |
| 56 | for (const suffix of ["", "-wal", "-shm"]) | |
| 57 | try { | |
| 58 | chmodSync(path + suffix, 0o600); | |
| 59 | } catch { | |
| 60 | // No such file (a journal mode without one): nothing to tighten. | |
| 61 | } | |
| 52 | 62 | // On a copy every migration is already recorded, so this is a no-op — |
| 53 | 63 | // and a list that grew since the copy was taken still runs what is new; |
| 54 | 64 | // one behind the copy boots and says what it does not know (#783). |
src/kernel/file_modes.ts added +32
| @@ -0,0 +1,32 @@ | ||
| 1 | /** | |
| 2 | * Files that hold the forge's state must not be readable by anyone else on | |
| 3 | * the machine. The database and its write-ahead files are created 0600 | |
| 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. | |
| 7 | */ | |
| 8 | ||
| 9 | import { statSync } from "node:fs"; | |
| 10 | import { join } from "node:path"; | |
| 11 | ||
| 12 | export const DB_FILES = ["forge.db", "forge.db-wal", "forge.db-shm"] as const; | |
| 13 | ||
| 14 | /** One sentence per database file that group or world can read, naming the fix. */ | |
| 15 | export function openDbFiles(dataDir: string): { name: string; sentence: string }[] { | |
| 16 | const found: { name: string; sentence: string }[] = []; | |
| 17 | for (const name of DB_FILES) { | |
| 18 | const path = join(dataDir, name); | |
| 19 | let mode: number; | |
| 20 | try { | |
| 21 | mode = statSync(path).mode & 0o777; | |
| 22 | } catch { | |
| 23 | continue; // absent: -wal and -shm come and go | |
| 24 | } | |
| 25 | if ((mode & 0o077) !== 0) | |
| 26 | found.push({ | |
| 27 | name, | |
| 28 | sentence: `${path} is mode ${mode.toString(8)}; it should be 0600 — run chmod 600 ${path}`, | |
| 29 | }); | |
| 30 | } | |
| 31 | return found; | |
| 32 | } |
src/kernel/vitals.ts +4
| @@ -22,6 +22,7 @@ import { | ||
| 22 | 22 | } from "../core/runs.ts"; |
| 23 | 23 | import { STALE_AFTER_MS } from "../runs/lease.ts"; |
| 24 | 24 | import { BUDGETS } from "./budgets.ts"; |
| 25 | import { openDbFiles } from "./file_modes.ts"; | |
| 25 | 26 | import { lastMirror, mirrorNeed } from "../snapshot/mirror.ts"; |
| 26 | 27 | import { lastSnapshotOutcome, snapshotNeed } from "../snapshot/outcome.ts"; |
| 27 | 28 | import { drillNeed, firstSeen as drillFirstSeen, lastVerdict } from "../snapshot/verdict.ts"; |
| @@ -291,6 +292,9 @@ export function vitals(input: VitalsInput): Vitals { | ||
| 291 | 292 | "/settings/capacity", |
| 292 | 293 | ); |
| 293 | 294 | |
| 295 | for (const file of openDbFiles(input.dataDir)) | |
| 296 | need(`file:${file.name}`, file.sentence, undefined, "/settings/capacity"); | |
| 297 | ||
| 294 | 298 | // Snapshots on the volume they copy, and no mirror, share its one point of failure. |
| 295 | 299 | const snaps = input.snapshots; |
| 296 | 300 | if ( |
src/ssh/keys.ts +21 −3
| @@ -17,9 +17,16 @@ import { | ||
| 17 | 17 | verify as cryptoVerify, |
| 18 | 18 | type KeyObject, |
| 19 | 19 | } from "node:crypto"; |
| 20 | import { mkdirSync, readFileSync, renameSync, writeFileSync } from "node:fs"; | |
| 20 | import { | |
| 21 | chmodSync, | |
| 22 | mkdirSync, | |
| 23 | readFileSync, | |
| 24 | renameSync, | |
| 25 | statSync, | |
| 26 | writeFileSync, | |
| 27 | } from "node:fs"; | |
| 21 | 28 | import { join } from "node:path"; |
| 22 | import { defect } from "../kernel/error.ts"; | |
| 29 | import { defect, ForgeError } from "../kernel/error.ts"; | |
| 23 | 30 | import { err, ok, type Result } from "../kernel/result.ts"; |
| 24 | 31 | import { Reader, Writer } from "./wire.ts"; |
| 25 | 32 | |
| @@ -278,8 +285,18 @@ export function loadOrCreateHostKey(dir: string): HostKey { | ||
| 278 | 285 | const path = join(dir, "host_ed25519"); |
| 279 | 286 | let privateKey: KeyObject; |
| 280 | 287 | try { |
| 281 | privateKey = createPrivateKey(readFileSync(path, "utf8")); | |
| 288 | const pem = readFileSync(path, "utf8"); | |
| 289 | // Same refusal as secret.key: fail closed, never chmod it quietly. | |
| 290 | const mode = statSync(path).mode & 0o777; | |
This refusal stops boot for an existing key at 0644. Confirm production's key mode before release, or the next deploy crash-loops. |
||
| 291 | if ((mode & 0o077) !== 0) { | |
| 292 | defect( | |
| 293 | "ssh.host_key_too_open", | |
| 294 | `${path} is mode ${mode.toString(8)}; it must be 0600 — fix the permissions and restart`, | |
| 295 | ); | |
| 296 | } | |
| 297 | privateKey = createPrivateKey(pem); | |
| 282 | 298 | } catch (cause) { |
| 299 | if (ForgeError.isInstance(cause)) throw cause; | |
| 283 | 300 | if ((cause as NodeJS.ErrnoException).code !== "ENOENT") { |
| 284 | 301 | defect("ssh.host_key", `cannot read host key at ${path}`, cause); |
| 285 | 302 | } |
| @@ -287,6 +304,7 @@ export function loadOrCreateHostKey(dir: string): HostKey { | ||
| 287 | 304 | const pem = pair.privateKey.export({ type: "pkcs8", format: "pem" }); |
| 288 | 305 | // Write-then-rename so a crash mid-write cannot leave a truncated key. |
| 289 | 306 | writeFileSync(`${path}.tmp`, pem, { mode: 0o600 }); |
| 307 | chmodSync(`${path}.tmp`, 0o600); | |
| 290 | 308 | renameSync(`${path}.tmp`, path); |
| 291 | 309 | privateKey = pair.privateKey; |
| 292 | 310 | } |
test/file-modes.test.ts added +61
| @@ -0,0 +1,61 @@ | ||
| 1 | /** Host keys and database files are 0600; too-open ones are refused or named (#3025). */ | |
| 2 | ||
| 3 | import { afterAll, beforeAll, expect, test } from "bun:test"; | |
| 4 | import { chmodSync, mkdtempSync, rmSync, statSync } from "node:fs"; | |
| 5 | import { tmpdir } from "node:os"; | |
| 6 | import { join } from "node:path"; | |
| 7 | import { createApp } from "../src/app.ts"; | |
| 8 | import { forgeVitals } from "../src/actions/health.ts"; | |
| 9 | import { loadOrCreateHostKey } from "../src/ssh/keys.ts"; | |
| 10 | ||
| 11 | const dirs: string[] = []; | |
| 12 | const tmp = () => { | |
| 13 | const d = mkdtempSync(join(tmpdir(), "forge-modes-")); | |
| 14 | dirs.push(d); | |
| 15 | return d; | |
| 16 | }; | |
| 17 | afterAll(() => { | |
| 18 | for (const d of dirs) rmSync(d, { recursive: true, force: true }); | |
| 19 | }); | |
| 20 | const mode = (p: string) => statSync(p).mode & 0o777; | |
| 21 | ||
| 22 | test("a fresh host key is 0600, and a 0640 one is refused with the path and mode", () => { | |
| 23 | const dir = tmp(); | |
| 24 | loadOrCreateHostKey(dir); | |
| 25 | expect(mode(join(dir, "host_ed25519"))).toBe(0o600); | |
| 26 | chmodSync(join(dir, "host_ed25519"), 0o640); | |
| 27 | expect(() => loadOrCreateHostKey(dir)).toThrow(/host_ed25519 is mode 640; it must be 0600/); | |
| 28 | expect(mode(join(dir, "host_ed25519"))).toBe(0o640); | |
| 29 | }); | |
| 30 | ||
| 31 | test("fresh database files are 0600", () => { | |
| 32 | const dataDir = tmp(); | |
| 33 | const app = createApp({ dataDir, port: 0 }); | |
| 34 | try { | |
| 35 | expect(mode(join(dataDir, "forge.db"))).toBe(0o600); | |
| 36 | for (const s of ["-wal", "-shm"]) { | |
| 37 | try { | |
| 38 | expect(mode(join(dataDir, "forge.db" + s))).toBe(0o600); | |
| 39 | } catch (e) { | |
| 40 | if ((e as NodeJS.ErrnoException).code !== "ENOENT") throw e; | |
| 41 | } | |
| 42 | } | |
| 43 | expect(forgeVitals(app).degraded.some((d) => d.cause.startsWith("file:"))).toBe(false); | |
| 44 | } finally { | |
| 45 | app.db.close(); | |
| 46 | } | |
| 47 | }); | |
| 48 | ||
| 49 | test("a 0644 forge.db boots, stays 0644, and shows as degraded with the chmod", () => { | |
| 50 | const dataDir = tmp(); | |
| 51 | createApp({ dataDir, port: 0 }).db.close(); | |
| 52 | chmodSync(join(dataDir, "forge.db"), 0o644); | |
| 53 | const app = createApp({ dataDir, port: 0 }); | |
| 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")}`); | |
| 58 | } finally { | |
| 59 | app.db.close(); | |
| 60 | } | |
| 61 | }); |
test/spawn-events.test.ts +1 −1
| @@ -116,7 +116,7 @@ test("the door starts a watcher for a new child and not for an existing one", as | ||
| 116 | 116 | const ctx = { app, request: deps().request, url: deps().url } as unknown as Ctx; |
| 117 | 117 | const raw = { name: "kid", org: "kids", apps: ["alice/notes"], dryRun: false }; |
| 118 | 118 | const live = () => events().filter((e) => e.kind === "forge.spawn.live").length; |
| 119 | const until = async (n: number) => { for (let i = 0; i < 100 && live() < n; i++) await Bun.sleep(20); }; | |
| 119 | const until = async (n: number) => { for (let i = 0; i < 500 && live() < n; i++) await Bun.sleep(20); }; | |
| 120 | 120 | |
| 121 | 121 | expect((await spawnDoor(ctx, alice, raw)).status).toBe(201); |
| 122 | 122 | await until(1); |
new above
offline
The comment says the umask can widen the mode. A umask only removes bits, so this chmod is belt-and-braces. Reword the comment to say that, or drop the chmod.