WealthySmart/codebase-review/04-security.md

Security Review — WealthySmart

Hat: Senior Application Security Engineer. Defensive review of the owner's own app (wealth.cescalante.dev) for hardening. All findings verified against actual code; key claims re-checked by the synthesizer (see README corrections).

Executive Summary

The most important finding is configuration, not code: backend/app/config.py ships working fallback credentials (admin/admin, a known SECRET_KEY) that take effect silently if a deploy ever runs without its env file — and the deploy pipeline regenerates .env.prod on every deploy, making that scenario plausible. Beyond that: login endpoints use non-constant-time != comparison with no rate limiting, CORS is allow_origins=["*"] with credentials enabled, JWTs live for 30 days, and PDF upload endpoints accept files of any size and type. On the positive side, the app uses parameterized queries throughout, safe subprocess invocation (no shell=True, controlled args, timeouts), hashed API tokens with expiry checks, and httpOnly + SameSite=Lax cookies. All critical fixes are low-effort.

Note (good news, verified): .env and .env.prod are properly gitignored and have never been committed — no secret exposure via the repository.

Threat Model

Assets: personal financial data (accounts, transactions, balances, pensions, water usage), admin credentials, JWT signing key, n8n API tokens, OpenAI API key, VAPID keys.

Actors:

• Internet attacker reaching wealth.cescalante.dev (it is publicly exposed)
• Malicious website visited by the logged-in user (CORS/CSRF surface)
• Credential brute-forcer (no rate limit, fixed username)
• Holder of a stolen token (30-day validity window)
• Indirect prompt injector — transaction descriptions originate from emails parsed by n8n; an attacker who controls a merchant name or email content gets text into the LLM agent's context

