Skip to content

Commit

Permalink
remote,build: error if containerignore is symlink
Browse files Browse the repository at this point in the history
Drop support for remote use-cases when `.containerignore` or
`.dockerignore` is a symlink pointing to arbitrary location on host.

Signed-off-by: Aditya R <arajan@redhat.com>
  • Loading branch information
flouthoc committed Jan 26, 2023
1 parent 0184ba5 commit 70e8f62
Show file tree
Hide file tree
Showing 9 changed files with 126 additions and 29 deletions.
8 changes: 8 additions & 0 deletions pkg/api/handlers/compat/images_build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -621,6 +622,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,
Expand Down Expand Up @@ -670,6 +677,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
From: fromImage,
IDMappingOptions: &idMappingOptions,
IgnoreUnrecognizedInstructions: query.Ignore,
IgnoreFile: ignoreFile,
Isolation: isolation,
Jobs: &jobs,
Labels: labels,
Expand Down
43 changes: 15 additions & 28 deletions pkg/bindings/images/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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
}
71 changes: 71 additions & 0 deletions pkg/util/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion test/buildah-bud/apply-podman-deltas
Original file line number Diff line number Diff line change
Expand Up @@ -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.
###############################################################################
Expand Down
1 change: 1 addition & 0 deletions test/e2e/build/containerignore-symlink/.dockerignore
2 changes: 2 additions & 0 deletions test/e2e/build/containerignore-symlink/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
FROM alpine
COPY / /dir
Empty file.
Empty file.
27 changes: 27 additions & 0 deletions test/e2e/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit 70e8f62

Please sign in to comment.