diff --git a/.golangci.yml b/.golangci.yml index a7466bd9..4d409185 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,124 +1,125 @@ -# .golangci.yml - -version: "2" -run: - timeout: 10m - -linters: - enable: - - bodyclose - - depguard - - dogsled - - dupl - - errcheck - - funlen - - gochecknoinits - - goconst - - gocritic - - gocyclo - - ineffassign - - mnd # replacement for gomnd - - nakedret - - revive # replacement for golint - - rowserrcheck - - staticcheck - - unconvert - - unparam - - unused # covers deadcode/varcheck/structcheck - - whitespace - exclusions: - paths: - - test/testdata_etc - - internal/cache - - internal/renameio - - internal/robustio - rules: - - path: _test\.go - linters: - - mnd - settings: - depguard: - rules: - main: - list-mode: lax - allow: - - $gostd - - github.com/checkmarx/ast-cli/internal - - github.com/gookit/color - - github.com/CheckmarxDev/containers-resolver/pkg/containerResolver - - github.com/Checkmarx/manifest-parser/pkg/parser/models - - github.com/Checkmarx/manifest-parser/pkg/parser - - github.com/Checkmarx/secret-detection/pkg/hooks/pre-commit - - github.com/Checkmarx/secret-detection/pkg/hooks/pre-receive - - github.com/Checkmarx/gen-ai-prompts/prompts/sast_result_remediation - - github.com/spf13/viper - - github.com/checkmarx/2ms/v3/lib/reporting - - github.com/checkmarx/2ms/v3/lib/secrets - - github.com/checkmarx/2ms/v3/pkg - - github.com/Checkmarx/gen-ai-wrapper - - github.com/spf13/cobra - - github.com/pkg/errors - - github.com/google - - github.com/MakeNowJust/heredoc - - github.com/jsumners/go-getport - - github.com/stretchr/testify/assert - - github.com/gofrs/flock - - github.com/golang-jwt/jwt/v5 - - github.com/checkmarx/go-keyring - - github.com/Checkmarx/containers-images-extractor/pkg/imagesExtractor - - github.com/Checkmarx/containers-types/types - dupl: - threshold: 500 - funlen: - lines: 200 - statements: 100 - goconst: - min-len: 2 - min-occurrences: 2 - gocritic: - enabled-tags: - - diagnostic - - experimental - - opinionated - - performance - - style - disabled-checks: - - dupImport # https://github.com/go-critic/go-critic/issues/845 - - ifElseChain - - octalLiteral - - whyNoLint - - wrapperFunc - gocyclo: - min-complexity: 15 - mnd: - checks: - - argument - - case - - condition - - return - revive: - rules: - - name: exported - arguments: - - disableStutteringCheck - govet: - settings: - printf: - funcs: - - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof - - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf - - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf - - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf - lll: - line-length: 185 - misspell: - locale: US - -formatters: - enable: - - gofmt - - goimports - settings: - goimports: - local-prefixes: - - github.com/golangci/golangci-lint +# .golangci.yml + +version: "2" +run: + timeout: 10m + +linters: + enable: + - bodyclose + - depguard + - dogsled + - dupl + - errcheck + - funlen + - gochecknoinits + - goconst + - gocritic + - gocyclo + - ineffassign + - mnd # replacement for gomnd + - nakedret + - revive # replacement for golint + - rowserrcheck + - staticcheck + - unconvert + - unparam + - unused # covers deadcode/varcheck/structcheck + - whitespace + exclusions: + paths: + - test/testdata_etc + - internal/cache + - internal/renameio + - internal/robustio + rules: + - path: _test\.go + linters: + - mnd + settings: + depguard: + rules: + main: + list-mode: lax + allow: + - $gostd + - github.com/checkmarx/ast-cli/internal + - github.com/gookit/color + - github.com/CheckmarxDev/containers-resolver/pkg/containerResolver + - github.com/Checkmarx/manifest-parser/pkg/parser/models + - github.com/Checkmarx/manifest-parser/pkg/parser + - github.com/Checkmarx/secret-detection/pkg/hooks/pre-commit + - github.com/Checkmarx/secret-detection/pkg/hooks/pre-receive + - github.com/Checkmarx/gen-ai-prompts/prompts/sast_result_remediation + - github.com/spf13/viper + - github.com/checkmarx/2ms/v3/lib/reporting + - github.com/checkmarx/2ms/v3/lib/secrets + - github.com/checkmarx/2ms/v3/pkg + - github.com/Checkmarx/gen-ai-wrapper + - github.com/spf13/cobra + - github.com/pkg/errors + - github.com/google + - github.com/MakeNowJust/heredoc + - github.com/jsumners/go-getport + - github.com/stretchr/testify/assert + - github.com/gofrs/flock + - github.com/golang-jwt/jwt/v5 + - github.com/checkmarx/go-keyring + - github.com/Checkmarx/containers-images-extractor/pkg/imagesExtractor + - github.com/Checkmarx/containers-syft-packages-extractor/pkg/syftPackagesExtractor + - github.com/Checkmarx/containers-types/types + dupl: + threshold: 500 + funlen: + lines: 200 + statements: 100 + goconst: + min-len: 2 + min-occurrences: 2 + gocritic: + enabled-tags: + - diagnostic + - experimental + - opinionated + - performance + - style + disabled-checks: + - dupImport # https://github.com/go-critic/go-critic/issues/845 + - ifElseChain + - octalLiteral + - whyNoLint + - wrapperFunc + gocyclo: + min-complexity: 15 + mnd: + checks: + - argument + - case + - condition + - return + revive: + rules: + - name: exported + arguments: + - disableStutteringCheck + govet: + settings: + printf: + funcs: + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf + lll: + line-length: 185 + misspell: + locale: US + +formatters: + enable: + - gofmt + - goimports + settings: + goimports: + local-prefixes: + - github.com/golangci/golangci-lint diff --git a/go.mod b/go.mod index c1949950..49d7be7f 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.26.6 require ( github.com/Checkmarx/ast-cx-hooks v1.0.9 - github.com/Checkmarx/containers-resolver v1.0.34 + github.com/Checkmarx/containers-resolver v1.0.37 github.com/Checkmarx/containers-types v1.0.9 github.com/Checkmarx/gen-ai-prompts v0.0.0-20240807143411-708ceec12b63 github.com/Checkmarx/gen-ai-wrapper v1.0.3 @@ -77,7 +77,7 @@ require ( github.com/BobuSumisu/aho-corasick v1.0.3 // indirect github.com/BurntSushi/toml v1.6.0 // indirect github.com/Checkmarx/containers-images-extractor v1.0.22 - github.com/Checkmarx/containers-syft-packages-extractor v1.0.25 // indirect + github.com/Checkmarx/containers-syft-packages-extractor v1.0.27 github.com/CycloneDX/cyclonedx-go v0.10.0 // indirect github.com/DataDog/zstd v1.5.6 // indirect github.com/Masterminds/goutils v1.1.1 // indirect @@ -245,7 +245,7 @@ require ( github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect github.com/rivo/uniseg v0.4.7 // indirect - github.com/rs/zerolog v1.34.0 // indirect + github.com/rs/zerolog v1.35.1 // indirect github.com/rubenv/sql-migrate v1.8.1 // indirect github.com/russross/blackfriday/v2 v2.1.0 // indirect github.com/rust-secure-code/go-rustaudit v0.0.0-20250226111315-e20ec32e963c // indirect diff --git a/go.sum b/go.sum index 88edeeb9..33b6c499 100644 --- a/go.sum +++ b/go.sum @@ -69,10 +69,10 @@ github.com/Checkmarx/ast-cx-hooks v1.0.9 h1:NZ8Ekjxbe/0tj48Lv1rKVzwo2iGMXG06vD6H github.com/Checkmarx/ast-cx-hooks v1.0.9/go.mod h1:GPHk8IJHQlCW7l8ye9/Bij57zYQGRG+pxJPiGgsR8cY= github.com/Checkmarx/containers-images-extractor v1.0.22 h1:kJZgwk28LwJZ7Xky+kzwL+JSZOlpwrGsZQhhz4L2t6s= github.com/Checkmarx/containers-images-extractor v1.0.22/go.mod h1:HyzVb8TtTDf56hGlSakalPXtzjJ6VhTYe9fmAcOS+V8= -github.com/Checkmarx/containers-resolver v1.0.34 h1:KULN8s8xb1tQtdH4yzHVdwN8GyLqtPCAkFWra10k7V0= -github.com/Checkmarx/containers-resolver v1.0.34/go.mod h1:VTZR+NQJnPrCSIkdWL4bdTshvPQI1mRpG+EQzLuQsWo= -github.com/Checkmarx/containers-syft-packages-extractor v1.0.25 h1:xKjQzVZkisZeqEHu8nXtNRYYDaAwSrxQhXbzeDDc/68= -github.com/Checkmarx/containers-syft-packages-extractor v1.0.25/go.mod h1:OPGYISPnKtVFl2mZrClErv83ZLjUPKjdQQsXLmx++oY= +github.com/Checkmarx/containers-resolver v1.0.37 h1:VkLuw8QXt04VvdWzjfoTHAqC7imizUp3fvLaTRRmHYs= +github.com/Checkmarx/containers-resolver v1.0.37/go.mod h1:aFcQhIKw98lYzh9Wl6LX11WRa3OarixFnx1IR7uMj9U= +github.com/Checkmarx/containers-syft-packages-extractor v1.0.27 h1:7wvrAiiXFGyd4C6jw90Sq35v1otofcy1yHtvzINV7tc= +github.com/Checkmarx/containers-syft-packages-extractor v1.0.27/go.mod h1:OPGYISPnKtVFl2mZrClErv83ZLjUPKjdQQsXLmx++oY= github.com/Checkmarx/containers-types v1.0.9 h1:LbHDj9LZ0x3f28wDx398WC19sw0U0EfEewHMLStBwvs= github.com/Checkmarx/containers-types v1.0.9/go.mod h1:KR0w8XCosq3+6jRCfQrH7i//Nj2u11qaUJM62CREFZA= github.com/Checkmarx/gen-ai-prompts v0.0.0-20240807143411-708ceec12b63 h1:SCuTcE+CFvgjbIxUNL8rsdB2sAhfuNx85HvxImKta3g= @@ -277,7 +277,6 @@ github.com/containerd/typeurl/v2 v2.3.0 h1:HZHPhRWo5XMy3QGQoPrUzbW/2ckwjfweHmOwl github.com/containerd/typeurl/v2 v2.3.0/go.mod h1:Qk+PAdUYArVj41TnGi6rJ+48RF0PkcTc4i/taoBcK0w= github.com/coreos/go-semver v0.3.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk= github.com/coreos/go-systemd/v22 v22.3.2/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= -github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= github.com/coreos/go-systemd/v22 v22.7.0 h1:LAEzFkke61DFROc7zNLX/WA2i5J8gYqe0rSj9KI28KA= github.com/coreos/go-systemd/v22 v22.7.0/go.mod h1:xNUYtjHu2EDXbsxz1i41wouACIwT7Ybq9o0BQhMwD0w= github.com/cpuguy83/go-md2man/v2 v2.0.1/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= @@ -707,7 +706,6 @@ github.com/mattn/go-colorable v0.1.4/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVc github.com/mattn/go-colorable v0.1.6/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= github.com/mattn/go-colorable v0.1.9/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= github.com/mattn/go-colorable v0.1.12/go.mod h1:u5H1YNBxpqRaxsYJYSkiCWKzEfiAb1Gb520KVy5xxl4= -github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg= github.com/mattn/go-colorable v0.1.14 h1:9A9LHSqF/7dyVVX6g0U9cwm9pG3kP9gSzcuIPHPsaIE= github.com/mattn/go-colorable v0.1.14/go.mod h1:6LmQG8QLFO4G5z1gPvYEzlUgJ2wF+stgPZH1UqBm1s8= github.com/mattn/go-isatty v0.0.3/go.mod h1:M+lRXTBqGeGNdLjl/ufCoiOlB5xdOkqRJdNxMWT7Zi4= @@ -716,8 +714,6 @@ github.com/mattn/go-isatty v0.0.10/go.mod h1:qgIWMr58cqv1PHHyhnkY9lrL7etaEgOFcME github.com/mattn/go-isatty v0.0.11/go.mod h1:PhnuNfih5lzO57/f3n+odYbM4JtupLOxQOAqxQCu2WE= github.com/mattn/go-isatty v0.0.12/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Kysco4FUpU= github.com/mattn/go-isatty v0.0.14/go.mod h1:7GGIvUiUoEMVVmxf/4nioHXj79iQHKdU27kJ6hsGG94= -github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM= -github.com/mattn/go-isatty v0.0.19/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= github.com/mattn/go-localereader v0.0.2-0.20220822084749-2491eb6c1c75 h1:P8UmIzZMYDR+NGImiFvErt6VWfIRPuGM+vyjiEdkmIw= @@ -879,9 +875,8 @@ github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6L github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ= github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc= -github.com/rs/xid v1.6.0/go.mod h1:7XoLgs4eV+QndskICGsho+ADou8ySMSjJKDIan90Nz0= -github.com/rs/zerolog v1.34.0 h1:k43nTLIwcTVQAncfCw4KZ2VY6ukYoZaBPNOE8txlOeY= -github.com/rs/zerolog v1.34.0/go.mod h1:bJsvje4Z08ROH4Nhs5iH600c3IkWhwp44iRc54W6wYQ= +github.com/rs/zerolog v1.35.1 h1:m7xQeoiLIiV0BCEY4Hs+j2NG4Gp2o2KPKmhnnLiazKI= +github.com/rs/zerolog v1.35.1/go.mod h1:EjML9kdfa/RMA7h/6z6pYmq1ykOuA8/mjWaEvGI+jcw= github.com/rubenv/sql-migrate v1.8.1 h1:EPNwCvjAowHI3TnZ+4fQu3a915OpnQoPAjTXCGOy2U0= github.com/rubenv/sql-migrate v1.8.1/go.mod h1:BTIKBORjzyxZDS6dzoiw6eAFYJ1iNlGAtjn4LGeVjS8= github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk= @@ -1313,10 +1308,8 @@ golang.org/x/sys v0.0.0-20220310020820-b874c991c1a5/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= diff --git a/internal/commands/scan.go b/internal/commands/scan.go index 41b24008..c109566c 100644 --- a/internal/commands/scan.go +++ b/internal/commands/scan.go @@ -22,6 +22,8 @@ import ( "time" "unicode" + syftExtractor "github.com/Checkmarx/containers-syft-packages-extractor/pkg/syftPackagesExtractor" + "github.com/Checkmarx/containers-types/types" "github.com/checkmarx/ast-cli/internal/commands/asca" "github.com/checkmarx/ast-cli/internal/commands/scarealtime" "github.com/checkmarx/ast-cli/internal/commands/util" @@ -119,10 +121,13 @@ const ( "\nIf you think that you have already purchased the relevant license, please contact our support team for assistance." + "\nLicensed packages: %s" containerResolutionFileName = "containers-resolution.json" - directoryCreationPrefix = "cx-" - ScsScoreCardType = "scorecard" - ScsSecretDetectionType = "secret-detection" - ScsRepoRequiredMsg = "SCS scan failed to start: Scorecard scan is missing required flags, please include in the ast-cli arguments: " + + // containerResolutionStatusFailed is the status the resolver writes into + // containers-resolution.json for an image it could not analyze. + containerResolutionStatusFailed = "Failed" + directoryCreationPrefix = "cx-" + ScsScoreCardType = "scorecard" + ScsSecretDetectionType = "secret-detection" + ScsRepoRequiredMsg = "SCS scan failed to start: Scorecard scan is missing required flags, please include in the ast-cli arguments: " + "--scs-repo-url your_repo_url --scs-repo-token your_repo_token" ScsRepoWarningMsg = "SCS scan warning: Unable to start Scorecard scan due to missing required flags, please include in the ast-cli arguments: " + "--scs-repo-url your_repo_url --scs-repo-token your_repo_token" @@ -2380,10 +2385,111 @@ func runContainerResolver(cmd *cobra.Command, directoryPath, containerImageFlag if containerResolverErr != nil { return containerResolverErr } + // Resolve returns nil even when individual images could not be analyzed, so the + // resolution file has to be inspected before the scan is allowed to continue. + return reportUnresolvedContainerImages(directoryPath) } return nil } +// reportUnresolvedContainerImages surfaces the images the resolver could not analyze. +// +// The resolver records an unresolvable image as a "Failed" entry and still returns nil, so without +// this check the CLI uploads a resolution file carrying no package data and the scan completes with +// 0 findings - byte for byte indistinguishable from a genuinely clean scan, with exit code 0 +// (AST-165915). +// +// Images named explicitly through --container-images are treated as an error: the user asked for +// those by name, so failing to scan one has to fail the pipeline. Images merely discovered inside +// the scanned sources only warn, which preserves the deliberate warn-rather-than-fail behaviour +// chosen in AST-146648 for private images the CLI cannot reach. +func reportUnresolvedContainerImages(directoryPath string) error { + resolutionFilePath := filepath.Join(directoryPath, ".checkmarx", "containers", containerResolutionFileName) + + content, err := os.ReadFile(resolutionFilePath) + if err != nil { + // Nothing was resolved, so there is nothing to report here. Any real failure of the + // resolution step itself was already returned by Resolve. + logger.PrintIfVerbose(fmt.Sprintf("Could not read container resolution file %s: %s", resolutionFilePath, err.Error())) + return nil + } + + // Decoded with the producer's own type, so the CLI cannot drift from the format the + // resolver writes. + var entries []syftExtractor.ContainerResolution + if unmarshalErr := json.Unmarshal(content, &entries); unmarshalErr != nil { + logger.PrintIfVerbose(fmt.Sprintf("Could not parse container resolution file %s: %s", resolutionFilePath, unmarshalErr.Error())) + return nil + } + + var requested, discovered []string + for i := range entries { + image := entries[i].ContainerImage + if !strings.EqualFold(image.Status, containerResolutionStatusFailed) { + continue + } + + description := fmt.Sprintf(" %s - %s", containerImageDisplayName(image.ImageName, image.ImageTag), containerImageFailureReason(image.ScanError)) + if isUserRequestedContainerImage(&entries[i]) { + requested = append(requested, description) + } else { + discovered = append(discovered, description) + } + } + + if len(discovered) > 0 { + logger.Print(fmt.Sprintf("WARNING: %s discovered in the scanned sources could not be resolved and %s NOT scanned:\n%s", + containerImageCount(len(discovered)), wasOrWere(len(discovered)), strings.Join(discovered, "\n"))) + } + + if len(requested) > 0 { + return errors.Errorf("%s could not be resolved and %s NOT scanned:\n%s", + containerImageCount(len(requested)), wasOrWere(len(requested)), strings.Join(requested, "\n")) + } + + return nil +} + +// isUserRequestedContainerImage reports whether the image was named explicitly through +// --container-images. An image can be reached from several locations at once, so a single +// UserInput origin is enough to treat it as explicitly requested. +func isUserRequestedContainerImage(entry *syftExtractor.ContainerResolution) bool { + for _, location := range entry.ContainerImage.ImageLocations { + if strings.EqualFold(location.Origin, types.UserInput) { + return true + } + } + return false +} + +func containerImageDisplayName(name, tag string) string { + if tag == "" { + return name + } + return name + ":" + tag +} + +func containerImageFailureReason(scanError string) string { + if scanError == "" { + return "the image could not be resolved" + } + return scanError +} + +func containerImageCount(count int) string { + if count == 1 { + return "1 container image" + } + return fmt.Sprintf("%d container images", count) +} + +func wasOrWere(count int) string { + if count == 1 { + return "was" + } + return "were" +} + func uploadZip(uploadsWrapper wrappers.UploadsWrapper, zipFilePath string, unzip, userProvidedZip bool, featureFlagsWrapper wrappers.FeatureFlagsWrapper) ( url, zipPath string, err error, diff --git a/internal/commands/scan_ast165915_test.go b/internal/commands/scan_ast165915_test.go new file mode 100644 index 00000000..0c152277 --- /dev/null +++ b/internal/commands/scan_ast165915_test.go @@ -0,0 +1,168 @@ +//go:build !integration + +package commands + +// Regression tests for AST-165915. +// +// The ticket reports two separate defects for the same customer scenario - scanning a locally +// built, single-architecture linux/arm64 image with +// `--container-images docker: --containers-local-resolution`: +// +// 1. container image analysis was pinned to linux/amd64, so a single-arch arm64 image never +// resolved. Fixed in containers-syft-packages-extractor v1.0.27 (no platform is forced) and +// containers-resolver v1.0.37 (Resolve calls AnalyzeImages again). +// 2. the resolution failure was swallowed: the CLI uploaded an empty resolution, the scan was +// marked Completed with 0 findings and the pipeline exited 0, indistinguishable from a +// genuinely clean scan. Fixed here, in runContainerResolver. +// +// Defect 1 lives entirely in the libraries, so what is asserted here is the CLI half: a resolved +// image must let the scan proceed, and an unresolved one must stop it. The payloads below were +// captured from real containers-resolver runs against a real Docker daemon, not hand-written. + +import ( + "os" + "path/filepath" + "testing" + + "github.com/checkmarx/ast-cli/internal/wrappers" + "github.com/spf13/cobra" + "gotest.tools/assert" +) + +// resolvedArm64Payload is what containers-resolver writes for a locally built single-architecture +// linux/arm64 image once the platform fix is in place. Captured verbatim (packages truncated) from +// a live run on an arm64 Docker host - before the fix this same image produced a Failed entry. +const resolvedArm64Payload = `[{ + "ContainerImage": { + "ImageName": "docker:ast165915-local", + "ImageTag": "arm64", + "Distribution": "alpine:3.20.10", + "ImageLocations": [{"Origin": "UserInput", "Path": "Custom Images", "FinalStage": false}], + "status": "Resolved" + }, + "ContainerPackages": [{"Name": "musl", "Version": "1.2.5-r1"}] + }]` + +// platformMismatchPayload is the customer's own failure, with the message the extractor now maps +// "mismatched platform" errors to. Before the platform fix this is what every arm64 image produced. +const platformMismatchPayload = `[{ + "ContainerImage": { + "ImageName": "docker:vrif/migration", + "ImageTag": "0.0.1-32fa28e8", + "ImageLocations": [{"Origin": "UserInput", "Path": "Custom Images", "FinalStage": false}], + "status": "Failed", + "ScanError": "The image architecture does not match the requested platform. Registry: index.docker.io" + }, + "ContainerPackages": [] + }]` + +// badTagPayload is the generic reproduction from case 00286204 on the ticket: a public image with a +// non-existent tag, no arm64 and no private registry involved. Captured verbatim from a live run. +const badTagPayload = `[{ + "ContainerImage": { + "ImageName": "debian", + "ImageTag": "non-existent-tag-999", + "Distribution": "NONE", + "ImageId": "debian:non-existent-tag-999", + "ImageLocations": [{"Origin": "UserInput", "Path": "Custom Images", "FinalStage": false}], + "status": "Failed", + "ScanError": "The requested image is not found or is unavailable. Registry: index.docker.io" + }, + "ContainerPackages": [] + }]` + +// fakeContainerResolver stands in for containers-resolver and reproduces the behaviour at the heart +// of defect 2: it writes a resolution file that may contain Failed entries and still returns nil. +type fakeContainerResolver struct { + payload string + gotImages []string + gotInvoked bool +} + +func (f *fakeContainerResolver) Resolve(scanPath, resolutionFolderPath string, images []string, isDebug bool) error { + f.gotInvoked = true + f.gotImages = images + + dir := filepath.Join(resolutionFolderPath, ".checkmarx", "containers") + if err := os.MkdirAll(dir, 0o750); err != nil { + return err + } + if err := os.WriteFile(filepath.Join(dir, containerResolutionFileName), []byte(f.payload), 0o600); err != nil { + return err + } + // The real resolver reports per-image failures only inside the file, never through this + // return value. That is precisely why the CLI has to inspect the file. + return nil +} + +// runTicketScenario drives the real runContainerResolver entry point with a resolver that produces +// the given payload, mirroring `cx scan create --container-images --containers-local-resolution`. +func runTicketScenario(t *testing.T, payload, containerImages string) (*fakeContainerResolver, error) { + t.Helper() + + fake := &fakeContainerResolver{payload: payload} + original := containerResolver + containerResolver = fake + t.Cleanup(func() { containerResolver = original }) + + cmd := &cobra.Command{} + cmd.Flags().Bool("debug", false, "") + + return fake, runContainerResolver(cmd, t.TempDir(), containerImages, true) +} + +// Defect 1, CLI half: the arm64 image the customer could not scan now resolves, so the scan must be +// allowed to continue without any error. +func TestAST165915_Defect1_LocallyBuiltArm64ImageLetsTheScanProceed(t *testing.T) { + fake, err := runTicketScenario(t, resolvedArm64Payload, "docker:ast165915-local:arm64") + + assert.NilError(t, err, "a resolved arm64 image must not block the scan") + assert.Assert(t, fake.gotInvoked, "the resolver must actually be invoked") + assert.Equal(t, len(fake.gotImages), 1) + assert.Equal(t, fake.gotImages[0], "docker:ast165915-local:arm64", "the image must reach the resolver unmangled, prefix included") +} + +// Defect 2, the ticket's headline scenario: the customer's arm64 image failing to resolve must stop +// the scan instead of completing with 0 findings and exit 0. +func TestAST165915_Defect2_PlatformMismatchStopsTheScan(t *testing.T) { + _, err := runTicketScenario(t, platformMismatchPayload, "docker:vrif/migration:0.0.1-32fa28e8") + + assert.Assert(t, err != nil, "an unresolved image must fail the scan, not complete silently") + assert.ErrorContains(t, err, "docker:vrif/migration:0.0.1-32fa28e8") + assert.ErrorContains(t, err, "NOT scanned") + assert.ErrorContains(t, err, "The image architecture does not match the requested platform") +} + +// Defect 2 is not arm64-specific. This is case 00286204's reproduction: a public image with a bad +// tag has to fail the same way, which is what the ticket asks for ("cover the general case"). +func TestAST165915_Defect2_AnyResolutionFailureStopsTheScan(t *testing.T) { + _, err := runTicketScenario(t, badTagPayload, "debian:non-existent-tag-999") + + assert.Assert(t, err != nil, "a generic resolution failure must fail the scan too") + assert.ErrorContains(t, err, "debian:non-existent-tag-999") + assert.ErrorContains(t, err, "The requested image is not found or is unavailable") +} + +// The exact regression: a Failed entry in the resolution file must never coexist with a nil error, +// because that combination is what made the scan indistinguishable from a clean one. +func TestAST165915_Defect2_FailedEntryNeverReportsSuccess(t *testing.T) { + for name, payload := range map[string]string{ + "platform mismatch": platformMismatchPayload, + "image not found": badTagPayload, + } { + t.Run(name, func(t *testing.T) { + _, err := runTicketScenario(t, payload, "some-image:tag") + assert.Assert(t, err != nil, "a Failed resolution entry must never be reported as success") + }) + } +} + +// Guards the one case that must keep the old lenient behaviour: an image discovered in the scanned +// sources rather than requested by name only warns, as AST-146648 deliberately chose. +func TestAST165915_DiscoveredImageStillOnlyWarns(t *testing.T) { + _, err := runTicketScenario(t, discoveredOnlyPayload, "") + assert.NilError(t, err, "an image only discovered in the sources must not fail the scan (AST-146648)") +} + +// Compile-time proof the fake honours the interface the CLI actually injects. +var _ wrappers.ContainerResolverWrapper = &fakeContainerResolver{} diff --git a/internal/commands/scan_container_unresolved_test.go b/internal/commands/scan_container_unresolved_test.go new file mode 100644 index 00000000..56e27940 --- /dev/null +++ b/internal/commands/scan_container_unresolved_test.go @@ -0,0 +1,120 @@ +//go:build !integration + +package commands + +import ( + "os" + "path/filepath" + "testing" + + "gotest.tools/assert" +) + +// realFailedResolution is the resolution file containers-resolver actually writes for +// `--container-images debian:non-existent-tag-999`, captured verbatim from a live run. Using the +// real payload keeps this test honest about the JSON shape (note the lower-case "status" key). +const realFailedResolution = `[ + { + "ContainerImage": { + "ImageName": "debian", + "ImageTag": "non-existent-tag-999", + "Distribution": "NONE", + "ImageHash": "", + "ImageId": "debian:non-existent-tag-999", + "ImageLocations": [ + {"Origin": "UserInput", "Path": "Custom Images", "FinalStage": false} + ], + "Layers": [], + "History": [], + "status": "Failed", + "ScanError": "The requested image is not found or is unavailable. Registry: index.docker.io" + }, + "ContainerPackages": [] + } +]` + +// discoveredOnlyPayload is a Failed entry reached only through Dockerfile discovery, never named +// explicitly by the user - the case AST-146648 deliberately chose to only warn about. +const discoveredOnlyPayload = `[{"ContainerImage":{"ImageName":"internal/app","ImageTag":"1.0", + "ImageLocations":[{"Origin":"Dockerfile","Path":"/src/Dockerfile"}], + "status":"Failed","ScanError":"The requested image is not found or is unavailable."}, + "ContainerPackages":[]}]` + +func writeResolution(t *testing.T, content string) string { + t.Helper() + dir := t.TempDir() + resolutionDir := filepath.Join(dir, ".checkmarx", "containers") + assert.NilError(t, os.MkdirAll(resolutionDir, 0o750)) + assert.NilError(t, os.WriteFile(filepath.Join(resolutionDir, containerResolutionFileName), []byte(content), 0o600)) + return dir +} + +// An image the user named with --container-images must fail the scan: the pipeline asked for it by +// name, so completing with 0 findings would be indistinguishable from a clean scan (AST-165915). +func TestReportUnresolvedContainerImages_UserRequestedImageFails(t *testing.T) { + err := reportUnresolvedContainerImages(writeResolution(t, realFailedResolution)) + + assert.Assert(t, err != nil, "a user-requested image that failed to resolve must return an error") + assert.ErrorContains(t, err, "debian:non-existent-tag-999") + assert.ErrorContains(t, err, "NOT scanned") + assert.ErrorContains(t, err, "The requested image is not found or is unavailable") +} + +// An image only discovered inside the scanned sources warns but does not fail, preserving the +// warn-rather-than-fail behaviour chosen in AST-146648 for images the CLI cannot reach. +func TestReportUnresolvedContainerImages_DiscoveredImageOnlyWarns(t *testing.T) { + assert.NilError(t, reportUnresolvedContainerImages(writeResolution(t, discoveredOnlyPayload))) +} + +// An image reachable both ways is still the user's explicit request, so it fails. +func TestReportUnresolvedContainerImages_MixedOriginCountsAsRequested(t *testing.T) { + mixed := `[{"ContainerImage":{"ImageName":"internal/app","ImageTag":"1.0", + "ImageLocations":[{"Origin":"Dockerfile","Path":"/src/Dockerfile"},{"Origin":"UserInput","Path":"Custom Images"}], + "status":"Failed","ScanError":"boom"},"ContainerPackages":[]}]` + + err := reportUnresolvedContainerImages(writeResolution(t, mixed)) + assert.Assert(t, err != nil, "an image also named by the user must fail the scan") + assert.ErrorContains(t, err, "internal/app:1.0") +} + +// The arm64 case from the ticket, once the platform fix is in: resolved images must not be reported. +func TestReportUnresolvedContainerImages_ResolvedImageIsSilent(t *testing.T) { + resolved := `[{"ContainerImage":{"ImageName":"docker:ast165915-local","ImageTag":"arm64", + "ImageLocations":[{"Origin":"UserInput","Path":"Custom Images"}],"status":"Resolved"}, + "ContainerPackages":[{"Name":"musl"}]}]` + + assert.NilError(t, reportUnresolvedContainerImages(writeResolution(t, resolved))) +} + +// A missing or unreadable resolution file must not invent a failure - Resolve already reports any +// failure of the resolution step itself through its return value. +func TestReportUnresolvedContainerImages_MissingFileIsNotAnError(t *testing.T) { + assert.NilError(t, reportUnresolvedContainerImages(t.TempDir())) +} + +func TestReportUnresolvedContainerImages_UnparsableFileIsNotAnError(t *testing.T) { + assert.NilError(t, reportUnresolvedContainerImages(writeResolution(t, "not json at all"))) +} + +// Several failures are reported together rather than one at a time. +func TestReportUnresolvedContainerImages_AggregatesMultipleFailures(t *testing.T) { + multiple := `[ + {"ContainerImage":{"ImageName":"a","ImageTag":"1","ImageLocations":[{"Origin":"UserInput"}],"status":"Failed","ScanError":"first"},"ContainerPackages":[]}, + {"ContainerImage":{"ImageName":"b","ImageTag":"2","ImageLocations":[{"Origin":"UserInput"}],"status":"Failed","ScanError":"second"},"ContainerPackages":[]}]` + + err := reportUnresolvedContainerImages(writeResolution(t, multiple)) + assert.Assert(t, err != nil) + assert.ErrorContains(t, err, "2 container images") + assert.ErrorContains(t, err, "a:1") + assert.ErrorContains(t, err, "b:2") +} + +// An image with no ScanError still has to be named, not reported as an empty reason. +func TestReportUnresolvedContainerImages_FailureWithoutScanErrorStillReported(t *testing.T) { + noReason := `[{"ContainerImage":{"ImageName":"c","ImageTag":"3","ImageLocations":[{"Origin":"UserInput"}],"status":"Failed"},"ContainerPackages":[]}]` + + err := reportUnresolvedContainerImages(writeResolution(t, noReason)) + assert.Assert(t, err != nil) + assert.ErrorContains(t, err, "c:3") + assert.ErrorContains(t, err, "the image could not be resolved") +} diff --git a/test/integration/container_images_validation_test.go b/test/integration/container_images_validation_test.go index 50df75a4..1afea76d 100644 --- a/test/integration/container_images_validation_test.go +++ b/test/integration/container_images_validation_test.go @@ -188,7 +188,8 @@ func TestContainerImageValidation_TarFiles(t *testing.T) { // Create an empty .tar file for testing. // Flag validation only checks that the .tar exists. Local resolution records it as - // Status=Failed in containers-resolution.json and continues; scan create still succeeds. + // Status=Failed in containers-resolution.json; since it was named explicitly via + // --container-images, scan create must fail (AST-165915). f, err := os.Create(emptyTarFile) assert.NilError(t, err, "Should create temp .tar file") f.Close() @@ -202,8 +203,8 @@ func TestContainerImageValidation_TarFiles(t *testing.T) { { name: "EmptyTarFile", tarFile: emptyTarFile, - shouldSucceed: true, - description: "Empty .tar file is recorded as unresolved and scan create still succeeds", + shouldSucceed: false, + description: "Empty .tar file named explicitly must fail the scan (AST-165915)", }, { name: "NonExistentTarFile", @@ -249,7 +250,8 @@ func TestContainerImageValidation_MixedTarAndRegularImages(t *testing.T) { f.Close() t.Run("EmptyTarAndRegularImage", func(t *testing.T) { - // Valid images are still resolved; the empty tar is recorded as Failed and does not abort scan create. + // nginx:alpine still resolves, but the empty tar was also named explicitly via + // --container-images, so it must fail the scan even mixed with a valid image (AST-165915). createASTIntegrationTestCommand(t) imageList := fmt.Sprintf("nginx:alpine,%s", emptyTarFile) testArgs := []string{ @@ -261,9 +263,8 @@ func TestContainerImageValidation_MixedTarAndRegularImages(t *testing.T) { flag(params.ScanTypes), params.ContainersTypeFlag, flag(params.ScanInfoFormatFlag), printer.FormatJSON, } - scanID, projectID := executeCreateScan(t, testArgs) - assert.Assert(t, scanID != "", "Scan ID should not be empty when mixing a valid image with an empty tar") - assert.Assert(t, projectID != "", "Project ID should not be empty when mixing a valid image with an empty tar") + err, _ := executeCommand(t, testArgs...) + assert.Assert(t, err != nil, "an unresolved image named explicitly must fail the scan even mixed with a valid one") }) t.Run("EmptyTarAndInvalidRegularImage", func(t *testing.T) { diff --git a/test/integration/container_scan_edge_cases_test.go b/test/integration/container_scan_edge_cases_test.go index b8d1f38d..9b3979d8 100644 --- a/test/integration/container_scan_edge_cases_test.go +++ b/test/integration/container_scan_edge_cases_test.go @@ -50,7 +50,8 @@ func TestContainerScan_TarFileValidation(t *testing.T) { tempDir := t.TempDir() t.Run("EmptyTarFile", func(t *testing.T) { - // Empty tar is not a container image; local resolution records Status=Failed and scan create still succeeds. + // Empty tar is not a container image; local resolution records Status=Failed. Since it was + // named explicitly via --container-images, scan create must fail (AST-165915). tarFile := filepath.Join(tempDir, "test-container.tar") f, err := os.Create(tarFile) assert.NilError(t, err) @@ -64,11 +65,9 @@ func TestContainerScan_TarFileValidation(t *testing.T) { flag(params.ContainerImagesFlag), tarFile, flag(params.BranchFlag), "dummy_branch", flag(params.ScanTypes), params.ContainersTypeFlag, - flag(params.ScanInfoFormatFlag), printer.FormatJSON, } - scanID, projectID := executeCreateScan(t, testArgs) - assert.Assert(t, scanID != "", "Scan ID should not be empty for empty tar file") - assert.Assert(t, projectID != "", "Project ID should not be empty for empty tar file") + scanErr, _ := executeCommand(t, testArgs...) + assert.Assert(t, scanErr != nil, "an empty tar named explicitly must fail the scan") }) t.Run("NonExistentTarFile", func(t *testing.T) { @@ -88,7 +87,8 @@ func TestContainerScan_TarFileValidation(t *testing.T) { }) t.Run("EmptyTarFileWithOtherImages", func(t *testing.T) { - // nginx:alpine is resolved; the empty tar is recorded as Failed and does not abort scan create. + // nginx:alpine still resolves, but the empty tar was also named explicitly via + // --container-images, so it must fail the scan even mixed with a valid image (AST-165915). tarFile := filepath.Join(tempDir, "another-test.tar") f, err := os.Create(tarFile) assert.NilError(t, err) @@ -102,11 +102,9 @@ func TestContainerScan_TarFileValidation(t *testing.T) { flag(params.ContainerImagesFlag), "nginx:alpine," + tarFile, flag(params.BranchFlag), "dummy_branch", flag(params.ScanTypes), params.ContainersTypeFlag, - flag(params.ScanInfoFormatFlag), printer.FormatJSON, } - scanID, projectID := executeCreateScan(t, testArgs) - assert.Assert(t, scanID != "", "Scan ID should not be empty when mixing a valid image with an empty tar") - assert.Assert(t, projectID != "", "Project ID should not be empty when mixing a valid image with an empty tar") + scanErr, _ := executeCommand(t, testArgs...) + assert.Assert(t, scanErr != nil, "an unresolved image named explicitly must fail the scan even mixed with a valid one") }) }