fix(defender): share the fingerprint module and separate lock errors from contention

Two issues cubic raised that I had not picked up, both in code added by this PR.

`depsFingerprint()` had been copy-pasted into four files, and the client's value
and the daemon's recorded value are compared to decide whether a daemon still
matches the tree on disk. Nothing kept the copies in step, so editing one would
have produced either a kill/respawn loop on every scan or a permanently stale
daemon: precisely the failures the check exists to prevent. It now lives in
scripts/deps-fingerprint.mjs, imported by both sides in each plugin. The state
file comment claiming the daemon reads the client's stamp was also stale, since
round three changed it to compute its own; corrected.

`acquireDepsLock()` returned null for both contention and genuine failure, and
the caller read every null as contention. A real error such as a permissions
problem therefore marked an install in flight, which suppresses daemon
replacement and leaves the old tree scanning indefinitely. It now reports the
two cases separately: contention scans with the current tree, an error skips the
event so the next hook retries.

Verified: the shared module is deterministic and the only remaining copy, a free
lock is taken, a live owner reports contention, and an unopenable lock path
reports an error rather than false contention. Both suites pass, 15 and 12.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: glebedel

plugins/security/stackone-defender-antigravity/scripts/defender-daemon.mjs

@@ -12,7 +12,7 @@ import { dirname, join, resolve } from "path";
 import { homedir } from "os";
 import { fileURLToPath } from "url";
 import { createServer } from "net";
-import { createHash } from "crypto";
+import { depsFingerprint as computeDepsFingerprint } from "./deps-fingerprint.mjs";
 import { unlinkSync, existsSync, readFileSync, appendFileSync, writeFileSync, mkdirSync, statSync, renameSync } from "fs";
 
 const PROTOCOL_VERSION = 1;
@@ -26,26 +26,6 @@ const DAEMON_STATE = join(homedir(), ".claude", "defender-daemon.json");
 const scriptDir = dirname(fileURLToPath(import.meta.url));
 const pluginRoot = resolve(scriptDir, "..");
 const configPath = join(scriptDir, "defender-daemon.config.json");
