Fix gosec security alerts

go.mod

@@ -10,7 +10,6 @@ require (
 	github.com/mattn/go-sqlite3 v1.14.44
 	github.com/pressly/goose/v3 v3.27.1
 	github.com/stretchr/testify v1.11.1
-	golang.org/x/term v0.43.0
 	gopkg.in/yaml.v3 v3.0.1
 )
 

go.sum

@@ -68,8 +68,6 @@ golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4=
 golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0=
 golang.org/x/sys v0.44.0 h1:ildZl3J4uzeKP07r2F++Op7E9B29JRUy+a27EibtBTQ=
 golang.org/x/sys v0.44.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw=
-golang.org/x/term v0.43.0 h1:S4RLU2sB31O/NCl+zFN9Aru9A/Cq2aqKpTZJ6B+DwT4=
-golang.org/x/term v0.43.0/go.mod h1:lrhlHNdQJHO+1qVYiHfFKVuVioJIheAc3fBSMFYEIsk=
 golang.org/x/text v0.36.0 h1:JfKh3XmcRPqZPKevfXVpI1wXPTqbkE5f7JA92a55Yxg=
 golang.org/x/text v0.36.0/go.mod h1:NIdBknypM8iqVmPiuco0Dh6P5Jcdk8lJL0CUebqK164=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=

internal/adapters/providers/amazon/provider.go

