Inboxintel/AUDIT_REPORT.md

InboxIntel — Full Application Audit Report

Date: 2026-07-02 · Scope: entire repository (backend src/, frontend/, migrations, CI/CD .gitea/workflows/, docker-compose*, deploy/, git history) · Phase: 1 (read-only)

Stack: .NET 8 / ASP.NET Core (Clean Architecture) · EF Core + PostgreSQL (pgvector image) · React 18 + Vite SPA · Google OAuth2 (cookie session) · Gitea Actions CI/CD · self-hosted Docker Compose (single-operator deployment).

Executive summary

This codebase is markedly more hardened than typical — previous hardening phases (V-01…V-12, visible in code comments) already addressed the classic killers: IDOR (EF global query filters), SSRF (a genuinely thorough SafeHttpGuard with DNS-rebinding defence), token encryption at rest, non-root containers, stack-trace suppression, forwarded-header trust, secret hygiene (nothing in git history), and confirmed-destructive-actions. No Critical findings. The most significant issues are: registered input validators that never execute (H-1), no rate limiting (H-2), and a data-at-rest posture for email bodies that should be a deliberate, documented decision (H-3). CI has real security gates (gitleaks + dependency audit) already.

Severity counts: Critical 0 · High 3 · Medium 6 · Low 7

---

1. Security

H-1 · FluentValidation validators are registered but NEVER executed

• File: src/InboxIntel.Application/DependencyInjection.cs:11 (registration) vs src/InboxIntel.Api/Program.cs (no AddFluentValidationAutoValidation() anywhere; no controller injects IValidator<>).
• Why it's a problem: Validators.cs defines real safety rules — CleanupRequestValidator requires Confirmed=true for Trash/HardDelete, SearchRequestValidator bounds paging, etc. — but FluentValidation 11.x requires explicit auto-validation enablement, which is absent. The rules are dead code; requests reach services unvalidated. Mitigating factor: the services re-check the most dangerous rules (CleanupService.cs:48 re-enforces Confirmed), and the controller clamps paging — so this is defence-in-depth loss, not an open hole. But rules that exist only in the validators (e.g. From <= To, LabelId-required) are silently unenforced.
• Fix: add services.AddFluentValidationAutoValidation() (package already referenced) in Program.cs; add a regression test posting an invalid DTO and asserting 400.
• Blast radius: isolated (1 line + tests). Verify no existing client sends payloads that would now 400.

H-2 · No rate limiting on any endpoint

