fix: harden proxy config and CLI input validation

src/main.ts

@@ -15,6 +15,7 @@ import {
 import { dropSudo } from "./dropSudo";
 import { ensureActorHasWriteAccess } from "./checkActorPermissions";
 import parseArgsStringToArgv from "string-argv";
+import { parsePort } from "./ports";
 import { writeProxyConfig } from "./writeProxyConfig";
 import { checkOutput } from "./checkOutput";
 
@@ -80,7 +81,7 @@ export async function main() {
       "Write the OpenAI Proxy model provider config into CODEX_HOME/config.toml"
     )
     .requiredOption("--codex-home <DIRECTORY>", "Path to Codex home directory")
-    .requiredOption("--port <port>", "Proxy server port", parseIntStrict)
+    .requiredOption("--port <port>", "Proxy server port", parsePort)
     .requiredOption(
       "--safety-strategy <strategy>",
       "Safety strategy to use. One of 'drop-sudo', 'read-only', 'unprivileged-user', or 'unsafe'."
@@ -307,21 +308,28 @@ export async function main() {
   program.parse();
 }
 
-function parseIntStrict(value: string): number {
-  const parsed = parseInt(value, 10);
-  if (isNaN(parsed)) {
-    throw new Error(`Invalid integer: ${value}`);
-  }
-  return parsed;
-}
-
 function parseExtraArgs(value: string): Array<string> {
   if (value.length === 0) {
     return [];
   }
 
   if (value.startsWith("[")) {
-    return JSON.parse(value);
+    let parsed: unknown;
+    try {
+      parsed = JSON.parse(value);
+    } catch (error) {
+      const message =
+        error instanceof Error ? error.message : "unknown JSON parse error";
+      throw new Error(`Invalid --extra-args JSON: ${message}`);
+    }
+
+    if (!Array.isArray(parsed) || !parsed.every((entry) => typeof entry === "string")) {
+      throw new Error(
+        "Invalid --extra-args JSON: expected a JSON array of strings."
+      );
+    }
+
+    return parsed;
   } else {
     return parseArgsStringToArgv(value);
   }

src/ports.ts

@@ -0,0 +1,49 @@
+const MIN_PORT = 1;
+const MAX_PORT = 65535;
+
+export function isValidPort(value: unknown): value is number {
+  return (
+    typeof value === "number" &&
+    Number.isInteger(value) &&
+    value >= MIN_PORT &&
+    value <= MAX_PORT
+  );
+}
+
+export function parsePort(value: string): number {
+  const trimmed = value.trim();
+  if (!/^\d+$/.test(trimmed)) {
+    throw invalidPortError(value);
+  }
+
+  const port = Number.parseInt(trimmed, 10);
+  if (!isValidPort(port)) {
+    throw invalidPortError(value);
+  }
+
+  return port;
+}
+
+export function ensureValidPort(value: unknown): number {
+  if (!isValidPort(value)) {
+    throw invalidPortError(formatPortValue(value));
+  }
+
+  return value;
+}
+
+function invalidPortError(value: string): Error {
+  return new Error(
+    `Invalid port: ${value}. Expected an integer between ${MIN_PORT} and ${MAX_PORT}.`
+  );
+}
+
+function formatPortValue(value: unknown): string {
+  if (typeof value === "string") {
+    return JSON.stringify(value);
+  }
+  if (value === undefined) {
+    return "undefined";
+  }
+  return String(value);
+}

src/readServerInfo.ts

@@ -1,5 +1,6 @@
 import * as core from "@actions/core";
 import * as fs from "fs/promises";
+import { ensureValidPort } from "./ports";
 
 /**
  * In theory, this is not called until `serverInfoFile` is non-empty, but we
@@ -10,11 +11,9 @@ export async function readServerInfo(serverInfoFile: string): Promise<void> {
     try {
       const contents = await fs.readFile(serverInfoFile, { encoding: "utf8" });
       const { port } = JSON.parse(contents);
-      if (typeof port !== "number") {
-        continue;
-      }
+      const parsedPort = ensureValidPort(port);
 
-      core.setOutput("port", port.toString());
+      core.setOutput("port", parsedPort.toString());
       return;
     } catch (error) {
       console.error(`Error reading server info: ${error}`);

src/runCodexExec.ts

@@ -188,7 +188,7 @@ export async function runCodexExec({
       });
     });
   } finally {
-    await cleanupOutputSchema(resolvedOutputSchema);
+    await cleanupOutputSchema(resolvedOutputSchema, runAsUser);
   }
 }
 
@@ -280,14 +280,15 @@ async function resolveOutputSchema(
     case "inline": {
       const dir = await createTempDir("codex-output-schema-", runAsUser);
       const file = path.join(dir, "schema.json");
-      await writeFile(file, schema.content);
+      await writeTempFile(file, schema.content, runAsUser);
       return { type: "temp", file, dir };
     }
   }
 }
 
 async function cleanupOutputSchema(
-  schema: ResolvedOutputSchema | null
+  schema: ResolvedOutputSchema | null,
+  runAsUser: string | null
 ): Promise<void> {
   if (schema == null) {
     return;
@@ -297,11 +298,40 @@ async function cleanupOutputSchema(
     case "explicit":
       return;
     case "temp":
-      await rm(schema.dir, { recursive: true, force: true });
+      if (runAsUser == null) {
+        await rm(schema.dir, { recursive: true, force: true });
+      } else {
+        await checkOutput(["sudo", "rm", "-rf", schema.dir]);
+      }
       return;
   }
 }
 
+async function writeTempFile(
+  file: string,
+  contents: string,
+  runAsUser: string | null
+): Promise<void> {
+  if (runAsUser == null) {
+    await writeFile(file, contents);
+    return;
+  }
+
+  const stagingDir = await mkdtemp(
+    path.join(os.tmpdir(), "codex-output-schema-staging-")
+  );
+  const stagingFile = path.join(stagingDir, path.basename(file));
+
+  try {
+    await writeFile(stagingFile, contents);
+    await checkOutput(["sudo", "cp", stagingFile, file]);
+    await checkOutput(["sudo", "chown", runAsUser, file]);
+    await checkOutput(["sudo", "chmod", "600", file]);
+  } finally {
+    await rm(stagingDir, { recursive: true, force: true });
+  }
+}
+
 async function createTempDir(
   prefix: string,
   runAsUser: string | null

src/writeProxyConfig.ts

@@ -5,6 +5,8 @@ import { SafetyStrategy } from "./runCodexExec";
 import { checkOutput } from "./checkOutput";
 
 const MODEL_PROVIDER = "codex-action-responses-proxy";
+const MANAGED_BLOCK_START = "# BEGIN codex-action managed proxy config";
+const MANAGED_BLOCK_END = "# END codex-action managed proxy config";
 
 export async function writeProxyConfig(
   codexHome: string,
@@ -20,22 +22,7 @@ export async function writeProxyConfig(
     existing = "";
   }
 
-  const header = `# Added by codex-action.
-model_provider = "${MODEL_PROVIDER}"
-
-
-`;
-  const table = `
-
-# Added by codex-action.
-[model_providers.${MODEL_PROVIDER}]
-name = "Codex Action Responses Proxy"
-base_url = "http://127.0.0.1:${port}/v1"
-wire_api = "responses"
-`;
-
-  // Prepend model_provider at the very top.
-  let output = `${header}${existing}${table}`;
+  const output = mergeProxyConfig(existing, port);
 
   if (safetyStrategy === "unprivileged-user") {
     // We know we have already created the CODEX_HOME directory, but it is owned
@@ -53,3 +40,86 @@ wire_api = "responses"
     await fs.writeFile(configPath, output, "utf8");
   }
 }
+
+function mergeProxyConfig(existing: string, port: number): string {
+  const newline = existing.includes("\r\n") ? "\r\n" : "\n";
+  const managed = buildManagedBlock(port, newline);
+  const cleaned = stripManagedProxyConfig(existing).trim();
+
+  if (cleaned.length === 0) {
+    return `${managed}${newline}`;
+  }
+
+  return `${managed}${newline}${newline}${cleaned}${newline}`;
+}
+
+function buildManagedBlock(port: number, newline: string): string {
+  return [
+    MANAGED_BLOCK_START,
+    `model_provider = "${MODEL_PROVIDER}"`,
+    "",
+    `[model_providers.${MODEL_PROVIDER}]`,
+    'name = "Codex Action Responses Proxy"',
+    `base_url = "http://127.0.0.1:${port}/v1"`,
+    'wire_api = "responses"',
+    MANAGED_BLOCK_END,
+  ].join(newline);
+}
+
+function stripManagedProxyConfig(existing: string): string {
+  const newline = existing.includes("\r\n") ? "\r\n" : "\n";
+  const normalized = existing.replace(/\r\n/g, "\n");
+  const withoutMarked = normalized.replace(
+    new RegExp(
+      `${escapeRegExp(MANAGED_BLOCK_START)}\\n[\\s\\S]*?${escapeRegExp(MANAGED_BLOCK_END)}(?:\\n+)?`,
+      "g"
+    ),
+    ""
+  );
+
+  const lines = withoutMarked.split("\n");
+  const kept: Array<string> = [];
+
+  for (let index = 0; index < lines.length; ) {
+    const line = lines[index];
+
+    if (line === `model_provider = "${MODEL_PROVIDER}"`) {
+      dropLegacyComment(kept);
+      index += 1;
+      while (index < lines.length && lines[index].trim().length === 0) {
+        index += 1;
+      }
+      continue;
+    }
+
+    if (line === `[model_providers.${MODEL_PROVIDER}]`) {
+      dropLegacyComment(kept);
+      index += 1;
+      while (index < lines.length && !lines[index].trim().startsWith("[")) {
+        index += 1;
+      }
+      while (index < lines.length && lines[index].trim().length === 0) {
+        index += 1;
+      }
+      continue;
+    }
+
+    kept.push(line);
+    index += 1;
+  }
+
+  return kept.join("\n").replace(/\n{3,}/g, "\n\n").replace(/\n+$/g, "").replace(/\n/g, newline);
+}
+
+function dropLegacyComment(lines: Array<string>): void {
+  while (lines.length > 0 && lines[lines.length - 1].trim().length === 0) {
+    lines.pop();
+  }
+  if (lines[lines.length - 1] === "# Added by codex-action.") {
+    lines.pop();
+  }
+}
+
+function escapeRegExp(value: string): string {
+  return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
+}

test/cliValidation.test.mjs

@@ -0,0 +1,73 @@
+import assert from "node:assert/strict";
+import { spawnSync } from "node:child_process";
+import { test } from "node:test";
+import { fileURLToPath } from "node:url";
+
+const mainPath = fileURLToPath(new URL("../dist/main.js", import.meta.url));
+
+function runCli(args) {
+  return spawnSync(process.execPath, [mainPath, ...args], {
+    encoding: "utf8",
+  });
+}
+
+function runRunCodexExec(extraArgs) {
+  return runCli([
+    "run-codex-exec",
+    "--prompt",
+    "hello",
+    "--prompt-file",
+    "",
+    "--codex-home",
+    "",
+    "--cd",
+    process.cwd(),
+    "--extra-args",
+    extraArgs,
+    "--output-file",
+    "",
+    "--output-schema-file",
+    "",
+    "--output-schema",
+    "",
+    "--sandbox",
+    "workspace-write",
+    "--model",
+    "",
+    "--effort",
+    "",
+    "--safety-strategy",
+    "unsafe",
+    "--codex-user",
+    "",
+  ]);
+}
+
+test("write-proxy-config rejects out-of-range ports", () => {
+  const result = runCli([
+    "write-proxy-config",
+    "--codex-home",
+    "/tmp/codex-home",
+    "--port",
+    "65536",
+    "--safety-strategy",
+    "unsafe",
+  ]);
+
+  assert.notEqual(result.status, 0);
+  assert.match(result.stderr, /Invalid port: 65536/);
+});
+
+test("run-codex-exec wraps malformed extra args JSON", () => {
+  const result = runRunCodexExec('["--model"');
+
+  assert.notEqual(result.status, 0);
+  assert.match(result.stderr, /Invalid --extra-args JSON:/);
+});
+
+test("run-codex-exec rejects non-string JSON args", () => {
+  const result = runRunCodexExec('["--model", 1]');
+
+  assert.notEqual(result.status, 0);
+  assert.match(result.stderr, /expected a JSON array of strings/);
+});