fix(use_computer): pass application name as a subprocess argument - #519
Draft
yonib05 wants to merge 3 commits into
Draft
fix(use_computer): pass application name as a subprocess argument#519yonib05 wants to merge 3 commits into
yonib05 wants to merge 3 commits into
Conversation
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 <name>" 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.
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.
yonib05
marked this pull request as draft
June 29, 2026 21:46
…d.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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
open_applicationandfocus_applicationbuilt their subprocess command or script by string-interpolating the application name. This changes them to pass the name as data rather than as part of the command text, which also avoids quoting issues for names with spaces or punctuation.open_application(Windows): usesubprocess.run(["cmd", "/c", "start", "", name], shell=False)instead ofsubprocess.run(f"start {name}", shell=True).focus_application(macOS): the AppleScript body is now constant (on run argv ... item 1 of argv ...) and the name is passed as anosascriptargv item.focus_application(Windows): PowerShell usesAppActivate($args[0])with the name passed as an argument.Adds regression tests covering the new argument forms and rejected names. Full
use_computersuite passes;ruff format/ruff checkclean.