Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions .github/workflows/e2e-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -130,13 +130,15 @@ jobs:
docker pull ghcr.io/stackloklabs/osv-mcp/server:0.1.3 &
docker pull ghcr.io/stackloklabs/gofetch/server:1.0.2 &
docker pull ghcr.io/stacklok/toolhive/egress-proxy:latest &
# yardstick is only needed for the vmcp test suite
# yardstick (Legacy 1.1.1) is the backend for the vmcp test suite
if [ "${{ matrix.label_filter }}" = "vmcp" ]; then
docker pull ghcr.io/stackloklabs/yardstick/yardstick-server:1.1.1 &
fi
# the time server is only used by the proxy test suite
# the time server and the Modern (1.2.0) yardstick (dual-era barrier mode)
# are only used by the proxy (streamable-http) test suite
if [ "${{ matrix.label_filter }}" = "proxy && !isolation" ]; then
docker pull ghcr.io/stacklok/dockyard/uvx/mcp-server-time:2026.1.26 &
docker pull ghcr.io/stackloklabs/yardstick/yardstick-server:1.2.0 &
fi
# Envoy is only used by the network-isolation test suite
if [ "${{ matrix.title }}" = "network-isolation" ]; then
Expand Down
5 changes: 5 additions & 0 deletions .github/workflows/test-e2e-lifecycle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ jobs:
runs-on: ubuntu-8cores-32gb
timeout-minutes: 30
env:
# Legacy (go-sdk v1.6.1) backend for the operator + virtualmcp suites.
YARDSTICK_IMAGE: ghcr.io/stackloklabs/yardstick/yardstick-server:1.1.1
# Modern (go-sdk v1.7) backend used only by the dual-era k8s spec (#5837).
YARDSTICK_IMAGE_DUAL_ERA: ghcr.io/stackloklabs/yardstick/yardstick-server:1.2.0
defaults:
run:
shell: bash
Expand Down Expand Up @@ -96,6 +99,7 @@ jobs:
# Pull and load all test server images in parallel to speed up CI
echo "Pulling and loading test server images..."
docker pull ${{ env.YARDSTICK_IMAGE }} &
docker pull ${{ env.YARDSTICK_IMAGE_DUAL_ERA }} &
docker pull ghcr.io/stackloklabs/gofetch/server:1.0.1 &
docker pull ghcr.io/stackloklabs/osv-mcp/server:0.0.7 &
docker pull python:3.9-slim &
Expand All @@ -106,6 +110,7 @@ jobs:

