From eebf115836bec458febf4141ae11c33131f21782 Mon Sep 17 00:00:00 2001 From: ZhuchkaTriplesix Date: Sat, 17 Oct 2026 12:54:37 +0300 Subject: [PATCH] fix(updater): a macOS update must carry the Team ID of the installed app, not just an intact seal (#1588) --- .../installers/macos_sparkle_installer.dart | 61 ++++++++++++++++++- .../updater/macos_signer_identity_test.dart | 59 ++++++++++++++++++ 2 files changed, 118 insertions(+), 2 deletions(-) create mode 100644 test/core/updater/macos_signer_identity_test.dart diff --git a/lib/core/updater/installers/macos_sparkle_installer.dart b/lib/core/updater/installers/macos_sparkle_installer.dart index f1be5f34..f731adb1 100644 --- a/lib/core/updater/installers/macos_sparkle_installer.dart +++ b/lib/core/updater/installers/macos_sparkle_installer.dart @@ -12,9 +12,14 @@ import 'update_install_utils.dart'; /// Sparkle integration can wrap this path later; for now we verify `codesign` /// on the downloaded bundle before swapping it in place. class MacosSparkleInstaller { - MacosSparkleInstaller({required this.context}); + MacosSparkleInstaller({ + required this.context, + Future Function(String executable, List arguments)? + runProcess, + }) : _runProcess = runProcess ?? Process.run; final UpdateInstallContext context; + final Future Function(String, List) _runProcess; Future install(File package) async { final lower = package.path.toLowerCase(); @@ -54,6 +59,7 @@ class MacosSparkleInstaller { ); } await _verifyCodesign(found); + await _verifySameSigner(installed: targetApp, update: found.path); newApp = found; } catch (_) { // The app keeps running: do not leave the extracted update behind. @@ -77,8 +83,32 @@ class MacosSparkleInstaller { exit(0); } + /// `codesign --verify` only proves the seal is intact: any validly signed + /// bundle, even ad-hoc, passed and replaced the app. The update must also + /// carry the Team ID of the installed app (#1588). + Future _verifySameSigner({ + required String installed, + required String update, + }) async { + final problem = signerMismatchProblem( + installedTeam: await _teamId(installed), + updateTeam: await _teamId(update), + ); + if (problem != null) throw AppUpdaterException(problem); + } + + /// The bundle's Team ID, or null when it has none (ad-hoc or unsigned: + /// `codesign -dv` fails or prints `TeamIdentifier=not set`). + Future _teamId(String bundle) async { + final result = + await _runProcess('codesign', ['-dv', '--verbose=4', bundle]); + if (result.exitCode != 0) return null; + // codesign writes the details to stderr. + return parseCodesignTeamId('${result.stderr}\n${result.stdout}'); + } + Future _verifyCodesign(Directory appBundle) async { - final result = await Process.run( + final result = await _runProcess( 'codesign', ['--verify', '--deep', '--strict', appBundle.path], ); @@ -92,3 +122,30 @@ class MacosSparkleInstaller { } } } + +/// The `TeamIdentifier=` of `codesign -dv` output, or null for none / `not set`. +String? parseCodesignTeamId(String codesignOutput) { + final match = RegExp(r'^TeamIdentifier=(.*)$', multiLine: true) + .firstMatch(codesignOutput); + final id = match?.group(1)?.trim(); + if (id == null || id.isEmpty || id == 'not set') return null; + return id; +} + +/// Why an update signed by [updateTeam] may not replace an app signed by +/// [installedTeam], or null when it may. +/// +/// An installed app with no Team ID (the unsigned or ad-hoc build) has +/// nothing to compare with: there the SHA-256 from the release's +/// `SHA256SUMS.txt`, checked before this, is the check. +String? signerMismatchProblem({ + required String? installedTeam, + required String? updateTeam, +}) { + if (installedTeam == null || updateTeam == installedTeam) return null; + return updateTeam == null + ? 'The downloaded update is not signed by the developer of the installed ' + 'app (Team ID $installedTeam). Installation aborted.' + : 'The downloaded update is signed by a different developer (Team ID ' + '$updateTeam, expected $installedTeam). Installation aborted.'; +} diff --git a/test/core/updater/macos_signer_identity_test.dart b/test/core/updater/macos_signer_identity_test.dart new file mode 100644 index 00000000..ddb8b278 --- /dev/null +++ b/test/core/updater/macos_signer_identity_test.dart @@ -0,0 +1,59 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:querya_desktop/core/updater/installers/macos_sparkle_installer.dart'; + +void main() { + group('parseCodesignTeamId (#1588)', () { + test('reads the Team ID of a Developer ID signature', () { + const out = 'Executable=/Applications/Querya.app/Contents/MacOS/querya\n' + 'Identifier=com.queryahub.querya\n' + 'Authority=Developer ID Application: QueryaHub (AB12CD34EF)\n' + 'TeamIdentifier=AB12CD34EF\n' + 'Sealed Resources version=2 rules=13 files=120\n'; + expect(parseCodesignTeamId(out), 'AB12CD34EF'); + }); + + test('an ad-hoc signature has none', () { + const out = 'Identifier=querya_desktop\n' + 'Signature=adhoc\n' + 'TeamIdentifier=not set\n'; + expect(parseCodesignTeamId(out), isNull); + }); + + test('no TeamIdentifier line has none', () { + expect(parseCodesignTeamId('code object is not signed at all'), isNull); + expect(parseCodesignTeamId(''), isNull); + }); + }); + + group('signerMismatchProblem (#1588)', () { + test('the same team may replace the app', () { + expect( + signerMismatchProblem(installedTeam: 'AB12CD34EF', updateTeam: 'AB12CD34EF'), + isNull, + ); + }); + + test('another team may not', () { + expect( + signerMismatchProblem(installedTeam: 'AB12CD34EF', updateTeam: 'ZZ99ZZ99ZZ'), + allOf(contains('different developer'), contains('ZZ99ZZ99ZZ'), + contains('AB12CD34EF')), + ); + }); + + test('an ad-hoc or unsigned update may not replace a signed app', () { + expect( + signerMismatchProblem(installedTeam: 'AB12CD34EF', updateTeam: null), + contains('not signed by the developer'), + ); + }); + + test('an unsigned installed app has nothing to compare with', () { + expect(signerMismatchProblem(installedTeam: null, updateTeam: null), isNull); + expect( + signerMismatchProblem(installedTeam: null, updateTeam: 'AB12CD34EF'), + isNull, + ); + }); + }); +}