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
Owner

Phase 1 security quick wins from the fresh-eyes review. Three commits, independent diffs.

Alert secret masking

The detail panel dumped a.Settings raw — SMTP passwords, bot tokens, API keys on screen and into any recording or screen share. The redaction allowlist moved to models.RedactAlertSettings so the backup export and the TUI render through one policy; panel keys are sorted so rows stop reshuffling every tick. Also fixes three table-view leaks: PagerDuty routing_key and Pushover user key mask to first4…last4, and discord/slack/webhook URLs render scheme+host only — the URL path is the credential.

/status/json public DTO

The handler serialized raw models.Site: LastError internals, Hostname, Port, DNSServer, AlertID, intervals all public, and every future Site field public the day it's added. statusSite now exposes exactly what the status page renders: Name, Type, URL, Status, Paused, LastCheck, Latency.

Replaces the vacuous TestStatusJSON_TokensStripped (injected via UpdateSiteConfig, which no-ops for unknown IDs — asserted over zero sites). The new test seeds the store, starts the engine, waits for live state, and asserts internal fields are absent from the raw JSON.

Fail-closed SSH key revocation

Worse than reviewed: keyCache.Invalidate() had zero callers — revocation only propagated via the 30s TTL, and a DB outage froze the stale key set indefinitely. Invalidate now clears the key set and is wired through userInvalidatingStore, a decorator at the composition root that drops the cache on AddUser/UpdateUser/DeleteUser/ImportData (backup import replaces the user table). Transient refresh errors retain the previous key set so a DB blip can't lock every admin out; a post-revocation refresh failure denies. Refresh errors are logged. First tests for the SSH auth gate (4).

Rider

QuietHTTPLog suppresses per-request stderr logging when the local TUI owns the terminal — request logs scribbled over the alt screen. Full logging unification stays in Phase 5.

Tests

go test -race ./... green, golangci-lint 0 issues. New: 4 keyCache tests (first cmd/uptop coverage), 2 TUI masking tests, 1 DTO leak test.

Phase 1 security quick wins from the fresh-eyes review. Three commits, independent diffs. ## Alert secret masking The detail panel dumped `a.Settings` raw — SMTP passwords, bot tokens, API keys on screen and into any recording or screen share. The redaction allowlist moved to `models.RedactAlertSettings` so the backup export and the TUI render through one policy; panel keys are sorted so rows stop reshuffling every tick. Also fixes three table-view leaks: PagerDuty `routing_key` and Pushover user key mask to first4…last4, and discord/slack/webhook URLs render scheme+host only — the URL path is the credential. ## /status/json public DTO The handler serialized raw `models.Site`: `LastError` internals, `Hostname`, `Port`, `DNSServer`, `AlertID`, intervals all public, and every future Site field public the day it's added. `statusSite` now exposes exactly what the status page renders: Name, Type, URL, Status, Paused, LastCheck, Latency. Replaces the vacuous `TestStatusJSON_TokensStripped` (injected via `UpdateSiteConfig`, which no-ops for unknown IDs — asserted over zero sites). The new test seeds the store, starts the engine, waits for live state, and asserts internal fields are absent from the raw JSON. ## Fail-closed SSH key revocation Worse than reviewed: `keyCache.Invalidate()` had **zero callers** — revocation only propagated via the 30s TTL, and a DB outage froze the stale key set indefinitely. `Invalidate` now clears the key set and is wired through `userInvalidatingStore`, a decorator at the composition root that drops the cache on `AddUser`/`UpdateUser`/`DeleteUser`/`ImportData` (backup import replaces the user table). Transient refresh errors retain the previous key set so a DB blip can't lock every admin out; a post-revocation refresh failure denies. Refresh errors are logged. First tests for the SSH auth gate (4). ## Rider `QuietHTTPLog` suppresses per-request stderr logging when the local TUI owns the terminal — request logs scribbled over the alt screen. Full logging unification stays in Phase 5. ## Tests `go test -race ./...` green, golangci-lint 0 issues. New: 4 keyCache tests (first cmd/uptop coverage), 2 TUI masking tests, 1 DTO leak test.
lerko added 3 commits 2026-06-11 16:36:59 +00:00
The alert detail panel dumped a.Settings raw — SMTP passwords, bot
tokens, API keys on screen and into any recording or screen share. The
table view leaked the PagerDuty routing key, Pushover user key, and
full discord/slack/webhook URLs (the URL path is the credential).

The redaction allowlist moves from internal/server to
models.RedactAlertSettings so the backup export and the TUI render
through one policy. Panel keys are sorted so rows stop reshuffling
every tick; webhook URLs show scheme+host only; keys show
first4…last4.
The handler serialized raw models.Site — LastError internals,
Hostname, Port, DNSServer, AlertID, intervals all public, and every
future Site field public the day it's added. statusSite now exposes
exactly what the status page renders: Name, Type, URL, Status, Paused,
LastCheck, Latency.

Replaces the vacuous TestStatusJSON_TokensStripped, which injected via
UpdateSiteConfig (a no-op for unknown IDs) and asserted over zero
sites. The new test seeds the store, starts the engine, waits for live
state, and asserts internal fields are absent from the raw JSON.
fix(security): make SSH key revocation fail closed
CI / test (pull_request) Successful in 2m37s
CI / lint (pull_request) Successful in 56s
CI / vulncheck (pull_request) Successful in 51s
92efb8e270
keyCache.Invalidate existed but had zero callers, and refresh silently
swallowed store errors — a revoked key kept working off the stale
cache for as long as the DB stayed down.

Invalidate now clears the key set (not just the timestamp) and is
wired through userInvalidatingStore, a decorator at the composition
root that drops the cache on AddUser/UpdateUser/DeleteUser/ImportData.
Transient refresh errors still retain the previous key set so a DB
blip can't lock every admin out, but a post-revocation refresh failure
denies. Refresh errors are logged. First tests for the SSH auth gate.

Also suppresses per-request HTTP logging when the local TUI owns the
terminal — request logs scribbled over the alt screen.
lerko merged commit 92efb8e270 into main 2026-06-11 16:43:21 +00:00
lerko deleted branch fix/secret-masking-status-dto 2026-06-11 16:43:22 +00:00
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: lerkolabs/uptop#103