From 264b1cad9a23131bad8ede3b4f0613e3dcb9002e Mon Sep 17 00:00:00 2001 From: MackDing Date: Fri, 22 May 2026 10:01:39 +0800 Subject: [PATCH 1/2] fix: avoid shell interpolation in permission dialogs --- gui_agents/s1/cli_app.py | 35 +++++++++++++++++++++++++++++------ gui_agents/s2/cli_app.py | 35 +++++++++++++++++++++++++++++------ gui_agents/s2_5/cli_app.py | 35 +++++++++++++++++++++++++++++------ gui_agents/s3/cli_app.py | 35 +++++++++++++++++++++++++++++------ 4 files changed, 116 insertions(+), 24 deletions(-) diff --git a/gui_agents/s1/cli_app.py b/gui_agents/s1/cli_app.py index 68ac3d5c..a2b0f57a 100644 --- a/gui_agents/s1/cli_app.py +++ b/gui_agents/s1/cli_app.py @@ -5,6 +5,7 @@ import os import platform import signal +import subprocess import sys import time @@ -138,16 +139,38 @@ def signal_handler(signum, frame): def show_permission_dialog(code: str, action_description: str): """Show a platform-specific permission dialog and return True if approved.""" + prompt_text = ( + "Do you want to execute this action?\n\n" + f"{code}" + ) + if action_description: + prompt_text += f" which will try to {action_description}" + if platform.system() == "Darwin": - result = os.system( - f'osascript -e \'display dialog "Do you want to execute this action?\n\n{code} which will try to {action_description}" with title "Action Permission" buttons {{"Cancel", "OK"}} default button "OK" cancel button "Cancel"\'' + script = ( + 'display dialog argv item 1 with title "Action Permission" ' + 'buttons {"Cancel", "OK"} default button "OK" cancel button "Cancel"' + ) + result = subprocess.run( + ["osascript", "-e", script, prompt_text], + check=False, ) - return result == 0 + return result.returncode == 0 elif platform.system() == "Linux": - result = os.system( - f'zenity --question --title="Action Permission" --text="Do you want to execute this action?\n\n{code}" --width=400 --height=200' + result = subprocess.run( + [ + "zenity", + "--question", + "--title", + "Action Permission", + "--text", + prompt_text, + "--width=400", + "--height=200", + ], + check=False, ) - return result == 0 + return result.returncode == 0 return False diff --git a/gui_agents/s2/cli_app.py b/gui_agents/s2/cli_app.py index f03aa8e1..f909c005 100644 --- a/gui_agents/s2/cli_app.py +++ b/gui_agents/s2/cli_app.py @@ -6,6 +6,7 @@ import platform import pyautogui import signal +import subprocess import sys import time @@ -131,16 +132,38 @@ def signal_handler(signum, frame): def show_permission_dialog(code: str, action_description: str): """Show a platform-specific permission dialog and return True if approved.""" + prompt_text = ( + "Do you want to execute this action?\n\n" + f"{code}" + ) + if action_description: + prompt_text += f" which will try to {action_description}" + if platform.system() == "Darwin": - result = os.system( - f'osascript -e \'display dialog "Do you want to execute this action?\n\n{code} which will try to {action_description}" with title "Action Permission" buttons {{"Cancel", "OK"}} default button "OK" cancel button "Cancel"\'' + script = ( + 'display dialog argv item 1 with title "Action Permission" ' + 'buttons {"Cancel", "OK"} default button "OK" cancel button "Cancel"' + ) + result = subprocess.run( + ["osascript", "-e", script, prompt_text], + check=False, ) - return result == 0 + return result.returncode == 0 elif platform.system() == "Linux": - result = os.system( - f'zenity --question --title="Action Permission" --text="Do you want to execute this action?\n\n{code}" --width=400 --height=200' + result = subprocess.run( + [ + "zenity", + "--question", + "--title", + "Action Permission", + "--text", + prompt_text, + "--width=400", + "--height=200", + ], + check=False, ) - return result == 0 + return result.returncode == 0 return False diff --git a/gui_agents/s2_5/cli_app.py b/gui_agents/s2_5/cli_app.py index a6e34fa8..6944da54 100644 --- a/gui_agents/s2_5/cli_app.py +++ b/gui_agents/s2_5/cli_app.py @@ -6,6 +6,7 @@ import platform import pyautogui import signal +import subprocess import sys import time @@ -131,16 +132,38 @@ def signal_handler(signum, frame): def show_permission_dialog(code: str, action_description: str): """Show a platform-specific permission dialog and return True if approved.""" + prompt_text = ( + "Do you want to execute this action?\n\n" + f"{code}" + ) + if action_description: + prompt_text += f" which will try to {action_description}" + if platform.system() == "Darwin": - result = os.system( - f'osascript -e \'display dialog "Do you want to execute this action?\n\n{code} which will try to {action_description}" with title "Action Permission" buttons {{"Cancel", "OK"}} default button "OK" cancel button "Cancel"\'' + script = ( + 'display dialog argv item 1 with title "Action Permission" ' + 'buttons {"Cancel", "OK"} default button "OK" cancel button "Cancel"' + ) + result = subprocess.run( + ["osascript", "-e", script, prompt_text], + check=False, ) - return result == 0 + return result.returncode == 0 elif platform.system() == "Linux": - result = os.system( - f'zenity --question --title="Action Permission" --text="Do you want to execute this action?\n\n{code}" --width=400 --height=200' + result = subprocess.run( + [ + "zenity", + "--question", + "--title", + "Action Permission", + "--text", + prompt_text, + "--width=400", + "--height=200", + ], + check=False, ) - return result == 0 + return result.returncode == 0 return False diff --git a/gui_agents/s3/cli_app.py b/gui_agents/s3/cli_app.py index 55816be0..aceabca1 100644 --- a/gui_agents/s3/cli_app.py +++ b/gui_agents/s3/cli_app.py @@ -6,6 +6,7 @@ import platform import pyautogui import signal +import subprocess import sys import time @@ -132,16 +133,38 @@ def signal_handler(signum, frame): def show_permission_dialog(code: str, action_description: str): """Show a platform-specific permission dialog and return True if approved.""" + prompt_text = ( + "Do you want to execute this action?\n\n" + f"{code}" + ) + if action_description: + prompt_text += f" which will try to {action_description}" + if platform.system() == "Darwin": - result = os.system( - f'osascript -e \'display dialog "Do you want to execute this action?\n\n{code} which will try to {action_description}" with title "Action Permission" buttons {{"Cancel", "OK"}} default button "OK" cancel button "Cancel"\'' + script = ( + 'display dialog argv item 1 with title "Action Permission" ' + 'buttons {"Cancel", "OK"} default button "OK" cancel button "Cancel"' + ) + result = subprocess.run( + ["osascript", "-e", script, prompt_text], + check=False, ) - return result == 0 + return result.returncode == 0 elif platform.system() == "Linux": - result = os.system( - f'zenity --question --title="Action Permission" --text="Do you want to execute this action?\n\n{code}" --width=400 --height=200' + result = subprocess.run( + [ + "zenity", + "--question", + "--title", + "Action Permission", + "--text", + prompt_text, + "--width=400", + "--height=200", + ], + check=False, ) - return result == 0 + return result.returncode == 0 return False From 3304c1428864a74a8053c13a5292820749076635 Mon Sep 17 00:00:00 2001 From: Blossom Date: Thu, 6 Aug 2026 12:40:42 +0800 Subject: [PATCH 2/2] test: cover permission dialog argument handling --- tests/test_permission_dialogs.py | 72 ++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 tests/test_permission_dialogs.py diff --git a/tests/test_permission_dialogs.py b/tests/test_permission_dialogs.py new file mode 100644 index 00000000..99aa8084 --- /dev/null +++ b/tests/test_permission_dialogs.py @@ -0,0 +1,72 @@ +import ast +import subprocess +import unittest +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import Mock + + +CLI_APPS = ( + "gui_agents/s1/cli_app.py", + "gui_agents/s2/cli_app.py", + "gui_agents/s2_5/cli_app.py", + "gui_agents/s3/cli_app.py", +) + + +def load_permission_dialog(path: str): + """Load only the dialog helper so CLI module side effects are not executed.""" + source_path = Path(path) + tree = ast.parse(source_path.read_text(encoding="utf-8"), filename=path) + function = next( + node + for node in tree.body + if isinstance(node, ast.FunctionDef) and node.name == "show_permission_dialog" + ) + module = SimpleNamespace( + platform=SimpleNamespace(system=Mock()), + subprocess=SimpleNamespace(run=Mock()), + ) + namespace = {"platform": module.platform, "subprocess": module.subprocess} + exec(compile(ast.Module(body=[function], type_ignores=[]), path, "exec"), namespace) + return namespace["show_permission_dialog"], module + + +class PermissionDialogTests(unittest.TestCase): + def test_macos_passes_model_text_as_osascript_argv(self): + model_text = '"; do shell script "touch /tmp/pwned"; "' + + for path in CLI_APPS: + with self.subTest(path=path): + show_permission_dialog, module = load_permission_dialog(path) + module.platform.system.return_value = "Darwin" + module.subprocess.run.return_value = subprocess.CompletedProcess([], 0) + + self.assertTrue(show_permission_dialog(model_text, "open settings")) + + argv = module.subprocess.run.call_args.args[0] + self.assertEqual(argv[:2], ["osascript", "-e"]) + self.assertIn("argv item 1", argv[2]) + self.assertNotIn(model_text, argv[2]) + self.assertIn(model_text, argv[3]) + self.assertEqual(module.subprocess.run.call_args.kwargs, {"check": False}) + + def test_linux_passes_model_text_as_zenity_argv(self): + model_text = '$(touch /tmp/pwned); `id`; "quoted"' + + for path in CLI_APPS: + with self.subTest(path=path): + show_permission_dialog, module = load_permission_dialog(path) + module.platform.system.return_value = "Linux" + module.subprocess.run.return_value = subprocess.CompletedProcess([], 1) + + self.assertFalse(show_permission_dialog(model_text, "open settings")) + + argv = module.subprocess.run.call_args.args[0] + self.assertEqual(argv[0], "zenity") + self.assertEqual(argv[argv.index("--text") + 1].count(model_text), 1) + self.assertEqual(module.subprocess.run.call_args.kwargs, {"check": False}) + + +if __name__ == "__main__": + unittest.main()