From 0964106988bb430ad5f1a2ec2a36431421811fc7 Mon Sep 17 00:00:00 2001 From: silverwind Date: Fri, 30 Oct 2026 08:08:32 +0100 Subject: [PATCH] fix: add missing access checks and size limits (#39942) (#40085) Backport https://github.com/go-gitea/gitea/pull/39942 Several handlers and parsers skipped checks that their siblings already perform. - Reject issue, comment, label, milestone and release mutations in archived repositories via the API with one guard per route group, matching the web UI - Hide timeline references and dependencies pointing at issues the doer cannot read, in the web UI and the API, including pull requests of the same repository without pull request access - Reject workflow runs with more than `MaxJobNumPerRun` jobs, including inline reusable workflow jobs, and show rejected pushes as invalid workflow runs - Bound how much of a Helm chart archive is inflated while looking for `Chart.yaml` `release/v28` lacks two helpers this needs, so they are included: `insertInvalidWorkflowRun`, split out of `handleInvalidWorkflows` by https://github.com/go-gitea/gitea/pull/39480, and `packages.NewLimitedReader` from https://github.com/go-gitea/gitea/pull/39366. Co-authored-by: bircni Co-authored-by: wxiaoguang --- models/actions/run_job.go | 1 - modules/packages/helm/metadata.go | 5 +- modules/packages/limited_reader.go | 29 ++++++++ routers/api/v1/api.go | 48 +++++++------ routers/api/v1/repo/issue_comment.go | 29 ++------ routers/web/repo/issue_view.go | 37 +--------- services/actions/invalid_workflow.go | 72 ++++++++++--------- services/actions/max_parallel_test.go | 29 +++++++- services/actions/notifier_helper.go | 6 ++ services/actions/rerun.go | 3 + services/actions/reusable_workflow.go | 9 ++- services/actions/run.go | 8 +++ services/issue/comments.go | 53 ++++++++++++++ services/issue/comments_test.go | 34 +++++++++ services/migrations/onedev.go | 13 ++-- services/migrations/onedev_test.go | 7 -- .../integration/api_issue_dependency_test.go | 30 ++++++++ tests/integration/api_issue_test.go | 21 ++++++ 18 files changed, 298 insertions(+), 136 deletions(-) create mode 100644 modules/packages/limited_reader.go create mode 100644 services/issue/comments_test.go diff --git a/models/actions/run_job.go b/models/actions/run_job.go index 4f770270684..7834c3dafba 100644 --- a/models/actions/run_job.go +++ b/models/actions/run_job.go @@ -25,7 +25,6 @@ import ( // MaxJobNumPerRun is the maximum number of jobs in a single run. // https://docs.github.com/en/actions/reference/limits#existing-system-limits -// TODO: check this limit when creating jobs const MaxJobNumPerRun = 256 // ActionRunJob represents a job of a run diff --git a/modules/packages/helm/metadata.go b/modules/packages/helm/metadata.go index 3bb6e86e5ec..3dc9951c9ef 100644 --- a/modules/packages/helm/metadata.go +++ b/modules/packages/helm/metadata.go @@ -10,6 +10,7 @@ import ( "io" "strings" + "gitea.dev/modules/packages" "gitea.dev/modules/util" "gitea.dev/modules/validation" @@ -69,6 +70,8 @@ type Dependency struct { Alias string `json:"alias,omitempty" yaml:"alias,omitempty"` } +const maxArchiveSize = 256 * 1024 * 1024 // tar members are inflated while looking for Chart.yaml + // ParseChartArchive parses the metadata of a Helm archive func ParseChartArchive(r io.Reader) (*Metadata, error) { gzr, err := gzip.NewReader(r) @@ -77,7 +80,7 @@ func ParseChartArchive(r io.Reader) (*Metadata, error) { } defer gzr.Close() - tr := tar.NewReader(gzr) + tr := tar.NewReader(packages.NewLimitedReader(gzr, maxArchiveSize)) for { hd, err := tr.Next() if err == io.EOF { diff --git a/modules/packages/limited_reader.go b/modules/packages/limited_reader.go new file mode 100644 index 00000000000..24b0ba717bd --- /dev/null +++ b/modules/packages/limited_reader.go @@ -0,0 +1,29 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package packages + +import ( + "io" + + "gitea.dev/modules/util" +) + +var ErrContentTooLarge = util.NewInvalidArgumentErrorf("content exceeds the size limit") + +type limitedReader struct { + io.LimitedReader +} + +// NewLimitedReader returns a reader that fails with ErrContentTooLarge once it reads a byte past limit +func NewLimitedReader(r io.Reader, limit int64) io.Reader { + return &limitedReader{io.LimitedReader{R: r, N: limit + 1}} +} + +func (l *limitedReader) Read(p []byte) (int, error) { + n, err := l.LimitedReader.Read(p) + if l.N == 0 { + return max(n-1, 0), ErrContentTooLarge + } + return n, err +} diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 0f558547f53..924917f251f 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -879,6 +879,10 @@ func mustEnableRepoProjects(ctx *context.APIContext) { // FIXME: for consistency, maybe most mustNotBeArchived checks should be replaced with mustEnableEditor func mustNotBeArchived(ctx *context.APIContext) { + switch ctx.Req.Method { + case http.MethodGet, http.MethodHead, http.MethodOptions: + return + } if ctx.Repo.Repository.IsArchived { ctx.APIError(http.StatusLocked, "repo is archived") return @@ -1487,7 +1491,7 @@ func Routes() *web.Router { Get(repo.GetReleaseByTag). Delete(reqToken(), reqRepoWriter(unit.TypeReleases), repo.DeleteReleaseByTag) }) - }, reqRepoReader(unit.TypeReleases)) + }, reqRepoReader(unit.TypeReleases), mustNotBeArchived) m.Post("/mirror-sync", reqToken(), reqRepoWriter(unit.TypeCode), mustNotBeArchived, repo.MirrorSync) m.Post("/push_mirrors-sync", reqAdmin(), reqToken(), mustNotBeArchived, repo.PushMirrorSync) m.Group("/push_mirrors", func() { @@ -1634,14 +1638,14 @@ func Routes() *web.Router { m.Group("/{username}/{reponame}", func() { m.Group("/issues", func() { m.Combo("").Get(repo.ListIssues). - Post(reqToken(), mustNotBeArchived, bind(api.CreateIssueOption{}), reqRepoReader(unit.TypeIssues), repo.CreateIssue) + Post(reqToken(), bind(api.CreateIssueOption{}), reqRepoReader(unit.TypeIssues), repo.CreateIssue) m.Get("/pinned", reqRepoReader(unit.TypeIssues), repo.ListPinnedIssues) m.Group("/comments", func() { m.Get("", repo.ListRepoIssueComments) m.Group("/{id}", func() { m.Combo(""). Get(repo.GetIssueComment). - Patch(mustNotBeArchived, reqToken(), bind(api.EditIssueCommentOption{}), repo.EditIssueComment). + Patch(reqToken(), bind(api.EditIssueCommentOption{}), repo.EditIssueComment). Delete(reqToken(), repo.DeleteIssueComment) m.Combo("/reactions"). Get(repo.GetIssueCommentReactions). @@ -1650,35 +1654,35 @@ func Routes() *web.Router { m.Group("/assets", func() { m.Combo(""). Get(repo.ListIssueCommentAttachments). - Post(reqToken(), mustNotBeArchived, repo.CreateIssueCommentAttachment) + Post(reqToken(), repo.CreateIssueCommentAttachment) m.Combo("/{attachment_id}"). Get(repo.GetIssueCommentAttachment). - Patch(reqToken(), mustNotBeArchived, bind(api.EditAttachmentOptions{}), repo.EditIssueCommentAttachment). - Delete(reqToken(), mustNotBeArchived, repo.DeleteIssueCommentAttachment) + Patch(reqToken(), bind(api.EditAttachmentOptions{}), repo.EditIssueCommentAttachment). + Delete(reqToken(), repo.DeleteIssueCommentAttachment) }, mustEnableAttachments) }) }) m.Group("/{index}", func() { m.Combo("").Get(repo.GetIssue). - Patch(reqToken(), mustNotBeArchived, bind(api.EditIssueOption{}), repo.EditIssue). - Delete(reqToken(), reqAdmin(), mustNotBeArchived, context.ReferencesGitRepo(), repo.DeleteIssue) + Patch(reqToken(), bind(api.EditIssueOption{}), repo.EditIssue). + Delete(reqToken(), reqAdmin(), context.ReferencesGitRepo(), repo.DeleteIssue) m.Combo("/assignees"). - Post(reqToken(), mustNotBeArchived, bind(api.IssueAssigneesOption{}), repo.AddIssueAssignees). - Delete(reqToken(), mustNotBeArchived, bind(api.IssueAssigneesOption{}), repo.DeleteIssueAssignees) + Post(reqToken(), bind(api.IssueAssigneesOption{}), repo.AddIssueAssignees). + Delete(reqToken(), bind(api.IssueAssigneesOption{}), repo.DeleteIssueAssignees) m.Get("/assignees/{assignee}", repo.CheckIssueAssignee) m.Group("/comments", func() { m.Combo("").Get(repo.ListIssueComments). - Post(reqToken(), mustNotBeArchived, bind(api.CreateIssueCommentOption{}), repo.CreateIssueComment) + Post(reqToken(), bind(api.CreateIssueCommentOption{}), repo.CreateIssueComment) m.Combo("/{id}", reqToken()).Patch(bind(api.EditIssueCommentOption{}), repo.EditIssueCommentDeprecated). Delete(repo.DeleteIssueCommentDeprecated) }) m.Get("/timeline", repo.ListIssueCommentsAndTimeline) m.Group("/labels", func() { m.Combo("").Get(repo.ListIssueLabels). - Post(reqToken(), mustNotBeArchived, bind(api.IssueLabelsOption{}), repo.AddIssueLabels). - Put(reqToken(), mustNotBeArchived, bind(api.IssueLabelsOption{}), repo.ReplaceIssueLabels). - Delete(reqToken(), mustNotBeArchived, repo.ClearIssueLabels) - m.Delete("/{id}", reqToken(), mustNotBeArchived, repo.DeleteIssueLabel) + Post(reqToken(), bind(api.IssueLabelsOption{}), repo.AddIssueLabels). + Put(reqToken(), bind(api.IssueLabelsOption{}), repo.ReplaceIssueLabels). + Delete(reqToken(), repo.ClearIssueLabels) + m.Delete("/{id}", reqToken(), repo.DeleteIssueLabel) }) m.Group("/times", func() { m.Combo(""). @@ -1706,16 +1710,16 @@ func Routes() *web.Router { m.Group("/assets", func() { m.Combo(""). Get(repo.ListIssueAttachments). - Post(reqToken(), mustNotBeArchived, repo.CreateIssueAttachment) + Post(reqToken(), repo.CreateIssueAttachment) m.Combo("/{attachment_id}"). Get(repo.GetIssueAttachment). - Patch(reqToken(), mustNotBeArchived, bind(api.EditAttachmentOptions{}), repo.EditIssueAttachment). - Delete(reqToken(), mustNotBeArchived, repo.DeleteIssueAttachment) + Patch(reqToken(), bind(api.EditAttachmentOptions{}), repo.EditIssueAttachment). + Delete(reqToken(), repo.DeleteIssueAttachment) }, mustEnableAttachments) m.Combo("/dependencies"). Get(repo.GetIssueDependencies). - Post(reqToken(), mustNotBeArchived, bind(api.IssueMeta{}), repo.CreateIssueDependency). - Delete(reqToken(), mustNotBeArchived, bind(api.IssueMeta{}), repo.RemoveIssueDependency) + Post(reqToken(), bind(api.IssueMeta{}), repo.CreateIssueDependency). + Delete(reqToken(), bind(api.IssueMeta{}), repo.RemoveIssueDependency) m.Combo("/blocks"). Get(repo.GetIssueBlocks). Post(reqToken(), bind(api.IssueMeta{}), repo.CreateIssueBlocking). @@ -1748,9 +1752,9 @@ func Routes() *web.Router { Delete(reqToken(), reqRepoWriter(unit.TypeIssues, unit.TypePullRequests), repo.DeleteMilestone) }) m.Group("/projects", func() { - addProjectRoutes(m, reqToken(), reqRepoWriter(unit.TypeProjects), mustNotBeArchived) + addProjectRoutes(m, reqToken(), reqRepoWriter(unit.TypeProjects)) }, reqRepoReader(unit.TypeProjects), mustEnableRepoProjects) - }, repoAssignment(), checkTokenPublicOnly()) + }, repoAssignment(), checkTokenPublicOnly(), mustNotBeArchived) }, tokenRequiresScopes(auth_model.AccessTokenScopeCategoryIssue)) // NOTE: these are Gitea package management API - see packages.CommonRoutes and packages.DockerContainerRoutes for endpoints that implement package manager APIs diff --git a/routers/api/v1/repo/issue_comment.go b/routers/api/v1/repo/issue_comment.go index 974d8872398..eb98ee43f56 100644 --- a/routers/api/v1/repo/issue_comment.go +++ b/routers/api/v1/repo/issue_comment.go @@ -5,14 +5,11 @@ package repo import ( - stdCtx "context" "errors" "net/http" issues_model "gitea.dev/models/issues" "gitea.dev/models/perm" - access_model "gitea.dev/models/perm/access" - repo_model "gitea.dev/models/repo" "gitea.dev/models/unit" user_model "gitea.dev/models/user" "gitea.dev/modules/optional" @@ -189,10 +186,14 @@ func ListIssueCommentsAndTimeline(ctx *context.APIContext) { ctx.APIErrorInternal(err) return } + if comments, err = issue_service.FilterReadableComments(ctx, ctx.Doer, ctx.Repo.Permission, issue, comments); err != nil { + ctx.APIErrorInternal(err) + return + } var apiComments []*api.TimelineComment for _, comment := range comments { - if comment.Type != issues_model.CommentTypeCode && isXRefCommentAccessible(ctx, ctx.Doer, comment, issue.RepoID) { + if comment.Type != issues_model.CommentTypeCode { comment.Issue = issue apiComments = append(apiComments, convert.ToTimelineComment(ctx, issue.Repo, comment, ctx.Doer)) } @@ -202,26 +203,6 @@ func ListIssueCommentsAndTimeline(ctx *context.APIContext) { ctx.JSON(http.StatusOK, &apiComments) } -func isXRefCommentAccessible(ctx stdCtx.Context, user *user_model.User, c *issues_model.Comment, issueRepoID int64) bool { - // Remove comments that the user has no permissions to see - if issues_model.CommentTypeIsRef(c.Type) && c.RefRepoID != issueRepoID && c.RefRepoID != 0 { - var err error - // Set RefRepo for description in template - c.RefRepo, err = repo_model.GetRepositoryByID(ctx, c.RefRepoID) - if err != nil { - return false - } - perm, err := access_model.GetDoerRepoPermission(ctx, c.RefRepo, user) - if err != nil { - return false - } - if !perm.CanReadIssuesOrPulls(c.RefIsPull) { - return false - } - } - return true -} - // ListRepoIssueComments returns all issue-comments for a repo func ListRepoIssueComments(ctx *context.APIContext) { // swagger:operation GET /repos/{owner}/{repo}/issues/comments issue issueGetRepoComments diff --git a/routers/web/repo/issue_view.go b/routers/web/repo/issue_view.go index 06d55f938e2..66625572900 100644 --- a/routers/web/repo/issue_view.go +++ b/routers/web/repo/issue_view.go @@ -165,31 +165,6 @@ func addParticipant(poster *user_model.User, participants []*user_model.User) [] return append(participants, poster) } -func filterXRefComments(ctx *context.Context, issue *issues_model.Issue) error { - // Remove comments that the user has no permissions to see - for i := 0; i < len(issue.Comments); { - c := issue.Comments[i] - if issues_model.CommentTypeIsRef(c.Type) && c.RefRepoID != issue.RepoID && c.RefRepoID != 0 { - var err error - // Set RefRepo for description in template - c.RefRepo, err = repo_model.GetRepositoryByID(ctx, c.RefRepoID) - if err != nil { - return err - } - perm, err := access_model.GetDoerRepoPermission(ctx, c.RefRepo, ctx.Doer) - if err != nil { - return err - } - if !perm.CanReadIssuesOrPulls(c.RefIsPull) { - issue.Comments = append(issue.Comments[:i], issue.Comments[i+1:]...) - continue - } - } - i++ - } - return nil -} - // combineXRefComments keeps only the first reference from each issue, carrying over the action of later ones func combineXRefComments(issue *issues_model.Issue) { first := make(map[int64]*issues_model.Comment) @@ -369,8 +344,9 @@ func ViewIssue(ctx *context.Context) { return } - if err := filterXRefComments(ctx, issue); err != nil { - ctx.ServerError("filterXRefComments", err) + var err error + if issue.Comments, err = issue_service.FilterReadableComments(ctx, ctx.Doer, ctx.Repo.Permission, issue, issue.Comments); err != nil { + ctx.ServerError("FilterReadableComments", err) return } combineXRefComments(issue) @@ -753,13 +729,6 @@ func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue ctx.ServerError("LoadAssigneeUserAndTeam", err) return } - } else if comment.Type == issues_model.CommentTypeRemoveDependency || comment.Type == issues_model.CommentTypeAddDependency { - if err = comment.LoadDepIssueDetails(ctx); err != nil { - if !issues_model.IsErrIssueNotExist(err) { - ctx.ServerError("LoadDepIssueDetails", err) - return - } - } } else if comment.Type.HasContentSupport() { rctx := renderhelper.NewRenderContextRepoComment(ctx, issue.Repo, renderhelper.RepoCommentOptions{ FootnoteContextID: strconv.FormatInt(comment.ID, 10), diff --git a/services/actions/invalid_workflow.go b/services/actions/invalid_workflow.go index fc211fbdcbc..56edb0a9776 100644 --- a/services/actions/invalid_workflow.go +++ b/services/actions/invalid_workflow.go @@ -32,40 +32,46 @@ func handleInvalidWorkflows(ctx context.Context, input *notifyInput, ref git.Ref if actionsConfig.IsWorkflowDisabled(entryName) { continue } - now := timeutil.TimeStampNow() - run := &actions_model.ActionRun{ - Title: util.EllipsisDisplayString(commit.MessageTitle(), 255), RepoID: input.Repo.ID, Repo: input.Repo, OwnerID: input.Repo.OwnerID, + insertInvalidWorkflowRun(ctx, &actions_model.ActionRun{ + Title: commit.MessageTitle(), RepoID: input.Repo.ID, Repo: input.Repo, OwnerID: input.Repo.OwnerID, WorkflowID: entryName, TriggerUserID: input.Doer.ID, TriggerUser: input.Doer, Ref: ref.String(), CommitSHA: commit.ID.String(), Event: input.Event, TriggerEvent: string(input.Event), EventPayload: string(payload), - WorkflowRepoID: input.Repo.ID, WorkflowCommitSHA: commit.ID.String(), Status: actions_model.StatusFailure, Started: now, Stopped: now, - } - if err := db.WithTx(ctx, func(ctx context.Context) error { - if run.Index, err = db.GetNextResourceIndex(ctx, "action_run_index", run.RepoID); err != nil { - return err - } - if err := db.Insert(ctx, run); err != nil { - return err - } - attempt := &actions_model.ActionRunAttempt{RepoID: run.RepoID, RunID: run.ID, Attempt: 1, TriggerUserID: run.TriggerUserID, Status: run.Status, Started: now, Stopped: now} - if err := db.Insert(ctx, attempt); err != nil { - return err - } - run.LatestAttemptID = attempt.ID - if err := actions_model.UpdateRun(ctx, run, "latest_attempt_id"); err != nil { - return err - } - content := fmt.Sprintf("**Invalid workflow file: %s**\n\n```\n%v\n```\n", entryName, parseErr) - return db.Insert(ctx, &actions_model.ActionRunJobSummary{ - RepoID: run.RepoID, RunID: run.ID, RunAttemptID: attempt.ID, Content: content, ContentSize: int64(len(content)), ContentType: actions_model.JobSummaryContentTypeMarkdown, - }) - }); err != nil { - log.Error("insert run for invalid workflow %q: %v", entryName, err) - continue - } - if err := createWorkflowCommitStatus(ctx, run.Repo, run.CommitSHA, entryName+" ("+run.TriggerEvent+")", run.WorkflowID, - commitstatus.CommitStatusFailure, run.Link(), "Invalid workflow file", false); err != nil { - log.Error("create commit status for invalid workflow %q: %v", entryName, err) - } - NotifyWorkflowRunStatusUpdate(ctx, run) + WorkflowRepoID: input.Repo.ID, WorkflowCommitSHA: commit.ID.String(), + }, parseErr) } } + +// insertInvalidWorkflowRun records run as failed with parseErr as its summary. +func insertInvalidWorkflowRun(ctx context.Context, run *actions_model.ActionRun, parseErr error) { + now := timeutil.TimeStampNow() + run.Title = util.EllipsisDisplayString(run.Title, 255) + run.Status, run.Started, run.Stopped = actions_model.StatusFailure, now, now + if err := db.WithTx(ctx, func(ctx context.Context) (err error) { + if run.Index, err = db.GetNextResourceIndex(ctx, "action_run_index", run.RepoID); err != nil { + return err + } + if err := db.Insert(ctx, run); err != nil { + return err + } + attempt := &actions_model.ActionRunAttempt{RepoID: run.RepoID, RunID: run.ID, Attempt: 1, TriggerUserID: run.TriggerUserID, Status: run.Status, Started: now, Stopped: now} + if err := db.Insert(ctx, attempt); err != nil { + return err + } + run.LatestAttemptID = attempt.ID + if err := actions_model.UpdateRun(ctx, run, "latest_attempt_id"); err != nil { + return err + } + content := fmt.Sprintf("**Invalid workflow file: %s**\n\n```\n%v\n```\n", run.WorkflowID, parseErr) + return db.Insert(ctx, &actions_model.ActionRunJobSummary{ + RepoID: run.RepoID, RunID: run.ID, RunAttemptID: attempt.ID, Content: content, ContentSize: int64(len(content)), ContentType: actions_model.JobSummaryContentTypeMarkdown, + }) + }); err != nil { + log.Error("insert run for invalid workflow %q: %v", run.WorkflowID, err) + return + } + if err := createWorkflowCommitStatus(ctx, run.Repo, run.CommitSHA, run.WorkflowID+" ("+run.TriggerEvent+")", run.WorkflowID, + commitstatus.CommitStatusFailure, run.Link(), "Invalid workflow file", false); err != nil { + log.Error("create commit status for invalid workflow %q: %v", run.WorkflowID, err) + } + NotifyWorkflowRunStatusUpdate(ctx, run) +} diff --git a/services/actions/max_parallel_test.go b/services/actions/max_parallel_test.go index a39e7ce709e..23f5f390bbe 100644 --- a/services/actions/max_parallel_test.go +++ b/services/actions/max_parallel_test.go @@ -4,7 +4,9 @@ package actions import ( + "fmt" "slices" + "strings" "testing" actions_model "gitea.dev/models/actions" @@ -13,6 +15,7 @@ import ( "gitea.dev/models/unittest" user_model "gitea.dev/models/user" "gitea.dev/modules/test" + "gitea.dev/modules/util" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -53,9 +56,8 @@ jobs: - run: echo hi ` -func insertMaxParallelRun(t *testing.T, workflow string, needApproval bool) *actions_model.ActionRun { - t.Helper() - run := &actions_model.ActionRun{ +func newMaxParallelRun(needApproval bool) *actions_model.ActionRun { + return &actions_model.ActionRun{ Title: "max-parallel", RepoID: 4, OwnerID: 1, @@ -70,6 +72,11 @@ func insertMaxParallelRun(t *testing.T, workflow string, needApproval bool) *act WorkflowRepoID: 4, WorkflowCommitSHA: "c2d72f548424103f01ee1dc02889c1e2bff816b0", } +} + +func insertMaxParallelRun(t *testing.T, workflow string, needApproval bool) *actions_model.ActionRun { + t.Helper() + run := newMaxParallelRun(needApproval) require.NoError(t, PrepareRunAndInsert(t.Context(), []byte(workflow), run, nil)) return run } @@ -304,3 +311,19 @@ func insertConcurrencyHolder(t *testing.T, index int64, group string) *actions_m require.NoError(t, db.Insert(ctx, holder)) return holder } + +func TestPrepareRunAndInsert_MaxJobNumPerRun(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + workflow := func(jobCount int) []byte { + var sb strings.Builder + sb.WriteString("on: push\njobs:\n") + for i := range jobCount { + fmt.Fprintf(&sb, " job%d:\n runs-on: ubuntu-latest\n steps:\n - run: echo\n", i) + } + return []byte(sb.String()) + } + require.NoError(t, PrepareRunAndInsert(t.Context(), workflow(actions_model.MaxJobNumPerRun), newMaxParallelRun(false), nil)) + err := PrepareRunAndInsert(t.Context(), workflow(actions_model.MaxJobNumPerRun+1), newMaxParallelRun(false), nil) + assert.ErrorIs(t, err, util.ErrInvalidArgument) + assert.ErrorContains(t, err, "exceeds the maximum") +} diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index 7bfee941963..eb28ade29ba 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -5,6 +5,7 @@ package actions import ( "context" + "errors" "fmt" "slices" "strings" @@ -25,6 +26,7 @@ import ( "gitea.dev/modules/log" "gitea.dev/modules/setting" api "gitea.dev/modules/structs" + "gitea.dev/modules/util" webhook_module "gitea.dev/modules/webhook" "gitea.dev/services/convert" ) @@ -410,6 +412,10 @@ func buildApproveAndInsertRun( } if err := PrepareRunAndInsert(ctx, dwf.Content, run, nil); err != nil { + if errors.Is(err, util.ErrInvalidArgument) && !isScopedRun { + insertInvalidWorkflowRun(ctx, run, err) + return nil + } return fmt.Errorf("PrepareRunAndInsert: %w", err) } return nil diff --git a/services/actions/rerun.go b/services/actions/rerun.go index 59d63f83dff..19b331652a6 100644 --- a/services/actions/rerun.go +++ b/services/actions/rerun.go @@ -352,6 +352,9 @@ func execRerunPlan(ctx context.Context, plan *rerunPlan) (*actions_model.ActionR hasWaitingJobs = hasWaitingJobs || (newJob.Status == actions_model.StatusWaiting && !newJob.IsReusableCaller) newJobs = append(newJobs, newJob) } + if err := checkRunJobLimit(ctx, plan.run.ID, newAttempt.ID, 0); err != nil { // inline reusable expansions only counted the jobs inserted before them + return err + } // Refresh each ancestor's status from its now-fresh children. // `newJobs` is appended top-down (caller before its children), so we walk it in reverse to refresh the deepest ancestor first. diff --git a/services/actions/reusable_workflow.go b/services/actions/reusable_workflow.go index 1ac89705a56..c9cd13c0e43 100644 --- a/services/actions/reusable_workflow.go +++ b/services/actions/reusable_workflow.go @@ -35,7 +35,7 @@ import ( // MaxReusableCallLevels allows nine calls across ten workflows, including the top-level workflow. const MaxReusableCallLevels = 8 -// checkRunJobLimit rejects an expansion that would push the attempt over actions_model.MaxJobNumPerRun. +// checkRunJobLimit rejects adding jobs that would push the attempt over actions_model.MaxJobNumPerRun. // checkCallerChain bounds nesting *depth*, but a reusable graph also fans out in *breadth*: without a // cumulative cap a tiny set of files can drive exponential job-row insertion and exhaust the database. func checkRunJobLimit(ctx context.Context, runID, attemptID int64, adding int) error { @@ -44,11 +44,16 @@ func checkRunJobLimit(ctx context.Context, runID, attemptID int64, adding int) e return fmt.Errorf("count existing jobs of run %d attempt %d: %w", runID, attemptID, err) } if existing+int64(adding) > actions_model.MaxJobNumPerRun { - return fmt.Errorf("workflow run exceeds the maximum of %d jobs", actions_model.MaxJobNumPerRun) + return runJobLimitError() } return nil } +func runJobLimitError() error { + err := util.NewInvalidArgumentErrorf("workflow run exceeds the maximum of %d jobs", actions_model.MaxJobNumPerRun) + return util.ErrorWrapTranslatable(err, "actions.runs.invalid_workflow_helper", err.Error()) +} + // loadReusableWorkflowSource resolves the workflow file referenced by a caller's `uses:` and returns its raw bytes, // along with the (repo_id, commit_sha) the file was loaded from. func loadReusableWorkflowSource(ctx context.Context, run *actions_model.ActionRun, caller *actions_model.ActionRunJob, ref *model.ReusableWorkflowUses) (content []byte, sourceRepoID int64, sourceCommitSHA string, err error) { diff --git a/services/actions/run.go b/services/actions/run.go index 38a069151f8..d0957694863 100644 --- a/services/actions/run.go +++ b/services/actions/run.go @@ -61,6 +61,7 @@ func PrepareRunAndInsert(ctx context.Context, content []byte, run *actions_model func InsertRun(ctx context.Context, run *actions_model.ActionRun, content []byte, vars map[string]string, inputs map[string]any, wfRawConcurrency *act_model.RawConcurrency) error { var cancelledConcurrencyJobs []*actions_model.ActionRunJob var needPostCommitEmit bool + pending := *run if err := db.WithTx(ctx, func(ctx context.Context) error { index, err := db.GetNextResourceIndex(ctx, "action_run_index", run.RepoID) if err != nil { @@ -120,6 +121,9 @@ func InsertRun(ctx context.Context, run *actions_model.ActionRun, content []byte if err != nil { return fmt.Errorf("parse workflow: %w", err) } + if len(jobs) > actions_model.MaxJobNumPerRun { + return runJobLimitError() + } titleChanged := len(jobs) > 0 && jobs[0].RunName != "" if titleChanged { run.Title = util.EllipsisDisplayString(jobs[0].RunName, 255) @@ -149,6 +153,9 @@ func InsertRun(ctx context.Context, run *actions_model.ActionRun, content []byte hasWaitingJobs = hasWaitingJobs || (runJob.Status == actions_model.StatusWaiting && !runJob.IsReusableCaller) runJobs = append(runJobs, runJob) } + if err := checkRunJobLimit(ctx, run.ID, runAttempt.ID, 0); err != nil { // inline reusable expansions only counted the jobs inserted before them + return err + } runAttempt.Status = actions_model.AggregateJobStatus(runJobs) if err := actions_model.UpdateRunAttempt(ctx, runAttempt, "status"); err != nil { @@ -164,6 +171,7 @@ func InsertRun(ctx context.Context, run *actions_model.ActionRun, content []byte return nil }); err != nil { + *run = pending // drop the ID and index of the rolled back insert return err } diff --git a/services/issue/comments.go b/services/issue/comments.go index 649f4bfd180..325e50a67ae 100644 --- a/services/issue/comments.go +++ b/services/issue/comments.go @@ -13,6 +13,7 @@ import ( access_model "gitea.dev/models/perm/access" repo_model "gitea.dev/models/repo" user_model "gitea.dev/models/user" + "gitea.dev/modules/cache" "gitea.dev/modules/git" "gitea.dev/modules/json" "gitea.dev/modules/log" @@ -196,3 +197,55 @@ func LoadCommentPushCommits(ctx context.Context, c *issues_model.Comment) error return nil } + +// FilterReadableComments drops comments pointing at issues doer cannot read and loads RefRepo of the rest +func FilterReadableComments(ctx context.Context, doer *user_model.User, perm access_model.Permission, issue *issues_model.Issue, comments issues_model.CommentList) (issues_model.CommentList, error) { + type refTarget struct { + repo *repo_model.Repository + perm access_model.Permission + } + targets := cache.NewEphemeralCache() + targets.Put("ref-target", issue.RepoID, refTarget{issue.Repo, perm}) + getRepoPerm := func(refRepoID int64) (refTarget, error) { + return cache.GetWithEphemeralCache(ctx, targets, "ref-target", refRepoID, func(ctx context.Context, repoID int64) (refTarget, error) { + repo, err := repo_model.GetRepositoryByID(ctx, repoID) + if err != nil { + return refTarget{}, err + } + repoPerm, err := access_model.GetDoerRepoPermission(ctx, repo, doer) + return refTarget{repo, repoPerm}, err + }) + } + + readable := comments[:0] + for _, c := range comments { + var refRepoID int64 + var refIsPull bool + switch { + case issues_model.CommentTypeIsRef(c.Type): + refRepoID, refIsPull = c.RefRepoID, c.RefIsPull + case c.Type == issues_model.CommentTypeAddDependency || c.Type == issues_model.CommentTypeRemoveDependency: + if err := c.LoadDepIssueDetails(ctx); err != nil && !issues_model.IsErrIssueNotExist(err) { + return nil, err + } + if c.DependentIssue != nil { + refRepoID, refIsPull = c.DependentIssue.RepoID, c.DependentIssue.IsPull + } + } + if refRepoID != 0 { + target, err := getRepoPerm(refRepoID) + if err != nil { + return nil, err + } + if !target.perm.CanReadIssuesOrPulls(refIsPull) { + continue + } + c.RefRepo = target.repo + if c.DependentIssue != nil { + c.DependentIssue.Repo = target.repo + } + } + readable = append(readable, c) + } + return readable, nil +} diff --git a/services/issue/comments_test.go b/services/issue/comments_test.go new file mode 100644 index 00000000000..cadf3f92d57 --- /dev/null +++ b/services/issue/comments_test.go @@ -0,0 +1,34 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package issue + +import ( + "testing" + + issues_model "gitea.dev/models/issues" + perm_model "gitea.dev/models/perm" + access_model "gitea.dev/models/perm/access" + repo_model "gitea.dev/models/repo" + "gitea.dev/models/unit" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFilterReadableCommentsChecksUnitOfSameRepoTargets(t *testing.T) { + var perm access_model.Permission + perm.SetUnitsWithDefaultAccessMode([]*repo_model.RepoUnit{{Type: unit.TypeIssues}}, perm_model.AccessModeRead) + issueRef := &issues_model.Comment{Type: issues_model.CommentTypeIssueRef, RefRepoID: 1} + pullRef := &issues_model.Comment{Type: issues_model.CommentTypePullRef, RefRepoID: 1, RefIsPull: true} + issueDependency := &issues_model.Comment{Type: issues_model.CommentTypeAddDependency, DependentIssue: &issues_model.Issue{RepoID: 1}} + pullDependency := &issues_model.Comment{Type: issues_model.CommentTypeAddDependency, DependentIssue: &issues_model.Issue{RepoID: 1, IsPull: true}} + repo := &repo_model.Repository{ID: 1} + + comments, err := FilterReadableComments(t.Context(), nil, perm, &issues_model.Issue{RepoID: 1, Repo: repo}, issues_model.CommentList{issueRef, pullRef, issueDependency, pullDependency}) + require.NoError(t, err) + assert.Equal(t, issues_model.CommentList{issueRef, issueDependency}, comments) + assert.Same(t, repo, issueRef.RefRepo) + assert.Same(t, repo, issueDependency.RefRepo) + assert.Same(t, repo, issueDependency.DependentIssue.Repo) +} diff --git a/services/migrations/onedev.go b/services/migrations/onedev.go index cf6a80f73aa..4da12a084bf 100644 --- a/services/migrations/onedev.go +++ b/services/migrations/onedev.go @@ -22,10 +22,7 @@ import ( "github.com/hashicorp/go-version" ) -const ( - OneDevRequiredVersion = "12.0.1" - maxOneDevVersionResponseSize = 1024 -) +const OneDevRequiredVersion = "12.0.1" var ( _ base.Downloader = &OneDevDownloader{} @@ -159,14 +156,12 @@ func (d *OneDevDownloader) getServerVersionByEndpoint(ctx context.Context, endpo if resp.StatusCode != http.StatusOK { return nil, fmt.Errorf("OneDev server version request failed: %s", resp.Status) } - // This patch doesn't change the situation, "decoder.Decode(&result)" in callAPI still reads the entire body - bytes, err := io.ReadAll(io.LimitReader(resp.Body, maxOneDevVersionResponseSize+1)) + // This patch doesn't change the situation, "decoder.Decode(&result)" in callAPI still reads the entire body. + // Also, the methods like GetMilestones can read plenty of responses into memory even if each single is small. + bytes, err := io.ReadAll(io.LimitReader(resp.Body, 1024)) if err != nil { return nil, err } - if len(bytes) > maxOneDevVersionResponseSize { - return nil, fmt.Errorf("OneDev server version response exceeds %d bytes", maxOneDevVersionResponseSize) - } ver, err := version.NewVersion(string(bytes)) if err != nil { return nil, fmt.Errorf("failed to parse OneDev server version %q: %w", string(bytes), err) diff --git a/services/migrations/onedev_test.go b/services/migrations/onedev_test.go index 581dfe37cd3..32d566b9560 100644 --- a/services/migrations/onedev_test.go +++ b/services/migrations/onedev_test.go @@ -11,7 +11,6 @@ import ( "os" "path/filepath" "runtime" - "strings" "sync/atomic" "testing" "time" @@ -40,12 +39,6 @@ func TestOneDevServerVersion(t *testing.T) { assert.NoError(t, err) downloader := func() *OneDevDownloader { return &OneDevDownloader{baseURL: baseURL, client: server.Client()} } - t.Run("LargeResponse", func(t *testing.T) { - respMap.Store(&respMapType{"/~api/version/server": {code: http.StatusOK, body: strings.Repeat("x", maxOneDevVersionResponseSize+1)}}) - _, err := downloader().getServerVersion(t.Context()) - assert.ErrorContains(t, err, "server version response exceeds") - }) - t.Run("LegacyEndpoint", func(t *testing.T) { respMap.Store(&respMapType{"/~api/version/server": {code: http.StatusOK, body: "12.0.1"}}) d := downloader() diff --git a/tests/integration/api_issue_dependency_test.go b/tests/integration/api_issue_dependency_test.go index 5685c217baf..b3464a8d7e3 100644 --- a/tests/integration/api_issue_dependency_test.go +++ b/tests/integration/api_issue_dependency_test.go @@ -6,6 +6,7 @@ package integration import ( "fmt" "net/http" + "slices" "strconv" "testing" @@ -189,3 +190,32 @@ func TestWebDeleteIssueDependencyCrossRepoPermission(t *testing.T) { session.MakeRequest(t, req, http.StatusOK) unittest.AssertNotExistsBean(t, &issues_model.IssueDependency{IssueID: targetIssue.ID, DependencyID: dependencyIssue.ID}) } + +func TestIssueTimelineDependencyCrossRepoPermission(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + targetIssue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{RepoID: 1, Index: 1}) + dependencyRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) + assert.True(t, dependencyRepo.IsPrivate) + dependencyIssue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{RepoID: dependencyRepo.ID, Index: 1}) + enableRepoDependencies(t, targetIssue.RepoID) + enableRepoDependencies(t, dependencyRepo.ID) + user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + assert.NoError(t, issues_model.CreateIssueDependency(t.Context(), user1, targetIssue, dependencyIssue)) + + session := loginUser(t, "user40") + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadIssue) + assertDependencyVisible := func(visible bool) { + t.Helper() + req := NewRequest(t, "GET", "/api/v1/repos/user2/repo1/issues/1/timeline").AddTokenAuth(token) + comments := DecodeJSON(t, MakeRequest(t, req, http.StatusOK), []*api.TimelineComment{}) + assert.Equal(t, visible, slices.ContainsFunc(comments, func(c *api.TimelineComment) bool { return c.Type == "add_dependency" })) + resp := session.MakeRequest(t, NewRequest(t, "GET", "/user2/repo1/issues/1"), http.StatusOK) + assert.Equal(t, visible, NewHTMLParser(t, resp.Body).Find(`.timeline-item a[href$="/org3/repo3/issues/1"]`).Length() > 0) + } + + assertDependencyVisible(false) + user40 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 40}) + assert.NoError(t, repo_service.AddOrUpdateCollaborator(t.Context(), dependencyRepo, user40, perm.AccessModeRead)) + assertDependencyVisible(true) +} diff --git a/tests/integration/api_issue_test.go b/tests/integration/api_issue_test.go index 1d687fa1323..c4051375026 100644 --- a/tests/integration/api_issue_test.go +++ b/tests/integration/api_issue_test.go @@ -8,6 +8,7 @@ import ( "net/http" "net/url" "strconv" + "strings" "sync" "testing" "time" @@ -648,3 +649,23 @@ func testAPIIssueProjects(t *testing.T) { }).AddTokenAuth(token) MakeRequest(t, req, http.StatusBadRequest) } + +func TestAPIArchivedRepoMutations(t *testing.T) { + defer tests.PrepareTestEnv(t)() + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + assert.NoError(t, repo_model.SetArchiveRepoState(t.Context(), repo, true)) + token := getUserToken(t, "user2", auth_model.AccessTokenScopeWriteIssue, auth_model.AccessTokenScopeWriteRepository) + for route, status := range map[string]int{ + "GET /issues/1/reactions": http.StatusOK, + "POST /issues/1/reactions": http.StatusLocked, + "DELETE /issues/comments/1": http.StatusLocked, + "DELETE /labels/1": http.StatusLocked, + "GET /releases/1": http.StatusOK, + "DELETE /releases/1": http.StatusLocked, + } { + t.Run(route, func(t *testing.T) { + method, path, _ := strings.Cut(route, " ") + MakeRequest(t, NewRequest(t, method, "/api/v1/repos/user2/repo1"+path).AddTokenAuth(token), status) + }) + } +}