Skip to content

ci: correct fork check in pull_request_target workflow - #33653

Merged
crisbeto merged 1 commit into
angular:mainfrom
ShraddhaZoman:fix-pull-request-target-fork-check
Sep 9, 2026
Merged

ci: correct fork check in pull_request_target workflow#33653
crisbeto merged 1 commit into
angular:mainfrom
ShraddhaZoman:fix-pull-request-target-fork-check

Conversation

@ShraddhaZoman

@ShraddhaZoman ShraddhaZoman commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Fixes #33632

The condition github.event.repository.fork == false is ineffective
in a pull_request_target context, since github.event.repository
always refers to the base repository, never the fork. This meant the
check always evaluated to true, allowing the job to run even for
fork-originated PRs.

Use github.event.pull_request.head.repo.fork instead, which
correctly reflects the PR's actual source repository.

Confirmed this approach with @mabrukhany-beep on the linked issue (#33632) before
implementing.

The condition github.event.repository.fork == false is ineffective in
a pull_request_target context, since github.event.repository always
refers to the base repository, never the fork. This meant the check
always evaluated to true, allowing the job to run even for
fork-originated PRs.

Use github.event.pull_request.head.repo.fork instead, which correctly
reflects the PR's actual source repository.

Fixes angular#33632
@pullapprove
pullapprove Bot requested a review from crisbeto August 9, 2026 12:54
@google-cla

google-cla Bot commented Aug 9, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@angular-robot angular-robot Bot added the area: build & ci Related the build and CI infrastructure of the project label Aug 9, 2026
@ShraddhaZoman

Copy link
Copy Markdown
Contributor Author

@googlebot I signed it!

@ShraddhaZoman

Copy link
Copy Markdown
Contributor Author

Hi @crisbeto — following up on this PR, it's been about a month since I opened it. The approach was confirmed by @mabrukhany-beep on the linked issue (#33632). Could you take a look, and approve the pending workflow runs / pullapprove review when you get a chance? Happy to make any changes if needed.

@crisbeto crisbeto added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release labels Sep 9, 2026
@crisbeto
crisbeto merged commit 1c79709 into angular:main Sep 9, 2026
16 of 18 checks passed
@crisbeto

crisbeto commented Sep 9, 2026

Copy link
Copy Markdown
Member

This PR was merged into the repository. The changes were merged into the following branches:

crisbeto pushed a commit that referenced this pull request Sep 9, 2026
The condition github.event.repository.fork == false is ineffective in
a pull_request_target context, since github.event.repository always
refers to the base repository, never the fork. This meant the check
always evaluated to true, allowing the job to run even for
fork-originated PRs.

Use github.event.pull_request.head.repo.fork instead, which correctly
reflects the PR's actual source repository.

Fixes #33632

(cherry picked from commit 1c79709)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: merge The PR is ready for merge by the caretaker area: build & ci Related the build and CI infrastructure of the project target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: github.event.repository.fork check is ineffective in pull_request_target workflows

2 participants