fix: pin Hookify rules before path inspection

Open rule files before trusting path metadata, compare the descriptor identity to the live path, and only then validate containment and read bytes. Replace the unsafe temp-file race simulation with an open-first descriptor/path-swap regression.
This commit is contained in:
Affaan Mustafa
2026-07-26 05:16:06 -04:00
parent 504261d069
commit 396fadc784
2 changed files with 63 additions and 47 deletions
+43 -39
View File
@@ -355,6 +355,31 @@ function loadRuleFile({
const filePath = path.join(claudeDir, fileName);
let fileDescriptor;
try {
const noFollow = fs.constants.O_NOFOLLOW || 0;
try {
fileDescriptor = fs.openSync(filePath, fs.constants.O_RDONLY | noFollow);
} catch (error) {
if (
error &&
['ELOOP', 'EMLINK', 'ENOENT', 'ENOTDIR'].includes(error.code)
) {
return {
rule: null,
diagnostic: diagnostic('HOOKIFY_RULE_FILE_UNSAFE', fileName, 'not a regular file'),
bytesRead: 0,
};
}
throw error;
}
const fileStat = fs.fstatSync(fileDescriptor);
if (!fileStat.isFile()) {
return {
rule: null,
diagnostic: diagnostic('HOOKIFY_RULE_FILE_UNSAFE', fileName, 'not a regular file'),
bytesRead: 0,
};
}
const linkStat = fs.lstatSync(filePath);
if (linkStat.isSymbolicLink() || !linkStat.isFile()) {
return {
@@ -363,6 +388,23 @@ function loadRuleFile({
bytesRead: 0,
};
}
if (
fileStat.dev !== linkStat.dev ||
fileStat.ino !== linkStat.ino ||
fileStat.mode !== linkStat.mode ||
fileStat.size !== linkStat.size ||
fileStat.mtimeMs !== linkStat.mtimeMs
) {
return {
rule: null,
diagnostic: diagnostic(
'HOOKIFY_RULE_FILE_UNSAFE',
fileName,
'rule identity changed during evaluation'
),
bytesRead: 0,
};
}
const realDirectory = fs.realpathSync(claudeDir);
if (expectedRealDirectory && realDirectory !== expectedRealDirectory) {
@@ -380,55 +422,17 @@ function loadRuleFile({
if (
path.dirname(realFile) !== realDirectory ||
realFile !== path.join(realDirectory, fileName)
) {
return {
rule: null,
diagnostic: diagnostic('HOOKIFY_RULE_FILE_UNSAFE', fileName, 'resolved outside project .claude'),
bytesRead: 0,
};
}
const noFollow = fs.constants.O_NOFOLLOW || 0;
fileDescriptor = fs.openSync(filePath, fs.constants.O_RDONLY | noFollow);
const fileStat = fs.fstatSync(fileDescriptor);
if (
fileStat.dev !== linkStat.dev ||
fileStat.ino !== linkStat.ino ||
fileStat.mode !== linkStat.mode ||
fileStat.size !== linkStat.size ||
fileStat.mtimeMs !== linkStat.mtimeMs
) {
return {
rule: null,
diagnostic: diagnostic(
'HOOKIFY_RULE_FILE_UNSAFE',
fileName,
'rule identity changed during evaluation'
'resolved outside project .claude'
),
bytesRead: 0,
};
}
if (
fs.realpathSync(claudeDir) !== realDirectory ||
fs.realpathSync(filePath) !== realFile
) {
return {
rule: null,
diagnostic: diagnostic(
'HOOKIFY_RULE_FILE_UNSAFE',
fileName,
'rule path changed during evaluation'
),
bytesRead: 0,
};
}
if (!fileStat.isFile()) {
return {
rule: null,
diagnostic: diagnostic('HOOKIFY_RULE_FILE_UNSAFE', fileName, 'not a regular file'),
bytesRead: 0,
};
}
if (fileStat.size > LIMITS.maxFileBytes) {
return {
rule: null,
+20 -8
View File
@@ -210,7 +210,7 @@ function runTests() {
});
})) passed++; else failed++;
if (test('rejects a rule path swapped through a symlink while the file is opened', () => {
if (test('opens a rule descriptor before path inspection and rejects a later symlink swap', () => {
withProject(({ projectRoot, claudeDir }) => {
const fileName = 'hookify.race.local.md';
const rulePath = path.join(claudeDir, fileName);
@@ -231,19 +231,28 @@ function runTests() {
'This outside rule must never be loaded.',
].join('\n'));
const originalOpenSync = fs.openSync;
const originalFstatSync = fs.fstatSync;
const originalLstatSync = fs.lstatSync;
let descriptorOpened = false;
let inspectedBeforeOpen = false;
let swapped = false;
fs.openSync = function openWithSwap(target, flags, ...args) {
if (!swapped && target === rulePath) {
fs.fstatSync = function markDescriptorOpen(descriptor, ...args) {
descriptorOpened = true;
return originalFstatSync.call(fs, descriptor, ...args);
};
fs.lstatSync = function inspectWithSwap(target, ...args) {
if (target === rulePath && !descriptorOpened) {
inspectedBeforeOpen = true;
} else if (!swapped && target === rulePath) {
swapped = true;
fs.renameSync(rulePath, backupPath);
fs.symlinkSync(outsidePath, rulePath);
const descriptor = originalOpenSync.call(fs, target, fs.constants.O_RDONLY, ...args);
const linkStat = originalLstatSync.call(fs, target, ...args);
fs.unlinkSync(rulePath);
fs.renameSync(backupPath, rulePath);
return descriptor;
return linkStat;
}
return originalOpenSync.call(fs, target, flags, ...args);
return originalLstatSync.call(fs, target, ...args);
};
try {
@@ -255,8 +264,11 @@ function runTests() {
});
assert.strictEqual(result.rule, null);
assert.strictEqual(result.diagnostic.code, 'HOOKIFY_RULE_FILE_UNSAFE');
assert.strictEqual(inspectedBeforeOpen, false);
assert.strictEqual(swapped, true);
} finally {
fs.openSync = originalOpenSync;
fs.fstatSync = originalFstatSync;
fs.lstatSync = originalLstatSync;
if (fs.existsSync(backupPath) && !fs.existsSync(rulePath)) {
fs.renameSync(backupPath, rulePath);
}