Skip to content

Commit f644f7e

Browse files
committed
fix(auto-fix): gate to same-repo PRs, avoid stray commits
Address review feedback on the auto-fix flow: - Gate the PR-branch checkout and the auto-commit/push steps to same-repository pull requests. Fork PRs previously hard-failed the checkout (the fork's branch isn't on origin) and could not push; now they are skipped with a warning. - Guard against tag refs: compute the destination branch from the PR head ref or a pushed branch ref, and skip otherwise so we never push HEAD to refs/heads/<tag> and create a stray branch. - Stage only tracked modifications with `git add -u` instead of `git add -A`, so unrelated untracked/generated files are not swept into the auto-fix commit. - docs/permissions.md: clarify the write permission is for the actions/checkout token, and correct the `[skip ci]` guidance (it only matters for PAT/App-token pushes, since the default GITHUB_TOKEN push does not trigger CI anyway). - Example workflow: downgrade `pull-requests: write` to `read` (the example uses no review/thread-comment feature; `read` is still needed for files-changed-only on pull_request events).
1 parent e3fd91b commit f644f7e

3 files changed

Lines changed: 51 additions & 16 deletions

File tree

.github/workflows/examples/auto-fix.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ jobs:
99
runs-on: ubuntu-latest
1010
permissions:
1111
contents: write # needed for auto-fix commits
12-
pull-requests: write
12+
pull-requests: read # needed to list changed files on pull_request events
1313
steps:
1414
- uses: actions/checkout@v7
1515
# For auto-fix commits to trigger new CI runs, use a PAT instead:

