feat(secrets): add Vault KV v2 backend behind secrets-vault

[mkoushni]

Summary

• Adds a kind: vault SecretProvider (praxis-policy-secrets-vault) behind the secrets-vault facade feature. A declared value's ref is <mount>/<path>#<field> → GET /v1/<mount>/data/<path>, then that string field of data.data.
• Every call uses the host HttpTransport (no Vault SDK, no second HTTP stack). KV GET is RetryPolicy::idempotent; login and renew-self are RetryPolicy::none. execute_with_retry is public so this backend can share that policy.
• Auth is Kubernetes or AppRole, with no default. Kubernetes rereads the projected SA token on every login. AppRole secret_id is { env } | { file } | { literal }; a literal is refused unless allow_insecure_literal: true. Address is https:// unless insecure_http: true.
• Token renewal is lazy on the next get_secret (no spawned ticker; same fix(identity-jwt): JWKS refresh task is cancelled at startup and never runs #29 hazard as JWKS). Renewable tokens renew after 2/3 of lease_duration minus jitter; a 403 reauthenticates once and is distinct from 404 → NotFound.

Depends on #97. This branch is stacked on feat/secret-provider. Do not merge until that lands; after it does, rebase so the extra seam commit drops out of the diff.

Closes #94.

secrets:
  providers:
    vault-prod:
      kind: vault
      address: https://vault.example.com:8200
      auth:
        method: kubernetes
        role: ppe
  values:
    session_password: { provider: vault-prod, ref: secret/ppe#password }

Compiling secrets-vault (including via builtins) does not register the factory. install_builtins does not wire it: the host calls register_vault_secret_provider with a transport. In-cluster Vault (RFC 1918 / Kubernetes DNS) is refused by the bundled HyperTransport unless the host uses with_allow_private_destinations.

Written against the Vault 1.19 KV v2 HTTP API. CI covers that contract with FakeTransport; this crate has no HTTP stack of its own.

Test plan

• cargo test -p praxis-policy-secrets-vault (35 tests: auth, field extract, 403 reauth, renewal, concurrent login, namespace header, env/file secret_id, KV timeout retry, login Connect not retried, SecretStore::resolve)
• cargo clippy -p praxis-policy-secrets-vault -p praxis-policy --features secrets-vault -- -D warnings
• make ci on the stacked branch after feat(secrets): add secret provider trait to PPE to support various ba… #97
• Confirm a Kubernetes login body carries the rotated JWT after the projected token is rewritten
• Confirm secret_id.literal without allow_insecure_literal: true is a config error

[praxis-bot]

PR Review: feat(secrets): add Vault KV v2 backend

Adds a kind: vault SecretProvider behind the secrets-vault facade feature, with Kubernetes and AppRole auth, lazy token renewal, and KV v2 reads through the host HttpTransport.

Assessment. Sound design. Lazy renewal avoids the #29 hazard, zeroize wraps credentials, secret material stays out of Debug/errors, 403 reauth is distinct from 404, and the reference parser rejects path traversal. Two convention/correctness findings below.

Severity	Count
Critical	0
Large	0
Medium	2

Reviewed by praxis-bot

[praxis-bot]

Re-review: feat(secrets): add Vault KV v2 backend

Three commits since last review:

• dfdfb39 pins zeroize to 1.9.0 (resolves previous comment)
• 54674ce removes the double X-Vault-Namespace on renew-self (resolves previous comment)
• 441bb7b CI retrigger (no code change)

Both previous findings addressed. No new issues in the fixes.

Severity	Count
Critical	0
Large	0
Medium	0

Reviewed by praxis-bot

[terylt]

Hi @mkoushni, overall nice work! Here are some findings:

1. Login should be undelivered_only(), not none()

provider.rs:246 and :257. Every other credential-minting call in PPE uses
RetryPolicy::undelivered_only(): the OAuth token exchange at
delegator-oauth/src/delegator.rs:344 and :511, the CIBA dispatch at
elicitation-ciba/src/approver.rs:275 and :368. This is the first one to pick
none().

The PR body's reason for none() is "a retried login that actually succeeded
mints a second token", which is exactly what undelivered_only() encodes.
should_retry gates on may_have_reached_peer(), so undelivered_only()
retries Connect and nothing that could already have been served: a Timeout
or an Io ends the loop, which is the case the concern is about. none()
additionally refuses to retry a connection that provably never reached Vault.

Failure: Vault rolls a pod and refuses connections for a few hundred ms during
PolicyEngine::initialize(). Login is one attempt, so startup fails, where every
other outbound credential call in PPE would have ridden through it.
a_login_connect_failure_is_not_retried asserts the current behaviour, so this
is a deliberate choice rather than an oversight, but I think it is the wrong one.

renew_self is a separate case and a weaker one: renewing a lease twice is the
same as renewing it once, nothing is minted, so idempotent() is defensible
there. undelivered_only() at minimum. As it stands one timed-out renew costs a
full re-login on that same read.

2. A # in a KV path silently reads the wrong secret

reference.rs:63-65. kv_url_path() interpolates mount and path into the
URL with no percent-encoding, and parse() deliberately admits a # in the
path: the doc comment at :22-23 says "the last # starts the field so a path
may contain # (unusual) without eating the field."

It does eat it, one layer down. Probed against the crate:

ref "secret/we#ird#password"  -> path "we#ird"
  -> "https://vault/v1/secret/data/we#ird"
  -> http::Uri path "/v1/secret/data/we"

http::Uri truncates at the fragment. If secret/data/we exists and holds a
password field, PPE serves that value with no error at any layer: not a
Reference error, not a 404, nothing. The engine reports a healthy resolve of
the wrong credential.

Two smaller ones from the same root:

• secret/app?version=1#password reaches Vault as /v1/secret/data/app?version=1.
  Undocumented and unvalidated version pinning through the ref.
• secret/my app#password parses, then fails at the transport as
  InvalidRequest("invalid uri character"), which names neither the ref nor the
  character.

Fix either way: percent-encode each segment in kv_url_path(), or reject
characters that are not path-safe in parse() and drop the # claim from the
doc comment. The .. and empty-segment checks at reference.rs:49 show the
intent was already to keep a ref from addressing something it did not name.

3. The hand-rolled JSON encoder is worth replacing with serde

provider.rs:320-365. json_string_object, write_json_str and hex_nibble
are about forty five lines re-implementing JSON string escaping on the one path
that carries a credential. I read it and believe it is correct, including the
arm ordering that puts \n \r \t \b \f ahead of the \u00XX fallback. But the
only test, login_json_round_trips_and_escapes, covers " and \. The control
character and \u00XX arms are untested.

serde is already a dependency of the crate. A #[derive(Serialize)] struct Login<'a> { role: &'a str, jwt: &'a str } plus serde_json::to_writer into the
same Zeroizing<Vec<u8>> keeps the property the hand-rolled version exists for,
no owned String copy of the secret, and gets the escaping from tested code.

While in there: repeated push grows the Vec, and a realloc leaves the old
buffer un-zeroed, since Zeroizing only wipes the allocation it is holding at
drop. Vec::with_capacity up front closes that. Defence in depth rather than a
hole: the Bytes::copy_from_slice at :237 is an un-zeroed copy of the same
bytes regardless, because that is what HttpRequest takes. The doc comment at
:318-319 is accurate about what it claims, which is only the serde_json::Value.

4. Backend errors do not say which Vault call failed

vault_status_error produces Vault HTTP 404. transport_err produces
Vault transport: connection failed: refused. Neither names login, renew-self or
KV read, and neither names the auth mount.

So a typo in auth.kubernetes.mount surfaces as Vault HTTP 404 on a value
whose KV path is fine. SecretStore::resolve wraps it as "secret X via
provider Y", which points the operator at the value rather than at the mount.
The KV path does much better: a 404 there is a NotFound naming the reference.

This crate's own SecretError doc says the variants exist so that an operator is
sent to the right party. Threading the operation, and for a login the auth mount,
into both constructors would hold up that end.

Smaller things

• lease_duration of 0 means renew on every read. provider.rs:112-115 does
  .unwrap_or(0), and renew_after(0) is 0, so renew_at == now. In Vault a
  lease_duration of 0 is "this token does not expire", which is the opposite. I
  checked reachability and could not get there through AppRole or Kubernetes
  login: both return a nonzero TTL in practice, and Vault will not issue a
  root-policy token through an auth method. So this is robustness, not a live
  bug. It is worth noting because the two tests that use a zero lease as a
  no-sleep way to force renewal bake the reading in, and because with finding 1
  unaddressed a missing field turns every read into an unretried login. An
  injectable clock would let those tests use a real lease.

• Nothing covers data.data: null. That is what a soft-deleted KV v2 version
  carries. Whether Vault 1.19 sends it with 200 or 404 decides whether the
  operator sees NotFound or Malformed("Vault KV response had no data.data map"), and the second reads like a parser bug rather than "someone ran
  vault kv delete". I did not confirm which status 1.19 returns. This belongs
  on the live-Vault checklist in the PR body, which is already unchecked.

• zeroize is not a workspace dependency. Every other dep in
  builtins/secrets/vault/Cargo.toml uses workspace = true. Zeroizing<String>
  is in the SecretProvider signature, so ppe-core at 1.9 with
  zeroize_derive and this crate at 1.9.0 unifying is currently a coincidence
  of both being ^1. Promote it to [workspace.dependencies]. Separately,
  dfdfb39 is titled "pin zeroize to 1.9.0" but "1.9.0" is ^1.9.0 and not a
  pin, which is worth not leaving in the history as a claim.

• impl ValidatedSettings { fn from_parsed } at provider.rs:69-73 is a
  private one-line forwarder to VaultSettings::from_config, declared on a type
  from another module. Three call sites can name config::VaultSettings::from_config
  directly.

• namespace is not in the docs. It is supported and tested but absent from
  the configuration.md section. The ref grammar's restrictions, no empty
  segments and no .., are not written down anywhere an operator reads either