Character profile sanitization improvements (#25579)

Validate that job and antag prototypes can actually be set in character profiles, rather than just checking if the prototype exists.

Make preferences system just call existing validation code when loading prototype from database, instead of some hacked-together stuff.

Also I made the character profile validation logic take dependencies in via parameter because fuck resolves.

Content.Client/Preferences/ClientPreferencesManager.cs

@@ -3,8 +3,10 @@ using System.Collections.Generic;
 using System.Linq;
 using Content.Shared.Preferences;
 using Robust.Client;
+using Robust.Shared.Configuration;
 using Robust.Shared.IoC;
 using Robust.Shared.Network;
+using Robust.Shared.Prototypes;
 using Robust.Shared.Utility;
 
 namespace Content.Client.Preferences
@@ -18,6 +20,8 @@ namespace Content.Client.Preferences
     {
         [Dependency] private readonly IClientNetManager _netManager = default!;
         [Dependency] private readonly IBaseClient _baseClient = default!;
+        [Dependency] private readonly IConfigurationManager _cfg = default!;
+        [Dependency] private readonly IPrototypeManager _prototypes = default!;
 
         public event Action? OnServerDataLoaded;
 
@@ -60,7 +64,7 @@ namespace Content.Client.Preferences
 
         public void UpdateCharacter(ICharacterProfile profile, int slot)
         {
-            profile.EnsureValid();
+            profile.EnsureValid(_cfg, _prototypes);
             var characters = new Dictionary<int, ICharacterProfile>(Preferences.Characters) {[slot] = profile};
             Preferences = new PlayerPreferences(characters, Preferences.SelectedCharacterIndex, Preferences.AdminOOCColor);
             var msg = new MsgUpdateCharacter

Content.Server/Preferences/Managers/ServerPreferencesManager.cs

@@ -99,7 +99,7 @@ namespace Content.Server.Preferences.Managers
 
             var curPrefs = prefsData.Prefs!;
 
-            profile.EnsureValid();
+            profile.EnsureValid(_cfg, _protos);
 
             var profiles = new Dictionary<int, ICharacterProfile>(curPrefs.Characters)
             {
@@ -270,34 +270,7 @@ namespace Content.Server.Preferences.Managers
 
             return new PlayerPreferences(prefs.Characters.Select(p =>
             {
-                ICharacterProfile newProf;
+                return new KeyValuePair<int, ICharacterProfile>(p.Key, p.Value.Validated(_cfg, _protos));
-                switch (p.Value)
-                {
-                    case HumanoidCharacterProfile hp:
-                    {
-                        var prototypeManager = IoCManager.Resolve<IPrototypeManager>();
-                        var selectedSpecies = HumanoidAppearanceSystem.DefaultSpecies;
-
-                        if (prototypeManager.TryIndex<SpeciesPrototype>(hp.Species, out var species) && species.RoundStart)
-                        {
-                            selectedSpecies = hp.Species;
-                        }
-
-                        newProf = hp
-                            .WithJobPriorities(
-                                hp.JobPriorities.Where(job =>
-                                    _protos.HasIndex<JobPrototype>(job.Key)))
-                            .WithAntagPreferences(
-                                hp.AntagPreferences.Where(antag =>
-                                    _protos.HasIndex<AntagPrototype>(antag)))
-                            .WithSpecies(selectedSpecies);
-                        break;
-                    }
-                    default:
-                        throw new NotSupportedException();
-                }
-
-                return new KeyValuePair<int, ICharacterProfile>(p.Key, newProf);
             }), prefs.SelectedCharacterIndex, prefs.AdminOOCColor);
         }
 

Content.Shared/Preferences/HumanoidCharacterProfile.cs

@@ -371,10 +371,8 @@ namespace Content.Shared.Preferences
             return Appearance.MemberwiseEquals(other.Appearance);
         }
 
-        public void EnsureValid()
+        public void EnsureValid(IConfigurationManager configManager, IPrototypeManager prototypeManager)
         {
-            var prototypeManager = IoCManager.Resolve<IPrototypeManager>();
-
             if (!prototypeManager.TryIndex<SpeciesPrototype>(Species, out var speciesPrototype) || speciesPrototype.RoundStart == false)
             {
                 Species = SharedHumanoidAppearanceSystem.DefaultSpecies;
@@ -390,15 +388,10 @@ namespace Content.Shared.Preferences
             };
 
             // ensure the species can be that sex and their age fits the founds
-            var age = Age;
+            if (!speciesPrototype.Sexes.Contains(sex))
-            if (speciesPrototype != null)
+                sex = speciesPrototype.Sexes[0];
-            {
+
-                if (!speciesPrototype.Sexes.Contains(sex))
+            var age = Math.Clamp(Age, speciesPrototype.MinAge, speciesPrototype.MaxAge);
-                {
-                    sex = speciesPrototype.Sexes[0];
-                }
-                age = Math.Clamp(Age, speciesPrototype.MinAge, speciesPrototype.MaxAge);
-            }
 
             var gender = Gender switch
             {
@@ -425,7 +418,6 @@ namespace Content.Shared.Preferences
 
             name = name.Trim();
 
-            var configManager = IoCManager.Resolve<IConfigurationManager>();
             if (configManager.GetCVar(CCVars.RestrictedNames))
             {
                 name = Regex.Replace(name, @"[^A-Z,a-z,0-9, -]", string.Empty);
@@ -487,7 +479,7 @@ namespace Content.Shared.Preferences
             };
 
             var priorities = new Dictionary<string, JobPriority>(JobPriorities
-                .Where(p => prototypeManager.HasIndex<JobPrototype>(p.Key) && p.Value switch
+                .Where(p => prototypeManager.TryIndex<JobPrototype>(p.Key, out var job) && job.SetPreference && p.Value switch
                 {
                     JobPriority.Never => false, // Drop never since that's assumed default.
                     JobPriority.Low => true,
@@ -497,7 +489,7 @@ namespace Content.Shared.Preferences
                 }));
 
             var antags = AntagPreferences
-                .Where(prototypeManager.HasIndex<AntagPrototype>)
+                .Where(id => prototypeManager.TryIndex<AntagPrototype>(id, out var antag) && antag.SetPreference)
                 .ToList();
 
             var traits = TraitPreferences
@@ -530,6 +522,13 @@ namespace Content.Shared.Preferences
             _traitPreferences.AddRange(traits);
         }
 
+        public ICharacterProfile Validated(IConfigurationManager configManager, IPrototypeManager prototypeManager)
+        {
+            var profile = new HumanoidCharacterProfile(this);
+            profile.EnsureValid(configManager, prototypeManager);
+            return profile;
+        }
+
         // sorry this is kind of weird and duplicated,
         /// working inside these non entity systems is a bit wack
         public static string GetName(string species, Gender gender)

Content.Shared/Preferences/ICharacterProfile.cs

@@ -1,4 +1,6 @@
 using Content.Shared.Humanoid;
+using Robust.Shared.Configuration;
+using Robust.Shared.Prototypes;
 
 namespace Content.Shared.Preferences
 {
@@ -13,6 +15,11 @@ namespace Content.Shared.Preferences
         /// <summary>
         ///     Makes this profile valid so there's no bad data like negative ages.
         /// </summary>
-        void EnsureValid();
+        void EnsureValid(IConfigurationManager configManager, IPrototypeManager prototypeManager);
+
+        /// <summary>
+        /// Gets a copy of this profile that has <see cref="EnsureValid"/> applied, i.e. no invalid data.
+        /// </summary>
+        ICharacterProfile Validated(IConfigurationManager configManager, IPrototypeManager prototypeManager);
     }
 }