Wire rate limit middleware into proxy runner chain

[jerm-dro]

Summary

Rate limiting CRD types and the token-bucket Limiter package were
merged in #4577. This PR wires the limiter into the proxy runner
middleware chain so rate limiting is enforced at runtime.

• Adds HTTP middleware that reads the parsed MCP request from context,
  rate-limits only tools/call requests, returns HTTP 429 with
  JSON-RPC error code -32029 and Retry-After header on rejection,
  and fails open on Redis errors
• Registers the middleware factory and positions it after auth and MCP
  parser in the middleware chain
• Maps spec.rateLimiting from the CRD to the RunConfig in the
  operator reconciler, carrying the CRD type directly
• Adds Ginkgo E2E acceptance tests that deploy Redis + MCPServer in a
  Kind cluster and verify rate limit enforcement

Closes #4551

Type of change

• New feature

Test plan

• Unit tests (task test)
• E2E tests (task test-e2e)
• Linting (task lint-fix)

Unit tests (12 new):

• Middleware handler: allowed, rejected (429 + -32029), fail-open,
  missing MCP context (500), non-tools/call passthrough
• Wiring: addRateLimitMiddleware nil/valid, PopulateMiddlewareConfigs
  presence/absence
• Operator: spec.rateLimiting flows to RunConfig

E2E tests (2 new):

• AC7: Burst of requests exceeds shared limit, 4th request gets HTTP
  429 with JSON-RPC -32029 and retryAfterSeconds
• AC8: MCPServer with both shared and per-tool config is accepted and
  reaches Running phase

Changes

File	Change
pkg/ratelimit/middleware.go	New: factory, handler, JSON-RPC error helpers, MiddlewareParams
pkg/runner/middleware.go	Register factory, add helper, call after MCP parser
pkg/runner/config.go	Add RateLimitConfig and RateLimitNamespace fields
pkg/runner/config_builder.go	Add WithRateLimitConfig builder option
cmd/thv-operator/controllers/mcpserver_runconfig.go	Map spec.rateLimiting to RunConfig
test/e2e/thv-operator/acceptance_tests/	New: Ginkgo E2E suite with Redis deploy and rate limit tests

Special notes for reviewers

• The middleware errors with HTTP 500 if ParsedMCPRequest is missing
  from context — this catches middleware ordering bugs at runtime
• Only tools/call requests are rate-limited; other MCP methods pass
  through unconditionally
• Redis client is created from SessionRedisConfig + the
  THV_SESSION_REDIS_PASSWORD env var, sharing connection info with
  session storage
• E2E tests follow the test/e2e/thv-operator/virtualmcp/ patterns

Large PR Justification

1140 lines across 11 files, but 844 lines are tests (unit + E2E
infrastructure). The 296 lines of production code span 5 files that
form a single logical change: middleware + wiring + operator mapping.

Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 60.39604% with 40 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.75%. Comparing base (257c467) to head (54376f3).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/ratelimit/middleware.go	50.00%	35 Missing and 1 partial ⚠️
pkg/runner/middleware.go	80.95%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4652      +/-   ##
==========================================
- Coverage   68.90%   68.75%   -0.15%     
==========================================
  Files         509      507       -2     
  Lines       52698    52699       +1     
==========================================
- Hits        36310    36233      -77     
- Misses      13580    13664      +84     
+ Partials     2808     2802       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jhrozek]

Review from Claude Code — 4 inline comments, all non-blocking nits.

[ChrisJBurns]

Multi-Agent Consensus Review

Agents consulted: mcp-protocol-expert, kubernetes-expert, security-advisor, code-reviewer, toolhive-expert

Consensus Summary

#	Finding	Consensus	Severity	Action
1	CRD type v1alpha1.RateLimitConfig embedded directly in RunConfig API contract	9/10	MEDIUM	Discuss
2	HTTP 500 for missing ParsedMCPRequest inconsistent with other middleware	7/10	MEDIUM	Discuss
3	Missing unit test: rate limit configured without Redis	8/10	LOW	Fix
4	Hardcoded redisImage instead of images.RedisImage	8/10	LOW	Fix
5	Missing JustAfterEach state dump in E2E suite	7/10	LOW	Fix
6	Duplicate retrySeconds computation	7/10	LOW	Fix
7	AddMiddleware(MiddlewareType, ...) vs config.Type	7/10	LOW	Fix

