Skip to content

[Security Fix] Enforce Cache-Control semantics in handleNavigation - #1

Open
hesam-oxe wants to merge 1 commit into
unlikelyzero:feat/pwa-supportfrom
hesam-oxe:fix/pwa-security-cache-control
Open

hesam-oxe wants to merge 1 commit into
unlikelyzero:feat/pwa-supportfrom
hesam-oxe:fix/pwa-security-cache-control

Conversation

@hesam-oxe

Copy link
Copy Markdown

Summary

Resolves the security concern raised by @sylvesterkaczmarek during review of nasa#8425: handleNavigation() wrote every successful navigation response to the offline cache without consulting the response's Cache-Control header. On deployments that serve authenticated or personalized pages, those responses persist in Cache Storage and remain readable offline — including by later, unauthenticated sessions on the same browser profile.

Changes

src/plugins/pwa/serviceWorkerHandlers.js

  • handleNavigation now checks isCacheableNavigationResponse() before cache.put():
    • responses guarded by Cache-Control: no-store, private or no-cache are never stored, even when they are 200 OK;
    • only complete 200 responses that were not the result of a redirect are stored;
    • the live response is still returned to the browser untouched — guarding only skips storage, it never alters the online code path.
  • Directive parsing (cacheControlForbidsCaching) is case-insensitive and tolerates whitespace and directive arguments (e.g. no-cache="Set-Cookie"), per RFC 9111.
  • The network-first strategy and the offline fallback (previously cached page → precached index.html) are unchanged. Pages withheld from the cache can, by design, never be served by the fallback.
  • Stale entries from previous builds are already purged during activate(), so deploying this build also evicts anything the earlier, permissive version may have cached.

src/plugins/pwa/serviceWorkerHandlersSecuritySpec.js (new)

  • proves public responses (with and without Cache-Control: public) are still cached for offline use;
  • proves no-store, private, no-cache, combined, mixed-case and argument-carrying header variants are never written to Cache Storage, even on 200 OK responses;
  • proves a guarded response is still served live when online, and never overwrites an already cached public page;
  • proves the offline fallback still serves cached public pages, falls back to the app shell for guarded pages, and propagates network errors when nothing cacheable exists.

Notes for reviewers

  • max-age=0 (a revalidation hint) intentionally remains cacheable; only directives that forbid storage or shared caching are enforced. must-revalidate / Vary: Authorization handling can be discussed as a follow-up.
  • The new spec keeps the project's *Spec.js naming so Karma's index-test.cjs discovery picks it up in CI.

Fixes the review feedback on nasa#8425.

Navigation responses guarded by Cache-Control: no-store, private or
no-cache may contain authenticated or user-specific data and must never
be written to the offline cache. handleNavigation now checks these
directives before calling cache.put(), tightens the cacheable status to
200, and keeps the network-first strategy and offline fallback intact.

Adds serviceWorkerHandlersSecuritySpec.js covering the security behavior
and the offline fallback to prevent regressions.

Resolves the concern raised by @sylvesterkaczmarek on nasa#8425.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant