Refactor legacy code (#35708)

And by the way, remove the legacy TODO, split large functions into small
ones, and add more tests

cmd/serv.go

@@ -13,7 +13,6 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
-	"time"
 	"unicode"
 
 	asymkey_model "code.gitea.io/gitea/models/asymkey"
@@ -32,7 +31,6 @@ import (
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/services/lfs"
 
-	"github.com/golang-jwt/jwt/v5"
 	"github.com/kballard/go-shellquote"
 	"github.com/urfave/cli/v3"
 )
@@ -133,27 +131,6 @@ func getAccessMode(verb, lfsVerb string) perm.AccessMode {
 	return perm.AccessModeNone
 }
 
-func getLFSAuthToken(ctx context.Context, lfsVerb string, results *private.ServCommandResults) (string, error) {
-	now := time.Now()
-	claims := lfs.Claims{
-		RegisteredClaims: jwt.RegisteredClaims{
-			ExpiresAt: jwt.NewNumericDate(now.Add(setting.LFS.HTTPAuthExpiry)),
-			NotBefore: jwt.NewNumericDate(now),
-		},
-		RepoID: results.RepoID,
-		Op:     lfsVerb,
-		UserID: results.UserID,
-	}
-	token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims)
-
-	// Sign and get the complete encoded token as a string using the secret
-	tokenString, err := token.SignedString(setting.LFS.JWTSecretBytes)
-	if err != nil {
-		return "", fail(ctx, "Failed to sign JWT Token", "Failed to sign JWT token: %v", err)
-	}
-	return "Bearer " + tokenString, nil
-}
-
 func runServ(ctx context.Context, c *cli.Command) error {
 	// FIXME: This needs to internationalised
 	setup(ctx, c.Bool("debug"))
@@ -283,7 +260,7 @@ func runServ(ctx context.Context, c *cli.Command) error {
 
 	// LFS SSH protocol
 	if verb == git.CmdVerbLfsTransfer {
-		token, err := getLFSAuthToken(ctx, lfsVerb, results)
+		token, err := lfs.GetLFSAuthTokenWithBearer(lfs.AuthTokenOptions{Op: lfsVerb, UserID: results.UserID, RepoID: results.RepoID})
 		if err != nil {
 			return err
 		}
@@ -294,7 +271,7 @@ func runServ(ctx context.Context, c *cli.Command) error {
 	if verb == git.CmdVerbLfsAuthenticate {
 		url := fmt.Sprintf("%s%s/%s.git/info/lfs", setting.AppURL, url.PathEscape(results.OwnerName), url.PathEscape(results.RepoName))
 
-		token, err := getLFSAuthToken(ctx, lfsVerb, results)
+		token, err := lfs.GetLFSAuthTokenWithBearer(lfs.AuthTokenOptions{Op: lfsVerb, UserID: results.UserID, RepoID: results.RepoID})
 		if err != nil {
 			return err
 		}

models/asymkey/ssh_key.go

@@ -67,13 +67,6 @@ func (key *PublicKey) OmitEmail() string {
 	return strings.Join(strings.Split(key.Content, " ")[:2], " ")
 }
 
-// AuthorizedString returns formatted public key string for authorized_keys file.
-//
-// TODO: Consider dropping this function
-func (key *PublicKey) AuthorizedString() string {
-	return AuthorizedStringForKey(key)
-}
-
 func addKey(ctx context.Context, key *PublicKey) (err error) {
 	if len(key.Fingerprint) == 0 {
 		key.Fingerprint, err = CalcFingerprint(key.Content)

models/asymkey/ssh_key_authorized_keys.go

@@ -17,29 +17,13 @@ import (
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
+
+	"golang.org/x/crypto/ssh"
 )
 
-//  _____          __  .__                 .__                  .___
+// AuthorizedStringCommentPrefix is a magic tag
-// /  _  \  __ ___/  |_|  |__   ___________|__|_______ ____   __| _/
+// some functions like RegeneratePublicKeys needs this tag to skip the keys generated by Gitea, while keep other keys
-// /  /_\  \|  |  \   __\  |  \ /  _ \_  __ \  \___   // __ \ / __ |
+const AuthorizedStringCommentPrefix = `# gitea public key`
-// /    |    \  |  /|  | |   Y  (  <_> )  | \/  |/    /\  ___// /_/ |
-// \____|__  /____/ |__| |___|  /\____/|__|  |__/_____ \\___  >____ |
-//         \/                 \/                      \/    \/     \/
-// ____  __.
-// |    |/ _|____ ___.__. ______
-// |      <_/ __ <   |  |/  ___/
-// |    |  \  ___/\___  |\___ \
-// |____|__ \___  > ____/____  >
-//         \/   \/\/         \/
-//
-// This file contains functions for creating authorized_keys files
-//
-// There is a dependence on the database within RegeneratePublicKeys however most of these functions probably belong in a module
-
-const (
-	tplCommentPrefix = `# gitea public key`
-	tplPublicKey     = tplCommentPrefix + "\n" + `command=%s,no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty,no-user-rc,restrict %s` + "\n"
-)
 
 var sshOpLocker sync.Mutex
 
@@ -50,17 +34,45 @@ func WithSSHOpLocker(f func() error) error {
 }
 
 // AuthorizedStringForKey creates the authorized keys string appropriate for the provided key
-func AuthorizedStringForKey(key *PublicKey) string {
+func AuthorizedStringForKey(key *PublicKey) (string, error) {
 	sb := &strings.Builder{}
-	_ = setting.SSH.AuthorizedKeysCommandTemplateTemplate.Execute(sb, map[string]any{
+	_, err := writeAuthorizedStringForKey(key, sb)
+	return sb.String(), err
+}
+
+// WriteAuthorizedStringForValidKey writes the authorized key for the provided key. If the key is invalid, it does nothing.
+func WriteAuthorizedStringForValidKey(key *PublicKey, w io.Writer) error {
+	validKey, err := writeAuthorizedStringForKey(key, w)
+	if !validKey {
+		log.Debug("WriteAuthorizedStringForValidKey: key %s is not valid: %v", key, err)
+		return nil
+	}
+	return err
+}
+
+func writeAuthorizedStringForKey(key *PublicKey, w io.Writer) (keyValid bool, err error) {
+	const tpl = AuthorizedStringCommentPrefix + "\n" + `command=%s,no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty,no-user-rc,restrict %s %s` + "\n"
+	pubKey, _, _, _, err := ssh.ParseAuthorizedKey([]byte(key.Content))
+	if err != nil {
+		return false, err
+	}
+	// now the key is valid, the code below could only return template/IO related errors
+	sbCmd := &strings.Builder{}
+	err = setting.SSH.AuthorizedKeysCommandTemplateTemplate.Execute(sbCmd, map[string]any{
 		"AppPath":     util.ShellEscape(setting.AppPath),
 		"AppWorkPath": util.ShellEscape(setting.AppWorkPath),
 		"CustomConf":  util.ShellEscape(setting.CustomConf),
 		"CustomPath":  util.ShellEscape(setting.CustomPath),
 		"Key":         key,
 	})
-
+	if err != nil {
-	return fmt.Sprintf(tplPublicKey, util.ShellEscape(sb.String()), key.Content)
+		return true, err
+	}
+	sshCommandEscaped := util.ShellEscape(sbCmd.String())
+	sshKeyMarshalled := strings.TrimSpace(string(ssh.MarshalAuthorizedKey(pubKey)))
+	sshKeyComment := fmt.Sprintf("user-%d", key.OwnerID)
+	_, err = fmt.Fprintf(w, tpl, sshCommandEscaped, sshKeyMarshalled, sshKeyComment)
+	return true, err
 }
 
 // appendAuthorizedKeysToFile appends new SSH keys' content to authorized_keys file.
@@ -112,7 +124,7 @@ func appendAuthorizedKeysToFile(keys ...*PublicKey) error {
 		if key.Type == KeyTypePrincipal {
 			continue
 		}
-		if _, err = f.WriteString(key.AuthorizedString()); err != nil {
+		if err = WriteAuthorizedStringForValidKey(key, f); err != nil {
 			return err
 		}
 	}
@@ -120,10 +132,9 @@ func appendAuthorizedKeysToFile(keys ...*PublicKey) error {
 }
 
 // RegeneratePublicKeys regenerates the authorized_keys file
-func RegeneratePublicKeys(ctx context.Context, t io.StringWriter) error {
+func RegeneratePublicKeys(ctx context.Context, t io.Writer) error {
 	if err := db.GetEngine(ctx).Where("type != ?", KeyTypePrincipal).Iterate(new(PublicKey), func(idx int, bean any) (err error) {
-		_, err = t.WriteString((bean.(*PublicKey)).AuthorizedString())
+		return WriteAuthorizedStringForValidKey(bean.(*PublicKey), t)
-		return err
 	}); err != nil {
 		return err
 	}
@@ -144,11 +155,11 @@ func RegeneratePublicKeys(ctx context.Context, t io.StringWriter) error {
 		scanner := bufio.NewScanner(f)
 		for scanner.Scan() {
 			line := scanner.Text()
-			if strings.HasPrefix(line, tplCommentPrefix) {
+			if strings.HasPrefix(line, AuthorizedStringCommentPrefix) {
 				scanner.Scan()
 				continue
 			}
-			_, err = t.WriteString(line + "\n")
+			_, err = io.WriteString(t, line+"\n")
 			if err != nil {
 				return err
 			}

models/repo/upload.go

@@ -127,16 +127,9 @@ func DeleteUploads(ctx context.Context, uploads ...*Upload) (err error) {
 
 	for _, upload := range uploads {
 		localPath := upload.LocalPath()
-		isFile, err := util.IsFile(localPath)
-		if err != nil {
-			log.Error("Unable to check if %s is a file. Error: %v", localPath, err)
-		}
-		if !isFile {
-			continue
-		}
-
 		if err := util.Remove(localPath); err != nil {
-			return fmt.Errorf("remove upload: %w", err)
+			// just continue, don't fail the whole operation if a file is missing (removed by others)
+			log.Error("unable to remove upload file %s: %v", localPath, err)
 		}
 	}
 

modules/git/hook.go

@@ -47,30 +47,16 @@ func GetHook(repoPath, name string) (*Hook, error) {
 		name: name,
 		path: filepath.Join(repoPath, filepath.Join("hooks", name+".d", name)),
 	}
-	isFile, err := util.IsFile(h.path)
+	if data, err := os.ReadFile(h.path); err == nil {
-	if err != nil {
-		return nil, err
-	}
-	if isFile {
-		data, err := os.ReadFile(h.path)
-		if err != nil {
-			return nil, err
-		}
 		h.IsActive = true
 		h.Content = string(data)
 		return h, nil
+	} else if !os.IsNotExist(err) {
+		return nil, err
 	}
 
 	samplePath := filepath.Join(repoPath, "hooks", name+".sample")
-	isFile, err = util.IsFile(samplePath)
+	if data, err := os.ReadFile(samplePath); err == nil {
-	if err != nil {
-		return nil, err
-	}
-	if isFile {
-		data, err := os.ReadFile(samplePath)
-		if err != nil {
-			return nil, err
-		}
 		h.Sample = string(data)
 	}
 	return h, nil

modules/setting/config_provider.go

@@ -202,11 +202,11 @@ func NewConfigProviderFromFile(file string) (ConfigProvider, error) {
 	loadedFromEmpty := true
 
 	if file != "" {
-		isFile, err := util.IsFile(file)
+		isExist, err := util.IsExist(file)
 		if err != nil {
-			return nil, fmt.Errorf("unable to check if %q is a file. Error: %v", file, err)
+			return nil, fmt.Errorf("unable to check if %q exists: %v", file, err)
 		}
-		if isFile {
+		if isExist {
 			if err = cfg.Append(file); err != nil {
 				return nil, fmt.Errorf("failed to load config file %q: %v", file, err)
 			}

modules/util/path.go

@@ -115,15 +115,10 @@ func IsDir(dir string) (bool, error) {
 	return false, err
 }
 
-// IsFile returns true if given path is a file,
+func IsRegularFile(filePath string) (bool, error) {
-// or returns false when it's a directory or does not exist.
+	f, err := os.Lstat(filePath)
-func IsFile(filePath string) (bool, error) {
-	f, err := os.Stat(filePath)
 	if err == nil {
-		return !f.IsDir(), nil
+		return f.Mode().IsRegular(), nil
-	}
-	if os.IsNotExist(err) {
-		return false, nil
 	}
 	return false, err
 }

routers/private/key.go

@@ -45,7 +45,7 @@ func UpdatePublicKeyInRepo(ctx *context.PrivateContext) {
 	ctx.PlainText(http.StatusOK, "success")
 }
 
-// AuthorizedPublicKeyByContent searches content as prefix (leak e-mail part)
+// AuthorizedPublicKeyByContent searches content as prefix (without comment part)
 // and returns public key found.
 func AuthorizedPublicKeyByContent(ctx *context.PrivateContext) {
 	content := ctx.FormString("content")
@@ -57,5 +57,14 @@ func AuthorizedPublicKeyByContent(ctx *context.PrivateContext) {
 		})
 		return
 	}
-	ctx.PlainText(http.StatusOK, publicKey.AuthorizedString())
+
+	authorizedString, err := asymkey_model.AuthorizedStringForKey(publicKey)
+	if err != nil {
+		ctx.JSON(http.StatusInternalServerError, private.Response{
+			Err:     err.Error(),
+			UserMsg: "invalid public key",
+		})
+		return
+	}
+	ctx.PlainText(http.StatusOK, authorizedString)
 }

services/asymkey/ssh_key_authorized_principals.go

@@ -25,10 +25,7 @@ import (
 // There is a dependence on the database within RewriteAllPrincipalKeys & RegeneratePrincipalKeys
 // The sshOpLocker is used from ssh_key_authorized_keys.go
 
-const (
+const authorizedPrincipalsFile = "authorized_principals"
-	authorizedPrincipalsFile = "authorized_principals"
-	tplCommentPrefix         = `# gitea public key`
-)
 
 // RewriteAllPrincipalKeys removes any authorized principal and rewrite all keys from database again.
 // Note: db.GetEngine(ctx).Iterate does not get latest data after insert/delete, so we have to call this function
@@ -90,10 +87,9 @@ func rewriteAllPrincipalKeys(ctx context.Context) error {
 	return util.Rename(tmpPath, fPath)
 }
 
-func regeneratePrincipalKeys(ctx context.Context, t io.StringWriter) error {
+func regeneratePrincipalKeys(ctx context.Context, t io.Writer) error {
 	if err := db.GetEngine(ctx).Where("type = ?", asymkey_model.KeyTypePrincipal).Iterate(new(asymkey_model.PublicKey), func(idx int, bean any) (err error) {
-		_, err = t.WriteString((bean.(*asymkey_model.PublicKey)).AuthorizedString())
+		return asymkey_model.WriteAuthorizedStringForValidKey(bean.(*asymkey_model.PublicKey), t)
-		return err
 	}); err != nil {
 		return err
 	}
@@ -114,11 +110,11 @@ func regeneratePrincipalKeys(ctx context.Context, t io.StringWriter) error {
 		scanner := bufio.NewScanner(f)
 		for scanner.Scan() {
 			line := scanner.Text()
-			if strings.HasPrefix(line, tplCommentPrefix) {
+			if strings.HasPrefix(line, asymkey_model.AuthorizedStringCommentPrefix) {
 				scanner.Scan()
 				continue
 			}
-			_, err = t.WriteString(line + "\n")
+			_, err = io.WriteString(t, line+"\n")
 			if err != nil {
 				return err
 			}

services/doctor/authorizedkeys.go

@@ -20,8 +20,6 @@ import (
 	asymkey_service "code.gitea.io/gitea/services/asymkey"
 )
 
-const tplCommentPrefix = `# gitea public key`
-
 func checkAuthorizedKeys(ctx context.Context, logger log.Logger, autofix bool) error {
 	if setting.SSH.StartBuiltinServer || !setting.SSH.CreateAuthorizedKeysFile {
 		return nil
@@ -47,7 +45,7 @@ func checkAuthorizedKeys(ctx context.Context, logger log.Logger, autofix bool) e
 	scanner := bufio.NewScanner(f)
 	for scanner.Scan() {
 		line := scanner.Text()
-		if strings.HasPrefix(line, tplCommentPrefix) {
+		if strings.HasPrefix(line, asymkey_model.AuthorizedStringCommentPrefix) {
 			continue
 		}
 		linesInAuthorizedKeys.Add(line)
@@ -67,7 +65,7 @@ func checkAuthorizedKeys(ctx context.Context, logger log.Logger, autofix bool) e
 	scanner = bufio.NewScanner(regenerated)
 	for scanner.Scan() {
 		line := scanner.Text()
-		if strings.HasPrefix(line, tplCommentPrefix) {
+		if strings.HasPrefix(line, asymkey_model.AuthorizedStringCommentPrefix) {
 			continue
 		}
 		if linesInAuthorizedKeys.Contains(line) {

services/lfs/server.go

@@ -18,6 +18,7 @@ import (
 	"regexp"
 	"strconv"
 	"strings"
+	"time"
 
 	actions_model "code.gitea.io/gitea/models/actions"
 	auth_model "code.gitea.io/gitea/models/auth"
@@ -54,6 +55,33 @@ type Claims struct {
 	jwt.RegisteredClaims
 }
 
+type AuthTokenOptions struct {
+	Op     string
+	UserID int64
+	RepoID int64
+}
+
+func GetLFSAuthTokenWithBearer(opts AuthTokenOptions) (string, error) {
+	now := time.Now()
+	claims := Claims{
+		RegisteredClaims: jwt.RegisteredClaims{
+			ExpiresAt: jwt.NewNumericDate(now.Add(setting.LFS.HTTPAuthExpiry)),
+			NotBefore: jwt.NewNumericDate(now),
+		},
+		RepoID: opts.RepoID,
+		Op:     opts.Op,
+		UserID: opts.UserID,
+	}
+	token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims)
+
+	// Sign and get the complete encoded token as a string using the secret
+	tokenString, err := token.SignedString(setting.LFS.JWTSecretBytes)
+	if err != nil {
+		return "", fmt.Errorf("failed to sign LFS JWT token: %w", err)
+	}
+	return "Bearer " + tokenString, nil
+}
+
 // DownloadLink builds a URL to download the object.
 func (rc *requestContext) DownloadLink(p lfs_module.Pointer) string {
 	return setting.AppURL + path.Join(url.PathEscape(rc.User), url.PathEscape(rc.Repo+".git"), "info/lfs/objects", url.PathEscape(p.Oid))
@@ -559,9 +587,6 @@ func authenticate(ctx *context.Context, repository *repo_model.Repository, autho
 }
 
 func handleLFSToken(ctx stdCtx.Context, tokenSHA string, target *repo_model.Repository, mode perm_model.AccessMode) (*user_model.User, error) {
-	if !strings.Contains(tokenSHA, ".") {
-		return nil, nil
-	}
 	token, err := jwt.ParseWithClaims(tokenSHA, &Claims{}, func(t *jwt.Token) (any, error) {
 		if _, ok := t.Method.(*jwt.SigningMethodHMAC); !ok {
 			return nil, fmt.Errorf("unexpected signing method: %v", t.Header["alg"])
@@ -569,7 +594,7 @@ func handleLFSToken(ctx stdCtx.Context, tokenSHA string, target *repo_model.Repo
 		return setting.LFS.JWTSecretBytes, nil
 	})
 	if err != nil {
-		return nil, nil
+		return nil, errors.New("invalid token")
 	}
 
 	claims, claimsOk := token.Claims.(*Claims)

services/lfs/server_test.go

@@ -0,0 +1,51 @@
+// Copyright 2025 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package lfs
+
+import (
+	"strings"
+	"testing"
+
+	perm_model "code.gitea.io/gitea/models/perm"
+	repo_model "code.gitea.io/gitea/models/repo"
+	"code.gitea.io/gitea/models/unittest"
+	"code.gitea.io/gitea/services/contexttest"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestMain(m *testing.M) {
+	unittest.MainTest(m)
+}
+
+func TestAuthenticate(t *testing.T) {
+	require.NoError(t, unittest.PrepareTestDatabase())
+	repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
+
+	token2, _ := GetLFSAuthTokenWithBearer(AuthTokenOptions{Op: "download", UserID: 2, RepoID: 1})
+	_, token2, _ = strings.Cut(token2, " ")
+	ctx, _ := contexttest.MockContext(t, "/")
+
+	t.Run("handleLFSToken", func(t *testing.T) {
+		u, err := handleLFSToken(ctx, "", repo1, perm_model.AccessModeRead)
+		require.Error(t, err)
+		assert.Nil(t, u)
+
+		u, err = handleLFSToken(ctx, "invalid", repo1, perm_model.AccessModeRead)
+		require.Error(t, err)
+		assert.Nil(t, u)
+
+		u, err = handleLFSToken(ctx, token2, repo1, perm_model.AccessModeRead)
+		require.NoError(t, err)
+		assert.EqualValues(t, 2, u.ID)
+	})
+
+	t.Run("authenticate", func(t *testing.T) {
+		const prefixBearer = "Bearer "
+		assert.False(t, authenticate(ctx, repo1, "", true, false))
+		assert.False(t, authenticate(ctx, repo1, prefixBearer+"invalid", true, false))
+		assert.True(t, authenticate(ctx, repo1, prefixBearer+token2, true, false))
+	})
+}

services/repository/generate.go

@@ -13,6 +13,7 @@ import (
 	"regexp"
 	"strconv"
 	"strings"
+	"sync"
 	"time"
 
 	git_model "code.gitea.io/gitea/models/git"
@@ -40,7 +41,12 @@ type expansion struct {
 	Transformers []transformer
 }
 
-var defaultTransformers = []transformer{
+var globalVars = sync.OnceValue(func() (ret struct {
+	defaultTransformers    []transformer
+	fileNameSanitizeRegexp *regexp.Regexp
+},
+) {
+	ret.defaultTransformers = []transformer{
 		{Name: "SNAKE", Transform: xstrings.ToSnakeCase},
 		{Name: "KEBAB", Transform: xstrings.ToKebabCase},
 		{Name: "CAMEL", Transform: xstrings.ToCamelCase},
@@ -48,21 +54,28 @@ var defaultTransformers = []transformer{
 		{Name: "LOWER", Transform: strings.ToLower},
 		{Name: "UPPER", Transform: strings.ToUpper},
 		{Name: "TITLE", Transform: util.ToTitleCase},
-}
+	}
 
-func generateExpansion(ctx context.Context, src string, templateRepo, generateRepo *repo_model.Repository, sanitizeFileName bool) string {
+	// invalid filename contents, based on https://github.com/sindresorhus/filename-reserved-regex
+	// "COM10" needs to be opened with UNC "\\.\COM10" on Windows, so itself is valid
+	ret.fileNameSanitizeRegexp = regexp.MustCompile(`(?i)[<>:"/\\|?*\x{0000}-\x{001F}]|^(con|prn|aux|nul|com\d|lpt\d)$`)
+	return ret
+})
+
+func generateExpansion(ctx context.Context, src string, templateRepo, generateRepo *repo_model.Repository) string {
+	transformers := globalVars().defaultTransformers
 	year, month, day := time.Now().Date()
 	expansions := []expansion{
 		{Name: "YEAR", Value: strconv.Itoa(year), Transformers: nil},
 		{Name: "MONTH", Value: fmt.Sprintf("%02d", int(month)), Transformers: nil},
-		{Name: "MONTH_ENGLISH", Value: month.String(), Transformers: defaultTransformers},
+		{Name: "MONTH_ENGLISH", Value: month.String(), Transformers: transformers},
 		{Name: "DAY", Value: fmt.Sprintf("%02d", day), Transformers: nil},
-		{Name: "REPO_NAME", Value: generateRepo.Name, Transformers: defaultTransformers},
+		{Name: "REPO_NAME", Value: generateRepo.Name, Transformers: transformers},
-		{Name: "TEMPLATE_NAME", Value: templateRepo.Name, Transformers: defaultTransformers},
+		{Name: "TEMPLATE_NAME", Value: templateRepo.Name, Transformers: transformers},
 		{Name: "REPO_DESCRIPTION", Value: generateRepo.Description, Transformers: nil},
 		{Name: "TEMPLATE_DESCRIPTION", Value: templateRepo.Description, Transformers: nil},
-		{Name: "REPO_OWNER", Value: generateRepo.OwnerName, Transformers: defaultTransformers},
+		{Name: "REPO_OWNER", Value: generateRepo.OwnerName, Transformers: transformers},
-		{Name: "TEMPLATE_OWNER", Value: templateRepo.OwnerName, Transformers: defaultTransformers},
+		{Name: "TEMPLATE_OWNER", Value: templateRepo.OwnerName, Transformers: transformers},
 		{Name: "REPO_LINK", Value: generateRepo.Link(), Transformers: nil},
 		{Name: "TEMPLATE_LINK", Value: templateRepo.Link(), Transformers: nil},
 		{Name: "REPO_HTTPS_URL", Value: generateRepo.CloneLinkGeneral(ctx).HTTPS, Transformers: nil},
@@ -80,32 +93,23 @@ func generateExpansion(ctx context.Context, src string, templateRepo, generateRe
 	}
 
 	return os.Expand(src, func(key string) string {
-		if expansion, ok := expansionMap[key]; ok {
+		if val, ok := expansionMap[key]; ok {
-			if sanitizeFileName {
+			return val
-				return fileNameSanitize(expansion)
-			}
-			return expansion
 		}
 		return key
 	})
 }
 
-// GiteaTemplate holds information about a .gitea/template file
+// giteaTemplateFileMatcher holds information about a .gitea/template file
-type GiteaTemplate struct {
+type giteaTemplateFileMatcher struct {
-	Path    string
+	LocalFullPath string
-	Content []byte
-
 	globs         []glob.Glob
 }
 
-// Globs parses the .gitea/template globs or returns them if they were already parsed
+func newGiteaTemplateFileMatcher(fullPath string, content []byte) *giteaTemplateFileMatcher {
-func (gt *GiteaTemplate) Globs() []glob.Glob {
+	gt := &giteaTemplateFileMatcher{LocalFullPath: fullPath}
-	if gt.globs != nil {
-		return gt.globs
-	}
-
 	gt.globs = make([]glob.Glob, 0)
-	scanner := bufio.NewScanner(bytes.NewReader(gt.Content))
+	scanner := bufio.NewScanner(bytes.NewReader(content))
 	for scanner.Scan() {
 		line := strings.TrimSpace(scanner.Text())
 		if line == "" || strings.HasPrefix(line, "#") {
@@ -113,73 +117,91 @@ func (gt *GiteaTemplate) Globs() []glob.Glob {
 		}
 		g, err := glob.Compile(line, '/')
 		if err != nil {
-			log.Info("Invalid glob expression '%s' (skipped): %v", line, err)
+			log.Debug("Invalid glob expression '%s' (skipped): %v", line, err)
 			continue
 		}
 		gt.globs = append(gt.globs, g)
 	}
-	return gt.globs
+	return gt
 }
 
-func readGiteaTemplateFile(tmpDir string) (*GiteaTemplate, error) {
+func (gt *giteaTemplateFileMatcher) HasRules() bool {
-	gtPath := filepath.Join(tmpDir, ".gitea", "template")
+	return len(gt.globs) != 0
-	if _, err := os.Stat(gtPath); os.IsNotExist(err) {
+}
+
+func (gt *giteaTemplateFileMatcher) Match(s string) bool {
+	for _, g := range gt.globs {
+		if g.Match(s) {
+			return true
+		}
+	}
+	return false
+}
+
+func readGiteaTemplateFile(tmpDir string) (*giteaTemplateFileMatcher, error) {
+	localPath := filepath.Join(tmpDir, ".gitea", "template")
+	if _, err := os.Stat(localPath); os.IsNotExist(err) {
 		return nil, nil
 	} else if err != nil {
 		return nil, err
 	}
 
-	content, err := os.ReadFile(gtPath)
+	content, err := os.ReadFile(localPath)
 	if err != nil {
 		return nil, err
 	}
 
-	return &GiteaTemplate{Path: gtPath, Content: content}, nil
+	return newGiteaTemplateFileMatcher(localPath, content), nil
 }
 
-func processGiteaTemplateFile(ctx context.Context, tmpDir string, templateRepo, generateRepo *repo_model.Repository, giteaTemplateFile *GiteaTemplate) error {
+func substGiteaTemplateFile(ctx context.Context, tmpDir, tmpDirSubPath string, templateRepo, generateRepo *repo_model.Repository) error {
-	if err := util.Remove(giteaTemplateFile.Path); err != nil {
+	tmpFullPath := filepath.Join(tmpDir, tmpDirSubPath)
-		return fmt.Errorf("remove .giteatemplate: %w", err)
+	if ok, err := util.IsRegularFile(tmpFullPath); !ok {
-	}
+		return err
-	if len(giteaTemplateFile.Globs()) == 0 {
-		return nil // Avoid walking tree if there are no globs
-	}
-	tmpDirSlash := strings.TrimSuffix(filepath.ToSlash(tmpDir), "/") + "/"
-	return filepath.WalkDir(tmpDirSlash, func(path string, d os.DirEntry, walkErr error) error {
-		if walkErr != nil {
-			return walkErr
 	}
 
-		if d.IsDir() {
+	content, err := os.ReadFile(tmpFullPath)
-			return nil
-		}
-
-		base := strings.TrimPrefix(filepath.ToSlash(path), tmpDirSlash)
-		for _, g := range giteaTemplateFile.Globs() {
-			if g.Match(base) {
-				content, err := os.ReadFile(path)
 	if err != nil {
 		return err
 	}
-
+	if err := util.Remove(tmpFullPath); err != nil {
-				generatedContent := []byte(generateExpansion(ctx, string(content), templateRepo, generateRepo, false))
-				if err := os.WriteFile(path, generatedContent, 0o644); err != nil {
 		return err
 	}
 
-				substPath := filepath.FromSlash(filepath.Join(tmpDirSlash, generateExpansion(ctx, base, templateRepo, generateRepo, true)))
+	generatedContent := generateExpansion(ctx, string(content), templateRepo, generateRepo)
-
+	substSubPath := filepath.Clean(filePathSanitize(generateExpansion(ctx, tmpDirSubPath, templateRepo, generateRepo)))
-				// Create parent subdirectories if needed or continue silently if it exists
+	newLocalPath := filepath.Join(tmpDir, substSubPath)
-				if err = os.MkdirAll(filepath.Dir(substPath), 0o755); err != nil {
+	regular, err := util.IsRegularFile(newLocalPath)
+	if canWrite := regular || os.IsNotExist(err); !canWrite {
+		return nil
+	}
+	if err := os.MkdirAll(filepath.Dir(newLocalPath), 0o755); err != nil {
 		return err
 	}
+	return os.WriteFile(newLocalPath, []byte(generatedContent), 0o644)
+}
 
-				// Substitute filename variables
+func processGiteaTemplateFile(ctx context.Context, tmpDir string, templateRepo, generateRepo *repo_model.Repository, fileMatcher *giteaTemplateFileMatcher) error {
-				if err = os.Rename(path, substPath); err != nil {
+	if err := util.Remove(fileMatcher.LocalFullPath); err != nil {
+		return fmt.Errorf("unable to remove .gitea/template: %w", err)
+	}
+	if !fileMatcher.HasRules() {
+		return nil // Avoid walking tree if there are no globs
+	}
+
+	return filepath.WalkDir(tmpDir, func(fullPath string, d os.DirEntry, walkErr error) error {
+		if walkErr != nil {
+			return walkErr
+		}
+		if d.IsDir() {
+			return nil
+		}
+		tmpDirSubPath, err := filepath.Rel(tmpDir, fullPath)
+		if err != nil {
 			return err
 		}
-				break
+		if fileMatcher.Match(filepath.ToSlash(tmpDirSubPath)) {
-			}
+			return substGiteaTemplateFile(ctx, tmpDir, tmpDirSubPath, templateRepo, generateRepo)
 		}
 		return nil
 	}) // end: WalkDir
@@ -219,13 +241,13 @@ func generateRepoCommit(ctx context.Context, repo, templateRepo, generateRepo *r
 	}
 
 	// Variable expansion
-	giteaTemplateFile, err := readGiteaTemplateFile(tmpDir)
+	fileMatcher, err := readGiteaTemplateFile(tmpDir)
 	if err != nil {
 		return fmt.Errorf("readGiteaTemplateFile: %w", err)
 	}
 
-	if giteaTemplateFile != nil {
+	if fileMatcher != nil {
-		err = processGiteaTemplateFile(ctx, tmpDir, templateRepo, generateRepo, giteaTemplateFile)
+		err = processGiteaTemplateFile(ctx, tmpDir, templateRepo, generateRepo, fileMatcher)
 		if err != nil {
 			return err
 		}
@@ -317,12 +339,17 @@ func (gro GenerateRepoOptions) IsValid() bool {
 		gro.IssueLabels || gro.ProtectedBranch // or other items as they are added
 }
 
-var fileNameSanitizeRegexp = regexp.MustCompile(`(?i)\.\.|[<>:\"/\\|?*\x{0000}-\x{001F}]|^(con|prn|aux|nul|com\d|lpt\d)$`)
+func filePathSanitize(s string) string {
-
+	fields := strings.Split(filepath.ToSlash(s), "/")
-// Sanitize user input to valid OS filenames
+	for i, field := range fields {
-//
+		field = strings.TrimSpace(strings.TrimSpace(globalVars().fileNameSanitizeRegexp.ReplaceAllString(field, "_")))
-//		Based on https://github.com/sindresorhus/filename-reserved-regex
+		if strings.HasPrefix(field, "..") {
-//	 Adds ".." to prevent directory traversal
+			field = "__" + field[2:]
-func fileNameSanitize(s string) string {
+		}
-	return strings.TrimSpace(fileNameSanitizeRegexp.ReplaceAllString(s, "_"))
+		if strings.EqualFold(field, ".git") {
+			field = "_" + field[1:]
+		}
+		fields[i] = field
+	}
+	return filepath.FromSlash(strings.Join(fields, "/"))
 }

services/repository/generate_test.go

@@ -4,13 +4,18 @@
 package repository
 
 import (
+	"os"
+	"path/filepath"
 	"testing"
 
+	repo_model "code.gitea.io/gitea/models/repo"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
-var giteaTemplate = []byte(`
+func TestGiteaTemplate(t *testing.T) {
+	giteaTemplate := []byte(`
 # Header
 
 # All .go files
@@ -23,48 +28,153 @@ text/*.txt
 **/modules/*
 `)
 
-func TestGiteaTemplate(t *testing.T) {
+	gt := newGiteaTemplateFileMatcher("", giteaTemplate)
-	gt := GiteaTemplate{Content: giteaTemplate}
+	assert.Len(t, gt.globs, 3)
-	assert.Len(t, gt.Globs(), 3)
 
 	tt := []struct {
 		Path  string
 		Match bool
 	}{
 		{Path: "main.go", Match: true},
-		{Path: "a/b/c/d/e.go", Match: true},
+		{Path: "sub/sub/foo.go", Match: true},
-		{Path: "main.txt", Match: false},
+
-		{Path: "a/b.txt", Match: false},
+		{Path: "a.txt", Match: false},
 		{Path: "text/a.txt", Match: true},
-		{Path: "text/b.txt", Match: true},
+		{Path: "sub/text/a.txt", Match: false},
-		{Path: "text/c.json", Match: false},
+		{Path: "text/a.json", Match: false},
+
 		{Path: "a/b/c/modules/README.md", Match: true},
 		{Path: "a/b/c/modules/d/README.md", Match: false},
 	}
 
 	for _, tc := range tt {
-		t.Run(tc.Path, func(t *testing.T) {
+		assert.Equal(t, tc.Match, gt.Match(tc.Path), "path: %s", tc.Path)
-			match := false
-			for _, g := range gt.Globs() {
-				if g.Match(tc.Path) {
-					match = true
-					break
-				}
-			}
-			assert.Equal(t, tc.Match, match)
-		})
 	}
 }
 
-func TestFileNameSanitize(t *testing.T) {
+func TestFilePathSanitize(t *testing.T) {
-	assert.Equal(t, "test_CON", fileNameSanitize("test_CON"))
+	assert.Equal(t, "test_CON", filePathSanitize("test_CON"))
-	assert.Equal(t, "test CON", fileNameSanitize("test CON "))
+	assert.Equal(t, "test CON", filePathSanitize("test CON "))
-	assert.Equal(t, "__traverse__", fileNameSanitize("../traverse/.."))
+	assert.Equal(t, "__/traverse/__", filePathSanitize(".. /traverse/ .."))
-	assert.Equal(t, "http___localhost_3003_user_test.git", fileNameSanitize("http://localhost:3003/user/test.git"))
+	assert.Equal(t, "./__/a/_git/b_", filePathSanitize("./../a/.git/ b: "))
-	assert.Equal(t, "_", fileNameSanitize("CON"))
+	assert.Equal(t, "_", filePathSanitize("CoN"))
-	assert.Equal(t, "_", fileNameSanitize("con"))
+	assert.Equal(t, "_", filePathSanitize("LpT1"))
-	assert.Equal(t, "_", fileNameSanitize("\u0000"))
+	assert.Equal(t, "_", filePathSanitize("CoM1"))
-	assert.Equal(t, "目标", fileNameSanitize("目标"))
+	assert.Equal(t, "_", filePathSanitize("\u0000"))
+	assert.Equal(t, "目标", filePathSanitize("目标"))
+	// unlike filepath.Clean, it only sanitizes, doesn't change the separator layout
+	assert.Equal(t, "", filePathSanitize("")) //nolint:testifylint // for easy reading
+	assert.Equal(t, ".", filePathSanitize("."))
+	assert.Equal(t, "/", filePathSanitize("/"))
+}
+
+func TestProcessGiteaTemplateFile(t *testing.T) {
+	tmpDir := filepath.Join(t.TempDir(), "gitea-template-test")
+
+	assertFileContent := func(path, expected string) {
+		data, err := os.ReadFile(filepath.Join(tmpDir, path))
+		if expected == "" {
+			assert.ErrorIs(t, err, os.ErrNotExist)
+			return
+		}
+		require.NoError(t, err)
+		assert.Equal(t, expected, string(data), "file content mismatch for %s", path)
+	}
+
+	assertSymLink := func(path, expected string) {
+		link, err := os.Readlink(filepath.Join(tmpDir, path))
+		if expected == "" {
+			assert.ErrorIs(t, err, os.ErrNotExist)
+			return
+		}
+		require.NoError(t, err)
+		assert.Equal(t, expected, link, "symlink target mismatch for %s", path)
+	}
+
+	require.NoError(t, os.MkdirAll(tmpDir+"/.gitea", 0o755))
+	require.NoError(t, os.WriteFile(tmpDir+"/.gitea/template", []byte("*\ninclude/**"), 0o644))
+	require.NoError(t, os.MkdirAll(tmpDir+"/sub", 0o755))
+	require.NoError(t, os.MkdirAll(tmpDir+"/include/foo/bar", 0o755))
+
+	require.NoError(t, os.WriteFile(tmpDir+"/sub/link-target", []byte("link target content from ${TEMPLATE_NAME}"), 0o644))
+	require.NoError(t, os.WriteFile(tmpDir+"/include/foo/bar/test.txt", []byte("include subdir ${TEMPLATE_NAME}"), 0o644))
+
+	// case-1
+	{
+		require.NoError(t, os.WriteFile(tmpDir+"/normal", []byte("normal content"), 0o644))
+		require.NoError(t, os.WriteFile(tmpDir+"/template", []byte("template from ${TEMPLATE_NAME}"), 0o644))
+	}
+
+	// case-2
+	{
+		require.NoError(t, os.Symlink(tmpDir+"/sub/link-target", tmpDir+"/link"))
+	}
+
+	// case-3
+	{
+		require.NoError(t, os.WriteFile(tmpDir+"/subst-${REPO_NAME}", []byte("dummy subst repo name"), 0o644))
+	}
+
+	// case-4
+	assertSubstTemplateName := func(normalContent, toLinkContent, fromLinkContent string) {
+		assertFileContent("subst-${TEMPLATE_NAME}-normal", normalContent)
+		assertFileContent("subst-${TEMPLATE_NAME}-to-link", toLinkContent)
+		assertFileContent("subst-${TEMPLATE_NAME}-from-link", fromLinkContent)
+	}
+	{
+		// will succeed
+		require.NoError(t, os.WriteFile(tmpDir+"/subst-${TEMPLATE_NAME}-normal", []byte("dummy subst template name normal"), 0o644))
+		// will skil if the path subst result is a link
+		require.NoError(t, os.WriteFile(tmpDir+"/subst-${TEMPLATE_NAME}-to-link", []byte("dummy subst template name to link"), 0o644))
+		require.NoError(t, os.Symlink(tmpDir+"/sub/link-target", tmpDir+"/subst-TemplateRepoName-to-link"))
+		// will be skipped since the source is a symlink
+		require.NoError(t, os.Symlink(tmpDir+"/sub/link-target", tmpDir+"/subst-${TEMPLATE_NAME}-from-link"))
+		// pre-check
+		assertSubstTemplateName("dummy subst template name normal", "dummy subst template name to link", "link target content from ${TEMPLATE_NAME}")
+	}
+
+	// process the template files
+	{
+		templateRepo := &repo_model.Repository{Name: "TemplateRepoName"}
+		generatedRepo := &repo_model.Repository{Name: "/../.gIt/name"}
+		fileMatcher, _ := readGiteaTemplateFile(tmpDir)
+		err := processGiteaTemplateFile(t.Context(), tmpDir, templateRepo, generatedRepo, fileMatcher)
+		require.NoError(t, err)
+		assertFileContent("include/foo/bar/test.txt", "include subdir TemplateRepoName")
+	}
+
+	// the lin target should never be modified, and since it is in a subdirectory, it is not affected by the template either
+	assertFileContent("sub/link-target", "link target content from ${TEMPLATE_NAME}")
+
+	// case-1
+	{
+		assertFileContent("no-such", "")
+		assertFileContent("normal", "normal content")
+		assertFileContent("template", "template from TemplateRepoName")
+	}
+
+	// case-2
+	{
+		// symlink with templates should be preserved (not read or write)
+		assertSymLink("link", tmpDir+"/sub/link-target")
+	}
+
+	// case-3
+	{
+		assertFileContent("subst-${REPO_NAME}", "")
+		assertFileContent("subst-/__/_gIt/name", "dummy subst repo name")
+	}
+
+	// case-4
+	{
+		// the paths with templates should have been removed, subst to a regular file, succeed, the link is preserved
+		assertSubstTemplateName("", "", "link target content from ${TEMPLATE_NAME}")
+		assertFileContent("subst-TemplateRepoName-normal", "dummy subst template name normal")
+		// subst to a link, skip, and the target is unchanged
+		assertSymLink("subst-TemplateRepoName-to-link", tmpDir+"/sub/link-target")
+		// subst from a link, skip, and the target is unchanged
+		assertSymLink("subst-${TEMPLATE_NAME}-from-link", tmpDir+"/sub/link-target")
+	}
 }
 
 func TestTransformers(t *testing.T) {
@@ -82,9 +192,9 @@ func TestTransformers(t *testing.T) {
 	}
 
 	input := "Abc_Def-XYZ"
-	assert.Len(t, defaultTransformers, len(cases))
+	assert.Len(t, globalVars().defaultTransformers, len(cases))
 	for i, c := range cases {
-		tf := defaultTransformers[i]
+		tf := globalVars().defaultTransformers[i]
 		require.Equal(t, c.name, tf.Name)
 		assert.Equal(t, c.expected, tf.Transform(input), "case %s", c.name)
 	}

tests/integration/cmd_keys_test.go

@@ -30,7 +30,7 @@ func Test_CmdKeys(t *testing.T) {
 				"with_key",
 				[]string{"keys", "-e", "git", "-u", "git", "-t", "ssh-rsa", "-k", "AAAAB3NzaC1yc2EAAAADAQABAAABgQDWVj0fQ5N8wNc0LVNA41wDLYJ89ZIbejrPfg/avyj3u/ZohAKsQclxG4Ju0VirduBFF9EOiuxoiFBRr3xRpqzpsZtnMPkWVWb+akZwBFAx8p+jKdy4QXR/SZqbVobrGwip2UjSrri1CtBxpJikojRIZfCnDaMOyd9Jp6KkujvniFzUWdLmCPxUE9zhTaPu0JsEP7MW0m6yx7ZUhHyfss+NtqmFTaDO+QlMR7L2QkDliN2Jl3Xa3PhuWnKJfWhdAq1Cw4oraKUOmIgXLkuiuxVQ6mD3AiFupkmfqdHq6h+uHHmyQqv3gU+/sD8GbGAhf6ftqhTsXjnv1Aj4R8NoDf9BS6KRkzkeun5UisSzgtfQzjOMEiJtmrep2ZQrMGahrXa+q4VKr0aKJfm+KlLfwm/JztfsBcqQWNcTURiCFqz+fgZw0Ey/de0eyMzldYTdXXNRYCKjs9bvBK+6SSXRM7AhftfQ0ZuoW5+gtinPrnmoOaSCEJbAiEiTO/BzOHgowiM="},
 				false,
-				"# gitea public key\ncommand=\"" + setting.AppPath + " --config=" + util.ShellEscape(setting.CustomConf) + " serv key-1\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty,no-user-rc,restrict ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQDWVj0fQ5N8wNc0LVNA41wDLYJ89ZIbejrPfg/avyj3u/ZohAKsQclxG4Ju0VirduBFF9EOiuxoiFBRr3xRpqzpsZtnMPkWVWb+akZwBFAx8p+jKdy4QXR/SZqbVobrGwip2UjSrri1CtBxpJikojRIZfCnDaMOyd9Jp6KkujvniFzUWdLmCPxUE9zhTaPu0JsEP7MW0m6yx7ZUhHyfss+NtqmFTaDO+QlMR7L2QkDliN2Jl3Xa3PhuWnKJfWhdAq1Cw4oraKUOmIgXLkuiuxVQ6mD3AiFupkmfqdHq6h+uHHmyQqv3gU+/sD8GbGAhf6ftqhTsXjnv1Aj4R8NoDf9BS6KRkzkeun5UisSzgtfQzjOMEiJtmrep2ZQrMGahrXa+q4VKr0aKJfm+KlLfwm/JztfsBcqQWNcTURiCFqz+fgZw0Ey/de0eyMzldYTdXXNRYCKjs9bvBK+6SSXRM7AhftfQ0ZuoW5+gtinPrnmoOaSCEJbAiEiTO/BzOHgowiM= user2@localhost\n",
+				"# gitea public key\ncommand=\"" + setting.AppPath + " --config=" + util.ShellEscape(setting.CustomConf) + " serv key-1\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty,no-user-rc,restrict ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQDWVj0fQ5N8wNc0LVNA41wDLYJ89ZIbejrPfg/avyj3u/ZohAKsQclxG4Ju0VirduBFF9EOiuxoiFBRr3xRpqzpsZtnMPkWVWb+akZwBFAx8p+jKdy4QXR/SZqbVobrGwip2UjSrri1CtBxpJikojRIZfCnDaMOyd9Jp6KkujvniFzUWdLmCPxUE9zhTaPu0JsEP7MW0m6yx7ZUhHyfss+NtqmFTaDO+QlMR7L2QkDliN2Jl3Xa3PhuWnKJfWhdAq1Cw4oraKUOmIgXLkuiuxVQ6mD3AiFupkmfqdHq6h+uHHmyQqv3gU+/sD8GbGAhf6ftqhTsXjnv1Aj4R8NoDf9BS6KRkzkeun5UisSzgtfQzjOMEiJtmrep2ZQrMGahrXa+q4VKr0aKJfm+KlLfwm/JztfsBcqQWNcTURiCFqz+fgZw0Ey/de0eyMzldYTdXXNRYCKjs9bvBK+6SSXRM7AhftfQ0ZuoW5+gtinPrnmoOaSCEJbAiEiTO/BzOHgowiM= user-2\n",
 			},
 			{"invalid", []string{"keys", "--not-a-flag=git"}, true, "Incorrect Usage: flag provided but not defined: -not-a-flag\n\n"},
 		}