Skip to content

Commit e628298

Browse files
fix(resolve): stop an Object.prototype name resolving as an engine or tool
ENGINES, ALIASES, TOOLS and AI_EXEC are plain object literals looked up with OBJ[key], so a target named after an Object.prototype member resolved truthy and was handed downstream as a real entry with no bin or install spec. $ moshcode install constructor TypeError: Cannot read properties of undefined (reading 'cmd') Same crash from `moshcode upgrade constructor`, `moshcode constructor` (passthrough), and `/install constructor` in the TUI, where it takes the whole session down instead of printing the unknown-target line. Resolve own properties only, so these names fall through to the existing usage/unknown-target paths.
1 parent 05cfd7f commit e628298

7 files changed

Lines changed: 55 additions & 7 deletions

File tree

bin/moshcode.mjs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,10 @@ async function main() {
224224
}
225225
if (cmd === "install") {
226226
const target = rest.find((a) => !a.startsWith("-"))?.toLowerCase();
227-
const entry = target && (ENGINES[target] || TOOLS[target]);
227+
// Own properties only — `install constructor` must print usage, not resolve
228+
// to something off Object.prototype and crash on its missing install spec.
229+
const entry = target
230+
&& ((Object.hasOwn(ENGINES, target) && ENGINES[target]) || (Object.hasOwn(TOOLS, target) && TOOLS[target]));
228231
if (!target || !entry) {
229232
console.error(`usage: moshcode install <engine|tool>\nengines:\n${engineList()}\ntools:\n${toolList()}`);
230233
process.exit(target ? 1 : 0);

src/engines.mjs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,10 @@ const ALIASES = { cc: "claude", "claude-code": "claude", openai: "codex", gpt: "
7878
export function resolveEngine(token) {
7979
if (!token) return null;
8080
const t = String(token).trim().toLowerCase();
81-
const key = ENGINES[t] ? t : ALIASES[t];
81+
// Own properties only: ENGINES/ALIASES are plain object literals, so a name
82+
// like `constructor` or `__proto__` would otherwise resolve to something off
83+
// Object.prototype and be handed on as an engine with no bin/install.
84+
const key = Object.hasOwn(ENGINES, t) ? t : Object.hasOwn(ALIASES, t) ? ALIASES[t] : null;
8285
return key ? [key, ENGINES[key]] : null;
8386
}
8487

@@ -145,7 +148,7 @@ const AI_EXEC = {
145148

146149
/** argv that runs `prompt` headlessly on `engine` (throws if it has no headless mode). */
147150
export function aiExecArgs(engine, prompt) {
148-
const fn = AI_EXEC[engine];
151+
const fn = Object.hasOwn(AI_EXEC, engine) ? AI_EXEC[engine] : null;
149152
if (!fn) throw new Error(`moshscript: ai() has no headless mode for "${engine}"`);
150153
return fn(String(prompt));
151154
}
@@ -154,7 +157,7 @@ export function aiExecArgs(engine, prompt) {
154157
export function pickAiEngine(preferred) {
155158
const order = preferred ? [preferred] : ["claude", "codex", "opencode", "gemini", "aider"];
156159
for (const key of order) {
157-
if (ENGINES[key] && AI_EXEC[key] && isInstalled(ENGINES[key].bin)) return key;
160+
if (Object.hasOwn(ENGINES, key) && Object.hasOwn(AI_EXEC, key) && isInstalled(ENGINES[key].bin)) return key;
158161
}
159162
return null;
160163
}

src/tools.mjs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,10 @@ export const TOOLS = {
3838
export function resolveTool(token) {
3939
if (!token) return null;
4040
const key = String(token).trim().toLowerCase();
41-
return TOOLS[key] ? [key, TOOLS[key]] : null;
41+
// Own properties only: TOOLS is a plain object literal, so a name like
42+
// `constructor` or `__proto__` would otherwise resolve to something off
43+
// Object.prototype and be handed on as a tool with no bin/install.
44+
return Object.hasOwn(TOOLS, key) ? [key, TOOLS[key]] : null;
4245
}
4346

4447
/** Tool entries annotated with native executable install status. */

src/tui.mjs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,9 @@ async function openShell(rawCmd) {
252252

253253
function installTarget(key) {
254254
return new Promise((resolve) => {
255-
const target = ENGINES[key] || TOOLS[key];
255+
// Own properties only — `/install constructor` must print the unknown-target
256+
// line, not resolve to something off Object.prototype and crash the pit.
257+
const target = (Object.hasOwn(ENGINES, key) && ENGINES[key]) || (Object.hasOwn(TOOLS, key) && TOOLS[key]);
256258
if (!target) { console.log(err(`unknown engine or tool "${key}"`)); return resolve(); }
257259
console.log(info(`installing ${key}: ${target.install.cmd} ${target.install.args.join(" ")}`));
258260
console.log(hr());

test/engines.test.mjs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { fileURLToPath } from "node:url";
1414
import { spawn } from "node:child_process";
1515
import test from "node:test";
1616

17-
import { ENGINES, agentLaunchArgs, exitReason, ranOk, runCmd } from "../src/engines.mjs";
17+
import { ENGINES, agentLaunchArgs, aiExecArgs, exitReason, pickAiEngine, ranOk, resolveEngine, runCmd } from "../src/engines.mjs";
1818

1919
const BIN = fileURLToPath(new URL("../bin/moshcode.mjs", import.meta.url));
2020
// The autonomous-session bypass flags each engine declares (engine.agentArgs).
@@ -157,3 +157,12 @@ test("ranOk and exitReason cover clean exits, bad codes, and spawn errors", asyn
157157
assert.equal(ranOk(missing), false);
158158
assert.match(exitReason(missing), /ENOENT|not found|spawn/i);
159159
});
160+
161+
test("engine lookup ignores inherited Object.prototype members", () => {
162+
// ENGINES/ALIASES/AI_EXEC are plain object literals, so an unknown name that
163+
// matches an Object.prototype member must not resolve as a real engine.
164+
assert.equal(resolveEngine("constructor"), null);
165+
assert.equal(resolveEngine("__proto__"), null);
166+
assert.equal(pickAiEngine("constructor"), null);
167+
assert.throws(() => aiExecArgs("constructor", "hi"), /no headless mode for "constructor"/);
168+
});

test/tools.test.mjs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,3 +184,19 @@ fs.writeFileSync(process.env.SHELL_CAPTURE, JSON.stringify(process.argv.slice(2)
184184
assert.deepEqual(JSON.parse(readFileSync(capture, "utf8")), ["-c", script]);
185185
});
186186
}
187+
188+
test("tool lookup ignores inherited Object.prototype members", () => {
189+
// TOOLS is a plain object literal: `TOOLS.constructor` is truthy but is not a
190+
// tool, so resolving it would hand a bin-less, install-less entry downstream.
191+
assert.equal(resolveTool("constructor"), null);
192+
assert.equal(resolveTool("__proto__"), null);
193+
assert.equal(resolveTool("valueOf"), null);
194+
});
195+
196+
test("moshcode install reports an Object.prototype name as unknown", async () => {
197+
const result = await run(["install", "constructor"]);
198+
199+
assert.equal(result.status, 1);
200+
assert.match(result.stderr, /usage: moshcode install <engine\|tool>/);
201+
assert.doesNotMatch(result.stderr, /TypeError/);
202+
});

test/tui.test.mjs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,3 +105,15 @@ test("TUI /shell and !cmd run an identical command identically", posixShell, asy
105105
assert.equal(viaBang.status, 0);
106106
assert.deepEqual(bracketed(viaSlash.stdout), bracketed(viaBang.stdout));
107107
});
108+
109+
// ENGINES/TOOLS are plain object literals, so an unknown target that happens to
110+
// name an Object.prototype member used to resolve truthy and reach `.install`,
111+
// killing the whole pit with a raw TypeError instead of printing the usual
112+
// unknown-target line.
113+
test("TUI /install rejects an Object.prototype name instead of crashing the pit", async () => {
114+
const result = await runTui("/install constructor\n/quit\n");
115+
116+
assert.equal(result.status, 0, result.stderr || result.stdout);
117+
assert.match(result.stdout, /unknown engine or tool "constructor"/);
118+
assert.doesNotMatch(result.stderr, /TypeError/);
119+
});

0 commit comments

Comments
 (0)