mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-07-08 11:56:59 +00:00
fix(agent): confine glob literal lookups to the search root (#5010)
GlobTool resolves its search root through _resolve_search_root (which confines it to the workspace or default allowlist), but the literal fast-path joined the model-supplied pattern onto that root without re-confining it. os.path.join lets an absolute pattern or one containing ../ escape the root, and normpath collapsed the .. segments, so glob returned the absolute path of arbitrary host files once they existed -- an existence/path oracle that bypasses the confinement read_file, write_file, grep, and ls all enforce. Keep the literal lookup inside the root via a commonpath containment check; an escaping literal falls through to the os.walk matcher, which only ever yields paths under the root. Wildcard matching was already confined.
This commit is contained in:
@@ -303,11 +303,26 @@ class GlobTool:
|
||||
base = os.path.abspath(root)
|
||||
if not os.path.isdir(base):
|
||||
return None, f"glob: {root}: not a directory"
|
||||
rbase = os.path.realpath(base)
|
||||
norm_pat = pattern.replace("\\", "/")
|
||||
# Fast path: literal pattern (no wildcards) → direct path lookup.
|
||||
if not any(c in norm_pat for c in "*?["):
|
||||
cand = os.path.normpath(os.path.join(base, norm_pat))
|
||||
if os.path.exists(cand):
|
||||
cand = os.path.realpath(os.path.join(base, norm_pat))
|
||||
# Keep the literal lookup inside the search root. os.path.join
|
||||
# lets an absolute pattern (or one containing ../) escape `base`,
|
||||
# which would turn glob into an existence/path oracle for
|
||||
# arbitrary host files — bypassing the workspace/allowlist
|
||||
# confinement that _resolve_search_root applies to the root.
|
||||
# An escaping literal falls through to the walk, which only ever
|
||||
# yields paths under base.
|
||||
nbase = os.path.normcase(rbase)
|
||||
try:
|
||||
inside = cand == rbase or os.path.commonpath(
|
||||
[os.path.normcase(cand), nbase]
|
||||
) == nbase
|
||||
except ValueError:
|
||||
inside = False
|
||||
if inside and os.path.exists(cand):
|
||||
return [cand], None
|
||||
# Literal not at exact path — fall through to walk so
|
||||
# e.g. "foo.py" still matches at any depth (like rglob).
|
||||
|
||||
@@ -140,6 +140,33 @@ async def test_grep_and_ls_confined_e2e(ws, admin):
|
||||
assert r["exit_code"] == 1 and "outside the workspace" in r["error"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_glob_confined_e2e(ws, admin):
|
||||
"""glob's literal fast-path must stay inside the workspace. A pattern with
|
||||
../ or an absolute path outside the root would otherwise leak the existence
|
||||
and full path of arbitrary host files (an oracle), even though read_file
|
||||
blocks reading them."""
|
||||
with open(os.path.join(ws, "found.py"), "w") as f:
|
||||
f.write("x")
|
||||
_, r = await execute_tool_block(_block("glob", json.dumps({"pattern": "found.py"})), owner="a", workspace=ws)
|
||||
assert r["exit_code"] == 0 and "found.py" in r["output"]
|
||||
|
||||
# a secret outside the workspace must not be discoverable via glob
|
||||
outside = tempfile.mkdtemp()
|
||||
secret = os.path.join(outside, "secret.txt")
|
||||
with open(secret, "w") as f:
|
||||
f.write("nope")
|
||||
# An escaping pattern must come back as "No files" (the not-found message),
|
||||
# not as a match that returns the file's path. The not-found message echoes
|
||||
# the pattern the model supplied, so the signal is the absence of a match,
|
||||
# not the absence of the path string.
|
||||
rel = os.path.relpath(secret, os.path.realpath(ws))
|
||||
_, r = await execute_tool_block(_block("glob", json.dumps({"pattern": rel})), owner="a", workspace=ws)
|
||||
assert r["exit_code"] == 0 and "No files" in r["output"] and secret not in r["output"]
|
||||
_, r = await execute_tool_block(_block("glob", json.dumps({"pattern": secret})), owner="a", workspace=ws)
|
||||
assert r["exit_code"] == 0 and "No files" in r["output"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_subprocess_cwd_is_workspace_e2e(ws, admin):
|
||||
"""python tool runs with cwd = workspace (OS-agnostic probe)."""
|
||||
|
||||
Reference in New Issue
Block a user