final-review adjustments (plugins): keep RSA pins across probe/reconnect, vendor/ path in README, precise noVNC errors

MANIFEST.sha256

@@ -1,4 +1,4 @@
-ec106912cacb7af32006cacf01f2c1f6286d3a22855e27445db663a577a72871  bot-sections/plugin.js
+fc38ac67cd7783bbc701a229cbfa4b0be6b24b1ea74a286571249c7b252e431f  bot-sections/plugin.js
 abc2ed7fa6b6e317827b3ffe58d4d686c6169b4f9559c040730d64d8da901325  bubble-mode/plugin.js
 8437e28c1a15ded71ab48b89e047a1481a1319d6d5f9f6fb6eefc7d8c06758e0  computer-viewer/agent-plugin/orgo-computer/__init__.py
 caaaf1e751ddcba1d4b216ab18bd4077d18c4c9c8e85915d45bdbf746a5f95fc  computer-viewer/agent-plugin/orgo-computer/plugin.yaml
@@ -12,8 +12,8 @@ b8ac63b1b052a029b46ac9bda394215e7b4c2268506c9becd86eb49190a9faf6  computer-viewe
 2f0c0b90e7e5c9db0187d6693d0c0515c442c3ae17aaa590520afbb7e22650a6  computer-viewer/hiperf-linux.sh
 e3217fd93aa6c78bab712d376793a1f052740f1a6f8bfa309c63d5087e7a3632  computer-viewer/hiperf-mac.sh
 37502a54eb33646ff4eb76866f0ef2d1bb86c1ab87ce8bb85c6f7e249950447f  computer-viewer/hiperf-windows.ps1
-093aee168eb589a88f16a7402654c038b6ebca1aa36e10ea75df811c00cc34d3  computer-viewer/plugin.js
+9c4f76c9bb89e4cbf49b1c04fb02a823238c161712b88fff600aa36d94b514f3  computer-viewer/plugin.js
 08b527c943eb410ffa1a35d7d14c018d9eba22363150c3dc35a70327f1e88a28  computer-viewer/vendor/novnc-rfb.mjs
-0aceb385ca7c9f1d305b2b0d7daa0f177b3a778c60beb6e901a2f0ba5622654c  task-dock/plugin.js
+fdc05be2a112a2fece0b0db4998c13981b1fc5d15ebe372760e9f4d6c033d99d  task-dock/plugin.js
 f3e9050ac9fd66008cf775208ddcb28cc3dd44c80c268b2cbd53244f0a270429  texting-style/__init__.py
 edc6cd000b606b21499ad175bc44d1375d38cec5a1ace89c5198d85726e828f9  texting-style/plugin.yaml

bot-sections/plugin.js

@@ -112,7 +112,7 @@ function sectionLadder() {
   return ladder
 }
 
