Swift · security · Shellby

If You Can't Parse the Command Name, Don't Allow It — Command Parsing Is the AI Agent's Security Boundary

A Shellby postmortem: the AI Agent grades commands read-only / mutating / destructive before executing, and the verdict rests entirely on parsing the real command name from the line. And sudo options, su elevation, env-assignment prefixes, a pipe inside quotes — each can fool a naive parser. Why the parser itself is the security boundary, and why the right default when it can't parse is 'deny'.

I’ve written before about Shellby’s AI Agent three-tier approval gate: before a command executes, a local CommandClassifier grades it — read-only auto-passes, mutating needs a click, destructive needs strong confirmation — layered with a user allow/deny list. That post covered the approval architecture. This one covers a lower-level, more critical issue: the correctness of this verdict rests entirely on “can you parse the real command name from the command line.” And the parser itself is the security boundary.

This week I revised that parser, and every bug was “parse wrong → verdict wrong → either false-allow or false-block.”

The verdict’s foundation is “get the command name”

The grading order (conservative first):

  1. Built-in destructive patterns vetorm -rf, dd, piping to sh, redirecting to a system path, these have top priority and the allowlist can’t pierce them (allowlist rm, and rm -rf / is still destructive);
  2. Hits a user denylist → destructive;
  3. isPureReadOnly: the whole command is a simple pipeline / chain of “built-in read-only set ∪ user allowlist,” with no redirect, command substitution, or backgrounding → read-only;
  4. Everything else → mutating (conservative: a truly read-only but complex command gets one more approval rather than a false pass).

Steps 2, 3, and 4 all first answer one question: what exactly is this segment’s command name? Three judgments — allowlistCandidates, commandUsesDenied, isReadOnlyPipeline — all converge on one parser, segmentCommandToken. Parse it wrong and the whole verdict above follows.

Elevation prefixes: can’t parse → return nil

The most dangerous class is elevation. A naive “take the first token as the command name, strip sudo along the way” is fooled like this:

sudo -u deploy pm2 list      # first token is sudo, strip it, second is -u…

If, to let some option through, you added -u to the allowlist, the naive parser treats -u, pm2 as read-only and auto-passes — when the command is actually running things as a different user. That’s a privilege-escalation bypass.

The right handling of elevation prefixes: if you can’t parse a trustworthy command name, return nil. And nil is uniformly understood by all three callers as “non-read-only, and no allowlist.”

static func segmentCommandToken(_ segment: String) -> String? {
    // ...
    while i < toks.count, isEnvAssignment(toks[i]) { i += 1 }   // skip VAR=val prefixes
    let name = commandBasename(raw).lowercased()
    if name == "su" || name == "doas" { return nil }            // post-elevation command name untrustworthy
    if name == "sudo" {
        i += 1
        while i < toks.count, isEnvAssignment(toks[i]) { i += 1 }
        if stripGroupChars(toks[i]).hasPrefix("-") { return nil } // sudo with options → give up
        continue                                                  // otherwise keep parsing the sudo'd command
    }
    // ...
}

A regression test nails the hole shut: classify("sudo -u deploy pm2 list", allow: ["-u", "pm2"]) must be .mutatingsudo is followed by -u (starts with -), so nil directly, and neither -u nor pm2 can enter the read-only judgment. su / doas unconditionally return nil, because after elevation the actually-executed command name is no longer trustworthy.

“Can’t parse” is not an error state, it’s a security verdict: when the command name is untrustworthy, default to approval, not to auto-pass.

A pipe inside quotes: one bug, two directions of error

The second class is quotes. Look at this classic read-only command:

ps aux | grep -E 'FutuOpenD|FTWebSocket' | grep -v grep

A naive split on | treats the literal | inside the regex as a pipe too, carving out a fragment FTWebSocket' as a command name — so a “allow ftwebsocket’” ghost appears on the allowlist button. One root cause (not recognizing quotes), wrong in two directions: it false-flags (carves out a dangerous fragment that doesn’t exist) AND makes a textbook read-only pipeline need one more approval.

