Quote declaration-derived arguments in suggested type-patch commands

packages/compiler/src/frontend/lowering/coercions/unions.ts

@@ -445,6 +445,7 @@ export function narrowedArmHelper(
     message ?? null,
     null,
     origin,
+    lowerer.targetPlatform === "win32",
   );
   if (!fn) throw new InternalCompilerError("lowerer bug: invalid checked union extraction");
   lowerer.coercions.checkedNarrowArms.set(name, {
@@ -549,6 +550,7 @@ export function checkedNarrowCall(
                 ? "undefined"
                 : "null"
               : lowerer.fmt(failing),
+            lowerer.targetPlatform === "win32",
           ));
     const thrown = nodeThrowExpr(1, "", message, type, loc);
     failure =

packages/compiler/src/frontend/lowering/decl-origin.ts

@@ -353,15 +353,51 @@ export function declarationSelector(decl: ts.Node): string | null {
   return null;
 }
 
+/** One argument of the suggested `scriptc types patch` command, quoted so
+ * the shell passes it as written (scr_type_origin.c quotes the same way):
+ * bare when it has only characters no shell treats specially; otherwise,
+ * for a POSIX shell, in single quotes with each `'` as `'\''`, and for
+ * Windows (cmd and PowerShell), in double quotes when it has none of the
+ * characters either shell expands, escapes or ends a string with there. Null when no quoting
+ * is inert there: the note then names the command without arguments.
+ * Package names, selectors and types come from package declarations. */
+export function shellArgument(value: string, windows: boolean): string | null {
+  if (/^[A-Za-z0-9_@+=:,./-]+$/.test(value)) return value;
+  if (!windows)
+    return /^[A-Za-z0-9_@%+=:,./-]+$/.test(value) ? value : `'${value.replaceAll("'", "'\\''")}'`;
+  for (const ch of value) {
+    const code = ch.codePointAt(0)!;
+    // PowerShell also takes the typographic quotes U+2018-U+201F as quotes.
+    if (code < 0x20 || code === 0x7f || (code >= 0x2018 && code <= 0x201f)) return null;
+    if ('"%!^$`'.includes(ch)) return null;
+  }
+  return `"${value}"`;
+}
+
+/** The suggested patch for a declaration: `scriptc types patch` with the
+ * package, the selector and the type, each quoted for the target's shell. */
+export function patchSuggestion(
+  pkg: string,
+  selector: string,
+  type: string,
+  windows: boolean,
+): string {
+  const args = [pkg, selector, type].map((a) => shellArgument(a, windows));
+  return args.every((a) => a !== null)
+    ? `if that declaration is wrong, patch it: scriptc types patch ${args.join(" ")}`
+    : "if that declaration is wrong, patch it with scriptc types patch";
+}
+
 /** The note a union extraction's TypeError carries for a failing `arm`
- * (the same text the runtime renders for checked conversions). */
-export function renderOriginNote(origin: string, arm: string): string {
+ * (the same text the runtime renders for checked conversions). `windows`:
+ * the program targets Windows, whose shells quote differently. */
+export function renderOriginNote(origin: string, arm: string, windows = false): string {
   const [pkg = "", version = "", , selector = "", where = "", declared = ""] =
     origin.split(ORIGIN_SEPARATOR);
   return (
     `\n    ${pkg}${version === "" ? "" : `@${version}`} declares ${selector}` +
     `${declared === "" ? "" : ` as ${declared}`} (${where})` +
-    `\n    if that declaration is wrong, patch it: scriptc types patch ${pkg} "${selector}" "| ${arm}"`
+    `\n    ${patchSuggestion(pkg, selector, `| ${arm}`, windows)}`
   );
 }
 

packages/compiler/src/frontend/lowering/union-narrow.ts

@@ -39,6 +39,8 @@ export function buildUnionNarrow(
   /** The target's package-declared origin (decl-origin.ts): each arm's
    * message names the declaration and the patch admitting that arm. */
   origin: string | null = null,
+  /** The program targets Windows (the patch command's quoting). */
+  windows = false,
 ): IrFunction | null {
   if (isUnitType(target) || target.kind === "void") return null;
   const targetTag = from.arms.findIndex((arm) => typeEquals(arm, target));
@@ -102,6 +104,7 @@ export function buildUnionNarrow(
                             ? "undefined"
                             : "null"
                           : formatType(arm),
+                        windows,
                       )),
                 type: STRING,
                 loc,

packages/compiler/test/decl-origin-quoting.test.ts

@@ -0,0 +1,73 @@
+/* The suggested `scriptc types patch` command in a declared-type note
+ * (frontend/lowering/decl-origin.ts) passes each value it carries as one
+ * argument. Package names, selectors and types come from package
+ * declarations. Run through sh with printf in place of scriptc, the POSIX
+ * form prints each value exactly and creates no file. The Windows form
+ * double-quotes values that cmd and PowerShell keep as written there, and
+ * leaves the arguments out when either shell would expand or escape one. */
+import { execFile } from "node:child_process";
+import { mkdtemp, readdir, rm } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { promisify } from "node:util";
+import { expect, test } from "vitest";
+import { patchSuggestion, shellArgument } from "../src/frontend/lowering/decl-origin.js";
+
+const exec = promisify(execFile);
+
+const hostile = [
+  "Profile.a;touch semicolon",
+  "Profile.$(touch subst)",
+  "Profile.`touch backtick`",
+  "Profile.it's \"quoted\" \\ end'",
+  "Profile.a&&touch and|touch pipe>out",
+  "Profile.a\nb",
+];
+
+test("plain values stay bare and types are quoted", () => {
+  expect(patchSuggestion("kit", "Profile.email", "| undefined", false)).toBe(
+    "if that declaration is wrong, patch it: scriptc types patch kit Profile.email '| undefined'",
+  );
+  expect(patchSuggestion("@scope/kit", "Terminal.columns()", "| undefined", true)).toBe(
+    'if that declaration is wrong, patch it: scriptc types patch @scope/kit "Terminal.columns()" "| undefined"',
+  );
+});
+
+test.skipIf(process.platform === "win32")(
+  "the POSIX form passes each value as one argument",
+  async () => {
+    const dir = await mkdtemp(join(tmpdir(), "scriptc-decl-origin-"));
+    try {
+      for (const selector of hostile) {
+        const pkg = "evil;touch pkg";
+        const command = patchSuggestion(pkg, selector, "| undefined", false).replace(
+          "if that declaration is wrong, patch it: scriptc types patch",
+          "printf '%s\\n'",
+        );
+        const { stdout } = await exec("sh", ["-c", command], { cwd: dir });
+        expect(stdout).toBe(`${pkg}\n${selector}\n| undefined\n`);
+        expect(await readdir(dir)).toEqual([]);
+      }
+    } finally {
+      await rm(dir, { recursive: true, force: true });
+    }
+  },
+);
+
+test("the Windows form double-quotes values only where both shells keep them", () => {
+  expect(shellArgument("Profile.a;b&c|d<e>f", true)).toBe('"Profile.a;b&c|d<e>f"');
+  for (const value of [
+    "Profile.$(x)",
+    "Profile.`x`",
+    'Profile."x"',
+    "Profile.%PATH%",
+    "Profile.!x!",
+    "Profile.“x”",
+    "Profile.‘x’",
+    "Profile.a\nb",
+  ])
+    expect(shellArgument(value, true), value).toBeNull();
+  expect(patchSuggestion("kit", "Profile.$(x)", "| undefined", true)).toBe(
+    "if that declaration is wrong, patch it with scriptc types patch",
+  );
+});

packages/runtime/src/scr_type_origin.c

@@ -42,6 +42,65 @@ static void scr_origin_put(ScrOriginBuf *b, const char *s, size_t n) {
 
 static void scr_origin_puts(ScrOriginBuf *b, const char *s) { scr_origin_put(b, s, strlen(s)); }
 
+/* One argument of the suggested `scriptc types patch` command, quoted as
+ * the shell passes it as written (decl-origin.ts's shellArgument quotes the
+ * same way): bare when it has only characters no shell treats specially;
+ * otherwise, for a POSIX shell, in single quotes with each ' as '\'', and
+ * on Windows (cmd and PowerShell), in double quotes when it has none of the
+ * characters either shell expands, escapes or ends a string with there.
+ * Package names, selectors and types come from package declarations.
+ * Returns false, writing nothing, when no quoting is inert. */
+static bool scr_origin_bare(const char *s, size_t n) {
+  if (n == 0) return false;
+  for (size_t i = 0; i < n; i++) {
+    unsigned char c = (unsigned char)s[i];
+    bool plain = (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') ||
+                 strchr("_@+=:,./-", c) != NULL;
+#ifndef _WIN32
+    plain = plain || c == '%';
+#endif
+    if (!plain || c == 0) return false;
+  }
+  return true;
+}
+
+static bool scr_origin_quotable(const char *s, size_t n) {
+#ifdef _WIN32
+  for (size_t i = 0; i < n; i++) {
+    unsigned char c = (unsigned char)s[i];
+    if (c < 0x20 || c == 0x7f || (c != 0 && strchr("\"%!^$`", c) != NULL)) return false;
+    /* PowerShell also takes the typographic quotes U+2018-U+201F (UTF-8
+     * E2 80 98..9F) as quotes. */
+    if (c == 0xe2 && i + 2 < n && (unsigned char)s[i + 1] == 0x80 &&
+        (unsigned char)s[i + 2] >= 0x98 && (unsigned char)s[i + 2] <= 0x9f)
+      return false;
+  }
+#else
+  (void)s;
+  (void)n;
+#endif
+  return true;
+}
+
+static void scr_origin_put_arg(ScrOriginBuf *b, const char *s, size_t n) {
+  if (scr_origin_bare(s, n)) {
+    scr_origin_put(b, s, n);
+    return;
+  }
+#ifdef _WIN32
+  scr_origin_puts(b, "\"");
+  scr_origin_put(b, s, n);
+  scr_origin_puts(b, "\"");
+#else
+  scr_origin_puts(b, "'");
+  for (size_t i = 0; i < n; i++) {
+    if (s[i] == '\'') scr_origin_puts(b, "'\\''");
+    else scr_origin_put(b, s + i, 1);
+  }
+  scr_origin_puts(b, "'");
+#endif
+}
+
 /* Field `index` of the origin string (not terminated: [*out, *out+len)). */
 static size_t scr_origin_field(const char *origin, int index, const char **out) {
   const char *p = origin;
@@ -116,29 +175,40 @@ void scr_type_origin_note(const char *origin) {
   }
   scr_origin_puts(&b, " (");
   scr_origin_put(&b, where, where_len);
-  scr_origin_puts(&b, ")\n    if that declaration is wrong, patch it: scriptc types patch ");
-  scr_origin_put(&b, pkg, pkg_len);
-  scr_origin_puts(&b, " \"");
-  scr_origin_put(&b, selector, selector_len);
+  scr_origin_puts(&b, ")\n    ");
+  ScrOriginBuf sel = {0}, type = {0};
+  scr_origin_put(&sel, selector, selector_len);
   /* $[3].email extends the selector as [].email: elements of an array
    * share one declared type. */
   for (size_t i = 1; path != NULL && i < path_len; i++) {
     if (path[i] == '[') {
-      scr_origin_puts(&b, "[]");
+      scr_origin_puts(&sel, "[]");
       while (i < path_len && path[i] != ']') i++;
     } else {
-      scr_origin_put(&b, path + i, 1);
+      scr_origin_put(&sel, path + i, 1);
     }
   }
-  scr_origin_puts(&b, "\" \"| ");
+  scr_origin_puts(&type, "| ");
   if (got != NULL) {
     const char *arm = scr_origin_arm(got, got_len);
-    if (arm != NULL) scr_origin_puts(&b, arm);
-    else scr_origin_put(&b, got, got_len);
+    if (arm != NULL) scr_origin_puts(&type, arm);
+    else scr_origin_put(&type, got, got_len);
   } else {
-    scr_origin_puts(&b, "undefined");
+    scr_origin_puts(&type, "undefined");
   }
-  scr_origin_puts(&b, "\"");
+  if (scr_origin_quotable(pkg, pkg_len) && scr_origin_quotable(sel.data, sel.len) &&
+      scr_origin_quotable(type.data, type.len)) {
+    scr_origin_puts(&b, "if that declaration is wrong, patch it: scriptc types patch ");
+    scr_origin_put_arg(&b, pkg, pkg_len);
+    scr_origin_puts(&b, " ");
+    scr_origin_put_arg(&b, sel.data, sel.len);
+    scr_origin_puts(&b, " ");
+    scr_origin_put_arg(&b, type.data, type.len);
+  } else {
+    scr_origin_puts(&b, "if that declaration is wrong, patch it with scriptc types patch");
+  }
+  free(sel.data);
+  free(type.data);
 
   ScrStr *next = scr_str_new(b.data, b.len);
   free(b.data);

packages/runtime/src/scr_type_origin.test.c

@@ -0,0 +1,24 @@
+/* Built by scr_type_origin.test.ts: a pending TypeError gains the
+ * declaration note for an origin whose package name and selector carry
+ * shell syntax; the test runs the suggested command through sh. */
+#include "scr_runtime.h"
+
+#include <stdio.h>
+#include <string.h>
+
+int main(int argc, char **argv) {
+  if (argc != 3) return 2;
+  /* package, version, range, selector, where, declared type */
+  char origin[1024];
+  snprintf(origin, sizeof origin, "%s\x1f" "1.0.0\x1f^1.0.0\x1f%s\x1findex.d.ts:3\x1fstring", argv[1],
+           argv[2]);
+  const char *text = "expected string at $, got undefined";
+  ScrExcCell *cell = scr_exc_current_cell();
+  cell->kind = SCR_EXC_OBJ;
+  cell->payload = scr_error_new(SCR_ERR_TYPE, scr_str_new(text, strlen(text)));
+  scr_type_origin_note(origin);
+  ScrError *e = cell->payload;
+  fwrite(e->message->data, 1, e->message->len, stdout);
+  fputc('\n', stdout);
+  return 0;
+}

packages/runtime/src/scr_type_origin.test.ts

@@ -0,0 +1,69 @@
+/* The suggested `scriptc types patch` command on a checked-conversion
+ * TypeError (scr_type_origin.c) passes each value it carries as one
+ * argument: package names and selectors come from package declarations,
+ * and a command copied from the message must not run anything they spell.
+ * Run through sh with printf in place of scriptc, the command prints the
+ * package, the selector and the type exactly, and creates no file. */
+import { execFile } from "node:child_process";
+import { mkdtemp, readdir, rm } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { promisify } from "node:util";
+import { expect, test } from "vitest";
+import { EXECUTABLE_RUNTIME_SOURCES } from "../../compiler/src/backend/native-toolchain.js";
+
+const exec = promisify(execFile);
+
+const hostile = [
+  ["kit", "Profile.email"],
+  ["evil;touch pkg", "Profile.a;touch semicolon"],
+  ["kit", "Profile.$(touch subst)"],
+  ["kit", "Profile.`touch backtick`"],
+  ["kit", "Profile.it's \"quoted\" \\ end'"],
+  ["kit", "Profile.a&&touch and|touch pipe>out"],
+] as const;
+
+test.skipIf(process.platform === "win32")(
+  "the suggested patch command passes declaration values as single arguments",
+  { timeout: 300_000 },
+  async () => {
+    const dir = await mkdtemp(join(tmpdir(), "scriptc-type-origin-"));
+    try {
+      const bin = join(dir, "origin");
+      await exec("clang", [
+        "-std=c11",
+        "-O1",
+        "-w",
+        "-pthread",
+        ...(process.platform === "linux" ? ["-D_GNU_SOURCE"] : []),
+        join(import.meta.dirname, "scr_type_origin.test.c"),
+        join(import.meta.dirname, "scr_type_origin.c"),
+        ...EXECUTABLE_RUNTIME_SOURCES.map((name) => join(import.meta.dirname, name)),
+        ...(process.platform === "linux" ? ["-lm"] : []),
+        "-o",
+        bin,
+      ]);
+      const sandbox = join(dir, "run");
+      await exec("mkdir", [sandbox]);
+      const commands: string[] = [];
+      for (const [pkg, selector] of hostile) {
+        const { stdout } = await exec(bin, [pkg, selector]);
+        const line = stdout.split("\n").find((l) => l.includes("scriptc types patch"));
+        expect(line, stdout).toBeDefined();
+        const command = line!.slice(line!.indexOf("scriptc types patch"));
+        commands.push(command);
+        const printed = await exec(
+          "sh",
+          ["-c", command.replace("scriptc types patch", "printf '%s\\n'")],
+          { cwd: sandbox },
+        );
+        expect(printed.stdout).toBe(`${pkg}\n${selector}\n| undefined\n`);
+        expect(await readdir(sandbox)).toEqual([]);
+      }
+      // Values without shell syntax stay bare.
+      expect(commands[0]).toBe("scriptc types patch kit Profile.email '| undefined'");
+    } finally {
+      await rm(dir, { recursive: true, force: true });
+    }
+  },
+);

tests/harness/type-patches.test.ts

@@ -121,7 +121,7 @@ describe("type patches", () => {
           expect(native.stderr).toBe(
             "Uncaught TypeError: expected string at $, got undefined\n" +
               "    profilekit@3.1.0 declares Profile.email as string (index.d.ts:7)\n" +
-              `    if that declaration is wrong, patch it: scriptc types patch profilekit "Profile.email" "| undefined"\n`,
+              `    if that declaration is wrong, patch it: scriptc types patch profilekit Profile.email '| undefined'\n`,
           );
         }
       } finally {
@@ -179,12 +179,12 @@ describe("type patches", () => {
       const result = await build(join(fixture, "unpatched/main.ts"), "unpatched", ["termctx"]);
       const native = await run(result.binaryPath, []);
       const patch = (selector: string) =>
-        `    if that declaration is wrong, patch it: scriptc types patch termctx "${selector}" "| undefined"`;
+        `    if that declaration is wrong, patch it: scriptc types patch termctx ${selector} '| undefined'`;
       expect(native.stdout).toBe(
         [
           "expected number at $, got undefined",
           "    termctx@1.2.0 declares Terminal.columns() as number (index.js:16)",
-          patch("Terminal.columns()"),
+          patch("'Terminal.columns()'"),
           "undefined is not representable in the target union (a value narrowed or asserted past it still held it)",
           "    termctx@1.2.0 declares OutputContext.width as number (index.js:24)",
           patch("OutputContext.width"),
@@ -206,7 +206,7 @@ describe("type patches", () => {
       const failure = (pkg: string, version: string) => [
         "undefined is not representable in the target union (a value narrowed or asserted past it still held it)",
         `    ${pkg}@${version} declares OutputContext.width as number (index.js:24)`,
-        `    if that declaration is wrong, patch it: scriptc types patch ${pkg} "OutputContext.width" "| undefined"`,
+        `    if that declaration is wrong, patch it: scriptc types patch ${pkg} OutputContext.width '| undefined'`,
       ];
       expect(native.stdout).toBe(
         [...failure("panectx", "2.0.0"), ...failure("gridctx", "4.1.0"), ""].join("\n"),