Skip to content
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

git: set token only for main remote access #1987

Merged
merged 1 commit into from
Feb 21, 2021

Conversation

tonistiigi
Copy link
Member

fix docker/build-push-action#300

When setting the token for the request scope it to the main URL only so that it is not used for fetching irrelevant submodules. If submodules don't understand the token they can fail the fetch.

This is documented in https://git-scm.com/docs/git-config#Documentation/git-config.txt-httplturlgt

@crazy-max @MarshalX

Signed-off-by: Tonis Tiigi tonistiigi@gmail.com

@tonistiigi tonistiigi changed the title git: set token only for main remore access git: set token only for main remote access Feb 19, 2021
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Copy link
Member

@crazy-max crazy-max left a comment

Choose a reason for hiding this comment

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

LGTM

@MarshalX
Copy link

Why do we add httpurl when the prefix is not found? Why does the logic not look like this: if there is a prefix, add a auth header. Otherwise no

@tonistiigi
Copy link
Member Author

@MarshalX Token auth is not specific to github. If repo is in another location we still need to set token header. The difference is that we set the token header only for the specific repository. But in case of github we know that a token always works for other github repositories as well(including public repos) so we don't set the token for a specific repository but to any repo under github.com.

@AkihiroSuda AkihiroSuda merged commit c6c1d97 into moby:master Feb 21, 2021
@thaJeztah
Copy link
Member

Does this need a backport for v0.8 ?

@tonistiigi
Copy link
Member Author

@crazy-max If you confirm this is safe to take to actions right away we can cherry pick

@crazy-max
Copy link
Member

@tonistiigi Yes safe to me. I also did a bit more digging and noticed that there was a potential SSRF about this config in GitLab but nothing that concerns us because this part is sanitized and also Git environment is controlled in the buildkit image.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Loading repositories with submodules is repeated. Failed to clone submodule from googlesource
5 participants