The fix is a quote-aware splitting state machine splitAware: | ; && || > < & inside '...' / "..." are all treated as literals. But there’s a safety backstop here you can’t skip — destructive pattern matching still runs on the whole raw string. A test asserts echo 'a; rm -rf /' is still .destructive: making separators inert inside quotes is for “correctly splitting the command name,” but quotes must never become a place to hide rm -rf. Tokenization is tokenization; the destructive bottom line scans the whole string.

Newlines, wrappers, paths: the destructive bottom line pierces every disguise

A few more plugged holes:

  • A newline is a separator too. ls\nsystemctl restart nginx — if you parse only the first line ls as read-only and pass it, the following line is missed. So the top-level separator set went from && || ; to include \n \r;
  • Basename-ize, but the destructive bottom line pierces full paths. /usr/bin/ls matches the allowlist by basename ls; while timeout 5 /bin/systemctl restart nginx with denylist systemctl must pierce the timeout wrapper and the /bin/ full path to judge destructive;
  • Wrapper prefixes pass through, but can’t pierce destructive patterns. nohup / env / exec / setsid / timeout pass through to the wrapped real command, and timeout skips duration args like 5s / 1.5m — but nohup rm -rf /data, timeout 5 rm -rf /data are still destructive, because the destructive pattern matches the whole lowercased string and no wrapper pierces it.

In one line: basename-ize and wrapper pass-through exist to “recognize an allowlisted command,” while the destructive bottom line scans the raw whole string — two logics in opposite directions, neither dispensable.

The allowlist button: not collecting tokens, but “assume added, re-judge once”

The approval dialog has a “one-click approve and add to allowlist.” Which tokens it offers a button for is itself a security question — you can’t offer an option that “does nothing when added, just misleads the user.”

allowlistCandidates is designed to be validation-style: not simply collecting command names, but assuming these tokens are added to the allowlist, re-classifying, and offering the button only if it truly becomes .readOnly. ps aux > out.txt has a redirect, can never be read-only no matter what you allowlist → returns nil, no misleading option; cat x | awk … | sort returns only awk (cat, sort are built-in read-only, filtered out). This locks “should the UI offer an allow button” and “how the classifier judges” into the same logic, preventing the two from diverging.

Aside: the sudo password is injected only at execution

Briefly on the sudo elevation password (AGENT-SUDO-01). The password is rewritten into the command only at the moment of execution; what’s shown to the user, written to history, and sent to the model is always the original command:

// -S reads the password from stdin; -p '' suppresses the prompt (else it mixes into stderr)
return "printf '%s\\n' \(shSingleQuote(password)) | \(env)sudo -S -p '' \(ls.rest)"

The password lives only in session memory, never on disk; a non-leading sudo (foo | sudo bar) can’t be injected, so it gives a clear hint instead of hanging.

Takeaways

  • The parser is the security boundary. When the grading verdict rests on “get the command name,” every parsing bug is a security bug — elevation prefixes, quotes, newlines, wrappers, full paths, any parse error can slip a dangerous command into read-only;
  • The default for “can’t parse” must be “deny.” When the command name is untrustworthy (elevation, options), return nil and have every caller treat nil as “non-read-only, no allowlist.” Fail-closed here means “if unsure of the command name, go to approval”;
  • Tokenization and the destructive bottom line are two logics in opposite directions. Quote-awareness, basename-ize, and wrapper pass-through exist to correctly recognize an allowlisted command; while the destructive pattern scans the raw whole string — quotes, wrappers, and paths can’t pierce it. Drop either logic and you have a hole;
  • Whether the UI offers an “allow” button must also go through the classifier. Decide candidates by “assume allowlisted, re-judge once,” not by collecting tokens separately — otherwise the UI and the verdict talk past each other, offering options that do nothing or mislead;
  • The same parser must be byte-aligned across stacks. The Swift and Flutter classifiers must be byte-for-byte identical — security logic fixed on one end with a hole on the other is no fix at all.

Comments

  • Loading…

Comments are reviewed before publishing; email is visible only to me.