From 1a2d77eec669fc3e77ea7cff87ea14c4bc35552a Mon Sep 17 00:00:00 2001 From: Oleksandr Svirets Date: Tue, 26 May 2026 17:43:43 +0300 Subject: [PATCH] EXP-22322: Support GH_USER/GH_TOKEN env vars to clone GitHub subrepos over HTTPS --- system7-tests/gitGitHubTokenAuthTests.m | 139 ++++++++++++++++++ .../case-cloneViaGitHubTokenAuth.sh | 93 ++++++++++++ system7.xcodeproj/project.pbxproj | 4 + system7/git/Git+Tests.h | 15 ++ system7/git/Git.m | 108 +++++++++++++- 5 files changed, 356 insertions(+), 3 deletions(-) create mode 100644 system7-tests/gitGitHubTokenAuthTests.m create mode 100755 system7-tests/integration/case-cloneViaGitHubTokenAuth.sh diff --git a/system7-tests/gitGitHubTokenAuthTests.m b/system7-tests/gitGitHubTokenAuthTests.m new file mode 100644 index 0000000..016ab7f --- /dev/null +++ b/system7-tests/gitGitHubTokenAuthTests.m @@ -0,0 +1,139 @@ +// +// 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 - argv builder - + +- (void)testReturnsEmptyWhenUserNil { + XCTAssertEqualObjects(@[], + [GitRepository gitHubTokenInsteadOfArgumentsForUser:nil token:@"abc"]); +} + +- (void)testReturnsEmptyWhenUserEmpty { + XCTAssertEqualObjects(@[], + [GitRepository gitHubTokenInsteadOfArgumentsForUser:@"" token:@"abc"]); +} + +- (void)testReturnsEmptyWhenTokenNil { + XCTAssertEqualObjects(@[], + [GitRepository gitHubTokenInsteadOfArgumentsForUser:@"alice" token:nil]); +} + +- (void)testReturnsEmptyWhenTokenEmpty { + XCTAssertEqualObjects(@[], + [GitRepository gitHubTokenInsteadOfArgumentsForUser:@"alice" token:@""]); +} + +- (void)testBuildsTwoInsteadOfPairsForSimpleCreds { + NSArray *const args = [GitRepository gitHubTokenInsteadOfArgumentsForUser:@"alice" + token:@"abc123"]; + + NSArray *const expected = @[ + @"-c", @"url.https://alice:abc123@github.com/.insteadOf=git@github.com:", + @"-c", @"url.https://alice:abc123@github.com/.insteadOf=ssh://git@github.com/", + ]; + XCTAssertEqualObjects(expected, args); +} + +- (void)testPercentEncodesSpecialCharsInToken { + // Token with every char that would break URL parsing if left raw. + NSString *const token = @"a/b@c:d%e#f g"; + NSArray *const args = [GitRepository gitHubTokenInsteadOfArgumentsForUser:@"alice" + token:token]; + + XCTAssertEqual((NSUInteger)4, args.count); + + NSString *const joined = [args componentsJoinedByString:@" "]; + // Each special character must be percent-encoded inside the userinfo. + XCTAssertTrue([joined containsString:@"a%2Fb%40c%3Ad%25e%23f%20g"], + @"token chars must be percent-encoded; joined argv was: %@", joined); + + // And the raw forms with the original separators must NOT leak through — + // a raw `@` after the token would terminate the userinfo prematurely. + XCTAssertFalse([joined containsString:@"a/b@c:d%e#f g"]); +} + +- (void)testPercentEncodesSpecialCharsInUser { + NSArray *const args = [GitRepository gitHubTokenInsteadOfArgumentsForUser:@"al ice@org" + token:@"abc"]; + + NSString *const joined = [args componentsJoinedByString:@" "]; + XCTAssertTrue([joined containsString:@"al%20ice%40org:abc@github.com/"], + @"user chars must be percent-encoded; joined argv was: %@", joined); +} + +- (void)testInsteadOfBaseUsesHTTPSAndGithubDotComOnly { + NSArray *const args = [GitRepository gitHubTokenInsteadOfArgumentsForUser:@"alice" + token:@"abc"]; + + NSString *const joined = [args componentsJoinedByString:@" "]; + XCTAssertTrue([joined containsString:@".insteadOf=git@github.com:"]); + XCTAssertTrue([joined containsString:@".insteadOf=ssh://git@github.com/"]); + XCTAssertTrue([joined containsString:@"url.https://alice:abc@github.com/"]); + + // No other hosts referenced — every "github.com" should be the only host. + XCTAssertFalse([joined containsString:@"gitlab"]); + XCTAssertFalse([joined containsString:@"bitbucket"]); +} + +#pragma mark - trace masking - + +- (void)testTraceMaskReplacesEncodedTokenSubstring { + NSString *const line = @"-c url.https://alice:abc123@github.com/.insteadOf=git@github.com: " + "clone git@github.com:readdle/RDPDFKit.git dest"; + + NSString *const masked = [GitRepository maskedTraceLine:line forToken:@"abc123"]; + + XCTAssertFalse([masked containsString:@"abc123"], @"raw token leaked: %@", masked); + XCTAssertTrue([masked containsString:@"https://alice:***@github.com/"]); + // The non-credential parts of the command line must be preserved. + XCTAssertTrue([masked containsString:@"git@github.com:readdle/RDPDFKit.git"]); + XCTAssertTrue([masked containsString:@"clone"]); +} + +- (void)testTraceMaskAlsoRedactsTokenWithSpecialChars { + // The encoded form is what actually appears in the argv; the raw form + // is masked as defense-in-depth in case git echoes it back somewhere. + NSString *const token = @"a/b@c"; + NSString *const line = @"-c url.https://alice:a%2Fb%40c@github.com/.insteadOf=git@github.com:"; + + NSString *const masked = [GitRepository maskedTraceLine:line forToken:token]; + + XCTAssertFalse([masked containsString:@"a%2Fb%40c"], @"encoded token leaked: %@", masked); + XCTAssertTrue([masked containsString:@"https://alice:***@github.com/"]); +} + +- (void)testTraceMaskNoOpWhenTokenEmpty { + NSString *const line = @"clone git@github.com:readdle/RDPDFKit.git dest"; + + XCTAssertEqualObjects(line, [GitRepository maskedTraceLine:line forToken:nil]); + XCTAssertEqualObjects(line, [GitRepository maskedTraceLine:line forToken:@""]); +} + +- (void)testTraceMaskAlsoRedactsLongToken { + // Verifies that masking doesn't depend on token length — `***` is applied + // regardless. (Earlier iterations partial-revealed long tokens; this guards + // against that regression.) + NSString *const token = @"ghp_AbCdEfGhIjKlMnOpQrStUvWxYz0123456789"; // 40 chars + NSString *const line = [NSString stringWithFormat:@"https://alice:%@@github.com/", token]; + + NSString *const masked = [GitRepository maskedTraceLine:line forToken:token]; + + XCTAssertTrue([masked containsString:@"https://alice:***@github.com/"], @"got: %@", masked); + XCTAssertFalse([masked containsString:token], @"raw token leaked: %@", masked); +} + +@end diff --git a/system7-tests/integration/case-cloneViaGitHubTokenAuth.sh b/system7-tests/integration/case-cloneViaGitHubTokenAuth.sh new file mode 100755 index 0000000..0fb2be1 --- /dev/null +++ b/system7-tests/integration/case-cloneViaGitHubTokenAuth.sh @@ -0,0 +1,93 @@ +#!/bin/sh + +# Integration test for the GH_USER/GH_TOKEN PAT auth mode. +# +# What the production change does: when GH_USER and GH_TOKEN are both set, +# s7 prepends `-c url.https://USER:TOKEN@github.com/.insteadOf=git@github.com:` +# (and the ssh:// variant) to every git invocation. This rewrites SSH GitHub +# URLs to HTTPS-with-token at network-resolution time, without touching +# .s7substate or persisting the token to any cloned subrepo's .git/config. +# +# Verified here, end-to-end: +# A. Without the env vars: no `-c url…insteadOf…` flags are injected. +# B. With the env vars: the flags appear on every git call, and the +# token is masked (`***`) in S7_TRACE_GIT output. +# C. The token does not leak to any file under the cloned subrepo's .git/. +# D. Real subrepo URLs (non-github.com) are untouched — the clone still +# succeeds and the original URL is stored verbatim in .git/config. +# +# Note: we don't drive a real `git@github.com:` clone to a fake remote here +# because git applies `insteadOf` rules with longest-prefix matching, not +# recursively, so a two-level rewrite chain can't be expressed cleanly. +# Trace inspection plus credential-leak guards prove the s7-layer behaves +# correctly; real github.com use is exercised in CI against the real PAT. + +GH_TEST_USER=testuser +GH_TEST_TOKEN=mySecretToken_xyz123 + + +# --- Set up pastey/rd2 with one subrepo (real local URL) --- + +git clone github/rd2 pastey/rd2 + +cd pastey/rd2 + +assert s7 init +assert git add . +assert git commit -m '"init s7"' + +assert s7 add --stage Dependencies/ReaddleLib '"$S7_ROOT/github/ReaddleLib"' +assert git commit -m '"add ReaddleLib subrepo"' +assert git push + + +# --- Scenario A: env unset → no insteadOf injection --- + +cd "$S7_ROOT/nik" + +unset GH_USER +unset GH_TOKEN + +S7_TRACE_GIT=1 git clone "$S7_ROOT/github/rd2" rd2-noauth 2> traceA.log + +assert test -d rd2-noauth/Dependencies/ReaddleLib +assert ! grep -q insteadOf= traceA.log +assert ! grep -q 'url\.https://' traceA.log + + +# --- Scenario B: env set → flags injected, token masked --- + +cd "$S7_ROOT/nik" + +export GH_USER="$GH_TEST_USER" +export GH_TOKEN="$GH_TEST_TOKEN" + +S7_TRACE_GIT=1 git clone "$S7_ROOT/github/rd2" rd2-auth 2> traceB.log + +# Subrepo clone must still succeed — local URLs in .s7substate are unaffected +# by the github.com-only insteadOf rule. +assert test -d rd2-auth/Dependencies/ReaddleLib + +# Both rewrite pairs must appear (covers `git@github.com:` and `ssh://git@github.com/`). +assert grep -q 'insteadOf=git@github.com:' traceB.log +assert grep -q 'insteadOf=ssh://git@github.com/' traceB.log + +# The HTTPS-with-userinfo URL must appear with the user but with the token +# replaced by `***`. +assert grep -qF 'url.https://testuser:***@github.com/' traceB.log + +# Raw token must NEVER appear in trace output. +assert ! grep -qF "$GH_TEST_TOKEN" traceB.log + + +# --- Scenario C: token does not land in any .git/config or .git/ file --- + +assert ! grep -rq "$GH_TEST_TOKEN" rd2-auth/.git/ +assert ! grep -rq "$GH_TEST_TOKEN" rd2-auth/Dependencies/ReaddleLib/.git/ + + +# --- Scenario D: subrepo's stored origin URL is the original local path --- + +SUBREPO_URL=$(git -C rd2-auth/Dependencies/ReaddleLib config remote.origin.url) +EXPECTED_URL="$S7_ROOT/github/ReaddleLib" +assert test "$SUBREPO_URL" = "$EXPECTED_URL" 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..7d8baea 100644 --- a/system7/git/Git+Tests.h +++ b/system7/git/Git+Tests.h @@ -20,6 +20,21 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, class) void (^testRepoConfigureOnInitBlock)(GitRepository *repo); @property (nonatomic, readonly) BOOL hasMergeConflict; +// Percent-encode every character that isn't RFC 3986 "unreserved". +// `%` itself is included in the escape set, so a literal `%` in a token +// becomes `%25` (avoids garbling the userinfo component of the URL). ++ (NSString *)urlEscapeUserinfo:(NSString *)input; + +// Pure builder for the `-c url..insteadOf=` argv prefix. +// Bypasses dispatch_once and process env so tests can probe arbitrary inputs. ++ (NSArray *)gitHubTokenInsteadOfArgumentsForUser:(nullable NSString *)user + token:(nullable NSString *)token; + +// Pure trace-mask helper: redacts any occurrence of `token` (both URL-encoded +// and raw) in `line` with `***`. ++ (NSString *)maskedTraceLine:(NSString *)line + forToken:(nullable NSString *)token; + @end NS_ASSUME_NONNULL_END diff --git a/system7/git/Git.m b/system7/git/Git.m index 57a3cab..f7cf0ff 100644 --- a/system7/git/Git.m +++ b/system7/git/Git.m @@ -53,13 +53,57 @@ + (NSString *)envGitExecutablePath { + (BOOL)envGitTraceEnabled { static dispatch_once_t onceToken; static BOOL traceEnabled; - + dispatch_once(&onceToken, ^{ traceEnabled = [[NSProcessInfo processInfo].environment[@"S7_TRACE_GIT"] intValue] != 0; }); return traceEnabled; } +// Gate for the GitHub PAT auth mode. Encapsulated in its own method so the +// activation policy (currently: auto when both GH_USER and GH_TOKEN are set) +// can later be swapped — e.g., to require an explicit `S7_USE_GH_TOKEN=1` — +// with a one-line change here. ++ (BOOL)envGitHubTokenAuthEnabled { + static dispatch_once_t onceToken; + static BOOL enabled; + dispatch_once(&onceToken, ^{ + NSDictionary *const env = [NSProcessInfo processInfo].environment; + enabled = env[@"GH_USER"].length > 0 && env[@"GH_TOKEN"].length > 0; + s7TraceGit(@"s7: GitHub token auth: enabled=%@\n", enabled ? @"YES" : @"NO"); + }); + return enabled; +} + +// Cached argv prefix to prepend to every `git` invocation when the auth +// gate is on. Reads GH_USER / GH_TOKEN from the process environment once. ++ (NSArray *)gitHubTokenInsteadOfArguments { + static dispatch_once_t onceToken; + static NSArray *args; + dispatch_once(&onceToken, ^{ + if (NO == [self envGitHubTokenAuthEnabled]) { + args = @[]; + return; + } + + NSDictionary *const env = [NSProcessInfo processInfo].environment; + args = [self gitHubTokenInsteadOfArgumentsForUser:env[@"GH_USER"] token:env[@"GH_TOKEN"]]; + }); + return args; +} + +// Internal trace-line builder used by `+runGitWithArguments:...`. Reads the +// PAT from the current process environment when the gate is on; otherwise +// just joins the argv as-is. ++ (NSString *)maskedTraceLineForArguments:(NSArray *)arguments { + NSString *const line = [arguments componentsJoinedByString:@" "]; + if (NO == [self envGitHubTokenAuthEnabled]) { + return line; + } + + return [self maskedTraceLine:line forToken:[NSProcessInfo processInfo].environment[@"GH_TOKEN"]]; +} + #pragma mark - Initialization - (nullable instancetype)initWithRepoPath:(NSString *)repoPath { @@ -325,9 +369,14 @@ + (int)runGitWithArguments:(NSArray *)arguments stdErrOutput:(NSString * _Nullable __autoreleasing * _Nullable)ppStdErrOutput currentDirectoryPath:(NSString * _Nullable)currentDirectoryPath { + NSArray *const credentialArguments = [self gitHubTokenInsteadOfArguments]; + NSArray *const finalArguments = credentialArguments.count > 0 + ? [credentialArguments arrayByAddingObjectsFromArray:arguments] + : arguments; + NSTask *task = [NSTask new]; [task setLaunchPath:[self envGitExecutablePath]]; - [task setArguments:arguments]; + [task setArguments:finalArguments]; if (currentDirectoryPath) { task.currentDirectoryURL = [NSURL fileURLWithPath:currentDirectoryPath]; } @@ -390,7 +439,7 @@ + (int)runGitWithArguments:(NSArray *)arguments return 1; } - s7TraceGit(@"s7: git %@\n", [task.arguments componentsJoinedByString:@" "]); + s7TraceGit(@"s7: git %@\n", [self maskedTraceLineForArguments:task.arguments]); [task waitUntilExit]; @@ -1464,6 +1513,59 @@ + (void)setTestRepoConfigureOnInitBlock:(void (^)(GitRepository * _Nonnull))test _testRepoConfigureOnInitBlock = testRepoConfigureOnInitBlock; } +#pragma mark - GitHub token auth helpers - + ++ (NSString *)urlEscapeUserinfo:(NSString *)input { + static NSCharacterSet *unreserved; + static dispatch_once_t onceToken; + dispatch_once(&onceToken, ^{ + unreserved = [NSCharacterSet characterSetWithCharactersInString: + @"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-._~"]; + }); + return [input stringByAddingPercentEncodingWithAllowedCharacters:unreserved]; +} + +// Build the `-c url..insteadOf=` argument list that, +// when injected before any `git` subcommand, transparently rewrites +// SSH-style github.com URLs (`git@github.com:` and `ssh://git@github.com/`) +// to HTTPS+PAT URLs. The rewrite happens only at network-resolution time — +// the URL stored in `.git/config` after `clone` remains the original SSH form, +// so the token never lands on disk. +// +// Returns an empty array when either credential is missing. ++ (NSArray *)gitHubTokenInsteadOfArgumentsForUser:(nullable NSString *)user + token:(nullable NSString *)token +{ + if (0 == user.length || 0 == token.length) { + return @[]; + } + + NSString *const httpsBase = [NSString stringWithFormat:@"https://%@:%@@github.com/", + [self urlEscapeUserinfo:user], + [self urlEscapeUserinfo:token]]; + return @[ + @"-c", [NSString stringWithFormat:@"url.%@.insteadOf=git@github.com:", httpsBase], + @"-c", [NSString stringWithFormat:@"url.%@.insteadOf=ssh://git@github.com/", httpsBase], + ]; +} + +// Redact the PAT from a trace line so it doesn't leak into CI logs. +// Replaces both the URL-encoded form (what actually appears in the argv) +// and the raw form (defense-in-depth) with `***`. ++ (NSString *)maskedTraceLine:(NSString *)line forToken:(nullable NSString *)token { + if (0 == token.length || 0 == line.length) { + return line; + } + + NSString *const encoded = [self urlEscapeUserinfo:token]; + NSString *masked = [line stringByReplacingOccurrencesOfString:encoded withString:@"***"]; + if (NO == [encoded isEqualToString:token]) { + masked = [masked stringByReplacingOccurrencesOfString:token withString:@"***"]; + } + + return masked; +} + - (BOOL)hasMergeConflict { NSString *stdOutOutput = nil; const int unmergedFilesStatus = [self runGitCommand:@"ls-files -u"