Close config trust-boundary gaps and gate the completion loader

- Trust-gate cache_dir, cache_enabled, llm_model, llm_max_concurrent and
  llm_token_budget so an untrusted local/repo config can't redirect cache
  writes or amplify paid-LLM usage; only honored from trusted sources.
  'config set' now also warns when these keys are written to an untrusted
  local config so the value isn't silently ignored on the next load.
- Scheme/host-validate llm_endpoint on accept (rejects file://, hostless and
  malformed forms) and re-validate at the root.go enforcement point so env/
  profile-sourced endpoints can't slip a non-http(s) scheme past
  RequireSecureURL, which only blocks http:// for non-localhost; this prevents
  leaking llm_api_key in cleartext.
- Gate the completion profile loader behind the TrustStore and strip control
  characters from completion descriptions.

Author: jeremy

internal/cli/root.go

@@ -126,6 +126,36 @@ func NewRootCmd() *cobra.Command {
 					}
 					return fmt.Errorf("base_url (%s): %w\nFix with: basecamp config unset base_url", source, err)
 				}
+
+				// Validate the LLM endpoint. The config-file accept path already
+				// rejects non-http(s)/hostless values, but env- and profile-sourced
+				// endpoints reach here unchecked, so re-validate the scheme/host
+				// (RequireSecureURL only blocks http:// for non-localhost — it
+				// would let file://, ssh:// etc. through). Then enforce HTTPS so an
+				// http:// non-localhost endpoint can't leak llm_api_key in
+				// cleartext. Empty endpoint is a no-op. Same config-subcommand skip.
+				if cfg.LLMEndpoint != "" && !config.IsHTTPURL(cfg.LLMEndpoint) {
+					if bareRoot {
+						initBareRootApp(cfg)
+						return nil
+					}
+					source := cfg.Sources["llm_endpoint"]
+					if source == "" {
+						source = "unknown"
+					}
+					return fmt.Errorf("llm_endpoint (%s): must be an http:// or https:// URL with a host\nFix with: basecamp config unset llm_endpoint", source)
+				}
+				if err := hostutil.RequireSecureURL(cfg.LLMEndpoint); err != nil {
+					if bareRoot {
+						initBareRootApp(cfg)
+						return nil
+					}
+					source := cfg.Sources["llm_endpoint"]
+					if source == "" {
+						source = "unknown"
+					}
+					return fmt.Errorf("llm_endpoint (%s): %w\nFix with: basecamp config unset llm_endpoint", source, err)
+				}
 			}
 
 			// Resolve behavior preferences: explicit flag > config > version.IsDev()

internal/commands/config.go

@@ -343,12 +343,13 @@ Valid keys: account_id, project_id (or project), todolist_id, base_url, cache_di
 				return fmt.Errorf("failed to write config: %w", err)
 			}
 
