From 67bd9d4f1eedb4728031504d0dd09d014c0f3e6f Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 30 Jun 2023 15:26:36 +0800 Subject: [PATCH 01/87] Restrict `[actions].DEFAULT_ACTIONS_URL` to only `github` or `self` (#25581) Resolve #24789 ## :warning: BREAKING :warning: Before this, `DEFAULT_ACTIONS_URL` cound be set to any custom URLs like `https://gitea.com` or `http://your-git-server,https://gitea.com`, and the default value was `https://gitea.com`. But now, `DEFAULT_ACTIONS_URL` supports only `github`(`https://github.com`) or `self`(the root url of current Gitea instance), and the default value is `github`. If it has configured with a URL, an error log will be displayed and it will fallback to `github`. Actually, what we really want to do is always make it `https://github.com`, however, this may not be acceptable for some instances of internal use, so there's extra support for `self`, but no more, even `https://gitea.com`. Please note that `uses: https://xxx/yyy/zzz` always works and it does exactly what it is supposed to do. Although it's breaking, I belive it should be backported to `v1.20` due to some security issues. Follow-up on the runner side: - https://gitea.com/gitea/act_runner/pulls/262 - https://gitea.com/gitea/act/pulls/70 --- custom/conf/app.example.ini | 4 +- .../config-cheat-sheet.en-us.md | 39 +++------ modules/setting/actions.go | 43 +++++++++- modules/setting/actions_test.go | 84 +++++++++++++++++++ routers/api/actions/runner/utils.go | 2 +- 5 files changed, 139 insertions(+), 33 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 57adce83c0..b2b6739f38 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -2541,8 +2541,8 @@ LEVEL = Info ;; Enable/Disable actions capabilities ;ENABLED = false ;; -;; Default address to get action plugins, e.g. the default value means downloading from "https://gitea.com/actions/checkout" for "uses: actions/checkout@v3" -;DEFAULT_ACTIONS_URL = https://gitea.com +;; Default platform to get action plugins, `github` for `https://github.com`, `self` for the current Gitea instance. +;DEFAULT_ACTIONS_URL = github ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/docs/content/doc/administration/config-cheat-sheet.en-us.md b/docs/content/doc/administration/config-cheat-sheet.en-us.md index 2b95110349..aefa351ecd 100644 --- a/docs/content/doc/administration/config-cheat-sheet.en-us.md +++ b/docs/content/doc/administration/config-cheat-sheet.en-us.md @@ -1376,39 +1376,22 @@ PROXY_HOSTS = *.github.com ## Actions (`actions`) - `ENABLED`: **false**: Enable/Disable actions capabilities -- `DEFAULT_ACTIONS_URL`: **https://gitea.com**: Default address to get action plugins, e.g. the default value means downloading from "" for "uses: actions/checkout@v3" +- `DEFAULT_ACTIONS_URL`: **github**: Default platform to get action plugins, `github` for `https://github.com`, `self` for the current Gitea instance. - `STORAGE_TYPE`: **local**: Storage type for actions logs, `local` for local disk or `minio` for s3 compatible object storage service, default is `local` or other name defined with `[storage.xxx]` - `MINIO_BASE_PATH`: **actions_log/**: Minio base path on the bucket only available when STORAGE_TYPE is `minio` -`DEFAULT_ACTIONS_URL` indicates where should we find the relative path action plugin. i.e. when use an action in a workflow file like +`DEFAULT_ACTIONS_URL` indicates where the Gitea Actions runners should find the actions with relative path. +For example, `uses: actions/checkout@v3` means `https://github.com/actions/checkout@v3` since the value of `DEFAULT_ACTIONS_URL` is `github`. +And it can be changed to `self` to make it `root_url_of_your_gitea/actions/checkout@v3`. -```yaml -name: versions -on: - push: - branches: - - main - - releases/* -jobs: - build: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v3 -``` +Please note that using `self` is not recommended for most cases, as it could make names globally ambiguous. +Additionally, it requires you to mirror all the actions you need to your Gitea instance, which may not be worth it. +Therefore, please use `self` only if you understand what you are doing. -Now we need to know how to get actions/checkout, this configuration is the default git server to get it. That means we will get the repository via git clone ${DEFAULT_ACTIONS_URL}/actions/checkout and fetch tag v3. - -To help people who don't want to mirror these actions in their git instances, the default value is https://gitea.com -To help people run actions totally in their network, they can change the value and copy all necessary action repositories into their git server. - -Of course we should support the form in future PRs like - -```yaml -steps: - - uses: gitea.com/actions/checkout@v3 -``` - -although Github don't support this form. +In earlier versions (<= 1.19), `DEFAULT_ACTIONS_URL` cound be set to any custom URLs like `https://gitea.com` or `http://your-git-server,https://gitea.com`, and the default value was `https://gitea.com`. +However, later updates removed those options, and now the only options are `github` and `self`, with the default value being `github`. +However, if you want to use actions from other git server, you can use a complete URL in `uses` field, it's supported by Gitea (but not GitHub). +Like `uses: https://gitea.com/actions/checkout@v3` or `uses: http://your-git-server/actions/checkout@v3`. ## Other (`other`) diff --git a/modules/setting/actions.go b/modules/setting/actions.go index 1c8075cd6c..a13330dcd1 100644 --- a/modules/setting/actions.go +++ b/modules/setting/actions.go @@ -5,6 +5,9 @@ package setting import ( "fmt" + "strings" + + "code.gitea.io/gitea/modules/log" ) // Actions settings @@ -13,13 +16,36 @@ var ( LogStorage *Storage // how the created logs should be stored ArtifactStorage *Storage // how the created artifacts should be stored Enabled bool - DefaultActionsURL string `ini:"DEFAULT_ACTIONS_URL"` + DefaultActionsURL defaultActionsURL `ini:"DEFAULT_ACTIONS_URL"` }{ Enabled: false, - DefaultActionsURL: "https://gitea.com", + DefaultActionsURL: defaultActionsURLGitHub, } ) +type defaultActionsURL string + +func (url defaultActionsURL) URL() string { + switch url { + case defaultActionsURLGitHub: + return "https://github.com" + case defaultActionsURLSelf: + return strings.TrimSuffix(AppURL, "/") + default: + // This should never happen, but just in case, use GitHub as fallback + return "https://github.com" + } +} + +const ( + defaultActionsURLGitHub = "github" // https://github.com + defaultActionsURLSelf = "self" // the root URL of the self-hosted Gitea instance + // DefaultActionsURL only supports GitHub and the self-hosted Gitea. + // It's intentionally not supported more, so please be cautious before adding more like "gitea" or "gitlab". + // If you get some trouble with `uses: username/action_name@version` in your workflow, + // please consider to use `uses: https://the_url_you_want_to_use/username/action_name@version` instead. +) + func loadActionsFrom(rootCfg ConfigProvider) error { sec := rootCfg.Section("actions") err := sec.MapTo(&Actions) @@ -27,6 +53,19 @@ func loadActionsFrom(rootCfg ConfigProvider) error { return fmt.Errorf("failed to map Actions settings: %v", err) } + if urls := string(Actions.DefaultActionsURL); urls != defaultActionsURLGitHub && urls != defaultActionsURLSelf { + url := strings.Split(urls, ",")[0] + if strings.HasPrefix(url, "https://") || strings.HasPrefix(url, "http://") { + log.Error("[actions] DEFAULT_ACTIONS_URL does not support %q as custom URL any longer, fallback to %q", + urls, + defaultActionsURLGitHub, + ) + Actions.DefaultActionsURL = defaultActionsURLGitHub + } else { + return fmt.Errorf("unsupported [actions] DEFAULT_ACTIONS_URL: %q", urls) + } + } + // don't support to read configuration from [actions] Actions.LogStorage, err = getStorage(rootCfg, "actions_log", "", nil) if err != nil { diff --git a/modules/setting/actions_test.go b/modules/setting/actions_test.go index a1cc8fe333..3645a3f5da 100644 --- a/modules/setting/actions_test.go +++ b/modules/setting/actions_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func Test_getStorageInheritNameSectionTypeForActions(t *testing.T) { @@ -95,3 +96,86 @@ STORAGE_TYPE = minio assert.EqualValues(t, "local", Actions.ArtifactStorage.Type) assert.EqualValues(t, "actions_artifacts", filepath.Base(Actions.ArtifactStorage.Path)) } + +func Test_getDefaultActionsURLForActions(t *testing.T) { + oldActions := Actions + oldAppURL := AppURL + defer func() { + Actions = oldActions + AppURL = oldAppURL + }() + + AppURL = "http://test_get_default_actions_url_for_actions:3000/" + + tests := []struct { + name string + iniStr string + wantErr assert.ErrorAssertionFunc + wantURL string + }{ + { + name: "default", + iniStr: ` +[actions] +`, + wantErr: assert.NoError, + wantURL: "https://github.com", + }, + { + name: "github", + iniStr: ` +[actions] +DEFAULT_ACTIONS_URL = github +`, + wantErr: assert.NoError, + wantURL: "https://github.com", + }, + { + name: "self", + iniStr: ` +[actions] +DEFAULT_ACTIONS_URL = self +`, + wantErr: assert.NoError, + wantURL: "http://test_get_default_actions_url_for_actions:3000", + }, + { + name: "custom url", + iniStr: ` +[actions] +DEFAULT_ACTIONS_URL = https://gitea.com +`, + wantErr: assert.NoError, + wantURL: "https://github.com", + }, + { + name: "custom urls", + iniStr: ` +[actions] +DEFAULT_ACTIONS_URL = https://gitea.com,https://github.com +`, + wantErr: assert.NoError, + wantURL: "https://github.com", + }, + { + name: "invalid", + iniStr: ` +[actions] +DEFAULT_ACTIONS_URL = gitea +`, + wantErr: assert.Error, + wantURL: "https://github.com", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg, err := NewConfigProviderFromData(tt.iniStr) + require.NoError(t, err) + if !tt.wantErr(t, loadActionsFrom(cfg)) { + return + } + assert.EqualValues(t, tt.wantURL, Actions.DefaultActionsURL.URL()) + }) + } +} diff --git a/routers/api/actions/runner/utils.go b/routers/api/actions/runner/utils.go index ab70f622b3..3370355f15 100644 --- a/routers/api/actions/runner/utils.go +++ b/routers/api/actions/runner/utils.go @@ -174,7 +174,7 @@ func generateTaskContext(t *actions_model.ActionTask) *structpb.Struct { "workspace": "", // string, The default working directory on the runner for steps, and the default location of your repository when using the checkout action. // additional contexts - "gitea_default_actions_url": setting.Actions.DefaultActionsURL, + "gitea_default_actions_url": setting.Actions.DefaultActionsURL.URL(), }) if err != nil { log.Error("structpb.NewStruct failed: %v", err) From 65acd1e9ef9eda01fd9b07a72abafa2d7bf83435 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 30 Jun 2023 17:03:05 +0800 Subject: [PATCH 02/87] Fix branch commit message too long problem (#25588) When branch's commit CommitMessage is too long, the column maybe too short.(TEXT 16K for mysql). This PR will fix it to only store the summary because these message will only show on branch list or possible future search? --- models/git/branch.go | 15 ++++++++------- models/git/branch_test.go | 11 ++++++++++- modules/repository/branch.go | 8 ++++---- services/repository/push.go | 2 +- 4 files changed, 23 insertions(+), 13 deletions(-) diff --git a/models/git/branch.go b/models/git/branch.go index adf8b0a78d..255b230b51 100644 --- a/models/git/branch.go +++ b/models/git/branch.go @@ -11,6 +11,7 @@ import ( "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" @@ -204,14 +205,14 @@ func DeleteBranches(ctx context.Context, repoID, doerID int64, branchIDs []int64 // UpdateBranch updates the branch information in the database. If the branch exist, it will update latest commit of this branch information // If it doest not exist, insert a new record into database -func UpdateBranch(ctx context.Context, repoID int64, branchName, commitID, commitMessage string, pusherID int64, commitTime time.Time) error { +func UpdateBranch(ctx context.Context, repoID, pusherID int64, branchName string, commit *git.Commit) error { cnt, err := db.GetEngine(ctx).Where("repo_id=? AND name=?", repoID, branchName). Cols("commit_id, commit_message, pusher_id, commit_time, is_deleted, updated_unix"). Update(&Branch{ - CommitID: commitID, - CommitMessage: commitMessage, + CommitID: commit.ID.String(), + CommitMessage: commit.Summary(), PusherID: pusherID, - CommitTime: timeutil.TimeStamp(commitTime.Unix()), + CommitTime: timeutil.TimeStamp(commit.Committer.When.Unix()), IsDeleted: false, }) if err != nil { @@ -224,10 +225,10 @@ func UpdateBranch(ctx context.Context, repoID int64, branchName, commitID, commi return db.Insert(ctx, &Branch{ RepoID: repoID, Name: branchName, - CommitID: commitID, - CommitMessage: commitMessage, + CommitID: commit.ID.String(), + CommitMessage: commit.Summary(), PusherID: pusherID, - CommitTime: timeutil.TimeStamp(commitTime.Unix()), + CommitTime: timeutil.TimeStamp(commit.Committer.When.Unix()), }) } diff --git a/models/git/branch_test.go b/models/git/branch_test.go index bb63660d07..ba69026927 100644 --- a/models/git/branch_test.go +++ b/models/git/branch_test.go @@ -11,6 +11,7 @@ import ( issues_model "code.gitea.io/gitea/models/issues" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" @@ -28,7 +29,15 @@ func TestAddDeletedBranch(t *testing.T) { secondBranch := unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo.ID, Name: "branch2"}) assert.True(t, secondBranch.IsDeleted) - err := git_model.UpdateBranch(db.DefaultContext, repo.ID, secondBranch.Name, secondBranch.CommitID, secondBranch.CommitMessage, secondBranch.PusherID, secondBranch.CommitTime.AsLocalTime()) + commit := &git.Commit{ + ID: git.MustIDFromString(secondBranch.CommitID), + CommitMessage: secondBranch.CommitMessage, + Committer: &git.Signature{ + When: secondBranch.CommitTime.AsLocalTime(), + }, + } + + err := git_model.UpdateBranch(db.DefaultContext, repo.ID, secondBranch.PusherID, secondBranch.Name, commit) assert.NoError(t, err) } diff --git a/modules/repository/branch.go b/modules/repository/branch.go index 7fd29e3f7d..bffadd62f4 100644 --- a/modules/repository/branch.go +++ b/modules/repository/branch.go @@ -77,9 +77,9 @@ func SyncRepoBranchesWithRepo(ctx context.Context, repo *repo_model.Repository, RepoID: repo.ID, Name: branch, CommitID: commit.ID.String(), - CommitMessage: commit.CommitMessage, + CommitMessage: commit.Summary(), PusherID: doerID, - CommitTime: timeutil.TimeStamp(commit.Author.When.Unix()), + CommitTime: timeutil.TimeStamp(commit.Committer.When.Unix()), }) } else if commit.ID.String() != dbb.CommitID { toUpdate = append(toUpdate, &git_model.Branch{ @@ -87,9 +87,9 @@ func SyncRepoBranchesWithRepo(ctx context.Context, repo *repo_model.Repository, RepoID: repo.ID, Name: branch, CommitID: commit.ID.String(), - CommitMessage: commit.CommitMessage, + CommitMessage: commit.Summary(), PusherID: doerID, - CommitTime: timeutil.TimeStamp(commit.Author.When.Unix()), + CommitTime: timeutil.TimeStamp(commit.Committer.When.Unix()), }) } } diff --git a/services/repository/push.go b/services/repository/push.go index 7e7069f580..8e4bab1562 100644 --- a/services/repository/push.go +++ b/services/repository/push.go @@ -259,7 +259,7 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { notification.NotifyPushCommits(ctx, pusher, repo, opts, commits) - if err = git_model.UpdateBranch(ctx, repo.ID, branch, newCommit.ID.String(), newCommit.CommitMessage, opts.PusherID, newCommit.Committer.When); err != nil { + if err = git_model.UpdateBranch(ctx, repo.ID, opts.PusherID, branch, newCommit); err != nil { return fmt.Errorf("git_model.UpdateBranch %s:%s failed: %v", repo.FullName(), branch, err) } From ed8a8af99f125cebc8291e510d61511f8e56b04f Mon Sep 17 00:00:00 2001 From: sebastian-sauer Date: Fri, 30 Jun 2023 18:08:18 +0200 Subject: [PATCH 03/87] Use AfterCommitId to get commit for Viewed functionality (#25529) the PullHeadCommitID is not always available when the PR is merged. Not sure if this is the best solution but in my simple tests it looks like this fixes the problem - happy to get any feedback. hopefully fixes https://github.com/go-gitea/gitea/issues/24813 --- templates/repo/diff/box.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl index 1198452f7e..8f570e5efc 100644 --- a/templates/repo/diff/box.tmpl +++ b/templates/repo/diff/box.tmpl @@ -147,7 +147,7 @@ {{end}} {{end}} {{if $isReviewFile}} -