Apple’s macOS App Store is auto-rejects any app that contains the stringitms-services. This is the custom URL prefix used for requesting an app installation from the iTunes App Store; however, sandboxed apps are prohibited from using these URLs. Apple’s automagical review processes are catching on the code in urllib’s parser’s handling of these URLs - even if the app in question never uses an itms-services:// URL. It’s present in the standard library; therefore the app is rejected.

Followinga discussion on discuss.python.org, this PR adds a --with-app-store-compliance option to configure that will patch out any code that is known to be an issue with app store compliance.

There is currently a single patch, in the Mac resources directory, patching the known occurrences ofitms-services. This patch is optionally applied on macOS if the configure flag is enabled, and isalways applied on iOS, because all apps will need to pass App Store compliance.

The option allows for a custom patch file to be provided (in case App Store rules change after support for a Python release has ceased). This also allows a platform other than iOS or macOS to apply a "compliance" patch by manually supplying one; although there's no known use case for this at present.

If the use of the patch is configured, a dry-run of the patching is performed early in the build process, against the original sources. This provides early feedback if the patch needs to be updated; if the patch cannot be applied, the build will fail.

The patch is actually applied during thelibinstall target. It is applied against the installed library, prior to bytecode compilation.

Any errors returned during this patch areignored. This is required because the installed library might be missing files that are included in the patch. If--disable-test-modules is configured, any missing files will generate rejected chunks, andpatch will return an error. However, in our case, we know (as a result of the validation check during the build) that the patchcan be successfully applied against the files thatdo exist, so the only failures must be due to missing files. Any rejected chunks are written to a.rej file in the build directory.

As the patch isalways applied on iOS, we'll get verification if the patch becomes incompatible (admittedly, only during buildbot validation, not during standard GitHub CI).

The underlying problemexists in 3.12, but fixing it requires adding a new build option, which arguably falls outside the "security fixes only" status of 3.12.

Fixes#120522.

Issue:Python 3.12 change results in Apple App Store rejection #120522

📚 Documentation preview 📚:https://cpython-previews--121947.org.readthedocs.build/

freakboy3742 added3 commits

July 18, 2024 07:50

Add documentation about app store compliance patching.

a476e1f

Add blurb.

748e29d

Perform app store compliance patching before compiling installed libr…

4bff322

…ary.

freakboy3742 added OS-mac 3.12

only security fixes

3.13

bugs and security fixes

OS-ios 3.14

bugs and security fixes

needs backport to 3.13bugs and security fixes labels

Jul 18, 2024

freakboy3742 requested review froma team,corona10 anderlend-aasland ascode owners

July 18, 2024 04:18

bedevere-appbot mentioned this pull request

Jul 18, 2024

Python 3.12 change results in Apple App Store rejection#120522

Closed

bedevere-appbot added the awaiting core review label

Jul 18, 2024

freakboy3742 commented

Jul 18, 2024

View reviewed changes

Makefile.pre.in

		.PHONY: check-app-store-compliance
		check-app-store-compliance:
		@if [ "$(APP_STORE_COMPLIANCE_PATCH)" != "" ]; then \
		patch --dry-run --quiet --force --strip 1 --directory "$(abs_srcdir)" --input "$(abs_srcdir)/$(APP_STORE_COMPLIANCE_PATCH)"; \

Copy link

ContributorAuthor

freakboy3742Jul 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Some explanation of the choices here:

--dry-run means no changes are applied
--quiet means no "patching xyz.py" messages are output
--force means no questions are asked of the user, and the patch is assumed to be forward.

freakboy3742 commented

Jul 18, 2024

View reviewed changes

Makefile.pre.in

		@ # due to files that are missing because of --disable-test-modules etc.
		@if [ "$(APP_STORE_COMPLIANCE_PATCH)" != "" ]; then \
		echo "Applying app store compliance patch"; \
		patch --force --reject-file "$(abs_builddir)/app-store-compliance.rej" --strip 2 --directory "$(DESTDIR)$(LIBDEST)" --input "$(abs_srcdir)/$(APP_STORE_COMPLIANCE_PATCH)" \|\| true ; \

Copy link

ContributorAuthor

freakboy3742Jul 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Some explanation of the choices here:

--force means no questions are asked of the user, and the patch is assumed to be forward.
--reject-file provides a file where rejected chunks will be output.

The--reject-file option is required because installing when--disable-test-modules is enabled results in a failure patchingtest/test_urlparse.py - which then fails again because thetest folder doesn't exist, so it can't write a.rej file. If the directorydid exist, there would be a.rej file in the distributed artefact.

.orig files shouldn't be generated because we know the patch applies cleanly as a result of pre-validation.

Copy link

ContributorAuthor

freakboy3742 commentedJul 18, 2024

An additional note for reviewers: It would be possible to enable checking the patch on macOS whennot installing; but it would require adding another configuration target to differentiate between "apply", and "check but don't apply" cases. Given the additional complexity, and the fact that the patch willalways be applied on iOS, I opted to omit this option - but I'm happy to revisit that.

freakboy3742 mentioned this pull request

Jul 18, 2024

gh-120522: Apply App Store compliance patch to installed products, not source#121830

Closed

freakboy3742 requested a review fromned-deily

July 18, 2024 04:29

Copy link

ContributorAuthor

freakboy3742 commentedJul 18, 2024

!buildbot iOS

Copy link

ContributorAuthor

freakboy3742 commentedJul 18, 2024

The iOS buildbot will fail because of#121832, but the host install stage will show evidence that the patch has been applied.

Copy link

ContributorAuthor

freakboy3742 commentedJul 18, 2024

@Yhg1s Could I ask for a release manager's call on back porting this to 3.12? As noted in the PR description, the problemexists in 3.12, but fixing it requires introducing a change to the build process, and it's not really a "security" issue by any conventional measure.

