fix(build): exclude Vite build manifests from Cloudflare asset uploads#1850
Merged
Conversation
On Cloudflare Workers the ASSETS binding serves any uploaded file whose path matches the request BEFORE the Worker runs. Cloudflare builds enable `build.manifest` (and `ssrManifest` for Pages Router) so the worker entry can compute lazy chunks, which writes Vite's manifests to `dist/client/.vite/`. Because nothing excluded that directory from the asset bundle, `/.vite/manifest.json` and `/.vite/ssr-manifest.json` were publicly fetchable, leaking the full source-file -> chunk mapping (including the paths of routes that are never linked from the UI). The Node production server already blocks `/.vite/` explicitly (server/static-file-cache.ts); this adds the Cloudflare-side equivalent by writing a `.assetsignore` into `dist/client` during the cloudflare-build closeBundle hook (right after the existing `_headers` generation, after the manifests have been read). wrangler matches `.assetsignore` with gitignore semantics, so the bare `.vite` entry excludes the directory and its contents from upload, and the file is never served. The merge logic preserves any user-authored `.assetsignore` and only appends missing patterns, so it is idempotent across rebuilds. The writer lives in a small typed helper (build/assets-ignore.ts) with focused unit tests; a real Cloudflare build test asserts the wiring emits `.assetsignore` with `.vite` while the manifest still exists on disk.
commit: |
Contributor
|
Member
Author
|
/bigbonk review for issues |
Contributor
There was a problem hiding this comment.
Review: fix(build): exclude Vite build manifests from Cloudflare asset uploads
Verdict: Approve. The fix is correct, well-scoped, and the premise checks out against authoritative Cloudflare docs. No blocking issues found.
Verified
- Root cause confirmed. Cloudflare's Worker script routing docs state plainly: "Cloudflare will first attempt to serve static assets if one matches the incoming request" before invoking the Worker. With
run_worker_firstunset (defaultfalse), an uploadeddist/client/.vite/manifest.jsonis served directly — the Worker's/.vite/block never runs. The premise holds. not_found_handling: "none"is not a red herring. I checkedbuild/run-prerender.ts:197which says "every request hits the worker first." That comment is about non-matching paths (page requests), and is consistent with this PR —not_found_handlingonly governs misses, not matching assets. No contradiction.- Parity claim is accurate. The Node prod server blocks
/.vite/atserver/static-file-cache.ts:248; this.assetsignoreis the correct Cloudflare-side equivalent. .assetsignoresemantics are correct. Per Cloudflare binding docs, the file uses.gitignoreformat, so a bare.viteline excludes the directory recursively..assetsignoreitself is never uploaded.- All deploy paths covered. No
run_worker_firstis set indeploy.ts(generateWranglerConfig) or anyexamples/*/wrangler.jsonc, so every Cloudflare target was vulnerable and is now fixed via the sharedvinext:cloudflare-buildcloseBundlehook. - Hook placement is correct.
ensureAssetsIgnore(clientDir)runs after the manifests are read for lazy-chunk computation (index.ts:4683-4706) and after_headersgeneration — so it never races the reads.
Implementation notes (non-blocking)
- The merge logic correctly preserves user content and is idempotent.
lines.join("\n")produces a single trailing newline; whitespace-only existing files fall through to the header branch with the full pattern set appended — both fine. - Unlike the
_headerswrite (guarded by!fs.existsSync),ensureAssetsIgnorealways runs and merges. This is the better behavior for a security guard (it self-heals if the file exists without.vite) and is intentional per the PR description.
Tests
tests/assets-ignore.test.ts(7) — pass. Good coverage: defaults, dir creation, idempotency, user-content preservation, no-duplicate, custom patterns.tests/cache-adapters-build.test.ts(1) — pass. Asserts.assetsignorecontains.viteand.vite/manifest.jsonstill exists on disk, so the exclusion is both wired and meaningful. Reusing the existing real CF build for zero extra CI cost is the right call.vp checkon all 4 changed files: format, lint, types all clean.
Suggestions (optional, not blocking)
- The PR's own "Verification suggestion" is worth taking: add a post-deploy smoke check (
curl -sw '%{http_code}' https://<preview>/.vite/manifest.json→404) toscripts/smoke-test.shto convert this from code-confirmed to live-confirmed. - Consider (separate PR) whether
_headers/_routes.jsonshould also be in.assetsignore— though wrangler treats_headersas config rather than a servable asset, so likely a non-issue. Out of scope here.
LGTM.
Contributor
|
Review posted on PR #1850. SummaryI reviewed PR #1850 ( What I verified:
I noted two optional, non-blocking suggestions: adopt the PR's own live smoke-test idea, and a separate-PR question about |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On Cloudflare Workers, the ASSETS binding serves any uploaded file matching the request path before the Worker runs. Cloudflare builds enable
build.manifest(andssrManifestfor Pages Router) so the worker entry can compute lazy chunks — this writes Vite's manifests todist/client/.vite/. Nothing excluded that directory from the uploaded asset bundle, so on a deployed Worker:returned the Vite build/SSR manifests to anyone. These leak the full source-file → output-chunk mapping, including the paths of routes that are never linked from the UI (e.g.
app/admin/internal-tools/page.tsx) — unauthenticated reconnaissance.This is an information disclosure the framework already intends to prevent: the Node production server explicitly blocks
/.vite/(server/static-file-cache.ts, with tests). The block simply never existed on the Cloudflare deployment target, because Cloudflare serves assets ahead of the Worker (the deploy config usesnot_found_handling: "none"and norun_worker_first).Fix
Write a
.assetsignoreintodist/clientduring thevinext:cloudflare-buildcloseBundlehook (right after the existing_headersgeneration, after the manifests have been read for lazy-chunk computation). wrangler reads.assetsignorefrom the assets directory and matches it with.gitignoresemantics via theignorepackage, so the bare.viteentry excludes the directory and everything beneath it from upload — and.assetsignoreitself is never served.packages/vinext/src/build/assets-ignore.ts(ensureAssetsIgnore) owns the write..assetsignoreand only appends missing patterns, so it's idempotent across rebuilds and never clobbers user config.dist/client/.vite/manifest.json, is unaffected) — they're only excluded from the Cloudflare asset upload.Why
.assetsignore(not delete /run_worker_first)@cloudflare/vite-pluginbuild andvinext deploy→wrangler deployboth honor.assetsignore.Scoped to
.vite(the confirmed issue). Source maps aren't emitted todist/clientby default; if we later want to exclude*.mapwhenbuild.sourcemapis opted into, the helper already accepts a custom pattern list.Testing
tests/assets-ignore.test.ts— unit tests for the helper: default patterns, dir creation, idempotency (no duplicate entries), preserving user content, and honoring custom patterns.tests/cache-adapters-build.test.ts— extends the existing real Cloudflare build to assertdist/client/.assetsignorecontains.vitewhiledist/client/.vite/manifest.jsonstill exists on disk (proves the exclusion is both wired and meaningful, at zero extra CI cost).Verification suggestion
After this merges, a deployed-Worker smoke check (
curl -sw '%{http_code}' https://<preview>/.vite/manifest.json→404) would convert this from code-confirmed to live-confirmed; happy to add it toscripts/smoke-test.shif desired.🤖 Generated with opencode