Fix dangerous command detection for mixed-case tool names

[matdev83]

Summary

• normalize dangerous command tool name checks so case-variants cannot bypass scanning
• add a regression test covering mixed-case tool names in tool calls

Testing

• python -m pytest tests/unit/core/services/test_dangerous_command_service.py -o addopts=
• python -m pytest -o addopts= (fails: missing pytest_asyncio, pytest_httpx, respx dependencies in environment)

---

https://chatgpt.com/codex/tasks/task_e_68ec26d70c8083338f26e9e0caaa409e

Summary by CodeRabbit

• New Features
  • Improved dangerous-command detection to be case-insensitive for tool names, ensuring consistent protection regardless of capitalization.
• Tests
  • Added unit tests validating case-insensitive matching for tool-triggered dangerous commands.

[coderabbitai]

Walkthrough

Introduces case-insensitive tool-name matching in DangerousCommandService by normalizing configured tool names. Updates scan logic to use a precomputed lowercased set. Adds a unit test verifying detection when tool name case differs, asserting the matched rule remains unchanged.

Changes

Cohort / File(s)	Summary
Service logic: case-insensitive tool-name matching src/core/services/dangerous_command_service.py	Adds an instance-level set of lowercased tool names derived from config.tool_names and updates scan to check against this normalized set. No public API changes.
Unit tests: case-variance coverage tests/unit/core/services/test_dangerous_command_service.py	Adds a test validating scan_tool_call detects dangerous commands when tool name differs only by case; asserts non-None result and rule name "git-reset-hard".

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Caller
  participant DangerousCommandService as Service
  participant Config

  Caller->>Service: scan(tool_name, args)
  Service->>Config: read tool_names
  note right of Service: Build _normalized_tool_names (lowercased)
  alt tool_name (case-insensitive) in _normalized_tool_names
    Service->>Service: evaluate command against rules
    Service-->>Caller: Detection result (rule e.g., "git-reset-hard")
  else not matched
    Service-->>Caller: None
  end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I twitch my whiskers, scan the land,
Case folds softly in my clever plan.
Tools shout loud or whisper small—
I hear them true, I catch them all.
A hop, a check, a subtle nod—
Dangerous commands? I give a prod. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title clearly and concisely summarizes the main change by indicating that dangerous command detection is improved to support mixed-case tool names, aligning directly with the implemented fix.
Docstring Coverage	✅ Passed	Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch codex/fix-remote-attack-vector-bug-62j5v5

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dd216cf and 9d639ed.

📒 Files selected for processing (2)

• src/core/services/dangerous_command_service.py (2 hunks)
• tests/unit/core/services/test_dangerous_command_service.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

**/*.py: Avoid using emojis in Python code comments or docstrings
Follow PEP 8 and use type hints for all functions
Import order must be: standard library, third-party, then local imports, separated by blank lines
Naming conventions: snake_case for variables and functions; PascalCase for classes
Error handling: use specific exceptions and include meaningful error messages
Prefer f-strings for string formatting

Files:

• src/core/services/dangerous_command_service.py
• tests/unit/core/services/test_dangerous_command_service.py

src/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.py: Catch specific exceptions; avoid broad except Exception blocks
If a broad exception must be caught, log with exc_info=True and re-raise a specific custom exception
Use the most specific exception class from src.core.common.exceptions that accurately describes the error
Provide clear, helpful error messages and include relevant details in the details dictionary of custom exceptions

Files:

• src/core/services/dangerous_command_service.py

🧬 Code graph analysis (1)

tests/unit/core/services/test_dangerous_command_service.py (2)

src/core/services/dangerous_command_service.py (2)

• DangerousCommandService (11-103)
• scan_tool_call (20-33)

src/core/domain/chat.py (2)

• ToolCall (49-54)
• FunctionCall (42-46)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: test (3.10)
• GitHub Check: Analyze (python)

🔇 Additional comments (3)

src/core/services/dangerous_command_service.py (2)

14-18: Verify config.tool_names typing or add defensive initialization.

Line 92 defensively checks isinstance(tool_name, str) before calling .lower(), but the initialization at line 17 calls .lower() directly on each item without type guards. If config.tool_names could contain non-string values, this would raise an AttributeError at initialization.

Please verify one of the following:

1. DangerousCommandConfig.tool_names is strictly typed as list[str] or set[str], making the defensive check unnecessary; or
2. Add defensive initialization to match the runtime safety at line 92:

-        self._normalized_tool_names: set[str] = {
-            tool_name.lower() for tool_name in self.config.tool_names
-        }
+        self._normalized_tool_names: set[str] = {
+            tool_name.lower() for tool_name in self.config.tool_names
+            if isinstance(tool_name, str)
+        }

---

92-93: LGTM! Case-insensitive tool name matching correctly implemented.

The implementation properly normalizes the tool name and performs case-insensitive lookup against the precomputed set. The defensive isinstance check on line 92 handles non-string tool names gracefully by defaulting to an empty string that won't match any configured tools.

tests/unit/core/services/test_dangerous_command_service.py (1)

106-123: Excellent regression test for case-insensitive tool name detection.

This test effectively validates that the security fix prevents bypassing dangerous command detection via mixed-case tool names (e.g., "Execute_Command" vs "execute_command"). The test is well-structured, clearly documented, and provides critical coverage for the vulnerability addressed in this PR.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}