From 39a175a91fa7013930d31ffccaee71461d488b20 Mon Sep 17 00:00:00 2001 From: Yolean gle-qa Date: Tue, 29 Sep 2026 05:57:05 +0000 Subject: [PATCH 1/9] y-cluster-sudoers: NOPASSWD rules for root-owned copies of the helpers only The rules named $YBIN/y-etc-hosts-write and $YBIN/y-localhost, files in the grantee's own checkout. Whoever can edit them, the grantee and every agent running as that user, could put anything there and run it as root. That was acceptable on a laptop where the user is the admin anyway, not on a shared or hardened host where an unprivileged bot account runs clusters. y-cluster-sudoers --write now installs root-owned snapshots (0755, owner 0) of both helpers to /usr/local/libexec/ystack, records their sha256 and source path in SOURCE there (no git as root: a checkout's config can run commands), and writes rules naming only those copies, validated with visudo -cf. It refuses to snapshot from a checkout owned by a grantee unless --allow-grantee-source; the intended use is an admin running it from their own checkout at a reviewed commit. Without --write it prints the rules, what would be installed and whether --write would refuse. Callers prefer the installed copy when it is root-owned and not group/other writable, and fall back to $YBIN otherwise (a sudo password, or an old rule): y-k8s-ingress-hosts for the hosts swap, warning when the copy differs from the checkout's; y-localhost for its self-escalation. Root-owned alone is not enough, because the helpers act on input the grantee controls, so they now limit what a NOPASSWD run can do: - y-etc-hosts-write only accepts changes inside the k8s-ingress-hosts block (everything outside must equal the current file, at most one block), and entries there must map plain hostnames to 127/8, 10/8, 172.16/12 or 192.168/16. Before, a grantee could replace /etc/hosts wholesale, e.g. point any public name at an address of their choice for all users of the host. - y-localhost only adds 127.0.0.0/8 aliases to lo. It took the address from /etc/hosts, which y-etc-hosts-write let the same user write, so it could bind any address, a public one included, to loopback and capture the host's traffic to it. Hostname argument validated, exact field match instead of a grep regex. Relates to Yolean/y-cluster#11 (install help for passwordless sudo, to a path where only root can update the binary). Tested on gle01 with temp targets (Y_ETC_HOSTS) and a copy of y-localhost on a fake hosts file: block append/removal pass; edits outside the block, public or 172.32 addresses, two or unbalanced blocks, odd hostnames and non-loopback aliases are refused. --write itself not run (needs root). Co-Authored-By: Claude Opus 5.5 --- bin/y-cluster-sudoers | 140 +++++++++++++++++++++++++++------------- bin/y-etc-hosts-write | 46 +++++++++++++ bin/y-k8s-ingress-hosts | 15 ++++- bin/y-localhost | 45 ++++++++++--- 4 files changed, 192 insertions(+), 54 deletions(-) diff --git a/bin/y-cluster-sudoers b/bin/y-cluster-sudoers index 68a1393e..b61313db 100755 --- a/bin/y-cluster-sudoers +++ b/bin/y-cluster-sudoers @@ -4,23 +4,31 @@ set -eo pipefail YSTACK_HOME="$(cd "$(dirname "$0")/.." && pwd)" 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 <&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 </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_OWNED=$(grantee_owned_sources) + [ -z "$GRANTEE_OWNED" ] || { echo "# --write would refuse, a grantee controls the source:"; echo "$GRANTEE_OWNED" | 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_OWNED=$(grantee_owned_sources) +if [ -n "$GRANTEE_OWNED" ] && [ "$ALLOW_GRANTEE_SOURCE" != "true" ]; then + echo "ERROR: refusing to install root-run helpers from a checkout a grantee controls:" >&2 + echo "$GRANTEE_OWNED" >&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 diff --git a/bin/y-etc-hosts-write b/bin/y-etc-hosts-write index 7ffa7622..31987cfc 100755 --- a/bin/y-etc-hosts-write +++ b/bin/y-etc-hosts-write @@ -13,6 +13,14 @@ 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). Other edits of /etc/hosts are for root to make. + Environment: Y_ETC_HOSTS Target path (default: /etc/hosts) @@ -58,6 +66,44 @@ 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 + +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) + } + $0 == s {inb=1; next} + $0 == e {inb=0; next} + inb && $0 !~ /^[[:space:]]*(#|$)/ { + ok = allowed($1) && NF >= 2 + for (i = 2; i <= NF; i++) if ($i !~ /^[A-Za-z0-9]([-A-Za-z0-9.]*[A-Za-z0-9])?$/) ok = 0 + if (!ok) print + }' "$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:" >&2 + echo "$BAD_ENTRIES" >&2 + exit 2 +fi + if cmp -s "$TMP" "$TARGET"; then echo "# $TARGET already up to date" exit 0 diff --git a/bin/y-k8s-ingress-hosts b/bin/y-k8s-ingress-hosts index f720551f..a92bcc37 100755 --- a/bin/y-k8s-ingress-hosts +++ b/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 diff --git a/bin/y-localhost b/bin/y-localhost index 26c599a4..38da0815 100755 --- a/bin/y-localhost +++ b/bin/y-localhost @@ -4,33 +4,62 @@ set -eo pipefail [ "$1" = "help" ] && echo ' Tries to bring up a loopback interface so port bindings can be done without conflicts + +Only 127.0.0.0/8 addresses: it runs as root through a NOPASSWD rule, and a public +address on lo would take over the host'"'"'s traffic to it. Escalates through the +root-owned copy that y-cluster-sudoers --write installs, when there is one. ' && exit 0 +PRIVILEGED_DIR=/usr/local/libexec/ystack + hostname=$1 [ -z "$hostname" ] && echo "First arg must be a hostname" && exit 1 +[[ "$hostname" =~ ^[A-Za-z0-9]([-A-Za-z0-9.]*[A-Za-z0-9])?$ ]] || { echo "Not a hostname: '$hostname'" >&2; exit 1; } + +# Root-owned and not writable by others, so a NOPASSWD rule for it grants only what it does +privileged_self() { + local f="$PRIVILEGED_DIR/y-localhost" perm + [ -f "$f" ] || return 1 + case "$(uname -s)" in + Darwin) perm=$(stat -f '%u %Lp' "$f") ;; + *) perm=$(stat -c '%u %a' "$f") ;; + esac + [[ "$perm" =~ ^0\ [0-7]?[0-7][0145][0145]$ ]] && echo "$f" +} +escalate() { + local self + if self=$(privileged_self); then exec sudo "$self" "$@"; fi + exec sudo "$0" "$@" +} platform="$(uname -s)" -ip=$(cat /etc/hosts | grep -e "[[:blank:]]\<$hostname\>" | awk '{ print $1 }') +ip=$(awk -v h="$hostname" '$1 !~ /^#/ { for (i = 2; i <= NF; i++) if ($i == h) { print $1; exit } }' /etc/hosts) [ -z "$ip" ] && >&2 echo "Host '$hostname' not found in /etc/hosts" && exit 1 [ "$2" = "show" ] && echo -n "$ip" && exit 0 -num=$(echo -n $ip | cut -d '.' -f 4) +if ! [[ "$ip" =~ ^127\.([0-9]{1,3})\.([0-9]{1,3})\.([0-9]{1,3})$ ]] \ + || [ "${BASH_REMATCH[1]}" -gt 255 ] || [ "${BASH_REMATCH[2]}" -gt 255 ] || [ "${BASH_REMATCH[3]}" -gt 255 ]; then + echo "Refusing $ip for $hostname: loopback aliases must be in 127.0.0.0/8" >&2 + exit 1 +fi + +num=$(echo -n "$ip" | cut -d '.' -f 4) case "$platform" in Darwin) - ifconfig lo0 | grep $ip > /dev/null && exit 0 + ifconfig lo0 | grep -F "$ip" > /dev/null && exit 0 echo "Loopback alias for $ip $hostname not found. Will try to create ..." - [ $(id -u) -eq 0 ] || exec sudo $0 "$@" - ifconfig lo0 alias $ip up + [ "$(id -u)" -eq 0 ] || escalate "$@" + ifconfig lo0 alias "$ip" up ;; Linux) - if [ -z "$(ip addr show dev lo label lo:$num)" ]; then + if [ -z "$(ip addr show dev lo label "lo:$num")" ]; then echo "Adding missing interface lo:$num ..." - [ $(id -u) -eq 0 ] || exec sudo $0 "$@" - ip addr add $ip/32 dev lo label lo:$num + [ "$(id -u)" -eq 0 ] || escalate "$@" + ip addr add "$ip/32" dev lo label "lo:$num" fi ;; *) From 026c7dd1d9dc5737d5c6a03ea46554a99b0a07e8 Mon Sep 17 00:00:00 2001 From: Yolean gle-qa Date: Tue, 29 Sep 2026 07:17:03 +0000 Subject: [PATCH 2/9] y-cluster-sudoers: don't exit silently when no grantee owns the source grantee_owned_sources ended in `[ "$owner" = "$u" ] && echo ...`, so when the last comparison was false (the intended case: an admin's own checkout) the function returned 1, and under set -e the GRANTEE_OWNED=$(...) assignment ended the script without a word. --write therefore installed nothing and wrote no rules, and the dry run stopped after its "would install" line. Found when the owner's run on gle01 left no /usr/local/libexec/ystack. The earlier tests used a checkout owned by the grantee, where the last comparison happens to be true. Tested now with a grantee owning nothing, and the whole --write path run as non-root against scratch paths (validation, install of both helpers, SOURCE, sudoers file 0440). Co-Authored-By: Claude Opus 5.5 --- bin/y-cluster-sudoers | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/y-cluster-sudoers b/bin/y-cluster-sudoers index b61313db..72d16965 100755 --- a/bin/y-cluster-sudoers +++ b/bin/y-cluster-sudoers @@ -83,7 +83,7 @@ grantee_owned_sources() { for h in "${HELPERS[@]}" ""; do owner=$(file_owner "$YBIN/$h") for u in "${USERS[@]}"; do - [ "$owner" = "$u" ] && echo "$YBIN/$h is owned by grantee $u" + if [ "$owner" = "$u" ]; then echo "$YBIN/$h is owned by grantee $u"; fi done done } From e42843af8acb4affcd3e7a6c982ac7296adc875a Mon Sep 17 00:00:00 2001 From: Yolean gle-qa Date: Tue, 29 Sep 2026 11:28:55 +0000 Subject: [PATCH 3/9] buckety-controller: site-userdata-blobs backend on the local versitygw checkit's blobs-v3/site-apply requests one bucket per site from the backend named site-userdata-blobs; on g2 that is GCS. The comment next to builds-registry-blobs said to add this entry, on the same versitygw, once a local cluster hosts sites, so checkit's per-site Buckety CRs are byte-identical here and on g2. The checkit appliance (checkit branch sprint-060-appliance) is that cluster. Co-Authored-By: Claude Opus 5.5 --- buckety-controller/buckety-controller.yaml | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/buckety-controller/buckety-controller.yaml b/buckety-controller/buckety-controller.yaml index d03b496e..13065442 100644 --- a/buckety-controller/buckety-controller.yaml +++ b/buckety-controller/buckety-controller.yaml @@ -67,10 +67,8 @@ backends: # # Deliberately NOT named site-userdata-blobs, which is what checkit's g2 # backend is called: that name belongs to per-site end-user blobs, and this -# bucket holds build artifacts. When cluster-local starts hosting sites, the -# right move is to ADD a site-userdata-blobs entry pointing at this same -# versitygw - at which point checkit's per-site CRs become byte-identical here, -# which is the whole point of the naming contract. +# bucket holds build artifacts; site-userdata-blobs below points at the same +# versitygw so checkit's per-site CRs are byte-identical here and on g2. - name: builds-registry-blobs driver: s3 config: @@ -84,3 +82,15 @@ backends: # deployment-patch.yaml). accessKeyID: ${VERSITYGW_ROOT_ACCESSKEY} secretAccessKey: ${VERSITYGW_ROOT_SECRETKEY} + +# Per-site end-user blobs: checkit's blobs-v3/site-apply Buckety CR asks for this +# backend by name, one bucket per site, on g2 a GCS bucket, here versitygw. +- name: site-userdata-blobs + driver: s3 + config: + implementation: versitygw + endpoint: http://y-s3-api.blobs.svc.cluster.local:9000 + region: us-east-1 + forcePathStyle: true + accessKeyID: ${VERSITYGW_ROOT_ACCESSKEY} + secretAccessKey: ${VERSITYGW_ROOT_SECRETKEY} From f2b6d916054c08de8877f0e195c34d2996c69028 Mon Sep 17 00:00:00 2001 From: Yolean gle-qa Date: Tue, 29 Sep 2026 11:59:15 +0000 Subject: [PATCH 4/9] y-bin-dependency-download: a binary appears only when complete The download went straight to the binary's final path and was extracted and made executable in place afterwards. A run that stopped after curl left the archive there, and every later run saw the file and executed it: "cannot execute binary file: Exec format error" until someone deleted it. A caller piping to head is enough to stop it, by SIGPIPE on the next line of download output. Parallel agents running `kubectl ... 2>&1 | head -3` on a checkout that needed kubectl 1.36.5 left both a ystack clone and a worktree with the 34 MB kubernetes-client tarball as y-kubectl-v1.36.5-bin. Now the download, the sha256 check and the unpacking all work on a temporary file next to the binary, which is renamed into place only when complete, so an interrupted run leaves no binary and a concurrent one sees either none or a finished one. The name symlink is replaced by rename as well: removing it first let a parallel run find it missing and fail (2 of 60 invocations in a 12-way test; 0 of 120 now). A regular file at the link's name is still refused, as before. Co-Authored-By: Claude Opus 5.5 --- bin/.gitignore | 3 +++ bin/y-bin-dependency-download | 37 +++++++++++++++++++++-------------- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/bin/.gitignore b/bin/.gitignore index f488d09f..15ee3abb 100644 --- a/bin/.gitignore +++ b/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 diff --git a/bin/y-bin-dependency-download b/bin/y-bin-dependency-download index cc34846b..dd7a27f6 100755 --- a/bin/y-bin-dependency-download +++ b/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 From 4d48d02f8ecdc88cedd6130d4a07c8550e5953ff Mon Sep 17 00:00:00 2001 From: Yolean gle-qa Date: Tue, 29 Sep 2026 12:09:45 +0000 Subject: [PATCH 5/9] y-etc-hosts-write: accept the route comment k8s-ingress-hosts writes k8s-ingress-hosts ends each entry with a comment naming the route it came from ("127.0.0.1 keycloak-admin.appliance # keycloak-v3/ keycloak-admin"), and the block policy from 39a175a checked every field after the IP as a hostname, so it refused every entry the tool writes. A field starting with # now ends the entry, as in hosts(5); an entry still needs at least one valid hostname before it, and a # inside a name is still refused. Co-Authored-By: Claude Opus 5.5 --- bin/y-etc-hosts-write | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/bin/y-etc-hosts-write b/bin/y-etc-hosts-write index 31987cfc..4ae49068 100755 --- a/bin/y-etc-hosts-write +++ b/bin/y-etc-hosts-write @@ -19,7 +19,8 @@ the block k8s-ingress-hosts owns: everything outside the lines # 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). Other edits of /etc/hosts are for root to make. +10/8, 172.16/12, 192.168/16), optionally followed by a # comment. Other edits of +/etc/hosts are for root to make. Environment: Y_ETC_HOSTS Target path (default: /etc/hosts) @@ -94,9 +95,14 @@ BAD_ENTRIES=$(awk -v s="$BLOCK_START" -v e="$BLOCK_END" ' $0 == s {inb=1; next} $0 == e {inb=0; next} inb && $0 !~ /^[[:space:]]*(#|$)/ { - ok = allowed($1) && NF >= 2 - for (i = 2; i <= NF; i++) if ($i !~ /^[A-Za-z0-9]([-A-Za-z0-9.]*[A-Za-z0-9])?$/) ok = 0 - if (!ok) print + 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 + } + if (!ok || names == 0) print }' "$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:" >&2 From 6aef3cba31444323795fe9a0a0c780b0ec4d7486 Mon Sep 17 00:00:00 2001 From: Yolean k8s-qa Date: Wed, 30 Sep 2026 05:24:12 +0000 Subject: [PATCH 6/9] y-etc-hosts-write: bounded input, and outside names keep their address Two ways a NOPASSWD grantee could still use the root-run writer against the host, found reviewing #85 and reproduced with Y_ETC_HOSTS on scratch files (material in bots-state/code-review/prs/ystack-85/ on metalblack): - The block could give a name that root maintains outside it another address. The block's position is free, and hosts(5) lookups take the first match, so a block placed first with "127.0.0.9 db.internal" shadowed root's "10.1.1.1 db.internal" for every user of the host. A block entry for a name with an entry outside the block must now use that entry's address. A duplicate with the same address is still accepted, so a hand-made line for a cluster name keeps working. - stdin went to a temporary file next to /etc/hosts with no bound, as root, so `yes | sudo y-etc-hosts-write` could fill the root filesystem including its root reserve, before any check ran. A valid 46 MB block of comments was also accepted as /etc/hosts. Input is now limited to 1 MiB, far above what k8s-ingress-hosts writes. Verified with the PR head's probes rerun against this version: the shadowing input and the 46 MB input are refused (exit 2); route comments, same-address duplicates and a 40000-entry block are accepted; the PR's refusals (public address, edits outside the block) are unchanged. shellcheck and y-script-lint are clean. Co-Authored-By: Claude Opus 5.5 (1M context) --- bin/y-etc-hosts-write | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/bin/y-etc-hosts-write b/bin/y-etc-hosts-write index 4ae49068..db0e5e6c 100755 --- a/bin/y-etc-hosts-write +++ b/bin/y-etc-hosts-write @@ -19,8 +19,9 @@ the block k8s-ingress-hosts owns: everything outside the lines # 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. Other edits of -/etc/hosts are for root to make. +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) @@ -56,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 @@ -86,12 +93,22 @@ if [ -f "$TARGET" ] && ! cmp -s <(outside_block "$TARGET") <(outside_block "$TMP 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 (!($i in outside)) outside[$i] = $1 + } + next + } + FNR == 1 {inb=0} $0 == s {inb=1; next} $0 == e {inb=0; next} inb && $0 !~ /^[[:space:]]*(#|$)/ { @@ -101,11 +118,12 @@ BAD_ENTRIES=$(awk -v s="$BLOCK_START" -v e="$BLOCK_END" ' for (i = 2; i <= NF && $i !~ /^#/; i++) { names++ if ($i !~ /^[A-Za-z0-9]([-A-Za-z0-9.]*[A-Za-z0-9])?$/) ok = 0 + if (($i in outside) && outside[$i] != $1) ok = 0 } if (!ok || names == 0) print - }' "$TMP") + }' "$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:" >&2 + 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 From ee9bc6a063214eb49bae85b8bfd745b9d614371c Mon Sep 17 00:00:00 2001 From: Yolean k8s-qa Date: Wed, 30 Sep 2026 05:37:05 +0000 Subject: [PATCH 7/9] y-cluster-sudoers: refuse sources a grantee can write, not only own --write checked the owner of the two helpers and of bin/ only. On a noble VM (classic sudo 1.9.15p5) a grantee got root without a password in three ways the owner check does not see, all with a root-owned checkout: a helper group-writable by the grantee's group, bin/ group-writable (the grantee exchanged a helper with its own file between the check and the install, renameat2 RENAME_EXCHANGE, 1 in 13 runs), and a root-created symlink to a file the grantee owns. A grantee-owned directory above the checkout let it swap the whole tree during --write. Report and logs: bots-state/code-review/prs/ystack-85/vm/ on metalblack. The check now walks the helpers, this script and every directory above them (on the physical path, pwd -P), and refuses when a grantee owns one, can write one (as root: sudo -u test -w, so group, other and ACL grants count), or one is a symlink. With nothing on the path writable by a grantee, nothing can change between the check and the copy. The script cannot vouch for itself, which the usage now says: a grantee who could write it before the admin runs it runs as root, so the admin checks with the show mode first, as root, which reports the same. Co-Authored-By: Claude Opus 5.5 (1M context) --- bin/y-cluster-sudoers | 53 +++++++++++++++++++++++++++++++------------ 1 file changed, 38 insertions(+), 15 deletions(-) diff --git a/bin/y-cluster-sudoers b/bin/y-cluster-sudoers index 72d16965..7b9a6a88 100755 --- a/bin/y-cluster-sudoers +++ b/bin/y-cluster-sudoers @@ -2,7 +2,7 @@ [ -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 @@ -25,8 +25,11 @@ Usernames: one or more users to grant NOPASSWD rules to. Defaults to the current user (via SUDO_USER or USER). 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 a checkout owned by one of the -grantees, whose content they control, unless --allow-grantee-source is given. +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: a grantee who +could write this script before you run it runs as root, so check with the plain +(show) mode first, as root, which reports the same. Rerun it after updating ystack to refresh the copies; y-k8s-ingress-hosts warns when they differ. EOF @@ -77,15 +80,35 @@ file_owner() { esac } -# A grantee who owns the checkout could change a helper right before the copy is taken -grantee_owned_sources() { - local h owner u - for h in "${HELPERS[@]}" ""; do - owner=$(file_owner "$YBIN/$h") +# 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 "$YBIN/$h is owned by grantee $u"; fi + 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 + done < <(source_paths) } sudo_rs_check() { @@ -104,8 +127,8 @@ sudo_rs_check() { if [ "$MODE" = "show" ]; then rules echo "# --write would install: ${HELPERS[*]/#/$YBIN/} -> $PRIVILEGED_DIR/ (root:0 0755)" - GRANTEE_OWNED=$(grantee_owned_sources) - [ -z "$GRANTEE_OWNED" ] || { echo "# --write would refuse, a grantee controls the source:"; echo "$GRANTEE_OWNED" | sed 's/^/# /'; } + 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 @@ -116,10 +139,10 @@ if [ "$(id -u)" -ne 0 ]; then exit 1 fi -GRANTEE_OWNED=$(grantee_owned_sources) -if [ -n "$GRANTEE_OWNED" ] && [ "$ALLOW_GRANTEE_SOURCE" != "true" ]; then +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_OWNED" >&2 + echo "$GRANTEE_CONTROLLED" >&2 echo "Run from your own ystack checkout, or pass --allow-grantee-source after reviewing it." >&2 exit 1 fi From 2dcac6cdc0d694ea97cf411bb14bcdcbfe91c54a Mon Sep 17 00:00:00 2001 From: Yolean k8s-qa Date: Wed, 30 Sep 2026 05:42:47 +0000 Subject: [PATCH 8/9] y-etc-hosts-write: compare names outside the block case-insensitively 6aef3cb refused a block entry that gives a name from outside the block another address, comparing names byte for byte. Host lookups ignore case, so "127.0.0.9 DB.INTERNAL" in the block was accepted and won over root's "10.1.1.1 db.internal": getent hosts and getaddrinfo both returned 127.0.0.9 first, and ping went there, whether the block came first or last (VM run, bots-state/code-review/prs/ystack-85/vm-fix/ 09b-case-bypass.log on metalblack). Both sides are now lowercased. Verified on scratch targets: DB.INTERNAL at the top and Db.Internal at the end refused (exit 2); "10.1.1.1 DB.internal" and "127.0.0.1 LOCALHOST" (same addresses as outside) and a route-comment block accepted. Co-Authored-By: Claude Opus 5.5 (1M context) --- bin/y-etc-hosts-write | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bin/y-etc-hosts-write b/bin/y-etc-hosts-write index db0e5e6c..1f0c8916 100755 --- a/bin/y-etc-hosts-write +++ b/bin/y-etc-hosts-write @@ -104,7 +104,7 @@ BAD_ENTRIES=$(awk -v s="$BLOCK_START" -v e="$BLOCK_END" ' 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 (!($i in outside)) outside[$i] = $1 + for (i = 2; i <= NF && $i !~ /^#/; i++) if (!(tolower($i) in outside)) outside[tolower($i)] = $1 } next } @@ -118,7 +118,8 @@ BAD_ENTRIES=$(awk -v s="$BLOCK_START" -v e="$BLOCK_END" ' for (i = 2; i <= NF && $i !~ /^#/; i++) { names++ if ($i !~ /^[A-Za-z0-9]([-A-Za-z0-9.]*[A-Za-z0-9])?$/) ok = 0 - if (($i in outside) && outside[$i] != $1) 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") From 943bfc4013e4214b2371c1bbc8d62b075c28b0c0 Mon Sep 17 00:00:00 2001 From: Yolean k8s-qa Date: Wed, 30 Sep 2026 05:42:47 +0000 Subject: [PATCH 9/9] y-cluster-sudoers: do not advise running the check as root ee9bc6a's usage told the admin to check with the show mode as root first. On the VM a grantee who could write bin/ replaced this script, and that advice ran the grantee's line as root before any report (prs/ystack-85/vm-fix/07-show-as-root.log on metalblack). The script cannot check itself, so the usage now says: show mode as yourself for what a grantee owns, and write access checked without executing the checkout (ls -ld on the path) before sudo. Co-Authored-By: Claude Opus 5.5 (1M context) --- bin/y-cluster-sudoers | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/bin/y-cluster-sudoers b/bin/y-cluster-sudoers index 7b9a6a88..cd0a6819 100755 --- a/bin/y-cluster-sudoers +++ b/bin/y-cluster-sudoers @@ -27,9 +27,10 @@ Usernames: one or more users to grant NOPASSWD rules to. 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: a grantee who -could write this script before you run it runs as root, so check with the plain -(show) mode first, as root, which reports the same. +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