From 30cb676fadd9afcb36847ec62317ce2469d5babe Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Mon, 26 Oct 2026 11:37:12 +0100 Subject: [PATCH] fix: write the admin config, password hash and setup token readable only by the app's own user --- lib/admin/__tests__/secret-file-modes.test.ts | 63 +++++++++++++++++++ lib/admin/config-manager.ts | 3 +- lib/admin/password.ts | 3 +- lib/setup/token.ts | 3 +- 4 files changed, 69 insertions(+), 3 deletions(-) create mode 100644 lib/admin/__tests__/secret-file-modes.test.ts diff --git a/lib/admin/__tests__/secret-file-modes.test.ts b/lib/admin/__tests__/secret-file-modes.test.ts new file mode 100644 index 000000000..489f97328 --- /dev/null +++ b/lib/admin/__tests__/secret-file-modes.test.ts @@ -0,0 +1,63 @@ +/** + * @vitest-environment node + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import path from 'node:path'; + +// config.json carries the session secret and the OAuth client secrets, +// admin.json the admin password hash, .setup-token the token that lets +// anyone finish the setup wizard. With the default umask they came out +// world-readable, so on a bind mount or a bare-metal install every local +// user could read the key behind every session cookie. They are written +// owner-only now, like the generated .session-secret. + +// Hoisted so the dirs are set before any admin module loads. +const dirs = await vi.hoisted(async () => { + const { mkdtempSync } = await import('node:fs'); + const { tmpdir } = await import('node:os'); + const { join } = await import('node:path'); + const config = mkdtempSync(join(tmpdir(), 'bulwark-modes-')); + process.env.ADMIN_CONFIG_DIR = config; + process.env.ADMIN_STATE_DIR = join(config, 'state'); + return { config, state: join(config, 'state') }; +}); + +const writeFile = vi.hoisted(() => vi.fn()); +vi.mock('node:fs/promises', async (importOriginal) => { + const actual = await importOriginal(); + writeFile.mockImplementation(actual.writeFile); + return { ...actual, default: { ...actual, writeFile }, writeFile }; +}); + +/** The mode the file at `target` (or its temp file) was created with. */ +function modeOf(target: string): unknown { + const call = writeFile.mock.calls.find(([file]) => file === target || file === `${target}.tmp`); + expect(call, target).toBeDefined(); + const options = call![2]; + return typeof options === 'object' && options !== null ? (options as { mode?: number }).mode : undefined; +} + +describe('files holding secrets are owner-only', () => { + beforeEach(() => { + writeFile.mockClear(); + }); + + it('writes config.json with mode 0600', async () => { + const { configManager } = await import('@/lib/admin/config-manager'); + await configManager.ensureLoaded(); + await configManager.setAdminConfig({ oauthClientSecret: 'client-secret' }); + expect(modeOf(path.join(dirs.config, 'config.json'))).toBe(0o600); + }); + + it('writes admin.json with mode 0600', async () => { + const { setInitialAdminPassword } = await import('@/lib/admin/password'); + expect(await setInitialAdminPassword('correct horse battery', { allowOverwrite: true })).toBe(true); + expect(modeOf(path.join(dirs.config, 'admin.json'))).toBe(0o600); + }); + + it('writes the setup token with mode 0600', async () => { + const { ensureSetupToken } = await import('@/lib/setup/token'); + await ensureSetupToken(); + expect(modeOf(path.join(dirs.state, '.setup-token'))).toBe(0o600); + }); +}); diff --git a/lib/admin/config-manager.ts b/lib/admin/config-manager.ts index 403be7537..ea3d22482 100644 --- a/lib/admin/config-manager.ts +++ b/lib/admin/config-manager.ts @@ -242,7 +242,8 @@ class ConfigManager { await ensureConfigDir(); const targetPath = getConfigPath(filename); const tmpPath = targetPath + '.tmp'; - await writeFile(tmpPath, JSON.stringify(data, null, 2), 'utf-8'); + // Owner-only: config.json carries the session secret and OAuth client secrets. + await writeFile(tmpPath, JSON.stringify(data, null, 2), { encoding: 'utf-8', mode: 0o600 }); await rename(tmpPath, targetPath); } } diff --git a/lib/admin/password.ts b/lib/admin/password.ts index c9772bd4a..d5c6677e9 100644 --- a/lib/admin/password.ts +++ b/lib/admin/password.ts @@ -90,7 +90,8 @@ async function writeConfigData(data: AdminConfigData): Promise { await ensureConfigDir(); const target = getConfigPath(ADMIN_CONFIG_FILE); const tmp = target + '.tmp'; - await writeFile(tmp, JSON.stringify(data, null, 2), 'utf-8'); + // Owner-only, like the other files that hold secrets. + await writeFile(tmp, JSON.stringify(data, null, 2), { encoding: 'utf-8', mode: 0o600 }); await rename(tmp, target); } diff --git a/lib/setup/token.ts b/lib/setup/token.ts index dec3e568e..1ae15f75c 100644 --- a/lib/setup/token.ts +++ b/lib/setup/token.ts @@ -58,7 +58,8 @@ export async function ensureSetupToken(ttlSeconds: number = DEFAULT_TTL_SECONDS) ttlSeconds, }; const path = getStatePath(TOKEN_FILE); - await writeFile(path, JSON.stringify(payload, null, 2), 'utf-8'); + // Owner-only: whoever reads it can finish the setup wizard. + await writeFile(path, JSON.stringify(payload, null, 2), { encoding: 'utf-8', mode: 0o600 }); return token; }