Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions libs/deepagents/deepagents/backends/sandbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -949,6 +949,9 @@ def delete(self, file_path: str) -> DeleteResult:
# boundary: it does not confine the deletion to any sandbox root or
# block traversal. Whatever the sandbox shell can reach, this can delete.
quoted = shlex.quote(file_path)
exists = self.execute(f"test -e {quoted} || test -L {quoted}")
if exists.exit_code != 0:
return DeleteResult(error=f"Error: '{file_path}' not found")
result = self.execute(f"rm -rf {quoted}")

if result.exit_code == 0:
Expand Down
43 changes: 42 additions & 1 deletion libs/deepagents/deepagents/middleware/filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
execute_accepts_timeout,
)
from deepagents.backends.utils import (
_GLOB_WILDCARD_CHARS,
_get_file_type,
_glob_anchor,
_paths_overlap,
Expand Down Expand Up @@ -162,7 +163,47 @@ def _find_delete_deny_patterns(rules: list[FilesystemPermission], target: str) -
if rule.mode != "deny" or "write" not in rule.operations:
continue
for pattern in rule.paths:
if pattern not in seen and _paths_overlap(target, _glob_anchor(pattern)):
if pattern in seen:
continue
anchor = _glob_anchor(pattern)
if any(c in _GLOB_WILDCARD_CHARS for c in pattern):
# Wildcard pattern — three cases in priority order:
# 1. Root anchor ("/**/x"): pattern can match anywhere, block all.
# 2. Target directly matches the glob: block.
# 3. Anchor is inside the delete subtree (anchor.is_relative_to(target)):
# recursive delete would remove matching descendants — block.
# 4. Target is below the anchor (target.is_relative_to(anchor)):
# safe to allow ONLY when the pattern suffix is a single,
# non-** component (fixed depth). "/work/*.log" can never match
# anything under "/work/notes.txt". Patterns with directory
# wildcards after the anchor ("/work/*/secrets",
# "/work/**/secrets") could match descendants of the target,
# so we fail closed for those.
if (
anchor == "/"
or wcglob.globmatch(target, pattern, flags=_FS_WCMATCH_FLAGS)
Comment thread
imnishitha marked this conversation as resolved.
Resolved
or PurePosixPath(anchor).is_relative_to(PurePosixPath(target))
Comment thread
imnishitha marked this conversation as resolved.
Resolved
):
overlaps = True
elif PurePosixPath(target).is_relative_to(PurePosixPath(anchor)):
# Target is below anchor. Safe to allow only when the pattern
# suffix is a single non-** component (fixed depth): a file glob
# like "*.log" can never match anything under a sibling target.
# Patterns with directory wildcards ("/work/*/secrets") could
# match descendants of target, so fail closed for those.
anchor_parts = PurePosixPath(anchor).parts
pattern_parts = PurePosixPath(pattern).parts
suffix = pattern_parts[len(anchor_parts) :]
single_filename_glob = len(suffix) == 1 and "**" not in suffix[0]
overlaps = not single_filename_glob

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new single-component wildcard exemption only checks whether target itself matches pattern, then allows all descendants below the anchor when the suffix has one component:

elif PurePosixPath(target).is_relative_to(PurePosixPath(anchor)):
    anchor_parts = PurePosixPath(anchor).parts
    pattern_parts = PurePosixPath(pattern).parts
    suffix = pattern_parts[len(anchor_parts) :]
    single_filename_glob = len(suffix) == 1 and "**" not in suffix[0]
    overlaps = not single_filename_glob

This bypasses deny rules for wildcard-matched directories. For example, with a deny rule /work/secret-*, deleting /work/secret-prod/config.yml does not directly match the glob, falls into this branch, and sets overlaps = False, even though the file is inside a denied path.

Remediation: Before allowing the single-component wildcard case, check whether the immediate child under the anchor matches the deny pattern. If an ancestor such as /work/secret-prod matches /work/secret-*, treat the delete as overlapping and deny it.

For more details, see the finding in Corridor.

Provide feedback: Reply with whether this is a valid vulnerability or false positive to help improve Corridor's accuracy.

else:
# No subtree relationship at all — unrelated paths.
overlaps = False
else:
# Literal pattern (no wildcards): keep the original subtree-overlap
# check so that a deny on "/work" blocks deletes of "/work/sub".
overlaps = _paths_overlap(target, anchor)
if overlaps:
seen.add(pattern)
denying.append(pattern)
return denying
Expand Down
39 changes: 23 additions & 16 deletions libs/deepagents/tests/unit_tests/backends/test_sandbox_backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ def __init__(self) -> None:
self._next_exit_code: int = 0
self._uploaded: list[tuple[str, bytes]] = []
self._file_store: dict[str, bytes] = {}
self._responses: list[tuple[str, int]] = []

@property
def id(self) -> str:
Expand All @@ -57,10 +58,13 @@ def execute(self, command: str, *, timeout: int | None = None) -> ExecuteRespons
has_tmp = any(".deepagents_edit_" in k for k in self._file_store)
if "old_path = base64.b64decode(" in command and has_tmp:
return self._simulate_edit_tmpfile(command)
output = self._next_output
exit_code = self._next_exit_code
self._next_output = "1"
self._next_exit_code = 0
if self._responses:
output, exit_code = self._responses.pop(0)
else:
output = self._next_output
exit_code = self._next_exit_code
self._next_output = "1"
self._next_exit_code = 0
return ExecuteResponse(output=output, exit_code=exit_code, truncated=False)

def _simulate_edit_tmpfile(self, command: str) -> ExecuteResponse:
Expand Down Expand Up @@ -1572,31 +1576,34 @@ def test_delete_directory_uses_recursive_rm(self) -> None:
assert "rm -rf" in sandbox.last_command
assert "/some/dir" in sandbox.last_command

def test_delete_missing_is_noop_success(self) -> None:
# `rm -f` ignores a missing path: exit 0, so delete reports success.
def test_delete_missing_returns_not_found(self) -> None:
# `test -e` exits 1 for a missing path; delete must return a not-found error.
sandbox = MockSandbox()
sandbox._next_output = ""
sandbox._next_exit_code = 0
sandbox._next_exit_code = 1 # test -e reports path absent
result = sandbox.delete("/missing.txt")
assert result.error is None
assert result.path == "/missing.txt"
assert result.path is None
assert result.error is not None
assert "not found" in result.error

def test_delete_failure_reports_output(self) -> None:
# A non-zero exit (e.g. a permission error) surfaces rm's stderr.
# A non-zero exit from rm (e.g. a permission error) surfaces rm's stderr.
sandbox = MockSandbox()
sandbox._next_output = "rm: cannot remove '/some/dir': Is a directory"
sandbox._next_exit_code = 1
# Queue: test -e succeeds (file exists), then rm -rf fails with output.
sandbox._responses = [
("", 0),
("rm: cannot remove '/some/dir': Is a directory", 1),
]
result = sandbox.delete("/some/dir")
assert result.path is None
assert result.error is not None
assert "Error deleting file" in result.error
assert "Is a directory" in result.error

def test_delete_failure_unknown_error(self) -> None:
# Non-zero exit with no output falls back to a generic message.
# Non-zero exit from rm with no output falls back to a generic message.
sandbox = MockSandbox()
sandbox._next_output = ""
sandbox._next_exit_code = 1
# Queue: test -e succeeds (file exists), then rm -rf fails silently.
sandbox._responses = [("", 0), ("", 1)]
result = sandbox.delete("/file.txt")
assert result.path is None
assert result.error is not None
Expand Down
7 changes: 7 additions & 0 deletions libs/deepagents/tests/unit_tests/test_permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,13 @@ class TestFindDeleteDenyPatterns:
pytest.param("/workshop/**", "/work", [], id="sibling-prefix-glob"),
pytest.param("/work2", "/work", [], id="sibling-prefix-literal"),
pytest.param("/work/secrets", "/work/logs", [], id="sibling-leaf"),
# Single-component file glob: non-matching siblings are allowed
pytest.param("/work/*.log", "/work/notes.txt", [], id="file-glob-does-not-block-non-matching-sibling"),
# Directory wildcard after anchor: target below anchor is blocked (fail closed)
pytest.param("/work/*/secrets", "/work/app", ["/work/*/secrets"], id="dir-wildcard-blocks-ancestor-target"),
pytest.param("/work/**/secrets", "/work/app", ["/work/**/secrets"], id="globstar-wildcard-blocks-ancestor-target"),
# Recursive glob with suffix: deleting /work/sub would remove /work/sub/a.log
pytest.param("/work/**/*.log", "/work/sub", ["/work/**/*.log"], id="recursive-glob-blocks-descendant-that-contains-match"),
# --- overlap in either direction -> blocked ----------------------
pytest.param("/work/a.txt", "/work/a.txt", ["/work/a.txt"], id="exact-file"),
pytest.param("/work", "/work", ["/work"], id="exact-dir-literal"),
Expand Down