From 2ab0b560788f62f26bca0cf4c7d60e61c6cddd2e Mon Sep 17 00:00:00 2001
From: nesquena-hermes <[email protected]>
Date: Tue, 2 Jun 2026 02:28:44 +0000
Subject: [PATCH] =?UTF-8?q?refine(hotfix):=20Opus=20SHOULD-FIX=20=E2=80=94?=
 =?UTF-8?q?=20drop=20redundant=20equality=20clause=20+=20tighten=20symlink?=
 =?UTF-8?q?=20test?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- 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.
---
 api/upload.py                  |  5 +++--
 tests/test_workspace_upload.py | 31 +++++++++++++++++--------------
 2 files changed, 20 insertions(+), 16 deletions(-)

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)