From 55a6169dda2331ac95fbb1d9836a43860c1d6870 Mon Sep 17 00:00:00 2001
From: CleverThis <hal9000@cleverthis.com>
Date: Thu, 14 May 2026 10:09:48 +0000
Subject: [PATCH] fix(security): fix file_tools.py validate_path startswith
 bypass #7478

Replace string-based startswith() path traversal guards with robust
Path.relative_to() across three files to prevent prefix-collision
bypass attacks.  The old checks using str(target).startswith(str(root))
could be evaded by paths like /tmp/abc123-escape when root is /tmp/abc123.

Files patched:
  - src/cleveragents/skills/builtins/file_ops.py (validate_sandbox_path)
  - src/cleveragents/resource/handlers/_base.py (_safe_resolve)
  - src/cleveragents/application/services/llm_actors.py (_write_to_sandbox)

ISSUES CLOSED: #7478
---
 .../application/services/llm_actors.py             | 13 +++++++++----
 src/cleveragents/resource/handlers/_base.py        | 14 ++++++++++----
 src/cleveragents/skills/builtins/file_ops.py       | 12 ++++++++++--
 3 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/src/cleveragents/application/services/llm_actors.py b/src/cleveragents/application/services/llm_actors.py
index 398e58ef2..3d6c026f1 100644
--- a/src/cleveragents/application/services/llm_actors.py
+++ b/src/cleveragents/application/services/llm_actors.py
@@ -11,6 +11,7 @@ from __future__ import annotations
 import os
 import re
 from datetime import UTC, datetime
+from pathlib import Path
 from typing import TYPE_CHECKING, Any, Protocol, runtime_checkable
 
 import structlog
@@ -490,13 +491,17 @@ class LLMExecuteActor:
         for match in pattern.finditer(llm_output):
             path = match.group(1).strip()
             content = match.group(2)
-            full_path = os.path.normpath(os.path.join(sandbox_root, path))
-            # Path traversal guard: reject paths escaping sandbox
-            if not full_path.startswith(sandbox_root + os.sep):
+            full_path = Path(os.path.normpath(os.path.join(sandbox_root, path)))
+            # Path traversal guard: use relative_to (not string prefix) to
+            # avoid the *prefix-collision bypass* on symlink resolution.
+            sandbox_root_resolved = Path(sandbox_root).resolve()
+            try:
+                full_path.relative_to(sandbox_root_resolved)
+            except ValueError:
                 logger.warning(
                     "Rejected path traversal in LLM output",
                     path=path,
-                    resolved=full_path,
+                    resolved=str(full_path),
                 )
                 continue
             os.makedirs(os.path.dirname(full_path), exist_ok=True)
diff --git a/src/cleveragents/resource/handlers/_base.py b/src/cleveragents/resource/handlers/_base.py
index 5941ca187..9cdb71850 100644
--- a/src/cleveragents/resource/handlers/_base.py
+++ b/src/cleveragents/resource/handlers/_base.py
@@ -179,13 +179,19 @@ class BaseResourceHandler:
         Raises:
             PermissionError: If *path* escapes the root via ``..`` or
                 symlink resolution.
+
+        Uses :meth:`Path.relative_to` (not string prefix matching) to
+        avoid the *prefix-collision bypass* exemplified by
+        ``startswith(root + os.sep)`` on symlinks and resolved paths.
         """
         root = Path(location).resolve()
         target = (root / path).resolve()
-        # Use root + os.sep to prevent prefix collision bypass:
-        # e.g. root=/tmp/foo must not match target=/tmp/foobar/secret
-        if target != root and not str(target).startswith(str(root) + os.sep):
-            raise PermissionError(f"Path '{path}' escapes resource root '{location}'")
+        try:
+            target.relative_to(root)
+        except ValueError as exc:
+            raise PermissionError(
+                f"Path '{path}' escapes resource root '{location}'"
+            ) from exc
         return target
 
     def read(self, *, resource: Resource, path: str = "") -> Content:
diff --git a/src/cleveragents/skills/builtins/file_ops.py b/src/cleveragents/skills/builtins/file_ops.py
index 035994e74..7b02b5484 100644
--- a/src/cleveragents/skills/builtins/file_ops.py
+++ b/src/cleveragents/skills/builtins/file_ops.py
@@ -69,6 +69,10 @@ def validate_sandbox_path(path_str: str, sandbox_root: str | None = None) -> Pat
 
     Raises ``ValueError`` with the rejected path when traversal is
     detected or the resolved path falls outside the sandbox root.
+
+    Uses :meth:`Path.relative_to` (not string prefix matching) to avoid
+    the *prefix-collision bypass*: a target like ``/tmp/abc123-escape``
+    would incorrectly pass ``startswith("/tmp/abc123")``.
     """
     if not path_str:
         raise ValueError("Path must not be empty")
@@ -77,8 +81,12 @@ def validate_sandbox_path(path_str: str, sandbox_root: str | None = None) -> Pat
     root = root.resolve()
 
     target = (root / path_str).resolve()
-    if not str(target).startswith(str(root)):
-        raise ValueError(f"Path traversal detected: '{path_str}' escapes sandbox root")
+    try:
+        target.relative_to(root)
+    except ValueError as exc:
+        raise ValueError(
+            f"Path traversal detected: '{path_str}' escapes sandbox root"
+        ) from exc
     return target
 
 
-- 
2.52.0