max / makenotwork
- Co-Authored-By
- Claude Opus 4.8 (1M context) <noreply@anthropic.com>
106 files changed,
+716 insertions,
-862 deletions
| @@ -107,3 +107,38 @@ | |||
| 107 | 107 | # Enable mt-db's setup-only mutations for the integration suite. Resolver 2 keeps | |
| 108 | 108 | # this feature out of the normal/production build. | |
| 109 | 109 | mt-db = { workspace = true, features = ["test-support"] } | |
| 110 | + | ||
| 111 | + | [workspace.lints.rust] | |
| 112 | + | unused = "warn" | |
| 113 | + | unreachable_pub = "warn" | |
| 114 | + | ||
| 115 | + | [workspace.lints.clippy] | |
| 116 | + | pedantic = { level = "warn", priority = -1 } | |
| 117 | + | # Allow-list tuned from a measured breakdown across server/multithreaded/pter | |
| 118 | + | # (2026-07-22). These are the high-churn / low-signal pedantic lints; everything | |
| 119 | + | # else in `pedantic` stays a warning. Keep this block identical across repos. | |
| 120 | + | module_name_repetitions = "allow" | |
| 121 | + | # Doc lints. No docs-completeness push is underway. | |
| 122 | + | missing_errors_doc = "allow" | |
| 123 | + | missing_panics_doc = "allow" | |
| 124 | + | doc_markdown = "allow" | |
| 125 | + | # Numeric casts. Endemic and mostly intentional in size and byte math. | |
| 126 | + | cast_possible_truncation = "allow" | |
| 127 | + | cast_sign_loss = "allow" | |
| 128 | + | cast_precision_loss = "allow" | |
| 129 | + | cast_possible_wrap = "allow" | |
| 130 | + | cast_lossless = "allow" | |
| 131 | + | # Subjective structure and style nags. High churn, low signal. | |
| 132 | + | must_use_candidate = "allow" | |
| 133 | + | too_many_lines = "allow" | |
| 134 | + | struct_excessive_bools = "allow" | |
| 135 | + | similar_names = "allow" | |
| 136 | + | items_after_statements = "allow" | |
| 137 | + | single_match_else = "allow" | |
| 138 | + | # Frequent false-positives in TUI and router-heavy code. | |
| 139 | + | match_same_arms = "allow" | |
| 140 | + | unnecessary_wraps = "allow" | |
| 141 | + | type_complexity = "allow" | |
| 142 | + | ||
| 143 | + | [lints] | |
| 144 | + | workspace = true |
| @@ -18,7 +18,7 @@ | |||
| 18 | 18 | /// One global version rather than a per-file hash: an upgrade to any watched | |
| 19 | 19 | /// file re-fetches all of them, which costs a handful of requests once and | |
| 20 | 20 | /// keeps the templates free of per-asset bookkeeping. What it buys is that a | |
| 21 | - | /// redeployed file can never be served stale from browser cache — for a chat | |
| 21 | + | /// redeployed file can never be served stale from browser cache, for a chat | |
| 22 | 22 | /// island that would mean old protocol logic talking to a new server, which | |
| 23 | 23 | /// presents as "chat is broken for some people" rather than as a cache bug. | |
| 24 | 24 | /// | |
| @@ -66,9 +66,7 @@ | |||
| 66 | 66 | ||
| 67 | 67 | /// Write `contents` to `path` only if it differs, to avoid needless rebuilds. | |
| 68 | 68 | fn write_if_changed(path: &Path, contents: &str) { | |
| 69 | - | let needs_write = fs::read_to_string(path) | |
| 70 | - | .map(|existing| existing != contents) | |
| 71 | - | .unwrap_or(true); | |
| 69 | + | let needs_write = fs::read_to_string(path).map_or(true, |existing| existing != contents); | |
| 72 | 70 | if needs_write { | |
| 73 | 71 | fs::write(path, contents) | |
| 74 | 72 | .unwrap_or_else(|e| panic!("failed to write {}: {e}", path.display())); | |
| @@ -76,7 +74,7 @@ | |||
| 76 | 74 | } | |
| 77 | 75 | ||
| 78 | 76 | /// Recursively hash every `.js` file under `dir` into `hasher`, in a | |
| 79 | - | /// deterministic order. A missing directory is a no-op — the first build on a | |
| 77 | + | /// deterministic order. A missing directory is a no-op, the first build on a | |
| 80 | 78 | /// fresh checkout runs before the frontend has been compiled. | |
| 81 | 79 | fn hash_dir_js(dir: &Path, hasher: &mut DefaultHasher) { | |
| 82 | 80 | let Ok(entries) = fs::read_dir(dir) else { |
| @@ -4,10 +4,10 @@ | |||
| 4 | 4 | -- moderation by owners/mods/superadmin. | |
| 5 | 5 | -- | |
| 6 | 6 | -- States: | |
| 7 | - | -- active — normal operation (default) | |
| 8 | - | -- restricted — block new thread creation for non-mods (existing threads still accept replies) | |
| 9 | - | -- frozen — read-only for everyone except mods doing mod actions | |
| 10 | - | -- archived — frozen + hidden from default community listings; surfaces under | |
| 7 | + | -- active: normal operation (default) | |
| 8 | + | -- restricted: block new thread creation for non-mods (existing threads still accept replies) | |
| 9 | + | -- frozen: read-only for everyone except mods doing mod actions | |
| 10 | + | -- archived: frozen + hidden from default community listings; surfaces under | |
| 11 | 11 | -- an explicit archived filter; reactivation = set back to active | |
| 12 | 12 | -- | |
| 13 | 13 | -- Authorization for transitions: community Owner/Moderator OR platform admin. | |
| @@ -16,7 +16,7 @@ | |||
| 16 | 16 | ADD COLUMN state TEXT NOT NULL DEFAULT 'active' | |
| 17 | 17 | CHECK (state IN ('active', 'restricted', 'frozen', 'archived')); | |
| 18 | 18 | ||
| 19 | - | -- Partial index for the archived filter view — most communities are active, so | |
| 19 | + | -- Partial index for the archived filter view; most communities are active, so | |
| 20 | 20 | -- a partial index keeps the listing query cheap. | |
| 21 | 21 | CREATE INDEX IF NOT EXISTS idx_communities_archived | |
| 22 | 22 | ON communities (name) |
| @@ -2,7 +2,7 @@ | |||
| 2 | 2 | -- `remove_image` sets removed_at but the object delete is best-effort; this | |
| 3 | 3 | -- column lets a background sweep find removed images whose object still needs | |
| 4 | 4 | -- purging (the pre-existing backlog and any inline-delete failures) and retry | |
| 5 | - | -- them convergently — once purged, an image is never revisited. | |
| 5 | + | -- them convergently: once purged, an image is never revisited. | |
| 6 | 6 | ALTER TABLE images ADD COLUMN IF NOT EXISTS s3_purged_at TIMESTAMPTZ; | |
| 7 | 7 | ||
| 8 | 8 | -- The sweep scans for removed-but-not-purged images; keep that lookup cheap. |
| @@ -2,14 +2,14 @@ | |||
| 2 | 2 | -- bounded again. Migration 027 dropped the previous `reply_count` column | |
| 3 | 3 | -- because it was maintained by application code that incremented on reply but | |
| 4 | 4 | -- missed the decrement paths (mod-remove, soft-delete), so it drifted upward | |
| 5 | - | -- permanently. The live replacement — a correlated `COUNT(*)` subquery per row | |
| 6 | - | -- — is correct but turns `?sort=replies` into a full-category aggregate on a | |
| 5 | + | -- permanently. The live replacement, a correlated `COUNT(*)` subquery per row, | |
| 6 | + | -- is correct but turns `?sort=replies` into a full-category aggregate on a | |
| 7 | 7 | -- cacheless public GET (the cheapest request for a scraper, one of the most | |
| 8 | 8 | -- expensive for Postgres). | |
| 9 | 9 | -- | |
| 10 | 10 | -- This counter is maintained by a TRIGGER, not application code. A trigger | |
| 11 | 11 | -- fires on every INSERT/UPDATE/DELETE regardless of which query issued it, so | |
| 12 | - | -- it cannot miss a mutation path the way the 022 application code did — that | |
| 12 | + | -- it cannot miss a mutation path the way the 022 application code did; that | |
| 13 | 13 | -- missed-path bug is exactly the root cause of the prior drift. `post_count` is | |
| 14 | 14 | -- the number of non-removed posts in the thread (including the OP); the display | |
| 15 | 15 | -- reply count is `GREATEST(post_count - 1, 0)`, computed at read time. |
| @@ -2,7 +2,7 @@ | |||
| 2 | 2 | -- soft-delete column. Posts carry two: `removed_at` (mod-remove, migration 011) | |
| 3 | 3 | -- and `deleted_at` (author soft-delete, migration 007). The 029 trigger only | |
| 4 | 4 | -- watched `removed_at`, so the day a feature starts setting `posts.deleted_at` | |
| 5 | - | -- (a "delete my own post" path), post_count would silently overcount — the exact | |
| 5 | + | -- (a "delete my own post" path), post_count would silently overcount, the exact | |
| 6 | 6 | -- drift class 027/029 were written to kill, just keyed on the dormant column. | |
| 7 | 7 | -- | |
| 8 | 8 | -- A post counts toward post_count iff it is active: BOTH columns null. The |
| @@ -1,6 +1,6 @@ | |||
| 1 | 1 | -- Allow a NULL actor_id in the mod log to denote a system action (e.g. a | |
| 2 | 2 | -- flag-threshold auto-hide), mirroring the posts.removed_by IS NULL convention. | |
| 3 | 3 | -- Previously every entry required a user actor, which forced auto-hide to record | |
| 4 | - | -- the flagger who tripped the threshold as the "moderator" — a false attribution | |
| 4 | + | -- the flagger who tripped the threshold as the "moderator", a false attribution | |
| 5 | 5 | -- on an auditable, exportable ledger. Additive: existing rows are unaffected. | |
| 6 | 6 | ALTER TABLE mod_log ALTER COLUMN actor_id DROP NOT NULL; |
| @@ -88,7 +88,7 @@ | |||
| 88 | 88 | const SESSION_USERNAME: &str = "username"; | |
| 89 | 89 | const SESSION_DISPLAY_NAME: &str = "display_name"; | |
| 90 | 90 | const SESSION_PERKS: &str = "perks"; | |
| 91 | - | /// The MNW **refresh** token — scoped (`perks:read`/`profile:read`), rotating, | |
| 91 | + | /// The MNW **refresh** token, scoped (`perks:read`/`profile:read`), rotating, | |
| 92 | 92 | /// and unable to act as the user on the sync API. This is the only MNW | |
| 93 | 93 | /// credential stored at rest (closing finding S13); the short-lived access | |
| 94 | 94 | /// token is used transiently for one userinfo fetch and never persisted. | |
| @@ -124,7 +124,7 @@ | |||
| 124 | 124 | None | |
| 125 | 125 | } | |
| 126 | 126 | }; | |
| 127 | - | // Perks default to empty — sessions predating the perks change still load. | |
| 127 | + | // Perks default to empty, sessions predating the perks change still load. | |
| 128 | 128 | let perks: UserPerks = session | |
| 129 | 129 | .get(SESSION_PERKS) | |
| 130 | 130 | .await | |
| @@ -177,7 +177,7 @@ | |||
| 177 | 177 | /// Axum extractor that requires an authenticated session. | |
| 178 | 178 | /// | |
| 179 | 179 | /// Yields the [`SessionUser`] directly, or rejects with a redirect to | |
| 180 | - | /// `/auth/login` — the exact behaviour ~30 write/settings handlers previously | |
| 180 | + | /// `/auth/login`, the exact behaviour ~30 write/settings handlers previously | |
| 181 | 181 | /// open-coded as `session_user.ok_or_else(|| Redirect::to("/auth/login")…)?`. | |
| 182 | 182 | /// Use this instead of `MaybeUser` whenever the handler needs a logged-in user. | |
| 183 | 183 | pub struct RequireUser(pub SessionUser); | |
| @@ -249,7 +249,7 @@ | |||
| 249 | 249 | /// that declines it (or hasn't shipped refresh tokens) still parses. | |
| 250 | 250 | #[serde(default)] | |
| 251 | 251 | refresh_token: Option<String>, | |
| 252 | - | /// Informational only — login longevity is governed by the MT session, not | |
| 252 | + | /// Informational only, login longevity is governed by the MT session, not | |
| 253 | 253 | /// the access token. | |
| 254 | 254 | #[serde(default)] | |
| 255 | 255 | #[allow(dead_code)] | |
| @@ -272,7 +272,7 @@ | |||
| 272 | 272 | Transport, | |
| 273 | 273 | BadResponse, | |
| 274 | 274 | /// No usable refresh token: none stored, or the stored one was expired / | |
| 275 | - | /// rotated / revoked (`invalid_grant`). The MT session is NOT torn down — | |
| 275 | + | /// rotated / revoked (`invalid_grant`). The MT session is NOT torn down, | |
| 276 | 276 | /// the user stays logged in with last-known perks and can re-link MNW. This | |
| 277 | 277 | /// is distinct from `Unauthorized`, which historically flushed the session. | |
| 278 | 278 | RefreshUnavailable, | |
| @@ -283,13 +283,13 @@ | |||
| 283 | 283 | /// `Unauthorized` means the bearer token is invalid or the user is gone. | |
| 284 | 284 | /// `Transport` covers network and 5xx. `BadResponse` covers other 4xx and parse | |
| 285 | 285 | /// errors. The login callback retries on `Transport`; `refresh_session` does | |
| 286 | - | /// not — the client can retry. | |
| 286 | + | /// not, the client can retry. | |
| 287 | 287 | async fn fetch_userinfo( | |
| 288 | 288 | http: &reqwest::Client, | |
| 289 | 289 | base_url: &str, | |
| 290 | 290 | access_token: &str, | |
| 291 | 291 | ) -> Result<UserinfoResponse, UserinfoError> { | |
| 292 | - | let url = format!("{}/oauth/userinfo", base_url); | |
| 292 | + | let url = format!("{base_url}/oauth/userinfo"); | |
| 293 | 293 | let res = http | |
| 294 | 294 | .get(&url) | |
| 295 | 295 | .bearer_auth(access_token) | |
| @@ -357,7 +357,7 @@ | |||
| 357 | 357 | if status.is_server_error() { | |
| 358 | 358 | return Err(UserinfoError::Transport); | |
| 359 | 359 | } | |
| 360 | - | // 4xx — invalid_grant (expired/rotated/revoked) or bad request. | |
| 360 | + | // 4xx, invalid_grant (expired/rotated/revoked) or bad request. | |
| 361 | 361 | let body = res.text().await.unwrap_or_default(); | |
| 362 | 362 | tracing::warn!(%status, %body, "refresh token exchange rejected"); | |
| 363 | 363 | Err(UserinfoError::RefreshUnavailable) | |
| @@ -380,8 +380,8 @@ | |||
| 380 | 380 | } | |
| 381 | 381 | // Mirror the full identity snapshot (username/display_name/avatar_url + perks) | |
| 382 | 382 | // into the users table so *other* users' posts JOIN against the current author | |
| 383 | - | // row, not a login-time freeze. This reuses the exact login upsert — including | |
| 384 | - | // the stale-username vacate — so "refresh" and "login" can never drift into two | |
| 383 | + | // row, not a login-time freeze. This reuses the exact login upsert, including | |
| 384 | + | // the stale-username vacate, so "refresh" and "login" can never drift into two | |
| 385 | 385 | // different mirror shapes (audit_review.md: apply_userinfo stale mirror). | |
| 386 | 386 | // Best-effort: rendering tolerates a momentarily stale row. | |
| 387 | 387 | if let Err(e) = upsert_login_user(&state.db, info).await { | |
| @@ -396,7 +396,7 @@ | |||
| 396 | 396 | /// token, and updates cached perks. **Never tears down the MT session**: login | |
| 397 | 397 | /// longevity is governed by the session itself, so a dead refresh token yields | |
| 398 | 398 | /// `RefreshUnavailable` (and clears the stored token) rather than logging the | |
| 399 | - | /// user out — they keep last-known perks and can re-link their MNW account. | |
| 399 | + | /// user out, they keep last-known perks and can re-link their MNW account. | |
| 400 | 400 | pub async fn refresh_session( | |
| 401 | 401 | state: &AppState, | |
| 402 | 402 | session: &Session, | |
| @@ -410,7 +410,7 @@ | |||
| 410 | 410 | let token = match exchange_refresh_token(state, &refresh_token).await { | |
| 411 | 411 | Ok(t) => t, | |
| 412 | 412 | Err(UserinfoError::RefreshUnavailable) => { | |
| 413 | - | // Dead refresh token — drop it, but keep the user logged in. | |
| 413 | + | // Dead refresh token, drop it, but keep the user logged in. | |
| 414 | 414 | if let Err(e) = session.remove::<String>(SESSION_REFRESH_TOKEN).await { | |
| 415 | 415 | tracing::warn!(error = %e, "failed to remove dead refresh token"); | |
| 416 | 416 | } | |
| @@ -426,7 +426,7 @@ | |||
| 426 | 426 | tracing::error!(error = %e, "failed to persist rotated refresh token"); | |
| 427 | 427 | } | |
| 428 | 428 | ||
| 429 | - | // The short-lived access token is used here and then discarded — never stored. | |
| 429 | + | // The short-lived access token is used here and then discarded, never stored. | |
| 430 | 430 | let info = fetch_userinfo(&state.http, &state.config.mnw_base_url, &token.access_token).await?; | |
| 431 | 431 | apply_userinfo(state, session, &info).await; | |
| 432 | 432 | Ok(info.perks) | |
| @@ -434,7 +434,7 @@ | |||
| 434 | 434 | ||
| 435 | 435 | // ── Handlers ── | |
| 436 | 436 | ||
| 437 | - | /// `GET /auth/login` — redirect to MNW OAuth authorize endpoint. | |
| 437 | + | /// `GET /auth/login`, redirect to MNW OAuth authorize endpoint. | |
| 438 | 438 | #[tracing::instrument(skip_all)] | |
| 439 | 439 | pub async fn login(State(state): State<AppState>, session: Session) -> impl IntoResponse { | |
| 440 | 440 | let verifier = generate_verifier(); | |
| @@ -449,7 +449,7 @@ | |||
| 449 | 449 | } | |
| 450 | 450 | ||
| 451 | 451 | // Request scoped userinfo access plus offline_access so MNW issues a | |
| 452 | - | // rotating refresh token — MT then holds no long-lived, sync-capable token. | |
| 452 | + | // rotating refresh token, MT then holds no long-lived, sync-capable token. | |
| 453 | 453 | let url = format!( | |
| 454 | 454 | "{}/oauth/authorize?response_type=code&client_id={}&redirect_uri={}&state={}&code_challenge={}&code_challenge_method=S256&scope={}", | |
| 455 | 455 | state.config.mnw_base_url, | |
| @@ -463,14 +463,14 @@ | |||
| 463 | 463 | Redirect::to(&url) | |
| 464 | 464 | } | |
| 465 | 465 | ||
| 466 | - | /// `GET /auth/reverify` — silent perk re-check via OIDC `prompt=none`. | |
| 466 | + | /// `GET /auth/reverify`, silent perk re-check via OIDC `prompt=none`. | |
| 467 | 467 | /// | |
| 468 | 468 | /// The zero-credential-at-rest alternative to the back-channel refresh token: | |
| 469 | 469 | /// MT bounces the browser through MNW's `/oauth/authorize?prompt=none` (no | |
| 470 | 470 | /// `offline_access`, so no refresh token is issued) and the callback uses the | |
| 471 | 471 | /// returned short-lived token for one userinfo fetch, storing nothing. If the | |
| 472 | 472 | /// MNW session has lapsed, MNW redirects back with `error=login_required` and | |
| 473 | - | /// the callback simply keeps the user's last-known perks. MT dogfoods both this | |
| 473 | + | /// the callback keeps the user's last-known perks. MT dogfoods both this | |
| 474 | 474 | /// and the refresh-token flow as the reference relying-party integration. | |
| 475 | 475 | #[tracing::instrument(skip_all)] | |
| 476 | 476 | pub async fn reverify(State(state): State<AppState>, session: Session) -> impl IntoResponse { | |
| @@ -501,13 +501,13 @@ | |||
| 501 | 501 | /// Retry backoffs for the OAuth token/userinfo round trips (two retries). | |
| 502 | 502 | const OAUTH_BACKOFFS: [std::time::Duration; 2] = [ | |
| 503 | 503 | std::time::Duration::from_millis(500), | |
| 504 | - | std::time::Duration::from_millis(1000), | |
| 504 | + | std::time::Duration::from_secs(1), | |
| 505 | 505 | ]; | |
| 506 | 506 | ||
| 507 | 507 | /// Exchange the authorization code for a token, retrying on transport/5xx. | |
| 508 | 508 | /// | |
| 509 | 509 | /// Returns the parsed token on success, or the `?error=` slug to redirect with. | |
| 510 | - | /// Parsing happens here so the caller never holds an un-parsed response — there | |
| 510 | + | /// Parsing happens here so the caller never holds an un-parsed response, there | |
| 511 | 511 | /// is no post-loop `unwrap()` to trip if the retry logic ever changes. | |
| 512 | 512 | async fn exchange_code_for_token( | |
| 513 | 513 | http: &reqwest::Client, | |
| @@ -518,7 +518,7 @@ | |||
| 518 | 518 | let token_url = format!("{}/oauth/token", config.mnw_base_url); | |
| 519 | 519 | tracing::info!(%token_url, "exchanging code for token"); | |
| 520 | 520 | // `attempt` is the retry counter (also logged), and the loop runs one past | |
| 521 | - | // the backoff array — an iterator-with-enumerate doesn't fit the N+1 shape. | |
| 521 | + | // the backoff array, an iterator-with-enumerate doesn't fit the N+1 shape. | |
| 522 | 522 | #[allow(clippy::needless_range_loop)] | |
| 523 | 523 | for attempt in 0..=OAUTH_BACKOFFS.len() { | |
| 524 | 524 | let res = http | |
| @@ -574,7 +574,7 @@ | |||
| 574 | 574 | } | |
| 575 | 575 | ||
| 576 | 576 | /// Fetch userinfo, retrying on transport/5xx. Returns userinfo or the `?error=` | |
| 577 | - | /// slug to redirect with. No post-loop `expect()` — the loop returns on success. | |
| 577 | + | /// slug to redirect with. No post-loop `expect()`, the loop returns on success. | |
| 578 | 578 | async fn fetch_userinfo_with_retry( | |
| 579 | 579 | http: &reqwest::Client, | |
| 580 | 580 | base_url: &str, | |
| @@ -587,7 +587,6 @@ | |||
| 587 | 587 | Err(UserinfoError::Transport) if attempt < OAUTH_BACKOFFS.len() => { | |
| 588 | 588 | tracing::warn!(attempt, "userinfo transport error, retrying"); | |
| 589 | 589 | sleep(OAUTH_BACKOFFS[attempt]).await; | |
| 590 | - | continue; | |
| 591 | 590 | } | |
| 592 | 591 | Err(UserinfoError::Transport) => { | |
| 593 | 592 | tracing::error!("userinfo transport failed after retries"); | |
| @@ -616,13 +615,13 @@ | |||
| 616 | 615 | let mut tx = db.begin().await?; | |
| 617 | 616 | mt_db::mutations::vacate_username_for_login(&mut tx, info.user_id, &info.username).await?; | |
| 618 | 617 | sqlx::query( | |
| 619 | - | r#" | |
| 618 | + | r" | |
| 620 | 619 | INSERT INTO users (mnw_account_id, username, display_name, avatar_url, is_fan_plus, is_creator) | |
| 621 | 620 | VALUES ($1, $2, $3, $4, $5, $6) | |
| 622 | 621 | ON CONFLICT (mnw_account_id) DO UPDATE | |
| 623 | 622 | SET username = $2, display_name = $3, avatar_url = $4, | |
| 624 | 623 | is_fan_plus = $5, is_creator = $6, updated_at = now() | |
| 625 | - | "#, | |
| 624 | + | ", | |
| 626 | 625 | ) | |
| 627 | 626 | .bind(info.user_id) | |
| 628 | 627 | .bind(&info.username) | |
| @@ -636,7 +635,7 @@ | |||
| 636 | 635 | Ok(()) | |
| 637 | 636 | } | |
| 638 | 637 | ||
| 639 | - | /// `GET /auth/callback` — exchange code for token, fetch userinfo, create session. | |
| 638 | + | /// `GET /auth/callback`, exchange code for token, fetch userinfo, create session. | |
| 640 | 639 | #[tracing::instrument(skip_all)] | |
| 641 | 640 | pub async fn callback( | |
| 642 | 641 | State(state): State<AppState>, | |
| @@ -646,8 +645,8 @@ | |||
| 646 | 645 | tracing::info!("OAuth callback received"); | |
| 647 | 646 | ||
| 648 | 647 | // Read and immediately consume the one-time OAuth params. Removing them up | |
| 649 | - | // front makes both single-use, so a failed state check — or a replayed | |
| 650 | - | // callback — cannot leave a reusable PKCE verifier behind in the session. | |
| 648 | + | // front makes both single-use, so a failed state check, or a replayed | |
| 649 | + | // callback, cannot leave a reusable PKCE verifier behind in the session. | |
| 651 | 650 | let stored_state: Option<String> = session.get(SESSION_OAUTH_STATE).await.unwrap_or(None); | |
| 652 | 651 | let stored_verifier: Option<String> = session.get(SESSION_PKCE_VERIFIER).await.unwrap_or(None); | |
| 653 | 652 | if let Err(e) = session.remove::<String>(SESSION_OAUTH_STATE).await { | |
| @@ -657,7 +656,7 @@ | |||
| 657 | 656 | tracing::warn!(error = %e, "failed to remove PKCE verifier from session"); | |
| 658 | 657 | } | |
| 659 | 658 | ||
| 660 | - | // Verify state nonce in constant time — it's a CSRF token, so compare it on | |
| 659 | + | // Verify state nonce in constant time, it's a CSRF token, so compare it on | |
| 661 | 660 | // the same timing-safe path as every other secret (no early-exit on length | |
| 662 | 661 | // or first differing byte). | |
| 663 | 662 | let state_ok = stored_state | |
| @@ -692,7 +691,7 @@ | |||
| 692 | 691 | } | |
| 693 | 692 | }; | |
| 694 | 693 | ||
| 695 | - | // Exchange code for token, then fetch userinfo — each retries on transport/5xx | |
| 694 | + | // Exchange code for token, then fetch userinfo, each retries on transport/5xx | |
| 696 | 695 | // and returns the `?error=` slug to redirect with on failure. | |
| 697 | 696 | let token = match exchange_code_for_token(&state.http, &state.config, &code, &verifier).await { | |
| 698 | 697 | Ok(t) => t, | |
| @@ -737,7 +736,7 @@ | |||
| 737 | 736 | return Redirect::to("/?error=account_suspended"); | |
| 738 | 737 | } | |
| 739 | 738 | ||
| 740 | - | // Save session — perks come from the same userinfo response, no second roundtrip. | |
| 739 | + | // Save session, perks come from the same userinfo response, no second roundtrip. | |
| 741 | 740 | let session_user = SessionUser { | |
| 742 | 741 | user_id: info.user_id, | |
| 743 | 742 | username: info.username, | |
| @@ -749,7 +748,7 @@ | |||
| 749 | 748 | // refreshes can mint short-lived access tokens without another OAuth round | |
| 750 | 749 | // trip. The access token was already used for the userinfo fetch above and | |
| 751 | 750 | // is now discarded. A provider that declined offline_access returns no | |
| 752 | - | // refresh token; then perk-refresh is simply unavailable until re-login. | |
| 751 | + | // refresh token; then perk-refresh is unavailable until re-login. | |
| 753 | 752 | if let Some(refresh_token) = token.refresh_token.as_deref() { | |
| 754 | 753 | if let Err(e) = session.insert(SESSION_REFRESH_TOKEN, refresh_token).await { | |
| 755 | 754 | tracing::error!(error = %e, "failed to save refresh token to session"); | |
| @@ -767,7 +766,7 @@ | |||
| 767 | 766 | Redirect::to("/") | |
| 768 | 767 | } | |
| 769 | 768 | ||
| 770 | - | /// `POST /auth/refresh` — re-fetch MNW userinfo and overwrite cached perks. | |
| 769 | + | /// `POST /auth/refresh`, re-fetch MNW userinfo and overwrite cached perks. | |
| 771 | 770 | /// | |
| 772 | 771 | /// Useful after the user takes an action that changed their MNW entitlements | |
| 773 | 772 | /// (e.g., subscribing to Fan+, upgrading a creator tier) so they don't have to | |
| @@ -780,9 +779,9 @@ | |||
| 780 | 779 | ) -> Result<Json<RefreshResponse>, StatusCode> { | |
| 781 | 780 | match refresh_session(&state, &session).await { | |
| 782 | 781 | Ok(perks) => Ok(Json(RefreshResponse { perks })), | |
| 783 | - | // 401 means "perks couldn't be refreshed" — NOT logged out. The session | |
| 782 | + | // 401 means "perks couldn't be refreshed", NOT logged out. The session | |
| 784 | 783 | // is intact; the frontend can surface a re-link affordance. No flush. | |
| 785 | - | Err(UserinfoError::Unauthorized) | Err(UserinfoError::RefreshUnavailable) => { | |
| 784 | + | Err(UserinfoError::Unauthorized | UserinfoError::RefreshUnavailable) => { | |
| 786 | 785 | Err(StatusCode::UNAUTHORIZED) | |
| 787 | 786 | } | |
| 788 | 787 | Err(UserinfoError::Transport) => Err(StatusCode::BAD_GATEWAY), | |
| @@ -795,7 +794,7 @@ | |||
| 795 | 794 | pub perks: UserPerks, | |
| 796 | 795 | } | |
| 797 | 796 | ||
| 798 | - | /// `POST /auth/logout` — flush session, redirect home. | |
| 797 | + | /// `POST /auth/logout`, flush session, redirect home. | |
| 799 | 798 | #[tracing::instrument(skip_all)] | |
| 800 | 799 | pub async fn logout(session: Session) -> impl IntoResponse { | |
| 801 | 800 | if let Err(e) = session.flush().await { |
| @@ -67,9 +67,7 @@ | |||
| 67 | 67 | platform_admin_id: std::env::var("PLATFORM_ADMIN_ID") | |
| 68 | 68 | .ok() | |
| 69 | 69 | .and_then(|s| Uuid::parse_str(&s).ok()), | |
| 70 | - | cookie_secure: std::env::var("COOKIE_SECURE") | |
| 71 | - | .map(|v| v != "false") | |
| 72 | - | .unwrap_or(true), | |
| 70 | + | cookie_secure: std::env::var("COOKIE_SECURE").map_or(true, |v| v != "false"), | |
| 73 | 71 | s3: S3Config::from_env(), | |
| 74 | 72 | internal_shared_secret: std::env::var("INTERNAL_SHARED_SECRET").ok(), | |
| 75 | 73 | trusted_proxies: parse_trusted_proxies( | |
| @@ -101,12 +99,11 @@ | |||
| 101 | 99 | /// match the bare host against the loopback set (any `127.0.0.0/8`, `localhost`, | |
| 102 | 100 | /// `::1`). | |
| 103 | 101 | fn is_loopback_url(url: &str) -> bool { | |
| 104 | - | let after_scheme = match url | |
| 102 | + | let Some(after_scheme) = url | |
| 105 | 103 | .strip_prefix("http://") | |
| 106 | 104 | .or_else(|| url.strip_prefix("https://")) | |
| 107 | - | { | |
| 108 | - | Some(rest) => rest, | |
| 109 | - | None => return false, | |
| 105 | + | else { | |
| 106 | + | return false; | |
| 110 | 107 | }; | |
| 111 | 108 | let authority = after_scheme.split('/').next().unwrap_or(""); | |
| 112 | 109 | let host_and_port = match authority.rsplit_once('@') { | |
| @@ -123,9 +120,7 @@ | |||
| 123 | 120 | if host == "localhost" || host == "::1" { | |
| 124 | 121 | return true; | |
| 125 | 122 | } | |
| 126 | - | host.parse::<IpAddr>() | |
| 127 | - | .map(|ip| ip.is_loopback()) | |
| 128 | - | .unwrap_or(false) | |
| 123 | + | host.parse::<IpAddr>().is_ok_and(|ip| ip.is_loopback()) | |
| 129 | 124 | } | |
| 130 | 125 | ||
| 131 | 126 | /// Parse `TRUSTED_PROXIES` (comma-separated IPs). Unset → loopback only; an | |
| @@ -184,7 +179,7 @@ | |||
| 184 | 179 | } | |
| 185 | 180 | ||
| 186 | 181 | #[test] | |
| 187 | - | #[should_panic] | |
| 182 | + | #[should_panic(expected = "must be https for a non-loopback deployment")] | |
| 188 | 183 | fn non_https_public_url_panics() { | |
| 189 | 184 | assert_secure_url("MNW_BASE_URL", "http://127.0.0.1.attacker.com/"); | |
| 190 | 185 | } |
| @@ -90,7 +90,7 @@ | |||
| 90 | 90 | /// | |
| 91 | 91 | /// Two delivery paths, in order: | |
| 92 | 92 | /// 1. `X-CSRF-Token` header (set by mt.js for every fetch/HTMX request). This | |
| 93 | - | /// is the fast path — the request body is never touched. | |
| 93 | + | /// is the fast path, the request body is never touched. | |
| 94 | 94 | /// 2. A hidden `csrf_token` form field, for graceful degradation when mt.js | |
| 95 | 95 | /// didn't run (JS disabled, asset failure). Only urlencoded bodies are | |
| 96 | 96 | /// inspected, and only when the header is absent; multipart uploads remain | |
| @@ -131,7 +131,7 @@ | |||
| 131 | 131 | .headers() | |
| 132 | 132 | .get("X-CSRF-Token") | |
| 133 | 133 | .and_then(|v| v.to_str().ok()) | |
| 134 | - | .map(|s| s.to_string()) | |
| 134 | + | .map(std::string::ToString::to_string) | |
| 135 | 135 | { | |
| 136 | 136 | return match session_token { | |
| 137 | 137 | Some(ref expected) if constant_time_compare(expected, &header_token) => { | |
| @@ -150,8 +150,7 @@ | |||
| 150 | 150 | .headers() | |
| 151 | 151 | .get(axum::http::header::CONTENT_TYPE) | |
| 152 | 152 | .and_then(|v| v.to_str().ok()) | |
| 153 | - | .map(|ct| ct.starts_with("application/x-www-form-urlencoded")) | |
| 154 | - | .unwrap_or(false); | |
| 153 | + | .is_some_and(|ct| ct.starts_with("application/x-www-form-urlencoded")); | |
| 155 | 154 | ||
| 156 | 155 | if !is_form { | |
| 157 | 156 | tracing::warn!(path = %path, "CSRF token missing"); | |
| @@ -159,12 +158,9 @@ | |||
| 159 | 158 | } | |
| 160 | 159 | ||
| 161 | 160 | let (parts, body) = request.into_parts(); | |
| 162 | - | let bytes = match axum::body::to_bytes(body, MAX_FORM_FALLBACK_BYTES).await { | |
| 163 | - | Ok(b) => b, | |
| 164 | - | Err(_) => { | |
| 165 | - | tracing::warn!(path = %path, "CSRF fallback: body too large or unreadable"); | |
| 166 | - | return (StatusCode::FORBIDDEN, "CSRF token required").into_response(); | |
| 167 | - | } | |
| 161 | + | let Ok(bytes) = axum::body::to_bytes(body, MAX_FORM_FALLBACK_BYTES).await else { | |
| 162 | + | tracing::warn!(path = %path, "CSRF fallback: body too large or unreadable"); | |
| 163 | + | return (StatusCode::FORBIDDEN, "CSRF token required").into_response(); | |
| 168 | 164 | }; | |
| 169 | 165 | ||
| 170 | 166 | let form_token = extract_form_field(&bytes, "csrf_token"); |
| @@ -3,7 +3,7 @@ | |||
| 3 | 3 | //! The router `.fallback` renders a branded, session-aware error page, but | |
| 4 | 4 | //! errors raised *inside* a handler (missing thread/community, a bad UUID in the | |
| 5 | 5 | //! path, a DB failure) used to short-circuit with bare `(StatusCode, &str)` | |
| 6 | - | //! plaintext — unbranded and jarring. These helpers render the same | |
| 6 | + | //! plaintext, unbranded and jarring. These helpers render the same | |
| 7 | 7 | //! `Error404Template`/`Error500Template` instead. | |
| 8 | 8 | //! | |
| 9 | 9 | //! They render a **session-less** shell (logged-out header), because the deep | |
| @@ -61,7 +61,7 @@ | |||
| 61 | 61 | } | |
| 62 | 62 | ||
| 63 | 63 | /// A fully static, template-free branded 500. Used only when the Askama render | |
| 64 | - | /// itself fails — falling back to [`internal_error`] there would just re-enter | |
| 64 | + | /// itself fails; falling back to [`internal_error`] there would re-enter | |
| 65 | 65 | /// the template engine that's already broken. No interpolation, so it can never | |
| 66 | 66 | /// fail to produce output. | |
| 67 | 67 | pub fn internal_error_static() -> Response { |
| @@ -1,17 +1,17 @@ | |||
| 1 | 1 | //! HMAC-SHA256 authentication for internal API requests from MNW. | |
| 2 | 2 | //! | |
| 3 | - | //! The signed message binds method + path + nonce as well as timestamp + body — | |
| 4 | - | //! `HMAC-SHA256(timestamp \n METHOD \n PATH \n NONCE \n body)` — sent in | |
| 3 | + | //! The signed message binds method + path + nonce as well as timestamp + body, | |
| 4 | + | //! `HMAC-SHA256(timestamp \n METHOD \n PATH \n NONCE \n body)`, sent in | |
| 5 | 5 | //! `X-Internal-{Timestamp,Signature,Nonce}`. Binding method+path stops a | |
| 6 | 6 | //! captured signature being replayed to a different endpoint; the nonce, checked | |
| 7 | 7 | //! against a single-use cache, stops it being re-sent at all within the 60s | |
| 8 | 8 | //! freshness window. | |
| 9 | 9 | //! | |
| 10 | 10 | //! A nonce is **mandatory**: there is exactly one verification path. A request | |
| 11 | - | //! with no `X-Internal-Nonce` is rejected outright (401), not downgraded — the | |
| 11 | + | //! with no `X-Internal-Nonce` is rejected outright (401), not downgraded, the | |
| 12 | 12 | //! legacy v1 (timestamp+body) format and its dual-accept fallback were deleted | |
| 13 | 13 | //! once the MNW signer moved fully to v2, closing the replay window an attacker | |
| 14 | - | //! could otherwise select by simply omitting the nonce header. | |
| 14 | + | //! could otherwise select by omitting the nonce header. | |
| 15 | 15 | ||
| 16 | 16 | use std::collections::HashMap; | |
| 17 | 17 | use std::sync::{LazyLock, Mutex}; | |
| @@ -39,7 +39,7 @@ | |||
| 39 | 39 | /// Process-wide cache of recently-seen request nonces, for single-use | |
| 40 | 40 | /// enforcement. MT runs as a single process (one `TcpListener`), so a local | |
| 41 | 41 | /// cache is authoritative. Entries are evicted once older than the freshness | |
| 42 | - | /// window — a request that old is already rejected by the timestamp check, so a | |
| 42 | + | /// window, a request that old is already rejected by the timestamp check, so a | |
| 43 | 43 | /// nonce can never be replayed after it ages out. Memory is therefore bounded | |
| 44 | 44 | /// by (request rate × window), and the internal rate limiter caps that. Nonces | |
| 45 | 45 | /// are inserted only AFTER the signature verifies, so unauthenticated traffic | |
| @@ -64,11 +64,13 @@ | |||
| 64 | 64 | /// Eviction is time-bucketed: the O(n) sweep of aged entries runs at most once | |
| 65 | 65 | /// per freshness window, not on every call, so the hot internal path stays | |
| 66 | 66 | /// effectively O(1) under the lock. Keeping an aged entry slightly longer is | |
| 67 | - | /// harmless — a request old enough to evict is already rejected by the timestamp | |
| 67 | + | /// harmless, a request old enough to evict is already rejected by the timestamp | |
| 68 | 68 | /// freshness check before it ever reaches here, so it can't be the nonce we'd | |
| 69 | 69 | /// have swept. Worst-case memory is ~2× the window's traffic instead of 1×. | |
| 70 | 70 | fn record_nonce(nonce: &str, now_unix: i64) -> bool { | |
| 71 | - | let mut cache = NONCE_CACHE.lock().unwrap_or_else(|e| e.into_inner()); | |
| 71 | + | let mut cache = NONCE_CACHE | |
| 72 | + | .lock() | |
| 73 | + | .unwrap_or_else(std::sync::PoisonError::into_inner); | |
| 72 | 74 | if now_unix - cache.last_sweep >= MAX_TIMESTAMP_AGE_SECS { | |
| 73 | 75 | cache | |
| 74 | 76 | .seen | |
| @@ -195,7 +197,7 @@ | |||
| 195 | 197 | } | |
| 196 | 198 | ||
| 197 | 199 | /// Verify a signed internal request, binding method + path + nonce. A nonce is | |
| 198 | - | /// mandatory — a request without one is rejected (401), never downgraded. This | |
| 200 | + | /// mandatory, a request without one is rejected (401), never downgraded. This | |
| 199 | 201 | /// is the single verification path; the legacy v1 (timestamp+body) fallback was | |
| 200 | 202 | /// deleted once the MNW signer moved fully to v2. Freshness is checked first. | |
| 201 | 203 | /// | |
| @@ -290,7 +292,7 @@ | |||
| 290 | 292 | ||
| 291 | 293 | #[test] | |
| 292 | 294 | fn signature_changes_with_each_bound_field() { | |
| 293 | - | // Pins that timestamp, method, path, nonce, and body each feed the MAC — | |
| 295 | + | // Pins that timestamp, method, path, nonce, and body each feed the MAC, | |
| 294 | 296 | // a mutation dropping any field would collide one of these pairs. | |
| 295 | 297 | let base = v2("s", "100", "POST", "/x", "n", b"body"); | |
| 296 | 298 | assert_ne!( | |
| @@ -489,7 +491,7 @@ | |||
| 489 | 491 | ||
| 490 | 492 | #[test] | |
| 491 | 493 | fn verify_rejects_missing_nonce() { | |
| 492 | - | // A request with no nonce is rejected outright — no v1 downgrade exists. | |
| 494 | + | // A request with no nonce is rejected outright, no v1 downgrade exists. | |
| 493 | 495 | let sig = v2("s", "1000", "POST", "/internal/x", "abc", b"body"); | |
| 494 | 496 | let (status, msg) = verify_signed_request( | |
| 495 | 497 | "s", | |
| @@ -571,7 +573,7 @@ | |||
| 571 | 573 | ||
| 572 | 574 | #[test] | |
| 573 | 575 | fn verify_check_order_freshness_before_signature() { | |
| 574 | - | // A stale timestamp must reject even when the sig is otherwise valid — | |
| 576 | + | // A stale timestamp must reject even when the sig is otherwise valid, | |
| 575 | 577 | // catches a mutation running the freshness check after signature verify. | |
| 576 | 578 | let sig = v2("s", "1000", "POST", "/x", "abc", b"abc"); | |
| 577 | 579 | let (_, msg) = verify_signed_request( |