From 5d6ec35bb43df049e69eb0496342809abf635892 Mon Sep 17 00:00:00 2001 From: shinchan-zhai Date: Wed, 22 Apr 2026 00:01:46 +0800 Subject: [PATCH] fix(agent): address critical issues from PR #1485 review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Prevent double-close panic on Stop() by using sync.Once in Scheduler, Brain, and Sentinel; remove duplicate Stop() call in main.go 2. Add trade quantity (100k) and leverage (125x) sanity caps to prevent LLM hallucinations or input errors from reaching the exchange 3. Mask secrets in onboarding setup state — only store "****" markers in SystemConfig instead of plaintext API keys/secrets Co-Authored-By: Claude Opus 4.6 --- agent/brain.go | 3 ++- agent/onboard.go | 19 +++++++++++++++---- agent/scheduler.go | 10 ++++++---- agent/sentinel.go | 3 ++- agent/trade.go | 14 ++++++++++++++ main.go | 3 +-- 6 files changed, 40 insertions(+), 12 deletions(-) diff --git a/agent/brain.go b/agent/brain.go index c3c267c9..03b16d37 100644 --- a/agent/brain.go +++ b/agent/brain.go @@ -17,6 +17,7 @@ type Brain struct { logger *slog.Logger http *http.Client stopCh chan struct{} + stopOnce sync.Once recentSignals sync.Map // debounce } @@ -29,7 +30,7 @@ func NewBrain(agent *Agent, logger *slog.Logger) *Brain { } } -func (b *Brain) Stop() { close(b.stopCh) } +func (b *Brain) Stop() { b.stopOnce.Do(func() { close(b.stopCh) }) } // cleanStaleSignals removes debounce entries older than 30 minutes. func (b *Brain) cleanStaleSignals() { diff --git a/agent/onboard.go b/agent/onboard.go index aa7fe436..17b6d597 100644 --- a/agent/onboard.go +++ b/agent/onboard.go @@ -64,13 +64,24 @@ func (a *Agent) saveSetupState(userID int64, s *SetupState) { a.store.SetSystemConfig(fmt.Sprintf("setup_step_%d", userID), s.Step) setConfig(a.store, userID, "exchange", s.Exchange) setConfig(a.store, userID, "exchange_id", s.ExchangeID) - setConfig(a.store, userID, "api_key", s.APIKey) - setConfig(a.store, userID, "api_secret", s.APISecret) - setConfig(a.store, userID, "passphrase", s.Passphrase) + // Store only a masked marker for secrets — full values stay in memory only. + // This prevents plaintext credentials from lingering in the config store + // if the setup flow is interrupted before clearSetupState runs. + if s.APIKey != "" { + setConfig(a.store, userID, "api_key", "****") + } + if s.APISecret != "" { + setConfig(a.store, userID, "api_secret", "****") + } + if s.Passphrase != "" { + setConfig(a.store, userID, "passphrase", "****") + } setConfig(a.store, userID, "ai_provider", s.AIProvider) setConfig(a.store, userID, "ai_model", s.AIModel) setConfig(a.store, userID, "ai_model_id", s.AIModelID) - setConfig(a.store, userID, "ai_key", s.AIKey) + if s.AIKey != "" { + setConfig(a.store, userID, "ai_key", "****") + } setConfig(a.store, userID, "ai_base_url", s.AIBaseURL) } diff --git a/agent/scheduler.go b/agent/scheduler.go index 41021c7a..a96da16b 100644 --- a/agent/scheduler.go +++ b/agent/scheduler.go @@ -6,13 +6,15 @@ import ( "log/slog" "nofx/safe" "strings" + "sync" "time" ) type Scheduler struct { - agent *Agent - logger *slog.Logger - stopCh chan struct{} + agent *Agent + logger *slog.Logger + stopCh chan struct{} + stopOnce sync.Once } func NewScheduler(a *Agent, l *slog.Logger) *Scheduler { @@ -51,7 +53,7 @@ func (s *Scheduler) Start(ctx context.Context) { }) } -func (s *Scheduler) Stop() { close(s.stopCh) } +func (s *Scheduler) Stop() { s.stopOnce.Do(func() { close(s.stopCh) }) } func (s *Scheduler) dailyReport() { if s.agent.traderManager == nil { return } diff --git a/agent/sentinel.go b/agent/sentinel.go index 3c5f0f22..a8c4a3ed 100644 --- a/agent/sentinel.go +++ b/agent/sentinel.go @@ -41,6 +41,7 @@ type Sentinel struct { http *http.Client logger *slog.Logger stopCh chan struct{} + stopOnce sync.Once } type pricePt struct { @@ -76,7 +77,7 @@ func (s *Sentinel) Start() { }) } -func (s *Sentinel) Stop() { close(s.stopCh) } +func (s *Sentinel) Stop() { s.stopOnce.Do(func() { close(s.stopCh) }) } func (s *Sentinel) SymbolCount() int { s.mu.RLock(); defer s.mu.RUnlock(); return len(s.symbols) } func (s *Sentinel) AddSymbol(sym string) { s.mu.Lock(); defer s.mu.Unlock(); for _, x := range s.symbols { if x == sym { return } }; s.symbols = append(s.symbols, sym) } func (s *Sentinel) RemoveSymbol(sym string) { s.mu.Lock(); defer s.mu.Unlock(); for i, x := range s.symbols { if x == sym { s.symbols = append(s.symbols[:i], s.symbols[i+1:]...); return } } } diff --git a/agent/trade.go b/agent/trade.go index 6a987a7d..14e08a79 100644 --- a/agent/trade.go +++ b/agent/trade.go @@ -194,17 +194,31 @@ func (a *Agent) executeTrade(ctx context.Context, trade *TradeAction) error { return fmt.Errorf("no running trader supports trade execution") } + // Sanity caps to prevent LLM hallucinations or input errors from causing damage. + const maxQuantity = 100000.0 + const maxLeverage = 125 + + if trade.Leverage > maxLeverage { + return fmt.Errorf("leverage %dx exceeds maximum allowed (%dx)", trade.Leverage, maxLeverage) + } + switch trade.Action { case "open_long": if trade.Quantity <= 0 { return fmt.Errorf("quantity must be > 0") } + if trade.Quantity > maxQuantity { + return fmt.Errorf("quantity %.4f exceeds maximum allowed (%.0f)", trade.Quantity, maxQuantity) + } _, err := underlyingTrader.OpenLong(trade.Symbol, trade.Quantity, trade.Leverage) return err case "open_short": if trade.Quantity <= 0 { return fmt.Errorf("quantity must be > 0") } + if trade.Quantity > maxQuantity { + return fmt.Errorf("quantity %.4f exceeds maximum allowed (%.0f)", trade.Quantity, maxQuantity) + } _, err := underlyingTrader.OpenShort(trade.Symbol, trade.Quantity, trade.Leverage) return err case "close_long": diff --git a/main.go b/main.go index dfd74c88..b99ead0d 100644 --- a/main.go +++ b/main.go @@ -169,8 +169,7 @@ func main() { } logger.Info("✅ HTTP server stopped") - nofxiAgent.Stop() - logger.Info("✅ NOFXi agent stopped") + // nofxiAgent.Stop() is handled by defer above // Stop all traders traderManager.StopAll()