Preview Update 31st July - #398
Conversation
Signed-off-by: A.Arnold <anarnold@redhat.com>
📝 WalkthroughWalkthroughThe preview build now uploads a short-lived artifact. A separate workflow deploys the artifact to ChangesPR preview lifecycle
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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. Comment |
Signed-off-by: A.Arnold <anarnold@redhat.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/pr-preview-deploy.yml (1)
42-53: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winUse env vars instead of interpolating step outputs directly into
github-scriptbodies.
pull_number: ${{ steps.pr.outputs.number }}(line 51) andconst prNumber = ${{ steps.pr.outputs.number }};/const sha = '${{ steps.sha.outputs.sha }}';(lines 112-113) interpolate context expressions directly into JavaScript source before execution. zizmor flags this template-expansion pattern as a code-injection risk.pr-preview-cleanup.ymlalready avoids this by passingPR_NUMBERthroughenv:and reading it withprocess.env.PR_NUMBER(lines 59-64 of that file). Apply the same pattern here.🔒 Proposed fix
- name: Get PR head SHA id: sha uses: actions/github-script@v7 + env: + PR_NUMBER: ${{ steps.pr.outputs.number }} with: github-token: ${{ secrets.GITHUB_TOKEN }} script: | const pr = await github.rest.pulls.get({ owner: context.repo.owner, repo: context.repo.repo, - pull_number: ${{ steps.pr.outputs.number }}, + pull_number: Number(process.env.PR_NUMBER), }); core.setOutput('sha', pr.data.head.sha.substring(0, 7));- name: Post or update preview comment uses: actions/github-script@v7 + env: + PR_NUMBER: ${{ steps.pr.outputs.number }} + COMMIT_SHA: ${{ steps.sha.outputs.sha }} with: github-token: ${{ secrets.GITHUB_TOKEN }} script: | - const prNumber = ${{ steps.pr.outputs.number }}; - const sha = '${{ steps.sha.outputs.sha }}'; + const prNumber = Number(process.env.PR_NUMBER); + const sha = process.env.COMMIT_SHA;Also applies to: 107-153
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/pr-preview-deploy.yml around lines 42 - 53, Update the github-script steps in pr-preview-deploy.yml, including “Get PR head SHA” and the later script around prNumber/sha, to pass step outputs through the action’s env block and read them via process.env inside JavaScript. Remove direct ${{ steps... }} interpolation from script bodies while preserving the existing PR number and SHA behavior.Source: Linters/SAST tools
.github/workflows/pr-preview.yml (1)
43-53: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse an env var for the PR number instead of inline interpolation.
Line 46 already exposes
PR_NUMBERas an env var for the Jekyll build. Line 53 re-interpolates${{ github.event.number }}directly into the shell command instead of reusing that pattern.pr-preview-cleanup.ymluses the safer env + variable pattern consistently (see itsPR_NUMBERenv andprocess.env.PR_NUMBERusage). Apply the same pattern here for consistency and defense-in-depth against template-expansion into shell.♻️ Proposed fix
- name: Save PR number for deploy workflow - run: echo "${{ github.event.number }}" > _site/.pr-number + env: + PR_NUMBER: ${{ github.event.number }} + run: echo "${PR_NUMBER}" > _site/.pr-number🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/pr-preview.yml around lines 43 - 53, Update the “Save PR number for deploy workflow” step to expose PR_NUMBER through the step’s env configuration and write the shell variable to _site/.pr-number, removing the inline github.event.number interpolation. Keep the existing Jekyll build PR_NUMBER usage unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/pr-preview-deploy.yml:
- Around line 10-12: Update the workflow’s top-level permissions block to
include actions: read alongside contents and pull-requests, so the artifact
download step using actions/download-artifact@v4 and a cross-run run-id can
authenticate successfully.
---
Nitpick comments:
In @.github/workflows/pr-preview-deploy.yml:
- Around line 42-53: Update the github-script steps in pr-preview-deploy.yml,
including “Get PR head SHA” and the later script around prNumber/sha, to pass
step outputs through the action’s env block and read them via process.env inside
JavaScript. Remove direct ${{ steps... }} interpolation from script bodies while
preserving the existing PR number and SHA behavior.
In @.github/workflows/pr-preview.yml:
- Around line 43-53: Update the “Save PR number for deploy workflow” step to
expose PR_NUMBER through the step’s env configuration and write the shell
variable to _site/.pr-number, removing the inline github.event.number
interpolation. Keep the existing Jekyll build PR_NUMBER usage unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e50d3c10-a3f6-4e6f-83d3-7e0d383ee11d
📒 Files selected for processing (3)
.github/workflows/pr-preview-cleanup.yml.github/workflows/pr-preview-deploy.yml.github/workflows/pr-preview.yml
| permissions: | ||
| contents: write | ||
| pull-requests: write |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does actions/download-artifact v4 require actions:read permission to download an artifact from a different workflow run via run-id in the same repository?
💡 Result:
Yes, when using actions/download-artifact@v4 to download an artifact from a different workflow run (even within the same repository), you must provide a github-token with the appropriate permissions [1][2][3]. By default, the actions/download-artifact action is scoped to the current workflow run [1][4]. To access artifacts from a different run, you must specify the github-token input [2][3]. This token must have at least actions:read permissions for the target repository to successfully authenticate the request via the GitHub API [5][6]. While the default GITHUB_TOKEN provided in a standard workflow environment is typically scoped to the current repository and workflow, it may lack the necessary permissions to access artifacts from other workflow runs unless specifically configured or replaced with a token (such as a Personal Access Token or a scoped installation token) that possesses the required actions:read access [5]. If the provided token lacks these permissions, the action will fail during the API call [5].
Citations:
- 1: https://github.com/actions/download-artifact/
- 2: https://github.com/actions/download-artifact/blob/v4.1.0/README.md
- 3: https://github.com/actions/download-artifact/tree/v4.1.4
- 4: https://github.com/actions/download-artifact/blob/v4.0.0/README.md
- 5: https://deepwiki.com/actions/download-artifact/3.3-cross-repository-downloads
- 6: https://github.com/actions/toolkit/blob/main/packages/artifact/README.md
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Workflow metadata:"
if [ -f .github/workflows/pr-preview-deploy.yml ]; then
sed -n '1,40p' .github/workflows/pr-preview-deploy.yml | cat -n
else
echo "missing .github/workflows/pr-preview-deploy.yml"
fi
echo
echo "Action download step:"
sed -n '20,36p' .github/workflows/pr-preview-deploy.yml | cat -n 2>/dev/null
echo
echo "Search github-token inputs in download-artifact usages:"
rg -n "download-artifact@v|github-token|run-id" .github/workflows || trueRepository: migtools/mta-documentation
Length of output: 2805
Add actions: read permission for cross-run artifact download.
actions/download-artifact@v4 uses the provided github-token to fetch artifacts from another run via run-id. Since this workflow declares an explicit permission block without actions: read, the deploy job cannot authenticate that artifact request and the preview deployment fails.
🔧 Proposed fix
permissions:
contents: write
pull-requests: write
+ actions: readApplies to lines: 10-12 and 27-33.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| permissions: | |
| contents: write | |
| pull-requests: write | |
| permissions: | |
| contents: write | |
| pull-requests: write | |
| actions: read |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/pr-preview-deploy.yml around lines 10 - 12, Update the
workflow’s top-level permissions block to include actions: read alongside
contents and pull-requests, so the artifact download step using
actions/download-artifact@v4 and a cross-run run-id can authenticate
successfully.
2 participants
PR #398 — Fork-Compatible PR Preview Workflows
Why this PR was needed
I made a fundamental mistake in PR #392 in which i worked on the logic that contributors work on branches within the
migtools/mta-documentationrepo itself.In practice, contributors work on forks. GitHub intentionally gives fork PRs a read-only
GITHUB_TOKENas a security measure — malicious fork code cannot write to the upstream repo. The original workflows checked for this and simply skipped, which is why every fork PR showed "job skipped."What PR #398 does differently
The original was a single workflow that built and deployed in one job. PR #398 splits this into three workflows using the pattern GitHub recommends for fork support.
pr-preview.yml— read-only token, builds Jekyll, uploads_siteas an artifactpr-preview-deploy.yml— new file, triggered byworkflow_run, always runs in the base repo context so the write token is valid even for fork PRspull_requestevent — skipped for forkspr-preview-cleanup.yml— switched topull_request_target, which always runs in the base repo contextKey technical detail
The
workflow_runandpull_request_targetevents always execute in the context of the base repository (main branch), not the fork — so the write token is valid and can push togh-pagesregardless of where the PR originated.Summary by CodeRabbit
New Features
Bug Fixes