Repository navigation
fix(auth): pick the push credential from the resolved forge, not the host name - #1124
Conversation
There was a problem hiding this comment.
The fix is right: carrying Remote { name, forge } through the push and fetch entry points puts the credential choice on the same resolution the API already uses, and the explicit-forge-beats-host-name ordering in forge_of is the behaviour the config documents. Threading it as a Copy struct instead of adding a parameter everywhere keeps the call sites honest, and the early return before any resolution means a tokenless run and local path remotes still touch no network.
Two nits inline, both in auth.rs, neither blocking. One is a real behaviour change in the auto path: detect_forge_with_probe matches forge names anywhere in the URL, so the repo name can now pick the push credential, which is exactly what a_repository_name_does_not_pick_the_forge was written to prevent. It is narrow (needs a SaaS hostname inside the path of a self-hosted remote) and the API side already behaves that way, so it is consistency rather than a new class of bug, but the one-line fix is in the comment.
Leaving Gitea/Forgejo to a follow-up is the right call, and the "Not changed" section says so clearly enough that nobody will read this as covering it.
Note that CI is still running (Test, Coverage, Build Release Binary, opengrep in progress at review time), so the 2435-passing claim is unverified from here.
| fn forge_of(url: &str, configured: ForgeKind) -> Option<ForgeKind> { | ||
| match configured { | ||
| ForgeKind::Auto if is_gitlab(url) => Some(ForgeKind::Gitlab), | ||
| ForgeKind::Auto => crate::forge::detect_forge_with_probe(url), |
There was a problem hiding this comment.
Nit: detect_forge_with_probe falls through to detect_forge_from_url, which matches github.com / gitlab.com / codeberg.org anywhere in the string, path included. This file deliberately went the other way: is_gitlab matches on host_of(url) only, and a_repository_name_does_not_pick_the_forge asserts a repo name must not select the credential. The auto branch now gives that up.
Concretely, remote https://git.acme.com/tools/gitlab.com-exporter.git with forge: auto resolves to GitLab off the path: with FERRFLOW_TOKEN set the push username flips from x-access-token to oauth2, and with only GITHUB_TOKEN set token_for_url returns None, so the push goes out with no credential at all. The existing test does not catch it because all four of its URLs are on github.com.
Detecting on the host keeps the probe cache key identical and leaves the SaaS short-circuits working:
| ForgeKind::Auto => crate::forge::detect_forge_with_probe(url), | |
| ForgeKind::Auto => crate::forge::extract_host(url) | |
| .and_then(|host| crate::forge::detect_forge_with_probe(&format!("https://{host}/"))), |
(forge_target in preview.rs passes the full URL too, so the API side has the same quirk. Worth fixing there in the same pass, or inside detect_forge_from_url.)
There was a problem hiding this comment.
Taken in #1132 (this PR had already merged).
| let ferrflow_token = std::env::var("FERRFLOW_TOKEN").ok(); | ||
| let gitlab_token = std::env::var("GITLAB_TOKEN").ok(); | ||
| let github_token = std::env::var("GITHUB_TOKEN").ok(); |
There was a problem hiding this comment.
Nit: an empty variable counts as present here. GITHUB_TOKEN: ${{ secrets.MISSING }} exports "", which passes the new guard and buys a network probe (up to 3 × 2s on a self-hosted host) for a credential that is then handed to git as an empty password. forge::resolve_token already filters empties; matching it makes the guard mean what it says.
| let ferrflow_token = std::env::var("FERRFLOW_TOKEN").ok(); | |
| let gitlab_token = std::env::var("GITLAB_TOKEN").ok(); | |
| let github_token = std::env::var("GITHUB_TOKEN").ok(); | |
| let ferrflow_token = std::env::var("FERRFLOW_TOKEN").ok().filter(|t| !t.is_empty()); | |
| let gitlab_token = std::env::var("GITLAB_TOKEN").ok().filter(|t| !t.is_empty()); | |
| let github_token = std::env::var("GITHUB_TOKEN").ok().filter(|t| !t.is_empty()); |
There was a problem hiding this comment.
Taken in #1132 (this PR had already merged).
Closes #1121.
The git push picked its credential by checking whether the remote's host contains
gitlab. The API side resolves the forge properly: the configuredforge, or inautomode the URL and then the/api/v4/versionprobe. On a self-hosted GitLab likegit.company.comthe two disagreed. API calls authenticated as GitLab, while the push took the GitHub branch: it never readGITLAB_TOKEN, and usedx-access-tokenwithGITHUB_TOKEN, orFERRFLOW_TOKENunder the wrong username.Change
The push now uses the same resolution as the API.
git::Remote { name, forge }replaces the bareremote_name: &stron every push and fetch entry point (push,push_tags,force_push_tags,force_push_branch,release_branch_foreign_commit,reset_branch_to_remote,delete_tag_if_unchanged,fetch_tags). Callers build it withRemote::of(&config.workspace), so the configured forge travels with the remote name it belongs to.configure_git_command/token_for_urltake that forge:forgein the config: used as is, including over the host name (forge: githubongitlab-mirror.acme.compushes withGITHUB_TOKEN)auto, host containinggitlab: GitLab, as before and with no networkauto, otherwise:detect_forge_with_probe, the functionbuild_forgealready uses. Its per-host cache is shared, so a run that has already probed for the API side does not probe again.FERRFLOW_TOKEN,GITLAB_TOKENandGITHUB_TOKENis set,token_for_urlreturns before resolving anything. A run with no token pays nothing, and local path remotes never reach the probe.GitHub and GitLab hosts behave exactly as before. The existing tests pass unchanged, apart from receiving
ForgeKind::Auto.Not changed
The issue notes that self-hosted Gitea/Forgejo likely has the same gap. It does: every non-GitLab forge still pushes as
x-access-tokenwithGITHUB_TOKENorFERRFLOW_TOKEN, andGITEA_TOKENis not read on the push side. Fixing that is a separate behaviour change (which variable, which username), so it is left out. Now that the resolved forge is available intoken_for_url, it is a small follow-up.Tests
With an explicit forge on a host that names no forge, so no network is involved:
forge: gitlabongit.company.compushes withGITLAB_TOKENasoauth2, not with theGITHUB_TOKENalso setgitlab-ci-token(feat(gitlab): authenticate CI job tokens with JOB-TOKEN and gitlab-ci-token #1120)forge: githubon a host containinggitlabpushes withGITHUB_TOKEN, so the configured forge beats the heuristicFull suite: 2435 passed. The
git::testsruntime is unchanged compared withmain.Docs: the
forgerow inconfiguration/config-file.md(EN, FR) says the same detection now picks the push credential.