From dfe64313dcca5c5e07686c927f08da081f2ec2ac Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Sun, 28 Jun 2026 03:04:39 +0000 Subject: [PATCH] theme-registration: preserve pending ext skin across reload + expand accent token allowlist Found via live browser testing of the first consumer (e-ink skin): - An extension-registered skin selected by the user was lost on reload: both the pre-paint inline script in index.html AND the boot-time appearance sync normalized the persisted skin against the (extension-less) known-skins list and clobbered localStorage back to 'default' before the extension's registerHermesSkin() could run. Both sites now PRESERVE an unknown non-default persisted skin (a likely pending extension skin), so it survives the reload and the extension re-applies it on load. The server push is skipped for a still-pending ext skin. - Expanded the token allowlist to the full accent family (--accent-hover/-text/-bg/-bg-strong/-rgb + --surface-subtle) so a skin can fully recolor accent-driven chrome (active rail, chips, profile labels), not just --accent. Value regex now also accepts a bare RGB triple for --accent-rgb. Regression tests added for both preservation sites. Full suite: 10876 passed. --- docs/EXTENSIONS.md | 19 +++++++++------- static/boot.js | 25 ++++++++++++++++------ static/index.html | 2 +- tests/test_extension_theme_registration.py | 25 ++++++++++++++++++++++ 4 files changed, 55 insertions(+), 16 deletions(-) diff --git a/docs/EXTENSIONS.md b/docs/EXTENSIONS.md index a53fcf38..59d0de53 100644 --- a/docs/EXTENSIONS.md +++ b/docs/EXTENSIONS.md @@ -351,15 +351,18 @@ written into CSS, every value is sanitized in core, once, so every theme extension inherits the guard: - **Allowed token names** (anything else is dropped): `--bg`, `--surface`, - `--surface2`, `--text`, `--text2`, `--muted`, `--accent`, `--accent2`, - `--accent3`, `--accent-contrast`, `--border`, `--border2`, `--hover-bg`, - `--code-bg`, `--code-text`, `--sidebar`, `--sidebar-text`, `--user-bubble`, - `--assistant-bubble`, `--success`, `--warning`, `--danger`, `--info`, `--link`. + `--surface2`, `--surface-subtle`, `--text`, `--text2`, `--muted`, `--accent`, + `--accent2`, `--accent3`, `--accent-contrast`, `--accent-hover`, + `--accent-text`, `--accent-bg`, `--accent-bg-strong`, `--accent-rgb`, + `--border`, `--border2`, `--hover-bg`, `--code-bg`, `--code-text`, + `--sidebar`, `--sidebar-text`, `--user-bubble`, `--assistant-bubble`, + `--success`, `--warning`, `--danger`, `--info`, `--link`. - **Allowed value shapes** (anything else is dropped): hex colors, `rgb()` / - `rgba()`, `hsl()` / `hsla()`, CSS color keywords, and simple - numeric-with-unit values (`px`/`em`/`rem`/`%`). Values containing `url()`, - `expression()`, semicolons, braces, or other CSS-injection vectors are - rejected. + `rgba()`, `hsl()` / `hsla()`, CSS color keywords, simple numeric-with-unit + values (`px`/`em`/`rem`/`%`), and a bare RGB triple (e.g. `0, 0, 0` for + `--accent-rgb`, which the app consumes inside `rgba(...)`). Values containing + `url()`, `expression()`, semicolons, braces, or other CSS-injection vectors + are rejected. - **Reserved keys are protected** — an extension cannot overwrite a built-in skin key (e.g. `default`, `ares`, `graphite`). - A descriptor with no valid tokens after sanitization is rejected entirely. diff --git a/static/boot.js b/static/boot.js index 03c67e99..b4f065e0 100644 --- a/static/boot.js +++ b/static/boot.js @@ -2013,15 +2013,17 @@ const _RESERVED_SKIN_KEYS=new Set((_SKINS||[]).map(s=>(s.value||s.name).toLowerC // CSS custom-property names a skin is allowed to set. Mirrors the documented // design-token contract; anything outside this set is dropped. const _ALLOWED_SKIN_TOKENS=new Set([ - '--bg','--surface','--surface2','--text','--text2','--muted', - '--accent','--accent2','--accent3','--accent-contrast', + '--bg','--surface','--surface2','--surface-subtle','--text','--text2','--muted', + '--accent','--accent2','--accent3','--accent-contrast','--accent-hover', + '--accent-text','--accent-bg','--accent-bg-strong','--accent-rgb', '--border','--border2','--hover-bg','--code-bg','--code-text', '--sidebar','--sidebar-text','--user-bubble','--assistant-bubble', '--success','--warning','--danger','--info','--link' ]); -// Accept only safe color / simple numeric-with-unit values. Rejects anything +// Accept only safe color / simple numeric-with-unit values, OR a bare RGB triple +// (e.g. "0, 0, 0" for --accent-rgb, consumed inside rgba(...)). Rejects anything // with url(), expression(), semicolons, braces, or other CSS-injection vectors. -const _SAFE_SKIN_VALUE_RE=/^(#(?:[0-9a-fA-F]{3,8})|rg(?:b|ba)\(\s*[0-9.,%\s/]+\)|hsl(?:a)?\(\s*[0-9.,%\s/deg]+\)|[a-zA-Z]{3,20}|[0-9.]+(?:px|em|rem|%)?)$/; +const _SAFE_SKIN_VALUE_RE=/^(#(?:[0-9a-fA-F]{3,8})|rg(?:b|ba)\(\s*[0-9.,%\s/]+\)|hsl(?:a)?\(\s*[0-9.,%\s/deg]+\)|[0-9]{1,3}\s*,\s*[0-9]{1,3}\s*,\s*[0-9]{1,3}|[a-zA-Z]{3,20}|[0-9.]+(?:px|em|rem|%)?)$/; function _sanitizeSkinTokens(tokens){ const out={}; @@ -2299,17 +2301,26 @@ window._applyTitlebarProfileVisibility=_applyTitlebarProfileVisibility; const lsTheme=(localStorage.getItem('hermes-theme')||'').trim().toLowerCase(); const lsSkin=(localStorage.getItem('hermes-skin')||'').trim().toLowerCase(); const lsAppearance=_normalizeAppearance(lsTheme||null,lsSkin||null); + // An unknown non-default persisted skin is most likely an extension-provided + // skin (registerHermesSkin) whose extension script hasn't registered it yet + // at this point in boot. Preserve it verbatim instead of normalizing it away + // to 'default' — the extension's registerHermesSkin() will inject the CSS and + // re-apply it once it loads. Without this, the boot sync would clobber the + // saved choice before the extension runs. + const lsSkinIsPendingExt=!!lsSkin&&lsSkin!=='default'&&!_VALID_SKINS.has(lsSkin)&&!_LEGACY_THEME_MAP[lsSkin]; const lsHasExplicitSkin=lsSkin&&lsSkin!=='default'; const lsHasExplicitTheme=lsTheme&&['system','light','dark'].includes(lsTheme); const theme=lsHasExplicitTheme?lsAppearance.theme:srvAppearance.theme; - const skin=lsHasExplicitSkin?lsAppearance.skin:srvAppearance.skin; + const skin=lsHasExplicitSkin?(lsSkinIsPendingExt?lsSkin:lsAppearance.skin):srvAppearance.skin; localStorage.setItem('hermes-theme',theme); _applyTheme(theme); localStorage.setItem('hermes-skin',skin); _applySkin(skin); // Reconcile: if localStorage and server disagree, push localStorage - // values to the server so the next refresh won't revert. - if((lsHasExplicitTheme||lsHasExplicitSkin)&&(theme!==srvAppearance.theme||skin!==srvAppearance.skin)){ + // values to the server so the next refresh won't revert. Skip the push for a + // still-pending extension skin (don't persist it server-side until it's a + // confirmed-registered skin — avoids writing a skin the server can't validate). + if((lsHasExplicitTheme||lsHasExplicitSkin)&&!lsSkinIsPendingExt&&(theme!==srvAppearance.theme||skin!==srvAppearance.skin)){ try{ api('/api/settings',{method:'POST',body:JSON.stringify({theme,skin})}); }catch(_){} diff --git a/static/index.html b/static/index.html index e63669b5..8287fc6a 100644 --- a/static/index.html +++ b/static/index.html @@ -17,7 +17,7 @@ - + diff --git a/tests/test_extension_theme_registration.py b/tests/test_extension_theme_registration.py index 64658a1f..ad92b25e 100644 --- a/tests/test_extension_theme_registration.py +++ b/tests/test_extension_theme_registration.py @@ -45,6 +45,31 @@ def test_reserved_core_skins_are_guarded(): ) +def test_pending_extension_skin_is_preserved_across_boot(): + """A persisted skin that isn't a known core skin (i.e. an extension skin + not yet registered at boot) must NOT be clobbered to 'default' by the + boot-time appearance sync, or it won't survive a reload.""" + assert "lsSkinIsPendingExt" in BOOT_JS, ( + "boot sync must detect a pending (unregistered) extension skin" + ) + # the boot sync must keep the raw ls value for a pending ext skin + assert "lsSkinIsPendingExt?lsSkin:lsAppearance.skin" in BOOT_JS, ( + "boot sync must preserve the raw pending-ext skin instead of normalizing it away" + ) + + +INDEX_HTML = (REPO / "static" / "index.html").read_text(encoding="utf-8") + + +def test_prepaint_inline_script_preserves_unknown_skin(): + """The pre-paint inline script in index.html must also preserve an unknown + persisted skin (a likely extension skin) rather than resetting localStorage + to 'default' before boot.js / the extension runs.""" + assert "pendingExt" in INDEX_HTML, ( + "pre-paint script must detect a pending extension skin and keep it" + ) + + # ── Layer 2: behavioral (Node harness drives the real functions) ───────────── _HARNESS = r"""