diff --git a/README.md b/README.md index 10d987a262..3c45bfca30 100644 --- a/README.md +++ b/README.md @@ -311,6 +311,8 @@ Add one of the following JSON blocks to your IDE's MCP settings. See **[Local Server OAuth Login](docs/oauth-login.md)** for the native-binary flow (no fixed port needed), the headless/device-code fallback, GitHub Enterprise Server / `ghe.com`, and bringing your own OAuth or GitHub App. +**Running headless (CI, Kubernetes, background agents)?** The stdio server can authenticate as a **GitHub App installation** with no browser, device code, or elicitation — see **[GitHub App Server-to-Server Authentication](docs/github-app-auth.md)**. This injects a high-privilege credential alongside the agent, so read the security guidance there first; it is not recommended without an independent security review. + **Or authenticate with a Personal Access Token.** Set `GITHUB_PERSONAL_ACCESS_TOKEN` instead (it takes precedence over OAuth): ```json diff --git a/cmd/github-mcp-server/main.go b/cmd/github-mcp-server/main.go index 231b0cf2c3..7629889293 100644 --- a/cmd/github-mcp-server/main.go +++ b/cmd/github-mcp-server/main.go @@ -1,6 +1,7 @@ package main import ( + "context" "errors" "fmt" "os" @@ -9,10 +10,12 @@ import ( "github.com/github/github-mcp-server/internal/buildinfo" "github.com/github/github-mcp-server/internal/ghmcp" + "github.com/github/github-mcp-server/internal/githubapp" "github.com/github/github-mcp-server/internal/oauth" "github.com/github/github-mcp-server/pkg/github" ghhttp "github.com/github/github-mcp-server/pkg/http" ghoauth "github.com/github/github-mcp-server/pkg/http/oauth" + "github.com/github/github-mcp-server/pkg/utils" "github.com/spf13/cobra" "github.com/spf13/pflag" "github.com/spf13/viper" @@ -37,6 +40,17 @@ var ( Long: `Start a server that communicates via standard input/output streams using JSON-RPC messages.`, RunE: func(_ *cobra.Command, _ []string) error { token := viper.GetString("personal_access_token") + + // GitHub App server-to-server auth (non-interactive). It is detected + // when any app-* setting is present; a partial configuration yields a + // clear error from the loader/validator below rather than silently + // falling back to another mode. + appID := viper.GetString("app-id") + appInstallationID := viper.GetString("app-installation-id") + appPrivateKeyPath := viper.GetString("app-private-key-path") + appPrivateKeyInline := viper.GetString("app-private-key") + appAuthRequested := appID != "" || appInstallationID != "" || appPrivateKeyPath != "" || appPrivateKeyInline != "" + oauthClientID := viper.GetString("oauth-client-id") oauthClientSecret := viper.GetString("oauth-client-secret") // Fall back to the build-time baked-in client (official releases) when none is @@ -45,13 +59,20 @@ var ( // --oauth-client-id. Recognizing the host via NormalizeHost means an explicit // GITHUB_HOST=github.com (or api.github.com) still counts as the default and keeps // zero-config login working. The secret tracks the id, so an explicitly provided - // id with no secret never picks up the baked-in secret. - if oauthClientID == "" && oauth.NormalizeHost(viper.GetString("host")) == "https://github.com" { + // id with no secret never picks up the baked-in secret. App auth opts out of this + // default so configuring an app never accidentally enables OAuth login too. + if oauthClientID == "" && !appAuthRequested && oauth.NormalizeHost(viper.GetString("host")) == "https://github.com" { oauthClientID = buildinfo.OAuthClientID oauthClientSecret = buildinfo.OAuthClientSecret } - if token == "" && oauthClientID == "" { - return errors.New("authentication required: set GITHUB_PERSONAL_ACCESS_TOKEN, or pass --oauth-client-id to log in via OAuth") + if token == "" && !appAuthRequested && oauthClientID == "" { + return errors.New("authentication required: set GITHUB_PERSONAL_ACCESS_TOKEN, configure GitHub App auth (GITHUB_APP_ID, GITHUB_APP_INSTALLATION_ID and GITHUB_APP_PRIVATE_KEY_PATH), or pass --oauth-client-id to log in via OAuth") + } + if appAuthRequested && token != "" { + return errors.New("GitHub App authentication and GITHUB_PERSONAL_ACCESS_TOKEN are mutually exclusive: set only one") + } + if appAuthRequested && oauthClientID != "" { + return errors.New("GitHub App authentication and OAuth login (--oauth-client-id) are mutually exclusive: set only one") } // If you're wondering why we're not using viper.GetStringSlice("toolsets"), @@ -116,7 +137,8 @@ var ( // client. The requested scopes default to the full supported set // (which filters out no tools); an explicit, narrower --oauth-scopes // both narrows the grant and hides tools needing other scopes. - if token == "" { + // Skipped for GitHub App auth, which sources tokens non-interactively. + if token == "" && !appAuthRequested { scopes := ghoauth.SupportedScopes if viper.IsSet("oauth-scopes") { if err := viper.UnmarshalKey("oauth-scopes", &scopes); err != nil { @@ -134,6 +156,17 @@ var ( stdioServerConfig.OAuthScopes = scopes } + // GitHub App server-to-server auth: load and parse the private key, + // then resolve the REST base URL so the server can mint installation + // tokens for the configured host (github.com, GHES, or ghe.com). + if appAuthRequested { + appConfig, err := buildAppAuthConfig(appID, appInstallationID, appPrivateKeyPath, appPrivateKeyInline, viper.GetString("host")) + if err != nil { + return err + } + stdioServerConfig.AppAuth = appConfig + } + return ghmcp.RunStdioServer(stdioServerConfig) }, } @@ -230,6 +263,15 @@ func init() { stdioCmd.Flags().StringSlice("oauth-scopes", nil, "Comma-separated OAuth scopes to request; also filters tools to those scopes. Defaults to the full supported set") stdioCmd.Flags().Int("oauth-callback-port", 0, "Fixed local port for the OAuth callback server. Defaults to a random port; set a fixed port when mapping it through Docker") + // stdio-specific GitHub App (server-to-server) flags. Provide an app ID, + // installation ID, and private key to authenticate non-interactively — no + // browser, device code, or elicitation. Intended for headless deployments. + // The private key itself has no flag (only GITHUB_APP_PRIVATE_KEY): a flag + // would place the key in the process arguments. Prefer the key file path. + stdioCmd.Flags().String("app-id", "", "GitHub App ID or client ID, enabling non-interactive server-to-server authentication") + stdioCmd.Flags().String("app-installation-id", "", "GitHub App installation ID to mint installation access tokens for") + stdioCmd.Flags().String("app-private-key-path", "", "Path to the GitHub App private key (PEM). Preferred over GITHUB_APP_PRIVATE_KEY: keeps the key off the command line and out of the environment") + // HTTP-specific flags httpCmd.Flags().Int("port", 8082, "HTTP server port") httpCmd.Flags().String("listen-host", "", "Host the HTTP server binds to (e.g. 127.0.0.1). Empty binds to all interfaces.") @@ -256,6 +298,9 @@ func init() { _ = viper.BindPFlag("oauth-client-secret", stdioCmd.Flags().Lookup("oauth-client-secret")) _ = viper.BindPFlag("oauth-scopes", stdioCmd.Flags().Lookup("oauth-scopes")) _ = viper.BindPFlag("oauth-callback-port", stdioCmd.Flags().Lookup("oauth-callback-port")) + _ = viper.BindPFlag("app-id", stdioCmd.Flags().Lookup("app-id")) + _ = viper.BindPFlag("app-installation-id", stdioCmd.Flags().Lookup("app-installation-id")) + _ = viper.BindPFlag("app-private-key-path", stdioCmd.Flags().Lookup("app-private-key-path")) _ = viper.BindPFlag("port", httpCmd.Flags().Lookup("port")) _ = viper.BindPFlag("listen-host", httpCmd.Flags().Lookup("listen-host")) _ = viper.BindPFlag("base-url", httpCmd.Flags().Lookup("base-url")) @@ -281,6 +326,60 @@ func main() { } } +// buildAppAuthConfig assembles the GitHub App server-to-server configuration: +// it loads and parses the private key and resolves the REST base URL for the +// configured host. The private key is read from a file (preferred) or an inline +// environment value; a missing or partial configuration yields a clear error. +func buildAppAuthConfig(appID, installationID, keyPath, keyInline, host string) (*githubapp.Config, error) { + keyBytes, err := loadAppPrivateKey(keyPath, keyInline) + if err != nil { + return nil, err + } + privateKey, err := githubapp.ParsePrivateKey(keyBytes) + if err != nil { + return nil, fmt.Errorf("invalid GitHub App private key: %w", err) + } + + apiHost, err := utils.NewAPIHost(host) + if err != nil { + return nil, fmt.Errorf("failed to parse host for GitHub App authentication: %w", err) + } + restURL, err := apiHost.BaseRESTURL(context.Background()) + if err != nil { + return nil, fmt.Errorf("failed to resolve REST URL for GitHub App authentication: %w", err) + } + + cfg := &githubapp.Config{ + AppID: appID, + InstallationID: installationID, + PrivateKey: privateKey, + BaseRESTURL: restURL.String(), + } + if err := cfg.Validate(); err != nil { + return nil, err + } + return cfg, nil +} + +// loadAppPrivateKey returns the GitHub App private key bytes from a file path +// (preferred — it keeps the key off argv and out of the environment) or from an +// inline value. The inline form tolerates literal "\n" escapes so a PEM survives +// being carried in a single-line environment variable. +func loadAppPrivateKey(path, inline string) ([]byte, error) { + switch { + case path != "": + data, err := os.ReadFile(path) //#nosec G304 -- operator-supplied path to their own key + if err != nil { + return nil, fmt.Errorf("reading GitHub App private key file: %w", err) + } + return data, nil + case inline != "": + return []byte(strings.ReplaceAll(inline, `\n`, "\n")), nil + default: + return nil, errors.New("GitHub App authentication requires a private key: set GITHUB_APP_PRIVATE_KEY_PATH (preferred) or GITHUB_APP_PRIVATE_KEY") + } +} + func wordSepNormalizeFunc(_ *pflag.FlagSet, name string) pflag.NormalizedName { from := []string{"_"} to := "-" diff --git a/docs/github-app-auth.md b/docs/github-app-auth.md new file mode 100644 index 0000000000..ed9a3db11f --- /dev/null +++ b/docs/github-app-auth.md @@ -0,0 +1,261 @@ +# GitHub App Server-to-Server Authentication (stdio) + +The local (stdio) GitHub MCP Server can authenticate as a **GitHub App +installation** instead of as a user. This is a **server-to-server** (s2s) flow: +the server signs a short-lived JSON Web Token (JWT) with your app's private key, +exchanges it for an installation access token, and refreshes that token +automatically. There is **no browser, no device code, and no elicitation**, so +it works in fully non-interactive environments — CI, Kubernetes, and background +agents such as Copilot's cloud agent. + +> [!WARNING] +> **Read this before you enable it.** This mode was added by popular demand, but +> it is **dangerous** and is **not recommended without an independent security +> review** of your deployment and of this implementation. +> +> - It places a **long-lived, high-privilege credential** (your app's private +> key) in the same environment as an AI agent. Anyone or anything that can read +> that environment can mint tokens that act as your app. +> - Installation access tokens minted here can act across **every repository the +> app is installed on**, with the app's full set of permissions. +> - Exposing credentials to agents — and **especially in the cloud** — is +> inherently risky. Treat this as a break-glass capability and proceed with +> **extreme caution**. +> +> If an interactive login is at all possible for your use case, prefer +> [OAuth login](oauth-login.md) instead, which keeps no long-lived secret next to +> the agent. + +## Contents + +- [When to use this](#when-to-use-this) +- [Why stdio only](#why-stdio-only) +- [How it works](#how-it-works) +- [Prerequisites](#prerequisites) +- [Configuration reference](#configuration-reference) +- [Injecting the private key safely](#injecting-the-private-key-safely) +- [Quick start](#quick-start) +- [Kubernetes](#kubernetes) +- [GitHub Enterprise Server and ghe.com](#github-enterprise-server-and-ghecom) +- [Reducing the blast radius](#reducing-the-blast-radius) +- [Troubleshooting](#troubleshooting) + +## When to use this + +Use GitHub App s2s auth only when **all** of the following hold: + +- The server runs **non-interactively** (no human to complete a browser or + device flow). +- The workload should act as an **organization-managed identity** (the app), + not a single user's Personal Access Token (PAT). +- You have reviewed the security implications above and accept them. + +For everything else, prefer [OAuth login](oauth-login.md) or a +[PAT](https://github.com/settings/personal-access-tokens/new). + +## Why stdio only + +This mode is deliberately limited to the **stdio** server, where the server runs +as a subprocess of a single trusted client and the minted token never crosses +that process boundary. + +It is intentionally **not** available for the `http` server. An HTTP server that +authenticated with a server-wide app identity would let **any** client that can +reach its endpoint act as the app, with the app's full permissions — turning a +network-reachable port into ambient, unauthenticated access to your whole +installation. The `http` server therefore keeps requiring a per-request +`Authorization` token, so every caller's identity and permissions stay explicit. + +If you need a hosted, networked deployment, authenticate callers at the +client/proxy layer and pass per-request tokens; don't give the server a standing +identity. + +## How it works + +1. The server builds a JWT and signs it with your app's private key (RS256). The + JWT is valid for under 10 minutes (GitHub's maximum) and identifies your app. +2. It calls `POST /app/installations/{installation_id}/access_tokens` with that + JWT to obtain an **installation access token** (prefixed `ghs_`), which is + valid for up to one hour. +3. Every GitHub API call uses that token. The server refreshes it about five + minutes before it expires, so long-running sessions keep working without any + intervention. + +The private key is held **in memory only**; the server never writes it or the +minted tokens to disk. + +## Prerequisites + +1. **Register a GitHub App** and generate a **private key** (Settings → your + app → *Private keys* → *Generate a private key*). GitHub downloads a `.pem` + file in PKCS#1 or PKCS#8 format — both are accepted. +2. **Install the app** on the account/organization and grant it the **minimum** + permissions and **only the repositories** it needs (see + [Reducing the blast radius](#reducing-the-blast-radius)). +3. Note three values: + - the **App ID** (or the app's **client ID** — either works as the JWT issuer), + - the **installation ID** (visible in the installation's settings URL, or via + the [installations API](https://docs.github.com/en/rest/apps/apps#list-installations-for-the-authenticated-app)), + - the path to the **private key** `.pem`. + +## Configuration reference + +App auth is enabled when **any** of these `app-*` settings is present; a +partial configuration produces a clear startup error. Settings apply only to the +`stdio` command. + +| Flag | Environment variable | Description | +|------|----------------------|-------------| +| `--app-id` | `GITHUB_APP_ID` | GitHub App ID or client ID. Becomes the JWT issuer. | +| `--app-installation-id` | `GITHUB_APP_INSTALLATION_ID` | Installation ID whose token is minted. | +| `--app-private-key-path` | `GITHUB_APP_PRIVATE_KEY_PATH` | Path to the private key PEM file. **Preferred** way to supply the key. | +| _(no flag)_ | `GITHUB_APP_PRIVATE_KEY` | The PEM contents inline. Use only where a file can't be mounted. Literal `\n` sequences are accepted so the key can live in a single-line variable. | + +There is intentionally **no flag** for the private key contents: a flag would +place the key in the process's command line (`ps`, `/proc//cmdline`), where +other processes could read it. + +App auth is **mutually exclusive** with a PAT (`GITHUB_PERSONAL_ACCESS_TOKEN`) +and with OAuth login (`--oauth-client-id`). Configure exactly one. + +## Injecting the private key safely + +The private key is the most sensitive value in this flow. In order of +preference: + +1. **A mounted secret file** (recommended). Point `GITHUB_APP_PRIVATE_KEY_PATH` + at a file your platform mounts from its secret store — a Kubernetes secret + volume, a Docker secret, or a tmpfs file written by your secret manager. The + key never touches the command line or the process environment. +2. **An inline environment variable** (`GITHUB_APP_PRIVATE_KEY`). Acceptable + where files can't be mounted, but the key is then readable by anything that + can inspect the process environment. Avoid this in shared or cloud + environments. + +Never pass the key on the command line, never bake it into an image, and never +commit it to source control. + +## Quick start + +Native binary, key on disk: + +```bash +github-mcp-server stdio \ + --app-id 123456 \ + --app-installation-id 7891011 \ + --app-private-key-path /secrets/github-app.pem +``` + +Equivalently, with environment variables: + +```bash +export GITHUB_APP_ID=123456 +export GITHUB_APP_INSTALLATION_ID=7891011 +export GITHUB_APP_PRIVATE_KEY_PATH=/secrets/github-app.pem +github-mcp-server stdio +``` + +Docker, mounting the key as a read-only file (preferred over passing it inline): + +```bash +docker run -i --rm \ + -v /secrets/github-app.pem:/secrets/github-app.pem:ro \ + -e GITHUB_APP_ID=123456 \ + -e GITHUB_APP_INSTALLATION_ID=7891011 \ + -e GITHUB_APP_PRIVATE_KEY_PATH=/secrets/github-app.pem \ + ghcr.io/github/github-mcp-server +``` + +## Kubernetes + +Store the key in a `Secret` and mount it as a file; pass the IDs as environment +variables. This keeps the key off the command line and out of the container's +environment. + +```yaml +apiVersion: v1 +kind: Secret +metadata: + name: github-app +type: Opaque +stringData: + private-key.pem: | + -----BEGIN RSA PRIVATE KEY----- + ... + -----END RSA PRIVATE KEY----- +--- +apiVersion: v1 +kind: Pod +metadata: + name: github-mcp-server +spec: + containers: + - name: github-mcp-server + image: ghcr.io/github/github-mcp-server + stdin: true + env: + - name: GITHUB_APP_ID + value: "123456" + - name: GITHUB_APP_INSTALLATION_ID + value: "7891011" + - name: GITHUB_APP_PRIVATE_KEY_PATH + value: /secrets/github-app/private-key.pem + volumeMounts: + - name: github-app + mountPath: /secrets/github-app + readOnly: true + volumes: + - name: github-app + secret: + secretName: github-app +``` + +## GitHub Enterprise Server and ghe.com + +Set the host with `--gh-host` / `GITHUB_HOST`; the server derives the correct +installation token endpoint from it, so tokens are minted against your instance +rather than github.com. Register the app and generate its key on that same host. + +```bash +github-mcp-server stdio \ + --gh-host https://github.example.com \ + --app-id 123456 \ + --app-installation-id 7891011 \ + --app-private-key-path /secrets/github-app.pem +``` + +- For GitHub Enterprise Server, prefix the host with `https://`. +- For `ghe.com`, use `https://YOURSUBDOMAIN.ghe.com`. + +## Reducing the blast radius + +Because the minted token can act across the whole installation, minimize what it +can do: + +- **Grant least privilege.** Enable only the app permissions the workload needs, + and prefer read-only where possible. +- **Scope the installation to specific repositories** rather than *All + repositories*. +- **Rotate the private key** periodically and immediately if it may have been + exposed (Settings → your app → *Private keys*). +- **Isolate the runtime.** Run the server where only trusted code shares its + process environment and mounted secrets. +- **Combine with `--read-only` and toolset/scoping flags** to further narrow + what the agent can invoke. See the + [Server Configuration Guide](server-configuration.md). + +## Troubleshooting + +- **`GitHub App authentication requires a private key`** — you set some `app-*` + values but no key. Set `GITHUB_APP_PRIVATE_KEY_PATH` (preferred) or + `GITHUB_APP_PRIVATE_KEY`. +- **`invalid GitHub App private key`** — the PEM could not be parsed. Ensure it + is the app's RSA private key in PKCS#1 or PKCS#8 form and was not truncated + (when inline, encode newlines as literal `\n`). +- **`installation token request failed: 401`** — usually a clock-skew problem or + the wrong App ID/key pairing. Check the host clock and that the key belongs to + the configured app. +- **`installation token request failed: 404`** — the installation ID is wrong, + or the app is not installed where you think. Re-check the installation ID. +- **`... and GITHUB_PERSONAL_ACCESS_TOKEN are mutually exclusive`** — a PAT is + also set in the environment. Unset it; choose exactly one auth mode. diff --git a/docs/oauth-login.md b/docs/oauth-login.md index 16c5dab67e..31a0c90dce 100644 --- a/docs/oauth-login.md +++ b/docs/oauth-login.md @@ -15,6 +15,13 @@ pass `--oauth-client-id` (see [Bring your own app](#bring-your-own-app)). > `http` command have their own authentication; see > [Remote Server](remote-server.md). +> **Running non-interactively?** OAuth still needs a human to complete the flow +> once. For fully headless deployments (CI, Kubernetes, background agents), +> authenticate as a GitHub App installation instead — see +> [GitHub App Server-to-Server Authentication](github-app-auth.md). Note the +> security warnings there: it keeps a high-privilege credential next to the +> agent and is not recommended without an independent security review. + ## Contents - [How it works](#how-it-works) diff --git a/go.mod b/go.mod index 358a271a7a..b11455139d 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,7 @@ require ( github.com/josephburnett/jd/v2 v2.5.0 github.com/lithammer/fuzzysearch v1.1.8 github.com/microcosm-cc/bluemonday v1.0.27 - github.com/modelcontextprotocol/go-sdk v1.7.0-pre.1 + github.com/modelcontextprotocol/go-sdk v1.7.0-pre.2 github.com/muesli/cache2go v0.0.0-20221011235721-518229cd8021 github.com/shurcooL/githubv4 v0.0.0-20240727222349-48295856cce7 github.com/shurcooL/graphql v0.0.0-20230722043721-ed46e5a46466 diff --git a/go.sum b/go.sum index f3f23b549a..64e5b47839 100644 --- a/go.sum +++ b/go.sum @@ -39,8 +39,8 @@ github.com/lithammer/fuzzysearch v1.1.8 h1:/HIuJnjHuXS8bKaiTMeeDlW2/AyIWk2brx1V8 github.com/lithammer/fuzzysearch v1.1.8/go.mod h1:IdqeyBClc3FFqSzYq/MXESsS4S0FsZ5ajtkr5xPLts4= github.com/microcosm-cc/bluemonday v1.0.27 h1:MpEUotklkwCSLeH+Qdx1VJgNqLlpY2KXwXFM08ygZfk= github.com/microcosm-cc/bluemonday v1.0.27/go.mod h1:jFi9vgW+H7c3V0lb6nR74Ib/DIB5OBs92Dimizgw2cA= -github.com/modelcontextprotocol/go-sdk v1.7.0-pre.1 h1:GlMIJyMHFX76bBSQuBCLXZ7pB9cGh4VBS6O5wGd0tgI= -github.com/modelcontextprotocol/go-sdk v1.7.0-pre.1/go.mod h1:dL7u98E/zjJTGzEq+j30jQ8K2k1mb6LeAH4inEcSGts= +github.com/modelcontextprotocol/go-sdk v1.7.0-pre.2 h1:3JwUps1pdSpXYndBMGO9SMca6CSkP9AKnOaKAkSSGHc= +github.com/modelcontextprotocol/go-sdk v1.7.0-pre.2/go.mod h1:dL7u98E/zjJTGzEq+j30jQ8K2k1mb6LeAH4inEcSGts= github.com/muesli/cache2go v0.0.0-20221011235721-518229cd8021 h1:31Y+Yu373ymebRdJN1cWLLooHH8xAr0MhKTEJGV/87g= github.com/muesli/cache2go v0.0.0-20221011235721-518229cd8021/go.mod h1:WERUkUryfUWlrHnFSO/BEUZ+7Ns8aZy7iVOGewxKzcc= github.com/pelletier/go-toml/v2 v2.2.4 h1:mye9XuhQ6gvn5h28+VilKrrPoQVanw5PMw/TB0t5Ec4= diff --git a/internal/ghmcp/oauth.go b/internal/ghmcp/oauth.go index abc6d3d11c..35e48f5bbc 100644 --- a/internal/ghmcp/oauth.go +++ b/internal/ghmcp/oauth.go @@ -3,10 +3,13 @@ package ghmcp import ( "context" "crypto/rand" + "errors" "fmt" "log/slog" + "strings" "github.com/github/github-mcp-server/internal/oauth" + "github.com/github/github-mcp-server/pkg/inventory" "github.com/modelcontextprotocol/go-sdk/mcp" ) @@ -91,41 +94,187 @@ func (p *sessionPrompter) PromptForm(ctx context.Context, prompt oauth.Prompt) e type oauthAuthenticator interface { HasToken() bool Authenticate(ctx context.Context, prompter oauth.Prompter) (*oauth.Outcome, error) + AwaitToken(ctx context.Context, flowID string) (*oauth.Outcome, error) + Cancel(flowID string) bool } -// createOAuthMiddleware returns receiving middleware that authorizes the session -// lazily, on the first tool call. Authorization is deferred until here (rather -// than at startup) because the prompts depend on an initialized session whose -// elicitation capabilities are known. +// oauthElicitIDPrefix identifies authorization responses in the multi-round-trip +// InputResponses map. The suffix is the manager's per-flow ID, which prevents a +// delayed response from an older prompt from affecting a newer flow. +const oauthElicitIDPrefix = "github_authorization:" + +// protocolVersionNoServerElicitation is the first MCP protocol version that +// forbids server-initiated JSON-RPC requests (SEP-2322): from this version on +// the server may not send elicitation/create while serving a request and must +// instead return an InputRequests map from the tool call (multi round-trip +// requests). It mirrors the go-sdk's internal constant of the same value, which +// the SDK does not export. +const protocolVersionNoServerElicitation = "2026-07-28" + +// serverMayInitiateElicitation reports whether the server is permitted to send +// elicitation requests to the client itself, which the spec allows only before +// protocol version 2026-07-28. A nil or un-negotiated session (only reached in +// unit tests; a real tools/call is always initialized) is treated as legacy. +func serverMayInitiateElicitation(ss *mcp.ServerSession) bool { + if ss == nil { + return true + } + params := ss.InitializeParams() + return params == nil || params.ProtocolVersion < protocolVersionNoServerElicitation +} + +// createOAuthToolMiddleware returns tool-handler middleware that authorizes the +// session lazily, on the first tool call. It runs inside the SDK's +// Server.callTool handler so results returned here still receive SDK +// finalization, including resultType: "input_required" for multi-round-trip +// responses. // // When a token is already available the call proceeds untouched. Otherwise the -// flow runs: secure channels (browser, URL elicitation) block until the token -// arrives and then the call proceeds; the last-resort channel returns the -// instruction to the user as a tool result and asks them to retry. -func createOAuthMiddleware(mgr oauthAuthenticator, logger *slog.Logger) func(next mcp.MethodHandler) mcp.MethodHandler { - return func(next mcp.MethodHandler) mcp.MethodHandler { - return func(ctx context.Context, method string, request mcp.Request) (mcp.Result, error) { - if method != "tools/call" || mgr.HasToken() { - return next(ctx, method, request) +// authorization flow runs, presenting its prompt over whichever channel the +// negotiated protocol allows: on protocol versions before 2026-07-28 the server +// elicits directly; from 2026-07-28 on, where server-initiated requests are +// forbidden (SEP-2322), it uses multi-round-trip elicitation returned from the +// tool call. Either way the last-resort channel returns the instruction as a +// tool result and asks the user to retry. +func createOAuthToolMiddleware(mgr oauthAuthenticator, logger *slog.Logger) inventory.ToolHandlerMiddleware { + return func(next mcp.ToolHandler) mcp.ToolHandler { + return func(ctx context.Context, req *mcp.CallToolRequest) (*mcp.CallToolResult, error) { + if !serverMayInitiateElicitation(req.Session) { + if flowID, response, ok := authorizationElicitResponse(req.Params.InputResponses); ok { + return resumeMultiRoundTripAuthorization(ctx, mgr, next, req, flowID, response, logger) + } } - callReq, ok := request.(*mcp.CallToolRequest) - if !ok { - return next(ctx, method, request) + if mgr.HasToken() { + return next(ctx, req) } - - outcome, err := mgr.Authenticate(ctx, &sessionPrompter{session: callReq.Session}) - if err != nil { - return nil, fmt.Errorf("github authorization failed: %w", err) + if serverMayInitiateElicitation(req.Session) { + return authorizeViaServerElicitation(ctx, mgr, next, req, logger) } - if outcome != nil && outcome.UserAction != nil { - logger.Info("surfacing github authorization instructions to user") - return &mcp.CallToolResult{ - Content: []mcp.Content{&mcp.TextContent{Text: outcome.UserAction.Message}}, - }, nil - } - return next(ctx, method, request) + return startMultiRoundTripAuthorization(ctx, mgr, next, req, logger) + } + } +} + +// authorizeViaServerElicitation drives authorization on legacy protocol versions +// (before 2026-07-28), where the server may present the prompt itself via +// server-initiated elicitation. It blocks until the token arrives, then proceeds. +func authorizeViaServerElicitation(ctx context.Context, mgr oauthAuthenticator, next mcp.ToolHandler, req *mcp.CallToolRequest, logger *slog.Logger) (*mcp.CallToolResult, error) { + outcome, err := mgr.Authenticate(ctx, &sessionPrompter{session: req.Session}) + if err != nil { + return nil, fmt.Errorf("github authorization failed: %w", err) + } + if outcome != nil && outcome.UserAction != nil { + logger.Info("surfacing github authorization instructions to user") + return &mcp.CallToolResult{ + Content: []mcp.Content{&mcp.TextContent{Text: outcome.UserAction.Message}}, + }, nil + } + return next(ctx, req) +} + +// authorizationElicitResponse finds the authorization response and extracts the +// flow ID encoded in its input-request key. +func authorizationElicitResponse(responses mcp.InputResponseMap) (string, *mcp.ElicitResult, bool) { + for id, response := range responses { + flowID, ok := strings.CutPrefix(id, oauthElicitIDPrefix) + if !ok || flowID == "" { + continue + } + result, _ := response.(*mcp.ElicitResult) + return flowID, result, true + } + return "", nil, false +} + +// startMultiRoundTripAuthorization starts authorization on protocol version +// 2026-07-28 or later. Server-initiated requests are forbidden there (SEP-2322), +// so the prompt is returned as an elicitation input request for the client to +// fulfill and retry. +func startMultiRoundTripAuthorization(ctx context.Context, mgr oauthAuthenticator, next mcp.ToolHandler, req *mcp.CallToolRequest, logger *slog.Logger) (*mcp.CallToolResult, error) { + outcome, err := mgr.Authenticate(ctx, nil) + if err != nil { + return nil, fmt.Errorf("github authorization failed: %w", err) + } + if outcome == nil || outcome.UserAction == nil { + // Already authorized (e.g. the server opened a browser and the flow + // completed); proceed. + return next(ctx, req) + } + + elicit := authorizationElicitParams(outcome.UserAction, &sessionPrompter{session: req.Session}) + if elicit == nil || outcome.FlowID == "" { + // The client cannot present an elicitation (no capability, or no URL to + // show), or the flow cannot be correlated; fall back to returning the + // instructions as a tool result. + logger.Info("surfacing github authorization instructions to user") + return &mcp.CallToolResult{ + Content: []mcp.Content{&mcp.TextContent{Text: outcome.UserAction.Message}}, + }, nil + } + logger.Info("requesting github authorization via elicitation") + return &mcp.CallToolResult{ + InputRequests: mcp.InputRequestMap{oauthElicitIDPrefix + outcome.FlowID: elicit}, + }, nil +} + +// resumeMultiRoundTripAuthorization handles the client's retry after it +// fulfilled the authorization elicitation. +func resumeMultiRoundTripAuthorization(ctx context.Context, mgr oauthAuthenticator, next mcp.ToolHandler, req *mcp.CallToolRequest, flowID string, response *mcp.ElicitResult, logger *slog.Logger) (*mcp.CallToolResult, error) { + if response == nil || response.Action != "accept" { + if !mgr.Cancel(flowID) { + return expiredAuthorizationResult(), nil } + return &mcp.CallToolResult{ + Content: []mcp.Content{&mcp.TextContent{Text: "GitHub authorization was declined. Retry when you're ready to authorize."}}, + }, nil + } + + outcome, err := mgr.AwaitToken(ctx, flowID) + if errors.Is(err, oauth.ErrStaleAuthorizationFlow) { + return expiredAuthorizationResult(), nil + } + if err != nil { + return nil, fmt.Errorf("github authorization failed: %w", err) + } + if outcome != nil && outcome.UserAction != nil { + // The user acknowledged the prompt but has not finished authorizing; + // surface the instructions so they can complete it and retry. + logger.Info("surfacing github authorization instructions to user") + return &mcp.CallToolResult{ + Content: []mcp.Content{&mcp.TextContent{Text: outcome.UserAction.Message}}, + }, nil + } + return next(ctx, req) +} + +func expiredAuthorizationResult() *mcp.CallToolResult { + return &mcp.CallToolResult{ + Content: []mcp.Content{&mcp.TextContent{Text: "This GitHub authorization prompt has expired. Retry the request to authorize again."}}, + } +} + +// authorizationElicitParams builds the elicitation that presents the +// authorization instructions to the user. It mirrors sessionPrompter's channel +// selection: URL-mode when the client supports it, otherwise form-mode. It +// returns nil when the client advertised no elicitation capability or there is +// no authorization URL to show, so the caller falls back to a tool-result +// message. +func authorizationElicitParams(ua *oauth.UserAction, p *sessionPrompter) *mcp.ElicitParams { + if ua.URL == "" { + return nil + } + message := "Authorize the GitHub MCP Server to continue." + if ua.UserCode != "" { + message = fmt.Sprintf("Enter code %s to authorize the GitHub MCP Server.", ua.UserCode) + } + switch { + case p.CanPromptURL(): + return &mcp.ElicitParams{Mode: "url", Message: message, URL: ua.URL, ElicitationID: rand.Text()} + case p.CanPromptForm(): + return &mcp.ElicitParams{Mode: "form", Message: ua.Message} + default: + return nil } } diff --git a/internal/ghmcp/oauth_test.go b/internal/ghmcp/oauth_test.go index 732d080e40..46c62d1156 100644 --- a/internal/ghmcp/oauth_test.go +++ b/internal/ghmcp/oauth_test.go @@ -9,10 +9,12 @@ import ( "net/http/httptest" "testing" + "github.com/github/github-mcp-server/internal/githubapp" "github.com/github/github-mcp-server/internal/oauth" "github.com/github/github-mcp-server/pkg/github" "github.com/github/github-mcp-server/pkg/http/headers" "github.com/github/github-mcp-server/pkg/utils" + "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -140,57 +142,49 @@ func TestSessionPrompterCapabilities(t *testing.T) { } } -func TestSessionPrompterPromptActions(t *testing.T) { +// TestSessionPrompterModernProtocolUnavailable verifies that on protocol version +// 2026-07-28 and later — the default negotiated by current clients — the server +// may not initiate elicitation (SEP-2322), so PromptURL and PromptForm report +// the prompt as undeliverable. This is what routes authorization to the +// multi-round-trip path instead (see authorizeViaMultiRoundTrip). +func TestSessionPrompterModernProtocolUnavailable(t *testing.T) { t.Parallel() - tests := []struct { - name string - action string - wantDecline bool - }{ - {name: "accept", action: "accept", wantDecline: false}, - {name: "decline", action: "decline", wantDecline: true}, - {name: "cancel", action: "cancel", wantDecline: true}, - } - caps := &mcp.ClientCapabilities{Elicitation: &mcp.ElicitationCapabilities{ URL: &mcp.URLElicitationCapabilities{}, Form: &mcp.FormElicitationCapabilities{}, }} - for _, tc := range tests { - // URL and form modes share the accept/decline mapping; cover both. - for _, mode := range []string{"url", "form"} { - t.Run(tc.name+"/"+mode, func(t *testing.T) { - t.Parallel() - - handler := func(_ context.Context, _ *mcp.ElicitRequest) (*mcp.ElicitResult, error) { - return &mcp.ElicitResult{Action: tc.action}, nil - } + // The handler should never be reached: the SDK blocks the server-initiated + // request before it leaves the server. + handler := func(_ context.Context, _ *mcp.ElicitRequest) (*mcp.ElicitResult, error) { + return &mcp.ElicitResult{Action: "accept"}, nil + } - got := runProbe(t, caps, handler, func(ctx context.Context, p *sessionPrompter) string { - var err error - if mode == "url" { - err = p.PromptURL(ctx, oauth.Prompt{Message: "msg", URL: "https://example.com/auth"}) - } else { - err = p.PromptForm(ctx, oauth.Prompt{Message: "msg"}) - } - if err == nil { - return "ok" - } - if err == oauth.ErrPromptDeclined { - return "declined" - } - return "error: " + err.Error() - }) + for _, mode := range []string{"url", "form"} { + t.Run(mode, func(t *testing.T) { + t.Parallel() - if tc.wantDecline { - assert.Equal(t, "declined", got) + got := runProbe(t, caps, handler, func(ctx context.Context, p *sessionPrompter) string { + var err error + if mode == "url" { + err = p.PromptURL(ctx, oauth.Prompt{Message: "msg", URL: "https://example.com/auth"}) } else { - assert.Equal(t, "ok", got) + err = p.PromptForm(ctx, oauth.Prompt{Message: "msg"}) + } + switch { + case err == nil: + return "ok" + case errors.Is(err, oauth.ErrPromptUnavailable): + return "unavailable" + default: + return "error: " + err.Error() } }) - } + + assert.Equal(t, "unavailable", got, + "server-initiated elicitation must be reported undeliverable on protocol 2026-07-28+") + }) } } @@ -247,6 +241,18 @@ type fakeAuthenticator struct { err error authCalls int lastPrompter oauth.Prompter + + // awaitOutcome/awaitErr are returned by AwaitToken; tokenAfterAwait flips + // HasToken to true once AwaitToken is called, simulating a flow that + // acquires the token while the user acts on the elicitation. + awaitOutcome *oauth.Outcome + awaitErr error + tokenAfterAwait bool + awaitCalls int + cancelCalls int + cancelResult bool + lastAwaitFlowID string + lastCancelFlowID string } func (f *fakeAuthenticator) HasToken() bool { return f.hasToken } @@ -257,34 +263,38 @@ func (f *fakeAuthenticator) Authenticate(_ context.Context, prompter oauth.Promp return f.outcome, f.err } -func TestCreateOAuthMiddleware(t *testing.T) { +func (f *fakeAuthenticator) AwaitToken(_ context.Context, flowID string) (*oauth.Outcome, error) { + f.awaitCalls++ + f.lastAwaitFlowID = flowID + if f.tokenAfterAwait { + f.hasToken = true + } + return f.awaitOutcome, f.awaitErr +} + +func (f *fakeAuthenticator) Cancel(flowID string) bool { + f.cancelCalls++ + f.lastCancelFlowID = flowID + return f.cancelResult +} + +func TestCreateOAuthToolMiddleware(t *testing.T) { t.Parallel() const nextText = "handler-ran" - newNext := func(called *bool) mcp.MethodHandler { - return func(_ context.Context, _ string, _ mcp.Request) (mcp.Result, error) { + newNext := func(called *bool) mcp.ToolHandler { + return func(_ context.Context, _ *mcp.CallToolRequest) (*mcp.CallToolResult, error) { *called = true return &mcp.CallToolResult{Content: []mcp.Content{&mcp.TextContent{Text: nextText}}}, nil } } - t.Run("non tool call passes through without authenticating", func(t *testing.T) { - t.Parallel() - fake := &fakeAuthenticator{hasToken: false} - var called bool - mw := createOAuthMiddleware(fake, discardLogger()) - _, err := mw(newNext(&called))(context.Background(), "initialize", &mcp.InitializeRequest{}) - require.NoError(t, err) - assert.True(t, called, "next should run") - assert.Zero(t, fake.authCalls, "authentication must not run for non tool calls") - }) - t.Run("existing token short circuits authentication", func(t *testing.T) { t.Parallel() fake := &fakeAuthenticator{hasToken: true} var called bool - mw := createOAuthMiddleware(fake, discardLogger()) - _, err := mw(newNext(&called))(context.Background(), "tools/call", &mcp.CallToolRequest{}) + mw := createOAuthToolMiddleware(fake, discardLogger()) + _, err := mw(newNext(&called))(context.Background(), &mcp.CallToolRequest{}) require.NoError(t, err) assert.True(t, called, "next should run") assert.Zero(t, fake.authCalls, "authentication must be skipped when a token already exists") @@ -294,15 +304,13 @@ func TestCreateOAuthMiddleware(t *testing.T) { t.Parallel() fake := &fakeAuthenticator{hasToken: false, outcome: nil, err: nil} var called bool - mw := createOAuthMiddleware(fake, discardLogger()) - res, err := mw(newNext(&called))(context.Background(), "tools/call", &mcp.CallToolRequest{}) + mw := createOAuthToolMiddleware(fake, discardLogger()) + res, err := mw(newNext(&called))(context.Background(), &mcp.CallToolRequest{}) require.NoError(t, err) assert.Equal(t, 1, fake.authCalls) assert.True(t, called, "next should run once authorized") - callRes, ok := res.(*mcp.CallToolResult) - require.True(t, ok) - require.Len(t, callRes.Content, 1) - assert.Equal(t, nextText, callRes.Content[0].(*mcp.TextContent).Text) + require.Len(t, res.Content, 1) + assert.Equal(t, nextText, res.Content[0].(*mcp.TextContent).Text) }) t.Run("pending user action is surfaced as a tool result", func(t *testing.T) { @@ -310,42 +318,261 @@ func TestCreateOAuthMiddleware(t *testing.T) { const message = "Open https://example.com/auth to authorize, then retry." fake := &fakeAuthenticator{hasToken: false, outcome: &oauth.Outcome{UserAction: &oauth.UserAction{Message: message}}} var called bool - mw := createOAuthMiddleware(fake, discardLogger()) - res, err := mw(newNext(&called))(context.Background(), "tools/call", &mcp.CallToolRequest{}) + mw := createOAuthToolMiddleware(fake, discardLogger()) + res, err := mw(newNext(&called))(context.Background(), &mcp.CallToolRequest{}) require.NoError(t, err) assert.False(t, called, "next must not run while the user still needs to authorize") - callRes, ok := res.(*mcp.CallToolResult) - require.True(t, ok) - require.Len(t, callRes.Content, 1) - assert.Equal(t, message, callRes.Content[0].(*mcp.TextContent).Text) + require.Len(t, res.Content, 1) + assert.Equal(t, message, res.Content[0].(*mcp.TextContent).Text) }) t.Run("authentication error is returned", func(t *testing.T) { t.Parallel() fake := &fakeAuthenticator{hasToken: false, err: assert.AnError} var called bool - mw := createOAuthMiddleware(fake, discardLogger()) - _, err := mw(newNext(&called))(context.Background(), "tools/call", &mcp.CallToolRequest{}) + mw := createOAuthToolMiddleware(fake, discardLogger()) + _, err := mw(newNext(&called))(context.Background(), &mcp.CallToolRequest{}) require.Error(t, err) assert.ErrorIs(t, err, assert.AnError) assert.False(t, called, "next must not run when authentication fails") }) } -// TestRunStdioServerRejectsTokenAndOAuth verifies the mutually-exclusive guard: -// supplying both a static token and an OAuth manager is rejected before the -// server starts, rather than silently preferring one for auth and the other for -// scope filtering. -func TestRunStdioServerRejectsTokenAndOAuth(t *testing.T) { +// runOAuthMiddlewareCall stands up an in-memory client/server pair with the +// OAuth middleware installed ahead of a probe tool, then calls the tool from a +// default (protocol 2026-07-28) client — driving the multi-round-trip +// authorization path. It returns the final tool-result text and whether the +// probe tool ultimately ran. +func runOAuthMiddlewareCall( + t *testing.T, + fake *fakeAuthenticator, + clientCaps *mcp.ClientCapabilities, + elicitationHandler func(context.Context, *mcp.ElicitRequest) (*mcp.ElicitResult, error), +) (string, bool) { + t.Helper() + + server := mcp.NewServer(&mcp.Implementation{Name: "test-server", Version: "v0.0.1"}, nil) + var toolRan bool + handler := createOAuthToolMiddleware(fake, discardLogger())(func(_ context.Context, _ *mcp.CallToolRequest) (*mcp.CallToolResult, error) { + toolRan = true + return &mcp.CallToolResult{Content: []mcp.Content{&mcp.TextContent{Text: "tool-ran"}}}, nil + }) + server.AddTool(&mcp.Tool{ + Name: probeToolName, + InputSchema: &jsonschema.Schema{Type: "object"}, + }, handler) + + st, ct := mcp.NewInMemoryTransports() + + ss, err := server.Connect(context.Background(), st, nil) + require.NoError(t, err) + t.Cleanup(func() { _ = ss.Close() }) + + client := mcp.NewClient(&mcp.Implementation{Name: "test-client", Version: "v0.0.1"}, &mcp.ClientOptions{ + Capabilities: clientCaps, + ElicitationHandler: elicitationHandler, + }) + cs, err := client.Connect(context.Background(), ct, nil) + require.NoError(t, err) + t.Cleanup(func() { _ = cs.Close() }) + + res, err := cs.CallTool(context.Background(), &mcp.CallToolParams{Name: probeToolName}) + require.NoError(t, err) + require.Len(t, res.Content, 1) + text, ok := res.Content[0].(*mcp.TextContent) + require.True(t, ok, "tool result should be text content") + return text.Text, toolRan +} + +// TestOAuthMiddlewareMultiRoundTrip exercises the protocol-2026-07-28 path, where +// server-initiated elicitation is forbidden and authorization must be presented +// as a multi-round-trip input request that the client fulfills and retries. +func TestOAuthMiddlewareMultiRoundTrip(t *testing.T) { t.Parallel() - mgr := oauth.NewManager(oauth.NewGitHubConfig("client-id", "", nil, "", 0), discardLogger()) - err := RunStdioServer(StdioServerConfig{ - Token: "ghp_static", - OAuthManager: mgr, + urlCaps := &mcp.ClientCapabilities{Elicitation: &mcp.ElicitationCapabilities{URL: &mcp.URLElicitationCapabilities{}}} + + t.Run("accepted elicitation authorizes and proceeds", func(t *testing.T) { + t.Parallel() + fake := &fakeAuthenticator{ + outcome: &oauth.Outcome{UserAction: &oauth.UserAction{URL: "https://example.com/auth", Message: "manual"}, FlowID: "flow-1"}, + tokenAfterAwait: true, + } + var elicited int + accept := func(_ context.Context, _ *mcp.ElicitRequest) (*mcp.ElicitResult, error) { + elicited++ + return &mcp.ElicitResult{Action: "accept"}, nil + } + + text, toolRan := runOAuthMiddlewareCall(t, fake, urlCaps, accept) + + assert.Equal(t, "tool-ran", text, "the tool should run once authorization completes") + assert.True(t, toolRan) + assert.Equal(t, 1, elicited, "the client should be asked to authorize exactly once") + assert.Equal(t, 1, fake.awaitCalls, "the middleware should await the token on retry") + assert.Equal(t, "flow-1", fake.lastAwaitFlowID) + assert.Zero(t, fake.cancelCalls) + assert.Nil(t, fake.lastPrompter, "the manager must not be given a prompter on this protocol") + }) + + t.Run("declined elicitation cancels and does not run the tool", func(t *testing.T) { + t.Parallel() + fake := &fakeAuthenticator{ + outcome: &oauth.Outcome{UserAction: &oauth.UserAction{URL: "https://example.com/auth", Message: "manual"}, FlowID: "flow-1"}, + cancelResult: true, + } + decline := func(_ context.Context, _ *mcp.ElicitRequest) (*mcp.ElicitResult, error) { + return &mcp.ElicitResult{Action: "decline"}, nil + } + + text, toolRan := runOAuthMiddlewareCall(t, fake, urlCaps, decline) + + assert.False(t, toolRan, "the tool must not run when authorization is declined") + assert.Contains(t, text, "declined") + assert.Equal(t, 1, fake.cancelCalls, "a decline should cancel the in-flight flow") + assert.Equal(t, "flow-1", fake.lastCancelFlowID) + assert.Zero(t, fake.awaitCalls) + }) + + t.Run("stale decline does not cancel the current flow", func(t *testing.T) { + t.Parallel() + fake := &fakeAuthenticator{ + outcome: &oauth.Outcome{UserAction: &oauth.UserAction{URL: "https://example.com/auth", Message: "manual"}, FlowID: "old-flow"}, + } + decline := func(_ context.Context, _ *mcp.ElicitRequest) (*mcp.ElicitResult, error) { + return &mcp.ElicitResult{Action: "decline"}, nil + } + + text, toolRan := runOAuthMiddlewareCall(t, fake, urlCaps, decline) + + assert.False(t, toolRan) + assert.Contains(t, text, "expired") + assert.Equal(t, "old-flow", fake.lastCancelFlowID) + assert.Zero(t, fake.awaitCalls) }) - require.Error(t, err) - assert.Contains(t, err.Error(), "mutually exclusive") + + t.Run("form-only client receives actionable instructions", func(t *testing.T) { + t.Parallel() + const ( + authURL = "https://example.com/auth" + message = "Open https://example.com/auth and enter code ABCD-1234." + ) + fake := &fakeAuthenticator{ + outcome: &oauth.Outcome{ + UserAction: &oauth.UserAction{URL: authURL, UserCode: "ABCD-1234", Message: message}, + FlowID: "flow-1", + }, + tokenAfterAwait: true, + } + var elicited *mcp.ElicitParams + accept := func(_ context.Context, req *mcp.ElicitRequest) (*mcp.ElicitResult, error) { + elicited = req.Params + return &mcp.ElicitResult{Action: "accept"}, nil + } + formCaps := &mcp.ClientCapabilities{Elicitation: &mcp.ElicitationCapabilities{Form: &mcp.FormElicitationCapabilities{}}} + + text, toolRan := runOAuthMiddlewareCall(t, fake, formCaps, accept) + + assert.True(t, toolRan) + assert.Equal(t, "tool-ran", text) + require.NotNil(t, elicited) + assert.Equal(t, "form", elicited.Mode) + assert.Contains(t, elicited.Message, authURL) + assert.Contains(t, elicited.Message, "ABCD-1234") + }) + + t.Run("no elicitation capability falls back to a tool-result message", func(t *testing.T) { + t.Parallel() + const message = "Open https://example.com/auth to authorize, then retry." + fake := &fakeAuthenticator{ + outcome: &oauth.Outcome{UserAction: &oauth.UserAction{URL: "https://example.com/auth", Message: message}, FlowID: "flow-1"}, + } + + // No elicitation capability advertised, and no handler needed since the + // middleware should not issue an input request. + text, toolRan := runOAuthMiddlewareCall(t, fake, &mcp.ClientCapabilities{}, nil) + + assert.False(t, toolRan, "the tool must not run before authorization completes") + assert.Equal(t, message, text, "the manual instructions should be surfaced as a tool result") + assert.Zero(t, fake.awaitCalls) + assert.Zero(t, fake.cancelCalls) + }) +} + +func TestOAuthMultiRoundTripResultType(t *testing.T) { + t.Parallel() + + fake := &fakeAuthenticator{ + outcome: &oauth.Outcome{ + UserAction: &oauth.UserAction{URL: "https://example.com/auth", Message: "manual"}, + FlowID: "flow-1", + }, + } + server := mcp.NewServer(&mcp.Implementation{Name: "test-server", Version: "v0.0.1"}, nil) + var toolRan bool + handler := createOAuthToolMiddleware(fake, discardLogger())(func(_ context.Context, _ *mcp.CallToolRequest) (*mcp.CallToolResult, error) { + toolRan = true + return &mcp.CallToolResult{Content: []mcp.Content{&mcp.TextContent{Text: "tool-ran"}}}, nil + }) + server.AddTool(&mcp.Tool{ + Name: probeToolName, + InputSchema: &jsonschema.Schema{Type: "object"}, + }, handler) + + st, ct := mcp.NewInMemoryTransports() + ss, err := server.Connect(context.Background(), st, nil) + require.NoError(t, err) + t.Cleanup(func() { _ = ss.Close() }) + + client := mcp.NewClient(&mcp.Implementation{Name: "test-client", Version: "v0.0.1"}, &mcp.ClientOptions{ + Capabilities: &mcp.ClientCapabilities{Elicitation: &mcp.ElicitationCapabilities{URL: &mcp.URLElicitationCapabilities{}}}, + MultiRoundTrip: &mcp.MultiRoundTripOptions{Disabled: true}, + }) + cs, err := client.Connect(context.Background(), ct, nil) + require.NoError(t, err) + t.Cleanup(func() { _ = cs.Close() }) + + res, err := cs.CallTool(context.Background(), &mcp.CallToolParams{Name: probeToolName}) + require.NoError(t, err) + assert.True(t, res.NeedsInput(), "the wire response must declare resultType input_required") + assert.Contains(t, res.InputRequests, oauthElicitIDPrefix+"flow-1") + assert.False(t, toolRan) +} + +// TestRunStdioServerRejectsMultipleAuthModes verifies the mutually-exclusive +// guard: supplying more than one of a static token, an OAuth manager, or GitHub +// App auth is rejected before the server starts, rather than silently preferring +// one for auth and another for scope filtering. +func TestRunStdioServerRejectsMultipleAuthModes(t *testing.T) { + t.Parallel() + + mgr := oauth.NewManager(oauth.NewGitHubConfig("client-id", "", nil, "", 0), discardLogger()) + + tests := []struct { + name string + cfg StdioServerConfig + }{ + { + name: "token and oauth", + cfg: StdioServerConfig{Token: "ghp_static", OAuthManager: mgr}, + }, + { + name: "token and app", + cfg: StdioServerConfig{Token: "ghp_static", AppAuth: &githubapp.Config{}}, + }, + { + name: "oauth and app", + cfg: StdioServerConfig{OAuthManager: mgr, AppAuth: &githubapp.Config{}}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := RunStdioServer(tt.cfg) + require.Error(t, err) + assert.Contains(t, err.Error(), "exactly one authentication mode") + }) + } } // TestCreateGitHubClientsTokenProvider proves the OAuth wiring: when a diff --git a/internal/ghmcp/server.go b/internal/ghmcp/server.go index 2267dd5d62..67c3b067e7 100644 --- a/internal/ghmcp/server.go +++ b/internal/ghmcp/server.go @@ -12,6 +12,7 @@ import ( "syscall" "time" + "github.com/github/github-mcp-server/internal/githubapp" "github.com/github/github-mcp-server/internal/oauth" "github.com/github/github-mcp-server/pkg/errors" "github.com/github/github-mcp-server/pkg/github" @@ -257,15 +258,31 @@ type StdioServerConfig struct { // are hidden. The default set is the full supported list, which hides // nothing; an explicit, narrower list filters accordingly. OAuthScopes []string + + // AppAuth, when non-nil, enables non-interactive GitHub App server-to-server + // authentication: the server mints and transparently refreshes installation + // access tokens from the app's private key, with no browser, device code, or + // elicitation. It suits headless deployments (CI, Kubernetes, background + // agents). It is mutually exclusive with a static Token and with + // OAuthManager. See internal/githubapp and docs/github-app-auth.md — this + // injects a high-privilege credential alongside the agent and should not be + // used without an independent security review. + AppAuth *githubapp.Config } // RunStdioServer is not concurrent safe. func RunStdioServer(cfg StdioServerConfig) error { - // OAuth login and a static token are mutually exclusive: they would - // disagree on how the token is sourced (lazy provider vs. static) and on - // scope filtering, so reject the ambiguous combination up front. - if cfg.OAuthManager != nil && cfg.Token != "" { - return fmt.Errorf("OAuthManager and a static Token are mutually exclusive: provide one or the other") + // A static token, OAuth login, and GitHub App auth are mutually exclusive: + // they disagree on how the token is sourced (static vs. lazy provider) and + // on scope filtering, so reject any ambiguous combination up front. + authModes := 0 + for _, on := range []bool{cfg.Token != "", cfg.OAuthManager != nil, cfg.AppAuth != nil} { + if on { + authModes++ + } + } + if authModes > 1 { + return fmt.Errorf("choose exactly one authentication mode: a static Token, OAuthManager (OAuth login), or AppAuth (GitHub App)") } // Create app context @@ -290,6 +307,20 @@ func RunStdioServer(cfg StdioServerConfig) error { logger := slog.New(slogHandler) logger.Info("starting server", "version", cfg.Version, "host", cfg.Host, "readOnly", cfg.ReadOnly, "lockdownEnabled", cfg.LockdownMode) + // GitHub App server-to-server auth mints installation tokens with no human + // in the loop. Build the provider here so it can use the configured logger. + var appProvider *githubapp.Provider + if cfg.AppAuth != nil { + // Surfaced loudly because this injects a high-privilege credential next + // to the agent; the detailed guidance lives in docs/github-app-auth.md. + logger.Warn("GitHub App server-to-server authentication is enabled; installation tokens minted here can act across every repository the app is installed on — review docs/github-app-auth.md and prefer least-privilege, repository-scoped installations") + provider, err := githubapp.NewProvider(*cfg.AppAuth, logger) + if err != nil { + return fmt.Errorf("failed to configure GitHub App authentication: %w", err) + } + appProvider = provider + } + // Determine the scope set used to filter tools. Classic PATs expose their // granted scopes via the API; OAuth uses the requested scopes (the default // set hides nothing, a narrower explicit set filters accordingly). Other @@ -311,41 +342,42 @@ func RunStdioServer(cfg StdioServerConfig) error { logger.Debug("skipping scope filtering for non-PAT token") } - // For OAuth, the token is resolved lazily: empty until the user authorizes - // on the first tool call, then refreshed for the rest of the session. + // For OAuth or GitHub App auth, the token is resolved lazily by a provider: + // empty until the user authorizes (OAuth) or minted on demand and refreshed + // (App). A static PAT, by contrast, is passed through unchanged. var tokenProvider func() string - if cfg.OAuthManager != nil { + var toolHandlerMiddleware []inventory.ToolHandlerMiddleware + switch { + case cfg.OAuthManager != nil: tokenProvider = cfg.OAuthManager.AccessToken + toolHandlerMiddleware = append(toolHandlerMiddleware, createOAuthToolMiddleware(cfg.OAuthManager, logger)) + case appProvider != nil: + tokenProvider = appProvider.AccessToken } ghServer, err := NewStdioMCPServer(ctx, github.MCPServerConfig{ - Version: cfg.Version, - Host: cfg.Host, - Token: cfg.Token, - EnabledToolsets: cfg.EnabledToolsets, - EnabledTools: cfg.EnabledTools, - EnabledFeatures: cfg.EnabledFeatures, - ReadOnly: cfg.ReadOnly, - Translator: t, - ContentWindowSize: cfg.ContentWindowSize, - LockdownMode: cfg.LockdownMode, - InsidersMode: cfg.InsidersMode, - ExcludeTools: cfg.ExcludeTools, - Logger: logger, - RepoAccessTTL: cfg.RepoAccessCacheTTL, - TokenScopes: tokenScopes, - TokenProvider: tokenProvider, + Version: cfg.Version, + Host: cfg.Host, + Token: cfg.Token, + EnabledToolsets: cfg.EnabledToolsets, + EnabledTools: cfg.EnabledTools, + EnabledFeatures: cfg.EnabledFeatures, + ReadOnly: cfg.ReadOnly, + Translator: t, + ContentWindowSize: cfg.ContentWindowSize, + LockdownMode: cfg.LockdownMode, + InsidersMode: cfg.InsidersMode, + ExcludeTools: cfg.ExcludeTools, + Logger: logger, + RepoAccessTTL: cfg.RepoAccessCacheTTL, + TokenScopes: tokenScopes, + TokenProvider: tokenProvider, + ToolHandlerMiddleware: toolHandlerMiddleware, }) if err != nil { return fmt.Errorf("failed to create MCP server: %w", err) } - // With OAuth, intercept tool calls to run the authorization flow on first - // use, before the handler tries to call GitHub with an empty token. - if cfg.OAuthManager != nil { - ghServer.AddReceivingMiddleware(createOAuthMiddleware(cfg.OAuthManager, logger)) - } - if cfg.ExportTranslations { // Once server is initialized, all translations are loaded dumpTranslations() diff --git a/internal/githubapp/githubapp.go b/internal/githubapp/githubapp.go new file mode 100644 index 0000000000..49072b93f0 --- /dev/null +++ b/internal/githubapp/githubapp.go @@ -0,0 +1,275 @@ +// Package githubapp implements non-interactive GitHub App server-to-server +// (s2s) authentication for the stdio server. +// +// Unlike the user-to-server OAuth flows in internal/oauth, this requires no +// human: no browser, no device code, no elicitation. It signs a short-lived +// JWT with the app's private key, exchanges it for an installation access +// token, and transparently refreshes that token before it expires. That makes +// it suitable for headless deployments — CI, Kubernetes, background agents. +// +// It only depends on the standard library and golang.org/x/oauth2. +// +// # Security +// +// This mode injects a long-lived, high-privilege credential (the app private +// key) into an environment shared with an AI agent, and the installation +// tokens it mints can act across every repository the app is installed on. It +// was added by popular demand for non-interactive deployments, but exposing +// credentials to agents — especially in the cloud — is dangerous and is not +// recommended without an independent security review. See +// docs/github-app-auth.md for the full guidance and least-privilege advice. +package githubapp + +import ( + "context" + "crypto" + "crypto/rand" + "crypto/rsa" + "crypto/sha256" + "crypto/x509" + "encoding/base64" + "encoding/json" + "encoding/pem" + "errors" + "fmt" + "io" + "log/slog" + "net/http" + "net/url" + "os" + "strings" + "sync" + "time" + + "golang.org/x/oauth2" +) + +const ( + // jwtLifetime is how long minted app JWTs are valid. GitHub rejects app JWTs + // whose exp is more than 10 minutes in the future; 9 minutes leaves headroom. + jwtLifetime = 9 * time.Minute + + // clockSkew backdates the JWT iat to tolerate small clock differences + // between this host and GitHub, which would otherwise reject the JWT. + clockSkew = 60 * time.Second + + // refreshBuffer refreshes installation tokens this long before their real + // expiry so an in-flight request never races the expiry boundary. + refreshBuffer = 5 * time.Minute + + // httpTimeout bounds each call to the installation token endpoint so a + // stalled GitHub API cannot block a tool call indefinitely. + httpTimeout = 30 * time.Second +) + +// Config describes a GitHub App installation used for server-to-server auth. +type Config struct { + // AppID is the GitHub App's App ID or client ID; it becomes the JWT issuer + // (iss). Both forms are accepted by GitHub. + AppID string + + // InstallationID identifies the installation whose access token is minted. + InstallationID string + + // PrivateKey signs the app JWT (RS256). Parse one with ParsePrivateKey. + PrivateKey *rsa.PrivateKey + + // BaseRESTURL is the REST API base, e.g. https://api.github.com/ for + // github.com or https://HOST/api/v3/ for GitHub Enterprise Server. + BaseRESTURL string +} + +// Validate reports whether the configuration is complete enough to mint tokens. +func (c Config) Validate() error { + switch { + case c.AppID == "": + return errors.New("GitHub App ID is required (GITHUB_APP_ID)") + case c.InstallationID == "": + return errors.New("GitHub App installation ID is required (GITHUB_APP_INSTALLATION_ID)") + case c.PrivateKey == nil: + return errors.New("GitHub App private key is required (GITHUB_APP_PRIVATE_KEY_PATH or GITHUB_APP_PRIVATE_KEY)") + case c.BaseRESTURL == "": + return errors.New("GitHub App REST base URL is required") + } + return nil +} + +// ParsePrivateKey parses a PEM-encoded RSA private key in PKCS#1 ("RSA PRIVATE +// KEY") or PKCS#8 ("PRIVATE KEY") form — the two formats GitHub issues for app +// keys. +func ParsePrivateKey(pemBytes []byte) (*rsa.PrivateKey, error) { + block, _ := pem.Decode(pemBytes) + if block == nil { + return nil, errors.New("no PEM block found in private key") + } + if key, err := x509.ParsePKCS1PrivateKey(block.Bytes); err == nil { + return key, nil + } + parsed, err := x509.ParsePKCS8PrivateKey(block.Bytes) + if err != nil { + return nil, fmt.Errorf("parsing private key (want PKCS#1 or PKCS#8 RSA): %w", err) + } + key, ok := parsed.(*rsa.PrivateKey) + if !ok { + return nil, fmt.Errorf("private key is %T, want an RSA key", parsed) + } + return key, nil +} + +// mintJWT builds and signs a short-lived app JWT (RS256) for the configured +// app, as required by the installation token endpoint. +func (c Config) mintJWT(now time.Time) (string, error) { + header := map[string]string{"alg": "RS256", "typ": "JWT"} + claims := map[string]any{ + "iat": now.Add(-clockSkew).Unix(), + "exp": now.Add(jwtLifetime).Unix(), + "iss": c.AppID, + } + + headerJSON, err := json.Marshal(header) + if err != nil { + return "", fmt.Errorf("encoding JWT header: %w", err) + } + claimsJSON, err := json.Marshal(claims) + if err != nil { + return "", fmt.Errorf("encoding JWT claims: %w", err) + } + + signingInput := base64.RawURLEncoding.EncodeToString(headerJSON) + "." + + base64.RawURLEncoding.EncodeToString(claimsJSON) + + digest := sha256.Sum256([]byte(signingInput)) + signature, err := rsa.SignPKCS1v15(rand.Reader, c.PrivateKey, crypto.SHA256, digest[:]) + if err != nil { + return "", fmt.Errorf("signing JWT: %w", err) + } + + return signingInput + "." + base64.RawURLEncoding.EncodeToString(signature), nil +} + +// installationTokenSource is an oauth2.TokenSource that mints GitHub App +// installation access tokens. It performs no caching itself; wrap it in +// oauth2.ReuseTokenSource (see NewProvider) for that. +type installationTokenSource struct { + cfg Config + httpClient *http.Client +} + +func newInstallationTokenSource(cfg Config, httpClient *http.Client) *installationTokenSource { + if httpClient == nil { + httpClient = &http.Client{Timeout: httpTimeout} + } + return &installationTokenSource{cfg: cfg, httpClient: httpClient} +} + +// Token mints a fresh installation access token. The returned token's Expiry is +// set refreshBuffer before the real expiry so callers refresh early. +func (s *installationTokenSource) Token() (*oauth2.Token, error) { + jwt, err := s.cfg.mintJWT(time.Now()) + if err != nil { + return nil, err + } + + endpoint, err := url.JoinPath(s.cfg.BaseRESTURL, "app", "installations", s.cfg.InstallationID, "access_tokens") + if err != nil { + return nil, fmt.Errorf("building installation token URL: %w", err) + } + + ctx, cancel := context.WithTimeout(context.Background(), httpTimeout) + defer cancel() + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, nil) + if err != nil { + return nil, fmt.Errorf("creating installation token request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+jwt) + req.Header.Set("Accept", "application/vnd.github+json") + req.Header.Set("X-GitHub-Api-Version", "2022-11-28") + + resp, err := s.httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("requesting installation token: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusCreated { + // The error body is GitHub's JSON message (never the token); include a + // bounded snippet to make misconfiguration diagnosable. + snippet, _ := io.ReadAll(io.LimitReader(resp.Body, 512)) + return nil, fmt.Errorf("installation token request failed: %s: %s", resp.Status, strings.TrimSpace(string(snippet))) + } + + var body struct { + Token string `json:"token"` + ExpiresAt time.Time `json:"expires_at"` + } + if err := json.NewDecoder(resp.Body).Decode(&body); err != nil { + return nil, fmt.Errorf("decoding installation token response: %w", err) + } + if body.Token == "" { + return nil, errors.New("installation token response did not contain a token") + } + + expiry := body.ExpiresAt + if !expiry.IsZero() { + expiry = expiry.Add(-refreshBuffer) + } + return &oauth2.Token{ + AccessToken: body.Token, + TokenType: "token", + Expiry: expiry, + }, nil +} + +// Provider supplies GitHub App installation access tokens, caching and +// refreshing them transparently. Its AccessToken method mirrors +// oauth.Manager.AccessToken so it can back BearerAuthTransport.TokenProvider. +type Provider struct { + source oauth2.TokenSource + logger *slog.Logger + + mu sync.Mutex + errLogged bool +} + +// NewProvider validates cfg and returns a Provider that mints and refreshes +// installation tokens. A nil logger logs to stderr. +func NewProvider(cfg Config, logger *slog.Logger) (*Provider, error) { + if err := cfg.Validate(); err != nil { + return nil, err + } + if logger == nil { + logger = slog.New(slog.NewTextHandler(os.Stderr, nil)) + } + // ReuseTokenSource caches the token and only calls the underlying source + // once the cached token is expired. Because Token() backdates Expiry by + // refreshBuffer, that refresh happens ~5 minutes before the real expiry. + source := oauth2.ReuseTokenSource(nil, newInstallationTokenSource(cfg, nil)) + return &Provider{source: source, logger: logger}, nil +} + +// AccessToken returns a currently valid installation access token, refreshing +// it if needed, or "" if a token could not be obtained. A fetch failure is +// logged once (until the next success) so a misconfiguration is visible without +// flooding the log on every tool call. +func (p *Provider) AccessToken() string { + tok, err := p.source.Token() + if err != nil { + p.mu.Lock() + if !p.errLogged { + p.errLogged = true + p.logger.Error("failed to obtain GitHub App installation token", "error", err) + } + p.mu.Unlock() + return "" + } + p.mu.Lock() + p.errLogged = false + p.mu.Unlock() + return tok.AccessToken +} + +// HasToken reports whether a valid token can currently be obtained. +func (p *Provider) HasToken() bool { + return p.AccessToken() != "" +} diff --git a/internal/githubapp/githubapp_test.go b/internal/githubapp/githubapp_test.go new file mode 100644 index 0000000000..c2d5eeff7f --- /dev/null +++ b/internal/githubapp/githubapp_test.go @@ -0,0 +1,271 @@ +package githubapp + +import ( + "bytes" + "crypto" + "crypto/ed25519" + "crypto/rand" + "crypto/rsa" + "crypto/sha256" + "crypto/x509" + "encoding/base64" + "encoding/json" + "encoding/pem" + "fmt" + "log/slog" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func newTestKey(t *testing.T) *rsa.PrivateKey { + t.Helper() + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + return key +} + +func pkcs1PEM(t *testing.T, key *rsa.PrivateKey) []byte { + t.Helper() + return pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(key)}) +} + +func pkcs8PEM(t *testing.T, key *rsa.PrivateKey) []byte { + t.Helper() + der, err := x509.MarshalPKCS8PrivateKey(key) + require.NoError(t, err) + return pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: der}) +} + +func TestParsePrivateKey(t *testing.T) { + key := newTestKey(t) + + t.Run("PKCS1", func(t *testing.T) { + got, err := ParsePrivateKey(pkcs1PEM(t, key)) + require.NoError(t, err) + assert.Equal(t, key.N, got.N) + }) + + t.Run("PKCS8", func(t *testing.T) { + got, err := ParsePrivateKey(pkcs8PEM(t, key)) + require.NoError(t, err) + assert.Equal(t, key.N, got.N) + }) + + t.Run("not PEM", func(t *testing.T) { + _, err := ParsePrivateKey([]byte("not a pem")) + require.Error(t, err) + assert.Contains(t, err.Error(), "no PEM block") + }) + + t.Run("non-RSA key", func(t *testing.T) { + _, priv, err := ed25519.GenerateKey(rand.Reader) + require.NoError(t, err) + der, err := x509.MarshalPKCS8PrivateKey(priv) + require.NoError(t, err) + keyPEM := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: der}) + + _, err = ParsePrivateKey(keyPEM) + require.Error(t, err) + assert.Contains(t, err.Error(), "want an RSA key") + }) +} + +func TestConfigValidate(t *testing.T) { + key := newTestKey(t) + base := Config{AppID: "123", InstallationID: "456", PrivateKey: key, BaseRESTURL: "https://api.github.com/"} + require.NoError(t, base.Validate()) + + tests := []struct { + name string + mutate func(c *Config) + want string + }{ + {"missing app id", func(c *Config) { c.AppID = "" }, "App ID is required"}, + {"missing installation id", func(c *Config) { c.InstallationID = "" }, "installation ID is required"}, + {"missing private key", func(c *Config) { c.PrivateKey = nil }, "private key is required"}, + {"missing base url", func(c *Config) { c.BaseRESTURL = "" }, "REST base URL is required"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := base + tt.mutate(&c) + err := c.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), tt.want) + }) + } +} + +// verifyJWT parses and verifies an app JWT against the public key and returns +// its claims, asserting the structural requirements GitHub enforces. +func verifyJWT(t *testing.T, token string, pub *rsa.PublicKey) map[string]any { + t.Helper() + parts := strings.Split(token, ".") + require.Len(t, parts, 3, "JWT must have three segments") + + headerJSON, err := base64.RawURLEncoding.DecodeString(parts[0]) + require.NoError(t, err) + var header map[string]string + require.NoError(t, json.Unmarshal(headerJSON, &header)) + assert.Equal(t, "RS256", header["alg"]) + assert.Equal(t, "JWT", header["typ"]) + + signingInput := parts[0] + "." + parts[1] + digest := sha256.Sum256([]byte(signingInput)) + signature, err := base64.RawURLEncoding.DecodeString(parts[2]) + require.NoError(t, err) + require.NoError(t, rsa.VerifyPKCS1v15(pub, crypto.SHA256, digest[:], signature), "signature must verify") + + claimsJSON, err := base64.RawURLEncoding.DecodeString(parts[1]) + require.NoError(t, err) + var claims map[string]any + require.NoError(t, json.Unmarshal(claimsJSON, &claims)) + return claims +} + +func TestMintJWT(t *testing.T) { + key := newTestKey(t) + cfg := Config{AppID: "my-app-id", PrivateKey: key} + + now := time.Now() + token, err := cfg.mintJWT(now) + require.NoError(t, err) + + claims := verifyJWT(t, token, &key.PublicKey) + assert.Equal(t, "my-app-id", claims["iss"]) + + iat := int64(claims["iat"].(float64)) + exp := int64(claims["exp"].(float64)) + assert.Equal(t, now.Add(-clockSkew).Unix(), iat, "iat should be backdated by the clock skew") + assert.Equal(t, now.Add(jwtLifetime).Unix(), exp) + assert.LessOrEqual(t, exp-iat, int64((10 * time.Minute).Seconds()), "JWT must live no longer than GitHub's 10 minute cap") +} + +// installationServer is a fake installation token endpoint that verifies the +// app JWT and returns a token expiring at expiresAt. It counts mint requests. +func installationServer(t *testing.T, pub *rsa.PublicKey, token string, expiresAt time.Time) (*httptest.Server, *atomic.Int32) { + t.Helper() + var calls atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls.Add(1) + assert.Equal(t, http.MethodPost, r.Method) + assert.Equal(t, "/app/installations/456/access_tokens", r.URL.Path) + + authz := r.Header.Get("Authorization") + require.True(t, strings.HasPrefix(authz, "Bearer "), "must send the app JWT as a bearer token") + verifyJWT(t, strings.TrimPrefix(authz, "Bearer "), pub) + + w.WriteHeader(http.StatusCreated) + _ = json.NewEncoder(w).Encode(map[string]any{ + "token": token, + "expires_at": expiresAt.UTC().Format(time.RFC3339), + }) + })) + t.Cleanup(srv.Close) + return srv, &calls +} + +func newTestConfig(key *rsa.PrivateKey, baseURL string) Config { + return Config{AppID: "123", InstallationID: "456", PrivateKey: key, BaseRESTURL: baseURL + "/"} +} + +func TestProviderFetchesToken(t *testing.T) { + key := newTestKey(t) + srv, calls := installationServer(t, &key.PublicKey, "ghs_fresh", time.Now().Add(time.Hour)) + + provider, err := NewProvider(newTestConfig(key, srv.URL), slog.New(slog.NewTextHandler(&bytes.Buffer{}, nil))) + require.NoError(t, err) + + assert.Equal(t, "ghs_fresh", provider.AccessToken()) + assert.True(t, provider.HasToken()) + assert.Equal(t, int32(1), calls.Load()) +} + +func TestProviderCachesToken(t *testing.T) { + key := newTestKey(t) + srv, calls := installationServer(t, &key.PublicKey, "ghs_cached", time.Now().Add(time.Hour)) + + provider, err := NewProvider(newTestConfig(key, srv.URL), slog.New(slog.NewTextHandler(&bytes.Buffer{}, nil))) + require.NoError(t, err) + + for range 3 { + assert.Equal(t, "ghs_cached", provider.AccessToken()) + } + assert.Equal(t, int32(1), calls.Load(), "a token valid for an hour should be minted only once") +} + +func TestProviderRefreshesNearExpiry(t *testing.T) { + key := newTestKey(t) + // expires within the refresh buffer, so the stored expiry is already in the + // past and every call re-mints. + srv, calls := installationServer(t, &key.PublicKey, "ghs_short", time.Now().Add(refreshBuffer-time.Minute)) + + provider, err := NewProvider(newTestConfig(key, srv.URL), slog.New(slog.NewTextHandler(&bytes.Buffer{}, nil))) + require.NoError(t, err) + + assert.Equal(t, "ghs_short", provider.AccessToken()) + assert.Equal(t, "ghs_short", provider.AccessToken()) + assert.Equal(t, int32(2), calls.Load(), "a token expiring within the refresh buffer should re-mint each call") +} + +func TestProviderErrorLoggedOnce(t *testing.T) { + key := newTestKey(t) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"message":"A JSON web token could not be decoded"}`)) + })) + t.Cleanup(srv.Close) + + var logBuf bytes.Buffer + logger := slog.New(slog.NewTextHandler(&logBuf, nil)) + provider, err := NewProvider(newTestConfig(key, srv.URL), logger) + require.NoError(t, err) + + assert.Empty(t, provider.AccessToken()) + assert.Empty(t, provider.AccessToken()) + assert.Equal(t, 1, strings.Count(logBuf.String(), "failed to obtain GitHub App installation token"), + "a repeated fetch failure should only be logged once") +} + +func TestProviderErrorIncludesStatus(t *testing.T) { + key := newTestKey(t) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message":"Not Found"}`)) + })) + t.Cleanup(srv.Close) + + source := newInstallationTokenSource(newTestConfig(key, srv.URL), srv.Client()) + _, err := source.Token() + require.Error(t, err) + assert.Contains(t, err.Error(), "404") + assert.Contains(t, err.Error(), "Not Found") +} + +func TestNewProviderValidates(t *testing.T) { + _, err := NewProvider(Config{}, nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "App ID is required") +} + +// Ensure the source returns an error rather than panicking on a token-less 201. +func TestSourceRejectsEmptyToken(t *testing.T) { + key := newTestKey(t) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusCreated) + _, _ = fmt.Fprint(w, `{"expires_at":"2099-01-01T00:00:00Z"}`) + })) + t.Cleanup(srv.Close) + + source := newInstallationTokenSource(newTestConfig(key, srv.URL), srv.Client()) + _, err := source.Token() + require.Error(t, err) + assert.Contains(t, err.Error(), "did not contain a token") +} diff --git a/internal/oauth/manager.go b/internal/oauth/manager.go index a78e919df7..8c16729f5b 100644 --- a/internal/oauth/manager.go +++ b/internal/oauth/manager.go @@ -2,6 +2,7 @@ package oauth import ( "context" + "crypto/rand" "errors" "log/slog" "net/http" @@ -20,6 +21,10 @@ const DefaultAuthTimeout = 5 * time.Minute // stalled GitHub token endpoint cannot block a tool call indefinitely. const tokenRefreshTimeout = 30 * time.Second +// ErrStaleAuthorizationFlow indicates that a prompt response belongs to an +// authorization flow that is no longer current. +var ErrStaleAuthorizationFlow = errors.New("authorization prompt has expired") + // flowStatus tracks the manager's single-flight authorization state. type flowStatus int @@ -37,6 +42,11 @@ type Outcome struct { // flow continues in the background; the user should retry once they have // completed it. UserAction *UserAction + + // FlowID correlates a user action with the authorization flow that produced + // it. Callers must pass it back to AwaitToken or Cancel so a delayed response + // cannot affect a newer flow. + FlowID string } // UserAction is an instruction for the user to complete authorization out of @@ -65,9 +75,12 @@ type Manager struct { mu sync.Mutex source oauth2.TokenSource // refreshing source, set once authorized + tokenGeneration uint64 // increments whenever source is replaced status flowStatus + flowID string pending *UserAction done chan struct{} + cancelFlow context.CancelFunc // cancels the in-flight flow, if any lastErr error refreshErrLogged bool // true once a refresh failure has been logged, reset on re-auth } @@ -93,11 +106,21 @@ func NewManager(cfg Config, logger *slog.Logger) *Manager { // re-authorization is required). It is cheap to call repeatedly: the underlying // token source caches and only refreshes when the token has expired. func (m *Manager) AccessToken() string { + token, _ := m.accessToken() + return token +} + +// accessToken returns the token together with the generation of the source it +// checked. Authenticate uses the generation to detect a source installed while +// token validation was in progress, without repeating a potentially blocking +// refresh request. +func (m *Manager) accessToken() (string, uint64) { m.mu.Lock() src := m.source + generation := m.tokenGeneration m.mu.Unlock() if src == nil { - return "" + return "", generation } // Refresh (if needed) happens here, off the lock, because ReuseTokenSource may // make a blocking network call and holding m.mu would serialize every tool call. @@ -109,17 +132,17 @@ func (m *Manager) AccessToken() string { // prompt. The oauth2 error carries the token endpoint's response, not the // access or refresh token. m.mu.Lock() - if !m.refreshErrLogged { + if m.tokenGeneration == generation && !m.refreshErrLogged { m.refreshErrLogged = true m.logger.Warn("OAuth token refresh failed; re-authorization required", "error", err) } m.mu.Unlock() - return "" + return "", generation } if !tok.Valid() { - return "" + return "", generation } - return tok.AccessToken + return tok.AccessToken, generation } // HasToken reports whether a valid token is currently available. @@ -137,63 +160,142 @@ func (m *Manager) HasToken() bool { // Only one flow runs at a time. Concurrent callers either join a running secure // flow, receive the pending user action, or are told to retry shortly. func (m *Manager) Authenticate(ctx context.Context, prompter Prompter) (*Outcome, error) { - if m.AccessToken() != "" { - return nil, nil - } + var flowID string + var done chan struct{} + for { + token, checkedTokenGeneration := m.accessToken() + if token != "" { + return nil, nil + } - m.mu.Lock() - switch m.status { - case statusAwaitingUser: - ua := m.pending - m.mu.Unlock() - return &Outcome{UserAction: ua}, nil - case statusStarting: - m.mu.Unlock() - return &Outcome{UserAction: &UserAction{ - Message: "GitHub authorization is already in progress. Please retry your request in a few seconds.", - }}, nil - case statusInProgress: - done := m.done + m.mu.Lock() + switch m.status { + case statusAwaitingUser: + ua := m.pending + flowID := m.flowID + m.mu.Unlock() + return &Outcome{UserAction: ua, FlowID: flowID}, nil + case statusStarting: + flowID := m.flowID + m.mu.Unlock() + return &Outcome{UserAction: &UserAction{ + Message: "GitHub authorization is already in progress. Please retry your request in a few seconds.", + }, FlowID: flowID}, nil + case statusInProgress: + done := m.done + flowID := m.flowID + m.mu.Unlock() + return m.joinWait(ctx, done, flowID) + } + + // A flow may have installed a token while the source above was being + // checked. Retry if the source changed before claiming the idle state. + if m.tokenGeneration != checkedTokenGeneration { + m.mu.Unlock() + continue + } + + // Idle: this call owns the new flow. + m.status = statusStarting + m.flowID = rand.Text() + flowID = m.flowID + m.lastErr = nil + m.done = make(chan struct{}) + done = m.done m.mu.Unlock() - return m.joinWait(ctx, done) + break } - // Idle: this call owns the new flow. - m.status = statusStarting - m.lastErr = nil - m.done = make(chan struct{}) - done := m.done - m.mu.Unlock() - plan, err := m.begin(prompter) if err != nil { - m.complete(nil, err) + m.complete(flowID, nil, err) return nil, err } + bgCtx, cancel := context.WithTimeout(context.Background(), DefaultAuthTimeout) m.mu.Lock() + if m.flowID != flowID { + m.mu.Unlock() + cancel() + return nil, ErrStaleAuthorizationFlow + } if plan.userAction != nil { m.status = statusAwaitingUser m.pending = plan.userAction } else { m.status = statusInProgress } + m.cancelFlow = cancel m.mu.Unlock() - - bgCtx, cancel := context.WithTimeout(context.Background(), DefaultAuthTimeout) - go m.runFlow(bgCtx, cancel, plan) + go m.runFlow(bgCtx, cancel, flowID, plan) if plan.userAction != nil { - return &Outcome{UserAction: plan.userAction}, nil + return &Outcome{UserAction: plan.userAction, FlowID: flowID}, nil } - return m.joinWait(ctx, done) + return m.joinWait(ctx, done, flowID) +} + +// AwaitToken blocks until the in-flight authorization flow yields a token, the +// flow ends without one, or ctx is done. It is the resume half of the +// multi-round-trip flow: a transport that presented the authorization prompt +// itself (via elicitation returned from a tool call) calls this once the user +// has acted, to wait for the background token acquisition to finish. +// +// It returns (nil, nil) once a token is available (proceed), (&Outcome{UserAction}, +// nil) when the user must still act out of band, or (nil, err) on failure. +func (m *Manager) AwaitToken(ctx context.Context, flowID string) (*Outcome, error) { + m.mu.Lock() + if flowID == "" || flowID != m.flowID { + m.mu.Unlock() + return nil, ErrStaleAuthorizationFlow + } + done := m.done + m.mu.Unlock() + if m.AccessToken() != "" { + return nil, nil + } + if done == nil { + // No flow is in flight; report whatever terminal state it left behind. + return m.outcomeAfterFlow(flowID) + } + select { + case <-done: + return m.outcomeAfterFlow(flowID) + case <-ctx.Done(): + return nil, ctx.Err() + } +} + +// Cancel retires the matching authorization flow and aborts its background +// callback listener or device poll. It returns false if flowID is stale. +func (m *Manager) Cancel(flowID string) bool { + m.mu.Lock() + if flowID == "" || flowID != m.flowID { + m.mu.Unlock() + return false + } + cancel := m.cancelFlow + m.status = statusIdle + m.flowID = "" + m.pending = nil + m.cancelFlow = nil + m.lastErr = context.Canceled + if m.done != nil { + close(m.done) + m.done = nil + } + m.mu.Unlock() + if cancel != nil { + cancel() + } + return true } // runFlow executes a prepared flow in the background and records the result. The // optional display prompt runs concurrently: a decline (or other failure) aborts // the flow, while an undeliverable prompt degrades to the manual fallback without // tearing the flow down, so the user can still authorize out of band. -func (m *Manager) runFlow(ctx context.Context, cancel context.CancelFunc, plan *flowPlan) { +func (m *Manager) runFlow(ctx context.Context, cancel context.CancelFunc, flowID string, plan *flowPlan) { defer cancel() if plan.display != nil { @@ -212,7 +314,7 @@ func (m *Manager) runFlow(ctx context.Context, cancel context.CancelFunc, plan * // prompt. Surface the manual instructions instead of failing, and // keep the background flow alive so the user can still authorize. m.logger.Debug("authorization prompt undeliverable; falling back to manual instructions", "reason", err) - m.fallBackToUserAction(plan.fallback) + m.fallBackToUserAction(flowID, plan.fallback) default: // A user decline (ErrPromptDeclined) or any other prompt failure // ends the flow. @@ -223,17 +325,17 @@ func (m *Manager) runFlow(ctx context.Context, cancel context.CancelFunc, plan * } tok, err := plan.run(ctx) - m.complete(tok, err) + m.complete(flowID, tok, err) } // fallBackToUserAction promotes a running secure flow to the manual user-action // channel after its prompt could not be delivered. The background flow keeps // running, so the user can complete authorization out of band and retry. It is a // no-op if the flow has already resolved. -func (m *Manager) fallBackToUserAction(ua *UserAction) { +func (m *Manager) fallBackToUserAction(flowID string, ua *UserAction) { m.mu.Lock() defer m.mu.Unlock() - if m.status != statusInProgress { + if m.flowID != flowID || m.status != statusInProgress { return } m.status = statusAwaitingUser @@ -248,12 +350,16 @@ func (m *Manager) fallBackToUserAction(ua *UserAction) { // complete records the flow result, installing a refreshing token source on // success, and wakes any joined callers. -func (m *Manager) complete(tok *oauth2.Token, err error) { +func (m *Manager) complete(flowID string, tok *oauth2.Token, err error) { m.mu.Lock() defer m.mu.Unlock() + if m.flowID != flowID { + return + } m.status = statusIdle m.pending = nil + m.cancelFlow = nil if err != nil { m.lastErr = err m.logger.Debug("oauth flow failed", "error", err) @@ -265,6 +371,7 @@ func (m *Manager) complete(tok *oauth2.Token, err error) { // client so a stalled token endpoint can't block a tool call forever. refreshCtx := context.WithValue(context.Background(), oauth2.HTTPClient, &http.Client{Timeout: tokenRefreshTimeout}) m.source = m.refreshConfig.TokenSource(refreshCtx, tok) + m.tokenGeneration++ m.refreshErrLogged = false m.logger.Info("github authorization complete") } @@ -277,28 +384,39 @@ func (m *Manager) complete(tok *oauth2.Token, err error) { // joinWait blocks until the running flow finishes or ctx is cancelled. If the // flow was promoted to the manual channel while waiting (its prompt could not be // delivered), it returns that user action rather than an error. -func (m *Manager) joinWait(ctx context.Context, done chan struct{}) (*Outcome, error) { +func (m *Manager) joinWait(ctx context.Context, done chan struct{}, flowID string) (*Outcome, error) { select { case <-done: - if m.AccessToken() != "" { - return nil, nil - } - m.mu.Lock() - pending := m.pending - err := m.lastErr - m.mu.Unlock() - if pending != nil { - return &Outcome{UserAction: pending}, nil - } - if err != nil { - return nil, err - } - return nil, errors.New("authorization did not complete") + return m.outcomeAfterFlow(flowID) case <-ctx.Done(): return nil, ctx.Err() } } +// outcomeAfterFlow reports the result once the flow's done channel has closed +// (or when there is no flow in flight): a token to proceed (nil, nil), a pending +// user action to surface, or the flow's error. +func (m *Manager) outcomeAfterFlow(flowID string) (*Outcome, error) { + m.mu.Lock() + if flowID == "" || flowID != m.flowID { + m.mu.Unlock() + return nil, ErrStaleAuthorizationFlow + } + pending := m.pending + err := m.lastErr + m.mu.Unlock() + if m.AccessToken() != "" { + return nil, nil + } + if pending != nil { + return &Outcome{UserAction: pending, FlowID: flowID}, nil + } + if err != nil { + return nil, err + } + return nil, errors.New("authorization did not complete") +} + func (m *Manager) oauth2Config(redirectURL string) *oauth2.Config { return &oauth2.Config{ ClientID: m.config.ClientID, diff --git a/internal/oauth/manager_test.go b/internal/oauth/manager_test.go index 6f43c03ef9..52d5a54309 100644 --- a/internal/oauth/manager_test.go +++ b/internal/oauth/manager_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/oauth2" ) // newManager wires a Manager to the fake GitHub server. By default the browser @@ -167,6 +168,7 @@ func TestAuthenticateLastDitchUserAction(t *testing.T) { require.NoError(t, err) require.NotNil(t, out) require.NotNil(t, out.UserAction) + require.NotEmpty(t, out.FlowID) assert.NotEmpty(t, out.UserAction.URL) assert.Contains(t, out.UserAction.Message, "open this URL") assert.Contains(t, out.UserAction.Message, securityAdvisory, @@ -178,12 +180,127 @@ func TestAuthenticateLastDitchUserAction(t *testing.T) { require.NoError(t, err) require.NotNil(t, out2.UserAction) assert.Equal(t, out.UserAction.URL, out2.UserAction.URL) + assert.Equal(t, out.FlowID, out2.FlowID) // The user opens the URL out of band; the background flow then completes. require.NoError(t, browserGet(out.UserAction.URL)) assert.Equal(t, "gho_access", waitForToken(t, m)) } +func TestAwaitTokenCompletesCurrentFlow(t *testing.T) { + f := newFakeGitHub(t) + m := newManager(t, f) + m.openURL = func(string) error { return errors.New("no browser") } + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + out, err := m.Authenticate(ctx, nil) + require.NoError(t, err) + require.NotNil(t, out) + require.NotNil(t, out.UserAction) + require.NotEmpty(t, out.FlowID) + + authDone := make(chan error, 1) + go func() { + authDone <- browserGet(out.UserAction.URL) + }() + + awaited, err := m.AwaitToken(ctx, out.FlowID) + require.NoError(t, err) + assert.Nil(t, awaited) + require.NoError(t, <-authDone) + assert.Equal(t, "gho_access", m.AccessToken()) +} + +func TestCancelAndAwaitTokenAreFlowScoped(t *testing.T) { + f := newFakeGitHub(t) + m := newManager(t, f) + m.openURL = func(string) error { return errors.New("no browser") } + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + first, err := m.Authenticate(ctx, nil) + require.NoError(t, err) + require.NotNil(t, first) + require.NotEmpty(t, first.FlowID) + + assert.True(t, m.Cancel(first.FlowID), "the current flow should be cancelled") + second, err := m.Authenticate(ctx, nil) + require.NoError(t, err) + require.NotNil(t, second) + require.NotNil(t, second.UserAction) + require.NotEmpty(t, second.FlowID) + require.NotEqual(t, first.FlowID, second.FlowID) + + assert.False(t, m.Cancel(first.FlowID), "a stale decline must not cancel the newer flow") + _, err = m.AwaitToken(ctx, first.FlowID) + assert.ErrorIs(t, err, ErrStaleAuthorizationFlow) + + authDone := make(chan error, 1) + go func() { + authDone <- browserGet(second.UserAction.URL) + }() + awaited, err := m.AwaitToken(ctx, second.FlowID) + require.NoError(t, err) + assert.Nil(t, awaited) + require.NoError(t, <-authDone) + assert.Equal(t, "gho_access", m.AccessToken()) +} + +type blockingTokenSource struct { + entered chan struct{} + release chan struct{} + once sync.Once +} + +func (s *blockingTokenSource) Token() (*oauth2.Token, error) { + s.once.Do(func() { close(s.entered) }) + <-s.release + return nil, errors.New("stale token source failed") +} + +func TestAuthenticateRechecksTokenBeforeStartingFlow(t *testing.T) { + f := newFakeGitHub(t) + m := newManager(t, f) + staleSource := &blockingTokenSource{ + entered: make(chan struct{}), + release: make(chan struct{}), + } + + m.mu.Lock() + m.source = staleSource + m.status = statusInProgress + m.flowID = "existing-flow" + m.done = make(chan struct{}) + m.mu.Unlock() + + type result struct { + out *Outcome + err error + } + authResult := make(chan result, 1) + go func() { + out, err := m.Authenticate(context.Background(), nil) + authResult <- result{out: out, err: err} + }() + + <-staleSource.entered + m.complete("existing-flow", &oauth2.Token{ + AccessToken: "installed-token", + TokenType: "bearer", + Expiry: time.Now().Add(time.Hour), + }, nil) + close(staleSource.release) + + got := <-authResult + require.NoError(t, got.err) + assert.Nil(t, got.out) + assert.Equal(t, "installed-token", m.AccessToken()) + assert.Empty(t, f.recordedGrants(), "a completed concurrent flow must not trigger a redundant authorization") +} + func TestAuthenticateDeviceFlow(t *testing.T) { f := newFakeGitHub(t) f.deviceToken = "gho_device_token" diff --git a/pkg/github/server.go b/pkg/github/server.go index 627cc678b2..67db83a77a 100644 --- a/pkg/github/server.go +++ b/pkg/github/server.go @@ -73,6 +73,11 @@ type MCPServerConfig struct { // token is obtained lazily on first use and refreshed thereafter. TokenProvider func() string + // ToolHandlerMiddleware wraps every registered tool handler. Unlike MCP + // receiving middleware, these wrappers execute inside Server.callTool, so + // SDK result finalization still runs on results they return. + ToolHandlerMiddleware []inventory.ToolHandlerMiddleware + // Additional server options to apply ServerOptions []MCPServerOption } @@ -105,7 +110,7 @@ func NewMCPServer(ctx context.Context, cfg *MCPServerConfig, deps ToolDependenci } // Register GitHub tools/resources/prompts from the inventory. - inv.RegisterAll(ctx, ghServer, deps) + inv.RegisterAll(ctx, ghServer, deps, cfg.ToolHandlerMiddleware...) // Register MCP App UI resources whenever the embedded UI assets are // available. The resources are static HTML and are only referenced by diff --git a/pkg/inventory/registry.go b/pkg/inventory/registry.go index 6505e6b5ef..915ed0aa1c 100644 --- a/pkg/inventory/registry.go +++ b/pkg/inventory/registry.go @@ -219,9 +219,9 @@ func shouldStripMCPAppsMetadata(ctx context.Context, featureFlagEnabled bool) bo // user identity from ctx would otherwise see context.Background() and // falsely report the flag off, even when the actual request arrived on the // /insiders route. -func (r *Inventory) RegisterTools(ctx context.Context, s *mcp.Server, deps any) { +func (r *Inventory) RegisterTools(ctx context.Context, s *mcp.Server, deps any, middleware ...ToolHandlerMiddleware) { for _, tool := range r.ToolsForRegistration(ctx) { - tool.RegisterFunc(s, deps) + tool.RegisterFunc(s, deps, middleware...) } } @@ -257,8 +257,8 @@ func (r *Inventory) RegisterPrompts(ctx context.Context, s *mcp.Server) { // RegisterAll registers all available tools, resources, and prompts with the server. // The context is used for feature flag evaluation. -func (r *Inventory) RegisterAll(ctx context.Context, s *mcp.Server, deps any) { - r.RegisterTools(ctx, s, deps) +func (r *Inventory) RegisterAll(ctx context.Context, s *mcp.Server, deps any, middleware ...ToolHandlerMiddleware) { + r.RegisterTools(ctx, s, deps, middleware...) r.RegisterResourceTemplates(ctx, s, deps) r.RegisterPrompts(ctx, s) } diff --git a/pkg/inventory/server_tool.go b/pkg/inventory/server_tool.go index beb70138eb..44a062ba2e 100644 --- a/pkg/inventory/server_tool.go +++ b/pkg/inventory/server_tool.go @@ -18,6 +18,10 @@ import ( // should define their own typed dependencies struct and type-assert as needed. type HandlerFunc func(deps any) mcp.ToolHandler +// ToolHandlerMiddleware wraps an MCP tool handler. Middleware is applied from +// right to left, so the first middleware passed to RegisterFunc executes first. +type ToolHandlerMiddleware func(next mcp.ToolHandler) mcp.ToolHandler + // ToolsetID is a unique identifier for a toolset. // Using a distinct type provides compile-time type safety. type ToolsetID string @@ -110,8 +114,11 @@ func (st *ServerTool) Handler(deps any) mcp.ToolHandler { // Icons are automatically applied from the toolset metadata if not already set. // A shallow copy of the tool is made to avoid mutating the original ServerTool. // Panics if the tool has no handler - all tools should have handlers. -func (st *ServerTool) RegisterFunc(s *mcp.Server, deps any) { +func (st *ServerTool) RegisterFunc(s *mcp.Server, deps any, middleware ...ToolHandlerMiddleware) { handler := st.Handler(deps) // This will panic if HandlerFunc is nil + for i := len(middleware) - 1; i >= 0; i-- { + handler = middleware[i](handler) + } // Make a shallow copy of the tool to avoid mutating the original toolCopy := st.Tool // Apply icons from toolset metadata if tool doesn't have icons set diff --git a/pkg/inventory/server_tool_test.go b/pkg/inventory/server_tool_test.go index adf012b1f5..c6d2a6fdd8 100644 --- a/pkg/inventory/server_tool_test.go +++ b/pkg/inventory/server_tool_test.go @@ -81,6 +81,51 @@ func TestNewServerToolWithContextHandler_ValidArguments_Succeeds(t *testing.T) { assert.Equal(t, "success: octocat/hello-world", textContent.Text) } +func TestServerToolRegisterFuncAppliesMiddleware(t *testing.T) { + tool := NewServerTool( + mcp.Tool{ + Name: "wrapped_tool", + InputSchema: &jsonschema.Schema{Type: "object"}, + }, + testToolsetMetadata("test"), + func(_ context.Context, _ *mcp.CallToolRequest) (*mcp.CallToolResult, error) { + return &mcp.CallToolResult{ + Content: []mcp.Content{&mcp.TextContent{Text: "handler"}}, + }, nil + }, + ) + + middlewareCalled := make(chan struct{}, 1) + middleware := func(next mcp.ToolHandler) mcp.ToolHandler { + return func(ctx context.Context, req *mcp.CallToolRequest) (*mcp.CallToolResult, error) { + middlewareCalled <- struct{}{} + return next(ctx, req) + } + } + + server := mcp.NewServer(&mcp.Implementation{Name: "test-server", Version: "v0.0.1"}, nil) + tool.RegisterFunc(server, nil, middleware) + st, ct := mcp.NewInMemoryTransports() + ss, err := server.Connect(context.Background(), st, nil) + require.NoError(t, err) + t.Cleanup(func() { _ = ss.Close() }) + + client := mcp.NewClient(&mcp.Implementation{Name: "test-client", Version: "v0.0.1"}, nil) + cs, err := client.Connect(context.Background(), ct, nil) + require.NoError(t, err) + t.Cleanup(func() { _ = cs.Close() }) + + result, err := cs.CallTool(context.Background(), &mcp.CallToolParams{Name: "wrapped_tool"}) + require.NoError(t, err) + select { + case <-middlewareCalled: + default: + t.Fatal("tool middleware was not called") + } + require.Len(t, result.Content, 1) + assert.Equal(t, "handler", result.Content[0].(*mcp.TextContent).Text) +} + func TestAnnotateHeaderParams(t *testing.T) { tool := &mcp.Tool{InputSchema: &jsonschema.Schema{ Type: "object", diff --git a/third-party-licenses.darwin.md b/third-party-licenses.darwin.md index 88235f3f40..fca63f2256 100644 --- a/third-party-licenses.darwin.md +++ b/third-party-licenses.darwin.md @@ -24,8 +24,8 @@ The following packages are included for the amd64, arm64 architectures. - [github.com/josephburnett/jd/v2](https://pkg.go.dev/github.com/josephburnett/jd/v2) ([MIT](https://github.com/josephburnett/jd/blob/v2.5.0/v2/LICENSE)) - [github.com/lithammer/fuzzysearch/fuzzy](https://pkg.go.dev/github.com/lithammer/fuzzysearch/fuzzy) ([MIT](https://github.com/lithammer/fuzzysearch/blob/v1.1.8/LICENSE)) - [github.com/microcosm-cc/bluemonday](https://pkg.go.dev/github.com/microcosm-cc/bluemonday) ([BSD-3-Clause](https://github.com/microcosm-cc/bluemonday/blob/v1.0.27/LICENSE.md)) - - [github.com/modelcontextprotocol/go-sdk](https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk) ([Apache-2.0](https://github.com/modelcontextprotocol/go-sdk/blob/v1.7.0-pre.1/LICENSE)) - - [github.com/modelcontextprotocol/go-sdk](https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk) ([MIT](https://github.com/modelcontextprotocol/go-sdk/blob/v1.7.0-pre.1/LICENSE)) + - [github.com/modelcontextprotocol/go-sdk](https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk) ([Apache-2.0](https://github.com/modelcontextprotocol/go-sdk/blob/v1.7.0-pre.2/LICENSE)) + - [github.com/modelcontextprotocol/go-sdk](https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk) ([MIT](https://github.com/modelcontextprotocol/go-sdk/blob/v1.7.0-pre.2/LICENSE)) - [github.com/muesli/cache2go](https://pkg.go.dev/github.com/muesli/cache2go) ([BSD-3-Clause](https://github.com/muesli/cache2go/blob/518229cd8021/LICENSE.txt)) - [github.com/pelletier/go-toml/v2](https://pkg.go.dev/github.com/pelletier/go-toml/v2) ([MIT](https://github.com/pelletier/go-toml/blob/v2.2.4/LICENSE)) - [github.com/sagikazarmark/locafero](https://pkg.go.dev/github.com/sagikazarmark/locafero) ([MIT](https://github.com/sagikazarmark/locafero/blob/v0.11.0/LICENSE)) diff --git a/third-party-licenses.linux.md b/third-party-licenses.linux.md index e3762f5c04..dc3798c769 100644 --- a/third-party-licenses.linux.md +++ b/third-party-licenses.linux.md @@ -24,8 +24,8 @@ The following packages are included for the 386, amd64, arm64 architectures. - [github.com/josephburnett/jd/v2](https://pkg.go.dev/github.com/josephburnett/jd/v2) ([MIT](https://github.com/josephburnett/jd/blob/v2.5.0/v2/LICENSE)) - [github.com/lithammer/fuzzysearch/fuzzy](https://pkg.go.dev/github.com/lithammer/fuzzysearch/fuzzy) ([MIT](https://github.com/lithammer/fuzzysearch/blob/v1.1.8/LICENSE)) - [github.com/microcosm-cc/bluemonday](https://pkg.go.dev/github.com/microcosm-cc/bluemonday) ([BSD-3-Clause](https://github.com/microcosm-cc/bluemonday/blob/v1.0.27/LICENSE.md)) - - [github.com/modelcontextprotocol/go-sdk](https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk) ([Apache-2.0](https://github.com/modelcontextprotocol/go-sdk/blob/v1.7.0-pre.1/LICENSE)) - - [github.com/modelcontextprotocol/go-sdk](https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk) ([MIT](https://github.com/modelcontextprotocol/go-sdk/blob/v1.7.0-pre.1/LICENSE)) + - [github.com/modelcontextprotocol/go-sdk](https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk) ([Apache-2.0](https://github.com/modelcontextprotocol/go-sdk/blob/v1.7.0-pre.2/LICENSE)) + - [github.com/modelcontextprotocol/go-sdk](https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk) ([MIT](https://github.com/modelcontextprotocol/go-sdk/blob/v1.7.0-pre.2/LICENSE)) - [github.com/muesli/cache2go](https://pkg.go.dev/github.com/muesli/cache2go) ([BSD-3-Clause](https://github.com/muesli/cache2go/blob/518229cd8021/LICENSE.txt)) - [github.com/pelletier/go-toml/v2](https://pkg.go.dev/github.com/pelletier/go-toml/v2) ([MIT](https://github.com/pelletier/go-toml/blob/v2.2.4/LICENSE)) - [github.com/sagikazarmark/locafero](https://pkg.go.dev/github.com/sagikazarmark/locafero) ([MIT](https://github.com/sagikazarmark/locafero/blob/v0.11.0/LICENSE)) diff --git a/third-party-licenses.windows.md b/third-party-licenses.windows.md index eb0743558a..db7db1ec1b 100644 --- a/third-party-licenses.windows.md +++ b/third-party-licenses.windows.md @@ -25,8 +25,8 @@ The following packages are included for the 386, amd64, arm64 architectures. - [github.com/josephburnett/jd/v2](https://pkg.go.dev/github.com/josephburnett/jd/v2) ([MIT](https://github.com/josephburnett/jd/blob/v2.5.0/v2/LICENSE)) - [github.com/lithammer/fuzzysearch/fuzzy](https://pkg.go.dev/github.com/lithammer/fuzzysearch/fuzzy) ([MIT](https://github.com/lithammer/fuzzysearch/blob/v1.1.8/LICENSE)) - [github.com/microcosm-cc/bluemonday](https://pkg.go.dev/github.com/microcosm-cc/bluemonday) ([BSD-3-Clause](https://github.com/microcosm-cc/bluemonday/blob/v1.0.27/LICENSE.md)) - - [github.com/modelcontextprotocol/go-sdk](https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk) ([Apache-2.0](https://github.com/modelcontextprotocol/go-sdk/blob/v1.7.0-pre.1/LICENSE)) - - [github.com/modelcontextprotocol/go-sdk](https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk) ([MIT](https://github.com/modelcontextprotocol/go-sdk/blob/v1.7.0-pre.1/LICENSE)) + - [github.com/modelcontextprotocol/go-sdk](https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk) ([Apache-2.0](https://github.com/modelcontextprotocol/go-sdk/blob/v1.7.0-pre.2/LICENSE)) + - [github.com/modelcontextprotocol/go-sdk](https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk) ([MIT](https://github.com/modelcontextprotocol/go-sdk/blob/v1.7.0-pre.2/LICENSE)) - [github.com/muesli/cache2go](https://pkg.go.dev/github.com/muesli/cache2go) ([BSD-3-Clause](https://github.com/muesli/cache2go/blob/518229cd8021/LICENSE.txt)) - [github.com/pelletier/go-toml/v2](https://pkg.go.dev/github.com/pelletier/go-toml/v2) ([MIT](https://github.com/pelletier/go-toml/blob/v2.2.4/LICENSE)) - [github.com/sagikazarmark/locafero](https://pkg.go.dev/github.com/sagikazarmark/locafero) ([MIT](https://github.com/sagikazarmark/locafero/blob/v0.11.0/LICENSE))