From 43a74261b32a97d64cd7a3948e612bc223e4de9a Mon Sep 17 00:00:00 2001
From: Erik Sipsma <erik@sipsma.dev>
Date: Thu, 18 Nov 2021 12:06:06 -0800
Subject: [PATCH] Remove symlink, not target, in FileOp.Rm.

Before this change, if the path provided to FileOp.Rm was a symlink then
the target of the symlink would be removed instead of the symlink
itself. Now, the symlink will be removed instead. However, any symlinks
present in the parent dirs of the specified path will still be resolved
before calling os.Remove; this change only results in the base of the
specified path not being followed.

Signed-off-by: Erik Sipsma <erik@sipsma.dev>
---
 client/client_test.go            | 36 ++++++++++++++++++++++++++++++++
 solver/llbsolver/file/backend.go |  8 ++++++-
 solver/pb/caps.go                |  7 +++++++
 3 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/client/client_test.go b/client/client_test.go
index a2d2dea72..74d62c12e 100644
--- a/client/client_test.go
+++ b/client/client_test.go
@@ -139,6 +139,7 @@ func TestIntegration(t *testing.T) {
 		testPullZstdImage,
 		testMergeOp,
 		testMergeOpCache,
+		testRmSymlink,
 	}, mirrors)
 
 	integration.Run(t, []integration.Test{
@@ -3824,6 +3825,41 @@ func testSourceMapFromRef(t *testing.T, sb integration.Sandbox) {
 	require.Equal(t, int32(1), srcs[0].Ranges[0].Start.Character)
 }
 
+func testRmSymlink(t *testing.T, sb integration.Sandbox) {
+	c, err := New(sb.Context(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	// Test that if FileOp.Rm is called on a symlink, then
+	// the symlink is removed rather than the target
+	mnt := llb.Image("alpine").
+		Run(llb.Shlex("touch /mnt/target")).
+		AddMount("/mnt", llb.Scratch())
+
+	mnt = llb.Image("alpine").
+		Run(llb.Shlex("ln -s target /mnt/link")).
+		AddMount("/mnt", mnt)
+
+	def, err := mnt.File(llb.Rm("link")).Marshal(sb.Context())
+	require.NoError(t, err)
+
+	destDir, err := ioutil.TempDir("", "buildkit")
+	require.NoError(t, err)
+	defer os.RemoveAll(destDir)
+
+	_, err = c.Solve(sb.Context(), def, SolveOpt{
+		Exports: []ExportEntry{
+			{
+				Type:      ExporterLocal,
+				OutputDir: destDir,
+			},
+		},
+	}, nil)
+	require.NoError(t, err)
+
+	require.NoError(t, fstest.CheckDirectoryEqualWithApplier(destDir, fstest.CreateFile("target", nil, 0644)))
+}
+
 func testProxyEnv(t *testing.T, sb integration.Sandbox) {
 	c, err := New(sb.Context(), sb.Address())
 	require.NoError(t, err)
diff --git a/solver/llbsolver/file/backend.go b/solver/llbsolver/file/backend.go
index 0060ad3fc..732e67474 100644
--- a/solver/llbsolver/file/backend.go
+++ b/solver/llbsolver/file/backend.go
@@ -146,10 +146,16 @@ func rm(ctx context.Context, d string, action pb.FileActionRm) error {
 }
 
 func rmPath(root, src string, allowNotFound bool) error {
-	p, err := fs.RootPath(root, filepath.Join("/", src))
+	src = filepath.Clean(src)
+	dir, base := filepath.Split(src)
+	if base == "" {
+		return errors.New("rmPath: invalid empty path")
+	}
+	dir, err := fs.RootPath(root, filepath.Join("/", dir))
 	if err != nil {
 		return err
 	}
+	p := filepath.Join(dir, base)
 
 	if err := os.RemoveAll(p); err != nil {
 		if errors.Is(err, os.ErrNotExist) && allowNotFound {
diff --git a/solver/pb/caps.go b/solver/pb/caps.go
index 5847b1754..a0d54ee16 100644
--- a/solver/pb/caps.go
+++ b/solver/pb/caps.go
@@ -57,6 +57,7 @@ const (
 	CapFileBase                       apicaps.CapID = "file.base"
 	CapFileRmWildcard                 apicaps.CapID = "file.rm.wildcard"
 	CapFileCopyIncludeExcludePatterns apicaps.CapID = "file.copy.includeexcludepatterns"
+	CapFileRmNoFollowSymlink          apicaps.CapID = "file.rm.nofollowsymlink"
 
 	CapConstraints apicaps.CapID = "constraints"
 	CapPlatform    apicaps.CapID = "platform"
@@ -328,6 +329,12 @@ func init() {
 		Status:  apicaps.CapStatusExperimental,
 	})
 
+	Caps.Init(apicaps.Cap{
+		ID:      CapFileRmNoFollowSymlink,
+		Enabled: true,
+		Status:  apicaps.CapStatusExperimental,
+	})
+
 	Caps.Init(apicaps.Cap{
 		ID:      CapFileCopyIncludeExcludePatterns,
 		Enabled: true,