PR Review
Reviewed head: 19f8b5fd5bafc26a9b453a8486219827c1c2eb6a

Summary

• Adds two explicit security-relaxation flags (DISABLE_SECURITY_PATTERNS, DISABLE_TOOL_VALIDATION) and wires them through settings/startup.
• Extends SecurityValidator to optionally skip dangerous-pattern checks in validate_path while keeping approved-directory boundary checks.
• Extends ToolMonitor to allow bypassing validation in trusted environments, plus docs/tests for new config behavior.

What looks good

• Splitting this from formatting/session behavior makes review surface much clearer.
• Keeping approved-directory boundary enforcement in validate_path even when patterns are disabled is the right safety baseline.
• Docs clearly label these as trusted-environment controls.

Issues / questions

1. [Blocker] src/claude/monitor.py (validate_tool_call) — disable_tool_validation currently bypasses all tool checks, not just allowlist/disallowlist checks.
   The early return happens before file-path validation (Read/Write/Edit) and before dangerous Bash pattern checks, so enabling this flag effectively disables multiple security controls at once. That is broader than the PR description/docs imply (“allow any tool name”).
   Suggestion: only skip allowlist/disallowlist branches, but continue running path and command safety validation.

2. [Important] docs/configuration.md + .env.example — docs/warnings currently understate blast radius of DISABLE_TOOL_VALIDATION given current implementation.
   If you keep the current broad bypass, docs should explicitly state it also disables path/command validation in ToolMonitor. If you narrow behavior per item Feature: Migrate to Python SDK #1, docs are fine as-is (with minor wording tweaks).

Suggested tests (if needed)

• Add regression tests proving that with disable_tool_validation=true:
  • disallowed tool-name checks are bypassed, but
  • invalid file paths and dangerous shell commands are still rejected.

Verdict

• ⚠️ Merge after fixes

Addressed the blocker in this PR ✅

What was fixed

• disable_tool_validation no longer short-circuits all validation in ToolMonitor.validate_tool_call.
• It now only bypasses tool-name allowlist/disallowlist checks.
• File path validation (Read/Write/Edit) and dangerous Bash pattern checks continue to run.

Tests added

• test_disable_tool_validation_still_rejects_invalid_file_path
• test_disable_tool_validation_still_rejects_dangerous_bash

Docs updated

• .env.example
• docs/configuration.md

Both now explicitly state that DISABLE_TOOL_VALIDATION only skips tool-name checks while path/command safety checks remain active.

Validation

Ran:

• poetry run pytest tests/unit/test_claude/test_session.py tests/unit/test_config.py tests/unit/test_security/test_validators.py -q -r w

Result: 54 passed, 0 warnings.

PR Review (follow-up)
Reviewed head: 20dc49aa3fbf919f728254b14cd41afa7cbee63c

Thanks — I re-checked the incremental changes and confirmed the original blocker is fixed:

• disable_tool_validation now skips only tool-name allow/disallow checks.
• Path validation and dangerous Bash checks still execute.
• Docs/test updates match the intended behavior.

One remaining point to consider from the added UTC cleanup commit:

1. [Important] src/claude/session.py (is_expired) — code now uses timezone-aware datetime.now(UTC), which is good, but older persisted session timestamps may be naive (from pre-change runs). Subtracting aware now from naive last_used can raise TypeError.
   • Suggestion: normalize loaded timestamps to UTC (assume naive = UTC) in deserialization, or add a compatibility guard in is_expired.

If you add that backward-compat guard, this looks good to merge.

Thanks — I implemented the fix locally, but can’t push from this bot account (403 on both upstream and fork). Posting exact patch as a maintainer-appliable suggestion.

Commit (local)

81a66075cab9a1e986e4d68a46b2330e39d92a9f

Patch

diff --git a/src/claude/session.py b/src/claude/session.py
index ea3c52c..9ae9468 100644
--- a/src/claude/session.py
+++ b/src/claude/session.py
@@ -27,6 +27,17 @@ ClaudeResponse = Union["CLIClaudeResponse", "SDKClaudeResponse"]
 logger = structlog.get_logger()
 
 
