feat(security): rate limiting, input sanitization, security headers & per-user forwarding#376
Open
alfonsodg wants to merge 1 commit into
Open
feat(security): rate limiting, input sanitization, security headers & per-user forwarding#376alfonsodg wants to merge 1 commit into
alfonsodg wants to merge 1 commit into
Conversation
…rs, and per-user email forwarding - Add rate-limiter.js: KV-based rate limiting for login (5/min) and register (3/5min) - Add sanitize-utils.js: XSS prevention for string inputs - Add security headers middleware (X-Content-Type-Options, X-Frame-Options, X-XSS-Protection) - Add per-user email forwarding: account-level forwardEmail field with PUT /account/setForward endpoint - Integrate sanitization in login/register params
There was a problem hiding this comment.
Pull request overview
This PR adds security hardening (KV-based rate limiting, basic request parameter sanitization, and global security headers) and introduces per-user email forwarding by adding a forward_email field to accounts and a new API to configure it.
Changes:
- Add KV-backed rate limiting for
/loginand/register, plus request param sanitization for those endpoints. - Add global response security headers middleware.
- Add per-account forwarding configuration (
PUT /account/setForward) and apply it during inbound email processing.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| mail-worker/src/utils/sanitize-utils.js | Adds sanitization helpers for strings/emails and shallow param sanitization. |
| mail-worker/src/utils/rate-limiter.js | Adds KV-backed rate limiting utilities and client IP extraction. |
| mail-worker/src/api/login-api.js | Applies rate limiting and sanitization to login/register endpoints. |
| mail-worker/src/hono/hono.js | Adds global security headers middleware. |
| mail-worker/src/entity/account.js | Adds forwardEmail mapped to forward_email column. |
| mail-worker/src/api/account-api.js | Adds PUT /account/setForward endpoint. |
| mail-worker/src/service/account-service.js | Implements persistence/validation logic for forwardEmail. |
| mail-worker/src/email/email.js | Forwards inbound mail to the per-user forwarding address after global forwarding. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+9
to
15
| const ip = rateLimiter.getClientIp(c); | ||
| if (await rateLimiter.isLimited(c.env, `login:${ip}`, 5, 60)) { | ||
| return c.json(result.fail('Too many login attempts, please try again later', 429), 429); | ||
| } | ||
| const params = sanitizeUtils.sanitizeParams(await c.req.json()); | ||
| const token = await loginService.login(c, params); | ||
| return c.json(result.ok({ token: token })); |
Comment on lines
+9
to
+12
| const ip = rateLimiter.getClientIp(c); | ||
| if (await rateLimiter.isLimited(c.env, `login:${ip}`, 5, 60)) { | ||
| return c.json(result.fail('Too many login attempts, please try again later', 429), 429); | ||
| } |
Comment on lines
+3
to
+12
| /** | ||
| * Sanitize string input — strip HTML tags and dangerous characters | ||
| */ | ||
| sanitizeString(input) { | ||
| if (typeof input !== 'string') return input; | ||
| return input | ||
| .replace(/[<>]/g, '') | ||
| .replace(/javascript:/gi, '') | ||
| .replace(/on\w+\s*=/gi, '') | ||
| .trim(); |
Comment on lines
+24
to
+30
| * Sanitize object fields recursively (shallow, string fields only) | ||
| */ | ||
| sanitizeParams(params) { | ||
| if (!params || typeof params !== 'object') return params; | ||
| const clean = {}; | ||
| for (const [key, value] of Object.entries(params)) { | ||
| clean[key] = typeof value === 'string' ? this.sanitizeString(value) : value; |
Comment on lines
+38
to
+40
| return c.req.header('cf-connecting-ip') || | ||
| c.req.header('x-forwarded-for')?.split(',')[0]?.trim() || | ||
| 'unknown'; |
Comment on lines
+4
to
+31
| * Check if request exceeds rate limit using KV store | ||
| * @param {object} env - Cloudflare env with kv binding | ||
| * @param {string} key - Unique identifier (IP or userId) | ||
| * @param {number} maxRequests - Max requests allowed in window | ||
| * @param {number} windowSeconds - Time window in seconds | ||
| * @returns {boolean} true if rate limited | ||
| */ | ||
| async isLimited(env, key, maxRequests = 10, windowSeconds = 60) { | ||
| const kvKey = `rl:${key}`; | ||
| const record = await env.kv.get(kvKey, { type: 'json' }); | ||
|
|
||
| if (!record) { | ||
| await env.kv.put(kvKey, JSON.stringify({ count: 1, ts: Date.now() }), { | ||
| expirationTtl: windowSeconds | ||
| }); | ||
| return false; | ||
| } | ||
|
|
||
| if (record.count >= maxRequests) { | ||
| return true; | ||
| } | ||
|
|
||
| record.count += 1; | ||
| const remaining = windowSeconds - Math.floor((Date.now() - record.ts) / 1000); | ||
| await env.kv.put(kvKey, JSON.stringify(record), { | ||
| expirationTtl: Math.max(remaining, 1) | ||
| }); | ||
| return false; |
Comment on lines
+11
to
+14
| await next(); | ||
| c.res.headers.set('X-Content-Type-Options', 'nosniff'); | ||
| c.res.headers.set('X-Frame-Options', 'DENY'); | ||
| c.res.headers.set('X-XSS-Protection', '1; mode=block'); |
Comment on lines
+269
to
+279
| async setForward(c, params, userId) { | ||
| const { accountId, forwardEmail } = params; | ||
| const accountRow = await this.selectById(c, accountId); | ||
| if (!accountRow || accountRow.userId !== userId) { | ||
| throw new BizError(t('noUserAccount')); | ||
| } | ||
| if (forwardEmail && !verifyUtils.isEmail(forwardEmail)) { | ||
| throw new BizError(t('notEmail')); | ||
| } | ||
| await orm(c).update(account).set({ forwardEmail: forwardEmail || '' }) | ||
| .where(and(eq(account.accountId, accountId), eq(account.userId, userId))).run(); |
Comment on lines
13
to
15
| isDel: integer('is_del').default(0).notNull(), | ||
| forwardEmail: text('forward_email').default(''), | ||
| }); |
Comment on lines
+183
to
+188
| if (account && account.forwardEmail) { | ||
| try { | ||
| await message.forward(account.forwardEmail); | ||
| } catch (e) { | ||
| console.error(`Per-user forward to ${account.forwardEmail} failed:`, e); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds security hardening and a per-user email forwarding feature.
Security Improvements
Rate Limiting (KV-based, zero dependencies):
Input Sanitization (
sanitize-utils.js):Security Headers (global middleware):
X-Content-Type-Options: nosniffX-Frame-Options: DENYX-XSS-Protection: 1; mode=blockNew Feature: Per-User Email Forwarding
forward_emailfield to theaccounttablePUT /account/setForward— allows each user to configure their own forwarding addressFiles Changed
mail-worker/src/utils/rate-limiter.js(new)mail-worker/src/utils/sanitize-utils.js(new)mail-worker/src/api/login-api.js(rate limit + sanitize)mail-worker/src/hono/hono.js(security headers)mail-worker/src/entity/account.js(forwardEmail field)mail-worker/src/api/account-api.js(setForward endpoint)mail-worker/src/service/account-service.js(setForward logic)mail-worker/src/email/email.js(per-user forwarding)Notes
ALTER TABLE account ADD COLUMN forward_email TEXT DEFAULT ''