ci: migrate claude-code-action workflows to Workload Identity Federation#662
Merged
Conversation
Replaces the static ANTHROPIC_API_KEY repo secret with Workload Identity Federation across the 6 workflows that invoke claude-code-action: the action exchanges this job's GitHub OIDC token for a short-lived access token. The federation rule is bound to this repository (repository_id-pinned). - Adds permissions: id-token: write to all 6 (none had it). - Pins claude-code-action to v1.0.132 — the WIF inputs landed in v1.0.130; @v1 is a moving tag. - No anthropic_workspace_id — the rule targets the org's default workspace, so the input is optional. The two notebook-execution steps (notebook-quality.yml, notebook- tests.yml) that pass ANTHROPIC_API_KEY as an env var to nbconvert/ pytest are NOT migrated here — those need a separate inline OIDC exchange (TODO comments added). Both gracefully degrade when the secret is absent: notebook-tests has an explicit skip-guard, and notebook-quality catches per-notebook failures.
There was a problem hiding this comment.
PR Review
Recommendation: APPROVE
Summary
Migrates 6 workflows from a static ANTHROPIC_API_KEY repo secret to Workload Identity Federation (WIF) for claude-code-action invocations, pinning the action to a SHA at v1.0.132. Long-lived secret usage is preserved (with TODOs) for the two notebook-execution steps that hit the API directly, since those need a different migration path.
Actionable Feedback (0 blocking, 1 optional)
No blocking items. One optional follow-up (already noted in the PR description, not blocking this PR):
-
notebook-quality.yml:102— pre-existing bug:env: anthropic_api_key:(lowercase). On Linux env vars are case-sensitive, so notebooks readingos.environ.get("ANTHROPIC_API_KEY")won't see it. The PR description flags this; consider a follow-up PR to uppercase it (matches the casing already used innotebook-tests.yml:179).
Detailed Review
Security
Strong net improvement:
- Replaces a long-lived shared secret with short-lived OIDC-exchanged tokens for the 6 workflows that invoke
claude-code-action. id-token: writeis correctly added at the right scope in each workflow (job-level where the existingpermissions:block was job-level, workflow-level where it was workflow-level).- The action is SHA-pinned (
bbfaf8e1...) rather than left on the moving@v1tag — this is the right call given that WIF inputs only landed in v1.0.130 and the repo already SHA-pinsastral-sh/setup-uv. - The federation rule, org ID, and service account ID are not secrets but resource identifiers; the PR description correctly notes the federation rule is repository_id-pinned, so leaking the IDs wouldn't let another repo's OIDC token impersonate this service account.
Code Quality
- All 6 workflows are migrated consistently — same SHA pin, same input set, same explanatory comment, same
id-token: writeaddition. Easy to audit. - The deferred-migration TODOs in
notebook-quality.ymlandnotebook-tests.ymlare well-written: they explain what still uses the static key, why it wasn't migrated here (needs inline OIDC exchange, not the action's inputs), and what mitigates the risk in the meantime (graceful degradation via|| echoand[ -z "$ANTHROPIC_API_KEY" ]guard respectively). - Scope discipline is good — the PR resists fixing the incidental lowercase-env bug, keeping this change focused on the WIF migration.
Suggestions
- The 5-line auth comment is duplicated across 6 files. It's fine as-is (each workflow is independent), but if the duplication grows, a composite action or workflow_call template could DRY it up. Not worth doing now.
- Optional: an
ANTHROPIC_API_KEYrepo-secret-deletion checklist for after the two TODOs are resolved would be nice to track somewhere (issue or follow-up PR description).
Positive Notes
- Clear, well-researched PR description: pin reasoning, deferred-migration justification, and the noted incidental bug all reflect careful diligence.
- Correct understanding of the security model (federation rule binding, workspace defaults, OIDC scope).
- Defensive sequencing — migrating only what can be migrated safely in one PR and explicitly documenting the rest.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces the static
ANTHROPIC_API_KEYrepo secret with Workload Identity Federation across the 6 workflows that invokeclaude-code-action: the action exchanges the job's GitHub OIDC token for a short-lived access token instead of reading a long-lived key.Changed (6 workflows):
claude-link-review,claude-model-check,claude-pr-review,lint-format,notebook-quality,notebook-testspermissions: id-token: writeto all 6 (none had it).claude-code-actionto v1.0.132 (bbfaf8e1) — the WIF inputs (anthropic_federation_rule_idetc.) landed in v1.0.130;@v1is a moving tag.anthropic_workspace_id— the rule targets the org's default workspace, so the input is optional (per the action's docs/setup.md).Not migrated here (TODOs added): the notebook-execution steps in
notebook-quality.ymlandnotebook-tests.ymlthat passANTHROPIC_API_KEYas anenv:var tonbconvert/pytest. Those call the API directly (not viaclaude-code-action) and need a separate inline OIDC exchange. Both gracefully degrade when the secret is absent —notebook-testshas an explicit skip-guard;notebook-qualitycatches per-notebook failures with|| echo.Incidental observation:
notebook-quality.yml's execution step setsenv: anthropic_api_key:(lowercase). On Linux that won't be readable asANTHROPIC_API_KEYby notebooks doingos.environ.get("ANTHROPIC_API_KEY")— possible pre-existing bug, left as-is here.Once merged, the
ANTHROPIC_API_KEYrepo secret remains needed only for the two TODO'd execution steps until those are migrated.