From 18f16878a84952cf5d0ddb583385dc340d1f5ff3 Mon Sep 17 00:00:00 2001 From: kolaente Date: Wed, 4 Mar 2026 10:22:18 +0100 Subject: [PATCH] fix: prevent nil pointer panic in mention notification listeners When a task bucket is updated without changing buckets (early return in updateTaskBucket), b.Task remains nil. The TaskUpdatedEvent was then dispatched with a nil Task, causing a nil pointer dereference in HandleTaskUpdatedMentions when accessing event.Task.Description. This adds: - A nil guard in TaskBucket.Update to skip event dispatch when b.Task is nil - Nil checks in HandleTaskUpdatedMentions, HandleTaskCreateMentions, HandleTaskCommentEditMentions, and UpdateTaskInSavedFilterViews - Tests verifying the handlers gracefully handle nil task events Closes #2351 --- pkg/models/kanban_task_bucket.go | 12 +++++++----- pkg/models/listeners.go | 16 ++++++++++++++++ pkg/models/mentions_test.go | 13 +++++++++++++ 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/pkg/models/kanban_task_bucket.go b/pkg/models/kanban_task_bucket.go index b19e6fb0c..35c623a0f 100644 --- a/pkg/models/kanban_task_bucket.go +++ b/pkg/models/kanban_task_bucket.go @@ -239,10 +239,12 @@ func (b *TaskBucket) Update(s *xorm.Session, a web.Auth) (err error) { return err } - doer, _ := user.GetFromAuth(a) - events.DispatchOnCommit(s, &TaskUpdatedEvent{ - Task: b.Task, - Doer: doer, - }) + if b.Task != nil { + doer, _ := user.GetFromAuth(a) + events.DispatchOnCommit(s, &TaskUpdatedEvent{ + Task: b.Task, + Doer: doer, + }) + } return nil } diff --git a/pkg/models/listeners.go b/pkg/models/listeners.go index 05ddec492..f139f9c43 100644 --- a/pkg/models/listeners.go +++ b/pkg/models/listeners.go @@ -245,6 +245,10 @@ func (s *HandleTaskCommentEditMentions) Handle(msg *message.Message) (err error) return err } + if event.Task == nil || event.Comment == nil { + return nil + } + sess := db.NewSession() defer sess.Close() @@ -390,6 +394,10 @@ func (s *HandleTaskCreateMentions) Handle(msg *message.Message) (err error) { return err } + if event.Task == nil { + return nil + } + sess := db.NewSession() defer sess.Close() @@ -422,6 +430,10 @@ func (s *HandleTaskUpdatedMentions) Handle(msg *message.Message) (err error) { return err } + if event.Task == nil { + return nil + } + sess := db.NewSession() defer sess.Close() @@ -541,6 +553,10 @@ func (l *UpdateTaskInSavedFilterViews) Handle(msg *message.Message) (err error) return err } + if event.Task == nil { + return nil + } + // This operation is potentially very resource-heavy, because we don't know if a task is included // in a filter until we evaluate that filter. We need to evaluate each filter individually - since // there can be many filters, this can take a while to execute. diff --git a/pkg/models/mentions_test.go b/pkg/models/mentions_test.go index a4a085482..4d8065807 100644 --- a/pkg/models/mentions_test.go +++ b/pkg/models/mentions_test.go @@ -22,6 +22,7 @@ import ( "regexp" "code.vikunja.io/api/pkg/db" + "code.vikunja.io/api/pkg/events" "code.vikunja.io/api/pkg/notifications" "code.vikunja.io/api/pkg/user" @@ -94,6 +95,18 @@ func TestFindMentionedUsersInText(t *testing.T) { } } +func TestHandleMentionsWithNilTask(t *testing.T) { + t.Run("HandleTaskUpdatedMentions should not panic with nil task", func(t *testing.T) { + events.TestListener(t, &TaskUpdatedEvent{Task: nil, Doer: nil}, &HandleTaskUpdatedMentions{}) + }) + t.Run("HandleTaskCreateMentions should not panic with nil task", func(t *testing.T) { + events.TestListener(t, &TaskCreatedEvent{Task: nil, Doer: nil}, &HandleTaskCreateMentions{}) + }) + t.Run("HandleTaskCommentEditMentions should not panic with nil task", func(t *testing.T) { + events.TestListener(t, &TaskCommentUpdatedEvent{Task: nil, Comment: nil, Doer: nil}, &HandleTaskCommentEditMentions{}) + }) +} + func TestSendingMentionNotification(t *testing.T) { u := &user.User{ID: 1}