Skip to content

Commit 407e2f0

Browse files
committed
Refactor HTTP response handling and improve error management
- Updated various handlers to use the error return value from `json.NewEncoder(w).Encode(...)` for better error handling. - Replaced direct calls to `json.NewEncoder(w).Encode(...)` with error checks to ensure proper logging and handling of potential write errors. - Removed unused error handling functions related to API error responses to streamline the codebase. - Improved TLS configuration in email service to enforce minimum TLS version. - Refactored backend server status parsing for clarity and maintainability. - Removed redundant functions for checking and counting non-hardware backends in the template helper. - Enhanced rollback handling in plugin migration to ensure proper resource management.
1 parent 495783f commit 407e2f0

37 files changed

Lines changed: 203 additions & 336 deletions

gearbox-agent/internal/api/websocket.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ func (h *WSHandler) HandleEvents(w http.ResponseWriter, r *http.Request) {
9797
h.logger.Error("WebSocket upgrade failed", "error", err)
9898
return
9999
}
100-
defer conn.Close()
100+
defer func() { _ = conn.Close() }()
101101

102102
// Subscribe to events
103103
sub := h.eventBus.Subscribe()

gearbox-agent/internal/plugins/certs/plugin.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ func (p *Plugin) handleDownload(w http.ResponseWriter, r *http.Request) {
194194
w.Header().Set("Content-Type", "application/x-pem-file")
195195
w.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%q", filename))
196196
w.Header().Set("Content-Length", fmt.Sprintf("%d", len(certData)))
197-
w.Write(certData)
197+
_, _ = w.Write(certData)
198198
return
199199
}
200200
}
@@ -241,7 +241,7 @@ func (p *Plugin) handleDownload(w http.ResponseWriter, r *http.Request) {
241241
w.Header().Set("Content-Type", "application/x-pem-file")
242242
w.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%q", filename))
243243
w.Header().Set("Content-Length", fmt.Sprintf("%d", len(certData)))
244-
w.Write(certData)
244+
_, _ = w.Write(certData)
245245
}
246246

247247
// GetCollector returns the certificate collector for use by other components.

