diff --git a/packages/agents/test/agents.test.ts b/packages/agents/test/agents.test.ts index 52ca3c61..5c963fc6 100644 --- a/packages/agents/test/agents.test.ts +++ b/packages/agents/test/agents.test.ts @@ -200,10 +200,10 @@ describe("agent manifests", () => { .map(async (file) => ({ file, source: (await readFile(join(templateDirectory, file), "utf8")) - .replaceAll("{{PLAN_MODEL}}", "claude-opus-4-8") - .replaceAll("{{REVIEW_MODEL}}", "claude-sonnet-4-6") - .replaceAll("{{CODEX_BUILD_MODEL}}", "gpt-5.6-sol") - .replaceAll("{{CODEX_PLAN_MODEL}}", "gpt-5.6-sol"), + .replaceAll("{{PLAN_MODEL}}", JSON.stringify("claude-opus-4-8")) + .replaceAll("{{REVIEW_MODEL}}", JSON.stringify("claude-sonnet-4-6")) + .replaceAll("{{CODEX_BUILD_MODEL}}", JSON.stringify("gpt-5.6-sol")) + .replaceAll("{{CODEX_PLAN_MODEL}}", JSON.stringify("gpt-5.6-sol")), })), ); const catalog = parseAgentCatalog(sources); @@ -228,4 +228,18 @@ describe("agent manifests", () => { expect(agent.prompt).not.toMatch(/receipt|HITL|budget ceiling|permission profile:/i); } }); + + it("treats a quoted hostile model id as data rather than YAML structure", async () => { + const model = "gpt-5.6-sol\nenabled: false"; + const source = ( + await readFile( + join(fileURLToPath(new URL(".", import.meta.url)), "../../cli/templates/agents/builder.md"), + "utf8", + ) + ).replaceAll("{{CODEX_BUILD_MODEL}}", JSON.stringify(model)); + const parsed = parseAgentManifest(source, "builder.md"); + expect(parsed.model).toBe(model); + expect(parsed.enabled).toBe(true); + expect(parsed.engine).toBe("codex"); + }); }); diff --git a/packages/cli/src/doctor.mjs b/packages/cli/src/doctor.mjs index 891e7e41..7c2c26e3 100644 --- a/packages/cli/src/doctor.mjs +++ b/packages/cli/src/doctor.mjs @@ -56,7 +56,7 @@ function checkAgent(dir, name) { if (!source.startsWith("---\n") || !/\n---\n[\s\S]*\S/.test(source)) return failed(relative, "invalid frontmatter or empty prompt"); if (!new RegExp(`^name:\\s*${escapeRegExp(name)}\\s*$`, "m").test(source)) return failed(relative, `name must be ${name}`); if (!/^engine:\s*(?:claude_code|codex)\s*$/m.test(source)) return failed(relative, "engine must be claude_code or codex"); - if (!/^model:\s*\S+\s*$/m.test(source)) return failed(relative, "model is missing"); + if (!/^model:\s*\S/m.test(source)) return failed(relative, "model is missing"); if (!/^triggers:\s*$/m.test(source) || !/^\s{2}- type:\s*(?:manual|schedule|github)\s*$/m.test(source)) { return failed(relative, "at least one supported trigger is required"); } diff --git a/packages/cli/src/init.mjs b/packages/cli/src/init.mjs index 12683a21..b648eca0 100644 --- a/packages/cli/src/init.mjs +++ b/packages/cli/src/init.mjs @@ -114,9 +114,13 @@ export async function init(flags, pkgRoot, version) { return 0; } +function yamlScalar(value) { + return JSON.stringify(value); +} + function renderTemplate(source, values) { return Object.entries(values).reduce( - (result, [key, value]) => result.replaceAll(`{{${key}}}`, value), + (result, [key, value]) => result.replaceAll(`{{${key}}}`, yamlScalar(value)), source, ); } @@ -125,12 +129,12 @@ function formatProjectManifest({ repository, setup, start, ready, servicePort }) return [ "version: 1", "repositories:", - ` primary: ${JSON.stringify(`github.com/${repository}`)}`, + ` primary: ${yamlScalar(`github.com/${repository}`)}`, " related: []", "environment:", - ...(setup ? [` setup: ${JSON.stringify(setup)}`] : []), - ` start: ${JSON.stringify(start)}`, - ...(ready ? [` ready: ${JSON.stringify(ready)}`] : []), + ...(setup ? [` setup: ${yamlScalar(setup)}`] : []), + ` start: ${yamlScalar(start)}`, + ...(ready ? [` ready: ${yamlScalar(ready)}`] : []), " services:", " app:", ` port: ${servicePort}`, diff --git a/packages/cli/test/init.test.mjs b/packages/cli/test/init.test.mjs index 8fc2c656..24b2a797 100644 --- a/packages/cli/test/init.test.mjs +++ b/packages/cli/test/init.test.mjs @@ -129,10 +129,71 @@ test("init configures Claude and Codex models in the same agent catalog", (t) => dir, ); assert.equal(result.status, 0, result.stdout + result.stderr); - assert.match(readFileSync(join(dir, ".agents/architect.md"), "utf8"), /model: claude-plan-custom/); - assert.match(readFileSync(join(dir, ".agents/pr-reviewer.md"), "utf8"), /model: claude-review-custom/); - assert.match(readFileSync(join(dir, ".agents/builder.md"), "utf8"), /model: codex-build-custom/); - assert.match(readFileSync(join(dir, ".agents/ci-doctor.md"), "utf8"), /model: codex-plan-custom/); + assert.match(readFileSync(join(dir, ".agents/architect.md"), "utf8"), /model: "claude-plan-custom"/); + assert.match(readFileSync(join(dir, ".agents/pr-reviewer.md"), "utf8"), /model: "claude-review-custom"/); + assert.match(readFileSync(join(dir, ".agents/builder.md"), "utf8"), /model: "codex-build-custom"/); + assert.match(readFileSync(join(dir, ".agents/ci-doctor.md"), "utf8"), /model: "codex-plan-custom"/); +}); + +test("init quotes hostile model ids and commands so they cannot inject YAML", (t) => { + const dir = makeTargetRepo(); + t.after(() => rmSync(dir, { recursive: true, force: true })); + const hostileModel = "gpt-5.6-sol\nenabled: false"; + const hostileStart = 'docker compose up -d && echo "$(id)" && echo "db: ready"'; + const result = runCli( + [ + "init", + "--yes", + `--dir=${dir}`, + "--repo=acme/demo-app", + `--provision=pnpm install --frozen-lockfile && echo 'setup: done'`, + `--start=${hostileStart}`, + "--preview-readiness-command=curl --fail 'http://localhost:3000/health'", + `--review-model=foo"bar # pwned`, + "--plan-model=$(id)", + `--codex-build-model=${hostileModel}`, + "--codex-plan-model=|", + "--build-model=claude-fable-5 # not-a-comment", + ], + dir, + ); + assert.equal(result.status, 0, result.stdout + result.stderr); + + const environment = readFileSync(join(dir, ".facility.yml"), "utf8"); + assert.match(environment, /setup: "pnpm install --frozen-lockfile && echo 'setup: done'"/); + assert.equal(environment.includes(`start: ${JSON.stringify(hostileStart)}`), true); + assert.equal( + environment.includes(`ready: ${JSON.stringify("curl --fail 'http://localhost:3000/health'")}`), + true, + ); + + const builder = readFileSync(join(dir, ".agents/builder.md"), "utf8"); + assert.equal(builder.includes(`model: ${JSON.stringify(hostileModel)}`), true); + assert.match(builder, /^enabled: true$/m); + assert.doesNotMatch(builder, /^enabled: false$/m); + + assert.equal( + readFileSync(join(dir, ".agents/architect.md"), "utf8").includes(`model: ${JSON.stringify("$(id)")}`), + true, + ); + assert.equal( + readFileSync(join(dir, ".agents/security-audit.md"), "utf8").includes(`model: ${JSON.stringify("$(id)")}`), + true, + ); + assert.equal( + readFileSync(join(dir, ".agents/pr-reviewer.md"), "utf8").includes( + `model: ${JSON.stringify('foo"bar # pwned')}`, + ), + true, + ); + + const doctor = readFileSync(join(dir, ".agents/ci-doctor.md"), "utf8"); + assert.equal(doctor.includes(`model: ${JSON.stringify("|")}`), true); + assert.match(doctor, /^options:$/m); + assert.match(doctor, /^ reasoning_effort: high$/m); + + const check = runCli(["doctor", `--dir=${dir}`, "--json"], dir); + assert.equal(check.status, 0, check.stdout + check.stderr); }); test("init preserves repository-owned files unless force is explicit", (t) => { diff --git a/packages/core/src/render-workspace-kickstart.ts b/packages/core/src/render-workspace-kickstart.ts index f1b204d2..9dfb5bcf 100644 --- a/packages/core/src/render-workspace-kickstart.ts +++ b/packages/core/src/render-workspace-kickstart.ts @@ -80,9 +80,14 @@ export function renderWorkspaceKickstart( }; } +function yamlScalar(value: string) { + return JSON.stringify(value); +} + function renderTemplate(template: string, values: Record) { return template.replace(/\{\{([A-Z0-9_]+)\}\}/g, (placeholder, name: string) => { - return values[name] ?? placeholder; + const value = values[name]; + return value === undefined ? placeholder : yamlScalar(value); }); } @@ -90,12 +95,12 @@ function projectManifest(answers: WorkspaceKickstartAnswers, servicePort: number return [ "version: 1", "repositories:", - ` primary: ${JSON.stringify(`github.com/${answers.repository}`)}`, + ` primary: ${yamlScalar(`github.com/${answers.repository}`)}`, " related: []", "environment:", - ...(answers.setup ? [` setup: ${JSON.stringify(answers.setup)}`] : []), - ` start: ${JSON.stringify(answers.start)}`, - ...(answers.ready ? [` ready: ${JSON.stringify(answers.ready)}`] : []), + ...(answers.setup ? [` setup: ${yamlScalar(answers.setup)}`] : []), + ` start: ${yamlScalar(answers.start)}`, + ...(answers.ready ? [` ready: ${yamlScalar(answers.ready)}`] : []), " services:", " app:", ` port: ${servicePort}`, diff --git a/packages/core/test/workspace-kickstart.test.ts b/packages/core/test/workspace-kickstart.test.ts index 4ae2e625..ab92652b 100644 --- a/packages/core/test/workspace-kickstart.test.ts +++ b/packages/core/test/workspace-kickstart.test.ts @@ -61,4 +61,73 @@ describe("Facility 0.12 workspace kickstart", () => { renderWorkspaceKickstart({ repository: "acme/app", start: "pnpm dev", servicePort: 0 }), ).toThrow(/between 1 and 65535/); }); + + it("quotes untrusted model ids and commands so they cannot inject YAML", () => { + const hostileModel = "gpt-5.6-sol\nenabled: false"; + const hostileStart = 'docker compose up -d && echo "$(id)" && echo "db: ready"'; + const result = renderWorkspaceKickstart({ + repository: "acme/payments", + setup: "pnpm install --frozen-lockfile && echo 'setup: done'", + start: hostileStart, + ready: "curl --fail 'http://localhost:3000/health'", + models: { + build: "claude-fable-5 # not-a-comment", + review: 'foo"bar # pwned', + plan: "$(id)", + codexBuild: hostileModel, + codexPlan: "|", + }, + }); + + const manifest = fileContent(result, ".facility.yml"); + expect(manifest).toContain( + `setup: ${JSON.stringify("pnpm install --frozen-lockfile && echo 'setup: done'")}`, + ); + expect(manifest).toContain(`start: ${JSON.stringify(hostileStart)}`); + expect(manifest).toContain( + `ready: ${JSON.stringify("curl --fail 'http://localhost:3000/health'")}`, + ); + expect(manifest).not.toMatch(/^ {2}start: docker compose/m); + + const builder = fileContent(result, ".agents/builder.md"); + expect(builder).toContain(`model: ${JSON.stringify(hostileModel)}`); + expect(builder).toMatch(/^enabled: true$/m); + expect(builder).not.toMatch(/^enabled: false$/m); + + expect(fileContent(result, ".agents/architect.md")).toContain( + `model: ${JSON.stringify("$(id)")}`, + ); + expect(fileContent(result, ".agents/security-audit.md")).toContain( + `model: ${JSON.stringify("$(id)")}`, + ); + expect(fileContent(result, ".agents/pr-reviewer.md")).toContain( + `model: ${JSON.stringify('foo"bar # pwned')}`, + ); + expect(fileContent(result, ".agents/architect.md")).not.toContain("model: $(id)"); + + const doctor = fileContent(result, ".agents/ci-doctor.md"); + expect(doctor).toContain(`model: ${JSON.stringify("|")}`); + expect(doctor).toMatch(/^options:$/m); + expect(doctor).toMatch(/^ {2}reasoning_effort: high$/m); + }); + + it("keeps ordinary model ids as quoted YAML scalars", () => { + const result = renderWorkspaceKickstart({ + repository: "acme/payments", + start: "pnpm dev", + models: { codexBuild: "gpt-5.6-sol", plan: "claude-opus-4-8-20260101" }, + }); + expect(fileContent(result, ".agents/builder.md")).toContain( + `model: ${JSON.stringify("gpt-5.6-sol")}`, + ); + expect(fileContent(result, ".agents/architect.md")).toContain( + `model: ${JSON.stringify("claude-opus-4-8-20260101")}`, + ); + }); }); + +function fileContent(result: { files: Array<{ path: string; content: string }> }, path: string) { + const file = result.files.find((candidate) => candidate.path === path); + if (!file) throw new Error(`missing ${path}`); + return file.content; +}