From f7a5ac5cf255f83814c7549c746b9a3147c787a2 Mon Sep 17 00:00:00 2001 From: pomidorry Date: Sun, 17 May 2026 00:50:56 +0300 Subject: [PATCH] use url.ParseGitURL in ParseRemoteAddr --- modules/git/remote.go | 92 +++++--------------------- modules/git/remote_test.go | 132 ++++++++++++++++++------------------- 2 files changed, 81 insertions(+), 143 deletions(-) diff --git a/modules/git/remote.go b/modules/git/remote.go index 01e3d73c0e..cec9fefea2 100644 --- a/modules/git/remote.go +++ b/modules/git/remote.go @@ -5,12 +5,12 @@ package git import ( "context" - "errors" "fmt" "net/url" "strings" "code.gitea.io/gitea/modules/git/gitcmd" + giturl "code.gitea.io/gitea/modules/git/url" "code.gitea.io/gitea/modules/util" ) @@ -81,91 +81,35 @@ func IsRemoteNotExistError(err error) bool { } // normalizeSSHURL converts SSH-SCP format URLs to standard ssh:// format for security -func normalizeSSHURL(remoteAddr string) (string, error) { - if strings.HasPrefix(remoteAddr, "ssh://") { - return remoteAddr, nil - } - if strings.Contains(remoteAddr, "://") { - return remoteAddr, errors.New("remoteAddr has a scheme") - } - if strings.Contains(remoteAddr, "\\") { - return remoteAddr, errors.New("remoteAddr has Windows path slashes") - } - if strings.Contains(remoteAddr, ":/") { - return remoteAddr, errors.New("remoteAddr could be Windows drive with forward slash") - } - if remoteAddr != "" && (remoteAddr[0] == '/' || remoteAddr[0] == '\\') { - return remoteAddr, errors.New("remoteAddr is a local file path") - } - - // Parse SSH-SCP format: [user@]host:path - colonIndex := strings.Index(remoteAddr, ":") - if colonIndex == -1 { - return remoteAddr, errors.New("remoteAddr has no colon") - } - - if colonIndex == 1 && len(remoteAddr) > 2 { - return remoteAddr, errors.New("remoteAddr could be Windows drive letter check (C:, D:, etc.)") - } - - hostPart := remoteAddr[:colonIndex] - pathPart := remoteAddr[colonIndex+1:] - - if hostPart == "" || pathPart == "" { - return remoteAddr, errors.New("remoteAddr has empty host or path") - } - - var user, host string - if atIndex := strings.LastIndex(hostPart, "@"); atIndex != -1 { - user = hostPart[:atIndex+1] // Include the @ - host = hostPart[atIndex+1:] - } else { - user = "git@" - host = hostPart - } - - if host == "" { - return remoteAddr, errors.New("Must have SSH host") - } - - return fmt.Sprintf("ssh://%s%s/%s", user, host, pathPart), nil -} - // ParseRemoteAddr checks if given remote address is valid, // and returns composed URL with needed username and password. func ParseRemoteAddr(remoteAddr, authUsername, authPassword string) (string, error) { remoteAddr = strings.TrimSpace(remoteAddr) - // First, try to normalize SSH-SCP format URLs to ssh:// format for security - normalizedAddr, err := normalizeSSHURL(remoteAddr) - if err == nil { - remoteAddr = normalizedAddr + u, err := giturl.ParseGitURL(remoteAddr) + if err != nil { + return "", &ErrInvalidCloneAddr{IsURLError: true, Host: remoteAddr} } - // Remote address can be HTTP/HTTPS/Git URL or SSH URL or local path. - if strings.HasPrefix(remoteAddr, "http://") || - strings.HasPrefix(remoteAddr, "https://") || - strings.HasPrefix(remoteAddr, "git://") { - u, err := url.Parse(remoteAddr) - if err != nil { - return "", &ErrInvalidCloneAddr{IsURLError: true, Host: remoteAddr} - } + switch u.Scheme { + case "http", "https", "git": if len(authUsername)+len(authPassword) > 0 { u.User = url.UserPassword(authUsername, authPassword) } - remoteAddr = u.String() - } else if strings.HasPrefix(remoteAddr, "ssh://") { - // Handle ssh:// URLs (including normalized ones) - u, err := url.Parse(remoteAddr) - if err != nil { - return "", &ErrInvalidCloneAddr{IsURLError: true, Host: remoteAddr} - } + return u.URL.String(), nil + case "ssh": + // SSH uses key-based auth only; username/password is not supported if len(authUsername)+len(authPassword) > 0 { - // SSH URLs don't support username/password auth, only key-based auth return "", &ErrInvalidCloneAddr{IsURLError: true, Host: remoteAddr} } - remoteAddr = u.String() + // Normalize SCP short syntax (git@host:path) into an ssh:// URL so + // downstream SSH handling can detect and use it consistently + if !strings.HasPrefix(u.Path, "/") { + u.Path = "/" + u.Path + } + return u.URL.String(), nil + default: + // Local path or unsupported scheme: pass through unchanged + return remoteAddr, nil } - - return remoteAddr, nil } diff --git a/modules/git/remote_test.go b/modules/git/remote_test.go index 5e143e4a57..4cd45bd7c2 100644 --- a/modules/git/remote_test.go +++ b/modules/git/remote_test.go @@ -9,58 +9,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestNormalizeSSHURL(t *testing.T) { - tests := []struct { - name string - input string - expected string - }{ - { - name: "SSH-SCP format with user", - input: "git@github.com:user/repo.git", - expected: "ssh://git@github.com/user/repo.git", - }, - { - name: "SSH-SCP format without user", - input: "github.com:user/repo.git", - expected: "ssh://git@github.com/user/repo.git", - }, - { - name: "Already ssh:// format", - input: "ssh://git@github.com/user/repo.git", - expected: "ssh://git@github.com/user/repo.git", - }, - { - name: "HTTP URL unchanged", - input: "https://github.com/user/repo.git", - expected: "https://github.com/user/repo.git", - }, - { - name: "Custom SSH user", - input: "myuser@example.com:path/to/repo.git", - expected: "ssh://myuser@example.com/path/to/repo.git", - }, - { - name: "Complex path", - input: "git@gitlab.com:group/subgroup/project.git", - expected: "ssh://git@gitlab.com/group/subgroup/project.git", - }, - { - name: "SSH with Port", - input: "ssh://git@example.com:2222/user/repo.git", - expected: "ssh://git@example.com:2222/user/repo.git", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result, _ := normalizeSSHURL(tt.input) - assert.Equal(t, tt.expected, result) - }) - } -} - -func TestParseRemoteAddrSSH(t *testing.T) { +func TestParseRemoteAddr(t *testing.T) { tests := []struct { name string remoteAddr string @@ -70,28 +19,73 @@ func TestParseRemoteAddrSSH(t *testing.T) { shouldError bool }{ { - name: "SSH-SCP format normalized", - remoteAddr: "git@github.com:user/repo.git", - authUser: "", - authPass: "", - expected: "ssh://git@github.com/user/repo.git", - shouldError: false, + name: "SSH SCP short syntax normalized to ssh://", + remoteAddr: "git@github.com:user/repo.git", + expected: "ssh://git@github.com/user/repo.git", }, { - name: "SSH URL with auth should error", + name: "SSH SCP custom user", + remoteAddr: "myuser@example.com:path/to/repo.git", + expected: "ssh://myuser@example.com/path/to/repo.git", + }, + { + name: "SSH SCP nested path", + remoteAddr: "git@gitlab.com:group/subgroup/project.git", + expected: "ssh://git@gitlab.com/group/subgroup/project.git", + }, + { + name: "SSH SCP IPv6 host", + remoteAddr: "git@[2001:db8::1]:user/repo.git", + expected: "ssh://git@[2001:db8::1]/user/repo.git", + }, + { + name: "ssh:// URL passed through", + remoteAddr: "ssh://git@github.com/user/repo.git", + expected: "ssh://git@github.com/user/repo.git", + }, + { + name: "ssh:// URL with port passed through", + remoteAddr: "ssh://git@example.com:2222/user/repo.git", + expected: "ssh://git@example.com:2222/user/repo.git", + }, + { + name: "SSH URL rejects username/password auth", remoteAddr: "git@github.com:user/repo.git", authUser: "user", authPass: "pass", - expected: "", shouldError: true, }, { - name: "HTTPS URL with auth", - remoteAddr: "https://github.com/user/repo.git", - authUser: "user", - authPass: "pass", - expected: "https://user:pass@github.com/user/repo.git", - shouldError: false, + name: "HTTPS URL with auth gets credentials injected", + remoteAddr: "https://github.com/user/repo.git", + authUser: "user", + authPass: "pass", + expected: "https://user:pass@github.com/user/repo.git", + }, + { + name: "HTTPS URL without auth unchanged", + remoteAddr: "https://github.com/user/repo.git", + expected: "https://github.com/user/repo.git", + }, + { + name: "git:// URL with auth gets credentials injected", + remoteAddr: "git://github.com/user/repo.git", + authUser: "user", + authPass: "pass", + expected: "git://user:pass@github.com/user/repo.git", + }, + { + name: "Local path passed through unchanged", + remoteAddr: "/srv/git/repo.git", + expected: "/srv/git/repo.git", + }, + { + // host:path without a user is not SCP syntax (ParseGitURL requires + // "@"), so it is treated as a local path and left unchanged instead + // of being silently rewritten to ssh://git@host/path + name: "host:path without user is not treated as SSH", + remoteAddr: "github.com:user/repo.git", + expected: "github.com:user/repo.git", }, } @@ -100,10 +94,10 @@ func TestParseRemoteAddrSSH(t *testing.T) { result, err := ParseRemoteAddr(tt.remoteAddr, tt.authUser, tt.authPass) if tt.shouldError { assert.Error(t, err) - } else { - assert.NoError(t, err) - assert.Equal(t, tt.expected, result) + return } + assert.NoError(t, err) + assert.Equal(t, tt.expected, result) }) } }