From 1dac0987d14d5739062c589ccfe10d263d9221a1 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 1 Mar 2026 20:53:57 -0800 Subject: [PATCH 1/4] prefer ReleaseAsset DownloadFunc and add migration client coverage --- services/migrations/dump.go | 11 ++-- services/migrations/dump_test.go | 105 +++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 5 deletions(-) create mode 100644 services/migrations/dump_test.go diff --git a/services/migrations/dump.go b/services/migrations/dump.go index 04a4569a12..98e1e5e059 100644 --- a/services/migrations/dump.go +++ b/services/migrations/dump.go @@ -295,25 +295,26 @@ func (g *RepositoryDumper) CreateReleases(_ context.Context, releases ...*base.R for _, asset := range release.Assets { attachLocalPath := filepath.Join(attachDir, asset.Name) - // SECURITY: We cannot check the DownloadURL and DownloadFunc are safe here - // ... we must assume that they are safe and simply download the attachment + // SECURITY: Prefer DownloadFunc and fall back to a migration-filtered HTTP client. // download attachment err := func(attachPath string) error { var rc io.ReadCloser var err error - if asset.DownloadURL == nil { + if asset.DownloadFunc != nil { rc, err = asset.DownloadFunc() if err != nil { return err } defer rc.Close() - } else { - resp, err := http.Get(*asset.DownloadURL) + } else if asset.DownloadURL != nil { + resp, err := NewMigrationHTTPClient().Get(*asset.DownloadURL) if err != nil { return err } defer resp.Body.Close() rc = resp.Body + } else { + return errors.New("missing download URL and download function") } fw, err := os.Create(attachPath) diff --git a/services/migrations/dump_test.go b/services/migrations/dump_test.go new file mode 100644 index 0000000000..212db07979 --- /dev/null +++ b/services/migrations/dump_test.go @@ -0,0 +1,105 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package migrations + +import ( + "context" + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "sync/atomic" + "testing" + + base "code.gitea.io/gitea/modules/migration" + "code.gitea.io/gitea/modules/setting" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRepositoryDumperReleaseAssetPrefersDownloadFunc(t *testing.T) { + var downloadURLHits int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt32(&downloadURLHits, 1) + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("remote")) + })) + t.Cleanup(server.Close) + + downloadURL := server.URL + "/asset" + var downloadFuncCalls int32 + asset := &base.ReleaseAsset{ + Name: "asset.txt", + DownloadURL: &downloadURL, + DownloadFunc: func() (io.ReadCloser, error) { + atomic.AddInt32(&downloadFuncCalls, 1) + return io.NopCloser(strings.NewReader("local")), nil + }, + } + release := &base.Release{ + TagName: "v1.0.0", + Assets: []*base.ReleaseAsset{asset}, + } + + baseDir := t.TempDir() + dumper, err := NewRepositoryDumper(context.Background(), baseDir, "owner", "repo", base.MigrateOptions{}) + require.NoError(t, err) + + require.NoError(t, dumper.CreateReleases(context.Background(), release)) + assert.Equal(t, int32(1), atomic.LoadInt32(&downloadFuncCalls)) + assert.Equal(t, int32(0), atomic.LoadInt32(&downloadURLHits)) + + attachRelative := filepath.Join("release_assets", release.TagName, asset.Name) + attachPath := filepath.Join(baseDir, "owner", "repo", attachRelative) + data, err := os.ReadFile(attachPath) + require.NoError(t, err) + assert.Equal(t, "local", string(data)) + require.NotNil(t, asset.DownloadURL) + assert.Equal(t, attachRelative, *asset.DownloadURL) +} + +func TestRepositoryDumperReleaseAssetUsesMigrationClient(t *testing.T) { + oldAllowed := setting.Migrations.AllowedDomains + oldBlocked := setting.Migrations.BlockedDomains + oldAllowLocal := setting.Migrations.AllowLocalNetworks + t.Cleanup(func() { + setting.Migrations.AllowedDomains = oldAllowed + setting.Migrations.BlockedDomains = oldBlocked + setting.Migrations.AllowLocalNetworks = oldAllowLocal + _ = Init() + }) + + setting.Migrations.AllowedDomains = "" + setting.Migrations.BlockedDomains = "" + setting.Migrations.AllowLocalNetworks = false + require.NoError(t, Init()) + + var downloadURLHits int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt32(&downloadURLHits, 1) + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("remote")) + })) + t.Cleanup(server.Close) + + downloadURL := server.URL + "/asset" + asset := &base.ReleaseAsset{ + Name: "asset.txt", + DownloadURL: &downloadURL, + } + release := &base.Release{ + TagName: "v1.0.0", + Assets: []*base.ReleaseAsset{asset}, + } + + baseDir := t.TempDir() + dumper, err := NewRepositoryDumper(context.Background(), baseDir, "owner", "repo", base.MigrateOptions{}) + require.NoError(t, err) + + assert.Error(t, dumper.CreateReleases(context.Background(), release)) + assert.Equal(t, int32(0), atomic.LoadInt32(&downloadURLHits)) +} From 80507f9aff06c2c18fd4fba36db1954484ff98e1 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 1 Mar 2026 23:47:32 -0800 Subject: [PATCH 2/4] Fix --- services/migrations/dump.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/migrations/dump.go b/services/migrations/dump.go index 98e1e5e059..2de24c8eb9 100644 --- a/services/migrations/dump.go +++ b/services/migrations/dump.go @@ -287,6 +287,7 @@ func (g *RepositoryDumper) CreateLabels(_ context.Context, labels ...*base.Label // CreateReleases creates releases func (g *RepositoryDumper) CreateReleases(_ context.Context, releases ...*base.Release) error { if g.opts.ReleaseAssets { + httpClient := NewMigrationHTTPClient() for _, release := range releases { attachDir := filepath.Join("release_assets", release.TagName) if err := os.MkdirAll(filepath.Join(g.baseDir, attachDir), os.ModePerm); err != nil { @@ -307,7 +308,7 @@ func (g *RepositoryDumper) CreateReleases(_ context.Context, releases ...*base.R } defer rc.Close() } else if asset.DownloadURL != nil { - resp, err := NewMigrationHTTPClient().Get(*asset.DownloadURL) + resp, err := httpClient.Get(*asset.DownloadURL) if err != nil { return err } From 1b39602db540a359b9ea57f146494d0a61cdfe84 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 1 Mar 2026 23:48:11 -0800 Subject: [PATCH 3/4] Fix copyright head year --- services/migrations/dump_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/migrations/dump_test.go b/services/migrations/dump_test.go index 212db07979..ef04ba1414 100644 --- a/services/migrations/dump_test.go +++ b/services/migrations/dump_test.go @@ -1,4 +1,4 @@ -// Copyright 2025 The Gitea Authors. All rights reserved. +// Copyright 2026 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT package migrations From ff1e48d81729ca1ff4fd105c041235f5f4891c16 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 2 Mar 2026 11:05:15 -0800 Subject: [PATCH 4/4] Fix test --- services/migrations/dump_test.go | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/services/migrations/dump_test.go b/services/migrations/dump_test.go index ef04ba1414..40908e5fb8 100644 --- a/services/migrations/dump_test.go +++ b/services/migrations/dump_test.go @@ -14,8 +14,10 @@ import ( "sync/atomic" "testing" + "code.gitea.io/gitea/models/unittest" base "code.gitea.io/gitea/modules/migration" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/test" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -46,7 +48,7 @@ func TestRepositoryDumperReleaseAssetPrefersDownloadFunc(t *testing.T) { } baseDir := t.TempDir() - dumper, err := NewRepositoryDumper(context.Background(), baseDir, "owner", "repo", base.MigrateOptions{}) + dumper, err := NewRepositoryDumper(context.Background(), baseDir, "owner", "repo", base.MigrateOptions{ReleaseAssets: true}) require.NoError(t, err) require.NoError(t, dumper.CreateReleases(context.Background(), release)) @@ -63,20 +65,11 @@ func TestRepositoryDumperReleaseAssetPrefersDownloadFunc(t *testing.T) { } func TestRepositoryDumperReleaseAssetUsesMigrationClient(t *testing.T) { - oldAllowed := setting.Migrations.AllowedDomains - oldBlocked := setting.Migrations.BlockedDomains - oldAllowLocal := setting.Migrations.AllowLocalNetworks - t.Cleanup(func() { - setting.Migrations.AllowedDomains = oldAllowed - setting.Migrations.BlockedDomains = oldBlocked - setting.Migrations.AllowLocalNetworks = oldAllowLocal - _ = Init() - }) + assert.NoError(t, unittest.PrepareTestDatabase()) - setting.Migrations.AllowedDomains = "" - setting.Migrations.BlockedDomains = "" - setting.Migrations.AllowLocalNetworks = false - require.NoError(t, Init()) + test.MockVariableValue(&setting.Migrations.AllowedDomains, "github.com")() + test.MockVariableValue(&setting.Migrations.AllowLocalNetworks, false)() + assert.NoError(t, Init()) var downloadURLHits int32 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -97,7 +90,7 @@ func TestRepositoryDumperReleaseAssetUsesMigrationClient(t *testing.T) { } baseDir := t.TempDir() - dumper, err := NewRepositoryDumper(context.Background(), baseDir, "owner", "repo", base.MigrateOptions{}) + dumper, err := NewRepositoryDumper(context.Background(), baseDir, "owner", "repo", base.MigrateOptions{ReleaseAssets: true}) require.NoError(t, err) assert.Error(t, dumper.CreateReleases(context.Background(), release))