Skip to content

Commit

Permalink
fix: improved error messages around network requests
Browse files Browse the repository at this point in the history
  • Loading branch information
PeterSchafer authored and CatalinSnyk committed Dec 10, 2024
1 parent 1f8adf3 commit f6fc5f7
Show file tree
Hide file tree
Showing 14 changed files with 265 additions and 24 deletions.
8 changes: 4 additions & 4 deletions cliv2/cmd/cliv2/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
}
}
3 changes: 3 additions & 0 deletions cliv2/cmd/cliv2/instrumentation.go
Original file line number Diff line number Diff line change
@@ -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"

Expand Down
37 changes: 30 additions & 7 deletions cliv2/cmd/cliv2/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,26 @@ import (
"os"
"os/exec"
"strings"
"sync"
"time"

"github.com/google/uuid"
"github.com/rs/zerolog"
"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"
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion cliv2/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions cliv2/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
2 changes: 1 addition & 1 deletion cliv2/internal/cliv2/cliv2.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions cliv2/internal/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
39 changes: 31 additions & 8 deletions cliv2/internal/proxy/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 {
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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) {
Expand All @@ -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
}
Expand All @@ -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,
Expand Down Expand Up @@ -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
}
66 changes: 66 additions & 0 deletions cliv2/internal/proxy/proxy_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package proxy_test

import (
"context"
"crypto/tls"
"crypto/x509"
"fmt"
"log"
"net/http"
"net/http/httptest"
"net/url"
"os"
"testing"
Expand Down Expand Up @@ -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)
})
}
1 change: 1 addition & 0 deletions cliv2/pkg/basic_workflows/legacycli.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions src/cli/exit-codes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@ export const EXIT_CODES = {
NO_SUPPORTED_PROJECTS_DETECTED: 3,
EX_UNAVAILABLE: 69,
EX_NOPERM: 77,
EX_TERMINATE: 44,
};
6 changes: 6 additions & 0 deletions src/lib/snyk-test/run-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -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);
Expand Down
Loading

0 comments on commit f6fc5f7

Please sign in to comment.