From f870a9dc826a5a17c8c61fefe087e6384a7d0104 Mon Sep 17 00:00:00 2001 From: Andrew Robinson Hodges Date: Mon, 4 Aug 2025 16:17:22 +0100 Subject: [PATCH 01/12] refactor: move UploadAndAnalyze logic to code-client-go --- internal/analysis/analysis.go | 209 ++++++++++++++++++++++++++++++++++ internal/deepcode/client.go | 3 +- scan.go | 47 +++++++- 3 files changed, 254 insertions(+), 5 deletions(-) diff --git a/internal/analysis/analysis.go b/internal/analysis/analysis.go index 8cb4cc4..d00d514 100644 --- a/internal/analysis/analysis.go +++ b/internal/analysis/analysis.go @@ -18,12 +18,15 @@ package analysis import ( + "bytes" "context" _ "embed" "encoding/json" "fmt" "io" + "math" "net/http" + "net/url" "strings" "time" @@ -46,6 +49,7 @@ import ( type AnalysisOrchestrator interface { RunTest(ctx context.Context, orgId string, b bundle.Bundle, target scan.Target, reportingOptions AnalysisConfig) (*sarif.SarifResponse, *scan.ResultMetaData, error) RunTestRemote(ctx context.Context, orgId string, reportingOptions AnalysisConfig) (*sarif.SarifResponse, *scan.ResultMetaData, error) + RunLegacyTest(ctx context.Context, bundleHash string, shardKey string, limitToFiles []string, severity int) (*sarif.SarifResponse, LegacyAnalysisStatus, error) } type AnalysisConfig struct { @@ -56,6 +60,51 @@ type AnalysisConfig struct { ProjectId *uuid.UUID CommitId *string } + +// Legacy analysis types and constants +const ( + legacyCompleteStatus = "COMPLETE" +) + +type LegacyAnalysisStatus struct { + Message string + Percentage int +} + +type LegacyAnalysisRequestKey struct { + Type string `json:"type"` + Hash string `json:"hash"` + LimitToFiles []string `json:"limitToFiles,omitempty"` + Shard string `json:"shard"` +} + +type legacyCodeRequestContextOrg struct { + Name string `json:"name"` + DisplayName string `json:"displayName"` + PublicId string `json:"publicId"` + Flags map[string]bool `json:"flags"` +} + +type legacyCodeRequestContext struct { + Initiator string `json:"initiator"` + Flow string `json:"flow,omitempty"` + Org legacyCodeRequestContextOrg `json:"org,omitempty"` +} + +type LegacyAnalysisRequest struct { + Key LegacyAnalysisRequestKey `json:"key"` + Severity int `json:"severity,omitempty"` + Prioritized bool `json:"prioritized,omitempty"` + Legacy bool `json:"legacy"` + AnalysisContext legacyCodeRequestContext `json:"analysisContext"` +} + +type LegacyAnalysisFailedError struct { + Msg string +} + +func (e LegacyAnalysisFailedError) Error() string { return e.Msg } + type analysisOrchestrator struct { httpClient codeClientHTTP.HTTPClient instrumentor observability.Instrumentor @@ -352,3 +401,163 @@ func (a *analysisOrchestrator) retrieveTestURL(ctx context.Context, client *test return nil, false, fmt.Errorf("unexpected response status \"%d\"", parsedResponse.StatusCode()) } } + +// Legacy analysis helper functions +func (a *analysisOrchestrator) newLegacyCodeRequestContext() legacyCodeRequestContext { + unknown := "unknown" + orgId := unknown + if a.config.Organization() != "" { + orgId = a.config.Organization() + } + + return legacyCodeRequestContext{ + Initiator: "IDE", + Flow: "language-server", + Org: legacyCodeRequestContextOrg{ + Name: unknown, + DisplayName: unknown, + PublicId: orgId, + }, + } +} + +func (a *analysisOrchestrator) createLegacyAnalysisRequestBody(bundleHash, shardKey string, limitToFiles []string, severity int) ([]byte, error) { + request := LegacyAnalysisRequest{ + Key: LegacyAnalysisRequestKey{ + Type: "file", + Hash: bundleHash, + LimitToFiles: limitToFiles, + }, + Legacy: false, + AnalysisContext: a.newLegacyCodeRequestContext(), + } + if len(shardKey) > 0 { + request.Key.Shard = shardKey + } + if severity > 0 { + request.Severity = severity + } + + requestBody, err := json.Marshal(request) + return requestBody, err +} + +func (a *analysisOrchestrator) getLegacyCodeApiUrl() (string, error) { + // Use the same logic as the original SnykCodeHTTPClient + if !a.config.IsFedramp() { + return a.config.SnykCodeApi(), nil + } + u, err := url.Parse(a.config.SnykCodeApi()) + if err != nil { + return "", err + } + + // Apply fedramp transformation (this might need adjustment based on the actual requirements) + u.Host = strings.Replace(u.Host, "deeproxy", "api", 1) + + if a.config.Organization() == "" { + return "", errors.New("Organization is required in a fedramp environment") + } + + u.Path = "/hidden/orgs/" + a.config.Organization() + "/code" + return u.String(), nil +} + +func (a *analysisOrchestrator) logLegacySarifResponse(method string, sarifResponse sarif.SarifResponse) { + a.logger.Debug(). + Str("method", method). + Str("status", sarifResponse.Status). + Float64("progress", sarifResponse.Progress). + Int("fetchingCodeTime", sarifResponse.Timing.FetchingCode). + Int("analysisTime", sarifResponse.Timing.Analysis). + Int("filesAnalyzed", len(sarifResponse.Coverage)). + Msg("Received response summary") +} + +func (a *analysisOrchestrator) RunLegacyTest(ctx context.Context, bundleHash string, shardKey string, limitToFiles []string, severity int) (*sarif.SarifResponse, LegacyAnalysisStatus, error) { + method := "analysis.RunLegacyTest" + span := a.instrumentor.StartSpan(ctx, method) + defer a.instrumentor.Finish(span) + + a.logger.Debug().Str("method", method).Str("bundleHash", bundleHash).Msg("API: Retrieving legacy analysis for bundle") + defer a.logger.Debug().Str("method", method).Str("bundleHash", bundleHash).Msg("API: Retrieving legacy analysis done") + + requestBody, err := a.createLegacyAnalysisRequestBody(bundleHash, shardKey, limitToFiles, severity) + if err != nil { + a.logger.Err(err).Str("method", method).Str("requestBody", string(requestBody)).Msg("error creating request body") + return nil, LegacyAnalysisStatus{}, err + } + + // Get the legacy code API URL + baseUrl, err := a.getLegacyCodeApiUrl() + if err != nil { + return nil, LegacyAnalysisStatus{}, err + } + + // Create HTTP request + analysisUrl := baseUrl + "/analysis" + req, err := http.NewRequestWithContext(span.Context(), "POST", analysisUrl, bytes.NewBuffer(requestBody)) + if err != nil { + a.logger.Err(err).Str("method", method).Msg("error creating HTTP request") + return nil, LegacyAnalysisStatus{}, err + } + + // Set headers + //req.Header.Set("Content-Type", "application/json") + //req.Header.Set("Authorization", "token "+a.config.Token()) + + // Make HTTP call + resp, err := a.httpClient.Do(req) + failed := LegacyAnalysisStatus{Message: "FAILED"} + if err != nil { + a.logger.Err(err).Str("method", method).Msg("error response from analysis") + return nil, failed, err + } + defer func() { + closeErr := resp.Body.Close() + if closeErr != nil { + a.logger.Err(closeErr).Msg("failed to close response body") + } + }() + + responseBody, err := io.ReadAll(resp.Body) + if err != nil { + a.logger.Err(err).Str("method", method).Msg("error reading response body") + return nil, failed, err + } + + // Check response status + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + a.logger.Err(err).Str("method", method).Str("responseBody", string(responseBody)).Int("statusCode", resp.StatusCode).Msg("error response from analysis") + return nil, failed, LegacyAnalysisFailedError{Msg: string(responseBody)} + } + + var response sarif.SarifResponse + err = json.Unmarshal(responseBody, &response) + if err != nil { + a.logger.Err(err).Str("method", method).Str("responseBody", string(responseBody)).Msg("error unmarshalling") + return nil, failed, err + } else { + a.logLegacySarifResponse(method, response) + } + + a.logger.Debug().Str("method", method).Str("bundleHash", bundleHash).Float64("progress", + response.Progress).Msgf("Status: %s", response.Status) + + if response.Status == failed.Message { + a.logger.Err(err).Str("method", method).Str("responseStatus", response.Status).Msg("analysis failed") + return nil, failed, LegacyAnalysisFailedError{Msg: string(responseBody)} + } + + if response.Status == "" { + a.logger.Err(err).Str("method", method).Str("responseStatus", response.Status).Msg("unknown response status (empty)") + return nil, failed, LegacyAnalysisFailedError{Msg: string(responseBody)} + } + + status := LegacyAnalysisStatus{Message: response.Status, Percentage: int(math.RoundToEven(response.Progress * 100))} + if response.Status != legacyCompleteStatus { + return nil, status, nil + } + + return &response, status, nil +} diff --git a/internal/deepcode/client.go b/internal/deepcode/client.go index 2a63564..f5d4982 100644 --- a/internal/deepcode/client.go +++ b/internal/deepcode/client.go @@ -28,11 +28,10 @@ import ( "regexp" "strconv" + "github.com/rs/zerolog" "github.com/snyk/code-client-go/config" "github.com/snyk/code-client-go/internal/util/encoding" - "github.com/rs/zerolog" - codeClientHTTP "github.com/snyk/code-client-go/http" "github.com/snyk/code-client-go/observability" ) diff --git a/scan.go b/scan.go index 50d5e98..919aee5 100644 --- a/scan.go +++ b/scan.go @@ -19,7 +19,6 @@ package codeclient import ( "context" - "github.com/google/uuid" "github.com/pkg/errors" "github.com/rs/zerolog" @@ -38,6 +37,7 @@ import ( type codeScanner struct { httpClient codeClientHTTP.HTTPClient bundleManager bundle.BundleManager + deepcodeClient deepcode.DeepcodeClient analysisOrchestrator analysis.AnalysisOrchestrator instrumentor observability.Instrumentor errorReporter observability.ErrorReporter @@ -63,6 +63,15 @@ type CodeScanner interface { files <-chan string, changedFiles map[string]bool, ) (*sarif.SarifResponse, string, error) + + UploadAndAnalyzeLegacy( + ctx context.Context, + requestId string, + target scan.Target, + shardKey string, + files <-chan string, + changedFiles map[string]bool, + ) (*sarif.SarifResponse, string, error) } var _ CodeScanner = (*codeScanner)(nil) @@ -106,6 +115,13 @@ func WithTrackerFactory(trackerFactory scan.TrackerFactory) OptionFunc { type AnalysisOption func(*analysis.AnalysisConfig) +type LegacyAnalysisOption struct { + bundleHash string + shardKey string + limitToFiles []string + severity int +} + func ReportLocalTest(projectName string, targetName string, targetReference string) AnalysisOption { return func(c *analysis.AnalysisConfig) { c.Report = true @@ -152,6 +168,7 @@ func NewCodeScanner( deepcodeClient := deepcode.NewDeepcodeClient(scanner.config, httpClient, scanner.logger, scanner.instrumentor, scanner.errorReporter) bundleManager := bundle.NewBundleManager(deepcodeClient, scanner.logger, scanner.instrumentor, scanner.errorReporter, scanner.trackerFactory) scanner.bundleManager = bundleManager + scanner.deepcodeClient = deepcodeClient analysisOrchestrator := analysis.NewAnalysisOrchestrator( scanner.config, httpClient, @@ -171,6 +188,7 @@ func NewCodeScanner( func (c *codeScanner) WithBundleManager(bundleManager bundle.BundleManager) *codeScanner { return &codeScanner{ bundleManager: bundleManager, + deepcodeClient: c.deepcodeClient, analysisOrchestrator: c.analysisOrchestrator, errorReporter: c.errorReporter, logger: c.logger, @@ -183,6 +201,7 @@ func (c *codeScanner) WithBundleManager(bundleManager bundle.BundleManager) *cod func (c *codeScanner) WithAnalysisOrchestrator(analysisOrchestrator analysis.AnalysisOrchestrator) *codeScanner { return &codeScanner{ bundleManager: c.bundleManager, + deepcodeClient: c.deepcodeClient, analysisOrchestrator: analysisOrchestrator, errorReporter: c.errorReporter, logger: c.logger, @@ -235,7 +254,6 @@ func (c *codeScanner) checkCancellationOrLogError(ctx context.Context, targetPat return returnError } -// UploadAndAnalyze returns a fake SARIF response for testing. Use target-service to run analysis on. func (c *codeScanner) UploadAndAnalyze( ctx context.Context, requestId string, @@ -247,7 +265,30 @@ func (c *codeScanner) UploadAndAnalyze( return response, bundleHash, err } -// UploadAndAnalyzeWithOptions returns a fake SARIF response for testing. Use target-service to run analysis on. +func (c *codeScanner) UploadAndAnalyzeLegacy( + ctx context.Context, + requestId string, + target scan.Target, + shardKey string, + files <-chan string, + changedFiles map[string]bool, +) (*sarif.SarifResponse, string, error) { + uploadedBundle, err := c.Upload(ctx, requestId, target, files, changedFiles) + if err != nil || uploadedBundle == nil || uploadedBundle.GetBundleHash() == "" { + c.logger.Debug().Msg("empty bundle, no Snyk Code analysis") + return nil, "", err + } + + bundleHash := uploadedBundle.GetBundleHash() + limitToFiles := uploadedBundle.GetLimitToFiles() + severity := 0 + + response, _, err := c.analysisOrchestrator.RunLegacyTest(ctx, bundleHash, shardKey, limitToFiles, severity) + + //TODO status? + return response, bundleHash, err +} + func (c *codeScanner) UploadAndAnalyzeWithOptions( ctx context.Context, requestId string, From c0b8082e24ed3801af2ec2e2c55e817a8273b10d Mon Sep 17 00:00:00 2001 From: Andrew Robinson Hodges Date: Tue, 5 Aug 2025 16:51:22 +0100 Subject: [PATCH 02/12] refactor: move UploadAndAnalyze logic to code-client-go --- http/http.go | 14 +++++++++++++- internal/analysis/analysis.go | 8 +++----- internal/analysis/mocks/analysis.go | 5 +++++ llm/api_client.go | 16 +++------------- llm/api_client_test.go | 4 ++-- 5 files changed, 26 insertions(+), 21 deletions(-) diff --git a/http/http.go b/http/http.go index cf08ef0..7d627bf 100644 --- a/http/http.go +++ b/http/http.go @@ -24,7 +24,7 @@ import ( "time" "github.com/rs/zerolog" - + "github.com/snyk/code-client-go/observability" ) @@ -158,3 +158,15 @@ func NewDefaultClientFactory() HTTPClientFactory { clientFunc := func() *http.Client { return http.DefaultClient } return clientFunc } + +func AddDefaultHeaders(req *http.Request, requestId string, orgId string) { + // if requestId is empty it will be enriched from the Gateway + if len(requestId) > 0 { + req.Header.Set("snyk-request-id", requestId) + } + if len(orgId) > 0 { + req.Header.Set("snyk-org-name", orgId) + } + req.Header.Set("Cache-Control", "private, max-age=0, no-cache") + req.Header.Set("Content-Type", "application/json") +} diff --git a/internal/analysis/analysis.go b/internal/analysis/analysis.go index d00d514..f2fc630 100644 --- a/internal/analysis/analysis.go +++ b/internal/analysis/analysis.go @@ -410,6 +410,7 @@ func (a *analysisOrchestrator) newLegacyCodeRequestContext() legacyCodeRequestCo orgId = a.config.Organization() } + // TODO - needs to be dynamically generated return legacyCodeRequestContext{ Initiator: "IDE", Flow: "language-server", @@ -496,15 +497,12 @@ func (a *analysisOrchestrator) RunLegacyTest(ctx context.Context, bundleHash str // Create HTTP request analysisUrl := baseUrl + "/analysis" - req, err := http.NewRequestWithContext(span.Context(), "POST", analysisUrl, bytes.NewBuffer(requestBody)) + req, err := http.NewRequestWithContext(span.Context(), http.MethodPost, analysisUrl, bytes.NewBuffer(requestBody)) if err != nil { a.logger.Err(err).Str("method", method).Msg("error creating HTTP request") return nil, LegacyAnalysisStatus{}, err } - - // Set headers - //req.Header.Set("Content-Type", "application/json") - //req.Header.Set("Authorization", "token "+a.config.Token()) + codeClientHTTP.AddDefaultHeaders(req, span.GetTraceId(), a.config.Organization()) // Make HTTP call resp, err := a.httpClient.Do(req) diff --git a/internal/analysis/mocks/analysis.go b/internal/analysis/mocks/analysis.go index 05f487a..538cb3b 100644 --- a/internal/analysis/mocks/analysis.go +++ b/internal/analysis/mocks/analysis.go @@ -21,6 +21,11 @@ type MockAnalysisOrchestrator struct { recorder *MockAnalysisOrchestratorMockRecorder } +func (m *MockAnalysisOrchestrator) RunLegacyTest(ctx context.Context, bundleHash string, shardKey string, limitToFiles []string, severity int) (*sarif.SarifResponse, analysis.LegacyAnalysisStatus, error) { + //TODO implement me + panic("implement me") +} + // MockAnalysisOrchestratorMockRecorder is the mock recorder for MockAnalysisOrchestrator. type MockAnalysisOrchestratorMockRecorder struct { mock *MockAnalysisOrchestrator diff --git a/llm/api_client.go b/llm/api_client.go index 465127d..09e750d 100644 --- a/llm/api_client.go +++ b/llm/api_client.go @@ -11,6 +11,8 @@ import ( "net/http" "net/url" "strings" + + codeClientHTTP "github.com/snyk/code-client-go/http" ) var ( @@ -74,7 +76,7 @@ func (d *DeepCodeLLMBindingImpl) submitRequest(ctx context.Context, url *url.URL return nil, err } - d.addDefaultHeaders(req, span.GetTraceId(), orgId) + codeClientHTTP.AddDefaultHeaders(req, span.GetTraceId(), orgId) resp, err := d.httpClientFunc().Do(req) //nolint:bodyclose // this seems to be a false positive if err != nil { @@ -258,15 +260,3 @@ func prepareDiffs(diffs []string) []string { } return encodedDiffs } - -func (d *DeepCodeLLMBindingImpl) addDefaultHeaders(req *http.Request, requestId string, orgId string) { - // if requestId is empty it will be enriched from the Gateway - if len(requestId) > 0 { - req.Header.Set("snyk-request-id", requestId) - } - if len(orgId) > 0 { - req.Header.Set("snyk-org-name", orgId) - } - req.Header.Set("Cache-Control", "private, max-age=0, no-cache") - req.Header.Set("Content-Type", "application/json") -} diff --git a/llm/api_client_test.go b/llm/api_client_test.go index 704c8b1..e46270e 100644 --- a/llm/api_client_test.go +++ b/llm/api_client_test.go @@ -14,6 +14,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + codeClientHTTP "github.com/snyk/code-client-go/http" "github.com/snyk/code-client-go/observability" ) @@ -262,10 +263,9 @@ func testLogger(t *testing.T) *zerolog.Logger { // Test with existing headers func TestAddDefaultHeadersWithExistingHeaders(t *testing.T) { - d := &DeepCodeLLMBindingImpl{} // Initialize your struct if needed req := &http.Request{Header: http.Header{"Existing-Header": {"existing-value"}}} - d.addDefaultHeaders(req, "", "") + codeClientHTTP.AddDefaultHeaders(req, "", "") cacheControl := req.Header.Get("Cache-Control") contentType := req.Header.Get("Content-Type") From f40eeb39209cd69f91260cf46fb2b18bbc4d5bdb Mon Sep 17 00:00:00 2001 From: Andrew Robinson Hodges Date: Tue, 5 Aug 2025 17:25:07 +0100 Subject: [PATCH 03/12] chore: move legacy analysis to its own file for separation of concerns --- internal/analysis/analysis.go | 205 ----------------------- internal/analysis/analysis_legacy.go | 235 +++++++++++++++++++++++++++ 2 files changed, 235 insertions(+), 205 deletions(-) create mode 100644 internal/analysis/analysis_legacy.go diff --git a/internal/analysis/analysis.go b/internal/analysis/analysis.go index f2fc630..b8f4e09 100644 --- a/internal/analysis/analysis.go +++ b/internal/analysis/analysis.go @@ -18,15 +18,12 @@ package analysis import ( - "bytes" "context" _ "embed" "encoding/json" "fmt" "io" - "math" "net/http" - "net/url" "strings" "time" @@ -61,50 +58,6 @@ type AnalysisConfig struct { CommitId *string } -// Legacy analysis types and constants -const ( - legacyCompleteStatus = "COMPLETE" -) - -type LegacyAnalysisStatus struct { - Message string - Percentage int -} - -type LegacyAnalysisRequestKey struct { - Type string `json:"type"` - Hash string `json:"hash"` - LimitToFiles []string `json:"limitToFiles,omitempty"` - Shard string `json:"shard"` -} - -type legacyCodeRequestContextOrg struct { - Name string `json:"name"` - DisplayName string `json:"displayName"` - PublicId string `json:"publicId"` - Flags map[string]bool `json:"flags"` -} - -type legacyCodeRequestContext struct { - Initiator string `json:"initiator"` - Flow string `json:"flow,omitempty"` - Org legacyCodeRequestContextOrg `json:"org,omitempty"` -} - -type LegacyAnalysisRequest struct { - Key LegacyAnalysisRequestKey `json:"key"` - Severity int `json:"severity,omitempty"` - Prioritized bool `json:"prioritized,omitempty"` - Legacy bool `json:"legacy"` - AnalysisContext legacyCodeRequestContext `json:"analysisContext"` -} - -type LegacyAnalysisFailedError struct { - Msg string -} - -func (e LegacyAnalysisFailedError) Error() string { return e.Msg } - type analysisOrchestrator struct { httpClient codeClientHTTP.HTTPClient instrumentor observability.Instrumentor @@ -401,161 +354,3 @@ func (a *analysisOrchestrator) retrieveTestURL(ctx context.Context, client *test return nil, false, fmt.Errorf("unexpected response status \"%d\"", parsedResponse.StatusCode()) } } - -// Legacy analysis helper functions -func (a *analysisOrchestrator) newLegacyCodeRequestContext() legacyCodeRequestContext { - unknown := "unknown" - orgId := unknown - if a.config.Organization() != "" { - orgId = a.config.Organization() - } - - // TODO - needs to be dynamically generated - return legacyCodeRequestContext{ - Initiator: "IDE", - Flow: "language-server", - Org: legacyCodeRequestContextOrg{ - Name: unknown, - DisplayName: unknown, - PublicId: orgId, - }, - } -} - -func (a *analysisOrchestrator) createLegacyAnalysisRequestBody(bundleHash, shardKey string, limitToFiles []string, severity int) ([]byte, error) { - request := LegacyAnalysisRequest{ - Key: LegacyAnalysisRequestKey{ - Type: "file", - Hash: bundleHash, - LimitToFiles: limitToFiles, - }, - Legacy: false, - AnalysisContext: a.newLegacyCodeRequestContext(), - } - if len(shardKey) > 0 { - request.Key.Shard = shardKey - } - if severity > 0 { - request.Severity = severity - } - - requestBody, err := json.Marshal(request) - return requestBody, err -} - -func (a *analysisOrchestrator) getLegacyCodeApiUrl() (string, error) { - // Use the same logic as the original SnykCodeHTTPClient - if !a.config.IsFedramp() { - return a.config.SnykCodeApi(), nil - } - u, err := url.Parse(a.config.SnykCodeApi()) - if err != nil { - return "", err - } - - // Apply fedramp transformation (this might need adjustment based on the actual requirements) - u.Host = strings.Replace(u.Host, "deeproxy", "api", 1) - - if a.config.Organization() == "" { - return "", errors.New("Organization is required in a fedramp environment") - } - - u.Path = "/hidden/orgs/" + a.config.Organization() + "/code" - return u.String(), nil -} - -func (a *analysisOrchestrator) logLegacySarifResponse(method string, sarifResponse sarif.SarifResponse) { - a.logger.Debug(). - Str("method", method). - Str("status", sarifResponse.Status). - Float64("progress", sarifResponse.Progress). - Int("fetchingCodeTime", sarifResponse.Timing.FetchingCode). - Int("analysisTime", sarifResponse.Timing.Analysis). - Int("filesAnalyzed", len(sarifResponse.Coverage)). - Msg("Received response summary") -} - -func (a *analysisOrchestrator) RunLegacyTest(ctx context.Context, bundleHash string, shardKey string, limitToFiles []string, severity int) (*sarif.SarifResponse, LegacyAnalysisStatus, error) { - method := "analysis.RunLegacyTest" - span := a.instrumentor.StartSpan(ctx, method) - defer a.instrumentor.Finish(span) - - a.logger.Debug().Str("method", method).Str("bundleHash", bundleHash).Msg("API: Retrieving legacy analysis for bundle") - defer a.logger.Debug().Str("method", method).Str("bundleHash", bundleHash).Msg("API: Retrieving legacy analysis done") - - requestBody, err := a.createLegacyAnalysisRequestBody(bundleHash, shardKey, limitToFiles, severity) - if err != nil { - a.logger.Err(err).Str("method", method).Str("requestBody", string(requestBody)).Msg("error creating request body") - return nil, LegacyAnalysisStatus{}, err - } - - // Get the legacy code API URL - baseUrl, err := a.getLegacyCodeApiUrl() - if err != nil { - return nil, LegacyAnalysisStatus{}, err - } - - // Create HTTP request - analysisUrl := baseUrl + "/analysis" - req, err := http.NewRequestWithContext(span.Context(), http.MethodPost, analysisUrl, bytes.NewBuffer(requestBody)) - if err != nil { - a.logger.Err(err).Str("method", method).Msg("error creating HTTP request") - return nil, LegacyAnalysisStatus{}, err - } - codeClientHTTP.AddDefaultHeaders(req, span.GetTraceId(), a.config.Organization()) - - // Make HTTP call - resp, err := a.httpClient.Do(req) - failed := LegacyAnalysisStatus{Message: "FAILED"} - if err != nil { - a.logger.Err(err).Str("method", method).Msg("error response from analysis") - return nil, failed, err - } - defer func() { - closeErr := resp.Body.Close() - if closeErr != nil { - a.logger.Err(closeErr).Msg("failed to close response body") - } - }() - - responseBody, err := io.ReadAll(resp.Body) - if err != nil { - a.logger.Err(err).Str("method", method).Msg("error reading response body") - return nil, failed, err - } - - // Check response status - if resp.StatusCode < 200 || resp.StatusCode >= 300 { - a.logger.Err(err).Str("method", method).Str("responseBody", string(responseBody)).Int("statusCode", resp.StatusCode).Msg("error response from analysis") - return nil, failed, LegacyAnalysisFailedError{Msg: string(responseBody)} - } - - var response sarif.SarifResponse - err = json.Unmarshal(responseBody, &response) - if err != nil { - a.logger.Err(err).Str("method", method).Str("responseBody", string(responseBody)).Msg("error unmarshalling") - return nil, failed, err - } else { - a.logLegacySarifResponse(method, response) - } - - a.logger.Debug().Str("method", method).Str("bundleHash", bundleHash).Float64("progress", - response.Progress).Msgf("Status: %s", response.Status) - - if response.Status == failed.Message { - a.logger.Err(err).Str("method", method).Str("responseStatus", response.Status).Msg("analysis failed") - return nil, failed, LegacyAnalysisFailedError{Msg: string(responseBody)} - } - - if response.Status == "" { - a.logger.Err(err).Str("method", method).Str("responseStatus", response.Status).Msg("unknown response status (empty)") - return nil, failed, LegacyAnalysisFailedError{Msg: string(responseBody)} - } - - status := LegacyAnalysisStatus{Message: response.Status, Percentage: int(math.RoundToEven(response.Progress * 100))} - if response.Status != legacyCompleteStatus { - return nil, status, nil - } - - return &response, status, nil -} diff --git a/internal/analysis/analysis_legacy.go b/internal/analysis/analysis_legacy.go new file mode 100644 index 0000000..66c1b99 --- /dev/null +++ b/internal/analysis/analysis_legacy.go @@ -0,0 +1,235 @@ +/* + * © 2024 Snyk Limited All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +//nolint:lll // Some of the lines in this file are going to be long for now. +package analysis + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "io" + "math" + "net/http" + "net/url" + "strings" + + codeClientHTTP "github.com/snyk/code-client-go/http" + "github.com/snyk/code-client-go/sarif" +) + +// Legacy analysis types and constants +const ( + legacyCompleteStatus = "COMPLETE" +) + +type LegacyAnalysisStatus struct { + Message string + Percentage int +} + +type LegacyAnalysisRequestKey struct { + Type string `json:"type"` + Hash string `json:"hash"` + LimitToFiles []string `json:"limitToFiles,omitempty"` + Shard string `json:"shard"` +} + +type legacyCodeRequestContextOrg struct { + Name string `json:"name"` + DisplayName string `json:"displayName"` + PublicId string `json:"publicId"` + Flags map[string]bool `json:"flags"` +} + +type legacyCodeRequestContext struct { + Initiator string `json:"initiator"` + Flow string `json:"flow,omitempty"` + Org legacyCodeRequestContextOrg `json:"org,omitempty"` +} + +type LegacyAnalysisRequest struct { + Key LegacyAnalysisRequestKey `json:"key"` + Severity int `json:"severity,omitempty"` + Prioritized bool `json:"prioritized,omitempty"` + Legacy bool `json:"legacy"` + AnalysisContext legacyCodeRequestContext `json:"analysisContext"` +} + +type LegacyAnalysisFailedError struct { + Msg string +} + +func (e LegacyAnalysisFailedError) Error() string { return e.Msg } + +// Legacy analysis helper functions +func (a *analysisOrchestrator) newLegacyCodeRequestContext() legacyCodeRequestContext { + unknown := "unknown" + orgId := unknown + if a.config.Organization() != "" { + orgId = a.config.Organization() + } + + // TODO - needs to be dynamically generated + return legacyCodeRequestContext{ + Initiator: "IDE", + Flow: "language-server", + Org: legacyCodeRequestContextOrg{ + Name: unknown, + DisplayName: unknown, + PublicId: orgId, + }, + } +} + +func (a *analysisOrchestrator) createLegacyAnalysisRequestBody(bundleHash, shardKey string, limitToFiles []string, severity int) ([]byte, error) { + request := LegacyAnalysisRequest{ + Key: LegacyAnalysisRequestKey{ + Type: "file", + Hash: bundleHash, + LimitToFiles: limitToFiles, + }, + Legacy: false, + AnalysisContext: a.newLegacyCodeRequestContext(), + } + if len(shardKey) > 0 { + request.Key.Shard = shardKey + } + if severity > 0 { + request.Severity = severity + } + + requestBody, err := json.Marshal(request) + return requestBody, err +} + +func (a *analysisOrchestrator) getLegacyCodeApiUrl() (string, error) { + // Use the same logic as the original SnykCodeHTTPClient + if !a.config.IsFedramp() { + return a.config.SnykCodeApi(), nil + } + u, err := url.Parse(a.config.SnykCodeApi()) + if err != nil { + return "", err + } + + // Apply fedramp transformation (this might need adjustment based on the actual requirements) + u.Host = strings.Replace(u.Host, "deeproxy", "api", 1) + + if a.config.Organization() == "" { + return "", errors.New("organization is required in a fedramp environment") + } + + u.Path = "/hidden/orgs/" + a.config.Organization() + "/code" + return u.String(), nil +} + +func (a *analysisOrchestrator) logLegacySarifResponse(method string, sarifResponse sarif.SarifResponse) { + a.logger.Debug(). + Str("method", method). + Str("status", sarifResponse.Status). + Float64("progress", sarifResponse.Progress). + Int("fetchingCodeTime", sarifResponse.Timing.FetchingCode). + Int("analysisTime", sarifResponse.Timing.Analysis). + Int("filesAnalyzed", len(sarifResponse.Coverage)). + Msg("Received response summary") +} + +func (a *analysisOrchestrator) RunLegacyTest(ctx context.Context, bundleHash string, shardKey string, limitToFiles []string, severity int) (*sarif.SarifResponse, LegacyAnalysisStatus, error) { + method := "analysis.RunLegacyTest" + span := a.instrumentor.StartSpan(ctx, method) + defer a.instrumentor.Finish(span) + + a.logger.Debug().Str("method", method).Str("bundleHash", bundleHash).Msg("API: Retrieving legacy analysis for bundle") + defer a.logger.Debug().Str("method", method).Str("bundleHash", bundleHash).Msg("API: Retrieving legacy analysis done") + + requestBody, err := a.createLegacyAnalysisRequestBody(bundleHash, shardKey, limitToFiles, severity) + if err != nil { + a.logger.Err(err).Str("method", method).Str("requestBody", string(requestBody)).Msg("error creating request body") + return nil, LegacyAnalysisStatus{}, err + } + + // Get the legacy code API URL + baseUrl, err := a.getLegacyCodeApiUrl() + if err != nil { + return nil, LegacyAnalysisStatus{}, err + } + + // Create HTTP request + analysisUrl := baseUrl + "/analysis" + req, err := http.NewRequestWithContext(span.Context(), http.MethodPost, analysisUrl, bytes.NewBuffer(requestBody)) + if err != nil { + a.logger.Err(err).Str("method", method).Msg("error creating HTTP request") + return nil, LegacyAnalysisStatus{}, err + } + codeClientHTTP.AddDefaultHeaders(req, span.GetTraceId(), a.config.Organization()) + + // Make HTTP call + resp, err := a.httpClient.Do(req) + failed := LegacyAnalysisStatus{Message: "FAILED"} + if err != nil { + a.logger.Err(err).Str("method", method).Msg("error response from analysis") + return nil, failed, err + } + defer func() { + closeErr := resp.Body.Close() + if closeErr != nil { + a.logger.Err(closeErr).Msg("failed to close response body") + } + }() + + responseBody, err := io.ReadAll(resp.Body) + if err != nil { + a.logger.Err(err).Str("method", method).Msg("error reading response body") + return nil, failed, err + } + + // Check response status + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + a.logger.Err(err).Str("method", method).Str("responseBody", string(responseBody)).Int("statusCode", resp.StatusCode).Msg("error response from analysis") + return nil, failed, LegacyAnalysisFailedError{Msg: string(responseBody)} + } + + var response sarif.SarifResponse + err = json.Unmarshal(responseBody, &response) + if err != nil { + a.logger.Err(err).Str("method", method).Str("responseBody", string(responseBody)).Msg("error unmarshalling") + return nil, failed, err + } else { + a.logLegacySarifResponse(method, response) + } + + a.logger.Debug().Str("method", method).Str("bundleHash", bundleHash).Float64("progress", + response.Progress).Msgf("Status: %s", response.Status) + + if response.Status == failed.Message { + a.logger.Err(err).Str("method", method).Str("responseStatus", response.Status).Msg("analysis failed") + return nil, failed, LegacyAnalysisFailedError{Msg: string(responseBody)} + } + + if response.Status == "" { + a.logger.Err(err).Str("method", method).Str("responseStatus", response.Status).Msg("unknown response status (empty)") + return nil, failed, LegacyAnalysisFailedError{Msg: string(responseBody)} + } + + status := LegacyAnalysisStatus{Message: response.Status, Percentage: int(math.RoundToEven(response.Progress * 100))} + if response.Status != legacyCompleteStatus { + return nil, status, nil + } + + return &response, status, nil +} From 992578d19d0083f88d854939021f0c8d5ad3f213 Mon Sep 17 00:00:00 2001 From: Andrew Robinson Hodges Date: Thu, 7 Aug 2025 16:51:33 +0100 Subject: [PATCH 04/12] feat: added status reporting to lecacy code scan --- http/http.go | 2 +- internal/analysis/analysis.go | 2 +- internal/analysis/analysis_legacy.go | 92 ++++++++++++++-------------- internal/analysis/mocks/analysis.go | 2 +- scan.go | 50 ++++++++++++++- scan/tracker.go | 6 ++ 6 files changed, 101 insertions(+), 53 deletions(-) diff --git a/http/http.go b/http/http.go index 7d627bf..7b6d549 100644 --- a/http/http.go +++ b/http/http.go @@ -24,7 +24,7 @@ import ( "time" "github.com/rs/zerolog" - + "github.com/snyk/code-client-go/observability" ) diff --git a/internal/analysis/analysis.go b/internal/analysis/analysis.go index b8f4e09..a86bc9a 100644 --- a/internal/analysis/analysis.go +++ b/internal/analysis/analysis.go @@ -46,7 +46,7 @@ import ( type AnalysisOrchestrator interface { RunTest(ctx context.Context, orgId string, b bundle.Bundle, target scan.Target, reportingOptions AnalysisConfig) (*sarif.SarifResponse, *scan.ResultMetaData, error) RunTestRemote(ctx context.Context, orgId string, reportingOptions AnalysisConfig) (*sarif.SarifResponse, *scan.ResultMetaData, error) - RunLegacyTest(ctx context.Context, bundleHash string, shardKey string, limitToFiles []string, severity int) (*sarif.SarifResponse, LegacyAnalysisStatus, error) + RunLegacyTest(ctx context.Context, bundleHash string, shardKey string, limitToFiles []string, severity int) (*sarif.SarifResponse, scan.LegacyScanStatus, error) } type AnalysisConfig struct { diff --git a/internal/analysis/analysis_legacy.go b/internal/analysis/analysis_legacy.go index 66c1b99..4fa4ee6 100644 --- a/internal/analysis/analysis_legacy.go +++ b/internal/analysis/analysis_legacy.go @@ -1,5 +1,5 @@ /* - * © 2024 Snyk Limited All rights reserved. + * © 2025 Snyk Limited All rights reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,6 +22,7 @@ import ( "context" "encoding/json" "errors" + "github.com/snyk/code-client-go/scan" "io" "math" "net/http" @@ -34,61 +35,57 @@ import ( // Legacy analysis types and constants const ( - legacyCompleteStatus = "COMPLETE" + StatusComplete = "COMPLETE" + StatusFailed = "FAILED" + StatusAnalyzing = "ANALYZING" ) -type LegacyAnalysisStatus struct { - Message string - Percentage int -} - -type LegacyAnalysisRequestKey struct { +type RequestKey struct { Type string `json:"type"` Hash string `json:"hash"` LimitToFiles []string `json:"limitToFiles,omitempty"` Shard string `json:"shard"` } -type legacyCodeRequestContextOrg struct { +type requestContextOrg struct { Name string `json:"name"` DisplayName string `json:"displayName"` PublicId string `json:"publicId"` Flags map[string]bool `json:"flags"` } -type legacyCodeRequestContext struct { - Initiator string `json:"initiator"` - Flow string `json:"flow,omitempty"` - Org legacyCodeRequestContextOrg `json:"org,omitempty"` +type requestContext struct { + Initiator string `json:"initiator"` + Flow string `json:"flow,omitempty"` + Org requestContextOrg `json:"org,omitempty"` } -type LegacyAnalysisRequest struct { - Key LegacyAnalysisRequestKey `json:"key"` - Severity int `json:"severity,omitempty"` - Prioritized bool `json:"prioritized,omitempty"` - Legacy bool `json:"legacy"` - AnalysisContext legacyCodeRequestContext `json:"analysisContext"` +type Request struct { + Key RequestKey `json:"key"` + Severity int `json:"severity,omitempty"` + Prioritized bool `json:"prioritized,omitempty"` + Legacy bool `json:"legacy"` + AnalysisContext requestContext `json:"analysisContext"` } -type LegacyAnalysisFailedError struct { +type FailedError struct { Msg string } -func (e LegacyAnalysisFailedError) Error() string { return e.Msg } +func (e FailedError) Error() string { return e.Msg } // Legacy analysis helper functions -func (a *analysisOrchestrator) newLegacyCodeRequestContext() legacyCodeRequestContext { +func (a *analysisOrchestrator) newRequestContext() requestContext { unknown := "unknown" orgId := unknown if a.config.Organization() != "" { orgId = a.config.Organization() } - // TODO - needs to be dynamically generated - return legacyCodeRequestContext{ + return requestContext{ Initiator: "IDE", Flow: "language-server", - Org: legacyCodeRequestContextOrg{ + Org: requestContextOrg{ Name: unknown, DisplayName: unknown, PublicId: orgId, @@ -96,15 +93,15 @@ func (a *analysisOrchestrator) newLegacyCodeRequestContext() legacyCodeRequestCo } } -func (a *analysisOrchestrator) createLegacyAnalysisRequestBody(bundleHash, shardKey string, limitToFiles []string, severity int) ([]byte, error) { - request := LegacyAnalysisRequest{ - Key: LegacyAnalysisRequestKey{ +func (a *analysisOrchestrator) createRequestBody(bundleHash, shardKey string, limitToFiles []string, severity int) ([]byte, error) { + request := Request{ + Key: RequestKey{ Type: "file", Hash: bundleHash, LimitToFiles: limitToFiles, }, Legacy: false, - AnalysisContext: a.newLegacyCodeRequestContext(), + AnalysisContext: a.newRequestContext(), } if len(shardKey) > 0 { request.Key.Shard = shardKey @@ -117,7 +114,7 @@ func (a *analysisOrchestrator) createLegacyAnalysisRequestBody(bundleHash, shard return requestBody, err } -func (a *analysisOrchestrator) getLegacyCodeApiUrl() (string, error) { +func (a *analysisOrchestrator) getCodeApiUrl() (string, error) { // Use the same logic as the original SnykCodeHTTPClient if !a.config.IsFedramp() { return a.config.SnykCodeApi(), nil @@ -138,7 +135,8 @@ func (a *analysisOrchestrator) getLegacyCodeApiUrl() (string, error) { return u.String(), nil } -func (a *analysisOrchestrator) logLegacySarifResponse(method string, sarifResponse sarif.SarifResponse) { +// TODO combine? +func (a *analysisOrchestrator) logSarifResponse(method string, sarifResponse sarif.SarifResponse) { a.logger.Debug(). Str("method", method). Str("status", sarifResponse.Status). @@ -149,24 +147,24 @@ func (a *analysisOrchestrator) logLegacySarifResponse(method string, sarifRespon Msg("Received response summary") } -func (a *analysisOrchestrator) RunLegacyTest(ctx context.Context, bundleHash string, shardKey string, limitToFiles []string, severity int) (*sarif.SarifResponse, LegacyAnalysisStatus, error) { +func (a *analysisOrchestrator) RunLegacyTest(ctx context.Context, bundleHash string, shardKey string, limitToFiles []string, severity int) (*sarif.SarifResponse, scan.LegacyScanStatus, error) { method := "analysis.RunLegacyTest" span := a.instrumentor.StartSpan(ctx, method) defer a.instrumentor.Finish(span) - a.logger.Debug().Str("method", method).Str("bundleHash", bundleHash).Msg("API: Retrieving legacy analysis for bundle") - defer a.logger.Debug().Str("method", method).Str("bundleHash", bundleHash).Msg("API: Retrieving legacy analysis done") + a.logger.Debug().Str("method", method).Str("bundleHash", bundleHash).Msg("API: Retrieving analysis for bundle") + defer a.logger.Debug().Str("method", method).Str("bundleHash", bundleHash).Msg("API: Retrieving analysis done") - requestBody, err := a.createLegacyAnalysisRequestBody(bundleHash, shardKey, limitToFiles, severity) + requestBody, err := a.createRequestBody(bundleHash, shardKey, limitToFiles, severity) if err != nil { a.logger.Err(err).Str("method", method).Str("requestBody", string(requestBody)).Msg("error creating request body") - return nil, LegacyAnalysisStatus{}, err + return nil, scan.LegacyScanStatus{}, err } // Get the legacy code API URL - baseUrl, err := a.getLegacyCodeApiUrl() + baseUrl, err := a.getCodeApiUrl() if err != nil { - return nil, LegacyAnalysisStatus{}, err + return nil, scan.LegacyScanStatus{}, err } // Create HTTP request @@ -174,13 +172,13 @@ func (a *analysisOrchestrator) RunLegacyTest(ctx context.Context, bundleHash str req, err := http.NewRequestWithContext(span.Context(), http.MethodPost, analysisUrl, bytes.NewBuffer(requestBody)) if err != nil { a.logger.Err(err).Str("method", method).Msg("error creating HTTP request") - return nil, LegacyAnalysisStatus{}, err + return nil, scan.LegacyScanStatus{}, err } codeClientHTTP.AddDefaultHeaders(req, span.GetTraceId(), a.config.Organization()) // Make HTTP call resp, err := a.httpClient.Do(req) - failed := LegacyAnalysisStatus{Message: "FAILED"} + failed := scan.LegacyScanStatus{Message: StatusFailed} if err != nil { a.logger.Err(err).Str("method", method).Msg("error response from analysis") return nil, failed, err @@ -201,7 +199,7 @@ func (a *analysisOrchestrator) RunLegacyTest(ctx context.Context, bundleHash str // Check response status if resp.StatusCode < 200 || resp.StatusCode >= 300 { a.logger.Err(err).Str("method", method).Str("responseBody", string(responseBody)).Int("statusCode", resp.StatusCode).Msg("error response from analysis") - return nil, failed, LegacyAnalysisFailedError{Msg: string(responseBody)} + return nil, failed, FailedError{Msg: string(responseBody)} } var response sarif.SarifResponse @@ -210,24 +208,24 @@ func (a *analysisOrchestrator) RunLegacyTest(ctx context.Context, bundleHash str a.logger.Err(err).Str("method", method).Str("responseBody", string(responseBody)).Msg("error unmarshalling") return nil, failed, err } else { - a.logLegacySarifResponse(method, response) + a.logSarifResponse(method, response) } a.logger.Debug().Str("method", method).Str("bundleHash", bundleHash).Float64("progress", - response.Progress).Msgf("Status: %s", response.Status) + response.Progress).Msgf("LegacyScanStatus: %s", response.Status) if response.Status == failed.Message { a.logger.Err(err).Str("method", method).Str("responseStatus", response.Status).Msg("analysis failed") - return nil, failed, LegacyAnalysisFailedError{Msg: string(responseBody)} + return nil, failed, FailedError{Msg: string(responseBody)} } if response.Status == "" { a.logger.Err(err).Str("method", method).Str("responseStatus", response.Status).Msg("unknown response status (empty)") - return nil, failed, LegacyAnalysisFailedError{Msg: string(responseBody)} + return nil, failed, FailedError{Msg: string(responseBody)} } - status := LegacyAnalysisStatus{Message: response.Status, Percentage: int(math.RoundToEven(response.Progress * 100))} - if response.Status != legacyCompleteStatus { + status := scan.LegacyScanStatus{Message: response.Status, Percentage: int(math.RoundToEven(response.Progress * 100))} + if response.Status != StatusComplete { return nil, status, nil } diff --git a/internal/analysis/mocks/analysis.go b/internal/analysis/mocks/analysis.go index 538cb3b..6b3c99d 100644 --- a/internal/analysis/mocks/analysis.go +++ b/internal/analysis/mocks/analysis.go @@ -21,7 +21,7 @@ type MockAnalysisOrchestrator struct { recorder *MockAnalysisOrchestratorMockRecorder } -func (m *MockAnalysisOrchestrator) RunLegacyTest(ctx context.Context, bundleHash string, shardKey string, limitToFiles []string, severity int) (*sarif.SarifResponse, analysis.LegacyAnalysisStatus, error) { +func (m *MockAnalysisOrchestrator) RunLegacyTest(ctx context.Context, bundleHash string, shardKey string, limitToFiles []string, severity int) (*sarif.SarifResponse, scan.LegacyScanStatus, error) { //TODO implement me panic("implement me") } diff --git a/scan.go b/scan.go index 919aee5..6cb71ed 100644 --- a/scan.go +++ b/scan.go @@ -19,9 +19,11 @@ package codeclient import ( "context" + "fmt" "github.com/google/uuid" "github.com/pkg/errors" "github.com/rs/zerolog" + "time" "github.com/snyk/code-client-go/bundle" "github.com/snyk/code-client-go/config" @@ -71,6 +73,7 @@ type CodeScanner interface { shardKey string, files <-chan string, changedFiles map[string]bool, + statusChannel chan<- scan.LegacyScanStatus, ) (*sarif.SarifResponse, string, error) } @@ -272,6 +275,7 @@ func (c *codeScanner) UploadAndAnalyzeLegacy( shardKey string, files <-chan string, changedFiles map[string]bool, + statusChannel chan<- scan.LegacyScanStatus, ) (*sarif.SarifResponse, string, error) { uploadedBundle, err := c.Upload(ctx, requestId, target, files, changedFiles) if err != nil || uploadedBundle == nil || uploadedBundle.GetBundleHash() == "" { @@ -283,10 +287,50 @@ func (c *codeScanner) UploadAndAnalyzeLegacy( limitToFiles := uploadedBundle.GetLimitToFiles() severity := 0 - response, _, err := c.analysisOrchestrator.RunLegacyTest(ctx, bundleHash, shardKey, limitToFiles, severity) + start := time.Now() + for { + response, status, err := c.analysisOrchestrator.RunLegacyTest(ctx, bundleHash, shardKey, limitToFiles, severity) - //TODO status? - return response, bundleHash, err + if err != nil { + c.logger.Error().Err(err). + Int("fileCount", len(uploadedBundle.GetFiles())). + Msg("error retrieving diagnostics...") + + statusChannel <- scan.LegacyScanStatus{ + Message: fmt.Sprintf("Analysis failed: %v", err), + ScanStopped: true, + } + + return nil, "", err + } + + if status.Message == analysis.StatusComplete { + c.logger.Trace().Msg("sending diagnostics...") + + statusChannel <- scan.LegacyScanStatus{ + Message: "Analysis complete.", + ScanStopped: true, + } + + return response, bundleHash, err + } else if status.Message == analysis.StatusAnalyzing { + c.logger.Trace().Msg("\"Analyzing\" message received, sending In-Progress message to client") + } + + if time.Since(start) > c.config.SnykCodeAnalysisTimeout() { + err := errors.New("analysis call timed out") + c.logger.Error().Err(err).Msg("timeout...") + + statusChannel <- scan.LegacyScanStatus{ + Message: "Snyk Code Analysis timed out", + ScanStopped: true, + } + return nil, "", err + } + + time.Sleep(1 * time.Second) + statusChannel <- status + } } func (c *codeScanner) UploadAndAnalyzeWithOptions( diff --git a/scan/tracker.go b/scan/tracker.go index a96eb13..3ebe767 100644 --- a/scan/tracker.go +++ b/scan/tracker.go @@ -24,3 +24,9 @@ type Tracker interface { Begin(title, message string) End(message string) } + +type LegacyScanStatus struct { + Message string + Percentage int + ScanStopped bool +} From 494e815265d7b92c62a6710588f1e5400faa5e65 Mon Sep 17 00:00:00 2001 From: Andrew Robinson Hodges Date: Thu, 7 Aug 2025 17:08:07 +0100 Subject: [PATCH 05/12] fix: lint --- scan.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/scan.go b/scan.go index 6cb71ed..65665d7 100644 --- a/scan.go +++ b/scan.go @@ -118,13 +118,6 @@ func WithTrackerFactory(trackerFactory scan.TrackerFactory) OptionFunc { type AnalysisOption func(*analysis.AnalysisConfig) -type LegacyAnalysisOption struct { - bundleHash string - shardKey string - limitToFiles []string - severity int -} - func ReportLocalTest(projectName string, targetName string, targetReference string) AnalysisOption { return func(c *analysis.AnalysisConfig) { c.Report = true From 8019c9f4a1eb9b8ce465652cb5a9a5152caa9e0e Mon Sep 17 00:00:00 2001 From: Andrew Robinson Hodges Date: Fri, 8 Aug 2025 11:10:03 +0100 Subject: [PATCH 06/12] fix: safer handling of channel for scan updates --- internal/analysis/mocks/analysis.go | 8 ++++-- scan.go | 40 ++++++++++++++--------------- scan/tracker.go | 12 ++++++--- 3 files changed, 34 insertions(+), 26 deletions(-) diff --git a/internal/analysis/mocks/analysis.go b/internal/analysis/mocks/analysis.go index 6b3c99d..173dd70 100644 --- a/internal/analysis/mocks/analysis.go +++ b/internal/analysis/mocks/analysis.go @@ -22,8 +22,12 @@ type MockAnalysisOrchestrator struct { } func (m *MockAnalysisOrchestrator) RunLegacyTest(ctx context.Context, bundleHash string, shardKey string, limitToFiles []string, severity int) (*sarif.SarifResponse, scan.LegacyScanStatus, error) { - //TODO implement me - panic("implement me") + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RunLegacyTest", ctx, bundleHash, shardKey, limitToFiles, severity) + ret0, _ := ret[0].(*sarif.SarifResponse) + ret1, _ := ret[1].( scan.LegacyScanStatus) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 } // MockAnalysisOrchestratorMockRecorder is the mock recorder for MockAnalysisOrchestrator. diff --git a/scan.go b/scan.go index 65665d7..f239271 100644 --- a/scan.go +++ b/scan.go @@ -276,8 +276,19 @@ func (c *codeScanner) UploadAndAnalyzeLegacy( return nil, "", err } - bundleHash := uploadedBundle.GetBundleHash() - limitToFiles := uploadedBundle.GetLimitToFiles() + response, bundleHash, err := c.analyzeLegacy(ctx, uploadedBundle, shardKey, statusChannel) + close(statusChannel) + return response, bundleHash, err +} + +func (c *codeScanner) analyzeLegacy( + ctx context.Context, + bundle bundle.Bundle, + shardKey string, + statusChannel chan<- scan.LegacyScanStatus, +) (*sarif.SarifResponse, string, error) { + bundleHash := bundle.GetBundleHash() + limitToFiles := bundle.GetLimitToFiles() severity := 0 start := time.Now() @@ -286,42 +297,29 @@ func (c *codeScanner) UploadAndAnalyzeLegacy( if err != nil { c.logger.Error().Err(err). - Int("fileCount", len(uploadedBundle.GetFiles())). + Int("fileCount", len(bundle.GetFiles())). Msg("error retrieving diagnostics...") - - statusChannel <- scan.LegacyScanStatus{ - Message: fmt.Sprintf("Analysis failed: %v", err), - ScanStopped: true, - } - + statusChannel <- scan.NewLegacyScanDoneStatus(fmt.Sprintf("Analysis failed: %v", err)) return nil, "", err } if status.Message == analysis.StatusComplete { c.logger.Trace().Msg("sending diagnostics...") - - statusChannel <- scan.LegacyScanStatus{ - Message: "Analysis complete.", - ScanStopped: true, - } - + statusChannel <- scan.NewLegacyScanDoneStatus("Analysis complete") return response, bundleHash, err } else if status.Message == analysis.StatusAnalyzing { - c.logger.Trace().Msg("\"Analyzing\" message received, sending In-Progress message to client") + c.logger.Trace().Msg("\"Analyzing\" message received") } if time.Since(start) > c.config.SnykCodeAnalysisTimeout() { err := errors.New("analysis call timed out") c.logger.Error().Err(err).Msg("timeout...") - - statusChannel <- scan.LegacyScanStatus{ - Message: "Snyk Code Analysis timed out", - ScanStopped: true, - } + statusChannel <- scan.NewLegacyScanDoneStatus("Snyk Code Analysis timed out") return nil, "", err } time.Sleep(1 * time.Second) + c.logger.Trace().Msg("sending In-Progress message to client") statusChannel <- status } } diff --git a/scan/tracker.go b/scan/tracker.go index 3ebe767..acaa41d 100644 --- a/scan/tracker.go +++ b/scan/tracker.go @@ -26,7 +26,13 @@ type Tracker interface { } type LegacyScanStatus struct { - Message string - Percentage int - ScanStopped bool + Message string + Percentage int +} + +func NewLegacyScanDoneStatus(message string) LegacyScanStatus { + return LegacyScanStatus{ + Percentage: 90, + Message: message, + } } From 8ec133ead22d44d172eafdf97608eceaa9be70da Mon Sep 17 00:00:00 2001 From: Andrew Robinson Hodges Date: Fri, 8 Aug 2025 11:50:43 +0100 Subject: [PATCH 07/12] chore: generate mocks --- internal/analysis/mocks/analysis.go | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/internal/analysis/mocks/analysis.go b/internal/analysis/mocks/analysis.go index 173dd70..9f7a796 100644 --- a/internal/analysis/mocks/analysis.go +++ b/internal/analysis/mocks/analysis.go @@ -21,15 +21,6 @@ type MockAnalysisOrchestrator struct { recorder *MockAnalysisOrchestratorMockRecorder } -func (m *MockAnalysisOrchestrator) RunLegacyTest(ctx context.Context, bundleHash string, shardKey string, limitToFiles []string, severity int) (*sarif.SarifResponse, scan.LegacyScanStatus, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "RunLegacyTest", ctx, bundleHash, shardKey, limitToFiles, severity) - ret0, _ := ret[0].(*sarif.SarifResponse) - ret1, _ := ret[1].( scan.LegacyScanStatus) - ret2, _ := ret[2].(error) - return ret0, ret1, ret2 -} - // MockAnalysisOrchestratorMockRecorder is the mock recorder for MockAnalysisOrchestrator. type MockAnalysisOrchestratorMockRecorder struct { mock *MockAnalysisOrchestrator @@ -47,6 +38,22 @@ func (m *MockAnalysisOrchestrator) EXPECT() *MockAnalysisOrchestratorMockRecorde return m.recorder } +// RunLegacyTest mocks base method. +func (m *MockAnalysisOrchestrator) RunLegacyTest(ctx context.Context, bundleHash, shardKey string, limitToFiles []string, severity int) (*sarif.SarifResponse, scan.LegacyScanStatus, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RunLegacyTest", ctx, bundleHash, shardKey, limitToFiles, severity) + ret0, _ := ret[0].(*sarif.SarifResponse) + ret1, _ := ret[1].(scan.LegacyScanStatus) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// RunLegacyTest indicates an expected call of RunLegacyTest. +func (mr *MockAnalysisOrchestratorMockRecorder) RunLegacyTest(ctx, bundleHash, shardKey, limitToFiles, severity interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RunLegacyTest", reflect.TypeOf((*MockAnalysisOrchestrator)(nil).RunLegacyTest), ctx, bundleHash, shardKey, limitToFiles, severity) +} + // RunTest mocks base method. func (m *MockAnalysisOrchestrator) RunTest(ctx context.Context, orgId string, b bundle.Bundle, target scan.Target, reportingOptions analysis.AnalysisConfig) (*sarif.SarifResponse, *scan.ResultMetaData, error) { m.ctrl.T.Helper() From b086cc440e8952cda18044174a6625c0b3d270d4 Mon Sep 17 00:00:00 2001 From: Andrew Robinson Hodges Date: Fri, 8 Aug 2025 16:32:16 +0100 Subject: [PATCH 08/12] test: add tests for legacy analysis --- internal/analysis/analysis_legacy_test.go | 722 ++++++++++++++++++++++ 1 file changed, 722 insertions(+) create mode 100644 internal/analysis/analysis_legacy_test.go diff --git a/internal/analysis/analysis_legacy_test.go b/internal/analysis/analysis_legacy_test.go new file mode 100644 index 0000000..f9aacd0 --- /dev/null +++ b/internal/analysis/analysis_legacy_test.go @@ -0,0 +1,722 @@ +/* + * © 2025 Snyk Limited All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package analysis_test + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "testing" + "time" + + "github.com/golang/mock/gomock" + "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + confMocks "github.com/snyk/code-client-go/config/mocks" + httpmocks "github.com/snyk/code-client-go/http/mocks" + "github.com/snyk/code-client-go/internal/analysis" + "github.com/snyk/code-client-go/observability/mocks" + "github.com/snyk/code-client-go/sarif" + "github.com/snyk/code-client-go/scan" + trackerMocks "github.com/snyk/code-client-go/scan/mocks" +) + +func mockLegacyAnalysisResponse(t *testing.T, mockHTTPClient *httpmocks.MockHTTPClient, sarifResponse sarif.SarifResponse, bundleHash string, orgId string, responseCode int) { + t.Helper() + responseBodyBytes, err := json.Marshal(sarifResponse) + assert.NoError(t, err) + expectedAnalysisUrl := fmt.Sprintf("http://localhost/analysis") + mockHTTPClient.EXPECT().Do(mock.MatchedBy(func(i interface{}) bool { + req := i.(*http.Request) + return req.URL.String() == expectedAnalysisUrl && req.Method == http.MethodPost + })).Times(1).Return(&http.Response{ + StatusCode: responseCode, + Header: http.Header{ + "Content-Type": []string{"application/json"}, + }, + Body: io.NopCloser(bytes.NewReader(responseBodyBytes)), + }, mockDeriveErrorFromStatusCode(responseCode)) +} + +func setupLegacy(t *testing.T, timeout *time.Duration, isFedramp bool, snykCodeApi string, orgId string) (*confMocks.MockConfig, *httpmocks.MockHTTPClient, *mocks.MockInstrumentor, *mocks.MockErrorReporter, *trackerMocks.MockTracker, *trackerMocks.MockTrackerFactory, zerolog.Logger) { + t.Helper() + ctrl := gomock.NewController(t) + mockSpan := mocks.NewMockSpan(ctrl) + mockSpan.EXPECT().GetTraceId().AnyTimes().Return("test-trace-id") + mockSpan.EXPECT().Context().AnyTimes().Return(context.Background()) + mockConfig := confMocks.NewMockConfig(ctrl) + mockConfig.EXPECT().Organization().AnyTimes().Return(orgId) + if snykCodeApi == "" { + snykCodeApi = "http://localhost" + } + mockConfig.EXPECT().SnykCodeApi().AnyTimes().Return(snykCodeApi) + mockConfig.EXPECT().IsFedramp().AnyTimes().Return(isFedramp) + if timeout == nil { + defaultTimeout := 120 * time.Second + timeout = &defaultTimeout + } + mockConfig.EXPECT().SnykCodeAnalysisTimeout().AnyTimes().Return(*timeout) + + mockHTTPClient := httpmocks.NewMockHTTPClient(ctrl) + + mockInstrumentor := mocks.NewMockInstrumentor(ctrl) + mockInstrumentor.EXPECT().StartSpan(gomock.Any(), gomock.Any()).Return(mockSpan).AnyTimes() + mockInstrumentor.EXPECT().Finish(gomock.Any()).AnyTimes() + mockErrorReporter := mocks.NewMockErrorReporter(ctrl) + mockTracker := trackerMocks.NewMockTracker(ctrl) + mockTrackerFactory := trackerMocks.NewMockTrackerFactory(ctrl) + mockTrackerFactory.EXPECT().GenerateTracker().Return(mockTracker).AnyTimes() + + logger := zerolog.Nop() + return mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, mockTracker, mockTrackerFactory, logger +} + +func TestAnalysis_RunLegacyTest_Success(t *testing.T) { + bundleHash := "test-bundle-hash" + shardKey := "test-shard-key" + limitToFiles := []string{"file1.js", "file2.js"} + severity := 2 + orgId := "test-org-id" + + mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, _, mockTrackerFactory, logger := setupLegacy(t, nil, false, "", orgId) + + // Create expected sarif response + sarifResponse := sarif.SarifResponse{ + Type: "sarif", + Progress: 1.0, + Status: analysis.StatusComplete, + Sarif: sarif.SarifDocument{ + Version: "2.1.0", + Runs: []sarif.Run{ + { + Tool: sarif.Tool{ + Driver: sarif.Driver{ + Name: "SnykCode", + Version: "1.0.0", + }, + }, + Results: []sarif.Result{}, + }, + }, + }, + Coverage: []sarif.SarifCoverage{ + { + Files: 5, + IsSupported: true, + Lang: "javascript", + }, + }, + Timing: struct { + FetchingCode int `json:"fetchingCode"` + Queue int `json:"queue"` + Analysis int `json:"analysis"` + }{ + FetchingCode: 100, + Analysis: 500, + }, + } + + mockLegacyAnalysisResponse(t, mockHTTPClient, sarifResponse, bundleHash, orgId, http.StatusOK) + + analysisOrchestrator := analysis.NewAnalysisOrchestrator( + mockConfig, + mockHTTPClient, + analysis.WithLogger(&logger), + analysis.WithInstrumentor(mockInstrumentor), + analysis.WithTrackerFactory(mockTrackerFactory), + analysis.WithErrorReporter(mockErrorReporter), + ) + + // run method under test + result, status, err := analysisOrchestrator.RunLegacyTest( + context.Background(), + bundleHash, + shardKey, + limitToFiles, + severity, + ) + + require.NoError(t, err) + assert.NotNil(t, result) + assert.Equal(t, analysis.StatusComplete, status.Message) + assert.Equal(t, 100, status.Percentage) + assert.Equal(t, sarifResponse.Sarif.Version, result.Sarif.Version) + assert.Equal(t, sarifResponse.Status, result.Status) + assert.Equal(t, sarifResponse.Progress, result.Progress) +} + +func TestAnalysis_RunLegacyTest_InProgress(t *testing.T) { + bundleHash := "test-bundle-hash" + shardKey := "test-shard-key" + limitToFiles := []string{"file1.js"} + severity := 1 + orgId := "test-org-id" + + mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, _, mockTrackerFactory, logger := setupLegacy(t, nil, false, "", orgId) + + // Create expected sarif response for in-progress status + sarifResponse := sarif.SarifResponse{ + Type: "sarif", + Progress: 0.6, + Status: analysis.StatusAnalyzing, + Coverage: []sarif.SarifCoverage{}, + Timing: struct { + FetchingCode int `json:"fetchingCode"` + Queue int `json:"queue"` + Analysis int `json:"analysis"` + }{ + FetchingCode: 50, + Analysis: 200, + }, + } + + mockLegacyAnalysisResponse(t, mockHTTPClient, sarifResponse, bundleHash, orgId, http.StatusOK) + + analysisOrchestrator := analysis.NewAnalysisOrchestrator( + mockConfig, + mockHTTPClient, + analysis.WithLogger(&logger), + analysis.WithInstrumentor(mockInstrumentor), + analysis.WithTrackerFactory(mockTrackerFactory), + analysis.WithErrorReporter(mockErrorReporter), + ) + + // run method under test + result, status, err := analysisOrchestrator.RunLegacyTest( + context.Background(), + bundleHash, + shardKey, + limitToFiles, + severity, + ) + + require.NoError(t, err) + assert.Nil(t, result) // No result when not complete + assert.Equal(t, analysis.StatusAnalyzing, status.Message) + assert.Equal(t, 60, status.Percentage) // 0.6 * 100 +} + +func TestAnalysis_RunLegacyTest_Failed(t *testing.T) { + bundleHash := "test-bundle-hash" + shardKey := "" + limitToFiles := []string{} + severity := 0 + orgId := "test-org-id" + + mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, _, mockTrackerFactory, logger := setupLegacy(t, nil, false, "", orgId) + + // Create expected sarif response for failed status + sarifResponse := sarif.SarifResponse{ + Type: "sarif", + Progress: 0.0, + Status: analysis.StatusFailed, + } + + mockLegacyAnalysisResponse(t, mockHTTPClient, sarifResponse, bundleHash, orgId, http.StatusOK) + + analysisOrchestrator := analysis.NewAnalysisOrchestrator( + mockConfig, + mockHTTPClient, + analysis.WithLogger(&logger), + analysis.WithInstrumentor(mockInstrumentor), + analysis.WithTrackerFactory(mockTrackerFactory), + analysis.WithErrorReporter(mockErrorReporter), + ) + + // run method under test + result, status, err := analysisOrchestrator.RunLegacyTest( + context.Background(), + bundleHash, + shardKey, + limitToFiles, + severity, + ) + + require.Error(t, err) + assert.IsType(t, analysis.FailedError{}, err) + assert.Nil(t, result) + assert.Equal(t, analysis.StatusFailed, status.Message) +} + +func TestAnalysis_RunLegacyTest_EmptyStatus(t *testing.T) { + bundleHash := "test-bundle-hash" + shardKey := "" + limitToFiles := []string{} + severity := 0 + orgId := "test-org-id" + + mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, _, mockTrackerFactory, logger := setupLegacy(t, nil, false, "", orgId) + + // Create expected sarif response with empty status + sarifResponse := sarif.SarifResponse{ + Type: "sarif", + Progress: 0.0, + Status: "", // Empty status should be treated as error + } + + mockLegacyAnalysisResponse(t, mockHTTPClient, sarifResponse, bundleHash, orgId, http.StatusOK) + + analysisOrchestrator := analysis.NewAnalysisOrchestrator( + mockConfig, + mockHTTPClient, + analysis.WithLogger(&logger), + analysis.WithInstrumentor(mockInstrumentor), + analysis.WithTrackerFactory(mockTrackerFactory), + analysis.WithErrorReporter(mockErrorReporter), + ) + + // run method under test + result, status, err := analysisOrchestrator.RunLegacyTest( + context.Background(), + bundleHash, + shardKey, + limitToFiles, + severity, + ) + + require.Error(t, err) + assert.IsType(t, analysis.FailedError{}, err) + assert.Nil(t, result) + assert.Equal(t, analysis.StatusFailed, status.Message) +} + +func TestAnalysis_RunLegacyTest_HTTPError(t *testing.T) { + bundleHash := "test-bundle-hash" + shardKey := "" + limitToFiles := []string{} + severity := 0 + orgId := "test-org-id" + + mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, _, mockTrackerFactory, logger := setupLegacy(t, nil, false, "", orgId) + + // Mock HTTP error response - need to check if the mock gives an error first + expectedAnalysisUrl := fmt.Sprintf("http://localhost/analysis") + mockHTTPClient.EXPECT().Do(mock.MatchedBy(func(i interface{}) bool { + req := i.(*http.Request) + return req.URL.String() == expectedAnalysisUrl && req.Method == http.MethodPost + })).Times(1).Return(&http.Response{ + StatusCode: http.StatusInternalServerError, + Header: http.Header{ + "Content-Type": []string{"application/json"}, + }, + Body: io.NopCloser(bytes.NewReader([]byte("Internal Server Error"))), + }, nil) // No error from HTTP client, but bad status code + + analysisOrchestrator := analysis.NewAnalysisOrchestrator( + mockConfig, + mockHTTPClient, + analysis.WithLogger(&logger), + analysis.WithInstrumentor(mockInstrumentor), + analysis.WithTrackerFactory(mockTrackerFactory), + analysis.WithErrorReporter(mockErrorReporter), + ) + + // run method under test + result, status, err := analysisOrchestrator.RunLegacyTest( + context.Background(), + bundleHash, + shardKey, + limitToFiles, + severity, + ) + + require.Error(t, err) + assert.IsType(t, analysis.FailedError{}, err) + assert.Nil(t, result) + assert.Equal(t, analysis.StatusFailed, status.Message) +} + +func TestAnalysis_RunLegacyTest_Fedramp(t *testing.T) { + bundleHash := "test-bundle-hash" + shardKey := "test-shard-key" + limitToFiles := []string{"file1.js"} + severity := 2 + orgId := "test-org-id" + + // Test with fedramp configuration + mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, _, mockTrackerFactory, logger := setupLegacy(t, nil, true, "https://deeproxy.snyk.io", orgId) + + // Create expected sarif response + sarifResponse := sarif.SarifResponse{ + Type: "sarif", + Progress: 1.0, + Status: analysis.StatusComplete, + Sarif: sarif.SarifDocument{ + Version: "2.1.0", + }, + } + + // Expect the fedramp URL transformation + expectedAnalysisUrl := fmt.Sprintf("https://api.snyk.io/hidden/orgs/%s/code/analysis", orgId) + mockHTTPClient.EXPECT().Do(mock.MatchedBy(func(i interface{}) bool { + req := i.(*http.Request) + return req.URL.String() == expectedAnalysisUrl && req.Method == http.MethodPost + })).Times(1).Return(&http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{ + "Content-Type": []string{"application/json"}, + }, + Body: io.NopCloser(bytes.NewReader(func() []byte { + responseBodyBytes, _ := json.Marshal(sarifResponse) + return responseBodyBytes + }())), + }, nil) + + analysisOrchestrator := analysis.NewAnalysisOrchestrator( + mockConfig, + mockHTTPClient, + analysis.WithLogger(&logger), + analysis.WithInstrumentor(mockInstrumentor), + analysis.WithTrackerFactory(mockTrackerFactory), + analysis.WithErrorReporter(mockErrorReporter), + ) + + // run method under test + result, status, err := analysisOrchestrator.RunLegacyTest( + context.Background(), + bundleHash, + shardKey, + limitToFiles, + severity, + ) + + require.NoError(t, err) + assert.NotNil(t, result) + assert.Equal(t, analysis.StatusComplete, status.Message) + assert.Equal(t, 100, status.Percentage) +} + +func TestAnalysis_RunLegacyTest_FedrampNoOrg(t *testing.T) { + bundleHash := "test-bundle-hash" + shardKey := "" + limitToFiles := []string{} + severity := 0 + orgId := "" // Empty org ID should cause error in fedramp + + mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, _, mockTrackerFactory, logger := setupLegacy(t, nil, true, "https://deeproxy.snyk.io", orgId) + + analysisOrchestrator := analysis.NewAnalysisOrchestrator( + mockConfig, + mockHTTPClient, + analysis.WithLogger(&logger), + analysis.WithInstrumentor(mockInstrumentor), + analysis.WithTrackerFactory(mockTrackerFactory), + analysis.WithErrorReporter(mockErrorReporter), + ) + + // run method under test + result, status, err := analysisOrchestrator.RunLegacyTest( + context.Background(), + bundleHash, + shardKey, + limitToFiles, + severity, + ) + + require.Error(t, err) + assert.Contains(t, err.Error(), "organization is required in a fedramp environment") + assert.Nil(t, result) + assert.Equal(t, scan.LegacyScanStatus{}, status) +} + +func TestAnalysis_RunLegacyTest_MalformedJSON(t *testing.T) { + bundleHash := "test-bundle-hash" + shardKey := "" + limitToFiles := []string{} + severity := 0 + orgId := "test-org-id" + + mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, _, mockTrackerFactory, logger := setupLegacy(t, nil, false, "", orgId) + + // Mock response with malformed JSON + expectedAnalysisUrl := fmt.Sprintf("http://localhost/analysis") + mockHTTPClient.EXPECT().Do(mock.MatchedBy(func(i interface{}) bool { + req := i.(*http.Request) + return req.URL.String() == expectedAnalysisUrl && req.Method == http.MethodPost + })).Times(1).Return(&http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{ + "Content-Type": []string{"application/json"}, + }, + Body: io.NopCloser(bytes.NewReader([]byte("invalid json{}"))), + }, nil) + + analysisOrchestrator := analysis.NewAnalysisOrchestrator( + mockConfig, + mockHTTPClient, + analysis.WithLogger(&logger), + analysis.WithInstrumentor(mockInstrumentor), + analysis.WithTrackerFactory(mockTrackerFactory), + analysis.WithErrorReporter(mockErrorReporter), + ) + + // run method under test + result, status, err := analysisOrchestrator.RunLegacyTest( + context.Background(), + bundleHash, + shardKey, + limitToFiles, + severity, + ) + + require.Error(t, err) + assert.Nil(t, result) + assert.Equal(t, analysis.StatusFailed, status.Message) +} + +func TestFailedError_Error(t *testing.T) { + err := analysis.FailedError{Msg: "test error message"} + assert.Equal(t, "test error message", err.Error()) +} + +// TestHelperFunctions tests the unexported helper functions through reflection or by testing them indirectly +func TestAnalysis_CreateRequestBody(t *testing.T) { + // Since createRequestBody is unexported, we test it indirectly by examining the request made in RunLegacyTest + bundleHash := "test-bundle-hash" + shardKey := "test-shard-key" + limitToFiles := []string{"file1.js", "file2.js"} + severity := 2 + orgId := "test-org-id" + + mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, _, mockTrackerFactory, logger := setupLegacy(t, nil, false, "", orgId) + + var capturedRequestBody []byte + expectedAnalysisUrl := fmt.Sprintf("http://localhost/analysis") + mockHTTPClient.EXPECT().Do(mock.MatchedBy(func(i interface{}) bool { + req := i.(*http.Request) + if req.URL.String() == expectedAnalysisUrl && req.Method == http.MethodPost { + // Capture the request body for validation + body, _ := io.ReadAll(req.Body) + capturedRequestBody = body + // Reset the body for the actual request + req.Body = io.NopCloser(bytes.NewReader(body)) + return true + } + return false + })).Times(1).Return(&http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{ + "Content-Type": []string{"application/json"}, + }, + Body: io.NopCloser(bytes.NewReader([]byte(`{"type":"sarif","progress":1.0,"status":"COMPLETE"}`))), + }, nil) + + analysisOrchestrator := analysis.NewAnalysisOrchestrator( + mockConfig, + mockHTTPClient, + analysis.WithLogger(&logger), + analysis.WithInstrumentor(mockInstrumentor), + analysis.WithTrackerFactory(mockTrackerFactory), + analysis.WithErrorReporter(mockErrorReporter), + ) + + // run method under test + _, _, err := analysisOrchestrator.RunLegacyTest( + context.Background(), + bundleHash, + shardKey, + limitToFiles, + severity, + ) + + require.NoError(t, err) + assert.NotEmpty(t, capturedRequestBody) + + // Parse and validate the request body structure + var request map[string]interface{} + err = json.Unmarshal(capturedRequestBody, &request) + require.NoError(t, err) + + // Validate request structure + assert.Equal(t, false, request["legacy"]) + assert.Contains(t, request, "key") + assert.Contains(t, request, "severity") + assert.Contains(t, request, "analysisContext") + + // Validate key structure + key := request["key"].(map[string]interface{}) + assert.Equal(t, "file", key["type"]) + assert.Equal(t, bundleHash, key["hash"]) + assert.Equal(t, shardKey, key["shard"]) + + // Validate limitToFiles + limitToFilesInterface := key["limitToFiles"].([]interface{}) + assert.Len(t, limitToFilesInterface, 2) + assert.Equal(t, "file1.js", limitToFilesInterface[0]) + assert.Equal(t, "file2.js", limitToFilesInterface[1]) + + // Validate severity + assert.Equal(t, float64(severity), request["severity"]) + + // Validate analysisContext + analysisContext := request["analysisContext"].(map[string]interface{}) + assert.Equal(t, "IDE", analysisContext["initiator"]) + assert.Equal(t, "language-server", analysisContext["flow"]) + + org := analysisContext["org"].(map[string]interface{}) + assert.Equal(t, orgId, org["publicId"]) + assert.Equal(t, "unknown", org["name"]) + assert.Equal(t, "unknown", org["displayName"]) +} + +func TestAnalysis_CreateRequestBody_NoShardKey(t *testing.T) { + bundleHash := "test-bundle-hash" + shardKey := "" // Empty shard key + limitToFiles := []string{} + severity := 0 + orgId := "test-org-id" + + mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, _, mockTrackerFactory, logger := setupLegacy(t, nil, false, "", orgId) + + var capturedRequestBody []byte + expectedAnalysisUrl := fmt.Sprintf("http://localhost/analysis") + mockHTTPClient.EXPECT().Do(mock.MatchedBy(func(i interface{}) bool { + req := i.(*http.Request) + if req.URL.String() == expectedAnalysisUrl && req.Method == http.MethodPost { + body, _ := io.ReadAll(req.Body) + capturedRequestBody = body + req.Body = io.NopCloser(bytes.NewReader(body)) + return true + } + return false + })).Times(1).Return(&http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{ + "Content-Type": []string{"application/json"}, + }, + Body: io.NopCloser(bytes.NewReader([]byte(`{"type":"sarif","progress":1.0,"status":"COMPLETE"}`))), + }, nil) + + analysisOrchestrator := analysis.NewAnalysisOrchestrator( + mockConfig, + mockHTTPClient, + analysis.WithLogger(&logger), + analysis.WithInstrumentor(mockInstrumentor), + analysis.WithTrackerFactory(mockTrackerFactory), + analysis.WithErrorReporter(mockErrorReporter), + ) + + // run method under test + _, _, err := analysisOrchestrator.RunLegacyTest( + context.Background(), + bundleHash, + shardKey, + limitToFiles, + severity, + ) + + require.NoError(t, err) + + // Parse and validate the request body + var request map[string]interface{} + err = json.Unmarshal(capturedRequestBody, &request) + require.NoError(t, err) + + key := request["key"].(map[string]interface{}) + // When shardKey is empty, it should still be included but as empty string (since shard field doesn't have omitempty) + assert.Contains(t, key, "shard") + assert.Equal(t, "", key["shard"]) + + // When severity is 0, it should not be included in the request + assert.NotContains(t, request, "severity") +} + +func TestAnalysis_GetCodeApiUrl_Regular(t *testing.T) { + // Test regular (non-fedramp) URL + orgId := "test-org-id" + mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, _, mockTrackerFactory, logger := setupLegacy(t, nil, false, "https://api.snyk.io", orgId) + + // We can't directly test getCodeApiUrl since it's unexported, but we can verify the URL used in requests + expectedAnalysisUrl := "https://api.snyk.io/analysis" + mockHTTPClient.EXPECT().Do(mock.MatchedBy(func(i interface{}) bool { + req := i.(*http.Request) + return req.URL.String() == expectedAnalysisUrl + })).Times(1).Return(&http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{ + "Content-Type": []string{"application/json"}, + }, + Body: io.NopCloser(bytes.NewReader([]byte(`{"type":"sarif","progress":1.0,"status":"COMPLETE"}`))), + }, nil) + + analysisOrchestrator := analysis.NewAnalysisOrchestrator( + mockConfig, + mockHTTPClient, + analysis.WithLogger(&logger), + analysis.WithInstrumentor(mockInstrumentor), + analysis.WithTrackerFactory(mockTrackerFactory), + analysis.WithErrorReporter(mockErrorReporter), + ) + + _, _, err := analysisOrchestrator.RunLegacyTest(context.Background(), "hash", "", []string{}, 0) + require.NoError(t, err) +} + +func TestAnalysis_GetCodeApiUrl_Fedramp(t *testing.T) { + // Test fedramp URL transformation + orgId := "test-org-id" + mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, _, mockTrackerFactory, logger := setupLegacy(t, nil, true, "https://deeproxy.snyk.io", orgId) + + // Verify the fedramp URL transformation: deeproxy -> api and adds org path + expectedAnalysisUrl := fmt.Sprintf("https://api.snyk.io/hidden/orgs/%s/code/analysis", orgId) + mockHTTPClient.EXPECT().Do(mock.MatchedBy(func(i interface{}) bool { + req := i.(*http.Request) + return req.URL.String() == expectedAnalysisUrl + })).Times(1).Return(&http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{ + "Content-Type": []string{"application/json"}, + }, + Body: io.NopCloser(bytes.NewReader([]byte(`{"type":"sarif","progress":1.0,"status":"COMPLETE"}`))), + }, nil) + + analysisOrchestrator := analysis.NewAnalysisOrchestrator( + mockConfig, + mockHTTPClient, + analysis.WithLogger(&logger), + analysis.WithInstrumentor(mockInstrumentor), + analysis.WithTrackerFactory(mockTrackerFactory), + analysis.WithErrorReporter(mockErrorReporter), + ) + + _, _, err := analysisOrchestrator.RunLegacyTest(context.Background(), "hash", "", []string{}, 0) + require.NoError(t, err) +} + +func TestAnalysis_GetCodeApiUrl_InvalidURL(t *testing.T) { + // Test with malformed URL in fedramp mode + orgId := "test-org-id" + mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, _, mockTrackerFactory, logger := setupLegacy(t, nil, true, ":::invalid-url", orgId) + + analysisOrchestrator := analysis.NewAnalysisOrchestrator( + mockConfig, + mockHTTPClient, + analysis.WithLogger(&logger), + analysis.WithInstrumentor(mockInstrumentor), + analysis.WithTrackerFactory(mockTrackerFactory), + analysis.WithErrorReporter(mockErrorReporter), + ) + + _, _, err := analysisOrchestrator.RunLegacyTest(context.Background(), "hash", "", []string{}, 0) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid") +} From 27c009397a8ebd06da7059b0f47f00f8f4f2cd83 Mon Sep 17 00:00:00 2001 From: Andrew Robinson Hodges Date: Fri, 8 Aug 2025 16:35:57 +0100 Subject: [PATCH 09/12] chore: linting --- internal/analysis/analysis_legacy_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/analysis/analysis_legacy_test.go b/internal/analysis/analysis_legacy_test.go index f9aacd0..096dafe 100644 --- a/internal/analysis/analysis_legacy_test.go +++ b/internal/analysis/analysis_legacy_test.go @@ -44,7 +44,7 @@ func mockLegacyAnalysisResponse(t *testing.T, mockHTTPClient *httpmocks.MockHTTP t.Helper() responseBodyBytes, err := json.Marshal(sarifResponse) assert.NoError(t, err) - expectedAnalysisUrl := fmt.Sprintf("http://localhost/analysis") + expectedAnalysisUrl := "http://localhost/analysis" mockHTTPClient.EXPECT().Do(mock.MatchedBy(func(i interface{}) bool { req := i.(*http.Request) return req.URL.String() == expectedAnalysisUrl && req.Method == http.MethodPost @@ -309,7 +309,7 @@ func TestAnalysis_RunLegacyTest_HTTPError(t *testing.T) { mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, _, mockTrackerFactory, logger := setupLegacy(t, nil, false, "", orgId) // Mock HTTP error response - need to check if the mock gives an error first - expectedAnalysisUrl := fmt.Sprintf("http://localhost/analysis") + expectedAnalysisUrl := "http://localhost/analysis" mockHTTPClient.EXPECT().Do(mock.MatchedBy(func(i interface{}) bool { req := i.(*http.Request) return req.URL.String() == expectedAnalysisUrl && req.Method == http.MethodPost @@ -448,7 +448,7 @@ func TestAnalysis_RunLegacyTest_MalformedJSON(t *testing.T) { mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, _, mockTrackerFactory, logger := setupLegacy(t, nil, false, "", orgId) // Mock response with malformed JSON - expectedAnalysisUrl := fmt.Sprintf("http://localhost/analysis") + expectedAnalysisUrl := "http://localhost/analysis" mockHTTPClient.EXPECT().Do(mock.MatchedBy(func(i interface{}) bool { req := i.(*http.Request) return req.URL.String() == expectedAnalysisUrl && req.Method == http.MethodPost @@ -500,7 +500,7 @@ func TestAnalysis_CreateRequestBody(t *testing.T) { mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, _, mockTrackerFactory, logger := setupLegacy(t, nil, false, "", orgId) var capturedRequestBody []byte - expectedAnalysisUrl := fmt.Sprintf("http://localhost/analysis") + expectedAnalysisUrl := "http://localhost/analysis" mockHTTPClient.EXPECT().Do(mock.MatchedBy(func(i interface{}) bool { req := i.(*http.Request) if req.URL.String() == expectedAnalysisUrl && req.Method == http.MethodPost { @@ -588,7 +588,7 @@ func TestAnalysis_CreateRequestBody_NoShardKey(t *testing.T) { mockConfig, mockHTTPClient, mockInstrumentor, mockErrorReporter, _, mockTrackerFactory, logger := setupLegacy(t, nil, false, "", orgId) var capturedRequestBody []byte - expectedAnalysisUrl := fmt.Sprintf("http://localhost/analysis") + expectedAnalysisUrl := "http://localhost/analysis" mockHTTPClient.EXPECT().Do(mock.MatchedBy(func(i interface{}) bool { req := i.(*http.Request) if req.URL.String() == expectedAnalysisUrl && req.Method == http.MethodPost { From 678a7374829c1773f7b017f2c13ef661f61c3035 Mon Sep 17 00:00:00 2001 From: Andrew Robinson Hodges Date: Fri, 8 Aug 2025 16:59:55 +0100 Subject: [PATCH 10/12] chore: remove unused code --- scan.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/scan.go b/scan.go index f239271..b3b4bf2 100644 --- a/scan.go +++ b/scan.go @@ -20,10 +20,11 @@ package codeclient import ( "context" "fmt" + "time" + "github.com/google/uuid" "github.com/pkg/errors" "github.com/rs/zerolog" - "time" "github.com/snyk/code-client-go/bundle" "github.com/snyk/code-client-go/config" @@ -164,7 +165,6 @@ func NewCodeScanner( deepcodeClient := deepcode.NewDeepcodeClient(scanner.config, httpClient, scanner.logger, scanner.instrumentor, scanner.errorReporter) bundleManager := bundle.NewBundleManager(deepcodeClient, scanner.logger, scanner.instrumentor, scanner.errorReporter, scanner.trackerFactory) scanner.bundleManager = bundleManager - scanner.deepcodeClient = deepcodeClient analysisOrchestrator := analysis.NewAnalysisOrchestrator( scanner.config, httpClient, @@ -184,7 +184,6 @@ func NewCodeScanner( func (c *codeScanner) WithBundleManager(bundleManager bundle.BundleManager) *codeScanner { return &codeScanner{ bundleManager: bundleManager, - deepcodeClient: c.deepcodeClient, analysisOrchestrator: c.analysisOrchestrator, errorReporter: c.errorReporter, logger: c.logger, @@ -197,7 +196,6 @@ func (c *codeScanner) WithBundleManager(bundleManager bundle.BundleManager) *cod func (c *codeScanner) WithAnalysisOrchestrator(analysisOrchestrator analysis.AnalysisOrchestrator) *codeScanner { return &codeScanner{ bundleManager: c.bundleManager, - deepcodeClient: c.deepcodeClient, analysisOrchestrator: analysisOrchestrator, errorReporter: c.errorReporter, logger: c.logger, From c6a713fd933a29a1ca197f714d9ab08be84c4bfb Mon Sep 17 00:00:00 2001 From: Andrew Robinson Hodges Date: Fri, 8 Aug 2025 17:02:56 +0100 Subject: [PATCH 11/12] chore: remove unused code --- scan.go | 1 - 1 file changed, 1 deletion(-) diff --git a/scan.go b/scan.go index b3b4bf2..ef86b3e 100644 --- a/scan.go +++ b/scan.go @@ -40,7 +40,6 @@ import ( type codeScanner struct { httpClient codeClientHTTP.HTTPClient bundleManager bundle.BundleManager - deepcodeClient deepcode.DeepcodeClient analysisOrchestrator analysis.AnalysisOrchestrator instrumentor observability.Instrumentor errorReporter observability.ErrorReporter From 67d185f51348b62623a1520ccb4e1266bcd6f04b Mon Sep 17 00:00:00 2001 From: Andrew Robinson Hodges Date: Mon, 11 Aug 2025 11:58:07 +0100 Subject: [PATCH 12/12] fix: code scanner - move channel closure to defer --- scan.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scan.go b/scan.go index ef86b3e..bf762b3 100644 --- a/scan.go +++ b/scan.go @@ -267,6 +267,7 @@ func (c *codeScanner) UploadAndAnalyzeLegacy( changedFiles map[string]bool, statusChannel chan<- scan.LegacyScanStatus, ) (*sarif.SarifResponse, string, error) { + defer close(statusChannel) uploadedBundle, err := c.Upload(ctx, requestId, target, files, changedFiles) if err != nil || uploadedBundle == nil || uploadedBundle.GetBundleHash() == "" { c.logger.Debug().Msg("empty bundle, no Snyk Code analysis") @@ -274,7 +275,6 @@ func (c *codeScanner) UploadAndAnalyzeLegacy( } response, bundleHash, err := c.analyzeLegacy(ctx, uploadedBundle, shardKey, statusChannel) - close(statusChannel) return response, bundleHash, err }