From b5b3bd5db50719762d8dddd7a061e88b4332b741 Mon Sep 17 00:00:00 2001 From: Shi Qi Date: Wed, 4 Nov 2026 09:56:34 -0800 Subject: [PATCH] fix: reject path separators in worktree prefixes --- README.md | 2 + docs/configuration.md | 9 +++- lib/commands/help.sh | 2 +- lib/core.sh | 17 ++++++++ tests/cmd_create_integration.bats | 72 +++++++++++++++++++++++++++++++ tests/cmd_pr.bats | 45 +++++++++++++++++-- tests/cmd_rename.bats | 27 ++++++++++++ tests/core_create_worktree.bats | 25 +++++++++++ tests/core_resolve_target.bats | 10 +++++ 9 files changed, 204 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 882cbd8..ef500d4 100644 --- a/README.md +++ b/README.md @@ -483,6 +483,8 @@ git gtr config set gtr.ui.color never **Adapter safety:** Generic `gtr.editor.default` and `gtr.ai.default` values must resolve to safe PATH commands. Filesystem paths such as `./tool` and shell wrapper forms such as `sh -c ...` are rejected. Override-backed adapters like `claude`, `cursor`, and `nano` may include additional flags, for example `claude --continue` or `nano -w`. +**Worktree naming:** `gtr.worktrees.prefix` is a folder-name prefix, such as `dev-`, and cannot contain `/` or `\`. Set `gtr.worktrees.dir` to choose the parent directory. See [worktree settings](docs/configuration.md#worktree-settings). + **Configuration precedence** (highest to lowest): 1. `git config --local` (`.git/config`) - personal overrides diff --git a/docs/configuration.md b/docs/configuration.md index d6a0df8..e3c56c0 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -97,7 +97,7 @@ gtr.worktrees.dir = .worktrees # Home directory (tilde expansion) gtr.worktrees.dir = ~/worktrees/my-project -# Folder prefix (default: "") +# Folder name prefix, without / or \ (default: "") gtr.worktrees.prefix = dev- # Default branch (default: auto-detect) @@ -107,6 +107,13 @@ gtr.defaultBranch = main gtr.defaultRemote = upstream ``` +The prefix is prepended literally to the sanitized folder name: `dev-` and +`feature/auth` produce `dev-feature-auth`. Empty prefixes are supported. Prefixes +containing `/` or `\` are rejected from every configuration source, including +`.gtrconfig` and `GTR_WORKTREES_PREFIX`. Use `gtr.worktrees.dir` to select a parent +directory; for example, replace a `team/dev-` prefix with a `dev-` prefix and a +base directory ending in `team`. + > [!IMPORTANT] > If storing worktrees inside the repository, add the directory to `.gitignore`. diff --git a/lib/commands/help.sh b/lib/commands/help.sh index bc355c6..9afd159 100644 --- a/lib/commands/help.sh +++ b/lib/commands/help.sh @@ -752,7 +752,7 @@ WORKFLOW EXAMPLES: CONFIGURATION OPTIONS: gtr.worktrees.dir Worktrees base directory - gtr.worktrees.prefix Worktree folder prefix (default: "") + gtr.worktrees.prefix Folder name prefix, no / or \ (default: "") gtr.defaultBranch Default branch (default: auto) gtr.defaultRemote Default remote (default: origin) gtr.editor.default Default editor diff --git a/lib/core.sh b/lib/core.sh index b2173e0..ca9b500 100644 --- a/lib/core.sh +++ b/lib/core.sh @@ -35,6 +35,18 @@ sanitize_branch_name() { printf "%s" "$branch" | sed -e 's/[\/\\ :*?"<>|#]/-/g' -e 's/^-*//' -e 's/-*$//' } +# Prefixes name folders; worktrees.dir selects their parent directory. +# Reject both POSIX and Windows separators instead of changing configured names. +_validate_worktree_prefix() { + case "$1" in + */*|*\\*) + log_error "Invalid gtr.worktrees.prefix: path separators are not allowed; use gtr.worktrees.dir for directories" + return 1 + ;; + esac + return 0 +} + # Canonicalize a path to its absolute form, resolving symlinks # Usage: canonicalize_path path # Returns: canonical path or empty string on failure @@ -413,6 +425,8 @@ resolve_target() { local prefix="$4" local path branch sanitized_name + _validate_worktree_prefix "$prefix" || return 1 + # Special case: ID 1 is always the repo root if [ "$identifier" = "1" ]; then path="$repo_root" @@ -819,6 +833,8 @@ create_worktree() { local no_checkout="${11:-0}" local add_source="${12:-}" + _validate_worktree_prefix "$prefix" || return 1 + local sanitized_name sanitized_name=$(_resolve_folder_name "$branch_name" "$custom_name" "$folder_override") || return 1 @@ -949,6 +965,7 @@ resolve_repo_context() { _ctx_repo_root=$(discover_repo_root) || return 1 _ctx_base_dir=$(resolve_base_dir "$_ctx_repo_root") _ctx_prefix=$(cfg_default gtr.worktrees.prefix GTR_WORKTREES_PREFIX "") + _validate_worktree_prefix "$_ctx_prefix" || return 1 } # List all worktree branch names (excluding main repo) diff --git a/tests/cmd_create_integration.bats b/tests/cmd_create_integration.bats index 7389a65..18427e9 100644 --- a/tests/cmd_create_integration.bats +++ b/tests/cmd_create_integration.bats @@ -28,6 +28,78 @@ _cmd_create_capture_stdout() { [ -d "$TEST_WORKTREES_DIR/new-feature" ] } +@test "cmd_create accepts an untrusted folder prefix with sanitized branch and custom names" { + git config -f .gtrconfig worktrees.prefix dev- + [ ! -d "$_GTR_TRUST_DIR" ] + + run "$PROJECT_ROOT/bin/gtr" new feature/auth --name backend --from HEAD --no-fetch --no-copy --no-hooks --yes + [ "$status" -eq 0 ] + [ -f "$TEST_WORKTREES_DIR/dev-feature-auth-backend/.git" ] + + run "$PROJECT_ROOT/bin/gtr" new review --folder chosen --from HEAD --no-fetch --no-copy --no-hooks --yes + [ "$status" -eq 0 ] + [ -f "$TEST_WORKTREES_DIR/dev-chosen/.git" ] + [ ! -d "$_GTR_TRUST_DIR" ] +} + +@test "cmd_create rejects traversal prefixes from shared local global and environment config" { + local source base_dir="$TEST_REPO/prefix-fixture/trees" + mkdir -p "$TEST_REPO/prefix-fixture" + git config gtr.worktrees.dir "$base_dir" + export GIT_CONFIG_GLOBAL="$TEST_REPO/prefix-global-config" + for source in shared local global environment; do + case "$source" in + shared) + git config -f .gtrconfig worktrees.prefix ../planted- + git add .gtrconfig + git commit -m "shared prefix" --quiet + ;; + local) git config gtr.worktrees.prefix ../planted- ;; + global) git config --global gtr.worktrees.prefix ../planted- ;; + environment) export GTR_WORKTREES_PREFIX=../planted- ;; + esac + + run "$PROJECT_ROOT/bin/gtr" new rejected-prefix --from HEAD --no-fetch --no-copy --no-hooks --yes + [ "$status" -eq 1 ] + [[ "$output" == *"Invalid gtr.worktrees.prefix"* ]] + [ ! -e "$base_dir" ] + [ ! -e "$TEST_REPO/prefix-fixture/planted-rejected-prefix" ] + ! git show-ref --verify --quiet refs/heads/rejected-prefix + [ ! -d "$_GTR_TRUST_DIR" ] + + case "$source" in + shared) git config -f .gtrconfig --unset worktrees.prefix ;; + local) git config --unset gtr.worktrees.prefix ;; + global) git config --global --unset gtr.worktrees.prefix ;; + environment) unset GTR_WORKTREES_PREFIX ;; + esac + done +} + +@test "cmd_create validates the effective prefix after config precedence" { + git config -f .gtrconfig worktrees.prefix ../planted- + git config gtr.worktrees.prefix dev- + + run "$PROJECT_ROOT/bin/gtr" new safe-override --from HEAD --no-fetch --no-copy --no-hooks --yes + + [ "$status" -eq 0 ] + [ -f "$TEST_WORKTREES_DIR/dev-safe-override/.git" ] +} + +@test "cmd_create rejects invalid prefixes without emitting porcelain records" { + git config gtr.worktrees.dir "$TEST_REPO/prefix-fixture/trees" + git config gtr.worktrees.prefix '../planted-' + local stdout_file="$TEST_REPO/prefix-stdout" stderr_file="$TEST_REPO/prefix-stderr" + + run bash -c '"$1" new rejected-prefix --from HEAD --no-fetch --porcelain >"$2" 2>"$3"' \ + _ "$PROJECT_ROOT/bin/gtr" "$stdout_file" "$stderr_file" + + [ "$status" -eq 1 ] + [ ! -s "$stdout_file" ] + grep -q "Invalid gtr.worktrees.prefix" "$stderr_file" + ! git show-ref --verify --quiet refs/heads/rejected-prefix +} + @test "cmd_create treats untrusted defaults.remote as a fetch operand" { # Every remote is local; --all must not cause an unrelated remote to be fetched. git remote add origin "$TEST_REPO" diff --git a/tests/cmd_pr.bats b/tests/cmd_pr.bats index e6957d4..6cf17f8 100644 --- a/tests/cmd_pr.bats +++ b/tests/cmd_pr.bats @@ -60,6 +60,44 @@ teardown() { [ "$(git config branch.review/pr-123.merge)" = "refs/heads/feature/from-pr" ] } +@test "cmd_pr fallback applies an ordinary folder prefix" { + git config -f .gtrconfig worktrees.prefix dev- + + cmd_pr 123 --remote origin --no-copy --no-hooks --yes + + [ -f "$TEST_WORKTREES_DIR/dev-feature-from-pr/.git" ] + [ "$(git -C "$TEST_WORKTREES_DIR/dev-feature-from-pr" rev-parse HEAD)" = "$TEST_PR_SHA" ] +} + +@test "cmd_pr rejects traversal prefixes before native checkout or fallback fetch" { + git config gtr.worktrees.dir "$TEST_REPO/prefix-fixture/trees" + git config -f .gtrconfig worktrees.prefix ../planted- + export TEST_GH_LOG="$TEST_MOCK_BIN/gh.log" + cat > "$TEST_MOCK_BIN/gh" <<'SCRIPT' +#!/usr/bin/env bash +printf '%s\n' "$*" >> "$TEST_GH_LOG" +if [ "$1" = "pr" ] && [ "$2" = "checkout" ] && [ "$3" = "--help" ]; then + printf ' --worktree path\n' +elif [ "$1" = "pr" ] && [ "$2" = "view" ]; then + printf '123\tfeature/from-pr\tocto\texample\thttps://github.com/base/example/pull/123\ttrue\ttrue\n' +fi +SCRIPT + local mode + for mode in native fallback; do + local args=() + [ "$mode" = "fallback" ] && args=(--remote origin) + + run "$PROJECT_ROOT/bin/gtr" pr 123 "${args[@]}" --no-copy --no-hooks --yes + + [ "$status" -eq 1 ] + [[ "$output" == *"Invalid gtr.worktrees.prefix"* ]] + [ ! -e "$TEST_GH_LOG" ] + [ ! -e "$TEST_REPO/prefix-fixture" ] + [ ! -e "$TEST_REPO/.git/FETCH_HEAD" ] + ! git show-ref --verify --quiet refs/heads/feature/from-pr + done +} + @test "cmd_pr reuses ssh remote matching resolved PR base repository" { git remote add upstream git@github.com:base/example.git @@ -115,6 +153,7 @@ SCRIPT } @test "cmd_pr delegates worktree creation to gh when supported" { + git config gtr.worktrees.prefix dev- export TEST_GH_LOG="$TEST_MOCK_BIN/gh.log" cat > "$TEST_MOCK_BIN/gh" <<'SCRIPT' #!/usr/bin/env bash @@ -144,12 +183,12 @@ SCRIPT cmd_pr 123 --no-copy --no-hooks --yes - [ -d "$TEST_WORKTREES_DIR/feature-from-pr" ] + [ -d "$TEST_WORKTREES_DIR/dev-feature-from-pr" ] local logged_path logged_branch IFS=$'\t' read -r logged_path logged_branch < "$TEST_GH_LOG" - [ "${logged_path##*/}" = "feature-from-pr" ] + [ "${logged_path##*/}" = "dev-feature-from-pr" ] [ "$logged_branch" = "feature/from-pr" ] - [ "$(git -C "$TEST_WORKTREES_DIR/feature-from-pr" rev-parse HEAD)" = "$TEST_PR_SHA" ] + [ "$(git -C "$TEST_WORKTREES_DIR/dev-feature-from-pr" rev-parse HEAD)" = "$TEST_PR_SHA" ] } @test "cmd_pr fetches the resolved PR base repository by default" { diff --git a/tests/cmd_rename.bats b/tests/cmd_rename.bats index 354285e..d417a59 100644 --- a/tests/cmd_rename.bats +++ b/tests/cmd_rename.bats @@ -32,6 +32,33 @@ teardown() { [ "$status" -ne 0 ] } +@test "cmd_rename preserves an ordinary folder prefix" { + git config gtr.worktrees.prefix dev- + create_worktree "$TEST_WORKTREES_DIR" "dev-" "old-name" "HEAD" "none" "1" >/dev/null + + cmd_rename old-name feature/renamed --yes + + [ -f "$TEST_WORKTREES_DIR/dev-feature-renamed/.git" ] + [ ! -e "$TEST_WORKTREES_DIR/dev-old-name" ] + [ "$(get_current_branch "$TEST_WORKTREES_DIR/dev-feature-renamed")" = "feature/renamed" ] +} + +@test "cmd_rename rejects traversal prefixes without moving the worktree or renaming its branch" { + create_test_worktree "old-name" + local old_path="$TEST_WT_PATH" + git config gtr.worktrees.dir "$TEST_REPO/prefix-fixture/trees" + git config -f .gtrconfig worktrees.prefix ../planted- + + run "$PROJECT_ROOT/bin/gtr" mv old-name renamed --yes + + [ "$status" -eq 1 ] + [[ "$output" == *"Invalid gtr.worktrees.prefix"* ]] + [ -f "$old_path/.git" ] + [ "$(get_current_branch "$old_path")" = "old-name" ] + [ ! -e "$TEST_REPO/prefix-fixture" ] + ! git show-ref --verify --quiet refs/heads/renamed +} + # ── Error cases ────────────────────────────────────────────────────────────── @test "cmd_rename fails with insufficient args" { diff --git a/tests/core_create_worktree.bats b/tests/core_create_worktree.bats index 7ce3709..a8b34e9 100644 --- a/tests/core_create_worktree.bats +++ b/tests/core_create_worktree.bats @@ -166,6 +166,31 @@ teardown() { [ -d "$wt_path" ] } +@test "create_worktree preserves empty dotted and spaced folder prefixes" { + local prefix wt_path index=0 + for prefix in "" "dev-" "." ".." "my team-"; do + wt_path=$(create_worktree "$TEST_WORKTREES_DIR" "$prefix" "prefix-$index" "HEAD" "none" "1") + [ "$wt_path" = "$TEST_WORKTREES_DIR/${prefix}prefix-$index" ] + [ -f "$wt_path/.git" ] + index=$((index + 1)) + done +} + +@test "create_worktree rejects prefix path separators before creating directories or branches" { + # Even a regression can only create destinations inside this disposable repo. + local base_dir="$TEST_REPO/prefix-fixture/trees" prefix + mkdir -p "$TEST_REPO/prefix-fixture" + for prefix in "../planted-" "group/../../planted-" "group/dev-" "/absolute-" \ + '..\planted-' 'group\dev-' 'C:\dev-'; do + run create_worktree "$base_dir" "$prefix" "rejected-prefix" "HEAD" "none" "1" + [ "$status" -eq 1 ] + [[ "$output" == *"Invalid gtr.worktrees.prefix"* ]] + [ ! -e "$base_dir" ] + [ ! -e "$TEST_REPO/prefix-fixture/planted-rejected-prefix" ] + ! git show-ref --verify --quiet refs/heads/rejected-prefix + done +} + @test "create_worktree applies custom name suffix" { local wt_path wt_path=$(create_worktree "$TEST_WORKTREES_DIR" "" "feature" "HEAD" "none" "1" "0" "backend") diff --git a/tests/core_resolve_target.bats b/tests/core_resolve_target.bats index fddb989..ded463e 100644 --- a/tests/core_resolve_target.bats +++ b/tests/core_resolve_target.bats @@ -52,6 +52,16 @@ teardown() { [[ "$result" == "0"$'\t'"$TEST_WORKTREES_DIR/wt-prefixed"* ]] } +@test "resolve_target rejects traversal prefixes even when the destination exists" { + local base_dir="$TEST_REPO/prefix-fixture/trees" + mkdir -p "$base_dir" "$TEST_REPO/prefix-fixture/planted-feature" + + run resolve_target "feature" "$TEST_REPO" "$base_dir" "../planted-" + + [ "$status" -eq 1 ] + [[ "$output" == *"Invalid gtr.worktrees.prefix"* ]] +} + @test "unpack_target parses TSV into context globals" { local tsv="0"$'\t'"/some/path"$'\t'"my-branch" unpack_target "$tsv"