Copy link

Member

ned-deily commentedJul 18, 2024

Thanks for pursuing this,@freakboy3742! I will get to reviewing it ASAP but it likely won't be in time for 3.13.0b4 tomorrow.

Copy link

Contributor

itamaro commentedJul 18, 2024

3.12 didn't even celebrate its first birthday yet! afaik it's still well within the "security AND bugfixes" realm.

ned-deily approved these changes

Jul 19, 2024

View reviewed changes

Copy link

Member

ned-deily left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Excellent! This looks really good and this time I wasn't able to think of any other strange configure / make options to try! Thanks for taking so much time and care to try to get this as good as possible. Given the complexity of our Unix builds, it wouldn't be surprising if someone does find another edge case but I think we've done more than our due diligence here.

bedevere-appbot added awaiting merge and removed awaiting core review labels

Jul 19, 2024

Copy link

Member

ned-deily commentedJul 19, 2024

@Yhg1s Could I ask for a release manager's call on back porting this to 3.12?

FWIW, I am -0 (maybe even -1) for backporting to 3.12. At several releases in at this point, 3.12 is at a stable point in its lifecycle. Making changes like this toconfigure and theMakefile are always a bit risky and, in this case, the affected audience is quite small and has likely already worked around the original App Store review problem with a bespoke patch; it's pretty easy to do and, if so, the additional convenience of an official patch is likely not worth the risk to breaking other uses.

Copy link

ContributorAuthor

freakboy3742 commentedJul 21, 2024

@Yhg1s Could I ask for a release manager's call on back porting this to 3.12?
FWIW, I am -0 (maybe even -1) for backporting to 3.12. At several releases in at this point, 3.12 is at a stable point in its lifecycle. Making changes like this toconfigure and theMakefile are always a bit risky and, in this case, the affected audience is quite small and has likely already worked around the original App Store review problem with a bespoke patch; it's pretty easy to do and, if so, the additional convenience of an official patch is likely not worth the risk to breaking other uses.

That's a solid enough argument for me. I'll backport to 3.13, but leave it at that.

Merge branch 'main' into appstore-patch

a521f2a

freakboy3742 merged commit728432c intopython:main

Jul 21, 2024

bedevere-appbot removed the awaiting merge label

Jul 21, 2024

Copy link

miss-islington-appbot commentedJul 21, 2024

Thanks@freakboy3742 for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

freakboy3742 deleted the appstore-patch branch

July 21, 2024 23:36

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request

Jul 21, 2024

pythongh-120522: Apply App Store compliance patch during installation (…

f6f4e3c

…pythonGH-121947)Adds a --with-app-store-compliance configuration option that patches out code known to be an issue with App Store review processes. This option is applied automatically on iOS, and optionally on macOS.(cherry picked from commit728432c)Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>

Copy link

bedevere-appbot commentedJul 21, 2024

GH-122105 is a backport of this pull request to the3.13 branch.

bedevere-appbot removed the needs backport to 3.13bugs and security fixes label

Jul 21, 2024

freakboy3742 added a commit that referenced this pull request

Jul 22, 2024

[3.13]gh-120522: Apply App Store compliance patch during installation (

587a8f8

GH-121947) (#122105)gh-120522: Apply App Store compliance patch during installation (GH-121947)Adds a --with-app-store-compliance configuration option that patches out code known to be an issue with App Store review processes. This option is applied automatically on iOS, and optionally on macOS.(cherry picked from commit728432c)Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>

Copy link

bedevere-bot commentedJul 22, 2024

⚠️⚠️⚠️ Buildbot failure⚠️⚠️⚠️

Hi! The buildbots390x Fedora LTO + PGO 3.x has failed when building commit728432c.

What do you need to do:

Don't panic.
Checkthe buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/#builders/545/builds/6225) and take a look at the build logs.
Check if the failure is related to this commit (728432c) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#builders/545/builds/6225

Failed tests:

test_pyrepl

Failed subtests:

test_inspect_keeps_globals_from_inspected_module - test.test_pyrepl.test_pyrepl.TestMain.test_inspect_keeps_globals_from_inspected_module

Summary of the results of the build (if available):

Click to see traceback logs

Traceback (most recent call last):  File"/home/dje/cpython-buildarea/3.x.edelsohn-fedora-z.lto-pgo/build/Lib/test/test_pyrepl/test_pyrepl.py", line995, in_run_repl_globals_testself.fail(f"{var}= not found in output")~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^AssertionError:__package__= not found in outputTraceback (most recent call last):  File"/home/dje/cpython-buildarea/3.x.edelsohn-fedora-z.lto-pgo/build/Lib/test/test_pyrepl/test_pyrepl.py", line995, in_run_repl_globals_testself.fail(f"{var}= not found in output")~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^AssertionError:__name__= not found in outputTraceback (most recent call last):  File"/home/dje/cpython-buildarea/3.x.edelsohn-fedora-z.lto-pgo/build/Lib/test/test_pyrepl/test_pyrepl.py", line995, in_run_repl_globals_testself.fail(f"{var}= not found in output")~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^AssertionError:__file__= not found in outputTraceback (most recent call last):  File"/home/dje/cpython-buildarea/3.x.edelsohn-fedora-z.lto-pgo/build/Lib/test/test_pyrepl/test_pyrepl.py", line995, in_run_repl_globals_testself.fail(f"{var}= not found in output")~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^AssertionError:FOO= not found in output

Copy link

ContributorAuthor

freakboy3742 commentedJul 22, 2024

As far as I can make out, the buildbot failure is a false positive. The code that has been merged won't do anything on non-Apple platforms; and test that is failing doesn't intersect with the change that would have been made.