fix(llm): send a saved API key only to the endpoint it was saved for

POST /api/llm/test and POST /api/llm/models take provider and base_url from
the form but fell back to the stored profile key (and, for the test, the
global key) when the form had none. Any base_url typed into the form, or
supplied by a crafted request, received the saved key.

The saved key is now used only when the request's provider and base URL
normalize to the ones it was saved with (sameLLMEndpoint, built on
agent.ConfigFrom). Otherwise the key must be entered in the form, and the
error says why.

cmd/artex/main.go

@@ -88,11 +88,7 @@ func run() int {
 	}
 	defer mgr.Close()
 
-	skillDir, err := config.SkillDir()
-	if err != nil {
-		log.Printf("[config] %v", err)
-		return 1
-	}
+	skillDir := config.SkillDir()
 	if abs, err := filepath.Abs(skillDir); err == nil {
 		skillDir = abs
 	}

config/config.go

@@ -4,9 +4,7 @@ package config
 
 import (
 	"encoding/json"
-	"errors"
 	"fmt"
-	"io/fs"
 	"net/url"
 	"os"
 	"path/filepath"
@@ -98,23 +96,16 @@ func Path() string {
 	return candidates[0]
 }
 
-// Load reads and parses the config file. A missing file yields a zero Config (so
-// callers fall back to defaults). A file that exists but cannot be read or parsed
-// is an error: ignoring it would run with defaults the operator did not choose.
-func Load() (Config, error) {
+// Load reads and parses the config file. A missing/unreadable file yields a zero
+// Config (so callers fall back to defaults) rather than an error.
+func Load() Config {
 	var c Config
-	path := Path()
-	b, err := os.ReadFile(path)
-	if errors.Is(err, fs.ErrNotExist) {
-		return c, nil
-	}
+	b, err := os.ReadFile(Path())
 	if err != nil {
-		return c, fmt.Errorf("read config file %s: %w", path, err)
+		return c
 	}
-	if err := json.Unmarshal(b, &c); err != nil {
-		return Config{}, fmt.Errorf("parse config file %s: %w", path, err)
-	}
-	return c, nil
+	_ = json.Unmarshal(b, &c)
+	return c
 }
 
 // SkillDir returns the skill root directory with precedence:
@@ -122,23 +113,17 @@ func Load() (Config, error) {
 //	env ARTEX_SKILL_DIR  >  config file (skill_dir)  >  BaseDir()/skills
 //
 // The directory is created if it does not exist.
-func SkillDir() (string, error) {
+func SkillDir() string {
 	var d string
 	if v := strings.TrimSpace(os.Getenv("ARTEX_SKILL_DIR")); v != "" {
 		d = v
+	} else if v := strings.TrimSpace(Load().SkillDir); v != "" {
+		d = v
 	} else {
-		c, err := Load()
-		if err != nil {
-			return "", err
-		}
-		if v := strings.TrimSpace(c.SkillDir); v != "" {
-			d = v
-		} else {
-			d = filepath.Join(BaseDir(), "skills")
-		}
+		d = filepath.Join(BaseDir(), "skills")
 	}
 	_ = os.MkdirAll(d, 0o755)
-	return d, nil
+	return d
 }
 
 // PostgresDSN resolves the connection string with precedence:
@@ -153,11 +138,7 @@ func PostgresDSN() (dsn, source string, err error) {
 	if v := strings.TrimSpace(os.Getenv("ARTEX_PG_DSN")); v != "" {
 		return v, "environment variable ARTEX_PG_DSN", nil
 	}
-	c, err := Load()
-	if err != nil {
-		return "", "", err
-	}
-	db := c.Database
+	db := Load().Database
 	if d := strings.TrimSpace(db.DSN); d != "" {
 		return d, "config file " + Path() + " (database.dsn)", nil
 	}

config/config_test.go

@@ -3,7 +3,6 @@ package config
 import (
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 )
 
@@ -41,24 +40,3 @@ func TestPostgresDSNPrecedence(t *testing.T) {
 		t.Fatalf("file dsn verbatim, got %q err %v", got, err)
 	}
 }
-
-// A config file that exists but is not valid JSON must fail loudly, not run with
-// defaults (or report "no database config") as if the file were absent.
-func TestMalformedConfigIsAnError(t *testing.T) {
-	cfgPath := filepath.Join(t.TempDir(), "config.json")
-	// What install.sh used to write for a password containing a double quote.
-	os.WriteFile(cfgPath, []byte(`{"database":{"host":"h","password":"a"b","dbname":"d"},"skill_dir":"/s"}`), 0o644)
-	t.Setenv("ARTEX_CONFIG", cfgPath)
-	t.Setenv("ARTEX_PG_DSN", "")
-	t.Setenv("ARTEX_SKILL_DIR", "")
-
-	if _, _, err := PostgresDSN(); err == nil || !strings.Contains(err.Error(), "parse config file") {
-		t.Fatalf("PostgresDSN: want a parse error, got %v", err)
-	}
-	if d, err := SkillDir(); err == nil {
-		t.Fatalf("SkillDir: want a parse error, got dir %q", d)
-	}
-	if _, err := Load(); err == nil {
-		t.Fatal("Load: want a parse error")
-	}
-}

install.sh

@@ -9,16 +9,6 @@ warn(){ printf '\033[33m[!]\033[0m %s\n' "$*"; }
 die(){  printf '\033[31m[x]\033[0m %s\n' "$*" >&2; exit 1; }
 ask(){  local p="$1" d="${2:-}" a; read -rp "$p${d:+ [$d]}: " a; echo "${a:-$d}"; }
 rand(){ head -c 18 /dev/urandom | base64 | tr -dc 'A-Za-z0-9' | head -c 24; }
-# json_str prints $1 as a JSON string literal: backslash, double quote and the
-# control characters read -r can return are escaped.
-json_str(){
-  local s="$1"
-  s="${s//\\/\\\\}"; s="${s//\"/\\\"}"
-  s="${s//$'\t'/\\t}"; s="${s//$'\r'/\\r}"
-  printf '"%s"' "$s"
-}
-# sed_repl escapes $1 for the replacement side of s|...|...|: \, | and &.
-sed_repl(){ printf '%s' "$1" | sed 's/[\\|&]/\\&/g'; }
 
 # ── docker environment check / auto-install ───────────────────
 ensure_docker(){
@@ -48,8 +38,8 @@ install_docker(){
     local pw key
     pw="$(ask 'Postgres password (press Enter to generate randomly)' "$(rand)")"
     key="$(ask 'ANTHROPIC_API_KEY (may be empty, configure later in the UI)' '')"
-    sed -i.bak "s|^POSTGRES_PASSWORD=.*|POSTGRES_PASSWORD=$(sed_repl "$pw")|" .env
-    sed -i.bak "s|^ANTHROPIC_API_KEY=.*|ANTHROPIC_API_KEY=$(sed_repl "$key")|" .env
+    sed -i.bak "s|^POSTGRES_PASSWORD=.*|POSTGRES_PASSWORD=${pw}|" .env
+    sed -i.bak "s|^ANTHROPIC_API_KEY=.*|ANTHROPIC_API_KEY=${key}|" .env
     rm -f .env.bak
     ok ".env generated (POSTGRES_PASSWORD set)"
   else
@@ -86,17 +76,16 @@ install_local(){
       DB_SSL="$(ask 'sslmode (disable/require)' disable)" ;;
   esac
 
-  # generate config.json (string values JSON-escaped: a password may contain " or \)
-  case "$DB_PORT" in (*[!0-9]*|'') die "port must be a number: $DB_PORT" ;; esac
+  # generate config.json
   cat > config.json <<JSON
 {
   "database": {
-    "host": $(json_str "$DB_HOST"),
+    "host": "${DB_HOST}",
     "port": ${DB_PORT},
-    "user": $(json_str "$DB_USER"),
-    "password": $(json_str "$DB_PASS"),
-    "dbname": $(json_str "$DB_NAME"),
-    "sslmode": $(json_str "$DB_SSL")
+    "user": "${DB_USER}",
+    "password": "${DB_PASS}",
+    "dbname": "${DB_NAME}",
+    "sslmode": "${DB_SSL}"
   }
 }
 JSON

server/llm_key_endpoint_test.go

@@ -0,0 +1,127 @@
+package server
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"sync/atomic"
+	"testing"
+
+	"github.com/Autumn-27/artex/agent"
+	"github.com/Autumn-27/artex/db"
+)
+
+func TestSameLLMEndpoint(t *testing.T) {
+	for _, tc := range []struct {
+		provA, baseA, provB, baseB string
+		want                       bool
+	}{
+		{"anthropic", "https://gw.example", "anthropic", "https://gw.example/", true},
+		{"anthropic", "https://gw.example/v1/messages", "anthropic", "https://gw.example", true},
+		{"", "", "anthropic", "", true}, // empty provider defaults to anthropic
+		{"openai", " https://gw.example/v1/chat/completions ", "openai", "https://gw.example/v1", true},
+		{"anthropic", "https://evil.example", "anthropic", "https://gw.example", false},
+		{"anthropic", "", "anthropic", "https://gw.example", false}, // canonical host != saved host
+		{"openai", "", "anthropic", "", false},                      // different canonical hosts
+		{"openai", "https://gw.example/v1", "openai-responses", "https://gw.example/v1", false},
+	} {
+		if got := sameLLMEndpoint(tc.provA, tc.baseA, tc.provB, tc.baseB); got != tc.want {
+			t.Errorf("sameLLMEndpoint(%q,%q,%q,%q)=%v want %v", tc.provA, tc.baseA, tc.provB, tc.baseB, got, tc.want)
+		}
+	}
+}
+
+// keyRecorder is a fake Anthropic endpoint that records the x-api-key it receives.
+func keyRecorder(t *testing.T) (*httptest.Server, *atomic.Int32, *atomic.Value) {
+	var hits atomic.Int32
+	var key atomic.Value
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		hits.Add(1)
+		key.Store(r.Header.Get("x-api-key"))
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"id":"m","type":"message","role":"assistant","model":"m","content":[{"type":"text","text":"OK"}],"stop_reason":"end_turn","usage":{"input_tokens":1,"output_tokens":1},"data":[]}`))
+	}))
+	t.Cleanup(srv.Close)
+	return srv, &hits, &key
+}
+
+func postJSON(t *testing.T, h http.HandlerFunc, path string, body map[string]any) map[string]any {
+	t.Helper()
+	b, _ := json.Marshal(body)
+	req := httptest.NewRequest(http.MethodPost, path, bytes.NewReader(b))
+	req.Header.Set("Content-Type", "application/json")
+	rec := httptest.NewRecorder()
+	h(rec, req)
+	var resp map[string]any
+	_ = json.Unmarshal(rec.Body.Bytes(), &resp)
+	return resp
+}
+
+// The global key must not follow a base_url typed into the test form.
+func TestLLMTestGlobalKeyOnlyForGlobalEndpoint(t *testing.T) {
+	saved, savedHits, savedKey := keyRecorder(t)
+	other, otherHits, _ := keyRecorder(t)
+	s := &Server{}
+	s.llmCfg = agent.ConfigFrom("anthropic", "m", saved.URL, "global-secret", "")
+
+	resp := postJSON(t, s.testLLM, "/api/llm/test", map[string]any{
+		"provider": "anthropic", "model": "m", "base_url": other.URL, "api_key": "", "streaming": false,
+	})
+	if otherHits.Load() != 0 || resp["ok"] != false {
+		t.Fatalf("global key went to a form-supplied host: hits=%d resp=%v", otherHits.Load(), resp)
+	}
+
+	postJSON(t, s.testLLM, "/api/llm/test", map[string]any{
+		"provider": "anthropic", "model": "m", "base_url": saved.URL + "/", "api_key": "", "streaming": false,
+	})
+	if k, _ := savedKey.Load().(string); savedHits.Load() != 1 || k != "global-secret" {
+		t.Fatalf("global key not used for its own endpoint: hits=%d key=%q", savedHits.Load(), k)
+	}
+}
+
+// A stored profile's key goes only to that profile's base URL, for both the
+// connection test and the model listing.
+func TestStoredProfileKeyOnlyForStoredEndpoint(t *testing.T) {
+	m, err := NewManager(t.TempDir(), "")
+	if err != nil {
+		t.Skipf("postgres unavailable (%v)", err)
+	}
+	t.Cleanup(func() { m.Close() })
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+	s := New(ctx, m, t.TempDir(), t.TempDir(), t.TempDir())
+	s.cfgMu.Lock()
+	s.llmCfg = agent.Config{} // no global key to fall back on
+	s.cfgMu.Unlock()
+
+	saved, savedHits, savedKey := keyRecorder(t)
+	other, otherHits, _ := keyRecorder(t)
+	pid, err := m.pg.SaveProfile(&db.LLMProfile{
+		Name: "key-endpoint", Format: "anthropic", Model: "m", BaseURL: saved.URL, APIKey: "profile-secret",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { _, _ = m.pg.Exec("DELETE FROM llm_profiles WHERE id=$1", pid) })
+
+	for _, ep := range []struct {
+		path string
+		h    http.HandlerFunc
+	}{{"/api/llm/test", s.testLLM}, {"/api/llm/models", s.pgListModels}} {
+		resp := postJSON(t, ep.h, ep.path, map[string]any{
+			"provider": "anthropic", "model": "m", "base_url": other.URL, "api_key": "", "profile_id": pid, "streaming": false,
+		})
+		if otherHits.Load() != 0 || resp["ok"] != false {
+			t.Fatalf("%s: stored key went to a form-supplied host: hits=%d resp=%v", ep.path, otherHits.Load(), resp)
+		}
+		before := savedHits.Load()
+		postJSON(t, ep.h, ep.path, map[string]any{
+			"provider": "anthropic", "model": "m", "base_url": saved.URL, "api_key": "", "profile_id": pid, "streaming": false,
+		})
+		if k, _ := savedKey.Load().(string); savedHits.Load() == before || k != "profile-secret" {
+			t.Fatalf("%s: stored key not used for its own endpoint: key=%q", ep.path, k)
+		}
+	}
+}

server/server.go

@@ -1466,19 +1466,38 @@ func (s *Server) testLLM(w http.ResponseWriter, r *http.Request) {
 	if cfg.NeedsAPIKey() {
 		// API key resolution precedence: form input > the named profile's stored key > the global config's key.
 		// A stored profile's key is never sent back to the browser, so the form is empty when testing a stored profile and the key comes from the DB.
+		// A saved key is only sent to the endpoint it was saved for: base_url and
+		// provider come from the form, so without this check the test button sends
+		// the saved key to any host typed into the form.
+		withheld := false
 		if stored != nil && cfg.APIKey == "" {
-			// Trim like ConfigFrom does for form values: a key stored with
-			// surrounding whitespace runs fine in conversations (the runtime
-			// path re-trims) but would be sent raw here and fail auth.
-			cfg.APIKey = strings.TrimSpace(stored.APIKey)
+			if sameLLMEndpoint(req.Provider, req.BaseURL, stored.Format, stored.BaseURL) {
+				// Trim like ConfigFrom does for form values: a key stored with
+				// surrounding whitespace runs fine in conversations (the runtime
+				// path re-trims) but would be sent raw here and fail auth.
+				cfg.APIKey = strings.TrimSpace(stored.APIKey)
+			} else {
+				withheld = true
+			}
 		}
 		if cfg.APIKey == "" {
 			s.cfgMu.Lock()
-			cfg.APIKey = s.llmCfg.APIKey
+			global := s.llmCfg
 			s.cfgMu.Unlock()
+			if global.APIKey != "" {
+				if sameLLMEndpoint(req.Provider, req.BaseURL, global.Provider(), global.BaseURL) {
+					cfg.APIKey = global.APIKey
+				} else {
+					withheld = true
+				}
+			}
 		}
 		if cfg.APIKey == "" {
-			writeJSON(w, 200, map[string]any{"ok": false, "error": "no API key provided"})
+			msg := "no API key provided"
+			if withheld {
+				msg = "no API key provided: the saved key is only sent to the endpoint it was saved for; enter the key to test a different base URL or provider"
+			}
+			writeJSON(w, 200, map[string]any{"ok": false, "error": msg})
 			return
 		}
 	}
@@ -1496,6 +1515,16 @@ func (s *Server) testLLM(w http.ResponseWriter, r *http.Request) {
 	})
 }
 
+// sameLLMEndpoint reports whether two provider/base URL pairs resolve to the same
+// endpoint after ConfigFrom's normalization (trimmed, endpoint suffix stripped;
+// an empty base URL means the provider's canonical host). A saved API key may only
+// be sent when the request's endpoint matches the one it was saved for.
+func sameLLMEndpoint(provA, baseA, provB, baseB string) bool {
+	a := agent.ConfigFrom(provA, "", baseA, "", "")
+	b := agent.ConfigFrom(provB, "", baseB, "", "")
+	return a.Format == b.Format && a.BaseURL == b.BaseURL
+}
+
 // truncateReply clips a connection-test reply for display. A model told to answer
 // "OK" can still ramble (or think out loud); the UI only needs enough to show it
 // really said something. Rune-based so multibyte text never splits mid-character.

server/server_mgmt.go

@@ -2053,17 +2053,27 @@ func (s *Server) pgListModels(w http.ResponseWriter, r *http.Request) {
 		writeErr(w, 400, err.Error())
 		return
 	}
-	// Resolve API key: form input > profile stored key.
+	// Resolve API key: form input > profile stored key. The stored key is only
+	// sent to the endpoint it was saved for, since base_url comes from the form.
 	apiKey := strings.TrimSpace(req.APIKey)
+	withheld := false
 	if apiKey == "" && req.ProfileID != nil {
 		if p, err := s.m.pg.ProfileByID(*req.ProfileID); err == nil && p != nil {
-			apiKey = p.APIKey
+			if sameLLMEndpoint(req.Provider, req.BaseURL, p.Format, p.BaseURL) {
+				apiKey = p.APIKey
+			} else {
+				withheld = p.APIKey != ""
+			}
 		}
 	}
 	provider := strings.TrimSpace(req.Provider)
 	// Ollama serves its model list keyless on the local server: no key gate.
 	if apiKey == "" && provider != "ollama" {
-		writeJSON(w, 200, map[string]any{"ok": false, "error": "no API Key provided"})
+		msg := "no API Key provided"
+		if withheld {
+			msg = "no API Key provided: the saved key is only sent to the endpoint it was saved for; enter the key to list models from a different base URL or provider"
+		}
+		writeJSON(w, 200, map[string]any{"ok": false, "error": msg})
 		return
 	}