feat: add InsAIts PostToolUse security monitoring hook#370
Open
Nomadu27 wants to merge 7 commits intoaffaan-m:mainfrom
Open
feat: add InsAIts PostToolUse security monitoring hook#370Nomadu27 wants to merge 7 commits intoaffaan-m:mainfrom
Nomadu27 wants to merge 7 commits intoaffaan-m:mainfrom
Conversation
- Add insaits-security-monitor.py: real-time AI security monitoring hook that catches credential exposure, prompt injection, hallucinations, and 20+ other anomaly types - Update hooks.json with InsAIts PostToolUse entry - Update hooks/README.md with InsAIts in PostToolUse table - Add InsAIts MCP server entry to mcp-configs/mcp-servers.json InsAIts (https://github.com/Nomadu27/InsAIts) is an open-source runtime security layer for multi-agent AI. It runs 100% locally and writes tamper-evident audit logs to .insaits_audit_session.jsonl. Install: pip install insa-its Co-Authored-By: Claude Opus 4.6 <[email protected]>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an InsAIts AI security monitor: documentation and config entries, a PreToolUse hook that runs a Node wrapper, a Python monitor that scans extracted tool input locally, writes per-run JSONL audit events, warns on non-critical findings, and blocks on critical findings. Changes
Sequence Diagram(s)sequenceDiagram
participant HookRunner as Hook Runner
participant RunWithFlags as run-with-flags.js
participant NodeWrapper as insaits-security-wrapper.js
participant PythonMonitor as insaits-security-monitor.py
participant InsAItsSDK as insa_its SDK
HookRunner->>RunWithFlags: invoke PreToolUse hook
RunWithFlags->>NodeWrapper: start wrapper, stream stdin
NodeWrapper->>PythonMonitor: spawn python (stream input, 14s timeout)
PythonMonitor->>InsAItsSDK: request analysis (local SDK)
InsAItsSDK-->>PythonMonitor: return anomalies (types, severities)
PythonMonitor->>PythonMonitor: write audit entry to .insaits_audit_session.jsonl
alt critical findings
PythonMonitor-->>NodeWrapper: emit blocking feedback, exit 2
else non-critical or none
PythonMonitor-->>NodeWrapper: emit warnings/ok, exit 0
end
NodeWrapper-->>RunWithFlags: proxy output and exit code
RunWithFlags-->>HookRunner: hook result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="mcp-configs/mcp-servers.json">
<violation number="1" location="mcp-configs/mcp-servers.json:93">
P2: `"command": "python"` will fail on many Linux distributions where only `python3` is available. Use `python3` for reliable cross-platform behavior (PEP 394).</violation>
</file>
<file name="hooks/README.md">
<violation number="1" location="hooks/README.md:39">
P2: Matcher `.*` fires a Python process on every tool call, unlike all other PostToolUse hooks which use targeted matchers (`Bash`, `Edit`, etc.). This will add noticeable latency to the entire workflow. Consider narrowing the matcher to the tool types that actually produce security-relevant output (e.g., `Bash|Write|Edit`).</violation>
<violation number="2" location="hooks/README.md:39">
P1: This entry introduces a dependency on an unvetted external PyPI package (`insa-its`) from an individual's repository. The project's review convention is to port hook scripts directly into the repo so they are versioned and reviewed together, rather than relying on external packages that could be compromised or changed without notice. Either vendor the relevant detection logic into `scripts/hooks/` or remove this entry until the package has been formally vetted.
(Based on your team's feedback about avoiding linking to unvetted external repositories.) [FEEDBACK_USED]</violation>
</file>
<file name="scripts/hooks/insaits-security-monitor.py">
<violation number="1" location="scripts/hooks/insaits-security-monitor.py:161">
P1: Wrap the SDK calls in `try/except`. Since this hook runs on every tool invocation (matcher `.*`), an unhandled exception from `insAItsMonitor()` or `send_message()` will crash with exit code 1 on every tool use, disrupting the entire Claude Code session. Catch exceptions and fall back to `sys.exit(0)` so a monitoring failure doesn't block the developer.</violation>
<violation number="2" location="scripts/hooks/insaits-security-monitor.py:186">
P0: `getattr()` on dict items always returns the default — the severity check is dead code. The InsAIts SDK returns anomalies as dicts (e.g., `anomaly['severity']`), so `getattr(a, "severity", "")` always yields `""`, making `has_critical` permanently `False`. The hook can never block a critical finding. Use `a.get("severity", "")` instead. The same issue affects `format_feedback()` (severity/type/detail always show defaults) and the audit log's `anomaly_types` (always empty strings).</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
hooks/README.md
Outdated
| | **Prettier format** | `Edit` | Auto-formats JS/TS files with Prettier after edits | | ||
| | **TypeScript check** | `Edit` | Runs `tsc --noEmit` after editing `.ts`/`.tsx` files | | ||
| | **console.log warning** | `Edit` | Warns about `console.log` statements in edited files | | ||
| | **InsAIts security monitor** | `.*` | Real-time AI security: catches credential exposure, prompt injection, hallucinations, behavioral anomalies (23 types). Writes audit log to `.insaits_audit_session.jsonl`. Requires `pip install insa-its`. [Details](../scripts/hooks/insaits-security-monitor.py) | |
There was a problem hiding this comment.
P1: This entry introduces a dependency on an unvetted external PyPI package (insa-its) from an individual's repository. The project's review convention is to port hook scripts directly into the repo so they are versioned and reviewed together, rather than relying on external packages that could be compromised or changed without notice. Either vendor the relevant detection logic into scripts/hooks/ or remove this entry until the package has been formally vetted.
(Based on your team's feedback about avoiding linking to unvetted external repositories.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/README.md, line 39:
<comment>This entry introduces a dependency on an unvetted external PyPI package (`insa-its`) from an individual's repository. The project's review convention is to port hook scripts directly into the repo so they are versioned and reviewed together, rather than relying on external packages that could be compromised or changed without notice. Either vendor the relevant detection logic into `scripts/hooks/` or remove this entry until the package has been formally vetted.
(Based on your team's feedback about avoiding linking to unvetted external repositories.) </comment>
<file context>
@@ -36,6 +36,7 @@ User request → Claude picks a tool → PreToolUse hook runs → Tool executes
| **Prettier format** | `Edit` | Auto-formats JS/TS files with Prettier after edits |
| **TypeScript check** | `Edit` | Runs `tsc --noEmit` after editing `.ts`/`.tsx` files |
| **console.log warning** | `Edit` | Warns about `console.log` statements in edited files |
+| **InsAIts security monitor** | `.*` | Real-time AI security: catches credential exposure, prompt injection, hallucinations, behavioral anomalies (23 types). Writes audit log to `.insaits_audit_session.jsonl`. Requires `pip install insa-its`. [Details](../scripts/hooks/insaits-security-monitor.py) |
### Lifecycle Hooks
</file context>
hooks/README.md
Outdated
| | **Prettier format** | `Edit` | Auto-formats JS/TS files with Prettier after edits | | ||
| | **TypeScript check** | `Edit` | Runs `tsc --noEmit` after editing `.ts`/`.tsx` files | | ||
| | **console.log warning** | `Edit` | Warns about `console.log` statements in edited files | | ||
| | **InsAIts security monitor** | `.*` | Real-time AI security: catches credential exposure, prompt injection, hallucinations, behavioral anomalies (23 types). Writes audit log to `.insaits_audit_session.jsonl`. Requires `pip install insa-its`. [Details](../scripts/hooks/insaits-security-monitor.py) | |
There was a problem hiding this comment.
P2: Matcher .* fires a Python process on every tool call, unlike all other PostToolUse hooks which use targeted matchers (Bash, Edit, etc.). This will add noticeable latency to the entire workflow. Consider narrowing the matcher to the tool types that actually produce security-relevant output (e.g., Bash|Write|Edit).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/README.md, line 39:
<comment>Matcher `.*` fires a Python process on every tool call, unlike all other PostToolUse hooks which use targeted matchers (`Bash`, `Edit`, etc.). This will add noticeable latency to the entire workflow. Consider narrowing the matcher to the tool types that actually produce security-relevant output (e.g., `Bash|Write|Edit`).</comment>
<file context>
@@ -36,6 +36,7 @@ User request → Claude picks a tool → PreToolUse hook runs → Tool executes
| **Prettier format** | `Edit` | Auto-formats JS/TS files with Prettier after edits |
| **TypeScript check** | `Edit` | Runs `tsc --noEmit` after editing `.ts`/`.tsx` files |
| **console.log warning** | `Edit` | Warns about `console.log` statements in edited files |
+| **InsAIts security monitor** | `.*` | Real-time AI security: catches credential exposure, prompt injection, hallucinations, behavioral anomalies (23 types). Writes audit log to `.insaits_audit_session.jsonl`. Requires `pip install insa-its`. [Details](../scripts/hooks/insaits-security-monitor.py) |
### Lifecycle Hooks
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (9)
hooks/hooks.json (3)
169-179: Hook lacks profile-based enable/disable viarun-with-flags.js.Other hooks in this file use the
run-with-flags.jswrapper to support runtime enable/disable viaECC_DISABLED_HOOKSenvironment variable and profile selection (ECC_HOOK_PROFILE). This hook bypasses that infrastructure, meaning users cannot easily disable it without editing hooks.json.Consider wrapping with the existing infrastructure for consistency:
♻️ Integrate with run-with-flags infrastructure
{ "matcher": "*", "hooks": [ { "type": "command", - "command": "python \"${CLAUDE_PLUGIN_ROOT}/scripts/hooks/insaits-security-monitor.py\"", + "command": "node \"${CLAUDE_PLUGIN_ROOT}/scripts/hooks/run-with-flags.js\" \"post:insaits-security\" \"scripts/hooks/insaits-security-monitor.py\" \"standard,strict\"", "timeout": 15 } ],Note: This requires
run-with-flags.jsto support Python script execution, or a Python-specific wrapper.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/hooks.json` around lines 169 - 179, The new hook directly calls the Python script "scripts/hooks/insaits-security-monitor.py" and bypasses the existing run-with-flags.js infrastructure, so update the hook entry to invoke the run-with-flags wrapper (or a Python-aware wrapper) instead of calling Python directly; reference the existing run-with-flags.js wrapper, ensure it forwards the original command string (python "${CLAUDE_PLUGIN_ROOT}/scripts/hooks/insaits-security-monitor.py") and honors ECC_DISABLED_HOOKS and ECC_HOOK_PROFILE, and if necessary add/extend a Python-specific wrapper that run-with-flags.js can invoke to preserve profile-based enable/disable semantics.
174-174: Usepython3for explicit Python 3 invocation.The command uses
pythonwhich may not resolve to Python 3 on all systems (especially macOS/Linux wherepythonmight be Python 2 or unset). Consider usingpython3for consistency and cross-platform reliability.💡 Suggested fix
- "command": "python \"${CLAUDE_PLUGIN_ROOT}/scripts/hooks/insaits-security-monitor.py\"", + "command": "python3 \"${CLAUDE_PLUGIN_ROOT}/scripts/hooks/insaits-security-monitor.py\"",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/hooks.json` at line 174, The hook command currently calls "python \"${CLAUDE_PLUGIN_ROOT}/scripts/hooks/insaits-security-monitor.py\"" which can resolve to Python 2 or be missing on some systems; update the command value for the "command" key in hooks.json to invoke "python3" instead (i.e., use python3 "${CLAUDE_PLUGIN_ROOT}/scripts/hooks/insaits-security-monitor.py") so the script runs with Python 3 consistently across platforms.
168-179: Consider marking the hook asasyncto avoid blocking the tool pipeline.This hook runs synchronously on every tool invocation (matcher
"*") with a 15-second timeout. Unlike the observe hook above it (lines 157-167) which uses"async": true, this hook will block Claude Code's response flow while InsAIts processes the output.If real-time blocking feedback isn't required (and per PostToolUse semantics, it can't block anyway), consider making this async:
♻️ Suggested async configuration
{ "matcher": "*", "hooks": [ { "type": "command", "command": "python \"${CLAUDE_PLUGIN_ROOT}/scripts/hooks/insaits-security-monitor.py\"", - "timeout": 15 + "timeout": 15, + "async": true } ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/hooks.json` around lines 168 - 179, The hook defined with matcher "*" and type "command" that runs the InsAIts script ("python \"${CLAUDE_PLUGIN_ROOT}/scripts/hooks/insaits-security-monitor.py\"") should be marked async to avoid blocking the tool pipeline; update the hook object to include "async": true (next to "type"/"command"/"timeout") so the InsAIts monitor runs asynchronously like the earlier observe hook and does not block Claude Code's response flow.mcp-configs/mcp-servers.json (1)
92-95: Considerpython3for cross-platform compatibility.Using
"command": "python"may fail on systems wherepythonis not aliased to Python 3 (common on macOS/Linux). Consider usingpython3for explicit version targeting, or document that users should ensurepythonresolves to Python 3.💡 Alternative for explicit Python 3
"insaits": { - "command": "python", + "command": "python3", "args": ["-m", "insa_its.mcp_server"],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcp-configs/mcp-servers.json` around lines 92 - 95, The Docker/JSON service entry "insaits" currently sets "command": "python" which can resolve to Python 2 on some systems; update the "insaits" service entry to use "python3" in the "command" field (or detect and fail-fast with a clear message) so the module "insa_its.mcp_server" runs under Python 3, and optionally add a short note in the service description that Python 3 is required to ensure cross-platform compatibility.hooks/README.md (1)
39-39: Documentation should clarify that blocking is not possible from PostToolUse.The entry states the hook catches various anomalies, but the underlying script uses exit code 2 to "block actions." Per lines 11-12 of this same file, PostToolUse hooks "can analyze output but cannot block." The documentation entry should clarify that CRITICAL findings will be surfaced as feedback but won't actually block execution.
📝 Suggested documentation update
-| **InsAIts security monitor** | `.*` | Real-time AI security: catches credential exposure, prompt injection, hallucinations, behavioral anomalies (23 types). Writes audit log to `.insaits_audit_session.jsonl`. Requires `pip install insa-its`. [Details](../scripts/hooks/insaits-security-monitor.py) | +| **InsAIts security monitor** | `.*` | Real-time AI security: detects credential exposure, prompt injection, hallucinations, behavioral anomalies (23 types). CRITICAL findings are surfaced as feedback (non-blocking). Writes audit log to `.insaits_audit_session.jsonl`. Requires `pip install insa-its`. [Details](../scripts/hooks/insaits-security-monitor.py) |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/README.md` at line 39, Update the hooks/README.md entry for "InsAIts security monitor" to explicitly state that although the insaits-security-monitor.py script may use exit code 2 to indicate a "block" internally, PostToolUse hooks (per the README's note on PostToolUse behavior) cannot actually prevent execution; instead, CRITICAL findings from InsAIts will be surfaced as feedback/audit (e.g., written to .insaits_audit_session.jsonl and reported) but will not stop the tool chain from proceeding. Mention the script name insaits-security-monitor.py and the audit file .insaits_audit_session.jsonl so readers can locate the implementation and logs.scripts/hooks/insaits-security-monitor.py (4)
68-96: Add type annotations to function signatures.Per coding guidelines for
**/*.py, type annotations should be used on all function signatures.♻️ Suggested type annotations
-def extract_content(data): +def extract_content(data: dict) -> tuple[str, str]: """Extract inspectable text from a Claude Code tool result."""-def write_audit(event): +def write_audit(event: dict) -> None: """Append an audit event to the JSONL audit log."""-def format_feedback(anomalies): +def format_feedback(anomalies: list) -> str: """Format detected anomalies as feedback for Claude Code."""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/hooks/insaits-security-monitor.py` around lines 68 - 96, The function extract_content lacks type annotations on its signature; update it to include proper typing for the input and return types (e.g., annotate the parameter data as Mapping[str, Any] or Dict[str, Any] and the return type as Tuple[str, str] or similar). Modify the extract_content definition to use those types (referencing the function name extract_content and variables data, text, context) and add any necessary imports (typing: Dict/Mapping, Any, Tuple) at the top of the file to satisfy static type checks and the project's coding guidelines.
159-159: Environment mutation violates immutability guidelines.
os.environ.setdefault("INSAITS_DEV_MODE", "true")mutates global state. Per coding guidelines, immutable patterns should be preferred. Consider passing configuration directly to the monitor constructor if the SDK supports it, or document this side effect clearly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/hooks/insaits-security-monitor.py` at line 159, The code mutates global environment via os.environ.setdefault("INSAITS_DEV_MODE", "true"); remove that mutation and instead pass the dev-mode flag through the monitor's constructor or config object (e.g., add a dev_mode=True parameter to the monitor constructor or a Config/Options object used when instantiating the monitor) so the setting is local to the instance; if the SDK truly lacks such an API, encapsulate the environment write in a clearly named helper (e.g., set_dev_mode_env()) and document the side effect in its docstring and call site, ensuring all references to os.environ.setdefault("INSAITS_DEV_MODE", "true") are replaced with the constructor/config approach or with a single documented helper.
152-156: Use theloggingmodule instead ofprint()for diagnostic output.Per coding guidelines for
**/*.py, theloggingmodule should be used instead ofprint()statements. This allows proper log level control and integrates better with observability tooling.♻️ Example refactor
+import logging + +logger = logging.getLogger("insaits") + # In the warning section: - print( - "[InsAIts] Not installed. Run: pip install insa-its", - file=sys.stderr, - ) + logger.warning("InsAIts SDK not installed. Run: pip install insa-its")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/hooks/insaits-security-monitor.py` around lines 152 - 156, Replace the print(...) diagnostic with the logging module: import logging at the top (and call logging.basicConfig(...) somewhere central if not already configured), then change the print call to logging.error("[InsAIts] Not installed. Run: pip install insa-its") and keep the subsequent sys.exit(0); update any surrounding code that relied on print's file=sys.stderr behavior to use logging levels instead.
106-109: Silent error swallowing on audit write failure.Failing to write the audit log is silently ignored. Per coding guidelines, errors should be handled explicitly. At minimum, emit a warning to stderr so operators know the audit trail is incomplete.
🛡️ Suggested fix
with open(AUDIT_FILE, "a", encoding="utf-8") as f: f.write(json.dumps(event) + "\n") - except OSError: - pass + except OSError as e: + print(f"[InsAIts] Warning: Failed to write audit log: {e}", file=sys.stderr)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/hooks/insaits-security-monitor.py` around lines 106 - 109, The current except OSError block swallows failures when writing to AUDIT_FILE; update the exception handling around the with open(AUDIT_FILE, "a", encoding="utf-8") write so that it does not silently ignore errors—catch OSError (or Exception) and emit a clear warning to stderr (including the exception details and the AUDIT_FILE path) so operators know the audit write failed; keep the handler lightweight (log to sys.stderr or use existing logger if available) and only suppress the exception after logging to avoid hiding problems.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/hooks/insaits-security-monitor.py`:
- Around line 191-196: The current PostToolUse hook uses sys.exit(2) to "block"
actions (see has_critical, print(feedback), sys.exit(2)) but PostToolUse runs
after the tool completes so exit code 2 can't prevent the action; either convert
this hook to a PreToolUse hook so blocking via sys.exit(2) will actually stop
execution, or if only post-execution alerting is intended, remove the blocking
exit and instead emit the feedback to stderr (print(feedback, file=sys.stderr))
and exit with 0 (or document that alerts are non-blocking) — update the hook
type and comments accordingly and keep references to has_critical and feedback
handling in the change.
- Around line 35-36: The docstring in scripts/hooks/insaits-security-monitor.py
incorrectly claims "Exit code 2 = critical issue found (blocks action)" but this
is a PostToolUse hook so exit code 2 cannot block; either move this hook to
PreToolUse if you need to block before tool execution, or update the hook's
docstring and behavior to treat critical findings as non-blocking warnings
(e.g., log as CRITICAL/WARNING and exit 0 or a non-blocking code). Locate
references to "Exit code 2" and the PostToolUse/PreToolUse wording in
insaits-security-monitor.py, then choose one: (A) relocate the hook to the
PreToolUse registration point so it can block, or (B) change the docstring and
any exit/return paths in the script (remove or repurpose exit code 2) to surface
critical issues as warnings without blocking. Ensure consistency between the
docstring and the actual exit behavior.
---
Nitpick comments:
In `@hooks/hooks.json`:
- Around line 169-179: The new hook directly calls the Python script
"scripts/hooks/insaits-security-monitor.py" and bypasses the existing
run-with-flags.js infrastructure, so update the hook entry to invoke the
run-with-flags wrapper (or a Python-aware wrapper) instead of calling Python
directly; reference the existing run-with-flags.js wrapper, ensure it forwards
the original command string (python
"${CLAUDE_PLUGIN_ROOT}/scripts/hooks/insaits-security-monitor.py") and honors
ECC_DISABLED_HOOKS and ECC_HOOK_PROFILE, and if necessary add/extend a
Python-specific wrapper that run-with-flags.js can invoke to preserve
profile-based enable/disable semantics.
- Line 174: The hook command currently calls "python
\"${CLAUDE_PLUGIN_ROOT}/scripts/hooks/insaits-security-monitor.py\"" which can
resolve to Python 2 or be missing on some systems; update the command value for
the "command" key in hooks.json to invoke "python3" instead (i.e., use python3
"${CLAUDE_PLUGIN_ROOT}/scripts/hooks/insaits-security-monitor.py") so the script
runs with Python 3 consistently across platforms.
- Around line 168-179: The hook defined with matcher "*" and type "command" that
runs the InsAIts script ("python
\"${CLAUDE_PLUGIN_ROOT}/scripts/hooks/insaits-security-monitor.py\"") should be
marked async to avoid blocking the tool pipeline; update the hook object to
include "async": true (next to "type"/"command"/"timeout") so the InsAIts
monitor runs asynchronously like the earlier observe hook and does not block
Claude Code's response flow.
In `@hooks/README.md`:
- Line 39: Update the hooks/README.md entry for "InsAIts security monitor" to
explicitly state that although the insaits-security-monitor.py script may use
exit code 2 to indicate a "block" internally, PostToolUse hooks (per the
README's note on PostToolUse behavior) cannot actually prevent execution;
instead, CRITICAL findings from InsAIts will be surfaced as feedback/audit
(e.g., written to .insaits_audit_session.jsonl and reported) but will not stop
the tool chain from proceeding. Mention the script name
insaits-security-monitor.py and the audit file .insaits_audit_session.jsonl so
readers can locate the implementation and logs.
In `@mcp-configs/mcp-servers.json`:
- Around line 92-95: The Docker/JSON service entry "insaits" currently sets
"command": "python" which can resolve to Python 2 on some systems; update the
"insaits" service entry to use "python3" in the "command" field (or detect and
fail-fast with a clear message) so the module "insa_its.mcp_server" runs under
Python 3, and optionally add a short note in the service description that Python
3 is required to ensure cross-platform compatibility.
In `@scripts/hooks/insaits-security-monitor.py`:
- Around line 68-96: The function extract_content lacks type annotations on its
signature; update it to include proper typing for the input and return types
(e.g., annotate the parameter data as Mapping[str, Any] or Dict[str, Any] and
the return type as Tuple[str, str] or similar). Modify the extract_content
definition to use those types (referencing the function name extract_content and
variables data, text, context) and add any necessary imports (typing:
Dict/Mapping, Any, Tuple) at the top of the file to satisfy static type checks
and the project's coding guidelines.
- Line 159: The code mutates global environment via
os.environ.setdefault("INSAITS_DEV_MODE", "true"); remove that mutation and
instead pass the dev-mode flag through the monitor's constructor or config
object (e.g., add a dev_mode=True parameter to the monitor constructor or a
Config/Options object used when instantiating the monitor) so the setting is
local to the instance; if the SDK truly lacks such an API, encapsulate the
environment write in a clearly named helper (e.g., set_dev_mode_env()) and
document the side effect in its docstring and call site, ensuring all references
to os.environ.setdefault("INSAITS_DEV_MODE", "true") are replaced with the
constructor/config approach or with a single documented helper.
- Around line 152-156: Replace the print(...) diagnostic with the logging
module: import logging at the top (and call logging.basicConfig(...) somewhere
central if not already configured), then change the print call to
logging.error("[InsAIts] Not installed. Run: pip install insa-its") and keep the
subsequent sys.exit(0); update any surrounding code that relied on print's
file=sys.stderr behavior to use logging levels instead.
- Around line 106-109: The current except OSError block swallows failures when
writing to AUDIT_FILE; update the exception handling around the with
open(AUDIT_FILE, "a", encoding="utf-8") write so that it does not silently
ignore errors—catch OSError (or Exception) and emit a clear warning to stderr
(including the exception details and the AUDIT_FILE path) so operators know the
audit write failed; keep the handler lightweight (log to sys.stderr or use
existing logger if available) and only suppress the exception after logging to
avoid hiding problems.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 114e18b5-c7a6-433c-9b91-e64ed178dcde
📒 Files selected for processing (4)
hooks/README.mdhooks/hooks.jsonmcp-configs/mcp-servers.jsonscripts/hooks/insaits-security-monitor.py
…tations, logging
Critical fixes:
- Convert hook from PostToolUse to PreToolUse so exit(2) blocking works
- Change all python references to python3 for cross-platform compat
- Add insaits-security-wrapper.js to bridge run-with-flags.js to Python
Standard fixes:
- Wrap hook with run-with-flags.js so users can disable via
ECC_DISABLED_HOOKS="pre:insaits-security"
- Add "async": true to hooks.json entry
- Add type annotations to all function signatures (Dict, List, Tuple, Any)
- Replace all print() statements with logging module (stderr)
- Fix silent OSError swallow in write_audit — now logs warning
- Remove os.environ.setdefault('INSAITS_DEV_MODE') — pass dev_mode=True
through monitor constructor instead
- Update hooks/README.md: moved to PreToolUse table, "detects" not
"catches", clarify blocking vs non-blocking behavior
Co-Authored-By: Claude Opus 4.6 <[email protected]>
affaan-m
reviewed
Mar 10, 2026
Owner
affaan-m
left a comment
affaan-m
left a comment
There was a problem hiding this comment.
Automated review: waiting for checks to complete.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
scripts/hooks/insaits-security-monitor.py (1)
184-187: Consider makingdev_modeconfigurable via environment variable.The
dev_mode=Trueis hardcoded, which may not be appropriate for production deployments. Consider reading from an environment variable for flexibility.♻️ Make dev_mode configurable
monitor: insAItsMonitor = insAItsMonitor( session_name="claude-code-hook", - dev_mode=True, + dev_mode=os.environ.get("INSAITS_DEV_MODE", "").lower() in ("1", "true", "yes"), )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/hooks/insaits-security-monitor.py` around lines 184 - 187, The code hardcodes dev_mode=True when instantiating insAItsMonitor which prevents toggling production behavior; change this to read from an environment variable (e.g., os.getenv or similar) and coerce to boolean before passing to insAItsMonitor(session_name="claude-code-hook", dev_mode=...), providing a sensible default (False) and optionally allowing a truthy value like "1"/"true"; update any related config or docs to mention the new environment variable name and validate the value where insAItsMonitor is constructed.scripts/hooks/insaits-security-wrapper.js (1)
35-49: Consider handling non-ENOENT spawn errors explicitly.The loop correctly handles
ENOENT(binary not found) by continuing to the next candidate. However, ifresult.errorexists but is notENOENT(e.g.,ETIMEDOUT,EPERM), the loop breaks and falls through to line 51-55 where the checkresult.error && result.error.code === 'ENOENT'won't match, causing stdout/stderr from a potentially failed spawn to be written.This is a minor edge case since timeout is handled separately and most spawn errors on valid binaries are rare.
♻️ Optional: Add explicit error logging for non-ENOENT errors
// ENOENT means binary not found — try next candidate if (result.error && result.error.code === 'ENOENT') { continue; } + // Other spawn errors (e.g., ETIMEDOUT) — log and break + if (result.error) { + process.stderr.write(`[InsAIts] Spawn error: ${result.error.message}\n`); + } break; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/hooks/insaits-security-wrapper.js` around lines 35 - 49, The loop over pythonCandidates uses spawnSync to execute pyScript but only treats ENOENT specially; if result.error exists and its code is not 'ENOENT' we should handle it explicitly instead of falling through—update the loop in scripts/hooks/insaits-security-wrapper.js around the spawnSync call (symbols: pythonCandidates, spawnSync, result, pyScript) to detect result.error && result.error.code !== 'ENOENT' and then log/emit the error or throw (so the caller knows a real spawn failure occurred) before breaking/returning, while still continuing for ENOENT to try the next candidate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/hooks/insaits-security-monitor.py`:
- Line 90: The code extracts the wrong key from the incoming payload; change the
data.get call in the tool_result assignment from "tool_response" to
"tool_output" (i.e., update tool_result: Any = data.get("tool_response", {}) →
use "tool_output") and update the nearby comment (around the current line 102)
to reference "tool_output" instead of "tool_response" so the variable and
comment match the hook input schema used elsewhere.
---
Nitpick comments:
In `@scripts/hooks/insaits-security-monitor.py`:
- Around line 184-187: The code hardcodes dev_mode=True when instantiating
insAItsMonitor which prevents toggling production behavior; change this to read
from an environment variable (e.g., os.getenv or similar) and coerce to boolean
before passing to insAItsMonitor(session_name="claude-code-hook", dev_mode=...),
providing a sensible default (False) and optionally allowing a truthy value like
"1"/"true"; update any related config or docs to mention the new environment
variable name and validate the value where insAItsMonitor is constructed.
In `@scripts/hooks/insaits-security-wrapper.js`:
- Around line 35-49: The loop over pythonCandidates uses spawnSync to execute
pyScript but only treats ENOENT specially; if result.error exists and its code
is not 'ENOENT' we should handle it explicitly instead of falling through—update
the loop in scripts/hooks/insaits-security-wrapper.js around the spawnSync call
(symbols: pythonCandidates, spawnSync, result, pyScript) to detect result.error
&& result.error.code !== 'ENOENT' and then log/emit the error or throw (so the
caller knows a real spawn failure occurred) before breaking/returning, while
still continuing for ENOENT to try the next candidate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e56b40cd-d785-4a91-9aa1-c3eaae5d057f
📒 Files selected for processing (5)
hooks/README.mdhooks/hooks.jsonmcp-configs/mcp-servers.jsonscripts/hooks/insaits-security-monitor.pyscripts/hooks/insaits-security-wrapper.js
🚧 Files skipped from review as they are similar to previous changes (2)
- hooks/hooks.json
- mcp-configs/mcp-servers.json
| """ | ||
| tool_name: str = data.get("tool_name", "") | ||
| tool_input: Dict[str, Any] = data.get("tool_input", {}) | ||
| tool_result: Any = data.get("tool_response", {}) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check what key name Claude Code hooks actually use for tool output
# Search for tool_output or tool_response in existing hook implementations
rg -n "tool_output|tool_response" --type=js --type=py -g '!node_modules'Repository: affaan-m/everything-claude-code
Length of output: 699
Fix the incorrect key name for tool output extraction.
Line 90 uses data.get("tool_response", {}) but the hook input schema uses tool_output. All test data and other hook implementations (post-bash-pr-created.js) use tool_output. Change tool_response to tool_output on line 90, and update the comment on line 102 accordingly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/hooks/insaits-security-monitor.py` at line 90, The code extracts the
wrong key from the incoming payload; change the data.get call in the tool_result
assignment from "tool_response" to "tool_output" (i.e., update tool_result: Any
= data.get("tool_response", {}) → use "tool_output") and update the nearby
comment (around the current line 102) to reference "tool_output" instead of
"tool_response" so the variable and comment match the hook input schema used
elsewhere.
There was a problem hiding this comment.
3 issues found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/hooks/insaits-security-wrapper.js">
<violation number="1" location="scripts/hooks/insaits-security-wrapper.js:60">
P1: Non-ENOENT spawn errors (timeout, signal kill, etc.) are silently swallowed and the wrapper exits 0, effectively bypassing the security check. When `result.status` is `null` (which it is for timeouts and signal kills), the fallback `? result.status : 0` makes this a fail-open. At minimum, log `result.error` to stderr so the user knows the security monitor didn't run.</violation>
</file>
<file name="hooks/hooks.json">
<violation number="1" location="hooks/hooks.json:73">
P1: Security hook is `async: true` so it can never block tool execution. The README documents this hook as "Blocks on critical findings" with exit code 2, but async hooks run in the background and cannot block. This makes the security monitor unable to prevent credential exposure or other critical findings from proceeding.</violation>
</file>
<file name="scripts/hooks/insaits-security-monitor.py">
<violation number="1" location="scripts/hooks/insaits-security-monitor.py:102">
P2: Dead code: in a PreToolUse hook the tool hasn't executed yet, so `tool_response` will never be populated. The `tool_result` variable and the block that appends its content to `text` are unreachable. Remove them to avoid confusion about when this hook fires.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
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 <[email protected]>
Defaults to true (no API key needed) but can be disabled by setting INSAITS_DEV_MODE=false for production deployments with an API key. Co-Authored-By: Claude Opus 4.6 <[email protected]>
There was a problem hiding this comment.
2 issues found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/hooks/insaits-security-monitor.py">
<violation number="1" location="scripts/hooks/insaits-security-monitor.py:178">
P2: Security monitor defaults to `dev_mode=True`. A security tool should default to its strictest mode; users who want relaxed checks during development should opt in by setting `INSAITS_DEV_MODE=true`, not opt out.</violation>
</file>
<file name="scripts/hooks/insaits-security-wrapper.js">
<violation number="1" location="scripts/hooks/insaits-security-wrapper.js:70">
P2: When the Python process is killed by an external signal (no `result.error`, null `result.status`), the partial `result.stdout` has already been written to the consumer at line 68 before this check runs. Unlike the `result.error` path which correctly writes `raw` (the original tool output) for fail-open, this path leaks partial/corrupt monitor output. Move this check above the `stdout`/`stderr` writes and pass through `raw` instead.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
scripts/hooks/insaits-security-monitor.py (2)
115-126: Avoid mutating the input event dictionary.The function modifies
eventin-place by addingtimestampandhashkeys. While the current call site creates a fresh dict literal, this pattern can cause subtle bugs if the dict is reused. As per coding guidelines, prefer creating new objects.♻️ Proposed fix to avoid mutation
def write_audit(event: Dict[str, Any]) -> None: """Append an audit event to the JSONL audit log.""" try: - event["timestamp"] = time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime()) - event["hash"] = hashlib.sha256( - json.dumps(event, sort_keys=True).encode() + enriched = { + **event, + "timestamp": time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime()), + } + enriched["hash"] = hashlib.sha256( + json.dumps(enriched, sort_keys=True).encode() ).hexdigest()[:16] with open(AUDIT_FILE, "a", encoding="utf-8") as f: - f.write(json.dumps(event) + "\n") + f.write(json.dumps(enriched) + "\n") except OSError as exc: log.warning("Failed to write audit log %s: %s", AUDIT_FILE, exc)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/hooks/insaits-security-monitor.py` around lines 115 - 126, The write_audit function mutates the input event by adding "timestamp" and "hash"; instead, make a shallow copy (e.g., new_event = dict(event)) and add the timestamp and computed hash to that copy, then write json.dumps(new_event) to AUDIT_FILE; ensure you compute the hash over the same sorted-keys JSON you will write (so include timestamp but not hash when hashing), and keep the existing OSError handling and AUDIT_FILE and function name write_audit unchanged.
169-169: Consider extracting magic numbers to named constants.The values
10(minimum content length) and4000(max text to scan) are magic numbers. Per coding guidelines: "Do not use hardcoded values; use constants or configuration instead."♻️ Proposed fix to define constants
# Add near line 78 with other constants MIN_CONTENT_LENGTH: int = 10 MAX_SCAN_LENGTH: int = 4000 DEFAULT_MODEL: str = "claude-opus"Then use
MIN_CONTENT_LENGTHat line 169 andMAX_SCAN_LENGTHat line 181.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/hooks/insaits-security-monitor.py` at line 169, Replace the magic literals used to validate and slice text with named constants: define MIN_CONTENT_LENGTH = 10 and MAX_SCAN_LENGTH = 4000 (and optionally DEFAULT_MODEL = "claude-opus" if present near other constants in the file), then update the conditional "if len(text.strip()) < 10:" to use MIN_CONTENT_LENGTH and the truncation logic that uses 4000 characters to use MAX_SCAN_LENGTH; ensure the new constants are declared alongside the other module-level constants and referenced by name in the checks and slicing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/hooks/insaits-security-monitor.py`:
- Around line 176-184: The InsAIts SDK instantiation and call are not
error-handled: wrap the insAItsMonitor(...) creation and the
monitor.send_message(...) call in try/except to catch and log exceptions, return
or set a safe fallback result, and avoid crashing the hook; specifically,
surround the insAItsMonitor(...) call (symbol: insAItsMonitor) and the
subsequent monitor.send_message(...) invocation (symbol: monitor.send_message)
with a single try block (or separate try blocks) that logs the exception details
and gracefully handles failures (e.g., set result to an error Dict with
status/fallback or return early) so the hook continues in a deterministic way
per coding guidelines.
---
Nitpick comments:
In `@scripts/hooks/insaits-security-monitor.py`:
- Around line 115-126: The write_audit function mutates the input event by
adding "timestamp" and "hash"; instead, make a shallow copy (e.g., new_event =
dict(event)) and add the timestamp and computed hash to that copy, then write
json.dumps(new_event) to AUDIT_FILE; ensure you compute the hash over the same
sorted-keys JSON you will write (so include timestamp but not hash when
hashing), and keep the existing OSError handling and AUDIT_FILE and function
name write_audit unchanged.
- Line 169: Replace the magic literals used to validate and slice text with
named constants: define MIN_CONTENT_LENGTH = 10 and MAX_SCAN_LENGTH = 4000 (and
optionally DEFAULT_MODEL = "claude-opus" if present near other constants in the
file), then update the conditional "if len(text.strip()) < 10:" to use
MIN_CONTENT_LENGTH and the truncation logic that uses 4000 characters to use
MAX_SCAN_LENGTH; ensure the new constants are declared alongside the other
module-level constants and referenced by name in the checks and slicing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fea0680e-ae60-479b-8b69-e55028229c0d
📒 Files selected for processing (3)
hooks/hooks.jsonscripts/hooks/insaits-security-monitor.pyscripts/hooks/insaits-security-wrapper.js
🚧 Files skipped from review as they are similar to previous changes (2)
- hooks/hooks.json
- scripts/hooks/insaits-security-wrapper.js
cubic-dev-ai P2: dev_mode now defaults to "false" (strict mode). Users opt in to dev mode by setting INSAITS_DEV_MODE=true. cubic-dev-ai P2: Move null-status check above stdout/stderr writes in wrapper so partial/corrupt output is never leaked. Pass through original raw input on signal kill, matching the result.error path. coderabbit major: Wrap insAItsMonitor() and send_message() in try/except so SDK errors don't crash the hook. Logs warning and exits 0 (fail-open) on exception. coderabbit nitpick: write_audit now creates a new dict (enriched) instead of mutating the caller's event dict. coderabbit nitpick: Extract magic numbers to named constants: MIN_CONTENT_LENGTH=10, MAX_SCAN_LENGTH=4000, DEFAULT_MODEL. Also: added env var documentation to module docstring. Co-Authored-By: Claude Opus 4.6 <[email protected]>
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/hooks/insaits-security-monitor.py">
<violation number="1" location="scripts/hooks/insaits-security-monitor.py:207">
P1: The hook fails open on InsAIts SDK errors (`sys.exit(0)`), which allows unscanned tool input to proceed and can bypass the security gate.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scripts/hooks/insaits-security-monitor.py (1)
205-207: Intentional fail-open — acknowledge the tradeoff.The bare
Exceptioncatch (flagged by Ruff BLE001) is intentional per the commit message: "fail-open (exit 0) on exceptions." This is acceptable for a security monitor that shouldn't break the workflow, but consider logging the exception type for easier debugging:♻️ Optional: log exception type for better diagnostics
except Exception as exc: - log.warning("SDK error, skipping security scan: %s", exc) + log.warning("SDK error (%s), skipping security scan: %s", type(exc).__name__, exc) sys.exit(0)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/hooks/insaits-security-monitor.py` around lines 205 - 207, The catch intentionally fails open but the warning should include the exception type for diagnostics: update the except Exception as exc handler so the log.warning call includes the exception's class name (e.g., exc.__class__.__name__ or type(exc).__name__) and optionally the exception repr/message while leaving the sys.exit(0) behavior unchanged; locate the except Exception as exc block and the log.warning/sys.exit(0) lines to make this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/hooks/insaits-security-monitor.py`:
- Around line 154-162: The loop that builds `lines` treats each `a` in
`anomalies` as an object using getattr, which returns defaults instead of actual
dict values; change the access to dict lookups (e.g., use a.get("severity",
"MEDIUM"), a.get("type", "UNKNOWN"), and a.get("details", "")) or add a small
helper like `extract_anomaly_field(a, field, default)` that handles dict/object
shapes, and apply this same fix for the other occurrences that use getattr on
anomalies (the code paths that reference "severity"/"type"/"details" later,
previously at the other two spots) so critical severities and the "details"
field are read correctly and the exit-2 logic can trigger.
---
Nitpick comments:
In `@scripts/hooks/insaits-security-monitor.py`:
- Around line 205-207: The catch intentionally fails open but the warning should
include the exception type for diagnostics: update the except Exception as exc
handler so the log.warning call includes the exception's class name (e.g.,
exc.__class__.__name__ or type(exc).__name__) and optionally the exception
repr/message while leaving the sys.exit(0) behavior unchanged; locate the except
Exception as exc block and the log.warning/sys.exit(0) lines to make this
change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 217e29b1-b046-4546-bab9-52f28c0a09c1
📒 Files selected for processing (2)
scripts/hooks/insaits-security-monitor.pyscripts/hooks/insaits-security-wrapper.js
- Add get_anomaly_attr() helper that handles both dict and object anomalies. The SDK's send_message() returns dicts, so getattr() was silently returning defaults -- critical blocking never triggered. - Fix field name: "detail" -> "details" (matches SDK schema). - Make fail-open/fail-closed configurable via INSAITS_FAIL_MODE env var (defaults to "open" for backward compatibility). - Include exception type name in fail-open log for diagnostics. - Normalize severity comparison with .upper() for case-insensitive matching. Co-Authored-By: Claude Opus 4.6 <[email protected]>
affaan-m
reviewed
Mar 10, 2026
Owner
affaan-m
left a comment
affaan-m
left a comment
There was a problem hiding this comment.
Automated review: waiting for checks to complete.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
scripts/hooks/insaits-security-monitor.py (2)
219-231: Consider narrowing the exception type or documenting the rationale.Ruff flags the broad
Exceptioncatch (BLE001). While this is intentional for SDK resilience, consider either:
- Catching more specific exceptions if the SDK documents them (e.g.,
RuntimeError,ValueError)- Adding a comment explaining the intentional broad catch for SDK fault tolerance
The current implementation is functional—the fail-open/fail-closed pattern with
INSAITS_FAIL_MODEis well-designed and the exception type name is correctly logged for diagnostics.💡 Optional: Add inline comment documenting the broad catch
- except Exception as exc: + except Exception as exc: # Broad catch intentional: unknown SDK internals🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/hooks/insaits-security-monitor.py` around lines 219 - 231, The except Exception block in the insaits-security-monitor (the try/except that reads INSAITS_FAIL_MODE and uses fail_mode, sys.exit(2)/sys.exit(0), and log.warning) is flagged as too broad (BLE001); either narrow the catch to specific exceptions raised by the SDK (e.g., RuntimeError/ValueError) if those are documented, or keep the broad catch but add a clear inline comment above the except explaining that a broad Exception catch is intentional to enforce the fail-open/fail-closed resilience for the SDK (mentioning INSAITS_FAIL_MODE behavior and why we exit with codes 2 or 0) so linters/reviewers understand the rationale.
248-251: Consider simplifying the single-element tuple check.The expression
in ("CRITICAL",)works but is unusual for a single-value check. If only one severity level triggers blocking,== "CRITICAL"is more readable. If you anticipate adding more blocking severities (e.g.,"HIGH"), extracting to a constant would improve maintainability.💡 Option A: Simplify to equality check
has_critical: bool = any( - get_anomaly_attr(a, "severity").upper() in ("CRITICAL",) for a in anomalies + get_anomaly_attr(a, "severity").upper() == "CRITICAL" for a in anomalies )💡 Option B: Extract to constant for extensibility
+BLOCKING_SEVERITIES: frozenset[str] = frozenset({"CRITICAL"}) + ... has_critical: bool = any( - get_anomaly_attr(a, "severity").upper() in ("CRITICAL",) for a in anomalies + get_anomaly_attr(a, "severity").upper() in BLOCKING_SEVERITIES for a in anomalies )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/hooks/insaits-security-monitor.py` around lines 248 - 251, The check for a single blocking severity is written as a single-element tuple membership test; update the has_critical computation to be clearer by either (A) changing the generator expression to compare equality (get_anomaly_attr(a, "severity").upper() == "CRITICAL") if only CRITICAL blocks, or (B) for extensibility extract a constant like BLOCKING_SEVERITIES = {"CRITICAL", "HIGH"} and use membership (get_anomaly_attr(a, "severity").upper() in BLOCKING_SEVERITIES) so has_critical remains readable and easy to extend; modify the line defining has_critical accordingly and reference get_anomaly_attr and anomalies in your change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@scripts/hooks/insaits-security-monitor.py`:
- Around line 219-231: The except Exception block in the
insaits-security-monitor (the try/except that reads INSAITS_FAIL_MODE and uses
fail_mode, sys.exit(2)/sys.exit(0), and log.warning) is flagged as too broad
(BLE001); either narrow the catch to specific exceptions raised by the SDK
(e.g., RuntimeError/ValueError) if those are documented, or keep the broad catch
but add a clear inline comment above the except explaining that a broad
Exception catch is intentional to enforce the fail-open/fail-closed resilience
for the SDK (mentioning INSAITS_FAIL_MODE behavior and why we exit with codes 2
or 0) so linters/reviewers understand the rationale.
- Around line 248-251: The check for a single blocking severity is written as a
single-element tuple membership test; update the has_critical computation to be
clearer by either (A) changing the generator expression to compare equality
(get_anomaly_attr(a, "severity").upper() == "CRITICAL") if only CRITICAL blocks,
or (B) for extensibility extract a constant like BLOCKING_SEVERITIES =
{"CRITICAL", "HIGH"} and use membership (get_anomaly_attr(a, "severity").upper()
in BLOCKING_SEVERITIES) so has_critical remains readable and easy to extend;
modify the line defining has_critical accordingly and reference get_anomaly_attr
and anomalies in your change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7eb87a57-7a59-41d8-8015-87a05cce4360
📒 Files selected for processing (1)
scripts/hooks/insaits-security-monitor.py
- Extract BLOCKING_SEVERITIES frozenset for extensible severity checks. - Add inline comment on broad Exception catch explaining intentional SDK fault-tolerance pattern (BLE001 acknowledged). Co-Authored-By: Claude Opus 4.6 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds InsAIts as a PostToolUse security monitoring hook for Claude Code. InsAIts is an open-source runtime security layer for multi-agent AI that detects anomalies in real-time — 100% locally, no data leaves the machine.
What this PR adds:
scripts/hooks/insaits-security-monitor.py— A Python PostToolUse hook that inspects every tool output for:hooks/hooks.json— Added InsAIts entry to PostToolUse hookshooks/README.md— Added InsAIts to the PostToolUse hooks tablemcp-configs/mcp-servers.json— Added InsAIts MCP server entry for users who prefer MCP integration over hooksHow it works:
.insaits_audit_session.jsonlWhy this is useful for Claude Code users:
sk-proj-...in a Write)pip install insa-its— works immediatelyInstall:
No API key needed. All processing runs locally.
Project: github.com/Nomadu27/InsAIts (Apache 2.0)
PyPI: pypi.org/project/insa-its
Type of Change
feat:New featureChecklist
Summary by cubic
Adds an InsAIts PreToolUse security monitor that scans tool inputs for credential leaks, prompt injection, and hallucinations before execution. Blocks on critical findings, fails open by default (configurable fail-closed), and writes a tamper‑evident audit log.
New Features
scripts/hooks/insaits-security-monitor.pydetects 20+ anomaly types; exit 2 blocks, 0 passes; writes.insaits_audit_session.jsonl.hooks/hooks.jsonwith a 15s timeout; runs synchronously viascripts/hooks/insaits-security-wrapper.jsandscripts/hooks/run-with-flags.js(disable withECC_DISABLED_HOOKS="pre:insaits-security").INSAITS_FAIL_MODEallows fail-closed.hooks/README.md; optional MCP server inmcp-configs/mcp-servers.json.Bug Fixes
get_anomaly_attrto handle dict/object anomalies so critical blocks trigger reliably.detailsand normalized severity with.upper()for accurate blocking.INSAITS_FAIL_MODE; logs now include exception types for diagnostics.BLOCKING_SEVERITIESconstant for extensible severity checks; documented intentional broad exception catch.Written for commit 9ea415c. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation