mirror of https://github.com/garrytan/gstack.git
fix(paths): shell-quote gstack-paths output so eval round-trips values
gstack-paths emitted bare KEY=VALUE lines, so the documented eval "$(gstack-paths)" re-parsed the values: backslashes were eaten as escapes (Windows $TMP C:\Users\... became C:Users...) and a space word-split the assignment, leaving the variable empty. Emit each value with printf %q so eval round-trips byte-for-byte; plain POSIX paths are unchanged. Round-trip regression tests cover backslashes, spaces, and embedded quotes. Closes #2374. Contributed by @fangearhq-boop (PR #2376); same fix independently by @yannickspiess (PR #1580). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
4122721eb0
commit
72aff66438
|
|
@ -13,9 +13,15 @@
|
|||
# PLAN_ROOT: GSTACK_PLAN_DIR -> CLAUDE_PLANS_DIR -> $HOME/.claude/plans -> .claude/plans
|
||||
# TMP_ROOT: TMPDIR -> TMP -> .gstack/tmp (and mkdir -p, best-effort)
|
||||
#
|
||||
# Security: output values are not sanitized — callers may receive paths with
|
||||
# shell-special characters if env vars contain them. Skills should always quote
|
||||
# expansions ("$GSTACK_STATE_ROOT", not $GSTACK_STATE_ROOT).
|
||||
# Output: values are emitted shell-quoted (printf %q) so `eval` round-trips them
|
||||
# byte-for-byte. This matters on Windows, where $TMP is a backslash path like
|
||||
# C:\Users\me\AppData\Local\Temp — with a bare `echo`, eval consumes the
|
||||
# backslashes as escapes and the caller gets C:UsersmeAppDataLocalTemp. A value
|
||||
# containing a space (C:\Program Files\Temp) is worse: eval word-splits it and
|
||||
# the variable ends up empty. Quoting here is the only fix that works, because
|
||||
# the corruption happens during eval, before the caller has anything to quote.
|
||||
# Callers should still quote expansions ("$GSTACK_STATE_ROOT") for the same
|
||||
# reason any path variable needs quoting.
|
||||
set -u
|
||||
|
||||
# State root: where gstack writes projects/, sessions/, analytics/.
|
||||
|
|
@ -69,6 +75,6 @@ _tmp_root="${_tmp_root%/}"
|
|||
# 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"
|
||||
printf 'GSTACK_STATE_ROOT=%q\n' "$_state_root"
|
||||
printf 'PLAN_ROOT=%q\n' "$_plan_root"
|
||||
printf 'TMP_ROOT=%q\n' "$_tmp_root"
|
||||
|
|
|
|||
|
|
@ -104,6 +104,57 @@ describe('gstack-paths', () => {
|
|||
expect(got).toHaveProperty('TMP_ROOT');
|
||||
});
|
||||
|
||||
// Regression: values must survive `eval "$(gstack-paths)"`, which is the
|
||||
// documented calling convention. A bare `echo` emits an unquoted RHS, so eval
|
||||
// re-parses it: backslashes become escapes and spaces become word separators.
|
||||
// On Windows $TMP is always a backslash path, so every skill that then runs
|
||||
// mktemp "$TMP_ROOT/..." fails and the bash block dies before doing any work.
|
||||
// These run identically on POSIX — the values are just strings.
|
||||
function evalRoundTrip(env: Record<string, string | undefined>, varName: string): string {
|
||||
const result = spawnSync(
|
||||
'bash',
|
||||
['-c', `eval "$(bash "$1")"; printf '%s' "\${${varName}}"`, 'sh', BIN],
|
||||
{
|
||||
env: { PATH: process.env.PATH, USERPROFILE: '', ...env } as Record<string, string>,
|
||||
encoding: 'utf-8',
|
||||
},
|
||||
);
|
||||
if (result.status !== 0) {
|
||||
throw new Error(`eval round-trip failed (status ${result.status}): ${result.stderr}`);
|
||||
}
|
||||
return result.stdout;
|
||||
}
|
||||
|
||||
// Values are POSIX-shaped on purpose: MSYS/Git Bash rewrites `C:\...` env
|
||||
// values to `/c/...` before bash sees them, so a literal Windows path would
|
||||
// assert the translation layer rather than the quoting. A backslash is a
|
||||
// backslash to eval either way, which is the behavior under test.
|
||||
test('eval round-trip preserves backslashes (#2374)', () => {
|
||||
// Skip on Windows: MSYS also rewrites backslashes to forward slashes in
|
||||
// env values, so a literal backslash cannot be injected through the
|
||||
// environment on a Git Bash runner. The escape-eating this guards against
|
||||
// is pure eval semantics, so exercising it on Linux/macOS CI is sufficient
|
||||
// — same reasoning as the HOME-unset skips above.
|
||||
if (process.platform === 'win32') return;
|
||||
const backslashed = '/tmp/back\\slash/dir';
|
||||
expect(evalRoundTrip({ TMPDIR: backslashed, HOME: '/h' }, 'TMP_ROOT')).toBe(backslashed);
|
||||
});
|
||||
|
||||
test('eval round-trip preserves spaces (#2374)', () => {
|
||||
// Bare echo made eval word-split this, leaving the variable empty and
|
||||
// emitting `<second-word>: command not found`.
|
||||
const spaced = '/tmp/two words/dir';
|
||||
expect(evalRoundTrip({ TMPDIR: spaced, HOME: '/h' }, 'TMP_ROOT')).toBe(spaced);
|
||||
});
|
||||
|
||||
test('eval round-trip preserves quotes, and leaves plain paths alone (#2374)', () => {
|
||||
expect(evalRoundTrip({ TMPDIR: "/tmp/o'brien", HOME: '/h' }, 'TMP_ROOT')).toBe("/tmp/o'brien");
|
||||
expect(evalRoundTrip({ GSTACK_HOME: '/tmp/state root' }, 'GSTACK_STATE_ROOT')).toBe(
|
||||
'/tmp/state root',
|
||||
);
|
||||
expect(evalRoundTrip({ HOME: '/tmp/myhome' }, 'PLAN_ROOT')).toBe('/tmp/myhome/.claude/plans');
|
||||
});
|
||||
|
||||
test('output is shell-evalable: only KEY=VALUE lines, no extra prose', () => {
|
||||
const result = spawnSync('bash', [BIN], {
|
||||
env: { PATH: process.env.PATH, USERPROFILE: '', HOME: '/tmp/h' } as Record<string, string>,
|
||||
|
|
|
|||
Loading…
Reference in New Issue