From 0b8e027b81c23261ba7f66346c44979ce3447ad3 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Fri, 23 Oct 2026 14:26:26 +0200 Subject: [PATCH] test(19.7-05): security review pins for the MCP endpoint's outer layers - TestEndpointGuards: a foreign browser Origin is 403 before the bearer is read, a body over the endpoint cap is 413, a long GET stream ends once its grant stops verifying (T-19.7-18, T-19.7-19) - TestDispatch: an argument key the input struct does not declare is refused before dispatch (T-19.7-17) - the gate requires TestEndpointGuards (untagged and tagged) and runs the demo's deploy check (T-19.7-37) --- modules/pier/dispatch_test.go | 7 ++ modules/pier/endpoint_guards_test.go | 109 +++++++++++++++++++++++++++ modules/pier/export_test.go | 3 + scripts/check-phase19.7.sh | 9 ++- 4 files changed, 126 insertions(+), 2 deletions(-) create mode 100644 modules/pier/endpoint_guards_test.go diff --git a/modules/pier/dispatch_test.go b/modules/pier/dispatch_test.go index c47ce476..df2c3cb7 100644 --- a/modules/pier/dispatch_test.go +++ b/modules/pier/dispatch_test.go @@ -121,6 +121,13 @@ func TestDispatch(t *testing.T) { if !res.IsError || !strings.Contains(resultText(t, res), "invalid arguments") { t.Fatalf("missing required title = %+v", res) } + // The reflected schema refuses keys the input struct does not + // declare (additionalProperties false), so nothing undeclared + // reaches the route's body. + res = call("disp_thing", map[string]any{"id": "abc", "title": "T", "user_id": 1}) + if !res.IsError || !strings.Contains(resultText(t, res), "invalid arguments") { + t.Fatalf("undeclared key = %+v", res) + } if thingHits.Load() != before { t.Fatal("a refused call reached the route") } diff --git a/modules/pier/endpoint_guards_test.go b/modules/pier/endpoint_guards_test.go new file mode 100644 index 00000000..81504976 --- /dev/null +++ b/modules/pier/endpoint_guards_test.go @@ -0,0 +1,109 @@ +package pier_test + +import ( + "bytes" + "context" + "io" + "net/http" + "strings" + "testing" + "time" + + "git.golem15.com/golem15/summercms/modules/pact" +) + +// TestEndpointGuards (T-19.7-18, T-19.7-19, Pitfall 10): the endpoint's own +// outer layers. A browser Origin other than the allowed ones is 403 before +// the bearer or the body is read (no Origin and the app's own origin pass); +// a request body above the endpoint's cap is 413; and a long-lived GET +// stream re-verifies its bearer and ends once the grant no longer verifies. +func TestEndpointGuards(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + verifier := newFakeVerifier(map[string]pact.MCPGrant{"oas_at_stream": grant(12, "c6", 6, "read")}) + in := startInstance(t, newApp(t, appSpec{guards: []pact.MCPGuard{&acmeGuard{}}, verifier: verifier}), acmePlugin{tools: []pact.MCPTool{echoTool()}}) + + post := func(token, origin string, body io.Reader) *http.Response { + t.Helper() + req, err := http.NewRequestWithContext(ctx, http.MethodPost, in.url(), body) + if err != nil { + t.Fatal(err) + } + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Accept", "application/json, text/event-stream") + req.Header.Set("Mcp-Protocol-Version", "2025-11-25") + if token != "" { + req.Header.Set("Authorization", "Bearer "+token) + } + if origin != "" { + req.Header.Set("Origin", origin) + } + res, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + _, _ = io.Copy(io.Discard, res.Body) + _ = res.Body.Close() + return res + } + + t.Run("origin check", func(t *testing.T) { + if res := post("", "https://evil.example", strings.NewReader(initializeBody)); res.StatusCode != http.StatusForbidden { + t.Fatalf("foreign origin without a bearer = %d, want 403 before the bearer check", res.StatusCode) + } + if res := post("oas_at_test", "https://evil.example", strings.NewReader(initializeBody)); res.StatusCode != http.StatusForbidden { + t.Fatalf("foreign origin = %d", res.StatusCode) + } + if res := post("oas_at_test", "http://acme.test", strings.NewReader(initializeBody)); res.StatusCode != http.StatusOK { + t.Fatalf("the app's own origin = %d", res.StatusCode) + } + if res := post("oas_at_test", "", strings.NewReader(initializeBody)); res.StatusCode != http.StatusOK { + t.Fatalf("no origin = %d", res.StatusCode) + } + }) + + t.Run("body cap", func(t *testing.T) { + big := bytes.Repeat([]byte(" "), 5<<20) + body := append([]byte(`{"jsonrpc":"2.0","id":1,"method":"initialize","params":{}}`), big...) + if res := post("oas_at_test", "", bytes.NewReader(body)); res.StatusCode != http.StatusRequestEntityTooLarge { + t.Fatalf("5 MiB body = %d, want 413", res.StatusCode) + } + }) + + t.Run("stream re-verifies its bearer", func(t *testing.T) { + in.pier().SetRecheck(50 * time.Millisecond) + sid := openSession(t, ctx, in, "oas_at_stream") + req, err := http.NewRequestWithContext(ctx, http.MethodGet, in.url(), nil) + if err != nil { + t.Fatal(err) + } + req.Header.Set("Authorization", "Bearer oas_at_stream") + req.Header.Set("Accept", "text/event-stream") + req.Header.Set("Mcp-Session-Id", sid) + req.Header.Set("Mcp-Protocol-Version", "2025-11-25") + res, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + t.Fatalf("GET stream = %d", res.StatusCode) + } + ended := make(chan struct{}) + go func() { + _, _ = io.Copy(io.Discard, res.Body) + close(ended) + }() + select { + case <-ended: + t.Fatal("the stream ended while the grant still verified") + case <-time.After(300 * time.Millisecond): + } + verifier.revoke("oas_at_stream") + select { + case <-ended: + case <-time.After(5 * time.Second): + t.Fatal("the stream outlived its revoked grant") + } + }) +} diff --git a/modules/pier/export_test.go b/modules/pier/export_test.go index a5fab929..9d8598ef 100644 --- a/modules/pier/export_test.go +++ b/modules/pier/export_test.go @@ -46,3 +46,6 @@ func (p *Pier) StatelessServers() int { defer p.stateless.mu.Unlock() return len(p.stateless.entries) } + +// SetRecheck sets how often a long GET stream re-verifies its bearer. +func (p *Pier) SetRecheck(d time.Duration) { p.recheck = d } diff --git a/scripts/check-phase19.7.sh b/scripts/check-phase19.7.sh index dcad87eb..0c919d98 100755 --- a/scripts/check-phase19.7.sh +++ b/scripts/check-phase19.7.sh @@ -21,7 +21,8 @@ # plugin's last commit before the phase). # consumers the applications at their bumped pins (D-31): go build ./... # and go test -short ./... in each, the demo also with -tags -# summercms_mcp and its pin check. +# summercms_mcp, its pin check and its deploy check (the nginx +# locations of the MCP endpoint). # docker Docker host only: the Postgres suites by required name in the # framework (cabana), the three plugins and the demo (tagged). # admin npm --prefix admin run typecheck, the vitest run (the phase's @@ -94,6 +95,8 @@ GO_TESTS=( # D-30, D-34). ./modules/pier:TestToolsList ./modules/pier:TestCatalog ./modules/pier:TestDispatch ./modules/pier:TestGuardRules ./modules/pier:TestSessions ./modules/pier:TestFanOut + # Plan 05 security review pins (T-19.7-18, T-19.7-19). + ./modules/pier:TestEndpointGuards ./modules/bouncer:TestDispatchMarker ./modules/cabana:TestDispatchMarker ./modules/surf:TestStreamingRoute ./modules/surf:TestPierMount ./modules/wristband:TestMCPScopes @@ -106,7 +109,7 @@ GO_TESTS=( GO_TAGGED_TESTS=( ./modules/pier:TestToolsList ./modules/pier:TestCatalog ./modules/pier:TestDispatch ./modules/pier:TestGuardRules ./modules/pier:TestSessions ./modules/pier:TestFanOut - ./modules/pier:TestPierTracerSmoke ./modules/pier:TestPierStatelessSmoke + ./modules/pier:TestPierTracerSmoke ./modules/pier:TestPierStatelessSmoke ./modules/pier:TestEndpointGuards ./modules/surf:TestPierMount ./modules/surf:TestStreamingRoute ./modules/surf:TestDispatchSmoke ) @@ -629,6 +632,8 @@ run_consumers() { echo "build: $DEMO_APP (-tags $MCP_TAG)" GO_FLAGS="-tags $MCP_TAG" go_short "$DEMO_APP" ./... demo_pins + # T-19.7-37: the demo's vhost opens exactly the MCP paths, unbuffered. + (cd "$DEMO_APP" && bash scripts/check-deploy.sh) echo "phase19.7 consumers passed" }