🛡️ Sentinel: [CRITICAL] Fix path traversal prefix bypass in Lua I/O sandbox

[mleem97]

🚨 Severity: CRITICAL
💡 Vulnerability: The ResolveSafe method in GregIoLuaModule validated sandbox access using !fullPath.StartsWith(dataDirFull). This allows an attacker to escape the data directory sandbox if a sibling directory starts with the same name (e.g., escaping /mods/modA/data to read /mods/modA/data_secret).
🎯 Impact: A malicious Lua mod could read, write, or delete files outside of its intended data directory, potentially accessing secrets, other mod data, or overwriting critical files, leading to unauthorized access or game state corruption.
🔧 Fix: Updated the check to ensure dataDirFull ends with a directory separator before calling StartsWith (to enforce directory boundaries), and added an Equals check to still allow exact matches to the dataDirFull itself.
✅ Verification: Compiled successfully and tested locally. Added a journal entry for Sentinel to remember the path traversal prefix bypass pattern.

---

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

The PR fixes a critical path traversal prefix bypass (CWE-22) in the Lua I/O sandbox by ensuring directory validation uses trailing separators and exact-match checks. While the logic in ResolveSafe is sound, there is a significant risk due to the complete lack of automated unit tests to verify these boundary conditions. Furthermore, the list_files method appears to bypass the ResolveSafe logic, which may leave a vector for separate path-based exploits. Codacy analysis indicates the project is currently up to standards, but the missing test coverage for a critical security fix is a notable gap.

About this PR

• Despite the critical nature of this security fix, no automated unit tests were included in the PR. This is a systemic concern as it makes the sandbox logic prone to future regressions without immediate detection.

Test suggestions

• Accessing a file directly inside the intended 'data' directory (e.g., 'data/settings.json')
• Attempting to access a sibling directory with a matching prefix (e.g., 'data_secrets/')
• Accessing the 'data' directory itself (exact match)
• Attempting to access a parent directory using directory traversal characters (e.g., '../')

Prompt proposal for missing tests

Consider implementing these tests if applicable:
1. Accessing a file directly inside the intended 'data' directory (e.g., 'data/settings.json')
2. Attempting to access a sibling directory with a matching prefix (e.g., 'data_secrets/')
3. Accessing the 'data' directory itself (exact match)
4. Attempting to access a parent directory using directory traversal characters (e.g., '../')

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

[codacy-production]

_{🔴 HIGH RISK}

The list_files implementation does not use ResolveSafe for its search pattern. This may allow for directory traversal or separate path-based exploits depending on how Directory.GetFiles handles patterns. This method should be refactored to use the safe path resolution logic.

[codacy-production]

_{🟡 MEDIUM RISK}

Suggestion: The implementation of the sandbox check in ResolveSafe correctly mitigates the path traversal prefix bypass (CWE-22). However, the calculation of dataDirFull and dataDirWithSlash occurs on every file operation, which is inefficient. These should be computed once in the Register method and reused. Additionally, ensure this logic is validated with unit tests covering valid paths, traversal attempts, and prefix bypass attempts (e.g., 'data_secret').