Skip to content
20 changes: 14 additions & 6 deletions chat/selfconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ type addMCPServerArgs struct {
Env []string `json:"env,omitempty" jsonschema:"environment variables, each as a KEY=VALUE string"`
URL string `json:"url,omitempty" jsonschema:"base URL for a remote MCP server (use instead of command)"`
Transport string `json:"transport,omitempty" jsonschema:"remote transport: http (default) or sse"`
Token string `json:"token,omitempty" jsonschema:"bearer token for a remote MCP server"`
Headers []string `json:"headers,omitempty" jsonschema:"custom HTTP headers for a remote MCP server, each as a KEY=VALUE string"`
}

// selfConfigToolDefs builds the ten self-configuration tools. reload is called
Expand Down Expand Up @@ -214,7 +216,11 @@ func selfConfigToolDefs(c *manage.Configurator, reload func()) []toolDef {
if tr == "" {
tr = "http"
}
fmt.Fprintf(&b, "- %s: %s %s\n", s.Name, tr, s.URL)
suffix := ""
if s.Authenticated {
suffix = " (authenticated)"
}
fmt.Fprintf(&b, "- %s: %s %s%s\n", s.Name, tr, s.URL, suffix)
} else {
fmt.Fprintf(&b, "- %s: %s %s\n", s.Name, s.Command, strings.Join(s.Args, " "))
}
Expand All @@ -227,11 +233,13 @@ func selfConfigToolDefs(c *manage.Configurator, reload func()) []toolDef {
addMCPServerArgs{}, func(args map[string]any) (string, error) {
name := argStr(args, "name")
srv := types.MCPServer{
Command: argStr(args, "command"),
Args: argStrSlice(args, "args"),
Env: argEnvMap(args, "env"),
URL: argStr(args, "url"),
Transport: argStr(args, "transport"),
Command: argStr(args, "command"),
Args: argStrSlice(args, "args"),
Env: argEnvMap(args, "env"),
URL: argStr(args, "url"),
Transport: argStr(args, "transport"),
BearerToken: argStr(args, "token"),
Headers: argEnvMap(args, "headers"),
}
if err := c.AddMCPServer(name, srv); err != nil {
return "", err
Expand Down
38 changes: 38 additions & 0 deletions chat/selfconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,41 @@ func TestAddMCPServerParsesEnv(t *testing.T) {
t.Fatalf("add_mcp_server: %q", out)
}
}

func TestAddMCPServerParsesTokenAndHeaders(t *testing.T) {
c := newToolConfigurator(t)
defs := selfConfigToolDefs(c, func() {})
out := runTool(t, defs, "add_mcp_server", map[string]any{
"name": "remote", "url": "https://x/mcp",
"token": "secret123",
"headers": []any{"X-Api-Key=k1"},
})
if !strings.Contains(out, "remote") {
t.Fatalf("add_mcp_server: %q", out)
}
srv, err := c.GetMCPServer("remote")
if err != nil {
t.Fatalf("GetMCPServer: %v", err)
}
if srv.BearerToken != "secret123" {
t.Fatalf("token: got %q, want %q", srv.BearerToken, "secret123")
}
if srv.Headers["X-Api-Key"] != "k1" {
t.Fatalf("headers: %v", srv.Headers)
}
}

func TestListMCPServersMarksAuthenticated(t *testing.T) {
c := newToolConfigurator(t)
defs := selfConfigToolDefs(c, func() {})
runTool(t, defs, "add_mcp_server", map[string]any{
"name": "authed", "url": "https://x/mcp", "token": "secret123",
})
out := runTool(t, defs, "list_mcp_servers", map[string]any{})
if !strings.Contains(out, "authed") || !strings.Contains(out, "authenticated") {
t.Fatalf("list_mcp_servers: %q", out)
}
if strings.Contains(out, "secret123") {
t.Fatalf("list_mcp_servers leaked the token: %q", out)
}
}
36 changes: 34 additions & 2 deletions cmd/mcpmanage.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ func parseAddArgs(args []string) (string, types.MCPServer, error) {
}
}
env := map[string]string{}
headers := map[string]string{}
name := ""
needValue := func(i int, flag string) (string, error) {
if i+1 >= len(args) {
Expand Down Expand Up @@ -104,6 +105,30 @@ func parseAddArgs(args []string) (string, types.MCPServer, error) {
return "", srv, fmt.Errorf("--env must be KEY=VALUE")
}
env[k] = val
case a == "--token":
v, err := needValue(i, "--token")
if err != nil {
return "", srv, err
}
srv.BearerToken, i = v, i+1
case strings.HasPrefix(a, "--token="):
srv.BearerToken = strings.TrimPrefix(a, "--token=")
case a == "--header":
v, err := needValue(i, "--header")
if err != nil {
return "", srv, err
}
k, val, ok := strings.Cut(v, "=")
if !ok {
return "", srv, fmt.Errorf("--header must be KEY=VALUE, got %q", v)
}
headers[k], i = val, i+1
case strings.HasPrefix(a, "--header="):
k, val, ok := strings.Cut(strings.TrimPrefix(a, "--header="), "=")
if !ok {
return "", srv, fmt.Errorf("--header must be KEY=VALUE")
}
headers[k] = val
case strings.HasPrefix(a, "-"):
return "", srv, fmt.Errorf("unknown flag: %s", a)
default:
Expand All @@ -123,6 +148,9 @@ func parseAddArgs(args []string) (string, types.MCPServer, error) {
if len(env) > 0 {
srv.Env = env
}
if len(headers) > 0 {
srv.Headers = headers
}
if srv.Transport != "" && srv.Transport != "http" && srv.Transport != "sse" {
return "", srv, fmt.Errorf("--transport must be http or sse, got %q", srv.Transport)
}
Expand All @@ -137,7 +165,7 @@ func mcpAdd(cfgr *manage.Configurator, args []string) int {
if err != nil {
fmt.Fprintf(os.Stderr, "%v\n", err)
fmt.Fprintln(os.Stderr, "usage: nib mcp add <name> [--env K=V]... -- <command> [args...]")
fmt.Fprintln(os.Stderr, " nib mcp add <name> --url <url> [--transport http|sse]")
fmt.Fprintln(os.Stderr, " nib mcp add <name> --url <url> [--transport http|sse] [--token T] [--header K=V]...")
return 1
}
if err := cfgr.AddMCPServer(name, srv); err != nil {
Expand All @@ -164,7 +192,11 @@ func mcpList(cfgr *manage.Configurator) int {
if tr == "" {
tr = "http"
}
fmt.Printf("%-20s %s %s\n", s.Name, tr, s.URL)
suffix := ""
if s.Authenticated {
suffix = " (authenticated)"
}
fmt.Printf("%-20s %s %s%s\n", s.Name, tr, s.URL, suffix)
} else {
fmt.Printf("%-20s %s\n", s.Name, strings.TrimSpace(s.Command+" "+strings.Join(s.Args, " ")))
}
Expand Down
93 changes: 89 additions & 4 deletions cmd/mcpmanage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"testing"

"github.com/mudler/nib/manage"
"github.com/mudler/nib/types"
)

func TestParseAddArgsStdio(t *testing.T) {
Expand Down Expand Up @@ -70,11 +71,11 @@ func TestParseAddArgsRepeatedEnv(t *testing.T) {

func TestParseAddArgsErrors(t *testing.T) {
cases := [][]string{
{}, // missing name
{"foo"}, // neither command nor url
{"foo", "--url", "http://x", "--", "cmd"}, // both url and command
{}, // missing name
{"foo"}, // neither command nor url
{"foo", "--url", "http://x", "--", "cmd"}, // both url and command
{"foo", "--transport", "ftp", "--url", "http://x"}, // bad transport
{"foo", "--env", "noequals", "--", "cmd"}, // bad env
{"foo", "--env", "noequals", "--", "cmd"}, // bad env
}
for i, args := range cases {
if _, _, err := parseAddArgs(args); err == nil {
Expand All @@ -94,6 +95,90 @@ func TestMCPTestMissingServer(t *testing.T) {
}
}

func TestParseAddArgsToken(t *testing.T) {
name, srv, err := parseAddArgs([]string{"remote", "--url", "https://x/mcp", "--token", "secret123"})
if err != nil {
t.Fatalf("parseAddArgs: %v", err)
}
if name != "remote" || srv.BearerToken != "secret123" {
t.Fatalf("got name=%q token=%q", name, srv.BearerToken)
}
}

func TestParseAddArgsTokenInlineEquals(t *testing.T) {
_, srv, err := parseAddArgs([]string{"remote", "--url=https://x/mcp", "--token=secret123"})
if err != nil {
t.Fatalf("parseAddArgs: %v", err)
}
if srv.BearerToken != "secret123" {
t.Fatalf("token: got %q, want %q", srv.BearerToken, "secret123")
}
}

func TestParseAddArgsRepeatedHeader(t *testing.T) {
_, srv, err := parseAddArgs([]string{"remote", "--url", "https://x/mcp", "--header", "X-Api-Key=k1", "--header=X-Other=v2"})
if err != nil {
t.Fatalf("parseAddArgs: %v", err)
}
if srv.Headers["X-Api-Key"] != "k1" || srv.Headers["X-Other"] != "v2" {
t.Fatalf("headers: %v", srv.Headers)
}
}

func TestParseAddArgsHeaderValueWithEquals(t *testing.T) {
// A header value may itself contain "=": strings.Cut must split on the
// first "=" only, leaving the rest of the value intact.
_, srv, err := parseAddArgs([]string{"remote", "--url", "https://x/mcp", "--header", "X-Foo=a=b"})
if err != nil {
t.Fatalf("parseAddArgs: %v", err)
}
if srv.Headers["X-Foo"] != "a=b" {
t.Fatalf("header X-Foo: got %q, want %q (headers=%v)", srv.Headers["X-Foo"], "a=b", srv.Headers)
}
}

func TestParseAddArgsAuthErrors(t *testing.T) {
cases := [][]string{
{"foo", "--url", "http://x", "--token"}, // --token needs a value
{"foo", "--url", "http://x", "--header", "noequals"}, // bad header
{"foo", "--url", "http://x", "--header"}, // --header needs a value
}
for i, args := range cases {
if _, _, err := parseAddArgs(args); err == nil {
t.Fatalf("case %d %v: expected error", i, args)
}
}
}

func TestMCPListShowsAuthenticatedMarker(t *testing.T) {
dir := t.TempDir()
cfgr := manage.New(dir, dir+"/config.yaml")
if err := cfgr.AddMCPServer("plain", types.MCPServer{URL: "https://a"}); err != nil {
t.Fatalf("AddMCPServer plain: %v", err)
}
if err := cfgr.AddMCPServer("authed", types.MCPServer{URL: "https://b", BearerToken: "tok"}); err != nil {
t.Fatalf("AddMCPServer authed: %v", err)
}
servers, err := cfgr.ListMCPServers()
if err != nil {
t.Fatalf("ListMCPServers: %v", err)
}
byName := map[string]manage.MCPServerInfo{}
for _, s := range servers {
byName[s.Name] = s
}
if byName["plain"].Authenticated {
t.Fatalf("plain should not be authenticated")
}
if !byName["authed"].Authenticated {
t.Fatalf("authed should be authenticated")
}
// mcpList itself writes to stdout; the redaction logic it depends on
// (MCPServerInfo.Authenticated) is exercised above. A full stdout-capture
// test isn't warranted here — mcpList has no existing stdout tests either,
// consistent with the rest of this file.
}

func TestIsMCPManageSubcommand(t *testing.T) {
for _, s := range []string{"add", "list", "remove", "test"} {
if !IsMCPManageSubcommand(s) {
Expand Down
35 changes: 29 additions & 6 deletions manage/mcpconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"os"
"path/filepath"
"sort"
"strings"

"github.com/mudler/nib/types"

Expand All @@ -13,11 +14,16 @@ import (

// MCPServerInfo is a configured MCP server in tool-facing form.
type MCPServerInfo struct {
Name string
Command string
Args []string
URL string
Transport string
Name string
Command string
Args []string
URL string
Transport string
// Authenticated is true if BearerToken or ANY custom header is set; the values
// themselves are never exposed here. Note this deliberately does not distinguish
// a real auth token from an ordinary custom header — its purpose is "don't
// silently print secrets", not "classify which headers are authentication".
Authenticated bool
}

// userConfigServers reads only the user config file's mcp_servers map (not the
Expand Down Expand Up @@ -79,7 +85,14 @@ func (c *Configurator) ListMCPServers() ([]MCPServerInfo, error) {
}
out := make([]MCPServerInfo, 0, len(servers))
for name, s := range servers {
out = append(out, MCPServerInfo{Name: name, Command: s.Command, Args: s.Args, URL: s.URL, Transport: s.Transport})
out = append(out, MCPServerInfo{
Name: name,
Command: s.Command,
Args: s.Args,
URL: s.URL,
Transport: s.Transport,
Authenticated: s.BearerToken != "" || len(s.Headers) > 0,
})
}
sort.Slice(out, func(i, j int) bool { return out[i].Name < out[j].Name })
return out, nil
Expand All @@ -102,6 +115,16 @@ func (c *Configurator) AddMCPServer(name string, srv types.MCPServer) error {
return fmt.Errorf("transport is only valid for remote servers (url must be set)")
}
}
if (srv.BearerToken != "" || len(srv.Headers) > 0) && srv.URL == "" {
return fmt.Errorf("token/headers are only valid for remote servers (url must be set)")
}
if srv.BearerToken != "" {
for k := range srv.Headers {
if strings.EqualFold(k, "Authorization") {
return fmt.Errorf("cannot set both token and an Authorization header")
}
}
}
servers, err := userConfigServers(c.configPath)
if err != nil {
return err
Expand Down
60 changes: 60 additions & 0 deletions manage/mcpconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,63 @@ func containsAll(s string, subs ...string) bool {
}
return true
}

func TestAddMCPServerAuthValidation(t *testing.T) {
c, _ := newTestConfigurator(t)
// token/headers only valid for remote (url) servers
if err := c.AddMCPServer("bad", types.MCPServer{Command: "cmd", BearerToken: "tok"}); err == nil {
t.Fatalf("expected error: token on stdio server")
}
if err := c.AddMCPServer("bad", types.MCPServer{Command: "cmd", Headers: map[string]string{"X-Api-Key": "k"}}); err == nil {
t.Fatalf("expected error: headers on stdio server")
}
// BearerToken + Headers["Authorization"] (any case) is ambiguous
if err := c.AddMCPServer("bad", types.MCPServer{URL: "https://x", BearerToken: "tok", Headers: map[string]string{"Authorization": "Bearer other"}}); err == nil {
t.Fatalf("expected error: BearerToken + Authorization header")
}
if err := c.AddMCPServer("bad", types.MCPServer{URL: "https://x", BearerToken: "tok", Headers: map[string]string{"authorization": "Bearer other"}}); err == nil {
t.Fatalf("expected error: BearerToken + lowercase authorization header")
}
// valid combinations
if err := c.AddMCPServer("ok1", types.MCPServer{URL: "https://x", BearerToken: "tok"}); err != nil {
t.Fatalf("AddMCPServer token: %v", err)
}
if err := c.AddMCPServer("ok2", types.MCPServer{URL: "https://y", Headers: map[string]string{"X-Api-Key": "k"}}); err != nil {
t.Fatalf("AddMCPServer headers: %v", err)
}
got, err := c.GetMCPServer("ok1")
if err != nil || got.BearerToken != "tok" {
t.Fatalf("GetMCPServer ok1: %+v, err=%v", got, err)
}
}

func TestListMCPServersRedactsAuth(t *testing.T) {
c, _ := newTestConfigurator(t)
if err := c.AddMCPServer("plain", types.MCPServer{URL: "https://a"}); err != nil {
t.Fatalf("AddMCPServer plain: %v", err)
}
if err := c.AddMCPServer("authed", types.MCPServer{URL: "https://b", BearerToken: "tok"}); err != nil {
t.Fatalf("AddMCPServer authed: %v", err)
}
if err := c.AddMCPServer("headed", types.MCPServer{URL: "https://c", Headers: map[string]string{"X-Api-Key": "k"}}); err != nil {
t.Fatalf("AddMCPServer headed: %v", err)
}
servers, err := c.ListMCPServers()
if err != nil {
t.Fatalf("ListMCPServers: %v", err)
}
byName := map[string]MCPServerInfo{}
for _, s := range servers {
byName[s.Name] = s
}
if byName["plain"].Authenticated {
t.Fatalf("plain server should not be marked authenticated")
}
if !byName["authed"].Authenticated {
t.Fatalf("authed server should be marked authenticated")
}
// headers-only exercises the len(s.Headers) > 0 side of the OR
if !byName["headed"].Authenticated {
t.Fatalf("headers-only server should be marked authenticated")
}
}
Loading
Loading