---
id: review-security-encryption-config
name: Review Security Specs, Key Rotation, and Config Integration
status: completed
depends_on: [security-specs, specify-key-rotation-protocol, specify-client-config-validation, config-hub-startup-system]
scope: moderate
risk: critical
impact: project
level: review
---

## Description

Independent review of security-critical specifications and their consistency with the new config system. This review covers two interrelated areas:

### 1. Key Rotation and Encryption Protocol

The key rotation protocol in `services.md` specifies multi-key format, crash-safe re-encryption transactions, and background sweep. The other session updated `services.md` to reference the two-layer key model from `hub-config.md` instead of `HUB_ENCRYPTION_KEY` env vars. This review must verify:

- `services.md` now consistently references the config-driven data encryption keys (not env vars)
- The key rotation protocol in `services.md` is compatible with the `EncryptionKeyRing` concept in `hub-config.md`
- No stray env var references (`HUB_ENCRYPTION_KEY`, `DATABASE_URL`, `REDIS_URL`) remain in any storage doc
- The `_encrypted` wrapper pattern in `hub-config.md` for postgres/redis credentials is consistent with how `client_secrets` are stored at rest

### 2. Config System Consistency with Storage

The config system is now specified in `docs/architecture/hub-config.md` and `docs/architecture/hub-startup.md`. ADR-008 was revised. This review must verify cross-document consistency:

- `storage/README.md` no longer references `Deno.env.get()` for DB credentials
- `storage/services.md` references the two-layer key model correctly
- `infrastructure.md` Docker deployment uses config file + secret, not `-e` flag env vars
- The startup sequence in `hub-startup.md` initializes the encryption key ring (Step 7) before any subsystem that needs it
- `identity.md` (api_keys, keypal) and `services.md` (client_secrets) are compatible with the config-driven startup

Both `hub-config.md` and `hub-startup.md` are draft-stage — this review should assess whether they're consistent enough with the storage docs to proceed with implementation.

## Acceptance Criteria

- [ ] Key rotation protocol in `services.md` is complete and references the config-driven key model (not env vars)
- [ ] No env var references for sensitive values remain in any storage doc (`HUB_ENCRYPTION_KEY`, `DATABASE_URL`, `REDIS_URL`, etc.)
- [ ] The two-layer key model (master key → config decryption → data encryption keys) is consistently described across `hub-config.md`, `services.md`, and `ADR-008`
- [ ] `storage/README.md` uses config-driven connection patterns (no `Deno.env.get()` for DB credentials)
- [ ] `infrastructure.md` Docker deployment uses Docker secret + config file mount (not `-e` env vars)
- [ ] `hub-startup.md` Step 7 (encryption key ring) happens before subsystem initialization that needs keys
- [ ] Client config validation timing is documented: validated on write (TypeBox schema), startup advisory validation on read
- [ ] SHA-256 tradeoff for API keys is documented with the high-entropy machine-generated rationale
- [ ] Payload redaction strategy for call graph nodes is documented

## References

- docs/architecture/hub-config.md (config system, two-layer keys, _encrypted wrapper)
- docs/architecture/hub-startup.md (startup sequence, encryption key ring init)
- docs/architecture/storage/services.md (key rotation protocol, client_secrets, clients config)
- docs/architecture/storage/identity.md (api_keys, audit_logs)
- docs/architecture/storage/README.md (config references)
- docs/architecture/infrastructure.md (Docker deployment)
- docs/decisions/ADR-008-secrets-encrypted-at-rest-with-key-versioning.md (revised)
- packages/core/config/types.ts (spoke-only config, to be extended)
- packages/core/utils/crypto.ts (encrypt/decrypt/generateEncryptionKey)

## Notes

The config system blocker (`config-hub-startup-system`) is now completed — it produced `hub-config.md` and `hub-startup.md`. This review checks whether the storage docs are fully consistent with those new specs and whether any env var references leaked through.

## Summary

Review completed 2026-04-23. 9 acceptance criteria checked: 7 PASS, 2 FAIL (1 critical, 1 related to same issue).

**Critical issue found and fixed:**
1. `Deno.env.get("DATABASE_URL")` in storage/README.md contradicts the "no env vars for secrets" rule → **Fixed**: replaced with `ALKHUB_DRIZZLE_KIT_URL` (non-sensitive CLI convenience var) and a dev-only placeholder URL with `changeme` credentials instead of real ones.

**Warnings fixed:**
- `PAYLOAD_TRUNCATION_THRESHOLD` env var in call-graph.md → **Fixed**: changed to `HubConfig.callGraph.payloadTruncationThreshold`
- Hardcoded `user:pass` in drizzle.config.ts snippet → **Fixed**: replaced with `alkhub:changeme` placeholder
- `storage-spec-phase1-resolutions.md` historical `HUB_ENCRYPTION_KEY` reference → **Fixed**: added "(Superseded — see ADR-008 revised and hub-config.md)"

Two-layer key model is consistent across hub-config.md, services.md, and ADR-008. Key rotation protocol is crash-safe. Startup sequence correctly orders encryption key ring before subsystem initialization.