Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions pkg/skaffold/docker/dockertest.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,14 @@ func (f *fakeImageFetcher) fetch(_ context.Context, image string, _ Config) (*v1
},
},
}, nil
case "parentpath:onbuild":
return &v1.ConfigFile{
Config: v1.Config{
OnBuild: []string{
"COPY ../secret.txt /secret.txt",
},
},
}, nil
case "library/ruby:2.3.0":
return nil, fmt.Errorf("retrieving image \"library/ruby:2.3.0\": unsupported MediaType: \"application/vnd.docker.distribution.manifest.v1+prettyjws\", see https://github.com/google/go-containerregistry/issues/377")
}
Expand Down
48 changes: 45 additions & 3 deletions pkg/skaffold/docker/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,13 +208,22 @@ func expandBuildArgs(nodes []*parser.Node, buildArgs map[string]*string) error {

func expandSrcGlobPatterns(workspace string, cpCmds []*copyCommand) ([]FromTo, error) {
var fts []FromTo
workspaceAbs, err := filepath.Abs(workspace)
if err != nil {
return nil, fmt.Errorf("resolving build context %q: %w", workspace, err)
}

for _, cpCmd := range cpCmds {
matchesOne := false

for _, p := range cpCmd.srcs {
path := filepath.Join(workspace, p)
path, relPath, err := resolveCopySourceInWorkspace(workspace, workspaceAbs, p)
if err != nil {
return nil, err
}

if _, err := os.Stat(path); err == nil {
fts = append(fts, FromTo{From: filepath.Clean(p), To: cpCmd.dest, ToIsDir: cpCmd.destIsDir, StartLine: cpCmd.startLine, EndLine: cpCmd.endLine})
fts = append(fts, FromTo{From: relPath, To: cpCmd.dest, ToIsDir: cpCmd.destIsDir, StartLine: cpCmd.startLine, EndLine: cpCmd.endLine})
matchesOne = true
continue
}
Expand All @@ -228,10 +237,17 @@ func expandSrcGlobPatterns(workspace string, cpCmds []*copyCommand) ([]FromTo, e
}

for _, f := range files {
rel, err := filepath.Rel(workspace, f)
fileAbs, err := filepath.Abs(f)
if err != nil {
return nil, fmt.Errorf("resolving copy source %q: %w", f, err)
}
rel, err := filepath.Rel(workspaceAbs, fileAbs)
if err != nil {
return nil, fmt.Errorf("getting relative path of %s", f)
}
if !isRelativePathInWorkspace(rel) {
return nil, fmt.Errorf("copy source %q resolves outside build context %q", p, workspace)
}

fts = append(fts, FromTo{From: rel, To: cpCmd.dest, ToIsDir: cpCmd.destIsDir, StartLine: cpCmd.startLine, EndLine: cpCmd.endLine})
}
Expand All @@ -248,6 +264,32 @@ func expandSrcGlobPatterns(workspace string, cpCmds []*copyCommand) ([]FromTo, e
return fts, nil
}

func resolveCopySourceInWorkspace(workspace, workspaceAbs, src string) (string, string, error) {
src = filepath.Clean(src)
if filepath.IsAbs(src) {
return "", "", fmt.Errorf("copy source %q resolves outside build context %q", src, workspace)
}

sourceAbs, err := filepath.Abs(filepath.Join(workspace, src))
if err != nil {
return "", "", fmt.Errorf("resolving copy source %q: %w", src, err)
}

rel, err := filepath.Rel(workspaceAbs, sourceAbs)
if err != nil {
return "", "", fmt.Errorf("getting relative path of %s", sourceAbs)
}
if !isRelativePathInWorkspace(rel) {
return "", "", fmt.Errorf("copy source %q resolves outside build context %q", src, workspace)
}

return sourceAbs, rel, nil
}

func isRelativePathInWorkspace(rel string) bool {
return rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) && !filepath.IsAbs(rel)
}

func extractCopyCommands(ctx context.Context, nodes []*parser.Node, onlyLastImage bool, cfg Config) ([]*copyCommand, error) {
stages := map[string]bool{
"scratch": true,
Expand Down
37 changes: 37 additions & 0 deletions pkg/skaffold/docker/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,43 @@ func TestReadCopyCmdsFromDockerfile(t *testing.T) {
}
}

func TestReadCopyCmdsFromDockerfileRejectsSourcesOutsideWorkspace(t *testing.T) {
tests := []struct {
description string
dockerfile string
}{
{
description: "direct copy parent path",
dockerfile: "FROM nginx\nCOPY ../secret.txt /secret.txt",
},
{
description: "onbuild copy parent path",
dockerfile: "FROM parentpath:onbuild",
},
}

for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
imageFetcher := fakeImageFetcher{}
t.Override(&RetrieveImage, imageFetcher.fetch)

tmp := t.NewTempDir()
tmp.Mkdir("workspace/project")
tmp.Write("workspace/secret.txt", "secret")

dockerfilePath := tmp.Path("workspace/project/Dockerfile")
err := os.WriteFile(dockerfilePath, []byte(test.dockerfile), 0644)
if err != nil {
t.Error(err)
}

cfg := mockConfig{mode: config.RunModes.Build}
_, err = ReadCopyCmdsFromDockerfile(context.Background(), false, dockerfilePath, tmp.Path("workspace/project"), make(map[string]*string), cfg)
t.CheckErrorContains("resolves outside build context", err)
})
}
}

func TestRemoveExtraBuildArgs(t *testing.T) {
tests := []struct {
description string
Expand Down