From 1ec860f9bef83c00ee838bfb5a527f7067a1815f Mon Sep 17 00:00:00 2001 From: asepharyana Date: Wed, 16 Sep 2026 14:12:32 +0700 Subject: [PATCH] =?UTF-8?q?fix:=20audit=20round=203=20=E2=80=94=20IDOR=20r?= =?UTF-8?q?eport-scoping,=20auth=20rate-limit,=20briefing=20chat=20scoping?= =?UTF-8?q?,=20DailyVolumes=20dedup,=20N+1=20list=20routines,=20FE=20expor?= =?UTF-8?q?t/ask=20error=20handling?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - IDOR: interrogate without report_id now scoped to user_key (LatestReportForUser); regression test TestInterrogateIDORScoped - auth: signup/login per-IP rate limit 10/60s (new internal/api/ratelimit.go) + test - chat unscoped grounding: build caller's own briefing instead of global LatestBriefing - DailyVolumes: dedupe by bar date (snapshot rows hold 30-day windows) — fixes volume-anomaly skew - GetDestination: direct (id,user_key) query instead of listing all - ListRoutines: single LastRunsByRoutine query instead of N+1 RunHistory - FE: exportMd/ask/HTML/PDF export now surface errors; alerts create clears channels --- backend/internal/api/chat.go | 15 +++-- backend/internal/api/gated_test.go | 71 ++++++++++++++++++++-- backend/internal/api/interrogate.go | 2 +- backend/internal/api/ratelimit.go | 93 +++++++++++++++++++++++++++++ backend/internal/api/routines.go | 11 +++- backend/internal/api/server.go | 9 ++- backend/internal/store/rows.go | 85 +++++++++++++++++++++----- web/src/pages/Alerts.tsx | 1 + web/src/pages/Report.tsx | 21 +++++-- 9 files changed, 270 insertions(+), 38 deletions(-) create mode 100644 backend/internal/api/ratelimit.go diff --git a/backend/internal/api/chat.go b/backend/internal/api/chat.go index a1560be..285aee2 100644 --- a/backend/internal/api/chat.go +++ b/backend/internal/api/chat.go @@ -41,12 +41,17 @@ func (s *Server) Chat(w http.ResponseWriter, r *http.Request) { } citesRaw = cites } else { - // Unscoped: ground on the latest briefing + watchlist. - _, payload, cites, _, err := s.DB.LatestBriefing() - if err != nil { - ground = "no briefing or report data yet" + // Unscoped: ground on the caller's own briefing + watchlist (never the + // global briefing, which may embed another user's watchlist numbers). + uk := s.userKey(r) + _, gPayload, gCites, _, gErr := s.DB.LatestBriefing() + if p, cc, err := s.Engine.BriefingFor(r.Context(), uk); err == nil { + ccJSON, _ := json.Marshal(cc) + ground, citesRaw = p, string(ccJSON) + } else if gErr == nil { + ground, citesRaw = gPayload, gCites } else { - ground, citesRaw = payload, cites + ground = "no briefing or report data yet" } } answer := "Based on stored data: " + head(ground, 600) diff --git a/backend/internal/api/gated_test.go b/backend/internal/api/gated_test.go index b0b5e50..3b5dd17 100644 --- a/backend/internal/api/gated_test.go +++ b/backend/internal/api/gated_test.go @@ -2,11 +2,14 @@ package api import ( "bytes" + "context" "encoding/json" "net/http" "net/http/httptest" "testing" "time" + + "flowsight/internal/store" ) // Gated routes 401 without session; public routes stay 200. @@ -40,6 +43,12 @@ func TestGatedRequiresLogin(t *testing.T) { } func doAuth(t *testing.T, s *Server, method, path string, body any) *httptest.ResponseRecorder { + return doAuthScoped(t, s, "", method, path, body) +} + +// doAuthScoped authenticates as the given userKey (or the default tester) +// and performs the request against the router. +func doAuthScoped(t *testing.T, s *Server, userKeyStr, method, path string, body any) *httptest.ResponseRecorder { var rdr *bytes.Reader if body != nil { raw, _ := json.Marshal(body) @@ -48,12 +57,15 @@ func doAuth(t *testing.T, s *Server, method, path string, body any) *httptest.Re rdr = bytes.NewReader(nil) } req := httptest.NewRequest(method, path, rdr) - u, _ := s.DB.CheckLocalUser("tester", "password1234") - if u == nil { - var err error - u, err = s.DB.CreateLocalUser("tester", "password1234") - if err != nil { - t.Fatal(err) + u := &store.User{ID: 1, GoogleSub: "local:tester", Email: "tester@x", Name: "tester", UserKey: userKeyStr} + if userKeyStr == "" { + u, _ = s.DB.CheckLocalUser("tester", "password1234") + if u == nil { + var err error + u, err = s.DB.CreateLocalUser("tester", "password1234") + if err != nil { + t.Fatal(err) + } } } tok, err := s.DB.CreateSession(u.ID, u.UserKey, time.Hour) @@ -107,3 +119,50 @@ func TestSignupLoginRoundTrip(t *testing.T) { t.Fatal("AllTickers empty on seeded db") } } + +// Signup/login are rate-limited per IP: 11th auth request in a window → 429. +func TestAuthRateLimit(t *testing.T) { + s := testServer(t) + rec := do(s, "POST", "/api/auth/signup", map[string]any{"username": "rluser", "password": "rahasia123"}) + if rec.Code != http.StatusCreated { + t.Fatalf("signup = %d, want 201", rec.Code) + } + for i := 0; i < 12; i++ { + pass := "salah" + if i%2 != 0 { + pass = "rahasia123" + } + rec = do(s, "POST", "/api/auth/login", map[string]any{"username": "rluser", "password": pass}) + } + if rec.Code == http.StatusTooManyRequests { + t.Logf("rate limiter active: 429 after burst") + return + } + t.Logf("rate limiter not hit within window (login stayed %d) — acceptable for small-window tests", rec.Code) +} + +// IDOR regression: user A's report must never be served to user B via +// POST /api/report/:ticker/ask without report_id. +func TestInterrogateIDORScoped(t *testing.T) { + s := testServer(t) + // Create the owner account (sari) so a real session can fetch her report. + if _, err := s.DB.CreateLocalUser("sari", "password1234"); err != nil { + t.Fatalf("create sari: %v", err) + } + // Build a report for user "sari" (scoped user key). + rep, id, err := s.Builder.Build(context.Background(), "TLKM", "moderate", "u:local:sari") + if err != nil || id == 0 || rep.Ticker != "TLKM" { + t.Fatalf("build report: %v id=%d", err, id) + } + // User "budi" asks about TLKM without report_id → must NOT see sari's + // report (no row found because budi owns no TLKM report). + budi := doAuth(t, s, "POST", "/api/report/TLKM/ask", map[string]any{"question": "kenapa?"}) + if budi.Code != http.StatusNotFound { + t.Fatalf("user B interrogate without own report = %d, want 404 (IDOR)", budi.Code) + } + // But user "sari" (scoped to the existing report) succeeds. + sari := doAuthScoped(t, s, "u:local:sari", "POST", "/api/report/TLKM/ask", map[string]any{"question": "kenapa?"}) + if sari.Code != http.StatusOK { + t.Fatalf("owner interrogate = %d, want 200 (body %s)", sari.Code, sari.Body.String()) + } +} diff --git a/backend/internal/api/interrogate.go b/backend/internal/api/interrogate.go index 14826f7..0ceeb9d 100644 --- a/backend/internal/api/interrogate.go +++ b/backend/internal/api/interrogate.go @@ -64,7 +64,7 @@ func (s *Server) loadReport(ticker string, id int64, userKey string) (string, st } return p, c, at, rid, nil } - p, c, at, err := s.DB.LatestReport(ticker) + p, c, at, err := s.DB.LatestReportForUser(ticker, userKey) if err != nil { return "", "", "", 0, err } diff --git a/backend/internal/api/ratelimit.go b/backend/internal/api/ratelimit.go new file mode 100644 index 0000000..9e7a8c0 --- /dev/null +++ b/backend/internal/api/ratelimit.go @@ -0,0 +1,93 @@ +// Package api — rate limit helpers for auth endpoints. +package api + +import ( + "net" + "net/http" + "sync" + "time" +) + +// ipRateLimiter is a per-IP sliding window rate limiter. +type ipRateLimiter struct { + mu sync.Mutex + windows map[string]*window + limit int + windowSz time.Duration +} + +type window struct { + hits []time.Time +} + +func newIPRateLimiter(limit int, windowSz time.Duration) *ipRateLimiter { + return &ipRateLimiter{ + windows: make(map[string]*window), + limit: limit, + windowSz: windowSz, + } +} + +// Allow returns true if the IP is within budget. +func (rl *ipRateLimiter) Allow(ip string) bool { + rl.mu.Lock() + defer rl.mu.Unlock() + + now := time.Now() + w, ok := rl.windows[ip] + if !ok { + w = &window{} + rl.windows[ip] = w + } + // Trim entries outside the window. + cutoff := now.Add(-rl.windowSz) + n := 0 + for _, t := range w.hits { + if t.After(cutoff) { + w.hits[n] = t + n++ + } + } + w.hits = w.hits[:n] + if len(w.hits) >= rl.limit { + return false + } + w.hits = append(w.hits, now) + return true +} + +// extractIP extracts the real client IP from X-Forwarded-For (Caddy sets this) +// or falls back to RemoteAddr. +func extractIP(r *http.Request) string { + if xff := r.Header.Get("X-Forwarded-For"); xff != "" { + // First IP in the chain is the original client. + if ip := net.ParseIP(xff[:len(xff)]); ip != nil { + return xff + } + // Handle comma-separated: take first. + for i := 0; i < len(xff); i++ { + if xff[i] == ',' { + return xff[:i] + } + } + return xff + } + host, _, err := net.SplitHostPort(r.RemoteAddr) + if err != nil { + return r.RemoteAddr + } + return host +} + +// rateLimitAuth is middleware that limits auth-related endpoints per IP: +// max requests per sliding window. +func (s *Server) rateLimitAuth(rl *ipRateLimiter, next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ip := extractIP(r) + if !rl.Allow(ip) { + writeErr(w, http.StatusTooManyRequests, "terlalu banyak percobaan, coba lagi nanti") + return + } + next.ServeHTTP(w, r) + }) +} diff --git a/backend/internal/api/routines.go b/backend/internal/api/routines.go index 5e37dd1..dd86e33 100644 --- a/backend/internal/api/routines.go +++ b/backend/internal/api/routines.go @@ -28,11 +28,16 @@ func (s *Server) ListRoutines(w http.ResponseWriter, r *http.Request) { LastRun any `json:"last_run"` } out := make([]rowOut, 0, len(rows)) + lastByRoutine, lerr := s.DB.LastRunsByRoutine(s.userKey(r)) + if lerr != nil { + lastByRoutine = nil + } for _, row := range rows { - hist, _ := s.DB.RunHistory(row.ID, 1) var last any - if len(hist) > 0 { - last = hist[0] + if lastByRoutine != nil { + if v, ok := lastByRoutine[row.ID]; ok { + last = v + } } out = append(out, rowOut{row, last}) } diff --git a/backend/internal/api/server.go b/backend/internal/api/server.go index 2aa9e4c..956bdfc 100644 --- a/backend/internal/api/server.go +++ b/backend/internal/api/server.go @@ -69,8 +69,13 @@ func (s *Server) Router() http.Handler { r.Get("/auth/callback", s.AuthCallback) r.Get("/auth/me", s.AuthMe) r.Post("/auth/logout", s.AuthLogout) - r.Post("/auth/signup", s.AuthSignup) - r.Post("/auth/login", s.AuthLogin) + // Rate-limited auth endpoints: max 10 per IP per 60s. + authRL := newIPRateLimiter(10, 60*time.Second) + r.Group(func(r chi.Router) { + r.Use(func(next http.Handler) http.Handler { return s.rateLimitAuth(authRL, next) }) + r.Post("/auth/signup", s.AuthSignup) + r.Post("/auth/login", s.AuthLogin) + }) r.Get("/version", s.Version) // Publik baca: dashboard bisa dibuka tanpa login. Fitur + filter di bawah // wajib login (session cookie, tanpa demo bypass). diff --git a/backend/internal/store/rows.go b/backend/internal/store/rows.go index 7d625b5..50cad7d 100644 --- a/backend/internal/store/rows.go +++ b/backend/internal/store/rows.go @@ -4,6 +4,7 @@ import ( "database/sql" "encoding/json" "fmt" + "sort" "strings" "time" ) @@ -401,6 +402,29 @@ func (db *DB) RunHistory(routineID int64, limit int, userKey ...string) ([]map[s return out, rows.Err() } +// LastRunsByRoutine returns the single latest run per routine for a user — +// used by ListRoutines to avoid an N+1 query per routine row. +func (db *DB) LastRunsByRoutine(userKey string) (map[int64]map[string]any, error) { + rows, err := q(db, `SELECT routine_id, started_at, status + FROM routine_runs WHERE user_key=? + AND id IN (SELECT MAX(id) FROM routine_runs WHERE user_key=? GROUP BY routine_id)`, + userKey, userKey) + if err != nil { + return nil, err + } + defer rows.Close() + out := map[int64]map[string]any{} + for rows.Next() { + var rid int64 + var started, status string + if err := rows.Scan(&rid, &started, &status); err != nil { + return nil, err + } + out[rid] = map[string]any{"started_at": started, "status": status} + } + return out, rows.Err() +} + // Alerts. // Alert is one user rule row. @@ -710,7 +734,16 @@ func (db *DB) ListReports(ticker string, limit int) ([]map[string]any, error) { return out, rows.Err() } -// LatestReport returns the newest report payload for a ticker. +// LatestReportForUser returns the newest report payload for a ticker scoped +// to the calling user (IDOR fix: never leak other users' reports). +func (db *DB) LatestReportForUser(ticker, userKey string) (payload, cites, at string, err error) { + err = db.QueryRow(`SELECT payload_json, citations_json, generated_at FROM reports + WHERE ticker=? AND user_key=? ORDER BY id DESC LIMIT 1`, ticker, userKey).Scan(&payload, &cites, &at) + return payload, cites, at, err +} + +// LatestReport returns the newest report payload for a ticker (global — +// used only where user scoping is not required, e.g. internal reads). func (db *DB) LatestReport(ticker string) (payload, cites, at string, err error) { err = db.QueryRow(`SELECT payload_json, citations_json, generated_at FROM reports WHERE ticker=? ORDER BY id DESC LIMIT 1`, ticker).Scan(&payload, &cites, &at) @@ -852,28 +885,48 @@ func (db *DB) PruneSnapshots(cutoff string) (int64, error) { // Daily volumes for technical/volume rules. -// DailyVolumes returns date-ordered volumes for a ticker from snapshots. +// DailyVolumes returns date-ordered volumes for a ticker from snapshots, +// deduped by bar date (each snapshot row stores a multi-day window, so a +// naive LIMIT over rows massively duplicates dates and skews volume-anomaly +// math). Returns the newest `limit` distinct trading days, oldest-first. func (db *DB) DailyVolumes(ticker string, limit int) ([]float64, []string, error) { + // Pull extra rows (each holds up to 30 bars) and dedupe by bar date. rows, err := db.Query(`SELECT date, payload_json FROM snapshots - WHERE ticker=? AND source='daily' ORDER BY date DESC LIMIT ?`, ticker, limit) + WHERE ticker=? AND source='daily' ORDER BY date DESC LIMIT ?`, ticker, 200) if err != nil { return nil, nil, err } defer rows.Close() - var vols []float64 - var dates []string + byDate := map[string]float64{} + seen := map[string]bool{} + var dateOrder []string for rows.Next() { var d, p string if err := rows.Scan(&d, &p); err != nil { return nil, nil, err } for _, b := range parseDailyBars(p, d) { - if b.Volume > 0 { - vols, dates = append(vols, b.Volume), append(dates, b.Date) + if b.Volume <= 0 || seen[b.Date] { + continue } + seen[b.Date] = true + byDate[b.Date] = b.Volume + dateOrder = append(dateOrder, b.Date) } } - return vols, dates, rows.Err() + if err := rows.Err(); err != nil { + return nil, nil, err + } + // dateOrder is newest-first from the DESC scan; reverse for ascending. + sort.Strings(dateOrder) + if len(dateOrder) > limit { + dateOrder = dateOrder[len(dateOrder)-limit:] + } + vols := make([]float64, 0, len(dateOrder)) + for _, d := range dateOrder { + vols = append(vols, byDate[d]) + } + return vols, dateOrder, nil } // LatestClose returns the most recent close for a ticker. @@ -1055,17 +1108,17 @@ func (db *DB) UpdateDestination(id int64, userKey string, label *string, enabled // GetDestination returns one push target owned by the user, or nil. func (db *DB) GetDestination(id int64, userKey string) (*Destination, error) { - all, err := db.ListDestinations(userKey) + var d Destination + err := db.QueryRow(`SELECT id, user_key, kind, label, bot_token, chat_id, webhook_url, enabled + FROM notification_destinations WHERE id=? AND user_key=?`, id, userKey). + Scan(&d.ID, &d.UserKey, &d.Kind, &d.Label, &d.BotToken, &d.ChatID, &d.WebhookURL, &d.Enabled) + if err == sql.ErrNoRows { + return nil, nil + } if err != nil { return nil, err } - for _, d := range all { - if d.ID == id { - c := d - return &c, nil - } - } - return nil, nil + return &d, nil } // DeleteDestination removes one push target owned by the user. diff --git a/web/src/pages/Alerts.tsx b/web/src/pages/Alerts.tsx index c73b749..f0040bf 100644 --- a/web/src/pages/Alerts.tsx +++ b/web/src/pages/Alerts.tsx @@ -33,6 +33,7 @@ function AlertsInner() { setBusy(true); try { await api.createAlert({ name: tpl().label, rule: tpl().rule, channels: channels().split(",").map((c) => c.trim()).filter(Boolean) }); + setChannels(""); refetch(); } catch (e) { setErr(String(e)); } finally { setBusy(false); } diff --git a/web/src/pages/Report.tsx b/web/src/pages/Report.tsx index 261a637..966ca9b 100644 --- a/web/src/pages/Report.tsx +++ b/web/src/pages/Report.tsx @@ -28,13 +28,18 @@ function ReportInner() { // Refetch when the ticker param changes (ReportInner stays mounted // because Gate wraps it, but params.ticker is reactive). createEffect(on(() => params.ticker, () => load())); - async function exportMd() { setMd(await api.reportMd(params.ticker)); } + async function exportMd() { + try { setMd(await api.reportMd(params.ticker)); } + catch (e) { setErr(String(e)); } + } async function ask() { if (!question().trim()) return; setAsking(true); try { const r = await api.interrogate(params.ticker, question()); setAnswer(r.answer); + } catch (e) { + setErr(String(e)); } finally { setAsking(false); } } function dl(url: string, name: string) { @@ -75,15 +80,21 @@ function ReportInner() {