fix(web): harden service worker against SPA HTML cache poison - #2289
Conversation
Bump cache schema to app-cache-v2 and only CacheFirst hashed /static JS/CSS with MIME validation on install, runtime, and background precache so 200 text/html SPA fallbacks cannot be stored as scripts.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe service worker now uses cache schema v2 with MIME- and path-aware validation. Install, activation, runtime routing, HTML invalidation, static asset caching, and background precaching reject invalid response types and handle legacy caches selectively. ChangesService worker cache schema v2
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant ServiceWorker
participant CacheStorage
participant Network
Browser->>ServiceWorker: request hashed /static/ JS or CSS
ServiceWorker->>CacheStorage: check cached response
CacheStorage-->>ServiceWorker: return cached response
ServiceWorker->>Network: fetch when missing or invalid
Network-->>ServiceWorker: return static response
ServiceWorker->>CacheStorage: store validated response
ServiceWorker-->>Browser: return response
Possibly related PRs
🚥 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 |
Deploying refly-branch-test with
|
| Latest commit: |
a8b2acb
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4745834c.refly-branch-test.pages.dev |
| Branch Preview URL: | https://fix-sw-cache-mime-guards.refly-branch-test.pages.dev |
Fail install (no skipWaiting) when critical precache fails so v2 does not claim clients or delete v1 with an incomplete cache. Also require HTML MIME and same-origin when clearing document shells.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7bdbd10498
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![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
🤖 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 `@apps/web/src/service-worker-sw.ts`:
- Around line 225-246: Bound each critical asset fetch in the install precache
flow by adding a per-request timeout with AbortController, and pass its signal
to fetch in the criticalUrls Promise.all map. Ensure the timeout aborts stalled
requests so installation fails promptly while preserving the existing fail-fast
validation and skipWaiting behavior.
🪄 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: 2ab6d390-162b-4628-a6a1-754c66692591
📒 Files selected for processing (1)
apps/web/src/service-worker-sw.ts
- Fix TS2322: cachedResponse is optional in Workbox callback param - On activate, copy MIME-valid /static JS/CSS from app-cache-v1 into v2 before deleting v1 so open tabs keep lazy chunks
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8904a8640c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Migrate v1 static assets sequentially, delete each source entry after copy, and never let QuotaExceededError block activate/claim.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51d6ca8555
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/src/service-worker-sw.ts (1)
286-298: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueUnnecessary
.clone()—responseisn't reused after this line.
response(fromlegacy.match(request)) is only ever passed totarget.put(); nothing else in this function reads it afterward. Cloning it just to hand the clone toput()adds an extra body tee for no benefit, which is a bit ironic in a function whose whole purpose is to be quota/memory-conscious.♻️ Proposed cleanup
const existing = await target.match(request); if (!existing) { - await target.put(request, response.clone()); + await target.put(request, response); migrated += 1; }🤖 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 `@apps/web/src/service-worker-sw.ts` around lines 286 - 298, Remove the unnecessary response.clone() call in the migration flow around legacy.match and target.put; pass the response directly to target.put(request, response), preserving the existing cache checks, migration count, and legacy deletion behavior.
🤖 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.
Nitpick comments:
In `@apps/web/src/service-worker-sw.ts`:
- Around line 286-298: Remove the unnecessary response.clone() call in the
migration flow around legacy.match and target.put; pass the response directly to
target.put(request, response), preserving the existing cache checks, migration
count, and legacy deletion behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b9bb45ff-8432-4899-9b14-ec1c5ed490b1
📒 Files selected for processing (1)
apps/web/src/service-worker-sw.ts
Also carry MIME-valid non-home/non-SSR document entries into v2 so offline refresh keeps working after legacy bucket delete. Drop unnecessary response.clone() on migrate put.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8b2acbd4b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
2 participants
Summary
app-cache-v2and drop only known legacy bucketapp-cache-v1on activate/static/**/*.{js,css}with MIME validation on install, runtime read/write, and background precache200 text/htmlSPA fallbacks so they can never be stored/served as scripts (fixes post-rollback MIME errors)Context
After a bad Pages deploy + rollback, some clients cached SPA HTML under JS URLs via CacheFirst +
statuses: [200]. Edge purge cannot heal Cache API poison. This change prevents recurrence and clears v1 for clients that can load the new SW.Note: Fully broken clients (app never boots) still need a one-time site-data clear; new SW registration requires enough JS to load.
Test plan
service-worker.<hash>.jsis emitted/static/js/*.jsCacheFirst hits are real JS MIMEtext/htmlunder a.jscache key → next request drops entry and refetches JSapp-cache-v1is deleted; unrelated caches untouched/v1/*and/api/*are not CacheFirst-cachedSummary by CodeRabbit
/api/and/v1/requests network-only; routed static hashed JS/CSS through a dedicated/static/CacheFirst strategy.