Skip to content

Commit 3af81fa

Browse files
authored
Merge pull request #59 from ApodexAI/fix/path-gate-bypass
fix(paths): make the path gate binding for in-process writes and shell commands
2 parents e8d664b + bc124df commit 3af81fa

6 files changed

Lines changed: 440 additions & 26 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,20 @@ Initial open-source release of FrontierAgent.
6262

6363
### Fixed
6464

65+
- **A write the path gate refused no longer reaches the host.** `write_file` and
66+
`file_editor_create` fall back to the sandbox when local authorization fails,
67+
and for the in-process `CurrentSandbox` that fallback is an `open()` in the
68+
harness (as root, in container mode), so a refused path was written anyway.
69+
That branch now authorizes the path itself; remote backends are unaffected.
70+
- **Paths are quoted before reaching a sandbox shell.** `file_editor`'s view /
71+
create / str_replace commands, `write_file`'s `mkdir -p`, and the Docker
72+
`mkdir -p` interpolated the path unquoted, so a path containing `;` ran
73+
commands that never passed the bash policy.
74+
- **A system directory named as the workspace root grants nothing.** The root
75+
arrives through `ExecutionScope` metadata (workload input), and
76+
`{"workspace_root": "/etc"}` made `/etc` readable and writable. System roots
77+
and direct children of `/usr` and `/etc` are refused; a run directory deep
78+
under `/var` or `/opt` (a container volume, macOS `$TMPDIR`) still works.
6579
- Bash policy: privilege escalation (`sudo`/`su`/…), remote/exfil clients
6680
(`ssh`/`nc`/`rsync`/…) and signal senders (`kill`/`pkill`/`killall`) are now
6781
refused in every allowlist mode, including the default `off`. The local CLI

‎plugins/tools/_path_auth.py‎

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,49 @@
4141
_WORD_SPLIT = re.compile(r"[^a-z0-9]+")
4242

4343