-const CSS = /* css */ `
+const PLUGIN_CSS = /* css */ `
 body.hermes-bot-sections [${LIST_ATTR}] {
   display: flex;
   flex-direction: column;
@@ -1271,7 +1271,7 @@ function injectStyle() {
   if (document.getElementById(STYLE_ID)) return
   const el = document.createElement('style')
   el.id = STYLE_ID
-  el.textContent = CSS
+  el.textContent = PLUGIN_CSS
   ;(document.head || document.documentElement).appendChild(el)
 }
 

bot-sections/plugin.test.mjs

@@ -7,8 +7,19 @@ const pluginPath = new URL('./plugin.js', import.meta.url)
 const SOURCE = fs.readFileSync(pluginPath, 'utf8')
 
 const HOOKS =
-  '\nglobalThis.__testHooks = { applyAssignData, applyAssignFileText, findBotsPaneRoots,' +
-  ' sectionForKey, sectionLadder, getOverrides: () => overrides, getCustomSections: () => customSections }\n'
+  '\nglobalThis.__testHooks = { applyAssignData, applyAssignFileText, findBotsPaneRoots, cssAttr,' +
+  ' HEADER_ATTR, sectionForKey, sectionLadder, getOverrides: () => overrides,' +
+  ' getCustomSections: () => customSections }\n'
+
+/** The parts of CSS.escape a section name can actually hit. */
+function cssEscape(value) {
+  return String(value).replace(/[^\w-]/g, ch => '\\' + ch)
+}
+
+/** Undo CSS string escaping, the way a selector parser would. */
+function cssUnescape(value) {
+  return String(value).replace(/\\(.)/g, '$1')
+}
 
 function classList(...names) {
   const set = new Set(names)
@@ -28,9 +39,15 @@ function el({ className = '', textContent = '', classes = [], parentElement = nu
 
 function load({ spans = [] } = {}) {
   const warnings = []
+  const escaped = []
   const context = {
     __host: { state: {} },
-    CSS: { escape: value => value },
+    CSS: {
+      escape: value => {
+        escaped.push(value)
+        return cssEscape(value)
+      }
+    },
     console: { warn: message => warnings.push(message) },
     document: {
       body: { classList: { add() {}, contains: () => false, remove() {} } },
@@ -50,7 +67,7 @@ function load({ spans = [] } = {}) {
     .concat(HOOKS)
 
   vm.runInNewContext(source, vm.createContext(context), { filename: pluginPath.pathname })
-  return { ...context.__testHooks, warnings }
+  return { ...context.__testHooks, warnings, escaped }
 }
 
 test('a valid bot-sections.json creates its sections and assigns its bots', () => {
@@ -164,3 +181,17 @@ test('findBotsPaneRoots() falls back to the grandparent when no flex column is f
   assert.equal(roots.length, 1)
   assert.equal(roots[0], grandparent)
 })
+
+test('a section name with a quote and a bracket still matches its own header', () => {
+  const kit = load()
+  const name = 'a"b]c'
+
+  const selector = `:scope > [${kit.HEADER_ATTR}="${kit.cssAttr(name)}"]`
+
+  // The module used to shadow the CSS Web API with its own stylesheet
+  // constant, so CSS.escape was never reached and the fallback regex ran.
+  assert.deepEqual(kit.escaped, [name])
+  const quoted = selector.match(/="(.*)"\]$/)
+  assert.ok(quoted, selector)
+  assert.equal(cssUnescape(quoted[1]), name)
+})

computer-viewer/README.md

@@ -86,7 +86,7 @@ viewed through the pane's noVNC connection.</em></p>
 
 ```
 plugin.js              the entire desktop plugin (single ESM file)
-vendor/novnc-rfb.mjs   bundled noVNC 1.7.0 RFB (installer copies it next to plugin.js)
+vendor/novnc-rfb.mjs   bundled noVNC 1.7.0 RFB (installer copies it to vendor/ on the target)
 connect-mac.sh         one-paste VNC bridge setup for a Mac
 connect-windows.ps1    same for Windows (TightVNC/UltraVNC + websockify + virtual display)
 connect-linux.sh       same for Linux (x11vnc / wayvnc + websockify)
@@ -373,23 +373,24 @@ be `computer-viewer`.
 
 ```bash
 # macOS / Linux (default profile)
-mkdir -p ~/.hermes/desktop-plugins/computer-viewer
+mkdir -p ~/.hermes/desktop-plugins/computer-viewer/vendor
 cp plugin.js ~/.hermes/desktop-plugins/computer-viewer/plugin.js
-cp vendor/novnc-rfb.mjs ~/.hermes/desktop-plugins/computer-viewer/novnc-rfb.mjs
+cp vendor/novnc-rfb.mjs ~/.hermes/desktop-plugins/computer-viewer/vendor/novnc-rfb.mjs
 cp connect-mac.sh connect-linux.sh hiperf-mac.sh hiperf-agent.py ~/.hermes/desktop-plugins/computer-viewer/
 chmod +x ~/.hermes/desktop-plugins/computer-viewer/connect-*.sh ~/.hermes/desktop-plugins/computer-viewer/hiperf-*.sh
 ```
 
-The kit installer places `novnc-rfb.mjs` next to `plugin.js`. The plugin
-verifies that file's SHA-256 before loading it, and only then falls back to
-the CDN.
+The kit installer places `novnc-rfb.mjs` in `computer-viewer/vendor/`. The
+plugin verifies that file's SHA-256 before loading it, and only then falls back
+to the CDN. A copy left next to `plugin.js` by an older release is still read
+(and hash-checked) if `vendor/` has none.
 
 Named Hermes profile:
 
 ```bash
-mkdir -p ~/.hermes/profiles/<name>/desktop-plugins/computer-viewer
+mkdir -p ~/.hermes/profiles/<name>/desktop-plugins/computer-viewer/vendor
 cp plugin.js ~/.hermes/profiles/<name>/desktop-plugins/computer-viewer/plugin.js
-cp vendor/novnc-rfb.mjs ~/.hermes/profiles/<name>/desktop-plugins/computer-viewer/novnc-rfb.mjs
+cp vendor/novnc-rfb.mjs ~/.hermes/profiles/<name>/desktop-plugins/computer-viewer/vendor/novnc-rfb.mjs
 cp connect-mac.sh connect-linux.sh hiperf-mac.sh hiperf-agent.py ~/.hermes/profiles/<name>/desktop-plugins/computer-viewer/
 chmod +x ~/.hermes/profiles/<name>/desktop-plugins/computer-viewer/connect-*.sh ~/.hermes/profiles/<name>/desktop-plugins/computer-viewer/hiperf-*.sh
 ```
@@ -398,7 +399,7 @@ Windows (typical):
 
 ```text
 %LOCALAPPDATA%\hermes\desktop-plugins\computer-viewer\plugin.js
-%LOCALAPPDATA%\hermes\desktop-plugins\computer-viewer\novnc-rfb.mjs
+%LOCALAPPDATA%\hermes\desktop-plugins\computer-viewer\vendor\novnc-rfb.mjs
 %LOCALAPPDATA%\hermes\desktop-plugins\computer-viewer\connect-windows.ps1
 ```
 
@@ -734,7 +735,7 @@ The address field picks a mode for you. These names only appear under
 
 | Mode | How it connects | When to use |
 |---|---|---|
-| **WebSocket** (default) | Loads the vendored noVNC 1.7.0 (`RFB`) the installer places next to `plugin.js`, after a SHA-256 check. Falls back to jsDelivr then `esm.sh` only if that file is **missing, truncated, or unreadable**; those CDN copies are unverified. A file that is present but **fails the hash check, or fails to import**, is treated as tampering: the pane stops with **Viewer file failed verification** and no CDN copy is loaded. | Full controls: scale, view-only, clipboard, Ctrl+Alt+Del, screenshot. |
+| **WebSocket** (default) | Loads the vendored noVNC 1.7.0 (`RFB`) the installer places in `computer-viewer/vendor/`, after a SHA-256 check. Falls back to jsDelivr then `esm.sh` only if that file is **missing, truncated, or unreadable**; those CDN copies are unverified. A file that is present but **fails the hash check, or fails to import**, is treated as tampering: the pane stops with **Viewer file failed verification** and no CDN copy is loaded. | Full controls: scale, view-only, clipboard, Ctrl+Alt+Del, screenshot. |
 | **Iframe** | `<iframe>` pointed at a hosted noVNC page. No CDN. Sandboxed; the page cannot read the clipboard unless **Allow this computer to use my clipboard** is on for that computer (Advanced, off by default). | CSP blocks the noVNC module, you're offline, or you already have `vnc.html`. |
 | **Session JSON** | `GET` a session document, then WebSocket/RFB as above. | Rotating desktops (paste the API URL or an API key). |
 
@@ -849,8 +850,8 @@ the other variant and the working choice is remembered per endpoint.
 
 - Passwords in `ctx.storage` are plain text on disk
   (`hermes.plugin.computer-viewer.*`).
-- Websocket mode loads the vendored noVNC next to `plugin.js` after a
-  SHA-256 check. Installs without that file (missing, truncated, unreadable)
+- Websocket mode loads the vendored noVNC from `computer-viewer/vendor/`
+  after a SHA-256 check. Installs without that file (missing, truncated, unreadable)
   fall back to the CDN (unverified); if the CDN is blocked too, use iframe
   mode. A hash mismatch does **not** fall back — it stops the connection.
 - The VNC server's RSA key is pinned on first connect (trust on first use).

computer-viewer/plugin.js

@@ -1399,6 +1399,24 @@ function rfbStatusDetail() {
   return why
 }
 
+// A load failure already recorded *why* it failed; that reason is the
+// actionable half, so it rides along in the error detail instead of being
+// dropped for a generic body. An import or crypto failure is not a hash
+// mismatch, and calling it one sends the operator hunting for tampering that
+// never happened.
+function novncErrorBody(code) {
+  const why = rfbStatusDetail()
+  if (code === 'vendored-tampered') {
+    if (engine.rfbLoad.reason === 'integrity') {
+      return `${ERRORS['vendored-tampered'].body} (hash mismatch)`
+    }
+    const body = 'The installed noVNC bundle could not be loaded. Re-run the kit installer.'
+    return why ? `${body} (${why})` : body
+  }
+  // Missing vendored copy *and* a blocked CDN: say both, not just the CDN.
+  return why ? `${ERRORS['cdn-blocked'].body} (${why})` : ERRORS['cdn-blocked'].body
+}
+
 function hexSha256(digest) {
   return Array.from(new Uint8Array(digest), b => b.toString(16).padStart(2, '0')).join('')
 }
@@ -1419,18 +1437,26 @@ function readFailureReason(error) {
   return /ENOENT|not found/i.test(message) ? 'missing' : 'read-failed'
 }
 
-function vendoredRfbPath(root) {
+function vendoredRfbPaths(root) {
   const sep = root.includes('\\') ? '\\' : '/'
   const base = root.endsWith('\\') || root.endsWith('/') ? root.slice(0, -1) : root
-  // Must match scripts/manifest-files.txt: computer-viewer/vendor/novnc-rfb.mjs
-  return base + sep + 'computer-viewer' + sep + 'vendor' + sep + 'novnc-rfb.mjs'
+  const dir = base + sep + 'computer-viewer' + sep
+  // First must match scripts/manifest-files.txt: computer-viewer/vendor/novnc-rfb.mjs.
+  // The legacy next-to-plugin.js location stays readable so a copy made by
+  // hand from an older README still loads — it is hash-checked either way, so
+  // the second path buys compatibility, not trust.
+  return [dir + 'vendor' + sep + 'novnc-rfb.mjs', dir + 'novnc-rfb.mjs']
 }
 
 function vendoredFailed(reason) {
   engine.rfbLoad = { source: null, reason }
   return null
 }
 
+// A vendored file that is present but wrong is an attack signal, not a
+// network hiccup: fail closed instead of silently reaching for the CDN.
+const VENDORED_TAMPERED_REASONS = { integrity: true, 'import-failed': true }
+
 async function loadVendoredRFB() {
   const bridge = globalThis.window && window.hermesDesktop
   // readPluginSource reads the whole file (16 MiB cap); readFileText is the
@@ -1446,28 +1472,41 @@ async function loadVendoredRFB() {
   }
   if (typeof root !== 'string' || !root) return vendoredFailed('no-bridge')
 
-  const filePath = vendoredRfbPath(root)
+  let reason = 'missing'
+  for (const filePath of vendoredRfbPaths(root)) {
+    const attempt = await loadVendoredRfbFrom(bridge, readSource, filePath)
+    if (attempt.ctor) return attempt.ctor
+    // A file that is present but wrong is an attack signal: stop here rather
+    // than let a second candidate paper over it.
+    if (VENDORED_TAMPERED_REASONS[attempt.reason]) return vendoredFailed(attempt.reason)
+    if (reason === 'missing') reason = attempt.reason
+  }
+  return vendoredFailed(reason)
+}
+
+// -> { ctor } once a candidate hashes and imports, else { reason }.
+async function loadVendoredRfbFrom(bridge, readSource, filePath) {
   let payload
   try {
     payload = await readSource.call(bridge, filePath)
   } catch (error) {
-    return vendoredFailed(readFailureReason(error))
+    return { ctor: null, reason: readFailureReason(error) }
   }
 
   const read = textFromReadFile(payload)
-  if (read.text == null) return vendoredFailed(read.reason || 'missing')
+  if (read.text == null) return { ctor: null, reason: read.reason || 'missing' }
   const source = read.text
 
   let hex
   try {
     const digest = await crypto.subtle.digest('SHA-256', new TextEncoder().encode(source))
     hex = hexSha256(digest)
   } catch {
-    return vendoredFailed('import-failed')
+    return { ctor: null, reason: 'import-failed' }
   }
   if (hex !== VENDORED_RFB_SHA256) {
     console.warn('[computer-viewer] vendored noVNC failed integrity check', filePath)
-    return vendoredFailed('integrity')
+    return { ctor: null, reason: 'integrity' }
   }
 
   const url = URL.createObjectURL(new Blob([source], { type: 'text/javascript' }))
@@ -1478,10 +1517,10 @@ async function loadVendoredRFB() {
     RFB = ctor
     engine.rfbLoad = { source: 'vendored', reason: null }
     console.info('[computer-viewer] noVNC source: vendored', filePath)
-    return RFB
+    return { ctor, reason: null }
   } catch (error) {
     console.warn('[computer-viewer] vendored noVNC failed to load', error)
-    return vendoredFailed('import-failed')
+    return { ctor: null, reason: 'import-failed' }
   } finally {
     try {
       URL.revokeObjectURL(url)
@@ -1491,10 +1530,6 @@ async function loadVendoredRFB() {
   }
 }
 
-// A vendored file that is present but wrong is an attack signal, not a
-// network hiccup: fail closed instead of silently reaching for the CDN.
-const VENDORED_TAMPERED_REASONS = { integrity: true, 'import-failed': true }
-
 async function loadRFB() {
   // Only a verified vendored constructor is cached. Anything else re-runs the
   // bridge attempt, so dropping a good file in place takes effect next connect.
@@ -1905,15 +1940,23 @@ function scheduleBackoff(endpoint, gen) {
     engine.backoffTimer = null
     if (!still(gen)) return
     if (!engine.paneVisible && !$ui.get().expanded) return
-    void connect(endpoint)
+    // Resolve at fire time: the copy captured when the timer was armed predates
+    // any pin written during the connection that just dropped.
+    void connect(liveEndpoint(endpoint))
   }, delay)
 }
 
-// The endpoint object the listeners captured can be a stale copy after a
-// persist; prefer the live one when the id still matches.
+// The endpoint object a listener, timer or retry captured is a snapshot: any
+// persist since then (the pinned server key, above all) replaced it. Every
+// merge that spreads an endpoint, and every reconnect, has to start from the
+// freshest copy or it writes the snapshot back and silently drops the pin.
 function liveEndpoint(endpoint) {
   const live = engine.endpoint
   if (live && endpoint && live.id === endpoint.id) return live
+  if (endpoint && endpoint.id) {
+    const stored = $settings.get().endpoints.find(item => item.id === endpoint.id)
+    if (stored) return stored
+  }
   return endpoint || {}
 }
 
@@ -1922,7 +1965,10 @@ function liveEndpoint(endpoint) {
 function fingerprintDecision(stored, presented) {
   const pin = String(stored || '')
   const seen = String(presented || '')
-  if (!seen) return { action: 'approve', detail: null }
+  // Nothing presented: either a non-RSA security type or a noVNC too old to
+  // hand us the key. Fine on a computer that was never pinned; on a pinned one
+  // it is the pin check being skipped, which is exactly what an attacker wants.
+  if (!seen) return { action: pin ? 'reject' : 'approve', detail: null }
   if (!pin) return { action: 'pin', detail: 'first contact: server key pinned' }
   if (pin === seen) return { action: 'approve', detail: null }
   return { action: 'reject', detail: null }
@@ -2098,7 +2144,7 @@ async function connect(endpoint) {
     if (!still(gen)) return
     if (iframeFallbackUrl) {
       const next = normalizeEndpoint({
-        ...endpoint,
+        ...liveEndpoint(endpoint),
         mode: 'iframe',
         iframeUrl: iframeFallbackUrl,
         probe: false
@@ -2118,7 +2164,7 @@ async function connect(endpoint) {
   } catch (error) {
     if (!still(gen)) return
     const code = error && error.code === 'vendored-tampered' ? 'vendored-tampered' : 'cdn-blocked'
-    setError(code, ERRORS[code].body)
+    setError(code, novncErrorBody(code))
     return
   }
   if (!still(gen)) return
@@ -2164,7 +2210,7 @@ async function connect(endpoint) {
         const fallback = iframeFallbackUrl
         iframeFallbackUrl = ''
         const next = normalizeEndpoint({
-          ...endpoint,
+          ...liveEndpoint(endpoint),
           mode: 'iframe',
           iframeUrl: fallback,
           probe: false
@@ -2190,9 +2236,12 @@ async function connect(endpoint) {
       persistWsProtocol(endpoint.id, variant)
       engine.reconnectAttempt = 0
       if (probing) {
+        // Merge onto the live copy: a server key pinned moments ago (the
+        // serververification listener fires before 'connect') lives there, and
+        // normalizeEndpoint drops any field the spread does not carry.
         persistEndpointFields(
           normalizeEndpoint({
-            ...endpoint,
+            ...liveEndpoint(endpoint),
             mode: 'websocket',
             wsUrl,
             probe: false
@@ -2274,7 +2323,7 @@ async function connect(endpoint) {
         const fallback = iframeFallbackUrl
         iframeFallbackUrl = ''
         const next = normalizeEndpoint({
-          ...endpoint,
+          ...liveEndpoint(endpoint),
           mode: 'iframe',
           iframeUrl: fallback,
           probe: false
@@ -3510,7 +3559,7 @@ function toggleHiperfEnabled(endpoint) {
     openHiperfEditor()
     return
   }
-  persistHiperfFields({ ...endpoint, hiperfEnabled: !endpoint.hiperfEnabled })
+  persistHiperfFields({ ...liveEndpoint(endpoint), hiperfEnabled: !endpoint.hiperfEnabled })
 }
 
 function hiperfSetupCommand(os) {