max / makenotwork
- Co-Authored-By
- Claude Opus 4.8 (1M context) <noreply@anthropic.com>
21 files changed,
+864 insertions,
-641 deletions
| @@ -427,6 +427,12 @@ | |||
| 427 | 427 | /// <https://metadefender.com/account>). Second-opinion layer; only | |
| 428 | 428 | /// invoked when another layer flagged the file as suspicious. | |
| 429 | 429 | pub metadefender_api_key: Option<String>, | |
| 430 | + | /// Minimum number of YARA rule files that must compile for the corpus to be | |
| 431 | + | /// considered healthy. `0` disables the check (default). Set it to the known | |
| 432 | + | /// deployed corpus size so a silent drop (a dependency/format change that | |
| 433 | + | /// makes rules uncompilable) fails boot loudly rather than degrading | |
| 434 | + | /// coverage unnoticed. | |
| 435 | + | pub yara_min_rule_files: usize, | |
| 430 | 436 | } | |
| 431 | 437 | ||
| 432 | 438 | impl ScanConfig { | |
| @@ -453,6 +459,10 @@ | |||
| 453 | 459 | .unwrap_or(true), | |
| 454 | 460 | abuse_ch_auth_key: std::env::var("ABUSE_CH_AUTH_KEY").ok().filter(|s| !s.is_empty()), | |
| 455 | 461 | metadefender_api_key: std::env::var("METADEFENDER_API_KEY").ok().filter(|s| !s.is_empty()), | |
| 462 | + | yara_min_rule_files: std::env::var("YARA_MIN_RULE_FILES") | |
| 463 | + | .ok() | |
| 464 | + | .and_then(|v| v.parse().ok()) | |
| 465 | + | .unwrap_or(0), | |
| 456 | 466 | }) | |
| 457 | 467 | } | |
| 458 | 468 | } |
| @@ -72,6 +72,10 @@ | |||
| 72 | 72 | // -- Scheduled publish -- | |
| 73 | 73 | pub const SCHEDULER_INTERVAL_SECS: u64 = 60; | |
| 74 | 74 | ||
| 75 | + | // How often the rate-limiter bucket-map sweeper reclaims stale GCRA entries. | |
| 76 | + | // Bounds limiter map size by active (not cumulative-unique) client keys. | |
| 77 | + | pub const GOVERNOR_SWEEP_INTERVAL_SECS: u64 = 60; | |
| 78 | + | ||
| 75 | 79 | // -- TOTP / 2FA -- | |
| 76 | 80 | pub const TOTP_SKEW: u8 = 1; // Allow +/-1 time step (+/-30s) | |
| 77 | 81 | pub const TOTP_STEP: u64 = 30; // 30-second windows |
| @@ -224,6 +224,11 @@ | |||
| 224 | 224 | ); | |
| 225 | 225 | } | |
| 226 | 226 | ||
| 227 | + | // All rate limiters were registered as they were built above; start the | |
| 228 | + | // periodic GC that sweeps their bucket maps so they don't grow unbounded for | |
| 229 | + | // process lifetime (Run #14 CHRONIC 1). Guarded by `Once` internally. | |
| 230 | + | crate::rate_limit::start_governor_sweeper(); | |
| 231 | + | ||
| 227 | 232 | app.layer(middleware::from_fn_with_state(state.clone(), access_gate::access_gate_middleware)) | |
| 228 | 233 | .layer(middleware::from_fn_with_state(state.clone(), security_headers_middleware)) | |
| 229 | 234 | .layer(middleware::from_fn(metrics::cache_control_middleware)) |
| @@ -141,6 +141,54 @@ | |||
| 141 | 141 | ||
| 142 | 142 | // ── Config builders ── | |
| 143 | 143 | ||
| 144 | + | // ── Bucket-map sweeping (Run #14 CHRONIC 1) ── | |
| 145 | + | ||
| 146 | + | /// Type-erased `retain_recent` GC hooks, one per limiter built below. Each hook | |
| 147 | + | /// sweeps one limiter's keyed GCRA store and returns its post-sweep entry count. | |
| 148 | + | /// | |
| 149 | + | /// tower_governor's in-memory store grows one entry per unique client key for | |
| 150 | + | /// process lifetime unless swept, so EVERY limiter must be registered here. The | |
| 151 | + | /// three `rate_limiter_*` constructors below are the only sanctioned way to | |
| 152 | + | /// build a limiter precisely because they register on construct — do not build a | |
| 153 | + | /// `GovernorConfig` directly (it would leak, unswept). The registry is touched | |
| 154 | + | /// only at startup (registration) and once per sweep interval, so the `Mutex` is | |
| 155 | + | /// effectively uncontended; no lock is ever held across an `.await`. | |
| 156 | + | static GOVERNOR_SWEEPERS: std::sync::Mutex<Vec<Box<dyn Fn() -> usize + Send + Sync>>> = | |
| 157 | + | std::sync::Mutex::new(Vec::new()); | |
| 158 | + | ||
| 159 | + | /// Register a limiter's GC hook. Monomorphized at each call site (where the | |
| 160 | + | /// limiter's concrete key type is known), so this stays non-generic. | |
| 161 | + | fn register_for_sweep(hook: impl Fn() -> usize + Send + Sync + 'static) { | |
| 162 | + | if let Ok(mut hooks) = GOVERNOR_SWEEPERS.lock() { | |
| 163 | + | hooks.push(Box::new(hook)); | |
| 164 | + | } | |
| 165 | + | } | |
| 166 | + | ||
| 167 | + | /// Spawn the periodic task that sweeps every registered limiter's bucket map. | |
| 168 | + | /// Call once at startup (guarded by `Once`, so extra calls — e.g. per-test | |
| 169 | + | /// `build_app` — are no-ops). Requires a Tokio runtime. | |
| 170 | + | pub fn start_governor_sweeper() { | |
| 171 | + | static STARTED: std::sync::Once = std::sync::Once::new(); | |
| 172 | + | STARTED.call_once(|| { | |
| 173 | + | tokio::spawn(async { | |
| 174 | + | let interval = | |
| 175 | + | std::time::Duration::from_secs(crate::constants::GOVERNOR_SWEEP_INTERVAL_SECS); | |
| 176 | + | loop { | |
| 177 | + | tokio::time::sleep(interval).await; | |
| 178 | + | // Collect counts without holding the lock across any await. | |
| 179 | + | let (limiters, retained) = { | |
| 180 | + | let Ok(hooks) = GOVERNOR_SWEEPERS.lock() else { | |
| 181 | + | continue; | |
| 182 | + | }; | |
| 183 | + | let retained: usize = hooks.iter().map(|hook| hook()).sum(); | |
| 184 | + | (hooks.len(), retained) | |
| 185 | + | }; | |
| 186 | + | tracing::debug!(limiters, retained_keys = retained, "swept governor bucket maps"); | |
| 187 | + | } | |
| 188 | + | }); | |
| 189 | + | }); | |
| 190 | + | } | |
| 191 | + | ||
| 144 | 192 | /// Build an IP-based rate limiter from a per-millisecond interval and burst size. | |
| 145 | 193 | pub fn rate_limiter_ms( | |
| 146 | 194 | ms: u64, | |
| @@ -151,7 +199,7 @@ | |||
| 151 | 199 | ::governor::middleware::StateInformationMiddleware, | |
| 152 | 200 | >, | |
| 153 | 201 | > { | |
| 154 | - | std::sync::Arc::new( | |
| 202 | + | let config = std::sync::Arc::new( | |
| 155 | 203 | tower_governor::governor::GovernorConfigBuilder::default() | |
| 156 | 204 | .key_extractor(CloudflareIpKeyExtractor) | |
| 157 | 205 | .per_millisecond(ms) | |
| @@ -159,7 +207,13 @@ | |||
| 159 | 207 | .use_headers() | |
| 160 | 208 | .finish() | |
| 161 | 209 | .expect("rate limiter config"), | |
| 162 | - | ) | |
| 210 | + | ); | |
| 211 | + | let limiter = config.limiter().clone(); | |
| 212 | + | register_for_sweep(move || { | |
| 213 | + | limiter.retain_recent(); | |
| 214 | + | limiter.len() | |
| 215 | + | }); | |
| 216 | + | config | |
| 163 | 217 | } | |
| 164 | 218 | ||
| 165 | 219 | /// Build an IP-based rate limiter from a per-second rate and burst size. | |
| @@ -172,7 +226,7 @@ | |||
| 172 | 226 | ::governor::middleware::StateInformationMiddleware, | |
| 173 | 227 | >, | |
| 174 | 228 | > { | |
| 175 | - | std::sync::Arc::new( | |
| 229 | + | let config = std::sync::Arc::new( | |
| 176 | 230 | tower_governor::governor::GovernorConfigBuilder::default() | |
| 177 | 231 | .key_extractor(CloudflareIpKeyExtractor) | |
| 178 | 232 | .per_second(per_sec) | |
| @@ -180,7 +234,13 @@ | |||
| 180 | 234 | .use_headers() | |
| 181 | 235 | .finish() | |
| 182 | 236 | .expect("rate limiter config"), | |
| 183 | - | ) | |
| 237 | + | ); | |
| 238 | + | let limiter = config.limiter().clone(); | |
| 239 | + | register_for_sweep(move || { | |
| 240 | + | limiter.retain_recent(); | |
| 241 | + | limiter.len() | |
| 242 | + | }); | |
| 243 | + | config | |
| 184 | 244 | } | |
| 185 | 245 | ||
| 186 | 246 | /// Build a per-SyncKit-app rate limiter from a per-millisecond interval and burst size. | |
| @@ -197,7 +257,7 @@ | |||
| 197 | 257 | ::governor::middleware::StateInformationMiddleware, | |
| 198 | 258 | >, | |
| 199 | 259 | > { | |
| 200 | - | std::sync::Arc::new( | |
| 260 | + | let config = std::sync::Arc::new( | |
| 201 | 261 | tower_governor::governor::GovernorConfigBuilder::default() | |
| 202 | 262 | .key_extractor(SyncAppKeyExtractor::new(secret)) | |
| 203 | 263 | .per_millisecond(ms) | |
| @@ -205,7 +265,13 @@ | |||
| 205 | 265 | .use_headers() | |
| 206 | 266 | .finish() | |
| 207 | 267 | .expect("synckit app rate limiter config"), | |
| 208 | - | ) | |
| 268 | + | ); | |
| 269 | + | let limiter = config.limiter().clone(); | |
| 270 | + | register_for_sweep(move || { | |
| 271 | + | limiter.retain_recent(); | |
| 272 | + | limiter.len() | |
| 273 | + | }); | |
| 274 | + | config | |
| 209 | 275 | } | |
| 210 | 276 | ||
| 211 | 277 | #[cfg(test)] |
| @@ -41,6 +41,11 @@ | |||
| 41 | 41 | Ok(row) | |
| 42 | 42 | } | |
| 43 | 43 | ||
| 44 | + | /// Safety cap on the media picker listing. Hitting it is logged at WARN so a | |
| 45 | + | /// creator with more than this many clean files in one folder isn't silently | |
| 46 | + | /// truncated (mirrors `versions::VERSIONS_LIST_HARD_CAP`). | |
| 47 | + | pub const MEDIA_LIST_HARD_CAP: i64 = 500; | |
| 48 | + | ||
| 44 | 49 | /// List media files for a user, optionally filtered by folder. | |
| 45 | 50 | #[tracing::instrument(skip_all)] | |
| 46 | 51 | pub async fn list_by_user_folder( | |
| @@ -50,21 +55,30 @@ | |||
| 50 | 55 | ) -> Result<Vec<DbMediaFile>> { | |
| 51 | 56 | let rows = if let Some(f) = folder { | |
| 52 | 57 | sqlx::query_as::<_, DbMediaFile>( | |
| 53 | - | "SELECT * FROM media_files WHERE user_id = $1 AND folder = $2 AND scan_status = 'clean' ORDER BY created_at DESC LIMIT 500", | |
| 58 | + | "SELECT * FROM media_files WHERE user_id = $1 AND folder = $2 AND scan_status = 'clean' ORDER BY created_at DESC LIMIT $3", | |
| 54 | 59 | ) | |
| 55 | 60 | .bind(user_id) | |
| 56 | 61 | .bind(f) | |
| 62 | + | .bind(MEDIA_LIST_HARD_CAP) | |
| 57 | 63 | .fetch_all(pool) | |
| 58 | 64 | .await? | |
| 59 | 65 | } else { | |
| 60 | 66 | sqlx::query_as::<_, DbMediaFile>( | |
| 61 | - | "SELECT * FROM media_files WHERE user_id = $1 AND scan_status = 'clean' ORDER BY created_at DESC LIMIT 500", | |
| 67 | + | "SELECT * FROM media_files WHERE user_id = $1 AND scan_status = 'clean' ORDER BY created_at DESC LIMIT $2", | |
| 62 | 68 | ) | |
| 63 | 69 | .bind(user_id) | |
| 70 | + | .bind(MEDIA_LIST_HARD_CAP) | |
| 64 | 71 | .fetch_all(pool) | |
| 65 | 72 | .await? | |
| 66 | 73 | }; | |
| 67 | 74 | ||
| 75 | + | if rows.len() as i64 == MEDIA_LIST_HARD_CAP { | |
| 76 | + | tracing::warn!( | |
| 77 | + | %user_id, cap = MEDIA_LIST_HARD_CAP, | |
| 78 | + | "list_by_user_folder hit hard cap; some media omitted from the picker" | |
| 79 | + | ); | |
| 80 | + | } | |
| 81 | + | ||
| 68 | 82 | Ok(rows) | |
| 69 | 83 | } | |
| 70 | 84 |
| @@ -42,6 +42,31 @@ | |||
| 42 | 42 | ||
| 43 | 43 | /// Insert a scan result record for audit trail. | |
| 44 | 44 | #[tracing::instrument(skip_all)] | |
| 45 | + | /// Remove every CDN-served image row referencing `s3_key`, across the three | |
| 46 | + | /// image tables that have no per-row scan gate: `item_images.s3_key`, | |
| 47 | + | /// `project_images.s3_key`, and `content_insertions.storage_key`. Returns the | |
| 48 | + | /// number of rows removed. | |
| 49 | + | /// | |
| 50 | + | /// On quarantine of these kinds, deleting the row IS the primary enforcement: it | |
| 51 | + | /// stops the app from ever rendering the (Cloudflare-served) URL again, and — | |
| 52 | + | /// critically — makes the key non-live so the durable S3-deletion queue will | |
| 53 | + | /// actually purge the object instead of parking it behind the `is_s3_key_live` | |
| 54 | + | /// guard. `storage_used` counters self-heal on the weekly | |
| 55 | + | /// `recalculate_all_storage_used` pass; we accept a transient over-count for a | |
| 56 | + | /// malicious upload rather than join through three ownership paths here. | |
| 57 | + | #[tracing::instrument(skip_all)] | |
| 58 | + | pub async fn purge_cdn_image_rows_by_key(db: &PgPool, s3_key: &str) -> Result<u64, sqlx::Error> { | |
| 59 | + | let mut removed = 0u64; | |
| 60 | + | for sql in [ | |
| 61 | + | "DELETE FROM item_images WHERE s3_key = $1", | |
| 62 | + | "DELETE FROM project_images WHERE s3_key = $1", | |
| 63 | + | "DELETE FROM content_insertions WHERE storage_key = $1", | |
| 64 | + | ] { | |
| 65 | + | removed += sqlx::query(sql).bind(s3_key).execute(db).await?.rows_affected(); | |
| 66 | + | } | |
| 67 | + | Ok(removed) | |
| 68 | + | } | |
| 69 | + | ||
| 45 | 70 | pub async fn insert_scan_result( | |
| 46 | 71 | db: &PgPool, | |
| 47 | 72 | s3_key: &str, |
| @@ -256,6 +256,14 @@ | |||
| 256 | 256 | Repository::open_bare(&canonical_repo).map_err(|_| GitError::RepoNotFound) | |
| 257 | 257 | } | |
| 258 | 258 | ||
| 259 | + | /// Open a bare repository at an already-resolved, validated path (e.g. one | |
| 260 | + | /// returned by [`repo_disk_path`]). Used to (re)open the repo inside a | |
| 261 | + | /// `spawn_blocking` closure, since `git2::Repository` is `!Send` and cannot | |
| 262 | + | /// cross the await boundary. | |
| 263 | + | pub fn open_repo_at(repo_path: &Path) -> Result<Repository, GitError> { | |
| 264 | + | Repository::open_bare(repo_path).map_err(|_| GitError::RepoNotFound) | |
| 265 | + | } | |
| 266 | + | ||
| 259 | 267 | /// Get basic repository info. | |
| 260 | 268 | pub fn repo_info(repo: &Repository, name: &str) -> RepoInfo { | |
| 261 | 269 | let description = std::fs::read_to_string(repo.path().join("description")) |
| @@ -25,12 +25,6 @@ | |||
| 25 | 25 | }) | |
| 26 | 26 | } | |
| 27 | 27 | ||
| 28 | - | fn parse_account_id_internal(account_id: &str) -> Result<stripe_shared::AccountId> { | |
| 29 | - | account_id.parse().map_err(|_| { | |
| 30 | - | AppError::Internal(anyhow::anyhow!("Invalid Stripe account ID")) | |
| 31 | - | }) | |
| 32 | - | } | |
| 33 | - | ||
| 34 | 28 | impl StripeClient { | |
| 35 | 29 | /// Create a Stripe Standard connected account for a creator. | |
| 36 | 30 | #[tracing::instrument(skip_all, name = "payments::create_connect_account")] | |
| @@ -155,7 +149,7 @@ | |||
| 155 | 149 | stripe_sub_id: &str, | |
| 156 | 150 | connected_account_id: &str, | |
| 157 | 151 | ) -> Result<()> { | |
| 158 | - | let acct = parse_account_id_internal(connected_account_id)?; | |
| 152 | + | let acct = Self::parse_account_id(connected_account_id)?; | |
| 159 | 153 | let sub_id = parse_subscription_id(stripe_sub_id)?; | |
| 160 | 154 | ||
| 161 | 155 | UpdateSubscription::new(sub_id) | |
| @@ -184,7 +178,7 @@ | |||
| 184 | 178 | stripe_sub_id: &str, | |
| 185 | 179 | connected_account_id: &str, | |
| 186 | 180 | ) -> Result<()> { | |
| 187 | - | let acct = parse_account_id_internal(connected_account_id)?; | |
| 181 | + | let acct = Self::parse_account_id(connected_account_id)?; | |
| 188 | 182 | let sub_id = parse_subscription_id(stripe_sub_id)?; | |
| 189 | 183 | ||
| 190 | 184 | ResumeSubscription::new(sub_id) | |
| @@ -207,7 +201,7 @@ | |||
| 207 | 201 | stripe_sub_id: &str, | |
| 208 | 202 | connected_account_id: &str, | |
| 209 | 203 | ) -> Result<()> { | |
| 210 | - | let acct = parse_account_id_internal(connected_account_id)?; | |
| 204 | + | let acct = Self::parse_account_id(connected_account_id)?; | |
| 211 | 205 | let sub_id = parse_subscription_id(stripe_sub_id)?; | |
| 212 | 206 | ||
| 213 | 207 | CancelSubscription::new(sub_id) | |
| @@ -264,7 +258,7 @@ | |||
| 264 | 258 | connected_account_id: &str, | |
| 265 | 259 | cancel: bool, | |
| 266 | 260 | ) -> Result<()> { | |
| 267 | - | let acct = parse_account_id_internal(connected_account_id)?; | |
| 261 | + | let acct = Self::parse_account_id(connected_account_id)?; | |
| 268 | 262 | let sub_id = parse_subscription_id(stripe_sub_id)?; | |
| 269 | 263 | UpdateSubscription::new(sub_id) | |
| 270 | 264 | .cancel_at_period_end(cancel) | |
| @@ -305,7 +299,7 @@ | |||
| 305 | 299 | payment_intent_id: &str, | |
| 306 | 300 | connected_account_id: &str, | |
| 307 | 301 | ) -> Result<()> { | |
| 308 | - | let acct = parse_account_id_internal(connected_account_id)?; | |
| 302 | + | let acct = Self::parse_account_id(connected_account_id)?; | |
| 309 | 303 | CreateRefund::new() | |
| 310 | 304 | .payment_intent(payment_intent_id.to_string()) | |
| 311 | 305 | .customize() | |
| @@ -319,3 +313,27 @@ | |||
| 319 | 313 | Ok(()) | |
| 320 | 314 | } | |
| 321 | 315 | } | |
| 316 | + | ||
| 317 | + | #[cfg(test)] | |
| 318 | + | mod tests { | |
| 319 | + | use super::*; | |
| 320 | + | ||
| 321 | + | // NOTE: async-stripe's `*Id` types are permissive newtypes — `FromStr` | |
| 322 | + | // accepts any non-pathological string without validating the `acct_`/`sub_` | |
| 323 | + | // prefix, so there is no error path to assert on normal input. These tests | |
| 324 | + | // pin what is actually observable: canonical IDs parse and round-trip, and | |
| 325 | + | // both account-id call sites now go through the single `parse_account_id` | |
| 326 | + | // (the divergent `parse_account_id_internal` was deleted in Run #14). | |
| 327 | + | ||
| 328 | + | #[test] | |
| 329 | + | fn account_id_parses_and_round_trips() { | |
| 330 | + | let acct = StripeClient::parse_account_id("acct_1A2b3C4d5E6f7G").unwrap(); | |
| 331 | + | assert_eq!(acct.to_string(), "acct_1A2b3C4d5E6f7G"); | |
| 332 | + | } | |
| 333 | + | ||
| 334 | + | #[test] | |
| 335 | + | fn subscription_id_parses_and_round_trips() { | |
| 336 | + | let sub = parse_subscription_id("sub_1A2b3C4d5E6f7G8h").unwrap(); | |
| 337 | + | assert_eq!(sub.to_string(), "sub_1A2b3C4d5E6f7G8h"); | |
| 338 | + | } | |
| 339 | + | } |
| @@ -48,9 +48,13 @@ | |||
| 48 | 48 | } | |
| 49 | 49 | ||
| 50 | 50 | /// Parse a connected account ID string into an `AccountId`. | |
| 51 | + | /// | |
| 52 | + | /// Account IDs are read from our own DB (`users.stripe_account_id`), so a | |
| 53 | + | /// parse failure is an internal invariant violation rather than bad user | |
| 54 | + | /// input — classify it `Internal` and keep the underlying error for ops. | |
| 51 | 55 | pub(crate) fn parse_account_id(account_id: &str) -> Result<stripe_shared::AccountId> { | |
| 52 | - | account_id.parse().map_err(|_| { | |
| 53 | - | AppError::BadRequest("Invalid Stripe account ID format".to_string()) | |
| 56 | + | account_id.parse().map_err(|e| { | |
| 57 | + | AppError::Internal(anyhow::anyhow!("Invalid Stripe account ID '{}': {}", account_id, e)) | |
| 54 | 58 | }) | |
| 55 | 59 | } | |
| 56 | 60 | } |