Prevent hang in git cat-file if repository is not a valid repository and other fixes

models/action.go

@@ -23,6 +23,7 @@ import (
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/timeutil"
+	"code.gitea.io/gitea/modules/util"
 
 	"xorm.io/builder"
 )
@@ -252,6 +253,21 @@ func (a *Action) GetBranch() string {
 	return strings.TrimPrefix(a.RefName, git.BranchPrefix)
 }
 
+// GetRefLink returns the action's ref link.
+func (a *Action) GetRefLink() string {
+	switch {
+	case strings.HasPrefix(a.RefName, git.BranchPrefix):
+		return a.GetRepoLink() + "/src/branch/" + util.PathEscapeSegments(strings.TrimPrefix(a.RefName, git.BranchPrefix))
+	case strings.HasPrefix(a.RefName, git.TagPrefix):
+		return a.GetRepoLink() + "/src/tag/" + util.PathEscapeSegments(strings.TrimPrefix(a.RefName, git.TagPrefix))
+	case len(a.RefName) == 40 && git.SHAPattern.MatchString(a.RefName):
+		return a.GetRepoLink() + "/src/commit/" + a.RefName
+	default:
+		// FIXME: we will just assume it's a branch - this was the old way - at some point we may want to enforce that there is always a ref here.
+		return a.GetRepoLink() + "/src/branch/" + util.PathEscapeSegments(strings.TrimPrefix(a.RefName, git.BranchPrefix))
+	}
+}
+
 // GetTag returns the action's repository tag.
 func (a *Action) GetTag() string {
 	return strings.TrimPrefix(a.RefName, git.TagPrefix)

models/org_team.go

@@ -114,6 +114,14 @@ func SearchTeam(opts *SearchTeamOptions) ([]*Team, int64, error) {
 
 // ColorFormat provides a basic color format for a Team
 func (t *Team) ColorFormat(s fmt.State) {
+	if t == nil {
+		log.ColorFprintf(s, "%d:%s (OrgID: %d) %-v",
+			log.NewColoredIDValue(0),
+			"<nil>",
+			log.NewColoredIDValue(0),
+			0)
+		return
+	}
 	log.ColorFprintf(s, "%d:%s (OrgID: %d) %-v",
 		log.NewColoredIDValue(t.ID),
 		t.Name,

models/repo/repo.go

@@ -173,6 +173,13 @@ func (repo *Repository) SanitizedOriginalURL() string {
 
 // ColorFormat returns a colored string to represent this repo
 func (repo *Repository) ColorFormat(s fmt.State) {
+	if repo == nil {
+		log.ColorFprintf(s, "%d:%s/%s",
+			log.NewColoredIDValue(0),
+			"<nil>",
+			"<nil>")
+		return
+	}
 	log.ColorFprintf(s, "%d:%s/%s",
 		log.NewColoredIDValue(repo.ID),
 		repo.OwnerName,

models/unittest/testdb.go

@@ -104,6 +104,26 @@ func MainTest(m *testing.M, pathToGiteaRoot string, fixtureFiles ...string) {
 		fatalTestError("util.CopyDir: %v\n", err)
 	}
 
+	ownerDirs, err := os.ReadDir(setting.RepoRootPath)
+	if err != nil {
+		fatalTestError("unable to read the new repo root: %v\n", err)
+	}
+	for _, ownerDir := range ownerDirs {
+		if !ownerDir.Type().IsDir() {
+			continue
+		}
+		repoDirs, err := os.ReadDir(filepath.Join(setting.RepoRootPath, ownerDir.Name()))
+		if err != nil {
+			fatalTestError("unable to read the new repo root: %v\n", err)
+		}
+		for _, repoDir := range repoDirs {
+			_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "objects", "pack"), 0755)
+			_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "objects", "info"), 0755)
+			_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "refs", "heads"), 0755)
+			_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "refs", "tag"), 0755)
+		}
+	}
+
 	exitStatus := m.Run()
 	if err = util.RemoveAll(repoRootPath); err != nil {
 		fatalTestError("util.RemoveAll: %v\n", err)
@@ -152,5 +172,22 @@ func PrepareTestEnv(t testing.TB) {
 	assert.NoError(t, util.RemoveAll(setting.RepoRootPath))
 	metaPath := filepath.Join(giteaRoot, "integrations", "gitea-repositories-meta")
 	assert.NoError(t, util.CopyDir(metaPath, setting.RepoRootPath))
+
+	ownerDirs, err := os.ReadDir(setting.RepoRootPath)
+	assert.NoError(t, err)
+	for _, ownerDir := range ownerDirs {
+		if !ownerDir.Type().IsDir() {
+			continue
+		}
+		repoDirs, err := os.ReadDir(filepath.Join(setting.RepoRootPath, ownerDir.Name()))
+		assert.NoError(t, err)
+		for _, repoDir := range repoDirs {
+			_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "objects", "pack"), 0755)
+			_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "objects", "info"), 0755)
+			_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "refs", "heads"), 0755)
+			_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "refs", "tag"), 0755)
+		}
+	}
+
 	base.SetupGiteaRoot() // Makes sure GITEA_ROOT is set
 }

