Skip to content

Commit 8811b09

Browse files
Honny1Honny1
committed
Fix .dockerignore wildcard negation directory descent
Buildah skipped excluded directories when negation patterns contained wildcards (For example: !**/*.go), because the descent check only matched literal prefixes. Extract the literal prefix before the first wildcard and descend when the directory is at or under it; when the prefix is empty, always descend. Same bug as Docker's classic builder (moby/moby#30018, moby/moby#45608). Fixes: #6615 Signed-off-by: Jan Rodák <hony.com@seznam.cz>
1 parent 656771c commit 8811b09

10 files changed

Lines changed: 262 additions & 89 deletions

File tree

‎add.go‎

Lines changed: 2 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -253,29 +253,6 @@ func getURL(src string, chown *idtools.IDPair, mountpoint, renameTarget string,
253253
return nil
254254
}
255255

256-
// includeDirectoryAnyway returns true if "path" is a prefix for an exception
257-
// known to "pm". If "path" is a directory that "pm" claims matches its list
258-
// of patterns, but "pm"'s list of exclusions contains a pattern for which
259-
// "path" is a prefix, then IncludeDirectoryAnyway() will return true.
260-
// This is not always correct, because it relies on the directory part of any
261-
// exception paths to be specified without wildcards.
262-
func includeDirectoryAnyway(path string, pm *fileutils.PatternMatcher) bool {
263-
if !pm.Exclusions() {
264-
return false
265-
}
266-
prefix := strings.TrimPrefix(path, string(os.PathSeparator)) + string(os.PathSeparator)
267-
for _, pattern := range pm.Patterns() {
268-
if !pattern.Exclusion() {
269-
continue
270-
}
271-
spec := strings.TrimPrefix(pattern.String(), string(os.PathSeparator))
272-
if strings.HasPrefix(spec, prefix) {
273-
return true
274-
}
275-
}
276-
return false
277-
}
278-
279256
// globbedToGlobbable takes a pathname which might include the '[', *, or ?
280257
// characters, and converts it into a glob pattern that matches itself by
281258
// marking the '[' characters as _not_ the beginning of match ranges and
@@ -708,7 +685,7 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
708685
}
709686
// Check for dockerignore-style exclusion of this item.
710687
if rel != "." {
711-
excluded, err := pm.Matches(filepath.ToSlash(rel)) //nolint:staticcheck
688+
excluded, err := pm.IsMatch(filepath.ToSlash(rel))
712689
if err != nil {
713690
return fmt.Errorf("checking if %q(%q) is excluded: %w", globbed, rel, err)
714691
}
@@ -717,7 +694,7 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
717694
// directories can only be skipped if we don't have to allow for the
718695
// possibility of finding things to include under them
719696
globInfo := localSourceStat.Results[globbed]
720-
if !globInfo.IsDir || !includeDirectoryAnyway(rel, pm) {
697+
if !globInfo.IsDir || !copier.ShouldDescendExcludedDir(rel, pm) {
721698
continue
722699
}
723700
} else {

‎copier/copier.go‎

Lines changed: 47 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1084,7 +1084,7 @@ func pathIsExcluded(root, path string, pm *fileutils.PatternMatcher) (string, bo
10841084
// Matches uses filepath.FromSlash() to convert candidates before
10851085
// checking if they match the patterns it's been given, implying that
10861086
// it expects Unix-style paths.
1087-
matches, err := pm.Matches(filepath.ToSlash(rel)) //nolint:staticcheck
1087+
matches, err := pm.IsMatch(filepath.ToSlash(rel))
10881088
if err != nil {
10891089
return rel, false, fmt.Errorf("copier: error checking if %q is excluded: %w", rel, err)
10901090
}
@@ -1094,6 +1094,47 @@ func pathIsExcluded(root, path string, pm *fileutils.PatternMatcher) (string, bo
10941094
return rel, false, nil
10951095
}
10961096

1097+
// ShouldDescendExcludedDir checks whether an excluded directory should still be
1098+
// descended into because a negation pattern in pm might match files under it.
1099+
// It handles literal prefix matches (e.g. !cmd/main.go for dir "cmd") and
1100+
// wildcard negations (e.g. !**/*.go, !*/*.go). The wildcard check extracts
1101+
// the literal prefix before the first wildcard and may intentionally
1102+
// overmatch (descend into directories that won't ultimately contain matches),
1103+
// which is safe because actual file-level matching happens later.
1104+
func ShouldDescendExcludedDir(dirPath string, pm *fileutils.PatternMatcher) bool {
1105+
if pm == nil || !pm.Exclusions() {
1106+
return false
1107+
}
1108+
dir := filepath.ToSlash(strings.TrimPrefix(dirPath, string(os.PathSeparator)))
1109+
for _, pattern := range pm.Patterns() {
1110+
if !pattern.Exclusion() {
1111+
continue
1112+
}
1113+
slashPattern := filepath.ToSlash(strings.TrimPrefix(pattern.String(), string(os.PathSeparator)))
1114+
1115+
// Literal-prefix check: the negation spec starts with this
1116+
// directory path, for example: !cmd/main.go matches dir "cmd"
1117+
if strings.HasPrefix(slashPattern, dir+"/") {
1118+
return true
1119+
}
1120+
1121+
// Wildcard-aware check: extract the literal prefix before
1122+
// the first wildcard character (*, ?, [), for example: !cmd/**/*.go matches dir "cmd"
1123+
// if the directory is at or under that literal prefix, a file beneath this
1124+
// directory could match the negation, so keep descending.
1125+
if firstWild := strings.IndexAny(slashPattern, "*?["); firstWild >= 0 {
1126+
literalPrefix := strings.TrimRight(slashPattern[:firstWild], "/")
1127+
if literalPrefix == "" {
1128+
return true
1129+
}
1130+
if dir == literalPrefix || strings.HasPrefix(dir, literalPrefix+"/") {
1131+
return true
1132+
}
1133+
}
1134+
}
1135+
return false
1136+
}
1137+
10971138
// resolvePath resolves symbolic links in paths, treating the specified
10981139
// directory as the root.
10991140
// Resolving the path this way, and using the result, is in no way secure
@@ -1532,32 +1573,12 @@ func copierHandlerGet(bulkWriter io.Writer, req request, pm *fileutils.PatternMa
15321573
}
15331574
if skip {
15341575
if d.IsDir() {
1535-
// if there are no "include
1536-
// this anyway" patterns at
1537-
// all, we don't need to
1538-
// descend into this particular
1539-
// directory if it's a directory
1540-
if !pm.Exclusions() {
1541-
return filepath.SkipDir
1542-
}
1543-
// if there are exclusion
1544-
// patterns for which this
1545-
// path is a prefix, we
1546-
// need to keep descending
1547-
for _, pattern := range pm.Patterns() {
1548-
if !pattern.Exclusion() {
1549-
continue
1550-
}
1551-
spec := strings.Trim(pattern.String(), string(os.PathSeparator))
1552-
trimmedPath := strings.Trim(skippedPath, string(os.PathSeparator))
1553-
if strings.HasPrefix(spec+string(os.PathSeparator), trimmedPath) {
1554-
// we can't just skip over
1555-
// this directory
1556-
return nil
1557-
}
1576+
// check if a negation pattern
1577+
// means we should descend into
1578+
// this excluded directory
1579+
if ShouldDescendExcludedDir(skippedPath, pm) {
1580+
return nil
15581581
}
1559-
// there are exclusions, but
1560-
// none of them apply here
15611582
return filepath.SkipDir
15621583
}
15631584
// skip this item, but if we're

‎copier/copier_test.go‎

Lines changed: 121 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"github.com/stretchr/testify/require"
2929
"github.com/tonistiigi/dchapes-mode"
3030
"go.podman.io/image/v5/types"
31+
"go.podman.io/storage/pkg/fileutils"
3132
"go.podman.io/storage/pkg/idtools"
3233
"go.podman.io/storage/pkg/reexec"
3334
)
@@ -1033,7 +1034,7 @@ func testGetMultiple(t *testing.T) {
10331034
"file-b",
10341035
"link-c",
10351036
"hlink-0",
1036-
// "subdir-a/file-c", // strings.HasPrefix("**/*-c", "subdir-a/") is false
1037+
"subdir-a/file-c",
10371038
"subdir-b/",
10381039
"subdir-b/file-n",
10391040
"subdir-b/file-o",
@@ -1159,8 +1160,8 @@ func testGetMultiple(t *testing.T) {
11591160
pattern: ".",
11601161
exclude: []string{"*", "!**/*-c"},
11611162
items: []string{
1162-
// "subdir-a/file-c", // strings.HasPrefix("**/*-c", "subdir-a/") is false
11631163
"link-c",
1164+
"subdir-a/file-c",
11641165
"subdir-c/",
11651166
"subdir-c/file-p",
11661167
"subdir-c/file-q",
@@ -3148,3 +3149,121 @@ func testChmod(t *testing.T) {
31483149
})
31493150
}
31503151
}
3152+
3153+
func TestShouldDescendExcludedDir(t *testing.T) {
3154+
tests := []struct {
3155+
name string
3156+
path string
3157+
patterns []string
3158+
want bool
3159+
}{
3160+
{
3161+
name: "nil matcher",
3162+
path: "cmd",
3163+
patterns: nil,
3164+
want: false,
3165+
},
3166+
{
3167+
name: "no exclusions",
3168+
path: "cmd",
3169+
patterns: []string{"*"},
3170+
want: false,
3171+
},
3172+
{
3173+
name: "literal prefix match",
3174+
path: "cmd",
3175+
patterns: []string{"*", "!cmd/main.go"},
3176+
want: true,
3177+
},
3178+
{
3179+
name: "literal prefix no match",
3180+
path: "other",
3181+
patterns: []string{"*", "!cmd/main.go"},
3182+
want: false,
3183+
},
3184+
{
3185+
name: "double star at start matches any dir",
3186+
path: "cmd",
3187+
patterns: []string{"**", "!**/*.go"},
3188+
want: true,
3189+
},
3190+
{
3191+
name: "double star at start matches nested dir",
3192+
path: "cmd/sub",
3193+
patterns: []string{"**", "!**/*.go"},
3194+
want: true,
3195+
},
3196+
{
3197+
name: "double star with prefix matches dir under prefix",
3198+
path: "cmd/sub",
3199+
patterns: []string{"**", "!cmd/**/*.go"},
3200+
want: true,
3201+
},
3202+
{
3203+
name: "double star with prefix no match for other dir",
3204+
path: "other",
3205+
patterns: []string{"**", "!cmd/**/*.go"},
3206+
want: false,
3207+
},
3208+
{
3209+
name: "single star at start matches any dir",
3210+
path: "cmd",
3211+
patterns: []string{"*", "!*/*.go"},
3212+
want: true,
3213+
},
3214+
{
3215+
name: "single star at start matches nested dir",
3216+
path: "cmd/sub",
3217+
patterns: []string{"*", "!*/*.go"},
3218+
want: true,
3219+
},
3220+
{
3221+
name: "single star with prefix matches dir under prefix",
3222+
path: "src/pkg",
3223+
patterns: []string{"**", "!src/*/*.go"},
3224+
want: true,
3225+
},
3226+
{
3227+
name: "single star with prefix no match for other dir",
3228+
path: "other",
3229+
patterns: []string{"**", "!src/*/*.go"},
3230+
want: false,
3231+
},
3232+
{
3233+
name: "leading slash is stripped",
3234+
path: "/cmd",
3235+
patterns: []string{"*", "!cmd/main.go"},
3236+
want: true,
3237+
},
3238+
{
3239+
name: "deep nested with double star prefix",
3240+
path: "src/internal/pkg",
3241+
patterns: []string{"**", "!src/**/*.go"},
3242+
want: true,
3243+
},
3244+
{
3245+
name: "dir prefix match is not a partial match",
3246+
path: "cmds",
3247+
patterns: []string{"*", "!cmd/main.go"},
3248+
want: false,
3249+
},
3250+
{
3251+
name: "non-exclusion patterns are ignored",
3252+
path: "cmd",
3253+
patterns: []string{"cmd/**/*.go"},
3254+
want: false,
3255+
},
3256+
}
3257+
for _, tt := range tests {
3258+
t.Run(tt.name, func(t *testing.T) {
3259+
var pm *fileutils.PatternMatcher
3260+
if tt.patterns != nil {
3261+
var err error
3262+
pm, err = fileutils.NewPatternMatcher(tt.patterns)
3263+
require.NoError(t, err)
3264+
}
3265+
got := ShouldDescendExcludedDir(tt.path, pm)
3266+
assert.Equal(t, tt.want, got, "ShouldDescendExcludedDir(%q)", tt.path)
3267+
})
3268+
}
3269+
}

‎tests/bud.bats‎

Lines changed: 37 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -590,8 +590,8 @@ _EOF
590590

591591
@test "bud with .dockerignore #1" {
592592
_prefetch alpine busybox
593-
run_buildah 125 build -t testbud $WITH_POLICY_JSON -f $BUDFILES/dockerignore/Dockerfile $BUDFILES/dockerignore
594-
expect_output --substring 'building.*"COPY subdir \./".*no such file or directory'
593+
# https://github.com/containers/buildah/issues/6615
594+
run_buildah build -t testbud $WITH_POLICY_JSON -f $BUDFILES/dockerignore/Dockerfile $BUDFILES/dockerignore
595595

596596
run_buildah build -t testbud $WITH_POLICY_JSON -f $BUDFILES/dockerignore/Dockerfile.succeed $BUDFILES/dockerignore
597597

@@ -605,7 +605,7 @@ _EOF
605605

606606
run_buildah 1 run myctr ls -l sub2.txt
607607

608-
run_buildah 1 run myctr ls -l subdir/
608+
run_buildah run myctr ls -l subdir/
609609
}
610610

611611
@test "bud --layers with --mount type bind should burst cache if symlink is changed" {
@@ -945,8 +945,8 @@ this is the output of test12"
945945

946946
@test "bud with .containerignore" {
947947
_prefetch alpine busybox
948-
run_buildah 125 build -t testbud $WITH_POLICY_JSON -f $BUDFILES/containerignore/Dockerfile $BUDFILES/containerignore
949-
expect_output --substring 'building.*"COPY subdir \./".*no such file or directory'
948+
# https://github.com/containers/buildah/issues/6615
949+
run_buildah build -t testbud $WITH_POLICY_JSON -f $BUDFILES/containerignore/Dockerfile $BUDFILES/containerignore
950950

951951
run_buildah build -t testbud $WITH_POLICY_JSON -f $BUDFILES/containerignore/Dockerfile.succeed $BUDFILES/containerignore
952952

@@ -960,7 +960,7 @@ this is the output of test12"
960960

961961
run_buildah 1 run myctr ls -l sub2.txt
962962

963-
run_buildah 1 run myctr ls -l subdir/
963+
run_buildah run myctr ls -l subdir/
964964
}
965965

966966
@test "bud with .dockerignore - unmatched" {
@@ -1032,8 +1032,8 @@ symlink(subdir)"
10321032

10331033
@test "bud with .dockerignore #6" {
10341034
_prefetch alpine busybox
1035-
run_buildah 125 build -t testbud $WITH_POLICY_JSON -f $BUDFILES/dockerignore6/Dockerfile $BUDFILES/dockerignore6
1036-
expect_output --substring 'building.*"COPY subdir \./".*no such file or directory'
1035+
# https://github.com/containers/buildah/issues/6615
1036+
run_buildah build -t testbud $WITH_POLICY_JSON -f $BUDFILES/dockerignore6/Dockerfile $BUDFILES/dockerignore6
10371037

10381038
run_buildah build -t testbud $WITH_POLICY_JSON -f $BUDFILES/dockerignore6/Dockerfile.succeed $BUDFILES/dockerignore6
10391039

@@ -1047,7 +1047,7 @@ symlink(subdir)"
10471047

10481048
run_buildah 1 run myctr ls -l sub2.txt
10491049

1050-
run_buildah 1 run myctr ls -l subdir/
1050+
run_buildah run myctr ls -l subdir/
10511051
}
10521052

10531053
@test "build with --platform without OS" {
@@ -4865,6 +4865,33 @@ _EOF
48654865
assert "$output" !~ file2
48664866
}
48674867

4868+
# https://github.com/containers/buildah/issues/6615
4869+
@test "bud copy with .dockerignore wildcard negation" {
4870+
_prefetch alpine
4871+
mytmpdir=${TEST_SCRATCH_DIR}/my-dir-wildcard
4872+
mkdir -p $mytmpdir/cmd
4873+
echo "package main" > $mytmpdir/cmd/main.go
4874+
echo "module test" > $mytmpdir/go.mod
4875+
echo "# readme" > $mytmpdir/README.md
4876+
4877+
cat > $mytmpdir/.dockerignore << _EOF
4878+
**
4879+
!go.mod
4880+
!**/*.go
4881+
_EOF
4882+
4883+
cat > $mytmpdir/Containerfile << _EOF
4884+
FROM alpine
4885+
COPY . /upload/
4886+
RUN find /upload -type f
4887+
_EOF
4888+
4889+
run_buildah build -t testbud $WITH_POLICY_JSON ${mytmpdir}
4890+
expect_output --substring "/upload/go.mod"
4891+
expect_output --substring "/upload/cmd/main.go"
4892+
assert "$output" !~ "README"
4893+
}
4894+
48684895
@test "bud-copy-workdir" {
48694896
target=testimage
48704897
run_buildah build $WITH_POLICY_JSON -t ${target} $BUDFILES/copy-workdir
@@ -5882,8 +5909,7 @@ EOF
58825909
run_buildah 1 run myctr ls -l sub2.txt
58835910
expect_output --substring "ls: sub2.txt: No such file or directory"
58845911

5885-
run_buildah 1 run myctr ls -l subdir/
5886-
expect_output --substring "ls: subdir/: No such file or directory"
5912+
run_buildah run myctr ls -l subdir/
58875913
}
58885914

58895915
@test "bud with network options" {

0 commit comments

Comments
 (0)