From 52fab7d9d740cb066ac96ee0992b1910f9580437 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 15 Jul 2026 22:51:19 -0400 Subject: [PATCH] fix(#2288): archive phase history under the outgoing milestone version (#2323) * fix(#2288): archive phase history under the outgoing milestone version phases.clear derived its archive directory from a live getMilestoneInfo() read, but new-milestone.md switches the milestone BEFORE phases.clear runs, so phase history was filed under the NEW milestone's -phases/ dir. Add a --archive-version override (threaded from new-milestone.md, captured before the switch) with precedence override -> live read -> dated label. Harden the version label against path traversal on both phases.clear and the sibling milestone-complete sink (the label is a moved directory name), and persist the outgoing version via a file + quoted shell expansion so untrusted STATE.md content is never re-parsed by the shell. Co-Authored-By: Claude Opus 4.8 (1M context) * docs(#2288): backfill PR number 2323 into changesets Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .changeset/clever-orcas-squeak.md | 5 + .changeset/humble-jaguars-swim.md | 5 + gsd-core/workflows/new-milestone.md | 33 ++- src/milestone.cts | 95 +++++++- .../golden-install-parity/antigravity.json | 2 +- .../golden-install-parity/augment.json | 2 +- .../golden-install-parity/claude-local.json | 2 +- .../golden-install-parity/claude.json | 2 +- .../fixtures/golden-install-parity/cline.json | 2 +- .../golden-install-parity/codebuddy.json | 2 +- .../fixtures/golden-install-parity/codex.json | 2 +- .../golden-install-parity/copilot.json | 2 +- .../golden-install-parity/cursor.json | 2 +- .../golden-install-parity/hermes.json | 2 +- .../fixtures/golden-install-parity/kilo.json | 2 +- .../fixtures/golden-install-parity/kimi.json | 2 +- .../golden-install-parity/opencode.json | 2 +- tests/fixtures/golden-install-parity/pi.json | 2 +- .../fixtures/golden-install-parity/qwen.json | 2 +- .../fixtures/golden-install-parity/trae.json | 2 +- .../golden-install-parity/windsurf.json | 2 +- .../fixtures/golden-install-parity/zcode.json | 2 +- tests/new-milestone-clear-phases.test.cjs | 224 ++++++++++++++++++ tests/workflow-size-baseline.json | 2 +- 24 files changed, 368 insertions(+), 32 deletions(-) create mode 100644 .changeset/clever-orcas-squeak.md create mode 100644 .changeset/humble-jaguars-swim.md diff --git a/.changeset/clever-orcas-squeak.md b/.changeset/clever-orcas-squeak.md new file mode 100644 index 000000000..cbc3ba19a --- /dev/null +++ b/.changeset/clever-orcas-squeak.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2323 +--- +**`phases.clear` now archives phase history under the outgoing milestone version, not the newly-switched one** — because `new-milestone` advances the milestone before clearing leftover phases, the phase-history archive was silently misfiled under the new milestone's `-phases/` directory. A new `--archive-version` override on `phases.clear` (threaded from the new-milestone workflow) files the archive under the previous milestone's version; without it, behavior is unchanged. (#2288) diff --git a/.changeset/humble-jaguars-swim.md b/.changeset/humble-jaguars-swim.md new file mode 100644 index 000000000..f44f48c72 --- /dev/null +++ b/.changeset/humble-jaguars-swim.md @@ -0,0 +1,5 @@ +--- +type: Security +pr: 2323 +--- +**`phases.clear --archive-version` and `milestone complete ` now reject version labels containing path separators or `..`** — the milestone version becomes a filesystem directory name that phase directories are moved into, so an unvalidated value could relocate phase history outside `.planning/milestones/`. Both now validate against a strict version-token pattern and fail loudly. (#2288) diff --git a/gsd-core/workflows/new-milestone.md b/gsd-core/workflows/new-milestone.md index 994091e1e..b48d5c5f3 100644 --- a/gsd-core/workflows/new-milestone.md +++ b/gsd-core/workflows/new-milestone.md @@ -182,9 +182,20 @@ blockers, todos) is preserved across the switch — symmetric with ```bash _GSD_SHIM_NAME="gsd-tools.cjs"; _GSD_RUNTIME_ROOT="${RUNTIME_DIR:-$(git rev-parse --show-toplevel 2>/dev/null || pwd)}"; GSD_TOOLS="${_GSD_RUNTIME_ROOT}/gsd-core/bin/${_GSD_SHIM_NAME}"; if [ -f "$GSD_TOOLS" ]; then gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${_GSD_RUNTIME_ROOT}/.claude/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${_GSD_RUNTIME_ROOT}/.claude/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${_GSD_RUNTIME_ROOT}/.codex/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${_GSD_RUNTIME_ROOT}/.codex/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif command -v gsd-tools >/dev/null 2>&1; then GSD_TOOLS="$(command -v gsd-tools)"; gsd_run() { "$GSD_TOOLS" "$@"; }; elif [ -f "${CLAUDE_CONFIG_DIR:-$HOME/.claude}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${CLAUDE_CONFIG_DIR:-$HOME/.claude}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${HERMES_HOME:-$HOME/.hermes}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${HERMES_HOME:-$HOME/.hermes}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${CURSOR_CONFIG_DIR:-$HOME/.cursor}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${CURSOR_CONFIG_DIR:-$HOME/.cursor}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${CODEX_HOME:-$HOME/.codex}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${CODEX_HOME:-$HOME/.codex}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${GEMINI_CONFIG_DIR:-$HOME/.gemini}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${GEMINI_CONFIG_DIR:-$HOME/.gemini}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${COPILOT_CONFIG_DIR:-$HOME/.copilot}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${COPILOT_CONFIG_DIR:-$HOME/.copilot}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${WINDSURF_CONFIG_DIR:-$HOME/.codeium/windsurf}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${WINDSURF_CONFIG_DIR:-$HOME/.codeium/windsurf}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${AUGMENT_CONFIG_DIR:-$HOME/.augment}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${AUGMENT_CONFIG_DIR:-$HOME/.augment}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${TRAE_CONFIG_DIR:-$HOME/.trae}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${TRAE_CONFIG_DIR:-$HOME/.trae}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${QWEN_CONFIG_DIR:-$HOME/.qwen}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${QWEN_CONFIG_DIR:-$HOME/.qwen}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${CODEBUDDY_CONFIG_DIR:-$HOME/.codebuddy}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${CODEBUDDY_CONFIG_DIR:-$HOME/.codebuddy}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${CLINE_CONFIG_DIR:-$HOME/.cline}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${CLINE_CONFIG_DIR:-$HOME/.cline}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${GROK_AGENTS_HOME:-$HOME/.agents}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${GROK_AGENTS_HOME:-$HOME/.agents}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${ANTIGRAVITY_CONFIG_DIR:-$HOME/.gemini/antigravity}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${ANTIGRAVITY_CONFIG_DIR:-$HOME/.gemini/antigravity}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${OPENCODE_CONFIG_DIR:-${XDG_CONFIG_HOME:-$HOME/.config}/opencode}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${OPENCODE_CONFIG_DIR:-${XDG_CONFIG_HOME:-$HOME/.config}/opencode}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${KILO_CONFIG_DIR:-${XDG_CONFIG_HOME:-$HOME/.config}/kilo}/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${KILO_CONFIG_DIR:-${XDG_CONFIG_HOME:-$HOME/.config}/kilo}/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; else echo "ERROR: gsd-tools.cjs not found at $GSD_TOOLS and gsd-tools is not on PATH. Run: npx -y @opengsd/gsd-core@latest --claude --local" >&2; exit 1; fi; if [ -n "${CLAUDE_ENV_FILE:-}" ] && [ -n "${GSD_TOOLS:-}" ]; then printf "export PATH='%s':\"\$PATH\"\n" "${GSD_TOOLS%/*}" >> "$CLAUDE_ENV_FILE" 2>/dev/null || true; fi +OUTGOING_MILESTONE=$(gsd_run query state.get milestone --raw 2>/dev/null || true) +printf '%s' "$OUTGOING_MILESTONE" > .planning/.gsd-outgoing-milestone 2>/dev/null || true +echo "Outgoing milestone (phase history archives under THIS version in step 6): ${OUTGOING_MILESTONE:-}" gsd_run query state.milestone-switch --milestone "v[X.Y]" --name "[Name]" ``` +**Capture the outgoing version now.** The lines above read the *current* (previous) milestone +version BEFORE the switch flips STATE.md's `milestone:` field to the new one, and persist it to +`.planning/.gsd-outgoing-milestone` so Step 6 can consume it via a shell variable — do NOT +transcribe the echoed value into a later command by hand. Step 6 reads that file back into +`--archive-version` so the previous milestone's phase directories archive under +`-phases/`, not the new one (#2288). Once `state.milestone-switch` runs, +current-milestone state no longer holds the outgoing version, which is why it is captured here. + The resulting Current Position section looks like: ```markdown @@ -206,12 +217,30 @@ SDK handler above — do not hand-edit STATE.md here. Delete MILESTONE-CONTEXT.md if exists (consumed). -Clear leftover phase directories from the previous milestone: +Clear leftover phase directories from the previous milestone. Read the outgoing version +persisted in Step 5 back into a shell variable and pass it as `--archive-version` so the +archive lands under the *previous* milestone's label — the switch in Step 5 has already +advanced current-milestone state, so without this override the archive would be mislabeled +with the *new* version (#2288). Use the shell variable directly (quoted) — never hand-retype +the captured value into the command, so untrusted STATE.md content cannot be re-parsed by the +shell: ```bash -gsd_run query phases.clear --confirm +OUTGOING_MILESTONE=$(cat .planning/.gsd-outgoing-milestone 2>/dev/null || true) +if [ -n "$OUTGOING_MILESTONE" ]; then + gsd_run query phases.clear --confirm --archive-version "$OUTGOING_MILESTONE" +else + gsd_run query phases.clear --confirm +fi +rm -f .planning/.gsd-outgoing-milestone 2>/dev/null || true ``` +If the captured file is empty or absent (a fresh project with no prior milestone), the +fallback branch runs `phases.clear --confirm` with no override — it then uses current-milestone +state, and a dated archive label only if no version label is resolvable at all. `phases.clear` +rejects any `--archive-version` value that is not a plain version token (no path separators or +`..`), so a malformed capture fails loudly rather than writing outside the archive directory. + Stage the phase archive move + source removal so they land in the same commit as the milestone start (atomic — no orphaned uncommitted deletions, no un-archived dirs carried forward). `phases.clear` archives each non-999 dir to `milestones/-phases/`; staging both dirs captures the new archive and the removals together (#1871). ```bash diff --git a/src/milestone.cts b/src/milestone.cts index 2cb958da6..905b7728d 100644 --- a/src/milestone.cts +++ b/src/milestone.cts @@ -37,6 +37,15 @@ const { planningPaths } = planningWorkspace; const { extractFrontmatter } = frontmatterMod; const { writeStateMd } = stateMod; +// #2288 security: a milestone version label becomes a filesystem directory +// component (`milestones/