mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-18 16:02:10 +00:00
fix(agents): keep redirects and chained writes off the allow path
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -238,7 +238,7 @@ split_segments "$cmd"
|
||||
seg_cwd="${cwd:-$PWD}"
|
||||
alt_cwd="" # where a `cd` that failed would have left the command
|
||||
saw_cd=0 # a `cd` moved the working directory somewhere
|
||||
proved=0 # at least one op came out inside a single root
|
||||
proved=0 # how many ops came out inside a single root
|
||||
only_ours=1 # ... and nothing else shares the command line
|
||||
|
||||
for seg in "${SEGMENTS[@]}"; do
|
||||
@@ -247,12 +247,12 @@ for seg in "${SEGMENTS[@]}"; do
|
||||
"") continue ;;
|
||||
mkdir | cp | mv | touch | chmod)
|
||||
check_fileops_segment "${SEG_TOKS[0]}"
|
||||
proved=1
|
||||
proved=$((proved + 1))
|
||||
continue
|
||||
;;
|
||||
tar | unzip)
|
||||
check_archive_segment "${SEG_TOKS[0]}"
|
||||
proved=1
|
||||
proved=$((proved + 1))
|
||||
continue
|
||||
;;
|
||||
cd)
|
||||
@@ -265,7 +265,10 @@ for seg in "${SEGMENTS[@]}"; do
|
||||
alt_cwd="$seg_cwd"
|
||||
seg_cwd="$new_cwd"
|
||||
else
|
||||
# Not the harmless segment an allow assumes: whatever this guard could not account for
|
||||
# may be a redirect, and a redirect writes. Leave the line to the normal flow.
|
||||
seg_cwd="" alt_cwd=""
|
||||
only_ours=0
|
||||
fi
|
||||
saw_cd=1
|
||||
continue
|
||||
@@ -279,6 +282,11 @@ for seg in "${SEGMENTS[@]}"; do
|
||||
only_ours=0
|
||||
done
|
||||
|
||||
[ "$proved" = 1 ] || exit 0
|
||||
[ "$only_ours" = 1 ] && decide allow "every path operand is under /tmp"
|
||||
# Exactly one write per line. Each segment is proved against the filesystem as it stands now,
|
||||
# and an earlier write can change what a later operand means: `cp -r /tmp/tree /tmp/live` that
|
||||
# recreates a symlink out of /tmp turns `/tmp/live/link` — a path under /tmp when this ran —
|
||||
# into a write through that symlink. Deletes compose safely and guard-rm-outside-tmp.sh allows
|
||||
# several, because `rm` unlinks a symlink rather than following it.
|
||||
[ "$proved" -ge 1 ] || exit 0
|
||||
[ "$only_ours" = 1 ] && [ "$proved" = 1 ] && decide allow "every path operand is inside a single root"
|
||||
exit 0
|
||||
|
||||
@@ -122,7 +122,10 @@ for seg in "${SEGMENTS[@]}"; do
|
||||
alt_cwd="$seg_cwd"
|
||||
seg_cwd="$new_cwd"
|
||||
else
|
||||
# Not the harmless segment an allow assumes: whatever this guard could not account for
|
||||
# may be a redirect, and a redirect writes. Leave the line to the normal flow.
|
||||
seg_cwd="" alt_cwd=""
|
||||
only_ours=0
|
||||
fi
|
||||
saw_cd=1
|
||||
continue
|
||||
|
||||
@@ -131,6 +131,7 @@ run $G ask "cd /tmp/does-not-exist; rm -rf backend/.env"
|
||||
run $G ask "cd /tmp/a && cd /tmp/b && rm -rf sub"
|
||||
run $G ask "rm -rf /tmp/clone/.git" # history is never in a class
|
||||
run $G ask "rm -rf /tmp/scratch/id_rsa.key"
|
||||
run $G none "cd /tmp >$OUT; rm -f /tmp/a"
|
||||
|
||||
echo
|
||||
echo "== allow-fileops-in-tmp.sh =="
|
||||
@@ -154,8 +155,8 @@ run $A none "cp $CWD/AGENTS.md /tmp/a"
|
||||
run $A none "tar -xzf /tmp/a.tar.gz -C $OUT"
|
||||
run $A none "cargo build"
|
||||
|
||||
run $A allow "mkdir -p /tmp/x; mv /tmp/a /tmp/x; chmod 755 /tmp/x"
|
||||
run $A allow "$(printf 'mv /tmp/a /tmp/b\nchmod 755 /tmp/b')"
|
||||
run $A none "mkdir -p /tmp/x; mv /tmp/a /tmp/x; chmod 755 /tmp/x" # one write per line
|
||||
run $A none "$(printf 'mv /tmp/a /tmp/b\nchmod 755 /tmp/b')"
|
||||
run $A ask "ls && mv /tmp/a /etc"
|
||||
run $A ask "$(printf 'mkdir -p /tmp/x\nchmod -R 777 %s' "$CWD")"
|
||||
run $A allow "cd /tmp/x && tar -xzf /tmp/a.tar.gz -C /tmp/out"
|
||||
@@ -175,6 +176,10 @@ run $A none "cp backend/secrets/token frontend/token.txt" # cp has no prompt
|
||||
# so what matters is it is not allowed
|
||||
run $A ask "mv $CWD/backend/credentials.json /tmp/x"
|
||||
run $A ask "cd /tmp/does-not-exist; mv .claude/settings.json settings.bak"
|
||||
# A segment this hook cannot read whole may carry a redirect, and an earlier write can change
|
||||
# what a later operand resolves to — neither may ride along on an allow.
|
||||
run $A none "cd /tmp >$OUT; mv /tmp/a /tmp/b"
|
||||
run $A none "cp -r /tmp/tree /tmp/live; cp /tmp/payload /tmp/live/link"
|
||||
|
||||
echo
|
||||
[ "$fails" = 0 ] && echo "ALL PASS" || { echo "$fails FAILURES"; exit 1; }
|
||||
|
||||
Reference in New Issue
Block a user