fix(pwa): harden shell service worker against favicon cache poisoning - #594
Merged
Conversation
The shell SW served /favicon.ico cache-first from 'freshell-shell-v1'
with no revalidation, ever. Two vectors could poison that entry with the
SPA-shell HTML: (1) install-time cache.addAll() caching whatever 200
came back -- including the SPA fallback served while dist/client was
mid-rebuild -- and (2) the fetch handler answering ANY failed request,
including icon requests, with caches.match('/index.html'). Once an HTML
body sat under /favicon.ico, browsers rendered a generic document icon
instead of the shell-with-flame, permanently, because the cache name
never changed and cached entries were never revalidated.
Fixes:
- Bump CACHE_NAME to 'freshell-shell-v2' so activate purges every
existing (potentially poisoned) v1 cache in the wild.
- Guard all cache writes by content-type: a text/html response is only
cacheable under '/' and '/index.html'; for any other shell asset it is
passed through but never persisted.
- Replace install's all-or-nothing cache.addAll with per-asset
fetch+guard that tolerates individual failures (the fetch handler
falls through to the network on cache misses anyway).
- Drop the index.html fallback for non-navigation requests; a failed
asset fetch now surfaces as a network error instead of HTML-as-icon.
Navigations keep their offline index.html fallback.
Adds test/unit/client/sw-shell-cache.test.ts pinning the contract:
legacy-cache purge, no-HTML-under-asset-paths (install and runtime),
no HTML fallback for assets, partial-precache install, cache-first
serving, navigation offline fallback, and /api + non-shell passthrough.
Generated with Amplifier
Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
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
1 participant
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
The tab favicon rendered as Chrome's generic document icon instead of the shell-with-flame. On refresh the correct icon appeared for a moment (from Chrome's favicon DB), then was replaced.
Root cause: the shell service worker serves
/favicon.icocache-first fromfreshell-shell-v1with no revalidation, ever, and two vectors could poison that entry with SPA-shell HTML:cache.addAll()cached whatever 200 came back — including the SPA fallback served for/favicon.icowhiledist/clientwas mid-rebuild..catch(() => caches.match('/index.html'))answered any failed request — including icon requests — with HTML.Once HTML sat under
/favicon.ico, the browser rendered a generic document icon permanently: the cache name never changed and entries were never revalidated.Fix
CACHE_NAMEtofreshell-shell-v2— activate purges every existing (potentially poisoned) v1 cache in the wild. Self-healing for affected users, no manual cache clearing.text/htmlis only cacheable under/and/index.html; for any other shell asset it passes through but is never persisted (closes both poisoning vectors).cache.addAll, tolerating individual failures (fetch handler falls through to network on cache misses anyway).index.htmlfallback for non-navigation requests — a failed asset fetch surfaces as a network error instead of HTML-as-icon. Navigations keep their offline fallback.Tests
New
test/unit/client/sw-shell-cache.test.ts(9 tests) evaluates the realsw.jssource against mockself/caches/fetchand pins the contract: legacy-cache purge, no-HTML-under-asset-paths (install + runtime), no HTML fallback for assets, partial-precache install, cache-first serving, navigation offline fallback,/api+ non-shell passthrough.TDD: 5/9 tests red against the old sw.js, all green after.
pwa-shell+static-cache-headerssuites pass;typecheck:clientclean.Generated with Amplifier
Co-Authored-By: Amplifier 240397093+microsoft-amplifier@users.noreply.github.com