Fixing issue #35530: Password Leak in Log Messages (#35584) (#35665)

Backport #35584 by @shashank-netapp

# Summary
The Gitea codebase was logging `Elasticsearch` and `Meilisearch`
connection strings directly to log files without sanitizing them. Since
connection strings often contain credentials in the format
`protocol://username:password@host:port`, this resulted in passwords
being exposed in plain text in log output.

Fix:
- wrapped all instances of setting.Indexer.RepoConnStr and
setting.Indexer.IssueConnStr with the `util.SanitizeCredentialURLs()`
function before logging them.

Fixes: #35530

Co-authored-by: shashank-netapp <108022276+shashank-netapp@users.noreply.github.com>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

models/issues/review_list.go

@@ -173,7 +173,7 @@ func GetReviewsByIssueID(ctx context.Context, issueID int64) (latestReviews, mig
 	reviewersMap := make(map[int64][]*Review)         // key is reviewer id
 	originalReviewersMap := make(map[int64][]*Review) // key is original author id
 	reviewTeamsMap := make(map[int64][]*Review)       // key is reviewer team id
-	countedReivewTypes := []ReviewType{ReviewTypeApprove, ReviewTypeReject, ReviewTypeRequest}
+	countedReivewTypes := []ReviewType{ReviewTypeApprove, ReviewTypeReject, ReviewTypeRequest, ReviewTypeComment}
 	for _, review := range reviews {
 		if review.ReviewerTeamID == 0 && slices.Contains(countedReivewTypes, review.Type) && !review.Dismissed {
 			if review.OriginalAuthorID != 0 {

models/issues/review_test.go

@@ -123,6 +123,7 @@ func TestGetReviewersByIssueID(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 
 	issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 3})
+	user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
 	user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
 	org3 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3})
 	user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4})
