From d33f00d2b108942a60017cb637ddfbeef4575ef7 Mon Sep 17 00:00:00 2001 From: aecsocket <43144841+aecsocket@users.noreply.github.com> Date: Fri, 19 Jun 2026 14:52:24 +0100 Subject: [PATCH] Tighten URL slug validation (#6442) * Tighten URL slug validation * slug sanitization in frontend --- .../ui/create/OrganizationCreateModal.vue | 11 +-- .../ui/create/ProjectCreateModal.vue | 9 +-- apps/frontend/src/utils/slugs.ts | 10 +++ apps/labrinth/src/routes/v2/users.rs | 4 +- .../src/routes/v2/version_creation.rs | 2 +- apps/labrinth/src/routes/v2/versions.rs | 2 +- .../src/routes/v3/project_creation/new.rs | 57 ++++++++++++++ apps/labrinth/src/routes/v3/users.rs | 2 +- .../src/routes/v3/version_creation.rs | 2 +- apps/labrinth/src/routes/v3/versions.rs | 2 +- apps/labrinth/src/util/validate.rs | 75 ++++++++++++++++++- 11 files changed, 153 insertions(+), 23 deletions(-) create mode 100644 apps/frontend/src/utils/slugs.ts diff --git a/apps/frontend/src/components/ui/create/OrganizationCreateModal.vue b/apps/frontend/src/components/ui/create/OrganizationCreateModal.vue index 2d209bb19..687e2c236 100644 --- a/apps/frontend/src/components/ui/create/OrganizationCreateModal.vue +++ b/apps/frontend/src/components/ui/create/OrganizationCreateModal.vue @@ -89,6 +89,8 @@ import { } from '@modrinth/ui' import { ref } from 'vue' +import { generateUrlSlug } from '~/utils/slugs' + import CreateLimitAlert from './CreateLimitAlert.vue' const router = useNativeRouter() @@ -148,7 +150,7 @@ async function createOrganization(): Promise { const value = { name: name.value.trim(), description: description.value.trim(), - slug: slug.value.trim().replace(/ +/g, ''), + slug: slug.value.trim(), } const result: any = await useBaseFetch('organization', { @@ -183,12 +185,7 @@ function hide(): void { function updateSlug(): void { if (!manualSlug.value) { - slug.value = name.value - .trim() - .toLowerCase() - .replaceAll(' ', '-') - .replaceAll(/[^a-zA-Z0-9!@$()`.+,_"-]/g, '') - .replaceAll(/--+/gm, '-') + slug.value = generateUrlSlug(name.value) } } diff --git a/apps/frontend/src/components/ui/create/ProjectCreateModal.vue b/apps/frontend/src/components/ui/create/ProjectCreateModal.vue index 01ad8a1ba..e1fae0797 100644 --- a/apps/frontend/src/components/ui/create/ProjectCreateModal.vue +++ b/apps/frontend/src/components/ui/create/ProjectCreateModal.vue @@ -145,6 +145,8 @@ import { } from '@modrinth/ui' import { computed, defineAsyncComponent, h } from 'vue' +import { generateUrlSlug } from '~/utils/slugs' + import CreateLimitAlert from './CreateLimitAlert.vue' type ProjectTypes = 'server' | 'project' @@ -461,12 +463,7 @@ async function show(event?: MouseEvent, options?: ShowOptions) { function updatedName() { if (!manualSlug.value) { - slug.value = name.value - .trim() - .toLowerCase() - .replaceAll(' ', '-') - .replaceAll(/[^a-zA-Z0-9!@$()`.+,_"-]/g, '') - .replaceAll(/--+/gm, '-') + slug.value = generateUrlSlug(name.value) } } diff --git a/apps/frontend/src/utils/slugs.ts b/apps/frontend/src/utils/slugs.ts new file mode 100644 index 000000000..3f123d1fe --- /dev/null +++ b/apps/frontend/src/utils/slugs.ts @@ -0,0 +1,10 @@ +const PROJECT_SLUG_UNSAFE_CHARS = /[^a-zA-Z0-9._-]/g + +export function generateUrlSlug(value: string) { + return value + .trim() + .toLowerCase() + .replaceAll(' ', '-') + .replaceAll(PROJECT_SLUG_UNSAFE_CHARS, '') + .replaceAll(/--+/gm, '-') +} diff --git a/apps/labrinth/src/routes/v2/users.rs b/apps/labrinth/src/routes/v2/users.rs index 79f5bc9e6..3d965598b 100644 --- a/apps/labrinth/src/routes/v2/users.rs +++ b/apps/labrinth/src/routes/v2/users.rs @@ -184,14 +184,14 @@ pub async fn projects_list( #[derive(Serialize, Deserialize, Validate, utoipa::ToSchema)] pub struct EditUser { - #[validate(length(min = 1, max = 39), regex(path = *crate::util::validate::RE_USERNAME))] + #[validate(length(min = 1, max = 39), regex(path = *crate::util::validate::RE_URL_SAFE))] pub username: Option, #[serde( default, skip_serializing_if = "Option::is_none", with = "::serde_with::rust::double_option" )] - #[validate(length(min = 1, max = 64), regex(path = *crate::util::validate::RE_USERNAME))] + #[validate(length(min = 1, max = 64), regex(path = *crate::util::validate::RE_URL_SAFE))] pub name: Option>, #[serde( default, diff --git a/apps/labrinth/src/routes/v2/version_creation.rs b/apps/labrinth/src/routes/v2/version_creation.rs index 3a21183ba..b3e0337f7 100644 --- a/apps/labrinth/src/routes/v2/version_creation.rs +++ b/apps/labrinth/src/routes/v2/version_creation.rs @@ -36,7 +36,7 @@ pub struct InitialVersionData { pub file_parts: Vec, #[validate( length(min = 1, max = 32), - regex(path = *crate::util::validate::RE_URL_SAFE) + regex(path = *crate::util::validate::RE_URL_SAFE_RELAXED) )] pub version_number: String, #[validate( diff --git a/apps/labrinth/src/routes/v2/versions.rs b/apps/labrinth/src/routes/v2/versions.rs index c7f2ffc3a..438c33175 100644 --- a/apps/labrinth/src/routes/v2/versions.rs +++ b/apps/labrinth/src/routes/v2/versions.rs @@ -314,7 +314,7 @@ pub struct EditVersion { pub name: Option, #[validate( length(min = 1, max = 32), - regex(path = *crate::util::validate::RE_URL_SAFE) + regex(path = *crate::util::validate::RE_URL_SAFE_RELAXED) )] pub version_number: Option, #[validate(length(max = 65536))] diff --git a/apps/labrinth/src/routes/v3/project_creation/new.rs b/apps/labrinth/src/routes/v3/project_creation/new.rs index cbfd33971..d996a25a4 100644 --- a/apps/labrinth/src/routes/v3/project_creation/new.rs +++ b/apps/labrinth/src/routes/v3/project_creation/new.rs @@ -101,6 +101,7 @@ impl ResponseError for CreateError { #[derive(Debug, Clone, Serialize, Deserialize, Validate, utoipa::ToSchema)] pub struct ProjectCreate { + #[validate(nested)] pub base: exp::base::Project, #[serde(flatten)] #[validate(nested)] @@ -338,3 +339,59 @@ pub async fn create( Ok(web::Json(project_id)) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::models::projects::ProjectStatus; + + fn project_create_with_slug(slug: &str) -> ProjectCreate { + ProjectCreate { + base: exp::base::Project { + name: "test project".into(), + slug: slug.into(), + summary: "test summary".into(), + description: String::new(), + requested_status: ProjectStatus::Approved, + organization_id: None, + }, + components: exp::ProjectEdit { + minecraft_mod: None, + minecraft_server: None, + minecraft_java_server: None, + minecraft_bedrock_server: None, + }, + } + } + + fn assert_project_slug_validation(slug: &str, expected_valid: bool) { + let result = project_create_with_slug(slug).validate(); + + assert_eq!( + result.is_ok(), + expected_valid, + "unexpected validation result for slug `{slug}`" + ); + } + + #[test] + fn project_create_accepts_url_safe_base_slugs() { + for slug in ["valid-slug", "valid_slug", "valid.slug", "valid123"] { + assert_project_slug_validation(slug, true); + } + } + + #[test] + fn project_create_rejects_unsafe_base_slugs() { + for slug in [ + "invalid/slug", + "../invalid", + r#"invalid"slug"#, + "invalid$slug", + "invalid slug", + "invalid#slug", + ] { + assert_project_slug_validation(slug, false); + } + } +} diff --git a/apps/labrinth/src/routes/v3/users.rs b/apps/labrinth/src/routes/v3/users.rs index b33457e7d..1d517126b 100644 --- a/apps/labrinth/src/routes/v3/users.rs +++ b/apps/labrinth/src/routes/v3/users.rs @@ -642,7 +642,7 @@ pub async fn orgs_list( #[derive(Serialize, Deserialize, Validate)] pub struct EditUser { - #[validate(length(min = 1, max = 39), regex(path = *crate::util::validate::RE_USERNAME))] + #[validate(length(min = 1, max = 39), regex(path = *crate::util::validate::RE_URL_SAFE))] pub username: Option, #[serde( default, diff --git a/apps/labrinth/src/routes/v3/version_creation.rs b/apps/labrinth/src/routes/v3/version_creation.rs index 58d007ff6..26e335639 100644 --- a/apps/labrinth/src/routes/v3/version_creation.rs +++ b/apps/labrinth/src/routes/v3/version_creation.rs @@ -56,7 +56,7 @@ pub struct InitialVersionData { pub file_parts: Vec, #[validate( length(min = 1, max = 32), - regex(path = *crate::util::validate::RE_URL_SAFE) + regex(path = *crate::util::validate::RE_URL_SAFE_RELAXED) )] pub version_number: String, #[validate( diff --git a/apps/labrinth/src/routes/v3/versions.rs b/apps/labrinth/src/routes/v3/versions.rs index 4039be60f..5011d346a 100644 --- a/apps/labrinth/src/routes/v3/versions.rs +++ b/apps/labrinth/src/routes/v3/versions.rs @@ -222,7 +222,7 @@ pub struct EditVersion { pub name: Option, #[validate( length(min = 1, max = 32), - regex(path = *crate::util::validate::RE_URL_SAFE) + regex(path = *crate::util::validate::RE_URL_SAFE_RELAXED) )] pub version_number: Option, #[validate(length(max = 65536))] diff --git a/apps/labrinth/src/util/validate.rs b/apps/labrinth/src/util/validate.rs index 06af10317..0d4c796bd 100644 --- a/apps/labrinth/src/util/validate.rs +++ b/apps/labrinth/src/util/validate.rs @@ -7,9 +7,12 @@ use validator::{ValidationErrors, ValidationErrorsKind}; use crate::models::pats::Scopes; pub static RE_URL_SAFE: LazyLock = - LazyLock::new(|| Regex::new(r#"^[a-zA-Z0-9!@$()`.+,_"-]*$"#).unwrap()); -pub static RE_USERNAME: LazyLock = - LazyLock::new(|| Regex::new(r#"^[a-zA-Z0-9_-]*$"#).unwrap()); + LazyLock::new(|| Regex::new(r#"^[a-zA-Z0-9._-]+$"#).unwrap()); + +// only used for versions +// TODO: percent-encode version names in URLs instead of treating them as slugs +pub static RE_URL_SAFE_RELAXED: LazyLock = + LazyLock::new(|| Regex::new(r#"^[a-zA-Z0-9!@$()`.+,_"-]+$"#).unwrap()); //TODO: In order to ensure readability, only the first error is printed, this may need to be expanded on in the future! pub fn validation_errors_to_string( @@ -159,4 +162,70 @@ mod tests { let result = validate_name(" "); assert!(result.is_err()); } + + fn assert_url_safe_regex( + regex: &LazyLock, + value: &str, + expected_valid: bool, + ) { + assert_eq!( + regex.is_match(value), + expected_valid, + "unexpected URL-safe validation result for `{value}`" + ); + } + + fn assert_url_safe_slug(slug: &str, expected_valid: bool) { + assert_url_safe_regex(&RE_URL_SAFE, slug, expected_valid); + } + + fn assert_url_safe_version(version: &str, expected_valid: bool) { + assert_url_safe_regex(&RE_URL_SAFE_RELAXED, version, expected_valid); + } + + #[test] + fn url_safe_regex_accepts_allowed_slug_punctuation() { + for slug in ["valid-slug", "valid_slug", "valid.slug", "valid123"] { + assert_url_safe_slug(slug, true); + } + } + + #[test] + fn url_safe_regex_rejects_unsafe_slug_punctuation() { + for slug in [ + "invalid/slug", + "../invalid", + r#"invalid"slug"#, + "invalid$slug", + "invalid slug", + "invalid#slug", + ] { + assert_url_safe_slug(slug, false); + } + } + + #[test] + fn url_safe_relaxed_regex_accepts_legacy_version_punctuation() { + for version in [ + "1.0.0", + "1.0.0+build", + "version$beta", + r#"version"quoted"#, + "version!@$()`.+,_-", + ] { + assert_url_safe_version(version, true); + } + } + + #[test] + fn url_safe_relaxed_regex_rejects_non_version_safe_punctuation() { + for version in [ + "invalid/version", + "../invalid", + "invalid space", + "invalid#version", + ] { + assert_url_safe_version(version, false); + } + } }