fix(security): phase 4 code quality and low-severity fixes
- Fix limitStr to handle multi-byte UTF-8 characters correctly - Sanitize log messages: strip ANSI escape sequences and newlines - URL-encode probe node_id instead of string concatenation - Fix follower resp.Body leak on non-200 responses - Make SSH host key path configurable via UPTOP_SSH_HOST_KEY env var - Add HTTP method checks on GET-only endpoints (405 for wrong methods) - Extract magic numbers into named constants across monitor/store/server - Standardize error output to stderr for all startup errors
This commit is contained in:
+7
-7
@@ -323,7 +323,7 @@ func runServe(args []string) {
|
||||
fmt.Printf("Using SQLite: %s\n", *flagDSN)
|
||||
}
|
||||
if dbErr != nil {
|
||||
fmt.Printf("Database connection error: %v\n", dbErr)
|
||||
fmt.Fprintf(os.Stderr, "database connection error: %v\n", dbErr)
|
||||
os.Exit(1)
|
||||
}
|
||||
defer ss.Close()
|
||||
@@ -341,7 +341,7 @@ func runServe(args []string) {
|
||||
|
||||
var s store.Store = ss
|
||||
if err := s.Init(); err != nil {
|
||||
fmt.Printf("Database init error: %v\n", err)
|
||||
fmt.Fprintf(os.Stderr, "database init error: %v\n", err)
|
||||
os.Exit(1)
|
||||
}
|
||||
if *demo {
|
||||
@@ -351,12 +351,12 @@ func runServe(args []string) {
|
||||
if *importKuma != "" {
|
||||
kb, err := importer.LoadKumaFile(*importKuma)
|
||||
if err != nil {
|
||||
fmt.Printf("Kuma import error: %v\n", err)
|
||||
fmt.Fprintf(os.Stderr, "kuma import error: %v\n", err)
|
||||
os.Exit(1)
|
||||
}
|
||||
backup := importer.ConvertKuma(kb)
|
||||
if err := s.ImportData(backup); err != nil {
|
||||
fmt.Printf("Import failed: %v\n", err)
|
||||
fmt.Fprintf(os.Stderr, "import failed: %v\n", err)
|
||||
os.Exit(1)
|
||||
}
|
||||
fmt.Printf("Imported %d monitors and %d alerts from Uptime Kuma v%s\n", len(backup.Sites), len(backup.Alerts), kb.Version)
|
||||
@@ -409,7 +409,7 @@ func runServe(args []string) {
|
||||
if isatty.IsTerminal(os.Stdout.Fd()) || isatty.IsCygwinTerminal(os.Stdout.Fd()) {
|
||||
p := tea.NewProgram(tui.InitialModel(true, s, eng), tea.WithAltScreen(), tea.WithMouseCellMotion())
|
||||
if _, err := p.Run(); err != nil {
|
||||
fmt.Printf("Error: %v\n", err)
|
||||
fmt.Fprintf(os.Stderr, "error: %v\n", err)
|
||||
}
|
||||
} else {
|
||||
fmt.Println("uptop running in HEADLESS mode")
|
||||
@@ -437,7 +437,7 @@ func runServe(args []string) {
|
||||
func startSSHServer(port int, db store.Store, eng *monitor.Engine, kc *keyCache) *ssh.Server {
|
||||
s, err := wish.NewServer(
|
||||
wish.WithAddress(fmt.Sprintf(":%d", port)),
|
||||
wish.WithHostKeyPath(".ssh/id_ed25519"),
|
||||
wish.WithHostKeyPath(envOrDefault("UPTOP_SSH_HOST_KEY", ".ssh/id_ed25519")),
|
||||
wish.WithPublicKeyAuth(func(ctx ssh.Context, key ssh.PublicKey) bool {
|
||||
return kc.IsAllowed(key)
|
||||
}),
|
||||
@@ -448,7 +448,7 @@ func startSSHServer(port int, db store.Store, eng *monitor.Engine, kc *keyCache)
|
||||
),
|
||||
)
|
||||
if err != nil {
|
||||
fmt.Printf("SSH server error: %v\n", err)
|
||||
fmt.Fprintf(os.Stderr, "SSH server error: %v\n", err)
|
||||
return nil
|
||||
}
|
||||
go func() {
|
||||
|
||||
@@ -3,10 +3,11 @@ package cluster
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"gitea.lerkolabs.com/lerko/uptop/internal/monitor"
|
||||
"net/http"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"gitea.lerkolabs.com/lerko/uptop/internal/monitor"
|
||||
)
|
||||
|
||||
type Config struct {
|
||||
@@ -57,8 +58,8 @@ func runFollowerLoop(ctx context.Context, cfg Config, eng *monitor.Engine) {
|
||||
resp, err := client.Do(req)
|
||||
isLeaderHealthy := false
|
||||
|
||||
if err == nil && resp.StatusCode == 200 {
|
||||
isLeaderHealthy = true
|
||||
if err == nil {
|
||||
isLeaderHealthy = resp.StatusCode == 200
|
||||
_ = resp.Body.Close()
|
||||
}
|
||||
|
||||
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
"fmt"
|
||||
"log"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
@@ -102,7 +103,8 @@ func probeRegister(ctx context.Context, client *http.Client, cfg ProbeConfig) er
|
||||
}
|
||||
|
||||
func probeFetchAssignments(ctx context.Context, client *http.Client, cfg ProbeConfig) ([]models.Site, error) {
|
||||
req, err := http.NewRequestWithContext(ctx, "GET", cfg.LeaderURL+"/api/probe/assignments?node_id="+cfg.NodeID, nil)
|
||||
assignURL := cfg.LeaderURL + "/api/probe/assignments?" + url.Values{"node_id": {cfg.NodeID}}.Encode()
|
||||
req, err := http.NewRequestWithContext(ctx, "GET", assignURL, nil)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
+28
-10
@@ -6,6 +6,8 @@ import (
|
||||
"fmt"
|
||||
"math/rand/v2"
|
||||
"net/http"
|
||||
"regexp"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
@@ -14,6 +16,13 @@ import (
|
||||
"gitea.lerkolabs.com/lerko/uptop/internal/store"
|
||||
)
|
||||
|
||||
const (
|
||||
maxLogEntries = 100
|
||||
pollInterval = 5 * time.Second
|
||||
pushGracePeriod = 5 * time.Second
|
||||
minCheckInterval = 5
|
||||
)
|
||||
|
||||
type Engine struct {
|
||||
mu sync.RWMutex
|
||||
liveState map[int]models.Site
|
||||
@@ -78,14 +87,23 @@ func (e *Engine) SetInsecureSkipVerify(skip bool) {
|
||||
e.insecureSkipVerify = skip
|
||||
}
|
||||
|
||||
var ansiRe = regexp.MustCompile(`\x1b\[[0-9;]*[a-zA-Z]`)
|
||||
|
||||
func sanitizeLog(s string) string {
|
||||
s = ansiRe.ReplaceAllString(s, "")
|
||||
s = strings.ReplaceAll(s, "\n", "\\n")
|
||||
s = strings.ReplaceAll(s, "\r", "")
|
||||
return s
|
||||
}
|
||||
|
||||
func (e *Engine) AddLog(msg string) {
|
||||
e.logMu.Lock()
|
||||
defer e.logMu.Unlock()
|
||||
ts := time.Now().Format("15:04:05")
|
||||
entry := fmt.Sprintf("[%s] %s", ts, msg)
|
||||
entry := fmt.Sprintf("[%s] %s", ts, sanitizeLog(msg))
|
||||
e.logStore = append([]string{entry}, e.logStore...)
|
||||
if len(e.logStore) > 100 {
|
||||
e.logStore = e.logStore[:100]
|
||||
if len(e.logStore) > maxLogEntries {
|
||||
e.logStore = e.logStore[:maxLogEntries]
|
||||
}
|
||||
go func() { _ = e.db.SaveLog(entry) }()
|
||||
}
|
||||
@@ -210,7 +228,7 @@ func (e *Engine) Start(ctx context.Context) {
|
||||
if err != nil {
|
||||
e.AddLog(fmt.Sprintf("Failed to load sites: %v", err))
|
||||
select {
|
||||
case <-time.After(5 * time.Second):
|
||||
case <-time.After(pollInterval):
|
||||
case <-ctx.Done():
|
||||
return
|
||||
}
|
||||
@@ -244,7 +262,7 @@ func (e *Engine) Start(ctx context.Context) {
|
||||
}
|
||||
|
||||
select {
|
||||
case <-time.After(5 * time.Second):
|
||||
case <-time.After(pollInterval):
|
||||
case <-ctx.Done():
|
||||
return
|
||||
}
|
||||
@@ -314,7 +332,7 @@ func (e *Engine) monitorRoutine(ctx context.Context, id int) {
|
||||
|
||||
if !e.IsActive() {
|
||||
select {
|
||||
case <-time.After(5 * time.Second):
|
||||
case <-time.After(pollInterval):
|
||||
case <-ctx.Done():
|
||||
return
|
||||
}
|
||||
@@ -330,7 +348,7 @@ func (e *Engine) monitorRoutine(ctx context.Context, id int) {
|
||||
|
||||
if site.Paused {
|
||||
select {
|
||||
case <-time.After(5 * time.Second):
|
||||
case <-time.After(pollInterval):
|
||||
case <-ctx.Done():
|
||||
return
|
||||
}
|
||||
@@ -338,8 +356,8 @@ func (e *Engine) monitorRoutine(ctx context.Context, id int) {
|
||||
}
|
||||
|
||||
interval := site.Interval
|
||||
if interval < 5 {
|
||||
interval = 5
|
||||
if interval < minCheckInterval {
|
||||
interval = minCheckInterval
|
||||
}
|
||||
jitter := time.Duration(rand.IntN(interval*100)) * time.Millisecond //nolint:gosec // non-security jitter
|
||||
select {
|
||||
@@ -380,7 +398,7 @@ func (e *Engine) checkByID(id int) {
|
||||
}
|
||||
|
||||
func (e *Engine) checkPush(site models.Site) {
|
||||
deadline := site.LastCheck.Add(time.Duration(site.Interval) * time.Second).Add(5 * time.Second)
|
||||
deadline := site.LastCheck.Add(time.Duration(site.Interval) * time.Second).Add(pushGracePeriod)
|
||||
if time.Now().After(deadline) {
|
||||
e.handleStatusChange(site, "DOWN", 0, 0)
|
||||
} else if site.Status != "UP" {
|
||||
|
||||
@@ -18,6 +18,8 @@ import (
|
||||
"gitea.lerkolabs.com/lerko/uptop/internal/store"
|
||||
)
|
||||
|
||||
const maxRequestBody = 1 << 20
|
||||
|
||||
func checkSecret(got, want string) bool {
|
||||
return subtle.ConstantTimeCompare([]byte(got), []byte(want)) == 1
|
||||
}
|
||||
@@ -204,6 +206,10 @@ func Start(cfg ServerConfig, s store.Store, eng *monitor.Engine) *http.Server {
|
||||
|
||||
// 1. Push Heartbeat
|
||||
mux.HandleFunc("/api/push", RateLimit(pushRL, func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.Method != http.MethodGet && r.Method != http.MethodPost {
|
||||
http.Error(w, "Method not allowed", http.StatusMethodNotAllowed)
|
||||
return
|
||||
}
|
||||
token := extractBearerToken(r)
|
||||
if token == "" {
|
||||
if qt := r.URL.Query().Get("token"); qt != "" {
|
||||
@@ -225,6 +231,10 @@ func Start(cfg ServerConfig, s store.Store, eng *monitor.Engine) *http.Server {
|
||||
|
||||
// 2. Health Check (For Cluster Follower)
|
||||
mux.HandleFunc("/api/health", func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.Method != http.MethodGet {
|
||||
http.Error(w, "Method not allowed", http.StatusMethodNotAllowed)
|
||||
return
|
||||
}
|
||||
if cfg.ClusterKey != "" && !checkSecret(r.Header.Get("X-Upkeep-Secret"), cfg.ClusterKey) {
|
||||
http.Error(w, "Unauthorized", http.StatusUnauthorized)
|
||||
return
|
||||
@@ -263,7 +273,7 @@ func Start(cfg ServerConfig, s store.Store, eng *monitor.Engine) *http.Server {
|
||||
http.Error(w, "Unauthorized", http.StatusUnauthorized)
|
||||
return
|
||||
}
|
||||
r.Body = http.MaxBytesReader(w, r.Body, 1<<20)
|
||||
r.Body = http.MaxBytesReader(w, r.Body, maxRequestBody)
|
||||
var data models.Backup
|
||||
if err := json.NewDecoder(r.Body).Decode(&data); err != nil {
|
||||
http.Error(w, "Invalid JSON", http.StatusBadRequest)
|
||||
@@ -287,7 +297,7 @@ func Start(cfg ServerConfig, s store.Store, eng *monitor.Engine) *http.Server {
|
||||
http.Error(w, "Unauthorized", http.StatusUnauthorized)
|
||||
return
|
||||
}
|
||||
r.Body = http.MaxBytesReader(w, r.Body, 1<<20)
|
||||
r.Body = http.MaxBytesReader(w, r.Body, maxRequestBody)
|
||||
var kb importer.KumaBackup
|
||||
if err := json.NewDecoder(r.Body).Decode(&kb); err != nil {
|
||||
log.Printf("Invalid Kuma JSON: %v", err)
|
||||
@@ -313,7 +323,7 @@ func Start(cfg ServerConfig, s store.Store, eng *monitor.Engine) *http.Server {
|
||||
http.Error(w, "Unauthorized", http.StatusUnauthorized)
|
||||
return
|
||||
}
|
||||
r.Body = http.MaxBytesReader(w, r.Body, 1<<20)
|
||||
r.Body = http.MaxBytesReader(w, r.Body, maxRequestBody)
|
||||
var req struct {
|
||||
ID string `json:"id"`
|
||||
Name string `json:"name"`
|
||||
@@ -340,6 +350,10 @@ func Start(cfg ServerConfig, s store.Store, eng *monitor.Engine) *http.Server {
|
||||
|
||||
// 7. Probe Assignment Fetch
|
||||
mux.HandleFunc("/api/probe/assignments", RateLimit(probeRL, func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.Method != http.MethodGet {
|
||||
http.Error(w, "Method not allowed", http.StatusMethodNotAllowed)
|
||||
return
|
||||
}
|
||||
if cfg.ClusterKey == "" || !checkSecret(r.Header.Get("X-Upkeep-Secret"), cfg.ClusterKey) {
|
||||
http.Error(w, "Unauthorized", http.StatusUnauthorized)
|
||||
return
|
||||
@@ -385,7 +399,7 @@ func Start(cfg ServerConfig, s store.Store, eng *monitor.Engine) *http.Server {
|
||||
http.Error(w, "Unauthorized", http.StatusUnauthorized)
|
||||
return
|
||||
}
|
||||
r.Body = http.MaxBytesReader(w, r.Body, 1<<20)
|
||||
r.Body = http.MaxBytesReader(w, r.Body, maxRequestBody)
|
||||
var req struct {
|
||||
NodeID string `json:"node_id"`
|
||||
Results []struct {
|
||||
@@ -416,6 +430,10 @@ func Start(cfg ServerConfig, s store.Store, eng *monitor.Engine) *http.Server {
|
||||
|
||||
// 9. Prometheus Metrics
|
||||
mux.HandleFunc("/metrics", func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.Method != http.MethodGet {
|
||||
http.Error(w, "Method not allowed", http.StatusMethodNotAllowed)
|
||||
return
|
||||
}
|
||||
if !cfg.MetricsPublic && cfg.ClusterKey != "" {
|
||||
if !checkSecret(r.Header.Get("X-Upkeep-Secret"), cfg.ClusterKey) {
|
||||
http.Error(w, "Unauthorized", http.StatusUnauthorized)
|
||||
|
||||
@@ -12,6 +12,13 @@ import (
|
||||
"gitea.lerkolabs.com/lerko/uptop/internal/models"
|
||||
)
|
||||
|
||||
const (
|
||||
maxCheckHistory = 1000
|
||||
checkHistoryPruneAt = 1100
|
||||
maxMaintenanceExport = 1000
|
||||
maxRequestBody = 1 << 20
|
||||
)
|
||||
|
||||
type SQLStore struct {
|
||||
db *sql.DB
|
||||
dialect Dialect
|
||||
@@ -351,10 +358,11 @@ func (s *SQLStore) SaveCheckFromNode(siteID int, nodeID string, latencyNs int64,
|
||||
}
|
||||
var count int
|
||||
_ = s.db.QueryRow(s.q("SELECT COUNT(*) FROM check_history WHERE site_id = ?"), siteID).Scan(&count)
|
||||
if count > 1100 {
|
||||
_, err = s.db.Exec(s.q(`DELETE FROM check_history WHERE site_id = ? AND id NOT IN (
|
||||
SELECT id FROM check_history WHERE site_id = ? ORDER BY checked_at DESC LIMIT 1000
|
||||
)`), siteID, siteID)
|
||||
if count > checkHistoryPruneAt {
|
||||
pruneQuery := fmt.Sprintf(`DELETE FROM check_history WHERE site_id = ? AND id NOT IN (
|
||||
SELECT id FROM check_history WHERE site_id = ? ORDER BY checked_at DESC LIMIT %d
|
||||
)`, maxCheckHistory)
|
||||
_, err = s.db.Exec(s.q(pruneQuery), siteID, siteID)
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
@@ -570,7 +578,7 @@ func (s *SQLStore) ExportData() (models.Backup, error) {
|
||||
if err != nil {
|
||||
return models.Backup{}, err
|
||||
}
|
||||
windows, err := s.GetAllMaintenanceWindows(1000)
|
||||
windows, err := s.GetAllMaintenanceWindows(maxMaintenanceExport)
|
||||
if err != nil {
|
||||
return models.Backup{}, err
|
||||
}
|
||||
|
||||
+7
-5
@@ -3,14 +3,15 @@ package tui
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"gitea.lerkolabs.com/lerko/uptop/internal/models"
|
||||
"gitea.lerkolabs.com/lerko/uptop/internal/monitor"
|
||||
"gitea.lerkolabs.com/lerko/uptop/internal/store"
|
||||
"math"
|
||||
"sort"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"gitea.lerkolabs.com/lerko/uptop/internal/models"
|
||||
"gitea.lerkolabs.com/lerko/uptop/internal/monitor"
|
||||
"gitea.lerkolabs.com/lerko/uptop/internal/store"
|
||||
|
||||
"github.com/charmbracelet/bubbles/viewport"
|
||||
tea "github.com/charmbracelet/bubbletea"
|
||||
"github.com/charmbracelet/harmonica"
|
||||
@@ -956,8 +957,9 @@ func siteOrder(s models.Site) int {
|
||||
}
|
||||
|
||||
func limitStr(text string, max int) string {
|
||||
if len(text) > max {
|
||||
return text[:max-3] + "..."
|
||||
runes := []rune(text)
|
||||
if len(runes) > max {
|
||||
return string(runes[:max-3]) + "..."
|
||||
}
|
||||
return text
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user