Bug Description
sseManager.get() in internal/server/mcp.go (lines ~60-65) unconditionally sets session.lastActive = time.Now() without checking if session is nil. When the session ID is not found in the map, Go returns the zero value (nil) for the pointer, and the field assignment panics.
Affected Code
File: internal/server/mcp.go, lines 60-66
func (m *sseManager) get(id string) (*sseSession, bool) {
m.mu.Lock()
defer m.mu.Unlock()
session, ok := m.sseSessions[id]
session.lastActive = time.Now() // PANICS when session is nil
return session, ok
}
Trigger
Any HTTP POST to /mcp with a ?sessionId=<invalid-or-expired-id> query parameter that doesn't match an active SSE session. The caller in httpHandler (line ~434) checks ok after calling get(), but the panic fires inside get() before the guard can execute:
session, ok = s.sseManager.get(sessionId)
if !ok { // <-- too late, already panicked
s.logger.DebugContext(ctx, "sse session not available")
}
Impact
- Severity: Critical — Any malformed, expired, or guessed session ID crashes the HTTP handler
- Any client reconnecting after a server restart with a stale session cookie triggers this
- Trivially reproducible:
curl -X POST "https://server/mcp?sessionId=nonexistent"
Steps to Reproduce
- Start a genai-toolbox MCP server with SSE transport enabled
- Send a POST request with an invalid session ID:
curl -X POST "http://localhost:5000/mcp?sessionId=does-not-exist" \
-H "Content-Type: application/json" \
-d '{"jsonrpc":"2.0","method":"tools/list","id":1}'
- Server panics with nil pointer dereference
Expected Behavior
get() should guard against nil session before accessing fields.
Proposed Solution
func (m *sseManager) get(id string) (*sseSession, bool) {
m.mu.Lock()
defer m.mu.Unlock()
session, ok := m.sseSessions[id]
if ok {
session.lastActive = time.Now()
}
return session, ok
}
This is a one-line fix that matches the existing pattern used in delete() (which already checks ok before accessing the session).
Happy to submit a PR for this fix.
Bug Description
sseManager.get()ininternal/server/mcp.go(lines ~60-65) unconditionally setssession.lastActive = time.Now()without checking ifsessionis nil. When the session ID is not found in the map, Go returns the zero value (nil) for the pointer, and the field assignment panics.Affected Code
File:
internal/server/mcp.go, lines 60-66Trigger
Any HTTP POST to
/mcpwith a?sessionId=<invalid-or-expired-id>query parameter that doesn't match an active SSE session. The caller inhttpHandler(line ~434) checksokafter callingget(), but the panic fires insideget()before the guard can execute:Impact
curl -X POST "https://server/mcp?sessionId=nonexistent"Steps to Reproduce
Expected Behavior
get()should guard against nil session before accessing fields.Proposed Solution
This is a one-line fix that matches the existing pattern used in
delete()(which already checksokbefore accessing the session).Happy to submit a PR for this fix.