Add HasEnvSource to ResolvedConfig

Amolith created 

The snapshot loader was silently falling back to manual ID entry for
presets that configure the repository via _COMMAND environ keys (e.g.
RESTIC_REPOSITORY_COMMAND). This happened because hasRepoSource checked
for RESTIC_REPOSITORY in the environ map but not for
RESTIC_REPOSITORY_COMMAND, which keld resolves to RESTIC_REPOSITORY at
execution time.

Add HasEnvSource(key) to ResolvedConfig in the config package, which
checks the environ map for both the direct key and the _COMMAND variant,
plus the process environment as a fallback. This centralises the
_COMMAND convention in one place so callers never need to remember it.

Simplify hasRepoSource in the restic package to delegate to HasEnvSource
for environ checks, replacing the hand-rolled map lookups and os.Getenv
calls.

Change summary

internal/config/config.go         | 31 +++++++++++
internal/config/resolved_test.go  | 87 +++++++++++++++++++++++++++++++++
internal/restic/list_snapshots.go | 20 +------
3 files changed, 121 insertions(+), 17 deletions(-)

Detailed changes

internal/config/config.go 🔗 

@@ -63,6 +63,37 @@ func (rc *ResolvedConfig) HasFlag(name string) bool {
 	return false
 }
 
+// commandSuffix is the suffix keld recognises in .environ keys to indicate
+// that the value is a shell command whose stdout should be captured and
+// used as the actual variable value (with the suffix stripped).
+//
+// For example, RESTIC_REPOSITORY_COMMAND = "op read ..." resolves to
+// RESTIC_REPOSITORY at execution time.
+const commandSuffix = "_COMMAND"
+
+// HasEnvSource reports whether the given environment variable will have
+// a value when the restic process runs. It checks three sources in
+// order:
+//
+//  1. Direct key in [ResolvedConfig.Environ] (e.g. RESTIC_REPOSITORY)
+//  2. _COMMAND variant in Environ (e.g. RESTIC_REPOSITORY_COMMAND),
+//     which keld resolves before exec
+//  3. Process environment via [os.Getenv]
+//
+// This centralises the _COMMAND convention so callers never need to
+// check for it themselves.
+func (rc *ResolvedConfig) HasEnvSource(key string) bool {
+	if rc.Environ != nil {
+		if _, ok := rc.Environ[key]; ok {
+			return true
+		}
+		if _, ok := rc.Environ[key+commandSuffix]; ok {
+			return true
+		}
+	}
+	return os.Getenv(key) != ""
+}
+
 // rawConfig is the entire parsed TOML file as nested string-keyed maps.
 type rawConfig map[string]any

internal/config/resolved_test.go 🔗 

@@ -0,0 +1,87 @@
+package config
+
+import (
+	"testing"
+)
+
+func TestHasEnvSource(t *testing.T) {
+	tests := []struct {
+		name    string
+		environ map[string]string
+		envKey  string
+		procEnv map[string]string // simulated process env
+		want    bool
+	}{
+		{
+			name:    "direct key present",
+			environ: map[string]string{"RESTIC_REPOSITORY": "s3:bucket"},
+			envKey:  "RESTIC_REPOSITORY",
+			want:    true,
+		},
+		{
+			name:    "command variant present",
+			environ: map[string]string{"RESTIC_REPOSITORY_COMMAND": "op read ..."},
+			envKey:  "RESTIC_REPOSITORY",
+			want:    true,
+		},
+		{
+			name:    "neither direct nor command",
+			environ: map[string]string{"OTHER_VAR": "value"},
+			envKey:  "RESTIC_REPOSITORY",
+			want:    false,
+		},
+		{
+			name:    "nil environ",
+			environ: nil,
+			envKey:  "RESTIC_REPOSITORY",
+			want:    false,
+		},
+		{
+			name:    "empty string value still counts as present",
+			environ: map[string]string{"RESTIC_REPOSITORY": ""},
+			envKey:  "RESTIC_REPOSITORY",
+			want:    true,
+		},
+		{
+			name:    "command variant for non-restic key",
+			environ: map[string]string{"AWS_ACCESS_KEY_ID_COMMAND": "op read ..."},
+			envKey:  "AWS_ACCESS_KEY_ID",
+			want:    true,
+		},
+		{
+			name:    "process env fallback",
+			environ: nil,
+			envKey:  "KELD_TEST_HAS_ENV_SOURCE",
+			procEnv: map[string]string{"KELD_TEST_HAS_ENV_SOURCE": "from-process"},
+			want:    true,
+		},
+		{
+			name:    "process env empty when not set",
+			environ: nil,
+			envKey:  "KELD_TEST_HAS_ENV_SOURCE",
+			want:    false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Tests using t.Setenv cannot run in parallel.
+			if len(tt.procEnv) == 0 {
+				t.Parallel()
+			}
+
+			for k, v := range tt.procEnv {
+				t.Setenv(k, v)
+			}
+
+			cfg := &ResolvedConfig{
+				Environ: tt.environ,
+			}
+
+			got := cfg.HasEnvSource(tt.envKey)
+			if got != tt.want {
+				t.Errorf("HasEnvSource(%q) = %v, want %v", tt.envKey, got, tt.want)
+			}
+		})
+	}
+}

internal/restic/list_snapshots.go 🔗 

@@ -5,7 +5,6 @@ import (
 	"errors"
 	"fmt"
 	"maps"
-	"os"
 	"os/exec"
 	"sort"
 
@@ -51,8 +50,7 @@ var globalFlags = map[string]bool{
 
 // hasRepoSource reports whether a repository location is available from
 // any source: CLI flags in the resolved config, the config's environ
-// section, or the process environment (e.g. RESTIC_REPOSITORY already
-// set in the user's shell).
+// section (including _COMMAND variants), or the process environment.
 func hasRepoSource(cfg *config.ResolvedConfig) bool {
 	for _, f := range cfg.Flags {
 		switch f.Name {
@@ -60,20 +58,8 @@ func hasRepoSource(cfg *config.ResolvedConfig) bool {
 			return true
 		}
 	}
-	if cfg.Environ != nil {
-		if _, ok := cfg.Environ["RESTIC_REPOSITORY"]; ok {
-			return true
-		}
-		if _, ok := cfg.Environ["RESTIC_REPOSITORY_FILE"]; ok {
-			return true
-		}
-	}
-	// Check the process environment as a last resort — the user may
-	// have RESTIC_REPOSITORY set in their shell outside of keld config.
-	if os.Getenv("RESTIC_REPOSITORY") != "" || os.Getenv("RESTIC_REPOSITORY_FILE") != "" {
-		return true
-	}
-	return false
+	return cfg.HasEnvSource("RESTIC_REPOSITORY") ||
+		cfg.HasEnvSource("RESTIC_REPOSITORY_FILE")
 }
 
 // buildSnapshotCmd constructs the argument vector for running