From 67f9c29816f2c802f33623a2d09ba2b0441ca4a7 Mon Sep 17 00:00:00 2001 From: Nicolas Date: Sun, 26 Apr 2026 19:34:09 +0200 Subject: [PATCH 01/13] Enhance URL sanitization to handle schemeless credentials and improve logging output --- modules/git/gitcmd/command.go | 2 +- modules/git/gitcmd/command_test.go | 3 +++ modules/util/sanitize.go | 17 ++++++++++++----- modules/util/sanitize_test.go | 10 +++++++++- services/migrations/migrate.go | 2 +- 5 files changed, 26 insertions(+), 8 deletions(-) diff --git a/modules/git/gitcmd/command.go b/modules/git/gitcmd/command.go index e9b51802febf3..8c1a42e0bd431 100644 --- a/modules/git/gitcmd/command.go +++ b/modules/git/gitcmd/command.go @@ -57,7 +57,7 @@ type Command struct { } func logArgSanitize(arg string) string { - if strings.Contains(arg, "://") && strings.Contains(arg, "@") { + if strings.Contains(arg, "://") || strings.Contains(arg, "@") { return util.SanitizeCredentialURLs(arg) } else if filepath.IsAbs(arg) { base := filepath.Base(arg) diff --git a/modules/git/gitcmd/command_test.go b/modules/git/gitcmd/command_test.go index 19ec02b8088dd..9971d4dfa4075 100644 --- a/modules/git/gitcmd/command_test.go +++ b/modules/git/gitcmd/command_test.go @@ -110,6 +110,9 @@ func TestCommandString(t *testing.T) { cmd = NewCommand("url: https://a:b@c/", "/root/dir-a/dir-b") assert.Equal(t, cmd.prog+` "url: https://sanitized-credential@c/" .../dir-a/dir-b`, cmd.LogString()) + + cmd = NewCommand("url: a:b@c/", "/root/dir-a/dir-b") + assert.Equal(t, cmd.prog+` "url: sanitized-credential@c/" .../dir-a/dir-b`, cmd.LogString()) } func TestRunStdError(t *testing.T) { diff --git a/modules/util/sanitize.go b/modules/util/sanitize.go index 0dd8b342a2a4f..2ed52be352598 100644 --- a/modules/util/sanitize.go +++ b/modules/util/sanitize.go @@ -5,6 +5,7 @@ package util import ( "bytes" + "regexp" "unicode" ) @@ -27,14 +28,20 @@ func SanitizeErrorCredentialURLs(err error) error { const userPlaceholder = "sanitized-credential" -var schemeSep = []byte("://") +var ( + schemeSep = []byte("://") + schemelessCredentialURL = regexp.MustCompile(`(^|[^A-Za-z0-9._~%!$&'()*+,;=-])([A-Za-z0-9._~%!$&'()*+,;=%-]+:[A-Za-z0-9._~%!$&'()*+,;=:%-]+@)([A-Za-z0-9.-]+(:[0-9]+)?)`) +) -// SanitizeCredentialURLs remove all credentials in URLs (starting with "scheme://") for the input string: "https://user:pass@domain.com" => "https://sanitized-credential@domain.com" +// SanitizeCredentialURLs remove all credentials in URLs for the input string: "https://user:pass@domain.com" => "https://sanitized-credential@domain.com" func SanitizeCredentialURLs(s string) string { bs := UnsafeStringToBytes(s) schemeSepPos := bytes.Index(bs, schemeSep) - if schemeSepPos == -1 || bytes.IndexByte(bs[schemeSepPos:], '@') == -1 { - return s // fast return if there is no URL scheme or no userinfo + if bytes.IndexByte(bs, '@') == -1 { + return s // fast return if there is no userinfo + } + if schemeSepPos == -1 { + return schemelessCredentialURL.ReplaceAllString(s, "${1}"+userPlaceholder+"@${3}") } out := make([]byte, 0, len(bs)+len(userPlaceholder)) for schemeSepPos != -1 { @@ -68,5 +75,5 @@ func SanitizeCredentialURLs(s string) string { schemeSepPos = bytes.Index(bs, schemeSep) } out = append(out, bs...) - return UnsafeBytesToString(out) + return schemelessCredentialURL.ReplaceAllString(UnsafeBytesToString(out), "${1}"+userPlaceholder+"@${3}") } diff --git a/modules/util/sanitize_test.go b/modules/util/sanitize_test.go index 0bcfd45ca4100..0d71b00284c00 100644 --- a/modules/util/sanitize_test.go +++ b/modules/util/sanitize_test.go @@ -54,8 +54,16 @@ func TestSanitizeCredentialURLs(t *testing.T) { "://@", }, { - "//u:p@h", // do not process URLs without explicit scheme, they are not treated as "valid" URLs because there is no scheme context in string "//u:p@h", + "//" + userPlaceholder + "@h", + }, + { + "fatal: unable to look up username:token@github.com (port 9418)", + "fatal: unable to look up " + userPlaceholder + "@github.com (port 9418)", + }, + { + "git failed for user:token@github.com/go-gitea/test_repo.git", + "git failed for " + userPlaceholder + "@github.com/go-gitea/test_repo.git", }, { "s://u@h", // the minimal pattern to be sanitized diff --git a/services/migrations/migrate.go b/services/migrations/migrate.go index 99f8dba92f470..0cdd96496ddbc 100644 --- a/services/migrations/migrate.go +++ b/services/migrations/migrate.go @@ -218,7 +218,7 @@ func migrateRepository(ctx context.Context, doer *user_model.User, downloader ba // We don't actually need to check the OriginalURL as it isn't used anywhere } - log.Trace("migrating git data from %s", repo.CloneURL) + log.Trace("migrating git data from %s", util.SanitizeCredentialURLs(repo.CloneURL)) messenger("repo.migrate.migrating_git") if err = uploader.CreateRepo(ctx, repo, opts); err != nil { return err From 1d3bafb7871dd5c7814fc51c639af5b9e3e5c870 Mon Sep 17 00:00:00 2001 From: Nicolas Date: Sun, 26 Apr 2026 21:01:02 +0200 Subject: [PATCH 02/13] fixes --- modules/util/sanitize.go | 47 +++++----------------------------------- 1 file changed, 5 insertions(+), 42 deletions(-) diff --git a/modules/util/sanitize.go b/modules/util/sanitize.go index 2ed52be352598..7099b268d73ef 100644 --- a/modules/util/sanitize.go +++ b/modules/util/sanitize.go @@ -4,9 +4,8 @@ package util import ( - "bytes" "regexp" - "unicode" + "strings" ) type sanitizedError struct { @@ -29,51 +28,15 @@ func SanitizeErrorCredentialURLs(err error) error { const userPlaceholder = "sanitized-credential" var ( - schemeSep = []byte("://") + schemeCredentialURL = regexp.MustCompile(`([A-Za-z][A-Za-z0-9+.-]*://)([A-Za-z0-9._~%!$&'()*+,;=:%-]+@)([A-Za-z0-9.-]+(:[0-9]+)?|$)`) schemelessCredentialURL = regexp.MustCompile(`(^|[^A-Za-z0-9._~%!$&'()*+,;=-])([A-Za-z0-9._~%!$&'()*+,;=%-]+:[A-Za-z0-9._~%!$&'()*+,;=:%-]+@)([A-Za-z0-9.-]+(:[0-9]+)?)`) ) // SanitizeCredentialURLs remove all credentials in URLs for the input string: "https://user:pass@domain.com" => "https://sanitized-credential@domain.com" func SanitizeCredentialURLs(s string) string { - bs := UnsafeStringToBytes(s) - schemeSepPos := bytes.Index(bs, schemeSep) - if bytes.IndexByte(bs, '@') == -1 { + if !strings.Contains(s, "@") { return s // fast return if there is no userinfo } - if schemeSepPos == -1 { - return schemelessCredentialURL.ReplaceAllString(s, "${1}"+userPlaceholder+"@${3}") - } - out := make([]byte, 0, len(bs)+len(userPlaceholder)) - for schemeSepPos != -1 { - schemeSepPos += 3 // skip the "://" - sepAtPos := -1 // the possible '@' position: "https://foo@[^here]host" - sepEndPos := schemeSepPos // the possible end position: "The https://host[^here] in log for test" - sepLoop: - for ; sepEndPos < len(bs); sepEndPos++ { - c := bs[sepEndPos] - if ('A' <= c && c <= 'Z') || ('a' <= c && c <= 'z') || ('0' <= c && c <= '9') { - continue - } - switch c { - case '@': - sepAtPos = sepEndPos - case '-', '.', '_', '~', '!', '$', '&', '\'', '(', ')', '*', '+', ',', ';', '=', ':', '%': - continue // due to RFC 3986, userinfo can contain - . _ ~ ! $ & ' ( ) * + , ; = : and any percent-encoded chars - default: - break sepLoop // if it is an invalid char for URL (eg: space, '/', and others), stop the loop - } - } - // if there is '@', and the string is like "s://u@h", then hide the "u" part - if sepAtPos != -1 && (schemeSepPos >= 4 && unicode.IsLetter(rune(bs[schemeSepPos-4]))) && sepAtPos-schemeSepPos > 0 && sepEndPos-sepAtPos > 0 { - out = append(out, bs[:schemeSepPos]...) - out = append(out, userPlaceholder...) - out = append(out, bs[sepAtPos:sepEndPos]...) - } else { - out = append(out, bs[:sepEndPos]...) - } - bs = bs[sepEndPos:] - schemeSepPos = bytes.Index(bs, schemeSep) - } - out = append(out, bs...) - return schemelessCredentialURL.ReplaceAllString(UnsafeBytesToString(out), "${1}"+userPlaceholder+"@${3}") + s = schemeCredentialURL.ReplaceAllString(s, "${1}"+userPlaceholder+"@${3}") + return schemelessCredentialURL.ReplaceAllString(s, "${1}"+userPlaceholder+"@${3}") } From 7fecb1528bcbdb73ea34804e8200ed04053e065f Mon Sep 17 00:00:00 2001 From: silverwind Date: Sun, 26 Apr 2026 21:30:30 +0200 Subject: [PATCH 03/13] remove duplicate % in userinfo regex char classes Co-Authored-By: Claude (Opus 4.7) --- modules/util/sanitize.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/util/sanitize.go b/modules/util/sanitize.go index 7099b268d73ef..de92660b14e5f 100644 --- a/modules/util/sanitize.go +++ b/modules/util/sanitize.go @@ -28,8 +28,8 @@ func SanitizeErrorCredentialURLs(err error) error { const userPlaceholder = "sanitized-credential" var ( - schemeCredentialURL = regexp.MustCompile(`([A-Za-z][A-Za-z0-9+.-]*://)([A-Za-z0-9._~%!$&'()*+,;=:%-]+@)([A-Za-z0-9.-]+(:[0-9]+)?|$)`) - schemelessCredentialURL = regexp.MustCompile(`(^|[^A-Za-z0-9._~%!$&'()*+,;=-])([A-Za-z0-9._~%!$&'()*+,;=%-]+:[A-Za-z0-9._~%!$&'()*+,;=:%-]+@)([A-Za-z0-9.-]+(:[0-9]+)?)`) + schemeCredentialURL = regexp.MustCompile(`([A-Za-z][A-Za-z0-9+.-]*://)([A-Za-z0-9._~!$&'()*+,;=:%-]+@)([A-Za-z0-9.-]+(:[0-9]+)?|$)`) + schemelessCredentialURL = regexp.MustCompile(`(^|[^A-Za-z0-9._~%!$&'()*+,;=-])([A-Za-z0-9._~!$&'()*+,;=%-]+:[A-Za-z0-9._~!$&'()*+,;=:%-]+@)([A-Za-z0-9.-]+(:[0-9]+)?)`) ) // SanitizeCredentialURLs remove all credentials in URLs for the input string: "https://user:pass@domain.com" => "https://sanitized-credential@domain.com" From e0db6b680d7ce0353671584350e8558b425e4e32 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 27 Apr 2026 04:29:56 +0800 Subject: [PATCH 04/13] fix --- modules/git/gitcmd/command.go | 6 ++---- modules/util/sanitize.go | 23 +++++++++++++++-------- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/modules/git/gitcmd/command.go b/modules/git/gitcmd/command.go index 8c1a42e0bd431..ee447dfd032c7 100644 --- a/modules/git/gitcmd/command.go +++ b/modules/git/gitcmd/command.go @@ -57,14 +57,12 @@ type Command struct { } func logArgSanitize(arg string) string { - if strings.Contains(arg, "://") || strings.Contains(arg, "@") { - return util.SanitizeCredentialURLs(arg) - } else if filepath.IsAbs(arg) { + if filepath.IsAbs(arg) { base := filepath.Base(arg) dir := filepath.Dir(arg) return ".../" + filepath.Join(filepath.Base(dir), base) } - return arg + return util.SanitizeCredentialURLs(arg) } func (c *Command) LogString() string { diff --git a/modules/util/sanitize.go b/modules/util/sanitize.go index de92660b14e5f..1ad0a8e876b0f 100644 --- a/modules/util/sanitize.go +++ b/modules/util/sanitize.go @@ -6,6 +6,7 @@ package util import ( "regexp" "strings" + "sync" ) type sanitizedError struct { @@ -27,16 +28,22 @@ func SanitizeErrorCredentialURLs(err error) error { const userPlaceholder = "sanitized-credential" -var ( - schemeCredentialURL = regexp.MustCompile(`([A-Za-z][A-Za-z0-9+.-]*://)([A-Za-z0-9._~!$&'()*+,;=:%-]+@)([A-Za-z0-9.-]+(:[0-9]+)?|$)`) - schemelessCredentialURL = regexp.MustCompile(`(^|[^A-Za-z0-9._~%!$&'()*+,;=-])([A-Za-z0-9._~!$&'()*+,;=%-]+:[A-Za-z0-9._~!$&'()*+,;=:%-]+@)([A-Za-z0-9.-]+(:[0-9]+)?)`) -) +var globalVars = sync.OnceValue(func() (ret struct { + schemeCredentialURL *regexp.Regexp + schemelessCredentialURL *regexp.Regexp +}) { + ret.schemeCredentialURL = regexp.MustCompile(`([A-Za-z][A-Za-z0-9+.-]*://)([A-Za-z0-9._~!$&'()*+,;=:%-]+@)([A-Za-z0-9.-]+(:[0-9]+)?|$)`) + ret.schemelessCredentialURL = regexp.MustCompile(`(^|[^A-Za-z0-9._~%!$&'()*+,;=-])([A-Za-z0-9._~!$&'()*+,;=%-]+:[A-Za-z0-9._~!$&'()*+,;=:%-]+@)([A-Za-z0-9.-]+(:[0-9]+)?)`) + return ret +}) // SanitizeCredentialURLs remove all credentials in URLs for the input string: "https://user:pass@domain.com" => "https://sanitized-credential@domain.com" func SanitizeCredentialURLs(s string) string { - if !strings.Contains(s, "@") { - return s // fast return if there is no userinfo + if strings.Contains(s, ":") && strings.Contains(s, "@") { + return globalVars().schemelessCredentialURL.ReplaceAllString(s, "${1}"+userPlaceholder+"@${3}") + } + if strings.Contains(s, "://") && strings.Contains(s, "@") { + return globalVars().schemeCredentialURL.ReplaceAllString(s, "${1}"+userPlaceholder+"@${3}") } - s = schemeCredentialURL.ReplaceAllString(s, "${1}"+userPlaceholder+"@${3}") - return schemelessCredentialURL.ReplaceAllString(s, "${1}"+userPlaceholder+"@${3}") + return s } From 7dd16469bdf0ac3806f77a3900aaca37e298c277 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 27 Apr 2026 04:43:14 +0800 Subject: [PATCH 05/13] comment --- modules/util/sanitize.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/modules/util/sanitize.go b/modules/util/sanitize.go index 1ad0a8e876b0f..5dd5b03aad0e5 100644 --- a/modules/util/sanitize.go +++ b/modules/util/sanitize.go @@ -31,13 +31,17 @@ const userPlaceholder = "sanitized-credential" var globalVars = sync.OnceValue(func() (ret struct { schemeCredentialURL *regexp.Regexp schemelessCredentialURL *regexp.Regexp -}) { - ret.schemeCredentialURL = regexp.MustCompile(`([A-Za-z][A-Za-z0-9+.-]*://)([A-Za-z0-9._~!$&'()*+,;=:%-]+@)([A-Za-z0-9.-]+(:[0-9]+)?|$)`) - ret.schemelessCredentialURL = regexp.MustCompile(`(^|[^A-Za-z0-9._~%!$&'()*+,;=-])([A-Za-z0-9._~!$&'()*+,;=%-]+:[A-Za-z0-9._~!$&'()*+,;=:%-]+@)([A-Za-z0-9.-]+(:[0-9]+)?)`) +}, +) { + // RFC 3986: userinfo can contain - . _ ~ ! $ & ' ( ) * + , ; = : and any percent-encoded chars + ret.schemeCredentialURL = regexp.MustCompile(`([A-Za-z][A-Za-z0-9+.-]*://)([A-Za-z0-9-._~!$&'()*+,;=:%]+@)([A-Za-z0-9.-]+(:[0-9]+)?|$)`) + ret.schemelessCredentialURL = regexp.MustCompile(`(^|[^A-Za-z0-9._~%!$&'()*+,;=-])([A-Za-z0-9-._~!$&'()*+,;=%]+:[A-Za-z0-9-._~!$&'()*+,;=:%]+@)([A-Za-z0-9.-]+(:[0-9]+)?)`) return ret }) -// SanitizeCredentialURLs remove all credentials in URLs for the input string: "https://user:pass@domain.com" => "https://sanitized-credential@domain.com" +// SanitizeCredentialURLs remove all credentials in URLs for the input string: +// * "https://userinfo@domain.com" => "https://sanitized-credential@domain.com" +// * "user:pass@domain.com" => "sanitized-credential@domain.com" func SanitizeCredentialURLs(s string) string { if strings.Contains(s, ":") && strings.Contains(s, "@") { return globalVars().schemelessCredentialURL.ReplaceAllString(s, "${1}"+userPlaceholder+"@${3}") From 5f4e3cc83b734d84c9822ab565b2c4f2881baa11 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 27 Apr 2026 05:24:15 +0800 Subject: [PATCH 06/13] fix --- modules/util/sanitize.go | 90 ++++++++++++++++++++++++++++------- modules/util/sanitize_test.go | 2 +- 2 files changed, 73 insertions(+), 19 deletions(-) diff --git a/modules/util/sanitize.go b/modules/util/sanitize.go index 5dd5b03aad0e5..eac0d60f12728 100644 --- a/modules/util/sanitize.go +++ b/modules/util/sanitize.go @@ -4,9 +4,8 @@ package util import ( - "regexp" + "bytes" "strings" - "sync" ) type sanitizedError struct { @@ -26,28 +25,83 @@ func SanitizeErrorCredentialURLs(err error) error { return sanitizedError{err: err} } -const userPlaceholder = "sanitized-credential" +var schemeSep = []byte("://") -var globalVars = sync.OnceValue(func() (ret struct { - schemeCredentialURL *regexp.Regexp - schemelessCredentialURL *regexp.Regexp -}, -) { - // RFC 3986: userinfo can contain - . _ ~ ! $ & ' ( ) * + , ; = : and any percent-encoded chars - ret.schemeCredentialURL = regexp.MustCompile(`([A-Za-z][A-Za-z0-9+.-]*://)([A-Za-z0-9-._~!$&'()*+,;=:%]+@)([A-Za-z0-9.-]+(:[0-9]+)?|$)`) - ret.schemelessCredentialURL = regexp.MustCompile(`(^|[^A-Za-z0-9._~%!$&'()*+,;=-])([A-Za-z0-9-._~!$&'()*+,;=%]+:[A-Za-z0-9-._~!$&'()*+,;=:%]+@)([A-Za-z0-9.-]+(:[0-9]+)?)`) - return ret -}) +const userPlaceholder = "(masked)" // SanitizeCredentialURLs remove all credentials in URLs for the input string: // * "https://userinfo@domain.com" => "https://sanitized-credential@domain.com" // * "user:pass@domain.com" => "sanitized-credential@domain.com" func SanitizeCredentialURLs(s string) string { - if strings.Contains(s, ":") && strings.Contains(s, "@") { - return globalVars().schemelessCredentialURL.ReplaceAllString(s, "${1}"+userPlaceholder+"@${3}") + sepAtPos := strings.Index(s, "@") + sepColPos := strings.Index(s, ":") + if sepColPos == -1 || sepColPos > sepAtPos { + return s // fast path, unlikely contain any URL } - if strings.Contains(s, "://") && strings.Contains(s, "@") { - return globalVars().schemeCredentialURL.ReplaceAllString(s, "${1}"+userPlaceholder+"@${3}") + + res := make([]byte, 0, len(s)+len(userPlaceholder)) // a best guess to avoid too many re-allocations + bs := UnsafeStringToBytes(s) + for { + leftPos := sepAtPos - 1 + leftLoop: + for leftPos >= 0 { + c := bs[leftPos] + switch c { + case '-', '.', '_', '~', '!', '$', '&', '\'', '(', ')', '*', '+', ',', ';', '=', ':', '%': + // RFC 3986, userinfo can contain - . _ ~ ! $ & ' ( ) * + , ; = : and any percent-encoded chars + default: + valid := 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z' || '0' <= c && c <= '9' + if !valid { + break leftLoop + } + } + leftPos-- + } + leftPos++ + + rightPos := sepAtPos + 1 + rightLoop: + for rightPos < len(bs) { + c := bs[rightPos] + switch c { + case '.', '-': + // valid host char + case '[': + // ipv6 begin + if rightPos != sepAtPos+1 { + break rightLoop + } + case ']': + // ipv6 end + rightPos++ + break rightLoop + default: + valid := 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z' || '0' <= c && c <= '9' + if !valid { + break rightLoop + } + } + rightPos++ + } + + leading, leftPart, rightPart := bs[:leftPos], bs[leftPos:sepAtPos], bs[sepAtPos+1:rightPos] + needSanitize := bytes.IndexByte(leftPart, ':') >= 0 || bytes.HasSuffix(leading, schemeSep) + needSanitize = needSanitize && len(leftPart) > 0 && len(rightPart) > 0 + // TODO: can also do more checks for right part, e.g.: ipv6 + if needSanitize { + res = append(res, leading...) + res = append(res, userPlaceholder...) + res = append(res, '@') + res = append(res, rightPart...) + } else { + res = append(res, bs[:rightPos]...) + } + bs = bs[rightPos:] + sepAtPos = bytes.IndexByte(bs, '@') + if sepAtPos == -1 { + break + } } - return s + res = append(res, bs...) + return UnsafeBytesToString(res) } diff --git a/modules/util/sanitize_test.go b/modules/util/sanitize_test.go index 0d71b00284c00..fe399d3d1cc48 100644 --- a/modules/util/sanitize_test.go +++ b/modules/util/sanitize_test.go @@ -35,7 +35,7 @@ func TestSanitizeCredentialURLs(t *testing.T) { }, { "ftp://x@", - "ftp://" + userPlaceholder + "@", + "ftp://x@", }, { "ftp://x/@", From 39db1e41e419af1ece6d970066d2c5f447433c3b Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 27 Apr 2026 05:26:46 +0800 Subject: [PATCH 07/13] fine tune --- modules/git/gitcmd/command_test.go | 4 ++-- modules/util/sanitize.go | 10 +++++----- modules/util/sanitize_test.go | 20 ++++++++++---------- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/modules/git/gitcmd/command_test.go b/modules/git/gitcmd/command_test.go index 9971d4dfa4075..1a9bfe7d75276 100644 --- a/modules/git/gitcmd/command_test.go +++ b/modules/git/gitcmd/command_test.go @@ -109,10 +109,10 @@ func TestCommandString(t *testing.T) { assert.Equal(t, cmd.prog+` a "-m msg" "it's a test" "say \"hello\""`, cmd.LogString()) cmd = NewCommand("url: https://a:b@c/", "/root/dir-a/dir-b") - assert.Equal(t, cmd.prog+` "url: https://sanitized-credential@c/" .../dir-a/dir-b`, cmd.LogString()) + assert.Equal(t, cmd.prog+` "url: https://(masked)@c/" .../dir-a/dir-b`, cmd.LogString()) cmd = NewCommand("url: a:b@c/", "/root/dir-a/dir-b") - assert.Equal(t, cmd.prog+` "url: sanitized-credential@c/" .../dir-a/dir-b`, cmd.LogString()) + assert.Equal(t, cmd.prog+` "url: (masked)@c/" .../dir-a/dir-b`, cmd.LogString()) } func TestRunStdError(t *testing.T) { diff --git a/modules/util/sanitize.go b/modules/util/sanitize.go index eac0d60f12728..7da9d97a46841 100644 --- a/modules/util/sanitize.go +++ b/modules/util/sanitize.go @@ -27,11 +27,11 @@ func SanitizeErrorCredentialURLs(err error) error { var schemeSep = []byte("://") -const userPlaceholder = "(masked)" +const userInfoPlaceholder = "(masked)" // SanitizeCredentialURLs remove all credentials in URLs for the input string: -// * "https://userinfo@domain.com" => "https://sanitized-credential@domain.com" -// * "user:pass@domain.com" => "sanitized-credential@domain.com" +// * "https://userinfo@domain.com" => "https://***@domain.com" +// * "user:pass@domain.com" => "***@domain.com" func SanitizeCredentialURLs(s string) string { sepAtPos := strings.Index(s, "@") sepColPos := strings.Index(s, ":") @@ -39,7 +39,7 @@ func SanitizeCredentialURLs(s string) string { return s // fast path, unlikely contain any URL } - res := make([]byte, 0, len(s)+len(userPlaceholder)) // a best guess to avoid too many re-allocations + res := make([]byte, 0, len(s)+len(userInfoPlaceholder)) // a best guess to avoid too many re-allocations bs := UnsafeStringToBytes(s) for { leftPos := sepAtPos - 1 @@ -90,7 +90,7 @@ func SanitizeCredentialURLs(s string) string { // TODO: can also do more checks for right part, e.g.: ipv6 if needSanitize { res = append(res, leading...) - res = append(res, userPlaceholder...) + res = append(res, userInfoPlaceholder...) res = append(res, '@') res = append(res, rightPart...) } else { diff --git a/modules/util/sanitize_test.go b/modules/util/sanitize_test.go index fe399d3d1cc48..60ff37594da3b 100644 --- a/modules/util/sanitize_test.go +++ b/modules/util/sanitize_test.go @@ -13,7 +13,7 @@ import ( func TestSanitizeErrorCredentialURLs(t *testing.T) { err := errors.New("error with https://a@b.com") se := SanitizeErrorCredentialURLs(err) - assert.Equal(t, "error with https://"+userPlaceholder+"@b.com", se.Error()) + assert.Equal(t, "error with https://"+userInfoPlaceholder+"@b.com", se.Error()) } func TestSanitizeCredentialURLs(t *testing.T) { @@ -27,11 +27,11 @@ func TestSanitizeCredentialURLs(t *testing.T) { }, { "https://mytoken@github.com/go-gitea/test_repo.git", - "https://" + userPlaceholder + "@github.com/go-gitea/test_repo.git", + "https://" + userInfoPlaceholder + "@github.com/go-gitea/test_repo.git", }, { "https://user:password@github.com/go-gitea/test_repo.git", - "https://" + userPlaceholder + "@github.com/go-gitea/test_repo.git", + "https://" + userInfoPlaceholder + "@github.com/go-gitea/test_repo.git", }, { "ftp://x@", @@ -43,11 +43,11 @@ func TestSanitizeCredentialURLs(t *testing.T) { }, { "ftp://u@x/@", // test multiple @ chars - "ftp://" + userPlaceholder + "@x/@", + "ftp://" + userInfoPlaceholder + "@x/@", }, { "😊ftp://u@x😊", // test unicode - "😊ftp://" + userPlaceholder + "@x😊", + "😊ftp://" + userInfoPlaceholder + "@x😊", }, { "://@", @@ -55,23 +55,23 @@ func TestSanitizeCredentialURLs(t *testing.T) { }, { "//u:p@h", - "//" + userPlaceholder + "@h", + "//" + userInfoPlaceholder + "@h", }, { "fatal: unable to look up username:token@github.com (port 9418)", - "fatal: unable to look up " + userPlaceholder + "@github.com (port 9418)", + "fatal: unable to look up " + userInfoPlaceholder + "@github.com (port 9418)", }, { "git failed for user:token@github.com/go-gitea/test_repo.git", - "git failed for " + userPlaceholder + "@github.com/go-gitea/test_repo.git", + "git failed for " + userInfoPlaceholder + "@github.com/go-gitea/test_repo.git", }, { "s://u@h", // the minimal pattern to be sanitized - "s://" + userPlaceholder + "@h", + "s://" + userInfoPlaceholder + "@h", }, { "URLs in log https://u:b@h and https://u:b@h:80/, with https://h.com and u@h.com", - "URLs in log https://" + userPlaceholder + "@h and https://" + userPlaceholder + "@h:80/, with https://h.com and u@h.com", + "URLs in log https://" + userInfoPlaceholder + "@h and https://" + userInfoPlaceholder + "@h:80/, with https://h.com and u@h.com", }, } From fdc2e0230be44ad0da555d3284882fc5a7c4d637 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 27 Apr 2026 05:30:04 +0800 Subject: [PATCH 08/13] fine tune --- modules/util/sanitize.go | 4 ++++ modules/util/sanitize_test.go | 20 ++++++++++++-------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/modules/util/sanitize.go b/modules/util/sanitize.go index 7da9d97a46841..b6eb7a6893dbb 100644 --- a/modules/util/sanitize.go +++ b/modules/util/sanitize.go @@ -85,6 +85,10 @@ func SanitizeCredentialURLs(s string) string { } leading, leftPart, rightPart := bs[:leftPos], bs[leftPos:sepAtPos], bs[sepAtPos+1:rightPos] + + // Either: + // * git log message: "user:pass@host" (it contains a colon in userinfo) + // * http like URL: "https://userinfo@host.com" (it has "://" before the userinfo) needSanitize := bytes.IndexByte(leftPart, ':') >= 0 || bytes.HasSuffix(leading, schemeSep) needSanitize = needSanitize && len(leftPart) > 0 && len(rightPart) > 0 // TODO: can also do more checks for right part, e.g.: ipv6 diff --git a/modules/util/sanitize_test.go b/modules/util/sanitize_test.go index 60ff37594da3b..452dab49387ea 100644 --- a/modules/util/sanitize_test.go +++ b/modules/util/sanitize_test.go @@ -33,6 +33,10 @@ func TestSanitizeCredentialURLs(t *testing.T) { "https://user:password@github.com/go-gitea/test_repo.git", "https://" + userInfoPlaceholder + "@github.com/go-gitea/test_repo.git", }, + { + "https://user:password@[::]/go-gitea/test_repo.git", + "https://" + userInfoPlaceholder + "@[::]/go-gitea/test_repo.git", + }, { "ftp://x@", "ftp://x@", @@ -57,14 +61,6 @@ func TestSanitizeCredentialURLs(t *testing.T) { "//u:p@h", "//" + userInfoPlaceholder + "@h", }, - { - "fatal: unable to look up username:token@github.com (port 9418)", - "fatal: unable to look up " + userInfoPlaceholder + "@github.com (port 9418)", - }, - { - "git failed for user:token@github.com/go-gitea/test_repo.git", - "git failed for " + userInfoPlaceholder + "@github.com/go-gitea/test_repo.git", - }, { "s://u@h", // the minimal pattern to be sanitized "s://" + userInfoPlaceholder + "@h", @@ -73,6 +69,14 @@ func TestSanitizeCredentialURLs(t *testing.T) { "URLs in log https://u:b@h and https://u:b@h:80/, with https://h.com and u@h.com", "URLs in log https://" + userInfoPlaceholder + "@h and https://" + userInfoPlaceholder + "@h:80/, with https://h.com and u@h.com", }, + { + "fatal: unable to look up username:token@github.com (port 9418)", + "fatal: unable to look up " + userInfoPlaceholder + "@github.com (port 9418)", + }, + { + "git failed for user:token@github.com/go-gitea/test_repo.git", + "git failed for " + userInfoPlaceholder + "@github.com/go-gitea/test_repo.git", + }, } for n, c := range cases { From 011dc70c25d56db524374f7957bbf56fe3f0dddb Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 27 Apr 2026 05:35:39 +0800 Subject: [PATCH 09/13] fix comment --- modules/util/sanitize_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/util/sanitize_test.go b/modules/util/sanitize_test.go index 452dab49387ea..7bb19438c206b 100644 --- a/modules/util/sanitize_test.go +++ b/modules/util/sanitize_test.go @@ -62,7 +62,7 @@ func TestSanitizeCredentialURLs(t *testing.T) { "//" + userInfoPlaceholder + "@h", }, { - "s://u@h", // the minimal pattern to be sanitized + "s://u@h", "s://" + userInfoPlaceholder + "@h", }, { From 69003ad002e455a94df55f13e2b06a0d1f3371ff Mon Sep 17 00:00:00 2001 From: silverwind Date: Tue, 28 Apr 2026 00:31:00 +0200 Subject: [PATCH 10/13] sanitize: fix IPv6 host parsing and fast-path SSH-URL leak - Consume "[...]" IP-literals as a single host token. The previous switch arms for `[`/`]` were dead: at `sepAtPos+1` the `[` fell through, and the next `:` broke the host loop. Output looked correct only because the unparsed tail was appended verbatim. - Replace the fast path's `sepColPos > sepAtPos` check (which used only the first `@` and `:`) with an existence test on both. Lines containing a `git@host:path` SSH URL followed by a credential URL were skipped. - Add tests for `[2001:db8::1]:8080`, multi-URL with `[::1]`, unmatched `[`, and SSH-URL-then-HTTPS. Co-Authored-By: Claude (Opus 4.7) --- modules/util/sanitize.go | 32 +++++++++++++++----------------- modules/util/sanitize_test.go | 17 +++++++++++++++++ 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/modules/util/sanitize.go b/modules/util/sanitize.go index b6eb7a6893dbb..05e65695dfa19 100644 --- a/modules/util/sanitize.go +++ b/modules/util/sanitize.go @@ -30,15 +30,15 @@ var schemeSep = []byte("://") const userInfoPlaceholder = "(masked)" // SanitizeCredentialURLs remove all credentials in URLs for the input string: -// * "https://userinfo@domain.com" => "https://***@domain.com" -// * "user:pass@domain.com" => "***@domain.com" +// * "https://userinfo@domain.com" => "https://(masked)@domain.com" +// * "user:pass@domain.com" => "(masked)@domain.com" func SanitizeCredentialURLs(s string) string { - sepAtPos := strings.Index(s, "@") - sepColPos := strings.Index(s, ":") - if sepColPos == -1 || sepColPos > sepAtPos { - return s // fast path, unlikely contain any URL + // Compare existence, not position: "git@host:path" SSH URLs put '@' before ':', + // so a position check would skip later credential URLs in the same log line. + sepAtPos := strings.IndexByte(s, '@') + if sepAtPos == -1 || strings.IndexByte(s, ':') == -1 { + return s // fast return if there is no URL scheme or no userinfo } - res := make([]byte, 0, len(s)+len(userInfoPlaceholder)) // a best guess to avoid too many re-allocations bs := UnsafeStringToBytes(s) for { @@ -60,21 +60,20 @@ func SanitizeCredentialURLs(s string) string { leftPos++ rightPos := sepAtPos + 1 + // Consume an IP-literal "[...]" (RFC 3986, e.g. IPv6) as one unit so inner ':' don't terminate the host walk. + if rightPos < len(bs) && bs[rightPos] == '[' { + if end := bytes.IndexByte(bs[rightPos:], ']'); end > 0 { + rightPos += end + 1 + } else { + rightPos++ // unmatched '['; let the host loop consume the rest + } + } rightLoop: for rightPos < len(bs) { c := bs[rightPos] switch c { case '.', '-': // valid host char - case '[': - // ipv6 begin - if rightPos != sepAtPos+1 { - break rightLoop - } - case ']': - // ipv6 end - rightPos++ - break rightLoop default: valid := 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z' || '0' <= c && c <= '9' if !valid { @@ -91,7 +90,6 @@ func SanitizeCredentialURLs(s string) string { // * http like URL: "https://userinfo@host.com" (it has "://" before the userinfo) needSanitize := bytes.IndexByte(leftPart, ':') >= 0 || bytes.HasSuffix(leading, schemeSep) needSanitize = needSanitize && len(leftPart) > 0 && len(rightPart) > 0 - // TODO: can also do more checks for right part, e.g.: ipv6 if needSanitize { res = append(res, leading...) res = append(res, userInfoPlaceholder...) diff --git a/modules/util/sanitize_test.go b/modules/util/sanitize_test.go index 7bb19438c206b..526e898b98066 100644 --- a/modules/util/sanitize_test.go +++ b/modules/util/sanitize_test.go @@ -37,6 +37,18 @@ func TestSanitizeCredentialURLs(t *testing.T) { "https://user:password@[::]/go-gitea/test_repo.git", "https://" + userInfoPlaceholder + "@[::]/go-gitea/test_repo.git", }, + { + "https://user:password@[2001:db8::1]:8080/go-gitea/test_repo.git", + "https://" + userInfoPlaceholder + "@[2001:db8::1]:8080/go-gitea/test_repo.git", + }, + { + "see https://u:p@[::1]/x and https://u2:p2@h2", + "see https://" + userInfoPlaceholder + "@[::1]/x and https://" + userInfoPlaceholder + "@h2", + }, + { + "https://user:secret@[abc", // unmatched '[' must still mask credentials + "https://" + userInfoPlaceholder + "@[abc", + }, { "ftp://x@", "ftp://x@", @@ -77,6 +89,11 @@ func TestSanitizeCredentialURLs(t *testing.T) { "git failed for user:token@github.com/go-gitea/test_repo.git", "git failed for " + userInfoPlaceholder + "@github.com/go-gitea/test_repo.git", }, + { + // SSH-form git URL ("git@host:path") must not let a later credential URL through + "failed remote git@github.com:foo, retried via https://user:tok@github.com/foo", + "failed remote git@github.com:foo, retried via https://" + userInfoPlaceholder + "@github.com/foo", + }, } for n, c := range cases { From 41c1f9d94b13bb1b93996888b0eeb757e9fe4f42 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 28 Apr 2026 11:45:02 +0800 Subject: [PATCH 11/13] revert AI slop --- modules/util/sanitize.go | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/modules/util/sanitize.go b/modules/util/sanitize.go index 05e65695dfa19..a7df0edf67907 100644 --- a/modules/util/sanitize.go +++ b/modules/util/sanitize.go @@ -30,15 +30,16 @@ var schemeSep = []byte("://") const userInfoPlaceholder = "(masked)" // SanitizeCredentialURLs remove all credentials in URLs for the input string: -// * "https://userinfo@domain.com" => "https://(masked)@domain.com" -// * "user:pass@domain.com" => "(masked)@domain.com" +// * "https://userinfo@domain.com" => "https://***@domain.com" +// * "user:pass@domain.com" => "***@domain.com" +// "***" is a magic string internally used, doesn't guarantee to be anything. func SanitizeCredentialURLs(s string) string { - // Compare existence, not position: "git@host:path" SSH URLs put '@' before ':', - // so a position check would skip later credential URLs in the same log line. - sepAtPos := strings.IndexByte(s, '@') - if sepAtPos == -1 || strings.IndexByte(s, ':') == -1 { - return s // fast return if there is no URL scheme or no userinfo + sepColPos := strings.Index(s, ":") + sepAtPos := sepColPos + 1 + strings.Index(s[sepColPos+1:], "@") + for sepAtPos == -1 { + return s // fast path, unlikely contain any URL } + res := make([]byte, 0, len(s)+len(userInfoPlaceholder)) // a best guess to avoid too many re-allocations bs := UnsafeStringToBytes(s) for { @@ -60,20 +61,21 @@ func SanitizeCredentialURLs(s string) string { leftPos++ rightPos := sepAtPos + 1 - // Consume an IP-literal "[...]" (RFC 3986, e.g. IPv6) as one unit so inner ':' don't terminate the host walk. - if rightPos < len(bs) && bs[rightPos] == '[' { - if end := bytes.IndexByte(bs[rightPos:], ']'); end > 0 { - rightPos += end + 1 - } else { - rightPos++ // unmatched '['; let the host loop consume the rest - } - } rightLoop: for rightPos < len(bs) { c := bs[rightPos] switch c { case '.', '-': // valid host char + case '[': + // ipv6 begin + if rightPos != sepAtPos+1 { + break rightLoop + } + case ']': + // ipv6 end + rightPos++ + break rightLoop default: valid := 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z' || '0' <= c && c <= '9' if !valid { @@ -90,6 +92,7 @@ func SanitizeCredentialURLs(s string) string { // * http like URL: "https://userinfo@host.com" (it has "://" before the userinfo) needSanitize := bytes.IndexByte(leftPart, ':') >= 0 || bytes.HasSuffix(leading, schemeSep) needSanitize = needSanitize && len(leftPart) > 0 && len(rightPart) > 0 + // TODO: can also do more checks for right part, e.g.: ipv6 if needSanitize { res = append(res, leading...) res = append(res, userInfoPlaceholder...) From 74207368ee49a946fa01351bf674c473d0cf1b49 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 28 Apr 2026 11:56:04 +0800 Subject: [PATCH 12/13] ipv6 parsing --- modules/util/sanitize.go | 11 ++++++++++- modules/util/sanitize_test.go | 8 ++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/modules/util/sanitize.go b/modules/util/sanitize.go index a7df0edf67907..4256f999f4ee5 100644 --- a/modules/util/sanitize.go +++ b/modules/util/sanitize.go @@ -5,6 +5,7 @@ package util import ( "bytes" + "net" "strings" ) @@ -78,6 +79,10 @@ func SanitizeCredentialURLs(s string) string { break rightLoop default: valid := 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z' || '0' <= c && c <= '9' + if bs[sepAtPos+1] == '[' { + // ipv6 host + valid = 'a' <= c && c <= 'f' || 'A' <= c && c <= 'F' || '0' <= c && c <= '9' || c == ':' + } if !valid { break rightLoop } @@ -92,7 +97,11 @@ func SanitizeCredentialURLs(s string) string { // * http like URL: "https://userinfo@host.com" (it has "://" before the userinfo) needSanitize := bytes.IndexByte(leftPart, ':') >= 0 || bytes.HasSuffix(leading, schemeSep) needSanitize = needSanitize && len(leftPart) > 0 && len(rightPart) > 0 - // TODO: can also do more checks for right part, e.g.: ipv6 + // TODO: can also do more checks for right part + // for example: ipv6 quick check + if needSanitize && rightPart[0] == '[' { + needSanitize = rightPart[len(rightPart)-1] == ']' && net.ParseIP(UnsafeBytesToString(rightPart[1:len(rightPart)-1])) != nil + } if needSanitize { res = append(res, leading...) res = append(res, userInfoPlaceholder...) diff --git a/modules/util/sanitize_test.go b/modules/util/sanitize_test.go index 526e898b98066..c1a80016f8d15 100644 --- a/modules/util/sanitize_test.go +++ b/modules/util/sanitize_test.go @@ -46,8 +46,12 @@ func TestSanitizeCredentialURLs(t *testing.T) { "see https://" + userInfoPlaceholder + "@[::1]/x and https://" + userInfoPlaceholder + "@h2", }, { - "https://user:secret@[abc", // unmatched '[' must still mask credentials - "https://" + userInfoPlaceholder + "@[abc", + "https://user:secret@[unclosed-ipv6", + "https://user:secret@[unclosed-ipv6", + }, + { + "https://user:secret@[invalid-ipv6]", + "https://user:secret@[invalid-ipv6]", }, { "ftp://x@", From 080b0e565fa802d32b48a2a145779404bc186ce5 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 28 Apr 2026 13:02:26 +0800 Subject: [PATCH 13/13] fine tune comments --- modules/util/sanitize.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/modules/util/sanitize.go b/modules/util/sanitize.go index 4256f999f4ee5..88ca34788fc61 100644 --- a/modules/util/sanitize.go +++ b/modules/util/sanitize.go @@ -36,14 +36,19 @@ const userInfoPlaceholder = "(masked)" // "***" is a magic string internally used, doesn't guarantee to be anything. func SanitizeCredentialURLs(s string) string { sepColPos := strings.Index(s, ":") - sepAtPos := sepColPos + 1 + strings.Index(s[sepColPos+1:], "@") + if sepColPos == -1 { + return s // fast path: no colon, unlikely contain any URL credential + } + sepAtPos := strings.Index(s[sepColPos+1:], "@") for sepAtPos == -1 { - return s // fast path, unlikely contain any URL + return s // fast path: no "@" after colon, unlikely contain any URL credential } + sepAtPos += sepColPos + 1 res := make([]byte, 0, len(s)+len(userInfoPlaceholder)) // a best guess to avoid too many re-allocations bs := UnsafeStringToBytes(s) for { + // left part (before "@") is likely to be the "userinfo" (single username, or "username:password") leftPos := sepAtPos - 1 leftLoop: for leftPos >= 0 { @@ -59,8 +64,10 @@ func SanitizeCredentialURLs(s string) string { } leftPos-- } + // left pos should point to the beginning of the left part, this pos is always valid in the buffer leftPos++ + // right part is likely to be the host (domain name, ip address) rightPos := sepAtPos + 1 rightLoop: for rightPos < len(bs) { @@ -93,7 +100,7 @@ func SanitizeCredentialURLs(s string) string { leading, leftPart, rightPart := bs[:leftPos], bs[leftPos:sepAtPos], bs[sepAtPos+1:rightPos] // Either: - // * git log message: "user:pass@host" (it contains a colon in userinfo) + // * git log message: "user:pass@host" (it contains a colon in userinfo), ignore "git@host" pattern // * http like URL: "https://userinfo@host.com" (it has "://" before the userinfo) needSanitize := bytes.IndexByte(leftPart, ':') >= 0 || bytes.HasSuffix(leading, schemeSep) needSanitize = needSanitize && len(leftPart) > 0 && len(rightPart) > 0