From 8d03184e55c9d84ee41f2eb58e53c8cc41eded07 Mon Sep 17 00:00:00 2001 From: dan-claude-bot Date: Sun, 19 Jul 2026 23:35:42 +0000 Subject: [PATCH] fix: gate 'users apply' on an empty file that would revoke everyone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A users file naming zero users is a valid instruction to revoke every operator on the box, and it is indistinguishable from the file a stray '>' produces. The per-user warnings apply already emitted arrive after the decision and scale wrong: twenty operators is twenty lines of scrollback, so the signal was loudest exactly where it read as noise. The /etc/rig/users ledger draws the line apply needs. An empty file against an empty ledger is an unambiguous no-op; against a populated one it closes every named door. Only the second now stops, states how many operators are at risk, and requires explicit consent: --yes, RIG_YES=1 (the installer-family variable bin/rig's uninstall_confirm already reads), or a y on a TTY. Without a terminal and without consent it exits 2 in that same refusal's words, rather than assume a yes it cannot ask for or hang on a prompt nothing can answer. A confirmation, not bootstrap's flat refusal of the same file (#57/#59): bootstrap asserts who lives on a box, apply converges, and converging to zero stays a legitimate de-provisioning. Ledger entries already marked revoked do not count toward the number, so a second identical run stays the silent no-op convergence promises. Mass revocation below the empty-file bright line is deliberately still ungated — that needs a threshold someone has to justify. Refs #65 --- CHANGELOG.md | 21 +++++++++ README.md | 29 ++++++++++++ commands/users-apply.sh | 70 ++++++++++++++++++++++++++++- test/cli.sh | 99 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 218 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cc80cc6..04447b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -214,6 +214,27 @@ on the way to cutting its first release, and this file starts there. pty (util-linux `script`, skipped where it is absent) and asserting the MESSAGE rather than the exit code, which the bug also produced. +- **`users apply` now tells "revoke everyone" apart from "I truncated the + file"** (#65) — a users file naming zero users is a valid instruction to + revoke every operator on the box, and it is indistinguishable from a file a + stray `>` produced. The per-user warnings apply already emitted arrive after + the decision and scale wrong: twenty operators is twenty lines of scrollback, + so the signal was loudest exactly where it read as noise. The `/etc/rig/users` + ledger draws the line apply needs — an empty file against an empty ledger is + an unambiguous no-op; against a populated one it closes every named door — so + only the second case now stops, states how many operators are about to be + revoked, and requires explicit consent: `--yes`, `RIG_YES=1` (the + installer-family variable `rig uninstall` already reads), or a `y` on a TTY. + Without a terminal and without consent it exits 2, in `uninstall_confirm`'s + words, rather than assume a yes it cannot ask for or hang on a prompt nothing + can answer. A **confirmation**, not `rig bootstrap`'s flat refusal of the same + file (#57/#59): bootstrap asserts who lives on a box, apply converges, and + converging to zero stays a legitimate de-provisioning. Ledger entries already + marked `revoked` don't count toward the number, so a second identical run + stays the silent no-op. Mass revocation below the empty-file bright line (a + file dropping 19 of 20) is deliberately still ungated — that needs a threshold + someone has to justify, and #65 stays open for it. + ## 0.2.0 — 2026-07-19 ### Added diff --git a/README.md b/README.md index 806c0ab..24aa2d4 100644 --- a/README.md +++ b/README.md @@ -876,6 +876,35 @@ refused as a username: this file names operators; root's fate is root-door polic `--file -` reads stdin. A bad file exits 2 with **every** error listed at once, before anything changes — one fix cycle, not one round-trip per line. +**A file naming zero users is confirmed, not refused (#65).** "Revoke +everyone" and "I truncated the file" are the same instruction in this format, +and a stray `>` writes the second one. apply cannot read intent — but it can +read the `/etc/rig/users` ledger, which draws the only line worth drawing: +against an empty ledger an empty file is an unambiguous no-op, and against a +populated one it closes every named door on the box. So that second case, and +only it, stops and asks — naming how many operators are about to go: + +```sh +rig users apply --file ./users # empty file + managed operators -> asks +rig users apply --file ./users --yes # ...or say yes up front +RIG_YES=1 rig users apply --file ./users # same, for automation +``` + +**Without a terminal and without consent it exits 2** rather than assume a +yes it cannot ask for — the contract `rig uninstall` already uses, and the +same `RIG_YES` the installer family reads. Consent that cannot be obtained is +not consent, and a prompt nothing can answer must not hang either. + +This is a *confirmation*, deliberately unlike `rig bootstrap`'s flat refusal +of the same file (see *Bootstrap*): bootstrap **asserts** who lives on a box, +so an empty answer contradicts itself, while apply **converges** — and +converging to zero is a complete, legitimate de-provisioning that has to keep +working. Already-revoked ledger entries don't count toward the number, so a +second identical run of an emptied file stays the silent no-op convergence +promises. Dropping *most* users — nineteen of twenty — is not yet gated; +that needs a threshold, where "the file is empty" is a bright line that needs +none. + **`@root` — seed keys from the door you came in through (#17).** A key field of exactly `@root` means "this user's `authorized_keys` becomes root's CURRENT `/root/.ssh/authorized_keys`". The point is lockout-avoidance: you diff --git a/commands/users-apply.sh b/commands/users-apply.sh index 44d009b..67fca52 100755 --- a/commands/users-apply.sh +++ b/commands/users-apply.sh @@ -18,9 +18,15 @@ die() { printf 'rig-users: ERROR: %s\n' "$1" >&2; exit "${2:-1}"; } usage() { cat <<'EOF' -usage: rig users apply --file +usage: rig users apply --file [--yes] --file users file (required; '-' reads it from stdin) + --yes consent, up front, to the one prompt this command can ask: + a file naming ZERO users against a box that still has + managed operators revokes all of them. RIG_YES=1 says the + same thing (the installer-family convention). Without + either, that case asks on a TTY and REFUSES (exit 2) + without one — it never assumes consent it cannot get. The file is line-based and bash-parseable on purpose — a rig box has no YAML parser and no jq, and gets neither for this. Whitespace-separated: user, @@ -72,11 +78,17 @@ EOF # --- args (validated before the root check, so errors are testable) --------- FILE="" +# RIG_YES is the installer-family consent contract (bin/rig's uninstall_confirm +# reads the same variable): how automation says yes where there is no terminal +# to say it on. Set here so --yes and the env var are one flag with two doors. +ASSUME_YES=0 +[ -n "${RIG_YES:-}" ] && ASSUME_YES=1 while [ $# -gt 0 ]; do case "$1" in --file) [ $# -ge 2 ] || die "--file needs a value" 2 FILE="$2"; shift 2 ;; + --yes) ASSUME_YES=1; shift ;; -h|--help) usage; exit 0 ;; *) die "unknown flag: $1" 2 ;; esac @@ -501,6 +513,62 @@ done # revoked, data kept, convergence never destroys. LEDGER=/etc/rig/users REVOKED=() + +# --- the empty-file gate (#65) ----------------------------------------------- +# "Revoke everyone" and "I truncated the file" are the same instruction in this +# file format, and apply cannot read intent. What it CAN read is the ledger, and +# that draws the only line worth drawing: a file naming zero users against an +# empty ledger is an unambiguous no-op, while the same file against a populated +# one closes every named door on the box. Only the second is dangerous. +# +# So this is a CONFIRMATION, not a refusal — deliberately unlike bootstrap's +# flat die on the same file (#57/#59). bootstrap ASSERTS who lives on a box, so +# an empty answer there is a self-contradiction; apply CONVERGES, and converging +# to zero is a complete, legitimate de-provisioning that must keep working. The +# difference between the two commands is the whole point, and it survives here. +# +# The per-user warnings below are not this gate and cannot replace it: they +# arrive after the decision, one line per operator, so the signal is loudest +# exactly where it reads as scrollback rather than as a question. +# +# Count FIRST, then speak: the message states a real number, and counting is +# what makes the already-converged case silent. An entry the ledger already +# marks `revoked`, or one whose account no longer exists, is not at risk — this +# run would not change it — so a second identical run of an emptied file stays +# the clean no-op convergence promises, with no prompt to answer twice. +# +# SCOPE: the bright line only. Whether a file that drops 19 of 20 operators +# deserves the same gate is the issue's open question — it needs a threshold +# someone has to justify, where "the file is empty" needs nothing. Left open. +if [ "${#USERS[@]}" -eq 0 ] && [ -r "$LEDGER" ] && [ "$ASSUME_YES" -eq 0 ]; then + AT_RISK=0 + while read -r prev pstate _; do + [ -n "$prev" ] || continue + [ "${pstate:-active}" != "revoked" ] || continue + id -u "$prev" >/dev/null 2>&1 || continue + AT_RISK=$((AT_RISK + 1)) + done < "$LEDGER" + if [ "$AT_RISK" -gt 0 ]; then + warn "this users file names ZERO users, and this box still manages $AT_RISK operator(s): applying it revokes every one of them — accounts expired, authorized_keys renamed, rig groups stripped. If the file was meant to be empty this is de-provisioning; if it was truncated by accident, stop here." + # No terminal means no consent, and a question nobody can answer must not + # be assumed into a yes or left to hang. Same shape and same words as + # bin/rig's uninstall_confirm, on purpose — one refusal in this codebase. + if [ ! -t 0 ]; then + printf 'rig-users: refusing to revoke every managed operator without --yes (no terminal to confirm on; RIG_YES=1 also means yes)\n' >&2 + exit 2 + fi + # `|| reply=""` is load-bearing: under `set -e` a read that hits EOF is a + # non-zero command, and an unguarded read would abort the script rather + # than take the safe default (#68, the same bug class). + printf 'rig-users: revoke all %s managed operator(s) on this box? [y/N] ' "$AT_RISK" + read -r reply || reply="" + case "$reply" in + y|Y|yes|YES|Yes) ;; + *) die "aborted — no operator was revoked" ;; + esac + fi +fi + if [ -r "$LEDGER" ]; then while read -r prev pstate _; do [ -n "$prev" ] || continue diff --git a/test/cli.sh b/test/cli.sh index ec6c0f3..fe12cd5 100644 --- a/test/cli.sh +++ b/test/cli.sh @@ -1070,11 +1070,110 @@ if [ "$(id -u)" -ne 0 ]; then check "users apply: '@root' fixture parses, refuses non-root" 1 "must run as root" \ "$ROOT/commands/users-apply.sh" --file "$FIX_BAD" check "users status: refuses non-root" 1 "must run as root" "$ROOT/commands/users-status.sh" + # --- the empty-file gate's flag surface (#65) ------------------------------ + # Arg parsing precedes the root check, so flag ACCEPTANCE is provable here: + # reaching "must run as root" (exit 1) means --yes was taken, and an exit 2 + # "unknown flag" would mean it was not. The gate's behaviour itself is + # root-only (it reads /etc/rig/users and revokes) and is grep-pinned below. + check "users apply: --yes is accepted" 1 "must run as root" \ + "$ROOT/commands/users-apply.sh" --file "$FIX_OK" --yes + # Order-independent: a consent flag that only worked before --file would be + # a trap for anyone appending it to an existing command line. + check "users apply: --yes is accepted before --file" 1 "must run as root" \ + "$ROOT/commands/users-apply.sh" --yes --file "$FIX_OK" + # --yes takes no value: it must not swallow the next argument. + check "users apply: --yes does not eat the following flag" 2 "unknown flag" \ + "$ROOT/commands/users-apply.sh" --file "$FIX_OK" --yes --nope + # The env door, same as --yes: RIG_YES is the installer-family contract + # (bin/rig's uninstall_confirm reads it), so it must not be an unknown-flag + # equivalent or a parse error either. + check "users apply: RIG_YES=1 parses" 1 "must run as root" \ + env RIG_YES=1 "$ROOT/commands/users-apply.sh" --file "$FIX_OK" else echo "skip: users non-root refusals (running as root)" fi rm -f "$FIX_OK" "$FIX_BAD" +# --- the empty-file gate itself (#65) ---------------------------------------- +# The gated path needs root and a populated /etc/rig/users, so the shipped +# script is grep-pinned instead — the house precedent for root-only refusals +# (the '@root' keyless-seed die above, the invoker gate below). +# +# Consent has three doors and no fourth: --yes, RIG_YES, or a y on a TTY. +check "users apply: --yes sets consent" 0 "" \ + grep -qE '^[[:space:]]*--yes\) ASSUME_YES=1' "$ROOT/commands/users-apply.sh" +check "users apply: RIG_YES is the env door for consent" 0 "" \ + grep -qF 'RIG_YES:-' "$ROOT/commands/users-apply.sh" +# The gate is ledger-gated, not file-gated: zero users ALONE is not the +# condition, or it would refuse the empty-ledger no-op the issue calls +# unambiguous. Both halves of the test must be present on the one line. +# The gate condition and the counter are grepped as LITERALS — single quotes +# intended throughout this block, the expansions are the script's own. +# shellcheck disable=SC2016 +check "users apply: the gate is zero-users AND a readable ledger" 0 "" \ + grep -qF 'if [ "${#USERS[@]}" -eq 0 ] && [ -r "$LEDGER" ] && [ "$ASSUME_YES" -eq 0 ]; then' \ + "$ROOT/commands/users-apply.sh" +# Counting precedes speaking, so the warning states a real number rather than +# "some users" — and an already-revoked entry is not at risk, which is what +# keeps a second identical run the silent no-op convergence promises. +# shellcheck disable=SC2016 +check "users apply: the gate counts before it warns" 0 "" \ + grep -qF 'AT_RISK=$((AT_RISK + 1))' "$ROOT/commands/users-apply.sh" +# shellcheck disable=SC2016 +check "users apply: already-revoked ledger entries are not at risk" 0 "" \ + grep -qF '[ "${pstate:-active}" != "revoked" ] || continue' "$ROOT/commands/users-apply.sh" +# shellcheck disable=SC2016 +count_at="$(grep -nF 'AT_RISK=$((AT_RISK + 1))' "$ROOT/commands/users-apply.sh" | head -n1 | cut -d: -f1)" +warn_at="$(grep -nF 'this users file names ZERO users' "$ROOT/commands/users-apply.sh" | head -n1 | cut -d: -f1)" +check "users apply: the count is taken before the message quotes it" \ + 0 "" test "${count_at:-999999}" -lt "${warn_at:-0}" +# No terminal and no consent is a REFUSAL, not an assumed yes and not a hang. +check "users apply: no TTY and no consent exits 2" 0 "" \ + grep -qF 'refusing to revoke every managed operator without --yes' \ + "$ROOT/commands/users-apply.sh" +check "users apply: the no-TTY refusal names RIG_YES as the other yes" 0 "" \ + grep -qF 'no terminal to confirm on; RIG_YES=1 also means yes' \ + "$ROOT/commands/users-apply.sh" +# EOF-safe read (#68's bug class): an unguarded `read -r reply` aborts under +# `set -e` instead of taking the safe default. The || is the whole fix. +check "users apply: the confirm read survives EOF" 0 "" \ + grep -qF 'read -r reply || reply=""' "$ROOT/commands/users-apply.sh" +check "users apply: no unguarded read in the gate" 1 "" \ + grep -nE '^[[:space:]]*read -r reply$' "$ROOT/commands/users-apply.sh" +# The gate must sit BEFORE the revocation loop — a confirmation asked after +# the first account is expired is not a confirmation. Line numbers, defaults +# fail closed, same idiom as the visudo ordering assert. +gate_at="$(grep -nF 'this users file names ZERO users' "$ROOT/commands/users-apply.sh" | head -n1 | cut -d: -f1)" +revoke_at="$(grep -nF 'usermod -L -e 1' "$ROOT/commands/users-apply.sh" | head -n1 | cut -d: -f1)" +check "users apply: the gate precedes the revocation loop" \ + 0 "" test "${gate_at:-999999}" -lt "${revoke_at:-0}" +# Scope guard, the mirror of the #57 one above: this is a CONFIRMATION, and +# apply must not have grown bootstrap's flat refusal of an empty file. A grep +# that finds nothing is the pass — the repo's negative-law idiom. +check "users apply: an empty file is still a legal de-provisioning input (gated, not refused)" 1 "" \ + grep -nE 'names no users' "$ROOT/commands/users-apply.sh" +# The deferred half of #65: mass revocation below the empty-file bright line +# is NOT gated. The gate's only trigger is a file naming zero users, so no +# CODE line may compare a revocation count against a threshold — comments are +# stripped first, since the scope note beside the gate says the word on +# purpose. Pinned so that adding a threshold is a deliberate edit to a failing +# test rather than a silent contract change. +check "users apply: partial mass revocation stays ungated (#65 open question)" 1 "" \ + grep -nEi '^[[:space:]]*[^#[:space:]].*(threshold|RIG_REVOKE_MAX|AT_RISK[^)]*(-gt|-ge)[[:space:]]*\$)' \ + "$ROOT/commands/users-apply.sh" + +# The empty-file gate is reachable only from a caller that can answer it. The +# ONE in-tree caller of apply is bootstrap's users phase, and it refuses a +# zero-user file at pre-flight (#57) — before it ever invokes apply — so no +# in-tree path reaches the gate without a TTY. Pin both halves: if a second +# caller appears, or bootstrap's refusal goes away, this stops being true. +callers="$(grep -rlF 'users-apply.sh' "$ROOT/commands" | grep -v 'users-apply.sh$' || true)" +check "users apply: bootstrap is its only in-tree caller" 0 "" \ + test "$callers" = "$ROOT/commands/bootstrap.sh" +check "users apply: bootstrap refuses a zero-user file before invoking it" \ + 0 "" test "$(grep -nF 'names no users' "$ROOT/commands/bootstrap.sh" | head -n1 | cut -d: -f1)" \ + -lt "${users_apply_at:-0}" + # Validate-then-apply: `visudo -c` must pass before anything lands in # /etc/sudoers.d — a bad drop-in takes down ALL of sudo, locking every admin # out of the escalation path apply just granted. Assert the order in the file,