diff --git a/scripts/lib/atomic-write.js b/scripts/lib/atomic-write.js index e3d41df0d..9e9e524fe 100644 --- a/scripts/lib/atomic-write.js +++ b/scripts/lib/atomic-write.js @@ -13,21 +13,34 @@ function writeFileAtomic(filePath, content, options = {}) { ); const mode = options.mode || 0o600; + if (options.validateParent) options.validateParent(); fs.mkdirSync(parentDir, { recursive: true }); let descriptor; try { + if (options.validateParent) options.validateParent(); descriptor = fs.openSync(tempPath, 'wx', mode); + if (options.validateParent) options.validateParent(); fs.writeFileSync(descriptor, content, { encoding: options.encoding || 'utf8' }); fs.fsyncSync(descriptor); fs.closeSync(descriptor); descriptor = undefined; + if (options.validateParent) options.validateParent(); + if (options.beforeRename) options.beforeRename(); fs.renameSync(tempPath, resolvedPath); } catch (error) { if (descriptor !== undefined) { fs.closeSync(descriptor); } - fs.rmSync(tempPath, { force: true }); + // If the parent was replaced, this pathname may now name somebody else's + // file. Leave the private staging file in its original directory. + let parentUnchanged = true; + try { + if (options.validateParent) options.validateParent(); + } catch (_error) { + parentUnchanged = false; + } + if (parentUnchanged) fs.rmSync(tempPath, { force: true }); throw error; } diff --git a/scripts/lib/install/claude-settings.js b/scripts/lib/install/claude-settings.js index dd34dd6fb..3c18668a6 100644 --- a/scripts/lib/install/claude-settings.js +++ b/scripts/lib/install/claude-settings.js @@ -389,9 +389,23 @@ function assertSettingsSnapshotUnchanged(settingsPath, snapshot) { function updateSettingsAtomic(settingsPath, transform, options = {}) { const update = () => { + const parentPath = path.dirname(path.resolve(settingsPath)); + const parentStats = fs.lstatSync(parentPath, { bigint: true }); + const validateParent = () => { + const current = fs.lstatSync(parentPath, { bigint: true }); + if ( + !current.isDirectory() || current.isSymbolicLink() + || current.dev !== parentStats.dev || current.ino !== parentStats.ino + ) { + const error = new Error(`Claude settings parent directory changed: ${parentPath}`); + error.code = 'ECC_SETTINGS_PARENT_CHANGED'; + throw error; + } + }; const maxAttempts = options.maxAttempts || 3; for (let attempt = 1; attempt <= maxAttempts; attempt += 1) { try { + validateParent(); const snapshot = readSettingsSnapshot(settingsPath); const result = transform(snapshot.settings); if (typeof options.beforeCommit === 'function') options.beforeCommit(); @@ -399,7 +413,14 @@ function updateSettingsAtomic(settingsPath, transform, options = {}) { writeFileAtomic( settingsPath, `${JSON.stringify(result.settings, null, 2)}\n`, - { encoding: 'utf8', mode: snapshot.mode } + { + encoding: 'utf8', + mode: snapshot.mode, + validateParent, + beforeRename() { + assertSettingsSnapshotUnchanged(settingsPath, snapshot); + }, + } ); return result; } catch (error) { diff --git a/tests/lib/claude-settings.test.js b/tests/lib/claude-settings.test.js index c31f191f9..26d094fc4 100644 --- a/tests/lib/claude-settings.test.js +++ b/tests/lib/claude-settings.test.js @@ -48,6 +48,67 @@ function clone(value) { return JSON.parse(JSON.stringify(value)); } +function assertAtomicParentReplacementRejected(stage) { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'claude-settings-parent-race-')); + const targetRoot = path.join(tempDir, 'target'); + const parkedRoot = path.join(tempDir, 'parked'); + const victimRoot = path.join(tempDir, 'victim'); + const settingsPath = path.join(targetRoot, 'settings.json'); + const victimPath = path.join(victimRoot, 'settings.json'); + const originalOpen = fs.openSync; + const originalFsync = fs.fsyncSync; + const targetContent = '{"target":true}\n'; + const victimContent = '{"victim":"preserve"}\n'; + let tempDescriptor; + let tempBasename; + let replaced = false; + const replaceParent = () => { + replaced = true; + fs.renameSync(targetRoot, parkedRoot); + fs.symlinkSync(victimRoot, targetRoot, process.platform === 'win32' ? 'junction' : 'dir'); + // A colliding path in the replacement directory must survive error cleanup. + fs.writeFileSync(path.join(victimRoot, tempBasename), 'unrelated replacement file'); + }; + try { + fs.mkdirSync(targetRoot); + fs.mkdirSync(victimRoot); + fs.writeFileSync(settingsPath, targetContent); + fs.writeFileSync(victimPath, victimContent); + fs.openSync = function(file, flags, ...args) { + const isTemp = typeof file === 'string' + && path.basename(file).startsWith('.settings.json.') && file.endsWith('.tmp'); + if (isTemp) tempBasename = path.basename(file); + if (isTemp && !replaced && stage === 'open') { + // Replace immediately after the temporary descriptor has been created. + const descriptor = originalOpen.call(fs, file, flags, ...args); + tempDescriptor = descriptor; + replaceParent(); + return descriptor; + } + const descriptor = originalOpen.call(fs, file, flags, ...args); + if (isTemp) tempDescriptor = descriptor; + return descriptor; + }; + fs.fsyncSync = function(descriptor) { + const result = originalFsync.call(fs, descriptor); + if (!replaced && stage === 'rename' && descriptor === tempDescriptor) replaceParent(); + return result; + }; + assert.throws( + () => updateSettingsAtomic(settingsPath, settings => ({ settings: { ...settings, managed: true } })), + /parent.*changed|changed.*parent/i + ); + assert.ok(replaced, 'must exercise a replacement inside the atomic writer'); + assert.strictEqual(fs.readFileSync(victimPath, 'utf8'), victimContent); + assert.strictEqual(fs.readFileSync(path.join(parkedRoot, 'settings.json'), 'utf8'), targetContent); + assert.strictEqual(fs.readFileSync(path.join(victimRoot, tempBasename), 'utf8'), 'unrelated replacement file'); + } finally { + fs.openSync = originalOpen; + fs.fsyncSync = originalFsync; + fs.rmSync(tempDir, { recursive: true, force: true }); + } +} + function runTests() { console.log('\n=== Testing install/claude-settings.js ===\n'); @@ -224,6 +285,40 @@ function runTests() { } })) passed++; else failed++; + for (const stage of ['open', 'rename']) { + if (test(`atomic settings updates reject parent replacement at ${stage} without touching its files`, () => { + assertAtomicParentReplacementRejected(stage); + })) passed++; else failed++; + } + + if (test('atomic settings updates preserve edits made while the replacement file is staged', () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'claude-settings-late-edit-')); + const settingsPath = path.join(tempDir, 'settings.json'); + const originalFsync = fs.fsyncSync; + let changed = false; + let fsyncCalls = 0; + try { + fs.writeFileSync(settingsPath, '{"theme":"initial"}\n'); + fs.fsyncSync = function(descriptor) { + const result = originalFsync.call(fs, descriptor); + // Lock creation is the first fsync; only change settings after the + // atomic writer has staged its first replacement payload. + fsyncCalls += 1; + if (!changed && fsyncCalls === 2) { + changed = true; + fs.writeFileSync(settingsPath, '{"theme":"late-edit"}\n'); + } + return result; + }; + updateSettingsAtomic(settingsPath, settings => ({ settings: { ...settings, managed: true } })); + assert.ok(changed); + assert.deepStrictEqual(JSON.parse(fs.readFileSync(settingsPath, 'utf8')), { theme: 'late-edit', managed: true }); + } finally { + fs.fsyncSync = originalFsync; + fs.rmSync(tempDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + if (test('atomic settings updates recover a stale invalid lock after its lease', () => { const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'claude-settings-stale-lock-')); const settingsPath = path.join(tempDir, 'settings.json');