fix: patch critical and high security vulnerabilities

- prevent path traversal in install/uninstall/purge via app name validation
- add exec timeout (30s default) to prevent watcher hang
- add file permission check for alerts.yaml
- bind web server to 127.0.0.1 by default, warn on 0.0.0.0
- add optional --token flag for web API authentication
- validate port is numeric in portInUseBy
- drain HTTP response bodies in notify/webhook
- clean up partial compose file on template render failure

Author: Higangssh

cmd/serve.go

@@ -9,6 +9,7 @@ func newServeCmd() *cobra.Command {
 	var host string
 	var port int
 	var demo bool
+	var token string
 
 	cmd := &cobra.Command{
 		Use:   "serve",
@@ -21,13 +22,17 @@ func newServeCmd() *cobra.Command {
 
 			srv := server.New(cfg, host, port, demo)
 			srv.SetVersion(Version)
+			if token != "" {
+				srv.SetToken(token)
+			}
 			return srv.Run()
 		},
 	}
 
 	cmd.Flags().StringVar(&host, "host", "127.0.0.1", "Host to bind to")
 	cmd.Flags().IntVar(&port, "port", 8080, "Port for the web dashboard")
 	cmd.Flags().BoolVar(&demo, "demo", false, "Run with realistic demo data (no real system calls)")
+	cmd.Flags().StringVar(&token, "token", "", "Bearer token for API authentication")
 
 	return cmd
 }

internal/alerts/notify.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
+	"io"
 	"net/http"
 	"time"
 )
