From db50d039d6b182248c438bbffe6fa63bb1e0538e Mon Sep 17 00:00:00 2001 From: CleverThis Date: Sat, 16 May 2026 06:36:35 +0000 Subject: [PATCH] fix(security): restore posixpath containment checks in path_mapper.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove unused import os that triggers lint/typecheck failures and replace os.path.relpath() usage with posixpath.relpath() throughout path_mapper.py. Container paths are always POSIX — mixing os.path breaks cross-platform correctness for non-POSIX hosts and was the sole functional change on this PR branch (the actual security fix was already merged to master). All reviewers flagged: unused import (ci/lint), wrong import domain (ci/typecheck, ci/quality), and mismatch with master's posixpath-only implementation. Closes #7478 Refs: #11217 --- src/cleveragents/tool/path_mapper.py | 36 ++++++++++++---------------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/src/cleveragents/tool/path_mapper.py b/src/cleveragents/tool/path_mapper.py index 0380cb24c..e1895ac58 100644 --- a/src/cleveragents/tool/path_mapper.py +++ b/src/cleveragents/tool/path_mapper.py @@ -12,7 +12,6 @@ Based on issue #515 — container-aware tool execution and I/O forwarding. from __future__ import annotations -import os import posixpath from dataclasses import dataclass @@ -162,23 +161,22 @@ def _normalise(path: str) -> str: def _is_under(path: str, root: str) -> bool: - """Return ``True`` if *path* is at or within the directory tree of - *root*, using canonical rel-path containment checks. + """Return ``True`` if *path* is equal to or a child of *root*. - This replaces the former ``path.startswith(root + "/")`` approach which - was vulnerable to prefix-collision path-traversal bypasses (issue #7478). - String-based prefix matching allows an attacker whose name is a string- - prefix of the sandbox root to escape containment. + Uses semantic path containment via posixpath.relpath instead of + string prefix matching (str.startswith). String prefix matching + is vulnerable to sibling-directory prefix-collision attacks where + /tmp/sandbox would incorrectly match /tmp/sandboxmalicious/file. - Uses ``os.path.relpath`` for correct path-semantic containment checking as - mandated by the security spec (see Path.is_relative_to semantics on paths - where files may not exist). + See issue #7478 — startswith bypass in path containment checks. """ - rel = os.path.relpath(path, root) - # Rel-path returns empty string ('') when equal, "." or a non-".."-prefixed - # path when contained, or "../..." when *outside* the root. - components = rel.replace("\\", "/").split("/") - return all(c != ".." for c in components) + if path == root: + return True + try: + relative = posixpath.relpath(path, root) + except (ValueError, TypeError): + return False + return not relative.startswith(".." + posixpath.sep) and relative != ".." def _relative_to(path: str, root: str) -> str: @@ -186,10 +184,6 @@ def _relative_to(path: str, root: str) -> str: Assumes :func:`_is_under` has already been checked. """ - rel = os.path.relpath(path, root) - # Strip leading "." for a child path (relpath returns "." when path - # equals root). When the path equals the root we get "" which is fine - # for callers that check for that separately. - if rel == ".": + if path == root: return "" - return rel.replace("\\", "/") + return path[len(root) + 1 :]