From 2de4cfcc1289f2b6d108eda1da39daec904fa4e8 Mon Sep 17 00:00:00 2001 From: prode Date: Mon, 8 Jun 2026 19:47:00 -0300 Subject: [PATCH] fix(mcp): add authorization header for github preset --- internal/cli/mcp.go | 32 ++++++++++++++++++++++++++++---- internal/cli/mcp_presets.go | 5 ++++- internal/cli/mcp_presets_test.go | 6 +++++- internal/cli/mcp_test.go | 20 +++++++++++++------- 4 files changed, 50 insertions(+), 13 deletions(-) diff --git a/internal/cli/mcp.go b/internal/cli/mcp.go index efd56d7..5681873 100644 --- a/internal/cli/mcp.go +++ b/internal/cli/mcp.go @@ -30,6 +30,7 @@ type MCPServer struct { Env map[string]string `json:"env,omitempty"` URL string `json:"url,omitempty"` Type string `json:"type,omitempty"` + Headers map[string]string `json:"headers,omitempty"` Disabled bool `json:"disabled,omitempty"` AutoApprove []string `json:"autoApprove,omitempty"` } @@ -117,6 +118,7 @@ type MCPAddOptions struct { Command string Args []string Env []string // raw "K=V" pairs; parsed in MCPAdd + Headers []string // raw "K=V" pairs; parsed in MCPAdd URL string Type string Disabled bool @@ -127,11 +129,12 @@ type MCPAddOptions struct { func mcpAdd(args []string) int { fs := flag.NewFlagSet("mcp add", flag.ContinueOnError) var opts MCPAddOptions - var argv, env, auto stringSliceFlag + var argv, env, headers, auto stringSliceFlag addRoot(fs, &opts.Root) fs.StringVar(&opts.Command, "command", "", "Executable for a stdio server.") fs.Var(&argv, "arg", "Argument for the command (repeatable).") fs.Var(&env, "env", "Environment variable K=V (repeatable).") + fs.Var(&headers, "header", "HTTP header K=V for a remote server (repeatable).") fs.StringVar(&opts.URL, "url", "", "Endpoint for a remote server.") fs.StringVar(&opts.Type, "type", "", "Remote transport: sse|http (default http when --url is set).") fs.BoolVar(&opts.Disabled, "disabled", false, "Add the server in a disabled state.") @@ -142,12 +145,13 @@ func mcpAdd(args []string) int { return failOnFlagParse(err) } if len(positionals) < 1 { - render.Err("usage: " + prog() + " mcp add NAME (--command CMD [--arg A]... | --url URL [--type sse|http]) [--env K=V]... [--disabled] [--force]") + render.Err("usage: " + prog() + " mcp add NAME (--command CMD [--arg A]... | --url URL [--type sse|http] [--header K=V]...) [--env K=V]... [--disabled] [--force]") return 1 } opts.Name = positionals[0] opts.Args = argv.values opts.Env = env.values + opts.Headers = headers.values opts.AutoApprove = auto.values if err := MCPAdd(opts); err != nil { render.Err(err.Error()) @@ -207,6 +211,10 @@ func buildMCPServer(opts MCPAddOptions) (MCPServer, error) { if err != nil { return srv, err } + headers, err := parseKV("--header", opts.Headers) + if err != nil { + return srv, err + } if hasURL { t := opts.Type if t == "" { @@ -217,10 +225,14 @@ func buildMCPServer(opts MCPAddOptions) (MCPServer, error) { } srv.URL = opts.URL srv.Type = t + srv.Headers = headers } else { if opts.Type != "" && opts.Type != "stdio" { return srv, fmt.Errorf("--type %q is only valid with --url; stdio servers omit it", opts.Type) } + if len(headers) > 0 { + return srv, fmt.Errorf("--header is only valid with --url remote servers") + } srv.Command = opts.Command srv.Args = opts.Args } @@ -231,6 +243,10 @@ func buildMCPServer(opts MCPAddOptions) (MCPServer, error) { } func parseEnvKV(pairs []string) (map[string]string, error) { + return parseKV("--env", pairs) +} + +func parseKV(flagName string, pairs []string) (map[string]string, error) { if len(pairs) == 0 { return nil, nil } @@ -238,11 +254,11 @@ func parseEnvKV(pairs []string) (map[string]string, error) { for _, p := range pairs { i := strings.Index(p, "=") if i <= 0 { - return nil, fmt.Errorf("--env must be in K=V form: got %q", p) + return nil, fmt.Errorf("%s must be in K=V form: got %q", flagName, p) } key := strings.TrimSpace(p[:i]) if key == "" { - return nil, fmt.Errorf("--env key is empty in %q", p) + return nil, fmt.Errorf("%s key is empty in %q", flagName, p) } out[key] = p[i+1:] } @@ -449,6 +465,14 @@ func validateMCPConfig(cfg MCPConfig) []validator.Issue { issues = append(issues, validator.Issue{File: "mcp.json", Msg: fmt.Sprintf("server '%s' has an empty env key", name)}) } } + for k := range s.Headers { + if strings.TrimSpace(k) == "" { + issues = append(issues, validator.Issue{File: "mcp.json", Msg: fmt.Sprintf("server '%s' has an empty header key", name)}) + } + } + if hasCmd && len(s.Headers) > 0 { + issues = append(issues, validator.Issue{File: "mcp.json", Msg: fmt.Sprintf("server '%s' is stdio but sets headers", name)}) + } } return issues } diff --git a/internal/cli/mcp_presets.go b/internal/cli/mcp_presets.go index 5a90b0e..f0817ad 100644 --- a/internal/cli/mcp_presets.go +++ b/internal/cli/mcp_presets.go @@ -21,6 +21,7 @@ type Preset struct { Args []string // stdio only URL string // remote only Type string // remote only: "sse" | "http" + Headers []string // remote only: static header K=V pairs; keep secret-free Note string // optional hint emitted after install (e.g. auth caveat) } @@ -47,7 +48,8 @@ var mcpPresetRegistry = map[string]Preset{ Transport: "http", URL: "https://api.githubcopilot.com/mcp/", Type: "http", - Note: "Requires GitHub auth — authenticate via your client's OAuth on first use (or add a PAT). No token is stored by csdd.", + Headers: []string{"Authorization=Bearer ${GITHUB_PAT}"}, + Note: "Requires GitHub auth — set GITHUB_PAT to a PAT with the required scopes before connecting. No token is stored by csdd.", }, } @@ -100,6 +102,7 @@ func MCPInstallPreset(opts MCPInstallPresetOptions) error { Name: p.Name, Command: p.Command, Args: p.Args, + Headers: p.Headers, URL: p.URL, Type: p.Type, Force: opts.Force, diff --git a/internal/cli/mcp_presets_test.go b/internal/cli/mcp_presets_test.go index 49fe2df..aa537fd 100644 --- a/internal/cli/mcp_presets_test.go +++ b/internal/cli/mcp_presets_test.go @@ -63,6 +63,9 @@ func TestMCPInstallGithub(t *testing.T) { if srv.URL != "https://api.githubcopilot.com/mcp/" || srv.Type != "http" { t.Errorf("github remote config wrong: %+v", srv) } + if srv.Headers["Authorization"] != "Bearer ${GITHUB_PAT}" { + t.Errorf("github preset must include Authorization header with env placeholder, got: %+v", srv.Headers) + } if srv.Command != "" || len(srv.Env) != 0 { t.Errorf("github preset must store no command and no secret env: %+v", srv) } @@ -179,7 +182,8 @@ func TestMCPInstallGithubEmitsAuthNote(t *testing.T) { if code != 0 { t.Fatal("install github should succeed") } - if !strings.Contains(strings.ToLower(out), "auth") { + lower := strings.ToLower(out) + if !strings.Contains(lower, "auth") || !strings.Contains(out, "GITHUB_PAT") { t.Errorf("github install should surface an auth note:\n%s", out) } } diff --git a/internal/cli/mcp_test.go b/internal/cli/mcp_test.go index 6ec284b..9907d16 100644 --- a/internal/cli/mcp_test.go +++ b/internal/cli/mcp_test.go @@ -19,7 +19,9 @@ func TestMCPAddStdioAndRemote(t *testing.T) { t.Fatal("stdio add should succeed") } if code, _, _ := run(t, "mcp", "add", "linear", - "--url", "https://mcp.linear.app/sse", "--type", "sse", "--root", dir); code != 0 { + "--url", "https://mcp.linear.app/sse", "--type", "sse", + "--header", "Authorization=Bearer ${LINEAR_API_KEY}", + "--root", dir); code != 0 { t.Fatal("remote add should succeed") } cfg, err := loadMCP(mcpJSONPath(dir)) @@ -31,7 +33,7 @@ func TestMCPAddStdioAndRemote(t *testing.T) { t.Errorf("stdio server not stored correctly: %+v", fsrv) } lin, ok := cfg.MCPServers["linear"] - if !ok || lin.URL == "" || lin.Type != "sse" { + if !ok || lin.URL == "" || lin.Type != "sse" || lin.Headers["Authorization"] != "Bearer ${LINEAR_API_KEY}" { t.Errorf("remote server not stored correctly: %+v", lin) } // stdio server must not carry a type, remote must not carry a command. @@ -58,6 +60,8 @@ func TestMCPAddErrors(t *testing.T) { {"mcp", "add", "x", "--root", dir}, // neither transport {"mcp", "add", "x", "--url", "u", "--type", "ftp", "--root", dir}, // bad remote type {"mcp", "add", "x", "--command", "c", "--env", "NOEQ", "--root", dir}, + {"mcp", "add", "x", "--url", "u", "--header", "NOEQ", "--root", dir}, + {"mcp", "add", "x", "--command", "c", "--header", "Authorization=Bearer x", "--root", dir}, {"mcp", "add", "Bad_Name", "--command", "c", "--root", dir}, // non-kebab } for _, args := range cases { @@ -210,22 +214,24 @@ func TestParseEnvKV(t *testing.T) { func TestValidateMCPConfigDirect(t *testing.T) { cfg := MCPConfig{MCPServers: map[string]MCPServer{ "good-stdio": {Command: "c"}, - "good-remote": {URL: "u", Type: "http"}, + "good-remote": {URL: "u", Type: "http", Headers: map[string]string{"Authorization": "Bearer ${TOKEN}"}}, "empty": {}, "both": {Command: "c", URL: "u", Type: "http"}, "bad-type": {URL: "u", Type: "ftp"}, "stdio-typed": {Command: "c", Type: "http"}, + "stdio-head": {Command: "c", Headers: map[string]string{"Authorization": "Bearer token"}}, + "empty-head": {URL: "u", Type: "http", Headers: map[string]string{"": "Bearer token"}}, }} issues := validateMCPConfig(cfg) - // Expect exactly the four broken servers to be flagged. - if len(issues) != 4 { - t.Fatalf("expected 4 issues, got %d: %v", len(issues), issues) + // Expect exactly the six broken servers to be flagged. + if len(issues) != 6 { + t.Fatalf("expected 6 issues, got %d: %v", len(issues), issues) } joined := "" for _, i := range issues { joined += i.String() + "\n" } - for _, want := range []string{"empty", "both", "bad-type", "stdio-typed"} { + for _, want := range []string{"empty", "both", "bad-type", "stdio-typed", "stdio-head", "empty-head"} { if !strings.Contains(joined, want) { t.Errorf("expected an issue mentioning %q:\n%s", want, joined) }