From 90f39043914688faf14aa2b3ead22de8bc85b955 Mon Sep 17 00:00:00 2001 From: Harshjeet Patil Date: Tue, 29 Sep 2026 12:44:43 +0530 Subject: [PATCH] changes added for the bug --- internal/services/sast-metadata.go | 24 ++++++++++++-------- internal/services/sast-metadata_test.go | 22 ++++++++++++++++++ internal/wrappers/mock/sast-metadata-mock.go | 11 +++++++-- internal/wrappers/sast-metadata-http.go | 9 +++----- 4 files changed, 48 insertions(+), 18 deletions(-) diff --git a/internal/services/sast-metadata.go b/internal/services/sast-metadata.go index 0005790a0..35e2be70d 100644 --- a/internal/services/sast-metadata.go +++ b/internal/services/sast-metadata.go @@ -2,6 +2,7 @@ package services import ( "context" + "errors" "strings" "sync" @@ -11,7 +12,8 @@ import ( ) const ( - BatchSize = 200 + // BatchSize must not exceed the sast-metadata backend's per-request scan-ids cap (currently 50). + BatchSize = 50 ) func GetSastMetadataByIDs(sastMetaDataWrapper wrappers.SastMetadataWrapper, scanIDs []string) (*wrappers.SastMetadataModel, error) { @@ -21,7 +23,7 @@ func GetSastMetadataByIDs(sastMetaDataWrapper wrappers.SastMetadataWrapper, scan var wg sync.WaitGroup results := make(chan wrappers.SastMetadataModel, totalBatches) - errors := make(chan error, totalBatches) + errCh := make(chan error, totalBatches) ctx := context.Background() for i := 0; i < totalBatches; i++ { @@ -46,21 +48,23 @@ func GetSastMetadataByIDs(sastMetaDataWrapper wrappers.SastMetadataWrapper, scan result, err := sastMetaDataWrapper.GetSastMetadataByIDs(batchParams) if err != nil { - errors <- err + errCh <- err return } results <- *result }() } - go func() { - wg.Wait() - close(results) - close(errors) - }() + wg.Wait() + close(results) + close(errCh) - if len(errors) > 0 { - return nil, <-errors + if len(errCh) > 0 { + var batchErrs []error + for batchErr := range errCh { + batchErrs = append(batchErrs, batchErr) + } + return nil, errors.Join(batchErrs...) } var models []wrappers.SastMetadataModel diff --git a/internal/services/sast-metadata_test.go b/internal/services/sast-metadata_test.go index ac96bfb82..34b18aa3f 100644 --- a/internal/services/sast-metadata_test.go +++ b/internal/services/sast-metadata_test.go @@ -48,6 +48,28 @@ func TestGetSastMetadataByIDs(t *testing.T) { } } +func TestGetSastMetadataByIDs_BatchFailureIsSurfacedNotPanicked(t *testing.T) { + // A failing batch must be reliably surfaced as an error, not raced past or dereferenced as nil. + scanIDs := append(createScanIDs(BatchSize), createErrorScanIDs(BatchSize)...) + scanIDs = append(scanIDs, createScanIDs(BatchSize)...) + + wrapper := &mock.SastMetadataMockWrapper{} + actual, err := GetSastMetadataByIDs(wrapper, scanIDs) + + assert.Nil(t, actual) + if assert.Error(t, err) { + assert.Contains(t, err.Error(), "Too many ids in url") + } +} + +func createErrorScanIDs(count int) []string { + scanIDs := make([]string, count) + for i := 0; i < count; i++ { + scanIDs[i] = fmt.Sprintf("TriggerError%d", i) + } + return scanIDs +} + func createScanIDs(count int) []string { scanIDs := make([]string, count) for i := 0; i < count; i++ { diff --git a/internal/wrappers/mock/sast-metadata-mock.go b/internal/wrappers/mock/sast-metadata-mock.go index 5ea41dc78..8b70c9095 100644 --- a/internal/wrappers/mock/sast-metadata-mock.go +++ b/internal/wrappers/mock/sast-metadata-mock.go @@ -1,6 +1,7 @@ package mock import ( + "fmt" "strings" "github.com/checkmarx/ast-cli/internal/wrappers" @@ -9,14 +10,20 @@ import ( type SastMetadataMockWrapper struct{} const ( - scanIDParam = "scan-ids" - concurrent = "ConcurrentTest" + scanIDParam = "scan-ids" + concurrent = "ConcurrentTest" + errorTrigger = "TriggerError" + tooManyIDsErrorCode = 3102 + tooManyIDsMessage = "Too many ids in url" ) func (s SastMetadataMockWrapper) GetSastMetadataByIDs(params map[string]string) ( *wrappers.SastMetadataModel, error, ) { + if strings.Contains(params[scanIDParam], errorTrigger) { + return nil, fmt.Errorf("%d - %s", tooManyIDsErrorCode, tooManyIDsMessage) + } if strings.Contains(params[scanIDParam], concurrent) { scanIDs := strings.Split(params[scanIDParam], ",") return &wrappers.SastMetadataModel{ diff --git a/internal/wrappers/sast-metadata-http.go b/internal/wrappers/sast-metadata-http.go index 7fa5139f5..23204d511 100644 --- a/internal/wrappers/sast-metadata-http.go +++ b/internal/wrappers/sast-metadata-http.go @@ -27,13 +27,10 @@ func (s *SastIncrementalHTTPWrapper) GetSastMetadataByIDs(params map[string]stri if err != nil { return nil, err } - decoder := json.NewDecoder(resp.Body) - defer func() { - if err == nil { - _ = resp.Body.Close() - } + _ = resp.Body.Close() }() + decoder := json.NewDecoder(resp.Body) switch resp.StatusCode { case http.StatusBadRequest, http.StatusInternalServerError: @@ -42,7 +39,7 @@ func (s *SastIncrementalHTTPWrapper) GetSastMetadataByIDs(params map[string]stri if err != nil { return nil, fmt.Errorf("%v %s", err, failedToParseGetAll) } - return nil, err + return nil, fmt.Errorf("%d - %s", errorModel.Code, errorModel.Message) case http.StatusOK: model := SastMetadataModel{} err = decoder.Decode(&model)