From ce439567a41a29f7b8da7c895c83957575efb28b Mon Sep 17 00:00:00 2001
From: Jakub Ciolek <jakub@ciolek.dev>
Date: Sun, 23 Jul 2023 15:18:37 +0200
Subject: [PATCH] file: Fix incorrect handling of non-existent files in
 llbsolver's rmPath

The os.RemoveAll() call returns nil if the path doesn't exist. When the
rmPath function is called with allowNotFound set to false, it doesn't change the
behaviour of the function.

Change the code so if allowNotFound is set to false, we first check
whether the file exists. If it doesn't exist, return an error.

Add tests for three relevant cases.

Signed-off-by: Jakub Ciolek <jakub@ciolek.dev>
---
 solver/llbsolver/file/backend.go      | 11 ++++----
 solver/llbsolver/file/backend_test.go | 36 +++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 5 deletions(-)
 create mode 100644 solver/llbsolver/file/backend_test.go

diff --git a/solver/llbsolver/file/backend.go b/solver/llbsolver/file/backend.go
index 6212066cd..f83593b79 100644
--- a/solver/llbsolver/file/backend.go
+++ b/solver/llbsolver/file/backend.go
@@ -161,14 +161,15 @@ func rmPath(root, src string, allowNotFound bool) error {
 	}
 	p := filepath.Join(dir, base)
 
-	if err := os.RemoveAll(p); err != nil {
-		if errors.Is(err, os.ErrNotExist) && allowNotFound {
-			return nil
+	if !allowNotFound {
+		_, err := os.Stat(p)
+
+		if errors.Is(err, os.ErrNotExist) {
+			return err
 		}
-		return err
 	}
 
-	return nil
+	return os.RemoveAll(p)
 }
 
 func docopy(ctx context.Context, src, dest string, action pb.FileActionCopy, u *copy.User, idmap *idtools.IdentityMapping) error {
diff --git a/solver/llbsolver/file/backend_test.go b/solver/llbsolver/file/backend_test.go
new file mode 100644
index 000000000..b13df1ef7
--- /dev/null
+++ b/solver/llbsolver/file/backend_test.go
@@ -0,0 +1,36 @@
+package file
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestRmPathNonExistentFileAllowNotFoundFalse(t *testing.T) {
+	root := t.TempDir()
+	err := rmPath(root, "doesnt_exist", false)
+	require.Error(t, err)
+	require.True(t, os.IsNotExist(err))
+}
+
+func TestRmPathNonExistentFileAllowNotFoundTrue(t *testing.T) {
+	root := t.TempDir()
+	require.NoError(t, rmPath(root, "doesnt_exist", true))
+}
+
+func TestRmPathFileExists(t *testing.T) {
+	root := t.TempDir()
+
+	src := filepath.Join(root, "exists")
+	file, err := os.Create(src)
+	require.NoError(t, err)
+	file.Close()
+
+	require.NoError(t, rmPath(root, "exists", false))
+
+	_, err = os.Stat(src)
+
+	require.True(t, os.IsNotExist(err))
+}