Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct handling of previews. #580

Merged
merged 2 commits into from
Aug 3, 2024
Merged

Conversation

freakboy3742
Copy link
Member

PR #575 modified the GitHub checkout associated with the preview action; in doing so, it inadvertently broke previews. This is because the CI action triggers on pull_request_target, not pull_request. pull_request_target defaults to checking out the SHA of the branch base, not the head.

We have to use pull_request_target for security reasons; if a pull_request trigger is used, malicious code could alter the repository or exfiltrate secrets.

This takes a different approach to the code removed by #575, explicitly naming the SHA of the PR branch, rather than referencing the PR as an indirect reference. Using the PR number as a proxy is potentially insecure as the PR itself could be modified.

To prove the preview is working, the PR modifies the label of the first entry on the https://beeware.org/news/events/ page. This is a change that is also made by #579.

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

@freakboy3742 freakboy3742 added the preview Approved for an automated preview label Aug 3, 2024
@freakboy3742
Copy link
Member Author

If my understanding is correct, this PR's preview run will use the current main status of the CI workflow, which doesn't have a new preview code (this is why pull_request_target exists). Once this is merged, we should be able to merge an existing PR (e.g, #579) with main, re-tag for preview, and see the desired effect.

@freakboy3742 freakboy3742 requested a review from glasnt August 3, 2024 08:03
Copy link

github-actions bot commented Aug 3, 2024

Visit the preview URL for this PR (updated for commit e135c34):

https://beeware-org--pr580-preview-fix-tqch4y3p.web.app

(expires Sat, 10 Aug 2024 08:05:08 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: b0da44bc067e7d9a4255c77cb2c5fce572218cec

@glasnt glasnt merged commit 5c751f5 into beeware:lektor Aug 3, 2024
2 checks passed
@freakboy3742 freakboy3742 mentioned this pull request Aug 3, 2024
4 tasks
kattni pushed a commit to kattni/beeware.github.io that referenced this pull request Aug 3, 2024
* Reference the SHA of the pull request HEAD in the checkout.

* Minor content change to validate previews.
@freakboy3742 freakboy3742 deleted the preview-fix branch August 4, 2024 02:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preview Approved for an automated preview
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants