fix(command-mode): close three confirm-gate bypasses: absolute path, find -delete/-exec rm, diskutil

Fluid.xcodeproj/project.pbxproj

@@ -13,6 +13,7 @@
 		7C5AF14C2F15041600DE21B0 /* MediaRemoteAdapter in Embed Frameworks */ = {isa = PBXBuildFile; productRef = 7C5AF14A2F15041600DE21B0 /* MediaRemoteAdapter */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; };
 		7C9A71022F58B00000FB7CAF /* TranscribeCpp in Frameworks */ = {isa = PBXBuildFile; productRef = 7C9A71012F58B00000FB7CAF /* TranscribeCpp */; };
 		7C91B0012F42AA0100C0DEF0 /* HotkeyShortcutTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7C91B0022F42AA0100C0DEF0 /* HotkeyShortcutTests.swift */; };
+		1A6BEFDDF86DAE98C18BC3BF /* CommandModeDestructiveCommandGapTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 96B2C564157D276092AC9C7D /* CommandModeDestructiveCommandGapTests.swift */; };
 		C0DE89000000000000000002 /* SettingsNavigationStateTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C0DE89000000000000000001 /* SettingsNavigationStateTests.swift */; };
 		7CDB0A2D2F3C4D5600FB7CAD /* DictationE2ETests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7CDB0A292F3C4D5600FB7CAD /* DictationE2ETests.swift */; };
 		B52000000000000000000002 /* PrivateAIDictationTokenBudgetTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B52000000000000000000001 /* PrivateAIDictationTokenBudgetTests.swift */; };
@@ -78,6 +79,7 @@
 		C0DE63700000000000000001 /* WhisperLanguageSelectionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WhisperLanguageSelectionTests.swift; sourceTree = "<group>"; };
 		7C078D8F2E3B339200FB7CAC /* FluidVoice Debug.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = "FluidVoice Debug.app"; sourceTree = BUILT_PRODUCTS_DIR; };
 		7C91B0022F42AA0100C0DEF0 /* HotkeyShortcutTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HotkeyShortcutTests.swift; sourceTree = "<group>"; };
+		96B2C564157D276092AC9C7D /* CommandModeDestructiveCommandGapTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CommandModeDestructiveCommandGapTests.swift; sourceTree = "<group>"; };
 		C0DE89000000000000000001 /* SettingsNavigationStateTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsNavigationStateTests.swift; sourceTree = "<group>"; };
 		7CDB0A202F3C4D5600FB7CAD /* FluidDictationIntegrationTests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = FluidDictationIntegrationTests.xctest; sourceTree = BUILT_PRODUCTS_DIR; };
 		7CDB0A292F3C4D5600FB7CAD /* DictationE2ETests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DictationE2ETests.swift; sourceTree = "<group>"; };
@@ -157,6 +159,7 @@
 				7CDB0A292F3C4D5600FB7CAD /* DictationE2ETests.swift */,
 				CD1C7A0000000000000000B1 /* CustomDictionaryManualEntryTests.swift */,
 				7C91B0022F42AA0100C0DEF0 /* HotkeyShortcutTests.swift */,
+				96B2C564157D276092AC9C7D /* CommandModeDestructiveCommandGapTests.swift */,
 				C0DE89000000000000000001 /* SettingsNavigationStateTests.swift */,
 				343B29013F4441D6A797D12D /* LLMClientRequestBodyTests.swift */,
 				980330F3CE464336ADCE3E23 /* TemperatureSupportTests.swift */,
@@ -333,6 +336,7 @@
 				B52000000000000000000002 /* PrivateAIDictationTokenBudgetTests.swift in Sources */,
 				CD1C7A0000000000000000B2 /* CustomDictionaryManualEntryTests.swift in Sources */,
 				7C91B0012F42AA0100C0DEF0 /* HotkeyShortcutTests.swift in Sources */,
+				1A6BEFDDF86DAE98C18BC3BF /* CommandModeDestructiveCommandGapTests.swift in Sources */,
 				C0DE89000000000000000002 /* SettingsNavigationStateTests.swift in Sources */,
 				86CAA2D4EF18433096185602 /* LLMClientRequestBodyTests.swift in Sources */,
 				272BFB5CB271489892CAE50C /* TemperatureSupportTests.swift in Sources */,

