From 92adbc07a2bab097885b8a73d2d28bf483c2f50a Mon Sep 17 00:00:00 2001 From: Anand Hegde Date: Sun, 20 Sep 2026 00:44:21 +0530 Subject: [PATCH] Add OpenPGP and X.509 commit signing SSH commit signing landed in #2789, which deliberately left the other two formats alone: a repository configured with commit.gpgsign and an OpenPGP or X.509 gpg.format silently produced an unsigned commit. This extends the same machinery to cover both, so GitUp now honours commit.gpgsign for every format Git itself supports. The signature is produced by running the configured signing program the way Git runs it, " --status-fd=2 -bsau ", with the commit buffer on stdin. Program resolution follows Git's rules: gpg.openpgp.program then gpg.program (its legacy synonym) then "gpg" for OpenPGP, and gpg.x509.program then "gpgsm" for X.509, with gpg.program deliberately not applying to X.509. When user.signingkey is unset, the committer identity is passed as the user ID, which is what Git does so that GnuPG selects the key matching the commit rather than whichever key it considers its own default. Success is decided by GnuPG's machine-readable status rather than its exit code alone, because GnuPG can exit zero without having produced a signature. That is the reason for --status-fd=2, and it matches how Git validates the same invocation. An unrecognised gpg.format is now an error instead of quietly producing an unsigned commit. Git rejects it too, and silently dropping the signature is the worst available outcome for someone who asked for signed commits. Every one of these rules was verified against git 2.50.1 by pointing gpg.program at a recording stub and comparing the resulting argument vector. --- GitUpKit/Core/GCCommitSigning-Tests.m | 148 ++++++++++++++++++++- GitUpKit/Core/GCCommitSigning.m | 116 +++++++++++++--- GitUpKit/Core/GCCommitSigningTestHelpers.h | 1 + GitUpKit/Core/GCCommitSigningTestHelpers.m | 4 + 4 files changed, 249 insertions(+), 20 deletions(-) diff --git a/GitUpKit/Core/GCCommitSigning-Tests.m b/GitUpKit/Core/GCCommitSigning-Tests.m index 0dc83c39..fd803fa1 100644 --- a/GitUpKit/Core/GCCommitSigning-Tests.m +++ b/GitUpKit/Core/GCCommitSigning-Tests.m @@ -58,6 +58,40 @@ static BOOL _WriteExecutable(NSString* path, NSString* contents) { return _WriteExecutable(path, contents) ? path : nil; } +// Stands in for gpg(1) or gpgsm(1): swallows the commit buffer on stdin, records the arguments +// it was passed, then writes an armored block on stdout and GnuPG's status output on stderr. +static NSString* _CreateFakeGPGSigner(NSString* directory, NSString* armorLabel, BOOL reportSignatureCreated, int exitStatus, NSString* argumentsPath) { + NSString* path = [directory stringByAppendingPathComponent:[[NSProcessInfo processInfo] globallyUniqueString]]; + NSMutableArray* lines = [NSMutableArray arrayWithObjects:@"#!/bin/sh", @"/bin/cat >/dev/null", nil]; + + if (argumentsPath) { + [lines addObject:[NSString stringWithFormat:@"printf '%%s\\n' \"$@\" > '%@'", argumentsPath]]; + } + if (reportSignatureCreated) { + [lines addObject:@"echo '[GNUPG:] SIG_CREATED D 1 8 00 1700000000 0123456789ABCDEF' >&2"]; + } + if (exitStatus == 0) { + [lines addObject:[NSString stringWithFormat:@"printf '%%s\\n' '-----BEGIN %@-----' 'fake-signature' '-----END %@-----'", armorLabel, armorLabel]]; + } else { + [lines addObject:@"echo signer failed >&2"]; + [lines addObject:[NSString stringWithFormat:@"exit %i", exitStatus]]; + } + [lines addObject:@""]; + + return _WriteExecutable(path, [lines componentsJoinedByString:@"\n"]) ? path : nil; +} + +static NSArray* _RecordedArguments(NSString* path) { + NSString* contents = [NSString stringWithContentsOfFile:path encoding:NSUTF8StringEncoding error:NULL]; + NSMutableArray* arguments = [[NSMutableArray alloc] init]; + for (NSString* line in [contents componentsSeparatedByString:@"\n"]) { + if (line.length) { + [arguments addObject:line]; + } + } + return arguments; +} + static GCCommit* _CreateCommitFromRepositoryIndex(GCRepository* repository, NSString* message, NSError** error) { GCCommit* commit = nil; git_index* index = NULL; @@ -87,7 +121,7 @@ static BOOL _WriteExecutable(NSString* path, NSString* contents) { @implementation GCEmptyRepositoryTests (GCCommitSigning) -- (void)testCommitSigningLeavesCommitsUnsignedWhenDisabledOrUnsupported { +- (void)testCommitSigningLeavesCommitsUnsignedWhenDisabled { [self updateFileAtPath:@"unsigned.txt" withString:@"unsigned\n"]; XCTAssertTrue([self.repository addFileToIndex:@"unsigned.txt" error:NULL]); @@ -95,14 +129,118 @@ - (void)testCommitSigningLeavesCommitsUnsignedWhenDisabledOrUnsupported { XCTAssertNotNil(unsignedCommit); XCTAssertNil(GCCommitSignature(unsignedCommit)); - XCTAssertTrue(_WriteLocalConfigOption(self.repository, @"commit.gpgsign", @"true")); + // Configuring a format without turning signing on must not sign either. XCTAssertTrue(_WriteLocalConfigOption(self.repository, @"gpg.format", @"openpgp")); + [self updateFileAtPath:@"format-only.txt" withString:@"format only\n"]; + XCTAssertTrue([self.repository addFileToIndex:@"format-only.txt" error:NULL]); + + GCCommit* formatOnlyCommit = _CreateCommitFromRepositoryIndex(self.repository, @"Format without gpgsign", NULL); + XCTAssertNotNil(formatOnlyCommit); + XCTAssertNil(GCCommitSignature(formatOnlyCommit)); +} + +- (void)testCommitSigningRejectsUnknownFormat { + XCTAssertTrue(_WriteLocalConfigOption(self.repository, @"commit.gpgsign", @"true")); + XCTAssertTrue(_WriteLocalConfigOption(self.repository, @"gpg.format", @"pgp")); + [self updateFileAtPath:@"unknown-format.txt" withString:@"unknown format\n"]; + XCTAssertTrue([self.repository addFileToIndex:@"unknown-format.txt" error:NULL]); + + NSError* error; + XCTAssertNil(_CreateCommitFromRepositoryIndex(self.repository, @"Unknown format", &error)); + XCTAssertTrue([error.localizedDescription containsString:@"gpg.format"]); +} + +- (void)testCommitSigningSignsWithOpenPGPByDefault { + NSString* argumentsPath = [self.temporaryPath stringByAppendingPathComponent:@"openpgp-arguments"]; + NSString* signer = _CreateFakeGPGSigner(self.temporaryPath, @"PGP SIGNATURE", YES, 0, argumentsPath); + + XCTAssertNotNil(signer); + XCTAssertTrue(_WriteLocalConfigOption(self.repository, @"commit.gpgsign", @"true")); + XCTAssertTrue(_WriteLocalConfigOption(self.repository, @"gpg.program", signer)); [self updateFileAtPath:@"openpgp.txt" withString:@"openpgp\n"]; XCTAssertTrue([self.repository addFileToIndex:@"openpgp.txt" error:NULL]); - GCCommit* openPGPCommit = _CreateCommitFromRepositoryIndex(self.repository, @"OpenPGP config remains unsigned", NULL); - XCTAssertNotNil(openPGPCommit); - XCTAssertNil(GCCommitSignature(openPGPCommit)); + // "gpg.format" is left unset, so this covers OpenPGP being the default format as well. + GCCommit* commit = _CreateCommitFromRepositoryIndex(self.repository, @"OpenPGP", NULL); + XCTAssertNotNil(commit); + XCTAssertTrue(GCCommitHasOpenPGPSignature(commit)); + + // With no "user.signingkey", Git hands GnuPG the committer identity rather than + // letting GnuPG fall back to whichever key it considers its own default. + XCTAssertEqualObjects(_RecordedArguments(argumentsPath), (@[ @"--status-fd=2", @"-bsau", @"Bot " ])); +} + +- (void)testCommitSigningPrefersOpenPGPProgramAndConfiguredSigningKey { + NSString* argumentsPath = [self.temporaryPath stringByAppendingPathComponent:@"openpgp-program-arguments"]; + NSString* legacyProgram = _CreateFakeGPGSigner(self.temporaryPath, @"PGP SIGNATURE", YES, 7, nil); + NSString* openPGPProgram = _CreateFakeGPGSigner(self.temporaryPath, @"PGP SIGNATURE", YES, 0, argumentsPath); + + XCTAssertNotNil(legacyProgram); + XCTAssertNotNil(openPGPProgram); + XCTAssertTrue(_WriteLocalConfigOption(self.repository, @"commit.gpgsign", @"true")); + XCTAssertTrue(_WriteLocalConfigOption(self.repository, @"gpg.format", @"openpgp")); + // "gpg.program" is the legacy synonym, so the more specific variable has to win. + XCTAssertTrue(_WriteLocalConfigOption(self.repository, @"gpg.program", legacyProgram)); + XCTAssertTrue(_WriteLocalConfigOption(self.repository, @"gpg.openpgp.program", openPGPProgram)); + XCTAssertTrue(_WriteLocalConfigOption(self.repository, @"user.signingkey", @"0123456789ABCDEF")); + [self updateFileAtPath:@"openpgp-program.txt" withString:@"openpgp program\n"]; + XCTAssertTrue([self.repository addFileToIndex:@"openpgp-program.txt" error:NULL]); + + GCCommit* commit = _CreateCommitFromRepositoryIndex(self.repository, @"OpenPGP program", NULL); + XCTAssertNotNil(commit); + XCTAssertTrue(GCCommitHasOpenPGPSignature(commit)); + XCTAssertEqualObjects(_RecordedArguments(argumentsPath), (@[ @"--status-fd=2", @"-bsau", @"0123456789ABCDEF" ])); +} + +- (void)testCommitSigningSupportsX509Format { + NSString* openPGPProgram = _CreateFakeGPGSigner(self.temporaryPath, @"PGP SIGNATURE", YES, 0, nil); + NSString* x509Program = _CreateFakeGPGSigner(self.temporaryPath, @"SIGNED MESSAGE", YES, 0, nil); + + XCTAssertNotNil(openPGPProgram); + XCTAssertNotNil(x509Program); + XCTAssertTrue(_WriteLocalConfigOption(self.repository, @"commit.gpgsign", @"true")); + XCTAssertTrue(_WriteLocalConfigOption(self.repository, @"gpg.format", @"x509")); + // Unlike "gpg.openpgp.program", "gpg.program" must not apply to X.509. + XCTAssertTrue(_WriteLocalConfigOption(self.repository, @"gpg.program", openPGPProgram)); + XCTAssertTrue(_WriteLocalConfigOption(self.repository, @"gpg.x509.program", x509Program)); + [self updateFileAtPath:@"x509.txt" withString:@"x509\n"]; + XCTAssertTrue([self.repository addFileToIndex:@"x509.txt" error:NULL]); + + GCCommit* commit = _CreateCommitFromRepositoryIndex(self.repository, @"X.509", NULL); + XCTAssertNotNil(commit); + XCTAssertTrue([GCCommitSignature(commit) containsString:@"BEGIN SIGNED MESSAGE"]); +} + +// GnuPG can exit successfully without having produced a signature, which is why Git asks +// for its machine-readable status instead of trusting the exit code alone. +- (void)testCommitSigningFailsWhenGPGReportsNoCreatedSignature { + NSString* signer = _CreateFakeGPGSigner(self.temporaryPath, @"PGP SIGNATURE", NO, 0, nil); + + XCTAssertNotNil(signer); + XCTAssertTrue(_WriteLocalConfigOption(self.repository, @"commit.gpgsign", @"true")); + XCTAssertTrue(_WriteLocalConfigOption(self.repository, @"gpg.format", @"openpgp")); + XCTAssertTrue(_WriteLocalConfigOption(self.repository, @"gpg.program", signer)); + [self updateFileAtPath:@"silent-signer.txt" withString:@"silent signer\n"]; + XCTAssertTrue([self.repository addFileToIndex:@"silent-signer.txt" error:NULL]); + + NSError* error; + XCTAssertNil(_CreateCommitFromRepositoryIndex(self.repository, @"Silent signer", &error)); + XCTAssertTrue([error.localizedDescription containsString:@"did not report a created signature"]); +} + +- (void)testCommitSigningFailsOnGPGFailure { + NSString* signer = _CreateFakeGPGSigner(self.temporaryPath, @"PGP SIGNATURE", YES, 5, nil); + + XCTAssertNotNil(signer); + XCTAssertTrue(_WriteLocalConfigOption(self.repository, @"commit.gpgsign", @"true")); + XCTAssertTrue(_WriteLocalConfigOption(self.repository, @"gpg.format", @"openpgp")); + XCTAssertTrue(_WriteLocalConfigOption(self.repository, @"gpg.program", signer)); + [self updateFileAtPath:@"failing-gpg.txt" withString:@"failing gpg\n"]; + XCTAssertTrue([self.repository addFileToIndex:@"failing-gpg.txt" error:NULL]); + + NSError* error; + XCTAssertNil(_CreateCommitFromRepositoryIndex(self.repository, @"Failing GPG", &error)); + XCTAssertTrue([error.localizedDescription containsString:@"non-zero status"]); } - (void)testCommitSigningRequiresSSHKey { diff --git a/GitUpKit/Core/GCCommitSigning.m b/GitUpKit/Core/GCCommitSigning.m index 76359c4a..61fd6010 100644 --- a/GitUpKit/Core/GCCommitSigning.m +++ b/GitUpKit/Core/GCCommitSigning.m @@ -21,6 +21,13 @@ #if !TARGET_OS_IPHONE +typedef NS_ENUM(NSUInteger, GCCommitSigningFormat) { + kGCCommitSigningFormat_None = 0, + kGCCommitSigningFormat_OpenPGP, + kGCCommitSigningFormat_X509, + kGCCommitSigningFormat_SSH +}; + static NSString* _StringFromTaskOutput(NSData* data) { return [[[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding] stringByTrimmingCharactersInSet:[NSCharacterSet whitespaceAndNewlineCharacterSet]]; } @@ -64,25 +71,29 @@ static BOOL _ReadConfigBool(GCRepository* repository, const char* variable, BOOL return success; } -static BOOL _ShouldSSHSignCommit(GCRepository* repository, BOOL* shouldSign, NSError** error) { +static BOOL _CommitSigningFormat(GCRepository* repository, GCCommitSigningFormat* signingFormat, NSError** error) { BOOL gpgSign = NO; if (!_ReadConfigBool(repository, "commit.gpgsign", &gpgSign, error)) { return NO; } if (!gpgSign) { - *shouldSign = NO; + *signingFormat = kGCCommitSigningFormat_None; return YES; } NSString* format = [[repository readConfigOptionForVariable:@"gpg.format" error:NULL] value]; - if (!format.length || ([format caseInsensitiveCompare:@"ssh"] != NSOrderedSame)) { - // Only SSH commit signing is currently supported. - // Preserve existing GitUp behavior for OpenPGP/X.509 configs by creating an unsigned commit. - *shouldSign = NO; - return YES; + if (!format.length || ([format caseInsensitiveCompare:@"openpgp"] == NSOrderedSame)) { + *signingFormat = kGCCommitSigningFormat_OpenPGP; // Same default as Git. + } else if ([format caseInsensitiveCompare:@"x509"] == NSOrderedSame) { + *signingFormat = kGCCommitSigningFormat_X509; + } else if ([format caseInsensitiveCompare:@"ssh"] == NSOrderedSame) { + *signingFormat = kGCCommitSigningFormat_SSH; + } else { + // Refuse rather than silently producing an unsigned commit, which is what Git does too. + GC_SET_GENERIC_ERROR(@"Invalid value for \"gpg.format\": %@", format); + return NO; } - *shouldSign = YES; return YES; } @@ -219,6 +230,76 @@ static BOOL _LooksLikeInlineSSHKey(NSString* key) { return signature; } +static NSString* _GPGProgram(GCRepository* repository, GCCommitSigningFormat format) { + NSString* program; + + if (format == kGCCommitSigningFormat_X509) { + program = [[repository readConfigOptionForVariable:@"gpg.x509.program" error:NULL] value]; + return program.length ? program.stringByExpandingTildeInPath : @"gpgsm"; + } + + program = [[repository readConfigOptionForVariable:@"gpg.openpgp.program" error:NULL] value]; + if (!program.length) { + // "gpg.program" is Git's legacy synonym for "gpg.openpgp.program" and deliberately + // does not apply to the X.509 format. + program = [[repository readConfigOptionForVariable:@"gpg.program" error:NULL] value]; + } + return program.length ? program.stringByExpandingTildeInPath : @"gpg"; +} + +static NSString* _GPGSigningKey(GCRepository* repository, const git_signature* committer) { + NSString* key = [[[repository readConfigOptionForVariable:@"user.signingkey" error:NULL] value] stringByTrimmingCharactersInSet:[NSCharacterSet whitespaceAndNewlineCharacterSet]]; + if (key.length) { + return key; + } + // Git falls back to the committer identity so GnuPG picks the key matching it rather + // than whichever key GnuPG happens to consider its own default. + return [NSString stringWithFormat:@"%s <%s>", committer->name, committer->email]; +} + +static NSString* _GPGSignatureForCommitBuffer(GCRepository* repository, NSData* commitBuffer, GCCommitSigningFormat format, const git_signature* committer, NSError** error) { + NSString* path = _CommitSigningPATH(repository, error); + if (!path) { + return nil; + } + + GCTask* task = _TaskWithPATH(repository, @"/usr/bin/env", path); + int status; + NSData* stdoutData; + NSData* stderrData; + NSArray* arguments = @[ _GPGProgram(repository, format), @"--status-fd=2", @"-bsau", _GPGSigningKey(repository, committer) ]; + if (![task runWithArguments:arguments stdin:commitBuffer stdout:&stdoutData stderr:&stderrData exitStatus:&status error:error]) { + return nil; + } + if (status != 0) { + if (error) { + *error = _TaskFailureError(@"GPG commit signer", status, stdoutData, stderrData); + } + return nil; + } + + // "--status-fd=2" interleaves GnuPG's machine-readable status with stderr. It is the only + // way to tell a real signature apart from a GnuPG that exited successfully without making one. + NSString* statusOutput = [[NSString alloc] initWithData:stderrData encoding:NSUTF8StringEncoding]; + if (![statusOutput containsString:@"[GNUPG:] SIG_CREATED "]) { + if (error) { + NSString* output = _StringFromTaskOutput(stderrData.length ? stderrData : stdoutData); + NSString* reason = output.length ? [NSString stringWithFormat:@": %@", output] : @""; + *error = GCNewError(kGCErrorCode_Generic, [NSString stringWithFormat:@"GPG commit signer did not report a created signature%@", reason]); + } + return nil; + } + + NSString* signature = _StringFromTaskOutput(stdoutData); + if (!signature.length) { + if (error) { + *error = GCNewError(kGCErrorCode_Generic, @"GPG commit signer did not return a signature"); + } + return nil; + } + return signature; +} + #endif GCCommit* GCCreateCommitFromTreeWithOptionalSignature(GCRepository* repository, git_tree* tree, const git_commit** parents, NSUInteger count, const git_signature* author, NSString* message, NSError** error) { @@ -231,8 +312,8 @@ static BOOL _LooksLikeInlineSSHKey(NSString* key) { #if !TARGET_OS_IPHONE git_buf commitBuffer = {0}; NSData* commitData = nil; - NSString* sshSignature = nil; - BOOL shouldSign = NO; + NSString* commitSignature = nil; + GCCommitSigningFormat signingFormat = kGCCommitSigningFormat_None; #endif git_oid oid; @@ -241,18 +322,23 @@ static BOOL _LooksLikeInlineSSHKey(NSString* key) { cleanedMessage = GCCleanedUpCommitMessage(message); cleanedMessageBytes = (const char*)cleanedMessage.bytes; #if !TARGET_OS_IPHONE - if (!_ShouldSSHSignCommit(repository, &shouldSign, error)) { + if (!_CommitSigningFormat(repository, &signingFormat, error)) { goto cleanup; } - if (shouldSign) { + if (signingFormat != kGCCommitSigningFormat_None) { CALL_LIBGIT2_FUNCTION_GOTO(cleanup, git_commit_create_buffer, &commitBuffer, repository.private, authorSignature, signature, NULL, cleanedMessageBytes, tree, count, parents); commitData = [[NSData alloc] initWithBytes:commitBuffer.ptr length:commitBuffer.size]; - sshSignature = _SSHSignatureForCommitBuffer(repository, commitData, error); - if (!sshSignature) { + if (signingFormat == kGCCommitSigningFormat_SSH) { + commitSignature = _SSHSignatureForCommitBuffer(repository, commitData, error); + } else { + commitSignature = _GPGSignatureForCommitBuffer(repository, commitData, signingFormat, signature, error); + } + if (!commitSignature) { goto cleanup; } - CALL_LIBGIT2_FUNCTION_GOTO(cleanup, git_commit_create_with_signature, &oid, repository.private, commitBuffer.ptr, sshSignature.UTF8String, "gpgsig"); + // Git writes every signing format into the "gpgsig" header, OpenPGP and X.509 included. + CALL_LIBGIT2_FUNCTION_GOTO(cleanup, git_commit_create_with_signature, &oid, repository.private, commitBuffer.ptr, commitSignature.UTF8String, "gpgsig"); } else { #endif CALL_LIBGIT2_FUNCTION_GOTO(cleanup, git_commit_create, &oid, repository.private, NULL, authorSignature, signature, NULL, cleanedMessageBytes, tree, count, parents); diff --git a/GitUpKit/Core/GCCommitSigningTestHelpers.h b/GitUpKit/Core/GCCommitSigningTestHelpers.h index 5090663c..8ac6ab75 100644 --- a/GitUpKit/Core/GCCommitSigningTestHelpers.h +++ b/GitUpKit/Core/GCCommitSigningTestHelpers.h @@ -20,4 +20,5 @@ extern NSString* GCCommitSignature(GCCommit* commit); extern BOOL GCCommitHasSSHSignature(GCCommit* commit); +extern BOOL GCCommitHasOpenPGPSignature(GCCommit* commit); extern BOOL GCConfigureSSHSigningWithKeyPath(GCRepository* repository, NSString* keyPath); diff --git a/GitUpKit/Core/GCCommitSigningTestHelpers.m b/GitUpKit/Core/GCCommitSigningTestHelpers.m index 7cd08cec..565f7eb0 100644 --- a/GitUpKit/Core/GCCommitSigningTestHelpers.m +++ b/GitUpKit/Core/GCCommitSigningTestHelpers.m @@ -36,6 +36,10 @@ BOOL GCCommitHasSSHSignature(GCCommit* commit) { return [GCCommitSignature(commit) containsString:@"BEGIN SSH SIGNATURE"]; } +BOOL GCCommitHasOpenPGPSignature(GCCommit* commit) { + return [GCCommitSignature(commit) containsString:@"BEGIN PGP SIGNATURE"]; +} + BOOL GCConfigureSSHSigningWithKeyPath(GCRepository* repository, NSString* keyPath) { return [repository writeConfigOptionForLevel:kGCConfigLevel_Local variable:@"commit.gpgsign" withValue:@"true" error:NULL] && [repository writeConfigOptionForLevel:kGCConfigLevel_Local