From b30d7e3653d30a92e4139903c47080edb301f384 Mon Sep 17 00:00:00 2001 From: 0xDevNinja Date: Thu, 14 May 2026 18:48:05 +0530 Subject: [PATCH] fix(gstack-paths): add --get so /codex skips eval-tilde footgun - Pattern 2 in #1329: `eval "$(~/.claude/skills/gstack/bin/gstack-paths)"` trips Claude Code's bash AST parser ("Unhandled node type: string") because of tilde inside $() inside double-quoted eval arg. Forces manual approval on every /codex. - `gstack-paths --get state-root|plan-root|tmp-root` prints one path, no `KEY=` prefix. Callers use `PLAN_ROOT=$(gstack-paths --get plan-root)`. - Bare invocation unchanged (KEY=VALUE form), so unmigrated skills keep working. - `codex/SKILL.md.tmpl` Step 0.6 switched + regenerated SKILL.md. - Patterns 1/3/4 + other skills using the eval form out of scope; one-at-a-time migration. Refs #1329 --- bin/gstack-paths | 51 +++++++++++++++++++++++++++++++---- codex/SKILL.md | 7 ++++- codex/SKILL.md.tmpl | 7 ++++- test/gstack-paths.test.ts | 57 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 115 insertions(+), 7 deletions(-) diff --git a/bin/gstack-paths b/bin/gstack-paths index eee603d61b..c74bf663fe 100755 --- a/bin/gstack-paths +++ b/bin/gstack-paths @@ -1,7 +1,15 @@ #!/usr/bin/env bash # gstack-paths — output portable state-root paths for skill bash blocks -# Usage: eval "$(gstack-paths)" → sets GSTACK_STATE_ROOT, PLAN_ROOT, TMP_ROOT -# Or: gstack-paths → prints GSTACK_STATE_ROOT=... etc. +# Usage: eval "$(gstack-paths)" → sets GSTACK_STATE_ROOT, PLAN_ROOT, TMP_ROOT +# gstack-paths → prints GSTACK_STATE_ROOT=... etc. +# gstack-paths --get → prints a single resolved path (no KEY= prefix) +# where is state-root | plan-root | tmp-root +# +# The --get form was added so callers can use `PLAN_ROOT=$(gstack-paths --get plan-root)` +# instead of `eval "$(gstack-paths)"`. The eval form trips Claude Code's PreToolUse +# bash AST parser (tilde inside $() inside double-quoted eval arg = "Unhandled node +# type: string"), forcing manual approval on every skill that needs portable roots. +# See issue #1329 Pattern 2. # # Resolves three roots with explicit fallback chains so skills work the same # whether installed as a Claude Code plugin (CLAUDE_PLUGIN_DATA / CLAUDE_PLANS_DIR @@ -18,6 +26,27 @@ # expansions ("$GSTACK_STATE_ROOT", not $GSTACK_STATE_ROOT). set -u +# Parse optional --get selector. +_get_key="" +case "${1:-}" in + --help|-h) + sed -n '2,18p' "$0" | sed 's/^# \{0,1\}//' + exit 0 + ;; + --get) + if [ -z "${2:-}" ]; then + echo "gstack-paths: --get requires a key (state-root|plan-root|tmp-root)" >&2 + exit 1 + fi + _get_key="$2" + ;; + "" ) ;; # bare invocation — backward-compatible default output + *) + echo "gstack-paths: unknown argument '$1' (expected --get )" >&2 + exit 1 + ;; +esac + # State root: where gstack writes projects/, sessions/, analytics/. if [ -n "${GSTACK_HOME:-}" ]; then _state_root="$GSTACK_HOME" @@ -56,6 +85,18 @@ fi # will discover that on their own write attempt. Don't fail the eval here. mkdir -p "$_tmp_root" 2>/dev/null || true -echo "GSTACK_STATE_ROOT=$_state_root" -echo "PLAN_ROOT=$_plan_root" -echo "TMP_ROOT=$_tmp_root" +if [ -n "$_get_key" ]; then + case "$_get_key" in + state-root) echo "$_state_root" ;; + plan-root) echo "$_plan_root" ;; + tmp-root) echo "$_tmp_root" ;; + *) + echo "gstack-paths: unknown key '$_get_key' (expected state-root|plan-root|tmp-root)" >&2 + exit 1 + ;; + esac +else + echo "GSTACK_STATE_ROOT=$_state_root" + echo "PLAN_ROOT=$_plan_root" + echo "TMP_ROOT=$_tmp_root" +fi diff --git a/codex/SKILL.md b/codex/SKILL.md index 0ff1e3e553..648d2fdaf2 100644 --- a/codex/SKILL.md +++ b/codex/SKILL.md @@ -872,8 +872,13 @@ This keeps the skill working whether installed as a Claude Code plugin (`CLAUDE_PLANS_DIR` set), a global `~/.claude/skills/gstack/` install, or a CI container where `HOME` may be unset and `/tmp` may be read-only. +`--get ` is preferred over `eval "$(...)"` here: the eval form trips Claude +Code's bash AST parser (tilde inside `$()` inside double-quoted eval arg) and +forces a manual confirmation on every run. Issue #1329 Pattern 2. + ```bash -eval "$(~/.claude/skills/gstack/bin/gstack-paths)" +PLAN_ROOT=$(~/.claude/skills/gstack/bin/gstack-paths --get plan-root) +TMP_ROOT=$(~/.claude/skills/gstack/bin/gstack-paths --get tmp-root) ``` After this, every subsequent bash block in this skill uses `"$PLAN_ROOT"` and diff --git a/codex/SKILL.md.tmpl b/codex/SKILL.md.tmpl index ed118a1193..564bdf34fd 100644 --- a/codex/SKILL.md.tmpl +++ b/codex/SKILL.md.tmpl @@ -98,8 +98,13 @@ This keeps the skill working whether installed as a Claude Code plugin (`CLAUDE_PLANS_DIR` set), a global `~/.claude/skills/gstack/` install, or a CI container where `HOME` may be unset and `/tmp` may be read-only. +`--get ` is preferred over `eval "$(...)"` here: the eval form trips Claude +Code's bash AST parser (tilde inside `$()` inside double-quoted eval arg) and +forces a manual confirmation on every run. Issue #1329 Pattern 2. + ```bash -eval "$(~/.claude/skills/gstack/bin/gstack-paths)" +PLAN_ROOT=$(~/.claude/skills/gstack/bin/gstack-paths --get plan-root) +TMP_ROOT=$(~/.claude/skills/gstack/bin/gstack-paths --get tmp-root) ``` After this, every subsequent bash block in this skill uses `"$PLAN_ROOT"` and diff --git a/test/gstack-paths.test.ts b/test/gstack-paths.test.ts index a63be45e0b..48706b1a02 100644 --- a/test/gstack-paths.test.ts +++ b/test/gstack-paths.test.ts @@ -98,4 +98,61 @@ describe('gstack-paths', () => { expect(line).toMatch(/^[A-Z_]+=.*/); } }); + + // --- --get CLI form (issue #1329 Pattern 2) --- + + function runGet(env: Record, args: string[]): { stdout: string; stderr: string; status: number } { + const r = spawnSync('bash', [BIN, ...args], { + env: { PATH: process.env.PATH, USERPROFILE: '', ...env } as Record, + encoding: 'utf-8', + }); + return { stdout: r.stdout || '', stderr: r.stderr || '', status: r.status ?? -1 }; + } + + describe('--get ', () => { + test('--get state-root prints just the resolved state root (no KEY= prefix)', () => { + const r = runGet({ HOME: '/tmp/myhome' }, ['--get', 'state-root']); + expect(r.status).toBe(0); + expect(r.stdout.trim()).toBe('/tmp/myhome/.gstack'); + expect(r.stdout).not.toContain('='); + }); + + test('--get plan-root respects the GSTACK_PLAN_DIR override', () => { + const r = runGet({ GSTACK_PLAN_DIR: '/tmp/explicit', HOME: '/h' }, ['--get', 'plan-root']); + expect(r.status).toBe(0); + expect(r.stdout.trim()).toBe('/tmp/explicit'); + }); + + test('--get tmp-root honors TMPDIR', () => { + const r = runGet({ TMPDIR: '/tmp/x', HOME: '/h' }, ['--get', 'tmp-root']); + expect(r.status).toBe(0); + expect(r.stdout.trim()).toBe('/tmp/x'); + }); + + test('--get with unknown key exits 1 and explains', () => { + const r = runGet({ HOME: '/h' }, ['--get', 'bogus']); + expect(r.status).toBe(1); + expect(r.stderr).toContain("unknown key 'bogus'"); + }); + + test('--get without a key exits 1', () => { + const r = runGet({ HOME: '/h' }, ['--get']); + expect(r.status).toBe(1); + expect(r.stderr).toContain('--get requires a key'); + }); + + test('unknown top-level flag exits 1', () => { + const r = runGet({ HOME: '/h' }, ['--bogus']); + expect(r.status).toBe(1); + expect(r.stderr).toContain('unknown argument'); + }); + + test('bare invocation stays backward-compatible (KEY=VALUE form)', () => { + const r = runGet({ HOME: '/tmp/h' }, []); + expect(r.status).toBe(0); + expect(r.stdout).toContain('GSTACK_STATE_ROOT='); + expect(r.stdout).toContain('PLAN_ROOT='); + expect(r.stdout).toContain('TMP_ROOT='); + }); + }); });