gh-100107: Make Py launcher ignore app aliases that launch Microsoft Store #114358

flagrama · 2024-01-20T21:42:44Z

Currently when you use the #!/usr/bin/env python3 shebang on Windows 10 or Windows 11 the Py launcher will search the PATH environment variable and find the default NTFS reparse points called App Execution Aliases Microsoft added to the OSes. These are effectively stub exes that, when Python is not installed from the Microsoft Store, will launch the Microsoft Store to install Python, or if launched with arguments will report an error.

This makes Windows 10/11 the only OS where the standard shebang does not work. It complicated packaging python scripts for ordinary users because they have an extra step, either disabling the app execution aliases, installing from the Microsoft Store, or launching the script manually. Windows ends up being the only OS that doesn't "just work" and changing the shebang to something else will break or degrade the experience in other OSes.

This PR solves this issue by checking if the exe found in the PATH is a reparse point. If it is, it will read the file and identify the alias that will launch the store and will drop out of the shebang handling as if nothing was found in the PATH. If it is an alias that does not launch the store, or an ordinary file it will just continue and use the found file as there shouldn't be any issues.

In my testing this now makes opening a script with the standard #!/usr/bin/env python3 shebang work without additional work by the user.

Note that I am not a Windows user or a Windows developer so it's possible that I've missed some workflow and don't know if it works, or my code may be inefficient. I've only really tested the workflow of the normal users that use a Python application I help develop and know that I have solved the issue there.

Issue: Script containing a "shebang" would not start under Windows #100107

ghost · 2024-01-20T21:42:46Z

All commit authors signed the Contributor License Agreement.

bedevere-app · 2024-01-20T21:42:50Z

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

zooba

It's got all the pieces we need, though it can be simplified quite a bit. Would also be ideal to move it to its own function, so that someone reading this function can see from the call what it's doing but they don't have to actually read the details unless they want to.

zooba · 2024-01-22T20:17:52Z

PC/launcher2.c

+        winerror(0, L"Failed to find %s on PATH\n", filename);
+        return RC_BAD_VIRTUAL_PATH;
+    }
+


Should be able to FindClose here and not worry about cleaning it up later.

zooba · 2024-01-22T20:18:32Z

PC/launcher2.c

@@ -826,6 +841,72 @@ searchPath(SearchInfo *search, const wchar_t *shebang, int shebangLength)
        return RC_BAD_VIRTUAL_PATH;
    }

+    // Make sure we didn't find a reparse point that will open the Microsoft Store


Let's start by refactoring all this out into another function ensure_no_redirector_stub that either returns 0 (okay) or the RC constant that should be returned from here.

zooba · 2024-01-22T20:19:00Z

PC/launcher2.c

