diff --git a/tools/release.py b/tools/release.py index 13c3dcb34155bb12411603a652327fd3e1dc480a..0263d9a9e6f6ea9ca23cac69112e12a977b6b1df 100755 --- a/tools/release.py +++ b/tools/release.py @@ -87,8 +87,17 @@ def clean(dry_run, moment): def run(command, **kwargs): - print('+', ' '.join(map(str, command)), flush=True) - subprocess.run(command, cwd=ROOT, check=True, **kwargs) + displayed = ['xcrun', 'notarytool', '[arguments redacted]'] if command[:2] == ['xcrun', 'notarytool'] else command + print('+', ' '.join(map(str, displayed)), flush=True) + try: + subprocess.run(command, cwd=ROOT, check=True, **kwargs) + except (subprocess.CalledProcessError, subprocess.TimeoutExpired) as error: + if displayed is not command: + error.cmd = displayed + position = 1 if isinstance(error, subprocess.CalledProcessError) else 0 + error.args = tuple(displayed if index == position else value + for index, value in enumerate(error.args)) + raise def history(): @@ -197,6 +206,26 @@ def notary(): return arguments if signs_in else None +def staple(bundle): + for attempt in range(2): + with tempfile.TemporaryFile(mode='w+') as log: + try: + try: + run(['xcrun', 'stapler', 'staple', bundle], stdout=log, + stderr=subprocess.STDOUT, timeout=120) + finally: + log.seek(0) + diagnostic = log.read() + print(diagnostic, end='', flush=True) + except subprocess.CalledProcessError as error: + # EX_NOHOST alone also means the bundle has never been notarized. + if attempt or error.returncode != 68 or not re.search( + r'^error is Error Domain=NSURLErrorDomain Code=-1005(?:\s|$)', diagnostic, re.M): + raise + else: + return + + def build_mac(platform, folder, developer_id, notarize, symbols): """The zipped app, which build_macos.py signs; 10.6's stays unsigned, as it predates Developer ID. Its zipped dSYM goes to `symbols`.""" @@ -216,7 +245,7 @@ def build_mac(platform, folder, developer_id, notarize, symbols): if notarize and platform != 'macos-10.6': zip_bundle(bundle, archive) run(['xcrun', 'notarytool', 'submit', archive, *notarize, '--wait']) - run(['xcrun', 'stapler', 'staple', bundle]) + staple(bundle) archive.unlink() zip_bundle(bundle, archive) return archive diff --git a/tools/test_release.py b/tools/test_release.py index bdf26fd375e3acd5b2e00eb85fab594699698d93..b3f6e5756bb0dd74b778eb2d432d02550a3edfd8 100644 --- a/tools/test_release.py +++ b/tools/test_release.py @@ -7,7 +7,10 @@ import json from pathlib import Path import runpy import stat +import subprocess +import sys import tempfile +import traceback import unittest from unittest.mock import Mock, patch @@ -19,6 +22,130 @@ def utc(text): class ReleaseTest(unittest.TestCase): + def test_notary_arguments_reach_child_without_entering_logs_or_tracebacks(self): + for status in (0, 7): + with self.subTest(status=status), tempfile.TemporaryDirectory() as folder: + record = Path(folder) / 'arguments.json' + secrets = ['PRIVATE-KEY-PATH', 'PRIVATE-KEY-ID', 'PRIVATE-ISSUER'] + command = ['xcrun', 'notarytool', 'submit', Path('archive.zip'), + '--key', secrets[0], '--key-id', secrets[1], '--issuer', secrets[2], '--wait'] + child = ('import json, pathlib, sys; ' + 'pathlib.Path(sys.argv[1]).write_text(json.dumps(sys.argv[3:])); ' + 'print("notary output"); print("notary diagnostic", file=sys.stderr); ' + 'sys.exit(int(sys.argv[2]))') + actual_run = subprocess.run + + def notary(arguments, **kwargs): + return actual_run([sys.executable, '-c', child, str(record), str(status), + *map(str, arguments)], **kwargs) + + printed = io.StringIO() + with tempfile.TemporaryFile(mode='w+') as output, tempfile.TemporaryFile(mode='w+') as errors, \ + patch.object(release['subprocess'], 'run', side_effect=notary), \ + redirect_stdout(printed): + if status: + try: + release['run'](command, stdout=output, stderr=errors) + except subprocess.CalledProcessError as error: + self.assertIs(type(error), subprocess.CalledProcessError) + self.assertEqual(error.returncode, status) + try: + raise RuntimeError('Release failed') from error + except RuntimeError as chained: + trace = ''.join(traceback.format_exception(type(chained), chained, chained.__traceback__)) + trace += repr(chained.__cause__) + repr(error.args) + else: + self.fail('The notary child did not fail') + else: + self.assertIsNone(release['run'](command, stdout=output, stderr=errors)) + trace = '' + output.seek(0) + errors.seek(0) + self.assertEqual(output.read(), 'notary output\n') + self.assertEqual(errors.read(), 'notary diagnostic\n') + self.assertEqual(json.loads(record.read_text()), list(map(str, command))) + self.assertIn('xcrun notarytool', printed.getvalue()) + for secret in secrets: + self.assertNotIn(secret, printed.getvalue() + trace) + + def test_notary_timeout_redacts_arguments_without_changing_failure(self): + command = ['xcrun', 'notarytool', 'submit', '--password', 'PRIVATE-PASSWORD'] + error = subprocess.TimeoutExpired(command, 3, output=b'output', stderr=b'diagnostic') + with patch.object(release['subprocess'], 'run', side_effect=error), redirect_stdout(io.StringIO()): + with self.assertRaises(subprocess.TimeoutExpired) as raised: + release['run'](command, timeout=3) + self.assertIs(raised.exception, error) + self.assertEqual(error.timeout, 3) + self.assertEqual(error.output, b'output') + self.assertEqual(error.stderr, b'diagnostic') + self.assertNotIn('PRIVATE-PASSWORD', repr(error) + repr(error.args) + + ''.join(traceback.format_exception(type(error), error, error.__traceback__))) + + def test_other_command_failure_keeps_its_arguments_and_output(self): + command = ['codesign', '--verify', Path('Snowbound.app')] + error = subprocess.CalledProcessError(2, command, output=b'output', stderr=b'diagnostic') + printed = io.StringIO() + with patch.object(release['subprocess'], 'run', side_effect=error), redirect_stdout(printed): + with self.assertRaises(subprocess.CalledProcessError) as raised: + release['run'](command) + self.assertIs(raised.exception, error) + self.assertIs(error.cmd, command) + self.assertEqual(error.output, b'output') + self.assertEqual(error.stderr, b'diagnostic') + self.assertIn('codesign --verify Snowbound.app', printed.getvalue()) + + def test_staple_retries_only_a_lost_connection_and_propagates_second_failure(self): + network = 'error is Error Domain=NSURLErrorDomain Code=-1005 "The network connection was lost."\n' + for failures in ([(68, network)], [(68, network), (68, network)], + [(68, 'The bundle has not been notarized.\n')], + [(77, network)], [(68, network.replace('-1005', '-10050'))]): + with self.subTest(failures=failures), tempfile.TemporaryDirectory() as folder: + calls = [] + errors = [] + + def failing_tool(command, **kwargs): + calls.append((command, kwargs)) + if len(calls) <= len(failures): + status, diagnostic = failures[len(calls) - 1] + kwargs['stdout'].write(diagnostic) + error = subprocess.CalledProcessError(status, command) + errors.append(error) + raise error + kwargs['stdout'].write('The staple and validate action worked!\n') + + printed = io.StringIO() + with patch.dict(release['staple'].__globals__, {'run': failing_tool}), redirect_stdout(printed): + if len(failures) == 1 and failures[0] == (68, network): + self.assertIsNone(release['staple'](Path(folder) / 'Snowbound.app')) + else: + with self.assertRaises(subprocess.CalledProcessError) as raised: + release['staple'](Path(folder) / 'Snowbound.app') + self.assertIs(raised.exception, errors[-1]) + expected = 2 if failures[0] == (68, network) else 1 + self.assertEqual(len(calls), expected) + self.assertEqual(printed.getvalue(), ''.join(text for _, text in failures) + + ('The staple and validate action worked!\n' if len(errors) < len(calls) else '')) + for command, kwargs in calls: + self.assertEqual(command, ['xcrun', 'stapler', 'staple', Path(folder) / 'Snowbound.app']) + self.assertEqual(kwargs['timeout'], 120) + self.assertEqual(kwargs['stderr'], subprocess.STDOUT) + + def test_staple_timeout_preserves_diagnostic_without_retrying(self): + command = ['xcrun', 'stapler', 'staple', Path('Snowbound.app')] + error = subprocess.TimeoutExpired(command, 120) + + def timeout(_, **kwargs): + kwargs['stdout'].write('Incomplete ticket download\n') + raise error + + printed = io.StringIO() + with patch.dict(release['staple'].__globals__, {'run': Mock(side_effect=timeout)}), redirect_stdout(printed): + with self.assertRaises(subprocess.TimeoutExpired) as raised: + release['staple'](command[-1]) + release['staple'].__globals__['run'].assert_called_once() + self.assertIs(raised.exception, error) + self.assertEqual(printed.getvalue(), 'Incomplete ticket download\n') + def test_download_bytes_survive_rejected_permissions(self): with tempfile.TemporaryDirectory() as folder: source, destination = Path(folder) / 'source', Path(folder) / 'download'