From f7f7fff340161fd65b020b16a138da48e879a74b Mon Sep 17 00:00:00 2001 From: fsabiu Date: Sun, 3 May 2026 15:28:02 +0200 Subject: [PATCH 1/2] fix: confine Docker copy dependency sources --- pkg/skaffold/docker/dockertest.go | 8 ++++++ pkg/skaffold/docker/parse.go | 42 +++++++++++++++++++++++++++++-- pkg/skaffold/docker/parse_test.go | 37 +++++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 2 deletions(-) diff --git a/pkg/skaffold/docker/dockertest.go b/pkg/skaffold/docker/dockertest.go index 833c51b713b..aca97eefee2 100644 --- a/pkg/skaffold/docker/dockertest.go +++ b/pkg/skaffold/docker/dockertest.go @@ -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") } diff --git a/pkg/skaffold/docker/parse.go b/pkg/skaffold/docker/parse.go index af4229ca1e2..2ab139d899c 100644 --- a/pkg/skaffold/docker/parse.go +++ b/pkg/skaffold/docker/parse.go @@ -212,9 +212,13 @@ func expandSrcGlobPatterns(workspace string, cpCmds []*copyCommand) ([]FromTo, e matchesOne := false for _, p := range cpCmd.srcs { - path := filepath.Join(workspace, p) + path, relPath, err := resolveCopySourceInWorkspace(workspace, 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 } @@ -232,6 +236,9 @@ func expandSrcGlobPatterns(workspace string, cpCmds []*copyCommand) ([]FromTo, e 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}) } @@ -248,6 +255,37 @@ func expandSrcGlobPatterns(workspace string, cpCmds []*copyCommand) ([]FromTo, e return fts, nil } +func resolveCopySourceInWorkspace(workspace, 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) + } + + workspaceAbs, err := filepath.Abs(workspace) + if err != nil { + return "", "", fmt.Errorf("resolving build context %q: %w", workspace, err) + } + + 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 filepath.Join(workspace, src), 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, diff --git a/pkg/skaffold/docker/parse_test.go b/pkg/skaffold/docker/parse_test.go index be46cd4e9b1..48869be1e64 100644 --- a/pkg/skaffold/docker/parse_test.go +++ b/pkg/skaffold/docker/parse_test.go @@ -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 From cd9e46d2733dd779932653e2307c3614c8ac9dee Mon Sep 17 00:00:00 2001 From: fsabiu Date: Sun, 3 May 2026 15:37:54 +0200 Subject: [PATCH 2/2] fix: reuse Docker workspace absolute path --- pkg/skaffold/docker/parse.go | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/pkg/skaffold/docker/parse.go b/pkg/skaffold/docker/parse.go index 2ab139d899c..7b732f54609 100644 --- a/pkg/skaffold/docker/parse.go +++ b/pkg/skaffold/docker/parse.go @@ -208,11 +208,16 @@ 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, relPath, err := resolveCopySourceInWorkspace(workspace, p) + path, relPath, err := resolveCopySourceInWorkspace(workspace, workspaceAbs, p) if err != nil { return nil, err } @@ -232,7 +237,11 @@ 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) } @@ -255,17 +264,12 @@ func expandSrcGlobPatterns(workspace string, cpCmds []*copyCommand) ([]FromTo, e return fts, nil } -func resolveCopySourceInWorkspace(workspace, src string) (string, string, error) { +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) } - workspaceAbs, err := filepath.Abs(workspace) - if err != nil { - return "", "", fmt.Errorf("resolving build context %q: %w", workspace, err) - } - sourceAbs, err := filepath.Abs(filepath.Join(workspace, src)) if err != nil { return "", "", fmt.Errorf("resolving copy source %q: %w", src, err) @@ -279,7 +283,7 @@ func resolveCopySourceInWorkspace(workspace, src string) (string, string, error) return "", "", fmt.Errorf("copy source %q resolves outside build context %q", src, workspace) } - return filepath.Join(workspace, src), rel, nil + return sourceAbs, rel, nil } func isRelativePathInWorkspace(rel string) bool {