+    }
+
+    if (findData.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT &&
+        findData.dwReserved0 & IO_REPARSE_TAG_APPEXECLINK) {


After refactoring, use this check to exit early from the new function. That will keep the overall indent level lower for the rest of it.

zooba · 2024-01-22T20:25:58Z

PC/launcher2.c

+            if (GetLastError() == ERROR_FILE_NOT_FOUND) {
+                debug(L"# %s on disappeared\n", buffer);
+                // The file found on PATH disappeared. Alias probably disabled by user while trying to run, let normal handling take over
+                RC_NO_SHEBANG;
+            }


I don't think this error adds anything - we could lose this race at any time, so probably best to just return RC_NO_SHEBANG for them all and it can fail if/when we try to launch it.

zooba · 2024-01-22T20:26:24Z

PC/launcher2.c

+            if (GetLastError() == ERROR_FILE_NOT_FOUND) {
+                debug(L"# %s on disappeared\n", buffer);
+                // The file found on PATH disappeared. Alias probably disabled by user while trying to run, let normal handling take over
+                RC_NO_SHEBANG;
+            }


Ditto - no benefit to calling this one out separately. Normal handling for all errors should be okay

zooba · 2024-01-22T20:28:08Z

PC/launcher2.c

+
+        appExecLinkFilePtr = (AppExecLinkFile*)&appExecLinkBuf;
+
+        wchar_t redirectorPackageId[] = L"Microsoft.DesktopAppInstaller";


Suggested change

wchar_t redirectorPackageId[] = L"Microsoft.DesktopAppInstaller";

const wchar_t *redirectorPackageId = L"Microsoft.DesktopAppInstaller_8wekyb3d8bbwe";

zooba · 2024-01-22T20:32:22Z

PC/launcher2.c

+        const size_t bufSize = sizeof(AppExecLinkFile);
+        wchar_t appExecLinkBuf[sizeof(AppExecLinkFile)];


These aren't necessary - just allocate the struct:

AppExecLinkFile appExecLink;

And then use sizeof(appExecLink) anywhere we need it, which ought to only be the DeviceIOControl call.

zooba · 2024-01-22T20:33:33Z

PC/launcher2.c

+        wchar_t partialId[30];
+        wcsncpy_s(partialId, redirectorStrLen + 1, appExecLinkFilePtr->stringList, redirectorStrLen);
+
+        if (0 == wcsncmp(partialId, &redirectorPackageId, redirectorStrLen)) {


A simple wcscmp ought to be fine here if you use the full package name. As far as I'm aware, the publisher ID has never changed and never will (it's always some kind of hash of "Microsoft").

bedevere-app · 2024-01-22T20:35:06Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

zooba · 2024-01-22T20:36:19Z

Adding a NEWS entry would be great, too. Just something like The ``py.exe`` launcher will no longer attempt to run the Microsoft Store redirector when launching a script containing a ``/usr/bin/env`` shebang.

move new code to own function simplify error handling simplify struct handling simplify string comparison

bedevere-app · 2024-01-22T23:57:50Z

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

…flagrama/cpython into py-launcher-shebang-appexeclink

flagrama · 2024-01-23T00:55:09Z

I have made the requested changes; please review again.

Everything looks better to me except the actual call to the function and checking it's return value.

Also, is there any chance of this getting backported into the next release of older supported versions?

PC/launcher2.c

zooba · 2024-01-23T17:00:35Z

One leaked handle and we're good. And yeah, we can backport this to earlier versions, though it's worth noting that once you've installed 3.12 you'll need a later 3.12 to upgrade the installer - it won't downgrade to a 3.11 version, even if it's "newer".

Co-authored-by: Steve Dower <steve.dower@microsoft.com>

miss-islington-app · 2024-01-25T00:23:32Z

Thanks @flagrama for the PR, and @zooba for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖

…Microsoft Store (pythonGH-114358) (cherry picked from commit d5c21c1) Co-authored-by: Vincent Cunningham <flagrama@users.noreply.github.com>

zooba · 2024-01-25T00:23:44Z

Thanks! And congratulations on your first PR!

bedevere-app · 2024-01-25T00:23:44Z

GH-114541 is a backport of this pull request to the 3.12 branch.

bedevere-app · 2024-01-25T00:23:49Z

GH-114542 is a backport of this pull request to the 3.11 branch.

…oft Store (GH-114358) (cherry picked from commit d5c21c1) Co-authored-by: Vincent Cunningham <flagrama@users.noreply.github.com>

…Microsoft Store (pythonGH-114358)

flagrama added 2 commits January 17, 2024 18:48

pylauncher: Ignore shebang if APPEXECLINK found in PATH

028712f

pylauncher: Only ignore alias that opens app installer

5fa3ca9

flagrama requested a review from a team as a code owner January 20, 2024 21:42

bedevere-app bot added the awaiting review label Jan 20, 2024

bedevere-app bot mentioned this pull request Jan 20, 2024

Script containing a "shebang" would not start under Windows #100107

Closed

pylauncher: add error handling

8f652d1

zooba requested changes Jan 22, 2024

View reviewed changes

bedevere-app bot added awaiting changes and removed awaiting review labels Jan 22, 2024

pylauncher: Address code review

adca3a9

move new code to own function simplify error handling simplify struct handling simplify string comparison

blurb-it bot and others added 5 commits January 23, 2024 00:05

📜🤖 Added by blurb_it.

9783c95

pylauncher: Move struct allocation closer to usage

1cf0235

Merge branch 'main' into py-launcher-shebang-appexeclink

ac0c60d

pylauncher: adjust newlines

ba7fe3e

Merge branch 'py-launcher-shebang-appexeclink' of https://github.com/…

eeecc2e

…flagrama/cpython into py-launcher-shebang-appexeclink

zooba reviewed Jan 23, 2024

View reviewed changes

PC/launcher2.c Show resolved Hide resolved

zooba approved these changes Jan 23, 2024

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting changes labels Jan 23, 2024

zooba added needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Jan 23, 2024

pylauncher: cleanup leaked handle

6c819f0

Co-authored-by: Steve Dower <steve.dower@microsoft.com>

zooba merged commit d5c21c1 into python:main Jan 25, 2024

bedevere-app bot removed the awaiting merge label Jan 25, 2024

bedevere-app bot removed the needs backport to 3.12 only security fixes label Jan 25, 2024

bedevere-app bot removed the needs backport to 3.11 only security fixes label Jan 25, 2024

zooba pushed a commit that referenced this pull request Jan 25, 2024

gh-100107: Make py.exe launcher ignore app aliases that launch Micros…

279bbd2

…oft Store (GH-114358) (cherry picked from commit d5c21c1) Co-authored-by: Vincent Cunningham <flagrama@users.noreply.github.com>

zooba pushed a commit that referenced this pull request Jan 25, 2024

gh-100107: Make py.exe launcher ignore app aliases that launch Micros…

ac5ed8a

…oft Store (GH-114358) (cherry picked from commit d5c21c1) Co-authored-by: Vincent Cunningham <flagrama@users.noreply.github.com>

aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024

pythongh-100107: Make py.exe launcher ignore app aliases that launch …

95c2046

…Microsoft Store (pythonGH-114358)

Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024

pythongh-100107: Make py.exe launcher ignore app aliases that launch …

936d860

…Microsoft Store (pythonGH-114358)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-100107: Make Py launcher ignore app aliases that launch Microsoft Store #114358

gh-100107: Make Py launcher ignore app aliases that launch Microsoft Store #114358

flagrama commented Jan 20, 2024 •

edited by bedevere-app bot

Loading

ghost commented Jan 20, 2024 •

edited by ghost

Loading

bedevere-app bot commented Jan 20, 2024

zooba left a comment

zooba Jan 22, 2024

zooba Jan 22, 2024

zooba Jan 22, 2024

zooba Jan 22, 2024

zooba Jan 22, 2024

zooba Jan 22, 2024

zooba Jan 22, 2024

zooba Jan 22, 2024

bedevere-app bot commented Jan 22, 2024

zooba commented Jan 22, 2024

bedevere-app bot commented Jan 22, 2024

flagrama commented Jan 23, 2024

zooba commented Jan 23, 2024

miss-islington-app bot commented Jan 25, 2024

zooba commented Jan 25, 2024

bedevere-app bot commented Jan 25, 2024

bedevere-app bot commented Jan 25, 2024


		appExecLinkFilePtr = (AppExecLinkFile*)&appExecLinkBuf;

		wchar_t redirectorPackageId[] = L"Microsoft.DesktopAppInstaller";

	wchar_t redirectorPackageId[] = L"Microsoft.DesktopAppInstaller";
	const wchar_t *redirectorPackageId = L"Microsoft.DesktopAppInstaller_8wekyb3d8bbwe";

		const size_t bufSize = sizeof(AppExecLinkFile);
		wchar_t appExecLinkBuf[sizeof(AppExecLinkFile)];

gh-100107: Make Py launcher ignore app aliases that launch Microsoft Store #114358

gh-100107: Make Py launcher ignore app aliases that launch Microsoft Store #114358

Conversation

flagrama commented Jan 20, 2024 • edited by bedevere-app bot Loading

ghost commented Jan 20, 2024 • edited by ghost Loading

bedevere-app bot commented Jan 20, 2024

zooba left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bedevere-app bot commented Jan 22, 2024

zooba commented Jan 22, 2024

bedevere-app bot commented Jan 22, 2024

flagrama commented Jan 23, 2024

zooba commented Jan 23, 2024

miss-islington-app bot commented Jan 25, 2024

zooba commented Jan 25, 2024

bedevere-app bot commented Jan 25, 2024

bedevere-app bot commented Jan 25, 2024

flagrama commented Jan 20, 2024 •

edited by bedevere-app bot

Loading

ghost commented Jan 20, 2024 •

edited by ghost

Loading