Repository navigation
Fix command injection in open_web via shell=True - #3
Conversation
There was a problem hiding this comment.
Pull request overview
This PR removes a command-injection path in open_web by eliminating shell=True execution and passing URLs to xdg-open as a literal argv element (with Flatpak support via flatpak-spawn --host).
Changes:
- Reworked
open_webto usesubprocess.Popen([...])(argv list) instead of shell-interpolated command strings. - Removed the generic
run_commandhelper and updated the onboarding support button to callopen_webdirectly.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/windows/Onboarding/OnboardingWindow.py |
Switches the support link handler from run_command(...) to open_web(...) and removes now-unused imports. |
src/backend/DeckManagement/HelperMethods.py |
Removes run_command and updates open_web to launch xdg-open (and flatpak-spawn --host) without shell interpretation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot I fixed the issue you found - please redo your review. |
Re-reviewed the current PR state; the launcher path now stays shell-free and missing launcher binaries are handled gracefully in |
open_webpassed a shell-interpolated URL string tosubprocess.Popenwithshell=True, allowing shell metacharacters in URLs sourced from store metadata to execute arbitrary commands.Changes
open_web: Replacedrun_command(f"xdg-open {url}")with a directsubprocess.Popencall using an argv list, eliminating shell interpretation of the URL entirely.flatpak-spawn --hostis prepended as discrete argv elements rather than concatenated into a shell string.