fix: address cubic-dev-ai review — 3 issues

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>
This commit is contained in:
Nomadu27
2026-03-10 18:08:19 +01:00
parent 44dc96d2c6
commit 6c56e541dd
3 changed files with 19 additions and 13 deletions

View File

@@ -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"]

View File

@@ -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);
});