fix(security): close XFF bypass and three secret-leak paths #100

2026-06-10T22:50:42Z

lerko commented

2026-06-10 22:50:42 +00:00

Four fixes hardening the secrets + rate-limit posture (the security cluster of the fresh-eyes first-cut backlog).

3a — X-Forwarded-For rate-limit bypass + memory DoS (`ratelimit.go`)

clientIP returned the raw XFF header, so an attacker rotating it minted unlimited distinct limiter keys — never tripping the limit and growing the visitors map without bound. XFF is now honored only when the immediate peer is a configured trusted proxy (UPTOP_TRUSTED_PROXIES, CIDRs or bare IPs), using the right-most non-trusted hop (RFC 7239); otherwise the key is the real RemoteAddr. The visitors map is bounded with LRU eviction as defense in depth.

3b — Export redaction denylist → per-provider allowlist (`server.go`)

The six-key denylist missed the actual credentials — the webhook URL for discord/slack/webhook/ntfy/gotify and api_key for opsgenie — exporting them in the clear. redactByProvider keeps only known-safe keys per provider type and redacts everything else, so unknown/new keys fail safe.

3c — `ImportData` plaintext secrets (`sqlstore.go`)

Import inserted raw json.Marshal(settings), bypassing the encryption AddAlert/UpdateAlert use. It now routes through marshalSettings, so a restore with UPTOP_ENCRYPTION_KEY set stores enc:-prefixed ciphertext, not plaintext.

3d — Alert error credential leak (`alert.go`)

Provider Send returned the raw *url.Error, whose URL carries the secret (Telegram bot token in the path, webhook secrets in the URL); it was persisted to AlertHealth.LastError and shown in the TUI. sanitizeError strips the URL, keeping the operation + underlying cause.

Tests

Trusted/untrusted XFF + spoofed-bypass + map-bound, the allowlist per provider, encrypted-on-import round-trip, URL-stripped errors. Full suite green under -race; golangci-lint clean locally. README documents UPTOP_TRUSTED_PROXIES + reverse-proxy setup.

Phase 3 of the fresh-eyes first-cut backlog (builds on #98, #99).

Four fixes hardening the secrets + rate-limit posture (the security cluster of the fresh-eyes first-cut backlog). ### 3a — X-Forwarded-For rate-limit bypass + memory DoS (`ratelimit.go`) `clientIP` returned the raw XFF header, so an attacker rotating it minted unlimited distinct limiter keys — never tripping the limit and growing the visitors map without bound. XFF is now honored **only when the immediate peer is a configured trusted proxy** (`UPTOP_TRUSTED_PROXIES`, CIDRs or bare IPs), using the right-most non-trusted hop (RFC 7239); otherwise the key is the real `RemoteAddr`. The visitors map is bounded with LRU eviction as defense in depth. ### 3b — Export redaction denylist → per-provider allowlist (`server.go`) The six-key denylist missed the actual credentials — the **webhook URL** for discord/slack/webhook/ntfy/gotify and **api_key** for opsgenie — exporting them in the clear. `redactByProvider` keeps only known-safe keys per provider type and redacts everything else, so unknown/new keys fail safe. ### 3c — `ImportData` plaintext secrets (`sqlstore.go`) Import inserted raw `json.Marshal(settings)`, bypassing the encryption `AddAlert`/`UpdateAlert` use. It now routes through `marshalSettings`, so a restore with `UPTOP_ENCRYPTION_KEY` set stores `enc:`-prefixed ciphertext, not plaintext. ### 3d — Alert error credential leak (`alert.go`) Provider `Send` returned the raw `*url.Error`, whose URL carries the secret (Telegram bot token in the path, webhook secrets in the URL); it was persisted to `AlertHealth.LastError` and shown in the TUI. `sanitizeError` strips the URL, keeping the operation + underlying cause. ## Tests Trusted/untrusted XFF + spoofed-bypass + map-bound, the allowlist per provider, encrypted-on-import round-trip, URL-stripped errors. Full suite green under `-race`; golangci-lint clean locally. README documents `UPTOP_TRUSTED_PROXIES` + reverse-proxy setup. Phase 3 of the fresh-eyes first-cut backlog (builds on #98, #99).

lerko added 1 commit 2026-06-10 22:50:42 +00:00

fix(security): close XFF bypass and three secret-leak paths

CI / test (pull_request) Successful in 2m36s

Details

CI / lint (pull_request) Successful in 56s

Details

CI / vulncheck (pull_request) Successful in 46s

Details

809620340e

Four fixes hardening the secrets and rate-limit posture a prior audit
left or that regressed:

X-Forwarded-For rate-limit bypass + memory DoS (ratelimit.go): clientIP
returned the raw XFF header, so an attacker rotating it minted unlimited
distinct limiter keys — never tripping the limit and growing the visitors
map without bound. XFF is now honored only when the immediate peer is a
configured trusted proxy (UPTOP_TRUSTED_PROXIES, CIDRs or bare IPs), using
the right-most non-trusted hop; otherwise the key is the real RemoteAddr.
The visitors map is bounded with LRU eviction as defense in depth.

Export redaction denylist -> per-provider allowlist (server.go): the old
six-key denylist missed the actual credentials — the webhook URL for
discord/slack/webhook/ntfy/gotify and api_key for opsgenie — exporting
them in the clear. redactByProvider keeps only known-safe keys per
provider type and redacts everything else, so unknown/new keys fail safe.

ImportData plaintext secrets (sqlstore.go): import inserted raw
json.Marshal(settings), bypassing the encryption AddAlert/UpdateAlert
use. It now routes through marshalSettings, so a restore with
UPTOP_ENCRYPTION_KEY set stores enc:-prefixed ciphertext, not plaintext.

Alert error credential leak (alert.go): provider Send returned the raw
*url.Error, whose URL carries the secret (Telegram bot token in the path,
webhook secrets in the URL); it was persisted to AlertHealth.LastError
and shown in the TUI. sanitizeError strips the URL, keeping the operation
and underlying cause.

Tests cover trusted/untrusted XFF + spoofed-bypass + map bound, the
allowlist per provider, encrypted-on-import round-trip, and URL-stripped
errors. README documents UPTOP_TRUSTED_PROXIES. Full suite green under
-race; golangci-lint clean.

lerko merged commit 809620340e into main

2026-06-10 23:52:09 +00:00

lerko deleted branch fix/security-xff-secrets

2026-06-10 23:52:09 +00:00

lerko referenced this pull request

2026-06-11 01:15:04 +00:00

fix(tui): move blocking DB IO out of Update/View into tea.Cmds #101

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: lerkolabs/uptop#100

fix(security): close XFF bypass and three secret-leak paths #100

3a — X-Forwarded-For rate-limit bypass + memory DoS (ratelimit.go)

3b — Export redaction denylist → per-provider allowlist (server.go)

3c — ImportData plaintext secrets (sqlstore.go)

3d — Alert error credential leak (alert.go)

Tests

3a — X-Forwarded-For rate-limit bypass + memory DoS (`ratelimit.go`)

3b — Export redaction denylist → per-provider allowlist (`server.go`)

3c — `ImportData` plaintext secrets (`sqlstore.go`)

3d — Alert error credential leak (`alert.go`)