From 233e01847cef565f6f122c7c91d78f325af53e06 Mon Sep 17 00:00:00 2001 From: iliya Date: Tue, 7 Apr 2026 02:04:28 +0300 Subject: [PATCH] fix(team): harden bootstrap state reads --- .../services/team/TeamBootstrapStateReader.ts | 194 ++++++++++++------ .../team/TeamBootstrapStateReader.test.ts | 88 +++++++- 2 files changed, 217 insertions(+), 65 deletions(-) diff --git a/src/main/services/team/TeamBootstrapStateReader.ts b/src/main/services/team/TeamBootstrapStateReader.ts index 3df80feb..04dedc5a 100644 --- a/src/main/services/team/TeamBootstrapStateReader.ts +++ b/src/main/services/team/TeamBootstrapStateReader.ts @@ -84,6 +84,89 @@ type BootstrapRuntimePhase = | 'failed' | 'canceled'; +type ComparableStat = { + dev?: number; + ino?: number; + size: number; + mode?: number; + mtimeMs?: number; +}; + +function isFiniteNumber(value: unknown): value is number { + return typeof value === 'number' && Number.isFinite(value); +} + +function sameFiniteNumber(a: unknown, b: unknown): boolean { + return isFiniteNumber(a) && isFiniteNumber(b) && a === b; +} + +function didValidatedFileChange(expected: ComparableStat, actual: ComparableStat): boolean { + const comparableIdentity = + isFiniteNumber(expected.dev) && + isFiniteNumber(actual.dev) && + isFiniteNumber(expected.ino) && + isFiniteNumber(actual.ino); + if (comparableIdentity) { + return expected.dev !== actual.dev || expected.ino !== actual.ino; + } + + if (sameFiniteNumber(expected.dev, actual.dev) && sameFiniteNumber(expected.ino, actual.ino)) { + return false; + } + + return ( + expected.size !== actual.size || + expected.mode !== actual.mode || + expected.mtimeMs !== actual.mtimeMs + ); +} + +async function readBoundRegularUtf8File( + targetPath: string, + maxBytes: number, + messages: { + notRegular: string; + oversized: string; + invalid: string; + } +): Promise<{ contents: string } | { issue: string } | null> { + try { + const validated = await fs.promises.lstat(targetPath); + if (validated.isSymbolicLink() || !validated.isFile()) { + return { issue: messages.notRegular }; + } + if (validated.size > maxBytes) { + return { issue: messages.oversized }; + } + + let handle: fs.promises.FileHandle; + try { + handle = await fs.promises.open(targetPath, 'r'); + } catch { + return { issue: messages.invalid }; + } + try { + const opened = await handle.stat(); + if (!opened.isFile() || didValidatedFileChange(validated, opened)) { + return { issue: messages.invalid }; + } + if (opened.size > maxBytes) { + return { issue: messages.oversized }; + } + return { + contents: await handle.readFile({ encoding: 'utf8' }), + }; + } finally { + await handle.close().catch(() => undefined); + } + } catch (error) { + if ((error as NodeJS.ErrnoException | undefined)?.code === 'ENOENT') { + return null; + } + return { issue: messages.invalid }; + } +} + function isBootstrapPhaseTerminal(phase: BootstrapRuntimePhase): boolean { return phase === 'completed' || phase === 'failed' || phase === 'canceled'; } @@ -136,30 +219,25 @@ function classifyBootstrapOwnerState(raw: RawBootstrapState): { async function inspectBootstrapState(teamName: string): Promise { const targetPath = getTeamBootstrapStatePath(teamName); + const file = await readBoundRegularUtf8File(targetPath, MAX_BOOTSTRAP_STATE_BYTES, { + notRegular: + 'Persisted deterministic bootstrap state is unreadable because bootstrap-state.json is a symlink or not a regular file.', + oversized: + 'Persisted deterministic bootstrap state is unreadable because bootstrap-state.json is oversized.', + invalid: + 'Persisted deterministic bootstrap state is unreadable because bootstrap-state.json is invalid, truncated, inaccessible, or changed while being read.', + }); + if (!file) { + return { raw: null }; + } + if ('issue' in file) { + return { + raw: null, + issue: file.issue, + }; + } try { - const stat = await fs.promises.lstat(targetPath); - if (stat.isSymbolicLink()) { - return { - raw: null, - issue: - 'Persisted deterministic bootstrap state is unreadable because bootstrap-state.json is a symlink.', - }; - } - if (!stat.isFile()) { - return { - raw: null, - issue: - 'Persisted deterministic bootstrap state is unreadable because bootstrap-state.json is not a regular file.', - }; - } - if (stat.size > MAX_BOOTSTRAP_STATE_BYTES) { - return { - raw: null, - issue: - 'Persisted deterministic bootstrap state is unreadable because bootstrap-state.json is oversized.', - }; - } - const raw = JSON.parse(await fs.promises.readFile(targetPath, 'utf8')) as RawBootstrapState; + const raw = JSON.parse(file.contents) as RawBootstrapState; if (raw.version !== 1) { return { raw: null, @@ -168,14 +246,11 @@ async function inspectBootstrapState(teamName: string): Promise { const targetPath = getTeamBootstrapLockMetadataPath(teamName); + const file = await readBoundRegularUtf8File(targetPath, MAX_BOOTSTRAP_LOCK_METADATA_BYTES, { + notRegular: '', + oversized: '', + invalid: '', + }); + if (!file || 'issue' in file) { + return null; + } try { - const stat = await fs.promises.lstat(targetPath); - if (stat.isSymbolicLink() || !stat.isFile() || stat.size > MAX_BOOTSTRAP_LOCK_METADATA_BYTES) { - return null; - } - const raw = JSON.parse( - await fs.promises.readFile(targetPath, 'utf8') - ) as RawBootstrapLockMetadata; + const raw = JSON.parse(file.contents) as RawBootstrapLockMetadata; if ( typeof raw.pid !== 'number' || !Number.isFinite(raw.pid) || @@ -373,28 +450,24 @@ async function readBootstrapJournalWarnings(teamName: string): Promise { const targetPath = getTeamBootstrapJournalPath(teamName); + const file = await readBoundRegularUtf8File(targetPath, MAX_BOOTSTRAP_JOURNAL_BYTES, { + notRegular: + 'Persisted deterministic bootstrap journal is unreadable because bootstrap-journal.jsonl is a symlink or not a regular file.', + oversized: + 'Persisted deterministic bootstrap journal is unreadable because bootstrap-journal.jsonl is oversized.', + invalid: + 'Persisted deterministic bootstrap journal is unreadable because bootstrap-journal.jsonl is invalid, truncated, inaccessible, or changed while being read.', + }); + if (!file) { + return {}; + } + if ('issue' in file) { + return { + issue: file.issue, + }; + } try { - const stat = await fs.promises.lstat(targetPath); - if (stat.isSymbolicLink()) { - return { - issue: - 'Persisted deterministic bootstrap journal is unreadable because bootstrap-journal.jsonl is a symlink.', - }; - } - if (!stat.isFile()) { - return { - issue: - 'Persisted deterministic bootstrap journal is unreadable because bootstrap-journal.jsonl is not a regular file.', - }; - } - if (stat.size > MAX_BOOTSTRAP_JOURNAL_BYTES) { - return { - issue: - 'Persisted deterministic bootstrap journal is unreadable because bootstrap-journal.jsonl is oversized.', - }; - } - - const raw = await fs.promises.readFile(targetPath, 'utf8'); + const raw = file.contents; const lines = raw .split('\n') .map((line) => line.trim()) @@ -446,7 +519,7 @@ async function inspectBootstrapJournal(teamName: string): Promise 0 && messages.length === 0) { return { issue: - 'Persisted deterministic bootstrap journal is unreadable because bootstrap-journal.jsonl is invalid, truncated, or inaccessible.', + 'Persisted deterministic bootstrap journal is unreadable because bootstrap-journal.jsonl is invalid, truncated, inaccessible, or changed while being read.', }; } @@ -466,13 +539,10 @@ async function inspectBootstrapJournal(teamName: string): Promise { contents: string; size?: number; symbolicLink?: boolean; + ino?: number; + dev?: number; + mode?: number; + mtimeMs?: number; + openedContents?: string; + openedSize?: number; + openedIno?: number; + openedDev?: number; + openedMode?: number; + openedMtimeMs?: number; } >(); @@ -23,6 +33,10 @@ const hoisted = vi.hoisted(() => { isFile: () => !entry.symbolicLink, isSymbolicLink: () => Boolean(entry.symbolicLink), size: entry.size ?? Buffer.byteLength(entry.contents, 'utf8'), + ino: entry.ino ?? 1, + dev: entry.dev ?? 1, + mode: entry.mode ?? 0o100600, + mtimeMs: entry.mtimeMs ?? 1, }; }); @@ -36,6 +50,30 @@ const hoisted = vi.hoisted(() => { return entry.contents; }); + const open = vi.fn(async (filePath: string) => { + const entry = files.get(norm(filePath)); + if (!entry) { + const error = new Error('ENOENT') as NodeJS.ErrnoException; + error.code = 'ENOENT'; + throw error; + } + return { + stat: vi.fn(async () => ({ + isFile: () => !entry.symbolicLink, + size: + entry.openedSize ?? + entry.size ?? + Buffer.byteLength(entry.openedContents ?? entry.contents, 'utf8'), + ino: entry.openedIno ?? entry.ino ?? 1, + dev: entry.openedDev ?? entry.dev ?? 1, + mode: entry.openedMode ?? entry.mode ?? 0o100600, + mtimeMs: entry.openedMtimeMs ?? entry.mtimeMs ?? 1, + })), + readFile: vi.fn(async () => entry.openedContents ?? entry.contents), + close: vi.fn(async () => undefined), + }; + }); + const access = vi.fn(async (filePath: string) => { const entry = files.get(norm(filePath)); if (!entry) { @@ -49,7 +87,7 @@ const hoisted = vi.hoisted(() => { files.delete(norm(filePath)); }); - return { files, lstat, readFile, access, rm }; + return { files, lstat, open, readFile, access, rm }; }); vi.mock('fs', async (importOriginal) => { @@ -59,6 +97,7 @@ vi.mock('fs', async (importOriginal) => { promises: { ...actual.promises, lstat: hoisted.lstat, + open: hoisted.open, readFile: hoisted.readFile, access: hoisted.access, rm: hoisted.rm, @@ -81,6 +120,7 @@ describe('TeamBootstrapStateReader', () => { beforeEach(() => { hoisted.files.clear(); hoisted.lstat.mockClear(); + hoisted.open.mockClear(); hoisted.readFile.mockClear(); hoisted.access.mockClear(); hoisted.rm.mockClear(); @@ -169,7 +209,49 @@ describe('TeamBootstrapStateReader', () => { state: 'assembling', message: 'Spawning teammate runtimes (1)', warnings: [ - 'Persisted deterministic bootstrap journal is unreadable because bootstrap-journal.jsonl is invalid, truncated, or inaccessible.', + 'Persisted deterministic bootstrap journal is unreadable because bootstrap-journal.jsonl is invalid, truncated, inaccessible, or changed while being read.', + ], + }, + }); + + killSpy.mockRestore(); + }); + + it('treats bootstrap-state replacement during read as degraded recovery, not trusted truth', async () => { + const killSpy = vi.spyOn(process, 'kill').mockImplementation(() => true as never); + + hoisted.files.set('/mock/teams/demo/bootstrap-state.json', { + contents: JSON.stringify({ + version: 1, + runId: 'run-123', + teamName: 'demo', + ownerPid: 4242, + startedAt: 1700000000000, + updatedAt: 1700000000500, + phase: 'spawning_members', + members: [], + }), + ino: 1, + openedIno: 2, + }); + hoisted.files.set('/mock/teams/demo/.bootstrap.lock/metadata.json', { + contents: JSON.stringify({ + pid: 4242, + runId: 'run-123', + ownerStartedAt: 1700000000000, + }), + }); + + await expect(readBootstrapRuntimeState('demo')).resolves.toMatchObject({ + teamName: 'demo', + isAlive: false, + runId: 'run-123', + progress: { + state: 'configuring', + message: + 'Deterministic bootstrap recovery is degraded because persisted bootstrap state is unreadable', + warnings: [ + 'Persisted deterministic bootstrap state is unreadable because bootstrap-state.json is invalid, truncated, inaccessible, or changed while being read.', ], }, }); @@ -336,7 +418,7 @@ describe('TeamBootstrapStateReader', () => { messageSeverity: 'warning', pid: 4242, warnings: [ - 'Persisted deterministic bootstrap state is unreadable because bootstrap-state.json is invalid, truncated, or inaccessible.', + 'Persisted deterministic bootstrap state is unreadable because bootstrap-state.json is invalid, truncated, inaccessible, or changed while being read.', 'Recent deterministic bootstrap events: bootstrap phase: spawning_members | alice: spawn_started', ], },