Include failed command in executor error messages

[sgaunet]

Problem

When command execution fails, the error message doesn't include which command failed, making debugging difficult.

Current Implementation

// pkg/executor/executor.go:106
if err := cmd.Start(); err != nil {
    return 0, fmt.Errorf("command execution failed: %w", err)
}

Error output:

Error: command execution failed: exec: "nonexistent": executable file not found in $PATH

Missing: Which command was attempted? What were the arguments?

Impact

User Experience:
When debugging failures, users need to know:

1. What command was attempted
2. What arguments were passed
3. Where the command was looked for (PATH)

Current workaround:
Users must add debug logging or use set -x in shell scripts.

Proposed Solution

Include Command in Error Messages

// pkg/executor/executor.go
func (e *Executor) Execute(command string, args []string) (int, error) {
    // Validate command
    if err := validateCommand(command); err != nil {
        return 0, fmt.Errorf("invalid command %q: %w", command, err)
    }
    
    // Create command
    cmd := exec.Command(command, args...)
    
    // Start command
    if err := cmd.Start(); err != nil {
        return 0, fmt.Errorf("failed to start command %q with args %v: %w", 
            command, args, err)
    }
    
    // Setup streams
    stdout, err := cmd.StdoutPipe()
    if err != nil {
        return 0, fmt.Errorf("failed to create stdout pipe for %q: %w", 
            command, err)
    }
    
    // ... similar for stderr ...
    
    // Wait for command
    if err := cmd.Wait(); err != nil {
        // Don't include command here - this is expected for non-zero exits
        return cmd.ProcessState.ExitCode(), nil
    }
    
    return 0, nil
}

Better Error Messages

Before:

Error: command execution failed: exec: "nonexistent": executable file not found in $PATH

After:

Error: failed to start command "nonexistent" with args ["-la", "/tmp"]: exec: "nonexistent": executable file not found in $PATH

Security Consideration

Issue: Including full command with arguments might expose sensitive data:

# Command with password
logwrap curl -u "user:password" https://api.example.com

# Error exposes password:
Error: failed to start command "curl" with args ["-u", "user:password", "https://..."]

Solution: Sanitize or truncate arguments in error messages.

Option 1: Truncate Long Arguments

func sanitizeArgs(args []string) []string {
    const maxLen = 50
    sanitized := make([]string, len(args))
    for i, arg := range args {
        if len(arg) > maxLen {
            sanitized[i] = arg[:maxLen] + "..."
        } else {
            sanitized[i] = arg
        }
    }
    return sanitized
}

// In error message:
fmt.Errorf("failed to start command %q with args %v: %w", 
    command, sanitizeArgs(args), err)

Option 2: Mask Sensitive Patterns

func maskSensitiveArgs(args []string) []string {
    masked := make([]string, len(args))
    for i, arg := range args {
        // Mask arguments that look like passwords/tokens
        if isSensitive(arg) {
            masked[i] = "***"
        } else {
            masked[i] = arg
        }
    }
    return masked
}

func isSensitive(arg string) bool {
    // Check for password-like patterns
    lowerArg := strings.ToLower(arg)
    return strings.Contains(lowerArg, "password") ||
           strings.Contains(lowerArg, "token") ||
           strings.Contains(lowerArg, "secret") ||
           strings.Contains(lowerArg, "key=")
}

Option 3: Count Arguments Only

// Conservative approach - don't show args at all
fmt.Errorf("failed to start command %q with %d arguments: %w", 
    command, len(args), err)

Recommended Approach

Combination:

1. Always include command name
2. Include argument count
3. Optionally include first N args (configurable)
4. Mask sensitive patterns

type ErrorContext struct {
    Command    string
    ArgCount   int
    SampleArgs []string  // First 3 non-sensitive args
}

func (e *Executor) Execute(command string, args []string) (int, error) {
    ctx := ErrorContext{
        Command:    command,
        ArgCount:   len(args),
        SampleArgs: getSafeArgs(args, 3),
    }
    
    if err := cmd.Start(); err != nil {
        return 0, fmt.Errorf("failed to start %q (%d args, sample: %v): %w",
            ctx.Command, ctx.ArgCount, ctx.SampleArgs, err)
    }
    // ...
}

Implementation Checklist

High Priority:

• Include command name in all error messages
• Include argument count
• Test error messages in unit tests

Medium Priority:

• Add argument sanitization
• Mask sensitive patterns
• Add configuration for verbosity level

Nice to Have:

• Add debug mode with full args
• Log full command to debug log
• Add structured error types

Error Message Guidelines

Follow these patterns:

// Validation errors - include what failed
fmt.Errorf("invalid command %q: %w", command, err)

// Startup errors - include command + context
fmt.Errorf("failed to start %q: %w", command, err)

// Pipe errors - include command + pipe type
fmt.Errorf("failed to create stdout pipe for %q: %w", command, err)

// Process errors - include command + exit code
fmt.Errorf("command %q exited with code %d", command, exitCode)

Test Cases

func TestExecutorErrorMessages(t *testing.T) {
    tests := []struct {
        name       string
        command    string
        args       []string
        wantErrMsg string
    }{
        {
            name:       "Command not found",
            command:    "nonexistent",
            args:       []string{"-la"},
            wantErrMsg: `failed to start command "nonexistent"`,
        },
        {
            name:       "Invalid command path",
            command:    "../etc/passwd",
            args:       []string{},
            wantErrMsg: `invalid command "../etc/passwd"`,
        },
        {
            name:       "Permission denied",
            command:    "/root/script.sh",
            args:       []string{},
            wantErrMsg: `failed to start command "/root/script.sh"`,
        },
    }
    
    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            exec := New()
            _, err := exec.Execute(tt.command, tt.args)
            
            assert.Error(t, err)
            assert.Contains(t, err.Error(), tt.wantErrMsg)
        })
    }
}

Configuration Option

Add optional verbosity for error messages:

# config.yaml
debug:
  verbose_errors: true  # Include full command with args in errors

type DebugConfig struct {
    VerboseErrors bool `yaml:"verbose_errors"`
}

// In executor:
if e.config.Debug.VerboseErrors {
    return 0, fmt.Errorf("failed to start %q with args %v: %w", 
        command, args, err)
} else {
    return 0, fmt.Errorf("failed to start %q (%d args): %w", 
        command, len(args), err)
}

Related Issues

• Improve security documentation and command validation boundaries #10 - Security documentation (mention error message sanitization)
• Add comprehensive package-level documentation #14 - Package documentation (document error handling)
• Add end-to-end integration tests for complete pipeline #9 - Integration tests (test error messages)

Benefits

Immediate:

• Easier debugging of command failures
• Better error messages for users
• Clearer logs

Long-term:

• Reduced support burden
• Better observability
• Consistent error format

References

• Go error handling: https://go.dev/blog/error-handling-and-go
• Error messages: https://github.com/golang/go/wiki/CodeReviewComments#error-strings