Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 98 additions & 0 deletions system7-tests/gitGitHubTokenAuthTests.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
//
// gitGitHubTokenAuthTests.m
// system7-tests
//
// Copyright © 2026 Readdle. All rights reserved.
//

#import <XCTest/XCTest.h>

#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<NSString *, NSString *> *const env =
[GitRepository gitHubTokenConfigEnvironmentForUser:@"alice" token:@"abc123" existingConfigCount:0];

// base64("alice:abc123") == "YWxpY2U6YWJjMTIz"
NSDictionary<NSString *, NSString *> *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<NSString *, NSString *> *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<NSString *, NSString *> *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<NSString *, NSString *> *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
4 changes: 4 additions & 0 deletions system7.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -215,6 +216,7 @@
BEE928A02456DA6500BD6B86 /* Git.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = Git.m; sourceTree = "<group>"; };
BEFAF7A025718AB7000D90C3 /* bootstrapTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = bootstrapTests.m; sourceTree = "<group>"; };
CE5BB61E25FA63A8002596B9 /* gitPackedRefsTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = gitPackedRefsTests.m; sourceTree = "<group>"; };
A11C0CE526052600A0010002 /* gitGitHubTokenAuthTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = gitGitHubTokenAuthTests.m; sourceTree = "<group>"; };
/* End PBXFileReference section */

/* Begin PBXFrameworksBuildPhase section */
Expand Down Expand Up @@ -351,6 +353,7 @@
BEE672CE2523B76500DB09BC /* iniParserTests.m */,
BEBE40C32576D04D00E39755 /* deinitTests.m */,
CE5BB61E25FA63A8002596B9 /* gitPackedRefsTests.m */,
A11C0CE526052600A0010002 /* gitGitHubTokenAuthTests.m */,
);
path = "system7-tests";
sourceTree = "<group>";
Expand Down Expand Up @@ -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 */,
Expand Down
4 changes: 4 additions & 0 deletions system7/git/Git+Tests.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ NS_ASSUME_NONNULL_BEGIN
@property (nonatomic, class) void (^testRepoConfigureOnInitBlock)(GitRepository *repo);
@property (nonatomic, readonly) BOOL hasMergeConflict;

+ (NSDictionary<NSString *, NSString *> *)gitHubTokenConfigEnvironmentForUser:(nullable NSString *)user
token:(nullable NSString *)token
existingConfigCount:(NSInteger)existingConfigCount;

@end

NS_ASSUME_NONNULL_END
118 changes: 118 additions & 0 deletions system7/git/Git.m
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,65 @@ + (BOOL)envGitTraceEnabled {
return traceEnabled;
}

// Username for HTTPS subrepo auth. S7_GIT_TOKEN, falling back to GH_TOKEN.
Comment thread
pastey marked this conversation as resolved.
Outdated
+ (nullable NSString *)envGitAuthUser {
NSDictionary<NSString *, NSString *> *const env = NSProcessInfo.processInfo.environment;
NSString *const user = env[@"S7_GIT_USER"];
Comment thread
pastey marked this conversation as resolved.
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<NSString *, NSString *> *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.<url>.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<NSString *, NSString *> *)gitHubTokenConfigEnvironment {
static dispatch_once_t onceToken;
static NSDictionary<NSString *, NSString *> *configEnvironment;
dispatch_once(&onceToken, ^{
NSDictionary<NSString *, NSString *> *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]) {
Comment thread
pastey marked this conversation as resolved.
Outdated
fprintf(stderr, "s7: subrepo network auth: HTTPS via token\n");
Comment thread
pastey marked this conversation as resolved.
Outdated
}
else {
s7TraceGit(@"s7: subrepo network auth: SSH (default)\n");
}
});
}

#pragma mark - Initialization

- (nullable instancetype)initWithRepoPath:(NSString *)repoPath {
Expand Down Expand Up @@ -332,6 +391,18 @@ + (int)runGitWithArguments:(NSArray<NSString *> *)arguments
task.currentDirectoryURL = [NSURL fileURLWithPath:currentDirectoryPath];
}

[self logSelectedAuthPathOnce];

// Inject the HTTPS auth config via the child's environment (see
Comment thread
pastey marked this conversation as resolved.
// +gitHubTokenConfigEnvironment). Empty on the SSH path, so dev machines and
// any non-token use are completely unaffected (no task.environment override).
NSDictionary<NSString *, NSString *> *const credentialEnvironment = [self gitHubTokenConfigEnvironment];
if (credentialEnvironment.count > 0) {
NSMutableDictionary<NSString *, NSString *> *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);
Expand Down Expand Up @@ -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.<url>.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<NSString *, NSString *> *)gitHubTokenConfigEnvironmentForUser:(nullable NSString *)user
token:(nullable NSString *)token
existingConfigCount:(NSInteger)existingConfigCount

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of passing existingConfigCount argument, I would directly access the

NSDictionary<NSString *, NSString *> *const env = NSProcessInfo.processInfo.environment;
        const NSInteger existingConfigCount = MAX(0, [env[@"GIT_CONFIG_COUNT"] integerValue]);

here. This way all the logic managing GIT_CONFIG_COUNT would be in a single place. Now it's spread out by two methods.
Plus NSInteger seems odd. Why not unsigned?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid point. Fixed with one small deviation: reading NSProcessInfo inside the builder would kill the tests, so instead the builder takes the environment as a parameter.

{
if (0 == user.length || 0 == token.length) {
return @{};
}

NSString *const userColonToken = [NSString stringWithFormat:@"%@:%@", user, token];
NSString *const basic = [[userColonToken dataUsingEncoding:NSUTF8StringEncoding] base64EncodedStringWithOptions:0];
Comment thread
pastey marked this conversation as resolved.
Outdated

NSArray<NSString *> *const sshSources = @[
@"git@github.com:",
@"ssh://git@github.com/",
];

NSMutableDictionary<NSString *, NSString *> *const result = [NSMutableDictionary new];
NSInteger index = MAX(0, existingConfigCount);
for (NSString *source in sshSources) {
// url.<base>.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;
Comment thread
pastey marked this conversation as resolved.
Outdated
}
// 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"
Expand Down
Loading