From b10fbf1f61ff54c8b99c9b07317d995df42a4187 Mon Sep 17 00:00:00 2001 From: Sihao Liu Date: Thu, 16 Apr 2026 21:54:50 -0700 Subject: [PATCH 1/2] Reject symlink aliases for cancel source and destination Using realpath-with-leaf-dereference on the user-provided mv source and destination lets a symlink aliasing /state.md or /cancel-state.md pass authorization. For the destination case the vulnerability is exploitable: `mv /state.md /tmp/link` (where /tmp/link -> /cancel-state.md) canonicalizes to the expected path and passes the check, but `mv` replaces the link at /tmp/link rather than creating /cancel-state.md, corrupting loop state and depositing state.md outside the loop dir. Introduce canonicalize_path_prefix in project-root.sh that resolves symlinks ONLY in the parent directory and preserves the basename verbatim. Symlinked project prefixes (e.g. /var vs /private/var) still match a canonical expected path, but a symlink at the leaf no longer impersonates the real filename. Rewire is_cancel_authorized to use the prefix-only helper for both src and dest, and document why the distinction matters in-place. Update the on-disk src_original probe to reference canonical_loop_dir so the symlink-rejection check runs against the real path rather than any user-supplied non-canonical form. Add two regression tests covering the destination and source symlink alias attacks. --- hooks/lib/loop-common.sh | 23 ++++++++++----- hooks/lib/project-root.sh | 48 ++++++++++++++++++++++++++++++++ tests/test-cancel-signal-file.sh | 45 ++++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+), 7 deletions(-) diff --git a/hooks/lib/loop-common.sh b/hooks/lib/loop-common.sh index ec4497bb..3e54e6ef 100755 --- a/hooks/lib/loop-common.sh +++ b/hooks/lib/loop-common.sh @@ -1190,15 +1190,21 @@ is_cancel_authorized() { # Normalize and validate source path. # - # Canonicalize the user-provided path so a symlinked prefix in the caller's - # command (e.g. /Users/x vs /private/Users/x on macOS, or /var vs - # /private/var) matches canonical_loop_dir resolved via resolve_project_root. + # Use canonicalize_path_prefix (NOT canonicalize_path): we need to resolve + # symlinks in the parent directory so a symlinked project prefix matches + # canonical_loop_dir, but we MUST NOT dereference a symlink at the leaf. + # Otherwise a symlink like /tmp/alias -> /state.md would canonicalize + # to /state.md and pass the check, but `mv` would then operate on + # the link path itself, escaping the loop directory and/or corrupting + # loop state. The on-disk symlink rejection below (src_original check) + # still fires because it probes the real state.md under canonical_loop_dir. + # # Re-lowercase after canonicalization because realpath on case-insensitive # filesystems may restore the original casing of path components, which # would diverge from the already-lowercased expected_* values. src=$(_normalize_path "$src") local src_canonical - src_canonical="$(canonicalize_path "$src")" + src_canonical="$(canonicalize_path_prefix "$src")" src_canonical="${src_canonical:-$src}" src_canonical=$(echo "$src_canonical" | tr '[:upper:]' '[:lower:]') local expected_src_state="${loop_dir_lower}state.md" @@ -1208,11 +1214,14 @@ is_cancel_authorized() { return 5 fi - # Normalize and validate destination path (same canonicalize+lowercase - # transformation as source; see src comment above for rationale). + # Normalize and validate destination path. Uses canonicalize_path_prefix + # for the same reason as src: a symlink alias pointing at the real + # cancel-state.md must NOT pass authorization, because `mv` onto a + # symlink replaces the link rather than creating /cancel-state.md, + # corrupting loop state and moving state.md outside the loop dir. dest=$(_normalize_path "$dest") local dest_canonical - dest_canonical="$(canonicalize_path "$dest")" + dest_canonical="$(canonicalize_path_prefix "$dest")" dest_canonical="${dest_canonical:-$dest}" dest_canonical=$(echo "$dest_canonical" | tr '[:upper:]' '[:lower:]') local expected_dest="${loop_dir_lower}cancel-state.md" diff --git a/hooks/lib/project-root.sh b/hooks/lib/project-root.sh index 3887788b..cb23403a 100644 --- a/hooks/lib/project-root.sh +++ b/hooks/lib/project-root.sh @@ -52,6 +52,49 @@ resolve_project_root() { printf '%s\n' "${canonical:-$root}" } +# canonicalize_path_prefix +# +# Resolves symlinks ONLY in the parent directory and reattaches the +# original basename verbatim. This is the right helper for comparing +# user-supplied filenames against an expected path inside a known +# directory: a symlink at /tmp/alias pointing at /real/loop/state.md +# MUST NOT canonicalize to /real/loop/state.md for comparison purposes, +# because `mv` operates on the link path itself. Resolving only the +# parent still lets a symlinked project prefix (e.g. /var vs /private/var +# on macOS) match a canonical expected path. +# +# If realpath on the parent fails, falls back to returning the input +# path unchanged (prefix cannot be canonicalized -> caller's comparison +# will correctly fail against a canonical expected path). +# +# Empty input prints nothing and returns 0. +# +canonicalize_path_prefix() { + local path="$1" + if [[ -z "$path" ]]; then + return 0 + fi + + local parent base parent_real + parent=$(dirname -- "$path") + base=$(basename -- "$path") + + if parent_real=$(realpath "$parent" 2>/dev/null) && [[ -n "$parent_real" ]]; then + printf '%s/%s\n' "${parent_real%/}" "$base" + return 0 + fi + + if command -v python3 >/dev/null 2>&1; then + parent_real=$(python3 -c 'import os,sys;print(os.path.realpath(sys.argv[1]))' "$parent" 2>/dev/null || true) + if [[ -n "$parent_real" ]]; then + printf '%s/%s\n' "${parent_real%/}" "$base" + return 0 + fi + fi + + printf '%s\n' "$path" +} + # canonicalize_path # # Prints the realpath of the input path. If the path itself does not @@ -60,6 +103,11 @@ resolve_project_root() { # If realpath is unavailable and python3 is missing, prints the input # path verbatim. # +# SECURITY NOTE: This helper dereferences symlinks at the leaf when +# the leaf exists. Do NOT use it to authorize a user-supplied path +# against an expected filename -- use canonicalize_path_prefix instead, +# which only resolves the parent. +# # Empty input prints nothing and returns 0. # canonicalize_path() { diff --git a/tests/test-cancel-signal-file.sh b/tests/test-cancel-signal-file.sh index 7d0ca8b9..420c5d5d 100755 --- a/tests/test-cancel-signal-file.sh +++ b/tests/test-cancel-signal-file.sh @@ -1373,6 +1373,51 @@ fi rm -rf "$SYMLINK_ROOT" 2>/dev/null || true +echo "HELPER TEST 9: is_cancel_authorized rejects destination symlink alias" +# Regression test for a P1 security issue: if the destination argument is a +# symlink that points at /cancel-state.md, canonicalizing the full +# path (leaf dereferenced) would let the alias pass authorization. `mv` +# would then operate on the link path itself, corrupting loop state and +# leaking state.md contents outside the loop dir. The fix resolves symlinks +# only in the parent directory and preserves the basename verbatim. +setup_test_loop "helper-9" +touch "$LOOP_DIR/.cancel-requested" +# Create the target file so the symlink would resolve if the prefix-only +# canonicalizer were relaxed back to full canonicalization. +touch "$LOOP_DIR/cancel-state.md" +ln -sfn "$LOOP_DIR/cancel-state.md" "$TEST_DIR/dest-alias" + +COMMAND_LOWER="mv ${LOOP_DIR}/state.md ${TEST_DIR}/dest-alias" +COMMAND_LOWER=$(to_lower "$COMMAND_LOWER") + +if is_cancel_authorized "$LOOP_DIR" "$COMMAND_LOWER"; then + fail "helper dest symlink alias" "returns non-zero (rejected)" "returns 0 (authorized)" +else + pass "is_cancel_authorized rejects destination symlink alias" +fi +rm -f "$TEST_DIR/dest-alias" "$LOOP_DIR/cancel-state.md" + +echo "HELPER TEST 10: is_cancel_authorized rejects source symlink alias" +# Regression test for a P1 security issue: if the source argument is a +# symlink aliasing /state.md, dereferencing the leaf would let it +# pass authorization. The on-disk symlink check (src_original) below +# would still catch this specific case because it probes the real path, +# but we defend in depth: the path comparison must reject the alias on +# its own. +setup_test_loop "helper-10" +touch "$LOOP_DIR/.cancel-requested" +ln -sfn "$LOOP_DIR/state.md" "$TEST_DIR/src-alias" + +COMMAND_LOWER="mv ${TEST_DIR}/src-alias ${LOOP_DIR}/cancel-state.md" +COMMAND_LOWER=$(to_lower "$COMMAND_LOWER") + +if is_cancel_authorized "$LOOP_DIR" "$COMMAND_LOWER"; then + fail "helper src symlink alias" "returns non-zero (rejected)" "returns 0 (authorized)" +else + pass "is_cancel_authorized rejects source symlink alias" +fi +rm -f "$TEST_DIR/src-alias" + # ======================================== # Summary # ======================================== From cf17140050c4e063f27924c2d56cc2279d81f4cd Mon Sep 17 00:00:00 2001 From: Sihao Liu Date: Thu, 16 Apr 2026 22:01:08 -0700 Subject: [PATCH 2/2] Use prefix-only canonicalization in read/write path validators The main FILE_PATH vs CORRECT_PATH comparison in loop-read-validator.sh and loop-write-validator.sh previously used canonicalize_path, which dereferences symlinks at the leaf. A planted symlink at the correct filename inside the loop dir would then canonicalize to its target and let the validator approve a Read or Write that follows the link out of the loop dir, expanding Claude's effective file-access reach beyond what the hook intends to permit. Switch both validators to canonicalize_path_prefix so a symlinked project ancestor still resolves correctly (the original bug this feature fixes) while a symlink at the leaf no longer impersonates the expected filename. This matches the same discipline applied to is_cancel_authorized and preserves the intended semantics that the basename is compared verbatim. Methodology-analysis checks in these validators intentionally keep full-path realpath because they use prefix-containment (path starts with loop-dir/) rather than equality, which correctly catches symlinks escaping the loop dir. --- hooks/loop-read-validator.sh | 10 ++++++---- hooks/loop-write-validator.sh | 16 +++++++++------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/hooks/loop-read-validator.sh b/hooks/loop-read-validator.sh index 5a27fc4f..b812288a 100755 --- a/hooks/loop-read-validator.sh +++ b/hooks/loop-read-validator.sh @@ -304,10 +304,12 @@ fi CORRECT_PATH="$ACTIVE_LOOP_DIR/$CLAUDE_FILENAME" -# Compare canonical (symlink-resolved) forms -- see loop-write-validator.sh -# for the rationale; the same reasoning applies to read paths. -_READ_FILE_REAL=$(canonicalize_path "$FILE_PATH") -_READ_CORRECT_REAL=$(canonicalize_path "$CORRECT_PATH") +# Compare prefix-canonical forms -- see loop-write-validator.sh for the +# rationale; the same reasoning applies to read paths. A planted symlink +# at the leaf would otherwise let a Read follow the link outside the loop +# dir and still pass this validator. +_READ_FILE_REAL=$(canonicalize_path_prefix "$FILE_PATH") +_READ_CORRECT_REAL=$(canonicalize_path_prefix "$CORRECT_PATH") if [[ "${_READ_FILE_REAL:-$FILE_PATH}" != "${_READ_CORRECT_REAL:-$CORRECT_PATH}" ]]; then FALLBACK="# Wrong Directory Path diff --git a/hooks/loop-write-validator.sh b/hooks/loop-write-validator.sh index 9f46c102..eac0744c 100755 --- a/hooks/loop-write-validator.sh +++ b/hooks/loop-write-validator.sh @@ -331,13 +331,15 @@ fi CORRECT_PATH="$ACTIVE_LOOP_DIR/$CLAUDE_FILENAME" -# Compare canonical (symlink-resolved) forms so the check is not fooled by -# equivalent paths expressed in different prefix forms (e.g. /var/... vs -# /private/var/... on macOS). A raw string compare would mis-handle a -# symlinked project prefix whenever one side was canonicalized upstream -# (e.g. by resolve_project_root) and the other was not. -_WRITE_FILE_REAL=$(canonicalize_path "$FILE_PATH") -_WRITE_CORRECT_REAL=$(canonicalize_path "$CORRECT_PATH") +# Compare prefix-canonical forms so the check is not fooled by equivalent +# paths expressed in different ancestor forms (e.g. /var/... vs /private/var/... +# on macOS) -- without dereferencing the leaf. Using full realpath here +# would let a planted symlink at / pointing outside +# the loop dir approve a write through the link, escalating Claude's write +# reach beyond the loop dir. canonicalize_path_prefix resolves the parent +# directory only; the basename is compared verbatim. +_WRITE_FILE_REAL=$(canonicalize_path_prefix "$FILE_PATH") +_WRITE_CORRECT_REAL=$(canonicalize_path_prefix "$CORRECT_PATH") if [[ "${_WRITE_FILE_REAL:-$FILE_PATH}" != "${_WRITE_CORRECT_REAL:-$CORRECT_PATH}" ]]; then FALLBACK="# Wrong Directory Path