From 61992f7f5e174515b3862112bcca48e337110d1b Mon Sep 17 00:00:00 2001 From: Affaan Mustafa Date: Thu, 30 Apr 2026 12:07:22 -0400 Subject: [PATCH] fix: harden loop-status snapshot writes --- scripts/loop-status.js | 23 +++++- tests/scripts/loop-status.test.js | 117 ++++++++++++++++++++++++------ 2 files changed, 117 insertions(+), 23 deletions(-) diff --git a/scripts/loop-status.js b/scripts/loop-status.js index cb728da6..6c7128b1 100644 --- a/scripts/loop-status.js +++ b/scripts/loop-status.js @@ -614,12 +614,20 @@ function hashString(value) { return crypto.createHash('sha256').update(String(value)).digest('hex'); } +function isWindowsReservedBasename(value) { + const basename = String(value).split('.')[0]; + return /^(con|prn|aux|nul|com[1-9]|lpt[1-9])$/i.test(basename); +} + function sanitizeSnapshotName(value, fallback = 'session') { const raw = String(value || '').trim() || fallback; const sanitized = raw.replace(/[^a-zA-Z0-9._-]/g, '_').replace(/^_+|_+$/g, ''); - if (sanitized && sanitized.length <= 96) { + if (sanitized && sanitized.length <= 96 && !isWindowsReservedBasename(sanitized)) { return sanitized; } + if (sanitized && isWindowsReservedBasename(sanitized)) { + return `${sanitized}-${hashString(raw).slice(0, 8)}`; + } const prefix = sanitized ? sanitized.slice(0, 48).replace(/[._-]+$/g, '') : fallback; return `${prefix || fallback}-${hashString(raw).slice(0, 12)}`; @@ -629,7 +637,18 @@ function atomicWriteJson(filePath, payload) { const data = JSON.stringify(payload, null, 2) + '\n'; const tempPath = `${filePath}.${process.pid}.${Date.now()}.${Math.random().toString(16).slice(2)}.tmp`; fs.writeFileSync(tempPath, data, 'utf8'); - fs.renameSync(tempPath, filePath); + try { + fs.renameSync(tempPath, filePath); + } catch (error) { + try { + fs.unlinkSync(tempPath); + } catch (cleanupError) { + if (cleanupError.code !== 'ENOENT') { + console.error(`[loop-status] WARNING: could not remove temporary snapshot file ${tempPath}: ${cleanupError.message}`); + } + } + throw error; + } } function getSnapshotPath(outputDir, session, usedNames) { diff --git a/tests/scripts/loop-status.test.js b/tests/scripts/loop-status.test.js index a57330e4..a3da4b89 100644 --- a/tests/scripts/loop-status.test.js +++ b/tests/scripts/loop-status.test.js @@ -6,10 +6,16 @@ const assert = require('assert'); const fs = require('fs'); const os = require('os'); const path = require('path'); -const { execFileSync } = require('child_process'); +const { spawnSync } = require('child_process'); const SCRIPT = path.join(__dirname, '..', '..', 'scripts', 'loop-status.js'); -const { analyzeTranscript, buildStatus, getStatusExitCode, parseArgs } = require('../../scripts/loop-status'); +const { + analyzeTranscript, + buildStatus, + getStatusExitCode, + parseArgs, + writeStatusSnapshots, +} = require('../../scripts/loop-status'); const NOW = '2026-04-30T10:00:00.000Z'; function run(args = [], options = {}) { @@ -25,25 +31,22 @@ function run(args = [], options = {}) { envOverrides.HOME = envOverrides.USERPROFILE; } - try { - const stdout = execFileSync('node', [SCRIPT, ...args], { - encoding: 'utf8', - stdio: ['pipe', 'pipe', 'pipe'], - timeout: 10000, - cwd: options.cwd || process.cwd(), - env: { - ...process.env, - ...envOverrides, - }, - }); - return { code: 0, stdout, stderr: '' }; - } catch (error) { - return { - code: error.status || 1, - stdout: error.stdout || '', - stderr: error.stderr || '', - }; - } + const result = spawnSync('node', [SCRIPT, ...args], { + encoding: 'utf8', + stdio: ['pipe', 'pipe', 'pipe'], + timeout: 10000, + cwd: options.cwd || process.cwd(), + env: { + ...process.env, + ...envOverrides, + }, + }); + + return { + code: result.status || (result.signal ? 1 : 0), + stdout: result.stdout || '', + stderr: result.stderr || '', + }; } function createTempHome() { @@ -631,6 +634,77 @@ function runTests() { } })) passed++; else failed++; + if (test('avoids Windows reserved basenames for session snapshots', () => { + const homeDir = createTempHome(); + const snapshotDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-loop-status-windows-name-')); + + try { + writeTranscript(homeDir, '-Users-affoon-project-windows-name', 'con.jsonl', [ + assistantMessage('2026-04-30T09:55:00.000Z', 'con', 'Loop checkpoint.'), + ]); + + const result = run([ + '--home', + homeDir, + '--now', + NOW, + '--json', + '--write-dir', + snapshotDir, + ]); + + assert.strictEqual(result.code, 0, result.stderr); + + const indexPath = path.join(snapshotDir, 'index.json'); + const indexPayload = JSON.parse(fs.readFileSync(indexPath, 'utf8')); + const snapshotName = path.basename(indexPayload.sessions[0].snapshotPath); + assert.strictEqual(indexPayload.sessions[0].sessionId, 'con'); + assert.notStrictEqual(snapshotName.toLowerCase(), 'con.json'); + + const snapshotPayload = JSON.parse(fs.readFileSync(indexPayload.sessions[0].snapshotPath, 'utf8')); + assert.strictEqual(snapshotPayload.schemaVersion, 'ecc.loop-status.session.v1'); + assert.strictEqual(snapshotPayload.session.sessionId, 'con'); + } finally { + fs.rmSync(homeDir, { recursive: true, force: true }); + fs.rmSync(snapshotDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + + if (test('cleans temporary snapshot files when atomic rename fails', () => { + const snapshotDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-loop-status-rename-failure-')); + const originalRenameSync = fs.renameSync; + + try { + fs.renameSync = () => { + throw new Error('simulated rename failure'); + }; + + assert.throws(() => writeStatusSnapshots({ + errors: [], + generatedAt: NOW, + sessions: [ + { + eventCount: 1, + lastEventAt: NOW, + pendingTools: [], + recommendedAction: 'No action needed.', + sessionId: 'rename-failure', + signals: [], + state: 'ok', + transcriptPath: path.join(snapshotDir, 'rename-failure.jsonl'), + }, + ], + source: {}, + }, snapshotDir), /simulated rename failure/); + + const tempFiles = fs.readdirSync(snapshotDir).filter(fileName => fileName.endsWith('.tmp')); + assert.deepStrictEqual(tempFiles, []); + } finally { + fs.renameSync = originalRenameSync; + fs.rmSync(snapshotDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + if (test('write-dir failures do not suppress normal stdout', () => { const homeDir = createTempHome(); @@ -655,6 +729,7 @@ function runTests() { const payload = parsePayload(result.stdout); assert.strictEqual(payload.schemaVersion, 'ecc.loop-status.v1'); assert.strictEqual(payload.sessions[0].sessionId, 'session-write-error'); + assert.match(result.stderr, /\[loop-status\] WARNING: could not write status snapshots:/); } finally { fs.rmSync(homeDir, { recursive: true, force: true }); }