diff --git a/CHANGELOG.md b/CHANGELOG.md index f70ed28..c2d8da2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixed +- `htpasswd.entry` and `system.timezone` observe everything their apply sets. The first left a credentials file world-readable whenever its hash already verified; the second left `/etc/timezone` naming another zone whenever the symlink was right. Both reported `already`. Two adverse cases, each seen red on a real machine first (#635, #636). + - A def calling another instruction with too few arguments is refused instead of binding the missing one to the empty string. `file.write(path)` inside a def overwrote the file with nothing and reported `ok.done` — a file destroyed under a success verdict. A plan-level call was always checked on both bounds; only the def side was not (#633). ## [0.13.0] - 2026-09-09 diff --git a/internal/std/htpasswd/entry.shellf b/internal/std/htpasswd/entry.shellf index cadcf5d..160f0d0 100644 --- a/internal/std/htpasswd/entry.shellf +++ b/internal/std/htpasswd/entry.shellf @@ -24,6 +24,12 @@ def entry(path: str, user: str, password: str) as root { u = shell { printf '%s' "$user" | grep -qE '^[A-Za-z0-9_.-]+$' } if !u { return err.badUser(u) } } + # Two fields, because the apply guarantees two things: the credential, and the mode that + # keeps it from being world-readable. Observing the hash alone reported `already` over a + # correct file at 644 and the `chmod` below never ran (#635) — the shape `sudo.write` and + # `sshd.config` already had, and this def did not. The mode is a constant this def chose, + # not a parameter, so it is a plain assertion: the awk numeric compare `file.mode` needs + # (#543) exists to echo back the *caller's* spelling, and there is no caller here. observe { return state(valid: shell { # The login is matched literally, the way the apply does. `grep "^$user:"` made @@ -37,7 +43,7 @@ def entry(path: str, user: str, password: str) as root { salt=$(printf '%s' "$hash" | cut -d'$' -f3) [ -n "$salt" ] || exit 1 [ "$(printf '%s' "$password" | openssl passwd -apr1 -stdin -salt "$salt")" = "$hash" ] - }.exit == 0) + }.exit == 0, secured: shell { [ "$(stat -c '%a' "$path" 2>/dev/null)" = "600" ] }.exit == 0) } apply { r = shell { diff --git a/internal/std/system/timezone.shellf b/internal/std/system/timezone.shellf index 2f14350..6e1e2b8 100644 --- a/internal/std/system/timezone.shellf +++ b/internal/std/system/timezone.shellf @@ -18,10 +18,23 @@ def timezone(zone: str) as root { z = shell { test -f "/usr/share/zoneinfo/$zone" } if !z { return err.unknownZone(z) } } + # Both files, because the apply writes both. Reading the link alone reported `already` on + # a host whose `/etc/localtime` was right and whose `/etc/timezone` named another zone — + # the exact disagreement the comment above says this def exists to prevent (#636). + # + # `recorded` is a comparison, not the file's contents: a field with no same-named + # parameter only has to be truthy, so returning `Europe/Berlin` would converge as happily + # as the right answer. `zone` is compared to the argument because a parameter carries + # that name; `recorded` cannot be, so it answers yes or no itself. + # + # On a host that ships no `/etc/timezone` the file simply is not there, the comparison + # fails, the apply creates it, and the next run converges — the def does not need to know + # whether the distribution is Debian. observe { # The link's target, not `date +%Z`: an abbreviation like CEST is seasonal and # shared by several zones, so comparing it would converge on the wrong continent. - return state(zone: shell { readlink -f /etc/localtime | sed 's#^/usr/share/zoneinfo/##' }.stdout) + return state(zone: shell { readlink -f /etc/localtime | sed 's#^/usr/share/zoneinfo/##' }.stdout, + recorded: shell { [ "$(cat /etc/timezone 2>/dev/null)" = "$zone" ] }.exit == 0) } apply { r = shell { diff --git a/test/e2e/adverse-cases.md b/test/e2e/adverse-cases.md index 11b2e2c..a1922fd 100644 --- a/test/e2e/adverse-cases.md +++ b/test/e2e/adverse-cases.md @@ -63,6 +63,15 @@ database that exists under the wrong owner, two logins where one is a regex matc other, an archive member emptied in place. Each was verified to fail before the fix and pass after — a case of this kind that was never seen red proves nothing at all, since a weak observe passes it by construction. +**And the `apply` must not run for any other reason.** A case of this kind tests the +`observe`, so the state it builds has to be one the def under test would call converged — +break anything else as well and the apply runs regardless, taking the assertion with it. +Both cases in #634 were already asserted by an existing plan and both assertions passed: +`adverse-htpasswd.entry.shellf:38` asserts mode 600 after a call that changes the password, +`adverse-system.timezone.shellf:13` asserts `/etc/timezone` after breaking the symlink too. +Neither could fail, and the defect each was written beside survived underneath it +from the day the def shipped until #634. + An argument case passes a path holding a space, a single quote and a `&`, or a name at a boundary (empty, very long, starting with a dash). It asserts the machine like any other case: the directory that exists is the one that was asked for, *whole*, and no sibling was @@ -73,10 +82,11 @@ ordinary names, which is the point. ## Coverage, and the gate that is not here yet -8 cases against 38 defs today. A CI gate requiring an adverse case per def would be 30 -named exemptions, which is not a gate — it is a file nobody re-reads, and it turns the -exemption from a signal into the norm. `def-coverage.sh` works because it is at 38/38. +28 cases over 22 defs, against 49 defs today. A CI gate requiring an adverse case per def +would be 27 named exemptions, which is not a gate — it is a file nobody re-reads, and it +turns the exemption from a signal into the norm. `def-coverage.sh` works because it is at +48/49, with its single exemption named and argued. -The gate lands when this directory covers the defs that declare an `observe` (~20) — those +The gate lands when this directory covers the defs that declare an `observe` (34) — those are the ones for which "hostile starting state" means anything; an action-shaped def (ADR-0029) has no state to get wrong. Until then the protection is #489 staying open. diff --git a/test/e2e/plans/adverse-htpasswd.entry-mode.shellf b/test/e2e/plans/adverse-htpasswd.entry-mode.shellf new file mode 100644 index 0000000..79517b8 --- /dev/null +++ b/test/e2e/plans/adverse-htpasswd.entry-mode.shellf @@ -0,0 +1,31 @@ +# `htpasswd.entry` observes the mode its own apply sets (#635). +# +# Right-shaped but wrong (adverse-cases.md): the credential in the file is **correct** — the +# stored hash verifies against the password asked for — and the file is world-readable. The +# observe answered only "does the hash verify", said yes, and the `chmod 600` at the end of +# the apply never ran. A credentials file at 644, reported converged. +# +# Distinct from `adverse-htpasswd.entry.shellf`, which also asserts the mode: there the +# password differs, so the apply runs and chmods on the way. An assertion the apply always +# satisfies cannot test an observe. Here the observe must be the thing that decides. +on target { + dir.ensure("/tmp/adv-htpasswd-mode") + as root { + unsafe shell { + # The salt is fixed so the hash is the one this password verifies against — the + # file is right, and only the mode is wrong. + printf 'covadmin:%s\n' "$(printf 'hunter2' | openssl passwd -apr1 -stdin -salt abcdefgh)" \ + > /tmp/adv-htpasswd-mode/users + chmod 644 /tmp/adv-htpasswd-mode/users + } + htpasswd.entry("/tmp/adv-htpasswd-mode/users", "covadmin", "hunter2") + shell { + [ "$(stat -c '%a' /tmp/adv-htpasswd-mode/users)" = "600" ] || exit 1 + # And the credential still verifies: the repair must not have dropped the line. + line=$(grep '^covadmin:' /tmp/adv-htpasswd-mode/users) || exit 1 + hash=${line#*:} + salt=$(printf '%s' "$hash" | cut -d'$' -f3) + [ "$(printf 'hunter2' | openssl passwd -apr1 -stdin -salt "$salt")" = "$hash" ] || exit 1 + } + } +} diff --git a/test/e2e/plans/adverse-system.timezone-recorded.shellf b/test/e2e/plans/adverse-system.timezone-recorded.shellf new file mode 100644 index 0000000..601cfd7 --- /dev/null +++ b/test/e2e/plans/adverse-system.timezone-recorded.shellf @@ -0,0 +1,23 @@ +# `system.timezone` observes both files its apply writes (#636). +# +# Right-shaped but wrong (adverse-cases.md): `/etc/localtime` already points at the wanted +# zone and `/etc/timezone` holds another one. The observe read the link alone, converged, and +# left the file — which is what `dpkg-reconfigure tzdata` reads, so the next tzdata upgrade +# undoes the def. +# +# Distinct from `adverse-system.timezone.shellf`, which breaks the link as well: there the +# apply runs whatever the second file holds, so its assertion on `/etc/timezone` passes +# without ever testing the observe. +on target { + as root { + unsafe shell { + ln -sf /usr/share/zoneinfo/Europe/Paris /etc/localtime + printf 'Europe/Berlin\n' > /etc/timezone + } + system.timezone("Europe/Paris") + shell { + grep -qx 'Europe/Paris' /etc/timezone || exit 1 + [ "$(readlink -f /etc/localtime)" = "/usr/share/zoneinfo/Europe/Paris" ] || exit 1 + } + } +}