From 3a806a58d0913e11b155ac096a34c34ba6b44e76 Mon Sep 17 00:00:00 2001 From: bircni Date: Mon, 24 Aug 2026 19:32:23 +0200 Subject: [PATCH] fix(attachments): enforce owning repository path (#39048) Reject attachment requests routed through a repository other than the attachment owner. --------- Co-authored-by: silverwind --- routers/web/repo/attachment.go | 14 +++++++------- tests/integration/attachment_test.go | 20 ++++++++++++++++++++ 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/routers/web/repo/attachment.go b/routers/web/repo/attachment.go index 5e0e4b62c14..1fa46b0c189 100644 --- a/routers/web/repo/attachment.go +++ b/routers/web/repo/attachment.go @@ -143,18 +143,18 @@ func ServeAttachment(ctx *context.Context, uuid string) { return } - // prevent visiting attachment from other repository directly - // 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, repoID, err := repo_service.GetAttachmentLinkedTypeAndRepoID(ctx, attach) if err != nil { ctx.ServerError("GetAttachmentLinkedTypeAndRepoID", err) return } + if repoID == 0 { + repoID = attach.RepoID + } + if ctx.Repo.Repository != nil && repoID != 0 && ctx.Repo.Repository.ID != repoID { + ctx.HTTPError(http.StatusNotFound) + return + } if unitType == unit.TypeInvalid { // unlinked attachment can only be accessed by the uploader if !(ctx.IsSigned && attach.UploaderID == ctx.Doer.ID) { // We block if not the uploader diff --git a/tests/integration/attachment_test.go b/tests/integration/attachment_test.go index f118f119574..155d182e22f 100644 --- a/tests/integration/attachment_test.go +++ b/tests/integration/attachment_test.go @@ -189,6 +189,26 @@ func testGetAttachment(t *testing.T) { tc.session.MakeRequest(t, req, tc.want) }) } + + attachment, err := repo_model.GetAttachmentByUUID(t.Context(), "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a12") + require.NoError(t, err) + defer func() { + attachment.RepoID = 2 + require.NoError(t, repo_model.UpdateAttachmentByUUID(t.Context(), attachment, "repo_id")) + }() + for _, testCase := range []struct { + name string + repoID int64 + }{ + {"RecordedRepository", 2}, + {"LegacyMissingRepository", 0}, + } { + t.Run("OtherRepositoryPath/"+testCase.name, func(t *testing.T) { + attachment.RepoID = testCase.repoID + require.NoError(t, repo_model.UpdateAttachmentByUUID(t.Context(), attachment, "repo_id")) + MakeRequest(t, NewRequest(t, "GET", "/user2/repo1/attachments/"+attachment.UUID), http.StatusNotFound) + }) + } } func testDeleteAttachmentPermissions(t *testing.T) {