Skip to content

Commit 14aea52

Browse files
feat(http): protect MRTR request state
Seal repository deletion targets for self-hosted HTTP with a stable AES-256-GCM key. Hide only delete_repository when no key is configured and expose an optional sealer interface for remote integrators. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4b04480c-c2e9-483e-9b0f-34830b76a2f8
1 parent c34055e commit 14aea52

12 files changed

Lines changed: 416 additions & 4 deletions

File tree

cmd/github-mcp-server/main.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,7 @@ var (
217217
EnabledFeatures: enabledFeatures,
218218
InsidersMode: viper.GetBool("insiders"),
219219
TrustProxyHeaders: viper.GetBool("trust-proxy-headers"),
220+
MRTRStateKey: os.Getenv(ghhttp.MRTRStateKeyEnv),
220221
}
221222

222223
return ghhttp.RunHTTPServer(httpConfig)

docs/streamable-http.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,27 @@ github-mcp-server http --scope-challenge
3232

3333
When `--scope-challenge` is enabled, requests with insufficient scopes receive a `403 Forbidden` response with a `WWW-Authenticate` header indicating the required scopes.
3434

35+
### Repository deletion and request-state encryption
36+
37+
The `delete_repository` tool uses multi-round-trip elicitation and carries its
38+
confirmed target through client-held request state. To expose this tool in HTTP
39+
mode, configure a stable 32-byte encryption key encoded with standard Base64:
40+
41+
```bash
42+
export GITHUB_MCP_SERVER_MRTR_STATE_KEY="$(openssl rand -base64 32 | tr -d '\n')"
43+
github-mcp-server http
44+
```
45+
46+
Use the same key on every replica that may handle a retry. Keep it secret and
47+
stable during deployments; changing it invalidates confirmations already in
48+
flight. If the variable is absent, `delete_repository` is not exposed by the
49+
HTTP server. If it is present but malformed, the server refuses to start.
50+
51+
This self-hosted key is independent of keys used by the hosted remote server.
52+
Integrators can provide their own request-state sealer through the exported
53+
`github.RequestStateSealer` interface and expose it from their tool dependencies
54+
through `github.RequestStateSealerProvider` without changing their key format.
55+
3556
### With OAuth Metadata Discovery
3657

3758
For use behind reverse proxies or with custom domains, expose OAuth metadata endpoints:

