fix(shell): recover per-token JSON separator leak in argv
Weak models re-emit `npm -v` as ["npm,","-v"] or ["npm\",","\"-v\""] — the JSON array separators leak into the tokens. The collapsed-array guard rejected these, and the model re-emitted the identical mangle until stage_loop_break killed the run (observed 6x on the web-ui QA workflow, session 508c8d58). Strip stray leading/trailing quote/comma per token so the call runs instead of looping the stage to death. Internal commas (--foo=a,b) are kept; a single fully-collapsed token still rejects. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+14
-9
@@ -167,19 +167,24 @@ class ShellTool(
|
|||||||
// argv[0] is the program name. A single element carrying whitespace is the weak-model "whole
|
// argv[0] is the program name. A single element carrying whitespace is the weak-model "whole
|
||||||
// command line in one string" collapse (["npm create vite@latest frontend"]) — split it into
|
// command line in one string" collapse (["npm create vite@latest frontend"]) — split it into
|
||||||
// tokens so the denylist, allowlist and executor all see a real argv, instead of rejecting it and
|
// tokens so the denylist, allowlist and executor all see a real argv, instead of rejecting it and
|
||||||
// looping the stage on a call the model reliably re-emits. A JSON-escape mangle collapses
|
// looping the stage on a call the model reliably re-emits. Weak models also leak the JSON array
|
||||||
// differently — it leaves quotes/commas in argv[0] (["npm\",\"-v"]) — and stays rejected: there is
|
// separators into the individual tokens — trailing commas (["npm,","-v"]) or wrapping escaped-
|
||||||
// no runnable program to recover. (Splitting on whitespace loses quoting of args with spaces —
|
// quotes (["npm\",","\"-v\""]) — which the model re-emits identically until the stage loop breaks.
|
||||||
// same ceiling as the `sh -c` join below; fine for agent shell use, not a general shell API.)
|
// Strip stray leading/trailing quote/comma from each token so those recover too. (Internal commas,
|
||||||
|
// --foo=a,b, are kept; a single token that is itself the whole collapsed array — "npm\",\"-v\"," —
|
||||||
|
// still has interior quotes/commas and stays rejected: there is no runnable program to recover.
|
||||||
|
// Splitting on whitespace loses quoting of args with spaces — same ceiling as the `sh -c` join
|
||||||
|
// below; fine for agent shell use, not a general shell API.)
|
||||||
private fun checkExecutable(raw: List<String>): ArgvParse {
|
private fun checkExecutable(raw: List<String>): ArgvParse {
|
||||||
val argv = if (raw.size == 1 && raw[0].any { it.isWhitespace() } &&
|
val stripped = raw.map { it.trim().trim { c -> c == '"' || c == ',' } }.filter { it.isNotEmpty() }
|
||||||
raw[0].none { it == '"' || it == ',' }
|
val argv = if (stripped.size == 1 && stripped[0].any { it.isWhitespace() } &&
|
||||||
|
stripped[0].none { it == '"' || it == ',' }
|
||||||
) {
|
) {
|
||||||
raw[0].trim().split(WHITESPACE_RE)
|
stripped[0].trim().split(WHITESPACE_RE)
|
||||||
} else {
|
} else {
|
||||||
raw
|
stripped
|
||||||
}
|
}
|
||||||
return if (argv[0].any { it == '"' || it == ',' || it.isWhitespace() }) {
|
return if (argv.isEmpty() || argv[0].any { it == '"' || it == ',' || it.isWhitespace() }) {
|
||||||
ArgvParse.Bad(
|
ArgvParse.Bad(
|
||||||
"argv[0] `${argv[0]}` is not a valid executable name — it looks like a collapsed array. " +
|
"argv[0] `${argv[0]}` is not a valid executable name — it looks like a collapsed array. " +
|
||||||
"Emit each token as a separate string element, e.g. [\"npm\", \"-v\"].",
|
"Emit each token as a separate string element, e.g. [\"npm\", \"-v\"].",
|
||||||
|
|||||||
+11
@@ -204,6 +204,17 @@ class ShellToolTest {
|
|||||||
assertTrue((result as ValidationResult.Invalid).reason.contains("collapsed array"), result.reason)
|
assertTrue((result as ValidationResult.Invalid).reason.contains("collapsed array"), result.reason)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `validateRequest recovers per-token separator leak into argv`() {
|
||||||
|
// Repro: gemma re-emitted `npm -v` as ["npm,","-v"] (trailing comma) and ["npm\",","\"-v\""]
|
||||||
|
// (wrapping escaped-quotes) six times → stage_loop_break → FAILED. Strip the stray quote/comma
|
||||||
|
// per token so the call runs instead of looping the stage to death.
|
||||||
|
listOf(listOf("npm,", "-v"), listOf("npm\",", "\"-v\"")).forEach { argv ->
|
||||||
|
val result = ShellTool().validateRequest(rawArgvRequest(argv))
|
||||||
|
assertTrue(result is ValidationResult.Valid, "$argv: ${(result as? ValidationResult.Invalid)?.reason}")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun `validateRequest splits a collapsed single-string command line into tokens`() {
|
fun `validateRequest splits a collapsed single-string command line into tokens`() {
|
||||||
// Repro: weak model emitted ["npm create vite@latest frontend"] — the whole command line as
|
// Repro: weak model emitted ["npm create vite@latest frontend"] — the whole command line as
|
||||||
|
|||||||
Reference in New Issue
Block a user