🛡️ Sentinel: HIGH Fix path traversal bypass in Lua Sandbox

[mleem97]

🚨 Severity: HIGH
💡 Vulnerability: Sandbox boundary checking used simple String.StartsWith on directory paths, which allowed prefix-matching bypasses. For example, a malicious path ending up as /mods/modA_secret would incorrectly pass the boundary check for /mods/modA.
🎯 Impact: A malicious Lua mod could escape its data directory and read/write files in sibling directories with similar prefixes, potentially leading to unauthorized data access or modification.
🔧 Fix: Updated the boundary check to require an exact match to the base directory or enforce that the path starts with the base directory plus a trailing directory separator.
✅ Verification: Tests were run and path traversal logic has been manually reviewed to prevent the prefix-match bypass.

---

PR created automatically by Jules for task 17803667132840490775 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 implements a critical fix for a high-severity path traversal vulnerability within the Lua sandbox by enforcing trailing separator checks during directory validation. Although the repository is currently 'up to standards' according to Codacy, the file GregIoLuaModule.cs is flagged for increased complexity without accompanying test coverage.

The most significant risk is the absence of automated verification for the new validation logic. While the intent of the fix is correct, the complexity of path resolution makes it prone to regressions. The review also identifies inconsistencies in directory handling for append operations and potential issues with relative path resolution if the working directory changes.

About this PR

• Despite the high-severity nature of the path traversal bypass being fixed, no automated unit or integration tests were included in this PR. Relying on manual verification for security-critical sandbox logic increases the risk of future regressions.

Test suggestions

• Verify that a path with a shared prefix but no directory separator (e.g., 'modA_secret' when the sandbox is 'modA') is rejected.
• Verify that the exact sandbox root directory path is allowed.
• Verify that a file nested inside the sandbox root directory is allowed.
• Verify that path traversal attempts using '../' are correctly resolved and blocked if they escape the sandbox root.

Prompt proposal for missing tests

Consider implementing these tests if applicable:
1. Verify that a path with a shared prefix but no directory separator (e.g., 'modA_secret' when the sandbox is 'modA') is rejected.
2. Verify that the exact sandbox root directory path is allowed.
3. Verify that a file nested inside the sandbox root directory is allowed.
4. Verify that path traversal attempts using '../' are correctly resolved and blocked if they escape the sandbox root.

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

[codacy-production]

_{🔴 HIGH RISK}

Suggestion: The validation logic in ResolveSafe is critical for preventing sandbox escapes but currently lacks test coverage and contains high complexity. Additionally, the boundary paths (dataDirFull and dataDirWithSlash) are recalculated on every call. It is recommended to refactor GregIoLuaModule.Register to pre-calculate these boundaries and implement a unit test suite (e.g., using NUnit) to verify that path traversal (../) and prefix-matching bypasses (e.g., /mods/modA_secret vs /mods/modA) are correctly handled.

[codacy-production]

_{🟡 MEDIUM RISK}

Suggestion: Try running the following prompt in your coding agent:

Update the list_files implementation in GregIoLuaModule.cs to use the absolute path of the data directory (via Path.GetFullPath) for both Directory.GetFiles and Path.GetRelativePath to ensure it remains robust against changes in the current working directory.

[codacy-production]

_{⚪ LOW RISK}

Suggestion: Try running the following prompt in your coding agent:

In GregIoLuaModule.cs, update the append_file implementation to ensure the parent directory exists before writing, using the same logic found in the write_file implementation.