internal/requeststate/sealer.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
package requeststate
2+
3+
import (
4+
"context"
5+
"crypto/aes"
6+
"crypto/cipher"
7+
"crypto/rand"
8+
"encoding/base64"
9+
"errors"
10+
"fmt"
11+
)
12+
13+
const keySize = 32
14+
15+
// Sealer protects request state with AES-256-GCM.
16+
type Sealer struct {
17+
aead cipher.AEAD
18+
}
19+
20+
// New constructs a sealer from a standard Base64-encoded 32-byte key.
21+
func New(encodedKey string) (*Sealer, error) {
22+
key, err := base64.StdEncoding.DecodeString(encodedKey)
23+
if err != nil {
24+
return nil, fmt.Errorf("decoding key: %w", err)
25+
}
26+
if len(key) != keySize {
27+
return nil, fmt.Errorf("decoded key must be %d bytes, got %d", keySize, len(key))
28+
}
29+
block, err := aes.NewCipher(key)
30+
if err != nil {
31+
return nil, fmt.Errorf("creating cipher: %w", err)
32+
}
33+
aead, err := cipher.NewGCM(block)
34+
if err != nil {
35+
return nil, fmt.Errorf("creating GCM: %w", err)
36+
}
37+
return &Sealer{aead: aead}, nil
38+
}
39+
40+
// Seal encrypts and authenticates plaintext into a URL-safe opaque token.
41+
func (s *Sealer) Seal(_ context.Context, plaintext []byte) (string, error) {
42+
nonce := make([]byte, s.aead.NonceSize())
43+
if _, err := rand.Read(nonce); err != nil {
44+
return "", fmt.Errorf("generating nonce: %w", err)
45+
}
46+
sealed := s.aead.Seal(nonce, nonce, plaintext, nil)
47+
return base64.RawURLEncoding.EncodeToString(sealed), nil
48+
}
49+
50+
// Open verifies and decrypts a token produced by Seal.
51+
func (s *Sealer) Open(token string) ([]byte, error) {
52+
if token == "" {
53+
return nil, errors.New("empty token")
54+
}
55+
sealed, err := base64.RawURLEncoding.DecodeString(token)
56+
if err != nil {
57+
return nil, fmt.Errorf("decoding token: %w", err)
58+
}
59+
nonceSize := s.aead.NonceSize()
60+
if len(sealed) < nonceSize {
61+
return nil, errors.New("token is too short")
62+
}
63+
plaintext, err := s.aead.Open(nil, sealed[:nonceSize], sealed[nonceSize:], nil)
64+
if err != nil {
65+
return nil, fmt.Errorf("opening token: %w", err)
66+
}
67+
return plaintext, nil
68+
}
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
package requeststate
2+
3+
import (
4+
"context"
5+
"encoding/base64"
6+
"strings"
7+
"testing"
8+
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
)
12+
13+
func TestSealer(t *testing.T) {
14+
key := base64.StdEncoding.EncodeToString([]byte("0123456789abcdef0123456789abcdef"))
15+
sealer, err := New(key)
16+
require.NoError(t, err)
17+
18+
t.Run("round trip", func(t *testing.T) {
19+
plaintext := []byte(`{"owner":"octo","repo":"repo"}`)
20+
token, err := sealer.Seal(context.Background(), plaintext)
21+
require.NoError(t, err)
22+
assert.NotContains(t, token, string(plaintext))
23+
24+
opened, err := sealer.Open(token)
25+
require.NoError(t, err)
26+
assert.Equal(t, plaintext, opened)
27+
})
28+
29+
t.Run("rejects tampering", func(t *testing.T) {
30+
token, err := sealer.Seal(context.Background(), []byte("state"))
31+
require.NoError(t, err)
32+
replacement := "A"
33+
if strings.HasSuffix(token, replacement) {
34+
replacement = "B"
35+
}
36+
37+
_, err = sealer.Open(token[:len(token)-1] + replacement)
38+
require.Error(t, err)
39+
})
40+
}
41+
42+
func TestNew(t *testing.T) {
43+
tests := []struct {
44+
name string
45+
key string
46+
}{
47+
{name: "empty key"},
48+
{name: "invalid Base64", key: "not-base64"},
49+
{name: "wrong decoded length", key: base64.StdEncoding.EncodeToString([]byte("too short"))},
50+
}
51+
52+
for _, tt := range tests {
53+
t.Run(tt.name, func(t *testing.T) {
54+
_, err := New(tt.key)
55+
require.Error(t, err)
56+
})
57+
}
58+
}

pkg/github/dependencies.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,9 @@ type BaseDeps struct {
127127

128128
// Observability exporters (includes logger)
129129
Obsv observability.Exporters
130+
131+
// StateSealer protects state sent through multi-round-trip requests.
132+
StateSealer RequestStateSealer
130133
}
131134

132135
// Compile-time assertion to verify that BaseDeps implements the ToolDependencies interface.
@@ -199,6 +202,9 @@ func (d BaseDeps) Metrics(ctx context.Context) metrics.Metrics {
199202
return d.Obsv.Metrics(ctx)
200203
}
201204

205+
// GetRequestStateSealer implements RequestStateSealerProvider.
206+
func (d BaseDeps) GetRequestStateSealer() RequestStateSealer { return d.StateSealer }
207+
202208
// IsFeatureEnabled checks if a feature flag is enabled.
203209
// Returns false if the feature checker is nil, flag name is empty, or an error occurs.
204210
// This allows tools to conditionally change behavior based on feature flags.
@@ -279,6 +285,9 @@ type RequestDeps struct {
279285

280286
// Observability exporters (includes logger)
281287
obsv observability.Exporters
288+
289+
// StateSealer protects state sent through multi-round-trip requests.
290+
StateSealer RequestStateSealer
282291
}
283292

284293
// NewRequestDeps creates a RequestDeps with the provided clients and configuration.
@@ -334,6 +343,9 @@ func (d *RequestDeps) GetClient(ctx context.Context) (*gogithub.Client, error) {
334343
return restClient, nil
335344
}
336345

346+
// GetRequestStateSealer implements RequestStateSealerProvider.
347+
func (d *RequestDeps) GetRequestStateSealer() RequestStateSealer { return d.StateSealer }
348+
337349
// GetGQLClient implements ToolDependencies.
338350
func (d *RequestDeps) GetGQLClient(ctx context.Context) (*githubv4.Client, error) {
339351
// extract the token from the context

pkg/github/repositories.go

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -705,17 +705,23 @@ func CreateRepository(t translations.TranslationHelperFunc) inventory.ServerTool
705705
}
706706

