fix: add timeout handling to legacy cli

cliv2/cmd/cliv2/main.go

@@ -4,14 +4,10 @@ package main
 import _ "github.com/snyk/go-application-framework/pkg/networking/fips_enable"
 
 import (
+	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
-	"io"
-	"os"
-	"os/exec"
-	"strings"
-	"time"
-
 	"github.com/rs/zerolog"
 	"github.com/snyk/cli-extension-dep-graph/pkg/depgraph"
 	"github.com/snyk/cli-extension-iac-rules/iacrules"
@@ -22,20 +18,26 @@ import (
 	"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/auth"
 	"github.com/snyk/go-application-framework/pkg/configuration"
-
 	localworkflows "github.com/snyk/go-application-framework/pkg/local_workflows"
 	"github.com/snyk/go-application-framework/pkg/networking"
 	"github.com/snyk/go-application-framework/pkg/runtimeinfo"
 	"github.com/snyk/go-application-framework/pkg/utils"
 	"github.com/snyk/go-application-framework/pkg/workflow"
-	"github.com/snyk/go-httpauth/pkg/httpauth"
 	"github.com/snyk/snyk-iac-capture/pkg/capture"
-
-	snykls "github.com/snyk/snyk-ls/ls_extension"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
+	"io"
+	"os"
+	"os/exec"
+	"strings"
+	"time"
+)
+
+import (
+	"github.com/snyk/go-application-framework/pkg/auth"
+	"github.com/snyk/go-httpauth/pkg/httpauth"
+	snykls "github.com/snyk/snyk-ls/ls_extension"
 )
 
 var internalOS string
@@ -355,7 +357,8 @@ func handleError(err error) HandleError {
 
 func displayError(err error) {
 	if err != nil {
-		if _, ok := err.(*exec.ExitError); !ok {
+		var exitError *exec.ExitError
+		if !errors.As(err, &exitError) {
 			if globalConfiguration.GetBool(localworkflows.OUTPUT_CONFIG_KEY_JSON) {
 				jsonError := JsonErrorStruct{
 					Ok:       false,
@@ -366,7 +369,11 @@ func displayError(err error) {
 				jsonErrorBuffer, _ := json.MarshalIndent(jsonError, "", "  ")
 				fmt.Println(string(jsonErrorBuffer))
 			} else {
-				fmt.Println(err)
+				if errors.Is(err, context.DeadlineExceeded) {
+					fmt.Println("command timed out")
+				} else {
+					fmt.Println(err)
+				}
 			}
 		}
 	}
@@ -479,8 +486,9 @@ func setTimeout(config configuration.Configuration, onTimeout func()) {
 	}
 	debugLogger.Printf("Command timeout set for %d seconds", timeout)
 	go func() {
-		<-time.After(time.Duration(timeout) * time.Second)
-		fmt.Fprintf(os.Stderr, "command timed out\n")
+		// we wait a bit longer than the timeout to ensure that the command has enough time to finish
+		<-time.After(time.Duration(timeout+5) * time.Second)
+		fmt.Fprintf(os.Stdout, "command timed out")
 		onTimeout()
 	}()
 }

cliv2/cmd/cliv2/main_test.go

@@ -1,16 +1,15 @@
 package main
 
 import (
-	"os"
-	"testing"
-	"time"
-
 	"github.com/snyk/go-application-framework/pkg/configuration"
 	localworkflows "github.com/snyk/go-application-framework/pkg/local_workflows"
 	"github.com/snyk/go-application-framework/pkg/workflow"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 	"github.com/stretchr/testify/assert"
+	"os"
+	"testing"
+	"time"
 )
 
 func cleanup() {
@@ -226,7 +225,7 @@ func Test_setTimeout(t *testing.T) {
 	select {
 	case <-exitedCh:
 		break
-	case <-time.After(5 * time.Second):
+	case <-time.After(8 * time.Second):
 		t.Fatal("timeout func never executed")
 	}
 }

cliv2/internal/cliv2/cliv2.go

@@ -4,7 +4,9 @@ Entry point class for the CLIv2 version.
 package cliv2
 
 import (
+	"context"
 	_ "embed"
+	"errors"
 	"fmt"
 	"io"
 	"log"
@@ -13,6 +15,7 @@ import (
 	"path"
 	"regexp"
 	"strings"
+	"time"
 
 	"github.com/gofrs/flock"
 	"github.com/snyk/go-application-framework/pkg/configuration"
@@ -74,6 +77,11 @@ func NewCLIv2(config configuration.Configuration, debugLogger *log.Logger) (*CLI
 	return &cli, nil
 }
 
+// SetV1BinaryLocation for testing purposes
+func (c *CLI) SetV1BinaryLocation(filePath string) {
+	c.v1BinaryLocation = filePath
+}
+
 func (c *CLI) Init() (err error) {
 	c.DebugLogger.Println("Init start")
 
@@ -200,7 +208,7 @@ func (c *CLI) GetBinaryLocation() string {
 }
 
 func (c *CLI) printVersion() {
-	fmt.Fprintln(c.stdout, GetFullVersion())
+	_, _ = fmt.Fprintln(c.stdout, GetFullVersion())
 }
 
 func (c *CLI) commandVersion(passthroughArgs []string) error {
@@ -235,8 +243,8 @@ func (c *CLI) commandAbout(proxyInfo *proxy.ProxyInfo, passthroughArgs []string)
 			}
 
 			fmt.Printf("Package: %s \n", strings.ReplaceAll(strings.ReplaceAll(fPath, "/licenses/", ""), "/"+f.Name(), ""))
-			fmt.Fprintln(c.stdout, string(data))
-			fmt.Fprint(c.stdout, separator)
+			_, _ = fmt.Fprintln(c.stdout, string(data))
+			_, _ = fmt.Fprint(c.stdout, separator)
 		}
 	}
 
@@ -340,16 +348,9 @@ func PrepareV1EnvironmentVariables(
 
 }
 
-func (c *CLI) PrepareV1Command(
-	cmd string,
-	args []string,
-	proxyInfo *proxy.ProxyInfo,
-	integrationName string,
-	integrationVersion string,
-) (snykCmd *exec.Cmd, err error) {
+func (c *CLI) PrepareV1Command(ctx context.Context, cmd string, args []string, proxyInfo *proxy.ProxyInfo, integrationName string, integrationVersion string) (snykCmd *exec.Cmd, err error) {
 	proxyAddress := fmt.Sprintf("http://%s:%[email protected]:%d", proxy.PROXY_USERNAME, proxyInfo.Password, proxyInfo.Port)
-
-	snykCmd = exec.Command(cmd, args...)
+	snykCmd = exec.CommandContext(ctx, cmd, args...)
 	snykCmd.Env, err = PrepareV1EnvironmentVariables(c.env, integrationName, integrationVersion, proxyAddress, proxyInfo.CertificateLocation, c.globalConfig, args)
 
 	if len(c.WorkingDirectory) > 0 {
@@ -360,8 +361,17 @@ func (c *CLI) PrepareV1Command(
 }
 
 func (c *CLI) executeV1Default(proxyInfo *proxy.ProxyInfo, passThroughArgs []string) error {
+	timeout := c.globalConfig.GetInt(configuration.TIMEOUT)
+	var ctx context.Context
+	var cancel context.CancelFunc
+	if timeout == 0 {
+		ctx = context.Background()
+	} else {
+		ctx, cancel = context.WithTimeout(context.Background(), time.Duration(timeout) * time.Second)
+		defer cancel()
+	}
 
-	snykCmd, err := c.PrepareV1Command(c.v1BinaryLocation, passThroughArgs, proxyInfo, c.GetIntegrationName(), GetFullVersion())
+	snykCmd, err := c.PrepareV1Command(ctx, c.v1BinaryLocation, passThroughArgs, proxyInfo, c.GetIntegrationName(), GetFullVersion())
 
 	if c.DebugLogger.Writer() != io.Discard {
 		c.DebugLogger.Println("Launching: ")
@@ -397,13 +407,16 @@ func (c *CLI) executeV1Default(proxyInfo *proxy.ProxyInfo, passThroughArgs []str
 	snykCmd.Stderr = c.stderr
 
 	if err != nil {
-		if evWarning, ok := err.(EnvironmentWarning); ok {
-			fmt.Fprintln(c.stdout, "WARNING! ", evWarning)
+		var evWarning EnvironmentWarning
+		if errors.As(err, &evWarning) {
+			_, _ = fmt.Fprintln(c.stdout, "WARNING! ", evWarning)
 		}
 	}
 
 	err = snykCmd.Run()
-
+	if errors.Is(ctx.Err(), context.DeadlineExceeded) {
+		return ctx.Err()
+	}
 	return err
 }
 
@@ -427,14 +440,17 @@ func DeriveExitCode(err error) int {
 	returnCode := constants.SNYK_EXIT_CODE_OK
 
 	if err != nil {
-		if exitError, ok := err.(*exec.ExitError); ok {
+		var exitError *exec.ExitError
+
+		if errors.As(err, &exitError) {
 			returnCode = exitError.ExitCode()
+		} else if errors.Is(err, context.DeadlineExceeded) {
+			returnCode = constants.SNYK_EXIT_CODE_EX_UNAVAILABLE
 		} else {
 			// got an error but it's not an ExitError
 			returnCode = constants.SNYK_EXIT_CODE_ERROR
 		}
 	}
-
 	return returnCode
 }
 

cliv2/internal/cliv2/cliv2_test.go

@@ -1,11 +1,13 @@
 package cliv2_test
 
 import (
+	"context"
 	"io"
 	"log"
 	"os"
 	"os/exec"
 	"path"
+	"runtime"
 	"sort"
 	"testing"
 	"time"
@@ -273,6 +275,7 @@ func Test_prepareV1Command(t *testing.T) {
 	cli, _ := cliv2.NewCLIv2(config, discardLogger)
 
 	snykCmd, err := cli.PrepareV1Command(
+		context.Background(),
 		"someExecutable",
 		expectedArgs,
 		getProxyInfoForTest(),
@@ -301,7 +304,7 @@ func Test_extractOnlyOnce(t *testing.T) {
 
 	// run once
 	assert.Nil(t, cli.Init())
-	cli.Execute(getProxyInfoForTest(), []string{"--help"})
+	_ = cli.Execute(getProxyInfoForTest(), []string{"--help"})
 	assert.FileExists(t, cli.GetBinaryLocation())
 	fileInfo1, _ := os.Stat(cli.GetBinaryLocation())
 
@@ -310,7 +313,7 @@ func Test_extractOnlyOnce(t *testing.T) {
 
 	// run twice
 	assert.Nil(t, cli.Init())
-	cli.Execute(getProxyInfoForTest(), []string{"--help"})
+	_ = cli.Execute(getProxyInfoForTest(), []string{"--help"})
 	assert.FileExists(t, cli.GetBinaryLocation())
 	fileInfo2, _ := os.Stat(cli.GetBinaryLocation())
 
@@ -460,3 +463,23 @@ func Test_clearCacheBigCache(t *testing.T) {
 	assert.DirExists(t, dir6)
 	assert.FileExists(t, currentVersion)
 }
+
+func Test_setTimeout(t *testing.T) {
+	if //goland:noinspection ALL
+	runtime.GOOS == "windows" {
+		t.Skip("Skipping test on windows")
+	}
+	config := configuration.NewInMemory()
+	cli, err := cliv2.NewCLIv2(config, discardLogger)
+	assert.NoError(t, err)
+	config.Set(configuration.TIMEOUT, 1)
+
+	// sleep for 2s
+	cli.SetV1BinaryLocation("/bin/sleep")
+	err := cli.Execute(getProxyInfoForTest(), []string{"2"})
+
+	assert.ErrorIs(t, err, context.DeadlineExceeded)
+
+	// ensure that -1 is correctly mapped if timeout is set
+	assert.Equal(t, constants.SNYK_EXIT_CODE_EX_UNAVAILABLE, cliv2.DeriveExitCode(err))
+}