mirror of
https://github.com/affaan-m/everything-claude-code.git
synced 2026-06-10 18:23:12 +08:00
fix(hooks): address coderabbit review — use lstatSync for symlink paths
CodeRabbit major on PR #1898: fs.statSync follows symlinks, so a dangling protected symlink (e.g. .eslintrc.js pointing at a missing target) would throw ENOENT and be treated as absent — letting an agent "replace" the symlink and bypass the protection. Swap statSync for lstatSync. lstat reports the link node itself regardless of whether its target exists, so protected entries that happen to be symlinks stay blocked. ENOENT handling is unchanged: only a genuinely missing path (no link, no file, no directory) counts as absent. Add a regression test that creates a dangling symlink at .eslintrc.js and verifies the hook still blocks Write. Skips cleanly on platforms/sandboxes that disallow symlink creation (EPERM/EACCES).
This commit is contained in:
@@ -100,15 +100,17 @@ function run(inputOrRaw, options = {}) {
|
||||
// config file in a project that has none is a legitimate bootstrap
|
||||
// path (e.g. scaffolding ESLint into a fresh repo).
|
||||
//
|
||||
// Fail closed on any stat error other than ENOENT. fs.existsSync would
|
||||
// swallow EACCES/EPERM and return false, which would let an agent
|
||||
// overwrite a file whose parent directory we cannot traverse. statSync
|
||||
// exposes the error code explicitly so we can treat only genuine
|
||||
// "file not found" as absent.
|
||||
// Fail closed on any stat error other than ENOENT. Use lstatSync so a
|
||||
// symlink at the protected path is treated as present even if its target
|
||||
// is missing — a dangling symlink at e.g. .eslintrc.js still represents
|
||||
// an existing config entry that an agent should not silently replace.
|
||||
// fs.existsSync would swallow EACCES/EPERM as false; lstatSync exposes
|
||||
// the error code so we can treat only genuine "path not found" (ENOENT)
|
||||
// as absent.
|
||||
let exists = true;
|
||||
try {
|
||||
fs.statSync(filePath);
|
||||
// stat succeeded — file exists.
|
||||
fs.lstatSync(filePath);
|
||||
// lstat succeeded — something (file, dir, or symlink) exists here.
|
||||
} catch (err) {
|
||||
if (err && err.code === 'ENOENT') {
|
||||
exists = false;
|
||||
|
||||
@@ -205,6 +205,51 @@ function runTests() {
|
||||
passed++;
|
||||
else failed++;
|
||||
|
||||
if (
|
||||
test('blocks protected paths that exist as a dangling symlink', () => {
|
||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-config-protect-'));
|
||||
try {
|
||||
const missingTarget = path.join(tmpDir, 'nowhere.js');
|
||||
const linkPath = path.join(tmpDir, '.eslintrc.js');
|
||||
try {
|
||||
fs.symlinkSync(missingTarget, linkPath);
|
||||
} catch (err) {
|
||||
// Windows without Developer Mode or certain sandboxes disallow
|
||||
// symlinks. Skip cleanly rather than fail the suite.
|
||||
if (err.code === 'EPERM' || err.code === 'EACCES') {
|
||||
console.log(' (skipped: symlink creation not permitted here)');
|
||||
return;
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
|
||||
const input = {
|
||||
tool_name: 'Write',
|
||||
tool_input: {
|
||||
file_path: linkPath,
|
||||
content: 'module.exports = {};'
|
||||
}
|
||||
};
|
||||
|
||||
const result = runHook(input);
|
||||
assert.strictEqual(result.code, 2, `Expected exit 2 for dangling symlink, got ${result.code}; stderr: ${result.stderr}`);
|
||||
assert.strictEqual(result.stdout, '', 'Blocked hook should not echo raw input');
|
||||
assert.ok(
|
||||
result.stderr.includes('BLOCKED: Modifying .eslintrc.js is not allowed.'),
|
||||
`Expected block message, got: ${result.stderr}`
|
||||
);
|
||||
} finally {
|
||||
try {
|
||||
fs.rmSync(tmpDir, { recursive: true, force: true });
|
||||
} catch {
|
||||
// best-effort cleanup
|
||||
}
|
||||
}
|
||||
})
|
||||
)
|
||||
passed++;
|
||||
else failed++;
|
||||
|
||||
if (
|
||||
test('still blocks writes to an existing protected config file', () => {
|
||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-config-protect-'));
|
||||
|
||||
Reference in New Issue
Block a user