From 671314ddcca557ae7f19dead428469a10ca0217e Mon Sep 17 00:00:00 2001 From: Som Samantray Date: Thu, 24 Sep 2026 20:23:47 +0530 Subject: [PATCH] fix(codex): guard mktemp failures in codex mode templates Every mktemp assignment in the three codex mode templates (review-mode, consult-mode, challenge-mode) was unguarded. bin/gstack-paths deliberately best-effort-creates $TMP_ROOT and defers failure discovery to the caller's own write attempt, so an unwritable $TMP_ROOT made these mktemp calls silently yield an empty variable. The next command then redirected stderr to 2>"", which failed before codex even ran, misattributing the failure to codex instead of the missing temp file. Guard every mktemp assignment with a loud exit, matching the existing repo convention (redact-doc.ts, ship/pr-body.md.tmpl, gstack-upgrade/SKILL.md.tmpl, supabase/verify-rls.sh, build-app.sh). challenge-mode's ${TMPERR:-$(mktemp ...)} wrapper is preserved unchanged; verified live that the guard composes correctly with the parameter-expansion default. Regenerated codex/sections/*.md from the updated .tmpl sources per the repo's SKILL.md generation workflow. Fixes #2881 --- codex/sections/challenge-mode.md | 2 +- codex/sections/challenge-mode.md.tmpl | 2 +- codex/sections/consult-mode.md | 4 +- codex/sections/consult-mode.md.tmpl | 4 +- codex/sections/review-mode.md | 4 +- codex/sections/review-mode.md.tmpl | 4 +- ...regression-pr1169-mktemp-fallbacks.test.ts | 42 +++++++++++++++++++ 7 files changed, 52 insertions(+), 10 deletions(-) diff --git a/codex/sections/challenge-mode.md b/codex/sections/challenge-mode.md index bf74041902..f9584b972b 100644 --- a/codex/sections/challenge-mode.md +++ b/codex/sections/challenge-mode.md @@ -34,7 +34,7 @@ if [ -z "$PYTHON_CMD" ]; then fi # Fix 1+2: wrap with timeout (gtimeout/timeout fallback chain via probe helper), # capture stderr to $TMPERR for auth error detection (was: 2>/dev/null). -TMPERR=${TMPERR:-$(mktemp "$TMP_ROOT/codex-err-XXXXXX")} +TMPERR=${TMPERR:-$(mktemp "$TMP_ROOT/codex-err-XXXXXX")} || { echo "ERROR: mktemp failed — cannot capture codex stderr; refusing to run codex challenge unmonitored." >&2; exit 1; } _gstack_codex_timeout_wrapper 600 codex exec "" -C "$_REPO_ROOT" -s read-only -c "model=\"${GSTACK_CODEX_MODEL:-gpt-6-astra}\"" -c 'model_reasoning_effort="high"' -c 'web_search="cached"' --json < /dev/null 2>"$TMPERR" | PYTHONUNBUFFERED=1 "$PYTHON_CMD" -u -c " import sys, json turn_completed_count = 0 diff --git a/codex/sections/challenge-mode.md.tmpl b/codex/sections/challenge-mode.md.tmpl index ee51379c4f..e5b2eefb1c 100644 --- a/codex/sections/challenge-mode.md.tmpl +++ b/codex/sections/challenge-mode.md.tmpl @@ -32,7 +32,7 @@ if [ -z "$PYTHON_CMD" ]; then fi # Fix 1+2: wrap with timeout (gtimeout/timeout fallback chain via probe helper), # capture stderr to $TMPERR for auth error detection (was: 2>/dev/null). -TMPERR=${TMPERR:-$(mktemp "$TMP_ROOT/codex-err-XXXXXX")} +TMPERR=${TMPERR:-$(mktemp "$TMP_ROOT/codex-err-XXXXXX")} || { echo "ERROR: mktemp failed — cannot capture codex stderr; refusing to run codex challenge unmonitored." >&2; exit 1; } _gstack_codex_timeout_wrapper 600 codex exec "" -C "$_REPO_ROOT" -s read-only {{CODEX_MODEL_CONFIG_FLAG}} -c 'model_reasoning_effort="high"' {{CODEX_WEB_SEARCH_FLAG}} --json < /dev/null 2>"$TMPERR" | PYTHONUNBUFFERED=1 "$PYTHON_CMD" -u -c " import sys, json turn_completed_count = 0 diff --git a/codex/sections/consult-mode.md b/codex/sections/consult-mode.md index 48c75b7909..8a76b8b04f 100644 --- a/codex/sections/consult-mode.md +++ b/codex/sections/consult-mode.md @@ -18,8 +18,8 @@ B) Start a new conversation 2. Create temp files: ```bash -TMPRESP=$(mktemp "$TMP_ROOT/codex-resp-XXXXXX") -TMPERR=$(mktemp "$TMP_ROOT/codex-err-XXXXXX") +TMPRESP=$(mktemp "$TMP_ROOT/codex-resp-XXXXXX") || { echo "ERROR: mktemp failed — cannot capture codex's response." >&2; exit 1; } +TMPERR=$(mktemp "$TMP_ROOT/codex-err-XXXXXX") || { echo "ERROR: mktemp failed — cannot capture codex stderr; refusing to run codex consult unmonitored." >&2; exit 1; } ``` 3. **Plan review auto-detection:** If the user's prompt is about reviewing a plan, diff --git a/codex/sections/consult-mode.md.tmpl b/codex/sections/consult-mode.md.tmpl index aa668a1111..e90dcd1c76 100644 --- a/codex/sections/consult-mode.md.tmpl +++ b/codex/sections/consult-mode.md.tmpl @@ -16,8 +16,8 @@ B) Start a new conversation 2. Create temp files: ```bash -TMPRESP=$(mktemp "$TMP_ROOT/codex-resp-XXXXXX") -TMPERR=$(mktemp "$TMP_ROOT/codex-err-XXXXXX") +TMPRESP=$(mktemp "$TMP_ROOT/codex-resp-XXXXXX") || { echo "ERROR: mktemp failed — cannot capture codex's response." >&2; exit 1; } +TMPERR=$(mktemp "$TMP_ROOT/codex-err-XXXXXX") || { echo "ERROR: mktemp failed — cannot capture codex stderr; refusing to run codex consult unmonitored." >&2; exit 1; } ``` 3. **Plan review auto-detection:** If the user's prompt is about reviewing a plan, diff --git a/codex/sections/review-mode.md b/codex/sections/review-mode.md index 84dc1f6172..c0376fd760 100644 --- a/codex/sections/review-mode.md +++ b/codex/sections/review-mode.md @@ -26,7 +26,7 @@ instructions get their own path (below). 1. Create temp files for output capture: ```bash -TMPERR=$(mktemp "$TMP_ROOT/codex-err-XXXXXX") +TMPERR=$(mktemp "$TMP_ROOT/codex-err-XXXXXX") || { echo "ERROR: mktemp failed — cannot capture codex stderr; refusing to run codex review unmonitored." >&2; exit 1; } ``` 2. Run the review. No prompt argument — scope comes from `--base` (or `--commit ` @@ -81,7 +81,7 @@ _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" cd "$_REPO_ROOT" ~/.claude/skills/gstack/bin/gstack-review-log --start codex-review _USER_INSTRUCTIONS="" -_PROMPT_FILE=$(mktemp "$TMP_ROOT/codex-prompt-XXXXXX") +_PROMPT_FILE=$(mktemp "$TMP_ROOT/codex-prompt-XXXXXX") || { echo "ERROR: mktemp failed — cannot write the codex review prompt file." >&2; exit 1; } { printf '%s\n' "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .claude/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. Do NOT modify agents/openai.yaml. Stay focused on repository code only." printf '\nCustom focus: %s\n\n' "$_USER_INSTRUCTIONS" diff --git a/codex/sections/review-mode.md.tmpl b/codex/sections/review-mode.md.tmpl index 7f1e92d13e..6a6c08e2e2 100644 --- a/codex/sections/review-mode.md.tmpl +++ b/codex/sections/review-mode.md.tmpl @@ -24,7 +24,7 @@ instructions get their own path (below). 1. Create temp files for output capture: ```bash -TMPERR=$(mktemp "$TMP_ROOT/codex-err-XXXXXX") +TMPERR=$(mktemp "$TMP_ROOT/codex-err-XXXXXX") || { echo "ERROR: mktemp failed — cannot capture codex stderr; refusing to run codex review unmonitored." >&2; exit 1; } ``` 2. Run the review. No prompt argument — scope comes from `--base` (or `--commit ` @@ -79,7 +79,7 @@ _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" cd "$_REPO_ROOT" ~/.claude/skills/gstack/bin/gstack-review-log --start codex-review _USER_INSTRUCTIONS="" -_PROMPT_FILE=$(mktemp "$TMP_ROOT/codex-prompt-XXXXXX") +_PROMPT_FILE=$(mktemp "$TMP_ROOT/codex-prompt-XXXXXX") || { echo "ERROR: mktemp failed — cannot write the codex review prompt file." >&2; exit 1; } { printf '%s\n' "IMPORTANT: Do NOT read or execute any files under ~/.claude/, ~/.agents/, .claude/skills/, or agents/. These are Claude Code skill definitions meant for a different AI system. Do NOT modify agents/openai.yaml. Stay focused on repository code only." printf '\nCustom focus: %s\n\n' "$_USER_INSTRUCTIONS" diff --git a/test/regression-pr1169-mktemp-fallbacks.test.ts b/test/regression-pr1169-mktemp-fallbacks.test.ts index 02cfd78f0c..6128251dbf 100644 --- a/test/regression-pr1169-mktemp-fallbacks.test.ts +++ b/test/regression-pr1169-mktemp-fallbacks.test.ts @@ -127,6 +127,48 @@ echo "SHOULD NOT REACH: $TMP_DIR"`; }); }); +// #2881: the three codex mode templates create $TMP_ROOT-scoped temp files +// (stderr captures, prompt files) with no failure guard. bin/gstack-paths +// deliberately best-effort-creates $TMP_ROOT ("the caller will discover +// [failure] on their own write attempt" — bin/gstack-paths:78-79), so an +// unwritable $TMP_ROOT makes these mktemp calls silently yield an empty +// variable. The next command then redirects stderr to `2>""`, which fails +// before codex even runs, and the error is misattributed to codex instead of +// to the missing temp file. Guards must abort loudly, matching the shape +// already pinned above for the five other call sites in this file and in +// test/regression-pr1169-build-app-sed.test.ts. +describe("#2881: codex mode template mktemp guards", () => { + test("review-mode.md.tmpl guards both mktemp assignments (TMPERR, _PROMPT_FILE)", () => { + const body = readScript("codex/sections/review-mode.md.tmpl"); + expect(body).toMatch(/TMPERR=\$\(mktemp "\$TMP_ROOT\/codex-err-XXXXXX"\)\s*\|\|\s*\{[^}]*exit 1/); + expect(body).toMatch(/_PROMPT_FILE=\$\(mktemp "\$TMP_ROOT\/codex-prompt-XXXXXX"\)\s*\|\|\s*\{[^}]*exit 1/); + }); + + test("consult-mode.md.tmpl guards both mktemp assignments (TMPRESP, TMPERR)", () => { + const body = readScript("codex/sections/consult-mode.md.tmpl"); + expect(body).toMatch(/TMPRESP=\$\(mktemp "\$TMP_ROOT\/codex-resp-XXXXXX"\)\s*\|\|\s*\{[^}]*exit 1/); + expect(body).toMatch(/TMPERR=\$\(mktemp "\$TMP_ROOT\/codex-err-XXXXXX"\)\s*\|\|\s*\{[^}]*exit 1/); + }); + + test("challenge-mode.md.tmpl guards its mktemp assignment and preserves the ${TMPERR:-...} wrapper", () => { + const body = readScript("codex/sections/challenge-mode.md.tmpl"); + expect(body).toMatch( + /TMPERR=\$\{TMPERR:-\$\(mktemp "\$TMP_ROOT\/codex-err-XXXXXX"\)\}\s*\|\|\s*\{[^}]*exit 1/, + ); + }); + + test("runtime: the guarded assignment aborts when mktemp fails, without invoking the guarded command", () => { + const { spawnSync } = require("node:child_process") as typeof import("node:child_process"); + const script = `mktemp() { return 1; } +TMPERR=$(mktemp "$TMP_ROOT/codex-err-XXXXXX") || { echo "ERROR: mktemp failed — cannot capture codex stderr; refusing to run codex review unmonitored." >&2; exit 1; } +echo "SHOULD NOT REACH: $TMPERR"`; + const r = spawnSync("bash", ["-c", script], { encoding: "utf-8", timeout: 10_000 }); + expect(r.status).toBe(1); + expect(r.stderr).toContain("mktemp failed"); + expect(r.stdout).not.toContain("SHOULD NOT REACH"); + }); +}); + describe("PR #1169 bug #5: supabase/verify-rls.sh mktemp fallback", () => { const SCRIPT = "supabase/verify-rls.sh";