Skip to main content

max / makenotwork

server: adopt lint block, fix clippy, fmt Green under 'SQLX_OFFLINE=true cargo clippy --all-targets -- -D warnings': let-else, write!/writeln! over format!-push, digit separators, explicit imports, Option<&T> over &Option<T>, borrow by-value params, bool-params to structs, #[must_use], concrete Default types. Scoped #[allow]s (with reasons) only for an axum async handler, tri-state PATCH option_option, zero-sized capability-witness maps, test float compares, and a test fixture's field convention. No SQL/route/auth/serde-field changes.
Co-Authored-By
Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-24 16:13 UTC
Signed with PGP, not checked
Commit: f82abccd4e883ec608bcfe2b8a2750ae12c1dd95
Parent: 7a2b0ec
495 files changed, +6352 insertions, -7127 deletions
@@ -52,12 +52,12 @@
52 52 **Rules:**
53 53 - Use `?` for error propagation. Never `.unwrap()` in production code.
54 54 - Use `.ok_or(AppError::NotFound)?` when an optional DB result must exist.
55 - - Use `AppError::BadRequest("message")` for user-caused errors — the string is shown directly.
55 + - Use `AppError::BadRequest("message")` for user-caused errors. The string is shown directly.
56 56 - Use `AppError::Validation("message")` for form validation failures (returns 422).
57 57 - Never expose internal error details to users. `Database` and `Internal` variants always show "Something went wrong."
58 58 - Convert external errors with `From` impls, not string formatting. Add `#[from]` to AppError variants for automatic conversion.
59 59
60 - On API routes (`/api/*`), a middleware layer (`json_error_layer`) automatically converts HTML error responses to `{"error": "message"}` JSON. Handlers don't need to handle this — it's transparent.
60 + On API routes (`/api/*`), a middleware layer (`json_error_layer`) automatically converts HTML error responses to `{"error": "message"}` JSON. Handlers don't need to handle this. It's transparent.
61 61
62 62 ## Route Handlers
63 63
@@ -85,12 +85,12 @@
85 85 - Every handler gets `#[tracing::instrument(skip_all, name = "...")]` for structured logging.
86 86 - Return type is always `Result<impl IntoResponse>`.
87 87 - Extract auth requirements via the type system: `AuthUser` (login required), `MaybeUser` (optional), `AdminUser` (admin only, returns 404 to hide admin routes from non-admins).
88 - - Askama templates implement `IntoResponse` — return the struct directly.
88 + - Askama templates implement `IntoResponse`. Return the struct directly.
89 89 - CSRF token goes into every template that renders forms.
90 90
91 91 ### HTMX Responses
92 92
93 - Full-page handlers extend `base.html` and include `session_user`, `csrf_token`, navigation, etc. HTMX handlers return partial templates — HTML fragments without the base layout.
93 + Full-page handlers extend `base.html` and include `session_user`, `csrf_token`, navigation, etc. HTMX handlers return partial templates (HTML fragments without the base layout).
94 94
95 95 ```rust
96 96 // Full page — extends base.html
@@ -161,7 +161,7 @@
161 161 - Always use positional parameters (`$1`, `$2`, ...). Never interpolate values into SQL strings.
162 162 - Use `sqlx::query_as::<_, DbRow>` for typed results. Use `sqlx::query!` only when the macro's compile-time checking is needed.
163 163 - `.fetch_one()` when exactly one row expected (errors on zero), `.fetch_optional()` when zero or one, `.fetch_all()` for lists.
164 - - Newtype ID wrappers (`UserId`, `ProjectId`, etc.) work directly with `.bind()` — they implement sqlx's `Encode`/`Decode`.
164 + - Newtype ID wrappers (`UserId`, `ProjectId`, etc.) work directly with `.bind()`. They implement sqlx's `Encode`/`Decode`.
165 165 - Multi-line SQL uses `r#"..."#` raw strings.
166 166
167 167 ### DB Row Types vs View Types
@@ -197,7 +197,7 @@
197 197 define_pg_uuid_id!(UserId, ProjectId, ItemId, VersionId, /* ... */);
198 198 ```
199 199
200 - This generates `UserId(Uuid)` with `Display`, `FromStr`, `sqlx::Type`, `Encode`, `Decode`, `Serialize`, `Deserialize`, and `Default` (generates new v4 UUID). Use these everywhere — never pass raw `Uuid` or `String` for IDs.
200 + This generates `UserId(Uuid)` with `Display`, `FromStr`, `sqlx::Type`, `Encode`, `Decode`, `Serialize`, `Deserialize`, and `Default` (generates new v4 UUID). Use these everywhere. Never pass raw `Uuid` or `String` for IDs.
201 201
202 202 ### String Enums
203 203
@@ -229,7 +229,7 @@
229 229 - Create indexes after the table definition, in the same migration.
230 230 - Prefer additive migrations (add columns, add tables). Destructive changes (drop columns, rename tables) need careful planning.
231 231 - Name migrations descriptively: `NNN_what_it_does.sql`.
232 - - **Indexes on growth tables must be `CONCURRENTLY`.** A plain `CREATE INDEX` takes an `ACCESS EXCLUSIVE` lock and blocks writes for the whole build — fine on a small table, a production write-stall on `transactions`/`page_views`/`subscriptions`/etc. `CREATE INDEX CONCURRENTLY` cannot run inside a transaction, so a migration that uses it **must start with the exact line `-- no-transaction`** (sqlx runs that file outside its per-migration transaction). Note the trade-off: a `-- no-transaction` migration is not atomic, so keep it to the single concurrent index build.
232 + - **Indexes on growth tables must be `CONCURRENTLY`.** A plain `CREATE INDEX` takes an `ACCESS EXCLUSIVE` lock and blocks writes for the whole build: fine on a small table, a production write-stall on `transactions`/`page_views`/`subscriptions`/etc. `CREATE INDEX CONCURRENTLY` cannot run inside a transaction, so a migration that uses it **must start with the exact line `-- no-transaction`** (sqlx runs that file outside its per-migration transaction). Note the trade-off: a `-- no-transaction` migration is not atomic, so keep it to the single concurrent index build.
233 233
234 234 ```sql
235 235 -- no-transaction
@@ -237,7 +237,7 @@
237 237 ON transactions (seller_user_id, created_at DESC);
238 238 ```
239 239
240 - The `migration_hygiene` test (`tests/migration_hygiene.rs`) enforces both rules — concurrent-on-growth-table and `IF NOT EXISTS` — for every migration past the frozen high-water mark (historical migrations can't change: sqlx checksums applied files). Bump `HIGH_WATER` there only after deliberately reviewing the migrations you're grandfathering.
240 + The `migration_hygiene` test (`tests/migration_hygiene.rs`) enforces both rules (concurrent-on-growth-table and `IF NOT EXISTS`) for every migration past the frozen high-water mark (historical migrations can't change: sqlx checksums applied files). Bump `HIGH_WATER` there only after deliberately reviewing the migrations you're grandfathering.
241 241
242 242 ```sql
243 243 -- Example: 004_file_scan_status.sql
@@ -284,8 +284,8 @@
284 284 ### Template Variables
285 285
286 286 Every full-page template needs at minimum:
287 - - `csrf_token: Option<String>` — for the CSRF meta tag
288 - - `session_user: Option<SessionUser>` — for the header (login state, avatar)
287 + - `csrf_token: Option<String>`, for the CSRF meta tag
288 + - `session_user: Option<SessionUser>`, for the header (login state, avatar)
289 289
290 290 ## Frontend Performance
291 291
@@ -459,11 +459,11 @@
459 459 - **Rust 2024 edition** (Rust 1.85+). Uses `gen` keyword restrictions and other 2024 features.
460 460 - No `.unwrap()` in production code. Use `?`, `.ok_or()`, or `unwrap_or_default()`.
461 461 - Prefer `Option::and_then`/`map` over `if let Some`/`match` for simple transforms.
462 - - File size guideline per root `CONTRIBUTING.md`: 500-line limit on branching logic, flat lists exempt. Route files follow the same rule — split into directory modules when they grow beyond 500 lines.
462 + - File size guideline per root `CONTRIBUTING.md`: 500-line limit on branching logic, flat lists exempt. Route files follow the same rule. Split into directory modules when they grow beyond 500 lines.
463 463
464 464 ## Dependencies
465 465
466 - Always use the latest stable release of every dependency. When upgrading introduces breaking API changes, update the code — never pin old versions to avoid migration work.
466 + Always use the latest stable release of every dependency. When upgrading introduces breaking API changes, update the code. Never pin old versions to avoid migration work.
467 467
468 468 ## Deployment
469 469
@@ -175,3 +175,35 @@
175 175 proptest = "1"
176 176 wiremock = "0.6"
177 177 pom-contract = { path = "../shared/pom-contract" }
178 +
179 + [lints.rust]
180 + unused = "warn"
181 + unreachable_pub = "warn"
182 +
183 + [lints.clippy]
184 + pedantic = { level = "warn", priority = -1 }
185 + # Allow-list tuned from a measured breakdown across server/multithreaded/pter
186 + # (2026-07-22). These are the high-churn / low-signal pedantic lints; everything
187 + # else in `pedantic` stays a warning. Keep this block identical across repos.
188 + module_name_repetitions = "allow"
189 + # Doc lints. No docs-completeness push is underway.
190 + missing_errors_doc = "allow"
191 + missing_panics_doc = "allow"
192 + doc_markdown = "allow"
193 + # Numeric casts. Endemic and mostly intentional in size and byte math.
194 + cast_possible_truncation = "allow"
195 + cast_sign_loss = "allow"
196 + cast_precision_loss = "allow"
197 + cast_possible_wrap = "allow"
198 + cast_lossless = "allow"
199 + # Subjective structure and style nags. High churn, low signal.
200 + must_use_candidate = "allow"
201 + too_many_lines = "allow"
202 + struct_excessive_bools = "allow"
203 + similar_names = "allow"
204 + items_after_statements = "allow"
205 + single_match_else = "allow"
206 + # Frequent false-positives in TUI and router-heavy code.
207 + match_same_arms = "allow"
208 + unnecessary_wraps = "allow"
209 + type_complexity = "allow"
M server/build.rs +9 -12
@@ -14,11 +14,11 @@
14 14 .map(|s| s.trim().to_string())
15 15 .unwrap_or_default();
16 16
17 - println!("cargo::rustc-env=GIT_HASH={}", hash);
17 + println!("cargo::rustc-env=GIT_HASH={hash}");
18 18 // Only re-run when HEAD changes
19 19 println!("cargo::rerun-if-changed=.git/HEAD");
20 20
21 - // Compile the TypeScript frontend to static/dist/ (best-effort — see fn).
21 + // Compile the TypeScript frontend to static/dist/ (best-effort, see fn).
22 22 build_frontend();
23 23
24 24 // --- Static asset fingerprinting ---
@@ -35,7 +35,7 @@
35 35
36 36 let mut hasher = DefaultHasher::new();
37 37 for path in &static_files {
38 - println!("cargo::rerun-if-changed={}", path);
38 + println!("cargo::rerun-if-changed={path}");
39 39 if let Ok(content) = fs::read(path) {
40 40 content.hash(&mut hasher);
41 41 }
@@ -52,18 +52,17 @@
52 52 let partial = format!(
53 53 r#" <link rel="preload" href="/static/fonts/Lato-Regular.woff2" as="font" type="font/woff2" crossorigin>
54 54 <link rel="preload" href="/static/fonts/ysrf.woff2" as="font" type="font/woff2" crossorigin>
55 - <link rel="stylesheet" href="/static/style.css?v={v}">
55 + <link rel="stylesheet" href="/static/style.css?v={version}">
56 56 <link rel="icon" href="/static/images/favicon.ico" type="image/x-icon">
57 57 <script src="/static/htmx.min.js"></script>
58 - <script src="/static/upload.js?v={v}"></script>
59 - <script type="module" src="/static/dist/core/index.js?v={v}"></script>"#,
60 - v = version,
58 + <script src="/static/upload.js?v={version}"></script>
59 + <script type="module" src="/static/dist/core/index.js?v={version}"></script>"#,
61 60 );
62 61
63 62 write_if_changed(Path::new("templates/_head_assets.html"), &partial);
64 63
65 64 // Per-page island loader macro. Heavy/page-specific islands (media player,
66 - // uploader, …) load on the pages that use them via
65 + // uploader, ...) load on the pages that use them via
67 66 // `{% import "_island.html" as island %}{% call island::island("name") %}`,
68 67 // cache-busted by the same content hash as the head assets.
69 68 let island_partial = r#"{% macro island(name) -%}
@@ -76,9 +75,7 @@
76 75
77 76 /// Write `contents` to `path` only if it differs, to avoid needless rebuilds.
78 77 fn write_if_changed(path: &Path, contents: &str) {
79 - let needs_write = fs::read_to_string(path)
80 - .map(|existing| existing != contents)
81 - .unwrap_or(true);
78 + let needs_write = fs::read_to_string(path).map_or(true, |existing| existing != contents);
82 79 if needs_write {
83 80 fs::write(path, contents)
84 81 .unwrap_or_else(|e| panic!("failed to write {}: {e}", path.display()));
@@ -90,7 +87,7 @@
90 87 ///
91 88 /// Self-contained: on a fresh checkout or a new build host (no `node_modules`)
92 89 /// it runs `npm ci` first, so there is no manual install gate before a deploy.
93 - /// Best-effort and non-fatal otherwise — an absent Node or a compile error only
90 + /// Best-effort and non-fatal otherwise, an absent Node or a compile error only
94 91 /// emits a `cargo::warning` and leaves the Rust build to succeed against
95 92 /// whatever `static/dist/` already holds. Set `MNW_SKIP_FRONTEND_BUILD=1` to
96 93 /// opt out entirely (e.g. a Node-less CI that doesn't need the JS).
@@ -10,7 +10,7 @@
10 10
11 11 fn main() {
12 12 let args: Vec<String> = env::args().collect();
13 - let password = args.get(1).map(|s| s.as_str()).unwrap_or("demo123");
13 + let password = args.get(1).map_or("demo123", std::string::String::as_str);
14 14
15 15 let salt = SaltString::generate(&mut OsRng);
16 16 let argon2 = Argon2::default();
@@ -19,6 +19,6 @@
19 19 .hash_password(password.as_bytes(), &salt)
20 20 .expect("Failed to hash password");
21 21
22 - println!("Password: {}", password);
23 - println!("Hash: {}", hash);
22 + println!("Password: {password}");
23 + println!("Hash: {hash}");
24 24 }
@@ -12,7 +12,7 @@
12 12 -- Backfill each artifact from its parent release's signature. For a
13 13 -- single-platform release this is exactly correct; for the buggy multi-platform
14 14 -- case it preserves the current (wrong-for-the-second-platform) behavior rather
15 - -- than blanking it — re-publishing the affected artifact writes the right one.
15 + -- than blanking it. Re-publishing the affected artifact writes the right one.
16 16 -- ota_releases.signature is left in place (forward-only migrations, live rows)
17 17 -- but is no longer read or written; it is dead after this migration.
18 18
@@ -2,7 +2,7 @@
2 2 --
3 3 -- Schema only (phase 2 / p2-tables). The group-scoped push/pull and membership
4 4 -- gating that USE the `group_id` column below land in p2-changelog. The server
5 - -- never sees the Group Content Key (GCK) or any plaintext — it stores opaque
5 + -- never sees the Group Content Key (GCK) or any plaintext; it stores opaque
6 6 -- sealed grants and ciphertext only. Design: wiki synckit-groups-design.
7 7
8 8 -- A group: a shared changelog owned by one admin, whose members each hold a
@@ -3,7 +3,7 @@
3 3 -- Supersedes the sync_log.group_id column added in 171. Keeping group entries in
4 4 -- sync_log meant every personal-scope query (pull, key rotation, status, cleanup)
5 5 -- had to remember `AND group_id IS NULL` or silently leak GCK-encrypted group rows
6 - -- into a user's personal sync -- and let personal key rotation re-encrypt them with
6 + -- into a user's personal sync, and let personal key rotation re-encrypt them with
7 7 -- the wrong key. A dedicated table makes that isolation structural: sync_log stays
8 8 -- purely personal and unchanged, and the group changelog evolves on its own.
9 9 -- Design: wiki synckit-groups-design.
@@ -3,14 +3,14 @@
3 3 //! When enabled, the entire site is reachable only by logged-in users who hold
4 4 //! a creator account or an active Fan+ subscription; everyone else is bounced
5 5 //! to `/login` with a notice. This backs the testnot.work staging mirror, whose
6 - //! data is a daily restore of production — gating it to Fan+/creator accounts
6 + //! data is a daily restore of production, gating it to Fan+/creator accounts
7 7 //! keeps that mirror off the open internet (the "available to anyone with a
8 8 //! Fan+ or creator account" rule), matching the testnot Fan+ perk.
9 9 //!
10 10 //! It is a COARSE pre-filter: it reads the cached session flags only (no DB
11 11 //! query, no session-tracking revalidation). The per-route `AuthUser` extractor
12 12 //! still enforces full auth underneath, so the gate never relaxes real
13 - //! authorization — it only narrows who reaches the routes at all. Default-off,
13 + //! authorization, it only narrows who reaches the routes at all. Default-off,
14 14 //! so production (`AccessGate::Open`) is completely unaffected.
15 15
16 16 use axum::{
@@ -40,7 +40,7 @@
40 40 .is_some_and(|rest| rest.starts_with('/'))
41 41 }
42 42
43 - // Authentication surface — without these the gate would lock out its own
43 + // Authentication surface, without these the gate would lock out its own
44 44 // login page and the assets/endpoints the login flow needs.
45 45 hit(path, "/login")
46 46 || hit(path, "/logout")
M server/src/auth.rs +33 -41
@@ -13,8 +13,8 @@
13 13 //! when enabled.
14 14 //!
15 15 //! Extractors: [`AuthUser`] (required login), [`MaybeUserUnverified`] (optional,
16 - //! no revocation check — public read-only pages only), [`MaybeUserVerified`]
17 - //! (optional with revocation check — anywhere identity actually gates behavior),
16 + //! no revocation check, public read-only pages only), [`MaybeUserVerified`]
17 + //! (optional with revocation check, anywhere identity actually gates behavior),
18 18 //! [`AdminUser`] (admin-only, hides routes with 404).
19 19
20 20 use argon2::{
@@ -165,29 +165,25 @@
165 165 // Every live session carries a tracking id, set at login by
166 166 // `track_session`. A session with USER_SESSION_KEY but no
167 167 // SESSION_TRACKING_KEY is a legacy pre-tracking session that cannot be
168 - // revoked — "log out everywhere", suspend, and password-change all act
168 + // revoked, "log out everywhere", suspend, and password-change all act
169 169 // on `user_sessions` rows it doesn't have. Refuse it (force re-login)
170 170 // rather than trust an unrevocable session (Run 20 Security). Matches
171 171 // the short-circuit `MaybeUserUnverified` already applies.
172 172 let mut user = user;
173 - let tracking_id = match session.get::<UserSessionId>(SESSION_TRACKING_KEY).await {
174 - Ok(Some(id)) => id,
175 - _ => {
176 - let _ = session.flush().await;
177 - return Err(AppError::Unauthorized);
178 - }
173 + let Ok(Some(tracking_id)) = session.get::<UserSessionId>(SESSION_TRACKING_KEY).await else {
174 + let _ = session.flush().await;
175 + return Err(AppError::Unauthorized);
179 176 };
180 177
181 178 // Validate the tracking row. Uses an in-memory cache to avoid hitting
182 - // the DB on every request — if this session was validated within
179 + // the DB on every request, if this session was validated within
183 180 // SESSION_TOUCH_CACHE_SECS, skip the query.
184 181 let cache_ttl = std::time::Duration::from_secs(constants::SESSION_TOUCH_CACHE_SECS);
185 182 let cached = state
186 183 .caches
187 184 .session_cache
188 185 .get(&tracking_id)
189 - .map(|entry| entry.elapsed() < cache_ttl)
190 - .unwrap_or(false);
186 + .is_some_and(|entry| entry.elapsed() < cache_ttl);
191 187
192 188 if !cached {
193 189 let result = match db::sessions::touch_session(&state.db, tracking_id).await {
@@ -240,9 +236,9 @@
240 236 }
241 237 }
242 238
243 - /// Extractor for optional authenticated users — returns None if not logged in.
239 + /// Extractor for optional authenticated users, returns None if not logged in.
244 240 ///
245 - /// **DANGER — this extractor does NOT validate the session against the database.**
241 + /// **DANGER, this extractor does NOT validate the session against the database.**
246 242 /// A revoked session (user clicked "log out everywhere", account suspended,
247 243 /// session row deleted) will still resolve to `Some(SessionUser)` here until
248 244 /// the cookie naturally expires. The name carries the warning: any handler
@@ -279,7 +275,7 @@
279 275
280 276 // Short-circuit legacy sessions (USER_SESSION_KEY present without a
281 277 // SESSION_TRACKING_KEY) to anonymous. Without this, a pre-tracking
282 - // session quietly survives `/logout-everywhere` — that sweep deletes
278 + // session quietly survives `/logout-everywhere`, that sweep deletes
283 279 // user_sessions rows, but a legacy session has no row to delete and
284 280 // would keep rendering as logged-in on every Unverified extractor
285 281 // until the cookie naturally expires.
@@ -305,7 +301,7 @@
305 301 ///
306 302 /// Costs one cached `touch_session` query per request (TTL = `SESSION_TOUCH_CACHE_SECS`).
307 303 /// Prefer this over `MaybeUserUnverified` anywhere the identity actually gates
308 - /// behavior — paid content access, OAuth flows, download grants, comments,
304 + /// behavior, paid content access, OAuth flows, download grants, comments,
309 305 /// or anything that writes to the DB on behalf of the user.
310 306 pub struct MaybeUserVerified(pub Option<SessionUser>);
311 307
@@ -333,12 +329,9 @@
333 329 // pre-tracking session that can't be revoked; treat it as anonymous
334 330 // rather than trust it (Run 20 Security), matching `AuthUser` and
335 331 // `MaybeUserUnverified`.
336 - let tracking_id = match session.get::<UserSessionId>(SESSION_TRACKING_KEY).await {
337 - Ok(Some(id)) => id,
338 - _ => {
339 - let _ = session.flush().await;
340 - return Ok(MaybeUserVerified(None));
341 - }
332 + let Ok(Some(tracking_id)) = session.get::<UserSessionId>(SESSION_TRACKING_KEY).await else {
333 + let _ = session.flush().await;
334 + return Ok(MaybeUserVerified(None));
342 335 };
343 336
344 337 let cache_ttl = std::time::Duration::from_secs(constants::SESSION_TOUCH_CACHE_SECS);
@@ -346,8 +339,7 @@
346 339 .caches
347 340 .session_cache
348 341 .get(&tracking_id)
349 - .map(|entry| entry.elapsed() < cache_ttl)
350 - .unwrap_or(false);
342 + .is_some_and(|entry| entry.elapsed() < cache_ttl);
351 343
352 344 if !cached {
353 345 let result = match db::sessions::touch_session(&state.db, tracking_id).await {
@@ -413,12 +405,12 @@
413 405 }
414 406
415 407 /// Mint an `AdminId` from the configured `ADMIN_USER_ID` for out-of-band admin
416 - /// contexts that have no HTTP session — specifically the `mnw-admin` CLI, which
408 + /// contexts that have no HTTP session, specifically the `mnw-admin` CLI, which
417 409 /// loads the same server env. Returns `None` when no admin is configured.
418 410 ///
419 411 /// This is the only constructor besides [`AdminUser::admin_id`], and it is
420 412 /// gated on the exact same config value that [`require_admin`] checks, so it
421 - /// cannot attribute a moderation action to a non-admin — preserving the
413 + /// cannot attribute a moderation action to a non-admin, preserving the
422 414 /// forgery-proof invariant while letting a headless admin tool stamp the audit
423 415 /// trail with the real actor instead of skipping it.
424 416 pub fn from_config(config: &crate::config::Config) -> Option<Self> {
@@ -434,7 +426,7 @@
434 426
435 427 impl AdminUser {
436 428 /// Mint the [`AdminId`] witness for this verified admin. The only way to
437 - /// obtain an `AdminId` — its private field can't be constructed elsewhere.
429 + /// obtain an `AdminId`, its private field can't be constructed elsewhere.
438 430 pub fn admin_id(&self) -> AdminId {
439 431 AdminId(self.0.id)
440 432 }
@@ -543,7 +535,7 @@
543 535 /// during `ssh-key-lookup` (after authenticating the user by SSH key) and the
544 536 /// CLI forwards. Because the assertion is keyed by the server-only
545 537 /// `signing_secret`, a leaked `ServiceAuth` token cannot forge one for another
546 - /// user — so internal handlers derive identity from this, never from a
538 + /// user, so internal handlers derive identity from this, never from a
547 539 /// caller-supplied `user_id` field. Handlers use `actor.user_id()` for scoping
548 540 /// and `actor.ensure_owns(resource.user_id)?` for ownership checks.
549 541 pub struct InternalActor(pub UserId);
@@ -590,7 +582,7 @@
590 582 /// Production: 46 MiB, 2 iterations (~600ms). With `fast-tests` feature: 8 MiB, 1 iteration (~10ms).
591 583 /// Verification auto-detects params from the hash string, so no feature flag needed there.
592 584 /// Synchronous Argon2id hash. CPU-bound (hundreds of ms); do NOT call from an
593 - /// async handler — use [`hash_password_async`], which runs this on a blocking
585 + /// async handler, use [`hash_password_async`], which runs this on a blocking
594 586 /// thread so a burst of signups can't starve the Tokio worker pool. The sync
595 587 /// form remains `pub` only for one-time `DUMMY_HASH` initializers and
596 588 /// test/integration fixtures that seed password hashes off the request path.
@@ -619,18 +611,18 @@
619 611 /// the parsed hash, not the instance) but explicit derivation pins our
620 612 /// boundary: this function only verifies Argon2 family hashes, anything
621 613 /// else fails out at `Algorithm::try_from`. Forward-compatible with a
622 - /// future algorithm migration — when one lands, add a dispatch table
614 + /// future algorithm migration, when one lands, add a dispatch table
623 615 /// instead of swapping the default instance under the verifier's feet.
624 616 ///
625 - /// CPU-bound (hundreds of ms); do NOT call from an async handler — use
617 + /// CPU-bound (hundreds of ms); do NOT call from an async handler, use
626 618 /// [`verify_password_async`], which runs this on a blocking thread so concurrent
627 619 /// logins can't starve the Tokio worker pool. Kept `pub(crate)` for the async
628 620 /// wrapper, the timing-equalizer dummy verifies, and tests.
629 621 pub(crate) fn verify_password(password: &str, hash: &str) -> Result<bool, AppError> {
630 622 // A stored hash that won't parse (corruption, or a non-Argon2 algorithm we
631 623 // don't verify) is a server-side data problem, not a 500 for the user: treat
632 - // it as a non-match so login simply fails, and log it for ops. Returning an
633 - // Internal error here would also be a (third-order) account oracle — it
624 + // it as a non-match so login fails, and log it for ops. Returning an
625 + // Internal error here would also be a (third-order) account oracle, it
634 626 // distinguishes "valid account, bad stored hash" from "valid account, wrong
635 627 // password" by status code (SEC minor, Run #23).
636 628 let reject = |what: &str, e: &dyn std::fmt::Display| {
@@ -691,13 +683,13 @@
691 683 }
692 684
693 685 /// Uniform password + account-status gate for relying-party logins (OAuth
694 - /// authorize, SyncKit auth) — the flows that reject 2FA accounts outright.
686 + /// authorize, SyncKit auth), the flows that reject 2FA accounts outright.
695 687 ///
696 688 /// Runs Argon2 first, then folds *every* refusal reason (wrong password,
697 689 /// suspended, deactivated, locked, 2FA-enabled) into a single accounted
698 690 /// decision: a denial always increments the failed-login counter, a success
699 691 /// always resets it. Collapsing the blocked-account cases into the wrong-password
700 - /// path is what stops the counter from becoming a confirmed-password oracle — a
692 + /// path is what stops the counter from becoming a confirmed-password oracle, a
701 693 /// correct guess against a 2FA/suspended account must be indistinguishable from a
702 694 /// wrong one (ultra-fuzz Run 3 / Run 11 Sec M1). Both relying parties call this
703 695 /// instead of open-coding the ordering, so the invariant lives in one place.
@@ -811,7 +803,7 @@
811 803
812 804 /// Send a new-device login notification if the user has other active sessions.
813 805 ///
814 - /// Fire-and-forget — spawns a background task. Only sends if the user has opted in
806 + /// Fire-and-forget, spawns a background task. Only sends if the user has opted in
815 807 /// and has more than one active session (meaning this is a new device).
816 808 #[allow(clippy::too_many_arguments)]
817 809 pub async fn maybe_send_login_notification(
@@ -880,7 +872,7 @@
880 872 /// Returns Some(count) if breached, None if clean or API unavailable.
881 873 ///
882 874 /// This check is advisory (it never blocks a password change), so a lookup
883 - /// failure fails open — but it must not fail *silently*. A network blip or
875 + /// failure fails open, but it must not fail *silently*. A network blip or
884 876 /// HIBP outage that disables breach checking is logged at WARN so the gap is
885 877 /// visible in observability rather than disappearing into a bare `?`.
886 878 pub async fn check_password_breach(password: &str) -> Option<u64> {
@@ -889,7 +881,7 @@
889 881 let hash = hex::encode(Sha1::digest(password.as_bytes())).to_uppercase();
890 882 let (prefix, suffix) = hash.split_at(5);
891 883
892 - let url = format!("https://api.pwnedpasswords.com/range/{}", prefix);
884 + let url = format!("https://api.pwnedpasswords.com/range/{prefix}");
893 885 let response = match crate::helpers::HTTP_CLIENT
894 886 .get(&url)
895 887 .header("User-Agent", "MakeNotWork-Security-Check")
@@ -959,7 +951,7 @@
959 951 #[test]
960 952 fn verify_password_unparseable_hash_is_non_match_not_error() {
961 953 // A corrupt / non-Argon2 stored hash must fail login cleanly (Ok(false)),
962 - // not 500 — avoids an availability bug and an account oracle (SEC, Run #23).
954 + // not 500, avoids an availability bug and an account oracle (SEC, Run #23).
963 955 for bad in [
964 956 "",
965 957 "not-a-phc-string",
@@ -1048,7 +1040,7 @@
1048 1040 }
1049 1041
1050 1042 #[tokio::test]
1051 - #[ignore] // Requires network access — run manually
1043 + #[ignore = "requires network access, run manually"]
1052 1044 async fn check_password_breach_known_breached() {
1053 1045 let result = check_password_breach("password").await;
1054 1046 assert!(result.is_some());
@@ -1056,7 +1048,7 @@
1056 1048 }
1057 1049
1058 1050 #[tokio::test]
1059 - #[ignore] // Requires network access — run manually
1051 + #[ignore = "requires network access, run manually"]
1060 1052 async fn check_password_breach_unknown() {
1061 1053 // A random 64-char string should not appear in any breach database
1062 1054 let random_pw = "xK9m2Qp7vL4nR8wJ3sY6dF1gH5bT0cU9eA2iO7lN4mP8qW3rX6zV1yB5jD0fG";
@@ -1,7 +1,7 @@
1 1 //! Bounded background-task queue for fire-and-forget work.
2 2 //!
3 3 //! Replaces per-request `tokio::spawn(...)` for low-priority work that
4 - //! competes with request handlers for the DB pool — email sends, mailing-list
4 + //! competes with request handlers for the DB pool, email sends, mailing-list
5 5 //! subscriptions, etc. Run #4 fixed the same shape for page views via
6 6 //! `db::page_views::PageViewTx`; Run #8 surfaced it again on the webhook
7 7 //! hot path (`routes/stripe/webhook/checkout_helpers.rs`), so this module
@@ -62,7 +62,7 @@
62 62 /// pulls tasks off the channel and runs each under a semaphore permit so
63 63 /// concurrent execution is bounded.
64 64 ///
65 - /// On shutdown (the `shutdown` watch fires — a value change or all senders
65 + /// On shutdown (the `shutdown` watch fires, a value change or all senders
66 66 /// dropped) the drainer stops taking new work, runs every already-queued task,
67 67 /// then waits for in-flight tasks to finish before exiting. Without this the
68 68 /// `bg` pool was the one undrained primitive: in-flight emails / cache purges /
@@ -1,4 +1,4 @@
1 - //! Build runner — dispatches and executes OTA builds via SSH to remote hosts.
1 + //! Build runner, dispatches and executes OTA builds via SSH to remote hosts.
2 2 //!
3 3 //! The scheduler calls `dispatch_pending_build()` each tick. If no build is
4 4 //! running and one is pending, it spawns a `tokio::spawn` task that SSHes to
@@ -66,7 +66,7 @@
66 66 let cmd = format!("rm -rf {}", shell_escape(build_dir));
67 67 let _ = tokio::time::timeout(
68 68 Duration::from_secs(SSH_CLEANUP_TIMEOUT_SECS),
69 - run_ssh_command(host, &cmd),
69 + Box::pin(run_ssh_command(host, &cmd)),
70 70 )
71 71 .await;
72 72 }
@@ -154,7 +154,7 @@
154 154
155 155 /// Check for a pending build and spawn it if no build is currently running.
156 156 ///
157 - /// Called from the scheduler loop. Non-blocking — spawns the build task and returns.
157 + /// Called from the scheduler loop. Non-blocking, spawns the build task and returns.
158 158 #[tracing::instrument(skip_all, name = "build_runner::dispatch")]
159 159 pub async fn dispatch_pending_build(ctx: &BuildCtx) {
160 160 // Recover from stale running builds (e.g. server crashed mid-build)
@@ -233,7 +233,7 @@
233 233 // Resolve each target to its build host up front. Synchronous failures (bad
234 234 // target format, no host configured) are tallied here; resolvable targets are
235 235 // grouped by host so independent hosts (e.g. linux vs darwin) build
236 - // concurrently while same-host targets stay serial — a multi-target release no
236 + // concurrently while same-host targets stay serial, a multi-target release no
237 237 // longer serializes end to end at up to 30 min/target (Perf-S2, Run 9).
238 238 let mut groups: Vec<(String, Vec<(String, String)>)> = Vec::new(); // host -> [(os, arch)]
239 239 for target_str in &config.targets {
@@ -247,18 +247,15 @@
247 247 continue;
248 248 };
249 249
250 - let host = match build_host_for_target(&ctx.config, target_os) {
251 - Some(h) => h,
252 - None => {
253 - let msg = format!("no build host for {target_os}, skipping {target_str}\n");
254 - tracing::warn!("{}", msg.trim());
255 - let _ = append_log_bounded(ctx, build.id, &msg).await;
256 - failed_count += 1;
257 - if first_error.is_none() {
258 - first_error = Some(format!("no build host for {target_os}"));
259 - }
260 - continue;
250 + let Some(host) = build_host_for_target(&ctx.config, target_os) else {
251 + let msg = format!("no build host for {target_os}, skipping {target_str}\n");
252 + tracing::warn!("{}", msg.trim());
253 + let _ = append_log_bounded(ctx, build.id, &msg).await;
254 + failed_count += 1;
255 + if first_error.is_none() {
256 + first_error = Some(format!("no build host for {target_os}"));
261 257 }
258 + continue;
262 259 };
263 260
264 261 let entry = (target_os.to_string(), arch.to_string());
@@ -281,7 +278,11 @@
281 278 let mut oks: Vec<TargetArtifact> = Vec::new();
282 279 let mut errs: Vec<TargetError> = Vec::new();
283 280 for (target_os, arch) in &targets {
284 - match execute_target(&ctx, &build, &config, &host, target_os, arch).await {
281 + match Box::pin(execute_target(
282 + &ctx, &build, &config, &host, target_os, arch,
283 + ))
284 + .await
285 + {
285 286 Ok((s3_key, signature)) => {
286 287 oks.push((target_os.clone(), arch.clone(), s3_key, signature));
287 288 }
@@ -393,7 +394,7 @@
393 394 .map(|app| app.creator_id);
394 395
395 396 // Record artifacts and enqueue each for malware scanning. The artifact stays
396 - // `pending` (not served) until the scan clears it — same gate as the item
397 + // `pending` (not served) until the scan clears it, same gate as the item
397 398 // channel.
398 399 for (target_os, arch, s3_key, signature) in &artifact_keys {
399 400 // Get file size from S3 via HEAD request (best-effort, use 0 if unavailable)
@@ -520,7 +521,7 @@
520 521 // Execute via SSH with timeout
521 522 let ssh_result = tokio::time::timeout(
522 523 Duration::from_secs(BUILD_TIMEOUT_SECS),
523 - run_ssh_command(host, &remote_script),
524 + Box::pin(run_ssh_command(host, &remote_script)),
524 525 )
525 526 .await;
526 527
@@ -528,11 +529,11 @@
528 529 Ok(Ok(output)) => output,
529 530 Ok(Err(e)) => {
530 531 // Cleanup remote build dir (best-effort)
531 - cleanup_remote_dir(host, &build_dir).await;
532 + Box::pin(cleanup_remote_dir(host, &build_dir)).await;
532 533 return Err(format!("SSH command failed: {e}"));
533 534 }
534 535 Err(_) => {
535 - cleanup_remote_dir(host, &build_dir).await;
536 + Box::pin(cleanup_remote_dir(host, &build_dir)).await;
536 537 return Err("build timed out".to_string());
537 538 }
538 539 };
@@ -562,7 +563,7 @@
562 563 run_scp_download(host, &format!("{scp_remote_path}.sig"), &local_sig_tmp).await;
563 564
564 565 // Cleanup remote build dir
565 - cleanup_remote_dir(host, &build_dir).await;
566 + Box::pin(cleanup_remote_dir(host, &build_dir)).await;
566 567
567 568 if let Err(e) = scp_result {
568 569 // The main artifact failed, but the .sig sidecar may already be on disk
@@ -585,7 +586,7 @@
585 586 String::new()
586 587 };
587 588
588 - // Upload to S3 via multipart streaming from disk — the previous
589 + // Upload to S3 via multipart streaming from disk, the previous
589 590 // implementation `tokio::fs::read` → `Vec<u8>` → `upload_object` pinned
590 591 // the entire artifact (up to ~100 MB per build) in RAM during upload.
591 592 // `upload_multipart` reads the file in chunks and lets the S3 SDK do
@@ -605,22 +606,22 @@
605 606 .await
606 607 .map_err(|e| format!("S3 multipart upload failed: {e}"));
607 608
608 - // Always remove the local temp file, even if the upload failed — leaving
609 + // Always remove the local temp file, even if the upload failed, leaving
609 610 // it on disk fills the build runner's tmp directory across retries.
610 611 let _ = tokio::fs::remove_file(&local_tmp).await;
611 612
612 613 upload_result?;
613 614
614 - if !signature.is_empty() {
615 + if signature.is_empty() {
616 + let _ =
617 + append_log_bounded(ctx, build.id, &format!("[{target}] uploaded to {s3_key}\n")).await;
618 + } else {
615 619 let _ = append_log_bounded(
616 620 ctx,
617 621 build.id,
618 622 &format!("[{target}] uploaded to {s3_key} (signed)\n"),
619 623 )
620 624 .await;
621 - } else {
622 - let _ =
623 - append_log_bounded(ctx, build.id, &format!("[{target}] uploaded to {s3_key}\n")).await;
624 625 }
625 626
626 627 Ok((s3_key.into_string(), signature))
@@ -672,8 +673,8 @@
672 673 .stdout(std::process::Stdio::piped())
673 674 .stderr(std::process::Stdio::piped())
674 675 // Kill the ssh process if this future is dropped (e.g. the 30-min build
675 - // timeout fires): otherwise the dropped future leaves ssh — and the
676 - // remote build it drives — running orphaned (ultra-fuzz Run 11 Perf).
676 + // timeout fires): otherwise the dropped future leaves ssh, and the
677 + // remote build it drives, running orphaned (ultra-fuzz Run 11 Perf).
677 678 .kill_on_drop(true)
678 679 .spawn()
679 680 .map_err(|e| format!("failed to spawn ssh: {e}"))?;
@@ -828,9 +829,9 @@
828 829 ///
829 830 /// The operator-configured `build_command` is a single string (e.g.
830 831 /// `RUSTFLAGS=--cfg cargo build --release`). Rather than interpolate it raw into
831 - /// the remote `sh -c` script — where its safety rested entirely on a
832 + /// the remote `sh -c` script, where its safety rested entirely on a
832 833 /// metacharacter denylist, one added allowed character away from reopening
833 - /// injection — it is tokenised into leading `NAME=VALUE` environment
834 + /// injection, it is tokenised into leading `NAME=VALUE` environment
834 835 /// assignments followed by a program and its arguments. `render` emits every
835 836 /// element individually shell-escaped, applying assignments via `env`, so no
836 837 /// operator byte can break out of its shell word. Shell injection is
@@ -934,8 +935,8 @@
934 935 }
935 936
936 937 /// Validate a build command for shell safety at config-write time. Validation is
937 - /// exactly "parses into a [`RemoteCommand`]" — the same parser the executor uses
938 - /// — so a stored command that validates here can never fail to render safely.
938 + /// exactly "parses into a [`RemoteCommand`]", the same parser the executor uses
939 + ///, so a stored command that validates here can never fail to render safely.
939 940 pub fn validate_build_command(cmd: &str) -> std::result::Result<(), String> {
940 941 RemoteCommand::parse(cmd).map(|_| ())
941 942 }
M server/src/csrf.rs +46 -45
M server/src/lib.rs +24 -26
M server/src/main.rs +24 -24
M server/src/rss.rs +45 -41