diff --git a/action.yml b/action.yml index 0e10e6a..f83d339 100644 --- a/action.yml +++ b/action.yml @@ -24,8 +24,8 @@ inputs: default: '20' claude-api-key: - description: 'Anthropic Claude API key for security analysis' - required: true + description: 'Anthropic Claude API key for security analysis. Optional: if omitted, the action uses keyless / environment-based auth resolved by the Anthropic SDK and Claude CLI (e.g. Workload Identity Federation via ANTHROPIC_FEDERATION_RULE_ID / ANTHROPIC_ORGANIZATION_ID / ANTHROPIC_SERVICE_ACCOUNT_ID plus an OIDC identity token, or ANTHROPIC_AUTH_TOKEN). Set those in the calling workflow env when using keyless auth.' + required: false default: '' claude-model: @@ -203,14 +203,14 @@ runs: exit 0 fi - # Validate API key is provided + # Auth: a static ANTHROPIC_API_KEY is optional. If no key was provided, + # unset the (empty) variable so the Claude CLI and Anthropic SDK can + # resolve keyless credentials from the environment (e.g. Workload + # Identity Federation). An empty-but-set ANTHROPIC_API_KEY would + # otherwise take precedence over keyless auth and break it. Truly + # missing credentials are surfaced by the scan's own validation. if [ -z "$ANTHROPIC_API_KEY" ]; then - echo "::error::ANTHROPIC_API_KEY is not set. Please provide the claude-api-key input to the action." - echo "Example usage:" - echo " - uses: anthropics/claude-code-security-reviewer@main" - echo " with:" - echo " claude-api-key: \$\{{ secrets.ANTHROPIC_API_KEY }}" - exit 1 + unset ANTHROPIC_API_KEY fi # Set timeout diff --git a/claudecode/claude_api_client.py b/claudecode/claude_api_client.py index 8433154..c8a2635 100644 --- a/claudecode/claude_api_client.py +++ b/claudecode/claude_api_client.py @@ -38,16 +38,22 @@ def __init__(self, self.timeout_seconds = timeout_seconds or DEFAULT_TIMEOUT_SECONDS self.max_retries = max_retries or DEFAULT_MAX_RETRIES - # Get API key from environment or parameter + # Resolve auth. An explicit key (arg or ANTHROPIC_API_KEY) is used + # directly. Otherwise, fall through to the SDK's own credential + # resolution rather than failing hard -- this enables keyless auth such + # as Workload Identity Federation (ANTHROPIC_FEDERATION_RULE_ID / + # ANTHROPIC_ORGANIZATION_ID / ANTHROPIC_SERVICE_ACCOUNT_ID plus an OIDC + # identity token) and ANTHROPIC_AUTH_TOKEN, all of which the Anthropic + # SDK auto-detects from the environment. self.api_key = api_key or os.environ.get("ANTHROPIC_API_KEY") - if not self.api_key: - raise ValueError( - "No Anthropic API key found. Please set ANTHROPIC_API_KEY environment variable " - "or provide api_key parameter." - ) - - # Initialize Anthropic client - self.client = Anthropic(api_key=self.api_key) + if self.api_key: + self.client = Anthropic(api_key=self.api_key) + else: + # No static key -- let the SDK auto-detect credentials from the + # environment. It raises a clear authentication error at call time + # if nothing is configured. + self.client = Anthropic() + logger.info("Claude API client initialized successfully") # Token usage summed over every successful messages.create in this client self.usage = { 'calls': 0, 'input_tokens': 0, 'output_tokens': 0, diff --git a/claudecode/github_action_audit.py b/claudecode/github_action_audit.py index c0087ae..45faca8 100644 --- a/claudecode/github_action_audit.py +++ b/claudecode/github_action_audit.py @@ -328,6 +328,27 @@ def run_security_audit(self, repo_dir: Path, prompt: str) -> Tuple[bool, str, Di timeout=self.timeout_seconds ) + # Parse BEFORE branching on the return code. The CLI writes its + # result envelope to stdout and STILL exits non-zero on an API + # error, so a prompt-too-long detected only on the returncode==0 + # path is unreachable: the loop burns all three retries resending + # the same oversized prompt and reports the generic "execution + # failed with return code 1" instead of falling back. + success, parsed_result = parse_json_with_fallbacks(result.stdout, "Claude Code output") + if success: + self._record_usage(parsed_result) + + # Match the PREFIX, not the whole string. The CLI now appends the + # measured counts - "Prompt is too long \u00b7 the request is + # ~1199699 tokens (limit 1000000) ..." - so the old equality test + # could never fire again. Measured on nexus-status, where a PR + # carrying build artifacts produced a 1.2M-token prompt. + if (success and isinstance(parsed_result, dict) and + parsed_result.get('type') == 'result' and + parsed_result.get('is_error') and + str(parsed_result.get('result') or '').startswith('Prompt is too long')): + return False, "PROMPT_TOO_LONG", {} + if result.returncode != 0: if attempt == NUM_RETRIES - 1: error_details = f"Claude Code execution failed with return code {result.returncode}\n" @@ -338,20 +359,8 @@ def run_security_audit(self, repo_dir: Path, prompt: str) -> Tuple[bool, str, Di time.sleep(5*attempt) # Note: We don't do exponential backoff here to keep the runtime reasonable continue # Retry - - # Parse JSON output - success, parsed_result = parse_json_with_fallbacks(result.stdout, "Claude Code output") - + if success: - self._record_usage(parsed_result) - # Check for "Prompt is too long" error that should trigger retry without diff - if (isinstance(parsed_result, dict) and - parsed_result.get('type') == 'result' and - parsed_result.get('subtype') == 'success' and - parsed_result.get('is_error') and - parsed_result.get('result') == 'Prompt is too long'): - return False, "PROMPT_TOO_LONG", {} - # Check for error_during_execution that should trigger retry if (isinstance(parsed_result, dict) and parsed_result.get('type') == 'result' and @@ -412,10 +421,29 @@ def validate_claude_available(self) -> Tuple[bool, str]: ) if result.returncode == 0: - # Also check if API key is configured - api_key = os.environ.get('ANTHROPIC_API_KEY', '') - if not api_key: - return False, "ANTHROPIC_API_KEY environment variable is not set" + # Accept either a static credential or keyless / environment-based + # auth (e.g. Workload Identity Federation, ANTHROPIC_AUTH_TOKEN), + # which the Claude CLI and Anthropic SDK resolve from the + # environment. + has_static_credential = bool( + os.environ.get('ANTHROPIC_API_KEY') + or os.environ.get('ANTHROPIC_AUTH_TOKEN') + ) + has_wif = all( + os.environ.get(var) + for var in ( + 'ANTHROPIC_FEDERATION_RULE_ID', + 'ANTHROPIC_ORGANIZATION_ID', + 'ANTHROPIC_SERVICE_ACCOUNT_ID', + ) + ) + if not (has_static_credential or has_wif): + return False, ( + "No Anthropic credentials configured. Set ANTHROPIC_API_KEY, " + "or configure Workload Identity Federation " + "(ANTHROPIC_FEDERATION_RULE_ID / ANTHROPIC_ORGANIZATION_ID / " + "ANTHROPIC_SERVICE_ACCOUNT_ID)." + ) return True, "" else: error_msg = f"Claude Code returned exit code {result.returncode}" @@ -498,9 +526,15 @@ def initialize_findings_filter(custom_filtering_instructions: Optional[str] = No try: # Check if we should use Claude API filtering use_claude_filtering = os.environ.get('ENABLE_CLAUDE_FILTERING', 'false').lower() == 'true' + # May be absent under keyless auth; the client resolves environment + # credentials in that case. Do NOT gate filtering on it -- requiring a + # key here silently turns the false-positive filter OFF under workload + # identity federation, which reads as a clean scan with noisier findings + # rather than as a failure. FindingsFilter already degrades to hard rules + # on its own if the credential turns out not to work. api_key = os.environ.get('ANTHROPIC_API_KEY') - if use_claude_filtering and api_key: + if use_claude_filtering: # Use full filtering with Claude API return FindingsFilter( use_hard_exclusions=True, diff --git a/claudecode/test_claude_runner.py b/claudecode/test_claude_runner.py index bb62de5..3a9b96f 100644 --- a/claudecode/test_claude_runner.py +++ b/claudecode/test_claude_runner.py @@ -64,7 +64,10 @@ def test_validate_claude_available_no_api_key(self, mock_run): success, error = runner.validate_claude_available() assert success is False - assert 'ANTHROPIC_API_KEY environment variable is not set' in error + # Since #116 the check accepts a federated credential too, so the + # message names both routes rather than the key alone. + assert 'No Anthropic credentials configured' in error + assert 'ANTHROPIC_FEDERATION_RULE_ID' in error @patch('subprocess.run') def test_validate_claude_available_not_installed(self, mock_run): @@ -477,6 +480,29 @@ def test_usage_accumulates_across_attempts(self, mock_run): assert abs(runner.usage['total_cost_usd'] - 0.30) < 1e-9 assert runner.usage['duration_ms'] == 1010 + @patch('subprocess.run') + def test_prompt_too_long_is_caught_on_a_nonzero_exit_with_counts(self, mock_run): + """The real shape: the CLI exits 1 AND appends the token counts. + + Both halves defeated the original check - it only looked at stdout when + the exit code was 0, and it compared the message for equality. Without + the fix this burns three retries and returns the generic return-code + error, so the caller never drops the diff and the PR gets no review. + """ + envelope = json.dumps({ + 'type': 'result', 'subtype': 'success', 'is_error': True, + 'api_error_status': 400, 'duration_ms': 562, 'num_turns': 1, + 'result': ('Prompt is too long \u00b7 the request is ~1199699 tokens ' + '(limit 1000000) but this conversation is only ~436850 tokens'), + }) + mock_run.return_value = Mock(returncode=1, stdout=envelope, stderr='') + runner = SimpleClaudeRunner() + with patch('pathlib.Path.exists', return_value=True): + success, err, _ = runner.run_security_audit(Path('/tmp/test'), 'p') + assert success is False + assert err == 'PROMPT_TOO_LONG' + assert mock_run.call_count == 1, 'must not retry the same oversized prompt' + @patch('subprocess.run') def test_usage_absent_fields_are_ignored(self, mock_run): mock_run.return_value = Mock(returncode=0, stdout=json.dumps({'type': 'result', 'result': '{"findings": []}'}), stderr='')