CompanySite/.agents/security-auditor/FORM_SECURITY_AUDIT.md

# Form Security Audit — Contact & Newsletter Endpoints

**Auditor:** security-auditor agent
**Date:** 2026-03-21
**Scope:** `/api/contact` (POST), `/api/newsletter` (POST), and their frontend forms
**Status:** ✅ No critical or high vulnerabilities. 1 medium, 2 low, 1 informational.

---

## Summary

Both form endpoints were audited for: CSRF protection, rate limiting, input validation, output sanitization, injection risks, and secret exposure. The codebase demonstrates solid security practices overall. Findings are documented below by severity.

---

## Security Controls — Status

| Control | Contact Form | Newsletter Form | Notes |
|---------|-------------|-----------------|-------|
| CSRF Protection | ✅ Origin/Referer check in middleware | ✅ Same | `validateCsrfOrigin()` in `middleware.ts:19-42` |
| Rate Limiting | ✅ 5 req/hr/IP | ✅ 3 req/hr/IP | In-memory sliding window |
| Input Validation | ✅ Server-side for all fields | ✅ Email validated | Allowlist for subject field |
| Output Sanitization | ✅ `escapeHtml()` in email HTML | N/A | `contact.ts:181-188` |
| Honeypot (Bot Detection) | ✅ `website` field check | ❌ No honeypot | Contact only |
| Payload Size Limit | ✅ 16KB max | ✅ 4KB max | Checked via `Content-Length` header |
| CORS | ✅ Origin allowlist in production | ✅ Same | Wildcard only in dev |
| XSS Prevention | ✅ `textContent` used on frontend | ✅ Same | No `innerHTML` with server data |
| Secret Exposure | ✅ Secrets server-only via `import.meta.env` | ✅ Same | No `PUBLIC_` prefix on sensitive keys |
| Error Info Disclosure | ✅ Generic error messages to client | ✅ Same | Full errors go to logger/Sentry only |

---

## Findings

### MEDIUM — M01: In-Memory Rate Limiter Lost on Server Restart

**File:** `src/pages/api/contact.ts:12`, `src/pages/api/newsletter.ts:12`
**OWASP:** A07:2021 Identification and Authentication Failures

**Description:**
Both endpoints use a module-level `Map` for rate limiting:
```typescript
const rateLimitStore = new Map<string, RateLimitEntry>();
```
This map is stored in process memory. On any server restart, crash, or deployment, the rate limit counters reset to zero. An attacker who knows this can circumvent rate limits by triggering restarts, or simply timing attacks to coincide with deploys.

**Risk:** Spam/abuse bursts become possible after any restart event.

**Recommendation:**
For production, migrate to a persistent store (Redis, Upstash, or a database-backed counter). For low-traffic sites, the current approach is acceptable with the understanding of this limitation. Consider adding a note to deployment runbooks that rate limit state is not persisted.

---

### LOW — L01: Honeypot Field CSS Positioning May Be Detected

**File:** `src/pages/contact.astro:158`
**OWASP:** A05:2021 Security Misconfiguration

**Description:**
The honeypot field uses CSS positioning to hide it from users:
```html
<div class="absolute -left-[9999px]" aria-hidden="true">
```
The `-left-[9999px]` approach is a widely documented honeypot pattern. Sophisticated bots that fingerprint common anti-spam techniques may detect and skip fields positioned this way. The `aria-hidden="true"` also signals to screen readers that the field does not exist, which is correct, but the approach is semi-known.

**Risk:** Low. Advanced bots may bypass honeypot, but rate limiting still applies.

**Recommendation:**
Consider randomizing the honeypot field `name` attribute server-side per session, or supplementing with a time-based challenge (track form render time — submissions faster than 3 seconds are likely bots). The current approach is adequate for common spam bots.

---

### LOW — L02: Content-Length Header Check Can Be Bypassed

**File:** `src/pages/api/contact.ts:252-259`, `src/pages/api/newsletter.ts:216-223`
**OWASP:** A04:2021 Insecure Design

**Description:**
The payload size check relies on the `Content-Length` request header:
```typescript
const contentLength = parseInt(request.headers.get('content-length') ?? '0', 10);
if (contentLength > 16384) { ... }
```
A client can omit the `Content-Length` header (using chunked transfer encoding) or send a falsely small value. The actual body would then be parsed regardless of size.

**Risk:** Low in practice — Astro/Node's HTTP parser has its own limits, and the validation still provides signal for well-behaved clients. Doesn't enable injection or data exfiltration.

