diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c9b519..786e6d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 Install via the [`platform-engineering-labs/formae-marketplace`](https://github.com/platform-engineering-labs/formae-marketplace). +## [Unreleased] + +### Fixed + +- The MCP now sends the HTTP basic credentials a profile declares, so agents + behind auth are reachable. Previously only `cli.api`'s `url` and `port` were + read out of a profile and no `Authorization` header was ever set, so every + tool call against an authenticated agent came back `401 unauthorized` even + though the credentials were sitting in the profile the whole time. This made + the entire hosted fleet invisible to the MCP. Credentials are picked up from + `cli.auth` and from auth nested under `cli.connection`, and are applied to + every request the client makes rather than only some of them. + ## [0.8.0] ### Changed diff --git a/internal/config/config.go b/internal/config/config.go index 06702ea..6804d83 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -66,6 +66,39 @@ func AgentEndpoint(profileName string) (url, port string, err error) { return url, port, nil } +// AgentCredentials resolves the HTTP basic credentials for an optional profile, +// following the same profile precedence as AgentEndpoint: an explicit profile +// if given, else the active pointer. +// +// Absent credentials are not an error. Most agents are reachable over the +// tailnet with no auth at all, so an empty result simply means "send no +// Authorization header". Only a profile that cannot be read is an error, and +// an environment with no profile configured yields nothing rather than failing. +func AgentCredentials(profileName string) (username, password string, err error) { + name := profileName + if name == "" { + active, aerr := profile.ActiveProfile() + if aerr != nil { + if errors.Is(aerr, profile.ErrNotInitialized) { + return "", "", nil + } + return "", "", aerr + } + name = active + } + + path, err := profile.ProfilePath(name) + if err != nil { + return "", "", err + } + data, rerr := os.ReadFile(path) + if rerr != nil { + return "", "", fmt.Errorf("profile %q not found: %w", name, rerr) + } + username, password = parseCliAuth(string(data)) + return username, password, nil +} + // endpointFromProfile reads a profile's PKL and extracts its cli.api endpoint. // A profile that exists but yields neither url nor port is a hard error. func endpointFromProfile(name string) (url, port string, err error) { @@ -171,3 +204,103 @@ func parseCliAPI(content string) (url, port string) { return url, port } + +var ( + usernamePattern = regexp.MustCompile(`username\s*=\s*"([^"]*)"`) + passwordPattern = regexp.MustCompile(`password\s*=\s*"([^"]*)"`) +) + +// opensBlock reports whether the line opens a block named name, as a whole +// identifier. Matches both `name {` and `name = new Something {`, which is how +// a profile writes `auth = new AuthBasic.CliConfig {`. +func opensBlock(line, name string) bool { + idx := strings.Index(line, name) + if idx < 0 || !strings.Contains(line[idx:], "{") { + return false + } + isIdentChar := func(b byte) bool { + return b == '_' || + (b >= 'a' && b <= 'z') || + (b >= 'A' && b <= 'Z') || + (b >= '0' && b <= '9') + } + if idx > 0 && isIdentChar(line[idx-1]) { + return false + } + if end := idx + len(name); end < len(line) && isIdentChar(line[end]) { + return false + } + return true +} + +// parseCliAuth extracts HTTP basic-auth credentials from the cli block of a +// profile's PKL. It accepts them wherever they sit under `cli` — today's +// `cli.auth = new AuthBasic.CliConfig { ... }` and the `cli.connection` +// nesting the CLI is deprecating towards — because both spell the credentials +// the same way and only their position differs. +// +// Credentials outside `cli` are deliberately ignored: the agent block carries +// the server side of the same plugin, and sending that as client credentials +// would be wrong. +// +// Like parseCliAPI this is brace-depth text parsing, not a PKL evaluator, so a +// profile that computes its credentials rather than writing them literally +// yields nothing and the caller falls back to an unauthenticated client. +func parseCliAuth(content string) (username, password string) { + inCli := false + inAuth := false + cliDepth := 0 + authDepth := 0 + + for _, line := range strings.Split(content, "\n") { + trimmed := strings.TrimSpace(line) + if strings.HasPrefix(trimmed, "//") { + continue + } + + if !inCli && opensBlock(trimmed, "cli") { + inCli = true + cliDepth = 0 + } + if inCli && !inAuth && opensBlock(trimmed, "auth") { + inAuth = true + authDepth = 0 + } + + if inCli && inAuth { + if m := usernamePattern.FindStringSubmatch(trimmed); len(m) > 1 { + username = m[1] + } + if m := passwordPattern.FindStringSubmatch(trimmed); len(m) > 1 { + password = m[1] + } + } + + for _, ch := range trimmed { + switch ch { + case '{': + if inCli { + cliDepth++ + } + if inAuth { + authDepth++ + } + case '}': + if inAuth { + authDepth-- + if authDepth == 0 { + inAuth = false + } + } + if inCli { + cliDepth-- + if cliDepth == 0 { + inCli = false + } + } + } + } + } + + return username, password +} diff --git a/internal/config/config_test.go b/internal/config/config_test.go index d6e2bae..0b97313 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -405,3 +405,169 @@ agent { t.Errorf("expected port '8080' (not agent.api.port 12345), got %q", port) } } + +func TestParseCliAuth_BasicCredentials(t *testing.T) { + content := `amends "formae:/Config.pkl" + +import "plugins:/AuthBasic.pkl" as AuthBasic + +cli { + api { + url = "http://fleet.example.com" + port = 8080 + } + auth = new AuthBasic.CliConfig { + username = "formae" + password = "s3cr3t" + } + disableUsageReporting = true +} +` + username, password := parseCliAuth(content) + if username != "formae" { + t.Errorf("expected username 'formae', got '%s'", username) + } + if password != "s3cr3t" { + t.Errorf("expected password 's3cr3t', got '%s'", password) + } +} + +func TestParseCliAuth_InsideConnectionBlock(t *testing.T) { + // The CLI deprecates `cli.auth` in favour of auth nested under + // `cli.connection`; credentials must be found in either position. + content := `amends "formae:/Config.pkl" + +cli { + connection = new Classic { + url = "http://fleet.example.com" + port = 8080 + auth { + username = "formae" + password = "s3cr3t" + } + } +} +` + username, password := parseCliAuth(content) + if username != "formae" { + t.Errorf("expected username 'formae', got '%s'", username) + } + if password != "s3cr3t" { + t.Errorf("expected password 's3cr3t', got '%s'", password) + } +} + +func TestParseCliAuth_NoAuthBlock(t *testing.T) { + content := `amends "formae:/Config.pkl" + +cli { + api { + url = "http://agent.example.com" + port = 8080 + } +} +` + username, password := parseCliAuth(content) + if username != "" || password != "" { + t.Errorf("expected no credentials, got '%s'/'%s'", username, password) + } +} + +func TestParseCliAuth_IgnoresCredentialsOutsideCli(t *testing.T) { + // The agent block declares its own auth plugin config; that is the + // agent's server-side setting and must never be sent as CLI credentials. + content := `amends "formae:/Config.pkl" + +agent { + auth { + username = "agent-side" + password = "not-ours" + } +} + +cli { + api { + url = "http://agent.example.com" + port = 8080 + } +} +` + username, password := parseCliAuth(content) + if username != "" || password != "" { + t.Errorf("expected no credentials from the agent block, got '%s'/'%s'", username, password) + } +} + +const sampleAuthProfile = `amends "formae:/Config.pkl" + +import "plugins:/AuthBasic.pkl" as AuthBasic + +cli { + api { + url = "http://fleet.example.com" + port = 8080 + } + auth = new AuthBasic.CliConfig { + username = "formae" + password = "s3cr3t" + } +} +` + +func TestAgentCredentials_FromExplicitProfile(t *testing.T) { + dir := t.TempDir() + t.Setenv("FORMAE_CONFIG_DIR", dir) + writeProfile(t, dir, "fleet", sampleAuthProfile) + + username, password, err := AgentCredentials("fleet") + if err != nil { + t.Fatal(err) + } + if username != "formae" || password != "s3cr3t" { + t.Errorf("expected formae/s3cr3t, got %s/%s", username, password) + } +} + +func TestAgentCredentials_ProfileWithoutAuthYieldsNone(t *testing.T) { + dir := t.TempDir() + t.Setenv("FORMAE_CONFIG_DIR", dir) + writeProfile(t, dir, "prod", sampleProfile) + + username, password, err := AgentCredentials("prod") + if err != nil { + t.Fatal(err) + } + if username != "" || password != "" { + t.Errorf("expected no credentials, got %s/%s", username, password) + } +} + +func TestAgentCredentials_FollowsActiveProfile(t *testing.T) { + dir := t.TempDir() + t.Setenv("FORMAE_CONFIG_DIR", dir) + writeProfile(t, dir, "fleet", sampleAuthProfile) + if err := os.WriteFile(filepath.Join(dir, "active"), []byte("fleet\n"), 0o644); err != nil { + t.Fatal(err) + } + + username, password, err := AgentCredentials("") + if err != nil { + t.Fatal(err) + } + if username != "formae" || password != "s3cr3t" { + t.Errorf("expected the active profile's credentials, got %s/%s", username, password) + } +} + +func TestAgentCredentials_NoProfileConfiguredYieldsNone(t *testing.T) { + dir := t.TempDir() + t.Setenv("FORMAE_CONFIG_DIR", dir) // empty, no active + + username, password, err := AgentCredentials("") + if err != nil { + t.Fatalf("an unconfigured environment must not be an error: %v", err) + } + if username != "" || password != "" { + t.Errorf("expected no credentials, got %s/%s", username, password) + } +} diff --git a/internal/server/client.go b/internal/server/client.go index b572092..df3f92a 100644 --- a/internal/server/client.go +++ b/internal/server/client.go @@ -11,28 +11,45 @@ import ( "time" ) +// BasicAuth carries the HTTP basic credentials a profile declares for its +// agent. A nil *BasicAuth means the agent is unauthenticated, which is the +// case for every tailnet-only agent. +type BasicAuth struct { + Username string + Password string +} + // FormaeClient is a lightweight HTTP client for the formae agent REST API. type FormaeClient struct { endpoint string + creds *BasicAuth httpClient *http.Client } -func NewFormaeClient(endpoint string) *FormaeClient { +func NewFormaeClient(endpoint string, creds *BasicAuth) *FormaeClient { return &FormaeClient{ endpoint: endpoint, + creds: creds, httpClient: &http.Client{ Timeout: 30 * time.Second, }, } } -func (c *FormaeClient) get(path string, query url.Values) ([]byte, int, error) { - u := c.endpoint + path - if len(query) > 0 { - u += "?" + query.Encode() +// do applies the profile's credentials, if any, and performs the request. Every +// request the client makes goes through here, so an agent behind basic auth is +// reachable from all of them rather than only the ones someone remembered. +func (c *FormaeClient) do(req *http.Request) (*http.Response, error) { + if c.creds != nil { + req.SetBasicAuth(c.creds.Username, c.creds.Password) } + return c.httpClient.Do(req) +} - resp, err := c.httpClient.Get(u) +// doRead performs the request and reads the whole body, which is what every +// caller of get/post wants. +func (c *FormaeClient) doRead(req *http.Request) ([]byte, int, error) { + resp, err := c.do(req) if err != nil { return nil, 0, fmt.Errorf("request failed: %w", err) } @@ -46,24 +63,31 @@ func (c *FormaeClient) get(path string, query url.Values) ([]byte, int, error) { return body, resp.StatusCode, nil } -func (c *FormaeClient) post(path string, query url.Values) ([]byte, int, error) { +func (c *FormaeClient) get(path string, query url.Values) ([]byte, int, error) { u := c.endpoint + path if len(query) > 0 { u += "?" + query.Encode() } - resp, err := c.httpClient.Post(u, "application/json", nil) + req, err := http.NewRequest(http.MethodGet, u, nil) if err != nil { - return nil, 0, fmt.Errorf("request failed: %w", err) + return nil, 0, fmt.Errorf("failed to create request: %w", err) } - defer func() { _ = resp.Body.Close() }() + return c.doRead(req) +} - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, resp.StatusCode, fmt.Errorf("failed to read response: %w", err) +func (c *FormaeClient) post(path string, query url.Values) ([]byte, int, error) { + u := c.endpoint + path + if len(query) > 0 { + u += "?" + query.Encode() } - return body, resp.StatusCode, nil + req, err := http.NewRequest(http.MethodPost, u, nil) + if err != nil { + return nil, 0, fmt.Errorf("failed to create request: %w", err) + } + req.Header.Set("Content-Type", "application/json") + return c.doRead(req) } // ListResources queries the agent for resources matching the given query string. @@ -151,7 +175,7 @@ func (c *FormaeClient) GetCommandStatus(commandID string, clientID string) (json } req.Header.Set("Client-ID", clientID) - resp, err := c.httpClient.Do(req) + resp, err := c.do(req) if err != nil { return nil, fmt.Errorf("request failed: %w", err) } @@ -188,7 +212,7 @@ func (c *FormaeClient) ListCommands(query string, maxResults string, clientID st } req.Header.Set("Client-ID", clientID) - resp, err := c.httpClient.Do(req) + resp, err := c.do(req) if err != nil { return nil, fmt.Errorf("request failed: %w", err) } @@ -306,7 +330,7 @@ func (c *FormaeClient) CancelCommands(query string, clientID string) (json.RawMe } req.Header.Set("Client-ID", clientID) - resp, err := c.httpClient.Do(req) + resp, err := c.do(req) if err != nil { return nil, fmt.Errorf("request failed: %w", err) } @@ -432,7 +456,7 @@ func (c *FormaeClient) postMultipartWithHeaders(path string, query url.Values, f req.Header.Set(k, v) } - resp, err := c.httpClient.Do(req) + resp, err := c.do(req) if err != nil { return nil, 0, fmt.Errorf("request failed: %w", err) } diff --git a/internal/server/client_test.go b/internal/server/client_test.go index cca7872..620e5cb 100644 --- a/internal/server/client_test.go +++ b/internal/server/client_test.go @@ -9,7 +9,7 @@ import ( // newTestFormaeClient creates a FormaeClient pointed at the given httptest.Server. func newTestFormaeClient(srv *httptest.Server) *FormaeClient { - c := NewFormaeClient(srv.URL) + c := NewFormaeClient(srv.URL, nil) c.httpClient = srv.Client() return c } @@ -192,3 +192,64 @@ func TestDestroyByQueryRejectsOtherStatus(t *testing.T) { t.Fatal("DestroyByQuery: expected error for 500, got nil") } } + +func TestFormaeClient_SendsBasicAuthWhenCredentialsPresent(t *testing.T) { + var gotUser, gotPass string + var gotOK bool + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotUser, gotPass, gotOK = r.BasicAuth() + _, _ = w.Write([]byte(`{"Resources":[]}`)) + })) + defer srv.Close() + + c := NewFormaeClient(srv.URL, &BasicAuth{Username: "formae", Password: "s3cr3t"}) + c.httpClient = srv.Client() + + if _, err := c.ListResources(""); err != nil { + t.Fatalf("ListResources: %v", err) + } + if !gotOK { + t.Fatal("expected an Authorization header carrying basic credentials, got none") + } + if gotUser != "formae" || gotPass != "s3cr3t" { + t.Errorf("expected formae/s3cr3t, got %s/%s", gotUser, gotPass) + } +} + +func TestFormaeClient_SendsNoAuthHeaderWithoutCredentials(t *testing.T) { + var sawAuthHeader bool + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + sawAuthHeader = r.Header.Get("Authorization") != "" + _, _ = w.Write([]byte(`{"Resources":[]}`)) + })) + defer srv.Close() + + c := NewFormaeClient(srv.URL, nil) + c.httpClient = srv.Client() + + if _, err := c.ListResources(""); err != nil { + t.Fatalf("ListResources: %v", err) + } + if sawAuthHeader { + t.Error("expected no Authorization header when the profile carries no credentials") + } +} + +func TestFormaeClient_SendsBasicAuthOnPost(t *testing.T) { + var gotOK bool + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _, gotOK = r.BasicAuth() + _, _ = w.Write([]byte(`{}`)) + })) + defer srv.Close() + + c := NewFormaeClient(srv.URL, &BasicAuth{Username: "formae", Password: "s3cr3t"}) + c.httpClient = srv.Client() + + if _, _, err := c.post("/api/v1/anything", nil); err != nil { + t.Fatalf("post: %v", err) + } + if !gotOK { + t.Error("expected basic credentials on a POST as well as a GET") + } +} diff --git a/internal/server/server.go b/internal/server/server.go index 70b096e..209e04e 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -70,13 +70,21 @@ func (s *Server) clientFor(profileName string) (*FormaeClient, error) { return nil, err } } else if s.forcedEndpoint != "" { - return NewFormaeClient(s.forcedEndpoint), nil + return NewFormaeClient(s.forcedEndpoint, nil), nil } url, port, err := config.AgentEndpoint(profileName) if err != nil { return nil, err } - return NewFormaeClient(url + ":" + port), nil + username, password, err := config.AgentCredentials(profileName) + if err != nil { + return nil, err + } + var creds *BasicAuth + if username != "" { + creds = &BasicAuth{Username: username, Password: password} + } + return NewFormaeClient(url+":"+port, creds), nil } // Run starts the MCP server with the given transport.