fix(s04,s20): resolve Gate2 safe_path conflict same as s03

s04_hooks: permission_hook checks path + asks user, but safe_path
still raised hard ValueError — user approval was ineffective, same
root cause as s03 (#482).

s20_comprehensive: permission_hook used safe_path directly inside
try/except, silently denying all writes outside workspace without
ever asking the user.  Now uses is_relative_to check + user prompt.

Both files also add read_file to the permission coverage.

s05-s08 intentionally NOT changed: their permission_hook does not
check paths at all — safe_path is their only path-safety defense.
This commit is contained in:
root
2026-07-28 17:54:48 +08:00
parent 4d8d420e41
commit 97b8541b36
2 changed files with 32 additions and 23 deletions

View File

@@ -377,13 +377,13 @@ def assemble_system_prompt(context: dict) -> str:
# ── Basic Tools ──
def safe_path(p: str, cwd: Path = None) -> Path:
# File tools stay inside the workspace or teammate worktree. Bash remains
# powerful on purpose and is controlled by the permission hook instead.
"""Convert a string path to a resolved Path relative to base.
NOTE: Path escape checking is now handled by permission_hook,
which asks the user for approval. safe_path only does str→Path conversion.
"""
base = cwd or WORKDIR
path = (base / p).resolve()
if not path.is_relative_to(base):
raise ValueError(f"Path escapes workspace: {p}")
return path
return (base / p).resolve()
def run_bash(command: str, cwd: Path = None,
@@ -401,7 +401,9 @@ def run_bash(command: str, cwd: Path = None,
def run_read(path: str, limit: int | None = None,
offset: int = 0, cwd: Path = None) -> str:
try:
lines = safe_path(path, cwd).read_text().splitlines()
base = cwd or WORKDIR
file_path = (base / path).resolve()
lines = file_path.read_text().splitlines()
offset = max(int(offset or 0), 0)
limit = int(limit) if limit is not None else None
lines = lines[offset:]
@@ -414,7 +416,8 @@ def run_read(path: str, limit: int | None = None,
def run_write(path: str, content: str, cwd: Path = None) -> str:
try:
fp = safe_path(path, cwd)
base = cwd or WORKDIR
fp = (base / path).resolve()
fp.parent.mkdir(parents=True, exist_ok=True)
fp.write_text(content)
return f"Wrote {len(content)} bytes to {path}"
@@ -425,7 +428,8 @@ def run_write(path: str, content: str, cwd: Path = None) -> str:
def run_edit(path: str, old_text: str, new_text: str,
cwd: Path = None) -> str:
try:
fp = safe_path(path, cwd)
base = cwd or WORKDIR
fp = (base / path).resolve()
text = fp.read_text()
if old_text not in text:
return f"Error: text not found in {path}"
@@ -909,12 +913,14 @@ def permission_hook(block):
choice = input(" Allow? [y/N] ").strip().lower()
if choice not in ("y", "yes"):
return "Permission denied by user"
if block.name in ("write_file", "edit_file"):
if block.name in ("read_file", "write_file", "edit_file"):
path = block.input.get("path", "")
try:
safe_path(path)
except Exception:
return f"Permission denied: path escapes workspace: {path}"
if not (WORKDIR / path).resolve().is_relative_to(WORKDIR):
print(f"\n\033[33m[permission] Access outside workspace\033[0m")
print(f" {block.name}: {path}")
choice = input(" Allow? [y/N] ").strip().lower()
if choice not in ("y", "yes"):
return "Permission denied by user"
if block.name.startswith("mcp__") and "deploy" in block.name:
print(f"\n\033[33m[permission] MCP destructive-looking tool: {block.name}\033[0m")
choice = input(" Allow? [y/N] ").strip().lower()