From 6c56e541dd9c1ba6524cffd61e89927ca1fea809 Mon Sep 17 00:00:00 2001
From: Nomadu27 <mitica274585@gmail.com>
Date: Tue, 10 Mar 2026 18:08:19 +0100
Subject: [PATCH] =?UTF-8?q?fix:=20address=20cubic-dev-ai=20review=20?=
 =?UTF-8?q?=E2=80=94=203=20issues?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

P1: Log non-ENOENT spawn errors (timeout, signal kill) to stderr
instead of silently exiting 0. Separate handling for result.error
and null result.status so users know when the security monitor
failed to run.

P1: Remove "async": true from hooks.json — async hooks run in the
background and cannot block tool execution. The security hook needs
to be synchronous so exit(2) actually prevents credential exposure
and other critical findings from proceeding.

P2: Remove dead tool_response/tool_result code from extract_content.
In a PreToolUse hook the tool hasn't executed yet, so tool_response
is never populated. Removed the variable and the unreachable branch
that appended its content.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 hooks/hooks.json                          |  1 -
 scripts/hooks/insaits-security-monitor.py | 12 ++----------
 scripts/hooks/insaits-security-wrapper.js | 19 +++++++++++++++++--
 3 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/hooks/hooks.json b/hooks/hooks.json
index 3edc6778..64b2fe83 100644
--- a/hooks/hooks.json
+++ b/hooks/hooks.json
@@ -70,7 +70,6 @@
           {
             "type": "command",
             "command": "node \"${CLAUDE_PLUGIN_ROOT}/scripts/hooks/run-with-flags.js\" \"pre:insaits-security\" \"scripts/hooks/insaits-security-wrapper.js\" \"standard,strict\"",
-            "async": true,
             "timeout": 15
           }
         ],
diff --git a/scripts/hooks/insaits-security-monitor.py b/scripts/hooks/insaits-security-monitor.py
index e0a8ab14..5e8c0b34 100644
--- a/scripts/hooks/insaits-security-monitor.py
+++ b/scripts/hooks/insaits-security-monitor.py
@@ -58,7 +58,7 @@ import logging
 import os
 import sys
 import time
-from typing import Any, Dict, List, Optional, Tuple
+from typing import Any, Dict, List, Tuple
 
 # Configure logging to stderr so it does not interfere with stdout protocol
 logging.basicConfig(
@@ -87,7 +87,6 @@ def extract_content(data: Dict[str, Any]) -> Tuple[str, str]:
     """
     tool_name: str = data.get("tool_name", "")
     tool_input: Dict[str, Any] = data.get("tool_input", {})
-    tool_result: Any = data.get("tool_response", {})
 
     text: str = ""
     context: str = ""
@@ -96,16 +95,9 @@ def extract_content(data: Dict[str, Any]) -> Tuple[str, str]:
         text = tool_input.get("content", "") or tool_input.get("new_string", "")
         context = "file:" + str(tool_input.get("file_path", ""))[:80]
     elif tool_name == "Bash":
+        # PreToolUse: the tool hasn't executed yet, inspect the command
         command: str = str(tool_input.get("command", ""))
-        # For PreToolUse we inspect the command itself
         text = command
-        # Also check tool_response if present (for flexibility)
-        if isinstance(tool_result, dict):
-            output = tool_result.get("output", "") or tool_result.get("stdout", "")
-            if output:
-                text = text + "\n" + output
-        elif isinstance(tool_result, str) and tool_result:
-            text = text + "\n" + tool_result
         context = "bash:" + command[:80]
     elif "content" in data:
         content: Any = data["content"]
diff --git a/scripts/hooks/insaits-security-wrapper.js b/scripts/hooks/insaits-security-wrapper.js
index 6223b368..eba87128 100644
--- a/scripts/hooks/insaits-security-wrapper.js
+++ b/scripts/hooks/insaits-security-wrapper.js
@@ -54,9 +54,24 @@ process.stdin.on('end', () => {
     process.exit(0);
   }
 
+  // Log non-ENOENT spawn errors (timeout, signal kill, etc.) so users
+  // know the security monitor did not run — fail-open with a warning.
+  if (result.error) {
+    process.stderr.write(`[InsAIts] Security monitor failed to run: ${result.error.message}\n`);
+    process.stdout.write(raw);
+    process.exit(0);
+  }
+
   if (result.stdout) process.stdout.write(result.stdout);
   if (result.stderr) process.stderr.write(result.stderr);
 
-  const code = Number.isInteger(result.status) ? result.status : 0;
-  process.exit(code);
+  // result.status is null when the process was killed by a signal or
+  // timed out.  Treat that as an error rather than silently passing.
+  if (!Number.isInteger(result.status)) {
+    const signal = result.signal || 'unknown';
+    process.stderr.write(`[InsAIts] Security monitor killed (signal: ${signal}). Tool execution continues.\n`);
+    process.exit(0);
+  }
+
+  process.exit(result.status);
 });