Skip to content

Fix lint workflow failing on fork PRs#5094

Merged
kenyonj merged 4 commits intomainfrom
kenyonj/fix-lint-fork-checkout
Mar 30, 2026
Merged

Fix lint workflow failing on fork PRs#5094
kenyonj merged 4 commits intomainfrom
kenyonj/fix-lint-fork-checkout

Conversation

@kenyonj
Copy link
Copy Markdown
Contributor

@kenyonj kenyonj commented Mar 30, 2026

Problem

The lint workflow fails on all fork PRs (e.g. #5075) with:

The process '/usr/bin/git' failed with exit code 1

The workflow uses pull_request_target and checks out github.event.pull_request.head.ref, which is just the branch name (e.g., askpt/add-openfeature). For fork PRs, that branch does not exist in github/explore — it only exists in the contributor's fork — so the fetch fails.

Fix

  1. Checkout by SHA + repository instead of branch ref — head.sha with head.repo.full_name works for both forks and same-repo branches.
  2. Skip auto-push for fork PRs — the workflow can't push to external forks, so the "commit and push" step is gated on head.repo.full_name == github.repository.
  3. For same-repo PRs, explicitly checkout the branch name before pushing so the push targets the correct ref.

The lint workflow uses pull_request_target which can't fetch fork
branches by ref name. Switch to checking out by SHA with the fork's
repository, which works for both forks and same-repo PRs.

Skip the auto-push step for fork PRs since we can't push to
external repos.
@kenyonj kenyonj requested a review from a team as a code owner March 30, 2026 19:38
Copilot AI review requested due to automatic review settings March 30, 2026 19:38
Use env var instead of direct expression interpolation for
head.ref in run blocks to prevent potential code injection
via attacker-controlled branch names.
@kenyonj kenyonj enabled auto-merge March 30, 2026 19:41
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the lint GitHub Actions workflow to run successfully on fork-based pull requests by checking out the PR’s exact commit and avoiding push attempts to forks, while preserving the auto-fix-and-push behavior for same-repo PRs.

Changes:

  • Switch actions/checkout to use pull_request.head.sha and pull_request.head.repo.full_name (with fallbacks) so fork PRs can be fetched.
  • Gate the “commit and push” step to only run for same-repo PRs.
  • Explicitly create/check out the PR branch name before pushing, and push to origin <branch>.
Comments suppressed due to low confidence (2)

.github/workflows/lint.yml:29

  • For fork PRs the workflow now checks out and runs rubocop -A, but the commit/push step is skipped. That means RuboCop can auto-correct locally and still exit successfully, so the check may pass even though the contributor’s branch remains non-compliant. Consider either (a) running RuboCop in check-only mode for forks (no -A) or (b) explicitly failing the job when env.changes == 'true' on forks with a message instructing the contributor to run RuboCop locally.
          ref: ${{ github.event.pull_request.head.sha || github.ref }}
          repository: ${{ github.event.pull_request.head.repo.full_name || github.repository }}

      - name: Setup Ruby
        uses: ruby/setup-ruby@v1.299.0
        with:
          bundler-cache: true

      - name: Run RuboCop with auto-correct
        run: |
          bundle exec rubocop -A

.github/workflows/lint.yml:47

  • The branch name from github.event.pull_request.head.ref is interpolated into shell commands unquoted. While uncommon, branch names can contain characters that break the shell invocation. Quote the ref (and consider using -- where applicable) to avoid unexpected parsing and to ensure the push targets the intended ref.
          HEAD_REF: ${{ github.event.pull_request.head.ref }}
        run: |
          git checkout -b "$HEAD_REF"
          git add .

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 16 to 20
- uses: actions/checkout@v6.0.2
with:
ref: ${{ github.event.pull_request.head.ref || github.ref }}
ref: ${{ github.event.pull_request.head.sha || github.ref }}
repository: ${{ github.event.pull_request.head.repo.full_name || github.repository }}

Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using pull_request_target while checking out github.event.pull_request.head.sha from head.repo.full_name means the job will execute untrusted fork code with the base repo’s GITHUB_TOKEN permissions (contents: write at the workflow level). This is a known security footgun: a malicious fork PR can run arbitrary code (e.g., via Bundler/RuboCop) and use the write-scoped token to modify the base repository. Consider splitting into two jobs/workflows: a fork-safe lint job (triggered by pull_request or with job-level permissions: contents: read and no pushing) and a same-repo auto-correct job (only when head.repo.full_name == github.repository) that has write permissions and does the commit/push.

This issue also appears in the following locations of the same file:

  • line 18
  • line 44

Copilot uses AI. Check for mistakes.
@kenyonj kenyonj added this pull request to the merge queue Mar 30, 2026
Separate the lint workflow into:
- lint: read-only job that checks out fork code safely with
  contents: read permission, runs rubocop without auto-correct
- autocorrect: write job that only runs for same-repo PRs,
  checks out by branch ref, and pushes auto-corrections

This prevents fork PRs from executing untrusted code with
write-scoped GITHUB_TOKEN.
@kenyonj kenyonj removed this pull request from the merge queue due to a manual request Mar 30, 2026
github-merge-queue bot pushed a commit that referenced this pull request Mar 30, 2026
@kenyonj kenyonj enabled auto-merge March 30, 2026 19:43
Comment on lines +24 to +28
- name: Run RuboCop
run: |
bundle exec rubocop

autocorrect:
@kenyonj kenyonj disabled auto-merge March 30, 2026 19:44
CodeQL flags cache poisoning risk when pull_request_target checks
out untrusted code with bundler-cache enabled. A malicious fork
could poison the cache via a crafted Gemfile.lock.

Disable caching for the lint job (which runs fork code) and use
a plain bundle install instead. The autocorrect job (same-repo
only) retains bundler-cache.
Comment on lines +19 to +24
- name: Setup Ruby
uses: ruby/setup-ruby@v1.299.0
with:
bundler-cache: false

- name: Install dependencies
Comment on lines +24 to +27
- name: Install dependencies
run: bundle install

- name: Run RuboCop
@kenyonj kenyonj enabled auto-merge March 30, 2026 19:45
@kenyonj kenyonj added this pull request to the merge queue Mar 30, 2026
Merged via the queue into main with commit 4e3ae09 Mar 30, 2026
10 of 11 checks passed
@kenyonj kenyonj deleted the kenyonj/fix-lint-fork-checkout branch March 30, 2026 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants