-
Notifications
You must be signed in to change notification settings - Fork 1
refactor: harden virtual file system #182
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
crepererum
commented
Nov 18, 2025
- add more limits
- improve path traversal to be cleaner and better tested
- add more limits - improve path traversal to be cleaner and better tested
e149569 to
4e6e1fb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the virtual file system to add stronger security limits and improve path traversal handling. The changes introduce path length and path segment limits, replace the previous string-based path resolution with a cleaner token-based traversal system, and add comprehensive test coverage for edge cases including NULL bytes and directory traversal patterns.
Key Changes
- Added
max_path_lengthandmax_path_segment_sizelimits to prevent DoS attacks via extremely long paths - Refactored path resolution to use a cleaner
PathTraversalenum (Up/Stay/Down) instead of string manipulation - Moved error types and limits into dedicated modules for better organization
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
host/src/vfs/path.rs |
New module implementing type-safe path parsing with PathSegment and PathTraversal types |
host/src/vfs/mod.rs |
Refactored path resolution to use the new traversal API, replaced string splitting with token-based iteration |
host/src/vfs/limits.rs |
Extracted limits configuration with new path-related constraints |
host/src/vfs/error.rs |
Moved LimitExceeded error type to dedicated module with proper trait implementations |
host/src/lib.rs |
Updated VfsState initialization to clone limits; removed unused test import |
host/tests/integration_tests/evil/test_utils.rs |
Refactored test utilities to support custom permissions for limit testing |
host/tests/integration_tests/evil/root.rs |
Added tests for path length and segment length limit enforcement |
host/tests/integration_tests/evil/fs.rs |
Extended test coverage to include NULL byte and path traversal patterns (\0, /x/..) |
guests/evil/src/root/path_long.rs |
New evil payload for testing path length limits |
guests/evil/src/root/mod.rs |
Added path_long module to exports |
guests/evil/src/lib.rs |
Registered path_long test case |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "/tmp", | ||
| "\0", | ||
| "/x/..", | ||
| ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I notice this list doesn't contain /bin, /boot, /lib . We (I) should (can) write a ticket for further hardening that describes threat vectors based around those folders (for example, linking an evil lib) (?)
Sl1mb0
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One important question that does not hold this PR up in any way.
I enjoy reading your code BTW 🤩