Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions src/upgrade.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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`] };
}

/**
Expand Down
53 changes: 51 additions & 2 deletions test/upgrade.test.mjs
Original file line number Diff line number Diff line change
@@ -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-"));
Expand Down Expand Up @@ -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);
});
Loading