feat(cubeapi): add reusable outbound URL security layer for callbacks and webhooks

[Lcos-000]

Summary

This PR introduces a reusable outbound URL security primitive for CubeAPI. Its immediate purpose is to harden auth_callback_url against SSRF and DNS rebinding; its longer-term purpose is to serve as the security foundation for upcoming webhook and event-delivery features.

Background & Motivation

auth_callback_url allows CubeAPI to delegate authentication to an external HTTP endpoint. Today the URL is accepted without validation, so a misconfigured or malicious value can target internal services such as 127.0.0.1, 169.254.169.254, or private subnets. This is a classic SSRF surface.

Several Webhook PRs (e.g., lifecycle event delivery) are also in flight. They will share the same problem: how to safely dispatch HTTP requests to user-controlled URLs. Instead of baking ad-hoc checks into each feature, this PR provides a single, reviewable, testable security layer that any outbound-calling feature can reuse.

What This PR Does

• Adds CubeAPI/src/security/outbound_url.rs — a self-contained SSRF-protection module that covers:

  • URL parsing and scheme validation
  • Host validation (no missing host, no embedded credentials, explicit localhost block)
  • IPv4/IPv6 classification: loopback, private, link-local, multicast, documentation, unique-local, IPv4-mapped addresses, etc.
  • DNS resolution with timeout and address pinning via reqwest::ClientBuilder::resolve
  • Hardened reqwest::Client construction: no redirects, no proxy, explicit timeouts
  • Bounded response-body helper read_body_with_limit

• Adds OutboundUrlSecurityConfig to ServerConfig and loads it from environment variables.

• Integrates with auth_callback_url:

  • Validates the callback URL during AppState::new(); startup fails fast on policy violation
  • Builds a dedicated hardened client for callback dispatch
  • Auth middleware uses this client instead of a default reqwest::Client

• Adds user documentation:

  • docs/zh/guide/outbound-url-security.md
  • docs/guide/outbound-url-security.md

Threat Model & Key Protections

Threat	Protection
Loopback / localhost	Rejects 127.0.0.1, ::1, localhost, and IPv4-mapped loopback
Private subnets	Rejects 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 100.64.0.0/10, etc.
Link-local / metadata	Rejects 169.254.0.0/16 including 169.254.169.254
IPv6 non-public	Rejects unique-local, multicast, documentation, etc.
DNS rebinding	Resolves at validation time and pins addresses with reqwest::resolve
Scheme abuse	Defaults to https only; http must be explicitly allowed
Credential leaks	Rejects URLs containing user:pass@
Redirect hijacking	Hardened client disables automatic redirects
Proxy leakage	Hardened client disables HTTP proxy usage
OOM from huge responses	read_body_with_limit caps response body size

Design Decisions

1. Use url.host() instead of url.host_str() for IP detection
   host_str() returns IPv6 literals with square brackets (e.g., "[::ffff:127.0.0.1]"), which cannot be parsed directly as IpAddr. Using url.host() avoids this platform/version-sensitive parsing issue entirely.

2. Return a reqwest::Client, not a raw response
   The security layer validates the URL and builds a hardened client, then hands control back to the caller. This keeps the primitive focused and reusable for callbacks, webhooks, log shippers, or any other outbound caller.

3. Disable proxy by default
   In WSL test environments we observed that reqwest::Client::new() routes traffic through HTTP_PROXY, which can return 502 for local test servers and is undesirable for callback/webhook traffic. The hardened client explicitly calls .no_proxy().

4. Startup-time validation for auth_callback_url
   Failing at startup is safer than failing on the first request. It also makes misconfigurations obvious during deployment.

How Webhook Modules Can Reuse This

use cube_api::security::outbound_url::{OutboundUrlPolicy, build_secure_client};
use std::time::Duration;

let policy = OutboundUrlPolicy::webhook_default();
let validated = policy.validate("https://your-webhook.example.com/hook").await?;
let client = build_secure_client(&validated, Duration::from_secs(5), Duration::from_secs(10))?;

let response = client
    .post("https://your-webhook.example.com/hook")
    .json(&payload)
    .send()
    .await?;

webhook_default() uses the same strict defaults as production: https only and no private addresses.

Configuration

Environment variables use __ as the nested separator:

