fix(repo): do not reuse pull mirror credentials for a new host (#39991)

Changing a pull mirror address with an empty password field carried the
stored credentials over to the new address. The password is write-only,
so a repo admin who did not set up the mirror could send the owner's
token to a host they control, or pull another repository on the same
host that the token can read.

Stored credentials are now reused only when the address is unchanged,
like a changed username already drops the stored password. The API
cannot change the address and push mirrors never reuse credentials, so
both are unaffected.

---------

Co-authored-by: silverwind <me@silverwind.io>
This commit is contained in:
Basavaraj S m
2026-11-07 12:35:10 +00:00
committed by GitHub
parent dd4672607b
commit 4f69599981
3 changed files with 26 additions and 28 deletions

View File

@@ -1003,7 +1003,7 @@
"repo.mirror_last_synced": "Last Synchronized",
"repo.mirror_password_placeholder": "(Unchanged)",
"repo.mirror_password_blank_placeholder": "(Unset)",
"repo.mirror_password_help": "Change the username to erase a stored password.",
"repo.mirror_password_help": "Change the username or the address to erase a stored password.",
"repo.watchers": "Watchers",
"repo.stargazers": "Stargazers",
"repo.stars_remove_warning": "This will remove all stars from this repository.",

View File

@@ -316,7 +316,10 @@ func handleSettingsPostMirror(ctx *context.Context) {
handleSettingRemoteAddrError(ctx, err, form)
return
}
if u.User != nil {
storedAddr := *u.URL
storedAddr.User = nil
// the stored credentials are write-only, never let a changed address receive them
if u.User != nil && strings.TrimSpace(form.MirrorAddress) == storedAddr.String() {
if form.MirrorUsername == "" {
form.MirrorUsername = u.User.Username()
}

View File

@@ -161,7 +161,7 @@ func TestDeleteTeam(t *testing.T) {
assert.False(t, repo_service.HasRepository(t.Context(), team, repo.ID))
}
func TestHandleSettingsPostMirrorPreservesExistingUsername(t *testing.T) {
func TestHandleSettingsPostMirrorReusesCredentialsOnlyForSameAddress(t *testing.T) {
defer test.MockVariableValue(&setting.Mirror.Enabled, true)()
unittest.PrepareTestEnv(t)
@@ -172,33 +172,28 @@ func TestHandleSettingsPostMirrorPreservesExistingUsername(t *testing.T) {
require.NoError(t, mirror_service.UpdateAddress(t.Context(), mirror, "https://existing-user:existing-password@example.com/user2/repo1.git"))
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
postMirrorSettings := func(form *forms.RepoSettingForm) string {
ctx, _ := contexttest.MockContext(t, mirrorRepo.Link()+"/settings")
contexttest.LoadUser(t, ctx, 2)
contexttest.LoadRepo(t, ctx, mirrorRepo.ID)
form.Interval = "8h"
web.SetForm(ctx, form)
handleSettingsPostMirror(ctx)
require.Equal(t, http.StatusSeeOther, ctx.Resp.WrittenStatus())
remoteURL, err := git.ParseRemoteAddressURL(t.Context(), mirrorRepo, mirror.GetRemoteName())
require.NoError(t, err)
return remoteURL.User.String()
}
ctx, _ := contexttest.MockContext(t, mirrorRepo.Link()+"/settings")
contexttest.LoadUser(t, ctx, user.ID)
contexttest.LoadRepo(t, ctx, mirrorRepo.ID)
web.SetForm(ctx, &forms.RepoSettingForm{
Interval: "8h",
assert.Equal(t, "existing-user:updated-password", postMirrorSettings(&forms.RepoSettingForm{
MirrorAddress: "https://example.com/user2/repo1.git",
MirrorPassword: "updated-password",
})
}))
assert.Equal(t, "https://example.com/user2/repo1.git", unittest.AssertExistsAndLoadBean(t, &repo_model.Mirror{RepoID: mirrorRepo.ID}).RemoteAddress)
assert.Equal(t, "https://example.com/user2/repo1.git", unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: mirrorRepo.ID}).OriginalURL)
handleSettingsPostMirror(ctx)
assert.Equal(t, http.StatusSeeOther, ctx.Resp.WrittenStatus())
updatedMirror := unittest.AssertExistsAndLoadBean(t, &repo_model.Mirror{RepoID: mirrorRepo.ID})
assert.Equal(t, "https://example.com/user2/repo1.git", updatedMirror.RemoteAddress)
updatedRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: mirrorRepo.ID})
assert.Equal(t, "https://example.com/user2/repo1.git", updatedRepo.OriginalURL)
remoteURL, err := git.ParseRemoteAddressURL(t.Context(), updatedRepo, updatedMirror.GetRemoteName())
require.NoError(t, err)
require.NotNil(t, remoteURL.User)
assert.Equal(t, "existing-user", remoteURL.User.Username())
password, ok := remoteURL.User.Password()
require.True(t, ok)
assert.Equal(t, "updated-password", password)
assert.Equal(t, "existing-user:", postMirrorSettings(&forms.RepoSettingForm{
MirrorAddress: "https://example.com/user2/private.git",
MirrorUsername: "existing-user",
}))
}