diff --git a/src/main/daemon/history-reader-memory.test.ts b/src/main/daemon/history-reader-memory.test.ts index 9a96e4e7cf8..cecb29b1bb8 100644 --- a/src/main/daemon/history-reader-memory.test.ts +++ b/src/main/daemon/history-reader-memory.test.ts @@ -9,7 +9,7 @@ import { } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' -import { afterEach, describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it } from 'vitest' import { HistoryReader } from './history-reader' import { getHistorySessionDirName } from './history-paths' import { @@ -17,10 +17,7 @@ import { TERMINAL_HISTORY_LOG_MAX_BYTES, TERMINAL_HISTORY_META_MAX_BYTES } from './terminal-history-file-limits' -import { - readTerminalHistoryJson, - TERMINAL_HISTORY_JSON_MAX_STRUCTURAL_TOKENS -} from './terminal-history-file-reader' +import { readTerminalHistoryJson } from './terminal-history-file-reader' const RETIRED_CHECKPOINT_READ_CAP_BYTES = 16 * 1024 * 1024 @@ -120,21 +117,54 @@ describe('terminal history restore memory limits', () => { ).toBe(false) }) - it('rejects structurally amplified checkpoints before parsing', () => { - const { sessionPath } = createSession('amplified-checkpoint') + // Why: the retired 1M-structural-token pre-scan was reachable by ordinary content — + // ~100k OSC-8 hyperlinks cross it, the assert threw, detectColdRestore swallowed it, + // and the terminal restored blank. Same user-visible loss as the retired byte cap. + it('restores a checkpoint carrying more OSC links than the retired structural-token cap', async () => { + const { basePath, sessionPath } = createSession('many-osc-links') + const oscLinks = Array.from({ length: 150_000 }, (_, index) => ({ + row: index, + startCol: 0, + endCol: 40, + uri: `https://example.com/build/${index}` + })) + writeFileSync(join(sessionPath, 'checkpoint.json'), checkpoint({ oscLinks })) + + const restore = await new HistoryReader(basePath).detectColdRestore('many-osc-links') + expect(restore?.snapshotAnsi).toBe('safe checkpoint') + expect(restore?.oscLinks).toHaveLength(oscLinks.length) + }) + + // Why assert the reader directly too: detectColdRestore hides any reader throw as a + // null checkpoint, so a reinstated pre-scan would stay invisible above. + it('parses link-dense checkpoints without a structural pre-scan budget', () => { + const { sessionPath } = createSession('link-dense-checkpoint') const checkpointPath = join(sessionPath, 'checkpoint.json') + writeFileSync(checkpointPath, `{"snapshotAnsi":"","values":[${'0,'.repeat(2_000_000)}0]}`) + expect( + readTerminalHistoryJson<{ values: number[] }>( + checkpointPath, + TERMINAL_HISTORY_CHECKPOINT_MAX_BYTES + ).values + ).toHaveLength(2_000_001) + }) + + // Why: the pre-scan also carried a 128-level nesting cap, and dropping it is only safe + // because V8 parses JSON iteratively — depth costs heap, not stack. A future engine that + // recursed would abort the daemon rather than throw into the callers' catch, so pin it. + it('parses deeply nested checkpoints without the retired nesting-depth cap', () => { + const { sessionPath } = createSession('deeply-nested-checkpoint') + const checkpointPath = join(sessionPath, 'checkpoint.json') + const depth = 50_000 writeFileSync( checkpointPath, - `{"snapshotAnsi":"","values":[${'0,'.repeat(TERMINAL_HISTORY_JSON_MAX_STRUCTURAL_TOKENS)}0]}` + `{"snapshotAnsi":"","nested":${'['.repeat(depth)}${']'.repeat(depth)}}` ) - const parseSpy = vi.spyOn(JSON, 'parse') - try { - expect(() => - readTerminalHistoryJson(checkpointPath, TERMINAL_HISTORY_CHECKPOINT_MAX_BYTES) - ).toThrow(/JSON structure exceeds/) - expect(parseSpy).not.toHaveBeenCalled() - } finally { - parseSpy.mockRestore() - } + expect( + readTerminalHistoryJson<{ nested: unknown[] }>( + checkpointPath, + TERMINAL_HISTORY_CHECKPOINT_MAX_BYTES + ).nested + ).toHaveLength(1) }) }) diff --git a/src/main/daemon/terminal-history-file-reader.ts b/src/main/daemon/terminal-history-file-reader.ts index 3d273e95b9e..237eca57bdd 100644 --- a/src/main/daemon/terminal-history-file-reader.ts +++ b/src/main/daemon/terminal-history-file-reader.ts @@ -2,10 +2,6 @@ import { readNodeFileSyncWithinLimit, readNodeFileWithinLimit } from '../../shared/node-bounded-file-reader' -import { assertJsonTextStructureWithinLimits } from '../../shared/json-text-structure-limit' - -export const TERMINAL_HISTORY_JSON_MAX_STRUCTURAL_TOKENS = 1_000_000 -export const TERMINAL_HISTORY_JSON_MAX_NESTING_DEPTH = 128 export function readTerminalHistoryBuffer(filePath: string, maxBytes: number): Buffer { return readNodeFileSyncWithinLimit(filePath, maxBytes).buffer @@ -15,17 +11,16 @@ export function readTerminalHistoryText(filePath: string, maxBytes: number): str return readTerminalHistoryBuffer(filePath, maxBytes).toString('utf8') } +// Why no JSON structure pre-scan here: checkpoint.json and meta.json are our own +// writer's output, not untrusted input — the byte cap still bounds the read and a +// corrupt file fails JSON.parse into every caller's existing catch. Untrusted JSON +// still goes through assertJsonTextStructureWithinLimits. export function readTerminalHistoryJson(filePath: string, maxBytes: number): T { - const text = readTerminalHistoryText(filePath, maxBytes) - assertJsonTextStructureWithinLimits(text, { - structuralTokens: TERMINAL_HISTORY_JSON_MAX_STRUCTURAL_TOKENS, - nestingDepth: TERMINAL_HISTORY_JSON_MAX_NESTING_DEPTH - }) - return JSON.parse(text) as T + return JSON.parse(readTerminalHistoryText(filePath, maxBytes)) as T } // Why: cold-restore payload reads must not block the main thread, but need the -// same byte and JSON-structure bounds as the sync readers. +// same byte bound as the sync readers. export async function readTerminalHistoryBufferAsync( filePath: string, maxBytes: number @@ -44,10 +39,5 @@ export async function readTerminalHistoryJsonAsync( filePath: string, maxBytes: number ): Promise { - const text = await readTerminalHistoryTextAsync(filePath, maxBytes) - assertJsonTextStructureWithinLimits(text, { - structuralTokens: TERMINAL_HISTORY_JSON_MAX_STRUCTURAL_TOKENS, - nestingDepth: TERMINAL_HISTORY_JSON_MAX_NESTING_DEPTH - }) - return JSON.parse(text) as T + return JSON.parse(await readTerminalHistoryTextAsync(filePath, maxBytes)) as T }