@@ -185,7 +186,10 @@ func postJSON(url string, payload interface{}) error {
 	if err != nil {
 		return fmt.Errorf("request failed: %w", err)
 	}
-	defer resp.Body.Close()
+	defer func() {
+		io.Copy(io.Discard, resp.Body)
+		resp.Body.Close()
+	}()
 
 	if resp.StatusCode >= 400 {
 		return fmt.Errorf("returned status %d", resp.StatusCode)

internal/alerts/playbook.go

@@ -1,7 +1,9 @@
 package alerts
 
 import (
+	"context"
 	"fmt"
+	"log"
 	"os/exec"
 	"strings"
 	"sync"
@@ -67,6 +69,8 @@ var dangerousPatterns = []string{
 }
 
 // IsDangerousCommand checks if a command matches known dangerous patterns.
+// NOTE: This is a best-effort blocklist and is NOT a complete security boundary.
+// It serves as a supplementary safety net; do not rely on it as the sole defense.
 func IsDangerousCommand(cmd string) bool {
 	lower := strings.ToLower(strings.TrimSpace(cmd))
 	for _, pattern := range dangerousPatterns {
@@ -135,18 +139,35 @@ func executeExec(rule Rule) PlaybookResult {
 		}
 	}
 
-	cmd := exec.Command("sh", "-c", rule.Exec)
+	log.Printf("[exec] rule=%q running: %s (timeout=%s)", rule.Name, rule.Exec, rule.ExecTimeout())
+
+	timeout := rule.ExecTimeout()
+	ctx, cancel := context.WithTimeout(context.Background(), timeout)
+	defer cancel()
+
+	cmd := exec.CommandContext(ctx, "sh", "-c", rule.Exec)
 	out, err := cmd.CombinedOutput()
 	output := strings.TrimSpace(string(out))
 
+	if ctx.Err() == context.DeadlineExceeded {
+		log.Printf("[exec] rule=%q timed out after %s", rule.Name, timeout)
+		return PlaybookResult{
+			Action:  "exec",
+			Success: false,
+			Output:  fmt.Sprintf("command timed out after %s", timeout),
+		}
+	}
+
 	if err != nil {
+		log.Printf("[exec] rule=%q failed: %v", rule.Name, err)
 		return PlaybookResult{
 			Action:  "exec",
 			Success: false,
 			Output:  fmt.Sprintf("command failed: %s (output: %s)", err, output),
 		}
 	}
 
+	log.Printf("[exec] rule=%q succeeded", rule.Name)
 	return PlaybookResult{
 		Action:  "exec",
 		Success: true,

internal/alerts/playbook_test.go

@@ -1,6 +1,7 @@
 package alerts
 
 import (
+	"strings"
 	"testing"
 	"time"
 )
@@ -131,3 +132,70 @@ func TestExecuteRestartNoContainers(t *testing.T) {
 		t.Error("restart with no containers should fail")
 	}
 }
+
+func TestExecTimeout(t *testing.T) {
+	rule := Rule{Action: "exec", Exec: "sleep 10", Timeout: "1s"}
+	result := ExecuteAction(rule)
+	if result.Success {
+		t.Error("long-running command should fail with timeout")
+	}
+	if !strings.Contains(result.Output, "timed out") {
+		t.Errorf("expected timeout message, got %q", result.Output)
+	}
+}
+
+func TestExecDefaultTimeout(t *testing.T) {
+	rule := Rule{Action: "exec", Exec: "echo fast"}
+	result := ExecuteAction(rule)
+	if !result.Success {
+		t.Errorf("fast command should succeed: %s", result.Output)
+	}
+}
+
+func TestExecCustomTimeout(t *testing.T) {
+	rule := Rule{Action: "exec", Exec: "echo ok", Timeout: "5s"}
+	result := ExecuteAction(rule)
+	if !result.Success {
+		t.Errorf("command should succeed within timeout: %s", result.Output)
+	}
+}
+
+func TestExecTimeoutParsing(t *testing.T) {
+	// Default
+	r := Rule{}
+	if r.ExecTimeout() != 30*time.Second {
+		t.Errorf("default timeout should be 30s, got %s", r.ExecTimeout())
+	}
+	// Custom
+	r.Timeout = "10s"
+	if r.ExecTimeout() != 10*time.Second {
+		t.Errorf("custom timeout should be 10s, got %s", r.ExecTimeout())
+	}
+	// Invalid falls back to 30s
+	r.Timeout = "invalid"
+	if r.ExecTimeout() != 30*time.Second {
+		t.Errorf("invalid timeout should fallback to 30s, got %s", r.ExecTimeout())
+	}
+}
+
+func TestIsDangerousCommandComment(t *testing.T) {
+	// Verify the function still works (blocklist is supplementary)
+	if !IsDangerousCommand("rm -rf /") {
+		t.Error("rm -rf / should still be blocked")
+	}
+	// Bypass example: encoded/obfuscated commands pass through
+	// This is expected — the blocklist is documented as supplementary
+	if IsDangerousCommand("perl -e 'system(\"rm -rf /\")'") {
+		// This should actually NOT be caught by simple string matching
+		// but the nested rm -rf / IS caught since it's in the string
+	}
+}
+
+func TestExecWithStrings(t *testing.T) {
+	// Verify the strings import is used
+	rule := Rule{Action: "exec", Exec: "echo hello"}
+	result := ExecuteAction(rule)
+	if result.Output != "hello" {
+		t.Errorf("expected 'hello', got %q", result.Output)
+	}
+}

internal/alerts/rules.go

@@ -17,7 +17,8 @@ type Rule struct {
 	Watch      []string `yaml:"watch,omitempty" json:"watch,omitempty"` // container names
 	Action     string   `yaml:"action" json:"action"`                   // "notify", "restart", "exec"
 	Exec       string   `yaml:"exec,omitempty" json:"exec,omitempty"`
-	Notify     string   `yaml:"notify,omitempty" json:"notify,omitempty"` // "webhook"
+	Timeout    string   `yaml:"timeout,omitempty" json:"timeout,omitempty"` // exec timeout (default 30s)
+	Notify     string   `yaml:"notify,omitempty" json:"notify,omitempty"`   // "webhook"
 	Cooldown   string   `yaml:"cooldown,omitempty" json:"cooldown,omitempty"`
 	MaxRetries int      `yaml:"max_retries,omitempty" json:"max_retries,omitempty"`
 }
@@ -47,8 +48,29 @@ func (r *Rule) CooldownDuration() time.Duration {
 	return d
 }
 
+// ExecTimeout parses the timeout string into a time.Duration.
+// Returns 30s if not set.
+func (r *Rule) ExecTimeout() time.Duration {
+	if r.Timeout == "" {
+		return 30 * time.Second
+	}
+	d, err := time.ParseDuration(r.Timeout)
+	if err != nil || d <= 0 {
+		return 30 * time.Second
+	}
+	return d
+}
+
 // LoadRules reads and parses an alerts YAML config file.
 func LoadRules(path string) (*AlertsConfig, error) {
+	// Warn if the file has overly permissive permissions (anything beyond 0600).
+	if info, err := os.Stat(path); err == nil {
+		mode := info.Mode().Perm()
+		if mode&0077 != 0 {
+			fmt.Fprintf(os.Stderr, "⚠️  alerts config %s has permissions %04o; consider chmod 0600 for security\n", path, mode)
+		}
+	}
+
 	data, err := os.ReadFile(path)
 	if err != nil {
 		return nil, fmt.Errorf("failed to read alerts config: %w", err)

internal/alerts/webhook.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
+	"io"
 	"net/http"
 	"time"
 )
@@ -37,7 +38,10 @@ func SendWebhook(url string, payload WebhookPayload) error {
 	if err != nil {
 		return fmt.Errorf("webhook request failed: %w", err)
 	}
-	defer resp.Body.Close()
+	defer func() {
+		io.Copy(io.Discard, resp.Body)
+		resp.Body.Close()
+	}()
 
 	if resp.StatusCode >= 400 {
 		return fmt.Errorf("webhook returned status %d", resp.StatusCode)

internal/install/install.go

@@ -436,6 +436,24 @@ func BaseDir() string {
 	return filepath.Join(home, ".homebutler", "apps")
 }
 
+// ValidateAppName checks that an app name is safe for use in file paths.
+// It rejects names containing path separators or traversal sequences.
+func ValidateAppName(name string) error {
+	if name == "" {
+		return fmt.Errorf("app name must not be empty")
+	}
+	if strings.Contains(name, "/") || strings.Contains(name, "\\") || strings.Contains(name, "..") {
+		return fmt.Errorf("invalid app name %q: must not contain '/', '\\', or '..'", name)
+	}
+	// After joining, the result must still be under BaseDir.
+	joined := filepath.Join(BaseDir(), name)
+	rel, err := filepath.Rel(BaseDir(), joined)
+	if err != nil || strings.HasPrefix(rel, "..") {
+		return fmt.Errorf("invalid app name %q: path escapes base directory", name)
+	}
+	return nil
+}
+
 // AppDir returns the directory for a specific app.
 func AppDir(appName string) string {
 	return filepath.Join(BaseDir(), appName)
@@ -579,8 +597,15 @@ func IsSpecialWarning(appName string) string {
 	return ""
 }
 
-// isPortInUse checks if a port is in use (cross-platform).
+// portInUseBy checks if a port is in use (cross-platform).
 func portInUseBy(port string) string {
+	// Validate port is purely numeric to prevent shell injection.
+	for _, c := range port {
+		if c < '0' || c > '9' {
+			return ""
+		}
+	}
+
 	// Try lsof (macOS/Linux) — gives process name
 	out, err := util.RunCmd("sh", "-c",
 		fmt.Sprintf("lsof -i :%s -sTCP:LISTEN 2>/dev/null | grep LISTEN | head -1 || true", port))
@@ -675,6 +700,8 @@ func Install(app App, opts InstallOptions) error {
 	defer f.Close()
 
 	if err := tmpl.Execute(f, ctx); err != nil {
+		f.Close()
+		os.Remove(composeFile)
 		return fmt.Errorf("failed to render compose file: %w", err)
 	}
 
@@ -694,6 +721,9 @@ func Install(app App, opts InstallOptions) error {
 
 // Uninstall stops the app and removes its containers.
 func Uninstall(appName string) error {
+	if err := ValidateAppName(appName); err != nil {
+		return err
+	}
 	appDir := GetInstalledPath(appName)
 	composeFile := filepath.Join(appDir, "docker-compose.yml")
 
@@ -711,6 +741,9 @@ func Uninstall(appName string) error {
 
 // Purge removes the app directory including data.
 func Purge(appName string) error {
+	if err := ValidateAppName(appName); err != nil {
+		return err
+	}
 	appDir := GetInstalledPath(appName)
 	if err := Uninstall(appName); err != nil {
 		return err
@@ -733,6 +766,9 @@ func Purge(appName string) error {
 
 // Status checks if the installed app is running.
 func Status(appName string) (string, error) {
+	if err := ValidateAppName(appName); err != nil {
+		return "", err
+	}
 	appDir := GetInstalledPath(appName)
 	composeFile := filepath.Join(appDir, "docker-compose.yml")
 

internal/install/install_test.go

@@ -800,6 +800,64 @@ func TestValidatePort(t *testing.T) {
 	}
 }
 
+func TestValidateAppName(t *testing.T) {
+	tests := []struct {
+		name    string
+		wantErr bool
+	}{
+		{"uptime-kuma", false},
+		{"my-app", false},
+		{"app123", false},
+		{"", true},
+		{"../../etc", true},
+		{"../foo", true},
+		{"foo/bar", true},
+		{"foo\\bar", true},
+		{"..", true},
+		{"foo/../bar", true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := ValidateAppName(tt.name)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("ValidateAppName(%q) error=%v, wantErr=%v", tt.name, err, tt.wantErr)
+			}
+		})
+	}
+}
+
+func TestPathTraversalBlocked(t *testing.T) {
+	// Uninstall, Purge, Status should reject path traversal
+	traversalNames := []string{"../../etc/passwd", "../foo", "foo/../../bar"}
+	for _, name := range traversalNames {
+		if _, err := Status(name); err == nil {
+			t.Errorf("Status(%q) should return error", name)
+		}
+		if err := Uninstall(name); err == nil {
+			t.Errorf("Uninstall(%q) should return error", name)
+		}
+		if err := Purge(name); err == nil {
+			t.Errorf("Purge(%q) should return error", name)
+		}
+	}
+}
+
+func TestPortInUseByRejectsNonNumeric(t *testing.T) {
+	// Non-numeric port strings should return empty (no injection)
+	injections := []string{
+		";rm -rf /",
+		"$(whoami)",
+		"80; cat /etc/passwd",
+		"80`id`",
+	}
+	for _, p := range injections {
+		result := portInUseBy(p)
+		if result != "" {
+			t.Errorf("portInUseBy(%q) should return empty for non-numeric input, got %q", p, result)
+		}
+	}
+}
+
 func TestInstallDryRun(t *testing.T) {
 	tmpDir, _ := os.MkdirTemp("", "homebutler-dryrun-*")
 	defer os.RemoveAll(tmpDir)