From 91c3a697698f5a2cb6ea750677a3a7dc8fd99aa4 Mon Sep 17 00:00:00 2001 From: Shi Qi Date: Wed, 4 Nov 2026 15:55:41 -0800 Subject: [PATCH] fix: preserve newlines in config symlink targets --- lib/config_safety.sh | 4 ++- tests/config_safety.bats | 56 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/lib/config_safety.sh b/lib/config_safety.sh index d6625ec..2dd9d57 100644 --- a/lib/config_safety.sh +++ b/lib/config_safety.sh @@ -90,7 +90,9 @@ _config_safety_path() { if [ -L "$candidate" ]; then links=$((links + 1)) [ "$links" -le 40 ] || { _config_safety_error "$candidate" 'symlink loop'; return 1; } - target=$(readlink "$candidate") || return 1 + # Suppress readlink's delimiter and keep target newlines with a sentinel. + target=$(readlink -n "$candidate" && printf '.') || return 1 + target="${target%.}" case "$target" in /*) current=/ ;; esac pending="$target${pending:+/$pending}" else diff --git a/tests/config_safety.bats b/tests/config_safety.bats index 1c0aff5..5bd81e9 100644 --- a/tests/config_safety.bats +++ b/tests/config_safety.bats @@ -113,6 +113,62 @@ assert_unsafe_shell_blocked() { assert_unsafe_create_blocked } +@test "unsafe absolute symlink target with trailing newlines is rejected" { + local target="$SAFETY_FIXTURE/included"$'\n\n' + git config -f "$target" gtr.hook.postCreate 'printf linked > "$SAFETY_MARKER"' + chmod 0660 "$target" + ln -s "$target" "$TEST_REPO/.git/linked-config" + git config include.path linked-config + + assert_unsafe_create_blocked + [[ "$output" == *"$target"* ]] +} + +@test "unsafe relative symlink target with a trailing newline is rejected" { + local target="$TEST_REPO/.git/included"$'\n' + git config -f "$target" gtr.hook.postCreate 'printf linked > "$SAFETY_MARKER"' + chmod 0660 "$target" + ln -s $'included\n' "$TEST_REPO/.git/linked-config" + git config include.path linked-config + + assert_unsafe_create_blocked + [[ "$output" == *"$target"* ]] +} + +@test "safe symlink target with trailing newlines is used instead of its writable truncated path" { + local target="$SAFETY_FIXTURE/included"$'\n\n' + git config -f "$target" gtr.hook.postCreate 'printf linked > "$SAFETY_MARKER"' + chmod 0600 "$target" + : > "$SAFETY_FIXTURE/included" + chmod 0660 "$SAFETY_FIXTURE/included" + ln -s "$target" "$TEST_REPO/.git/linked-config" + git config include.path linked-config + + run "$PROJECT_ROOT/bin/gtr" new safe-newline --from HEAD --no-fetch --no-copy + [ "$status" -eq 0 ] + [ -d "$SAFETY_FIXTURE/attacker destination/safe-newline" ] + [ "$(cat "$SAFETY_MARKER")" = linked ] +} + +@test "readlink failure rejects config even when it prints a safe target" { + git config -f "$SAFETY_FIXTURE/protected" gtr.hook.postCreate 'printf linked > "$SAFETY_MARKER"' + ln -s "$SAFETY_FIXTURE/protected" "$TEST_REPO/.git/linked-config" + git config include.path linked-config + readlink() { + printf '%s\n' "$SAFETY_FIXTURE/protected" + return 1 + } + export SAFETY_FIXTURE + export -f readlink + + run "$PROJECT_ROOT/bin/gtr" new unsafe-config --from HEAD --no-fetch --no-copy + [ "$status" -ne 0 ] + [ ! -e "$SAFETY_MARKER" ] + [ ! -e "$SAFETY_FIXTURE/attacker destination" ] + run git show-ref --verify --quiet refs/heads/unsafe-config + [ "$status" -eq 1 ] +} + @test "symlink parent permissions are checked before following a safe target" { mkdir "$SAFETY_FIXTURE/shared" git config -f "$SAFETY_FIXTURE/protected" gtr.hook.postCreate 'printf linked > "$SAFETY_MARKER"'