refine(hotfix): Opus SHOULD-FIX — drop redundant equality clause + tighten symlink test

- The is_relative_to() check already covers the workspace==target equality case
  (is_relative_to(A,A) is True), so the '!= workspace' prefix was redundant.
- Symlink test now asserts a hard 403 (proves the guard fires, not just that
  nothing leaked) and cleans up the out-of-workspace escape dir in a finally.
This commit is contained in:
nesquena-hermes 2026-06-02 02:28:44 +00:00
parent f112d8aa9a
commit 2ab0b56078
2 changed files with 20 additions and 16 deletions

View file

@ -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)

View file

@ -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)