Skip to content

fix: guard nil map in web search context to prevent plugin crash - #935

Open
crspeller wants to merge 1 commit into
masterfrom
cursor/fix-web-search-context-nil-map-panic-bb5c
Open

fix: guard nil map in web search context to prevent plugin crash#935
crspeller wants to merge 1 commit into
masterfrom
cursor/fix-web-search-context-nil-map-panic-bb5c

Conversation

@crspeller

@crspeller crspeller commented Jul 28, 2026

Copy link
Copy Markdown
Member

Summary

Fixes a crash when parsing the web_search_context post prop.

In conversations/web_search_context.go, unmarshalWebSearchContext unmarshals the prop value into a map[string]interface{}. For certain inputs json.Unmarshal succeeds but leaves the map nil, and the unconditional writes that follow then panic:

panic: assignment to entry in nil map
  conversations/web_search_context.go:100

Fix: guard against a nil map right after a successful unmarshal (if params == nil { params = make(map[string]interface{}) }). This is the minimal change that addresses the root cause — the subsequent writes are now always safe regardless of the prop value.

Added a table-driven test (TestUnmarshalWebSearchContext) covering the nil-map case, empty object, a valid context payload, invalid JSON, and non-object JSON.

QA / proof:

Before the fix, the parsing path panicked at web_search_context.go:100 (assignment to entry in nil map). After the fix, all cases pass:

--- PASS: TestUnmarshalWebSearchContext (0.00s)
    --- PASS: TestUnmarshalWebSearchContext/literal_null_does_not_panic (0.00s)
    --- PASS: TestUnmarshalWebSearchContext/empty_object_resets_tracking (0.00s)
    --- PASS: TestUnmarshalWebSearchContext/valid_context_is_parsed_and_tracking_reset (0.00s)
    --- PASS: TestUnmarshalWebSearchContext/invalid_json_returns_nil (0.00s)
    --- PASS: TestUnmarshalWebSearchContext/non-object_json_returns_nil (0.00s)
ok  github.com/mattermost/mattermost-plugin-agents/v2/conversations

Full conversations package tests, go vet, and golangci-lint all pass.

Ticket Link

Release Note

Fixed a crash when parsing certain values of the web_search_context post prop.
Open in Web Open in Cursor 

Summary by CodeRabbit

  • Bug Fixes

    • Fixed an issue that could cause web search context processing to fail when stored context was empty or represented as null.
    • Improved handling of invalid or non-object context data to prevent unexpected errors.
  • Tests

    • Added coverage for empty, valid, null, invalid, and non-object context values.

Unmarshaling the literal JSON "null" from the web_search_context post
prop succeeds and leaves the target map nil. The unconditional writes
that followed panicked with "assignment to entry in nil map", crashing
the plugin process and taking all AI routes offline. Any authenticated
user could trigger this by DMing a bot with props.web_search_context
set to "null".

Guard against a nil map after unmarshaling and add a table-driven test
covering null, empty-object, valid, and invalid inputs.

Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The web search context unmarshaller now initializes nil maps before resetting tracking fields. Table-driven tests cover null, empty, valid, invalid, and non-object JSON inputs.

Changes

Web search context handling

Layer / File(s) Summary
Null context handling and regression tests
conversations/web_search_context.go, conversations/web_search_context_test.go
The unmarshaller prevents nil-map writes after "null" input, and tests tracking resets, valid contexts, invalid JSON, and non-object values.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested labels: Setup Cloud Test Server

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main fix: guarding a nil web search context map to prevent crashes.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cursor/fix-web-search-context-nil-map-panic-bb5c

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

🤖 LLM Evaluation Results

OpenAI

⚠️ Overall: 22/28 tests passed (78.6%)

Provider Total Passed Failed Pass Rate
⚠️ OPENAI 28 22 6 78.6%

❌ Failed Evaluations

Show 6 failures

OPENAI

1. TestConversationMentionHandling/[openai]_conversation_from_attribution_long_thread.json

  • Score: 0.00
  • Rubric: is a list of bugs
  • Reason: The output does not provide an actual list of bugs; it states it cannot access the bug tracker and instead provides a template to fill in. Therefore it is not a list of bugs.

