[v12.0/forgejo] fix: don't allow credentials in migrate/push mirror URL (#9078)

**Backport:** https://codeberg.org/forgejo/forgejo/pulls/9064

It is no longer possible to specify the user and password when providing a URL for migrating a repository, the fields dedicated to that purpose on the form must be used instead. This is to prevent that those credentials are displayed in the repository settings that are visible by the repository admins, in the case where the migration is a mirror.

<!--start release-notes-assistant-->

## Release notes
<!--URL:https://codeberg.org/forgejo/forgejo-->
- Security bug fixes
  - [PR](https://codeberg.org/forgejo/forgejo/pulls/9064): <!--number 9064 --><!--line 0 --><!--description ZG9uJ3QgYWxsb3cgY3JlZGVudGlhbHMgaW4gbWlncmF0ZS9wdXNoIG1pcnJvciBVUkw=-->don't allow credentials in migrate/push mirror URL<!--description-->
<!--end release-notes-assistant-->

Co-authored-by: Gergely Nagy <forgejo@gergo.csillger.hu>
Co-authored-by: Gusted <postmaster@gusted.xyz>
Co-authored-by: Earl Warren <contact@earl-warren.org>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9078
Reviewed-by: 0ko <0ko@noreply.codeberg.org>
Co-authored-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>
Co-committed-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>

models/error.go

@@ -121,6 +121,7 @@ type ErrInvalidCloneAddr struct {
 	IsInvalidPath      bool
 	IsProtocolInvalid  bool
 	IsPermissionDenied bool
+	HasCredentials     bool
 	LocalPath          bool
 }
 
@@ -143,6 +144,9 @@ func (err *ErrInvalidCloneAddr) Error() string {
 	if err.IsURLError {
 		return fmt.Sprintf("migration/cloning from '%s' is not allowed: the provided url is invalid", err.Host)
 	}
+	if err.HasCredentials {
+		return fmt.Sprintf("migration/cloning from '%s' is not allowed: the provided url contains credentials", err.Host)
+	}
 
 	return fmt.Sprintf("migration/cloning from '%s' is not allowed", err.Host)
 }

options/locale_next/locale_en-US.json

@@ -54,6 +54,7 @@
         "other": "wants to merge %[1]d commits from <code>%[2]s</code> into <code id=\"%[4]s\">%[3]s</code>"
     },
     "repo.form.cannot_create": "All spaces in which you can create repositories have reached the limit of repositories.",
+    "migrate.form.error.url_credentials": "The URL contains contains credentials, put them in the username and password fields respectively",
     "repo.issue_indexer.title": "Issue Indexer",
     "search.milestone_kind": "Search milestones…",
     "incorrect_root_url": "This Forgejo instance is configured to be served on \"%s\". You are currently viewing Forgejo through a different URL, which may cause parts of the application to break. The canonical URL is controlled by Forgejo admins via the ROOT_URL setting in the app.ini.",

routers/api/v1/repo/migrate.go

@@ -283,6 +283,8 @@ func handleRemoteAddrError(ctx *context.APIContext, err error) {
 			}
 		case addrErr.IsInvalidPath:
 			ctx.Error(http.StatusUnprocessableEntity, "", "Invalid local path, it does not exist or not a directory.")
+		case addrErr.HasCredentials:
+			ctx.Error(http.StatusUnprocessableEntity, "", "The URL contains credentials.")
 		default:
 			ctx.Error(http.StatusInternalServerError, "ParseRemoteAddr", "Unknown error type (ErrInvalidCloneAddr): "+err.Error())
 		}

routers/api/v1/repo/mirror.go

@@ -441,6 +441,8 @@ func HandleRemoteAddressError(ctx *context.APIContext, err error) {
 			ctx.Error(http.StatusBadRequest, "CreatePushMirror", "Invalid Url ")
 		case addrErr.IsPermissionDenied:
 			ctx.Error(http.StatusUnauthorized, "CreatePushMirror", "Permission denied")
+		case addrErr.HasCredentials:
+			ctx.Error(http.StatusBadRequest, "CreatePushMirror", "The URL contains credentials")
 		default:
 			ctx.Error(http.StatusBadRequest, "CreatePushMirror", "Unknown error")
 		}

routers/web/repo/migrate.go

@@ -138,6 +138,8 @@ func handleMigrateRemoteAddrError(ctx *context.Context, err error, tpl base.TplN
 			}
 		case addrErr.IsInvalidPath:
 			ctx.RenderWithErr(ctx.Tr("repo.migrate.invalid_local_path"), tpl, form)
+		case addrErr.HasCredentials:
+			ctx.RenderWithErr(ctx.Tr("migrate.form.error.url_credentials"), tpl, form)
 		default:
 			log.Error("Error whilst updating url: %v", err)
 			ctx.RenderWithErr(ctx.Tr("form.url_error", "unknown"), tpl, form)

routers/web/repo/setting/setting.go

