diff --git a/models/error.go b/models/error.go index ebaa8a135d..99c8ded766 100644 --- a/models/error.go +++ b/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) } diff --git a/options/locale_next/locale_en-US.json b/options/locale_next/locale_en-US.json index c897ad7ff2..9d39f40558 100644 --- a/options/locale_next/locale_en-US.json +++ b/options/locale_next/locale_en-US.json @@ -54,6 +54,7 @@ "other": "wants to merge %[1]d commits from %[2]s into %[3]s" }, "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)", diff --git a/routers/api/v1/repo/migrate.go b/routers/api/v1/repo/migrate.go index a848a950db..e58545c2f6 100644 --- a/routers/api/v1/repo/migrate.go +++ b/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()) } diff --git a/routers/api/v1/repo/mirror.go b/routers/api/v1/repo/mirror.go index 08ef68cbfc..fa2d5abb11 100644 --- a/routers/api/v1/repo/mirror.go +++ b/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") } diff --git a/routers/web/repo/migrate.go b/routers/web/repo/migrate.go index 86d2461e94..3a5cf30dbe 100644 --- a/routers/web/repo/migrate.go +++ b/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) diff --git a/routers/web/repo/setting/setting.go b/routers/web/repo/setting/setting.go index 595fdace83..ec18fa55a9 100644 --- a/routers/web/repo/setting/setting.go +++ b/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) } diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index d040b41395..11aac1fd52 100644 --- a/services/forms/repo_form.go +++ b/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) } diff --git a/tests/integration/fixtures/TestPullMirrorRedactCredentials/mirror.yml b/tests/integration/fixtures/TestPullMirrorRedactCredentials/mirror.yml new file mode 100644 index 0000000000..d5010d433f --- /dev/null +++ b/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: "" diff --git a/tests/integration/fixtures/TestPullMirrorRedactCredentials/repository.yml b/tests/integration/fixtures/TestPullMirrorRedactCredentials/repository.yml new file mode 100644 index 0000000000..ea76a569ea --- /dev/null +++ b/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: '[]' diff --git a/tests/integration/mirror_pull_test.go b/tests/integration/mirror_pull_test.go index c98af1b773..666ac3d3fd 100644 --- a/tests/integration/mirror_pull_test.go +++ b/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) +} diff --git a/tests/integration/mirror_push_test.go b/tests/integration/mirror_push_test.go index 73b275ed2d..23bb550d9f 100644 --- a/tests/integration/mirror_push_test.go +++ b/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) } diff --git a/tests/integration/repo_migrate_test.go b/tests/integration/repo_migrate_test.go index 233a55ef8f..90c6779bb9 100644 --- a/tests/integration/repo_migrate_test.go +++ b/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"]) + }) +}