From 8894231b41c318ef433f35d83d5e07df9f41fd3b Mon Sep 17 00:00:00 2001 From: Rhys Lloyd Date: Wed, 7 Jan 2026 10:43:19 -0800 Subject: [PATCH 1/3] backend: plumb target info into checks --- pkg/model/nats.go | 9 ++++++--- pkg/service/nats_mapfix.go | 12 +++++++++--- pkg/web_api/mapfixes.go | 30 ++++++++++++++++++++++++------ validation/src/nats_types.rs | 4 ++++ 4 files changed, 43 insertions(+), 12 deletions(-) diff --git a/pkg/model/nats.go b/pkg/model/nats.go index 081ba13..39725b4 100644 --- a/pkg/model/nats.go +++ b/pkg/model/nats.go @@ -31,9 +31,12 @@ type CheckSubmissionRequest struct{ } type CheckMapfixRequest struct{ - MapfixID int64 - ModelID uint64 - SkipChecks bool + MapfixID int64 + ModelID uint64 + SkipChecks bool + DisplayName string + Creator string + GameID uint32 } type ValidateSubmissionRequest struct { diff --git a/pkg/service/nats_mapfix.go b/pkg/service/nats_mapfix.go index 62989c2..47bf3f7 100644 --- a/pkg/service/nats_mapfix.go +++ b/pkg/service/nats_mapfix.go @@ -33,14 +33,20 @@ func (svc *Service) NatsCreateMapfix( } func (svc *Service) NatsCheckMapfix( - MapfixID int64, - ModelID uint64, - SkipChecks bool, + MapfixID int64, + ModelID uint64, + SkipChecks bool, + DisplayName string, + Creator string, + GameID uint32, ) error { validate_request := model.CheckMapfixRequest{ MapfixID: MapfixID, ModelID: ModelID, SkipChecks: SkipChecks, + DisplayName: DisplayName, + Creator: Creator, + GameID: GameID, } j, err := json.Marshal(validate_request) diff --git a/pkg/web_api/mapfixes.go b/pkg/web_api/mapfixes.go index c3a659e..d93e7d3 100644 --- a/pkg/web_api/mapfixes.go +++ b/pkg/web_api/mapfixes.go @@ -538,13 +538,13 @@ func (svc *Service) ActionMapfixTriggerSubmit(ctx context.Context, params api.Ac return ErrUserInfo } - // read mapfix (this could be done with a transaction WHERE clause) - mapfix, err := svc.inner.GetMapfix(ctx, params.MapfixID) + userId, err := userInfo.GetUserID() if err != nil { return err } - userId, err := userInfo.GetUserID() + // read mapfix (this could be done with a transaction WHERE clause) + mapfix, err := svc.inner.GetMapfix(ctx, params.MapfixID) if err != nil { return err } @@ -555,6 +555,12 @@ func (svc *Service) ActionMapfixTriggerSubmit(ctx context.Context, params api.Ac return ErrPermissionDeniedNotSubmitter } + // read map to get current DisplayName and such + target_map, err := svc.inner.GetMap(ctx, int64(mapfix.TargetAssetID)) + if err != nil { + return err + } + // transaction target_status := model.MapfixStatusSubmitting update := service.NewMapfixUpdate() @@ -569,6 +575,9 @@ func (svc *Service) ActionMapfixTriggerSubmit(ctx context.Context, params api.Ac mapfix.ID, mapfix.AssetID, false, + target_map.DisplayName, + target_map.Creator, + target_map.GameID, ) if err != nil { return err @@ -600,13 +609,13 @@ func (svc *Service) ActionMapfixTriggerSubmitUnchecked(ctx context.Context, para return ErrUserInfo } - // read mapfix (this could be done with a transaction WHERE clause) - mapfix, err := svc.inner.GetMapfix(ctx, params.MapfixID) + userId, err := userInfo.GetUserID() if err != nil { return err } - userId, err := userInfo.GetUserID() + // read mapfix (this could be done with a transaction WHERE clause) + mapfix, err := svc.inner.GetMapfix(ctx, params.MapfixID) if err != nil { return err } @@ -626,6 +635,12 @@ func (svc *Service) ActionMapfixTriggerSubmitUnchecked(ctx context.Context, para return ErrPermissionDeniedNeedRoleMapfixReview } + // read map to get current DisplayName and such + target_map, err := svc.inner.GetMap(ctx, int64(mapfix.TargetAssetID)) + if err != nil { + return err + } + // transaction target_status := model.MapfixStatusSubmitting update := service.NewMapfixUpdate() @@ -640,6 +655,9 @@ func (svc *Service) ActionMapfixTriggerSubmitUnchecked(ctx context.Context, para mapfix.ID, mapfix.AssetID, true, + target_map.DisplayName, + target_map.Creator, + target_map.GameID, ) if err != nil { return err diff --git a/validation/src/nats_types.rs b/validation/src/nats_types.rs index f418759..dfb0b91 100644 --- a/validation/src/nats_types.rs +++ b/validation/src/nats_types.rs @@ -41,6 +41,10 @@ pub struct CheckMapfixRequest{ pub MapfixID:u64, pub ModelID:u64, pub SkipChecks:bool, + // target map info + pub DisplayName:String, + pub Creator:String, + pub GameID:u32, } #[expect(nonstandard_style)] -- 2.49.1 From 9e47ca51779feed2f559d5d4075af725a819247f Mon Sep 17 00:00:00 2001 From: Rhys Lloyd Date: Wed, 7 Jan 2026 11:28:27 -0800 Subject: [PATCH 2/3] validation: generalize StringEquality to EqualityCheck --- validation/src/check.rs | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/validation/src/check.rs b/validation/src/check.rs index afcc810..8c6ed6c 100644 --- a/validation/src/check.rs +++ b/validation/src/check.rs @@ -323,14 +323,14 @@ pub fn get_model_info<'a>(dom:&'a rbx_dom_weak::WeakDom,model_instance:&'a rbx_d } } -// check if an observed string matches an expected string -pub struct StringEquality<'a,Str>{ - observed:&'a str, - expected:Str, +// check if an observed value matches an expected value +pub struct EqualityCheck{ + observed:Obs, + expected:Exp, } -impl<'a,Str> StringEquality<'a,Str> +impl EqualityCheck where - &'a str:PartialEq, + Obs:PartialEq, { /// Compute the StringCheck, passing through the provided value on success. fn check(self,value:T)->Result{ @@ -341,7 +341,7 @@ impl<'a,Str> StringEquality<'a,Str> } } } -impl std::fmt::Display for StringEquality<'_,Str>{ +impl std::fmt::Display for EqualityCheck{ fn fmt(&self,f:&mut std::fmt::Formatter<'_>)->std::fmt::Result{ write!(f,"expected: {}, observed: {}",self.expected,self.observed) } @@ -464,7 +464,7 @@ struct Exists; struct Absent; enum DisplayNameError<'a>{ - TitleCase(StringEquality<'a,String>), + TitleCase(EqualityCheck<&'a str,String>), Empty(StringEmpty), TooLong(usize), StringValue(StringValueError), @@ -482,7 +482,7 @@ fn check_display_name<'a>(display_name:Result<&'a str,StringValueError>)->Result } // Check title case - let display_name=StringEquality{ + let display_name=EqualityCheck{ observed:display_name, expected:display_name.to_title_case(), }.check(display_name).map_err(DisplayNameError::TitleCase)?; @@ -514,9 +514,9 @@ fn check_creator<'a>(creator:Result<&'a str,StringValueError>)->Result<&'a str,C struct MapCheck<'a>{ // === METADATA CHECKS === // The root must be of class Model - model_class:Result<(),StringEquality<'a,&'static str>>, + model_class:Result<(),EqualityCheck<&'a str,&'static str>>, // Model's name must be in snake case - model_name:Result<(),StringEquality<'a,String>>, + model_name:Result<(),EqualityCheck<&'a str,String>>, // Map must have a StringValue named DisplayName. // Value must not be empty, must be in title case. display_name:Result<&'a str,DisplayNameError<'a>>, @@ -557,13 +557,13 @@ struct MapCheck<'a>{ impl<'a> ModelInfo<'a>{ fn check(self)->MapCheck<'a>{ // Check class is exactly "Model" - let model_class=StringEquality{ + let model_class=EqualityCheck{ observed:self.model_class, expected:"Model", }.check(()); // Check model name is snake case - let model_name=StringEquality{ + let model_name=EqualityCheck{ observed:self.model_name, expected:self.model_name.to_snake_case(), }.check(()); -- 2.49.1 From 474655f4a3c800ddb29581dd3fad7af3c814aae2 Mon Sep 17 00:00:00 2001 From: Rhys Lloyd Date: Wed, 7 Jan 2026 11:25:08 -0800 Subject: [PATCH 3/3] validation: Check that mapfixes do not change DisplayName, Creator, GameID --- Cargo.lock | 1 + validation/Cargo.toml | 1 + validation/src/check.rs | 253 +++++++++++++++++++++-------- validation/src/check_mapfix.rs | 2 +- validation/src/check_submission.rs | 2 +- validation/src/nats_types.rs | 2 +- validation/src/rbx_util.rs | 12 ++ 7 files changed, 203 insertions(+), 70 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a9246fa..596d32b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1044,6 +1044,7 @@ dependencies = [ "rust-grpc", "serde", "serde_json", + "serde_repr", "siphasher", "tokio", "tonic", diff --git a/validation/Cargo.toml b/validation/Cargo.toml index 8aa88d2..317af72 100644 --- a/validation/Cargo.toml +++ b/validation/Cargo.toml @@ -14,6 +14,7 @@ rbx_xml = "2.0.0" regex = { version = "1.11.3", default-features = false } serde = { version = "1.0.215", features = ["derive"] } serde_json = "1.0.133" +serde_repr = "0.1.19" siphasher = "1.0.1" tokio = { version = "1.41.1", features = ["macros", "rt-multi-thread", "signal"] } heck = "0.5.0" diff --git a/validation/src/check.rs b/validation/src/check.rs index 8c6ed6c..9ba4a23 100644 --- a/validation/src/check.rs +++ b/validation/src/check.rs @@ -33,29 +33,6 @@ macro_rules! lazy_regex{ }}; } -#[expect(nonstandard_style)] -pub struct CheckRequest{ - ModelID:u64, - SkipChecks:bool, -} - -impl From for CheckRequest{ - fn from(value:crate::nats_types::CheckMapfixRequest)->Self{ - Self{ - ModelID:value.ModelID, - SkipChecks:value.SkipChecks, - } - } -} -impl From for CheckRequest{ - fn from(value:crate::nats_types::CheckSubmissionRequest)->Self{ - Self{ - ModelID:value.ModelID, - SkipChecks:value.SkipChecks, - } - } -} - #[derive(Clone,Copy,Debug,Hash,Eq,PartialEq,Ord,PartialOrd)] struct ModeID(u64); impl ModeID{ @@ -465,49 +442,144 @@ struct Absent; enum DisplayNameError<'a>{ TitleCase(EqualityCheck<&'a str,String>), + CannotChange(EqualityCheck<&'a str,String>), Empty(StringEmpty), TooLong(usize), StringValue(StringValueError), } -fn check_display_name<'a>(display_name:Result<&'a str,StringValueError>)->Result<&'a str,DisplayNameError<'a>>{ - // DisplayName StringValue can be missing or whatever - let display_name=display_name.map_err(DisplayNameError::StringValue)?; - // DisplayName cannot be "" - let display_name=check_empty(display_name).map_err(DisplayNameError::Empty)?; - - // DisplayName cannot exceed 50 characters - if 50{ + CannotChange(EqualityCheck<&'a str,String>), Empty(StringEmpty), TooLong(usize), StringValue(StringValueError), } -fn check_creator<'a>(creator:Result<&'a str,StringValueError>)->Result<&'a str,CreatorError>{ - // Creator StringValue can be missing or whatever - let creator=creator.map_err(CreatorError::StringValue)?; - // Creator cannot be "" - let creator=check_empty(creator).map_err(CreatorError::Empty)?; +enum GameIDError{ + CannotChange(EqualityCheck), + Parse(ParseGameIDError), +} - // Creator cannot exceed 50 characters - if 50{ + display_name:Result<&'a str,DisplayNameError<'a>>, + creator:Result<&'a str,CreatorError<'a>>, + game_id:Result, +} + +pub struct NoMapInfo; +impl CheckModelInfo for NoMapInfo{ + fn check<'a>(self,map_info:MapInfo<'a>)->CheckedMapInfo<'a>{ + fn check_display_name(display_name:Result<&str,StringValueError>)->Result<&str,DisplayNameError<'_>>{ + // DisplayName StringValue can be missing or whatever + let display_name=display_name.map_err(DisplayNameError::StringValue)?; + + // DisplayName cannot be "" + let display_name=check_empty(display_name).map_err(DisplayNameError::Empty)?; + + // DisplayName cannot exceed 50 characters + if 50)->Result<&str,CreatorError<'_>>{ + // Creator StringValue can be missing or whatever + let creator=creator.map_err(CreatorError::StringValue)?; + + // Creator cannot be "" + let creator=check_empty(creator).map_err(CreatorError::Empty)?; + + // Creator cannot exceed 50 characters + if 50)->Result{ + // Creator StringValue can be missing or whatever + let game_id=game_id.map_err(GameIDError::Parse)?; + + Ok(game_id) + } + + // Check display name is not empty and has title case + let display_name=check_display_name(map_info.display_name); + + // Check Creator is not empty + let creator=check_creator(map_info.creator); + + // Check GameID (model name was prefixed with bhop_ surf_ etc) + let game_id=check_game_id(map_info.game_id); + + CheckedMapInfo{ + display_name, + creator, + game_id, + } } +} +impl CheckModelInfo for MapInfoOwned{ + fn check<'a>(self,map_info:MapInfo<'a>)->CheckedMapInfo<'a>{ + fn check_display_name(display_name:Result<&str,StringValueError>,target_display_name:String)->Result<&str,DisplayNameError<'_>>{ + // DisplayName StringValue can be missing or whatever + let display_name=display_name.map_err(DisplayNameError::StringValue)?; - Ok(creator) + // Mapfix cannot change display name + let display_name=EqualityCheck{ + observed:display_name, + expected:target_display_name, + }.check(display_name).map_err(DisplayNameError::CannotChange)?; + + Ok(display_name) + } + fn check_creator(creator:Result<&str,StringValueError>,target_creator:String)->Result<&str,CreatorError<'_>>{ + // Creator StringValue can be missing or whatever + let creator=creator.map_err(CreatorError::StringValue)?; + + // Mapfix cannot change creator + let creator=EqualityCheck{ + observed:creator, + expected:target_creator, + }.check(creator).map_err(CreatorError::CannotChange)?; + + Ok(creator) + } + fn check_game_id(game_id:Result,target_game_id:GameID)->Result{ + // Creator StringValue can be missing or whatever + let game_id=game_id.map_err(GameIDError::Parse)?; + + // Mapfix cannot change game_id + let game_id=EqualityCheck{ + observed:game_id, + expected:target_game_id, + }.check(game_id).map_err(GameIDError::CannotChange)?; + + Ok(game_id) + } + + // Check display name is not empty and has title case + let display_name=check_display_name(map_info.display_name,self.display_name); + + // Check Creator is not empty + let creator=check_creator(map_info.creator,self.creator); + + // Check GameID (model name was prefixed with bhop_ surf_ etc) + let game_id=check_game_id(map_info.game_id,self.game_id); + + CheckedMapInfo{ + display_name, + creator, + game_id, + } + } } /// The result of every map check. @@ -522,10 +594,10 @@ struct MapCheck<'a>{ display_name:Result<&'a str,DisplayNameError<'a>>, // Map must have a StringValue named Creator. // Value must not be empty. - creator:Result<&'a str,CreatorError>, + creator:Result<&'a str,CreatorError<'a>>, // The prefix of the model's name must match the game it was submitted for. // bhop_ for bhop, and surf_ for surf - game_id:Result, + game_id:Result, // === MODE CHECKS === // MapStart must exist @@ -555,7 +627,7 @@ struct MapCheck<'a>{ } impl<'a> ModelInfo<'a>{ - fn check(self)->MapCheck<'a>{ + fn check(self,model_info:I)->MapCheck<'a>{ // Check class is exactly "Model" let model_class=EqualityCheck{ observed:self.model_class, @@ -568,14 +640,11 @@ impl<'a> ModelInfo<'a>{ expected:self.model_name.to_snake_case(), }.check(()); - // Check display name is not empty and has title case - let display_name=check_display_name(self.map_info.display_name); - - // Check Creator is not empty - let creator=check_creator(self.map_info.creator); - - // Check GameID (model name was prefixed with bhop_ surf_ etc) - let game_id=self.map_info.game_id; + let CheckedMapInfo{ + display_name, + creator, + game_id, + }=model_info.check(self.map_info); // MapStart must exist let mapstart=if self.counts.mode_start_counts.contains_key(&ModeID::MAIN){ @@ -788,19 +857,22 @@ impl MapCheck<'_>{ let display_name=match &self.display_name{ Ok(_)=>passed!("DisplayName"), Err(DisplayNameError::TitleCase(context))=>summary_format!("DisplayName","DisplayName must have Title Case: {context}"), + Err(DisplayNameError::CannotChange(context))=>summary_format!("DisplayName","DisplayName cannot be changed: {context}"), Err(DisplayNameError::Empty(context))=>summary_format!("DisplayName","Invalid DisplayName: {context}"), Err(DisplayNameError::TooLong(context))=>summary_format!("DisplayName","DisplayName is too long: {context} characters (50 characters max)"), Err(DisplayNameError::StringValue(context))=>summary_format!("DisplayName","DisplayName StringValue: {context}"), }; let creator=match &self.creator{ Ok(_)=>passed!("Creator"), + Err(CreatorError::CannotChange(context))=>summary_format!("Creator","Creator cannot be changed: {context}"), Err(CreatorError::Empty(context))=>summary_format!("Creator","Invalid Creator: {context}"), Err(CreatorError::TooLong(context))=>summary_format!("Creator","Creator is too long: {context} characters (50 characters max)"), Err(CreatorError::StringValue(context))=>summary_format!("Creator","Creator StringValue: {context}"), }; let game_id=match &self.game_id{ Ok(_)=>passed!("GameID"), - Err(ParseGameIDError)=>summary!("GameID","Model name must be prefixed with bhop_ surf_ or flytrials_".to_owned()), + Err(GameIDError::CannotChange(context))=>summary_format!("GameID","GameID cannot be changed: {context}"), + Err(GameIDError::Parse(ParseGameIDError))=>summary!("GameID","Model name must be prefixed with bhop_ surf_ or flytrials_".to_owned()), }; let mapstart=match &self.mapstart{ Ok(Exists)=>passed!("MapStart"), @@ -961,8 +1033,55 @@ pub struct CheckListAndVersion{ pub version:u64, } +pub trait CheckModelInfo{ + fn check<'a>(self,map_info:MapInfo<'a>)->CheckedMapInfo<'a>; +} + +pub trait CheckSpecialization{ + type ModelInfo:CheckModelInfo; + fn info(self)->(CheckRequest,Self::ModelInfo); +} + + +#[expect(nonstandard_style)] +pub struct CheckRequest{ + ModelID:u64, + SkipChecks:bool, +} + +impl CheckSpecialization for crate::nats_types::CheckMapfixRequest{ + type ModelInfo=MapInfoOwned; + fn info(self)->(CheckRequest,Self::ModelInfo) { + ( + CheckRequest{ + ModelID:self.ModelID, + SkipChecks:self.SkipChecks, + }, + MapInfoOwned{ + display_name:self.DisplayName, + creator:self.Creator, + game_id:self.GameID, + } + ) + } +} +impl CheckSpecialization for crate::nats_types::CheckSubmissionRequest{ + type ModelInfo=NoMapInfo; + fn info(self)->(CheckRequest,Self::ModelInfo) { + ( + CheckRequest{ + ModelID:self.ModelID, + SkipChecks:self.SkipChecks, + }, + NoMapInfo + ) + } +} + impl crate::message_handler::MessageHandler{ - pub async fn check_inner(&self,check_info:CheckRequest)->Result{ + pub async fn check_inner(&self,check_info:R)->Result{ + let (check_info,target_model_info)=check_info.info(); + // discover asset creator and latest version let info=self.cloud_context.get_asset_info( rbx_asset::cloud::GetAssetLatestRequest{asset_id:check_info.ModelID} @@ -1002,7 +1121,7 @@ impl crate::message_handler::MessageHandler{ let model_info=get_model_info(&dom,model_instance); // convert the model information into a structured report - let map_check=model_info.check(); + let map_check=model_info.check(target_model_info); // check the report, generate an error message if it fails the check let status=match map_check.result(){ diff --git a/validation/src/check_mapfix.rs b/validation/src/check_mapfix.rs index 6d75980..4fdc403 100644 --- a/validation/src/check_mapfix.rs +++ b/validation/src/check_mapfix.rs @@ -17,7 +17,7 @@ impl std::error::Error for Error{} impl crate::message_handler::MessageHandler{ pub async fn check_mapfix(&self,check_info:CheckMapfixRequest)->Result<(),Error>{ let mapfix_id=check_info.MapfixID; - let check_result=self.check_inner(check_info.into()).await; + let check_result=self.check_inner(check_info).await; // update the mapfix depending on the result match check_result{ diff --git a/validation/src/check_submission.rs b/validation/src/check_submission.rs index 40260c4..7aa3795 100644 --- a/validation/src/check_submission.rs +++ b/validation/src/check_submission.rs @@ -17,7 +17,7 @@ impl std::error::Error for Error{} impl crate::message_handler::MessageHandler{ pub async fn check_submission(&self,check_info:CheckSubmissionRequest)->Result<(),Error>{ let submission_id=check_info.SubmissionID; - let check_result=self.check_inner(check_info.into()).await; + let check_result=self.check_inner(check_info).await; // update the submission depending on the result match check_result{ diff --git a/validation/src/nats_types.rs b/validation/src/nats_types.rs index dfb0b91..703a373 100644 --- a/validation/src/nats_types.rs +++ b/validation/src/nats_types.rs @@ -44,7 +44,7 @@ pub struct CheckMapfixRequest{ // target map info pub DisplayName:String, pub Creator:String, - pub GameID:u32, + pub GameID:crate::rbx_util::GameID, } #[expect(nonstandard_style)] diff --git a/validation/src/rbx_util.rs b/validation/src/rbx_util.rs index 39287f3..f1bc027 100644 --- a/validation/src/rbx_util.rs +++ b/validation/src/rbx_util.rs @@ -31,6 +31,9 @@ fn find_first_child_name_and_class<'a>(dom:&'a rbx_dom_weak::WeakDom,instance:&r instance.children().iter().filter_map(|&r|dom.get_by_ref(r)).find(|inst|inst.name==name&&inst.class==class) } +#[derive(serde_repr::Deserialize_repr)] +#[repr(u32)] +#[derive(Clone,Copy,PartialEq)] pub enum GameID{ Bhop=1, Surf=2, @@ -66,6 +69,15 @@ impl TryFrom for GameID{ } } } +impl std::fmt::Display for GameID{ + fn fmt(&self,f:&mut std::fmt::Formatter<'_>)->std::fmt::Result{ + match self{ + GameID::Bhop=>write!(f,"Bhop"), + GameID::Surf=>write!(f,"Surf"), + GameID::FlyTrials=>write!(f,"FlyTrials"), + } + } +} pub struct MapInfo<'a>{ pub display_name:Result<&'a str,StringValueError>, -- 2.49.1