HashiCorp Vault KV v2 provider example, and per-site naming in the AWS example - #2
Merged
Merged
Conversation
docs/SPEC.md wraps examples/vault-provider/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.
Creates the shared-harness subset the Vault example needs: config, bootstrap, make target, and a raw-HTTP test helper, proven against a real Vault dev server before any provider code exists. Pulled hashicorp/vault:latest and pinned it by digest: hashicorp/vault@sha256:47f14a6acb98f48d798a07df7c83f23a6e636e1cf724c5f8ff165cb32667a1e2 (sys/health reports version 2.1.1). Container started with that digest and confirmed unsealed before running the suite. Goal: Create the shared-harness subset this example needs and prove the suite can reach a real Vault dev server. Tests: examples/vault-provider/tests/test-vault-harness.php, Tests_Vault_Harness (3 tests): reachable/unsealed, KV v2 mounted at secret/, wipe() clears everything under wp/. Interpretation: none. Manual check: none.
Gives make test-examples a hosted run against a real Vault, pinned by the same digest as the Makefile comment, without touching any existing job. Goal: Give make test-examples a hosted run against a real Vault, pinned by digest, without touching the existing jobs. Tests: none executable locally beyond YAML validity (checked with js-yaml); the job's run itself is a manual check in P1-03. Interpretation: none. Manual check: none (deferred to P1-03).
Goal: Push the branch and record what only a human can confirm about the harness. Tests: none. Interpretation: none. Manual check: see progress log entry.
…client, get(), and delete()
Adds Vault_KV2_Provider with every interface method present, the HTTP
client and error mapping, path mapping, get() for both slots, and
delete(). set(), retire_previous(), and list_secrets() return
WP_Error("Not implemented.") until P2-02.
Goal: Create examples/vault-provider/secrets.php with every interface
method present, the HTTP client and error mapping, path mapping,
get() for both slots, delete(), and the declarations, tested offline
through pre_http_request.
Tests: examples/vault-provider/tests/test-vault-provider-paths.php,
Tests_Vault_Provider_Paths (16 tests, entirely offline via
pre_http_request).
Interpretation: none.
Manual check: none.
…(); run the conformance suite against Vault Completes the write path: set() asserts max_versions: 2 only on create, retire_previous() destroys exactly version N-1, and list_secrets() walks one LIST for namespaces and one LIST per namespace with blank fingerprints (filled in by P4-01). WP_Secrets_Provider_Conformance now runs green against the dev server. Goal: Complete the write path with max_versions: 2 on create, make every interface method behave, and get WP_Secrets_Provider_Conformance green against the dev server. Tests: test-vault-provider-conformance.php, Tests_Vault_Provider_Conformance (14 inherited tests, 13 pass, 1 skipped by the base class itself); test-vault-provider.php, Tests_Vault_Provider (8 tests). Interpretation: none. Manual check: none.
Goal: Push the branch with the provider core in place. Tests: none. Interpretation: none. Manual check: see progress log entry.
Adds end-to-end tests for the version translation: retiring never resurrects an older version, only two versions survive in Vault itself, N-1 is strict even when older versions are still present (create_metadata with a higher max_versions so pruning cannot be the cause), and a soft-deleted N-1 or CURRENT reads as absent rather than error. No defect found in previous_version()/retire_previous(); no provider code changed. Goal: Test the version translation end to end: retiring never resurrects, only two versions survive, N-1 is strict even when older versions exist, and a soft-deleted N-1 reads as absent. Tests: examples/vault-provider/tests/test-vault-provider.php, 7 new tests added to Tests_Vault_Provider. Interpretation: none. Manual check: none.
Goal: Push the branch with the version semantics proven. Tests: none. Interpretation: none. Manual check: see progress log entry.
…tadata Adds flag_is_set()/write_flag() for the rotation flag (set writes "1", clear writes "0" because Vault rejects an empty custom_metadata map), wired into set() after the wp_secret_changed action so a flag failure never undoes a landed value write. list_secrets() now reads each secret's metadata to report created, has_previous, and needs_rotation. Goal: Write and read the rotation flag with the detailed spec's failure rule, and make list_secrets() report created, has_previous, and needs_rotation from metadata. Tests: examples/vault-provider/tests/test-vault-provider.php, 9 new tests added to Tests_Vault_Provider. Interpretation: none. Manual check: none.
…timeout measurement Proves site scope is isolated per blog, network scope is shared, and a sealed or unreachable Vault -- or a token refused on write -- is WP_Error from every method, never null or a silent success. Measures the REQUEST_TIMEOUT assumption; it stays at 5 seconds. Goal: Prove site scope is per blog and network scope is shared, prove a sealed or unreachable Vault is WP_Error from every method, and measure the REQUEST_TIMEOUT assumption. Tests: test-vault-provider-multisite.php, Tests_Vault_Provider_Multisite (3 tests, multisite-gated); 3 tests added to Tests_Vault_Provider (sealed, unreachable, permission denied). Interpretation: none. Measurement: connection refused (http://127.0.0.1:1) -- 0.0051s, error code secret_store_unavailable. Non-routable (http://10.255.255.1:8200) -- 4.035s (bounded near, under, REQUEST_TIMEOUT=5s -- the timeout is honoured). Examples suite single-site pass wall clock: ~1.5s for 62 tests (PHPUnit summary line), no meaningful time spent waiting on timeouts. REQUEST_TIMEOUT left at 5. Manual check: none.
Goal: push the branch with the provider functionally complete; no code or docs change in scope for this task besides the progress log entry. Tests: none (push-only task). Interpretation: n/a. Manual check: NOT VERIFIED (human)
…capturing the request Goal: AWS_Secrets_Manager_Provider::aws_name() mapped site scope to wp/<name> with no blog ID, so every blog on a network shared the same AWS secret for a given name. The shipped provider keeps site scope per site, so the example was wrong, not the interface. Tests: examples/aws-secrets-manager/tests/test-aws-secrets-manager-naming.php, offline via pre_http_request, mirroring the Vault paths test's fake response shape: site-scope names include the blog id, network-scope names are unchanged, set() uses the same site-scoped name, listing maps site-scoped names back to WP names and ignores flat/foreign names, and (skipped off multisite) the blog id is read at call time after switch_to_blog(). Interpretation: added a private scope_prefix( $network ) returning 'wp-network/' or 'wp/site/' . get_current_blog_id() . '/', used by both aws_name() and wp_name() so the two stay in lock-step; read at call time (not cached) so a mid-request switch_to_blog() is honoured, matching the Vault provider's approach. Left the 'site' fingerprint scope for network secrets alone (tracked separately, not this task). README: rewrote the Naming section for the new shape and added "Upgrading from an earlier copy of this example" describing the AWS-side rename (create under wp/site/1/<name>, delete the old wp/<name>) and why no compatibility read ships before 1.0. Manual check: bin/ci-local.sh --keep and make reference-check both pass; examples suite green on both single-site and multisite passes (67 tests each, only the new multisite-gated test differs in skip count); phpcs clean on the touched files.
Goal: push the branch with the AWS fix isolated in its own commit. Tests: none (push-only task). Manual check: NOT VERIFIED (human)
Goal: record the two-slot cap on a many-version backend as a decision,
and bring the four spec pages whose statements the Vault example
touches in line with what it showed.
Tests: none executable (reviewer checks each page still has exactly
three ## sections in order). bin/ci-local.sh --keep and make
reference-check both green.
ADR 0009 ("Cap a many-version backend to two slots"): same frontmatter,
table, and section shape as ADR 0008. Context is the KV v2/two-slot
mismatch and the two problems it creates (hidden versions readable to
any Vault token; "previous" undefined past two versions). Decision is
max_versions: 2 on create, PREVIOUS strictly N-1, destroy not
soft-delete on retire. Consequences cover secrets created outside the
provider, retiring to no previous version by design, the undefined
docblock going to the Trac ticket via open-questions.md, and this ADR
as the record to amend if two slots prove wrong. Dated today per SPEC
§3; links the detailed spec, the README, and ADR 0008.
Only "As built" changed on all four spec pages (code matches the
proposal, so "Why" gets nothing): versioning.md gained a paragraph on
the Vault translation pointing at previous_version() and ADR 0009;
rotation.md's "Retiring the previous value" gained one sentence on
destroy vs soft-delete; providers-and-keyrings.md's "Supporting
surface" gained a sentence naming both provider examples and that
make test-examples runs the conformance suite against Vault;
extension-points.md's conformance-suite paragraph gained a sentence
on the same, framed as a second known-good subject whose backend
does not share the two-slot shape.
Manual check: grep -c '^## ' on all four pages reports 3; ls
docs/decisions/ shows 0009 as the only new file; the ADR 0009 link
left dangling by P6-01's README now resolves.
…d index both
Goal: record what the Vault example found in the three tracking pages,
write the one dev journal entry, and list the new pages in docs/index.md.
Tests: none executable. Reviewer checks frontmatter and that no existing
sentence in the three tracking pages changed. bin/ci-local.sh --keep and
make reference-check both green.
open-questions.md: appended "What the Vault example added" under "Host
and platform providers" (did not touch the existing KMS-keyring-has-no-
example sentence, left for build/kms-keyring); added two new sections
before "Testability smells" -- "What 'previous' means on a backend with
more than two versions" (🟡) and "A provider outside the WordPress
boundary still needs a root key" (🟢).
test-coverage-gaps.md: appended "The Vault example's failure paths are
simulated" (🟢) -- sealed/flag-write failures via pre_http_request, the
unreachable case is real, OpenBao is a human check, only the pinned
digest is tested.
proposal-questions.md, question 2: appended what the Vault translation
needed and that it didn't define "previous" past two versions; did not
touch the AWS sentence or the silence paragraph.
Journal entry docs/journal/2026-09-24-a-vault-provider.md ("A Vault
provider"): what was built, what it found (the AWS site-scope bug, the
conformance suite passing unchanged, the undefined "previous", local
key material still needed outside the boundary, the AWS fingerprint-
scope inconsistency left alone), what was left out, what it means for
the Trac patch. Links the README, ADR 0008, ADR 0009, and the N-1 test
by name.
docs/index.md: added the ADR 0009 line under decisions/ and the journal
entry line under journal/, matching the existing format.
Manual check: git diff --word-diff main..HEAD on the three tracking
pages shows only additions; head -5 of the journal entry shows title/
description/date with today's date; grep -c 'a-vault-provider'
docs/index.md is 1.
…l checks Goal: push the finished branch, clean up the local service container, and record every check that needs a human. Tests: none (push/cleanup-only task). Manual check: NOT VERIFIED (human)
Goal: write_flag() no longer clobbers custom_metadata keys other tools set on the same secret, and the Vault provider's docblocks describe actual Vault behaviour instead of stale task IDs. Tests: added test_setting_and_clearing_the_flag_preserves_other_custom_metadata, which seeds owner via a raw POST, then confirms both set-with-flag and set-without-flag preserve it alongside the correct needs_rotation value and MAX_VERSIONS. Strengthened test_an_unreachable_vault_is_an_error_not_absence to assert WP_SECRETS_ERROR_STORE_UNAVAILABLE for get(), list_secrets(), and delete(). Interpretation: write_flag() now takes the metadata already read by set() (the pre-write $meta) rather than re-reading; custom_metadata is unaffected by the data-write in between, so this is equivalent to a fresh read but avoids an extra request. Manual check: bin/ci-local.sh --keep and make reference-check both green, single-site and multisite.
Goal: the test helper never turns a transport failure or unexpected HTTP status into "absent" or "wiped" -- it fails the running test with the method, URL, and error, so a flaky connection fails where it happens and a negative assertion can never pass just because Vault was never reached. Tests: added test_the_helper_fails_loudly_when_vault_is_unreachable and test_wipe_fails_loudly_when_vault_is_unreachable to Tests_Vault_Harness, both constructing Vault_Test_Server( 'http://127.0.0.1:1' ) and expecting PHPUnit\Framework\AssertionFailedError. Interpretation: request() now calls Assert::fail() on WP_Error from wp_remote_request(); metadata()/list_keys() still treat 404 as absence but fail on any other unexpected code; wipe_recursive() fails on a non-204 DELETE. Added optional constructor arg $addr = null (falls back to VAULT_ADDR then the default) so tests can point the helper at an unreachable address without touching the environment. Manual check: ran the examples suite against the pinned Vault dev container (digest-pinned image from the Makefile) both single-site and multisite via wp-env -- 70 tests each pass, all green, new harness tests included. bin/ci-local.sh --keep and make reference-check also green.
…d files
Goal: every published statement about the Vault example matches the
code and works on any checkout, and no Foundry task ID or
PROGRESS.md-style reference remains in a shipped file, mechanically
enforced from here on.
Tests: added a docs/foundry.json constraint,
no-foundry-task-ids-in-shipped-files, matching [PR][0-9]+-[0-9]{2} across
examples/, Makefile, .github/, README.md, docs/journal/,
docs/decisions/, docs/spec/, docs/reference/, src/, plugin/, cli/,
tests/, bin/ -- would have hit Makefile:59, ci.yml:193, and the
pre-R1-01 secrets.php.
Interpretation: (a) generalized the wp-env --env-cwd example to
"wp-content/plugins/$(basename \"$PWD\")" in the Vault README and
Makefile comment, and said to run from the repo root; (b) corrected the
README's description of
test_previous_is_strictly_n_minus_1_even_when_older_versions_survive
to match its actual body (max_versions raised via the helper,
retire_previous() through the provider); (c) question 3 now describes
the R1-01 merge-preserving write instead of the false "rejects an
empty map" claim; (d) OpenBao section and test-coverage-gaps.md point
at "a commit message" instead of "the phase-6 progress entry"; (e)
test-coverage-gaps.md's unreachable-Vault description now says closed
local port, and the doubled blank line above that section's --- is
gone; (f) root README's Platform bindings sentence now says Vault runs
against a live dev server and AWS runs offline through
pre_http_request; (g) dropped "(pinned digest, from P1-01)" and
"(P1-01)" from the Makefile and ci.yml comments.
Manual check: grep for 'plugins/vault-provider', 'progress entry', and
'rejects an empty map' all empty; the three pinned digests (README,
Makefile, ci.yml) still identical; the one relative link in
examples/vault-provider/README.md resolves. bin/ci-local.sh --keep and
make reference-check both green, including the new constraint.
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.
Reconciles the examples harness both flights built: one test-examples target that needs Moto and Vault and runs single site then multisite, one examples CI job with both service containers, and the bootstrap loading example test helpers before the example classes. Vault's ADR becomes 0010, since the KMS keyring branch landed 0009 first.
ericmann
marked this pull request as ready for review
September 25, 2026 01:38
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
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 HashiCorp Vault KV v2 provider example described in
examples/vault-provider/SPEC.md. Vault numbers its versions (1, 2, 3, …) rather than keeping two slots, so this is the first backend that tests whether theCURRENT/PREVIOUSmodel is right. It is the second of the pre-Trac pieces in ADR 0008.Example
examples/vault-provider/: a single-file drop-in with no SDK, configured byWP_SECRETS_VAULT_ADDR,_TOKEN,_MOUNTand_NAMESPACE.max_versions: 2, so Vault holds no history the API can't reach.PREVIOUSis strictly version N-1, so retiring a version never brings back an older one. Retiring destroys the version rather than soft-deleting it.needs_rotationis stored incustom_metadata, merged with any other keys already there.wp/site/<blog_id>/…and network scope towp/network/….Fix to the AWS Secrets Manager example
wp/<name>with no blog ID, so on multisite every site shared one AWS secret per name. They now map towp/site/<blog_id>/<name>. The README includes upgrade steps for existing deployments.Tests
make test-examplestarget and oneexamplesCI job, with both Moto and Vault, run single site and multisite.Docs: the versioning spec page, journal tracking pages (proposal question 2 now records what Vault showed), the READMEs, and the journal entry "A Vault provider".
Still to verify by hand: a run against OpenBao, a test against a genuinely sealed (non-dev) Vault, and the AWS rename checked against live AWS.