From f0e6a9b7885cab459e3c44aea26edf1c3d86b6ac Mon Sep 17 00:00:00 2001 From: Michael Lam Date: Sun, 3 May 2026 18:18:27 -0700 Subject: [PATCH 1/2] fix: keep login assets out of service worker cache --- api/routes.py | 6 ++- static/sw.js | 73 +++++++++++++++++++------- tests/test_service_worker_api_cache.py | 39 ++++++++++++++ tests/test_sprint19.py | 19 +++++++ 4 files changed, 118 insertions(+), 19 deletions(-) diff --git a/api/routes.py b/api/routes.py index 8c430178..2ee9bff6 100644 --- a/api/routes.py +++ b/api/routes.py @@ -1489,7 +1489,7 @@ button:hover{background:rgba(124,185,255,.25)}
- + """ # ── Insights endpoint ────────────────────────────────────────────────────────── @@ -1621,9 +1621,13 @@ def handle_get(handler, parsed) -> bool: _login_strings = _LOGIN_LOCALE[ _resolve_login_locale_key(_lang) ] + from urllib.parse import quote + from api.updates import WEBUI_VERSION + version_token = quote(WEBUI_VERSION, safe="") _page = ( _LOGIN_PAGE_HTML.replace("{{BOT_NAME}}", _bn) .replace("{{BOT_NAME_INITIAL}}", _bn[0].upper()) + .replace("{{WEBUI_VERSION}}", version_token) .replace("{{LANG}}", _html.escape(_login_strings["lang"])) .replace("{{LOGIN_TITLE}}", _html.escape(_login_strings["title"])) .replace("{{LOGIN_SUBTITLE}}", _html.escape(_login_strings["subtitle"])) diff --git a/static/sw.js b/static/sw.js index cb8e2071..3fe629e4 100644 --- a/static/sw.js +++ b/static/sw.js @@ -16,11 +16,12 @@ const CACHE_NAME = 'hermes-shell-__WEBUI_VERSION__'; // here, every cache lookup against `?v=...` URLs would miss and fall through // to network, defeating the pre-cache. // -// Unversioned assets (`./`, manifest.json, favicons) are referenced from -// index.html without a cache-bust query, so they stay unversioned here too. +// Do not pre-cache './' or login assets here: under password auth they can be +// either the authenticated app shell or login code, and stale cached responses +// can make valid password submits fail until the user clears browser cache. +// Navigations populate './' only after a successful non-redirect network load. const VQ = '?v=__WEBUI_VERSION__'; const SHELL_ASSETS = [ - './', './static/style.css' + VQ, './static/boot.js' + VQ, './static/ui.js' + VQ, @@ -65,8 +66,10 @@ self.addEventListener('activate', (event) => { // Fetch strategy: // - API calls (/api/*, /stream) → always network (never cache) +// - Login assets → always network (never cache stale auth code) +// - Page navigations → network-first so auth redirects/cookies are honored // - Shell assets → cache-first with network fallback -// - Everything else → network-first, fall back to offline page +// - Everything else → network-only self.addEventListener('fetch', (event) => { const url = new URL(event.request.url); @@ -77,6 +80,16 @@ self.addEventListener('fetch', (event) => { // prevents the browser from seeing a new cache version after local patches. if (url.pathname.endsWith('/sw.js')) return; + // Login assets must always hit the network. Older login.js builds have had + // subpath-sensitive auth POST paths; if the service worker caches one, the + // password can keep failing until the user manually clears browser cache. + if ( + url.pathname.endsWith('/login') || + url.pathname.endsWith('/static/login.js') + ) { + return; + } + // API and streaming endpoints — always go to network. // The WebUI may be mounted under a subpath such as /hermes/, so API // requests can look like /hermes/api/sessions rather than /api/sessions. @@ -90,6 +103,44 @@ self.addEventListener('fetch', (event) => { return; // let browser handle normally } + // Page navigations must be network-first. A stale cached './' response can + // otherwise hide the server's 302-to-login after auth expiry, or ignore a + // freshly set login cookie until the user manually refreshes. + if (event.request.mode === 'navigate') { + event.respondWith( + fetch(event.request).then((response) => { + if ( + event.request.method === 'GET' && + response.status === 200 && + !response.redirected + ) { + const clone = response.clone(); + caches.open(CACHE_NAME).then((cache) => cache.put('./', clone)); + } + return response; + }).catch(() => { + return caches.match('./').then((cached) => cached || new Response( + '' + + '

You are offline

' + + '

Hermes requires a server connection. Please check your network and try again.

' + + '', + { headers: { 'Content-Type': 'text/html' } } + )); + }) + ); + return; + } + + // Only explicit shell assets use cache-first. Everything else should hit the + // network so stale one-off files (especially auth/login scripts) do not get + // trapped in CacheStorage until a manual cache clear. + const scopePath = new URL(self.registration.scope).pathname; + const relPath = url.pathname.startsWith(scopePath) + ? url.pathname.slice(scopePath.length) + : url.pathname.replace(/^\/+/, ''); + const shellPath = './' + relPath.replace(/^\/+/, '') + url.search; + if (!SHELL_ASSETS.includes(shellPath)) return; + // Shell assets: cache-first event.respondWith( caches.match(event.request).then((cached) => { @@ -104,20 +155,6 @@ self.addEventListener('fetch', (event) => { caches.open(CACHE_NAME).then((cache) => cache.put(event.request, clone)); } return response; - }).catch(() => { - // Offline fallback for navigation requests. - // Note: caches.match() returns a Promise (always truthy in a `||` check), - // so we must await/then to unwrap it — otherwise the `new Response(...)` - // branch is dead code and the browser falls back to its default offline page. - if (event.request.mode === 'navigate') { - return caches.match('./').then((cached) => cached || new Response( - '' + - '

You are offline

' + - '

Hermes requires a server connection. Please check your network and try again.

' + - '', - { headers: { 'Content-Type': 'text/html' } } - )); - } }); }) ); diff --git a/tests/test_service_worker_api_cache.py b/tests/test_service_worker_api_cache.py index 3118357f..4238efde 100644 --- a/tests/test_service_worker_api_cache.py +++ b/tests/test_service_worker_api_cache.py @@ -35,3 +35,42 @@ def test_service_worker_does_not_intercept_its_own_script(): assert "url.pathname.endsWith('/sw.js')" in SW_SRC, ( "service worker must bypass /sw.js so a stale cached worker cannot block cache-version updates" ) + + +def test_service_worker_uses_network_first_for_page_navigation(): + """Page navigations must hit the server before cache so expired auth redirects work.""" + navigate_idx = SW_SRC.find("event.request.mode === 'navigate'") + assert navigate_idx != -1, "service worker must special-case page navigations" + fetch_idx = SW_SRC.find("fetch(event.request)", navigate_idx) + cache_idx = SW_SRC.find("caches.match", navigate_idx) + assert fetch_idx != -1, "navigation branch must try the live server first" + assert cache_idx != -1, "navigation branch may use cached shell only as offline fallback" + assert fetch_idx < cache_idx, ( + "navigation requests must be network-first, not cache-first, so auth redirects " + "and freshly set login cookies are honored without a manual refresh" + ) + + +def test_service_worker_does_not_precache_page_shell_under_auth(): + """Do not cache './' during install; it may be the authenticated app or login redirect.""" + shell_block = SW_SRC[SW_SRC.find("const SHELL_ASSETS"):SW_SRC.find("];", SW_SRC.find("const SHELL_ASSETS"))] + assert "'./'" not in shell_block and '"./"' not in shell_block, ( + "pre-caching './' can serve a stale authenticated app shell while logged out; " + "navigation should populate shell cache only after a successful non-redirect network load" + ) + + +def test_service_worker_never_caches_login_page_or_login_script(): + assert "url.pathname.endsWith('/login')" in SW_SRC or "url.pathname.includes('/login')" in SW_SRC, ( + "service worker must bypass the login page so stale auth UI cannot survive until cache clear" + ) + assert "url.pathname.endsWith('/static/login.js')" in SW_SRC, ( + "service worker must bypass static/login.js so stale login handlers cannot block password submit" + ) + + +def test_service_worker_only_cache_puts_shell_assets_or_valid_navigation_shell(): + assert "SHELL_ASSETS.includes(shellPath)" in SW_SRC, ( + "non-navigation cache puts must be limited to the explicit app shell asset allowlist; " + "a generic cache-first handler can trap stale login.js until users clear cache" + ) diff --git a/tests/test_sprint19.py b/tests/test_sprint19.py index b3ce6584..69f7055a 100644 --- a/tests/test_sprint19.py +++ b/tests/test_sprint19.py @@ -60,6 +60,25 @@ def test_login_page_served(): assert r.status == 200 assert "Sign in" in html assert "Hermes" in html + assert 'src="static/login.js?v=' in html + assert 'src="/static/login.js"' not in html + + +def test_login_page_cache_busts_login_script(): + """GET /login must version login.js so stale cache/SW entries cannot trap old auth code.""" + from api import routes + + assert "static/login.js?v={{WEBUI_VERSION}}" in routes._LOGIN_PAGE_HTML + + +def test_login_route_injects_webui_version_for_login_script(): + """The /login route should replace the login.js version placeholder.""" + from pathlib import Path + + src = Path(__file__).resolve().parents[1].joinpath("api", "routes.py").read_text(encoding="utf-8") + login_block = src[src.find('if parsed.path == "/login"'):src.find('if parsed.path == "/api/auth/status"')] + assert "WEBUI_VERSION" in login_block + assert "{{WEBUI_VERSION}}" in login_block # ── Security headers ───────────────────────────────────────────────────── From c93c7efd2057083a21dd42a2357a900525217dd1 Mon Sep 17 00:00:00 2001 From: Michael Lam Date: Sun, 3 May 2026 19:44:02 -0700 Subject: [PATCH 2/2] docs: explain relative login script path --- api/routes.py | 1 + 1 file changed, 1 insertion(+) diff --git a/api/routes.py b/api/routes.py index 2ee9bff6..3c8b8086 100644 --- a/api/routes.py +++ b/api/routes.py @@ -1489,6 +1489,7 @@ button:hover{background:rgba(124,185,255,.25)}
+ """