Include file extension checks in attachment API (#32151)

From testing, I found that issue posters and users with repository write
access are able to edit attachment names in a way that circumvents the
instance-level file extension restrictions using the edit attachment
APIs. This snapshot adds checks for these endpoints.

3 files changed, 83 insertions, 2 deletions

diff --git a/tests/integration/api_comment_attachment_test.go b/tests/integration/api_comment_attachment_test.go
index 0ec950d4c2..623467938a 100644
--- a/tests/integration/api_comment_attachment_test.go
+++ b/tests/integration/api_comment_attachment_test.go
@@ -151,7 +151,7 @@ func TestAPICreateCommentAttachmentWithUnallowedFile(t *testing.T) {
 func TestAPIEditCommentAttachment(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
 
-	const newAttachmentName = "newAttachmentName"
+	const newAttachmentName = "newAttachmentName.txt"
 
 	attachment := unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: 6})
 	comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: attachment.CommentID})
@@ -173,6 +173,27 @@ func TestAPIEditCommentAttachment(t *testing.T) {
 	unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: apiAttachment.ID, CommentID: comment.ID, Name: apiAttachment.Name})
 }
 
+func TestAPIEditCommentAttachmentWithUnallowedFile(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+
+	attachment := unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: 6})
+	comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: attachment.CommentID})
+	issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: comment.IssueID})
+	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: issue.RepoID})
+	repoOwner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
+	session := loginUser(t, repoOwner.Name)
+	token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteIssue)
+
+	filename := "file.bad"
+	urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/comments/%d/assets/%d",
+		repoOwner.Name, repo.Name, comment.ID, attachment.ID)
+	req := NewRequestWithValues(t, "PATCH", urlStr, map[string]string{
+		"name": filename,
+	}).AddTokenAuth(token)
+
+	session.MakeRequest(t, req, http.StatusUnprocessableEntity)
+}
+
 func TestAPIDeleteCommentAttachment(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
 
diff --git a/tests/integration/api_issue_attachment_test.go b/tests/integration/api_issue_attachment_test.go
index b4196ec6db..6806d27df2 100644
--- a/tests/integration/api_issue_attachment_test.go
+++ b/tests/integration/api_issue_attachment_test.go
@@ -126,7 +126,7 @@ func TestAPICreateIssueAttachmentWithUnallowedFile(t *testing.T) {
 func TestAPIEditIssueAttachment(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
 
-	const newAttachmentName = "newAttachmentName"
+	const newAttachmentName = "hello_world.txt"
 
 	attachment := unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: 1})
 	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: attachment.RepoID})
@@ -147,6 +147,26 @@ func TestAPIEditIssueAttachment(t *testing.T) {
 	unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: apiAttachment.ID, IssueID: issue.ID, Name: apiAttachment.Name})
 }
 
+func TestAPIEditIssueAttachmentWithUnallowedFile(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+
+	attachment := unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: 1})
+	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: attachment.RepoID})
+	issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: attachment.IssueID})
+	repoOwner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
+
+	session := loginUser(t, repoOwner.Name)
+	token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteIssue)
+
+	filename := "file.bad"
+	urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/%d/assets/%d", repoOwner.Name, repo.Name, issue.Index, attachment.ID)
+	req := NewRequestWithValues(t, "PATCH", urlStr, map[string]string{
+		"name": filename,
+	}).AddTokenAuth(token)
+
+	session.MakeRequest(t, req, http.StatusUnprocessableEntity)
+}
+
 func TestAPIDeleteIssueAttachment(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
 
diff --git a/tests/integration/api_releases_attachment_test.go b/tests/integration/api_releases_attachment_test.go
new file mode 100644
index 0000000000..5df3042437
--- /dev/null
+++ b/tests/integration/api_releases_attachment_test.go
@@ -0,0 +1,40 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package integration
+
+import (
+	"fmt"
+	"net/http"
+	"testing"
+
+	auth_model "code.gitea.io/gitea/models/auth"
+	repo_model "code.gitea.io/gitea/models/repo"
+	"code.gitea.io/gitea/models/unittest"
+	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/test"
+	"code.gitea.io/gitea/tests"
+)
+
+func TestAPIEditReleaseAttachmentWithUnallowedFile(t *testing.T) {
+	// Limit the allowed release types (since by default there is no restriction)
+	defer test.MockVariableValue(&setting.Repository.Release.AllowedTypes, ".exe")()
+	defer tests.PrepareTestEnv(t)()
+
+	attachment := unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: 9})
+	release := unittest.AssertExistsAndLoadBean(t, &repo_model.Release{ID: attachment.ReleaseID})
+	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: attachment.RepoID})
+	repoOwner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
+
+	session := loginUser(t, repoOwner.Name)
+	token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)
+
+	filename := "file.bad"
+	urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/releases/%d/assets/%d", repoOwner.Name, repo.Name, release.ID, attachment.ID)
+	req := NewRequestWithValues(t, "PATCH", urlStr, map[string]string{
+		"name": filename,
+	}).AddTokenAuth(token)
+
+	session.MakeRequest(t, req, http.StatusUnprocessableEntity)
+}