Merge pull request #10524 from icecrime/10513_carry_failed_tests

Ensure Dockerfile in context

api/client/commands.go

@@ -39,6 +39,7 @@ import (
 	"github.com/docker/docker/pkg/parsers/filters"
 	"github.com/docker/docker/pkg/promise"
 	"github.com/docker/docker/pkg/signal"
+	"github.com/docker/docker/pkg/symlink"
 	"github.com/docker/docker/pkg/term"
 	"github.com/docker/docker/pkg/timeutils"
 	"github.com/docker/docker/pkg/units"
@@ -147,36 +148,36 @@ func (cli *DockerCli) CmdBuild(args ...string) error {
 			return err
 		}
 
-		var filename string       // path to Dockerfile
-		var origDockerfile string // used for error msg
+		filename := *dockerfileName // path to Dockerfile
 
 		if *dockerfileName == "" {
 			// No -f/--file was specified so use the default
-			origDockerfile = api.DefaultDockerfileName
-			*dockerfileName = origDockerfile
+			*dockerfileName = api.DefaultDockerfileName
 			filename = path.Join(absRoot, *dockerfileName)
-		} else {
-			origDockerfile = *dockerfileName
-			if filename, err = filepath.Abs(*dockerfileName); err != nil {
-				return err
-			}
+		}
 
-			// Verify that 'filename' is within the build context
-			if !strings.HasSuffix(absRoot, string(os.PathSeparator)) {
-				absRoot += string(os.PathSeparator)
-			}
-			if !strings.HasPrefix(filename, absRoot) {
-				return fmt.Errorf("The Dockerfile (%s) must be within the build context (%s)", *dockerfileName, root)
-			}
+		origDockerfile := *dockerfileName // used for error msg
+
+		if filename, err = filepath.Abs(filename); err != nil {
+			return err
+		}
 
-			// Now reset the dockerfileName to be relative to the build context
-			*dockerfileName = filename[len(absRoot):]
+		// Verify that 'filename' is within the build context
+		filename, err = symlink.FollowSymlinkInScope(filename, absRoot)
+		if err != nil {
+			return fmt.Errorf("The Dockerfile (%s) must be within the build context (%s)", origDockerfile, root)
+		}
+
+		// Now reset the dockerfileName to be relative to the build context
+		*dockerfileName, err = filepath.Rel(absRoot, filename)
+		if err != nil {
+			return err
 		}
 
-		if _, err = os.Stat(filename); os.IsNotExist(err) {
-			return fmt.Errorf("Can not locate Dockerfile: %s", origDockerfile)
+		if _, err = os.Lstat(filename); os.IsNotExist(err) {
+			return fmt.Errorf("Cannot locate Dockerfile: %s", origDockerfile)
 		}
-		var includes []string = []string{"."}
+		var includes = []string{"."}
 
 		excludes, err := utils.ReadDockerIgnore(path.Join(root, ".dockerignore"))
 		if err != nil {

builder/evaluator.go

@@ -24,7 +24,7 @@ import (
 	"fmt"
 	"io"
 	"os"
-	"path"
+	"path/filepath"
 	"strings"
 
 	log "github.com/Sirupsen/logrus"
@@ -32,6 +32,7 @@ import (
 	"github.com/docker/docker/daemon"
 	"github.com/docker/docker/engine"
 	"github.com/docker/docker/pkg/fileutils"
+	"github.com/docker/docker/pkg/symlink"
 	"github.com/docker/docker/pkg/tarsum"
 	"github.com/docker/docker/registry"
 	"github.com/docker/docker/runconfig"
@@ -169,12 +170,15 @@ func (b *Builder) Run(context io.Reader) (string, error) {
 
 // Reads a Dockerfile from the current context. It assumes that the
 // 'filename' is a relative path from the root of the context
-func (b *Builder) readDockerfile(filename string) error {
-	filename = path.Join(b.contextPath, filename)
+func (b *Builder) readDockerfile(origFile string) error {
+	filename, err := symlink.FollowSymlinkInScope(filepath.Join(b.contextPath, origFile), b.contextPath)
+	if err != nil {
+		return fmt.Errorf("The Dockerfile (%s) must be within the build context", origFile)
+	}
 
-	fi, err := os.Stat(filename)
+	fi, err := os.Lstat(filename)
 	if os.IsNotExist(err) {
-		return fmt.Errorf("Cannot build a directory without a Dockerfile")
+		return fmt.Errorf("Cannot locate specified Dockerfile: %s", origFile)
 	}
 	if fi.Size() == 0 {
 		return ErrDockerfileEmpty
@@ -201,13 +205,13 @@ func (b *Builder) readDockerfile(filename string) error {
 	// Note that this assumes the Dockerfile has been read into memory and
 	// is now safe to be removed.
 
-	excludes, _ := utils.ReadDockerIgnore(path.Join(b.contextPath, ".dockerignore"))
+	excludes, _ := utils.ReadDockerIgnore(filepath.Join(b.contextPath, ".dockerignore"))
 	if rm, _ := fileutils.Matches(".dockerignore", excludes); rm == true {
-		os.Remove(path.Join(b.contextPath, ".dockerignore"))
+		os.Remove(filepath.Join(b.contextPath, ".dockerignore"))
 		b.context.(tarsum.BuilderContext).Remove(".dockerignore")
 	}
 	if rm, _ := fileutils.Matches(b.dockerfileName, excludes); rm == true {
-		os.Remove(path.Join(b.contextPath, b.dockerfileName))
+		os.Remove(filepath.Join(b.contextPath, b.dockerfileName))
 		b.context.(tarsum.BuilderContext).Remove(b.dockerfileName)
 	}
 

integration-cli/docker_api_containers_test.go

@@ -299,3 +299,70 @@ func TestGetContainerStats(t *testing.T) {
 	}
 	logDone("container REST API - check GET containers/stats")
 }
+
+func TestBuildApiDockerfilePath(t *testing.T) {
+	// Test to make sure we stop people from trying to leave the
+	// build context when specifying the path to the dockerfile
+	buffer := new(bytes.Buffer)
+	tw := tar.NewWriter(buffer)
+	defer tw.Close()
+
+	dockerfile := []byte("FROM busybox")
+	if err := tw.WriteHeader(&tar.Header{
+		Name: "Dockerfile",
+		Size: int64(len(dockerfile)),
+	}); err != nil {
+		t.Fatalf("failed to write tar file header: %v", err)
+	}
+	if _, err := tw.Write(dockerfile); err != nil {
+		t.Fatalf("failed to write tar file content: %v", err)
+	}
+	if err := tw.Close(); err != nil {
+		t.Fatalf("failed to close tar archive: %v", err)
+	}
+
+	out, err := sockRequestRaw("POST", "/build?dockerfile=../Dockerfile", buffer, "application/x-tar")
+	if err == nil {
+		t.Fatalf("Build was supposed to fail: %s", out)
+	}
+
+	if !strings.Contains(string(out), "must be within the build context") {
+		t.Fatalf("Didn't complain about leaving build context: %s", out)
+	}
+
+	logDone("container REST API - check build w/bad Dockerfile path")
+}
+
+func TestBuildApiDockerfileSymlink(t *testing.T) {
+	// Test to make sure we stop people from trying to leave the
+	// build context when specifying a symlink as the path to the dockerfile
+	buffer := new(bytes.Buffer)
+	tw := tar.NewWriter(buffer)
+	defer tw.Close()
+
+	if err := tw.WriteHeader(&tar.Header{
+		Name:     "Dockerfile",
+		Typeflag: tar.TypeSymlink,
+		Linkname: "/etc/passwd",
+	}); err != nil {
+		t.Fatalf("failed to write tar file header: %v", err)
+	}
+	if err := tw.Close(); err != nil {
+		t.Fatalf("failed to close tar archive: %v", err)
+	}
+
+	out, err := sockRequestRaw("POST", "/build", buffer, "application/x-tar")
+	if err == nil {
+		t.Fatalf("Build was supposed to fail: %s", out)
+	}
+
+	// The reason the error is "Cannot locate specified Dockerfile" is because
+	// in the builder, the symlink is resolved within the context, therefore
+	// Dockerfile -> /etc/passwd becomes etc/passwd from the context which is
+	// a nonexistent file.
+	if !strings.Contains(string(out), "Cannot locate specified Dockerfile: Dockerfile") {
+		t.Fatalf("Didn't complain about leaving build context: %s", out)
+	}
+
+	logDone("container REST API - check build w/bad Dockerfile symlink path")
+}

integration-cli/docker_cli_build_test.go

@@ -4630,3 +4630,67 @@ func TestBuildFromOfficialNames(t *testing.T) {
 	}
 	logDone("build - from official names")
 }
+
+func TestBuildDockerfileOutsideContext(t *testing.T) {
+	name := "testbuilddockerfileoutsidecontext"
+	tmpdir, err := ioutil.TempDir("", name)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tmpdir)
+	ctx := filepath.Join(tmpdir, "context")
+	if err := os.MkdirAll(ctx, 0755); err != nil {
+		t.Fatal(err)
+	}
+	if err := ioutil.WriteFile(filepath.Join(ctx, "Dockerfile"), []byte("FROM busybox"), 0644); err != nil {
+		t.Fatal(err)
+	}
+	wd, err := os.Getwd()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.Chdir(wd)
+	if err := os.Chdir(ctx); err != nil {
+		t.Fatal(err)
+	}
+	if err := ioutil.WriteFile(filepath.Join(tmpdir, "outsideDockerfile"), []byte("FROM busbox"), 0644); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Symlink("../outsideDockerfile", filepath.Join(ctx, "dockerfile1")); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Symlink(filepath.Join(tmpdir, "outsideDockerfile"), filepath.Join(ctx, "dockerfile2")); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Link("../outsideDockerfile", filepath.Join(ctx, "dockerfile3")); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Link(filepath.Join(tmpdir, "outsideDockerfile"), filepath.Join(ctx, "dockerfile4")); err != nil {
+		t.Fatal(err)
+	}
+	for _, dockerfilePath := range []string{
+		"../outsideDockerfile",
+		filepath.Join(ctx, "dockerfile1"),
+		filepath.Join(ctx, "dockerfile2"),
+		filepath.Join(ctx, "dockerfile3"),
+		filepath.Join(ctx, "dockerfile4"),
+	} {
+		out, _, err := runCommandWithOutput(exec.Command(dockerBinary, "build", "-t", name, "--no-cache", "-f", dockerfilePath, "."))
+		if err == nil {
+			t.Fatalf("Expected error with %s. Out: %s", dockerfilePath, out)
+		}
+		deleteImages(name)
+	}
+
+	os.Chdir(tmpdir)
+
+	// Path to Dockerfile should be resolved relative to working directory, not relative to context.
+	// There is a Dockerfile in the context, but since there is no Dockerfile in the current directory, the following should fail
+	out, _, err := runCommandWithOutput(exec.Command(dockerBinary, "build", "-t", name, "--no-cache", "-f", "Dockerfile", ctx))
+	if err == nil {
+		t.Fatalf("Expected error. Out: %s", out)
+	}
+	deleteImages(name)
+
+	logDone("build - Dockerfile outside context")
+}

integration-cli/docker_utils.go

@@ -274,6 +274,15 @@ func daemonHost() string {
 }
 
 func sockRequest(method, endpoint string, data interface{}) ([]byte, error) {
+	jsonData := bytes.NewBuffer(nil)
+	if err := json.NewEncoder(jsonData).Encode(data); err != nil {
+		return nil, err
+	}
+
+	return sockRequestRaw(method, endpoint, jsonData, "application/json")
+}
+
+func sockRequestRaw(method, endpoint string, data io.Reader, ct string) ([]byte, error) {
 	daemon := daemonHost()
 	daemonUrl, err := url.Parse(daemon)
 	if err != nil {
@@ -296,17 +305,16 @@ func sockRequest(method, endpoint string, data interface{}) ([]byte, error) {
 	client := httputil.NewClientConn(c, nil)
 	defer client.Close()
 
-	jsonData := bytes.NewBuffer(nil)
-	if err := json.NewEncoder(jsonData).Encode(data); err != nil {
-		return nil, err
-	}
-
-	req, err := http.NewRequest(method, endpoint, jsonData)
-	req.Header.Set("Content-Type", "application/json")
+	req, err := http.NewRequest(method, endpoint, data)
 	if err != nil {
 		return nil, fmt.Errorf("could not create new request: %v", err)
 	}
 
+	if ct == "" {
+		ct = "application/json"
+	}
+	req.Header.Set("Content-Type", ct)
+
 	resp, err := client.Do(req)
 	if err != nil {
 		return nil, fmt.Errorf("could not perform request: %v", err)