E58C contenthash: do not follow nested symlinks when computing checksum · moby/buildkit@50bd827 · GitHub
[go: up one dir, main page]
Skip to content

Commit 50bd827

Browse files
jsternbergjsternberg
committed
contenthash: do not follow nested symlinks when computing checksum
Nested symlinks were not intended to be followed after the first level of wildcard expansion. When include/exclude patterns were used or a wildcard was present higher up in the tree, we would erroneously follow ALL symlinks to compute the hash. When a symlink was broken, this would result in an error. We weren't supposed to be following the symlinks at all. This fixes things by changing `includedPaths` to also return whether a path should follow links. If a parent directory exists for the path, then the symlink is not followed. Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
1 parent e8c0115 commit 50bd827

File tree

2 files changed

+35
-10
lines changed

2 files changed

+35
-10
lines changed

‎cache/contenthash/checksum.go‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,10 @@ import (
2727

2828
var errNotFound = errors.Errorf("not found")
2929

30-
var defaultManager *cacheManager
31-
var defaultManagerOnce sync.Once
30+
var (
31+
defaultManager *cacheManager
32+
defaultManagerOnce sync.Once
33+
)
3234

3335
func getDefaultManager() *cacheManager {
3436
defaultManagerOnce.Do(func() {
@@ -82,6 +84,7 @@ type includedPath struct {
8284
included bool
8385
includeMatchInfo patternmatcher.MatchInfo
8486
excludeMatchInfo patternmatcher.MatchInfo
87+
followLinks bool
8588
}
8689

8790
type cacheManager struct {
@@ -416,17 +419,16 @@ func (cc *cacheContext) Checksum(ctx context.Context, mountable cache.Mountable,
416419
return "", err
417420
}
418421

419-
if opts.FollowLinks {
420-
for i, w := range includedPaths {
421-
if w.record.Type == CacheRecordTypeSymlink {
422-
dgst, err := cc.lazyChecksum(ctx, m, w.path, opts.FollowLinks)
423-
if err != nil {
424-
return "", err
425-
}
426-
includedPaths[i].record = &CacheRecord{Digest: dgst}
422+
for i, w := range includedPaths {
423+
if w.followLinks && w.record.Type == CacheRecordTypeSymlink {
424+
dgst, err := cc.lazyChecksum(ctx, m, w.path, opts.FollowLinks)
425+
if err != nil {
426+
return "", err
427427
}
428+
includedPaths[i].record = &CacheRecord{Digest: dgst}
428429
}
429430
}
431+
430432
if len(includedPaths) == 0 {
431433
return digest.FromBytes([]byte{}), nil
432434
}
@@ -582,6 +584,10 @@ func (cc *cacheContext) includedPaths(ctx context.Context, m *mount, p string, o
582584
}
583585

584586
maybeIncludedPath := &includedPath{path: fn}
587+
if parentDir == nil && opts.FollowLinks {
588+
maybeIncludedPath.followLinks = true
589+
}
590+
585591
var shouldInclude bool
586592
if opts.Wildcard {
587593
if p != "" && (lastMatchedDir == "" || !strings.HasPrefix(fn, lastMatchedDir+"/")) {

‎cache/contenthash/checksum_test.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -527,6 +527,25 @@ func TestSymlinksNoFollow(t *testing.T) {
527527
require.NoError(t, err)
528528
require.Equal(t, expectedSym, dgst)
529529

530+
expectedSym = digest.Digest("sha256:9b761577efcb1239cf4be971914c5d7404914dd32ff436401af1764dc5446b83")
531+
532+
// Broken symlink is not followed in subdirectory.
533+
dgst, err = cc.Checksum(context.TODO(), ref, "foo", ChecksumOpts{FollowLinks: true}, nil)
534+
require.NoError(t, err)
535+
require.Equal(t, expectedSym, dgst)
536+
537+
expectedSym = digest.Digest("sha256:e14a98332f46b81993ff4a3b7898bb00fc3d245d84e4c42e57c9ee6b129c7c41")
538+
539+
// Same with wildcard used.
540+
dgst, err = cc.Checksum(context.TODO(), ref, "fo?", ChecksumOpts{FollowLinks: true, Wildcard: true}, nil)
541+
require.NoError(t, err)
542+
require.Equal(t, expectedSym, dgst)
543+
544+
// Still works with exclude pattern.
545+
dgst, err = cc.Checksum(context.TODO(), ref, "foo", ChecksumOpts{FollowLinks: true, ExcludePatterns: []string{"*.git"}}, nil)
546+
require.NoError(t, err)
547+
require.Equal(t, expectedSym, dgst)
548+
530549
err = ref.Release(context.TODO())
531550
require.NoError(t, err)
532551
}

0 commit comments

Comments
 (0)
0