ci: post Claude reviews as claude[bot] again - #229
Merged
Conversation
Switching the review workflow to `claude-code-base-action` (#216) was necessary for `workflow_run` — the wrapper rejects it as an automation event with no PR entity — but it silently cost the review its GitHub identity. Comments now come from `github-actions[bot]` instead of `claude[bot]`. The identity was never an input we could set. `claude-code-action` trades the job's OIDC token for a Claude GitHub App installation token in `src/github/token.ts`, before it runs anything; the base action has no such code, so nothing in this workflow could re-enable it. Confirmed by comparing comment authors: PR #192 has a `claude[bot]` comment, the #216-era runs do not. Re-implement the exchange as a step, mirroring the wrapper: mint an OIDC token with audience `claude-code-github-action` (Anthropic validates the `aud` claim, so the default audience is rejected), POST it to `api.anthropic.com/api/github/github-app-token-exchange`, and use the returned installation token for every comment write. Revoke it in a final step rather than letting it idle out its remaining ~40 minutes. Identity is kept strictly cosmetic. `continue-on-error` plus a `|| github.token` fallback on each consumer means an uninstalled app, a changed endpoint, or a network blip downgrades authorship instead of losing a review. `gh run cancel` on the quota path keeps using the workflow token: the app token's grants are contents/pull-requests/issues, with no `actions: write`. Three details found by testing the step against a local stub rather than by reading the upstream source: * `ACTIONS_ID_TOKEN_REQUEST_{URL,TOKEN}` must be read as shell variables. The `env` expression context only exposes workflow/job/step-declared variables, so mapping these runner-injected ones in yields empty strings and the exchange is never attempted — it would have failed open, always, and looked like the app was uninstalled. * `-w '%{http_code}'` is unusable as a success signal here: on a connection-teardown retry curl emits the template once per attempt, so the capture reads "000200" and matches no status test. A recovered transient fault would have cost the identity. Judge by curl's exit code with `--fail-with-body`, which still saves the error JSON. * Bodies go to a file, never piped to jq. Under `-sS --retry` curl writes "Transient problem … will retry" to stderr, which a `2>&1` capture splices into the JSON. Verified with a stub covering the happy path through a forced retry, a permanent 401, an unreachable endpoint, and a missing OIDC context: all four fall back cleanly and exit 0. actionlint clean. Note this cannot take effect until it lands on the default branch — the exchange refuses workflows absent from it, and `workflow_run` reads the default-branch copy regardless.
📝 WalkthroughWalkthroughThe workflow now mints a Claude GitHub App token through OIDC, uses it for review and comment operations, retains the workflow token for run cancellation, and revokes the App token in a terminal cleanup step. ChangesClaude App token workflow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant OIDCProvider
participant GitHubAppTokenAPI
participant GitHubCommentsAPI
participant GitHubActionsAPI
GitHubActions->>OIDCProvider: Request OIDC identity token
OIDCProvider-->>GitHubActions: Return OIDC token
GitHubActions->>GitHubAppTokenAPI: Exchange OIDC token
GitHubAppTokenAPI-->>GitHubActions: Return installation token
GitHubActions->>GitHubCommentsAPI: Publish review and edit comments with App token
GitHubActions->>GitHubActionsAPI: Cancel run with workflow token
GitHubActions->>GitHubAppTokenAPI: Revoke installation token
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📊 Source Code Metrics (this PR vs
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Claude Code review skipped — usage limit reached.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 152f809f68
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/claude-code-review.yml (1)
448-465: 🔒 Security & Privacy | 🔵 TrivialResidual OIDC-credential exposure to the agentic Bash tool (pre-existing, not introduced here).
Now that the exchange mechanics are explicit in this file, worth flagging for awareness:
id-token: writeis granted for the whole job, soACTIONS_ID_TOKEN_REQUEST_URL/ACTIONS_ID_TOKEN_REQUEST_TOKENremain readable by every step's shell — including this one, which runs with--dangerously-skip-permissionsand unrestricted Bash over the checked-out code. This is the same class of exposure documented publicly forclaude-code-action-style workflows: The ACTIONS_ID_TOKEN_REQUEST_TOKEN and ACTIONS_ID_TOKEN_REQUEST_URL environment variables are the credentials required to request that OIDC token, so a party who obtains them can independently exchange for an installation token, bypassing this workflow's later revoke step entirely.Mitigating factors already in place here: the job only runs for same-repo, non-bot actors (i.e., PR authors with write access already), narrowing this from the fully-external triage-workflow chain in the cited research to a prompt-injection-from-first-party-code scenario. Worth confirming with the team that this residual risk is accepted, or considering isolating the mint/use/revoke sequence in a job with narrower permissions than the review step itself.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/claude-code-review.yml around lines 448 - 465, Restrict the OIDC permission scope for the review step by isolating the app-token mint/use/revoke sequence from the Claude action step that runs with --dangerously-skip-permissions, or otherwise ensure ACTIONS_ID_TOKEN_REQUEST_URL and ACTIONS_ID_TOKEN_REQUEST_TOKEN are unavailable to the agentic Bash environment. Preserve the existing GH_TOKEN fallback behavior while preventing the review action from independently minting credentials.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/claude-code-review.yml:
- Around line 725-739: Update the curl invocation in the “Revoke the Claude
GitHub App token” step to include --fail-with-body, matching the failure
handling used by the Mint step. Preserve the existing warning fallback so HTTP
errors trigger it while successful revocations remain unchanged.
- Around line 204-300: Mask the intermediate OIDC bearer credential immediately
after extracting it into `oidc` and before using it in the exchange request.
Update the `oidc` handling in the “Mint a Claude GitHub App token” step, while
preserving the existing empty-token fallback and later `Authorization` usage.
---
Outside diff comments:
In @.github/workflows/claude-code-review.yml:
- Around line 448-465: Restrict the OIDC permission scope for the review step by
isolating the app-token mint/use/revoke sequence from the Claude action step
that runs with --dangerously-skip-permissions, or otherwise ensure
ACTIONS_ID_TOKEN_REQUEST_URL and ACTIONS_ID_TOKEN_REQUEST_TOKEN are unavailable
to the agentic Bash environment. Preserve the existing GH_TOKEN fallback
behavior while preventing the review action from independently minting
credentials.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a41af579-a9fb-4543-9bb9-c578e3988b0a
📒 Files selected for processing (1)
.github/workflows/claude-code-review.yml
Comment on lines
+204
to
+300
| - name: Mint a Claude GitHub App token | ||
| id: app-token | ||
| if: steps.pr.outputs.found == 'true' | ||
| continue-on-error: true | ||
| run: | | ||
| set -euo pipefail | ||
|
|
||
| # `ACTIONS_ID_TOKEN_REQUEST_{URL,TOKEN}` are injected into the step's | ||
| # process environment by the runner when `id-token: write` is granted. | ||
| # They are read here as plain shell variables, deliberately NOT mapped | ||
| # in through the `env` expression context: that context exposes only | ||
| # variables declared in the workflow, job, or step, so mapping these | ||
| # would expand to the empty string and the exchange would never even be | ||
| # attempted. `@actions/core`'s `getIDToken` reads them from | ||
| # `process.env` for the same reason. | ||
| if [[ -z "${ACTIONS_ID_TOKEN_REQUEST_URL:-}" || -z "${ACTIONS_ID_TOKEN_REQUEST_TOKEN:-}" ]]; then | ||
| echo "::warning::No OIDC request context; falling back to the workflow token." | ||
| exit 0 | ||
| fi | ||
|
|
||
| # Both calls write their body to a file and are judged by curl's exit | ||
| # code. Two things this avoids, both verified against curl 8.7: | ||
| # * Never pipe curl's stdout into jq. Under `-sS --retry` curl writes | ||
| # "Warning: Transient problem … will retry" to *stderr*, so a | ||
| # `2>&1` capture splices that prose into the JSON and breaks the | ||
| # parse of a call that actually succeeded on its second attempt. | ||
| # * Never use `-w '%{http_code}'` as the success signal. On a | ||
| # connection-teardown retry curl emits the write-out template once | ||
| # *per attempt*, so the capture reads "000200" rather than "200" | ||
| # and no plain status comparison matches — a single transient blip | ||
| # would silently cost us the identity. | ||
| # `--fail-with-body` makes a 4xx/5xx a nonzero exit *and* still saves | ||
| # the error JSON, so the diagnostics below survive. | ||
| # | ||
| # `--retry-connrefused` rather than `--retry-all-errors`: retries cover | ||
| # transient transport faults (curl already retries 5xx/429 by default), | ||
| # while an auth or workflow-validation 401 is permanent — retrying it | ||
| # four times only delays the fallback and fills the log with the same | ||
| # error. Retries here are also why the body must be re-read after each | ||
| # call rather than cached. | ||
| body="$(mktemp)" | ||
|
|
||
| # The audience must be `claude-code-github-action`, not the default | ||
| # `https://github.com/<owner>` — Anthropic validates the `aud` claim. | ||
| # The runner's URL already carries a query string (`?api-version=2.0`), | ||
| # hence `&` rather than `?`. | ||
| if ! curl --fail-with-body -sS --retry 3 --retry-connrefused \ | ||
| --connect-timeout 5 --max-time 30 \ | ||
| -H "Authorization: Bearer $ACTIONS_ID_TOKEN_REQUEST_TOKEN" \ | ||
| -o "$body" \ | ||
| "${ACTIONS_ID_TOKEN_REQUEST_URL}&audience=claude-code-github-action"; then | ||
| echo "::warning::Could not mint an OIDC token; falling back to the workflow token." | ||
| rm -f "$body" | ||
| exit 0 | ||
| fi | ||
|
|
||
| oidc="$(jq -r '.value // empty' <"$body" 2>/dev/null || true)" | ||
|
|
||
| if [[ -z "$oidc" ]]; then | ||
| echo "::warning::OIDC response carried no token; falling back to the workflow token." | ||
| rm -f "$body" | ||
| exit 0 | ||
| fi | ||
|
|
||
| if ! curl --fail-with-body -sS --retry 3 --retry-connrefused \ | ||
| --connect-timeout 5 --max-time 30 \ | ||
| -X POST \ | ||
| -H "Authorization: Bearer $oidc" \ | ||
| -o "$body" \ | ||
| https://api.anthropic.com/api/github/github-app-token-exchange; then | ||
| # The failure worth naming: the exchange only issues a token for a | ||
| # workflow that exists on the default branch, so a PR that adds or | ||
| # edits this file cannot mint one until it merges (reported as | ||
| # `workflow_not_found_on_default_branch`, or a 401 "workflow | ||
| # validation failed"). | ||
| echo "::warning::App token exchange failed; falling back to the workflow token. $(jq -r '.error.message // .message // "no message"' <"$body" 2>/dev/null || true)" | ||
| rm -f "$body" | ||
| exit 0 | ||
| fi | ||
|
|
||
| # Upstream accepts either key, so mirror that rather than guessing. | ||
| token="$(jq -r '.token // .app_token // empty' <"$body" 2>/dev/null || true)" | ||
|
|
||
| if [[ -z "$token" ]]; then | ||
| echo "::warning::Exchange returned no token; falling back to the workflow token." | ||
| rm -f "$body" | ||
| exit 0 | ||
| fi | ||
|
|
||
| # Mask before anything else can echo it, then hand it to later steps as | ||
| # an output. Masking is what makes the step output safe to use: it is | ||
| # applied to the whole job's log stream, not just this step. | ||
| echo "::add-mask::$token" | ||
| echo "token=$token" >>"$GITHUB_OUTPUT" | ||
| rm -f "$body" | ||
| echo "Minted a Claude GitHub App token; comments will post as claude[bot]." | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
Mask the intermediate OIDC token too.
The audience (claude-code-github-action) and exchange endpoint match upstream claude-code-action's implementation: OIDC Retrieval: The function getOidcToken calls core.getIDToken with the audience claude-code-github-action and Token Exchange: The OIDC token and requested permissions are sent to https://api.anthropic.com/api/github/github-app-token-exchange via exchangeForAppToken. Retry/parsing logic looks correct.
One gap: $token gets ::add-mask:: (line 296) but the intermediate $oidc bearer credential (line 260) never does, despite being used as an Authorization: Bearer header in a subsequent call. If any future diagnostic line echoes it, it leaks unredacted into the job log.
🔒 Mask the OIDC token right after extraction
oidc="$(jq -r '.value // empty' <"$body" 2>/dev/null || true)"
if [[ -z "$oidc" ]]; then
echo "::warning::OIDC response carried no token; falling back to the workflow token."
rm -f "$body"
exit 0
fi
+
+ echo "::add-mask::$oidc"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: Mint a Claude GitHub App token | |
| id: app-token | |
| if: steps.pr.outputs.found == 'true' | |
| continue-on-error: true | |
| run: | | |
| set -euo pipefail | |
| # `ACTIONS_ID_TOKEN_REQUEST_{URL,TOKEN}` are injected into the step's | |
| # process environment by the runner when `id-token: write` is granted. | |
| # They are read here as plain shell variables, deliberately NOT mapped | |
| # in through the `env` expression context: that context exposes only | |
| # variables declared in the workflow, job, or step, so mapping these | |
| # would expand to the empty string and the exchange would never even be | |
| # attempted. `@actions/core`'s `getIDToken` reads them from | |
| # `process.env` for the same reason. | |
| if [[ -z "${ACTIONS_ID_TOKEN_REQUEST_URL:-}" || -z "${ACTIONS_ID_TOKEN_REQUEST_TOKEN:-}" ]]; then | |
| echo "::warning::No OIDC request context; falling back to the workflow token." | |
| exit 0 | |
| fi | |
| # Both calls write their body to a file and are judged by curl's exit | |
| # code. Two things this avoids, both verified against curl 8.7: | |
| # * Never pipe curl's stdout into jq. Under `-sS --retry` curl writes | |
| # "Warning: Transient problem … will retry" to *stderr*, so a | |
| # `2>&1` capture splices that prose into the JSON and breaks the | |
| # parse of a call that actually succeeded on its second attempt. | |
| # * Never use `-w '%{http_code}'` as the success signal. On a | |
| # connection-teardown retry curl emits the write-out template once | |
| # *per attempt*, so the capture reads "000200" rather than "200" | |
| # and no plain status comparison matches — a single transient blip | |
| # would silently cost us the identity. | |
| # `--fail-with-body` makes a 4xx/5xx a nonzero exit *and* still saves | |
| # the error JSON, so the diagnostics below survive. | |
| # | |
| # `--retry-connrefused` rather than `--retry-all-errors`: retries cover | |
| # transient transport faults (curl already retries 5xx/429 by default), | |
| # while an auth or workflow-validation 401 is permanent — retrying it | |
| # four times only delays the fallback and fills the log with the same | |
| # error. Retries here are also why the body must be re-read after each | |
| # call rather than cached. | |
| body="$(mktemp)" | |
| # The audience must be `claude-code-github-action`, not the default | |
| # `https://github.com/<owner>` — Anthropic validates the `aud` claim. | |
| # The runner's URL already carries a query string (`?api-version=2.0`), | |
| # hence `&` rather than `?`. | |
| if ! curl --fail-with-body -sS --retry 3 --retry-connrefused \ | |
| --connect-timeout 5 --max-time 30 \ | |
| -H "Authorization: Bearer $ACTIONS_ID_TOKEN_REQUEST_TOKEN" \ | |
| -o "$body" \ | |
| "${ACTIONS_ID_TOKEN_REQUEST_URL}&audience=claude-code-github-action"; then | |
| echo "::warning::Could not mint an OIDC token; falling back to the workflow token." | |
| rm -f "$body" | |
| exit 0 | |
| fi | |
| oidc="$(jq -r '.value // empty' <"$body" 2>/dev/null || true)" | |
| if [[ -z "$oidc" ]]; then | |
| echo "::warning::OIDC response carried no token; falling back to the workflow token." | |
| rm -f "$body" | |
| exit 0 | |
| fi | |
| if ! curl --fail-with-body -sS --retry 3 --retry-connrefused \ | |
| --connect-timeout 5 --max-time 30 \ | |
| -X POST \ | |
| -H "Authorization: Bearer $oidc" \ | |
| -o "$body" \ | |
| https://api.anthropic.com/api/github/github-app-token-exchange; then | |
| # The failure worth naming: the exchange only issues a token for a | |
| # workflow that exists on the default branch, so a PR that adds or | |
| # edits this file cannot mint one until it merges (reported as | |
| # `workflow_not_found_on_default_branch`, or a 401 "workflow | |
| # validation failed"). | |
| echo "::warning::App token exchange failed; falling back to the workflow token. $(jq -r '.error.message // .message // "no message"' <"$body" 2>/dev/null || true)" | |
| rm -f "$body" | |
| exit 0 | |
| fi | |
| # Upstream accepts either key, so mirror that rather than guessing. | |
| token="$(jq -r '.token // .app_token // empty' <"$body" 2>/dev/null || true)" | |
| if [[ -z "$token" ]]; then | |
| echo "::warning::Exchange returned no token; falling back to the workflow token." | |
| rm -f "$body" | |
| exit 0 | |
| fi | |
| # Mask before anything else can echo it, then hand it to later steps as | |
| # an output. Masking is what makes the step output safe to use: it is | |
| # applied to the whole job's log stream, not just this step. | |
| echo "::add-mask::$token" | |
| echo "token=$token" >>"$GITHUB_OUTPUT" | |
| rm -f "$body" | |
| echo "Minted a Claude GitHub App token; comments will post as claude[bot]." | |
| oidc="$(jq -r '.value // empty' <"$body" 2>/dev/null || true)" | |
| if [[ -z "$oidc" ]]; then | |
| echo "::warning::OIDC response carried no token; falling back to the workflow token." | |
| rm -f "$body" | |
| exit 0 | |
| fi | |
| echo "::add-mask::$oidc" | |
| if ! curl --fail-with-body -sS --retry 3 --retry-connrefused \ | |
| --connect-timeout 5 --max-time 30 \ | |
| -X POST \ | |
| -H "Authorization: Bearer $oidc" \ | |
| -o "$body" \ | |
| https://api.anthropic.com/api/github/github-app-token-exchange; then |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/claude-code-review.yml around lines 204 - 300, Mask the
intermediate OIDC bearer credential immediately after extracting it into `oidc`
and before using it in the exchange request. Update the `oidc` handling in the
“Mint a Claude GitHub App token” step, while preserving the existing empty-token
fallback and later `Authorization` usage.
Comment on lines
+725
to
+739
| - name: Revoke the Claude GitHub App token | ||
| if: always() && steps.app-token.outputs.token != '' | ||
| continue-on-error: true | ||
| env: | ||
| APP_TOKEN: ${{ steps.app-token.outputs.token }} | ||
| run: | | ||
| set -euo pipefail | ||
|
|
||
| curl -sS -X DELETE \ | ||
| --connect-timeout 5 --max-time 10 \ | ||
| -H "Accept: application/vnd.github+json" \ | ||
| -H "Authorization: Bearer $APP_TOKEN" \ | ||
| -H "X-GitHub-Api-Version: 2022-11-28" \ | ||
| "${GITHUB_API_URL:-https://api.github.com}/installation/token" >/dev/null || \ | ||
| echo "::warning::Could not revoke the app token; it expires on its own within the hour." |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Revoke call never detects HTTP-level failures.
Confirmed the endpoint/auth pattern itself is correct — GitHub's own example for this endpoint uses the same DELETE .../installation/token with a Bearer token. However, the curl invocation here has no -f/--fail-with-body, unlike the two calls in the "Mint" step above which deliberately add --fail-with-body for the same reason. Without it, curl exits 0 on any HTTP response code, so a 401/403/404 revoke failure is silently swallowed — the ::warning:: never fires and the run reports "revoked" even when it wasn't.
🐛 Add `--fail-with-body` so failures are actually detected
- curl -sS -X DELETE \
+ curl --fail-with-body -sS -X DELETE \
--connect-timeout 5 --max-time 10 \
-H "Accept: application/vnd.github+json" \
-H "Authorization: Bearer $APP_TOKEN" \
-H "X-GitHub-Api-Version: 2022-11-28" \
"${GITHUB_API_URL:-https://api.github.com}/installation/token" >/dev/null || \
echo "::warning::Could not revoke the app token; it expires on its own within the hour."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: Revoke the Claude GitHub App token | |
| if: always() && steps.app-token.outputs.token != '' | |
| continue-on-error: true | |
| env: | |
| APP_TOKEN: ${{ steps.app-token.outputs.token }} | |
| run: | | |
| set -euo pipefail | |
| curl -sS -X DELETE \ | |
| --connect-timeout 5 --max-time 10 \ | |
| -H "Accept: application/vnd.github+json" \ | |
| -H "Authorization: Bearer $APP_TOKEN" \ | |
| -H "X-GitHub-Api-Version: 2022-11-28" \ | |
| "${GITHUB_API_URL:-https://api.github.com}/installation/token" >/dev/null || \ | |
| echo "::warning::Could not revoke the app token; it expires on its own within the hour." | |
| - name: Revoke the Claude GitHub App token | |
| if: always() && steps.app-token.outputs.token != '' | |
| continue-on-error: true | |
| env: | |
| APP_TOKEN: ${{ steps.app-token.outputs.token }} | |
| run: | | |
| set -euo pipefail | |
| curl --fail-with-body -sS -X DELETE \ | |
| --connect-timeout 5 --max-time 10 \ | |
| -H "Accept: application/vnd.github+json" \ | |
| -H "Authorization: Bearer $APP_TOKEN" \ | |
| -H "X-GitHub-Api-Version: 2022-11-28" \ | |
| "${GITHUB_API_URL:-https://api.github.com}/installation/token" >/dev/null || \ | |
| echo "::warning::Could not revoke the app token; it expires on its own within the hour." |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/claude-code-review.yml around lines 725 - 739, Update the
curl invocation in the “Revoke the Claude GitHub App token” step to include
--fail-with-body, matching the failure handling used by the Mint step. Preserve
the existing warning fallback so HTTP errors trigger it while successful
revocations remain unchanged.
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
1 participant
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.
Restores the
claude[bot]authorship that PR #216 gave up when the review workflow moved fromclaude-code-actiontoclaude-code-base-action.Why it broke
The move to the base action was correct — the wrapper classifies
workflow_runas an automation event with no PR entity and refuses to run. But the wrapper also does something the base action doesn't: before running anything, it trades the job's OIDC token for a Claude GitHub App installation token (src/github/token.ts). That's what made every comment show up asclaude[bot].There's no input for it. The exchange lives in the wrapper's TypeScript, so no amount of configuring the base action brings it back — it had to be re-implemented.
Confirmed rather than assumed:
claude[bot](uid 209825114)github-actions[bot]What this does
A
Mint a Claude GitHub App tokenstep mirroring the wrapper's two hops:claude-code-github-action— not the defaulthttps://github.com/<owner>. Anthropic validates theaudclaim.POSTit toapi.anthropic.com/api/github/github-app-token-exchange; the reply is a GitHub App installation token.Every comment-writing step then uses
${{ steps.app-token.outputs.token || github.token }}, and a final step revokes the token instead of leaving it to idle out its remaining ~40 minutes.Identity is cosmetic, by construction
continue-on-error: trueon the mint step plus the|| github.tokenfallback on each consumer: an uninstalled app, a changed endpoint, or a transient network fault downgrades authorship togithub-actions[bot]and the review still publishes. Losing the avatar must never cost a review.One deliberate exception: the quota path's
gh run cancelkeeps the workflow token explicitly. The app token's grants are contents/pull-requests/issues — noactions: write— so a blanket swap would have broken self-cancellation on a 429.Three bugs the empirical test caught
I tested the step against a local stub server rather than trusting my reading of the upstream source. Each of these was in my own first draft and would have shipped silently:
ACTIONS_ID_TOKEN_REQUEST_{URL,TOKEN}can't be read via${{ env.… }}. That context exposes only workflow/job/step-declared variables, not runner-injected ones. Mapping them in yields empty strings, so the step would have failed open always — indistinguishable from an uninstalled app.@actions/core'sgetIDTokenreads them fromprocess.envfor the same reason.-w '%{http_code}'is unusable as the success signal. On a connection-teardown retry, curl emits the write-out template once per attempt, so the capture reads000200, not200, and matches no status comparison. A single recovered blip would have cost the identity. Now judged by curl's exit code with--fail-with-body, which still saves the error JSON for diagnostics.-sS --retry, curl writesWarning: Transient problem … will retryto stderr; a2>&1capture splices that prose into the JSON and breaks the parse of a call that actually succeeded.Also switched
--retry-all-errors→--retry-connrefused: a workflow-validation 401 is permanent, and retrying it 4× only delays the fallback and repeats the same log line.Verification
token=ghs_…set, exit 0Invalid OIDC token), fallback, exit 0id-tokenabsent)Tests ran the step's
run:block extracted verbatim from the workflow with only the endpoint URL redirected, so the logic under test is the logic that ships.actionlintclean on this file and all 12 workflows.Reviewer notes
workflow_not_found_on_default_branch), andworkflow_runreads the default-branch copy anyway. So the review on this PR will still post asgithub-actions[bot]— that is expected, and is itself the fallback path working. The first PR after merge is the real test.Summary by CodeRabbit