From ea1b3cb82c7cd4c2720a094af27b278eb3015576 Mon Sep 17 00:00:00 2001 From: Rob Gonnella Date: Sun, 28 Sep 2025 10:50:53 -0400 Subject: [PATCH 1/6] feat: adds option to force update new branch in contents routes Allows users to specify a "force" option in API /contents routes when modifying files in a new branch. When "force" is true, and the branch already exists, a force push will occur provided the branch does not have a branch protection rule that disables force pushing. This is useful as a way to manage a branch remotely through only the API. For example in an automated release tool you can pull commits, analyze, and update a release PR branch all remotely without needing to clone or perform any local git operations. [#35538](https://github.com/go-gitea/gitea/issues/35538) --- .gitignore | 4 +- models/git/branch.go | 19 +++++ modules/structs/repo_file.go | 2 + routers/api/v1/repo/file.go | 3 +- services/packages/cargo/index.go | 2 +- services/repository/files/cherry_pick.go | 2 +- services/repository/files/patch.go | 2 +- services/repository/files/temp_repo.go | 3 +- services/repository/files/update.go | 32 ++++++-- templates/swagger/v1_json.tmpl | 20 +++++ .../integration/api_repo_file_create_test.go | 48 ++++++++++++ .../integration/api_repo_file_delete_test.go | 48 ++++++++++++ .../integration/api_repo_file_update_test.go | 51 ++++++++++++ .../integration/api_repo_files_change_test.go | 78 +++++++++++++++++++ 14 files changed, 301 insertions(+), 13 deletions(-) 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/models/git/branch.go b/models/git/branch.go index 54351649cc5ec..34f516d97d5fe 100644 --- a/models/git/branch.go +++ b/models/git/branch.go @@ -61,6 +61,25 @@ func (err ErrBranchAlreadyExists) Unwrap() error { return util.ErrAlreadyExist } +// ErrBranchProtected represents an error that a branch cannot be force updated due to branch protections +type ErrBranchProtected struct { + BranchName string +} + +// IsErrBranchAlreadyExists checks if an error is an ErrBranchAlreadyExists. +func IsErrBranchProtected(err error) bool { + _, ok := err.(ErrBranchProtected) + return ok +} + +func (err ErrBranchProtected) Error() string { + return fmt.Sprintf("branch cannot be force updated due to branch protection rules [name: %s]", err.BranchName) +} + +func (err ErrBranchProtected) Unwrap() error { + return util.ErrPermissionDenied +} + // ErrBranchNameConflict represents an error that branch name conflicts with other branch. type ErrBranchNameConflict struct { BranchName string diff --git a/modules/structs/repo_file.go b/modules/structs/repo_file.go index 99efe19e4fe6e..ad162df960ecc 100644 --- a/modules/structs/repo_file.go +++ b/modules/structs/repo_file.go @@ -14,6 +14,8 @@ type FileOptions struct { BranchName string `json:"branch" binding:"GitRefName;MaxSize(100)"` // new_branch (optional) will make a new branch from `branch` before creating the file NewBranchName string `json:"new_branch" binding:"GitRefName;MaxSize(100)"` + // force (optional) will force update the new branch if it already exists + Force bool `json:"force"` // `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..ddf0a315c27ed 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, + Force: commonOpts.Force, Committer: &files_service.IdentityOptions{ GitUserName: commonOpts.Committer.Name, GitUserEmail: commonOpts.Committer.Email, @@ -595,7 +596,7 @@ func handleChangeRepoFilesError(ctx *context.APIContext, err error) { ctx.APIError(http.StatusForbidden, err) return } - if git_model.IsErrBranchAlreadyExists(err) || files_service.IsErrFilenameInvalid(err) || pull_service.IsErrSHADoesNotMatch(err) || + if git_model.IsErrBranchAlreadyExists(err) || git_model.IsErrBranchProtected(err) || files_service.IsErrFilenameInvalid(err) || pull_service.IsErrSHADoesNotMatch(err) || files_service.IsErrFilePathInvalid(err) || files_service.IsErrRepoFileAlreadyExists(err) || files_service.IsErrCommitIDDoesNotMatch(err) || files_service.IsErrSHAOrCommitIDNotProvided(err) { ctx.APIError(http.StatusUnprocessableEntity, err) 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..90f1c5970edfc 100644 --- a/services/repository/files/temp_repo.go +++ b/services/repository/files/temp_repo.go @@ -354,13 +354,14 @@ 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 diff --git a/services/repository/files/update.go b/services/repository/files/update.go index e871f777e544a..c0bce25c4d51a 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 + Force bool } type RepoFileOptions struct { @@ -168,19 +169,30 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use } // A NewBranch can be specified for the file to be created/updated in a new branch. - // Check to make sure the branch does not already exist, otherwise we can't proceed. - // If we aren't branching to a new branch, make sure user can commit to the given branch + // Check to to see if the branch already exist. If it does, ensure Force option is set + // and VerifyBranchProtection passes if opts.NewBranch != opts.OldBranch { exist, err := git_model.IsBranchExist(ctx, repo.ID, opts.NewBranch) if err != nil { return nil, err } + if exist { - return nil, git_model.ErrBranchAlreadyExists{ - BranchName: opts.NewBranch, + if opts.Force { + // ensure branch can be force pushed + if err := VerifyBranchProtection(ctx, repo, doer, opts.NewBranch, treePaths, opts.Force); err != nil { + return nil, git_model.ErrBranchProtected{ + BranchName: opts.NewBranch, + } + } + } else { + // 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 { + } else if err := VerifyBranchProtection(ctx, repo, doer, opts.OldBranch, treePaths, opts.Force); err != nil { return nil, err } @@ -303,7 +315,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 { + if err := t.Push(ctx, doer, commitHash, opts.NewBranch, opts.Force); err != nil { log.Error("%T %v", err, err) return nil, err } @@ -685,7 +697,7 @@ func writeRepoObjectForRename(ctx context.Context, t *TemporaryUploadRepository, } // VerifyBranchProtection verify the branch protection for modifying the given treePath on the given branch -func VerifyBranchProtection(ctx context.Context, repo *repo_model.Repository, doer *user_model.User, branchName string, treePaths []string) error { +func VerifyBranchProtection(ctx context.Context, repo *repo_model.Repository, doer *user_model.User, branchName string, treePaths []string, force bool) error { protectedBranch, err := git_model.GetFirstMatchProtectedBranchRule(ctx, repo.ID, branchName) if err != nil { return err @@ -695,6 +707,7 @@ func VerifyBranchProtection(ctx context.Context, repo *repo_model.Repository, do globUnprotected := protectedBranch.GetUnprotectedFilePatterns() globProtected := protectedBranch.GetProtectedFilePatterns() canUserPush := protectedBranch.CanUserPush(ctx, doer) + canUserForcePush := protectedBranch.CanUserForcePush(ctx, doer) for _, treePath := range treePaths { isUnprotectedFile := false if len(globUnprotected) != 0 { @@ -705,6 +718,11 @@ func VerifyBranchProtection(ctx context.Context, repo *repo_model.Repository, do UserName: doer.LowerName, } } + if force && !canUserForcePush && !isUnprotectedFile { + return ErrUserCannotCommit{ + UserName: doer.LowerName, + } + } if protectedBranch.IsProtectedFile(globProtected, treePath) { return pull_service.ErrFilePathProtected{ Path: treePath, diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 77a622cb63546..d4f577c5cfada 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -22013,6 +22013,11 @@ }, "x-go-name": "Files" }, + "force": { + "description": "force updates new_branch if branch already exists", + "type": "boolean", + "x-go-name": "Force" + }, "message": { "description": "message (optional) for the commit of this file. if not supplied, a default message will be used", "type": "string", @@ -22849,6 +22854,11 @@ "dates": { "$ref": "#/definitions/CommitDateOptions" }, + "force": { + "description": "force updates new_branch if branch already exists", + "type": "boolean", + "x-go-name": "Force" + }, "message": { "description": "message (optional) for the commit of this file. if not supplied, a default message will be used", "type": "string", @@ -23857,6 +23867,11 @@ "dates": { "$ref": "#/definitions/CommitDateOptions" }, + "force": { + "description": "force updates new_branch if branch already exists", + "type": "boolean", + "x-go-name": "Force" + }, "message": { "description": "message (optional) for the commit of this file. if not supplied, a default message will be used", "type": "string", @@ -28659,6 +28674,11 @@ "type": "string", "x-go-name": "FromPath" }, + "force": { + "description": "force updates new_branch if branch already exists", + "type": "boolean", + "x-go-name": "Force" + }, "message": { "description": "message (optional) for the commit of this file. if not supplied, a default message will be used", "type": "string", diff --git a/tests/integration/api_repo_file_create_test.go b/tests/integration/api_repo_file_create_test.go index af3bc546803a1..9d4a1a30bcf9f 100644 --- a/tests/integration/api_repo_file_create_test.go +++ b/tests/integration/api_repo_file_create_test.go @@ -228,6 +228,54 @@ func TestAPICreateFile(t *testing.T) { assert.Equal(t, expectedDownloadURL, *fileResponse.Content.DownloadURL) assert.Equal(t, createFileOptions.Message+"\n", fileResponse.Commit.Message) + // Test fails creating a file in a branch that already exists without force + createFileOptions = getCreateFileOptions() + createFileOptions.BranchName = repo1.DefaultBranch + createFileOptions.NewBranchName = "develop" + createFileOptions.Force = false + fileID++ + treePath = fmt.Sprintf("new/file%d.txt", fileID) + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s", user2.Name, repo1.Name, treePath), &createFileOptions). + AddTokenAuth(token2) + MakeRequest(t, req, http.StatusUnprocessableEntity) + + // Test succeeds creating a file in a branch that already exists with force + createFileOptions = getCreateFileOptions() + createFileOptions.BranchName = repo1.DefaultBranch + createFileOptions.NewBranchName = "develop" + createFileOptions.Force = true + fileID++ + treePath = fmt.Sprintf("new/file%d.txt", fileID) + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s", user2.Name, repo1.Name, treePath), &createFileOptions). + AddTokenAuth(token2) + resp = MakeRequest(t, req, http.StatusCreated) + DecodeJSON(t, resp, &fileResponse) + expectedHTMLURL = fmt.Sprintf(setting.AppURL+"user2/repo1/src/branch/develop/new/file%d.txt", fileID) + expectedDownloadURL = fmt.Sprintf(setting.AppURL+"user2/repo1/raw/branch/develop/new/file%d.txt", fileID) + assert.Equal(t, expectedSHA, fileResponse.Content.SHA) + assert.Equal(t, expectedHTMLURL, *fileResponse.Content.HTMLURL) + assert.Equal(t, expectedDownloadURL, *fileResponse.Content.DownloadURL) + assert.Equal(t, createFileOptions.Message+"\n", fileResponse.Commit.Message) + + // Test fails creating a file in a branch that already exists with force and branch protection enabled + createFileOptions = getCreateFileOptions() + createFileOptions.BranchName = repo1.DefaultBranch + createFileOptions.NewBranchName = "develop" + createFileOptions.Force = true + 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) + fileID++ + treePath = fmt.Sprintf("new/file%d.txt", fileID) + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s", user2.Name, repo1.Name, treePath), &createFileOptions). + AddTokenAuth(token2) + MakeRequest(t, req, http.StatusUnprocessableEntity) + // Test creating a file without a message createFileOptions = getCreateFileOptions() createFileOptions.Message = "" diff --git a/tests/integration/api_repo_file_delete_test.go b/tests/integration/api_repo_file_delete_test.go index 9dd47f93e6167..107b7c7f5bf4b 100644 --- a/tests/integration/api_repo_file_delete_test.go +++ b/tests/integration/api_repo_file_delete_test.go @@ -91,6 +91,54 @@ func TestAPIDeleteFile(t *testing.T) { assert.Nil(t, fileResponse.Content) assert.Equal(t, deleteFileOptions.Message+"\n", fileResponse.Commit.Message) + // Test fails deleting a file in a branch that already exists without force + fileID++ + treePath = fmt.Sprintf("delete/file%d.txt", fileID) + createFile(user2, repo1, treePath) + deleteFileOptions = getDeleteFileOptions() + deleteFileOptions.BranchName = repo1.DefaultBranch + deleteFileOptions.NewBranchName = "develop" + deleteFileOptions.Force = false + req = NewRequestWithJSON(t, "DELETE", fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s", user2.Name, repo1.Name, treePath), &deleteFileOptions). + AddTokenAuth(token2) + MakeRequest(t, req, http.StatusUnprocessableEntity) + + // Test succeeds deleting a file in a branch that already exists with force + fileID++ + treePath = fmt.Sprintf("delete/file%d.txt", fileID) + createFile(user2, repo1, treePath) + deleteFileOptions = getDeleteFileOptions() + deleteFileOptions.BranchName = repo1.DefaultBranch + deleteFileOptions.NewBranchName = "develop" + deleteFileOptions.Force = true + req = NewRequestWithJSON(t, "DELETE", fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s", user2.Name, repo1.Name, treePath), &deleteFileOptions). + AddTokenAuth(token2) + resp = MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &fileResponse) + assert.NotNil(t, fileResponse) + assert.Nil(t, fileResponse.Content) + assert.Equal(t, deleteFileOptions.Message+"\n", fileResponse.Commit.Message) + + // Test fails creating a file in a branch that already exists with force and branch protection enabled + fileID++ + treePath = fmt.Sprintf("delete/file%d.txt", fileID) + createFile(user2, repo1, treePath) + deleteFileOptions = getDeleteFileOptions() + deleteFileOptions.BranchName = repo1.DefaultBranch + deleteFileOptions.NewBranchName = "develop" + deleteFileOptions.Force = true + 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) + req = NewRequestWithJSON(t, "DELETE", fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s", user2.Name, repo1.Name, treePath), &deleteFileOptions). + AddTokenAuth(token2) + MakeRequest(t, req, http.StatusUnprocessableEntity) + // Test deleting file without a message fileID++ treePath = fmt.Sprintf("delete/file%d.txt", fileID) diff --git a/tests/integration/api_repo_file_update_test.go b/tests/integration/api_repo_file_update_test.go index 9a56711da6a1d..29e9170e66bdb 100644 --- a/tests/integration/api_repo_file_update_test.go +++ b/tests/integration/api_repo_file_update_test.go @@ -180,6 +180,57 @@ func TestAPIUpdateFile(t *testing.T) { assert.Equal(t, expectedDownloadURL, *fileResponse.Content.DownloadURL) assert.Equal(t, updateFileOptions.Message+"\n", fileResponse.Commit.Message) + // Test fails creating a file in a branch that already exists without force + updateFileOptions = getUpdateFileOptions() + updateFileOptions.BranchName = repo1.DefaultBranch + updateFileOptions.NewBranchName = "develop" + updateFileOptions.Force = false + fileID++ + treePath = fmt.Sprintf("update/file%d.txt", fileID) + createFile(user2, repo1, treePath) + req = NewRequestWithJSON(t, "PUT", fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s", user2.Name, repo1.Name, treePath), &updateFileOptions). + AddTokenAuth(token2) + MakeRequest(t, req, http.StatusUnprocessableEntity) + + // Test succeeds creating a file in a branch that already exists with force + updateFileOptions = getUpdateFileOptions() + updateFileOptions.BranchName = repo1.DefaultBranch + updateFileOptions.NewBranchName = "develop" + updateFileOptions.Force = true + fileID++ + treePath = fmt.Sprintf("update/file%d.txt", fileID) + createFile(user2, repo1, treePath) + req = NewRequestWithJSON(t, "PUT", fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s", user2.Name, repo1.Name, treePath), &updateFileOptions). + AddTokenAuth(token2) + resp = MakeRequest(t, req, http.StatusOK) + DecodeJSON(t, resp, &fileResponse) + + assert.Equal(t, expectedSHA, fileResponse.Content.SHA) + assert.Equal(t, expectedHTMLURL, *fileResponse.Content.HTMLURL) + assert.Equal(t, expectedDownloadURL, *fileResponse.Content.DownloadURL) + assert.Equal(t, updateFileOptions.Message+"\n", fileResponse.Commit.Message) + expectedHTMLURL = fmt.Sprintf(setting.AppURL+"user2/repo1/src/branch/develop/update/file%d.txt", fileID) + expectedDownloadURL = fmt.Sprintf(setting.AppURL+"user2/repo1/raw/branch/develop/update/file%d.txt", fileID) + // Test fails creating a file in a branch that already exists with force and branch protection enabled + updateFileOptions = getUpdateFileOptions() + updateFileOptions.BranchName = repo1.DefaultBranch + updateFileOptions.NewBranchName = "develop" + updateFileOptions.Force = true + 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) + fileID++ + treePath = fmt.Sprintf("update/file%d.txt", fileID) + createFile(user2, repo1, treePath) + req = NewRequestWithJSON(t, "PUT", fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s", user2.Name, repo1.Name, treePath), &updateFileOptions). + AddTokenAuth(token2) + MakeRequest(t, req, http.StatusUnprocessableEntity) + // Test updating a file and renaming it updateFileOptions = getUpdateFileOptions() updateFileOptions.BranchName = repo1.DefaultBranch diff --git a/tests/integration/api_repo_files_change_test.go b/tests/integration/api_repo_files_change_test.go index 999bcdc680fbc..9366eb3967f8f 100644 --- a/tests/integration/api_repo_files_change_test.go +++ b/tests/integration/api_repo_files_change_test.go @@ -164,6 +164,84 @@ func TestAPIChangeFiles(t *testing.T) { 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.Force = 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) + MakeRequest(t, req, http.StatusUnprocessableEntity) + + // Test succeeds creating a file in a branch that already exists with force + changeFilesOptions = getChangeFilesOptions() + changeFilesOptions.BranchName = repo1.DefaultBranch + changeFilesOptions.NewBranchName = "develop" + changeFilesOptions.Force = 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.Force = 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) + MakeRequest(t, req, http.StatusUnprocessableEntity) + // Test updating a file and renaming it changeFilesOptions = getChangeFilesOptions() changeFilesOptions.BranchName = repo1.DefaultBranch From 9910e2dc708849a3d8ffbe15be66dcb98c2ed106 Mon Sep 17 00:00:00 2001 From: Rob Gonnella Date: Mon, 6 Oct 2025 10:26:36 -0400 Subject: [PATCH 2/6] chore: fixes formatting errors reported by linting tools --- templates/swagger/v1_json.tmpl | 16 ++++++++-------- tests/integration/api_repo_file_update_test.go | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index d4f577c5cfada..f3afa23ac3694 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -22014,7 +22014,7 @@ "x-go-name": "Files" }, "force": { - "description": "force updates new_branch if branch already exists", + "description": "force (optional) will force update the new branch if it already exists", "type": "boolean", "x-go-name": "Force" }, @@ -22855,7 +22855,7 @@ "$ref": "#/definitions/CommitDateOptions" }, "force": { - "description": "force updates new_branch if branch already exists", + "description": "force (optional) will force update the new branch if it already exists", "type": "boolean", "x-go-name": "Force" }, @@ -23868,7 +23868,7 @@ "$ref": "#/definitions/CommitDateOptions" }, "force": { - "description": "force updates new_branch if branch already exists", + "description": "force (optional) will force update the new branch if it already exists", "type": "boolean", "x-go-name": "Force" }, @@ -28669,16 +28669,16 @@ "dates": { "$ref": "#/definitions/CommitDateOptions" }, + "force": { + "description": "force (optional) will force update the new branch if it already exists", + "type": "boolean", + "x-go-name": "Force" + }, "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" }, - "force": { - "description": "force updates new_branch if branch already exists", - "type": "boolean", - "x-go-name": "Force" - }, "message": { "description": "message (optional) for the commit of this file. if not supplied, a default message will be used", "type": "string", diff --git a/tests/integration/api_repo_file_update_test.go b/tests/integration/api_repo_file_update_test.go index 29e9170e66bdb..a22df5dccaaff 100644 --- a/tests/integration/api_repo_file_update_test.go +++ b/tests/integration/api_repo_file_update_test.go @@ -205,12 +205,12 @@ func TestAPIUpdateFile(t *testing.T) { resp = MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &fileResponse) + expectedHTMLURL = fmt.Sprintf(setting.AppURL+"user2/repo1/src/branch/develop/update/file%d.txt", fileID) + expectedDownloadURL = fmt.Sprintf(setting.AppURL+"user2/repo1/raw/branch/develop/update/file%d.txt", fileID) assert.Equal(t, expectedSHA, fileResponse.Content.SHA) assert.Equal(t, expectedHTMLURL, *fileResponse.Content.HTMLURL) assert.Equal(t, expectedDownloadURL, *fileResponse.Content.DownloadURL) assert.Equal(t, updateFileOptions.Message+"\n", fileResponse.Commit.Message) - expectedHTMLURL = fmt.Sprintf(setting.AppURL+"user2/repo1/src/branch/develop/update/file%d.txt", fileID) - expectedDownloadURL = fmt.Sprintf(setting.AppURL+"user2/repo1/raw/branch/develop/update/file%d.txt", fileID) // Test fails creating a file in a branch that already exists with force and branch protection enabled updateFileOptions = getUpdateFileOptions() updateFileOptions.BranchName = repo1.DefaultBranch From e08badabcc7a574c813a64e99df4c856e36383e3 Mon Sep 17 00:00:00 2001 From: Rob Gonnella Date: Mon, 6 Oct 2025 11:06:25 -0400 Subject: [PATCH 3/6] chore: fixes comment on IsErrBranchProtected func --- models/git/branch.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/git/branch.go b/models/git/branch.go index 34f516d97d5fe..854bf9146906b 100644 --- a/models/git/branch.go +++ b/models/git/branch.go @@ -66,7 +66,7 @@ type ErrBranchProtected struct { BranchName string } -// IsErrBranchAlreadyExists checks if an error is an ErrBranchAlreadyExists. +// IsErrBranchProtected checks if an error is an ErrBranchProtected. func IsErrBranchProtected(err error) bool { _, ok := err.(ErrBranchProtected) return ok From 437fa8f5a85bc2b31fabdc134fc94fe0c41f3b96 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 6 Oct 2025 23:45:28 +0800 Subject: [PATCH 4/6] fix --- models/git/branch.go | 19 ------- modules/git/error.go | 41 ++++++++------- modules/structs/repo_file.go | 10 ++-- routers/api/v1/repo/file.go | 9 +++- services/repository/files/temp_repo.go | 3 -- services/repository/files/update.go | 29 +++-------- templates/swagger/v1_json.tmpl | 48 ++++++++--------- .../integration/api_repo_file_create_test.go | 48 ----------------- .../integration/api_repo_file_delete_test.go | 48 ----------------- .../integration/api_repo_file_update_test.go | 51 ------------------- .../integration/api_repo_files_change_test.go | 13 ++--- 11 files changed, 72 insertions(+), 247 deletions(-) diff --git a/models/git/branch.go b/models/git/branch.go index 854bf9146906b..54351649cc5ec 100644 --- a/models/git/branch.go +++ b/models/git/branch.go @@ -61,25 +61,6 @@ func (err ErrBranchAlreadyExists) Unwrap() error { return util.ErrAlreadyExist } -// ErrBranchProtected represents an error that a branch cannot be force updated due to branch protections -type ErrBranchProtected struct { - BranchName string -} - -// IsErrBranchProtected checks if an error is an ErrBranchProtected. -func IsErrBranchProtected(err error) bool { - _, ok := err.(ErrBranchProtected) - return ok -} - -func (err ErrBranchProtected) Error() string { - return fmt.Sprintf("branch cannot be force updated due to branch protection rules [name: %s]", err.BranchName) -} - -func (err ErrBranchProtected) Unwrap() error { - return util.ErrPermissionDenied -} - // ErrBranchNameConflict represents an error that branch name conflicts with other branch. type ErrBranchNameConflict struct { BranchName string diff --git a/modules/git/error.go b/modules/git/error.go index 7d131345d0670..c8e7264c10ea1 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.Split(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 ad162df960ecc..5733823a58996 100644 --- a/modules/structs/repo_file.go +++ b/modules/structs/repo_file.go @@ -8,14 +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 commited to the base branch NewBranchName string `json:"new_branch" binding:"GitRefName;MaxSize(100)"` - // force (optional) will force update the new branch if it already exists - Force bool `json:"force"` + // force_push (optional) will do a force-push if the new branch already exists + ForcePush bool `json:"force_push_new_branch"` // `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 ddf0a315c27ed..2b6348c2fb1eb 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -355,7 +355,7 @@ func ReqChangeRepoFileOptionsAndCheck(ctx *context.APIContext) { Message: commonOpts.Message, OldBranch: commonOpts.BranchName, NewBranch: commonOpts.NewBranchName, - Force: commonOpts.Force, + ForcePush: commonOpts.ForcePush, Committer: &files_service.IdentityOptions{ GitUserName: commonOpts.Committer.Name, GitUserEmail: commonOpts.Committer.Email, @@ -592,11 +592,16 @@ 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 } - if git_model.IsErrBranchAlreadyExists(err) || git_model.IsErrBranchProtected(err) || files_service.IsErrFilenameInvalid(err) || pull_service.IsErrSHADoesNotMatch(err) || + if git_model.IsErrBranchAlreadyExists(err) || files_service.IsErrFilenameInvalid(err) || pull_service.IsErrSHADoesNotMatch(err) || files_service.IsErrFilePathInvalid(err) || files_service.IsErrRepoFileAlreadyExists(err) || files_service.IsErrCommitIDDoesNotMatch(err) || files_service.IsErrSHAOrCommitIDNotProvided(err) { ctx.APIError(http.StatusUnprocessableEntity, err) diff --git a/services/repository/files/temp_repo.go b/services/repository/files/temp_repo.go index 90f1c5970edfc..40e7901b850c8 100644 --- a/services/repository/files/temp_repo.go +++ b/services/repository/files/temp_repo.go @@ -366,9 +366,6 @@ func (t *TemporaryUploadRepository) Push(ctx context.Context, doer *user_model.U 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 c0bce25c4d51a..b07055d57aaa6 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -60,7 +60,7 @@ type ChangeRepoFilesOptions struct { Committer *IdentityOptions Dates *CommitDateOptions Signoff bool - Force bool + ForcePush bool } type RepoFileOptions struct { @@ -169,30 +169,22 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use } // A NewBranch can be specified for the file to be created/updated in a new branch. - // Check to to see if the branch already exist. If it does, ensure Force option is set - // and VerifyBranchProtection passes + // Check to make sure the branch does not already exist, otherwise we can't proceed. + // If we aren't branching to a new branch, make sure user can commit to the given branch if opts.NewBranch != opts.OldBranch { exist, err := git_model.IsBranchExist(ctx, repo.ID, opts.NewBranch) if err != nil { return nil, err } - if exist { - if opts.Force { - // ensure branch can be force pushed - if err := VerifyBranchProtection(ctx, repo, doer, opts.NewBranch, treePaths, opts.Force); err != nil { - return nil, git_model.ErrBranchProtected{ - BranchName: opts.NewBranch, - } - } - } else { + 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, opts.Force); err != nil { + } else if err := VerifyBranchProtection(ctx, repo, doer, opts.OldBranch, treePaths); err != nil { return nil, err } @@ -315,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, opts.Force); err != nil { - log.Error("%T %v", err, err) + if err := t.Push(ctx, doer, commitHash, opts.NewBranch, opts.ForcePush); err != nil { return nil, err } @@ -697,7 +688,7 @@ func writeRepoObjectForRename(ctx context.Context, t *TemporaryUploadRepository, } // VerifyBranchProtection verify the branch protection for modifying the given treePath on the given branch -func VerifyBranchProtection(ctx context.Context, repo *repo_model.Repository, doer *user_model.User, branchName string, treePaths []string, force bool) error { +func VerifyBranchProtection(ctx context.Context, repo *repo_model.Repository, doer *user_model.User, branchName string, treePaths []string) error { protectedBranch, err := git_model.GetFirstMatchProtectedBranchRule(ctx, repo.ID, branchName) if err != nil { return err @@ -707,7 +698,6 @@ func VerifyBranchProtection(ctx context.Context, repo *repo_model.Repository, do globUnprotected := protectedBranch.GetUnprotectedFilePatterns() globProtected := protectedBranch.GetProtectedFilePatterns() canUserPush := protectedBranch.CanUserPush(ctx, doer) - canUserForcePush := protectedBranch.CanUserForcePush(ctx, doer) for _, treePath := range treePaths { isUnprotectedFile := false if len(globUnprotected) != 0 { @@ -718,11 +708,6 @@ func VerifyBranchProtection(ctx context.Context, repo *repo_model.Repository, do UserName: doer.LowerName, } } - if force && !canUserForcePush && !isUnprotectedFile { - return ErrUserCannotCommit{ - UserName: doer.LowerName, - } - } if protectedBranch.IsProtectedFile(globProtected, treePath) { return pull_service.ErrFilePathProtected{ Path: treePath, diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index f3afa23ac3694..fa28de9bdee99 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,18 +22013,18 @@ }, "x-go-name": "Files" }, - "force": { - "description": "force (optional) will force update the new branch if it already exists", + "force_push_new_branch": { + "description": "force_push (optional) will do a force-push if the new branch already exists", "type": "boolean", - "x-go-name": "Force" + "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 commited to the base branch", "type": "string", "x-go-name": "NewBranchName" }, @@ -22839,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" }, @@ -22854,18 +22854,18 @@ "dates": { "$ref": "#/definitions/CommitDateOptions" }, - "force": { - "description": "force (optional) will force update the new branch if it already exists", + "force_push_new_branch": { + "description": "force_push (optional) will do a force-push if the new branch already exists", "type": "boolean", - "x-go-name": "Force" + "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 commited to the base branch", "type": "string", "x-go-name": "NewBranchName" }, @@ -23857,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" }, @@ -23867,18 +23867,18 @@ "dates": { "$ref": "#/definitions/CommitDateOptions" }, - "force": { - "description": "force (optional) will force update the new branch if it already exists", + "force_push_new_branch": { + "description": "force_push (optional) will do a force-push if the new branch already exists", "type": "boolean", - "x-go-name": "Force" + "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 commited to the base branch", "type": "string", "x-go-name": "NewBranchName" }, @@ -28654,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" }, @@ -28669,10 +28669,10 @@ "dates": { "$ref": "#/definitions/CommitDateOptions" }, - "force": { - "description": "force (optional) will force update the new branch if it already exists", + "force_push_new_branch": { + "description": "force_push (optional) will do a force-push if the new branch already exists", "type": "boolean", - "x-go-name": "Force" + "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", @@ -28680,12 +28680,12 @@ "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 commited to the base branch", "type": "string", "x-go-name": "NewBranchName" }, diff --git a/tests/integration/api_repo_file_create_test.go b/tests/integration/api_repo_file_create_test.go index 9d4a1a30bcf9f..af3bc546803a1 100644 --- a/tests/integration/api_repo_file_create_test.go +++ b/tests/integration/api_repo_file_create_test.go @@ -228,54 +228,6 @@ func TestAPICreateFile(t *testing.T) { assert.Equal(t, expectedDownloadURL, *fileResponse.Content.DownloadURL) assert.Equal(t, createFileOptions.Message+"\n", fileResponse.Commit.Message) - // Test fails creating a file in a branch that already exists without force - createFileOptions = getCreateFileOptions() - createFileOptions.BranchName = repo1.DefaultBranch - createFileOptions.NewBranchName = "develop" - createFileOptions.Force = false - fileID++ - treePath = fmt.Sprintf("new/file%d.txt", fileID) - req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s", user2.Name, repo1.Name, treePath), &createFileOptions). - AddTokenAuth(token2) - MakeRequest(t, req, http.StatusUnprocessableEntity) - - // Test succeeds creating a file in a branch that already exists with force - createFileOptions = getCreateFileOptions() - createFileOptions.BranchName = repo1.DefaultBranch - createFileOptions.NewBranchName = "develop" - createFileOptions.Force = true - fileID++ - treePath = fmt.Sprintf("new/file%d.txt", fileID) - req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s", user2.Name, repo1.Name, treePath), &createFileOptions). - AddTokenAuth(token2) - resp = MakeRequest(t, req, http.StatusCreated) - DecodeJSON(t, resp, &fileResponse) - expectedHTMLURL = fmt.Sprintf(setting.AppURL+"user2/repo1/src/branch/develop/new/file%d.txt", fileID) - expectedDownloadURL = fmt.Sprintf(setting.AppURL+"user2/repo1/raw/branch/develop/new/file%d.txt", fileID) - assert.Equal(t, expectedSHA, fileResponse.Content.SHA) - assert.Equal(t, expectedHTMLURL, *fileResponse.Content.HTMLURL) - assert.Equal(t, expectedDownloadURL, *fileResponse.Content.DownloadURL) - assert.Equal(t, createFileOptions.Message+"\n", fileResponse.Commit.Message) - - // Test fails creating a file in a branch that already exists with force and branch protection enabled - createFileOptions = getCreateFileOptions() - createFileOptions.BranchName = repo1.DefaultBranch - createFileOptions.NewBranchName = "develop" - createFileOptions.Force = true - 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) - fileID++ - treePath = fmt.Sprintf("new/file%d.txt", fileID) - req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s", user2.Name, repo1.Name, treePath), &createFileOptions). - AddTokenAuth(token2) - MakeRequest(t, req, http.StatusUnprocessableEntity) - // Test creating a file without a message createFileOptions = getCreateFileOptions() createFileOptions.Message = "" diff --git a/tests/integration/api_repo_file_delete_test.go b/tests/integration/api_repo_file_delete_test.go index 107b7c7f5bf4b..9dd47f93e6167 100644 --- a/tests/integration/api_repo_file_delete_test.go +++ b/tests/integration/api_repo_file_delete_test.go @@ -91,54 +91,6 @@ func TestAPIDeleteFile(t *testing.T) { assert.Nil(t, fileResponse.Content) assert.Equal(t, deleteFileOptions.Message+"\n", fileResponse.Commit.Message) - // Test fails deleting a file in a branch that already exists without force - fileID++ - treePath = fmt.Sprintf("delete/file%d.txt", fileID) - createFile(user2, repo1, treePath) - deleteFileOptions = getDeleteFileOptions() - deleteFileOptions.BranchName = repo1.DefaultBranch - deleteFileOptions.NewBranchName = "develop" - deleteFileOptions.Force = false - req = NewRequestWithJSON(t, "DELETE", fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s", user2.Name, repo1.Name, treePath), &deleteFileOptions). - AddTokenAuth(token2) - MakeRequest(t, req, http.StatusUnprocessableEntity) - - // Test succeeds deleting a file in a branch that already exists with force - fileID++ - treePath = fmt.Sprintf("delete/file%d.txt", fileID) - createFile(user2, repo1, treePath) - deleteFileOptions = getDeleteFileOptions() - deleteFileOptions.BranchName = repo1.DefaultBranch - deleteFileOptions.NewBranchName = "develop" - deleteFileOptions.Force = true - req = NewRequestWithJSON(t, "DELETE", fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s", user2.Name, repo1.Name, treePath), &deleteFileOptions). - AddTokenAuth(token2) - resp = MakeRequest(t, req, http.StatusOK) - DecodeJSON(t, resp, &fileResponse) - assert.NotNil(t, fileResponse) - assert.Nil(t, fileResponse.Content) - assert.Equal(t, deleteFileOptions.Message+"\n", fileResponse.Commit.Message) - - // Test fails creating a file in a branch that already exists with force and branch protection enabled - fileID++ - treePath = fmt.Sprintf("delete/file%d.txt", fileID) - createFile(user2, repo1, treePath) - deleteFileOptions = getDeleteFileOptions() - deleteFileOptions.BranchName = repo1.DefaultBranch - deleteFileOptions.NewBranchName = "develop" - deleteFileOptions.Force = true - 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) - req = NewRequestWithJSON(t, "DELETE", fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s", user2.Name, repo1.Name, treePath), &deleteFileOptions). - AddTokenAuth(token2) - MakeRequest(t, req, http.StatusUnprocessableEntity) - // Test deleting file without a message fileID++ treePath = fmt.Sprintf("delete/file%d.txt", fileID) diff --git a/tests/integration/api_repo_file_update_test.go b/tests/integration/api_repo_file_update_test.go index a22df5dccaaff..9a56711da6a1d 100644 --- a/tests/integration/api_repo_file_update_test.go +++ b/tests/integration/api_repo_file_update_test.go @@ -180,57 +180,6 @@ func TestAPIUpdateFile(t *testing.T) { assert.Equal(t, expectedDownloadURL, *fileResponse.Content.DownloadURL) assert.Equal(t, updateFileOptions.Message+"\n", fileResponse.Commit.Message) - // Test fails creating a file in a branch that already exists without force - updateFileOptions = getUpdateFileOptions() - updateFileOptions.BranchName = repo1.DefaultBranch - updateFileOptions.NewBranchName = "develop" - updateFileOptions.Force = false - fileID++ - treePath = fmt.Sprintf("update/file%d.txt", fileID) - createFile(user2, repo1, treePath) - req = NewRequestWithJSON(t, "PUT", fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s", user2.Name, repo1.Name, treePath), &updateFileOptions). - AddTokenAuth(token2) - MakeRequest(t, req, http.StatusUnprocessableEntity) - - // Test succeeds creating a file in a branch that already exists with force - updateFileOptions = getUpdateFileOptions() - updateFileOptions.BranchName = repo1.DefaultBranch - updateFileOptions.NewBranchName = "develop" - updateFileOptions.Force = true - fileID++ - treePath = fmt.Sprintf("update/file%d.txt", fileID) - createFile(user2, repo1, treePath) - req = NewRequestWithJSON(t, "PUT", fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s", user2.Name, repo1.Name, treePath), &updateFileOptions). - AddTokenAuth(token2) - resp = MakeRequest(t, req, http.StatusOK) - DecodeJSON(t, resp, &fileResponse) - - expectedHTMLURL = fmt.Sprintf(setting.AppURL+"user2/repo1/src/branch/develop/update/file%d.txt", fileID) - expectedDownloadURL = fmt.Sprintf(setting.AppURL+"user2/repo1/raw/branch/develop/update/file%d.txt", fileID) - assert.Equal(t, expectedSHA, fileResponse.Content.SHA) - assert.Equal(t, expectedHTMLURL, *fileResponse.Content.HTMLURL) - assert.Equal(t, expectedDownloadURL, *fileResponse.Content.DownloadURL) - assert.Equal(t, updateFileOptions.Message+"\n", fileResponse.Commit.Message) - // Test fails creating a file in a branch that already exists with force and branch protection enabled - updateFileOptions = getUpdateFileOptions() - updateFileOptions.BranchName = repo1.DefaultBranch - updateFileOptions.NewBranchName = "develop" - updateFileOptions.Force = true - 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) - fileID++ - treePath = fmt.Sprintf("update/file%d.txt", fileID) - createFile(user2, repo1, treePath) - req = NewRequestWithJSON(t, "PUT", fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s", user2.Name, repo1.Name, treePath), &updateFileOptions). - AddTokenAuth(token2) - MakeRequest(t, req, http.StatusUnprocessableEntity) - // Test updating a file and renaming it updateFileOptions = getUpdateFileOptions() updateFileOptions.BranchName = repo1.DefaultBranch diff --git a/tests/integration/api_repo_files_change_test.go b/tests/integration/api_repo_files_change_test.go index 9366eb3967f8f..47fe5066a7138 100644 --- a/tests/integration/api_repo_files_change_test.go +++ b/tests/integration/api_repo_files_change_test.go @@ -161,14 +161,13 @@ 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.Force = false + changeFilesOptions.ForcePush = false fileID++ createTreePath = fmt.Sprintf("new/file%d.txt", fileID) updateTreePath = fmt.Sprintf("update/file%d.txt", fileID) @@ -181,13 +180,14 @@ func TestAPIChangeFiles(t *testing.T) { url = fmt.Sprintf("/api/v1/repos/%s/%s/contents", user2.Name, repo1.Name) req = NewRequestWithJSON(t, "POST", url, &changeFilesOptions). AddTokenAuth(token2) - MakeRequest(t, req, http.StatusUnprocessableEntity) + 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.Force = true + changeFilesOptions.ForcePush = true fileID++ createTreePath = fmt.Sprintf("new/file%d.txt", fileID) updateTreePath = fmt.Sprintf("update/file%d.txt", fileID) @@ -227,7 +227,7 @@ func TestAPIChangeFiles(t *testing.T) { changeFilesOptions = getChangeFilesOptions() changeFilesOptions.BranchName = repo1.DefaultBranch changeFilesOptions.NewBranchName = "develop" - changeFilesOptions.Force = true + changeFilesOptions.ForcePush = true fileID++ createTreePath = fmt.Sprintf("new/file%d.txt", fileID) updateTreePath = fmt.Sprintf("update/file%d.txt", fileID) @@ -240,7 +240,8 @@ func TestAPIChangeFiles(t *testing.T) { url = fmt.Sprintf("/api/v1/repos/%s/%s/contents", user2.Name, repo1.Name) req = NewRequestWithJSON(t, "POST", url, &changeFilesOptions). AddTokenAuth(token2) - MakeRequest(t, req, http.StatusUnprocessableEntity) + 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() From 2499e323e73cbdedbcf51ec1966d239d390bcabb Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 7 Oct 2025 00:13:12 +0800 Subject: [PATCH 5/6] fix --- modules/structs/repo_file.go | 4 ++-- templates/swagger/v1_json.tmpl | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/modules/structs/repo_file.go b/modules/structs/repo_file.go index 5733823a58996..4729bde491539 100644 --- a/modules/structs/repo_file.go +++ b/modules/structs/repo_file.go @@ -12,10 +12,10 @@ type FileOptions struct { Message string `json:"message"` // 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 base branch for the changes. If not supplied, the changes will be commited to the base branch + // 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_new_branch"` + 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/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index fa28de9bdee99..0df8356fd9c38 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -22013,7 +22013,7 @@ }, "x-go-name": "Files" }, - "force_push_new_branch": { + "force_push": { "description": "force_push (optional) will do a force-push if the new branch already exists", "type": "boolean", "x-go-name": "ForcePush" @@ -22024,7 +22024,7 @@ "x-go-name": "Message" }, "new_branch": { - "description": "new_branch (optional) will make a new branch from base branch for the changes. If not supplied, the changes will be commited to the base branch", + "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" }, @@ -22854,7 +22854,7 @@ "dates": { "$ref": "#/definitions/CommitDateOptions" }, - "force_push_new_branch": { + "force_push": { "description": "force_push (optional) will do a force-push if the new branch already exists", "type": "boolean", "x-go-name": "ForcePush" @@ -22865,7 +22865,7 @@ "x-go-name": "Message" }, "new_branch": { - "description": "new_branch (optional) will make a new branch from base branch for the changes. If not supplied, the changes will be commited to the base branch", + "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" }, @@ -23867,7 +23867,7 @@ "dates": { "$ref": "#/definitions/CommitDateOptions" }, - "force_push_new_branch": { + "force_push": { "description": "force_push (optional) will do a force-push if the new branch already exists", "type": "boolean", "x-go-name": "ForcePush" @@ -23878,7 +23878,7 @@ "x-go-name": "Message" }, "new_branch": { - "description": "new_branch (optional) will make a new branch from base branch for the changes. If not supplied, the changes will be commited to the base branch", + "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" }, @@ -28669,7 +28669,7 @@ "dates": { "$ref": "#/definitions/CommitDateOptions" }, - "force_push_new_branch": { + "force_push": { "description": "force_push (optional) will do a force-push if the new branch already exists", "type": "boolean", "x-go-name": "ForcePush" @@ -28685,7 +28685,7 @@ "x-go-name": "Message" }, "new_branch": { - "description": "new_branch (optional) will make a new branch from base branch for the changes. If not supplied, the changes will be commited to the base branch", + "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" }, From ab29873129050886873cff885b6713fee6eb75ab Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 7 Oct 2025 00:22:59 +0800 Subject: [PATCH 6/6] fix lint --- modules/git/error.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/git/error.go b/modules/git/error.go index c8e7264c10ea1..d4b5412da9d7d 100644 --- a/modules/git/error.go +++ b/modules/git/error.go @@ -115,8 +115,8 @@ func (err *ErrPushRejected) GenerateMessage() { } messageBuilder := &strings.Builder{} - lines := strings.Split(err.StdErr, "\n") - for _, line := range lines { + lines := strings.SplitSeq(err.StdErr, "\n") + for line := range lines { line, ok := strings.CutPrefix(line, prefixRemote) if !ok { continue