Skip to content

Download ssh deps over http, allow passing in credentials #627

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

Closed
wants to merge 1 commit into from

Conversation

aig787
Copy link

@aig787 aig787 commented Jan 16, 2022

This is an attempt to deal with #401 by creating a thin credential wrapper (https://git-scm.com/docs/git-credential-store#_storage_format) that uses the GIT_CREDENTIALS environment variable. Then it rewrites any ssh dependencies (which wouldn't have worked anyway) with their https equivalent. I've tested this as working on a project of mine with the following Cross.toml and GIT_CREDENTIALS=https://aig787:<github personal access token>@github.com

[build.env]
passthrough = [
    "GIT_CREDENTIALS",
    "CARGO_NET_GIT_FETCH_WITH_CLI"
]

I put it here because it's where git gets installed, but it should be easy to move it elsewhere if there's a better fit.

@aig787 aig787 requested review from Dylan-DPC-zz and a team as code owners January 16, 2022 05:33
Copy link
Contributor

@Alexhuszagh Alexhuszagh left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. However, a few changes I believe are required to fix issues that are present here:

  1. Git credentials aren't executables.
  2. Not all git servers support password-based authentication over HTTPS.
  3. Storing credentials in /usr/local/bin, rather than /usr/local/cross or /usr/local/share, seems to be unusual as a result.

echo url=$GIT_CREDENTIALS
EOF

chmod +x /usr/local/bin/git_env_credential
Copy link
Contributor

Choose a reason for hiding this comment

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

The credentials file is a plain-text file format, and shouldn't need to be executable. The file format is:

https://user:[email protected]

@@ -36,3 +36,13 @@ if_ubuntu install_packages \
g++ \
libc6-dev \
pkg-config

# Allow for passing in git credentials via environment variable in the format described here https://git-scm.com/docs/git-credential-store#_storage_format
cat <<'EOF' >/usr/local/bin/git_env_credential
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't an executable, and probably shouldn't be present in /usr/local/bin. We write files in other locations, or it could be present in /usr/local/share with a filename prefix (like cross_git_env_credential), just to ensure there's never any conflict.


chmod +x /usr/local/bin/git_env_credential
git config --system credential.helper "/usr/local/bin/git_env_credential"
git config --system url."https://".insteadOf ssh://git@
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem very sound, as SSH deps aren't interchangeable with HTTP deps for all git servers right now, specifically for Github, which is the most important git server available.

Support for HTTP password-based authentication was removed from Github in August 2021, now requiring SSH or token-based authentication, so it's probably a bad idea to re-write URLs with SSH to HTTP.

Copy link
Contributor

Choose a reason for hiding this comment

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

A much better idea would be to allow passing SSH credentials (better yet, the agent) to the container if possible, as shown in #401.

@Alexhuszagh
Copy link
Contributor

This was fixed in #684.

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

Successfully merging this pull request may close these issues.

3 participants