Add high-level validations for invalid image commands

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

load.go

@@ -142,9 +142,15 @@ func (s *Source) validate(failContext ...string) (retErr error) {
  }
 
  if s.DockerImage.Cmd != nil {
+ // If someone *really* wants to extract the entire rootfs, they need to say so explicitly.
+ // We won't fill this in for them, particularly because this is almost certainly not the user's intent.
+ if s.Path == "" {
+ retErr = goerrors.Join(retErr, errors.Errorf("source path cannot be empty"))
+ }
+
  for _, mnt := range s.DockerImage.Cmd.Mounts {
- if mnt.Dest == s.Path {
- return errors.Wrap(errInvalidMountConfig, "")
+ if err := mnt.validate(s.Path); err != nil {
+ retErr = goerrors.Join(retErr, err)
  }
  if err := mnt.Spec.validate("docker image source with ref", "'"+s.DockerImage.Ref+"'"); err != nil {
  retErr = goerrors.Join(retErr, err)

load_test.go

@@ -236,6 +236,68 @@ func TestSourceValidation(t *testing.T) {
  Generate: []*SourceGenerator{{Gomod: &GeneratorGomod{}}},
  },
  },
+ {
+ title: "docker images with cmd source must specify a path to extract",
+ expectErr: true,
+ src: Source{
+ Path: "",
+ DockerImage: &SourceDockerImage{
+ Ref: "notexists:latest",
+ Cmd: &Command{
+ Steps: []*BuildStep{
+ {Command: ":"},
+ },
+ },
+ },
+ },
+ },
+ {
+ title: "cmd souce mount dest must not be /",
+ expectErr: true,
+ src: Source{
+ Path: "/foo",
+ DockerImage: &SourceDockerImage{
+ Ref: "notexists:latest",
+ Cmd: &Command{
+ Steps: []*BuildStep{
+ {Command: ":"},
+ },
+ Mounts: []SourceMount{
+ {
+ Dest: "/",
+ Spec: Source{
+ Inline: &SourceInline{
+ File: &SourceInlineFile{},
+ },
+ },
+ },
+ },
+ },
+ },
+ },
+ },
+ {
+ title: "cmd source mount dest must not be a descendent of the extracted source path",
+ expectErr: true,
+ src: Source{
+ Path: "/foo",
+ DockerImage: &SourceDockerImage{
+ Ref: "notexists:latest",
+ Cmd: &Command{
+ Mounts: []SourceMount{
+ {
+ Dest: "/foo",
+ Spec: Source{
+ Inline: &SourceInline{
+ File: &SourceInlineFile{},
+ },
+ },
+ },
+ },
+ },
+ },
+ },
+ },
  }
 
  for _, tc := range cases {

source.go

@@ -6,6 +6,7 @@ import (
  "fmt"
  "io"
  "path/filepath"
+ "strings"
 
  "github.com/moby/buildkit/client/llb"
  "github.com/moby/buildkit/identity"
@@ -30,6 +31,7 @@ func getFilter(src Source, forMount bool, opts ...llb.ConstraintsOpt) llb.StateO
  // if we're using a mount for these sources, the mount will handle path extraction
  path = "/"
  }
+
  switch {
  case src.HTTP != nil,
  src.Git != nil,
@@ -196,6 +198,35 @@ func (s *Source) AsMount(name string, sOpt SourceOpts, opts ...llb.ConstraintsOp
 
 var errInvalidMountConfig = errors.New("invalid mount config")
 
+func pathHasPrefix(s string, prefix string) bool {
+ if s == prefix {
+ return true
+ }
+
+ s = filepath.Clean(s)
+ prefix = filepath.Clean(prefix)
+
+ if s == prefix {
+ return true
+ }
+
+ if strings.HasPrefix(s, prefix+"/") {
+ return true
+ }
+ return false
+}
+
+func (m *SourceMount) validate(root string) error {
+ if m.Dest == "/" {
+ return errors.Wrap(errInvalidMountConfig, "mount destination must not be \"/\"")
+ }
+ if root != "/" && pathHasPrefix(m.Dest, root) {
+ // We cannot support this as the base mount for subPath will shadow the mount being done here.
+ return errors.Wrapf(errInvalidMountConfig, "mount destination (%s) must not be a descendent of the target source path (%s)", m.Dest, root)
+ }
+ return nil
+}
+
 // must not be called with a nil cmd pointer
 // subPath must be a valid non-empty path
 func generateSourceFromImage(st llb.State, cmd *Command, sOpts SourceOpts, subPath string, opts ...llb.ConstraintsOpt) (llb.State, error) {
@@ -220,12 +251,8 @@ func generateSourceFromImage(st llb.State, cmd *Command, sOpts SourceOpts, subPa
  baseRunOpts := []llb.RunOption{CacheDirsToRunOpt(cmd.CacheDirs, "", "")}
 
  for _, src := range cmd.Mounts {
- if src.Dest == "/" {
- return llb.Scratch(), errors.Wrap(errInvalidMountConfig, "mount destination must not be \"/\"")
- }
- if subPath != "/" && filepath.HasPrefix(src.Dest, subPath) {
- // We cannot support this as the base mount for subPath will shadow the mount being done here.
- return llb.Scratch(), errors.Wrapf(errInvalidMountConfig, "mount destination (%s) must not be a descendent of the target source path (%s)", src.Dest, subPath)
+ if err := src.validate(subPath); err != nil {
+ return llb.Scratch(), err
  }
 
  srcSt, err := src.Spec.AsMount(src.Dest, sOpts, opts...)

source_test.go

@@ -1041,3 +1041,43 @@ func (stubMetaResolver) ResolveImageConfig(ctx context.Context, ref string, opt
  }
  return ref, "", dt, nil
 }
+
+func Test_pathHasPrefix(t *testing.T) {
+ type testCase struct {
+ path string
+ prefix string
+ expect bool
+ }
+ cases := []testCase{
+ {"/foo", "/foobar", false},
+ {"/foo", "/foo", true},
+ {"/foo/", "/foo", true},
+ {"/foo/", "/foo/", true},
+ {"//foo", "/foo", true},
+ {"//foo/", "/foo", true},
+ {"/foo/bar", "/foo", true},
+ {"/foo/bar/", "/foo", true},
+ {"/foo/bar/", "/foo/", true},
+ {"/foo/bar", "/foo/", true},
+ {"/foo/bar", "/bar", false},
+ {"/foo/bar", "/foo/bar/baz", false},
+ {"/foo/bar/baz", "/foo/bar", true},
+ {"/foo//bar", "/foo", true},
+ {"/foo//bar/", "/foo", true},
+ {"/foo//bar/", "/foo/", true},
+ {"/foo//bar/", "/foo//", true},
+ }
+
+ // Replace / char which is special for go tests with something less special.
+ forTestName := func(s string) string {
+ return strings.Replace(s, "/", "__", -1)
+ }
+
+ for _, tc := range cases {
+ name := fmt.Sprintf("path=%s,prefix=%s", forTestName(tc.path), forTestName(tc.prefix))
+ t.Run(name, func(t *testing.T) {
+ hasPrefix := pathHasPrefix(tc.path, tc.prefix)
+ assert.Equal(t, hasPrefix, tc.expect)
+ })
+ }
+}