CalvinBackup/gstack
1
0
Fork 0
mirror of https://github.com/garrytan/gstack.git synced 2026-09-09 06:28:59 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix: mktemp failure aborts loudly at all three skill-content sites; failed upgrade swap restores the backup (#2679)

Browse Source 
An empty $(mktemp) result silently disabled the redaction pass (redact-doc
resolver, ship pr-body) and made /gstack-upgrade's vendored path destructive:
clone lands at "/gstack", the swap mv fails, and rm -rf then deletes BOTH the
live install's backup and "". All three sites now guard the assignment with a
loud exit; the vendored block additionally restores the backup when the swap
fails (same failure class — backup deletion after a failed mv) and the GitLab
MR path sends the SCANNED file's bytes instead of re-rendering an unscanned
heredoc. bin/gstack-redact rejects an explicit empty --from-file path instead
of silently falling through to stdin.

Receipts: 6 of 8 new regression checks fail on a v1.77.0.0 scratch worktree.

Fixes #2679

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-08-31 20:47:41 +00:00
co-authored by Claude Fable 5
parent 95d3aa17e9
commit 04d61024d7
10 changed files with 124 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files
bin/gstack-redact
+9
View File
@@ -123,6 +123,15 @@ function flag(name: string): boolean {
function readInput(): string {
const file = arg("--from-file");
// An explicitly-passed EMPTY path must error, not silently fall through to
// stdin: skill blocks pass "$FILE" from a $(mktemp) that may have failed,
// and the stdin fallback then scans nothing while looking green (#2679).
if (file === "") {
process.stderr.write(
"gstack-redact: --from-file requires a non-empty path (did mktemp fail?)\n",
);
process.exit(1);
}
if (file) {
const st = fs.statSync(file);
if (st.size > MAX_STDIN_BYTES) {
gstack-upgrade/SKILL.md
+11 -5
View File
@@ -170,12 +170,18 @@ If `$STASH_OUTPUT` contains "Saved working directory", warn the user: "Note: loc
**For vendored installs** (vendored, vendored-global):
```bash
PARENT=$(dirname "$INSTALL_DIR")
TMP_DIR=$(mktemp -d)
git clone --depth 1 https://github.com/garrytan/gstack.git "$TMP_DIR/gstack"
TMP_DIR=$(mktemp -d) || { echo "ERROR: mktemp failed — aborting upgrade (install untouched)." >&2; exit 1; }
git clone --depth 1 https://github.com/garrytan/gstack.git "$TMP_DIR/gstack" || { echo "ERROR: clone failed — aborting upgrade (install untouched)." >&2; rm -rf "$TMP_DIR"; exit 1; }
mv "$INSTALL_DIR" "$INSTALL_DIR.bak"
mv "$TMP_DIR/gstack" "$INSTALL_DIR"
cd "$INSTALL_DIR" && ./setup
rm -rf "$INSTALL_DIR.bak" "$TMP_DIR"
if mv "$TMP_DIR/gstack" "$INSTALL_DIR"; then
cd "$INSTALL_DIR" && ./setup
rm -rf "$INSTALL_DIR.bak" "$TMP_DIR"
else
mv "$INSTALL_DIR.bak" "$INSTALL_DIR"
echo "ERROR: swap failed — previous install restored; upgrade aborted." >&2
rm -rf "$TMP_DIR"
exit 1
fi
```
### Step 4.5: Handle local vendored copy
gstack-upgrade/SKILL.md.tmpl
+11 -5
View File
@@ -167,12 +167,18 @@ If `$STASH_OUTPUT` contains "Saved working directory", warn the user: "Note: loc
**For vendored installs** (vendored, vendored-global):
```bash
PARENT=$(dirname "$INSTALL_DIR")
TMP_DIR=$(mktemp -d)
git clone --depth 1 https://github.com/garrytan/gstack.git "$TMP_DIR/gstack"
TMP_DIR=$(mktemp -d) || { echo "ERROR: mktemp failed — aborting upgrade (install untouched)." >&2; exit 1; }
git clone --depth 1 https://github.com/garrytan/gstack.git "$TMP_DIR/gstack" || { echo "ERROR: clone failed — aborting upgrade (install untouched)." >&2; rm -rf "$TMP_DIR"; exit 1; }
mv "$INSTALL_DIR" "$INSTALL_DIR.bak"
mv "$TMP_DIR/gstack" "$INSTALL_DIR"
cd "$INSTALL_DIR" && {{SETUP_COMMAND}}
rm -rf "$INSTALL_DIR.bak" "$TMP_DIR"
if mv "$TMP_DIR/gstack" "$INSTALL_DIR"; then
cd "$INSTALL_DIR" && {{SETUP_COMMAND}}
rm -rf "$INSTALL_DIR.bak" "$TMP_DIR"
else
mv "$INSTALL_DIR.bak" "$INSTALL_DIR"
echo "ERROR: swap failed — previous install restored; upgrade aborted." >&2
rm -rf "$TMP_DIR"
exit 1
fi
```
### Step 4.5: Handle local vendored copy
scripts/resolvers/redact-doc.ts
+1 -1
View File
@@ -62,7 +62,7 @@ REDACT_VIS=$(~/.claude/skills/gstack/bin/gstack-config get redact_repo_visibilit
[ -z "$REDACT_VIS" ] && REDACT_VIS=$(gh repo view --json visibility -q .visibility 2>/dev/null | tr 'A-Z' 'a-z')
[ -z "$REDACT_VIS" ] && REDACT_VIS=$(glab repo view -F json 2>/dev/null | grep -o '"visibility":"[^"]*"' | head -1 | sed 's/.*:"//;s/"//' | tr 'A-Z' 'a-z')
REDACT_VIS="\${REDACT_VIS:-unknown}"
REDACT_FILE=$(mktemp)
REDACT_FILE=$(mktemp) || { echo "ERROR: mktemp failed — refusing to send unscanned ${sink.noun}." >&2; exit 1; }
cat > "$REDACT_FILE" <<'REDACT_BODY_EOF'
<the exact ${sink.noun} goes here>
REDACT_BODY_EOF
ship/sections/pr-body.md
+5 -5
View File
@@ -170,7 +170,7 @@ the PR (a live-format credential inside the fence still blocks).
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)
PR_BODY_FILE=$(mktemp) || { echo "ERROR: mktemp failed — cannot scan the PR body; refusing to create the PR unscanned." >&2; exit 1; }
cat > "$PR_BODY_FILE" <<'PR_BODY_EOF'
<PR body from above>
PR_BODY_EOF
@@ -200,10 +200,10 @@ rm -f "$PR_BODY_FILE"
```bash
# MR title MUST start with v$NEW_VERSION — enforced on every run, no exceptions.
# (See Step 19 idempotency block + bin/gstack-pr-title-rewrite.sh for the rule.)
glab mr create -b <base> -t "v$NEW_VERSION <type>: <summary>" -d "$(cat <<'EOF'
<MR body from above>
EOF
)"
# Send the SCANNED file's bytes — scan-at-sink means never re-render the body
# from a fresh heredoc (that reopens the scan-vs-send gap).
glab mr create -b <base> -t "v$NEW_VERSION <type>: <summary>" -d "$(cat "$PR_BODY_FILE")"
rm -f "$PR_BODY_FILE"
```
**If neither CLI is available:**
ship/sections/pr-body.md.tmpl
+5 -5
View File
@@ -168,7 +168,7 @@ the PR (a live-format credential inside the fence still blocks).
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)
PR_BODY_FILE=$(mktemp) || { echo "ERROR: mktemp failed — cannot scan the PR body; refusing to create the PR unscanned." >&2; exit 1; }
cat > "$PR_BODY_FILE" <<'PR_BODY_EOF'
<PR body from above>
PR_BODY_EOF
@@ -198,10 +198,10 @@ rm -f "$PR_BODY_FILE"
```bash
# MR title MUST start with v$NEW_VERSION — enforced on every run, no exceptions.
# (See Step 19 idempotency block + bin/gstack-pr-title-rewrite.sh for the rule.)
glab mr create -b <base> -t "v$NEW_VERSION <type>: <summary>" -d "$(cat <<'EOF'
<MR body from above>
EOF
)"
# Send the SCANNED file's bytes — scan-at-sink means never re-render the body
# from a fresh heredoc (that reopens the scan-vs-send gap).
glab mr create -b <base> -t "v$NEW_VERSION <type>: <summary>" -d "$(cat "$PR_BODY_FILE")"
rm -f "$PR_BODY_FILE"
```
**If neither CLI is available:**
spec/sections/gate-and-file.md
+1 -1
View File
@@ -59,7 +59,7 @@ REDACT_VIS=$(~/.claude/skills/gstack/bin/gstack-config get redact_repo_visibilit
[ -z "$REDACT_VIS" ] && REDACT_VIS=$(gh repo view --json visibility -q .visibility 2>/dev/null | tr 'A-Z' 'a-z')
[ -z "$REDACT_VIS" ] && REDACT_VIS=$(glab repo view -F json 2>/dev/null | grep -o '"visibility":"[^"]*"' | head -1 | sed 's/.*:"//;s/"//' | tr 'A-Z' 'a-z')
REDACT_VIS="${REDACT_VIS:-unknown}"
REDACT_FILE=$(mktemp)
REDACT_FILE=$(mktemp) || { echo "ERROR: mktemp failed — refusing to send unscanned the spec body." >&2; exit 1; }
cat > "$REDACT_FILE" <<'REDACT_BODY_EOF'
<the exact the spec body goes here>
REDACT_BODY_EOF
test/fixtures/golden/codex-ship-SKILL.md
Vendored
+5 -5
View File
@@ -2452,7 +2452,7 @@ the PR (a live-format credential inside the fence still blocks).
REDACT_VIS=$($GSTACK_ROOT/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)
PR_BODY_FILE=$(mktemp) || { echo "ERROR: mktemp failed — cannot scan the PR body; refusing to create the PR unscanned." >&2; exit 1; }
cat > "$PR_BODY_FILE" <<'PR_BODY_EOF'
<PR body from above>
PR_BODY_EOF
@@ -2482,10 +2482,10 @@ rm -f "$PR_BODY_FILE"
```bash
# MR title MUST start with v$NEW_VERSION — enforced on every run, no exceptions.
# (See Step 19 idempotency block + bin/gstack-pr-title-rewrite.sh for the rule.)
glab mr create -b <base> -t "v$NEW_VERSION <type>: <summary>" -d "$(cat <<'EOF'
<MR body from above>
EOF
)"
# Send the SCANNED file's bytes — scan-at-sink means never re-render the body
# from a fresh heredoc (that reopens the scan-vs-send gap).
glab mr create -b <base> -t "v$NEW_VERSION <type>: <summary>" -d "$(cat "$PR_BODY_FILE")"
rm -f "$PR_BODY_FILE"
```
**If neither CLI is available:**
test/fixtures/golden/factory-ship-SKILL.md
Vendored
+5 -5
View File
@@ -2879,7 +2879,7 @@ the PR (a live-format credential inside the fence still blocks).
REDACT_VIS=$($GSTACK_ROOT/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)
PR_BODY_FILE=$(mktemp) || { echo "ERROR: mktemp failed — cannot scan the PR body; refusing to create the PR unscanned." >&2; exit 1; }
cat > "$PR_BODY_FILE" <<'PR_BODY_EOF'
<PR body from above>
PR_BODY_EOF
@@ -2909,10 +2909,10 @@ rm -f "$PR_BODY_FILE"
```bash
# MR title MUST start with v$NEW_VERSION — enforced on every run, no exceptions.
# (See Step 19 idempotency block + bin/gstack-pr-title-rewrite.sh for the rule.)
glab mr create -b <base> -t "v$NEW_VERSION <type>: <summary>" -d "$(cat <<'EOF'
<MR body from above>
EOF
)"
# Send the SCANNED file's bytes — scan-at-sink means never re-render the body
# from a fresh heredoc (that reopens the scan-vs-send gap).
glab mr create -b <base> -t "v$NEW_VERSION <type>: <summary>" -d "$(cat "$PR_BODY_FILE")"
rm -f "$PR_BODY_FILE"
```
**If neither CLI is available:**
test/regression-pr1169-mktemp-fallbacks.test.ts
+71
View File
@@ -56,6 +56,77 @@ describe("PR #1169 bug #4: gstack-telemetry-sync mktemp fallback", () => {
});
});
// #2679: three skill-content mktemp sites ran unguarded. An empty result
// ("" on mktemp failure) silently disabled the redaction pass (redact-doc
// resolver + ship pr-body) and — the destructive one — made /gstack-upgrade's
// vendored path clone to "/gstack", fail the swap, then `rm -rf` BOTH the
// live install's backup and "". Guards must abort loudly; the upgrade block
// must also restore the backup when the swap fails (same failure class:
// backup deletion after a failed mv).
describe("#2679: skill-content mktemp guards", () => {
test("redact-doc resolver guards REDACT_FILE=$(mktemp) with a loud exit", () => {
// The guard line contains a ${sink.noun} interpolation in the resolver
// source, so match to end-of-line rather than [^}]* (which stops at the
// interpolation's closing brace).
const body = readScript("scripts/resolvers/redact-doc.ts");
expect(body).toMatch(/REDACT_FILE=\$\(mktemp\)\s*\|\|\s*\{.*exit 1/);
// And the rendered output (interpolation resolved) carries the guard too.
const rendered = readScript("spec/sections/gate-and-file.md");
expect(rendered).toMatch(/REDACT_FILE=\$\(mktemp\)\s*\|\|\s*\{[^}]*exit 1/);
});
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/);
});
test("ship pr-body GitLab path sends the SCANNED file, never a re-rendered heredoc", () => {
const body = readScript("ship/sections/pr-body.md.tmpl");
expect(body).toContain('-d "$(cat "$PR_BODY_FILE")"');
expect(body).not.toMatch(/glab mr create[^\n]*-d "\$\(cat <<'EOF'/);
});
test("gstack-upgrade vendored block guards mktemp -d and clone with loud aborts", () => {
const body = readScript("gstack-upgrade/SKILL.md.tmpl");
expect(body).toMatch(/TMP_DIR=\$\(mktemp -d\)\s*\|\|\s*\{[^}]*exit 1/);
expect(body).toMatch(/git clone[^\n]*\|\|\s*\{[^}]*exit 1/);
});
test("gstack-upgrade vendored block restores the backup on a failed swap (no unconditional backup rm)", () => {
const body = readScript("gstack-upgrade/SKILL.md.tmpl");
expect(body).toMatch(/if mv "\$TMP_DIR\/gstack" "\$INSTALL_DIR"; then/);
expect(body).toMatch(/mv "\$INSTALL_DIR\.bak" "\$INSTALL_DIR"/);
// The backup rm must live inside the success branch, not after the block.
const block = body.slice(body.indexOf('if mv "$TMP_DIR/gstack"'));
const successRm = block.indexOf('rm -rf "$INSTALL_DIR.bak"');
const elseBranch = block.indexOf("else");
expect(successRm).toBeGreaterThan(-1);
expect(successRm).toBeLessThan(elseBranch);
});
test("runtime: the guarded assignment aborts when mktemp fails", () => {
const { spawnSync } = require("node:child_process") as typeof import("node:child_process");
const script = `mktemp() { return 1; }
TMP_DIR=$(mktemp -d) || { echo "ERROR: mktemp failed — aborting upgrade (install untouched)." >&2; exit 1; }
echo "SHOULD NOT REACH: $TMP_DIR"`;
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");
});
test("runtime: gstack-redact --from-file '' errors loudly instead of falling through to stdin", () => {
const { spawnSync } = require("node:child_process") as typeof import("node:child_process");
const r = spawnSync(
"bun",
[path.join(ROOT, "bin", "gstack-redact"), "--from-file", "", "--json"],
{ encoding: "utf-8", input: "sk-ant-api03-not-really-a-key", timeout: 15_000 },
);
expect(r.status).toBe(1);
expect(r.stderr).toContain("non-empty path");
});
});
describe("PR #1169 bug #5: supabase/verify-rls.sh mktemp fallback", () => {
const SCRIPT = "supabase/verify-rls.sh";