OUTBOUND_URL_SECURITY__ALLOWED_SCHEMES=https
OUTBOUND_URL_SECURITY__ALLOW_PRIVATE_IPS=false
OUTBOUND_URL_SECURITY__RESOLVE_TIMEOUT_MS=5000

For local development only:

OUTBOUND_URL_SECURITY__ALLOWED_SCHEMES=http,https
OUTBOUND_URL_SECURITY__ALLOW_PRIVATE_IPS=true

Testing

cargo fmt -- --check
cargo clippy --package cube-api --all-targets
cargo test --package cube-api

Results: 130 passed; 0 failed.

Additional deployment-level smoke tests were performed in WSL:

1. AUTH_CALLBACK_URL=http://127.0.0.1:9999/auth with default policy → CubeAPI exits with an SSRF policy error.
2. With OUTBOUND_URL_SECURITY__ALLOW_PRIVATE_IPS=true and a local Python HTTP server as callback → CubeAPI starts and the callback server receives the expected X-Request-Path, X-Request-Method, and X-API-Key headers.

Validation Examples

Rejected by default:

URL	Reason
http://example.com/hook	Scheme not allowed
https://127.0.0.1/hook	Loopback
https://localhost/hook	Forbidden host
https://10.0.0.1/hook	Private
https://169.254.169.254/hook	Link-local / metadata
https://[::ffff:127.0.0.1]/hook	IPv4-mapped loopback
https://user:pass@example.com/hook	Embedded credentials

Accepted by default:

URL	Reason
https://example.com/hook	Public domain → public IP
https://1.2.3.4/hook	Public IP literal
https://example.com:8443/hook	Custom port preserved

Checklist

• Code follows rustfmt style
• cargo clippy --package cube-api --all-targets passes
• Unit tests added and all existing tests pass
• User-facing documentation updated (EN + ZH)
• Deployment-level smoke test performed
• Real staging/deployment test (follow-up recommended)

Future Work

• Wire the hardened client into the HttpLogger webhook log backend stub.
• Build a full webhook subscription/delivery module on top of this security primitive.

Related to #642

[cubesandboxbot]

Suggestion: consider documenting the scope of try_parsing(true)

The diff adds .try_parsing(true) to the Environment source:

.add_source(
    config::Environment::default()
        .separator("__")
        .try_parsing(true)         // ← applied globally
        .list_separator(",")
        .with_list_parse_key("outbound_url_security.allowed_schemes"),
)

try_parsing(true) tells config to parse all env var values as typed values ("true" → bool, "5000" → number), not just for allowed_schemes. This is global to every key in ServerConfig.

In practice this works because serde handles both string and numeric representations. However, the global scope is worth an inline comment explaining the reason (presumably to make list_separator + with_list_parse_key work together). Without one, a future maintainer might wonder why try_parsing was turned on globally, or accidentally break it when removing it thinking it's unused.

No functional defect observed.

[cubesandboxbot]

Observation: read_body_with_limit and #[allow(dead_code)] — unreachable production code with a dependency cost

read_body_with_limit (and development(), webhook_default(), BodyLimitError::BodyTooLarge) are marked #[allow(dead_code)] because the only caller that would use them (webhook delivery) hasn't landed yet.

The functional impact is small, but it's worth noting that:

1. This ships ~50 lines of production code as dead code
2. The stream feature on reqwest (added in Cargo.toml) is pulled in solely for read_body_with_limit's bytes_stream() call — as visible in the Cargo.lock diff, it transitively adds tokio-util, wasm-streams, and futures-util under reqwest's dependency tree. These wouldn't be needed if the dead code were removed or cfg(test)-gated.

Consider gating with a feature flag (e.g. features = ["webhooks"]) rather than #[allow(dead_code)] on production items. Alternatively, the entire function and related types could be wrapped in #[cfg(feature = "webhooks")] for clarity about what's intended for future use vs. currently active.

[cubesandboxbot]

Review: PR #1092 — feat(cubeapi): add reusable outbound URL security layer for callbacks and webhooks

Review generated by AI. This is not a human review.

---

Overview

This PR adds a comprehensive SSRF-protection module (CubeAPI/src/security/outbound_url.rs) that validates outbound URLs, resolves and classifies their IP addresses, and builds a hardened reqwest::Client with DNS pinning, disabled redirects, and no proxy. It integrates the security layer with auth_callback_url, fails fast at startup on policy violations, and adds EN/ZH documentation.