gearbox-agent/internal/plugins/haproxy/plugin.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ func (p *Plugin) handleStats(w http.ResponseWriter, r *http.Request) {
195195
// Allow requesting raw CSV format
196196
if r.URL.Query().Get("format") == "csv" {
197197
w.Header().Set("Content-Type", "text/csv")
198-
w.Write([]byte(csvData))
198+
_, _ = w.Write([]byte(csvData))
199199
return
200200
}
201201

gearbox/cmd/server/main.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ func main() {
7878
if err != nil {
7979
log.Fatalf("Failed to initialize database: %v", err)
8080
}
81-
defer db.Close()
81+
defer func() { _ = db.Close() }()
8282
logger.Info("database initialized",
8383
"path", cfg.DatabasePath,
8484
"retention_hours", cfg.DatabaseRetentionHours)
@@ -117,7 +117,7 @@ func main() {
117117
if cfg.AdminPassword == "" {
118118
// SECURITY: Write password to secure file with restrictive permissions
119119
// File will be auto-deleted after first password change
120-
credentialsFile := "data/admin-credentials.txt"
120+
credentialsFile := "data/admin-credentials.txt" //#nosec G101 -- Not a credential; this is a file path constant
121121
credentialsContent := fmt.Sprintf("ADMIN USER CREATED\n\nEmail: admin\nPassword: %s\n\nIMPORTANT: You will be forced to change this password on first login.\nThis file will be automatically deleted after you change your password.\n", adminPassword)
122122

123123
if err := os.WriteFile(credentialsFile, []byte(credentialsContent), 0600); err != nil {
@@ -367,7 +367,9 @@ func main() {
367367
dataSourceRegistry := widget.NewDataSourceRegistry(logger)
368368

369369
// Register core widgets
370-
widgets.RegisterCoreWidgets(widgetRegistry)
370+
if err := widgets.RegisterCoreWidgets(widgetRegistry); err != nil {
371+
log.Fatalf("Failed to register core widgets: %v", err)
372+
}
371373

372374
// Register HAProxy-specific widgets
373375
if err := dashboardPlugin.RegisterHAProxyWidgets(widgetRegistry); err != nil {
@@ -464,7 +466,7 @@ func main() {
464466
http.Error(w, "Not Found", http.StatusNotFound)
465467
return
466468
}
467-
w.Write(data)
469+
_, _ = w.Write(data)
468470
})
469471
r.Get("/favicon.svg", func(w http.ResponseWriter, r *http.Request) {
470472
w.Header().Set("Content-Type", "image/svg+xml")
@@ -474,7 +476,7 @@ func main() {
474476
http.Error(w, "Not Found", http.StatusNotFound)
475477
return
476478
}
477-
w.Write(data)
479+
_, _ = w.Write(data)
478480
})
479481

480482
// Static file server for CSS, JavaScript, and other assets

gearbox/internal/framework/agent/client.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ func createTLSConfig() *tls.Config {
7373
fmt.Fprintf(os.Stderr, "WARNING: TLS certificate verification is DISABLED (GEARBOX_INSECURE_TLS=true)\n")
7474
fmt.Fprintf(os.Stderr, "WARNING: This is NOT recommended for production use\n")
7575
return &tls.Config{
76-
InsecureSkipVerify: true,
76+
InsecureSkipVerify: true, //#nosec G402 -- User explicitly opted in via GEARBOX_INSECURE_TLS env var
7777
MinVersion: tls.VersionTLS12,
7878
}
7979
}
@@ -82,7 +82,7 @@ func createTLSConfig() *tls.Config {
8282
caCertPath := os.Getenv("AGENT_CA_CERT_PATH")
8383
if caCertPath != "" {
8484
// Load CA certificate for validating agent certificates
85-
caCertPEM, err := os.ReadFile(caCertPath)
85+
caCertPEM, err := os.ReadFile(caCertPath) //#nosec G304 -- Path from trusted env var AGENT_CA_CERT_PATH
8686
if err != nil {
8787
fmt.Fprintf(os.Stderr, "ERROR: Failed to read CA certificate from %s: %v\n", caCertPath, err)
8888
fmt.Fprintf(os.Stderr, "ERROR: Falling back to system certificate pool\n")
@@ -117,7 +117,7 @@ func createTLSConfig() *tls.Config {
117117
// doRequest performs an HTTP request with authentication.
118118
func (c *Client) doRequest(method, path string, query url.Values) ([]byte, error) {
119119
fullURL := c.baseURL + path
120-
if query != nil && len(query) > 0 {
120+
if len(query) > 0 {
121121
fullURL += "?" + query.Encode()
122122
}
123123

@@ -169,7 +169,7 @@ func (c *Client) doRequestWithBody(method, path string, reqBody interface{}) ([]
169169
// doRequestWithBodyAndQuery performs an HTTP request with a JSON body and query parameters.
170170
func (c *Client) doRequestWithBodyAndQuery(method, path string, reqBody interface{}, query url.Values) ([]byte, error) {
171171
fullURL := c.baseURL + path
172-
if query != nil && len(query) > 0 {
172+
if len(query) > 0 {
173173
fullURL += "?" + query.Encode()
174174
}
175175

gearbox/internal/framework/agent/websocket.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ func (w *WSClient) connectAndRead(ctx context.Context) error {
154154
w.mu.Lock()
155155
w.connected = false
156156
if w.conn != nil {
157-
w.conn.Close()
157+
_ = w.conn.Close()
158158
w.conn = nil
159159
}
160160
cb := w.connectionCallback

gearbox/internal/framework/collector/websocket_manager.go

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -257,39 +257,6 @@ func (m *WebSocketManager) triggerMetadataRefresh(serverID string) {
257257
}()
258258
}
259259

260-
// triggerStatsRefresh triggers an immediate stats refresh for a server.
261-
func (m *WebSocketManager) triggerStatsRefresh(serverID string) {
262-
manager, exists := m.registry.GetCollector(serverID)
263-
if !exists {
264-
m.logger.Debug("cannot refresh stats: collector not found",
265-
"server_id", serverID)
266-
return
267-
}
268-
269-
// Refresh stats synchronously so cache is updated before SSE event
270-
if err := manager.RefreshStats(); err != nil {
271-
m.logger.Debug("failed to refresh stats",
272-
"server_id", serverID,
273-
"error", err)
274-
}
275-
}
276-
277-
// triggerMetricsRefresh triggers an immediate metrics refresh for a server.
278-
func (m *WebSocketManager) triggerMetricsRefresh(serverID string) {
279-
manager, exists := m.registry.GetCollector(serverID)
280-
if !exists {
281-
m.logger.Debug("cannot refresh metrics: collector not found",
282-
"server_id", serverID)
283-
return
284-
}
285-
286-
// Refresh metrics synchronously so cache is updated before SSE event
287-
if err := manager.RefreshMetrics(); err != nil {
288-
m.logger.Debug("failed to refresh metrics",
289-
"server_id", serverID,
290-
"error", err)
291-
}
292-
}
293260

294261
// updateStatsFromEvent updates the cache with stats from a WebSocket event.
295262
// This avoids a redundant API call to the agent since it already sent the stats.

gearbox/internal/framework/dashboard/storage.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ func NewStorage(dataDir string, logger *slog.Logger) (*Storage, error) {
2525

2626
// Ensure the dashboards directory exists
2727
dashboardDir := filepath.Join(dataDir, "dashboards")
28-
if err := os.MkdirAll(dashboardDir, 0755); err != nil {
28+
if err := os.MkdirAll(dashboardDir, 0750); err != nil {
2929
return nil, fmt.Errorf("failed to create dashboards directory: %w", err)
3030
}
3131

@@ -47,7 +47,7 @@ func (s *Storage) Load(slug string) (*Dashboard, error) {
4747
// LoadFromPath loads a dashboard from a specific file path.
4848
func (s *Storage) LoadFromPath(filePath string) (*Dashboard, error) {
4949
// Read the YAML file
50-
data, err := os.ReadFile(filePath)
50+
data, err := os.ReadFile(filePath) //#nosec G304 -- filePath is constructed from slug via filepath.Join, not direct user input
5151
if err != nil {
5252
if os.IsNotExist(err) {
5353
return nil, fmt.Errorf("dashboard not found: %s", filePath)
@@ -113,7 +113,7 @@ func (s *Storage) Save(dashboard *Dashboard) error {
113113
}
114114

115115
// Write to file
116-
if err := os.WriteFile(filePath, data, 0644); err != nil {
116+
if err := os.WriteFile(filePath, data, 0600); err != nil {
117117
return fmt.Errorf("failed to write dashboard file: %w", err)
118118
}
119119

gearbox/internal/framework/database/alerts.go

Lines changed: 16 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ func (d *DB) GetAlertRules(serverID string) ([]models.AlertRule, error) {
175175
if err != nil {
176176
return nil, err
177177
}
178-
defer rows.Close()
178+
defer func() { _ = rows.Close() }()
179179

180180
var rules []models.AlertRule
181181
for rows.Next() {
@@ -227,7 +227,7 @@ func (d *DB) GetEnabledAlertRules(serverID string) ([]models.AlertRule, error) {
227227
if err != nil {
228228
return nil, err
229229
}
230-
defer rows.Close()
230+
defer func() { _ = rows.Close() }()
231231

232232
var rules []models.AlertRule
233233
for rows.Next() {
@@ -348,7 +348,7 @@ func (d *DB) getAlertsByStatus(serverID, status string) ([]models.Alert, error)
348348
if err != nil {
349349
return nil, err
350350
}
351-
defer rows.Close()
351+
defer func() { _ = rows.Close() }()
352352

353353
return d.scanAlertsWithEmails(rows)
354354
}
@@ -373,66 +373,11 @@ func (d *DB) GetRecentAlerts(serverID string, limit int) ([]models.Alert, error)
373373
if err != nil {
374374
return nil, err
375375
}
376-
defer rows.Close()
376+
defer func() { _ = rows.Close() }()
377377

378378
return d.scanAlertsWithEmails(rows)
379379
}
380380

381-
// scanAlerts scans alert rows into Alert structs (legacy, without user emails).
382-
func (d *DB) scanAlerts(rows *sql.Rows) ([]models.Alert, error) {
383-
var alerts []models.Alert
384-
for rows.Next() {
385-
var a models.Alert
386-
var ruleID, acknowledgedBy sql.NullInt64
387-
var metric, affectedEntity, metadata sql.NullString
388-
var metricValue, threshold sql.NullFloat64
389-
var acknowledgedAt, resolvedAt, silencedUntil sql.NullTime
390-
391-
err := rows.Scan(
392-
&a.ID, &ruleID, &a.BoxID, &a.Type, &a.Severity, &a.Status,
393-
&a.Title, &a.Message, &metric, &metricValue, &threshold,
394-
&affectedEntity, &a.TriggeredAt, &acknowledgedAt, &acknowledgedBy,
395-
&resolvedAt, &silencedUntil, &a.NotifiedEmail, &a.NotifiedWebhook, &metadata,
396-
)
397-
if err != nil {
398-
return nil, err
399-
}
400-
401-
if ruleID.Valid {
402-
a.RuleID = ruleID.Int64
403-
}
404-
if metric.Valid {
405-
a.Metric = metric.String
406-
}
407-
if metricValue.Valid {
408-
a.MetricValue = metricValue.Float64
409-
}
410-
if threshold.Valid {
411-
a.Threshold = threshold.Float64
412-
}
413-
if affectedEntity.Valid {
414-
a.AffectedEntity = affectedEntity.String
415-
}
416-
if acknowledgedAt.Valid {
417-
a.AcknowledgedAt = &acknowledgedAt.Time
418-
}
419-
if acknowledgedBy.Valid {
420-
a.AcknowledgedBy = &acknowledgedBy.Int64
421-
}
422-
if resolvedAt.Valid {
423-
a.ResolvedAt = &resolvedAt.Time
424-
}
425-
if silencedUntil.Valid {
426-
a.SilencedUntil = &silencedUntil.Time
427-
}
428-
if metadata.Valid {
429-
a.Metadata = metadata.String
430-
}
431-
432-
alerts = append(alerts, a)
433-
}
434-
return alerts, nil
435-
}
436381

437382
// scanAlertsWithEmails scans alert rows with user email joins into Alert structs.
438383
func (d *DB) scanAlertsWithEmails(rows *sql.Rows) ([]models.Alert, error) {
@@ -562,7 +507,7 @@ func (d *DB) GetAlertSummary(serverID string) (*models.AlertSummary, error) {
562507
var status string
563508
var count int
564509
if err := rows.Scan(&status, &count); err != nil {
565-
rows.Close()
510+
_ = rows.Close()
566511
return nil, err
567512
}
568513
switch status {
@@ -574,7 +519,7 @@ func (d *DB) GetAlertSummary(serverID string) (*models.AlertSummary, error) {
574519
summary.TotalResolved = count
575520
}
576521
}
577-
rows.Close()
522+
_ = rows.Close()
578523

579524
// Count by severity (active only)
580525
rows, err = d.db.Query(`
@@ -588,12 +533,12 @@ func (d *DB) GetAlertSummary(serverID string) (*models.AlertSummary, error) {
588533
var severity string
589534
var count int
590535
if err := rows.Scan(&severity, &count); err != nil {
591-
rows.Close()
536+
_ = rows.Close()
592537
return nil, err
593538
}
594539
summary.BySeverity[severity] = count
595540
}
596-
rows.Close()
541+
_ = rows.Close()
597542

598543
// Count by type (active only)
599544
rows, err = d.db.Query(`
@@ -607,12 +552,12 @@ func (d *DB) GetAlertSummary(serverID string) (*models.AlertSummary, error) {
607552
var alertType string
608553
var count int
609554
if err := rows.Scan(&alertType, &count); err != nil {
610-
rows.Close()
555+
_ = rows.Close()
611556
return nil, err
612557
}
613558
summary.ByType[alertType] = count
614559
}
615-
rows.Close()
560+
_ = rows.Close()
616561

617562
// Get recent alerts
618563
summary.RecentAlerts, _ = d.GetRecentAlerts(serverID, 10)
@@ -762,7 +707,7 @@ func (m AlertMetadata) String() string {
762707

763708
func ParseAlertMetadata(s string) AlertMetadata {
764709
var m AlertMetadata
765-
json.Unmarshal([]byte(s), &m)
710+
_ = json.Unmarshal([]byte(s), &m)
766711
return m
767712
}
768713

@@ -802,7 +747,7 @@ func (d *DB) GetAlertNotes(alertID int64) ([]AlertNote, error) {
802747
if err != nil {
803748
return nil, err
804749
}
805-
defer rows.Close()
750+
defer func() { _ = rows.Close() }()
806751

807752
var notes []AlertNote
808753
for rows.Next() {
@@ -828,7 +773,7 @@ func (d *DB) AcknowledgeAlertWithNote(alertID int64, userID string, note string)
828773
if err != nil {
829774
return err
830775
}
831-
defer tx.Rollback()
776+
defer func() { _ = tx.Rollback() }()
832777

833778
// Update alert status
834779
_, err = tx.Exec(`
@@ -863,7 +808,7 @@ func (d *DB) ResolveAlertWithNote(alertID int64, userID string, note string) err
863808
if err != nil {
864809
return err
865810
}
866-
defer tx.Rollback()
811+
defer func() { _ = tx.Rollback() }()
867812

868813
// Update alert status with resolved_by
869814
_, err = tx.Exec(`
@@ -913,7 +858,7 @@ func (d *DB) GetUserAlertSubscriptions(userID int64) ([]UserAlertSubscription, e
913858
if err != nil {
914859
return nil, err
915860
}
916-
defer rows.Close()
861+
defer func() { _ = rows.Close() }()
917862

918863
var subs []UserAlertSubscription
919864
for rows.Next() {
@@ -972,7 +917,7 @@ func (d *DB) GetUsersToNotify(serverID, alertType, severity string) ([]int64, er
972917
if err != nil {
973918
return nil, err
974919
}
975-
defer rows.Close()
920+
defer func() { _ = rows.Close() }()
976921

977922
var userIDs []int64
978923
for rows.Next() {

0 commit comments

Comments
 (0)