@@ -542,7 +542,13 @@ func SettingsPost(ctx *context.Context) {
 
 		mirror_service.AddPullMirrorToQueue(repo.ID)
 
-		ctx.Flash.Info(ctx.Tr("repo.settings.pull_mirror_sync_in_progress", repo.OriginalURL))
+		sanitizedOriginalURL, err := util.SanitizeURL(repo.OriginalURL)
+		if err != nil {
+			ctx.ServerError("SanitizeURL", err)
+			return
+		}
+
+		ctx.Flash.Info(ctx.Tr("repo.settings.pull_mirror_sync_in_progress", sanitizedOriginalURL))
 		ctx.Redirect(repo.Link() + "/settings")
 
 	case "push-mirror-sync":
@@ -1091,6 +1097,8 @@ func handleSettingRemoteAddrError(ctx *context.Context, err error, form *forms.R
 			}
 		case addrErr.IsInvalidPath:
 			ctx.RenderWithErr(ctx.Tr("repo.migrate.invalid_local_path"), tplSettingsOptions, form)
+		case addrErr.HasCredentials:
+			ctx.RenderWithErr(ctx.Tr("migrate.form.error.url_credentials"), tplSettingsOptions, form)
 		default:
 			ctx.ServerError("Unknown error", err)
 		}

services/forms/repo_form.go

