fix(linux): add secretstorage to platform-critical packages (ACS-310)

apps/backend/core/client.py

@@ -16,6 +16,7 @@
 import json
 import logging
 import os
+import platform
 import shutil
 import subprocess
 import threading
@@ -24,11 +25,11 @@
 from typing import Any
 
 from core.platform import (
     get_claude_detection_paths_structured,
     get_comspec_path,
     is_macos,
     is_windows,
     validate_cli_path,
 )
 
 logger = logging.getLogger(__name__)
@@ -140,18 +141,83 @@
 _CLI_CACHE_LOCK = threading.Lock()
 
 
-def _get_claude_detection_paths() -> dict[str, list[str] | str]:
+def _get_claude_detection_paths() -> dict[str, list[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 platform-specific paths where Claude CLI might be installed.
+
+    IMPORTANT: This function mirrors the frontend's getClaudeDetectionPaths()
+    in apps/frontend/src/main/cli-tool-manager.ts. Both implementations MUST
+    be kept in sync to ensure consistent detection behavior across the
+    Python backend and Electron frontend.
+
+    When adding new detection paths, update BOTH:
+    1. This function (_get_claude_detection_paths in client.py)
+    2. getClaudeDetectionPaths() in cli-tool-manager.ts
 
     Returns:
-        Dict with 'homebrew', 'platform', and 'nvm_versions_dir' keys
+        Dict with 'homebrew', 'platform', and 'nvm' path lists
     """
-    return get_claude_detection_paths_structured()
+    home_dir = Path.home()
+    is_windows = platform.system() == "Windows"
+
+    homebrew_paths = [
+        "/opt/homebrew/bin/claude",  # Apple Silicon
+        "/usr/local/bin/claude",  # Intel Mac
+    ]
+
+    if is_windows:
+        platform_paths = [
+            str(home_dir / "AppData" / "Local" / "Programs" / "claude" / "claude.exe"),
+            str(home_dir / "AppData" / "Roaming" / "npm" / "claude.cmd"),
+            str(home_dir / ".local" / "bin" / "claude.exe"),
+            "C:\\Program Files\\Claude\\claude.exe",
+            "C:\\Program Files (x86)\\Claude\\claude.exe",
+        ]
+    else:
+        platform_paths = [
+            str(home_dir / ".local" / "bin" / "claude"),
+            str(home_dir / "bin" / "claude"),
+        ]
+
+    nvm_versions_dir = str(home_dir / ".nvm" / "versions" / "node")
+
+    return {
+        "homebrew": homebrew_paths,
+        "platform": platform_paths,
+        "nvm_versions_dir": nvm_versions_dir,
+    }
+
+
+def _is_secure_path(path_str: str) -> bool:
+    """
+    Validate that a path doesn't contain dangerous characters.
+
+    Prevents command injection attacks by rejecting paths with shell metacharacters,
+    directory traversal patterns, or environment variable expansion.
+
+    Args:
+        path_str: Path to validate
+
+    Returns:
+        True if the path is safe, False otherwise
+    """
+    import re
+
+    dangerous_patterns = [
+        r'[;&|`${}[\]<>!"^]',  # Shell metacharacters
+        r"%[^%]+%",  # Windows environment variable expansion
+        r"\.\./",  # Unix directory traversal
+        r"\.\.\\",  # Windows directory traversal
+        r"[\r\n]",  # Newlines (command injection)
+    ]
+
+    for pattern in dangerous_patterns:
+        if re.search(pattern, path_str):
+            return False
+
+    return True
 
 
 def _validate_claude_cli(cli_path: str) -> tuple[bool, str | None]:
@@ -174,11 +240,13 @@
     import re
 
     # Security validation: reject paths with shell metacharacters or directory traversal
-    if not validate_cli_path(cli_path):
+    if not _is_secure_path(cli_path):
         logger.warning(f"Rejecting insecure Claude CLI path: {cli_path}")
         return False, None
 
     try:
+        is_windows = platform.system() == "Windows"
+
         # Augment PATH with the CLI directory for proper resolution
         env = os.environ.copy()
         cli_dir = os.path.dirname(cli_path)
@@ -189,9 +257,11 @@
         # /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()
+        if is_windows and cli_path.lower().endswith((".cmd", ".bat")):
+            # Get cmd.exe path from environment or use default
+            cmd_exe = os.environ.get("ComSpec") or os.path.join(
+                os.environ.get("SystemRoot", "C:\\Windows"), "System32", "cmd.exe"
+            )
             # Use double-quoted command line for paths with spaces
             cmd_line = f'""{cli_path}" --version"'
             result = subprocess.run(
@@ -209,7 +279,7 @@
                 text=True,
                 timeout=5,
                 env=env,
-                creationflags=subprocess.CREATE_NO_WINDOW if is_windows() else 0,
+                creationflags=subprocess.CREATE_NO_WINDOW if is_windows else 0,
             )
 
         if result.returncode == 0:
@@ -247,6 +317,7 @@
             logger.debug(f"Using cached Claude CLI path: {cached}")
             return cached
 
+    is_windows = platform.system() == "Windows"
     paths = _get_claude_detection_paths()
 
     # 1. Check environment variable override
@@ -272,7 +343,7 @@
             return which_path
 
     # 3. Homebrew paths (macOS)
-    if is_macos():
+    if platform.system() == "Darwin":
         for hb_path in paths["homebrew"]:
             if Path(hb_path).exists():
                 valid, version = _validate_claude_cli(hb_path)
@@ -283,7 +354,7 @@
                     return hb_path
 
     # 4. NVM paths (Unix only) - check Node.js version manager installations
-    if not is_windows():
+    if not is_windows:
         nvm_dir = Path(paths["nvm_versions_dir"])
         if nvm_dir.exists():
             try:

apps/backend/core/dependency_validator.py

@@ -8,6 +8,8 @@
 import sys
 from pathlib import Path
 
+from core.platform import is_linux, is_windows
+
 
 def validate_platform_dependencies() -> None:
     """
@@ -18,14 +20,21 @@ def validate_platform_dependencies() -> None:
                    with helpful installation instructions.
     """
     # Check Windows-specific dependencies
-    # pywin32 is required on all Python versions on Windows (ACS-306)
-    # The MCP library unconditionally imports win32api on Windows
-    if sys.platform == "win32":
+    if is_windows() and sys.version_info >= (3, 12):
         try:
             import pywintypes  # noqa: F401
         except ImportError:
             _exit_with_pywin32_error()
 
+    # Check Linux-specific dependencies (ACS-310)
+    # Note: secretstorage is optional for app functionality (falls back to .env),
+    # but we validate it to ensure proper OAuth token storage via keyring
+    if is_linux():
+        try:
+            import secretstorage  # noqa: F401
+        except ImportError:
+            _warn_missing_secretstorage()
+
 
 def _exit_with_pywin32_error() -> None:
     """Exit with helpful error message for missing pywin32."""
@@ -76,3 +85,39 @@ def _exit_with_pywin32_error() -> None:
         "\n"
         f"Current Python: {sys.executable}\n"
     )
+
+
+def _warn_missing_secretstorage() -> None:
+    """Emit warning message for missing secretstorage.
+
+    Note: This is a warning, not a hard error - the app will fall back to .env
+    file storage for OAuth tokens. We warn users to ensure they understand the
+    security implications.
+    """
+    # Use sys.prefix to detect the virtual environment path
+    venv_activate = Path(sys.prefix) / "bin" / "activate"
+
+    sys.stderr.write(
+        "Warning: Linux dependency 'secretstorage' is not installed.\n"
+        "\n"
+        "Auto Claude can use secretstorage for secure OAuth token storage via\n"
+        "the system keyring (gnome-keyring, kwallet, etc.). Without it, tokens\n"
+        "will be stored in plaintext in your .env file.\n"
+        "\n"
+        "To enable keyring integration:\n"
+        "1. Activate your virtual environment:\n"
+        f"   source {venv_activate}\n"
+        "\n"
+        "2. Install secretstorage:\n"
+        "   pip install 'secretstorage>=3.3.3'\n"
+        "\n"
+        "   Or reinstall all dependencies:\n"
+        "   pip install -r requirements.txt\n"
+        "\n"
+        "Note: The app will continue to work, but OAuth tokens will be stored\n"
+        "in your .env file instead of the system keyring.\n"
+        "\n"
+        f"Current Python: {sys.executable}\n"
+    )
+    sys.stderr.flush()
+    # Continue execution - this is a warning, not a blocking error

apps/backend/core/simple_client.py

@@ -96,12 +96,9 @@ def create_simple_client(
         "max_turns": max_turns,
         "cwd": str(cwd.resolve()) if cwd else None,
         "env": sdk_env,
+        "max_thinking_tokens": max_thinking_tokens,
     }
 
-    # Only add max_thinking_tokens if not None (Haiku doesn't support extended thinking)
-    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

apps/backend/runners/github/orchestrator.py

@@ -660,122 +660,18 @@
 
             if not has_commits and not has_file_changes:
                 base_sha = previous_review.reviewed_commit_sha[:8]
-
-                # Check if CI status has changed since last review
-                # If CI was failing before but now passes, we need to update the verdict
-                current_failing = ci_status.get("failing", 0)
-                current_awaiting = ci_status.get("awaiting_approval", 0)
-
-                # Helper to detect CI-related blockers (includes workflows pending)
-                def is_ci_blocker(b: str) -> bool:
-                    return b.startswith("CI Failed:") or b.startswith(
-                        "Workflows Pending:"
-                    )
-
-                previous_blockers = getattr(previous_review, "blockers", [])
-                previous_was_blocked_by_ci = (
-                    previous_review.verdict == MergeVerdict.BLOCKED
-                    and any(is_ci_blocker(b) for b in previous_blockers)
-                )
-
-                # Determine the appropriate verdict based on current CI status
-                # CI/Workflow status check (both block merging)
-                ci_or_workflow_blocking = current_failing > 0 or current_awaiting > 0
-
-                if ci_or_workflow_blocking:
-                    # CI is still failing or workflows pending - keep blocked verdict
-                    updated_verdict = MergeVerdict.BLOCKED
-                    if current_failing > 0:
-                        updated_reasoning = (
-                            f"No code changes since last review. "
-                            f"{current_failing} CI check(s) still failing."
-                        )
-                        failed_checks = ci_status.get("failed_checks", [])
-                        ci_note = (
-                            f" Failing: {', '.join(failed_checks)}"
-                            if failed_checks
-                            else ""
-                        )
-                        no_change_summary = (
-                            f"No new commits since last review. "
-                            f"CI status: {current_failing} check(s) failing.{ci_note}"
-                        )
-                    else:
-                        updated_reasoning = (
-                            f"No code changes since last review. "
-                            f"{current_awaiting} workflow(s) awaiting approval."
-                        )
-                        no_change_summary = (
-                            f"No new commits since last review. "
-                            f"{current_awaiting} workflow(s) awaiting maintainer approval."
-                        )
-                elif previous_was_blocked_by_ci and not ci_or_workflow_blocking:
-                    # CI/Workflows have recovered! Update verdict to reflect this
-                    safe_print(
-                        "[Followup] CI recovered - updating verdict from BLOCKED",
-                        flush=True,
-                    )
-                    # Check for remaining non-CI blockers (use helper defined above)
-                    non_ci_blockers = [
-                        b for b in previous_blockers if not is_ci_blocker(b)
-                    ]
-
-                    # Determine verdict based on findings AND remaining blockers
-                    if non_ci_blockers:
-                        # There are still non-CI blockers - stay blocked
-                        updated_verdict = MergeVerdict.BLOCKED
-                        updated_reasoning = (
-                            "CI checks now passing. Non-CI blockers still remain: "
-                            + ", ".join(non_ci_blockers[:3])
-                        )
-                    elif previous_review.findings:
-                        # Check finding severity - only low severity is non-blocking
-                        findings = previous_review.findings
-                        high_medium = [
-                            f
-                            for f in findings
-                            if f.severity
-                            in (
-                                ReviewSeverity.HIGH,
-                                ReviewSeverity.MEDIUM,
-                                ReviewSeverity.CRITICAL,
-                            )
-                        ]
-                        if high_medium:
-                            # There are blocking findings - needs revision
-                            updated_verdict = MergeVerdict.NEEDS_REVISION
-                            updated_reasoning = f"CI checks now passing. {len(high_medium)} code finding(s) still require attention."
-                        else:
-                            # Only low-severity findings - safe to merge
-                            updated_verdict = MergeVerdict.READY_TO_MERGE
-                            updated_reasoning = f"CI checks now passing. {len(findings)} non-blocking suggestion(s) to consider."
-                    else:
-                        updated_verdict = MergeVerdict.READY_TO_MERGE
-                        updated_reasoning = (
-                            "CI checks now passing. No outstanding code issues."
-                        )
-                    no_change_summary = (
-                        "No new commits since last review. "
-                        "CI checks are now passing. Previous findings still apply."
-                    )
-                else:
-                    # No CI-related changes, keep previous verdict
-                    updated_verdict = previous_review.verdict
-                    updated_reasoning = "No changes since last review."
-                    no_change_summary = "No new commits since last review. Previous findings still apply."
-
                 safe_print(
                     f"[Followup] No changes since last review at {base_sha}",
                     flush=True,
                 )
 
                 # Build blockers list - always filter out CI blockers first, then add current
                 blockers = list(previous_blockers)
                 # Remove ALL CI-related blockers (CI Failed + Workflows Pending)
                 blockers = [b for b in blockers if not is_ci_blocker(b)]
 
                 # Add back only currently failing CI checks
                 if current_failing > 0:
                     failed_checks = ci_status.get("failed_checks", [])
                     for check_name in failed_checks:
                         blocker_msg = f"CI Failed: {check_name}"
@@ -783,13 +679,13 @@
                             blockers.append(blocker_msg)
 
                 # Add back workflows pending if any
                 if current_awaiting > 0:
                     blocker_msg = f"Workflows Pending: {current_awaiting} workflow(s) awaiting maintainer approval"
                     if blocker_msg not in blockers:
                         blockers.append(blocker_msg)
 
                 # Map verdict to overall_status (consistent with rest of codebase)
                 if updated_verdict == MergeVerdict.BLOCKED:
                     overall_status = "request_changes"
                 elif updated_verdict == MergeVerdict.NEEDS_REVISION:
                     overall_status = "request_changes"

apps/backend/runners/github/services/parallel_orchestrator_reviewer.py

@@ -532,31 +532,12 @@ async def review(self, context: PRContext) -> PRReviewResult:
                         head_sha, context.pr_number
                     )
                     project_root = worktree_path
-                    # Count files in worktree to give user visibility (with limit to avoid slowdown)
-                    MAX_FILE_COUNT = 10000
-                    try:
-                        file_count = 0
-                        for f in worktree_path.rglob("*"):
-                            if f.is_file() and ".git" not in f.parts:
-                                file_count += 1
-                                if file_count >= MAX_FILE_COUNT:
-                                    break
-                    except (OSError, PermissionError):
-                        file_count = 0
-                    file_count_str = (
-                        f"{file_count:,}+"
-                        if file_count >= MAX_FILE_COUNT
-                        else f"{file_count:,}"
-                    )
-                    # Always log worktree creation with file count (not gated by DEBUG_MODE)
-                    safe_print(
-                        f"[PRReview] Created temporary worktree: {worktree_path.name} ({file_count_str} files)",
-                        flush=True,
-                    )
-                    safe_print(
-                        f"[PRReview] Worktree contains PR branch HEAD: {head_sha[:8]}",
-                        flush=True,
-                    )
+                    if DEBUG_MODE:
+                        safe_print(
+                            f"[PRReview] DEBUG: Using worktree as "
+                            f"project_root={project_root}",
+                            flush=True,
+                        )
                 except (RuntimeError, ValueError) as e:
                     if DEBUG_MODE:
                         safe_print(