@@ -17,6 +17,7 @@ import (
 	"os/exec"
 	"regexp"
 	"strconv"
+	"strings"
 	"time"
 
 	"github.com/eshaffer321/monarchmoney-sync-backend/internal/adapters/providers"
@@ -25,6 +26,21 @@ import (
 // validProfilePattern matches alphanumeric, dash, and underscore characters only
 var validProfilePattern = regexp.MustCompile(`^[a-zA-Z0-9_-]+$`)
 
+// allowedCLIArgsPattern limits arguments to the scraper flags and scalar values
+// produced by buildCLIArgs.
+var allowedCLIArgsPattern = regexp.MustCompile(`^[a-zA-Z0-9_-]+$`)
+
+const (
+	amazonScraperCommand = "amazon-scraper"
+	npxCommand           = "npx"
+	scraperPackageName   = "amazon-order-scraper"
+)
+
+type cliCommand struct {
+	name   string
+	useNpx bool
+}
+
 // isValidProfile checks if a profile name is safe to pass to the CLI
 func isValidProfile(profile string) bool {
 	if profile == "" {
@@ -188,18 +204,26 @@ func (p *Provider) buildCLIArgs(opts providers.FetchOptions) []string {
 // executeCLI executes the amazon-order-scraper CLI and returns the output
 func (p *Provider) executeCLI(ctx context.Context, args []string) ([]byte, error) {
 	// Try to find the CLI
-	cliPath, useNpx := p.findCLI()
+	cli, err := p.findCLI()
+	if err != nil {
+		return nil, err
+	}
+	if err := validateCLIArgs(args); err != nil {
+		return nil, err
+	}
 
 	var cmd *exec.Cmd
-	if useNpx {
+	// Keep executable names literal for static analysis; args are validated before assignment.
+	if cli.useNpx {
 		// Use npx to run the package
-		npxArgs := append([]string{"amazon-order-scraper"}, args...)
-		cmd = exec.CommandContext(ctx, cliPath, npxArgs...)
-		p.logger.Debug("executing CLI via npx", slog.String("args", fmt.Sprintf("%v", npxArgs)))
+		cmd = exec.CommandContext(ctx, "npx")
+		cmd.Args = append([]string{npxCommand, scraperPackageName}, args...)
+		p.logger.Debug("executing CLI via npx", slog.String("args", fmt.Sprintf("%v", cmd.Args[1:])))
 	} else {
 		// Direct execution
-		cmd = exec.CommandContext(ctx, cliPath, args...)
-		p.logger.Debug("executing CLI directly", slog.String("path", cliPath), slog.String("args", fmt.Sprintf("%v", args)))
+		cmd = exec.CommandContext(ctx, "amazon-scraper")
+		cmd.Args = append([]string{amazonScraperCommand}, args...)
+		p.logger.Debug("executing CLI directly", slog.String("command", cli.name), slog.String("args", fmt.Sprintf("%v", args)))
 	}
 
 	var stdout, stderr bytes.Buffer
@@ -209,7 +233,7 @@ func (p *Provider) executeCLI(ctx context.Context, args []string) ([]byte, error
 		cmd.Env = append(os.Environ(), "BROWSER_DATA_DIR="+p.browserDataDir)
 	}
 
-	err := cmd.Run()
+	err = cmd.Run()
 	if err != nil {
 		// Check exit code for specific errors
 		if exitErr, ok := err.(*exec.ExitError); ok {
@@ -240,19 +264,35 @@ func (p *Provider) loginCommand() string {
 
 // findCLI locates the amazon-order-scraper CLI
 // Returns the path and whether to use npx
-func (p *Provider) findCLI() (string, bool) {
+func (p *Provider) findCLI() (cliCommand, error) {
 	// First, try to find globally installed CLI
-	if path, err := exec.LookPath("amazon-scraper"); err == nil {
-		return path, false
+	if _, err := exec.LookPath(amazonScraperCommand); err == nil {
+		return cliCommand{name: amazonScraperCommand}, nil
 	}
 
 	// Fall back to npx
-	if path, err := exec.LookPath("npx"); err == nil {
-		return path, true
+	if _, err := exec.LookPath(npxCommand); err == nil {
+		return cliCommand{name: npxCommand, useNpx: true}, nil
 	}
 
-	// Last resort: assume npx is available
-	return "npx", true
+	return cliCommand{}, fmt.Errorf("amazon-order-scraper CLI not available: install %q or %q", amazonScraperCommand, npxCommand)
+}
+
+func validateCLIArgs(args []string) error {
+	for _, arg := range args {
+		switch arg {
+		case "--since", "--until", "--days", "--profile", "--headless", "--stdout":
+			continue
+		}
+		if strings.HasPrefix(arg, "--") {
+			return fmt.Errorf("unsupported amazon CLI flag: %q", arg)
+		}
+		if !allowedCLIArgsPattern.MatchString(arg) {
+			return fmt.Errorf("unsafe amazon CLI argument: %q", arg)
+		}
+	}
+
+	return nil
 }
 
 // GetOrderDetails fetches details for a specific order
@@ -285,13 +325,16 @@ func (p *Provider) GetRateLimit() time.Duration {
 // HealthCheck verifies the provider can connect and authenticate
 func (p *Provider) HealthCheck(ctx context.Context) error {
 	// Try to find the CLI
-	cliPath, useNpx := p.findCLI()
+	cli, err := p.findCLI()
+	if err != nil {
+		return err
+	}
 
 	var cmd *exec.Cmd
-	if useNpx {
-		cmd = exec.CommandContext(ctx, cliPath, "amazon-order-scraper", "--help")
+	if cli.useNpx {
+		cmd = exec.CommandContext(ctx, npxCommand, scraperPackageName, "--help")
 	} else {
-		cmd = exec.CommandContext(ctx, cliPath, "--help")
+		cmd = exec.CommandContext(ctx, amazonScraperCommand, "--help")
 	}
 
 	if err := cmd.Run(); err != nil {

internal/adapters/providers/amazon/provider_test.go

@@ -1,11 +1,16 @@
 package amazon
 
 import (
+	"context"
+	"os"
+	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 
 	"github.com/eshaffer321/monarchmoney-sync-backend/internal/adapters/providers"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 // TestProvider_ImplementsInterface verifies the provider implements the interface
@@ -59,6 +64,11 @@ func TestProvider_WithConfig(t *testing.T) {
 	assert.Equal(t, "/tmp/itemize-amazon", provider.browserDataDir)
 }
 
+func TestNewProvider_RejectsUnsafeProfile(t *testing.T) {
+	provider := NewProvider(nil, &ProviderConfig{Profile: "wife;rm-rf"})
+	assert.Empty(t, provider.profile)
+}
+
 // TestProvider_BuildCLIArgs tests CLI argument building
 func TestProvider_BuildCLIArgs(t *testing.T) {
 	tests := []struct {
@@ -104,6 +114,157 @@ func TestProvider_BuildCLIArgs(t *testing.T) {
 	}
 }
 
+func TestValidateCLIArgs(t *testing.T) {
+	assert.NoError(t, validateCLIArgs([]string{
+		"--since", "2024-11-01",
+		"--until", "2024-11-30",
+		"--days", "14",
+		"--profile", "wife",
+		"--headless",
+		"--stdout",
+	}))
+	assert.Error(t, validateCLIArgs([]string{"--stdout", ";rm"}))
+	assert.Error(t, validateCLIArgs([]string{"--stdout", "--eval"}))
+}
+
+func TestExecuteCLIUsesDirectScraperWithValidatedArgsAndBrowserDir(t *testing.T) {
+	binDir := t.TempDir()
+	writeExecutable(t, binDir, amazonScraperCommand, `#!/bin/sh
+printf 'args=%s\n' "$*"
+printf 'browser=%s\n' "$BROWSER_DATA_DIR"
+`)
+	t.Setenv("PATH", binDir)
+
+	provider := NewProvider(nil, &ProviderConfig{BrowserDataDir: "/tmp/amazon-browser"})
+	output, err := provider.executeCLI(context.Background(), []string{"--days", "14", "--stdout"})
+
+	require.NoError(t, err)
+	assert.Contains(t, string(output), "args=--days 14 --stdout")
+	assert.Contains(t, string(output), "browser=/tmp/amazon-browser")
+}
+
+func TestExecuteCLIFallsBackToNpx(t *testing.T) {
+	binDir := t.TempDir()
+	writeExecutable(t, binDir, npxCommand, `#!/bin/sh
+printf 'args=%s\n' "$*"
+`)
+	t.Setenv("PATH", binDir)
+
+	provider := NewProvider(nil, nil)
+	output, err := provider.executeCLI(context.Background(), []string{"--stdout"})
+
+	require.NoError(t, err)
+	assert.Contains(t, string(output), "args=amazon-order-scraper --stdout")
+}
+
+func TestExecuteCLIReturnsValidationErrorBeforeRunningCommand(t *testing.T) {
+	binDir := t.TempDir()
+	markerPath := filepath.Join(binDir, "ran")
+	writeExecutable(t, binDir, amazonScraperCommand, "#!/bin/sh\ntouch "+markerPath+"\n")
+	t.Setenv("PATH", binDir)
+
+	provider := NewProvider(nil, nil)
+	output, err := provider.executeCLI(context.Background(), []string{"--stdout", "--eval"})
+
+	require.Error(t, err)
+	assert.Nil(t, output)
+	assert.Contains(t, err.Error(), "unsupported amazon CLI flag")
+	_, statErr := os.Stat(markerPath)
+	assert.True(t, os.IsNotExist(statErr))
+}
+
+func TestExecuteCLILoginRequiredIncludesProfileAndBrowserDir(t *testing.T) {
+	binDir := t.TempDir()
+	writeExecutable(t, binDir, amazonScraperCommand, "#!/bin/sh\nexit 2\n")
+	t.Setenv("PATH", binDir)
+
+	provider := NewProvider(nil, &ProviderConfig{
+		Profile:        "wife",
+		BrowserDataDir: "/tmp/amazon-browser",
+	})
+	output, err := provider.executeCLI(context.Background(), []string{"--stdout"})
+
+	require.Error(t, err)
+	assert.Nil(t, output)
+	assert.Contains(t, err.Error(), "amazon login required")
+	assert.Contains(t, err.Error(), `BROWSER_DATA_DIR="/tmp/amazon-browser"`)
+	assert.Contains(t, err.Error(), "--profile wife")
+}
+
+func TestFindCLIReportsMissingExecutable(t *testing.T) {
+	t.Setenv("PATH", t.TempDir())
+
+	provider := NewProvider(nil, nil)
+	cli, err := provider.findCLI()
+
+	require.Error(t, err)
+	assert.Empty(t, cli.name)
+	assert.Contains(t, err.Error(), "not available")
+}
+
+func TestHealthCheckUsesAvailableCLI(t *testing.T) {
+	tests := []struct {
+		name           string
+		executableName string
+		wantArg        string
+	}{
+		{
+			name:           "direct scraper",
+			executableName: amazonScraperCommand,
+			wantArg:        "--help",
+		},
+		{
+			name:           "npx fallback",
+			executableName: npxCommand,
+			wantArg:        "amazon-order-scraper --help",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			binDir := t.TempDir()
+			argsPath := filepath.Join(binDir, "args.txt")
+			writeExecutable(t, binDir, tt.executableName, `#!/bin/sh
+printf '%s' "$*" > "`+argsPath+`"
+`)
+			t.Setenv("PATH", binDir)
+
+			provider := NewProvider(nil, nil)
+			require.NoError(t, provider.HealthCheck(context.Background()))
+
+			data, err := os.ReadFile(argsPath)
+			require.NoError(t, err)
+			assert.Equal(t, tt.wantArg, string(data))
+		})
+	}
+}
+
+func TestHealthCheckReturnsCommandError(t *testing.T) {
+	binDir := t.TempDir()
+	writeExecutable(t, binDir, amazonScraperCommand, "#!/bin/sh\nexit 1\n")
+	t.Setenv("PATH", binDir)
+
+	provider := NewProvider(nil, nil)
+	err := provider.HealthCheck(context.Background())
+
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "CLI not available")
+}
+
+func TestLoginCommandDefaultsToDirectScraper(t *testing.T) {
+	provider := NewProvider(nil, &ProviderConfig{Profile: "wife"})
+
+	assert.Equal(t, "run 'amazon-scraper --login --profile wife' to authenticate", provider.loginCommand())
+}
+
+func writeExecutable(t *testing.T, dir, name, content string) {
+	t.Helper()
+
+	path := filepath.Join(dir, name)
+	content = strings.ReplaceAll(content, "\r\n", "\n")
+	require.NoError(t, os.WriteFile(path, []byte(content), 0700))
+}
+
 // TestOrder_Interface verifies Order implements providers.Order
 func TestOrder_Interface(t *testing.T) {
 	var _ providers.Order = (*Order)(nil)