action.yml

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,10 @@ runs:
458458
}
459459
460460
- name: Checkout PR branch for auto-fix push capability
461-
if: (inputs.auto-fix == 'true' || inputs.auto-fix == true) && github.event_name == 'pull_request'
461+
if: >-
462+
(inputs.auto-fix == 'true' || inputs.auto-fix == true)
463+
&& github.event_name == 'pull_request'
464+
&& github.event.pull_request.head.repo.full_name == github.repository
462465
shell: nu {0}
463466
run: |
464467
let head_ref = $env.GITHUB_HEAD_REF
@@ -469,6 +472,15 @@ runs:
469472
print $"(ansi green)Switched to PR branch \"($head_ref)\"(ansi reset)"
470473
}
471474
475+
- name: Warn that auto-fix is skipped on forked pull requests
476+
if: >-
477+
(inputs.auto-fix == 'true' || inputs.auto-fix == true)
478+
&& github.event_name == 'pull_request'
479+
&& github.event.pull_request.head.repo.full_name != github.repository
480+
shell: nu {0}
481+
run: |
482+
print "::warning title=Auto-fix skipped::auto-fix cannot push to a third-party fork's branch, so no formatting commit was made. Apply clang-format fixes from within the fork or run cpp-linter locally."
483+
472484
- name: Run cpp-linter
473485
id: cpp-linter
474486
shell: nu {0}
@@ -531,9 +543,28 @@ runs:
531543
^$'($env.UV_INSTALL_DIR)/uv' ...$uv_args cpp-linter ...$args
532544
533545
- name: Auto-commit clang-format fixes
534-
if: inputs.auto-fix == 'true' || inputs.auto-fix == true
546+
if: >-
547+
(inputs.auto-fix == 'true' || inputs.auto-fix == true)
548+
&& (github.event_name != 'pull_request'
549+
|| github.event.pull_request.head.repo.full_name == github.repository)
535550
shell: nu {0}
536551
run: |
552+
# Determine the destination branch up-front: the PR head ref, or the
553+
# pushed branch. Bail out on tag refs (or anything that is not a branch)
554+
# so we never push HEAD to refs/heads/<tag> and create a stray branch.
555+
let head_ref = $env.GITHUB_HEAD_REF
556+
let branch = if ($head_ref | is-not-empty) {
557+
$head_ref
558+
} else if ($env.GITHUB_REF | str starts-with "refs/heads/") {
559+
$env.GITHUB_REF_NAME
560+
} else {
561+
""
562+
}
563+
if ($branch | is-empty) {
564+
print $"::notice title=Auto-fix skipped::($env.GITHUB_REF) is not a branch or pull request ref; skipping auto-fix commit."
565+
exit 0
566+
}
567+
537568
# Refresh index first so stat-only differences don't create false positives
538569
^git update-index -q --refresh
539570
let has_changes = (^git diff-index --name-status --exit-code HEAD -- | complete | $in.exit_code == 1)
@@ -550,19 +581,20 @@ runs:
550581
}
551582
let git_user = $"user.name=($git_user_name)"
552583
let git_email = $"user.email=($git_user_email)"
553-
^git add -A
584+
# Stage only tracked modifications (clang-format edits existing sources).
585+
# Avoid `git add -A`, which would also sweep in unrelated untracked files.
586+
^git add -u
554587
let commit_msg = if ('${{ inputs.auto-fix-commit-msg }}' | is-empty) {
555588
'style: apply styling format fix'
556589
} else {
557590
'${{ inputs.auto-fix-commit-msg }}'
558591
}
559592
^git -c $git_user -c $git_email commit -m $"($commit_msg)"
560-
let branch = $env.GITHUB_HEAD_REF | default $env.GITHUB_REF_NAME
561593
let push_result = (^git push origin $"HEAD:refs/heads/($branch)") | complete
562594
if $push_result.exit_code != 0 {
563595
let stderr_lower = ($push_result.stderr | str downcase)
564596
if ($stderr_lower | str contains "403") or ($stderr_lower | str contains "refused") or ($stderr_lower | str contains "not have permission") {
565-
print $"::warning title=Auto-fix push failed::This action does not have permission to push to this branch. When using auto-fix on pull_request events from a third-party fork, the GITHUB_TOKEN cannot write to the fork repository. See docs/permissions.md for details.(ansi reset)"
597+
print $"::warning title=Auto-fix push failed::This action does not have permission to push to this branch. Ensure the token used by actions/checkout has `contents: write` (branch protection rules may also block the push). See docs/permissions.md for details."
566598
} else {
567599
print $"::warning title=Auto-fix push failed::(ansi yellow)($push_result.stderr)(ansi reset)"
568600
}

docs/permissions.md

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -89,29 +89,32 @@ in addition to any other permissions needed for other features:
8989
contents: write # (1)!
9090
```
9191

92-
1. Needed to commit and push the formatted changes back to the PR branch.
92+
1. Needed by the token used in `actions/checkout` to commit and push the
93+
formatted changes back to the branch.
9394

9495
!!! warning "CI re-triggering with auto-fix"
9596

9697
The default `GITHUB_TOKEN` **cannot** trigger new CI runs when pushing
9798
a commit. If you need the auto-fix commit to trigger CI checks
9899
(e.g. to verify the fix builds clean), use a personal access token
99-
(PAT) with `contents: write` scope:
100+
(PAT) with `contents: write` scope on the checkout step:
100101

101102
```yaml
102103
- uses: actions/checkout@v7
103104
with:
104105
token: ${{ secrets.MY_PAT }}
105106
```
106107

107-
When using the default `GITHUB_TOKEN`, you can include `[skip ci]` in
108-
the auto-fix commit message to avoid unnecessary CI runs on the
109-
fix commit itself. See the [`auto-fix-commit-msg`](./inputs-outputs.md#auto-fix-commit-msg) input.
108+
Conversely, if you use a PAT or GitHub App token (whose pushes **do**
109+
trigger CI) but do not want the auto-fix commit itself to start a new
110+
run, include `[skip ci]` in the commit message via the
111+
[`auto-fix-commit-msg`](./inputs-outputs.md#auto-fix-commit-msg) input.
112+
With the default `GITHUB_TOKEN`, `[skip ci]` is unnecessary since the
113+
push does not trigger CI anyway.
110114

111115
!!! warning "Pull requests from third-party forks"
112116

113-
Auto-fix does not work on pull requests from third-party forks. The
114-
`GITHUB_TOKEN` lacks write permission to the fork repository, and
115-
`git push` to the fork's branch is not possible. Consider
116-
restricting `auto-fix` to `push` events or pull requests from the
117-
same repository.
117+
Auto-fix is automatically skipped for pull requests from third-party
118+
forks: the `GITHUB_TOKEN` cannot push to the fork's branch, so the
119+
action emits a warning and makes no commit. Use `auto-fix` on `push`
120+
events or on pull requests from the same repository.

0 commit comments

Comments
 (0)