NAS-142189 / 27.0.0-BETA.1 / Accept a Claude Code OAuth token as the auth secret - #9
Conversation
claude-code-action v1.0.187 takes claude_code_oauth_token as a first-class alternative to anthropic_api_key, so subscription-billed repos need no API key — and no self-hosted runner or custom action. Either secret satisfies the workflow; a job with neither fails before the checkout instead of deep inside the review step.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| # Fail before spending a runner minute on a checkout: with neither | ||
| # secret the review step errors anyway, only later and less clearly. | ||
| - name: Check an auth secret is present |
There was a problem hiding this comment.
MEDIUM — This step moves the "no auth" failure earlier in the job, which makes a comment in claude-review-self.yml untrue. claude-review-self.yml:40-41 says of a fork PR:
pull_requestgives a fork run no secrets, soanthropic-api-keyarrives empty and the review step fails — it cannot spend tokens.
With this step in place the review step never runs on that path: Check an auth secret is present exits 1 before the checkout, so a writer opening a PR from a fork now fails here, not at Automatic PR Review. The conclusion the comment draws (degrades, spends no tokens) still holds; the mechanism it names does not, and that file is otherwise precise about which step does what ("the step dies on MODULE_NOT_FOUND", "the review job's own actions/checkout"), so the specificity reads as load-bearing.
Fix in claude-review-self.yml:41 — e.g. "…arrives empty and the job fails its auth-secret check before the review runs".
.github/workflows/claude-review-self.yml:41
| uses: anthropics/claude-code-action@v1.0.187 | ||
| with: | ||
| # Both pass through as env vars; the action treats an empty one as | ||
| # absent and requires at least one. Callers should set exactly one. |
There was a problem hiding this comment.
LOW — "Callers should set exactly one" is advice with nothing behind it, and both usage snippets invite the opposite. The check above enforces at least one; the header block (claude-review.yml:42-44) and README.md:181-183 show the two lines adjacently with only a # comment between them, so a caller who copies the block verbatim and has both org secrets defined passes both. Nothing then says which credential wins, and the two bill different accounts — a repo that meant to stay on API billing could silently start drawing down a subscription (or the reverse) with no signal in the log.
Cheap to close either way: extend the check to warn (or fail) when both are non-empty, or state in the comment which one the action prefers so the outcome is at least predictable.
|
Two findings, both LOW. Nothing here blocks the gate. The shape of the change is right for this repo. I also checked the paths where a guard like this usually leaks. An empty-but-mapped secret is treated as absent, so no caller is newly rejected. A non-empty but expired OAuth token passes the guard and dies in the review step, where Both findings are on the inline threads:
One thing I could not verify offline: that The |
Review findings: the fork-PR comment in claude-review-self.yml named the review step as where an empty secret dies, but the new guard fails first; and "set exactly one" was advice with nothing behind it while both usage snippets invite copying both lines. The guard now fails on both-set too — the two secrets bill different accounts, so which wins must not be an undocumented pick.
| # or, on subscription auth instead of API billing: | ||
| claude-code-oauth-token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} |
There was a problem hiding this comment.
MEDIUM — This example maps both secrets, which is the one configuration the new guard rejects. # or, ... reads as an alternative to a human, but both lines are live YAML: a caller copying this block gets both mappings.
That is harmless only while CLAUDE_CODE_OAUTH_TOKEN does not exist in the repo (it resolves empty). The interesting case is the migration this PR exists for — a repo adding CLAUDE_CODE_OAUTH_TOKEN without deleting CLAUDE_API_KEY, which is the normal way to switch billing. With both secrets defined and this block copied verbatim, Check exactly one auth secret is present exits 1 and every review job fails.
The prose at README.md:204 already says both fails before the checkout, so the example contradicts the paragraph two lines under it. Note also that this repo's own caller (claude-review-self.yml:60) maps one, not both.
| # or, on subscription auth instead of API billing: | |
| claude-code-oauth-token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} | |
| # or, on subscription auth instead of API billing — set exactly one of | |
| # the two; mapping both fails the job before the checkout: | |
| # claude-code-oauth-token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} |
| # # or, on subscription auth instead of API billing: | ||
| # claude-code-oauth-token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} |
There was a problem hiding this comment.
MEDIUM — Same contradiction as the README example: the usage block shows both secrets mapped, and the guard added at line 195 fails the job when both are set. This is the copy-paste source for a new consumer, so it hands them the rejected configuration — a repo that adds CLAUDE_CODE_OAUTH_TOKEN while CLAUDE_API_KEY is still defined (the ordinary way to switch billing) gets a red review job on every PR with no change of its own to explain it.
| # # or, on subscription auth instead of API billing: | |
| # claude-code-oauth-token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} | |
| # # or, on subscription auth instead of API billing — set exactly one | |
| # # of the two; mapping both fails the job: | |
| # # claude-code-oauth-token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} |
| Anthropic API key. Provide this or claude-code-oauth-token; a step | ||
| fails the job when both are missing. Mapped by the caller, since the | ||
| secret name differs per repo. |
There was a problem hiding this comment.
LOW — The description documents one half of the guard. It says the job fails "when both are missing" and stops there, so a caller reading the reusable workflow's own interface — the only place they see these secrets described in their editor — learns nothing about the both-set failure. claude-code-oauth-token's description below mentions no guard at all. The README carries the full rule; these two do not.
| Anthropic API key. Provide this or claude-code-oauth-token; a step | |
| fails the job when both are missing. Mapped by the caller, since the | |
| secret name differs per repo. | |
| Anthropic API key. Set exactly one of this and | |
| claude-code-oauth-token; a step fails the job when both are missing, | |
| and also when both are set. Mapped by the caller, since the secret | |
| name differs per repo. |
| echo "::error::pass only one of anthropic-api-key and claude-code-oauth-token; both are set," | ||
| echo "::error::and they bill different accounts — which one wins would be undocumented." |
There was a problem hiding this comment.
LOW — Two ::error:: lines for one sentence produce two separate annotations, the second beginning mid-sentence with "and they bill different accounts". Every other multi-line message in this file prefixes only the first line and leaves the continuation as a plain echo (lines 172-175, 184-188, and the neither branch just above), which keeps one annotation and puts the rest in the log.
| echo "::error::pass only one of anthropic-api-key and claude-code-oauth-token; both are set," | |
| echo "::error::and they bill different accounts — which one wins would be undocumented." | |
| echo "::error::pass only one of anthropic-api-key and claude-code-oauth-token; both are set." | |
| echo "They bill different accounts, so which one wins would be undocumented." |
Review findings: both usage snippets mapped both secrets — live YAML, not an either/or — which is exactly what the guard now fails on, so a migrating caller copying the block would red every review. Comment out the alternative line. Also document the both-set failure in both secret descriptions, and emit the both-set error as one annotation.
| # secrets: | ||
| # anthropic-api-key: ${{ secrets.CLAUDE_API_KEY }} | ||
| # # or, on subscription auth instead of API billing — set exactly one | ||
| # # of the two; mapping both fails the job: |
There was a problem hiding this comment.
LOW — "mapping both fails the job" describes a mapping-based rule; the guard is value-based. It compares $API_KEY and $OAUTH_TOKEN, and GitHub gives a mapped-but-nonexistent secret the same empty string as an unmapped one, so mapping both lines passes whenever only one of the two repo secrets actually exists. The secret descriptions below get this right ("when both are set"); these two snippets — here and README.md:183 — do not.
The gap it hides is the one this repo's own guidelines put first. A caller mapping claude-code-oauth-token: ${{ secrets.CLAUDE_CODE_OATH_TOKEN }} — one transposed letter — resolves empty, the guard sees a single credential and passes, and the review runs on API billing while the caller believes it is on their subscription. Nothing says so. That is not fixable from here (Actions cannot distinguish "unmapped" from "mapped to nothing"), so the useful move is wording that does not promise a check the guard cannot make: "setting both fails the job" rather than "mapping both".
Not a repeat of the earlier thread on this line — that one was about the snippet shipping both mappings as live YAML, which this push fixed.
| echo "::error::pass only one of anthropic-api-key and claude-code-oauth-token; both are set." | ||
| echo "They bill different accounts, so which one wins would be undocumented." | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
LOW — This guard fails before the checkout, and the two if: always() steps at the end of the job then run against a workspace that has no tooling in it. Report denied tool requests (line 408) and Enforce review threshold (line 428) both invoke node .claude-review/tooling/review/*.mjs, and neither file exists yet, so each dies with Cannot find module. A caller who mapped neither secret, or both, sees three red steps and two module-resolution errors after the annotation that actually explains the problem.
The job still fails closed, so this is legibility only. It is also not new — Assert the triggering event and Check the access gate ran have the same shape above. What this change does is move it onto the common path: a trigger misconfiguration or a check-member infrastructure failure is rare, while "I mapped the wrong number of auth secrets" is the first thing every repo adopting the OAuth token gets wrong, and the step's own comment gives clearer failure as its reason for existing. steps.review.outcome != '', or a tooling-ready marker output, would keep the two tail steps out of a pre-checkout exit.
bugclerk
changed the title
|
This PR has been merged and conversations have been locked. |
2 participants
Summary
anthropics/claude-code-action@v1.0.187acceptsclaude_code_oauth_token(the tokenclaude setup-tokengenerates from a subscription login) as a first-class alternative toanthropic_api_key— verified in the pinned version'saction.ymlandbase-action/src/validate-env.ts, which requires at least one of the two and exports both to the CLI as env vars.claude-code-oauth-tokensecret, passed through to the action;anthropic-api-keybecomes optional.No caller changes required — existing
anthropic-api-keycallers are untouched. A subscription-billed repo switches by mappingclaude-code-oauth-tokeninstead.