Skip to main content

max / makenotwork

Choose rate limits at runtime, so CI tests the limiter that ships The `fast-tests` feature swapped the auth and sandbox rate-limit constants, which meant the limiter under test was never the limiter in production. The sweep runs `cargo test --all-features`, so CI only ever exercised the relaxed values; a plain `cargo test` used production values but skipped the tests needing relaxed ones. Neither configuration covered both. The profile is `constants::RateLimits` now, carried on Config and passed from build_app into the route builders. Production is the Default and the constants are unconditional, so no feature can quietly relax what ships. This is what lets both profiles coexist in one run. The six tests in workflows::rate_limiting build a production router and assert the real thresholds; everything else builds relaxed routers, because a test that logs in six times is not asking to be throttled. Those six were `#[ignore]`d under `fast-tests`, which is how astra ran them, so they never executed on CI at all. Their stated flakiness was the relaxed bucket refilling at 100/sec faster than a loaded box could drain it; production refills at 2/sec, so the cause is gone and the ignores with it. `--all-features` now runs 1,240 tests with 4 ignored, against 1,232 and 12 before. fast-tests keeps only the cheap Argon2id parameters, documented in Cargo.toml so it does not re-acquire anything load-bearing. Also carries three unrelated doc-comment edits that were already uncommitted in constants.rs, config.rs and lib.rs (ClamAV tail-scan rationale, a stale founder-pricing reference, and audit findings moving to GoingsOn). They are doc-only and could not be split out without leaving a non-compiling commit. Phase 1 of wiki testing-posture.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-08-05 04:01 UTC
Signed with PGP, not checked
Commit: 532688a51aadcaa4cb5f78b2aa38b9bd32300934
Parent: d758d8b
14 files changed, +154 insertions, -101 deletions
@@ -8,6 +8,15 @@
8 8 publish = false
9 9
10 10 [features]
11 + # Cheap Argon2id parameters (8 MiB, 1 iteration) so a suite that seeds hundreds
12 + # of password hashes is not spending ~600ms on each. Scoped to `auth::hash_password`
13 + # and nothing else: verification reads its parameters from the hash string, so
14 + # the production verifier is still what runs.
15 + #
16 + # It used to swap the rate-limit constants too, which meant the limiter under
17 + # test was never the limiter that ships. Those are runtime config now
18 + # (`constants::RateLimits`), so this feature can no longer change how the server
19 + # behaves under load. Do not add anything to it that can.
11 20 fast-tests = []
12 21
13 22 [dependencies]
@@ -1007,6 +1007,7 @@
1007 1007 user_pages_host: std::sync::Arc::from("u.localhost"),
1008 1008 access_gate: crate::config::AccessGate::Open,
1009 1009 sso: None,
1010 + rate_limits: crate::constants::RateLimits::production(),
1010 1011 build: BuildConfig {
1011 1012 trigger_token: None,
1012 1013 host_linux: None,
@@ -1088,6 +1089,7 @@
1088 1089 user_pages_host: std::sync::Arc::from("u.localhost"),
1089 1090 access_gate: crate::config::AccessGate::Open,
1090 1091 sso: None,
1092 + rate_limits: crate::constants::RateLimits::production(),
1091 1093 build: BuildConfig {
1092 1094 trigger_token: None,
1093 1095 host_linux: None,
@@ -65,6 +65,9 @@
65 65 /// `provider_url`'s OAuth endpoints instead of a local password form, used
66 66 /// on the testnot mirror so a password is only ever entered on production.
67 67 pub sso: Option<SsoConfig>,
68 + /// Rate-limit profile the router is built with. Production everywhere that
69 + /// is not a test; see [`crate::constants::RateLimits`].
70 + pub rate_limits: crate::constants::RateLimits,
68 71 }
69 72
70 73 /// Native build pipeline configuration (`BUILD_*`, `GIT_*`).
@@ -105,7 +108,7 @@
105 108 /// Creator-tier and Fan+ Stripe price maps plus the founder-window flag.
106 109 ///
107 110 /// Missing annual/founder entries fall back per the checkout logic (annual →
108 - /// monthly, founder → sticker); see `project_founder_pricing.md`.
111 + /// monthly, founder → sticker).
109 112 #[derive(Clone)]
110 113 pub struct CreatorTierPricing {
111 114 /// Stripe Price ID for the Fan+ subscription ($8/mo). Enables Fan+ checkout when set.
@@ -491,6 +494,7 @@
491 494 user_pages_host: Arc::from(user_pages_host),
492 495 access_gate,
493 496 sso,
497 + rate_limits: crate::constants::RateLimits::production(),
494 498 build: BuildConfig {
495 499 trigger_token: build_trigger_token,
496 500 host_linux: build_host_linux,
@@ -1050,6 +1054,7 @@
1050 1054 user_pages_host: Arc::from("u.localhost"),
1051 1055 access_gate: AccessGate::Open,
1052 1056 sso: None,
1057 + rate_limits: crate::constants::RateLimits::production(),
1053 1058 build: BuildConfig {
1054 1059 trigger_token: None,
1055 1060 host_linux: None,
@@ -151,15 +151,15 @@
151 151
152 152 // Rate limiting
153 153 // Auth endpoints (login, join): burst 5, then 2/sec.
154 - // fast-tests: relaxed to burst 20 so lockout tests can fire 5+ attempts without hitting rate limiter.
155 - #[cfg(not(feature = "fast-tests"))]
154 + //
155 + // These are the values that ship, unconditionally. They used to be swapped by
156 + // `#[cfg(feature = "fast-tests")]`, which meant the limiter under test was never
157 + // the limiter in production: the sweep runs `cargo test --all-features`, so CI
158 + // only ever exercised the relaxed one, while a plain `cargo test` skipped the
159 + // tests that need relaxed values. Neither configuration covered both. The
160 + // profile is chosen at runtime now, see [`RateLimits`].
156 161 pub const AUTH_RATE_LIMIT_MS: u64 = 500;
157 - #[cfg(not(feature = "fast-tests"))]
158 162 pub const AUTH_RATE_LIMIT_BURST: u32 = 5;
159 - #[cfg(feature = "fast-tests")]
160 - pub const AUTH_RATE_LIMIT_MS: u64 = 10;
161 - #[cfg(feature = "fast-tests")]
162 - pub const AUTH_RATE_LIMIT_BURST: u32 = 20;
163 163 // Username validation: burst 10, then 1/sec
164 164 pub const VALIDATE_RATE_LIMIT_PER_SEC: u64 = 1;
165 165 pub const VALIDATE_RATE_LIMIT_BURST: u32 = 10;
@@ -300,9 +300,16 @@
300 300 /// Maximum number of bytes fed to YARA in a single scan. yara-x's `Scanner`
301 301 /// walks the whole slice, which demand-pages the entire mmap resident, so an
302 302 /// 8 GiB object would otherwise pin 8 GiB of page cache per scan (×
303 - /// `SCAN_MAX_CONCURRENT`). Malware signatures cluster near a file's start, and
304 - /// ClamAV (streamed, uncapped) is the full-file backstop, so scanning a generous
305 - /// prefix is the right trade. Above this, YARA sees the prefix and logs the cap.
303 + /// `SCAN_MAX_CONCURRENT`). Malware signatures cluster near a file's start, so
304 + /// scanning a generous prefix is the right trade. Above this, YARA sees the
305 + /// prefix and logs the cap.
306 + ///
307 + /// Do NOT raise this on the assumption that ClamAV covers the tail. ClamAV is a
308 + /// full-file backstop only up to the operator-declared
309 + /// [`crate::config::ScanConfig::clamav_max_scan_bytes`], because clamd does not
310 + /// expose its own limits over the socket. Undeclared coverage is fail-closed:
311 + /// `yara_tail_unscanned` holds anything past this prefix for review rather than
312 + /// certifying it Clean (ultra-fuzz Run #24 Security MODERATE).
306 313 pub const SCAN_YARA_MAX_BYTES: usize = 512 * 1024 * 1024; // 512 MiB
307 314
308 315 /// Per-call deadline for the optional external second-opinion lookups
@@ -438,20 +445,68 @@
438 445 pub const SANDBOX_EXPIRY_SECS: i64 = 3600; // 1 hour
439 446 /// How often the cleanup job runs.
440 447 pub const SANDBOX_CLEANUP_INTERVAL_SECS: u64 = 300; // 5 minutes
441 - /// Rate limit: sandbox creation.
442 - /// Production: 1 per 30 seconds, burst 2. fast-tests: 1 per 10ms, burst 10.
443 - /// Run integration tests with `cargo test --features fast-tests` to avoid rate-limit failures.
444 - #[cfg(not(feature = "fast-tests"))]
448 + /// Rate limit: sandbox creation, 1 per 30 seconds, burst 2. The value that
449 + /// ships; see [`RateLimits`] for how tests relax it.
445 450 pub const SANDBOX_RATE_LIMIT_MS: u64 = 30_000;
446 - #[cfg(not(feature = "fast-tests"))]
447 451 pub const SANDBOX_RATE_LIMIT_BURST: u32 = 2;
448 - #[cfg(feature = "fast-tests")]
449 - pub const SANDBOX_RATE_LIMIT_MS: u64 = 10;
450 - #[cfg(feature = "fast-tests")]
451 - pub const SANDBOX_RATE_LIMIT_BURST: u32 = 10;
452 452 /// Max concurrent active sandboxes per IP.
453 453 pub const SANDBOX_MAX_PER_IP: i64 = 3;
454 454
455 + /// Which rate-limit values a router is built with.
456 + ///
457 + /// Carried on [`crate::config::Config`] and passed to the route builders, so
458 + /// the choice is made once where the app is assembled rather than by a compile
459 + /// feature. Two consequences worth the plumbing:
460 + ///
461 + /// - The production limiter is the default and is what every build ships. There
462 + /// is no feature flag that can quietly relax it.
463 + /// - A single test run can exercise both profiles. The suite builds relaxed
464 + /// routers so unrelated tests are not throttled, and the rate-limit tests
465 + /// build a production router and assert the real thresholds. Previously those
466 + /// tests were `#[ignore]`d under `fast-tests` because the relaxed bucket
467 + /// refills faster than a loaded machine can drain it, so on CI they never ran
468 + /// at all.
469 + #[derive(Debug, Clone, Copy, PartialEq, Eq)]
470 + pub struct RateLimits {
471 + /// Auth endpoints (login, join, email actions, public pages).
472 + pub auth_ms: u64,
473 + pub auth_burst: u32,
474 + /// Sandbox account creation.
475 + pub sandbox_ms: u64,
476 + pub sandbox_burst: u32,
477 + }
478 +
479 + impl RateLimits {
480 + /// The values that ship. Always the default.
481 + pub const fn production() -> Self {
482 + Self {
483 + auth_ms: AUTH_RATE_LIMIT_MS,
484 + auth_burst: AUTH_RATE_LIMIT_BURST,
485 + sandbox_ms: SANDBOX_RATE_LIMIT_MS,
486 + sandbox_burst: SANDBOX_RATE_LIMIT_BURST,
487 + }
488 + }
489 +
490 + /// Relaxed values for tests that are not about rate limiting and would
491 + /// otherwise be throttled by their own setup traffic. A lockout test needs
492 + /// to fire more than five auth attempts; a fixture needs more than one
493 + /// sandbox per 30 seconds.
494 + pub const fn relaxed() -> Self {
495 + Self {
496 + auth_ms: 10,
497 + auth_burst: 20,
498 + sandbox_ms: 10,
499 + sandbox_burst: 10,
500 + }
501 + }
502 + }
503 +
504 + impl Default for RateLimits {
505 + fn default() -> Self {
506 + Self::production()
507 + }
508 + }
509 +
455 510 // ── Compile-time invariants on the constants above ───────────────────────────
456 511 //
457 512 // Encoded as `const _: () = assert!(...)` rather than `#[test]` functions: these
@@ -3,7 +3,7 @@
3 3 //! # Design
4 4 //!
5 5 //! Design plans, deploy runbooks, and the business/strategy/SOP docs live in the
6 - //! maintainer wiki; the risk history is in the gitignored `docs/audit_review.md`.
6 + //! maintainer wiki; audit and fuzz findings are tracked as problems in GoingsOn.
7 7 //! <!-- wiki: mnw-server-overview -->
8 8
9 9 /// Tracing filter used when `RUST_LOG` is unset, which is how production runs.
@@ -521,8 +521,8 @@
521 521 // structural envelope so global middleware, static-file mounts, and
522 522 // the few bare GETs below can attach to a plain `Router<AppState>`.
523 523 let csrf_routes = csrf::CsrfRouter::new()
524 - .merge(auth_routes())
525 - .merge(api_routes())
524 + .merge(auth_routes(state.config.rate_limits))
525 + .merge(api_routes(state.config.rate_limits))
526 526 .merge(storage_routes())
527 527 .merge(stripe_routes())
528 528 .merge(admin_routes(state.clone()))
@@ -540,7 +540,7 @@
540 540 .merge(build_routes())
541 541 .finalize();
542 542 let app = Router::new()
543 - .merge(page_routes())
543 + .merge(page_routes(state.config.rate_limits))
544 544 .merge(sso_routes())
545 545 .merge(csrf_routes)
546 546 .merge(git_routes())
@@ -37,11 +37,8 @@
37 37 });
38 38
39 39 /// Register authentication routes with rate limiting.
40 - pub fn auth_routes() -> CsrfRouter<AppState> {
41 - let auth_rate_limit = rate_limiter_ms(
42 - constants::AUTH_RATE_LIMIT_MS,
43 - constants::AUTH_RATE_LIMIT_BURST,
44 - );
40 + pub fn auth_routes(limits: constants::RateLimits) -> CsrfRouter<AppState> {
41 + let auth_rate_limit = rate_limiter_ms(limits.auth_ms, limits.auth_burst);
45 42 let validate_rate_limit = rate_limiter_per_sec(
46 43 constants::VALIDATE_RATE_LIMIT_PER_SEC,
47 44 constants::VALIDATE_RATE_LIMIT_BURST,
@@ -99,6 +99,11 @@
99 99 /// checkout is unconfigured and bails). Set to exercise creator-tier flows.
100 100 pub creator_tier_prices:
101 101 Option<std::collections::HashMap<makenotwork::db::CreatorTier, String>>,
102 + /// Rate-limit profile. `None` relaxes the limits, which is what almost every
103 + /// test wants: a test that logs in six times is not asking to be throttled.
104 + /// Set `Some(RateLimits::production())` to assert the thresholds that ship,
105 + /// which is what `workflows::rate_limiting` does.
106 + pub rate_limits: Option<makenotwork::constants::RateLimits>,
102 107 }
103 108
104 109 /// Full test harness: isolated database, in-process app, cookie-aware client.
@@ -129,6 +134,20 @@
129 134 Self::build(BuildOptions::default()).await
130 135 }
131 136
137 + /// Harness whose router carries the rate limits that ship, rather than the
138 + /// relaxed profile every other harness uses. This is the only way to assert
139 + /// the real thresholds, and the reason the limits are runtime config: under
140 + /// the old `fast-tests` feature the two profiles could not coexist in one
141 + /// run, so CI exercised the relaxed limiter and never the production one.
142 + #[allow(dead_code)]
143 + pub(crate) async fn with_production_rate_limits() -> Self {
144 + Self::build(BuildOptions {
145 + rate_limits: Some(makenotwork::constants::RateLimits::production()),
146 + ..Default::default()
147 + })
148 + .await
149 + }
150 +
132 151 /// Harness with in-memory storage backend.
133 152 #[allow(dead_code)]
134 153 pub(crate) async fn with_storage() -> Self {
@@ -354,6 +373,9 @@
354 373 user_pages_host: std::sync::Arc::from("u.localhost"),
355 374 access_gate: opts.access_gate,
356 375 sso: opts.sso.clone(),
376 + rate_limits: opts
377 + .rate_limits
378 + .unwrap_or_else(makenotwork::constants::RateLimits::relaxed),
357 379 build: BuildConfig {
358 380 trigger_token: opts.build_trigger_token,
359 381 host_linux: None,
@@ -68,6 +68,9 @@
68 68 user_pages_host: std::sync::Arc::from("u.localhost"),
69 69 access_gate: makenotwork::config::AccessGate::Open,
70 70 sso: None,
71 + // The load runner drives thousands of requests from one IP, so the
72 + // production limiter would measure the limiter rather than the server.
73 + rate_limits: makenotwork::constants::RateLimits::relaxed(),
71 74 build: BuildConfig {
72 75 trigger_token: None,
73 76 host_linux: None,
@@ -5,21 +5,18 @@
5 5 //! on every request, and SmartIpKeyExtractor (fallback from CloudflareIpKeyExtractor)
6 6 //! uses that header for keying, so rate limiting works in-process.
7 7 //!
8 - //! ## Flakiness on CI (astra)
8 + //! Every test here builds `TestHarness::with_production_rate_limits()`, so the
9 + //! thresholds asserted are the ones that ship. The rest of the suite builds
10 + //! relaxed routers, because a test that logs in six times is not asking to be
11 + //! throttled.
9 12 //!
10 - //! Under `--features fast-tests` the token bucket refills at 100/sec (burst
11 - //! 20). On a fast Mac this is easy to deplete sequentially, but astra under
12 - //! `--test-threads=8` + postgres contention slows per-request execution past
13 - //! the refill rate, the bucket never empties and the test fails. Tests
14 - //! tagged `#[cfg_attr(feature = "fast-tests", ignore = "...")]` for that reason.
15 - //!
16 - //! Run them locally with:
17 - //!
18 - //! ```sh
19 - //! TEST_DATABASE_URL="postgres:///postgres" \
20 - //! cargo test --features fast-tests --test integration \
21 - //! -- --ignored --test-threads=1 rate_limit
22 - //! ```
13 + //! These used to be `#[ignore]`d under `--features fast-tests`, which is how
14 + //! astra ran them: the relaxed bucket refills at 100/sec, and under
15 + //! `--test-threads=8` plus postgres contention the machine could not drain it
16 + //! faster than it filled, so the 429 never came. That made the limiter that
17 + //! actually ships the one thing CI never exercised. Production values refill at
18 + //! 2/sec, which a loaded box outruns comfortably, so the tests run everywhere
19 + //! now and the feature flag is out of the picture.
23 20
24 21 use crate::harness::TestHarness;
25 22 use makenotwork::constants::{
@@ -31,12 +28,8 @@
31 28 /// Send AUTH_RATE_LIMIT_BURST + 1 login attempts rapidly and verify the last
32 29 /// one returns 429 Too Many Requests.
33 30 #[tokio::test]
34 - #[cfg_attr(
35 - feature = "fast-tests",
36 - ignore = "flaky under contention on astra; run with --ignored --test-threads=1"
37 - )]
38 31 async fn auth_rate_limit_triggers_on_burst() {
39 - let mut h = TestHarness::new().await;
32 + let mut h = TestHarness::with_production_rate_limits().await;
40 33
41 34 // Use a distinct IP so we don't collide with other tests
42 35 h.client.set_forwarded_ip("10.0.0.1");
@@ -72,12 +65,8 @@
72 65 /// After triggering a rate limit, the 429 response must include a `retry-after`
73 66 /// header so clients know when to retry.
74 67 #[tokio::test]
75 - #[cfg_attr(
76 - feature = "fast-tests",
77 - ignore = "flaky under contention on astra; run with --ignored --test-threads=1"
78 - )]
79 68 async fn rate_limit_returns_retry_after_header() {
80 - let mut h = TestHarness::new().await;
69 + let mut h = TestHarness::with_production_rate_limits().await;
81 70 h.client.set_forwarded_ip("10.0.1.1");
82 71
83 72 let mut last_resp = None;
@@ -107,12 +96,8 @@
107 96 /// Exhaust the rate limit from one IP, then verify a different IP is not
108 97 /// affected. Uses X-Forwarded-For to distinguish IPs.
109 98 #[tokio::test]
110 - #[cfg_attr(
111 - feature = "fast-tests",
112 - ignore = "flaky under contention on astra; run with --ignored --test-threads=1"
113 - )]
114 99 async fn rate_limit_different_ips_independent() {
115 - let mut h = TestHarness::new().await;
100 + let mut h = TestHarness::with_production_rate_limits().await;
116 101
117 102 // Exhaust burst from IP "1.2.3.4" using passkey endpoint (fast, no Argon2)
118 103 h.client.set_forwarded_ip("1.2.3.4");
@@ -145,14 +130,9 @@
145 130 // Sandbox rate limiting
146 131
147 132 /// Send SANDBOX_RATE_LIMIT_BURST + 1 POST /sandbox requests and verify 429.
148 - /// Skipped with `fast-tests`, relaxed rate limits make this test meaningless.
149 133 #[tokio::test]
150 - #[cfg_attr(
151 - feature = "fast-tests",
152 - ignore = "flaky under contention on astra; run with --ignored --test-threads=1"
153 - )]
154 134 async fn sandbox_rate_limit_triggers() {
155 - let mut h = TestHarness::new().await;
135 + let mut h = TestHarness::with_production_rate_limits().await;
156 136 h.client.set_forwarded_ip("10.0.2.1");
157 137
158 138 let mut got_429 = false;
@@ -178,12 +158,8 @@
178 158 /// As a logged-in creator, send API_WRITE_RATE_LIMIT_BURST + 1 POST requests
179 159 /// to a write endpoint and verify 429.
180 160 #[tokio::test]
181 - #[cfg_attr(
182 - feature = "fast-tests",
183 - ignore = "flaky under contention on astra; run with --ignored --test-threads=1"
184 - )]
185 161 async fn api_write_rate_limit_triggers() {
186 - let mut h = TestHarness::new().await;
162 + let mut h = TestHarness::with_production_rate_limits().await;
187 163 h.client.set_forwarded_ip("10.0.3.1");
188 164 let _user_id = h.create_creator("ratelimiter").await;
189 165
@@ -215,16 +191,10 @@
215 191 /// router (`email_action_routes`). Run #11 found `/login-link`, `/reset-password`,
216 192 /// `/verify-email`, `/confirm-delete`, and `/unsubscribe` uncapped while only
217 193 /// `/forgot-password` was limited. This pins that the cap now fires on the
218 - /// previously-uncapped routes. Ignored under fast-tests for the same
219 - /// token-bucket-refill-vs-request-rate flakiness as the other rate-limit tests;
220 - /// run with `--ignored --test-threads=1`.
194 + /// previously-uncapped routes.
221 195 #[tokio::test]
222 - #[cfg_attr(
223 - feature = "fast-tests",
224 - ignore = "flaky under contention on astra; run with --ignored --test-threads=1"
225 - )]
226 196 async fn email_action_routes_are_rate_limited() {
227 - let mut h = TestHarness::new().await;
197 + let mut h = TestHarness::with_production_rate_limits().await;
228 198 h.client.set_forwarded_ip("10.0.7.7");
229 199
230 200 let mut got_429 = false;
@@ -177,7 +177,7 @@
177 177 /// - Write routes (POST/PUT/DELETE): burst 10, 2/sec per IP
178 178 /// - Export routes: burst 3, 1/sec per IP (stricter, prevents bulk extraction)
179 179 /// - Read routes (GET): no rate limit (alpha scale)
180 - pub fn api_routes() -> CsrfRouter<AppState> {
180 + pub fn api_routes(limits: constants::RateLimits) -> CsrfRouter<AppState> {
181 181 let write_rate_limit = crate::helpers::rate_limiter_ms(
182 182 constants::API_WRITE_RATE_LIMIT_MS,
183 183 constants::API_WRITE_RATE_LIMIT_BURST,
@@ -555,10 +555,8 @@
555 555 // Password/code-verifying TOTP mutations, strict auth-strength rate limit
556 556 // (matching login), not the looser API-write limit, so confirm-code and
557 557 // disable/regenerate password checks can't be ground (ultra-fuzz Run 10 Sec M1).
558 - let totp_sensitive_rate_limit = crate::helpers::rate_limiter_ms(
559 - constants::AUTH_RATE_LIMIT_MS,
560 - constants::AUTH_RATE_LIMIT_BURST,
561 - );
558 + let totp_sensitive_rate_limit =
559 + crate::helpers::rate_limiter_ms(limits.auth_ms, limits.auth_burst);
562 560 let totp_sensitive_routes = CsrfRouter::new()
563 561 .route("/api/users/me/totp/confirm", post_csrf(totp::confirm))
564 562 .route("/api/users/me/totp/disable", post_csrf(totp::disable))