fix: ignore file ownership for copy from context

.github/workflows/unit-tests.yaml

@@ -2,9 +2,9 @@ name: Unit tests
 
 on:
   push:
-    branches: ['main']
+    branches: ["main"]
   pull_request:
-    branches: ['main']
+    branches: ["main"]
 
 permissions:
   contents: read
@@ -13,8 +13,10 @@ jobs:
   tests:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/setup-go@0c52d547c9bc32b1aa3301fd7a9cb496313a4491 # v4.01
-      with:
-        go-version: '1.22'
-    - uses: actions/checkout@b0e28b5ac45a892f91e7d036f8200cf5ed489415 # v3
-    - run: make test
+      - uses: actions/setup-go@0c52d547c9bc32b1aa3301fd7a9cb496313a4491 # v4.01
+        with:
+          go-version: "1.22"
+      - uses: actions/checkout@b0e28b5ac45a892f91e7d036f8200cf5ed489415 # v3
+      # Some unit tests need to be run as root to function correctly as they
+      # may attempt to run `chown`.
+      - run: sudo make test

pkg/commands/add.go

@@ -56,7 +56,7 @@ func (a *AddCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui
 		chmod = fs.FileMode(0o600)
 	}
 
-	uid, gid, err := util.GetUserGroup(a.cmd.Chown, replacementEnvs)
+	uid, gid, err := util.GetUserGroup(a.cmd.Chown, replacementEnvs, "")
 	if err != nil {
 		return errors.Wrap(err, "getting user group from chown")
 	}

pkg/commands/add_test.go

@@ -123,6 +123,9 @@ func setupAddTest(t *testing.T) string {
 }
 
 func Test_AddCommand(t *testing.T) {
+	if os.Getuid() != 0 {
+		t.Skip("Test requires root privileges as it attempts to chown")
+	}
 	tempDir := setupAddTest(t)
 
 	fileContext := util.FileContext{Root: tempDir}

pkg/commands/copy.go

@@ -53,7 +53,7 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu
 	}
 
 	replacementEnvs := buildArgs.ReplacementEnvs(config.Env)
