Refuse empty folder paths and name Starr skip errors by instance key

pkg/folders/folder_test.go

@@ -1,8 +1,11 @@
 package folders
 
 import (
+	"errors"
+	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 )
@@ -13,6 +16,53 @@ func (noopLogger) Printf(string, ...any) {}
 func (noopLogger) Errorf(string, ...any) {}
 func (noopLogger) Debugf(string, ...any) {}
 
+type captureLogger struct {
+	errs []string
+}
+
+func (c *captureLogger) Printf(string, ...any) {}
+func (c *captureLogger) Debugf(string, ...any) {}
+func (c *captureLogger) Errorf(msg string, v ...any) {
+	c.errs = append(c.errs, fmt.Sprintf(msg, v...))
+}
+
+func TestValidateListRequiresPath(t *testing.T) {
+	t.Parallel()
+
+	err := ValidateList([]*FolderConfig{{DeleteOrig: true}}, func(string) (uint64, bool, error) {
+		return 0, false, nil
+	})
+	if !errors.Is(err, ErrNoPath) {
+		t.Fatalf("empty path: %v", err)
+	}
+}
+
+func TestCheckSkipsEmptyPath(t *testing.T) {
+	t.Parallel()
+
+	cwd, err := os.Getwd()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	log := &captureLogger{}
+
+	good, names := Check([]*FolderConfig{{Path: "", DeleteOrig: true}}, log)
+	if len(good) != 0 || len(names) != 0 {
+		t.Fatalf("watched empty path: %v %v", names, good)
+	}
+
+	for _, name := range names {
+		if name == cwd {
+			t.Fatal("empty path must not watch the working directory")
+		}
+	}
+
+	if len(log.errs) == 0 || !strings.Contains(log.errs[0], "empty path") {
+		t.Fatalf("log %v", log.errs)
+	}
+}
+
 func TestNormalizeExcludePaths(t *testing.T) {
 	t.Parallel()
 

pkg/folders/path.go

@@ -26,6 +26,11 @@ func Check(list []*FolderConfig, log Logs) ([]*FolderConfig, []string) {
 	)
 
 	for _, folder := range list {
+		if strings.TrimSpace(folder.Path) == "" {
+			log.Errorf("Folder (empty path): cannot watch")
+			continue
+		}
+
 		folder.Path, err = filepath.Abs(expandHomedir(folder.Path))
 		if err != nil {
 			log.Errorf("Folder '%s' (bad path): %v", folder.Path, err)

pkg/folders/validate.go

@@ -3,20 +3,28 @@ package folders
 import (
 	"errors"
 	"fmt"
+	"strings"
 
 	"golift.io/cnfg"
 )
 
 // ErrNilConfig is returned when a folder list contains a nil entry.
 var ErrNilConfig = errors.New("nil config entry")
 
+// ErrNoPath is returned when a folder instance has no watch path.
+var ErrNoPath = errors.New("path is required")
+
 // ValidateList applies folder defaults and parses max_bytes.
 func ValidateList(list []*FolderConfig, parseMax func(string) (uint64, bool, error)) error {
 	for idx := range list {
 		if list[idx] == nil {
 			return ErrNilConfig
 		}
 
+		if strings.TrimSpace(list[idx].Path) == "" {
+			return ErrNoPath
+		}
+
 		if list[idx].DeleteAfter == nil {
 			list[idx].DeleteAfter = &cnfg.Duration{Duration: DefaultDeleteAfter}
 		}

pkg/unpackerr/cnfgfile.go

@@ -451,7 +451,7 @@ func expandHomedir(filePath string) string {
 	return expanded
 }
 
-func (u *Unpackerr) validateApp(conf *StarrConfig, app starr.App) error {
+func (u *Unpackerr) validateApp(conf *StarrConfig, app starr.App, slug string) error {
 	conf.Name = strings.TrimSpace(conf.Name)
 
 	if err := checkStarrName(conf.Name); err != nil {
@@ -460,7 +460,7 @@ func (u *Unpackerr) validateApp(conf *StarrConfig, app starr.App) error {
 
 	label := conf.Label(app)
 
-	if err := u.requireStarrAccess(conf, label); err != nil {
+	if err := u.requireStarrAccess(conf, label, slug); err != nil {
 		return err
 	}
 
@@ -502,14 +502,14 @@ func (u *Unpackerr) validateApp(conf *StarrConfig, app starr.App) error {
 	return nil
 }
 
-func (u *Unpackerr) requireStarrAccess(conf *StarrConfig, label string) error {
+func (u *Unpackerr) requireStarrAccess(conf *StarrConfig, label, slug string) error {
 	if conf.URL == "" {
-		u.Errorf("Missing %s URL in one of your configurations, skipped and ignored.", label)
+		u.Errorf("Missing %s URL in instance %q, skipped and ignored.", label, slug)
 		return ErrInvalidURL // this error is not printed.
 	}
 
 	if conf.APIKey == "" {
-		u.Errorf("Missing %s API Key in one of your configurations, skipped and ignored.", label)
+		u.Errorf("Missing %s API Key in instance %q, skipped and ignored.", label, slug)
 		return ErrInvalidKey // this error is not printed at startup; PUT returns it.
 	}
 
@@ -518,8 +518,8 @@ func (u *Unpackerr) requireStarrAccess(conf *StarrConfig, label string) error {
 	}
 
 	if len(conf.APIKey) < apiKeyMinLength {
-		u.Errorf("%s (%s) API Key is too short (%d < %d), skipped and ignored.",
-			label, conf.URL, len(conf.APIKey), apiKeyMinLength)
+		u.Errorf("%s instance %q (%s) API Key is too short (%d < %d), skipped and ignored.",
+			label, slug, conf.URL, len(conf.APIKey), apiKeyMinLength)
 
 		return fmt.Errorf("%s (%s) %w, your key length: %d",
 			label, conf.URL, ErrInvalidKey, len(conf.APIKey))

pkg/unpackerr/cnfgfile_test.go

@@ -1,6 +1,7 @@
 package unpackerr
 
 import (
+	"bytes"
 	"encoding/json"
 	"errors"
 	"os"
@@ -765,7 +766,7 @@ func TestValidateAppUsesInstanceLabel(t *testing.T) {
 	conf.URL = "ftp://127.0.0.1:8989"
 	conf.APIKey = key
 
-	err := unpack.validateApp(conf, starr.Sonarr)
+	err := unpack.validateApp(conf, starr.Sonarr, "uhd")
 	if err == nil || !strings.Contains(err.Error(), "Sportarr") {
 		t.Fatalf("invalid URL: %v", err)
 	}
@@ -774,7 +775,7 @@ func TestValidateAppUsesInstanceLabel(t *testing.T) {
 	conf.URL = "http://127.0.0.1:8989"
 	conf.APIKey = "short"
 
-	err = unpack.validateApp(conf, starr.Sonarr)
+	err = unpack.validateApp(conf, starr.Sonarr, "uhd")
 	if err == nil || !strings.Contains(err.Error(), "Sportarr") {
 		t.Fatalf("short key: %v", err)
 	}
@@ -783,12 +784,41 @@ func TestValidateAppUsesInstanceLabel(t *testing.T) {
 	conf.URL = "http://127.0.0.1:8989"
 	conf.APIKey = key
 
-	err = unpack.validateApp(conf, starr.Sonarr)
+	err = unpack.validateApp(conf, starr.Sonarr, "uhd")
 	if err == nil || !strings.Contains(err.Error(), "Sportarr") {
 		t.Fatalf("max bytes: %v", err)
 	}
 }
 
+func TestValidateAppSkipLogsInstanceSlug(t *testing.T) {
+	t.Parallel()
+
+	var logs bytes.Buffer
+
+	unpack := New()
+	unpack.Error.SetOutput(&logs)
+
+	err := unpack.validateApp(&StarrConfig{}, starr.Readarr, "0")
+	if !errors.Is(err, ErrInvalidURL) {
+		t.Fatalf("empty URL: %v", err)
+	}
+
+	if !strings.Contains(logs.String(), `instance "0"`) {
+		t.Fatalf("skip log missing slug: %q", logs.String())
+	}
+
+	logs.Reset()
+
+	err = unpack.validateApp(&StarrConfig{URL: "http://127.0.0.1:8787"}, starr.Readarr, "books")
+	if !errors.Is(err, ErrInvalidKey) {
+		t.Fatalf("empty key: %v", err)
+	}
+
+	if !strings.Contains(logs.String(), `instance "books"`) {
+		t.Fatalf("skip log missing slug: %q", logs.String())
+	}
+}
+
 func TestValidateStarrListRejectsBadName(t *testing.T) {
 	t.Parallel()
 
@@ -804,6 +834,10 @@ func TestValidateStarrListRejectsBadName(t *testing.T) {
 		t.Fatalf("got %v want ErrInvalidName", err)
 	}
 
+	if !strings.Contains(err.Error(), `instance "0"`) {
+		t.Fatalf("fatal error missing slug: %v", err)
+	}
+
 	if len(unpack.Sonarr) != 1 {
 		t.Fatalf("bad name must not skip the instance, got %d", len(unpack.Sonarr))
 	}

pkg/unpackerr/configput.go

@@ -722,7 +722,7 @@ func putStarrList[T any, P starrApp[T]](
 		}
 
 		server := asStarr[T, P](item)
-		if err := unpackerr.validateApp(server.conf(), app); err != nil {
+		if err := unpackerr.validateApp(server.conf(), app, key); err != nil {
 			// Env overlay may recreate a slug the PUT omitted. Incomplete
 			// leftovers (URL in env, key in the deleted file row) must not
 			// 400 the save; startup already skips those. A PUT-body row
@@ -809,6 +809,8 @@ func (u *Unpackerr) putFolders(raw json.RawMessage) (bool, error) {
 		return false, err
 	}
 
+	dropInvalidEnvOverlay(next.Folder, preview.Folders, validateFolderList)
+
 	if err := validateFolderList(preview.Folders); err != nil {
 		return false, err
 	}

pkg/unpackerr/configput_test.go

@@ -499,6 +499,11 @@ func TestConfigPutFoldersValidationDoesNotApply(t *testing.T) {
 	if len(unpack.fileConfig.Folders) != 0 {
 		t.Fatalf("rejected folder staged: %+v", unpack.fileConfig.Folders)
 	}
+
+	emptyPath := `{"interval":"1s","buffer":1000,"folder":{"foo2":{"delete_original":true}}}`
+	if rec := doAuth(t, unpack, http.MethodPut, "/api/config/folders", emptyPath, key); rec.Code != http.StatusBadRequest {
+		t.Fatalf("empty folder path %d %s", rec.Code, rec.Body.String())
+	}
 }
 
 func TestConfigPutWebhooksValidationDoesNotApply(t *testing.T) {
@@ -1984,6 +1989,53 @@ func TestConfigPutFoldersKeepsSiblingExcludePaths(t *testing.T) {
 	}
 }
 
+// Env DELETE_ORIGINAL without a path must not 400 a save of a different slug.
+func TestConfigPutFoldersOtherSlugWhenEnvHasNoPath(t *testing.T) {
+	t.Setenv("UN_FOLDER_foo2_DELETE_ORIGINAL", "true")
+
+	unpack := testAuthUnpackerr(t)
+	unpack.ConfigFile = filepath.Join(t.TempDir(), "unpackerr.conf")
+	unpack.snapshotFileConfig()
+
+	res, err := cnfg.ParseENV(unpack.Config, unpack.EnvPrefix)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	unpack.envUsed = envSuffixes(res.Used, unpack.EnvPrefix)
+
+	watch := t.TempDir()
+
+	body, err := json.Marshal(map[string]any{
+		"interval": "1s",
+		"buffer":   1000,
+		"folder": map[string]any{
+			"tv": map[string]any{"path": watch},
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	put := doAuth(t, unpack, http.MethodPut, "/api/config/folders", string(body), putKey(unpack))
+	if put.Code != http.StatusOK {
+		t.Fatalf("put %d %s", put.Code, put.Body.String())
+	}
+
+	if unpack.fileConfig.Folders["foo2"] != nil {
+		t.Fatalf("env slug written to file: %+v", unpack.fileConfig.Folders)
+	}
+
+	if unpack.Folders["foo2"] != nil {
+		t.Fatalf("incomplete env leftover on live: %+v", unpack.Folders["foo2"])
+	}
+
+	got := unpack.Folders["tv"]
+	if got == nil || got.Path != watch {
+		t.Fatalf("live tv %+v", got)
+	}
+}
+
 func envSonarrURLUnpackerr(t *testing.T, secret string) *Unpackerr {
 	t.Helper()
 

pkg/unpackerr/folder.go

@@ -20,10 +20,18 @@ func (u *Unpackerr) validateFolders() error {
 }
 
 func validateFolderList(list InstanceMap[FolderConfig]) error {
-	for key := range list {
+	for key, folder := range list {
 		if err := validateInstanceSlug(key); err != nil {
 			return err
 		}
+
+		if folder == nil {
+			return errNilConfigEntry
+		}
+
+		if strings.TrimSpace(folder.Path) == "" {
+			return fmt.Errorf("folder %q: %w", key, folders.ErrNoPath)
+		}
 	}
 
 	if err := folders.ValidateList(instanceValues(list), parseOptionalMaxBytes); err != nil {

pkg/unpackerr/starrpoll.go

@@ -31,13 +31,13 @@ func validateStarrList[T any, P starrApp[T]](unpack *Unpackerr, list InstanceMap
 		}
 
 		server := asStarr[T, P](item)
-		if err := unpack.validateApp(server.conf(), app); err != nil {
+		if err := unpack.validateApp(server.conf(), app, key); err != nil {
 			if skipInvalidApp(err) {
 				delete(list, key)
 				continue
 			}
 
-			return err
+			return fmt.Errorf("%s instance %q: %w", app, key, err)
 		}
 
 		server.connect()
@@ -49,7 +49,11 @@ func validateStarrList[T any, P starrApp[T]](unpack *Unpackerr, list InstanceMap
 func warnDuplicateStarrNames[T any, P starrApp[T]](
 	unpack *Unpackerr, seen map[string]string, app starr.App, list InstanceMap[T],
 ) {
-	for _, item := range instanceValues(list) {
+	for slug, item := range list {
+		if item == nil {
+			continue
+		}
+
 		server := asStarr[T, P](item)
 		cfg := server.conf()
 		name := strings.TrimSpace(cfg.Name)
@@ -58,17 +62,17 @@ func warnDuplicateStarrNames[T any, P starrApp[T]](
 			continue
 		}
 
-		key := strings.ToLower(name)
-		loc := fmt.Sprintf("%s (%s)", app, cfg.URL)
+		dupKey := strings.ToLower(name)
+		loc := fmt.Sprintf("%s instance %q (%s)", app, slug, cfg.URL)
 
-		if prev, ok := seen[key]; ok {
+		if prev, ok := seen[dupKey]; ok {
 			unpack.Errorf("Config Warning: duplicate Starr instance name %q on %s and %s; hook exclude cannot tell them apart",
 				name, prev, loc)
 
 			continue
 		}
 
-		seen[key] = loc
+		seen[dupKey] = loc
 	}
 }