mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-18 08:01:26 +00:00
fix(agents): stop the scratch-dir guards prompting on quoted text (#10703)
* fix(agents): stop the scratch-dir guards prompting on quoted text * fix(agents): keep prompting past wrapper flags and quoted heredoc markers * fix(agents): only treat a line-ending delimiter as a heredoc opener * fix(agents): refuse a heredoc opener whose redirect carries a quote * fix(agents): stop the wrapper scan at a quoted word instead of a word count * fix(agents): scan a wrapper's operands to the end of the segment * fix(agents): scan a heredoc body that is piped into a shell * fix(agents): only treat a quoted, unexecuted heredoc body as data * fix(agents): split separators before looking for the shell running a heredoc * fix(agents): require a reading consumer before treating a body as data
This commit is contained in:
Ruben Fiszel
GitHub
@@ -15,13 +15,106 @@ set -f
|
||||
# the command splits on `; & |` and newlines, and a leading env assignment or process wrapper
|
||||
# (`timeout 5 rm`, `xargs rm`) is skipped before the command word is read.
|
||||
#
|
||||
# The split set also carries the characters that open a nested command — `$(`, backticks, `( )`
|
||||
# and `{ }` — because a rule matches the verb inside one (`echo $(rm -rf ~)` prompts), and a
|
||||
# separator that only ends statements would read that as an `echo`.
|
||||
#
|
||||
# The scan is textual, so a heredoc body that merely contains the verb (`cat > s.sh <<EOF` …)
|
||||
# reads as a command and prompts. Left that way on purpose: parsing heredocs to suppress it
|
||||
# would risk dropping a real trailing command, and an extra prompt is the safe failure.
|
||||
# The split set also carries the characters that open a nested command — `$(`, backticks and
|
||||
# `( )` — because a rule matches the verb inside one (`echo $(rm -rf ~)` prompts), and a
|
||||
# separator that only ends statements would read that as an `echo`. Braces are handled as
|
||||
# words rather than separators, since splitting on them cuts `xargs -I {} … rm` in half and
|
||||
# strands the `rm` in a segment that no longer knows a wrapper preceded it.
|
||||
|
||||
# 0 iff <text> ($1) starts with a command that only reads its input. An allowlist, because the
|
||||
# opposite — naming the shells to avoid — would have to be complete: an unlisted one (`ash`,
|
||||
# `rbash`, `busybox sh`) executes the body while the guard calls it data. Unrecognized here only
|
||||
# costs a prompt. Text with no command word in it is not evidence of a reader either.
|
||||
reads_only() {
|
||||
local w
|
||||
for w in $1; do
|
||||
w="${w//[\"\'\\]/}"
|
||||
w="${w%%<<*}" # a redirect needs no space: `cat<<EOF`
|
||||
case "$w" in "" | -* | *=* | [0-9]* | '>'* | '<'*) continue ;; esac
|
||||
case "${w##*/}" in
|
||||
cat | tee | head | tail | grep | sed | awk | sort | uniq | wc | cut | diff | tr \
|
||||
| jq | yq | gh | git | base64 | column | envsubst | python | python3 | node \
|
||||
| psql | mysql | sqlite3 | wmill) return 0 ;;
|
||||
esac
|
||||
return 1
|
||||
done
|
||||
return 1
|
||||
}
|
||||
|
||||
# A heredoc body is data rather than commands only when its delimiter is quoted and nothing
|
||||
# executes it; a rule doesn't match a verb inside such a body, and a PR body would otherwise
|
||||
# prompt for every `rm` in its text. Dropping one needs all of that, a delimiter that could
|
||||
# really open a heredoc, and a terminator line — failing any part, nothing is dropped.
|
||||
strip_heredoc_bodies() {
|
||||
local -a lines=()
|
||||
local line delim rest after trimmed piped quoted i j n
|
||||
while IFS= read -r line; do lines+=("$line"); done <<< "$1"
|
||||
n=${#lines[@]}
|
||||
i=0
|
||||
while [ "$i" -lt "$n" ]; do
|
||||
line="${lines[$i]}"
|
||||
printf '%s\n' "$line"
|
||||
i=$((i + 1))
|
||||
# A `#` opens a comment, and a comment opens no heredoc — including mid-line, as in
|
||||
# `echo hi # cat <<EOF`. Cutting there also discards a `#` that is really part of a word or
|
||||
# a string, which at worst leaves a real body to be scanned: an extra prompt, never a lost one.
|
||||
line="${line%%'#'*}"
|
||||
case "$line" in *'<<'*) ;; *) continue ;; esac
|
||||
rest="${line#*<<}"
|
||||
rest="${rest#-}" # <<- strips leading tabs from the body
|
||||
rest="${rest#"${rest%%[![:space:]]*}"}"
|
||||
delim="${rest%%[[:space:]]*}"
|
||||
# Whatever follows the delimiter word decides whether this line could open a heredoc at
|
||||
# all. Only a redirect or a pipe can (`cat <<EOF > f`); prose after it means the `<<` sits
|
||||
# inside a string (`echo "cat <<EOF and more"`), and dropping down to a line that happens
|
||||
# to match would discard the real commands in between. A quote anywhere in the remainder
|
||||
# says the same thing, since `echo "cat <<EOF > f"` ends its redirect-looking text with the
|
||||
# closing quote. That also refuses `cat <<EOF > "f"`, a real heredoc, which only over-prompts.
|
||||
after="${rest#"$delim"}"
|
||||
after="${after#"${after%%[![:space:]]*}"}"
|
||||
case "$after" in
|
||||
*[\"\'\\]*) continue ;;
|
||||
"" | '>'* | '<'* | '|'* | [0-9]'>'* | [0-9]'<'*) ;;
|
||||
*) continue ;;
|
||||
esac
|
||||
# A real delimiter is a bare word or one wholly quoted (`<<'EOF'`, `<<\EOF`); a stray quote
|
||||
# left in it means the `<<` was quoted prose.
|
||||
quoted=0
|
||||
case "$delim" in
|
||||
\'*\' | \"*\") delim="${delim:1:${#delim}-2}" quoted=1 ;;
|
||||
\\?*) delim="${delim#\\}" quoted=1 ;;
|
||||
esac
|
||||
case "$delim" in
|
||||
[A-Za-z_]*) ;;
|
||||
*) continue ;;
|
||||
esac
|
||||
case "$delim" in *[!A-Za-z0-9_]*) continue ;; esac
|
||||
# Only a quoted delimiter makes the body inert. Unquoted, the shell expands it before the
|
||||
# consumer ever sees it, so a `$(rm -rf ~)` written in the body runs whatever reads it.
|
||||
[ "$quoted" = 1 ] || continue
|
||||
# Two commands can see this body: the one the `<<` belongs to, and anything it is then piped
|
||||
# into. The first is whatever was started last before the `<<`, so splitting the text there
|
||||
# on separators and substitution openers and taking the final piece finds `cat` in
|
||||
# `--title "fix(agents): …" --body "$(cat <<`, without the title's parenthesis standing in
|
||||
# for it. A line continuation (`bash \` then `<<'EOF'`) leaves that piece empty, which is
|
||||
# not evidence of a reader and so keeps the body.
|
||||
reads_only "$(printf '%s' "${line%%<<*}" | tr ';&|()`' '\n' | grep -v '^[[:space:]]*$' | tail -1)" || continue
|
||||
piped="$after"
|
||||
while :; do
|
||||
case "$piped" in *'|'*) ;; *) break ;; esac
|
||||
piped="${piped#*|}"
|
||||
reads_only "${piped%%|*}" || continue 2
|
||||
done
|
||||
j="$i"
|
||||
while [ "$j" -lt "$n" ]; do
|
||||
trimmed="${lines[$j]#"${lines[$j]%%[![:space:]]*}"}"
|
||||
[ "$trimmed" = "$delim" ] && break
|
||||
j=$((j + 1))
|
||||
done
|
||||
[ "$j" -lt "$n" ] && i=$((j + 1))
|
||||
done
|
||||
}
|
||||
|
||||
runs_verb() {
|
||||
local verb="$1" seg w wrapped
|
||||
while IFS= read -r seg; do
|
||||
@@ -33,19 +126,24 @@ runs_verb() {
|
||||
case "$w" in
|
||||
"$verb" | */"$verb") return 0 ;;
|
||||
*=*) ;; # leading env assignment
|
||||
-* | [0-9]*) ;; # a wrapper's own flag, or its duration
|
||||
*'>'* | *'<'*) ;; # leading redirect, `>/dev/null rm ...`
|
||||
'!' | if | then | elif | else | while | until | do) ;; # keywords, never the command
|
||||
timeout | time | nice | nohup | stdbuf | command | builtin | noglob | xargs | sudo | env) wrapped=1 ;;
|
||||
-* | *'>'* | *'<'*) ;; # a flag, or a leading redirect
|
||||
[0-9]*) [ "$wrapped" = 1 ] || break ;; # a wrapper's duration, not `1:` in prose
|
||||
'!' | '{' | '}' | if | then | elif | else | while | until | do) ;; # never the command
|
||||
timeout | time | nice | nohup | stdbuf | command | builtin | noglob | xargs | sudo | env)
|
||||
wrapped=1 ;;
|
||||
# A wrapper's option value is indistinguishable from a command name (`stdbuf -o L rm`),
|
||||
# so past a wrapper the whole segment is scanned instead of stopping at the first
|
||||
# ordinary word. Before one, that word is the command and the verb cannot follow it.
|
||||
# so past a wrapper the scan runs to the end of the segment instead of stopping at the
|
||||
# first ordinary word. Before one, that word is the command and the verb cannot follow
|
||||
# it. Nothing bounds the scan: a wrapper takes unboundedly many operands
|
||||
# (`env -u A -u B …`), and any cutoff — a word count, or stopping at the first quoted
|
||||
# word — drops the prompt for a real `sudo -u 'root' rm`. Prose after a wrapper is the
|
||||
# price, and it only over-prompts.
|
||||
*) [ "$wrapped" = 1 ] || break ;;
|
||||
esac
|
||||
done
|
||||
# `tr` and not `${2//[...]}`: a `}` inside the bracket expression closes the expansion
|
||||
# itself, which silently leaves the command unsplit and every separator unseen.
|
||||
done <<< "$(printf '%s' "$2" | tr ';&|(){}`' '\n')"
|
||||
done <<< "$(strip_heredoc_bodies "$2" | tr ';&|()`' '\n')"
|
||||
return 1
|
||||
}
|
||||
|
||||
|
||||
@@ -19,10 +19,11 @@ run() { # run <hook> <allow|ask|none> <command>
|
||||
else
|
||||
got=$(printf '%s' "$out" | jq -r '.hookSpecificOutput.permissionDecision // "PARSE-ERROR"' 2>/dev/null || echo PARSE-ERROR)
|
||||
fi
|
||||
local shown="${cmd//$'\n'/ ⏎ }"
|
||||
if [ "$got" = "$want" ]; then
|
||||
printf ' ok %-5s %s\n' "$got" "$cmd"
|
||||
printf ' ok %-5s %s\n' "$got" "$shown"
|
||||
else
|
||||
printf 'FAIL want=%-5s got=%-5s %s\n %s\n' "$want" "$got" "$cmd" "$out"
|
||||
printf 'FAIL want=%-5s got=%-5s %s\n %s\n' "$want" "$got" "$shown" "$out"
|
||||
fails=$((fails + 1))
|
||||
fi
|
||||
}
|
||||
@@ -59,6 +60,48 @@ run $G ask 'r\m -rf /etc'
|
||||
run $G ask "! rm -rf /etc"
|
||||
run $G ask "if true; then rm -rf /etc; fi"
|
||||
run $G ask ">/dev/null rm -rf $OUT"
|
||||
# Data that merely mentions a verb is not a command. Both of these prompted in the field.
|
||||
run $G none "$(printf 'gh pr create --body "$(cat <<%sEOF%s\ndrop `rm` and `mv` from the ask list\nrm is now guarded here\nEOF\n)"' "'" "'")"
|
||||
run $G none "$(printf 'claude -p "run these in order:\n1: rm -rf /tmp/a\n2: mv /tmp/b /tmp/c"')"
|
||||
# A wrapper's own flags and assignments are unbounded, so they may not be charged against the
|
||||
# scan that looks past it — these run rm and must prompt.
|
||||
run $G ask "env -i HOME=/tmp PATH=/usr/bin LANG=C USER=root SHELL=/bin/sh rm -rf /etc"
|
||||
run $G ask "sudo -E -H -u root FOO=1 BAR=2 rm -rf $OUT"
|
||||
run $G ask "xargs -a f -d d -E e -I {} -L 1 -n 1 rm /etc"
|
||||
run $G ask "env -u A -u B -u C -u D -u E -u F -u G rm -rf /etc"
|
||||
run $G ask "sudo -u 'root' rm -rf /etc"
|
||||
run $G ask "$(printf 'echo hi # cat <<EOF\nrm -rf /etc\nEOF')"
|
||||
# A `<<` inside a quoted string or a comment opens no heredoc, so the command under it is real.
|
||||
run $G ask "$(printf 'echo "cat <<EOF"\nrm -rf /etc\nEOF')"
|
||||
run $G ask "$(printf 'echo "cat <<EOF and more"\nrm -rf /etc\nEOF')"
|
||||
run $G ask "$(printf 'echo "cat <<EOF "\nrm -rf /etc\nEOF')"
|
||||
run $G ask "$(printf '# usage: cat <<EOF\nrm -rf /etc\nEOF')"
|
||||
run $G ask "$(printf 'echo "cat <<EOF > f"\nrm -rf /etc\nEOF')"
|
||||
run $G ask "$(printf 'echo "cat <<true > /tmp/a"\nrm -rf /etc\ntrue')"
|
||||
run $G ask "$(printf "echo 'cat <<EOF | tee'\nrm -rf /etc\nEOF")"
|
||||
# A body fed to a shell is executed, so it is commands and not data.
|
||||
run $G ask "$(printf 'bash <<EOF\nrm -rf /etc\nEOF')"
|
||||
run $G ask "$(printf 'cat <<EOF | bash\nrm -rf /etc\nEOF')"
|
||||
run $G ask "$(printf 'ssh host <<EOF\nrm -rf /etc\nEOF')"
|
||||
run $G ask "$(printf 'bash<<%sEOF%s\nrm -rf /etc\nEOF' "'" "'")"
|
||||
run $G ask "$(printf '/bin/sh <<EOF\nrm -rf /etc\nEOF')"
|
||||
run $G ask "$(printf 'cat <<%sEOF%s|bash\nrm -rf /etc\nEOF' "'" "'")"
|
||||
run $G ask "$(printf 'out=$(bash <<%sEOF%s\nrm -rf /etc\nEOF\n)' "'" "'")"
|
||||
run $G ask "$(printf 'bash \\\n <<%sEOF%s\nrm -rf /etc\nEOF' "'" "'")"
|
||||
run $G ask "$(printf 'ash <<%sEOF%s\nrm -rf /etc\nEOF' "'" "'")"
|
||||
run $G ask "$(printf 'busybox sh <<%sEOF%s\nrm -rf /etc\nEOF' "'" "'")"
|
||||
run $G ask "$(printf 'sudo -s <<%sEOF%s\nrm -rf /etc\nEOF' "'" "'")"
|
||||
run $G ask "$(printf '(bash <<%sEOF%s)\nrm -rf /etc\nEOF' "'" "'")"
|
||||
|
||||
# A redirect or pipe after the delimiter is still a real heredoc.
|
||||
run $G none "$(printf 'cat <<%sEOF%s > /tmp/a\nrm -rf /etc\nEOF' "'" "'")"
|
||||
run $G none "$(printf 'cat <<%sEOF%s 2>&1 | tee /tmp/a\nrm -rf /etc\nEOF' "'" "'")"
|
||||
# An unquoted body is expanded before its consumer sees it, so it is code.
|
||||
run $G ask "$(printf 'cat <<EOF > /tmp/a\n$(rm -rf /etc)\nEOF')"
|
||||
run $G ask "$(printf 'cat <<EOF > /tmp/a\nrm -rf /etc\nEOF')"
|
||||
# ... but a real command after a heredoc still is one.
|
||||
run $G ask "$(printf 'cat <<EOF > /tmp/s.sh\nhello\nEOF\nrm -rf %s' "$OUT")"
|
||||
run $G ask "$(printf 'echo "a << b"\nrm -rf %s' "$OUT")"
|
||||
run $G none "git rm frontend/foo.ts"
|
||||
run $G none 'echo $(ls /tmp)'
|
||||
run $G none 'grep -rn "rm" backend/'
|
||||
@@ -80,6 +123,8 @@ run $A ask "timeout --signal KILL 5 mv /tmp/a /etc"
|
||||
run $A ask "time -f FORMAT chmod 777 $OUT"
|
||||
run $A ask "'mv' /tmp/a /etc"
|
||||
run $A ask 'ch\mod 777 /etc'
|
||||
run $A none "$(printf 'claude -p "run these in order:\n1: rm -rf /tmp/a\n2: mv /tmp/b /tmp/c"')"
|
||||
run $A ask "env -i A=1 B=2 C=3 D=4 E=5 F=6 mv /tmp/a /etc"
|
||||
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"
|
||||
|
||||
Reference in New Issue
Block a user