Skip to content

Conversation

@maarten-ic
Copy link
Collaborator

No description provided.

@maarten-ic
Copy link
Collaborator Author

The diff is generated okay (link), but a workflow triggered from a fork doesn't have permission to comment on the Pull Request. This is a security feature of GitHub to prevent that anyone that can create a PR can exfiltrate a secret token with write access to the repository. That's why the Action is failing.

Some options to work around this:

  1. Use the pull_request_target event trigger, with some checks to make this safe-ish: https://michaelheap.com/access-secrets-from-forks/
  2. Use the workflow_run trigger to make a comment to the Pull Request after the diff workflow has run: https://docs.github.com/en/actions/writing-workflows/choosing-when-your-workflow-runs/events-that-trigger-workflows#workflow_run

I think option 2 is the better of these, what do you think @olivhoenen?

@olivhoenen
Copy link
Collaborator

Option 2 sounds good indeed.

@maarten-ic
Copy link
Collaborator Author

The workflow is now split into two parts:

  1. pr_diff.yml is triggered by the pull_request event. This creates a diff of the generated DD XML (see output).
  2. pr_diff_comment_on_pr.yml is triggered by the workflow_run event, and will run when workflow 1 is successful. This workflow adds a comment to the PR so reviewers can easily navigate to the diff.

N.B. the workflow_run event only triggers for workflows defined in the default branch (develop). Therefore no comment is generated on this PR, because the second workflow is not yet committed to develop.
Both workflows are tested successfully in my fork, see maarten-ic#2.

@maarten-ic maarten-ic marked this pull request as ready for review April 7, 2025 13:16
Copy link
Collaborator

@SimonPinches SimonPinches left a comment

Choose a reason for hiding this comment

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

LGTM

For security reasons, pull_request workflows triggered from forks don't
have write permissions, and therefore cannot comment on the PR.

A second workflow (triggered by workflow_run) with sufficient
permissions will take care of posting the comment now.
@olivhoenen olivhoenen force-pushed the technical/pr-review-diff branch from 7f6823e to 7b5df1f Compare April 14, 2025 12:17
@olivhoenen olivhoenen merged commit c652d74 into iterorganization:develop Apr 14, 2025
1 check passed
@maarten-ic maarten-ic deleted the technical/pr-review-diff branch April 14, 2025 12:18
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