fix(use_computer): pass application name as a subprocess argument

src/strands_tools/use_computer.py

@@ -24,6 +24,7 @@
 import logging
 import os
 import platform
+import re
 import subprocess
 import time
 from datetime import datetime
@@ -642,6 +643,19 @@ def extract_text_from_image(image_path: str, min_confidence: float = 0.5) -> Lis
     return results
 
 
+# The application name is passed to launch/focus mechanisms as a separate
+# argument (never interpolated into a shell command or script body), so normal
+# printable names cannot be parsed as code. As light defense-in-depth we still
+# reject control characters and newlines, which have no place in an app name and
+# could otherwise confuse logging or downstream tools.
+_CONTROL_CHARS = re.compile(r"[\x00-\x1f\x7f]")
+
+
+def _is_valid_app_name(app_name: str) -> bool:
+    """Return True if app_name is non-empty and contains no control characters."""
+    return bool(app_name) and _CONTROL_CHARS.search(app_name) is None
+
+
 def open_application(app_name: str) -> str:
     """
     Launch an application cross-platform.
@@ -658,7 +672,8 @@ def open_application(app_name: str) -> str:
         str: Success or error message detailing the result of the operation.
 
     Platform Support:
-        - Windows: Uses the 'start' command
+        - Windows: Uses os.startfile, which launches via the shell association
+          without going through cmd.exe
         - macOS: Uses the 'open -a' command
         - Linux: Attempts to run app_name directly as a command
     """
@@ -682,10 +697,24 @@ def open_application(app_name: str) -> str:
     # Use mapped name if available, otherwise use original
     actual_app_name = app_mappings.get(app_name.lower(), app_name)
 
+    # Names not covered by the known mapping must be plain, printable names.
+    if app_name.lower() not in app_mappings and not _is_valid_app_name(app_name):
+        return f"Invalid application name: '{app_name}'"
+
     try:
         if system == "windows":
-            result = subprocess.run(f"start {actual_app_name}", shell=True, capture_output=True, text=True)
-        elif system == "darwin":  # macOS
+            # Use os.startfile rather than 'cmd /c start'. Even with shell=False,
+            # subprocess joins the argv into a command line via list2cmdline, which
+            # only quotes items containing whitespace. A spaceless payload such as
+            # "notepad&whoami" would therefore be handed to cmd.exe unquoted and the
+            # '&' re-parsed as a command separator, allowing command injection.
+            # os.startfile launches the app/document through the shell association
+            # API directly and never invokes cmd.exe, so metacharacters like
+            # & | < > ^ ( ) % stay inert.
+            os.startfile(actual_app_name)
+            return f"Launched {actual_app_name}"
+
+        if system == "darwin":  # macOS
             result = subprocess.run(["open", "-a", actual_app_name], capture_output=True, text=True)
         elif system == "linux":
             result = subprocess.run([actual_app_name.lower()], capture_output=True, text=True)
@@ -741,14 +770,22 @@ def focus_application(app_name: str, timeout: float = 2.0) -> bool:
     system = platform.system().lower()
     start_time = time.time()
 
+    if not _is_valid_app_name(app_name):
+        logger.warning(f"Invalid application name for focus: {app_name}")
+        return False
+
     try:
         if system == "darwin":  # macOS
-            # Use AppleScript to bring app to front with timeout
-            script = f'tell application "{app_name}" to activate'
+            # Pass the application name as a script argument (available via 'argv')
+            # instead of interpolating it into the AppleScript source, so the name
+            # is treated as data rather than code.
+            script = "on run argv\n  tell application (item 1 of argv) to activate\nend run"
 
             # Set up a process with timeout
             try:
-                result = subprocess.run(["osascript", "-e", script], check=True, capture_output=True, timeout=timeout)
+                result = subprocess.run(
+                    ["osascript", "-e", script, app_name], check=True, capture_output=True, timeout=timeout
+                )
                 if result.returncode != 0:
                     logger.warning(f"Focus application returned non-zero exit code: {result.returncode}")
                     return False
