fix(install): pin Claude settings parent during atomic replacement

Reject directory replacement after temporary file creation or staging, preserve unrelated files during cleanup, and retry settings edits observed before the final rename. Add three regression tests for the review findings.
This commit is contained in:
haelyra
2026-09-07 16:31:33 -04:00
parent f59cfd57c2
commit dbe8bfbba9
3 changed files with 131 additions and 2 deletions
+14 -1
View File
@@ -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;
}
+22 -1
View File
@@ -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) {
+95
View File
@@ -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');