Fix #476: stop using JSON.stringify/double quotes as shell quoting in four exec sites (#533)

* Fix: use argv exec and single-quote escaping for the four #476 shell-injection sites

JSON.stringify and raw double-quote interpolation were used as shell quoting,
but /bin/sh still expands $(...), backticks, and ${} inside double quotes.

- is-generated.mjs / live.mjs runScript: switch execSync string commands to
  execFileSync argv form, which never invokes a shell. Closes the remote path
  where a source file named `$(...)` executes during the live-mode walk.
- skills.mjs hook command + hook-lib.mjs ignore-value suggestion: values that
  must stay shell strings now use POSIX single-quote escaping instead of
  JSON/double quotes. The doctor's hook-token parser learns the single-quoted
  absolute form so it keeps verifying user-level installs.

Adds regression tests for the single-quoted absolute hook form and the
single-quoted ignore-value suggestion. Verified end to end in a browser through
a real live-mode wrap walk against a hostile-named source file.

Prepared with AI assistance (Cursor) under maintainer instruction.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Test: lock in POSIX single-quoting for a $(...) absolute install path (#476)

Follow-up from security review: prove an install path embedding $(...) is
single-quoted in the written hook manifest, not double-quoted.

Prepared with AI assistance (Cursor) under maintainer instruction.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Fix: quote ignore-command args per platform so Windows cmd.exe keeps spaces (#533)

Greptile flagged that switching quoteCommandArg to POSIX single quotes fixed
$(...) injection on /bin/sh but regressed Windows cmd.exe, where single quotes
are literal, so a --file path containing spaces was split and the ignore scope
was stored malformed.

The suggested command runs on the same machine the hook fired on, so branch on
process.platform (the pattern skills.mjs already uses): single-quote on POSIX
for the #476 fix, and keep the original double-quote escaping on Windows so
that path's behavior is unchanged. Adds a regression test asserting both forms.

Prepared with AI assistance (Cursor) under maintainer instruction.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Test: prove the POSIX hook guard is inert under /bin/sh and Windows keeps double quotes (#533)

Greptile's probe could not reach the generated manifest, leaving the hook
command contract unverified. Convert that into committed proof:

- POSIX: install with a $(touch pwned) absolute path, then actually execute the
  generated guard under /bin/sh from a clean cwd and assert no marker file
  appears and the guard exits 0 (single-quoted substitution stays inert).
- Windows: drive copyProviderHooks as win32 in-process and assert the command
  keeps the double-quoted absolute path (usable when the install path has
  spaces; $(...) is inert on cmd.exe anyway).

Test-only; source quoting is unchanged.

Prepared with AI assistance (Cursor) under maintainer instruction.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>

skill/scripts/hook-lib.mjs

@@ -1112,7 +1112,19 @@ function formatFindingIgnoreCommand(finding) {
 function quoteCommandArg(value) {
   const text = String(value || '').trim();
   if (/^[A-Za-z0-9._:-]+$/.test(text)) return text;
-  return `"${text.replace(/\\/g, '\\\\').replace(/"/g, '\\"')}"`;
+  // The suggestion is meant to be run on this same machine, so quote for its
+  // shell. POSIX /bin/sh still expands $(...), backticks, and ${} inside
+  // double quotes, and these values come from scanned file content (a
+  // font-family name) or a file path, so untrusted input must be
+  // single-quoted (issue #476). Windows cmd.exe performs no such command
+  // substitution, but it treats a single quote as a literal character rather
+  // than a grouping delimiter, so a value or path containing spaces has to
+  // stay double-quoted there (Greptile #533). Keep the pre-existing
+  // double-quote escaping on Windows so that path's behavior is unchanged.
+  if (process.platform === 'win32') {
+    return `"${text.replace(/\\/g, '\\\\').replace(/"/g, '\\"')}"`;
+  }
+  return `'${text.replace(/'/g, `'\\''`)}'`;
 }
 
 function relativize(filePath, cwd) {

skill/scripts/lib/is-generated.mjs

@@ -13,7 +13,7 @@
  *      within the first ~300 characters — catches non-git projects.
  */
 
-import { execSync } from 'node:child_process';
+import { execFileSync } from 'node:child_process';
 import fs from 'node:fs';
 import path from 'node:path';
 
@@ -41,7 +41,10 @@ export function isGeneratedFile(filePath, options = {}) {
 
 function isGitIgnored(absPath, cwd) {
   try {
-    execSync(`git check-ignore --quiet ${JSON.stringify(absPath)}`, {
+    // argv form, never a shell: this runs on every file the live-mode source
+    // walk reaches, so a hostile filename embedding $(...) or backticks must
+    // not be interpretable (issue #476). JSON.stringify is not shell quoting.
+    execFileSync('git', ['check-ignore', '--quiet', absPath], {
       cwd,
       stdio: 'ignore',
     });

skill/scripts/lib/staleness-deep.mjs

@@ -244,7 +244,8 @@ const HOOK_MARKER = /skills\/impeccable\/scripts\/hook(?:-before-edit)?\.mjs/;
 //   * bundle-relative: node ".agents/.../hook.mjs"
 //   * legacy unquoted: node .claude/.../hook.mjs
 //   * guarded (#399):  [ ! -f "PATH" ] || node "PATH"   (PATH twice, identical)
-//   * absolute:        node "/Users/.../hook.mjs"        (user-level installs)
+//   * absolute (#476): [ ! -f 'PATH' ] || node 'PATH'   (single-quoted since
+//                      the shell-injection fix; older installs double-quote)
 //   * github portable: node "$(git rev-parse --show-toplevel)/.../hook.mjs"
 // A quoted path wins; the guard's two occurrences are identical, so the first
 // quoted match is the path. Otherwise fall back to the whitespace/metachar-
@@ -255,6 +256,12 @@ function hookScriptTokenFrom(command) {
   if (!HOOK_MARKER.test(str)) return null;
   const quoted = str.match(/"([^"]*skills\/impeccable\/scripts\/hook(?:-before-edit)?\.mjs)"/);
   if (quoted) return quoted[1];
+  // A path containing an apostrophe serializes as '\'' inside single quotes;
+  // no regex reassembles that, and the bare fallback would misread a fragment
+  // of it, so return null: the caller never asserts on a path it can't parse.
+  if (str.includes("'\\''")) return null;
+  const singleQuoted = str.match(/'([^']*skills\/impeccable\/scripts\/hook(?:-before-edit)?\.mjs)'/);
+  if (singleQuoted) return singleQuoted[1];
   const bare = str.match(/([^\s"'|&;()]*skills\/impeccable\/scripts\/hook(?:-before-edit)?\.mjs)/);
   return bare ? bare[1] : null;
 }

skill/scripts/live.mjs

@@ -17,7 +17,7 @@
  *   node live.mjs --help
  */
 
-import { execSync } from 'node:child_process';
+import { execFileSync } from 'node:child_process';
 import fs from 'node:fs';
 import path from 'node:path';
 import { fileURLToPath } from 'node:url';
@@ -316,11 +316,17 @@ function globToRegex(pattern) {
 
 function runScript(name, args, options = {}) {
   const scriptPath = path.join(__dirname, name);
-  const cmd = `node "${scriptPath}" ${args.map(a => `"${a}"`).join(' ')}`;
   try {
-    return execSync(cmd, { encoding: 'utf-8', cwd: options.cwd || process.cwd(), timeout: 15_000 });
+    // argv form, never a shell: string interpolation into double quotes would
+    // let a `"` or `$(...)` in any future caller's arg escape into the shell
+    // (issue #476).
+    return execFileSync(process.execPath, [scriptPath, ...args], {
+      encoding: 'utf-8',
+      cwd: options.cwd || process.cwd(),
+      timeout: 15_000,
+    });
   } catch (err) {
-    // execSync throws on non-zero exit; return stdout if any
+    // execFileSync throws on non-zero exit; return stdout if any
     return err.stdout || err.message || '';
   }
 }