1599: recreate fifos and skip other special files during COPY

README.md

@@ -138,6 +138,7 @@ expect - see [Known Issues](#known-issues).
       - [Flag `FF_KANIKO_VOLUME_SKIP_MKDIR`](#flag-ff_kaniko_volume_skip_mkdir)
       - [Flag `FF_KANIKO_PRESERVE_HARDLINKS`](#flag-ff_kaniko_preserve_hardlinks)
       - [Flag `FF_KANIKO_RELATIVE_LINK_TARGETS`](#flag-ff_kaniko_relative_link_targets)
+      - [Flag `FF_KANIKO_COPY_SKIP_SPECIAL_FILES`](#flag-ff_kaniko_copy_skip_special_files)
       - [Flag `FF_KANIKO_SKIP_WRITE_WHITEOUTS`](#flag-ff_kaniko_skip_write_whiteouts)
       - [Flag `FF_KANIKO_BUILDKIT_ARG_ENV_PRECEDENCE`](#flag-ff_kaniko_buildkit_arg_env_precedence)
       - [Flag `FF_KANIKO_INFER_CROSS_STAGE_CACHE_KEY`](#flag-ff_kaniko_infer_cross_stage_cache_key)
@@ -1280,6 +1281,12 @@ Set this flag to `true` to write hardlink targets relative to the tar root.
 Defaults to `true`.
 Will be deprecated in `v1.29.0`.
 
+#### Flag `FF_KANIKO_COPY_SKIP_SPECIAL_FILES`
+
+`COPY` reads each source file to copy it, which is wrong for anything that is not a regular file. A socket fails the build with `ENXIO`, and a block or character device is read as if it were a file, baking its contents into the image in place of the device node.
+Set this flag to `true` to skip both with a warning instead. Fifos are always recreated with `mkfifo` regardless of this flag, since opening one hung the build outright. Defaults to `false`.
+Becomes default in `v1.29.0`.
+
 #### Flag `FF_KANIKO_SKIP_WRITE_WHITEOUTS`
 
 When kaniko extracts a cached layer it applies the layer's whiteouts by deleting the target files, but it also writes the `.wh.<name>` marker files onto the working filesystem. With `--cache-copy-layers` a later cross-stage `COPY --from=<stage>` copies such a marker verbatim and commits it as a real whiteout, so a cache-hit build deletes a file that the cache-miss build kept.

integration/dockerfiles/Dockerfile_test_issue_1599

@@ -0,0 +1,24 @@
+# 1599: COPY of a fifo hangs forever on v1.28.1. Source files are copied by
+# opening and reading them, and open(2) on a fifo blocks until a writer shows
+# up, so the build never terminates and never times out. Covers the fifo inside
+# a copied directory, named directly as the source, and with --chown/--chmod.
+ARG IMAGE_REPO
+FROM ${IMAGE_REPO}busybox AS builder
+RUN mkdir /work \
+    && echo "content" > /work/regular.txt \
+    && mkfifo -m 640 /work/pipe
+
+FROM ${IMAGE_REPO}busybox
+# fifo inside a copied directory
+COPY --from=builder /work/ /work/
+RUN test -p /work/pipe
+RUN test "$(stat -c %a /work/pipe)" = "640"
+RUN grep -q 'content' /work/regular.txt
+# fifo named directly as the copy source
+COPY --from=builder /work/pipe /direct/pipe
+RUN test -p /direct/pipe
+# --chown and --chmod must apply to a fifo like any other file
+COPY --chown=1000:1000 --chmod=600 --from=builder /work/pipe /owned/pipe
+RUN test -p /owned/pipe
+RUN test "$(stat -c %u:%g /owned/pipe)" = "1000:1000"
+RUN test "$(stat -c %a /owned/pipe)" = "600"

integration/dockerfiles/Dockerfile_test_issue_1599_2

@@ -0,0 +1,13 @@
+# 1599: a fifo COPY must respect the kaniko directory guard like any other COPY.
+# CreateFifo removes the destination before calling mkfifo, and the fifo branch
+# skipped the check that refuses copies into /kaniko. A fifo copied over
+# /kaniko/tini replaces the init every RUN execs through, so the build dies on
+# the next RUN instead of skipping the copy with a warning.
+# Breaks on the fifo COPY path added in PR #948.
+ARG IMAGE_REPO
+FROM ${IMAGE_REPO}busybox AS builder
+RUN mkfifo /pipe
+
+FROM ${IMAGE_REPO}busybox
+COPY --from=builder /pipe /kaniko/tini
+RUN echo alive

integration/images.go

@@ -132,6 +132,7 @@ var KanikoEnv = []string{
 	"FF_KANIKO_PATH_SCOPED_REGISTRY_AUTH=1",
 	"FF_KANIKO_DEPRECATE_LAYERLESS_CACHE_ENTRIES=1",
 	"FF_KANIKO_PLATFORM_CACHE_KEY=1",
+	"FF_KANIKO_COPY_SKIP_SPECIAL_FILES=1",
 	"KANIKO_PRINT_PLAN=1",
 	"KANIKO_TELEMETRY_ENDPOINT",
 	"OTEL_EXPORTER_OTLP_HEADERS",
@@ -301,6 +302,9 @@ var diffArgsMap = map[string][]string{
 	// But we discovered a new issue with this. For builtins, buildkit will emit "whiteout" files,
 	// to remember that it was removed, we don't. So we end up with a diff in the resulting image.
 	"TestRun/test_Dockerfile_test_issue_mz511": {"--extra-ignore-files=etc/.wh.nsswitch.conf", "--extra-ignore-layer-length-mismatch"},
+	// 1599: docker copies the fifo into /kaniko, we refuse to write there at all,
+	// so the docker image keeps a /kaniko/tini our image never has.
+	"TestRun/test_Dockerfile_test_issue_1599_2": {"--extra-ignore-files=kaniko/", "--extra-ignore-layer-length-mismatch"},
 	// mz793: with FF_KANIKO_VOLUME_SKIP_MKDIR off, VOLUME creates the directory fresh on
 	// each build, so its mtime differs between the two cached builds. That divergence is the
 	// known volume non-determinism the flag fixes, here we only assert the build no longer panics.
@@ -415,6 +419,8 @@ var expectedWarnings = map[string]string{
 	// mz991: the repro needs a MAINTAINER, which warns twice, once from the buildkit
 	// linter and once from kaniko skipping the command. Both lines say "is deprecated".
 	"Dockerfile_test_issue_mz991": "is deprecated",
+	// 1599: a fifo COPY into /kaniko must warn and be skipped like any other file.
+	"Dockerfile_test_issue_1599_2": "Skipping copy targeting kaniko directory",
 }
 
 func checkNoWarnings(dockerfile string, out []byte) error {

pkg/commands/copy.go

@@ -127,6 +127,19 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu
 				continue
 			}
 			c.snapshotFiles = append(c.snapshotFiles, destPath)
+		} else if fi.Mode()&os.ModeNamedPipe != 0 {
+			// Opening a fifo blocks until it has a writer, so recreate it instead.
+			exclude, err := util.CreateFifo(fullPath, destPath, fi, c.fileContext, uid, gid, chmod, useDefaultChmod)
+			if err != nil {
+				return fmt.Errorf("copying fifo: %w", err)
+			}
+			if exclude {
+				continue
+			}
+			c.snapshotFiles = append(c.snapshotFiles, destPath)
+		} else if !fi.Mode().IsRegular() && kConfig.FF.CopySkipSpecialFiles {
+			logrus.Warnf("Ignoring special file %s, not copying to %s", fullPath, destPath)
+			continue
 		} else {
 			// ... Else, we want to copy over a file
 			exclude, err := util.CopyFile(fullPath, destPath, c.fileContext, uid, gid, chmod, useDefaultChmod)

pkg/config/featureflags.go

@@ -34,6 +34,7 @@ type FeatureFlags struct {
 	CleanKanikoDir                 bool
 	CopyAsRoot                     bool
 	CopyChmodOnImplicitDirs        bool
+	CopySkipSpecialFiles           bool
 	CrossRepoMount                 bool
 	DeprecateInterStageRestore     bool
 	DeprecateLayerlessCacheEntries bool
@@ -108,6 +109,7 @@ func InitFeatureFlags() {
 		CleanKanikoDir:                 featureFlag("FF_KANIKO_CLEAN_KANIKO_DIR", true),
 		CopyAsRoot:                     featureFlag("FF_KANIKO_COPY_AS_ROOT", false),
 		CopyChmodOnImplicitDirs:        featureFlag("FF_KANIKO_COPY_CHMOD_ON_IMPLICIT_DIRS", false),
+		CopySkipSpecialFiles:           featureFlag("FF_KANIKO_COPY_SKIP_SPECIAL_FILES", false),
 		CrossRepoMount:                 featureFlag("FF_KANIKO_CROSS_REPO_MOUNT", false),
 		DeprecateInterStageRestore:     featureFlag("FF_KANIKO_DEPRECATE_INTER_STAGE_RESTORE", true),
 		DeprecateLayerlessCacheEntries: featureFlag("FF_KANIKO_DEPRECATE_LAYERLESS_CACHE_ENTRIES", false),

pkg/util/fs_util.go

@@ -37,6 +37,7 @@ import (
 	"github.com/moby/go-archive"
 	"github.com/moby/patternmatcher"
 	"github.com/moby/patternmatcher/ignorefile"
+	"github.com/osscontainertools/kaniko/pkg/assert"
 	"github.com/osscontainertools/kaniko/pkg/config"
 	"github.com/osscontainertools/kaniko/pkg/timing"
 	otiai10Cpy "github.com/otiai10/copy"
@@ -825,21 +826,38 @@ func CopyDir(src, dest string, context FileContext, uid, gid int64, chmod mode.S
 			}
 		} else if IsSymlink(fi) {
 			// If file is a symlink, we want to create the same relative symlink
-			if _, err := CopySymlink(fullPath, destPath, context); err != nil {
+			exclude, err := CopySymlink(fullPath, destPath, context)
+			if err != nil {
 				return nil, err
 			}
+			// This loop already skipped matches
+			assert.Assert("util.copydir.symlink-not-excluded", !exclude, "CopySymlink refused to copy %s to %s", fullPath, destPath)
 		} else if linkDst, ok := checkCopyHardlink(fi, destPath, hardlinksSeen); ok && config.FF.PreserveHardlinks {
 			// #2594: inode already copied — create a hardlink instead of duplicating content.
 			logrus.Tracef("Creating hardlink %s -> %s", destPath, linkDst)
 			if err := os.Link(linkDst, destPath); err != nil {
 				return nil, err
 			}
 			isHardlink = true
+		} else if fi.Mode()&os.ModeNamedPipe != 0 {
+			// Opening a fifo blocks until it has a writer, so recreate it instead.
+			exclude, err := CreateFifo(fullPath, destPath, fi, context, uid, gid, chmod, useDefaultChmod)
+			if err != nil {
+				return nil, err
+			}
+			// This loop already skipped matches
+			assert.Assert("util.copydir.fifo-not-excluded", !exclude, "CreateFifo refused to copy %s to %s", fullPath, destPath)
+		} else if !fi.Mode().IsRegular() && config.FF.CopySkipSpecialFiles {
+			logrus.Warnf("Ignoring special file %s, not copying to %s", fullPath, destPath)
+			continue
 		} else {
 			// ... Else, we want to copy over a file
-			if _, err := CopyFile(fullPath, destPath, context, uid, gid, chmod, useDefaultChmod); err != nil {
+			exclude, err := CopyFile(fullPath, destPath, context, uid, gid, chmod, useDefaultChmod)
+			if err != nil {
 				return nil, err
 			}
+			// This loop already skipped matches
+			assert.Assert("util.copydir.file-not-excluded", !exclude, "CopyFile refused to copy %s to %s", fullPath, destPath)
 		}
 		if !IsSymlink(fi) && !isHardlink {
 			updates = append(updates, timestampUpdate{src: fullPath, dest: destPath})
@@ -855,6 +873,47 @@ func CopyDir(src, dest string, context FileContext, uid, gid int64, chmod mode.S
 	return copiedFiles, nil
 }
 
+// CreateFifo recreates the fifo at src as dest. Opening a fifo blocks until it
+// has a writer, so it can never be copied by reading it.
+func CreateFifo(src, dest string, fi os.FileInfo, context FileContext, uid, gid int64, chmod mode.Set, useDefaultChmod bool) (bool, error) {
+	if context.ExcludesFile(src) {
+		logrus.Debugf("%s found in .dockerignore, ignoring", src)
+		return true, nil
+	}
+	if HasFilepathPrefix(dest, config.KanikoDir, false) {
+		logrus.Warnf("Skipping copy targeting kaniko directory: %s", dest)
+		logrus.Info("Writes to the kaniko directory are blocked to prevent overwriting the executor.")
+		logrus.Info("To copy files there, relocate kaniko with KANIKO_DIR: https://github.com/osscontainertools/kaniko#bootstrapping-kaniko")
+		return true, nil
+	}
+	if src == dest {
+		// Recreating the fifo in place would drop it and take its readers with it.
+		return false, nil
+	}
+	uid, gid = DetermineTargetFileOwnership(fi, uid, gid)
+	if err := createParentDirectory(dest, int(uid), int(gid), chmod.Apply(0o755)); err != nil {
+		return false, err
+	}
+	if FilepathExists(dest) {
+		if err := os.RemoveAll(dest); err != nil {
+			return false, err
+		}
+	}
+	perm := fi.Mode()
+	if !useDefaultChmod {
+		perm = chmod.Apply(perm)
+	}
+	logrus.Tracef("Creating fifo %s", dest)
+	if err := unix.Mkfifo(dest, uint32(perm.Perm())); err != nil {
+		return false, fmt.Errorf("creating fifo %s: %w", dest, err)
+	}
+	// mkfifo applies the umask, chmod to get the mode we were actually asked for
+	if err := os.Chmod(dest, perm.Perm()); err != nil {
+		return false, err
+	}
+	return false, os.Lchown(dest, int(uid), int(gid))
+}
+
 func checkCopyHardlink(fi os.FileInfo, dest string, seen map[uint64]string) (string, bool) {
 	stat := getSyscallStatT(fi)
 	if stat == nil || stat.Nlink <= 1 {
@@ -1554,8 +1613,38 @@ func (NoAtimeFS) Open(name string) (fs.File, error) {
 	return os.OpenFile(name, os.O_RDONLY|unix.O_NOATIME, 0)
 }
 
+// Without StatFS, fs.Stat opens the file to stat it, which blocks on a fifo.
+func (NoAtimeFS) Stat(name string) (fs.FileInfo, error) {
+	return os.Stat(name)
+}
+
+// Without ReadLinkFS, fs.Lstat degrades to fs.Stat and dereferences the link.
+func (NoAtimeFS) Lstat(name string) (fs.FileInfo, error) {
+	return os.Lstat(name)
+}
+
+// Without ReadLinkFS, fs.ReadLink refuses to read any link at all.
+func (NoAtimeFS) ReadLink(name string) (string, error) {
+	return os.Readlink(name)
+}
+
 type OSFS struct{}
 
 func (OSFS) Open(name string) (fs.File, error) {
 	return os.Open(name)
 }
+
+// Without StatFS, fs.Stat opens the file to stat it, which blocks on a fifo.
+func (OSFS) Stat(name string) (fs.FileInfo, error) {
+	return os.Stat(name)
+}
+
+// Without ReadLinkFS, fs.Lstat degrades to fs.Stat and dereferences the link.
+func (OSFS) Lstat(name string) (fs.FileInfo, error) {
+	return os.Lstat(name)
+}
+
+// Without ReadLinkFS, fs.ReadLink refuses to read any link at all.
+func (OSFS) ReadLink(name string) (string, error) {
+	return os.Readlink(name)
+}