Entry points: /api/auth/login (×2 implementations), /api/v1/pensions/upload, /api/v1/municipal-receipts/upload, /api/copilotkit/* (Hono), /api/v1/agent/agui, n8n token-authenticated POSTs.

Findings

SEC-01: Working Fallback Credentials in Default Configuration — Critical

Location: backend/app/config.py:6, 9, 10 (verified) Evidence:

class Settings(BaseSettings):
    DATABASE_URL: str = "postgresql://wealthy_user:wealthy_pass@db:5432/wealthysmart"
    SECRET_KEY: str = "change-me-in-production"
    ADMIN_USERNAME: str = "admin"
    ADMIN_PASSWORD: str = "admin"

Exploit scenario: Deploy pipeline regenerates .env.prod from Gitea secrets each deploy (see reference_gitea_deploy). A pipeline regression or missing secret silently boots the app accepting admin:admin, and the known SECRET_KEY lets anyone forge valid JWTs even without logging in. Remediation: Remove defaults; validate at startup:

SECRET_KEY: str  # no default
@model_validator(mode="after")
def _no_weak_defaults(self):
    if self.SECRET_KEY in ("", "change-me-in-production"):
        raise ValueError("SECRET_KEY must be set")
    if self.ADMIN_PASSWORD in ("", "admin"):
        raise ValueError("ADMIN_PASSWORD must be set")
    return self

A failed boot is loud (healthcheck fails, deploy aborts) — exactly what you want.

SEC-02: Non-Constant-Time Credential Comparison — High

Location: backend/app/main.py:165–169, backend/app/api/v1/endpoints/auth.py:12–19 Evidence: Both login implementations:

if (body.username != settings.ADMIN_USERNAME
        or body.password != settings.ADMIN_PASSWORD):
    raise HTTPException(status_code=401, ...)

Exploit scenario: Short-circuit or plus string != leaks timing information distinguishing username-fail from password-fail and partially leaking prefix matches under many measurements. Marginal for a remote attacker, but the fix is one line and the password is also stored in plaintext config. Remediation: hmac.compare_digest() on both fields, both endpoints. Better: store a bcrypt/argon2 hash of the admin password (passlib) instead of the plaintext in env.

SEC-03: No Rate Limiting on Login — High

Location: Both login endpoints (main.py:163–179, endpoints/auth.py:10–21) Evidence: No limiter, lockout, or backoff anywhere in the codebase. Exploit scenario: Username is effectively fixed ("admin"); an attacker brute-forces the password at full line speed against a public endpoint. Remediation: slowapi with 5/minute per IP on both login routes, or a simple in-memory failure counter with exponential backoff (single-process app, in-memory is fine). Also consider consolidating to one login endpoint — two implementations means two places to harden forever.

SEC-04: CORS ["*"] with Credentials Enabled — Medium

Location: backend/app/main.py:79–85 (verified line 81) Evidence:

app.add_middleware(CORSMiddleware,
    allow_origins=["*"], allow_credentials=True,
    allow_methods=["*"], allow_headers=["*"])

Exploit scenario: Starlette reflects the request Origin when allow_credentials=True, so any website gets credentialed CORS approval. Mitigating factor: the ws_token cookie is SameSite=Lax, so browsers won't attach it to cross-site XHR/fetch — which currently blunts this from Critical to Medium. But the protection is one cookie-attribute change away from vanishing, and Authorization-header flows aren't SameSite-protected. Remediation: allow_origins=["https://wealth.cescalante.dev", "http://localhost:3000"], explicit methods and headers.

SEC-05: 30-Day JWT Expiry — Medium

Location: backend/app/config.py:7 (verified), backend/app/auth.py:18–20 Evidence: ACCESS_TOKEN_EXPIRE_MINUTES: int = 60 * 24 * 30 # 30 days Exploit scenario: A stolen cookie/token works for a month; there is no revocation mechanism for JWTs (only API tokens are revocable). Remediation: For a single-user personal app, 24h–7d is a reasonable convenience/security balance (8h would force daily logins). Pick e.g. 7 days, or implement short access + refresh tokens if login friction matters.

SEC-06: No Upload Size/Type Validation — Medium

Location: backend/app/api/v1/endpoints/pensions.py:104–162, municipal_receipts.py:197–228 Evidence: files: list[UploadFile] then await file.read() — unbounded read, no content-type or magic-byte check (endpoints are authenticated). Exploit scenario: Memory-exhaustion DoS via huge upload (requires auth — the n8n token or the user's own session — so primarily a robustness issue); junk files reach pdftotext. Remediation: 10 MB cap via bounded read, require pdf_bytes.startswith(b"%PDF"), record per-file rejections in the existing errors[] response field.

SEC-07: Missing Security Headers — Low

Location: backend/app/main.py (absent middleware); more importantly frontend/server.ts since it serves the HTML Evidence: No CSP, X-Content-Type-Options, X-Frame-Options, or HSTS set by either layer. Remediation: Set headers at the Hono layer (the user-facing one): X-Frame-Options: DENY, X-Content-Type-Options: nosniff, Referrer-Policy: strict-origin-when-cross-origin, and a CSP once inline-script usage is audited. HSTS can be set at nginx-proxy.

SEC-08: Cookie secure=False Relies on a Comment — Low

Location: backend/app/main.py:177 Evidence: secure=False, # set True behind TLS in production via nginx — but nothing actually sets it to True in production; nginx terminating TLS does not add the Secure attribute to an app-issued cookie. Exploit scenario: The cookie would be sent over plain HTTP if the user ever hits an http:// URL for the domain (pre-HSTS first visit, or a downgrade). Remediation: Drive it from config: secure=settings.COOKIE_SECURE (True in prod env). Pair with HSTS.

SEC-09: LLM Agent — Indirect Prompt Injection Surface — Medium (added by synthesis)

Location: backend/app/agent/tools.py (tool outputs include merchant names, notes), data path: email → n8n → POST /transactions/ → agent context Evidence: Transaction descriptions originate from external emails. When the assistant lists recent transactions, attacker-influenceable text (a crafted merchant name) enters the model's context. Mitigating factors: all agent tools are read-only queries (verified — no tool mutates data), and the agent endpoint requires authentication, so impact is bounded to misleading the user, not data modification or exfiltration to third parties. Remediation: Keep tools read-only as a hard rule (document it); if write-tools are ever added, require explicit user confirmation in the UI (renderAndWaitForResponse) for every mutation. Consider delimiting untrusted strings in tool outputs.

SEC-10: Positive Findings (No Issue) — Informational

• API token handling is correct (auth.py:42–54): SHA-256 hash lookup, is_active flag, expiry checked against now. Tokens are revocable.
• Subprocess usage is safe (pension_pdf.py:46–53, municipal_receipt_pdf.py:79–86): list-args, no shell=True, timeouts, Python-generated temp filenames — no command-injection path.
• No raw SQL string interpolation found — ORM/parameterized throughout (the ad-hoc migrations in db.py are static strings).
• httpOnly + SameSite=Lax cookie — correct baseline against XSS token theft and CSRF.
• .env files properly gitignored, never committed (verified via git ls-files and history).

Hardening Checklist

Priority 1 — do before anything else (≈half a day total):

• SEC-01: remove config defaults; fail-fast startup validation
• SEC-02: hmac.compare_digest (or hashed admin password) in both login endpoints
• SEC-03: rate-limit login (5/min/IP); consolidate the duplicate login endpoints
• SEC-04: pin CORS origins

Priority 2 — same week:

• SEC-05: reduce token expiry (7 days suggested)
• SEC-06: upload size cap + %PDF magic check
• SEC-07: security headers at the Hono layer; HSTS at nginx
• SEC-08: secure=True cookie in production via config

Priority 3 — ongoing posture:

• Keep agent tools read-only by policy; confirm-before-write if that ever changes (SEC-09)
• Audit logging for login attempts and token creation
• Scope n8n API tokens to the endpoints they need (currently any valid token hits any endpoint)
• Database backup with encryption; test restore
• Dependency update cadence (Dependabot/Renovate on requirements.txt and package.json)