diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index fa11acaee2..9975729fd6 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -399,6 +399,7 @@ func prepareMigrationTasks() []*migration { newMigration(323, "Add support for actions concurrency", v1_26.AddActionsConcurrency), newMigration(324, "Fix closed milestone completeness for milestones with no issues", v1_26.FixClosedMilestoneCompleteness), + newMigration(325, "Fix missed repo_id when migrate attachments", v1_26.FixMissedRepoIDWhenMigrateAttachments), } return preparedMigrations } diff --git a/models/migrations/v1_26/v325.go b/models/migrations/v1_26/v325.go new file mode 100644 index 0000000000..d81540f44e --- /dev/null +++ b/models/migrations/v1_26/v325.go @@ -0,0 +1,18 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_26 + +import ( + "xorm.io/xorm" +) + +func FixMissedRepoIDWhenMigrateAttachments(x *xorm.Engine) error { + _, err := x.Exec("UPDATE `attachment` SET `repo_id` = (SELECT `repo_id` FROM `issue` WHERE `issue`.`id` = `attachment`.`issue_id`) WHERE `issue_id` > 0 AND (`repo_id` IS NULL OR `repo_id` = 0);") + if err != nil { + return err + } + + _, err = x.Exec("UPDATE `attachment` SET `repo_id` = (SELECT `repo_id` FROM `release` WHERE `release`.`id` = `attachment`.`release_id`) WHERE `release_id` > 0 AND (`repo_id` IS NULL OR `repo_id` = 0);") + return err +} diff --git a/models/migrations/v1_26/v325_test.go b/models/migrations/v1_26/v325_test.go new file mode 100644 index 0000000000..d4a66fee81 --- /dev/null +++ b/models/migrations/v1_26/v325_test.go @@ -0,0 +1,45 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_26 + +import ( + "testing" + + "code.gitea.io/gitea/models/migrations/base" + "code.gitea.io/gitea/modules/timeutil" + + "github.com/stretchr/testify/require" +) + +func Test_FixMissedRepoIDWhenMigrateAttachments(t *testing.T) { + type Attachment struct { + ID int64 `xorm:"pk autoincr"` + UUID string `xorm:"uuid UNIQUE"` + RepoID int64 `xorm:"INDEX"` // this should not be zero + IssueID int64 `xorm:"INDEX"` // maybe zero when creating + ReleaseID int64 `xorm:"INDEX"` // maybe zero when creating + UploaderID int64 `xorm:"INDEX DEFAULT 0"` // Notice: will be zero before this column added + CommentID int64 `xorm:"INDEX"` + Name string + DownloadCount int64 `xorm:"DEFAULT 0"` + Size int64 `xorm:"DEFAULT 0"` + CreatedUnix timeutil.TimeStamp `xorm:"created"` + } + + type Issue struct { + ID int64 `xorm:"pk autoincr"` + RepoID int64 `xorm:"INDEX"` + } + + type Release struct { + ID int64 `xorm:"pk autoincr"` + RepoID int64 `xorm:"INDEX"` + } + + // Prepare and load the testing database + x, deferrable := base.PrepareTestEnv(t, 0, new(Attachment), new(Issue), new(Release)) + defer deferrable() + + require.NoError(t, FixMissedRepoIDWhenMigrateAttachments(x)) +} diff --git a/models/repo/release.go b/models/repo/release.go index 68fb6b1724..e2010c8a38 100644 --- a/models/repo/release.go +++ b/models/repo/release.go @@ -93,6 +93,10 @@ func init() { db.RegisterModel(new(Release)) } +// LegacyAttachmentMissingRepoIDCutoff marks the date when repo_id started to be written during uploads +// (2026-01-16T00:00:00Z). Older rows might have repo_id=0 and should be tolerated once. +const LegacyAttachmentMissingRepoIDCutoff timeutil.TimeStamp = 1768521600 + func (r *Release) LoadRepo(ctx context.Context) (err error) { if r.Repo != nil { return nil @@ -186,6 +190,13 @@ func AddReleaseAttachments(ctx context.Context, releaseID int64, attachmentUUIDs } for i := range attachments { + if attachments[i].RepoID == 0 && attachments[i].CreatedUnix < LegacyAttachmentMissingRepoIDCutoff { + attachments[i].RepoID = rel.RepoID + if _, err = db.GetEngine(ctx).ID(attachments[i].ID).Cols("repo_id").Update(attachments[i]); err != nil { + return fmt.Errorf("update attachment repo_id [%d]: %w", attachments[i].ID, err) + } + } + if attachments[i].RepoID != rel.RepoID { return util.NewPermissionDeniedErrorf("attachment belongs to different repository") } diff --git a/models/repo/release_test.go b/models/repo/release_test.go index 8e30e76f49..2a09ffb36d 100644 --- a/models/repo/release_test.go +++ b/models/repo/release_test.go @@ -6,6 +6,7 @@ package repo import ( "testing" + "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/util" @@ -51,3 +52,41 @@ func TestAddReleaseAttachmentsRejectsDifferentRepo(t *testing.T) { assert.NoError(t, err) assert.Zero(t, attach.ReleaseID, "attachment should not be linked to release on failure") } + +func TestAddReleaseAttachmentsAllowsLegacyMissingRepoID(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + legacyUUID := "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a20" // attachment 10 has repo_id 0 + err := AddReleaseAttachments(t.Context(), 1, []string{legacyUUID}) + assert.NoError(t, err) + + attach, err := GetAttachmentByUUID(t.Context(), legacyUUID) + assert.NoError(t, err) + assert.EqualValues(t, 1, attach.RepoID) + assert.EqualValues(t, 1, attach.ReleaseID) +} + +func TestAddReleaseAttachmentsRejectsRecentZeroRepoID(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + recentUUID := "a0eebc99-9c0b-4ef8-bb6d-6bb9bd3800aa" + attachment := &Attachment{ + UUID: recentUUID, + RepoID: 0, + IssueID: 0, + ReleaseID: 0, + CommentID: 0, + Name: "recent-zero", + CreatedUnix: LegacyAttachmentMissingRepoIDCutoff + 1, + } + assert.NoError(t, db.Insert(t.Context(), attachment)) + + err := AddReleaseAttachments(t.Context(), 1, []string{recentUUID}) + assert.Error(t, err) + assert.ErrorIs(t, err, util.ErrPermissionDenied) + + attach, err := GetAttachmentByUUID(t.Context(), recentUUID) + assert.NoError(t, err) + assert.Zero(t, attach.ReleaseID) + assert.Zero(t, attach.RepoID) +} diff --git a/routers/web/repo/attachment.go b/routers/web/repo/attachment.go index c8501792ce..ae52eb2ffa 100644 --- a/routers/web/repo/attachment.go +++ b/routers/web/repo/attachment.go @@ -133,14 +133,15 @@ func ServeAttachment(ctx *context.Context, uuid string) { } // prevent visiting attachment from other repository directly - if ctx.Repo.Repository != nil && ctx.Repo.Repository.ID != attach.RepoID { + // The check will be ignored before this code merged. + if attach.CreatedUnix > repo_model.LegacyAttachmentMissingRepoIDCutoff && ctx.Repo.Repository != nil && ctx.Repo.Repository.ID != attach.RepoID { ctx.HTTPError(http.StatusNotFound) return } - unitType, err := repo_service.GetAttachmentLinkedType(ctx, attach) + unitType, repoID, err := repo_service.GetAttachmentLinkedTypeAndRepoID(ctx, attach) if err != nil { - ctx.ServerError("GetAttachmentLinkedType", err) + ctx.ServerError("GetAttachmentLinkedTypeAndRepoID", err) return } @@ -152,7 +153,7 @@ func ServeAttachment(ctx *context.Context, uuid string) { } else { // If we have the linked type, we need to check access var perm access_model.Permission if ctx.Repo.Repository == nil { - repo, err := repo_model.GetRepositoryByID(ctx, attach.RepoID) + repo, err := repo_model.GetRepositoryByID(ctx, repoID) if err != nil { ctx.ServerError("GetRepositoryByID", err) return diff --git a/services/migrations/gitea_uploader.go b/services/migrations/gitea_uploader.go index f32c095d76..335c0fb294 100644 --- a/services/migrations/gitea_uploader.go +++ b/services/migrations/gitea_uploader.go @@ -318,6 +318,7 @@ func (g *GiteaLocalUploader) CreateReleases(ctx context.Context, releases ...*ba } attach := repo_model.Attachment{ UUID: uuid.New().String(), + RepoID: g.repo.ID, Name: asset.Name, DownloadCount: int64(*asset.DownloadCount), Size: int64(*asset.Size), diff --git a/services/repository/repository.go b/services/repository/repository.go index 318ab423e5..ae64f0116a 100644 --- a/services/repository/repository.go +++ b/services/repository/repository.go @@ -221,25 +221,28 @@ func MakeRepoPrivate(ctx context.Context, repo *repo_model.Repository) (err erro }) } -// GetAttachmentLinkedType returns the linked type of attachment if any -func GetAttachmentLinkedType(ctx context.Context, a *repo_model.Attachment) (unit.Type, error) { +// GetAttachmentLinkedTypeAndRepoID returns the linked type and repository id of attachment if any +func GetAttachmentLinkedTypeAndRepoID(ctx context.Context, a *repo_model.Attachment) (unit.Type, int64, error) { if a.IssueID != 0 { iss, err := issues_model.GetIssueByID(ctx, a.IssueID) if err != nil { - return unit.TypeIssues, err + return unit.TypeIssues, 0, err } unitType := unit.TypeIssues if iss.IsPull { unitType = unit.TypePullRequests } - return unitType, nil + return unitType, iss.RepoID, nil } if a.ReleaseID != 0 { - _, err := repo_model.GetReleaseByID(ctx, a.ReleaseID) - return unit.TypeReleases, err + rel, err := repo_model.GetReleaseByID(ctx, a.ReleaseID) + if err != nil { + return unit.TypeReleases, 0, err + } + return unit.TypeReleases, rel.RepoID, nil } - return unit.TypeInvalid, nil + return unit.TypeInvalid, 0, nil } // CheckDaemonExportOK creates/removes git-daemon-export-ok for git-daemon... diff --git a/services/repository/repository_test.go b/services/repository/repository_test.go index b3447ae166..395138b5e4 100644 --- a/services/repository/repository_test.go +++ b/services/repository/repository_test.go @@ -16,25 +16,27 @@ import ( "github.com/stretchr/testify/require" ) -func TestAttachLinkedType(t *testing.T) { +func TestAttachLinkedTypeAndRepoID(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) testCases := []struct { name string attachID int64 expectedUnitType unit.Type + expectedRepoID uint64 }{ - {"LinkedIssue", 1, unit.TypeIssues}, - {"LinkedComment", 3, unit.TypePullRequests}, - {"LinkedRelease", 9, unit.TypeReleases}, - {"Notlinked", 10, unit.TypeInvalid}, + {"LinkedIssue", 1, unit.TypeIssues, 1}, + {"LinkedComment", 3, unit.TypePullRequests, 1}, + {"LinkedRelease", 9, unit.TypeReleases, 1}, + {"Notlinked", 10, unit.TypeInvalid, 0}, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { attach, err := repo_model.GetAttachmentByID(t.Context(), tc.attachID) assert.NoError(t, err) - unitType, err := GetAttachmentLinkedType(t.Context(), attach) + unitType, repoID, err := GetAttachmentLinkedTypeAndRepoID(t.Context(), attach) assert.NoError(t, err) assert.Equal(t, tc.expectedUnitType, unitType) + assert.Equal(t, tc.expectedRepoID, repoID) }) } }