From 4a1a69d30952d7390e732e351dbd54185120c92b Mon Sep 17 00:00:00 2001 From: Waleed Zafar Date: Sat, 19 Sep 2026 00:25:11 +0500 Subject: [PATCH 1/3] feat(auth): make claude-api-key optional so the action can run keyless MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why Every repo in the org is moving off static Anthropic API keys to workload identity federation (NEX-2513). This action is the last consumer that hard-fails without `claude-api-key`, so it alone keeps a static key alive in repo secrets. ## What changes Cherry-picks upstream PR anthropics/claude-code-security-review#116 (@ralphpina), which is open against an upstream `main` that has not moved since 2026-02-11: - `action.yml` — `claude-api-key` becomes `required: false`, and the scan step unsets an empty `ANTHROPIC_API_KEY` rather than exiting 1. An empty-but-set value outranks keyless auth, so unsetting it is what makes federation reachable. - `claude_api_client.py` — construct `Anthropic()` with no key when none is present and let the SDK resolve environment credentials, instead of raising. - `github_action_audit.py` — `validate_claude_available()` accepts a static credential or federation env, with an error message that names both. Passing `claude-api-key` behaves exactly as before; this only adds a fallback path when it is absent. Two fork-local fixes on top of the upstream patch: - the conflict against our cost-metrics change (#3) is resolved by keeping the keyless client construction and our `usage` / `_record_usage` block; - `logger.info("Claude API client initialized successfully")` had ended up as the last statement of `_record_usage` in #3, so it logged on every API response. Moved back into `__init__`. Co-Authored-By: Claude Opus 5 (1M context) --- action.yml | 18 +++++++++--------- claudecode/claude_api_client.py | 24 +++++++++++++++--------- claudecode/github_action_audit.py | 27 +++++++++++++++++++++++---- 3 files changed, 47 insertions(+), 22 deletions(-) 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..5ff4a04 100644 --- a/claudecode/github_action_audit.py +++ b/claudecode/github_action_audit.py @@ -412,10 +412,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}" From a37e79d730b2eec8ec550d5c9f1361f59150f604 Mon Sep 17 00:00:00 2001 From: Waleed Zafar Date: Sat, 19 Sep 2026 00:44:08 +0500 Subject: [PATCH 2/3] fix(auth): keep false-positive filtering on under keyless auth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why Upstream #116 unblocked ClaudeAPIClient but missed create_findings_filter, which gates Claude filtering on `use_claude_filtering and api_key`. Under federation there is no api_key, so the false-positive filter silently switched off — a keyless scan on nexus-status came back with `"justification": "Claude filtering disabled"` while still reporting findings and exiting 0. That is the worst shape of failure here: the scan looks clean and the PR comment just gets noisier. ## What changes Gate on `use_claude_filtering` alone and let the client resolve the credential. FindingsFilter already validates API access on construction and degrades to hard rules if it fails, so the key check bought nothing that the fallback did not. Co-Authored-By: Claude Opus 5 (1M context) --- claudecode/github_action_audit.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/claudecode/github_action_audit.py b/claudecode/github_action_audit.py index 5ff4a04..8711cf6 100644 --- a/claudecode/github_action_audit.py +++ b/claudecode/github_action_audit.py @@ -517,9 +517,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, From becec67c73d626bdc91df6c509510051c5b500da Mon Sep 17 00:00:00 2001 From: Waleed Zafar Date: Sat, 19 Sep 2026 01:21:12 +0500 Subject: [PATCH 3/3] fix(scan): reach the drop-the-diff fallback when the prompt is too long MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why The fallback that re-runs the audit without the diff has been unreachable. nexus-status hit it for real: a PR that swept in build artifacts produced a ~1.2M-token prompt against the 1M limit, and instead of retrying without the diff the action reported `Claude Code execution failed with return code 1` and the PR got no review at all. Two independent faults, either one fatal: - The detection sat inside the `returncode == 0` branch. The CLI writes its result envelope to stdout and *still* exits non-zero on an API error, so stdout was never parsed. Worse, the loop treated it as a transient failure and spent all three retries resending the same oversized prompt. - The test was `result == 'Prompt is too long'`, exact equality. The CLI now appends the measured counts — `Prompt is too long · the request is ~1199699 tokens (limit 1000000) …` — so equality could never hold again. This is not the same gap the 406 work closed. That change made an oversized diff *fetchable* by assembling it from per-file patches; it makes the prompt bigger, not smaller. Nothing downstream bounded what that diff cost in tokens. ## What changes - `github_action_audit.py`: parse stdout before branching on the return code, and match the message by prefix. Usage is still recorded exactly once per attempt, so a billed prompt-too-long attempt keeps being counted. - `test_claude_runner.py`: a regression test built from the observed envelope — non-zero exit plus the counts-bearing message — asserting both that the fallback fires and that the oversized prompt is not retried. It fails against the previous code with the production error string. - Same file: the `validate_claude_available` assertion still expected the key-only message that upstream #116 replaced. Updated to the message that names both credential routes; the suite was 180/1 and is now 181/0. Co-Authored-By: Claude Opus 5 (1M context) --- claudecode/github_action_audit.py | 35 +++++++++++++++++++------------ claudecode/test_claude_runner.py | 28 ++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 14 deletions(-) diff --git a/claudecode/github_action_audit.py b/claudecode/github_action_audit.py index 8711cf6..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 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='')