diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 69409d6..8252e0f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -264,3 +264,46 @@ jobs: exit 1 - run: make test-examples + + # A dispatch-layer smoke test, not another correctness suite: three real + # dispatch bugs (--version=previous returning the current value, + # --format rejected for want of a description line, and migrate-legacy + # unregistered without @subcommand) reached a green PHPUnit suite because + # nothing exercised the real `wp` binary end to end. 7.4 is the floor + # this API has to run on; 8.3 is the newest PHP the rest of the matrix + # covers. No Composer step: the smoke test provisions its own WP-CLI phar + # and WordPress checkout and needs neither `vendor/` nor the WordPress + # test suite. + smoke: + name: "Smoke / PHP ${{ matrix.php }}" + needs: static + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + php: ['7.4', '8.3'] + services: + mysql: + image: mysql:8.0 + env: + MYSQL_ALLOW_EMPTY_PASSWORD: 'yes' + MYSQL_DATABASE: wordpress_smoke + ports: + - 3306:3306 + options: >- + --health-cmd="mysqladmin ping" + --health-interval=10s + --health-timeout=5s + --health-retries=5 + steps: + - name: Check out + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - name: Set up PHP ${{ matrix.php }} + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 + with: + php-version: ${{ matrix.php }} + extensions: sodium, mysqli + coverage: none + + - run: make smoke DB_HOST=127.0.0.1 diff --git a/.gitignore b/.gitignore index cf57298..4cd26ac 100644 --- a/.gitignore +++ b/.gitignore @@ -24,3 +24,5 @@ site/.astro/ # Spacefast CLI link and state. Written wherever sf publish runs from; never commit it. .spacefast/ +# The WP-CLI smoke test's throwaway install; see bin/smoke-install.sh. +/.smoke/ diff --git a/Makefile b/Makefile index a1a96cd..dbcae07 100644 --- a/Makefile +++ b/Makefile @@ -13,9 +13,10 @@ DB_NAME ?= wordpress_test DB_USER ?= root DB_PASS ?= DB_HOST ?= 127.0.0.1 +SMOKE_DB_NAME ?= wordpress_smoke .DEFAULT_GOAL := help -.PHONY: help install lint lint-fix compat analyse test test-ms test-examples coverage reference reference-check ci clean +.PHONY: help install lint lint-fix compat analyse test test-ms test-examples coverage reference reference-check ci smoke clean help: ## Show this help. @grep -hE '^[a-zA-Z_-]+:.*?## ' $(MAKEFILE_LIST) \ @@ -54,7 +55,11 @@ reference: ## Regenerate docs/reference/ from source docblocks. reference-check: ## Fail if docs/reference/ is stale relative to the source. php bin/gen-reference.php --check -ci: lint compat analyse reference-check test test-ms ## Everything CI runs. +smoke: ## Provision the throwaway install and run the WP-CLI smoke test. + SMOKE_DB_NAME=$(SMOKE_DB_NAME) DB_USER=$(DB_USER) DB_PASS="$(DB_PASS)" DB_HOST=$(DB_HOST) WP_VERSION=$(WP_VERSION) bin/smoke-install.sh + tests/smoke/smoke.sh + +ci: lint compat analyse reference-check test test-ms smoke ## Everything CI runs. # Local Vault dev server for the vault-provider example (pinned digest): # docker run -d --name secrets-api-vault -p 8201:8200 -e VAULT_DEV_ROOT_TOKEN_ID=dev-root --cap-add=IPC_LOCK hashicorp/vault@sha256:47f14a6acb98f48d798a07df7c83f23a6e636e1cf724c5f8ff165cb32667a1e2 diff --git a/README.md b/README.md index 26a4602..0ea039e 100644 --- a/README.md +++ b/README.md @@ -27,7 +27,8 @@ composer install bin/ci-local.sh ``` -Add `--keep` to leave the environment running between iterations. +Add `--keep` to leave the environment running between iterations. `bin/ci-local.sh` now also runs +the WP-CLI smoke test inside wp-env, against its own throwaway install. If you already have a WordPress test suite and a database, skip wp-env entirely: @@ -49,6 +50,7 @@ target list. | `make test-examples` | phpunit against `examples/*/tests`, needs Moto running (see `examples/README.md`); not part of `make ci` | | `make coverage` | phpunit with an HTML coverage report (see `docs/journal/test-coverage-gaps.md` re: wp-env) | | `make reference` / `make reference-check` | regenerate `docs/reference/` from docblocks / fail if it is stale | +| `make smoke` | provision a throwaway WordPress in `.smoke/` and drive `wp secret` / `wp network-secret` end to end (needs MySQL and network access) | | `make ci` | all of the above | Runners without egress to wordpress.org can point the installer at a mirror with `WP_MIRROR_BASE` @@ -167,8 +169,9 @@ stores credentials, and a flaw in it is a flaw in the thing protecting everythin CI (`.github/workflows/ci.yml`) is a thin wrapper around the `make` targets above, running on github.com's hosted runners: static analysis gates a PHP 7.4/8.0/8.3 × WordPress latest/trunk -matrix plus a multisite job, plus an `examples` job that runs the platform bindings against a -Vault service container. See [`docs/reference/ci.md`](docs/reference/ci.md). +matrix plus a multisite job, a PHP 7.4/8.3 smoke job driving a real `wp` binary, and an +`examples` job that runs the platform bindings against Moto and Vault service containers. See +[`docs/reference/ci.md`](docs/reference/ci.md). ## License diff --git a/bin/ci-local.sh b/bin/ci-local.sh index 4467a8c..40ebf4b 100755 --- a/bin/ci-local.sh +++ b/bin/ci-local.sh @@ -71,5 +71,16 @@ echo "==> test (multisite)" "${WP_ENV[@]}" run --env-cwd="$CONTAINER_CWD" tests-cli \ env WP_MULTISITE=1 vendor/bin/phpunit -c phpunit-multisite.xml.dist +# The smoke test provisions its own throwaway install under .smoke/, with its +# own database (wordpress_smoke), inside wp-env's *development* `cli` +# container -- never the `tests-cli` container or its wordpress_test +# database, which the PHPUnit suite above owns. The two scripts run directly, +# rather than through `make smoke`, because the `cli` container has no `make`. +echo "==> smoke (WP-CLI against a throwaway install)" +"${WP_ENV[@]}" run --env-cwd="$CONTAINER_CWD" cli \ + env DB_HOST=mysql DB_USER=root DB_PASS=password bash bin/smoke-install.sh +"${WP_ENV[@]}" run --env-cwd="$CONTAINER_CWD" cli \ + bash tests/smoke/smoke.sh + echo echo "All green." diff --git a/bin/smoke-install.sh b/bin/smoke-install.sh new file mode 100755 index 0000000..6122d94 --- /dev/null +++ b/bin/smoke-install.sh @@ -0,0 +1,162 @@ +#!/usr/bin/env bash +# +# Provision a throwaway single-site WordPress install for the WP-CLI smoke test. +# +# This script owns everything under .smoke/: a pinned wp-cli.phar, its download +# cache, and a full WordPress checkout with its own "wordpress_smoke" database. +# It never touches wp-env's own dev or test environment, because that one shares +# wp-content with PHPUnit -- and a drop-in the smoke test writes would sit in +# front of the PHPUnit suite. See tests/smoke/SPEC.md. +# +set -euo pipefail +cd "$(dirname "$0")/.." + +# Pinned wp-cli release. Resolved from +# https://api.github.com/repos/wp-cli/wp-cli/releases/latest on 2026-09-24 and +# verified against that release's own published +# wp-cli-2.12.0.phar.sha256 before computing this digest -- the same discipline +# ci.yml applies to action SHAs. +WP_CLI_VERSION="2.12.0" +WP_CLI_SHA256="ce34ddd838f7351d6759068d09793f26755463b4a4610a5a5c0a97b68220d85c" +WP_CLI_URL="https://github.com/wp-cli/wp-cli/releases/download/v${WP_CLI_VERSION}/wp-cli-${WP_CLI_VERSION}.phar" + +SMOKE_DIR="$PWD/.smoke" +SMOKE_DB_NAME="${SMOKE_DB_NAME:-wordpress_smoke}" +DB_USER="${DB_USER:-root}" +DB_PASS="${DB_PASS:-}" +DB_HOST="${DB_HOST:-127.0.0.1}" +WP_VERSION="${WP_VERSION:-latest}" +SMOKE_URL="${SMOKE_URL:-http://smoke.test}" + +# The PHPUnit suite drops and recreates wordpress_test on every run; this +# script drops and recreates whatever SMOKE_DB_NAME names. Refusing when they +# collide protects the PHPUnit suite's database from the smoke test, and vice +# versa. +if [ "$SMOKE_DB_NAME" = "wordpress_test" ]; then + echo "SMOKE_DB_NAME must not be wordpress_test: that database belongs to the PHPUnit suite." >&2 + exit 1 +fi + +export WP_CLI_CACHE_DIR="$SMOKE_DIR/cache" + +WP=( php -d display_errors=stderr -d log_errors=0 "$SMOKE_DIR/wp-cli.phar" --path="$SMOKE_DIR/wordpress" --allow-root ) + +download() { + # $1 = URL, $2 = destination path. + if command -v curl >/dev/null 2>&1; then + curl -fsSL -o "$2" "$1" + elif command -v wget >/dev/null 2>&1; then + wget -nv -O "$2" "$1" + else + php -r 'copy($argv[1], $argv[2]);' -- "$1" "$2" + fi +} + +sha256_of() { + if command -v sha256sum >/dev/null 2>&1; then + sha256sum "$1" | awk '{print $1}' + else + shasum -a 256 "$1" | awk '{print $1}' + fi +} + +fetch_wp_cli() { + mkdir -p "$SMOKE_DIR" "$WP_CLI_CACHE_DIR" + + if [ -f "$SMOKE_DIR/wp-cli.phar" ]; then + ACTUAL=$(sha256_of "$SMOKE_DIR/wp-cli.phar") + if [ "$ACTUAL" = "$WP_CLI_SHA256" ]; then + return + fi + fi + + download "$WP_CLI_URL" "$SMOKE_DIR/wp-cli.phar" + + ACTUAL=$(sha256_of "$SMOKE_DIR/wp-cli.phar") + if [ "$ACTUAL" != "$WP_CLI_SHA256" ]; then + rm -f "$SMOKE_DIR/wp-cli.phar" + echo "wp-cli.phar checksum mismatch." >&2 + echo "expected: $WP_CLI_SHA256" >&2 + echo "actual: $ACTUAL" >&2 + exit 1 + fi +} + +fetch_wp_cli + +if [ ! -f "$SMOKE_DIR/wordpress/wp-load.php" ]; then + mkdir -p "$SMOKE_DIR/wordpress" + "${WP[@]}" core download --version="$WP_VERSION" +fi + +# Every run starts single-site with no drop-in: a leftover secrets.php or +# multisite wp-config would make this script's own idempotence lie about what +# state a fresh run leaves behind. +rm -f "$SMOKE_DIR/wordpress/wp-config.php" "$SMOKE_DIR/wordpress/wp-content/secrets.php" + +"${WP[@]}" config create \ + --dbname="$SMOKE_DB_NAME" \ + --dbuser="$DB_USER" \ + --dbpass="$DB_PASS" \ + --dbhost="$DB_HOST" \ + --skip-check \ + --force + +# Drop and recreate the database with mysqli directly, rather than a `wp db` +# subcommand or the `mysql` client binary, so this script stays dependency-free +# beyond wp and php. Host/port/socket parsing mirrors install_db() in +# bin/install-wp-tests.sh. Arguments are passed as $argv, never interpolated +# into the PHP source. +IFS=':' read -r DB_HOSTNAME DB_SOCK_OR_PORT <<< "$DB_HOST" +DB_PORT="" +DB_SOCKET="" +if [[ "$DB_SOCK_OR_PORT" =~ ^[0-9]+$ ]]; then + DB_PORT="$DB_SOCK_OR_PORT" +elif [ -n "${DB_SOCK_OR_PORT:-}" ]; then + DB_SOCKET="$DB_SOCK_OR_PORT" +fi + +php -r ' + list( $host, $user, $pass, $name, $port, $socket ) = array_slice( $argv, 1 ); + + $port = $port !== "" ? (int) $port : null; + $socket = $socket !== "" ? $socket : null; + + mysqli_report( MYSQLI_REPORT_OFF ); + $link = mysqli_init(); + + if ( ! $link || ! @mysqli_real_connect( $link, $host, $user, $pass, "", $port, $socket ) ) { + fwrite( STDERR, "mysqli connect error: " . mysqli_connect_error() . "\n" ); + exit( 1 ); + } + + if ( ! mysqli_query( $link, "DROP DATABASE IF EXISTS `" . $name . "`" ) ) { + fwrite( STDERR, "mysqli error: " . mysqli_error( $link ) . "\n" ); + exit( 1 ); + } + + if ( ! mysqli_query( $link, "CREATE DATABASE `" . $name . "`" ) ) { + fwrite( STDERR, "mysqli error: " . mysqli_error( $link ) . "\n" ); + exit( 1 ); + } +' -- "$DB_HOSTNAME" "$DB_USER" "$DB_PASS" "$SMOKE_DB_NAME" "$DB_PORT" "$DB_SOCKET" + +"${WP[@]}" core install \ + --url="$SMOKE_URL" \ + --title="Secrets API smoke" \ + --admin_user=smoke \ + --admin_password="$(php -r 'echo bin2hex(random_bytes(16));')" \ + --admin_email=smoke@example.com \ + --skip-email + +# Relative on purpose: the same link resolves on the host and inside the +# wp-env container. +ln -sfn ../../../.. "$SMOKE_DIR/wordpress/wp-content/plugins/secrets-api" +"${WP[@]}" plugin activate secrets-api + +KEY="$("${WP[@]}" secret generate-key)" +# --quiet: `wp config set` otherwise echoes the constant's value in its +# "Success" line, which would put the generated key in plain sight. +"${WP[@]}" config set WP_SECRETS_KEY "$KEY" --type=constant --quiet + +echo "Smoke install ready: $SMOKE_DIR/wordpress (database $SMOKE_DB_NAME)" diff --git a/cli/class-wp-cli-secret-command.php b/cli/class-wp-cli-secret-command.php index ad9d3bb..b20a5b5 100644 --- a/cli/class-wp-cli-secret-command.php +++ b/cli/class-wp-cli-secret-command.php @@ -114,10 +114,9 @@ public function set( $args, $assoc_args ) { * [--slot=] * : Which stored version to read. * - * Named --slot rather than --version because WP-CLI consumes `--version` - * itself before a subcommand ever sees it: passing --version=previous - * silently yielded the current value, since the flag was swallowed and the - * synopsis default filled in behind it. + * Named --slot rather than --version to avoid the value being dropped by + * wrappers such as `wp-env run`, which consume --version themselves (the + * original bug), and to avoid confusion with `wp --version`. * --- * default: current * options: diff --git a/docs/index.md b/docs/index.md index f9eaeec..74a540b 100644 --- a/docs/index.md +++ b/docs/index.md @@ -71,6 +71,7 @@ directory holds everything longer than that. - [`2026-09-04-0-1-0-is-public.md`](journal/2026-09-04-0-1-0-is-public.md) — devlog: what 0.1.0 shipped, what it left out, and the road to 7.2. - [`2026-09-24-a-kms-keyring.md`](journal/2026-09-24-a-kms-keyring.md) — devlog: the first real `WP_Secrets_Keyring`, the root-key cache and `rotate --from` it drove, and what it found. - [`2026-09-24-a-vault-provider.md`](journal/2026-09-24-a-vault-provider.md) — devlog: the Vault KV v2 example, the AWS site-scope bug it found, and what stayed open. +- [`2026-09-24-testing-the-cli-for-real.md`](journal/2026-09-24-testing-the-cli-for-real.md) — devlog: the WP-CLI smoke test, the root-key bug it found, and the last 🟡 coverage gap closing. - [`open-questions.md`](journal/open-questions.md) — what is still deliberately undecided. - [`proposal-questions.md`](journal/proposal-questions.md) — the five questions the proposal asked, and the answers so far. - [`test-coverage-gaps.md`](journal/test-coverage-gaps.md) — paths the suite cannot reach and what was verified by hand. diff --git a/docs/journal/2026-09-24-testing-the-cli-for-real.md b/docs/journal/2026-09-24-testing-the-cli-for-real.md new file mode 100644 index 0000000..3e94051 --- /dev/null +++ b/docs/journal/2026-09-24-testing-the-cli-for-real.md @@ -0,0 +1,91 @@ +--- +title: "Testing the CLI for real" +description: "A bash harness now drives a real wp binary against a throwaway install, closing the last coverage gap marked as needing an answer before the core patch." +date: 2026-09-24 +--- + +# Testing the CLI for real + +[`tests/smoke/smoke.sh`][smoke] exists now. It provisions a throwaway WordPress install under +`.smoke/`, with a pinned `wp-cli.phar`, and drives `wp secret` and `wp network-secret` through a +real `wp` process rather than calling `WP_CLI_Secret_Command`'s methods directly. `make ci` and +`bin/ci-local.sh` both run it, and CI now has a `smoke` job on PHP 7.4 and 8.3. [ADR 0008][adr8] +called this out as the one piece of pre-Trac work that tests the surface rather than waiting on +silence to become confirmation. + +## What was built + +Five cases. A: every subcommand on both `wp secret` and `wp network-secret` is registered, and +each `wp secret` subcommand's synopsis flags match an exact table, in both directions — a new flag +with no row and a row with no flag both fail loudly. B: the exit-code contract (0 found, 1 +absent, 2 broken), masking and `--reveal`, slots, every `list` format, `retire`, `delete`, +`generate-key`, +`import-option`, and `migrate-legacy --dry-run`. C: rotation end to end, with the previous key +moved into `wp-config.php` and a fresh key generated, then confirmed still decryptable and +reported healthy afterward. D: drop-in loading through the real loader — a syntax error, a thrown +exception, a wrong-type provider global, and a no-op drop-in, each checked against what +`wp secret dropin` reports and what `wp secret get` does. E: `wp core multisite-convert`, a second +site, and the site/network scope split — a site-2 secret is invisible from site 1, and a network +secret round-trips from both. + +Nothing under `src/` or `cli/` changed to make this pass, with one exception worth stating +plainly: `WP_Secrets_Key_Manager` did not preserve the root key across `wp core +multisite-convert` before this work started. Case E's first run failed on the network-secret +health check — every secret written before conversion became silently undecryptable, +contradicting what [`docs/spec/network.md`][network] already claimed. That was diagnosed and +fixed: the wrapped root key is adopted from the pre-conversion site option into `wp_sitemeta` on +the first post-conversion read. `convert_to_multisite` now asserts on it directly — a secret set +immediately before `wp core multisite-convert` is read back with `--reveal --field=value` right +after the network activates, and must still equal what was written. + +## What it found + +The concrete thing PHPUnit could not catch: on 4 September, `--version=previous` silently +returned the current value. This harness does not reproduce that symptom through a real `wp`. +WP-CLI 2.12.0 hands `--version` after a command through to the subcommand rather than consuming +it itself; what actually dropped the flag was `wp-env run`, before WP-CLI ever saw it. The flag +is `--slot` now regardless, since a subcommand-level flag should not share a name with a global +one WP-CLI does recognise (`wp --version`) even when that global flag is not the one eating it +here. Case A's synopsis check and case B's `get --slot=previous` assertion both pin the rename. +On the current tree, `get --version=previous` reaches `get()`, which declares `--slot` and not +`--version`, and WP-CLI rejects it: exit 1, `unknown --version parameter` on stderr. Case A +asserts both. Reintroducing `--version` by hand shows the other side: `get()` declares the flag +again, the real binary hands it `--version=previous`, and the previous value comes back. The +suite still fails, on the synopsis table, the `--version=previous` row, and both `--slot=previous` +rows, so it catches the rename, not the 4 September symptom. Separately, case E's first run +failed on the network-secret health check after `wp core multisite-convert`, because +`WP_Secrets_Key_Manager` did not preserve the root key across the conversion. That is fixed, and +`convert_to_multisite` now asserts directly that a secret set before conversion still decrypts +after it. + +Reintroducing the other two historical bugs by hand — deleting a `--format` description line, +dropping an `@subcommand` tag — still makes the suite fail for the reason each was supposed to. +That reintroduce-and-revert cycle is recorded in `tests/smoke/smoke.sh`'s own header comment and +in the commit that added it. + +Past that: every subcommand dispatches the way its docblock says, `list --format=*` behaves the +same across json, csv, and ids, and the multisite pass, once the root-key fix landed, round-trips +correctly on both sides of the site/network split. + +## What was deliberately left out + +`rotate --from` waits for `build/kms-keyring`, which is adding the flag; this suite does not test +a flag that does not exist yet. Byte-for-byte output comparison was skipped in favor of +exit-code and content assertions, because WP-CLI's table formatting is not part of this project's +contract. The AWS Secrets Manager and (forthcoming) Vault and KMS examples are exercised by their +own READMEs, not by this harness, since `examples/` is explicitly excluded from `make ci`. And the +uncatchable fatal — a drop-in class that implements an interface but omits a method — stays a +recorded, not desired, outcome: case D confirms the process exits non-zero with a PHP fatal on +stderr, but there is no userland way to turn that into a passing assertion instead of a documented +limit. + +## What it means for the Trac patch + +The CLI surface that ships with 7.2 now has an end-to-end test that `make ci` runs on every push, +on the PHP floor and the newest supported version. `docs/journal/test-coverage-gaps.md`'s only +🟡 entry — CLI dispatch untested — is gone; what is left there is 🟢 tracking only. That was the +condition ADR 0008 set before the Trac ticket opens. + +[smoke]: ../../tests/smoke/smoke.sh +[adr8]: ../decisions/0008-the-trac-ticket-replaces-thread-confirmation.md +[network]: ../spec/network.md diff --git a/docs/journal/test-coverage-gaps.md b/docs/journal/test-coverage-gaps.md index 612b4bb..26faf08 100644 --- a/docs/journal/test-coverage-gaps.md +++ b/docs/journal/test-coverage-gaps.md @@ -28,7 +28,7 @@ rather than overclaiming. --- -## 🟢 Drop-in file loading is not directly covered by an automated test +## 🟢 Drop-in file loading's uncatchable fatal remains outside automated coverage `wp_secrets_api_load_dropin()` (in `secrets-api.php`) runs once, during `wp_secrets_api_bootstrap()`, which itself runs once per PHP process via `muplugins_loaded`. Both @@ -36,7 +36,9 @@ that function and `_wp_secrets_get_store()` / `_wp_secrets_get_key_manager()` ca in a function-local `static` on first call, with no reset hook. By the time any test method's body runs — even in a `@runInSeparateProcess` test — the process's one bootstrap pass has already completed, so a drop-in file placed on disk from within a test body arrives too late to affect -that process's `wp_secrets_api_load_dropin()` call. +that process's `wp_secrets_api_load_dropin()` call. `tests/smoke/smoke.sh` case D works around this +the same way a real install does: it writes the drop-in to disk and then invokes a fresh `wp` +process, so `wp_secrets_api_load_dropin()`'s own `require`-and-`try`/`catch` runs for real. **What is covered:** the consumption side — `_wp_secrets_get_store()` and `_wp_secrets_get_key_manager()` correctly using `$GLOBALS['wp_secrets_store']` / @@ -44,64 +46,20 @@ that process's `wp_secrets_api_load_dropin()` call. `WP_Secrets_Broken_Keyring` when `$GLOBALS['wp_secrets_dropin_broken']` is set — is tested directly in `tests/phpunit/test-secrets-extension-points.php` by setting those same globals in an isolated process, exactly as a real drop-in would, before the first call that would cache a -default. - -**What is not covered:** `wp_secrets_api_load_dropin()`'s own `require`-and-`try`/`catch` around -an actual drop-in file. That behaviour was verified empirically instead, once, directly against -the PHP engine on both 7.4.33 and 8.5.7: - -- A syntax error in the required file *is* caught as a `ParseError` by `catch ( \Throwable $e )` - around the `require` — confirmed on both versions. -- A class that `implements` an interface but omits a required method is an **uncatchable fatal - error**, even inside that same `try`/`catch` — also confirmed on both versions. This is a - PHP-engine limitation, not a bug in the catching code: there is no userland way to intercept it. - -So a malformed drop-in fails safely for syntax errors and thrown exceptions, but a drop-in whose -class silently fails to fully implement its interface can still produce a fatal error page. Worth -another look if a process-spawning integration test is judged worth the complexity later. - - ---- - -## 🟡 CLI dispatch is not covered by any test - -Every WP-CLI test in this suite instantiates the command class and calls the method directly. That -covers the method bodies well and covers **nothing** about how WP-CLI actually reaches them, which -is a layer with its own rules: flag-name reservations, docblock synopsis parsing, and method-name -to subcommand-name mapping. - -Three real bugs lived there undetected until the commands were run end to end for the first -time, all with green tests throughout: - -- **`--version=previous` silently returned the current value.** WP-CLI consumes `--version` before - a subcommand sees it; the synopsis default then filled in `current`. The flag is now `--slot`. - This is the worst of the three: no error, just the wrong secret. -- **`--format` was rejected as an unknown parameter** on all four commands that declare it. WP-CLI - will not register a parameter whose synopsis block has no `: description` line, and every - `[--format=]` went straight into its `---` YAML. -- **`wp secret migrate-legacy` did not exist**, despite every document saying it did. WP-CLI - derives subcommand names from method names, so `migrate_legacy()` registered as - `migrate_legacy`. Fixed with `@subcommand`. - -**What would catch this:** a smoke test that shells out to a real `wp` binary against a real -install and asserts on exit codes and output — the WordPress test suite cannot do this, and -wp-env can. Not built. Until it is, treat any change to a command's docblock synopsis or method -name as untested, and run it by hand. - -Cheap interim discipline: `wp help secret ` shows the synopsis WP-CLI actually built. -If a flag is missing there, it is missing everywhere. `--from` on `wp secret rotate` was checked -this way by hand when it was generalised; the output is recorded in the body of the commit that added `--from`. - - ---- - -## 🟢 `wp secret set --stdin`'s own code path is not covered by an automated test - -`WP_CLI_Secret_Command::set()` reads `--stdin` via `file_get_contents( 'php://stdin' )`. Faking -that stream meaningfully from inside a PHPUnit process would need a real pipe (`proc_open`), and -getting it wrong risks hanging the whole test run waiting on a stream nothing is writing to — not -worth it for one branch. Every other branch of `set()` (positional value, missing value, the -shell-history warning, success/porcelain/error reporting) is covered directly. +default. `wp_secrets_api_load_dropin()`'s own `require`-and-`try`/`catch` around an actual drop-in +file is now exercised through the real loader by `tests/smoke/smoke.sh` case D, which writes each +of a syntax-error drop-in, a throw-on-load drop-in, a wrong-type provider-global drop-in, and a +sets-nothing drop-in, and asserts on `wp secret get`'s exit code and `wp secret dropin`'s report +for each. + +**What is not covered:** the one case case D records rather than asserts as desired — a class that +`implements` an interface but omits a required method is an **uncatchable fatal error**, even +inside that same `try`/`catch`. This is a PHP-engine limitation, not a bug in the catching code: +there is no userland way to intercept it, so there is nothing case D's assertions could turn green +for. Verified empirically, once, directly against the PHP engine on both 7.4.33 and 8.5.7 (before +case D existed), and confirmed again end to end by case D against the smoke suite's own PHP 7.4.33: +the run exits non-zero with a PHP fatal on stderr, recorded as expected behaviour rather than +desired. Worth another look if a future PHP makes this catchable. --- diff --git a/docs/reference/ci.md b/docs/reference/ci.md index ecdb0cd..8074db5 100644 --- a/docs/reference/ci.md +++ b/docs/reference/ci.md @@ -16,6 +16,7 @@ make analyse # phpstan make test # phpunit, single site make test-ms # phpunit, multisite make reference-check # docs/reference/ matches the source docblocks +make smoke # WP-CLI end to end against a throwaway install in .smoke/ make ci # all of the above ``` @@ -45,6 +46,24 @@ make ci `bin/install-wp-tests.sh` takes ` [db-host] [wp-version]` and honours `WP_TESTS_DIR` and `WP_CORE_DIR`. +### The WP-CLI smoke test + +`make smoke` (`bin/smoke-install.sh` then `tests/smoke/smoke.sh`) is a bash harness that drives a +real, pinned `wp-cli.phar` against a real WordPress install it provisions itself under `.smoke/` — +never wp-env's own `wp-content`, never the PHPUnit suite's database. It exists because every other +test in this repository instantiates `WP_CLI_Secret_Command` directly and calls its methods, +which covers nothing about how WP-CLI actually dispatches to them; see +[`docs/journal/2026-09-24-testing-the-cli-for-real.md`](../journal/2026-09-24-testing-the-cli-for-real.md) +and `tests/smoke/SPEC.md`. + +Variables it reads: `SMOKE_DB_NAME` (defaults to `wordpress_smoke`, and refuses to run if set to +`wordpress_test`, which belongs to the PHPUnit suite), `DB_USER`, `DB_PASS`, `DB_HOST`, +`WP_VERSION`, and `SMOKE_URL`. It needs egress to GitHub releases (the `wp-cli.phar` download) and +wordpress.org (`wp core download`). Through `bin/ci-local.sh` it runs inside wp-env's `cli` +container, against wp-env's own development MySQL rather than a separate service. The install +under `.smoke/` is disposable: `bin/smoke-install.sh` drops and recreates its database and +overwrites `wp-config.php` on every run. + ## Air-gapped and mirrored environments `bin/install-wp-tests.sh` reads two environment variables so it never has to reach @@ -98,6 +117,8 @@ Each SHA in the workflow came from the GitHub API at the version noted beside it copied out of documentation or a README. An unverified pin is really a pin to whatever the last person pasted. +`wp-cli.phar` is pinned the same way, by version and SHA-256, in `bin/smoke-install.sh`. + ## Matrix | Job | PHP | WordPress | Notes | @@ -105,9 +126,9 @@ person pasted. | `static` | 8.3 | — | lint + compat + analyse. Gates everything else. | | `test` | 7.4, 8.0, 8.3 | latest, trunk | Single site | | `test-multisite` | 8.3 | latest | Multisite suite | -| `examples` | 8.3 | latest | `make test-examples` against a Moto (AWS emulator) service container, pinned by digest. Not part of `make ci`. | +| `smoke` | 7.4, 8.3 | latest | WP-CLI end to end, single site then multisite | +| `examples` | 8.3 | latest | `make test-examples` against Moto (an AWS emulator) and a Vault dev server, both pinned by digest, single site then multisite. Not part of `make ci`. | | `reference-docs` | 8.3 | — | `bin/gen-reference.php --check`: the committed docs/reference/ matches the source. No Composer install. | -| `examples` | 8.3 | latest | `make test-examples` against a Vault dev-mode service container, single site and multisite. Outside `make ci` because it needs the container. | The 7.4 leg is not optional. Core's floor is 7.4 and `src/` must run there; PHPCompatibilityWP catches syntax statically, but only a running 7.4 catches runtime behaviour differences. diff --git a/docs/reference/wp-cli.md b/docs/reference/wp-cli.md index 68485e8..bd662f7 100644 --- a/docs/reference/wp-cli.md +++ b/docs/reference/wp-cli.md @@ -84,7 +84,7 @@ wp network-secret get [--slot=] [--reveal] [--field=] [--for | Option | Description | |---|---| | `` | The secret's namespaced name. | -| `[--slot=]` | Which stored version to read. Named --slot rather than --version because WP-CLI consumes `--version` itself before a subcommand ever sees it: passing --version=previous silently yielded the current value, since the flag was swallowed and the synopsis default filled in behind it. Default: `current`. Options: `current`, `previous`. | +| `[--slot=]` | Which stored version to read. Named --slot rather than --version to avoid the value being dropped by wrappers such as `wp-env run`, which consume --version themselves (the original bug), and to avoid confusion with `wp --version`. Default: `current`. Options: `current`, `previous`. | | `[--reveal]` | Show the actual value. Without this, it is masked. | | `[--field=]` | Print a single field (name, fingerprint, value) instead of a table. | | `[--format=]` | Render output in a particular format. Default: `table`. Options: `table`, `csv`, `json`. | @@ -323,7 +323,7 @@ wp secret get [--slot=] [--reveal] [--field=] [--format=` | The secret's namespaced name. | -| `[--slot=]` | Which stored version to read. Named --slot rather than --version because WP-CLI consumes `--version` itself before a subcommand ever sees it: passing --version=previous silently yielded the current value, since the flag was swallowed and the synopsis default filled in behind it. Default: `current`. Options: `current`, `previous`. | +| `[--slot=]` | Which stored version to read. Named --slot rather than --version to avoid the value being dropped by wrappers such as `wp-env run`, which consume --version themselves (the original bug), and to avoid confusion with `wp --version`. Default: `current`. Options: `current`, `previous`. | | `[--reveal]` | Show the actual value. Without this, it is masked. | | `[--field=]` | Print a single field (name, fingerprint, value) instead of a table. | | `[--format=]` | Render output in a particular format. Default: `table`. Options: `table`, `csv`, `json`. | diff --git a/docs/spec/extension-points.md b/docs/spec/extension-points.md index d659928..1e270ed 100644 --- a/docs/spec/extension-points.md +++ b/docs/spec/extension-points.md @@ -171,8 +171,10 @@ There is one gap worth knowing about. PHP treats some class declaration errors i an uncatchable fatal, even inside the `try`/`catch` around the `require`. The usual culprit is a class that `implements` an interface but omits one of its methods. Userland cannot intercept that, and it takes down the whole request rather than failing in a contained way. Run `php -l` over a -drop-in and load a real request before trusting it in production. See -[`test-coverage-gaps.md`](../journal/test-coverage-gaps.md), "Drop-in file loading". +drop-in and load a real request before trusting it in production. `tests/smoke/smoke.sh` case D +checks the syntax-error, throw-on-load, and wrong-type cases through the real loader; only the +fatal remains a manual check. See [`test-coverage-gaps.md`](../journal/test-coverage-gaps.md), +"Drop-in file loading". **Error codes.** Every `WP_Error` this API returns uses one of a fixed set of codes, defined in `src/wp-includes/secrets.php`: diff --git a/docs/spec/network.md b/docs/spec/network.md index 8e33f0c..0d91f22 100644 --- a/docs/spec/network.md +++ b/docs/spec/network.md @@ -34,6 +34,10 @@ Both derive from the single root key with `sodium_crypto_kdf_derive_from_key( 32 start at 1 and the context strings differ, so the network subkey cannot collide with a site subkey. The root key is stored via `get_site_option()` / `add_site_option()`, which is `wp_sitemeta` on multisite and `wp_options` on a single site. Master keys are never stored. +Converting a single site to a network leaves the wrapped root key in the former main site's +options row, since conversion does not move it; the first `get_site_option()` miss on the new +network falls back to that row, adopts it into `wp_sitemeta`, and removes the stranded copy, so +secrets written before conversion keep decrypting. **Storage.** `WP_Secrets_Option_Store` in `src/wp-includes/class-wp-secrets-option-store.php` uses the prefix `_wp_secret_` with `get_option()` for site scope and `_wp_network_secret_` with diff --git a/docs/spec/retrieval.md b/docs/spec/retrieval.md index 3a692ea..741646c 100644 --- a/docs/spec/retrieval.md +++ b/docs/spec/retrieval.md @@ -45,6 +45,8 @@ wrong type, `wp_secrets_api_load_dropin()` in `secrets-api.php` sets `WP_SECRETS_ERROR_STORE_UNAVAILABLE`. There is no fallback to the default provider. `tests/phpunit/test-secrets-three-state-contract.php` and `tests/phpunit/test-secrets-broken-dropin-fallbacks.php` cover this. +`tests/smoke/smoke.sh` case D checks the same fail-closed behaviour through the real loader and +the exit-code contract (2, not 1). **No filter.** There is no `apply_filters()` call anywhere under `src/`, not only on the retrieval path. `test_no_apply_filters_anywhere_in_src()` in `tests/phpunit/test-architecture.php` reads diff --git a/docs/spec/scope.md b/docs/spec/scope.md index 90a0c05..01239bb 100644 --- a/docs/spec/scope.md +++ b/docs/spec/scope.md @@ -52,7 +52,8 @@ the named surface, all in `src/wp-includes/secrets.php` unless noted: `wp secret` with `set`, `get`, `delete`, `list`, `retire`, `import-option`, `migrate-legacy`, `rotate`, `generate-key`, `health`, and `dropin`. `WP_CLI_Secret_Network_Command` registers `wp network-secret` with the same subcommands for network scope. Both load only when `WP_CLI` is -defined and true. +defined and true. Both are exercised end to end against a real `wp` binary by +`tests/smoke/smoke.sh`, on single site and multisite. **Site Health.** Three tests (`secrets_api_key_source`, undecryptable secrets, `secrets_api_needs_rotation`) and a debug-information section, in diff --git a/phpcs.xml.dist b/phpcs.xml.dist index 7ced480..a6906da 100644 --- a/phpcs.xml.dist +++ b/phpcs.xml.dist @@ -15,6 +15,8 @@ /examples/* /site/* + + /.smoke/* diff --git a/src/wp-includes/class-wp-secrets-key-manager.php b/src/wp-includes/class-wp-secrets-key-manager.php index 2528a83..db65b6e 100644 --- a/src/wp-includes/class-wp-secrets-key-manager.php +++ b/src/wp-includes/class-wp-secrets-key-manager.php @@ -200,7 +200,7 @@ public function get_master_key( $scope, $site_id = null ) { * @return string|WP_Error 32 raw bytes on success. WP_Error on failure. */ public function get_root_key() { - $wrapped = get_site_option( self::ROOT_KEY_OPTION ); + $wrapped = $this->get_wrapped_root_key(); if ( false === $wrapped ) { return $this->generate_root_key(); @@ -242,7 +242,7 @@ public function get_root_key() { * @return true|WP_Error */ public function rotate_site_key( WP_Secrets_Keyring $old_keyring, WP_Secrets_Keyring $new_keyring ) { - $wrapped = get_site_option( self::ROOT_KEY_OPTION ); + $wrapped = $this->get_wrapped_root_key(); if ( ! is_string( $wrapped ) ) { return new WP_Error( @@ -294,6 +294,42 @@ public function rotate_site_key( WP_Secrets_Keyring $old_keyring, WP_Secrets_Key return true; } + /** + * Returns the wrapped root key, adopting a pre-conversion copy if needed. + * + * A single site converted to multisite keeps its wrapped root key in its own + * (now main site's) options row: `update_site_option()` only starts writing to + * sitemeta once `is_multisite()` is true, and the conversion process does not + * move existing option rows. Without this, a network's first read would find no + * sitemeta row and mint a brand new root key, stranding every secret written + * before conversion. + * + * @since 7.2.0 + * + * @return string|false The wrapped root key, or false if none exists anywhere. + */ + private function get_wrapped_root_key() { + $wrapped = get_site_option( self::ROOT_KEY_OPTION ); + + if ( false !== $wrapped || ! is_multisite() ) { + return $wrapped; + } + + $main_site_id = get_main_site_id(); + $legacy = get_blog_option( $main_site_id, self::ROOT_KEY_OPTION ); + + if ( ! is_string( $legacy ) ) { + return false; + } + + if ( add_site_option( self::ROOT_KEY_OPTION, $legacy ) ) { + delete_blog_option( $main_site_id, self::ROOT_KEY_OPTION ); + } + + // Re-read: handles a concurrent request that already adopted it. + return get_site_option( self::ROOT_KEY_OPTION ); + } + /** * Generates a new root key and persists it. * diff --git a/tests/phpunit/test-wp-secrets-key-manager.php b/tests/phpunit/test-wp-secrets-key-manager.php index 84c330e..0a763ed 100644 --- a/tests/phpunit/test-wp-secrets-key-manager.php +++ b/tests/phpunit/test-wp-secrets-key-manager.php @@ -168,6 +168,90 @@ public function test_site_scope_master_key_differs_across_blogs() { $this->assertNotSame( $site_key_on_blog_1, $site_key_on_blog_2 ); } + /** + * Simulates single-site-to-multisite conversion: moves the wrapped root key + * from its main-site option row into sitemeta, the way a fresh + * WP_Secrets_Key_Manager finds it after `wp core multisite-convert`. + * + * Requires the multisite suite; skips under single-site since sitemeta and + * the main site's options are the same table there. + */ + public function test_get_root_key_adopts_a_pre_conversion_root_key() { + if ( ! is_multisite() ) { + $this->markTestSkipped( 'Requires multisite: proves the main-site option row is adopted.' ); + } + + $manager = new WP_Secrets_Key_Manager(); + $root_key = $manager->get_root_key(); + + $main_site_id = get_main_site_id(); + $wrapped = get_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION ); + + delete_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION ); + update_blog_option( $main_site_id, WP_Secrets_Key_Manager::ROOT_KEY_OPTION, $wrapped ); + + $fresh_manager = new WP_Secrets_Key_Manager(); + $adopted_key = $fresh_manager->get_root_key(); + + $this->assertTrue( hash_equals( $root_key, $adopted_key ) ); + $this->assertNotFalse( get_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION ) ); + $this->assertFalse( get_blog_option( $main_site_id, WP_Secrets_Key_Manager::ROOT_KEY_OPTION ) ); + } + + /** + * Requires the multisite suite; skips under single-site for the same reason + * as the adoption test above. + */ + public function test_secret_written_before_conversion_still_decrypts() { + if ( ! is_multisite() ) { + $this->markTestSkipped( 'Requires multisite: proves secrets survive conversion.' ); + } + + $main_site_id = get_main_site_id(); + + $this->assertTrue( wp_set_secret( 'secrets-api/pre-conversion', 'shh' ) ); + + $wrapped = get_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION ); + delete_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION ); + update_blog_option( $main_site_id, WP_Secrets_Key_Manager::ROOT_KEY_OPTION, $wrapped ); + + $secret = wp_get_secret( 'secrets-api/pre-conversion' ); + + $this->assertNotWPError( $secret ); + $this->assertSame( 'shh', $secret->reveal() ); + } + + /** + * Requires the multisite suite; skips under single-site for the same reason + * as the adoption test above. + */ + public function test_rotate_site_key_works_after_conversion_before_any_read() { + if ( ! is_multisite() ) { + $this->markTestSkipped( 'Requires multisite: proves rotation adopts a pre-conversion key too.' ); + } + + $main_site_id = get_main_site_id(); + $right_keyring = new WP_Secrets_Config_Key_Provider(); + $manager = new WP_Secrets_Key_Manager( $right_keyring ); + + $manager->get_root_key(); + $master_before = $manager->get_master_key( 'site' ); + + $wrapped = get_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION ); + delete_site_option( WP_Secrets_Key_Manager::ROOT_KEY_OPTION ); + update_blog_option( $main_site_id, WP_Secrets_Key_Manager::ROOT_KEY_OPTION, $wrapped ); + + $fresh_manager = new WP_Secrets_Key_Manager( $right_keyring ); + $rotated = $fresh_manager->rotate_site_key( $right_keyring, $right_keyring ); + + $this->assertNotWPError( $rotated ); + $this->assertTrue( $rotated ); + + $master_after = $fresh_manager->get_master_key( 'site' ); + + $this->assertSame( $master_before, $master_after ); + } + public function test_rotate_fails_when_no_root_key_exists() { $manager = new WP_Secrets_Key_Manager(); $keyring = new WP_Secrets_Config_Key_Provider(); diff --git a/tests/smoke/SPEC.md b/tests/smoke/SPEC.md index 1e4a153..8cdbd18 100644 --- a/tests/smoke/SPEC.md +++ b/tests/smoke/SPEC.md @@ -1,7 +1,9 @@ # Spec: WP-CLI smoke test -Status: planned. Part of the pre-Trac work in -[ADR 0008](../../docs/decisions/0008-the-trac-ticket-replaces-thread-confirmation.md). +Status: built. Part of the pre-Trac work in +[ADR 0008](../../docs/decisions/0008-the-trac-ticket-replaces-thread-confirmation.md). See +[`docs/journal/2026-09-24-testing-the-cli-for-real.md`](../../docs/journal/2026-09-24-testing-the-cli-for-real.md) +for the journal entry. ## Why diff --git a/tests/smoke/smoke.sh b/tests/smoke/smoke.sh new file mode 100755 index 0000000..d0db9a0 --- /dev/null +++ b/tests/smoke/smoke.sh @@ -0,0 +1,728 @@ +#!/usr/bin/env bash +# +# WP-CLI smoke test: drives a real `wp` binary against the throwaway install +# bin/smoke-install.sh provisions, to cover subcommand and flag registration, +# the 0/1/2 exit-code contract, rotation end to end, drop-in loading through +# the real loader, and a multisite pass. See tests/smoke/SPEC.md. +# +# Requires bin/smoke-install.sh to have already provisioned .smoke/wordpress/. +# This script never touches wp-env's own wp-content: it only ever reaches +# into .smoke/, the install the install script owns. +# +# Regression proof: each of the three historical CLI dispatch bugs was +# reintroduced by hand in cli/class-wp-cli-secret-command.php, run against +# this suite, confirmed to fail, and reverted (git checkout --). Nothing +# below is a standing test for the bug's absence beyond the assertions +# already in cases A and B; this block records which of those assertions is +# the one that catches each bug. +# +# 1. Renaming get()'s `[--slot=]` synopsis entry (and the +# $assoc_args['slot'] reads) back to `[--version=]` -- +# reproducing the original defect. Caught by "secret get synopsis +# flags match the table" and the "--version=previous does not select +# the previous slot" / "get --version=previous exits 1" / +# "WP-CLI rejects --version as an undeclared get parameter" rows (case +# A), and by "get --slot=previous exits 0" / "get --slot=previous +# returns the demoted value (bug 1, end to end)" (case B). Through the +# real binary, --version=previous then reaches get() and selects the +# previous slot: WP-CLI 2.12.0 passes --version after a command +# through to the subcommand. The 4 September symptom, seen through +# `wp-env run`, came from that wrapper dropping the flag before +# WP-CLI ever saw it, not from WP-CLI itself consuming --version; the +# rename to --slot is what these rows pin, not the wrapper's +# behaviour. +# 2. Deleting the `: Render output in a particular format.` description +# line under list()'s `[--format=]` -- WP-CLI drops a +# parameter that has no description. Caught by "secret list synopsis +# flags match the table" (case A) and by every list --format=* +# assertion in case B (json, csv, fields, namespace/ids), since the +# flag no longer registers at all. +# 3. Deleting the `@subcommand migrate-legacy` tag -- the method reverts +# to WP-CLI's default subcommand name derived from the PHP method name +# (migrate_legacy, with an underscore). Caught by "secret migrate-legacy +# is registered" and "network-secret migrate-legacy is registered" +# (case A registration loop), by "secret migrate-legacy synopsis flags +# match the table" (case A), and by "migrate-legacy --dry-run exits 0 +# with no prototype rows" (case B). +# +set -euo pipefail +cd "$(dirname "$0")/../.." + +# --- configuration --- + +SMOKE_DIR="$PWD/.smoke" +SMOKE_URL="${SMOKE_URL:-http://smoke.test}" +export WP_CLI_CACHE_DIR="$SMOKE_DIR/cache" PAGER=cat WP_CLI_PAGER=cat + +WP=( php -d display_errors=stderr -d log_errors=0 "$SMOKE_DIR/wp-cli.phar" --path="$SMOKE_DIR/wordpress" --allow-root ) + +NS="smoke-$$" +DROPIN="$SMOKE_DIR/wordpress/wp-content/secrets.php" + +# One row per subcommand, "sub:flags". A new flag without a row fails the +# run, and so does a flag that vanishes from the synopsis: the comparison +# below is an exact set match in both directions, not a subset check. +EXPECTED_FLAGS=" +set:--stdin --porcelain +get:--slot --reveal --field --format +delete:--yes +list:--namespace --fields --field --format +retire:--yes +import-option: +migrate-legacy:--dry-run --name --map --namespace --format +rotate:--from --yes +generate-key: +health:--format +dropin:--verbose +" + +N=0 +PASS=0 +FAIL=0 +ERR_FILE="$(mktemp)" + +if [ ! -f "$SMOKE_DIR/wp-cli.phar" ] || [ ! -f "$SMOKE_DIR/wordpress/wp-config.php" ]; then + echo "No smoke install found at $SMOKE_DIR. Run bin/smoke-install.sh first." >&2 + exit 1 +fi + +# --- helpers --- + +# ok "": record and print a passing TAP line. +ok() { + N=$((N + 1)) + PASS=$((PASS + 1)) + printf 'ok %d - %s\n' "$N" "$1" +} + +# not_ok "" "": record and print a failing TAP line, with an +# optional one-line diagnostic. Never prints OUT, a value, a key, or a +# command line. +not_ok() { + N=$((N + 1)) + FAIL=$((FAIL + 1)) + printf 'not ok %d - %s\n' "$N" "$1" + if [ -n "${2:-}" ]; then + printf '# %s\n' "$2" + fi +} + +# diag "": print a single-line comment, for use inside a diagnostic. +diag() { + printf '# %s\n' "$1" +} + +# run : execute a command, capturing stdout into OUT (trailing +# newlines stripped), stderr into ERR (via ERR_FILE), and the exit status +# into STATUS. Never prints anything itself. +run() { + STATUS=0 + OUT="$("$@" 2>"$ERR_FILE")" || STATUS=$? + ERR="$(cat "$ERR_FILE")" +} + +# run_stdin : same contract as run(), but the command's +# stdin comes from rather than the harness's own stdin. Used for +# `set --stdin`, which cannot be piped into safely from inside a function +# that also needs to capture stdout. +run_stdin() { + local input_file="$1" + shift + STATUS=0 + OUT="$("$@" <"$input_file" 2>"$ERR_FILE")" || STATUS=$? + ERR="$(cat "$ERR_FILE")" +} + +# assert_status "": pass when the last run()'s STATUS equals . +assert_status() { + local expected="$1" desc="$2" + if [ "$STATUS" -eq "$expected" ]; then + ok "$desc" + else + not_ok "$desc" "exit status was $STATUS, expected $expected: $desc" + fi +} + +# assert_out_eq "" "": pass when the last run()'s OUT equals +# . The diagnostic reports only the two lengths, never the values. +assert_out_eq() { + local expected="$1" desc="$2" + if [ "$OUT" = "$expected" ]; then + ok "$desc" + else + not_ok "$desc" "output length ${#OUT} did not match expected length ${#expected}: $desc" + fi +} + +# assert_out_contains "" "": pass when OUT contains . +assert_out_contains() { + local needle="$1" desc="$2" + case "$OUT" in + *"$needle"*) ok "$desc" ;; + *) not_ok "$desc" "output did not contain expected text: $desc" ;; + esac +} + +# assert_out_not_contains "" "": pass when OUT does not +# contain . +assert_out_not_contains() { + local needle="$1" desc="$2" + case "$OUT" in + *"$needle"*) not_ok "$desc" "output unexpectedly contained the given text: $desc" ;; + *) ok "$desc" ;; + esac +} + +# assert_err_contains "" "": pass when ERR contains . +# A diagnostic may print the exit status, the description, and at most the +# first three lines of ERR. +assert_err_contains() { + local needle="$1" desc="$2" + case "$ERR" in + *"$needle"*) ok "$desc" ;; + *) + not_ok "$desc" "status=$STATUS desc=$desc$(printf '\n'; printf '%s\n' "$ERR" | head -n 3)" + ;; + esac +} + +# normalize_flags "": one space-separated set of --flag tokens, +# sorted and de-duplicated, for an exact-set comparison. +normalize_flags() { + printf '%s' "$1" | tr ' ' '\n' | sed '/^$/d' | sort -u | tr '\n' ' ' | sed 's/ $//' +} + +# synopsis_flags : runs `wp help secret `, keeps only the lines +# between the SYNOPSIS header and the next header line (a line starting +# with an upper-case letter in column 1 -- WP-CLI word-wraps long +# synopses onto indented continuation lines, so every line in the section +# is taken), extracts --flag tokens, and prints them normalised. +synopsis_flags() { + local sub="$1" section + run "${WP[@]}" help secret "$sub" + section=$(printf '%s\n' "$OUT" | awk ' + /^SYNOPSIS/ { in_section = 1; next } + in_section && /^[A-Z]/ { exit } + in_section { print } + ') + printf '%s' "$(printf '%s\n' "$section" | grep -o -- '--[a-zA-Z][a-zA-Z-]*' | sort -u | tr '\n' ' ' | sed 's/ $//' || true)" +} + +# write_dropin: reads the drop-in body from stdin, writes it to $DROPIN, and +# records that this run wrote it, so the EXIT trap can clean it up even if +# the run aborts before case D's own cleanup runs. +WROTE_DROPIN=0 +write_dropin() { + cat >"$DROPIN" + WROTE_DROPIN=1 +} + +# remove_dropin: deletes $DROPIN if this run wrote it, and clears the flag. +remove_dropin() { + if [ "$WROTE_DROPIN" -eq 1 ]; then + rm -f "$DROPIN" + WROTE_DROPIN=0 + fi +} + +# finish: EXIT trap. Removes any drop-in this run wrote, prints the TAP plan +# and summary, and exits non-zero if anything failed. +finish() { + local exit_status=$? + remove_dropin + rm -f "$ERR_FILE" + printf '1..%d\n' "$N" + printf '# passed %d, failed %d\n' "$PASS" "$FAIL" + if [ "$FAIL" -ne 0 ] || [ "$exit_status" -ne 0 ]; then + exit 1 + fi + exit 0 +} +trap finish EXIT + +# --- case A: registration --- + +case_a_registration() { + # 11 subcommands, one row each: import-option, migrate-legacy, and + # generate-key by their hyphenated names. + local subcommands="set get delete list retire import-option migrate-legacy rotate generate-key health dropin" + local cmd sub + + for cmd in secret network-secret; do + for sub in $subcommands; do + run "${WP[@]}" cli has-command "$cmd $sub" + assert_status 0 "$cmd $sub is registered" + done + done + + # Flag table: checked for `secret` only, as the detailed spec words it. + # Exact-set comparison in both directions, so a new flag without a row + # fails loudly and a flag that vanishes from the synopsis fails loudly + # too. + local row expected_sub expected_flags actual_flags + local saved_ifs="$IFS" + IFS=' +' + for row in $EXPECTED_FLAGS; do + [ -n "$row" ] || continue + expected_sub="${row%%:*}" + expected_flags="${row#*:}" + actual_flags=$(synopsis_flags "$expected_sub") + if [ "$(normalize_flags "$actual_flags")" = "$(normalize_flags "$expected_flags")" ]; then + ok "secret $expected_sub synopsis flags match the table" + else + not_ok "secret $expected_sub synopsis flags match the table" \ + "expected [$(normalize_flags "$expected_flags")] got [$(normalize_flags "$actual_flags")]" + fi + done + IFS="$saved_ifs" + + # Pin WP-CLI's side of bug 1, not only its fix: `get` declares --slot, not + # --version, so --version=previous is passed through to get() by + # WP-CLI (which hands --version after a command to the subcommand) and + # rejected there as an undeclared parameter. This is already implied by + # the exact-set check above (`get`'s row has no --version), stated here + # explicitly and end to end. + run "${WP[@]}" secret set "${NS}/slotpin" "smoke-value-a-$$" + assert_status 0 "set ${NS}/slotpin to value a" + run "${WP[@]}" secret set "${NS}/slotpin" "smoke-value-b-$$" + assert_status 0 "set ${NS}/slotpin to value b" + run "${WP[@]}" secret get "${NS}/slotpin" --version=previous --reveal --field=value + assert_out_not_contains "smoke-value-a-$$" "--version=previous does not select the previous slot" + assert_status 1 "get --version=previous exits 1: WP-CLI passes the flag to get, which does not declare it" + assert_err_contains "unknown --version parameter" "WP-CLI rejects --version as an undeclared get parameter" + run "${WP[@]}" secret delete "${NS}/slotpin" --yes + assert_status 0 "delete ${NS}/slotpin" +} + +# --- case B: behaviour and exit codes --- + +case_b_behaviour() { + local v1="smoke-value-one-$$" v2="smoke-value-two-$$" + local s="${NS}/basic" + local stdin_file json_file + + # 1. set with a positional value. + run "${WP[@]}" secret set "$s" "$v1" + assert_status 0 "set with a positional value exits 0" + + # 2. set --stdin. + stdin_file="$(mktemp)" + printf '%s\n' "$v1" >"$stdin_file" + run_stdin "$stdin_file" "${WP[@]}" secret set "${NS}/stdin" --stdin + assert_status 0 "set --stdin from a pipe exits 0" + rm -f "$stdin_file" + + run "${WP[@]}" secret get "${NS}/stdin" --reveal --field=value + assert_status 0 "get ${NS}/stdin exits 0" + assert_out_eq "$v1" "set --stdin stores the piped value with the trailing newline trimmed" + + # 3. set --porcelain. + run "${WP[@]}" secret set "${NS}/porcelain" "$v1" --porcelain + assert_status 0 "set --porcelain exits 0" + local porcelain_out="$OUT" + if [ "$(printf '%s' "$porcelain_out" | wc -l)" -eq 0 ]; then + ok "set --porcelain prints exactly one line" + else + not_ok "set --porcelain prints exactly one line" "output contained more than one line" + fi + run "${WP[@]}" secret get "${NS}/porcelain" --field=fingerprint + assert_out_eq "$porcelain_out" "set --porcelain prints only the fingerprint" + + # 4. masking, --reveal, and the table. + run "${WP[@]}" secret get "$s" + assert_status 0 "get $s exits 0" + assert_out_not_contains "$v1" "get masks the value by default" + + run "${WP[@]}" secret get "$s" --reveal --field=value + assert_status 0 "get $s --reveal --field=value exits 0" + assert_out_eq "$v1" "get --reveal --field=value prints exactly the value" + + run "${WP[@]}" secret get "$s" --reveal + assert_status 0 "get $s --reveal exits 0" + assert_out_contains "$v1" "get --reveal shows the value in the table" + + # 5. slots: set a second value, previous must still be the first. + run "${WP[@]}" secret set "$s" "$v2" + assert_status 0 "set $s to a second value exits 0" + + run "${WP[@]}" secret get "$s" --slot=previous --reveal --field=value + assert_status 0 "get --slot=previous exits 0" + assert_out_eq "$v1" "get --slot=previous returns the demoted value (bug 1, end to end)" + + run "${WP[@]}" secret get "$s" --reveal --field=value + assert_status 0 "get $s current slot exits 0" + assert_out_eq "$v2" "get with no --slot returns the current value" + + # 6. --format=json. + run "${WP[@]}" secret get "$s" --format=json + assert_status 0 "get --format=json exits 0" + json_file="$(mktemp)" + printf '%s' "$OUT" >"$json_file" + run php -r 'exit( null === json_decode( file_get_contents( $argv[1] ), true ) ? 1 : 0 );' -- "$json_file" + assert_status 0 "get --format=json is valid JSON" + run php -r ' + $rows = json_decode( file_get_contents( $argv[1] ), true ); + if ( ! is_array( $rows ) || 1 !== count( $rows ) ) { + exit( 1 ); + } + echo $rows[0]["name"]; + ' -- "$json_file" + assert_out_eq "$s" "get --format=json decodes to exactly one row named $s" + rm -f "$json_file" + + # 1. list: a second namespace, JSON/CSV/fields/namespace filters, and + # never a value. + run "${WP[@]}" secret set "${NS}-b/other" "$v1" + assert_status 0 "set ${NS}-b/other exits 0" + + run "${WP[@]}" secret list + assert_status 0 "list exits 0" + assert_out_not_contains "$v1" "list never shows a value" + assert_out_not_contains "$v2" "list never shows a value (second value)" + + run "${WP[@]}" secret list --format=json + assert_status 0 "list --format=json exits 0" + assert_out_not_contains "$v1" "list --format=json never shows a value" + assert_out_not_contains "$v2" "list --format=json never shows a value (second value)" + json_file="$(mktemp)" + printf '%s' "$OUT" >"$json_file" + run php -r 'exit( null === json_decode( file_get_contents( $argv[1] ), true ) ? 1 : 0 );' -- "$json_file" + assert_status 0 "list --format=json is valid JSON" + rm -f "$json_file" + + run "${WP[@]}" secret list --format=csv + assert_status 0 "list --format=csv exits 0" + case "$OUT" in + name,*) ok "list --format=csv starts with a name column" ;; + *) not_ok "list --format=csv starts with a name column" "first bytes of output did not start with name," ;; + esac + assert_out_not_contains "$v1" "list --format=csv never shows a value" + assert_out_not_contains "$v2" "list --format=csv never shows a value (second value)" + + run "${WP[@]}" secret list --fields=name,fingerprint --format=csv + assert_status 0 "list --fields=name,fingerprint --format=csv exits 0" + local first_line + first_line=$(printf '%s\n' "$OUT" | head -n 1) + if [ "$first_line" = "name,fingerprint" ]; then + ok "list --fields=name,fingerprint --format=csv header matches exactly" + else + not_ok "list --fields=name,fingerprint --format=csv header matches exactly" "header line did not match name,fingerprint" + fi + + run "${WP[@]}" secret list --namespace="$NS" --format=ids + assert_status 0 "list --namespace=$NS --format=ids exits 0" + assert_out_contains "$s" "list --namespace returns names in the namespace" + assert_out_not_contains "${NS}-b/other" "list --namespace filters on the namespace prefix" + assert_out_not_contains "$v1" "list never shows a value" + assert_out_not_contains "$v2" "list never shows a value (second value)" + + # 2. retire. + run "${WP[@]}" secret retire "$s" --yes + assert_status 0 "retire $s --yes exits 0" + run "${WP[@]}" secret get "$s" --slot=previous + assert_status 1 "get --slot=previous exits 1 after retire" + run "${WP[@]}" secret get "$s" + assert_status 0 "get $s still exits 0 after retire" + + # 3. delete. + run "${WP[@]}" secret delete "$s" --yes + assert_status 0 "delete $s --yes exits 0" + run "${WP[@]}" secret get "$s" + assert_status 1 "get exits 1 after delete" + + # 4. absence and caller error. + run "${WP[@]}" secret get "${NS}/never-set" + assert_status 1 "get of a name never set exits 1" + + run "${WP[@]}" secret set "${NS}/no-value" + if [ "$STATUS" -ne 0 ]; then + ok "set with no value exits non-zero" + else + not_ok "set with no value exits non-zero" "exit status was 0" + fi + + # 5. generate-key. + run "${WP[@]}" secret generate-key + assert_status 0 "generate-key exits 0" + if [ "${#OUT}" -eq 44 ]; then + ok "generate-key prints 44 characters" + else + not_ok "generate-key prints 44 characters" "output length was ${#OUT}, expected 44" + fi + run php -r 'echo strlen( (string) base64_decode( $argv[1], true ) );' -- "$OUT" + assert_out_eq "32" "generate-key decodes to exactly 32 bytes" + + # 6. health and dropin. + run "${WP[@]}" secret health --format=json + assert_status 0 "health --format=json exits 0" + json_file="$(mktemp)" + printf '%s' "$OUT" >"$json_file" + run php -r 'exit( null === json_decode( file_get_contents( $argv[1] ), true ) ? 1 : 0 );' -- "$json_file" + assert_status 0 "health --format=json is valid JSON" + rm -f "$json_file" + + run "${WP[@]}" secret dropin + assert_status 0 "dropin exits 0" + assert_out_contains "Drop-in active: no" "dropin reports no drop-in" + + # 7. import-option: copy, not move. + local opt_name="smoke_${$}_opt" + run "${WP[@]}" option add "$opt_name" "$v2" + assert_status 0 "option add $opt_name exits 0" + run "${WP[@]}" secret import-option "$opt_name" "${NS}/imported" + assert_status 0 "import-option exits 0" + run "${WP[@]}" secret get "${NS}/imported" --reveal --field=value + assert_out_eq "$v2" "import-option copies the option's value into the secret" + run "${WP[@]}" option get "$opt_name" + assert_status 0 "the source option is left in place after import-option" + run "${WP[@]}" option delete "$opt_name" + assert_status 0 "option delete $opt_name exits 0" + + # 8. migrate-legacy. + run "${WP[@]}" secret migrate-legacy --dry-run + assert_status 0 "migrate-legacy --dry-run exits 0 with no prototype rows" + + # 9. single-site refusal (case E's last bullet, checked here). + run "${WP[@]}" network-secret get "${NS}/anything" + if [ "$STATUS" -ne 0 ]; then + ok "network-secret refuses on a single-site install" + else + not_ok "network-secret refuses on a single-site install" "exit status was 0" + fi + assert_err_contains "multisite" "network-secret's refusal explains why" +} + +# --- case C: rotation --- + +case_c_rotation() { + local r="${NS}/rotate" vr="smoke-value-rotate-$$" + local old new json_file + + # 1. Refusal first, while WP_SECRETS_KEY_PREVIOUS is still undefined: + # bin/smoke-install.sh recreates wp-config.php on every run. + run "${WP[@]}" secret rotate --yes + if [ "$STATUS" -ne 0 ]; then + ok "rotate without WP_SECRETS_KEY_PREVIOUS refuses" + else + not_ok "rotate without WP_SECRETS_KEY_PREVIOUS refuses" "exit status was 0" + fi + assert_err_contains "WP_SECRETS_KEY_PREVIOUS" "rotate without WP_SECRETS_KEY_PREVIOUS refuses with its explanatory message" + + # 2. Move the current key to PREVIOUS, generate a new one. Never print + # either value. + run "${WP[@]}" secret set "$r" "$vr" + assert_status 0 "set $r exits 0" + + old="$("${WP[@]}" config get WP_SECRETS_KEY)" + run "${WP[@]}" config set WP_SECRETS_KEY_PREVIOUS "$old" --type=constant --quiet + assert_status 0 "config set WP_SECRETS_KEY_PREVIOUS exits 0" + + new="$("${WP[@]}" secret generate-key)" + run "${WP[@]}" config set WP_SECRETS_KEY "$new" --type=constant --quiet + assert_status 0 "config set WP_SECRETS_KEY to a new value exits 0" + + local current_key previous_key + current_key="$("${WP[@]}" config get WP_SECRETS_KEY)" + previous_key="$("${WP[@]}" config get WP_SECRETS_KEY_PREVIOUS)" + if [ "$current_key" != "$previous_key" ]; then + ok "WP_SECRETS_KEY differs from WP_SECRETS_KEY_PREVIOUS before rotate" + else + not_ok "WP_SECRETS_KEY differs from WP_SECRETS_KEY_PREVIOUS before rotate" "the two config values were equal" + fi + unset old new current_key previous_key + + # 3. rotate --yes now succeeds. + run "${WP[@]}" secret rotate --yes + assert_status 0 "rotate --yes exits 0 with the previous key configured" + + # 4. The value still decrypts. + run "${WP[@]}" secret get "$r" --reveal --field=value + assert_status 0 "get $r --reveal --field=value exits 0 after rotation" + assert_out_eq "$vr" "the value still decrypts after rotation" + + # 5. health reports nothing undecryptable. The CLI's "check" column + # holds the Site Health label text ("All secrets can be decrypted" / + # "Some secrets cannot be decrypted"), not the string "undecryptable" + # itself, so the row is matched on "decrypt" rather than the literal + # word the detailed spec's check name uses. + run "${WP[@]}" secret health --format=json + assert_status 0 "health --format=json exits 0 after rotation" + json_file="$(mktemp)" + printf '%s' "$OUT" >"$json_file" + run php -r ' + $rows = json_decode( file_get_contents( $argv[1] ), true ); + foreach ( (array) $rows as $row ) { + if ( false !== stripos( $row["check"], "decrypt" ) ) { + echo $row["status"]; + exit( 0 ); + } + } + exit( 1 ); + ' -- "$json_file" + assert_out_eq "good" "health reports no undecryptable secrets after rotation" + rm -f "$json_file" + + # 4. --from=config is refused while the config keyring is still the + # active one: there is no other keyring to re-wrap under. + run "${WP[@]}" secret rotate --from=config --yes + if [ "$STATUS" -ne 0 ]; then + ok "rotate --from=config refuses without a drop-in keyring" + else + not_ok "rotate --from=config refuses without a drop-in keyring" "exit status was 0" + fi + assert_err_contains "--from=config only applies" "rotate --from=config explains why it refused" +} + +# --- case D: drop-in loading --- + +case_d_dropin() { + local d="${NS}/dropin" vd="smoke-value-dropin-$$" + + # Something to read for the "sets nothing" row, and for the wrong-type + # and fatal rows too: those cases only need get to reach the loader, + # not to succeed. + run "${WP[@]}" secret set "$d" "$vd" + assert_status 0 "set $d exits 0 before any drop-in is present" + + # Syntax error. + write_dropin <<-'EOF' + "$json_file" + run php -r 'exit( null === json_decode( file_get_contents( $argv[1] ), true ) ? 1 : 0 );' -- "$json_file" + assert_status 0 "network-secret health --format=json is valid JSON" + rm -f "$json_file" +} + +# --- main --- + +main() { + case_a_registration + case_b_behaviour + case_c_rotation + case_d_dropin + convert_to_multisite + case_e_multisite +} + +main