# Load all images into kind
kind load docker-image --name toolhive ${{ env.YARDSTICK_IMAGE }}
kind load docker-image --name toolhive ${{ env.YARDSTICK_IMAGE_DUAL_ERA }}
kind load docker-image --name toolhive ghcr.io/stackloklabs/gofetch/server:1.0.1
kind load docker-image --name toolhive ghcr.io/stackloklabs/osv-mcp/server:0.0.7
kind load docker-image --name toolhive python:3.9-slim
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ require (
go.opentelemetry.io/otel/exporters/jaeger v1.17.0 // indirect
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.43.0 // indirect
go.opentelemetry.io/otel/exporters/zipkin v1.21.0 // indirect
go.opentelemetry.io/proto/otlp v1.10.0 // indirect
go.opentelemetry.io/proto/otlp v1.10.0
go.uber.org/atomic v1.11.0 // indirect
go.uber.org/multierr v1.11.0 // indirect
go.yaml.in/yaml/v2 v2.4.4 // indirect
Expand All @@ -299,7 +299,7 @@ require (
google.golang.org/genproto/googleapis/api v0.0.0-20260526163538-3dc84a4a5aaa // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20260523011958-0a33c5d7ca68 // indirect
google.golang.org/grpc v1.82.1 // indirect
google.golang.org/protobuf v1.36.11 // indirect
google.golang.org/protobuf v1.36.11
gopkg.in/evanphx/json-patch.v4 v4.13.0 // indirect
gopkg.in/inf.v0 v0.9.1 // indirect
gopkg.in/warnings.v0 v0.1.2 // indirect
Expand Down
9 changes: 9 additions & 0 deletions pkg/transport/proxy/streamable/streamable_proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -1042,6 +1042,15 @@ func (p *HTTPProxy) resolveSessionForRequest(
// ignore any client-supplied Mcp-Session-Id. Never fall through to the
// session-lookup path below with it -- see the confidentiality note
// in this function's doc comment.
//
// This is safe only because, in this proxy, Mcp-Session-Id has no
// downstream authority: it is purely an in-process response-correlation
// token (the backend is stateless stdio, unlike the transparent proxy,
// which maps it to a backend-assigned SID via session metadata). If this
// proxy ever gains a shared session store or a stateful backend mapping,
// "ignore the client SID on Modern" must stay true -- silently reusing or
// looking up a known client SID here would reopen a session-validation
// bypass. See TestModernNeverReusesClientSessionIDAsRoutingToken.
token, err := uuid.NewRandom()
if err != nil {
writeHTTPError(w, http.StatusInternalServerError, fmt.Sprintf("Failed to generate routing token: %v", err))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
"io"
"net"
"net/http"
"runtime"
"sync"
"testing"
"time"

Expand Down Expand Up @@ -144,3 +146,95 @@ func TestHTTPProxy_StartMountsAuthDiscoveryEndpoint(t *testing.T) {
require.NoError(t, json.NewDecoder(resp.Body).Decode(&body))
assert.Equal(t, "https://example.com", body["resource"])
}

// syncMapLen counts entries in a sync.Map for test assertions.
func syncMapLen(m *sync.Map) int {
n := 0
m.Range(func(_, _ any) bool { n++; return true })
return n
}

// TestModernLoadDoesNotAccumulateState drives a burst of concurrent Modern
// (2026-07-28) requests through the full per-request lifecycle --
// resolveSessionForRequest mints a routing token, doRequest registers a
// waiter/idRestore entry keyed on it and deletes both via its deferred
// cleanup once the response arrives -- and proves nothing is left behind.
// This is the regression test for the unbounded-growth concern behind
// toolhive-core#169.
//
// waiters/idRestore/sessionManager are asserted as hard, exact-zero checks:
// createWaiter's cleanup runs synchronously inside doRequest before
// handleSingleRequest ever writes the HTTP response, so by the time every
// client in the burst has read its response, every cleanup has already run --
// no settle delay needed, and a regression that skips cleanup fails this
// deterministically.
//
// The goroutine count is checked too, but only as a generous, best-effort
// bound: this package's request path spawns no per-request goroutine, so any
// growth here would come from net/http's own idle-connection handling, not
// from code owned by this package -- hence the loose tolerance rather than a
// strict equality.
//
//nolint:paralleltest // reads process-wide runtime.NumGoroutine() as a baseline and starts an HTTP proxy; must run serially
func TestModernLoadDoesNotAccumulateState(t *testing.T) {
port := pickFreePort(t)
proxy, ctx, cancel := startProxyWithBackend(t, port)
t.Cleanup(cancel)
t.Cleanup(func() { _ = proxy.Stop(ctx) })

url := fmt.Sprintf("http://127.0.0.1:%d%s", port, StreamableHTTPEndpoint)

const requestCount = 1000
// Cap in-flight requests well under the proxy's 100-slot message-channel
// buffer. Firing all 1000 at once can exceed that buffer and produce
// dropped-response 504s (#5952, a proxy backpressure gap) --
// unrelated to the accumulation invariant this test checks, so throttle
// around it rather than let it flake the run.
const maxInFlight = 50
sem := make(chan struct{}, maxInFlight)
baseline := runtime.NumGoroutine()

var wg sync.WaitGroup
for i := 0; i < requestCount; i++ {
wg.Add(1)
sem <- struct{}{}
go func(id int) {
defer wg.Done()
defer func() { <-sem }()

req, err := http.NewRequest(
http.MethodPost, url, bytes.NewReader([]byte(modernBody(id, "tools/list"))))
if !assert.NoError(t, err) {
return
}
req.Header.Set("Content-Type", "application/json")

resp, err := http.DefaultClient.Do(req)
if !assert.NoError(t, err) {
return
}
defer func() {
_, _ = io.Copy(io.Discard, resp.Body)
resp.Body.Close()
}()
assert.Equal(t, http.StatusOK, resp.StatusCode)
}(i)
}

done := make(chan struct{})
go func() { wg.Wait(); close(done) }()
select {
case <-done:
case <-time.After(30 * time.Second):
t.Fatal("timeout waiting for the request burst to complete")
}

assert.Zero(t, proxy.sessionManager.Count(), "Modern requests must never register a session")
assert.Zero(t, syncMapLen(&proxy.waiters), "waiters must be fully cleaned up once all responses are delivered")
assert.Zero(t, syncMapLen(&proxy.idRestore), "idRestore must be fully cleaned up once all responses are delivered")

http.DefaultClient.CloseIdleConnections()
assert.Eventually(t, func() bool {
return runtime.NumGoroutine() <= baseline+20
}, time.Second, 20*time.Millisecond, "goroutine count did not settle back near baseline after the burst")
}
57 changes: 57 additions & 0 deletions pkg/transport/proxy/streamable/streamable_proxy_modern_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@ import (
"fmt"
"io"
"net/http"
"net/http/httptest"
"testing"
"time"

"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"golang.org/x/exp/jsonrpc2"
Expand Down Expand Up @@ -175,6 +177,61 @@ func TestModernRequestIgnoresClientSessionID(t *testing.T) {
assert.False(t, ok, "Modern path must not register the foreign session id")
}

// TestModernNeverReusesClientSessionIDAsRoutingToken is the mutation-check
// analogue of the transparent proxy's TestGuardUnknownSessionFiresDespiteForgedModernRevision.
// Where that test proves an UNKNOWN session ID still triggers the Legacy
// guard, this one closes the complementary gap: it registers a REAL, known
// session in sessionManager and proves resolveSessionForRequest still mints
// a fresh routing token instead of reusing it.
//
// TestModernRequestIgnoresClientSessionID (above) only exercises a bogus,
// unregistered session ID, so it cannot detect a regression that consults
// sessionManager as a fallback before minting: on a lookup miss such code
// would still mint a fresh token and pass that test. Using a real registered
// session here closes that hole -- this test FAILS if the Modern branch is
// ever re-keyed on Mcp-Session-Id presence (i.e. reuses a known client SID as
// the routing token, or calls sessionManager.Get/AddWithID for it at all).
func TestModernNeverReusesClientSessionIDAsRoutingToken(t *testing.T) {
t.Parallel()

proxy := NewHTTPProxy("127.0.0.1", 0, nil, nil)
t.Cleanup(func() { _ = proxy.Stop(context.Background()) })

knownSessionID := uuid.New().String()
require.NoError(t, proxy.sessionManager.AddWithID(knownSessionID))

newModernRequest := func() *jsonrpc2.Request {
req, err := jsonrpc2.NewCall(jsonrpc2.Int64ID(1), "tools/call", json.RawMessage(
`{"_meta":{"io.modelcontextprotocol/protocolVersion":"2026-07-28",`+
`"io.modelcontextprotocol/clientCapabilities":{}}}`))
require.NoError(t, err)
return req
}

resolve := func() string {
httpReq := httptest.NewRequest(http.MethodPost, StreamableHTTPEndpoint, http.NoBody)
httpReq.Header.Set("MCP-Protocol-Version", mcp.MCPVersionModern)
httpReq.Header.Set("Mcp-Session-Id", knownSessionID)

token, setHeader, err := proxy.resolveSessionForRequest(httptest.NewRecorder(), httpReq, newModernRequest())
require.NoError(t, err)
assert.False(t, setHeader, "Modern branch must never ask the client to adopt a session")
return token
}

token1 := resolve()
token2 := resolve()

assert.NotEqual(t, knownSessionID, token1, "Modern branch must not reuse the known client session id as its routing token")
assert.NotEqual(t, knownSessionID, token2, "Modern branch must not reuse the known client session id as its routing token")
assert.NotEqual(t, token1, token2, "each Modern request must mint its own fresh routing token")

_, err := uuid.Parse(token1)
assert.NoError(t, err, "routing token must be a freshly minted UUID, not a passthrough of client input")
_, err = uuid.Parse(token2)
assert.NoError(t, err, "routing token must be a freshly minted UUID, not a passthrough of client input")
}

// TestModernClassificationErrorsReturn400 verifies that ClassifyRevision errors
// on the single-request path are rendered as HTTP 400 JSON-RPC error responses
// with the spec-defined error code.
Expand Down
Loading
Loading