-	uid, gid, err := getUserGroup(c.cmd.Chown, replacementEnvs)
+	uid, gid, err := getUserGroup(c.cmd.Chown, replacementEnvs, c.From())
 	logrus.Debugf("found uid %v and gid %v for chown string %v", uid, gid, c.cmd.Chown)
 	if err != nil {
 		return errors.Wrap(err, "getting user group from chown")

pkg/commands/copy_test.go

@@ -296,6 +296,9 @@ func Test_CachingCopyCommand_ExecuteCommand(t *testing.T) {
 }
 
 func TestCopyExecuteCmd(t *testing.T) {
+	if os.Getuid() != 0 {
+		t.Skip("Test requires root privileges as it attempts to chown")
+	}
 	tempDir := setupTestTemp(t)
 
 	cfg := &v1.Config{
@@ -431,6 +434,9 @@ func Test_resolveIfSymlink(t *testing.T) {
 }
 
 func Test_CopyEnvAndWildcards(t *testing.T) {
+	if os.Getuid() != 0 {
+		t.Skip("Test requires root privileges as it attempts to chown")
+	}
 	setupDirs := func(t *testing.T) (string, string) {
 		testDir := t.TempDir()
 
@@ -495,6 +501,9 @@ func Test_CopyEnvAndWildcards(t *testing.T) {
 }
 
 func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) {
+	if os.Getuid() != 0 {
+		t.Skip("Test requires root privileges as it attempts to chown")
+	}
 	setupDirs := func(t *testing.T) (string, string) {
 		testDir := t.TempDir()
 
@@ -1006,7 +1015,7 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) {
 		uid := os.Getuid()
 		gid := os.Getgid()
 
-		getUserGroup = func(userStr string, _ []string) (int64, int64, error) {
+		getUserGroup = func(userStr string, _ []string, _ string) (int64, int64, error) {
 			return int64(uid), int64(gid), nil
 		}
 
@@ -1051,7 +1060,7 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) {
 		original := getUserGroup
 		defer func() { getUserGroup = original }()
 
-		getUserGroup = func(userStr string, _ []string) (int64, int64, error) {
+		getUserGroup = func(userStr string, _ []string, _ string) (int64, int64, error) {
 			return 12345, 12345, nil
 		}
 

pkg/commands/workdir.go

@@ -65,7 +65,7 @@ func (w *WorkdirCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile
 
 		if config.User != "" {
 			logrus.Debugf("Fetching uid and gid for USER '%s'", config.User)
-			uid, gid, err = util.GetUserGroup(config.User, replacementEnvs)
+			uid, gid, err = util.GetUserGroup(config.User, replacementEnvs, "")
 			if err != nil {
 				return errors.Wrapf(err, "identifying uid and gid for user %s", config.User)
 			}

pkg/executor/build.go

@@ -227,11 +227,18 @@ func (s *stageBuilder) populateCompositeKey(command commands.DockerCommand, file
 		}
 	}
 
+	var addPathOptions []AddPathOption
+	if f, ok := command.(interface{ From() string }); ok {
+		if f.From() == "" {
+			addPathOptions = append(addPathOptions, IgnoreOwnerAndGroup())
+		}
+	}
+
 	// Add the next command to the cache key.
 	compositeKey.AddKey(command.String())
 
 	for _, f := range files {
-		if err := compositeKey.AddPath(f, s.fileContext); err != nil {
+		if err := compositeKey.AddPath(f, s.fileContext, addPathOptions...); err != nil {
 			return compositeKey, err
 		}
 	}

pkg/executor/build_test.go

@@ -20,6 +20,7 @@ import (
 	"archive/tar"
 	"bytes"
 	"fmt"
+	"os"
 	"path/filepath"
 	"reflect"
 	"sort"
@@ -948,6 +949,7 @@ func Test_stageBuilder_build(t *testing.T) {
 		crossStageDeps     map[int][]string
 		mockGetFSFromImage func(root string, img v1.Image, extract util.ExtractFunction) ([]string, error)
 		shouldInitSnapshot bool
+		skipReason         string
 	}
 
 	testCases := []testcase{
@@ -1136,6 +1138,13 @@ func Test_stageBuilder_build(t *testing.T) {
 			}
 		}(),
 		func() testcase {
+			name := "copy command cache enabled and key is not in cache"
+			if os.Getuid() != 0 {
+				return testcase{
+					description: name,
+					skipReason:  "test requires root, attempts chown",
+				}
+			}
 			dir, filenames := tempDirAndFile(t)
 			filename := filenames[0]
 			tarContent := []byte{}
@@ -1174,7 +1183,7 @@ COPY %s foo.txt
 
 			cmds := stage.Commands
 			return testcase{
-				description: "copy command cache enabled and key is not in cache",
+				description: name,
 				opts:        opts,
 				config:      &v1.ConfigFile{Config: v1.Config{WorkingDir: destDir}},
 				layerCache:  &fakeLayerCache{},
@@ -1193,6 +1202,13 @@ COPY %s foo.txt
 			}
 		}(),
 		func() testcase {
+			name := "cached run command followed by uncached copy command results in consistent read and write hashes"
+			if os.Getuid() != 0 {
+				return testcase{
+					description: name,
+					skipReason:  "test requires root, attempts chown",
+				}
+			}
 			dir, filenames := tempDirAndFile(t)
 			filename := filenames[0]
 			tarContent := generateTar(t, filename)
@@ -1251,7 +1267,7 @@ COPY %s bar.txt
 
 			cmds := stage.Commands
 			return testcase{
-				description: "cached run command followed by uncached copy command results in consistent read and write hashes",
+				description: name,
 				opts:        &config.KanikoOptions{Cache: true, CacheCopyLayers: true, CacheRunLayers: true},
 				rootDir:     dir,
 				config:      &v1.ConfigFile{Config: v1.Config{WorkingDir: destDir}},
@@ -1267,6 +1283,14 @@ COPY %s bar.txt
 			}
 		}(),
 		func() testcase {
+			name := "copy command followed by cached run command results in consistent read and write hashes"
+			if os.Getuid() != 0 {
+				return testcase{
+					description: name,
+					skipReason:  "test requires root, attempts chown",
+				}
+			}
+
 			dir, filenames := tempDirAndFile(t)
 			filename := filenames[0]
 			tarContent := generateTar(t, filename)
@@ -1325,7 +1349,7 @@ RUN foobar
 
 			cmds := stage.Commands
 			return testcase{
-				description: "copy command followed by cached run command results in consistent read and write hashes",
+				description: name,
 				opts:        &config.KanikoOptions{Cache: true, CacheRunLayers: true},
 				rootDir:     dir,
 				config:      &v1.ConfigFile{Config: v1.Config{WorkingDir: destDir}},
@@ -1471,6 +1495,9 @@ RUN foobar
 	}
 	for _, tc := range testCases {
 		t.Run(tc.description, func(t *testing.T) {
+			if tc.skipReason != "" {
+				t.Skip(tc.skipReason)
+			}
 			var fileName string
 			if tc.commands == nil {
 				file, err := filesystem.CreateTemp("", "foo")

pkg/executor/cache_probe_test.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"net/http/httptest"
 	"net/url"
+	"os"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -34,6 +35,9 @@ import (
 )
 
 func TestDoCacheProbe(t *testing.T) {
+	if os.Getuid() != 0 {
+		t.Skip("Test setup requires root privileges as it attempts to chown")
+	}
 	t.Run("Empty", func(t *testing.T) {
 		testDir, fn := setupCacheProbeTests(t)
 		defer fn()

pkg/executor/composite_cache.go

@@ -55,15 +55,31 @@ func (s *CompositeCache) Hash() (string, error) {
 	return util.SHA256(strings.NewReader(s.Key()))
 }
 
-func (s *CompositeCache) AddPath(p string, context util.FileContext) error {
+type addPathOptions struct {
+	ignoreOwnerAndGroup bool
+}
+
+type AddPathOption func(*addPathOptions)
+
+func IgnoreOwnerAndGroup() func(*addPathOptions) {
+	return func(o *addPathOptions) {
+		o.ignoreOwnerAndGroup = true
+	}
+}
+
+func (s *CompositeCache) AddPath(p string, context util.FileContext, fopts ...AddPathOption) error {
+	var opts addPathOptions
+	for _, f := range fopts {
+		f(&opts)
+	}
 	sha := sha256.New()
 	fi, err := filesystem.FS.Lstat(p)
 	if err != nil {
 		return errors.Wrap(err, "could not add path")
 	}
 
 	if fi.Mode().IsDir() {
-		empty, k, err := hashDir(p, context)
+		empty, k, err := hashDir(opts.ignoreOwnerAndGroup, p, context)
 		if err != nil {
 			return err
 		}
@@ -79,7 +95,7 @@ func (s *CompositeCache) AddPath(p string, context util.FileContext) error {
 	if context.ExcludesFile(p) {
 		return nil
 	}
-	fh, err := util.CacheHasher()(p)
+	fh, err := util.CacheHasher(opts.ignoreOwnerAndGroup)(p)
 	if err != nil {
 		return err
 	}
@@ -92,7 +108,7 @@ func (s *CompositeCache) AddPath(p string, context util.FileContext) error {
 }
 
 // HashDir returns a hash of the directory.
-func hashDir(p string, context util.FileContext) (bool, string, error) {
+func hashDir(ignoreOwnerAndGroup bool, p string, context util.FileContext) (bool, string, error) {
 	sha := sha256.New()
 	empty := true
 	if err := filesystem.Walk(p, func(path string, fi os.FileInfo, err error) error {
@@ -104,7 +120,7 @@ func hashDir(p string, context util.FileContext) (bool, string, error) {
 			return nil
 		}
 
-		fileHash, err := util.CacheHasher()(path)
+		fileHash, err := util.CacheHasher(ignoreOwnerAndGroup)(path)
 		if err != nil {
 			return err
 		}

pkg/executor/copy_multistage_test.go

@@ -48,6 +48,9 @@ func readDirectory(dirName string) ([]fs.FileInfo, error) {
 }
 
 func TestCopyCommand_Multistage(t *testing.T) {
+	if os.Getuid() != 0 {
+		t.Skip("Test requires root privileges as it attempts to chown")
+	}
 	t.Run("copy a file across multistage", func(t *testing.T) {
 		testDir, fn := setupMultistageTests(t)
 		defer fn()

pkg/util/command_util.go

@@ -354,9 +354,17 @@ Loop:
 	return nil
 }
 
-func GetUserGroup(chownStr string, env []string) (int64, int64, error) {
+func GetUserGroup(chownStr string, env []string, from string) (int64, int64, error) {
+	// All files and directories copied from the build context are created with a
+	// UID and GID of 0 unless the optional --chown flag specifies a given
+	// username, groupname, or UID/GID combination to request specific ownership
+	// of the copied content.
 	if chownStr == "" {
-		return DoNotChangeUID, DoNotChangeGID, nil
+		if from == "" {
+			return 0, 0, nil
+		} else {
+			return DoNotChangeUID, DoNotChangeGID, nil
+		}
 	}
 
 	chown, err := ResolveEnvironmentReplacement(chownStr, env, false)

pkg/util/command_util_test.go

@@ -536,6 +536,7 @@ func TestGetUserGroup(t *testing.T) {
 		description  string
 		chown        string
 		env          []string
+		from         string
 		mockIDGetter func(userStr string, groupStr string) (uint32, uint32, error)
 		// needed, in case uid is a valid number, but group is a name
 		mockGroupIDGetter func(groupStr string) (*user.Group, error)
@@ -571,6 +572,16 @@ func TestGetUserGroup(t *testing.T) {
 			mockIDGetter: func(string, string) (uint32, uint32, error) {
 				return 0, 0, fmt.Errorf("should not be called")
 			},
+			expectedU: 0,
+			expectedG: 0,
+		},
+		{
+			description: "empty chown and non-empty from",
+			chown:       "",
+			from:        "foo",
+			mockIDGetter: func(string, string) (uint32, uint32, error) {
+				return 100, 1000, nil
+			},
 			expectedU: -1,
 			expectedG: -1,
 		},
@@ -582,7 +593,7 @@ func TestGetUserGroup(t *testing.T) {
 				getUIDAndGIDFunc = originalIDGetter
 			}()
 			getUIDAndGIDFunc = tc.mockIDGetter
-			uid, gid, err := GetUserGroup(tc.chown, tc.env)
+			uid, gid, err := GetUserGroup(tc.chown, tc.env, tc.from)
 			testutil.CheckErrorAndDeepEqual(t, tc.shdErr, err, uid, tc.expectedU)
 			testutil.CheckErrorAndDeepEqual(t, tc.shdErr, err, gid, tc.expectedG)
 		})