+def _to_utc(dt: datetime) -> datetime:
+    """Normalize datetime to timezone-aware UTC.
+
+    Backward compatibility: legacy persisted sessions may contain naive
+    timestamps; treat naive values as UTC.
+    """
+    if dt.tzinfo is None:
+        return dt.replace(tzinfo=UTC)
+    return dt.astimezone(UTC)
+
+
 @dataclass
 class ClaudeSession:
     """Claude Code session state."""
@@ -44,12 +55,12 @@ class ClaudeSession:
 
     def is_expired(self, timeout_hours: int) -> bool:
         """Check if session has expired."""
-        age = datetime.now(UTC) - self.last_used
+        age = datetime.now(UTC) - _to_utc(self.last_used)
         return age > timedelta(hours=timeout_hours)
 
     def update_usage(self, response: ClaudeResponse) -> None:
         """Update session with usage from response."""
-        self.last_used = datetime.now(UTC)
+        self.last_used = _to_utc(datetime.now(UTC))
         self.total_cost += response.cost
         self.total_turns += response.num_turns
         self.message_count += 1
@@ -82,8 +93,8 @@ class ClaudeSession:
             session_id=data["session_id"],
             user_id=data["user_id"],
             project_path=Path(data["project_path"]),
-            created_at=datetime.fromisoformat(data["created_at"]),
-            last_used=datetime.fromisoformat(data["last_used"]),
+            created_at=_to_utc(datetime.fromisoformat(data["created_at"])),
+            last_used=_to_utc(datetime.fromisoformat(data["last_used"])),
             total_cost=data.get("total_cost", 0.0),
             total_turns=data.get("total_turns", 0),
             message_count=data.get("message_count", 0),
diff --git a/tests/unit/test_claude/test_session.py b/tests/unit/test_claude/test_session.py
index 19ad771..93bc6c4 100644
--- a/tests/unit/test_claude/test_session.py
+++ b/tests/unit/test_claude/test_session.py
@@ -123,6 +123,40 @@ class TestClaudeSession:
         assert restored.message_count == original.message_count
         assert restored.tools_used == original.tools_used
 
+    def test_from_dict_normalizes_legacy_naive_timestamps(self):
+        """Legacy naive timestamps should be normalized to UTC-aware datetimes."""
+        data = {
+            "session_id": "test-session",
+            "user_id": 123,
+            "project_path": "/test/path",
+            "created_at": "2026-02-18T10:00:00",
+            "last_used": "2026-02-18T10:30:00",
+            "total_cost": 0.0,
+            "total_turns": 0,
+            "message_count": 0,
+            "tools_used": [],
+        }
+
+        restored = ClaudeSession.from_dict(data)
+
+        assert restored.created_at.tzinfo is not None
+        assert restored.last_used.tzinfo is not None
+        assert restored.created_at.tzinfo == UTC
+        assert restored.last_used.tzinfo == UTC
+
+    def test_is_expired_handles_legacy_naive_last_used(self):
+        """Expiry check should not crash on naive legacy timestamps."""
+        naive_old = datetime.now() - timedelta(hours=30)
+        session = ClaudeSession(
+            session_id="legacy-session",
+            user_id=123,
+            project_path=Path("/test/path"),
+            created_at=naive_old,
+            last_used=naive_old,
+        )
+
+        assert session.is_expired(24) is True
+
 
 class TestInMemorySessionStorage:
     """Test in-memory session storage."""

Validation run

poetry run pytest tests/unit/test_claude/test_session.py -q → 17 passed

Implemented the patch from the latest review comment and pushed it.

Cross-reference for traceability:

• Dedicated datetime cleanup PR: chore: replace deprecated utcnow() with timezone-aware UTC timestamps #41 (chore: replace deprecated utcnow() with timezone-aware UTC timestamps #41)
• Specific commit: alexx-ftw/claude-code-telegram@308e1c7

Applied on this branch:

• Added _to_utc(dt) normalization helper in src/claude/session.py
• Updated session datetime handling:
  • is_expired() now normalizes last_used
  • update_usage() stores normalized UTC timestamp
  • from_dict() normalizes parsed created_at / last_used for legacy naive values
• Added regression tests in tests/unit/test_claude/test_session.py:
  • test_from_dict_normalizes_legacy_naive_timestamps
  • test_is_expired_handles_legacy_naive_last_used

Validation:

• poetry run pytest tests/unit/test_claude/test_session.py -q -r w
• Result: 13 passed, 0 warnings.