Refuse YARDMASTER_* variables as provider key names

cmd/yardmaster/main_test.go

@@ -163,6 +163,20 @@ func TestSwitchyardEnvPassesOnlyWhatSwitchyardNeeds(t *testing.T) {
 			t.Errorf("env contains %q:\n%s", leak, joined)
 		}
 	}
+
+	// A hand-written config can't name YardMaster's own variables either.
+	os.WriteFile(config, []byte(`[llm_clients.evil]
+format = "openai_chat"
+base_url = "https://attacker.example/v1"
+api_key_env = "YARDMASTER_SECRET_KEY"
+`), 0o600)
+	env, err = switchyardEnv(keys)(config)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if joined := strings.Join(env, "\n"); strings.Contains(joined, "must-not-leak") {
+		t.Errorf("a config naming YARDMASTER_SECRET_KEY got its value:\n%s", joined)
+	}
 }
 
 func TestFirstAdmin(t *testing.T) {

internal/deploy/apply.go

@@ -119,6 +119,10 @@ func (a *Applier) Preview(ctx context.Context, candidate string) (*Preview, erro
 		return p, nil
 	}
 	for _, name := range parsed.KeyEnvNames() {
+		if !secrets.ValidName(name) {
+			p.Errors = fmt.Sprintf("api_key_env %q can't hold a provider key: use a variable name like OPENROUTER_API_KEY, outside YARDMASTER_*", name)
+			return p, nil
+		}
 		status, err := a.keys.Status(name)
 		if err != nil {
 			return nil, err

internal/deploy/deploy_test.go

@@ -6,6 +6,8 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+
+	"yardmaster/internal/secrets"
 )
 
 func f(v float64) *float64 { return &v }
@@ -224,3 +226,28 @@ func TestIsJev(t *testing.T) {
 		}
 	}
 }
+
+func TestYardMasterVariablesCantHoldKeys(t *testing.T) {
+	d := sample()
+	d.Clients[0].KeyEnv = "YARDMASTER_SECRET_KEY"
+	if err := d.Validate(); err == nil || !strings.Contains(err.Error(), "outside YARDMASTER_*") {
+		t.Errorf("wizard: %v", err)
+	}
+
+	keys, err := secrets.Open(t.TempDir(), "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	a := &Applier{keys: keys, configPath: filepath.Join(t.TempDir(), "config.toml")}
+	p, err := a.Preview(t.Context(), `[llm_clients.evil]
+format = "openai_chat"
+base_url = "https://attacker.example/v1"
+api_key_env = "YARDMASTER_SECRET_KEY"
+`)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(p.Errors, `"YARDMASTER_SECRET_KEY" can't hold a provider key`) {
+		t.Errorf("raw TOML: errors %q", p.Errors)
+	}
+}

internal/deploy/model.go

@@ -160,7 +160,7 @@ func (d *Deployment) Validate() error {
 		switch c.Auth {
 		case AuthKey:
 			if !secrets.ValidName(c.KeyEnv) {
-				add("provider %q: key variable must look like OPENROUTER_API_KEY", c.Name)
+				add("provider %q: key variable must look like OPENROUTER_API_KEY, outside YARDMASTER_*", c.Name)
 			}
 		case AuthNone:
 		default:

internal/secrets/secrets.go

@@ -60,8 +60,16 @@ const (
 
 var envNamePattern = regexp.MustCompile(`^[A-Z_][A-Z0-9_]{0,127}$`)
 
-// ValidName reports whether name can be used as an environment variable name.
-func ValidName(name string) bool { return envNamePattern.MatchString(name) }
+// reservedPrefix names YardMaster's own settings, among them its secrets (master key,
+// proxy secret, admin password). No provider key can be read from one.
+const reservedPrefix = "YARDMASTER_"
+
+// ValidName reports whether name can hold a provider key: an environment variable name
+// outside YARDMASTER_*. A config naming one of YardMaster's own variables would otherwise
+// hand its value to switchyard-server, which sends it to the provider's base URL.
+func ValidName(name string) bool {
+	return envNamePattern.MatchString(name) && !strings.HasPrefix(name, reservedPrefix)
+}
 
 // Store holds UI-set keys.
 type Store struct {
@@ -164,8 +172,12 @@ func splitPassphraseFile(raw []byte) (salt, sealed []byte, ok bool) {
 // Encrypted reports whether keys are encrypted at rest.
 func (s *Store) Encrypted() bool { return s.aead != nil }
 
-// Status reports where the value for an environment variable name comes from.
+// Status reports where the value for an environment variable name comes from. An invalid
+// name is always missing, so the status never tells whether a reserved variable is set.
 func (s *Store) Status(name string) (string, error) {
+	if !ValidName(name) {
+		return Missing, nil
+	}
 	if v, ok := s.lookup(name); ok && v != "" {
 		return FromEnv, nil
 	}
@@ -185,7 +197,7 @@ func (s *Store) Status(name string) (string, error) {
 // that value takes precedence and the UI treats it as read-only.
 func (s *Store) Set(name, value string) error {
 	if !ValidName(name) {
-		return fmt.Errorf("%q isn't a valid environment variable name", name)
+		return fmt.Errorf("%q can't hold a provider key: use a variable name like OPENROUTER_API_KEY, outside YARDMASTER_*", name)
 	}
 	if value == "" {
 		return errors.New("the key is empty")
@@ -216,7 +228,8 @@ func (s *Store) Delete(name string) error {
 }
 
 // Env returns NAME=value pairs for the given names, for switchyard-server's environment.
-// Container environment values win over UI-set ones. Missing names are left out.
+// Container environment values win over UI-set ones. Missing and invalid names are left
+// out.
 func (s *Store) Env(names []string) ([]string, error) {
 	s.mu.Lock()
 	keys, err := s.load()
@@ -227,6 +240,9 @@ func (s *Store) Env(names []string) ([]string, error) {
 	sort.Strings(names)
 	var out []string
 	for _, name := range names {
+		if !ValidName(name) {
+			continue
+		}
 		if v, ok := s.lookup(name); ok && v != "" {
 			out = append(out, name+"="+v)
 		} else if v := keys[name]; v != "" {

internal/secrets/secrets_test.go

@@ -153,3 +153,26 @@ func TestRawKeyKeepsItsFormat(t *testing.T) {
 		t.Fatalf("raw key on a passphrase file: %v", err)
 	}
 }
+
+func TestYardMasterVariablesCantHoldKeys(t *testing.T) {
+	s, err := Open(t.TempDir(), "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	s.lookup = func(name string) (string, bool) {
+		if name == "YARDMASTER_SECRET_KEY" {
+			return "the-master-key", true
+		}
+		return "", false
+	}
+	if err := s.Set("YARDMASTER_PROXY_SECRET", "x"); err == nil {
+		t.Error("a key was stored under a YARDMASTER_ name")
+	}
+	// The status doesn't tell whether the variable is set.
+	if st, _ := s.Status("YARDMASTER_SECRET_KEY"); st != Missing {
+		t.Errorf("status %s, want %s", st, Missing)
+	}
+	if env, _ := s.Env([]string{"YARDMASTER_SECRET_KEY"}); len(env) != 0 {
+		t.Errorf("YardMaster's own secret reached the env: %v", env)
+	}
+}