44+
#: Roots that can never BECOME a workspace: naming one as the workspace root
45+
#: would authorize the whole system tree for reads, and — outside the service
46+
#: checkout, which is the only thing ``_is_isolated_workspace_root`` refuses —
47+
#: for writes too. The workspace root arrives through ExecutionScope metadata,
48+
#: which carries workload input: a benchmark runner's trial directory is a
49+
#: legitimate value, ``/etc`` is not, and this module cannot tell which caller
50+
#: filled the key in. So the VALUE is constrained rather than the caller.
51+
#:
52+
#: A path may still live under one of these (a container's ``/var/lib/...``
53+
#: volume, ``/opt/app/run-17``): only naming the root itself, or a direct
54+
#: system subdirectory of it, is refused. ``/tmp`` is absent — a scratch
55+
#: workspace under it is ordinary — and so is ``/home``, whose per-user
56+
#: subdirectories are where a local CLI run actually lives.
57+
_NEVER_A_WORKSPACE_ROOT = frozenset({
58+
"/", "/etc", "/usr", "/bin", "/sbin", "/lib", "/lib64", "/boot", "/dev",
59+
"/proc", "/sys", "/var", "/root", "/opt", "/home",
60+
})
61+
62+
63+
#: Roots whose direct children are no more suitable than the root itself:
64+
#: ``/usr/local`` and ``/etc/cron.d`` grant exactly the writes that make naming
65+
#: them an escalation. ``/var`` and ``/opt`` are deliberately absent — a
66+
#: container volume at ``/var/lib/app/run`` and macOS ``$TMPDIR`` under
67+
#: ``/var/folders/...`` are real run directories, and refusing them is the
68+
#: over-denial that loses a run's deliverables.
69+
_NEVER_A_WORKSPACE_PARENT = frozenset({"/usr", "/etc", "/bin", "/sbin", "/boot", "/proc", "/sys"})
70+
71+
72+
def _is_system_root(candidate: Path) -> bool:
73+
"""Whether ``candidate`` is a system root, or a direct child of a system
74+
directory whose children are equally unsuitable."""
75+
# Compare canonical paths too: macOS maps /etc and /var into /private,
76+
# and other hosts may expose system directories through aliases.
77+
roots = {Path(root).resolve() for root in _NEVER_A_WORKSPACE_ROOT}
78+
parents = {Path(root).resolve() for root in _NEVER_A_WORKSPACE_PARENT}
79+
return candidate in roots or candidate.parent in parents
80+
81+
4482
def _configured_workspace_root() -> Path | None:
45-
"""Return the explicit workspace root, if one was configured for this task."""
83+
"""Return the explicit workspace root, if one was configured for this task.
84+
85+
The value is checked, not its source: see ``_NEVER_A_WORKSPACE_ROOT``.
86+
"""
4687
scope = get_current_execution_scope()
4788
metadata = scope.metadata if scope else {}
4889
raw_root = (
@@ -56,6 +97,12 @@ def _configured_workspace_root() -> Path | None:
5697
if not workspace_root.is_dir():
5798
logger.warning("Ignoring invalid CODING_WORKSPACE_ROOT '%s'", raw_root)
5899
return None
100+
if _is_system_root(workspace_root):
101+
logger.warning(
102+
"Refusing workspace root '%s': a system directory cannot be a "
103+
"workspace, so it grants no filesystem access", raw_root,
104+
)
105+
return None
59106
return workspace_root
60107

61108

‎plugins/tools/_sandbox.py‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3000,6 +3000,22 @@ def shell_quote(s: str) -> str:
30003000
return "'" + s.replace("'", "'\\''") + "'"
30013001

30023002

3003+
def _local_write_refusal(path: str) -> str:
3004+
"""Why the path gate refuses writing ``path`` on this host, or "".
3005+
3006+
Only consulted for the in-process write branch below. Imported lazily:
3007+
``_path_auth`` imports this module.
3008+
"""
3009+
try:
3010+
from plugins.tools._path_auth import _is_path_allowed
3011+
except Exception as exc: # pragma: no cover - broken install
3012+
return f"path authorization unavailable: {exc}"
3013+
allowed, reason = _is_path_allowed(path, write_access=True)
3014+
if allowed:
3015+
return ""
3016+
return reason or f"write to {path!r} is not authorized"
3017+
3018+
30033019
def sandbox_write_file(
30043020
sandbox: Any, path: str, content: str, *, mode: str = "w",
30053021
) -> tuple[bool, str]:
@@ -3021,9 +3037,19 @@ def sandbox_write_file(
30213037
# (128 KB), so routing deliverables through it fails with E2BIG somewhere
30223038
# around 96 KB of content — and container mode has no /tmp fallback, so the
30233039
# write becomes a hard error. The harness process already has the mount
3024-
# bound read-write and the path has passed ``_path_auth``; writing it
3025-
# directly is both unlimited and cheaper (no bwrap + python3 spawn).
3040+
# bound read-write, so writing it directly is both unlimited and cheaper
3041+
# (no bwrap + python3 spawn).
3042+
#
3043+
# This branch is an ordinary ``open()`` in the harness process — as root, in
3044+
# container mode — so it is NOT a sandbox boundary, and the path has to be
3045+
# authorized HERE. Its callers (``write_file``, ``file_editor_create``)
3046+
# reach the sandbox precisely when ``_path_auth`` refused a local write, and
3047+
# for a CurrentSandbox that made the refusal a detour rather than a denial:
3048+
# a path the gate rejected was written to the host anyway.
30263049
if isinstance(sandbox, CurrentSandbox):
3050+
reason = _local_write_refusal(path)
3051+
if reason:
3052+
return False, reason
30273053
try:
30283054
p = Path(path)
30293055
new_parent = not p.parent.exists()
@@ -3051,7 +3077,7 @@ def sandbox_write_file(
30513077
if mode == "w" and isinstance(sandbox, DockerSandbox):
30523078
parent = os.path.dirname(path)
30533079
if parent:
3054-
sandbox.commands.run(f"mkdir -p {parent}", timeout=10)
3080+
sandbox.commands.run(f"mkdir -p {shell_quote(parent)}", timeout=10)
30553081
with tempfile.NamedTemporaryFile(
30563082
mode="w", encoding="utf-8", delete=False, suffix=".tmp",
30573083
) as f:

‎plugins/tools/file_editor.py‎

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,17 @@
33
from __future__ import annotations
44

55
import logging
6+
import re
67

78
from frontier_agent.core.tool import tool
9+
from plugins.tools import _sandbox
810
from plugins.tools._deliverable_policy import output_write_error
911
from plugins.tools._path_auth import _authorized_local_path
1012
from plugins.tools._sandbox import (
1113
aget_sandbox,
1214
arun_sandbox_cmd,
1315
asandbox_write_file,
16+
shell_quote,
1417
)
1518

1619
logger = logging.getLogger(__name__)
@@ -39,6 +42,16 @@ async def file_editor_view(path: str, view_range: str = "") -> str:
3942
if not path or not path.strip():
4043
return "Error: path is required."
4144

45+
line_range: tuple[int, int] | None = None
46+
if view_range:
47+
match = re.fullmatch(r"\s*([0-9]+)\s*-\s*([0-9]+)\s*", view_range)
48+
if match is None:
49+
return "Error: view_range must be a positive line range like 1-50."
50+
start, end = int(match[1]), int(match[2])
51+
if start < 1 or end < start:
52+
return "Error: view_range must be a positive line range like 1-50."
53+
line_range = start, end
54+
4255
local_path, _reason = _authorized_local_path(path)
4356
if local_path is not None:
4457
from plugins.tools.ignore_rules import discover_repo_root, should_ignore_path
@@ -64,12 +77,7 @@ async def file_editor_view(path: str, view_range: str = "") -> str:
6477
lines = local_path.read_text(encoding="utf-8").splitlines()
6578
except Exception as e:
6679
return f"Error viewing {path}: {e}"
67-
if view_range and "-" in view_range:
68-
start_s, end_s = view_range.split("-", 1)
69-
start = max(int(start_s.strip() or "1"), 1)
70-
end = max(int(end_s.strip() or str(start)), start)
71-
else:
72-
start, end = 1, len(lines)
80+
start, end = line_range if line_range is not None else (1, len(lines))
7381
rendered = "\n".join(f"{idx}: {line}" for idx, line in enumerate(lines[start - 1:end], start=start))
7482
return _truncate(rendered or "(empty file)")
7583

@@ -78,33 +86,33 @@ async def file_editor_view(path: str, view_range: str = "") -> str:
7886
except RuntimeError as e:
7987
return f"Error: {e}"
8088

89+
# Every path below is interpolated into a shell command, so it is quoted
90+
# once here: an unquoted path containing ``;`` or ``$(…)`` would run
91+
# arbitrary commands in the sandbox without passing the bash policy.
92+
q = shell_quote(path)
8193
try:
8294
check = await arun_sandbox_cmd(
8395
sandbox,
84-
f"test -d {path} && echo DIR || echo FILE", timeout=10,
96+
f"test -d {q} && echo DIR || echo FILE", timeout=10,
8597
)
8698
is_dir = "DIR" in check.stdout
8799

88100
if is_dir:
89101
result = await arun_sandbox_cmd(
90102
sandbox,
91-
f"find {path} -maxdepth 2 -not -path '*/.git/*' "
103+
f"find {q} -maxdepth 2 -not -path '*/.git/*' "
92104
f"-not -path '*/node_modules/*' -not -path '*/__pycache__/*' "
93105
f"| head -100 | sort",
94106
timeout=15,
95107
)
96108
return _truncate(result.stdout or f"(empty directory: {path})")
97109

98110
# File view
99-
if view_range:
100-
parts = view_range.split("-")
101-
if len(parts) == 2:
102-
start, end = parts[0].strip(), parts[1].strip()
103-
cmd = f"sed -n '{start},{end}p' {path} | cat -n"
104-
else:
105-
cmd = f"cat -n {path}"
111+
if line_range is not None:
112+
start, end = line_range
113+
cmd = f"sed -n {shell_quote(f'{start},{end}p')} -- {q} | cat -n"
106114
else:
107-
cmd = f"cat -n {path}"
115+
cmd = f"cat -n -- {q}"
108116

109117
result = await arun_sandbox_cmd(sandbox, cmd, timeout=15)
110118

@@ -154,9 +162,9 @@ async def file_editor_create(path: str, content: str) -> str:
154162
try:
155163
import os
156164
parent = os.path.dirname(path)
157-
if parent:
165+
if parent and not isinstance(sandbox, _sandbox.CurrentSandbox):
158166
await arun_sandbox_cmd(
159-
sandbox, f"mkdir -p {parent}", timeout=10,
167+
sandbox, f"mkdir -p {shell_quote(parent)}", timeout=10,
160168
)
161169

162170
# Write via python in sandbox (avoids files.write permission issues)
@@ -165,7 +173,7 @@ async def file_editor_create(path: str, content: str) -> str:
165173
return f"Error creating {path}: {err}"
166174

167175
result = await arun_sandbox_cmd(
168-
sandbox, f"wc -l < {path}", timeout=5,
176+
sandbox, f"wc -l < {shell_quote(path)}", timeout=5,
169177
)
170178
lines = result.stdout.strip() if result.stdout else "?"
171179

@@ -232,7 +240,9 @@ async def file_editor_str_replace(path: str, old_str: str, new_str: str) -> str:
232240

233241
try:
234242
# Read current content via cat (more reliable than files.read for all paths)
235-
result = await arun_sandbox_cmd(sandbox, f"cat {path}", timeout=10)
243+
result = await arun_sandbox_cmd(
244+
sandbox, f"cat -- {shell_quote(path)}", timeout=10,
245+
)
236246
if result.exit_code != 0:
237247
return f"Error: Cannot read {path}: {result.stderr or 'file not found'}"
238248
content = result.stdout

‎plugins/tools/write_file.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import os
1111

1212
from frontier_agent.core.tool import tool
13+
from plugins.tools import _sandbox
1314
from plugins.tools._deliverable_policy import output_write_error
1415
from plugins.tools._path_auth import _authorized_local_path
1516
from plugins.tools._sandbox import (
@@ -19,6 +20,7 @@
1920
resolve_runtime_path,
2021
resolve_sandbox_mode,
2122
sandbox_available,
23+
shell_quote,
2224
)
2325

2426
logger = logging.getLogger(__name__)
@@ -73,9 +75,9 @@ async def write_file(path: str, content: str, append: bool = False) -> str:
7375
try:
7476
sandbox = await aget_sandbox()
7577
parent = os.path.dirname(path)
76-
if parent:
78+
if parent and not isinstance(sandbox, _sandbox.CurrentSandbox):
7779
await arun_sandbox_cmd(
78-
sandbox, f"mkdir -p {parent}", timeout=10,
80+
sandbox, f"mkdir -p {shell_quote(parent)}", timeout=10,
7981
)
8082

8183
mode = "a" if append else "w"

0 commit comments

Comments
 (0)