AWS KMS keyring example, request-scoped root-key caching, and rotate --from - #3
Merged
Merged
Conversation
…ing pages The provider entry said nobody had built against WP_Secrets_Provider. The AWS Secrets Manager example has, verified live, and building it found four defects outside the interface. What stays open is breadth: an independent host implementation, an automated conformance run, and a KMS keyring example. Proposal questions 2 to 4 now say no objections were raised, and call that silence rather than confirmation. Also drops the gitignore entry for the private reviewer asks, which are no longer planned.
ADR 0002 counted additions beyond the proposal as done only once the make/core thread confirmed them. Three weeks of exposure on the thread, the docs site, Core Slack, and dev chat produced support and no critique, which is silence rather than confirmation, and Beta 1 is 20 to 22 October. Review of the additions moves to the Trac ticket, whose description lists each one by name. Before the ticket opens: a KMS keyring example, a Vault provider example, and a WP-CLI smoke test. ADR 0002's status and first criterion point at the amendment, and the docs index gains the new record.
The pre-Trac work ADR 0008 names, each specced next to where its code will land. Writing them surfaced three things before any code: - WP_Secrets_Key_Manager unwraps the root key on every master-key derivation, uncached. A KMS keyring would pay a round trip per secret read, not the once per request examples/README.md claims. The KMS spec fixes it in the key manager, so it reaches the patch. - wp secret rotate hard-codes the config keyring on both sides, so no shipped command can move an existing site onto a new keyring. The spec adds --from. - The AWS Secrets Manager example maps site scope to wp/<name> with no blog id, so every site on a network shares one AWS secret per name. The Vault spec carries the fix as a separate commit. The Vault spec pins "previous" to strictly N-1, so retiring never resurrects an older version, and caps max_versions at 2 so Vault keeps no history the API cannot reach. The smoke test provisions its own install rather than sharing wp-env's wp-content, joins make ci, and also closes the --stdin and drop-in loading gaps.
docs/SPEC.md wraps examples/kms-keyring/SPEC.md in the shape the Foundry planner reads, adding the repository's documentation, journal, and no-publish rules. docs/foundry.json is pre-seeded with the verify commands and auto permission mode for an unattended run.
Goal: Add WP_Secrets_Keyring_Conformance, run it against WP_Secrets_Config_Key_Provider and Mock_Keyring, and make Mock_Keyring a conforming keyring double. Tests: WP_Secrets_Keyring_Conformance adds six tests (round trip, non-determinism, garbage/truncated/flipped-byte rejection, non-empty key source). Both Tests_Secrets_ConfigKeyringConformance and Tests_Secrets_MockKeyringConformance extend it and pass on single-site and multisite. Interpretation: none needed; followed the detailed spec's exact byte layout for Mock_Keyring's wrap()/unwrap() (8-byte nonce + key material + 32-byte SHA-256 tag, base64-encoded after the marker). Manual check: bin/ci-local.sh --keep and make reference-check both green.
… docblock Goal: Document the wrap() non-determinism requirement that WP_Secrets_Key_Manager::rotate_site_key() and WP_Secrets_Keyring_Conformance already depend on, and point implementers at the conformance suite. Tests: no new test file; the executable form is test_two_wraps_of_the_same_bytes_return_different_strings from P0-01, and tests/phpunit/test-architecture.php passes unchanged. Interpretation: none needed; used the exact sentence from the spec. Manual check: make reference && git diff --stat docs/reference/ touched only classes.md; bin/ci-local.sh --keep and make reference-check both green.
Goal: Push the branch and record that phase 0 has no manual check. Tests: none new; full suite is green from P0-02. Manual check: none required by SPEC.
… request Goal: Unwrap the root key once per request instead of once per master-key derivation, keyed on the stored wrapped value. Tests: Added seven tests to test-wp-secrets-key-manager.php covering repeated derivations, many secret reads, rotation priming the cache without a second unwrap, a changed wrapped value forcing a fresh unwrap, an unwrap error not being cached, generate_root_key() priming the cache, and the returned root key being a caller-owned copy. All pass on single-site and multisite. Interpretation: rotate_site_key() also treats the cache as authoritative when $old_keyring is the exact same instance as $this->keyring and the stored wrapped value matches $cached_wrapped, skipping a redundant unwrap() call -- required by the acceptance test "rotate_site_key updates the cache without another unwrap", and safe because a different $old_keyring instance (the "wrong keyring" rotation failure test) still forces a real unwrap attempt. Updated two pre-existing tests whose assertions depended on the old per-call unwrap behavior, now made obsolete by design: test_rotation_does_not_change_any_derived_master_key checks the "old keyring is no longer sufficient" assertion against a fresh manager instance, since the manager that just performed the rotation now legitimately has a valid primed cache. Three-state contract's test_key_unavailable_is_wp_error_not_null now corrupts the stored wrapped root key option instead of redefining WP_SECRETS_KEY mid-request, since a changed derivation constant no longer forces a fresh unwrap within the same request/cache -- that is the entire point of this task. Manual check: bin/ci-local.sh --keep and make reference-check both green (475 tests, single-site + multisite).
Goal: Make the documentation true for the new key manager: correct the once-per-request claim, record caching in the spec page's As built and Why, and add ADR 0009. Tests: none new (documentation). make reference-check passes; spec page headings remain exactly As proposed / As built / Why in order. Manual check: grep -n '^## ' docs/spec/providers-and-keyrings.md prints the three headings in order; grep -c 'once per request' examples/README.md is 1; docs/decisions/0009-root-key-cached-for-the-request.md exists; bin/ci-local.sh --keep and make reference-check both green.
Goal: Push the branch and record that phase 1 has no manual check. Tests: none new; full suite is green from P1-02. Manual check: none required by SPEC.
…active keyring
Goal: Let `wp secret rotate` move the root key from the config keyring
onto whatever keyring is active, keeping today's behaviour as the
default.
Tests: Added 7 tests to test-wp-cli-secret-command.php: unknown --from
value, --from=config refused against the config keyring (option
unchanged), --from=config-previous refused when both constants match,
a real config-previous rotation round trip, a real config -> drop-in
keyring move (verified with wp_get_secret() before/after and by
unwrapping the stored option with the mock), and a no-key-material-in-
output check across log/success/warning/errors. All pass single-site and
multisite.
Interpretation: none needed; followed the detailed spec's exact flag
names, default, refusal conditions and messages.
Manual check: wp help secret rotate synopsis is
"wp secret rotate [--from=<keyring>] [--yes]" as required. Full output
below.
Measurement:
NAME
wp secret rotate
DESCRIPTION
Re-wraps the root key under the active keyring.
SYNOPSIS
wp secret rotate [--from=<keyring>] [--yes]
There are two cases, chosen with --from. `--from=config-previous` (the
default) is a site key change: the root key, currently wrapped under
WP_SECRETS_KEY_PREVIOUS, is re-wrapped under the current WP_SECRETS_KEY.
`--from=config` is moving the root key onto a new keyring: a secrets.php
drop-in has installed one, and the root key, currently wrapped under the
config keyring's WP_SECRETS_KEY, is re-wrapped under that new keyring. No
secret is ever re-encrypted: rotation only changes what the root key is
wrapped under, not the root key's own bytes.
OPTIONS
[--from=<keyring>]
Which keyring currently wraps the root key.
---
default: config-previous
options:
- config-previous
- config
---
[--yes]
Skip the confirmation prompt.
EXAMPLES
$ wp secret rotate --yes
$ wp secret rotate --from=config --yes
bin/ci-local.sh --keep and make reference-check both green (481 tests,
single-site + multisite).
Goal: Push the branch and record that phase 2 has no manual check beyond the wp help output already in P2-01's commit. Tests: none new; full suite is green from P2-01. Manual check: none required by SPEC (wp help secret rotate output is in the P2-01 commit).
…nager conformance run
Goal: Create phpunit-examples.xml.dist, tests/bootstrap-examples.php and
make test-examples, give the AWS Secrets Manager example an emulator
endpoint, and run WP_Secrets_Provider_Conformance against it on Moto.
Tests: examples/aws-secrets-manager/tests/test-aws-secrets-manager-conformance.php
runs every WP_Secrets_Provider_Conformance test against Moto (14 pass, 1
skipped -- the read-only refusal test, correctly skipped for a writable
provider), plus test_loading_the_example_does_not_install_a_provider_without_the_constants.
15 tests total, green.
Interpretation: the wildcard testsuite directive
`<directory prefix="test-" suffix=".php">examples/*/tests</directory>`
did find the one existing test file (PHPUnit's iterator expands the glob
itself), so no explicit-path fallback was needed.
Manual check: `grep -n '^ci:' Makefile` does not mention test-examples.
php -l examples/aws-secrets-manager/secrets.php is clean. bin/ci-local.sh
--keep and make reference-check both green (481 tests, single-site +
multisite, main suites unaffected).
Measurement: docker pull motoserver/moto:latest, then
`docker image inspect --format '{{index .RepoDigests 0}}' motoserver/moto:latest`
gives motoserver/moto@sha256:91fd602a21f49cf9eb82fdf474015a3c131d40104c8297ea6a2ca920708ae32c.
Container: docker run -d --name secrets-api-moto-kms -p 5051:5000
motoserver/moto@sha256:91fd602a21f49cf9eb82fdf474015a3c131d40104c8297ea6a2ca920708ae32c.
curl -sf http://localhost:5051/moto-api/ returns 200.
npx @wordpress/env run --env-cwd=wp-content/plugins/kms-keyring tests-cli
vendor/bin/phpunit -c phpunit-examples.xml.dist: 15 tests, 25 assertions,
1 skipped, OK.
Goal: Run make test-examples in CI against Moto, pinned by digest,
without touching the existing jobs.
Tests: none new (CI configuration). YAML parses (ruby -ryaml). grep -n
'motoserver/moto@sha256:' .github/workflows/ci.yml finds the pin and it
matches docker inspect secrets-api-moto-kms --format '{{.Config.Image}}'.
Manual check: bin/ci-local.sh --keep and make reference-check both
green, 481 tests unaffected.
Goal: Push the branch and record that the examples CI job can only be observed green on a pull request. Tests: none new; the full suite and the examples suite are green from P3-02. Manual check: NOT VERIFIED (human) — examples CI job green on the PR.
…nce suite against Moto Goal: Add a single-file WP_Secrets_Keyring over KMS Encrypt/Decrypt with SigV4 by hand, and prove it conforms against Moto. Tests: examples/aws-kms-keyring/tests/test-aws-kms-keyring-conformance.php (Tests_AWS_KMS_Keyring_Conformance extends WP_Secrets_Keyring_Conformance, plus one extra test that loading the example installs no keyring without the wp-config constants). Fixture examples/aws-kms-keyring/tests/class-moto-kms-fixture.php creates one KMS key per test class via TrentService.CreateKey on Moto. Interpretation: none -- the task text specified the file layout, constants, method bodies, and fixture behaviour in full. Manual check: none for this task; the live-KMS run is deferred to phase 5 per docs/PLAN.md Decisions "Manual checks". 22 tests green (15 from the AWS Secrets Manager suite plus 7 new KMS conformance tests, 1 skipped as expected) via wp-env tests-cli phpunit-examples.xml.dist. bin/ci-local.sh --keep (481 main-suite tests, single and multisite) and make reference-check both green.
Goal: Write the one journal entry for this piece of work, in the voice of docs/journal/2026-09-04-0-1-0-is-public.md, and list it in the index. Tests: none new (documentation). Interpretation: none -- the section list, links, and title were fully specified. Manual check: none for this task. docs/journal/2026-09-24-a-kms-keyring.md: "What I built", "What it found" (the three already-known items plus the Mock_Keyring gap and the open-questions.md link), "What I left out", "What it means for the patch". Links the example README, the KMS test file, ADR 0008, and ADR 0009. docs/index.md gains the entry before open-questions.md, same format as the 0.1.0 line. Frontmatter has title/description/date matching the filename. grep -c '0008' = 1. docs/journal/_drafts/notes.md untouched (git diff --quiet passes). bin/ci-local.sh --keep and make reference-check both green.
…heck as not verified Goal: Finish the flight: push, clean up the local emulator, and log the human step that remains. Tests: none new; everything is green from P5-03 and the examples suite was last run green in P4-02 or later. Manual check: NOT VERIFIED (human) -- live KMS run per examples/aws-kms-keyring/SPEC.md "Done when".
…-state contract test Goal: Put back the original end-to-end scenario of test_key_unavailable_is_wp_error_not_null (an unusable WP_SECRETS_KEY defined after secrets exist yields WP_SECRETS_ERROR_KEY_UNAVAILABLE, never null), which P1-01 replaced with a different scenario, keeping the corrupted-option scenario as its own test. Tests: test_key_unavailable_is_wp_error_not_null now writes through a hand-built WP_Secrets_Libsodium_Provider (bypassing the request-scoped root-key cache, ADR 0009), defines WP_SECRETS_KEY = 424242, and asserts wp_get_secret() returns WP_Error with WP_SECRETS_ERROR_KEY_UNAVAILABLE. The former corrupted-wrapped-root-key body moved, assertions unchanged, to test_a_corrupted_wrapped_root_key_is_wp_error_not_null. Interpretation: none needed; the task fully specified the restored scenario. Manual check: bin/ci-local.sh --keep and make reference-check both green; ran the full suite single-site and multisite with the filtered class, 11/11 pass in each mode.
… wp-env path, ci.md, KMS README CI sentence Goal: Make every published statement this flight added true: the journal entry's Mock_Keyring paragraph, the wp-env command in three READMEs, the examples-job description in docs/reference/ci.md, and the KMS README's claim about CI. Tests: documentation only; verified with the greps named in R1-02 (no more 'plugins/kms-keyring', no false open-questions.md claim, corrected Mock_Keyring finding, ci.md names both examples, no 'does not provide by default' claim left in the KMS README) plus bin/ci-local.sh --keep and make reference-check. Interpretation: the wp-env command in all three READMEs now derives the plugin directory name via $(basename "$PWD"), matching how bin/ci-local.sh computes CONTAINER_CWD, with a clause noting it must run from the repository root. Manual check: git diff --stat touches only the five named files (plus docs/PROGRESS.md, updated separately by the Foundry tool).
… task IDs from published docs Goal: Make published docs true and self-contained — fix the AWS Secrets Manager README's false claim that CI provides no Moto, and remove three Foundry task-ID references (one pointing at docs/PROGRESS.md, which is stripped from docs/ before merge) from journal entries. Tests: none new (documentation). Verified with the greps specified in the task: no "does not provide by default" claim remains, no bare [PR]#-## task-ID pattern remains in published docs, and the corrected CI-job sentence is present in examples/aws-secrets-manager/README.md. Interpretation: none needed; task specified exact replacement wording. Manual check: bin/ci-local.sh --keep and make reference-check both pass (see verify output); journal frontmatter (title/description/date) intact.
The plan, progress log, reviews, summary, and pipeline state were working files for the build. The branch's substance is in its commits, docs, and journal entry. CLAUDE.md goes back to main's copy, and the lock-file ignore rule goes with the pipeline.
CLAUDE.md now says a tracking page's date: field is the date of its last substantive change.
ericmann
changed the title
ericmann
marked this pull request as ready for review
September 25, 2026 01:33
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds the AWS KMS keyring example and what its spec (
examples/aws-kms-keyring/SPEC.md) said comes with it. This is the first of the pre-Trac pieces in ADR 0008.Changes to
src/(part of the Trac patch)WP_Secrets_Key_Managernow keeps the unwrapped root key for the rest of the request, tied to the stored wrapped value. With a remote keyring such as KMS, that means one call per request instead of one per secret read. A rotation, re-wrap, or restore replaces the cached key, and anunwrap()error is never cached. ADR 0009 records the decision.WP_Secrets_Keyring::wrap()'s docblock now states thatwrap()must be non-deterministic, whichrotate_site_key()already relied on.CLI
wp secret rotate --from=config-previous|configre-wraps the root key under whichever keyring is active. This is how an existing site moves onto KMS.Tests
WP_Secrets_Keyring_Conformance, run against the config keyring and the mock.phpunit-examples.xml.dist,make test-examples, and theexamplesCI job with a pinned Moto container). It includes an automated conformance run for the AWS Secrets Manager example.Example
examples/aws-kms-keyring/: a single-file SigV4 drop-in with a fixed encryption context, a pinnedKeyId, and akms1:prefix that makes adopting it over a config-keyring root key fail with a clear error. Its README includes the adoption walkthrough.Docs: updated spec pages, journal tracking pages, the READMEs, and the journal entry "A KMS keyring".
Still to verify by hand: a live AWS KMS run, covering a fresh site, adoption with
rotate --from=config, and the one-Decrypt-per-request count. The count check already passes against Moto.