fix(security): close find -exec / write side-doors in shell command whitelist (CWE-184)

src/gaia/agents/tools/shell_tools.py

@@ -87,6 +87,26 @@
     "git",  # Individual git subcommands checked separately
 }
 
+# Actions/predicates that turn otherwise read-only commands into a write,
+# delete, or arbitrary-command-execution primitive. The whitelist only checks
+# the command NAME, so these must be inspected explicitly or an allowed command
+# (find/sort/uniq) becomes a bypass (CWE-184).
+#
+# find: -exec/-execdir/-ok/-okdir run any binary (incl. ones NOT in
+# ALLOWED_COMMANDS); -delete removes files; -fprint/-fprintf/-fls write files.
+# The read-only predicates (-print/-print0/-printf/-ls/-name/-type/…) are fine.
+DANGEROUS_FIND_ACTIONS = {
+    "-exec",
+    "-execdir",
+    "-ok",
+    "-okdir",
+    "-delete",
+    "-fprint",
+    "-fprint0",
+    "-fprintf",
+    "-fls",
+}
+
 # Safe read-only git subcommands
 SAFE_GIT_COMMANDS = {
     "status",
@@ -345,6 +365,82 @@ def _validate_command(
                         "has_errors": True,
                         "hint": "Allowed: Get-*, Select-Object, Format-List, Format-Table, Where-Object, Sort-Object",
                     }
+        # Special handling for find - block predicates that run, delete, or
+        # write files. Without this, `find ... -exec touch {} +` executes a
+        # binary that is NOT in ALLOWED_COMMANDS, bypassing the whitelist.
+        elif cmd_base == "find":
+            for part in cmd_parts[1:]:
+                if part.lower() in DANGEROUS_FIND_ACTIONS:
+                    return {
+                        "status": "error",
+                        "error": (
+                            f"find action '{part}' is not allowed: it can run "
+                            "arbitrary commands, delete, or write files, "
+                            "bypassing the read-only command whitelist."
+                        ),
+                        "has_errors": True,
+                        "hint": "Use read-only find predicates only: -name, -type, -path, -print, -ls.",
+                    }
+        # Special handling for sort - -o/--output writes to a file. Cover every
+        # spelling: -o FILE, -oFILE, -o=FILE, bundled short clusters (-ro), and
+        # every GNU long-option abbreviation of --output (--o, --out, --output=).
+        # Any short cluster containing 'o' is treated as -o; this over-blocks a
+        # few exotic clusters (e.g. -to, separator 'o'), which is acceptable for
+        # a read-only security guard.
+        elif cmd_base == "sort":
+            for part in cmd_parts[1:]:
+                part_lower = part.lower()
+                flag = part_lower.split("=", 1)[0]
+                is_output = False
+                if flag.startswith("--"):
+                    # --output and any unambiguous abbreviation (--o, --out, ...).
+                    if len(flag) > 2 and "--output".startswith(flag):
+                        is_output = True
+                elif part_lower.startswith("-") and part_lower != "-":
+                    # The leading run of letters is the short-flag cluster;
+                    # anything after it is an attached value (-oFILE, -ro/tmp/x).
+                    cluster = re.match(r"[a-z]*", flag[1:]).group(0)
+                    if "o" in cluster:
+                        is_output = True
+                if is_output:
+                    return {
+                        "status": "error",
+                        "error": "sort -o/--output writes to a file, which is not allowed under the read-only command policy.",
+                        "has_errors": True,
+                        "hint": "Drop -o/--output and read sort's result from stdout (e.g. 'sort file' or 'sort file | head').",
+                    }
+        # Special handling for uniq - a second file operand is an output file.
+        elif cmd_base == "uniq":
+            # Flags that consume the following token as their value; the operand
+            # counter must skip that value so it isn't mistaken for a file.
+            _uniq_value_flags = {
+                "-f",
+                "--skip-fields",
+                "-s",
+                "--skip-chars",
+                "-w",
+                "--check-chars",
+            }
+            operands = []
+            skip_next = False
+            for part in cmd_parts[1:]:
+                if skip_next:
+                    skip_next = False
+                    continue
+                if part in _uniq_value_flags:
+                    skip_next = True
+                    continue
+                if part.startswith("-") and part != "-":
+                    continue  # flag (incl. --flag=value and bundled short flags)
+                operands.append(part)
+            # operands = [input, output]; a second operand is a write target.
+            if len(operands) >= 2:
+                return {
+                    "status": "error",
+                    "error": "uniq with an output file is not allowed: it writes to disk, violating the read-only command policy.",
+                    "has_errors": True,
+                    "hint": "Use a single input (or stdin) and read stdout, e.g. 'uniq file' or 'sort file | uniq'.",
+                }
         elif cmd_base not in ALLOWED_COMMANDS:
             return {
                 "status": "error",

tests/unit/test_shell_guardrails.py

@@ -150,6 +150,117 @@ def test_clean_command_not_flagged(self):
         assert not DANGEROUS_SHELL_OPERATORS.search("cat file.txt")
 
 
+# ---------------------------------------------------------------------------
+# find / sort / uniq write & exec side-doors (CWE-184: find -exec bypass)
+# ---------------------------------------------------------------------------
+
+
+class TestFindActionGuards:
+    """find is whitelisted as read-only, but several predicates run, delete,
+    or write files. These must be blocked or find becomes a whitelist bypass.
+    """
+
+    def test_find_exec_blocked(self):
+        result = validate("find /tmp -maxdepth 0 -exec touch /tmp/canary {} +")
+        assert result is not None
+        assert result["status"] == "error"
+        assert "find" in result["error"].lower()
+
+    def test_find_execdir_blocked(self):
+        result = validate("find /tmp -execdir touch {} +")
+        assert result is not None
+
+    def test_find_ok_blocked(self):
+        assert validate("find /tmp -name x -ok rm {} ;") is not None
+
+    def test_find_okdir_blocked(self):
+        assert validate("find /tmp -okdir rm {} ;") is not None
+
+    def test_find_delete_blocked(self):
+        assert validate("find /tmp -name x -delete") is not None
+
+    def test_find_fprint_blocked(self):
+        assert validate("find . -fprint /tmp/canary") is not None
+
+    def test_find_fprintf_blocked(self):
+        assert validate("find . -fprintf /tmp/canary hi") is not None
+
+    def test_find_fls_blocked(self):
+        assert validate("find . -fls /tmp/canary") is not None
+
+    def test_find_fprint0_blocked(self):
+        # -fprint0 writes null-separated results to FILE, same as -fprint.
+        assert validate("find . -fprint0 /tmp/canary") is not None
+
+    def test_find_exec_uppercase_blocked(self):
+        # Token is lowercased before matching, so case tricks don't help.
+        assert validate("find /tmp -EXEC touch {} +") is not None
+
+    # Read-only predicates must still be allowed
+    def test_find_print_allowed(self):
+        assert validate("find /tmp -maxdepth 2 -print") is None
+
+    def test_find_printf_allowed(self):
+        # -printf writes to STDOUT (read-only); must not be confused with -fprintf.
+        assert validate("find . -printf %p") is None
+
+    def test_find_ls_allowed(self):
+        assert validate("find . -ls") is None
+
+    def test_find_name_type_allowed(self):
+        assert validate("find . -name foo.py -type f") is None
+
+
+class TestSortOutputGuard:
+    def test_sort_output_short_blocked(self):
+        result = validate("sort -o /tmp/canary /etc/hostname")
+        assert result is not None
+        assert result["status"] == "error"
+
+    def test_sort_output_long_blocked(self):
+        assert validate("sort --output=/tmp/canary /etc/hostname") is not None
+
+    def test_sort_output_attached_blocked(self):
+        # -oFILE attached form must not slip past.
+        assert validate("sort -o/tmp/canary /etc/hostname") is not None
+
+    def test_sort_output_bundled_attached_blocked(self):
+        # -ro/tmp/x == -r -o /tmp/x: cluster + attached value in one token.
+        assert validate("sort -ro/tmp/canary /etc/hostname") is not None
+
+    def test_sort_output_bundled_blocked(self):
+        # Bundled short cluster -ro == -r -o.
+        assert validate("sort -ro /tmp/canary /etc/hostname") is not None
+
+    def test_sort_output_abbreviation_blocked(self):
+        # GNU sort accepts unambiguous long-option abbreviations of --output.
+        assert validate("sort --out=/tmp/canary /etc/hostname") is not None
+        assert validate("sort --o /tmp/canary /etc/hostname") is not None
+
+    def test_sort_plain_allowed(self):
+        assert validate("sort file.txt") is None
+
+    def test_sort_flags_allowed(self):
+        assert validate("sort -r -u file.txt") is None
+
+
+class TestUniqOutputGuard:
+    def test_uniq_output_file_blocked(self):
+        result = validate("uniq in.txt out.txt")
+        assert result is not None
+        assert result["status"] == "error"
+
+    def test_uniq_single_input_allowed(self):
+        assert validate("uniq file.txt") is None
+
+    def test_uniq_count_flag_allowed(self):
+        assert validate("uniq -c file.txt") is None
+
+    def test_uniq_value_flag_not_counted_as_operand(self):
+        # -f consumes '2'; only one operand (file.txt) remains -> allowed.
+        assert validate("uniq -f 2 file.txt") is None
+
+
 # ---------------------------------------------------------------------------
 # PowerShell cmdlet filtering
 # ---------------------------------------------------------------------------