@@ -130,6 +131,12 @@ func TestGetReviewersByIssueID(t *testing.T) {
 
 	expectedReviews := []*issues_model.Review{}
 	expectedReviews = append(expectedReviews,
+		&issues_model.Review{
+			ID:          5,
+			Reviewer:    user1,
+			Type:        issues_model.ReviewTypeComment,
+			UpdatedUnix: 946684810,
+		},
 		&issues_model.Review{
 			ID:          7,
 			Reviewer:    org3,
@@ -168,8 +175,9 @@ func TestGetReviewersByIssueID(t *testing.T) {
 	for _, review := range allReviews {
 		assert.NoError(t, review.LoadReviewer(db.DefaultContext))
 	}
-	if assert.Len(t, allReviews, 5) {
+	if assert.Len(t, allReviews, 6) {
 		for i, review := range allReviews {
+			assert.Equal(t, expectedReviews[i].ID, review.ID)
 			assert.Equal(t, expectedReviews[i].Reviewer, review.Reviewer)
 			assert.Equal(t, expectedReviews[i].Type, review.Type)
 			assert.Equal(t, expectedReviews[i].UpdatedUnix, review.UpdatedUnix)

models/perm/access/repo_permission.go

@@ -348,10 +348,8 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use
 
 	for _, u := range repo.Units {
 		for _, team := range teams {
-			unitAccessMode := minAccessMode
-			if teamMode, exist := team.UnitAccessModeEx(ctx, u.Type); exist {
-				unitAccessMode = max(perm.unitsMode[u.Type], unitAccessMode, teamMode)
-			}
+			teamMode, _ := team.UnitAccessModeEx(ctx, u.Type)
+			unitAccessMode := max(perm.unitsMode[u.Type], minAccessMode, teamMode)
 			perm.unitsMode[u.Type] = unitAccessMode
 		}
 	}

models/perm/access/repo_permission_test.go

@@ -197,4 +197,37 @@ func TestGetUserRepoPermission(t *testing.T) {
 		assert.Equal(t, perm_model.AccessModeWrite, perm.unitsMode[unit.TypeCode])
 		assert.Equal(t, perm_model.AccessModeRead, perm.unitsMode[unit.TypeIssues])
 	})
+
+	repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) // org private repo, same org as repo 32
+	require.NoError(t, repo3.LoadOwner(ctx))
+	require.True(t, repo3.Owner.IsOrganization())
+	require.NoError(t, db.TruncateBeans(ctx, &organization.TeamUnit{}, &Access{})) // The user has access set of that repo, remove it, it is useless for our test
+	require.NoError(t, db.Insert(ctx, &organization.TeamRepo{OrgID: org.ID, TeamID: team.ID, RepoID: repo3.ID}))
+	t.Run("DoerWithNoopTeamOnPrivateRepo", func(t *testing.T) {
+		perm, err := GetUserRepoPermission(ctx, repo3, user)
+		require.NoError(t, err)
+		assert.Equal(t, perm_model.AccessModeNone, perm.AccessMode)
+		assert.Equal(t, perm_model.AccessModeNone, perm.unitsMode[unit.TypeCode])
+		assert.Equal(t, perm_model.AccessModeNone, perm.unitsMode[unit.TypeIssues])
+	})
+
+	require.NoError(t, db.Insert(ctx, &organization.TeamUnit{OrgID: org.ID, TeamID: team.ID, Type: unit.TypeCode, AccessMode: perm_model.AccessModeNone}))
+	require.NoError(t, db.Insert(ctx, &organization.TeamUnit{OrgID: org.ID, TeamID: team.ID, Type: unit.TypeIssues, AccessMode: perm_model.AccessModeRead}))
+	t.Run("DoerWithReadIssueTeamOnPrivateRepo", func(t *testing.T) {
+		perm, err := GetUserRepoPermission(ctx, repo3, user)
+		require.NoError(t, err)
+		assert.Equal(t, perm_model.AccessModeNone, perm.AccessMode)
+		assert.Equal(t, perm_model.AccessModeNone, perm.unitsMode[unit.TypeCode])
+		assert.Equal(t, perm_model.AccessModeRead, perm.unitsMode[unit.TypeIssues])
+	})
+
+	require.NoError(t, db.Insert(ctx, repo_model.Collaboration{RepoID: repo3.ID, UserID: user.ID, Mode: perm_model.AccessModeWrite}))
+	require.NoError(t, db.Insert(ctx, Access{RepoID: repo3.ID, UserID: user.ID, Mode: perm_model.AccessModeWrite}))
+	t.Run("DoerWithReadIssueTeamAndWriteCollaboratorOnPrivateRepo", func(t *testing.T) {
+		perm, err := GetUserRepoPermission(ctx, repo3, user)
+		require.NoError(t, err)
+		assert.Equal(t, perm_model.AccessModeWrite, perm.AccessMode)
+		assert.Equal(t, perm_model.AccessModeWrite, perm.unitsMode[unit.TypeCode])
+		assert.Equal(t, perm_model.AccessModeWrite, perm.unitsMode[unit.TypeIssues])
+	})
 }

modules/git/url/url_test.go

@@ -34,12 +34,12 @@ func TestParseGitURLs(t *testing.T) {
 			},
 		},
 		{
-			kase: "git@[fe80:14fc:cec5:c174:d88%2510]:go-gitea/gitea.git",
+			kase: "git@[fe80::14fc:cec5:c174:d88%2510]:go-gitea/gitea.git",
 			expected: &GitURL{
 				URL: &url.URL{
 					Scheme: "ssh",
 					User:   url.User("git"),
-					Host:   "[fe80:14fc:cec5:c174:d88%10]",
+					Host:   "[fe80::14fc:cec5:c174:d88%10]",
 					Path:   "go-gitea/gitea.git",
 				},
 				extraMark: 1,
@@ -137,11 +137,11 @@ func TestParseGitURLs(t *testing.T) {
 			},
 		},
 		{
-			kase: "https://[fe80:14fc:cec5:c174:d88%2510]:20/go-gitea/gitea.git",
+			kase: "https://[fe80::14fc:cec5:c174:d88%2510]:20/go-gitea/gitea.git",
 			expected: &GitURL{
 				URL: &url.URL{
 					Scheme: "https",
-					Host:   "[fe80:14fc:cec5:c174:d88%10]:20",
+					Host:   "[fe80::14fc:cec5:c174:d88%10]:20",
 					Path:   "/go-gitea/gitea.git",
 				},
 				extraMark: 0,

modules/hcaptcha/hcaptcha_test.go

@@ -4,7 +4,10 @@
 package hcaptcha
 
 import (
+	"errors"
+	"io"
 	"net/http"
+	"net/url"
 	"os"
 	"strings"
 	"testing"
@@ -21,6 +24,33 @@ func TestMain(m *testing.M) {
 	os.Exit(m.Run())
 }
 
+type mockTransport struct{}
+
+func (mockTransport) RoundTrip(req *http.Request) (*http.Response, error) {
+	if req.URL.String() != verifyURL {
+		return nil, errors.New("unsupported url")
+	}
+
+	body, err := io.ReadAll(req.Body)
+	if err != nil {
+		return nil, err
+	}
+
+	bodyValues, err := url.ParseQuery(string(body))
+	if err != nil {
+		return nil, err
+	}
+
+	var responseText string
+	if bodyValues.Get("response") == dummyToken {
+		responseText = `{"success":true,"credit":false,"hostname":"dummy-key-pass","challenge_ts":"2025-10-08T16:02:56.136Z"}`
+	} else {
+		responseText = `{"success":false,"error-codes":["invalid-input-response"]}`
+	}
+
+	return &http.Response{Request: req, Body: io.NopCloser(strings.NewReader(responseText))}, nil
+}
+
 func TestCaptcha(t *testing.T) {
 	tt := []struct {
 		Name   string
@@ -54,7 +84,8 @@ func TestCaptcha(t *testing.T) {
 	for _, tc := range tt {
 		t.Run(tc.Name, func(t *testing.T) {
 			client, err := New(tc.Secret, WithHTTP(&http.Client{
-				Timeout: time.Second * 5,
+				Timeout:   time.Second * 5,
+				Transport: mockTransport{},
 			}))
 			if err != nil {
 				// The only error that can be returned from creating a client

modules/indexer/code/indexer.go

@@ -22,6 +22,7 @@ import (
 	"code.gitea.io/gitea/modules/process"
 	"code.gitea.io/gitea/modules/queue"
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/util"
 )
 
 var (
@@ -166,12 +167,12 @@ func Init() {
 				log.Fatal("PID: %d Unable to initialize the bleve Repository Indexer at path: %s Error: %v", os.Getpid(), setting.Indexer.RepoPath, err)
 			}
 		case "elasticsearch":
-			log.Info("PID: %d Initializing Repository Indexer at: %s", os.Getpid(), setting.Indexer.RepoConnStr)
+			log.Info("PID: %d Initializing Repository Indexer at: %s", os.Getpid(), util.SanitizeCredentialURLs(setting.Indexer.RepoConnStr))
 			defer func() {
 				if err := recover(); err != nil {
 					log.Error("PANIC whilst initializing repository indexer: %v\nStacktrace: %s", err, log.Stack(2))
 					log.Error("The indexer files are likely corrupted and may need to be deleted")
-					log.Error("You can completely remove the \"%s\" index to make Gitea recreate the indexes", setting.Indexer.RepoConnStr)
+					log.Error("You can completely remove the \"%s\" index to make Gitea recreate the indexes", util.SanitizeCredentialURLs(setting.Indexer.RepoConnStr))
 				}
 			}()
 
@@ -181,7 +182,7 @@ func Init() {
 				cancel()
 				(*globalIndexer.Load()).Close()
 				close(waitChannel)
-				log.Fatal("PID: %d Unable to initialize the elasticsearch Repository Indexer connstr: %s Error: %v", os.Getpid(), setting.Indexer.RepoConnStr, err)
+				log.Fatal("PID: %d Unable to initialize the elasticsearch Repository Indexer connstr: %s Error: %v", os.Getpid(), util.SanitizeCredentialURLs(setting.Indexer.RepoConnStr), err)
 			}
 
 		default:

modules/indexer/issues/indexer.go

@@ -25,6 +25,7 @@ import (
 	"code.gitea.io/gitea/modules/process"
 	"code.gitea.io/gitea/modules/queue"
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/util"
 )
 
 // IndexerMetadata is used to send data to the queue, so it contains only the ids.
@@ -100,7 +101,7 @@ func InitIssueIndexer(syncReindex bool) {
 			issueIndexer = elasticsearch.NewIndexer(setting.Indexer.IssueConnStr, setting.Indexer.IssueIndexerName)
 			existed, err = issueIndexer.Init(ctx)
 			if err != nil {
-				log.Fatal("Unable to issueIndexer.Init with connection %s Error: %v", setting.Indexer.IssueConnStr, err)
+				log.Fatal("Unable to issueIndexer.Init with connection %s Error: %v", util.SanitizeCredentialURLs(setting.Indexer.IssueConnStr), err)
 			}
 		case "db":
 			issueIndexer = db.GetIndexer()
@@ -108,7 +109,7 @@ func InitIssueIndexer(syncReindex bool) {
 			issueIndexer = meilisearch.NewIndexer(setting.Indexer.IssueConnStr, setting.Indexer.IssueConnAuth, setting.Indexer.IssueIndexerName)
 			existed, err = issueIndexer.Init(ctx)
 			if err != nil {
-				log.Fatal("Unable to issueIndexer.Init with connection %s Error: %v", setting.Indexer.IssueConnStr, err)
+				log.Fatal("Unable to issueIndexer.Init with connection %s Error: %v", util.SanitizeCredentialURLs(setting.Indexer.IssueConnStr), err)
 			}
 		default:
 			log.Fatal("Unknown issue indexer type: %s", setting.Indexer.IssueType)

routers/web/auth/oauth2_provider.go

@@ -639,6 +639,7 @@ func handleAuthorizationCode(ctx *context.Context, form forms.AccessTokenForm, s
 			ErrorCode:        oauth2_provider.AccessTokenErrorCodeInvalidRequest,
 			ErrorDescription: "cannot proceed your request",
 		})
+		return
 	}
 	resp, tokenErr := oauth2_provider.NewAccessTokenResponse(ctx, authorizationCode.Grant, serverKey, clientKey)
 	if tokenErr != nil {