@@ -105,6 +105,9 @@ func ParseRemoteAddr(remoteAddr, authUsername, authPassword string) (string, err
 		if err != nil {
 			return "", &models.ErrInvalidCloneAddr{IsURLError: true, Host: remoteAddr}
 		}
+		if u.User != nil {
+			return "", &models.ErrInvalidCloneAddr{Host: remoteAddr, HasCredentials: true}
+		}
 		if len(authUsername)+len(authPassword) > 0 {
 			u.User = url.UserPassword(authUsername, authPassword)
 		}

tests/integration/fixtures/TestPullMirrorRedactCredentials/mirror.yml

@@ -0,0 +1,9 @@
+-
+  id: 1001
+  repo_id: 1
+  interval: 3600
+  enable_prune: false
+  updated_unix: 0
+  next_update_unix: 0
+  lfs_enabled: false
+  lfs_endpoint: ""

tests/integration/fixtures/TestPullMirrorRedactCredentials/repository.yml

@@ -0,0 +1,34 @@
+-
+  id: 1001
+  owner_id: 2
+  owner_name: user2
+  lower_name: repo1001
+  name: repo1001
+  default_branch: master
+  original_url: https://:TOKEN@example.com/example/example.git
+  num_watches: 0
+  num_stars: 0
+  num_forks: 0
+  num_issues: 0
+  num_closed_issues: 0
+  num_pulls: 0
+  num_closed_pulls: 0
+  num_milestones: 0
+  num_closed_milestones: 0
+  num_projects: 0
+  num_closed_projects: 0
+  is_private: false
+  is_empty: false
+  is_archived: false
+  is_mirror: true
+  status: 0
+  is_fork: false
+  fork_id: 0
+  is_template: false
+  template_id: 0
+  size: 0
+  is_fsck_enabled: true
+  close_issues_via_commit_in_any_branch: false
+  created_unix: 1751320800
+  updated_unix: 1751320800
+  topics: '[]'

tests/integration/mirror_pull_test.go

@@ -1,9 +1,11 @@
 // Copyright 2019 The Gitea Authors. All rights reserved.
+// Copyright 2025 The Forgejo Authors. All rights reserved.
 // SPDX-License-Identifier: MIT
 
 package integration
 
 import (
+	"net/http"
 	"testing"
 
 	"forgejo.org/models/db"
@@ -13,6 +15,7 @@ import (
 	"forgejo.org/modules/git"
 	"forgejo.org/modules/gitrepo"
 	"forgejo.org/modules/migration"
+	forgejo_context "forgejo.org/services/context"
 	mirror_service "forgejo.org/services/mirror"
 	release_service "forgejo.org/services/release"
 	repo_service "forgejo.org/services/repository"
@@ -101,3 +104,18 @@ func TestMirrorPull(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, initCount, count)
 }
+
+func TestPullMirrorRedactCredentials(t *testing.T) {
+	defer unittest.OverrideFixtures("tests/integration/fixtures/TestPullMirrorRedactCredentials")()
+	defer tests.PrepareTestEnv(t)()
+
+	session := loginUser(t, "user2")
+	session.MakeRequest(t, NewRequestWithValues(t, "POST", "/user2/repo1001/settings", map[string]string{
+		"_csrf":  GetCSRF(t, session, "/user2/repo1001/settings"),
+		"action": "mirror-sync",
+	}), http.StatusSeeOther)
+
+	flashCookie := session.GetCookie(forgejo_context.CookieNameFlash)
+	assert.NotNil(t, flashCookie)
+	assert.Equal(t, "info%3DPulling%2Bchanges%2Bfrom%2Bthe%2Bremote%2Bhttps%253A%252F%252Fexample.com%252Fexample%252Fexample.git%2Bat%2Bthe%2Bmoment.", flashCookie.Value)
+}

tests/integration/mirror_push_test.go

@@ -17,6 +17,7 @@ import (
 	"time"
 
 	asymkey_model "forgejo.org/models/asymkey"
+	auth_model "forgejo.org/models/auth"
 	"forgejo.org/models/db"
 	repo_model "forgejo.org/models/repo"
 	"forgejo.org/models/unit"
@@ -26,7 +27,9 @@ import (
 	"forgejo.org/modules/gitrepo"
 	"forgejo.org/modules/optional"
 	"forgejo.org/modules/setting"
+	api "forgejo.org/modules/structs"
 	"forgejo.org/modules/test"
+	"forgejo.org/modules/translation"
 	gitea_context "forgejo.org/services/context"
 	doctor "forgejo.org/services/doctor"
 	"forgejo.org/services/migrations"
@@ -38,6 +41,46 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func TestPushMirrorRedactCredential(t *testing.T) {
+	defer test.MockVariableValue(&setting.Mirror.Enabled, true)()
+	defer tests.PrepareTestEnv(t)()
+
+	session := loginUser(t, "user2")
+	cloneAddr := "https://:TOKEN@example.com/example/example.git"
+
+	t.Run("Web route", func(t *testing.T) {
+		defer tests.PrintCurrentTest(t)()
+
+		resp := session.MakeRequest(t, NewRequestWithValues(t, "POST", "/user2/repo1/settings", map[string]string{
+			"_csrf":                GetCSRF(t, session, "/user2/repo1/settings"),
+			"action":               "push-mirror-add",
+			"push_mirror_address":  cloneAddr,
+			"push_mirror_interval": "0",
+		}), http.StatusOK)
+
+		htmlDoc := NewHTMLParser(t, resp.Body)
+		assert.Contains(t,
+			htmlDoc.doc.Find(".ui.negative.message").Text(),
+			translation.NewLocale("en-US").Tr("migrate.form.error.url_credentials"),
+		)
+	})
+
+	t.Run("API route", func(t *testing.T) {
+		defer tests.PrintCurrentTest(t)()
+
+		token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)
+		resp := MakeRequest(t, NewRequestWithJSON(t, "POST", "/api/v1/repos/user2/repo1/push_mirrors", &api.CreatePushMirrorOption{
+			RemoteAddress: cloneAddr,
+			Interval:      "0",
+		}).AddTokenAuth(token), http.StatusBadRequest)
+
+		var respBody map[string]any
+		DecodeJSON(t, resp, &respBody)
+
+		assert.Equal(t, "The URL contains credentials", respBody["message"])
+	})
+}
+
 func TestMirrorPush(t *testing.T) {
 	onGiteaRun(t, testMirrorPush)
 }

tests/integration/repo_migrate_test.go

@@ -1,4 +1,5 @@
 // Copyright 2017 The Gitea Authors. All rights reserved.
+// Copyright 2025 The Forgejo Authors. All rights reserved.
 // SPDX-License-Identifier: MIT
 
 package integration
@@ -9,7 +10,9 @@ import (
 	"net/http/httptest"
 	"testing"
 
+	auth_model "forgejo.org/models/auth"
 	"forgejo.org/modules/structs"
+	"forgejo.org/modules/translation"
 	"forgejo.org/tests"
 
 	"github.com/stretchr/testify/assert"
@@ -55,3 +58,44 @@ func TestRepoMigrate(t *testing.T) {
 		})
 	}
 }
+
+func TestRepoMigrateCredentials(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+
+	session := loginUser(t, "user2")
+	cloneAddr := "https://:TOKEN@example.com/example/example.git"
+
+	t.Run("Web route", func(t *testing.T) {
+		defer tests.PrintCurrentTest(t)()
+
+		resp := session.MakeRequest(t, NewRequestWithValues(t, "POST", "/repo/migrate?service_type=1", map[string]string{
+			"_csrf":      GetCSRF(t, session, "/repo/migrate?service_type=1"),
+			"clone_addr": cloneAddr,
+			"uid":        "2",
+			"repo_name":  "example",
+			"service":    "1",
+		}), http.StatusOK)
+
+		htmlDoc := NewHTMLParser(t, resp.Body)
+		assert.Contains(t,
+			htmlDoc.doc.Find(".ui.negative.message").Text(),
+			translation.NewLocale("en-US").Tr("migrate.form.error.url_credentials"),
+		)
+	})
+
+	t.Run("API route", func(t *testing.T) {
+		defer tests.PrintCurrentTest(t)()
+
+		token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)
+		resp := MakeRequest(t, NewRequestWithJSON(t, "POST", "/api/v1/repos/migrate", &structs.MigrateRepoOptions{
+			CloneAddr:   cloneAddr,
+			RepoOwnerID: 2,
+			RepoName:    "example",
+		}).AddTokenAuth(token), http.StatusUnprocessableEntity)
+
+		var respBody map[string]any
+		DecodeJSON(t, resp, &respBody)
+
+		assert.Equal(t, "The URL contains credentials.", respBody["message"])
+	})
+}