authorclover caruso <git@paperclover.net> 2026-10-12 14:39:31-07:00
committerclover caruso <git@paperclover.net> 2026-10-12 15:17:43-07:00
log81ed5f25b504b1dcedc44acbe3137cb4d70d34de
treef8fe8e8cbf521cdd8e184527954e1ef76e237e0a
parent76298065bb1d6758c3bce206ebf9a981c49759ed
signature Signed by SSH key SHA256:52mNGHRsVFBDED9IAX5pe+LRWUefqTbxEReunq21QvU (~clover)

fix(release): redact notary credentials and retry a lost staple connection

Keep notary arguments out of command logs and subprocess exception strings, repr and chains. Retry stapling once only for the observed EX_NOHOST plus NSURLErrorNetworkConnectionLost diagnostic, with a 120-second bound per attempt. fixes #142 Assisted-by: gpt-6.1-sol

2 files changed, 159 insertions(+), 3 deletions(-)

tools/release.py+32-3
...@@ -87,8 +87,17 @@ def clean(dry_run, moment):...@@ -87,8 +87,17 @@ def clean(dry_run, moment):
8787
8888
89def run(command, **kwargs):89def run(command, **kwargs):
90 print('+', ' '.join(map(str, command)), flush=True)90 displayed = ['xcrun', 'notarytool', '[arguments redacted]'] if command[:2] == ['xcrun', 'notarytool'] else command
91 subprocess.run(command, cwd=ROOT, check=True, **kwargs)91 print('+', ' '.join(map(str, displayed)), flush=True)
92 try:
93 subprocess.run(command, cwd=ROOT, check=True, **kwargs)
94 except (subprocess.CalledProcessError, subprocess.TimeoutExpired) as error:
95 if displayed is not command:
96 error.cmd = displayed
97 position = 1 if isinstance(error, subprocess.CalledProcessError) else 0
98 error.args = tuple(displayed if index == position else value
99 for index, value in enumerate(error.args))
100 raise
92101
93102
94def history():103def history():
...@@ -197,6 +206,26 @@ def notary():...@@ -197,6 +206,26 @@ def notary():
197 return arguments if signs_in else None206 return arguments if signs_in else None
198207
199208
209def staple(bundle):
210 for attempt in range(2):
211 with tempfile.TemporaryFile(mode='w+') as log:
212 try:
213 try:
214 run(['xcrun', 'stapler', 'staple', bundle], stdout=log,
215 stderr=subprocess.STDOUT, timeout=120)
216 finally:
217 log.seek(0)
218 diagnostic = log.read()
219 print(diagnostic, end='', flush=True)
220 except subprocess.CalledProcessError as error:
221 # EX_NOHOST alone also means the bundle has never been notarized.
222 if attempt or error.returncode != 68 or not re.search(
223 r'^error is Error Domain=NSURLErrorDomain Code=-1005(?:\s|$)', diagnostic, re.M):
224 raise
225 else:
226 return
227
228
200def build_mac(platform, folder, developer_id, notarize, symbols):229def build_mac(platform, folder, developer_id, notarize, symbols):
201 """The zipped app, which build_macos.py signs; 10.6's stays unsigned, as it predates Developer ID.230 """The zipped app, which build_macos.py signs; 10.6's stays unsigned, as it predates Developer ID.
202 Its zipped dSYM goes to `symbols`."""231 Its zipped dSYM goes to `symbols`."""
...@@ -216,7 +245,7 @@ def build_mac(platform, folder, developer_id, notarize, symbols):...@@ -216,7 +245,7 @@ def build_mac(platform, folder, developer_id, notarize, symbols):
216 if notarize and platform != 'macos-10.6':245 if notarize and platform != 'macos-10.6':
217 zip_bundle(bundle, archive)246 zip_bundle(bundle, archive)
218 run(['xcrun', 'notarytool', 'submit', archive, *notarize, '--wait'])247 run(['xcrun', 'notarytool', 'submit', archive, *notarize, '--wait'])
219 run(['xcrun', 'stapler', 'staple', bundle])248 staple(bundle)
220 archive.unlink()249 archive.unlink()
221 zip_bundle(bundle, archive)250 zip_bundle(bundle, archive)
222 return archive251 return archive
tools/test_release.py+127
...@@ -7,7 +7,10 @@ import json...@@ -7,7 +7,10 @@ import json
7from pathlib import Path7from pathlib import Path
8import runpy8import runpy
9import stat9import stat
10import subprocess
11import sys
10import tempfile12import tempfile
13import traceback
11import unittest14import unittest
12from unittest.mock import Mock, patch15from unittest.mock import Mock, patch
1316
...@@ -19,6 +22,130 @@ def utc(text):...@@ -19,6 +22,130 @@ def utc(text):
1922
2023
21class ReleaseTest(unittest.TestCase):24class ReleaseTest(unittest.TestCase):
25 def test_notary_arguments_reach_child_without_entering_logs_or_tracebacks(self):
26 for status in (0, 7):
27 with self.subTest(status=status), tempfile.TemporaryDirectory() as folder:
28 record = Path(folder) / 'arguments.json'
29 secrets = ['PRIVATE-KEY-PATH', 'PRIVATE-KEY-ID', 'PRIVATE-ISSUER']
30 command = ['xcrun', 'notarytool', 'submit', Path('archive.zip'),
31 '--key', secrets[0], '--key-id', secrets[1], '--issuer', secrets[2], '--wait']
32 child = ('import json, pathlib, sys; '
33 'pathlib.Path(sys.argv[1]).write_text(json.dumps(sys.argv[3:])); '
34 'print("notary output"); print("notary diagnostic", file=sys.stderr); '
35 'sys.exit(int(sys.argv[2]))')
36 actual_run = subprocess.run
37
38 def notary(arguments, **kwargs):
39 return actual_run([sys.executable, '-c', child, str(record), str(status),
40 *map(str, arguments)], **kwargs)
41
42 printed = io.StringIO()
43 with tempfile.TemporaryFile(mode='w+') as output, tempfile.TemporaryFile(mode='w+') as errors, \
44 patch.object(release['subprocess'], 'run', side_effect=notary), \
45 redirect_stdout(printed):
46 if status:
47 try:
48 release['run'](command, stdout=output, stderr=errors)
49 except subprocess.CalledProcessError as error:
50 self.assertIs(type(error), subprocess.CalledProcessError)
51 self.assertEqual(error.returncode, status)
52 try:
53 raise RuntimeError('Release failed') from error
54 except RuntimeError as chained:
55 trace = ''.join(traceback.format_exception(type(chained), chained, chained.__traceback__))
56 trace += repr(chained.__cause__) + repr(error.args)
57 else:
58 self.fail('The notary child did not fail')
59 else:
60 self.assertIsNone(release['run'](command, stdout=output, stderr=errors))
61 trace = ''
62 output.seek(0)
63 errors.seek(0)
64 self.assertEqual(output.read(), 'notary output\n')
65 self.assertEqual(errors.read(), 'notary diagnostic\n')
66 self.assertEqual(json.loads(record.read_text()), list(map(str, command)))
67 self.assertIn('xcrun notarytool', printed.getvalue())
68 for secret in secrets:
69 self.assertNotIn(secret, printed.getvalue() + trace)
70
71 def test_notary_timeout_redacts_arguments_without_changing_failure(self):
72 command = ['xcrun', 'notarytool', 'submit', '--password', 'PRIVATE-PASSWORD']
73 error = subprocess.TimeoutExpired(command, 3, output=b'output', stderr=b'diagnostic')
74 with patch.object(release['subprocess'], 'run', side_effect=error), redirect_stdout(io.StringIO()):
75 with self.assertRaises(subprocess.TimeoutExpired) as raised:
76 release['run'](command, timeout=3)
77 self.assertIs(raised.exception, error)
78 self.assertEqual(error.timeout, 3)
79 self.assertEqual(error.output, b'output')
80 self.assertEqual(error.stderr, b'diagnostic')
81 self.assertNotIn('PRIVATE-PASSWORD', repr(error) + repr(error.args) +
82 ''.join(traceback.format_exception(type(error), error, error.__traceback__)))
83
84 def test_other_command_failure_keeps_its_arguments_and_output(self):
85 command = ['codesign', '--verify', Path('Snowbound.app')]
86 error = subprocess.CalledProcessError(2, command, output=b'output', stderr=b'diagnostic')
87 printed = io.StringIO()
88 with patch.object(release['subprocess'], 'run', side_effect=error), redirect_stdout(printed):
89 with self.assertRaises(subprocess.CalledProcessError) as raised:
90 release['run'](command)
91 self.assertIs(raised.exception, error)
92 self.assertIs(error.cmd, command)
93 self.assertEqual(error.output, b'output')
94 self.assertEqual(error.stderr, b'diagnostic')
95 self.assertIn('codesign --verify Snowbound.app', printed.getvalue())
96
97 def test_staple_retries_only_a_lost_connection_and_propagates_second_failure(self):
98 network = 'error is Error Domain=NSURLErrorDomain Code=-1005 "The network connection was lost."\n'
99 for failures in ([(68, network)], [(68, network), (68, network)],
100 [(68, 'The bundle has not been notarized.\n')],
101 [(77, network)], [(68, network.replace('-1005', '-10050'))]):
102 with self.subTest(failures=failures), tempfile.TemporaryDirectory() as folder:
103 calls = []
104 errors = []
105
106 def failing_tool(command, **kwargs):
107 calls.append((command, kwargs))
108 if len(calls) <= len(failures):
109 status, diagnostic = failures[len(calls) - 1]
110 kwargs['stdout'].write(diagnostic)
111 error = subprocess.CalledProcessError(status, command)
112 errors.append(error)
113 raise error
114 kwargs['stdout'].write('The staple and validate action worked!\n')
115
116 printed = io.StringIO()
117 with patch.dict(release['staple'].__globals__, {'run': failing_tool}), redirect_stdout(printed):
118 if len(failures) == 1 and failures[0] == (68, network):
119 self.assertIsNone(release['staple'](Path(folder) / 'Snowbound.app'))
120 else:
121 with self.assertRaises(subprocess.CalledProcessError) as raised:
122 release['staple'](Path(folder) / 'Snowbound.app')
123 self.assertIs(raised.exception, errors[-1])
124 expected = 2 if failures[0] == (68, network) else 1
125 self.assertEqual(len(calls), expected)
126 self.assertEqual(printed.getvalue(), ''.join(text for _, text in failures) +
127 ('The staple and validate action worked!\n' if len(errors) < len(calls) else ''))
128 for command, kwargs in calls:
129 self.assertEqual(command, ['xcrun', 'stapler', 'staple', Path(folder) / 'Snowbound.app'])
130 self.assertEqual(kwargs['timeout'], 120)
131 self.assertEqual(kwargs['stderr'], subprocess.STDOUT)
132
133 def test_staple_timeout_preserves_diagnostic_without_retrying(self):
134 command = ['xcrun', 'stapler', 'staple', Path('Snowbound.app')]
135 error = subprocess.TimeoutExpired(command, 120)
136
137 def timeout(_, **kwargs):
138 kwargs['stdout'].write('Incomplete ticket download\n')
139 raise error
140
141 printed = io.StringIO()
142 with patch.dict(release['staple'].__globals__, {'run': Mock(side_effect=timeout)}), redirect_stdout(printed):
143 with self.assertRaises(subprocess.TimeoutExpired) as raised:
144 release['staple'](command[-1])
145 release['staple'].__globals__['run'].assert_called_once()
146 self.assertIs(raised.exception, error)
147 self.assertEqual(printed.getvalue(), 'Incomplete ticket download\n')
148
22 def test_download_bytes_survive_rejected_permissions(self):149 def test_download_bytes_survive_rejected_permissions(self):
23 with tempfile.TemporaryDirectory() as folder:150 with tempfile.TemporaryDirectory() as folder:
24 source, destination = Path(folder) / 'source', Path(folder) / 'download'151 source, destination = Path(folder) / 'source', Path(folder) / 'download'