🛡️ Sentinel: [CRITICAL] Fix path traversal in CustomEmployeeManager

[mleem97]

🚨 Severity: CRITICAL
💡 Vulnerability: The CustomEmployeeManager.Register method accepted arbitrary id strings, which were later used unvalidated in file paths (e.g. Path.Combine(assetsDir, employeeId + ext);), leading to a path traversal vulnerability.
🎯 Impact: An attacker or malicious mod could supply a crafted employeeId (e.g., ../../sensitive_file) to read arbitrary files on the system by breaking out of the ModAssets directory.
🔧 Fix: Added input validation in Register to reject ids that contain invalid file name characters or ...
✅ Verification: Compiled successfully and tests passed.

---

PR created automatically by Jules for task 5385635832670229017 started by @mleem97

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity · 0 duplication

Metric	Results
Complexity	0
Duplication	0

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

_{TIP This summary will be updated as you push new changes.}

[codacy-production]

Pull Request Overview

This PR addresses a critical path traversal vulnerability in the CustomEmployeeManager.Register method by implementing character validation on employee identifiers. While the code changes align with the security requirements, the lack of accompanying unit tests for a critical security fix is a major concern that should prevent merging until addressed.

Furthermore, this is the fourth recorded instance of path traversal vulnerabilities in this project within a short timeframe. This systemic pattern suggests that manually implementing validation in each manager is error-prone and indicates a need for a centralized, reusable validation utility.

About this PR

• The PR does not include any new or updated test files. For a critical security fix involving path traversal, it is essential to include automated unit tests to verify the validation logic and prevent regressions.
• There is a recurring pattern of path traversal vulnerabilities in this project. Instead of isolated fixes, consider centralizing path validation into a utility class (e.g., PathValidator) to ensure consistent security across the API.

Test suggestions

• Successfully register an employee with a valid alphanumeric ID.
• Attempt to register an employee with an ID containing path traversal characters (e.g., '../secrets').
• Attempt to register an employee with an ID containing invalid filename characters (e.g., 'emp:id' or 'emp*id').
• Verify that invalid registration attempts return 0 and trigger a log entry in CrashLog.

Prompt proposal for missing tests

Consider implementing these tests if applicable:
1. Successfully register an employee with a valid alphanumeric ID.
2. Attempt to register an employee with an ID containing path traversal characters (e.g., '../secrets').
3. Attempt to register an employee with an ID containing invalid filename characters (e.g., 'emp:id' or 'emp*id').
4. Verify that invalid registration attempts return 0 and trigger a log entry in CrashLog.

_{TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback}

[codacy-production]

_{🔴 HIGH RISK}

Suggestion: This critical security fix must be verified with automated tests to ensure cross-platform safety (e.g., handling both '/' and '' on different OS runners). Given the frequency of these issues, consider centralizing this logic.

Try running this prompt in your IDE agent:

Create a comprehensive unit test suite for CustomEmployeeManager.Register that verifies it correctly rejects path traversal attempts using payloads like ../, ..\, absolute paths, and invalid filename characters.

[codacy-production]

_{⚪ LOW RISK}

Suggestion: Use string.IsNullOrWhiteSpace(id) to prevent identifiers that consist only of whitespace characters, ensuring more robust input validation.

Suggested change

	if (string.IsNullOrEmpty(id)) return 0;
	if (string.IsNullOrWhiteSpace(id)) return 0;```
	<!-- e34d5167-b092-49eb-b8c8-33859ab00079 -->

[codacy-production]

_{⚪ LOW RISK}

Nitpick: Add a blank line before the new header to maintain consistent Markdown formatting and readability.