• File: src/InboxIntel.Api/Program.cs (no AddRateLimiter/middleware anywhere).
• Why: login (/auth/login → OAuth), search (now with FTS + trigram + future AI), export (PDF generation), unsubscribe (server-side outbound HTTP), and AI endpoints are all unthrottled. A single authenticated user (or an unauthenticated client hammering /auth/login//appinfo) can exhaust CPU/DB/outbound quota. For the future multi-user platform (see docs/discovery/multi-provider/), this is a prerequisite.
• Fix: ASP.NET Core's built-in RateLimiter — a global fixed-window per-IP policy + stricter policies on auth, export, unsubscribe, ai. Return 429 with Retry-After.
• Blast radius: isolated (Program.cs + policy constants + tests).

Verified-good (no finding)

• Authorization / IDOR: [Authorize] on ApiControllerBase; only AppInfo + Auth.login are [AllowAnonymous] (correct). Global query filters on every tenant-scoped entity enforce per-user isolation even if a query forgets its Where — covered by TenantIsolationTests.
• Injection: all data access via EF parameterisation; no raw SQL string concatenation in app code; FTS uses websearch_to_tsquery parameters. Frontend has zero dangerouslySetInnerHTML/innerHTML/eval; the search-highlight feature deliberately uses non-HTML sentinels rendered as escaped React elements (XSS-safe by construction).
• SSRF: SafeHttpGuard validates scheme, resolves DNS and checks every address against loopback/private/link-local/CGNAT/metadata/multicast (v4+v6, v4-mapped), fails closed on unknown families, and is paired with AllowAutoRedirect=false (DependencyInjection.cs:65). This is better than most production code.
• Secrets: none hardcoded (empty placeholders in appsettings.json); no .env ever committed (git history checked); real secrets live in git-ignored deploy/.env* and Gitea Actions secrets; pre-commit hook + CI gitleaks scan both guard regressions.
• Session cookies: HttpOnly, SameSite=Lax, Secure-always outside dev, 401-not-redirect for XHR. Forwarded headers only trusted from configured proxy CIDRs (V-08).
• Headers: nosniff, X-Frame-Options DENY, Referrer-Policy, COOP on API; HSTS in prod.

M-1 · No absolute session lifetime (sliding-only)

• File: Program.cs:58-59 — ExpireTimeSpan = 7d with SlidingExpiration = true.
• Why: a session that's touched at least weekly renews forever; a stolen cookie never expires as long as the attacker uses it. No server-side revocation list exists either (cookie is self-contained).
• Fix: add an absolute cap via an issued-at claim checked in OnValidatePrincipal (e.g. re-auth after 30 days), and/or a session-stamp validated against the DB to enable revocation. (The multi-provider design doc 02/06 already specs DB-backed sessions — this aligns.)
• Blast radius: isolated.

M-2 · No Content-Security-Policy on the SPA or API

• File: Program.cs:156-167 (comment says CSP "report-only for now" but none is actually set); frontend/nginx.conf serves the SPA without security headers.
• Why: CSP is the main mitigation layer against any future XSS slip; currently absent.
• Fix: add CSP (default-src 'self'; connect-src API origin; etc.) + nosniff/XFO to the SPA nginx config; optionally a report-only CSP on the API.
• Blast radius: isolated (nginx conf + one middleware line), needs SPA smoke-test (Chart.js inline styles etc.).

L-1 · CSRF: no antiforgery tokens (accepted-risk, documented here)

• SameSite=Lax cookies + strict CORS allowlist + JSON-only POST bodies make classic CSRF impractical in modern browsers. Acceptable for now; revisit if cookie SameSite is ever relaxed or non-JSON form endpoints are added.

L-2 · AllowedHosts: "*" (appsettings.json)

• Host-header filtering disabled; low risk behind the proxy but set it to the real hostnames at production deployment.

---

2. Data protection (top priority)

Inventory of sensitive data:

Data	Where	At-rest protection	Verdict
Google OAuth refresh/access tokens	users.EncryptedRefreshToken	Encrypted (Data Protection API, keys on /keys volume)	✅ good
Passwords	—	None stored (OAuth-only login; no password column exists)	✅ best-possible
Email bodies / subjects / snippets	emails.BodyText etc.	Plaintext in Postgres	⚠️ H-3 below
Sender names/addresses (PII)	senders, emails	Plaintext	part of H-3
User email + display name	users	Plaintext	part of H-3
DB password	deploy/.env (git-ignored) + Actions secret	not in repo	✅
Data Protection keys	/keys Docker volume	filesystem	M-3 below
IPs / payment / health data	—	not collected	✅ n/a

H-3 · Email content stored in plaintext at rest, with unbounded retention

• File: src/InboxIntel.Domain/Entities/Email.cs (BodyText, Snippet, Subject); Postgres pgdata volume; also flows into PDF/CSV/JSON exports and (when enabled) to the local Ollama process.
• Why: the entire product is a copy of the user's mailbox. On this self-hosted, single-operator deployment the DB lives on the operator's own disk — a defensible posture — but: (a) anyone with disk/volume/backup access reads all mail; (b) there is no retention or purge policy (mail persists even after unsubscribe/cleanup in Gmail; "remove account" flows don't exist yet); (c) the planned multi-user platform (docs/discovery/multi-provider) makes plaintext-bodies-readable-by-host-admin a real privacy issue (its own security doc promises "admins never read users' mail" — the DB must back that up).
• Fix (phased): 1) Document the current posture in README/threat model (deliberate, local-first). 2) Enable pgcrypto/field-level encryption or full-disk/volume encryption before any multi-user deployment. 3) Add a data-retention setting + purge job and an account-deletion path (GDPR-style erasure). 4) Ensure DB backups inherit the same protection.
• Blast radius: documentation = trivial; field-level encryption = large (touches search — FTS can't index encrypted columns; would need architectural decision). Recommend volume-level encryption + retention/deletion first.

M-3 · Data Protection keys stored unencrypted on the /keys volume

• File: Program.cs:26-28 — PersistKeysToFileSystem without ProtectKeysWith*.
• Why: whoever reads the volume can decrypt all stored refresh tokens. Same-disk-as-DB caveat applies, but keys and ciphertext living side-by-side weakens the encryption's value.
• Fix: ProtectKeysWithCertificate(...) (cert from env/secret), or OS-level DPAPI on Windows hosts; at minimum document the volume-permissions requirement.
• Blast radius: isolated, but requires a key-migration step for existing tokens.

M-4 · Default DB credentials in appsettings.json

• File: appsettings.json:3 — Password=inboxintel as the fallback connection string.
• Why: if a deployment forgets the env override, the app happily connects with a guessable password (compose enforces POSTGRES_PASSWORD but a non-compose deployment might not).
• Fix: empty the default and fail fast at startup with a clear message when unset.
• Blast radius: isolated (plus updating dev docs to use user-secrets).

Logging & transit — verified

• Logs: no token/password logging found; the one PII-ish log is SmtpEmailSender.cs:28 (recipient address + subject at Info when SMTP is unconfigured) — acceptable, downgrade to Debug if desired (L-3). Serilog request logging does not include query strings or bodies.
• Transit: TLS terminates at nginx (HSTS enabled in prod); API+DB bound to loopback/compose network only. DB connection itself is non-TLS — fine while Postgres is co-located on the compose network; must add SSL Mode=Require if the DB ever moves to another host (L-4).

---

3. Dependencies & supply chain

• dotnet list package --vulnerable --include-transitive: clean (all projects) — the earlier round of CVE pins (System.Text.Json 8.0.6 etc.) is holding, and CI re-checks on every PR.
• npm audit (production deps): 0 vulnerabilities.
• M-5 · npm audit (dev deps): vite 5.x depends on a vulnerable esbuild (1 moderate, 1 high, dev-server-only vectors). Not shipped to users and CI already scopes to prod deps — but the fix is a routine vite 5→6/7 upgrade. Effort S.
• Duplication (L-5): both MailKit (Infrastructure) and the frontend carry sizeable deps; react-grid-layout exists solely for the draggable dashboard the redesign plans to retire — candidate for removal with the Analytics migration. No abandoned packages spotted.
• CI supply-chain posture: gitleaks (full history) + NuGet/npm audits are required checks — good. No SAST/CodeQL (L-6, nice-to-have).

4. Error handling & reliability

• Good: global UseExceptionHandler + RFC7807 ProblemDetails (no stack traces to clients); Polly retry/backoff on Gmail sync; idempotent sync upserts; AI calls wrapped in try/catch with non-AI fallbacks ("sync must never fail because the AI provider is down").
• M-6 · Known EF model warning: Email has a global query filter but is the required end of the Email↔EmailLabel relationship — logged on every boot; can yield surprising results when filtered parents are excluded. Fix: matching filter on EmailLabel (one line) + test. (Long-noted in logs; this is the nudge to actually do it.)
• L-7 · Multi-step writes without explicit transactions: e.g. GoogleAuthEvents upsert and sync batches rely on EF's single-SaveChanges transactionality — mostly fine; the sync cursor-advance + batch-upsert pairing is the one place an explicit transaction would guard a mid-batch crash (currently self-heals via idempotent re-sync — acceptable).

5. Performance

• Fixed this cycle (verified live): relevance-ranked FTS with weighted tsvector + GIN; trigram GIN indexes for the previously non-sargable sender/domain .Contains(); HNSW vector index ready for semantic search.
• Remaining (all Low, roadmapped): offset pagination degrades on deep pages (keyset planned in docs/discovery/05); frontend ships one 678 KB JS bundle (no code-splitting — vite manualChunks/dynamic import, effort S); no HTTP caching/ETags on read-heavy endpoints (sidebar counts are fetched per page-load); docker-compose frontend nginx lacks gzip/brotli confirmation for the bundle.
• No N+1 patterns found (queries project with joins; aggregates precomputed in AnalyticsAggregate).

6. Code quality & architecture

• Clean Architecture discipline is genuinely observed (dependency rule intact; thin controllers; DTO mapping at boundaries). Config is env-driven throughout.
• L-8: AiService.GenerateQueryAsync returns raw LLM output as a search query (advisory-only, becomes a search string — harmless today; keep it that way when NL search lands: model output must stay data, never an executable/action).
• Dead-ish code: none significant; docs/specs/* and discovery docs are current.

7. Testing & CI

• 41 tests (unit: parser/guard/unsubscribe/embeddings-null; integration: authz challenge, tenant isolation, pagination clamp, search fallback ordering). Critical security invariants (IDOR filter, SSRF guard, confirmed-destructive) are tested — better than most.
• Gap (M-7): no live-Postgres test harness — FTS/ranking/fuzzy/pgvector paths are verified manually against staging (documented in commit messages) but not repeatably in CI. A Testcontainers-Postgres (pgvector image) job would convert those throwaway verifications into permanent regression tests. Effort M.
• Gap (L-9): no coverage for CleanupService/SyncService beyond compilation; no E2E of the OAuth flow (hard without creds — acceptable).
• CI: build+test+secrets+deps as required PR checks; auto-deploy to staging with a post-deploy health gate; prod is tag-gated. Solid. Missing: lint/format check in CI (the pre-commit hook enforces locally; add dotnet format --verify-no-changes job, effort S) (L-10).

---

Prioritized remediation plan

#	Finding	Sev	Fix effort	Blast radius
1	H-1 Enable FluentValidation auto-validation + 400 tests	High	S	Isolated (1 line + tests; verify clients)
2	H-2 Rate limiting (global + auth/export/unsub/AI policies)	High	S–M	Isolated (Program.cs + tests)
3	H-3 Data-at-rest posture: document now; retention setting + purge job + account-deletion; volume-encryption guidance; (defer field-level encryption decision)	High	S (doc) → M (retention) → L (encryption)	Doc: none · Retention: moderate · Encryption: large/architectural
4	M-3 Protect Data Protection keys at rest	Med	S–M	Isolated + key migration
5	M-1 Absolute session lifetime (+ revocation groundwork)	Med	S	Isolated
6	M-4 Remove default DB password; fail fast	Med	S	Isolated
7	M-2 CSP + security headers on the SPA nginx	Med	S	Isolated (needs SPA smoke test)
8	M-6 Fix EF query-filter warning (EmailLabel filter)	Med	S	Isolated + test
9	M-5 Vite upgrade (dev-dep CVEs)	Med	S	Frontend build only
10	M-7 Testcontainers live-Postgres CI job	Med	M	CI + new test project wiring
11	L-2/L-3/L-4/L-10 (AllowedHosts, SMTP log level, DB TLS note, CI format check)	Low	S each	Isolated
12	L-5 bundle split / grid-layout removal (with redesign)	Low	S–M	Frontend

Stopping here per Phase 1 instructions — no changes made. Awaiting approval of this plan (or an edited subset) before implementing anything.