From 2176e84ab977011ff2bc3f3a9066020cc674f6b1 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 25 Feb 2026 09:21:07 +0800 Subject: [PATCH] Fix path resolving (#36734) --- modules/util/path.go | 78 +++++++++++++++++++++++--- modules/util/path_test.go | 68 +++++++++++++++++++++++ services/repository/generate.go | 82 +++++++++++++--------------- services/repository/generate_test.go | 79 ++++++++++++++++++--------- 4 files changed, 228 insertions(+), 79 deletions(-) diff --git a/modules/util/path.go b/modules/util/path.go index 0cb8ab7ece..48447d7b90 100644 --- a/modules/util/path.go +++ b/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) { - f, err := os.Lstat(filePath) - if err == nil { - return f.Mode().IsRegular(), nil +var ErrNotRegularPathFile = errors.New("not a regular file") + +// ReadRegularPathFile reads a file with given sub path in root dir. +// 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. diff --git a/modules/util/path_test.go b/modules/util/path_test.go index 79c37e55f7..2469088b3a 100644 --- a/modules/util/path_test.go +++ b/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") + }) +} diff --git a/services/repository/generate.go b/services/repository/generate.go index bc37bc7bfe..83e9c22e54 100644 --- a/services/repository/generate.go +++ b/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 - globs []glob.Glob + relPath string + globs []glob.Glob } -func newGiteaTemplateFileMatcher(fullPath string, content []byte) *giteaTemplateFileMatcher { - gt := &giteaTemplateFileMatcher{LocalFullPath: fullPath} +func newGiteaTemplateFileMatcher(relPath string, content []byte) *giteaTemplateFileMatcher { + 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") - content, err := readLocalTmpRepoFileContent(localPath, 1024*1024) + templateRelPath := filepath.Join(".gitea", "template") + 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 := readLocalTmpRepoFileContent(tmpFullPath, 1024*1024) + content, err := util.ReadRegularPathFile(tmpDir, tmpDirSubPath, 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) - 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) + return util.WriteRegularPathFile(tmpDir, substSubPath, []byte(generatedContent), 0o755, 0o644) } -func processGiteaTemplateFile(ctx context.Context, tmpDir string, templateRepo, generateRepo *repo_model.Repository, fileMatcher *giteaTemplateFileMatcher) error { - if err := util.Remove(fileMatcher.LocalFullPath); err != nil { - return fmt.Errorf("unable to remove .gitea/template: %w", err) +// processGiteaTemplateFile processes and removes the .gitea/template file, does variable expansion for template files +// and save the processed files to the filesystem. It returns a list of skipped files that are not regular paths. +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) } diff --git a/services/repository/generate_test.go b/services/repository/generate_test.go index 432de4dc59..160dbb9a06 100644 --- a/services/repository/generate_test.go +++ b/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" - - _ = 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) + assertFileContent("real-dir/real-file", "origin content") } } +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