From e51ed38e43d4aae672a0da8e0358db58a50817b1 Mon Sep 17 00:00:00 2001 From: aecsocket Date: Sun, 10 May 2026 11:35:58 +0100 Subject: [PATCH 1/5] Analytics request loader and game version validation --- apps/labrinth/AGENTS.md | 11 +++- apps/labrinth/src/routes/internal/admin.rs | 77 +++++++++++++++++++++- 2 files changed, 86 insertions(+), 2 deletions(-) mode change 120000 => 100644 apps/labrinth/AGENTS.md diff --git a/apps/labrinth/AGENTS.md b/apps/labrinth/AGENTS.md deleted file mode 120000 index 681311eb9c..0000000000 --- a/apps/labrinth/AGENTS.md +++ /dev/null @@ -1 +0,0 @@ -CLAUDE.md \ No newline at end of file diff --git a/apps/labrinth/AGENTS.md b/apps/labrinth/AGENTS.md new file mode 100644 index 0000000000..efc4fe35a2 --- /dev/null +++ b/apps/labrinth/AGENTS.md @@ -0,0 +1,10 @@ +- Use `ApiError` as the error type for API routes +- Prefer `ApiError::Internal` and `ApiError::Request` over `ApiError::InvalidInput` + - Use `eyre!` to construct a value for `Internal` and `Request` variants +- Error messages (both for errors and exceptions) must be formatted as per the Rust API guidelines: + - lowercase message + - no trailing punctuation + - wrap code items e.g. type names in backticks +- Prefer `wrap_internal_err`, `wrap_request_err` when attaching context to an existing error (like Anyhow `context` or Eyre `wrap_err`) +- All operations should ideally have some context attached + - Database operations can have a message like `.wrap_internal_err("failed to fetch XYZ")` diff --git a/apps/labrinth/src/routes/internal/admin.rs b/apps/labrinth/src/routes/internal/admin.rs index 87d98b989f..0170bc3269 100644 --- a/apps/labrinth/src/routes/internal/admin.rs +++ b/apps/labrinth/src/routes/internal/admin.rs @@ -1,5 +1,7 @@ use crate::auth::validate::get_user_record_from_bearer_token; use crate::database::PgPool; +use crate::database::models::legacy_loader_fields::MinecraftGameVersion; +use crate::database::models::loader_fields::Loader; use crate::database::redis::RedisPool; use crate::models::analytics::{Download, DownloadReason}; use crate::models::ids::ProjectId; @@ -12,12 +14,65 @@ use crate::util::date::get_current_tenths_of_ms; use crate::util::error::Context; use crate::util::guards::admin_key_guard; use actix_web::{HttpRequest, HttpResponse, patch, post, web}; +use arc_swap::ArcSwapOption; +use eyre::eyre; use serde::Deserialize; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::net::Ipv4Addr; use std::sync::Arc; +use std::time::{Duration, Instant}; use tracing::trace; +const DOWNLOAD_TAGS_CACHE_TTL: Duration = Duration::from_secs(60 * 5); + +struct DownloadTagsCache { + expires: Instant, + loaders: HashSet, + game_versions: HashSet, +} + +static DOWNLOAD_TAGS_CACHE: ArcSwapOption = + ArcSwapOption::const_empty(); + +/// Fetches download tags from the database or returns a cached version. +/// +/// We cache tags since we get a large volume of download ingests, and querying +/// the database or even Redis for each request is too expensive. +async fn valid_download_tags( + pool: &PgPool, + redis: &RedisPool, +) -> Result, ApiError> { + let now = Instant::now(); + let cached = DOWNLOAD_TAGS_CACHE.load(); + if let Some(cached) = &*cached + && cached.expires > now + { + return Ok(cached.clone()); + } + + let loaders = Loader::list(pool, redis) + .await + .wrap_internal_err("failed to fetch loaders")? + .into_iter() + .map(|loader| loader.loader) + .collect(); + let game_versions = MinecraftGameVersion::list(None, None, pool, redis) + .await + .wrap_internal_err("failed to fetch game versions")? + .into_iter() + .map(|game_version| game_version.version) + .collect(); + + let cache = Arc::new(DownloadTagsCache { + expires: now + DOWNLOAD_TAGS_CACHE_TTL, + loaders, + game_versions, + }); + DOWNLOAD_TAGS_CACHE.store(Some(cache.clone())); + + Ok(cache) +} + pub fn config(cfg: &mut utoipa_actix_web::service_config::ServiceConfig) { cfg.service( utoipa_actix_web::scope("/admin") @@ -139,6 +194,26 @@ pub async fn count_download( None }; + if let Some(meta) = &meta { + let valid_download_tags = valid_download_tags(&pool, &redis) + .await + .wrap_internal_err("failed to fetch valid download tags")?; + if !valid_download_tags.loaders.contains(&meta.loader) { + return Err(ApiError::Request(eyre!( + "invalid download loader specified" + ))); + } + + if !valid_download_tags + .game_versions + .contains(&meta.game_version) + { + return Err(ApiError::Request(eyre!( + "invalid download game version specified" + ))); + } + } + let download = Download { recorded: get_current_tenths_of_ms(), domain: url.host_str().unwrap_or_default().to_string(), From eb710abec96536c76e7eeedee0e528ab532b7ff0 Mon Sep 17 00:00:00 2001 From: aecsocket Date: Sun, 10 May 2026 11:45:48 +0100 Subject: [PATCH 2/5] tweak agents --- apps/labrinth/AGENTS.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/apps/labrinth/AGENTS.md b/apps/labrinth/AGENTS.md index efc4fe35a2..2e6a6e36ff 100644 --- a/apps/labrinth/AGENTS.md +++ b/apps/labrinth/AGENTS.md @@ -8,3 +8,13 @@ - Prefer `wrap_internal_err`, `wrap_request_err` when attaching context to an existing error (like Anyhow `context` or Eyre `wrap_err`) - All operations should ideally have some context attached - Database operations can have a message like `.wrap_internal_err("failed to fetch XYZ")` +- You can perform real-time queries against the databases in the Docker Compose + - `docker exec labrinth-postgres psql -c "select 1"` + - `docker exec labrinth-redis redis-cli flushall` + - `docker exec labrinth-clickhouse clickhouse-client "select 1"` + - On some machines, you may have to use `podman` instead of `docker` - check which one is available first +- Hardcoded credentials for admin: + - `Authorization: Bearer mra_admin` for default admin user + - `Authorization: Bearer mra_user` for a regular user + - `Modrinth-Admin: feedbeef` as admin key +- If some steps require you to create a project/mod or version for testing, ask the user to go into the web frontend and manually create a project/version From b75e2a46fcf49b2ace8f8742d316b84944354ac5 Mon Sep 17 00:00:00 2001 From: aecsocket Date: Sun, 10 May 2026 11:51:16 +0100 Subject: [PATCH 3/5] factor tags into its own util --- apps/labrinth/src/routes/internal/admin.rs | 57 +------------------ apps/labrinth/src/util/mod.rs | 1 + apps/labrinth/src/util/tags.rs | 64 ++++++++++++++++++++++ 3 files changed, 67 insertions(+), 55 deletions(-) create mode 100644 apps/labrinth/src/util/tags.rs diff --git a/apps/labrinth/src/routes/internal/admin.rs b/apps/labrinth/src/routes/internal/admin.rs index 0170bc3269..c1fa524790 100644 --- a/apps/labrinth/src/routes/internal/admin.rs +++ b/apps/labrinth/src/routes/internal/admin.rs @@ -1,7 +1,5 @@ use crate::auth::validate::get_user_record_from_bearer_token; use crate::database::PgPool; -use crate::database::models::legacy_loader_fields::MinecraftGameVersion; -use crate::database::models::loader_fields::Loader; use crate::database::redis::RedisPool; use crate::models::analytics::{Download, DownloadReason}; use crate::models::ids::ProjectId; @@ -13,66 +11,15 @@ use crate::search::SearchBackend; use crate::util::date::get_current_tenths_of_ms; use crate::util::error::Context; use crate::util::guards::admin_key_guard; +use crate::util::tags::valid_download_tags; use actix_web::{HttpRequest, HttpResponse, patch, post, web}; -use arc_swap::ArcSwapOption; use eyre::eyre; use serde::Deserialize; -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; use std::net::Ipv4Addr; use std::sync::Arc; -use std::time::{Duration, Instant}; use tracing::trace; -const DOWNLOAD_TAGS_CACHE_TTL: Duration = Duration::from_secs(60 * 5); - -struct DownloadTagsCache { - expires: Instant, - loaders: HashSet, - game_versions: HashSet, -} - -static DOWNLOAD_TAGS_CACHE: ArcSwapOption = - ArcSwapOption::const_empty(); - -/// Fetches download tags from the database or returns a cached version. -/// -/// We cache tags since we get a large volume of download ingests, and querying -/// the database or even Redis for each request is too expensive. -async fn valid_download_tags( - pool: &PgPool, - redis: &RedisPool, -) -> Result, ApiError> { - let now = Instant::now(); - let cached = DOWNLOAD_TAGS_CACHE.load(); - if let Some(cached) = &*cached - && cached.expires > now - { - return Ok(cached.clone()); - } - - let loaders = Loader::list(pool, redis) - .await - .wrap_internal_err("failed to fetch loaders")? - .into_iter() - .map(|loader| loader.loader) - .collect(); - let game_versions = MinecraftGameVersion::list(None, None, pool, redis) - .await - .wrap_internal_err("failed to fetch game versions")? - .into_iter() - .map(|game_version| game_version.version) - .collect(); - - let cache = Arc::new(DownloadTagsCache { - expires: now + DOWNLOAD_TAGS_CACHE_TTL, - loaders, - game_versions, - }); - DOWNLOAD_TAGS_CACHE.store(Some(cache.clone())); - - Ok(cache) -} - pub fn config(cfg: &mut utoipa_actix_web::service_config::ServiceConfig) { cfg.service( utoipa_actix_web::scope("/admin") diff --git a/apps/labrinth/src/util/mod.rs b/apps/labrinth/src/util/mod.rs index 83fce22401..19688c7670 100644 --- a/apps/labrinth/src/util/mod.rs +++ b/apps/labrinth/src/util/mod.rs @@ -17,5 +17,6 @@ pub mod ratelimit; pub mod redis; pub mod routes; pub mod sentry; +pub mod tags; pub mod validate; pub mod webhook; diff --git a/apps/labrinth/src/util/tags.rs b/apps/labrinth/src/util/tags.rs new file mode 100644 index 0000000000..aaaa729b3a --- /dev/null +++ b/apps/labrinth/src/util/tags.rs @@ -0,0 +1,64 @@ +use crate::database::PgPool; +use crate::database::models::legacy_loader_fields::MinecraftGameVersion; +use crate::database::models::loader_fields::Loader; +use crate::database::redis::RedisPool; +use crate::routes::ApiError; +use crate::util::error::Context; +use arc_swap::ArcSwapOption; +use std::collections::HashSet; +use std::sync::Arc; +use std::time::{Duration, Instant}; + +/// Cached set of valid loaders and game version tags. +/// +/// Fetched using [`valid_download_tags`]. +#[derive(Debug)] +pub struct DownloadTagsCache { + expires: Instant, + pub loaders: HashSet, + pub game_versions: HashSet, +} + +/// Fetches download tags from the database or returns a cached version. +/// +/// We cache tags since we get a large volume of download ingests, and querying +/// the database or even Redis for each request is too expensive. +pub async fn valid_download_tags( + pool: &PgPool, + redis: &RedisPool, +) -> Result, ApiError> { + const DOWNLOAD_TAGS_CACHE_TTL: Duration = Duration::from_secs(60 * 5); + + static DOWNLOAD_TAGS_CACHE: ArcSwapOption = + ArcSwapOption::const_empty(); + + let now = Instant::now(); + let cached = DOWNLOAD_TAGS_CACHE.load(); + if let Some(cached) = &*cached + && cached.expires > now + { + return Ok(cached.clone()); + } + + let loaders = Loader::list(pool, redis) + .await + .wrap_internal_err("failed to fetch loaders")? + .into_iter() + .map(|loader| loader.loader) + .collect(); + let game_versions = MinecraftGameVersion::list(None, None, pool, redis) + .await + .wrap_internal_err("failed to fetch game versions")? + .into_iter() + .map(|game_version| game_version.version) + .collect(); + + let cache = Arc::new(DownloadTagsCache { + expires: now + DOWNLOAD_TAGS_CACHE_TTL, + loaders, + game_versions, + }); + DOWNLOAD_TAGS_CACHE.store(Some(cache.clone())); + + Ok(cache) +} From f50687695efda08c00c14c5713633be3bb3096c7 Mon Sep 17 00:00:00 2001 From: aecsocket Date: Mon, 11 May 2026 13:46:19 +0100 Subject: [PATCH 4/5] lock cache refresh to avoid cache stampede --- apps/labrinth/src/util/tags.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/apps/labrinth/src/util/tags.rs b/apps/labrinth/src/util/tags.rs index aaaa729b3a..98faeb2d51 100644 --- a/apps/labrinth/src/util/tags.rs +++ b/apps/labrinth/src/util/tags.rs @@ -8,6 +8,7 @@ use arc_swap::ArcSwapOption; use std::collections::HashSet; use std::sync::Arc; use std::time::{Duration, Instant}; +use tokio::sync::Mutex; /// Cached set of valid loaders and game version tags. /// @@ -31,6 +32,17 @@ pub async fn valid_download_tags( static DOWNLOAD_TAGS_CACHE: ArcSwapOption = ArcSwapOption::const_empty(); + static DOWNLOAD_TAGS_CACHE_REFRESH_LOCK: Mutex<()> = Mutex::const_new(()); + + let now = Instant::now(); + let cached = DOWNLOAD_TAGS_CACHE.load(); + if let Some(cached) = &*cached + && cached.expires > now + { + return Ok(cached.clone()); + } + + let _refresh_lock = DOWNLOAD_TAGS_CACHE_REFRESH_LOCK.lock().await; let now = Instant::now(); let cached = DOWNLOAD_TAGS_CACHE.load(); From a1849b4ef36f77d103439d9da64ad5f091ea845a Mon Sep 17 00:00:00 2001 From: aecsocket Date: Mon, 11 May 2026 13:59:32 +0100 Subject: [PATCH 5/5] Make analytics fields opptional --- apps/labrinth/src/routes/internal/admin.rs | 27 ++++++++++++++-------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/apps/labrinth/src/routes/internal/admin.rs b/apps/labrinth/src/routes/internal/admin.rs index c1fa524790..667e16ac4e 100644 --- a/apps/labrinth/src/routes/internal/admin.rs +++ b/apps/labrinth/src/routes/internal/admin.rs @@ -42,9 +42,9 @@ pub struct DownloadBody { /// [`DOWNLOAD_META_HEADER`] header. #[derive(Debug, Clone, Deserialize)] pub struct DownloadMeta { - pub reason: DownloadReason, - pub game_version: String, - pub loader: String, + pub reason: Option, + pub game_version: Option, + pub loader: Option, } pub const DOWNLOAD_META_HEADER: &str = "modrinth-download-meta"; @@ -145,15 +145,16 @@ pub async fn count_download( let valid_download_tags = valid_download_tags(&pool, &redis) .await .wrap_internal_err("failed to fetch valid download tags")?; - if !valid_download_tags.loaders.contains(&meta.loader) { + if let Some(loader) = &meta.loader + && !valid_download_tags.loaders.contains(loader) + { return Err(ApiError::Request(eyre!( "invalid download loader specified" ))); } - if !valid_download_tags - .game_versions - .contains(&meta.game_version) + if let Some(game_version) = &meta.game_version + && !valid_download_tags.game_versions.contains(game_version) { return Err(ApiError::Request(eyre!( "invalid download game version specified" @@ -198,13 +199,19 @@ pub async fn count_download( .collect(), reason: meta .as_ref() - .map(|m| m.reason.to_string()) + .and_then(|m| m.reason.as_ref()) + .map(|s| s.to_string()) .unwrap_or_default(), game_version: meta .as_ref() - .map(|m| m.game_version.clone()) + .and_then(|m| m.game_version.as_ref()) + .map(|s| s.to_string()) + .unwrap_or_default(), + loader: meta + .as_ref() + .and_then(|m| m.loader.as_ref()) + .map(|s| s.to_string()) .unwrap_or_default(), - loader: meta.as_ref().map(|m| m.loader.clone()).unwrap_or_default(), }; trace!("added download {download:#?}");