wrapper-guard: model quote removal and escaping as separate operations
ci/woodpecker/pr/ci Pipeline was successful
ci/woodpecker/pr/ci Pipeline was successful
Round six of the same class: a program NAME is not one SPELLING. Two findings, and the second is one this change's own predecessor introduced. A FOURTH name consumer never went through the shared site. Checkout detection still recognized git by a raw whole-command regex, so `g"it" clone`, `g'it' clone` and `g\it clone` into $HOME were all allowed. `/usr/bin/git` blocked only because the raw text still happened to contain contiguous `git` — the same passing presentation that established nothing during the curl rounds. It now uses CMD_NAMES and NAME_PREFIX like the other three, so all four consumers share one definition of what a name looks like. Routing it through NAME_PREFIX also repaired an over-block the arm had carried from the start: the old regex found `git` INSIDE a longer word, so `mygit clone` and `gitfoo clone` were refused at every previous head. That is the mycurl and curl-wrapper class, and refusing it is how a guard gets routed around instead of repaired. The normalization itself was creating names the shell never runs. It deleted every backslash regardless of quote context, but a backslash inside single quotes is literal, so `'cu\rl' --config` names a program called cu\rl and was refused. The same holds inside double quotes before any character other than $, `, " or backslash. Both were false positives, and both were regressions — the pre-PR head allowed them. Quote removal and escape handling are different operations that were sharing one context-blind deletion pass. They are now a small state machine that follows the actual rule: outside quotes a backslash escapes the next character; inside single quotes everything is literal; inside double quotes a backslash is special only before $, `, " or backslash. Quote characters drop without splitting the word, and the substitution flattening that makes `$(which curl)` resolve to a bare name is unchanged. An over-block is not the safe direction. A guard that refuses legitimate work gets routed around rather than fixed, which is the same outcome as a bypass and arrives faster. Unchanged and still disclosed: names absent from the literal text — assembled from braces or variables — remain invisible to text matching, and `$((curl))` is over-matched at every head including the pre-PR one. Fixtures: 173 -> 184. Every one added here discriminates against the previous head1c3e79a9(8 fail there: 3 git bypasses, 5 over-blocks), and the positives also fail at the pre-PR headdf83a9ee. Suite green at head, bash -n and shellcheck clean, enumeration gate 55/38/18.
This commit is contained in:
@@ -32,7 +32,17 @@ FIXTURES="$TMP/fixtures.tsv"
|
|||||||
{
|
{
|
||||||
printf '2\t{"tool_input":{"command":"git clone https://example.invalid/x ~/wt"}}\tcheckout into $HOME is refused\n'
|
printf '2\t{"tool_input":{"command":"git clone https://example.invalid/x ~/wt"}}\tcheckout into $HOME is refused\n'
|
||||||
printf '2\t{"tool_input":{"command":"git worktree add ~/wt topic"}}\tworktree into $HOME is refused\n'
|
printf '2\t{"tool_input":{"command":"git worktree add ~/wt topic"}}\tworktree into $HOME is refused\n'
|
||||||
|
printf '2\t{"tool_input":{"command":"g\\"it\\" clone https://example.invalid/x $HOME/wt"}}\ta double quote inside git does not hide a checkout\n'
|
||||||
|
printf '2\t{"tool_input":{"command":"g'"'"'it'"'"' clone https://example.invalid/x $HOME/wt"}}\ta single quote inside git does not hide a checkout\n'
|
||||||
|
printf '2\t{"tool_input":{"command":"g\\\\it clone https://example.invalid/x $HOME/wt"}}\tan unquoted escape inside git does not hide a checkout\n'
|
||||||
printf '0\t{"tool_input":{"command":"git clone https://example.invalid/x /src/wt"}}\tcheckout onto a work filesystem is fine\n'
|
printf '0\t{"tool_input":{"command":"git clone https://example.invalid/x /src/wt"}}\tcheckout onto a work filesystem is fine\n'
|
||||||
|
# Routing this arm through the shared name site also repaired an over-block it
|
||||||
|
# had carried from the start: the old whole-command regex found `git` INSIDE a
|
||||||
|
# longer word, so these two were refused at every head before this commit.
|
||||||
|
# Same class as mycurl and curl-wrapper, and refusing them is how a guard gets
|
||||||
|
# routed around instead of repaired.
|
||||||
|
printf '0\t{"tool_input":{"command":"mygit clone https://example.invalid/x $HOME/wt"}}\tmygit is a different program and its checkout is not ours\n'
|
||||||
|
printf '0\t{"tool_input":{"command":"gitfoo clone https://example.invalid/x $HOME/wt"}}\tthe name has to end where git ends\n'
|
||||||
printf '0\t{"tool_input":{"command":"curl -s -X GET https://git.example.invalid/api/v1/repos/a/b/pulls/1"}}\treads are never blocked\n'
|
printf '0\t{"tool_input":{"command":"curl -s -X GET https://git.example.invalid/api/v1/repos/a/b/pulls/1"}}\treads are never blocked\n'
|
||||||
printf '2\t{"tool_input":{"command":"curl -X POST -d @b https://git.example.invalid/api/v1/repos/a/b/pulls/1/reviews"}}\treview write has a wrapper\n'
|
printf '2\t{"tool_input":{"command":"curl -X POST -d @b https://git.example.invalid/api/v1/repos/a/b/pulls/1/reviews"}}\treview write has a wrapper\n'
|
||||||
printf '2\t{"tool_input":{"command":"curl -X POST -d @b https://git.example.invalid/api/v1/repos/a/b/pulls/1/merge"}}\tmerge write has a wrapper\n'
|
printf '2\t{"tool_input":{"command":"curl -X POST -d @b https://git.example.invalid/api/v1/repos/a/b/pulls/1/merge"}}\tmerge write has a wrapper\n'
|
||||||
@@ -323,6 +333,22 @@ FIXTURES="$TMP/fixtures.tsv"
|
|||||||
printf '2\t{"tool_input":{"command":"cu\\"rl\\" --config /tmp/provider-write.cfg"}}\ta quote inside the word does not make it another program\t--config/-K\n'
|
printf '2\t{"tool_input":{"command":"cu\\"rl\\" --config /tmp/provider-write.cfg"}}\ta quote inside the word does not make it another program\t--config/-K\n'
|
||||||
printf '2\t{"tool_input":{"command":"cu'"'"'rl'"'"' --config /tmp/provider-write.cfg"}}\tand a single quote inside it is the same word again\t--config/-K\n'
|
printf '2\t{"tool_input":{"command":"cu'"'"'rl'"'"' --config /tmp/provider-write.cfg"}}\tand a single quote inside it is the same word again\t--config/-K\n'
|
||||||
printf '2\t{"tool_input":{"command":"/usr/bin/cu\\\\rl --config /tmp/provider-write.cfg"}}\tescaping is ordinary word formation, not a disguise\t--config/-K\n'
|
printf '2\t{"tool_input":{"command":"/usr/bin/cu\\\\rl --config /tmp/provider-write.cfg"}}\tescaping is ordinary word formation, not a disguise\t--config/-K\n'
|
||||||
|
# A backslash is NOT uniformly removed. It is literal inside single quotes,
|
||||||
|
# and inside double quotes when it precedes anything other than $, `, ",
|
||||||
|
# backslash, or newline. These spell a different program and must stay allowed.
|
||||||
|
printf '0\t{"tool_input":{"command":"'"'"'cu\\\\rl'"'"' --config /tmp/provider-write.cfg"}}\ta backslash inside single quotes remains literal\n'
|
||||||
|
printf '0\t{"tool_input":{"command":"\\"cu\\\\rl\\" --config /tmp/provider-write.cfg"}}\ta backslash before r inside double quotes remains literal\n'
|
||||||
|
printf '0\t{"tool_input":{"command":"'"'"'g\\\\it'"'"' clone https://example.invalid/x $HOME/wt"}}\ta literal backslash in a single-quoted non-git name is not a checkout\n'
|
||||||
|
printf '0\t{"tool_input":{"command":"\\"g\\\\it\\" clone https://example.invalid/x $HOME/wt"}}\ta literal backslash in a double-quoted non-git name is not a checkout\n'
|
||||||
|
# The other branch of the same rule: OUTSIDE quotes a backslash escapes the
|
||||||
|
# next character, so an escaped quote is a literal quote IN the name and the
|
||||||
|
# program is not curl. Held separately from the cases above because it is a
|
||||||
|
# different arm of the state machine, and an arm without a fixture is a rule
|
||||||
|
# that is not held.
|
||||||
|
printf '0\t{"tool_input":{"command":"cu\\\\\\"rl\\\\\\" --config /tmp/provider-write.cfg"}}\tan escaped quote is a literal quote in the name\n'
|
||||||
|
# Three quoted segments concatenate into ONE word. This is the shape that
|
||||||
|
# distinguishes quote removal from token separation, so it is worth its own line.
|
||||||
|
printf '2\t{"tool_input":{"command":"\\"cu\\"'"'"'r'"'"'\\"l\\" --config /tmp/provider-write.cfg"}}\tadjacent quoted segments are one word, and that word is curl\t--config/-K\n'
|
||||||
printf '2\t{"tool_input":{"command":"g\\"h\\" api -X POST repos/a/b/issues -f title=x"}}\tthe CLI name is a word on the same terms\n'
|
printf '2\t{"tool_input":{"command":"g\\"h\\" api -X POST repos/a/b/issues -f title=x"}}\tthe CLI name is a word on the same terms\n'
|
||||||
printf '2\t{"tool_input":{"command":"/usr/bin/g\\\\h api -X POST repos/a/b/issues -f title=x"}}\tincluding when it is escaped behind a path\n'
|
printf '2\t{"tool_input":{"command":"/usr/bin/g\\\\h api -X POST repos/a/b/issues -f title=x"}}\tincluding when it is escaped behind a path\n'
|
||||||
# The FLAG is the same recognition problem as the name, and is read the same
|
# The FLAG is the same recognition problem as the name, and is read the same
|
||||||
|
|||||||
@@ -49,9 +49,9 @@ CMD="$(printf '%s' "$INPUT" | jq -r '.tool_input.command // empty' 2>/dev/null |
|
|||||||
CMD="$(printf '%s' "$CMD" | sed -e ':a' -e 'N' -e '$!ba' -e 's/\\\n//g')"
|
CMD="$(printf '%s' "$CMD" | sed -e ':a' -e 'N' -e '$!ba' -e 's/\\\n//g')"
|
||||||
|
|
||||||
# A second reading of the SAME command, used by every check below that has to
|
# A second reading of the SAME command, used by every check below that has to
|
||||||
# recognize a program by NAME. Quote characters, backslashes and the punctuation
|
# recognize a program by NAME. It applies shell word formation without executing
|
||||||
# of command substitution are DELETED, so a name the shell will resolve is a word
|
# expansions, so a name the shell will resolve is a word here regardless of how
|
||||||
# here regardless of how it was dressed:
|
# it was dressed:
|
||||||
#
|
#
|
||||||
# "/usr/bin/curl" --config /tmp/req quoted absolute path
|
# "/usr/bin/curl" --config /tmp/req quoted absolute path
|
||||||
# './curl' --config /tmp/req quoted relative path
|
# './curl' --config /tmp/req quoted relative path
|
||||||
@@ -73,10 +73,14 @@ CMD="$(printf '%s' "$CMD" | sed -e ':a' -e 'N' -e '$!ba' -e 's/\\\n//g')"
|
|||||||
# because the whitespace happened to land after a slash — a passing case that
|
# because the whitespace happened to land after a slash — a passing case that
|
||||||
# established nothing.
|
# established nothing.
|
||||||
#
|
#
|
||||||
# So the characters are DELETED rather than replaced, which is what quote removal
|
# Quote removal and escape handling are separate operations. Outside quotes, a
|
||||||
# is, and backslashes go with them because escaping is ordinary word formation
|
# backslash escapes the next character. Inside single quotes it is literal.
|
||||||
# too. Deletion also handles substitution: `$(which curl)` becomes `which curl`,
|
# Inside double quotes it escapes only $, `, ", backslash, or newline; before
|
||||||
# where the name is a word on its own.
|
# anything else both the backslash and following character remain literal. Quote
|
||||||
|
# characters themselves are dropped without splitting the word. The existing
|
||||||
|
# substitution flattening remains: unquoted and double-quoted $, (, ), and ` are
|
||||||
|
# dropped, so `$(which curl)` becomes `which curl`, where the name is a word on
|
||||||
|
# its own.
|
||||||
#
|
#
|
||||||
# This does NOT try to be a shell. It cannot see a name that is absent from the
|
# This does NOT try to be a shell. It cannot see a name that is absent from the
|
||||||
# text — assembled from variables, or reached through a wrapper script that
|
# text — assembled from variables, or reached through a wrapper script that
|
||||||
@@ -89,10 +93,57 @@ CMD="$(printf '%s' "$CMD" | sed -e ':a' -e 'N' -e '$!ba' -e 's/\\\n//g')"
|
|||||||
# these commands inside quotes on a Bash line is refused too. Applying that same
|
# these commands inside quotes on a Bash line is refused too. Applying that same
|
||||||
# rule here keeps the file coherent — the alternative is a guard where the
|
# rule here keeps the file coherent — the alternative is a guard where the
|
||||||
# payload arm treats quotes as text and the name arms treat them as armour.
|
# payload arm treats quotes as text and the name arms treat them as armour.
|
||||||
CMD_NAMES="$(printf '%s' "$CMD" | tr -d '"'"'"'\`()$\\')"
|
normalize_command_names() {
|
||||||
|
awk '
|
||||||
|
BEGIN { state = "outside"; out = "" }
|
||||||
|
{
|
||||||
|
if (NR > 1) out = out "\n"
|
||||||
|
for (i = 1; i <= length($0); i++) {
|
||||||
|
c = substr($0, i, 1)
|
||||||
|
if (state == "single") {
|
||||||
|
if (c == "\047") state = "outside"
|
||||||
|
else out = out c
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if (state == "double") {
|
||||||
|
if (c == "\"") {
|
||||||
|
state = "outside"
|
||||||
|
} else if (c == "\\") {
|
||||||
|
if (i == length($0)) {
|
||||||
|
out = out c
|
||||||
|
} else {
|
||||||
|
nextc = substr($0, i + 1, 1)
|
||||||
|
if (nextc == "$" || nextc == "`" || nextc == "\"" || nextc == "\\") {
|
||||||
|
out = out nextc
|
||||||
|
} else {
|
||||||
|
out = out c nextc
|
||||||
|
}
|
||||||
|
i++
|
||||||
|
}
|
||||||
|
} else if (c != "$" && c != "(" && c != ")" && c != "`") {
|
||||||
|
out = out c
|
||||||
|
}
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if (c == "\\") {
|
||||||
|
if (i == length($0)) out = out c
|
||||||
|
else { out = out substr($0, i + 1, 1); i++ }
|
||||||
|
} else if (c == "\047") {
|
||||||
|
state = "single"
|
||||||
|
} else if (c == "\"") {
|
||||||
|
state = "double"
|
||||||
|
} else if (c != "$" && c != "(" && c != ")" && c != "`") {
|
||||||
|
out = out c
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
END { printf "%s", out }
|
||||||
|
'
|
||||||
|
}
|
||||||
|
CMD_NAMES="$(printf '%s' "$CMD" | normalize_command_names)"
|
||||||
|
|
||||||
# The one place the shape of a program NAME is written down. Both callers below
|
# The one place the shape of a program NAME is written down. Every name consumer
|
||||||
# use it, so the next fix to this class lands in a single location instead of
|
# below uses it, so the next fix to this class lands in a single location instead of
|
||||||
# being applied to whichever arm review happened to probe. The prefix must end
|
# being applied to whichever arm review happened to probe. The prefix must end
|
||||||
# at a slash: `mycurl` and `curl-wrapper` are different programs, and blocking
|
# at a slash: `mycurl` and `curl-wrapper` are different programs, and blocking
|
||||||
# them is the over-block that gets a guard routed around instead of repaired.
|
# them is the over-block that gets a guard routed around instead of repaired.
|
||||||
@@ -197,7 +248,7 @@ if [ "$home_known" -eq 1 ]; then
|
|||||||
home_re="$home_re|$(printf '%s' "$HOME_DIR" | sed 's/[][\.*^$+?(){}|]/\\&/g')"
|
home_re="$home_re|$(printf '%s' "$HOME_DIR" | sed 's/[][\.*^$+?(){}|]/\\&/g')"
|
||||||
fi
|
fi
|
||||||
|
|
||||||
if printf '%s' "$CMD" | grep -Eq 'git[^|;&]*(clone|worktree[[:space:]]+add)'; then
|
if printf '%s' "$CMD_NAMES" | grep -Eq "${NAME_PREFIX}git[[:space:]]+[^|;&]*(clone([[:space:]]|$)|worktree[[:space:]]+add([[:space:]]|$))"; then
|
||||||
if [ "$home_known" -eq 0 ]; then
|
if [ "$home_known" -eq 0 ]; then
|
||||||
cat <<EOF
|
cat <<EOF
|
||||||
BLOCKED: this is a checkout, and \$HOME is unset or unusable in this shell.
|
BLOCKED: this is a checkout, and \$HOME is unset or unusable in this shell.
|
||||||
|
|||||||
Reference in New Issue
Block a user