From 176720b55a260bab03a1286b42b45f550c1a5828 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Thu, 2 Jul 2026 18:27:27 +0000 Subject: [PATCH] fix(profiles): guard non-string cached model in dropdown render + purge from cache predicate (Codex gate finding on #5412) A poisoned cache row {name:'x', model:{}} passed the name-only cache predicate but threw 'p.model.split is not a function' synchronously on profile-chip click, bricking profile switching until localStorage was cleared. Guard both render sites (typeof p.model==='string') and reject unsafe model types in _profileDropdownDataCacheUsable; regression covers {model:{}} and {model:123}. --- static/panels.js | 15 ++++++++++++--- tests/test_profile_dropdown_fast_open.py | 14 +++++++++++++- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/static/panels.js b/static/panels.js index d426a663..bf083460 100644 --- a/static/panels.js +++ b/static/panels.js @@ -5938,7 +5938,16 @@ function _profileDropdownDataCacheUsable(data){ data && Array.isArray(data.profiles) && data.profiles.length && - data.profiles.every(p=>p&&typeof p.name==='string') + data.profiles.every(p=> + p && + typeof p.name==='string' && + // Renderer-read fields must be safe types: renderProfileDropdown / + // renderProfilesPanel call p.model.split('/') guarded only by truthiness, + // so a poisoned cached row like {name:"x", model:{}} would pass a + // name-only check yet throw synchronously on dropdown open (bricking + // profile switching). Reject rows whose model is a non-string truthy value. + (p.model==null || typeof p.model==='string') + ) ); } @@ -6112,7 +6121,7 @@ async function loadProfilesPanel() { card.className = 'profile-card'; card.dataset.name = p.name; const meta = []; - if (p.model) meta.push(p.model.split('/').pop()); + if (typeof p.model === 'string' && p.model) meta.push(p.model.split('/').pop()); if (p.provider) meta.push(p.provider); if (p.total_skills && p.total_skills > 0) meta.push(t('profile_skill_count', p.total_skills).replace(String(p.total_skills), `${p.enabled_skills} / ${p.total_skills}`)); const gwDot = p.gateway_running @@ -6291,7 +6300,7 @@ function renderProfileDropdown(data) { const opt = document.createElement('div'); opt.className = 'profile-opt' + (p.name === active ? ' active' : ''); const meta = []; - if (p.model) meta.push(p.model.split('/').pop()); + if (typeof p.model === 'string' && p.model) meta.push(p.model.split('/').pop()); if (p.total_skills && p.total_skills > 0) meta.push(t('profile_skill_count', p.total_skills).replace(String(p.total_skills), `${p.enabled_skills} / ${p.total_skills}`)); const gwDot = ``; const checkmark = p.name === active ? ' ' : ''; diff --git a/tests/test_profile_dropdown_fast_open.py b/tests/test_profile_dropdown_fast_open.py index 751cf1ed..34b3bf64 100644 --- a/tests/test_profile_dropdown_fast_open.py +++ b/tests/test_profile_dropdown_fast_open.py @@ -45,7 +45,11 @@ def test_profile_dropdown_opens_shell_before_network_fetch(): def test_profile_dropdown_uses_shared_fetch_promise_and_validated_local_storage_cache(): data_cache_body = _function_body(PANELS_JS, "function _profileDropdownDataCacheUsable(data){") assert "Array.isArray(data.profiles)" in data_cache_body - assert "data.profiles.every(p=>p&&typeof p.name==='string')" in data_cache_body + assert "data.profiles.every(" in data_cache_body + assert "typeof p.name==='string'" in data_cache_body + # #5412 gate: the predicate must also reject rows whose renderer-read `model` + # field is a non-string truthy value (poison {name,model:{}} bricked the dropdown). + assert "typeof p.model==='string'" in data_cache_body cache_body = _function_body(PANELS_JS, "function _profileDropdownCacheUsable(data){") assert "_profileDropdownDataCacheUsable(data)" in cache_body @@ -259,6 +263,14 @@ def test_poisoned_profile_cache_opens_then_switches_after_fresh_refresh(): (async () => {{ await runPoisonedCase([null]); await runPoisonedCase([{{}}]); + // Regression (#5412 gate): a row that PASSES the name check but has a + // non-string `model` used to slip the cache predicate and throw + // `p.model.split is not a function` synchronously on dropdown open, + // bricking profile switching until localStorage was cleared. The + // predicate must now reject it (usable===false) AND the renderer must + // not throw even if such a row reaches it. + await runPoisonedCase([{{ name: 'default', is_default: true, model: {{}} }}, {{ name: 'other', visible: true }}]); + await runPoisonedCase([{{ name: 'default', is_default: true, model: 123 }}, {{ name: 'other', visible: true }}]); await runSingleProfilePreservesSharedCache(); await runFreshSingleProfileReplacesStaleMultiCache(); }})().catch((err) => {{ console.error(err && err.stack || err); process.exit(1); }});