From 7145ca9dfe25bf2d9700990a5b2c2117dbce865a Mon Sep 17 00:00:00 2001
From: gaurav0107 <gauravdubey0107@gmail.com>
Date: Fri, 15 May 2026 01:09:43 +0530
Subject: [PATCH] =?UTF-8?q?fix(hooks):=20address=20coderabbit=20review=20?=
 =?UTF-8?q?=E2=80=94=20use=20lstatSync=20for=20symlink=20paths?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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).
---
 scripts/hooks/config-protection.js    | 16 +++++-----
 tests/hooks/config-protection.test.js | 45 +++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 7 deletions(-)

diff --git a/scripts/hooks/config-protection.js b/scripts/hooks/config-protection.js
index d8d459a0..625da3b7 100644
--- a/scripts/hooks/config-protection.js
+++ b/scripts/hooks/config-protection.js
@@ -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;
diff --git a/tests/hooks/config-protection.test.js b/tests/hooks/config-protection.test.js
index 51382409..ec57c137 100644
--- a/tests/hooks/config-protection.test.js
+++ b/tests/hooks/config-protection.test.js
@@ -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-'));