From 9257b9738f57837bc64f325ea91959476e66a412 Mon Sep 17 00:00:00 2001 From: Dima R <90623914+cx-dmitri-rivin@users.noreply.github.com> Date: Mon, 14 Sep 2026 14:31:43 +0300 Subject: [PATCH 1/5] AST-165915: fail the scan when a requested container image cannot be resolved containers-resolver records an image it could not analyze as a "Failed" entry in containers-resolution.json and still returns nil from Resolve. runContainerResolver only checked that return value, so the CLI compressed and uploaded a resolution file carrying no package data, the scan was marked Completed with 0 findings, and the pipeline exited 0 - byte for byte indistinguishable from a genuinely clean scan. A GitLab job with no human reading the log passed its security gate on an image that was never scanned. This is the second defect in AST-165915 and it is not specific to the arm64 platform mismatch that motivated the ticket: any resolution failure behaves the same way. Reproduced with a public image and a bad tag, no arm64 and no private registry involved: cx scan create ... --container-images debian:non-existent-tag-999 -> status Completed, findings 0, exit 0 reportUnresolvedContainerImages now reads the resolution file after a successful resolve and reports every "Failed" entry. Images named explicitly through --container-images return an error, because the pipeline asked for them by name and not scanning one has to fail. Images merely discovered inside the scanned sources only warn, preserving the warn-rather-than-fail behaviour AST-146648 deliberately chose for private images the CLI cannot reach. The resolution payload is parsed through a small local struct rather than by importing containers-syft-packages-extractor, so this adds no dependency. Note the arm64 half of the ticket is fixed in the libraries and still needs the containers-resolver bump, which is pending that repo's release. Co-Authored-By: Claude Opus 5 (1M context) --- internal/commands/scan.go | 129 +++++++++++++++++- .../scan_container_unresolved_test.go | 118 ++++++++++++++++ 2 files changed, 243 insertions(+), 4 deletions(-) create mode 100644 internal/commands/scan_container_unresolved_test.go diff --git a/internal/commands/scan.go b/internal/commands/scan.go index 41b24008..210b81c8 100644 --- a/internal/commands/scan.go +++ b/internal/commands/scan.go @@ -119,10 +119,16 @@ 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" + // containerImageOriginUserInput marks an image the user named explicitly through + // --container-images, as opposed to one discovered inside the scanned sources. + containerImageOriginUserInput = "UserInput" + 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 +2386,125 @@ 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 } +// containerResolutionEntry mirrors just enough of +// .checkmarx/containers/containers-resolution.json to tell which images the resolver failed on. +// It is declared here rather than imported from containers-syft-packages-extractor so that the +// CLI takes on no additional dependency for this check. +type containerResolutionEntry struct { + ContainerImage struct { + ImageName string `json:"ImageName"` + ImageTag string `json:"ImageTag"` + Status string `json:"status"` + ScanError string `json:"ScanError"` + ImageLocations []struct { + Origin string `json:"Origin"` + } `json:"ImageLocations"` + } `json:"ContainerImage"` +} + +// 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 + } + + var entries []containerResolutionEntry + 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 containerResolutionEntry) bool { + for _, location := range entry.ContainerImage.ImageLocations { + if strings.EqualFold(location.Origin, containerImageOriginUserInput) { + 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_container_unresolved_test.go b/internal/commands/scan_container_unresolved_test.go new file mode 100644 index 00000000..e5a168ba --- /dev/null +++ b/internal/commands/scan_container_unresolved_test.go @@ -0,0 +1,118 @@ +//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": [] + } +]` + +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) { + discovered := `[{"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":[]}]` + + assert.NilError(t, reportUnresolvedContainerImages(writeResolution(t, discovered))) +} + +// 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") +} From a8a95be82911409ef9a607d96dbcd52260c84d92 Mon Sep 17 00:00:00 2001 From: Dima R <90623914+cx-dmitri-rivin@users.noreply.github.com> Date: Tue, 15 Sep 2026 08:30:45 +0300 Subject: [PATCH 2/5] AST-165915: reuse containers-types for image locations instead of redeclaring them The unresolved-image check declared its own "UserInput" constant and an inline struct for the image locations. Both already exist in containers-types, which is already a direct dependency of the CLI and is on the depguard allowlist in .golangci.yml, so redeclaring them was duplication with a real risk of drifting from the producer. Only the ContainerResolution/ContainerImage envelope is still restated locally. That type lives in containers-syft-packages-extractor, which the CLI depends on only indirectly and which is not on the allowlist - importing it would make the CLI a direct consumer of that module. The same split exists upstream, where the extractor declares its own ImageLocation rather than using the one in containers-types; moving the resolution payload into containers-types would remove both copies, but that is a cross-repo change beyond this fix. go.mod and go.sum are unchanged: nothing new was added, the CLI just stopped re-declaring what it already had. Co-Authored-By: Claude Opus 5 (1M context) --- internal/commands/scan.go | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/internal/commands/scan.go b/internal/commands/scan.go index 210b81c8..2913c030 100644 --- a/internal/commands/scan.go +++ b/internal/commands/scan.go @@ -22,6 +22,7 @@ import ( "time" "unicode" + "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" @@ -122,13 +123,10 @@ const ( // containerResolutionStatusFailed is the status the resolver writes into // containers-resolution.json for an image it could not analyze. containerResolutionStatusFailed = "Failed" - // containerImageOriginUserInput marks an image the user named explicitly through - // --container-images, as opposed to one discovered inside the scanned sources. - containerImageOriginUserInput = "UserInput" - 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: " + + 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" @@ -2395,18 +2393,19 @@ func runContainerResolver(cmd *cobra.Command, directoryPath, containerImageFlag // containerResolutionEntry mirrors just enough of // .checkmarx/containers/containers-resolution.json to tell which images the resolver failed on. -// It is declared here rather than imported from containers-syft-packages-extractor so that the -// CLI takes on no additional dependency for this check. +// +// The envelope (ContainerResolution/ContainerImage) lives in containers-syft-packages-extractor, +// which the CLI only depends on indirectly and which is not on the depguard allowlist, so it is +// restated here. The image locations, however, are the shared contract in containers-types and are +// reused from there rather than redeclared. type containerResolutionEntry struct { ContainerImage struct { - ImageName string `json:"ImageName"` - ImageTag string `json:"ImageTag"` + ImageName string + ImageTag string Status string `json:"status"` - ScanError string `json:"ScanError"` - ImageLocations []struct { - Origin string `json:"Origin"` - } `json:"ImageLocations"` - } `json:"ContainerImage"` + ScanError string + ImageLocations []types.ImageLocation + } } // reportUnresolvedContainerImages surfaces the images the resolver could not analyze. @@ -2470,7 +2469,7 @@ func reportUnresolvedContainerImages(directoryPath string) error { // UserInput origin is enough to treat it as explicitly requested. func isUserRequestedContainerImage(entry containerResolutionEntry) bool { for _, location := range entry.ContainerImage.ImageLocations { - if strings.EqualFold(location.Origin, containerImageOriginUserInput) { + if strings.EqualFold(location.Origin, types.UserInput) { return true } } From 1ceb26bcb2f893eb1eb9b40eb224c32d33b5dbc2 Mon Sep 17 00:00:00 2001 From: Dima R <90623914+cx-dmitri-rivin@users.noreply.github.com> Date: Tue, 15 Sep 2026 08:35:33 +0300 Subject: [PATCH 3/5] AST-165915: decode the resolution file with the producer's own type The unresolved-image check declared a local struct mirroring containers-resolution.json. That was a copy of a format this repo does not own, and it had already drifted once during development: the "status" key is lower-case while every neighbouring key is not, which a hand-written mirror only gets right by luck. ContainerResolution/ContainerImage now come from containers-syft-packages-extractor - the package that writes the file - so the CLI cannot disagree with the producer about its own format. The extractor exports no origin constant, so types.UserInput from containers-types is still what classifies an image as explicitly requested. This is not a new dependency. The module is already in the graph and already linked into the binary through containers-resolver; the only change is that go.mod stops marking it indirect. go.sum is untouched and the binary is unaffected. Added to the depguard allowlist next to containers-images-extractor, which is imported the same way. Co-Authored-By: Claude Opus 5 (1M context) --- .golangci.yml | 249 +++++++++++++++++++------------------- go.mod | 2 +- internal/commands/scan.go | 24 +--- 3 files changed, 131 insertions(+), 144 deletions(-) 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..6cab2a32 100644 --- a/go.mod +++ b/go.mod @@ -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.25 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 diff --git a/internal/commands/scan.go b/internal/commands/scan.go index 2913c030..7c63d3fd 100644 --- a/internal/commands/scan.go +++ b/internal/commands/scan.go @@ -22,6 +22,7 @@ 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" @@ -2391,23 +2392,6 @@ func runContainerResolver(cmd *cobra.Command, directoryPath, containerImageFlag return nil } -// containerResolutionEntry mirrors just enough of -// .checkmarx/containers/containers-resolution.json to tell which images the resolver failed on. -// -// The envelope (ContainerResolution/ContainerImage) lives in containers-syft-packages-extractor, -// which the CLI only depends on indirectly and which is not on the depguard allowlist, so it is -// restated here. The image locations, however, are the shared contract in containers-types and are -// reused from there rather than redeclared. -type containerResolutionEntry struct { - ContainerImage struct { - ImageName string - ImageTag string - Status string `json:"status"` - ScanError string - ImageLocations []types.ImageLocation - } -} - // 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 @@ -2430,7 +2414,9 @@ func reportUnresolvedContainerImages(directoryPath string) error { return nil } - var entries []containerResolutionEntry + // 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 @@ -2467,7 +2453,7 @@ func reportUnresolvedContainerImages(directoryPath string) error { // 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 containerResolutionEntry) bool { +func isUserRequestedContainerImage(entry syftExtractor.ContainerResolution) bool { for _, location := range entry.ContainerImage.ImageLocations { if strings.EqualFold(location.Origin, types.UserInput) { return true From d5d3d1c2f7a457d8017bec89e0afa62c0e53e9a0 Mon Sep 17 00:00:00 2001 From: Dima R <90623914+cx-dmitri-rivin@users.noreply.github.com> Date: Tue, 15 Sep 2026 11:16:04 +0300 Subject: [PATCH 4/5] AST-165915: bump containers-resolver to v1.0.37 and add ticket regression tests The bump activates the platform half of the ticket: containers-resolver v1.0.37 calls AnalyzeImages instead of AnalyzeImagesWithPlatform("linux/amd64"), and containers-syft-packages-extractor v1.0.27 no longer defaults an empty platform specifier to linux/amd64. A locally built single-architecture linux/arm64 image now resolves instead of failing, verified against the released tag on an arm64 Docker host: status "Resolved", 14 packages. No new modules enter the graph. syft stays at v1.21.0 and stereoscope at v0.1.0, so none of the aws-sdk-go-v2 or cloud.google.com/go trees that reviewers objected to come back. Stripped binary size is unchanged: 80,447,458 -> 80,447,810 bytes, a difference of 352 bytes. zerolog moves v1.34.0 -> v1.35.1 because v1.0.37 requires it; that is the only transitive change. scan_ast165915_test.go drives the real runContainerResolver entry point rather than the helper, so it covers the path the ticket describes end to end. The resolution payloads are captured verbatim from live containers-resolver runs against a real Docker daemon, including the lower-case "status" key that a hand-written fixture gets right only by luck. Coverage: - a resolved arm64 image lets the scan proceed, image reaching the resolver with its docker: prefix intact - the customer's platform mismatch stops the scan instead of completing with 0 findings and exit 0 - case 00286204's generic reproduction, a public image with a bad tag, fails the same way - the fix is not arm64-specific - a Failed entry never coexists with a nil error, which was the actual defect - an image only discovered in the scanned sources still merely warns, per AST-146648 Confirmed these fail without the fix: disabling the check makes all three defect-2 tests fail, and re-enabling it makes them pass. Co-Authored-By: Claude Opus 5 (1M context) --- go.mod | 6 +- go.sum | 19 +-- internal/commands/scan_ast165915_test.go | 173 +++++++++++++++++++++++ 3 files changed, 182 insertions(+), 16 deletions(-) create mode 100644 internal/commands/scan_ast165915_test.go diff --git a/go.mod b/go.mod index 6cab2a32..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 + 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_ast165915_test.go b/internal/commands/scan_ast165915_test.go new file mode 100644 index 00000000..b4d442b3 --- /dev/null +++ b/internal/commands/scan_ast165915_test.go @@ -0,0 +1,173 @@ +//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) { + discovered := `[{"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":[]}]` + + _, err := runTicketScenario(t, discovered, "") + 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{} From f13844d031000cbed08f2ebe4e449b1bb4b84cdd Mon Sep 17 00:00:00 2001 From: Dima R <90623914+cx-dmitri-rivin@users.noreply.github.com> Date: Wed, 16 Sep 2026 09:35:16 +0300 Subject: [PATCH 5/5] AST-165915: fix lint findings and align container tests with new fail-fast behavior golangci-lint flagged a duplicated JSON literal (goconst) between the new regression test and an existing one, and a value-passed 224-byte struct (gocritic hugeParam) in isUserRequestedContainerImage. Also update the integration tests that asserted the old swallowed-failure behavior for an explicitly-named, unresolvable container image - they now expect the scan to fail, matching the fix in this PR. Co-Authored-By: Claude Sonnet 5 --- internal/commands/scan.go | 4 ++-- internal/commands/scan_ast165915_test.go | 7 +------ .../commands/scan_container_unresolved_test.go | 14 ++++++++------ .../container_images_validation_test.go | 15 ++++++++------- .../container_scan_edge_cases_test.go | 18 ++++++++---------- 5 files changed, 27 insertions(+), 31 deletions(-) diff --git a/internal/commands/scan.go b/internal/commands/scan.go index 7c63d3fd..c109566c 100644 --- a/internal/commands/scan.go +++ b/internal/commands/scan.go @@ -2430,7 +2430,7 @@ func reportUnresolvedContainerImages(directoryPath string) error { } description := fmt.Sprintf(" %s - %s", containerImageDisplayName(image.ImageName, image.ImageTag), containerImageFailureReason(image.ScanError)) - if isUserRequestedContainerImage(entries[i]) { + if isUserRequestedContainerImage(&entries[i]) { requested = append(requested, description) } else { discovered = append(discovered, description) @@ -2453,7 +2453,7 @@ func reportUnresolvedContainerImages(directoryPath string) error { // 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 { +func isUserRequestedContainerImage(entry *syftExtractor.ContainerResolution) bool { for _, location := range entry.ContainerImage.ImageLocations { if strings.EqualFold(location.Origin, types.UserInput) { return true diff --git a/internal/commands/scan_ast165915_test.go b/internal/commands/scan_ast165915_test.go index b4d442b3..0c152277 100644 --- a/internal/commands/scan_ast165915_test.go +++ b/internal/commands/scan_ast165915_test.go @@ -160,12 +160,7 @@ func TestAST165915_Defect2_FailedEntryNeverReportsSuccess(t *testing.T) { // 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) { - discovered := `[{"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":[]}]` - - _, err := runTicketScenario(t, discovered, "") + _, err := runTicketScenario(t, discoveredOnlyPayload, "") assert.NilError(t, err, "an image only discovered in the sources must not fail the scan (AST-146648)") } diff --git a/internal/commands/scan_container_unresolved_test.go b/internal/commands/scan_container_unresolved_test.go index e5a168ba..56e27940 100644 --- a/internal/commands/scan_container_unresolved_test.go +++ b/internal/commands/scan_container_unresolved_test.go @@ -33,6 +33,13 @@ const realFailedResolution = `[ } ]` +// 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() @@ -56,12 +63,7 @@ func TestReportUnresolvedContainerImages_UserRequestedImageFails(t *testing.T) { // 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) { - discovered := `[{"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":[]}]` - - assert.NilError(t, reportUnresolvedContainerImages(writeResolution(t, discovered))) + assert.NilError(t, reportUnresolvedContainerImages(writeResolution(t, discoveredOnlyPayload))) } // An image reachable both ways is still the user's explicit request, so it fails. 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") }) }