Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider filesystem types for mount points when ignoring system paths #2675

Merged
merged 2 commits into from
Feb 28, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 2 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ require (
github.com/anchore/bubbly v0.0.0-20231115134915-def0aba654a9
github.com/anchore/clio v0.0.0-20240209204744-cb94e40a4f65
github.com/anchore/fangs v0.0.0-20231201140849-5075d28d6d8b
github.com/anchore/go-collections v0.0.0-20240216171411-9321230ce537
github.com/anchore/go-logger v0.0.0-20230725134548-c21dafa1ec5a
github.com/anchore/go-macholibre v0.0.0-20220308212642-53e6d0aaf6fb
github.com/anchore/go-testutils v0.0.0-20200925183923-d5f45b0d3c04
Expand Down Expand Up @@ -53,6 +54,7 @@ require (
github.com/mitchellh/go-homedir v1.1.0
github.com/mitchellh/hashstructure/v2 v2.0.2
github.com/mitchellh/mapstructure v1.5.0
github.com/moby/sys/mountinfo v0.7.1
github.com/olekukonko/tablewriter v0.0.5
github.com/opencontainers/go-digest v1.0.0
github.com/pelletier/go-toml v1.9.5
Expand Down Expand Up @@ -80,8 +82,6 @@ require (
modernc.org/sqlite v1.29.2
)

require github.com/anchore/go-collections v0.0.0-20240216171411-9321230ce537

require (
dario.cat/mergo v1.0.0 // indirect
github.com/AdaLogics/go-fuzz-headers v0.0.0-20230811130428-ced1acdcaa24 // indirect
Expand Down Expand Up @@ -161,7 +161,6 @@ require (
github.com/mitchellh/copystructure v1.2.0 // indirect
github.com/mitchellh/reflectwalk v1.0.2 // indirect
github.com/moby/locker v1.0.1 // indirect
github.com/moby/sys/mountinfo v0.6.2 // indirect
github.com/moby/sys/sequential v0.5.0 // indirect
github.com/moby/sys/signal v0.7.0 // indirect
github.com/muesli/ansi v0.0.0-20211031195517-c9f0611b6c70 // indirect
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -571,8 +571,8 @@ github.com/mitchellh/reflectwalk v1.0.2 h1:G2LzWKi524PWgd3mLHV8Y5k7s6XUvT0Gef6zx
github.com/mitchellh/reflectwalk v1.0.2/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw=
github.com/moby/locker v1.0.1 h1:fOXqR41zeveg4fFODix+1Ch4mj/gT0NE1XJbp/epuBg=
github.com/moby/locker v1.0.1/go.mod h1:S7SDdo5zpBK84bzzVlKr2V0hz+7x9hWbYC/kq7oQppc=
github.com/moby/sys/mountinfo v0.6.2 h1:BzJjoreD5BMFNmD9Rus6gdd1pLuecOFPt8wC+Vygl78=
github.com/moby/sys/mountinfo v0.6.2/go.mod h1:IJb6JQeOklcdMU9F5xQ8ZALD+CUr5VlGpwtX+VE0rpI=
github.com/moby/sys/mountinfo v0.7.1 h1:/tTvQaSJRr2FshkhXiIpux6fQ2Zvc4j7tAhMTStAG2g=
github.com/moby/sys/mountinfo v0.7.1/go.mod h1:IJb6JQeOklcdMU9F5xQ8ZALD+CUr5VlGpwtX+VE0rpI=
github.com/moby/sys/sequential v0.5.0 h1:OPvI35Lzn9K04PBbCLW0g4LcFAJgHsvXsRyewg5lXtc=
github.com/moby/sys/sequential v0.5.0/go.mod h1:tH2cOOs5V9MlPiXcQzRC+eEyab644PWKGRYaaV5ZZlo=
github.com/moby/sys/signal v0.7.0 h1:25RW3d5TnQEoKvRbEKUGay6DCQ46IxAVTT9CUMgmsSI=
Expand Down
6 changes: 0 additions & 6 deletions syft/internal/fileresolver/directory.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,6 @@ import (
"github.com/anchore/syft/syft/internal/windows"
)

var unixSystemRuntimePrefixes = []string{
"/proc",
"/dev",
"/sys",
}

var ErrSkipPath = errors.New("skip path")

var _ file.Resolver = (*Directory)(nil)
Expand Down
94 changes: 57 additions & 37 deletions syft/internal/fileresolver/directory_indexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"path/filepath"
"strings"

"github.com/moby/sys/mountinfo"
"github.com/wagoodman/go-partybus"
"github.com/wagoodman/go-progress"

Expand All @@ -34,12 +35,18 @@ type directoryIndexer struct {

func newDirectoryIndexer(path, base string, visitors ...PathIndexVisitor) *directoryIndexer {
i := &directoryIndexer{
path: path,
base: base,
tree: filetree.New(),
index: filetree.NewIndex(),
pathIndexVisitors: append([]PathIndexVisitor{requireFileInfo, disallowByFileType, disallowUnixSystemRuntimePath}, visitors...),
errPaths: make(map[string]error),
path: path,
base: base,
tree: filetree.New(),
index: filetree.NewIndex(),
pathIndexVisitors: append(
[]PathIndexVisitor{
requireFileInfo,
disallowByFileType,
newUnixSystemMountFinder().disallowUnixSystemRuntimePath},
visitors...,
),
errPaths: make(map[string]error),
}

// these additional stateful visitors should be the first thing considered when walking / indexing
Expand Down Expand Up @@ -443,11 +450,50 @@ func (r *directoryIndexer) disallowRevisitingVisitor(_, path string, _ os.FileIn
return nil
}

func disallowUnixSystemRuntimePath(base, path string, _ os.FileInfo, _ error) error {
// note: we need to consider all paths relative to the base when being filtered, which is how they would appear
// when the resolver is being used. Then something like /some/mountpoint/dev with a base of /some/mountpoint
// would be considered as /dev when being filtered.
if internal.HasAnyOfPrefixes(relativePath(base, path), unixSystemRuntimePrefixes...) {
type unixSystemMountFinder struct {
disallowedMountPaths []string
}

func newUnixSystemMountFinder() unixSystemMountFinder {
infos, err := mountinfo.GetMounts(nil)
if err != nil {
log.WithFields("error", err).Warnf("unable to get system mounts")
return unixSystemMountFinder{}
}

return unixSystemMountFinder{
disallowedMountPaths: keepUnixSystemMountPaths(infos),
}
}

func keepUnixSystemMountPaths(infos []*mountinfo.Info) []string {
var mountPaths []string
for _, info := range infos {
if info == nil {
continue
}
// on a unix system you may see:
// - tmpfs: a memory-based filesystem (e.g. /tmp, /dev, /sys/firmware)
// - devtmpfs: a virtual filesystem that provides access to devices (e.g. /dev)
// - shm: a shared memory filesystem (e.g. /dev/shm)
// - proc: a virtual filesystem that provides access to process information (e.g. /proc)
// - sysfs: a virtual filesystem that provides access to kernel information (e.g. /sys)

// on a darwin system you may see:
// - devfs: a virtual filesystem that provides access to devices (e.g. /dev)

switch info.FSType {
case "tmpfs", "devfs", "devtmpfs", "shm", "proc", "sysfs":
wagoodman marked this conversation as resolved.
Show resolved Hide resolved
log.WithFields("mountpoint", info.Mountpoint).Debug("ignoring system mountpoint")

mountPaths = append(mountPaths, info.Mountpoint)
}
}
return mountPaths
}

func (f unixSystemMountFinder) disallowUnixSystemRuntimePath(_, path string, _ os.FileInfo, _ error) error {
if internal.HasAnyOfPrefixes(path, f.disallowedMountPaths...) {
return fs.SkipDir
}
return nil
Expand Down Expand Up @@ -475,32 +521,6 @@ func requireFileInfo(_, _ string, info os.FileInfo, _ error) error {
return nil
}

func relativePath(basePath, givenPath string) string {
var relPath string
var relErr error

if basePath != "" {
relPath, relErr = filepath.Rel(basePath, givenPath)
cleanPath := filepath.Clean(relPath)
if relErr == nil {
if cleanPath == "." {
relPath = string(filepath.Separator)
} else {
relPath = cleanPath
}
}
if !filepath.IsAbs(relPath) {
relPath = string(filepath.Separator) + relPath
}
}

if relErr != nil || basePath == "" {
relPath = givenPath
}

return relPath
}

func indexingProgress(path string) (*progress.Stage, *progress.Manual) {
stage := &progress.Stage{}
prog := progress.NewManual(-1)
Expand Down
113 changes: 108 additions & 5 deletions syft/internal/fileresolver/directory_indexer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ import (
"io/fs"
"os"
"path"
"path/filepath"
"sort"
"strings"
"testing"

"github.com/google/go-cmp/cmp"
"github.com/moby/sys/mountinfo"
"github.com/scylladb/go-set/strset"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -435,7 +437,38 @@ func Test_relativePath(t *testing.T) {
}
}

func relativePath(basePath, givenPath string) string {
var relPath string
var relErr error

if basePath != "" {
relPath, relErr = filepath.Rel(basePath, givenPath)
cleanPath := filepath.Clean(relPath)
if relErr == nil {
if cleanPath == "." {
relPath = string(filepath.Separator)
} else {
relPath = cleanPath
}
}
if !filepath.IsAbs(relPath) {
relPath = string(filepath.Separator) + relPath
}
}

if relErr != nil || basePath == "" {
relPath = givenPath
}

return relPath
}

func Test_disallowUnixSystemRuntimePath(t *testing.T) {
unixSubject := unixSystemMountFinder{
// mock out detecting the mount points
disallowedMountPaths: []string{"/proc", "/sys", "/dev"},
}

tests := []struct {
name string
base string
Expand All @@ -455,24 +488,94 @@ func Test_disallowUnixSystemRuntimePath(t *testing.T) {
wantErr: require.NoError,
},
{
name: "base, matching path",
name: "do not consider base when matching paths (non-matching)",
base: "/a/b/c",
path: "/a/b/c/dev",
wantErr: require.Error,
wantErr: require.NoError,
},
{
name: "base, non-matching path due to bad relative alignment",
name: "do not consider base when matching paths (matching)",
base: "/a/b/c",
path: "/dev",
wantErr: require.NoError,
wantErr: require.Error,
wagoodman marked this conversation as resolved.
Show resolved Hide resolved
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if tt.wantErr == nil {
tt.wantErr = require.NoError
}
tt.wantErr(t, disallowUnixSystemRuntimePath(tt.base, tt.path, nil, nil))
tt.wantErr(t, unixSubject.disallowUnixSystemRuntimePath(tt.base, tt.path, nil, nil))
})
}
}

func Test_keepUnixSystemMountPaths(t *testing.T) {

tests := []struct {
name string
infos []*mountinfo.Info
want []string
}{
{
name: "all valid filesystems",
infos: []*mountinfo.Info{
{
Mountpoint: "/etc/hostname",
FSType: "/dev/vda1",
},
{
Mountpoint: "/sys/fs/cgroup",
FSType: "cgroup",
},
{
Mountpoint: "/",
FSType: "overlay",
},
},
want: nil,
},
{
name: "no valid filesystems",
infos: []*mountinfo.Info{
{
Mountpoint: "/proc",
FSType: "proc",
},
{
Mountpoint: "/sys",
FSType: "sysfs",
},
{
Mountpoint: "/dev",
FSType: "devfs",
},
{
Mountpoint: "/dev-new",
FSType: "devtmpfs",
},
{
Mountpoint: "/run",
FSType: "tmpfs",
},
{
Mountpoint: "/run/lock",
FSType: "tmpfs",
},
},
want: []string{
"/proc",
"/sys",
"/dev",
"/dev-new",
"/run",
"/run/lock",
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, keepUnixSystemMountPaths(tt.infos))
})
}
}
6 changes: 5 additions & 1 deletion syft/internal/fileresolver/directory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1109,6 +1109,10 @@ func Test_directoryResolver_FileContentsByLocation(t *testing.T) {
}

func Test_isUnixSystemRuntimePath(t *testing.T) {
unixSubject := unixSystemMountFinder{
// mock out detecting the mount points
disallowedMountPaths: []string{"/proc", "/sys", "/dev"},
}
tests := []struct {
path string
expected error
Expand Down Expand Up @@ -1144,7 +1148,7 @@ func Test_isUnixSystemRuntimePath(t *testing.T) {
}
for _, test := range tests {
t.Run(test.path, func(t *testing.T) {
assert.Equal(t, test.expected, disallowUnixSystemRuntimePath("", test.path, nil, nil))
assert.Equal(t, test.expected, unixSubject.disallowUnixSystemRuntimePath("", test.path, nil, nil))
})
}
}
Expand Down