feat: comment on security of runExternal, refactor based on gemini suggestions

Author: denyszhak

cmd/cli/commands/launch.go

@@ -54,15 +54,16 @@ var containerApps = map[string]containerApp{
 
 // hostApp describes a native CLI app launched on the host.
 type hostApp struct {
-	envFn func(baseURL string) []string
+	envFn              func(baseURL string) []string
+	configInstructions func(baseURL string) []string // for apps that need manual config
 }
 
 // hostApps are launched as native executables on the host.
 var hostApps = map[string]hostApp{
 	"opencode": {envFn: openaiEnv(openaiPathSuffix)},
 	"codex":    {envFn: openaiEnv("/v1")},
 	"claude":   {envFn: anthropicEnv},
-	"clawdbot": {envFn: nil},
+	"clawdbot": {configInstructions: clawdbotConfigInstructions},
 }
 
 // supportedApps is derived from the registries above.
@@ -198,18 +199,18 @@ func launchContainerApp(cmd *cobra.Command, ca containerApp, baseURL string, ima
 // launchHostApp launches a native host app executable.
 func launchHostApp(cmd *cobra.Command, bin string, baseURL string, cli hostApp, appArgs []string, dryRun bool) error {
 	if _, err := exec.LookPath(bin); err != nil {
-		cmd.Printf("%q executable not found in PATH.\n", bin)
+		cmd.PrintErrf("%q executable not found in PATH.\n", bin)
 		if cli.envFn != nil {
-			cmd.Printf("Configure your app to use:\n")
+			cmd.PrintErrf("Configure your app to use:\n")
 			for _, e := range cli.envFn(baseURL) {
-				cmd.Printf("  %s\n", e)
+				cmd.PrintErrf("  %s\n", e)
 			}
 		}
 		return fmt.Errorf("%s not found; please install it and re-run", bin)
 	}
 
 	if cli.envFn == nil {
-		return launchUnconfigurableHostApp(cmd, bin, baseURL, dryRun)
+		return launchUnconfigurableHostApp(cmd, bin, baseURL, cli, dryRun)
 	}
 
 	env := cli.envFn(baseURL)
@@ -224,24 +225,35 @@ func launchHostApp(cmd *cobra.Command, bin string, baseURL string, cli hostApp,
 }
 
 // launchUnconfigurableHostApp handles host apps that need manual config rather than env vars.
-func launchUnconfigurableHostApp(cmd *cobra.Command, bin string, baseURL string, dryRun bool) error {
+func launchUnconfigurableHostApp(cmd *cobra.Command, bin string, baseURL string, cli hostApp, dryRun bool) error {
 	enginesEP := baseURL + openaiPathSuffix
 	cmd.Printf("Configure %s to use Docker Model Runner:\n", bin)
 	cmd.Printf("  Base URL: %s\n", enginesEP)
 	cmd.Printf("  API type: openai-completions\n")
-	cmd.Printf("  API key:  docker-model-runner\n")
-	if bin == "clawdbot" {
+	cmd.Printf("  API key:  %s\n", dummyAPIKey)
+
+	if cli.configInstructions != nil {
 		cmd.Printf("\nExample:\n")
-		cmd.Printf("  clawdbot config set models.providers.docker-model-runner.baseUrl %q\n", enginesEP)
-		cmd.Printf("  clawdbot config set models.providers.docker-model-runner.api openai-completions\n")
-		cmd.Printf("  clawdbot config set models.providers.docker-model-runner.apiKey docker-model-runner\n")
+		for _, line := range cli.configInstructions(baseURL) {
+			cmd.Printf("  %s\n", line)
+		}
 	}
 	if dryRun {
 		return nil
 	}
 	return runExternal(cmd, nil, bin)
 }
 
+// clawdbotConfigInstructions returns configuration commands for clawdbot.
+func clawdbotConfigInstructions(baseURL string) []string {
+	ep := baseURL + openaiPathSuffix
+	return []string{
+		fmt.Sprintf("clawdbot config set models.providers.docker-model-runner.baseUrl %q", ep),
+		"clawdbot config set models.providers.docker-model-runner.api openai-completions",
+		fmt.Sprintf("clawdbot config set models.providers.docker-model-runner.apiKey %s", dummyAPIKey),
+	}
+}
+
 // openaiEnv returns an env builder that sets OpenAI-compatible
 // environment variables using the given path suffix.
 func openaiEnv(suffix string) func(string) []string {
@@ -280,6 +292,8 @@ func withEnv(extra ...string) []string {
 }
 
 // runExternal executes a program inheriting stdio.
+// Security: prog and progArgs are either hardcoded values or user-provided
+// arguments that the user explicitly intends to pass to the launched app.
 func runExternal(cmd *cobra.Command, env []string, prog string, progArgs ...string) error {
 	c := exec.Command(prog, progArgs...)
 	c.Stdout = cmd.OutOrStdout()

cmd/cli/commands/launch_test.go

@@ -283,44 +283,51 @@ func TestLaunchHostAppDryRunAnthropic(t *testing.T) {
 }
 
 func TestLaunchHostAppNotFound(t *testing.T) {
-	buf := new(bytes.Buffer)
-	cmd := newTestCmd(buf)
+	stdout := new(bytes.Buffer)
+	stderr := new(bytes.Buffer)
+	cmd := &cobra.Command{}
+	cmd.SetOut(stdout)
+	cmd.SetErr(stderr)
 
 	cli := hostApp{envFn: openaiEnv(openaiPathSuffix)}
 	err := launchHostApp(cmd, "nonexistent-binary-xyz", testBaseURL, cli, nil, false)
 	require.Error(t, err)
 	require.Contains(t, err.Error(), "not found")
 
-	output := buf.String()
-	require.Contains(t, output, "not found in PATH")
-	require.Contains(t, output, "Configure your app to use:")
+	errOutput := stderr.String()
+	require.Contains(t, errOutput, "not found in PATH")
+	require.Contains(t, errOutput, "Configure your app to use:")
 }
 
 func TestLaunchHostAppNotFoundNilEnvFn(t *testing.T) {
-	buf := new(bytes.Buffer)
-	cmd := newTestCmd(buf)
+	stdout := new(bytes.Buffer)
+	stderr := new(bytes.Buffer)
+	cmd := &cobra.Command{}
+	cmd.SetOut(stdout)
+	cmd.SetErr(stderr)
 
 	cli := hostApp{envFn: nil}
 	err := launchHostApp(cmd, "nonexistent-binary-xyz", testBaseURL, cli, nil, false)
 	require.Error(t, err)
 
-	output := buf.String()
-	require.Contains(t, output, "not found in PATH")
-	require.NotContains(t, output, "Configure your app to use:")
+	errOutput := stderr.String()
+	require.Contains(t, errOutput, "not found in PATH")
+	require.NotContains(t, errOutput, "Configure your app to use:")
 }
 
 func TestLaunchUnconfigurableHostAppDryRun(t *testing.T) {
 	buf := new(bytes.Buffer)
 	cmd := newTestCmd(buf)
 
-	err := launchUnconfigurableHostApp(cmd, "clawdbot", testBaseURL, true)
+	cli := hostApp{configInstructions: clawdbotConfigInstructions}
+	err := launchUnconfigurableHostApp(cmd, "clawdbot", testBaseURL, cli, true)
 	require.NoError(t, err)
 
 	output := buf.String()
 	require.Contains(t, output, "Configure clawdbot to use Docker Model Runner:")
 	require.Contains(t, output, "Base URL: "+testBaseURL+"/engines/v1")
 	require.Contains(t, output, "API type: openai-completions")
-	require.Contains(t, output, "API key:  docker-model-runner")
+	require.Contains(t, output, "API key:  "+dummyAPIKey)
 	require.Contains(t, output, "clawdbot config set models.providers.docker-model-runner.baseUrl")
 }