🛡️ Sentinel: [HIGH] Fix path traversal in employee portrait loading

.jules/sentinel.md

@@ -21,3 +21,8 @@
 **Vulnerability:** `CustomEmployeeManager.Register` accepted arbitrary employee IDs without validation, which were later used directly in `Path.Combine` to construct image loading paths, enabling path traversal (CWE-22).
 **Learning:** Identifiers provided by mods or external sources must be treated as untrusted input and validated before being used in file system operations.
 **Prevention:** Validate input strings that form part of a file path before concatenating them. Reject them if they contain directory traversal characters like `..`, `Path.DirectorySeparatorChar`, `Path.AltDirectorySeparatorChar`, or any invalid filename characters (using `Path.GetInvalidFileNameChars()`).
+
+## 2024-10-24 - Defense-in-Depth for File Operations
+**Vulnerability:** Path traversal vulnerability in `SetPortrait` where `employeeId` was used directly in `Path.Combine` without validation, relying entirely on upstream validation during registration.
+**Learning:** Even when inputs like `employeeId` are validated at creation/registration, any subsequent methods that consume them for file system operations must independently validate them to protect against direct calls, missing validations, or deserialization bypasses. Additionally, security checks must not use early returns if fallback logic (like default UI colors) is present later in the method.
+**Prevention:** Always validate identifiers used in file paths at the point of use using `IndexOfAny(Path.GetInvalidFileNameChars()) >= 0 || Contains("..")`. Wrap vulnerable operations in conditional blocks rather than returning early to ensure fallback state is correctly applied.

src/API/CustomEmployeeManager.cs

@@ -891,10 +891,15 @@ private static void SetPortrait(Transform card, string employeeId)
 
             string assetsDir = Path.Combine(MelonEnvironment.UserDataDirectory, "ModAssets");
             string? imagePath = null;
-            foreach (var ext in new[] { ".jpg", ".png" })
+
+            // [Security] Prevent path traversal in employee portrait loading
+            if (employeeId.IndexOfAny(Path.GetInvalidFileNameChars()) < 0 && !employeeId.Contains(".."))
             {
-                string candidate = Path.Combine(assetsDir, employeeId + ext);
-                if (File.Exists(candidate)) { imagePath = candidate; break; }
+                foreach (var ext in new[] { ".jpg", ".png" })
+                {
+                    string candidate = Path.Combine(assetsDir, employeeId + ext);
+                    if (File.Exists(candidate)) { imagePath = candidate; break; }
+                }
             }
 
             if (imagePath != null)