Merge pull request containers#16315 from flouthoc/remote-ignore-symlink

remote,build: ignore if `.containerignore` or `.dockerignore` is a symlink outside of buildContext

pkg/api/handlers/compat/images_build.go

@@ -24,6 +24,7 @@ import (
 	"github.com/containers/podman/v4/pkg/auth"
 	"github.com/containers/podman/v4/pkg/channel"
 	"github.com/containers/podman/v4/pkg/rootless"
+	"github.com/containers/podman/v4/pkg/util"
 	"github.com/containers/storage/pkg/archive"
 	"github.com/docker/docker/pkg/jsonmessage"
 	"github.com/gorilla/schema"
@@ -625,6 +626,12 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
 	reporter := channel.NewWriter(make(chan []byte))
 	defer reporter.Close()
 
+	_, ignoreFile, err := util.ParseDockerignore(containerFiles, contextDirectory)
+	if err != nil {
+		utils.Error(w, http.StatusInternalServerError, fmt.Errorf("processing ignore file: %w", err))
+		return
+	}
+
 	runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime)
 	buildOptions := buildahDefine.BuildOptions{
 		AddCapabilities:         addCaps,
@@ -674,6 +681,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
 		From:                           fromImage,
 		IDMappingOptions:               &idMappingOptions,
 		IgnoreUnrecognizedInstructions: query.Ignore,
+		IgnoreFile:                     ignoreFile,
 		Isolation:                      isolation,
 		Jobs:                           &jobs,
 		Labels:                         labels,

pkg/bindings/images/build.go

@@ -22,6 +22,7 @@ import (
 	"github.com/containers/podman/v4/pkg/auth"
 	"github.com/containers/podman/v4/pkg/bindings"
 	"github.com/containers/podman/v4/pkg/domain/entities"
+	"github.com/containers/podman/v4/pkg/util"
 	"github.com/containers/storage/pkg/fileutils"
 	"github.com/containers/storage/pkg/ioutils"
 	"github.com/containers/storage/pkg/regexp"
@@ -405,14 +406,6 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO
 		stdout = options.Out
 	}
 
-	excludes := options.Excludes
-	if len(excludes) == 0 {
-		excludes, err = parseDockerignore(options.ContextDirectory)
-		if err != nil {
-			return nil, err
-		}
-	}
-
 	contextDir, err = filepath.Abs(options.ContextDirectory)
 	if err != nil {
 		logrus.Errorf("Cannot find absolute path of %v: %v", options.ContextDirectory, err)
@@ -458,20 +451,26 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO
 		if strings.HasPrefix(containerfile, contextDir+string(filepath.Separator)) {
 			containerfile = strings.TrimPrefix(containerfile, contextDir+string(filepath.Separator))
 			dontexcludes = append(dontexcludes, "!"+containerfile)
+			dontexcludes = append(dontexcludes, "!"+containerfile+".dockerignore")
+			dontexcludes = append(dontexcludes, "!"+containerfile+".containerignore")
 		} else {
 			// If Containerfile does not exist, assume it is in context directory and do Not add to tarfile
 			if _, err := os.Lstat(containerfile); err != nil {
 				if !os.IsNotExist(err) {
 					return nil, err
 				}
 				containerfile = c
+				dontexcludes = append(dontexcludes, "!"+containerfile)
+				dontexcludes = append(dontexcludes, "!"+containerfile+".dockerignore")
+				dontexcludes = append(dontexcludes, "!"+containerfile+".containerignore")
 			} else {
 				// If Containerfile does exist and not in the context directory, add it to the tarfile
 				tarContent = append(tarContent, containerfile)
 			}
 		}
 		newContainerFiles = append(newContainerFiles, filepath.ToSlash(containerfile))
 	}
+
 	if len(newContainerFiles) > 0 {
 		cFileJSON, err := json.Marshal(newContainerFiles)
 		if err != nil {
@@ -480,6 +479,14 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO
 		params.Set("dockerfile", string(cFileJSON))
 	}
 
+	excludes := options.Excludes
+	if len(excludes) == 0 {
+		excludes, _, err = util.ParseDockerignore(newContainerFiles, options.ContextDirectory)
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	// build secrets are usually absolute host path or relative to context dir on host
 	// in any case move secret to current context and ship the tar.
 	if secrets := options.CommonBuildOpts.Secrets; len(secrets) > 0 {
@@ -769,23 +776,3 @@ func nTar(excludes []string, sources ...string) (io.ReadCloser, error) {
 	})
 	return rc, nil
 }
-
-func parseDockerignore(root string) ([]string, error) {
-	ignore, err := os.ReadFile(filepath.Join(root, ".containerignore"))
-	if err != nil {
-		var dockerIgnoreErr error
-		ignore, dockerIgnoreErr = os.ReadFile(filepath.Join(root, ".dockerignore"))
-		if dockerIgnoreErr != nil && !os.IsNotExist(dockerIgnoreErr) {
-			return nil, err
-		}
-	}
-	rawexcludes := strings.Split(string(ignore), "\n")
-	excludes := make([]string, 0, len(rawexcludes))
-	for _, e := range rawexcludes {
-		if len(e) == 0 || e[0] == '#' {
-			continue
-		}
-		excludes = append(excludes, e)
-	}
-	return excludes, nil
-}

pkg/util/utils.go

@@ -27,6 +27,7 @@ import (
 	"github.com/containers/podman/v4/pkg/signal"
 	"github.com/containers/storage/pkg/idtools"
 	stypes "github.com/containers/storage/types"
+	securejoin "github.com/cyphar/filepath-securejoin"
 	"github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/term"
@@ -56,6 +57,76 @@ func parseCreds(creds string) (string, string) {
 	return up[0], up[1]
 }
 
+// Takes build context and validates `.containerignore` or `.dockerignore`
+// if they are symlink outside of buildcontext. Returns list of files to be
+// excluded and resolved path to the ignore files inside build context or error
+func ParseDockerignore(containerfiles []string, root string) ([]string, string, error) {
+	ignoreFile := ""
+	path, err := securejoin.SecureJoin(root, ".containerignore")
+	if err != nil {
+		return nil, ignoreFile, err
+	}
+	// set resolved ignore file so imagebuildah
+	// does not attempts to re-resolve it
+	ignoreFile = path
+	ignore, err := os.ReadFile(path)
+	if err != nil {
+		var dockerIgnoreErr error
+		path, symlinkErr := securejoin.SecureJoin(root, ".dockerignore")
+		if symlinkErr != nil {
+			return nil, ignoreFile, symlinkErr
+		}
+		// set resolved ignore file so imagebuildah
+		// does not attempts to re-resolve it
+		ignoreFile = path
+		ignore, dockerIgnoreErr = os.ReadFile(path)
+		if os.IsNotExist(dockerIgnoreErr) {
+			// In this case either ignorefile was not found
+			// or it is a symlink to unexpected file in such
+			// case manually set ignorefile to `/dev/null` so
+			// internally imagebuildah does not attempts to re-resolve
+			// this invalid symlink and instead reads a blank file.
+			ignoreFile = "/dev/null"
+		}
+		// after https://github.com/containers/buildah/pull/4239 build supports
+		// <Containerfile>.containerignore or <Containerfile>.dockerignore as ignore file
+		// so remote must support parsing that.
+		if dockerIgnoreErr != nil {
+			for _, containerfile := range containerfiles {
+				if _, err := os.Stat(filepath.Join(root, containerfile+".containerignore")); err == nil {
+					path, symlinkErr = securejoin.SecureJoin(root, containerfile+".containerignore")
+					if symlinkErr == nil {
+						ignoreFile = path
+						ignore, dockerIgnoreErr = os.ReadFile(path)
+					}
+				}
+				if _, err := os.Stat(filepath.Join(root, containerfile+".dockerignore")); err == nil {
+					path, symlinkErr = securejoin.SecureJoin(root, containerfile+".dockerignore")
+					if symlinkErr == nil {
+						ignoreFile = path
+						ignore, dockerIgnoreErr = os.ReadFile(path)
+					}
+				}
+				if dockerIgnoreErr == nil {
+					break
+				}
+			}
+		}
+		if dockerIgnoreErr != nil && !os.IsNotExist(dockerIgnoreErr) {
+			return nil, ignoreFile, err
+		}
+	}
+	rawexcludes := strings.Split(string(ignore), "\n")
+	excludes := make([]string, 0, len(rawexcludes))
+	for _, e := range rawexcludes {
+		if len(e) == 0 || e[0] == '#' {
+			continue
+		}
+		excludes = append(excludes, e)
+	}
+	return excludes, ignoreFile, nil
+}
+
 // ParseRegistryCreds takes a credentials string in the form USERNAME:PASSWORD
 // and returns a DockerAuthConfig
 func ParseRegistryCreds(creds string) (*types.DockerAuthConfig, error) {

test/buildah-bud/apply-podman-deltas

@@ -245,7 +245,8 @@ skip_if_remote "Explicit request in buildah PR 4190 to skip this on remote" \
 # BEGIN tests which are skipped due to actual podman or podman-remote bugs.
 
 skip_if_remote "different error messages between podman & podman-remote" \
-               "bud with .dockerignore #2"
+               "bud with .dockerignore #2" \
+               "bud with .dockerignore #4"
 
 # END   tests which are skipped due to actual podman or podman-remote bugs.
 ###############################################################################

test/e2e/build/containerignore-symlink/.dockerignore

@@ -0,0 +1 @@
+/tmp/private_file

test/e2e/build/containerignore-symlink/Dockerfile

@@ -0,0 +1,2 @@
+FROM alpine
+COPY / /dir

test/e2e/build_test.go

@@ -461,6 +461,33 @@ RUN find /test`, ALPINE)
 		Expect(session.OutputToString()).To(ContainSubstring("/test/dummy"))
 	})
 
+	It("podman remote build must not allow symlink for ignore files", func() {
+		// Create a random file where symlink must be resolved
+		// but build should not be able to access it.
+		f, err := os.Create(filepath.Join("/tmp", "private_file"))
+		Expect(err).ToNot(HaveOccurred())
+		// Mark hello to be ignored in outerfile, but it should not be ignored.
+		_, err = f.WriteString("hello\n")
+		Expect(err).ToNot(HaveOccurred())
+		defer f.Close()
+
+		if IsRemote() {
+			podmanTest.StopRemoteService()
+			podmanTest.StartRemoteService()
+		} else {
+			Skip("Only valid at remote test")
+		}
+
+		session := podmanTest.Podman([]string{"build", "--pull-never", "-t", "test", "build/containerignore-symlink/"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+
+		session = podmanTest.Podman([]string{"run", "--rm", "test", "ls", "/dir"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		Expect(session.OutputToString()).To(ContainSubstring("hello"))
+	})
+
 	It("podman remote test container/docker file is not at root of context dir", func() {
 		if IsRemote() {
 			podmanTest.StopRemoteService()