Overall

Well-structured PR with clean commit separation, correct middleware chain ordering, and solid test coverage (unit + E2E). The middleware follows established factory patterns, fail-open on Redis errors is the right resilience trade-off, and the security properties are sound — Redis key isolation is correct, no rate limit bypass vectors exist, and tool name lookups use pre-built maps (not dynamic key interpolation).

The most architecturally notable point is embedding v1alpha1.RateLimitConfig directly in RunConfig, which is documented as ToolHive's API contract. Other middleware configs (authz, audit, telemetry) use pkg/-local types, and ScalingConfig/SessionRedisConfig are runner-local types with conversion at the operator boundary. The CRD coupling is a team design decision worth making explicitly. The remaining findings are test polish items.

---

Generated with Claude Code

[ChrisJBurns]

[MEDIUM | 9/10 consensus] This embeds a CRD type directly in RunConfig, which is documented as "part of ToolHive's API contract." Other middleware configs (AuthzConfig, AuditConfig, TelemetryConfig) use pkg/-local types, and ScalingConfig/SessionRedisConfig are runner-local types with conversion at the operator boundary. A CRD version bump (v1alpha1 → v1beta1) would be a breaking change to RunConfig serialization, and the Swagger output exposes internal CRD type paths.

Consider defining runner-local types mirroring the CRD structure (like ScalingConfig/SessionRedisConfig) with conversion at the operator boundary — or accept the coupling with a comment noting the v1alpha1 dependency and migration implications.

[jerm-dro]

Intentional. Rate limiting is tightly coupled to the consuming MCPServer and is only used in the Kubernetes operator context. A CRD version rename (v1alpha1 → v1beta1) changes the import path but not the struct layout or JSON tags, so serialization is unaffected. The risk of not building a runner-local mirror is low and can be deferred.

[ChrisJBurns]

[MEDIUM | 7/10 consensus] Returning HTTP 500 here is inconsistent with other middleware handling the same condition:

• authz middleware (pkg/authz/middleware.go:192): returns HTTP 400
• webhook middleware (pkg/webhook/validating/middleware.go:88): passes through to next handler

Since the method filter on line 88 already handles non-tools/call requests, the pass-through approach (like webhooks) may be safer — it avoids rejecting legitimate non-JSON-RPC requests (health checks, GET for SSE streams) if middleware ordering changes in the future.

[jerm-dro]

Fixed. Non-JSON-RPC requests (health checks, SSE streams) have no MCP context and now pass through unconditionally. The nil check and method check are combined into a single guard.

[ChrisJBurns]

[LOW | 8/10 consensus] Missing error path test case: RateLimitConfig set but ScalingConfig/SessionRedis nil. This is the most important validation at the RunConfig level since CEL only covers CRD admission, not direct RunConfig construction.

Suggested change

	},
	tests := []struct {
	name string
	config *RunConfig
	wantAppended bool
	wantErr bool
	}{
	{
	name: "nil RateLimitConfig returns input unchanged",
	config: &RunConfig{},
	wantAppended: false,
	},
	{
	name: "rate limit without Redis returns error",
	config: &RunConfig{
	RateLimitConfig: &v1alpha1.RateLimitConfig{
	Shared: &v1alpha1.RateLimitBucket{
	MaxTokens: 10,
	RefillPeriod: metav1.Duration{Duration: time.Minute},
	},
	},
	},
	wantErr: true,
	},
	{

[jerm-dro]

Fixed. Added test case for RateLimitConfig set with nil ScalingConfig/SessionRedis — verifies the error mentions sessionStorage.

[ChrisJBurns]

[LOW | 8/10 consensus] test/e2e/images/images.go already exports images.RedisImage with the same value "redis:7-alpine", and the scaling tests in virtualmcp/ use it. Using the centralized constant keeps image versions in sync for automated dependency updates (Renovate).

[jerm-dro]

Fixed. Now uses images.RedisImage from the centralized test/e2e/images/ package.