fix preview checkout #834
fix preview checkout #834
Conversation
|
""" WalkthroughThe deploy-preview GitHub Actions workflow was restructured to separate build and deploy phases. It now triggers on specific pull request events targeting the master branch, builds the project artifact in one job, and deploys using that artifact in a dependent job with expanded permissions and secret usage isolated to deployment. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant GitHub
participant DeployWorkflow
Developer->>GitHub: Open/Synchronize/Reopen PR targeting master
GitHub->>DeployWorkflow: Trigger deploy-preview workflow
DeployWorkflow->>DeployWorkflow: Build job: checkout PR head SHA, install, build, upload artifact
DeployWorkflow->>DeployWorkflow: Deploy job: download artifact, deploy using secrets, comment on PR
Possibly related PRs
Suggested labels
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai , why does this not work as expected. the pull-request deployment preview should checkout the pr branch head commit. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 2274 @nerdCopter I can see why this isn't working as expected. Your PR is actually trying to fix this exact issue by adding When you modify a workflow file in a PR, GitHub still uses the version of that workflow from the target branch (master), not the version being proposed in your PR. This creates a "chicken and egg" situation - your fix won't be applied until after it's merged. The workflow that ran on your PR was using the old version from master that doesn't have the explicit checkout reference, which is why it didn't check out the PR head commit as expected. Your changes look correct and should fix the issue once they're merged. After merging, future PRs will properly check out their head commits for previews. (_/） |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/deploy-preview.yml (2)
4-4: Verify event filter covers all desired triggers
Restricting toopened,synchronize, andreopenedon PRs targetingmasteravoids unnecessary runs, but if you need previews on other actions (e.g.labeled,unlabeled) or want a manual trigger, consider adding those event types or aworkflow_dispatch.
20-21: Pin checkout to the exact PR commit
Specifyingref: ${{ github.event.pull_request.head.sha }}is correct to deploy the precise PR head. As an optional improvement, you can harden and extend the clone step:with: ref: ${{ github.event.pull_request.head.sha }} + fetch-depth: 0 # retrieve full history if you rely on tags or deep logs + persist-credentials: false # don’t persist GITHUB_TOKEN in the workspace
|
@blckmn , please confirm and merge as needed. |
|
Yes, i've tryed to use preview url recently, and seen strange bbe version. |
|
@coderabbitai, please review. does this have the same security issue as betaflight/betaflight-configurator#4468 (review) ? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainResponse from running the script: Let me look at the security issue referenced in the other repository: Response from running the script: @nerdCopter Yes, this PR has exactly the same security issue as the one you referenced. The workflow uses This combination is problematic because:
Possible solutions:
Let me know if you'd like more details about implementing any of these solutions! 🏁 Script executed: Length of output: 1558 🏁 Script executed: Length of output: 1256 @nerdCopter Yes, this workflow has the same security vulnerability. The workflow uses This combination creates a security risk because:
Recommended fixes:
This is a commonly overlooked security issue in GitHub Actions that can lead to secret exfiltration. |
|
@coderabbitai, please implement the fix in the single .yml file. EDIT: number 2 Alternative |
|
Tip For best results, initiate chat on the files or code changes. A summary of the edits CodeRabbit can apply:
A plan of each step that CodeRabbit will take in order to execute the edits for each file (with inline rough draft snippets): ✏️ .github/workflows/deploy-preview.ymlReplace the existing on:
pull_request_target:
types: [opened, synchronize, reopened]
branches:
- masterto: on:
pull_request:
types: [opened, synchronize, reopened]
branches:
- masterA summary of the context that CodeRabbit is considering across the codebase:
|
|
Tip For best results, initiate chat on the files or code changes. |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
Tip For best results, initiate chat on the files or code changes. |
This comment was marked as outdated.
This comment was marked as outdated.
|
|
Preview URL: https://a7f82e12.betaflight-blackbox.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.github/workflows/deploy-preview.yml (3)
15-17: Consider addingfetch-depth: 0for full Git history
By defaultactions/checkoutfetches only a single commit. If your build relies on Git history (e.g., generating changelogs, tags, or versioning), you should setfetch-depth: 0. Otherwise you can skip this.- - name: Checkout code - uses: actions/checkout@v4 - with: - ref: ${{ github.event.pull_request.head.sha }} - persist-credentials: false + - name: Checkout code + uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.sha }} + persist-credentials: false + fetch-depth: 0
34-38: Pinupload-artifactto the latest stable version and set retention
You’ve chosenactions/upload-artifact@v4, which may be a beta. Pinning tov3ensures stability. You can also addretention-daysto automatically expire build artifacts.- - name: Upload build artifact - uses: actions/upload-artifact@v4 - with: - name: dist-files - path: dist + - name: Upload build artifact + uses: actions/upload-artifact@v3 + with: + name: dist-files + path: dist + retention-days: 1
40-48: Restrict deploy job permissions to least privilege
The deploy job currently requests:permissions: actions: read contents: read deployments: write issues: write pull-requests: writeIf you only need to create deployments and comment on the PR, you can drop
issues: writeand the defaultcontents: read:- permissions: - actions: read - contents: read - deployments: write - issues: write - pull-requests: write + permissions: + deployments: write + pull-requests: write
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
.github/workflows/deploy-preview.yml(2 hunks)
🔇 Additional comments (2)
.github/workflows/deploy-preview.yml (2)
49-51: Reevaluate the deploy job timeout
The deploy job is capped at 5 minutes. Make sure your Cloudflare Pages deploy reliably finishes within this window; otherwise consider raisingtimeout-minutes.Please verify your average deploy duration and adjust the timeout accordingly.
52-57: The artifact download step cleanly separates code execution from secrets—good job. The syntax and inputs look correct.
Thes URL hash is a unique URL hash for the deployment, it is NOT the same as the git short hash. |
|
Tip For best results, initiate chat on the files or code changes. @blckmn Thanks for the clarification! You're absolutely correct - the hash in the Cloudflare Pages deployment URL (a7f82e12 in this case) is a unique identifier generated by Cloudflare for that specific deployment and is completely unrelated to the git commit hash. Cloudflare generates these unique URL hashes for each deployment to ensure they have stable, distinct URLs. This is standard behavior for their preview deployments regardless of which git commit triggered the build. |
|
It works. |
* fix preview checkout * coderabbit recommendede issues: write permission * implement security improvement
4 participants
ref: ${{ github.event.pull_request.head.sha }}for the Deployment PreviewSummary by CodeRabbit