Fix missing repository id when migrating release attachments (#36389) (#36413)

This PR fixes missed repo_id on the migration of attachments to Gitea.
It also provides a doctor check to fix the dirty data on the database.

Backport #36389
This commit is contained in:
Lunny Xiao
2026-01-20 13:36:45 -08:00
committed by GitHub
parent 38125a8d1d
commit 4cdb8a7f96
6 changed files with 74 additions and 17 deletions

View File

@@ -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")
}

View File

@@ -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)
}

View File

@@ -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

View File

@@ -320,6 +320,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),

View File

@@ -224,25 +224,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...

View File

@@ -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 int64
}{
{"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)
})
}
}