diff --git a/CHANGELOG.md b/CHANGELOG.md index e119c96..9308ef2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## 0.5.2 - Unreleased +- Added trusted Codex CLI config passthrough for explicit config files while rejecting repository-controlled passthrough config, thanks @brad-ai-agent. - Added a MiniMax HTTP provider for `map`, `review`, and `revalidate`, with local schema validation and explicit unsupported `fix` handling, thanks @ferminquant. ## 0.5.1 - 2026-06-10 diff --git a/README.md b/README.md index 7a2e321..0eac1cd 100644 --- a/README.md +++ b/README.md @@ -117,6 +117,11 @@ Clawpatch. Use any Codex sandbox mode, or `bypass`/`none` to pass `--dangerously-bypass-approvals-and-sandbox` when the host environment already provides isolation. +Trusted config loaded with `--config` or `CLAWPATCH_CONFIG` can pass primitive +Codex CLI config through `provider.codexConfig`. Repository-discovered config +files cannot set this field because Codex config can affect provider routing +and credential lookup. + Supported provider names today: - `codex`: local Codex CLI diff --git a/docs/configuration.md b/docs/configuration.md index 5857ac3..b0ca262 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -33,7 +33,8 @@ Default shape: "provider": { "name": "codex", "model": null, - "reasoningEffort": null + "reasoningEffort": null, + "codexConfig": {} }, "commands": { "typecheck": null, @@ -72,5 +73,12 @@ Environment overrides: - `CLAWPATCH_MODEL` - `CLAWPATCH_REASONING_EFFORT` +`provider.codexConfig` passes primitive values to Codex as `-c key=value`. +Only config loaded by `--config` or `CLAWPATCH_CONFIG` may set non-empty +Codex passthrough config. Auto-discovered repository and state config files +are rejected if they set it, because Codex config can change provider routing +and credential lookup. Keep secrets out of config files; use Codex provider +settings such as `env_key` to read an already-exported environment variable. + `git.commit` and `git.openPr` are reserved config fields. The current CLI does not commit or open PRs. diff --git a/docs/providers.md b/docs/providers.md index 0ce2106..b9fe560 100644 --- a/docs/providers.md +++ b/docs/providers.md @@ -54,6 +54,32 @@ When `reasoningEffort` is unset, Clawpatch does not pass a reasoning override and Codex uses its own configured default. Explicit values are passed to Codex as `model_reasoning_effort`. +Trusted Codex CLI config passthrough: + +```json +{ + "provider": { + "name": "codex", + "model": null, + "reasoningEffort": null, + "codexConfig": { + "model_provider": "local", + "model_providers.local.base_url": "https://example.invalid/v1", + "model_providers.local.env_key": "CLAWPATCH_CODEX_API_KEY" + } + } +} +``` + +Load a config like this with `clawpatch --config trusted-config.json ...` or +`CLAWPATCH_CONFIG=trusted-config.json`. Clawpatch rejects non-empty +`provider.codexConfig` from auto-discovered repository or state config files so +a checkout cannot silently redirect Codex provider routing or credential lookup. +Values are limited to strings, finite numbers, booleans, and `null`, then passed +as repeated `-c key=value` arguments before `--model` and reasoning overrides. +Do not place raw secrets in `codexConfig`; point Codex at an explicit env var +instead. + ## OpenCode The `opencode` provider shells out to the local [OpenCode CLI](https://opencode.ai/docs/cli/). diff --git a/src/app.ts b/src/app.ts index eaa0bdf..9798b57 100644 --- a/src/app.ts +++ b/src/app.ts @@ -100,7 +100,14 @@ export async function initCommand( const paths = statePaths(stateDir); await ensureStateDirs(paths); const project = await detectProject(context.root); - const detectedConfig = { ...config, commands: project.detected.commands }; + const detectedConfig = { + ...config, + provider: { + ...config.provider, + codexConfig: {}, + }, + commands: project.detected.commands, + }; const previous = await readProject(paths); if (previous !== null && flags["force"] !== true) { throw new ClawpatchError("project already initialized; use --force", 2, "already-initialized"); @@ -1348,7 +1355,16 @@ export async function doctorCommand( context: AppContext, flags: Record = {}, ): Promise { - const loaded = await loadProjectState(context).catch(() => null); + let loaded: Awaited> | null; + try { + loaded = await loadProjectState(context); + } catch (error) { + if (error instanceof ClawpatchError && error.code === "not-initialized") { + loaded = null; + } else { + throw error; + } + } const root = loaded?.root ?? context.root; const providerName = stringFlag(flags, "provider") ?? @@ -1959,6 +1975,7 @@ function providerOptions(config: ReturnType) { return { model: config.provider.model, reasoningEffort: config.provider.reasoningEffort, + codexConfig: config.provider.codexConfig, skipGitRepoCheck: config.provider.skipGitRepoCheck, }; } diff --git a/src/config.test.ts b/src/config.test.ts new file mode 100644 index 0000000..157f63d --- /dev/null +++ b/src/config.test.ts @@ -0,0 +1,94 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { join } from "node:path"; +import { defaultConfig, loadConfig } from "./config.js"; +import { fixtureRoot, testOptions, writeFixture } from "./test-helpers.js"; + +const originalConfig = process.env["CLAWPATCH_CONFIG"]; +const originalStateDir = process.env["CLAWPATCH_STATE_DIR"]; + +beforeEach(() => { + delete process.env["CLAWPATCH_CONFIG"]; + delete process.env["CLAWPATCH_STATE_DIR"]; +}); + +afterEach(() => { + if (originalConfig === undefined) { + delete process.env["CLAWPATCH_CONFIG"]; + } else { + process.env["CLAWPATCH_CONFIG"] = originalConfig; + } + if (originalStateDir === undefined) { + delete process.env["CLAWPATCH_STATE_DIR"]; + } else { + process.env["CLAWPATCH_STATE_DIR"] = originalStateDir; + } +}); + +function configWithCodexPassthrough() { + return { + ...defaultConfig(), + provider: { + ...defaultConfig().provider, + codexConfig: { + model_provider: "openai", + "model_providers.openai.env_key": "OPENAI_API_KEY", + }, + }, + }; +} + +describe("loadConfig", () => { + it("defaults Codex passthrough config to an empty object", async () => { + const root = await fixtureRoot("clawpatch-default-config-"); + + const config = await loadConfig(root, testOptions(root)); + + expect(config.provider.codexConfig).toEqual({}); + }); + + it("rejects Codex passthrough config from project config", async () => { + const root = await fixtureRoot("clawpatch-project-codex-config-"); + await writeFixture(root, "clawpatch.config.json", JSON.stringify(configWithCodexPassthrough())); + + await expect(loadConfig(root, testOptions(root))).rejects.toThrow( + /provider\.codexConfig may only be set/u, + ); + }); + + it("rejects Codex passthrough config from state-dir config", async () => { + const root = await fixtureRoot("clawpatch-state-codex-config-root-"); + const stateDir = await fixtureRoot("clawpatch-state-codex-config-"); + await writeFixture(stateDir, "config.json", JSON.stringify(configWithCodexPassthrough())); + + await expect(loadConfig(root, { ...testOptions(root), stateDir })).rejects.toThrow( + /provider\.codexConfig may only be set/u, + ); + }); + + it("accepts Codex passthrough config from --config", async () => { + const root = await fixtureRoot("clawpatch-explicit-codex-config-"); + const configPath = join(root, "trusted-config.json"); + await writeFixture(root, "trusted-config.json", JSON.stringify(configWithCodexPassthrough())); + + const config = await loadConfig(root, { ...testOptions(root), config: configPath }); + + expect(config.provider.codexConfig).toEqual({ + model_provider: "openai", + "model_providers.openai.env_key": "OPENAI_API_KEY", + }); + }); + + it("accepts Codex passthrough config from CLAWPATCH_CONFIG", async () => { + const root = await fixtureRoot("clawpatch-env-codex-config-"); + const configPath = join(root, "trusted-config.json"); + await writeFixture(root, "trusted-config.json", JSON.stringify(configWithCodexPassthrough())); + process.env["CLAWPATCH_CONFIG"] = configPath; + + const config = await loadConfig(root, testOptions(root)); + + expect(config.provider.codexConfig).toEqual({ + model_provider: "openai", + "model_providers.openai.env_key": "OPENAI_API_KEY", + }); + }); +}); diff --git a/src/config.ts b/src/config.ts index 2803727..a5517b3 100644 --- a/src/config.ts +++ b/src/config.ts @@ -22,6 +22,13 @@ export type GlobalOptions = { noInput: boolean; }; +type ConfigSource = "option" | "env" | "state-dir" | "project" | "state"; + +type ConfigDiscovery = { + path: string; + source: ConfigSource; +}; + export const defaultCommands: ProjectCommands = { typecheck: null, lint: null, @@ -47,6 +54,7 @@ export function defaultConfig(): ClawpatchConfig { name: "codex", model: null, reasoningEffort: null, + codexConfig: {}, }, commands: defaultCommands, review: { @@ -67,8 +75,9 @@ export function defaultConfig(): ClawpatchConfig { } export async function loadConfig(root: string, options: GlobalOptions): Promise { - const configPath = await discoverConfigPath(root, options); - const base = configPath === null ? defaultConfig() : await readJson(configPath, configSchema); + const discovery = await discoverConfigPath(root, options); + const base = discovery === null ? defaultConfig() : await readJson(discovery.path, configSchema); + assertTrustedCodexConfig(base, discovery?.source ?? null); return { ...base, stateDir: options.stateDir ?? process.env["CLAWPATCH_STATE_DIR"] ?? base.stateDir, @@ -102,23 +111,45 @@ function parseReasoningEffort(value: string | undefined) { ); } -async function discoverConfigPath(root: string, options: GlobalOptions): Promise { +function assertTrustedCodexConfig(config: ClawpatchConfig, source: ConfigSource | null): void { + if (Object.keys(config.provider.codexConfig).length === 0) { + return; + } + if (source === "option" || source === "env") { + return; + } + throw new ClawpatchError( + "provider.codexConfig may only be set from --config or CLAWPATCH_CONFIG; repository and state config cannot control Codex provider settings", + 2, + "invalid-usage", + ); +} + +async function discoverConfigPath( + root: string, + options: GlobalOptions, +): Promise { if (options.config !== undefined) { - return resolve(options.config); + return { path: resolve(options.config), source: "option" }; } if (process.env["CLAWPATCH_CONFIG"] !== undefined) { - return resolve(process.env["CLAWPATCH_CONFIG"]); + return { path: resolve(process.env["CLAWPATCH_CONFIG"]), source: "env" }; } const configuredStateDir = options.stateDir ?? process.env["CLAWPATCH_STATE_DIR"]; - const candidates = [ + const candidates: ConfigDiscovery[] = [ ...(configuredStateDir === undefined ? [] - : [join(resolve(root, configuredStateDir), "config.json")]), - join(root, "clawpatch.config.json"), - join(root, ".clawpatch", "config.json"), + : [ + { + path: join(resolve(root, configuredStateDir), "config.json"), + source: "state-dir" as const, + }, + ]), + { path: join(root, "clawpatch.config.json"), source: "project" }, + { path: join(root, ".clawpatch", "config.json"), source: "state" }, ]; for (const candidate of candidates) { - if (await pathExists(candidate)) { + if (await pathExists(candidate.path)) { return candidate; } } diff --git a/src/provider.test.ts b/src/provider.test.ts index c499cc4..00eb0d0 100644 --- a/src/provider.test.ts +++ b/src/provider.test.ts @@ -11,6 +11,7 @@ const { acpxFailureMessage, assertCursorRuntimeVersionAllowed, acpxPromptRetries, + addCodexConfigArgs, addCodexModelArgs, addCodexSandboxArgs, assertClaudeVersionAllowed, @@ -295,10 +296,63 @@ describe("Codex provider args", () => { addCodexModelArgs(args, { model: "gpt-5.5", reasoningEffort: "xhigh", + codexConfig: { + model_provider: "local", + "model_providers.local.base_url": "https://example.invalid/v1", + }, skipGitRepoCheck: false, }); - expect(args).toEqual(["exec", "--model", "gpt-5.5", "-c", 'model_reasoning_effort="xhigh"']); + expect(args).toEqual([ + "exec", + "-c", + 'model_provider="local"', + "-c", + 'model_providers.local.base_url="https://example.invalid/v1"', + "--model", + "gpt-5.5", + "-c", + 'model_reasoning_effort="xhigh"', + ]); + }); + + it("renders primitive Codex passthrough values in stable key order", () => { + const args = ["exec"]; + + addCodexConfigArgs(args, { + z_flag: true, + model_provider: "local", + "model_providers.local.max_retries": 2, + "model_providers.local.optional": null, + }); + + expect(args).toEqual([ + "exec", + "-c", + 'model_provider="local"', + "-c", + "model_providers.local.max_retries=2", + "-c", + "model_providers.local.optional=null", + "-c", + "z_flag=true", + ]); + }); + + it("rejects unsafe Codex passthrough keys", () => { + const args = ["exec"]; + + expect(() => addCodexConfigArgs(args, { "model provider": "local" })).toThrow( + /invalid Codex config key/u, + ); + }); + + it("rejects non-finite Codex passthrough numbers", () => { + const args = ["exec"]; + + expect(() => addCodexConfigArgs(args, { retries: Number.NaN })).toThrow( + /finite number required/u, + ); }); it("passes the Git repo check bypass to Codex when requested", () => { diff --git a/src/provider.ts b/src/provider.ts index 99ea224..70f5552 100644 --- a/src/provider.ts +++ b/src/provider.ts @@ -25,6 +25,7 @@ import { reviewInspectedSchema, reviewOutputSchema, revalidateOutputSchema, + type CodexConfig, type ReasoningEffort, } from "./types.js"; @@ -151,6 +152,7 @@ function parseOrThrow(schema: ZodType, input: unknown, label: string): T { export type ProviderOptions = { model: string | null; reasoningEffort: ReasoningEffort | null; + codexConfig?: CodexConfig; skipGitRepoCheck: boolean; }; @@ -2257,6 +2259,7 @@ function addCodexModelArgs(args: string[], options: ProviderOptions): void { if (options.skipGitRepoCheck) { args.push("--skip-git-repo-check"); } + addCodexConfigArgs(args, options.codexConfig ?? {}); if (options.model !== null) { args.push("--model", options.model); } @@ -2265,6 +2268,34 @@ function addCodexModelArgs(args: string[], options: ProviderOptions): void { } } +const CODEX_CONFIG_KEY = /^[A-Za-z0-9_][A-Za-z0-9_.-]*$/u; + +function addCodexConfigArgs(args: string[], config: CodexConfig): void { + for (const [key, value] of Object.entries(config).toSorted(([left], [right]) => + left.localeCompare(right), + )) { + args.push("-c", renderCodexConfigEntry(key, value)); + } +} + +function renderCodexConfigEntry(key: string, value: CodexConfig[string]): string { + if (!CODEX_CONFIG_KEY.test(key)) { + throw new ClawpatchError(`invalid Codex config key: ${key}`, 2, "invalid-usage"); + } + if (typeof value === "number" && !Number.isFinite(value)) { + throw new ClawpatchError( + `invalid Codex config value for ${key}: finite number required`, + 2, + "invalid-usage", + ); + } + const encoded = JSON.stringify(value); + if (encoded === undefined) { + throw new ClawpatchError(`invalid Codex config value for ${key}`, 2, "invalid-usage"); + } + return `${key}=${encoded}`; +} + const OPENCODE_READ_ONLY_PERMISSION = JSON.stringify({ bash: "deny", edit: "deny", @@ -2893,6 +2924,7 @@ function acpxPromptRetries(): number { export const __testing = { acpxFailureMessage, acpxPromptRetries, + addCodexConfigArgs, addCodexModelArgs, addCodexSandboxArgs, addClaudeModelArgs, diff --git a/src/types.ts b/src/types.ts index 94cefc1..c91304c 100644 --- a/src/types.ts +++ b/src/types.ts @@ -96,6 +96,12 @@ export const reasoningEffortSchema = z.enum(reasoningEfforts); export type ReasoningEffort = z.infer; +const codexConfigValueSchema = z.union([z.string(), z.number(), z.boolean(), z.null()]); + +export const codexConfigSchema = z.record(z.string(), codexConfigValueSchema); + +export type CodexConfig = z.infer; + export const projectRecordSchema = z.object({ schemaVersion: z.literal(1), projectId: z.string(), @@ -128,6 +134,7 @@ export const configSchema = z.object({ name: z.string(), model: z.string().nullable(), reasoningEffort: reasoningEffortSchema.nullable().optional().default(null), + codexConfig: codexConfigSchema.optional().default({}), }), commands: projectCommandsSchema, review: z.object({ diff --git a/src/workflow.test.ts b/src/workflow.test.ts index 978b9cd..d122401 100644 --- a/src/workflow.test.ts +++ b/src/workflow.test.ts @@ -671,6 +671,100 @@ describe("workflow", () => { }, ); + it.runIf(process.platform !== "win32")( + "passes trusted Codex config through the spawned provider process", + async () => { + const root = await fixtureRoot("clawpatch-codex-config-e2e-"); + await writeFixture( + root, + "package.json", + JSON.stringify({ + name: "codex-config", + bin: { app: "src/index.ts" }, + }), + ); + await writeFixture(root, "src/index.ts", "export const value = 'ok';\n"); + const trustedConfigPath = join(root, "trusted-config.json"); + await writeFixture( + root, + "trusted-config.json", + JSON.stringify({ + ...defaultConfig(), + provider: { + ...defaultConfig().provider, + codexConfig: { + model_provider: "local", + "model_providers.local.base_url": "https://example.invalid/v1", + "model_providers.local.env_key": "CLAWPATCH_TEST_API_KEY", + }, + }, + }), + ); + const binDir = join(root, "bin"); + const codexShim = join(binDir, "codex"); + const capturedArgsPath = join(root, "codex-args.json"); + await writeFixture( + root, + "bin/codex", + [ + "#!/usr/bin/env node", + 'const { writeFileSync } = require("node:fs");', + "const args = process.argv.slice(2);", + 'if (args.includes("--version")) { console.log("codex fake 0.130.0"); process.exit(0); }', + `writeFileSync(${JSON.stringify(capturedArgsPath)}, JSON.stringify(args), "utf8");`, + 'const outputIndex = args.indexOf("--output-last-message");', + "if (outputIndex === -1 || outputIndex + 1 >= args.length) {", + ' console.error("missing --output-last-message");', + " process.exit(2);", + "}", + "const payload = {", + " findings: [],", + ' inspected: { files: ["src/index.ts"], symbols: [], notes: ["trusted codex config"] },', + "};", + 'writeFileSync(args[outputIndex + 1], JSON.stringify(payload), "utf8");', + "", + ].join("\n"), + ); + await chmod(codexShim, 0o755); + const previousPath = process.env["PATH"]; + process.env["PATH"] = `${binDir}${delimiter}${previousPath ?? ""}`; + try { + const context = await makeContext({ ...testOptions(root), config: trustedConfigPath }); + + await initCommand(context, {}); + const persistedConfig = JSON.parse( + await readFile(join(root, ".clawpatch", "config.json"), "utf8"), + ) as { provider?: { codexConfig?: unknown } }; + expect(persistedConfig.provider?.codexConfig).toEqual({}); + expect((await loadConfig(root, testOptions(root))).provider.codexConfig).toEqual({}); + await mapCommand(context); + await reviewCommand(context, { limit: "1" }); + const capturedArgs = JSON.parse(await readFile(capturedArgsPath, "utf8")) as string[]; + const codexConfigArgs: string[] = []; + for (let index = 0; index < capturedArgs.length; index += 1) { + if (capturedArgs[index] === "-c") { + const value = capturedArgs[index + 1]; + if (value !== undefined) { + codexConfigArgs.push(value); + } + } + } + + expect(codexConfigArgs).toEqual([ + 'model_provider="local"', + 'model_providers.local.base_url="https://example.invalid/v1"', + 'model_providers.local.env_key="CLAWPATCH_TEST_API_KEY"', + ]); + } finally { + if (previousPath === undefined) { + delete process.env["PATH"]; + } else { + process.env["PATH"] = previousPath; + } + } + }, + ); + it.runIf(process.platform !== "win32")( "times out wedged codex exec review children", async () => { @@ -2124,6 +2218,28 @@ describe("workflow", () => { } }); + it("rejects untrusted Codex passthrough config in doctor", async () => { + const root = await fixtureRoot("clawpatch-doctor-untrusted-codex-config-"); + await writeFixture( + root, + "clawpatch.config.json", + JSON.stringify({ + ...defaultConfig(), + provider: { + ...defaultConfig().provider, + codexConfig: { + model_provider: "local", + }, + }, + }), + ); + const context = await makeContext(testOptions(root)); + + await expect(doctorCommand(context, { provider: "mock" })).rejects.toThrow( + /provider\.codexConfig may only be set/u, + ); + }); + it("allows fix dry-run when only the default state dir is dirty", async () => { const root = await fixtureRoot("clawpatch-state-dirty-"); await runCommand(