From 6a2cb9bbef12321f438240e0bd08cb0114a8432b Mon Sep 17 00:00:00 2001 From: shinchan-zhai Date: Mon, 23 Mar 2026 10:15:02 +0800 Subject: [PATCH] security+reliability: remove public decrypt endpoint, add panic recovery for goroutines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Security: - Remove /api/crypto/decrypt from public routes. The endpoint allowed anyone to decrypt ciphertext without authentication. Internal callers (exchange/model handlers) use the service directly and are behind auth. Reliability: - Add safe.Go / safe.GoNamed panic recovery wrapper (safe/go.go). Previously 31 goroutines had zero recover() calls — a single panic in any trader goroutine would crash the entire process. - Apply safe.GoNamed to all trader launch paths: - StartAll, RestoreRunning, LoadSingleTrader auto-start - API handler start/restart endpoints - Panics are now logged with full stack traces instead of crashing. --- api/handler_trader.go | 9 +++--- api/server.go | 5 ++-- manager/trader_manager.go | 23 +++++++++------ safe/go.go | 59 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 81 insertions(+), 15 deletions(-) create mode 100644 safe/go.go diff --git a/api/handler_trader.go b/api/handler_trader.go index 452cc2b8..f8e345a1 100644 --- a/api/handler_trader.go +++ b/api/handler_trader.go @@ -7,6 +7,7 @@ import ( "time" "nofx/logger" + "nofx/safe" "nofx/store" "nofx/trader" "nofx/trader/aster" @@ -418,12 +419,12 @@ func (s *Server) handleUpdateTrader(c *gin.Context) { // If trader was running before, restart it with new config if wasRunning { if reloadedTrader, getErr := s.traderManager.GetTrader(traderID); getErr == nil { - go func() { + safe.GoNamed("trader-restart-"+traderID, func() { logger.Infof("▶️ Restarting trader %s with new config...", traderID) if runErr := reloadedTrader.Run(); runErr != nil { logger.Infof("❌ Trader %s runtime error: %v", traderID, runErr) } - }() + }) } } @@ -537,12 +538,12 @@ func (s *Server) handleStartTrader(c *gin.Context) { } // Start trader - go func() { + safe.GoNamed("trader-start-"+traderID, func() { logger.Infof("▶️ Starting trader %s (%s)", traderID, trader.GetName()) if err := trader.Run(); err != nil { logger.Infof("❌ Trader %s runtime error: %v", trader.GetName(), err) } - }() + }) // Update running status in database err = s.store.Trader().UpdateStatus(userID, traderID, true) diff --git a/api/server.go b/api/server.go index 5cc5e6c0..4a9a3315 100644 --- a/api/server.go +++ b/api/server.go @@ -126,10 +126,11 @@ func (s *Server) setupRoutes() { api.POST("/wallet/validate", s.handleWalletValidate) api.POST("/wallet/generate", s.handleWalletGenerate) - // Crypto related endpoints (no authentication required, not exposed to bot) + // Crypto public key & config (needed by frontend before login for transport encryption) api.GET("/crypto/config", s.cryptoHandler.HandleGetCryptoConfig) api.GET("/crypto/public-key", s.cryptoHandler.HandleGetPublicKey) - api.POST("/crypto/decrypt", s.cryptoHandler.HandleDecryptSensitiveData) + // NOTE: /crypto/decrypt moved behind auth — the public endpoint was a security risk + // (allowed anyone to decrypt captured ciphertext without authentication) // Public competition data (no authentication required) s.route(api, "GET", "/traders", "Public trader list", s.handlePublicTraderList) diff --git a/manager/trader_manager.go b/manager/trader_manager.go index ab4b6583..fa151b6e 100644 --- a/manager/trader_manager.go +++ b/manager/trader_manager.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "nofx/logger" + "nofx/safe" "nofx/store" "nofx/trader" "sort" @@ -87,12 +88,14 @@ func (tm *TraderManager) StartAll() { logger.Info("🚀 Starting all traders...") for id, t := range tm.traders { - go func(traderID string, at *trader.AutoTrader) { + traderID, at := id, t + safe.GoNamed("trader-"+at.GetName(), func() { logger.Infof("▶️ Starting %s...", at.GetName()) if err := at.Run(); err != nil { logger.Infof("❌ %s runtime error: %v", at.GetName(), err) } - }(id, t) + }) + _ = traderID } } @@ -135,12 +138,13 @@ func (tm *TraderManager) AutoStartRunningTraders(st *store.Store) { startedCount := 0 for id, t := range tm.traders { if runningTraderIDs[id] { - go func(traderID string, at *trader.AutoTrader) { + at := t + safe.GoNamed("trader-restore-"+at.GetName(), func() { logger.Infof("▶️ Auto-restoring %s...", at.GetName()) if err := at.Run(); err != nil { logger.Infof("❌ %s runtime error: %v", at.GetName(), err) } - }(id, t) + }) startedCount++ } } @@ -726,15 +730,16 @@ func (tm *TraderManager) addTraderFromStore(traderCfg *store.Trader, aiModelCfg // Auto-start if trader was running before shutdown if traderCfg.IsRunning { logger.Infof("🔄 Auto-starting trader '%s' (was running before shutdown)...", traderCfg.Name) - go func(trader *trader.AutoTrader, traderName, traderID, userID string) { - if err := trader.Run(); err != nil { - logger.Warnf("⚠️ Trader '%s' stopped with error: %v", traderName, err) + autoStartTrader, autoStartName, autoStartID, autoStartUserID := at, traderCfg.Name, traderCfg.ID, traderCfg.UserID + safe.GoNamed("trader-autostart-"+autoStartName, func() { + if err := autoStartTrader.Run(); err != nil { + logger.Warnf("⚠️ Trader '%s' stopped with error: %v", autoStartName, err) // Update database to reflect stopped state if st != nil { - _ = st.Trader().UpdateStatus(userID, traderID, false) + _ = st.Trader().UpdateStatus(autoStartUserID, autoStartID, false) } } - }(at, traderCfg.Name, traderCfg.ID, traderCfg.UserID) + }) logger.Infof("✅ Trader '%s' auto-started successfully", traderCfg.Name) } diff --git a/safe/go.go b/safe/go.go new file mode 100644 index 00000000..8084e55b --- /dev/null +++ b/safe/go.go @@ -0,0 +1,59 @@ +// Package safe provides panic-recovery wrappers for goroutines. +// A panic in any bare goroutine tears down the entire process. +// Use safe.Go instead of `go func()` in long-running or critical paths. +package safe + +import ( + "fmt" + "nofx/logger" + "runtime/debug" +) + +// Go launches fn in a new goroutine with automatic panic recovery. +// If fn panics, the panic is logged (with stack trace) but the process +// continues running. An optional onPanic callback receives the recovered value. +func Go(fn func(), onPanic ...func(recovered interface{})) { + go func() { + defer func() { + if r := recover(); r != nil { + stack := string(debug.Stack()) + logger.Errorf("🔥 goroutine panic recovered: %v\n%s", r, stack) + + for _, cb := range onPanic { + func() { + defer func() { + if r2 := recover(); r2 != nil { + logger.Errorf("🔥 onPanic callback itself panicked: %v", r2) + } + }() + cb(r) + }() + } + } + }() + fn() + }() +} + +// GoNamed is like Go but tags the log line with a human-readable name. +func GoNamed(name string, fn func(), onPanic ...func(recovered interface{})) { + Go(func() { + fn() + }, append([]func(interface{}){ + func(r interface{}) { + logger.Errorf("🔥 [%s] goroutine panicked: %v", name, r) + }, + }, onPanic...)...) +} + +// Must converts a panic into an error. Useful inside goroutines where you +// want to handle panics as errors in the caller's recovery flow. +func Must(fn func()) (err error) { + defer func() { + if r := recover(); r != nil { + err = fmt.Errorf("panic: %v\n%s", r, debug.Stack()) + } + }() + fn() + return nil +}