From f6fc5f7ed643fbb78826cb95104057c9662664b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20Scha=CC=88fer?= <101886095+PeterSchafer@users.noreply.github.com> Date: Mon, 9 Dec 2024 15:52:36 +0100 Subject: [PATCH] fix: improved error messages around network requests --- cliv2/cmd/cliv2/configuration.go | 8 +- cliv2/cmd/cliv2/instrumentation.go | 3 + cliv2/cmd/cliv2/main.go | 37 ++++-- cliv2/go.mod | 2 +- cliv2/go.sum | 4 +- cliv2/internal/cliv2/cliv2.go | 2 +- cliv2/internal/constants/constants.go | 1 + cliv2/internal/proxy/proxy.go | 39 ++++-- cliv2/internal/proxy/proxy_test.go | 66 ++++++++++ cliv2/pkg/basic_workflows/legacycli.go | 1 + src/cli/exit-codes.ts | 1 + src/lib/snyk-test/run-test.ts | 6 + test/jest/acceptance/error-catalog.spec.ts | 117 ++++++++++++++++++ .../acceptance/snyk-code/snyk-code.spec.ts | 2 +- 14 files changed, 265 insertions(+), 24 deletions(-) create mode 100644 test/jest/acceptance/error-catalog.spec.ts diff --git a/cliv2/cmd/cliv2/configuration.go b/cliv2/cmd/cliv2/configuration.go index 98c9310698..1195c43251 100644 --- a/cliv2/cmd/cliv2/configuration.go +++ b/cliv2/cmd/cliv2/configuration.go @@ -11,9 +11,9 @@ import ( ) func defaultOAuthFF(config configuration.Configuration) configuration.DefaultValueFunction { - return func(existingValue interface{}) interface{} { + return func(existingValue interface{}) (interface{}, error) { if _, ok := os.LookupEnv(auth.CONFIG_KEY_OAUTH_TOKEN); ok { - return true + return true, nil } keysThatMightDisableOAuth := config.GetAllKeysThatContainValues(configuration.AUTHENTICATION_BEARER_TOKEN) @@ -23,10 +23,10 @@ func defaultOAuthFF(config configuration.Configuration) configuration.DefaultVal for _, key := range keysThatMightDisableOAuth { keyType := config.GetKeyType(key) if keyType == configuration.EnvVarKeyType { - return false + return false, nil } } - return true + return true, nil } } diff --git a/cliv2/cmd/cliv2/instrumentation.go b/cliv2/cmd/cliv2/instrumentation.go index cb77cfca5a..3e55943c88 100644 --- a/cliv2/cmd/cliv2/instrumentation.go +++ b/cliv2/cmd/cliv2/instrumentation.go @@ -1,5 +1,8 @@ package main +// !!! This import needs to be the first import, please do not change this !!! +import _ "github.com/snyk/go-application-framework/pkg/networking/fips_enable" + import ( "strings" diff --git a/cliv2/cmd/cliv2/main.go b/cliv2/cmd/cliv2/main.go index c66705a017..0a02fb75c3 100644 --- a/cliv2/cmd/cliv2/main.go +++ b/cliv2/cmd/cliv2/main.go @@ -12,6 +12,7 @@ import ( "os" "os/exec" "strings" + "sync" "time" "github.com/google/uuid" @@ -19,21 +20,18 @@ import ( "github.com/snyk/cli-extension-dep-graph/pkg/depgraph" "github.com/snyk/cli-extension-iac-rules/iacrules" "github.com/snyk/cli-extension-sbom/pkg/sbom" + "github.com/snyk/cli/cliv2/internal/cliv2" + "github.com/snyk/cli/cliv2/internal/constants" "github.com/snyk/container-cli/pkg/container" "github.com/snyk/go-application-framework/pkg/analytics" "github.com/snyk/go-application-framework/pkg/app" "github.com/snyk/go-application-framework/pkg/configuration" "github.com/snyk/go-application-framework/pkg/instrumentation" + "github.com/snyk/go-application-framework/pkg/local_workflows/network_utils" + "github.com/snyk/go-application-framework/pkg/local_workflows/output_workflow" "github.com/spf13/cobra" "github.com/spf13/pflag" - "github.com/snyk/cli/cliv2/internal/cliv2" - "github.com/snyk/cli/cliv2/internal/constants" - - "github.com/snyk/go-application-framework/pkg/local_workflows/output_workflow" - - "github.com/snyk/go-application-framework/pkg/local_workflows/network_utils" - localworkflows "github.com/snyk/go-application-framework/pkg/local_workflows" "github.com/snyk/go-application-framework/pkg/local_workflows/content_type" "github.com/snyk/go-application-framework/pkg/local_workflows/json_schemas" @@ -541,9 +539,19 @@ func MainWithErrorCode() int { // add workflows as commands createCommandsForWorkflows(rootCommand, globalEngine) + errorList := []error{} + errorListMutex := sync.Mutex{} + // init NetworkAccess ua := networking.UserAgent(networking.UaWithConfig(globalConfiguration), networking.UaWithRuntimeInfo(rInfo), networking.UaWithOS(internalOS)) networkAccess := globalEngine.GetNetworkAccess() + networkAccess.AddErrorHandler(func(err error, ctx context.Context) error { + errorListMutex.Lock() + defer errorListMutex.Unlock() + + errorList = append(errorList, err) + return err + }) networkAccess.AddHeaderField("x-snyk-cli-version", cliv2.GetFullVersion()) networkAccess.AddHeaderField("snyk-interaction-id", instrumentation.AssembleUrnFromUUID(interactionId)) networkAccess.AddHeaderField( @@ -584,7 +592,13 @@ func MainWithErrorCode() int { } if err != nil { + for _, tempError := range errorList { + cliAnalytics.AddError(tempError) + } + cliAnalytics.AddError(err) + + err = legacyCLITerminated(err, errorList) } displayError(err, globalEngine.GetUserInterface(), globalConfiguration) @@ -622,6 +636,15 @@ func MainWithErrorCode() int { return exitCode } +func legacyCLITerminated(err error, errorList []error) error { + exitErr, isExitError := err.(*exec.ExitError) + if isExitError && exitErr.ExitCode() == constants.SNYK_EXIT_CODE_TS_CLI_TERMINATED { + errorList = append([]error{err}, errorList...) + err = errors.Join(errorList...) + } + return err +} + func setTimeout(config configuration.Configuration, onTimeout func()) { timeout := config.GetInt(configuration.TIMEOUT) if timeout == 0 { diff --git a/cliv2/go.mod b/cliv2/go.mod index a4f1d4ec63..4050c2ca8d 100644 --- a/cliv2/go.mod +++ b/cliv2/go.mod @@ -17,7 +17,7 @@ require ( github.com/snyk/cli-extension-sbom v0.0.0-20241016065306-0df2be5b3b8f github.com/snyk/container-cli v0.0.0-20240821111304-7ca1c415a5d7 github.com/snyk/error-catalog-golang-public v0.0.0-20241030160523-0aa643bb7069 - github.com/snyk/go-application-framework v0.0.0-20241206091813-21505aec617a + github.com/snyk/go-application-framework v0.0.0-20241209184421-ca04d08658d4 github.com/snyk/go-httpauth v0.0.0-20240307114523-1f5ea3f55c65 github.com/snyk/snyk-iac-capture v0.6.5 github.com/snyk/snyk-ls v0.0.0-20241206144109-2533da8ba468 diff --git a/cliv2/go.sum b/cliv2/go.sum index 0376ab1afd..820cb5484c 100644 --- a/cliv2/go.sum +++ b/cliv2/go.sum @@ -760,8 +760,8 @@ github.com/snyk/container-cli v0.0.0-20240821111304-7ca1c415a5d7 h1:Zn5BcV76oFAb github.com/snyk/container-cli v0.0.0-20240821111304-7ca1c415a5d7/go.mod h1:38w+dcAQp9eG3P5t2eNS9eG0reut10AeJjLv5lJ5lpM= github.com/snyk/error-catalog-golang-public v0.0.0-20241030160523-0aa643bb7069 h1:Oj/BJAEMEuBjTAQ72UYB4tR0IZKOB2ZtdDnAnJDL1BM= github.com/snyk/error-catalog-golang-public v0.0.0-20241030160523-0aa643bb7069/go.mod h1:Ytttq7Pw4vOCu9NtRQaOeDU2dhBYUyNBe6kX4+nIIQ4= -github.com/snyk/go-application-framework v0.0.0-20241206091813-21505aec617a h1:6a7bVP4VO/ncy9It706Szm7BFN3jieHpWAyDvj/MNnA= -github.com/snyk/go-application-framework v0.0.0-20241206091813-21505aec617a/go.mod h1:Gz5Hqztenx4KDyaSu6IXryLVLWeT2g+oYnp7Tk41t5U= +github.com/snyk/go-application-framework v0.0.0-20241209184421-ca04d08658d4 h1:J61rW7l0xsv+Uho6vh83M6yOfB3mxuu4dJsBttg89UU= +github.com/snyk/go-application-framework v0.0.0-20241209184421-ca04d08658d4/go.mod h1:Gz5Hqztenx4KDyaSu6IXryLVLWeT2g+oYnp7Tk41t5U= github.com/snyk/go-httpauth v0.0.0-20240307114523-1f5ea3f55c65 h1:CEQuYv0Go6MEyRCD3YjLYM2u3Oxkx8GpCpFBd4rUTUk= github.com/snyk/go-httpauth v0.0.0-20240307114523-1f5ea3f55c65/go.mod h1:88KbbvGYlmLgee4OcQ19yr0bNpXpOr2kciOthaSzCAg= github.com/snyk/policy-engine v0.31.3 h1:FepCg6QN/X8uvxYjF+WwB2aiBPJB+NENDgKQeI/FwLg= diff --git a/cliv2/internal/cliv2/cliv2.go b/cliv2/internal/cliv2/cliv2.go index 5e03660d79..3a6bc13433 100644 --- a/cliv2/internal/cliv2/cliv2.go +++ b/cliv2/internal/cliv2/cliv2.go @@ -476,7 +476,7 @@ func DeriveExitCode(err error) int { if errors.As(err, &exitError) { returnCode = exitError.ExitCode() // map errors in subprocesses to exit code 2 to remain the documented exit code range - if returnCode < 0 { + if returnCode < 0 || returnCode == constants.SNYK_EXIT_CODE_TS_CLI_TERMINATED { returnCode = constants.SNYK_EXIT_CODE_ERROR } } else if errors.Is(err, context.DeadlineExceeded) { diff --git a/cliv2/internal/constants/constants.go b/cliv2/internal/constants/constants.go index 185ad09b35..b84b8cd4ee 100644 --- a/cliv2/internal/constants/constants.go +++ b/cliv2/internal/constants/constants.go @@ -5,6 +5,7 @@ const SNYK_EXIT_CODE_VULNERABILITIES_FOUND = 1 const SNYK_EXIT_CODE_ERROR = 2 const SNYK_EXIT_CODE_UNSUPPORTED_PROJECTS = 3 const SNYK_EXIT_CODE_EX_UNAVAILABLE = 69 +const SNYK_EXIT_CODE_TS_CLI_TERMINATED = 44 const SNYK_INTEGRATION_NAME = "CLI_V1_PLUGIN" const SNYK_INTEGRATION_NAME_ENV = "SNYK_INTEGRATION_NAME" const SNYK_INTEGRATION_VERSION_ENV = "SNYK_INTEGRATION_VERSION" diff --git a/cliv2/internal/proxy/proxy.go b/cliv2/internal/proxy/proxy.go index 9d96a49299..81f381ddad 100644 --- a/cliv2/internal/proxy/proxy.go +++ b/cliv2/internal/proxy/proxy.go @@ -20,6 +20,7 @@ import ( pkg_utils "github.com/snyk/go-application-framework/pkg/utils" "github.com/snyk/go-application-framework/pkg/networking/middleware" + networktypes "github.com/snyk/go-application-framework/pkg/networking/network_types" "github.com/snyk/go-httpauth/pkg/httpauth" "github.com/elazarl/goproxy" @@ -42,6 +43,8 @@ type WrapperProxy struct { proxyUsername string proxyPassword string addHeaderFunc func(*http.Request) error + config configuration.Configuration + errHandlerFunc networktypes.ErrorHandlerFunc } type ProxyInfo struct { @@ -136,6 +139,7 @@ func NewWrapperProxy(config configuration.Configuration, cliVersion string, debu p.addHeaderFunc = func(request *http.Request) error { return nil } p.DebugLogger = debugLogger p.CertificateLocation = ca.CertFile + p.config = config insecureSkipVerify := config.GetBool(configuration.INSECURE_HTTPS) @@ -179,6 +183,9 @@ func (p *WrapperProxy) ProxyInfo() *ProxyInfo { // request might 401 or 403, such as permissions or entitlements. const headerSnykAuthFailed = "snyk-auth-failed" +// Header to signal that the typescript CLI should terminate execution. +const headerSnykTerminate = "snyk-terminate" + func (p *WrapperProxy) replaceVersionHandler(r *http.Request, ctx *goproxy.ProxyCtx) (*http.Request, *http.Response) { if err := p.addHeaderFunc(r); err != nil { if errors.Is(err, middleware.ErrAuthenticationFailed) { @@ -192,6 +199,25 @@ func (p *WrapperProxy) replaceVersionHandler(r *http.Request, ctx *goproxy.Proxy return r, nil } +func (p *WrapperProxy) handleResponse(resp *http.Response, ctx *goproxy.ProxyCtx) *http.Response { + networking.LogResponse(resp, p.DebugLogger) + + if authFailed := resp.Request.Header.Get(headerSnykAuthFailed); authFailed != "" { + resp.Header.Set(headerSnykAuthFailed, authFailed) + } + + err := middleware.HandleResponse(resp, p.config) + if err == nil { + return resp + } + + if p.errHandlerFunc != nil && p.errHandlerFunc(err, resp.Request.Context()) != nil { + resp.Header.Set(headerSnykTerminate, "true") + } + + return resp +} + func (p *WrapperProxy) checkBasicCredentials(user, password string) bool { return user == p.proxyUsername && p.proxyPassword == password } @@ -215,14 +241,7 @@ func (p *WrapperProxy) Start() error { proxy.Logger = log.New(&pkg_utils.ToZeroLogDebug{Logger: p.DebugLogger}, "", 0) proxy.OnRequest().DoFunc(p.replaceVersionHandler) proxy.OnRequest().HandleConnect(p) - proxy.OnResponse().DoFunc(func(resp *http.Response, ctx *goproxy.ProxyCtx) *http.Response { - networking.LogResponse(resp, p.DebugLogger) - - if authFailed := resp.Request.Header.Get(headerSnykAuthFailed); authFailed != "" { - resp.Header.Set(headerSnykAuthFailed, authFailed) - } - return resp - }) + proxy.OnResponse().DoFunc(p.handleResponse) proxy.Verbose = true proxyServer := &http.Server{ Handler: proxy, @@ -322,3 +341,7 @@ func (p *WrapperProxy) Transport() *http.Transport { func (p *WrapperProxy) SetHeaderFunction(addHeaderFunc func(*http.Request) error) { p.addHeaderFunc = addHeaderFunc } + +func (p *WrapperProxy) SetErrorHandlerFunction(errHandler networktypes.ErrorHandlerFunc) { + p.errHandlerFunc = errHandler +} diff --git a/cliv2/internal/proxy/proxy_test.go b/cliv2/internal/proxy/proxy_test.go index e56a4aee1b..bb4161d82d 100644 --- a/cliv2/internal/proxy/proxy_test.go +++ b/cliv2/internal/proxy/proxy_test.go @@ -1,11 +1,13 @@ package proxy_test import ( + "context" "crypto/tls" "crypto/x509" "fmt" "log" "net/http" + "net/http/httptest" "net/url" "os" "testing" @@ -275,3 +277,67 @@ func Test_proxyPropagatesAuthFailureHeader(t *testing.T) { wp.Close() } + +func Test_proxyWithErrorHandler(t *testing.T) { + basecache := "testcache" + version := "1.1.1" + + config := setup(t, basecache, version) + config.Set(configuration.INSECURE_HTTPS, false) + defer teardown(t, basecache) + + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + }) + server := httptest.NewServer(handler) + defer server.Close() + + t.Run("intercepts traffic based on configuration", func(t *testing.T) { + config.Set(configuration.API_URL, server.URL) + wp, err := proxy.NewWrapperProxy(config, version, &debugLogger, caData) + assert.Nil(t, err) + + // register err handler for the proxy + wp.SetErrorHandlerFunction(func(err error, ctx context.Context) error { + return err + }) + + err = wp.Start() + defer wp.Close() + assert.Nil(t, err) + + useProxyAuth := true + proxiedClient, err := helper_getHttpClient(wp, useProxyAuth) + assert.Nil(t, err) + + res, err := proxiedClient.Get(server.URL) + assert.NotNil(t, res) + assert.Equal(t, res.Header.Get("snyk-terminate"), "true") + assert.Nil(t, err) + }) + + t.Run("does not intercept external traffic", func(t *testing.T) { + // the local server is not in the configuration, so it's not intercepted + config.Set(configuration.API_URL, "http://api.snyk.io") + wp, err := proxy.NewWrapperProxy(config, version, &debugLogger, caData) + assert.Nil(t, err) + + // register err handler for the proxy + wp.SetErrorHandlerFunction(func(err error, ctx context.Context) error { + return err + }) + + err = wp.Start() + defer wp.Close() + assert.Nil(t, err) + + useProxyAuth := true + proxiedClient, err := helper_getHttpClient(wp, useProxyAuth) + assert.Nil(t, err) + + res, err := proxiedClient.Get(server.URL) + assert.NotNil(t, res) + assert.Empty(t, res.Header.Get("snyk-terminate")) + assert.Nil(t, err) + }) +} diff --git a/cliv2/pkg/basic_workflows/legacycli.go b/cliv2/pkg/basic_workflows/legacycli.go index 6eb3ac0136..bae4e7e1f9 100644 --- a/cliv2/pkg/basic_workflows/legacycli.go +++ b/cliv2/pkg/basic_workflows/legacycli.go @@ -175,6 +175,7 @@ func createInternalProxy(config configuration.Configuration, debugLogger *zerolo return headersErr } wrapperProxy.SetHeaderFunction(proxyHeaderFunc) + wrapperProxy.SetErrorHandlerFunction(networkAccess.GetErrorHandler()) err = wrapperProxy.Start() if err != nil { diff --git a/src/cli/exit-codes.ts b/src/cli/exit-codes.ts index f762e35e6c..699001c75d 100644 --- a/src/cli/exit-codes.ts +++ b/src/cli/exit-codes.ts @@ -4,4 +4,5 @@ export const EXIT_CODES = { NO_SUPPORTED_PROJECTS_DETECTED: 3, EX_UNAVAILABLE: 69, EX_NOPERM: 77, + EX_TERMINATE: 44, }; diff --git a/src/lib/snyk-test/run-test.ts b/src/lib/snyk-test/run-test.ts index 4db89f492b..b23bd77ed2 100644 --- a/src/lib/snyk-test/run-test.ts +++ b/src/lib/snyk-test/run-test.ts @@ -86,6 +86,7 @@ import { } from '../package-managers'; import { PackageExpanded } from 'snyk-resolve-deps/dist/types'; import { normalizeTargetFile } from '../normalize-target-file'; +import { EXIT_CODES } from '../../cli/exit-codes'; const debug = debugModule('snyk:run-test'); @@ -531,6 +532,11 @@ function sendTestPayload( if (error) { return reject(error); } + + if (res?.headers?.['snyk-terminate']) { + process.exit(EXIT_CODES.EX_TERMINATE); + } + if (res.statusCode !== 200) { const err = handleTestHttpErrorResponse(res, body); debug('sendTestPayload request URL:', payload.url); diff --git a/test/jest/acceptance/error-catalog.spec.ts b/test/jest/acceptance/error-catalog.spec.ts new file mode 100644 index 0000000000..a0d3ff8933 --- /dev/null +++ b/test/jest/acceptance/error-catalog.spec.ts @@ -0,0 +1,117 @@ +import { runSnykCLI } from '../util/runSnykCLI'; +import { fakeServer, getFirstIPv4Address } from '../../acceptance/fake-server'; +import { getServerPort } from '../util/getServerPort'; + +interface Workflow { + type: string; + cmd: string; +} + +const integrationWorkflows: Workflow[] = [ + { + type: 'typescript', + cmd: 'test', + }, + { + type: 'golang/native', + cmd: 'code test', + }, +]; + +describe.each(integrationWorkflows)( + 'outputs Error Catalog errors', + ({ cmd, type }) => { + const snykOrg = '11111111-2222-3333-4444-555555555555'; + let env: any = { + ...process.env, + }; + + describe('authentication errors', () => { + describe(`${type} workflow`, () => { + it(`snyk ${cmd}`, async () => { + const { code, stdout } = await runSnykCLI(cmd, { + env: { + ...env, + SNYK_TOKEN: '1234', + }, + }); + + expect(code).toBe(2); + expect(stdout).toContain('Authentication error (SNYK-0005)'); + }); + }); + }); + + describe('other network errors', () => { + let server: ReturnType; + const ipAddr = getFirstIPv4Address(); + const port = getServerPort(process); + const baseApi = '/api/v1'; + beforeAll((done) => { + env = { + ...env, + SNYK_API: 'http://' + ipAddr + ':' + port + baseApi, + SNYK_HOST: 'http://' + ipAddr + ':' + port, + SNYK_TOKEN: '123456789', + SNYK_HTTP_PROTOCOL_UPGRADE: '0', + SNYK_CFG_ORG: snykOrg, + }; + server = fakeServer(baseApi, 'snykToken'); + server.listen(port, () => { + done(); + }); + }); + afterEach(() => { + server.restore(); + }); + afterAll((done) => { + server.close(() => { + done(); + }); + }); + + describe('internal server errors', () => { + describe(`${type} workflow`, () => { + it(`snyk ${cmd}`, async () => { + server.setStatusCode(500); + const { code } = await runSnykCLI(`${cmd}`, { env }); + expect(code).toBe(2); + const analyticsRequest = server + .getRequests() + .filter((value) => + (value.url as string).includes( + `/api/hidden/orgs/${snykOrg}/analytics`, + ), + ) + .pop(); + expect( + analyticsRequest?.body.data.attributes.interaction.errors[0].code, + ).toEqual('500'); + }); + }); + }); + + describe('bad request errors', () => { + describe(`${type} workflow`, () => { + it(`snyk ${cmd}`, async () => { + server.setStatusCode(400); + const { code } = await runSnykCLI(`${cmd}`, { env }); + expect(code).toBe(2); + + const analyticsRequest = server + .getRequests() + .filter((value) => + (value.url as string).includes( + `/api/hidden/orgs/${snykOrg}/analytics`, + ), + ) + .pop(); + expect( + analyticsRequest?.body.data.attributes.interaction.errors[0].code, + ).toEqual('400'); + }); + }); + }); + }); + }, +); diff --git a/test/jest/acceptance/snyk-code/snyk-code.spec.ts b/test/jest/acceptance/snyk-code/snyk-code.spec.ts index 9a2d01eb61..bf9df7a2c5 100644 --- a/test/jest/acceptance/snyk-code/snyk-code.spec.ts +++ b/test/jest/acceptance/snyk-code/snyk-code.spec.ts @@ -116,7 +116,7 @@ describe('snyk code test', () => { ); expect(stderr).toBe(''); - expect(stdout).toContain('Snyk Code is not supported for org'); + expect(stdout).toContain('Snyk Code is not enabled'); expect(code).toBe(EXIT_CODE_FAIL_WITH_ERROR); });