Merge pull request #5676 from nalind/escape-globs

Add(): re-escape any globbed items that included escapes

add.go

@@ -230,6 +230,18 @@ func includeDirectoryAnyway(path string, pm *fileutils.PatternMatcher) bool {
 	return false
 }
 
+// globbedToGlobbable takes a pathname which might include the '[', *, or ?
+// characters, and converts it into a glob pattern that matches itself by
+// marking the '[' characters as _not_ the beginning of match ranges and
+// escaping the * and ? characters.
+func globbedToGlobbable(glob string) string {
+	result := glob
+	result = strings.ReplaceAll(result, "[", "[[]")
+	result = strings.ReplaceAll(result, "?", "\\?")
+	result = strings.ReplaceAll(result, "*", "\\*")
+	return result
+}
+
 // Add copies the contents of the specified sources into the container's root
 // filesystem, optionally extracting contents of local files that look like
 // non-empty archives.
@@ -517,27 +529,27 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
 
 		// Iterate through every item that matched the glob.
 		itemsCopied := 0
-		for _, glob := range localSourceStat.Globbed {
-			rel := glob
-			if filepath.IsAbs(glob) {
-				if rel, err = filepath.Rel(contextDir, glob); err != nil {
-					return fmt.Errorf("computing path of %q relative to %q: %w", glob, contextDir, err)
+		for _, globbed := range localSourceStat.Globbed {
+			rel := globbed
+			if filepath.IsAbs(globbed) {
+				if rel, err = filepath.Rel(contextDir, globbed); err != nil {
+					return fmt.Errorf("computing path of %q relative to %q: %w", globbed, contextDir, err)
 				}
 			}
 			if strings.HasPrefix(rel, ".."+string(os.PathSeparator)) {
-				return fmt.Errorf("possible escaping context directory error: %q is outside of %q", glob, contextDir)
+				return fmt.Errorf("possible escaping context directory error: %q is outside of %q", globbed, contextDir)
 			}
 			// Check for dockerignore-style exclusion of this item.
 			if rel != "." {
 				excluded, err := pm.Matches(filepath.ToSlash(rel)) // nolint:staticcheck
 				if err != nil {
-					return fmt.Errorf("checking if %q(%q) is excluded: %w", glob, rel, err)
+					return fmt.Errorf("checking if %q(%q) is excluded: %w", globbed, rel, err)
 				}
 				if excluded {
 					// non-directories that are excluded are excluded, no question, but
 					// directories can only be skipped if we don't have to allow for the
 					// possibility of finding things to include under them
-					globInfo := localSourceStat.Results[glob]
+					globInfo := localSourceStat.Results[globbed]
 					if !globInfo.IsDir || !includeDirectoryAnyway(rel, pm) {
 						continue
 					}
@@ -554,7 +566,7 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
 				// due to potentially not having anything in the tarstream that we passed.
 				itemsCopied++
 			}
-			st := localSourceStat.Results[glob]
+			st := localSourceStat.Results[globbed]
 			pipeReader, pipeWriter := io.Pipe()
 			wg.Add(1)
 			go func() {
@@ -584,7 +596,7 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
 					StripSetgidBit: options.StripSetgidBit,
 					StripStickyBit: options.StripStickyBit,
 				}
-				getErr = copier.Get(contextDir, contextDir, getOptions, []string{glob}, writer)
+				getErr = copier.Get(contextDir, contextDir, getOptions, []string{globbedToGlobbable(globbed)}, writer)
 				closeErr = writer.Close()
 				if renameTarget != "" && renamedItems > 1 {
 					renameErr = fmt.Errorf("internal error: renamed %d items when we expected to only rename 1", renamedItems)

tests/conformance/conformance_test.go

@@ -1421,6 +1421,13 @@ var internalTestCases = []testCase{
 		dockerRegex:  "(?s)RUN env.*?Running in [0-9a-z]+(.*?)---",
 	},
 
+	{
+		name:              "copy-escape-glob",
+		contextDir:        "copy-escape-glob",
+		fsSkip:            []string{"(dir):app:mtime", "(dir):app2:mtime", "(dir):app3:mtime", "(dir):app4:mtime", "(dir):app5:mtime"},
+		dockerUseBuildKit: true,
+	},
+
 	{
 		name:         "copy file to root",
 		dockerfile:   "Dockerfile.copyfrom_1",

tests/conformance/testdata/copy-escape-glob/Dockerfile

@@ -0,0 +1,12 @@
+FROM scratch
+WORKDIR /app
+# The "[]" sequence immediately after the "[" signals to filepath.Glob() that
+# it is _not_ the beginning of a "class" chunk in the globbing pattern.  N.B.
+# this is a golang-specific way to do it, different from what glob(7) suggests.
+COPY ./app/[[]xyz]/file.txt /app/
+COPY ./app/[[]xyz]/[[]abc]/file.txt /app2/
+COPY ./app/* /app3/
+# This should only match one file.
+COPY ./app/n\?pe/file.txt /app4/file.txt
+# This should only match one file.
+COPY ./app/st\*uv/file.txt /app5/file.txt

tests/conformance/testdata/copy-escape-glob/app/[xyz]/[abc]/file.txt

@@ -0,0 +1 @@
+also a file

tests/conformance/testdata/copy-escape-glob/app/[xyz]/file.txt

@@ -0,0 +1 @@
+a file

tests/conformance/testdata/copy-escape-glob/app/jkl?/file.txt

@@ -0,0 +1 @@
+another

tests/conformance/testdata/copy-escape-glob/app/n?pe/file.txt

@@ -0,0 +1 @@
+and also

tests/conformance/testdata/copy-escape-glob/app/nope/file.txt

@@ -0,0 +1 @@
+but not

tests/conformance/testdata/copy-escape-glob/app/st*uv/file.txt

@@ -0,0 +1 @@
+and yet

tests/conformance/testdata/copy-escape-glob/app/strauv/file.txt

@@ -0,0 +1 @@
+not this one