gh-120522: Apply App Store compliance patch during installation

[freakboy3742]

A revised approach to the change made by #120984 (which was reverted) and #121830, incorporating feedback from the review of those patches.

Apple’s macOS App Store is auto-rejects any app that contains the string itms-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.

Following a 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 of itms-services. This patch is optionally applied on macOS if the configure flag is enabled, and is always 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 the libinstall target. It is applied against the installed library, prior to bytecode compilation.

Any errors returned during this patch are ignored. 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, and patch will return an error. However, in our case, we know (as a result of the validation check during the build) that the patch can be successfully applied against the files that do 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 is always applied on iOS, we'll get verification if the patch becomes incompatible (admittedly, only during buildbot validation, not during standard GitHub CI).

The underlying problem exists 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]

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]

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 patching test/test_urlparse.py - which then fails again because the test folder doesn't exist, so it can't write a .rej file. If the directory did 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.

[freakboy3742]

An additional note for reviewers: It would be possible to enable checking the patch on macOS when not 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 will always be applied on iOS, I opted to omit this option - but I'm happy to revisit that.

[freakboy3742]

!buildbot iOS

[freakboy3742]

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

[freakboy3742]

@Yhg1s Could I ask for a release manager's call on back porting this to 3.12? As noted in the PR description, the problem exists 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.

[ned-deily]

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.

[itamaro]

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

[ned-deily]

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.

[ned-deily]

@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 to configure and the Makefile 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.

[freakboy3742]

@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 to configure and the Makefile 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.

[miss-islington-app]

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

[bedevere-app]

GH-122105 is a backport of this pull request to the 3.13 branch.

[bedevere-bot]

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

Hi! The buildbot s390x Fedora LTO + PGO 3.x has failed when building commit 728432c.

What do you need to do:

1. Don't panic.
2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
3. 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.
4. Check if the failure is related to this commit (728432c) or if it is a false positive.
5. 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", line 995, in _run_repl_globals_test
    self.fail(f"{var}= not found in output")
    ~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: __package__= not found in output


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", line 995, in _run_repl_globals_test
    self.fail(f"{var}= not found in output")
    ~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: __name__= not found in output


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", line 995, in _run_repl_globals_test
    self.fail(f"{var}= not found in output")
    ~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: __file__= not found in output


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", line 995, in _run_repl_globals_test
    self.fail(f"{var}= not found in output")
    ~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: FOO= not found in output

[freakboy3742]

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.