From 7e14dfb73e221d6718e4069dc2f400319ebc40bf Mon Sep 17 00:00:00 2001
From: Esteban Espin <estebanespin@MacBook-Air-de-Esteban.local>
Date: Sun, 26 Jul 2026 15:35:39 -0600
Subject: [PATCH] fix(upgrade): quote MOSHCODE_HOME and installer URL in the
 self-upgrade shell

selfSpec() embedded both values raw in a `sh -c` command line. An apostrophe
in the install path (e.g. /Users/o'brien/moshcode) broke `moshcode upgrade`
with a shell syntax error, and a hostile path could break out of the quotes
and inject extra shell into the update command. POSIX single-quote escaping
(' -> '\'') fixes both; selfSpec is exported with injectable home/url so the
quoting is unit-tested end to end through a real sh.
---
 src/upgrade.mjs       | 11 +++++++--
 test/upgrade.test.mjs | 53 +++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/src/upgrade.mjs b/src/upgrade.mjs
index aa9b071..b9b26ae 100644
--- a/src/upgrade.mjs
+++ b/src/upgrade.mjs
@@ -28,9 +28,16 @@ function selfVersion() {
   catch { return null; }
 }
 
-function selfSpec() {
+// POSIX-safe single-quoting for values embedded in the `sh -c` command line:
+// wrap in '…' and escape every ' as '\'' . Without this an apostrophe in the
+// install path (e.g. /Users/o'brien/moshcode) broke self-upgrade with a shell
+// syntax error — and a hostile path could break out of the quotes and inject
+// extra shell into the update command.
+const shQ = (value) => `'${String(value).replace(/'/g, `'\\''`)}'`;
+
+export function selfSpec(home = MOSHCODE_HOME, url = SELF_URL) {
   // Export MOSHCODE_HOME so install.sh updates the exact dir we run from.
-  return { cmd: "sh", args: ["-c", `export MOSHCODE_HOME='${MOSHCODE_HOME}'; curl -fsSL ${SELF_URL} | sh -s -- update`] };
+  return { cmd: "sh", args: ["-c", `export MOSHCODE_HOME=${shQ(home)}; curl -fsSL ${shQ(url)} | sh -s -- update`] };
 }
 
 /**
diff --git a/test/upgrade.test.mjs b/test/upgrade.test.mjs
index 2a6fbdb..2c938a7 100644
--- a/test/upgrade.test.mjs
+++ b/test/upgrade.test.mjs
@@ -1,10 +1,10 @@
 import assert from "node:assert/strict";
-import { chmodSync, mkdtempSync, writeFileSync } from "node:fs";
+import { chmodSync, existsSync, mkdtempSync, mkdirSync, readFileSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import path from "node:path";
 import test from "node:test";
 
-import { planUpgrade } from "../src/upgrade.mjs";
+import { planUpgrade, selfSpec } from "../src/upgrade.mjs";
 
 function withFakeTools(fn) {
   const dir = mkdtempSync(path.join(tmpdir(), "moshcode-upgrade-"));
@@ -64,3 +64,52 @@ test("unknown upgrade targets remain visible to the caller", () => {
   assert.deepEqual(plan.unknown, ["not-a-tool"]);
   assert.equal(plan.items.length, 0);
 });
+
+// The self-upgrade spec embeds MOSHCODE_HOME + the installer URL in a `sh -c`
+// command line. Values must be POSIX single-quote escaped: an apostrophe in the
+// install path used to break the command with a shell syntax error (and a
+// hostile path could inject extra shell). Run the generated command for real,
+// with a stub `curl` on PATH that records the env it sees and emits a no-op
+// "installer".
+function runSelfSpec(home) {
+  const root = mkdtempSync(path.join(tmpdir(), "moshcode-selfspec-"));
+  const bin = path.join(root, "bin");
+  mkdirSync(bin);
+  const capture = path.join(root, "home.txt");
+  writeFileSync(path.join(bin, "curl"), `#!/bin/sh\nprintf '%s' "$MOSHCODE_HOME" > ${JSON.stringify(capture)}\necho ':'\n`);
+  chmodSync(path.join(bin, "curl"), 0o755);
+  return { root, bin, capture };
+}
+
+test("selfSpec survives an apostrophe in MOSHCODE_HOME", async () => {
+  const home = "/Users/o'brien/moshcode home";
+  const { bin, capture } = runSelfSpec(home);
+  const spec = selfSpec(home, "https://example.com/install.sh");
+  const before = process.env.PATH;
+  process.env.PATH = `${bin}${path.delimiter}${before || ""}`;
+  try {
+    const { runCmd, ranOk } = await import("../src/engines.mjs");
+    const result = await runCmd(spec.cmd, spec.args);
+    assert.ok(ranOk(result), `self-upgrade command should exit 0, got ${JSON.stringify(result)}`);
+  } finally {
+    process.env.PATH = before;
+  }
+  assert.equal(readFileSync(capture, "utf8"), home);
+});
+
+test("selfSpec cannot be broken out of by a hostile install path", async () => {
+  const { bin, capture, root } = runSelfSpec("unused");
+  const marker = path.join(root, "INJECTED");
+  const home = `/tmp/x'; touch '${marker}'; '`;
+  const spec = selfSpec(home, "https://example.com/install.sh");
+  const before = process.env.PATH;
+  process.env.PATH = `${bin}${path.delimiter}${before || ""}`;
+  try {
+    const { runCmd } = await import("../src/engines.mjs");
+    await runCmd(spec.cmd, spec.args);
+  } finally {
+    process.env.PATH = before;
+  }
+  assert.equal(existsSync(marker), false, "path must not inject shell into the update command");
+  assert.equal(readFileSync(capture, "utf8"), home);
+});