fix(repos): require protected confirmation state

Give stdio a process-local request-state sealer and make deletion fail closed without one. Preserve legacy any-of OAuth behavior globally while documenting and enforcing delete_repository's conjunctive delete_repo and repo requirements.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 4b04480c-c2e9-483e-9b0f-34830b76a2f8

Author: SamMorrowDrums

README.md

@@ -1309,7 +1309,7 @@ The following sets of tools are available:
   - `repo`: Repository name (string, required)
 
 - **delete_repository** - Delete repository
-  - **Required OAuth Scopes (any of)**: `delete_repo`, `repo`
+  - **Required OAuth Scopes (all required)**: `delete_repo`, `repo`
   - `owner`: Repository owner (username or organization) (string, required)
   - `repo`: Repository name (string, required)
 

cmd/github-mcp-server/generate_docs.go

@@ -221,13 +221,13 @@ func writeToolDoc(buf *strings.Builder, tool inventory.ServerTool) {
 
 	// OAuth scopes if present
 	if len(tool.RequiredScopes) > 0 {
-		// Scope filtering uses "any of" semantics (see scopes.HasRequiredScopes),
-		// so when multiple required scopes are listed, render them as alternatives
-		// rather than implying all are required.
 		scopeList := "`" + strings.Join(tool.RequiredScopes, "`, `") + "`"
-		if len(tool.RequiredScopes) > 1 {
+		switch {
+		case len(tool.RequiredScopeGroups) > 1:
+			fmt.Fprintf(buf, "  - **Required OAuth Scopes (all required)**: %s\n", scopeList)
+		case len(tool.RequiredScopes) > 1:
 			fmt.Fprintf(buf, "  - **Required OAuth Scopes (any of)**: %s\n", scopeList)
-		} else {
+		default:
 			fmt.Fprintf(buf, "  - **Required OAuth Scopes**: %s\n", scopeList)
 		}
 

cmd/github-mcp-server/main_test.go

@@ -3,9 +3,12 @@ package main
 import (
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
+	"github.com/github/github-mcp-server/pkg/inventory"
 	"github.com/google/jsonschema-go/jsonschema"
+	"github.com/modelcontextprotocol/go-sdk/mcp"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -38,6 +41,40 @@ func TestGitHubAppFlagsAreStdioOnly(t *testing.T) {
 	assert.Nil(t, httpCmd.Flags().Lookup("app-id"))
 }
 
+func TestWriteToolDocScopeSemantics(t *testing.T) {
+	tests := []struct {
+		name string
+		tool inventory.ServerTool
+		want string
+	}{
+		{
+			name: "legacy multi-scope tools use any-of",
+			tool: inventory.ServerTool{
+				Tool:           mcp.Tool{Name: "legacy", Annotations: &mcp.ToolAnnotations{Title: "Legacy"}},
+				RequiredScopes: []string{"repo", "read:org"},
+			},
+			want: "**Required OAuth Scopes (any of)**",
+		},
+		{
+			name: "conjunctive scope groups use all-required",
+			tool: inventory.ServerTool{
+				Tool:                mcp.Tool{Name: "conjunctive", Annotations: &mcp.ToolAnnotations{Title: "Conjunctive"}},
+				RequiredScopes:      []string{"delete_repo", "repo"},
+				RequiredScopeGroups: [][]string{{"delete_repo"}, {"repo"}},
+			},
+			want: "**Required OAuth Scopes (all required)**",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var buf strings.Builder
+			writeToolDoc(&buf, tt.tool)
+			assert.Contains(t, buf.String(), tt.want)
+		})
+	}
+}
+
 func TestSchemaTypeString(t *testing.T) {
 	tests := []struct {
 		name   string

internal/ghmcp/server.go

@@ -13,6 +13,7 @@ import (
 	"time"
 
 	"github.com/github/github-mcp-server/internal/oauth"
+	"github.com/github/github-mcp-server/internal/requeststate"
 	"github.com/github/github-mcp-server/pkg/errors"
 	"github.com/github/github-mcp-server/pkg/github"
 	"github.com/github/github-mcp-server/pkg/http/transport"
@@ -169,6 +170,10 @@ func NewStdioMCPServer(ctx context.Context, cfg github.MCPServerConfig) (*mcp.Se
 		featureChecker,
 		obs,
 	)
+	deps.StateSealer, err = requeststate.NewRandom()
+	if err != nil {
+		return nil, fmt.Errorf("failed to configure request-state protection: %w", err)
+	}
 	// Build and register the tool/resource/prompt inventory
 	inventoryBuilder := github.NewInventory(cfg.Translator, github.WithHost(hostType)).
 		WithDeprecatedAliases(github.DeprecatedToolAliases).

internal/requeststate/sealer.go

@@ -17,6 +17,15 @@ type Sealer struct {
 	aead cipher.AEAD
 }
 
+// NewRandom constructs a sealer with a process-local random key.
+func NewRandom() (*Sealer, error) {
+	key := make([]byte, keySize)
+	if _, err := rand.Read(key); err != nil {
+		return nil, fmt.Errorf("generating key: %w", err)
+	}
+	return newFromKey(key)
+}
+
 // New constructs a sealer from a standard Base64-encoded 32-byte key.
 func New(encodedKey string) (*Sealer, error) {
 	key, err := base64.StdEncoding.DecodeString(encodedKey)
@@ -26,6 +35,10 @@ func New(encodedKey string) (*Sealer, error) {
 	if len(key) != keySize {
 		return nil, fmt.Errorf("decoded key must be %d bytes, got %d", keySize, len(key))
 	}
+	return newFromKey(key)
+}
+
+func newFromKey(key []byte) (*Sealer, error) {
 	block, err := aes.NewCipher(key)
 	if err != nil {
 		return nil, fmt.Errorf("creating cipher: %w", err)

internal/requeststate/sealer_test.go

@@ -39,6 +39,16 @@ func TestSealer(t *testing.T) {
 	})
 }
 
+func TestNewRandom(t *testing.T) {
+	sealer, err := NewRandom()
+	require.NoError(t, err)
+	token, err := sealer.Seal(context.Background(), []byte("state"))
+	require.NoError(t, err)
+	opened, err := sealer.Open(token)
+	require.NoError(t, err)
+	assert.Equal(t, []byte("state"), opened)
+}
+
 func TestNew(t *testing.T) {
 	tests := []struct {
 		name string

pkg/github/dependencies.go

@@ -245,7 +245,6 @@ func NewTool[In, Out any](
 	})
 	st.RequiredScopes = scopes.ToStringSlice(requiredScopes...)
 	st.AcceptedScopes = scopes.ExpandScopes(requiredScopes...)
-	st.RequiredScopeGroups = scopes.ExpandScopeGroups(requiredScopes...)
 	return st
 }
 
@@ -269,7 +268,6 @@ func NewToolFromHandler(
 	})
 	st.RequiredScopes = scopes.ToStringSlice(requiredScopes...)
 	st.AcceptedScopes = scopes.ExpandScopes(requiredScopes...)
-	st.RequiredScopeGroups = scopes.ExpandScopeGroups(requiredScopes...)
 	return st
 }
 

pkg/github/repositories.go

@@ -760,36 +760,36 @@ func DeleteRepository(t translations.TranslationHelperFunc) inventory.ServerTool
 
 			fullName := owner + "/" + repo
 			sealer := requestStateSealerFromDeps(deps)
-			var deletionState *deleteRepositoryState
+			if sealer == nil {
+				return utils.NewToolResultError("Repository deletion is unavailable because request-state protection is not configured."), nil, nil
+			}
+			var deletionState deleteRepositoryState
 			var responses mcp.InputResponseMap
 			if req != nil && req.Params != nil {
 				responses = req.Params.InputResponses
 			}
 			response, ok := responses[deleteRepositoryConfirmationID]
 			if !ok {
-				var requestState string
-				if sealer != nil {
-					client, err := deps.GetClient(ctx)
-					if err != nil {
-						return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err)
-					}
-					repositoryID, result := repositoryIDForDeletion(ctx, client, owner, repo)
-					if result != nil {
-						return result, nil, nil
-					}
-					state, err := json.Marshal(deleteRepositoryState{
-						Owner:        owner,
-						Repo:         repo,
-						RepositoryID: repositoryID,
-						ExpiresAt:    time.Now().Add(deleteRepositoryConfirmationTTL).Unix(),
-					})
-					if err != nil {
-						return nil, nil, fmt.Errorf("failed to marshal repository deletion state: %w", err)
-					}
-					requestState, err = sealer.Seal(ctx, state)
-					if err != nil {
-						return nil, nil, fmt.Errorf("failed to seal repository deletion state: %w", err)
-					}
+				client, err := deps.GetClient(ctx)
+				if err != nil {
+					return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err)
+				}
+				repositoryID, result := repositoryIDForDeletion(ctx, client, owner, repo)
+				if result != nil {
+					return result, nil, nil
+				}
+				state, err := json.Marshal(deleteRepositoryState{
+					Owner:        owner,
+					Repo:         repo,
+					RepositoryID: repositoryID,
+					ExpiresAt:    time.Now().Add(deleteRepositoryConfirmationTTL).Unix(),
+				})
+				if err != nil {
+					return nil, nil, fmt.Errorf("failed to marshal repository deletion state: %w", err)
+				}
+				requestState, err := sealer.Seal(ctx, state)
+				if err != nil {
+					return nil, nil, fmt.Errorf("failed to seal repository deletion state: %w", err)
 				}
 				return &mcp.CallToolResult{
 					InputRequests: mcp.InputRequestMap{
@@ -813,28 +813,24 @@ func DeleteRepository(t translations.TranslationHelperFunc) inventory.ServerTool
 				}, nil, nil
 			}
 
-			if sealer != nil {
-				if req.Params.RequestState == "" {
-					return utils.NewToolResultError("Repository deletion confirmation state was missing. The repository was not deleted."), nil, nil
-				}
-				stateJSON, err := sealer.Open(req.Params.RequestState)
-				if err != nil {
-					return utils.NewToolResultError("Repository deletion confirmation state was invalid. The repository was not deleted."), nil, nil
-				}
-				var state deleteRepositoryState
-				if err := json.Unmarshal(stateJSON, &state); err != nil {
-					return utils.NewToolResultError("Repository deletion confirmation state was invalid. The repository was not deleted."), nil, nil
-				}
-				if state.Owner != owner || state.Repo != repo {
-					return utils.NewToolResultError("Repository deletion target changed after confirmation was requested. The repository was not deleted."), nil, nil
-				}
-				if state.ExpiresAt <= time.Now().Unix() {
-					return utils.NewToolResultError("Repository deletion confirmation expired. The repository was not deleted."), nil, nil
-				}
-				if state.RepositoryID == 0 {
-					return utils.NewToolResultError("Repository deletion confirmation state was invalid. The repository was not deleted."), nil, nil
-				}
-				deletionState = &state
+			if req.Params.RequestState == "" {
+				return utils.NewToolResultError("Repository deletion confirmation state was missing. The repository was not deleted."), nil, nil
+			}
+			stateJSON, err := sealer.Open(req.Params.RequestState)
+			if err != nil {
+				return utils.NewToolResultError("Repository deletion confirmation state was invalid. The repository was not deleted."), nil, nil
+			}
+			if err := json.Unmarshal(stateJSON, &deletionState); err != nil {
+				return utils.NewToolResultError("Repository deletion confirmation state was invalid. The repository was not deleted."), nil, nil
+			}
+			if deletionState.Owner != owner || deletionState.Repo != repo {
+				return utils.NewToolResultError("Repository deletion target changed after confirmation was requested. The repository was not deleted."), nil, nil
+			}
+			if deletionState.ExpiresAt <= time.Now().Unix() {
+				return utils.NewToolResultError("Repository deletion confirmation expired. The repository was not deleted."), nil, nil
+			}
+			if deletionState.RepositoryID == 0 {
+				return utils.NewToolResultError("Repository deletion confirmation state was invalid. The repository was not deleted."), nil, nil
 			}
 
 			confirmation, ok := response.(*mcp.ElicitResult)
@@ -853,14 +849,12 @@ func DeleteRepository(t translations.TranslationHelperFunc) inventory.ServerTool
 			if err != nil {
 				return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err)
 			}
-			if deletionState != nil {
-				currentRepositoryID, result := repositoryIDForDeletion(ctx, client, owner, repo)
-				if result != nil {
-					return result, nil, nil
-				}
-				if currentRepositoryID != deletionState.RepositoryID {
-					return utils.NewToolResultError("Repository identity changed after confirmation was requested. The repository was not deleted."), nil, nil
-				}
+			currentRepositoryID, result := repositoryIDForDeletion(ctx, client, owner, repo)
+			if result != nil {
+				return result, nil, nil
+			}
+			if currentRepositoryID != deletionState.RepositoryID {
+				return utils.NewToolResultError("Repository identity changed after confirmation was requested. The repository was not deleted."), nil, nil
 			}
 			resp, err := client.Repositories.Delete(ctx, owner, repo)
 			if err != nil {
@@ -885,6 +879,7 @@ func DeleteRepository(t translations.TranslationHelperFunc) inventory.ServerTool
 	)
 	tool.MinimumProtocolVersion = inventory.ProtocolVersionMultiRoundTrip
 	tool.RequiredElicitationMode = inventory.ElicitationModeForm
+	tool.RequiredScopeGroups = scopes.ExpandScopeGroups(scopes.DeleteRepo, scopes.Repo)
 	return tool
 }