Fix path resolving (#36734)

modules/util/path.go

@@ -64,7 +64,7 @@ func PathJoinRelX(elem ...string) string {
 	return PathJoinRel(elems...)
 }
 
-const pathSeparator = string(os.PathSeparator)
+const filepathSeparator = string(os.PathSeparator)
 
 // FilePathJoinAbs joins the path elements into a single file path, each element is cleaned by filepath.Clean separately.
 // All slashes/backslashes are converted to path separators before cleaning, the result only contains path separators.
@@ -82,7 +82,7 @@ func FilePathJoinAbs(base string, sub ...string) string {
 	if isOSWindows() {
 		elems[0] = filepath.Clean(base)
 	} else {
-		elems[0] = filepath.Clean(strings.ReplaceAll(base, "\\", pathSeparator))
+		elems[0] = filepath.Clean(strings.ReplaceAll(base, "\\", filepathSeparator))
 	}
 	if !filepath.IsAbs(elems[0]) {
 		// This shouldn't happen. If there is really necessary to pass in relative path, return the full path with filepath.Abs() instead
@@ -93,9 +93,9 @@ func FilePathJoinAbs(base string, sub ...string) string {
 			continue
 		}
 		if isOSWindows() {
-			elems = append(elems, filepath.Clean(pathSeparator+s))
+			elems = append(elems, filepath.Clean(filepathSeparator+s))
 		} else {
-			elems = append(elems, filepath.Clean(pathSeparator+strings.ReplaceAll(s, "\\", pathSeparator)))
+			elems = append(elems, filepath.Clean(filepathSeparator+strings.ReplaceAll(s, "\\", filepathSeparator)))
 		}
 	}
 	// the elems[0] must be an absolute path, just join them together
@@ -115,12 +115,72 @@ func IsDir(dir string) (bool, error) {
 	return false, err
 }
 
-func IsRegularFile(filePath string) (bool, error) {
+var ErrNotRegularPathFile = errors.New("not a regular file")
-	f, err := os.Lstat(filePath)
+
-	if err == nil {
+// ReadRegularPathFile reads a file with given sub path in root dir.
-		return f.Mode().IsRegular(), nil
+// It returns error when the path is not a regular file, or any parent path is not a regular directory.
+func ReadRegularPathFile(root, filePathIn string, limit int) ([]byte, error) {
+	pathFields := strings.Split(PathJoinRelX(filePathIn), "/")
+
+	targetPathBuilder := strings.Builder{}
+	targetPathBuilder.Grow(len(root) + len(filePathIn) + 2)
+	targetPathBuilder.WriteString(root)
+	targetPathString := root
+	for i, subPath := range pathFields {
+		targetPathBuilder.WriteByte(filepath.Separator)
+		targetPathBuilder.WriteString(subPath)
+		targetPathString = targetPathBuilder.String()
+
+		expectFile := i == len(pathFields)-1
+		st, err := os.Lstat(targetPathString)
+		if err != nil {
+			return nil, err
+		}
+		if expectFile && !st.Mode().IsRegular() || !expectFile && !st.Mode().IsDir() {
+			return nil, fmt.Errorf("%w: %s", ErrNotRegularPathFile, filePathIn)
+		}
 	}
-	return false, err
+	f, err := os.Open(targetPathString)
+	if err != nil {
+		return nil, err
+	}
+	defer f.Close()
+	return ReadWithLimit(f, limit)
+}
+
+// WriteRegularPathFile writes data to a file with given sub path in root dir, it creates parent directories if necessary.
+// The file is created with fileMode, and the directories are created with dirMode.
+// It returns error when the path already exists but is not a regular file, or any parent path is not a regular directory.
+func WriteRegularPathFile(root, filePathIn string, data []byte, dirMode, fileMode os.FileMode) error {
+	pathFields := strings.Split(PathJoinRelX(filePathIn), "/")
+
+	targetPathBuilder := strings.Builder{}
+	targetPathBuilder.Grow(len(root) + len(filePathIn) + 2)
+	targetPathBuilder.WriteString(root)
+	targetPathString := root
+	for i, subPath := range pathFields {
+		targetPathBuilder.WriteByte(filepath.Separator)
+		targetPathBuilder.WriteString(subPath)
+		targetPathString = targetPathBuilder.String()
+
+		expectFile := i == len(pathFields)-1
+		st, err := os.Lstat(targetPathString)
+		if err == nil {
+			if expectFile && !st.Mode().IsRegular() || !expectFile && !st.Mode().IsDir() {
+				return fmt.Errorf("%w: %s", ErrNotRegularPathFile, filePathIn)
+			}
+			continue
+		}
+		if !os.IsNotExist(err) {
+			return err
+		}
+		if !expectFile {
+			if err = os.Mkdir(targetPathString, dirMode); err != nil {
+				return err
+			}
+		}
+	}
+	return os.WriteFile(targetPathString, data, fileMode)
 }
 
 // IsExist checks whether a file or directory exists.

modules/util/path_test.go

@@ -6,6 +6,7 @@ package util
 import (
 	"net/url"
 	"os"
+	"path/filepath"
 	"runtime"
 	"testing"
 
@@ -230,3 +231,70 @@ func TestListDirRecursively(t *testing.T) {
 	require.NoError(t, err)
 	assert.ElementsMatch(t, []string{"d1/f-d1", "d1/s1/f-d1s1"}, res)
 }
+
+func TestReadWriteRegularPathFile(t *testing.T) {
+	const readLimit = 10000
+	tmpDir := t.TempDir()
+	rootDir := tmpDir + "/root"
+	_ = os.Mkdir(rootDir, 0o755)
+	_ = os.WriteFile(tmpDir+"/other-file", []byte("other-content"), 0o755)
+	_ = os.Mkdir(rootDir+"/real-dir", 0o755)
+	_ = os.WriteFile(rootDir+"/real-dir/real-file", []byte("dummy-content"), 0o644)
+	_ = os.Symlink(rootDir+"/real-dir", rootDir+"/link-dir")
+	_ = os.Symlink(rootDir+"/real-dir/real-file", rootDir+"/real-dir/link-file")
+
+	t.Run("Read", func(t *testing.T) {
+		content, err := os.ReadFile(filepath.Join(rootDir, "../other-file"))
+		require.NoError(t, err)
+		assert.Equal(t, "other-content", string(content))
+
+		content, err = ReadRegularPathFile(rootDir, "../other-file", readLimit)
+		require.ErrorIs(t, err, os.ErrNotExist)
+		assert.Empty(t, string(content))
+
+		content, err = ReadRegularPathFile(rootDir, "real-dir/real-file", readLimit)
+		require.NoError(t, err)
+		assert.Equal(t, "dummy-content", string(content))
+
+		_, err = ReadRegularPathFile(rootDir, "link-dir/real-file", readLimit)
+		require.ErrorIs(t, err, ErrNotRegularPathFile)
+		_, err = ReadRegularPathFile(rootDir, "real-dir/link-file", readLimit)
+		require.ErrorIs(t, err, ErrNotRegularPathFile)
+		_, err = ReadRegularPathFile(rootDir, "link-dir/link-file", readLimit)
+		require.ErrorIs(t, err, ErrNotRegularPathFile)
+	})
+
+	t.Run("Write", func(t *testing.T) {
+		assertFileContent := func(path, expected string) {
+			data, err := os.ReadFile(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)
+		}
+
+		err := WriteRegularPathFile(rootDir, "new-dir/new-file", []byte("new-content"), 0o755, 0o644)
+		require.NoError(t, err)
+		assertFileContent(rootDir+"/new-dir/new-file", "new-content")
+
+		err = WriteRegularPathFile(rootDir, "link-dir/real-file", []byte("new-content"), 0o755, 0o644)
+		require.ErrorIs(t, err, ErrNotRegularPathFile)
+		err = WriteRegularPathFile(rootDir, "link-dir/link-file", []byte("new-content"), 0o755, 0o644)
+		require.ErrorIs(t, err, ErrNotRegularPathFile)
+		err = WriteRegularPathFile(rootDir, "link-dir/new-file", []byte("new-content"), 0o755, 0o644)
+		require.ErrorIs(t, err, ErrNotRegularPathFile)
+		err = WriteRegularPathFile(rootDir, "real-dir/link-file", []byte("new-content"), 0o755, 0o644)
+		require.ErrorIs(t, err, ErrNotRegularPathFile)
+
+		err = WriteRegularPathFile(rootDir, "../other-file", []byte("new-content"), 0o755, 0o644)
+		require.NoError(t, err)
+		assertFileContent(rootDir+"/../other-file", "other-content")
+		assertFileContent(rootDir+"/other-file", "new-content")
+
+		err = WriteRegularPathFile(rootDir, "real-dir/real-file", []byte("changed-content"), 0o755, 0o644)
+		require.NoError(t, err)
+		assertFileContent(rootDir+"/real-dir/real-file", "changed-content")
+	})
+}

services/repository/generate.go

@@ -103,12 +103,12 @@ func generateExpansion(ctx context.Context, src string, templateRepo, generateRe
 
 // giteaTemplateFileMatcher holds information about a .gitea/template file
 type giteaTemplateFileMatcher struct {
-	LocalFullPath string
+	relPath string
-	globs         []glob.Glob
+	globs   []glob.Glob
 }
 
-func newGiteaTemplateFileMatcher(fullPath string, content []byte) *giteaTemplateFileMatcher {
+func newGiteaTemplateFileMatcher(relPath string, content []byte) *giteaTemplateFileMatcher {
-	gt := &giteaTemplateFileMatcher{LocalFullPath: fullPath}
+	gt := &giteaTemplateFileMatcher{relPath: relPath}
 	gt.globs = make([]glob.Glob, 0)
 	scanner := bufio.NewScanner(bytes.NewReader(content))
 	for scanner.Scan() {
@@ -139,64 +139,44 @@ func (gt *giteaTemplateFileMatcher) Match(s string) bool {
 	return false
 }
 
-func readLocalTmpRepoFileContent(localPath string, limit int) ([]byte, error) {
-	ok, err := util.IsRegularFile(localPath)
-	if err != nil {
-		return nil, err
-	} else if !ok {
-		return nil, fs.ErrNotExist
-	}
-
-	f, err := os.Open(localPath)
-	if err != nil {
-		return nil, err
-	}
-	defer f.Close()
-
-	return util.ReadWithLimit(f, limit)
-}
-
 func readGiteaTemplateFile(tmpDir string) (*giteaTemplateFileMatcher, error) {
-	localPath := filepath.Join(tmpDir, ".gitea", "template")
+	templateRelPath := filepath.Join(".gitea", "template")
-	content, err := readLocalTmpRepoFileContent(localPath, 1024*1024)
+	content, err := util.ReadRegularPathFile(tmpDir, templateRelPath, 1024*1024)
 	if err != nil {
-		return nil, err
+		return nil, util.Iif(errors.Is(err, util.ErrNotRegularPathFile), os.ErrNotExist, err)
 	}
-	return newGiteaTemplateFileMatcher(localPath, content), nil
+	return newGiteaTemplateFileMatcher(templateRelPath, content), nil
 }
 
 func substGiteaTemplateFile(ctx context.Context, tmpDir, tmpDirSubPath string, templateRepo, generateRepo *repo_model.Repository) error {
-	tmpFullPath := filepath.Join(tmpDir, tmpDirSubPath)
+	content, err := util.ReadRegularPathFile(tmpDir, tmpDirSubPath, 1024*1024)
-	content, err := readLocalTmpRepoFileContent(tmpFullPath, 1024*1024)
 	if err != nil {
-		return util.Iif(errors.Is(err, fs.ErrNotExist), nil, err)
+		if errors.Is(err, fs.ErrNotExist) {
+			return nil
+		}
+		return err
 	}
-	if err := util.Remove(tmpFullPath); err != nil {
+	if err := os.Remove(util.FilePathJoinAbs(tmpDir, tmpDirSubPath)); err != nil {
 		return err
 	}
 
 	generatedContent := generateExpansion(ctx, string(content), templateRepo, generateRepo)
 	substSubPath := filePathSanitize(generateExpansion(ctx, tmpDirSubPath, templateRepo, generateRepo))
-	newLocalPath := filepath.Join(tmpDir, substSubPath)
+	return util.WriteRegularPathFile(tmpDir, substSubPath, []byte(generatedContent), 0o755, 0o644)
-	regular, err := util.IsRegularFile(newLocalPath)
-	if canWrite := regular || errors.Is(err, fs.ErrNotExist); !canWrite {
-		return nil
-	}
-	if err := os.MkdirAll(filepath.Dir(newLocalPath), 0o755); err != nil {
-		return err
-	}
-	return os.WriteFile(newLocalPath, []byte(generatedContent), 0o644)
 }
 
-func processGiteaTemplateFile(ctx context.Context, tmpDir string, templateRepo, generateRepo *repo_model.Repository, fileMatcher *giteaTemplateFileMatcher) error {
+// processGiteaTemplateFile processes and removes the .gitea/template file, does variable expansion for template files
-	if err := util.Remove(fileMatcher.LocalFullPath); err != nil {
+// and save the processed files to the filesystem. It returns a list of skipped files that are not regular paths.
-		return fmt.Errorf("unable to remove .gitea/template: %w", err)
+func processGiteaTemplateFile(ctx context.Context, tmpDir string, templateRepo, generateRepo *repo_model.Repository, fileMatcher *giteaTemplateFileMatcher) (skippedFiles []string, _ error) {
+	// Why not use "os.Root" here: symlink is unsafe even in the same root but "os.Root" can't help, it's more difficult to use "os.Root" to do the WalkDir.
+	if err := os.Remove(util.FilePathJoinAbs(tmpDir, fileMatcher.relPath)); err != nil {
+		return nil, fmt.Errorf("unable to remove .gitea/template: %w", err)
 	}
 	if !fileMatcher.HasRules() {
-		return nil // Avoid walking tree if there are no globs
+		return skippedFiles, nil // Avoid walking tree if there are no globs
 	}
 
-	return filepath.WalkDir(tmpDir, func(fullPath string, d os.DirEntry, walkErr error) error {
+	err := filepath.WalkDir(tmpDir, func(fullPath string, d os.DirEntry, walkErr error) error {
 		if walkErr != nil {
 			return walkErr
 		}
@@ -208,10 +188,22 @@ func processGiteaTemplateFile(ctx context.Context, tmpDir string, templateRepo,
 			return err
 		}
 		if fileMatcher.Match(filepath.ToSlash(tmpDirSubPath)) {
-			return substGiteaTemplateFile(ctx, tmpDir, tmpDirSubPath, templateRepo, generateRepo)
+			err := substGiteaTemplateFile(ctx, tmpDir, tmpDirSubPath, templateRepo, generateRepo)
+			if errors.Is(err, util.ErrNotRegularPathFile) {
+				skippedFiles = append(skippedFiles, tmpDirSubPath)
+			} else if err != nil {
+				return err
+			}
 		}
 		return nil
 	}) // end: WalkDir
+	if err != nil {
+		return nil, err
+	}
+	if err = util.RemoveAll(util.FilePathJoinAbs(tmpDir, ".git")); err != nil {
+		return nil, err
+	}
+	return skippedFiles, nil
 }
 
 func generateRepoCommit(ctx context.Context, repo, templateRepo, generateRepo *repo_model.Repository, tmpDir string) error {
@@ -236,7 +228,7 @@ func generateRepoCommit(ctx context.Context, repo, templateRepo, generateRepo *r
 	// Variable expansion
 	fileMatcher, err := readGiteaTemplateFile(tmpDir)
 	if err == nil {
-		err = processGiteaTemplateFile(ctx, tmpDir, templateRepo, generateRepo, fileMatcher)
+		_, err = processGiteaTemplateFile(ctx, tmpDir, templateRepo, generateRepo, fileMatcher)
 		if err != nil {
 			return fmt.Errorf("processGiteaTemplateFile: %w", err)
 		}

services/repository/generate_test.go

@@ -74,7 +74,7 @@ func TestFilePathSanitize(t *testing.T) {
 	assert.Equal(t, ".", filePathSanitize("/"))
 }
 
-func TestProcessGiteaTemplateFile(t *testing.T) {
+func TestProcessGiteaTemplateFileGenerate(t *testing.T) {
 	tmpDir := filepath.Join(t.TempDir(), "gitea-template-test")
 
 	assertFileContent := func(path, expected string) {
@@ -97,6 +97,8 @@ func TestProcessGiteaTemplateFile(t *testing.T) {
 		assert.Equal(t, expected, link, "symlink target mismatch for %s", path)
 	}
 
+	require.NoError(t, os.MkdirAll(tmpDir+"/.git", 0o755))
+	require.NoError(t, os.WriteFile(tmpDir+"/.git/config", []byte("git-config-dummy"), 0o644))
 	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))
@@ -127,10 +129,20 @@ func TestProcessGiteaTemplateFile(t *testing.T) {
 		assertFileContent("subst-${TEMPLATE_NAME}-to-link", toLinkContent)
 		assertFileContent("subst-${TEMPLATE_NAME}-from-link", fromLinkContent)
 	}
+
+	// case-5
+	{
+		require.NoError(t, os.MkdirAll(tmpDir+"/real-dir", 0o755))
+		require.NoError(t, os.WriteFile(tmpDir+"/real-dir/real-file", []byte("origin content"), 0o644))
+		require.NoError(t, os.MkdirAll(tmpDir+"/include/subst-${TEMPLATE_NAME}-link-dir", 0o755))
+		require.NoError(t, os.WriteFile(tmpDir+"/include/subst-${TEMPLATE_NAME}-link-dir/real-file", []byte("template content"), 0o644))
+		require.NoError(t, os.Symlink(tmpDir+"/real-dir", tmpDir+"/include/subst-TemplateRepoName-link-dir"))
+	}
+
 	{
 		// 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
+		// will be skipped 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
@@ -143,9 +155,20 @@ func TestProcessGiteaTemplateFile(t *testing.T) {
 	{
 		templateRepo := &repo_model.Repository{Name: "TemplateRepoName"}
 		generatedRepo := &repo_model.Repository{Name: "/../.gIt/name"}
+		assertFileContent(".git/config", "git-config-dummy")
 		fileMatcher, _ := readGiteaTemplateFile(tmpDir)
-		err := processGiteaTemplateFile(t.Context(), tmpDir, templateRepo, generatedRepo, fileMatcher)
+		skippedFiles, err := processGiteaTemplateFile(t.Context(), tmpDir, templateRepo, generatedRepo, fileMatcher)
 		require.NoError(t, err)
+		assert.Equal(t, []string{
+			"include/subst-${TEMPLATE_NAME}-link-dir/real-file",
+			"include/subst-TemplateRepoName-link-dir",
+			"link",
+			"subst-${TEMPLATE_NAME}-from-link",
+			"subst-${TEMPLATE_NAME}-to-link",
+			"subst-TemplateRepoName-to-link",
+		}, skippedFiles)
+		assertFileContent(".git/config", "")
+		assertFileContent(".gitea/template", "")
 		assertFileContent("include/foo/bar/test.txt", "include subdir TemplateRepoName")
 	}
 
@@ -182,32 +205,38 @@ func TestProcessGiteaTemplateFile(t *testing.T) {
 		assertSymLink("subst-${TEMPLATE_NAME}-from-link", tmpDir+"/sub/link-target")
 	}
 
+	// case-5
 	{
-		templateFilePath := tmpDir + "/.gitea/template"
+		assertFileContent("real-dir/real-file", "origin content")
-
-		_ = os.Remove(templateFilePath)
-		_, err := os.Lstat(templateFilePath)
-		require.ErrorIs(t, err, fs.ErrNotExist)
-		_, err = readGiteaTemplateFile(tmpDir) // no template file
-		require.ErrorIs(t, err, fs.ErrNotExist)
-
-		_ = os.WriteFile(templateFilePath+".target", []byte("test-data-target"), 0o644)
-		_ = os.Symlink(templateFilePath+".target", templateFilePath)
-		content, _ := os.ReadFile(templateFilePath)
-		require.Equal(t, "test-data-target", string(content))
-		_, err = readGiteaTemplateFile(tmpDir) // symlinked template file
-		require.ErrorIs(t, err, fs.ErrNotExist)
-
-		_ = os.Remove(templateFilePath)
-		_ = os.WriteFile(templateFilePath, []byte("test-data-regular"), 0o644)
-		content, _ = os.ReadFile(templateFilePath)
-		require.Equal(t, "test-data-regular", string(content))
-		fm, err := readGiteaTemplateFile(tmpDir) // regular template file
-		require.NoError(t, err)
-		assert.Len(t, fm.globs, 1)
 	}
 }
 
+func TestProcessGiteaTemplateFileRead(t *testing.T) {
+	tmpDir := t.TempDir()
+	_ = os.Mkdir(tmpDir+"/.gitea", 0o755)
+	templateFilePath := tmpDir + "/.gitea/template"
+	_ = os.Remove(templateFilePath)
+	_, err := os.Lstat(templateFilePath)
+	require.ErrorIs(t, err, fs.ErrNotExist)
+	_, err = readGiteaTemplateFile(tmpDir) // no template file
+	require.ErrorIs(t, err, fs.ErrNotExist)
+
+	_ = os.WriteFile(templateFilePath+".target", []byte("test-data-target"), 0o644)
+	_ = os.Symlink(templateFilePath+".target", templateFilePath)
+	content, _ := os.ReadFile(templateFilePath)
+	require.Equal(t, "test-data-target", string(content))
+	_, err = readGiteaTemplateFile(tmpDir) // symlinked template file
+	require.ErrorIs(t, err, fs.ErrNotExist)
+
+	_ = os.Remove(templateFilePath)
+	_ = os.WriteFile(templateFilePath, []byte("test-data-regular"), 0o644)
+	content, _ = os.ReadFile(templateFilePath)
+	require.Equal(t, "test-data-regular", string(content))
+	fm, err := readGiteaTemplateFile(tmpDir) // regular template file
+	require.NoError(t, err)
+	assert.Len(t, fm.globs, 1)
+}
+
 func TestTransformers(t *testing.T) {
 	cases := []struct {
 		name     string