diff --git a/packages/web/bin/lib/DOCUMENTATION.md b/packages/web/bin/lib/DOCUMENTATION.md index 24141432..a85d6a8d 100644 --- a/packages/web/bin/lib/DOCUMENTATION.md +++ b/packages/web/bin/lib/DOCUMENTATION.md @@ -78,6 +78,18 @@ These modules hold reusable, non-presentational logic for commands. - `cli-paths.js` - Data, run, log, settings, tunnel profile, and managed-local config paths. +- `cli-settings-accessors.js` + - Minimal settings.json read/write for CLI contexts that must not load the + full web settings runtime (`connect-url` relay identity resolution). + - Mirrors the settings runtime's guarantees so a CLI read-modify-write can + never corrupt shared state: atomic tmp+rename writes (no concurrent reader + in the running app can observe a torn file), a strict read that throws on + corrupt/unreadable payloads, and the same `0600` file mode. + - The strict read gates relay identity regeneration exactly like the server + runtime: a swallowed read failure can never mint a replacement signing or + encryption keypair, which would change `serverId` and orphan every paired + device and push binding. + - `cli-process.js` - PID files, instance registry files, process identity checks, runtime metadata checks, and process termination helpers. 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..50d415bd --- /dev/null +++ b/packages/web/bin/lib/cli-settings-accessors.js @@ -0,0 +1,103 @@ +// 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. Fall back to copying the COMPLETE tmp file + // so persistence never wedges. Note: copyFile is NOT atomic — this is a + // last-resort path confined to Windows, matching the settings runtime's + // fallback, not a substitute for the atomic rename used everywhere else. + 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..a48829fc --- /dev/null +++ b/packages/web/bin/lib/cli-settings-accessors.test.js @@ -0,0 +1,180 @@ +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, overrides = {}) => + createSettingsAccessors({ + fsPromises: fs.promises, + path, + dataDir: dir, + settingsFileName: 'settings.json', + ...overrides, + }); + +// Wraps writeFile so each write lands in two chunks with a pause in between — +// a stand-in for a large, slow write on a real disk (one open handle, so the +// file grows from the prefix to the full payload). With a non-atomic writer a +// concurrent reader deterministically catches the half-written file in that +// window; with the atomic tmp+rename writer the target only ever changes via a +// complete rename, so the window is never observable. +const makeSlowWriteFs = () => { + const realFs = fs.promises; + const slowWriteFile = async (filePath, data) => { + const handle = await realFs.open(filePath, 'w'); + try { + const half = Math.floor(data.length / 2); + await handle.writeFile(data.slice(0, half), 'utf8'); + await new Promise((resolve) => setTimeout(resolve, 30)); + await handle.writeFile(data.slice(half), 'utf8'); + } finally { + await handle.close(); + } + }; + return { slowWriteFile, fsPromises: { ...realFs, writeFile: slowWriteFile } }; +}; + +// Runs `writer` against filePath while a concurrent reader hammers it; returns +// how many times the reader observed an unparseable (torn) payload. ENOENT +// during the very first write is not a tear and is excluded. +const countTornReads = async (filePath, writer, iterations) => { + const big = { theme: 'dark', filler: 'x'.repeat(4096) }; + let torn = 0; + let stop = false; + const reader = (async () => { + while (!stop) { + try { + const parsed = JSON.parse(await fs.promises.readFile(filePath, 'utf8')); + if (parsed && typeof parsed === 'object') { + expect(parsed.theme).toBe('dark'); + } + } catch (error) { + if (error?.code !== 'ENOENT') { + torn += 1; + } + } + await new Promise((resolve) => setTimeout(resolve, 0)); + } + })(); + for (let i = 0; i < iterations; i += 1) { + await writer({ ...big, n: i }); + } + stop = true; + await reader; + return torn; +}; + +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('atomic writes: concurrent readers never observe a torn file, even under slow writes', async () => { + await withTempDir(async (dir) => { + const { fsPromises } = makeSlowWriteFs(); + const accessors = makeAccessors(dir, { fsPromises }); + const filePath = path.join(dir, 'settings.json'); + + // Each write is chunked with a pause, yet the reader must never see a + // partial payload: the target only changes via a complete atomic rename. + const torn = await countTornReads(filePath, (settings) => accessors.writeSettingsToDisk(settings), 20); + expect(torn).toBe(0); + + const leftovers = fs.readdirSync(dir).filter((name) => name.startsWith('settings.json.tmp-')); + expect(leftovers).toEqual([]); + }); + }); + + it('demonstrates the protected failure mode: a naive direct writer tears under the same slow write', async () => { + await withTempDir(async (dir) => { + const { slowWriteFile } = makeSlowWriteFs(); + const filePath = path.join(dir, 'settings.json'); + + // The old CLI accessor wrote straight to settings.json with writeFile. + // The same slow-write load therefore MUST produce torn reads — proving + // the concurrency test above can actually fail on the pre-fix writer. + const torn = await countTornReads( + filePath, + (settings) => slowWriteFile(filePath, JSON.stringify(settings)), + 20, + ); + expect(torn).toBeGreaterThan(0); + }); + }); + + 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,