Merge pull request #6967 from snyk/fix/CLI-1634-risk-score-dropped-with-explicit-path-v2

fix: Exclude passthrough args after -- from scan input directories

Author: danskmt

cliv2/pkg/core/main.go

@@ -162,10 +162,18 @@ func updateConfigFromParameter(config configuration.Configuration, args []string
 	}
 	config.Set(configuration.UNKNOWN_ARGS, doubleDashArgs)
 
-	// only consider the first positional argument as input directory if it is not behind a double dash.
-	if len(args) > 0 && !utils.Contains(doubleDashArgs, args[0]) {
-		config.Set(configuration.INPUT_DIRECTORY, args)
-		config.Set(localworkflows.ConfigurationNewAuthenticationToken, args[0])
+	// Only positional args before "--" are input directories. Everything after "--" is
+	// passthrough (e.g. build-tool flags like `-s settings.xml`) and must not leak into
+	// INPUT_DIRECTORY, otherwise it gets misread as a path/package and force-routes to legacy.
+	// cobra appends the post-"--" tokens as the suffix of args, so trim them off.
+	positionalArgs := args
+	if len(doubleDashArgs) > 0 && len(args) >= len(doubleDashArgs) {
+		positionalArgs = args[:len(args)-len(doubleDashArgs)]
+	}
+
+	if len(positionalArgs) > 0 {
+		config.Set(configuration.INPUT_DIRECTORY, positionalArgs)
+		config.Set(localworkflows.ConfigurationNewAuthenticationToken, positionalArgs[0])
 	}
 }
 

cliv2/pkg/core/main_test.go

@@ -185,71 +185,104 @@ func Test_CreateCommandsForWorkflowWithSubcommands(t *testing.T) {
 	assert.True(t, cmd2.Hidden)
 }
 
