Skip to content

Commit 7e14dfb

Browse files
Esteban Espinkhipu-agent
authored andcommitted
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.
1 parent 05cfd7f commit 7e14dfb

2 files changed

Lines changed: 60 additions & 4 deletions

File tree

src/upgrade.mjs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,16 @@ function selfVersion() {
2828
catch { return null; }
2929
}
3030

31-
function selfSpec() {
31+
// POSIX-safe single-quoting for values embedded in the `sh -c` command line:
32+
// wrap in '…' and escape every ' as '\'' . Without this an apostrophe in the
33+
// install path (e.g. /Users/o'brien/moshcode) broke self-upgrade with a shell
34+
// syntax error — and a hostile path could break out of the quotes and inject
35+
// extra shell into the update command.
36+
const shQ = (value) => `'${String(value).replace(/'/g, `'\\''`)}'`;
37+
38+
export function selfSpec(home = MOSHCODE_HOME, url = SELF_URL) {
3239
// Export MOSHCODE_HOME so install.sh updates the exact dir we run from.
33-
return { cmd: "sh", args: ["-c", `export MOSHCODE_HOME='${MOSHCODE_HOME}'; curl -fsSL ${SELF_URL} | sh -s -- update`] };
40+
return { cmd: "sh", args: ["-c", `export MOSHCODE_HOME=${shQ(home)}; curl -fsSL ${shQ(url)} | sh -s -- update`] };
3441
}
3542

3643
/**

test/upgrade.test.mjs

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
import assert from "node:assert/strict";
2-
import { chmodSync, mkdtempSync, writeFileSync } from "node:fs";
2+
import { chmodSync, existsSync, mkdtempSync, mkdirSync, readFileSync, writeFileSync } from "node:fs";
33
import { tmpdir } from "node:os";
44
import path from "node:path";
55
import test from "node:test";
66

7-
import { planUpgrade } from "../src/upgrade.mjs";
7+
import { planUpgrade, selfSpec } from "../src/upgrade.mjs";
88

99
function withFakeTools(fn) {
1010
const dir = mkdtempSync(path.join(tmpdir(), "moshcode-upgrade-"));
@@ -64,3 +64,52 @@ test("unknown upgrade targets remain visible to the caller", () => {
6464
assert.deepEqual(plan.unknown, ["not-a-tool"]);
6565
assert.equal(plan.items.length, 0);
6666
});
67+
68+
// The self-upgrade spec embeds MOSHCODE_HOME + the installer URL in a `sh -c`
69+
// command line. Values must be POSIX single-quote escaped: an apostrophe in the
70+
// install path used to break the command with a shell syntax error (and a
71+
// hostile path could inject extra shell). Run the generated command for real,
72+
// with a stub `curl` on PATH that records the env it sees and emits a no-op
73+
// "installer".
74+
function runSelfSpec(home) {
75+
const root = mkdtempSync(path.join(tmpdir(), "moshcode-selfspec-"));
76+
const bin = path.join(root, "bin");
77+
mkdirSync(bin);
78+
const capture = path.join(root, "home.txt");
79+
writeFileSync(path.join(bin, "curl"), `#!/bin/sh\nprintf '%s' "$MOSHCODE_HOME" > ${JSON.stringify(capture)}\necho ':'\n`);
80+
chmodSync(path.join(bin, "curl"), 0o755);
81+
return { root, bin, capture };
82+
}
83+
84+
test("selfSpec survives an apostrophe in MOSHCODE_HOME", async () => {
85+
const home = "/Users/o'brien/moshcode home";
86+
const { bin, capture } = runSelfSpec(home);
87+
const spec = selfSpec(home, "https://example.com/install.sh");
88+
const before = process.env.PATH;
89+
process.env.PATH = `${bin}${path.delimiter}${before || ""}`;
90+
try {
91+
const { runCmd, ranOk } = await import("../src/engines.mjs");
92+
const result = await runCmd(spec.cmd, spec.args);
93+
assert.ok(ranOk(result), `self-upgrade command should exit 0, got ${JSON.stringify(result)}`);
94+
} finally {
95+
process.env.PATH = before;
96+
}
97+
assert.equal(readFileSync(capture, "utf8"), home);
98+
});
99+
100+
test("selfSpec cannot be broken out of by a hostile install path", async () => {
101+
const { bin, capture, root } = runSelfSpec("unused");
102+
const marker = path.join(root, "INJECTED");
103+
const home = `/tmp/x'; touch '${marker}'; '`;
104+
const spec = selfSpec(home, "https://example.com/install.sh");
105+
const before = process.env.PATH;
106+
process.env.PATH = `${bin}${path.delimiter}${before || ""}`;
107+
try {
108+
const { runCmd } = await import("../src/engines.mjs");
109+
await runCmd(spec.cmd, spec.args);
110+
} finally {
111+
process.env.PATH = before;
112+
}
113+
assert.equal(existsSync(marker), false, "path must not inject shell into the update command");
114+
assert.equal(readFileSync(capture, "utf8"), home);
115+
});

0 commit comments

Comments
 (0)