From cebdc90ed9b63d8461fadfdecd21b454c203e927 Mon Sep 17 00:00:00 2001 From: "roman.s" <55788842+majorissuerep@users.noreply.github.com> Date: Mon, 27 Jul 2026 08:31:58 +0300 Subject: [PATCH] fix(api): accept fully-qualified refs in contents API (#38650) `GET/POST` repository contents endpoints resolve the `ref` query parameter through `ResolveRefCommit`, which previously only accepted short branch/tag names or commit IDs. Clients that pass fully-qualified Git refs (GitHub-compatible), e.g. `ref=refs%2Fheads%2Fmain` or `ref=refs%2Ftags%2Fv1.0`, received 404 "object does not exist". This change accepts fully-qualified **`refs/heads/*`** and **`refs/tags/*`** only, then falls back to the existing short-name and SHA logic. Other `refs/*` prefixes (e.g. `refs/pull/`, `refs/for/`) are rejected. Fixes #38197 --------- Co-authored-by: roman s Co-authored-by: wxiaoguang --- routers/api/v1/utils/git.go | 52 ++++-- tests/integration/api_repo_files_get_test.go | 29 +++- .../integration/api_repo_get_contents_test.go | 155 +++++++++--------- 3 files changed, 140 insertions(+), 96 deletions(-) diff --git a/routers/api/v1/utils/git.go b/routers/api/v1/utils/git.go index 0ee6af5664..d63ab2274c 100644 --- a/routers/api/v1/utils/git.go +++ b/routers/api/v1/utils/git.go @@ -5,6 +5,7 @@ package utils import ( "errors" + "strings" git_model "gitea.dev/models/git" repo_model "gitea.dev/models/repo" @@ -20,23 +21,50 @@ type RefCommit struct { CommitID string } -// ResolveRefCommit resolve ref to a commit if exist +// ResolveRefCommit resolve ref to a commit if it exists. +// inputRef may be a short branch/tag name, a commit ID, or a fully-qualified +// git ref (e.g. refs/heads/main, refs/tags/v1.0) for GitHub client compatibility. func ResolveRefCommit(ctx reqctx.RequestContext, repo *repo_model.Repository, inputRef string, minCommitIDLen ...int) (_ *RefCommit, err error) { + refCommit := RefCommit{InputRef: inputRef} + + var testRefBranch, testRefTag git.RefName + if strings.HasPrefix(inputRef, "refs/") { + // Fully-qualified refs first so clients can pass unambiguous names. + testRef := git.RefName(inputRef) + if testRef.IsBranch() { + testRefBranch = testRef + } else if testRef.IsTag() { + testRefTag = testRef + } + } else { + testRefBranch, testRefTag = git.RefNameFromBranch(inputRef), git.RefNameFromTag(inputRef) + } + + if testRefBranch != "" { + if exist, _ := git_model.IsBranchExist(ctx, repo.ID, testRefBranch.ShortName()); exist { + refCommit.RefName = testRefBranch + } + } + if testRefTag != "" { + // TODO: use git model instead of git command in the future? Model seems to be faster. + if git.IsTagExist(ctx, repo, testRefTag.ShortName()) { + refCommit.RefName = testRefTag + } + } + if refCommit.RefName == "" { + if git.IsStringLikelyCommitID(git.ObjectFormatFromName(repo.ObjectFormatName), inputRef, minCommitIDLen...) { + refCommit.RefName = git.RefNameFromCommit(inputRef) + } + } + + if refCommit.RefName == "" { + return nil, git.ErrNotExist{ID: inputRef} + } + gitRepo, err := git.RepositoryFromRequestContextOrOpen(ctx, repo) if err != nil { return nil, err } - refCommit := RefCommit{InputRef: inputRef} - if exist, _ := git_model.IsBranchExist(ctx, repo.ID, inputRef); exist { - refCommit.RefName = git.RefNameFromBranch(inputRef) - } else if git.IsTagExist(ctx, repo, inputRef) { - refCommit.RefName = git.RefNameFromTag(inputRef) - } else if git.IsStringLikelyCommitID(git.ObjectFormatFromName(repo.ObjectFormatName), inputRef, minCommitIDLen...) { - refCommit.RefName = git.RefNameFromCommit(inputRef) - } - if refCommit.RefName == "" { - return nil, git.ErrNotExist{ID: inputRef} - } if refCommit.Commit, err = gitRepo.GetCommit(ctx, refCommit.RefName.String()); err != nil { return nil, err } diff --git a/tests/integration/api_repo_files_get_test.go b/tests/integration/api_repo_files_get_test.go index 8c197a9cc4..3d7f0f5623 100644 --- a/tests/integration/api_repo_files_get_test.go +++ b/tests/integration/api_repo_files_get_test.go @@ -63,33 +63,50 @@ func TestAPIGetRequestedFiles(t *testing.T) { req := NewRequest(t, "GET", "/api/v1/repos/user2/repo1/file-contents?body="+url.QueryEscape(string(reqBodyParam))) resp := MakeRequest(t, req, http.StatusOK) ret := DecodeJSON(t, resp, []*api.ContentsResponse{}) - expected := []*api.ContentsResponse{getExpectedContentsResponseForContents(repo1.DefaultBranch, "branch", lastCommit.ID.String())} + expected := []*api.ContentsResponse{getExpectedContentsResponseForContents("master", "refs/heads/master", lastCommit.ID.String())} assert.Equal(t, expected, ret) }) t.Run("User2NoRef", func(t *testing.T) { ret := requestFiles(t, "/api/v1/repos/user2/repo1/file-contents", []string{"README.md"}) - expected := []*api.ContentsResponse{getExpectedContentsResponseForContents(repo1.DefaultBranch, "branch", lastCommit.ID.String())} + expected := []*api.ContentsResponse{getExpectedContentsResponseForContents("master", "refs/heads/master", lastCommit.ID.String())} assert.Equal(t, expected, ret) }) t.Run("User2RefBranch", func(t *testing.T) { ret := requestFiles(t, "/api/v1/repos/user2/repo1/file-contents?ref=master", []string{"README.md"}) - expected := []*api.ContentsResponse{getExpectedContentsResponseForContents(repo1.DefaultBranch, "branch", lastCommit.ID.String())} + expected := []*api.ContentsResponse{getExpectedContentsResponseForContents("master", "refs/heads/master", lastCommit.ID.String())} assert.Equal(t, expected, ret) }) t.Run("User2RefTag", func(t *testing.T) { ret := requestFiles(t, "/api/v1/repos/user2/repo1/file-contents?ref=v1.1", []string{"README.md"}) - expected := []*api.ContentsResponse{getExpectedContentsResponseForContents("v1.1", "tag", lastCommit.ID.String())} + expected := []*api.ContentsResponse{getExpectedContentsResponseForContents("v1.1", "refs/tags/v1.1", lastCommit.ID.String())} assert.Equal(t, expected, ret) }) t.Run("User2RefCommit", func(t *testing.T) { - ret := requestFiles(t, "/api/v1/repos/user2/repo1/file-contents?ref=65f1bf27bc3bf70f64657658635e66094edbcb4d", []string{"README.md"}) - expected := []*api.ContentsResponse{getExpectedContentsResponseForContents("65f1bf27bc3bf70f64657658635e66094edbcb4d", "commit", lastCommit.ID.String())} + commitID := "65f1bf27bc3bf70f64657658635e66094edbcb4d" + ret := requestFiles(t, "/api/v1/repos/user2/repo1/file-contents?ref="+commitID, []string{"README.md"}) + expected := []*api.ContentsResponse{getExpectedContentsResponseForContents(commitID, git.RefNameFromCommit(commitID), lastCommit.ID.String())} assert.Equal(t, expected, ret) }) t.Run("User2RefNotExist", func(t *testing.T) { ret := requestFiles(t, "/api/v1/repos/user2/repo1/file-contents?ref=not-exist", []string{"README.md"}, http.StatusNotFound) assert.Empty(t, ret) }) + t.Run("User2RefFullBranch", func(t *testing.T) { + ret := requestFiles(t, "/api/v1/repos/user2/repo1/file-contents?ref=refs/heads/master", []string{"README.md"}) + expected := []*api.ContentsResponse{getExpectedContentsResponseForContents("refs/heads/master", "refs/heads/master", lastCommit.ID.String())} + assert.Equal(t, expected, ret) + }) + t.Run("User2RefFullTag", func(t *testing.T) { + ret := requestFiles(t, "/api/v1/repos/user2/repo1/file-contents?ref=refs/tags/v1.1", []string{"README.md"}) + expected := []*api.ContentsResponse{getExpectedContentsResponseForContents("refs/tags/v1.1", "refs/tags/v1.1", lastCommit.ID.String())} + assert.Equal(t, expected, ret) + }) + t.Run("User2RefInternalPullRejected", func(t *testing.T) { + // repo1 has refs/pull/2/head in fixtures; contents API must not resolve it + assert.True(t, git.IsReferenceExist(t.Context(), gitRepo, "refs/pull/2/head")) + ret := requestFiles(t, "/api/v1/repos/user2/repo1/file-contents?ref="+url.QueryEscape("refs/pull/2/head"), []string{"README.md"}, http.StatusNotFound) + assert.Empty(t, ret) + }) t.Run("PermissionCheck", func(t *testing.T) { filesOptions := &api.GetFilesOptions{Files: []string{"README.md"}} diff --git a/tests/integration/api_repo_get_contents_test.go b/tests/integration/api_repo_get_contents_test.go index 92d90d8eda..9a358262ae 100644 --- a/tests/integration/api_repo_get_contents_test.go +++ b/tests/integration/api_repo_get_contents_test.go @@ -24,10 +24,10 @@ import ( "github.com/stretchr/testify/require" ) -func getExpectedContentsResponseForContents(ref, refType, lastCommitSHA string) *api.ContentsResponse { +func getExpectedContentsResponseForContents(inputRef string, expectedRef git.RefName, lastCommitSHA string) *api.ContentsResponse { treePath := "README.md" - selfURL := setting.AppURL + "api/v1/repos/user2/repo1/contents/" + treePath + "?ref=" + ref - htmlURL := setting.AppURL + "user2/repo1/src/" + refType + "/" + ref + "/" + treePath + selfURL := setting.AppURL + "api/v1/repos/user2/repo1/contents/" + treePath + "?ref=" + url.QueryEscape(inputRef) + htmlURL := setting.AppURL + "user2/repo1/src/" + expectedRef.RefWebLinkPath() + "/" + treePath gitURL := setting.AppURL + "api/v1/repos/user2/repo1/git/blobs/4b4851ad51df6a7d9f25c979345979eaeb5b349f" return &api.ContentsResponse{ Name: treePath, @@ -43,7 +43,7 @@ func getExpectedContentsResponseForContents(ref, refType, lastCommitSHA string) URL: &selfURL, HTMLURL: &htmlURL, GitURL: &gitURL, - DownloadURL: new(setting.AppURL + "user2/repo1/raw/" + refType + "/" + ref + "/" + treePath), + DownloadURL: new(setting.AppURL + "user2/repo1/raw/" + expectedRef.RefWebLinkPath() + "/" + treePath), Links: &api.FileLinksResponse{ Self: &selfURL, GitURL: &gitURL, @@ -82,92 +82,91 @@ func testAPIGetContents(t *testing.T, _ *url.URL) { require.NoError(t, err) defer gitRepo.Close() - // Make a new branch in repo1 - newBranch := "test_branch" - err = repo_service.CreateNewBranch(t.Context(), user2, repo1, gitRepo, repo1.DefaultBranch, newBranch) + defaultBranchCommitID, err := gitRepo.GetBranchCommitID(t.Context(), repo1.DefaultBranch) require.NoError(t, err) - commitID, err := gitRepo.GetBranchCommitID(t.Context(), repo1.DefaultBranch) - require.NoError(t, err) - // Make a new tag in repo1 - newTag := "test_tag" - err = gitRepo.CreateTag(t.Context(), newTag, commitID) - require.NoError(t, err) - /*** END SETUP ***/ + t.Run("FileNotFound", func(t *testing.T) { + req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/no-such/file.md", user2.Name, repo1.Name) + resp := MakeRequest(t, req, http.StatusNotFound) + assert.Contains(t, resp.Body.String(), "object does not exist [id: , rel_path: no-such]") + }) - // not found - req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/no-such/file.md", user2.Name, repo1.Name) - resp := MakeRequest(t, req, http.StatusNotFound) - assert.Contains(t, resp.Body.String(), "object does not exist [id: , rel_path: no-such]") + t.Run("NoRef", func(t *testing.T) { + ref := git.RefNameFromBranch(repo1.DefaultBranch) + req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s", user2.Name, repo1.Name, treePath) + resp := MakeRequest(t, req, http.StatusOK) + contentsResponse := DecodeJSON(t, resp, &api.ContentsResponse{}) + expectedContentsResponse := getExpectedContentsResponseForContents("master", ref, defaultBranchCommitID) + assert.Equal(t, *expectedContentsResponse, *contentsResponse) + }) - // ref is default ref - ref := repo1.DefaultBranch - refType := "branch" - req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?ref=%s", user2.Name, repo1.Name, treePath, ref) - resp = MakeRequest(t, req, http.StatusOK) - contentsResponse := DecodeJSON(t, resp, &api.ContentsResponse{}) - lastCommit, _ := gitRepo.GetCommitByPath(t.Context(), "README.md") - expectedContentsResponse := getExpectedContentsResponseForContents(ref, refType, lastCommit.ID.String()) - assert.Equal(t, *expectedContentsResponse, *contentsResponse) + t.Run("NewlyCreatedBranchSynced", func(t *testing.T) { + newBranch := "test_branch" + err = repo_service.CreateNewBranch(t.Context(), user2, repo1, gitRepo, repo1.DefaultBranch, newBranch) + require.NoError(t, err) - // No ref - refType = "branch" - req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s", user2.Name, repo1.Name, treePath) - resp = MakeRequest(t, req, http.StatusOK) - contentsResponse = DecodeJSON(t, resp, &api.ContentsResponse{}) - expectedContentsResponse = getExpectedContentsResponseForContents(repo1.DefaultBranch, refType, lastCommit.ID.String()) - assert.Equal(t, *expectedContentsResponse, *contentsResponse) + ref := git.RefNameFromBranch(newBranch) + req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?ref=%s", user2.Name, repo1.Name, treePath, ref.ShortName()) + resp := MakeRequest(t, req, http.StatusOK) + contentsResponse := DecodeJSON(t, resp, &api.ContentsResponse{}) + branchCommit, _ := gitRepo.GetBranchCommit(t.Context(), ref.ShortName()) + lastCommit, _ := branchCommit.GetCommitByPath(t.Context(), gitRepo, "README.md") + expectedContentsResponse := getExpectedContentsResponseForContents(ref.ShortName(), ref, lastCommit.ID.String()) + assert.Equal(t, *expectedContentsResponse, *contentsResponse) + }) - // ref is the branch we created above in setup - ref = newBranch - refType = "branch" - req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?ref=%s", user2.Name, repo1.Name, treePath, ref) - resp = MakeRequest(t, req, http.StatusOK) - contentsResponse = DecodeJSON(t, resp, &api.ContentsResponse{}) - branchCommit, _ := gitRepo.GetBranchCommit(t.Context(), ref) - lastCommit, _ = branchCommit.GetCommitByPath(t.Context(), gitRepo, "README.md") - expectedContentsResponse = getExpectedContentsResponseForContents(ref, refType, lastCommit.ID.String()) - assert.Equal(t, *expectedContentsResponse, *contentsResponse) + t.Run("NewlyCreatedTagSynced", func(t *testing.T) { + newTag := "test_tag" + err = gitRepo.CreateTag(t.Context(), newTag, defaultBranchCommitID) + require.NoError(t, err) + ref := git.RefNameFromTag(newTag) + tagCommit, _ := gitRepo.GetTagCommit(t.Context(), ref.ShortName()) + lastCommit, _ := tagCommit.GetCommitByPath(t.Context(), gitRepo, "README.md") + expectedContentsResponse := getExpectedContentsResponseForContents(ref.ShortName(), ref, lastCommit.ID.String()) - // ref is the new tag we created above in setup - ref = newTag - refType = "tag" - req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?ref=%s", user2.Name, repo1.Name, treePath, ref) - resp = MakeRequest(t, req, http.StatusOK) - contentsResponse = DecodeJSON(t, resp, &api.ContentsResponse{}) - tagCommit, _ := gitRepo.GetTagCommit(t.Context(), ref) - lastCommit, _ = tagCommit.GetCommitByPath(t.Context(), gitRepo, "README.md") - expectedContentsResponse = getExpectedContentsResponseForContents(ref, refType, lastCommit.ID.String()) - assert.Equal(t, *expectedContentsResponse, *contentsResponse) + req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?ref=%s", user2.Name, repo1.Name, treePath, ref.ShortName()) + resp := MakeRequest(t, req, http.StatusOK) + contentsResponse := DecodeJSON(t, resp, &api.ContentsResponse{}) + assert.Equal(t, *expectedContentsResponse, *contentsResponse) + }) - // ref is a commit - ref = commitID - refType = "commit" - req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?ref=%s", user2.Name, repo1.Name, treePath, ref) - resp = MakeRequest(t, req, http.StatusOK) - contentsResponse = DecodeJSON(t, resp, &api.ContentsResponse{}) - expectedContentsResponse = getExpectedContentsResponseForContents(ref, refType, commitID) - assert.Equal(t, *expectedContentsResponse, *contentsResponse) + t.Run("CommitRef", func(t *testing.T) { + ref := git.RefNameFromCommit(defaultBranchCommitID) + req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?ref=%s", user2.Name, repo1.Name, treePath, ref.ShortName()) + resp := MakeRequest(t, req, http.StatusOK) + contentsResponse := DecodeJSON(t, resp, &api.ContentsResponse{}) + expectedContentsResponse := getExpectedContentsResponseForContents(ref.ShortName(), ref, defaultBranchCommitID) + assert.Equal(t, *expectedContentsResponse, *contentsResponse) + }) - // Test file contents a file with a bad ref - ref = "badref" - req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?ref=%s", user2.Name, repo1.Name, treePath, ref) - MakeRequest(t, req, http.StatusNotFound) + t.Run("NotExistingRef", func(t *testing.T) { + req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?ref=not-existing", user2.Name, repo1.Name, treePath) + MakeRequest(t, req, http.StatusNotFound) + }) - // Test accessing private ref with user token that does not have access - should fail - req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s", user2.Name, repo16.Name, treePath). - AddTokenAuth(token4) - MakeRequest(t, req, http.StatusNotFound) + t.Run("InternalRef", func(t *testing.T) { + // Internal pull ref exists in the test repo but must not be accepted via contents API + assert.True(t, git.IsReferenceExist(t.Context(), gitRepo, "refs/pull/2/head")) + req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s?ref=%s", user2.Name, repo1.Name, treePath, url.QueryEscape("refs/pull/2/head")) + MakeRequest(t, req, http.StatusNotFound) + }) - // Test access private ref of owner of token - req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/readme.md", user2.Name, repo16.Name). - AddTokenAuth(token2) - MakeRequest(t, req, http.StatusOK) + t.Run("Permission", func(t *testing.T) { + // Test accessing private ref with user token that does not have access - should fail + req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s", user2.Name, repo16.Name, treePath). + AddTokenAuth(token4) + MakeRequest(t, req, http.StatusNotFound) - // Test access of org org3 private repo file by owner user2 - req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s", org3.Name, repo3.Name, treePath). - AddTokenAuth(token2) - MakeRequest(t, req, http.StatusOK) + // Test access private ref of owner of token + req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/readme.md", user2.Name, repo16.Name). + AddTokenAuth(token2) + MakeRequest(t, req, http.StatusOK) + + // Test access of org org3 private repo file by owner user2 + req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/contents/%s", org3.Name, repo3.Name, treePath). + AddTokenAuth(token2) + MakeRequest(t, req, http.StatusOK) + }) } func testAPIGetContentsRefFormats(t *testing.T) {