-func Test_runMainWorkflow_unknownargs(t *testing.T) {
+// setupMainWorkflowTestEnv wires up the global engine/config that runMainWorkflow needs and
+// returns a fresh per-invocation config plus a command. Callers should `defer cleanup()`.
+func setupMainWorkflowTestEnv(t *testing.T) (configuration.Configuration, *cobra.Command) {
+	t.Helper()
+
+	globalConfiguration = configuration.New()
+	globalConfiguration.Set(configuration.DEBUG, true)
+	globalEngine = workflow.NewWorkFlowEngine(globalConfiguration)
+
+	noopWorkflow := func(workflow.InvocationContext, []workflow.Data) ([]workflow.Data, error) {
+		return []workflow.Data{}, nil
+	}
+	for _, name := range []string{"command", localworkflows.WORKFLOWID_OUTPUT_WORKFLOW.Host} {
+		opts := workflow.ConfigurationOptionsFromFlagset(pflag.NewFlagSet("pla", pflag.ContinueOnError))
+		_, err := globalEngine.Register(workflow.NewWorkflowIdentifier(name), opts, noopWorkflow)
+		require.NoError(t, err)
+	}
+	require.NoError(t, localworkflows.InitDataTransformationWorkflow(globalEngine))
+	_ = globalEngine.Init()
+	require.NoError(t, localworkflows.InitFilterFindingsWorkflow(globalEngine))
+
+	return configuration.NewWithOpts(configuration.WithAutomaticEnv()), &cobra.Command{Use: "command"}
+}
+
+// Test_runMainWorkflow_inputDirectoryParsing is the CLI-1631 regression coverage. It asserts how
+// positional paths and "--" passthrough args map onto INPUT_DIRECTORY and UNKNOWN_ARGS. The key
+// invariant: passthrough tokens after "--" must never leak into INPUT_DIRECTORY (which would make
+// the downstream flow router misread them as package names and force the legacy, no-Risk-Score flow),
+// regardless of the path shape. wantInputDirs == nil means INPUT_DIRECTORY must be left unset so it
+// later defaults to the working directory.
+func Test_runMainWorkflow_inputDirectoryParsing(t *testing.T) {
 	tests := map[string]struct {
-		inputDir    string
-		unknownArgs []string
+		positionalArgs  []string
+		rawArgs         []string
+		wantInputDirs   []string
+		wantUnknownArgs []string
 	}{
-		"input dir with unknown arguments":    {inputDir: "a/b/c", unknownArgs: []string{"a", "b", "c"}},
-		"no input dir with unknown arguments": {inputDir: "", unknownArgs: []string{"a", "b", "c"}},
-		"input dir without unknown arguments": {inputDir: "a", unknownArgs: []string{}},
+		"path with -- passthrough, current dir": {
+			positionalArgs:  []string{".", "-s", "settings.xml"},
+			rawArgs:         []string{"snyk", "test", ".", "--", "-s", "settings.xml"},
+			wantInputDirs:   []string{"."},
+			wantUnknownArgs: []string{"-s", "settings.xml"},
+		},
+		"path with -- passthrough, relative": {
+			positionalArgs:  []string{"sub/project", "-s", "settings.xml"},
+			rawArgs:         []string{"snyk", "test", "sub/project", "--", "-s", "settings.xml"},
+			wantInputDirs:   []string{"sub/project"},
+			wantUnknownArgs: []string{"-s", "settings.xml"},
+		},
+		"path with -- passthrough, absolute": {
+			positionalArgs:  []string{"/home/user/project", "-s", "settings.xml"},
+			rawArgs:         []string{"snyk", "test", "/home/user/project", "--", "-s", "settings.xml"},
+			wantInputDirs:   []string{"/home/user/project"},
+			wantUnknownArgs: []string{"-s", "settings.xml"},
+		},
+		"only -- passthrough, no path": {
+			positionalArgs:  []string{"-s", "settings.xml"},
+			rawArgs:         []string{"snyk", "test", "--", "-s", "settings.xml"},
+			wantInputDirs:   nil,
+			wantUnknownArgs: []string{"-s", "settings.xml"},
+		},
+		"no --, single path": {
+			positionalArgs: []string{"."},
+			rawArgs:        []string{"snyk", "test", "."},
+			wantInputDirs:  []string{"."},
+		},
+		"no --, multiple paths": {
+			positionalArgs: []string{"dir1", "dir2"},
+			rawArgs:        []string{"snyk", "test", "dir1", "dir2"},
+			wantInputDirs:  []string{"dir1", "dir2"},
+		},
+		"bare command, no path no --": {
+			positionalArgs: []string{},
+			rawArgs:        []string{"snyk", "test"},
+			wantInputDirs:  nil,
+		},
 	}
 
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
-			expectedInputDir := tc.inputDir
-			expectedUnknownArgs := tc.unknownArgs
-
 			defer cleanup()
-			globalConfiguration = configuration.New()
-			globalConfiguration.Set(configuration.DEBUG, true)
-			globalEngine = workflow.NewWorkFlowEngine(globalConfiguration)
-
-			fn := func(invocation workflow.InvocationContext, input []workflow.Data) ([]workflow.Data, error) {
-				return []workflow.Data{}, nil
-			}
-
-			// setup workflow engine to contain a workflow with subcommands
-			commandList := []string{"command", localworkflows.WORKFLOWID_OUTPUT_WORKFLOW.Host}
-			for _, v := range commandList {
-				workflowConfig := workflow.ConfigurationOptionsFromFlagset(pflag.NewFlagSet("pla", pflag.ContinueOnError))
-				workflowId1 := workflow.NewWorkflowIdentifier(v)
-				_, err := globalEngine.Register(workflowId1, workflowConfig, fn)
-				assert.NoError(t, err)
-			}
+			config, cmd := setupMainWorkflowTestEnv(t)
 
-			// Register our data transformation workflow
-			err := localworkflows.InitDataTransformationWorkflow(globalEngine)
-			assert.NoError(t, err)
+			require.NoError(t, runMainWorkflow(config, cmd, tc.positionalArgs, tc.rawArgs))
 
-			_ = globalEngine.Init()
-			// Register our data filter workflow
-			err = localworkflows.InitFilterFindingsWorkflow(globalEngine)
-			assert.NoError(t, err)
-
-			config := configuration.NewWithOpts(configuration.WithAutomaticEnv())
-			cmd := &cobra.Command{
-				Use: "command",
+			if tc.wantInputDirs == nil {
+				assert.Nil(t, config.Get(configuration.INPUT_DIRECTORY),
+					"INPUT_DIRECTORY must be left unset so it defaults to the working directory")
+			} else {
+				assert.Equal(t, tc.wantInputDirs, config.GetStringSlice(configuration.INPUT_DIRECTORY),
+					"passthrough args after -- must not leak into INPUT_DIRECTORY")
 			}
 
-			positionalArgs := []string{expectedInputDir}
-			positionalArgs = append(positionalArgs, expectedUnknownArgs...)
-
-			rawArgs := []string{"app", "command", "--sad", expectedInputDir}
-			if len(expectedUnknownArgs) > 0 {
-				rawArgs = append(rawArgs, "--")
-				rawArgs = append(rawArgs, expectedUnknownArgs...)
+			if len(tc.wantUnknownArgs) == 0 {
+				assert.Empty(t, config.GetStringSlice(configuration.UNKNOWN_ARGS))
+			} else {
+				assert.Equal(t, tc.wantUnknownArgs, config.GetStringSlice(configuration.UNKNOWN_ARGS))
 			}
-
-			// call method under test
-			err = runMainWorkflow(config, cmd, positionalArgs, rawArgs)
-			assert.Nil(t, err)
-
-			actualInputDir := config.GetString(configuration.INPUT_DIRECTORY)
-			assert.Equal(t, expectedInputDir, actualInputDir)
-
-			actualUnknownArgs := config.GetStringSlice(configuration.UNKNOWN_ARGS)
-			assert.Equal(t, expectedUnknownArgs, actualUnknownArgs)
 		})
 	}
 }