diff --git a/ROADMAP.md b/ROADMAP.md index 88499a9b0..c3e5f9cfd 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1035,7 +1035,7 @@ Legend: `shipped` ≥95% checked · `in-flight` 1–94% · `drafted` 0% · `—` | [105-agent-scope-hardening](./specs/105-agent-scope-hardening/) | `in-flight` | 94/113 (83%) | | [106-security-residual-fixes](./specs/106-security-residual-fixes/) | `shipped` | 18/19 (95%) | | [107-server-edition-sso-hardening](./specs/107-server-edition-sso-hardening/) | `shipped` | 126/126 (100%) | -| [108-profiles-v3](./specs/108-profiles-v3/) | `shipped` | 185/186 (99%) | +| [108-profiles-v3](./specs/108-profiles-v3/) | `shipped` | 189/190 (99%) | | [109-ux-navigation-consistency](./specs/109-ux-navigation-consistency/) | `shipped` | 233/234 (100%) | | [110-catalog-popularity](./specs/110-catalog-popularity/) | `in-flight` | 19/23 (83%) | | [112-client-header-forwarding](./specs/112-client-header-forwarding/) | `shipped` | 38/40 (95%) | diff --git a/cmd/mcpproxy/auth_cmd.go b/cmd/mcpproxy/auth_cmd.go index 00a05c8b0..4114efe13 100644 --- a/cmd/mcpproxy/auth_cmd.go +++ b/cmd/mcpproxy/auth_cmd.go @@ -6,7 +6,6 @@ import ( "fmt" "io" "os" - "path/filepath" "strings" "time" @@ -105,19 +104,19 @@ func init() { authLoginCmd.Flags().BoolVar(&authAll, "all", false, "Authenticate all servers that require OAuth") authLoginCmd.Flags().BoolVar(&authForce, "force", false, "Skip confirmation prompt when using --all") authLoginCmd.Flags().StringVarP(&authLogLevel, "log-level", "l", "info", "Log level (trace, debug, info, warn, error)") - authLoginCmd.Flags().StringVarP(&authConfigPath, "config", "c", "", "Path to MCP configuration file (default: ~/.mcpproxy/mcp_config.json)") + addConfigFlag(authLoginCmd.Flags(), &authConfigPath, "Path to MCP configuration file (default: ~/.mcpproxy/mcp_config.json)") authLoginCmd.Flags().DurationVar(&authTimeout, "timeout", 5*time.Minute, "Authentication timeout") // Define flags for auth status command authStatusCmd.Flags().StringVarP(&authServerName, "server", "s", "", "Server name to check status for (optional)") authStatusCmd.Flags().StringVarP(&authLogLevel, "log-level", "l", "info", "Log level (trace, debug, info, warn, error)") - authStatusCmd.Flags().StringVarP(&authConfigPath, "config", "c", "", "Path to MCP configuration file (default: ~/.mcpproxy/mcp_config.json)") + addConfigFlag(authStatusCmd.Flags(), &authConfigPath, "Path to MCP configuration file (default: ~/.mcpproxy/mcp_config.json)") authStatusCmd.Flags().BoolVar(&authAll, "all", false, "Show status for all servers") // Define flags for auth logout command authLogoutCmd.Flags().StringVarP(&authServerName, "server", "s", "", "Server name to logout from (required)") authLogoutCmd.Flags().StringVarP(&authLogLevel, "log-level", "l", "info", "Log level (trace, debug, info, warn, error)") - authLogoutCmd.Flags().StringVarP(&authConfigPath, "config", "c", "", "Path to MCP configuration file (default: ~/.mcpproxy/mcp_config.json)") + addConfigFlag(authLogoutCmd.Flags(), &authConfigPath, "Path to MCP configuration file (default: ~/.mcpproxy/mcp_config.json)") authLogoutCmd.Flags().DurationVar(&authTimeout, "timeout", 30*time.Second, "Logout timeout") // Mark required flags @@ -585,20 +584,17 @@ func displayAuthStatusPretty(servers []map[string]interface{}) error { } func loadAuthConfig() (*config.Config, error) { - var configFile string - if authConfigPath != "" { - configFile = authConfigPath - } else { - homeDir, err := os.UserHomeDir() - if err != nil { - return nil, fmt.Errorf("failed to get user home directory: %w", err) + cfgPath := resolveCLIConfigPath(authConfigPath) + if cfgPath == "" { + var err error + if cfgPath, err = defaultHomeConfigPath(); err != nil { + return nil, err } - configFile = filepath.Join(homeDir, ".mcpproxy", "mcp_config.json") } - globalConfig, err := config.LoadFromFile(configFile) + globalConfig, err := config.LoadFromFile(cfgPath) if err != nil { - return nil, fmt.Errorf("failed to load config from %s: %w", configFile, err) + return nil, fmt.Errorf("failed to load config from %s: %w", cfgPath, err) } // Respect global --data-dir flag diff --git a/cmd/mcpproxy/call_cmd.go b/cmd/mcpproxy/call_cmd.go index 09afbf80d..70ba9b93e 100644 --- a/cmd/mcpproxy/call_cmd.go +++ b/cmd/mcpproxy/call_cmd.go @@ -5,7 +5,6 @@ import ( "encoding/json" "fmt" "os" - "path/filepath" "strings" "time" @@ -147,7 +146,7 @@ func init() { callToolCmd.Flags().StringVarP(&callToolName, "tool-name", "t", "", "Tool name in format server:tool_name (required)") callToolCmd.Flags().StringVarP(&callJSONArgs, "json_args", "j", "{}", "JSON arguments for the tool (default: {})") callToolCmd.Flags().StringVarP(&callLogLevel, "log-level", "l", "info", "Log level (trace, debug, info, warn, error)") - callToolCmd.Flags().StringVarP(&callConfigPath, "config", "c", "", "Path to MCP configuration file (default: ~/.mcpproxy/mcp_config.json)") + addConfigFlag(callToolCmd.Flags(), &callConfigPath, "Path to MCP configuration file (default: ~/.mcpproxy/mcp_config.json)") callToolCmd.Flags().DurationVar(&callTimeout, "timeout", 30*time.Second, "Tool call timeout") callToolCmd.Flags().StringVarP(&callOutputFormat, "output", "o", "pretty", "Output format (pretty, json)") @@ -179,7 +178,7 @@ func setupToolVariantFlags(cmd *cobra.Command) { cmd.Flags().StringVarP(&callToolName, "tool-name", "t", "", "Tool name in format server:tool_name (required)") cmd.Flags().StringVarP(&callJSONArgs, "json_args", "j", "{}", "JSON arguments for the tool (default: {})") cmd.Flags().StringVarP(&callLogLevel, "log-level", "l", "info", "Log level (trace, debug, info, warn, error)") - cmd.Flags().StringVarP(&callConfigPath, "config", "c", "", "Path to MCP configuration file (default: ~/.mcpproxy/mcp_config.json)") + addConfigFlag(cmd.Flags(), &callConfigPath, "Path to MCP configuration file (default: ~/.mcpproxy/mcp_config.json)") cmd.Flags().DurationVar(&callTimeout, "timeout", 30*time.Second, "Tool call timeout") cmd.Flags().StringVarP(&callOutputFormat, "output", "o", "pretty", "Output format (pretty, json)") @@ -207,17 +206,12 @@ Example: // loadCallConfig loads the MCP configuration file for call command func loadCallConfig() (*config.Config, error) { - var configFilePath string - - if callConfigPath != "" { - configFilePath = callConfigPath - } else { - // Use default path - homeDir, err := os.UserHomeDir() - if err != nil { - return nil, fmt.Errorf("failed to get user home directory: %w", err) + configFilePath := resolveCLIConfigPath(callConfigPath) + if configFilePath == "" { + var err error + if configFilePath, err = defaultHomeConfigPath(); err != nil { + return nil, err } - configFilePath = filepath.Join(homeDir, ".mcpproxy", "mcp_config.json") } // Check if config file exists diff --git a/cmd/mcpproxy/catalog_cmd.go b/cmd/mcpproxy/catalog_cmd.go index 95214c741..af64b3c01 100644 --- a/cmd/mcpproxy/catalog_cmd.go +++ b/cmd/mcpproxy/catalog_cmd.go @@ -73,7 +73,7 @@ the curated official/popular sections with no query. 'catalog search'/'catalog add' supersede 'registry search'/'registry add'. 'registry list'/'add-source'/'edit'/'remove' still manage catalog SOURCES.`, } - cmd.PersistentFlags().StringVarP(®istryConfigPath, "config", "c", "", "Path to MCP configuration file") + addConfigFlag(cmd.PersistentFlags(), ®istryConfigPath, "Path to MCP configuration file") cmd.AddCommand(newCatalogSearchCmd(), newCatalogShowCmd(), newCatalogAddCmd()) return cmd } diff --git a/cmd/mcpproxy/cli_config.go b/cmd/mcpproxy/cli_config.go index 8ea62cc85..6008a661e 100644 --- a/cmd/mcpproxy/cli_config.go +++ b/cmd/mcpproxy/cli_config.go @@ -1,8 +1,10 @@ package main import ( + "fmt" "io" "os" + "path/filepath" "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" ) @@ -23,8 +25,14 @@ var cliDiagnosticsWriter io.Writer = os.Stderr func loadCLIConfig(explicitPath string) (*config.Config, error) { var cfg *config.Config var err error - if explicitPath != "" { - cfg, err = config.LoadFromFile(explicitPath) + if path := resolveCLIConfigPath(explicitPath); path != "" { + cfg, err = config.LoadFromFile(path) + } else if dataDir != "" && !legacyConfigExists() { + // A --data-dir was given but no config file exists anywhere: use + // defaults rooted at that directory. Legacy discovery would create + // $HOME/.mcpproxy/mcp_config.json and report the HOME defaults, which + // contradicts the data dir the operator named. + cfg = config.DefaultConfig() } else { cfg, err = config.Load() } @@ -42,6 +50,52 @@ func loadCLIConfig(explicitPath string) (*config.Config, error) { return cfg, nil } +// resolveCLIConfigPath picks the config file a management command reads. The +// command's own --config wins, then the global -c/--config, then +// /mcp_config.json when --data-dir was given and that file exists. +// "" means legacy discovery (cwd, then $HOME/.mcpproxy). Every per-command +// loader goes through it so the global flags behave the same in every position +// and for every subcommand, whether or not it registers a local --config. +func resolveCLIConfigPath(local string) string { + if local != "" { + return local + } + if configFile != "" { + return configFile + } + if dataDir != "" { + p := config.GetConfigPath(dataDir) + if _, err := os.Stat(p); err == nil { + return p + } + } + return "" +} + +// legacyConfigExists reports whether the loader's discovery locations (cwd, +// then $HOME/.mcpproxy) already hold a config file. +func legacyConfigExists() bool { + if _, err := os.Stat(config.ConfigFileName); err == nil { + return true + } + if home, err := os.UserHomeDir(); err == nil { + if _, err := os.Stat(filepath.Join(home, config.DefaultDataDir, config.ConfigFileName)); err == nil { + return true + } + } + return false +} + +// defaultHomeConfigPath is the documented fallback for loaders that require an +// existing file (auth, call, code, tools). +func defaultHomeConfigPath() (string, error) { + home, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("failed to get user home directory: %w", err) + } + return filepath.Join(home, ".mcpproxy", "mcp_config.json"), nil +} + // Per-command loaders for commands whose config flow previously used bare // config.Load() and ignored --data-dir (GH #908). diff --git a/cmd/mcpproxy/cli_config_resolve_test.go b/cmd/mcpproxy/cli_config_resolve_test.go new file mode 100644 index 000000000..b73c2807c --- /dev/null +++ b/cmd/mcpproxy/cli_config_resolve_test.go @@ -0,0 +1,201 @@ +package main + +import ( + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" +) + +// loaderCase describes one per-command config loader and how to set/clear its +// local --config variable. +type loaderCase struct { + name string + setPath func(string) + load func() (*config.Config, error) +} + +func managementLoaderCases() []loaderCase { + return []loaderCase{ + {"doctor", func(p string) { doctorConfigPath = p }, loadDoctorConfig}, + {"auth", func(p string) { authConfigPath = p }, loadAuthConfig}, + {"call", func(p string) { callConfigPath = p }, loadCallConfig}, + {"code", func(p string) { codeConfigPath = p }, loadCodeConfig}, + {"tools", func(p string) { configPath = p }, loadToolsConfig}, + {"token", func(p string) { tokenConfigPath = p }, loadTokenConfig}, + {"registry", func(p string) { registryConfigPath = p }, loadRegistryConfig}, + {"upstream", func(p string) { upstreamConfigPath = p }, loadUpstreamConfig}, + {"status", func(string) {}, loadStatusConfig}, + } +} + +func writeListenConfig(t *testing.T, path, listen string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatal(err) + } + body := fmt.Sprintf(`{"listen":%q,"mcpServers":[]}`, listen) + if err := os.WriteFile(path, []byte(body), 0o600); err != nil { + t.Fatal(err) + } +} + +// resetLoaderGlobals clears every path variable the loaders read and restores +// them after the test. +func resetLoaderGlobals(t *testing.T) { + t.Helper() + oldCfg, oldDir := configFile, dataDir + t.Cleanup(func() { + configFile, dataDir = oldCfg, oldDir + for _, tc := range managementLoaderCases() { + tc.setPath("") + } + }) + configFile, dataDir = "", "" + for _, tc := range managementLoaderCases() { + tc.setPath("") + } +} + +// F-06: the global -c applies to every management command even though the +// command registers (and leaves empty) its own local --config. +func TestLoadersHonorGlobalConfigFlag(t *testing.T) { + for _, tc := range managementLoaderCases() { + t.Run(tc.name, func(t *testing.T) { + resetLoaderGlobals(t) + home := t.TempDir() + t.Setenv("HOME", home) + cfgPath := filepath.Join(t.TempDir(), "global.json") + writeListenConfig(t, cfgPath, "127.0.0.1:18999") + configFile = cfgPath + + cfg, err := tc.load() + if err != nil { + t.Fatalf("load: %v", err) + } + if cfg.Listen != "127.0.0.1:18999" { + t.Errorf("listen = %q, want the global -c file's 127.0.0.1:18999", cfg.Listen) + } + }) + } +} + +// F-06: with only -d, management commands read /mcp_config.json and +// never create a default under $HOME. +func TestLoadersPreferDataDirConfig(t *testing.T) { + for _, tc := range managementLoaderCases() { + t.Run(tc.name, func(t *testing.T) { + resetLoaderGlobals(t) + home := t.TempDir() + t.Setenv("HOME", home) + t.Chdir(t.TempDir()) + d := filepath.Join(t.TempDir(), "d") + writeListenConfig(t, filepath.Join(d, "mcp_config.json"), "127.0.0.1:18998") + dataDir = d + + cfg, err := tc.load() + if err != nil { + t.Fatalf("load: %v", err) + } + if cfg.Listen != "127.0.0.1:18998" { + t.Errorf("listen = %q, want the data dir's 127.0.0.1:18998", cfg.Listen) + } + if cfg.DataDir != d { + t.Errorf("DataDir = %q, want %q", cfg.DataDir, d) + } + if _, statErr := os.Stat(filepath.Join(home, ".mcpproxy", "mcp_config.json")); statErr == nil { + t.Error("a default config was created under $HOME") + } + }) + } +} + +// F-06 (review O1): with only -d and no config file anywhere, every +// management loader (registry/catalog included) must neither fail nor create +// $HOME/.mcpproxy/mcp_config.json. +func TestLoadersDataDirWithoutConfigCreateNothingInHome(t *testing.T) { + for _, tc := range managementLoaderCases() { + t.Run(tc.name, func(t *testing.T) { + resetLoaderGlobals(t) + home := t.TempDir() + t.Setenv("HOME", home) + t.Chdir(t.TempDir()) + d := filepath.Join(t.TempDir(), "nonexistent") + dataDir = d + + cfg, err := tc.load() + if err != nil { + t.Skipf("loader requires an existing config file: %v", err) + } + if cfg.DataDir != d { + t.Errorf("DataDir = %q, want %q", cfg.DataDir, d) + } + if _, statErr := os.Stat(filepath.Join(home, ".mcpproxy", "mcp_config.json")); statErr == nil { + t.Error("a default config was created under $HOME") + } + }) + } +} + +// A -d without a config file anywhere must not fabricate $HOME/.mcpproxy. +func TestLoadCLIConfigDataDirWithoutFileCreatesNothingInHome(t *testing.T) { + resetLoaderGlobals(t) + home := t.TempDir() + t.Setenv("HOME", home) + t.Chdir(t.TempDir()) + dataDir = filepath.Join(t.TempDir(), "empty") + + cfg, err := loadCLIConfig("") + if err != nil { + t.Fatalf("loadCLIConfig: %v", err) + } + if cfg.DataDir != dataDir { + t.Errorf("DataDir = %q, want %q", cfg.DataDir, dataDir) + } + if _, statErr := os.Stat(filepath.Join(home, ".mcpproxy")); statErr == nil { + t.Error("$HOME/.mcpproxy was created") + } +} + +func TestUpstreamConfigFilePathMatchesLoadPath(t *testing.T) { + resetLoaderGlobals(t) + t.Setenv("HOME", t.TempDir()) + t.Chdir(t.TempDir()) + d := filepath.Join(t.TempDir(), "d") + want := filepath.Join(d, "mcp_config.json") + writeListenConfig(t, want, "127.0.0.1:18997") + dataDir = d + + cfg, err := loadUpstreamConfig() + if err != nil { + t.Fatal(err) + } + if got := upstreamConfigFilePath(cfg); got != want { + t.Errorf("upstreamConfigFilePath = %q, want the file that was read: %q", got, want) + } +} + +func TestResolveCLIConfigPathPrecedence(t *testing.T) { + resetLoaderGlobals(t) + d := t.TempDir() + writeListenConfig(t, filepath.Join(d, "mcp_config.json"), "127.0.0.1:1") + dataDir = d + + if got := resolveCLIConfigPath("local.json"); got != "local.json" { + t.Errorf("local flag must win, got %q", got) + } + configFile = "global.json" + if got := resolveCLIConfigPath(""); got != "global.json" { + t.Errorf("global -c must beat -d, got %q", got) + } + configFile = "" + if got := resolveCLIConfigPath(""); got != filepath.Join(d, "mcp_config.json") { + t.Errorf("-d config must be used when present, got %q", got) + } + dataDir = t.TempDir() + if got := resolveCLIConfigPath(""); got != "" { + t.Errorf("-d without a config file falls back to discovery, got %q", got) + } +} diff --git a/cmd/mcpproxy/code_cmd.go b/cmd/mcpproxy/code_cmd.go index e15a1b2e7..c9e5adfd8 100644 --- a/cmd/mcpproxy/code_cmd.go +++ b/cmd/mcpproxy/code_cmd.go @@ -7,7 +7,6 @@ import ( "fmt" "io" "os" - "path/filepath" "strings" "time" @@ -134,12 +133,12 @@ func init() { codeExecCmd.Flags().IntVar(&codeMaxToolCalls, "max-tool-calls", 0, "Maximum number of tool calls (0 = unlimited)") codeExecCmd.Flags().StringSliceVar(&codeAllowedSrvs, "allowed-servers", []string{}, "Comma-separated list of allowed server names (empty = all allowed)") codeExecCmd.Flags().StringVarP(&codeLogLevel, "log-level", "l", "info", "Log level (trace, debug, info, warn, error)") - codeExecCmd.Flags().StringVarP(&codeConfigPath, "config", "c", "", "Path to MCP configuration file (default: ~/.mcpproxy/mcp_config.json)") + addConfigFlag(codeExecCmd.Flags(), &codeConfigPath, "Path to MCP configuration file (default: ~/.mcpproxy/mcp_config.json)") codeExecCmd.Flags().StringVar(&codeLanguage, "language", "javascript", "Source code language: javascript, typescript") // The scripts commands resolve the same config FILE as exec, so they take // the same --config override. - codeScriptsListCmd.Flags().StringVarP(&codeConfigPath, "config", "c", "", "Path to MCP configuration file (default: ~/.mcpproxy/mcp_config.json)") + addConfigFlag(codeScriptsListCmd.Flags(), &codeConfigPath, "Path to MCP configuration file (default: ~/.mcpproxy/mcp_config.json)") // Add examples codeExecCmd.Example = ` # Execute inline code with input @@ -652,14 +651,10 @@ func outputResultFromMCP(result *mcp.CallToolResult) error { // config file has been chosen, so deriving from it would disagree with the // file actually loaded (and with the daemon) about where stored scripts live. func codeConfigFilePath() (string, error) { - if codeConfigPath != "" { - return codeConfigPath, nil + if p := resolveCLIConfigPath(codeConfigPath); p != "" { + return p, nil } - homeDir, err := os.UserHomeDir() - if err != nil { - return "", fmt.Errorf("failed to get user home directory: %w", err) - } - return filepath.Join(homeDir, ".mcpproxy", "mcp_config.json"), nil + return defaultHomeConfigPath() } // loadCodeConfig loads the MCP configuration file for code command diff --git a/cmd/mcpproxy/doctor_cmd.go b/cmd/mcpproxy/doctor_cmd.go index 398bbfc49..9e27cf7b3 100644 --- a/cmd/mcpproxy/doctor_cmd.go +++ b/cmd/mcpproxy/doctor_cmd.go @@ -11,6 +11,7 @@ import ( "github.com/spf13/cobra" "go.uber.org/zap" + clioutput "github.com/smart-mcp-proxy/mcpproxy-go/internal/cli/output" "github.com/smart-mcp-proxy/mcpproxy-go/internal/cliclient" "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" "github.com/smart-mcp-proxy/mcpproxy-go/internal/contracts" @@ -74,16 +75,27 @@ func GetDoctorCommand() *cobra.Command { } func init() { - doctorCmd.Flags().StringVarP(&doctorOutput, "output", "o", "pretty", "Output format (pretty, json)") + doctorCmd.Flags().StringVarP(&doctorOutput, "output", "o", "pretty", "Output format (pretty, json, yaml)") doctorCmd.Flags().StringVarP(&doctorLogLevel, "log-level", "l", "warn", "Log level") - doctorCmd.Flags().StringVarP(&doctorConfigPath, "config", "c", "", "Path to config file") + addConfigFlag(doctorCmd.Flags(), &doctorConfigPath, "Path to config file") doctorCmd.Flags().StringVar(&doctorServerFilter, "server", "", "Limit health checks to a single upstream server (by name)") } -func runDoctor(_ *cobra.Command, _ []string) error { +func runDoctor(cmd *cobra.Command, _ []string) error { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() + // The local -o shadows the root -o/--json; honour the global ones when + // doctor's own flag was not given. + if !cmd.Flags().Changed("output") { + switch { + case globalJSONOutput: + doctorOutput = "json" + case globalOutputFormat != "" && globalOutputFormat != "table": + doctorOutput = globalOutputFormat + } + } + // Load configuration globalConfig, err := loadDoctorConfig() if err != nil { @@ -91,6 +103,9 @@ func runDoctor(_ *cobra.Command, _ []string) error { return err } + doctorSecretLiterals = []string{globalConfig.APIKey} + defer func() { doctorSecretLiterals = nil }() + // Create logger logger, err := createDoctorLogger(doctorLogLevel) if err != nil { @@ -330,8 +345,18 @@ func outputDiagnostics(diag map[string]interface{}, info map[string]interface{}, } func outputDiagnosticsWithProfileChecks(diag map[string]interface{}, info map[string]interface{}, quarantineStats []quarantineServerStats, envHint string, attention *cliclient.AttentionResponse, profileChecks []profileCheck) error { + // The one funnel for every doctor format: credentials (the keyed + // web_ui_url from GET /api/v1/info, tokens in upstream error messages, the + // admin key itself) never reach stdout. The report is made to be shared. + diag = redactDoctorMap(diag) + info = redactDoctorMap(info) + envHint = redactDoctorString(envHint) + attention = redactDoctorTyped(attention) + profileChecks = redactDoctorTyped(profileChecks) + quarantineStats = redactDoctorTyped(quarantineStats) + switch doctorOutput { - case "json": + case "json", "yaml": // Combine diagnostics with info for JSON output combined := map[string]interface{}{ "diagnostics": diag, @@ -351,6 +376,18 @@ func outputDiagnosticsWithProfileChecks(diag map[string]interface{}, info map[st if profileChecks != nil { combined["profile_checks"] = profileChecks } + if doctorOutput == "yaml" { + formatter, err := clioutput.NewFormatter("yaml") + if err != nil { + return err + } + output, err := formatter.Format(combined) + if err != nil { + return fmt.Errorf("failed to format output: %w", err) + } + fmt.Print(output) + return nil + } output, err := json.MarshalIndent(combined, "", " ") if err != nil { return fmt.Errorf("failed to format output: %w", err) @@ -601,6 +638,8 @@ func outputDiagnosticsWithProfileChecks(diag map[string]interface{}, info map[st fmt.Println("━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━") displaySecurityFeaturesStatus() fmt.Println("━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━") + default: + return fmt.Errorf("unsupported output format %q for doctor (pretty, json, yaml)", doctorOutput) } return nil diff --git a/cmd/mcpproxy/doctor_fix_cmd.go b/cmd/mcpproxy/doctor_fix_cmd.go index ed70e9359..6cdf429db 100644 --- a/cmd/mcpproxy/doctor_fix_cmd.go +++ b/cmd/mcpproxy/doctor_fix_cmd.go @@ -53,7 +53,7 @@ func init() { doctorFixCmd.Flags().BoolVar(&doctorFixExecute, "execute", false, "Apply the fix (default: dry_run)") doctorFixCmd.Flags().StringVarP(&doctorFixOutput, "output", "o", "pretty", "Output format (pretty, json)") doctorFixCmd.Flags().StringVarP(&doctorFixLogLevel, "log-level", "l", "warn", "Log level") - doctorFixCmd.Flags().StringVarP(&doctorFixConfigPth, "config", "c", "", "Path to config file") + addConfigFlag(doctorFixCmd.Flags(), &doctorFixConfigPth, "Path to config file") _ = doctorFixCmd.MarkFlagRequired("server") doctorCmd.AddCommand(doctorFixCmd) } diff --git a/cmd/mcpproxy/doctor_redact.go b/cmd/mcpproxy/doctor_redact.go new file mode 100644 index 000000000..ae89d0729 --- /dev/null +++ b/cmd/mcpproxy/doctor_redact.go @@ -0,0 +1,101 @@ +package main + +import ( + "encoding/json" + "reflect" + "regexp" + "strings" + + "github.com/smart-mcp-proxy/mcpproxy-go/internal/oauth" +) + +// doctorRedactionMask replaces credential query params, URL userinfo passwords +// and the literal admin key in every `mcpproxy doctor` output format. The +// report is meant to be pasted into issues and chats, so there is no opt-out; +// `mcpproxy status --show-key` / `--web-url` are the explicit ways to get the +// key. +const doctorRedactionMask = "REDACTED" + +// doctorSecretLiterals holds secrets (the admin API key) that must never appear +// in doctor output even outside a URL. runDoctor sets it for the run. +var doctorSecretLiterals []string + +var doctorURLPattern = regexp.MustCompile(`(?i)\b[a-z][a-z0-9+.-]*://[^\s"'<>]+`) + +func doctorMask(string) string { return doctorRedactionMask } + +// redactDoctorString masks credentials in every URL embedded in s, then scrubs +// each registered secret literal. +func redactDoctorString(s string) string { + if s == "" { + return s + } + if strings.Contains(s, "://") { + s = doctorURLPattern.ReplaceAllStringFunc(s, func(u string) string { + return oauth.RedactURLQueryParamsWith(u, doctorMask) + }) + } + for _, lit := range doctorSecretLiterals { + if lit != "" { + s = strings.ReplaceAll(s, lit, doctorRedactionMask) + } + } + return s +} + +// redactDoctorValue walks generic JSON values (maps, slices, strings) and +// returns a redacted copy; other scalars pass through. +func redactDoctorValue(v interface{}) interface{} { + switch t := v.(type) { + case string: + return redactDoctorString(t) + case map[string]interface{}: + out := make(map[string]interface{}, len(t)) + for k, val := range t { + out[k] = redactDoctorValue(val) + } + return out + case []interface{}: + out := make([]interface{}, len(t)) + for i, val := range t { + out[i] = redactDoctorValue(val) + } + return out + default: + return v + } +} + +// redactDoctorTyped redacts a typed payload (attention response, profile +// checks, ...) through its JSON form. When nothing needs redacting the original +// value is returned untouched, so field order in the JSON output is unchanged. +func redactDoctorTyped[T any](v T) T { + raw, err := json.Marshal(v) + if err != nil { + return v + } + var generic interface{} + if err := json.Unmarshal(raw, &generic); err != nil { + return v + } + redacted := redactDoctorValue(generic) + if reflect.DeepEqual(generic, redacted) { + return v + } + rawRedacted, err := json.Marshal(redacted) + if err != nil { + return v + } + var out T + if err := json.Unmarshal(rawRedacted, &out); err != nil { + return v + } + return out +} + +func redactDoctorMap(m map[string]interface{}) map[string]interface{} { + if m == nil { + return nil + } + return redactDoctorValue(m).(map[string]interface{}) +} diff --git a/cmd/mcpproxy/doctor_redact_test.go b/cmd/mcpproxy/doctor_redact_test.go new file mode 100644 index 000000000..161bdfceb --- /dev/null +++ b/cmd/mcpproxy/doctor_redact_test.go @@ -0,0 +1,152 @@ +package main + +import ( + "encoding/json" + "strings" + "testing" +) + +const ( + redactTestAdminKey = "SECRET-ADMIN-KEY-0123456789" + redactTestToken = "tok-SECRET-999" +) + +func redactTestInputs() (diag, info map[string]interface{}) { + diag = map[string]interface{}{ + "total_issues": 1, + "upstream_errors": []interface{}{ + map[string]interface{}{ + "server": "github", + "error_message": "dial https://h.example/mcp?token=" + redactTestToken + "&x=1 failed", + }, + }, + } + info = map[string]interface{}{ + "version": "v0.0.0", + "web_ui_url": "http://127.0.0.1:8080/ui/?apikey=" + redactTestAdminKey, + } + return diag, info +} + +func TestDoctorOutput_RedactsAdminKeyInEveryFormat(t *testing.T) { + oldFmt := doctorOutput + t.Cleanup(func() { doctorOutput = oldFmt }) + + for _, format := range []string{"json", "yaml", "pretty"} { + t.Run(format, func(t *testing.T) { + doctorOutput = format + diag, info := redactTestInputs() + var runErr error + stdout, stderr := captureStd(func() { + runErr = outputDiagnostics(diag, info, nil, "", nil) + }) + if runErr != nil { + t.Fatalf("outputDiagnostics: %v", runErr) + } + all := stdout + stderr + for _, secret := range []string{redactTestAdminKey, redactTestToken} { + if strings.Contains(all, secret) { + t.Fatalf("%s output leaks %q:\n%s", format, secret, all) + } + } + if format == "pretty" { + return + } + if strings.TrimSpace(stdout) == "" { + t.Fatalf("%s output is empty", format) + } + if !strings.Contains(stdout, "web_ui_url") || !strings.Contains(stdout, "apikey=REDACTED") { + t.Errorf("%s output must keep web_ui_url with apikey=REDACTED:\n%s", format, stdout) + } + if !strings.Contains(stdout, "x=1") { + t.Errorf("%s output must keep non-secret query params:\n%s", format, stdout) + } + if format == "json" { + var parsed map[string]interface{} + if err := json.Unmarshal([]byte(stdout), &parsed); err != nil { + t.Fatalf("json output does not parse: %v", err) + } + } + }) + } +} + +func TestDoctorOutput_ScrubsAdminKeyLiteral(t *testing.T) { + oldFmt, oldLit := doctorOutput, doctorSecretLiterals + t.Cleanup(func() { doctorOutput, doctorSecretLiterals = oldFmt, oldLit }) + doctorOutput = "json" + doctorSecretLiterals = []string{"LITERAL-KEY-ABCDEFGH"} + + diag := map[string]interface{}{ + "total_issues": 1, + "note": "the key LITERAL-KEY-ABCDEFGH leaked outside a URL", + } + var runErr error + stdout, _ := captureStd(func() { + runErr = outputDiagnostics(diag, nil, nil, "", nil) + }) + if runErr != nil { + t.Fatalf("outputDiagnostics: %v", runErr) + } + if strings.Contains(stdout, "LITERAL-KEY-ABCDEFGH") { + t.Fatalf("literal admin key leaked:\n%s", stdout) + } + if !strings.Contains(stdout, "REDACTED") { + t.Fatalf("expected REDACTED marker:\n%s", stdout) + } +} + +func TestDoctorOutput_UnsupportedFormatErrors(t *testing.T) { + oldFmt := doctorOutput + t.Cleanup(func() { doctorOutput = oldFmt }) + doctorOutput = "xml" + + var runErr error + captureStd(func() { + runErr = outputDiagnostics(map[string]interface{}{"total_issues": 0}, nil, nil, "", nil) + }) + if runErr == nil { + t.Fatal("expected an error for an unsupported doctor output format") + } + for _, want := range []string{`"xml"`, "pretty", "json", "yaml"} { + if !strings.Contains(runErr.Error(), want) { + t.Errorf("error %q must mention %s", runErr.Error(), want) + } + } +} + +func TestStatusOutput_MasksKeyInEveryFormat(t *testing.T) { + const key = "test0123456789abcdefghijklmnopqrstuvwxyz6789" + oldShow := statusShowKey + t.Cleanup(func() { statusShowKey = oldShow }) + + for _, format := range []string{"json", "yaml", "table"} { + for _, show := range []bool{false, true} { + name := format + "/masked" + if show { + name = format + "/show-key" + } + t.Run(name, func(t *testing.T) { + info := &StatusInfo{ + State: "running", + ListenAddr: "127.0.0.1:8080", + APIKey: key, + WebUIURL: "http://127.0.0.1:8080/ui/?apikey=" + key, + Endpoints: map[string]string{}, + } + statusShowKey = show + if !show { + maskStatusCredentials(info) + } + var runErr error + stdout, _ := captureStd(func() { runErr = printStatusOutput(info, format) }) + if runErr != nil { + t.Fatalf("printStatusOutput: %v", runErr) + } + if got := strings.Contains(stdout, key); got != show { + t.Fatalf("key present=%v want %v:\n%s", got, show, stdout) + } + }) + } + } +} diff --git a/cmd/mcpproxy/execute_root_test.go b/cmd/mcpproxy/execute_root_test.go new file mode 100644 index 000000000..8d2fc8f23 --- /dev/null +++ b/cmd/mcpproxy/execute_root_test.go @@ -0,0 +1,84 @@ +package main + +import ( + "bytes" + "strings" + "testing" + + "github.com/spf13/cobra" + + "github.com/smart-mcp-proxy/mcpproxy-go/internal/runtime" +) + +func newExecuteRootFixture(runE func() error) *cobra.Command { + root := &cobra.Command{Use: "mcpproxy"} + root.AddCommand(&cobra.Command{ + Use: "connect", + RunE: func(*cobra.Command, []string) error { return runE() }, + }) + return root +} + +// F-09: a RunE error was printed by cobra AND by main(), so the whole block +// (including the "Fixes:" list) appeared twice. +func TestExecuteRoot_PrintsErrorOnce(t *testing.T) { + guard := &runtime.BindingGuardError{ + Bindings: []runtime.BindingRef{{ClientID: "cursor", Profile: "work-readonly", Mode: "locked"}}, + Fixes: []runtime.GuardFix{ + {Kind: "require_mcp_auth"}, + {Kind: "set_anonymous_profile", Target: "work-readonly"}, + }, + } + root := newExecuteRootFixture(func() error { return describeConnectFailure(guard, "cursor") }) + var cobraErr, stderr bytes.Buffer + root.SetErr(&cobraErr) + root.SetOut(&cobraErr) + root.SetArgs([]string{"connect"}) + + code := executeRoot(root, &stderr) + + all := cobraErr.String() + stderr.String() + if got := strings.Count(all, "Fixes:"); got != 1 { + t.Errorf("Fixes: printed %d times, want 1:\n%s", got, all) + } + if got := strings.Count(all, "Error:"); got != 1 { + t.Errorf("Error: printed %d times, want 1:\n%s", got, all) + } + if code != 1 { + t.Errorf("exit code = %d, want 1", code) + } +} + +func TestExecuteRoot_UnknownCommandKeepsHelpHint(t *testing.T) { + root := newExecuteRootFixture(func() error { return nil }) + var cobraErr, stderr bytes.Buffer + root.SetErr(&cobraErr) + root.SetOut(&cobraErr) + root.SetArgs([]string{"nope"}) + + code := executeRoot(root, &stderr) + + all := cobraErr.String() + stderr.String() + if got := strings.Count(all, `unknown command "nope"`); got != 1 { + t.Errorf("unknown command printed %d times, want 1:\n%s", got, all) + } + if !strings.Contains(all, "Run 'mcpproxy --help' for usage.") { + t.Errorf("missing help hint:\n%s", all) + } + if code == ExitCodeSuccess { + t.Error("exit code must be non-zero") + } +} + +func TestExecuteRoot_SuccessIsSilent(t *testing.T) { + root := newExecuteRootFixture(func() error { return nil }) + var cobraErr, stderr bytes.Buffer + root.SetErr(&cobraErr) + root.SetArgs([]string{"connect"}) + if code := executeRoot(root, &stderr); code != ExitCodeSuccess { + t.Errorf("exit code = %d", code) + } + if cobraErr.Len()+stderr.Len() != 0 { + t.Errorf("unexpected output: %q %q", cobraErr.String(), stderr.String()) + } +} diff --git a/cmd/mcpproxy/main.go b/cmd/mcpproxy/main.go index be5e62848..a4dc67cb3 100644 --- a/cmd/mcpproxy/main.go +++ b/cmd/mcpproxy/main.go @@ -27,6 +27,7 @@ import ( "context" "errors" "fmt" + "io" "os" "os/signal" "strings" @@ -119,8 +120,7 @@ func main() { rootCmd.SetVersionTemplate(versionLine()) // Add global flags - rootCmd.PersistentFlags().StringVarP(&configFile, "config", "c", "", "Configuration file path") - rootCmd.PersistentFlags().StringVarP(&dataDir, "data-dir", "d", "", "Data directory path (default: ~/.mcpproxy)") + registerRootPathFlags(rootCmd) rootCmd.PersistentFlags().StringVar(&logLevel, "log-level", "", "Log level (trace, debug, info, warn, error) - defaults: server=info, other commands=warn") rootCmd.PersistentFlags().BoolVar(&logToFile, "log-to-file", false, "Enable logging to file in standard OS location (default: console only)") rootCmd.PersistentFlags().StringVar(&logDir, "log-dir", "", "Custom log directory path (overrides standard OS location)") @@ -255,14 +255,30 @@ func main() { // Default to server command for backward compatibility rootCmd.RunE = runServer - if err := rootCmd.Execute(); err != nil { - // Check for specific error types to return appropriate exit codes - exitCode := classifyError(err) - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - os.Exit(exitCode) + if code := executeRoot(rootCmd, os.Stderr); code != ExitCodeSuccess { + os.Exit(code) } } +// executeRoot runs the command tree and reports a failure exactly once. Cobra +// prints "Error: " itself unless SilenceErrors is set, and main() used to +// print it again, so every RunE error (including the connect binding-guard +// "Fixes:" list) appeared twice. Errors are silenced in cobra and written here +// once; usage output on flag errors is unchanged (SilenceUsage is untouched). +func executeRoot(root *cobra.Command, stderr io.Writer) int { + root.SilenceErrors = true + err := root.Execute() + if err == nil { + return ExitCodeSuccess + } + fmt.Fprintf(stderr, "Error: %v\n", err) + // Cobra appends this hint itself only when it prints the error. + if strings.HasPrefix(err.Error(), "unknown command") { + fmt.Fprintf(stderr, "Run '%s --help' for usage.\n", root.CommandPath()) + } + return classifyError(err) +} + func createSearchServersCommand() *cobra.Command { var registryFlag, searchFlag, tagFlag string var listRegistries bool diff --git a/cmd/mcpproxy/path_flag.go b/cmd/mcpproxy/path_flag.go new file mode 100644 index 000000000..7d907a9ec --- /dev/null +++ b/cmd/mcpproxy/path_flag.go @@ -0,0 +1,53 @@ +package main + +import ( + "errors" + "strings" + + "github.com/spf13/cobra" + "github.com/spf13/pflag" +) + +// nonEmptyPathValue is the pflag.Value behind every --config / --data-dir flag. +// An explicit empty value (`-c ""`, `--config=`, an unset shell variable that +// expanded to nothing) used to be indistinguishable from "no flag", so a +// management command silently fell back to discovery and, when nothing was +// found, created $HOME/.mcpproxy/mcp_config.json and reported an empty server +// list. Reject it at parse time instead. +type nonEmptyPathValue struct { + target *string +} + +func newPathFlag(p *string) pflag.Value { + return &nonEmptyPathValue{target: p} +} + +func (v *nonEmptyPathValue) String() string { + if v == nil || v.target == nil { + return "" + } + return *v.target +} + +func (v *nonEmptyPathValue) Set(s string) error { + if strings.TrimSpace(s) == "" { + return errors.New("must not be empty; omit the flag to use the default") + } + *v.target = s + return nil +} + +// Type keeps --help and --help-json identical to a plain string flag. +func (v *nonEmptyPathValue) Type() string { return "string" } + +// addConfigFlag registers a --config/-c flag that rejects empty values. +func addConfigFlag(fs *pflag.FlagSet, p *string, usage string) { + fs.VarP(newPathFlag(p), "config", "c", usage) +} + +// registerRootPathFlags registers the root persistent -c/--config and +// -d/--data-dir flags. +func registerRootPathFlags(root *cobra.Command) { + root.PersistentFlags().VarP(newPathFlag(&configFile), "config", "c", "Configuration file path") + root.PersistentFlags().VarP(newPathFlag(&dataDir), "data-dir", "d", "Data directory path (default: ~/.mcpproxy)") +} diff --git a/cmd/mcpproxy/path_flag_test.go b/cmd/mcpproxy/path_flag_test.go new file mode 100644 index 000000000..08bd771fb --- /dev/null +++ b/cmd/mcpproxy/path_flag_test.go @@ -0,0 +1,113 @@ +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/spf13/cobra" + "github.com/spf13/pflag" +) + +func TestConfigFlagsRejectEmptyValue(t *testing.T) { + t.Run("root flags reject empty values", func(t *testing.T) { + oldCfg, oldDir := configFile, dataDir + t.Cleanup(func() { configFile, dataDir = oldCfg, oldDir }) + + cases := []struct { + name string + args []string + want string + }{ + {"short before child", []string{"-c", "", "child"}, "--config"}, + {"long before child", []string{"--config", "", "child"}, "--config"}, + {"equals form", []string{"--config=", "child"}, "--config"}, + {"after child", []string{"child", "-c", ""}, "--config"}, + {"data dir", []string{"-d", "", "child"}, "--data-dir"}, + {"whitespace only", []string{"-c", " ", "child"}, "--config"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + ran := false + root := &cobra.Command{Use: "root", SilenceUsage: true, SilenceErrors: true} + registerRootPathFlags(root) + root.AddCommand(&cobra.Command{Use: "child", RunE: func(*cobra.Command, []string) error { ran = true; return nil }}) + root.SetArgs(tc.args) + err := root.Execute() + if err == nil { + t.Fatalf("expected an error for %v", tc.args) + } + if !strings.Contains(err.Error(), tc.want) || !strings.Contains(err.Error(), "must not be empty") { + t.Errorf("error %q should name %s and say it must not be empty", err.Error(), tc.want) + } + if ran { + t.Error("command must not run with an empty path flag") + } + if _, statErr := os.Stat(filepath.Join(home, ".mcpproxy", "mcp_config.json")); statErr == nil { + t.Error("no default config may be created") + } + if got := classifyError(err); got == ExitCodeSuccess { + t.Error("exit code must be non-zero") + } + }) + } + }) + + t.Run("non-empty values and help type are unchanged", func(t *testing.T) { + oldCfg, oldDir := configFile, dataDir + t.Cleanup(func() { configFile, dataDir = oldCfg, oldDir }) + root := &cobra.Command{Use: "root", SilenceUsage: true, SilenceErrors: true, RunE: func(*cobra.Command, []string) error { return nil }} + registerRootPathFlags(root) + root.SetArgs([]string{"-c", "/x/cfg.json", "-d", "/x/data"}) + if err := root.Execute(); err != nil { + t.Fatal(err) + } + if configFile != "/x/cfg.json" || dataDir != "/x/data" { + t.Errorf("got config=%q data=%q", configFile, dataDir) + } + for _, n := range []string{"config", "data-dir"} { + if typ := root.PersistentFlags().Lookup(n).Value.Type(); typ != "string" { + t.Errorf("%s Type() = %q, want string", n, typ) + } + } + }) + + t.Run("every command-local config flag rejects empty", func(t *testing.T) { + cmds := map[string]*cobra.Command{ + "upstream list": upstreamListCmd, + "upstream logs": upstreamLogsCmd, + "doctor": doctorCmd, + "doctor fix": doctorFixCmd, + "auth login": authLoginCmd, + "auth status": authStatusCmd, + "auth logout": authLogoutCmd, + "call tool": callToolCmd, + "call tool-read": callToolReadCmd, + "call tool-write": callToolWriteCmd, + "call tool-destructive": callToolDestructiveCmd, + "code exec": codeExecCmd, + "code scripts list": codeScriptsListCmd, + "tools list": toolsListCmd, + "token (persistent)": GetTokenCommand(), + "catalog (persistent)": GetCatalogCommand(), + "registry (persistent)": GetRegistryCommand(), + } + for name, c := range cmds { + t.Run(name, func(t *testing.T) { + var f *pflag.Flag + if f = c.Flags().Lookup("config"); f == nil { + f = c.PersistentFlags().Lookup("config") + } + if f == nil { + t.Fatalf("%s has no config flag", name) + } + if err := f.Value.Set(""); err == nil { + t.Errorf("%s: empty --config must be rejected", name) + } + }) + } + }) +} diff --git a/cmd/mcpproxy/registry_cmd.go b/cmd/mcpproxy/registry_cmd.go index 0e307e0c7..b8589d537 100644 --- a/cmd/mcpproxy/registry_cmd.go +++ b/cmd/mcpproxy/registry_cmd.go @@ -67,7 +67,7 @@ daemon. 'list' and 'search' use the daemon when available and otherwise read the registries directly.`, } - cmd.PersistentFlags().StringVarP(®istryConfigPath, "config", "c", "", "Path to MCP configuration file") + addConfigFlag(cmd.PersistentFlags(), ®istryConfigPath, "Path to MCP configuration file") cmd.AddCommand(newRegistryListCmd(), newRegistrySearchCmd(), newRegistryAddCmd(), newRegistryAddSourceCmd(), newRegistryEditCmd(), newRegistryRemoveCmd()) return cmd } @@ -622,19 +622,16 @@ func truncateStr(s string, max int) string { // command's --config flag and the global --data-dir, falling back to defaults // so 'list'/'search' still work without a config file. func loadRegistryConfig() (*config.Config, error) { - var cfg *config.Config - var err error - if registryConfigPath != "" { - cfg, err = config.LoadFromFile(registryConfigPath) - } else { - cfg, err = config.Load() - } + // Go through loadCLIConfig so a --data-dir with no config file anywhere + // uses defaults rooted at that directory instead of creating + // $HOME/.mcpproxy/mcp_config.json. + cfg, err := loadCLIConfig(registryConfigPath) if err != nil { // Discovery should still work with defaults if no config is present. cfg = config.DefaultConfig() - } - if dataDir != "" { - cfg.DataDir = dataDir + if dataDir != "" { + cfg.DataDir = dataDir + } } return cfg, nil } diff --git a/cmd/mcpproxy/token_cmd.go b/cmd/mcpproxy/token_cmd.go index 4899f6ae3..b7eec49bf 100644 --- a/cmd/mcpproxy/token_cmd.go +++ b/cmd/mcpproxy/token_cmd.go @@ -53,7 +53,7 @@ Examples: mcpproxy token revoke deploy-bot`, } - tokenCmd.PersistentFlags().StringVarP(&tokenConfigPath, "config", "c", "", "Path to configuration file") + addConfigFlag(tokenCmd.PersistentFlags(), &tokenConfigPath, "Path to configuration file") // Subcommands tokenCmd.AddCommand(newTokenCreateCmd()) diff --git a/cmd/mcpproxy/tools_cmd.go b/cmd/mcpproxy/tools_cmd.go index 7e643a34a..30f6eb70f 100644 --- a/cmd/mcpproxy/tools_cmd.go +++ b/cmd/mcpproxy/tools_cmd.go @@ -7,7 +7,6 @@ import ( "fmt" "net/url" "os" - "path/filepath" "regexp" "strings" "time" @@ -321,7 +320,7 @@ func initToolsFlags() { toolsListCmd.Flags().StringVarP(&serverName, "server", "s", "", "Name of the upstream server to query (optional; omit for global list)") toolsListCmd.Flags().StringVarP(&toolsLogLevel, "log-level", "l", "info", "Log level (trace, debug, info, warn, error)") - toolsListCmd.Flags().StringVarP(&configPath, "config", "c", "", "Path to MCP configuration file (default: ~/.mcpproxy/mcp_config.json)") + addConfigFlag(toolsListCmd.Flags(), &configPath, "Path to MCP configuration file (default: ~/.mcpproxy/mcp_config.json)") toolsListCmd.Flags().DurationVarP(&timeout, "timeout", "t", 30*time.Second, "Connection timeout") toolsListCmd.Flags().BoolVar(&traceTransport, "trace-transport", false, "Enable detailed HTTP/SSE frame-by-frame tracing") @@ -776,17 +775,12 @@ func runToolsSetEnabled(args []string, enabled bool) error { // loadToolsConfig loads the MCP configuration file for tools command func loadToolsConfig() (*config.Config, error) { - var configFilePath string - - if configPath != "" { - configFilePath = configPath - } else { - // Use default path - homeDir, err := os.UserHomeDir() - if err != nil { - return nil, fmt.Errorf("failed to get user home directory: %w", err) + configFilePath := resolveCLIConfigPath(configPath) + if configFilePath == "" { + var err error + if configFilePath, err = defaultHomeConfigPath(); err != nil { + return nil, err } - configFilePath = filepath.Join(homeDir, ".mcpproxy", "mcp_config.json") } // Check if config file exists diff --git a/cmd/mcpproxy/upstream_cmd.go b/cmd/mcpproxy/upstream_cmd.go index 114927069..deef7fb94 100644 --- a/cmd/mcpproxy/upstream_cmd.go +++ b/cmd/mcpproxy/upstream_cmd.go @@ -340,7 +340,7 @@ func init() { // Define flags (note: output format handled by global --output/-o flag from root command) upstreamListCmd.Flags().StringVar(&upstreamListProfile, "profile", "", "Show only the servers of this profile's effective server set; TOOLS is the number of tools visible under it (Spec 108)") upstreamListCmd.Flags().StringVarP(&upstreamLogLevel, "log-level", "l", "warn", "Log level (trace, debug, info, warn, error)") - upstreamListCmd.Flags().StringVarP(&upstreamConfigPath, "config", "c", "", "Path to MCP configuration file") + addConfigFlag(upstreamListCmd.Flags(), &upstreamConfigPath, "Path to MCP configuration file") upstreamListCmd.Flags().StringArrayVar(&upstreamListStatus, "status", nil, "Filter by health status (repeatable; a comma-separated value is equivalent to repeating the flag): "+ "ready, connecting, sign_in_required, needs_review, needs_secret, needs_config, error, disabled") @@ -348,7 +348,7 @@ func init() { upstreamLogsCmd.Flags().IntVarP(&upstreamLogsTail, "tail", "n", 50, "Number of log lines to show") upstreamLogsCmd.Flags().BoolVarP(&upstreamLogsFollow, "follow", "f", false, "Follow log output (requires daemon)") upstreamLogsCmd.Flags().StringVarP(&upstreamLogLevel, "log-level", "l", "warn", "Log level") - upstreamLogsCmd.Flags().StringVarP(&upstreamConfigPath, "config", "c", "", "Path to config file") + addConfigFlag(upstreamLogsCmd.Flags(), &upstreamConfigPath, "Path to config file") upstreamLogsCmd.Flags().StringVarP(&upstreamServerName, "server", "s", "", "Name of the upstream server") // Add --all and --force flags to enable/disable/restart @@ -940,11 +940,8 @@ func outputError(err error, code string) error { // subcommand. Config-mode mutations must use this same path as loading so an // explicit root --config file is never redirected to DataDir/mcp_config.json. func upstreamConfigFilePath(globalConfig *config.Config) string { - if upstreamConfigPath != "" { - return upstreamConfigPath - } - if configFile != "" { - return configFile + if p := resolveCLIConfigPath(upstreamConfigPath); p != "" { + return p } if globalConfig != nil { return config.GetConfigPath(globalConfig.DataDir) diff --git a/docs/cli-management-commands.md b/docs/cli-management-commands.md index 78fe093ac..87fbe96a0 100644 --- a/docs/cli-management-commands.md +++ b/docs/cli-management-commands.md @@ -11,6 +11,15 @@ MCPProxy provides two command groups: All commands support both **daemon mode** (fast, via socket) and **standalone mode** (direct connection). +## Global flags + +`-c/--config` and `-d/--data-dir` are accepted before or after the command name +and apply to every management command. An empty value (`-c ""`, `--config=`) is an +error, never "use the default". A command's own `--config` wins over the global +one. With only `-d DIR`, management commands read `DIR/mcp_config.json` when it +exists; they never create a default configuration under `~/.mcpproxy` when a +path was given. + ## Command Reference ### `mcpproxy upstream list` @@ -405,7 +414,7 @@ mcpproxy doctor [flags] ``` **Flags:** -- `--output, -o` - Output format (pretty, json) [default: pretty] +- `--output, -o` - Output format (pretty, json, yaml) [default: pretty]; the global `--json` is shorthand for `-o json` - `--log-level, -l` - Log level [default: warn] - `--config, -c` - Path to config file - `--server` - Limit health checks to a single upstream server by name (Spec 044) @@ -425,6 +434,12 @@ mcpproxy doctor --output=json mcpproxy doctor --server=github ``` +The report is made to be shared, so credentials are always redacted in every +format: query parameters such as `?apikey=` or `?token=` in any URL (including +`web_ui_url`) print as `REDACTED`, and so does the admin API key wherever it +would appear. Use `mcpproxy status --show-key` or `mcpproxy status --web-url` +when you need the key itself. + **Health Checks:** - Upstream server connection errors - OAuth authentication requirements diff --git a/docs/features/profiles.md b/docs/features/profiles.md index f65f2df96..8f8dacf2d 100644 --- a/docs/features/profiles.md +++ b/docs/features/profiles.md @@ -115,7 +115,7 @@ Activity records, session rows and the Clients page show the profile together wi `mcpproxy connect` (and the Connect screens) never write the instance admin API key into a client's config. They mint a per-client credential (`mcp_cli_...`), bound to a profile, valid on MCP endpoints only. Reassigning a client to another profile takes effect on its next request without touching its config file, and its live session is told its tool list changed. Reassign from the Web UI Clients page, the macOS Clients view or tray submenu, `mcpproxy client set-profile`, or the `profiles` MCP tool. Details are in [Connect clients](./connect-clients.md). -A **locked** client cannot switch: `set_profile` and `/mcp/p/` get the same refusal as an unknown profile. A **switchable** client may switch to the profiles in its bound profile's `switchable_to` and nowhere else. +A **locked** client cannot switch: `set_profile` answers `cannot switch to profile '': this client's profile is locked`, and `/mcp/p/` gets the same refusal as an unknown profile. A **switchable** client may switch to the profiles in its bound profile's `switchable_to` and nowhere else. ### Callers without a credential @@ -183,6 +183,7 @@ The `set_profile` MCP tool switches the active profile **inside a live session** - Passing an empty string (`""`) clears the selection and returns to all servers. `active_profile` always reports the **stored session selection** — `""` after a clear, even for a token with a [`profile_pin`](./agent-tokens.md#profile-pinning) — while `servers` reports the **effective scope** the session can actually reach after the update: the pin's servers for a pinned token (nothing once the pinned profile has been deleted), the URL profile on a `/mcp/p/` endpoint, otherwise the selection or every configured server. - The `servers` list is always bounded by the caller's credential, using the same rule that scopes `retrieve_tools`: for an [agent token](./agent-tokens.md) scoped to specific servers it is the intersection of the effective profile (resolved pin > URL > session, see [Which profile applies](#which-profile-applies)) with the token's `allowed_servers`, so a token restricted to one server is never told about the others. On a `/mcp/p/` endpoint the URL still governs the request, so `set_profile("other")` there stores `other` as `active_profile` but reports ` ∩ allowed_servers` in `servers`. API-key and socket callers see the full lists. - An unknown slug is rejected. An administrator (API key, socket, anonymous back-compat) gets the discovery affordance: `unknown profile '' (available: research, deploy)`. An agent token gets `unknown profile ''` with no list at all: it may select only the profiles overlapping its `allowed_servers` (or its pin while the pin still has reach), and a profile entirely outside its reach (an empty profile, a profile whose servers are all outside `allowed_servers`, or the token's own pin once it no longer exists or no longer overlaps the token's servers) is rejected with that same error rather than confirmed as existing. A pinned token asking for any profile OTHER than its pin (see [profile pinning](./agent-tokens.md#profile-pinning)) is rejected with that same `unknown profile ''` error too — never a distinct "pinned to..." message, which would let the token confirm from the wording alone that it is pinned, and to what, from a refusal aimed at a different slug. The check looks only at the requested slug (and the token's pin) and tests the token's own `allowed_servers` against that profile's precomputed server set — its cost does not depend on how many other profiles are configured, on how many servers the requested profile declares or on how many servers are configured at all, only on the size of the token's own grant — so a token cannot learn which profiles or servers exist, or whether it is pinned, from `set_profile`, by body or by timing. +- A [client credential](#client-credentials-and-bindings) (`mcp_cli_...`) is told why a switch failed, by its own binding mode alone: a locked client gets `cannot switch to profile '': this client's profile is locked`, a switchable client `cannot switch to profile '': it is not a profile this client may switch to`. The text never depends on whether the slug exists or is in `switchable_to` (every refused slug gets the same text with the slug substituted) and never names the bound profile. Setting `profile` to the client's own bound profile is still admitted. - Session state is cleared automatically on session close. `set_profile` is available on the default `/mcp` server and the `call_tool` / `code_execution` routing-mode servers. diff --git a/internal/profile/set_profile_refusals.go b/internal/profile/set_profile_refusals.go new file mode 100644 index 000000000..717b650d0 --- /dev/null +++ b/internal/profile/set_profile_refusals.go @@ -0,0 +1,18 @@ +package profile + +// The MCP set_profile refusal texts a Spec 108 client credential receives +// (contracts/refusals.md; golden testdata/contract/set_profile_refusals.json). +// +// They depend only on the caller's own credential (its kind and profile mode), +// never on whether the requested slug exists or is in the binding's +// switchable_to, so for one caller every refused slug returns byte-identical +// text with the slug substituted (the uniform-refusal property). Neither names +// the bound profile. Every other caller (agent tokens, anonymous, confined +// anonymous, URL-scoped) keeps the Spec 105 "unknown profile ''" text. +const ( + // SetProfileLockedRefusalFormat is the refusal for a locked client credential. + SetProfileLockedRefusalFormat = "cannot switch to profile '%s': this client's profile is locked" + // SetProfileNotSwitchableRefusalFormat is the refusal for a switchable + // client credential asking for a profile it may not switch to. + SetProfileNotSwitchableRefusalFormat = "cannot switch to profile '%s': it is not a profile this client may switch to" +) diff --git a/internal/profile/set_profile_refusals_test.go b/internal/profile/set_profile_refusals_test.go new file mode 100644 index 000000000..2940f68c2 --- /dev/null +++ b/internal/profile/set_profile_refusals_test.go @@ -0,0 +1,19 @@ +package profile + +import ( + "encoding/json" + "fmt" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestSetProfileRefusalGolden(t *testing.T) { + var golden map[string]string + require.NoError(t, json.Unmarshal(readFixture(t, "set_profile_refusals.json"), &golden)) + + require.Equal(t, strings.ReplaceAll(golden["locked"], "

", "%s"), SetProfileLockedRefusalFormat) + require.Equal(t, strings.ReplaceAll(golden["switchable"], "

", "%s"), SetProfileNotSwitchableRefusalFormat) + require.Equal(t, "cannot switch to profile 'x': this client's profile is locked", fmt.Sprintf(SetProfileLockedRefusalFormat, "x")) +} diff --git a/internal/profile/testdata/contract/set_profile_refusals.json b/internal/profile/testdata/contract/set_profile_refusals.json new file mode 100644 index 000000000..c38c3a621 --- /dev/null +++ b/internal/profile/testdata/contract/set_profile_refusals.json @@ -0,0 +1,7 @@ +{ + "_comment": "Spec 108 (fix-usertest-cli, F-11, contracts/refusals.md): the MCP set_profile refusal texts by caller class, byte-for-byte after substituting

(the requested slug) and (the administrator's available profile names). Client credentials get the first two (per-caller uniform: the text never depends on whether the slug exists or is in switchable_to, and never names the bound profile); every other caller keeps the Spec 105 text. TestSetProfileRefusalGolden (internal/profile) pins the constants to this file and TestSetProfileV3_ClientRefusalTextMatchesGolden (internal/server) pins the handler.", + "locked": "cannot switch to profile '

': this client's profile is locked", + "switchable": "cannot switch to profile '

': it is not a profile this client may switch to", + "scoped_unknown": "unknown profile '

'", + "admin_unknown": "unknown profile '

' (available: )" +} diff --git a/internal/registries/httpclient.go b/internal/registries/httpclient.go index a87b42843..104e88c08 100644 --- a/internal/registries/httpclient.go +++ b/internal/registries/httpclient.go @@ -10,6 +10,7 @@ import ( "net/url" "strings" "sync" + "sync/atomic" "time" ) @@ -36,12 +37,18 @@ const ( // registryRetryBaseDelay is the first backoff; each subsequent retry doubles it // (500ms, then 1s). A var (not const) so tests can shrink it. -var registryRetryBaseDelay = 500 * time.Millisecond +var registryRetryBaseDelay atomic.Int64 // nanoseconds; atomic because a leaked warm-behind fetch may still read it // registryMaxBodyBytes caps how much of a registry response we buffer in memory, // bounding a large or hostile body (a real official page of 100 servers is a few -// hundred KB, so 16 MiB is generous). A var so tests can shrink it. -var registryMaxBodyBytes int64 = 16 << 20 +// hundred KB, so 16 MiB is generous). Atomic so tests can shrink it while a +// background (warm-behind) fetch goroutine from an earlier test is still reading. +var registryMaxBodyBytes atomic.Int64 + +func init() { + registryMaxBodyBytes.Store(16 << 20) + registryRetryBaseDelay.Store(int64(500 * time.Millisecond)) +} var ( registryHTTPClientOnce sync.Once @@ -143,7 +150,7 @@ func registryGet(ctx context.Context, reg *RegistryEntry, reqURL string) ([]byte if attempt > 1 { // Back off before retrying, but bail out immediately if the parent // context is already done. - delay := registryRetryBaseDelay * time.Duration(1<<(attempt-2)) + delay := time.Duration(registryRetryBaseDelay.Load()) * time.Duration(1<<(attempt-2)) select { case <-ctx.Done(): return nil, ctx.Err() @@ -180,7 +187,7 @@ func registryGet(ctx context.Context, reg *RegistryEntry, reqURL string) ([]byte // Cap the buffered body so a large/hostile response can't OOM us. Read // one byte past the cap to detect an over-limit body. - body, readErr := io.ReadAll(io.LimitReader(resp.Body, registryMaxBodyBytes+1)) + body, readErr := io.ReadAll(io.LimitReader(resp.Body, registryMaxBodyBytes.Load()+1)) resp.Body.Close() if readErr != nil { if ctx.Err() != nil { @@ -189,9 +196,9 @@ func registryGet(ctx context.Context, reg *RegistryEntry, reqURL string) ([]byte lastErr = readErr continue } - if int64(len(body)) > registryMaxBodyBytes { + if int64(len(body)) > registryMaxBodyBytes.Load() { // Not transient — a retry would hit the same oversized body. - return nil, fmt.Errorf("registry response exceeds %d bytes", registryMaxBodyBytes) + return nil, fmt.Errorf("registry response exceeds %d bytes", registryMaxBodyBytes.Load()) } // Retry server-side failures while attempts remain. diff --git a/internal/registries/httpclient_test.go b/internal/registries/httpclient_test.go index 01e6f390c..46c4b4a22 100644 --- a/internal/registries/httpclient_test.go +++ b/internal/registries/httpclient_test.go @@ -14,9 +14,9 @@ import ( // restores the production delay afterwards. func withFastRetries(t *testing.T) { t.Helper() - prev := registryRetryBaseDelay - registryRetryBaseDelay = time.Millisecond - t.Cleanup(func() { registryRetryBaseDelay = prev }) + prev := registryRetryBaseDelay.Load() + registryRetryBaseDelay.Store(int64(time.Millisecond)) + t.Cleanup(func() { registryRetryBaseDelay.Store(prev) }) } // swapRegistryClient overrides the shared registry HTTP client for a test and @@ -156,9 +156,9 @@ func TestRegistryGet_ParentContextStopsRetry(t *testing.T) { // large/hostile registry response fails fast instead of allocating unbounded. func TestRegistryGet_RejectsOversizedBody(t *testing.T) { withFastRetries(t) - prev := registryMaxBodyBytes - registryMaxBodyBytes = 16 - t.Cleanup(func() { registryMaxBodyBytes = prev }) + prev := registryMaxBodyBytes.Load() + registryMaxBodyBytes.Store(16) + t.Cleanup(func() { registryMaxBodyBytes.Store(prev) }) srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.Header().Set("Content-Type", "application/json") diff --git a/internal/server/profile_tool.go b/internal/server/profile_tool.go index 41da8647e..f3834b0b8 100644 --- a/internal/server/profile_tool.go +++ b/internal/server/profile_tool.go @@ -65,6 +65,27 @@ func buildSetProfileTool() mcp.Tool { // the pre-105 payload byte-for-byte — the selected profile's servers, or every // configured server on clear — because SC-005 names no FR-003 exception for // them (an administrator is never pinned, so only the URL tier could differ). +// setProfileRefusal is the refusal for a slug the caller may not select. A +// Spec 108 client credential gets text chosen by its own profile mode alone +// (locked: "this client's profile is locked"; switchable: "not a profile this +// client may switch to"), so it can tell why the switch failed without learning +// anything about the slug: the text never depends on whether the slug exists or +// is in switchable_to, is byte-identical for every refused slug with the slug +// substituted, and never names the bound profile (FR-018; the admission lookup +// and its timing class are untouched). Every other scoped caller keeps the +// Spec 105 uniform "unknown profile ''" — an agent token must never be +// able to confirm from the wording that it is pinned (round 9 MUST-FIX 2). +func setProfileRefusal(ctx context.Context, slug string) *mcp.CallToolResult { + if ac := auth.AuthContextFromContext(ctx); ac.IsClientCredential() { + format := profile.SetProfileNotSwitchableRefusalFormat + if ac.ProfileMode == auth.ProfileModeLocked { + format = profile.SetProfileLockedRefusalFormat + } + return mcp.NewToolResultError(fmt.Sprintf(format, slug)) + } + return mcp.NewToolResultError(fmt.Sprintf("unknown profile '%s'", slug)) +} + func (p *MCPProxyServer) handleSetProfile(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { slug := strings.TrimSpace(request.GetString("profile", "")) @@ -88,7 +109,7 @@ func (p *MCPProxyServer) handleSetProfile(ctx context.Context, request mcp.CallT // to a fresh build instead. Fail closed with the exact uniform // refusal every other non-selectable slug gets: no state change, no // disclosure, no build (Spec 105 PR D review round 11, MUST-FIX). - return mcp.NewToolResultError(fmt.Sprintf("unknown profile '%s'", slug)), nil + return setProfileRefusal(ctx, slug), nil } cfg := profiles.cfg anonymousBindingGuard := anonymousProfileCaller(ctx) && p.bindingGuardActive(profiles) @@ -127,7 +148,7 @@ func (p *MCPProxyServer) handleSetProfile(ctx context.Context, request mcp.CallT if slug != "" { if !profiles.selectable(ctx, slug) { if auth.IsScopedCaller(ctx) || anonymousProfileConfined { - return mcp.NewToolResultError(fmt.Sprintf("unknown profile '%s'", slug)), nil + return setProfileRefusal(ctx, slug), nil } return mcp.NewToolResultError(fmt.Sprintf("unknown profile '%s' (available: %s)", slug, strings.Join(profiles.selectableNames(ctx), ", "))), nil } diff --git a/internal/server/profiles_v3_acceptance_test.go b/internal/server/profiles_v3_acceptance_test.go index e700e5b86..846373974 100644 --- a/internal/server/profiles_v3_acceptance_test.go +++ b/internal/server/profiles_v3_acceptance_test.go @@ -290,7 +290,7 @@ func TestProfilesV3Acceptance_Check5_LockedCannotSwitchAndManagementHidden(t *te res, err := setProfile(ctx, "work-full") require.NoError(t, err) require.True(t, res.IsError) - require.Equal(t, "unknown profile 'work-full'", resultText(t, res)) + require.Equal(t, expectedSetProfileRefusal(t, ctx, "work-full"), resultText(t, res)) require.Empty(t, proxy.sessionStore.GetActiveProfile("acc5-locked")) require.Equal(t, "work-readonly", proxy.ResolveProfileV3(ctx, idx).Name) }) @@ -322,10 +322,11 @@ func TestProfilesV3Acceptance_Check5_LockedCannotSwitchAndManagementHidden(t *te require.False(t, ok.IsError, resultText(t, ok)) require.Equal(t, "work-full", proxy.sessionStore.GetActiveProfile("acc5-switchable")) - refused, err := setProfile(sessionCtx(clientCtx("codex", "work-readonly", auth.ProfileModeSwitchable), "acc5-switchable-2"), "admin-all") + switchableCtx := sessionCtx(clientCtx("codex", "work-readonly", auth.ProfileModeSwitchable), "acc5-switchable-2") + refused, err := setProfile(switchableCtx, "admin-all") require.NoError(t, err) require.True(t, refused.IsError) - require.Equal(t, "unknown profile 'admin-all'", resultText(t, refused), "an undeclared target gets the uniform refusal") + require.Equal(t, expectedSetProfileRefusal(t, switchableCtx, "admin-all"), resultText(t, refused), "an undeclared target gets the per-caller uniform refusal") }) t.Run("the profiles management tool is for an administrator session only", func(t *testing.T) { diff --git a/internal/server/set_profile_refusal_v3_test.go b/internal/server/set_profile_refusal_v3_test.go new file mode 100644 index 000000000..c60c71e5f --- /dev/null +++ b/internal/server/set_profile_refusal_v3_test.go @@ -0,0 +1,113 @@ +package server + +import ( + "context" + "encoding/json" + "os" + "strings" + "testing" + + "github.com/mark3labs/mcp-go/mcp" + "github.com/stretchr/testify/require" + + "github.com/smart-mcp-proxy/mcpproxy-go/internal/auth" +) + +// setProfileRefusalGolden reads the shared golden of the set_profile refusal +// texts (internal/profile/testdata/contract/set_profile_refusals.json). +func setProfileRefusalGolden(t *testing.T) map[string]string { + t.Helper() + raw, err := os.ReadFile("../profile/testdata/contract/set_profile_refusals.json") + require.NoError(t, err) + var golden map[string]string + require.NoError(t, json.Unmarshal(raw, &golden)) + return golden +} + +// expectedSetProfileRefusal is the golden refusal text a caller gets for a +// refused slug: client credentials by mode, everyone else the Spec 105 text. +func expectedSetProfileRefusal(t *testing.T, ctx context.Context, slug string) string { + t.Helper() + golden := setProfileRefusalGolden(t) + key := "scoped_unknown" + if ac := auth.AuthContextFromContext(ctx); ac.IsClientCredential() { + if ac.ProfileMode == auth.ProfileModeLocked { + key = "locked" + } else { + key = "switchable" + } + } + return strings.ReplaceAll(golden[key], "

", slug) +} + +// F-11 (user test 2026-10-02): a locked client asking for a profile it cannot +// have was told "unknown profile 'work-full'" although the profile exists. The +// text now depends only on the caller's own credential, never on the slug. +func TestSetProfileV3_ClientRefusalTextMatchesGolden(t *testing.T) { + proxy, _ := newProfilesV3Fixture(t) + golden := setProfileRefusalGolden(t) + + call := func(ctx context.Context, slug string) *mcp.CallToolResult { + request := mcp.CallToolRequest{} + request.Params.Arguments = map[string]interface{}{"profile": slug} + result, err := proxy.handleSetProfile(ctx, request) + require.NoError(t, err) + return result + } + + t.Run("locked client: existing and missing slugs get the same locked text", func(t *testing.T) { + for i, slug := range []string{"work-full", "legacy", "nope"} { + sid := "locked-golden-" + string(rune('a'+i)) + ctx := sessionCtx(clientCtx("cursor", "work-readonly", auth.ProfileModeLocked), sid) + result := call(ctx, slug) + require.True(t, result.IsError) + text := resultText(t, result) + require.Equal(t, strings.ReplaceAll(golden["locked"], "

", slug), text) + require.NotContains(t, text, "work-readonly", "a refusal never names the bound profile") + require.NotContains(t, text, "available") + require.Empty(t, proxy.sessionStore.GetActiveProfile(sid)) + } + }) + + t.Run("switchable client: non-declared and missing slugs get the switchable text", func(t *testing.T) { + for i, slug := range []string{"legacy", "nope"} { + sid := "switchable-golden-" + string(rune('a'+i)) + ctx := sessionCtx(clientCtx("laptop", "work-readonly", auth.ProfileModeSwitchable), sid) + result := call(ctx, slug) + require.True(t, result.IsError) + text := resultText(t, result) + require.Equal(t, strings.ReplaceAll(golden["switchable"], "

", slug), text) + require.NotContains(t, text, "work-readonly") + require.Empty(t, proxy.sessionStore.GetActiveProfile(sid)) + } + }) + + t.Run("switchable client may still switch to its declared target", func(t *testing.T) { + ctx := sessionCtx(clientCtx("laptop", "work-readonly", auth.ProfileModeSwitchable), "switchable-ok") + require.False(t, call(ctx, "work-full").IsError) + require.Equal(t, "work-full", proxy.sessionStore.GetActiveProfile("switchable-ok")) + }) + + t.Run("locked client may still select its own base and clear", func(t *testing.T) { + ctx := sessionCtx(clientCtx("cursor", "work-readonly", auth.ProfileModeLocked), "locked-own") + require.False(t, call(ctx, "work-readonly").IsError) + require.False(t, call(ctx, "").IsError) + }) + + t.Run("unpinned agent token keeps the Spec 105 text", func(t *testing.T) { + ctx := setProfileScopedCtx("agent-golden", "github") + result := call(ctx, "nope") + require.True(t, result.IsError) + require.Equal(t, strings.ReplaceAll(golden["scoped_unknown"], "

", "nope"), resultText(t, result)) + }) + + t.Run("administrator keeps the available list", func(t *testing.T) { + result := call(setProfileAdminCtx("admin-golden"), "nope") + require.True(t, result.IsError) + text := resultText(t, result) + prefix := strings.ReplaceAll(strings.SplitN(golden["admin_unknown"], "", 2)[0], "

", "nope") + require.True(t, strings.HasPrefix(text, prefix), text) + require.Contains(t, text, "work-full") + require.Contains(t, text, "work-readonly") + }) +} diff --git a/internal/server/set_profile_v3_test.go b/internal/server/set_profile_v3_test.go index 179b73ece..59dc9c66d 100644 --- a/internal/server/set_profile_v3_test.go +++ b/internal/server/set_profile_v3_test.go @@ -71,7 +71,8 @@ func TestSetProfileV3SwitchableClientCanSelectDeclaredTarget(t *testing.T) { result, err := proxy.handleSetProfile(ctx, request) require.NoError(t, err) require.True(t, result.IsError) - require.Equal(t, "unknown profile 'work-full'", resultText(t, result)) + require.Equal(t, expectedSetProfileRefusal(t, ctx, "work-full"), resultText(t, result)) + require.Equal(t, "cannot switch to profile 'work-full': this client's profile is locked", resultText(t, result)) require.Empty(t, proxy.sessionStore.GetActiveProfile("locked-set-profile")) }) @@ -327,7 +328,7 @@ func TestSetProfileV3_ManagementAndSwitchingMatrix(t *testing.T) { require.True(t, result.IsError, "%s: set_profile(%q) must be refused", row.name, slug) text := resultText(t, result) require.Equal(t, "", proxy.sessionStore.GetActiveProfile(sessionIDFromContext(ctx)), "a refusal leaves the session unchanged") - require.Equal(t, fmt.Sprintf("unknown profile '%s'", slug), text) + require.Equal(t, expectedSetProfileRefusal(t, ctx, slug), text) if row.base != "" && row.base != slug { require.NotContains(t, text, row.base, "a refusal must not name the caller's base profile (FR-018)") } @@ -501,9 +502,10 @@ func TestSetProfileV3_ManagementAndSwitchingMatrix(t *testing.T) { other := mcp.CallToolRequest{} other.Params.Arguments = map[string]interface{}{"profile": "legacy"} - refused, err := proxy.handleSetProfile(sessionCtx(clientCtx("laptop", "work-readonly", "locked"), sid), other) + lockedCtx := sessionCtx(clientCtx("laptop", "work-readonly", "locked"), sid) + refused, err := proxy.handleSetProfile(lockedCtx, other) require.NoError(t, err) require.True(t, refused.IsError) - require.Equal(t, "unknown profile 'legacy'", resultText(t, refused)) + require.Equal(t, expectedSetProfileRefusal(t, lockedCtx, "legacy"), resultText(t, refused)) }) } diff --git a/specs/108-profiles-v3/contracts/mcp-tools.md b/specs/108-profiles-v3/contracts/mcp-tools.md index fff01db58..f5bcbb504 100644 --- a/specs/108-profiles-v3/contracts/mcp-tools.md +++ b/specs/108-profiles-v3/contracts/mcp-tools.md @@ -26,7 +26,7 @@ Absent from `tools/list` and refused at call (`tool not found` uniform shape) wh ## `set_profile` (semantics change only) -Admission per FR-022; every refusal uses the Spec 105 `profileNotSelectable` uniform body. Response unchanged: `{active_profile, servers}` plus `profile_source` (new field, additive), which reports only the caller's own selection — `session` with the selected slug, or `none`/empty `active_profile` after `set_profile("")` even when the next request falls through to a pin, binding or anonymous base (FR-018). +Admission per FR-022; every refusal is non-disclosing: a client credential (`mcp_cli_`) gets the per-caller uniform text of `contracts/refusals.md` (locked: `cannot switch to profile '

': this client's profile is locked`; switchable: `cannot switch to profile '

': it is not a profile this client may switch to`; the text depends only on the credential's own mode, never on the slug, and never names the bound profile), every other caller keeps the Spec 105 uniform `unknown profile '

'` (administrators: with the `available:` list). `/mcp/p/` keeps the Spec 105 `profileNotSelectable` body. Response unchanged: `{active_profile, servers}` plus `profile_source` (new field, additive), which reports only the caller's own selection — `session` with the selected slug, or `none`/empty `active_profile` after `set_profile("")` even when the next request falls through to a pin, binding or anonymous base (FR-018). ## `profiles` (new, admin only — FR-017, research D12/D14) diff --git a/specs/108-profiles-v3/contracts/refusals.md b/specs/108-profiles-v3/contracts/refusals.md index 009bc37b3..35acd1663 100644 --- a/specs/108-profiles-v3/contracts/refusals.md +++ b/specs/108-profiles-v3/contracts/refusals.md @@ -13,7 +13,8 @@ Texts are exact (tests compare byte-for-byte after substituting ``, `` formatter, differing only in the echoed name and per-request identifiers | `status=blocked`, `block_reason=profile_code_execution` | | `code_execution` disabled by profile, via the dedicated route | REST `POST /api/v1/code/exec` (FR-014) | `403 {"ok":false,"error":{"code":"PROFILE_BLOCKED","message":"blocked by profile: code execution is disabled for this profile"},"request_id":…}` (the route's existing `CodeExecResponse` error shape) — the route exists only to execute code, so admitting that a profile disabled it discloses nothing (the profile is not named); takes precedence over the global `FEATURE_DISABLED` answer | `status=blocked`, `block_reason=profile_code_execution` | | Management tool hidden by profile / client credential | `upstream_servers`, `quarantine_security`, `profiles` (MCP; `upstream_servers`/`quarantine_security` also via REST `POST /api/v1/tools/call`) | uniform unknown-tool shape (REST: the route's unknown-tool error) | `status=blocked`, `block_reason=profile_management`, recorded where the shared handler runs (REST `/tools/call`); an MCP `tools/call` of a filter-hidden built-in (`code_execution`, management tools) is answered by mcp-go's `WithToolFilter` before any handler and writes no row | -| `set_profile` / `/mcp/p/` not permitted (locked, not in `switchable_to`, not selectable, unknown) | `set_profile`, profile URL | Spec 105 `profileNotSelectable` uniform body/status | `policy_decision` record (existing) | +| `/mcp/p/` not permitted (locked, not in `switchable_to`, not selectable, unknown) | profile URL | Spec 105 `profileNotSelectable` uniform body/status (unchanged) | `policy_decision` record (existing) | +| `set_profile` not permitted (locked, not in `switchable_to`, not selectable, unknown) | `set_profile` | **Client credentials** (`mcp_cli_`) get a per-caller uniform text chosen by the credential's own mode alone: locked `cannot switch to profile '

': this client's profile is locked`, switchable `cannot switch to profile '

': it is not a profile this client may switch to`. The text never depends on whether the slug exists or is in `switchable_to` (every refused slug returns byte-identical text with the slug substituted) and never names the bound profile (FR-018, D27). Every other caller keeps the Spec 105 text: agent tokens and anonymous callers `unknown profile '

'`, administrators `unknown profile '

' (available: )`. Golden `internal/profile/testdata/contract/set_profile_refusals.json` | `policy_decision` record (existing) | | Mutating `upstream_servers` op by an agent token, client credential or **confined anonymous** caller (FR-016) | `upstream_servers` | existing Spec 028 `AuthorizeServerOp` refusal text | existing | | Binding op on a client without an active client credential | `PUT /clients/{id}/binding`, CLI `client set-profile|lock|unlock`, MCP `profiles assign` | `409 {"error":"client has no active client credential; connect it with a profile first","code":"no_client_credential"}` (MCP: same object as the tool error) | none (nothing changed) | | Another connect of the client is mid-write (FR-021a in-flight claim) | `POST /connect/{client}`, `POST /clients/{id}/rotate`, `POST /clients/{id}/rotate/finalize`, `PUT /clients/{id}/binding` (bulk-assign: per client in `skipped[]`), CLI `connect`, MCP `profiles assign` | `409 {"error":"a connect of is already in progress; retry when it finishes","code":"connect_in_progress"}` (MCP: same object as the tool error) | none (nothing changed) | diff --git a/specs/108-profiles-v3/spec.md b/specs/108-profiles-v3/spec.md index 2d6b9ae11..b7aca334c 100644 --- a/specs/108-profiles-v3/spec.md +++ b/specs/108-profiles-v3/spec.md @@ -100,7 +100,7 @@ A user connects Cursor with "Work Read-only", locked. Later they move Cursor to 1. **Given** `require_mcp_auth` on, or off with `anonymous_profile` set to a profile not wider than the binding (FR-008a — e.g. `work-readonly` with no `switchable_to`; otherwise scenario 7's refusal applies), **When** the user connects Cursor with profile `work-readonly` (any surface), **Then** a client credential `client-cursor` (kind `client`, pinned `work-readonly`, mode `locked`) is minted, written into Cursor's config via the client's supported carrier, and the instance admin API key is **not** written; the preview shows the credential masked and names the profile. 2. **Given** Cursor bound and connected, **When** `mcpproxy client set-profile cursor work-full` runs, **Then** Cursor's next request resolves to `work-full` with `profile_source=pin` (the command keeps Cursor's `locked` mode, and a locked client credential resolves at the pin tier, FR-020; a `switchable` one would report `binding`), no client config file changes (byte-identical), and Cursor's live session receives `notifications/tools/list_changed`. *(audit acceptance check 4)* 3. **Given** the same, **When** the reassignment is made from the Web UI Clients page, the macOS Clients view or tray submenu, or the MCP `profiles` tool (`operation: assign`), **Then** the outcome is identical and each writes one activity record `type=profile_change` naming actor, surface, client, previous and new profile. -4. **Given** Cursor bound `locked` to `work-readonly`, **When** its session calls `set_profile("work-full")` or initializes via `/mcp/p/work-full`, **Then** it receives the uniform non-disclosing refusal (Spec 105 shape). **Given** mode `switchable` and `work-readonly.switchable_to = ["work-full"]`, **Then** `set_profile("work-full")` succeeds and `set_profile("admin-all")` gets the uniform refusal. **Given** Cursor bound `locked` to `work-readonly` and its client config pointing at `/mcp/p/work-readonly` (or its session calling `set_profile("work-readonly")`), **Then** the request is admitted — naming the caller's own pin or bound profile is not a switch (FR-022), exactly as a pinned token may initialize `/mcp/p/` today. *(audit acceptance check 5, first half)* +4. **Given** Cursor bound `locked` to `work-readonly`, **When** its session calls `set_profile("work-full")` or initializes via `/mcp/p/work-full`, **Then** `set_profile("work-full")` receives the per-caller uniform refusal of contracts/refusals.md and initializing via `/mcp/p/work-full` the Spec 105 `unknown profile` refusal. **Given** mode `switchable` and `work-readonly.switchable_to = ["work-full"]`, **Then** `set_profile("work-full")` succeeds and `set_profile("admin-all")` gets the per-caller uniform refusal. **Given** Cursor bound `locked` to `work-readonly` and its client config pointing at `/mcp/p/work-readonly` (or its session calling `set_profile("work-readonly")`), **Then** the request is admitted — naming the caller's own pin or bound profile is not a switch (FR-022), exactly as a pinned token may initialize `/mcp/p/` today. *(audit acceptance check 5, first half)* 5. **Given** an admin credential session (API key or socket) with no effective profile, **When** it lists tools, **Then** the `profiles` tool is present and callable; **Given** a pinned or bound session, an anonymous session, or an admin session whose effective profile does not set `management_tools: true`, **Then** `profiles` is absent from `tools/list` and a call to it is refused. *(audit acceptance check 5, second half)* 6. **Given** a client whose credential is revoked or forgotten, **When** it next calls `/mcp`, **Then** it is rejected `401` and the Clients surfaces show it as "credential revoked — reconnect". 7. **Given** `require_mcp_auth` is off **and** `anonymous_profile` is unset or wider than the binding to profile P (the US5-4 condition — wider by servers, tier cap, unannotated handling, any admitted tool incl. deny/allow rules, `switchable_to` reachability or the code-execution/management capabilities, FR-008a — evaluated by the one shared function, data-model §7), **When** any surface tries to bind a client to P (`locked`, or `switchable` with a non-empty pin — connect, `client add`, binding change, bulk move, bulk admin-key upgrade with a profile, MCP `assign`) or makes an existing binding bypassable (a profile edit, classification, delete or `reassign_to` that shrinks what the binding reaches — including a member of P's `switchable_to` — or grows what anonymous callers reach), **Then** the operation is **refused** with `409 {code: "binding_bypassable_without_auth", fixes: [...]}` — "a client bound to a profile could escape it by omitting its credential while authentication is optional" — and nothing is minted, written or changed; the fixes offered are "turn on `require_mcp_auth`" (listing the unidentified clients that would lose access) and "set `anonymous_profile` to a profile not wider than the binding" (target P when P itself qualifies — for a `locked` binding only if P's `switchable_to` reaches nothing wider); after either fix the same request succeeds. With a non-wider `anonymous_profile` already set, the binding succeeds with no warning. The refusal text and codes are identical on REST, CLI, Web UI, macOS and MCP (FR-008a). @@ -219,7 +219,7 @@ A user who keeps `require_mcp_auth` off (for legacy clients) can still confine a - **FR-015a** (REST discovery, codex round 3): the REST routes that return tool names, descriptions or schemas to a non-administrator caller MUST apply the caller's own effective profile, so a pinned `kind=agent` token cannot read over REST a tool its profile hides from every MCP discovery surface (US1). At `638fa805a` these routes filter by the token's `allowed_servers` only (`canSeeServer`, `internal/httpapi/scope.go`) and ignore the pin entirely: `GET /api/v1/index/search` (`handleSearchTools`, `SearchToolsScoped` with a server-name predicate), `GET /api/v1/tools` without view-as parameters (`handleGetGlobalTools`), `GET /api/v1/servers/{id}/tools` (`handleGetServerTools`), `GET /api/v1/servers/{id}/tools/export` and `GET /api/v1/servers/{id}/tools/{tool}/diff`. From 108-d (the PR that lifts the FR-009a gate), for a caller whose resolution carries a profile (on REST: a pinned `kind=agent` token), each of them evaluates the FR-010 decision with the same predicate `retrieve_tools` uses: `GET /index/search` goes through `SearchToolsAdmitted` (filter before limit; the REST response gains no `hidden_by_profile` field — its shape is unchanged, excluded tools are simply absent); `GET /tools`, `GET /servers/{id}/tools` and the export omit excluded rows (exactly the non-admin `GET /tools?profile=` visible-row set, FR-032, without its `counts`); the diff route answers an excluded tool with the route's existing unknown-tool `404`. A server outside the pin's `servers` is treated like a server outside `allowed_servers` (its tools are absent; the `/servers/{id}/*` subtree answers the existing scoped `404`). Administrators (no profile) and unpinned agent tokens are unchanged. Server-level REST reads (`GET /servers`, per-server status, logs, diagnostics) keep today's Spec 105 token scoping and are not narrowed by the pin in this spec — they name servers, not tools, and the pin's server list is already disclosed to the operator who minted the token. - **FR-016**: When a session has an effective profile, `management_tools` on that profile decides the profile-level visibility of the management tools `upstream_servers` and `quarantine_security`: `false` → absent from `tools/list` and refused at call; `true` → visible, subject to the credential's own rules below; **unset** (every legacy profile, and any v3 profile that leaves the field unset — per-field inheritance, research D1) → pre-108 behaviour, with the two exceptions below (confined anonymous callers, client credentials). The `profiles` tool has no legacy behaviour and follows FR-017 only. `management_tools: true` never widens a credential beyond its existing rules: `quarantine_security` stays administrator-only, `profiles` follows FR-017, and **every agent token — including a client credential — keeps the Spec 028 `AuthorizeServerOp` policy unchanged**: `upstream_servers` is limited to the non-mutating operations `list` and `tail_log`, on servers inside its effective scope; every config-mutating or process-controlling operation (`add`, `add_from_registry`, `remove`, `update`, `patch`, `enable`, `disable`, `restart`, `refresh`) is refused. (Rationale: `patch` can replace a stdio server's `command`/`args` and `restart` re-launches it before any tool-level quarantine check — granting them to an AI-client credential would be arbitrary host code execution inside the credential's "scope"; server management stays an operator action on the Web UI, macOS app, CLI or an administrator MCP session — research D14.) An anonymous caller confined by `anonymous_profile` (source `anonymous`, or a URL/session selection admitted under that confinement) is treated like a v3 profile regardless of its target's `management_tools` being unset: management tools are hidden unless the effective profile sets `management_tools: true` (the `anonymous_profile` knob is new, so no legacy population exists); anonymous callers never get `quarantine_security` or `profiles` when confined. When the confining profile does set `management_tools: true`, a confined anonymous caller gets **exactly the client-credential op set** — `upstream_servers` `list` and `tail_log` on servers inside its effective scope, every mutating or process-controlling operation refused — because enforcement evaluates `AuthorizeServerOp` for it as a **non-administrator**: today's back-compat `auth.AnonymousContext()` is `AuthTypeAdmin` (`internal/auth/context.go`), which `AuthorizeServerOp` exempts from the entire `agentDeniedServerOps` denylist (`internal/auth/server_ops.go`), so without this rule any uncredentialed local process could `patch` a stdio server's command and `restart` it (research D14). The non-administrator view is not limited to the `AuthorizeServerOp` call: **every** `IsAdmin()` branch that shapes what a scoped caller sees, in a handler a confined anonymous caller reaches, MUST read the same view (`auth.ScopedView(ac, resolution)`, data-model §3) — at `638fa805a` these are the `upstream_servers` `list` server filter (`internal/server/mcp.go` ~4255), `handleTailLog`'s choice between the whole-file and the attributed log reader (~6283) and its container-mention redaction of `last_error` (~6323), and the scoped refusal-shape branch of `handleCallToolVariant` (~2587, US5-2) — so a confined anonymous caller gets the attributed, redacted `tail_log` output and the scoped list and refusal shapes of a client credential, never the administrator branch (research D29). An **unconfined** anonymous caller (`anonymous_profile` unset) keeps today's admin-typed behaviour unchanged. A client credential without a `management_tools: true` profile (including the default "All servers" binding and profiles with the field unset) sees no management tools; with one, it sees `upstream_servers` limited as above — an intended change from keyless clients, which ran as anonymous administrators (research D5). Global `disable_management`/`read_only_mode` remain ceilings. - **FR-017**: The `profiles` tool MUST be listed only for administrator credentials of kind `api_key` or `socket` (never an anonymous caller, including an unconfined one, and never agent or client tokens) whose effective profile is none or sets `management_tools: true`, and MUST be blocked by `read_only_mode` and `disable_management` for mutating operations only; the two global gates are read per call from the live configuration. Because `buildManagementTools()` returns no tools at all under `read_only_mode` or `disable_management` (`internal/server/mcp.go`), `profiles` MUST NOT be registered through it: it is registered unconditionally on the retrieve, call-tool and code-exec servers (visibility per session by `WithToolFilter`), and the two global gates are checked per operation inside its handler, so its read operations stay available when either gate is on. -- **FR-018**: `set_profile` MUST honour FR-022 and return the Spec 105 uniform refusal for every non-permitted target (locked, not in `switchable_to`, not selectable, unknown); naming the caller's own base is admitted (FR-022). The response's new `profile_source` field (and its `active_profile`) report only what the caller selected: `session` with the selected slug, or `none`/empty after `set_profile("")`, even when the next request falls through to a pin, binding or `anonymous_profile` base — a base is never named back to a non-administrator caller (research D27). `set_profile("")` is never a switch and is admitted for **every** caller, as today (`profile_tool.go`: an empty slug clears the selection and is always accepted): it clears the stored selection and the next resolution falls through to the caller's base — the pin, the binding (a switchable client returns to its bound profile, `profile_source=binding`), `anonymous_profile`, or none; it is never answered with the uniform refusal. +- **FR-018**: `set_profile` MUST honour FR-022 and return the per-caller uniform refusal of contracts/refusals.md for every non-permitted target (locked, not in `switchable_to`, not selectable, unknown; a client credential is told whether its binding is locked or switchable, every other caller gets the Spec 105 text); naming the caller's own base is admitted (FR-022). The response's new `profile_source` field (and its `active_profile`) report only what the caller selected: `session` with the selected slug, or `none`/empty after `set_profile("")`, even when the next request falls through to a pin, binding or `anonymous_profile` base — a base is never named back to a non-administrator caller (research D27). `set_profile("")` is never a switch and is admitted for **every** caller, as today (`profile_tool.go`: an empty slug clears the selection and is always accepted): it clears the stored selection and the next resolution falls through to the caller's base — the pin, the binding (a switchable client returns to its bound profile, `profile_source=binding`), `anonymous_profile`, or none; it is never answered with the uniform refusal. - **FR-019**: Aggregated prompts and resources keep Spec 105 server-scope filtering; tier policy and tool rules do not apply to prompts or resources (they have no tier) — recorded as a decision, not a gap, and pinned by a regression test (a profile's `max_tier`, `unannotated` and `tools.deny` leave `prompts/list` and `prompts/get` identical to a legacy profile with the same `servers`; resources are not aggregated today, and a future resource aggregation inherits the same test), so a shared visibility predicate can never start filtering them silently. **C. Resolution, identity and binding (PV-A/PV-B)** diff --git a/specs/108-profiles-v3/tasks.md b/specs/108-profiles-v3/tasks.md index bc8d8ff15..1cee8c485 100644 --- a/specs/108-profiles-v3/tasks.md +++ b/specs/108-profiles-v3/tasks.md @@ -338,6 +338,15 @@ Four findings from a live demo of `main` at `b3191a059`; Spec 109's T160–T165 - [x] T151 [US2] #9 radio and checkbox labels sit next to their control: one unlayered rule in `frontend/src/assets/main.css`, pinned by `frontend/tests/unit/form-control-label-shim.spec.ts` and the Playwright sweep `e2e/web-ui-sweep/demo-ux-fixes.spec.ts` - [x] T152 [US2] Docs and bookkeeping for T148–T151: `docs/features/profiles.md`, `contracts/refusals.md`, `contracts/mcp-tools.md`, FR-011, FR-042, FR-048, `parity-matrix.json`, research D39 +## Phase 17: PR fix-usertest-cli — CLI and `set_profile` findings from the first-run user test (2026-10-02) + +Go only. F-01, F-06 and F-09 are CLI-wide (not profile-specific) and are listed for traceability; F-11 changes the Spec 108 client-credential refusal. + +- [x] T153 F-01 `mcpproxy doctor` never prints the admin key or a URL credential in any format; `-o yaml` is a real format. Tests: `TestDoctorOutput_RedactsAdminKeyInEveryFormat`, `TestDoctorOutput_ScrubsAdminKeyLiteral`, `TestDoctorOutput_UnsupportedFormatErrors`, `TestStatusOutput_MasksKeyInEveryFormat` +- [x] T154 F-06 the global `-c`/`-d` are authoritative for every management command; an empty value is rejected at parse time. Tests: `TestConfigFlagsRejectEmptyValue`, `TestLoadersHonorGlobalConfigFlag`, `TestLoadersPreferDataDirConfig`, `TestLoadCLIConfigDataDirWithoutFileCreatesNothingInHome`, `TestUpstreamConfigFilePathMatchesLoadPath`, `TestResolveCLIConfigPathPrecedence` +- [x] T155 F-09 a command error is printed once: `executeRoot`. Tests: `TestExecuteRoot_PrintsErrorOnce`, `TestExecuteRoot_UnknownCommandKeepsHelpHint` +- [x] T156 [US2] F-11 `set_profile` tells a client credential whether its binding is locked or switchable (FR-018, per-caller uniform refusal). Golden `internal/profile/testdata/contract/set_profile_refusals.json`; tests `TestSetProfileRefusalGolden`, `TestSetProfileV3_ClientRefusalTextMatchesGolden`, and the updated `set_profile_v3_test.go` and `profiles_v3_acceptance_test.go` expectations + --- ## Dependencies & Execution Order @@ -373,4 +382,4 @@ MVP = 108-a + 108-b (US1 discovery via config + `/mcp/p/` or pinned tokens ## Task Count -186 tasks: setup 3 · a 14 · b 13 · c 24 · d 19 · e 14 · f 17 · g 8 · h 4 · i 16 · j 11 · k 21 · l 9 · retro-go 7 · fix-1458 1 · demo-ux-fixes 5 (counted mechanically; the 108-l plan carries the counting script). History: codex round 4 added T005a; codex round 3 added T046c, T052b; codex round 1 added T004a, T016a, T027a, T027b, T028a, T030a, T033a, T040a, T055a; 108-j's composable/link-map tasks were merged into Spec 109-k and replaced by profile-specific UI tasks; 108-j added T106a, T107a, T108a, T109a; the post-merge macOS and Go review fixes added to k and retro-go; 108-l added T124a, T126a, T127; fix-1458 added T147; demo-ux-fixes added T148–T152. +190 tasks: setup 3 · a 14 · b 13 · c 24 · d 19 · e 14 · f 17 · g 8 · h 4 · i 16 · j 11 · k 21 · l 9 · retro-go 7 · fix-1458 1 · demo-ux-fixes 5 · fix-usertest-cli 4 (counted mechanically; the 108-l plan carries the counting script). History: codex round 4 added T005a; codex round 3 added T046c, T052b; codex round 1 added T004a, T016a, T027a, T027b, T028a, T030a, T033a, T040a, T055a; 108-j's composable/link-map tasks were merged into Spec 109-k and replaced by profile-specific UI tasks; 108-j added T106a, T107a, T108a, T109a; the post-merge macOS and Go review fixes added to k and retro-go; 108-l added T124a, T126a, T127; fix-1458 added T147; demo-ux-fixes added T148–T152; fix-usertest-cli added T153–T156.