Skip to content

Commit 69b5db1

Browse files
authored
fix(sw): scope every fetch-handler caches.match() to its owned cache (#514) (#612)
* fix(sw): scope every fetch-handler caches.match() to its owned cache (#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. * docs: sync README test-count metrics for the new SW cache-scoping regression 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. * test(sw): tighten cache-scoping regression assertions per review 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). * docs: sync README test-count metrics for the tightened SW cache-scoping test The review-driven tightening split one assertion into two, adding a 4th test (7436→7437) without changing the file count.
1 parent 2847305 commit 69b5db1

3 files changed

Lines changed: 92 additions & 8 deletions

File tree

README.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
<img src="https://img.shields.io/badge/Storage-IndexedDB_v8-F59E0B" alt="IndexedDB v8">
1414
<img src="https://img.shields.io/badge/PWA-v3.0-5BB974?logo=pwa" alt="PWA v3.0">
1515
<img src="https://img.shields.io/badge/i18n-19_locales-2940_keys-0EA5E9" alt="i18n 19 locales — 2940 keys">
16-
<img src="https://img.shields.io/badge/Tests-7433%2B_%2F_597_files-22C55E" alt="7433+ tests / 597 files">
16+
<img src="https://img.shields.io/badge/Tests-7437%2B_%2F_598_files-22C55E" alt="7437+ tests / 598 files">
1717
<img src="https://img.shields.io/codecov/c/github/qnbs/WorldScript-Studio?logo=codecov&label=Coverage" alt="Codecov Coverage">
1818
<img src="https://img.shields.io/badge/License-MIT-22C55E" alt="License MIT">
1919
<img src="https://img.shields.io/github/actions/workflow/status/qnbs/WorldScript-Studio/.github/workflows/ci.yml?branch=main&logo=github" alt="CI Status">
@@ -511,7 +511,7 @@ The Settings → AI panel shows a live GPU status badge with adapter details and
511511
| **Document Export** | docx + jszip | Word-compatible `.docx` generation (lazy-loaded) |
512512
| **PWA** | Service Worker + Web App Manifest v3 | Offline support, installability, Workbox chunking |
513513
| **i18n** | Custom React Context (`I18nContext.tsx`) | 2940 keys × 19 locales (de/en/es/fr/it + ar/he/fa RTL Beta + ja/zh/pt/el/fi/sv/hu/is/eu/ru/ko Beta); EN fallback; `localStorage` persistence |
514-
| **Testing** | Vitest 4.x (7433+ tests / 597 files) + Playwright E2E | Unit/integration + cross-browser E2E; Stryker mutation (manual workflow) |
514+
| **Testing** | Vitest 4.x (7437+ tests / 598 files) + Playwright E2E | Unit/integration + cross-browser E2E; Stryker mutation (manual workflow) |
515515
| **Code Quality** | Biome (lint + format) + TypeScript 7 (tsgo) strict | `--error-on-warnings` in CI; zero `any` policy |
516516
| **Visualization** | Force-directed graph | Interactive character relationship network |
517517
| **Desktop** | Tauri v2 | Cross-platform installer; auto-updater via `latest.json` |
@@ -549,7 +549,7 @@ WorldScript-Studio/
549549
│ ├── sw.js # PWA Service Worker
550550
│ └── manifest.json # PWA Web App Manifest v3
551551
├── tests/
552-
│ ├── unit/ # Vitest unit tests (7433+ tests, 597 files) — count spans tests/, components/, packages/*/tests/, not just this folder
552+
│ ├── unit/ # Vitest unit tests (7437+ tests, 598 files) — count spans tests/, components/, packages/*/tests/, not just this folder
553553
│ │ ├── ai/ # aiSmallModules, aiCoreFallbackPaths
554554
│ │ └── settings/ # WebLlmPanel, AiSections
555555
│ └── e2e/ # Playwright specs + helpers.ts
@@ -711,7 +711,7 @@ The main pipeline is [`.github/workflows/ci.yml`](.github/workflows/ci.yml). Opt
711711
| `scorecard` | weekly + `main` push | OpenSSF Scorecard — SARIF uploaded to GitHub Code Scanning |
712712

713713
**Current test metrics (2026-09-04, source-synchronized; CI remains authoritative for pass/fail):**
714-
- **7433+ unit tests** across **597 test files** — CI is authoritative for pass/fail
714+
- **7437+ unit tests** across **598 test files** — CI is authoritative for pass/fail
715715
- Coverage thresholds: lines ≥ 80 · branches ≥ 66 · functions ≥ 72 · statements ≥ 78 — enforced in CI (see Codecov badge for live metrics)
716716
- i18n: **2940 keys × 19 locales** (en/de/fr/es/it + ar/he/fa RTL Beta + ja/zh/pt/el/fi/sv/hu/is/eu/ru/ko Beta)
717717

public/sw.js

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,8 @@ async function trimCache(cacheName, maxEntries) {
8888
// ── Offline fallback per resource type ───────────────────────
8989
async function offlineFallback(request) {
9090
if (request.destination === 'document') {
91-
const cached = await caches.match(`${BASE}offline.html`);
91+
// QNBS-v3: scoped to CACHE_STATIC (where offline.html is precached) — an unscoped caches.match() searches every cache on the shared qnbs.github.io origin, not just this app's own.
92+
const cached = await caches.match(`${BASE}offline.html`, { cacheName: CACHE_STATIC });
9293
return cached || new Response('<!doctype html><title>Offline</title><p>WorldScript Studio ist offline.</p>', {
9394
headers: { 'Content-Type': 'text/html' },
9495
status: 503,
@@ -241,7 +242,8 @@ self.addEventListener('fetch', (event) => {
241242
(request.destination === 'script' || request.destination === 'style')
242243
) {
243244
event.respondWith(
244-
caches.match(request).then((cached) => {
245+
// QNBS-v3: scoped to CACHE_STATIC (where the network path below stores it) — same shared-origin rationale as offlineFallback's fix above.
246+
caches.match(request, { cacheName: CACHE_STATIC }).then((cached) => {
245247
const networkFetch = fetch(request).then((response) => {
246248
if (response.ok) {
247249
caches.open(CACHE_STATIC).then((c) => c.put(request, response.clone()));
@@ -290,8 +292,9 @@ self.addEventListener('fetch', (event) => {
290292
return response;
291293
})
292294
.catch(async () =>
293-
(await caches.match(request)) ||
294-
(await caches.match(`${BASE}index.html`)) ||
295+
// QNBS-v3: each lookup scoped to the cache it's actually written to (CACHE_DYNAMIC for the navigated URL, CACHE_STATIC for the precached SPA shell) — same shared-origin rationale as above.
296+
(await caches.match(request, { cacheName: CACHE_DYNAMIC })) ||
297+
(await caches.match(`${BASE}index.html`, { cacheName: CACHE_STATIC })) ||
295298
offlineFallback(request)
296299
)
297300
);
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
// @vitest-environment node
2+
import { readFileSync } from 'node:fs';
3+
import { fileURLToPath } from 'node:url';
4+
import { describe, expect, it } from 'vitest';
5+
6+
// QNBS-v3: regression guard for #514 — CacheStorage is origin-scoped, not path-scoped, so a bare
7+
// caches.match(request) on a shared origin like qnbs.github.io searches every cache on the origin,
8+
// not just this app's own. sw.js is a classic service worker (uses `self`, not importable), so we
9+
// assert its source contract instead of executing it, mirroring swLocaleStrategy.test.ts's pattern.
10+
const swSource = readFileSync(
11+
fileURLToPath(new URL('../../public/sw.js', import.meta.url)),
12+
'utf8',
13+
);
14+
15+
/** Extract the body of the `self.addEventListener('fetch', ...)` handler. */
16+
function fetchHandlerBlock(src: string): string {
17+
const start = src.indexOf("self.addEventListener('fetch'");
18+
expect(start).toBeGreaterThan(-1);
19+
const end = src.indexOf("self.addEventListener('message'", start);
20+
expect(end).toBeGreaterThan(start);
21+
return src.slice(start, end);
22+
}
23+
24+
/** Extract the body of the `offlineFallback` helper, called from every fetch-handler catch path. */
25+
function offlineFallbackBlock(src: string): string {
26+
const start = src.indexOf('async function offlineFallback');
27+
expect(start).toBeGreaterThan(-1);
28+
const end = src.indexOf('\n}', start);
29+
expect(end).toBeGreaterThan(start);
30+
return src.slice(start, end);
31+
}
32+
33+
/** Every top-level `caches.match(...)` call found in a source block (not `cache.match(...)` on an already-opened, already-scoped handle). Strips `//` comments first so prose mentioning `caches.match()` can't masquerade as a real call site, and normalizes the `${BASE}` interpolation to a plain placeholder so expected-value strings in this file never need to embed a real template-literal placeholder themselves. */
34+
function cachesDotMatchCalls(block: string): string[] {
35+
const codeOnly = block
36+
.split('\n')
37+
.map((line) => line.replace(/\/\/.*$/, ''))
38+
.join('\n');
39+
const calls: string[] = [];
40+
const re = /\bcaches\.match\([^;]*?\)/g;
41+
let m: RegExpExecArray | null = re.exec(codeOnly);
42+
while (m !== null) {
43+
calls.push(m[0].replace(/\$\{BASE\}/, '<BASE>'));
44+
m = re.exec(codeOnly);
45+
}
46+
return calls;
47+
}
48+
49+
describe('service worker — caches.match() is always scoped to an owned cache (#514)', () => {
50+
it('the fetch handler contains exactly the 3 known caches.match() call sites', () => {
51+
// QNBS-v3: exact count, not a lower bound — a lower bound would let a call site silently disappear (e.g. an accidental merge/refactor) without this regression test failing.
52+
const calls = cachesDotMatchCalls(fetchHandlerBlock(swSource));
53+
expect(calls.length).toBe(3);
54+
});
55+
56+
it('the JS/CSS Cache-First lookup reads from CACHE_STATIC, where the network path writes it', () => {
57+
const start = swSource.indexOf('JS / CSS bundles');
58+
expect(start).toBeGreaterThan(-1);
59+
const end = swSource.indexOf('Locale JSON', start);
60+
expect(end).toBeGreaterThan(start);
61+
const calls = cachesDotMatchCalls(swSource.slice(start, end));
62+
expect(calls).toEqual(['caches.match(request, { cacheName: CACHE_STATIC })']);
63+
});
64+
65+
it("the navigation fallback reads the navigated URL from CACHE_DYNAMIC and the SPA shell from CACHE_STATIC — not each other's cache", () => {
66+
const start = swSource.indexOf('Navigation — Network First');
67+
expect(start).toBeGreaterThan(-1);
68+
const end = swSource.indexOf('Everything else', start);
69+
expect(end).toBeGreaterThan(start);
70+
const calls = cachesDotMatchCalls(swSource.slice(start, end));
71+
expect(calls).toEqual([
72+
'caches.match(request, { cacheName: CACHE_DYNAMIC })',
73+
'caches.match(`<BASE>index.html`, { cacheName: CACHE_STATIC })',
74+
]);
75+
});
76+
77+
it('offlineFallback (reachable from every fetch-handler catch path) reads offline.html from CACHE_STATIC, where it is precached', () => {
78+
const calls = cachesDotMatchCalls(offlineFallbackBlock(swSource));
79+
expect(calls).toEqual(['caches.match(`<BASE>offline.html`, { cacheName: CACHE_STATIC })']);
80+
});
81+
});

0 commit comments

Comments
 (0)