fix: pin and bound Hookify rule reads

Open with supported no-follow and nonblocking flags before relying on file metadata. Validate descriptor, pathname and rule-directory consistency before and after reading at most64KiB plus one overflow byte. Reject observed replacement, mutation, premature EOF and non-regular files, and close descriptors on every outcome.

Add12 race and I/O regressions. Keep the Hermes uppercase protected-file fixture real on case-sensitive filesystems and verify that the installed guard still permits first-time creation.

Validation: full6688 reported tests across347 files,12 race tests, full80% coverage gates, scoped Hookify coverage, installed Hookify and Hermes workflows, packed artifact lifecycle, lint, signatures and IOC.

scripts/hooks/hookify-runtime.js

@@ -212,21 +212,60 @@ function normalizeRule(frontmatter, message, sourcePath) {
   });
 }
 
-function readRuleFile(filePath) {
-  const stat = fs.lstatSync(filePath);
-  if (stat.isSymbolicLink()) throw new Error('symbolic links are not loaded');
-  if (!stat.isFile()) throw new Error('rule path is not a regular file');
-  if (stat.size > MAX_RULE_BYTES) throw new Error('rule exceeds ' + MAX_RULE_BYTES + ' bytes');
+function sameRuleIdentity(left, right) {
+  return ['dev', 'ino', 'uid', 'gid', 'mode', 'nlink'].every(key => left[key] === right[key]);
+}
 
-  const noFollow = fs.constants.O_NOFOLLOW || 0;
-  const fd = fs.openSync(filePath, fs.constants.O_RDONLY | noFollow);
+function sameRuleSnapshot(left, right) {
+  return sameRuleIdentity(left, right)
+    && ['size', 'mtimeNs', 'ctimeNs'].every(key => left[key] === right[key]);
+}
+
+function assertRuleSnapshot(filePath, fd, openedStat, directoryStat) {
+  const directory = fs.lstatSync(path.dirname(filePath), { bigint: true });
+  const current = fs.lstatSync(filePath, { bigint: true });
+  if (directory.isSymbolicLink() || !directory.isDirectory()
+    || !sameRuleIdentity(directory, directoryStat)
+    || current.isSymbolicLink() || !current.isFile()
+    || !sameRuleSnapshot(current, openedStat)
+    || !sameRuleSnapshot(fs.fstatSync(fd, { bigint: true }), openedStat)) {
+    throw new Error('rule file or directory changed while reading');
+  }
+}
+
+function readBoundedRule(fd) {
+  const bytes = Buffer.alloc(MAX_RULE_BYTES + 1);
+  let total = 0;
+  while (total < bytes.length) {
+    const count = fs.readSync(fd, bytes, total, bytes.length - total, total);
+    if (count === 0) break;
+    total += count;
+  }
+  if (total > MAX_RULE_BYTES) throw new Error('rule exceeds ' + MAX_RULE_BYTES + ' bytes');
+  return bytes.subarray(0, total);
+}
+
+function readRuleFile(filePath, directoryStat) {
+  const flags = fs.constants.O_RDONLY | (fs.constants.O_NOFOLLOW || 0)
+    | (fs.constants.O_NONBLOCK || 0);
+  let fd;
   try {
-    const openedStat = fs.fstatSync(fd);
-    if (!openedStat.isFile() || openedStat.size > MAX_RULE_BYTES) {
-      throw new Error('rule exceeds ' + MAX_RULE_BYTES + ' bytes or is not a regular file');
+    fd = fs.openSync(filePath, flags);
+  } catch (error) {
+    if (error.code === 'ELOOP') throw new Error('symbolic links are not loaded');
+    throw error;
+  }
+  try {
+    const openedStat = fs.fstatSync(fd, { bigint: true });
+    if (!openedStat.isFile()) throw new Error('rule path is not a regular file');
+    if (openedStat.size > BigInt(MAX_RULE_BYTES)) {
+      throw new Error('rule exceeds ' + MAX_RULE_BYTES + ' bytes');
     }
-    const source = fs.readFileSync(fd, 'utf8');
-    const document = extractFrontmatter(source);
+    assertRuleSnapshot(filePath, fd, openedStat, directoryStat);
+    const bytes = readBoundedRule(fd);
+    assertRuleSnapshot(filePath, fd, openedStat, directoryStat);
+    if (BigInt(bytes.length) !== openedStat.size) throw new Error('rule size changed while reading');
+    const document = extractFrontmatter(bytes.toString('utf8'));
     if (!document) throw new Error('missing YAML frontmatter');
     const frontmatter = parseRuleFrontmatter(document.yaml);
     return normalizeRule(frontmatter, document.message, filePath);
@@ -241,9 +280,10 @@ function loadRules(projectRoot, options = {}) {
   const excludedPaths = options.excludedPaths || new Set();
   const rulesDir = path.join(path.resolve(projectRoot), '.claude');
   let entries;
+  let directoryStat;
   try {
-    const dirStat = fs.lstatSync(rulesDir);
-    if (dirStat.isSymbolicLink() || !dirStat.isDirectory()) {
+    directoryStat = fs.lstatSync(rulesDir, { bigint: true });
+    if (directoryStat.isSymbolicLink() || !directoryStat.isDirectory()) {
       return { rules, diagnostics: [diagnostic('.claude', 'rule directory must be a real directory')] };
     }
     entries = fs.readdirSync(rulesDir, { withFileTypes: true });
@@ -269,7 +309,7 @@ function loadRules(projectRoot, options = {}) {
   }
   for (const entry of eligibleCandidates.slice(0, MAX_RULES)) {
     try {
-      const rule = readRuleFile(path.join(rulesDir, entry.name));
+      const rule = readRuleFile(path.join(rulesDir, entry.name), directoryStat);
       if (rule.enabled) rules.push(rule);
     } catch (error) {
       diagnostics.push(diagnostic(entry.name, error.message));

tests/hermes-hooks.test.js

@@ -90,11 +90,11 @@ run('blocks edits to every protected config name', () => {
   }
 });
 
-run('blocks case-variant names (case-insensitive filesystems)', () => {
+run('blocks existing case-variant protected names on every filesystem', () => {
   const target = path.join(tmp, 'ESLINT.CONFIG.MJS');
-  fs.writeFileSync(path.join(tmp, 'eslint.config.mjs'), 'export default {}\n');
-  // On a case-sensitive filesystem the variant is a distinct name; the hook
-  // must still refuse it so a macOS/Windows rename cannot smuggle an edit.
+  fs.writeFileSync(target, 'export default {}\n');
+  // The exact variant must exist: first-time config creation is allowed.
+  assert.ok(fs.existsSync(target), 'the probed protected file must exist');
   const { stdout } = runHook(GUARD, {
     hook_event_name: 'pre_tool_call',
     tool_name: 'patch',
@@ -248,13 +248,21 @@ run('hermes-hooks module installs via the real installer and the landed guard bl
   assert.ok(fs.existsSync(landed), 'guard must land under ~/.hermes/hooks/hermes/');
   assert.ok(fs.existsSync(path.join(home, '.hermes', 'docs', 'HERMES-HOOKS.md')), 'doc must land');
   const t = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-guard-'));
-  const target = path.join(t, 'eslint.config.mjs');
+  const target = path.join(t, 'ESLINT.CONFIG.MJS');
   fs.writeFileSync(target, 'export default {}\n');
   const g = spawnSync('python3', [landed], {
     input: JSON.stringify({ hook_event_name: 'pre_tool_call', tool_name: 'patch', tool_input: { path: target, old_string: 'a', new_string: 'b' } }),
     encoding: 'utf8', timeout: 15000,
   });
   assert.ok((g.stdout || '').includes('"decision": "block"'), 'landed guard must block');
+  fs.unlinkSync(target);
+  const bootstrap = spawnSync('python3', [landed], {
+    input: JSON.stringify({ hook_event_name: 'pre_tool_call', tool_name: 'patch', tool_input: { path: target } }),
+    encoding: 'utf8', timeout: 15000,
+  });
+  assert.strictEqual(bootstrap.status, 0, bootstrap.stderr);
+  assert.strictEqual(bootstrap.stdout.trim(), '', 'first-time config creation remains allowed');
+
   fs.rmSync(home, { recursive: true, force: true });
   fs.rmSync(t, { recursive: true, force: true });
 });

tests/hooks/hookify-file-race.test.js

@@ -0,0 +1,202 @@
+'use strict';
+
+const assert = require('node:assert/strict');
+const fs = require('node:fs');
+const os = require('node:os');
+const path = require('node:path');
+const { Module, createRequire } = require('node:module');
+const { spawnSync } = require('node:child_process');
+const { test } = require('node:test');
+
+const runtimePath = path.resolve(__dirname, '../../scripts/hooks/hookify-runtime.js');
+const MAX_BYTES = 64 * 1024;
+const ruleText = name => '---\nname: ' + name + '\nevent: bash\naction: block\npattern: BLOCK_ME\n---\nLocal policy.\n';
+
+function fixture(t, options = {}) {
+  const root = fs.mkdtempSync(path.join(fs.realpathSync(os.tmpdir()), 'ecc-hookify-race-'));
+  t.after(() => fs.rmSync(root, { recursive: true, force: true }));
+  const directory = path.join(root, '.claude');
+  fs.mkdirSync(directory);
+  const file = path.join(directory, 'hookify.race.local.md');
+  fs.writeFileSync(file, ruleText('trusted'));
+  const descriptors = new Set();
+  const reads = [];
+  const flags = [];
+  const io = {
+    ...fs,
+    constants: { ...fs.constants, ...(options.noFollow === false ? { O_NOFOLLOW: 0 } : {}) },
+    openSync(name, openFlags, ...args) {
+      if (name === file && options.beforeOpen) options.beforeOpen({ root, directory, file });
+      if (options.requireNonblocking) assert.ok(openFlags & fs.constants.O_NONBLOCK);
+      const fd = fs.openSync(name, openFlags, ...args);
+      descriptors.add(fd);
+      flags.push(openFlags);
+      return fd;
+    },
+    readFileSync(fd, ...args) {
+      if (descriptors.has(fd) && options.beforeRead) options.beforeRead({ root, directory, file });
+      const result = fs.readFileSync(fd, ...args);
+      if (descriptors.has(fd)) reads.push(Buffer.byteLength(result));
+      return result;
+    },
+    readSync(fd, buffer, offset, length, position) {
+      if (options.beforeRead) options.beforeRead({ root, directory, file });
+      const size = fs.readSync(fd, buffer, offset, options.shortReads ? Math.min(7, length) : length, position);
+      reads.push(size);
+      return size;
+    },
+    closeSync(fd) {
+      fs.closeSync(fd);
+      descriptors.delete(fd);
+    },
+  };
+  const copy = new Module(runtimePath, module);
+  copy.filename = runtimePath;
+  copy.paths = Module._nodeModulePaths(path.dirname(runtimePath));
+  const localRequire = createRequire(runtimePath);
+  copy.require = name => name === 'fs' ? io : localRequire(name);
+  copy._compile(fs.readFileSync(runtimePath, 'utf8'), runtimePath);
+  return { root, directory, file, io, runtime: copy.exports, descriptors, reads, flags };
+}
+
+function rejected(result, context) {
+  assert.equal(result.rules.length, 0, 'raced content must not become a rule');
+  assert.equal(result.diagnostics.length, 1);
+  assert.match(result.diagnostics[0], /changed|symbolic|regular|exceeds|EIO/);
+  assert.equal(context.descriptors.size, 0, 'every opened rule descriptor must close');
+}
+
+test('growth after metadata validation cannot read over the rule ceiling plus one byte', t => {
+  let grown = false;
+  const context = fixture(t, { beforeRead({ file }) {
+    if (grown) return;
+    grown = true;
+    fs.appendFileSync(file, 'x'.repeat(MAX_BYTES * 4));
+  } });
+  rejected(context.runtime.loadRules(context.root), context);
+  assert.ok(grown, 'the real file must grow at the read boundary');
+  assert.ok(context.reads.reduce((sum, size) => sum + size, 0) <= MAX_BYTES + 1);
+});
+
+test('same-inode replacement during reading is rejected before parsing', t => {
+  let replaced = false;
+  const context = fixture(t, { beforeRead({ file }) {
+    if (replaced) return;
+    replaced = true;
+    fs.writeFileSync(file, ruleText('outside'));
+    fs.utimesSync(file, new Date(2000), new Date(2000));
+  } });
+  rejected(context.runtime.loadRules(context.root), context);
+});
+
+test('permission changes during reading invalidate the pinned rule snapshot', t => {
+  if (process.platform === 'win32') { t.skip('POSIX file modes required'); return; }
+  let changed = false;
+  const context = fixture(t, { beforeRead({ file }) {
+    if (changed) return;
+    changed = true;
+    fs.chmodSync(file, 0o600);
+  } });
+  fs.chmodSync(context.file, 0o644);
+  rejected(context.runtime.loadRules(context.root), context);
+});
+
+test('path replacement after opening cannot substitute or retain a stale rule', t => {
+  let replaced = false;
+  const context = fixture(t, { beforeRead({ file }) {
+    if (replaced) return;
+    replaced = true;
+    fs.renameSync(file, file + '.original');
+    fs.writeFileSync(file, ruleText('outside'));
+  } });
+  rejected(context.runtime.loadRules(context.root), context);
+});
+
+test('replacement of the rule directory cannot read outside-project rule data', t => {
+  if (process.platform === 'win32') { t.skip('directory symlink creation requires POSIX'); return; }
+  let replaced = false;
+  const context = fixture(t, { beforeOpen({ root, directory }) {
+    if (replaced) return;
+    replaced = true;
+    const outside = path.join(root, 'outside');
+    fs.mkdirSync(outside);
+    fs.writeFileSync(path.join(outside, 'hookify.race.local.md'), ruleText('outside'));
+    fs.renameSync(directory, path.join(root, 'original'));
+    fs.symlinkSync(outside, directory);
+  } });
+  rejected(context.runtime.loadRules(context.root), context);
+  assert.equal(context.reads.length, 0, 'no outside-project bytes may be read');
+});
+
+test('platforms without O_NOFOLLOW reject a raced leaf symlink before any read', t => {
+  if (process.platform === 'win32') { t.skip('symlink creation requires developer mode'); return; }
+  let replaced = false;
+  const context = fixture(t, { noFollow: false, beforeOpen({ root, file }) {
+    if (replaced) return;
+    replaced = true;
+    const outside = path.join(root, 'outside.md');
+    fs.writeFileSync(outside, ruleText('outside'));
+    fs.unlinkSync(file);
+    fs.symlinkSync(outside, file);
+  } });
+  rejected(context.runtime.loadRules(context.root), context);
+  assert.equal(context.reads.length, 0, 'the fallback must reject before consuming bytes');
+});
+
+test('a different real rule directory is rejected before reading its contents', t => {
+  let replaced = false;
+  const context = fixture(t, { beforeOpen({ root, directory }) {
+    if (replaced) return;
+    replaced = true;
+    fs.renameSync(directory, path.join(root, 'original'));
+    fs.mkdirSync(directory);
+    fs.writeFileSync(path.join(directory, 'hookify.race.local.md'), ruleText('outside'));
+  } });
+  rejected(context.runtime.loadRules(context.root), context);
+  assert.equal(context.reads.length, 0);
+});
+
+test('a disappeared rule is diagnosed without opening or reading another file', t => {
+  const context = fixture(t, { beforeOpen({ file }) { fs.unlinkSync(file); } });
+  const result = context.runtime.loadRules(context.root);
+  assert.deepEqual(result.rules, []);
+  assert.match(result.diagnostics[0], /ENOENT/);
+  assert.equal(context.descriptors.size, 0);
+  assert.equal(context.reads.length, 0);
+});
+
+test('premature EOF cannot turn an incomplete file snapshot into a rule', t => {
+  const context = fixture(t);
+  context.io.readSync = () => 0;
+  rejected(context.runtime.loadRules(context.root), context);
+});
+
+test('short descriptor reads load a stable rule exactly at the byte limit', t => {
+  const context = fixture(t, { shortReads: true });
+  const text = ruleText('trusted');
+  fs.writeFileSync(context.file, text + 'x'.repeat(MAX_BYTES - Buffer.byteLength(text)));
+  const result = context.runtime.loadRules(context.root);
+  assert.deepEqual(result.rules.map(rule => rule.name), ['trusted']);
+  assert.deepEqual(result.diagnostics, []);
+  assert.equal(context.reads.reduce((sum, size) => sum + size, 0), MAX_BYTES);
+  assert.equal(context.descriptors.size, 0);
+});
+
+test('read failures close the descriptor and report a bounded diagnostic', t => {
+  const context = fixture(t, { beforeRead() {
+    const error = new Error('EIO injected read failure');
+    error.code = 'EIO';
+    throw error;
+  } });
+  rejected(context.runtime.loadRules(context.root), context);
+});
+
+test('a FIFO is opened nonblocking and rejected without consuming bytes', t => {
+  if (process.platform === 'win32') { t.skip('POSIX FIFO required'); return; }
+  const context = fixture(t, { requireNonblocking: true });
+  fs.unlinkSync(context.file);
+  assert.equal(spawnSync('mkfifo', [context.file]).status, 0);
+  rejected(context.runtime.loadRules(context.root), context);
+  assert.ok(context.flags[0] & fs.constants.O_NONBLOCK);
+  assert.equal(context.reads.length, 0);
+});