Skip to content

Support keyless / environment-based authentication (Workload Identity Federation) - #116

Open
ralphpina wants to merge 1 commit into
anthropics:mainfrom
ralphpina:rpina/keyless-env-auth
Open

ralphpina wants to merge 1 commit into
anthropics:mainfrom
ralphpina:rpina/keyless-env-auth

Conversation

@ralphpina

Copy link
Copy Markdown

Summary

Makes claude-api-key optional so the action can authenticate keylessly via credentials the Anthropic SDK and Claude CLI already resolve from the environment — most importantly Workload Identity Federation (WIF), but also ANTHROPIC_AUTH_TOKEN, Amazon Bedrock / Google Vertex, or an ant profile.

Motivation

Organizations moving off static Anthropic API keys (to WIF / short-lived OIDC-exchanged tokens) can already run anthropics/claude-code-action keylessly — but not claude-code-security-review. It requires claude-api-key and hard-fails without it, which forces a static key to stay in repo secrets. On a repo that otherwise runs fully keyless, this one action blocks revoking the last static key.

What changed (all backward compatible)

  1. action.yml — claude-api-key is now required: false. When no key is provided, the scan step unsets the (empty) ANTHROPIC_API_KEY — an empty-but-set value otherwise takes precedence over keyless auth in the SDK and breaks it.
  2. claudecode/claude_api_client.py — when no explicit/env key is present, construct Anthropic() with no api_key and let the SDK auto-detect environment credentials, instead of raising ValueError.
  3. claudecode/github_action_audit.py — validate_claude_available() accepts a static credential or WIF env (ANTHROPIC_FEDERATION_RULE_ID / ANTHROPIC_ORGANIZATION_ID / ANTHROPIC_SERVICE_ACCOUNT_ID) / ANTHROPIC_AUTH_TOKEN, with an updated error message.

Providing claude-api-key behaves exactly as before — this only adds a fallback path when it's absent.

Using keyless auth (WIF example)

The action deliberately does not mint the OIDC token itself (keeps it minimal and provider-agnostic); the calling workflow sets the WIF env before invoking it:

permissions:
  id-token: write   # allow minting the GitHub OIDC token
jobs:
  security-review:
    steps:
      - uses: anthropics/claude-code-security-review@<sha>
        # no claude-api-key
        env:
          ANTHROPIC_FEDERATION_RULE_ID: ${{ vars.ANTHROPIC_FEDERATION_RULE_ID }}
          ANTHROPIC_ORGANIZATION_ID:    ${{ vars.ANTHROPIC_ORGANIZATION_ID }}
          ANTHROPIC_SERVICE_ACCOUNT_ID: ${{ vars.ANTHROPIC_SERVICE_ACCOUNT_ID }}
          ANTHROPIC_IDENTITY_TOKEN:     ${{ steps.oidc.outputs.token }}  # GH OIDC token, audience https://api.anthropic.com

Testing

  • py_compile on both changed modules ✅; action.yml parses as valid YAML ✅.
  • Key-present path is unchanged (same Anthropic(api_key=...) construction, same validation result).
  • ⚠️ I could not run the action's CI or a live WIF GitHub Actions run end-to-end in my environment — the keyless path needs a WIF-configured repo plus your CI to validate. Happy to iterate on anything.

🤖 Generated with Claude Code

Make `claude-api-key` optional so the action can authenticate via
credentials the Anthropic SDK and Claude CLI resolve from the
environment -- Workload Identity Federation, ANTHROPIC_AUTH_TOKEN,
Bedrock/Vertex, etc. -- instead of requiring a static API key.

- action.yml: claude-api-key required:false; unset an empty
  ANTHROPIC_API_KEY so it does not outrank keyless auth in the SDK.
- claude_api_client.py: fall through to Anthropic() (SDK auto-detect)
  when no key is provided, instead of raising.
- github_action_audit.py: accept a static credential OR WIF env in
  validate_claude_available().

Providing claude-api-key behaves exactly as before; this only adds a
fallback when it is absent.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
waleedzafar68 added a commit to Alliance-Innovations/claude-code-security-review that referenced this pull request Sep 18, 2026
## Why
Upstream anthropics#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) <noreply@anthropic.com>
waleedzafar68 added a commit to Alliance-Innovations/claude-code-security-review that referenced this pull request Sep 18, 2026
## 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 anthropics#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) <noreply@anthropic.com>
waleedzafar68 added a commit to Alliance-Innovations/claude-code-security-review that referenced this pull request Sep 18, 2026
…s (NEX-2513) (#4)

* feat(auth): make claude-api-key optional so the action can run keyless

## 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#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) <noreply@anthropic.com>

* fix(auth): keep false-positive filtering on under keyless auth

## Why
Upstream anthropics#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) <noreply@anthropic.com>

* fix(scan): reach the drop-the-diff fallback when the prompt is too long

## 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 anthropics#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) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant