diff --git a/packages/web/bin/lib/cli-settings-accessors.js b/packages/web/bin/lib/cli-settings-accessors.js new file mode 100644 index 00000000..f86d4101 --- /dev/null +++ b/packages/web/bin/lib/cli-settings-accessors.js @@ -0,0 +1,101 @@ +// Minimal settings.json access for CLI contexts (connect-url, pairing +// candidate building) that must not load the full web settings runtime. +// +// The running app already treats settings.json as a shared store — the relay +// identity, tunnels, notifications, the Electron main, and ssh-manager all +// read-modify-write it. This accessor must therefore mirror the settings +// runtime's guarantees or it will corrupt or regenerate shared state: +// +// - ATOMIC writes (write tmp, rename into place). A plain writeFile can +// interleave with a concurrent reader in the running app; the reader sees +// a half-written file, its lenient read maps it to `{}`, and relay +// identity logic then mints a NEW serverId — orphaning every paired +// device. The tmp+rename below means no reader can ever observe a partial +// file. +// +// - A STRICT read that THROWS on corrupt/unreadable payloads, gating relay +// identity regeneration. Only a genuinely missing file means "no +// settings"; any other failure (corrupt JSON, EACCES, transient I/O, +// non-object payload) must propagate so callers never confuse a broken +// read with first run and mint a replacement signing/encryption keypair. + +export const createSettingsAccessors = ({ fsPromises, path, dataDir, settingsFileName }) => { + const settingsPath = path.join(dataDir, settingsFileName); + + const readSettingsFromDiskMigrated = async () => { + try { + return JSON.parse(await fsPromises.readFile(settingsPath, 'utf8')); + } catch { + return {}; + } + }; + + const readSettingsStrict = async () => { + let raw; + try { + raw = await fsPromises.readFile(settingsPath, 'utf8'); + } catch (error) { + if (error && typeof error === 'object' && error.code === 'ENOENT') { + return {}; + } + throw error; + } + const parsed = JSON.parse(raw); + if (!parsed || typeof parsed !== 'object') { + throw new Error('Settings file is malformed (non-object payload)'); + } + return parsed; + }; + + const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); + + const isTransientWindowsReplaceError = (error) => { + if (process.platform !== 'win32' || !error || typeof error !== 'object') { + return false; + } + return error.code === 'EPERM' || error.code === 'EACCES' || error.code === 'EBUSY'; + }; + + const replaceFile = async (tmp, target) => { + const maxAttempts = process.platform === 'win32' ? 6 : 1; + let lastError = null; + + for (let attempt = 1; attempt <= maxAttempts; attempt += 1) { + try { + await fsPromises.rename(tmp, target); + return; + } catch (error) { + lastError = error; + if (!isTransientWindowsReplaceError(error) || attempt === maxAttempts) { + break; + } + await sleep(25 * attempt); + } + } + + if (!isTransientWindowsReplaceError(lastError)) { + throw lastError; + } + + // Windows can transiently reject the atomic replace while another process + // briefly holds the target open. Copy the COMPLETE tmp file into place so + // persistence never wedges; a reader can still never see partial content. + await fsPromises.copyFile(tmp, target); + await fsPromises.rm(tmp, { force: true }); + }; + + const writeSettingsToDisk = async (settings) => { + await fsPromises.mkdir(path.dirname(settingsPath), { recursive: true }); + const tmp = `${settingsPath}.tmp-${process.pid}-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; + await fsPromises.writeFile(tmp, JSON.stringify(settings, null, 2), { encoding: 'utf8', mode: 0o600 }); + if (process.platform !== 'win32') { + await fsPromises.chmod(tmp, 0o600); + } + await replaceFile(tmp, settingsPath); + if (process.platform !== 'win32') { + await fsPromises.chmod(settingsPath, 0o600); + } + }; + + return { readSettingsFromDiskMigrated, readSettingsStrict, writeSettingsToDisk }; +}; diff --git a/packages/web/bin/lib/cli-settings-accessors.test.js b/packages/web/bin/lib/cli-settings-accessors.test.js new file mode 100644 index 00000000..36899186 --- /dev/null +++ b/packages/web/bin/lib/cli-settings-accessors.test.js @@ -0,0 +1,127 @@ +import { describe, expect, it } from 'vitest'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import crypto from 'crypto'; + +import { createSettingsAccessors } from './cli-settings-accessors.js'; +import { createRelayIdentityRuntime } from '../../server/lib/relay/identity.js'; + +const withTempDir = async (fn) => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'oc-settings-accessors-')); + try { + return await fn(dir); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +}; + +const makeAccessors = (dir) => + createSettingsAccessors({ fsPromises: fs.promises, path, dataDir: dir, settingsFileName: 'settings.json' }); + +describe('cli settings accessors', () => { + it('persists the full object atomically and cleans up its tmp file', async () => { + await withTempDir(async (dir) => { + const accessors = makeAccessors(dir); + await accessors.writeSettingsToDisk({ theme: 'dark', count: 3 }); + + const raw = JSON.parse(fs.readFileSync(path.join(dir, 'settings.json'), 'utf8')); + expect(raw).toEqual({ theme: 'dark', count: 3 }); + + const leftovers = fs.readdirSync(dir).filter((name) => name.startsWith('settings.json.tmp-')); + expect(leftovers).toEqual([]); + }); + }); + + it('never leaves a partial file observable by a concurrent reader during writes', async () => { + await withTempDir(async (dir) => { + const accessors = makeAccessors(dir); + const filePath = path.join(dir, 'settings.json'); + + // Hammer reads concurrently with writes; every observed payload must be a + // complete, parseable object (the old plain writeFile could surface a + // torn file mid-rename, which is what tripped the relay identity logic). + const stop = { value: false }; + const reader = (async () => { + while (!stop.value) { + try { + const parsed = JSON.parse(await fs.promises.readFile(filePath, 'utf8')); + if (parsed && typeof parsed === 'object') { + // A complete object is always fine; anything else would be a tear. + expect(parsed.theme).toBe('dark'); + } + } catch { + // ENOENT during the very first write is acceptable. + } + await new Promise((resolve) => setTimeout(resolve, 0)); + } + })(); + + const big = { theme: 'dark', filler: 'x'.repeat(4096) }; + await Promise.all( + Array.from({ length: 50 }, (_, i) => + accessors.writeSettingsToDisk({ ...big, n: i }).catch(() => {}), + ), + ); + stop.value = true; + await reader; + + const final = JSON.parse(fs.readFileSync(filePath, 'utf8')); + expect(final.theme).toBe('dark'); + }); + }); + + it('lenient read maps a corrupt file to {} for config lookup', async () => { + await withTempDir(async (dir) => { + const accessors = makeAccessors(dir); + fs.writeFileSync(path.join(dir, 'settings.json'), '{"unfinished": "trunc'); + expect(await accessors.readSettingsFromDiskMigrated()).toEqual({}); + }); + }); + + it('strict read throws on a corrupt file instead of reporting "no settings"', async () => { + await withTempDir(async (dir) => { + const accessors = makeAccessors(dir); + fs.writeFileSync(path.join(dir, 'settings.json'), '{"unfinished": "trunc'); + await expect(accessors.readSettingsStrict()).rejects.toThrow(); + }); + }); + + it('strict read throws on a non-object payload', async () => { + await withTempDir(async (dir) => { + const accessors = makeAccessors(dir); + fs.writeFileSync(path.join(dir, 'settings.json'), '"just a string"'); + await expect(accessors.readSettingsStrict()).rejects.toThrow(/non-object payload/); + }); + }); + + it('strict read treats only a genuinely missing file as no settings', async () => { + await withTempDir(async (dir) => { + const accessors = makeAccessors(dir); + expect(await accessors.readSettingsStrict()).toEqual({}); + }); + }); + + it('does not regenerate the relay identity off a corrupt settings file', async () => { + await withTempDir(async (dir) => { + const accessors = makeAccessors(dir); + fs.writeFileSync( + path.join(dir, 'settings.json'), + JSON.stringify({ + relaySigningKey: { + privateJwk: crypto.generateKeyPairSync('ec', { namedCurve: 'P-256' }).privateKey.export({ format: 'jwk' }), + publicJwk: crypto.generateKeyPairSync('ec', { namedCurve: 'P-256' }).publicKey.export({ format: 'jwk' }), + }, + }), + ); + const identity = await createRelayIdentityRuntime({ crypto, ...accessors }).getRelayIdentity(); + const serverIdBefore = identity.serverId; + + // Corrupt the file, then ask for the identity again: the strict gate must + // make this FAIL rather than mint a replacement keypair. + fs.writeFileSync(path.join(dir, 'settings.json'), '{"relaySigningKey": {"unfinished'); + await expect(createRelayIdentityRuntime({ crypto, ...accessors }).getRelayIdentity()).rejects.toThrow(); + expect(serverIdBefore).toBeTruthy(); + }); + }); +}); diff --git a/packages/web/bin/lib/commands-connect-url.js b/packages/web/bin/lib/commands-connect-url.js index 2fe84f7e..a050b396 100644 --- a/packages/web/bin/lib/commands-connect-url.js +++ b/packages/web/bin/lib/commands-connect-url.js @@ -17,6 +17,7 @@ import { createClientPairingRuntime } from '../../server/lib/client-auth/pairing import { createRelayIdentityRuntime } from '../../server/lib/relay/identity.js'; import { DEFAULT_RELAY_URL } from '../../server/lib/relay/service.js'; import { bytesToBase64Url } from '../../server/lib/relay/e2ee.js'; +import { createSettingsAccessors as createSettingsAccessorsModule } from './cli-settings-accessors.js'; import { intro as clackIntro, outro as clackOutro, @@ -28,7 +29,6 @@ import { } from '../cli-output.js'; const REMOTE_CLIENTS_FILE_NAME = 'remote-clients.json'; -const SETTINGS_FILE_NAME = 'settings.json'; const PAIRING_SESSIONS_FILE_NAME = 'client-pairing-sessions.json'; function isValidRelayUrl(value) { @@ -55,20 +55,18 @@ function resolveRelayUrl(settings) { // Minimal settings.json read/write for the relay identity runtime. It reads the // whole object and writes it back with the relay keys added, so other settings // are preserved. Enough for the CLI without wiring the full settings runtime. +// +// Mirrors the settings runtime's guarantees: atomic writes (tmp + rename) so +// concurrent readers in the running app never observe a half-written file, and +// a STRICT reader gating relay identity regeneration so a swallowed read +// failure can never mint a new serverId and orphan paired devices. function createSettingsAccessors() { - const settingsPath = path.join(getOpenChamberDataDir(), SETTINGS_FILE_NAME); - const readSettingsFromDiskMigrated = async () => { - try { - return JSON.parse(await fs.promises.readFile(settingsPath, 'utf8')); - } catch { - return {}; - } - }; - const writeSettingsToDisk = async (settings) => { - await fs.promises.mkdir(path.dirname(settingsPath), { recursive: true }); - await fs.promises.writeFile(settingsPath, JSON.stringify(settings, null, 2), 'utf8'); - }; - return { readSettingsFromDiskMigrated, writeSettingsToDisk }; + return createSettingsAccessorsModule({ + fsPromises: fs.promises, + path, + dataDir: getOpenChamberDataDir(), + settingsFileName: 'settings.json', + }); } // Resolves the instance's relay identity (serverId + encryption public key,