diff --git a/README.md b/README.md index aaf87e8..ae51e68 100644 --- a/README.md +++ b/README.md @@ -171,7 +171,7 @@ Beyond token scopes, each mode passes a `--allowedTools` allowlist and a `--disa **Fork pull requests and the `GITHUB_TOKEN` downgrade.** GitHub automatically restricts the ambient `GITHUB_TOKEN` to read-only for any `pull_request`-triggered run originating from a fork, regardless of what `permissions:` a workflow declares — this is a platform-level protection, not something any input here controls. That's why review mode's own example (`examples/direct/claude-review.yml`) uses the direct form and deliberately leaves `github_token` unset: doing so makes the run authenticate as the Claude Code GitHub App (`claude[bot]`) instead, a separately-minted credential via OIDC that isn't subject to that downgrade. Reviewing a fork pull request through the reusable workflow's default `GITHUB_TOKEN` would appear to run successfully but silently fail to post — worth knowing if a repository that accepts fork pull requests seems to review nothing. Separately, GitHub's own "Require approval for first-time contributors" repository setting (on by default for public repositories) gates _any_ workflow run — including this one — from an unknown contributor's fork pull request until a maintainer approves it once in the Actions UI, on top of everything above. -**A pull request cannot reliably self-test a change to its own OIDC-authenticated workflow file.** Any workflow using the App-identity/OIDC path (leaving `github_token` unset, as `claude-review.yml`, `claude-triage.yml`, and `claude-interactive.yml` all do here) is subject to a GitHub platform check that the calling workflow file be byte-identical to the version on the default branch — the same mechanism the "Automated upstream bumps" section below documents for `dependabot.yml`. A pull request that edits one of these three workflow files fails that check on its own first run against itself: upstream detects this and gracefully no-ops (`Skipping action due to workflow validation`, a real warning, not a crash), so the run still reports success, but no API call happens and nothing gets posted — confirmed live, not theorised, when `headroom_enabled: true` was added to `claude-review.yml` in the same pull request that first tried to dogfood it. Passing `github_token: ${{ github.token }}` explicitly sidesteps this entirely by skipping OIDC, but doing that permanently on one of these three files would give up the App identity's own capabilities (`resolve_stale_threads` and friends) for every future run, not just self-modifying ones — not a trade worth making for a narrow, rare edge case. Validate a change to one of these files with a separate, temporary workflow instead (one that does not itself modify the file under test), then remove it once confirmed; the change becomes fully self-testable again the moment it lands on the default branch. +**A pull request cannot reliably self-test a change to its own OIDC-authenticated workflow file, and this action fails loudly when that happens rather than silently doing nothing.** Any workflow using the App-identity/OIDC path (leaving `github_token` unset, as `claude-review.yml`, `claude-triage.yml`, and `claude-interactive.yml` all do here) is subject to a GitHub platform check that the calling workflow file be byte-identical to the version on the default branch — the same mechanism the "Automated upstream bumps" section below documents for `dependabot.yml`. A pull request that edits one of these three workflow files fails that check on its own first run against itself; the same check also fails, independent of anything the pull request itself touches, when the branch the workflow file lives on has simply fallen behind a since-changed copy of the file on the default branch. Upstream itself treats a failure either way as a soft skip (`Skipping action due to workflow validation`, a warning, not a crash) and exits successfully having done nothing — reasonable for a repository adopting one of these workflows for the very first time, before the file exists on its own default branch yet, but actively misleading for an already-working workflow that has quietly stopped reviewing anything with no visible signal. `Resolve Claude Code result` detects this specific case (a winning attempt with no `conclusion` output at all — the one upstream code path this can come from, since the only other cause of a missing `conclusion`, "no trigger found," is provably unreachable through this wrapper, which always sets a `prompt`) and fails the job outright instead, with an `::error::` naming the two possible causes and pointing back here — confirmed live against `adpeak/adpeak-mono`, where a branch that had fallen behind a same-day rewrite of its own `claude-review.yml` on `main` hit exactly this. Passing `github_token: ${{ github.token }}` explicitly sidesteps this entirely by skipping OIDC, but doing that permanently on one of these three files would give up the App identity's own capabilities (`resolve_stale_threads` and friends) for every future run, not just self-modifying ones — not a trade worth making for a narrow, rare edge case. Validate a change to one of these files with a separate, temporary workflow instead (one that does not itself modify the file under test), then remove it once confirmed; the change becomes fully self-testable again the moment it lands on the default branch. If the loud failure fires and neither cause applies — this pull request does not touch the workflow file, and the file already matches the default branch — merge or rebase the latest default branch into this branch, which resolves it in the branch-staleness case even without a workflow-file conflict of its own. **Untrusted text reaches the model.** Issue bodies, comments, pull request descriptions, and the contents of a pull request's own files are attacker-influenced input on a public repository or one accepting fork pull requests. The shared prompt tells Claude to treat all of it as data rather than instructions and to report injection attempts, but a prompt is mitigation, not a guarantee. The real controls are the per-mode token scopes and tool allowlists above: assume the prompt can be subverted and check that the blast radius is acceptable if it is. diff --git a/action.yml b/action.yml index dd50456..b1fc0f8 100644 --- a/action.yml +++ b/action.yml @@ -1675,6 +1675,13 @@ runs: exit 1 fi + # Upstream's run.ts only ever leaves its own `conclusion` output unset on two code paths: the workflow-validation-mismatch skip (setupGitHubToken() throws WorkflowValidationSkipError, upstream returns cleanly with exit 0 rather than failing the step) and a "no trigger found" skip -- and the second one is provably unreachable through this wrapper, since every mode this action runs always sets a non-empty `prompt` (see "Compose prompt" above), which is exactly what upstream's own trigger check requires. So a winning attempt with no conclusion at all can only mean the workflow-validation skip: github_token was left unset (the direct-form, App-identity path), and GitHub's own OIDC platform check rejected this run's workflow file as not byte-identical to what is currently on the repository's default branch -- either because this run's own workflow file was edited in the same pull request, or because this branch has fallen behind a since-changed copy of the file on the default branch. Upstream treats this as a soft, silent skip (exit 0, nothing posted) on the reasoning that it is expected the first time a repository adopts one of these workflows, before the file exists on its own default branch yet -- but silently doing nothing is exactly as wrong for an already-adopted, previously-working workflow that has quietly stopped reviewing anything, which is the far more common case in practice. Fail loudly here instead in every case, rather than reporting a misleading green success with no comment, no review, and no visible signal anything is wrong. + WINNER_CONCLUSION_VAR="CONCLUSION_${WINNER}" + if [ -z "${!WINNER_CONCLUSION_VAR:-}" ]; then + echo "::error::Run Claude Code (attempt ${WINNER}) reported success but Claude Code itself never ran. github_token is unset for this call, and GitHub's own OIDC platform check rejected this run's workflow file as not byte-identical to what is currently on the repository's default branch -- either this run's own workflow file was edited in the same pull request, or this branch has fallen behind a since-changed copy of the file on the default branch. See https://github.com/ExaDev/claude-code-action#security-notes, 'A pull request cannot reliably self-test a change to its own OIDC-authenticated workflow file', for the full mechanism. If this is genuinely the first run on a repository that just adopted this workflow, merging this pull request resolves it; otherwise, merge or rebase the latest default branch into this branch so the workflow file matches again." >&2 + exit 1 + fi + echo "Credential attempt ${WINNER} succeeded." for field in conclusion execution_file branch_name session_id structured_output winning_cred_type winning_cred_token; do