From 7377f0594f22a331d58bcb3a3ba006d9d79fe553 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcos=20M=C3=A9ndez?= Date: Mon, 28 Sep 2026 02:58:36 +0000 Subject: [PATCH 1/6] feat: keel-wp and keel-wordpress-update, with the turnkey names as symlinks The two commands an operator types on this appliance are written by this overlay, so the name they carry is this project's choice and nobody else's. The Keel name becomes the real command and the turnkey-* name is kept as a symlink to it, which is the general policy of decision 0015 of the handbook (tracker#12). Both names keep working: somebody who pasted a command out of TurnKey's documentation, or who wrote a script against turnkey-wp last year, must not silently lose it. The links are committed into the overlay rather than made in conf.d/main. fab-apply-overlay copies an overlay with "cp -TdR" and -d keeps a link a link; this recipe applies its overlay twice, once through COMMON_OVERLAYS and once as the product-local ROOT_OVERLAY, and a second copy over an existing link still leaves a link. tests/wrappers.bats asserts both by making that copy with the same command the build uses and then running the copied turnkey-wp, because a link that resolves is not a link that works. What either command does is unchanged. keel-wp runs wp-cli through runuser as www-data, never as root, with an explicit --path, and owns /var/www/.wp-cli. keel-wordpress-update refuses a caller who is not root, updates core, verifies WordPress's own per file checksums and puts the ownership boundary back; its refusal now names whichever of its two names was typed. The paths each script hands to another program are overridable from the environment, as keel-wp's WP_DIR, WP_USR and WP_CACHE already were, with the hard coded values as the defaults, so a booted appliance behaves identically and a test can drive the pair against a scratch tree. That is what makes them measurable: both were at no coverage at all and are now at 100 percent of their lines, 9/9 and 18/18, with every branch driven. --- overlay/usr/local/bin/keel-wp | 23 ++ overlay/usr/local/bin/turnkey-wp | 16 +- overlay/usr/local/sbin/keel-wordpress-update | 34 +++ .../usr/local/sbin/turnkey-wordpress-update | 25 +- tests/coverage.sh | 7 +- tests/wrappers.bats | 271 ++++++++++++++++++ 6 files changed, 334 insertions(+), 42 deletions(-) create mode 100755 overlay/usr/local/bin/keel-wp mode change 100755 => 120000 overlay/usr/local/bin/turnkey-wp create mode 100755 overlay/usr/local/sbin/keel-wordpress-update mode change 100755 => 120000 overlay/usr/local/sbin/turnkey-wordpress-update create mode 100644 tests/wrappers.bats diff --git a/overlay/usr/local/bin/keel-wp b/overlay/usr/local/bin/keel-wp new file mode 100755 index 0000000..66383a4 --- /dev/null +++ b/overlay/usr/local/bin/keel-wp @@ -0,0 +1,23 @@ +#!/bin/bash -e +# wp-cli against this appliance's WordPress, run as the web server's own user +# and never as root, with the path given explicitly so the command works from +# any directory. +# +# The Keel name is the real command; `turnkey-wp` beside it is a symlink to +# this file (decision 0015 of the handbook), so a script written against the +# old name, or a command pasted out of TurnKey's documentation, keeps working. + +[[ -z "$DEBUG" ]] || set -x + +WP_DIR=${WP_DIR:-/var/www/wordpress} +WP_USR=${WP_USR:-www-data} +WP_CACHE=${WP_CACHE:-/var/www/.wp-cli} +WP_CLI=${WP_CLI:-/usr/local/bin/wp} + +if [[ ! -d "$WP_CACHE" ]]; then + mkdir -p "$WP_CACHE" +fi +chown -R "$WP_USR":"$WP_USR" "$WP_CACHE" + +runuser "$WP_USR" -s /bin/bash \ + -c "$WP_CLI --path='$WP_DIR' $(printf '%q ' "$@")" diff --git a/overlay/usr/local/bin/turnkey-wp b/overlay/usr/local/bin/turnkey-wp deleted file mode 100755 index 1b3bfd1..0000000 --- a/overlay/usr/local/bin/turnkey-wp +++ /dev/null @@ -1,15 +0,0 @@ -#!/bin/bash -e - -[[ -z "$DEBUG" ]] || set -x - -WP_DIR=${WP_DIR:-/var/www/wordpress} -WP_USR=${WP_USR:-www-data} -WP_CACHE=${WP_CACHE:-/var/www/.wp-cli} - -if [[ ! -d "$WP_CACHE" ]]; then - mkdir -p "$WP_CACHE" -fi -chown -R "$WP_USR":"$WP_USR" "$WP_CACHE" - -runuser "$WP_USR" -s /bin/bash \ - -c "/usr/local/bin/wp --path='$WP_DIR' $(printf '%q ' "$@")" diff --git a/overlay/usr/local/bin/turnkey-wp b/overlay/usr/local/bin/turnkey-wp new file mode 120000 index 0000000..57f8fe9 --- /dev/null +++ b/overlay/usr/local/bin/turnkey-wp @@ -0,0 +1 @@ +keel-wp \ No newline at end of file diff --git a/overlay/usr/local/sbin/keel-wordpress-update b/overlay/usr/local/sbin/keel-wordpress-update new file mode 100755 index 0000000..f0a7441 --- /dev/null +++ b/overlay/usr/local/sbin/keel-wordpress-update @@ -0,0 +1,34 @@ +#!/bin/bash +# A supervised WordPress core update: update core, verify WordPress's own per +# file checksums, and put the appliance's ownership boundary back afterwards +# (README.rst). Automatic core updates are off in wp-config.php because an +# unsupervised update does not restore that boundary. +# +# The Keel name is the real command; `turnkey-wordpress-update` beside it is a +# symlink to this file (decision 0015 of the handbook). The refusal below names +# whichever of the two the operator typed. +set -euo pipefail + +WPROOT="${WPROOT:-/var/www/wordpress}" +WP_USER="${WP_USER:-www-data}" +WP_CLI="${WP_CLI:-/usr/local/bin/wp}" +me=${0##*/} + +test "$(id -u)" -eq 0 || { + echo "$me must run as root" >&2 + exit 1 +} + +"$WP_CLI" --allow-root --path="$WPROOT" core update +"$WP_CLI" --allow-root --path="$WPROOT" core verify-checksums + +chown -R root:root "$WPROOT" +find "$WPROOT" -type d -exec chmod 0755 {} + +find "$WPROOT" -type f -exec chmod 0644 {} + +for runtime_dir in uploads cache upgrade plugins themes; do + install -d -o "$WP_USER" -g "$WP_USER" -m 0755 \ + "$WPROOT/wp-content/$runtime_dir" + chown -R "$WP_USER:$WP_USER" "$WPROOT/wp-content/$runtime_dir" +done +chown root:"$WP_USER" "$WPROOT/wp-config.php" +chmod 0640 "$WPROOT/wp-config.php" diff --git a/overlay/usr/local/sbin/turnkey-wordpress-update b/overlay/usr/local/sbin/turnkey-wordpress-update deleted file mode 100755 index 5ca0167..0000000 --- a/overlay/usr/local/sbin/turnkey-wordpress-update +++ /dev/null @@ -1,24 +0,0 @@ -#!/bin/bash -set -euo pipefail - -WPROOT=/var/www/wordpress -USER=www-data - -test "$(id -u)" -eq 0 || { - echo "turnkey-wordpress-update must run as root" >&2 - exit 1 -} - -/usr/local/bin/wp --allow-root --path="$WPROOT" core update -/usr/local/bin/wp --allow-root --path="$WPROOT" core verify-checksums - -chown -R root:root "$WPROOT" -find "$WPROOT" -type d -exec chmod 0755 {} + -find "$WPROOT" -type f -exec chmod 0644 {} + -for runtime_dir in uploads cache upgrade plugins themes; do - install -d -o "$USER" -g "$USER" -m 0755 \ - "$WPROOT/wp-content/$runtime_dir" - chown -R "$USER:$USER" "$WPROOT/wp-content/$runtime_dir" -done -chown root:"$USER" "$WPROOT/wp-config.php" -chmod 0640 "$WPROOT/wp-config.php" diff --git a/overlay/usr/local/sbin/turnkey-wordpress-update b/overlay/usr/local/sbin/turnkey-wordpress-update new file mode 120000 index 0000000..b64b729 --- /dev/null +++ b/overlay/usr/local/sbin/turnkey-wordpress-update @@ -0,0 +1 @@ +keel-wordpress-update \ No newline at end of file diff --git a/tests/coverage.sh b/tests/coverage.sh index 968b967..3b94271 100755 --- a/tests/coverage.sh +++ b/tests/coverage.sh @@ -1,8 +1,9 @@ #!/bin/bash # Line coverage of the shell this project writes, measured with kcov over # the bats suite (decision 0004). The measured files are the first boot -# library, the first boot hook itself, the logic of the boot test, the build -# time archive checks and the script that enables the signed archive; the bar of +# library, the first boot hook itself, the two operator commands the overlay +# ships, the logic of the boot test, the build time archive checks and the +# script that enables the signed archive; the bar of # decision 0003 applies to all of them. Exits 1 below the threshold, 2 when a # tool is missing. tests/boot-test.sh is the thin main that runs keel and LXC # as root and is exercised by the container run in test-appliance.yml, not @@ -24,7 +25,7 @@ done report="${COVERAGE_DIR:-$(mktemp -d)}" # The include pattern is the whitelist, so no exclude pattern is needed; an # exclude of /tests/ would drop tests/lib/boot-test-lib.sh with it. -kcov --include-pattern=/lib/wordpress.sh,/firstboot.d/40wordpress,/tests/lib/boot-test-lib.sh,/bin/keel-archive-check,/conf.d/zz-project-packages,/conf.d/zzz-keel-archive \ +kcov --include-pattern=/lib/wordpress.sh,/firstboot.d/40wordpress,/overlay/usr/local/bin/keel-wp,/overlay/usr/local/sbin/keel-wordpress-update,/tests/lib/boot-test-lib.sh,/bin/keel-archive-check,/conf.d/zz-project-packages,/conf.d/zzz-keel-archive \ "$report" bats "$here" json="$(find "$report" -mindepth 2 -maxdepth 2 -name coverage.json -not -path "*/kcov-merged/*" | head -1)" diff --git a/tests/wrappers.bats b/tests/wrappers.bats new file mode 100644 index 0000000..e3f28ad --- /dev/null +++ b/tests/wrappers.bats @@ -0,0 +1,271 @@ +#!/usr/bin/env bats +# Unit tests of the two operator commands this appliance writes itself: +# +# overlay/usr/local/bin/keel-wp wp-cli as the web user +# overlay/usr/local/sbin/keel-wordpress-update the supervised core update +# +# and of the compatibility names beside them, `turnkey-wp` and +# `turnkey-wordpress-update`, which are symlinks to those two (decision 0015). +# +# Both scripts are executed for real against scratch trees. What they hand to +# another program is a stub first in PATH that writes a line per call into a +# log the tests read: `runuser`, `chown`, `install` and `id`. wp-cli is a stub +# named by WP_CLI. No test needs root, a web server, a database or a network. +# +# The symlink is not asserted with `test -L`. A link that resolves is not a +# link that works, and docs/traps.md of the handbook records "asserting a +# configuration value is not asserting the behaviour it was supposed to +# produce" as a recurring defect here. So the compatibility name is *run*, and +# it is run once through a copy of the overlay made the way the build makes it +# (`cp -TdR`, which is what fab-apply-overlay executes). + +setup() { + # Before PATH is bent: `id` becomes a stub below, and these two want the + # real one. + ME="$(id -un)" + + REPO="$BATS_TEST_DIRNAME/.." + BIN="$REPO/overlay/usr/local/bin" + SBIN="$REPO/overlay/usr/local/sbin" + WP="$BIN/keel-wp" + UPDATE="$SBIN/keel-wordpress-update" + + S="$BATS_TEST_TMPDIR" + CALLS="$S/calls" + : > "$CALLS" + + WPROOT="$S/wordpress" + mkdir -p "$WPROOT/wp-content" + : > "$WPROOT/wp-config.php" + : > "$WPROOT/index.php" + + STUBS="$S/bin" + mkdir -p "$STUBS" + _stub runuser + _stub chown + _stub install + # id: root unless a test says otherwise, so the updater's guard can be + # driven both ways without being root. + cat > "$STUBS/id" <> "$CALLS" +echo "\${KEEL_TEST_UID:-0}" +EOF + chmod +x "$STUBS/id" + # wp-cli: logs, and fails the one subcommand a test names. + WP_CLI="$STUBS/wp" + cat > "$WP_CLI" <> "$CALLS" +for arg in "\$@"; do + case "\$arg" in --allow-root|--path=*) continue ;; esac + first=\$arg; break +done +[ "\${KEEL_TEST_WP_FAIL:-}" = "\$*" ] && exit "\${KEEL_TEST_WP_CODE:-1}" +exit 0 +EOF + chmod +x "$WP_CLI" + + PATH="$STUBS:$PATH" + export PATH WP_CLI + export WP_DIR="$WPROOT" WPROOT + export WP_USR="$ME" WP_USER="$ME" + export WP_CACHE="$S/wp-cli-cache" +} + +_stub() { + cat > "$STUBS/$1" <> "$CALLS" +exit \${KEEL_TEST_${1^^}_CODE:-0} +EOF + chmod +x "$STUBS/$1" +} + +# _runuser_command: the string keel-wp handed to runuser, which is the whole +# of what it asked the web user to run. +_runuser_command() { + sed -n 's/^runuser [^ ]* -s [^ ]* -c //p' "$CALLS" +} + +# --- keel-wp: the wrapper an operator types ---------------------------------- + +@test "keel-wp is the real command and turnkey-wp is a relative symlink to it" { + [ -f "$WP" ] && [ ! -L "$WP" ] + [ -x "$WP" ] + [ -L "$BIN/turnkey-wp" ] + [ "$(readlink "$BIN/turnkey-wp")" = keel-wp ] +} + +@test "keel-wordpress-update is the real command and the turnkey name links to it" { + [ -f "$UPDATE" ] && [ ! -L "$UPDATE" ] + [ -x "$UPDATE" ] + [ -L "$SBIN/turnkey-wordpress-update" ] + [ "$(readlink "$SBIN/turnkey-wordpress-update")" = keel-wordpress-update ] +} + +@test "keel-wp runs wp-cli as the web user, with an explicit path, never as root" { + run "$WP" option get siteurl + [ "$status" -eq 0 ] + grep -q "^runuser $WP_USR -s /bin/bash -c " "$CALLS" + [[ "$(_runuser_command)" == *"--path='$WPROOT'"* ]] + [[ "$(_runuser_command)" != *--allow-root* ]] +} + +@test "keel-wp passes its arguments through to wp-cli" { + run "$WP" option get siteurl + [ "$status" -eq 0 ] + [[ "$(_runuser_command)" == *"option get siteurl"* ]] +} + +@test "keel-wp quotes an argument that carries a space" { + run "$WP" post create --post_title='TurnKey v19 acceptance' --porcelain + [ "$status" -eq 0 ] + [[ "$(_runuser_command)" == *"--post_title=TurnKey\\ v19\\ acceptance"* ]] +} + +@test "keel-wp quotes an argument that would otherwise end the command" { + run "$WP" eval "echo 'x'; rm -rf /" + [ "$status" -eq 0 ] + [[ "$(_runuser_command)" != *"; rm -rf /"* ]] +} + +@test "keel-wp creates the wp-cli cache directory when it is not there" { + [ ! -d "$WP_CACHE" ] + run "$WP" core version + [ "$status" -eq 0 ] + [ -d "$WP_CACHE" ] + grep -q "^chown -R $WP_USR:$WP_USR $WP_CACHE\$" "$CALLS" +} + +@test "keel-wp leaves an existing cache directory alone and still owns it" { + mkdir -p "$WP_CACHE" + : > "$WP_CACHE/already-here" + run "$WP" core version + [ "$status" -eq 0 ] + [ -f "$WP_CACHE/already-here" ] + grep -q "^chown -R $WP_USR:$WP_USR $WP_CACHE\$" "$CALLS" +} + +@test "keel-wp reports the exit code wp-cli gave it" { + KEEL_TEST_RUNUSER_CODE=3 run "$WP" core verify-checksums + [ "$status" -eq 3 ] +} + +@test "keel-wp fails when the cache cannot be owned" { + KEEL_TEST_CHOWN_CODE=1 run "$WP" core version + [ "$status" -eq 1 ] + grep -q '^chown ' "$CALLS" + [ -z "$(_runuser_command)" ] +} + +@test "DEBUG makes keel-wp trace the command it runs" { + # kcov measures bash by turning xtrace on itself: it points BASH_ENV at a + # helper that sets its own PS4 and BASH_XTRACEFD, so under coverage the + # script's own trace goes to kcov's reader and never to this test. This + # one run is therefore left uninstrumented, with the ordinary prefix and + # stderr to trace to. Every line it executes is covered by the tests + # above, so nothing is lost from the measurement. + run env -u BASH_ENV DEBUG=1 PS4='+ ' BASH_XTRACEFD=2 "$WP" core version + [ "$status" -eq 0 ] + [[ "$output" == *"+ runuser"* ]] +} + +# --- the compatibility name, run rather than inspected ----------------------- + +@test "turnkey-wp runs the same command keel-wp does" { + [ -L "$BIN/turnkey-wp" ] + run "$BIN/turnkey-wp" option get siteurl + [ "$status" -eq 0 ] + grep -q "^runuser $WP_USR -s /bin/bash -c " "$CALLS" + [[ "$(_runuser_command)" == *"--path='$WPROOT'"* ]] + [[ "$(_runuser_command)" == *"option get siteurl"* ]] +} + +@test "the overlay copy the build makes keeps turnkey-wp a working link" { + # cp -TdR is exactly what fab-apply-overlay runs (cmd_apply_overlay in + # fab), so this is the build's own copy step and not an imitation of it. + # Twice, because the Makefile applies this overlay twice: once through + # COMMON_OVERLAYS and once as the product-local ROOT_OVERLAY. A second + # copy over an existing link must leave a link and not a copy of its + # target. + root="$S/root.patched" + mkdir -p "$root" + cp -TdR "$REPO/overlay" "$root" + cp -TdR "$REPO/overlay" "$root" + [ -L "$root/usr/local/bin/turnkey-wp" ] + [ "$(readlink "$root/usr/local/bin/turnkey-wp")" = keel-wp ] + [ -x "$root/usr/local/bin/keel-wp" ] + run "$root/usr/local/bin/turnkey-wp" option get siteurl + [ "$status" -eq 0 ] + [[ "$(_runuser_command)" == *"option get siteurl"* ]] +} + +@test "the overlay copy keeps turnkey-wordpress-update a working link" { + root="$S/root.patched" + mkdir -p "$root" + cp -TdR "$REPO/overlay" "$root" + [ -L "$root/usr/local/sbin/turnkey-wordpress-update" ] + run "$root/usr/local/sbin/turnkey-wordpress-update" + [ "$status" -eq 0 ] + grep -q "^wp --allow-root --path=$WPROOT core update\$" "$CALLS" +} + +@test "turnkey-wordpress-update refuses a non-root caller under its own name" { + [ -L "$SBIN/turnkey-wordpress-update" ] + KEEL_TEST_UID=1000 run "$SBIN/turnkey-wordpress-update" + [ "$status" -eq 1 ] + [[ "$output" == *"turnkey-wordpress-update must run as root"* ]] +} + +# --- keel-wordpress-update: the supervised core update ----------------------- + +@test "keel-wordpress-update refuses a caller who is not root" { + KEEL_TEST_UID=1000 run "$UPDATE" + [ "$status" -eq 1 ] + [[ "$output" == *"keel-wordpress-update must run as root"* ]] + [ ! -s "$CALLS" ] || ! grep -q '^wp ' "$CALLS" +} + +@test "keel-wordpress-update updates core and then verifies the checksums" { + run "$UPDATE" + [ "$status" -eq 0 ] + grep -q "^wp --allow-root --path=$WPROOT core update\$" "$CALLS" + grep -q "^wp --allow-root --path=$WPROOT core verify-checksums\$" "$CALLS" +} + +@test "keel-wordpress-update stops when the update fails" { + KEEL_TEST_WP_FAIL="--allow-root --path=$WPROOT core update" \ + run "$UPDATE" + [ "$status" -eq 1 ] + grep -q "^wp --allow-root --path=$WPROOT core update\$" "$CALLS" + ! grep -q 'verify-checksums' "$CALLS" +} + +@test "keel-wordpress-update stops when the checksums do not verify" { + KEEL_TEST_WP_FAIL="--allow-root --path=$WPROOT core verify-checksums" \ + run "$UPDATE" + [ "$status" -eq 1 ] + grep -q "^wp --allow-root --path=$WPROOT core verify-checksums\$" "$CALLS" + ! grep -q '^chown -R root:root' "$CALLS" +} + +@test "keel-wordpress-update puts the ownership boundary back" { + run "$UPDATE" + [ "$status" -eq 0 ] + grep -q "^chown -R root:root $WPROOT\$" "$CALLS" + grep -q "^chown root:$WP_USER $WPROOT/wp-config.php\$" "$CALLS" + [ "$(stat -c %a "$WPROOT/wp-config.php")" = 640 ] + [ "$(stat -c %a "$WPROOT/index.php")" = 644 ] + [ "$(stat -c %a "$WPROOT")" = 755 ] +} + +@test "keel-wordpress-update leaves the five runtime directories to the web server" { + run "$UPDATE" + [ "$status" -eq 0 ] + for dir in uploads cache upgrade plugins themes; do + grep -q "^install -d -o $WP_USER -g $WP_USER -m 0755 $WPROOT/wp-content/$dir\$" \ + "$CALLS" + grep -q "^chown -R $WP_USER:$WP_USER $WPROOT/wp-content/$dir\$" "$CALLS" + done +} From b3666e40f8705e14e2cd6f76c9a0677509c5cd7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcos=20M=C3=A9ndez?= Date: Mon, 28 Sep 2026 02:58:46 +0000 Subject: [PATCH 2/6] refactor: every call site asks for the Keel name A caller left on the old name still works through the symlink, which is exactly why a call site is easy to miss: the build would have passed either way. So they were found by grep rather than by the gate. conf.d/main verifies WordPress's per file checksums with keel-wp during the build, and its comment about who uses wp-cli afterwards says keel-wp too. README.rst documents the supervised core update as keel-wordpress-update in both places it names it. tests/v19.sh, the release acceptance flow, drives keel-wp through siteurl, post create, get and delete, core version, check-update and verify-checksums, and runs the updater as keel-wordpress-update. tests/v19.sh also asks the compatibility names for the same answers on a real appliance: turnkey-wp has to report the same core version and the same siteurl keel-wp just did, and the updater's root guard has to hold under turnkey-wordpress-update as well. Reading the link would have proved neither. firstboot.d/40wordpress is not a call site: it calls wp-cli directly with --allow-root, not through either wrapper. --- README.rst | 4 ++-- conf.d/main | 4 ++-- tests/README.md | 10 ++++++++++ tests/v19.sh | 41 ++++++++++++++++++++++++++++------------- 4 files changed, 42 insertions(+), 17 deletions(-) diff --git a/README.rst b/README.rst index 097c78c..4fe3370 100644 --- a/README.rst +++ b/README.rst @@ -22,7 +22,7 @@ and on top of that: directories intentionally contain web-writable executable code; install only updates and extensions you trust. - WordPress core is root-owned and does not update automatically. Apply a - supervised core update as ``root`` with ``turnkey-wordpress-update``. + supervised core update as ``root`` with ``keel-wordpress-update``. The command verifies official WordPress core checksums and restores the appliance ownership boundary after updating. @@ -170,7 +170,7 @@ it either: it is unpacked from a release archive pinned by digest in checksums. WordPress core therefore updates through WordPress's own mechanism, either from the dashboard or, preferably on this appliance, with:: - turnkey-wordpress-update + keel-wordpress-update which runs ``wp core update``, verifies WordPress's checksums again and puts the ownership boundary back: core and ``wp-config.php`` root owned, and only diff --git a/conf.d/main b/conf.d/main index e91f3ea..f55cb11 100755 --- a/conf.d/main +++ b/conf.d/main @@ -59,7 +59,7 @@ sed -i "s|^allow_url_fopen.*|allow_url_fopen = On|" /etc/php/?.?/apache2/php.ini grep -q '^allow_url_fopen = On$' /etc/php/?.?/apache2/php.ini # wp-cli, pinned and verified. The first boot hook is the only thing that uses -# it unattended, and a person uses it afterwards through turnkey-wp. +# it unattended, and a person uses it afterwards through keel-wp. curl --proto '=https' --tlsv1.2 -fsSL \ "https://github.com/wp-cli/wp-cli/releases/download/v${WP_CLI_VERSION}/wp-cli-${WP_CLI_VERSION}.phar" \ -o /usr/local/bin/wp @@ -91,7 +91,7 @@ chown -R "$WEB_USER:$WEB_USER" "$WPROOT" # WordPress's own per file checksums, asked of WordPress. This is the second, # independent check on the same bytes: the digest above says the archive is the # one we pinned, this says the files in it are the ones WordPress published. -turnkey-wp core verify-checksums --version="$WP_VERSION" +keel-wp core verify-checksums --version="$WP_VERSION" # No wp-config.php, and none is written here. Assert it, because the whole # point of this recipe is that the layer carries no credential. diff --git a/tests/README.md b/tests/README.md index 244000d..8f1cb43 100644 --- a/tests/README.md +++ b/tests/README.md @@ -30,6 +30,15 @@ the spec. as `PATH` stubs that log every call, and a scratch `INITHOOKS_PATH` whose `lib` is a symlink to the real library, so kcov measures the file the appliance ships. +- `wrappers.bats`: unit tests of the two operator commands this overlay + writes, `overlay/usr/local/bin/keel-wp` and + `overlay/usr/local/sbin/keel-wordpress-update`, and of the `turnkey-wp` and + `turnkey-wordpress-update` symlinks beside them (decision 0015 of the + handbook). `runuser`, `chown`, `install` and `id` are `PATH` stubs that log + every call, and wp-cli is a stub named by `WP_CLI`, so no test needs root, a + web server or a network. The compatibility names are run rather than + inspected, once through a copy of the overlay made with `cp -TdR`, which is + what `fab-apply-overlay` executes. - `keel-archive.bats`: unit tests of `conf.d/zzz-keel-archive`, the last conf script, which enables the project's signed APT archive and refuses to unless the key is in the image and the build time source is gone. @@ -51,6 +60,7 @@ Debian packages `bats` (1.11) and `kcov` (43); no root: bats tests/wordpress.bats bats tests/hook.bats + bats tests/wrappers.bats bats tests/boot-test.bats bats tests/keel-archive.bats COVERAGE_THRESHOLD=95 tests/coverage.sh diff --git a/tests/v19.sh b/tests/v19.sh index 42a3692..4069f69 100755 --- a/tests/v19.sh +++ b/tests/v19.sh @@ -19,11 +19,11 @@ if "$hook" --pass="$password" --email=admin@example.com \ exit 1 fi "$hook" --pass="$password" --email=admin@example.com --domain=localhost -test "$(turnkey-wp option get siteurl)" = https://localhost +test "$(keel-wp option get siteurl)" = https://localhost "$hook" --pass="$password" --email=admin@example.com --domain=http://localhost -test "$(turnkey-wp option get siteurl)" = http://localhost +test "$(keel-wp option get siteurl)" = http://localhost "$hook" --pass="$password" --email=admin@example.com --domain=https://localhost -test "$(turnkey-wp option get siteurl)" = https://localhost +test "$(keel-wp option get siteurl)" = https://localhost # Authenticate as the provisioned administrator. login_url=$base/wp-login.php @@ -37,21 +37,36 @@ curl "${curl_args[@]}" -L -b "$work/cookies" -c "$work/cookies" \ grep -Eq 'Dashboard|wp-admin-bar' "$work/dashboard.html" # Create meaningful state, then prove it and the authenticated session survive restart. -post_id=$(turnkey-wp post create --post_status=publish \ +post_id=$(keel-wp post create --post_status=publish \ --post_title='TurnKey v19 acceptance' \ --post_content='qa-wordpress-persistence' --porcelain) test -n "$post_id" -test "$(turnkey-wp post get "$post_id" --field=post_content)" = \ +test "$(keel-wp post get "$post_id" --field=post_content)" = \ qa-wordpress-persistence -before=$(turnkey-wp core version) -turnkey-wp core check-update --format=json >"$work/update.json" +before=$(keel-wp core version) +keel-wp core check-update --format=json >"$work/update.json" python3 -c 'import json,sys; assert isinstance(json.load(open(sys.argv[1])), list)' \ "$work/update.json" +test "$(keel-wp core version)" = "$before" +keel-wp core verify-checksums +if runuser -u www-data -- /usr/local/sbin/keel-wordpress-update; then + echo 'non-root WordPress updater unexpectedly succeeded' >&2 + exit 1 +fi + +# The compatibility names of decision 0015, exercised rather than inspected: a +# link that resolves is not a link that works. turnkey-wp is asked the same +# question keel-wp was just asked and has to give the same answer, and the +# updater's root guard has to hold under the old name too. +test -L /usr/local/bin/turnkey-wp +test "$(readlink /usr/local/bin/turnkey-wp)" = keel-wp test "$(turnkey-wp core version)" = "$before" -turnkey-wp core verify-checksums +test "$(turnkey-wp option get siteurl)" = "$(keel-wp option get siteurl)" +test -L /usr/local/sbin/turnkey-wordpress-update +test "$(readlink /usr/local/sbin/turnkey-wordpress-update)" = keel-wordpress-update if runuser -u www-data -- /usr/local/sbin/turnkey-wordpress-update; then - echo 'non-root WordPress updater unexpectedly succeeded' >&2 + echo 'non-root WordPress updater unexpectedly succeeded under the compatibility name' >&2 exit 1 fi @@ -67,14 +82,14 @@ done systemctl restart mariadb.service apache2.service systemctl --quiet is-active mariadb.service apache2.service -test "$(turnkey-wp post get "$post_id" --field=post_content)" = \ +test "$(keel-wp post get "$post_id" --field=post_content)" = \ qa-wordpress-persistence curl "${curl_args[@]}" -b "$work/cookies" "$admin_url" \ >"$work/dashboard-after-restart.html" grep -Eq 'Dashboard|wp-admin-bar' "$work/dashboard-after-restart.html" curl "${curl_args[@]}" "$base/?p=$post_id" >"$work/post-after-restart.html" grep -Fq 'qa-wordpress-persistence' "$work/post-after-restart.html" -turnkey-wp post delete "$post_id" --force >/dev/null +keel-wp post delete "$post_id" --force >/dev/null ! grep -F -- "$password" /var/log/inithooks.log for key in AUTH_KEY SECURE_AUTH_KEY LOGGED_IN_KEY NONCE_KEY \ @@ -88,8 +103,8 @@ done cat >"$result" < Date: Mon, 28 Sep 2026 02:58:55 +0000 Subject: [PATCH 3/6] docs: the changelog entry and the measured coverage Without a changelog entry the newest installable version stays the old one and the change never reaches a machine, which is the trap docs/traps.md of the handbook records and what the "package / changelog" gate exists to catch. COVERAGE.md records what was measured rather than what was hoped for: eight shell files now, 99.12 percent (562/567) over 217 bats tests, none failing, with keel-wp at 100 percent (9/9) and keel-wordpress-update at 100 percent (18/18). The threshold stays 97, the lowest measured file, which is still 40wordpress. --- COVERAGE.md | 33 +++++++++++++++++++++++++++------ changelog | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 6 deletions(-) diff --git a/COVERAGE.md b/COVERAGE.md index 8ca22a6..ebb798d 100644 --- a/COVERAGE.md +++ b/COVERAGE.md @@ -4,12 +4,14 @@ Standard: decisions 0003 (90 percent per repository, 95 for code the project writes) and 0004 (bats plus kcov for shell; a build and a boot on LXC as the acceptance test of a recipe, docs/org-plan.md section 1). -## Measured 2026-09-27 +## Measured 2026-09-28 | File | Test | Lines | Note | | --- | --- | --- | --- | | `overlay/usr/lib/inithooks/lib/wordpress.sh` | `tests/wordpress.bats` (40 tests) | 99.00 percent (99/100) under kcov | every function and every branch | | `overlay/usr/lib/inithooks/firstboot.d/40wordpress` | `tests/hook.bats` (29 tests) | 97.73 percent (43/44) under kcov | the hook itself, run for real | +| `overlay/usr/local/bin/keel-wp` | `tests/wrappers.bats` (21 tests) | 100 percent (9/9) under kcov | both cache branches, the quoting, the exit code it hands back, `DEBUG`, and the `turnkey-wp` link run for real | +| `overlay/usr/local/sbin/keel-wordpress-update` | `tests/wrappers.bats` (the same 21) | 100 percent (18/18) under kcov | the root guard refused and satisfied, each wp-cli call made to fail, and the whole ownership boundary | | `tests/lib/boot-test-lib.sh` | `tests/boot-test.bats` (73 tests) | 98.95 percent (282/285) under kcov | parsing, addresses, deadlines, the container marks, every verdict, and the image carrying none of the build time archive files | | `conf.d/zzz-keel-archive` | `tests/keel-archive.bats` (13 tests) | 100 percent (26/26) under kcov | every way it enables and every way it refuses, including a staging keyring left in the image | | `conf.d/zz-project-packages` | `tests/project-packages.bats` (14 tests) | 100 percent (31/31) under kcov | shared with keel-nodebb, where the pattern is maintained | @@ -19,21 +21,40 @@ acceptance test of a recipe, docs/org-plan.md section 1). | `conf.d/main` | the build | integration only | build time script, 0004 pragmatic limits | | `tests/boot-test.sh` | itself | integration only | the thin main of the acceptance test: keel and LXC as root | -Total over the six measured shell files: **99.07 percent (535/540)**, 196 bats -tests, none failing. `tests/coverage.sh` fails below `COVERAGE_THRESHOLD`, which the workflow +Total over the eight measured shell files: **99.12 percent (562/567)**, 217 +bats tests, none failing. `tests/coverage.sh` fails below `COVERAGE_THRESHOLD`, which the workflow sets to **97**, the lowest measured file. It is only ever raised (decision 0006). $ COVERAGE_THRESHOLD=97 tests/coverage.sh kcov line coverage (threshold 97 percent): + 100.00 18/18 keel-wordpress-update + 100.00 9/9 keel-wp 100.00 31/31 zz-project-packages 100.00 26/26 zzz-keel-archive + 99.00 99/100 wordpress.sh 97.73 43/44 40wordpress 98.95 282/285 boot-test-lib.sh - 99.00 99/100 wordpress.sh 100.00 54/54 keel-archive-check - 100.00 29/29 zz-project-packages - 100.00 26/26 keel-archive-check + +### The two operator commands, and the link beside each + +`keel-wp` and `keel-wordpress-update` are the commands an operator types, and +they are written by this overlay, so decision 0003's 95 percent applies to +them. They were at nothing until 2026-09-28 and are now at 100 percent of +their lines with every branch driven: the cache directory both present and +absent, a `chown` that fails, the exit code of wp-cli handed back, `DEBUG`, +the root guard refused and satisfied, and each of the two wp-cli calls made to +fail so the script stops before it touches ownership. + +The compatibility names `turnkey-wp` and `turnkey-wordpress-update` are +symlinks (decision 0015 of the handbook) and are **run**, not inspected. +`test -L` says a link exists; it does not say the appliance still answers to +the old name. One of those tests copies the whole overlay with `cp -TdR`, +which is literally what `fab-apply-overlay` executes, does it twice because +the Makefile applies this overlay twice, and then runs the copied +`turnkey-wp`. That is the build's own copy step, so the link is proved to +survive it rather than assumed to. ### The five lines that are not covered, and why diff --git a/changelog b/changelog index d722e4f..06eea09 100644 --- a/changelog +++ b/changelog @@ -1,3 +1,42 @@ +turnkey-wordpress-19.0 (4) turnkey; urgency=low + + * The two operator commands this appliance writes itself now carry the + project's own name: /usr/local/bin/keel-wp and + /usr/local/sbin/keel-wordpress-update are the real files, and + /usr/local/bin/turnkey-wp and /usr/local/sbin/turnkey-wordpress-update + are symlinks to them. Both names keep working, which is the point: an + operator who pasted a command out of TurnKey's documentation, or who + wrote a script against turnkey-wp last year, must not silently lose it. + The general rule is decision 0015 of the handbook (tracker#12). + + * The symlinks are committed into the overlay rather than made in + conf.d/main, because fab-apply-overlay copies an overlay with "cp -TdR" + and -d keeps a link a link. This recipe applies its overlay twice, once + through COMMON_OVERLAYS and once as the product-local ROOT_OVERLAY, and a + second copy over an existing link still leaves a link. Both facts are + asserted by running the copied turnkey-wp, not by looking at it. + + * Nothing about what either command does has changed. keel-wp still runs + wp-cli through runuser as www-data, never as root, with an explicit + --path, and still owns /var/www/.wp-cli; keel-wordpress-update still + refuses a caller who is not root, updates core, verifies WordPress's own + per file checksums and puts the ownership boundary back. The refusal now + names whichever of the two names was typed. + + * The paths both scripts hand to another program are overridable from the + environment, the way keel-wp's WP_DIR, WP_USR and WP_CACHE already were, + so the pair can be driven against a scratch tree. The defaults are the + values that were hard coded, so a booted appliance behaves identically. + That is what let them be measured: they were at no coverage at all and + are now at 100 percent of their lines, every branch driven, in + tests/wrappers.bats (COVERAGE.md). + + * conf.d/main, README.rst and tests/v19.sh call the Keel names. tests/v19.sh + also asks the compatibility names for the same answers, because a link + that resolves is not a link that works. + + -- Keel Linux maintainers Mon, 28 Sep 2026 03:00:00 +0000 + turnkey-wordpress-19.0 (3) turnkey; urgency=low * The build verifies the project's own APT archive instead of reading it From 3dd2beccf4c1485ec4ceb003a20fafc6b0f2171a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcos=20M=C3=A9ndez?= Date: Mon, 28 Sep 2026 04:37:09 +0000 Subject: [PATCH 4/6] fix: the updater must not take its target or its web user from the caller Review of #6 found that the one safeguard the description put up for judgement was the one the suite could not fail on, and that the refactor which made the script testable had opened a footgun on a script that runs as root. The web user. tests/wrappers.bats took the expected value from id -un, and under bats USER is already id -un, so ${USER:-www-data} and ${WP_USER:-www-data} expanded to the same string and no assertion could tell them apart: the reviewer reintroduced USER="${USER:-www-data}" throughout and all 21 tests stayed green. The web user in the tests is now the sentinel keel-test-web-user, which no account can be called, and a test runs the updater with USER=root and requires wp-content to still go to the sentinel. chown, install and runuser are stubs that only log, so the value never needed to name a real account. The target. WPROOT, WP_USER and WP_CLI were readable from the environment under those names, so WP_CLI=/bin/true WPROOT=/ keel-wordpress-update would chown and chmod the whole filesystem and strip setuid from sudo, su and passwd. It needs root to trigger and there is no constrained root path to this script, but the hard coded version could not express it at all, and WPROOT is not an obscure name: conf.d/main of this repository uses it for this same path. The three are now KEEL_TEST_WPROOT, KEEL_TEST_WP_USER and KEEL_TEST_WP_CLI, the prefix this suite already uses for exactly this purpose, and a test asserts that the unprefixed names change nothing. Belt and braces on a destructive root script: the updater refuses a target that is not a WordPress before it rewrites anything, by the same wp-includes/version.php that conf.d/main asserts after unpacking the release. keel-wp gave its WP_CLI back and calls /usr/local/bin/wp literally again. It never needed the knob, because runuser is stubbed and the command string is never executed, and the test asserts the shipped path instead. All three mutations are caught now: USER="${USER:-www-data}" fails three tests, un-namespacing WPROOT fails eight, and dropping the WordPress guard fails one. Also in this file, from the same review: it is -L that would flatten the compatibility link, not the absence of -d. -P is already the default under -R and -d only adds --preserve=links, which is about hard links, so the comment claiming -d was load-bearing described a property the test did not check -- dropping -d from the test's own cp failed nothing. There is now a test that checks the real one: a plain cp -TR still yields a working turnkey-wp, and cp -TLR turns it into a second regular file. --- overlay/usr/local/bin/keel-wp | 3 +- overlay/usr/local/sbin/keel-wordpress-update | 22 +++- tests/wrappers.bats | 132 ++++++++++++++----- 3 files changed, 119 insertions(+), 38 deletions(-) diff --git a/overlay/usr/local/bin/keel-wp b/overlay/usr/local/bin/keel-wp index 66383a4..c913c5c 100755 --- a/overlay/usr/local/bin/keel-wp +++ b/overlay/usr/local/bin/keel-wp @@ -12,7 +12,6 @@ WP_DIR=${WP_DIR:-/var/www/wordpress} WP_USR=${WP_USR:-www-data} WP_CACHE=${WP_CACHE:-/var/www/.wp-cli} -WP_CLI=${WP_CLI:-/usr/local/bin/wp} if [[ ! -d "$WP_CACHE" ]]; then mkdir -p "$WP_CACHE" @@ -20,4 +19,4 @@ fi chown -R "$WP_USR":"$WP_USR" "$WP_CACHE" runuser "$WP_USR" -s /bin/bash \ - -c "$WP_CLI --path='$WP_DIR' $(printf '%q ' "$@")" + -c "/usr/local/bin/wp --path='$WP_DIR' $(printf '%q ' "$@")" diff --git a/overlay/usr/local/sbin/keel-wordpress-update b/overlay/usr/local/sbin/keel-wordpress-update index f0a7441..55ff46c 100755 --- a/overlay/usr/local/sbin/keel-wordpress-update +++ b/overlay/usr/local/sbin/keel-wordpress-update @@ -5,13 +5,20 @@ # unsupervised update does not restore that boundary. # # The Keel name is the real command; `turnkey-wordpress-update` beside it is a -# symlink to this file (decision 0015 of the handbook). The refusal below names +# symlink to this file (decision 0015 of the handbook). The refusals below name # whichever of the two the operator typed. set -euo pipefail -WPROOT="${WPROOT:-/var/www/wordpress}" -WP_USER="${WP_USER:-www-data}" -WP_CLI="${WP_CLI:-/usr/local/bin/wp}" +# These three are constants of the appliance. They are readable from +# KEEL_TEST_* only so tests/wrappers.bats can drive this script against a +# scratch tree. The prefix is not decoration: this runs as root and rewrites +# the owner and the mode of every file under its target, so the name that +# chooses that target must not be one an operator could already be exporting +# for something else. WPROOT is exactly such a name -- conf.d/main of this +# repository uses it for this same path. +WPROOT="${KEEL_TEST_WPROOT:-/var/www/wordpress}" +WP_USER="${KEEL_TEST_WP_USER:-www-data}" +WP_CLI="${KEEL_TEST_WP_CLI:-/usr/local/bin/wp}" me=${0##*/} test "$(id -u)" -eq 0 || { @@ -19,6 +26,13 @@ test "$(id -u)" -eq 0 || { exit 1 } +# And the target is a WordPress before anything is rewritten, whatever chose +# the path. conf.d/main asserts this same file after unpacking the release. +test -f "$WPROOT/wp-includes/version.php" || { + echo "$me: $WPROOT is not a WordPress installation" >&2 + exit 1 +} + "$WP_CLI" --allow-root --path="$WPROOT" core update "$WP_CLI" --allow-root --path="$WPROOT" core verify-checksums diff --git a/tests/wrappers.bats b/tests/wrappers.bats index e3f28ad..e6ca635 100644 --- a/tests/wrappers.bats +++ b/tests/wrappers.bats @@ -10,20 +10,22 @@ # Both scripts are executed for real against scratch trees. What they hand to # another program is a stub first in PATH that writes a line per call into a # log the tests read: `runuser`, `chown`, `install` and `id`. wp-cli is a stub -# named by WP_CLI. No test needs root, a web server, a database or a network. +# named by KEEL_TEST_WP_CLI. No test needs root, a web server, a database or a +# network. # -# The symlink is not asserted with `test -L`. A link that resolves is not a -# link that works, and docs/traps.md of the handbook records "asserting a +# The web user is a sentinel that no account on the machine can be called, and +# never `$(id -un)`. The updater must not read the web user from `USER`, which +# root's login environment sets to `root`, and a test whose expected value +# happens to equal `$USER` cannot tell the two apart. +# +# The symlink is not asserted with `test -L` alone. A link that resolves is not +# a link that works, and docs/traps.md of the handbook records "asserting a # configuration value is not asserting the behaviour it was supposed to # produce" as a recurring defect here. So the compatibility name is *run*, and -# it is run once through a copy of the overlay made the way the build makes it +# it is run through a copy of the overlay made the way the build makes it # (`cp -TdR`, which is what fab-apply-overlay executes). setup() { - # Before PATH is bent: `id` becomes a stub below, and these two want the - # real one. - ME="$(id -un)" - REPO="$BATS_TEST_DIRNAME/.." BIN="$REPO/overlay/usr/local/bin" SBIN="$REPO/overlay/usr/local/sbin" @@ -34,8 +36,15 @@ setup() { CALLS="$S/calls" : > "$CALLS" + # A name no account on any machine has, so an assertion on it cannot be + # satisfied by whatever the test runner's own user happens to be. + WEB_USER=keel-test-web-user + WPROOT="$S/wordpress" - mkdir -p "$WPROOT/wp-content" + mkdir -p "$WPROOT/wp-content" "$WPROOT/wp-includes" + # The updater refuses a target that is not a WordPress, and this is the + # file it looks for; conf.d/main asserts the same one after unpacking. + printf ' "$WPROOT/wp-includes/version.php" : > "$WPROOT/wp-config.php" : > "$WPROOT/index.php" @@ -52,25 +61,25 @@ echo "id \$*" >> "$CALLS" echo "\${KEEL_TEST_UID:-0}" EOF chmod +x "$STUBS/id" - # wp-cli: logs, and fails the one subcommand a test names. - WP_CLI="$STUBS/wp" - cat > "$WP_CLI" < "$STUBS/wp" <> "$CALLS" -for arg in "\$@"; do - case "\$arg" in --allow-root|--path=*) continue ;; esac - first=\$arg; break -done [ "\${KEEL_TEST_WP_FAIL:-}" = "\$*" ] && exit "\${KEEL_TEST_WP_CODE:-1}" exit 0 EOF - chmod +x "$WP_CLI" + chmod +x "$STUBS/wp" PATH="$STUBS:$PATH" - export PATH WP_CLI - export WP_DIR="$WPROOT" WPROOT - export WP_USR="$ME" WP_USER="$ME" - export WP_CACHE="$S/wp-cli-cache" + export PATH + # keel-wp's three knobs are the ones it already shipped with and are the + # appliance's own. The updater's are KEEL_TEST_ prefixed because they exist + # only for these tests: that script runs as root and rewrites the owner and + # the mode of every file under its target, so the name that chooses the + # target must not be one an operator could already have exported. + export WP_DIR="$WPROOT" WP_USR="$WEB_USER" WP_CACHE="$S/wp-cli-cache" + export KEEL_TEST_WPROOT="$WPROOT" KEEL_TEST_WP_USER="$WEB_USER" + export KEEL_TEST_WP_CLI="$STUBS/wp" } _stub() { @@ -94,6 +103,8 @@ _runuser_command() { [ -f "$WP" ] && [ ! -L "$WP" ] [ -x "$WP" ] [ -L "$BIN/turnkey-wp" ] + # Relative, not absolute: an absolute link does not resolve inside + # fab-chroot, so conf.d/main would not find it at build time. [ "$(readlink "$BIN/turnkey-wp")" = keel-wp ] } @@ -107,8 +118,8 @@ _runuser_command() { @test "keel-wp runs wp-cli as the web user, with an explicit path, never as root" { run "$WP" option get siteurl [ "$status" -eq 0 ] - grep -q "^runuser $WP_USR -s /bin/bash -c " "$CALLS" - [[ "$(_runuser_command)" == *"--path='$WPROOT'"* ]] + grep -q "^runuser $WEB_USER -s /bin/bash -c " "$CALLS" + [[ "$(_runuser_command)" == *"/usr/local/bin/wp --path='$WPROOT'"* ]] [[ "$(_runuser_command)" != *--allow-root* ]] } @@ -135,7 +146,7 @@ _runuser_command() { run "$WP" core version [ "$status" -eq 0 ] [ -d "$WP_CACHE" ] - grep -q "^chown -R $WP_USR:$WP_USR $WP_CACHE\$" "$CALLS" + grep -q "^chown -R $WEB_USER:$WEB_USER $WP_CACHE\$" "$CALLS" } @test "keel-wp leaves an existing cache directory alone and still owns it" { @@ -144,10 +155,10 @@ _runuser_command() { run "$WP" core version [ "$status" -eq 0 ] [ -f "$WP_CACHE/already-here" ] - grep -q "^chown -R $WP_USR:$WP_USR $WP_CACHE\$" "$CALLS" + grep -q "^chown -R $WEB_USER:$WEB_USER $WP_CACHE\$" "$CALLS" } -@test "keel-wp reports the exit code wp-cli gave it" { +@test "keel-wp hands back the exit code runuser gave it" { KEEL_TEST_RUNUSER_CODE=3 run "$WP" core verify-checksums [ "$status" -eq 3 ] } @@ -171,13 +182,13 @@ _runuser_command() { [[ "$output" == *"+ runuser"* ]] } -# --- the compatibility name, run rather than inspected ----------------------- +# --- the compatibility names, run rather than inspected ---------------------- @test "turnkey-wp runs the same command keel-wp does" { [ -L "$BIN/turnkey-wp" ] run "$BIN/turnkey-wp" option get siteurl [ "$status" -eq 0 ] - grep -q "^runuser $WP_USR -s /bin/bash -c " "$CALLS" + grep -q "^runuser $WEB_USER -s /bin/bash -c " "$CALLS" [[ "$(_runuser_command)" == *"--path='$WPROOT'"* ]] [[ "$(_runuser_command)" == *"option get siteurl"* ]] } @@ -205,12 +216,33 @@ _runuser_command() { root="$S/root.patched" mkdir -p "$root" cp -TdR "$REPO/overlay" "$root" + cp -TdR "$REPO/overlay" "$root" [ -L "$root/usr/local/sbin/turnkey-wordpress-update" ] run "$root/usr/local/sbin/turnkey-wordpress-update" [ "$status" -eq 0 ] grep -q "^wp --allow-root --path=$WPROOT core update\$" "$CALLS" } +@test "it is -L that would flatten the link, and -d is not what prevents it" { + # What the overlay step has to avoid is dereferencing. -P is already the + # default under -R, so a plain -R keeps the link, and -d only adds + # --preserve=links, which is about hard links and does nothing here. That + # is asserted rather than left in a comment, because the comment above + # says the build's flags are safe and this is the reason they are. + plain="$S/plain"; deref="$S/deref" + cp -TR "$REPO/overlay" "$plain" + cp -TLR "$REPO/overlay" "$deref" + # -R without -d: still a link, and still a working command. + [ -L "$plain/usr/local/bin/turnkey-wp" ] + run "$plain/usr/local/bin/turnkey-wp" core version + [ "$status" -eq 0 ] + # -L: a second regular file, and the compatibility name stops being a link + # to anything. This is the flag that would break the property. + [ ! -L "$deref/usr/local/bin/turnkey-wp" ] + [ -f "$deref/usr/local/bin/turnkey-wp" ] + [ ! -L "$deref/usr/local/sbin/turnkey-wordpress-update" ] +} + @test "turnkey-wordpress-update refuses a non-root caller under its own name" { [ -L "$SBIN/turnkey-wordpress-update" ] KEEL_TEST_UID=1000 run "$SBIN/turnkey-wordpress-update" @@ -224,7 +256,16 @@ _runuser_command() { KEEL_TEST_UID=1000 run "$UPDATE" [ "$status" -eq 1 ] [[ "$output" == *"keel-wordpress-update must run as root"* ]] - [ ! -s "$CALLS" ] || ! grep -q '^wp ' "$CALLS" + ! grep -q '^wp ' "$CALLS" +} + +@test "keel-wordpress-update refuses a target that is not a WordPress" { + rm -f "$WPROOT/wp-includes/version.php" + run "$UPDATE" + [ "$status" -eq 1 ] + [[ "$output" == *"is not a WordPress installation"* ]] + ! grep -q '^wp ' "$CALLS" + ! grep -q '^chown ' "$CALLS" } @test "keel-wordpress-update updates core and then verifies the checksums" { @@ -254,7 +295,7 @@ _runuser_command() { run "$UPDATE" [ "$status" -eq 0 ] grep -q "^chown -R root:root $WPROOT\$" "$CALLS" - grep -q "^chown root:$WP_USER $WPROOT/wp-config.php\$" "$CALLS" + grep -q "^chown root:$WEB_USER $WPROOT/wp-config.php\$" "$CALLS" [ "$(stat -c %a "$WPROOT/wp-config.php")" = 640 ] [ "$(stat -c %a "$WPROOT/index.php")" = 644 ] [ "$(stat -c %a "$WPROOT")" = 755 ] @@ -264,8 +305,35 @@ _runuser_command() { run "$UPDATE" [ "$status" -eq 0 ] for dir in uploads cache upgrade plugins themes; do - grep -q "^install -d -o $WP_USER -g $WP_USER -m 0755 $WPROOT/wp-content/$dir\$" \ + grep -q "^install -d -o $WEB_USER -g $WEB_USER -m 0755 $WPROOT/wp-content/$dir\$" \ "$CALLS" - grep -q "^chown -R $WP_USER:$WP_USER $WPROOT/wp-content/$dir\$" "$CALLS" + grep -q "^chown -R $WEB_USER:$WEB_USER $WPROOT/wp-content/$dir\$" "$CALLS" done } + +# --- the two names that must not come from the caller's environment ---------- + +@test "USER in the environment does not decide who owns wp-content" { + # The file this replaced opened `USER=www-data`, a plain assignment that + # masks the inherited value. Reading `${USER:-www-data}` instead would take + # root's login environment, where USER is root, and hand every runtime + # directory to root:root: uploads and plugin installs would fail from the + # first supervised update onwards, with nothing pointing back at it. + USER=root run "$UPDATE" + [ "$status" -eq 0 ] + grep -q "^chown -R $WEB_USER:$WEB_USER $WPROOT/wp-content/uploads\$" "$CALLS" + ! grep -q '^chown -R root:root .*wp-content' "$CALLS" + grep -q "^chown root:$WEB_USER $WPROOT/wp-config.php\$" "$CALLS" +} + +@test "WPROOT in the environment does not decide what this script rewrites" { + # It runs as root and rewrites the owner and the mode of every file under + # its target, so the variable that chooses that target is deliberately not + # a name an operator could already be exporting. conf.d/main of this very + # repository uses WPROOT for this same path. + WPROOT=/ WP_USER=root WP_CLI=/bin/true run "$UPDATE" + [ "$status" -eq 0 ] + grep -q "^chown -R root:root $KEEL_TEST_WPROOT\$" "$CALLS" + ! grep -qx 'chown -R root:root /' "$CALLS" + grep -q "^wp --allow-root --path=$KEEL_TEST_WPROOT core update\$" "$CALLS" +} From 5a077913d2d642edaf03f008fef526006f92b8cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcos=20M=C3=A9ndez?= Date: Mon, 28 Sep 2026 04:37:25 +0000 Subject: [PATCH 5/6] fix: the compatibility check on a real appliance passed if both sides failed tests/v19.sh compared two command substitutions inside test: test "$(turnkey-wp option get siteurl)" = "$(keel-wp option get siteurl)" set -euo pipefail is on, but a command substitution in test's arguments does not trip errexit. With wp-cli broken, the path wrong or Apache down, both sides are empty and this is test "" = "", which passes. That is docs/traps.md of the handbook, "asserting the configuration is not asserting the behaviour", in the one check offered as proof that the old name still answers on a real machine. It now compares against the literal the script already asserted at line 22, which cannot pass on a double failure. $before, the yardstick for the two core version comparisons, is asserted non-empty where it is taken, for the same reason. --- tests/v19.sh | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/v19.sh b/tests/v19.sh index 4069f69..c53ab62 100755 --- a/tests/v19.sh +++ b/tests/v19.sh @@ -45,6 +45,7 @@ test "$(keel-wp post get "$post_id" --field=post_content)" = \ qa-wordpress-persistence before=$(keel-wp core version) +test -n "$before" keel-wp core check-update --format=json >"$work/update.json" python3 -c 'import json,sys; assert isinstance(json.load(open(sys.argv[1])), list)' \ "$work/update.json" @@ -57,12 +58,17 @@ fi # The compatibility names of decision 0015, exercised rather than inspected: a # link that resolves is not a link that works. turnkey-wp is asked the same -# question keel-wp was just asked and has to give the same answer, and the +# questions keel-wp was just asked and has to give the same answers, and the # updater's root guard has to hold under the old name too. +# +# Each answer is compared against a literal the script already knows, never +# against a second command substitution: a command substitution in test's +# arguments does not trip errexit, so comparing two of them passes as +# test "" = "" when wp-cli is broken and both sides are empty. test -L /usr/local/bin/turnkey-wp test "$(readlink /usr/local/bin/turnkey-wp)" = keel-wp test "$(turnkey-wp core version)" = "$before" -test "$(turnkey-wp option get siteurl)" = "$(keel-wp option get siteurl)" +test "$(turnkey-wp option get siteurl)" = https://localhost test -L /usr/local/sbin/turnkey-wordpress-update test "$(readlink /usr/local/sbin/turnkey-wordpress-update)" = keel-wordpress-update if runuser -u www-data -- /usr/local/sbin/turnkey-wordpress-update; then From ebd9b0ba5144025c29c6d05c2222fa4fd31969fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcos=20M=C3=A9ndez?= Date: Mon, 28 Sep 2026 04:37:25 +0000 Subject: [PATCH 6/6] docs: the corrected mechanism, the new numbers, and a stale comment The claim that -d is what keeps a symlink a symlink was wrong in COVERAGE.md and in the changelog as well as in the test. -R copies a symlink as a symlink unless -L is given; -P is already the default and -d only adds --preserve=links, which is about hard links. Measured with coreutils 9.7 and now asserted by a test rather than only written down. The conclusion is unchanged: the build's copy step is safe, but for a different reason than the one given. The changelog entry also records the KEEL_TEST_ prefixed overrides and why the prefix is not decoration, the new refusal of a target that is not a WordPress, that keel-wp gained no knob at all, and that both links are relative because an absolute one does not resolve inside fab-chroot. Numbers re-measured: 221 bats tests, none failing, 99.12 percent (564/569) over the eight measured files, keel-wp 8/8 and keel-wordpress-update 21/21. .github/workflows/tests.yml still said tests/coverage.sh measures six shell files; it has measured eight since the previous commit on this branch. The threshold below it is unchanged and still correct. --- .github/workflows/tests.yml | 2 +- COVERAGE.md | 44 ++++++++++++++++++++++++---------- changelog | 48 ++++++++++++++++++++++++++++--------- 3 files changed, 69 insertions(+), 25 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 7be6eaf..524d7b3 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -17,7 +17,7 @@ permissions: jobs: tests: - # tests/coverage.sh measures six shell files (COVERAGE.md). The threshold + # tests/coverage.sh measures eight shell files (COVERAGE.md). The threshold # is the lowest of them and is only ever raised (decision 0006). This job # produces the check "tests / coverage", the required status on master. The # job id is part of that name, so renaming it silently detaches the diff --git a/COVERAGE.md b/COVERAGE.md index ebb798d..c0c526c 100644 --- a/COVERAGE.md +++ b/COVERAGE.md @@ -10,8 +10,8 @@ acceptance test of a recipe, docs/org-plan.md section 1). | --- | --- | --- | --- | | `overlay/usr/lib/inithooks/lib/wordpress.sh` | `tests/wordpress.bats` (40 tests) | 99.00 percent (99/100) under kcov | every function and every branch | | `overlay/usr/lib/inithooks/firstboot.d/40wordpress` | `tests/hook.bats` (29 tests) | 97.73 percent (43/44) under kcov | the hook itself, run for real | -| `overlay/usr/local/bin/keel-wp` | `tests/wrappers.bats` (21 tests) | 100 percent (9/9) under kcov | both cache branches, the quoting, the exit code it hands back, `DEBUG`, and the `turnkey-wp` link run for real | -| `overlay/usr/local/sbin/keel-wordpress-update` | `tests/wrappers.bats` (the same 21) | 100 percent (18/18) under kcov | the root guard refused and satisfied, each wp-cli call made to fail, and the whole ownership boundary | +| `overlay/usr/local/bin/keel-wp` | `tests/wrappers.bats` (25 tests) | 100 percent (8/8) under kcov | both cache branches, the quoting, the exit code it hands back, `DEBUG`, and the `turnkey-wp` link run for real | +| `overlay/usr/local/sbin/keel-wordpress-update` | `tests/wrappers.bats` (the same 25) | 100 percent (21/21) under kcov | both guards refused and satisfied, each wp-cli call made to fail, the whole ownership boundary, and the two names it must not take from the environment | | `tests/lib/boot-test-lib.sh` | `tests/boot-test.bats` (73 tests) | 98.95 percent (282/285) under kcov | parsing, addresses, deadlines, the container marks, every verdict, and the image carrying none of the build time archive files | | `conf.d/zzz-keel-archive` | `tests/keel-archive.bats` (13 tests) | 100 percent (26/26) under kcov | every way it enables and every way it refuses, including a staging keyring left in the image | | `conf.d/zz-project-packages` | `tests/project-packages.bats` (14 tests) | 100 percent (31/31) under kcov | shared with keel-nodebb, where the pattern is maintained | @@ -21,15 +21,15 @@ acceptance test of a recipe, docs/org-plan.md section 1). | `conf.d/main` | the build | integration only | build time script, 0004 pragmatic limits | | `tests/boot-test.sh` | itself | integration only | the thin main of the acceptance test: keel and LXC as root | -Total over the eight measured shell files: **99.12 percent (562/567)**, 217 +Total over the eight measured shell files: **99.12 percent (564/569)**, 221 bats tests, none failing. `tests/coverage.sh` fails below `COVERAGE_THRESHOLD`, which the workflow sets to **97**, the lowest measured file. It is only ever raised (decision 0006). $ COVERAGE_THRESHOLD=97 tests/coverage.sh kcov line coverage (threshold 97 percent): - 100.00 18/18 keel-wordpress-update - 100.00 9/9 keel-wp + 100.00 21/21 keel-wordpress-update + 100.00 8/8 keel-wp 100.00 31/31 zz-project-packages 100.00 26/26 zzz-keel-archive 99.00 99/100 wordpress.sh @@ -43,18 +43,36 @@ sets to **97**, the lowest measured file. It is only ever raised (decision they are written by this overlay, so decision 0003's 95 percent applies to them. They were at nothing until 2026-09-28 and are now at 100 percent of their lines with every branch driven: the cache directory both present and -absent, a `chown` that fails, the exit code of wp-cli handed back, `DEBUG`, -the root guard refused and satisfied, and each of the two wp-cli calls made to -fail so the script stops before it touches ownership. +absent, a `chown` that fails, the exit code handed back, `DEBUG`, the root +guard and the is-this-a-WordPress guard each refused and satisfied, and each +of the two wp-cli calls made to fail so the script stops before it touches +ownership. + +Two of the tests assert a name the updater must **not** read. It runs as root +and rewrites the owner and mode of everything under its target, so it takes +that target from `KEEL_TEST_WPROOT` and not from `WPROOT`, which an operator +may already be exporting and which `conf.d/main` uses for the same path; and +it takes the web user from `KEEL_TEST_WP_USER` and never from `USER`, which +in root's login environment is `root`. The web user in these tests is the +sentinel `keel-test-web-user`, deliberately not `$(id -un)`: with the expected +value equal to `$USER`, neither assertion could tell the two apart. The compatibility names `turnkey-wp` and `turnkey-wordpress-update` are symlinks (decision 0015 of the handbook) and are **run**, not inspected. `test -L` says a link exists; it does not say the appliance still answers to -the old name. One of those tests copies the whole overlay with `cp -TdR`, -which is literally what `fab-apply-overlay` executes, does it twice because -the Makefile applies this overlay twice, and then runs the copied -`turnkey-wp`. That is the build's own copy step, so the link is proved to -survive it rather than assumed to. +the old name. Two of those tests copy the whole overlay with `cp -TdR`, which +is literally what `fab-apply-overlay` executes, do it twice because the +Makefile applies this overlay twice, and then run the copied command. That is +the build's own copy step, so the link is proved to survive it rather than +assumed to. + +What makes it survive is worth stating correctly, because a copy step is the +kind of thing that gets written again from this note. `-R` copies a symlink as +a symlink unless `-L` is given: `-P` is already the default under `-R`, and +`-d` only adds `--preserve=links`, which is about **hard** links and does +nothing for this. So the property is the absence of `-L`, not the presence of +`-d`, and a test asserts exactly that: a plain `cp -TR` still yields a working +`turnkey-wp`, and `cp -TLR` turns it into a second regular file. ### The five lines that are not covered, and why diff --git a/changelog b/changelog index 06eea09..d9ad656 100644 --- a/changelog +++ b/changelog @@ -10,11 +10,20 @@ turnkey-wordpress-19.0 (4) turnkey; urgency=low The general rule is decision 0015 of the handbook (tracker#12). * The symlinks are committed into the overlay rather than made in - conf.d/main, because fab-apply-overlay copies an overlay with "cp -TdR" - and -d keeps a link a link. This recipe applies its overlay twice, once - through COMMON_OVERLAYS and once as the product-local ROOT_OVERLAY, and a - second copy over an existing link still leaves a link. Both facts are - asserted by running the copied turnkey-wp, not by looking at it. + conf.d/main. fab-apply-overlay copies an overlay with "cp -TdR", and what + keeps a symlink a symlink there is -R without -L: -P is already the + default under -R, and -d only adds --preserve=links, which is about hard + links. So the property is the absence of -L, and that is what the tests + assert -- a plain "cp -TR" still gives a working turnkey-wp and "cp -TLR" + turns it into a second regular file. This recipe also applies its overlay + twice, once through COMMON_OVERLAYS and once as the product-local + ROOT_OVERLAY, and a second copy over an existing link still leaves a link. + Every one of these is asserted by running the copied command, not by + looking at it. + + * Both links are relative, turnkey-wp -> keel-wp rather than an absolute + path, because an absolute link does not resolve inside fab-chroot and + conf.d/main calls the command at build time. * Nothing about what either command does has changed. keel-wp still runs wp-cli through runuser as www-data, never as root, with an explicit @@ -23,12 +32,29 @@ turnkey-wordpress-19.0 (4) turnkey; urgency=low per file checksums and puts the ownership boundary back. The refusal now names whichever of the two names was typed. - * The paths both scripts hand to another program are overridable from the - environment, the way keel-wp's WP_DIR, WP_USR and WP_CACHE already were, - so the pair can be driven against a scratch tree. The defaults are the - values that were hard coded, so a booted appliance behaves identically. - That is what let them be measured: they were at no coverage at all and - are now at 100 percent of their lines, every branch driven, in + * keel-wordpress-update reads its WordPress root, its web user and its + wp-cli path from KEEL_TEST_WPROOT, KEEL_TEST_WP_USER and KEEL_TEST_WP_CLI + when those are set, so the tests can drive it against a scratch tree. The + prefix is deliberate rather than decorative: the script runs as root and + rewrites the owner and the mode of every file under its target, so the + name that chooses the target must not be one an operator could already be + exporting for something else, and WPROOT is exactly such a name -- + conf.d/main of this repository uses it for this same path. keel-wp gained + no new knob at all; its WP_DIR, WP_USR and WP_CACHE are the ones it + already shipped with. + + * keel-wordpress-update now refuses a target that is not a WordPress, by + the same wp-includes/version.php that conf.d/main asserts after + unpacking, before it rewrites anything. It reads its web user from + KEEL_TEST_WP_USER and never from USER: the previous file opened + USER=www-data, a plain assignment that masks the inherited value, and + reading ${USER:-www-data} instead would take root's login environment and + hand every wp-content runtime directory to root, so uploads and plugin + installs would fail from the first supervised update onwards. Both are + asserted. + + * That is what let both scripts be measured: they were at no coverage at + all and are now at 100 percent of their lines, every branch driven, in tests/wrappers.bats (COVERAGE.md). * conf.d/main, README.rst and tests/v19.sh call the Keel names. tests/v19.sh