diff --git a/docs/modelcontextprotocol-io/authentication.mdx b/docs/modelcontextprotocol-io/authentication.mdx index 78bf23a1f..98131d5db 100644 --- a/docs/modelcontextprotocol-io/authentication.mdx +++ b/docs/modelcontextprotocol-io/authentication.mdx @@ -47,6 +47,14 @@ Successfully authenticated! ✓ Successfully logged in ``` +### Personal vs. organization namespaces + +GitHub authentication always grants your personal namespace, `io.github./*`. + +To publish under an **organization** namespace (`io.github./*`), you must be an **Owner** of that organization. Ordinary org membership is no longer sufficient: the registry checks your membership role and only grants the org namespace to admins. This prevents anyone who merely belongs to an org from publishing — or overwriting — servers under the org's name. + +If you authenticate with a Personal Access Token (for example in CI), the token must include the `read:org` scope for the registry to see your organization role. A token without `read:org` can still publish to your personal namespace, but org publishing will be silently unavailable. The token needs **no** repository scopes — the registry never reads or writes your code. + ## DNS Authentication DNS authentication is a domain-based authentication method that relies on a DNS TXT record. diff --git a/docs/modelcontextprotocol-io/github-actions.mdx b/docs/modelcontextprotocol-io/github-actions.mdx index 0da50cdf8..b28827e4b 100644 --- a/docs/modelcontextprotocol-io/github-actions.mdx +++ b/docs/modelcontextprotocol-io/github-actions.mdx @@ -81,6 +81,10 @@ on: jobs: publish: runs-on: ubuntu-latest + # Source MCP_GITHUB_TOKEN from a protected Environment, not a plain repo + # secret, and restrict that environment to your default branch / release + # tags. See "Securing your registry token in CI" below for why this matters. + environment: mcp-registry-publish permissions: contents: read @@ -195,13 +199,30 @@ jobs: You may need to add a secret to the repository depending on which authentication method you choose: - **GitHub OIDC Authentication**: No dedicated secret necessary. -- **GitHub PAT Authentication**: Add a `MCP_GITHUB_TOKEN` secret with a GitHub Personal Access Token (PAT) that has `read:org` and `read:user` scopes. +- **GitHub PAT Authentication**: Add a `MCP_GITHUB_TOKEN` secret with a GitHub Personal Access Token (PAT) that has `read:org` and `read:user` scopes. The `read:org` scope is what lets the registry confirm you are an **Owner** of the organization before granting its namespace; the token needs **no** repository scopes (the registry never reads or writes your code). - **DNS Authentication**: Add a `MCP_PRIVATE_KEY` secret with your Ed25519 private key. You may also need to add secrets for your package registry. For example, the workflow above needs an `NPM_TOKEN` secret with your npm token. For information about how to add secrets to a repository, see [Using secrets in GitHub Actions](https://docs.github.com/en/actions/how-tos/write-workflows/choose-what-workflows-do/use-secrets). +### Securing your registry token in CI + +Whichever token you use to authenticate, treat it as a high-value credential. When you authenticate to an **organization** namespace, the resulting registry token can publish — and overwrite — **any** server under `io.github./*`, not just the one in this repository. The relevant question for your threat model is therefore: *who can cause code to run in a job where this secret is exposed?* By default that is **every repository writer** (via branch or tag pushes), not only org Owners — so a plain repo secret quietly widens publish access to everyone with write access. + +GitHub gives you the tools to close that gap. We recommend, in increasing order of strength: + +1. **Store the token as an [Environment secret](https://docs.github.com/en/actions/how-tos/deploy/configure-and-manage-deployments/manage-environments), not a repository or organization secret.** A job only receives it when it declares `environment:` (as the PAT example above does). +2. **Restrict that environment to your default branch and/or release tags** with a [deployment branch rule](https://docs.github.com/en/actions/how-tos/deploy/configure-and-manage-deployments/manage-environments#deployment-branches-and-tags), and protect that branch with [required pull request reviews](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/about-protected-branches). Now only reviewed, maintainer-approved code can ever reach the token. +3. **(Strongest) Add a [required reviewer](https://docs.github.com/en/actions/how-tos/deploy/configure-and-manage-deployments/manage-environments#required-reviewers) to the environment** so each publish pauses for an explicit human approval. + +Two pitfalls to avoid regardless of the above: + +- **Never publish from a `pull_request_target` workflow that checks out PR-head code.** That trigger runs with your secrets in the base-repo context, so an untrusted fork PR could exfiltrate the token. Environment branch rules do **not** protect you here (the ref is still your base branch) — only required reviewers do. +- **Avoid self-hosted runners for the publish job on public repositories**, where fork PRs may be able to schedule jobs onto them. + +By default, pull requests **from forks** receive no secrets and cannot push branches, so external contributors cannot reach the token without one of the misconfigurations above. + ## Step 3: Tag and Release Create and push a version tag to trigger the workflow: diff --git a/internal/api/handlers/v0/auth/github_at.go b/internal/api/handlers/v0/auth/github_at.go index af5e7931a..4960fb46f 100644 --- a/internal/api/handlers/v0/auth/github_at.go +++ b/internal/api/handlers/v0/auth/github_at.go @@ -74,8 +74,9 @@ func (h *GitHubHandler) ExchangeToken(ctx context.Context, githubToken string) ( return nil, fmt.Errorf("failed to get GitHub user: %w", err) } - // Get user's organizations - orgs, err := h.getGitHubUserOrgs(ctx, user.Login, githubToken) + // Get the organizations the user administers. Org namespaces are only granted + // to org Owners (membership role "admin"), not to ordinary members. + orgs, err := h.getGitHubAdminOrgs(ctx, githubToken) if err != nil { return nil, fmt.Errorf("failed to get GitHub organizations: %w", err) } @@ -133,8 +134,68 @@ func (h *GitHubHandler) getGitHubUser(ctx context.Context, token string) (*GitHu return &user, nil } -func (h *GitHubHandler) getGitHubUserOrgs(ctx context.Context, username string, token string) ([]GitHubUserOrOrg, error) { - req, err := http.NewRequestWithContext(ctx, http.MethodGet, h.baseURL+"/users/"+username+"/orgs", nil) +// githubOrgRoleAdmin is the membership role GitHub returns for an organization +// Owner. It is the only role we treat as carrying publish authority for the org +// namespace. (GitHub has no org-level "maintainer" role. GET /user/memberships/orgs +// returns role "admin" (Owner), "member", or "billing_manager"; only "admin" +// carries publish authority, so the other two are intentionally not granted.) +const githubOrgRoleAdmin = "admin" + +// orgMembershipsPageSize is the page size we request when listing the user's org +// memberships. 100 is GitHub's maximum. +const orgMembershipsPageSize = 100 + +// githubOrgMembership is one entry from GET /user/memberships/orgs: the +// authenticated user's membership in a single organization, including their role. +type githubOrgMembership struct { + State string `json:"state"` + Role string `json:"role"` + Organization GitHubUserOrOrg `json:"organization"` +} + +// getGitHubAdminOrgs returns the organizations in which the authenticated user is +// an Owner (membership role "admin"). +// +// This deliberately uses GET /user/memberships/orgs rather than +// GET /users/{username}/orgs. The latter only returns *public* memberships and +// carries no role, so it cannot distinguish an Owner from an ordinary member — +// historically every org a user belonged to was granted publish access to its +// namespace regardless of the user's role. The memberships endpoint returns the +// caller's role per org (and includes private memberships), letting us grant the +// org namespace only to people who actually administer the org. +// +// The endpoint requires the read:org scope. A minimal token used only for +// personal-namespace publishing won't have it and will get a 403 here; that is +// treated as "no admin orgs" (see fetchOrgMembershipsPage) so personal publishing +// keeps working without asking users to over-scope their token. +func (h *GitHubHandler) getGitHubAdminOrgs(ctx context.Context, token string) ([]GitHubUserOrOrg, error) { + var adminOrgs []GitHubUserOrOrg + + for page := 1; ; page++ { + memberships, err := h.fetchOrgMembershipsPage(ctx, token, page) + if err != nil { + return nil, err + } + + for _, m := range memberships { + if m.Role == githubOrgRoleAdmin { + adminOrgs = append(adminOrgs, m.Organization) + } + } + + if len(memberships) < orgMembershipsPageSize { + break + } + } + + return adminOrgs, nil +} + +// fetchOrgMembershipsPage fetches a single page of the authenticated user's +// active organization memberships. +func (h *GitHubHandler) fetchOrgMembershipsPage(ctx context.Context, token string, page int) ([]githubOrgMembership, error) { + url := fmt.Sprintf("%s/user/memberships/orgs?state=active&per_page=%d&page=%d", h.baseURL, orgMembershipsPageSize, page) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { return nil, fmt.Errorf("failed to create request: %w", err) } @@ -145,20 +206,37 @@ func (h *GitHubHandler) getGitHubUserOrgs(ctx context.Context, username string, resp, err := http.DefaultClient.Do(req) if err != nil { - return nil, fmt.Errorf("failed to get user organizations: %w", err) + return nil, fmt.Errorf("failed to get user organization memberships: %w", err) } defer resp.Body.Close() + // GitHub returns 403 here for two very different reasons, which we must not + // conflate: + // 1. The token lacks the read:org scope. This is the common, benign case for a + // minimal personal-publishing token: GET /user still works, but org + // memberships are forbidden. We degrade gracefully to "no admin orgs" so + // personal-namespace publishing keeps working without over-scoping. + // 2. Rate limiting. GitHub signals primary/secondary rate limits with 403 (or + // 429), setting X-RateLimit-Remaining: 0 and/or Retry-After. Degrading here + // would silently strip a legitimate Owner's org grant, so we fail closed + // (return an error) rather than mistake a throttle for "not an admin". + if resp.StatusCode == http.StatusForbidden { + if resp.Header.Get("X-RateLimit-Remaining") == "0" || resp.Header.Get("Retry-After") != "" { + return nil, fmt.Errorf("GitHub API rate limit exceeded while listing org memberships (status 403): %s", readErrorBody(resp.Body)) + } + return nil, nil + } + if resp.StatusCode != http.StatusOK { return nil, fmt.Errorf("GitHub API error (status %d): %s", resp.StatusCode, readErrorBody(resp.Body)) } - var orgs []GitHubUserOrOrg - if err := json.NewDecoder(resp.Body).Decode(&orgs); err != nil { - return nil, fmt.Errorf("failed to decode organizations response: %w", err) + var memberships []githubOrgMembership + if err := json.NewDecoder(resp.Body).Decode(&memberships); err != nil { + return nil, fmt.Errorf("failed to decode organization memberships response: %w", err) } - return orgs, nil + return memberships, nil } // buildPermissions builds permissions based on GitHub user and their organizations @@ -181,7 +259,7 @@ func (h *GitHubHandler) buildPermissions(username string, orgs []GitHubUserOrOrg ResourcePattern: fmt.Sprintf("io.github.%s/*", username), }) - // Add permissions for each organization + // Add permissions for each organization the user administers (Owner role) for _, org := range orgs { permissions = append(permissions, auth.Permission{ Action: auth.PermissionActionPublish, diff --git a/internal/api/handlers/v0/auth/github_at_test.go b/internal/api/handlers/v0/auth/github_at_test.go index c0fa4ac97..f784b4ee9 100644 --- a/internal/api/handlers/v0/auth/github_at_test.go +++ b/internal/api/handlers/v0/auth/github_at_test.go @@ -22,9 +22,29 @@ import ( const ( githubUserEndpoint = "/user" - githubOrgsEndpoint = "/users/testuser/orgs" + // githubOrgsEndpoint is the endpoint used to list the authenticated user's org + // memberships (with role). It replaces the old public-only /users/{user}/orgs. + githubOrgsEndpoint = "/user/memberships/orgs" ) +// orgMembership mirrors a single entry of GET /user/memberships/orgs for use in +// mock GitHub API responses. Role is "admin" (Owner) or "member". +type orgMembership struct { + State string `json:"state"` + Role string `json:"role"` + Organization v0auth.GitHubUserOrOrg `json:"organization"` +} + +// adminMemberships wraps orgs as active "admin"-role memberships, the shape the +// memberships endpoint returns for an org Owner. +func adminMemberships(orgs []v0auth.GitHubUserOrOrg) []orgMembership { + memberships := make([]orgMembership, 0, len(orgs)) + for _, org := range orgs { + memberships = append(memberships, orgMembership{State: "active", Role: "admin", Organization: org}) + } + return memberships +} + func TestGitHubHandler_ExchangeToken(t *testing.T) { // Create test handler with mock config testSeed := make([]byte, ed25519.SeedSize) @@ -96,12 +116,12 @@ func TestGitHubHandler_ExchangeToken(t *testing.T) { w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(user) //nolint:errcheck case githubOrgsEndpoint: - orgs := []v0auth.GitHubUserOrOrg{ + memberships := adminMemberships([]v0auth.GitHubUserOrOrg{ {Login: "test-org-1", ID: 1}, {Login: "test-org-2", ID: 2}, - } + }) w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(orgs) //nolint:errcheck + json.NewEncoder(w).Encode(memberships) //nolint:errcheck default: w.WriteHeader(http.StatusNotFound) } @@ -225,10 +245,9 @@ func TestGitHubHandler_ExchangeToken(t *testing.T) { } w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(user) //nolint:errcheck - case "/users/user with spaces/orgs": - orgs := []v0auth.GitHubUserOrOrg{} + case githubOrgsEndpoint: w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(orgs) //nolint:errcheck + json.NewEncoder(w).Encode([]orgMembership{}) //nolint:errcheck } })) defer mockServer.Close() @@ -264,12 +283,12 @@ func TestGitHubHandler_ExchangeToken(t *testing.T) { w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(user) //nolint:errcheck case githubOrgsEndpoint: - orgs := []v0auth.GitHubUserOrOrg{ + memberships := adminMemberships([]v0auth.GitHubUserOrOrg{ {Login: "valid-org", ID: 1}, {Login: "org with spaces", ID: 2}, // Invalid name - } + }) w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(orgs) //nolint:errcheck + json.NewEncoder(w).Encode(memberships) //nolint:errcheck } })) defer mockServer.Close() @@ -317,6 +336,198 @@ func TestGitHubHandler_ExchangeToken(t *testing.T) { }) } +func TestGitHubHandler_ExchangeToken_OrgRoles(t *testing.T) { + testSeed := make([]byte, ed25519.SeedSize) + _, err := rand.Read(testSeed) + require.NoError(t, err) + + cfg := &config.Config{ + JWTPrivateKey: hex.EncodeToString(testSeed), + } + + t.Run("member-role org is not granted, only admin orgs", func(t *testing.T) { + // The user is an Owner (admin) of one org and an ordinary member of another. + // Only the org they administer should yield a publish permission. + mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case githubUserEndpoint: + user := v0auth.GitHubUserOrOrg{Login: "testuser", ID: 12345} + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(user) //nolint:errcheck + case githubOrgsEndpoint: + memberships := []orgMembership{ + {State: "active", Role: "admin", Organization: v0auth.GitHubUserOrOrg{Login: "admin-org", ID: 1}}, + {State: "active", Role: "member", Organization: v0auth.GitHubUserOrOrg{Login: "member-org", ID: 2}}, + } + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(memberships) //nolint:errcheck + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer mockServer.Close() + + handler := v0auth.NewGitHubHandler(cfg) + handler.SetBaseURL(mockServer.URL) + + ctx := context.Background() + response, err := handler.ExchangeToken(ctx, "valid-github-token") + require.NoError(t, err) + require.NotNil(t, response) + + jwtManager := auth.NewJWTManager(cfg) + claims, err := jwtManager.ValidateToken(ctx, response.RegistryToken) + require.NoError(t, err) + + patterns := make([]string, 0, len(claims.Permissions)) + for _, perm := range claims.Permissions { + patterns = append(patterns, perm.ResourcePattern) + } + assert.ElementsMatch(t, []string{"io.github.testuser/*", "io.github.admin-org/*"}, patterns) + assert.NotContains(t, patterns, "io.github.member-org/*") + }) + + t.Run("missing read:org scope (403) still grants personal namespace", func(t *testing.T) { + // A minimal token without read:org authenticates fine but is forbidden from + // reading org memberships. Personal-namespace publishing must still work. + mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case githubUserEndpoint: + user := v0auth.GitHubUserOrOrg{Login: "testuser", ID: 12345} + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(user) //nolint:errcheck + case githubOrgsEndpoint: + w.WriteHeader(http.StatusForbidden) + w.Write([]byte(`{"message": "Token does not have the required scope"}`)) //nolint:errcheck + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer mockServer.Close() + + handler := v0auth.NewGitHubHandler(cfg) + handler.SetBaseURL(mockServer.URL) + + ctx := context.Background() + response, err := handler.ExchangeToken(ctx, "valid-github-token") + require.NoError(t, err) + require.NotNil(t, response) + + jwtManager := auth.NewJWTManager(cfg) + claims, err := jwtManager.ValidateToken(ctx, response.RegistryToken) + require.NoError(t, err) + assert.Len(t, claims.Permissions, 1) + assert.Equal(t, "io.github.testuser/*", claims.Permissions[0].ResourcePattern) + }) + + t.Run("admin org on a later page is still granted (pagination)", func(t *testing.T) { + // Page 1 is a full page (100) of member-role orgs; the only admin org is on + // page 2. The pagination loop must not stop at the first page, or the Owner's + // org grant would be silently dropped. + mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case githubUserEndpoint: + user := v0auth.GitHubUserOrOrg{Login: "testuser", ID: 12345} + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(user) //nolint:errcheck + case githubOrgsEndpoint: + w.Header().Set("Content-Type", "application/json") + if r.URL.Query().Get("page") == "1" { + page1 := make([]orgMembership, 0, 100) + for i := 0; i < 100; i++ { + page1 = append(page1, orgMembership{ + State: "active", Role: "member", + Organization: v0auth.GitHubUserOrOrg{Login: fmt.Sprintf("member-org-%d", i), ID: 1000 + i}, + }) + } + json.NewEncoder(w).Encode(page1) //nolint:errcheck + return + } + // page 2: a single admin org, signalling the last (short) page. + page2 := []orgMembership{ + {State: "active", Role: "admin", Organization: v0auth.GitHubUserOrOrg{Login: "late-admin-org", ID: 2}}, + } + json.NewEncoder(w).Encode(page2) //nolint:errcheck + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer mockServer.Close() + + handler := v0auth.NewGitHubHandler(cfg) + handler.SetBaseURL(mockServer.URL) + + ctx := context.Background() + response, err := handler.ExchangeToken(ctx, "valid-github-token") + require.NoError(t, err) + require.NotNil(t, response) + + jwtManager := auth.NewJWTManager(cfg) + claims, err := jwtManager.ValidateToken(ctx, response.RegistryToken) + require.NoError(t, err) + + patterns := make([]string, 0, len(claims.Permissions)) + for _, perm := range claims.Permissions { + patterns = append(patterns, perm.ResourcePattern) + } + assert.ElementsMatch(t, []string{"io.github.testuser/*", "io.github.late-admin-org/*"}, patterns) + }) + + t.Run("memberships server error fails closed (no token issued)", func(t *testing.T) { + // A 5xx (or any non-200, non-scope-403) from the memberships endpoint must + // abort the exchange rather than silently degrade to personal-only perms. + mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case githubUserEndpoint: + user := v0auth.GitHubUserOrOrg{Login: "testuser", ID: 12345} + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(user) //nolint:errcheck + case githubOrgsEndpoint: + w.WriteHeader(http.StatusInternalServerError) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer mockServer.Close() + + handler := v0auth.NewGitHubHandler(cfg) + handler.SetBaseURL(mockServer.URL) + + ctx := context.Background() + response, err := handler.ExchangeToken(ctx, "valid-github-token") + require.Error(t, err) + assert.Nil(t, response) + }) + + t.Run("rate-limit 403 fails closed (not treated as missing scope)", func(t *testing.T) { + // A 403 with X-RateLimit-Remaining: 0 is a throttle, not a missing-scope + // signal. Degrading would strip a real Owner's org grant, so it must error. + mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case githubUserEndpoint: + user := v0auth.GitHubUserOrOrg{Login: "testuser", ID: 12345} + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(user) //nolint:errcheck + case githubOrgsEndpoint: + w.Header().Set("X-RateLimit-Remaining", "0") + w.WriteHeader(http.StatusForbidden) + w.Write([]byte(`{"message": "API rate limit exceeded"}`)) //nolint:errcheck + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer mockServer.Close() + + handler := v0auth.NewGitHubHandler(cfg) + handler.SetBaseURL(mockServer.URL) + + ctx := context.Background() + response, err := handler.ExchangeToken(ctx, "valid-github-token") + require.Error(t, err) + assert.Nil(t, response) + }) +} + func TestJWTTokenValidation(t *testing.T) { testSeed := make([]byte, ed25519.SeedSize) _, err := rand.Read(testSeed) @@ -537,9 +748,9 @@ func TestValidGitHubNames(t *testing.T) { } w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(user) //nolint:errcheck - case "/users/" + tc.username + "/orgs": + case githubOrgsEndpoint: w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(tc.orgs) //nolint:errcheck + json.NewEncoder(w).Encode(adminMemberships(tc.orgs)) //nolint:errcheck } })) defer mockServer.Close()