-// Must stay in step with the same function in scan-tool-result.mjs.
-function depsFingerprint() {
-  try {
-    const pkg = JSON.parse(readFileSync(join(pluginRoot, "package.json"), "utf8"));
-    const hash = createHash("sha256").update(
-      JSON.stringify({ dependencies: pkg.dependencies ?? {}, overrides: pkg.overrides ?? {} }),
-    );
-    // npm resolves from the lockfile when one is present, so a lockfile-only change
-    // (a transitive bump that needed no override) also changes what lands on disk.
-    try {
-      hash.update(readFileSync(join(pluginRoot, "package-lock.json")));
-    } catch {
-      // No lockfile: package.json alone decides resolution.
-    }
-    return hash.digest("hex");
-  } catch {
-    return null;
-  }
-}
-
 const requireFrom = createRequire(join(pluginRoot, "package.json"));
 
 function rotateLogIfNeeded() {
@@ -296,9 +276,9 @@ server.listen(SOCKET_PATH, () => {
     const state = {
       pid: process.pid,
       defenderVersion,
-      // Same stamp the client writes after an install. Recording it lets the client
-      // tell that this daemon predates a dependency change and needs replacing.
-      depsStamp: depsFingerprint(),
+      // Computed here, not read from the client's stamp file, so the client can tell
+      // that this daemon predates a dependency change and needs replacing.
+      depsStamp: computeDepsFingerprint(pluginRoot),
       protocolVersion: PROTOCOL_VERSION,
       startedAt: new Date().toISOString(),
       socket: SOCKET_PATH,

plugins/security/stackone-defender-antigravity/scripts/deps-fingerprint.mjs

@@ -0,0 +1,30 @@
+/**
+ * Fingerprint of everything that decides which package versions end up in node_modules.
+ *
+ * Imported by both scan-tool-result.mjs and defender-daemon.mjs so the client's expected
+ * value and the daemon's recorded value can never drift. They are compared to decide
+ * whether a running daemon still matches the tree on disk, so two copies of this logic
+ * would mean either a kill/respawn loop on every scan or a permanently stale daemon.
+ */
+import { readFileSync } from "fs";
+import { join } from "path";
+import { createHash } from "crypto";
+
+export function depsFingerprint(pluginRoot) {
+  try {
+    const pkg = JSON.parse(readFileSync(join(pluginRoot, "package.json"), "utf8"));
+    const hash = createHash("sha256").update(
+      JSON.stringify({ dependencies: pkg.dependencies ?? {}, overrides: pkg.overrides ?? {} }),
+    );
+    // npm resolves from the lockfile when one is present, so a lockfile-only change
+    // (a transitive bump that needed no override) also changes what lands on disk.
+    try {
+      hash.update(readFileSync(join(pluginRoot, "package-lock.json")));
+    } catch {
+      // No lockfile: package.json alone decides resolution.
+    }
+    return hash.digest("hex");
+  } catch {
+    return null;
+  }
+}

plugins/security/stackone-defender-antigravity/scripts/scan-tool-result.mjs

@@ -42,7 +42,9 @@ import {
   writeFileSync,
 } from "fs";
 import { execSync, spawn } from "child_process";
-import { createHash } from "crypto";
+import { depsFingerprint as computeDepsFingerprint } from "./deps-fingerprint.mjs";
+
+const depsFingerprint = () => computeDepsFingerprint(pluginRoot);
 import net from "net";
 
 const scriptDir = dirname(fileURLToPath(import.meta.url));
@@ -96,29 +98,6 @@ function readPluginDeps() {
   }
 }
 
-// Fingerprint of everything that decides which versions end up in node_modules.
-// `overrides` matters as much as `dependencies` here: security pins for transitive
-// packages live there, and a plugin upgrade that only moves a pin would otherwise
-// leave an existing install on the old, vulnerable version.
-function depsFingerprint() {
-  try {
-    const pkg = JSON.parse(readFileSync(join(pluginRoot, "package.json"), "utf8"));
-    const hash = createHash("sha256").update(
-      JSON.stringify({ dependencies: pkg.dependencies ?? {}, overrides: pkg.overrides ?? {} }),
-    );
-    // npm resolves from the lockfile when one is present, so a lockfile-only change
-    // (a transitive bump that needed no override) also changes what lands on disk.
-    try {
-      hash.update(readFileSync(join(pluginRoot, "package-lock.json")));
-    } catch {
-      // No lockfile: package.json alone decides resolution.
-    }
-    return hash.digest("hex");
-  } catch {
-    return null;
-  }
-}
-
 function readDepsStamp() {
   try {
     return readFileSync(DEPS_STAMP_PATH, "utf8").trim();
@@ -136,24 +115,30 @@ const DEPS_LOCK_STALE_MS = 180_000;
 // a daemon mid-install would let the replacement import a half-written node_modules.
 let depsInstallInFlight = false;
 
+// Contention and failure are different answers. Contention means another hook is
+// installing, so the tree will be current shortly and this hook should leave the daemon
+// alone. A genuine error means the refresh cannot happen at all, and pretending an
+// install is in flight would keep a stale daemon serving the old tree indefinitely.
 function acquireDepsLock() {
   for (let attempt = 0; attempt < 2; attempt += 1) {
     try {
-      return openSync(DEPS_LOCK_PATH, "wx");
+      return { fd: openSync(DEPS_LOCK_PATH, "wx"), contended: false };
     } catch (err) {
       if (err.code !== "EEXIST") {
         process.stderr.write(`[Defender] Dependency lock failed — scanner disabled: ${err.message}\n`);
-        return null;
+        return { fd: null, contended: false };
       }
       try {
-        if (Date.now() - statSync(DEPS_LOCK_PATH).mtimeMs <= DEPS_LOCK_STALE_MS) return null;
+        if (Date.now() - statSync(DEPS_LOCK_PATH).mtimeMs <= DEPS_LOCK_STALE_MS) {
+          return { fd: null, contended: true };
+        }
         unlinkSync(DEPS_LOCK_PATH);
       } catch {
         // The owner released it between our open and our stat. Try once more.
       }
     }
   }
-  return null;
+  return { fd: null, contended: true };
 }
 
 function depsUpToDate(missing) {
@@ -171,13 +156,20 @@ function ensureDepsInstalled() {
 
   // Serialise installs. After an upgrade every concurrent hook sees the same stale
   // stamp, and npm is not safe to run against one prefix from several processes.
-  const lockFd = acquireDepsLock();
-  if (lockFd === null) {
+  const lock = acquireDepsLock();
+  if (lock.fd === null) {
+    if (!lock.contended) {
+      // The lock could not be taken at all, so the refresh will not happen. Do not mark
+      // an install in flight: that would suppress daemon replacement and quietly keep the
+      // old tree scanning. Skip this event instead and let the next hook retry.
+      return false;
+    }
     // Another hook owns the install. The tree on disk is in flux, so this process must
     // not touch the daemon either; scan with what is already running, if anything.
     depsInstallInFlight = true;
     return !missing;
   }
+  const lockFd = lock.fd;
 
   try {
     // Recheck under the lock: whoever held it first may have finished the install.

plugins/security/stackone-defender/scripts/defender-daemon.mjs

@@ -12,7 +12,7 @@ import { dirname, join, resolve } from "path";
 import { homedir } from "os";
 import { fileURLToPath } from "url";
 import { createServer } from "net";
-import { createHash } from "crypto";
+import { depsFingerprint as computeDepsFingerprint } from "./deps-fingerprint.mjs";
 import { unlinkSync, existsSync, readFileSync, appendFileSync, writeFileSync, mkdirSync, statSync, renameSync } from "fs";
 
 const PROTOCOL_VERSION = 1;
@@ -26,26 +26,6 @@ const DAEMON_STATE = join(homedir(), ".claude", "defender-daemon.json");
 const scriptDir = dirname(fileURLToPath(import.meta.url));
 const pluginRoot = resolve(scriptDir, "..");
 const configPath = join(scriptDir, "defender-daemon.config.json");
-// Must stay in step with the same function in scan-tool-result.mjs.
-function depsFingerprint() {
-  try {
-    const pkg = JSON.parse(readFileSync(join(pluginRoot, "package.json"), "utf8"));
-    const hash = createHash("sha256").update(
-      JSON.stringify({ dependencies: pkg.dependencies ?? {}, overrides: pkg.overrides ?? {} }),
-    );
-    // npm resolves from the lockfile when one is present, so a lockfile-only change
-    // (a transitive bump that needed no override) also changes what lands on disk.
-    try {
-      hash.update(readFileSync(join(pluginRoot, "package-lock.json")));
-    } catch {
-      // No lockfile: package.json alone decides resolution.
-    }
-    return hash.digest("hex");
-  } catch {
-    return null;
-  }
-}
-
 const requireFrom = createRequire(join(pluginRoot, "package.json"));
 
 function rotateLogIfNeeded() {
@@ -296,9 +276,9 @@ server.listen(SOCKET_PATH, () => {
     const state = {
       pid: process.pid,
       defenderVersion,
-      // Same stamp the client writes after an install. Recording it lets the client
-      // tell that this daemon predates a dependency change and needs replacing.
-      depsStamp: depsFingerprint(),
+      // Computed here, not read from the client's stamp file, so the client can tell
+      // that this daemon predates a dependency change and needs replacing.
+      depsStamp: computeDepsFingerprint(pluginRoot),
       protocolVersion: PROTOCOL_VERSION,
       startedAt: new Date().toISOString(),
       socket: SOCKET_PATH,

plugins/security/stackone-defender/scripts/deps-fingerprint.mjs

@@ -0,0 +1,30 @@
+/**
+ * Fingerprint of everything that decides which package versions end up in node_modules.
+ *
+ * Imported by both scan-tool-result.mjs and defender-daemon.mjs so the client's expected
+ * value and the daemon's recorded value can never drift. They are compared to decide
+ * whether a running daemon still matches the tree on disk, so two copies of this logic
+ * would mean either a kill/respawn loop on every scan or a permanently stale daemon.
+ */
+import { readFileSync } from "fs";
+import { join } from "path";
+import { createHash } from "crypto";
+
+export function depsFingerprint(pluginRoot) {
+  try {
+    const pkg = JSON.parse(readFileSync(join(pluginRoot, "package.json"), "utf8"));
+    const hash = createHash("sha256").update(
+      JSON.stringify({ dependencies: pkg.dependencies ?? {}, overrides: pkg.overrides ?? {} }),
+    );
+    // npm resolves from the lockfile when one is present, so a lockfile-only change
+    // (a transitive bump that needed no override) also changes what lands on disk.
+    try {
+      hash.update(readFileSync(join(pluginRoot, "package-lock.json")));
+    } catch {
+      // No lockfile: package.json alone decides resolution.
+    }
+    return hash.digest("hex");
+  } catch {
+    return null;
+  }
+}

plugins/security/stackone-defender/scripts/scan-tool-result.mjs

@@ -23,7 +23,9 @@ import {
   writeFileSync,
 } from "fs";
 import { execSync, spawn } from "child_process";
-import { createHash } from "crypto";
+import { depsFingerprint as computeDepsFingerprint } from "./deps-fingerprint.mjs";
+
+const depsFingerprint = () => computeDepsFingerprint(pluginRoot);
 import net from "net";
 
 const scriptDir = dirname(fileURLToPath(import.meta.url));
@@ -83,29 +85,6 @@ function readPluginDeps() {
   }
 }
 
-// Fingerprint of everything that decides which versions end up in node_modules.
-// `overrides` matters as much as `dependencies` here: security pins for transitive
-// packages live there, and a plugin upgrade that only moves a pin would otherwise
-// leave an existing install on the old, vulnerable version.
-function depsFingerprint() {
-  try {
-    const pkg = JSON.parse(readFileSync(join(pluginRoot, "package.json"), "utf8"));
-    const hash = createHash("sha256").update(
-      JSON.stringify({ dependencies: pkg.dependencies ?? {}, overrides: pkg.overrides ?? {} }),
-    );
-    // npm resolves from the lockfile when one is present, so a lockfile-only change
-    // (a transitive bump that needed no override) also changes what lands on disk.
-    try {
-      hash.update(readFileSync(join(pluginRoot, "package-lock.json")));
-    } catch {
-      // No lockfile: package.json alone decides resolution.
-    }
-    return hash.digest("hex");
-  } catch {
-    return null;
-  }
-}
-
 function readDepsStamp() {
   try {
     return readFileSync(DEPS_STAMP_PATH, "utf8").trim();
@@ -123,24 +102,30 @@ const DEPS_LOCK_STALE_MS = 180_000;
 // a daemon mid-install would let the replacement import a half-written node_modules.
 let depsInstallInFlight = false;
 
+// Contention and failure are different answers. Contention means another hook is
+// installing, so the tree will be current shortly and this hook should leave the daemon
+// alone. A genuine error means the refresh cannot happen at all, and pretending an
+// install is in flight would keep a stale daemon serving the old tree indefinitely.
 function acquireDepsLock() {
   for (let attempt = 0; attempt < 2; attempt += 1) {
     try {
-      return openSync(DEPS_LOCK_PATH, "wx");
+      return { fd: openSync(DEPS_LOCK_PATH, "wx"), contended: false };
     } catch (err) {
       if (err.code !== "EEXIST") {
         process.stderr.write(`[Defender] Dependency lock failed — scanner disabled: ${err.message}\n`);
-        return null;
+        return { fd: null, contended: false };
       }
       try {
-        if (Date.now() - statSync(DEPS_LOCK_PATH).mtimeMs <= DEPS_LOCK_STALE_MS) return null;
+        if (Date.now() - statSync(DEPS_LOCK_PATH).mtimeMs <= DEPS_LOCK_STALE_MS) {
+          return { fd: null, contended: true };
+        }
         unlinkSync(DEPS_LOCK_PATH);
       } catch {
         // The owner released it between our open and our stat. Try once more.
       }
     }
   }
-  return null;
+  return { fd: null, contended: true };
 }
 
 function depsUpToDate(missing) {
@@ -158,13 +143,20 @@ function ensureDepsInstalled() {
 
   // Serialise installs. After an upgrade every concurrent hook sees the same stale
   // stamp, and npm is not safe to run against one prefix from several processes.
-  const lockFd = acquireDepsLock();
-  if (lockFd === null) {
+  const lock = acquireDepsLock();
+  if (lock.fd === null) {
+    if (!lock.contended) {
+      // The lock could not be taken at all, so the refresh will not happen. Do not mark
+      // an install in flight: that would suppress daemon replacement and quietly keep the
+      // old tree scanning. Skip this event instead and let the next hook retry.
+      return false;
+    }
     // Another hook owns the install. The tree on disk is in flux, so this process must
     // not touch the daemon either; scan with what is already running, if anything.
     depsInstallInFlight = true;
     return !missing;
   }
+  const lockFd = lock.fd;
 
   try {
     // Recheck under the lock: whoever held it first may have finished the install.