From 3bc1afa9f1ff0cf79883313e18adbe3f4d9ef805 Mon Sep 17 00:00:00 2001 From: meels Date: Tue, 28 Jul 2026 14:55:04 +0200 Subject: [PATCH] Fix ingest subprocess dropping the filename argument (C1) spawn(..., { shell: true }) made Node concatenate argv without quoting; cmd.exe re-tokenized it and the ingest prompt arrived as a bare "ingest" with the filename silently discarded, while the run still reported success. claude.exe is a real executable here, so no shell is needed: drop shell: true and pass argv directly. isSafeFilename is unchanged (still correct defence in depth) but its comment now reflects that the spawn path carries no injection surface today. --- .obsidian/plugins/webinar-dash/main.js | 30 +++++++++++++------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/.obsidian/plugins/webinar-dash/main.js b/.obsidian/plugins/webinar-dash/main.js index c40afe8..1a87929 100644 --- a/.obsidian/plugins/webinar-dash/main.js +++ b/.obsidian/plugins/webinar-dash/main.js @@ -156,18 +156,14 @@ 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, plus every control character. +// Defence in depth. The spawn path no longer uses a shell, so nothing here is +// load-bearing against injection today — but the guard costs nothing and would +// still hold if the single-string `shell: true` form is ever needed for a .cmd +// shim. `^` is cmd.exe's escape character; control characters are rejected +// because a NUL byte makes spawn() throw synchronously. // -// `^` 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.) +// Apostrophes, spaces, cyrillic, em dashes and `!` stay allowed — they appear in +// real filenames in this vault. const UNSAFE_CHARS = /["`$&|;<>%^\u0000-\u001f\u007f]/; function isSafeFilename(name) { @@ -492,10 +488,14 @@ class WebinarDashPlugin extends PluginBase { let child; try { - child = spawn("claude", ["-p", `ingest "${file.name}"`], { - cwd: base, - shell: true, - }); + // `claude` resolves to a real .exe here, so libuv finds it via PATH and + // PATHEXT with no shell involved. Do NOT add `shell: true`: with a shell, + // Node concatenates argv without quoting and cmd.exe re-tokenizes, so the + // prompt arrives as two arguments and the filename is silently discarded. + // Verified: shell:true yields ["-p","ingest","Webinar script.md"], where the + // program sees only "ingest". If a .cmd shim ever needs supporting, use the + // single-string form spawn(`claude -p "ingest ${name}"`, { shell: true }). + child = spawn("claude", ["-p", `ingest "${file.name}"`], { cwd: base }); } catch (err) { // spawn() throws synchronously for some argument shapes. Without this the // timer would run forever and the row would stay disabled until reload.