fix: reject cmd.exe caret and control chars; fix subprocess lifecycle bugs in runIngest

.obsidian/plugins/webinar-dash/main.js

@@ -158,9 +158,17 @@ function reconcileConcepts(rows, conceptNames) {
 
 // `shell: true` is required on Windows to resolve `claude.cmd`, which puts the
 // filename into a shell string. Reject anything cmd.exe or a POSIX shell would
-// interpret. Apostrophes and cyrillic are safe inside double quotes and are
+// interpret, plus every control character.
-// present in real filenames, so they stay allowed.
+//
-const UNSAFE_CHARS = /["`$&|;<>%\r\n]/;
+// `^` is cmd.exe's escape character and belongs to the same class Node escapes
+// in its CVE-2024-27980 mitigation for exactly this spawn-through-.cmd shape.
+// Control characters are rejected because a NUL byte makes spawn() throw
+// synchronously, which would otherwise strand the row mid-run.
+//
+// Apostrophes, spaces, cyrillic, em dashes and `!` stay allowed — they are safe
+// inside double quotes and appear in real filenames in this vault. (`!` would
+// matter only under `setlocal enabledelayedexpansion`, which is not in play.)
+const UNSAFE_CHARS = /["`$&|;<>%^\u0000-\u001f\u007f]/;
 
 function isSafeFilename(name) {
   if (typeof name !== "string" || name.length === 0) return false;
@@ -349,6 +357,7 @@ function renderRightPane(container, parsed, reconciled) {
 
 class WebinarDashPlugin extends PluginBase {
   async onload() {
+    this.running = new Set();
     this.registerMarkdownCodeBlockProcessor("webinar-dash", async (source, el, ctx) => {
       const cfg = parseConfig(source);
       const root = el.createDiv({ cls: "webinar-dash" });
@@ -411,11 +420,19 @@ class WebinarDashPlugin extends PluginBase {
   }
 
   runIngest(file, rowEl) {
-    if (rowEl.dataset.wdRunning === "1") return;
-
     const Notice = OB ? OB.Notice : null;
     const notify = (msg) => { if (Notice) new Notice(msg); };
 
+    // In-flight state is keyed on the file, not the row. Obsidian rebuilds the
+    // row on every re-render, so a row-scoped guard would let a re-render hand
+    // out a fresh row whose guard is unset — and a second click would then run
+    // a second unsupervised agent against the same file, concurrently writing
+    // the same wiki pages as the first.
+    if (this.running.has(file.path)) {
+      notify(`Already ingesting ${file.name}.`);
+      return;
+    }
+
     if (!isSafeFilename(file.name)) {
       rowEl.createDiv({
         cls: "wd-output",
@@ -438,54 +455,76 @@ class WebinarDashPlugin extends PluginBase {
       return;
     }
 
-    rowEl.dataset.wdRunning = "1";
+    // A retry reuses the same row. Clear the previous run's status and output
+    // so they are replaced rather than stacked on top of each other.
+    rowEl.querySelectorAll(".wd-status, .wd-output").forEach((el) => el.remove());
+
     const button = rowEl.querySelector("button");
     if (button) button.disabled = true;
+    this.running.add(file.path);
 
     const status = rowEl.createSpan({ cls: "wd-status wd-status-running", text: "running 0s" });
+    const output = rowEl.createDiv({ cls: "wd-output", text: "" });
     const started = Date.now();
     const timer = window.setInterval(() => {
       status.setText(`running ${Math.round((Date.now() - started) / 1000)}s`);
     }, 1000);
+    this.registerInterval(timer);
 
-    const output = rowEl.createDiv({ cls: "wd-output", text: "" });
+    // Bounded as it accumulates, not only when displayed, so a long or noisy
+    // run cannot grow this string without limit.
     let buffered = "";
-    const append = (chunk) => {
+    const append = (text) => {
-      buffered += chunk.toString();
+      buffered = (buffered + text).slice(-8000);
-      output.setText(buffered.slice(-4000));
+      output.setText(buffered);
       output.scrollTop = output.scrollHeight;
     };
 
-    const child = spawn("claude", ["-p", `ingest "${file.name}"`], {
+    // Single exit path: every way this run can end clears the timer, releases
-      cwd: base,
+    // the file, and re-enables the button.
-      shell: true,
+    const finish = (cls, label) => {
-    });
+      window.clearInterval(timer);
+      this.running.delete(file.path);
+      status.className = `wd-status ${cls}`;
+      status.setText(label);
+      if (button) button.disabled = false;
+    };
 
+    let child;
+    try {
+      child = spawn("claude", ["-p", `ingest "${file.name}"`], {
+        cwd: base,
+        shell: true,
+      });
+    } catch (err) {
+      // spawn() throws synchronously for some argument shapes. Without this the
+      // timer would run forever and the row would stay disabled until reload.
+      append(`\nCould not start claude: ${err.message}`);
+      finish("wd-status-failed", "failed");
+      return;
+    }
+
+    // Decode as UTF-8 across chunk boundaries. Raw Buffer chunks split wherever
+    // the OS buffer ends, and this vault's output is full of Cyrillic and em
+    // dashes that would otherwise decode as replacement characters.
+    child.stdout.setEncoding("utf8");
+    child.stderr.setEncoding("utf8");
     child.stdout.on("data", append);
     child.stderr.on("data", append);
 
     child.on("error", (err) => {
-      window.clearInterval(timer);
-      status.className = "wd-status wd-status-failed";
-      status.setText("failed");
       append(`\nCould not start claude: ${err.message}\nIs it on PATH?`);
-      rowEl.dataset.wdRunning = "0";
+      finish("wd-status-failed", "failed");
-      if (button) button.disabled = false;
     });
 
     child.on("close", (code) => {
-      window.clearInterval(timer);
       const secs = Math.round((Date.now() - started) / 1000);
       if (code === 0) {
-        status.className = "wd-status wd-status-done";
+        finish("wd-status-done", `done in ${secs}s`);
-        status.setText(`done in ${secs}s`);
         notify(`Ingested ${file.name}. Reopen the dashboard to refresh.`);
       } else {
-        status.className = "wd-status wd-status-failed";
+        finish("wd-status-failed", `failed - exit ${code}`);
-        status.setText(`failed - exit ${code}`);
       }
-      rowEl.dataset.wdRunning = "0";
-      if (button) button.disabled = false;
     });
   }
 }

.obsidian/plugins/webinar-dash/test/safety.test.js

@@ -26,6 +26,14 @@ test("isSafeFilename rejects shell metacharacters", () => {
   }
 });
 
+test("isSafeFilename rejects the cmd.exe escape character and control characters", () => {
+  // `^` escapes the next character in cmd.exe, so it can defuse the closing
+  // quote. NUL additionally makes spawn() throw synchronously.
+  for (const bad of ["a^b.md", "a\u0000b.md", "a\u001bb.md", "a\u007fb.md"]) {
+    assert.equal(isSafeFilename(bad), false, `should reject: ${JSON.stringify(bad)}`);
+  }
+});
+
 test("isSafeFilename rejects path traversal", () => {
   assert.equal(isSafeFilename("../secrets.md"), false);
   assert.equal(isSafeFilename("a/../../b.md"), false);