fix sudoers setup to block privilege escalation

bin/.gitignore

@@ -1,5 +1,8 @@
 *-bin
 *-dist
+# y-bin-dependency-download's temporaries, left only by a killed run
+.download-*
+.link-*
 
 # our executables are symlinked to real names
 buildctl

bin/y-bin-dependency-download

@@ -43,32 +43,39 @@ bin_link=$YBIN/$bin_name
   [ -z "$YSTACK_BIN_DOWNLOAD_CACHE" ] || bin_cache="$YSTACK_BIN_DOWNLOAD_CACHE/$bin_uniq"
   [ -z "$bin_cache" ] || [ ! -f "$bin_cache" ] || url="file://$bin_cache"
   echo "Missing $bin_name binary. Downloading a known version from $url ..." 1>&2
-  case "$url" in
-    *.tar.gz) ;;
-    *.gz) bin_file=$bin_file.gz ;;
-  esac
-  curl -o $bin_file -L -f $url
-  if ! echo "$sha256  $bin_file" | $SHA256SUM -c - 1>&2; then $SHA256SUM $bin_file && rm $bin_file && exit 1; fi
+  # Download and unpack under a temporary name, renamed to $bin_file only when complete:
+  # an interrupted download (a caller's `| head` is enough) or one still running in another
+  # process must never be taken for the binary.
+  dl=$(mktemp "$YBIN/.download-$bin_uniq.XXXXXX")
+  trap 'rm -f "$dl" "$dl.gunzip"' EXIT
+  curl -o $dl -L -f $url
+  if ! echo "$sha256  $dl" | $SHA256SUM -c - 1>&2; then $SHA256SUM $dl && exit 1; fi
   if [ ! -z "$bin_tar_path" ]; then
     tmp=$(mktemp -d)
-    (cd $tmp; tar -xv${bin_tar_compression}f $bin_file --strip-components=$(echo $bin_tar_path | awk -F'/' '{print NF-1}') $bin_tar_extract) \
-      || (cd $tmp; tar -tv${bin_tar_compression}f $bin_file; exit 1)
-    mv "$tmp/$(basename $bin_tar_path)" $bin_file
+    (cd $tmp; tar -xv${bin_tar_compression}f $dl --strip-components=$(echo $bin_tar_path | awk -F'/' '{print NF-1}') $bin_tar_extract) \
+      || (cd $tmp; tar -tv${bin_tar_compression}f $dl; exit 1)
+    mv "$tmp/$(basename $bin_tar_path)" $dl
+    rm -rf "$tmp"
   fi
   if [ ! -z "$bin_zip_path" ]; then
     tmp=$(mktemp -d)
-    (cd $tmp; unzip $bin_file)
-    mv "$tmp/$bin_zip_path" $bin_file
+    (cd $tmp; unzip $dl)
+    mv "$tmp/$bin_zip_path" $dl
+    rm -rf "$tmp"
   fi
   case "$url" in
     *.tar.gz) ;;
-    *.gz) gunzip $bin_file && bin_file=$(echo $bin_file | sed 's/\.gz$//g') ;;
+    *.gz) gzip -dc $dl > $dl.gunzip && mv $dl.gunzip $dl ;;
   esac
+  chmod 755 $dl
+  mv -f $dl $bin_file
+  trap - EXIT
   [ -z "$bin_cache" ] || [ -f "$bin_cache" ] || cp -v $bin_file $bin_cache 1>&2
-  chmod a+x $bin_file
 }
 
 if [ "$(readlink $bin_link)" != "$bin_file" ]; then
-  [ -L $bin_link ] && rm $bin_link
-  ln -s $bin_file $bin_link
+  [ ! -e $bin_link ] || [ -L $bin_link ] || { echo "Not a symlink, not replacing: $bin_link" >&2; exit 1; }
+  # replaced by rename, so a concurrent run never sees the link missing
+  ln -s $bin_file $YBIN/.link-$bin_name.$$
+  mv -f $YBIN/.link-$bin_name.$$ $bin_link
 fi

bin/y-cluster-sudoers

@@ -2,62 +2,116 @@
 [ -z "$DEBUG" ] || set -x
 set -eo pipefail
 
-YSTACK_HOME="$(cd "$(dirname "$0")/.." && pwd)"
+YSTACK_HOME="$(cd "$(dirname "$0")/.." && pwd -P)"
 YBIN="$YSTACK_HOME/bin"
