diff --git a/.gitignore b/.gitignore index a580861a51db4..821b1b8c672e8 100644 --- a/.gitignore +++ b/.gitignore @@ -25,6 +25,9 @@ __debug_bin* # Visual Studio /.vs/ +# mise version managment tool +mise.toml + *.cgo1.go *.cgo2.c _cgo_defun.c @@ -121,4 +124,3 @@ prime/ /AGENT.md /CLAUDE.md /llms.txt - diff --git a/modules/git/error.go b/modules/git/error.go index 7d131345d0670..d4b5412da9d7d 100644 --- a/modules/git/error.go +++ b/modules/git/error.go @@ -98,28 +98,31 @@ func (err *ErrPushRejected) Unwrap() error { // GenerateMessage generates the remote message from the stderr func (err *ErrPushRejected) GenerateMessage() { - messageBuilder := &strings.Builder{} - i := strings.Index(err.StdErr, "remote: ") - if i < 0 { - err.Message = "" + // The stderr is like this: + // + // > remote: error: push is rejected ..... + // > To /work/gitea/tests/integration/gitea-integration-sqlite/gitea-repositories/user2/repo1.git + // > ! [remote rejected] 44e67c77559211d21b630b902cdcc6ab9d4a4f51 -> develop (pre-receive hook declined) + // > error: failed to push some refs to '/work/gitea/tests/integration/gitea-integration-sqlite/gitea-repositories/user2/repo1.git' + // + // The local message contains sensitive information, so we only need the remote message + const prefixRemote = "remote: " + const prefixError = "error: " + pos := strings.Index(err.StdErr, prefixRemote) + if pos < 0 { + err.Message = "push is rejected" return } - for { - if len(err.StdErr) <= i+8 { - break - } - if err.StdErr[i:i+8] != "remote: " { - break - } - i += 8 - nl := strings.IndexByte(err.StdErr[i:], '\n') - if nl >= 0 { - messageBuilder.WriteString(err.StdErr[i : i+nl+1]) - i = i + nl + 1 - } else { - messageBuilder.WriteString(err.StdErr[i:]) - i = len(err.StdErr) + + messageBuilder := &strings.Builder{} + lines := strings.SplitSeq(err.StdErr, "\n") + for line := range lines { + line, ok := strings.CutPrefix(line, prefixRemote) + if !ok { + continue } + line = strings.TrimPrefix(line, prefixError) + messageBuilder.WriteString(strings.TrimSpace(line) + "\n") } err.Message = strings.TrimSpace(messageBuilder.String()) } diff --git a/modules/structs/repo_file.go b/modules/structs/repo_file.go index 99efe19e4fe6e..4729bde491539 100644 --- a/modules/structs/repo_file.go +++ b/modules/structs/repo_file.go @@ -8,12 +8,14 @@ import "time" // FileOptions options for all file APIs type FileOptions struct { - // message (optional) for the commit of this file. if not supplied, a default message will be used + // message (optional) is the commit message of the changes. If not supplied, a default message will be used Message string `json:"message"` - // branch (optional) to base this file from. if not given, the default branch is used + // branch (optional) is the base branch for the changes. If not supplied, the default branch is used BranchName string `json:"branch" binding:"GitRefName;MaxSize(100)"` - // new_branch (optional) will make a new branch from `branch` before creating the file + // new_branch (optional) will make a new branch from base branch for the changes. If not supplied, the changes will be committed to the base branch NewBranchName string `json:"new_branch" binding:"GitRefName;MaxSize(100)"` + // force_push (optional) will do a force-push if the new branch already exists + ForcePush bool `json:"force_push"` // `author` and `committer` are optional (if only one is given, it will be used for the other, otherwise the authenticated user will be used) Author Identity `json:"author"` Committer Identity `json:"committer"` diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index cd787b9da33e1..2b6348c2fb1eb 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -355,6 +355,7 @@ func ReqChangeRepoFileOptionsAndCheck(ctx *context.APIContext) { Message: commonOpts.Message, OldBranch: commonOpts.BranchName, NewBranch: commonOpts.NewBranchName, + ForcePush: commonOpts.ForcePush, Committer: &files_service.IdentityOptions{ GitUserName: commonOpts.Committer.Name, GitUserEmail: commonOpts.Committer.Email, @@ -591,6 +592,11 @@ func UpdateFile(ctx *context.APIContext) { } func handleChangeRepoFilesError(ctx *context.APIContext, err error) { + if git.IsErrPushRejected(err) { + err := err.(*git.ErrPushRejected) + ctx.APIError(http.StatusForbidden, err.Message) + return + } if files_service.IsErrUserCannotCommit(err) || pull_service.IsErrFilePathProtected(err) { ctx.APIError(http.StatusForbidden, err) return diff --git a/services/packages/cargo/index.go b/services/packages/cargo/index.go index 605335d0f171a..ebcaa3e56dc92 100644 --- a/services/packages/cargo/index.go +++ b/services/packages/cargo/index.go @@ -306,7 +306,7 @@ func alterRepositoryContent(ctx context.Context, doer *user_model.User, repo *re return err } - return t.Push(ctx, doer, commitHash, repo.DefaultBranch) + return t.Push(ctx, doer, commitHash, repo.DefaultBranch, false) } func writeObjectToIndex(ctx context.Context, t *files_service.TemporaryUploadRepository, path string, r io.Reader) error { diff --git a/services/repository/files/cherry_pick.go b/services/repository/files/cherry_pick.go index 6818bb343d2ee..e3b6f678f8707 100644 --- a/services/repository/files/cherry_pick.go +++ b/services/repository/files/cherry_pick.go @@ -131,7 +131,7 @@ func CherryPick(ctx context.Context, repo *repo_model.Repository, doer *user_mod } // Then push this tree to NewBranch - if err := t.Push(ctx, doer, commitHash, opts.NewBranch); err != nil { + if err := t.Push(ctx, doer, commitHash, opts.NewBranch, false); err != nil { return nil, err } diff --git a/services/repository/files/patch.go b/services/repository/files/patch.go index 093072dff20b8..e754ed13c8291 100644 --- a/services/repository/files/patch.go +++ b/services/repository/files/patch.go @@ -201,7 +201,7 @@ func ApplyDiffPatch(ctx context.Context, repo *repo_model.Repository, doer *user } // Then push this tree to NewBranch - if err := t.Push(ctx, doer, commitHash, opts.NewBranch); err != nil { + if err := t.Push(ctx, doer, commitHash, opts.NewBranch, false); err != nil { return nil, err } diff --git a/services/repository/files/temp_repo.go b/services/repository/files/temp_repo.go index dcbe368357b77..40e7901b850c8 100644 --- a/services/repository/files/temp_repo.go +++ b/services/repository/files/temp_repo.go @@ -354,20 +354,18 @@ func (t *TemporaryUploadRepository) CommitTree(ctx context.Context, opts *Commit } // Push the provided commitHash to the repository branch by the provided user -func (t *TemporaryUploadRepository) Push(ctx context.Context, doer *user_model.User, commitHash, branch string) error { +func (t *TemporaryUploadRepository) Push(ctx context.Context, doer *user_model.User, commitHash, branch string, force bool) error { // Because calls hooks we need to pass in the environment env := repo_module.PushingEnvironment(doer, t.repo) if err := git.Push(ctx, t.basePath, git.PushOptions{ Remote: t.repo.RepoPath(), Branch: strings.TrimSpace(commitHash) + ":" + git.BranchPrefix + strings.TrimSpace(branch), Env: env, + Force: force, }); err != nil { if git.IsErrPushOutOfDate(err) { return err } else if git.IsErrPushRejected(err) { - rejectErr := err.(*git.ErrPushRejected) - log.Info("Unable to push back to repo from temporary repo due to rejection: %s (%s)\nStdout: %s\nStderr: %s\nError: %v", - t.repo.FullName(), t.basePath, rejectErr.StdOut, rejectErr.StdErr, rejectErr.Err) return err } log.Error("Unable to push back to repo from temporary repo: %s (%s)\nError: %v", diff --git a/services/repository/files/update.go b/services/repository/files/update.go index e871f777e544a..b07055d57aaa6 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -60,6 +60,7 @@ type ChangeRepoFilesOptions struct { Committer *IdentityOptions Dates *CommitDateOptions Signoff bool + ForcePush bool } type RepoFileOptions struct { @@ -176,8 +177,11 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use return nil, err } if exist { - return nil, git_model.ErrBranchAlreadyExists{ - BranchName: opts.NewBranch, + if !opts.ForcePush { + // branch exists but force option not set + return nil, git_model.ErrBranchAlreadyExists{ + BranchName: opts.NewBranch, + } } } } else if err := VerifyBranchProtection(ctx, repo, doer, opts.OldBranch, treePaths); err != nil { @@ -303,8 +307,7 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use } // Then push this tree to NewBranch - if err := t.Push(ctx, doer, commitHash, opts.NewBranch); err != nil { - log.Error("%T %v", err, err) + if err := t.Push(ctx, doer, commitHash, opts.NewBranch, opts.ForcePush); err != nil { return nil, err } diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 77a622cb63546..0df8356fd9c38 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -21995,7 +21995,7 @@ "$ref": "#/definitions/Identity" }, "branch": { - "description": "branch (optional) to base this file from. if not given, the default branch is used", + "description": "branch (optional) is the base branch for the changes. If not supplied, the default branch is used", "type": "string", "x-go-name": "BranchName" }, @@ -22013,13 +22013,18 @@ }, "x-go-name": "Files" }, + "force_push": { + "description": "force_push (optional) will do a force-push if the new branch already exists", + "type": "boolean", + "x-go-name": "ForcePush" + }, "message": { - "description": "message (optional) for the commit of this file. if not supplied, a default message will be used", + "description": "message (optional) is the commit message of the changes. If not supplied, a default message will be used", "type": "string", "x-go-name": "Message" }, "new_branch": { - "description": "new_branch (optional) will make a new branch from `branch` before creating the file", + "description": "new_branch (optional) will make a new branch from base branch for the changes. If not supplied, the changes will be committed to the base branch", "type": "string", "x-go-name": "NewBranchName" }, @@ -22834,7 +22839,7 @@ "$ref": "#/definitions/Identity" }, "branch": { - "description": "branch (optional) to base this file from. if not given, the default branch is used", + "description": "branch (optional) is the base branch for the changes. If not supplied, the default branch is used", "type": "string", "x-go-name": "BranchName" }, @@ -22849,13 +22854,18 @@ "dates": { "$ref": "#/definitions/CommitDateOptions" }, + "force_push": { + "description": "force_push (optional) will do a force-push if the new branch already exists", + "type": "boolean", + "x-go-name": "ForcePush" + }, "message": { - "description": "message (optional) for the commit of this file. if not supplied, a default message will be used", + "description": "message (optional) is the commit message of the changes. If not supplied, a default message will be used", "type": "string", "x-go-name": "Message" }, "new_branch": { - "description": "new_branch (optional) will make a new branch from `branch` before creating the file", + "description": "new_branch (optional) will make a new branch from base branch for the changes. If not supplied, the changes will be committed to the base branch", "type": "string", "x-go-name": "NewBranchName" }, @@ -23847,7 +23857,7 @@ "$ref": "#/definitions/Identity" }, "branch": { - "description": "branch (optional) to base this file from. if not given, the default branch is used", + "description": "branch (optional) is the base branch for the changes. If not supplied, the default branch is used", "type": "string", "x-go-name": "BranchName" }, @@ -23857,13 +23867,18 @@ "dates": { "$ref": "#/definitions/CommitDateOptions" }, + "force_push": { + "description": "force_push (optional) will do a force-push if the new branch already exists", + "type": "boolean", + "x-go-name": "ForcePush" + }, "message": { - "description": "message (optional) for the commit of this file. if not supplied, a default message will be used", + "description": "message (optional) is the commit message of the changes. If not supplied, a default message will be used", "type": "string", "x-go-name": "Message" }, "new_branch": { - "description": "new_branch (optional) will make a new branch from `branch` before creating the file", + "description": "new_branch (optional) will make a new branch from base branch for the changes. If not supplied, the changes will be committed to the base branch", "type": "string", "x-go-name": "NewBranchName" }, @@ -28639,7 +28654,7 @@ "$ref": "#/definitions/Identity" }, "branch": { - "description": "branch (optional) to base this file from. if not given, the default branch is used", + "description": "branch (optional) is the base branch for the changes. If not supplied, the default branch is used", "type": "string", "x-go-name": "BranchName" }, @@ -28654,18 +28669,23 @@ "dates": { "$ref": "#/definitions/CommitDateOptions" }, + "force_push": { + "description": "force_push (optional) will do a force-push if the new branch already exists", + "type": "boolean", + "x-go-name": "ForcePush" + }, "from_path": { "description": "from_path (optional) is the path of the original file which will be moved/renamed to the path in the URL", "type": "string", "x-go-name": "FromPath" }, "message": { - "description": "message (optional) for the commit of this file. if not supplied, a default message will be used", + "description": "message (optional) is the commit message of the changes. If not supplied, a default message will be used", "type": "string", "x-go-name": "Message" }, "new_branch": { - "description": "new_branch (optional) will make a new branch from `branch` before creating the file", + "description": "new_branch (optional) will make a new branch from base branch for the changes. If not supplied, the changes will be committed to the base branch", "type": "string", "x-go-name": "NewBranchName" }, diff --git a/tests/integration/api_repo_files_change_test.go b/tests/integration/api_repo_files_change_test.go index 999bcdc680fbc..47fe5066a7138 100644 --- a/tests/integration/api_repo_files_change_test.go +++ b/tests/integration/api_repo_files_change_test.go @@ -161,9 +161,88 @@ func TestAPIChangeFiles(t *testing.T) { assert.Equal(t, expectedUpdateHTMLURL, *filesResponse.Files[1].HTMLURL) assert.Equal(t, expectedUpdateDownloadURL, *filesResponse.Files[1].DownloadURL) assert.Nil(t, filesResponse.Files[2]) + assert.Equal(t, changeFilesOptions.Message+"\n", filesResponse.Commit.Message) + + // Test fails creating a file in a branch that already exists without force + changeFilesOptions = getChangeFilesOptions() + changeFilesOptions.BranchName = repo1.DefaultBranch + changeFilesOptions.NewBranchName = "develop" + changeFilesOptions.ForcePush = false + fileID++ + createTreePath = fmt.Sprintf("new/file%d.txt", fileID) + updateTreePath = fmt.Sprintf("update/file%d.txt", fileID) + deleteTreePath = fmt.Sprintf("delete/file%d.txt", fileID) + changeFilesOptions.Files[0].Path = createTreePath + changeFilesOptions.Files[1].Path = updateTreePath + changeFilesOptions.Files[2].Path = deleteTreePath + createFile(user2, repo1, updateTreePath) + createFile(user2, repo1, deleteTreePath) + url = fmt.Sprintf("/api/v1/repos/%s/%s/contents", user2.Name, repo1.Name) + req = NewRequestWithJSON(t, "POST", url, &changeFilesOptions). + AddTokenAuth(token2) + resp = MakeRequest(t, req, http.StatusUnprocessableEntity) + assert.Contains(t, resp.Body.String(), `"message":"branch already exists [name: develop]"`) + // Test succeeds creating a file in a branch that already exists with force + changeFilesOptions = getChangeFilesOptions() + changeFilesOptions.BranchName = repo1.DefaultBranch + changeFilesOptions.NewBranchName = "develop" + changeFilesOptions.ForcePush = true + fileID++ + createTreePath = fmt.Sprintf("new/file%d.txt", fileID) + updateTreePath = fmt.Sprintf("update/file%d.txt", fileID) + deleteTreePath = fmt.Sprintf("delete/file%d.txt", fileID) + changeFilesOptions.Files[0].Path = createTreePath + changeFilesOptions.Files[1].Path = updateTreePath + changeFilesOptions.Files[2].Path = deleteTreePath + createFile(user2, repo1, updateTreePath) + createFile(user2, repo1, deleteTreePath) + url = fmt.Sprintf("/api/v1/repos/%s/%s/contents", user2.Name, repo1.Name) + req = NewRequestWithJSON(t, "POST", url, &changeFilesOptions). + AddTokenAuth(token2) + resp = MakeRequest(t, req, http.StatusCreated) + DecodeJSON(t, resp, &filesResponse) + expectedCreateHTMLURL = fmt.Sprintf(setting.AppURL+"user2/repo1/src/branch/develop/new/file%d.txt", fileID) + expectedCreateDownloadURL = fmt.Sprintf(setting.AppURL+"user2/repo1/raw/branch/develop/new/file%d.txt", fileID) + expectedUpdateHTMLURL = fmt.Sprintf(setting.AppURL+"user2/repo1/src/branch/develop/update/file%d.txt", fileID) + expectedUpdateDownloadURL = fmt.Sprintf(setting.AppURL+"user2/repo1/raw/branch/develop/update/file%d.txt", fileID) + assert.Equal(t, expectedCreateSHA, filesResponse.Files[0].SHA) + assert.Equal(t, expectedCreateHTMLURL, *filesResponse.Files[0].HTMLURL) + assert.Equal(t, expectedCreateDownloadURL, *filesResponse.Files[0].DownloadURL) + assert.Equal(t, expectedUpdateSHA, filesResponse.Files[1].SHA) + assert.Equal(t, expectedUpdateHTMLURL, *filesResponse.Files[1].HTMLURL) + assert.Equal(t, expectedUpdateDownloadURL, *filesResponse.Files[1].DownloadURL) + assert.Nil(t, filesResponse.Files[2]) assert.Equal(t, changeFilesOptions.Message+"\n", filesResponse.Commit.Message) + // Test fails creating a file in a branch that already exists with force and branch protection enabled + protectionReq := NewRequestWithJSON(t, "POST", "/api/v1/repos/user2/repo1/branch_protections", &api.BranchProtection{ + RuleName: "develop", + BranchName: "develop", + Priority: 1, + EnablePush: true, + EnableForcePush: false, + }).AddTokenAuth(token2) + MakeRequest(t, protectionReq, http.StatusCreated) + changeFilesOptions = getChangeFilesOptions() + changeFilesOptions.BranchName = repo1.DefaultBranch + changeFilesOptions.NewBranchName = "develop" + changeFilesOptions.ForcePush = true + fileID++ + createTreePath = fmt.Sprintf("new/file%d.txt", fileID) + updateTreePath = fmt.Sprintf("update/file%d.txt", fileID) + deleteTreePath = fmt.Sprintf("delete/file%d.txt", fileID) + changeFilesOptions.Files[0].Path = createTreePath + changeFilesOptions.Files[1].Path = updateTreePath + changeFilesOptions.Files[2].Path = deleteTreePath + createFile(user2, repo1, updateTreePath) + createFile(user2, repo1, deleteTreePath) + url = fmt.Sprintf("/api/v1/repos/%s/%s/contents", user2.Name, repo1.Name) + req = NewRequestWithJSON(t, "POST", url, &changeFilesOptions). + AddTokenAuth(token2) + resp = MakeRequest(t, req, http.StatusForbidden) + assert.Contains(t, resp.Body.String(), `"message":"branch develop is protected from force push"`) + // Test updating a file and renaming it changeFilesOptions = getChangeFilesOptions() changeFilesOptions.BranchName = repo1.DefaultBranch