Skip to content

Commit

Permalink
Add high-level validations for invalid image commands
Browse files Browse the repository at this point in the history
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
  • Loading branch information
cpuguy83 committed Jun 26, 2024
1 parent a4d7f29 commit 5395775
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 8 deletions.
10 changes: 8 additions & 2 deletions load.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
62 changes: 62 additions & 0 deletions load_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
19 changes: 13 additions & 6 deletions source.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,17 @@ func (s *Source) AsMount(name string, sOpt SourceOpts, opts ...llb.ConstraintsOp

var errInvalidMountConfig = errors.New("invalid mount config")

func (m *SourceMount) validate(root string) error {
if m.Dest == "/" {
return errors.Wrap(errInvalidMountConfig, "mount destination must not be \"/\"")
}
if root != "/" && filepath.HasPrefix(m.Dest, root) {

Check failure on line 203 in source.go

View workflow job for this annotation

GitHub Actions / lint

SA1019: filepath.HasPrefix has been deprecated since Go 1.0 because it shouldn't be used: HasPrefix does not respect path boundaries and does not ignore case when required. (staticcheck)
// 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) {
Expand All @@ -220,12 +231,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...)
Expand Down

0 comments on commit 5395775

Please sign in to comment.