After the last post, the obvious question was: what did Code Reviewer miss the first time it reviewed Gopher? The answer was another five findings. Then inbox0 got the same treatment. The patterns are consistent enough now that I can predict the categories before I run the review.
The Finding That Appears in Every App: OAuth CSRF
The OAuth state parameter showed up again. In Gopher, Google OAuth had no state at all. In inbox0, Google OAuth had no state either. Microsoft OAuth in inbox0 was worse: get_microsoft_auth_url generated a random state and included it in the URL, but the value was never stored anywhere. The callback accepted any state -- or no state -- without complaint. It looked correct on a quick read. It was not.
The fix is the same everywhere: generate secrets.token_urlsafe(32) in the route handler, store it in request.session, pass it to the URL builder, then pop and validate it in the callback with secrets.compare_digest. The Microsoft case required adding the state query param to both callback signatures so FastAPI would bind it correctly.
The One That Surprised Me: chmod 666
inbox0 has a Docker entrypoint script that initialises the SQLite database. At the end of the script, one line: chmod 666 "$DB_FILE". World-readable and world-writable. Every process in the container, every user on a host with a misconfigured volume mount, could read the file directly. Session tokens, encrypted email credentials, sweep history -- all of it accessible to anything that could reach the filesystem.
The change is two characters: chmod 600. Owner read/write only. It is the kind of thing that starts as a quick workaround during development -- the app user could not write to the file, so permissions got opened up -- and then never gets revisited. The entrypoint also runs as root and the app probably does not, so the 600 approach requires the container to run as a consistent user. That is worth verifying separately.
Encryption Key to Stdout
When DB_ENCRYPTION_KEY was not set, inbox0's _get_encryption_key() generated a random key and printed it to stdout so the operator could copy it into their .env. Every container runtime that captures stdout -- Docker logs, systemd journal, CI/CD pipelines -- now has a copy of the encryption key. The fix is the same one applied to every other app: fail at startup with a clear error. The key is a required configuration item. If it is missing, the application should not start. Auto-generating cryptographic material and printing it to a log is not a feature.
The same function also had a hardcoded PBKDF2 salt (b"email_triageSalt2024") used when the supplied key was not already 32 bytes base64. A hardcoded salt defeats the purpose of salting -- any attacker with access to the source can precompute derived keys. The fix adds a DB_ENCRYPTION_SALT environment variable; the legacy hardcoded value stays as a fallback so existing deployments continue to decrypt their data without a migration. New deployments should set a unique salt.
CORS with Wildcard and Credentials
inbox0 configured CORSMiddleware with allow_origins=["*"] and allow_credentials=True simultaneously. This combination is a contradiction: the CORS specification does not permit wildcard origins when credentials are involved, and some implementations silently strip credentials or raise an error at runtime. Either way, the intent is clearly wrong. The fix: a CORS_ORIGINS environment variable that defaults to the production domain. The allowed methods are now explicit rather than ["*"].
The Gopher Round Two Findings
Beyond the OAuth CSRF, Gopher had four more issues from the second review pass. The Content-Security-Policy header was missing entirely -- the security headers middleware set X-Frame-Options, HSTS, and X-Content-Type-Options but never added a CSP. One line, added. The markdown renderer in app.js already called escapeHtml before processing, so XSS from raw input was blocked, but the output had no tag allowlist. A sanitizeMarkdownHtml function now strips anything that is not strong, em, ul, ol, li, br, p, code, or pre from the final output.
The invite redemption path had a read-check-write race: verify_invite_token confirmed the token was unused, then separately use_invite marked it consumed. Two concurrent requests could both pass the check before either one wrote. The fix adds AND used_by IS NULL to the UPDATE statement and checks the affected row count. Only the first write wins; the second gets zero rows and is rejected. The last finding was the require_admin function trusting the role from the session JOIN rather than explicitly re-fetching from the users table. The JOIN already returns the current value, so the data was effectively fresh -- but the explicit get_user_by_id call makes the intent unambiguous and removes any future risk from caching.
The Tally
Across DrawmaticAI, Code Reviewer, Gopher (two passes), and inbox0: thirty-three security findings closed. OAuth CSRF state appeared in every single app with an OAuth flow. Hardcoded admin credentials appeared in three. Ephemeral keys appeared in three. SSRF appeared in two. The review keeps finding the same categories in the same places because the same shortcuts get taken during development across every app. The value of running the review is not finding exotic bugs. It is finding the shortcuts before they reach production.