fix(security): harden OAuth login, server-side SSRF, audit logging, and agent token#133
Merged
Conversation
Move the agent's SSRF guard into serverbee_common::ssrf (single source of truth) and apply it to the server-side service-monitor checkers. A new is_monitor_safe_addr / resolve_and_check_monitor blocks loopback, link-local (incl. cloud metadata 169.254.169.254) and NAT64 while allowing RFC1918 private ranges so legitimate internal monitoring keeps working. HTTP/TCP/SSL checkers connect only to validated addresses (closing DNS-rebind); the DNS checker rejects metadata/loopback nameservers.
The SSRF guard restricted HTTP keyword checks to ports 80/443, which broke monitoring of services on non-standard ports (e.g. :8080, :3000) — a common internal-monitoring case the address-level guard already supports. Add validate_monitor_url (any port; scheme + embedded-credentials checks retained) and use it in the HTTP keyword checker. is_monitor_safe_addr still blocks loopback/link-local/metadata regardless of port. The strict validate_url (80/443 only) stays in use on the agent's global-only IP-quality path.
restore_backup renames the live DB to .pre-restore and moves the uploaded file onto db_path, but state.db's pooled connections still hold the pre-restore inode. Writing the settings.restore audit row through state.db therefore lands it in the discarded database, which is lost after the mandatory restart — exactly the record that matters most. Open a short-lived connection to the freshly restored file so the entry persists into the DB the server reopens, and emit a tracing warning as a durable fallback.
The HTTP keyword checker built a single reqwest client pinned to the original host via resolve_to_addrs but left reqwest's default redirect policy, which auto-follows up to 10 hops. A monitored endpoint could 3xx-redirect to 169.254.169.254 or an internal host and bypass the SSRF guard entirely. Disable auto-redirect and follow manually, re-running validate_monitor_url + resolve_and_check_monitor and re-pinning the client on every hop (mirrors the agent ip_quality fetcher and widget_module). Redirects are followed as GET without the body or custom headers so neither is replayed to a redirected host.
The SSRF guard added to the tcp/ssl/http_keyword checkers silently flips any pre-existing loopback/localhost monitor from passing to failing on upgrade and can page the notification group after two checker cycles, since targets were never validated at config time. Reject targets whose literal host is loopback, link-local, the cloud-metadata endpoint, or "localhost" in ServiceMonitorService::create/update with a 422, so the restriction surfaces at configuration time instead of as runtime false-alerts. RFC1918/ULA private ranges stay allowed for internal monitoring; dns (nameserver-only guard) and whois are not address-checked. Document the restriction and update the service-monitor integration test to assert the create-time rejection.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Security-hardening pass for the auth, service-monitor, and admin surfaces. Findings came from a multi-role audit plus a regression-focused review; each fix is committed independently.
Auth
Debug.Authorizationheader over the?token=query string (CWE-598), keeping the query param as a fallback for proxies that strip the header.Server-side SSRF (service-monitor checkers)
serverbee_common::ssrfmodule. Block loopback / link-local / cloud-metadata (169.254.169.254) / NAT64 and IPv4-mapped forms, while still allowing RFC1918/ULA private ranges for legitimate internal monitoring. Connect only to the validated addresses to close the DNS-rebinding window.validate_monitor_url) so non-standard service ports keep working; the strict 80/443 validator stays on the agent's global-only path.Admin surface
Test Plan
cargo test -p serverbee-common— 132 passedcargo test -p serverbee-server --lib— 571 passedcargo test -p serverbee-server --test integration— 66 passedcargo clippy --workspace -- -D warnings— 0 warnings