From ce0b198202c461787ead7799bac94e90d9cddd91 Mon Sep 17 00:00:00 2001 From: PratikDhanave Date: Fri, 21 Aug 2026 14:23:36 +0530 Subject: [PATCH] mcp: prevent MemoryEventStore.After panic on index past the end (#1132) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary `MemoryEventStore.After` guarded only the lower bound of the requested index: ```go start := index + 1 if dl.first > start { // too old → ErrEventsPurged return nil, fmt.Errorf(... ErrEventsPurged) } return slices.Clone(dl.data[start-dl.first:]), nil ``` There was no *upper*-bound guard, so an `index` greater than the highest stored event made the final slice expression panic with `slice bounds out of range`. The boundary `start-dl.first == len(dl.data)` (resume from the last event seen) is a valid empty replay, so only strictly-greater indices faulted. This is reachable from untrusted input: `parseEventID` accepts any non-negative index from the client-supplied `Last-Event-ID` header, and `serveGET` passes it straight to `eventStore.After`. Direct callers of the exported `EventStore.After` API crash outright. Fixes #1131 ## Change Return an empty replay for an index at or beyond the latest stored event — there is nothing after it — consistent with the already-valid `index == last` case. `ErrEventsPurged` would be misleading here, since those events were never sent rather than purged. ## Testing Extended `TestMemoryEventStoreAfter` with indices past the end of a stream (`3`, `100`, and one past a second stream), asserting an empty replay with no error. Verified the new cases **panic on the pre-fix code** (`panic: runtime error: slice bounds out of range [3:2]`) and **pass with the fix**. `go vet ./mcp/` and the full `go test ./mcp/` suite pass. --------- Co-authored-by: Yaroslav Shevchuk --- mcp/event.go | 9 ++++++--- mcp/event_test.go | 4 ++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/mcp/event.go b/mcp/event.go index bbb3e6c..2b02a07 100644 --- a/mcp/event.go +++ b/mcp/event.go @@ -337,12 +337,15 @@ func (s *MemoryEventStore) After(_ context.Context, sessionID, streamID string, if !ok { return nil, fmt.Errorf("MemoryEventStore.After: unknown stream ID %v in session %q", streamID, sessionID) } - start := index + 1 - if dl.first > start { + start := (index + 1) - dl.first + if start < 0 { return nil, fmt.Errorf("MemoryEventStore.After: index %d, stream ID %v, session %q: %w", index, streamID, sessionID, ErrEventsPurged) } - return slices.Clone(dl.data[start-dl.first:]), nil + if start >= len(dl.data) { + return nil, nil + } + return slices.Clone(dl.data[start:]), nil } return func(yield func([]byte, error) bool) { diff --git a/mcp/event_test.go b/mcp/event_test.go index 91ddb9d..fedc92c 100644 --- a/mcp/event_test.go +++ b/mcp/event_test.go @@ -299,7 +299,11 @@ func TestMemoryEventStoreAfter(t *testing.T) { {"S1", "1", 0, []string{"d2", "d3"}, ""}, {"S1", "1", 1, []string{"d3"}, ""}, {"S1", "1", 2, nil, ""}, + {"S1", "1", 3, nil, ""}, + {"S1", "1", 100, nil, ""}, // past the end of stream "1" (highest is 2) {"S1", "2", 0, nil, ""}, + {"S1", "2", 1, nil, ""}, + {"S1", "2", 100, nil, ""}, {"S1", "3", 0, nil, "unknown stream ID"}, {"S2", "0", 0, nil, "unknown session ID"}, } {