Skip to content

fix(uptime): allow RFC 1918 IPs for admin-configured monitors#855

Merged
Wikid82 merged 13 commits intodevelopmentfrom
feature/beta-release
Mar 18, 2026
Merged

fix(uptime): allow RFC 1918 IPs for admin-configured monitors#855
Wikid82 merged 13 commits intodevelopmentfrom
feature/beta-release

Conversation

@Wikid82
Copy link
Owner

@Wikid82 Wikid82 commented Mar 17, 2026

Problem

HTTP/HTTPS uptime monitors targeting LAN addresses (192.168.x.x, 10.x.x.x, 172.16.x.x) permanently reported "down" on fresh installs. The root cause: SSRF protection operates at two independent checkpoints — the URL validator (DNS-resolution phase) and the safe dialer (TCP-connect phase). Fixing only one layer still blocks the connection at the other.

Fixes issues 6 and 7 from the fresh-install bug report.

Solution

Add a WithAllowRFC1918() functional option to both protection layers and pass it from uptime_service.go to both simultaneously. The option is off by default — no other call site is affected.

RFC 1918 scope (strict)

Only the three canonical private ranges are permitted when the option is active:

  • 10.0.0.0/8
  • 172.16.0.0/12
  • 192.168.0.0/16

Still unconditionally blocked:

  • 169.254.0.0/16 — link-local / cloud metadata (including 169.254.169.254)
  • Loopback (127.0.0.0/8) — unless WithAllowLocalhost() is also set
  • All other private/reserved ranges

Changes

File Change
backend/internal/network/safeclient.go IsRFC1918() predicate; AllowRFC1918 field + WithAllowRFC1918() option; safeDialer bypass in both validation and selection loops
backend/internal/security/url_validator.go AllowRFC1918 field + WithAllowRFC1918() option; Phase 4 private-IP loop bypass for both IPv4-mapped and plain IPv6 addresses
backend/internal/services/uptime_service.go WithAllowRFC1918() passed to both ValidateExternalURL and NewSafeHTTPClient with a coordinating comment

Tests

21 new tests across 3 packages (92.1% / 94.1% / 86.0% coverage):

  • TestIsRFC1918_* — comprehensive predicate coverage including boundary addresses, nil input, IPv4-mapped IPv6
  • TestSafeDialer_AllowRFC1918_* — direct dial to 192.168.1.1, metadata still blocked, loopback still blocked
  • TestValidateExternalURL_WithAllowRFC1918_* — all three ranges permitted; metadata and loopback still rejected
  • TestValidateExternalURL_RFC1918BlockedByDefaultregression guard: confirms RFC 1918 is blocked when option is absent
  • TestCheckMonitor_* — uptime service integration tests

Security

  • 169.254.169.254 protection preserved — falls through RFC 1918 check (not in CIDRs) and hits the cloud-metadata error path unchanged
  • IsRFC1918(nil) returns false — nil-safe
  • All 7 other ValidateExternalURL call sites are untouched
  • AllowRFC1918 defaults to false — no behaviour change without explicit opt-in
  • The bypass requires a valid admin session (RequireManagementAccess()) to configure a monitor

renovate bot and others added 4 commits March 17, 2026 17:17
…n-major-updates

chore(deps): update release-drafter/release-drafter digest to 44a942e (feature/beta-release)
HTTP/HTTPS uptime monitors targeting LAN addresses (192.168.x.x,
10.x.x.x, 172.16.x.x) permanently reported 'down' on fresh installs
because SSRF protection rejects RFC 1918 ranges at two independent
checkpoints: the URL validator (DNS-resolution layer) and the safe
dialer (TCP-connect layer). Fixing only one layer leaves the monitor
broken in practice.

- Add IsRFC1918() predicate to the network package covering only the
  three RFC 1918 CIDRs; 169.254.x.x (link-local / cloud metadata)
  and loopback are intentionally excluded
- Add WithAllowRFC1918() functional option to both SafeHTTPClient and
  ValidationConfig; option defaults to false so existing behaviour is
  unchanged for every call site except uptime monitors
- In uptime_service.go, pass WithAllowRFC1918() to both
  ValidateExternalURL and NewSafeHTTPClient together; a coordinating
  comment documents that both layers must be relaxed as a unit
- 169.254.169.254 and the full 169.254.0.0/16 link-local range remain
  unconditionally blocked; the cloud-metadata error path is preserved
- 21 new tests across three packages, including an explicit regression
  guard that confirms RFC 1918 blocks are still applied without the
  option set (TestValidateExternalURL_RFC1918BlockedByDefault)

