mirror of
https://github.com/coder/coder.git
synced 2026-09-22 05:05:20 +08:00
fix: fix nil pointer dereference in ReportTask (#19045)
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 <tk@coder.com>
This commit is contained in:
+6
-2
@@ -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
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user