feat(config): unify env and forced_env with interpolation support (#6)

* feat(config): unify env and forced_env with interpolation support

Merge `env` and `forced_env` into a single `env` key with three value types:
- Exact credential reference: value matches a defined credential name
- Template interpolation: `{{ credential-name }}` substituted inline
- Literal value: no credential refs, used as-is

All env entries are now admin-controlled (forced) and cannot be overridden
by client requests.

Changes:
- Add interpolate.go with FindCredentialRefs, Interpolate, ResolveEnvValue
- Update config validation to support unified env with template refs
- Update executor to use ResolveEnvValue for all three value types
- Deprecate forced_env (still works with warning)
- Update README with new env syntax documentation

BREAKING: None - forced_env still works (deprecated)

* fix: address PR review comments

- Remove unused NormalizeCredentialRef function (dead code)
- Improve error message to include credential name for debugging

* fix(config): remove unused IsExactCredentialRef function

Dead code - logic already inlined in ClassifyEnvValue.

Author: dedene

README.md

@@ -211,22 +211,34 @@ tools:
 By default, blocked patterns run in `arg` mode (each argument is matched independently). Use
 `match: command` when a regex needs to span multiple args (for example `repo\\s+delete`).
 
-### Forced environment variables
+### Environment variables
 
-Variables that are always set and cannot be overridden by the agent:
+The `env` key supports three value types — all entries are admin-controlled and cannot be overridden by the agent:
 
 ```yaml
+credentials:
+  gog-keyring-password:
+    source: pass:gog/keyring
+  db-password:
+    source: op://vault/db/password
+
 tools:
   gog:
     binary: /home/linuxbrew/.linuxbrew/bin/gog
     env:
+      # Credential reference: value matches a defined credential name
       GOG_KEYRING_PASSWORD: gog-keyring-password
-    forced_env:
+
+      # Template interpolation: {{ name }} substituted inline
+      DATABASE_URL: "postgres://app:{{ db-password }}@localhost/mydb"
+
+      # Literal value: no credential refs, used as-is
       GOG_ENABLE_COMMANDS: 'gmail,calendar,drive,tasks,contacts,keep,time'
 ```
 
-The agent cannot change `GOG_ENABLE_COMMANDS` — it's stripped from inherited environment and set by
-the daemon.
+The agent cannot override any `env` entry — values are stripped from inherited environment and set by the daemon.
+
+> **Deprecated:** `forced_env` is deprecated. Use `env` instead — literal values (without credential refs) work the same way.
 
 ### Output redaction
 

internal/config/config.go

@@ -171,9 +171,11 @@ type CredentialDef struct {
 
 // ToolDef defines a wrapped tool.
 type ToolDef struct {
-	Binary       string            `yaml:"binary"`
-	Timeout      string            `yaml:"timeout,omitempty"`
-	Env          map[string]string `yaml:"env,omitempty"`
+	Binary   string            `yaml:"binary"`
+	Timeout  string            `yaml:"timeout,omitempty"`
+	Env      map[string]string `yaml:"env,omitempty"` // Unified env: credential refs, {{ interpolation }}, or literals
+	// Deprecated: Use Env instead. ForcedEnv values are always treated as literals.
+	// Will be removed in a future version.
 	ForcedEnv    map[string]string `yaml:"forced_env,omitempty"`
 	Mode         string            `yaml:"mode,omitempty"` // "blocklist" (default) or "allowlist"
 	BlockedArgs  []BlockedArg      `yaml:"blocked_args,omitempty"`
@@ -282,15 +284,24 @@ func (c *Config) Validate() error {
 			log.Printf("[WARN] tool %q: binary %q not found on disk: %v", toolName, tool.Binary, err)
 		}
 
-		for envVar, credName := range tool.Env {
+		// Build credential names set for validation
+		credNames := CredentialNamesSet(c.Credentials)
+
+		// Validate env entries (unified: credential refs, {{ interpolation }}, or literals)
+		for envVar, value := range tool.Env {
 			if !envVarNameRegex.MatchString(envVar) {
 				return fmt.Errorf("tool %q: invalid env var name %q", toolName, envVar)
 			}
-			if _, ok := c.Credentials[credName]; !ok {
-				return fmt.Errorf("tool %q: references undefined credential %q", toolName, credName)
+			// Validate any credential references in the value
+			if missing := ValidateEnvRefs(value, credNames); len(missing) > 0 {
+				return fmt.Errorf("tool %q: env %q references undefined credential(s): %v", toolName, envVar, missing)
 			}
 		}
 
+		// Validate forced_env (deprecated) - emit warning and validate var names
+		if len(tool.ForcedEnv) > 0 {
+			log.Printf("[WARN] tool %q: forced_env is deprecated, use env instead (values without credential refs are treated as literals)", toolName)
+		}
 		for envVar := range tool.ForcedEnv {
 			if !envVarNameRegex.MatchString(envVar) {
 				return fmt.Errorf("tool %q: invalid forced_env var name %q", toolName, envVar)

internal/config/config_test.go

@@ -806,24 +806,43 @@ func TestValidate_EmptyBinary(t *testing.T) {
 }
 
 func TestValidate_MissingCredentialRef(t *testing.T) {
+	// With unified env, plain values are literals (allowed).
+	// Template refs {{ name }} must reference defined credentials.
 	cfg := &Config{
 		Credentials: map[string]CredentialDef{}, // no credentials defined
 		Tools: map[string]ToolDef{
 			"gh": {
 				Binary: "/usr/bin/gh",
-				Env:    map[string]string{"GH_TOKEN": "nonexistent-cred"},
+				Env:    map[string]string{"GH_TOKEN": "{{ nonexistent-cred }}"},
 			},
 		},
 	}
 	err := cfg.Validate()
 	if err == nil {
-		t.Fatal("Validate() should reject undefined credential ref")
+		t.Fatal("Validate() should reject undefined credential ref in template")
 	}
 	if !strings.Contains(err.Error(), "undefined credential") {
 		t.Errorf("error = %v, want 'undefined credential'", err)
 	}
 }
 
+func TestValidate_LiteralEnvValue(t *testing.T) {
+	// Plain values without {{ refs }} are treated as literals (allowed).
+	cfg := &Config{
+		Credentials: map[string]CredentialDef{},
+		Tools: map[string]ToolDef{
+			"gh": {
+				Binary: "/usr/bin/gh",
+				Env:    map[string]string{"PATH": "/usr/bin:/bin"},
+			},
+		},
+	}
+	err := cfg.Validate()
+	if err != nil {
+		t.Errorf("Validate() unexpected error for literal env value: %v", err)
+	}
+}
+
 func TestValidate_ValidCredentialRef(t *testing.T) {
 	cfg := &Config{
 		Credentials: map[string]CredentialDef{

internal/config/interpolate.go

@@ -0,0 +1,138 @@
+// Package config handles loading and parsing the wrappers.yaml configuration.
+package config
+
+import (
+	"fmt"
+	"regexp"
+)
+
+// credentialNameRe matches valid credential names in {{ name }} templates.
+// More restrictive than the existing credentialRefRe: only allows valid credential name chars.
+var credentialNameRe = regexp.MustCompile(`\{\{\s*([a-zA-Z0-9_-]+)\s*\}\}`)
+
+// FindCredentialRefs extracts all credential reference names from a template string.
+// Returns unique names in order of first appearance.
+// Example: "prefix:{{ foo }}:{{ bar }}:{{ foo }}" → ["foo", "bar"]
+func FindCredentialRefs(value string) []string {
+	matches := credentialNameRe.FindAllStringSubmatch(value, -1)
+	if len(matches) == 0 {
+		return nil
+	}
+
+	seen := make(map[string]bool)
+	refs := make([]string, 0, len(matches))
+	for _, m := range matches {
+		if len(m) > 1 {
+			name := m[1]
+			if !seen[name] {
+				seen[name] = true
+				refs = append(refs, name)
+			}
+		}
+	}
+	return refs
+}
+
+// HasCredentialRefs returns true if the value contains any {{ name }} templates.
+func HasCredentialRefs(value string) bool {
+	return credentialNameRe.MatchString(value)
+}
+
+// Interpolate replaces all {{ name }} templates in value with resolved credentials.
+// The resolver function is called for each unique credential name.
+// Returns error if any credential resolution fails.
+func Interpolate(value string, resolver func(name string) (string, error)) (string, error) {
+	refs := FindCredentialRefs(value)
+	if len(refs) == 0 {
+		return value, nil
+	}
+
+	// Resolve all unique credentials first
+	resolved := make(map[string]string, len(refs))
+	for _, name := range refs {
+		secret, err := resolver(name)
+		if err != nil {
+			return "", fmt.Errorf("credential %q: %w", name, err)
+		}
+		resolved[name] = secret
+	}
+
+	// Replace all occurrences
+	result := credentialNameRe.ReplaceAllStringFunc(value, func(match string) string {
+		// Extract name from match (handles whitespace)
+		m := credentialNameRe.FindStringSubmatch(match)
+		if len(m) > 1 {
+			return resolved[m[1]]
+		}
+		return match
+	})
+
+	return result, nil
+}
+
+// ClassifyEnvValue determines how an env value should be resolved:
+//   - "credential": value is an exact credential name → fetch entire value
+//   - "interpolate": value contains {{ refs }} → interpolate
+//   - "literal": no credential refs → use as-is
+func ClassifyEnvValue(value string, credentialNames map[string]struct{}) string {
+	// Check exact match first
+	if _, exists := credentialNames[value]; exists {
+		return "credential"
+	}
+	// Check for template refs
+	if HasCredentialRefs(value) {
+		return "interpolate"
+	}
+	return "literal"
+}
+
+// ValidateEnvRefs validates that all credential references in an env value exist.
+// For exact credential matches, checks the value itself.
+// For interpolated values, checks all {{ name }} refs.
+// Returns list of missing credential names, or nil if all valid.
+func ValidateEnvRefs(value string, credentialNames map[string]struct{}) []string {
+	classification := ClassifyEnvValue(value, credentialNames)
+
+	switch classification {
+	case "credential":
+		// Already validated by ClassifyEnvValue returning "credential"
+		return nil
+	case "interpolate":
+		refs := FindCredentialRefs(value)
+		var missing []string
+		for _, ref := range refs {
+			if _, exists := credentialNames[ref]; !exists {
+				missing = append(missing, ref)
+			}
+		}
+		return missing
+	default:
+		return nil
+	}
+}
+
+// ResolveEnvValue resolves an env value to its final string.
+// Handles all three cases: exact credential, interpolated, and literal.
+func ResolveEnvValue(value string, credentialNames map[string]struct{}, resolver func(name string) (string, error)) (string, error) {
+	classification := ClassifyEnvValue(value, credentialNames)
+
+	switch classification {
+	case "credential":
+		return resolver(value)
+	case "interpolate":
+		return Interpolate(value, resolver)
+	default:
+		return value, nil
+	}
+}
+
+// CredentialNamesSet builds a set of credential names from a credentials map.
+// Helper to avoid repeatedly building this set.
+func CredentialNamesSet(credentials map[string]CredentialDef) map[string]struct{} {
+	names := make(map[string]struct{}, len(credentials))
+	for name := range credentials {
+		names[name] = struct{}{}
+	}
+	return names
+}
+