🛡️ Sentinel: [CRITICAL] Fix prefix-matching path traversal in Lua IO sandbox

[mleem97]

🚨 Severity: CRITICAL
💡 Vulnerability: A prefix-matching path traversal vulnerability existed in GregIoLuaModule.cs. String.StartsWith on file paths without a trailing directory separator allowed escaping the sandbox to sibling directories that share the same prefix (e.g. /mods/Lua/my_mod/data_secret bypassing the /mods/Lua/my_mod/data boundary).
🎯 Impact: Malicious or vulnerable Lua mods could access or modify files in other mods' data directories (if they share the same prefix name) or other similarly named directories outside their intended sandbox.
🔧 Fix: Ensured the base directory used for the StartsWith sandbox check strictly ends with a Path.DirectorySeparatorChar, and also added an Equals check for exact directory matching.
✅ Verification: Code review of the added Path.DirectorySeparatorChar logic and running the dotnet test tests/gregCore.Tests.csproj test suite.

---

PR created automatically by Jules for task 14974776097076936158 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]

Pull Request Overview

This PR addresses a critical security vulnerability involving prefix-matching path traversal within the Lua IO sandbox. The changes ensure that base directory validation strictly handles directory separators and exact matches, preventing access to sibling directories.

While the code changes appear to satisfy the intent, the PR is currently missing necessary unit tests to verify the fix and prevent regressions in this high-risk area. Furthermore, the GregIoLuaModule has been flagged as a complex file with no coverage, increasing the risk of merging these changes without automated verification. There is also a performance and potential information disclosure concern regarding how paths are normalized during every IO operation.

About this PR

• This PR modifies a security-critical IO sandbox to fix a traversal vulnerability but does not include any new unit tests. Given that the file is complex and currently lacks test coverage, adding automated tests to verify the sandbox boundaries (especially the prefix-matching case) is essential to prevent future regressions.

Test suggestions

• Missing recommended test scenario: Verify exact match access to the data directory itself
• Missing recommended test scenario: Verify access to a file within the sandboxed data directory
• Missing recommended test scenario: Verify access is denied to a sibling directory sharing the same name prefix
• Missing recommended test scenario: Verify access is denied when using standard path traversal sequences (../)
• Missing automatable coverage for security-critical validation logic in GregIoLuaModule.cs

Prompt proposal for missing tests

Consider implementing these tests if applicable:
1. Missing recommended test scenario: Verify exact match access to the data directory itself
2. Missing recommended test scenario: Verify access to a file within the sandboxed data directory
3. Missing recommended test scenario: Verify access is denied to a sibling directory sharing the same name prefix
4. Missing recommended test scenario: Verify access is denied when using standard path traversal sequences (../)
5. Missing automatable coverage for security-critical validation logic in GregIoLuaModule.cs

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

[codacy-production]

_{🔴 HIGH RISK}

This module implements a security-critical IO sandbox. The current changes fix a significant vulnerability but lack accompanying tests. It is highly recommended to add unit tests that verify both valid access within the sandbox and rejected access for traversal attempts, specifically including the prefix-matching case (e.g., /data_secret) fixed in this PR.

See Complexity in Codacy
See Coverage in Codacy

[codacy-production]

_{🟡 MEDIUM RISK}

Suggestion: Normalize dataDir to an absolute path once in the Register method and pre-calculate boundary strings (e.g., dataDirFull, dataDirWithSep). Recalculating these via Path.GetFullPath on every file operation is inefficient and, if dataDir is relative, can leak absolute host paths through list_files. Refactoring these into the Register scope will improve performance and security.