Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 7 additions & 1 deletion internal/std/htpasswd/entry.shellf
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
15 changes: 14 additions & 1 deletion internal/std/system/timezone.shellf
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
18 changes: 14 additions & 4 deletions test/e2e/adverse-cases.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
31 changes: 31 additions & 0 deletions test/e2e/plans/adverse-htpasswd.entry-mode.shellf
Original file line number Diff line number Diff line change
@@ -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
}
}
}
23 changes: 23 additions & 0 deletions test/e2e/plans/adverse-system.timezone-recorded.shellf
Original file line number Diff line number Diff line change
@@ -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
}
}
}
Loading