From d933012ad49e09e35048b907775c59b84088400c Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Thu, 5 Mar 2026 19:28:50 -0800 Subject: [PATCH] correctness: validate WT_*_BASE config paths are absolute, reject globs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Config path variables (WT_MAIN_REPO_ROOT, WT_WORKTREES_BASE, WT_IDEA_FILES_BASE) are now validated on every bin/wt-* invocation. Relative paths and glob characters are rejected with actionable error messages pointing to the config file or git config to fix. This prevents subtle bugs where relative paths silently resolve against whatever CWD the script happens to run from, producing unpredictable worktree locations or metadata vault paths. Globs could cause unexpected shell expansion in find/for loops. Shell startup (source wt.sh) is never affected — validation only runs in bin/wt-* scripts. wt-context is exempt so users can fix bad config. --- bin/wt-add | 1 + bin/wt-cd | 1 + bin/wt-list | 1 + bin/wt-metadata-export | 1 + bin/wt-metadata-import | 1 + bin/wt-remove | 1 + bin/wt-switch | 1 + lib/wt-common | 54 ++++++++++ test/integration/wt-path-validation.bats | 89 +++++++++++++++++ test/unit/wt-common.bats | 119 +++++++++++++++++++++++ 10 files changed, 269 insertions(+) create mode 100644 test/integration/wt-path-validation.bats diff --git a/bin/wt-add b/bin/wt-add index e85a5ff..445a19f 100755 --- a/bin/wt-add +++ b/bin/wt-add @@ -77,6 +77,7 @@ else echo "Error: Cannot find wt-common" >&2 exit 1 fi +wt_require_valid_config || exit 1 usage() { echo "Usage: $(basename "$0") [git worktree add arguments...]" diff --git a/bin/wt-cd b/bin/wt-cd index 377fcf7..f240c29 100755 --- a/bin/wt-cd +++ b/bin/wt-cd @@ -40,6 +40,7 @@ else echo "Error: Cannot find wt-common" >&2 exit 1 fi +wt_require_valid_config || exit 1 # Source wt-choose using the helper wt_source wt-choose diff --git a/bin/wt-list b/bin/wt-list index 0a9f451..158c5ea 100755 --- a/bin/wt-list +++ b/bin/wt-list @@ -42,6 +42,7 @@ else echo "Error: Cannot find wt-common" >&2 exit 1 fi +wt_require_valid_config || exit 1 usage() { cat <&2 exit 1 fi +wt_require_valid_config || exit 1 usage() { cat >&2 <&2 exit 1 fi +wt_require_valid_config || exit 1 # Source wt-choose using the helper wt_source wt-choose diff --git a/bin/wt-remove b/bin/wt-remove index fe1d270..039284a 100755 --- a/bin/wt-remove +++ b/bin/wt-remove @@ -46,6 +46,7 @@ else echo "Error: Cannot find wt-common" >&2 exit 1 fi +wt_require_valid_config || exit 1 # Source wt-choose using the helper wt_source wt-choose diff --git a/bin/wt-switch b/bin/wt-switch index 5b4f206..4d42f58 100755 --- a/bin/wt-switch +++ b/bin/wt-switch @@ -48,6 +48,7 @@ else echo "Error: Cannot find wt-common" >&2 exit 1 fi +wt_require_valid_config || exit 1 # Source wt-choose using the helper wt_source wt-choose diff --git a/lib/wt-common b/lib/wt-common index 3914d49..6ee31bf 100644 --- a/lib/wt-common +++ b/lib/wt-common @@ -248,6 +248,60 @@ _wt_expand_path() { echo "${path/#\~\//$HOME/}" } +# Check if a path value is valid for a directory config variable. +# Valid means: absolute path (starts with /) and contains no glob characters. +# Args: $1 = path value +# Returns: 0 if valid, 1 if invalid +_wt_is_valid_path_config() { + local path="$1" + + # Must be absolute + [[ "$path" != /* ]] && return 1 + + # Must not contain glob metacharacters + # shellcheck disable=SC2254 # intentional glob check via case pattern + case "$path" in + *\**|*\?*|*\[*) return 1 ;; + esac + + return 0 +} + +# Validate that all directory config variables are absolute paths without globs. +# Prints actionable error messages to stderr on failure. +# Returns: 0 if all valid, 1 if any invalid +# +# This is called by bin/wt-* scripts after bootstrap. It is NOT called +# during shell startup (wt.sh sourcing) to avoid breaking the user's shell. +wt_require_valid_config() { + local rc=0 + local var val + + for var in WT_MAIN_REPO_ROOT WT_WORKTREES_BASE WT_IDEA_FILES_BASE; do + eval "val=\"\${$var:-}\"" + if [[ -z "$val" ]]; then + continue # empty/unset — fallback defaults handle this + fi + if ! _wt_is_valid_path_config "$val"; then + error "$var must be an absolute path (no globs): $val" + rc=1 + fi + done + + if [[ $rc -ne 0 ]]; then + local config_file="" + if [[ -n "${WT_CONTEXT_NAME:-}" ]]; then + config_file="$HOME/.wt/repos/${WT_CONTEXT_NAME}.conf" + fi + if [[ -n "$config_file" && -f "$config_file" ]]; then + printf " Fix the value(s) in: %s\n" "$config_file" >&2 + else + printf " Check your context config or git local config (wt.* keys)\n" >&2 + fi + fi + + return $rc +} # ───────────────────────────────────────────────────────────────────────────── # Color support (only if output is a TTY) diff --git a/test/integration/wt-path-validation.bats b/test/integration/wt-path-validation.bats new file mode 100644 index 0000000..f2f43f0 --- /dev/null +++ b/test/integration/wt-path-validation.bats @@ -0,0 +1,89 @@ +#!/usr/bin/env bats + +# Integration tests for path validation in bin/wt-* scripts + +setup() { + load '../test_helper/common' + setup_test_env + + # Create mock repo + REPO=$(create_mock_repo "$BATS_TEST_TMPDIR/repo") + + # Create test context with valid config (don't load into env — + # we want the subprocess to read from .conf, not inherited env) + create_test_context "test" "$REPO" +} + +teardown() { + teardown_test_env +} + +# Helper: unset all WT_* config vars so subprocess reads from .conf +_unset_wt_vars() { + unset WT_MAIN_REPO_ROOT WT_WORKTREES_BASE WT_IDEA_FILES_BASE + unset WT_ACTIVE_WORKTREE WT_BASE_BRANCH WT_METADATA_PATTERNS + unset WT_CONTEXT_NAME +} + +# ============================================================================= +# bin scripts abort on invalid config +# ============================================================================= + +@test "wt-add aborts with relative WT_WORKTREES_BASE" { + local conf="$TEST_HOME/.wt/repos/test.conf" + sed -i.bak 's|^WT_WORKTREES_BASE=.*|WT_WORKTREES_BASE="relative/worktrees"|' "$conf" + _unset_wt_vars + + run "$TEST_HOME/.wt/bin/wt-add" -b some-branch + assert_failure + assert_output --partial "WT_WORKTREES_BASE" + assert_output --partial "must be an absolute path" +} + +@test "wt-list aborts with glob in WT_MAIN_REPO_ROOT" { + local conf="$TEST_HOME/.wt/repos/test.conf" + sed -i.bak 's|^WT_MAIN_REPO_ROOT=.*|WT_MAIN_REPO_ROOT="/tmp/repo-*"|' "$conf" + _unset_wt_vars + + run "$TEST_HOME/.wt/bin/wt-list" + assert_failure + assert_output --partial "WT_MAIN_REPO_ROOT" + assert_output --partial "must be an absolute path" +} + +@test "wt-switch aborts with relative WT_IDEA_FILES_BASE" { + local conf="$TEST_HOME/.wt/repos/test.conf" + sed -i.bak 's|^WT_IDEA_FILES_BASE=.*|WT_IDEA_FILES_BASE="idea-files"|' "$conf" + _unset_wt_vars + + run "$TEST_HOME/.wt/bin/wt-switch" "$REPO" + assert_failure + assert_output --partial "WT_IDEA_FILES_BASE" + assert_output --partial "must be an absolute path" +} + +@test "wt-context is not guarded and works with invalid config" { + local conf="$TEST_HOME/.wt/repos/test.conf" + sed -i.bak 's|^WT_WORKTREES_BASE=.*|WT_WORKTREES_BASE="relative/worktrees"|' "$conf" + _unset_wt_vars + + run "$TEST_HOME/.wt/bin/wt-context" --list + assert_success +} + +@test "error message includes config file path" { + local conf="$TEST_HOME/.wt/repos/test.conf" + sed -i.bak 's|^WT_WORKTREES_BASE=.*|WT_WORKTREES_BASE="relative/worktrees"|' "$conf" + _unset_wt_vars + + run "$TEST_HOME/.wt/bin/wt-list" + assert_failure + assert_output --partial "test.conf" +} + +@test "valid absolute paths pass validation normally" { + _unset_wt_vars + + run "$TEST_HOME/.wt/bin/wt-list" + assert_success +} diff --git a/test/unit/wt-common.bats b/test/unit/wt-common.bats index 5420c59..5556da6 100644 --- a/test/unit/wt-common.bats +++ b/test/unit/wt-common.bats @@ -749,3 +749,122 @@ teardown() { assert_equal "$WT_BASE_BRANCH" "pre-existing" } +# ============================================================================= +# Tests for _wt_is_valid_path_config() +# ============================================================================= + +@test "_wt_is_valid_path_config accepts absolute path" { + run _wt_is_valid_path_config "/home/user/worktrees" + assert_success +} + +@test "_wt_is_valid_path_config accepts absolute path with spaces" { + run _wt_is_valid_path_config "/tmp/my worktrees" + assert_success +} + +@test "_wt_is_valid_path_config rejects relative path" { + run _wt_is_valid_path_config "worktrees/foo" + assert_failure +} + +@test "_wt_is_valid_path_config rejects dot-relative path" { + run _wt_is_valid_path_config "./worktrees" + assert_failure +} + +@test "_wt_is_valid_path_config rejects parent-relative path" { + run _wt_is_valid_path_config "../worktrees" + assert_failure +} + +@test "_wt_is_valid_path_config rejects glob with asterisk" { + run _wt_is_valid_path_config "/tmp/wt-*" + assert_failure +} + +@test "_wt_is_valid_path_config rejects glob with question mark" { + run _wt_is_valid_path_config "/tmp/wt-?" + assert_failure +} + +@test "_wt_is_valid_path_config rejects glob with bracket" { + run _wt_is_valid_path_config "/tmp/wt-[0-9]" + assert_failure +} + +@test "_wt_is_valid_path_config rejects empty string" { + run _wt_is_valid_path_config "" + assert_failure +} + +# ============================================================================= +# Tests for wt_require_valid_config() +# ============================================================================= + +@test "wt_require_valid_config passes when all paths are absolute" { + export WT_MAIN_REPO_ROOT="/home/user/repo" + export WT_WORKTREES_BASE="/home/user/worktrees" + export WT_IDEA_FILES_BASE="/home/user/idea-files" + + run --separate-stderr wt_require_valid_config + assert_success + assert_equal "$stderr" "" +} + +@test "wt_require_valid_config fails for relative WT_MAIN_REPO_ROOT" { + export WT_MAIN_REPO_ROOT="relative/repo" + export WT_WORKTREES_BASE="/home/user/worktrees" + export WT_IDEA_FILES_BASE="/home/user/idea-files" + + run --separate-stderr wt_require_valid_config + assert_failure + [[ "$stderr" == *"WT_MAIN_REPO_ROOT"* ]] + [[ "$stderr" == *"relative/repo"* ]] +} + +@test "wt_require_valid_config fails for glob in WT_WORKTREES_BASE" { + export WT_MAIN_REPO_ROOT="/home/user/repo" + export WT_WORKTREES_BASE="/home/user/wt-*" + export WT_IDEA_FILES_BASE="/home/user/idea-files" + + run --separate-stderr wt_require_valid_config + assert_failure + [[ "$stderr" == *"WT_WORKTREES_BASE"* ]] +} + +@test "wt_require_valid_config reports all invalid vars" { + export WT_MAIN_REPO_ROOT="relative/repo" + export WT_WORKTREES_BASE="../worktrees" + export WT_IDEA_FILES_BASE="/valid/path" + + run --separate-stderr wt_require_valid_config + assert_failure + [[ "$stderr" == *"WT_MAIN_REPO_ROOT"* ]] + [[ "$stderr" == *"WT_WORKTREES_BASE"* ]] +} + +@test "wt_require_valid_config shows config file path when context is set" { + export WT_MAIN_REPO_ROOT="relative/repo" + export WT_WORKTREES_BASE="/valid/path" + export WT_IDEA_FILES_BASE="/valid/path" + export WT_CONTEXT_NAME="mycontext" + + # Create the config file so the message references it + mkdir -p "$HOME/.wt/repos" + echo 'WT_MAIN_REPO_ROOT="relative/repo"' > "$HOME/.wt/repos/mycontext.conf" + + run --separate-stderr wt_require_valid_config + assert_failure + [[ "$stderr" == *"mycontext.conf"* ]] +} + +@test "wt_require_valid_config skips empty variables" { + unset WT_MAIN_REPO_ROOT + unset WT_WORKTREES_BASE + unset WT_IDEA_FILES_BASE + + run --separate-stderr wt_require_valid_config + assert_success +} +