From ba3e16c4b4f9a6f73197f46805211cd1dd7b9bbb Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 10 Nov 2021 18:10:38 +0800 Subject: [PATCH] fix unit test --- integrations/api_repo_lfs_migrate_test.go | 3 ++ integrations/mirror_push_test.go | 2 + modules/hostmatcher/hostmatcher.go | 47 ++++++++++++----------- modules/hostmatcher/hostmatcher_test.go | 13 +++++-- modules/migrations/migrate.go | 17 +++++--- modules/migrations/migrate_test.go | 2 + 6 files changed, 53 insertions(+), 31 deletions(-) diff --git a/integrations/api_repo_lfs_migrate_test.go b/integrations/api_repo_lfs_migrate_test.go index 9acc96e4f473..01f26d782f91 100644 --- a/integrations/api_repo_lfs_migrate_test.go +++ b/integrations/api_repo_lfs_migrate_test.go @@ -12,6 +12,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/lfs" + "code.gitea.io/gitea/modules/migrations" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" @@ -25,6 +26,7 @@ func TestAPIRepoLFSMigrateLocal(t *testing.T) { oldAllowLocalNetworks := setting.Migrations.AllowLocalNetworks setting.ImportLocalPaths = true setting.Migrations.AllowLocalNetworks = true + _ = migrations.Init() user := db.AssertExistsAndLoadBean(t, &models.User{ID: 1}).(*models.User) session := loginUser(t, user.Name) @@ -47,4 +49,5 @@ func TestAPIRepoLFSMigrateLocal(t *testing.T) { setting.ImportLocalPaths = oldImportLocalPaths setting.Migrations.AllowLocalNetworks = oldAllowLocalNetworks + _ = migrations.Init() } diff --git a/integrations/mirror_push_test.go b/integrations/mirror_push_test.go index 3d2966cfdcd4..740d013a42b2 100644 --- a/integrations/mirror_push_test.go +++ b/integrations/mirror_push_test.go @@ -14,6 +14,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/migrations" "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/setting" mirror_service "code.gitea.io/gitea/services/mirror" @@ -29,6 +30,7 @@ func testMirrorPush(t *testing.T, u *url.URL) { defer prepareTestEnv(t)() setting.Migrations.AllowLocalNetworks = true + _ = migrations.Init() user := db.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) srcRepo := db.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository) diff --git a/modules/hostmatcher/hostmatcher.go b/modules/hostmatcher/hostmatcher.go index ceb9b8c081af..527c568d5ee8 100644 --- a/modules/hostmatcher/hostmatcher.go +++ b/modules/hostmatcher/hostmatcher.go @@ -5,7 +5,6 @@ package hostmatcher import ( - "fmt" "net" "path/filepath" "strings" @@ -19,9 +18,9 @@ type HostMatchList struct { SettingKeyHint string SettingValue string - // host name or built-in network name - names []string - ipNets []*net.IPNet + // patterns for host names or built-in networks + patterns []string + ipNets []*net.IPNet } // MatchBuiltinAll all hosts are matched @@ -48,17 +47,17 @@ func ParseHostMatchList(settingKeyHint string, hostList string) *HostMatchList { if err == nil { hl.ipNets = append(hl.ipNets, ipNet) } else { - hl.names = append(hl.names, s) + hl.patterns = append(hl.patterns, s) } } return hl } // ParseSimpleMatchList parse a simple matchlist (no built-in networks, no CIDR support) -func ParseSimpleMatchList(settingKeyHint string, matchList string, includeLocalNetwork bool) *HostMatchList { +func ParseSimpleMatchList(settingKeyHint string, matchList string) *HostMatchList { hl := &HostMatchList{ SettingKeyHint: settingKeyHint, - SettingValue: matchList + fmt.Sprintf("(local-network:%v)", includeLocalNetwork), + SettingValue: matchList, } for _, s := range strings.Split(matchList, ",") { s = strings.ToLower(strings.TrimSpace(s)) @@ -66,38 +65,40 @@ func ParseSimpleMatchList(settingKeyHint string, matchList string, includeLocalN continue } if s == MatchBuiltinLoopback || s == MatchBuiltinPrivate || s == MatchBuiltinExternal { - // for built-in names, we convert it from "private" => "[p]rivate" for internal usage and keep the same result as `matchlist` - hl.names = append(hl.names, "["+s[:1]+"]"+s[1:]) + // for built-in patterns, we convert it from "private" => "[p]rivate" for internal usage and keep the same result as `matchlist` + hl.patterns = append(hl.patterns, "["+s[:1]+"]"+s[1:]) } else { // we keep the same result as `matchlist`, so no CIDR support here - hl.names = append(hl.names, s) + hl.patterns = append(hl.patterns, s) } } - if includeLocalNetwork { - hl.names = append(hl.names, MatchBuiltinPrivate) - } return hl } +// AppendPattern appends more patterns to match +func (hl *HostMatchList) AppendPattern(pattern string) { + hl.patterns = append(hl.patterns, pattern) +} + // IsEmpty checks if the check list is empty func (hl *HostMatchList) IsEmpty() bool { - return hl == nil || (len(hl.names) == 0 && len(hl.ipNets) == 0) + return hl == nil || (len(hl.patterns) == 0 && len(hl.ipNets) == 0) } -func (hl *HostMatchList) checkNames(host string) bool { +func (hl *HostMatchList) checkPattern(host string) bool { host = strings.ToLower(strings.TrimSpace(host)) - for _, name := range hl.names { - switch name { + for _, pattern := range hl.patterns { + switch pattern { case "": case MatchBuiltinExternal: case MatchBuiltinPrivate: case MatchBuiltinLoopback: - // ignore empty string or built-in network names + // ignore empty string or built-in network patterns continue case MatchBuiltinAll: return true default: - if matched, _ := filepath.Match(name, host); matched { + if matched, _ := filepath.Match(pattern, host); matched { return true } } @@ -106,8 +107,8 @@ func (hl *HostMatchList) checkNames(host string) bool { } func (hl *HostMatchList) checkIP(ip net.IP) bool { - for _, name := range hl.names { - switch name { + for _, pattern := range hl.patterns { + switch pattern { case "": continue case MatchBuiltinAll: @@ -139,7 +140,7 @@ func (hl *HostMatchList) MatchHostName(host string) bool { if hl == nil { return false } - if hl.checkNames(host) { + if hl.checkPattern(host) { return true } if ip := net.ParseIP(host); ip != nil { @@ -154,7 +155,7 @@ func (hl *HostMatchList) MatchIPAddr(ip net.IP) bool { return false } host := ip.String() // nil-safe, we will get "" if ip is nil - return hl.checkNames(host) || hl.checkIP(ip) + return hl.checkPattern(host) || hl.checkIP(ip) } // MatchHostOrIP checks if the host or IP matches an allow/deny(block) list diff --git a/modules/hostmatcher/hostmatcher_test.go b/modules/hostmatcher/hostmatcher_test.go index d5e45c5b37e4..66030a32f1ed 100644 --- a/modules/hostmatcher/hostmatcher_test.go +++ b/modules/hostmatcher/hostmatcher_test.go @@ -132,22 +132,29 @@ func TestHostOrIPMatchesList(t *testing.T) { } test(cases) - hl = ParseSimpleMatchList("", "loopback, *.domain.com", true) + hl = ParseSimpleMatchList("", "loopback, *.domain.com") cases = []tc{ {"loopback", nil, true}, {"", net.ParseIP("127.0.0.1"), false}, {"sub.domain.com", nil, true}, {"other.com", nil, false}, - {"", net.ParseIP("192.168.1.1"), true}, {"", net.ParseIP("1.1.1.1"), false}, } test(cases) - hl = ParseSimpleMatchList("", "external", false) + hl = ParseSimpleMatchList("", "external") cases = []tc{ {"", net.ParseIP("192.168.1.1"), false}, {"", net.ParseIP("1.1.1.1"), false}, {"external", nil, true}, } test(cases) + + hl = ParseSimpleMatchList("", "") + cases = []tc{ + {"", net.ParseIP("192.168.1.1"), false}, + {"", net.ParseIP("1.1.1.1"), false}, + {"external", nil, false}, + } + test(cases) } diff --git a/modules/migrations/migrate.go b/modules/migrations/migrate.go index b3e88e42ff93..0b85d7a4c3b2 100644 --- a/modules/migrations/migrate.go +++ b/modules/migrations/migrate.go @@ -75,11 +75,9 @@ func IsMigrateURLAllowed(remoteURL string, doer *models.User) error { hostName, _, err := net.SplitHostPort(u.Host) if err != nil { // u.Host can be "host" or "host:port" + err = nil hostName = u.Host } - if err != nil { - return &models.ErrInvalidCloneAddr{Host: u.Host, IsURLError: true} - } addrList, err := net.LookupIP(hostName) if err != nil { return &models.ErrInvalidCloneAddr{Host: u.Host, NotResolvedIP: true} @@ -469,7 +467,16 @@ func migrateRepository(downloader base.Downloader, uploader base.Uploader, opts // Init migrations service func Init() error { // TODO: maybe we can deprecate these legacy ALLOWED_DOMAINS/ALLOW_LOCALNETWORKS/BLOCKED_DOMAINS, use ALLOWED_HOST_LIST/BLOCKED_HOST_LIST instead - allowList = hostmatcher.ParseSimpleMatchList("migrations.ALLOWED_DOMAINS/ALLOW_LOCALNETWORKS", setting.Migrations.AllowedDomains, setting.Migrations.AllowLocalNetworks) - blockList = hostmatcher.ParseSimpleMatchList("migrations.BLOCKED_DOMAINS", setting.Migrations.BlockedDomains, false) + + blockList = hostmatcher.ParseSimpleMatchList("migrations.BLOCKED_DOMAINS", setting.Migrations.BlockedDomains) + + allowList = hostmatcher.ParseSimpleMatchList("migrations.ALLOWED_DOMAINS/ALLOW_LOCALNETWORKS", setting.Migrations.AllowedDomains) + if allowList.IsEmpty() { + // the default policy is that migration module can access external hosts + allowList.AppendPattern(hostmatcher.MatchBuiltinExternal) + } + if setting.Migrations.AllowLocalNetworks { + allowList.AppendPattern(hostmatcher.MatchBuiltinPrivate) + } return nil } diff --git a/modules/migrations/migrate_test.go b/modules/migrations/migrate_test.go index 3cd0938ef6af..db472ce85994 100644 --- a/modules/migrations/migrate_test.go +++ b/modules/migrations/migrate_test.go @@ -22,6 +22,7 @@ func TestMigrateWhiteBlocklist(t *testing.T) { nonAdminUser := db.AssertExistsAndLoadBean(t, &models.User{Name: "user2"}).(*models.User) setting.Migrations.AllowedDomains = "github.com" + setting.Migrations.AllowLocalNetworks = false assert.NoError(t, Init()) err := IsMigrateURLAllowed("https://gitlab.com/gitlab/gitlab.git", nonAdminUser) @@ -47,6 +48,7 @@ func TestMigrateWhiteBlocklist(t *testing.T) { assert.Error(t, err) setting.Migrations.AllowLocalNetworks = true + assert.NoError(t, Init()) err = IsMigrateURLAllowed("https://10.0.0.1/go-gitea/gitea.git", nonAdminUser) assert.NoError(t, err)