refactor: remove redundant backend CLI detection (~230 lines) (#1367)
* refactor: remove redundant backend CLI detection (~230 lines) The Claude Agent SDK bundles its own CLI, making the backend's CLI detection code unnecessary. This removes: - find_claude_cli() and related functions from client.py - _validate_claude_cli() security validation - clear_claude_cli_cache() cache management - CLI cache variables and threading locks Changes: - client.py: Remove ~200 lines of CLI detection, add simple CLAUDE_CLI_PATH env var override for SDK - simple_client.py: Remove find_claude_cli import/usage - auth.py: Replace find_executable() with shutil.which() - cli-tool-manager.ts: Update comment (no longer synced with backend) The frontend retains CLI detection for the Terminal tab feature. Backend agents now rely on the SDK's bundled CLI by default. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: add validate_cli_path() security validation and cleanup unused imports - Add validate_cli_path() to CLAUDE_CLI_PATH handling in client.py and simple_client.py to prevent command injection via shell metacharacters, directory traversal, etc. - Add logging for CLAUDE_CLI_PATH override in simple_client.py for consistency - Remove unused imports: shutil, subprocess, get_comspec_path from client.py - Fix import sorting in auth.py (ruff isort) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Andy
@@ -9,12 +9,11 @@ for custom API endpoints.
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
import shutil
|
||||
import subprocess
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
from core.platform import (
|
||||
find_executable,
|
||||
get_claude_detection_paths,
|
||||
is_linux,
|
||||
is_macos,
|
||||
is_windows,
|
||||
@@ -239,7 +238,7 @@ def _decrypt_token_macos(encrypted_data: str) -> str:
|
||||
ValueError: If decryption fails or Claude CLI not available
|
||||
"""
|
||||
# Verify Claude CLI is installed (required for future decryption implementation)
|
||||
if not find_executable("claude", get_claude_detection_paths()):
|
||||
if not shutil.which("claude"):
|
||||
raise ValueError(
|
||||
"Claude Code CLI not found. Please install it from https://code.claude.com"
|
||||
)
|
||||
|
||||
+6
-230
@@ -16,17 +16,12 @@ import copy
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
import shutil
|
||||
import subprocess
|
||||
import threading
|
||||
import time
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
from core.platform import (
|
||||
get_claude_detection_paths_structured,
|
||||
get_comspec_path,
|
||||
is_macos,
|
||||
is_windows,
|
||||
validate_cli_path,
|
||||
)
|
||||
@@ -130,220 +125,6 @@ def invalidate_project_cache(project_dir: Path | None = None) -> None:
|
||||
logger.debug(f"Invalidated project index cache for {project_dir}")
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# Claude CLI Path Detection
|
||||
# =============================================================================
|
||||
# Cross-platform detection of Claude Code CLI binary.
|
||||
# This mirrors the frontend's cli-tool-manager.ts logic to ensure consistency.
|
||||
|
||||
_CLAUDE_CLI_CACHE: dict[str, str | None] = {}
|
||||
_CLI_CACHE_LOCK = threading.Lock()
|
||||
|
||||
|
||||
def _get_claude_detection_paths() -> dict[str, list[str] | str]:
|
||||
"""
|
||||
Get all candidate paths for Claude CLI detection.
|
||||
|
||||
This is a thin wrapper around the platform module's implementation.
|
||||
See core/platform/__init__.py:get_claude_detection_paths_structured()
|
||||
for the canonical implementation.
|
||||
|
||||
Returns:
|
||||
Dict with 'homebrew', 'platform', and 'nvm_versions_dir' keys
|
||||
"""
|
||||
return get_claude_detection_paths_structured()
|
||||
|
||||
|
||||
def _validate_claude_cli(cli_path: str) -> tuple[bool, str | None]:
|
||||
"""
|
||||
Validate that a Claude CLI path is executable and returns a version.
|
||||
|
||||
Includes security validation to prevent command injection attacks.
|
||||
|
||||
Args:
|
||||
cli_path: Path to the Claude CLI executable
|
||||
|
||||
Returns:
|
||||
Tuple of (is_valid, version_string or None)
|
||||
|
||||
Note:
|
||||
Cross-references with frontend's validateClaudeCliAsync() in
|
||||
apps/frontend/src/main/ipc-handlers/claude-code-handlers.ts
|
||||
Both should be kept in sync for consistent behavior.
|
||||
"""
|
||||
import re
|
||||
|
||||
# Security validation: reject paths with shell metacharacters or directory traversal
|
||||
if not validate_cli_path(cli_path):
|
||||
logger.warning(f"Rejecting insecure Claude CLI path: {cli_path}")
|
||||
return False, None
|
||||
|
||||
try:
|
||||
# Augment PATH with the CLI directory for proper resolution
|
||||
env = os.environ.copy()
|
||||
cli_dir = os.path.dirname(cli_path)
|
||||
if cli_dir:
|
||||
env["PATH"] = cli_dir + os.pathsep + env.get("PATH", "")
|
||||
|
||||
# For Windows .cmd/.bat files, use cmd.exe with proper quoting
|
||||
# /d = disable AutoRun registry commands
|
||||
# /s = strip first and last quotes, preserving inner quotes
|
||||
# /c = run command then terminate
|
||||
if is_windows() and cli_path.lower().endswith((".cmd", ".bat")):
|
||||
# Get cmd.exe path from platform module
|
||||
cmd_exe = get_comspec_path()
|
||||
# Use double-quoted command line for paths with spaces
|
||||
cmd_line = f'""{cli_path}" --version"'
|
||||
result = subprocess.run(
|
||||
[cmd_exe, "/d", "/s", "/c", cmd_line],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=5,
|
||||
env=env,
|
||||
creationflags=subprocess.CREATE_NO_WINDOW,
|
||||
)
|
||||
else:
|
||||
result = subprocess.run(
|
||||
[cli_path, "--version"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=5,
|
||||
env=env,
|
||||
creationflags=subprocess.CREATE_NO_WINDOW if is_windows() else 0,
|
||||
)
|
||||
|
||||
if result.returncode == 0:
|
||||
# Extract version from output (e.g., "claude-code version 1.0.0")
|
||||
output = result.stdout.strip()
|
||||
match = re.search(r"(\d+\.\d+\.\d+)", output)
|
||||
version = match.group(1) if match else output.split("\n")[0]
|
||||
return True, version
|
||||
|
||||
return False, None
|
||||
except (subprocess.TimeoutExpired, FileNotFoundError, OSError) as e:
|
||||
logger.debug(f"Claude CLI validation failed for {cli_path}: {e}")
|
||||
return False, None
|
||||
|
||||
|
||||
def find_claude_cli() -> str | None:
|
||||
"""
|
||||
Find the Claude Code CLI binary path.
|
||||
|
||||
Uses cross-platform detection with the following priority:
|
||||
1. CLAUDE_CLI_PATH environment variable (user override)
|
||||
2. shutil.which() - system PATH lookup
|
||||
3. Homebrew paths (macOS)
|
||||
4. NVM paths (Unix - checks Node.js version manager)
|
||||
5. Platform-specific standard locations
|
||||
|
||||
Returns:
|
||||
Path to Claude CLI if found and valid, None otherwise
|
||||
"""
|
||||
# Check cache first
|
||||
cache_key = "claude_cli"
|
||||
with _CLI_CACHE_LOCK:
|
||||
if cache_key in _CLAUDE_CLI_CACHE:
|
||||
cached = _CLAUDE_CLI_CACHE[cache_key]
|
||||
logger.debug(f"Using cached Claude CLI path: {cached}")
|
||||
return cached
|
||||
|
||||
paths = _get_claude_detection_paths()
|
||||
|
||||
# 1. Check environment variable override
|
||||
env_path = os.environ.get("CLAUDE_CLI_PATH")
|
||||
if env_path:
|
||||
if Path(env_path).exists():
|
||||
valid, version = _validate_claude_cli(env_path)
|
||||
if valid:
|
||||
logger.info(f"Using CLAUDE_CLI_PATH: {env_path} (v{version})")
|
||||
with _CLI_CACHE_LOCK:
|
||||
_CLAUDE_CLI_CACHE[cache_key] = env_path
|
||||
return env_path
|
||||
logger.warning(f"CLAUDE_CLI_PATH is set but invalid: {env_path}")
|
||||
|
||||
# 2. Try shutil.which() - most reliable cross-platform PATH lookup
|
||||
which_path = shutil.which("claude")
|
||||
if which_path:
|
||||
valid, version = _validate_claude_cli(which_path)
|
||||
if valid:
|
||||
logger.info(f"Found Claude CLI in PATH: {which_path} (v{version})")
|
||||
with _CLI_CACHE_LOCK:
|
||||
_CLAUDE_CLI_CACHE[cache_key] = which_path
|
||||
return which_path
|
||||
|
||||
# 3. Homebrew paths (macOS)
|
||||
if is_macos():
|
||||
for hb_path in paths["homebrew"]:
|
||||
if Path(hb_path).exists():
|
||||
valid, version = _validate_claude_cli(hb_path)
|
||||
if valid:
|
||||
logger.info(f"Found Claude CLI (Homebrew): {hb_path} (v{version})")
|
||||
with _CLI_CACHE_LOCK:
|
||||
_CLAUDE_CLI_CACHE[cache_key] = hb_path
|
||||
return hb_path
|
||||
|
||||
# 4. NVM paths (Unix only) - check Node.js version manager installations
|
||||
if not is_windows():
|
||||
nvm_dir = Path(paths["nvm_versions_dir"])
|
||||
if nvm_dir.exists():
|
||||
try:
|
||||
# Get all version directories and sort by version (newest first)
|
||||
version_dirs = []
|
||||
for entry in nvm_dir.iterdir():
|
||||
if entry.is_dir() and entry.name.startswith("v"):
|
||||
# Parse version: v20.0.0 -> (20, 0, 0)
|
||||
try:
|
||||
parts = entry.name[1:].split(".")
|
||||
if len(parts) == 3:
|
||||
version_dirs.append(
|
||||
(tuple(int(p) for p in parts), entry.name)
|
||||
)
|
||||
except ValueError:
|
||||
continue
|
||||
|
||||
# Sort by version descending (newest first)
|
||||
version_dirs.sort(reverse=True)
|
||||
|
||||
for _, version_name in version_dirs:
|
||||
nvm_claude = nvm_dir / version_name / "bin" / "claude"
|
||||
if nvm_claude.exists():
|
||||
valid, version = _validate_claude_cli(str(nvm_claude))
|
||||
if valid:
|
||||
logger.info(
|
||||
f"Found Claude CLI (NVM): {nvm_claude} (v{version})"
|
||||
)
|
||||
with _CLI_CACHE_LOCK:
|
||||
_CLAUDE_CLI_CACHE[cache_key] = str(nvm_claude)
|
||||
return str(nvm_claude)
|
||||
except OSError as e:
|
||||
logger.debug(f"Error scanning NVM directory: {e}")
|
||||
|
||||
# 5. Platform-specific standard locations
|
||||
for plat_path in paths["platform"]:
|
||||
if Path(plat_path).exists():
|
||||
valid, version = _validate_claude_cli(plat_path)
|
||||
if valid:
|
||||
logger.info(f"Found Claude CLI: {plat_path} (v{version})")
|
||||
with _CLI_CACHE_LOCK:
|
||||
_CLAUDE_CLI_CACHE[cache_key] = plat_path
|
||||
return plat_path
|
||||
|
||||
# Not found
|
||||
logger.warning(
|
||||
"Claude CLI not found. Install with: npm install -g @anthropic-ai/claude-code"
|
||||
)
|
||||
with _CLI_CACHE_LOCK:
|
||||
_CLAUDE_CLI_CACHE[cache_key] = None
|
||||
return None
|
||||
|
||||
|
||||
def clear_claude_cli_cache() -> None:
|
||||
"""Clear the Claude CLI path cache, forcing re-detection on next call."""
|
||||
with _CLI_CACHE_LOCK:
|
||||
_CLAUDE_CLI_CACHE.clear()
|
||||
logger.debug("Claude CLI cache cleared")
|
||||
|
||||
|
||||
from agents.tools_pkg import (
|
||||
CONTEXT7_TOOLS,
|
||||
ELECTRON_TOOLS,
|
||||
@@ -1014,14 +795,6 @@ def create_client(
|
||||
print(" - CLAUDE.md: disabled by project settings")
|
||||
print()
|
||||
|
||||
# Find Claude CLI path for SDK
|
||||
# This ensures the SDK can find the Claude Code binary even if it's not in PATH
|
||||
cli_path = find_claude_cli()
|
||||
if cli_path:
|
||||
print(f" - Claude CLI: {cli_path}")
|
||||
else:
|
||||
print(" - Claude CLI: using SDK default detection")
|
||||
|
||||
# Build options dict, conditionally including output_format
|
||||
options_kwargs: dict[str, Any] = {
|
||||
"model": model,
|
||||
@@ -1046,9 +819,12 @@ def create_client(
|
||||
"enable_file_checkpointing": True,
|
||||
}
|
||||
|
||||
# Add CLI path if found (helps SDK find Claude Code in non-standard locations)
|
||||
if cli_path:
|
||||
options_kwargs["cli_path"] = cli_path
|
||||
# Optional: Allow CLI path override via environment variable
|
||||
# The SDK bundles its own CLI, but users can override if needed
|
||||
env_cli_path = os.environ.get("CLAUDE_CLI_PATH")
|
||||
if env_cli_path and validate_cli_path(env_cli_path):
|
||||
options_kwargs["cli_path"] = env_cli_path
|
||||
logger.info(f"Using CLAUDE_CLI_PATH override: {env_cli_path}")
|
||||
|
||||
# Add structured output format if specified
|
||||
# See: https://platform.claude.com/docs/en/agent-sdk/structured-outputs
|
||||
|
||||
@@ -21,6 +21,7 @@ Example usage:
|
||||
client = create_simple_client(agent_type="insights", cwd=project_dir)
|
||||
"""
|
||||
|
||||
import logging
|
||||
from pathlib import Path
|
||||
|
||||
from agents.tools_pkg import get_agent_config, get_default_thinking_level
|
||||
@@ -30,9 +31,11 @@ from core.auth import (
|
||||
require_auth_token,
|
||||
validate_token_not_encrypted,
|
||||
)
|
||||
from core.client import find_claude_cli
|
||||
from core.platform import validate_cli_path
|
||||
from phase_config import get_thinking_budget
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def create_simple_client(
|
||||
agent_type: str = "merge_resolver",
|
||||
@@ -95,10 +98,8 @@ def create_simple_client(
|
||||
thinking_level = get_default_thinking_level(agent_type)
|
||||
max_thinking_tokens = get_thinking_budget(thinking_level)
|
||||
|
||||
# Find Claude CLI path (handles non-standard installations)
|
||||
cli_path = find_claude_cli()
|
||||
|
||||
# Build options dict
|
||||
# Note: SDK bundles its own CLI, so no cli_path detection needed
|
||||
options_kwargs = {
|
||||
"model": model,
|
||||
"system_prompt": system_prompt,
|
||||
@@ -112,8 +113,10 @@ def create_simple_client(
|
||||
if max_thinking_tokens is not None:
|
||||
options_kwargs["max_thinking_tokens"] = max_thinking_tokens
|
||||
|
||||
# Add CLI path if found
|
||||
if cli_path:
|
||||
options_kwargs["cli_path"] = cli_path
|
||||
# Optional: Allow CLI path override via environment variable
|
||||
env_cli_path = os.environ.get("CLAUDE_CLI_PATH")
|
||||
if env_cli_path and validate_cli_path(env_cli_path):
|
||||
options_kwargs["cli_path"] = env_cli_path
|
||||
logger.info(f"Using CLAUDE_CLI_PATH override: {env_cli_path}")
|
||||
|
||||
return ClaudeSDKClient(options=ClaudeAgentOptions(**options_kwargs))
|
||||
|
||||
@@ -146,15 +146,9 @@ interface ClaudeDetectionPaths {
|
||||
* This pure function consolidates path configuration used by both sync
|
||||
* and async detection methods.
|
||||
*
|
||||
* IMPORTANT: This function has a corresponding implementation in the Python backend:
|
||||
* apps/backend/core/client.py (_get_claude_detection_paths)
|
||||
*
|
||||
* Both implementations MUST be kept in sync to ensure consistent detection behavior
|
||||
* across the Electron frontend and Python backend.
|
||||
*
|
||||
* When adding new detection paths, update BOTH:
|
||||
* 1. This function (getClaudeDetectionPaths in cli-tool-manager.ts)
|
||||
* 2. _get_claude_detection_paths() in client.py
|
||||
* Note: This is the single source of truth for CLI detection paths.
|
||||
* The Python backend relies on the Claude Agent SDK's bundled CLI,
|
||||
* so it no longer needs its own path detection logic.
|
||||
*
|
||||
* @param homeDir - User's home directory (from os.homedir())
|
||||
* @returns Object containing homebrew, platform, and NVM paths
|
||||
|
||||
Reference in New Issue
Block a user