Skip to content

Commit c5e88fa

Browse files
authored
fix: file navigator (#9307)
## 📝 Summary File browser path restriction can be subverted
1 parent 1a15460 commit c5e88fa

2 files changed

Lines changed: 120 additions & 4 deletions

File tree

marimo/_plugins/ui/_impl/file_browser.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -293,10 +293,16 @@ def _list_directory(
293293
# such as CloudPath
294294
path = self._create_path(args.path)
295295

296-
if self._restrict_navigation and path in self._initial_path.parents:
297-
raise RuntimeError(
298-
"Navigation is restricted; navigating to a parent of initial path is not allowed."
299-
)
296+
if self._restrict_navigation:
297+
try:
298+
path.resolve(strict=True).relative_to(
299+
self._initial_path.resolve()
300+
)
301+
# NB. RuntimeError vs OSError depends on the version of python.
302+
except (ValueError, RuntimeError, OSError):
303+
raise RuntimeError(
304+
"Navigation is restricted; navigating outside the initial path is not allowed."
305+
) from None
300306
folders: list[TypedFileBrowserFileInfo] = []
301307
files: list[TypedFileBrowserFileInfo] = []
302308

tests/_plugins/ui/_impl/test_file_browser.py

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,116 @@ def test_navigation_restriction() -> None:
6363
assert "Navigation is restricted" in str(e.value)
6464

6565

66+
def test_navigation_restriction_allows_subdirectory(tmp_path: Path) -> None:
67+
"""Navigation within the initial directory must succeed."""
68+
root = tmp_path / "root"
69+
child = root / "child"
70+
child.mkdir(parents=True)
71+
(child / "file.txt").touch()
72+
73+
fb = file_browser(initial_path=root, restrict_navigation=True)
74+
response = fb._list_directory(ListDirectoryArgs(path=str(child)))
75+
assert isinstance(response, ListDirectoryResponse)
76+
file_names = [f["name"] for f in response.files]
77+
assert "file.txt" in file_names
78+
79+
80+
def test_navigation_restriction_sibling(tmp_path: Path) -> None:
81+
"""Sibling directories must be rejected, not just direct parents."""
82+
restricted = tmp_path / "restricted"
83+
sibling = tmp_path / "sibling"
84+
restricted.mkdir()
85+
sibling.mkdir()
86+
(sibling / "secret.txt").touch()
87+
88+
fb = file_browser(initial_path=restricted, restrict_navigation=True)
89+
with pytest.raises(RuntimeError, match="Navigation is restricted"):
90+
fb._list_directory(ListDirectoryArgs(path=str(sibling)))
91+
92+
93+
def test_navigation_restriction_absolute_path(tmp_path: Path) -> None:
94+
"""Arbitrary absolute paths outside the root must be rejected."""
95+
restricted = tmp_path / "restricted"
96+
restricted.mkdir()
97+
98+
fb = file_browser(initial_path=restricted, restrict_navigation=True)
99+
with pytest.raises(RuntimeError, match="Navigation is restricted"):
100+
fb._list_directory(ListDirectoryArgs(path="/tmp"))
101+
102+
103+
def test_navigation_restriction_symlink_escape(tmp_path: Path) -> None:
104+
"""A symlink inside the restricted dir pointing outside must be rejected."""
105+
restricted = tmp_path / "restricted"
106+
restricted.mkdir()
107+
outside = tmp_path / "outside"
108+
outside.mkdir()
109+
(outside / "secret.txt").touch()
110+
111+
# Symlink lives inside restricted/ but resolves outside
112+
escape_link = restricted / "escape"
113+
try:
114+
escape_link.symlink_to(outside)
115+
except OSError:
116+
pytest.skip("Cannot create symlinks on this system")
117+
118+
fb = file_browser(initial_path=restricted, restrict_navigation=True)
119+
with pytest.raises(RuntimeError, match="Navigation is restricted"):
120+
fb._list_directory(ListDirectoryArgs(path=str(escape_link)))
121+
122+
123+
def test_navigation_restriction_symlink_loop(tmp_path: Path) -> None:
124+
"""Circular symlinks must not hang or silently succeed."""
125+
restricted = tmp_path / "restricted"
126+
restricted.mkdir()
127+
128+
link_a = restricted / "link_a"
129+
link_b = restricted / "link_b"
130+
try:
131+
link_a.symlink_to(link_b)
132+
link_b.symlink_to(link_a)
133+
except OSError:
134+
pytest.skip("Cannot create symlinks on this system")
135+
136+
fb = file_browser(initial_path=restricted, restrict_navigation=True)
137+
with pytest.raises(RuntimeError, match="Navigation is restricted"):
138+
fb._list_directory(ListDirectoryArgs(path=str(link_a)))
139+
140+
141+
def test_navigation_restriction_internal_symlink(tmp_path: Path) -> None:
142+
"""A symlink that resolves to a path inside the restricted dir is allowed."""
143+
restricted = tmp_path / "restricted"
144+
child = restricted / "child"
145+
child.mkdir(parents=True)
146+
(child / "file.txt").touch()
147+
148+
# Symlink inside restricted/ pointing to another subdir of restricted/
149+
alias = restricted / "alias"
150+
try:
151+
alias.symlink_to(child)
152+
except OSError:
153+
pytest.skip("Cannot create symlinks on this system")
154+
155+
fb = file_browser(initial_path=restricted, restrict_navigation=True)
156+
response = fb._list_directory(ListDirectoryArgs(path=str(alias)))
157+
assert isinstance(response, ListDirectoryResponse)
158+
file_names = [f["name"] for f in response.files]
159+
assert "file.txt" in file_names
160+
161+
162+
def test_navigation_restriction_dotdot_escape(tmp_path: Path) -> None:
163+
"""Path traversal via .. components must be caught."""
164+
restricted = tmp_path / "restricted"
165+
sibling = tmp_path / "sibling"
166+
restricted.mkdir()
167+
sibling.mkdir()
168+
(sibling / "secret.txt").touch()
169+
170+
fb = file_browser(initial_path=restricted, restrict_navigation=True)
171+
traversal = str(restricted / ".." / "sibling")
172+
with pytest.raises(RuntimeError, match="Navigation is restricted"):
173+
fb._list_directory(ListDirectoryArgs(path=traversal))
174+
175+
66176
def test_name_method() -> None:
67177
fb = file_browser(initial_path=Path.cwd())
68178
fb._value = [

0 commit comments

Comments
 (0)