-
Notifications
You must be signed in to change notification settings - Fork 0
[EXP-23262]: S7: support subrepo cloning via GH_USER / GH_TOKEN #92
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,106 @@ | ||
| // | ||
| // 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" processEnvironment:@{}]); | ||
| } | ||
|
|
||
| - (void)testReturnsEmptyWhenUserEmpty { | ||
| XCTAssertEqualObjects(@{}, [GitRepository gitHubTokenConfigEnvironmentForUser:@"" token:@"abc" processEnvironment:@{}]); | ||
| } | ||
|
|
||
| - (void)testReturnsEmptyWhenTokenNil { | ||
| XCTAssertEqualObjects(@{}, [GitRepository gitHubTokenConfigEnvironmentForUser:@"alice" token:nil processEnvironment:@{}]); | ||
| } | ||
|
|
||
| - (void)testReturnsEmptyWhenTokenEmpty { | ||
| XCTAssertEqualObjects(@{}, [GitRepository gitHubTokenConfigEnvironmentForUser:@"alice" token:@"" processEnvironment:@{}]); | ||
| } | ||
|
|
||
| - (void)testBuildsHeaderAuthEntriesFromZero { | ||
| NSDictionary<NSString *, NSString *> *const env = | ||
| [GitRepository gitHubTokenConfigEnvironmentForUser:@"alice" token:@"abc123" processEnvironment:@{}]; | ||
|
|
||
| // 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" processEnvironment:@{@"GIT_CONFIG_COUNT": @"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)testNegativeOrGarbageExistingCountClampsToZero { | ||
| NSDictionary<NSString *, NSString *> *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<NSString *, NSString *> *const env = | ||
| [GitRepository gitHubTokenConfigEnvironmentForUser:user token:token processEnvironment:@{}]; | ||
|
|
||
| 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" processEnvironment:@{}]; | ||
|
|
||
| NSString *const joined = [[env.allKeys arrayByAddingObjectsFromArray:env.allValues] componentsJoinedByString:@" "]; | ||
| XCTAssertFalse([joined containsString:@"gitlab"]); | ||
| XCTAssertFalse([joined containsString:@"bitbucket"]); | ||
| } | ||
|
|
||
| @end |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -60,6 +60,66 @@ + (BOOL)envGitTraceEnabled { | |||||
| return traceEnabled; | ||||||
| } | ||||||
|
|
||||||
| + (nullable NSString *)envGitAuthUser { | ||||||
| NSDictionary<NSString *, NSString *> *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; | ||||||
| } | ||||||
|
|
||||||
| + (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; | ||||||
| } | ||||||
|
|
||||||
| // 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.<url>.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. | ||||||
| + (nullable NSDictionary<NSString *, NSString *> *)gitHubTokenAuthTaskEnvironment { | ||||||
| static dispatch_once_t onceToken; | ||||||
| static NSDictionary<NSString *, NSString *> *taskEnvironment; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm always scared by uninitialised variable. Even the static ones.
Suggested change
|
||||||
| dispatch_once(&onceToken, ^{ | ||||||
| NSDictionary<NSString *, NSString *> *const env = NSProcessInfo.processInfo.environment; | ||||||
| NSDictionary<NSString *, NSString *> *const configEnvironment = | ||||||
| [self gitHubTokenConfigEnvironmentForUser:[self envGitAuthUser] | ||||||
| token:[self envGitAuthToken] | ||||||
| processEnvironment:env]; | ||||||
| if (configEnvironment.count > 0) { | ||||||
| NSMutableDictionary<NSString *, NSString *> *const merged = [env mutableCopy]; | ||||||
| [merged addEntriesFromDictionary:configEnvironment]; | ||||||
| taskEnvironment = [merged copy]; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would suggest to put all the merging logic into the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Plus, I would suggest to remove the call for I don't think we have some unscrupulous people casting NSDictionary to NSMutableDictionary around here. |
||||||
| } | ||||||
| }); | ||||||
| return taskEnvironment; | ||||||
| } | ||||||
|
|
||||||
| + (void)logSelectedAuthPathOnce { | ||||||
| static dispatch_once_t onceToken; | ||||||
| dispatch_once(&onceToken, ^{ | ||||||
| if (nil != [self gitHubTokenAuthTaskEnvironment]) { | ||||||
| logInfo("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 +392,16 @@ + (int)runGitWithArguments:(NSArray<NSString *> *)arguments | |||||
| task.currentDirectoryURL = [NSURL fileURLWithPath:currentDirectoryPath]; | ||||||
| } | ||||||
|
|
||||||
| [self logSelectedAuthPathOnce]; | ||||||
|
|
||||||
| // Inject the HTTPS auth config via the child's environment (see | ||||||
|
pastey marked this conversation as resolved.
|
||||||
| // +gitHubTokenAuthTaskEnvironment). nil on the SSH path, so dev machines and | ||||||
| // any non-token use are completely unaffected (no task.environment override). | ||||||
| NSDictionary<NSString *, NSString *> *const environmentWithAuth = [self gitHubTokenAuthTaskEnvironment]; | ||||||
| if (nil != environmentWithAuth) { | ||||||
| task.environment = environmentWithAuth; | ||||||
| } | ||||||
|
|
||||||
| // 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 +1534,49 @@ + (void)setTestRepoConfigureOnInitBlock:(void (^)(GitRepository * _Nonnull))test | |||||
| _testRepoConfigureOnInitBlock = testRepoConfigureOnInitBlock; | ||||||
| } | ||||||
|
|
||||||
| // 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- | ||||||
| // 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 | ||||||
| // any existing GIT_CONFIG_COUNT 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 | ||||||
| processEnvironment:(NSDictionary<NSString *, NSString *> *)processEnvironment | ||||||
| { | ||||||
| if (0 == user.length || 0 == token.length) { | ||||||
| return @{}; | ||||||
| } | ||||||
|
|
||||||
| NSMutableDictionary<NSString *, NSString *> *const result = [NSMutableDictionary new]; | ||||||
| __block NSUInteger nextConfigPairIndex = (NSUInteger)MAX(0, [processEnvironment[@"GIT_CONFIG_COUNT"] integerValue]); | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit:
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My bad. Thanks 👍 |
||||||
| __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; | ||||||
| }; | ||||||
|
|
||||||
| // url.<base>.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/"); | ||||||
|
|
||||||
| 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. | ||||||
| addConfigKV(@"http.https://github.com/.extraheader", [NSString stringWithFormat:@"Authorization: Basic %@", basic]); | ||||||
|
|
||||||
| result[@"GIT_CONFIG_COUNT"] = [NSString stringWithFormat:@"%lu", (unsigned long)nextConfigPairIndex]; | ||||||
| return result; | ||||||
| } | ||||||
|
|
||||||
| - (BOOL)hasMergeConflict { | ||||||
| NSString *stdOutOutput = nil; | ||||||
| const int unmergedFilesStatus = [self runGitCommand:@"ls-files -u" | ||||||
|
|
||||||
Uh oh!
There was an error while loading. Please reload this page.