models/user/user.go

@@ -160,6 +160,12 @@ type SearchOrganizationsOptions struct {
 
 // ColorFormat writes a colored string to identify this struct
 func (u *User) ColorFormat(s fmt.State) {
+	if u == nil {
+		log.ColorFprintf(s, "%d:%s",
+			log.NewColoredIDValue(0),
+			log.NewColoredValue("<nil>"))
+		return
+	}
 	log.ColorFprintf(s, "%d:%s",
 		log.NewColoredIDValue(u.ID),
 		log.NewColoredValue(u.Name))

modules/git/batch_reader.go

@@ -27,6 +27,20 @@ type WriteCloserError interface {
 	CloseWithError(err error) error
 }
 
+// EnsureValidGitRepository runs git rev-parse in the repository path - thus ensuring that the repository is a valid repository.
+// Run before opening git cat-file.
+// This is needed otherwise the git cat-file will hang for invalid repositories.
+func EnsureValidGitRepository(ctx context.Context, repoPath string) error {
+	stderr := strings.Builder{}
+	err := NewCommandContext(ctx, "rev-parse").
+		SetDescription(fmt.Sprintf("%s rev-parse [repo_path: %s]", GitExecutable, repoPath)).
+		RunInDirFullPipeline(repoPath, nil, &stderr, nil)
+	if err != nil {
+		return ConcatenateError(err, (&stderr).String())
+	}
+	return nil
+}
+
 // CatFileBatchCheck opens git cat-file --batch-check in the provided repo and returns a stdin pipe, a stdout reader and cancel function
 func CatFileBatchCheck(ctx context.Context, repoPath string) (WriteCloserError, *bufio.Reader, func()) {
 	batchStdinReader, batchStdinWriter := io.Pipe()

modules/git/repo_base_nogogit.go

@@ -50,6 +50,11 @@ func OpenRepositoryCtx(ctx context.Context, repoPath string) (*Repository, error
 		return nil, errors.New("no such file or directory")
 	}
 
+	// Now because of some insanity with git cat-file not immediately failing if not run in a valid git directory we need to run git rev-parse first!
+	if err := EnsureValidGitRepository(ctx, repoPath); err != nil {
+		return nil, err
+	}
+
 	repo := &Repository{
 		Path:     repoPath,
 		tagCache: newObjectCache(),

modules/git/repo_commit_nogogit.go

@@ -37,7 +37,10 @@ func (repo *Repository) ResolveReference(name string) (string, error) {
 func (repo *Repository) GetRefCommitID(name string) (string, error) {
 	wr, rd, cancel := repo.CatFileBatchCheck(repo.Ctx)
 	defer cancel()
-	_, _ = wr.Write([]byte(name + "\n"))
+	_, err := wr.Write([]byte(name + "\n"))
+	if err != nil {
+		return "", err
+	}
 	shaBs, _, _, err := ReadBatchLine(rd)
 	if IsErrNotExist(err) {
 		return "", ErrNotExist{name, ""}

modules/indexer/code/bleve.go

@@ -275,6 +275,12 @@ func (b *BleveIndexer) Index(repo *repo_model.Repository, sha string, changes *r
 	batch := gitea_bleve.NewFlushingBatch(b.indexer, maxBatchSize)
 	if len(changes.Updates) > 0 {
 
+		// Now because of some insanity with git cat-file not immediately failing if not run in a valid git directory we need to run git rev-parse first!
+		if err := git.EnsureValidGitRepository(git.DefaultContext, repo.RepoPath()); err != nil {
+			log.Error("Unable to open git repo: %s for %-v: %v", repo.RepoPath(), repo, err)
+			return err
+		}
+
 		batchWriter, batchReader, cancel := git.CatFileBatch(git.DefaultContext, repo.RepoPath())
 		defer cancel()
 

modules/indexer/code/elastic_search.go

@@ -247,6 +247,11 @@ func (b *ElasticSearchIndexer) addDelete(filename string, repo *repo_model.Repos
 func (b *ElasticSearchIndexer) Index(repo *repo_model.Repository, sha string, changes *repoChanges) error {
 	reqs := make([]elastic.BulkableRequest, 0)
 	if len(changes.Updates) > 0 {
+		// Now because of some insanity with git cat-file not immediately failing if not run in a valid git directory we need to run git rev-parse first!
+		if err := git.EnsureValidGitRepository(git.DefaultContext, repo.RepoPath()); err != nil {
+			log.Error("Unable to open git repo: %s for %-v: %v", repo.RepoPath(), repo, err)
+			return err
+		}
 
 		batchWriter, batchReader, cancel := git.CatFileBatch(git.DefaultContext, repo.RepoPath())
 		defer cancel()

options/locale/locale_en-US.ini

@@ -2526,6 +2526,9 @@ auths.oauth2_required_claim_name = Required Claim Name
 auths.oauth2_required_claim_name_helper = Set this name to restrict login from this source to users with a claim with this name
 auths.oauth2_required_claim_value = Required Claim Value
 auths.oauth2_required_claim_value_helper = Set this value to restrict login from this source to users with a claim with this name and value
+auths.oauth2_group_claim_name = Claim name providing group names for this source. (Optional)
+auths.oauth2_admin_group = Group Claim value for administrator users. (Optional - requires claim name above)
+auths.oauth2_restricted_group = Group Claim value for restricted users. (Optional - requires claim name above)
 auths.enable_auto_register = Enable Auto Registration
 auths.sspi_auto_create_users = Automatically create users
 auths.sspi_auto_create_users_helper = Allow SSPI auth method to automatically create new accounts for users that login for the first time

routers/web/repo/compare.go

@@ -685,6 +685,7 @@ func CompareDiff(ctx *context.Context) {
 		return
 	}
 
+	ctx.Data["PullRequestWorkInProgressPrefixes"] = setting.Repository.PullRequest.WorkInProgressPrefixes
 	ctx.Data["DirectComparison"] = ci.DirectComparison
 	ctx.Data["OtherCompareSeparator"] = ".."
 	ctx.Data["CompareSeparator"] = "..."
@@ -762,7 +763,6 @@ func CompareDiff(ctx *context.Context) {
 	ctx.Data["IsDiffCompare"] = true
 	ctx.Data["RequireTribute"] = true
 	ctx.Data["RequireEasyMDE"] = true
-	ctx.Data["PullRequestWorkInProgressPrefixes"] = setting.Repository.PullRequest.WorkInProgressPrefixes
 	setTemplateIfExists(ctx, pullRequestTemplateKey, nil, pullRequestTemplateCandidates)
 	ctx.Data["IsAttachmentEnabled"] = setting.Attachment.Enabled
 	upload.AddUploadContext(ctx, "comment")

routers/web/web.go

@@ -718,7 +718,7 @@ func RegisterRoutes(m *web.Route) {
 		}, context.RepoMustNotBeArchived(), reqRepoIssueReader)
 		// FIXME: should use different URLs but mostly same logic for comments of issue and pull request.
 		// So they can apply their own enable/disable logic on routers.
-		m.Group("/issues", func() {
+		m.Group("/{type:issues|pulls}", func() {
 			m.Group("/{index}", func() {
 				m.Post("/title", repo.UpdateIssueTitle)
 				m.Post("/content", repo.UpdateIssueContent)

services/pull/pull.go

@@ -746,7 +746,8 @@ func GetIssuesLastCommitStatus(issues models.IssueList) (map[int64]*models.Commi
 		if !ok {
 			gitRepo, err = git.OpenRepository(issue.Repo.RepoPath())
 			if err != nil {
-				return nil, err
+				log.Error("Cannot open git repository %-v for issue #%d[%d]. Error: %v", issue.Repo, issue.Index, issue.ID, err)
+				continue
 			}
 			gitRepos[issue.RepoID] = gitRepo
 		}

templates/user/dashboard/feeds.tmpl

@@ -17,71 +17,67 @@
 						{{else if eq .GetOpType 2}}
 							{{$.i18n.Tr "action.rename_repo" (.GetContent|Escape) (.GetRepoLink|Escape) (.ShortRepoPath|Escape) | Str2html}}
 						{{else if eq .GetOpType 5}}
-							{{ $branchLink := .GetBranch | PathEscapeSegments | Escape}}
 							{{if .Content}}
-								{{$.i18n.Tr "action.commit_repo" (.GetRepoLink|Escape) $branchLink (Escape .GetBranch) (.ShortRepoPath|Escape) | Str2html}}
+								{{$.i18n.Tr "action.commit_repo" (.GetRepoLink|Escape) (.GetRefLink|Escape) (Escape .GetBranch) (.ShortRepoPath|Escape) | Str2html}}
 							{{else}}
-								{{$.i18n.Tr "action.create_branch" (.GetRepoLink|Escape) $branchLink (Escape .GetBranch) (.ShortRepoPath|Escape) | Str2html}}
+								{{$.i18n.Tr "action.create_branch" (.GetRepoLink|Escape) (.GetRefLink|Escape) (Escape .GetBranch) (.ShortRepoPath|Escape) | Str2html}}
 							{{end}}
 						{{else if eq .GetOpType 6}}
 							{{ $index := index .GetIssueInfos 0}}
-							{{$.i18n.Tr "action.create_issue" (.GetRepoLink|Escape) $index (.ShortRepoPath|Escape) | Str2html}}
+							{{$.i18n.Tr "action.create_issue" ((printf "%s/issues/%s" .GetRepoLink $index) |Escape) $index (.ShortRepoPath|Escape) | Str2html}}
 						{{else if eq .GetOpType 7}}
 							{{ $index := index .GetIssueInfos 0}}
-							{{$.i18n.Tr "action.create_pull_request" (.GetRepoLink|Escape) $index (.ShortRepoPath|Escape) | Str2html}}
+							{{$.i18n.Tr "action.create_pull_request" ((printf "%s/pulls/%s" .GetRepoLink $index) |Escape) $index (.ShortRepoPath|Escape) | Str2html}}
 						{{else if eq .GetOpType 8}}
 							{{$.i18n.Tr "action.transfer_repo" .GetContent (.GetRepoLink|Escape) (.ShortRepoPath|Escape) | Str2html}}
 						{{else if eq .GetOpType 9}}
-							{{ $tagLink := .GetTag | PathEscapeSegments | Escape}}
-							{{$.i18n.Tr "action.push_tag" (.GetRepoLink|Escape) $tagLink (.ShortRepoPath|Escape) .GetTag | Str2html}}
+							{{$.i18n.Tr "action.push_tag" (.GetRepoLink|Escape) (.GetRefLink|Escape) (.GetTag|Escape) (.ShortRepoPath|Escape) | Str2html}}
 						{{else if eq .GetOpType 10}}
 							{{ $index := index .GetIssueInfos 0}}
-							{{$.i18n.Tr "action.comment_issue" (.GetRepoLink|Escape) $index (.ShortRepoPath|Escape) | Str2html}}
+							{{$.i18n.Tr "action.comment_issue" ((printf "%s/issues/%s" .GetRepoLink $index) |Escape) $index (.ShortRepoPath|Escape) | Str2html}}
 						{{else if eq .GetOpType 11}}
 							{{ $index := index .GetIssueInfos 0}}
-							{{$.i18n.Tr "action.merge_pull_request" (.GetRepoLink|Escape) $index (.ShortRepoPath|Escape) | Str2html}}
+							{{$.i18n.Tr "action.merge_pull_request" ((printf "%s/pulls/%s" .GetRepoLink $index) |Escape) $index (.ShortRepoPath|Escape) | Str2html}}
 						{{else if eq .GetOpType 12}}
 							{{ $index := index .GetIssueInfos 0}}
-							{{$.i18n.Tr "action.close_issue" (.GetRepoLink|Escape) $index (.ShortRepoPath|Escape) | Str2html}}
+							{{$.i18n.Tr "action.close_issue" ((printf "%s/issues/%s" .GetRepoLink $index) |Escape) $index (.ShortRepoPath|Escape) | Str2html}}
 						{{else if eq .GetOpType 13}}
 							{{ $index := index .GetIssueInfos 0}}
-							{{$.i18n.Tr "action.reopen_issue" (.GetRepoLink|Escape) $index (.ShortRepoPath|Escape) | Str2html}}
+							{{$.i18n.Tr "action.reopen_issue" ((printf "%s/issues/%s" .GetRepoLink $index) |Escape) $index (.ShortRepoPath|Escape) | Str2html}}
 						{{else if eq .GetOpType 14}}
 							{{ $index := index .GetIssueInfos 0}}
-							{{$.i18n.Tr "action.close_pull_request" (.GetRepoLink|Escape) $index (.ShortRepoPath|Escape) | Str2html}}
+							{{$.i18n.Tr "action.close_pull_request" ((printf "%s/pulls/%s" .GetRepoLink $index) |Escape) $index (.ShortRepoPath|Escape) | Str2html}}
 						{{else if eq .GetOpType 15}}
 							{{ $index := index .GetIssueInfos 0}}
-							{{$.i18n.Tr "action.reopen_pull_request" (.GetRepoLink|Escape) $index (.ShortRepoPath|Escape) | Str2html}}
+							{{$.i18n.Tr "action.reopen_pull_request" ((printf "%s/pulls/%s" .GetRepoLink $index) |Escape) $index (.ShortRepoPath|Escape) | Str2html}}
 						{{else if eq .GetOpType 16}}
 							{{ $index := index .GetIssueInfos 0}}
 							{{$.i18n.Tr "action.delete_tag" (.GetRepoLink|Escape) (.GetTag|Escape) (.ShortRepoPath|Escape) | Str2html}}
 						{{else if eq .GetOpType 17}}
 							{{ $index := index .GetIssueInfos 0}}
 							{{$.i18n.Tr "action.delete_branch" (.GetRepoLink|Escape) (.GetBranch|Escape) (.ShortRepoPath|Escape) | Str2html}}
 						{{else if eq .GetOpType 18}}
-							{{ $branchLink := .GetBranch | PathEscapeSegments}}
-							{{$.i18n.Tr "action.mirror_sync_push" (.GetRepoLink|Escape) $branchLink (.GetBranch|Escape) (.ShortRepoPath|Escape) | Str2html}}
+							{{$.i18n.Tr "action.mirror_sync_push" (.GetRepoLink|Escape) (.GetRefLink|Escape) (.GetBranch|Escape) (.ShortRepoPath|Escape) | Str2html}}
 						{{else if eq .GetOpType 19}}
-							{{$.i18n.Tr "action.mirror_sync_create" (.GetRepoLink|Escape) (.GetBranch|Escape) (.ShortRepoPath|Escape) | Str2html}}
+							{{$.i18n.Tr "action.mirror_sync_create" (.GetRepoLink|Escape) (.GetRefLink|Escape) (.GetBranch|Escape) (.ShortRepoPath|Escape) | Str2html}}
 						{{else if eq .GetOpType 20}}
 							{{$.i18n.Tr "action.mirror_sync_delete" (.GetRepoLink|Escape) (.GetBranch|Escape) (.ShortRepoPath|Escape) | Str2html}}
 						{{else if eq .GetOpType 21}}
 							{{ $index := index .GetIssueInfos 0}}
-							{{$.i18n.Tr "action.approve_pull_request" (.GetRepoLink|Escape) $index (.ShortRepoPath|Escape) | Str2html}}
+							{{$.i18n.Tr "action.approve_pull_request" ((printf "%s/pulls/%s" .GetRepoLink $index) |Escape) $index (.ShortRepoPath|Escape) | Str2html}}
 						{{else if eq .GetOpType 22}}
 							{{ $index := index .GetIssueInfos 0}}
-							{{$.i18n.Tr "action.reject_pull_request" (.GetRepoLink|Escape) $index (.ShortRepoPath|Escape) | Str2html}}
+							{{$.i18n.Tr "action.reject_pull_request" ((printf "%s/pulls/%s" .GetRepoLink $index) |Escape) $index (.ShortRepoPath|Escape) | Str2html}}
 						{{else if eq .GetOpType 23}}
 							{{ $index := index .GetIssueInfos 0}}
-							{{$.i18n.Tr "action.comment_pull" (.GetRepoLink|Escape) $index (.ShortRepoPath|Escape) | Str2html}}
+							{{$.i18n.Tr "action.comment_pull" ((printf "%s/pulls/%s" .GetRepoLink $index) |Escape) $index (.ShortRepoPath|Escape) | Str2html}}
 						{{else if eq .GetOpType 24}}
-							{{ $branchLink := .GetBranch | PathEscapeSegments | Escape}}
 							{{ $linkText := .Content | RenderEmoji }}
-							{{$.i18n.Tr "action.publish_release" (.GetRepoLink|Escape) $branchLink (.ShortRepoPath|Escape) $linkText | Str2html}}
+							{{$.i18n.Tr "action.publish_release" (.GetRepoLink|Escape) (.GetRefLink|Escape) (.ShortRepoPath|Escape) $linkText | Str2html}}
 						{{else if eq .GetOpType 25}}
 							{{ $index := index .GetIssueInfos 0}}
 							{{ $reviewer := index .GetIssueInfos 1}}
-							{{$.i18n.Tr "action.review_dismissed" (.GetRepoLink|Escape) $index (.ShortRepoPath|Escape) $reviewer | Str2html}}
+							{{$.i18n.Tr "action.review_dismissed" ((printf "%s/pulls/%s" .GetRepoLink $index) |Escape) $index (.ShortRepoPath|Escape) $reviewer | Str2html}}
 						{{end}}
 					</p>
 					{{if or (eq .GetOpType 5) (eq .GetOpType 18)}}