fix(repos): expire deletion confirmations

Bind sealed repository deletion state to the immutable repository ID and a ten-minute expiry. Re-check identity before deletion so replay cannot affect a recreated repository.

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

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

Author: SamMorrowDrums

pkg/github/repositories.go

@@ -10,6 +10,7 @@ import (
 	"slices"
 	"strconv"
 	"strings"
+	"time"
 
 	ghErrors "github.com/github/github-mcp-server/pkg/errors"
 	"github.com/github/github-mcp-server/pkg/ifc"
@@ -708,11 +709,14 @@ const (
 	DeleteRepositoryToolName          = "delete_repository"
 	deleteRepositoryConfirmationID    = "delete_repository_confirmation"
 	deleteRepositoryConfirmationField = "repository_name"
+	deleteRepositoryConfirmationTTL   = 10 * time.Minute
 )
 
 type deleteRepositoryState struct {
-	Owner string `json:"owner"`
-	Repo  string `json:"repo"`
+	Owner        string `json:"owner"`
+	Repo         string `json:"repo"`
+	RepositoryID int64  `json:"repository_id"`
+	ExpiresAt    int64  `json:"expires_at"`
 }
 
 // DeleteRepository creates a tool that deletes a GitHub repository after the
@@ -756,6 +760,7 @@ func DeleteRepository(t translations.TranslationHelperFunc) inventory.ServerTool
 
 			fullName := owner + "/" + repo
 			sealer := requestStateSealerFromDeps(deps)
+			var deletionState *deleteRepositoryState
 			var responses mcp.InputResponseMap
 			if req != nil && req.Params != nil {
 				responses = req.Params.InputResponses
@@ -764,7 +769,20 @@ func DeleteRepository(t translations.TranslationHelperFunc) inventory.ServerTool
 			if !ok {
 				var requestState string
 				if sealer != nil {
-					state, err := json.Marshal(deleteRepositoryState{Owner: owner, Repo: repo})
+					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)
 					}
@@ -810,6 +828,13 @@ func DeleteRepository(t translations.TranslationHelperFunc) inventory.ServerTool
 				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
 			}
 
 			confirmation, ok := response.(*mcp.ElicitResult)
@@ -828,6 +853,15 @@ 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
+				}
+			}
 			resp, err := client.Repositories.Delete(ctx, owner, repo)
 			if err != nil {
 				return ghErrors.NewGitHubAPIErrorResponse(ctx,
@@ -854,6 +888,21 @@ func DeleteRepository(t translations.TranslationHelperFunc) inventory.ServerTool
 	return tool
 }
 
+func repositoryIDForDeletion(ctx context.Context, client *github.Client, owner, repo string) (int64, *mcp.CallToolResult) {
+	repository, resp, err := client.Repositories.Get(ctx, owner, repo)
+	if err != nil {
+		return 0, ghErrors.NewGitHubAPIErrorResponse(ctx,
+			fmt.Sprintf("failed to get repository: %s/%s", owner, repo),
+			resp,
+			err,
+		)
+	}
+	if resp != nil && resp.Body != nil {
+		defer func() { _ = resp.Body.Close() }()
+	}
+	return repository.GetID(), nil
+}
+
 // FetchRepoIsPrivate returns whether a repository is private. It is a thin
 // wrapper around the GitHub Repositories.Get endpoint provided as a shared
 // helper for IFC label computation across tools.

pkg/github/repositories_test.go

@@ -3042,6 +3042,10 @@ func Test_DeleteRepository(t *testing.T) {
 		sealer, err := requeststate.New(base64.StdEncoding.EncodeToString([]byte("0123456789abcdef0123456789abcdef")))
 		require.NoError(t, err)
 		client := NewMockedHTTPClient(
+			WithRequestMatchHandler(
+				GetReposByOwnerByRepo,
+				mockResponse(t, http.StatusOK, map[string]any{"id": 123}),
+			),
 			WithRequestMatchHandler(
 				DeleteReposByOwnerByRepo,
 				mockResponse(t, http.StatusNoContent, nil),
@@ -3099,15 +3103,54 @@ func Test_DeleteRepository(t *testing.T) {
 		assert.Contains(t, getErrorResult(t, result).Text, "state was invalid")
 	})
 
-	t.Run("refuses a changed deletion target", func(t *testing.T) {
+	t.Run("refuses expired deletion state", func(t *testing.T) {
 		sealer, err := requeststate.New(base64.StdEncoding.EncodeToString([]byte("0123456789abcdef0123456789abcdef")))
 		require.NoError(t, err)
+		stateJSON, err := json.Marshal(deleteRepositoryState{
+			Owner:        "owner",
+			Repo:         "repo",
+			RepositoryID: 123,
+			ExpiresAt:    time.Now().Add(-time.Minute).Unix(),
+		})
+		require.NoError(t, err)
+		state, err := sealer.Seal(context.Background(), stateJSON)
+		require.NoError(t, err)
 		deps := BaseDeps{
 			Client:      mustNewGHClient(t, NewMockedHTTPClient()),
 			StateSealer: sealer,
 		}
 		handler := serverTool.Handler(deps)
 
+		request := createMCPRequest(map[string]any{"owner": "owner", "repo": "repo"})
+		request.Params.RequestState = state
+		request.Params.InputResponses = mcp.InputResponseMap{
+			deleteRepositoryConfirmationID: &mcp.ElicitResult{
+				Action: "accept",
+				Content: map[string]any{
+					deleteRepositoryConfirmationField: "owner/repo",
+				},
+			},
+		}
+		result, err := handler(ContextWithDeps(context.Background(), deps), &request)
+		require.NoError(t, err)
+		require.True(t, result.IsError)
+		assert.Contains(t, getErrorResult(t, result).Text, "confirmation expired")
+	})
+
+	t.Run("refuses a changed deletion target", func(t *testing.T) {
+		sealer, err := requeststate.New(base64.StdEncoding.EncodeToString([]byte("0123456789abcdef0123456789abcdef")))
+		require.NoError(t, err)
+		deps := BaseDeps{
+			Client: mustNewGHClient(t, NewMockedHTTPClient(
+				WithRequestMatchHandler(
+					GetReposByOwnerByRepo,
+					mockResponse(t, http.StatusOK, map[string]any{"id": 123}),
+				),
+			)),
+			StateSealer: sealer,
+		}
+		handler := serverTool.Handler(deps)
+
 		firstRequest := createMCPRequest(map[string]any{"owner": "owner", "repo": "repo"})
 		firstResult, err := handler(ContextWithDeps(context.Background(), deps), &firstRequest)
 		require.NoError(t, err)
@@ -3128,6 +3171,50 @@ func Test_DeleteRepository(t *testing.T) {
 		assert.Contains(t, getErrorResult(t, result).Text, "target changed")
 	})
 
+	t.Run("refuses a recreated repository", func(t *testing.T) {
+		sealer, err := requeststate.New(base64.StdEncoding.EncodeToString([]byte("0123456789abcdef0123456789abcdef")))
+		require.NoError(t, err)
+		var repositoryLookups int
+		client := NewMockedHTTPClient(
+			WithRequestMatchHandler(
+				GetReposByOwnerByRepo,
+				http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+					repositoryLookups++
+					id := 123
+					if repositoryLookups > 1 {
+						id = 456
+					}
+					w.WriteHeader(http.StatusOK)
+					require.NoError(t, json.NewEncoder(w).Encode(map[string]any{"id": id}))
+				}),
+			),
+		)
+		deps := BaseDeps{
+			Client:      mustNewGHClient(t, client),
+			StateSealer: sealer,
+		}
+		handler := serverTool.Handler(deps)
+
+		firstRequest := createMCPRequest(map[string]any{"owner": "owner", "repo": "repo"})
+		firstResult, err := handler(ContextWithDeps(context.Background(), deps), &firstRequest)
+		require.NoError(t, err)
+
+		retry := createMCPRequest(map[string]any{"owner": "owner", "repo": "repo"})
+		retry.Params.RequestState = firstResult.RequestState
+		retry.Params.InputResponses = mcp.InputResponseMap{
+			deleteRepositoryConfirmationID: &mcp.ElicitResult{
+				Action: "accept",
+				Content: map[string]any{
+					deleteRepositoryConfirmationField: "owner/repo",
+				},
+			},
+		}
+		result, err := handler(ContextWithDeps(context.Background(), deps), &retry)
+		require.NoError(t, err)
+		require.True(t, result.IsError)
+		assert.Contains(t, getErrorResult(t, result).Text, "identity changed")
+	})
+
 	t.Run("completes multi-round-trip elicitation before deleting", func(t *testing.T) {
 		httpClient := NewMockedHTTPClient(
 			WithRequestMatchHandler(