From 421cfba3cfb46081c9b8c19d6049f1ad9d947b9d Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 27 Jul 2025 10:17:33 +0200 Subject: [PATCH] fix: return error when user is not repo writer (#8690) - If the doer isn't a issue/pull writer, return a error. - Fixes a panic (NPE), because the callers of `prepareForReplaceOrAdd` simply checked if there was a error returned to see if the user was allowed. It didn't check if a statuscode was written. This is specifically a issue when the automatic token by Forgejo actions is used. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8690 Reviewed-by: Earl Warren Co-authored-by: Gusted Co-committed-by: Gusted --- routers/api/v1/repo/issue_label.go | 2 +- tests/integration/api_issue_label_test.go | 25 +++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/routers/api/v1/repo/issue_label.go b/routers/api/v1/repo/issue_label.go index 3b2935305c..f2e79ea417 100644 --- a/routers/api/v1/repo/issue_label.go +++ b/routers/api/v1/repo/issue_label.go @@ -384,7 +384,7 @@ func prepareForReplaceOrAdd(ctx *context.APIContext, form api.IssueLabelsOption) if !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) { ctx.Status(http.StatusForbidden) - return nil, nil, nil + return nil, nil, errors.New("not issue/pull writer") } err = issue_service.SetIssueUpdateDate(ctx, issue, form.Updated, ctx.Doer) diff --git a/tests/integration/api_issue_label_test.go b/tests/integration/api_issue_label_test.go index 160774a7e5..665e4b2f2a 100644 --- a/tests/integration/api_issue_label_test.go +++ b/tests/integration/api_issue_label_test.go @@ -336,3 +336,28 @@ func TestAPIModifyOrgLabels(t *testing.T) { AddTokenAuth(token) MakeRequest(t, req, http.StatusNoContent) } + +func TestAPIReplaceIssueLabelsActionsToken(t *testing.T) { + require.NoError(t, unittest.LoadFixtures()) + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{RepoID: repo.ID}) + label := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{RepoID: repo.ID}) + owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + + task := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: 47}) + task.RepoID = repo.ID + task.OwnerID = owner.ID + task.IsForkPullRequest = true // Read permission. + require.NoError(t, task.GenerateToken()) + + // Explicitly need "is_fork_pull_request". + require.NoError(t, actions_model.UpdateTask(t.Context(), task, "repo_id", "owner_id", "is_fork_pull_request", "token", "token_salt", "token_hash", "token_last_eight")) + + urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/%d/labels", + owner.Name, repo.Name, issue.Index) + req := NewRequestWithJSON(t, "PUT", urlStr, &api.IssueLabelsOption{ + Labels: []any{label.Name}, + }).AddTokenAuth(task.Token) + MakeRequest(t, req, http.StatusForbidden) +}