diff --git a/.ai-context/COMMANDS.md b/.ai-context/COMMANDS.md index 5dec535d..287b9657 100644 --- a/.ai-context/COMMANDS.md +++ b/.ai-context/COMMANDS.md @@ -190,6 +190,10 @@ the v1.x compatibility window; new automation should use `--debug-wrapper`. - `basectl gh worktree prune` previews safe merged-worktree cleanup by default; pass `--closed-unmerged` to include clean worktrees tied to closed, unmerged pull requests. + - `basectl gh review fetch ` creates a detached review worktree in + the peer `../-review-worktrees` root by default. An explicit + `--root` must resolve outside the repository; nested and symlinked nested + roots are rejected before GitHub or Git mutation. - `basectl gh project doctor --project ` - inspect Project metadata fields against the Base Project schema. - `basectl gh project configure --project <title>` - create or repair the diff --git a/cli/bash/commands/basectl/subcommands/gh.sh b/cli/bash/commands/basectl/subcommands/gh.sh index acca303c..54dace4c 100644 --- a/cli/bash/commands/basectl/subcommands/gh.sh +++ b/cli/bash/commands/basectl/subcommands/gh.sh @@ -650,18 +650,20 @@ base_gh_slug() { base_gh_issue_worktree_path() { local issue="$1" slug="$2" - local repo_root repo_name repo_parent slug_short + local repo_root repo_name repo_parent worktree_root slug_short repo_root="$(git rev-parse --show-toplevel)" || return 1 + repo_root="$(base_gh_resolve_physical_path "$repo_root")" || return 1 repo_name="$(basename "$repo_root")" repo_parent="$(dirname "$repo_root")" + worktree_root="$(base_gh_validate_worktree_root "$repo_parent/$repo_name-worktrees" "issue worktree root")" || return 1 slug_short="${slug:0:40}" while [[ "$slug_short" == *- ]]; do slug_short="${slug_short%-}" done [[ -n "$slug_short" ]] || slug_short="work" - printf '%s/%s-worktrees/%s-%s\n' "$repo_parent" "$repo_name" "$issue" "$slug_short" + printf '%s/%s-%s\n' "$worktree_root" "$issue" "$slug_short" } base_gh_issue_category() { diff --git a/cli/bash/commands/basectl/subcommands/gh_branch_worktree.sh b/cli/bash/commands/basectl/subcommands/gh_branch_worktree.sh index efb41dfb..40c3913f 100644 --- a/cli/bash/commands/basectl/subcommands/gh_branch_worktree.sh +++ b/cli/bash/commands/basectl/subcommands/gh_branch_worktree.sh @@ -820,6 +820,104 @@ base_gh_resolve_physical_path() { (cd "$path" && pwd -P) } +base_gh_normalize_absolute_path() { + local path="$1" + local component + local -a components=() normalized=() + + [[ "$path" == /* ]] || return 1 + local IFS='/' + read -r -a components <<< "$path" + for component in "${components[@]}"; do + case "$component" in + ''|.) + ;; + ..) + if ((${#normalized[@]} > 0)); then + normalized=("${normalized[@]:0:${#normalized[@]}-1}") + fi + ;; + *) + normalized+=("$component") + ;; + esac + done + + if ((${#normalized[@]} == 0)); then + printf '/\n' + else + (IFS=/; printf '/%s\n' "${normalized[*]}") + fi +} + +base_gh_resolve_path_for_creation() { + local path="$1" + local candidate suffix physical_candidate + + [[ -n "$path" ]] || return 1 + if [[ "$path" != /* ]]; then + path="$PWD/$path" + fi + + candidate="$path" + suffix="" + while [[ ! -e "$candidate" ]]; do + [[ "$candidate" != "/" ]] || return 1 + suffix="/$(basename "$candidate")$suffix" + candidate="$(dirname "$candidate")" + done + [[ -d "$candidate" ]] || return 1 + + physical_candidate="$(cd "$candidate" && pwd -P)" || return 1 + base_gh_normalize_absolute_path "$physical_candidate$suffix" +} + +base_gh_path_is_within_physical_root() { + local path="$1" + local root="$2" + local candidate + + candidate="$path" + while [[ ! -e "$candidate" ]]; do + [[ "$candidate" != "/" ]] || return 1 + candidate="$(dirname "$candidate")" || return 1 + done + [[ -d "$candidate" ]] || return 1 + candidate="$(cd "$candidate" && pwd -P)" || return 1 + + while :; do + [[ "$candidate" -ef "$root" ]] && return 0 + [[ "$candidate" == "/" ]] && return 1 + candidate="$(cd "$candidate/.." && pwd -P)" || return 1 + done +} + +base_gh_validate_worktree_root() { + local requested="$1" + local label="${2:-worktree root}" + local repo_root resolved_root + + repo_root="$(git rev-parse --show-toplevel 2>/dev/null)" || { + base_gh_error "Unable to resolve the repository root before validating $label." + return 1 + } + repo_root="$(base_gh_resolve_physical_path "$repo_root")" || { + base_gh_error "Unable to resolve the physical repository root before validating $label." + return 1 + } + resolved_root="$(base_gh_resolve_path_for_creation "$requested")" || { + base_gh_error "Unable to resolve $label '$requested'." + return 1 + } + + if base_gh_path_is_within_physical_root "$resolved_root" "$repo_root"; then + base_gh_error "Refusing $label '$requested': it resolves inside repository root '$repo_root'. Use a peer directory outside the repository." + return 1 + fi + + printf '%s\n' "$resolved_root" +} + base_gh_list_worktree_branches() { base_git_list_worktree_branches . } diff --git a/cli/bash/commands/basectl/subcommands/gh_review.sh b/cli/bash/commands/basectl/subcommands/gh_review.sh index 1876faac..d41248dc 100644 --- a/cli/bash/commands/basectl/subcommands/gh_review.sh +++ b/cli/bash/commands/basectl/subcommands/gh_review.sh @@ -46,7 +46,8 @@ Options: --name <slug> Required lower-case kebab-case review name. --repo <owner/name> Pull request repository. Defaults to origin and must match the checkout's origin repository. - --root <path> Review worktree root. Defaults to ../<repo>-review-worktrees. + --root <path> Review worktree root outside the repository. Defaults to + ../<repo>-review-worktrees; nested roots are rejected. --refresh Replace an existing clean review worktree with the current PR head. -h, --help Show this help text. EOF @@ -129,6 +130,7 @@ base_gh_review_default_root() { base_gh_review_resolve_root() { local requested="${1:-}" + local validate_root="${2:-0}" local root if [[ -n "$requested" ]]; then @@ -136,6 +138,9 @@ base_gh_review_resolve_root() { else root="$(base_gh_review_default_root)" || return 1 fi + if ((validate_root)); then + root="$(base_gh_validate_worktree_root "$root" "review worktree root")" || return 1 + fi mkdir -p "$root" || return 1 (cd "$root" && pwd -P) } @@ -321,7 +326,7 @@ base_gh_review_fetch() { } fi root="${parsed_options[root]:-}" - review_root="$(base_gh_review_resolve_root "$root")" || { + review_root="$(base_gh_review_resolve_root "$root" 1)" || { base_gh_error "Unable to prepare the review worktree root." return 1 } diff --git a/cli/bash/commands/basectl/tests/gh-review.bats b/cli/bash/commands/basectl/tests/gh-review.bats index f4e77aa9..db1fdbd8 100644 --- a/cli/bash/commands/basectl/tests/gh-review.bats +++ b/cli/bash/commands/basectl/tests/gh-review.bats @@ -113,6 +113,72 @@ review_metadata_path() { jq -e '.pull_request == 2621 and .slug == "stale-worktree" and .head_sha == env.BASE_REVIEW_TEST_HEAD_SHA' "$metadata" >/dev/null } +@test "review fetch rejects a nested worktree root before GitHub or Git mutation" { + local repo nested_root + + create_review_repo + repo="$BASE_REVIEW_TEST_REPO" + nested_root="$repo/review-worktrees" + + run_review "$repo" fetch 2621 --name nested-root --root "$nested_root" + + [ "$status" -eq 1 ] + [[ "$output" == *"Refusing review worktree root"* ]] + [[ "$output" == *"inside repository root"* ]] + [ ! -e "$nested_root" ] + ! git -C "$repo" show-ref --verify --quiet refs/base-review/pr/2621/nested-root +} + +@test "review fetch rejects a symlinked nested worktree root" { + local repo nested_link + + create_review_repo + repo="$BASE_REVIEW_TEST_REPO" + nested_link="$TEST_TMPDIR/review-worktrees-link" + ln -s "$repo" "$nested_link" + + run_review "$repo" fetch 2621 --name symlinked-root --root "$nested_link/children" + + [ "$status" -eq 1 ] + [[ "$output" == *"Refusing review worktree root"* ]] + [[ "$output" == *"inside repository root"* ]] + ! git -C "$repo" show-ref --verify --quiet refs/base-review/pr/2621/symlinked-root +} + +@test "review fetch rejects a missing-sibling traversal into the repository" { + local repo nested_root + + create_review_repo + repo="$BASE_REVIEW_TEST_REPO" + nested_root="$TEST_TMPDIR/review-repo-shadow/../review-repo/review-worktrees" + + run_review "$repo" fetch 2621 --name traversal-root --root "$nested_root" + + [ "$status" -eq 1 ] + [[ "$output" == *"Refusing review worktree root"* ]] + [[ "$output" == *"inside repository root"* ]] + [ ! -e "$TEST_TMPDIR/review-repo/review-worktrees" ] + ! git -C "$repo" show-ref --verify --quiet refs/base-review/pr/2621/traversal-root +} + +@test "review fetch treats glob characters in a peer root literally" { + local repo review_root worktree + + create_review_repo + repo="$BASE_REVIEW_TEST_REPO" + review_root="$TEST_TMPDIR/review[*?]-worktrees" + mkdir -p "$review_root" + review_root="$(cd "$review_root" && pwd -P)" + worktree="$review_root/pr-2621-literal-glob" + write_review_gh_mock + + run_review "$repo" fetch 2621 --name literal-glob --repo basefoundry/base --root "$review_root" + + [ "$status" -eq 0 ] + [[ "$output" == *"CREATED $worktree"* ]] + [ -d "$worktree" ] +} + @test "review prune removes only a verified merged review checkout" { local repo review_root worktree metadata diff --git a/docs/command-reference.md b/docs/command-reference.md index e10b9729..41ef1810 100644 --- a/docs/command-reference.md +++ b/docs/command-reference.md @@ -241,7 +241,7 @@ daily project loop commands from the local checkout. | `basectl gh branch stale` | Report stale local branches. Stable inspection JSON uses the shared v1 envelope. | `--days <days>`, `--format <text\|json>` | | `basectl gh branch prune` | Prune safe merged branches. Pass `--closed-unmerged` to include branches whose pull requests were closed without merging. | `--dry-run`, `--yes`, `--remote`, `--closed-unmerged` | | `basectl gh worktree prune` | Prune stale merged branch-attached worktrees. Pass `--closed-unmerged` to include clean worktrees tied to closed, unmerged pull requests; unmanaged detached worktree metadata remains an explicit `git worktree prune` operation. Use `gh review prune` for Base-owned review worktrees. | `--dry-run`, `--yes`, `--closed-unmerged` | -| `basectl gh review fetch <number>` | Fetch a pull-request head into a Base-owned namespaced ref and create a detached review worktree. It never creates an ordinary local `pr-*` branch. | `--name <slug>`, `--repo <owner/name>`, `--root <path>`, `--refresh` | +| `basectl gh review fetch <number>` | Fetch a pull-request head into a Base-owned namespaced ref and create a detached review worktree outside the repository. It never creates an ordinary local `pr-*` branch; an explicit `--root` inside the repository or one of its descendants is rejected before Git or GitHub mutation. | `--name <slug>`, `--repo <owner/name>`, `--root <path>`, `--refresh` | | `basectl gh review prune` | Preview or remove safe Base-owned detached review worktrees and their namespaced refs. The default is dry-run; open, dirty, changed, invalid, missing, and unowned checkouts are retained. | `--dry-run`, `--yes`, `--closed-unmerged`, `--pr <number>`, `--root <path>` | | `basectl gh project doctor` | Inspect GitHub Project metadata against the Base Project schema. | `--project <title>`, `--owner <login>`, `--schema base-project` | | `basectl gh project configure` | Create or repair Base-managed Project metadata. | `--project <title>`, `--owner <login>`, `--repo <owner/name>`, `--schema base-project`, `--config <path>`, `--copy-fields-from <title>`, `--replace-project`, `--initiative-option <name>`, `--dry-run` | diff --git a/docs/github-workflow.md b/docs/github-workflow.md index b172ee13..3cdf668f 100644 --- a/docs/github-workflow.md +++ b/docs/github-workflow.md @@ -408,6 +408,13 @@ Issue work should use: ~/work/base-worktrees/<slug> ``` +Base-supported worktree creation keeps implementation and review roots next to +the repository, never inside it. `basectl gh issue start` prints the canonical +peer path, and `basectl gh review fetch` rejects an explicit root that resolves +to the repository or any descendant of it, including through a symlink. This +keeps worktrees out of the primary checkout so workspace synchronization can +reason about the checkout independently of unmerged work. + Before creating a worktree, check whether the current checkout is already a linked worktree for the intended issue: @@ -526,8 +533,11 @@ refs/base-review/pr/<number>/<slug> The worktree is detached, the review ref is not an ordinary `refs/heads/pr-*` branch, and the default root is next to the repository root represented by the common Git directory. Use `--root <path>` when a different -review workspace is needed. Use `--refresh` only to replace an existing clean -review checkout with the current pull-request head. +review workspace is needed; the path must resolve outside the repository and +its descendants. Use `--refresh` only to replace an existing clean review +checkout with the current pull-request head. Existing nested worktrees are not +moved or deleted automatically; inspect and migrate them explicitly after +preserving any local work. The optional `--repo <owner/name>` selector must match the checkout's `origin` repository; use a checkout of that repository when reviewing an upstream PR.