fix: file navigator#9307
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR closes a path traversal / navigation restriction bypass in the file browser by tightening the server-side containment check and adding regression tests for common escape attempts.
Changes:
- Hardened
_list_directorynavigation restriction to require the requested path to be contained within the initial path. - Added tests covering sibling directory access, arbitrary absolute paths, and
..traversal escapes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
marimo/_plugins/ui/_impl/file_browser.py |
Replaces the parent-only restriction check with a containment check based on resolve() + relative_to(). |
tests/_plugins/ui/_impl/test_file_browser.py |
Adds regression tests for multiple navigation bypass vectors. |
| if self._restrict_navigation: | ||
| try: | ||
| path.resolve().relative_to(self._initial_path.resolve()) | ||
| except ValueError: |
There was a problem hiding this comment.
Path.resolve() can raise exceptions other than ValueError (e.g., RuntimeError for symlink loops, OSError for invalid paths). Currently those would bubble up as unexpected errors rather than a consistent "navigation restricted" failure. Consider catching those exceptions and raising a controlled RuntimeError (or a more specific exception type) with an appropriate message.
| except ValueError: | |
| except (ValueError, RuntimeError, OSError): |
There was a problem hiding this comment.
i would add these errors
There was a problem hiding this comment.
symlink loops
Symlink loops create an OSError, but added
|
🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.23.3-dev34 |
📝 Summary
File browser path restriction can be subverted