@@ -763,14 +800,16 @@ def focus_application(app_name: str, timeout: float = 2.0) -> bool:
                 return False
 
         elif system == "windows":
-            # Use PowerShell to focus window
+            # Pass the application name as a script argument (available via 'args')
+            # instead of interpolating it into the PowerShell source, so the name
+            # is treated as data rather than code.
             script = (
-                f"Add-Type -AssemblyName Microsoft.VisualBasic; "
-                f"[Microsoft.VisualBasic.Interaction]::AppActivate('{app_name}')"
+                "Add-Type -AssemblyName Microsoft.VisualBasic; "
+                "[Microsoft.VisualBasic.Interaction]::AppActivate($args[0])"
             )
             try:
                 result = subprocess.run(
-                    ["powershell", "-Command", script], check=True, capture_output=True, timeout=timeout
+                    ["powershell", "-Command", script, app_name], check=True, capture_output=True, timeout=timeout
                 )
                 if result.returncode != 0:
                     return False

tests/test_use_computer.py

@@ -332,10 +332,72 @@ class TestApplicationManagement:
 
     @pytest.mark.parametrize("system", ["windows", "darwin", "linux"])
     def test_open_application(self, system):
-        with patch("platform.system", return_value=system), patch("subprocess.run") as mock_run:
+        with (
+            patch("platform.system", return_value=system),
+            patch("subprocess.run") as mock_run,
+            patch("os.startfile", create=True) as mock_startfile,
+        ):
             mock_run.return_value = MagicMock(returncode=0)
             result = open_application("test_app")
             assert "Launched" in result
+            if system == "windows":
+                mock_startfile.assert_called_once()
+
+    def test_open_application_windows_uses_startfile(self):
+        """On Windows the app is launched via os.startfile, never through cmd.exe."""
+        with (
+            patch("platform.system", return_value="windows"),
+            patch("subprocess.run") as mock_run,
+            patch("os.startfile", create=True) as mock_startfile,
+        ):
+            open_application("notepad")
+            mock_startfile.assert_called_once_with("notepad")
+            # cmd.exe is never invoked, so there is no shell command line to reparse.
+            mock_run.assert_not_called()
+
+    def test_open_application_injection_payload_passed_as_data(self):
+        """A spaceless command-chaining payload is launched literally, never reparsed by cmd.exe.
+
+        With the previous 'cmd /c start "" <name>' approach, list2cmdline only quotes
+        argv items containing whitespace, so a spaceless payload like "notepad&whoami"
+        produced the command line `cmd /c start "" notepad&whoami` and cmd.exe reparsed
+        '&' as a command separator (command injection). os.startfile receives the exact
+        literal string and never invokes a command interpreter.
+        """
+        payload = "notepad&whoami"
+        with (
+            patch("platform.system", return_value="windows"),
+            patch("subprocess.run") as mock_run,
+            patch("os.startfile", create=True) as mock_startfile,
+        ):
+            open_application(payload)
+            # The exact literal payload is passed to startfile with no shell involved.
+            mock_startfile.assert_called_once_with(payload)
+            # cmd.exe is never spawned, so the payload cannot be split on '&'.
+            mock_run.assert_not_called()
+
+    def test_open_application_rejects_control_characters(self):
+        """A name containing control characters is rejected before any launch attempt."""
+        with (
+            patch("platform.system", return_value="windows"),
+            patch("subprocess.run") as mock_run,
+            patch("os.startfile", create=True) as mock_startfile,
+        ):
+            result = open_application("notepad\nwhoami")
+            assert "Invalid application name" in result
+            mock_run.assert_not_called()
+            mock_startfile.assert_not_called()
+
+    def test_open_application_accepts_plus_in_name(self):
+        """A legitimate name like 'C++ Builder' is accepted and launched literally."""
+        with (
+            patch("platform.system", return_value="windows"),
+            patch("subprocess.run") as mock_run,
+            patch("os.startfile", create=True) as mock_startfile,
+        ):
+            open_application("C++ Builder")
+            mock_startfile.assert_called_once_with("C++ Builder")
+            mock_run.assert_not_called()
 
     def test_close_application(self):
         mock_process = MagicMock()
