fix: don't allow credentials in migrate/push mirror URL

Do not allow credentials to be present in the URLs that are provided for
migrations and push mirrors. They have to be given via the dedicated
input fields. Give a error when this happens.

There's nothing wrong with trying have the backend "correct" this, but
would be a larger patch than necessary in the context of a security fix.
This can be done in public.
This commit is contained in:
Gusted 2025-08-21 02:07:50 +02:00 committed by Earl Warren
commit 9f955b300b
No known key found for this signature in database
GPG key ID: 0579CB2928A78A00
7 changed files with 16 additions and 0 deletions

View file

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

View file

@ -54,6 +54,7 @@
"other": "wants to merge %[1]d commits from <code>%[2]s</code> into <code id=\"%[4]s\">%[3]s</code>" "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.", "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", "repo.issue_indexer.title": "Issue Indexer",
"search.milestone_kind": "Search milestones…", "search.milestone_kind": "Search milestones…",
"repo.settings.push_mirror.branch_filter.label": "Branch filter (optional)", "repo.settings.push_mirror.branch_filter.label": "Branch filter (optional)",

View file

@ -283,6 +283,8 @@ func handleRemoteAddrError(ctx *context.APIContext, err error) {
} }
case addrErr.IsInvalidPath: case addrErr.IsInvalidPath:
ctx.Error(http.StatusUnprocessableEntity, "", "Invalid local path, it does not exist or not a directory.") 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: default:
ctx.Error(http.StatusInternalServerError, "ParseRemoteAddr", "Unknown error type (ErrInvalidCloneAddr): "+err.Error()) ctx.Error(http.StatusInternalServerError, "ParseRemoteAddr", "Unknown error type (ErrInvalidCloneAddr): "+err.Error())
} }

View file

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

View file

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

View file

@ -1116,6 +1116,8 @@ func handleSettingRemoteAddrError(ctx *context.Context, err error, form *forms.R
} }
case addrErr.IsInvalidPath: case addrErr.IsInvalidPath:
ctx.RenderWithErr(ctx.Tr("repo.migrate.invalid_local_path"), tplSettingsOptions, form) 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: default:
ctx.ServerError("Unknown error", err) ctx.ServerError("Unknown error", err)
} }

View file

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