diff --git a/README.md b/README.md index 73bd5f0e..1a06c069 100644 --- a/README.md +++ b/README.md @@ -311,7 +311,7 @@ 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. +For non-interactive stdio deployments, see **[GitHub App Authentication](docs/github-app-auth.md)**. **Or authenticate with a Personal Access Token.** Set `GITHUB_PERSONAL_ACCESS_TOKEN` instead (it takes precedence over OAuth): diff --git a/cmd/github-mcp-server/main.go b/cmd/github-mcp-server/main.go index 76298892..7671706b 100644 --- a/cmd/github-mcp-server/main.go +++ b/cmd/github-mcp-server/main.go @@ -40,11 +40,6 @@ 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") @@ -59,14 +54,13 @@ 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. App auth opts out of this - // default so configuring an app never accidentally enables OAuth login too. + // id with no secret never picks up the baked-in secret. if oauthClientID == "" && !appAuthRequested && oauth.NormalizeHost(viper.GetString("host")) == "https://github.com" { oauthClientID = buildinfo.OAuthClientID oauthClientSecret = buildinfo.OAuthClientSecret } 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") + return errors.New("authentication required: set GITHUB_PERSONAL_ACCESS_TOKEN, configure GitHub App auth, 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") @@ -137,7 +131,6 @@ 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. - // Skipped for GitHub App auth, which sources tokens non-interactively. if token == "" && !appAuthRequested { scopes := ghoauth.SupportedScopes if viper.IsSet("oauth-scopes") { @@ -156,15 +149,12 @@ 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")) + tokenProvider, err := newGitHubAppTokenProvider(appID, appInstallationID, appPrivateKeyPath, appPrivateKeyInline, viper.GetString("host")) if err != nil { return err } - stdioServerConfig.AppAuth = appConfig + stdioServerConfig.TokenProvider = tokenProvider } return ghmcp.RunStdioServer(stdioServerConfig) @@ -263,11 +253,7 @@ 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. + // The private key has no flag because passing it in argv would expose it. 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") @@ -326,19 +312,11 @@ 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) { +func newGitHubAppTokenProvider(appID, installationID, keyPath, keyInline, host string) (func() string, 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 { @@ -349,22 +327,18 @@ func buildAppAuthConfig(appID, installationID, keyPath, keyInline, host string) return nil, fmt.Errorf("failed to resolve REST URL for GitHub App authentication: %w", err) } - cfg := &githubapp.Config{ + provider, err := githubapp.NewProvider(githubapp.Config{ AppID: appID, InstallationID: installationID, - PrivateKey: privateKey, + PrivateKeyPEM: keyBytes, BaseRESTURL: restURL.String(), + }, nil) + if err != nil { + return nil, fmt.Errorf("failed to configure GitHub App authentication: %w", err) } - if err := cfg.Validate(); err != nil { - return nil, err - } - return cfg, nil + return provider.AccessToken, 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 != "": diff --git a/cmd/github-mcp-server/main_test.go b/cmd/github-mcp-server/main_test.go new file mode 100644 index 00000000..476f3087 --- /dev/null +++ b/cmd/github-mcp-server/main_test.go @@ -0,0 +1,38 @@ +package main + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestLoadAppPrivateKey(t *testing.T) { + t.Run("file", func(t *testing.T) { + path := filepath.Join(t.TempDir(), "app.pem") + require.NoError(t, os.WriteFile(path, []byte("from-file"), 0o600)) + + key, err := loadAppPrivateKey(path, "from-inline") + require.NoError(t, err) + assert.Equal(t, []byte("from-file"), key) + }) + + t.Run("inline", func(t *testing.T) { + key, err := loadAppPrivateKey("", `first\nsecond`) + require.NoError(t, err) + assert.Equal(t, []byte("first\nsecond"), key) + }) + + t.Run("missing", func(t *testing.T) { + _, err := loadAppPrivateKey("", "") + require.Error(t, err) + assert.Contains(t, err.Error(), "private key") + }) +} + +func TestGitHubAppFlagsAreStdioOnly(t *testing.T) { + assert.NotNil(t, stdioCmd.Flags().Lookup("app-id")) + assert.Nil(t, httpCmd.Flags().Lookup("app-id")) +} diff --git a/docs/github-app-auth.md b/docs/github-app-auth.md index ed9a3db1..f1da08c7 100644 --- a/docs/github-app-auth.md +++ b/docs/github-app-auth.md @@ -1,143 +1,34 @@ -# GitHub App Server-to-Server Authentication (stdio) +# GitHub App authentication -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. +The local stdio server can authenticate as a GitHub App installation without a +browser, device flow, or elicitation. It signs a short-lived JWT with the app's +private key, exchanges it for an installation access token, and refreshes the +token before it expires. + +This authentication mode is not available for the `http` command. HTTP clients +must continue to provide their own `Authorization` token. > [!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. +> The private key can mint tokens for every repository and permission granted to +> the installation. Keep it out of source control, restrict access to the server +> process, and install the app only on the repositories it needs. -## Contents +## Configuration -- [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. +Configure exactly one of a Personal Access Token, OAuth login, or GitHub App +authentication. | 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. | +| `--app-id` | `GITHUB_APP_ID` | App ID or client ID used as the JWT issuer | +| `--app-installation-id` | `GITHUB_APP_INSTALLATION_ID` | Installation whose access token is used | +| `--app-private-key-path` | `GITHUB_APP_PRIVATE_KEY_PATH` | Path to the private key PEM | +| _(none)_ | `GITHUB_APP_PRIVATE_KEY` | PEM contents, optionally with literal `\n` escapes | -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. +A mounted private-key file is preferred. There is no flag for inline PEM +contents because command-line arguments may be visible to other processes. -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: +## Usage ```bash github-mcp-server stdio \ @@ -146,7 +37,7 @@ github-mcp-server stdio \ --app-private-key-path /secrets/github-app.pem ``` -Equivalently, with environment variables: +The equivalent environment configuration is: ```bash export GITHUB_APP_ID=123456 @@ -155,7 +46,7 @@ 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): +For Docker, mount the key read-only: ```bash docker run -i --rm \ @@ -166,96 +57,17 @@ docker run -i --rm \ 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). +For GitHub Enterprise Server or `ghe.com`, also set `--gh-host` or +`GITHUB_HOST`. The server derives the installation-token endpoint from that +host. ## Troubleshooting -- **`GitHub App authentication requires a private key`** — you set some `app-*` - values but no key. Set `GITHUB_APP_PRIVATE_KEY_PATH` (preferred) or +- **Private key required**: set `GITHUB_APP_PRIVATE_KEY_PATH` 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. +- **Invalid private key**: provide the RSA PEM generated in the GitHub App + settings. PKCS#1 and PKCS#8 keys are supported. +- **401 from the installation-token endpoint**: verify the app ID or client ID, + private key, target host, and system clock. +- **404 from the installation-token endpoint**: verify the installation ID and + that the app is installed on the target host. diff --git a/docs/oauth-login.md b/docs/oauth-login.md index 31a0c90d..92fc79c9 100644 --- a/docs/oauth-login.md +++ b/docs/oauth-login.md @@ -15,12 +15,8 @@ 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. +> For non-interactive stdio deployments, see +> [GitHub App authentication](github-app-auth.md). ## Contents diff --git a/internal/ghmcp/oauth_test.go b/internal/ghmcp/oauth_test.go index 46c62d11..b3588762 100644 --- a/internal/ghmcp/oauth_test.go +++ b/internal/ghmcp/oauth_test.go @@ -9,7 +9,6 @@ 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" @@ -539,10 +538,6 @@ func TestOAuthMultiRoundTripResultType(t *testing.T) { 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() @@ -557,12 +552,12 @@ func TestRunStdioServerRejectsMultipleAuthModes(t *testing.T) { cfg: StdioServerConfig{Token: "ghp_static", OAuthManager: mgr}, }, { - name: "token and app", - cfg: StdioServerConfig{Token: "ghp_static", AppAuth: &githubapp.Config{}}, + name: "token and provider", + cfg: StdioServerConfig{Token: "ghp_static", TokenProvider: func() string { return "token" }}, }, { - name: "oauth and app", - cfg: StdioServerConfig{OAuthManager: mgr, AppAuth: &githubapp.Config{}}, + name: "oauth and provider", + cfg: StdioServerConfig{OAuthManager: mgr, TokenProvider: func() string { return "token" }}, }, } for _, tt := range tests { @@ -575,10 +570,8 @@ func TestRunStdioServerRejectsMultipleAuthModes(t *testing.T) { } } -// TestCreateGitHubClientsTokenProvider proves the OAuth wiring: when a -// TokenProvider is configured the REST client authenticates with the provider's -// current token on every request (and never pins a stale one), which is what the -// lazy, refreshing OAuth token depends on. +// TestCreateGitHubClientsTokenProvider verifies that clients resolve the +// provider for every request instead of pinning a token. func TestCreateGitHubClientsTokenProvider(t *testing.T) { t.Parallel() diff --git a/internal/ghmcp/server.go b/internal/ghmcp/server.go index 67c3b067..f13bdc47 100644 --- a/internal/ghmcp/server.go +++ b/internal/ghmcp/server.go @@ -12,7 +12,6 @@ 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" @@ -63,7 +62,7 @@ func createGitHubClients(cfg github.MCPServerConfig, apiHost utils.APIHostResolv return nil, fmt.Errorf("failed to get Raw URL: %w", err) } - // Construct REST client. When a TokenProvider is configured (OAuth), we + // Construct REST client. When a TokenProvider is configured, we // authenticate via BearerAuthTransport and skip go-github's WithAuthToken: // the latter installs its own round tripper that would pin the static token // and shadow the dynamic one. @@ -259,30 +258,20 @@ type StdioServerConfig struct { // 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 + // TokenProvider supplies a token for each GitHub API request. + TokenProvider func() string } // RunStdioServer is not concurrent safe. func RunStdioServer(cfg StdioServerConfig) error { - // 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} { + for _, on := range []bool{cfg.Token != "", cfg.OAuthManager != nil, cfg.TokenProvider != 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)") + return fmt.Errorf("choose exactly one authentication mode: a static Token, OAuthManager, or TokenProvider") } // Create app context @@ -307,20 +296,6 @@ 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 @@ -342,17 +317,11 @@ func RunStdioServer(cfg StdioServerConfig) error { logger.Debug("skipping scope filtering for non-PAT token") } - // 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 + tokenProvider := cfg.TokenProvider var toolHandlerMiddleware []inventory.ToolHandlerMiddleware - switch { - case cfg.OAuthManager != nil: + if 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{ diff --git a/internal/githubapp/githubapp.go b/internal/githubapp/githubapp.go index 49072b93..bdd04af2 100644 --- a/internal/githubapp/githubapp.go +++ b/internal/githubapp/githubapp.go @@ -1,23 +1,4 @@ -// 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 provides GitHub App installation access tokens. package githubapp import ( @@ -36,7 +17,6 @@ import ( "log/slog" "net/http" "net/url" - "os" "strings" "sync" "time" @@ -45,48 +25,35 @@ import ( ) 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. + jwtLifetime = 9 * time.Minute + clockSkew = time.Minute 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 + 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 is used as the JWT issuer. GitHub accepts an app ID or client ID. 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 + // PrivateKeyPEM is the RSA key used to sign app JWTs. + PrivateKeyPEM []byte // 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 { +func (c Config) validate() error { switch { case c.AppID == "": - return errors.New("GitHub App ID is required (GITHUB_APP_ID)") + return errors.New("GitHub App ID or client 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: + case len(c.PrivateKeyPEM) == 0: 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") @@ -94,10 +61,7 @@ func (c Config) Validate() error { 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) { +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") @@ -116,14 +80,12 @@ func ParsePrivateKey(pemBytes []byte) (*rsa.PrivateKey, error) { 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) { +func mintJWT(appID string, privateKey *rsa.PrivateKey, 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, + "iss": appID, } headerJSON, err := json.Marshal(header) @@ -139,7 +101,7 @@ func (c Config) mintJWT(now time.Time) (string, error) { base64.RawURLEncoding.EncodeToString(claimsJSON) digest := sha256.Sum256([]byte(signingInput)) - signature, err := rsa.SignPKCS1v15(rand.Reader, c.PrivateKey, crypto.SHA256, digest[:]) + signature, err := rsa.SignPKCS1v15(rand.Reader, privateKey, crypto.SHA256, digest[:]) if err != nil { return "", fmt.Errorf("signing JWT: %w", err) } @@ -147,25 +109,21 @@ func (c Config) mintJWT(now time.Time) (string, error) { 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 + privateKey *rsa.PrivateKey httpClient *http.Client } -func newInstallationTokenSource(cfg Config, httpClient *http.Client) *installationTokenSource { +func newInstallationTokenSource(cfg Config, privateKey *rsa.PrivateKey, httpClient *http.Client) *installationTokenSource { if httpClient == nil { httpClient = &http.Client{Timeout: httpTimeout} } - return &installationTokenSource{cfg: cfg, httpClient: httpClient} + return &installationTokenSource{cfg: cfg, privateKey: privateKey, 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()) + jwt, err := mintJWT(s.cfg.AppID, s.privateKey, time.Now()) if err != nil { return nil, err } @@ -193,9 +151,10 @@ func (s *installationTokenSource) Token() (*oauth2.Token, error) { 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)) + snippet, readErr := io.ReadAll(io.LimitReader(resp.Body, 512)) + if readErr != nil { + return nil, fmt.Errorf("installation token request failed: %s (reading response: %w)", resp.Status, readErr) + } return nil, fmt.Errorf("installation token request failed: %s: %s", resp.Status, strings.TrimSpace(string(snippet))) } @@ -209,21 +168,17 @@ func (s *installationTokenSource) Token() (*oauth2.Token, error) { 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) + if body.ExpiresAt.IsZero() { + return nil, errors.New("installation token response did not contain an expiry") } return &oauth2.Token{ AccessToken: body.Token, TokenType: "token", - Expiry: expiry, + Expiry: body.ExpiresAt.Add(-refreshBuffer), }, 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. +// Provider caches and refreshes GitHub App installation access tokens. type Provider struct { source oauth2.TokenSource logger *slog.Logger @@ -232,26 +187,22 @@ type Provider struct { 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 { + if err := cfg.validate(); err != nil { return nil, err } - if logger == nil { - logger = slog.New(slog.NewTextHandler(os.Stderr, nil)) + privateKey, err := parsePrivateKey(cfg.PrivateKeyPEM) + if err != nil { + return nil, fmt.Errorf("invalid GitHub App private key: %w", err) } - // 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)) + if logger == nil { + logger = slog.Default() + } + source := oauth2.ReuseTokenSource(nil, newInstallationTokenSource(cfg, privateKey, 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. +// AccessToken returns a cached token or refreshes it before expiry. func (p *Provider) AccessToken() string { tok, err := p.source.Token() if err != nil { @@ -268,8 +219,3 @@ func (p *Provider) AccessToken() string { 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 index c2d5eeff..6828dbc2 100644 --- a/internal/githubapp/githubapp_test.go +++ b/internal/githubapp/githubapp_test.go @@ -33,7 +33,7 @@ func newTestKey(t *testing.T) *rsa.PrivateKey { func pkcs1PEM(t *testing.T, key *rsa.PrivateKey) []byte { t.Helper() - return pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(key)}) + return pkcs1PEMBytes(key) } func pkcs8PEM(t *testing.T, key *rsa.PrivateKey) []byte { @@ -47,19 +47,19 @@ func TestParsePrivateKey(t *testing.T) { key := newTestKey(t) t.Run("PKCS1", func(t *testing.T) { - got, err := ParsePrivateKey(pkcs1PEM(t, key)) + 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)) + 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")) + _, err := parsePrivateKey([]byte("not a pem")) require.Error(t, err) assert.Contains(t, err.Error(), "no PEM block") }) @@ -71,7 +71,7 @@ func TestParsePrivateKey(t *testing.T) { require.NoError(t, err) keyPEM := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: der}) - _, err = ParsePrivateKey(keyPEM) + _, err = parsePrivateKey(keyPEM) require.Error(t, err) assert.Contains(t, err.Error(), "want an RSA key") }) @@ -79,24 +79,24 @@ func TestParsePrivateKey(t *testing.T) { 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()) + base := Config{AppID: "123", InstallationID: "456", PrivateKeyPEM: pkcs1PEM(t, 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 app id", func(c *Config) { c.AppID = "" }, "App ID or client 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 private key", func(c *Config) { c.PrivateKeyPEM = 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() + err := c.validate() require.Error(t, err) assert.Contains(t, err.Error(), tt.want) }) @@ -132,10 +132,8 @@ func verifyJWT(t *testing.T, token string, pub *rsa.PublicKey) map[string]any { func TestMintJWT(t *testing.T) { key := newTestKey(t) - cfg := Config{AppID: "my-app-id", PrivateKey: key} - now := time.Now() - token, err := cfg.mintJWT(now) + token, err := mintJWT("my-app-id", key, now) require.NoError(t, err) claims := verifyJWT(t, token, &key.PublicKey) @@ -173,7 +171,18 @@ func installationServer(t *testing.T, pub *rsa.PublicKey, token string, expiresA } func newTestConfig(key *rsa.PrivateKey, baseURL string) Config { - return Config{AppID: "123", InstallationID: "456", PrivateKey: key, BaseRESTURL: baseURL + "/"} + return Config{AppID: "123", InstallationID: "456", PrivateKeyPEM: pkcs1PEMBytes(key), BaseRESTURL: baseURL + "/"} +} + +func pkcs1PEMBytes(key *rsa.PrivateKey) []byte { + return pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(key)}) +} + +func newTestTokenSource(t *testing.T, cfg Config, client *http.Client) *installationTokenSource { + t.Helper() + privateKey, err := parsePrivateKey(cfg.PrivateKeyPEM) + require.NoError(t, err) + return newInstallationTokenSource(cfg, privateKey, client) } func TestProviderFetchesToken(t *testing.T) { @@ -184,7 +193,6 @@ func TestProviderFetchesToken(t *testing.T) { require.NoError(t, err) assert.Equal(t, "ghs_fresh", provider.AccessToken()) - assert.True(t, provider.HasToken()) assert.Equal(t, int32(1), calls.Load()) } @@ -242,7 +250,7 @@ func TestProviderErrorIncludesStatus(t *testing.T) { })) t.Cleanup(srv.Close) - source := newInstallationTokenSource(newTestConfig(key, srv.URL), srv.Client()) + source := newTestTokenSource(t, newTestConfig(key, srv.URL), srv.Client()) _, err := source.Token() require.Error(t, err) assert.Contains(t, err.Error(), "404") @@ -252,20 +260,31 @@ func TestProviderErrorIncludesStatus(t *testing.T) { func TestNewProviderValidates(t *testing.T) { _, err := NewProvider(Config{}, nil) require.Error(t, err) - assert.Contains(t, err.Error(), "App ID is required") + assert.Contains(t, err.Error(), "App ID or client ID is required") } -// Ensure the source returns an error rather than panicking on a token-less 201. -func TestSourceRejectsEmptyToken(t *testing.T) { +func TestSourceRejectsIncompleteTokenResponse(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) + tests := []struct { + name string + body string + want string + }{ + {name: "missing token", body: `{"expires_at":"2099-01-01T00:00:00Z"}`, want: "did not contain a token"}, + {name: "missing expiry", body: `{"token":"ghs_token"}`, want: "did not contain an expiry"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusCreated) + _, _ = fmt.Fprint(w, tt.body) + })) + 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") + source := newTestTokenSource(t, newTestConfig(key, srv.URL), srv.Client()) + _, err := source.Token() + require.Error(t, err) + assert.Contains(t, err.Error(), tt.want) + }) + } } diff --git a/pkg/github/server.go b/pkg/github/server.go index 67db83a7..43e09400 100644 --- a/pkg/github/server.go +++ b/pkg/github/server.go @@ -69,8 +69,7 @@ type MCPServerConfig struct { TokenScopes []string // TokenProvider, when non-nil, supplies the GitHub token for each API - // request instead of the static Token. It backs OAuth login, where the - // token is obtained lazily on first use and refreshed thereafter. + // request instead of the static Token. TokenProvider func() string // ToolHandlerMiddleware wraps every registered tool handler. Unlike MCP diff --git a/pkg/http/transport/bearer.go b/pkg/http/transport/bearer.go index 0c12ddfc..6f2ae7fc 100644 --- a/pkg/http/transport/bearer.go +++ b/pkg/http/transport/bearer.go @@ -13,9 +13,7 @@ type BearerAuthTransport struct { Token string // TokenProvider, when non-nil, supplies the bearer token for each request - // and takes precedence over Token. It backs OAuth, where the token is - // obtained after the client is built and is refreshed over the session's - // lifetime. It may return an empty string before authorization completes. + // and takes precedence over Token. TokenProvider func() string } @@ -25,8 +23,6 @@ func (t *BearerAuthTransport) RoundTrip(req *http.Request) (*http.Response, erro if t.TokenProvider != nil { token = t.TokenProvider() } - // Before OAuth authorization completes the token is empty; send an - // unauthenticated request rather than an empty "Bearer " header. if token != "" { req.Header.Set(headers.AuthorizationHeader, "Bearer "+token) }