fix: reject unsafe macOS ACLs on hook trust storage

lib/commands/init.sh

@@ -145,8 +145,8 @@ cmd_init() {
   case "$cache_dir" in /*) ;; *) cache_dir="$(pwd -P)/$cache_dir" ;; esac
   local cache_file="$cache_dir/init-${func_name}.${shell}"
   # Bump when generated helpers change, even if GTR_VERSION is unchanged.
-  # Schema 13 preserves whole postCd records alongside picker path identities.
-  local cache_schema="${GTR_INIT_CACHE_VERSION:-13}"
+  # Schema 14 rejects write-capable macOS ACLs on trust storage.
+  local cache_schema="${GTR_INIT_CACHE_VERSION:-14}"
   local cache_stamp="# gtr-cache: version=${GTR_VERSION:-unknown} init=${cache_schema} func=$func_name shell=$shell"
 
   # A matching public stamp alone does not make executable cache data trusted.
@@ -171,7 +171,7 @@ cmd_init() {
 # functions keeps cached integration independent of the installation pathname.
 _init_emit_config_safety_helper() {
   local shell_name="$1" validator
-  validator=$(typeset -f _config_safety_error _config_safety_node _config_safety_path _gtr_config_check)
+  validator=$(typeset -f _config_safety_error _config_safety_acl_check _config_safety_node _config_safety_path _gtr_config_check)
   validator="$validator"$'\n_gtr_config_check'
   if [ "$shell_name" = fish ]; then
     validator=$(printf '%s' "$validator" | sed -e 's/\\/\\\\/g' -e "s/'/\\\\'/g")
@@ -225,6 +225,7 @@ SNAPSHOT
 # Keep this template identical to the storage helpers in lib/hooks.sh; tests
 # compare them so the CLI and generated shells enforce the same trust boundary.
 _init_emit_trust_storage_helpers() {
+  typeset -f _config_safety_acl_check
   cat <<'STORAGE'
 # Read ownership and permission bits on both GNU and BSD systems.
 _hooks_storage_stat() {
@@ -246,8 +247,10 @@ _hooks_storage_stat() {
 # Prints the physical directory; new components are private even with umask 000.
 _hooks_trust_directory() {
   local remaining="$1" action="${2:-}" current="/" component candidate metadata owner mode
-  local link_target links=0 current_uid
+  local link_target links=0 current_uid current_os
   current_uid=$(id -u) || return 1
+  current_os=$(uname -s) || return 1
+  _config_safety_acl_check / "$current_os" >/dev/null || return 1
   case "$remaining" in
     /*) ;;
     *) remaining="$(pwd -P)/$remaining" ;;
@@ -284,21 +287,23 @@ _hooks_trust_directory() {
     if [ "$((8#$mode & 8#22))" -ne 0 ]; then
       [ "$((8#$mode & 8#1000))" -ne 0 ] || return 1
     fi
+    _config_safety_acl_check "$candidate" "$current_os" >/dev/null || return 1
     current="$candidate"
   done
   # Even a sticky or root-owned trust store cannot hold this user's approvals.
   metadata=$(_hooks_storage_stat "$current") || return 1
   [ "${metadata%%:*}" = "$current_uid" ] || return 1
   mode="${metadata#*:}"
   [ "$((8#$mode & 8#22))" -eq 0 ] || return 1
+  _config_safety_acl_check "$current" "$current_os" >/dev/null || return 1
   printf '%s\n' "$current"
 }
 
 # Only accept regular, user-owned, non-writable-by-others marker files. Resolve
 # the validated directory once so subsequent operations use its physical path.
 # In create mode a missing marker is allowed, but an unsafe existing one is not.
 _hooks_safe_trust_path() {
-  local trust_path="$1" action="${2:-}" directory name metadata mode current_uid
+  local trust_path="$1" action="${2:-}" directory name metadata mode current_uid current_os
   name="${trust_path##*/}"
   case "$name" in ''|*[!0-9a-f]*) return 1 ;; esac
   [ "${#name}" -eq 64 ] || return 1
