fix(close-root): the gate judges AllowGroups/DenyGroups too — same door, other hinge

Round-2 convergence (codex + claude-bot): sshd enforces the group
directives against the candidate's ACTUAL membership, and the gate read
only the *Users pair — an admin outside 'AllowGroups sudo' still
reached ADMIN_OK=1, and root closed on a false proof. The gate now
resolves id -Gn and judges both group directives with the *Users
discipline: DenyGroups flags on a held-group literal or ANY
pattern/host-qualified token; AllowGroups, when set, passes only on a
literal token naming a held group (a pattern that would admit proves
nothing — over-refusing stays the safe error). id failing yields no
groups, which makes a set AllowGroups flag: fail closed there too.

Both requested regressions ride the sourced lib (unmet AllowGroups,
DenyGroups naming a held group) plus the pattern/pass cases, and grep
guards pin the shipped gate to the verdicts and to real membership.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

README.md

@@ -631,8 +631,10 @@ written, and `sshd -T -C user=<admin>` must resolve a per-user effective
 config that accepts the login (`pubkeyauthentication yes`, no `DenyUsers`
 hit — where any pattern or `USER@HOST` entry counts as a hit, fail closed,
 since `DenyUsers dan*` really denies admin `dan` and rig will not re-implement
-sshd's pattern engine to prove a miss — and `AllowUsers`, if set, names them
+sshd's pattern engine to prove a miss — `AllowUsers`, if set, names them
-literally), so a `Match` block elsewhere cannot quietly exclude the admin
+literally, and the same fail-closed pair for `DenyGroups`/`AllowGroups`
+judged against the admin's actual groups), so a `Match` block elsewhere
+cannot quietly exclude the admin
 while every file looks right. The refusal names which check failed, per
 candidate. What no local check can prove: that you *hold* the private key,
 and how a `Match Address` rule treats your real client address (the probe

commands/lib/users-config.sh

@@ -176,3 +176,46 @@ deny_verdict() {
   done
   return 0
 }
+
+# group_deny_verdict <space-separated groups> <denygroups token...>
+#
+# deny_verdict's sibling for sshd's DenyGroups, judged against the
+# candidate's ACTUAL group membership (id -Gn), same fail-closed rule:
+# empty output = every token is provably irrelevant — literal and naming
+# none of the candidate's groups. A literal token naming a group they are
+# in flags, and so does any pattern or host-qualified token, because a
+# token this side of sshd cannot prove irrelevant may be the one that
+# denies. Pure text→text, sourced by the harness.
+group_deny_verdict() {
+  local groups="$1" tok g; shift
+  for tok in "$@"; do
+    case "$tok" in
+      *[*?]*) printf "sshd DenyGroups has pattern entry '%s' — cannot prove it misses this user's groups; make it literal or remove it, then re-run" "$tok"; return 0 ;;
+      *@*) printf "sshd DenyGroups has host-qualified entry '%s' — whether it bites depends on the client address, which no local check can prove; make it literal or remove it, then re-run" "$tok"; return 0 ;;
+      *) for g in $groups; do
+           if [ "$tok" = "$g" ]; then
+             printf "sshd DenyGroups names '%s' — a group this user is in" "$g"; return 0
+           fi
+         done ;;
+    esac
+  done
+  return 0
+}
+
+# group_allow_verdict <space-separated groups> <allowgroups token...>
+#
+# AllowGroups' direction: when the directive is set, sshd admits only
+# members of a matching group, so the proof must be a LITERAL token
+# naming a group the candidate is in. A pattern that would in fact admit
+# them proves nothing here (same stance as AllowUsers: over-refusing is
+# the safe error), so no literal hit → flag. Pure text→text.
+group_allow_verdict() {
+  local groups="$1" tok g; shift
+  for tok in "$@"; do
+    for g in $groups; do
+      [ "$tok" = "$g" ] && return 0
+    done
+  done
+  printf "sshd AllowGroups is set and no entry literally names a group this user is in — add their group (or them to a named group), then re-run"
+  return 0
+}

commands/users-close-root.sh

