security: close config trust-boundary gaps and gate completion loader

internal/cli/root.go

@@ -98,7 +98,13 @@ func NewRootCmd() *cobra.Command {
 					return err
 				}
 				// Re-apply env and flag overrides (they take precedence over profile values)
-				config.LoadFromEnv(cfg)
+				if err := config.LoadFromEnv(cfg); err != nil {
+					if bareRoot {
+						initBareRootApp(cfg)
+						return nil
+					}
+					return err
+				}
 				config.ApplyOverrides(cfg, config.FlagOverrides{
 					Account:  flags.Account,
 					Project:  flags.Project,
@@ -126,6 +132,24 @@ 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
+				// and enforce HTTPS (credential-gated) 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 err := validateLLMEndpoint(cfg.LLMEndpoint, cfg.LLMProvider, cfg.LLMAPIKey); 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()
@@ -528,6 +552,37 @@ func promptForProfile(cfg *config.Config) (string, error) {
 	return selected.ID, nil
 }
 
+// validateLLMEndpoint rejects non-http(s)/hostless endpoints always, and requires
+// HTTPS for a non-localhost endpoint only when a credential could be transmitted.
+//
+// RequireSecureURL only blocks http:// for non-localhost — it would let file://,
+// ssh:// etc. through — so the IsHTTPURL scheme/host check stays unconditional.
+//
+// The HTTPS/credential gate is skipped for credential-less providers ("ollama",
+// "apple", "none", "disabled"): they never transmit llm_api_key, so a remote http:// endpoint
+// (e.g. a LAN Ollama) is allowed even when a global key exists for a different
+// provider. For credentialed/ambiguous providers ("openai", "anthropic", "auto",
+// or "") the gate applies — an http:// non-localhost endpoint is rejected when an
+// llm_api_key is present, since the secret would otherwise leak in cleartext.
+// Without a key there is no secret to leak. An empty endpoint is a no-op.
+func validateLLMEndpoint(endpoint, provider, apiKey string) error {
+	if endpoint == "" {
+		return nil
+	}
+	if !config.IsHTTPURL(endpoint) {
+		return fmt.Errorf("must be an http:// or https:// URL with a host")
+	}
+	switch provider {
+	case "ollama", "apple", "none", "disabled":
+		// Credential-less providers never send the key; skip the HTTPS gate.
+		return nil
+	}
+	if apiKey != "" {
+		return hostutil.RequireSecureURL(endpoint)
+	}
+	return nil
+}
+
 // isConfigCmd returns true if cmd is "config" or any of its subcommands.
 // Used to skip HTTPS enforcement so users can repair a bad base_url.
 func isConfigCmd(cmd *cobra.Command) bool {

internal/cli/root_test.go

@@ -14,6 +14,51 @@ import (
 	"github.com/basecamp/basecamp-cli/internal/version"
 )
 
+// TestLLMEndpointValidation exercises the production validateLLMEndpoint helper
+// in root.go: the scheme/host check is unconditional, while the HTTPS gate is
+// only enforced for credentialed/ambiguous providers when a key is present.
+// Credential-less providers (ollama, apple, none) never send the key, so a
+// remote http endpoint is allowed even when a key exists for a different provider.
+func TestLLMEndpointValidation(t *testing.T) {
+	tests := []struct {
+		name     string
+		endpoint string
+		provider string
+		apiKey   string
+		wantOK   bool
+	}{
+		{"file scheme rejected", "file:///etc/passwd", "", "", false},
+		{"ssh scheme rejected", "ssh://host", "", "", false},
+		{"hostless https rejected", "https:example.com", "", "", false},
+		{"http remote with credential rejected", "http://remote-host:1234", "", "secret", false},
+		{"http remote without credential accepted", "http://remote-host:1234", "", "", true},
+		{"https remote accepted", "https://remote-host", "", "", true},
+		{"https remote with credential accepted", "https://remote-host", "", "secret", true},
+		{"localhost with credential accepted", "http://localhost:11434", "", "secret", true},
+		{"empty endpoint no-op", "", "", "", true},
+		{"empty endpoint with key no-op", "", "", "secret", true},
+		// Credential-less provider: remote http allowed even with a stray key.
+		{"ollama remote http with key accepted", "http://192.168.1.10:11434", "ollama", "secret", true},
+		// Credentialed/ambiguous providers: key still gates remote http.
+		{"openai remote http with key rejected", "http://remote:1234", "openai", "secret", false},
+		{"auto remote http with key rejected", "http://remote:1234", "auto", "secret", false},
+		{"anthropic remote http with key rejected", "http://remote:1234", "anthropic", "secret", false},
+		// Unconditional scheme check fires before the credential-less exemption.
+		{"ollama file scheme rejected", "file:///etc/passwd", "ollama", "secret", false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := validateLLMEndpoint(tt.endpoint, tt.provider, tt.apiKey)
+			if tt.wantOK {
+				assert.NoError(t, err)
+			} else {
+				assert.Error(t, err)
+			}
+		})
+	}
+}
+
 func TestResolvePreferences(t *testing.T) {
 	boolPtr := func(b bool) *bool { return &b }
 	intPtr := func(i int) *int { return &i }

internal/commands/config.go

@@ -297,6 +297,11 @@ Valid keys: account_id, project_id (or project), todolist_id, base_url, cache_di
 					return output.ErrUsage(fmt.Sprintf("llm_provider must be one of: anthropic, openai, ollama, apple, none, auto (got %q)", value))
 				}
 				configData[key] = value
+			case "llm_endpoint":
+				if !config.IsHTTPURL(value) {
+					return output.ErrUsage("llm_endpoint must be an http:// or https:// URL with a host")
+				}
+				configData[key] = value
 			case "llm_max_concurrent":
 				level, err := strconv.Atoi(value)
 				if err != nil || level < 1 || level > 10 {
@@ -343,12 +348,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 +412,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 || (r >= 0x80 && r <= 0x9f) {
+			return -1
+		}
+		return r
+	}, s)
+}
+
 // rankProjects returns projects sorted by priority:
 // 1. HQ (purpose="hq")
 // 2. Bookmarked

internal/completion/completer_test.go

@@ -24,6 +24,28 @@ func newTestCmd() *cobra.Command {
 	return cmd
 }
 
+func TestSanitizeCompletionDesc(t *testing.T) {
+	tests := []struct {
+		name string
+		in   string
+		want string
+	}{
+		{"C0 control SOH stripped", "a\x01b", "ab"},
+		{"C0 control US stripped", "a\x1fb", "ab"},
+		{"DEL stripped", "a\x7fb", "ab"},
+		{"C1 control 0x80 stripped", "a\u0080b", "ab"},
+		{"C1 control 0x9f stripped", "a\u009fb", "ab"},
+		{"valid accented rune passes through", "café", "café"},
+		{"valid emoji rune passes through", "party🎉", "party🎉"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.want, sanitizeCompletionDesc(tt.in))
+		})
+	}
+}
+
 func TestRankProjects(t *testing.T) {
 	now := time.Now()
 	projects := []CachedProject{