fix: fix nil pointer dereference in ReportTask (#19045)

ThomasK33 · web-flow · commit d1595781e13f · 2025-07-28T11:23:29.000+02:00
This pull request addresses a bug related to a nil pointer dereference
in the task reporting functionality.

### Bug Fixes and Error Handling:

* Updated `RegisterTools` in `mcp.go` to skip registering the
`ReportTask` tool in the remote MCP context when a task reporter is not
configured, preventing potential nil pointer dereference panics.
* Added a check in `toolsdk.go` to ensure task reporting dependencies
are available before invoking the reporter, returning an appropriate
error if not.

### Test Coverage:

* Added `TestReportTaskNilPointerDeref` in `toolsdk_test.go` to verify
that the system does not panic when task reporting dependencies are
missing and instead returns a clear error message.
* Added `TestReportTaskWithReporter` in `toolsdk_test.go` to validate
correct behavior when a task reporter is configured, ensuring the
handler processes the request as expected.

Signed-off-by: Thomas Kosiewski &lt;tk@coder.com&gt;
diff --git a/coderd/mcp/mcp.go b/coderd/mcp/mcp.go
@@ -79,11 +79,15 @@ func (s *Server) RegisterTools(client *codersdk.Client) error {
 		return xerrors.Errorf("failed to initialize tool dependencies: %w", err)
 	}
 
-	// Register all available tools
+	// Register all available tools, but exclude tools that require dependencies not available in the
+	// remote MCP context
 	for _, tool := range toolsdk.All {
+		if tool.Name == toolsdk.ToolNameReportTask {
+			continue
+		}
+
 		s.mcpServer.AddTools(mcpFromSDK(tool, toolDeps))
 	}
-
 	return nil
 }
 
diff --git a/codersdk/toolsdk/toolsdk.go b/codersdk/toolsdk/toolsdk.go
@@ -253,6 +253,10 @@ ONLY report an "idle" or "failure" state if you have FULLY completed the task.
 		if len(args.Summary) > 160 {
 			return codersdk.Response{}, xerrors.New("summary must be less than 160 characters")
 		}
+		// Check if task reporting is available to prevent nil pointer dereference
+		if deps.report == nil {
+			return codersdk.Response{}, xerrors.New("task reporting not available. Please ensure a task reporter is configured.")
+		}
 		err := deps.report(args)
 		if err != nil {
 			return codersdk.Response{}, err
diff --git a/codersdk/toolsdk/toolsdk_test.go b/codersdk/toolsdk/toolsdk_test.go
@@ -686,3 +686,57 @@ func TestMain(m *testing.M) {
 
 	os.Exit(code)
 }
+
+func TestReportTaskNilPointerDeref(t *testing.T) {
+	t.Parallel()
+
+	// Create deps without a task reporter (simulating remote MCP server scenario)
+	client, _ := coderdtest.NewWithDatabase(t, nil)
+	deps, err := toolsdk.NewDeps(client)
+	require.NoError(t, err)
+
+	// Prepare test arguments
+	args := toolsdk.ReportTaskArgs{
+		Summary: "Test task",
+		Link:    "https://example.com",
+		State:   string(codersdk.WorkspaceAppStatusStateWorking),
+	}
+
+	_, err = toolsdk.ReportTask.Handler(t.Context(), deps, args)
+
+	// We expect an error, not a panic
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "task reporting not available")
+}
+
+func TestReportTaskWithReporter(t *testing.T) {
+	t.Parallel()
+
+	// Create deps with a task reporter
+	client, _ := coderdtest.NewWithDatabase(t, nil)
+
+	called := false
+	reporter := func(args toolsdk.ReportTaskArgs) error {
+		called = true
+		require.Equal(t, "Test task", args.Summary)
+		require.Equal(t, "https://example.com", args.Link)
+		require.Equal(t, string(codersdk.WorkspaceAppStatusStateWorking), args.State)
+		return nil
+	}
+
+	deps, err := toolsdk.NewDeps(client, toolsdk.WithTaskReporter(reporter))
+	require.NoError(t, err)
+
+	args := toolsdk.ReportTaskArgs{
+		Summary: "Test task",
+		Link:    "https://example.com",
+		State:   string(codersdk.WorkspaceAppStatusStateWorking),
+	}
+
+	result, err := toolsdk.ReportTask.Handler(t.Context(), deps, args)
+	require.NoError(t, err)
+	require.True(t, called)
+
+	// Verify response
+	require.Equal(t, "Thanks for reporting!", result.Message)
+}

Original file line number	Diff line number	Diff line change
`@@ -79,11 +79,15 @@ func (s Server) RegisterTools(client codersdk.Client) error {`
`79`	`79`	`return xerrors.Errorf("failed to initialize tool dependencies: %w", err)`
`80`	`80`	`}`
`81`	`81`
`82`		`- // Register all available tools`
	`82`	`+ // Register all available tools, but exclude tools that require dependencies not available in the`
	`83`	`+ // remote MCP context`
`83`	`84`	`for _, tool := range toolsdk.All {`
	`85`	`+ if tool.Name == toolsdk.ToolNameReportTask {`
	`86`	`+ continue`
	`87`	`+ }`
	`88`	`+`
`84`	`89`	`s.mcpServer.AddTools(mcpFromSDK(tool, toolDeps))`
`85`	`90`	`}`
`86`		`-`
`87`	`91`	`return nil`
`88`	`92`	`}`
`89`	`93`