-
-
Notifications
You must be signed in to change notification settings - Fork 34.4k
gh-137586: Replace 'osascript' with 'open' on macOS in webbrowser #146439
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
00fa6c7
f697cd5
7781033
6066221
d54293f
080197e
fdd6649
8e1eef4
e193626
98dd1d8
bdfc2e6
614078f
c77f4b8
c725927
1316dbf
612d276
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -491,10 +491,13 @@ def register_standard_browsers(): | |
| _tryorder = [] | ||
|
|
||
| if sys.platform == 'darwin': | ||
| register("MacOSX", None, MacOSXOSAScript('default')) | ||
| register("chrome", None, MacOSXOSAScript('google chrome')) | ||
| register("firefox", None, MacOSXOSAScript('firefox')) | ||
| register("safari", None, MacOSXOSAScript('safari')) | ||
| register("MacOS", None, MacOS('default')) | ||
| register("chrome", None, MacOS('google chrome')) | ||
| register("chromium", None, MacOS('chromium')) | ||
| register("firefox", None, MacOS('firefox')) | ||
| register("safari", None, MacOS('safari')) | ||
| register("opera", None, MacOS('opera')) | ||
| register("microsoft-edge", None, MacOS('microsoft edge')) | ||
| # macOS can use below Unix support (but we prefer using the macOS | ||
| # specific stuff) | ||
|
|
||
|
|
@@ -613,8 +616,84 @@ def open(self, url, new=0, autoraise=True): | |
| # | ||
|
|
||
| if sys.platform == 'darwin': | ||
| def _macos_default_browser_bundle_id(): | ||
| """Return the bundle ID of the default web browser. | ||
|
|
||
| Reads the LaunchServices preferences file that macOS maintains | ||
| when the user sets a default browser. Returns None if the file | ||
| is absent or no https handler is recorded. | ||
| """ | ||
| import plistlib, os | ||
| plist = os.path.expanduser( | ||
| '~/Library/Preferences/com.apple.LaunchServices/' | ||
| 'com.apple.launchservices.secure.plist' | ||
| ) | ||
| try: | ||
| with open(plist, 'rb') as f: | ||
| data = plistlib.load(f) | ||
| for handler in data.get('LSHandlers', []): | ||
| if handler.get('LSHandlerURLScheme') == 'https': | ||
| return (handler.get('LSHandlerRoleAll') | ||
| or handler.get('LSHandlerRoleViewer')) | ||
| except Exception: | ||
| pass | ||
| return None | ||
|
|
||
| class MacOS(BaseBrowser): | ||
| """Launcher class for macOS browsers, using /usr/bin/open. | ||
|
|
||
| For http/https URLs with the default browser, /usr/bin/open is called | ||
| directly; macOS routes these to the registered browser. | ||
|
|
||
| For all other URL schemes (e.g. file://) and for named browsers, | ||
| /usr/bin/open -b <bundle-id> is used so that the URL is always passed | ||
| to a browser application rather than dispatched by the OS file handler. | ||
| This prevents file injection attacks where a file:// URL pointing to an | ||
| executable bundle could otherwise be launched by the OS. | ||
|
|
||
| Named browsers with known bundle IDs use -b; unknown names fall back | ||
| to -a. | ||
| """ | ||
|
|
||
| _BUNDLE_IDS = { | ||
| 'google chrome': 'com.google.Chrome', | ||
| 'firefox': 'org.mozilla.firefox', | ||
| 'safari': 'com.apple.Safari', | ||
| 'chromium': 'org.chromium.Chromium', | ||
| 'opera': 'com.operasoftware.Opera', | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have Opera installed on macOS, and confirm this is the correct bundle ID in the plist, but whilst this works: import webbrowser
web = webbrowser.get("firefox")
web.open("https://www.python.org/")This doesn't: import webbrowser
web = webbrowser.get("opera")
web.open("https://www.python.org/")Traceback (most recent call last):
File "/Users/hugo/github/python/cpython/main/1.py", line 2, in <module>
web = webbrowser.get("opera")
File "/Users/hugo/github/python/cpython/main/Lib/webbrowser.py", line 68, in get
raise Error("could not locate runnable browser")
webbrowser.Error: could not locate runnable browserDo these new ones need registering too, or removing from here?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for catching this. I registered the browsers and fixed the Edge bundle. Tested both Opera and Edge locally. |
||
| 'microsoft edge': 'com.microsoft.edgemac', | ||
| } | ||
|
|
||
| def open(self, url, new=0, autoraise=True): | ||
| sys.audit("webbrowser.open", url) | ||
| self._check_url(url) | ||
| if self.name == 'default': | ||
| proto, sep, _ = url.partition(':') | ||
| if sep and proto.lower() in {'http', 'https'}: | ||
| cmd = ['/usr/bin/open', url] | ||
| else: | ||
| bundle_id = _macos_default_browser_bundle_id() | ||
| if bundle_id: | ||
| cmd = ['/usr/bin/open', '-b', bundle_id, url] | ||
| else: | ||
| cmd = ['/usr/bin/open', url] | ||
| else: | ||
| bundle_id = self._BUNDLE_IDS.get(self.name.lower()) | ||
| if bundle_id: | ||
| cmd = ['/usr/bin/open', '-b', bundle_id, url] | ||
| else: | ||
| cmd = ['/usr/bin/open', '-a', self.name, url] | ||
| proc = subprocess.run(cmd, stderr=subprocess.DEVNULL) | ||
| return proc.returncode == 0 | ||
|
|
||
| class MacOSXOSAScript(BaseBrowser): | ||
| def __init__(self, name='default'): | ||
| import warnings | ||
| warnings.warn( | ||
| "MacOSXOSAScript is deprecated, use MacOS instead.", | ||
| DeprecationWarning, | ||
| stacklevel=2, | ||
| ) | ||
| super().__init__(name) | ||
|
|
||
| def open(self, url, new=0, autoraise=True): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| Add :class:`!MacOSX` to :mod:`webbrowser` for macOS, which opens URLs via | ||
| ``/usr/bin/open`` instead of piping AppleScript to ``osascript``. | ||
| Deprecate :class:`!MacOSXOSAScript` in favour of :class:`!MacOSX`. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| Fix a PATH-injection vulnerability in :mod:`webbrowser` on macOS where | ||
| ``osascript`` was invoked without an absolute path. The new :class:`!MacOSX` | ||
| class uses ``/usr/bin/open`` directly, eliminating the dependency on | ||
| ``osascript`` entirely. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function leaks memory due to not performing objective-c reference count updates.
Also: This introduces a new dependency on an Apple system framework, which in the past has caused problems for folks starting new worker processes using
os.fork(without exec-in a new executable).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed that hardcoding
/usr/bin/osascriptaddresses the PATH injection issue directly. This PR is intended as an incremental improvement on top of that.The core motivation is that
osascriptis increasingly blocked at the EDR/MDM level in enterprise environments as a broad response to ClickFix campaigns and supply chain attacks like the axios incident./usr/bin/openis both incrementally safer as a purpose-built URL dispatch primitive and far less likely to be subject to those same restrictions. Whenosascriptis blocked, users of Python-based tools may see a failure with no obvious connection toosascript, and the path to diagnosing an endpoint security policy conflict is not straightforward for most users.On
_macos_default_browser_bundle_id(): agreed, I'll fix the missing Objective-C reference count cleanup and address theos.fork()concern.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ended up replacing the AppKit/ctypes implementation with a
plistlibread of the LaunchServices preferences file to address the memory and forking risks.