fix(security): pin and verify the drift-check script the moq template fetches and executes - #22
fix(security): pin and verify the drift-check script the moq template fetches and executes#22yakimoto wants to merge 1 commit into
Conversation
… fetches and executes
The MoQ draft-drift workflow template pulled a shell script from the `main` branch of
wave-av/wave-moq-edge over plain `curl`, chmod'd it, and ran it — no pin, no checksum. Anyone who
could push to that branch, or anyone who compromised it, got code execution on every PR in every
repo that adopted this template. `actions/checkout` was on the mutable `v5` tag and the job carried
default token permissions.
The fix follows the standard this repo already states in `public-repo-guard.yml` — vendored-first,
version-pinned AND SHA-256-verified:
* vendored `scripts/check-moq-draft-version.sh` wins and nothing is fetched
* the fallback fetch is pinned to commit c47c67c8 and checked against its SHA-256 before chmod
* actions/checkout pinned to 93cb6efe (v5.0.1), the same pin this repo already uses
* least-privilege `permissions: contents: read`, `persist-credentials: false`, and a concurrency
group
Receipt — zizmor --persona=auditor on the template:
before: 5 findings (1 high, 3 medium, 1 low)
after: No findings to report.
The curl-and-execute itself is not one of zizmor's audits, so the checksum gate closes a hole the
linter never reported. Content at the pinned commit is byte-identical to `main` today
(sha256 0d671c34…), so this changes what CAN happen, not what does.
Verified: raw.githubusercontent fetch at the pinned SHA returns 2488 bytes matching the recorded
digest; `yaml.safe_load` parses; wave-moq-edge is public so the unauthenticated fetch resolves.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 54 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Comment |
| # wave-av/wave-moq-edge @ scripts/check-moq-draft-version.sh | ||
| SCRIPT_COMMIT: c47c67c81c2b1acef0b54f07a26911ba3316d780 | ||
| SCRIPT_SHA256: 0d671c3457b6bda5247681f5c12b9472908028b29f5a6f5074549349ab57363a |
There was a problem hiding this comment.
🔍 README still describes the old unpinned fetch and org-wide single-source pin
workflow-templates/README.md:11-15 states the template "Pulls the canonical drift-check script from wave-av/wave-moq-edge so the IETF-current pin lives in exactly one place across the org." With the fetch now frozen to SCRIPT_COMMIT, repos that do not vendor the script will keep running a snapshot until this template's pin is bumped and copied downstream — the documented "single place, always current" behaviour no longer holds. The README (and possibly the template properties description mentioning "currently draft-17", workflow-templates/moq-draft-drift.properties.json) should be updated to describe the vendored-first + pinned-fallback model and the refresh procedure.
Was this helpful? React with 👍 or 👎 to provide feedback.
ApprovabilityVerdict: Approved ac0af14 Security hardening change that pins dependencies to commit SHAs and adds SHA-256 integrity verification for a fetched script. The author owns this workflow template file. The unresolved comment requests only documentation updates (README), not code fixes. You can customize Macroscope's approvability policy. Learn more. |


The MoQ draft-drift workflow template fetched a shell script over plain
curlfrom themainbranch of another repo,chmod +x'd it, and ran it:No pin, no checksum, no
permissions:block. Anyone who could push towave-av/wave-moq-edge@main— or anyone who compromised it — got code execution on every PR, every push to main, and every Monday 06:00 cron, in every repo that adopted this template.actions/checkout@v5is a mutable tag, so that is a second unpinned input on the same job.The standard already existed in this repo
public-repo-guard.yml, three files over, states it in its own header comment:This template is now held to that same bar rather than a new one.
What changed
scripts/check-moq-draft-version.shexists in the adopting repo, it runs and nothing is fetched.c47c67c8, content checked againstsha256 0d671c34…viasha256sum -c -beforechmod.--proto '=https' --tlsv1.2on the curl.actions/checkoutpinned to93cb6efe(v5.0.1) — the same pinpublic-repo-guard.ymlin this repo already uses.permissions: contents: read,persist-credentials: false, and a concurrency group.Receipt
Worth naming: the curl-and-execute is not one of zizmor's audits. The linter reported the unpinned action, the missing permissions and the missing concurrency — it never reported the remote script. The checksum gate closes a hole no tool in the pipeline was looking for.
Content at the pinned commit is byte-identical to
maintoday (2488 bytes,0d671c34…), so this changes what can happen, not what does.Fleet sweep
I swept all 138 repos in the org for live copies rather than assuming the template was the only instance. Exactly one exists —
wave-moq-edge/.github/workflows/moq-draft-drift.yml, the source repo, where the script is vendored so no fetch occurs. Its unpinnedcheckoutand default permissions are fixed in wave-av/wave-moq-edge#150: 4 zizmor findings → 0.Note on
pull_requestThis trigger runs with a read-only token and no secrets, so executing a contributor's vendored script is ordinary CI behaviour and is left as-is. The header comment now says explicitly not to convert it to
pull_request_target, which would hand a fork's code a write-scoped token.Note
Medium Risk
Changes CI supply-chain and permission boundaries for a fleet-adopted workflow template; behavior for repos with a vendored script stays the same, but unpinned remote script execution is removed.
Overview
Hardens
workflow-templates/moq-draft-drift.ymlso the MoQ drift gate matches the vendored-first, pinned-and-verified posture documented inpublic-repo-guard.yml.Drift script supply chain: When
scripts/check-moq-draft-version.shis not in the repo, the workflow no longer curlsmainfromwave-moq-edgeand runs it unchecked. It uses a vendored copy when present; otherwise it fetches a commit-pinned URL, runssha256sum -cbeforechmod, and tightens curl with--proto '=https' --tlsv1.2.Workflow hygiene:
actions/checkoutmoves from the mutablev5tag to the same commit SHA aspublic-repo-guard, withpersist-credentials: false. The workflow addspermissions: contents: read, a concurrency group, and a header comment on pin refresh and why not to usepull_request_target.Reviewed by Cursor Bugbot for commit ac0af14. Configure here.
Summary by cubic
Secures the MoQ draft-drift workflow by running a vendored script when present and using a pinned, checksum-verified fallback. Also pins
actions/checkoutand tightens permissions to close a cross-repo supply-chain risk.scripts/check-moq-draft-version.shif present; otherwise fetch fromwave-av/wave-moq-edgeat a specific commit and verify SHA-256 beforechmod.actions/checkoutto a specific commit and setpersist-credentials: false.permissions: contents: readand a concurrency group.pull_request_target.Written for commit ac0af14. Summary will update on new commits.