Skip to content

LLM-review sudoers setup #86

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
solsson merged 4 commits into from
Sep 30, 2026
Merged

LLM-review sudoers setup #86

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6aef3cb
y-etc-hosts-write: bounded input, and outside names keep their address
Sep 30, 2026
ee9bc6a
y-cluster-sudoers: refuse sources a grantee can write, not only own
Sep 30, 2026
2dcac6c
y-etc-hosts-write: compare names outside the block case-insensitively
Sep 30, 2026
943bfc4
y-cluster-sudoers: do not advise running the check as root
Sep 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 39 additions & 15 deletions bin/y-cluster-sudoers
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -25,8 +25,12 @@ 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: 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
Expand Down Expand Up @@ -77,15 +81,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() {
Expand All @@ -104,8 +128,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
Expand All @@ -116,10 +140,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
Expand Down
29 changes: 24 additions & 5 deletions bin/y-etc-hosts-write
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 (!(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:]]*(#|$)/ {
Expand All @@ -101,11 +118,13 @@ 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
# 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" "$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
Expand Down
Loading