The architecture is sound: a small public API surface (OutboundUrlSecurityConfig → OutboundUrlPolicy → validate() → ValidatedUrl → build_secure_client()), clean separation of concerns, and well-defined error types. The test suite is thorough, covering loopback, private, link-local, CGNAT, IPv4-mapped IPv6, documentation ranges, DNS timeouts, and edge cases like mixed public/private resolution.

I found three issues (one documentation, one correctness-adjacent, one code quality) and several minor observations.

---

Findings

1. [documentation] Inline doc comment uses wrong env var prefix

File: CubeAPI/src/security/outbound_url.rs, lines 303–305

The doc comment on OutboundUrlSecurityConfig shows:

/// CUBE_API_OUTBOUND_URL_SECURITY__ALLOWED_SCHEMES=https
/// CUBE_API_OUTBOUND_URL_SECURITY__ALLOW_PRIVATE_IPS=false
/// CUBE_API_OUTBOUND_URL_SECURITY__RESOLVE_TIMEOUT_MS=5000

But the actual environment variable prefix is OUTBOUND_URL_SECURITY__... (no CUBE_API_ prefix), because config::Environment::default() uses the root namespace with no prefix:

.add_source(
    config::Environment::default()
        .separator("__")
        .try_parsing(true)
        .list_separator(",")
        .with_list_parse_key("outbound_url_security.allowed_schemes"),
)

The user-facing docs in docs/guide/outbound-url-security.md use the correct prefix (OUTBOUND_URL_SECURITY__...). The inline rustdoc is inconsistent and would confuse any operator who reads the struct-level docs instead of the Markdown guide.

Fix: Remove the CUBE_API_ prefix from the inline doc comment to match reality.

2. [correctness] Misleading error variant when port_or_known_default() returns None

File: CubeAPI/src/security/outbound_url.rs, lines 498–499

let port = url
    .port_or_known_default()
    .ok_or_else(|| OutboundUrlError::UnsupportedScheme(url.scheme().to_string()))?;

If someone configures OUTBOUND_URL_SECURITY__ALLOWED_SCHEMES=ws and passes ws://example.com/hook (a scheme with no well-known default port in the url crate), the scheme passes validate_scheme() but then port_or_known_default() returns None. The error surfaced is UnsupportedScheme("ws"), which is misleading — the scheme was allowed, it just has no default port and the URL didn't explicitly specify one.

With the default schemes (http, https) this cannot trigger since both have well-known ports. But the allowed_schemes field is user-configurable, so an operator adding a non-standard scheme would receive a confusing error.

Suggestion: Consider either:

• (a) Add a dedicated #[error("URL has no port and scheme {0} has no default port")] variant, e.g. NoDefaultPort(String), or
• (b) Document that configured schemes must either have a known default port in the url crate or include an explicit port in the URL.

3. [code quality] Redundant Content-Length parsing in read_body_with_limit

File: CubeAPI/src/security/outbound_url.rs, lines 716–726

if let Some(len) = response
    .headers()
    .get(reqwest::header::CONTENT_LENGTH)
    .and_then(|v| v.to_str().ok())
    .and_then(|s| s.parse::<u64>().ok())
    .or(response.content_length())

response.content_length() (from the http::Response trait) already reads and parses the Content-Length header. The manual headers().get() → .to_str() → .parse() chain before the .or() fallback is dead code in practice: if the manual parsing fails (non-UTF-8 header value), content_length() also fails because it parses the header as UTF-8 bytes internally.

Fix: Simplify to:

if let Some(len) = response.content_length() {
    if len > max_bytes as u64 {
        return Err(BodyLimitError::BodyTooLarge);
    }
}

---

Minor Observations

1. localhost check only catches the exact string "localhost" — This is fine as a defense-in-depth measure. Other localhost-like names (localhost.localdomain, localhost6) will resolve via DNS and be caught by the IP-level checks in validate_ip. The comment on line 556 says "Reject localhost explicitly regardless of DNS behavior", which correctly documents the intent.

2. DNS pinning is performed at AppState::new() startup — If a callback URL's DNS record changes after startup (e.g. the upstream service migrates to a new IP), the pinned client still uses the originally resolved addresses. For auth_callback_url (a stable admin-configured endpoint), this is acceptable and is in fact the desired anti-rebinding behavior. Worth documenting in the rustdoc of build_secure_client that the client permanently pins the IP addresses resolved at construction time.