2. TestConversationMentionHandling/[openai]_conversation_from_attribution_long_thread.json

  • Score: 0.00
  • Rubric: includes a description of each bug
  • Reason: The output does not provide descriptions of actual bugs; it only states it cannot access the tracker and provides a template/fields to fill in later. Therefore it does not include a description of each bug.

3. TestConversationMentionHandling/[openai]_conversation_from_attribution_long_thread.json

  • Score: 0.00
  • Rubric: attributes each bug to a user
  • Reason: The output does not list any bugs and therefore does not attribute each bug to a specific user; it only provides a template and asks for the bug reports to be shared.

4. TestConversationMentionHandling/[openai]_conversation_from_attribution_long_thread.json

  • Score: 0.00
  • Rubric: attributes the bug about trying to save without a color and the save button not doing anything to @maria.nunez
  • Reason: The output does not mention the specific bug about trying to save without a color and the save button not doing anything, nor does it attribute that bug to @maria.nunez. It only provides a generic request for bug reports and a template.

5. TestConversationMentionHandling/[openai]_conversation_from_attribution_long_thread.json

  • Score: 0.00
  • Rubric: the bug about the end user being able to change channel banner is attributed to @maria.nunez
  • Reason: The output does not mention any specific bug about an end user being able to change the channel banner, nor does it attribute such a bug to @maria.nunez. It only provides a generic template and request for more information.

6. TestDirectMessageConversations/[openai]_bot_dm_tool_introspection

  • Score: 0.00
  • Rubric: mentions Github and refers to the documentation
  • Reason: The output refers to documentation (docs.mattermost.com) but does not mention GitHub anywhere. Since the rubric requires both mentioning GitHub and referring to the documentation, it fails.

Anthropic

⚠️ Overall: 21/28 tests passed (75.0%)

Provider Total Passed Failed Pass Rate
⚠️ ANTHROPIC 28 21 7 75.0%

❌ Failed Evaluations

Show 7 failures

ANTHROPIC

1. TestReactEval/[anthropic]_react_cat_message

  • Score: 0.00
  • Rubric: The word/emoji is a cat emoji or a heart/love emoji
  • Reason: The output is the literal text "heart_eyes_cat", not an actual cat emoji (e.g., 😺/🐱) or a heart/love emoji (e.g., ❤️/😍).

2. TestConversationMentionHandling/[anthropic]_conversation_from_attribution_long_thread.json

  • Score: 0.00
  • Rubric: is a list of bugs
  • Reason: The output states it cannot access bug trackers and suggests ways to obtain a list, but it does not actually provide a list of bugs.

3. TestConversationMentionHandling/[anthropic]_conversation_from_attribution_long_thread.json

  • Score: 0.00
  • Rubric: includes a description of each bug
  • Reason: The output does not list any bugs or provide descriptions of each bug; it only states lack of access and suggests ways to obtain bug information.

4. TestConversationMentionHandling/[anthropic]_conversation_from_attribution_long_thread.json

  • Score: 0.00
  • Rubric: attributes each bug to a user
  • Reason: The output does not list any bugs at all, and therefore does not attribute each bug to a user.

5. TestConversationMentionHandling/[anthropic]_conversation_from_attribution_long_thread.json

  • Score: 0.00
  • Rubric: attributes the bug about trying to save without a color and the save button not doing anything to @maria.nunez
  • Reason: The output does not attribute any specific bug (saving without a color and the save button not doing anything) to @maria.nunez; it only states lack of access and suggests ways to retrieve bug lists.

6. TestConversationMentionHandling/[anthropic]_conversation_from_attribution_long_thread.json

  • Score: 0.00
  • Rubric: the bug about the end user being able to change channel banner is attributed to @maria.nunez
  • Reason: The output contains a general statement about not having access to bug tracking systems and suggestions for how to find bugs. It does not mention the specific bug about an end user being able to change the channel banner, nor does it attribute that bug to @maria.nunez.

7. TestDirectMessageConversations/[anthropic]_bot_dm_tool_introspection

  • Score: 0.50
  • Rubric: mentions Github and refers to the documentation
  • Reason: The output refers to documentation (docs.mattermost.com) but does not mention GitHub anywhere, so it does not satisfy the rubric requirement to mention GitHub and refer to the documentation.

This comment was automatically generated by the eval CI pipeline.

@crspeller
crspeller marked this pull request as ready for review July 28, 2026 16:03
@crspeller
crspeller requested a review from nickmisasi July 28, 2026 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants