diff --git a/api/upload.py b/api/upload.py index f53918d5..9399c197 100644 --- a/api/upload.py +++ b/api/upload.py @@ -424,8 +424,9 @@ def handle_workspace_upload(handler): # outside the root (read trust model). For an UPLOAD target that's not # acceptable: a planted symlink subpath would let mkdir() + writes create # files OUTSIDE the workspace. Require the resolved target to be inside - # the workspace before creating anything. - if target_dir.resolve() != workspace.resolve() and not target_dir.resolve().is_relative_to(workspace.resolve()): + # the workspace before creating anything. (is_relative_to is True for the + # workspace==target equality case, so the normal subpath='' path passes.) + if not target_dir.resolve().is_relative_to(workspace.resolve()): return j(handler, {'error': 'Upload target escapes workspace'}, status=403) target_dir.mkdir(parents=True, exist_ok=True) diff --git a/tests/test_workspace_upload.py b/tests/test_workspace_upload.py index 7836b605..25cfb73a 100644 --- a/tests/test_workspace_upload.py +++ b/tests/test_workspace_upload.py @@ -503,17 +503,20 @@ class TestWorkspaceUploadSymlinkTarget: escape.mkdir(parents=True, exist_ok=True) link = ws / "outlink" try: - os.symlink(str(escape), str(link)) - except (OSError, NotImplementedError): - import pytest - pytest.skip("symlinks not supported in this environment") - result, status = post_multipart( - "/api/workspace/upload", - {"session_id": sid, "path": "outlink"}, - {"file": ("pwned.txt", b"should not land outside")}, - ) - # Either a 403 escape rejection, or at minimum nothing written outside. - assert status == 403 or not (escape / "pwned.txt").exists(), ( - f"upload escaped workspace via symlink: status={status} result={result}" - ) - assert not (escape / "pwned.txt").exists() + try: + os.symlink(str(escape), str(link)) + except (OSError, NotImplementedError): + import pytest + pytest.skip("symlinks not supported in this environment") + result, status = post_multipart( + "/api/workspace/upload", + {"session_id": sid, "path": "outlink"}, + {"file": ("pwned.txt", b"should not land outside")}, + ) + # The escaping target must be rejected outright, and nothing may land + # outside the workspace. + assert status == 403, f"expected 403, got status={status} result={result}" + assert not (escape / "pwned.txt").exists() + finally: + import shutil + shutil.rmtree(escape, ignore_errors=True)