diff --git a/controllers/storage.go b/controllers/storage.go index 5db743d7..de46a069 100644 --- a/controllers/storage.go +++ b/controllers/storage.go @@ -29,6 +29,7 @@ import ( "strings" "time" + securejoin "github.com/cyphar/filepath-securejoin" "github.com/go-git/go-git/v5/plumbing/format/gitignore" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -87,8 +88,7 @@ func (s Storage) SetArtifactURL(artifact *sourcev1.Artifact) { artifact.URL = fmt.Sprintf(format, s.Hostname, strings.TrimLeft(artifact.Path, "/")) } -// SetHostname sets the hostname of the given URL string to the current Storage.Hostname -// and returns the result. +// SetHostname sets the hostname of the given URL string to the current Storage.Hostname and returns the result. func (s Storage) SetHostname(URL string) string { u, err := url.Parse(URL) if err != nil { @@ -110,8 +110,7 @@ func (s *Storage) RemoveAll(artifact sourcev1.Artifact) error { return os.RemoveAll(dir) } -// RemoveAllButCurrent removes all files for the given v1beta1.Artifact base dir, -// excluding the current one. +// RemoveAllButCurrent removes all files for the given v1beta1.Artifact base dir, excluding the current one. func (s *Storage) RemoveAllButCurrent(artifact sourcev1.Artifact) error { localPath := s.LocalPath(artifact) dir := filepath.Dir(localPath) @@ -136,8 +135,7 @@ func (s *Storage) RemoveAllButCurrent(artifact sourcev1.Artifact) error { return nil } -// ArtifactExist returns a boolean indicating whether the v1beta1.Artifact exists in storage -// and is a regular file. +// ArtifactExist returns a boolean indicating whether the v1beta1.Artifact exists in storage and is a regular file. func (s *Storage) ArtifactExist(artifact sourcev1.Artifact) bool { fi, err := os.Lstat(s.LocalPath(artifact)) if err != nil { @@ -146,14 +144,13 @@ func (s *Storage) ArtifactExist(artifact sourcev1.Artifact) bool { return fi.Mode().IsRegular() } -// ArchiveFileFilter must return true if a file should not be included -// in the archive after inspecting the given path and/or os.FileInfo. +// ArchiveFileFilter must return true if a file should not be included in the archive after inspecting the given path +// and/or os.FileInfo. type ArchiveFileFilter func(p string, fi os.FileInfo) bool -// SourceIgnoreFilter returns an ArchiveFileFilter that filters out -// files matching sourceignore.VCSPatterns and any of the provided -// patterns. If an empty gitignore.Pattern slice is given, the matcher -// is set to sourceignore.NewDefaultMatcher. +// SourceIgnoreFilter returns an ArchiveFileFilter that filters out files matching sourceignore.VCSPatterns and any of +// the provided patterns. +// If an empty gitignore.Pattern slice is given, the matcher is set to sourceignore.NewDefaultMatcher. func SourceIgnoreFilter(ps []gitignore.Pattern, domain []string) ArchiveFileFilter { matcher := sourceignore.NewDefaultMatcher(ps, domain) if len(ps) > 0 { @@ -167,10 +164,9 @@ func SourceIgnoreFilter(ps []gitignore.Pattern, domain []string) ArchiveFileFilt } } -// Archive atomically archives the given directory as a tarball to the -// given v1beta1.Artifact path, excluding directories and any -// ArchiveFileFilter matches. If successful, it sets the checksum and -// last update time on the artifact. +// Archive atomically archives the given directory as a tarball to the given v1beta1.Artifact path, excluding +// directories and any ArchiveFileFilter matches. +// If successful, it sets the checksum and last update time on the artifact. func (s *Storage) Archive(artifact *sourcev1.Artifact, dir string, filter ArchiveFileFilter) (err error) { if f, err := os.Stat(dir); os.IsNotExist(err) || !f.IsDir() { return fmt.Errorf("invalid dir path: %s", dir) @@ -345,9 +341,8 @@ func (s *Storage) Copy(artifact *sourcev1.Artifact, reader io.Reader) (err error return nil } -// CopyFromPath atomically copies the contents of the given path to the path of -// the v1beta1.Artifact. If successful, the checksum and last update time on the -// artifact is set. +// CopyFromPath atomically copies the contents of the given path to the path of the v1beta1.Artifact. +// If successful, the checksum and last update time on the artifact is set. func (s *Storage) CopyFromPath(artifact *sourcev1.Artifact, path string) (err error) { f, err := os.Open(path) if err != nil { @@ -357,10 +352,10 @@ func (s *Storage) CopyFromPath(artifact *sourcev1.Artifact, path string) (err er return s.Copy(artifact, f) } -// CopyToPath copies the contents of the given artifact to the path. +// CopyToPath copies the contents in the (sub)path of the given artifact to the given path. func (s *Storage) CopyToPath(artifact *sourcev1.Artifact, subPath, toPath string) error { // create a tmp directory to store artifact - tmp, err := os.MkdirTemp("", "flux-include") + tmp, err := os.MkdirTemp("", "flux-include-") if err != nil { return err } @@ -375,7 +370,7 @@ func (s *Storage) CopyToPath(artifact *sourcev1.Artifact, subPath, toPath string defer f.Close() // untar the artifact - untarPath := filepath.Join(tmp, "tar") + untarPath := filepath.Join(tmp, "unpack") if _, err = untar.Untar(f, untarPath); err != nil { return err } @@ -386,15 +381,17 @@ func (s *Storage) CopyToPath(artifact *sourcev1.Artifact, subPath, toPath string } // copy the artifact content to the destination dir - fromPath := filepath.Join(untarPath, subPath) + fromPath, err := securejoin.SecureJoin(untarPath, subPath) + if err != nil { + return err + } if err := fs.RenameWithFallback(fromPath, toPath); err != nil { return err } return nil } -// Symlink creates or updates a symbolic link for the given v1beta1.Artifact -// and returns the URL for the symlink. +// Symlink creates or updates a symbolic link for the given v1beta1.Artifact and returns the URL for the symlink. func (s *Storage) Symlink(artifact sourcev1.Artifact, linkName string) (string, error) { localPath := s.LocalPath(artifact) dir := filepath.Dir(localPath) @@ -431,13 +428,16 @@ func (s *Storage) Lock(artifact sourcev1.Artifact) (unlock func(), err error) { return mutex.Lock() } -// LocalPath returns the local path of the given artifact (that is: relative to -// the Storage.BasePath). +// LocalPath returns the secure local path of the given artifact (that is: relative to the Storage.BasePath). func (s *Storage) LocalPath(artifact sourcev1.Artifact) string { if artifact.Path == "" { return "" } - return filepath.Join(s.BasePath, artifact.Path) + path, err := securejoin.SecureJoin(s.BasePath, artifact.Path) + if err != nil { + return "" + } + return path } // newHash returns a new SHA1 hash.