From 284c44d04f3e5fe064f2ff6e928a224cb14e2320 Mon Sep 17 00:00:00 2001 From: Aditya R Date: Wed, 13 Mar 2024 20:02:10 +0530 Subject: [PATCH 1/3] remote,build: error if containerignore is symlink Drop support for remote use-cases when `.containerignore` or `.dockerignore` is a symlink pointing to arbitrary location on host. Signed-off-by: Aditya R --- pkg/api/handlers/compat/images_build.go | 8 +++ pkg/bindings/images/build.go | 43 ++++------- pkg/util/utils.go | 71 +++++++++++++++++++ test/buildah-bud/apply-podman-deltas | 3 +- .../containerignore-symlink/.dockerignore | 1 + .../build/containerignore-symlink/Dockerfile | 2 + test/e2e/build/containerignore-symlink/hello | 0 test/e2e/build/containerignore-symlink/world | 0 test/e2e/build_test.go | 27 +++++++ 9 files changed, 126 insertions(+), 29 deletions(-) create mode 120000 test/e2e/build/containerignore-symlink/.dockerignore create mode 100644 test/e2e/build/containerignore-symlink/Dockerfile create mode 100644 test/e2e/build/containerignore-symlink/hello create mode 100644 test/e2e/build/containerignore-symlink/world diff --git a/pkg/api/handlers/compat/images_build.go b/pkg/api/handlers/compat/images_build.go index f1bee8b502..de5022eb6e 100644 --- a/pkg/api/handlers/compat/images_build.go +++ b/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" @@ -627,6 +628,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, @@ -676,6 +683,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) { From: fromImage, IDMappingOptions: &idMappingOptions, IgnoreUnrecognizedInstructions: query.Ignore, + IgnoreFile: ignoreFile, Isolation: isolation, Jobs: &jobs, Labels: labels, diff --git a/pkg/bindings/images/build.go b/pkg/bindings/images/build.go index 32f42a62a3..88763590ff 100644 --- a/pkg/bindings/images/build.go +++ b/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,6 +451,8 @@ 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 { @@ -465,6 +460,9 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO 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) @@ -472,6 +470,7 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO } 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 { @@ -776,23 +783,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 -} diff --git a/pkg/util/utils.go b/pkg/util/utils.go index e7952a0bc3..6b99c0390e 100644 --- a/pkg/util/utils.go +++ b/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 + // .containerignore or .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) { diff --git a/test/buildah-bud/apply-podman-deltas b/test/buildah-bud/apply-podman-deltas index 06d69ed034..4c4ab17af3 100755 --- a/test/buildah-bud/apply-podman-deltas +++ b/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. ############################################################################### diff --git a/test/e2e/build/containerignore-symlink/.dockerignore b/test/e2e/build/containerignore-symlink/.dockerignore new file mode 120000 index 0000000000..7ec1325806 --- /dev/null +++ b/test/e2e/build/containerignore-symlink/.dockerignore @@ -0,0 +1 @@ +/tmp/private_file \ No newline at end of file diff --git a/test/e2e/build/containerignore-symlink/Dockerfile b/test/e2e/build/containerignore-symlink/Dockerfile new file mode 100644 index 0000000000..0f64ccd186 --- /dev/null +++ b/test/e2e/build/containerignore-symlink/Dockerfile @@ -0,0 +1,2 @@ +FROM alpine +COPY / /dir diff --git a/test/e2e/build/containerignore-symlink/hello b/test/e2e/build/containerignore-symlink/hello new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/e2e/build/containerignore-symlink/world b/test/e2e/build/containerignore-symlink/world new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/e2e/build_test.go b/test/e2e/build_test.go index b783007ae4..032b60b9c3 100644 --- a/test/e2e/build_test.go +++ b/test/e2e/build_test.go @@ -456,6 +456,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() From a74493f9ad860382d36c56f699bb62d4141c30ef Mon Sep 17 00:00:00 2001 From: Lokesh Mandvekar Date: Mon, 15 Apr 2024 08:33:15 -0400 Subject: [PATCH 2/3] CI fixes from Chris Evich and Ed Santiago Signed-off-by: Lokesh Mandvekar --- test/buildah-bud/apply-podman-deltas | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/buildah-bud/apply-podman-deltas b/test/buildah-bud/apply-podman-deltas index 4c4ab17af3..c8bdbcf386 100755 --- a/test/buildah-bud/apply-podman-deltas +++ b/test/buildah-bud/apply-podman-deltas @@ -246,7 +246,8 @@ skip_if_remote "Explicit request in buildah PR 4190 to skip this on remote" \ skip_if_remote "different error messages between podman & podman-remote" \ "bud with .dockerignore #2" \ - "bud with .dockerignore #4" + "bud with .dockerignore #4" \ + "bud with .dockerignore #6" # END tests which are skipped due to actual podman or podman-remote bugs. ############################################################################### From 0ef28c0dad466c06270a43db73b17573466b0763 Mon Sep 17 00:00:00 2001 From: Aditya R Date: Fri, 3 Nov 2023 19:31:17 +0530 Subject: [PATCH 3/3] remote,test: remove .dockerignore which is a symlink It seems certain test infrastructure prevents cloning repo which contains symlink outside of the repo itself, generate symlink for such test by the testsuite itself just before running test and remove it when test is completed. Signed-off-by: Aditya R (cherry picked from commit 607aff55fa1a3b80328e8010049380728fde1d62) Signed-off-by: Lokesh Mandvekar --- .../build/containerignore-symlink/.dockerignore | 1 - test/e2e/build_test.go | 14 +++++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) delete mode 120000 test/e2e/build/containerignore-symlink/.dockerignore diff --git a/test/e2e/build/containerignore-symlink/.dockerignore b/test/e2e/build/containerignore-symlink/.dockerignore deleted file mode 120000 index 7ec1325806..0000000000 --- a/test/e2e/build/containerignore-symlink/.dockerignore +++ /dev/null @@ -1 +0,0 @@ -/tmp/private_file \ No newline at end of file diff --git a/test/e2e/build_test.go b/test/e2e/build_test.go index 032b60b9c3..c01fb8b7c3 100644 --- a/test/e2e/build_test.go +++ b/test/e2e/build_test.go @@ -459,13 +459,25 @@ RUN find /test`, ALPINE) 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")) + privateFile := filepath.Join("/tmp", "private_file") + f, err := os.Create(privateFile) 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() + // Create .dockerignore which is a symlink to /tmp/private_file. + currentDir, err := os.Getwd() + Expect(err).ToNot(HaveOccurred()) + ignoreFile := filepath.Join(currentDir, "build/containerignore-symlink/.dockerignore") + err = os.Symlink(privateFile, ignoreFile) + Expect(err).ToNot(HaveOccurred()) + // Remove created .dockerignore for this test when test ends. + defer func() { + os.Remove(ignoreFile) + }() + if IsRemote() { podmanTest.StopRemoteService() podmanTest.StartRemoteService()