-
 SUDOERS_FILE="/etc/sudoers.d/ystack-cluster"
+# Root-owned snapshots of the helpers: a NOPASSWD rule for a file its grantee can edit
+# would be root for the grantee, so the rules name these copies only
+PRIVILEGED_DIR="/usr/local/libexec/ystack"
+HELPERS=(y-etc-hosts-write y-localhost)
 
 usage() {
   cat >&2 <<EOF
-Usage: y-cluster-sudoers [--write] [username ...]
+Usage: y-cluster-sudoers [--write] [--allow-grantee-source] [username ...]
 
 Maintains passwordless sudo rules for automatic development cluster setup.
 
-Without --write: prints the sudoers rules to stdout.
-With    --write: writes the rules to $SUDOERS_FILE (requires sudo).
+Without --write: prints the sudoers rules and what --write would install.
+With    --write: as root, installs root-owned copies of ${HELPERS[*]}
+                 from this checkout to $PRIVILEGED_DIR and writes the rules,
+                 which name only those copies, to $SUDOERS_FILE.
 
 Usernames: one or more users to grant NOPASSWD rules to.
            Defaults to the current user (via SUDO_USER or USER).
 
-If the script is in an encrypted homedir, pipe from a sudo -u invocation:
-  sudo -u USER $0 USERNAME | sudo tee $SUDOERS_FILE
+Run --write from a checkout you trust at a commit you have reviewed, as it copies
+the helpers that will run as root. It refuses when a grantee owns or can write the
+helpers, this script or any directory above them, or when one of those is a symlink,
+unless --allow-grantee-source is given. It cannot vouch for itself: if a grantee
+could write this script, running it as root in any mode runs their code. The show
+mode run as yourself lists what a grantee owns; group and ACL write access you check
+without executing the checkout, e.g. ls -ld on the path, before running it with sudo.
+Rerun it after updating ystack to refresh the copies; y-k8s-ingress-hosts warns
+when they differ.
 EOF
   exit 0
 }
 
 [ "$1" = "-h" ] || [ "$1" = "--help" ] && usage
 
 MODE=show
-case "$1" in
-  --write) MODE=write; shift ;;
-  --show)  shift ;;
-esac
+ALLOW_GRANTEE_SOURCE=false
+while [ $# -gt 0 ]; do
+  case "$1" in
+    --write) MODE="write"; shift ;;
+    --show) shift ;;
+    --allow-grantee-source) ALLOW_GRANTEE_SOURCE=true; shift ;;
+    -*) echo "ERROR: unknown option '$1'" >&2; exit 1 ;;
+    *) break ;;
+  esac
+done
 
 USERS=("$@")
 for u in "${USERS[@]}"; do
-  case "$u" in -*) echo "ERROR: invalid username '$u' (did you mean to put --write first?)" >&2; exit 1 ;; esac
+  case "$u" in -*) echo "ERROR: options go before usernames, got '$u'" >&2; exit 1 ;; esac
+  id -u "$u" >/dev/null 2>&1 || { echo "ERROR: no such user '$u'" >&2; exit 1; }
 done
 [ ${#USERS[@]} -eq 0 ] && USERS=("${SUDO_USER:-$USER}")
 
 rules() {
   echo "# ystack development cluster automation"
   echo "# Generated by y-cluster-sudoers for: ${USERS[*]}"
-  echo "# YSTACK_HOME=$YSTACK_HOME"
+  echo "# Helpers installed from $YSTACK_HOME to $PRIVILEGED_DIR, see $PRIVILEGED_DIR/SOURCE"
   for u in "${USERS[@]}"; do
     cat <<EOF
-
-# y-localhost: loopback alias management ($u)
-$u ALL=(root) NOPASSWD: $YBIN/y-localhost *
-
+# y-localhost: loopback alias management ($u), 127.0.0.0/8 only
+$u ALL=(root) NOPASSWD: $PRIVILEGED_DIR/y-localhost *
 # y-k8s-ingress-hosts: /etc/hosts management ($u)
-# Argument-free on purpose: the rule this replaced covered the
-# k8s-ingress-hosts binary with any flags, and its -host-file flag
-# turned that into a write-any-file-as-root grant.
-$u ALL=(root) NOPASSWD: $YBIN/y-etc-hosts-write
+# Argument-free on purpose, and the copy only accepts changes inside the
+# k8s-ingress-hosts block that map to loopback or private addresses.
+$u ALL=(root) NOPASSWD: $PRIVILEGED_DIR/y-etc-hosts-write
 EOF
   done
 }
 
+file_owner() {
+  case "$(uname -s)" in
+    Darwin) stat -f '%Su' "$1" ;;
+    *) stat -c '%U' "$1" ;;
+  esac
+}
+
+# The helpers, this script, and every directory above them
+source_paths() {
+  local p d
+  for p in "${HELPERS[@]/#/$YBIN/}" "$YBIN/y-cluster-sudoers"; do
+    d=$p
+    while :; do
+      echo "$d"
+      [ "$d" = / ] && break
+      d=$(dirname "$d")
+    done
+  done | sort -u
+}
+
+# A grantee who owns or can write any source path can change a helper between this check
+# and the copy, or swap the checkout itself; a symlink can point anywhere. Write access
+# (group, other, ACL) is checked only as root, which --write is.
+grantee_controlled_sources() {
+  local p owner u
+  while IFS= read -r p; do
+    if [ -L "$p" ]; then echo "$p is a symlink"; fi
+    owner=$(file_owner "$p")
+    for u in "${USERS[@]}"; do
+      if [ "$owner" = "$u" ]; then
+        echo "$p is owned by grantee $u"
+      elif [ "$(id -u)" -eq 0 ] && sudo -n -u "$u" test -w "$p"; then
+        echo "$p is writable by grantee $u"
+      fi
+    done
+  done < <(source_paths)
+}
+
 sudo_rs_check() {
   # sudo-rs (default on Ubuntu 25.10+) has a bug where NOPASSWD rules from
   # /etc/sudoers.d/ are listed by "sudo -l" but not applied during execution.
@@ -71,31 +125,51 @@ sudo_rs_check() {
   fi
 }
 
-if [ "$MODE" = "write" ]; then
-  if [ $(id -u) -ne 0 ]; then
-    if sudo -n true 2>/dev/null; then
-      echo "Writing sudoers rules requires root. Re-running with sudo ..."
-      exec sudo "$0" --write "${USERS[@]}"
-    else
-      echo "Can't sudo as $USER. A user with sudo rights can run for example:" >&2
-      echo "  sudo -u $USER $0 ${USERS[*]} | sudo tee $SUDOERS_FILE" >&2
-      exit 1
-    fi
-  fi
-  TMPFILE=$(mktemp)
-  rules > "$TMPFILE"
-  if visudo -cf "$TMPFILE"; then
-    cp "$TMPFILE" "$SUDOERS_FILE"
-    chmod 0440 "$SUDOERS_FILE"
-    rm "$TMPFILE"
-    echo "Wrote $SUDOERS_FILE"
-    sudo_rs_check
-  else
-    rm "$TMPFILE"
-    echo "ERROR: Generated sudoers rules failed validation" >&2
-    exit 1
-  fi
-else
+if [ "$MODE" = "show" ]; then
   rules
+  echo "# --write would install: ${HELPERS[*]/#/$YBIN/} -> $PRIVILEGED_DIR/ (root:0 0755)"
+  GRANTEE_CONTROLLED=$(grantee_controlled_sources)
+  [ -z "$GRANTEE_CONTROLLED" ] || { echo "# --write would refuse, a grantee controls the source:"; echo "$GRANTEE_CONTROLLED" | sed 's/^/#   /'; }
   sudo_rs_check
+  exit 0
+fi
+
+if [ "$(id -u)" -ne 0 ]; then
+  echo "Writing sudoers rules requires root. Run with sudo, from a checkout root can read:" >&2
+  echo "  sudo $0 --write ${USERS[*]}" >&2
+  exit 1
+fi
+
+GRANTEE_CONTROLLED=$(grantee_controlled_sources)
+if [ -n "$GRANTEE_CONTROLLED" ] && [ "$ALLOW_GRANTEE_SOURCE" != "true" ]; then
+  echo "ERROR: refusing to install root-run helpers from a checkout a grantee controls:" >&2
+  echo "$GRANTEE_CONTROLLED" >&2
+  echo "Run from your own ystack checkout, or pass --allow-grantee-source after reviewing it." >&2
+  exit 1
 fi
+
+TMPFILE=$(mktemp)
+trap 'rm -f "$TMPFILE"' EXIT
+rules > "$TMPFILE"
+if ! visudo -cf "$TMPFILE"; then
+  echo "ERROR: Generated sudoers rules failed validation" >&2
+  exit 1
+fi
+
+install -d -o 0 -g 0 -m 0755 "$PRIVILEGED_DIR"
+for h in "${HELPERS[@]}"; do
+  install -o 0 -g 0 -m 0755 "$YBIN/$h" "$PRIVILEGED_DIR/$h"
+done
+# No git as root in a checkout root does not own: its config can run commands
+{
+  echo "# Installed by y-cluster-sudoers --write for: ${USERS[*]}"
+  echo "installed=$(date -u +%Y-%m-%dT%H:%M:%SZ)"
+  echo "source=$YSTACK_HOME"
+  (cd "$PRIVILEGED_DIR" && for h in "${HELPERS[@]}"; do shasum -a 256 "$h" 2>/dev/null || sha256sum "$h"; done)
+} > "$PRIVILEGED_DIR/SOURCE.tmp"
+chmod 0644 "$PRIVILEGED_DIR/SOURCE.tmp"
+mv "$PRIVILEGED_DIR/SOURCE.tmp" "$PRIVILEGED_DIR/SOURCE"
+
+install -o 0 -g 0 -m 0440 "$TMPFILE" "$SUDOERS_FILE"
+echo "Installed ${HELPERS[*]} to $PRIVILEGED_DIR, wrote $SUDOERS_FILE"
+sudo_rs_check

bin/y-etc-hosts-write

@@ -13,6 +13,16 @@ path the way a rule for a flag-taking binary can.
 Refuses input that is empty or has no 127.0.0.1 entry, and swaps the file in
 with rename(2) so a reader never sees a half-written hosts file.
 
+It runs as root for users who are not root otherwise, so it only lets them manage
+the block k8s-ingress-hosts owns: everything outside the lines
+  # generated using k8s-ingress-hosts start #
+  # generated using k8s-ingress-hosts end #
+must stay as it is in the current file, there is at most one such block, and each
+entry in it maps plain hostnames to a loopback or private IPv4 address (127/8,
+10/8, 172.16/12, 192.168/16), optionally followed by a # comment. A name that has
+an entry outside the block keeps that address. Input is limited to 1 MiB. Other
+edits of /etc/hosts are for root to make.
+
 Environment:
   Y_ETC_HOSTS  Target path (default: /etc/hosts)
 
@@ -47,7 +57,13 @@ fi
 TMP=$(mktemp "$TARGET_DIR/.hosts.XXXXXX")
 trap 'rm -f "$TMP"' EXIT
 
-cat > "$TMP"
+# Root writes this into the target's filesystem, so the input is bounded
+MAX_BYTES=1048576
+head -c $((MAX_BYTES + 1)) > "$TMP"
+if [ "$(wc -c < "$TMP")" -gt "$MAX_BYTES" ]; then
+  echo "ERROR: refusing input over $MAX_BYTES bytes" >&2
+  exit 2
+fi
 
 if [ ! -s "$TMP" ]; then
   echo "ERROR: refusing to write an empty $TARGET" >&2
@@ -58,6 +74,61 @@ if ! grep -qE '^[[:space:]]*127\.0\.0\.1[[:space:]]' "$TMP"; then
   exit 2
 fi
 
+BLOCK_START='# generated using k8s-ingress-hosts start #'
+BLOCK_END='# generated using k8s-ingress-hosts end #'
+
+# Lines outside the managed block, markers excluded
+outside_block() {
+  awk -v s="$BLOCK_START" -v e="$BLOCK_END" '$0 == s {inb=1; next} $0 == e {inb=0; next} !inb' "$1"
+}
+
+if [ "$(grep -cxF "$BLOCK_START" "$TMP")" -gt 1 ] || [ "$(grep -cxF "$BLOCK_START" "$TMP")" != "$(grep -cxF "$BLOCK_END" "$TMP")" ]; then
+  echo "ERROR: refusing input without exactly zero or one k8s-ingress-hosts block" >&2
+  exit 2
+fi
+
+if [ -f "$TARGET" ] && ! cmp -s <(outside_block "$TARGET") <(outside_block "$TMP"); then
+  echo "ERROR: refusing input that changes $TARGET outside the k8s-ingress-hosts block:" >&2
+  diff <(outside_block "$TARGET") <(outside_block "$TMP") >&2 || true # y-script-lint:disable=or-true # diff exits 1 when it has output, which is the case being reported
+  exit 2
+fi
+
+# Two passes over the input: the first records the addresses of names outside the block,
+# so a block entry cannot give such a name another address (the block may come first)
+BAD_ENTRIES=$(awk -v s="$BLOCK_START" -v e="$BLOCK_END" '
+  function octet(x) { return x ~ /^[0-9]+$/ && x + 0 <= 255 }
+  function allowed(ip,   o) {
+    if (split(ip, o, ".") != 4 || !octet(o[1]) || !octet(o[2]) || !octet(o[3]) || !octet(o[4])) return 0
+    return o[1] == 127 || o[1] == 10 || (o[1] == 172 && o[2] >= 16 && o[2] <= 31) || (o[1] == 192 && o[2] == 168)
+  }
+  NR == FNR {
+    if ($0 == s) { inb = 1 } else if ($0 == e) { inb = 0 }
+    else if (!inb && $0 !~ /^[[:space:]]*(#|$)/) {
+      for (i = 2; i <= NF && $i !~ /^#/; i++) if (!(tolower($i) in outside)) outside[tolower($i)] = $1
+    }
+    next
+  }
+  FNR == 1 {inb=0}
+  $0 == s {inb=1; next}
+  $0 == e {inb=0; next}
+  inb && $0 !~ /^[[:space:]]*(#|$)/ {
+    ok = allowed($1)
+    names = 0
+    # k8s-ingress-hosts ends each entry with a comment naming its route
+    for (i = 2; i <= NF && $i !~ /^#/; i++) {
+      names++
+      if ($i !~ /^[A-Za-z0-9]([-A-Za-z0-9.]*[A-Za-z0-9])?$/) ok = 0
+      # Host lookups ignore case, so DB.INTERNAL is db.internal
+      if ((tolower($i) in outside) && outside[tolower($i)] != $1) ok = 0
+    }
+    if (!ok || names == 0) print
+  }' "$TMP" "$TMP")
+if [ -n "$BAD_ENTRIES" ]; then
+  echo "ERROR: refusing k8s-ingress-hosts entries that are not a loopback or private IPv4 address followed by hostnames, or that give a name from outside the block another address:" >&2
+  echo "$BAD_ENTRIES" >&2
+  exit 2
+fi
+
 if cmp -s "$TMP" "$TARGET"; then
   echo "# $TARGET already up to date"
   exit 0

bin/y-k8s-ingress-hosts

@@ -210,7 +210,20 @@ if cmp -s "$NEW_HOSTS" "$HOSTS_FILE"; then
 fi
 
 if [ "$HOSTS_FILE" = "/etc/hosts" ] && [ "$(id -u)" -ne 0 ]; then
-  sudo "$YBIN/y-etc-hosts-write" < "$NEW_HOSTS"
+  # The root-owned copy y-cluster-sudoers --write installs is what its NOPASSWD rule names;
+  # the checkout's own copy only works with a sudo password (or an old, broader rule)
+  WRITER=/usr/local/libexec/ystack/y-etc-hosts-write
+  case "$(uname -s)" in
+    Darwin) WRITER_PERM=$(stat -f '%u %Lp' "$WRITER" 2>/dev/null) || WRITER_PERM= ;;
+    *) WRITER_PERM=$(stat -c '%u %a' "$WRITER" 2>/dev/null) || WRITER_PERM= ;;
+  esac
+  if [[ "$WRITER_PERM" =~ ^0\ [0-7]?[0-7][0145][0145]$ ]]; then
+    cmp -s "$WRITER" "$YBIN/y-etc-hosts-write" \
+      || echo "# $WRITER differs from $YBIN/y-etc-hosts-write, rerun y-cluster-sudoers --write to update it" >&2
+  else
+    WRITER="$YBIN/y-etc-hosts-write"
+  fi
+  sudo "$WRITER" < "$NEW_HOSTS"
 else
   Y_ETC_HOSTS="$HOSTS_FILE" "$YBIN/y-etc-hosts-write" < "$NEW_HOSTS"
 fi