@@ -627,16 +689,25 @@ class TestFocusApplication:
     @pytest.mark.parametrize(
         "system,expected_command",
         [
-            ("darwin", ["osascript", "-e", 'tell application "TestApp" to activate']),
+            (
+                "darwin",
+                [
+                    "osascript",
+                    "-e",
+                    "on run argv\n  tell application (item 1 of argv) to activate\nend run",
+                    "TestApp",
+                ],
+            ),
             (
                 "windows",
                 [
                     "powershell",
                     "-Command",
                     (
                         "Add-Type -AssemblyName Microsoft.VisualBasic; "
-                        "[Microsoft.VisualBasic.Interaction]::AppActivate('TestApp')"
+                        "[Microsoft.VisualBasic.Interaction]::AppActivate($args[0])"
                     ),
+                    "TestApp",
                 ],
             ),
             ("linux", ["wmctrl", "-a", "TestApp"]),
@@ -675,6 +746,54 @@ def test_focus_application_unknown_system(self):
             result = focus_application("TestApp")
             assert result is False
 
+    def test_focus_application_injection_payload_passed_as_data_macos(self):
+        """An AppleScript injection payload reaches osascript only as inert trailing argv data."""
+        from src.strands_tools.use_computer import focus_application
+
+        payload = 'TestApp" & do shell script "echo PWNED > /tmp/pwned'
+        with patch("platform.system", return_value="darwin"), patch("subprocess.run") as mock_run, patch("time.sleep"):
+            mock_run.return_value = MagicMock(returncode=0)
+            focus_application(payload)
+            args = mock_run.call_args[0][0]
+            # The script body is the constant 'on run argv' template; the payload
+            # is only the trailing argv item, so it is never parsed as AppleScript.
+            assert args[2] == "on run argv\n  tell application (item 1 of argv) to activate\nend run"
+            assert args[-1] == payload
+            assert "do shell script" not in args[2]
+
+    def test_focus_application_rejects_control_characters_macos(self):
+        """A name containing control characters is rejected before any subprocess call."""
+        from src.strands_tools.use_computer import focus_application
+
+        with patch("platform.system", return_value="darwin"), patch("subprocess.run") as mock_run, patch("time.sleep"):
+            mock_run.return_value = MagicMock(returncode=0)
+            assert focus_application("TestApp\nactivate") is False
+            mock_run.assert_not_called()
+
+    def test_focus_application_accepts_plus_in_name_macos(self):
+        """A legitimate name like 'C++ Builder' is accepted and passed as a trailing argv item."""
+        from src.strands_tools.use_computer import focus_application
+
+        with patch("platform.system", return_value="darwin"), patch("subprocess.run") as mock_run, patch("time.sleep"):
+            mock_run.return_value = MagicMock(returncode=0)
+            focus_application("C++ Builder")
+            args = mock_run.call_args[0][0]
+            assert args[-1] == "C++ Builder"
+            assert args[2] == "on run argv\n  tell application (item 1 of argv) to activate\nend run"
+
+    def test_focus_application_script_is_static_macos(self):
+        """A benign name reaches osascript as data, with a constant script body."""
+        from src.strands_tools.use_computer import focus_application
+
+        with patch("platform.system", return_value="darwin"), patch("subprocess.run") as mock_run, patch("time.sleep"):
+            mock_run.return_value = MagicMock(returncode=0)
+            focus_application("Safari")
+            args = mock_run.call_args[0][0]
+            # The app name is the trailing argv item, not interpolated into the script body.
+            assert args[-1] == "Safari"
+            assert "Safari" not in args[2]
+            assert "do shell script" not in args[2]
+
 
 class TestHandleAnalyzeScreenshotPytesseract:
     """Tests for screenshot analysis handling"""