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 all commits
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
96 changes: 59 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,52 @@ 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
}
// we're only interested in ignoring the logical filesystems typically found at these mount points:
// - /proc
// - procfs
// - proc
// - /sys
// - sysfs
// - /dev
// - devfs - BSD/darwin flavored systems and old linux systems
// - devtmpfs - driver core maintained /dev tmpfs
// - udev - userspace implementation that replaced devfs
// - tmpfs - used for /dev in special instances (within a container)

switch info.FSType {
case "proc", "procfs", "sysfs", "devfs", "devtmpfs", "udev", "tmpfs":
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 +523,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
176 changes: 152 additions & 24 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,44 +437,170 @@ 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
path string
wantErr require.ErrorAssertionFunc
name string
path string
base string
expected error
}{
{
name: "no base, matching path",
base: "",
path: "/dev",
wantErr: require.Error,
name: "relative path to proc is allowed",
path: "proc/place",
},
{
name: "relative path within proc is not allowed",
path: "/proc/place",
expected: fs.SkipDir,
},
{
name: "no base, non-matching path",
base: "",
path: "/not-dev",
wantErr: require.NoError,
name: "path exactly to proc is not allowed",
path: "/proc",
expected: fs.SkipDir,
},
{
name: "base, matching path",
base: "/a/b/c",
path: "/a/b/c/dev",
wantErr: require.Error,
name: "similar to proc",
path: "/pro/c",
},
{
name: "base, non-matching path due to bad relative alignment",
base: "/a/b/c",
path: "/dev",
wantErr: require.NoError,
name: "similar to proc",
path: "/pro",
},
{
name: "dev is not allowed",
path: "/dev",
expected: fs.SkipDir,
},
{
name: "sys is not allowed",
path: "/sys",
expected: fs.SkipDir,
},
{
name: "unrelated allowed path",
path: "/something/sys",
},
{
name: "do not consider base when matching paths (non-matching)",
base: "/a/b/c",
path: "/a/b/c/dev",
},
{
name: "do not consider base when matching paths (matching)",
base: "/a/b/c",
path: "/dev",
expected: fs.SkipDir,
},
}
for _, test := range tests {
t.Run(test.path, func(t *testing.T) {
assert.Equal(t, test.expected, unixSubject.disallowUnixSystemRuntimePath(test.base, test.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: "/proc-2",
FSType: "procfs",
},
{
Mountpoint: "/sys",
FSType: "sysfs",
},
{
Mountpoint: "/dev",
FSType: "devfs",
},
{
Mountpoint: "/dev-u",
FSType: "udev",
},
{
Mountpoint: "/dev-tmp",
FSType: "devtmpfs",
},
{
Mountpoint: "/run",
FSType: "tmpfs",
},
},
want: []string{
"/proc",
"/proc-2",
"/sys",
"/dev",
"/dev-u",
"/dev-tmp",
"/run",
},
},
}
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))
assert.Equal(t, tt.want, keepUnixSystemMountPaths(tt.infos))
})
}
}