Fix review pipeline bugs, improve performance, and harden against prompt injection

README.md

@@ -99,7 +99,7 @@ jobs:
 
 ## Security Considerations
 
-This action is not hardened against prompt injection attacks and should only be used to review trusted PRs. We recommend [configuring your repository](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/managing-github-actions-settings-for-a-repository#controlling-changes-from-forks-to-workflows-in-public-repositories) to use the "Require approval for all external contributors" option to ensure workflows only run after a maintainer has reviewed the PR.
+This action includes several defense-in-depth mitigations against prompt injection: GitHub credentials are removed from the review subprocess environment and from the checkout's git config before analysis, the subprocess's shell access is restricted to an allowlist of read-only git commands (with network tools additionally denylisted), and the review prompt instructs Claude to treat instructions embedded in PR content as a malicious signal to report rather than follow. These measures raise the bar but are not a sandbox and cannot eliminate prompt-injection risk — the action should still only be used to review trusted PRs. We recommend [configuring your repository](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/managing-github-actions-settings-for-a-repository#controlling-changes-from-forks-to-workflows-in-public-repositories) to use the "Require approval for all external contributors" option to ensure workflows only run after a maintainer has reviewed the PR.
 
 ## Configuration Options
 

action.yml

@@ -152,8 +152,13 @@ runs:
       shell: bash
       run: |
         echo "::group::Install gh CLI"
-        # Install GitHub CLI for PR operations
-        sudo apt-get update && sudo apt-get install -y gh
+        # Install GitHub CLI for PR operations (pre-installed on GitHub-hosted
+        # runners - skip the ~30s apt round-trip when already present)
+        if command -v gh >/dev/null 2>&1; then
+          echo "gh already installed: $(gh --version | head -n 1)"
+        else
+          sudo apt-get update && sudo apt-get install -y gh
+        fi
         echo "::endgroup::"
 
     - name: Get PR info for issue_comment events
@@ -346,7 +351,7 @@ runs:
       if: steps.claudecode-check.outputs.enable_claudecode == 'true'
       uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4
       with:
-        node-version: '18'
+        node-version: '22'
 
     - name: Setup git for diffing
       if: steps.claudecode-check.outputs.enable_claudecode == 'true'
@@ -387,7 +392,10 @@ runs:
         echo "::group::Install Deps"
         pip install -r "$ACTION_PATH/claudecode/requirements.txt"
         npm install -g @anthropic-ai/claude-code
-        sudo apt-get update && sudo apt-get install -y jq
+        # jq is pre-installed on GitHub-hosted runners - only install if missing
+        if ! command -v jq >/dev/null 2>&1; then
+          sudo apt-get update && sudo apt-get install -y jq
+        fi
         echo "::endgroup::"
 
     - name: Run ClaudeCode scan
@@ -436,7 +444,17 @@ runs:
 
         # Set timeout
         export CLAUDE_TIMEOUT="$CLAUDECODE_TIMEOUT"
-
+
+        # Remove credentials that actions/checkout persisted into git config.
+        # All git fetches happened in earlier steps and later steps use the
+        # GITHUB_TOKEN env var, so nothing after this point needs them - but
+        # the Claude review subprocess (which explores this checkout) must
+        # not be able to read the workflow token from .git/config.
+        for key in $(git config --local --list --name-only 2>/dev/null | grep -i 'extraheader$' || true); do
+          git config --local --unset-all "$key" || true
+          echo "Removed persisted git credential config: $key"
+        done
+
         # Run ClaudeCode audit with verbose debugging
         export REPO_PATH=$(pwd)
         cd "$ACTION_PATH"

claudecode/claude_api_client.py

@@ -11,6 +11,7 @@
 from claudecode.constants import (
     DEFAULT_CLAUDE_MODEL, DEFAULT_TIMEOUT_SECONDS, DEFAULT_MAX_RETRIES,
     RATE_LIMIT_BACKOFF_MAX, PROMPT_TOKEN_LIMIT,
+    FILTER_FILE_CONTEXT_LINES, FILTER_FILE_MAX_CHARS,
 )
 from claudecode.json_parser import parse_json_with_fallbacks
 from claudecode.logger import get_logger
@@ -52,14 +53,18 @@ def __init__(self,
 
     def validate_api_access(self) -> Tuple[bool, str]:
         """Validate that API access is working.
-
+
+        Pings the same model used for filtering, so a misconfigured or
+        retired CLAUDE_MODEL is caught here instead of silently failing
+        (fail-open) on every per-finding call later.
+
         Returns:
             Tuple of (success, error_message)
         """
         try:
             # Simple test call to verify API access
             self.client.messages.create(
-                model="claude-3-5-haiku-20241022",
+                model=self.model,
                 max_tokens=10,
                 messages=[{"role": "user", "content": "Hello"}],
                 timeout=10
@@ -216,15 +221,16 @@ def _generate_single_finding_prompt(self,
 - Description: {(pr_context.get('description') or 'No description')[:500]}...
 """
 
-        # Get file content if available
+        # Get file content if available (windowed around the finding line,
+        # with line numbers so the model can verify the flagged location)
         file_path = finding.get('file', '')
         file_content = ""
         if file_path:
-            success, content, error = self._read_file(file_path)
+            success, content, error = self._read_file(file_path, focus_line=finding.get('line'))
             if success:
                 file_content = f"""
 
-File Content ({file_path}):
+File Content ({file_path}, with line numbers):
 ```
 {content}
 ```"""
@@ -289,12 +295,17 @@ def _generate_single_finding_prompt(self,
 }}"""
 
 
-    def _read_file(self, file_path: str) -> Tuple[bool, str, str]:
+    def _read_file(self, file_path: str, focus_line: Optional[int] = None) -> Tuple[bool, str, str]:
         """Read a file and format it with line numbers.
-
+
+        Large files are windowed around focus_line (or truncated from the top
+        when no focus line is given) so a single huge file can't blow up the
+        filter prompt.
+
         Args:
             file_path: Path to the file to read
-
+            focus_line: Optional 1-based line number to center the window on
+
         Returns:
             Tuple of (success, formatted_content, error_message)
         """
@@ -324,14 +335,94 @@ def _read_file(self, file_path: str) -> Tuple[bool, str, str]:
                 # Try with latin-1 encoding as fallback
                 with open(path, 'r', encoding='latin-1') as f:
                     content = f.read()
-            
-            return True, content, ""
+
+            return True, self._format_file_window(content, focus_line), ""
 
         except Exception as e:
             error_msg = f"Error reading file {file_path}: {str(e)}"
             logger.error(error_msg)
             return False, "", error_msg
 
+    @staticmethod
+    def _format_file_window(content: str,
+                            focus_line: Optional[int] = None,
+                            context_lines: int = FILTER_FILE_CONTEXT_LINES,
+                            max_chars: int = FILTER_FILE_MAX_CHARS) -> str:
+        """Add line numbers and window content around a focus line.
+
+        Small files are returned whole (numbered). For larger files, a window
+        of ±context_lines around focus_line is used. The character budget is
+        applied by shrinking the window symmetrically around the focus line -
+        never by chopping off the tail - so the flagged line is always present
+        in what the filter model sees.
+        """
+        lines = content.split('\n')
+        total_lines = len(lines)
+
+        # A stale finding may reference a line beyond EOF - clamp it so the
+        # window still shows the end of the file instead of nothing.
+        if isinstance(focus_line, int) and focus_line > total_lines:
+            focus_line = total_lines
+
+        start = 0
+        end = total_lines
+        if total_lines > 2 * context_lines:
+            if isinstance(focus_line, int) and focus_line > 0:
+                start = max(0, focus_line - 1 - context_lines)
+                end = min(total_lines, focus_line - 1 + context_lines + 1)
+            else:
+                end = 2 * context_lines
+
+        def render(window_start: int, window_end: int) -> str:
+            numbered = []
+            if window_start > 0:
+                numbered.append(f"... ({window_start} earlier lines omitted)")
+            for idx in range(window_start, window_end):
+                numbered.append(f"{idx + 1:>6}\t{lines[idx]}")
+            if window_end < total_lines:
+                numbered.append(f"... ({total_lines - window_end} later lines omitted)")
+            return '\n'.join(numbered)
+
+        result = render(start, end)
+        if len(result) <= max_chars:
+            return result
+
+        # Over budget: grow a window outward from the focus line, one line at
+        # a time, so the focus line is guaranteed to fit within max_chars.
+        if isinstance(focus_line, int) and 0 < focus_line <= total_lines:
+            anchor = focus_line - 1
+        else:
+            anchor = start
+        anchor = min(max(anchor, start), end - 1)
+
+        def line_cost(idx: int) -> int:
+            return len(f"{idx + 1:>6}\t{lines[idx]}") + 1  # +1 for newline
+
+        budget = max(max_chars - 200, 200)  # headroom for the omission markers
+
+        if line_cost(anchor) > budget:
+            # Even the focus line alone exceeds the budget - include a
+            # truncated version of it rather than nothing.
+            focus_text = lines[anchor][:budget]
+            return f"{anchor + 1:>6}\t{focus_text}\n... (line truncated; surrounding content omitted)"
+
+        low = high = anchor
+        used = line_cost(anchor)
+        while True:
+            grew = False
+            if low - 1 >= start and used + line_cost(low - 1) <= budget:
+                low -= 1
+                used += line_cost(low)
+                grew = True
+            if high + 1 < end and used + line_cost(high + 1) <= budget:
+                high += 1
+                used += line_cost(high)
+                grew = True
+            if not grew:
+                break
+
+        return render(low, high + 1)
+
 
 def get_claude_api_client(model: str = DEFAULT_CLAUDE_MODEL,
                          api_key: Optional[str] = None,

claudecode/constants.py

@@ -9,10 +9,17 @@
 DEFAULT_TIMEOUT_SECONDS = 180  # 3 minutes
 DEFAULT_MAX_RETRIES = 3
 RATE_LIMIT_BACKOFF_MAX = 30  # Maximum backoff time for rate limits
+GITHUB_REQUEST_TIMEOUT = 30  # Timeout for GitHub API HTTP requests
+# Concurrency for per-finding Claude API validation calls
+FILTER_MAX_WORKERS = 4
 
 # Token Limits
 PROMPT_TOKEN_LIMIT = 16384  # Output cap for filter/validator API calls
 
+# File-content windowing for per-finding filter prompts
+FILTER_FILE_CONTEXT_LINES = 150  # Lines of context around the finding line
+FILTER_FILE_MAX_CHARS = 40000  # Hard cap on file content embedded per finding
+
 # Diff Construction Limits
 DEFAULT_MAX_DIFF_CHARS = 800000  # 800k characters (~200k tokens; fits comfortably in 1M context models)
 # Conversion factor for deprecated MAX_DIFF_LINES -> MAX_DIFF_CHARS

claudecode/findings_filter.py

@@ -1,12 +1,13 @@
 """Findings filter for reducing false positives in code review results."""
 
 import re
+from concurrent.futures import ThreadPoolExecutor
 from typing import Dict, Any, List, Tuple, Optional, Pattern
 import time
 from dataclasses import dataclass, field
 
 from claudecode.claude_api_client import ClaudeAPIClient
-from claudecode.constants import DEFAULT_CLAUDE_MODEL
+from claudecode.constants import DEFAULT_CLAUDE_MODEL, FILTER_MAX_WORKERS
 from claudecode.logger import get_logger
 
 logger = get_logger(__name__)
@@ -168,8 +169,8 @@ def get_exclusion_reason(cls, finding: Dict[str, Any]) -> Optional[str]:
                 if pattern.search(combined_text):
                     return "Regex injection finding (not applicable)"
 
-        # Check memory safety patterns - exclude if NOT in C/C++ files
-        c_cpp_extensions = {'.c', '.cc', '.cpp', '.h'}
+        # Check memory safety patterns - exclude if NOT in C/C++/Objective-C files
+        c_cpp_extensions = {'.c', '.cc', '.cpp', '.cxx', '.h', '.hh', '.hpp', '.hxx', '.m', '.mm'}
         file_ext = ''
         if '.' in file_path:
             file_ext = f".{file_path.lower().split('.')[-1]}"
@@ -295,15 +296,26 @@ def filter_findings(self,
         excluded_claude = []
 
         if self.use_claude_filtering and self.claude_client and findings_after_hard:
-            # Process findings individually
-            logger.info(f"Processing {len(findings_after_hard)} findings individually through Claude API")
-
-            for orig_idx, finding in findings_after_hard:
-                # Call Claude API for single finding
-                success, analysis_result, error_msg = self.claude_client.analyze_single_finding(
+            # Process findings individually, in parallel (each analysis is an
+            # independent API call; ordering of results is preserved by map)
+            logger.info(f"Processing {len(findings_after_hard)} findings individually through Claude API "
+                        f"({FILTER_MAX_WORKERS} workers)")
+
+            def _analyze(item):
+                _, finding = item
+                return self.claude_client.analyze_single_finding(
                     finding, pr_context, self.custom_filtering_instructions
                 )
-
+
+            max_workers = min(FILTER_MAX_WORKERS, len(findings_after_hard))
+            with ThreadPoolExecutor(max_workers=max_workers) as executor:
+                analysis_results = list(executor.map(_analyze, findings_after_hard))
+
+            api_failures = 0
+
+            for (orig_idx, finding), (success, analysis_result, error_msg) in zip(
+                findings_after_hard, analysis_results
+            ):
                 if success and analysis_result:
                     # Process Claude's analysis for single finding
                     confidence = analysis_result.get('confidence_score', 10.0)
@@ -335,13 +347,21 @@ def filter_findings(self,
                 else:
                     # Claude API call failed for this finding - keep it with warning
                     logger.warning(f"Claude API call failed for finding {orig_idx}: {error_msg}")
+                    api_failures += 1
                     enriched_finding = finding.copy()
                     enriched_finding['_filter_metadata'] = {
                         'confidence_score': 10.0,  # Default high confidence
                         'justification': f'Claude API failed: {error_msg}',
                     }
                     findings_after_claude.append(enriched_finding)
                     stats.kept_findings += 1
+
+            if api_failures and api_failures == len(findings_after_hard):
+                logger.warning(
+                    f"Claude filtering was effectively disabled for this run: all "
+                    f"{api_failures} validation calls failed and every finding was "
+                    f"kept unvalidated (fail-open)."
+                )
         else:
             # Claude filtering disabled or no client - keep all findings from hard filter
             for orig_idx, finding in findings_after_hard: