ci: verify the bundled runtime is built from source, not supplied by PRs - #5759
Conversation
The files in v3/internal/assetserver/bundledassets are go:embed'ed into every Wails v3 app, and minified JS cannot be reviewed by eye — a PR supplying its own bundle bytes is a malware vector. - Pin esbuild to 0.28.1 in the runtime Taskfile (npx esbuild@latest made byte-for-byte verification impossible) and expose a public runtime:build:assets task that rebuilds only the embedded bundles. - Add a Verify Runtime Assets check on every PR: when bundledassets or runtime source changed, hermetically rebuild from the PR's source and fail unless the committed bundles match byte-for-byte. Runs with a read-only token and no secrets, and self-skips on unrelated PRs so it can be made a required status check. - Watch bundledassets in publish-npm so any bundle bytes that land on master are overwritten by a trusted source-derived rebuild. - Resync the committed bundles with master's source: rebuilding revealed they were already stale (missing DateFromTime and the mac:WebViewWebContentProcessDidTerminate event present in src/) — exactly the drift this check is meant to catch.
WalkthroughThis PR pins the runtime esbuild version, adds a bundle rebuild task, regenerates ChangesRuntime bundle build and verification
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant PR as Pull Request
participant Workflow as verify-runtime-assets.yml
participant Task as task runner
participant Repo as Repository
PR->>Workflow: trigger on pull request to master
Workflow->>Repo: diff HEAD vs HEAD^1 for bundledassets/runtime paths
alt no relevant changes
Workflow-->>PR: set relevant=false and skip
else relevant changes detected
Workflow->>Task: npm ci and task runtime:build:assets
Task->>Repo: rebuild bundledassets output
Workflow->>Repo: compare rebuilt output vs committed bundledassets
alt mismatch
Workflow-->>PR: fail with remediation instructions
else match
Workflow-->>PR: report success
end
end
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
task runs build:debug and build:production as concurrent deps; two simultaneous 'npx esbuild@0.28.1' invocations race installing into the npx cache on a cold runner and one dies (seen on the JS Tests job). Pin esbuild as an exact devDependency and call the npm-ci-installed binary: one install, no build-time network fetch, and the version now lives in package-lock.json where npm ci enforces it.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
v3/internal/runtime/Taskfile.yaml (1)
31-36: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the repeated esbuild binary path into a var.
The literal
desktop/@wailsio/runtime/node_modules/.bin/esbuildpath is duplicated across build:debug and build:production. A task-level var would reduce duplication if the path ever changes.♻️ Suggested refactor
+vars: + ESBUILD: desktop/@wailsio/runtime/node_modules/.bin/esbuild + build:debug: internal: true cmds: - - desktop/@wailsio/runtime/node_modules/.bin/esbuild desktop/@wailsio/runtime/src/index.ts ... + - "{{.ESBUILD}} desktop/@wailsio/runtime/src/index.ts ..."
🤖 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 `@v3/internal/runtime/Taskfile.yaml`:
- Around line 31-36: The build:debug and build:production tasks in Taskfile.yaml
directly invoke desktop/@wailsio/runtime/node_modules/.bin/esbuild, so they will
fail unless dependencies are installed first. Add install-deps as a dependency
on these task entries so the build:assets and build:runtime task chains can run
successfully without requiring a prior npm install.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d783fa63-af13-46e4-9fe4-1fd127fb5111
⛔ Files ignored due to path filters (1)
v3/internal/runtime/desktop/@wailsio/runtime/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
v3/internal/runtime/Taskfile.yamlv3/internal/runtime/desktop/@wailsio/runtime/package.json
✅ Files skipped from review due to trivial changes (1)
- v3/internal/runtime/desktop/@wailsio/runtime/package.json
Review feedback: build:debug/build:production invoked the local esbuild binary but nothing guaranteed node_modules existed, so a bare 'task runtime:build:assets' failed without a prior npm install. Both tasks now dep on install-deps, which gets 'run: once' so the two parallel builds don't race concurrent npm installs. The duplicated binary path moves into an ESBUILD var.
Why
v3/internal/assetserver/bundledassets/runtime{,.debug}.jsare go:embed'ed into every Wails v3 application. Minified JS cannot be reviewed by eye, so a PR that supplies its own bundle bytes is a straightforward malware vector — and today nothing checks that the committed bundles correspond to the source. Worse,publish-npm's post-merge rebuild only watches the runtime source paths, so a bundle-only change on master would never be overwritten.Proof this is a real blind spot: rebuilding master's bundles from master's own source produced a diff — the committed bundles are already stale (missing
DateFromTimeand themac:WebViewWebContentProcessDidTerminateevent that exist insrc/). That drift shipped unnoticed; a hostile diff would hide the same way.What
npx esbuild@latest, which makes byte-for-byte verification impossible) and add a publicruntime:build:assetstask that rebuilds only the embedded bundles.bundledassets/**orv3/internal/runtime/**changed, hermetically rebuild from the PR's source and fail unless the committed bundles match byte-for-byte. Runs under plainpull_requestwith a read-only token and no secrets, so building untrusted source is safe. Self-skips (green, ~20s) when no runtime paths changed, so it can be marked required without wedging unrelated PRs.publish-npmnow also watchesbundledassets/**, so any bundle bytes that land on master get overwritten by a trusted source-derived rebuild.Contributor impact
Anyone touching runtime source keeps committing the rebuilt bundles as before — the check just proves the bytes match. On mismatch the failure message gives the exact regen commands (
npm ci+task runtime:build:assets).Follow-up for a maintainer
Mark “Bundled runtime matches source” as a required status check in branch protection — that's what turns this from a signal into a guarantee.
Note: #5462 will grow a trivial bundle conflict on its next master-merge (both sides touch the minified files); resolution is just
task runtime:build:assetson the merged tree.Type of change
How Has This Been Tested?
Rebuilt the bundles from a clean checkout (
rm -rf node_modules+task runtime:build:assets) on macOS with Node 20 and compared against CI's Node 22 output: byte-identical, confirming the pinned esbuild makes verification deterministic across environments.Rebuilt master's bundles from master's source with the pinned esbuild, which surfaced the pre-existing drift this PR fixes (committed bundles were missing
DateFromTimeand themac:WebViewWebContentProcessDidTerminateevent).The verify workflow runs on this PR itself (it touches
bundledassets/**), exercising the "relevant change → rebuild → compare" path; unrelated PRs exercise the self-skip path.Windows
macOS
Linux (ubuntu-latest, via the workflow run on this PR)
Test Configuration
Not applicable: this changes CI workflows and the runtime build Taskfile, not platform runtime code, so
wails doctoroutput is not relevant.Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Chores