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 <roman.sukach@dust-labs.com> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
2026-07-27 16:55:17 +02:00 · 2026-07-27 08:31:58 +03:00
parent 4da9b59414
commit cebdc90ed9
3 changed files with 140 additions and 96 deletions
--- 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
 	}
--- 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"}}
--- 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) {