Skip to content

Commit ecf397e

Browse files
LivXueclaude
andcommitted
fix(agent): let pushd and popd move the walk the way they move the shell
`pushd ..; cat outside.txt` read the file. The walk asked whether a segment's first word was `cd`, so the builtin that moves the shell exactly as far, and is on every interactive user's fingers, stepped over the fence untouched. Its destination is now checked the same way `cd`'s is. The stack is what the walk cannot follow. `popd` and a bare `pushd` take their destination from entries an earlier push put there, and each of those was cleared on the way in, so the shallowest place either can land is where the command began. The walk adds that place rather than guessing which one, which is the same answer a separator that proves nothing already gets. Co-authored-by: Claude (claude-opus-5[1m]) <noreply@anthropic.com>
1 parent a131dd1 commit ecf397e

2 files changed

Lines changed: 43 additions & 1 deletion

File tree

‎raven/agent/tools/shell.py‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -468,6 +468,12 @@ def _steps_outside(
468468
nothing -- and after any of them, every relative path in the rest of
469469
the command resolves somewhere the operator did not allow.
470470
471+
``pushd`` moves the shell exactly as far and is read the same way. Its
472+
stack is what this cannot follow: ``popd`` and a bare ``pushd`` take
473+
their destination from entries earlier pushes already cleared, so the
474+
walk falls back to the shallowest of those, which is where the command
475+
began.
476+
471477
Each `cd` starts from where the last one landed, so
472478
`cd subdir && cd ..` ends where it began rather than being read as
473479
leaving. Which is knowable only where the separator says so: `&&` runs
@@ -490,7 +496,13 @@ def _steps_outside(
490496
# segment with the bracket already gone, and the bracket is now
491497
# one more separator that proves nothing.
492498
tokens = _unwrap_command_wrappers(raw_tokens)
493-
if not tokens or tokens[0] != "cd":
499+
if not tokens or tokens[0] not in ("cd", "pushd", "popd"):
500+
continue
501+
if tokens[0] == "popd":
502+
# A stack return lands on a directory some earlier push put
503+
# there, and each of those was cleared on the way in. The
504+
# shallowest of them is where the command began.
505+
here = cls._either(here, [cwd])
494506
continue
495507
arguments = list(tokens[1:])
496508
while arguments and arguments[0] in ("-L", "-P"):
@@ -503,6 +515,12 @@ def _steps_outside(
503515
elif arguments and arguments[0] == "-":
504516
# ``$OLDPWD`` is a directory some earlier ``cd`` already passed.
505517
continue
518+
if not arguments and tokens[0] == "pushd":
519+
# A bare `pushd` swaps the top two entries rather than naming a
520+
# destination. Both were cleared on the way in, so the worst
521+
# case is the one `popd` gets.
522+
here = cls._either(here, [cwd])
523+
continue
506524
target = arguments[0] if arguments else env.get("HOME", "")
507525
if not target:
508526
continue

‎tests/test_shell_workspace_fence.py‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -734,6 +734,30 @@ def test_a_proven_walk_survives_a_separator_later_in_the_command(fenced: ExecToo
734734
assert refusal(fenced, command) is None
735735

736736

737+
@pytest.mark.parametrize(
738+
"command",
739+
["pushd ..; ls", "pushd /; cat etc/passwd", "pushd -- ..; ls"],
740+
ids=["up", "root", "after-terminator"],
741+
)
742+
def test_pushd_leaves_by_the_same_door_as_cd(fenced: ExecTool, command: str) -> None:
743+
"""`pushd` is a `cd` that also remembers where it was.
744+
745+
The walk read `cd` alone, so the builtin that moves the shell exactly as
746+
far, and is on every interactive user's fingers, stepped over the fence
747+
untouched. Its destination is checked the same way; what the stack does
748+
afterwards is why the walk stops carrying (see the test below).
749+
"""
750+
assert refusal(fenced, command) is not None
751+
752+
753+
def test_a_stack_return_stops_the_walk_from_carrying(fenced: ExecTool) -> None:
754+
"""`popd` and a bare `pushd` take their destination off a stack this
755+
cannot see, so a walk carried past one would be guessing. The stricter
756+
reading holds instead, the same answer a bracket already gets.
757+
"""
758+
assert refusal(fenced, "pushd subdir && popd && cd .. && ls") is not None
759+
760+
737761
def test_a_subshell_makes_the_walk_strict_again(fenced: ExecTool) -> None:
738762
"""A `cd` inside a subshell does not outlive it, and the splitter has
739763
already dropped the bracket that said so. Carrying the directory forward

0 commit comments

Comments
 (0)