# Multithreaded — Code Review

**Date:** 2026-04-12
**Version:** 0.3.2
**Reviewer:** Claude (Opus 4.6)
**Scope:** Full codebase review — all Rust source, SQL migrations, templates, CSS, JS, deploy config, tests

## Summary

Multithreaded is a forum application (~8,800 LOC Rust, ~20,800 total) built on Axum/PostgreSQL with MNW OAuth integration. 3-crate workspace (mt-core, mt-db, main app). 21 migrations, 21 HTML templates, 510 lines JS, 1,761 lines CSS. 225+ tests (35 unit + 190 integration). 0 clippy warnings. Security posture is strong. Code is clean and well-organized.

**Overall: A** — consistent with the standing audit grade. No new vulnerabilities found. Several minor structural items noted below.

---

## Findings

### [MEDIUM] Dependency advisories — 4 active

`cargo audit` reports 4 vulnerabilities:

| Crate | Advisory | Severity | Fix |
|-------|----------|----------|-----|
| aws-lc-sys 0.38.0 | RUSTSEC-2026-0044 | HIGH | `cargo update -p aws-lc-sys` (>=0.39.0) |
| aws-lc-sys 0.38.0 | RUSTSEC-2026-0048 | HIGH 7.4 | Same |
| rustls-webpki 0.103.9 | RUSTSEC-2026-0049 | — | `cargo update -p rustls-webpki` (>=0.103.10) |
| rand 0.8.5 + 0.9.2 | RUSTSEC-2026-0097 | — | Unsoundness with custom loggers (not applicable here, but upgrade when deps allow) |

Additionally 3 allowed warnings (rsa, lru — transitive, no direct fix available).

aws-lc-sys and rustls-webpki are straightforward `cargo update` fixes. The rand advisory (RUSTSEC-2026-0097) affects `rand::rng()` with custom loggers — MT doesn't use custom loggers, so no practical impact, but both 0.8.5 (direct dep) and 0.9.2 (transitive via axum/governor) are flagged. Consider bumping the direct `rand` dep from 0.8 to 0.9 when convenient.

### [MEDIUM] Three route files exceed 500-line guideline

Per project conventions, files with >500 lines of branching logic should be split.

| File | Lines | Content |
|------|-------|---------|
| `src/routes/mod.rs` | 578 | Route tree, form types, 15+ helper functions |
| `src/routes/forum/views.rs` | 628 | 11 view handlers |
| `src/routes/forum/actions.rs` | 600 | 10 action handlers |

`mod.rs` could split helpers into `routes/helpers.rs` (validation, permission checks, markdown rendering) and keep forms + route tree in `mod.rs`. The forum files could split by page area (thread views vs. listing views, thread actions vs. post actions).

`queries.rs` (1,503) and `mutations.rs` (879) are exempt — flat lists of SQL query functions with no branching logic.

### [LOW] Pagination partial not used consistently

`templates/pages/forum_directory.html` (lines 40-49) inlines pagination markup instead of using `{% include "partials/pagination.html" %}`. Only `thread.html` uses the partial. If the pagination design changes, it needs updating in two places.

### [LOW] CSS: `.form-inline-row` has contradictory display properties

```css
.form-inline-row { display: inline; gap: 0.25rem; flex-direction: row; align-items: center; }
```

`gap`, `flex-direction`, and `align-items` are flex/grid properties — they have no effect with `display: inline`. Used on 2 admin forms. Should be `display: inline-flex`.

### [LOW] No `Cache-Control` on HTML responses

Static files get caching from tower-http `ServeDir`. Image proxy sets `max-age=31536000`. But regular HTML pages have no explicit cache control headers. Adding `private, no-cache` for authenticated pages prevents stale content after logout/session change.

### [INFO] Test harness duplication

`tests/harness/mod.rs`: `new()` and `new_with_admin()` share ~40 lines of identical setup code. `tests/harness/client.rs`: `send_raw` and `send_raw_with_token` duplicate ~60 lines. Not a correctness issue, but a maintenance burden.

### [INFO] `MaybeUser` silently swallows session errors

In `auth.rs`, the `MaybeUser` extractor returns `MaybeUser(None)` on any session read error. This is intentionally infallible, but session store failures silently deauthenticate users without logging at the extraction point. A `tracing::warn!` on the error path would help debugging.