Fixes issues 6 and 7 from the fresh-install bug report.
@github-advanced-security
Copy link
Contributor

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

  • The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
  • Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
  • You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 17, 2026

✅ Supply Chain Verification Results

PASSED

📦 SBOM Summary

  • Components: 1483

🔍 Vulnerability Scan

Severity Count
🔴 Critical 0
🟠 High 0
🟡 Medium 4
🟢 Low 2
Total 8

📎 Artifacts

  • SBOM (CycloneDX JSON) and Grype results available in workflow artifacts

Generated by Supply Chain Verification workflow • View Details

@Wikid82 Wikid82 self-assigned this Mar 17, 2026
@Wikid82 Wikid82 added this to Charon Mar 17, 2026
@github-project-automation github-project-automation bot moved this to Backlog in Charon Mar 17, 2026
@codecov
Copy link

codecov bot commented Mar 17, 2026

Codecov Report

❌ Patch coverage is 91.48936% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
backend/internal/network/safeclient.go 92.30% 1 Missing and 1 partial ⚠️
backend/internal/security/url_validator.go 80.00% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@Wikid82 Wikid82 marked this pull request as ready for review March 18, 2026 02:34
Copilot AI review requested due to automatic review settings March 18, 2026 02:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Enables admin-configured uptime monitors to target RFC 1918 private IP ranges by adding an explicit opt-in (WithAllowRFC1918) to both SSRF enforcement layers (URL validation + dial-time guard), plus accompanying unit/integration tests and documentation.

Changes:

  • Add AllowRFC1918 + WithAllowRFC1918() to both internal/security URL validation and internal/network safe dialer logic (with shared network.IsRFC1918 predicate).
  • Wire the option through UptimeService.checkMonitor for HTTP/HTTPS monitors (dual-layer relaxation).
  • Add tests for RFC 1918 behavior across network/security/services and update planning/QA docs.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
backend/internal/network/safeclient.go Adds IsRFC1918, option flag, and safe dialer bypass/selection behavior for RFC 1918.
backend/internal/security/url_validator.go Adds AllowRFC1918 config + option and skips private-IP blocking for RFC 1918 when enabled (including IPv4-mapped IPv6 handling).
backend/internal/services/uptime_service.go Passes WithAllowRFC1918() to both validator and safe HTTP client; documents rationale.
backend/internal/network/safeclient_test.go Adds RFC 1918 predicate and safe dialer/client tests.
backend/internal/security/url_validator_test.go Adds RFC 1918 validation option tests (including mapped IPv6 cases).
backend/internal/services/uptime_service_test.go Adds integration tests for monitor checks.
docs/plans/current_spec.md Documents the implementation plan, invariants, and test plan.
docs/reports/qa_report_pr3.md Adds QA/security report for PR-3 verification steps and outcomes.
.github/workflows/auto-changelog.yml Updates pinned release-drafter action SHA.

You can also share your feedback on Copilot code review. Take the survey.

Wikid82 and others added 7 commits March 17, 2026 23:15
…n-major-updates

chore(deps): update module github.com/greenpau/caddy-security to v1.1.49 (feature/beta-release)
…fore error reporting

- IPv4-mapped cloud metadata (::ffff:169.254.169.254) previously fell through
  the IPv4-mapped IPv6 detection block and returned the generic private-IP error
  instead of the cloud-metadata error, making the two cases inconsistent
- The IPv4-mapped error path used ip.String() (the raw ::ffff:… form) directly
  rather than sanitizeIPForError, potentially leaking the unsanitized IPv6
  address in error messages visible to callers
- Now extracts the IPv4 from the mapped address before both the cloud-metadata
  comparison and the sanitization call, so ::ffff:169.254.169.254 produces the
  same "access to cloud metadata endpoints is blocked" error as 169.254.169.254
  and the error message is always sanitized through the shared helper
- Updated the corresponding test to assert the cloud-metadata message and the
  absence of the raw IPv6 representation in the error text
…message

The settings handler SSRF test table expected the generic "private ip"
error string for the cloud-metadata case (169.254.169.254). After the
url_validator was updated to return a distinct "cloud metadata" error for
that address, the handler test's errorContains check failed on every CI run.

Updated the test case expectation from "private" to "cloud metadata" to
match the more precise error message now produced by the validator.
@Wikid82 Wikid82 merged commit 0314574 into development Mar 18, 2026
38 checks passed
@github-project-automation github-project-automation bot moved this from Backlog to Done in Charon Mar 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants