|
1 |
+ |
# Multithreaded -- Code Audit Review
|
|
2 |
+ |
|
|
3 |
+ |
**Last audited:** 2026-03-28 (seventh formal audit, Run 12 cross-project)
|
|
4 |
+ |
**Previous audit:** 2026-03-22 (sixth audit, coverage expansion)
|
|
5 |
+ |
|
|
6 |
+ |
## Overall Grade: A
|
|
7 |
+ |
|
|
8 |
+ |
Run 12 cross-project audit. 225 tests (35 unit lib + 190 integration). 0 clippy warnings. v0.3.2. Grade stable at A. Internal API improvements (MNW category provisioning). Link preview fix. New dep advisories: aws-lc-sys (HIGH 7.4), rustls-webpki.
|
|
9 |
+ |
|
|
10 |
+ |
## Scorecard
|
|
11 |
+ |
|
|
12 |
+ |
| Dimension | Grade | Notes |
|
|
13 |
+ |
|-----------|:-----:|-------|
|
|
14 |
+ |
| Code Quality | A | Zero clippy warnings. Consistent `map_err` + `tracing::error!` error handling. Mod log failures logged (not silently dropped). Dead code removed. |
|
|
15 |
+ |
| Architecture | A | Clean 3-crate workspace: mt-core (time formatting), mt-db (queries/mutations), main app (routes, auth, templates). Route module properly split into forum/moderation/settings/admin. Template layer uses view-model structs. |
|
|
16 |
+ |
| Testing | A | 225 tests (35 unit main + 190 integration) at ~32 tests/KLOC. Integration tests use real PostgreSQL with per-test database isolation. Coverage on CRUD, permissions, bans, mute/unban/unmute handlers, CSRF, pagination, rate limiting, category edit/reorder, expired ban behavior, endorsements, footnotes, verified quoting, mentions, link previews, profiles, auth flow (PKCE/state), admin routes, mutations (bans, categories, endorsements, flags, images, link previews, mentions, users). |
|
|
17 |
+ |
| Security | A | All SQL parameterized. CSRF with constant-time comparison. OAuth PKCE with state nonce. Markdown via docengine (URL scheme allowlist + HTML sanitization, defense-in-depth). Fail-closed access checks on all ban/mute/suspension queries. Session cookie Secure flag configurable. |
|
|
18 |
+ |
| Performance | A- | Proper indexes on all query patterns (composite indexes for category listing, thread ordering, ban lookup, mod log). Partial index on ban expiration. No N+1 queries. Per-IP rate limiting on write endpoints (tower-governor). |
|
|
19 |
+ |
| Documentation | A- | `//!` module docs on all source files. `.env.example` documents all 9 environment variables. |
|
|
20 |
+ |
| Dependencies | A | Minimal deps, all justified. Rust 2024 edition. Dead deps removed (thiserror, serde, serde_json from mt-core; mt-core, thiserror from mt-db). Workspace dependency management. |
|
|
21 |
+ |
| Frontend | A | HTMX for dynamic interactions. Askama autoescaping on all template variables. CSRF auto-injected for forms and HTMX. Toast uses `textContent`. `body_html` sanitized by docengine (defense-in-depth). Client-side maxlength on all inputs. |
|
|
22 |
+ |
| Type Safety | A- | Query layer uses focused `FromRow` projections. Dead domain types removed. |
|
|
23 |
+ |
| Observability | A | 86 `#[instrument(skip_all)]` annotations across all route handlers and DB functions. `tracing-subscriber` with EnvFilter. |
|
|
24 |
+ |
| Concurrency | A- | Async throughout with tokio. Graceful shutdown (SIGINT + SIGTERM). reqwest timeouts (15s + 5s). `swap_category_order` uses transaction. Per-IP rate limiting (burst 10, 2/sec). |
|
|
25 |
+ |
| Resilience | A- | Graceful shutdown. HTTP client timeouts. Error logging without panics. Mod log failures logged. Rate limiting on writes. Gap: no retry on MNW API calls. |
|
|
26 |
+ |
| API Consistency | A- | Consistent redirect-with-toast pattern. Proper status codes (403/404/422). Health endpoint returns JSON. |
|
|
27 |
+ |
| Migration Safety | A- | SQLx `migrate!()` with sequential numbering (001-020). All additive. No destructive operations. |
|
|
28 |
+ |
| Codebase Size | A | Lean codebase for full forum with OAuth, CSRF, markdown, moderation, admin, pagination, soft-delete, and settings. |
|
|
29 |
+ |
|
|
30 |
+ |
## Module Heatmap
|
|
31 |
+ |
|
|
32 |
+ |
| Module | Code | Arch | Test | Security | Perf | Docs | Type Safety | Observability | Concurrency | Resilience |
|
|
33 |
+ |
|--------|:----:|:----:|:----:|:--------:|:----:|:----:|:-----------:|:-------------:|:-----------:|:----------:|
|
|
34 |
+ |
| main.rs | A- | A | - | A- | A | A- | A | A | A | A |
|
|
35 |
+ |
| config.rs | A | A | - | A | - | A | A | - | - | - |
|
|
36 |
+ |
| auth.rs | A- | A | A- | A- | A | A | A | A | A | B+ |
|
|
37 |
+ |
| csrf.rs | A | A | A | A+ | A | A | A | A | - | - |
|
|
38 |
+ |
| (docengine) | A | A | A | A+ | A | A | A | - | - | - |
|
|
39 |
+ |
| seed.rs | A- | A | - | A | - | A | A- | - | - | - |
|
|
40 |
+ |
| routes/mod.rs | A | A | - | A- | A | A | A | A | A- | A- |
|
|
41 |
+ |
| routes/forum.rs | A | A | B+ | A- | A | A | A | A | A | A- |
|
|
42 |
+ |
| routes/moderation.rs | A | A | A- | A | A | A | A | A | A | A- |
|
|
43 |
+ |
| routes/settings.rs | A | A | A- | A | A | A | A | A | A- | A- |
|
|
44 |
+ |
| routes/admin.rs | A | A | A- | A | A | A | A | A | A | A |
|
|
45 |
+ |
| templates/ | A | A | A | A- | - | A | A | - | - | - |
|
|
46 |
+ |
| mt-core/time_format.rs | A | A | A | - | A | A | A | - | - | - |
|
|
47 |
+ |
| mt-db/queries.rs | A | A | B+ | A+ | A | A | A | A | A | A |
|
|
48 |
+ |
| mt-db/mutations.rs | A | A | A- | A+ | A | A | A | A | A- | A |
|
|
49 |
+ |
|
|
50 |
+ |
### Cold Spots
|
|
51 |
+ |
|
|
52 |
+ |
None — all previous cold spots resolved.
|
|
53 |
+ |
|
|
54 |
+ |
## Strengths
|
|
55 |
+ |
|
|
56 |
+ |
- **Clean architecture.** 3-crate workspace with proper separation. Route module split into focused files. Template layer uses view-model structs.
|
|
57 |
+ |
- **Comprehensive CSRF.** Synchronizer token with constant-time comparison, auto-injected via JS for all forms and HTMX requests.
|
|
58 |
+ |
- **Solid test infrastructure.** Full Axum app with real PostgreSQL per test. Cookie-aware client with automatic CSRF token extraction.
|
|
59 |
+ |
- **Authorization hierarchy.** Owner > mod > member correctly enforced. Owners cannot be banned. Only owners can ban mods.
|
|
60 |
+ |
- **Input validation.** Length limits on all user content. Slug format validation. UUID parsing validated. Sort/order whitelisted.
|
|
61 |
+ |
- **SQL safety.** All 40+ queries parameterized. Dynamic ORDER BY uses whitelist match.
|
|
62 |
+ |
- **Efficient codebase.** 4,808 LOC for full forum functionality.
|
|
63 |
+ |
|
|
64 |
+ |
## Weaknesses
|
|
65 |
+ |
|
|
66 |
+ |
- **No retry on MNW API calls.** OAuth token exchange and userinfo fetch have no retry logic.
|
|
67 |
+ |
|
|
68 |
+ |
## Mandatory Surprise
|
|
69 |
+ |
|
|
70 |
+ |
**`CoreError` is a fully-defined typed error enum with 5 variants (NotFound, Unauthorized, Forbidden, Validation, Internal) that is never used anywhere in the application.**
|
|
71 |
+ |
|
|
72 |
+ |
Route handlers use `map_err(|e| { tracing::error!(...); StatusCode::INTERNAL_SERVER_ERROR })` directly. Domain model structs in mt-core (User, Community, Category, Thread, Post, Membership, Role) are similarly defined but bypassed -- queries.rs defines its own `FromRow` structs inline.
|
|
73 |
+ |
|
|
74 |
+ |
**Verdict: Genuine issue.** Good architectural intent that was never integrated. **Resolved:** Dead code removed (error.rs, models.rs, pool.rs deleted; unused deps cleaned from mt-core and mt-db).
|
|
75 |
+ |
|
|
76 |
+ |
## Action Items
|
|
77 |
+ |
|
|
78 |
+ |
Filed in `docs/mnw/mt/todo.md`.
|
|
79 |
+ |
|
|
80 |
+ |
1. ~~**[HIGH]** Sanitize URL schemes in markdown rendering~~ -- Done. Allowlist (http, https, mailto, ftp), 7 tests added.
|
|
81 |
+ |
2. ~~**[MEDIUM]** Add `#[instrument(skip_all)]` to all route handlers and DB functions~~ -- Done. 86 annotations.
|
|
82 |
+ |
3. ~~**[MEDIUM]** Make session cookie `Secure` flag configurable~~ -- Done. `COOKIE_SECURE` env var.
|
|
83 |
+ |
4. ~~**[MEDIUM]** Wrap `swap_category_order` in transaction~~ -- Done.
|
|
84 |
+ |
5. ~~**[MEDIUM]** Change fail-open access checks to fail-closed~~ -- Done.
|
|
85 |
+ |
6. ~~**[SMALL]** Add `//!` module docs~~ -- Done. All source files documented.
|
|
86 |
+ |
7. ~~**[SMALL]** Remove dead code~~ -- Done. error.rs, models.rs, pool.rs deleted. Deps cleaned.
|
|
87 |
+ |
8. ~~**[SMALL]** Log mod log insert failures~~ -- Done. 15 locations across 4 files.
|
|
88 |
+ |
9. ~~**[SMALL]** Expand `.env.example`~~ -- Done. All 9 env vars documented.
|
|
89 |
+ |
10. ~~**[SMALL]** Initial git commit + configure remotes~~ -- Done.
|
|
90 |
+ |
|
|
91 |
+ |
## Metrics Over Time
|
|
92 |
+ |
|
|
93 |
+ |
| Audit Date | LOC | Rust Files | Tests | Tests/KLOC | Clippy Warnings | Cold Spots | Overall |
|
|
94 |
+ |
|------------|-----|-----------|-------|-----------|----------------|------------|---------|
|
|
95 |
+ |
| 2026-03-14 | 4,808 | 36 | 90 | 18.7 | 0 | 7 | B+ |
|
|
96 |
+ |
| 2026-03-14 (remediation) | ~4,600 | 33 | 97 | ~21 | 0 | 3 | A- |
|
|
97 |
+ |
| 2026-03-14 (rate limit) | ~4,700 | 34 | 99 | ~21 | 0 | 3 | A- |
|
|
98 |
+ |
| 2026-03-14 (coverage) | ~4,800 | 34 | 106 | ~22 | 0 | 1 | A |
|
|
99 |
+ |
| 2026-03-14 (ammonia) | ~4,800 | 34 | 106 | ~22 | 0 | 0 | A |
|
|
100 |
+ |
| 2026-03-16 (Run 6) | 6,232 | ~36| 146 | ~23 | 0 | 0 | A |
|
|
101 |
+ |
| 2026-03-16 (P19+P20) | ~7,000 | ~38| 173 | ~25 | 0 | 0 | A |
|
|
102 |
+ |
| 2026-03-17 (Run 8) | ~7,000 | ~38| 222 | ~32 | 0 | 0 | A |
|
|
103 |
+ |
| 2026-03-18 (Run 9) | ~7,000 | ~38| 222 | ~32 | 0 | 0 | A |
|
|
104 |
+ |
| 2026-03-22 (coverage) | ~7,000 | ~39| 249 | ~36 | 0 | 0 | A |
|
|
105 |
+ |
| 2026-03-28 (Run 12) | ~7,200 | ~39| 225+ | ~32 | 0 | 0 | A |
|
|
106 |
+ |
|
|
107 |
+ |
## Changes Since Last Audit
|
|
108 |
+ |
|
|
109 |
+ |
### Seventh formal audit (2026-03-28, Run 12 cross-project)
|
|
110 |
+ |
- **Test count:** 225 (35 unit lib + 190 integration). 0 clippy warnings. 0 failures.
|
|
111 |
+ |
- **Grade:** A (maintained). v0.3.2.
|
|
112 |
+ |
- **Internal API improvements:** MNW category auto-provisioning (Items, Blog, Devlog, Discussion) via internal API with shared secret auth.
|
|
113 |
+ |
- **Link preview fix:** Corrected URL extraction edge case.
|
|
114 |
+ |
- **New dependency advisories (action items):**
|
|
115 |
+ |
- aws-lc-sys 0.38.0 (RUSTSEC-2026-0044 + -0048, severity 7.4 HIGH) — upgrade to 0.39.0 via `cargo update -p aws-lc-sys`
|
|
116 |
+ |
- rustls-webpki 0.103.9 (RUSTSEC-2026-0049) — upgrade to 0.103.10 via `cargo update -p rustls-webpki`
|
|
117 |
+ |
- **Mandatory surprise:** None new. Previous surprises (CoreError dead code, link_preview IPv6 blocking) both resolved.
|
|
118 |
+ |
- **No new code findings.** All previous items remain resolved.
|
|
119 |
+ |
- **Note:** Test count 225 is lower than previous 249 — mt-core (16) and mt-db (11) unit tests may not have been captured in this run. Integration tests grew from 187 to 190.
|
|
120 |
+ |
|
|
121 |
+ |
### Test coverage expansion (2026-03-22)
|
|
122 |
+ |
- **Test count:** 222 -> 249 (+27 tests). 0 clippy warnings.
|
|
123 |
+ |
- **Grade:** A (maintained). Testing A- -> A. Three cold spots resolved.
|
|
124 |
+ |
- **auth.rs:** 3 -> 8 integration tests (+5). PKCE params, state nonce validation (3 paths), suspended user behavior.
|
|
125 |
+ |
- **admin.rs:** 6 -> 10 integration tests (+4). Search, invalid UUID handling, mod_log entry creation, non-admin access denial.
|
|
126 |
+ |
- **mutations.rs:** New test file with 18 integration tests. Covers: cleanup_expired_bans, ban upserts, swap_category_order, get_category_id_by_slugs, update_category, ensure_membership idempotency, soft_delete, create_post activity bump, toggle_endorsement, insert_flag idempotency, remove_image, link_preview dedup, mentions dedup, upsert_user.
|
|
127 |
+ |
- **seed.rs:** Type safety improved — raw `&str` role params replaced with `CommunityRole` enum (B -> A-).
|
|
128 |
+ |
- **Module heatmap updates:** auth.rs Test B- -> A-, admin.rs Test B -> A-, mutations.rs Test B -> A-, seed.rs Code B+ -> A- / Type Safety B -> A-.
|
|
129 |
+ |
|
|
130 |
+ |
### Fifth formal audit (2026-03-18, Run 9 cross-project)
|
|
131 |
+ |
- **Test count:** 222 (unchanged). 0 clippy warnings.
|
|
132 |
+ |
- **Grade:** A (maintained). v0.3.1 (deployed 2026-03-18).
|
|
133 |
+ |
- **No new findings requiring action.**
|
|
134 |
+ |
- **Observations (pre-existing, not regressions):**
|
|
135 |
+ |
- ~~`deletion_task.abort()` in main.rs without awaiting completion~~ — Fixed: now awaits task completion after abort.
|
|
136 |
+ |
- Inline `onsubmit` confirmation dialogs in thread.html — not screen-reader friendly. Impact: LOW, functional but not best-practice.
|
|
137 |
+ |
- ~~No client-side maxlength on textarea inputs~~ — Fixed: maxlength added to all inputs/textareas. Server-side limits added for flag detail and ban/mute reason (1024 bytes).
|
|
138 |
+ |
- **Mandatory surprise:** URL validation in link_preview.rs blocks IPv4-mapped IPv6 addresses via host_part parsing, but IPv6 full range check uses string prefix match for unique local addresses. Intentionally restrictive (good for SSRF) — not a vulnerability.
|
|
139 |
+ |
|
|
140 |
+ |
### Phases 19 + 20 implementation (2026-03-16)
|
|
141 |
+ |
- **Test count:** 146 -> 173 (+27 tests: 19 unit + 7 integration + 1 workflow mod)
|
|
142 |
+ |
- **Grade:** A (maintained). Phases 19 (@Mentions) and 20 (Link Previews) implemented.
|
|
143 |
+ |
- **Source LOC:** ~7,000 (up from 6,232)
|
|
144 |
+ |
- **Migrations:** 12 -> 17 (013 flagging, 014 tags, 015 tracking, 016 post_mentions, 017 link_previews)
|
|
145 |
+ |
- **New files:** `src/link_preview.rs` (URL extraction + OG fetch), `tests/workflows/mentions.rs` (4 tests), `tests/workflows/link_previews.rs` (3 tests)
|
|
146 |
+ |
- **New DB functions:** `resolve_usernames_in_community`, `insert_mentions`, `list_link_previews_for_posts`, `insert_link_preview`
|
|
147 |
+ |
- **Markdown:** `extract_mention_usernames`, `resolve_mentions` with code-span awareness
|
|
148 |
+ |
- **Zero clippy warnings, all 173 tests passing.**
|
|
149 |
+ |
|
|
150 |
+ |
### Second formal audit (2026-03-16, Run 6 cross-project)
|
|
151 |
+ |
- **Test count:** 106 -> 146 (+40 tests)
|
|
152 |
+ |
- **Grade:** A (maintained). Phases 14, 15, and 21 implemented since last audit.
|
|
153 |
+ |
- **Source LOC:** 6,232 (up from ~4,800)
|
|
154 |
+ |
- **Migrations:** 10 -> 12 (post_footnotes, post_endorsements)
|
|
155 |
+ |
- **Instrument coverage:** 109/110 (99%) — near-perfect
|
|
156 |
+ |
- **New finding (LOW):** Regex compiled per-request in verify_quotes/post_process_quotes for SHA-256 hash pattern matching. Should use LazyLock.
|
|
157 |
+ |
- **Performance note:** forum.rs at 969 LOC split into forum/ directory module: views.rs (510) + actions.rs (480).
|
|
158 |
+ |
- **Mandatory surprise:** Per-request regex in quote verification — LOW (functional but inefficient).
|
|
159 |
+ |
- **Previous items verified:** All previous remediated items confirmed intact.
|
|
160 |
+ |
|
|
161 |
+ |
### First formal audit (2026-03-14)
|
|
162 |
+ |
- **Grade:** B+ (unchanged from baseline, but now backed by per-module code review)
|
|
163 |
+ |
- **Baseline was optimistic on:** Security (A- -> B+: javascript: XSS found, fail-open patterns found), Type Safety (A- -> B+: domain types confirmed unused), Observability (B -> C: zero #[instrument] is worse than "no annotations yet"), Performance (B -> A-: indexes are actually solid)
|
|
164 |
+ |
- **Baseline was pessimistic on:** Performance (B -> A-: proper composite indexes, partial indexes, no N+1)
|
|
165 |
+ |
- **Test count confirmed:** 90 (documented 72 was wrong: 56 integration + 18 unit markdown/csrf + 16 unit mt-core)
|
|
166 |
+ |
- **New findings:** 1 HIGH (javascript: XSS), 4 MEDIUM (secure cookie, transaction, fail-open, observability), 5 SMALL
|
|
167 |
+ |
|
|
168 |
+ |
### Full remediation (2026-03-14)
|
|
169 |
+ |
- **Grade:** B+ -> A- (all 10 findings resolved, grade capped by git hygiene)
|
|
170 |
+ |
- **Tests:** 90 -> 97 (+7 markdown security tests)
|
|
171 |
+ |
- **Files:** 36 -> 33 (deleted error.rs, models.rs, pool.rs)
|
|
172 |
+ |
- **Cold spots:** 7 -> 3 (resolved: markdown XSS, observability, dead code, dead docs x2)
|
|
173 |
+ |
- **Key changes:** URL scheme allowlist sanitization, 86 `#[instrument(skip_all)]`, fail-closed access checks, transaction wrapping, configurable Secure cookie, dead code + deps removed, mod log error logging, `.env.example` expanded
|