perf(platform): skip CSP/nonce middleware for hashed assets#2847
Closed
Israeltheminer wants to merge 1 commit into
Closed
perf(platform): skip CSP/nonce middleware for hashed assets#2847Israeltheminer wants to merge 1 commit into
Israeltheminer wants to merge 1 commit into
Conversation
On a cold load ~150 content-hashed JS/CSS chunks each ran the global secureHeaders middleware (a per-request CSP build + crypto nonce) on the single Bun event loop, though a static script/style response never uses the nonce'd CSP. Bypass secureHeaders for content-hashed /assets/* (mirroring the existing canvas-preview and /dav guards) and re-assert X-Content-Type-Options: nosniff on the file response — the one header that matters for a script byte stream.
Collaborator
Author
|
Closing — net negative on review. Security: this introduces a clickjacking hole. Skipping the security-headers middleware by URL pattern means a forged Benefit doesn't hold up: the per-request CSP nonce it avoids costs microseconds — it can't explain the ~20s cold-load TTFB. That bottleneck is delivery/infra (#2842), not this middleware. Keeping the security headers intact on every path. Cold-load will be pursued via the Caddy |
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.
Problem
Follow-up to the caching fix (#2843): repeat visits are now fast, but the first (cold) visit is still slow. Part of that is self-inflicted — on a cold load the ~150 content-hashed JS/CSS chunks each pass through the global
secureHeadersmiddleware, which builds a full CSP string and a fresh crypto nonce per request, on the single Bun event loop. A static script/style byte stream never uses that nonce'd CSP, so it's wasted work paid ~150× during the burst. (Confirmed: onmain, a request to/assets/vendor-katex-….jscomes back withcontent-security-policy: default-src 'self'; script-src 'nonce-…'.)Change
Bypass
secureHeadersfor content-hashed/assets/*in theapp.use('*')guard — mirroring the existing/canvas-previewand/dav/*bypasses — and re-assertX-Content-Type-Options: nosniffon the static-file response (the one header that matters for a script/style stream; CSP/X-Frame/Referrer are document-level). Uses the sameIMMUTABLE_ASSETpattern as the cache rule, so only hashed JS/CSS bypass; un-hashed images under/assets/still get the full middleware.Verification
bun run --filter @tale/platform test server.test.ts→ green (57), incl. a new test asserting a hashed-asset response carries no CSP while/api/healthstill does. Verified it fails onmain(asset gets the full CSP) and passes here.typecheckclean;oxfmt --check+oxlintclean.Honest caveat — this is measure-after
I can't prove from the HAR alone that this middleware is the cold-load bottleneck (the box's raw bandwidth and Caddy's on-the-fly gzip/zstd are also candidates). It's a correct optimization regardless (don't build a per-request nonce for static bytes). Next step: deploy, capture a fresh cold HAR, and measure first-load TTFB/throughput. If it's not enough, the documented escalation is moving
/assets/*to Caddyfile_server(bigger infra change; see #2842).