Conversation
…_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
crates/edgezero-core/src/extractor.rs
Dismissed
| #[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
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.
There was a problem hiding this comment.
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.
ChristianPavilonis
left a comment
There was a problem hiding this comment.
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_namerejects 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
Summary
[stores.secrets]configuration into each adapter while preserving the existing low-level manual dispatch behavior.Design
SecretStoreis a single two-argument trait —get_bytes(store_name, key)— mirroring theKvStore/KvHandlenaming convention.SecretHandleis the ergonomic cloneable wrapper injected into request context. Platforms with named secret stores (Fastly) usestore_nameto open the right store; Cloudflare and Axum ignore it (flat namespaces).Changes
crates/edgezero-coreSecretStoretrait (get_bytes(store_name, key)),SecretHandlewrapper, sanitizedSecretErrormapping, request-context/extractor support,InMemorySecretStore+NoopSecretStoretest helpers,secret_store_contract_tests!macro, and[stores.secrets]manifest schema with adapter overrides.crates/edgezero-adapter-fastlyFastlySecretStore(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-cloudflareCloudflareSecretStore(ignores store name; flat namespace), manifest-aware secret wiring, and a private shared store-handle injection path for KV + secrets.crates/edgezero-adapter-axumEnvSecretStore(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.rsedgezero build.examples/app-demo/crates/app-demo-core/src/handlers.rssecrets_echodemo handler with allowlisted smoke-test keys and tests for success, sanitized failure, and invalid input.examples/app-demo/edgezero.toml[stores.secrets]configuration.examples/app-demo/crates/app-demo-adapter-fastly/fastly.tomlscripts/smoke_test_secrets.shdocs/guide/configuration.md[[environment.secrets]].Closes
Closes #59
Test plan
cargo test --workspace --all-targetscargo clippy --workspace --all-targets --all-features -- -D warningscargo check --workspace --all-targets --features "fastly cloudflare"wasm32-wasip1(Fastly) /wasm32-unknown-unknown(Cloudflare)edgezero-cli devcargo 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 buildChecklist
{id}syntax (not:id)edgezero_core(nothttpcrate)