@@ -312,6 +317,8 @@ _hooks_safe_trust_path() {
     [ "${metadata%%:*}" = "$current_uid" ] || return 1
     mode="${metadata#*:}"
     [ "$((8#$mode & 8#22))" -eq 0 ] || return 1
+    current_os=$(uname -s) || return 1
+    _config_safety_acl_check "$trust_path" "$current_os" >/dev/null || return 1
   else
     [ "$action" = create ] || return 1
   fi
@@ -329,7 +336,8 @@ _init_emit_posix_trust_helpers() {
 
   # Keep the snapshot encoding and extraction identical to the CLI.
   _init_emit_snapshot_helpers
-  _init_emit_trust_storage_helpers | sed 's/_hooks_/__FUNC___hooks_/g'
+  _init_emit_trust_storage_helpers |
+    sed -e 's/_hooks_/__FUNC___hooks_/g' -e 's/_config_safety_acl_check/__FUNC___check_storage_acl/g'
 
   # Keep case patterns balanced inside this substitution for Bash 3.2's parser.
   template=$(cat <<'EOF'

lib/config_safety.sh

@@ -8,6 +8,24 @@ _config_safety_error() {
   return 1
 }
 
+# POSIX ACL write access is bounded by the group-class mode bits. macOS ACLs
+# are independent of those bits; conservatively reject write-capable grants.
+# Shared by configuration and trust storage checks. Call only for non-symlinks.
+# Usage: _config_safety_acl_check <path> <os>; prints a reason only on failure.
+_config_safety_acl_check() {
+  local node="$1" os="$2" acl
+  [ "$os" = Darwin ] || return 0
+  acl=$(LC_ALL=C ls -lde "$node" 2>/dev/null || exit 1) || {
+    printf '%s\n' 'cannot inspect ACL'
+    return 1
+  }
+  if printf '%s\n' "$acl" | sed '1d' | grep -Eq '^[[:space:]]*[0-9]+:.* allow (.*[, ])?(write|append|delete|add_file|add_subdirectory|delete_child|writeattr|writeextattr|writesecurity|chown)(,|$| )'; then
+    printf '%s\n' 'write-capable ACL'
+    return 1
+  fi
+  return 0
+}
+
 # Check a single path component without following symlinks. The caller walks
 # both the link's parent chain and its target. Dynamic locals belong to
 # _gtr_config_check; never accept a caller-supplied cached validation result.
@@ -46,15 +64,8 @@ _config_safety_node() {
       _config_safety_error "$path" 'writable by group or other users'; return 1
     fi
   fi
-  # POSIX ACL write access is bounded by the group-class mode bits. macOS ACLs
-  # are independent of those bits; conservatively reject write-capable grants.
-  if [ "$_config_safety_os" = Darwin ]; then
-    acl=$(LC_ALL=C ls -lde "$path" 2>/dev/null) ||
-      { _config_safety_error "$path" 'cannot inspect ACL'; return 1; }
-    if printf '%s\n' "$acl" | sed '1d' | grep -Eq '^[[:space:]]*[0-9]+:.* allow (.*[, ])?(write|append|delete|add_file|add_subdirectory|delete_child|writeattr|writeextattr|writesecurity|chown)(,|$| )'; then
-      _config_safety_error "$path" 'write-capable ACL'; return 1
-    fi
-  fi
+  acl=$(_config_safety_acl_check "$path" "$_config_safety_os") ||
+    { _config_safety_error "$path" "$acl"; return 1; }
   if [ "$_config_safety_sticky" -eq 0 ]; then
     _config_safety_checked+=("$path")
   fi

lib/hooks.sh

@@ -39,8 +39,10 @@ _hooks_storage_stat() {
 # Prints the physical directory; new components are private even with umask 000.
 _hooks_trust_directory() {
   local remaining="$1" action="${2:-}" current="/" component candidate metadata owner mode
-  local link_target links=0 current_uid
+  local link_target links=0 current_uid current_os
   current_uid=$(id -u) || return 1
+  current_os=$(uname -s) || return 1
+  _config_safety_acl_check / "$current_os" >/dev/null || return 1
   case "$remaining" in
     /*) ;;
     *) remaining="$(pwd -P)/$remaining" ;;
@@ -77,21 +79,23 @@ _hooks_trust_directory() {
     if [ "$((8#$mode & 8#22))" -ne 0 ]; then
       [ "$((8#$mode & 8#1000))" -ne 0 ] || return 1
     fi
+    _config_safety_acl_check "$candidate" "$current_os" >/dev/null || return 1
     current="$candidate"
   done
   # Even a sticky or root-owned trust store cannot hold this user's approvals.
   metadata=$(_hooks_storage_stat "$current") || return 1
   [ "${metadata%%:*}" = "$current_uid" ] || return 1
   mode="${metadata#*:}"
   [ "$((8#$mode & 8#22))" -eq 0 ] || return 1
+  _config_safety_acl_check "$current" "$current_os" >/dev/null || return 1
   printf '%s\n' "$current"
 }
 
 # Only accept regular, user-owned, non-writable-by-others marker files. Resolve
 # the validated directory once so subsequent operations use its physical path.
 # In create mode a missing marker is allowed, but an unsafe existing one is not.
 _hooks_safe_trust_path() {
-  local trust_path="$1" action="${2:-}" directory name metadata mode current_uid
+  local trust_path="$1" action="${2:-}" directory name metadata mode current_uid current_os
   name="${trust_path##*/}"
   case "$name" in ''|*[!0-9a-f]*) return 1 ;; esac
   [ "${#name}" -eq 64 ] || return 1
@@ -105,6 +109,8 @@ _hooks_safe_trust_path() {
     [ "${metadata%%:*}" = "$current_uid" ] || return 1
     mode="${metadata#*:}"
     [ "$((8#$mode & 8#22))" -eq 0 ] || return 1
+    current_os=$(uname -s) || return 1
+    _config_safety_acl_check "$trust_path" "$current_os" >/dev/null || return 1
   else
     [ "$action" = create ] || return 1
   fi

tests/init.bats

@@ -1148,14 +1148,14 @@ run_generated_v2_trust_upgrade() (
   mkdir -p "$XDG_CACHE_HOME/gtr"
   for shell_name in bash zsh fish; do
     for func_name in gtr gwtr; do
-      for schema in 12 11 10 9 8 7 6; do
+      for schema in 13 12 11 10 9 8 7 6; do
         cache_file="$XDG_CACHE_HOME/gtr/init-$func_name.$shell_name"
         printf '# gtr-cache: version=test init=%s func=%s shell=%s\nold-cache\n' \
           "$schema" "$func_name" "$shell_name" > "$cache_file"
         run cmd_init "$shell_name" --as "$func_name"
         [ "$status" -eq 0 ]
         [[ "$output" != *old-cache* ]]
-        [[ "$(head -1 "$cache_file")" == *"init=13"* ]]
+        [[ "$(head -1 "$cache_file")" == *"init=14"* ]]
       done
     done
   done
@@ -1184,15 +1184,15 @@ run_documented_init_upgrade() (
     if [ "$shell_name" = zsh ]; then
       snippet='compdef() { :; }'$'\n'"$snippet"
     fi
-    for schema in 6 7 8 9 10 11 12 13; do
+    for schema in 6 7 8 9 10 11 12 13 14; do
       version="$cli_version"
-      [ "$schema" != 13 ] || version=old-version
+      [ "$schema" != 14 ] || version=old-version
       printf '# gtr-cache: version=%s init=%s func=gtr shell=%s\n' \
         "$version" "$schema" "$shell_name" > "$cache_file"
       printf '%s\n' 'echo stale > "$STALE_CACHE_MARKER"' >> "$cache_file"
       "$shell_name" -c "$snippet"
       [ ! -f "$STALE_CACHE_MARKER" ]
-      [[ "$(head -1 "$cache_file")" == *"version=$cli_version init=13 "* ]]
+      [[ "$(head -1 "$cache_file")" == *"version=$cli_version init=14 "* ]]
     done
   done
 )

tests/trust_storage.bats

@@ -204,9 +204,9 @@ SH
 
 @test "generated storage helpers stay identical to CLI helpers" {
   local expected actual
-  expected=$(declare -f _hooks_storage_stat _hooks_trust_directory _hooks_safe_trust_path)
+  expected=$(declare -f _config_safety_acl_check _hooks_storage_stat _hooks_trust_directory _hooks_safe_trust_path)
   eval "$(_init_emit_trust_storage_helpers)"
-  actual=$(declare -f _hooks_storage_stat _hooks_trust_directory _hooks_safe_trust_path)
+  actual=$(declare -f _config_safety_acl_check _hooks_storage_stat _hooks_trust_directory _hooks_safe_trust_path)
   [ "$expected" = "$actual" ]
 }
 
@@ -285,3 +285,175 @@ exercise_generated_storage() (
   command -v fish >/dev/null || skip 'fish is not installed'
   exercise_generated_storage fish
 }
+
+@test "macOS trust storage rejects write ACLs on directories and markers without changing them" {
+  [ "$(uname -s)" = Darwin ] || skip 'macOS ACL syntax'
+  _hooks_mark_trusted "$TEST_REPO/.gtrconfig"
+  local node permission before mode
+  for node in "$_GTR_TRUST_DIR" "$XDG_CONFIG_HOME/gtr" "$XDG_CONFIG_HOME" "$TRUST_MARKER"; do
+    mode=$(file_mode "$node")
+    for permission in write append delete writeattr writeextattr writesecurity chown; do
+      chmod +a "everyone allow $permission" "$node"
+      before=$(LC_ALL=C ls -lde "$node")
+      [ "$(file_mode "$node")" = "$mode" ]
+      assert_storage_rejected
+      [ "$(LC_ALL=C ls -lde "$node")" = "$before" ]
+      chmod -N "$node"
+    done
+  done
+  for permission in add_file add_subdirectory delete_child; do
+    chmod +a "everyone allow $permission" "$_GTR_TRUST_DIR"
+    assert_storage_rejected
+    chmod -N "$_GTR_TRUST_DIR"
+  done
+}
+
+@test "macOS trust storage refuses creation beneath an ACL-writable ancestor" {
+  [ "$(uname -s)" = Darwin ] || skip 'macOS ACL syntax'
+  mkdir "$XDG_CONFIG_HOME"
+  chmod +a 'everyone allow add_subdirectory' "$XDG_CONFIG_HOME"
+  assert_storage_rejected
+  [ ! -e "$XDG_CONFIG_HOME/gtr" ]
+}
+
+@test "macOS trust storage checks ACLs through symlink parents and targets" {
+  [ "$(uname -s)" = Darwin ] || skip 'macOS ACL syntax'
+  mkdir "$TEST_REPO/real-config" "$TEST_REPO/link-parent"
+  ln -s "$TEST_REPO/real-config" "$TEST_REPO/link-parent/link"
+  ln -s "$TEST_REPO/link-parent/link" "$XDG_CONFIG_HOME"
+  _hooks_mark_trusted "$TEST_REPO/.gtrconfig"
+  local node
+  for node in "$TEST_REPO/real-config" "$TEST_REPO/link-parent"; do
+    chmod +a 'everyone allow delete_child' "$node"
+    assert_storage_rejected
+    chmod -N "$node"
+  done
+  _hooks_are_trusted "$TEST_REPO/.gtrconfig"
+}
+
+@test "macOS trust storage permits read-only and deny ACLs" {
+  [ "$(uname -s)" = Darwin ] || skip 'macOS ACL syntax'
+  _hooks_mark_trusted "$TEST_REPO/.gtrconfig"
+  local node
+  for node in "$XDG_CONFIG_HOME" "$XDG_CONFIG_HOME/gtr" "$_GTR_TRUST_DIR" "$TRUST_MARKER"; do
+    chmod +a 'everyone allow read,readattr,readextattr,readsecurity' "$node"
+  done
+  _hooks_are_trusted "$TEST_REPO/.gtrconfig"
+  _hooks_mark_trusted "$TEST_REPO/.gtrconfig"
+  chmod +a 'everyone deny write,append,delete' "$TRUST_MARKER"
+  _hooks_are_trusted "$TEST_REPO/.gtrconfig"
+  chmod -N "$TRUST_MARKER"
+}
+
+# Exercise inspection failures without changing real permissions or requiring
+# another user. All paths except the selected fixture use the actual ls.
+mock_unreadable_acl() {
+  export ACL_UNREADABLE_PATH="$1" REAL_LS
+  REAL_LS=$(command -v ls)
+  mkdir -p "$TEST_REPO/mock-bin"
+  cat > "$TEST_REPO/mock-bin/ls" <<'SH'
+#!/usr/bin/env bash
+for arg in "$@"; do last="$arg"; done
+if [ "$last" = "$ACL_UNREADABLE_PATH" ]; then exit 1; fi
+exec "$REAL_LS" "$@"
+SH
+  chmod +x "$TEST_REPO/mock-bin/ls"
+  export PATH="$TEST_REPO/mock-bin:$PATH"
+}
+
+@test "macOS trust storage fails closed when ACL inspection fails" {
+  [ "$(uname -s)" = Darwin ] || skip 'macOS ACL syntax'
+  _hooks_mark_trusted "$TEST_REPO/.gtrconfig"
+  mock_unreadable_acl /
+  local node
+  for node in / "$XDG_CONFIG_HOME" "$_GTR_TRUST_DIR" "$TRUST_MARKER"; do
+    export ACL_UNREADABLE_PATH="$node"
+    assert_storage_rejected
+  done
+}
+
+# Reproduce the attack without another OS account: rename the existing,
+# user-owned approval to the key for unreviewed definitions. Its contents and
+# mode remain valid, so storage ACLs must block both reads and hook execution.
+replay_marker_for_changed_definitions() {
+  local original_marker="$TRUST_MARKER"
+  git config -f "$TEST_REPO/.gtrconfig" hooks.postCd 'echo unreviewed > "$REPO_ROOT/result"'
+  TRUST_MARKER=$(_hooks_current_trust_path "$TEST_REPO/.gtrconfig")
+  [ "$TRUST_MARKER" != "$original_marker" ]
+  chmod +a 'everyone allow search,add_file,delete_child' "$_GTR_TRUST_DIR"
+  mv "$original_marker" "$TRUST_MARKER"
+  [ "$(cat "$TRUST_MARKER")" = "$TEST_REPO/.gtrconfig" ]
+}
+
+@test "macOS ACL-renamed approvals cannot execute unreviewed CLI hooks" {
+  [ "$(uname -s)" = Darwin ] || skip 'macOS ACL syntax'
+  _hooks_mark_trusted "$TEST_REPO/.gtrconfig"
+  replay_marker_for_changed_definitions
+  run run_hooks_export postCd "REPO_ROOT=$TEST_REPO"
+  [ "$status" -eq 0 ]
+  [[ "$output" == *'Untrusted .gtrconfig hooks'* ]]
+  [ ! -e "$TEST_REPO/result" ]
+}
+
+# Generate while safe, then change ACLs so cached helpers must inspect live
+# storage. Exercise the validator directly as well as the public hook path.
+exercise_generated_storage_acls() {
+  local shell_name="$1" node
+  [ "$(uname -s)" = Darwin ] || skip 'macOS ACL syntax'
+  command -v "$shell_name" >/dev/null || skip "$shell_name is not installed"
+  export INIT_FIXTURE="$TEST_REPO/init.$shell_name" REPO_FIXTURE="$TEST_REPO" TRUST_MARKER
+  cmd_init "$shell_name" --as safe_gtr > "$INIT_FIXTURE"
+  _hooks_mark_trusted "$TEST_REPO/.gtrconfig"
+  for node in "$_GTR_TRUST_DIR" "$XDG_CONFIG_HOME/gtr" "$XDG_CONFIG_HOME" "$TRUST_MARKER"; do
+    chmod +a 'everyone allow write' "$node"
+    run "$shell_name" -c 'source "$INIT_FIXTURE"; safe_gtr_hooks_safe_trust_path "$TRUST_MARKER"'
+    [ "$status" -eq 1 ]
+    run "$shell_name" -c 'source "$INIT_FIXTURE"; safe_gtr_run_post_cd_hooks "$REPO_FIXTURE"'
+    [ ! -e "$TEST_REPO/result" ]
+    chmod -N "$node"
+    chmod +a 'everyone allow read,readattr,readsecurity' "$node"
+  done
+  "$shell_name" -c 'source "$INIT_FIXTURE"; safe_gtr_run_post_cd_hooks "$REPO_FIXTURE"'
+  [ "$(cat "$TEST_REPO/result")" = approved ]
+  rm "$TEST_REPO/result"
+  chmod +a 'everyone deny write,append,delete' "$TRUST_MARKER"
+  "$shell_name" -c 'source "$INIT_FIXTURE"; safe_gtr_run_post_cd_hooks "$REPO_FIXTURE"'
+  [ "$(cat "$TEST_REPO/result")" = approved ]
+  rm "$TEST_REPO/result"
+  chmod -N "$TRUST_MARKER"
+  mock_unreadable_acl "$TRUST_MARKER"
+  run "$shell_name" -c 'source "$INIT_FIXTURE"; safe_gtr_hooks_safe_trust_path "$TRUST_MARKER"'
+  [ "$status" -eq 1 ]
+  export ACL_UNREADABLE_PATH=''
+  # Both the link's intermediate parent and its physical target need ACL checks.
+  mv "$XDG_CONFIG_HOME" "$TEST_REPO/real-config"
+  mkdir "$TEST_REPO/link-parent"
+  ln -s "$TEST_REPO/real-config" "$TEST_REPO/link-parent/link"
+  ln -s "$TEST_REPO/link-parent/link" "$XDG_CONFIG_HOME"
+  for node in "$TEST_REPO/real-config" "$TEST_REPO/link-parent"; do
+    chmod +a 'everyone allow delete_child' "$node"
+    run "$shell_name" -c 'source "$INIT_FIXTURE"; safe_gtr_hooks_safe_trust_path "$TRUST_MARKER"'
+    [ "$status" -eq 1 ]
+    chmod -N "$node"
+  done
+  "$shell_name" -c 'source "$INIT_FIXTURE"; safe_gtr_run_post_cd_hooks "$REPO_FIXTURE"'
+  [ "$(cat "$TEST_REPO/result")" = approved ]
+  rm "$TEST_REPO/result"
+  replay_marker_for_changed_definitions
+  run "$shell_name" -c 'source "$INIT_FIXTURE"; safe_gtr_run_post_cd_hooks "$REPO_FIXTURE"'
+  [ "$status" -eq 0 ]
+  [[ "$output" == *'Untrusted .gtrconfig hooks'* ]]
+  [ ! -e "$TEST_REPO/result" ]
+}
+
+@test "generated Bash helpers reject macOS ACLs and renamed approvals while permitting safe ACLs" {
+  exercise_generated_storage_acls bash
+}
+
+@test "generated Zsh helpers reject macOS ACLs and renamed approvals while permitting safe ACLs" {
+  exercise_generated_storage_acls zsh
+}
+
+@test "generated Fish helpers reject macOS ACLs and renamed approvals while permitting safe ACLs" {
+  exercise_generated_storage_acls fish
+}