fix(sw): scope every fetch-handler caches.match() to its owned cache (#514) - #612
Conversation
…514) CacheStorage is origin-scoped, not path-scoped. On a shared origin like qnbs.github.io (hosting multiple independent GitHub Pages projects), a bare caches.match(request) searches every cache on the origin, not just this app's own CACHE_STATIC/CACHE_DYNAMIC/CACHE_IMAGES — in principle a different project's cached response for a coincidentally-identical full URL could be served here. This is the read-path counterpart to DA-03 (#513), which fixed the same shared-origin invariant for cache deletion. Scopes all 4 unscoped caches.match() call sites (JS/CSS Cache-First, navigation fallback's two lookups, and the offlineFallback helper reachable from every fetch-handler catch path) to the explicit cache each one's value actually lives in, via the standard { cacheName } match option — the same pattern already used elsewhere in this file for reads via an opened cache handle. Regression test mirrors the existing source-contract style for this classic (non-importable) worker script: asserts every caches.match() call in the fetch handler and in offlineFallback carries an explicit cacheName, and is confirmed to fail against the pre-fix source.
…ression test check-doc-metrics.mjs computes its expected count from the actual Vitest source set — adding tests/unit/swCacheMatchScoping.test.ts (3 tests) shifted 597→598 files and 7433→7436 tests.
🤖 CodeAnt AI — Review Status
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Reviewer's GuideThe PR fixes shared-origin cache leakage risk by adding explicit cacheName scoping to all four relevant caches.match() reads in the service worker, adds source-based regression coverage for the fetch and offline fallback paths, and updates README test metrics. Sequence diagram for scoped service worker cache readssequenceDiagram
participant Browser
participant ServiceWorker
participant CacheStorage
participant Network
Browser->>ServiceWorker: fetch(request)
alt script or style
ServiceWorker->>CacheStorage: match(request, { cacheName: CACHE_STATIC })
alt cache miss
ServiceWorker->>Network: fetch(request)
ServiceWorker->>CacheStorage: put(request, response) in CACHE_STATIC
end
else navigation
ServiceWorker->>CacheStorage: match(request, { cacheName: CACHE_DYNAMIC })
alt navigation cache miss
ServiceWorker->>Network: fetch(request)
ServiceWorker->>CacheStorage: match(BASE + index.html, { cacheName: CACHE_STATIC })
alt fallback cache miss
ServiceWorker->>CacheStorage: match(BASE + offline.html, { cacheName: CACHE_STATIC })
end
end
end
CacheStorage-->>ServiceWorker: response or cache miss
ServiceWorker-->>Browser: response
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Docker |  |
Sep 5, 2026 5:11a.m. | Review ↗ |
| Python | |
Sep 5, 2026 5:11a.m. | Review ↗ |
| Rust | |
Sep 5, 2026 5:11a.m. | Review ↗ |
| Shell | |
Sep 5, 2026 5:11a.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
There was a problem hiding this comment.
The service worker cache scoping fix is correctly implemented and addresses the shared-origin security concern described in #514. All four caches.match() call sites now explicitly specify cacheName, preventing cross-app cache pollution on shared origins like qnbs.github.io. The contract-based test coverage is appropriate for a classic service worker. README metric updates reflect the new test file. No defects found - ready to merge.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
🏁 CodeAnt Quality Gate ResultsCommit: ✅ Overall Status: PASSEDQuality Gate Details
|
|
Warning Review limit reachedNext included review available in 40 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 76 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (3)
Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour. 📝 WalkthroughWalkthroughThe service worker now scopes cache reads to owned caches. A regression test validates the source contract. README test metrics were updated to 7,436+ tests across 598 files. ChangesService-worker cache read scoping
README test metrics
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to Service-worker cache reads are now restricted to the application’s owned caches, preventing same-origin cache results from other projects while preserving the existing asset, navigation, and offline fallback behavior. No merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
CodeAnt Nitpicks2 code suggestions1. The fetch-handler test requires only two calls, although the handler currently has three, so one lookup could disappear without the regression test failing.Incomplete implementation · 2. The assertion accepts any owned cache name, so a lookup in the wrong cache can pass even though it misses the response written or precached elsewhere.Incomplete implementation · |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1bc0275698
ℹ️ 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".
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Two real gaps: (1) the fetch-handler call-count assertion used a lower
bound, so a call site could silently disappear without failing; (2) the
cacheName assertion accepted any of the three owned caches, so a lookup
scoped to the wrong cache (e.g. reading CACHE_IMAGES for a value written to
CACHE_STATIC) would still pass. Verified the fix by injecting a wrong-cache
mistake locally and confirming it now fails, then reverting.
Each call site is now checked against its exact expected cache name;
the ${BASE} interpolation is normalized to a plain placeholder in the
extracted call text so the expected-value strings don't need to embed a
real template-literal placeholder (avoids fighting biome's
noTemplateCurlyInString on a literal string, without a suppression).
…ng test The review-driven tightening split one assertion into two, adding a 4th test (7436→7437) without changing the file count.
There was a problem hiding this comment.
Gates Passed
See analysis details in CodeScene
Quality Gate Profile: The Bare Minimum
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
* chore(release): bump version to v1.28.4 Patch release reconciling release-truth documentation with everything merged to main since v1.28.3 (62 commits / ~40 PRs, audited against live GitHub state, not assumed from commit subjects): - fix: PWA first-install unprompted reload (#585, PR #613) - fix: shared-origin service-worker cache-read isolation (#514, PR #612) - fix: Factory Reset could reboot into Settings instead of Welcome Portal (PR #592) - fix: preserve-first desktop corruption recovery (PR #542) and a distinct filesystem-I/O recovery action (PR #545) - fix: intentionally cleared project metadata no longer reappears (PR #546) - a11y: Welcome/Home dashboard WCAG AA contrast + reduced-motion cascade fix + default appearance preset change (#565, PR #609); ManuscriptEditor contrast (PR #560) - security: fflate ZIP64-parsing DoS override (PR #595); routine dependency floor bumps (PR #587, #561, #562, #594) - docs: R-15 secure desktop storage design contract admitted (PRs #564, #580, #581, #582, #584) — design only, no implementation yet - tests: visual regression testing repaired — baselines were directory listings, not the application (PR #610); IDB reset-quiescence hardening (PR #596); WelcomePortal E2E navigation made locale-independent (PR #590) Everything classified as pure internal/CI-governance churn (PR-size exception plumbing, dual-graph tooling, toolchain pins) is omitted from CHANGELOG.md as non-user-facing. Version bumped via the existing sync scripts (sync-tauri-version.mjs, sync-sw-version.mjs) across package.json, src-tauri/Cargo.toml, src-tauri/tauri.conf.json, src-tauri/Cargo.lock, AGENTS.md, and public/sw.js's APP_VERSION. CHANGELOG.md and README.md use the established release-candidate marker convention (<!-- release-candidate: v1.28.4 -->) so the dated entry and version badge are truthful before the v1.28.4 tag exists; both markers are removed in a follow-up post-release truth-sync once the tag and GitHub Release are published, matching the v1.28.2/v1.28.3 precedent. TODO.md's Current Sprint section was archived (its final "release cut remains open" bullet is now resolved — v1.28.2 and v1.28.3 both shipped) and replaced with the actual current sprint: this release cut followed by the R-15 desktop at-rest encryption priority program. AUDIT.md is intentionally not touched here — its release-gate entry requires real post-merge CI/CodeQL run evidence that doesn't exist until after this PR merges and the tag is cut, matching how every prior release's AUDIT.md entry was written (a follow-up commit, not part of the release-prep PR itself). * docs(release): correct premature done-marker on the v1.28.4 TODO item TODO.md's Current Sprint marked the release cut as done (checked 'v1.28.4' release cut, reconciling ... AUDIT.md truth ...) while this same PR's own Non-goals section correctly states AUDIT.md is not touched here, and while no tag, GitHub Release, or release artifacts exist yet. Corrected to in-progress language naming PR #615 directly and listing what actually remains pending (tag, release, artifacts, post-release AUDIT.md evidence). * docs(release): correct R-15 gate language and credit PR #596's real fix Two corrections from review, verified against live evidence before fixing: 1. TODO.md's Current Sprint claimed R-15 desktop at-rest encryption implementation was being prioritized now. docs/native/DESKTOP- MIGRATION-ROADMAP-REV3.md explicitly forbids pulling Wave 3/4 R-15 implementation ahead of unresolved Wave 2 authority prerequisites, and CORE-MIGRATION-LEDGER.md row 10 records S5_IMPLEMENTATION_READY=NO. Corrected to state R-15 design is complete but implementation stays gated behind the still-open Wave 2 prerequisite (ledger row 9: the project state-shape compatibility adapter), which is what this sprint's desktop-storage work actually is. 2. CHANGELOG.md listed PR #596 only as generic IDB test hardening under Tests. Verified against its actual diff: deleteDatabase() previously resolved on a genuine onerror or an onblocked event as if deletion succeeded, so wipeAllAppData() could report Factory Reset complete while a database was never actually deleted. onerror now rejects; onblocked waits for the connection to close before giving up. This is a real production data-integrity fix, not test hardening, and now has its own Fixed entry.
User description
Fixes #514.
Problem
CacheStorageis origin-scoped, not path-scoped.qnbs.github.iohosts multiple independent GitHub Pages projects on different paths, so a barecaches.match(request)inpublic/sw.js's fetch handler searches every cache on the origin, not just this app's ownCACHE_STATIC/CACHE_DYNAMIC/CACHE_IMAGES. In principle a different project's cachedResponsefor a request whose full URL happens to coincide with one WorldScript Studio fetches could be served here.Root cause
Four
caches.match()call sites never passed an explicitcacheName:caches.match(request)caches.match(request)andcaches.match(${BASE}index.html)offlineFallback()'s document branch —caches.match(${BASE}offline.html)(reachable from every fetch-handler catch path; not explicitly named in public/sw.js fetch handler: caches.match() reads are not scoped to owned caches #514 but the identical gap, so fixed alongside it)This is the read-path counterpart to DA-03 (#513), which fixed the same shared-origin invariant for cache deletion. Practical exploitability is low (per #514's own analysis — collision requires another origin-sibling app fetching/caching the exact same full URL), which is why this was correctly deferred rather than treated as a release blocker.
Scope
public/sw.jsfetch handler and theofflineFallbackhelper it calls, only. No behavior change for any request whose URL doesn't collide with a foreign cache entry — every scoped lookup targets exactly the cache each value is actually written to.Non-goals
Fix
Scoped all 4 call sites to the cache each one's value is actually written to, via the standard
{ cacheName }match option — the same pattern already used elsewhere in this file for reads via an opened cache handle (e.g. the image Cache-First branch).Regression coverage
tests/unit/swCacheMatchScoping.test.ts— mirrors the existing source-contract test style for this classic (non-importable,self-using) worker script (seeswLocaleStrategy.test.ts). Asserts everycaches.match()call in the fetch handler and inofflineFallbackcarries an explicitcacheName. Verified via a manual negative-control: stashed the fix, confirmed 2 of 3 assertions fail against the original unscoped code, restored the fix, confirmed all 3 pass.Validation
pnpm exec vitest run tests/unit/swCacheMatchScoping.test.ts— 3/3 pass.pnpm exec biome check --error-on-warnings public/sw.js tests/unit/swCacheMatchScoping.test.ts— clean.node scripts/check-doc-metrics.mjs— clean after syncing README's test-count metrics (598 files/7436 tests) for the new test file.Summary by Sourcery
Restrict service-worker cache reads to their owning caches and add regression coverage for shared-origin isolation.
Bug Fixes:
Documentation:
Tests:
caches.match()calls specify an owned cache.CodeAnt-AI Description
Prevent offline pages and cached assets from being served from other apps on the same domain
What Changed
Impact
✅ Prevents cross-app cached content✅ Keeps offline navigation within this app✅ Protects service-worker cache behavior from regressions💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by cubic
Scopes service worker cache reads to their owning caches so a request on the shared
qnbs.github.ioorigin can't be served a response from another project's cache.caches.match()calls in the fetch handler andofflineFallbacknow pass an explicitcacheName.Written for commit 1db048d. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Documentation