review-security

name	review-security
description	Review code changes for security issues specific to this encrypted secret vault. Use when modifying auth, encryption, scope enforcement, or any data-access paths.

Security review

This is an encrypted secret store - security mistakes leak credentials.

CONVENTIONS (CRITICAL)

Auth

• ALWAYS validate JWT signature against Cloudflare JWKS + check issuer + AUD
• ALWAYS reject unregistered service tokens even if Access JWT is valid
• ALWAYS check users table first, fall back to ALLOWED_EMAILS only if table is empty
• ALWAYS verify enabled flag - disabled users are rejected even with valid JWT
• ALWAYS use isAdmin(auth) for user/role management endpoints
• NEVER fall back between auth modes - partial config = hard error
• NEVER store service token credentials on disk

Encryption

• ONLY crypto.subtle - no third-party crypto
• ONLY AES-256-GCM with random 12-byte IV per secret
• Envelope encryption - each secret gets a random DEK; DEK wrapped by master KEK (ENCRYPTION_KEY)
• ENCRYPTION_KEY is a Wrangler secret - NEVER in code or wrangler.jsonc
• Key rotation supported via /admin/rotate-key - re-wraps all DEKs with a new master key

Integrity (two layers)

• AAD binding - secret key name is passed as GCM Additional Authenticated Data; decryption fails if key name is wrong
• HMAC-SHA256 - computed on write, verified on read. Uses INTEGRITY_KEY (or HKDF-derived). NEVER use the encryption key directly as HMAC key
• HMAC binds key name + ciphertext + IV + encrypted_dek + dek_iv, preventing ciphertext and DEK swap attacks
• AAD catches key-name rebinding at the GCM layer; HMAC catches all tampering with an independent key
• Tamper detection: if HMAC verification fails, return error - do not decrypt

Scope enforcement

• ALWAYS call hasScope(auth, scope) as a gate check before any route touching secrets
• ALWAYS call hasAccess(auth, scope, secretTags) per-resource when iterating secrets with tag filtering
• Scopes resolved from role via roles table + role_policies table: read, write, delete, *
• Policy-based RBAC: role_policies binds scopes to specific tags; hasAccess() enforces this
• Legacy allowed_tags on roles still works as fallback via hasTagAccess() (deprecated)
• Token management: restricted to interactive auth only
• Audit log + user/role management: restricted to interactive auth + admin role
• Admin operations (re-encrypt, rotate-key) intentionally bypass tag restrictions - they must process ALL secrets

Feature flags

• Flags are plaintext in KV - they are configuration values, not secrets
• NEVER store sensitive data (credentials, keys, tokens) as flags - use encrypted secrets instead
• Flag operations use the same auth model and scope enforcement as secrets
• All flag operations are audit-logged

Input validation

• ALWAYS validate ENCRYPTION_KEY format (64 hex chars) - validated on first use
• Body size limits enforced via Zod: value 1MB, key 256 chars, description 1000 chars
• Failed auth attempts logged: method: "rejected", action: "auth_failed"
• Security headers: HSTS, X-Request-ID, CSP (HTML only), X-Content-Type-Options

Error handling

• ALWAYS wrap encrypt()/decrypt() in try-catch → 500
• NEVER return stack traces, SQL errors, or key fragments in error responses

CLI

• ALWAYS use execFileSync - never execSync with string interpolation
• JWT stored locally is short-lived. Service token creds are env-var-only.

See auth flow for the full authentication walkthrough.

REVIEW CHECKLIST

• No secret values logged, returned in list endpoints, or included in errors
• New endpoints have scope guards via hasScope() or isAdmin() and per-resource tag guards via hasAccess()
• All data access calls audit() with requestId
• No raw ENCRYPTION_KEY exposure outside encrypt/decrypt
• Auth middleware cannot be bypassed (route ordering in Hono)
• D1 queries use .bind(), never string interpolation
• Crypto ops wrapped in try-catch
• No new routes above auth middleware in index.ts unless intentionally public
• Error responses don't leak internals
• Verify HMAC computed on write and verified on read for new endpoints touching secrets

KNOWN GAPS

• No rate limiting - relying on Cloudflare edge protection (native Rate Limiting binding available for future use)