### [INFO] No index on `posts.removed_at`

The moderation page queries removed posts and thread views filter by removal status. At scale, a partial index `WHERE removed_at IS NOT NULL` on `posts` would help. Current data volumes don't require it.

---

## Strengths

- **Security is defense-in-depth.** CSRF (constant-time comparison, auto-injected), SSRF (comprehensive private IP blocking on link previews), XSS (docengine sanitization + Askama autoescaping), SQL injection (all parameterized), rate limiting (per-IP + per-user), HMAC internal API with replay protection, EXIF stripping on uploads, CSP headers. 11 dedicated XSS tests.
- **Immutable post model.** Posts cannot be edited or deleted by users — only corrected via footnotes or mod-removed (content preserved). Philosophically consistent and well-executed.
- **Test infrastructure is excellent.** Per-test database isolation. Cookie-aware HTTP client with auto CSRF extraction. Full Axum app in-process. 22 workflow test files covering every feature area.
- **Clean 3-crate separation.** mt-core has zero deps beyond chrono. mt-db has no web framework deps. Main crate handles all HTTP concerns. No circular dependencies.
- **Production hardening.** Systemd service with 18 security directives (NoNewPrivileges, ProtectSystem=strict, ProtectHome, PrivateTmp, RestrictAddressFamilies, etc.). Memory cap 512M. Graceful shutdown.
- **Quote verification.** SHA-256 hash of quoted text verified server-side before rendering attribution. Prevents fabricated quotes.
- **Observability.** 86+ `#[instrument(skip_all)]` annotations. Structured logging via tracing with env filter.

## Security Checklist

| Check | Status |
|-------|--------|
| SQL injection | Pass — all queries parameterized via SQLx |
| XSS | Pass — docengine sanitization + Askama autoescaping + CSP |
| CSRF | Pass — synchronizer token, constant-time comparison, all mutating routes |
| SSRF | Pass — link preview validates IPs (IPv4+IPv6 private ranges blocked) |
| Auth bypass | Pass — fail-closed access checks, session cycling on login |
| IDOR | Pass — all resource access checks use authenticated user context |
| Rate limiting | Pass — per-IP (tower-governor) + per-user (15 posts/60s) + upload rate (20/hr) |
| Secrets in source | Pass — env-based config, no secrets committed (env templates have placeholders) |
| Dependency advisories | Fail — 4 active (2 HIGH via aws-lc-sys, fixable with cargo update) |

## Metrics

| Metric | Value |
|--------|-------|
| Rust source LOC | ~8,800 |
| Total LOC (all files) | ~20,800 |
| Rust source files | 39 |
| Test files | 24 (harness + workflows) |
| Test count | 225+ (35 unit + 190 integration) |
| Tests/KLOC | ~26 |
| Clippy warnings | 0 |
| Migrations | 21 |
| HTML templates | 21 |
| Query functions | 55+ |
| Mutation functions | 30+ |
| Dependencies (direct) | 28 |
| Audit advisories | 4 (2 HIGH, 2 low/info) |

## Action Items

1. ~~**[MEDIUM]** Run `cargo update -p aws-lc-sys -p rustls-webpki` to fix 3 of 4 advisories~~ — Done. aws-lc-sys 0.39.1, rustls-webpki 0.103.11.
2. ~~**[MEDIUM]** Split `routes/mod.rs` helpers into `routes/helpers.rs`~~ — Done. mod.rs 281, helpers.rs 315.
3. ~~**[LOW]** Fix `.form-inline-row` CSS to `display: inline-flex`~~ — Done.
4. ~~**[LOW]** Use pagination partial in `forum_directory.html`~~ — Done.
5. ~~**[LOW]** Add `Cache-Control: private, no-cache` to HTML responses~~ — Done.
6. ~~**[MEDIUM]** Split `forum/views.rs` and `forum/actions.rs`~~ — Done. views.rs 424, thread.rs 224, posts.rs 443, actions.rs 172.
7. **[INFO]** Transitive dep advisories (rand, rsa, lru) — no fix available, monitor upstream.
8. **[INFO]** Add partial index on `posts.removed_at` when data volume warrants it.
9. **[INFO]** Add `tracing::warn!` to `MaybeUser` extractor on session read errors.