**Recommendation:**
Add a runtime body-size guard after parsing. Check `JSON.stringify(body).length` or limit `request.body` reading with a max-byte stream reader. Example:
```typescript
const rawText = await request.text();
if (rawText.length > 16384) {
  return new Response(JSON.stringify({ success: false, error: 'Request body too large.' }), { status: 413 });
}
const body = JSON.parse(rawText);
```

---

### INFORMATIONAL — I01: Budget Field Silently Dropped

**File:** `src/pages/contact.astro:324-331` (frontend), `src/pages/api/contact.ts:289-295` (backend)

**Description:**
The contact form collects a `budget` radio button field (e.g. `< $5K`, `$5K – $15K`) but the backend `ContactFormData` interface does not include `budget`, so the value is never read, validated, or included in the notification email.

**Risk:** None (data integrity gap, not a security issue). Business impact: sales team never sees budget preference.

**Recommendation:**
Either add `budget` to `ContactFormData` and the email template, or remove the field from the frontend form. As-is, it creates a UX expectation mismatch.

---

## What Was Verified as Secure

### CSRF Protection (middleware.ts:19-42)
The `validateCsrfOrigin()` function correctly:
- Targets only state-changing methods (`POST`, `PUT`, `PATCH`, `DELETE`)
- Only applies to `/api/` routes
- Skips enforcement in development (avoids localhost friction)
- Returns 403 with CORS headers (preventing silent failures in browser)
- Validates both `Origin` and falls back to `Referer`
- Rejects requests with neither header in production

### CORS (contact.ts:193-215, newsletter.ts:157-179)
- Production: explicit allowlist (`workroot.in`, `www.workroot.in`)
- Development: wildcard (`*`) for DX
- `Vary: Origin` header present to prevent CDN caching issues
- `OPTIONS` preflight handler returns 204

### Input Validation (contact.ts:65-91)
- Name: 2–100 char bounds
- Email: regex + 254 char RFC limit
- Phone: optional, regex-validated when present
- Subject: strict allowlist (no free-text injection possible)
- Message: 10–5000 char bounds
- All fields trimmed before validation

### HTML Injection in Email (contact.ts:181-188)
The `escapeHtml()` function correctly escapes `&`, `<`, `>`, `"`, `'` for all user inputs rendered in the HTML email template. No XSS vector in email clients.

### Secret Handling (.env.example)
- All sensitive keys (`SMTP_PASS`, `MAILCHIMP_API_KEY`, `CONVERTKIT_API_KEY`) use `import.meta.env` (server-side only)
- No `PUBLIC_` prefix on any sensitive variable
- `.env.example` only contains placeholders, no real secrets

### Frontend XSS Prevention (contact.astro:799, Footer.astro)
- Server error messages are rendered via `textContent`, never `innerHTML`
- Toast icon SVG is static/trusted markup, not from server input
- Form values are read via `.value` and sent as JSON, never reflected into DOM

### ConvertKit API Key Exposure (newsletter.ts:101-105)
The ConvertKit integration sends `api_key` in the request body to the ConvertKit API. This is server-to-server only (never exposed to the browser), which is correct. The ConvertKit v3 API requires this pattern.

---

## Risk Matrix

| ID | Severity | Likelihood | Impact | Priority |
|----|----------|-----------|--------|----------|
| M01 | Medium | Medium | Low | Monitor — consider Redis for high-traffic |
| L01 | Low | Low | Low | Accept — rate limiting backstop exists |
| L02 | Low | Low | Low | Harden if DDoS is a concern |
| I01 | Info | N/A | N/A | Fix for business value |

---

## OWASP Top 10:2025 Coverage

| OWASP Category | Status | Notes |
|----------------|--------|-------|
| A01 Broken Access Control | ✅ PASS | No IDOR, CSRF protected |
| A02 Security Misconfiguration | ⚠️ LOW (L01) | Honeypot fingerprint risk |
| A03 Injection | ✅ PASS | Allowlists, escaping, no SQL/eval |
| A04 Insecure Design | ⚠️ LOW (L02) | Content-Length bypass |
| A05 Security Misconfiguration | ✅ PASS | Headers set in middleware |
| A06 Vulnerable Components | ℹ️ Not audited | Separate dependency scan recommended |
| A07 Auth Failures | ⚠️ MEDIUM (M01) | Rate limit memory volatility |
| A08 Software Integrity | ℹ️ Not audited | lock file audit recommended |
| A09 Logging Failures | ✅ PASS | Sentry + structured logger |
| A10 SSRF | ✅ PASS | No user-controlled URLs fetched |