diff --git a/api/client/commands.go b/api/client/commands.go index 647516da6b..6562709f3e 100644 --- a/api/client/commands.go +++ b/api/client/commands.go @@ -161,6 +161,9 @@ func (cli *DockerCli) CmdBuild(args ...string) error { if _, err = os.Stat(filename); os.IsNotExist(err) { return fmt.Errorf("no Dockerfile found in %s", cmd.Arg(0)) } + if err = utils.ValidateContextDirectory(root); err != nil { + return fmt.Errorf("Error checking context is accessible: '%s'. Please check permissions and try again.", err) + } context, err = archive.Tar(root, archive.Uncompressed) } var body io.Reader diff --git a/integration-cli/build_tests/TestBuildWithInaccessibleFilesInContext/inaccessibledirectory/Dockerfile b/integration-cli/build_tests/TestBuildWithInaccessibleFilesInContext/inaccessibledirectory/Dockerfile new file mode 100644 index 0000000000..0964b8e87c --- /dev/null +++ b/integration-cli/build_tests/TestBuildWithInaccessibleFilesInContext/inaccessibledirectory/Dockerfile @@ -0,0 +1,2 @@ +FROM busybox +ADD . /foo/ diff --git a/integration-cli/build_tests/TestBuildWithInaccessibleFilesInContext/inaccessibledirectory/directoryWeCantStat/bar b/integration-cli/build_tests/TestBuildWithInaccessibleFilesInContext/inaccessibledirectory/directoryWeCantStat/bar new file mode 100644 index 0000000000..257cc5642c --- /dev/null +++ b/integration-cli/build_tests/TestBuildWithInaccessibleFilesInContext/inaccessibledirectory/directoryWeCantStat/bar @@ -0,0 +1 @@ +foo diff --git a/integration-cli/build_tests/TestBuildWithInaccessibleFilesInContext/inaccessiblefile/Dockerfile b/integration-cli/build_tests/TestBuildWithInaccessibleFilesInContext/inaccessiblefile/Dockerfile new file mode 100644 index 0000000000..0964b8e87c --- /dev/null +++ b/integration-cli/build_tests/TestBuildWithInaccessibleFilesInContext/inaccessiblefile/Dockerfile @@ -0,0 +1,2 @@ +FROM busybox +ADD . /foo/ diff --git a/integration-cli/build_tests/TestBuildWithInaccessibleFilesInContext/inaccessiblefile/fileWithoutReadAccess b/integration-cli/build_tests/TestBuildWithInaccessibleFilesInContext/inaccessiblefile/fileWithoutReadAccess new file mode 100644 index 0000000000..b25f9a2a19 --- /dev/null +++ b/integration-cli/build_tests/TestBuildWithInaccessibleFilesInContext/inaccessiblefile/fileWithoutReadAccess @@ -0,0 +1 @@ +should make `docker build` throw an error diff --git a/integration-cli/build_tests/TestBuildWithInaccessibleFilesInContext/linksdirectory/Dockerfile b/integration-cli/build_tests/TestBuildWithInaccessibleFilesInContext/linksdirectory/Dockerfile new file mode 100644 index 0000000000..0964b8e87c --- /dev/null +++ b/integration-cli/build_tests/TestBuildWithInaccessibleFilesInContext/linksdirectory/Dockerfile @@ -0,0 +1,2 @@ +FROM busybox +ADD . /foo/ diff --git a/integration-cli/build_tests/TestBuildWithInaccessibleFilesInContext/linksdirectory/g b/integration-cli/build_tests/TestBuildWithInaccessibleFilesInContext/linksdirectory/g new file mode 120000 index 0000000000..5fc3f33923 --- /dev/null +++ b/integration-cli/build_tests/TestBuildWithInaccessibleFilesInContext/linksdirectory/g @@ -0,0 +1 @@ +../../../../../../../../../../../../../../../../../../../azA \ No newline at end of file diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index cd3e06d60c..c4d6f12925 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -2,8 +2,10 @@ package main import ( "fmt" + "os" "os/exec" "path/filepath" + "strings" "testing" ) @@ -119,6 +121,92 @@ func TestAddWholeDirToRoot(t *testing.T) { logDone("build - add whole directory to root") } +// Issue #5270 - ensure we throw a better error than "unexpected EOF" +// when we can't access files in the context. +func TestBuildWithInaccessibleFilesInContext(t *testing.T) { + buildDirectory := filepath.Join(workingDirectory, "build_tests", "TestBuildWithInaccessibleFilesInContext") + addUserCmd := exec.Command("adduser", "unprivilegeduser") + out, _, err := runCommandWithOutput(addUserCmd) + errorOut(err, t, fmt.Sprintf("failed to add user: %v %v", out, err)) + + { + // This is used to ensure we detect inaccessible files early during build in the cli client + pathToInaccessibleFileBuildDirectory := filepath.Join(buildDirectory, "inaccessiblefile") + pathToFileWithoutReadAccess := filepath.Join(pathToInaccessibleFileBuildDirectory, "fileWithoutReadAccess") + + err = os.Chown(pathToFileWithoutReadAccess, 0, 0) + errorOut(err, t, fmt.Sprintf("failed to chown file to root: %s", err)) + err = os.Chmod(pathToFileWithoutReadAccess, 0700) + errorOut(err, t, fmt.Sprintf("failed to chmod file to 700: %s", err)) + + buildCommandStatement := fmt.Sprintf("%s build -t inaccessiblefiles .", dockerBinary) + buildCmd := exec.Command("su", "unprivilegeduser", "-c", buildCommandStatement) + buildCmd.Dir = pathToInaccessibleFileBuildDirectory + out, exitCode, err := runCommandWithOutput(buildCmd) + if err == nil || exitCode == 0 { + t.Fatalf("build should have failed: %s %s", err, out) + } + + // check if we've detected the failure before we started building + if !strings.Contains(out, "no permission to read from ") { + t.Fatalf("output should've contained the string: no permission to read from ") + } + + if !strings.Contains(out, "Error checking context is accessible") { + t.Fatalf("output should've contained the string: Error checking context is accessible") + } + } + { + // This is used to ensure we detect inaccessible directories early during build in the cli client + pathToInaccessibleDirectoryBuildDirectory := filepath.Join(buildDirectory, "inaccessibledirectory") + pathToDirectoryWithoutReadAccess := filepath.Join(pathToInaccessibleDirectoryBuildDirectory, "directoryWeCantStat") + pathToFileInDirectoryWithoutReadAccess := filepath.Join(pathToDirectoryWithoutReadAccess, "bar") + + err = os.Chown(pathToDirectoryWithoutReadAccess, 0, 0) + errorOut(err, t, fmt.Sprintf("failed to chown directory to root: %s", err)) + err = os.Chmod(pathToDirectoryWithoutReadAccess, 0444) + errorOut(err, t, fmt.Sprintf("failed to chmod directory to 755: %s", err)) + err = os.Chmod(pathToFileInDirectoryWithoutReadAccess, 0700) + errorOut(err, t, fmt.Sprintf("failed to chmod file to 444: %s", err)) + + buildCommandStatement := fmt.Sprintf("%s build -t inaccessiblefiles .", dockerBinary) + buildCmd := exec.Command("su", "unprivilegeduser", "-c", buildCommandStatement) + buildCmd.Dir = pathToInaccessibleDirectoryBuildDirectory + out, exitCode, err := runCommandWithOutput(buildCmd) + if err == nil || exitCode == 0 { + t.Fatalf("build should have failed: %s %s", err, out) + } + + // check if we've detected the failure before we started building + if !strings.Contains(out, "can't stat") { + t.Fatalf("output should've contained the string: can't access %s", out) + } + + if !strings.Contains(out, "Error checking context is accessible") { + t.Fatalf("output should've contained the string: Error checking context is accessible") + } + + } + { + // This is used to ensure we don't follow links when checking if everything in the context is accessible + // This test doesn't require that we run commands as an unprivileged user + pathToDirectoryWhichContainsLinks := filepath.Join(buildDirectory, "linksdirectory") + + buildCmd := exec.Command(dockerBinary, "build", "-t", "testlinksok", ".") + buildCmd.Dir = pathToDirectoryWhichContainsLinks + out, exitCode, err := runCommandWithOutput(buildCmd) + if err != nil || exitCode != 0 { + t.Fatalf("build should have worked: %s %s", err, out) + } + + deleteImages("testlinksok") + + } + deleteImages("inaccessiblefiles") + logDone("build - ADD from context with inaccessible files must fail") + logDone("build - ADD from context with accessible links must work") +} + // TODO: TestCaching // TODO: TestADDCacheInvalidation diff --git a/integration-cli/docker_test_vars.go b/integration-cli/docker_test_vars.go index f8bd5c116b..78e0685a9d 100644 --- a/integration-cli/docker_test_vars.go +++ b/integration-cli/docker_test_vars.go @@ -1,7 +1,9 @@ package main import ( + "fmt" "os" + "os/exec" ) // the docker binary to use @@ -18,6 +20,15 @@ var workingDirectory string func init() { if dockerBin := os.Getenv("DOCKER_BINARY"); dockerBin != "" { dockerBinary = dockerBin + } else { + whichCmd := exec.Command("which", "docker") + out, _, err := runCommandWithOutput(whichCmd) + if err == nil { + dockerBinary = stripTrailingCharacters(out) + } else { + fmt.Printf("ERROR: couldn't resolve full path to the Docker binary") + os.Exit(1) + } } if registryImage := os.Getenv("REGISTRY_IMAGE"); registryImage != "" { registryImageName = registryImage diff --git a/utils/utils.go b/utils/utils.go index 6e3a3c1b60..1a140f650d 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -1051,3 +1051,40 @@ func TreeSize(dir string) (size int64, err error) { }) return } + +// ValidateContextDirectory checks if all the contents of the directory +// can be read and returns an error if some files can't be read +// symlinks which point to non-existing files don't trigger an error +func ValidateContextDirectory(srcPath string) error { + var finalError error + + filepath.Walk(filepath.Join(srcPath, "."), func(filePath string, f os.FileInfo, err error) error { + // skip this directory/file if it's not in the path, it won't get added to the context + _, err = filepath.Rel(srcPath, filePath) + if err != nil && os.IsPermission(err) { + return nil + } + + if _, err := os.Stat(filePath); err != nil && os.IsPermission(err) { + finalError = fmt.Errorf("can't stat '%s'", filePath) + return err + } + // skip checking if symlinks point to non-existing files, such symlinks can be useful + lstat, _ := os.Lstat(filePath) + if lstat.Mode()&os.ModeSymlink == os.ModeSymlink { + return err + } + + if !f.IsDir() { + currentFile, err := os.Open(filePath) + if err != nil && os.IsPermission(err) { + finalError = fmt.Errorf("no permission to read from '%s'", filePath) + return err + } else { + currentFile.Close() + } + } + return nil + }) + return finalError +}