Sources/Fluid/Services/CommandModeService.swift

@@ -437,7 +437,7 @@ final class CommandModeService: ObservableObject {
                 }
 
                 // Check if we need confirmation for destructive commands
-                if SettingsStore.shared.commandModeConfirmBeforeExecute, self.isDestructiveCommand(tc.command) {
+                if SettingsStore.shared.commandModeConfirmBeforeExecute, Self.isDestructiveCommand(tc.command) {
                     self.pendingCommand = PendingCommand(
                         id: tc.id,
                         command: tc.command,
@@ -559,48 +559,149 @@ final class CommandModeService: ObservableObject {
         }
     }
 
-    private func isDestructiveCommand(_ command: String) -> Bool {
-        let cmd = command.lowercased()
-
-        // Commands that start with these are destructive
-        let destructivePrefixes = [
-            "rm ", "rm\t", "rmdir ", "rm -", // delete
-            "mv ", "mv\t", // move/rename
-            "sudo ", // elevated privileges
-            "kill ", "pkill ", "killall ", // terminate processes
-            "chmod ", "chown ", "chgrp ", // change permissions/ownership
-            "dd ", // disk operations
-            "mkfs", "format", // filesystem formatting
-            "> ", // overwrite file
-            "truncate ", // truncate file
-            "shred ", // secure delete
-        ]
-
-        // Check if command starts with any destructive prefix
-        if destructivePrefixes.contains(where: { cmd.hasPrefix($0) }) {
-            return true
+    /// Splits a command into the simple commands a shell would run, on unquoted
+    /// `&&`, `||`, `;`, `&`, `|`, and newline, so every command in a chain gets
+    /// checked, not just the first one before a separator.
+    /// Splits on unquoted `;`, `&&`, `||`, `|`, newline. Quotes are tracked so a
+    /// separator inside a quoted argument doesn't split, and a backslash-escaped
+    /// quote doesn't close early, real shells honor `\"` inside a double-quoted
+    /// string as a literal character, not the end of the string.
+    private nonisolated static func splitIntoSimpleCommands(_ command: String) -> [String] {
+        var parts: [String] = []
+        var current = ""
+        var quote: Character?
+        var escaped = false
+        for c in command {
+            if escaped {
+                current.append(c)
+                escaped = false
+                continue
+            }
+            if c == "\\", quote != "'" {
+                current.append(c)
+                escaped = true
+                continue
+            }
+            if let q = quote {
+                current.append(c)
+                if c == q { quote = nil }
+                continue
+            }
+            if c == "\"" || c == "'" {
+                quote = c
+                current.append(c)
+            } else if ";&|\n".contains(c) {
+                parts.append(current)
+                current = ""
+            } else {
+                current.append(c)
+            }
         }
+        parts.append(current)
+        return parts.map { $0.trimmingCharacters(in: .whitespaces) }.filter { !$0.isEmpty }
+    }
 
-        // Check for destructive patterns anywhere in piped commands
-        let destructivePatterns = [
-            "| rm ", "| sudo ", "| dd ",
-            "; rm ", "; sudo ",
-            "&& rm ", "&& sudo ",
-            "xargs rm", "xargs -I",
-        ]
-
-        if destructivePatterns.contains(where: { cmd.contains($0) }) {
-            return true
+    /// Characters that mean this simple command isn't plain text: a redirect, a
+    /// subshell, or command substitution. Any of these riding along with an
+    /// otherwise safe command means it isn't actually safe, `echo $(rm -rf x)`
+    /// is not safe just because `echo` is. Filenames with these characters will
+    /// also require confirmation; that's an acceptable cost of not being a full
+    /// shell parser.
+    private static let unsafeCharacters = CharacterSet(charactersIn: "$`(){}<>")
+
+    /// `$` and backtick stay live for substitution inside double quotes; a real
+    /// shell still evaluates `"$(rm -rf x)"`. Everything else in
+    /// `unsafeCharacters` (redirects, braces, parens on their own) is inert
+    /// inside double quotes, same as any other literal character.
+    private static let unsafeInsideDoubleQuotes = CharacterSet(charactersIn: "$`")
+
+    /// True if an unsafe character appears where it's actually live: unquoted,
+    /// or (for `$`/backtick specifically) inside double quotes. Single quotes
+    /// suppress everything, and an escaped character is a literal, not a
+    /// redirect or substitution, `echo '>'` and `echo \>` pass an ordinary
+    /// argument.
+    private nonisolated static func hasUnquotedUnsafeCharacter(_ simpleCommand: String) -> Bool {
+        var quote: Character?
+        var escaped = false
+        for c in simpleCommand {
+            if escaped {
+                escaped = false
+                continue
+            }
+            if c == "\\", quote != "'" {
+                escaped = true
+                continue
+            }
+            if let q = quote {
+                if c == q {
+                    quote = nil
+                } else if q == "\"", c.unicodeScalars.contains(where: unsafeInsideDoubleQuotes.contains) {
+                    return true
+                }
+                continue
+            }
+            if c == "\"" || c == "'" {
+                quote = c
+            } else if c.unicodeScalars.contains(where: unsafeCharacters.contains) {
+                return true
+            }
         }
+        return false
+    }
 
-        // rm with flags like -rf, -r, -f anywhere
-        if cmd.contains("rm -") {
-            return true
+    /// The leading word of a simple command, with a leading env assignment
+    /// dropped (`LC_ALL=C ls` runs `ls`) and surrounding quotes stripped if the
+    /// whole word is one quoted span.
+    private nonisolated static func leadingCommand(_ simpleCommand: String) -> String? {
+        var words = simpleCommand.split(separator: " ").map(String.init)
+        while let first = words.first, isEnvironmentAssignmentWord(first) {
+            words.removeFirst()
+        }
+        guard var word = words.first else { return nil }
+        if word.count >= 2, let q = word.first, (q == "\"" || q == "'"), word.last == q {
+            word.removeFirst()
+            word.removeLast()
         }
+        return word
+    }
 
-        return false
+    /// `NAME=value` in leading position scopes an env var to the command that
+    /// follows; `LC_ALL=C rm -rf victim` runs `rm`, not `lc_all=c`.
+    private nonisolated static func isEnvironmentAssignmentWord(_ word: String) -> Bool {
+        guard let equalsIndex = word.firstIndex(of: "=") else { return false }
+        let name = word[word.startIndex..<equalsIndex]
+        guard let first = name.first, first.isLetter || first == "_" else { return false }
+        return name.allSatisfy { $0.isLetter || $0.isNumber || $0 == "_" }
+    }
+
+    /// Commands that never take a destructive action regardless of arguments or
+    /// flags. Short and conservative on purpose: anything not recognized asks
+    /// for confirmation, rather than trying to enumerate every way a command
+    /// could be dangerous. Add a command only after auditing every invocation.
+    private static let knownSafeCommands: Set<String> = [
+        "ls", "cat", "echo", "pwd", "whoami", "date", "hostname", "uname",
+        "head", "tail", "wc", "file", "stat", "which", "type", "printenv",
+    ]
+
+    /// A path-qualified command is only trusted if it also lives in one of
+    /// these directories. A bare name resolving to `ls` is the real system
+    /// `ls`; a path like `./ls` or `/tmp/cat` could be an arbitrary planted
+    /// script that merely shares the name.
+    private static let trustedDirectories: Set<String> = ["/bin", "/usr/bin", "/sbin", "/usr/sbin"]
+
+    nonisolated static func isDestructiveCommand(_ command: String) -> Bool {
+        splitIntoSimpleCommands(command.lowercased()).contains { !isSafeSimpleCommand($0) }
     }
 
+    private nonisolated static func isSafeSimpleCommand(_ simpleCommand: String) -> Bool {
+        guard !hasUnquotedUnsafeCharacter(simpleCommand) else { return false }
+        guard let name = leadingCommand(simpleCommand) else { return true }
+        guard knownSafeCommands.contains((name as NSString).lastPathComponent) else { return false }
+        guard name.contains("/") else { return true }
+        return trustedDirectories.contains((name as NSString).deletingLastPathComponent)
+    }
+
+
     private func executeCommand(_ command: String, workingDirectory: String?, callId: String, purpose: String? = nil) async {
         self.currentStep = .executing(command)