From 84689dd68aa0c498abe95c140078d6539c7f83c5 Mon Sep 17 00:00:00 2001 From: nikpivkin Date: Wed, 25 Dec 2024 19:13:20 +0600 Subject: [PATCH 1/8] feat(image): prevent scanning oversized container images Signed-off-by: nikpivkin --- .../configuration/cli/trivy_image.md | 1 + .../references/configuration/config-file.md | 3 + go.mod | 2 +- pkg/commands/artifact/run.go | 1 + pkg/fanal/artifact/image/image.go | 70 +++++++++++++++++++ pkg/fanal/artifact/image/image_test.go | 10 +++ pkg/fanal/types/image.go | 1 + pkg/flag/image_flags.go | 20 ++++++ 8 files changed, 107 insertions(+), 1 deletion(-) diff --git a/docs/docs/references/configuration/cli/trivy_image.md b/docs/docs/references/configuration/cli/trivy_image.md index d9c312602190..952f1892843f 100644 --- a/docs/docs/references/configuration/cli/trivy_image.md +++ b/docs/docs/references/configuration/cli/trivy_image.md @@ -79,6 +79,7 @@ trivy image [flags] IMAGE_NAME --license-confidence-level float specify license classifier's confidence level (default 0.9) --license-full eagerly look for licenses in source code headers and license files --list-all-pkgs output all packages in the JSON report regardless of vulnerability + --max-image-size string maximum image size to process, specified in a human-readable format (e.g., '44kB', '17MB'); an error will be returned if the image exceeds this size --misconfig-scanners strings comma-separated list of misconfig scanners to use for misconfiguration scanning (default [azure-arm,cloudformation,dockerfile,helm,kubernetes,terraform,terraformplan-json,terraformplan-snapshot]) --module-dir string specify directory to the wasm modules that will be loaded (default "$HOME/.trivy/modules") --no-progress suppress progress bar diff --git a/docs/docs/references/configuration/config-file.md b/docs/docs/references/configuration/config-file.md index fe6332522ee0..cb5555e02007 100644 --- a/docs/docs/references/configuration/config-file.md +++ b/docs/docs/references/configuration/config-file.md @@ -137,6 +137,9 @@ image: # Same as '--input' input: "" + # Same as '--max-image-size' + max-size: "" + # Same as '--platform' platform: "" diff --git a/go.mod b/go.mod index d3f25c355534..e0e4b58566bf 100644 --- a/go.mod +++ b/go.mod @@ -45,6 +45,7 @@ require ( github.com/docker/cli v27.4.1+incompatible github.com/docker/docker v27.4.1+incompatible github.com/docker/go-connections v0.5.0 + github.com/docker/go-units v0.5.0 github.com/fatih/color v1.18.0 github.com/go-git/go-git/v5 v5.12.0 github.com/go-openapi/runtime v0.28.0 // indirect @@ -217,7 +218,6 @@ require ( github.com/docker/distribution v2.8.3+incompatible // indirect github.com/docker/docker-credential-helpers v0.8.2 // indirect github.com/docker/go-metrics v0.0.1 // indirect - github.com/docker/go-units v0.5.0 // indirect github.com/docker/libtrust v0.0.0-20160708172513-aabc10ec26b7 // indirect github.com/dsnet/compress v0.0.1 // indirect github.com/dustin/go-humanize v1.0.1 // indirect diff --git a/pkg/commands/artifact/run.go b/pkg/commands/artifact/run.go index 8769ede2b8cb..80d2f77710ee 100644 --- a/pkg/commands/artifact/run.go +++ b/pkg/commands/artifact/run.go @@ -587,6 +587,7 @@ func (r *runner) initScannerConfig(ctx context.Context, opts flag.Options) (Scan Host: opts.PodmanHost, }, ImageSources: opts.ImageSources, + MaxImageSize: opts.MaxImageSize, }, // For misconfiguration scanning diff --git a/pkg/fanal/artifact/image/image.go b/pkg/fanal/artifact/image/image.go index b4350b25b866..8bf9dcd9ff13 100644 --- a/pkg/fanal/artifact/image/image.go +++ b/pkg/fanal/artifact/image/image.go @@ -5,11 +5,13 @@ import ( "errors" "io" "os" + "path/filepath" "reflect" "slices" "strings" "sync" + "github.com/docker/go-units" v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/samber/lo" "golang.org/x/xerrors" @@ -36,6 +38,8 @@ type Artifact struct { handlerManager handler.Manager artifactOption artifact.Option + + cacheDir string } type LayerInfo struct { @@ -60,6 +64,11 @@ func NewArtifact(img types.Image, c cache.ArtifactCache, opt artifact.Option) (a return nil, xerrors.Errorf("config analyzer group error: %w", err) } + cacheDir, err := os.MkdirTemp("", "layers") + if err != nil { + return nil, xerrors.Errorf("failed to create a temp dir: %w", err) + } + return Artifact{ logger: log.WithPrefix("image"), image: img, @@ -70,6 +79,7 @@ func NewArtifact(img types.Image, c cache.ArtifactCache, opt artifact.Option) (a handlerManager: handlerManager, artifactOption: opt, + cacheDir: cacheDir, }, nil } @@ -88,6 +98,11 @@ func (a Artifact) Inspect(ctx context.Context) (artifact.Reference, error) { diffIDs := a.diffIDs(configFile) a.logger.Debug("Detected diff ID", log.Any("diff_ids", diffIDs)) + defer os.RemoveAll(a.cacheDir) + if err := a.checkImageSize(diffIDs); err != nil { + return artifact.Reference{}, err + } + // Try retrieving a remote SBOM document if res, err := a.retrieveRemoteSBOM(ctx); err == nil { // Found SBOM @@ -198,6 +213,56 @@ func (a Artifact) consolidateCreatedBy(diffIDs, layerKeys []string, configFile * return layerKeyMap } +func (a Artifact) checkImageSize(diffIDs []string) error { + maxSize := a.artifactOption.ImageOption.MaxImageSize + if maxSize == 0 { + return nil + } + + imageSize, err := a.imageSize(diffIDs) + if err != nil { + return xerrors.Errorf("failed to calculate image size: %w", err) + } + + if imageSize > maxSize { + return xerrors.Errorf( + "uncompressed image size %s exceeds maximum allowed size %s", + units.HumanSizeWithPrecision(float64(imageSize), 3), units.HumanSize(float64(maxSize)), + ) + } + return nil +} + +func (a Artifact) imageSize(diffIDs []string) (int64, error) { + var imageSize int64 + + for _, diffID := range diffIDs { + layerSize, err := a.saveLayer(diffID) + if err != nil { + return -1, xerrors.Errorf("failed to save layer: %w", err) + } + imageSize += layerSize + } + + return imageSize, nil +} + +func (a Artifact) saveLayer(diffID string) (int64, error) { + _, rc, err := a.uncompressedLayer(diffID) + if err != nil { + return -1, xerrors.Errorf("unable to get uncompressed layer %s: %w", diffID, err) + } + defer rc.Close() + + f, err := os.Create(filepath.Join(a.cacheDir, diffID)) + if err != nil { + return -1, xerrors.Errorf("failed to create a file: %w", err) + } + defer f.Close() + + return io.Copy(f, rc) +} + func (a Artifact) inspect(ctx context.Context, missingImage string, layerKeys, baseDiffIDs []string, layerKeyMap map[string]LayerInfo, configFile *v1.ConfigFile) error { @@ -361,6 +426,11 @@ func (a Artifact) uncompressedLayer(diffID string) (string, io.ReadCloser, error digest = d.String() } + f, err := os.Open(filepath.Join(a.cacheDir, diffID)) + if err == nil { + return digest, f, nil + } + rc, err := layer.Uncompressed() if err != nil { return "", nil, xerrors.Errorf("failed to get the layer content (%s): %w", diffID, err) diff --git a/pkg/fanal/artifact/image/image_test.go b/pkg/fanal/artifact/image/image_test.go index f7e80e3cf578..df030859eed5 100644 --- a/pkg/fanal/artifact/image/image_test.go +++ b/pkg/fanal/artifact/image/image_test.go @@ -6,6 +6,7 @@ import ( "testing" "time" + "github.com/docker/go-units" v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -348,6 +349,7 @@ func TestArtifact_Inspect(t *testing.T) { imagePath: "../../test/testdata/alpine-311.tar.gz", artifactOpt: artifact.Option{ LicenseScannerOption: analyzer.LicenseScannerOption{Full: true}, + ImageOption: types.ImageOptions{MaxImageSize: units.GB}, }, missingBlobsExpectation: cache.ArtifactCacheMissingBlobsExpectation{ Args: cache.ArtifactCacheMissingBlobsArgs{ @@ -2243,6 +2245,14 @@ func TestArtifact_Inspect(t *testing.T) { }, wantErr: "put artifact failed", }, + { + name: "sad path, image size is larger than the maximum", + imagePath: "../../test/testdata/alpine-311.tar.gz", + artifactOpt: artifact.Option{ + ImageOption: types.ImageOptions{MaxImageSize: units.MB * 1}, + }, + wantErr: "uncompressed image size 5.86MB exceeds maximum allowed size 1MB", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/fanal/types/image.go b/pkg/fanal/types/image.go index 91cdfb44b60c..ac8406eaa322 100644 --- a/pkg/fanal/types/image.go +++ b/pkg/fanal/types/image.go @@ -53,6 +53,7 @@ type ImageOptions struct { PodmanOptions PodmanOptions ContainerdOptions ContainerdOptions ImageSources ImageSources + MaxImageSize int64 } type DockerOptions struct { diff --git a/pkg/flag/image_flags.go b/pkg/flag/image_flags.go index aaa3fc65b930..d56cae500cdf 100644 --- a/pkg/flag/image_flags.go +++ b/pkg/flag/image_flags.go @@ -1,6 +1,7 @@ package flag import ( + "github.com/docker/go-units" v1 "github.com/google/go-containerregistry/pkg/v1" "golang.org/x/xerrors" @@ -58,6 +59,12 @@ var ( Values: xstrings.ToStringSlice(ftypes.AllImageSources), Usage: "image source(s) to use, in priority order", } + MaxImageSize = Flag[string]{ + Name: "max-image-size", + ConfigName: "image.max-size", + Default: "", + Usage: "maximum image size to process, specified in a human-readable format (e.g., '44kB', '17MB'); an error will be returned if the image exceeds this size", + } ) type ImageFlagGroup struct { @@ -68,6 +75,7 @@ type ImageFlagGroup struct { DockerHost *Flag[string] PodmanHost *Flag[string] ImageSources *Flag[[]string] + MaxImageSize *Flag[string] } type ImageOptions struct { @@ -78,6 +86,7 @@ type ImageOptions struct { DockerHost string PodmanHost string ImageSources ftypes.ImageSources + MaxImageSize int64 } func NewImageFlagGroup() *ImageFlagGroup { @@ -89,6 +98,7 @@ func NewImageFlagGroup() *ImageFlagGroup { DockerHost: DockerHostFlag.Clone(), PodmanHost: PodmanHostFlag.Clone(), ImageSources: SourceFlag.Clone(), + MaxImageSize: MaxImageSize.Clone(), } } @@ -105,6 +115,7 @@ func (f *ImageFlagGroup) Flags() []Flagger { f.DockerHost, f.PodmanHost, f.ImageSources, + f.MaxImageSize, } } @@ -124,6 +135,14 @@ func (f *ImageFlagGroup) ToOptions() (ImageOptions, error) { } platform = ftypes.Platform{Platform: pl} } + var maxSize int64 + if value := f.MaxImageSize.Value(); value != "" { + parsedSize, err := units.FromHumanSize(value) + if err != nil { + return ImageOptions{}, xerrors.Errorf("invalid max image size %q: %w", value, err) + } + maxSize = parsedSize + } return ImageOptions{ Input: f.Input.Value(), @@ -133,5 +152,6 @@ func (f *ImageFlagGroup) ToOptions() (ImageOptions, error) { DockerHost: f.DockerHost.Value(), PodmanHost: f.PodmanHost.Value(), ImageSources: xstrings.ToTSlice[ftypes.ImageSource](f.ImageSources.Value()), + MaxImageSize: maxSize, }, nil } From eddcbf8f9dab99875853b76d6c845220cd333993 Mon Sep 17 00:00:00 2001 From: nikpivkin Date: Wed, 25 Dec 2024 23:31:07 +0600 Subject: [PATCH 2/8] refactor: pull layers in parallel Signed-off-by: nikpivkin --- pkg/fanal/artifact/image/image.go | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/pkg/fanal/artifact/image/image.go b/pkg/fanal/artifact/image/image.go index 8bf9dcd9ff13..a2898998fe54 100644 --- a/pkg/fanal/artifact/image/image.go +++ b/pkg/fanal/artifact/image/image.go @@ -99,7 +99,7 @@ func (a Artifact) Inspect(ctx context.Context) (artifact.Reference, error) { a.logger.Debug("Detected diff ID", log.Any("diff_ids", diffIDs)) defer os.RemoveAll(a.cacheDir) - if err := a.checkImageSize(diffIDs); err != nil { + if err := a.checkImageSize(ctx, diffIDs); err != nil { return artifact.Reference{}, err } @@ -213,13 +213,13 @@ func (a Artifact) consolidateCreatedBy(diffIDs, layerKeys []string, configFile * return layerKeyMap } -func (a Artifact) checkImageSize(diffIDs []string) error { +func (a Artifact) checkImageSize(ctx context.Context, diffIDs []string) error { maxSize := a.artifactOption.ImageOption.MaxImageSize if maxSize == 0 { return nil } - imageSize, err := a.imageSize(diffIDs) + imageSize, err := a.imageSize(ctx, diffIDs) if err != nil { return xerrors.Errorf("failed to calculate image size: %w", err) } @@ -233,15 +233,25 @@ func (a Artifact) checkImageSize(diffIDs []string) error { return nil } -func (a Artifact) imageSize(diffIDs []string) (int64, error) { +func (a Artifact) imageSize(ctx context.Context, diffIDs []string) (int64, error) { var imageSize int64 - for _, diffID := range diffIDs { - layerSize, err := a.saveLayer(diffID) - if err != nil { - return -1, xerrors.Errorf("failed to save layer: %w", err) - } - imageSize += layerSize + p := parallel.NewPipeline(a.artifactOption.Parallel, false, diffIDs, + func(_ context.Context, diffID string) (int64, error) { + layerSize, err := a.saveLayer(diffID) + if err != nil { + return -1, xerrors.Errorf("failed to save layer: %w", err) + } + return layerSize, nil + }, + func(layerSize int64) error { + imageSize += layerSize + return nil + }, + ) + + if err := p.Do(ctx); err != nil { + return -1, xerrors.Errorf("pipeline error: %w", err) } return imageSize, nil From 29027791ee46f963292669a2b6db0c4866ce56da Mon Sep 17 00:00:00 2001 From: nikpivkin Date: Thu, 26 Dec 2024 12:58:01 +0600 Subject: [PATCH 3/8] refactor: clear an error when image size is exceeded Signed-off-by: nikpivkin --- cmd/trivy/main.go | 8 ++++++++ pkg/fanal/artifact/image/image.go | 13 +++++++++---- pkg/types/error.go | 9 +++++++++ 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/cmd/trivy/main.go b/cmd/trivy/main.go index 6cbda13b9cf3..58167a2a079c 100644 --- a/cmd/trivy/main.go +++ b/cmd/trivy/main.go @@ -3,6 +3,7 @@ package main import ( "context" "errors" + "fmt" "os" "golang.org/x/xerrors" @@ -21,6 +22,13 @@ func main() { if errors.As(err, &exitError) { os.Exit(exitError.Code) } + + var userErr *types.UserError + if errors.As(err, &userErr) { + fmt.Println("Error: " + userErr.Error()) + os.Exit(1) + } + log.Fatal("Fatal error", log.Err(err)) } } diff --git a/pkg/fanal/artifact/image/image.go b/pkg/fanal/artifact/image/image.go index a2898998fe54..a5d92d905e3f 100644 --- a/pkg/fanal/artifact/image/image.go +++ b/pkg/fanal/artifact/image/image.go @@ -3,6 +3,7 @@ package image import ( "context" "errors" + "fmt" "io" "os" "path/filepath" @@ -26,6 +27,7 @@ import ( "github.com/aquasecurity/trivy/pkg/log" "github.com/aquasecurity/trivy/pkg/parallel" "github.com/aquasecurity/trivy/pkg/semaphore" + trivyTypes "github.com/aquasecurity/trivy/pkg/types" ) type Artifact struct { @@ -225,10 +227,13 @@ func (a Artifact) checkImageSize(ctx context.Context, diffIDs []string) error { } if imageSize > maxSize { - return xerrors.Errorf( - "uncompressed image size %s exceeds maximum allowed size %s", - units.HumanSizeWithPrecision(float64(imageSize), 3), units.HumanSize(float64(maxSize)), - ) + return &trivyTypes.UserError{ + Message: fmt.Sprintf( + "uncompressed image size %s exceeds maximum allowed size %s", + units.HumanSizeWithPrecision(float64(imageSize), 3), + units.HumanSize(float64(maxSize)), + ), + } } return nil } diff --git a/pkg/types/error.go b/pkg/types/error.go index 5a1614e07dd0..6ea8ced4e34a 100644 --- a/pkg/types/error.go +++ b/pkg/types/error.go @@ -11,3 +11,12 @@ type ExitError struct { func (e *ExitError) Error() string { return fmt.Sprintf("exit status %d", e.Code) } + +// UserError represents an error with a user-friendly message. +type UserError struct { + Message string +} + +func (e *UserError) Error() string { + return e.Message +} From adaea747a3cb61c63b860cb572f511ac3e2545a6 Mon Sep 17 00:00:00 2001 From: nikpivkin Date: Thu, 26 Dec 2024 14:08:52 +0600 Subject: [PATCH 4/8] test: add tests for image flags Signed-off-by: nikpivkin --- pkg/flag/image_flags_test.go | 91 ++++++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 pkg/flag/image_flags_test.go diff --git a/pkg/flag/image_flags_test.go b/pkg/flag/image_flags_test.go new file mode 100644 index 000000000000..97105f6b8449 --- /dev/null +++ b/pkg/flag/image_flags_test.go @@ -0,0 +1,91 @@ +package flag_test + +import ( + "testing" + + "github.com/docker/go-units" + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/spf13/viper" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/aquasecurity/trivy/pkg/fanal/types" + "github.com/aquasecurity/trivy/pkg/flag" +) + +func TestImageFlagGroup_ToOptions(t *testing.T) { + type fields struct { + maxImgSize string + platform string + } + tests := []struct { + name string + fields fields + want flag.ImageOptions + wantErr string + }{ + { + name: "happy default (without flags)", + fields: fields{}, + want: flag.ImageOptions{}, + }, + { + name: "happy path with max image size", + fields: fields{ + maxImgSize: "10mb", + }, + want: flag.ImageOptions{ + MaxImageSize: units.MB * 10, + }, + }, + { + name: "invalid max image size", + fields: fields{ + maxImgSize: "10foo", + }, + wantErr: "invalid max image size", + }, + { + name: "happy path with platform", + fields: fields{ + platform: "linux/amd64", + }, + want: flag.ImageOptions{ + Platform: types.Platform{ + Platform: &v1.Platform{ + OS: "linux", + Architecture: "amd64", + }, + }, + }, + }, + { + name: "invalid platform", + fields: fields{ + platform: "unknown/unknown/unknown/unknown", + }, + wantErr: "unable to parse platform", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Cleanup(viper.Reset) + + setValue(flag.MaxImageSize.ConfigName, tt.fields.maxImgSize) + setValue(flag.PlatformFlag.ConfigName, tt.fields.platform) + + f := &flag.ImageFlagGroup{ + MaxImageSize: flag.MaxImageSize.Clone(), + Platform: flag.PlatformFlag.Clone(), + } + + got, err := f.ToOptions() + if tt.wantErr != "" { + assert.ErrorContains(t, err, tt.wantErr) + return + } + require.NoError(t, err) + assert.EqualExportedValues(t, tt.want, got) + }) + } +} From a6ce2edbc161eaa0a0db257fc106ab2bd629093c Mon Sep 17 00:00:00 2001 From: nikpivkin Date: Thu, 26 Dec 2024 14:37:53 +0600 Subject: [PATCH 5/8] test: add integration tests Signed-off-by: nikpivkin --- integration/docker_engine_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/integration/docker_engine_test.go b/integration/docker_engine_test.go index a543d22e466f..ff36fc0ddbf1 100644 --- a/integration/docker_engine_test.go +++ b/integration/docker_engine_test.go @@ -25,6 +25,7 @@ func TestDockerEngine(t *testing.T) { ignoreStatus []string severity []string ignoreIDs []string + maxImageSize string input string golden string wantErr string @@ -34,6 +35,12 @@ func TestDockerEngine(t *testing.T) { input: "testdata/fixtures/images/alpine-39.tar.gz", golden: "testdata/alpine-39.json.golden", }, + { + name: "alpine:3.9, with max image size", + maxImageSize: "100mb", + input: "testdata/fixtures/images/alpine-39.tar.gz", + golden: "testdata/alpine-39.json.golden", + }, { name: "alpine:3.9, with high and critical severity", severity: []string{ @@ -195,6 +202,12 @@ func TestDockerEngine(t *testing.T) { input: "badimage:latest", wantErr: "unable to inspect the image (badimage:latest)", }, + { + name: "sad path, image size is larger than the maximum", + input: "testdata/fixtures/images/alpine-39.tar.gz", + maxImageSize: "1mb", + wantErr: "uncompressed image size 5.8MB exceeds maximum allowed size 1MB", + }, } // Set up testing DB @@ -263,6 +276,11 @@ func TestDockerEngine(t *testing.T) { require.NoError(t, err, "failed to write .trivyignore") defer os.Remove(trivyIgnore) } + + if tt.maxImageSize != "" { + osArgs = append(osArgs, []string{"--max-image-size", tt.maxImageSize}...) + } + osArgs = append(osArgs, tt.input) // Run Trivy From bba405b6f2efce6e051aa02e0bb8d477961a1307 Mon Sep 17 00:00:00 2001 From: nikpivkin Date: Fri, 27 Dec 2024 14:44:21 +0600 Subject: [PATCH 6/8] refactor: remove cached layers in clean Signed-off-by: nikpivkin --- pkg/fanal/artifact/image/image.go | 16 +++++++++------- pkg/fanal/artifact/image/image_test.go | 2 ++ pkg/fanal/artifact/image/remote_sbom_test.go | 3 +++ pkg/fanal/test/integration/registry_test.go | 1 + 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/pkg/fanal/artifact/image/image.go b/pkg/fanal/artifact/image/image.go index a5d92d905e3f..6419998c6286 100644 --- a/pkg/fanal/artifact/image/image.go +++ b/pkg/fanal/artifact/image/image.go @@ -41,7 +41,7 @@ type Artifact struct { artifactOption artifact.Option - cacheDir string + layerCacheDir string } type LayerInfo struct { @@ -81,7 +81,7 @@ func NewArtifact(img types.Image, c cache.ArtifactCache, opt artifact.Option) (a handlerManager: handlerManager, artifactOption: opt, - cacheDir: cacheDir, + layerCacheDir: cacheDir, }, nil } @@ -100,8 +100,10 @@ func (a Artifact) Inspect(ctx context.Context) (artifact.Reference, error) { diffIDs := a.diffIDs(configFile) a.logger.Debug("Detected diff ID", log.Any("diff_ids", diffIDs)) - defer os.RemoveAll(a.cacheDir) if err := a.checkImageSize(ctx, diffIDs); err != nil { + if err := os.RemoveAll(a.layerCacheDir); err != nil { + log.Error("Failed to remove layer cache", log.Err(err)) + } return artifact.Reference{}, err } @@ -158,8 +160,8 @@ func (a Artifact) Inspect(ctx context.Context) (artifact.Reference, error) { }, nil } -func (Artifact) Clean(_ artifact.Reference) error { - return nil +func (a Artifact) Clean(_ artifact.Reference) error { + return os.RemoveAll(a.layerCacheDir) } func (a Artifact) calcCacheKeys(imageID string, diffIDs []string) (string, []string, error) { @@ -269,7 +271,7 @@ func (a Artifact) saveLayer(diffID string) (int64, error) { } defer rc.Close() - f, err := os.Create(filepath.Join(a.cacheDir, diffID)) + f, err := os.Create(filepath.Join(a.layerCacheDir, diffID)) if err != nil { return -1, xerrors.Errorf("failed to create a file: %w", err) } @@ -441,7 +443,7 @@ func (a Artifact) uncompressedLayer(diffID string) (string, io.ReadCloser, error digest = d.String() } - f, err := os.Open(filepath.Join(a.cacheDir, diffID)) + f, err := os.Open(filepath.Join(a.layerCacheDir, diffID)) if err == nil { return digest, f, nil } diff --git a/pkg/fanal/artifact/image/image_test.go b/pkg/fanal/artifact/image/image_test.go index df030859eed5..29c0a7c06535 100644 --- a/pkg/fanal/artifact/image/image_test.go +++ b/pkg/fanal/artifact/image/image_test.go @@ -2272,6 +2272,8 @@ func TestArtifact_Inspect(t *testing.T) { assert.ErrorContains(t, err, tt.wantErr, tt.name) return } + defer a.Clean(got) + require.NoError(t, err, tt.name) assert.Equal(t, tt.want, got) }) diff --git a/pkg/fanal/artifact/image/remote_sbom_test.go b/pkg/fanal/artifact/image/remote_sbom_test.go index bdd2562101f1..1999d732af82 100644 --- a/pkg/fanal/artifact/image/remote_sbom_test.go +++ b/pkg/fanal/artifact/image/remote_sbom_test.go @@ -170,6 +170,8 @@ func TestArtifact_InspectRekorAttestation(t *testing.T) { assert.ErrorContains(t, err, tt.wantErr) return } + defer a.Clean(got) + require.NoError(t, err, tt.name) got.BOM = nil assert.Equal(t, tt.want, got) @@ -312,6 +314,7 @@ func TestArtifact_inspectOCIReferrerSBOM(t *testing.T) { assert.ErrorContains(t, err, tt.wantErr) return } + defer a.Clean(got) require.NoError(t, err, tt.name) got.BOM = nil diff --git a/pkg/fanal/test/integration/registry_test.go b/pkg/fanal/test/integration/registry_test.go index 35a32536600e..e6fc7445cc87 100644 --- a/pkg/fanal/test/integration/registry_test.go +++ b/pkg/fanal/test/integration/registry_test.go @@ -256,6 +256,7 @@ func analyze(ctx context.Context, imageRef string, opt types.ImageOptions) (*typ if err != nil { return nil, err } + defer ar.Clean(imageInfo) imageDetail, err := ap.ApplyLayers(imageInfo.ID, imageInfo.BlobIDs) if err != nil { From db8e61c52ad62461940abca8adc11ba29f7a3cb1 Mon Sep 17 00:00:00 2001 From: nikpivkin Date: Fri, 27 Dec 2024 18:20:27 +0600 Subject: [PATCH 7/8] feat(image): check compressed size Signed-off-by: nikpivkin --- integration/docker_engine_test.go | 4 +-- pkg/fanal/artifact/image/image.go | 47 +++++++++++++++++++++++--- pkg/fanal/artifact/image/image_test.go | 12 +++++-- 3 files changed, 54 insertions(+), 9 deletions(-) diff --git a/integration/docker_engine_test.go b/integration/docker_engine_test.go index ff36fc0ddbf1..fe557efe2a65 100644 --- a/integration/docker_engine_test.go +++ b/integration/docker_engine_test.go @@ -205,8 +205,8 @@ func TestDockerEngine(t *testing.T) { { name: "sad path, image size is larger than the maximum", input: "testdata/fixtures/images/alpine-39.tar.gz", - maxImageSize: "1mb", - wantErr: "uncompressed image size 5.8MB exceeds maximum allowed size 1MB", + maxImageSize: "3mb", + wantErr: "uncompressed image size 5.8MB exceeds maximum allowed size 3MB", }, } diff --git a/pkg/fanal/artifact/image/image.go b/pkg/fanal/artifact/image/image.go index 6419998c6286..59d07752f521 100644 --- a/pkg/fanal/artifact/image/image.go +++ b/pkg/fanal/artifact/image/image.go @@ -217,12 +217,31 @@ func (a Artifact) consolidateCreatedBy(diffIDs, layerKeys []string, configFile * return layerKeyMap } +func limitErrorMessage(typ string, maxSize, imageSize int64) string { + return fmt.Sprintf( + "%s image size %s exceeds maximum allowed size %s", typ, + units.HumanSizeWithPrecision(float64(imageSize), 3), + units.HumanSize(float64(maxSize)), + ) +} + func (a Artifact) checkImageSize(ctx context.Context, diffIDs []string) error { maxSize := a.artifactOption.ImageOption.MaxImageSize if maxSize == 0 { return nil } + compressedSize, err := a.compressedImageSize(diffIDs) + if err != nil { + return nil + } + + if compressedSize > maxSize { + return &trivyTypes.UserError{ + Message: limitErrorMessage("compressed", maxSize, compressedSize), + } + } + imageSize, err := a.imageSize(ctx, diffIDs) if err != nil { return xerrors.Errorf("failed to calculate image size: %w", err) @@ -230,16 +249,34 @@ func (a Artifact) checkImageSize(ctx context.Context, diffIDs []string) error { if imageSize > maxSize { return &trivyTypes.UserError{ - Message: fmt.Sprintf( - "uncompressed image size %s exceeds maximum allowed size %s", - units.HumanSizeWithPrecision(float64(imageSize), 3), - units.HumanSize(float64(maxSize)), - ), + Message: limitErrorMessage("uncompressed", maxSize, imageSize), } } return nil } +func (a Artifact) compressedImageSize(diffIDs []string) (int64, error) { + var totalSize int64 + for _, diffID := range diffIDs { + h, err := v1.NewHash(diffID) + if err != nil { + return -1, xerrors.Errorf("invalid layer ID (%s): %w", diffID, err) + } + + layer, err := a.image.LayerByDiffID(h) + if err != nil { + return -1, xerrors.Errorf("failed to get the layer (%s): %w", diffID, err) + } + layerSize, err := layer.Size() + if err != nil { + return -1, xerrors.Errorf("failed to get layer size: %w", err) + } + totalSize += layerSize + } + + return totalSize, nil +} + func (a Artifact) imageSize(ctx context.Context, diffIDs []string) (int64, error) { var imageSize int64 diff --git a/pkg/fanal/artifact/image/image_test.go b/pkg/fanal/artifact/image/image_test.go index 29c0a7c06535..e2fdf1417f45 100644 --- a/pkg/fanal/artifact/image/image_test.go +++ b/pkg/fanal/artifact/image/image_test.go @@ -2246,12 +2246,20 @@ func TestArtifact_Inspect(t *testing.T) { wantErr: "put artifact failed", }, { - name: "sad path, image size is larger than the maximum", + name: "sad path, compressed image size is larger than the maximum", imagePath: "../../test/testdata/alpine-311.tar.gz", artifactOpt: artifact.Option{ ImageOption: types.ImageOptions{MaxImageSize: units.MB * 1}, }, - wantErr: "uncompressed image size 5.86MB exceeds maximum allowed size 1MB", + wantErr: "compressed image size 3.03MB exceeds maximum allowed size 1MB", + }, + { + name: "sad path, image size is larger than the maximum", + imagePath: "../../test/testdata/alpine-311.tar.gz", + artifactOpt: artifact.Option{ + ImageOption: types.ImageOptions{MaxImageSize: units.MB * 4}, + }, + wantErr: "uncompressed image size 5.86MB exceeds maximum allowed size 4MB", }, } for _, tt := range tests { From e3a827b343e775ed0485002eb37b667b53b3ed1a Mon Sep 17 00:00:00 2001 From: nikpivkin Date: Fri, 27 Dec 2024 18:25:57 +0600 Subject: [PATCH 8/8] refactor: use log for logging error on exit Signed-off-by: nikpivkin --- cmd/trivy/main.go | 4 +--- pkg/fanal/artifact/image/image.go | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/cmd/trivy/main.go b/cmd/trivy/main.go index 58167a2a079c..84d8879fbdbc 100644 --- a/cmd/trivy/main.go +++ b/cmd/trivy/main.go @@ -3,7 +3,6 @@ package main import ( "context" "errors" - "fmt" "os" "golang.org/x/xerrors" @@ -25,8 +24,7 @@ func main() { var userErr *types.UserError if errors.As(err, &userErr) { - fmt.Println("Error: " + userErr.Error()) - os.Exit(1) + log.Fatal("Error", log.Err(userErr)) } log.Fatal("Fatal error", log.Err(err)) diff --git a/pkg/fanal/artifact/image/image.go b/pkg/fanal/artifact/image/image.go index 59d07752f521..89b2bc57f4f6 100644 --- a/pkg/fanal/artifact/image/image.go +++ b/pkg/fanal/artifact/image/image.go @@ -68,7 +68,7 @@ func NewArtifact(img types.Image, c cache.ArtifactCache, opt artifact.Option) (a cacheDir, err := os.MkdirTemp("", "layers") if err != nil { - return nil, xerrors.Errorf("failed to create a temp dir: %w", err) + return nil, xerrors.Errorf("failed to create a cache layers temp dir: %w", err) } return Artifact{