Skip to content

Add provider-neutral secret store support#230

Open
prk-Jr wants to merge 18 commits intomainfrom
feature/secret-store-abstraction
Open

Add provider-neutral secret store support#230
prk-Jr wants to merge 18 commits intomainfrom
feature/secret-store-abstraction

Conversation

@prk-Jr
Copy link
Copy Markdown
Contributor

@prk-Jr prk-Jr commented Mar 24, 2026

Summary

  • Add a provider-neutral runtime secret store so handlers can read secrets consistently across Fastly, Cloudflare, and Axum.
  • Wire manifest-driven [stores.secrets] configuration into each adapter while preserving the existing low-level manual dispatch behavior.
  • Add security hardening, demo coverage, smoke-test support, and docs so the new secret-store path is safer to adopt and review.

Design

SecretStore is a single two-argument trait — get_bytes(store_name, key) — mirroring the KvStore / KvHandle naming convention. SecretHandle is the ergonomic cloneable wrapper injected into request context. Platforms with named secret stores (Fastly) use store_name to open the right store; Cloudflare and Axum ignore it (flat namespaces).

Changes

Crate / File Change
crates/edgezero-core Added two-arg SecretStore trait (get_bytes(store_name, key)), SecretHandle wrapper, sanitized SecretError mapping, request-context/extractor support, InMemorySecretStore + NoopSecretStore test helpers, secret_store_contract_tests! macro, and [stores.secrets] manifest schema with adapter overrides.
crates/edgezero-adapter-fastly Added FastlySecretStore (opens a named Fastly SecretStore per lookup), manifest-aware secret wiring, and a private shared store-handle injection path for KV + secrets.
crates/edgezero-adapter-cloudflare Added CloudflareSecretStore (ignores store name; flat namespace), manifest-aware secret wiring, and a private shared store-handle injection path for KV + secrets.
crates/edgezero-adapter-axum Added EnvSecretStore (reads from env vars; ignores store name), service/dev-server secret injection, and TCP integration tests for secret-present / missing / no-store cases.
crates/edgezero-cli/src/main.rs Added adapter-specific secret-store binding messages during edgezero build.
examples/app-demo/crates/app-demo-core/src/handlers.rs Added the secrets_echo demo handler with allowlisted smoke-test keys and tests for success, sanitized failure, and invalid input.
examples/app-demo/edgezero.toml Added the demo route and [stores.secrets] configuration.
examples/app-demo/crates/app-demo-adapter-fastly/fastly.toml Added local secret-store config for Fastly smoke testing.
scripts/smoke_test_secrets.sh Added adapter smoke checks for Axum, Fastly, and Cloudflare secret access.
docs/guide/configuration.md Documented runtime secret-store configuration and the distinction from [[environment.secrets]].

Closes

Closes #59

Test plan

  • cargo test --workspace --all-targets
  • cargo clippy --workspace --all-targets --all-features -- -D warnings
  • cargo check --workspace --all-targets --features "fastly cloudflare"
  • WASM builds: wasm32-wasip1 (Fastly) / wasm32-unknown-unknown (Cloudflare)
  • Manual testing via edgezero-cli dev
  • Other: cargo test -p app-demo-core, bash -n scripts/smoke_test_secrets.sh, ./docs/node_modules/.bin/prettier --check docs/guide/configuration.md, npm --prefix docs run build

Checklist

  • Changes follow CLAUDE.md conventions
  • No Tokio deps added to core or adapter crates
  • Route params use {id} syntax (not :id)
  • Types imported from edgezero_core (not http crate)
  • New code has tests
  • No secrets or credentials committed

prk-Jr added 16 commits March 24, 2026 14:20
…_secrets

Implements the Cloudflare secret store adapter backed by worker::Env::secret().
Adds dispatch_with_secrets and dispatch_with_kv_and_secrets dispatch functions,
and updates run_app to wire secrets through on every request.
- Prevent secret name leakage: NotFound/Validation/Internal errors now
  return generic messages with no secret identifiers in response bodies
- Enforce consistent adapter opt-in: secret_store_enabled(adapter) replaces
  stores.secrets.is_some() so per-adapter enabled=false is respected
- Fix Fastly SDK panic path: try_get/try_plaintext replace get/plaintext
- Add manifest-level enabled flag with per-adapter override support
- Fix EnvSecretStore: use var_os on unix for binary secret support
- Serialize env-mutation tests with tokio::sync::Mutex + RAII EnvOverride
- Add adapter-specific CLI build messages for secret store bindings
@prk-Jr prk-Jr self-assigned this Mar 24, 2026
#[async_trait(?Send)]
impl FromRequest for Secrets {
async fn from_request(ctx: &RequestContext) -> Result<Self, EdgeError> {
ctx.secret_handle().map(Secrets).ok_or_else(|| {

Check failure

Code scanning / CodeQL

Cleartext logging of sensitive information High

This operation writes
ctx.secret_handle()
to a log file.

Copilot Autofix

AI 3 days ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed. Added an inline safety comment explaining that ctx.secret_handle() returns a handle object (not secret bytes), and the error message contains only store configuration info. No secret values are logged — this is a false positive from the function name containing "secret".

…tore + SecretHandle

Replace the redundant single-arg SecretStore / SecretHandle pair and the
two-arg SecretStoreProvider / SecretProviderHandle pair with a single
two-arg SecretStore trait and SecretHandle wrapper, matching the
KvStore / KvHandle naming convention.

All adapters now implement SecretStore with get_bytes(store_name, key).
Fastly opens a named store per lookup (FastlyNamedStore internal helper);
Cloudflare and Axum ignore store_name (flat namespaces).
InMemorySecretStore and NoopSecretStore are the test/noop implementations.
Copy link
Copy Markdown
Contributor

@ChristianPavilonis ChristianPavilonis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review: Add provider-neutral secret store support

Overall: Well-structured implementation that cleanly mirrors the KV store pattern. Security hardening is thoughtful (sanitized errors, name validation, demo handler allowlisting). Comprehensive test coverage across all adapters. A few API design and code duplication issues worth addressing.

CI: All checks passing.

Findings: 3 High, 7 Medium, 4 Low (details in inline comments below)

Positives

  • Excellent security hardening — secret names sanitized in error conversion, verified by tests
  • Clean architectural parity with KV store pattern
  • validate_name rejects empty, control-char, and oversized names
  • Demo handler allowlist prevents secret exfiltration
  • Comprehensive test coverage (460+ lines, contract tests, TCP integration tests)
  • Proper #[must_use] on builder methods

- Consolidate dispatch: both adapters now resolve handles in run_app and
  call dispatch_with_handles directly; promotes resolve_kv_handle,
  resolve_secret_handle, dispatch_with_handles to pub(crate)
- Remove dead _secret_binding param from Cloudflare dispatch_with_kv_and_secrets
- Replace DummyKvStore with NoopKvStore in both adapter store_handles tests
- Extract shared EnvOverride + env_guard into axum test_utils module
- Export NoopKvStore under test-utils feature to match secret-store pattern
- Add doc comments to RequestContext::kv_handle and secret_handle
- Add GHAS safety comment to Secrets::from_request in extractor.rs
- Document breaking-change on run_app for both Fastly and Cloudflare adapters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Secret Store Abstraction

2 participants