From 9ef44c20ad9d4aafb34ea9124c521d2416a1980b Mon Sep 17 00:00:00 2001 From: Jonathan Segev Date: Sat, 27 Jun 2026 10:55:19 -0400 Subject: [PATCH 1/3] fix(use_computer): pass application name as a subprocess argument Pass the application name to the platform launch and focus mechanisms as a separate argument rather than interpolating it into the command string or script source, so the name is always treated as data. - open_application: use the list form ["cmd", "/c", "start", "", name] with shell=False on Windows instead of a formatted "start " string. - focus_application: pass the name to osascript via "on run argv" and to PowerShell via $args[0], keeping the script body constant. - Validate that names not in the known mapping are plain, printable names. Adds regression tests covering the argument forms and name validation. --- src/strands_tools/use_computer.py | 47 ++++++++++++++++++++++----- tests/test_use_computer.py | 54 +++++++++++++++++++++++++++++-- 2 files changed, 91 insertions(+), 10 deletions(-) diff --git a/src/strands_tools/use_computer.py b/src/strands_tools/use_computer.py index 45db9b42..48321655 100644 --- a/src/strands_tools/use_computer.py +++ b/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,17 @@ def extract_text_from_image(image_path: str, min_confidence: float = 0.5) -> Lis return results +# Application names are expected to be plain, printable names (letters, digits, +# spaces, and a few common punctuation characters). Reject anything else so that +# names are always treated as data by the underlying launch/focus mechanisms. +_VALID_APP_NAME = re.compile(r"^[\w\s.\-()&']+$") + + +def _is_valid_app_name(app_name: str) -> bool: + """Return True if app_name is a plain, printable application name.""" + return bool(app_name) and _VALID_APP_NAME.fullmatch(app_name) is not None + + def open_application(app_name: str) -> str: """ Launch an application cross-platform. @@ -682,9 +694,18 @@ 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) + # Pass the application name as a separate argument (shell=False) so it + # is treated as data by 'start' rather than parsed by the shell. The + # empty string is the optional window title argument for 'start'. + result = subprocess.run( + ["cmd", "/c", "start", "", actual_app_name], capture_output=True, text=True, shell=False + ) elif system == "darwin": # macOS result = subprocess.run(["open", "-a", actual_app_name], capture_output=True, text=True) elif system == "linux": @@ -741,14 +762,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 +792,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 diff --git a/tests/test_use_computer.py b/tests/test_use_computer.py index a8049e36..617e20cb 100644 --- a/tests/test_use_computer.py +++ b/tests/test_use_computer.py @@ -337,6 +337,23 @@ def test_open_application(self, system): result = open_application("test_app") assert "Launched" in result + def test_open_application_windows_no_shell(self): + """On Windows the app name is passed as a list argument with shell disabled.""" + with patch("platform.system", return_value="windows"), patch("subprocess.run") as mock_run: + mock_run.return_value = MagicMock(returncode=0) + open_application("notepad") + args, kwargs = mock_run.call_args + assert args[0] == ["cmd", "/c", "start", "", "notepad"] + assert kwargs.get("shell") is False + + def test_open_application_rejects_injection_payload(self): + """A command-chaining payload is rejected before any subprocess call.""" + with patch("platform.system", return_value="windows"), patch("subprocess.run") as mock_run: + mock_run.return_value = MagicMock(returncode=0) + result = open_application("notepad & whoami > C:\\temp\\pwned.txt") + assert "Invalid application name" in result + mock_run.assert_not_called() + def test_close_application(self): mock_process = MagicMock() mock_process.info = {"name": "test_app"} @@ -627,7 +644,15 @@ 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", [ @@ -635,8 +660,9 @@ class TestFocusApplication: "-Command", ( "Add-Type -AssemblyName Microsoft.VisualBasic; " - "[Microsoft.VisualBasic.Interaction]::AppActivate('TestApp')" + "[Microsoft.VisualBasic.Interaction]::AppActivate($args[0])" ), + "TestApp", ], ), ("linux", ["wmctrl", "-a", "TestApp"]), @@ -675,6 +701,30 @@ def test_focus_application_unknown_system(self): result = focus_application("TestApp") assert result is False + def test_focus_application_passes_name_as_argument_macos(self): + """The app name is passed as a separate argv item, never inside the script source.""" + 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) + # A payload with a double-quote is rejected by validation before any subprocess call. + assert focus_application(payload) is False + mock_run.assert_not_called() + + 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""" From e5c02f5768646aa4092f6cd57446dc7ea8c39455 Mon Sep 17 00:00:00 2001 From: Jonathan Segev Date: Sat, 27 Jun 2026 21:33:58 -0400 Subject: [PATCH 2/3] fix(use_computer): only reject control characters in application names The application name is passed to launch and focus mechanisms as a separate argument and is never interpolated into a shell command or AppleScript/PowerShell body, so normal printable names cannot be parsed as code. The previous allowlist regex rejected legitimate names such as 'C++ Builder' and names containing colons or other punctuation, while still permitting some punctuation, so it added little value at the cost of a breaking change. Relax validation to reject only control characters and newlines as light defense-in-depth, and keep regression tests asserting injection payloads remain inert argv data. --- src/strands_tools/use_computer.py | 14 ++++---- tests/test_use_computer.py | 59 +++++++++++++++++++++++++++---- 2 files changed, 60 insertions(+), 13 deletions(-) diff --git a/src/strands_tools/use_computer.py b/src/strands_tools/use_computer.py index 48321655..b8cca635 100644 --- a/src/strands_tools/use_computer.py +++ b/src/strands_tools/use_computer.py @@ -643,15 +643,17 @@ def extract_text_from_image(image_path: str, min_confidence: float = 0.5) -> Lis return results -# Application names are expected to be plain, printable names (letters, digits, -# spaces, and a few common punctuation characters). Reject anything else so that -# names are always treated as data by the underlying launch/focus mechanisms. -_VALID_APP_NAME = re.compile(r"^[\w\s.\-()&']+$") +# 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 a plain, printable application name.""" - return bool(app_name) and _VALID_APP_NAME.fullmatch(app_name) is not None + """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: diff --git a/tests/test_use_computer.py b/tests/test_use_computer.py index 617e20cb..361a4816 100644 --- a/tests/test_use_computer.py +++ b/tests/test_use_computer.py @@ -346,14 +346,35 @@ def test_open_application_windows_no_shell(self): assert args[0] == ["cmd", "/c", "start", "", "notepad"] assert kwargs.get("shell") is False - def test_open_application_rejects_injection_payload(self): - """A command-chaining payload is rejected before any subprocess call.""" + def test_open_application_injection_payload_passed_as_data(self): + """A command-chaining payload is passed as a single inert argv item, not parsed by a shell.""" + payload = "notepad & whoami > C:\\temp\\pwned.txt" with patch("platform.system", return_value="windows"), patch("subprocess.run") as mock_run: mock_run.return_value = MagicMock(returncode=0) - result = open_application("notepad & whoami > C:\\temp\\pwned.txt") + open_application(payload) + args, kwargs = mock_run.call_args + # shell is disabled and the entire payload is the trailing list item, + # so '&' is data and never reaches a command interpreter. + assert kwargs.get("shell") is False + assert args[0] == ["cmd", "/c", "start", "", payload] + + def test_open_application_rejects_control_characters(self): + """A name containing control characters is rejected before any subprocess call.""" + with patch("platform.system", return_value="windows"), patch("subprocess.run") as mock_run: + mock_run.return_value = MagicMock(returncode=0) + result = open_application("notepad\nwhoami") assert "Invalid application name" in result mock_run.assert_not_called() + def test_open_application_accepts_plus_in_name(self): + """A legitimate name like 'C++ Builder' is accepted and passed as an argument.""" + with patch("platform.system", return_value="windows"), patch("subprocess.run") as mock_run: + mock_run.return_value = MagicMock(returncode=0) + open_application("C++ Builder") + args, kwargs = mock_run.call_args + assert kwargs.get("shell") is False + assert args[0] == ["cmd", "/c", "start", "", "C++ Builder"] + def test_close_application(self): mock_process = MagicMock() mock_process.info = {"name": "test_app"} @@ -701,17 +722,41 @@ def test_focus_application_unknown_system(self): result = focus_application("TestApp") assert result is False - def test_focus_application_passes_name_as_argument_macos(self): - """The app name is passed as a separate argv item, never inside the script source.""" + 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) - # A payload with a double-quote is rejected by validation before any subprocess call. - assert focus_application(payload) is False + 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 From 46a7f087fd8484ea32cd97ab92a40b915f769cf8 Mon Sep 17 00:00:00 2001 From: Jonathan Segev Date: Wed, 1 Jul 2026 20:32:10 -0400 Subject: [PATCH 3/3] fix(use_computer): launch Windows apps via os.startfile to prevent cmd.exe injection The Windows open_application path launched apps with subprocess.run(["cmd", "/c", "start", "", actual_app_name], shell=False). Even with shell=False, subprocess builds the command line with list2cmdline, which only quotes argv items containing whitespace. A spaceless payload such as "notepad&whoami" therefore produced the command line `cmd /c start "" notepad&whoami`, and cmd.exe reparsed '&' as a command separator, allowing command injection. The _CONTROL_CHARS allowlist does not block cmd metacharacters (& | < > ^ ( ) %). Launch via os.startfile instead, which uses the shell association API directly and never invokes cmd.exe, so metacharacters stay inert and the exact literal name is used. Mapped-name behavior is unchanged, and focus_application (already argv-safe) is untouched. The prior injection test only used a spaced payload, which list2cmdline quotes, masking the bug. Tests now assert os.startfile is called with the exact literal name (including a spaceless "notepad&whoami" payload) and that cmd.exe is never spawned. --- src/strands_tools/use_computer.py | 22 +++++---- tests/test_use_computer.py | 76 ++++++++++++++++++++----------- 2 files changed, 64 insertions(+), 34 deletions(-) diff --git a/src/strands_tools/use_computer.py b/src/strands_tools/use_computer.py index b8cca635..52d6e900 100644 --- a/src/strands_tools/use_computer.py +++ b/src/strands_tools/use_computer.py @@ -672,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 """ @@ -702,13 +703,18 @@ def open_application(app_name: str) -> str: try: if system == "windows": - # Pass the application name as a separate argument (shell=False) so it - # is treated as data by 'start' rather than parsed by the shell. The - # empty string is the optional window title argument for 'start'. - result = subprocess.run( - ["cmd", "/c", "start", "", actual_app_name], capture_output=True, text=True, shell=False - ) - 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) diff --git a/tests/test_use_computer.py b/tests/test_use_computer.py index 361a4816..9d3b7073 100644 --- a/tests/test_use_computer.py +++ b/tests/test_use_computer.py @@ -332,48 +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_no_shell(self): - """On Windows the app name is passed as a list argument with shell disabled.""" - with patch("platform.system", return_value="windows"), patch("subprocess.run") as mock_run: - mock_run.return_value = MagicMock(returncode=0) + 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") - args, kwargs = mock_run.call_args - assert args[0] == ["cmd", "/c", "start", "", "notepad"] - assert kwargs.get("shell") is False + 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 command-chaining payload is passed as a single inert argv item, not parsed by a shell.""" - payload = "notepad & whoami > C:\\temp\\pwned.txt" - with patch("platform.system", return_value="windows"), patch("subprocess.run") as mock_run: - mock_run.return_value = MagicMock(returncode=0) + """A spaceless command-chaining payload is launched literally, never reparsed by cmd.exe. + + With the previous 'cmd /c start "" ' 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) - args, kwargs = mock_run.call_args - # shell is disabled and the entire payload is the trailing list item, - # so '&' is data and never reaches a command interpreter. - assert kwargs.get("shell") is False - assert args[0] == ["cmd", "/c", "start", "", 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 subprocess call.""" - with patch("platform.system", return_value="windows"), patch("subprocess.run") as mock_run: - mock_run.return_value = MagicMock(returncode=0) + """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 passed as an argument.""" - with patch("platform.system", return_value="windows"), patch("subprocess.run") as mock_run: - mock_run.return_value = MagicMock(returncode=0) + """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") - args, kwargs = mock_run.call_args - assert kwargs.get("shell") is False - assert args[0] == ["cmd", "/c", "start", "", "C++ Builder"] + mock_startfile.assert_called_once_with("C++ Builder") + mock_run.assert_not_called() def test_close_application(self): mock_process = MagicMock()