707707
const (
708+
DeleteRepositoryToolName = "delete_repository"
708709
deleteRepositoryConfirmationID = "delete_repository_confirmation"
709710
deleteRepositoryConfirmationField = "repository_name"
710711
)
711712

713+
type deleteRepositoryState struct {
714+
Owner string `json:"owner"`
715+
Repo string `json:"repo"`
716+
}
717+
712718
// DeleteRepository creates a tool that deletes a GitHub repository after the
713719
// user confirms its full name through elicitation.
714720
func DeleteRepository(t translations.TranslationHelperFunc) inventory.ServerTool {
715721
tool := NewTool(
716722
ToolsetMetadataRepos,
717723
mcp.Tool{
718-
Name: "delete_repository",
724+
Name: DeleteRepositoryToolName,
719725
Description: t("TOOL_DELETE_REPOSITORY_DESCRIPTION", "Delete a GitHub repository after the user confirms the exact owner/repository name"),
720726
Annotations: &mcp.ToolAnnotations{
721727
Title: t("TOOL_DELETE_REPOSITORY_USER_TITLE", "Delete repository"),
@@ -749,12 +755,24 @@ func DeleteRepository(t translations.TranslationHelperFunc) inventory.ServerTool
749755
}
750756

751757
fullName := owner + "/" + repo
758+
sealer := requestStateSealerFromDeps(deps)
752759
var responses mcp.InputResponseMap
753760
if req != nil && req.Params != nil {
754761
responses = req.Params.InputResponses
755762
}
756763
response, ok := responses[deleteRepositoryConfirmationID]
757764
if !ok {
765+
var requestState string
766+
if sealer != nil {
767+
state, err := json.Marshal(deleteRepositoryState{Owner: owner, Repo: repo})
768+
if err != nil {
769+
return nil, nil, fmt.Errorf("failed to marshal repository deletion state: %w", err)
770+
}
771+
requestState, err = sealer.Seal(ctx, state)
772+
if err != nil {
773+
return nil, nil, fmt.Errorf("failed to seal repository deletion state: %w", err)
774+
}
775+
}
758776
return &mcp.CallToolResult{
759777
InputRequests: mcp.InputRequestMap{
760778
deleteRepositoryConfirmationID: &mcp.ElicitParams{
@@ -773,9 +791,27 @@ func DeleteRepository(t translations.TranslationHelperFunc) inventory.ServerTool
773791
},
774792
},
775793
},
794+
RequestState: requestState,
776795
}, nil, nil
777796
}
778797

798+
if sealer != nil {
799+
if req.Params.RequestState == "" {
800+
return utils.NewToolResultError("Repository deletion confirmation state was missing. The repository was not deleted."), nil, nil
801+
}
802+
stateJSON, err := sealer.Open(req.Params.RequestState)
803+
if err != nil {
804+
return utils.NewToolResultError("Repository deletion confirmation state was invalid. The repository was not deleted."), nil, nil
805+
}
806+
var state deleteRepositoryState
807+
if err := json.Unmarshal(stateJSON, &state); err != nil {
808+
return utils.NewToolResultError("Repository deletion confirmation state was invalid. The repository was not deleted."), nil, nil
809+
}
810+
if state.Owner != owner || state.Repo != repo {
811+
return utils.NewToolResultError("Repository deletion target changed after confirmation was requested. The repository was not deleted."), nil, nil
812+
}
813+
}
814+
779815
confirmation, ok := response.(*mcp.ElicitResult)
780816
if !ok {
781817
return utils.NewToolResultError("Repository deletion confirmation was invalid. The repository was not deleted."), nil, nil

pkg/github/repositories_test.go

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"time"
1212

1313
"github.com/github/github-mcp-server/internal/githubv4mock"
14+
"github.com/github/github-mcp-server/internal/requeststate"
1415
"github.com/github/github-mcp-server/internal/toolsnaps"
1516
"github.com/github/github-mcp-server/pkg/inventory"
1617
"github.com/github/github-mcp-server/pkg/raw"
@@ -3037,6 +3038,96 @@ func Test_DeleteRepository(t *testing.T) {
30373038
assert.Contains(t, getTextResult(t, result).Text, "owner/repo was deleted")
30383039
})
30393040

3041+
t.Run("seals and verifies the deletion target", func(t *testing.T) {
3042+
sealer, err := requeststate.New(base64.StdEncoding.EncodeToString([]byte("0123456789abcdef0123456789abcdef")))
3043+
require.NoError(t, err)
3044+
client := NewMockedHTTPClient(
3045+
WithRequestMatchHandler(
3046+
DeleteReposByOwnerByRepo,
3047+
mockResponse(t, http.StatusNoContent, nil),
3048+
),
3049+
)
3050+
deps := BaseDeps{
3051+
Client: mustNewGHClient(t, client),
3052+
StateSealer: sealer,
3053+
}
3054+
handler := serverTool.Handler(deps)
3055+
3056+
firstRequest := createMCPRequest(map[string]any{"owner": "owner", "repo": "repo"})
3057+
firstResult, err := handler(ContextWithDeps(context.Background(), deps), &firstRequest)
3058+
require.NoError(t, err)
3059+
require.NotEmpty(t, firstResult.RequestState)
3060+
3061+
retry := createMCPRequest(map[string]any{"owner": "owner", "repo": "repo"})
3062+
retry.Params.RequestState = firstResult.RequestState
3063+
retry.Params.InputResponses = mcp.InputResponseMap{
3064+
deleteRepositoryConfirmationID: &mcp.ElicitResult{
3065+
Action: "accept",
3066+
Content: map[string]any{
3067+
deleteRepositoryConfirmationField: "owner/repo",
3068+
},
3069+
},
3070+
}
3071+
result, err := handler(ContextWithDeps(context.Background(), deps), &retry)
3072+
require.NoError(t, err)
3073+
require.False(t, result.IsError)
3074+
assert.Contains(t, getTextResult(t, result).Text, "owner/repo was deleted")
3075+
})
3076+
3077+
t.Run("refuses tampered deletion state", func(t *testing.T) {
3078+
sealer, err := requeststate.New(base64.StdEncoding.EncodeToString([]byte("0123456789abcdef0123456789abcdef")))
3079+
require.NoError(t, err)
3080+
deps := BaseDeps{
3081+
Client: mustNewGHClient(t, NewMockedHTTPClient()),
3082+
StateSealer: sealer,
3083+
}
3084+
handler := serverTool.Handler(deps)
3085+
3086+
request := createMCPRequest(map[string]any{"owner": "owner", "repo": "repo"})
3087+
request.Params.RequestState = "tampered"
3088+
request.Params.InputResponses = mcp.InputResponseMap{
3089+
deleteRepositoryConfirmationID: &mcp.ElicitResult{
3090+
Action: "accept",
3091+
Content: map[string]any{
3092+
deleteRepositoryConfirmationField: "owner/repo",
3093+
},
3094+
},
3095+
}
3096+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
3097+
require.NoError(t, err)
3098+
require.True(t, result.IsError)
3099+
assert.Contains(t, getErrorResult(t, result).Text, "state was invalid")
3100+
})
3101+
3102+
t.Run("refuses a changed deletion target", func(t *testing.T) {
3103+
sealer, err := requeststate.New(base64.StdEncoding.EncodeToString([]byte("0123456789abcdef0123456789abcdef")))
3104+
require.NoError(t, err)
3105+
deps := BaseDeps{
3106+
Client: mustNewGHClient(t, NewMockedHTTPClient()),
3107+
StateSealer: sealer,
3108+
}
3109+
handler := serverTool.Handler(deps)
3110+
3111+
firstRequest := createMCPRequest(map[string]any{"owner": "owner", "repo": "repo"})
3112+
firstResult, err := handler(ContextWithDeps(context.Background(), deps), &firstRequest)
3113+
require.NoError(t, err)
3114+
3115+
retry := createMCPRequest(map[string]any{"owner": "owner", "repo": "another"})
3116+
retry.Params.RequestState = firstResult.RequestState
3117+
retry.Params.InputResponses = mcp.InputResponseMap{
3118+
deleteRepositoryConfirmationID: &mcp.ElicitResult{
3119+
Action: "accept",
3120+
Content: map[string]any{
3121+
deleteRepositoryConfirmationField: "owner/repo",
3122+
},
3123+
},
3124+
}
3125+
result, err := handler(ContextWithDeps(context.Background(), deps), &retry)
3126+
require.NoError(t, err)
3127+
require.True(t, result.IsError)
3128+
assert.Contains(t, getErrorResult(t, result).Text, "target changed")
3129+
})
3130+
30403131
t.Run("completes multi-round-trip elicitation before deleting", func(t *testing.T) {
30413132
httpClient := NewMockedHTTPClient(
30423133
WithRequestMatchHandler(

0 commit comments

Comments
 (0)