From d25fc65ba89428233c16909249a2314f41f63485 Mon Sep 17 00:00:00 2001 From: Nicolas CHAUVIN Date: Sun, 13 Sep 2026 17:41:25 +0200 Subject: [PATCH] fix(std): match a group name literally in user.group --- CHANGELOG.md | 8 +++-- internal/std/user/user.shellf | 7 +++- .../e2e/plans/adverse-user.group-regex.shellf | 32 +++++++++++++++++++ 3 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 test/e2e/plans/adverse-user.group-regex.shellf diff --git a/CHANGELOG.md b/CHANGELOG.md index 304e152..122b556 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,12 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - ADR-0057: policy goes in `shellf.conf` at the project root, written in the shellf language; modes and inputs stay flags, and a flag outranks the file. The plan sits above both, as `as root` already does. No user or system config — on a fleet tool, a per-operator default outside the repository is drift nobody can review (#663). +### Fixed + +- `sysctl.set` observes the drop-in it writes as well as the running kernel. A host whose kernel already held the value and whose `/etc/sysctl.d` file was gone reported `already`, and the setting was lost at the next reboot. The def's own comment argued against reading the file, correctly, then concluded "the kernel only" — a false dilemma (#658). + +- `user.group` matches a group name literally. `grep -qx` read it as a regular expression, and Debian allows `.` in a group name — so a request for `a.b` was satisfied by membership of `axb`, and the def reported `already` over a user it never added. The same class as #598, which fixed it in `htpasswd.entry` (#660). + ## [0.14.0] - 2026-09-10 ### Added @@ -20,8 +26,6 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixed -- `sysctl.set` observes the drop-in it writes as well as the running kernel. A host whose kernel already held the value and whose `/etc/sysctl.d` file was gone reported `already`, and the setting was lost at the next reboot. The def's own comment argued against reading the file, correctly, then concluded "the kernel only" — a false dilemma (#658). - - Three dropped failures. A reconnecting control host no longer leaks the connection it replaces — one descriptor per run, for the agent's whole life. An agent that cannot open its channel says so at the first ask instead of timing out on `no control host attached`. A local workdir that cannot be created fails the run, naming itself (#638). - `shellf status` takes the inputs `run` takes: `--vars`, `--set`, `-v` and `--agent-ttl`. The command that answers "what would this plan see?" could not be handed what the plan sees — a plan using `${k}` from `--set` failed to resolve under `status` while applying cleanly under `run`. Both now register one shared flag definition (#640). diff --git a/internal/std/user/user.shellf b/internal/std/user/user.shellf index 5cf8986..4c5a8b4 100644 --- a/internal/std/user/user.shellf +++ b/internal/std/user/user.shellf @@ -53,7 +53,12 @@ def ensure(name: str, shell: str, system: bool = false) as root { # the next step either, and the second contradicts the resident model that makes a second run # cost no round trip (#510). def group(user: str, group: str) as root { - observe { return state(member: shell { id -nG "$user" | tr ' ' '\n' | grep -qx "$group" }.exit == 0) } + # -F and --: a group name is matched literally. `grep -qx "$group"` read it as a regular + # expression, and Debian allows `.` in a group name — so a request for `a.b` was satisfied by + # membership of `axb`, and this def reported `already` over a user it never added (#660). The + # same class as #598, which fixed it in `htpasswd.entry`; the shape survived here. `--` for + # the reason #639 needed it: a name starting with a dash is otherwise read as options. + observe { return state(member: shell { id -nG "$user" | tr ' ' '\n' | grep -qxF -- "$group" }.exit == 0) } apply { r = shell { usermod -aG "$group" "$user" } if !r { return err.runtime(r) } diff --git a/test/e2e/plans/adverse-user.group-regex.shellf b/test/e2e/plans/adverse-user.group-regex.shellf new file mode 100644 index 0000000..695db18 --- /dev/null +++ b/test/e2e/plans/adverse-user.group-regex.shellf @@ -0,0 +1,32 @@ +# `user.group` matches the group name literally (#660). +# +# Right-shaped but wrong (adverse-cases.md): the user is in a group whose name the requested one +# **matches as a regex**. `grep -qx "$group"` read `advg.b` as a pattern, the `.` matched the `x` +# of `advgxb`, and the def reported `already` over a user who was never added to the group asked +# for. +# +# The same class as #598, which fixed it in `htpasswd.entry` where `a.b` read another account's +# hash. The shape survived here. +# +# Debian allows `.` in a group name, so nothing exotic is required — which is the point. +on target { + as root { + unsafe shell { + getent group advgxb >/dev/null || groupadd advgxb + getent group advg.b >/dev/null || groupadd advg.b + id advgrpuser >/dev/null 2>&1 || useradd -M -s /bin/false advgrpuser + # In the look-alike group only: the state the observe must not accept. + usermod -G advgxb advgrpuser + # -F here too. Written without it, this guard reproduces the very bug under test — + # `advg.b` matches `advgxb` — and fails the setup instead of the def. Measured. + id -nG advgrpuser | tr ' ' '\n' | grep -qxF -- 'advg.b' && exit 1 + exit 0 + } + user.group("advgrpuser", "advg.b") + shell { + id -nG advgrpuser | tr ' ' '\n' | grep -qxF -- 'advg.b' || exit 1 + # And the group it was already in is still there: `usermod -aG` appends. + id -nG advgrpuser | tr ' ' '\n' | grep -qxF -- 'advgxb' || exit 1 + } + } +}