From e595cbd7f3e1766f562616ea08f01a2fdafec44b Mon Sep 17 00:00:00 2001 From: Oleksandr Hleba Date: Tue, 30 Jun 2026 17:24:51 +0200 Subject: [PATCH 1/3] Added PAT support for subrepo cloning --- system7-tests/gitGitHubTokenAuthTests.m | 98 ++++++++++++++++++++ system7.xcodeproj/project.pbxproj | 4 + system7/git/Git+Tests.h | 4 + system7/git/Git.m | 118 ++++++++++++++++++++++++ 4 files changed, 224 insertions(+) create mode 100644 system7-tests/gitGitHubTokenAuthTests.m diff --git a/system7-tests/gitGitHubTokenAuthTests.m b/system7-tests/gitGitHubTokenAuthTests.m new file mode 100644 index 0000000..ff595b8 --- /dev/null +++ b/system7-tests/gitGitHubTokenAuthTests.m @@ -0,0 +1,98 @@ +// +// gitGitHubTokenAuthTests.m +// system7-tests +// +// Copyright © 2026 Readdle. All rights reserved. +// + +#import + +#import "Git.h" +#import "Git+Tests.h" + +@interface gitGitHubTokenAuthTests : XCTestCase +@end + +@implementation gitGitHubTokenAuthTests + +#pragma mark - GIT_CONFIG_* env builder - + +- (void)testReturnsEmptyWhenUserNil { + XCTAssertEqualObjects(@{}, [GitRepository gitHubTokenConfigEnvironmentForUser:nil token:@"abc" existingConfigCount:0]); +} + +- (void)testReturnsEmptyWhenUserEmpty { + XCTAssertEqualObjects(@{}, [GitRepository gitHubTokenConfigEnvironmentForUser:@"" token:@"abc" existingConfigCount:0]); +} + +- (void)testReturnsEmptyWhenTokenNil { + XCTAssertEqualObjects(@{}, [GitRepository gitHubTokenConfigEnvironmentForUser:@"alice" token:nil existingConfigCount:0]); +} + +- (void)testReturnsEmptyWhenTokenEmpty { + XCTAssertEqualObjects(@{}, [GitRepository gitHubTokenConfigEnvironmentForUser:@"alice" token:@"" existingConfigCount:0]); +} + +- (void)testBuildsHeaderAuthEntriesFromZero { + NSDictionary *const env = + [GitRepository gitHubTokenConfigEnvironmentForUser:@"alice" token:@"abc123" existingConfigCount:0]; + + // base64("alice:abc123") == "YWxpY2U6YWJjMTIz" + NSDictionary *const expected = @{ + @"GIT_CONFIG_COUNT": @"3", + @"GIT_CONFIG_KEY_0": @"url.https://github.com/.insteadOf", + @"GIT_CONFIG_VALUE_0": @"git@github.com:", + @"GIT_CONFIG_KEY_1": @"url.https://github.com/.insteadOf", + @"GIT_CONFIG_VALUE_1": @"ssh://git@github.com/", + @"GIT_CONFIG_KEY_2": @"http.https://github.com/.extraheader", + @"GIT_CONFIG_VALUE_2": @"Authorization: Basic YWxpY2U6YWJjMTIz", + }; + XCTAssertEqualObjects(expected, env); +} + +- (void)testAppendsPastExistingConfigCount { + // A nested s7 inherits the parent's injected GIT_CONFIG_COUNT=3 and must not + // clobber entries 0..2. + NSDictionary *const env = + [GitRepository gitHubTokenConfigEnvironmentForUser:@"alice" token:@"abc" existingConfigCount:3]; + + XCTAssertEqualObjects(@"6", env[@"GIT_CONFIG_COUNT"]); + XCTAssertEqualObjects(@"url.https://github.com/.insteadOf", env[@"GIT_CONFIG_KEY_3"]); + XCTAssertEqualObjects(@"git@github.com:", env[@"GIT_CONFIG_VALUE_3"]); + XCTAssertEqualObjects(@"ssh://git@github.com/", env[@"GIT_CONFIG_VALUE_4"]); + XCTAssertEqualObjects(@"http.https://github.com/.extraheader", env[@"GIT_CONFIG_KEY_5"]); + // base64("alice:abc") == "YWxpY2U6YWJj" + XCTAssertEqualObjects(@"Authorization: Basic YWxpY2U6YWJj", env[@"GIT_CONFIG_VALUE_5"]); + // Must not clobber the caller's existing entries (indices 0..2). + XCTAssertNil(env[@"GIT_CONFIG_KEY_0"]); + XCTAssertNil(env[@"GIT_CONFIG_VALUE_2"]); +} + +- (void)testTokenNeverAppearsRawOnlyInBase64Header { + NSString *const user = @"alice"; + NSString *const token = @"ghp_SuperSecret/@:%123"; // chars that would have needed URL-escaping + NSDictionary *const env = + [GitRepository gitHubTokenConfigEnvironmentForUser:user token:token existingConfigCount:0]; + + NSString *const expectedBasic = + [[[NSString stringWithFormat:@"%@:%@", user, token] dataUsingEncoding:NSUTF8StringEncoding] + base64EncodedStringWithOptions:0]; + + // The raw token must not appear in any entry — it rides only in the header, + // base64-encoded. (base64 needs no percent-encoding for arbitrary bytes.) + NSString *const joined = [env.allValues componentsJoinedByString:@"\n"]; + XCTAssertFalse([joined containsString:token], @"raw token leaked into config: %@", joined); + NSString *const expectedHeader = [NSString stringWithFormat:@"Authorization: Basic %@", expectedBasic]; + XCTAssertEqualObjects(expectedHeader, env[@"GIT_CONFIG_VALUE_2"]); +} + +- (void)testGithubDotComOnly { + NSDictionary *const env = + [GitRepository gitHubTokenConfigEnvironmentForUser:@"alice" token:@"abc" existingConfigCount:0]; + + NSString *const joined = [[env.allKeys arrayByAddingObjectsFromArray:env.allValues] componentsJoinedByString:@" "]; + XCTAssertFalse([joined containsString:@"gitlab"]); + XCTAssertFalse([joined containsString:@"bitbucket"]); +} + +@end diff --git a/system7.xcodeproj/project.pbxproj b/system7.xcodeproj/project.pbxproj index 69561d0..187a31f 100644 --- a/system7.xcodeproj/project.pbxproj +++ b/system7.xcodeproj/project.pbxproj @@ -97,6 +97,7 @@ BEE928A12456DA6500BD6B86 /* Git.m in Sources */ = {isa = PBXBuildFile; fileRef = BEE928A02456DA6500BD6B86 /* Git.m */; }; BEFAF7A125718AB7000D90C3 /* bootstrapTests.m in Sources */ = {isa = PBXBuildFile; fileRef = BEFAF7A025718AB7000D90C3 /* bootstrapTests.m */; }; CE5BB61F25FA63A8002596B9 /* gitPackedRefsTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CE5BB61E25FA63A8002596B9 /* gitPackedRefsTests.m */; }; + A11C0CE526052600A0010001 /* gitGitHubTokenAuthTests.m in Sources */ = {isa = PBXBuildFile; fileRef = A11C0CE526052600A0010002 /* gitGitHubTokenAuthTests.m */; }; /* End PBXBuildFile section */ /* Begin PBXCopyFilesBuildPhase section */ @@ -215,6 +216,7 @@ BEE928A02456DA6500BD6B86 /* Git.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = Git.m; sourceTree = ""; }; BEFAF7A025718AB7000D90C3 /* bootstrapTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = bootstrapTests.m; sourceTree = ""; }; CE5BB61E25FA63A8002596B9 /* gitPackedRefsTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = gitPackedRefsTests.m; sourceTree = ""; }; + A11C0CE526052600A0010002 /* gitGitHubTokenAuthTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = gitGitHubTokenAuthTests.m; sourceTree = ""; }; /* End PBXFileReference section */ /* Begin PBXFrameworksBuildPhase section */ @@ -351,6 +353,7 @@ BEE672CE2523B76500DB09BC /* iniParserTests.m */, BEBE40C32576D04D00E39755 /* deinitTests.m */, CE5BB61E25FA63A8002596B9 /* gitPackedRefsTests.m */, + A11C0CE526052600A0010002 /* gitGitHubTokenAuthTests.m */, ); path = "system7-tests"; sourceTree = ""; @@ -623,6 +626,7 @@ BEE1CD03246A9F6F00E2CC3B /* controlTests.m in Sources */, BE394D3D2486ADB500ED6E05 /* S7CheckoutCommand.m in Sources */, CE5BB61F25FA63A8002596B9 /* gitPackedRefsTests.m in Sources */, + A11C0CE526052600A0010001 /* gitGitHubTokenAuthTests.m in Sources */, BE3A29E524604C10004A2B31 /* statusTests.m in Sources */, 6DD4A8392750E8A40050F3FD /* S7IniConfigOptions.m in Sources */, BE8218892452C1DE00E878A8 /* configParserTests.m in Sources */, diff --git a/system7/git/Git+Tests.h b/system7/git/Git+Tests.h index 60553ce..23c2149 100644 --- a/system7/git/Git+Tests.h +++ b/system7/git/Git+Tests.h @@ -20,6 +20,10 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, class) void (^testRepoConfigureOnInitBlock)(GitRepository *repo); @property (nonatomic, readonly) BOOL hasMergeConflict; ++ (NSDictionary *)gitHubTokenConfigEnvironmentForUser:(nullable NSString *)user + token:(nullable NSString *)token + existingConfigCount:(NSInteger)existingConfigCount; + @end NS_ASSUME_NONNULL_END diff --git a/system7/git/Git.m b/system7/git/Git.m index 57a3cab..61bbf96 100644 --- a/system7/git/Git.m +++ b/system7/git/Git.m @@ -60,6 +60,65 @@ + (BOOL)envGitTraceEnabled { return traceEnabled; } +// Username for HTTPS subrepo auth. S7_GIT_TOKEN, falling back to GH_TOKEN. ++ (nullable NSString *)envGitAuthUser { + NSDictionary *const env = NSProcessInfo.processInfo.environment; + NSString *const user = env[@"S7_GIT_USER"]; + if (user.length > 0) { + return user; + } + NSString *const ghUser = env[@"GH_USER"]; + return ghUser.length > 0 ? ghUser : nil; +} + +// Token for HTTPS subrepo auth. S7_GIT_TOKEN, falling back to GH_TOKEN. ++ (nullable NSString *)envGitAuthToken { + NSDictionary *const env = NSProcessInfo.processInfo.environment; + NSString *const token = env[@"S7_GIT_TOKEN"]; + if (token.length > 0) { + return token; + } + NSString *const ghToken = env[@"GH_TOKEN"]; + return ghToken.length > 0 ? ghToken : nil; +} + +// HTTPS+token auth is auto-enabled when both credentials are present. ++ (BOOL)envGitHubTokenAuthEnabled { + return [self envGitAuthUser].length > 0 && [self envGitAuthToken].length > 0; +} + +// GIT_CONFIG_* entries that authenticate github.com over HTTPS: insteadOf +// rewrites SSH URLs to the clean https URL and an http..extraheader carries +// the PAT as a Basic-auth header (see +gitHubTokenConfigEnvironmentForUser:...). +// Delivered through the child's environment rather than `-c` argv, so the token +// never appears in the process argument list (`ps`, /proc, crash reporters), and +// — riding in a header, not a URL — nothing git echoes can leak it. `.git/config` +// keeps the original SSH URL, so the token never lands on disk either. ++ (NSDictionary *)gitHubTokenConfigEnvironment { + static dispatch_once_t onceToken; + static NSDictionary *configEnvironment; + dispatch_once(&onceToken, ^{ + NSDictionary *const env = NSProcessInfo.processInfo.environment; + const NSInteger existingConfigCount = MAX(0, [env[@"GIT_CONFIG_COUNT"] integerValue]); + configEnvironment = [self gitHubTokenConfigEnvironmentForUser:[self envGitAuthUser] + token:[self envGitAuthToken] + existingConfigCount:existingConfigCount]; + }); + return configEnvironment; +} + ++ (void)logSelectedAuthPathOnce { + static dispatch_once_t onceToken; + dispatch_once(&onceToken, ^{ + if ([self envGitHubTokenAuthEnabled]) { + fprintf(stderr, "s7: subrepo network auth: HTTPS via token\n"); + } + else { + s7TraceGit(@"s7: subrepo network auth: SSH (default)\n"); + } + }); +} + #pragma mark - Initialization - (nullable instancetype)initWithRepoPath:(NSString *)repoPath { @@ -332,6 +391,18 @@ + (int)runGitWithArguments:(NSArray *)arguments task.currentDirectoryURL = [NSURL fileURLWithPath:currentDirectoryPath]; } + [self logSelectedAuthPathOnce]; + + // Inject the HTTPS auth config via the child's environment (see + // +gitHubTokenConfigEnvironment). Empty on the SSH path, so dev machines and + // any non-token use are completely unaffected (no task.environment override). + NSDictionary *const credentialEnvironment = [self gitHubTokenConfigEnvironment]; + if (credentialEnvironment.count > 0) { + NSMutableDictionary *const environment = [NSProcessInfo.processInfo.environment mutableCopy]; + [environment addEntriesFromDictionary:credentialEnvironment]; + task.environment = environment; + } + // https://stackoverflow.com/questions/49184623/nstask-race-condition-with-readabilityhandler-block // we must use semaphore to make sure we finish reading from pipes properly once task finished it's execution. dispatch_semaphore_t pipeCloseSemaphore = dispatch_semaphore_create(0); @@ -1464,6 +1535,53 @@ + (void)setTestRepoConfigureOnInitBlock:(void (^)(GitRepository * _Nonnull))test _testRepoConfigureOnInitBlock = testRepoConfigureOnInitBlock; } +// Pure builder for +gitHubTokenConfigEnvironment. Kept free of process-env reads +// so tests can probe arbitrary inputs. Returns @{} when either credential is +// missing. +// +// The PAT travels in an HTTP Authorization header, never in a URL: +// * insteadOf rewrites every SSH github.com shape to the CLEAN (credential- +// free) https URL, so nothing git echoes (errors, GIT_TRACE_CURL, +// remote.origin.url) can ever carry the token; +// * a github.com-scoped http..extraheader supplies "Basic base64(user: +// token)" — the same mechanism actions/checkout uses. +// base64 swallows arbitrary token bytes, so no percent-encoding is needed. +// Delivered via GIT_CONFIG_* env: off-disk, off-argv. New entries append past +// existingConfigCount so a nested s7 (which inherits the parent's injected +// GIT_CONFIG_COUNT) doesn't clobber it. ++ (NSDictionary *)gitHubTokenConfigEnvironmentForUser:(nullable NSString *)user + token:(nullable NSString *)token + existingConfigCount:(NSInteger)existingConfigCount +{ + if (0 == user.length || 0 == token.length) { + return @{}; + } + + NSString *const userColonToken = [NSString stringWithFormat:@"%@:%@", user, token]; + NSString *const basic = [[userColonToken dataUsingEncoding:NSUTF8StringEncoding] base64EncodedStringWithOptions:0]; + + NSArray *const sshSources = @[ + @"git@github.com:", + @"ssh://git@github.com/", + ]; + + NSMutableDictionary *const result = [NSMutableDictionary new]; + NSInteger index = MAX(0, existingConfigCount); + for (NSString *source in sshSources) { + // url..insteadOf is multi-valued: each index contributes one rule. + result[[NSString stringWithFormat:@"GIT_CONFIG_KEY_%ld", (long)index]] = @"url.https://github.com/.insteadOf"; + result[[NSString stringWithFormat:@"GIT_CONFIG_VALUE_%ld", (long)index]] = source; + ++index; + } + // Scoped to github.com so the header is never attached to any other host. + result[[NSString stringWithFormat:@"GIT_CONFIG_KEY_%ld", (long)index]] = @"http.https://github.com/.extraheader"; + result[[NSString stringWithFormat:@"GIT_CONFIG_VALUE_%ld", (long)index]] = [NSString stringWithFormat:@"Authorization: Basic %@", basic]; + ++index; + + result[@"GIT_CONFIG_COUNT"] = [NSString stringWithFormat:@"%ld", (long)index]; + return result; +} + - (BOOL)hasMergeConflict { NSString *stdOutOutput = nil; const int unmergedFilesStatus = [self runGitCommand:@"ls-files -u" From ea7348ec67b18861518264bc029acf4541bd1177 Mon Sep 17 00:00:00 2001 From: Oleksandr Hleba Date: Mon, 6 Jul 2026 10:49:45 +0200 Subject: [PATCH 2/3] Adjusted based on code review --- system7-tests/gitGitHubTokenAuthTests.m | 24 ++++--- system7/git/Git+Tests.h | 2 +- system7/git/Git.m | 93 ++++++++++++------------- system7/main.m | 10 +++ 4 files changed, 71 insertions(+), 58 deletions(-) diff --git a/system7-tests/gitGitHubTokenAuthTests.m b/system7-tests/gitGitHubTokenAuthTests.m index ff595b8..6c71439 100644 --- a/system7-tests/gitGitHubTokenAuthTests.m +++ b/system7-tests/gitGitHubTokenAuthTests.m @@ -18,24 +18,24 @@ @implementation gitGitHubTokenAuthTests #pragma mark - GIT_CONFIG_* env builder - - (void)testReturnsEmptyWhenUserNil { - XCTAssertEqualObjects(@{}, [GitRepository gitHubTokenConfigEnvironmentForUser:nil token:@"abc" existingConfigCount:0]); + XCTAssertEqualObjects(@{}, [GitRepository gitHubTokenConfigEnvironmentForUser:nil token:@"abc" processEnvironment:@{}]); } - (void)testReturnsEmptyWhenUserEmpty { - XCTAssertEqualObjects(@{}, [GitRepository gitHubTokenConfigEnvironmentForUser:@"" token:@"abc" existingConfigCount:0]); + XCTAssertEqualObjects(@{}, [GitRepository gitHubTokenConfigEnvironmentForUser:@"" token:@"abc" processEnvironment:@{}]); } - (void)testReturnsEmptyWhenTokenNil { - XCTAssertEqualObjects(@{}, [GitRepository gitHubTokenConfigEnvironmentForUser:@"alice" token:nil existingConfigCount:0]); + XCTAssertEqualObjects(@{}, [GitRepository gitHubTokenConfigEnvironmentForUser:@"alice" token:nil processEnvironment:@{}]); } - (void)testReturnsEmptyWhenTokenEmpty { - XCTAssertEqualObjects(@{}, [GitRepository gitHubTokenConfigEnvironmentForUser:@"alice" token:@"" existingConfigCount:0]); + XCTAssertEqualObjects(@{}, [GitRepository gitHubTokenConfigEnvironmentForUser:@"alice" token:@"" processEnvironment:@{}]); } - (void)testBuildsHeaderAuthEntriesFromZero { NSDictionary *const env = - [GitRepository gitHubTokenConfigEnvironmentForUser:@"alice" token:@"abc123" existingConfigCount:0]; + [GitRepository gitHubTokenConfigEnvironmentForUser:@"alice" token:@"abc123" processEnvironment:@{}]; // base64("alice:abc123") == "YWxpY2U6YWJjMTIz" NSDictionary *const expected = @{ @@ -54,7 +54,7 @@ - (void)testAppendsPastExistingConfigCount { // A nested s7 inherits the parent's injected GIT_CONFIG_COUNT=3 and must not // clobber entries 0..2. NSDictionary *const env = - [GitRepository gitHubTokenConfigEnvironmentForUser:@"alice" token:@"abc" existingConfigCount:3]; + [GitRepository gitHubTokenConfigEnvironmentForUser:@"alice" token:@"abc" processEnvironment:@{@"GIT_CONFIG_COUNT": @"3"}]; XCTAssertEqualObjects(@"6", env[@"GIT_CONFIG_COUNT"]); XCTAssertEqualObjects(@"url.https://github.com/.insteadOf", env[@"GIT_CONFIG_KEY_3"]); @@ -68,11 +68,19 @@ - (void)testAppendsPastExistingConfigCount { XCTAssertNil(env[@"GIT_CONFIG_VALUE_2"]); } +- (void)testNegativeOrGarbageExistingCountClampsToZero { + NSDictionary *const env = + [GitRepository gitHubTokenConfigEnvironmentForUser:@"alice" token:@"abc" processEnvironment:@{@"GIT_CONFIG_COUNT": @"-5"}]; + + XCTAssertEqualObjects(@"3", env[@"GIT_CONFIG_COUNT"]); + XCTAssertEqualObjects(@"url.https://github.com/.insteadOf", env[@"GIT_CONFIG_KEY_0"]); +} + - (void)testTokenNeverAppearsRawOnlyInBase64Header { NSString *const user = @"alice"; NSString *const token = @"ghp_SuperSecret/@:%123"; // chars that would have needed URL-escaping NSDictionary *const env = - [GitRepository gitHubTokenConfigEnvironmentForUser:user token:token existingConfigCount:0]; + [GitRepository gitHubTokenConfigEnvironmentForUser:user token:token processEnvironment:@{}]; NSString *const expectedBasic = [[[NSString stringWithFormat:@"%@:%@", user, token] dataUsingEncoding:NSUTF8StringEncoding] @@ -88,7 +96,7 @@ - (void)testTokenNeverAppearsRawOnlyInBase64Header { - (void)testGithubDotComOnly { NSDictionary *const env = - [GitRepository gitHubTokenConfigEnvironmentForUser:@"alice" token:@"abc" existingConfigCount:0]; + [GitRepository gitHubTokenConfigEnvironmentForUser:@"alice" token:@"abc" processEnvironment:@{}]; NSString *const joined = [[env.allKeys arrayByAddingObjectsFromArray:env.allValues] componentsJoinedByString:@" "]; XCTAssertFalse([joined containsString:@"gitlab"]); diff --git a/system7/git/Git+Tests.h b/system7/git/Git+Tests.h index 23c2149..322ab7c 100644 --- a/system7/git/Git+Tests.h +++ b/system7/git/Git+Tests.h @@ -22,7 +22,7 @@ NS_ASSUME_NONNULL_BEGIN + (NSDictionary *)gitHubTokenConfigEnvironmentForUser:(nullable NSString *)user token:(nullable NSString *)token - existingConfigCount:(NSInteger)existingConfigCount; + processEnvironment:(NSDictionary *)processEnvironment; @end diff --git a/system7/git/Git.m b/system7/git/Git.m index 61bbf96..fb8d8df 100644 --- a/system7/git/Git.m +++ b/system7/git/Git.m @@ -60,7 +60,6 @@ + (BOOL)envGitTraceEnabled { return traceEnabled; } -// Username for HTTPS subrepo auth. S7_GIT_TOKEN, falling back to GH_TOKEN. + (nullable NSString *)envGitAuthUser { NSDictionary *const env = NSProcessInfo.processInfo.environment; NSString *const user = env[@"S7_GIT_USER"]; @@ -71,7 +70,6 @@ + (nullable NSString *)envGitAuthUser { return ghUser.length > 0 ? ghUser : nil; } -// Token for HTTPS subrepo auth. S7_GIT_TOKEN, falling back to GH_TOKEN. + (nullable NSString *)envGitAuthToken { NSDictionary *const env = NSProcessInfo.processInfo.environment; NSString *const token = env[@"S7_GIT_TOKEN"]; @@ -82,36 +80,39 @@ + (nullable NSString *)envGitAuthToken { return ghToken.length > 0 ? ghToken : nil; } -// HTTPS+token auth is auto-enabled when both credentials are present. -+ (BOOL)envGitHubTokenAuthEnabled { - return [self envGitAuthUser].length > 0 && [self envGitAuthToken].length > 0; -} - -// GIT_CONFIG_* entries that authenticate github.com over HTTPS: insteadOf -// rewrites SSH URLs to the clean https URL and an http..extraheader carries -// the PAT as a Basic-auth header (see +gitHubTokenConfigEnvironmentForUser:...). +// Full environment for the spawned git process when GitHub HTTPS token auth +// is enabled (both credentials present); nil when it's off. Computed once: the +// process environment plus GIT_CONFIG_* entries that authenticate github.com +// over HTTPS — insteadOf rewrites SSH URLs to the clean https URL and an +// http..extraheader carries the PAT as a Basic-auth header (see +// +gitHubTokenConfigEnvironmentForUser:token:processEnvironment:). // Delivered through the child's environment rather than `-c` argv, so the token // never appears in the process argument list (`ps`, /proc, crash reporters), and // — riding in a header, not a URL — nothing git echoes can leak it. `.git/config` // keeps the original SSH URL, so the token never lands on disk either. -+ (NSDictionary *)gitHubTokenConfigEnvironment { ++ (nullable NSDictionary *)gitHubTokenAuthTaskEnvironment { static dispatch_once_t onceToken; - static NSDictionary *configEnvironment; + static NSDictionary *taskEnvironment; dispatch_once(&onceToken, ^{ NSDictionary *const env = NSProcessInfo.processInfo.environment; - const NSInteger existingConfigCount = MAX(0, [env[@"GIT_CONFIG_COUNT"] integerValue]); - configEnvironment = [self gitHubTokenConfigEnvironmentForUser:[self envGitAuthUser] - token:[self envGitAuthToken] - existingConfigCount:existingConfigCount]; + NSDictionary *const configEnvironment = + [self gitHubTokenConfigEnvironmentForUser:[self envGitAuthUser] + token:[self envGitAuthToken] + processEnvironment:env]; + if (configEnvironment.count > 0) { + NSMutableDictionary *const merged = [env mutableCopy]; + [merged addEntriesFromDictionary:configEnvironment]; + taskEnvironment = [merged copy]; + } }); - return configEnvironment; + return taskEnvironment; } + (void)logSelectedAuthPathOnce { static dispatch_once_t onceToken; dispatch_once(&onceToken, ^{ - if ([self envGitHubTokenAuthEnabled]) { - fprintf(stderr, "s7: subrepo network auth: HTTPS via token\n"); + if (nil != [self gitHubTokenAuthTaskEnvironment]) { + logInfo("s7: subrepo network auth: HTTPS via token\n"); } else { s7TraceGit(@"s7: subrepo network auth: SSH (default)\n"); @@ -394,13 +395,11 @@ + (int)runGitWithArguments:(NSArray *)arguments [self logSelectedAuthPathOnce]; // Inject the HTTPS auth config via the child's environment (see - // +gitHubTokenConfigEnvironment). Empty on the SSH path, so dev machines and + // +gitHubTokenAuthTaskEnvironment). nil on the SSH path, so dev machines and // any non-token use are completely unaffected (no task.environment override). - NSDictionary *const credentialEnvironment = [self gitHubTokenConfigEnvironment]; - if (credentialEnvironment.count > 0) { - NSMutableDictionary *const environment = [NSProcessInfo.processInfo.environment mutableCopy]; - [environment addEntriesFromDictionary:credentialEnvironment]; - task.environment = environment; + NSDictionary *const environmentWithAuth = [self gitHubTokenAuthTaskEnvironment]; + if (nil != environmentWithAuth) { + task.environment = environmentWithAuth; } // https://stackoverflow.com/questions/49184623/nstask-race-condition-with-readabilityhandler-block @@ -1535,9 +1534,9 @@ + (void)setTestRepoConfigureOnInitBlock:(void (^)(GitRepository * _Nonnull))test _testRepoConfigureOnInitBlock = testRepoConfigureOnInitBlock; } -// Pure builder for +gitHubTokenConfigEnvironment. Kept free of process-env reads -// so tests can probe arbitrary inputs. Returns @{} when either credential is -// missing. +// Pure builder for +gitHubTokenAuthTaskEnvironment. The process environment +// comes in as a parameter so tests can probe arbitrary inputs. Returns @{} +// when either credential is missing. // // The PAT travels in an HTTP Authorization header, never in a URL: // * insteadOf rewrites every SSH github.com shape to the CLEAN (credential- @@ -1547,38 +1546,34 @@ + (void)setTestRepoConfigureOnInitBlock:(void (^)(GitRepository * _Nonnull))test // token)" — the same mechanism actions/checkout uses. // base64 swallows arbitrary token bytes, so no percent-encoding is needed. // Delivered via GIT_CONFIG_* env: off-disk, off-argv. New entries append past -// existingConfigCount so a nested s7 (which inherits the parent's injected -// GIT_CONFIG_COUNT) doesn't clobber it. +// any existing GIT_CONFIG_COUNT so a nested s7 (which inherits the parent's +// injected GIT_CONFIG_COUNT) doesn't clobber it. + (NSDictionary *)gitHubTokenConfigEnvironmentForUser:(nullable NSString *)user token:(nullable NSString *)token - existingConfigCount:(NSInteger)existingConfigCount + processEnvironment:(NSDictionary *)processEnvironment { if (0 == user.length || 0 == token.length) { return @{}; } - NSString *const userColonToken = [NSString stringWithFormat:@"%@:%@", user, token]; - NSString *const basic = [[userColonToken dataUsingEncoding:NSUTF8StringEncoding] base64EncodedStringWithOptions:0]; + NSMutableDictionary *const result = [NSMutableDictionary new]; + __block NSUInteger nextConfigPairIndex = (NSUInteger)MAX(0, [processEnvironment[@"GIT_CONFIG_COUNT"] integerValue]); + __auto_type addConfigKV = ^ void (NSString *key, NSString *value) { + result[[NSString stringWithFormat:@"GIT_CONFIG_KEY_%lu", (unsigned long)nextConfigPairIndex]] = key; + result[[NSString stringWithFormat:@"GIT_CONFIG_VALUE_%lu", (unsigned long)nextConfigPairIndex]] = value; + ++nextConfigPairIndex; + }; - NSArray *const sshSources = @[ - @"git@github.com:", - @"ssh://git@github.com/", - ]; + // url..insteadOf is multi-valued: each index contributes one rule. + addConfigKV(@"url.https://github.com/.insteadOf", @"git@github.com:"); + addConfigKV(@"url.https://github.com/.insteadOf", @"ssh://git@github.com/"); - NSMutableDictionary *const result = [NSMutableDictionary new]; - NSInteger index = MAX(0, existingConfigCount); - for (NSString *source in sshSources) { - // url..insteadOf is multi-valued: each index contributes one rule. - result[[NSString stringWithFormat:@"GIT_CONFIG_KEY_%ld", (long)index]] = @"url.https://github.com/.insteadOf"; - result[[NSString stringWithFormat:@"GIT_CONFIG_VALUE_%ld", (long)index]] = source; - ++index; - } + NSString *const userColonToken = [NSString stringWithFormat:@"%@:%@", user, token]; + NSString *const basic = [[userColonToken dataUsingEncoding:NSUTF8StringEncoding] base64EncodedStringWithOptions:0]; // Scoped to github.com so the header is never attached to any other host. - result[[NSString stringWithFormat:@"GIT_CONFIG_KEY_%ld", (long)index]] = @"http.https://github.com/.extraheader"; - result[[NSString stringWithFormat:@"GIT_CONFIG_VALUE_%ld", (long)index]] = [NSString stringWithFormat:@"Authorization: Basic %@", basic]; - ++index; + addConfigKV(@"http.https://github.com/.extraheader", [NSString stringWithFormat:@"Authorization: Basic %@", basic]); - result[@"GIT_CONFIG_COUNT"] = [NSString stringWithFormat:@"%ld", (long)index]; + result[@"GIT_CONFIG_COUNT"] = [NSString stringWithFormat:@"%lu", (unsigned long)nextConfigPairIndex]; return result; } diff --git a/system7/main.m b/system7/main.m index 09e7c99..196913e 100644 --- a/system7/main.m +++ b/system7/main.m @@ -96,6 +96,16 @@ void printHelp(void) { help_puts(" When provided, forces the merge driver to use a strategy that resolves conflicts in subrepo"); help_puts(" references by favoring the branch specified in the variable, as well as retargeting \"their\""); help_puts(" added subrepos to the branch in question."); + help_puts(""); + help_puts(" S7_GIT_USER / S7_GIT_TOKEN"); + help_puts(" Make s7 authenticate github.com subrepo network operations (clone, fetch,"); + help_puts(" push) over HTTPS with the given username and Personal Access Token instead"); + help_puts(" of SSH. When both variables are set, s7 rewrites SSH github.com subrepo"); + help_puts(" URLs to HTTPS and passes the token to git in an HTTP header; the token is"); + help_puts(" never written to disk and never appears in process arguments. When either"); + help_puts(" variable is missing, s7 leaves git alone (SSH keys as usual)."); + help_puts(" GH_USER / GH_TOKEN are honored as a fallback when the S7_* variables are"); + help_puts(" not set."); } Class commandClassByName(NSString *commandName) { From 027fc053e737859e27914f26ea4949a2b70d1452 Mon Sep 17 00:00:00 2001 From: Oleksandr Hleba Date: Tue, 7 Jul 2026 16:02:47 +0200 Subject: [PATCH 3/3] Updates after code review --- system7-tests/gitGitHubTokenAuthTests.m | 39 ++++++++++++++++--------- system7/git/Git+Tests.h | 6 ++-- system7/git/Git.m | 38 ++++++++---------------- 3 files changed, 41 insertions(+), 42 deletions(-) diff --git a/system7-tests/gitGitHubTokenAuthTests.m b/system7-tests/gitGitHubTokenAuthTests.m index 6c71439..f980371 100644 --- a/system7-tests/gitGitHubTokenAuthTests.m +++ b/system7-tests/gitGitHubTokenAuthTests.m @@ -17,25 +17,25 @@ @implementation gitGitHubTokenAuthTests #pragma mark - GIT_CONFIG_* env builder - -- (void)testReturnsEmptyWhenUserNil { - XCTAssertEqualObjects(@{}, [GitRepository gitHubTokenConfigEnvironmentForUser:nil token:@"abc" processEnvironment:@{}]); +- (void)testReturnsNilWhenUserNil { + XCTAssertNil([GitRepository gitHubTokenAuthTaskEnvironmentForUser:nil token:@"abc" processEnvironment:@{}]); } -- (void)testReturnsEmptyWhenUserEmpty { - XCTAssertEqualObjects(@{}, [GitRepository gitHubTokenConfigEnvironmentForUser:@"" token:@"abc" processEnvironment:@{}]); +- (void)testReturnsNilWhenUserEmpty { + XCTAssertNil([GitRepository gitHubTokenAuthTaskEnvironmentForUser:@"" token:@"abc" processEnvironment:@{}]); } -- (void)testReturnsEmptyWhenTokenNil { - XCTAssertEqualObjects(@{}, [GitRepository gitHubTokenConfigEnvironmentForUser:@"alice" token:nil processEnvironment:@{}]); +- (void)testReturnsNilWhenTokenNil { + XCTAssertNil([GitRepository gitHubTokenAuthTaskEnvironmentForUser:@"alice" token:nil processEnvironment:@{}]); } -- (void)testReturnsEmptyWhenTokenEmpty { - XCTAssertEqualObjects(@{}, [GitRepository gitHubTokenConfigEnvironmentForUser:@"alice" token:@"" processEnvironment:@{}]); +- (void)testReturnsNilWhenTokenEmpty { + XCTAssertNil([GitRepository gitHubTokenAuthTaskEnvironmentForUser:@"alice" token:@"" processEnvironment:@{}]); } - (void)testBuildsHeaderAuthEntriesFromZero { NSDictionary *const env = - [GitRepository gitHubTokenConfigEnvironmentForUser:@"alice" token:@"abc123" processEnvironment:@{}]; + [GitRepository gitHubTokenAuthTaskEnvironmentForUser:@"alice" token:@"abc123" processEnvironment:@{}]; // base64("alice:abc123") == "YWxpY2U6YWJjMTIz" NSDictionary *const expected = @{ @@ -54,7 +54,7 @@ - (void)testAppendsPastExistingConfigCount { // A nested s7 inherits the parent's injected GIT_CONFIG_COUNT=3 and must not // clobber entries 0..2. NSDictionary *const env = - [GitRepository gitHubTokenConfigEnvironmentForUser:@"alice" token:@"abc" processEnvironment:@{@"GIT_CONFIG_COUNT": @"3"}]; + [GitRepository gitHubTokenAuthTaskEnvironmentForUser:@"alice" token:@"abc" processEnvironment:@{@"GIT_CONFIG_COUNT": @"3"}]; XCTAssertEqualObjects(@"6", env[@"GIT_CONFIG_COUNT"]); XCTAssertEqualObjects(@"url.https://github.com/.insteadOf", env[@"GIT_CONFIG_KEY_3"]); @@ -70,17 +70,30 @@ - (void)testAppendsPastExistingConfigCount { - (void)testNegativeOrGarbageExistingCountClampsToZero { NSDictionary *const env = - [GitRepository gitHubTokenConfigEnvironmentForUser:@"alice" token:@"abc" processEnvironment:@{@"GIT_CONFIG_COUNT": @"-5"}]; + [GitRepository gitHubTokenAuthTaskEnvironmentForUser:@"alice" token:@"abc" processEnvironment:@{@"GIT_CONFIG_COUNT": @"-5"}]; XCTAssertEqualObjects(@"3", env[@"GIT_CONFIG_COUNT"]); XCTAssertEqualObjects(@"url.https://github.com/.insteadOf", env[@"GIT_CONFIG_KEY_0"]); } +- (void)testInheritedEnvironmentPassesThrough { + // The returned dictionary is the COMPLETE child environment: everything the + // process already had, plus the auth entries. + NSDictionary *const env = + [GitRepository gitHubTokenAuthTaskEnvironmentForUser:@"alice" + token:@"abc" + processEnvironment:@{@"HOME": @"/Users/alice", @"GIT_CONFIG_COUNT": @"1"}]; + + XCTAssertEqualObjects(@"/Users/alice", env[@"HOME"]); + XCTAssertEqualObjects(@"4", env[@"GIT_CONFIG_COUNT"]); + XCTAssertEqualObjects(@"url.https://github.com/.insteadOf", env[@"GIT_CONFIG_KEY_1"]); +} + - (void)testTokenNeverAppearsRawOnlyInBase64Header { NSString *const user = @"alice"; NSString *const token = @"ghp_SuperSecret/@:%123"; // chars that would have needed URL-escaping NSDictionary *const env = - [GitRepository gitHubTokenConfigEnvironmentForUser:user token:token processEnvironment:@{}]; + [GitRepository gitHubTokenAuthTaskEnvironmentForUser:user token:token processEnvironment:@{}]; NSString *const expectedBasic = [[[NSString stringWithFormat:@"%@:%@", user, token] dataUsingEncoding:NSUTF8StringEncoding] @@ -96,7 +109,7 @@ - (void)testTokenNeverAppearsRawOnlyInBase64Header { - (void)testGithubDotComOnly { NSDictionary *const env = - [GitRepository gitHubTokenConfigEnvironmentForUser:@"alice" token:@"abc" processEnvironment:@{}]; + [GitRepository gitHubTokenAuthTaskEnvironmentForUser:@"alice" token:@"abc" processEnvironment:@{}]; NSString *const joined = [[env.allKeys arrayByAddingObjectsFromArray:env.allValues] componentsJoinedByString:@" "]; XCTAssertFalse([joined containsString:@"gitlab"]); diff --git a/system7/git/Git+Tests.h b/system7/git/Git+Tests.h index 322ab7c..a0fd537 100644 --- a/system7/git/Git+Tests.h +++ b/system7/git/Git+Tests.h @@ -20,9 +20,9 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, class) void (^testRepoConfigureOnInitBlock)(GitRepository *repo); @property (nonatomic, readonly) BOOL hasMergeConflict; -+ (NSDictionary *)gitHubTokenConfigEnvironmentForUser:(nullable NSString *)user - token:(nullable NSString *)token - processEnvironment:(NSDictionary *)processEnvironment; ++ (nullable NSDictionary *)gitHubTokenAuthTaskEnvironmentForUser:(nullable NSString *)user + token:(nullable NSString *)token + processEnvironment:(NSDictionary *)processEnvironment; @end diff --git a/system7/git/Git.m b/system7/git/Git.m index fb8d8df..c557e32 100644 --- a/system7/git/Git.m +++ b/system7/git/Git.m @@ -80,30 +80,16 @@ + (nullable NSString *)envGitAuthToken { return ghToken.length > 0 ? ghToken : nil; } -// Full environment for the spawned git process when GitHub HTTPS token auth -// is enabled (both credentials present); nil when it's off. Computed once: the -// process environment plus GIT_CONFIG_* entries that authenticate github.com -// over HTTPS — insteadOf rewrites SSH URLs to the clean https URL and an -// http..extraheader carries the PAT as a Basic-auth header (see -// +gitHubTokenConfigEnvironmentForUser:token:processEnvironment:). -// Delivered through the child's environment rather than `-c` argv, so the token -// never appears in the process argument list (`ps`, /proc, crash reporters), and -// — riding in a header, not a URL — nothing git echoes can leak it. `.git/config` -// keeps the original SSH URL, so the token never lands on disk either. +// Cached-once front for +gitHubTokenAuthTaskEnvironmentForUser:token:processEnvironment:, +// resolved against this process's credentials and environment. nil when token +// auth is off (either credential missing). + (nullable NSDictionary *)gitHubTokenAuthTaskEnvironment { static dispatch_once_t onceToken; - static NSDictionary *taskEnvironment; + static NSDictionary *taskEnvironment = nil; dispatch_once(&onceToken, ^{ - NSDictionary *const env = NSProcessInfo.processInfo.environment; - NSDictionary *const configEnvironment = - [self gitHubTokenConfigEnvironmentForUser:[self envGitAuthUser] - token:[self envGitAuthToken] - processEnvironment:env]; - if (configEnvironment.count > 0) { - NSMutableDictionary *const merged = [env mutableCopy]; - [merged addEntriesFromDictionary:configEnvironment]; - taskEnvironment = [merged copy]; - } + taskEnvironment = [self gitHubTokenAuthTaskEnvironmentForUser:[self envGitAuthUser] + token:[self envGitAuthToken] + processEnvironment:NSProcessInfo.processInfo.environment]; }); return taskEnvironment; } @@ -1548,15 +1534,15 @@ + (void)setTestRepoConfigureOnInitBlock:(void (^)(GitRepository * _Nonnull))test // Delivered via GIT_CONFIG_* env: off-disk, off-argv. New entries append past // any existing GIT_CONFIG_COUNT so a nested s7 (which inherits the parent's // injected GIT_CONFIG_COUNT) doesn't clobber it. -+ (NSDictionary *)gitHubTokenConfigEnvironmentForUser:(nullable NSString *)user - token:(nullable NSString *)token - processEnvironment:(NSDictionary *)processEnvironment ++ (nullable NSDictionary *)gitHubTokenAuthTaskEnvironmentForUser:(nullable NSString *)user + token:(nullable NSString *)token + processEnvironment:(NSDictionary *)processEnvironment { if (0 == user.length || 0 == token.length) { - return @{}; + return nil; } - NSMutableDictionary *const result = [NSMutableDictionary new]; + NSMutableDictionary *const result = [processEnvironment mutableCopy]; __block NSUInteger nextConfigPairIndex = (NSUInteger)MAX(0, [processEnvironment[@"GIT_CONFIG_COUNT"] integerValue]); __auto_type addConfigKV = ^ void (NSString *key, NSString *value) { result[[NSString stringWithFormat:@"GIT_CONFIG_KEY_%lu", (unsigned long)nextConfigPairIndex]] = key;