review: plugin follow-ups from bot review of #266

src-tauri/src/commands/plugin.rs

@@ -530,16 +530,29 @@ pub fn plugins_root(state: State<'_, AppState>) -> AppResult<String> {
 /// Reject encoded input that could decode beyond MAX_ZIP_BYTES — decoding
 /// first would allocate the whole payload in memory. Call before decode.
 pub fn ensure_zip_b64_within_cap(encoded: &str) -> AppResult<()> {
-    // Decoded upper bound: 3 bytes per full 4-char group plus the 1-2 bytes
-    // of a trailing partial group (padding decodes to nothing). Exact, so it
-    // agrees with install_impl's check on the decoded bytes.
-    let len = encoded.trim().len();
-    let decoded = len / 4 * 3
-        + match len % 4 {
-            2 => 1,
-            3 => 2,
-            _ => 0,
-        };
+    // Exact decoded length: 3 bytes per full 4-char group, 1-2 bytes for an
+    // unpadded tail group, minus trailing '=' (they decode to nothing), so
+    // the estimate agrees with install_impl's check on the decoded bytes.
+    // Padding matters: 10 MiB % 3 == 1, so a cap-sized payload ends in "=="
+    // and ignoring that overcounts by 2 bytes and rejects a legal zip.
+    let s = encoded.trim();
+    let len = s.len();
+    let mut decoded = len / 4 * 3;
+    match len % 4 {
+        2 => decoded += 1,
+        3 => decoded += 2,
+        _ => {}
+    }
+    // At most the final group pads; saturate so garbage like "====" cannot
+    // underflow (the caller's decode rejects it anyway).
+    let pad = s
+        .as_bytes()
+        .iter()
+        .rev()
+        .take(2)
+        .take_while(|&b| *b == b'=')
+        .count();
+    decoded = decoded.saturating_sub(pad);
     if decoded > MAX_ZIP_BYTES {
         return Err(AppError::config(
             "plugin_zip_invalid",
@@ -676,6 +689,22 @@ mod tests {
         zip::ZipArchive::new(std::io::Cursor::new(bytes)).unwrap()
     }
 
+    #[test]
+    fn zip_b64_cap_accepts_padded_input_at_exactly_the_cap() {
+        use base64::{engine::general_purpose::STANDARD, Engine};
+        // 10 MiB % 3 == 1, so a cap-sized payload encodes with "==" padding;
+        // ignoring the padding overcounted by 2 and rejected a legal zip.
+        let encoded = STANDARD.encode(vec![b'x'; MAX_ZIP_BYTES]);
+        assert!(ensure_zip_b64_within_cap(&encoded).is_ok());
+    }
+
+    #[test]
+    fn zip_b64_cap_rejects_one_byte_over() {
+        use base64::{engine::general_purpose::STANDARD, Engine};
+        let encoded = STANDARD.encode(vec![b'x'; MAX_ZIP_BYTES + 1]);
+        assert!(ensure_zip_b64_within_cap(&encoded).is_err());
+    }
+
     #[cfg(desktop)]
     #[tokio::test]
     async fn local_exec_runs_command_and_captures_output() {

src-tauri/src/server.rs

@@ -336,14 +336,21 @@ fn dispatch(
         "install_plugin" => {
             use base64::{engine::general_purpose::STANDARD, Engine};
             let b64: String = arg(&args, "base64Zip")?;
+            // Same region-button contract as the Tauri command — passing None
+            // here would silently skip the area mismatch validation.
+            let area = optional_string_arg(&args, "area")?;
             crate::commands::plugin::ensure_zip_b64_within_cap(&b64).map_err(err_value)?;
             let bytes = STANDARD.decode(b64.trim()).map_err(|e| {
                 err_value(AppError::config(
                     "crypto_base64_decode_failed",
                     json!({ "err": e.to_string() }),
                 ))
             })?;
-            ok(crate::commands::plugin::install_impl(state, &bytes, None))
+            ok(crate::commands::plugin::install_impl(
+                state,
+                &bytes,
+                area.as_deref(),
+            ))
         }
         "list_plugins" => ok(crate::db::plugin::list(&state.db)),
         "set_plugin_enabled" => {

src/lib/components/AppShell.svelte

@@ -439,8 +439,10 @@
             .filter((tab) =>
                 open(tab.id)
                 && (tab.type === "ssh" || tab.type === "local"))
-            // sessionId 可为空串：断线时 iframe 保活（图表不丢），exec 报
-            // plugin_no_exec 由插件显示断连态；重连后 sessionId 恢复。
+            // sessionId may be "" on a disconnected tab: the tab stays listed
+            // so its iframes stay mounted (charts survive the drop; exec would
+            // answer plugin_no_exec), but pluginAreaActive hides the region
+            // itself until the session comes back.
             .map((tab) => ({tabId: tab.id, sessionId: app.sessionIdForTab(tab.id) ?? ""}));
     }
     let pluginSideTabs = $derived(pluginTabsFor(plugins.isSideOpen));

src/lib/plugins/PluginManager.svelte

@@ -644,8 +644,12 @@
     opacity: 0;
     transition: opacity 0.12s;
   }
+  /* focus-within: the hover-revealed enable/uninstall controls must also be
+     visible when a keyboard user tabs into them. */
   .block:hover .cell-hover,
-  .seg:hover .cell-hover {
+  .block:focus-within .cell-hover,
+  .seg:hover .cell-hover,
+  .seg:focus-within .cell-hover {
     opacity: 1;
   }
   .cell-label {

src/lib/plugins/bridge.test.ts

@@ -54,6 +54,12 @@ describe("isPluginRequest", () => {
     expect(isPluginRequest(req({payload: {command: "ls", timeoutMs: "3000"}}))).toBe(false);
   });
 
+  it("rejects non-finite, fractional and non-positive timeoutMs", () => {
+    for (const timeoutMs of [Number.NaN, Number.POSITIVE_INFINITY, 0, -3000, null, 3000.5])
+      expect(isPluginRequest(req({payload: {command: "ls", timeoutMs}}))).toBe(false);
+    expect(isPluginRequest(req({payload: {command: "ls", timeoutMs: 3000}}))).toBe(true);
+  });
+
   it("rejects non-object payloads", () => {
     expect(isPluginRequest(req({payload: null}))).toBe(false);
     expect(isPluginRequest("exec")).toBe(false);

src/lib/plugins/bridge.ts

@@ -68,7 +68,14 @@ function isExecPayload(p: unknown): boolean {
   const req = p as Record<string, unknown>;
   if (typeof req.command !== "string" || req.command.length === 0) return false;
   if (req.command.length > MAX_COMMAND_LENGTH) return false;
-  if (req.timeoutMs !== undefined && typeof req.timeoutMs !== "number") return false;
+  // A safe positive integer: the backend reads u64 on both transports, so a
+  // fractional value would error on Tauri but silently become the default
+  // timeout on headless — one contract at the bridge. typeof narrows the
+  // unknown so the comparison type-checks; isSafeInteger never throws or
+  // coerces, so symbols/bigints fail the typeof check instead.
+  const timeoutMs = req.timeoutMs;
+  if (timeoutMs !== undefined && (typeof timeoutMs !== "number" || !Number.isSafeInteger(timeoutMs) || timeoutMs <= 0))
+    return false;
   return true;
 }