EXP-22322: Support GH_USER/GH_TOKEN env vars to clone GitHub subrepos over HTTPS#91
Open
Piranja wants to merge 1 commit into
Open
EXP-22322: Support GH_USER/GH_TOKEN env vars to clone GitHub subrepos over HTTPS#91Piranja wants to merge 1 commit into
Piranja wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ticket link
EXP-22322
PR description
In CI we use a GitHub PAT that only works over HTTPS, but
.s7substatelists every subrepo with an SSH URL (
[email protected]:readdle/...)because the same file is used locally with developers' personal SSH
keys. We do not want to rewrite
.s7substate.This PR adds an auto-activated auth mode: when both
GH_USERandGH_TOKENare set in the environment, s7 transparently uses HTTPS+PATfor every git network operation, while leaving
.s7substateand thecloned subrepos'
.git/configuntouched. Works recursively throughnested s7 subrepos (e.g., RDPDFKit → TesseractOCR) since they inherit
the env vars.
Implementation uses git's own
url.<base>.insteadOfconfig injectedvia
-cflags on every git invocation. Concretely, s7 prepends:-c url.https://USER:[email protected]/[email protected]:
-c url.https://USER:[email protected]/.insteadOf=ssh://[email protected]/
at the single chokepoint
+[GitRepository runGitWithArguments:stdOutOutput:stdErrOutput:currentDirectoryPath:]in
Git.m. Both SSH URL shapes ([email protected]:…andssh://[email protected]/…) are covered. Git applies the rewrite only atnetwork-resolution time — the URL stored in
.git/configafterclonestays as the original SSH form, so the token never lands on disk and
existing URL-comparison logic (
S7PostCheckoutHook.m:checkSubrepoUrlChanged)is unaffected.
Why centralize at the chokepoint rather than rewrite in
.s7substateor at the clone call sites:
.s7substatemust keep SSH URLs for local SSH-key users.+runGitWithArguments:..., so one injection covers everything.+executeCommand:(used only forgit initand localgit merge --no-edit) doesn't touch remote URLs, so it doesn't needthe rewrite.
Other design choices worth flagging:
+envGitHubTokenAuthEnabled,currently auto-on when both env vars are set. Swapping to require an
explicit
S7_USE_GH_TOKEN=1opt-in is a one-line change there..s7substate(and the nested RDPDFKit.s7substate) and theGH_*naming.
GH_USER/GH_TOKENso tokenswith
/,@,:,%, etc. don't break URL parsing.S7_TRACE_GIT=1, the token is replaced with***in the s7-printed command line. A one-line startup banners7: GitHub token auth: enabled=YES|NOmakes it easy to confirm thegate from CI logs.
Tests:
system7-tests/gitGitHubTokenAuthTests.m(12 cases)covering the argv builder, percent-encoding of special chars in both
user and token, and trace-line masking.
system7-tests/integration/case-cloneViaGitHubTokenAuth.shexercises the env-unset / env-set paths end-to-end via
S7_TRACE_GIT=1, verifies the token never appears in the clonedsubrepo's
.git/, and confirms non-github URLs are untouched.pre-existing
case-cloneS7andLFSmixedRepo.sh, which fails on thesame git-lfs hook-template drift seen before this branch (unrelated
to PAT auth).