Skip to content

Commit

Permalink
Consider filesystem types for mount points when ignoring system paths (…
Browse files Browse the repository at this point in the history 
…#2675)

* consider fs types for mount points when ignoring system paths

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

* address feedback

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

---------

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
  • Loading branch information
@wagoodman
wagoodman committed Feb 28, 2024
1 parent 63171b5 commit 48e5672
Show file tree
Hide file tree
Showing 6 changed files with 215 additions and 113 deletions.
5 changes: 2 additions & 3 deletions go.mod
View file
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
View file
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
View file
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
View file
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
View file
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))
})
}
}

0 comments on commit 48e5672

Please sign in to comment.