3. OutboundUrlPolicy::development() is gated behind #[cfg(feature = "webhooks")] — This means that without the webhooks feature, there is no obvious constructor for a relaxed policy. The auth-test helpers work around this by mutating fields on a strict() policy directly. If local-development usage without --features webhooks becomes common, consider either moving development() out of the feature gate or adding a development() that is always available.

4. webhooks feature not enabled by default — The feature flag adds reqwest/stream and its transitive dependencies (futures-util, tokio-util, wasm-streams), all of which show up in Cargo.lock regardless of whether the feature is enabled. This is a cosmetic concern only — the dependencies are pulled in by reqwest's feature resolution but only compiled when the feature is active. No action needed.

5. The OutboundUrlSecurityConfig inline doc quotes CUBE_API_ prefix (finding Welcome to CubeSandbox Discussions! #1 above — noted again for emphasis).

6. Tests comprehensively cover the IP classification matrix. The IPv4 tests cover loopback, private (3 ranges), link-local, CGNAT, IETF protocol assignments, this-network, and broadcast. IPv6 tests cover loopback, unique-local, multicast, IPv4-mapped variants, and documentation. This is thorough and commendable.

---

Integration Assessment

Config: The try_parsing(true) + list_separator(",") + with_list_parse_key("outbound_url_security.allowed_schemes") additions to the environment source handle typed parsing correctly. The ServerConfig::Default implementation initializes outbound_url_security to OutboundUrlSecurityConfig::default().

State: AppState::new() now returns anyhow::Result<Self> instead of Self. All callers (main.rs, auth tests, routes tests) are updated with .await? or .unwrap() as appropriate. The auth callback tests correctly relax the policy to allow http://127.0.0.1 URLs.

Auth middleware: Uses state.auth_callback_client.as_ref().unwrap_or(&state.http_client) — correct fallback when no callback is configured. The hardened client's DNS pinning on the original hostname means the .post(&callback_url) call in the middleware uses the pinned IPs transparently.

Build: The url crate is added as a direct dependency. The Cargo.lock diff shows all transitive dependencies resolving cleanly.

---

Verdict

The code is well-structured, the threat model is clearly articulated, and the IP classification logic is correct for the standard non-public ranges. The three issues identified (doc comment prefix, misleading error variant for missing default port, redundant Content-Length parsing) are all low severity and straightforward to fix.

130 tests pass (as reported by the author); the test additions in this PR cover the IP matrix thoroughly. No functional bugs were found in the core validation logic.

[cubesandboxbot]

Doc comment uses wrong env var prefix

The inline rustdoc shows CUBE_API_OUTBOUND_URL_SECURITY__ALLOWED_SCHEMES etc., but the config builder in config/mod.rs uses config::Environment::default() with no prefix:

.add_source(
    config::Environment::default()
        .separator("__")
        .try_parsing(true)
        .list_separator(",")
        .with_list_parse_key("outbound_url_security.allowed_schemes"),
)

So the actual env var is just OUTBOUND_URL_SECURITY__ALLOWED_SCHEMES (no CUBE_API_). The Markdown docs (docs/guide/outbound-url-security.md) use the correct prefix. Update this doc comment to match reality.

[cubesandboxbot]

Misleading error variant when port_or_known_default() returns None

If an operator configures ALLOWED_SCHEMES with a scheme that has no well-known default port in the url crate (e.g. "ws") and passes a URL without an explicit port, the error UnsupportedScheme(scheme) is misleading — the scheme was allowed, it just lacks a known default port.

Consider adding a dedicated error variant like NoDefaultPort(String), or document that user-configured schemes must have either a known default port or an explicit :port in the URL.

[cubesandboxbot]

Redundant Content-Length parsing

response.content_length() (from http::Response) already reads and parses the Content-Length header, so the manual chain (headers().get() → .to_str() → .parse()) is dead code. In practice, if the manual parsing fails (e.g. non-UTF-8 header value), content_length() also fails because it parses the header as UTF-8 bytes internally.

Can be simplified to:

if let Some(len) = response.content_length() {
    if len > max_bytes as u64 {
        return Err(BodyLimitError::BodyTooLarge);
    }
}