Plan: canopy-common fail-closed encryption-mode guard (Issue #438)

On this page

Status

Step Description Status

1

Add encryption_keys_from_env_or_fail_in_production(var_name: &str) → Result<Option<EncryptionKeys>, String> to crates/canopy-common/src/crypto.rs. Reads CANOPY_ENV (default production), delegates to enforce_production_encryption_guard() (the pure helper that holds the policy). Dev env + missing key → Ok(None) with tracing::warn!. Non-dev env + missing key → Err with descriptive message naming the env var. Malformed key → Err in any env (delegated to encryption_keys_from_env). Preserves the rotation-window tracing::info! from canopy-persons’s current inline code.

Done (2026-05-14)

2

Extract pure guard logic as enforce_production_encryption_guard(var_name, env, keys) and add 6 unit tests against it. Design deviation: canopy-common has #![forbid(unsafe_code)], and std::env::set_var is unsafe in Rust 2024 — so the env-touching outer function can’t be unit-tested directly. The pure helper takes env-state as parameters; the outer function just reads CANOPY_ENV and delegates. Tests cover prod-no-key (Err), prod-with-key (Ok(Some)), dev-no-key (Ok(None)), default-unset-prod (Err), case-insensitive DEVELOPMENT (Ok(None)), rotation-window-with-previous (Ok(Some)). 80/80 canopy-common tests pass.

Done (2026-05-14) — pure-helper refactor accepted in lieu of env-mutating tests.

3

Migrate services/canopy-persons/src/main.rs:48-76 to call the new function. The 29-line block (load + production-check + warn + rotation-info-log) collapses to 7 lines — call function, wrap in EncryptionKey tuple. The new function carries the warn + info-log internally so the call site doesn’t repeat them.

Done (2026-05-14)

4

CHANGELOG entry under === Changed; this plan filed; precommit Q1-Q8 answered via subagent verification per .githooks/pre-commit (commit ae8251f).

Done (2026-05-14)

Issue: #438
Branch: feat/canopy-common-fail-closed-encryption-guard
Labels: compliance::pub-1075, priority::medium, service::shared-crates, type::security, workflow::ready

Context

Canopy stores encrypted SSNs at rest per ADR-017. The current production-vs-development decision logic — "if production and no encryption key, refuse to start" — lives inline at services/canopy-persons/src/main.rs:48-76. canopy-persons is the only service that consumes CANOPY_ENCRYPTION_KEY today, but ANY future service touching encrypted PII should inherit the same fail-closed posture without re-implementing the guard.

The 2026-05-09 external review flagged that CRAIG has the equivalent guard in its shared crate (craig-common/src/settings.rs:8); canopy should match. Today’s canopy implementation is functionally correct for canopy-persons but:

  1. Not reusable: every new encryption-key consumer would re-implement the same env != "development" check or — worse — forget it.

  2. Easy to silently bypass: canopy_common::crypto::encryption_keys_from_env() returns Ok(None) when CANOPY_ENCRYPTION_KEY is unset. A future service that consumes that without canopy-persons’s env-check wrapper would silently store plaintext SSNs.

  3. Not at the API surface: the contract "production must have a key" is enforced in service main() blocks, not in canopy-common’s type signatures.

The fix moves the guard into canopy-common as a typed function with explicit failure semantics. canopy-persons is migrated to call it. No behavior change in production today — canopy-persons still fails-closed; the change is architectural — the rule is now where future consumers will find it.

Code references

  • crates/canopy-common/src/crypto.rs:47-65 — current encryption_key_from_env(var_name: &str) (raw, no guard, returns Result<Option<[u8; 32]>, String>).

  • crates/canopy-common/src/crypto.rs:86-101 — encryption_keys_from_env(var_name: &str) (wraps key + rotation; returns Result<Option<EncryptionKeys>, String>).

  • crates/canopy-common/src/crypto.rs:106-117 — decrypt_with_rotation() (key rotation support, leave alone).

  • services/canopy-persons/src/main.rs:48-76 — current inline guard, to be replaced.

Scope

In scope (1 MR feat/canopy-common-fail-closed-encryption-guard):

  • New public function in crates/canopy-common/src/crypto.rs.

  • canopy-persons migration to call it (replaces 29-line inline block with 4 lines).

  • 4 new unit tests in crates/canopy-common/src/crypto.rs #[cfg(test)] mod tests.

  • This plan filed at docs/modules/ROOT/pages/plans/archive/canopy-common-fail-closed-encryption-guard.adoc.

  • CHANGELOG entry.

Out of scope:

  • Audit of other fail-open patterns in canopy (e.g. JWT signing-key absence, RabbitMQ connection failure on boot). #438 names the encryption-mode guard specifically; broader silent-degradation audit is a future MR if findings warrant.

  • Changes to encryption-key format, rotation semantics, or decrypt_with_rotation() behavior. Those are ADR-017 territory.

  • Adding new CANOPY_ENCRYPTION_KEY consumers. canopy-persons remains the only one.

  • OpenAPI snapshot regeneration. No API surface changes; cargo xtask api-docs is not required.

  • Deprecating encryption_keys_from_env. The unguarded function stays public — key rotation tooling and tests need raw access. Doc comment will steer service-startup callers to the new function.

Dependencies

  • No upstream code or plan dependencies. #438 is independent of the other Tier 1 issues (#435, #437, #433, #436).

  • Convention dependencies: ADR-013 plan format, ADR-017 for the secret-loading contract, project pre-commit Q1-Q8 protocol with subagent verification (.githooks/pre-commit from commit ae8251f).

Design

New function

// crates/canopy-common/src/crypto.rs (after the existing `encryption_keys_from_env`)

/// Load encryption keys from env, applying the production fail-closed guard.
///
/// In `CANOPY_ENV=development`, returns `Ok(None)` if no key is set (encryption
/// disabled, suitable for tests and local dev). Logs a `tracing::warn!` so the
/// disabled-encryption state is visible in dev logs.
///
/// In any other environment (including `CANOPY_ENV` unset, which defaults to
/// production per fail-closed semantics), missing `{var_name}` returns `Err`.
/// `{var_name}_PREVIOUS` remains optional in both envs (key rotation support).
/// Malformed values always `Err` regardless of env (delegated to
/// `encryption_keys_from_env`).
///
/// When both current and previous keys are present, logs a `tracing::info!`
/// so the rolling-rotation window is observable.
///
/// This is the production-correct entry point for any service that touches
/// encrypted-at-rest data per ADR-017. Use this in `main()` startup paths
/// instead of `encryption_keys_from_env` unless the caller has its own
/// fail-closed wrapper (key rotation tools, tests).
pub fn encryption_keys_from_env_or_fail_in_production(
    var_name: &str,
) -> Result<Option<EncryptionKeys>, String> {
    let env = std::env::var("CANOPY_ENV").unwrap_or_else(|_| "production".to_string());
    let is_dev = env.eq_ignore_ascii_case("development");

    let keys = encryption_keys_from_env(var_name)?;

    match (&keys, is_dev) {
        (None, false) => {
            return Err(format!(
                "{var_name} is required when CANOPY_ENV != development (current: {env:?}). \
                 Set a base64-encoded 256-bit key (openssl rand -base64 32). \
                 See docs/modules/ROOT/pages/adrs/adr-017-encrypted-secrets-at-rest.adoc."
            ));
        }
        (None, true) => {
            tracing::warn!(
                "{var_name} not set; running with encryption DISABLED \
                 (CANOPY_ENV=development). Encrypted-at-rest columns will store plaintext. \
                 See ADR-017."
            );
        }
        (Some(k), _) if k.previous.is_some() => {
            tracing::info!(
                "{var_name} rotation window active: decrypt will fall back to \
                 {var_name}_PREVIOUS on auth-tag failure."
            );
        }
        _ => {}
    }
    Ok(keys)
}

Why default CANOPY_ENV unset → "production"

  • Fail-closed default: if an operator forgets to set the env var, production-grade behavior is what they get.

  • Aligns with ADR-017 (encrypted secrets at rest are the operational default).

  • Dev/test path already explicitly sets CANOPY_ENV=development (devstack compose, test harness) — no regression.

Why tracing::warn! not error! for the dev-no-key path

  • It IS the expected state in dev (running locally without secrets bootstrap). error! would cause alerting noise and Grafana dashboards to red-flag a normal state.

  • warn! is loud enough to be visible without being an alert.

canopy-persons migration

Replace services/canopy-persons/src/main.rs:48-76 (the load + production-check + warn + rotation-info-log block) with:

let encryption_key = EncryptionKey(
    crypto::encryption_keys_from_env_or_fail_in_production("CANOPY_ENCRYPTION_KEY")
        .map_err(|e| anyhow::anyhow!(e))?,
);

The new function emits the warn! and info! logs internally, so the call site no longer repeats them. The downstream consumers of encryption_key (api/mod.rs, export.rs, store/models.rs) are unchanged — they read the EncryptionKey(Option<EncryptionKeys>) tuple the same way.

Unit tests (4)

In crates/canopy-common/src/crypto.rs #[cfg(test)] mod tests. Env-var tests are NOT parallel-safe (process-shared env), but the existing crypto.rs test module does not yet have an env-var-mutating test. Introduce a std::sync::Mutex static guard to serialize the 4 new tests. Tests must clean up env vars they set (std::env::remove_var).

Test names + behavior:

  • or_fail_in_production_prod_missing_key_errs — CANOPY_ENV=production, no CANOPY_ENCRYPTION_KEY → Err containing "CANOPY_ENV != development".

  • or_fail_in_production_prod_with_key_ok — CANOPY_ENV=production + valid base64 key → Ok(Some(_)).

  • or_fail_in_production_dev_missing_key_ok — CANOPY_ENV=development, no key → Ok(None). Cannot easily assert on tracing::warn! output in unit test — assert behavior, not log.

  • or_fail_in_production_unset_env_treated_as_production — CANOPY_ENV unset, no key → Err (default fail-closed).

Files Touched

File Change

crates/canopy-common/src/crypto.rs

Add encryption_keys_from_env_or_fail_in_production() (~35 LOC including doc-comment) + 4 unit tests + Mutex guard (~50 LOC).

services/canopy-persons/src/main.rs

Replace 29-line inline guard (:48-76) with 4-line call to new function.

docs/modules/ROOT/pages/plans/archive/canopy-common-fail-closed-encryption-guard.adoc

New plan (this file).

CHANGELOG.adoc

New entry under == Unreleased / === Changed.

No changes to: OpenAPI snapshots (no API surface), database migrations (no schema change), Antora nav (no new ADR/plan link surface), service Cargo.toml files (no dependency change — tracing already a transitive dependency through canopy-common’s existing usage).

Verification

  1. cargo nextest run -p canopy-common --lib — 4 new unit tests pass + all existing crypto tests still pass.

  2. cargo build -p canopy-persons — compiles cleanly with the new call site.

  3. cargo fmt --check --all and cargo clippy --all-targets — -D warnings — zero warnings (per .claude/docs/coding-conventions.md).

  4. cargo xtask validate — full battery green. This is the trusted pre-push gate per .claude/docs/git-workflow.md.

  5. Manual smoke: cargo xtask dev refresh and confirm canopy-persons starts cleanly. The tracing::warn! line should appear if CANOPY_ENCRYPTION_KEY is unset under CANOPY_ENV=development; otherwise no log (the success path is silent except for the rotation-window info-log when both keys are present).

Documentation Updates

  • CHANGELOG.adoc — entry under == Unreleased / === Changed.

  • This plan filed at docs/modules/ROOT/pages/plans/archive/canopy-common-fail-closed-encryption-guard.adoc.

  • .claude/docs/services.md — only update if a canopy-common section exists with encryption-key references; otherwise N/A. (Audit: no canopy-common section in services.md as of 2026-05-14; this checkbox stays N/A.)

  • .claude/docs/security.md — if the encryption-key-loading contract is documented there, update to point at the new function. Otherwise N/A.

  • Plan moves to plans/archive/ post-merge per ADR-013.

Why this approach (vs alternatives)

  • Don’t centralize ALL fail-closed concerns in one MR. #438 names the encryption-mode guard specifically; broader silent-degradation audit is a separate concern. Bundling them would inflate the diff and obscure the targeted fix.

  • Don’t make the function unconditionally Err on missing key. That would break the test/dev path that explicitly relies on plaintext columns for fixture loading.

  • Don’t shorten the function name. encryption_keys_from_env_or_fail_in_production is verbose but the contract is self-documenting at every call site. The shorter encryption_keys_from_env already exists and is unguarded; the explicit name disambiguates.

  • Don’t change the CANOPY_ENV default to "explicit-or-error". Operators forget env vars; defaulting to production is the safe failure mode. The cost is one well-documented surprise; the cost of the alternative is plaintext SSNs in production.

Risk + Rollback

  • Risk: misconfigured CANOPY_ENV=development in deployment could silently disable encryption. Mitigation: the tracing::warn! is visible in Prom/Loki dashboards; CHANGELOG flag-call-out covers the new default-to-production semantics.

  • Risk: future encryption-key consumers might import encryption_keys_from_env directly and bypass the guard. Mitigation: leave encryption_keys_from_env public (key rotation still needs raw access) but the new function’s doc comment recommends _or_fail_in_production for service-startup use. Future code review catches direct usage in main.rs blocks.

  • Rollback: revert the MR; canopy-persons returns to inline guard. No DB schema change, no data migration, no API surface change.

Edit this page · default