chore: fix legacy git ref problems (#38827)

1. add correct "refs/heads" prefix to the branch name for commit graph
2. fix incorrect cache key in GetCommitGraphsCount
3. remove the "--" trim for the tag name, there is no security vulnerability, we never do so anywhere else

Signed-off-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

modules/base/tool.go

@@ -23,7 +23,7 @@ import (
 // EncodeSha256 string to sha256 hex value.
 func EncodeSha256(str string) string {
 	h := sha256.New()
-	_, _ = h.Write([]byte(str))
+	_, _ = h.Write(util.UnsafeStringToBytes(str))
 	return hex.EncodeToString(h.Sum(nil))
 }
 

routers/web/repo/commit.go

@@ -116,21 +116,26 @@ func Graph(ctx *context.Context) {
 	hidePRRefs := ctx.FormBool("hide-pr-refs")
 	ctx.Data["HidePRRefs"] = hidePRRefs
 	branches := ctx.FormStrings("branch")
-	realBranches := make([]string, len(branches))
+	ctx.Data["SelectedBranches"] = branches
-	copy(realBranches, branches)
-	for i, branch := range realBranches {
-		if strings.HasPrefix(branch, "--") {
-			realBranches[i] = git.BranchPrefix + branch
-		}
-	}
-	ctx.Data["SelectedBranches"] = realBranches
-	files := ctx.FormStrings("file")
 
-	graphCommitsCount, err := ctx.Repo.GetCommitGraphsCount(ctx, hidePRRefs, realBranches, files)
+	branchRefs := make([]string, 0, len(branches))
+	for _, branchName := range branches {
+		branchRef := branchName
+		if !strings.HasPrefix(branchRef, "refs/") {
+			branchRef = git.BranchPrefix + branchRef
+		}
+		branchRefs = append(branchRefs, branchRef)
+	}
+
+	files := ctx.FormStrings("file")
+	graphCommitsCount, err := ctx.Repo.GetCommitGraphsCount(ctx, hidePRRefs, branchRefs, files)
 	if err != nil {
-		log.Warn("GetCommitGraphsCount error for generate graph exclude prs: %t branches: %s in %-v, Will Ignore branches and try again. Underlying Error: %v", hidePRRefs, branches, ctx.Repo.Repository, err)
+		if len(branchRefs) > 0 {
-		realBranches = []string{}
+			// maybe: "fatal: bad revision '.....'" if a ref doesn't exist
-		graphCommitsCount, err = ctx.Repo.GetCommitGraphsCount(ctx, hidePRRefs, realBranches, files)
+			// maybe it's better to show a 404 page instead of the unclear retry, anyway, a retry isn't harmful, so keep the old behavior
+			branchRefs = nil
+			graphCommitsCount, err = ctx.Repo.GetCommitGraphsCount(ctx, hidePRRefs, branchRefs, files)
+		}
 		if err != nil {
 			ctx.ServerError("GetCommitGraphsCount", err)
 			return
@@ -138,8 +143,7 @@ func Graph(ctx *context.Context) {
 	}
 
 	page := ctx.FormInt("page")
-
+	graph, err := gitgraph.GetCommitGraph(ctx, ctx.Repo.GitRepo, page, 0, hidePRRefs, branchRefs, files)
-	graph, err := gitgraph.GetCommitGraph(ctx, ctx.Repo.GitRepo, page, 0, hidePRRefs, realBranches, files)
 	if err != nil {
 		ctx.ServerError("GetCommitGraph", err)
 		return

services/context/repo.go

@@ -22,6 +22,7 @@ import (
 	repo_model "gitea.dev/models/repo"
 	unit_model "gitea.dev/models/unit"
 	user_model "gitea.dev/models/user"
+	"gitea.dev/modules/base"
 	"gitea.dev/modules/cache"
 	"gitea.dev/modules/git"
 	"gitea.dev/modules/httplib"
@@ -252,16 +253,17 @@ func (r *Repository) CanCreateIssueDependencies(ctx context.Context, user *user_
 }
 
 // GetCommitGraphsCount returns cached commit count for current view
-func (r *Repository) GetCommitGraphsCount(ctx context.Context, hidePRRefs bool, branches, files []string) (int64, error) {
+func (r *Repository) GetCommitGraphsCount(ctx context.Context, hidePRRefs bool, refs, files []string) (int64, error) {
-	cacheKey := fmt.Sprintf("commits-count-%d-graph-%t-%s-%s", r.Repository.ID, hidePRRefs, branches, files)
+	refFileKey := strings.Join(refs, "\x00") + "\x00\x00" + strings.Join(files, "\x00")
-
+	refFileKey = base.EncodeSha256(refFileKey)
+	cacheKey := cache.SafeCacheKey(fmt.Sprintf("git-commits-graph-count:%d:%v", r.Repository.ID, hidePRRefs), refFileKey)
 	return cache.GetInt64(cacheKey, func() (int64, error) {
-		if len(branches) == 0 {
+		if len(refs) == 0 {
 			return git.AllCommitsCount(ctx, r.Repository, hidePRRefs, files...)
 		}
 		return git.CommitsCount(ctx, r.Repository,
 			git.CommitsCountOptions{
-				Revision: branches,
+				Revision: refs,
 				RelPath:  files,
 			})
 	})

services/release/release.go

@@ -90,8 +90,6 @@ func createTag(ctx context.Context, gitRepo *git.Repository, rel *repo_model.Rel
 				return false, fmt.Errorf("GetProtectedTags: %w", err)
 			}
 
-			// Trim '--' prefix to prevent command line argument vulnerability.
-			rel.TagName = strings.TrimPrefix(rel.TagName, "--")
 			isAllowed, err := git_model.IsUserAllowedToControlTag(ctx, protectedTags, rel.TagName, rel.PublisherID)
 			if err != nil {
 				return false, err

services/repository/gitgraph/graph.go

@@ -14,7 +14,7 @@ import (
 )
 
 // GetCommitGraph return a list of commit (GraphItems) from all branches
-func GetCommitGraph(ctx context.Context, gitRepo *git.Repository, page, maxAllowedColors int, hidePRRefs bool, branches, files []string) (*Graph, error) {
+func GetCommitGraph(ctx context.Context, gitRepo *git.Repository, page, maxAllowedColors int, hidePRRefs bool, refs, files []string) (*Graph, error) {
 	format := "DATA:%D|%H|%ad|%h|%s"
 
 	if page == 0 {
@@ -27,7 +27,7 @@ func GetCommitGraph(ctx context.Context, gitRepo *git.Repository, page, maxAllow
 		graphCmd.AddArguments("--exclude=" + git.PullPrefix + "*")
 	}
 
-	if len(branches) == 0 {
+	if len(refs) == 0 {
 		graphCmd.AddArguments("--tags", "--branches")
 	}
 
@@ -35,8 +35,8 @@ func GetCommitGraph(ctx context.Context, gitRepo *git.Repository, page, maxAllow
 		AddOptionFormat("-n %d", setting.UI.GraphMaxCommitNum*page).
 		AddOptionFormat("--pretty=format:%s", format)
 
-	if len(branches) > 0 {
+	if len(refs) > 0 {
-		graphCmd.AddDynamicArguments(branches...)
+		graphCmd.AddDynamicArguments(refs...)
 	}
 	if len(files) > 0 {
 		graphCmd.AddDashesAndList(files...)