mirror of
https://github.com/NoFxAiOS/nofx.git
synced 2026-07-07 04:50:57 +08:00
security+reliability: remove public decrypt endpoint, add panic recovery for goroutines
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.
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
|
||||
59
safe/go.go
Normal file
59
safe/go.go
Normal file
@@ -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
|
||||
}
|
||||
Reference in New Issue
Block a user