lerkolabs/uptop
1
1
Fork 0
Code Issues 17 Pull Requests 1 Actions Packages Projects 2 Releases 1 Wiki Activity

fix(security): secret masking, status DTO, fail-closed SSH revocation (Phase 1) #103

Merged
lerko merged 3 commits from fix/secret-masking-status-dto into main 2026-06-11 16:43:21 +00:00
Conversation 0 Commits 3 Files Changed 8
+79 -52
2 changed files with 79 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 0b64d13bb6 - Show all commits
internal/server/server.go
+37 -40
View File
@@ -33,40 +33,8 @@ func extractBearerToken(r *http.Request) string {
return "" return ""
} }
// safeSettingKeys lists, per provider type, the settings that are NOT secret // Alert-settings redaction policy lives in models.RedactAlertSettings so the
// and may be exported in the clear. Everything else is redacted. Providers // TUI detail panel and this export path share one allowlist.
// absent from this map (discord, slack, webhook, pushover) carry their secret
// in a field a denylist would miss — the webhook URL, the pushover token/user —
// so all of their settings are redacted.
var safeSettingKeys = map[string]map[string]bool{
"email": {"host": true, "port": true, "to": true, "from": true},
"ntfy": {"topic": true, "priority": true},
"telegram": {"chat_id": true},
"pagerduty": {"severity": true},
"gotify": {"priority": true},
"opsgenie": {"priority": true, "eu": true},
}
// redactByProvider keeps only the known-safe keys for the alert type and
// redacts everything else. An allowlist fails safe: an unknown or newly added
// setting is redacted by default instead of leaking. This closes the denylist
// gap where url (discord/slack/webhook/ntfy/gotify) and api_key (opsgenie) —
// the actual credentials — were exported in the clear.
func redactByProvider(alertType string, settings map[string]string) map[string]string {
safe := safeSettingKeys[alertType]
redacted := make(map[string]string, len(settings))
for k, v := range settings {
switch {
case v == "":
redacted[k] = ""
case safe[k]:
redacted[k] = v
default:
redacted[k] = "***REDACTED***"
}
}
return redacted
}
var statusTpl = template.Must(template.New("status").Parse(` var statusTpl = template.Must(template.New("status").Parse(`
<!DOCTYPE html> <!DOCTYPE html>
@@ -211,6 +179,23 @@ type ServerConfig struct {
MetricsPublic bool MetricsPublic bool
CORSOrigin string CORSOrigin string
TrustedProxies []*net.IPNet TrustedProxies []*net.IPNet
// QuietHTTPLog disables per-request stderr logging. Set when the local
// TUI owns the terminal — request logs would scribble over the alt screen.
QuietHTTPLog bool
}
// statusSite is the public DTO for /status/json. models.Site must never be
// serialized raw here: it carries internal fields (LastError, Hostname, Port,
// DNSServer, AlertID, Token, ...) and every field added to it would become
// public by default. Field names match what the status page JS reads.
type statusSite struct {
Name string
Type string
URL string
Status string
Paused bool
LastCheck time.Time
Latency time.Duration
} }
func Start(cfg ServerConfig, s store.Store, eng *monitor.Engine) *http.Server { func Start(cfg ServerConfig, s store.Store, eng *monitor.Engine) *http.Server {
@@ -278,7 +263,7 @@ func Start(cfg ServerConfig, s store.Store, eng *monitor.Engine) *http.Server {
} }
if r.URL.Query().Get("redact_secrets") != "false" { if r.URL.Query().Get("redact_secrets") != "false" {
for i := range data.Alerts { for i := range data.Alerts {
data.Alerts[i].Settings = redactByProvider(data.Alerts[i].Type, data.Alerts[i].Settings) data.Alerts[i].Settings = models.RedactAlertSettings(data.Alerts[i].Type, data.Alerts[i].Settings)
} }
} }
_ = json.NewEncoder(w).Encode(data) //nolint:errcheck _ = json.NewEncoder(w).Encode(data) //nolint:errcheck
@@ -483,18 +468,27 @@ func Start(cfg ServerConfig, s store.Store, eng *monitor.Engine) *http.Server {
maintSet[mw.MonitorID] = true maintSet[mw.MonitorID] = true
} }
} }
public := make(map[int]statusSite, len(state))
for id, site := range state { for id, site := range state {
site.Token = "" status := site.Status
if allInMaint || maintSet[site.ID] || (site.ParentID > 0 && maintSet[site.ParentID]) { if allInMaint || maintSet[site.ID] || (site.ParentID > 0 && maintSet[site.ParentID]) {
site.Status = "MAINT" status = "MAINT"
}
public[id] = statusSite{
Name: site.Name,
Type: site.Type,
URL: site.URL,
Status: status,
Paused: site.Paused,
LastCheck: site.LastCheck,
Latency: site.Latency,
} }
state[id] = site
} }
if cfg.CORSOrigin != "" { if cfg.CORSOrigin != "" {
w.Header().Set("Access-Control-Allow-Origin", cfg.CORSOrigin) w.Header().Set("Access-Control-Allow-Origin", cfg.CORSOrigin)
} }
w.Header().Set("Content-Type", "application/json") w.Header().Set("Content-Type", "application/json")
_ = json.NewEncoder(w).Encode(state) //nolint:errcheck _ = json.NewEncoder(w).Encode(public) //nolint:errcheck
})) }))
} }
@@ -502,7 +496,10 @@ func Start(cfg ServerConfig, s store.Store, eng *monitor.Engine) *http.Server {
fmt.Println("WARNING: Cluster mode active without TLS. Secrets transmitted in cleartext.") fmt.Println("WARNING: Cluster mode active without TLS. Secrets transmitted in cleartext.")
} }
handler := loggingMiddleware(cfg.TrustedProxies, securityHeadersMiddleware(mux)) handler := securityHeadersMiddleware(mux)
if !cfg.QuietHTTPLog {
handler = loggingMiddleware(cfg.TrustedProxies, handler)
}
if cfg.TLSCert != "" { if cfg.TLSCert != "" {
handler = hstsMiddleware(handler) handler = hstsMiddleware(handler)
} }
internal/server/server_test.go
+42 -12
View File
@@ -2,6 +2,7 @@ package server
import ( import (
"bytes" "bytes"
"context"
"encoding/json" "encoding/json"
"fmt" "fmt"
"net" "net"
@@ -480,15 +481,32 @@ func TestStatusPage_Enabled(t *testing.T) {
} }
} }
func TestStatusJSON_TokensStripped(t *testing.T) { func TestStatusJSON_PublicDTOOnly(t *testing.T) {
ts := newTestServer(t, "secret", true) ts := newTestServer(t, "secret", true)
// Inject a site with a token into engine state // Seed a push monitor (no network IO) through the store and start the
ts.engine.UpdateSiteConfig(models.Site{ID: 1, Name: "test", Type: "push", Token: "secret-token", Status: "UP"}) // engine so its poll loop loads it into live state — the path real sites
// Need to inject directly since UpdateSiteConfig only updates existing // take. The old version of this test injected via UpdateSiteConfig, which
func() { // no-ops for unknown IDs, so it asserted over zero sites and passed
ts.engine.RecordHeartbeat("unused") // just to exercise, won't match // against a server that leaked tokens.
}() ts.store.sites = []models.Site{{
ID: 1, Name: "test", Type: "push", Token: "secret-token",
Hostname: "internal-host", LastError: "internal failure detail", AlertID: 3,
}}
ctx, cancel := context.WithCancel(context.Background())
ts.engine.Start(ctx)
t.Cleanup(func() {
cancel()
ts.engine.Stop()
})
deadline := time.Now().Add(2 * time.Second)
for time.Now().Before(deadline) && len(ts.engine.GetLiveState()) == 0 {
time.Sleep(10 * time.Millisecond)
}
if len(ts.engine.GetLiveState()) == 0 {
t.Fatal("engine never loaded the seeded site")
}
resp, err := http.Get(ts.baseURL + "/status/json") resp, err := http.Get(ts.baseURL + "/status/json")
if err != nil { if err != nil {
@@ -498,11 +516,23 @@ func TestStatusJSON_TokensStripped(t *testing.T) {
if resp.StatusCode != 200 { if resp.StatusCode != 200 {
t.Errorf("expected 200, got %d", resp.StatusCode) t.Errorf("expected 200, got %d", resp.StatusCode)
} }
var state map[string]models.Site
json.NewDecoder(resp.Body).Decode(&state) // Decode raw so absent struct fields can't mask leaked JSON keys.
var state map[string]map[string]any
if err := json.NewDecoder(resp.Body).Decode(&state); err != nil {
t.Fatal(err)
}
if len(state) != 1 {
t.Fatalf("expected 1 site in status JSON, got %d", len(state))
}
for _, site := range state { for _, site := range state {
if site.Token != "" { if site["Name"] != "test" {
t.Error("expected token stripped from status JSON response") t.Errorf("expected Name to be public, got %v", site["Name"])
}
for _, leaked := range []string{"Token", "LastError", "Hostname", "Port", "DNSServer", "AlertID", "AcceptedCodes", "Interval"} {
if _, ok := site[leaked]; ok {
t.Errorf("status JSON leaks internal field %q", leaked)
}
} }
} }
} }
@@ -656,7 +686,7 @@ func TestRedactByProvider(t *testing.T) {
} }
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
out := redactByProvider(tt.typ, tt.in) out := models.RedactAlertSettings(tt.typ, tt.in)
for _, k := range tt.redacted { for _, k := range tt.redacted {
if out[k] != "***REDACTED***" { if out[k] != "***REDACTED***" {
t.Errorf("key %q: expected redacted, got %q", k, out[k]) t.Errorf("key %q: expected redacted, got %q", k, out[k])