harden(#2622): DOM-bound plugin handlers + tab.path validation (Codex round-2)

Codex found two more once the config-guard bug was fixed (Opus concurred on #2):
1. panels.js _buildPluginCard built the Open button + enable toggle with inline
   onclick/onchange that interpolated tab.path / plugin.key into a JS-string-in-
   attribute context — HTML-escaping is insufficient there (quote breakout).
   Now rendered inert + bound via addEventListener with RAW closure values.
2. tab.path was unvalidated. Added _VALID_PLUGIN_TAB_PATH (^/[A-Za-z0-9._~/-]{0,255}$)
   in load_plugins() — absolute, no quotes/query/fragment/control chars.
Also Opus nit: deep-merge now coerces dashboard_plugins values to bool + str keys.
Regression tests added for both.

api/config.py

@@ -5043,7 +5043,9 @@ def save_settings(settings: dict) -> dict:
     if isinstance(_dashboard_plugins, dict):
         current_dash = current.get("dashboard_plugins", {})
         if isinstance(current_dash, dict):
-            current_dash.update(_dashboard_plugins)
+            # Coerce values to bool + keep only str keys so settings.json can't be
+            # polluted with non-bool/non-str junk from a crafted POST.
+            current_dash.update({k: bool(v) for k, v in _dashboard_plugins.items() if isinstance(k, str)})
             current["dashboard_plugins"] = current_dash
     for k, v in settings.items():
         # dashboard_plugins is deep-merged above (not a flat allowlisted scalar).

api/plugins.py

@@ -24,6 +24,11 @@ logger = logging.getLogger(__name__)
 # a settings key). Lowercase alnum + - / _, 1-64 chars, must start with a letter.
 _VALID_PLUGIN_NAME = re.compile(r"^[a-z][a-z0-9_-]{0,63}$")
 
+# Valid tab.path: a clean same-origin absolute path. Must start with '/', then
+# only safe path chars — no quotes, whitespace, control chars, query ('?') or
+# fragment ('#') so it can't break out of a JS-string nav arg or shadow routes.
+_VALID_PLUGIN_TAB_PATH = re.compile(r"^/[A-Za-z0-9._~/-]{0,255}$")
+
 # plugin_name -> manifest dict (as loaded from manifest.json)
 PLUGIN_MANIFESTS: dict[str, dict] = {}
 
@@ -67,6 +72,14 @@ def load_plugins() -> None:
         tab = manifest.get("tab", {})
         tab_path = tab.get("path", f"/{name}")
 
+        # Validate tab.path: it's a same-origin route the plugin page is served
+        # at AND a value passed into client-side navigation. Require a clean
+        # absolute path — no quotes/control chars/query/fragment — so a hostile
+        # manifest can't shadow odd routes or inject via the path.
+        if not _VALID_PLUGIN_TAB_PATH.match(str(tab_path)):
+            logger.warning("Skipping plugin %s with invalid tab.path %r (must match %s)", name, tab_path, _VALID_PLUGIN_TAB_PATH.pattern)
+            continue
+
         if name in PLUGIN_MANIFESTS:
             logger.warning("Duplicate plugin name skipped: %s (already loaded)", name)
             continue

static/panels.js

@@ -6471,14 +6471,18 @@ function _buildPluginCard(plugin){
 const enabled=plugin&&plugin.enabled!==false;
   const tab=plugin&&plugin.tab;
   const isDashboardPlugin=!!(tab&&tab.path);
+  // No inline onclick/onchange: an inline handler interpolates tab.path/key into
+  // a JS-string-in-attribute context where HTML-escaping is insufficient (a
+  // crafted value could break out). Render inert markup + bind listeners below
+  // with the raw closure values.
   const openBtn=enabled&&tab&&tab.path
-    ? `<a href="${esc(tab.path)}" class="plugin-open-btn" onclick="switchPluginPage(event,'${esc(tab.path)}','${esc(tab.label||plugin.name)}');return false;">${esc(tab.label||plugin.name||'Open')} \u2197</a>`
+    ? `<a href="${esc(tab.path)}" class="plugin-open-btn">${esc(tab.label||plugin.name||'Open')} \u2197</a>`
     : '';
   const toggleHtml=enabled&&isDashboardPlugin
     ? `<div class="plugin-card-footer-row">
          <span class="plugin-toggle-label">${t('plugins_enable_toggle')||'Enabled'}</span>
          <label class="plugin-toggle-switch">
-           <input type="checkbox" checked onchange="handlePluginEnableToggle('${esc(plugin.key)}',this.checked)">
+           <input type="checkbox" class="plugin-enable-toggle" checked>
            <span class="plugin-toggle-slider"></span>
          </label>
        </div>`
@@ -6486,7 +6490,7 @@ const enabled=plugin&&plugin.enabled!==false;
     ? `<div class="plugin-card-footer-row">
          <span class="plugin-toggle-label">${t('plugins_enable_toggle')||'Enable'}</span>
          <label class="plugin-toggle-switch">
-           <input type="checkbox" onchange="handlePluginEnableToggle('${esc(plugin.key)}',this.checked)">
+           <input type="checkbox" class="plugin-enable-toggle">
            <span class="plugin-toggle-slider"></span>
          </label>
        </div>`
@@ -6520,6 +6524,22 @@ const enabled=plugin&&plugin.enabled!==false;
       ${toggleHtml}
     </div>
   `;
+  // Bind handlers with the RAW closure values (not interpolated into inline JS),
+  // so a hostile tab.path/key can't break out of a JS-string attribute context.
+  if(tab&&tab.path){
+    const _openEl=card.querySelector('.plugin-open-btn');
+    if(_openEl){
+      const _p=tab.path, _l=tab.label||plugin.name;
+      _openEl.addEventListener('click', function(ev){ switchPluginPage(ev, _p, _l); });
+    }
+  }
+  if(isDashboardPlugin){
+    const _tog=card.querySelector('.plugin-enable-toggle');
+    if(_tog){
+      const _k=plugin.key;
+      _tog.addEventListener('change', function(){ handlePluginEnableToggle(_k, this.checked); });
+    }
+  }
   return card;
 }
 

tests/test_plugins_panel.py

@@ -418,6 +418,29 @@ class TestPluginNameValidation:
         assert not plugins._VALID_PLUGIN_NAME.match("")
         assert not plugins._VALID_PLUGIN_NAME.match("1leading-digit")
 
+    def test_invalid_tab_path_rejected(self):
+        import api.plugins as plugins
+        assert plugins._VALID_PLUGIN_TAB_PATH.match("/demo-dashboard")
+        assert plugins._VALID_PLUGIN_TAB_PATH.match("/nic/branch_sync")
+        # Must be absolute, no quotes/JS-breakout/query/fragment/control chars.
+        assert not plugins._VALID_PLUGIN_TAB_PATH.match("demo")          # not absolute
+        assert not plugins._VALID_PLUGIN_TAB_PATH.match("/x');alert(1)//")  # quote breakout
+        assert not plugins._VALID_PLUGIN_TAB_PATH.match("/x?y=1")         # query
+        assert not plugins._VALID_PLUGIN_TAB_PATH.match("/x#frag")        # fragment
+        assert not plugins._VALID_PLUGIN_TAB_PATH.match("/x y")           # whitespace
+
+    def test_open_button_and_toggle_use_no_inline_handlers(self):
+        # tab.path / plugin.key must not be interpolated into inline onclick/
+        # onchange JS (HTML-escaping is insufficient for a JS-string context).
+        # They're bound via addEventListener with raw closure values instead.
+        js = read("static/panels.js")
+        start = js.find("function _buildPluginCard")
+        seg = js[start:js.find("return card;", start)]
+        assert "onclick=\"switchPluginPage" not in seg
+        assert "onchange=\"handlePluginEnableToggle" not in seg
+        assert "addEventListener('click'" in seg
+        assert "addEventListener('change'" in seg
+
 
 class TestPluginCollisionDetection:
     """Tests for plugin name and tab.path collision detection."""