From 3e55fb8d8ae733c3623ae88ebb82255f92d053ce Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 3 Oct 2026 20:48:58 +0000 Subject: [PATCH] G3: give every logging-bin mktemp a ${TMPDIR:-/tmp} template Bare mktemp, mktemp -t and literal /tmp templates in the learnings, question and preference logs (plus jsonl-merge, distill, the two dashboards, /ship's PR body and /document-release's run dir) now use "${TMPDIR:-/tmp}/.XXXXXX", the form the INV-3 lint accepts. When the temp file cannot be created, the learnings, question and preference logs now say the entry was not recorded and how to fix TMPDIR instead of exiting with only mktemp's own error. Three tests that pinned the bare PR_BODY_FILE=$(mktemp) and one that pinned the /tmp run dir migrate. GNU and BSD mktemp already honor TMPDIR without a template, so the macOS-sandbox drop in #2952 is unverified here; the template makes the temp location explicit on every mktemp. Co-authored-by: Davon Johnson --- bin/gstack-community-dashboard | 2 +- bin/gstack-distill-free-text | 2 +- bin/gstack-jsonl-merge | 2 +- bin/gstack-learnings-log | 2 +- bin/gstack-question-log | 2 +- bin/gstack-question-preference | 2 +- bin/gstack-security-dashboard | 2 +- .../sections/release-body.md.tmpl | 2 +- ship/sections/pr-body.md.tmpl | 2 +- test/document-skills-redaction.test.ts | 2 +- test/learnings.test.ts | 33 ++++++++++++++++++ ...regression-pr1169-mktemp-fallbacks.test.ts | 34 +++++++++++++++++-- test/ship-publication-gates.test.ts | 2 +- test/workflow-excerpt.test.ts | 2 +- 14 files changed, 77 insertions(+), 14 deletions(-) diff --git a/bin/gstack-community-dashboard b/bin/gstack-community-dashboard index 2b40b25b7..1a336c96a 100755 --- a/bin/gstack-community-dashboard +++ b/bin/gstack-community-dashboard @@ -36,7 +36,7 @@ fi # ─── Fetch aggregated stats from edge function ──────────────── # HTTP status captured (#1947): a backend failure must read as "unknown", # never as a healthy "Weekly active installs: 0". -TMPBODY="$(mktemp)" +TMPBODY="$(mktemp "${TMPDIR:-/tmp}/gstack-community.XXXXXX")" trap 'rm -f "$TMPBODY"' EXIT SUPA_HOST="${SUPABASE_URL#*://}"; SUPA_HOST="${SUPA_HOST%%/*}" HTTP_CODE="$(_receipted_curl open community-dashboard "$SUPA_HOST" community-pulse-fetch "user-invoked dashboard" --no-payload \ diff --git a/bin/gstack-distill-free-text b/bin/gstack-distill-free-text index 86cbd8918..0a2fdddec 100755 --- a/bin/gstack-distill-free-text +++ b/bin/gstack-distill-free-text @@ -129,7 +129,7 @@ fi # Heredoc into temp file (avoids $(cat <<'PROMPT'...) which choked the # bash parser on apostrophes elsewhere in the script). -DISTILL_PROMPT_FILE=$(mktemp) +DISTILL_PROMPT_FILE=$(mktemp "${TMPDIR:-/tmp}/gstack-distill.XXXXXX") trap 'rm -f "$DISTILL_PROMPT_FILE"' EXIT cat > "$DISTILL_PROMPT_FILE" <<'PROMPT' You are gstack dream-cycle distiller. Below are free-text responses the diff --git a/bin/gstack-jsonl-merge b/bin/gstack-jsonl-merge index 9c4e22688..ce439d2ac 100755 --- a/bin/gstack-jsonl-merge +++ b/bin/gstack-jsonl-merge @@ -45,7 +45,7 @@ BASE="$1" OURS="$2" THEIRS="$3" -TMP=$(mktemp /tmp/gstack-jsonl-merge.XXXXXX) || exit 1 +TMP=$(mktemp "${TMPDIR:-/tmp}/gstack-jsonl-merge.XXXXXX") || exit 1 trap 'rm -f "$TMP" 2>/dev/null || true' EXIT python3 - "$BASE" "$OURS" "$THEIRS" > "$TMP" <<'PYEOF' diff --git a/bin/gstack-learnings-log b/bin/gstack-learnings-log index fd0efaa56..4c4a2b6e6 100755 --- a/bin/gstack-learnings-log +++ b/bin/gstack-learnings-log @@ -24,7 +24,7 @@ INPUT="$1" # Validate and sanitize input. Errors surface (#1950): stderr is captured and # printed on failure instead of swallowed — a silent exit 1 here cost Windows # users every AI-logged learning. -TMPERR=$(mktemp) +TMPERR=$(mktemp "${TMPDIR:-/tmp}/gstack-learnings-log.XXXXXX") || { echo "gstack-learnings-log: could not create a temp file in ${TMPDIR:-/tmp}, so the learning was not recorded. Fix: point TMPDIR at a writable directory." >&2; exit 1; } trap 'rm -f "$TMPERR"' EXIT set +e VALIDATED=$(printf '%s' "$INPUT" | GSTACK_LIB_DIR="$SCRIPT_DIR/../lib" bun -e " diff --git a/bin/gstack-question-log b/bin/gstack-question-log index d769bf371..d0d729ca5 100755 --- a/bin/gstack-question-log +++ b/bin/gstack-question-log @@ -43,7 +43,7 @@ mkdir -p "$GSTACK_HOME/projects/$SLUG" INPUT="$1" # Validate and enrich from registry. -TMPERR=$(mktemp) +TMPERR=$(mktemp "${TMPDIR:-/tmp}/gstack-question-log.XXXXXX") || { echo "gstack-question-log: could not create a temp file in ${TMPDIR:-/tmp}, so the question was not logged. Fix: point TMPDIR at a writable directory." >&2; exit 1; } trap 'rm -f "$TMPERR"' EXIT set +e VALIDATED=$(printf '%s' "$INPUT" | GSTACK_LIB_DIR="$SCRIPT_DIR/../lib" bun -e " diff --git a/bin/gstack-question-preference b/bin/gstack-question-preference index 51de1347e..d4452d04e 100755 --- a/bin/gstack-question-preference +++ b/bin/gstack-question-preference @@ -143,7 +143,7 @@ do_write() { fi ensure_file local TMPERR - TMPERR=$(mktemp) + TMPERR=$(mktemp "${TMPDIR:-/tmp}/gstack-question-preference.XXXXXX") || { echo "gstack-question-preference: could not create a temp file in ${TMPDIR:-/tmp}, so the preference was not saved. Fix: point TMPDIR at a writable directory." >&2; return 1; } # Use function-local cleanup via RETURN trap so variable lookup only happens # while the function is on the stack (avoids EXIT-trap unbound-var race). trap "rm -f '$TMPERR'" RETURN diff --git a/bin/gstack-security-dashboard b/bin/gstack-security-dashboard index 864fd8f94..5d2d355a0 100755 --- a/bin/gstack-security-dashboard +++ b/bin/gstack-security-dashboard @@ -47,7 +47,7 @@ fi # Fetch with the HTTP status captured (#1947). A backend failure must read # as "unknown", never as a healthy "0 attacks" — fake zeros on a security # surface are indistinguishable from good news. -TMPBODY="$(mktemp)" +TMPBODY="$(mktemp "${TMPDIR:-/tmp}/gstack-security.XXXXXX")" trap 'rm -f "$TMPBODY"' EXIT SUPA_HOST="${SUPABASE_URL#*://}"; SUPA_HOST="${SUPA_HOST%%/*}" HTTP_CODE="$(_receipted_curl open security-dashboard "$SUPA_HOST" community-pulse-fetch "user-invoked dashboard" --no-payload \ diff --git a/document-release/sections/release-body.md.tmpl b/document-release/sections/release-body.md.tmpl index 41d257d2a..3e14f84f1 100644 --- a/document-release/sections/release-body.md.tmpl +++ b/document-release/sections/release-body.md.tmpl @@ -245,7 +245,7 @@ near the write-back. 1. Create a private run directory, then replace **every** `` below with its printed absolute path. This literal path survives separate shell calls; do not substitute `$$`. ```bash -mktemp -d /tmp/gstack-doc-release-XXXXXXXX +mktemp -d "${TMPDIR:-/tmp}/gstack-doc-release-XXXXXXXX" ``` Fetch the existing PR/MR body using the platform from the shared Step 0. If no PR/MR exists, skip body/title updates and continue to the summary. diff --git a/ship/sections/pr-body.md.tmpl b/ship/sections/pr-body.md.tmpl index 259b3fdf4..962587df4 100644 --- a/ship/sections/pr-body.md.tmpl +++ b/ship/sections/pr-body.md.tmpl @@ -119,7 +119,7 @@ saved section file path as `DOCS_SECTION_FILE`. REDACT_VIS=$(~/.claude/skills/gstack/bin/gstack-config get redact_repo_visibility 2>/dev/null) [ -z "$REDACT_VIS" ] && REDACT_VIS=$(gh repo view --json visibility -q .visibility 2>/dev/null | tr 'A-Z' 'a-z') REDACT_VIS="${REDACT_VIS:-unknown}" -PR_BODY_FILE=$(mktemp) || { echo "ERROR: mktemp failed — cannot scan the PR body; refusing to create the PR unscanned." >&2; exit 1; } +PR_BODY_FILE=$(mktemp "${TMPDIR:-/tmp}/gstack-pr-body.XXXXXX") || { echo "ERROR: mktemp failed — cannot scan the PR body; refusing to create the PR unscanned." >&2; exit 1; } { cat <<'PR_BODY_EOF' PR_BODY_EOF diff --git a/test/document-skills-redaction.test.ts b/test/document-skills-redaction.test.ts index 4c856e2b3..1d09079b1 100644 --- a/test/document-skills-redaction.test.ts +++ b/test/document-skills-redaction.test.ts @@ -35,7 +35,7 @@ describe("/document-release redaction", () => { expect(RELEASE).toMatch(/exit 3 \(HIGH\).*do NOT edit/i); }); test("separate shell calls share an explicit run directory and never re-read raw tracker text", () => { - expect(RELEASE).toContain('mktemp -d /tmp/gstack-doc-release-XXXXXXXX'); + expect(RELEASE).toContain('mktemp -d "${TMPDIR:-/tmp}/gstack-doc-release-XXXXXXXX"'); expect(RELEASE).not.toContain('/tmp/gstack-pr-body-$$'); expect(RELEASE).not.toContain(''); expect(RELEASE).toContain('pathlib.Path(sys.argv[1]).read_text()'); diff --git a/test/learnings.test.ts b/test/learnings.test.ts index e43fe7c04..15f38984f 100644 --- a/test/learnings.test.ts +++ b/test/learnings.test.ts @@ -321,3 +321,36 @@ describe('gstack-learnings-search edge cases', () => { expect(output).toContain('confidence: 0/10'); }); }); + +// G3 (#2952): the temp file honors TMPDIR, and a temp-file failure says the +// learning was not recorded instead of exiting with only mktemp's own error. +describe('G3: learnings-log temp file and TMPDIR', () => { + test('logs through a private TMPDIR', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-learn-tmpdir-')); + try { + execSync(`${BIN}/gstack-learnings-log '{"skill":"review","type":"pitfall","key":"tmpdir-ok","insight":"x","confidence":5,"source":"observed"}'`, { + cwd: ROOT, env: { ...process.env, GSTACK_HOME: tmpDir, TMPDIR: tmp }, encoding: 'utf-8', timeout: 15000, + }); + expect(fs.readFileSync(findLearningsFile()!, 'utf-8')).toContain('tmpdir-ok'); + expect(fs.readdirSync(tmp)).toEqual([]); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + + test('an unusable TMPDIR fails loudly and names the dropped learning', () => { + let status = 0; + let stderr = ''; + try { + execSync(`${BIN}/gstack-learnings-log '{"skill":"review","type":"pitfall","key":"tmpdir-bad","insight":"x","confidence":5,"source":"observed"}'`, { + cwd: ROOT, env: { ...process.env, GSTACK_HOME: tmpDir, TMPDIR: path.join(tmpDir, 'does-not-exist') }, encoding: 'utf-8', stdio: 'pipe', timeout: 15000, + }); + } catch (e: any) { + status = e.status; + stderr = String(e.stderr); + } + expect(status).not.toBe(0); + expect(stderr).toContain('the learning was not recorded'); + expect(stderr).toContain('TMPDIR'); + }); +}); diff --git a/test/regression-pr1169-mktemp-fallbacks.test.ts b/test/regression-pr1169-mktemp-fallbacks.test.ts index 02cfd78f0..4e13403cd 100644 --- a/test/regression-pr1169-mktemp-fallbacks.test.ts +++ b/test/regression-pr1169-mktemp-fallbacks.test.ts @@ -75,9 +75,10 @@ describe("#2679: skill-content mktemp guards", () => { expect(rendered).toMatch(/REDACT_FILE=\$\(mktemp\)\s*\|\|\s*\{[^}]*exit 1/); }); - test("ship pr-body template guards PR_BODY_FILE=$(mktemp) with a loud exit", () => { + test("ship pr-body template guards PR_BODY_FILE=$(mktemp ...) with a loud exit", () => { const body = readScript("ship/sections/pr-body.md.tmpl"); - expect(body).toMatch(/PR_BODY_FILE=\$\(mktemp\)\s*\|\|\s*\{[^}]*exit 1/); + // G3 (#2952): the mktemp now carries a ${TMPDIR:-/tmp} template. + expect(body).toMatch(/PR_BODY_FILE=\$\(mktemp "\$\{TMPDIR:-\/tmp\}\/[^"]+"\)\s*\|\|\s*\{[^}]*exit 1/); }); test("ship pr-body GitLab path sends the SCANNED file, never a re-rendered heredoc", () => { @@ -151,3 +152,32 @@ describe("PR #1169 bug #5: supabase/verify-rls.sh mktemp fallback", () => { expect(guard).not.toBeNull(); }); }); + +// G3 (#2952): bare `mktemp`, `mktemp -t` and literal `/tmp/` templates ignore +// the TMPDIR a sandboxed agent shell sets, and a failed mktemp under `set -e` +// silently dropped the learning or question being logged. +describe("G3: mktemp honors TMPDIR in logging bins and lane-owned skill bash", () => { + const FILES = [ + "bin/gstack-learnings-log", + "bin/gstack-question-log", + "bin/gstack-question-preference", + "bin/gstack-jsonl-merge", + "bin/gstack-distill-free-text", + "bin/gstack-community-dashboard", + "bin/gstack-security-dashboard", + "ship/sections/pr-body.md.tmpl", + "document-release/sections/release-body.md.tmpl", + ]; + for (const rel of FILES) { + test(`${rel}: every temp-dir mktemp uses a \${TMPDIR:-/tmp} template`, () => { + const calls = [...readScript(rel).matchAll(/(?:\$\(|^[ \t]*)(mktemp\b[^\n)]*)/gm)].map((m) => m[1]); + expect(calls.length).toBeGreaterThan(0); + for (const call of calls) { + expect(call).not.toMatch(/^mktemp\s*$/); + expect(call).not.toMatch(/^mktemp\s+(-d\s+)?-t\b/); + expect(call).not.toMatch(/^mktemp\s+(-d\s+)?["']?\/tmp\//); + expect(call).toMatch(/\$\{TMPDIR:-\/tmp\}\//); + } + }); + } +}); diff --git a/test/ship-publication-gates.test.ts b/test/ship-publication-gates.test.ts index e305ec529..19cf71955 100644 --- a/test/ship-publication-gates.test.ts +++ b/test/ship-publication-gates.test.ts @@ -52,7 +52,7 @@ exec ${JSON.stringify(process.execPath)} ${JSON.stringify(scanner)} "$@" test('publication composes the saved documentation section unchanged and refuses to scan without it', () => { const block = template.match(/```bash\n(: "\$\{NEW_TITLE:[\s\S]*?)\n```/)?.[1]!; - const compose = block.slice(block.indexOf('PR_BODY_FILE=$(mktemp)'), block.indexOf('~/.claude/skills/gstack/bin/gstack-redact --from-file')); + const compose = block.slice(block.indexOf('PR_BODY_FILE=$(mktemp'), block.indexOf('~/.claude/skills/gstack/bin/gstack-redact --from-file')); const root = mkdtempSync(join(tmpdir(), 'ship-compose-')); try { const section = '**Status:** current — no edits.\n\n- Diagram drift: none (no diagrams in any doc).'; diff --git a/test/workflow-excerpt.test.ts b/test/workflow-excerpt.test.ts index 9744e52df..4b82715e4 100644 --- a/test/workflow-excerpt.test.ts +++ b/test/workflow-excerpt.test.ts @@ -108,7 +108,7 @@ describe('workflow judge excerpts', () => { test('ship publishes existing PRs only after shared body composition and scan', () => { const text = readWorkflowExcerpt('ship/SKILL.md', '# Ship:', '## Important Rules'); const publish = text.slice(text.indexOf('## Step 19:'), text.indexOf('## Step 20:')); - const compose = publish.indexOf('PR_BODY_FILE=$(mktemp)'); + const compose = publish.indexOf('PR_BODY_FILE=$(mktemp'); const scan = publish.indexOf('gstack-redact --from-file "$PR_BODY_FILE"'); const edit = publish.indexOf('gh pr edit --body-file'); expect(compose).toBeGreaterThan(0);