From 536ddad03ac0fe1fe05b2c40fed4d0217c5ac6ec Mon Sep 17 00:00:00 2001 From: Amolith Date: Fri, 10 Apr 2026 12:26:48 -0600 Subject: [PATCH] Validate required inputs in non-interactive mode When running non-interactively, keld now validates that required inputs are present before executing restic. Two cases checked: - Multiple presets with no --preset specified: returns an error listing available presets instead of silently using global defaults. - backup with no paths: returns an error directing user to pass paths as arguments or set _arguments in the preset. Task: td-H0EDND6 --- cmd/interactive.go | 2 +- cmd/interactive_test.go | 2 +- cmd/noninteractive_errors_test.go | 112 ++++++++++++++++++++++++++++++ cmd/root.go | 16 +++++ cmd/root_noninteractive_test.go | 2 +- cmd/subcommands_test.go | 3 +- 6 files changed, 132 insertions(+), 5 deletions(-) create mode 100644 cmd/noninteractive_errors_test.go diff --git a/cmd/interactive.go b/cmd/interactive.go index 436e56d6dd88ca13c0f78450d87f69311942d9ac..0178ffc550bd31f0ce4f676805bfcd3674b87957 100644 --- a/cmd/interactive.go +++ b/cmd/interactive.go @@ -20,4 +20,4 @@ func isInteractive() bool { return false } return isStdinTerminal() -} \ No newline at end of file +} diff --git a/cmd/interactive_test.go b/cmd/interactive_test.go index fe397446d44bbbfeb7a289184ae86f3147669e3a..476a03ddce95b9ec92b5a43cf320351a68b83243 100644 --- a/cmd/interactive_test.go +++ b/cmd/interactive_test.go @@ -71,4 +71,4 @@ func TestIsInteractive(t *testing.T) { } }) } -} \ No newline at end of file +} diff --git a/cmd/noninteractive_errors_test.go b/cmd/noninteractive_errors_test.go new file mode 100644 index 0000000000000000000000000000000000000000..b0ebe77531dbd35b8679cc5aa31e5328b42434b2 --- /dev/null +++ b/cmd/noninteractive_errors_test.go @@ -0,0 +1,112 @@ +package cmd + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestBackupNonInteractiveMissingPaths(t *testing.T) { + // Not parallel: mutates package-level isStdinTerminal. + + original := isStdinTerminal + t.Cleanup(func() { + isStdinTerminal = original + }) + isStdinTerminal = func() bool { return false } + + // Config with a valid preset but no backup paths. + dir := t.TempDir() + cfg := filepath.Join(dir, "config.toml") + err := os.WriteFile(cfg, []byte(` +[global] +repo = "/repos/default" + +["home@"] +tag = "home" + +["@cloud"] +repo = "/repos/cloud" +`), 0o600) + if err != nil { + t.Fatalf("writing fixture: %v", err) + } + t.Setenv("HOME", dir) + + setRootFlagValuesForTest(t, "home@cloud", true, cfg) + t.Setenv("KELD_CONFIG_FILE", "") + t.Setenv("KELD_DRYRUN", "") + + backup := lookupSubcommand(t, "backup") + _, err = captureStdout(t, func() error { + return backup.RunE(backup, []string{}) + }) + + if err == nil { + t.Fatal("expected error for backup with no paths, got nil") + } + + errMsg := err.Error() + // Should mention paths specifically. + if !strings.Contains(errMsg, "path") { + t.Fatalf("expected error to mention paths, got: %v", err) + } +} + +func TestBackupNonInteractiveMultiplePresetsNoSelection(t *testing.T) { + // Not parallel: mutates package-level isStdinTerminal. + + original := isStdinTerminal + t.Cleanup(func() { + isStdinTerminal = original + }) + isStdinTerminal = func() bool { return false } + + // Config with multiple presets (home@cloud, work@cloud). + dir := t.TempDir() + cfg := filepath.Join(dir, "config.toml") + err := os.WriteFile(cfg, []byte(` +[global] +repo = "/repos/default" + +["home@"] +tag = "home" + +["home@".backup] +_arguments = "/home" + +["work@"] +tag = "work" + +["work@".backup] +_arguments = "/work" + +["@cloud"] +repo = "/repos/cloud" +`), 0o600) + if err != nil { + t.Fatalf("writing fixture: %v", err) + } + t.Setenv("HOME", dir) + + // No --preset specified. + setRootFlagValuesForTest(t, "", true, cfg) + t.Setenv("KELD_CONFIG_FILE", "") + t.Setenv("KELD_DRYRUN", "") + + backup := lookupSubcommand(t, "backup") + _, err = captureStdout(t, func() error { + return backup.RunE(backup, []string{}) + }) + + if err == nil { + t.Fatal("expected error for multiple presets with no selection, got nil") + } + + errMsg := err.Error() + // Should mention multiple presets. + if !strings.Contains(errMsg, "preset") { + t.Fatalf("expected error to mention presets, got: %v", err) + } +} diff --git a/cmd/root.go b/cmd/root.go index 1cae4d1c4c20d50e0e474f5fff1e3b9a759574ad..d64bbe2d32905b4677c41accd43cb54f3b1488ff 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -100,6 +100,15 @@ func runCommand(commandName string, cmd *cobra.Command, rawArgs []string, sessio preset = *sessionPreset } + // In non-interactive mode with no preset and multiple presets + // available, require explicit --preset selection. + if !interactive && preset == "" { + presets := config.Presets() + if len(presets) > 1 { + return fmt.Errorf("keld: multiple presets defined (%s); specify --preset in non-interactive mode", strings.Join(presets, ", ")) + } + } + if preset != "" { if err := validatePreset(preset); err != nil { return err @@ -131,6 +140,13 @@ func runCommand(commandName string, cmd *cobra.Command, rawArgs []string, sessio return err } + // In non-interactive mode, validate that required inputs are present. + if !interactive { + if commandName == "backup" && len(cfg.Arguments) == 0 { + return fmt.Errorf("keld backup: no paths specified; pass paths as arguments or set _arguments in the preset") + } + } + restic.WarnUnknownFlags(cfg.Command, cfg.Flags) if config.IsDryRun() { diff --git a/cmd/root_noninteractive_test.go b/cmd/root_noninteractive_test.go index dc2409e22b120d559635870cfb2feaccdb533e63..e97df1db49c3c51a0078d1ec18ce0c4f65209e71 100644 --- a/cmd/root_noninteractive_test.go +++ b/cmd/root_noninteractive_test.go @@ -35,4 +35,4 @@ func TestRootRunNonInteractiveRequiresSubcommand(t *testing.T) { if !strings.Contains(errMsg, "subcommand") { t.Fatalf("expected error to mention subcommand, got: %v", err) } -} \ No newline at end of file +} diff --git a/cmd/subcommands_test.go b/cmd/subcommands_test.go index 3c6df4275af94c47b3f3d6fc18035902e52488cd..0b146e97e8f4d9bfae92153d2eb655ec84c0d4fc 100644 --- a/cmd/subcommands_test.go +++ b/cmd/subcommands_test.go @@ -27,7 +27,6 @@ func TestSubcommandNoArgsNonInteractiveRunsDirectly(t *testing.T) { out, err := captureStdout(t, func() error { return backup.RunE(backup, []string{}) }) - // Should NOT get a bubbletea TTY error. Instead, should either: // - Succeed with dry-run output (if config has paths), or // - Fail with a sensible error about missing inputs. @@ -112,4 +111,4 @@ func TestSubcommandNoArgsInteractiveEntersSession(t *testing.T) { if !strings.Contains(errMsg, "TTY") && !strings.Contains(errMsg, "tty") { t.Fatalf("expected bubbletea TTY error in interactive mode, got: %v", err) } -} \ No newline at end of file +}