@@ -34,9 +34,11 @@ not expired), then two reachability proofs (#17) — `sudo -n true` under
 runuser must answer (NOPASSWD sudo is effective, not merely written), and
 `sshd -T -C user=...` must resolve a per-user effective config that accepts
 the login (pubkeyauthentication yes, no DenyUsers hit — where any pattern or
-host-qualified Deny entry counts as a hit, fail closed — and AllowUsers, if
+host-qualified Deny entry counts as a hit, fail closed — AllowUsers, if set,
-set, names them literally). The refusal names which check failed, per
+names them literally, and the same pair of rules for DenyGroups/AllowGroups
-candidate. Run rig users apply first; never close the only door.
+judged against the admin's actual groups from id -Gn). The refusal names
+which check failed, per candidate. Run rig users apply first; never close
+the only door.
 
 Before running, verify your admin login in a SEPARATE session — `ssh
 <admin>@<box>` while this one stays open. Root SSH is the door being welded
@@ -180,6 +182,24 @@ while IFS= read -r a; do
       deny_reason="$(deny_verdict "$a" ${deny_line#* })"
       [ -n "$deny_reason" ] && flag "$deny_reason"
     fi
+    # The group directives close the same door through the other hinge: sshd
+    # enforces Allow/DenyGroups against the candidate's ACTUAL membership, so
+    # the gate resolves id -Gn and judges both with the same fail-closed
+    # discipline as the *Users pair. id failing yields no groups, which makes
+    # a set AllowGroups flag — the safe direction.
+    a_groups="$(id -Gn -- "$a" 2>/dev/null)"
+    denyg_line="$(printf '%s\n' "$perT" | grep -i '^denygroups ' | head -n1)"
+    if [ -n "$denyg_line" ]; then
+      # shellcheck disable=SC2086  # word-splitting the tokens is the point
+      denyg_reason="$(group_deny_verdict "$a_groups" ${denyg_line#* })"
+      [ -n "$denyg_reason" ] && flag "$denyg_reason"
+    fi
+    allowg_line="$(printf '%s\n' "$perT" | grep -i '^allowgroups ' | head -n1)"
+    if [ -n "$allowg_line" ]; then
+      # shellcheck disable=SC2086  # word-splitting the tokens is the point
+      allowg_reason="$(group_allow_verdict "$a_groups" ${allowg_line#* })"
+      [ -n "$allowg_reason" ] && flag "$allowg_reason"
+    fi
     if printf '%s\n' "$perT" | grep -qi '^allowusers ' \
         && ! printf '%s\n' "$perT" | grep -i '^allowusers ' | tr ' ' '\n' | grep -qx "$a"; then
       flag "sshd AllowUsers is set and does not name this user"

test/cli.sh

@@ -588,6 +588,34 @@ check "users close-root: deny_verdict fails closed on USER@HOST forms" \
 deny_pass() { [ -z "$(deny_v "$@")" ]; } # empty verdict IS the pass
 check "users close-root: deny_verdict passes provably-irrelevant literals" \
   0 "" deny_pass admin root git backup
+# The group directives, same discipline, judged against the candidate's ACTUAL
+# membership (the review's regressions: an unmet AllowGroups, a DenyGroups
+# naming a group they hold). First arg is the id -Gn word list.
+groups_v() { # groups_v <fn> <groups> <token...>
+  bash -c 'set -euo pipefail
+    . "$1/commands/lib/users-config.sh"; shift
+    "$@"' _ "$ROOT" "$@"
+}
+groups_pass() { [ -z "$(groups_v "$@")" ]; }
+check "users close-root: DenyGroups naming a held group flags" \
+  0 "a group this user is in" groups_v group_deny_verdict "dan sudo rig-admin" backup sudo
+check "users close-root: DenyGroups fails closed on patterns" \
+  0 "pattern entry 'rig-*'" groups_v group_deny_verdict "dan rig-admin" "rig-*"
+check "users close-root: DenyGroups passes provably-irrelevant literals" \
+  0 "" groups_pass group_deny_verdict "dan rig-admin" docker backup
+check "users close-root: an unmet AllowGroups flags (fail closed)" \
+  0 "no entry literally names a group this user is in" groups_v group_allow_verdict "dan rig-admin" sudo
+check "users close-root: AllowGroups pattern is no proof (fail closed)" \
+  0 "no entry literally names" groups_v group_allow_verdict "dan rig-admin" "rig-*"
+check "users close-root: a literally-named held group passes AllowGroups" \
+  0 "" groups_pass group_allow_verdict "dan rig-admin" sudo rig-admin
+# ...and the shipped gate consults both, against real membership.
+# shellcheck disable=SC2016
+check "users close-root: the gate consults the group verdicts" 0 "" \
+  grep -qE '^[[:space:]]*denyg_reason="\$\(group_deny_verdict ' "$ROOT/commands/users-close-root.sh"
+# shellcheck disable=SC2016
+check "users close-root: the gate resolves real membership (id -Gn)" 0 "" \
+  grep -qF -- 'id -Gn -- "$a"' "$ROOT/commands/users-close-root.sh"
 # ...and the shipped gate must actually consult it (call, not comment).
 # shellcheck disable=SC2016
 check "users close-root: the gate consults deny_verdict" 0 "" \