From f1618ee59b6d1c0308bb474c827a2b1f24b12a95 Mon Sep 17 00:00:00 2001 From: Gani Georgiev Date: Wed, 15 Jul 2026 12:03:07 +0300 Subject: [PATCH] fixed unhandled panic and wrapped all internal goroutines --- CHANGELOG.md | 3 ++ apis/batch.go | 5 ++-- apis/realtime.go | 26 ++++++++--------- core/base.go | 4 +-- core/notify_watcher.go | 5 ++-- plugins/jsvm/jsvm.go | 5 ++-- pocketbase.go | 8 +++--- tools/cron/cron.go | 4 +-- .../filesystem/internal/s3blob/s3/uploader.go | 9 +++--- tools/filesystem/internal/s3blob/s3blob.go | 5 ++-- tools/routine/routine.go | 15 ++++++++++ tools/routine/routine_test.go | 28 +++++++++++++++++-- tools/search/provider.go | 5 ++-- 13 files changed, 85 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a94378f6..563b892e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ - Fixed View collection `*` validator and added more friendly error messages ([#7761](https://github.com/pocketbase/pocketbase/issues/7761)). +- ⚠️ Security fix for unhandled panic in internal worker goroutines ([#7762](https://github.com/pocketbase/pocketbase/discussions/7762)). + _To prevent this from showing again, all existing internal worker functions were wrapped with [`routine.SafeWrap(f)`](https://pkg.go.dev/github.com/pocketbase/pocketbase/tools/routine#SafeWrap) (auto recovers and returns any eventual panic as regular error)._ + ## v0.39.6 diff --git a/apis/batch.go b/apis/batch.go index 9542640b..61d7bcd4 100644 --- a/apis/batch.go +++ b/apis/batch.go @@ -18,6 +18,7 @@ import ( "github.com/pocketbase/pocketbase/core" "github.com/pocketbase/pocketbase/tools/filesystem" "github.com/pocketbase/pocketbase/tools/router" + "github.com/pocketbase/pocketbase/tools/routine" "github.com/pocketbase/pocketbase/tools/types" "github.com/spf13/cast" ) @@ -195,7 +196,7 @@ func (p *batchProcessor) Process(batch []*core.InternalRequest, timeout time.Dur p.stopCh <- struct{}{} }() - go func() { + routine.FireAndForget(func() { err := p.process(txApp, batch, 0) if err != nil { @@ -216,7 +217,7 @@ func (p *batchProcessor) Process(batch []*core.InternalRequest, timeout time.Dur } p.errCh <- err - }() + }) select { case responseErr := <-p.errCh: diff --git a/apis/realtime.go b/apis/realtime.go index 72e437c4..455fa478 100644 --- a/apis/realtime.go +++ b/apis/realtime.go @@ -10,8 +10,8 @@ import ( "strings" "time" - validation "github.com/pocketbase/ozzo-validation/v4" "github.com/pocketbase/dbx" + validation "github.com/pocketbase/ozzo-validation/v4" "github.com/pocketbase/pocketbase/core" "github.com/pocketbase/pocketbase/tools/hook" "github.com/pocketbase/pocketbase/tools/picker" @@ -260,7 +260,7 @@ func realtimeUpdateClientsAuth(app core.App, authRecord *core.Record) error { group := new(errgroup.Group) for _, chunk := range chunks { - group.Go(func() error { + group.Go(routine.SafeWrap(func() error { for _, client := range chunk { clientAuth, _ := client.Get(RealtimeClientAuthKey).(*core.Record) if clientAuth != nil && @@ -275,7 +275,7 @@ func realtimeUpdateClientsAuth(app core.App, authRecord *core.Record) error { } return nil - }) + })) } return group.Wait() @@ -288,7 +288,7 @@ func realtimeUnsetClientsAuthByRecordModelOrProxy(app core.App, authModel core.M group := new(errgroup.Group) for _, chunk := range chunks { - group.Go(func() error { + group.Go(routine.SafeWrap(func() error { for _, client := range chunk { clientAuth, _ := client.Get(RealtimeClientAuthKey).(*core.Record) if clientAuth != nil && @@ -299,7 +299,7 @@ func realtimeUnsetClientsAuthByRecordModelOrProxy(app core.App, authModel core.M } return nil - }) + })) } return group.Wait() @@ -312,7 +312,7 @@ func realtimeUnsetClientsAuthByCollection(app core.App, collection *core.Collect group := new(errgroup.Group) for _, chunk := range chunks { - group.Go(func() error { + group.Go(routine.SafeWrap(func() error { for _, client := range chunk { clientAuth, _ := client.Get(RealtimeClientAuthKey).(*core.Record) if clientAuth != nil && clientAuth.Collection().Name == collection.Name { @@ -321,7 +321,7 @@ func realtimeUnsetClientsAuthByCollection(app core.App, collection *core.Collect } return nil - }) + })) } return group.Wait() @@ -623,7 +623,7 @@ func realtimeBroadcastRecord(app core.App, action string, record *core.Record, d } for _, chunk := range chunks { - group.Go(func() error { + group.Go(routine.SafeWrap(func() error { var clientAuth *core.Record for _, client := range chunk { @@ -766,7 +766,7 @@ func realtimeBroadcastRecord(app core.App, action string, record *core.Record, d } return nil - }) + })) } return group.Wait() @@ -782,7 +782,7 @@ func realtimeBroadcastDryCacheKey(app core.App, key string) error { group := new(errgroup.Group) for _, chunk := range chunks { - group.Go(func() error { + group.Go(routine.SafeWrap(func() error { for _, client := range chunk { messages, ok := client.Get(key).([]subscriptions.Message) if !ok { @@ -801,7 +801,7 @@ func realtimeBroadcastDryCacheKey(app core.App, key string) error { } return nil - }) + })) } return group.Wait() @@ -817,7 +817,7 @@ func realtimeUnsetDryCacheKey(app core.App, key string) error { group := new(errgroup.Group) for _, chunk := range chunks { - group.Go(func() error { + group.Go(routine.SafeWrap(func() error { for _, client := range chunk { if client.Get(key) != nil { client.Unset(key) @@ -825,7 +825,7 @@ func realtimeUnsetDryCacheKey(app core.App, key string) error { } return nil - }) + })) } return group.Wait() diff --git a/core/base.go b/core/base.go index 4551e3f4..0fbcb6aa 100644 --- a/core/base.go +++ b/core/base.go @@ -1458,7 +1458,7 @@ func (app *BaseApp) initLogger() error { }, }) - go func() { + routine.FireAndForget(func() { ctx := context.Background() for { @@ -1469,7 +1469,7 @@ func (app *BaseApp) initLogger() error { handler.WriteAll(ctx) } } - }() + }) app.logger = slog.New(handler) diff --git a/core/notify_watcher.go b/core/notify_watcher.go index 1f548cda..40d0a166 100644 --- a/core/notify_watcher.go +++ b/core/notify_watcher.go @@ -10,6 +10,7 @@ import ( "github.com/fatih/color" "github.com/fsnotify/fsnotify" "github.com/pocketbase/pocketbase/tools/hook" + "github.com/pocketbase/pocketbase/tools/routine" "github.com/pocketbase/pocketbase/tools/security" ) @@ -152,7 +153,7 @@ func createNotifyDirWatcher(app App, instanceId string, localNotifyDirPath strin } // watch - go func() { + routine.FireAndForget(func() { defer stopDebounceTimer() for { @@ -204,7 +205,7 @@ func createNotifyDirWatcher(app App, instanceId string, localNotifyDirPath strin } } } - }() + }) return watcher, err } diff --git a/plugins/jsvm/jsvm.go b/plugins/jsvm/jsvm.go index c26330aa..38038cc8 100644 --- a/plugins/jsvm/jsvm.go +++ b/plugins/jsvm/jsvm.go @@ -33,6 +33,7 @@ import ( "github.com/fsnotify/fsnotify" "github.com/pocketbase/pocketbase/core" "github.com/pocketbase/pocketbase/plugins/jsvm/internal/types/generated" + "github.com/pocketbase/pocketbase/tools/routine" "github.com/pocketbase/pocketbase/tools/template" ) @@ -407,7 +408,7 @@ func (p *plugin) watchHooks() error { }) // start listening for events. - go func() { + routine.FireAndForget(func() { defer stopDebounceTimer() for { @@ -437,7 +438,7 @@ func (p *plugin) watchHooks() error { color.Red("Watch error:", err) } } - }() + }) // add directories to watch // diff --git a/pocketbase.go b/pocketbase.go index 91b92c81..9f0e1a74 100644 --- a/pocketbase.go +++ b/pocketbase.go @@ -186,21 +186,21 @@ func (pb *PocketBase) Execute() error { done := make(chan bool, 1) // listen for interrupt signal to gracefully shutdown the application - go func() { + routine.FireAndForget(func() { sigch := make(chan os.Signal, 1) signal.Notify(sigch, os.Interrupt, syscall.SIGTERM) <-sigch done <- true - }() + }) // execute the root command - go func() { + routine.FireAndForget(func() { // note: leave to the commands to decide whether to print their error pb.RootCmd.Execute() done <- true - }() + }) <-done diff --git a/tools/cron/cron.go b/tools/cron/cron.go index 595f5814..387da7c8 100644 --- a/tools/cron/cron.go +++ b/tools/cron/cron.go @@ -193,7 +193,7 @@ func (c *Cron) Start() { c.runDue(time.Now()) // run after each tick - go func() { + routine.FireAndForget(func() { for { select { case <-c.tickerDone: @@ -202,7 +202,7 @@ func (c *Cron) Start() { c.runDue(t) } } - }() + }) }) c.mux.Unlock() } diff --git a/tools/filesystem/internal/s3blob/s3/uploader.go b/tools/filesystem/internal/s3blob/s3/uploader.go index 1c12ba3a..5c759392 100644 --- a/tools/filesystem/internal/s3blob/s3/uploader.go +++ b/tools/filesystem/internal/s3blob/s3/uploader.go @@ -14,6 +14,7 @@ import ( "strings" "sync" + "github.com/pocketbase/pocketbase/tools/routine" "golang.org/x/sync/errgroup" ) @@ -339,7 +340,7 @@ func (u *Uploader) multipartUpload(ctx context.Context, initPart []byte, optReqF if len(initPart) != 0 { totalWorkers-- initPartNumber := u.lastPartNumber - g.Go(func() error { + g.Go(routine.SafeWrap(func() error { mp, err := u.uploadPart(ctx, initPartNumber, initPart, optReqFuncs...) if err != nil { return err @@ -350,13 +351,13 @@ func (u *Uploader) multipartUpload(ctx context.Context, initPart []byte, optReqF u.mu.Unlock() return nil - }) + })) } totalWorkers = max(totalWorkers, 1) for i := 0; i < totalWorkers; i++ { - g.Go(func() error { + g.Go(routine.SafeWrap(func() error { for { part, num, err := u.nextPart() if err != nil { @@ -377,7 +378,7 @@ func (u *Uploader) multipartUpload(ctx context.Context, initPart []byte, optReqF } return nil - }) + })) } return g.Wait() diff --git a/tools/filesystem/internal/s3blob/s3blob.go b/tools/filesystem/internal/s3blob/s3blob.go index 041bc1de..303b2670 100644 --- a/tools/filesystem/internal/s3blob/s3blob.go +++ b/tools/filesystem/internal/s3blob/s3blob.go @@ -42,6 +42,7 @@ import ( "github.com/pocketbase/pocketbase/tools/filesystem/blob" "github.com/pocketbase/pocketbase/tools/filesystem/internal/s3blob/s3" + "github.com/pocketbase/pocketbase/tools/routine" ) const defaultPageSize = 1000 @@ -359,7 +360,7 @@ func (w *writer) Write(p []byte) (int, error) { // error uploading to S3. func (w *writer) open(r io.Reader, closePipeOnError bool) { // This goroutine will keep running until Close, unless there's an error. - go func() { + routine.FireAndForget(func() { defer func() { close(w.donec) }() @@ -378,7 +379,7 @@ func (w *writer) open(r io.Reader, closePipeOnError bool) { } w.err = err } - }() + }) } // Close completes the writer and closes it. Any error occurring during write diff --git a/tools/routine/routine.go b/tools/routine/routine.go index 3398654a..49d2a756 100644 --- a/tools/routine/routine.go +++ b/tools/routine/routine.go @@ -1,6 +1,7 @@ package routine import ( + "fmt" "log" "runtime" "sync" @@ -33,3 +34,17 @@ func FireAndForget(f func(), wg ...*sync.WaitGroup) { f() }() } + +// SafeWrap wraps the provided function with auto panic recover handling +// and returns any eventual panic as regular error. +func SafeWrap(f func() error) func() error { + return func() (err error) { + defer func() { + if r := recover(); r != nil { + err = fmt.Errorf("[SafeWrap] recovered from panic: %v", r) + } + }() + + return f() + } +} diff --git a/tools/routine/routine_test.go b/tools/routine/routine_test.go index dcb6ace6..1032a883 100644 --- a/tools/routine/routine_test.go +++ b/tools/routine/routine_test.go @@ -1,6 +1,7 @@ package routine_test import ( + "strings" "sync" "testing" @@ -12,7 +13,7 @@ func TestFireAndForget(t *testing.T) { fn := func() { called = true - panic("test") + panic("test_recover") } wg := &sync.WaitGroup{} @@ -22,6 +23,29 @@ func TestFireAndForget(t *testing.T) { wg.Wait() if !called { - t.Error("Expected fn to be called.") + t.Fatal("Expected fn to be called.") + } +} + +func TestSafeWrap(t *testing.T) { + called := false + + fn := func() error { + called = true + panic("test_recover") + } + + err := routine.SafeWrap(fn)() + + if !called { + t.Fatal("Expected fn to be called.") + } + + if err == nil { + t.Fatal("Expected fn panic to be converted to error") + } + + if !strings.Contains(err.Error(), "test_recover") { + t.Fatal("Expected the returned error to contain the recovered panic value") } } diff --git a/tools/search/provider.go b/tools/search/provider.go index 02a1f7b7..aa3bb429 100644 --- a/tools/search/provider.go +++ b/tools/search/provider.go @@ -10,6 +10,7 @@ import ( "github.com/pocketbase/dbx" "github.com/pocketbase/pocketbase/tools/dbutils" "github.com/pocketbase/pocketbase/tools/inflector" + "github.com/pocketbase/pocketbase/tools/routine" "golang.org/x/sync/errgroup" ) @@ -336,8 +337,8 @@ func (s *Provider) Exec(items any) (*Result, error) { if !s.skipTotal { // execute the 2 queries concurrently errg := new(errgroup.Group) - errg.Go(countExec) - errg.Go(modelsExec) + errg.Go(routine.SafeWrap(countExec)) + errg.Go(routine.SafeWrap(modelsExec)) if err := errg.Wait(); err != nil { return nil, err }