Don't skip symlinks during local source upload

docs/shp_build_upload.md

@@ -17,8 +17,8 @@ In case a source bundle image is defined, the bundling feature is used, which wi
 source code into a bundle container and upload it to the specified container registry. Instead of
 executing using Git in the source step, it will use the container registry to obtain the source code.
 
-	$ shp buildrun upload <build-name>
-	$ shp buildrun upload <build-name> /path/to/repository
+	$ shp build upload <build-name>
+	$ shp build upload <build-name> /path/to/repository
 
 
 ```

pkg/shp/cmd/build/upload.go

@@ -56,8 +56,8 @@ In case a source bundle image is defined, the bundling feature is used, which wi
 source code into a bundle container and upload it to the specified container registry. Instead of
 executing using Git in the source step, it will use the container registry to obtain the source code.
 
-	$ shp buildrun upload <build-name>
-	$ shp buildrun upload <build-name> /path/to/repository
+	$ shp build upload <build-name>
+	$ shp build upload <build-name> /path/to/repository
 `
 
 	// targetBaseDir directory where data will be uploaded.
@@ -211,7 +211,7 @@ func (u *UploadCommand) performDataStreaming(target *streamer.Target) error {
 
 	fmt.Fprintf(u.ioStreams.Out, "Streaming %q to the Build POD %q ...\n", u.sourceDir, target.Pod)
 	// creates an in-memory tarball with source directory data, and ready to start data streaming
-	tarball, err := streamer.NewTar(u.sourceDir)
+	tarball, err := streamer.NewTar(u.sourceDir, u.ioStreams)
 	if err != nil {
 		return err
 	}

pkg/shp/streamer/tar.go

@@ -10,18 +10,20 @@ import (
 	"strings"
 
 	ignore "github.com/sabhiram/go-gitignore"
+	"k8s.io/cli-runtime/pkg/genericclioptions"
 )
 
 // Tar helper to create a tar instance based on a source directory, skipping entries that are not
 // desired like `.git` directory and entries in `.gitignore` file.
 type Tar struct {
-	src       string            // base directory
-	gitIgnore *ignore.GitIgnore // matcher for git ignored files
+	src       string                       // base directory
+	gitIgnore *ignore.GitIgnore            // matcher for git ignored files
+	ioStreams *genericclioptions.IOStreams // io streams for user-facing output
 }
 
 // skipPath inspect each path and makes sure it skips files the tar helper can't handle.
 func (t *Tar) skipPath(fpath string, stat fs.FileInfo) bool {
-	if !stat.Mode().IsRegular() {
+	if !stat.Mode().IsRegular() && stat.Mode()&fs.ModeSymlink == 0 {
 		return true
 	}
 	if strings.HasPrefix(fpath, path.Join(t.src, ".git")) {
@@ -43,7 +45,7 @@ func (t *Tar) Create(w io.Writer) error {
 		if t.skipPath(fpath, stat) {
 			return nil
 		}
-		return writeFileToTar(tw, t.src, fpath, stat)
+		return writeFileToTar(tw, t, fpath, stat)
 	}); err != nil {
 		return err
 	}
@@ -63,8 +65,8 @@ func (t *Tar) bootstrap() error {
 }
 
 // NewTar instantiate a tar helper based on the source directory path informed.
-func NewTar(src string) (*Tar, error) {
-	t := &Tar{src: src}
+func NewTar(src string, ioStreams *genericclioptions.IOStreams) (*Tar, error) {
+	t := &Tar{src: src, ioStreams: ioStreams}
 	return t, t.bootstrap()
 }
 

pkg/shp/streamer/tar_test.go

@@ -3,16 +3,38 @@
 import (
 	"archive/tar"
 	"io"
+	"os"
 	"strings"
 	"testing"
 
 	o "github.com/onsi/gomega"
+	"k8s.io/cli-runtime/pkg/genericclioptions"
 )
 
 func Test_Tar(t *testing.T) {
 	g := o.NewGomegaWithT(t)
 
-	tarHelper, err := NewTar("../../..")
+	ioStreams, _, _, errOut := genericclioptions.NewTestIOStreams()
+
+	baseDir := "../../.."
+
+	symlinks := []struct {
+		name   string
+		target string
+	}{
+		{name: "symlink_inside_tree", target: "../test"},
+		{name: "symlink_outside_tree", target: "../../foobar/test"},
+	}
+
+	for _, symlink := range symlinks {
+		path := baseDir + "/test/" + symlink.name
+		if err := os.Symlink(symlink.target, path); err != nil {
+			t.Fatalf("Couldn't setup test symlink: %v", err)
+		}
+		t.Cleanup(func() { os.Remove(path) })
+	}
+
+	tarHelper, err := NewTar(baseDir, &ioStreams)
 	g.Expect(err).To(o.BeNil())
 
 	reader, writer := io.Pipe()
@@ -25,7 +47,7 @@
 	}()
 
 	tarReader := tar.NewReader(reader)
-	counter := 0
+	counter, symlink := 0, 0
 	for {
 		header, err := tarReader.Next()
 		if err != nil {
@@ -37,10 +59,19 @@
 		counter++
 		name := header.Name
 
-		// making sure that undesired entries are not present on the list of files caputured by the
+		if header.Linkname != "" {
+			symlink++
+		}
+
+		// making sure that undesired entries are not present on the list of files captured by the
 		// tar helper
 		g.Expect(strings.HasPrefix(name, ".git/")).To(o.BeFalse())
 		g.Expect(strings.HasPrefix(name, "_output/")).To(o.BeFalse())
 	}
 	g.Expect(counter > 10).To(o.BeTrue())
+
+	stdErr := errOut.String()
+
+	g.Expect(symlink > 1).To(o.BeTrue())
+	g.Expect(stdErr).To(o.ContainSubstring("points outside the source directory"))
 }

pkg/shp/streamer/util.go

@@ -2,6 +2,7 @@ package streamer
 
 import (
 	"archive/tar"
+	"fmt"
 	"io"
 	"io/fs"
 	"os"
@@ -21,24 +22,76 @@ func trimPrefix(prefix, fpath string) string {
 	return strings.TrimPrefix(strings.ReplaceAll(fpath, prefix, ""), string(filepath.Separator))
 }
 
-func writeFileToTar(tw *tar.Writer, src, fpath string, stat fs.FileInfo) error {
+func writeFileToTar(tw *tar.Writer, t *Tar, fpath string, stat fs.FileInfo) error {
 	header, err := tar.FileInfoHeader(stat, stat.Name())
 	if err != nil {
 		return err
 	}
 
-	header.Name = trimPrefix(src, fpath)
+	header.Name = trimPrefix(t.src, fpath)
+
+	// Symlink
+	if stat.Mode()&fs.ModeSymlink != 0 {
+		target, err := os.Readlink(fpath)
+		if err != nil {
+			return err
+		}
+
+		// resolving target to absolute path, to determine if it is pointing
+		// outside the source directory.
+		var absSrc, absTarget string
+
+		if absSrc, err = filepath.Abs(t.src); err != nil {
+			return err
+		}
+
+		if filepath.IsAbs(target) {
+			absTarget = target
+		} else {
+			fullTarget := filepath.Join(filepath.Dir(fpath), target)
+			if absTarget, err = filepath.Abs(fullTarget); err != nil {
+				return err
+			}
+		}
+
+		if outside, _ := isSymlinkTargetOutsideOfDir(absSrc, absTarget); outside {
+			relPath, _ := filepath.Rel(t.src, fpath)
+			fmt.Fprintf(t.ioStreams.ErrOut, "WARNING: symlink %q points outside the source directory %q (target: %q)\n", relPath, absSrc, target)
+		}
+
+		header.Linkname = target
+	}
+
 	if err := tw.WriteHeader(header); err != nil {
 		return err
 	}
 
-	// #nosec G304 intentionally opening file from variable
-	f, err := os.Open(fpath)
+	// Copy regular file content
+	if stat.Mode().IsRegular() {
+		// #nosec G304 intentionally opening file from variable
+		f, err := os.Open(fpath)
+		if err != nil {
+			return err
+		}
+		if _, err := io.Copy(tw, f); err != nil {
+			return err
+		}
+		return f.Close()
+	}
+
+	return nil
+}
+
+func isSymlinkTargetOutsideOfDir(dir, target string) (bool, error) {
+	realTarget, err := filepath.EvalSymlinks(target)
 	if err != nil {
-		return err
+		realTarget = filepath.Clean(target)
 	}
-	if _, err := io.Copy(tw, f); err != nil {
-		return err
+
+	rel, err := filepath.Rel(dir, realTarget)
+	if err != nil {
+		return false, err
 	}
-	return f.Close()
+
+	return !filepath.IsLocal(rel), nil
 }