-			// Warn when writing authority keys to local config without trust
-			if !global && isAuthorityKey(key) {
+			// Warn when writing trust-gated keys to local config without trust —
+			// otherwise the value is silently ignored on the next load.
+			if !global && isTrustGatedKey(key) {
 				absPath, _ := filepath.Abs(configPath)
 				ts := config.LoadTrustStore(config.GlobalConfigDir())
 				if ts == nil || !ts.IsTrusted(configPath) {
-					fmt.Fprintf(os.Stderr, "warning: authority key %q in local config requires trust to take effect; run:\n  basecamp config trust %s\n", key, config.ShellQuote(absPath))
+					fmt.Fprintf(os.Stderr, "warning: %q in local config requires trust to take effect; run:\n  basecamp config trust %s\n", key, config.ShellQuote(absPath))
 				}
 			}
 
@@ -406,6 +407,21 @@ func isAuthorityKey(key string) bool {
 	return false
 }
 
+// isTrustGatedKey reports whether key is ignored when loaded from an untrusted
+// local/repo config. It's the authority keys plus the cache/LLM keys gated for
+// the same reason (cache redirection, paid-model substitution, cost
+// amplification), so `config set` warns before a local write silently no-ops.
+func isTrustGatedKey(key string) bool {
+	if isAuthorityKey(key) {
+		return true
+	}
+	switch key {
+	case "cache_dir", "cache_enabled", "llm_model", "llm_max_concurrent", "llm_token_budget":
+		return true
+	}
+	return false
+}
+
 // redactSecret returns "(set)" if the value is non-empty, empty string otherwise.
 func redactSecret(value string) string {
 	if value != "" {

internal/commands/config_test.go

@@ -337,12 +337,41 @@ func TestConfigSet_AuthorityKeyWarnsWithPath(t *testing.T) {
 	stderr := string(buf[:n])
 	absPath := filepath.Join(tmpDir, ".basecamp", "config.json")
 
-	assert.Contains(t, stderr, "authority key")
+	assert.Contains(t, stderr, `"base_url"`)
 	assert.Contains(t, stderr, "requires trust")
 	assert.Contains(t, stderr, absPath, "warning must include the exact config path")
 	assert.Contains(t, stderr, "'"+absPath+"'", "path must be single-quoted for shell safety")
 }
 
+// TestConfigSet_GatedNonAuthorityKeyWarns verifies the trust warning also fires
+// for the cache/LLM keys gated on load (cache_dir, llm_model, …), so a local
+// `config set` doesn't silently produce a value that's ignored next run.
+func TestConfigSet_GatedNonAuthorityKeyWarns(t *testing.T) {
+	app, _ := setupConfigTestApp(t)
+
+	tmpDir, _ := filepath.EvalSymlinks(t.TempDir())
+	origDir, _ := os.Getwd()
+	require.NoError(t, os.Chdir(tmpDir))
+	defer os.Chdir(origDir)
+	require.NoError(t, os.MkdirAll(".basecamp", 0755))
+
+	origStderr := os.Stderr
+	r, w, _ := os.Pipe()
+	os.Stderr = w
+
+	err := executeConfigCommand(app, "set", "cache_dir", "/tmp/somewhere")
+	require.NoError(t, err)
+
+	w.Close()
+	var buf [4096]byte
+	n, _ := r.Read(buf[:])
+	os.Stderr = origStderr
+
+	stderr := string(buf[:n])
+	assert.Contains(t, stderr, `"cache_dir"`)
+	assert.Contains(t, stderr, "requires trust")
+}
+
 // --- Config project tests ---
 
 // setupConfigProjectTestApp creates a test app wired to an httptest server

internal/completion/cache.go

@@ -10,6 +10,8 @@ import (
 	"path/filepath"
 	"sync"
 	"time"
+
+	"github.com/basecamp/basecamp-cli/internal/config"
 )
 
 // CachedProject holds project data for tab completion.
@@ -366,13 +368,21 @@ func loadConfigForCompletion() *configForCompletion {
 		loadProfilesFromFile(cfg, globalPath)
 	}
 
+	// profiles is an authority key (it can redirect authenticated traffic), so
+	// repo/local configs must be explicitly trusted before their profiles feed
+	// completion — mirroring config.loadFromFile's trust gating. System/global
+	// configs above are trusted by location.
+	trust := config.LoadTrustStore(config.GlobalConfigDir())
+
 	// Repo config (walk up to find .git, then .basecamp/config.json)
 	if dir, err := os.Getwd(); err == nil {
 		for {
 			gitPath := filepath.Join(dir, ".git")
 			if fi, err := os.Stat(gitPath); err == nil && fi.IsDir() {
 				repoConfig := filepath.Join(dir, ".basecamp", "config.json")
-				loadProfilesFromFile(cfg, repoConfig)
+				if trust != nil && trust.IsTrusted(repoConfig) {
+					loadProfilesFromFile(cfg, repoConfig)
+				}
 				break
 			}
 			parent := filepath.Dir(dir)
@@ -385,7 +395,9 @@ func loadConfigForCompletion() *configForCompletion {
 
 	// Local config
 	localPath := filepath.Join(".basecamp", "config.json")
-	loadProfilesFromFile(cfg, localPath)
+	if trust != nil && trust.IsTrusted(localPath) {
+		loadProfilesFromFile(cfg, localPath)
+	}
 
 	return cfg
 }

internal/completion/completer.go

@@ -91,7 +91,7 @@ func (c *Completer) ProjectCompletion() cobra.CompletionFunc {
 				// Use ID as completion value with name as description
 				completion := cobra.CompletionWithDesc(
 					fmt.Sprintf("%d", p.ID),
-					p.Name,
+					sanitizeCompletionDesc(p.Name),
 				)
 				completions = append(completions, completion)
 			}
@@ -164,7 +164,7 @@ func (c *Completer) PeopleCompletion() cobra.CompletionFunc {
 				}
 				completion := cobra.CompletionWithDesc(
 					fmt.Sprintf("%d", p.ID),
-					desc,
+					sanitizeCompletionDesc(desc),
 				)
 				completions = append(completions, completion)
 			}
@@ -237,7 +237,7 @@ func (c *Completer) AccountCompletion() cobra.CompletionFunc {
 				strings.HasPrefix(nameLower, toCompleteLower) ||
 				strings.Contains(nameLower, toCompleteLower) {
 				// Use ID as completion value with name as description
-				completions = append(completions, cobra.CompletionWithDesc(idStr, a.Name))
+				completions = append(completions, cobra.CompletionWithDesc(idStr, sanitizeCompletionDesc(a.Name)))
 			}
 		}
 
@@ -270,14 +270,27 @@ func (c *Completer) ProfileCompletion() cobra.CompletionFunc {
 			if strings.HasPrefix(nameLower, toCompleteLower) ||
 				strings.Contains(nameLower, toCompleteLower) {
 				// Use name as completion value with base URL as description
-				completions = append(completions, cobra.CompletionWithDesc(p.Name, p.BaseURL))
+				completions = append(completions, cobra.CompletionWithDesc(p.Name, sanitizeCompletionDesc(p.BaseURL)))
 			}
 		}
 
 		return completions, cobra.ShellCompDirectiveNoFileComp
 	}
 }
 
+// sanitizeCompletionDesc drops control characters (including ESC) from a
+// completion description. Descriptions can carry API- or config-controlled
+// strings (project/person/account names, profile base_url) which the shell
+// renders to the terminal; stripping control bytes prevents terminal injection.
+func sanitizeCompletionDesc(s string) string {
+	return strings.Map(func(r rune) rune {
+		if r < 0x20 || r == 0x7f {
+			return -1
+		}
+		return r
+	}, s)
+}
+
 // rankProjects returns projects sorted by priority:
 // 1. HQ (purpose="hq")
 // 2. Bookmarked

internal/config/config.go

@@ -4,6 +4,7 @@ package config
 import (
 	"encoding/json"
 	"fmt"
+	"net/url"
 	"os"
 	"path/filepath"
 	"strings"
@@ -196,12 +197,24 @@ func loadFromFile(cfg *Config, path string, source Source, trust *TrustStore) {
 		cfg.Sources["scope"] = string(source)
 	}
 	if v, ok := fileCfg["cache_dir"].(string); ok && v != "" {
-		cfg.CacheDir = v
-		cfg.Sources["cache_dir"] = string(source)
+		// cache_dir redirects every cache write (completion, resilience, TUI
+		// workspace, recents, traces). An untrusted local/repo config could
+		// point it at any user-writable path, so gate it like other authority
+		// keys. filepath.Clean normalizes the accepted value.
+		if untrusted {
+			fmt.Fprintf(os.Stderr, "warning: ignoring cache_dir %q from %s config at %s\n  (authority key from local/repo config; run `basecamp config trust %s` to allow)\n", v, source, path, ShellQuote(path))
+		} else {
+			cfg.CacheDir = filepath.Clean(v)
+			cfg.Sources["cache_dir"] = string(source)
+		}
 	}
 	if v, ok := fileCfg["cache_enabled"].(bool); ok {
-		cfg.CacheEnabled = v
-		cfg.Sources["cache_enabled"] = string(source)
+		if untrusted {
+			fmt.Fprintf(os.Stderr, "warning: ignoring cache_enabled from %s config at %s\n  (authority key from local/repo config; run `basecamp config trust %s` to allow)\n", source, path, ShellQuote(path))
+		} else {
+			cfg.CacheEnabled = v
+			cfg.Sources["cache_enabled"] = string(source)
+		}
 	}
 	if v, ok := fileCfg["format"].(string); ok && v != "" {
 		cfg.Format = v
@@ -237,8 +250,14 @@ func loadFromFile(cfg *Config, path string, source Source, trust *TrustStore) {
 		}
 	}
 	if v, ok := fileCfg["llm_model"].(string); ok && v != "" {
-		cfg.LLMModel = v
-		cfg.Sources["llm_model"] = string(source)
+		// Gate like other LLM authority keys: an untrusted config could
+		// silently substitute a costlier paid model.
+		if untrusted {
+			fmt.Fprintf(os.Stderr, "warning: ignoring llm_model %q from %s config at %s\n  (authority key from local/repo config; run `basecamp config trust %s` to allow)\n", v, source, path, ShellQuote(path))
+		} else {
+			cfg.LLMModel = v
+			cfg.Sources["llm_model"] = string(source)
+		}
 	}
 	if v, ok := fileCfg["llm_api_key"].(string); ok && v != "" {
 		// Secret: only from global/system config, never local/repo
@@ -252,6 +271,11 @@ func loadFromFile(cfg *Config, path string, source Source, trust *TrustStore) {
 	if v, ok := fileCfg["llm_endpoint"].(string); ok && v != "" {
 		if untrusted {
 			fmt.Fprintf(os.Stderr, "warning: ignoring llm_endpoint %q from %s config at %s\n  (authority key from local/repo config; run `basecamp config trust %s` to allow)\n", v, source, path, ShellQuote(path))
+		} else if !IsHTTPURL(v) {
+			// Reject non-http(s) schemes (file://, etc.) and hostless/malformed
+			// forms. https enforcement for non-localhost endpoints happens later
+			// in root.go via RequireSecureURL.
+			fmt.Fprintf(os.Stderr, "warning: ignoring llm_endpoint %q from %s config at %s (must be an http:// or https:// URL with a host)\n", v, source, path)
 		} else {
 			cfg.LLMEndpoint = v
 			cfg.Sources["llm_endpoint"] = string(source)
@@ -260,7 +284,11 @@ func loadFromFile(cfg *Config, path string, source Source, trust *TrustStore) {
 	if v, ok := fileCfg["llm_max_concurrent"]; ok {
 		if fv, ok := v.(float64); ok {
 			iv := int(fv)
-			if iv >= 1 && iv <= 10 && fv == float64(iv) {
+			// Gate like other LLM authority keys: block a malicious repo from
+			// inflating paid-LLM concurrency (cost amplification).
+			if untrusted {
+				fmt.Fprintf(os.Stderr, "warning: ignoring llm_max_concurrent from %s config at %s\n  (authority key from local/repo config; run `basecamp config trust %s` to allow)\n", source, path, ShellQuote(path))
+			} else if iv >= 1 && iv <= 10 && fv == float64(iv) {
 				cfg.LLMMaxConcurrent = iv
 				cfg.Sources["llm_max_concurrent"] = string(source)
 			}
@@ -269,7 +297,10 @@ func loadFromFile(cfg *Config, path string, source Source, trust *TrustStore) {
 	if v, ok := fileCfg["llm_token_budget"]; ok {
 		if fv, ok := v.(float64); ok {
 			iv := int(fv)
-			if iv >= 100 && iv <= 100000 && fv == float64(iv) {
+			// Gate like other LLM authority keys (cost amplification).
+			if untrusted {
+				fmt.Fprintf(os.Stderr, "warning: ignoring llm_token_budget from %s config at %s\n  (authority key from local/repo config; run `basecamp config trust %s` to allow)\n", source, path, ShellQuote(path))
+			} else if iv >= 100 && iv <= 100000 && fv == float64(iv) {
 				cfg.LLMTokenBudget = iv
 				cfg.Sources["llm_token_budget"] = string(source)
 			}
@@ -660,6 +691,19 @@ func NormalizeBaseURL(url string) string {
 	return strings.TrimSuffix(url, "/")
 }
 
+// IsHTTPURL reports whether rawURL is an absolute http(s) URL with a host.
+// Used to reject non-web schemes (file://, etc.) and hostless forms
+// (e.g. "https:example.com", which url.Parse accepts with an empty Host)
+// for llm_endpoint, since those would later be concatenated into a request
+// URL and produce confusing/invalid requests.
+func IsHTTPURL(rawURL string) bool {
+	u, err := url.Parse(rawURL)
+	if err != nil {
+		return false
+	}
+	return (u.Scheme == "http" || u.Scheme == "https") && u.Host != ""
+}
+
 // ShellQuote returns a POSIX single-quoted string safe for copy-paste into
 // a shell. Single quotes inside the value are escaped as '\” (end quote,
 // escaped literal quote, resume quote).