From 0dbc6230286e113accbc6d5e829ce8dae1d1f5d4 Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Sun, 28 Jul 2024 23:11:40 +0800 Subject: [PATCH 01/24] Hide the "Details" link of commit status when the user cannot access actions (#30156) Fix #26685 If a commit status comes from Gitea Actions and the user cannot access the repo's actions unit (the user does not have the permission or the actions unit is disabled), a 404 page will occur after clicking the "Details" link. We should hide the "Details" link in this case. (cherry picked from commit 7dec8de9147b20c014d68bb1020afe28a263b95a) Conflicts: routers/web/repo/commit.go trivial context commit --- models/git/commit_status.go | 40 +++++++++++++++++++++++++++++++- models/git/commit_status_test.go | 25 ++++++++++++++++++++ routers/web/repo/branch.go | 5 ++++ routers/web/repo/commit.go | 21 ++++++++++++++--- routers/web/repo/compare.go | 2 +- routers/web/repo/issue.go | 11 +++++++++ routers/web/repo/pull.go | 14 ++++++++++- routers/web/repo/repo.go | 3 +++ routers/web/repo/view.go | 3 +++ routers/web/user/home.go | 6 +++++ routers/web/user/notification.go | 7 ++++++ 11 files changed, 131 insertions(+), 6 deletions(-) diff --git a/models/git/commit_status.go b/models/git/commit_status.go index d975f0572c..76870f9eb1 100644 --- a/models/git/commit_status.go +++ b/models/git/commit_status.go @@ -141,13 +141,17 @@ func GetNextCommitStatusIndex(ctx context.Context, repoID int64, sha string) (in return newIdx, nil } -func (status *CommitStatus) loadAttributes(ctx context.Context) (err error) { +func (status *CommitStatus) loadRepository(ctx context.Context) (err error) { if status.Repo == nil { status.Repo, err = repo_model.GetRepositoryByID(ctx, status.RepoID) if err != nil { return fmt.Errorf("getRepositoryByID [%d]: %w", status.RepoID, err) } } + return nil +} + +func (status *CommitStatus) loadCreator(ctx context.Context) (err error) { if status.Creator == nil && status.CreatorID > 0 { status.Creator, err = user_model.GetUserByID(ctx, status.CreatorID) if err != nil { @@ -157,6 +161,13 @@ func (status *CommitStatus) loadAttributes(ctx context.Context) (err error) { return nil } +func (status *CommitStatus) loadAttributes(ctx context.Context) (err error) { + if err := status.loadRepository(ctx); err != nil { + return err + } + return status.loadCreator(ctx) +} + // APIURL returns the absolute APIURL to this commit-status. func (status *CommitStatus) APIURL(ctx context.Context) string { _ = status.loadAttributes(ctx) @@ -168,6 +179,21 @@ func (status *CommitStatus) LocaleString(lang translation.Locale) string { return lang.TrString("repo.commitstatus." + status.State.String()) } +// HideActionsURL set `TargetURL` to an empty string if the status comes from Gitea Actions +func (status *CommitStatus) HideActionsURL(ctx context.Context) { + if status.Repo == nil { + if err := status.loadRepository(ctx); err != nil { + log.Error("loadRepository: %v", err) + return + } + } + + prefix := fmt.Sprintf("%s/actions", status.Repo.Link()) + if strings.HasPrefix(status.TargetURL, prefix) { + status.TargetURL = "" + } +} + // CalcCommitStatus returns commit status state via some status, the commit statues should order by id desc func CalcCommitStatus(statuses []*CommitStatus) *CommitStatus { if len(statuses) == 0 { @@ -471,3 +497,15 @@ func ConvertFromGitCommit(ctx context.Context, commits []*git.Commit, repo *repo repo, ) } + +// CommitStatusesHideActionsURL hide Gitea Actions urls +func CommitStatusesHideActionsURL(ctx context.Context, statuses []*CommitStatus) { + idToRepos := make(map[int64]*repo_model.Repository) + for _, status := range statuses { + if status.Repo == nil { + status.Repo = idToRepos[status.RepoID] + } + status.HideActionsURL(ctx) + idToRepos[status.RepoID] = status.Repo + } +} diff --git a/models/git/commit_status_test.go b/models/git/commit_status_test.go index 07b9031c5c..bff3d3dccf 100644 --- a/models/git/commit_status_test.go +++ b/models/git/commit_status_test.go @@ -4,9 +4,11 @@ package git_test import ( + "fmt" "testing" "time" + actions_model "code.gitea.io/gitea/models/actions" "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" repo_model "code.gitea.io/gitea/models/repo" @@ -240,3 +242,26 @@ func TestFindRepoRecentCommitStatusContexts(t *testing.T) { assert.Equal(t, "compliance/lint-backend", contexts[0]) } } + +func TestCommitStatusesHideActionsURL(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) + run := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: 791, RepoID: repo.ID}) + assert.NoError(t, run.LoadAttributes(db.DefaultContext)) + + statuses := []*git_model.CommitStatus{ + { + RepoID: repo.ID, + TargetURL: fmt.Sprintf("%s/jobs/%d", run.Link(), run.Index), + }, + { + RepoID: repo.ID, + TargetURL: "https://mycicd.org/1", + }, + } + + git_model.CommitStatusesHideActionsURL(db.DefaultContext, statuses) + assert.Empty(t, statuses[0].TargetURL) + assert.Equal(t, "https://mycicd.org/1", statuses[1].TargetURL) +} diff --git a/routers/web/repo/branch.go b/routers/web/repo/branch.go index f879a98786..4897a5f4fc 100644 --- a/routers/web/repo/branch.go +++ b/routers/web/repo/branch.go @@ -70,6 +70,11 @@ func Branches(ctx *context.Context) { ctx.ServerError("LoadBranches", err) return } + if !ctx.Repo.CanRead(unit.TypeActions) { + for key := range commitStatuses { + git_model.CommitStatusesHideActionsURL(ctx, commitStatuses[key]) + } + } commitStatus := make(map[string]*git_model.CommitStatus) for commitID, cs := range commitStatuses { diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index bf9a448ec5..3da8b1506c 100644 --- a/routers/web/repo/commit.go +++ b/routers/web/repo/commit.go @@ -16,6 +16,7 @@ import ( "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" repo_model "code.gitea.io/gitea/models/repo" + unit_model "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/charset" @@ -81,7 +82,7 @@ func Commits(ctx *context.Context) { ctx.ServerError("CommitsByRange", err) return } - ctx.Data["Commits"] = git_model.ConvertFromGitCommit(ctx, commits, ctx.Repo.Repository) + ctx.Data["Commits"] = processGitCommits(ctx, commits) ctx.Data["Username"] = ctx.Repo.Owner.Name ctx.Data["Reponame"] = ctx.Repo.Repository.Name @@ -199,7 +200,7 @@ func SearchCommits(ctx *context.Context) { return } ctx.Data["CommitCount"] = len(commits) - ctx.Data["Commits"] = git_model.ConvertFromGitCommit(ctx, commits, ctx.Repo.Repository) + ctx.Data["Commits"] = processGitCommits(ctx, commits) ctx.Data["Keyword"] = query if all { @@ -264,7 +265,7 @@ func FileHistory(ctx *context.Context) { } } - ctx.Data["Commits"] = git_model.ConvertFromGitCommit(ctx, commits, ctx.Repo.Repository) + ctx.Data["Commits"] = processGitCommits(ctx, commits) ctx.Data["Username"] = ctx.Repo.Owner.Name ctx.Data["Reponame"] = ctx.Repo.Repository.Name @@ -375,6 +376,9 @@ func Diff(ctx *context.Context) { if err != nil { log.Error("GetLatestCommitStatus: %v", err) } + if !ctx.Repo.CanRead(unit_model.TypeActions) { + git_model.CommitStatusesHideActionsURL(ctx, statuses) + } ctx.Data["CommitStatus"] = git_model.CalcCommitStatus(statuses) ctx.Data["CommitStatuses"] = statuses @@ -454,3 +458,14 @@ func RawDiff(ctx *context.Context) { return } } + +func processGitCommits(ctx *context.Context, gitCommits []*git.Commit) []*git_model.SignCommitWithStatuses { + commits := git_model.ConvertFromGitCommit(ctx, gitCommits, ctx.Repo.Repository) + if !ctx.Repo.CanRead(unit_model.TypeActions) { + for _, commit := range commits { + commit.Status.HideActionsURL(ctx) + git_model.CommitStatusesHideActionsURL(ctx, commit.Statuses) + } + } + return commits +} diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index 088e5150f6..38d6004ec6 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -643,7 +643,7 @@ func PrepareCompareDiff( return false } - commits := git_model.ConvertFromGitCommit(ctx, ci.CompareInfo.Commits, ci.HeadRepo) + commits := processGitCommits(ctx, ci.CompareInfo.Commits) ctx.Data["Commits"] = commits ctx.Data["CommitCount"] = len(commits) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index dcc1cdd467..b48b078736 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -346,6 +346,11 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption opt ctx.ServerError("GetIssuesAllCommitStatus", err) return } + if !ctx.Repo.CanRead(unit.TypeActions) { + for key := range commitStatuses { + git_model.CommitStatusesHideActionsURL(ctx, commitStatuses[key]) + } + } if err := issues.LoadAttributes(ctx); err != nil { ctx.ServerError("issues.LoadAttributes", err) @@ -1777,6 +1782,12 @@ func ViewIssue(ctx *context.Context) { ctx.ServerError("LoadPushCommits", err) return } + if !ctx.Repo.CanRead(unit.TypeActions) { + for _, commit := range comment.Commits { + commit.Status.HideActionsURL(ctx) + git_model.CommitStatusesHideActionsURL(ctx, commit.Statuses) + } + } } else if comment.Type == issues_model.CommentTypeAddTimeManual || comment.Type == issues_model.CommentTypeStopTracking || comment.Type == issues_model.CommentTypeDeleteTimeManual { diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index aa1f506483..a9213790cb 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -515,6 +515,10 @@ func PrepareMergedViewPullInfo(ctx *context.Context, issue *issues_model.Issue) ctx.ServerError("GetLatestCommitStatus", err) return nil } + if !ctx.Repo.CanRead(unit.TypeActions) { + git_model.CommitStatusesHideActionsURL(ctx, commitStatuses) + } + if len(commitStatuses) != 0 { ctx.Data["LatestCommitStatuses"] = commitStatuses ctx.Data["LatestCommitStatus"] = git_model.CalcCommitStatus(commitStatuses) @@ -577,6 +581,10 @@ func PrepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C ctx.ServerError("GetLatestCommitStatus", err) return nil } + if !ctx.Repo.CanRead(unit.TypeActions) { + git_model.CommitStatusesHideActionsURL(ctx, commitStatuses) + } + if len(commitStatuses) > 0 { ctx.Data["LatestCommitStatuses"] = commitStatuses ctx.Data["LatestCommitStatus"] = git_model.CalcCommitStatus(commitStatuses) @@ -669,6 +677,10 @@ func PrepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C ctx.ServerError("GetLatestCommitStatus", err) return nil } + if !ctx.Repo.CanRead(unit.TypeActions) { + git_model.CommitStatusesHideActionsURL(ctx, commitStatuses) + } + if len(commitStatuses) > 0 { ctx.Data["LatestCommitStatuses"] = commitStatuses ctx.Data["LatestCommitStatus"] = git_model.CalcCommitStatus(commitStatuses) @@ -835,7 +847,7 @@ func ViewPullCommits(ctx *context.Context) { ctx.Data["Username"] = ctx.Repo.Owner.Name ctx.Data["Reponame"] = ctx.Repo.Repository.Name - commits := git_model.ConvertFromGitCommit(ctx, prInfo.Commits, ctx.Repo.Repository) + commits := processGitCommits(ctx, prInfo.Commits) ctx.Data["Commits"] = commits ctx.Data["CommitCount"] = len(commits) diff --git a/routers/web/repo/repo.go b/routers/web/repo/repo.go index 7e20d3afaa..652738afda 100644 --- a/routers/web/repo/repo.go +++ b/routers/web/repo/repo.go @@ -683,6 +683,9 @@ func SearchRepo(ctx *context.Context) { ctx.JSON(http.StatusInternalServerError, nil) return } + if !ctx.Repo.CanRead(unit.TypeActions) { + git_model.CommitStatusesHideActionsURL(ctx, latestCommitStatuses) + } results := make([]*repo_service.WebSearchRepository, len(repos)) for i, repo := range repos { diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index 5295bfdb2a..caa4bfae1b 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -368,6 +368,9 @@ func loadLatestCommitData(ctx *context.Context, latestCommit *git.Commit) bool { if err != nil { log.Error("GetLatestCommitStatus: %v", err) } + if !ctx.Repo.CanRead(unit_model.TypeActions) { + git_model.CommitStatusesHideActionsURL(ctx, statuses) + } ctx.Data["LatestCommitStatus"] = git_model.CalcCommitStatus(statuses) ctx.Data["LatestCommitStatuses"] = statuses diff --git a/routers/web/user/home.go b/routers/web/user/home.go index df22c3fb8d..0a1b08c57b 100644 --- a/routers/web/user/home.go +++ b/routers/web/user/home.go @@ -17,6 +17,7 @@ import ( activities_model "code.gitea.io/gitea/models/activities" asymkey_model "code.gitea.io/gitea/models/asymkey" "code.gitea.io/gitea/models/db" + git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" "code.gitea.io/gitea/models/organization" repo_model "code.gitea.io/gitea/models/repo" @@ -597,6 +598,11 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { ctx.ServerError("GetIssuesLastCommitStatus", err) return } + if !ctx.Repo.CanRead(unit.TypeActions) { + for key := range commitStatuses { + git_model.CommitStatusesHideActionsURL(ctx, commitStatuses[key]) + } + } // ------------------------------- // Fill stats to post to ctx.Data. diff --git a/routers/web/user/notification.go b/routers/web/user/notification.go index 2105cfe5c5..f8b68fb18e 100644 --- a/routers/web/user/notification.go +++ b/routers/web/user/notification.go @@ -13,8 +13,10 @@ import ( activities_model "code.gitea.io/gitea/models/activities" "code.gitea.io/gitea/models/db" + git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/optional" @@ -303,6 +305,11 @@ func NotificationSubscriptions(ctx *context.Context) { ctx.ServerError("GetIssuesAllCommitStatus", err) return } + if !ctx.Repo.CanRead(unit.TypeActions) { + for key := range commitStatuses { + git_model.CommitStatusesHideActionsURL(ctx, commitStatuses[key]) + } + } ctx.Data["CommitLastStatus"] = lastStatus ctx.Data["CommitStatuses"] = commitStatuses ctx.Data["Issues"] = issues From c8e5e3986583d886f1eb333fff48b4d35de488ac Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Sun, 4 Aug 2024 08:54:32 +0200 Subject: [PATCH 02/24] Hide the "Details" link of commit status when the user cannot access actions (testifylint) --- models/git/commit_status_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/git/commit_status_test.go b/models/git/commit_status_test.go index bff3d3dccf..1014ee1e13 100644 --- a/models/git/commit_status_test.go +++ b/models/git/commit_status_test.go @@ -244,11 +244,11 @@ func TestFindRepoRecentCommitStatusContexts(t *testing.T) { } func TestCommitStatusesHideActionsURL(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) + require.NoError(t, unittest.PrepareTestDatabase()) repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) run := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: 791, RepoID: repo.ID}) - assert.NoError(t, run.LoadAttributes(db.DefaultContext)) + require.NoError(t, run.LoadAttributes(db.DefaultContext)) statuses := []*git_model.CommitStatus{ { From 170c1c5152175d3b308392dafce54b2df5e0f37b Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Sun, 4 Aug 2024 09:30:36 +0200 Subject: [PATCH 03/24] Hide the "Details" link of commit status when the user cannot access actions (followup) commit.Status may be nil --- routers/web/repo/commit.go | 3 +++ routers/web/repo/issue.go | 3 +++ 2 files changed, 6 insertions(+) diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index 3da8b1506c..ec9e49eb9c 100644 --- a/routers/web/repo/commit.go +++ b/routers/web/repo/commit.go @@ -463,6 +463,9 @@ func processGitCommits(ctx *context.Context, gitCommits []*git.Commit) []*git_mo commits := git_model.ConvertFromGitCommit(ctx, gitCommits, ctx.Repo.Repository) if !ctx.Repo.CanRead(unit_model.TypeActions) { for _, commit := range commits { + if commit.Status == nil { + continue + } commit.Status.HideActionsURL(ctx) git_model.CommitStatusesHideActionsURL(ctx, commit.Statuses) } diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index b48b078736..5a0dcd830f 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1784,6 +1784,9 @@ func ViewIssue(ctx *context.Context) { } if !ctx.Repo.CanRead(unit.TypeActions) { for _, commit := range comment.Commits { + if commit.Status == nil { + continue + } commit.Status.HideActionsURL(ctx) git_model.CommitStatusesHideActionsURL(ctx, commit.Statuses) } From 7850fa30a5cb66faa76324fe87a923a5720a6456 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 29 Jul 2024 09:32:54 +0800 Subject: [PATCH 04/24] Make GetRepositoryByName more safer (#31712) Fix #31708 (cherry picked from commit d109923ed8e58bce0ad26b47385edbc79403803d) --- models/repo/repo.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/models/repo/repo.go b/models/repo/repo.go index 6db7c30513..cd6be48b90 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -766,17 +766,18 @@ func GetRepositoryByOwnerAndName(ctx context.Context, ownerName, repoName string // GetRepositoryByName returns the repository by given name under user if exists. func GetRepositoryByName(ctx context.Context, ownerID int64, name string) (*Repository, error) { - repo := &Repository{ - OwnerID: ownerID, - LowerName: strings.ToLower(name), - } - has, err := db.GetEngine(ctx).Get(repo) + var repo Repository + has, err := db.GetEngine(ctx). + Where("`owner_id`=?", ownerID). + And("`lower_name`=?", strings.ToLower(name)). + NoAutoCondition(). + Get(&repo) if err != nil { return nil, err } else if !has { return nil, ErrRepoNotExist{0, ownerID, "", name} } - return repo, err + return &repo, err } // getRepositoryURLPathSegments returns segments (owner, reponame) extracted from a url From 2be49a37453e0668840a40de59f920c15cad83ef Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Mon, 29 Jul 2024 15:51:02 +0900 Subject: [PATCH 05/24] Fix loadRepository error when access user dashboard (#31719) (cherry picked from commit 7b388630ecb4537f9bb04e55cbb10eb7cf83b9c5) --- models/git/commit_status.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/models/git/commit_status.go b/models/git/commit_status.go index 76870f9eb1..422cf0b4ee 100644 --- a/models/git/commit_status.go +++ b/models/git/commit_status.go @@ -181,6 +181,10 @@ func (status *CommitStatus) LocaleString(lang translation.Locale) string { // HideActionsURL set `TargetURL` to an empty string if the status comes from Gitea Actions func (status *CommitStatus) HideActionsURL(ctx context.Context) { + if status.RepoID == 0 { + return + } + if status.Repo == nil { if err := status.loadRepository(ctx); err != nil { log.Error("loadRepository: %v", err) From 6e63afe31f43eaf5ff7c8595ddeaf8515c2dc0c0 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Tue, 30 Jul 2024 00:45:24 +0800 Subject: [PATCH 06/24] Fix API endpoint for registration-token (#31722) Partially fix #31707. Related to #30656 (cherry picked from commit bf5ae79c5163b8dd6a3185711ad11893b1270f62) --- routers/api/v1/repo/action.go | 2 +- templates/swagger/v1_json.tmpl | 66 +++++++++++++++++----------------- 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index 72232d66ef..d7a49a78be 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -486,7 +486,7 @@ func (Action) ListVariables(ctx *context.APIContext) { // GetRegistrationToken returns the token to register repo runners func (Action) GetRegistrationToken(ctx *context.APIContext) { - // swagger:operation GET /repos/{owner}/{repo}/runners/registration-token repository repoGetRunnerRegistrationToken + // swagger:operation GET /repos/{owner}/{repo}/actions/runners/registration-token repository repoGetRunnerRegistrationToken // --- // summary: Get a repository's actions runner registration token // produces: diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 39b92f4e79..3379e3f4dc 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -4564,6 +4564,39 @@ } } }, + "/repos/{owner}/{repo}/actions/runners/registration-token": { + "get": { + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "Get a repository's actions runner registration token", + "operationId": "repoGetRunnerRegistrationToken", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "$ref": "#/responses/RegistrationToken" + } + } + } + }, "/repos/{owner}/{repo}/actions/secrets": { "get": { "produces": [ @@ -14705,39 +14738,6 @@ } } }, - "/repos/{owner}/{repo}/runners/registration-token": { - "get": { - "produces": [ - "application/json" - ], - "tags": [ - "repository" - ], - "summary": "Get a repository's actions runner registration token", - "operationId": "repoGetRunnerRegistrationToken", - "parameters": [ - { - "type": "string", - "description": "owner of the repo", - "name": "owner", - "in": "path", - "required": true - }, - { - "type": "string", - "description": "name of the repo", - "name": "repo", - "in": "path", - "required": true - } - ], - "responses": { - "200": { - "$ref": "#/responses/RegistrationToken" - } - } - } - }, "/repos/{owner}/{repo}/signing-key.gpg": { "get": { "produces": [ From 6a4d1dfab84c6ac15e106a2953ee70dc61fe8ab4 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Tue, 30 Jul 2024 01:15:02 +0800 Subject: [PATCH 07/24] fix(api): owner ID should be zero when created repo secret (#31715) - Change condition to include `RepoID` equal to 0 for organization secrets --------- Signed-off-by: Bo-Yi Wu Co-authored-by: Giteabot (cherry picked from commit d39bce7f003cf2137a5a561ed488c7b638e52275) Conflicts: routers/api/v1/repo/action.go trivial context conflict (PathParams vs Params) --- routers/api/v1/repo/action.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index d7a49a78be..568826859b 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -117,12 +117,11 @@ func (Action) CreateOrUpdateSecret(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - owner := ctx.Repo.Owner repo := ctx.Repo.Repository opt := web.GetForm(ctx).(*api.CreateOrUpdateSecretOption) - _, created, err := secret_service.CreateOrUpdateSecret(ctx, owner.ID, repo.ID, ctx.Params("secretname"), opt.Data) + _, created, err := secret_service.CreateOrUpdateSecret(ctx, 0, repo.ID, ctx.Params("secretname"), opt.Data) if err != nil { if errors.Is(err, util.ErrInvalidArgument) { ctx.Error(http.StatusBadRequest, "CreateOrUpdateSecret", err) @@ -174,10 +173,9 @@ func (Action) DeleteSecret(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - owner := ctx.Repo.Owner repo := ctx.Repo.Repository - err := secret_service.DeleteSecretByName(ctx, owner.ID, repo.ID, ctx.Params("secretname")) + err := secret_service.DeleteSecretByName(ctx, 0, repo.ID, ctx.Params("secretname")) if err != nil { if errors.Is(err, util.ErrInvalidArgument) { ctx.Error(http.StatusBadRequest, "DeleteSecret", err) From 92fbc8e21672259e4c64036bdd68c5649aee2299 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Tue, 30 Jul 2024 02:46:45 +0800 Subject: [PATCH 08/24] Set owner id to zero when GetRegistrationToken for repo (#31725) Fix #31707. It's split from #31724. Although #31724 could also fix #31707, it has change a lot so it's not a good idea to backport it. (cherry picked from commit 81fa471119a6733d257f63f8c2c1f4acc583d21b) --- routers/api/v1/repo/action.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index 568826859b..0c7506b13b 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -504,7 +504,7 @@ func (Action) GetRegistrationToken(ctx *context.APIContext) { // "200": // "$ref": "#/responses/RegistrationToken" - shared.GetRegistrationToken(ctx, ctx.Repo.Repository.OwnerID, ctx.Repo.Repository.ID) + shared.GetRegistrationToken(ctx, 0, ctx.Repo.Repository.ID) } var _ actions_service.API = new(Action) From 43b184cf07a64a4a2900c8bcd1ffaf2ede7daa3c Mon Sep 17 00:00:00 2001 From: Jason Song Date: Tue, 30 Jul 2024 10:27:28 +0800 Subject: [PATCH 09/24] Move `registerActionsCleanup` to `initActionsTasks` (#31721) There's already `initActionsTasks`; it will avoid additional check for if Actions enabled to move `registerActionsCleanup` into it. And we don't really need `OlderThanConfig`. (cherry picked from commit f989f464386139592b6911cad1be4c901eb97fe5) --- services/actions/cleanup.go | 3 +-- services/cron/tasks_actions.go | 11 +++++++++++ services/cron/tasks_basic.go | 18 ------------------ 3 files changed, 12 insertions(+), 20 deletions(-) diff --git a/services/actions/cleanup.go b/services/actions/cleanup.go index 5376c2624c..6ccc8dd198 100644 --- a/services/actions/cleanup.go +++ b/services/actions/cleanup.go @@ -5,7 +5,6 @@ package actions import ( "context" - "time" "code.gitea.io/gitea/models/actions" "code.gitea.io/gitea/modules/log" @@ -13,7 +12,7 @@ import ( ) // Cleanup removes expired actions logs, data and artifacts -func Cleanup(taskCtx context.Context, olderThan time.Duration) error { +func Cleanup(taskCtx context.Context) error { // TODO: clean up expired actions logs // clean up expired artifacts diff --git a/services/cron/tasks_actions.go b/services/cron/tasks_actions.go index 0875792503..9b5e0b9f41 100644 --- a/services/cron/tasks_actions.go +++ b/services/cron/tasks_actions.go @@ -19,6 +19,7 @@ func initActionsTasks() { registerStopEndlessTasks() registerCancelAbandonedJobs() registerScheduleTasks() + registerActionsCleanup() } func registerStopZombieTasks() { @@ -63,3 +64,13 @@ func registerScheduleTasks() { return actions_service.StartScheduleTasks(ctx) }) } + +func registerActionsCleanup() { + RegisterTaskFatal("cleanup_actions", &BaseConfig{ + Enabled: true, + RunAtStart: true, + Schedule: "@midnight", + }, func(ctx context.Context, _ *user_model.User, _ Config) error { + return actions_service.Cleanup(ctx) + }) +} diff --git a/services/cron/tasks_basic.go b/services/cron/tasks_basic.go index 3869382d22..2a213ae515 100644 --- a/services/cron/tasks_basic.go +++ b/services/cron/tasks_basic.go @@ -13,7 +13,6 @@ import ( "code.gitea.io/gitea/models/webhook" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/services/actions" "code.gitea.io/gitea/services/auth" "code.gitea.io/gitea/services/migrations" mirror_service "code.gitea.io/gitea/services/mirror" @@ -157,20 +156,6 @@ func registerCleanupPackages() { }) } -func registerActionsCleanup() { - RegisterTaskFatal("cleanup_actions", &OlderThanConfig{ - BaseConfig: BaseConfig{ - Enabled: true, - RunAtStart: true, - Schedule: "@midnight", - }, - OlderThan: 24 * time.Hour, - }, func(ctx context.Context, _ *user_model.User, config Config) error { - realConfig := config.(*OlderThanConfig) - return actions.Cleanup(ctx, realConfig.OlderThan) - }) -} - func initBasicTasks() { if setting.Mirror.Enabled { registerUpdateMirrorTask() @@ -187,7 +172,4 @@ func initBasicTasks() { if setting.Packages.Enabled { registerCleanupPackages() } - if setting.Actions.Enabled { - registerActionsCleanup() - } } From 49eb8316637616e5bac6b24a30a7df764d6bdc9b Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Tue, 30 Jul 2024 11:56:25 +0900 Subject: [PATCH 10/24] Fix Null Pointer error for CommitStatusesHideActionsURL (#31731) Fix https://github.com/go-gitea/gitea/pull/30156#discussion_r1695247028 Forgot fixing it in #31719 (cherry picked from commit 0a11bce87f07233d5f02554b8f3b4a2aabd37769) --- models/git/commit_status.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/models/git/commit_status.go b/models/git/commit_status.go index 422cf0b4ee..53d1ddc8c3 100644 --- a/models/git/commit_status.go +++ b/models/git/commit_status.go @@ -506,6 +506,10 @@ func ConvertFromGitCommit(ctx context.Context, commits []*git.Commit, repo *repo func CommitStatusesHideActionsURL(ctx context.Context, statuses []*CommitStatus) { idToRepos := make(map[int64]*repo_model.Repository) for _, status := range statuses { + if status == nil { + continue + } + if status.Repo == nil { status.Repo = idToRepos[status.RepoID] } From c784a5874066ca1a1fd518408d5767b4eb57bd69 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Tue, 30 Jul 2024 13:37:43 +0900 Subject: [PATCH 11/24] Fix the display of project type for deleted projects (#31732) Fix: #31727 After: ![image](https://github.com/user-attachments/assets/1dfb4b31-3bd6-47f7-b126-650f33f453e2) (cherry picked from commit 75d0b61546e00390afdd850149de525dd64336a5) Conflicts: options/locale/locale_en-US.ini trivial conflict & fix excessive uppercase to unify with the other translations --- models/project/project.go | 7 +++++++ options/locale/locale_en-US.ini | 1 + routers/web/repo/issue.go | 2 +- templates/repo/issue/view_content/comments.tmpl | 14 ++++++++++---- 4 files changed, 19 insertions(+), 5 deletions(-) diff --git a/models/project/project.go b/models/project/project.go index fe5d408f64..8cebf34b5e 100644 --- a/models/project/project.go +++ b/models/project/project.go @@ -103,6 +103,13 @@ type Project struct { ClosedDateUnix timeutil.TimeStamp } +// Ghost Project is a project which has been deleted +const GhostProjectID = -1 + +func (p *Project) IsGhost() bool { + return p.ID == GhostProjectID +} + func (p *Project) LoadOwner(ctx context.Context) (err error) { if p.Owner != nil { return nil diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index d0a0dc0696..7da7107b9e 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -3860,6 +3860,7 @@ variables.update.failed = Failed to edit variable. variables.update.success = The variable has been edited. [projects] +deleted.display_name = Deleted Project type-1.display_name = Individual project type-2.display_name = Repository project type-3.display_name = Organization project diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 5a0dcd830f..afac2c5266 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1697,7 +1697,7 @@ func ViewIssue(ctx *context.Context) { } ghostProject := &project_model.Project{ - ID: -1, + ID: project_model.GhostProjectID, Title: ctx.Locale.TrString("repo.issues.deleted_project"), } diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index 6d14d72646..019638bfb0 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -582,13 +582,19 @@ {{template "shared/user/authorlink" .Poster}} {{$oldProjectDisplayHtml := "Unknown Project"}} {{if .OldProject}} - {{$trKey := printf "projects.type-%d.display_name" .OldProject.Type}} - {{$oldProjectDisplayHtml = HTMLFormat `%s` (ctx.Locale.Tr $trKey) .OldProject.Title}} + {{$tooltip := ctx.Locale.Tr "projects.deleted.display_name"}} + {{if not .OldProject.IsGhost}} + {{$tooltip = ctx.Locale.Tr (printf "projects.type-%d.display_name" .OldProject.Type)}} + {{end}} + {{$oldProjectDisplayHtml = HTMLFormat `%s` $tooltip .OldProject.Title}} {{end}} {{$newProjectDisplayHtml := "Unknown Project"}} {{if .Project}} - {{$trKey := printf "projects.type-%d.display_name" .Project.Type}} - {{$newProjectDisplayHtml = HTMLFormat `%s` (ctx.Locale.Tr $trKey) .Project.Title}} + {{$tooltip := ctx.Locale.Tr "projects.deleted.display_name"}} + {{if not .Project.IsGhost}} + {{$tooltip = ctx.Locale.Tr (printf "projects.type-%d.display_name" .Project.Type)}} + {{end}} + {{$newProjectDisplayHtml = HTMLFormat `%s` $tooltip .Project.Title}} {{end}} {{if and (gt .OldProjectID 0) (gt .ProjectID 0)}} {{ctx.Locale.Tr "repo.issues.change_project_at" $oldProjectDisplayHtml $newProjectDisplayHtml $createdStr}} From 51f414d46677440cf87e269c310b1f2496d272c1 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 31 Jul 2024 11:03:30 +0800 Subject: [PATCH 12/24] Improve names of cron jobs for Actions (#31736) Before: image After: image (cherry picked from commit 9ac57c957f9dc88b1394008d6efb79f29a4c3396) Conflicts: options/locale/locale_en-US.ini trivial context conflict --- options/locale/locale_en-US.ini | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 7da7107b9e..9821c1a293 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -3021,10 +3021,10 @@ dashboard.delete_old_actions.started = Delete all old activities from database s dashboard.update_checker = Update checker dashboard.delete_old_system_notices = Delete all old system notices from database dashboard.gc_lfs = Garbage collect LFS meta objects -dashboard.stop_zombie_tasks = Stop zombie tasks -dashboard.stop_endless_tasks = Stop endless tasks -dashboard.cancel_abandoned_jobs = Cancel abandoned jobs -dashboard.start_schedule_tasks = Start schedule tasks +dashboard.stop_zombie_tasks = Stop zombie actions tasks +dashboard.stop_endless_tasks = Stop endless actions tasks +dashboard.cancel_abandoned_jobs = Cancel abandoned actions jobs +dashboard.start_schedule_tasks = Start schedule actions tasks dashboard.sync_branch.started = Branch sync started dashboard.sync_tag.started = Tag sync started dashboard.rebuild_issue_indexer = Rebuild issue indexer From 2302cf63c8b395b22c07d50e5fb080a9990688b5 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 31 Jul 2024 18:29:48 +0800 Subject: [PATCH 13/24] Distinguish LFS object errors to ignore missing objects during migration (#31702) Fix #31137. Replace #31623 #31697. When migrating LFS objects, if there's any object that failed (like some objects are losted, which is not really critical), Gitea will stop migrating LFS immediately but treat the migration as successful. This PR checks the error according to the [LFS api doc](https://github.com/git-lfs/git-lfs/blob/main/docs/api/batch.md#successful-responses). > LFS object error codes should match HTTP status codes where possible: > > - 404 - The object does not exist on the server. > - 409 - The specified hash algorithm disagrees with the server's acceptable options. > - 410 - The object was removed by the owner. > - 422 - Validation error. If the error is `404`, it's safe to ignore it and continue migration. Otherwise, stop the migration and mark it as failed to ensure data integrity of LFS objects. And maybe we should also ignore others errors (maybe `410`? I'm not sure what's the difference between "does not exist" and "removed by the owner".), we can add it later when some users report that they have failed to migrate LFS because of an error which should be ignored. (cherry picked from commit 09b56fc0690317891829906d45c1d645794c63d5) --- modules/lfs/http_client.go | 7 +++---- modules/lfs/shared.go | 37 ++++++++++++++++++++++++++++++++++ modules/repository/repo.go | 5 +++++ services/repository/migrate.go | 1 + 4 files changed, 46 insertions(+), 4 deletions(-) diff --git a/modules/lfs/http_client.go b/modules/lfs/http_client.go index 7ee2449b0e..4859fe61e1 100644 --- a/modules/lfs/http_client.go +++ b/modules/lfs/http_client.go @@ -136,14 +136,13 @@ func (c *HTTPClient) performOperation(ctx context.Context, objects []Pointer, dc for _, object := range result.Objects { if object.Error != nil { - objectError := errors.New(object.Error.Message) - log.Trace("Error on object %v: %v", object.Pointer, objectError) + log.Trace("Error on object %v: %v", object.Pointer, object.Error) if uc != nil { - if _, err := uc(object.Pointer, objectError); err != nil { + if _, err := uc(object.Pointer, object.Error); err != nil { return err } } else { - if err := dc(object.Pointer, nil, objectError); err != nil { + if err := dc(object.Pointer, nil, object.Error); err != nil { return err } } diff --git a/modules/lfs/shared.go b/modules/lfs/shared.go index 675d2328b7..a4326b57b2 100644 --- a/modules/lfs/shared.go +++ b/modules/lfs/shared.go @@ -4,7 +4,11 @@ package lfs import ( + "errors" + "fmt" "time" + + "code.gitea.io/gitea/modules/util" ) const ( @@ -64,6 +68,39 @@ type ObjectError struct { Message string `json:"message"` } +var ( + // See https://github.com/git-lfs/git-lfs/blob/main/docs/api/batch.md#successful-responses + // LFS object error codes should match HTTP status codes where possible: + // 404 - The object does not exist on the server. + // 409 - The specified hash algorithm disagrees with the server's acceptable options. + // 410 - The object was removed by the owner. + // 422 - Validation error. + + ErrObjectNotExist = util.ErrNotExist // the object does not exist on the server + ErrObjectHashMismatch = errors.New("the specified hash algorithm disagrees with the server's acceptable options") + ErrObjectRemoved = errors.New("the object was removed by the owner") + ErrObjectValidation = errors.New("validation error") +) + +func (e *ObjectError) Error() string { + return fmt.Sprintf("[%d] %s", e.Code, e.Message) +} + +func (e *ObjectError) Unwrap() error { + switch e.Code { + case 404: + return ErrObjectNotExist + case 409: + return ErrObjectHashMismatch + case 410: + return ErrObjectRemoved + case 422: + return ErrObjectValidation + default: + return errors.New(e.Message) + } +} + // PointerBlob associates a Git blob with a Pointer. type PointerBlob struct { Hash string diff --git a/modules/repository/repo.go b/modules/repository/repo.go index a863bec996..e08bc376b8 100644 --- a/modules/repository/repo.go +++ b/modules/repository/repo.go @@ -6,6 +6,7 @@ package repository import ( "context" + "errors" "fmt" "io" "strings" @@ -182,6 +183,10 @@ func StoreMissingLfsObjectsInRepository(ctx context.Context, repo *repo_model.Re downloadObjects := func(pointers []lfs.Pointer) error { err := lfsClient.Download(ctx, pointers, func(p lfs.Pointer, content io.ReadCloser, objectError error) error { if objectError != nil { + if errors.Is(objectError, lfs.ErrObjectNotExist) { + log.Warn("Repo[%-v]: Ignore missing LFS object %-v: %v", repo, p, objectError) + return nil + } return objectError } diff --git a/services/repository/migrate.go b/services/repository/migrate.go index 5800f2b5cb..39ced04ae3 100644 --- a/services/repository/migrate.go +++ b/services/repository/migrate.go @@ -172,6 +172,7 @@ func MigrateRepositoryGitData(ctx context.Context, u *user_model.User, lfsClient := lfs.NewClient(endpoint, httpTransport) if err = repo_module.StoreMissingLfsObjectsInRepository(ctx, repo, gitRepo, lfsClient); err != nil { log.Error("Failed to store missing LFS objects for repository: %v", err) + return repo, fmt.Errorf("StoreMissingLfsObjectsInRepository: %w", err) } } } From 6844258c67e4643f1637150142325e8eff3b070a Mon Sep 17 00:00:00 2001 From: Jason Song Date: Thu, 1 Aug 2024 17:04:04 +0800 Subject: [PATCH 14/24] Clarify Actions resources ownership (#31724) Fix #31707. Also related to #31715. Some Actions resources could has different types of ownership. It could be: - global: all repos and orgs/users can use it. - org/user level: only the org/user can use it. - repo level: only the repo can use it. There are two ways to distinguish org/user level from repo level: 1. `{owner_id: 1, repo_id: 2}` for repo level, and `{owner_id: 1, repo_id: 0}` for org level. 2. `{owner_id: 0, repo_id: 2}` for repo level, and `{owner_id: 1, repo_id: 0}` for org level. The first way seems more reasonable, but it may not be true. The point is that although a resource, like a runner, belongs to a repo (it can be used by the repo), the runner doesn't belong to the repo's org (other repos in the same org cannot use the runner). So, the second method makes more sense. And the first way is not user-friendly to query, we must set the repo id to zero to avoid wrong results. So, #31715 should be right. And the most simple way to fix #31707 is just: ```diff - shared.GetRegistrationToken(ctx, ctx.Repo.Repository.OwnerID, ctx.Repo.Repository.ID) + shared.GetRegistrationToken(ctx, 0, ctx.Repo.Repository.ID) ``` However, it is quite intuitive to set both owner id and repo id since the repo belongs to the owner. So I prefer to be compatible with it. If we get both owner id and repo id not zero when creating or finding, it's very clear that the caller want one with repo level, but set owner id accidentally. So it's OK to accept it but fix the owner id to zero. (cherry picked from commit a33e74d40d356e8f628ac06a131cb203a3609dec) --- models/actions/runner.go | 25 ++++++++--- models/actions/runner_token.go | 28 +++++++++++- models/actions/variable.go | 38 ++++++++++------ models/secret/secret.go | 46 +++++++++++++------- tests/integration/api_user_variables_test.go | 4 +- 5 files changed, 103 insertions(+), 38 deletions(-) diff --git a/models/actions/runner.go b/models/actions/runner.go index 3302a930a1..175f211c72 100644 --- a/models/actions/runner.go +++ b/models/actions/runner.go @@ -26,14 +26,25 @@ import ( ) // ActionRunner represents runner machines +// +// It can be: +// 1. global runner, OwnerID is 0 and RepoID is 0 +// 2. org/user level runner, OwnerID is org/user ID and RepoID is 0 +// 3. repo level runner, OwnerID is 0 and RepoID is repo ID +// +// Please note that it's not acceptable to have both OwnerID and RepoID to be non-zero, +// or it will be complicated to find runners belonging to a specific owner. +// For example, conditions like `OwnerID = 1` will also return runner {OwnerID: 1, RepoID: 1}, +// but it's a repo level runner, not an org/user level runner. +// To avoid this, make it clear with {OwnerID: 0, RepoID: 1} for repo level runners. type ActionRunner struct { ID int64 UUID string `xorm:"CHAR(36) UNIQUE"` Name string `xorm:"VARCHAR(255)"` Version string `xorm:"VARCHAR(64)"` - OwnerID int64 `xorm:"index"` // org level runner, 0 means system + OwnerID int64 `xorm:"index"` Owner *user_model.User `xorm:"-"` - RepoID int64 `xorm:"index"` // repo level runner, if OwnerID also is zero, then it's a global + RepoID int64 `xorm:"index"` Repo *repo_model.Repository `xorm:"-"` Description string `xorm:"TEXT"` Base int // 0 native 1 docker 2 virtual machine @@ -176,7 +187,7 @@ func init() { type FindRunnerOptions struct { db.ListOptions RepoID int64 - OwnerID int64 + OwnerID int64 // it will be ignored if RepoID is set Sort string Filter string IsOnline optional.Option[bool] @@ -193,8 +204,7 @@ func (opts FindRunnerOptions) ToConds() builder.Cond { c = c.Or(builder.Eq{"repo_id": 0, "owner_id": 0}) } cond = cond.And(c) - } - if opts.OwnerID > 0 { + } else if opts.OwnerID > 0 { // OwnerID is ignored if RepoID is set c := builder.NewCond().And(builder.Eq{"owner_id": opts.OwnerID}) if opts.WithAvailable { c = c.Or(builder.Eq{"repo_id": 0, "owner_id": 0}) @@ -297,6 +307,11 @@ func DeleteRunner(ctx context.Context, id int64) error { // CreateRunner creates new runner. func CreateRunner(ctx context.Context, t *ActionRunner) error { + if t.OwnerID != 0 && t.RepoID != 0 { + // It's trying to create a runner that belongs to a repository, but OwnerID has been set accidentally. + // Remove OwnerID to avoid confusion; it's not worth returning an error here. + t.OwnerID = 0 + } return db.Insert(ctx, t) } diff --git a/models/actions/runner_token.go b/models/actions/runner_token.go index ccd9bbccb3..fd6ba7ecad 100644 --- a/models/actions/runner_token.go +++ b/models/actions/runner_token.go @@ -15,12 +15,23 @@ import ( ) // ActionRunnerToken represents runner tokens +// +// It can be: +// 1. global token, OwnerID is 0 and RepoID is 0 +// 2. org/user level token, OwnerID is org/user ID and RepoID is 0 +// 3. repo level token, OwnerID is 0 and RepoID is repo ID +// +// Please note that it's not acceptable to have both OwnerID and RepoID to be non-zero, +// or it will be complicated to find tokens belonging to a specific owner. +// For example, conditions like `OwnerID = 1` will also return token {OwnerID: 1, RepoID: 1}, +// but it's a repo level token, not an org/user level token. +// To avoid this, make it clear with {OwnerID: 0, RepoID: 1} for repo level tokens. type ActionRunnerToken struct { ID int64 Token string `xorm:"UNIQUE"` - OwnerID int64 `xorm:"index"` // org level runner, 0 means system + OwnerID int64 `xorm:"index"` Owner *user_model.User `xorm:"-"` - RepoID int64 `xorm:"index"` // repo level runner, if orgid also is zero, then it's a global + RepoID int64 `xorm:"index"` Repo *repo_model.Repository `xorm:"-"` IsActive bool // true means it can be used @@ -58,7 +69,14 @@ func UpdateRunnerToken(ctx context.Context, r *ActionRunnerToken, cols ...string } // NewRunnerToken creates a new active runner token and invalidate all old tokens +// ownerID will be ignored and treated as 0 if repoID is non-zero. func NewRunnerToken(ctx context.Context, ownerID, repoID int64) (*ActionRunnerToken, error) { + if ownerID != 0 && repoID != 0 { + // It's trying to create a runner token that belongs to a repository, but OwnerID has been set accidentally. + // Remove OwnerID to avoid confusion; it's not worth returning an error here. + ownerID = 0 + } + token, err := util.CryptoRandomString(40) if err != nil { return nil, err @@ -84,6 +102,12 @@ func NewRunnerToken(ctx context.Context, ownerID, repoID int64) (*ActionRunnerTo // GetLatestRunnerToken returns the latest runner token func GetLatestRunnerToken(ctx context.Context, ownerID, repoID int64) (*ActionRunnerToken, error) { + if ownerID != 0 && repoID != 0 { + // It's trying to get a runner token that belongs to a repository, but OwnerID has been set accidentally. + // Remove OwnerID to avoid confusion; it's not worth returning an error here. + ownerID = 0 + } + var runnerToken ActionRunnerToken has, err := db.GetEngine(ctx).Where("owner_id=? AND repo_id=?", ownerID, repoID). OrderBy("id DESC").Get(&runnerToken) diff --git a/models/actions/variable.go b/models/actions/variable.go index 8aff844659..d0f917d923 100644 --- a/models/actions/variable.go +++ b/models/actions/variable.go @@ -5,7 +5,6 @@ package actions import ( "context" - "errors" "strings" "code.gitea.io/gitea/models/db" @@ -15,6 +14,18 @@ import ( "xorm.io/builder" ) +// ActionVariable represents a variable that can be used in actions +// +// It can be: +// 1. global variable, OwnerID is 0 and RepoID is 0 +// 2. org/user level variable, OwnerID is org/user ID and RepoID is 0 +// 3. repo level variable, OwnerID is 0 and RepoID is repo ID +// +// Please note that it's not acceptable to have both OwnerID and RepoID to be non-zero, +// or it will be complicated to find variables belonging to a specific owner. +// For example, conditions like `OwnerID = 1` will also return variable {OwnerID: 1, RepoID: 1}, +// but it's a repo level variable, not an org/user level variable. +// To avoid this, make it clear with {OwnerID: 0, RepoID: 1} for repo level variables. type ActionVariable struct { ID int64 `xorm:"pk autoincr"` OwnerID int64 `xorm:"UNIQUE(owner_repo_name)"` @@ -29,30 +40,26 @@ func init() { db.RegisterModel(new(ActionVariable)) } -func (v *ActionVariable) Validate() error { - if v.OwnerID != 0 && v.RepoID != 0 { - return errors.New("a variable should not be bound to an owner and a repository at the same time") - } - return nil -} - func InsertVariable(ctx context.Context, ownerID, repoID int64, name, data string) (*ActionVariable, error) { + if ownerID != 0 && repoID != 0 { + // It's trying to create a variable that belongs to a repository, but OwnerID has been set accidentally. + // Remove OwnerID to avoid confusion; it's not worth returning an error here. + ownerID = 0 + } + variable := &ActionVariable{ OwnerID: ownerID, RepoID: repoID, Name: strings.ToUpper(name), Data: data, } - if err := variable.Validate(); err != nil { - return variable, err - } return variable, db.Insert(ctx, variable) } type FindVariablesOpts struct { db.ListOptions - OwnerID int64 RepoID int64 + OwnerID int64 // it will be ignored if RepoID is set Name string } @@ -60,8 +67,13 @@ func (opts FindVariablesOpts) ToConds() builder.Cond { cond := builder.NewCond() // Since we now support instance-level variables, // there is no need to check for null values for `owner_id` and `repo_id` - cond = cond.And(builder.Eq{"owner_id": opts.OwnerID}) cond = cond.And(builder.Eq{"repo_id": opts.RepoID}) + if opts.RepoID != 0 { // if RepoID is set + // ignore OwnerID and treat it as 0 + cond = cond.And(builder.Eq{"owner_id": 0}) + } else { + cond = cond.And(builder.Eq{"owner_id": opts.OwnerID}) + } if opts.Name != "" { cond = cond.And(builder.Eq{"name": strings.ToUpper(opts.Name)}) diff --git a/models/secret/secret.go b/models/secret/secret.go index 35bed500b9..ce0ad65a79 100644 --- a/models/secret/secret.go +++ b/models/secret/secret.go @@ -5,7 +5,6 @@ package secret import ( "context" - "errors" "fmt" "strings" @@ -22,6 +21,19 @@ import ( ) // Secret represents a secret +// +// It can be: +// 1. org/user level secret, OwnerID is org/user ID and RepoID is 0 +// 2. repo level secret, OwnerID is 0 and RepoID is repo ID +// +// Please note that it's not acceptable to have both OwnerID and RepoID to be non-zero, +// or it will be complicated to find secrets belonging to a specific owner. +// For example, conditions like `OwnerID = 1` will also return secret {OwnerID: 1, RepoID: 1}, +// but it's a repo level secret, not an org/user level secret. +// To avoid this, make it clear with {OwnerID: 0, RepoID: 1} for repo level secrets. +// +// Please note that it's not acceptable to have both OwnerID and RepoID to zero, global secrets are not supported. +// It's for security reasons, admin may be not aware of that the secrets could be stolen by any user when setting them as global. type Secret struct { ID int64 OwnerID int64 `xorm:"INDEX UNIQUE(owner_repo_name) NOT NULL"` @@ -46,6 +58,15 @@ func (err ErrSecretNotFound) Unwrap() error { // InsertEncryptedSecret Creates, encrypts, and validates a new secret with yet unencrypted data and insert into database func InsertEncryptedSecret(ctx context.Context, ownerID, repoID int64, name, data string) (*Secret, error) { + if ownerID != 0 && repoID != 0 { + // It's trying to create a secret that belongs to a repository, but OwnerID has been set accidentally. + // Remove OwnerID to avoid confusion; it's not worth returning an error here. + ownerID = 0 + } + if ownerID == 0 && repoID == 0 { + return nil, fmt.Errorf("%w: ownerID and repoID cannot be both zero, global secrets are not supported", util.ErrInvalidArgument) + } + encrypted, err := secret_module.EncryptSecret(setting.SecretKey, data) if err != nil { return nil, err @@ -56,9 +77,6 @@ func InsertEncryptedSecret(ctx context.Context, ownerID, repoID int64, name, dat Name: strings.ToUpper(name), Data: encrypted, } - if err := secret.Validate(); err != nil { - return secret, err - } return secret, db.Insert(ctx, secret) } @@ -66,29 +84,25 @@ func init() { db.RegisterModel(new(Secret)) } -func (s *Secret) Validate() error { - if s.OwnerID == 0 && s.RepoID == 0 { - return errors.New("the secret is not bound to any scope") - } - return nil -} - type FindSecretsOptions struct { db.ListOptions - OwnerID int64 RepoID int64 + OwnerID int64 // it will be ignored if RepoID is set SecretID int64 Name string } func (opts FindSecretsOptions) ToConds() builder.Cond { cond := builder.NewCond() - if opts.OwnerID > 0 { + + cond = cond.And(builder.Eq{"repo_id": opts.RepoID}) + if opts.RepoID != 0 { // if RepoID is set + // ignore OwnerID and treat it as 0 + cond = cond.And(builder.Eq{"owner_id": 0}) + } else { cond = cond.And(builder.Eq{"owner_id": opts.OwnerID}) } - if opts.RepoID > 0 { - cond = cond.And(builder.Eq{"repo_id": opts.RepoID}) - } + if opts.SecretID != 0 { cond = cond.And(builder.Eq{"id": opts.SecretID}) } diff --git a/tests/integration/api_user_variables_test.go b/tests/integration/api_user_variables_test.go index dd5501f0b9..9fd84ddf81 100644 --- a/tests/integration/api_user_variables_test.go +++ b/tests/integration/api_user_variables_test.go @@ -19,7 +19,7 @@ func TestAPIUserVariables(t *testing.T) { session := loginUser(t, "user1") token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteUser) - t.Run("CreateRepoVariable", func(t *testing.T) { + t.Run("CreateUserVariable", func(t *testing.T) { cases := []struct { Name string ExpectedStatus int @@ -70,7 +70,7 @@ func TestAPIUserVariables(t *testing.T) { } }) - t.Run("UpdateRepoVariable", func(t *testing.T) { + t.Run("UpdateUserVariable", func(t *testing.T) { variableName := "test_update_var" url := fmt.Sprintf("/api/v1/user/actions/variables/%s", variableName) req := NewRequestWithJSON(t, "POST", url, api.CreateVariableOption{ From f6b1407e4c34c30001acacb4e782109b4de34579 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Thu, 1 Aug 2024 17:33:40 +0800 Subject: [PATCH 15/24] Add permission description for API to add repo collaborator (#31744) Fix #31552. (cherry picked from commit 333c9ed8cab961b6dd58b04edc47a57dc4d6dbab) --- modules/structs/repo_collaborator.go | 1 + templates/swagger/v1_json.tmpl | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/modules/structs/repo_collaborator.go b/modules/structs/repo_collaborator.go index 946a6ec7e7..7d39b5a798 100644 --- a/modules/structs/repo_collaborator.go +++ b/modules/structs/repo_collaborator.go @@ -5,6 +5,7 @@ package structs // AddCollaboratorOption options when adding a user as a collaborator of a repository type AddCollaboratorOption struct { + // enum: read,write,admin Permission *string `json:"permission"` } diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 3379e3f4dc..d14e7c4b66 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -19925,6 +19925,11 @@ "properties": { "permission": { "type": "string", + "enum": [ + "read", + "write", + "admin" + ], "x-go-name": "Permission" } }, From 3fdaabcdcf4285996ab8ad29e957250fca7cffde Mon Sep 17 00:00:00 2001 From: Jason Song Date: Thu, 1 Aug 2024 18:02:46 +0800 Subject: [PATCH 16/24] Use UTC as default timezone when schedule Actions cron tasks (#31742) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix #31657. According to the [doc](https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#onschedule) of GitHub Actions, The timezone for cron should be UTC, not the local timezone. And Gitea Actions doesn't have any reasons to change this, so I think it's a bug. However, Gitea Actions has extended the syntax, as it supports descriptors like `@weekly` and `@every 5m`, and supports specifying the timezone like `TZ=UTC 0 10 * * *`. So we can make it use UTC only when the timezone is not specified, to be compatible with GitHub Actions, and also respect the user's specified. It does break the feature because the times to run tasks would be changed, and it may confuse users. So I don't think we should backport this. ## ⚠️ BREAKING ⚠️ If the server's local time zone is not UTC, a scheduled task would run at a different time after upgrading Gitea to this version. (cherry picked from commit 21a73ae642b15982a911837775c9583deb47220c) --- models/actions/schedule.go | 20 ++++---- models/actions/schedule_spec.go | 25 +++++++++- models/actions/schedule_spec_test.go | 71 ++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 12 deletions(-) create mode 100644 models/actions/schedule_spec_test.go diff --git a/models/actions/schedule.go b/models/actions/schedule.go index 3646a046a0..c751ef51ca 100644 --- a/models/actions/schedule.go +++ b/models/actions/schedule.go @@ -13,8 +13,6 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/timeutil" webhook_module "code.gitea.io/gitea/modules/webhook" - - "github.com/robfig/cron/v3" ) // ActionSchedule represents a schedule of a workflow file @@ -53,8 +51,6 @@ func GetReposMapByIDs(ctx context.Context, ids []int64) (map[int64]*repo_model.R return repos, db.GetEngine(ctx).In("id", ids).Find(&repos) } -var cronParser = cron.NewParser(cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow | cron.Descriptor) - // CreateScheduleTask creates new schedule task. func CreateScheduleTask(ctx context.Context, rows []*ActionSchedule) error { // Return early if there are no rows to insert @@ -80,19 +76,21 @@ func CreateScheduleTask(ctx context.Context, rows []*ActionSchedule) error { now := time.Now() for _, spec := range row.Specs { + specRow := &ActionScheduleSpec{ + RepoID: row.RepoID, + ScheduleID: row.ID, + Spec: spec, + } // Parse the spec and check for errors - schedule, err := cronParser.Parse(spec) + schedule, err := specRow.Parse() if err != nil { continue // skip to the next spec if there's an error } + specRow.Next = timeutil.TimeStamp(schedule.Next(now).Unix()) + // Insert the new schedule spec row - if err = db.Insert(ctx, &ActionScheduleSpec{ - RepoID: row.RepoID, - ScheduleID: row.ID, - Spec: spec, - Next: timeutil.TimeStamp(schedule.Next(now).Unix()), - }); err != nil { + if err = db.Insert(ctx, specRow); err != nil { return err } } diff --git a/models/actions/schedule_spec.go b/models/actions/schedule_spec.go index 91240459a0..923e5f7807 100644 --- a/models/actions/schedule_spec.go +++ b/models/actions/schedule_spec.go @@ -5,6 +5,8 @@ package actions import ( "context" + "strings" + "time" "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" @@ -32,8 +34,29 @@ type ActionScheduleSpec struct { Updated timeutil.TimeStamp `xorm:"updated"` } +// Parse parses the spec and returns a cron.Schedule +// Unlike the default cron parser, Parse uses UTC timezone as the default if none is specified. func (s *ActionScheduleSpec) Parse() (cron.Schedule, error) { - return cronParser.Parse(s.Spec) + parser := cron.NewParser(cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow | cron.Descriptor) + schedule, err := parser.Parse(s.Spec) + if err != nil { + return nil, err + } + + // If the spec has specified a timezone, use it + if strings.HasPrefix(s.Spec, "TZ=") || strings.HasPrefix(s.Spec, "CRON_TZ=") { + return schedule, nil + } + + specSchedule, ok := schedule.(*cron.SpecSchedule) + // If it's not a spec schedule, like "@every 5m", timezone is not relevant + if !ok { + return schedule, nil + } + + // Set the timezone to UTC + specSchedule.Location = time.UTC + return specSchedule, nil } func init() { diff --git a/models/actions/schedule_spec_test.go b/models/actions/schedule_spec_test.go new file mode 100644 index 0000000000..0c26fce4b2 --- /dev/null +++ b/models/actions/schedule_spec_test.go @@ -0,0 +1,71 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package actions + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestActionScheduleSpec_Parse(t *testing.T) { + // Mock the local timezone is not UTC + local := time.Local + tz, err := time.LoadLocation("Asia/Shanghai") + require.NoError(t, err) + defer func() { + time.Local = local + }() + time.Local = tz + + now, err := time.Parse(time.RFC3339, "2024-07-31T15:47:55+08:00") + require.NoError(t, err) + + tests := []struct { + name string + spec string + want string + wantErr assert.ErrorAssertionFunc + }{ + { + name: "regular", + spec: "0 10 * * *", + want: "2024-07-31T10:00:00Z", + wantErr: assert.NoError, + }, + { + name: "invalid", + spec: "0 10 * *", + want: "", + wantErr: assert.Error, + }, + { + name: "with timezone", + spec: "TZ=America/New_York 0 10 * * *", + want: "2024-07-31T14:00:00Z", + wantErr: assert.NoError, + }, + { + name: "timezone irrelevant", + spec: "@every 5m", + want: "2024-07-31T07:52:55Z", + wantErr: assert.NoError, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := &ActionScheduleSpec{ + Spec: tt.spec, + } + got, err := s.Parse() + tt.wantErr(t, err) + + if err == nil { + assert.Equal(t, tt.want, got.Next(now).UTC().Format(time.RFC3339)) + } + }) + } +} From ee8f3e09f8d24cd5104a916ae0f0ceac8173306b Mon Sep 17 00:00:00 2001 From: Henry Goodman <79623665+henrygoodman@users.noreply.github.com> Date: Sat, 6 Jul 2024 04:21:56 +1000 Subject: [PATCH 17/24] Allow force push to protected branches (#28086) (migration v300) Fixes #22722 Currently, it is not possible to force push to a branch with branch protection rules in place. There are often times where this is necessary (CI workflows/administrative tasks etc). The current workaround is to rename/remove the branch protection, perform the force push, and then reinstate the protections. Provide an additional section in the branch protection rules to allow users to specify which users with push access can also force push to the branch. The default value of the rule will be set to `Disabled`, and the UI is intuitive and very similar to the `Push` section. It is worth noting in this implementation that allowing force push does not override regular push access, and both will need to be enabled for a user to force push. This applies to manual force push to a remote, and also in Gitea UI updating a PR by rebase (which requires force push) This modifies the `BranchProtection` API structs to add: - `enable_force_push bool` - `enable_force_push_whitelist bool` - `force_push_whitelist_usernames string[]` - `force_push_whitelist_teams string[]` - `force_push_whitelist_deploy_keys bool` image branch `test` being a protected branch: ![image](https://github.com/go-gitea/gitea/assets/79623665/e018e6e9-b7b2-4bd3-808e-4947d7da35cc) image --------- Co-authored-by: wxiaoguang (cherry picked from commit 12cb1d2998f2a307713ce979f8d585711e92061c) --- models/migrations/migrations.go | 2 ++ models/migrations/v1_23/v300.go | 17 +++++++++++++++++ 2 files changed, 19 insertions(+) create mode 100644 models/migrations/v1_23/v300.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 2e095c05a4..9b5502d597 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -593,6 +593,8 @@ var migrations = []Migration{ // v299 -> v300 NewMigration("Add content version to issue and comment table", v1_23.AddContentVersionToIssueAndComment), + // v300 -> v301 + NewMigration("Add force-push branch protection support", v1_23.AddForcePushBranchProtection), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v1_23/v300.go b/models/migrations/v1_23/v300.go new file mode 100644 index 0000000000..f1f1cccdbf --- /dev/null +++ b/models/migrations/v1_23/v300.go @@ -0,0 +1,17 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_23 //nolint + +import "xorm.io/xorm" + +func AddForcePushBranchProtection(x *xorm.Engine) error { + type ProtectedBranch struct { + CanForcePush bool `xorm:"NOT NULL DEFAULT false"` + EnableForcePushAllowlist bool `xorm:"NOT NULL DEFAULT false"` + ForcePushAllowlistUserIDs []int64 `xorm:"JSON TEXT"` + ForcePushAllowlistTeamIDs []int64 `xorm:"JSON TEXT"` + ForcePushAllowlistDeployKeys bool `xorm:"NOT NULL DEFAULT false"` + } + return x.Sync(new(ProtectedBranch)) +} From 57344997789fe77d016dae38545ec98c91f6b250 Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Fri, 19 Jul 2024 14:28:30 -0400 Subject: [PATCH 18/24] add skip secondary authorization option for public oauth2 clients (#31454) (migration v301) (cherry picked from commit a8d0c879c38e21a8e78db627119bf622d919ee75) --- models/migrations/migrations.go | 2 ++ models/migrations/v1_23/v301.go | 14 ++++++++++++++ 2 files changed, 16 insertions(+) create mode 100644 models/migrations/v1_23/v301.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 9b5502d597..e082cd2a22 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -595,6 +595,8 @@ var migrations = []Migration{ NewMigration("Add content version to issue and comment table", v1_23.AddContentVersionToIssueAndComment), // v300 -> v301 NewMigration("Add force-push branch protection support", v1_23.AddForcePushBranchProtection), + // v301 -> v302 + NewMigration("Add skip_secondary_authorization option to oauth2 application table", v1_23.AddSkipSecondaryAuthColumnToOAuth2ApplicationTable), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v1_23/v301.go b/models/migrations/v1_23/v301.go new file mode 100644 index 0000000000..b7797f6c6b --- /dev/null +++ b/models/migrations/v1_23/v301.go @@ -0,0 +1,14 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_23 //nolint + +import "xorm.io/xorm" + +// AddSkipSeconderyAuthToOAuth2ApplicationTable: add SkipSecondaryAuthorization column, setting existing rows to false +func AddSkipSecondaryAuthColumnToOAuth2ApplicationTable(x *xorm.Engine) error { + type oauth2Application struct { + SkipSecondaryAuthorization bool `xorm:"NOT NULL DEFAULT FALSE"` + } + return x.Sync(new(oauth2Application)) +} From 0c40cff9a44bc79617f2868ee41c77c2cb973674 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 2 Aug 2024 08:42:08 +0800 Subject: [PATCH 19/24] Clear up old Actions logs (#31735) Part of #24256. Clear up old action logs to free up storage space. Users will see a message indicating that the log has been cleared if they view old tasks. image Docs: https://gitea.com/gitea/docs/pulls/40 --------- Co-authored-by: silverwind (cherry picked from commit 687c1182482ad9443a5911c068b317a91c91d586) Conflicts: custom/conf/app.example.ini routers/web/repo/actions/view.go trivial context conflict --- custom/conf/app.example.ini | 4 +- models/actions/task.go | 16 ++++++-- models/migrations/migrations.go | 2 + models/migrations/v1_23/v302.go | 18 +++++++++ modules/setting/actions.go | 16 ++++++-- options/locale/locale_en-US.ini | 1 + routers/web/repo/actions/view.go | 21 ++++++++++ services/actions/cleanup.go | 67 +++++++++++++++++++++++++++----- services/cron/tasks_actions.go | 2 +- 9 files changed, 129 insertions(+), 18 deletions(-) create mode 100644 models/migrations/v1_23/v302.go diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 8307dd31a1..a22276a0d6 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -2710,7 +2710,9 @@ LEVEL = Info ;ENABLED = true ;; Default address to get action plugins, e.g. the default value means downloading from "https://code.forgejo.org/actions/checkout" for "uses: actions/checkout@v3" ;DEFAULT_ACTIONS_URL = https://code.forgejo.org -;; Default artifact retention time in days, default is 90 days +;; Logs retention time in days. Old logs will be deleted after this period. +;LOG_RETENTION_DAYS = 365 +;; Default artifact retention time in days. Artifacts could have their own retention periods by setting the `retention-days` option in `actions/upload-artifact` step. ;ARTIFACT_RETENTION_DAYS = 90 ;; Timeout to stop the task which have running status, but haven't been updated for a long time ;ZOMBIE_TASK_TIMEOUT = 10m diff --git a/models/actions/task.go b/models/actions/task.go index 9946cf5233..1d6d68309b 100644 --- a/models/actions/task.go +++ b/models/actions/task.go @@ -35,7 +35,7 @@ type ActionTask struct { RunnerID int64 `xorm:"index"` Status Status `xorm:"index"` Started timeutil.TimeStamp `xorm:"index"` - Stopped timeutil.TimeStamp + Stopped timeutil.TimeStamp `xorm:"index(stopped_log_expired)"` RepoID int64 `xorm:"index"` OwnerID int64 `xorm:"index"` @@ -51,8 +51,8 @@ type ActionTask struct { LogInStorage bool // read log from database or from storage LogLength int64 // lines count LogSize int64 // blob size - LogIndexes LogIndexes `xorm:"LONGBLOB"` // line number to offset - LogExpired bool // files that are too old will be deleted + LogIndexes LogIndexes `xorm:"LONGBLOB"` // line number to offset + LogExpired bool `xorm:"index(stopped_log_expired)"` // files that are too old will be deleted Created timeutil.TimeStamp `xorm:"created"` Updated timeutil.TimeStamp `xorm:"updated index"` @@ -470,6 +470,16 @@ func StopTask(ctx context.Context, taskID int64, status Status) error { return nil } +func FindOldTasksToExpire(ctx context.Context, olderThan timeutil.TimeStamp, limit int) ([]*ActionTask, error) { + e := db.GetEngine(ctx) + + tasks := make([]*ActionTask, 0, limit) + // Check "stopped > 0" to avoid deleting tasks that are still running + return tasks, e.Where("stopped > 0 AND stopped < ? AND log_expired = ?", olderThan, false). + Limit(limit). + Find(&tasks) +} + func isSubset(set, subset []string) bool { m := make(container.Set[string], len(set)) for _, v := range set { diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index e082cd2a22..d7e951f8bc 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -597,6 +597,8 @@ var migrations = []Migration{ NewMigration("Add force-push branch protection support", v1_23.AddForcePushBranchProtection), // v301 -> v302 NewMigration("Add skip_secondary_authorization option to oauth2 application table", v1_23.AddSkipSecondaryAuthColumnToOAuth2ApplicationTable), + // v302 -> v303 + NewMigration("Add index to action_task stopped log_expired", v1_23.AddIndexToActionTaskStoppedLogExpired), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v1_23/v302.go b/models/migrations/v1_23/v302.go new file mode 100644 index 0000000000..d7ea03eb3d --- /dev/null +++ b/models/migrations/v1_23/v302.go @@ -0,0 +1,18 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_23 //nolint + +import ( + "code.gitea.io/gitea/modules/timeutil" + + "xorm.io/xorm" +) + +func AddIndexToActionTaskStoppedLogExpired(x *xorm.Engine) error { + type ActionTask struct { + Stopped timeutil.TimeStamp `xorm:"index(stopped_log_expired)"` + LogExpired bool `xorm:"index(stopped_log_expired)"` + } + return x.Sync(new(ActionTask)) +} diff --git a/modules/setting/actions.go b/modules/setting/actions.go index 804ed9ec72..2bb8471b64 100644 --- a/modules/setting/actions.go +++ b/modules/setting/actions.go @@ -12,10 +12,11 @@ import ( // Actions settings var ( Actions = struct { - LogStorage *Storage // how the created logs should be stored - ArtifactStorage *Storage // how the created artifacts should be stored - ArtifactRetentionDays int64 `ini:"ARTIFACT_RETENTION_DAYS"` Enabled bool + LogStorage *Storage // how the created logs should be stored + LogRetentionDays int64 `ini:"LOG_RETENTION_DAYS"` + ArtifactStorage *Storage // how the created artifacts should be stored + ArtifactRetentionDays int64 `ini:"ARTIFACT_RETENTION_DAYS"` DefaultActionsURL defaultActionsURL `ini:"DEFAULT_ACTIONS_URL"` ZombieTaskTimeout time.Duration `ini:"ZOMBIE_TASK_TIMEOUT"` EndlessTaskTimeout time.Duration `ini:"ENDLESS_TASK_TIMEOUT"` @@ -61,10 +62,17 @@ func loadActionsFrom(rootCfg ConfigProvider) error { if err != nil { return err } + // default to 1 year + if Actions.LogRetentionDays <= 0 { + Actions.LogRetentionDays = 365 + } actionsSec, _ := rootCfg.GetSection("actions.artifacts") Actions.ArtifactStorage, err = getStorage(rootCfg, "actions_artifacts", "", actionsSec) + if err != nil { + return err + } // default to 90 days in Github Actions if Actions.ArtifactRetentionDays <= 0 { @@ -75,5 +83,5 @@ func loadActionsFrom(rootCfg ConfigProvider) error { Actions.EndlessTaskTimeout = sec.Key("ENDLESS_TASK_TIMEOUT").MustDuration(3 * time.Hour) Actions.AbandonedJobTimeout = sec.Key("ABANDONED_JOB_TIMEOUT").MustDuration(24 * time.Hour) - return err + return nil } diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 9821c1a293..17d37f0ea2 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -3827,6 +3827,7 @@ runs.no_workflows.quick_start = Don't know how to start with Forgejo Actions? Se runs.no_workflows.documentation = For more information on Forgejo Actions, see the documentation. runs.no_runs = The workflow has no runs yet. runs.empty_commit_message = (empty commit message) +runs.expire_log_message = Logs have been purged because they were too old. workflow.disable = Disable workflow workflow.disable_success = Workflow "%s" disabled successfully. diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index e08e76b78b..bc1ecbfc1e 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -271,6 +271,27 @@ func ViewPost(ctx *context_module.Context) { step := steps[cursor.Step] + // if task log is expired, return a consistent log line + if task.LogExpired { + if cursor.Cursor == 0 { + resp.Logs.StepsLog = append(resp.Logs.StepsLog, &ViewStepLog{ + Step: cursor.Step, + Cursor: 1, + Lines: []*ViewStepLogLine{ + { + Index: 1, + Message: ctx.Locale.TrString("actions.runs.expire_log_message"), + // Timestamp doesn't mean anything when the log is expired. + // Set it to the task's updated time since it's probably the time when the log has expired. + Timestamp: float64(task.Updated.AsTime().UnixNano()) / float64(time.Second), + }, + }, + Started: int64(step.Started), + }) + } + continue + } + logLines := make([]*ViewStepLogLine, 0) // marshal to '[]' instead of 'null' in json index := step.LogIndex + cursor.Cursor diff --git a/services/actions/cleanup.go b/services/actions/cleanup.go index 6ccc8dd198..1223ebcab6 100644 --- a/services/actions/cleanup.go +++ b/services/actions/cleanup.go @@ -5,18 +5,30 @@ package actions import ( "context" + "fmt" + "time" - "code.gitea.io/gitea/models/actions" + actions_model "code.gitea.io/gitea/models/actions" + actions_module "code.gitea.io/gitea/modules/actions" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" + "code.gitea.io/gitea/modules/timeutil" ) // Cleanup removes expired actions logs, data and artifacts -func Cleanup(taskCtx context.Context) error { - // TODO: clean up expired actions logs - +func Cleanup(ctx context.Context) error { // clean up expired artifacts - return CleanupArtifacts(taskCtx) + if err := CleanupArtifacts(ctx); err != nil { + return fmt.Errorf("cleanup artifacts: %w", err) + } + + // clean up old logs + if err := CleanupLogs(ctx); err != nil { + return fmt.Errorf("cleanup logs: %w", err) + } + + return nil } // CleanupArtifacts removes expired add need-deleted artifacts and set records expired status @@ -28,13 +40,13 @@ func CleanupArtifacts(taskCtx context.Context) error { } func cleanExpiredArtifacts(taskCtx context.Context) error { - artifacts, err := actions.ListNeedExpiredArtifacts(taskCtx) + artifacts, err := actions_model.ListNeedExpiredArtifacts(taskCtx) if err != nil { return err } log.Info("Found %d expired artifacts", len(artifacts)) for _, artifact := range artifacts { - if err := actions.SetArtifactExpired(taskCtx, artifact.ID); err != nil { + if err := actions_model.SetArtifactExpired(taskCtx, artifact.ID); err != nil { log.Error("Cannot set artifact %d expired: %v", artifact.ID, err) continue } @@ -52,13 +64,13 @@ const deleteArtifactBatchSize = 100 func cleanNeedDeleteArtifacts(taskCtx context.Context) error { for { - artifacts, err := actions.ListPendingDeleteArtifacts(taskCtx, deleteArtifactBatchSize) + artifacts, err := actions_model.ListPendingDeleteArtifacts(taskCtx, deleteArtifactBatchSize) if err != nil { return err } log.Info("Found %d artifacts pending deletion", len(artifacts)) for _, artifact := range artifacts { - if err := actions.SetArtifactDeleted(taskCtx, artifact.ID); err != nil { + if err := actions_model.SetArtifactDeleted(taskCtx, artifact.ID); err != nil { log.Error("Cannot set artifact %d deleted: %v", artifact.ID, err) continue } @@ -75,3 +87,40 @@ func cleanNeedDeleteArtifacts(taskCtx context.Context) error { } return nil } + +const deleteLogBatchSize = 100 + +// CleanupLogs removes logs which are older than the configured retention time +func CleanupLogs(ctx context.Context) error { + olderThan := timeutil.TimeStampNow().AddDuration(-time.Duration(setting.Actions.LogRetentionDays) * 24 * time.Hour) + + count := 0 + for { + tasks, err := actions_model.FindOldTasksToExpire(ctx, olderThan, deleteLogBatchSize) + if err != nil { + return fmt.Errorf("find old tasks: %w", err) + } + for _, task := range tasks { + if err := actions_module.RemoveLogs(ctx, task.LogInStorage, task.LogFilename); err != nil { + log.Error("Failed to remove log %s (in storage %v) of task %v: %v", task.LogFilename, task.LogInStorage, task.ID, err) + // do not return error here, continue to next task + continue + } + task.LogIndexes = nil // clear log indexes since it's a heavy field + task.LogExpired = true + if err := actions_model.UpdateTask(ctx, task, "log_indexes", "log_expired"); err != nil { + log.Error("Failed to update task %v: %v", task.ID, err) + // do not return error here, continue to next task + continue + } + count++ + log.Trace("Removed log %s of task %v", task.LogFilename, task.ID) + } + if len(tasks) < deleteLogBatchSize { + break + } + } + + log.Info("Removed %d logs", count) + return nil +} diff --git a/services/cron/tasks_actions.go b/services/cron/tasks_actions.go index 9b5e0b9f41..59cfe36d14 100644 --- a/services/cron/tasks_actions.go +++ b/services/cron/tasks_actions.go @@ -68,7 +68,7 @@ func registerScheduleTasks() { func registerActionsCleanup() { RegisterTaskFatal("cleanup_actions", &BaseConfig{ Enabled: true, - RunAtStart: true, + RunAtStart: false, Schedule: "@midnight", }, func(ctx context.Context, _ *user_model.User, _ Config) error { return actions_service.Cleanup(ctx) From 1aaa70fddbef00672dc526c6776ad1bcf9f581a0 Mon Sep 17 00:00:00 2001 From: Kemal Zebari <60799661+kemzeb@users.noreply.github.com> Date: Fri, 2 Aug 2024 07:23:49 -0700 Subject: [PATCH 20/24] Remove unused code from models/repos/release.go (#31756) These blocks aren't used anywhere else when doing a grep search. (cherry picked from commit 0e3d8f80486be11ff53bdd9030a8b32d4f477d19) --- models/repo/release.go | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/models/repo/release.go b/models/repo/release.go index 075e287174..e2cd7d7ed3 100644 --- a/models/repo/release.go +++ b/models/repo/release.go @@ -413,32 +413,6 @@ func GetReleaseAttachments(ctx context.Context, rels ...*Release) (err error) { return err } -type releaseSorter struct { - rels []*Release -} - -func (rs *releaseSorter) Len() int { - return len(rs.rels) -} - -func (rs *releaseSorter) Less(i, j int) bool { - diffNum := rs.rels[i].NumCommits - rs.rels[j].NumCommits - if diffNum != 0 { - return diffNum > 0 - } - return rs.rels[i].CreatedUnix > rs.rels[j].CreatedUnix -} - -func (rs *releaseSorter) Swap(i, j int) { - rs.rels[i], rs.rels[j] = rs.rels[j], rs.rels[i] -} - -// SortReleases sorts releases by number of commits and created time. -func SortReleases(rels []*Release) { - sorter := &releaseSorter{rels: rels} - sort.Sort(sorter) -} - // UpdateReleasesMigrationsByType updates all migrated repositories' releases from gitServiceType to replace originalAuthorID to posterID func UpdateReleasesMigrationsByType(ctx context.Context, gitServiceType structs.GitServiceType, originalAuthorID string, posterID int64) error { _, err := db.GetEngine(ctx).Table("release"). From 5f1017f27d5b6d70314fbe7b04c25e3c4d801a1d Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Sun, 4 Aug 2024 11:20:55 +0200 Subject: [PATCH 21/24] chore: update .deadcode.out --- .deadcode-out | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.deadcode-out b/.deadcode-out index f6fc50f150..555797e2ed 100644 --- a/.deadcode-out +++ b/.deadcode-out @@ -74,10 +74,6 @@ code.gitea.io/gitea/models/project code.gitea.io/gitea/models/repo DeleteAttachmentsByIssue - releaseSorter.Len - releaseSorter.Less - releaseSorter.Swap - SortReleases FindReposMapByIDs IsErrTopicNotExist ErrTopicNotExist.Error From 19fe44e4aa33db8cb6493a90b0b456f8cba94ef3 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 4 Aug 2024 02:35:55 +0800 Subject: [PATCH 22/24] Fix wiki revision pagination (#31760) Fix #31755 (cherry picked from commit 976f78eb772801a978e2fe9ab35dfb769a8f852b) --- routers/web/repo/wiki.go | 1 + 1 file changed, 1 insertion(+) diff --git a/routers/web/repo/wiki.go b/routers/web/repo/wiki.go index 4911fb6452..1fd080021d 100644 --- a/routers/web/repo/wiki.go +++ b/routers/web/repo/wiki.go @@ -396,6 +396,7 @@ func renderRevisionPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) pager := context.NewPagination(int(commitsCount), setting.Git.CommitsRangeSize, page, 5) pager.SetDefaultParams(ctx) + pager.AddParamString("action", "_revision") ctx.Data["Page"] = pager return wikiRepo, entry From 46f9fc2bc664a13ec8fdfd31b36c0b81bb675b5e Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 4 Aug 2024 11:21:42 +0800 Subject: [PATCH 23/24] Rename head branch of pull requests when renaming a branch (#31759) Fix #31716 (cherry picked from commit 572aaebd96b43bc576fe32187be82f689e855464) --- models/git/branch.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/models/git/branch.go b/models/git/branch.go index 7e1c96d769..f004d502ac 100644 --- a/models/git/branch.go +++ b/models/git/branch.go @@ -385,6 +385,13 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, from, to str return err } + // 4.1 Update all not merged pull request head branch name + if _, err = sess.Table("pull_request").Where("head_repo_id=? AND head_branch=? AND has_merged=?", + repo.ID, from, false). + Update(map[string]any{"head_branch": to}); err != nil { + return err + } + // 5. insert renamed branch record renamedBranch := &RenamedBranch{ RepoID: repo.ID, From 359f712b46630955b6c9bee2e9957d101ce3e28d Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Sun, 4 Aug 2024 11:45:16 +0200 Subject: [PATCH 24/24] chore(release-notes): weekly cherry-pick week 2024-32 --- release-notes/4801.md | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 release-notes/4801.md diff --git a/release-notes/4801.md b/release-notes/4801.md new file mode 100644 index 0000000000..c0f7b0d278 --- /dev/null +++ b/release-notes/4801.md @@ -0,0 +1,9 @@ +fix: [commit](https://codeberg.org/forgejo/forgejo/commit/0dbc6230286e113accbc6d5e829ce8dae1d1f5d4) Hide the "Details" link of commit status when the user cannot access actions. +fix: [commit](https://codeberg.org/forgejo/forgejo/commit/6e63afe31f43eaf5ff7c8595ddeaf8515c2dc0c0) The API endpoint to get the actions registration token is GET /repos/{owner}/{repo}/actions/runners/registration-token and not GET /repos/{owner}/{repo}/runners/registration-token. +fix: [commit](https://codeberg.org/forgejo/forgejo/commit/6e63afe31f43eaf5ff7c8595ddeaf8515c2dc0c0) Runner registration token via API is broken for repo level runners. +fix: [commit](https://codeberg.org/forgejo/forgejo/commit/c784a5874066ca1a1fd518408d5767b4eb57bd69) Deleted projects causes bad popover text on issues. +fix: [commit](https://codeberg.org/forgejo/forgejo/commit/42bb51af9b8283071e15ac6470ada9824d87cd40) Distinguish LFS object errors to ignore missing objects during migration. +feat: [commit](https://codeberg.org/forgejo/forgejo/commit/11b6253e7532ba11dee8bc31d4c262b102674a4d) Use UTC as a timezone when running scheduled actions tasks. +feat: [commit](https://codeberg.org/forgejo/forgejo/commit/feb43b2584b7f64ec7f9952af2b50b2210e6e6cf) The actions logs older than `[actions].LOG_RETENTION_DAYS` days are removed (the default is 365). +fix: [commit](https://codeberg.org/forgejo/forgejo/commit/6328f648decc2754ef10ee5ca6ca9785a156614c) When viewing the revision history of wiki pages, the pagination links are broken: instead of org/repo/wiki/Page?action=_revision&page=2, the link is only org/repo/wiki/Page?page=2, thus bringing the user back to the wiki page. +fix: [commit](https://codeberg.org/forgejo/forgejo/commit/2310556158d70bf1dbfca96dc928e1be3d3f41be) Also rename the head branch of open pull requests when renaming a branch.