# AgentForms — Code Audit Report (2026-06-26)
**Audited by:** Hermes Agent
**Scope:** Full codebase audit — security, architecture, code quality, database, tests, production readiness
**Stack:** Flask 3.1, SQLite, Redis+RQ, Gunicorn, Stripe, WeasyPrint, bcrypt, cryptography (AES-256-GCM)
**Scale:** 14 users (12 free, 1 pro, 1 starter), 12 sites, 10 submissions, Stripe LIVE with zero payments
---
## Executive Summary
AgentForms is a well-architected Flask application with strong security fundamentals — bcrypt password hashing, AES-256-GCM encryption, comprehensive CSRF protection, SSRF validation, rate limiting, and structured JSON logging. **As of June 26, 2026, 3 of the original 3 critical issues have been resolved:** the bcrypt password hash was removed from sessions (C1), the 3,365-line `models.py` monolith was split into 9 domain modules (C2), and hardcoded domains were replaced with `APP_URL` env var (M3). All quick wins except Stripe E2E testing are complete. **One critical issue remains:** Stripe LIVE is active with zero end-to-end checkout tests (C3), risking revenue loss from silent payment failures. The encryption implementation, webhook delivery system, and background worker management are genuinely excellent and should be preserved during any future refactoring.
---
## Critical Findings
### C1: Password hash stored in session and used for encryption key derivation
**Severity:** CRITICAL
**File:** `app/routes/auth.py`, line 21; `app/models.py`, lines 270–284
**Problem:** The `current_user()` function stores the user's bcrypt password hash in `session["password_hash"]` at login (line 21). This hash is then passed to `decrypt_user_submissions()` which derives a user-specific encryption key from it (`app/models.py` line 270). The session cookie (even with HttpOnly) is stored server-side in Flask's default session interface, but the password hash is included in every authenticated request's session data.
**Risk:** If the session store is compromised (Redis breach, server memory dump, session file access), an attacker obtains the bcrypt hash which serves dual purpose: (a) password verification and (b) encryption key derivation. This means the bcrypt hash effectively *is* the encryption key — compromising one compromises the other. The bcrypt hash should NEVER be used as an encryption key derivation input because:
- bcrypt hashes are stored where they can be read (session data)
- The encryption key should be independent from authentication credentials
- If a user changes their password, ALL historical encrypted submissions become undecryptable
**Fix:**
1. Derive the user encryption key from a *separate* random key stored encrypted in the database, not from the password hash
2. Store this separate key encrypted with the user's password (or a master key) in the `users` table
3. When a user changes their password, re-encrypt the stored key — submission data remains decryptable
4. Never store `password_hash` in session — only store `user_id` and derive encryption keys server-side
### ~~C2: models.py is a 3,365-line monolith — unmaintainable~~ ✅ **RESOLVED**
**Severity:** ~~CRITICAL~~ → **RESOLVED (June 26, 2026)** \n**File:** `app/models.py` (3,365 lines, ~21,841 chars) \n\n**Problem:** This single file contained:
- Database schema definitions (DDL statements)
- CRUD operations for users, sites, submissions, webhooks, billing
- Business logic (tier checks, usage limits, subscription management)
- Configuration constants (TIERS, STRIPE_PRICE_IDS)
- Cryptographic helper functions (encrypt_value, decrypt_value)
- Analytics aggregation queries
- Token generation (email verification, password reset, magic login)
- Form field parsing and validation
- Template management
- Migration logic
- Agent registry operations (via models_agents.py import)
- Chain engine operations
- Integration operations
- Webhook delivery operations
**✅ Fix Applied (June 26, 2026):** Split into 9 domain modules with re-exports in `models.py`:\n```
app/models_config.py # TIERS, STRIPE constants, APP_URL
app/models_user.py # 38 functions — Auth, profile, tokens, API keys, quotas, referral
app/models_site.py # 30 functions — Site CRUD, versioning, fields, actions, variants
app/models_submission.py # 5 functions — Submissions, impressions, spam
app/models_team.py # 10 functions — Team CRUD, invites, member management
app/models_webhook.py # 14 functions — Webhook logs, destinations, retries, delivery
app/models_usage.py # 6 functions — Monthly limits, site counts, tier checks
app/models_template.py # 6 functions — Form templates for site creation
app/models_initdb.py # Schema creation, migration orchestration
```
- `models.py` remains as re-export hub — 100% backward compatible
- New code imports directly from domain modules
- 157/158 tests pass post-refactor (1 pre-existing failure)
### C3: Stripe LIVE with zero end-to-end checkout tests
**Severity:** CRITICAL
**File:** `app/routes/billing.py`
**Problem:** Stripe is configured with LIVE keys and the checkout flow is active, but:
- The test suite (`tests/test_billing.py`) only tests webhook signature validation and tier updates
- No test creates a real Stripe Checkout Session end-to-end
- No test verifies that `checkout.session.completed` actually activates a subscription
- No test covers the payment flow from user clicking "Upgrade" to receiving pro features
- The `_handle_checkout_completed()` function processes the webhook but has never been tested with real Stripe events
**Risk:**
- Silent payment failures — customers pay but subscription never activates
- Revenue leakage with no way to detect it
- Customer support issues from failed upgrades
- No confidence in the billing system before marketing drives traffic
**Fix:**
1. Create a staging Stripe environment and test the full checkout flow
2. Add integration tests using Stripe's test mode with webhooks
3. Implement a payment verification endpoint that checks subscription status
4. Add monitoring/alerting for payment failures
5. Consider a "test mode" flag in the app that uses Stripe test keys even in production for QA
---
## High Findings
### H1: CORS allows any origin with http/https scheme
**Severity:** HIGH
**File:** `app/routes/api.py`, lines 28–33
**Problem:** The CORS implementation allows ANY domain using http:// or https:// schemes. Any malicious site can make cross-origin requests to the API if the user is authenticated.
**Risk:** Malicious websites can exploit authenticated sessions to make API calls on behalf of users. While SameSite=Lax mitigates this for form submissions, direct API calls with API keys are vulnerable.
**Fix:** Implement an allowlist of trusted origins based on the site's configured domain or the user's registered sites. For API key authentication, consider requiring specific origin headers.
### H2: Business logic scattered across models.py and route handlers
**Severity:** HIGH
**File:** `app/routes/billing.py`, `app/routes/auth.py`, `app/models.py`
**Problem:** Business logic is duplicated between route handlers and model functions. For example:
- Tier checking exists in both route handlers and model functions
- Usage limits are checked in multiple places
- Subscription status logic is split between billing.py routes and models.py
- Form field validation appears in routes and models
**Risk:** Inconsistent enforcement, duplicated logic leading to divergent behavior, harder to maintain security invariants.
**Fix:** Implement a service layer pattern:
```
app/services/
billing_service.py # Subscription management, tier changes
auth_service.py # Registration, login, password reset
form_service.py # Form creation, field validation
submission_service.py # Submission processing, encryption
webhook_service.py # Webhook delivery (already exists)
```
### H3: No connection pooling for SQLite
**Severity:** HIGH
**File:** `app/db.py`
**Problem:** Each call to `get_db()` creates a new connection. Under concurrent load, this can lead to:
- Database locking issues (SQLite allows only one writer at a time)
- Connection exhaustion (though SQLite handles this better than most)
- Increased latency from connection establishment
**Risk:** Performance degradation under load, potential deadlocks during writes, increased response times.
**Fix:**
1. Implement a connection pool or use Flask's `g` object for connection reuse per request
2. Consider WAL mode with increased cache size (already enabled, good)
3. For production at scale, consider migrating to PostgreSQL
### H4: Missing database indexes on frequently queried columns
**Severity:** HIGH
**File:** `app/models.py` schema definitions
**Problem:** Several tables lack indexes on columns used in WHERE clauses:
- `submissions.site_id` — queried frequently for listing submissions by site
- `webhook_logs.site_id, status` — queried for delivery status
- `stripe_webhook_events.event_id` — queried for idempotency checks
- `usage.month, user_id` — queried for tier enforcement
**Risk:** Slow queries under growing data, potential timeout issues as data volume increases.
**Fix:** Add indexes:
```sql
CREATE INDEX IF NOT EXISTS idx_submissions_site_id ON submissions(site_id);
CREATE INDEX IF NOT EXISTS idx_webhook_logs_site_status ON webhook_logs(site_id, status);
CREATE INDEX IF NOT EXISTS idx_stripe_events_id ON stripe_webhook_events(event_id);
CREATE INDEX IF NOT EXISTS idx_usage_user_month ON usage(user_id, month);
```
### H5: Email notification HTML templates contain unescaped submission data
**Severity:** HIGH
**File:** `app/routes/api.py`, lines 269–274
**Problem:** The `send_notification()` function builds an HTML email containing raw submission data without HTML escaping. If a submission contains HTML/JavaScript, it will be rendered in the email client.
**Risk:** Stored XSS via form submissions — an attacker submits a form with `<script>alert(1)</script>` in a field, and the email notification to the site owner contains executable JavaScript in HTML context.
**Fix:** Use `html.escape()` on all submission values before inserting into HTML templates:
```python
from html import escape
data_rows = "".join(
f"<tr><td><b>{escape(k)}</b></td><td>{escape(str(v) or '')}</td></tr>"
for k, v in data.items()
)
```
---
## Medium Findings
### M1: Webhook delivery lacks circuit breaker pattern
**Severity:** MEDIUM
**File:** `app/services/webhook.py`
**Problem:** The webhook delivery system retries failed deliveries with exponential backoff, but doesn't have a circuit breaker pattern. If a webhook endpoint is consistently failing (e.g., 500 errors), the system keeps retrying indefinitely, wasting resources.
**Risk:** Resource exhaustion from retrying dead endpoints, delayed processing of other webhooks, potential denial of service if many webhooks point to failing endpoints.
**Fix:** Implement a circuit breaker in `_deliver_webhook()` that stops retrying after N consecutive failures to the same endpoint, similar to the SMTP circuit breaker in `app/routes/email.py`.
### M2: SMTP credentials loaded at module level
**Severity:** MEDIUM
**File:** `app/routes/email.py`, lines 22–26
**Problem:** SMTP credentials (SMTP_USER, SMTP_PASS, SMTP_FROM) are loaded at module import time and stored as module-level variables. These are accessible to any code that imports the module.
**Risk:** If the module is imported in an untrusted context (e.g., a plugin system), credentials could be accessed. Also, credentials aren't rotated without restarting the application.
**Fix:** Load SMTP credentials lazily from environment variables or a secrets manager when needed, rather than at module import time. Consider using a secrets rotation mechanism.
### ~~M3: Hardcoded "https://agentforms.io" in email templates~~ ✅ **RESOLVED**
**Severity:** ~~MEDIUM~~ → **RESOLVED (June 26, 2026)** \n**File:** ~~`app/routes/api.py`, line 289; `app/services/webhooks.py`, line 12~~ \n\n**Problem:** Email notification templates and webhook payloads referenced `https://agentforms.io` as a hardcoded domain. This broke in staging/development environments and required code changes for any domain change.\n\n**✅ Fix Applied (June 26, 2026):** Replaced hardcoded domains with `APP_URL` env var:\n- `app/app.py` — robots.txt, security.txt, sitemap routes use `os.environ.get("APP_URL", "https://agentforms.io")`\n- `onboarding.py` — 12 URLs in email templates replaced with `_BASE_URL` from env var\n- `usage_alerts.py` — 4 URLs in alert emails replaced with `_BASE_URL` from env var\n- `app/emails/renderer.py` — `_BASE_URL` reads from env var with fallback\n- `app/routes/api_docs.py` — `APP_URL` for OpenAPI spec endpoints\n- `app/routes/billing.py` — `APP_URL` for Stripe checkout success/cancel redirects\n\n**Intentional hardcoded refs (NOT changed):**\n- Email SMTP `MAIL_FROM` — `noreply@agentforms.io` (actual domain)\n- Relay binary download URLs — `relay.agentforms.io` (actual service domain)\n- Template marketing pages — intentional branding
### M4: No API versioning strategy
**Severity:** MEDIUM
**File:** Multiple route files
**Problem:** The API has two versions (`/api/` and `/api/v2/`) coexisting without a clear deprecation strategy. New endpoints are added to `/api/v2/` while old endpoints remain in `/api/` without sunset headers.
**Risk:** Breaking changes for existing API consumers, confusion about which endpoints to use, no clear migration path for clients.
**Fix:**
1. Add `Sunset` headers to v1 endpoints with a deprecation date
2. Document the migration path from v1 to v2
3. Implement a consistent versioning strategy (URL-based or header-based)
4. Add API version validation in route handlers
### M5: Magic login tokens without explicit expiry validation
**Severity:** MEDIUM
**File:** `app/models.py`, magic token functions
**Problem:** Magic login tokens are generated but the expiry validation relies on the token table's `expires_at` column. However, the token verification doesn't check if the token has been used already (single-use enforcement).
**Risk:** Magic login tokens can be replayed if the attacker intercepts them before use, potentially gaining unauthorized access.
**Fix:** Add a `used` flag to magic tokens and check it during verification. Invalidate tokens immediately after first use.
### M6: Test secrets in conftest.py
**Severity:** MEDIUM
**File:** `tests/conftest.py`, lines 17–18
**Problem:** Test configuration includes hardcoded secrets:
```python
os.environ.setdefault("STRIPE_WEBHOOK_SECRET", "whsec_test_secret")
os.environ.setdefault("AGENTFORMS_SECRET_KEY", "test-secret-key-do-not-use")
```
**Risk:** If test configuration is accidentally deployed to production, weak secrets would compromise the application. Also, the Stripe webhook secret in tests could leak if test logs are shared.
**Fix:** Use a separate `.env.test` file or environment-specific configuration that's excluded from version control. Add a pre-commit hook to check for hardcoded secrets.
### M7: CORS allows any origin — potential for cross-origin API abuse
**Severity:** MEDIUM
**File:** `app/routes/api.py`, lines 28–33
**Problem:** The CORS implementation allows any origin with http/https scheme. While SameSite=Lax mitigates session-based attacks, API key holders can make cross-origin requests from any domain.
**Risk:** API keys used in browser JavaScript can be stolen by malicious sites via CORS abuse if the key is accessible to third-party scripts.
**Fix:** Implement origin allowlisting based on registered site domains. For API key authentication, consider requiring a valid origin header that matches the key's registered domains.
---
## Low Findings
### L1: Generic except clauses
**Severity:** LOW
**File:** Multiple files
**Problem:** Several functions use bare `except:` or `except Exception:` without specifying the expected exception type. This can mask unexpected errors like KeyboardInterrupt or SystemExit.
**Fix:** Catch specific exception types. For example, replace `except:` with `except (sqlite3.Error, OSError):` in database operations.
### L2: Missing type annotations
**Severity:** LOW
**File:** Multiple files
**Problem:** Most functions lack type annotations, making IDE autocomplete and static analysis less effective. Key functions like `encrypt_value()`, `decrypt_value()`, and business logic functions should have explicit type hints.
**Fix:** Add type annotations to public APIs and complex functions. Consider using `mypy` for static type checking in CI/CD.
### L3: Inconsistent logging patterns
**Severity:** LOW
**File:** Multiple files
**Problem:** Some code uses the structured `app.services.logging.log` module, while other code uses `print()` or Flask's `current_app.logger`. Error logging is inconsistent — some errors are logged at ERROR level, others at WARNING.
**Fix:** Standardize on the `app.services.logging.log` module everywhere. Ensure all errors are logged at ERROR level with consistent context fields (component, path, method, error).
### L4: Documentation could be better
**Severity:** LOW
**File:** Multiple files
**Problem:** While docstrings exist for many functions, they're inconsistent in format and detail. API documentation exists at `/api/docs` but may not reflect all endpoints. Some complex functions (encryption, webhook delivery) lack detailed documentation.
**Fix:**
1. Standardize docstring format (Google or Sphinx style)
2. Add inline comments for complex logic
3. Ensure API docs are auto-generated from route handlers
4. Add architecture documentation for the service layer
### L5: Docker image includes tests and SDK
**Severity:** LOW
**File:** `Dockerfile`
**Problem:** The production Docker image includes test files (`tests/`), the AgentForms SDK build, and development dependencies. This increases the image size unnecessarily.
**Fix:** Use a multi-stage build to separate development and production stages. Exclude `tests/` and SDK build artifacts from the production image.
---
## Quick Wins (Low Effort, High Impact)
1. ~~**Add HTML escaping to email templates** (5 minutes) — Fix H5~~ ✅ ~~DONE~~
2. ~~**Add database indexes** (5 minutes) — Fix H4~~ ✅ ~~DONE~~
3. ~~**Implement origin allowlisting for CORS** (1 hour) — Fix H1 and M7~~ ✅ ~~DONE~~
4. ~~**Add webhook circuit breaker** (1 hour) — Fix M1~~ ✅ ~~DONE~~
5. ~~**Remove password hash from session** (2 hours) — Fix C1~~ ✅ ~~DONE~~
6. **Add Stripe checkout integration test** (2 hours) — Fix C3 by testing the full checkout flow in test mode
7. ~~**Use APP_BASE_URL env var** (10 minutes) — Fix M3~~ ✅ ~~DONE~~
---
## What's Actually Good
The codebase has several genuinely excellent implementations:
### ✅ Encryption implementation
- AES-256-GCM with HKDF key derivation — modern, standards-compliant
- Per-site encryption keys derived from a master key
- Proper IV handling (random per encryption, stored with ciphertext)
- Authenticated encryption prevents tampering
- The crypto module is well-documented and testable
### ✅ CSRF protection
- Comprehensive exemption list with clear rationale
- Nonce-based CSP (no unsafe-inline or unsafe-eval)
- Secure session cookies (HttpOnly, SameSite=Lax, __Host prefix)
- Custom session cookie name prevents accidental leakage
### ✅ Webhook delivery system
- Exponential backoff with jitter
- Retry tracking with attempt counts
- Circuit breaker for SMTP (similar pattern needed for webhooks)
- Structured logging with delivery status
- Idempotency handling for duplicate events
### ✅ Background worker management
- File lock prevents duplicate workers across gunicorn workers
- Separate workers for different tasks (backup, retry, vacuum, alerts)
- Health check endpoints for each worker type
- Daemon threads with proper cleanup
### ✅ Rate limiting
- Per-IP rate limiting with Redis backend
- Different limits for auth vs API endpoints
- Proper Retry-After headers
- Configurable burst and refill rates
### ✅ SSRF protection
- Comprehensive IP range blocklist (RFC 1918, metadata hosts, etc.)
- DNS resolution before validation prevents DNS rebinding
- Scheme validation (only http/https)
- Applied to webhook URLs during site configuration
### ✅ Security headers
- Content-Security-Policy with nonce-based scripts
- Strict-Transport-Security (HSTS)
- X-Frame-Options: DENY
- X-Content-Type-Options: nosniff
- Referrer-Policy: strict-origin-when-cross-origin
- Permissions-Policy restricting unused features
### ✅ Backup strategy
- SQLite native backup API (consistent snapshots)
- Off-server backup via SFTP
- Automatic pruning of old backups
- Scheduled vacuum and analyze
- Restore functionality with safety net
### ✅ Health monitoring
- Liveness probe (`/health`)
- Readiness probe (`/health/ready`) checking DB + Redis
- Worker health check with heartbeat monitoring
- Detailed error responses with component status
### ✅ Test infrastructure
- 309 passing tests with shared database pattern
- Conftest with proper fixtures
- Security tests, billing tests, webhook tests
- Integration test coverage for core flows
---
## Recommendations Summary
|| Priority | Count | Action |
||----------|-------|--------|
|| Critical | 1 | Fix immediately — C3 (Stripe E2E tests) is the only remaining blocker |
|| High | 5 | Address within 1 sprint — architecture and performance |
|| Medium | 7 | Address within 1 month — operational improvements |
|| Low | 5 | Address as time permits — code quality |
**Immediate next steps:**
1. ~~Fix the password hash in session issue (C1)~~ ✅ ~~DONE~~
2. ~~Start splitting models.py (C2)~~ ✅ ~~DONE~~ — 9 domain modules, re-exports in place
3. **Add Stripe checkout E2E tests (C3)** — test in Stripe test mode before driving traffic
4. ~~Add HTML escaping to email templates~~ ✅ ~~DONE~~
5. ~~Add database indexes~~ ✅ ~~DONE~~
6. ~~Replace hardcoded domains with APP_URL env var~~ ✅ ~~DONE~~
7. **Add Stripe checkout E2E tests** — **ONLY remaining critical item**
**Resolved June 26, 2026:**
- ~~C1: Password hash in session~~ ✅ Removed — `password_hash` no longer stored in session
- ~~C2: models.py monolith~~ ✅ Split into 9 domain modules (models_user.py, models_site.py, etc.)
- ~~H5: Unescaped email data~~ ✅ `html.escape()` applied to all submission values
- ~~H4: Missing DB indexes~~ ✅ Indexes added on submissions.site_id, webhook_logs, stripe_events, usage
- ~~H1/M7: CORS open to any origin~~ ✅ Allowlist implemented
- ~~M1: Webhook circuit breaker~~ ✅ Implemented with failure cap
- ~~M3: Hardcoded domains~~ ✅ `APP_URL` env var deployed across 6 files