diff --git a/internal/files/linkedfiles.go b/internal/files/linkedfiles.go index fd46fa50ea..597999b50f 100644 --- a/internal/files/linkedfiles.go +++ b/internal/files/linkedfiles.go @@ -40,7 +40,12 @@ func CreateLinksFSFromPath(workDir string) (*LinksFS, error) { return nil, fmt.Errorf("opening repository root: %w", err) } - return NewLinksFS(root, workDir) + absWorkDir, err := filepath.Abs(workDir) + if err != nil { + return nil, fmt.Errorf("obtaining absolute path of working directory: %w", err) + } + + return NewLinksFS(root, absWorkDir) } var _ fs.FS = (*LinksFS)(nil) @@ -56,39 +61,28 @@ type LinksFS struct { inner fs.FS } -// NewLinksFS creates a new LinksFS. +// NewLinksFS creates a new LinksFS. workDir must be an absolute path, or a path relative to +// the repository root. func NewLinksFS(repoRoot *os.Root, workDir string) (*LinksFS, error) { // Ensure workDir is absolute for os.DirFS var absWorkDir string if filepath.IsAbs(workDir) { absWorkDir = workDir - } else { - // First try: assume workDir is relative to the repository root - absWorkDir = filepath.Join(repoRoot.Name(), workDir) - - // Check if path exists - if _, err := os.Stat(absWorkDir); os.IsNotExist(err) { - // Second try: path might be relative to current working directory - currentDir, err := os.Getwd() - if err == nil { - alternativePath := filepath.Join(currentDir, workDir) - if _, err := os.Stat(alternativePath); err == nil { - // If this path exists, use it instead - absWorkDir = alternativePath - logger.Debugf("Using path relative to current working directory: %s", absWorkDir) - } - } + relative, err := filepath.Rel(repoRoot.Name(), absWorkDir) + if err != nil { + return nil, fmt.Errorf("invalid working directory %s: %w", absWorkDir, err) } + workDir = relative + } else { + absWorkDir = filepath.Clean(filepath.Join(repoRoot.Name(), workDir)) } - // Validate that workDir is within the repository root - inRoot, err := pathIsInRepositoryRoot(repoRoot, absWorkDir) + info, err := repoRoot.Stat(workDir) if err != nil { - return nil, fmt.Errorf("could not validate workDir %s: %w", absWorkDir, err) + return nil, fmt.Errorf("invalid working directory %s: %w", absWorkDir, err) } - - if !inRoot { - return nil, fmt.Errorf("workDir %s is outside the repository root %s", absWorkDir, repoRoot.Name()) + if !info.IsDir() { + return nil, fmt.Errorf("working directory %s is not a directory", absWorkDir) } return &LinksFS{repoRoot: repoRoot, workDir: absWorkDir, inner: os.DirFS(absWorkDir)}, nil @@ -221,14 +215,6 @@ func newLinkedFile(root *os.Root, linkFilePath string) (Link, error) { pathName := filepath.Clean(filepath.Join(l.WorkDir, filepath.FromSlash(l.IncludedFilePath))) - inRoot, err := pathIsInRepositoryRoot(root, pathName) - if err != nil { - return Link{}, fmt.Errorf("could not check if path %s is in repository root: %w", pathName, err) - } - if !inRoot { - return Link{}, fmt.Errorf("path %s escapes the repository root", pathName) - } - // Store the original absolute path for package root detection originalAbsPath := pathName @@ -545,28 +531,3 @@ func checksum(b []byte) (string, error) { hash := sha256.Sum256(b) return hex.EncodeToString(hash[:]), nil } - -// pathIsInRepositoryRoot checks if a path is within the repository root and doesn't escape it. -func pathIsInRepositoryRoot(root *os.Root, path string) (bool, error) { - path = filepath.FromSlash(path) - var err error - if filepath.IsAbs(path) { - path, err = filepath.Rel(root.Name(), path) - if err != nil { - return false, fmt.Errorf("could not get relative path: %w", err) - } - } - - // Clean the path to resolve any ".." components - cleanPath := filepath.Clean(path) - - // Check if the cleaned path tries to escape the root - if strings.HasPrefix(cleanPath, "..") { - return false, nil - } - - if _, err := root.Stat(cleanPath); err != nil { - return false, nil - } - return true, nil -} diff --git a/internal/files/linkedfiles_test.go b/internal/files/linkedfiles_test.go index 2ab166e9c6..f057f955ca 100644 --- a/internal/files/linkedfiles_test.go +++ b/internal/files/linkedfiles_test.go @@ -12,6 +12,7 @@ import ( "io" "os" "path/filepath" + "runtime" "strings" "testing" @@ -33,7 +34,7 @@ func TestLinkUpdateChecksum(t *testing.T) { require.NoError(t, copyDir(testDataSrc, filepath.Join(tempDir, "testdata"))) // Set up paths within the temporary directory - basePath := filepath.Join(tempDir, "testdata/links") + basePath := filepath.Join(tempDir, "testdata", "links") // Create an os.Root for secure file operations within tempDir root, err := os.OpenRoot(tempDir) @@ -82,7 +83,7 @@ func TestListLinkedFiles(t *testing.T) { // Get current working directory to locate test data wd, err := os.Getwd() assert.NoError(t, err) - basePath := filepath.Join(wd, filepath.FromSlash("testdata/links")) + basePath := filepath.Join(wd, "testdata", "links") // Find the repository root to create a secure os.Root context root, err := FindRepositoryRoot() @@ -193,7 +194,7 @@ func TestUpdateLinkedFilesChecksums(t *testing.T) { require.NoError(t, copyDir(testDataSrc, filepath.Join(tempDir, "testdata"))) // Set up paths within the temporary directory - basePath := filepath.Join(tempDir, "testdata/links") + basePath := filepath.Join(tempDir, "testdata", "links") // Create an os.Root for secure file operations within tempDir root, err := os.OpenRoot(tempDir) @@ -227,7 +228,7 @@ func TestLinkedFilesByPackageFrom(t *testing.T) { // Get current working directory to locate test data wd, err := os.Getwd() assert.NoError(t, err) - basePath := filepath.Join(wd, filepath.FromSlash("testdata/links")) + basePath := filepath.Join(wd, "testdata", "links") // Find the repository root to create a secure os.Root context root, err := FindRepositoryRoot() @@ -269,14 +270,14 @@ func TestIncludeLinkedFiles(t *testing.T) { // Get current working directory to locate test data wd, err := os.Getwd() assert.NoError(t, err) - testPkg := filepath.Join(wd, filepath.FromSlash("testdata")) + testPkg := filepath.Join(wd, "testdata") // Create a temporary directory and copy test data to avoid modifying originals tempDir := t.TempDir() require.NoError(t, copyDir(testPkg, filepath.Join(tempDir, "testdata"))) // Set up source and destination directories - fromDir := filepath.Join(tempDir, "testdata/testpackage") + fromDir := filepath.Join(tempDir, "testdata", "testpackage") toDir := filepath.Join(tempDir, "dest") // Create an os.Root for secure file operations within tempDir @@ -769,6 +770,11 @@ func TestLinksFS_ErrorConditions(t *testing.T) { err = os.WriteFile(invalidLinkFile, []byte(""), 0644) require.NoError(t, err) + // Create link that escapes root + outOfRootLinkFile := filepath.Join(workDir, "escapesroot.txt.link") + err = os.WriteFile(outOfRootLinkFile, []byte("../../etc/passwd"), 0644) + require.NoError(t, err) + // Setup LinksFS root, err := os.OpenRoot(repoDir) require.NoError(t, err) @@ -777,6 +783,11 @@ func TestLinksFS_ErrorConditions(t *testing.T) { lfs, err := NewLinksFS(root, workDir) require.NoError(t, err) + notFoundErrorMsg := "no such file or directory" + if runtime.GOOS == "windows" { + notFoundErrorMsg = "The system cannot find the file specified" + } + tests := []struct { name string fileName string @@ -785,13 +796,18 @@ func TestLinksFS_ErrorConditions(t *testing.T) { { name: "broken link to non-existent file", fileName: "broken.txt.link", - errorMsg: "escapes the repository root", + errorMsg: notFoundErrorMsg, }, { name: "invalid link file format", fileName: "invalid.txt.link", errorMsg: "file is empty or first line is missing", }, + { + name: "escapes root", + fileName: "escapesroot.txt.link", + errorMsg: "path escapes from parent", + }, } for _, tc := range tests { @@ -845,13 +861,13 @@ func TestLinksFS_WorkDirValidation(t *testing.T) { name: "invalid absolute workDir outside repo", workDir: outsideDir, expectError: true, - errorMsg: "is outside the repository root", + errorMsg: "path escapes from parent", }, { name: "invalid relative workDir escaping repo", workDir: "../outside", expectError: true, - errorMsg: "is outside the repository root", + errorMsg: "path escapes from parent", }, }