Skip to content

Tier-1 gate bypass: _fw_split_command_segments misses the newline, &, |& and ;; separators, and shlex silently drops input after a mid-word # #75

Closed

Description

@DimitriGeelen

Context

  • Framework version: 1.6.258
  • Project: 050-email-archive (shared-tooling mode)
  • Reported: 2026-09-25T12:22:18Z

Description

agents/context/lib/safe-commands.sh — _fw_split_command_segments recognises four of the
shell's command separators. The shell has more, and each omission is a Tier-1 task-gate
bypass of the same shape: a segment the splitter never emits is a segment nobody classifies,
so is_bash_safe_command classifies the whole compound by its first word — which is exactly
the first-word-only defect T-2147 (upstream #60) was written to remove.

Measured on a consumer project (Ubuntu, bash 5.x, python 3.12), before the patch below:

is_bash_safe_command 'git status && mv /a /b'   -> BLOCKED   (T-2147, correct)
is_bash_safe_command $'git status\nmv /a /b'    -> SAFE      (newline)
is_bash_safe_command 'git status & mv /a /b'    -> SAFE      (background &)
is_bash_safe_command 'git status |& mv /a /b'   -> SAFE      (|& pipe)
is_bash_safe_command 'git status ;; mv /a /b'   -> SAFE      (;;)
is_bash_safe_command 'echo a#b && mv /a /b'     -> SAFE      (see below)

The newline case is how this was found — a worker issued an ordinary multi-line Bash command.
A newline is plain whitespace to shlex, so both lines land in one segment.

The last case is not about separators at all. shlex's default commenters='#' fires on a
mid-word # under whitespace_split, so echo a#b && mv /a /b tokenises to
['echo','a'] — the lexer discards the operator and the entire right-hand command, and the
splitter returns success on input it silently truncated. Real shells only begin a comment at
a word boundary. Silent input loss is acceptable in a shell-like lexer and disqualifying in a
security gate.

mv is used as the payload above rather than rm on purpose: the caller runs
has_bash_write_pattern over the raw command first and that catches rm/redirect/sed -i/
tee regardless of segmentation, so an rm-based repro would be masked. mv is a real write
that only the per-segment classifier can stop.

Root cause

The contract was stated as a predicate over segments ("every segment must be safe") while the
function deciding what a segment is had no contract and no test. Under-segmentation produces
the same observable as a compound that never had the extra segment, so the class was invisible
by construction — it survived both T-2147 and T-2179.

Fix

Three rules, all fail-closed. Nothing is added to any allowlist; the classifier is strictly
stricter than before, and the only change in the permissive direction is that all-safe
multi-line reads are now correctly allowed because every line is actually checked.

  1. Newlines separate. Quoted newlines are data; \<newline> is a line continuation;
    word-initial # is a comment, stripped by a quote-aware pre-pass so shlex's commenters
    can be disabled and no input is ever silently dropped.
  2. Every real separator segments: && || ; ;; | |& &. Redirect operators stay inline —
    writes remain has_bash_write_pattern's job.
  3. Anything else built purely of shell punctuation exits 1 — (, ), ;;&, and whatever
    separator nobody has thought of yet. Unbalanced quotes, a trailing backslash, and a token
    carrying a quoted newline likewise. The class is closed, not just its known members.

Tests

29 added (26 unit + 3 integration through the hook). Four assert the splitter's positive
output
— segment count and exact text — so a splitter that emits nothing, or loses a
segment, can no longer satisfy the BLOCKED cases for the wrong reason. That check's absence is
why the class stayed invisible. Existing T-2147/T-2179 tests pass unmodified: 268 pass, 0 fail
across tests/unit + tests/integration.

Twelve negative controls were run (each re-introducing one defect and required to turn a named
test red; restored tree green). Two findings worth passing on to anyone reviewing this:

  • A mutation that breaks bash syntax turns the whole suite red for the wrong reason — the
    file never sources. One control did this (54 red from an unterminated if) and looked
    maximally successful while proving nothing. Every mutation is now bash -n-gated.
  • Five cases are held by two layers (the new guard plus shlex's own ValueError, the
    base-word allowlist, or the caller's while read re-splitting). Removing only the new guard
    left the suite green, which reads identically to "vacuous test" but means the opposite. A
    control has to remove every layer holding its case.

Separate issue, not included here

The same probe found a hole in has_bash_write_pattern, whose redirect regex is
'[^2>&]>[^>&]|>>'. The leading exclusion was meant to skip 2>&1 but excludes all of 2>
and &>, so bin/fw audit 2> out.txt and bin/fw audit &> out.txt both report no-write and
safe — a redirect that truncates a file, allowed with no active task. Different function,
different root cause; filed separately downstream and not patched by this diff.

Patch

diff --git a/.agentic-framework/agents/context/lib/safe-commands.sh b/.agentic-framework/agents/context/lib/safe-commands.sh
index af2668b4..b6a265a6 100644
--- a/.agentic-framework/agents/context/lib/safe-commands.sh
+++ b/.agentic-framework/agents/context/lib/safe-commands.sh
@@ -22,32 +22,152 @@
 # `echo "a && b"`) are NOT treated as separators, and so `&&`/`||` combine
 # into single tokens rather than two `&` tokens. Exit 1 (no output) on
 # unbalanced quotes — caller treats that as "not safe" rather than guessing.
+#
+# T-2182: the T-2147 splitter recognised only FOUR of the shell's command
+# separators, and shlex silently discarded a third category of input. Each
+# omission was a gate bypass of the same shape — a segment the splitter never
+# emitted was a segment nobody classified, so an allowlisted read on the left
+# laundered an arbitrary command on the right:
+#
+#   is_bash_safe_command 'git status && mv /a /b'   -> BLOCKED (T-2147, correct)
+#   is_bash_safe_command $'git status\nmv /a /b'    -> SAFE    (newline)
+#   is_bash_safe_command 'git status & mv /a /b'    -> SAFE    (background &)
+#   is_bash_safe_command 'git status |& mv /a /b'   -> SAFE    (|& pipe)
+#   is_bash_safe_command 'echo a#b && mv /a /b'     -> SAFE    (see below)
+#
+# All four measured on this host before the fix. The first is how it was found:
+# a worker issued an ordinary multi-line command. A newline is plain whitespace
+# to shlex, so both lines landed in one segment and were classified by the first
+# word alone — the exact first-word-only defect T-2147 existed to kill, reachable
+# again through a separator it did not know about.
+#
+# The fourth is subtler and is not about separators at all: shlex's default
+# `commenters='#'` fires on a MID-WORD `#` under whitespace_split, so
+# `echo a#b && mv /a /b` tokenised to just ['echo','a'] — shlex threw the
+# operator and the whole right-hand command away, and the splitter reported
+# success on an input it had silently truncated. Real shells only start a
+# comment at a word boundary, so the pre-pass below strips comments itself
+# (quote-aware, word-initial only) and shlex's commenters are disabled.
+#
+# Three rules, all in the fail-closed direction:
+#   1. Newlines separate. Quoted newlines are data; `\<newline>` is a line
+#      continuation; every other newline ends a segment.
+#   2. Every real separator segments: && || ; ;; | |& &. Redirect operators are
+#      kept inline (writes are the caller's has_bash_write_pattern's job).
+#   3. Anything else built purely of shell punctuation — `(`, `)`, `;;&`, any
+#      construct not in those two sets — exits 1. An unrecognised construct is
+#      never guessed at; the caller reads exit 1 as "not safe".
 _fw_split_command_segments() {
     local cmd="$1"
     python3 - "$cmd" <<'PYEOF' 2>/dev/null
 import sys, shlex
 
 cmd = sys.argv[1]
+
+# Characters shlex(punctuation_chars=True) groups into operator tokens.
+PUNCT = set("();<>|&")
+# Real command separators — each one ends a segment.
+OPS = {"&&", "||", ";", ";;", "|", "|&", "&"}
+# Redirects: not separators. Harmless to a segment's classification; the
+# caller runs has_bash_write_pattern over the whole raw command first.
+REDIRECTS = {">", ">>", "<", "<<", "<<<", ">&", "<&", ">|", "&>", "&>>", "<>"}
+
+
+def logical_lines(s):
+    """Split on unquoted newlines, honouring quotes, `\\<newline>` line
+    continuations and word-initial `#` comments. Raises ValueError on an
+    unbalanced quote or a trailing backslash."""
+    lines, cur = [], []
+    quote = None
+    i, n = 0, len(s)
+    while i < n:
+        ch = s[i]
+        at_word_start = (not cur) or cur[-1].isspace()
+        if quote is None and ch == "\\":
+            if i + 1 >= n:
+                raise ValueError("trailing backslash")
+            # A continuation joins the two lines into one logical line.
+            cur.append(" " if s[i + 1] == "\n" else ch + s[i + 1])
+            i += 2
+            continue
+        if quote == '"' and ch == "\\" and i + 1 < n:
+            cur.append(ch + s[i + 1])
+            i += 2
+            continue
+        if quote is None and ch in ("'", '"'):
+            quote = ch
+            cur.append(ch)
+            i += 1
+            continue
+        if quote is not None:
+            if ch == quote:
+                quote = None
+            cur.append(ch)
+            i += 1
+            continue
+        if ch == "\n":
+            lines.append("".join(cur))
+            cur = []
+            i += 1
+            continue
+        if ch == "#" and at_word_start:
+            while i < n and s[i] != "\n":
+                i += 1
+            continue
+        cur.append(ch)
+        i += 1
+    if quote is not None:
+        raise ValueError("unbalanced quote")
+    lines.append("".join(cur))
+    return lines
+
+
 try:
-    lexer = shlex.shlex(cmd, posix=True, punctuation_chars=True)
-    lexer.whitespace_split = True
-    tokens = list(lexer)
+    lines = logical_lines(cmd)
 except ValueError:
     sys.exit(1)
 
-OPS = {"&&", "||", ";", "|"}
 segments, current = [], []
-for tok in tokens:
-    if tok in OPS:
-        segments.append(current)
-        current = []
-    else:
-        current.append(tok)
+for line in lines:
+    lexer = shlex.shlex(line, posix=True, punctuation_chars=True)
+    lexer.whitespace_split = True
+    # Comments are already stripped above. Leaving shlex's commenters on
+    # would let a mid-word `#` swallow the rest of the line, operators and
+    # all — silent input loss, which is the one thing a gate must not do.
+    lexer.commenters = ""
+    try:
+        tokens = list(lexer)
+    except ValueError:
+        sys.exit(1)
+    for tok in tokens:
+        if tok in OPS:
+            segments.append(current)
+            current = []
+        elif tok in REDIRECTS:
+            current.append(tok)
+        elif tok and all(c in PUNCT for c in tok):
+            # Unrecognised shell construct — fail closed rather than guess.
+            sys.exit(1)
+        else:
+            current.append(tok)
+    # End of a logical line is itself a separator.
+    segments.append(current)
+    current = []
 segments.append(current)
 
+out = []
 for seg in segments:
-    if seg:
-        print(" ".join(seg))
+    if not seg:
+        continue
+    text = " ".join(seg)
+    if "\n" in text or "\r" in text:
+        # A quoted newline inside a token cannot be represented in this
+        # line-delimited protocol; the caller would re-split it on the
+        # newline and classify fragments. Fail closed, explicitly.
+        sys.exit(1)
+    out.append(text)
+
+print("\n".join(out))
 PYEOF
 }
 
@@ -275,6 +395,8 @@ _fw_is_single_command_safe() {
 # as safe. Pre-T-2147 this classified the whole compound command by its
 # first word alone, so `cd DIR && bin/fw audit` was blocked — `cd` isn't
 # allowlisted for compounds, even though `audit` is (M-02, T-2147).
+# T-2182: "every segment" is only as strong as the splitter's notion of a
+# separator. See _fw_split_command_segments for the four it was missing.
 is_bash_safe_command() {
     local cmd="$1"
 

Reported via fw upstream report from field installation

Activity

  1. DimitriGeelen commented on Oct 12, 2026

    @DimitriGeelen
    OwnerAuthor

    [AEF intake] Closed: fixed — 2026-10-12

    Fixed under T-2834 in commit 59a5ee9 (on the bleeding-edge branch; it ships in the next release).

    The splitter introduced in this commit handles newline, &, |&, ;; and mid-word #. Verified on bleeding-edge with the issue's own repro strings: every one of them is blocked.

    These status updates are automated, but Dimitri personally looks at every issue and decides what happens to it. Replies on this issue are read.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions