fix: don't allow credentials in migrate/push mirror URL (#9064)

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9064
Reviewed-by: 0ko <0ko@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@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…",
     "repo.settings.push_mirror.branch_filter.label": "Branch filter (optional)",

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

@@ -442,6 +442,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

@@ -543,7 +543,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":
@@ -1110,6 +1116,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

@@ -30,6 +30,7 @@ import (
 	"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"
@@ -42,6 +43,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"])
+	})
+}