mirror of
https://github.com/rohitg00/ai-engineering-from-scratch.git
synced 2026-10-02 01:54:39 +08:00
fix: sandbox path jail bypassed by a slashless symlink (escape) (#249)
* fix: sandbox path jail bypassed by a slashless symlink (escape) 26-sandbox-runner-denylist: the path jail (_check_path_jail) only resolves and prefix-checks arguments for which _looks_like_path() is true, which requires a path separator (or exactly ./..). A symlink whose name has no slash — e.g. `link.txt` in the project root pointing at /etc/passwd — is therefore never jail-checked, so `cat link.txt` escapes the jail and reads files outside the project root, exactly what the module's advertised "symlink-safe path jail" is meant to prevent. (`sub/link.txt`, having a slash, is correctly denied; the asymmetry is the tell.) Also jail-check arguments that exist on disk under the root (os.path.lexists), so a slashless symlink is resolved and refused. lexists catches the symlink even when its target is missing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test: skip slashless-symlink jail test where symlinks are unsupported os.symlink raises OSError on Windows without Developer Mode/admin, which would fail this test spuriously in such CI. Skip instead of failing. Addresses CodeRabbit review feedback on the PR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
c2e9899b20
commit
56d3877d98
@@ -215,9 +215,13 @@ def _looks_like_path(arg: str) -> bool:
|
||||
def _check_path_jail(argv: Sequence[str], cfg: SandboxConfig) -> str | None:
|
||||
root = cfg.project_root
|
||||
for arg in argv[1:]:
|
||||
if not _looks_like_path(arg):
|
||||
if not arg or arg.startswith("-"):
|
||||
continue
|
||||
if arg.startswith("-"):
|
||||
# Also jail-check slashless names that exist under root: a symlink like
|
||||
# `link.txt` -> /etc/passwd has no path separator, so _looks_like_path
|
||||
# misses it, yet realpath still escapes the jail. lexists() catches the
|
||||
# symlink even when its target is missing.
|
||||
if not _looks_like_path(arg) and not os.path.lexists(os.path.join(root, arg)):
|
||||
continue
|
||||
# Resolve against root if arg is relative; let absolute paths stay absolute.
|
||||
candidate = arg
|
||||
|
||||
@@ -116,6 +116,23 @@ class PathJailChecks(unittest.TestCase):
|
||||
reason = _check_path_jail(["echo", "-n"], cfg)
|
||||
self.assertIsNone(reason)
|
||||
|
||||
def test_slashless_symlink_to_outside_is_denied(self) -> None:
|
||||
# A symlink whose name has no slash points outside the root. The module
|
||||
# advertises a "symlink-safe path jail via realpath prefix check", so the
|
||||
# jail must resolve and refuse it even though _looks_like_path misses it.
|
||||
root, cfg = _make_root()
|
||||
outside_dir = tempfile.mkdtemp(prefix="sandbox-outside-")
|
||||
secret = os.path.join(outside_dir, "secret.txt")
|
||||
with open(secret, "w", encoding="utf-8") as fh:
|
||||
fh.write("TOP-SECRET\n")
|
||||
try:
|
||||
os.symlink(secret, os.path.join(root, "link.txt"))
|
||||
except OSError:
|
||||
self.skipTest("symlinks not supported on this platform")
|
||||
reason = _check_path_jail(["cat", "link.txt"], cfg)
|
||||
self.assertIsNotNone(reason)
|
||||
self.assertIn("outside project root", reason)
|
||||
|
||||
|
||||
class TruncateTests(unittest.TestCase):
|
||||
def test_under_cap_not_truncated(self) -> None:
|
||||
|
||||
Reference in New Issue
Block a user