🛡️ Sentinel: CRITICAL Fix prefix path traversal in Lua IO sandbox

[mleem97]

🚨 Severity: CRITICAL
💡 Vulnerability: Prefix-matching path traversal in GregIoLuaModule.cs where String.StartsWith was used to validate path confines. This allowed a mod in /mods/modA to bypass the sandbox by accessing /mods/modA_secret using a path like ../modA_secret/file.txt.
🎯 Impact: Malicious mods could escape their restricted data directory and write or delete files in sibling directories with matching prefixes, leading to data destruction or arbitrary file manipulation.
🔧 Fix: Appended Path.DirectorySeparatorChar to the base directory before evaluating StartsWith and explicitly allowed exact matches to the base directory itself.
✅ Verification: Ran dotnet build and dotnet test to confirm compilation and verify test stability.

---

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

While this PR addresses the critical prefix-matching vulnerability in the Lua IO sandbox, several secondary security and stability issues remain in GregIoLuaModule.cs. The most significant is a remaining path traversal vulnerability in the list_files pattern parameter and a potential Denial of Service vector due to unrestricted directory recursion.

Furthermore, despite this being a critical security fix in a complex and high-risk file, no new test files or updated unit tests were included in the PR. Although Codacy indicates the project is 'up to standards', the lack of verification for these specific logic changes is a significant gap. These issues should be addressed to ensure the sandbox is truly robust against exploitation.

About this PR

• This PR modifies critical security logic but does not include any new test cases or modifications to existing tests. For a fix regarding path traversal, it is essential to have unit tests that verify both the fix and edge cases (e.g., sibling directories, exact matches).

Test suggestions

• Found recommended test scenario: Accessing a file within the allowed data directory (e.g., data/config.txt)
• Found recommended test scenario: Accessing the base data directory path exactly
• Found recommended test scenario: Attempting to access a sibling directory with a matching prefix (e.g., ../modA_secret/file.txt)
• Found recommended test scenario: Attempting to access a file outside the sandbox via standard traversal (e.g., ../../outside.txt)
• Missing recommended test scenario: Accessing files outside the sandbox using the traversal sequences in the list_files pattern parameter
• Missing recommended test scenario: Verification of parent directory creation in append_file

Prompt proposal for missing tests

Consider implementing these tests if applicable:
1. Found recommended test scenario: Accessing a file within the allowed data directory (e.g., data/config.txt)
2. Found recommended test scenario: Accessing the base data directory path exactly
3. Found recommended test scenario: Attempting to access a sibling directory with a matching prefix (e.g., ../modA_secret/file.txt)
4. Found recommended test scenario: Attempting to access a file outside the sandbox via standard traversal (e.g., ../../outside.txt)
5. Missing recommended test scenario: Accessing files outside the sandbox using the traversal sequences in the list_files pattern parameter
6. Missing recommended test scenario: Verification of parent directory creation in append_file

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

[codacy-production]

_{🔴 HIGH RISK}

The list_files implementation contains multiple risks:

1. Path Traversal: The pattern argument is not sanitized, allowing an attacker to use sequences like ../../ to list files outside the sandbox.
2. Denial of Service: Using SearchOption.AllDirectories without a depth limit can be exploited to cause a DoS by crawling deep directory structures.
3. Cross-Platform Compatibility: The default *.* pattern will fail to match files without extensions on Linux/macOS.

Sanitize the pattern variable to block directory separators and '..', and consider restricting search depth or using SearchOption.TopDirectoryOnly.

[codacy-production]

_{🟡 MEDIUM RISK}

Suggestion: Unlike write_file, this implementation does not create the parent directory if it is missing, which will cause a DirectoryNotFoundException if the provided path includes a new subdirectory. Update the function to call Directory.CreateDirectory before File.AppendAllText.

[codacy-production]

_{⚪ LOW RISK}

Exposing the absolute host path via greg.io.data_dir provides unnecessary information about the host system (CWE-200), such as the username and directory structure. Consider exposing a virtualized path like 'data/' instead.

[codacy-production]

_{⚪ LOW RISK}

Nitpick: Path.GetFullPath(dataDir) is called on every sandbox validation. Consider pre-calculating this value and the version with a trailing slash during the Register method to avoid redundant filesystem calls.