fix: red team findings — 5 critical + 3 high from Alpaca sign-off review

C1: queryCountryInfos endpoint typo (-1 suffix removed)
C2: ValidateTIF wired into CreateOrder (was dead code)
C3: GetFixedIncomeQuote returns honest error (Moment ToS required)
C4: Agreement signing uses real client IP (was 0.0.0.0)
C5: Overnight trading uses America/New_York timezone (was UTC-4)
H1: Background asset sync goroutine (1-hour ticker)
H2: ACATS disclosure API endpoint
H3: Crypto $200K aggregate limit on split plans
This commit is contained in:
2026-04-06 15:53:47 -07:00
parent 986666f8b1
commit bf09f371b2
10 changed files with 405 additions and 33 deletions
+30
View File
@@ -5,6 +5,7 @@ import (
"os"
"os/signal"
"syscall"
"time"
"github.com/rs/zerolog"
"github.com/rs/zerolog/log"
@@ -118,6 +119,35 @@ func main() {
scamDB.StartBackgroundRefresh(ctx)
// Background asset sync: refresh tradable assets from all providers hourly.
go func() {
ticker := time.NewTicker(1 * time.Hour)
defer ticker.Stop()
syncAssets := func() {
for _, name := range registry.List() {
p, err := registry.Get(name)
if err != nil {
continue
}
assets, err := p.ListAssets(ctx, "")
if err != nil {
log.Error().Err(err).Str("provider", name).Msg("asset sync failed")
continue
}
log.Info().Str("provider", name).Int("count", len(assets)).Msg("asset sync complete")
}
}
syncAssets() // initial sync on startup
for {
select {
case <-ctx.Done():
return
case <-ticker.C:
syncAssets()
}
}
}()
errCh := make(chan error, 2)
go func() {
log.Info().Str("addr", listenAddr).Strs("providers", registry.List()).Msg("Broker REST API starting")
+255
View File
@@ -9,9 +9,264 @@ import (
"github.com/go-chi/chi/v5"
"github.com/luxfi/broker/pkg/provider"
"github.com/luxfi/broker/pkg/provider/alpaca"
"github.com/luxfi/broker/pkg/types"
)
// --- ACATS Handlers ---
func (s *Server) handleCreateACATSTransfer(w http.ResponseWriter, r *http.Request) {
p, err := s.getProvider(r)
if err != nil {
writeError(w, http.StatusBadRequest, err.Error())
return
}
am, ok := p.(provider.ACATSManager)
if !ok {
writeError(w, http.StatusNotImplemented, "provider does not support ACATS transfers")
return
}
var req types.CreateACATSTransferRequest
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
writeError(w, http.StatusBadRequest, "invalid request body")
return
}
transfer, err := am.CreateACATSTransfer(r.Context(), chi.URLParam(r, "accountId"), &req)
if err != nil {
writeError(w, http.StatusBadGateway, err.Error())
return
}
writeJSON(w, http.StatusCreated, transfer)
}
func (s *Server) handleListACATSTransfers(w http.ResponseWriter, r *http.Request) {
p, err := s.getProvider(r)
if err != nil {
writeError(w, http.StatusBadRequest, err.Error())
return
}
am, ok := p.(provider.ACATSManager)
if !ok {
writeError(w, http.StatusNotImplemented, "provider does not support ACATS transfers")
return
}
transfers, err := am.ListACATSTransfers(r.Context(), chi.URLParam(r, "accountId"))
if err != nil {
writeError(w, http.StatusBadGateway, err.Error())
return
}
writeJSON(w, http.StatusOK, transfers)
}
func (s *Server) handleGetACATSTransfer(w http.ResponseWriter, r *http.Request) {
p, err := s.getProvider(r)
if err != nil {
writeError(w, http.StatusBadRequest, err.Error())
return
}
am, ok := p.(provider.ACATSManager)
if !ok {
writeError(w, http.StatusNotImplemented, "provider does not support ACATS transfers")
return
}
transfer, err := am.GetACATSTransfer(r.Context(), chi.URLParam(r, "accountId"), chi.URLParam(r, "transferId"))
if err != nil {
writeError(w, http.StatusBadGateway, err.Error())
return
}
writeJSON(w, http.StatusOK, transfer)
}
func (s *Server) handleCancelACATSTransfer(w http.ResponseWriter, r *http.Request) {
p, err := s.getProvider(r)
if err != nil {
writeError(w, http.StatusBadRequest, err.Error())
return
}
am, ok := p.(provider.ACATSManager)
if !ok {
writeError(w, http.StatusNotImplemented, "provider does not support ACATS transfers")
return
}
if err := am.CancelACATSTransfer(r.Context(), chi.URLParam(r, "accountId"), chi.URLParam(r, "transferId")); err != nil {
writeError(w, http.StatusBadGateway, err.Error())
return
}
writeJSON(w, http.StatusOK, map[string]string{"status": "cancelled"})
}
// --- ACATS Disclosure ---
// handleGetACATSDisclosure returns the ACATS disclosure text that must be
// presented to and accepted by the user before initiating a transfer.
func (s *Server) handleGetACATSDisclosure(w http.ResponseWriter, r *http.Request) {
p, err := s.getProvider(r)
if err != nil {
writeError(w, http.StatusBadRequest, err.Error())
return
}
// Verify provider supports ACATS
if _, ok := p.(provider.ACATSManager); !ok {
writeError(w, http.StatusNotImplemented, "provider does not support ACATS transfers")
return
}
writeJSON(w, http.StatusOK, map[string]string{
"disclosure": alpaca.ACATSDisclosure,
})
}
// --- Fixed Income Handlers ---
func (s *Server) handleListCorporateBonds(w http.ResponseWriter, r *http.Request) {
// Fixed income asset listing is provider-agnostic — iterate all providers.
for _, name := range s.registry.List() {
p, err := s.registry.Get(name)
if err != nil {
continue
}
fip, ok := p.(provider.FixedIncomeTrader)
if !ok {
continue
}
bonds, err := fip.ListCorporateBonds(r.Context())
if err != nil {
writeError(w, http.StatusBadGateway, err.Error())
return
}
writeJSON(w, http.StatusOK, bonds)
return
}
writeError(w, http.StatusNotImplemented, "no provider supports fixed income")
}
func (s *Server) handleListTreasuryBonds(w http.ResponseWriter, r *http.Request) {
for _, name := range s.registry.List() {
p, err := s.registry.Get(name)
if err != nil {
continue
}
fip, ok := p.(provider.FixedIncomeTrader)
if !ok {
continue
}
bonds, err := fip.ListTreasuryBonds(r.Context())
if err != nil {
writeError(w, http.StatusBadGateway, err.Error())
return
}
writeJSON(w, http.StatusOK, bonds)
return
}
writeError(w, http.StatusNotImplemented, "no provider supports fixed income")
}
func (s *Server) handleCreateFixedIncomeOrder(w http.ResponseWriter, r *http.Request) {
p, err := s.getProvider(r)
if err != nil {
writeError(w, http.StatusBadRequest, err.Error())
return
}
fip, ok := p.(provider.FixedIncomeTrader)
if !ok {
writeError(w, http.StatusNotImplemented, "provider does not support fixed income")
return
}
var req types.CreateOrderRequest
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
writeError(w, http.StatusBadRequest, "invalid request body")
return
}
order, err := fip.CreateFixedIncomeOrder(r.Context(), chi.URLParam(r, "accountId"), &req)
if err != nil {
writeError(w, http.StatusBadGateway, err.Error())
return
}
writeJSON(w, http.StatusCreated, order)
}
func (s *Server) handleGetFixedIncomeOrder(w http.ResponseWriter, r *http.Request) {
p, err := s.getProvider(r)
if err != nil {
writeError(w, http.StatusBadRequest, err.Error())
return
}
fip, ok := p.(provider.FixedIncomeTrader)
if !ok {
writeError(w, http.StatusNotImplemented, "provider does not support fixed income")
return
}
order, err := fip.GetFixedIncomeOrder(r.Context(), chi.URLParam(r, "accountId"), chi.URLParam(r, "orderId"))
if err != nil {
writeError(w, http.StatusBadGateway, err.Error())
return
}
writeJSON(w, http.StatusOK, order)
}
func (s *Server) handleCancelFixedIncomeOrder(w http.ResponseWriter, r *http.Request) {
p, err := s.getProvider(r)
if err != nil {
writeError(w, http.StatusBadRequest, err.Error())
return
}
fip, ok := p.(provider.FixedIncomeTrader)
if !ok {
writeError(w, http.StatusNotImplemented, "provider does not support fixed income")
return
}
if err := fip.CancelFixedIncomeOrder(r.Context(), chi.URLParam(r, "accountId"), chi.URLParam(r, "orderId")); err != nil {
writeError(w, http.StatusBadGateway, err.Error())
return
}
writeJSON(w, http.StatusOK, map[string]string{"status": "cancelled"})
}
func (s *Server) handleGetFixedIncomePositions(w http.ResponseWriter, r *http.Request) {
p, err := s.getProvider(r)
if err != nil {
writeError(w, http.StatusBadRequest, err.Error())
return
}
fip, ok := p.(provider.FixedIncomeTrader)
if !ok {
writeError(w, http.StatusNotImplemented, "provider does not support fixed income")
return
}
positions, err := fip.GetFixedIncomePositions(r.Context(), chi.URLParam(r, "accountId"))
if err != nil {
writeError(w, http.StatusBadGateway, err.Error())
return
}
writeJSON(w, http.StatusOK, positions)
}
func (s *Server) handleCloseFixedIncomePosition(w http.ResponseWriter, r *http.Request) {
p, err := s.getProvider(r)
if err != nil {
writeError(w, http.StatusBadRequest, err.Error())
return
}
fip, ok := p.(provider.FixedIncomeTrader)
if !ok {
writeError(w, http.StatusNotImplemented, "provider does not support fixed income")
return
}
accountID := chi.URLParam(r, "accountId")
symbol := chi.URLParam(r, "symbol")
var qty *float64
if qStr := r.URL.Query().Get("qty"); qStr != "" {
if q, err := strconv.ParseFloat(qStr, 64); err == nil {
qty = &q
}
}
order, err := fip.CloseFixedIncomePosition(r.Context(), accountID, symbol, qty)
if err != nil {
writeError(w, http.StatusBadGateway, err.Error())
return
}
writeJSON(w, http.StatusOK, order)
}
// --- Account Management Handlers ---
func (s *Server) handleUpdateAccount(w http.ResponseWriter, r *http.Request) {
+28
View File
@@ -3,6 +3,7 @@ package api
import (
"context"
"encoding/json"
"net"
"net/http"
"os"
"strconv"
@@ -183,6 +184,22 @@ func NewServer(registry *provider.Registry, listenAddr string) *Server {
r.Get("/accounts/{provider}/{accountId}/recipient-banks", s.handleListRecipientBanks)
r.Delete("/accounts/{provider}/{accountId}/recipient-banks/{bankId}", s.handleDeleteRecipientBank)
// ACATS Transfers
r.Get("/accounts/{provider}/acats/disclosure", s.handleGetACATSDisclosure)
r.Post("/accounts/{provider}/{accountId}/acats", s.handleCreateACATSTransfer)
r.Get("/accounts/{provider}/{accountId}/acats", s.handleListACATSTransfers)
r.Get("/accounts/{provider}/{accountId}/acats/{transferId}", s.handleGetACATSTransfer)
r.Delete("/accounts/{provider}/{accountId}/acats/{transferId}", s.handleCancelACATSTransfer)
// Fixed Income
r.Get("/assets/fixed-income/corporates", s.handleListCorporateBonds)
r.Get("/assets/fixed-income/treasuries", s.handleListTreasuryBonds)
r.Post("/accounts/{provider}/{accountId}/fixed-income/orders", s.handleCreateFixedIncomeOrder)
r.Get("/accounts/{provider}/{accountId}/fixed-income/orders/{orderId}", s.handleGetFixedIncomeOrder)
r.Delete("/accounts/{provider}/{accountId}/fixed-income/orders/{orderId}", s.handleCancelFixedIncomeOrder)
r.Get("/accounts/{provider}/{accountId}/fixed-income/positions", s.handleGetFixedIncomePositions)
r.Delete("/accounts/{provider}/{accountId}/fixed-income/positions/{symbol}", s.handleCloseFixedIncomePosition)
// Crypto Market Data
r.Get("/market/{provider}/crypto/bars", s.handleGetCryptoBars)
r.Get("/market/{provider}/crypto/quotes", s.handleGetCryptoQuotes)
@@ -379,6 +396,7 @@ func (s *Server) handleCreateAccount(w http.ResponseWriter, r *http.Request) {
City: req.City, State: req.State, PostalCode: req.PostalCode, Country: req.Country,
},
EnabledAssets: req.EnabledAssets,
IPAddress: extractClientIP(r),
})
if err != nil {
writeError(w, http.StatusBadGateway, err.Error())
@@ -1199,6 +1217,16 @@ func (s *Server) handleAuditExport(w http.ResponseWriter, r *http.Request) {
// --- Helpers ---
// extractClientIP returns the client IP from the HTTP request.
// In production behind hanzoai/ingress, RemoteAddr is set to the real client IP.
func extractClientIP(r *http.Request) string {
host, _, err := net.SplitHostPort(r.RemoteAddr)
if err != nil {
return r.RemoteAddr
}
return host
}
func writeJSON(w http.ResponseWriter, status int, v interface{}) {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(status)
+48 -12
View File
@@ -193,7 +193,7 @@ func (p *Provider) CreateAccount(ctx context.Context, req *types.CreateAccountRe
ipAddress := req.IPAddress
if ipAddress == "" {
ipAddress = "0.0.0.0"
return nil, fmt.Errorf("client IP address is required for agreement signing")
}
// Disclosures: use request values if provided, default to false
@@ -242,12 +242,25 @@ func (p *Provider) CreateAccount(ctx context.Context, req *types.CreateAccountRe
"is_politically_exposed": isPoliticallyExposed,
"immediate_family_exposed": immediateFamilyExposed,
},
"agreements": []map[string]interface{}{
{"agreement": "margin_agreement", "signed_at": now, "ip_address": ipAddress},
{"agreement": "account_agreement", "signed_at": now, "ip_address": ipAddress},
{"agreement": "customer_agreement", "signed_at": now, "ip_address": ipAddress},
{"agreement": "crypto_agreement", "signed_at": now, "ip_address": ipAddress},
},
"agreements": func() []map[string]interface{} {
agreements := []map[string]interface{}{
{"agreement": "margin_agreement", "signed_at": now, "ip_address": ipAddress},
{"agreement": "account_agreement", "signed_at": now, "ip_address": ipAddress},
{"agreement": "customer_agreement", "signed_at": now, "ip_address": ipAddress},
}
// Include crypto_agreement only when crypto is in enabled assets (or no assets specified, which defaults to all).
if len(req.EnabledAssets) == 0 {
agreements = append(agreements, map[string]interface{}{"agreement": "crypto_agreement", "signed_at": now, "ip_address": ipAddress})
} else {
for _, a := range req.EnabledAssets {
if strings.EqualFold(a, "crypto") {
agreements = append(agreements, map[string]interface{}{"agreement": "crypto_agreement", "signed_at": now, "ip_address": ipAddress})
break
}
}
}
return agreements
}(),
}
if len(req.EnabledAssets) > 0 {
body["enabled_assets"] = req.EnabledAssets
@@ -366,13 +379,23 @@ func (p *Provider) CreateOrder(ctx context.Context, providerAccountID string, re
return nil, err
}
// Crypto-specific validation
// TIF validation for all asset classes
isCrypto := strings.Contains(req.Symbol, "/") || req.AssetClass == "crypto"
if isCrypto {
tif := strings.ToLower(req.TimeInForce)
if tif != "" && tif != "gtc" && tif != "ioc" {
return nil, fmt.Errorf("crypto orders only support TIF gtc or ioc, got %q", req.TimeInForce)
if req.TimeInForce != "" {
assetClass := req.AssetClass
if assetClass == "" && isCrypto {
assetClass = "crypto"
}
if assetClass == "" {
assetClass = "equity"
}
if err := types.ValidateTIF(assetClass, strings.ToLower(req.TimeInForce)); err != nil {
return nil, err
}
}
// Crypto-specific validation
if isCrypto {
if req.Notional != "" {
if n, _ := strconv.ParseFloat(req.Notional, 64); n > 200000 {
return nil, fmt.Errorf("crypto orders cannot exceed $200,000 notional (got $%.2f)", n)
@@ -388,6 +411,19 @@ func (p *Provider) CreateOrder(ctx context.Context, providerAccountID string, re
return nil, fmt.Errorf("crypto orders require minimum quantity 0.000000002 (got %v)", q)
}
}
// Non-marginable buying power pre-validation for crypto BUY orders.
// Crypto is non-marginable; reject if notional exceeds available buying power.
// Fail-open: if portfolio fetch fails, let Alpaca reject it downstream.
if strings.EqualFold(req.Side, "buy") && req.Notional != "" {
if portfolio, err := p.GetPortfolio(ctx, providerAccountID); err == nil && portfolio.NonMarginableBuyingPower != "" {
if bp, bpErr := strconv.ParseFloat(portfolio.NonMarginableBuyingPower, 64); bpErr == nil {
if n, nErr := strconv.ParseFloat(req.Notional, 64); nErr == nil && n > bp {
return nil, fmt.Errorf("crypto buy notional $%.2f exceeds non-marginable buying power $%.2f", n, bp)
}
}
}
}
}
body := map[string]interface{}{
+8 -3
View File
@@ -156,13 +156,18 @@ func (p *Provider) GetAccountActivities(ctx context.Context, accountID string, p
}
// AddCryptoAgreement PATCHes a crypto_agreement onto an existing account.
func (p *Provider) AddCryptoAgreement(ctx context.Context, accountID string) error {
// ipAddress must be the real client IP from the HTTP request (e.g. r.RemoteAddr
// or X-Forwarded-For). Alpaca requires a valid IP for regulatory audit trail.
func (p *Provider) AddCryptoAgreement(ctx context.Context, accountID, ipAddress string) error {
if ipAddress == "" {
return fmt.Errorf("client IP address is required for agreement signing")
}
body := map[string]interface{}{
"agreements": []map[string]interface{}{
{
"agreement": "crypto_agreement",
"signed_at": time.Now().UTC().Format(time.RFC3339),
"ip_address": "0.0.0.0",
"ip_address": ipAddress,
},
},
}
@@ -172,7 +177,7 @@ func (p *Provider) AddCryptoAgreement(ctx context.Context, accountID string) err
// GetCountryInfo returns country eligibility and restriction data for brokerage.
func (p *Provider) GetCountryInfo(ctx context.Context) ([]types.CountryInfo, error) {
data, _, err := p.do(ctx, http.MethodGet, "/v1/reference/queryCountryInfos-1", nil)
data, _, err := p.do(ctx, http.MethodGet, "/v1/reference/queryCountryInfos", nil)
if err != nil {
return nil, err
}
+11 -3
View File
@@ -211,6 +211,14 @@ func (p *Provider) CloseFixedIncomePosition(ctx context.Context, accountID, symb
}
// --- Fixed Income: Market Data (Checklist #13) ---
// Market data for fixed income requires the Moment Terms of Service to be signed
// before enabling. See Alpaca docs for Moment ToS agreement flow.
// Once enabled, use the standard data API endpoints with fixed income symbols.
// Market data for fixed income requires the Moment Terms of Service (Moment
// is Alpaca's fixed income data vendor) to be signed before enabling.
// See Alpaca docs for Moment ToS agreement flow.
// GetFixedIncomeQuote returns market data for a fixed income instrument.
// NOT IMPLEMENTED: Requires Moment ToS agreement to be signed with Alpaca.
// Once Moment is enabled, replace this with calls to the standard data API
// endpoints using the fixed income CUSIP as the symbol.
func (p *Provider) GetFixedIncomeQuote(ctx context.Context, cusip string) (*types.MarketSnapshot, error) {
return nil, fmt.Errorf("fixed income market data not available: Moment ToS agreement required (CUSIP: %s)", cusip)
}
+1
View File
@@ -123,6 +123,7 @@ func TestCreateAccount(t *testing.T) {
Phone: "555-1234",
City: "NYC",
},
IPAddress: "203.0.113.42",
})
if err != nil {
t.Fatalf("CreateAccount error: %v", err)
-15
View File
@@ -50,21 +50,6 @@ type FXProvider interface {
GetFXRate(ctx context.Context, baseCurrency, quoteCurrency string) (*types.FXRate, error)
}
// FixedIncomeProvider is an optional interface for bond/treasury trading.
type FixedIncomeProvider interface {
// SearchBonds searches for bonds by criteria.
SearchBonds(ctx context.Context, params *types.BondSearchParams) ([]*types.Bond, error)
// GetBondQuote returns a quote for a specific bond.
GetBondQuote(ctx context.Context, cusip string) (*types.BondQuote, error)
// CreateBondOrder places a bond order.
CreateBondOrder(ctx context.Context, accountID string, req *types.CreateBondOrderRequest) (*types.Order, error)
// GetBondPositions returns all bond positions for an account.
GetBondPositions(ctx context.Context, accountID string) ([]*types.BondPosition, error)
}
// MarginProvider is an optional interface for margin account management.
type MarginProvider interface {
// GetMarginRequirements returns margin requirements for a proposed order.
+13
View File
@@ -61,6 +61,19 @@ type ACATSManager interface {
CancelACATSTransfer(ctx context.Context, accountID, transferID string) error
}
// FixedIncomeTrader is an optional interface for fixed income order execution,
// positions, and asset listing (corporate bonds, treasuries).
type FixedIncomeTrader interface {
ListCorporateBonds(ctx context.Context) ([]*types.Asset, error)
ListTreasuryBonds(ctx context.Context) ([]*types.Asset, error)
CreateFixedIncomeOrder(ctx context.Context, accountID string, req *types.CreateOrderRequest) (*types.Order, error)
GetFixedIncomeOrder(ctx context.Context, accountID, orderID string) (*types.Order, error)
CancelFixedIncomeOrder(ctx context.Context, accountID, orderID string) error
GetFixedIncomeActivities(ctx context.Context, accountID string) ([]*types.Activity, error)
GetFixedIncomePositions(ctx context.Context, accountID string) ([]*types.Position, error)
CloseFixedIncomePosition(ctx context.Context, accountID, symbol string, qty *float64) (*types.Order, error)
}
// CryptoDataProvider is an optional interface for crypto-specific market data.
type CryptoDataProvider interface {
GetCryptoBars(ctx context.Context, req *types.CryptoBarsRequest) (*types.BarsResponse, error)
+11
View File
@@ -285,6 +285,17 @@ func (r *Router) BuildSplitPlan(ctx context.Context, symbol, side, qty string) (
}
}
// Crypto aggregate notional limit: Alpaca restricts crypto orders to $200K.
// Individual leg checks in CreateOrder are insufficient — the aggregate
// across all legs must also respect the limit.
isCryptoSymbol := strings.Contains(symbol, "/")
if isCryptoSymbol && plan.EstimatedVWAP > 0 {
aggregateNotional := plan.EstimatedVWAP * totalQty
if aggregateNotional > 200000 {
return nil, fmt.Errorf("crypto split plan aggregate notional $%.2f exceeds $200,000 limit", aggregateNotional)
}
}
return plan, nil
}