fix: add auth guard to email notify endpoint#116
Conversation
The /api/email/notify endpoint was accessible without authentication. In production it's protected by Cloudflare Service Binding (internal only), but the dev fallback path was completely open. Add X-Internal-Secret header validation for non-service-binding requests, using the shared ENCRYPTION_KEY. Update both email-worker callers to pass the secret when using the fallback URL. Closes #106
Skip the X-Internal-Secret check when ENCRYPTION_KEY is not set in the environment. This allows E2E tests and local dev setups without secrets to continue working, while production (which always has the key) remains protected.
gusye1234
left a comment
There was a problem hiding this comment.
Code Review — PR #116: fix: add auth guard to email notify endpoint
What it does
Adds authentication to the /api/email/notify endpoint when accessed via the dev fallback path (non-service-binding requests). Cloudflare Service Binding requests (identified by http://internal URL prefix) are trusted implicitly; all other callers must pass X-Internal-Secret matching ENCRYPTION_KEY.
Code Quality Checklist
Functionality
- Code does what it's supposed to do
- Edge cases handled (missing header, mismatched secret, no ENCRYPTION_KEY configured)
- Error handling appropriate (401 with generic message)
- No logic errors
Code Quality
- Readable and well-structured
- Guard is at the top of the handler, clear short-circuit pattern
- Both email-worker callers updated consistently (index.ts and imap-poller-do.ts)
- Test updated to use
http://internalURL to simulate service binding
Code Simplification
- Minimal, focused change
- No over-engineering
Security
- Closes a real vulnerability — previously the dev fallback endpoint was unauthenticated
- No hardcoded secrets
- Minor: Using
ENCRYPTION_KEYas an auth token is pragmatic but dual-purposes the key. IfENCRYPTION_KEYever gets rotated or leaked, it now also compromises this auth path. A dedicatedINTERNAL_API_SECRETwould provide better isolation — but for an internal dev fallback path, this is acceptable. - Minor: String comparison
secret !== cfEnv.ENCRYPTION_KEYis technically vulnerable to timing attacks. For an internal endpoint this is fine, butcrypto.timingSafeEqualwould be the hardened approach.
Tests Coverage
- Test updated to pass through the guard (uses
http://internalURL) - No explicit tests for the 401 rejection path (missing/wrong secret). Would be good to add a test case that calls from a non-internal URL without the header and asserts 401.
Project Syncing
- Email-worker index.ts and imap-poller-do.ts both send the secret header on fallback
- Web endpoint checks it consistently
Observations
-
URL-prefix trust model: Relying on
req.url.startsWith("http://internal")to identify service binding requests is correct for Cloudflare Workers — service bindings use the internal hostname. This is a well-established pattern. -
Guard skipped when
ENCRYPTION_KEYis falsy: The conditioncfEnv.ENCRYPTION_KEYmeans in environments without this env var, the endpoint remains open. This is presumably intentional for local dev, but worth documenting.
Verdict
Clean, targeted security fix. Closes an unauthenticated endpoint vulnerability on the dev fallback path. The two minor notes (dedicated secret, timing-safe compare) are nice-to-haves for a follow-up. Ready to merge.
Summary
X-Internal-Secretheader validation to/api/email/notifyfor non-service-binding requestsTest plan
/api/email/notifyreturn 401Closes #106