From 7532420f3b4f7c62fb6c0de3c92b24ad91c380a4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= <pawel.gronowski@docker.com>
Date: Tue, 16 Apr 2024 13:42:56 +0200
Subject: [PATCH] container/SetupWorkingDirectory: Don't mutate config
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Don't mutate the container's `Config.WorkingDir` permanently with a
cleaned path when creating a working directory.

Move the `filepath.Clean` to the `translateWorkingDir` instead.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
---
 container/container.go                  |  4 +-
 daemon/container.go                     |  2 +-
 integration/build/build_test.go         | 66 +++++++++++++++++++++++++
 integration/container/run_linux_test.go | 30 +++++++++++
 4 files changed, 99 insertions(+), 3 deletions(-)

diff --git a/container/container.go b/container/container.go
index 018300350d..9809124d8e 100644
--- a/container/container.go
+++ b/container/container.go
@@ -299,8 +299,8 @@ func (container *Container) SetupWorkingDirectory(rootIdentity idtools.Identity)
 		return nil
 	}
 
-	container.Config.WorkingDir = filepath.Clean(container.Config.WorkingDir)
-	pth, err := container.GetResourcePath(container.Config.WorkingDir)
+	workdir := filepath.Clean(container.Config.WorkingDir)
+	pth, err := container.GetResourcePath(workdir)
 	if err != nil {
 		return err
 	}
diff --git a/daemon/container.go b/daemon/container.go
index c86ecf8c98..f8df74e4be 100644
--- a/daemon/container.go
+++ b/daemon/container.go
@@ -364,6 +364,6 @@ func translateWorkingDir(config *containertypes.Config) error {
 	if !system.IsAbs(wd) {
 		return fmt.Errorf("the working directory '%s' is invalid, it needs to be an absolute path", config.WorkingDir)
 	}
-	config.WorkingDir = wd
+	config.WorkingDir = filepath.Clean(wd)
 	return nil
 }
diff --git a/integration/build/build_test.go b/integration/build/build_test.go
index 826f4d5eb4..8d3da03a96 100644
--- a/integration/build/build_test.go
+++ b/integration/build/build_test.go
@@ -689,6 +689,72 @@ func TestBuildPlatformInvalid(t *testing.T) {
 	assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter))
 }
 
+// TestBuildWorkdirNoCacheMiss is a regression test for https://github.com/moby/moby/issues/47627
+func TestBuildWorkdirNoCacheMiss(t *testing.T) {
+	ctx := setupTest(t)
+
+	for _, tc := range []struct {
+		name       string
+		dockerfile string
+	}{
+		{name: "trailing slash", dockerfile: "FROM busybox\nWORKDIR /foo/"},
+		{name: "no trailing slash", dockerfile: "FROM busybox\nWORKDIR /foo"},
+	} {
+		dockerfile := tc.dockerfile
+		t.Run(tc.name, func(t *testing.T) {
+			source := fakecontext.New(t, "", fakecontext.WithDockerfile(dockerfile))
+			defer source.Close()
+
+			apiClient := testEnv.APIClient()
+
+			buildAndGetID := func() string {
+				resp, err := apiClient.ImageBuild(ctx, source.AsTarReader(t), types.ImageBuildOptions{
+					Version: types.BuilderV1,
+				})
+				assert.NilError(t, err)
+				defer resp.Body.Close()
+
+				id := readBuildImageIDs(t, resp.Body)
+				assert.Check(t, id != "")
+				return id
+			}
+
+			firstId := buildAndGetID()
+			secondId := buildAndGetID()
+
+			assert.Check(t, is.Equal(firstId, secondId), "expected cache to be used")
+		})
+	}
+}
+
+func readBuildImageIDs(t *testing.T, rd io.Reader) string {
+	t.Helper()
+	decoder := json.NewDecoder(rd)
+	for {
+		var jm jsonmessage.JSONMessage
+		if err := decoder.Decode(&jm); err != nil {
+			if err == io.EOF {
+				break
+			}
+			assert.NilError(t, err)
+		}
+
+		if jm.Aux == nil {
+			continue
+		}
+
+		var auxId struct {
+			ID string `json:"ID"`
+		}
+
+		if json.Unmarshal(*jm.Aux, &auxId); auxId.ID != "" {
+			return auxId.ID
+		}
+	}
+
+	return ""
+}
+
 func writeTarRecord(t *testing.T, w *tar.Writer, fn, contents string) {
 	err := w.WriteHeader(&tar.Header{
 		Name:     fn,
diff --git a/integration/container/run_linux_test.go b/integration/container/run_linux_test.go
index 810ede184b..867544705d 100644
--- a/integration/container/run_linux_test.go
+++ b/integration/container/run_linux_test.go
@@ -326,3 +326,33 @@ func TestStaticIPOutsideSubpool(t *testing.T) {
 
 	assert.Check(t, is.Contains(b.String(), "inet 10.42.1.3/16"))
 }
+
+func TestWorkingDirNormalization(t *testing.T) {
+	ctx := setupTest(t)
+	apiClient := testEnv.APIClient()
+
+	for _, tc := range []struct {
+		name    string
+		workdir string
+	}{
+		{name: "trailing slash", workdir: "/tmp/"},
+		{name: "no trailing slash", workdir: "/tmp"},
+	} {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			cID := container.Run(ctx, t, apiClient,
+				container.WithImage("busybox"),
+				container.WithWorkingDir(tc.workdir),
+			)
+
+			defer container.Remove(ctx, t, apiClient, cID, containertypes.RemoveOptions{Force: true})
+
+			inspect := container.Inspect(ctx, t, apiClient, cID)
+
+			assert.Check(t, is.Equal(inspect.Config.WorkingDir, "/tmp"))
+		})
+
+	}
+}