fix: Improve security validations for RPC urls #605
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR adds SSRF protection for custom RPC URLs by introducing allowed hosts and private IP blocking configuration, implementing URL validation logic with dangerous hostname and IP checks, and integrating these checks into RPC provider initialization across EVM, Solana, and Stellar chains. Additionally, it makes fee fields optional in swap information structures. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ProviderService as Provider Service<br/>(EVM/Solana/Stellar)
participant Validator as URL Validator<br/>(validate_rpc_url)
participant SecurityLayer as Security Checks
participant HTTPClient as HTTP Client
Client->>ProviderService: initialize_provider(rpc_url)
ProviderService->>Validator: validate_rpc_url(url, allowed_hosts, block_private)
Validator->>SecurityLayer: Parse & validate scheme
SecurityLayer-->>Validator: ✓ http/https only
Validator->>SecurityLayer: Check allowed hosts
SecurityLayer-->>Validator: ✓ or ✗ (if allow-list exists)
Validator->>SecurityLayer: Block dangerous hostnames
SecurityLayer-->>Validator: ✓ (reject localhost, metadata.*)
Validator->>SecurityLayer: Validate IP address
SecurityLayer-->>Validator: ✓ (reject private/loopback/metadata IPs)
alt Validation Success
Validator-->>ProviderService: Ok(())
ProviderService->>HTTPClient: Create client with no-redirect policy
HTTPClient-->>ProviderService: ✓ Client ready
ProviderService-->>Client: ✓ Provider initialized
else Validation Failure
Validator-->>ProviderService: Err(ValidationMessage)
ProviderService-->>Client: ✗ NetworkConfiguration error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/provider/stellar/mod.rs (1)
369-398:initialize_raw_provider()lacks URL validation and both functions leak credentials in error messages.
initialize_raw_provider()is called with untrusted URLs (line 470) but does not validate them. Add the samevalidate_rpc_url()call as ininitialize_provider()to apply consistent security controls across all outbound paths.Both
initialize_provider()(line 378) andinitialize_raw_provider()(line 394) embed the rawurlin error messages, which can leak credentials, API keys, or query parameters into logs and error responses. Replace withnormalize_url_for_log(url)as already done inretry_raw_request()(line 453).Proposed patch
fn initialize_provider(&self, url: &str) -> Result<Client, ProviderError> { // Layer 2 validation: Re-validate URL security as a safety net let allowed_hosts = crate::config::ServerConfig::get_allowed_rpc_hosts(); let block_private_ips = crate::config::ServerConfig::get_block_private_ips(); crate::utils::validate_rpc_url(url, &allowed_hosts, block_private_ips).map_err(|e| { ProviderError::NetworkConfiguration(format!("RPC URL security validation failed: {e}")) })?; + let url_for_log = normalize_url_for_log(url); Client::new(url).map_err(|e| { ProviderError::NetworkConfiguration(format!( - "Failed to create Stellar RPC client: {e} - URL: '{url}'" + "Failed to create Stellar RPC client: {e} - URL: '{url_for_log}'" )) }) } fn initialize_raw_provider(&self, url: &str) -> Result<ReqwestClient, ProviderError> { + // Layer 2 validation: ensure the raw HTTP path is equally protected + let allowed_hosts = crate::config::ServerConfig::get_allowed_rpc_hosts(); + let block_private_ips = crate::config::ServerConfig::get_block_private_ips(); + crate::utils::validate_rpc_url(url, &allowed_hosts, block_private_ips).map_err(|e| { + ProviderError::NetworkConfiguration(format!("RPC URL security validation failed: {e}")) + })?; + + let url_for_log = normalize_url_for_log(url); ReqwestClient::builder() .timeout(self.timeout_seconds) .use_rustls_tls() .redirect(reqwest::redirect::Policy::none()) // Prevent SSRF via redirect chains .build() .map_err(|e| { ProviderError::NetworkConfiguration(format!( - "Failed to create HTTP client for raw RPC: {e} - URL: '{url}'" + "Failed to create HTTP client for raw RPC: {e} - URL: '{url_for_log}'" )) }) }
🤖 Fix all issues with AI agents
In @src/config/config_file/network/common.rs:
- Line 12: NetworkConfigCommon::validate() currently reads ALLOWED_RPC_HOSTS and
BLOCK_PRIVATE_IPS from the environment which makes tests flaky; update validate
to take these settings as parameters (e.g., add params allowed_rpc_hosts:
Option<String> and block_private_ips: Option<bool> or accept a &ServerConfig)
and use those values instead of env::var inside NetworkConfigCommon::validate(),
then update call sites to pass ServerConfig-derived values (or test-supplied
values) so tests can inject deterministic values; alternatively, if changing
signature is infeasible, add a small helper function that accepts overrides and
let tests call validate_with_overrides(...) while production still calls the
env-driven validate().
In @src/models/relayer/mod.rs:
- Around line 1153-1158: The error message includes the raw RPC URL (config.url)
which may leak secrets; update the map_err closure on the validate_rpc_url call
to use crate::utils::sanitize_url(&config.url) instead of config.url when
building RelayerValidationError::InvalidRpcUrl so the message shows a sanitized
URL; keep the existing error string (err) concatenation and leave
validate_rpc_url and RelayerValidationError::InvalidRpcUrl unchanged otherwise.
In @src/services/provider/solana/mod.rs:
- Around line 563-570: Add a prominent module-level security note in
src/services/provider/solana/mod.rs (near the top-level docs) stating that
unlike EVM/Stellar the Solana provider cannot disable HTTP redirects because
RpcClient::new_with_timeout_and_commitment does not expose HTTP client config,
and that redirect-based SSRF remains possible even with URL validation—advise
using a strict ALLOWED_RPC_HOSTS list in production. Also add a short TODO
mentioning tracking an issue to add solana-rpc-client as a direct dependency and
switch to HttpSender::new_with_client with a custom reqwest::Client configured
with Policy::none() to fully mitigate redirects.
In @src/utils/url_security.rs:
- Around line 50-58: The allow-list check is doing a case-sensitive equality
(allowed == host) which fails for differently-cased DNS names; update the
comparison to be case-insensitive by using Rust's eq_ignore_ascii_case (e.g.,
replace any(|allowed| allowed == host) with any(|allowed|
allowed.eq_ignore_ascii_case(host))) so allowed_hosts is matched against host in
a case-insensitive manner.
- Around line 60-68: The current logic only rejects dangerous hostnames when
block_private is true, but metadata IPs are always blocked via
is_metadata_endpoint; change this so cloud metadata hostnames are always
blocked: extract is_dangerous_hostname into two predicates (e.g.,
is_cloud_metadata_hostname(host) and is_local_or_internal_hostname(host)), call
is_cloud_metadata_hostname(host) unconditionally and return the same error, and
keep the block_private conditional for is_local_or_internal_hostname(host);
update references to is_dangerous_hostname in the URL-checking flow and ensure
is_metadata_endpoint() remains an unconditional IP check.
🧹 Nitpick comments (11)
src/config/server_config.rs (1)
300-311: Add test coverage for the new getter method.The parsing logic correctly handles comma-separated hosts, trims whitespace, and filters empty strings. However, there are no tests covering this new functionality.
Consider adding test cases to verify:
- Empty string handling
- Whitespace trimming
- Comma-separated list parsing
- Multiple consecutive commas
- Trailing/leading commas
✅ Example test case
Add this test to the existing test module:
#[test] fn test_get_allowed_rpc_hosts() { let _lock = match ENV_MUTEX.lock() { Ok(guard) => guard, Err(poisoned) => poisoned.into_inner(), }; // Test empty/not set env::remove_var("ALLOWED_RPC_HOSTS"); assert_eq!(ServerConfig::get_allowed_rpc_hosts(), Vec::<String>::new()); // Test single host env::set_var("ALLOWED_RPC_HOSTS", "eth-mainnet.g.alchemy.com"); assert_eq!( ServerConfig::get_allowed_rpc_hosts(), vec!["eth-mainnet.g.alchemy.com".to_string()] ); // Test multiple hosts with whitespace env::set_var("ALLOWED_RPC_HOSTS", "host1.com, host2.com , host3.com"); assert_eq!( ServerConfig::get_allowed_rpc_hosts(), vec!["host1.com".to_string(), "host2.com".to_string(), "host3.com".to_string()] ); // Test empty strings filtered env::set_var("ALLOWED_RPC_HOSTS", "host1.com,,host2.com,"); assert_eq!( ServerConfig::get_allowed_rpc_hosts(), vec!["host1.com".to_string(), "host2.com".to_string()] ); env::remove_var("ALLOWED_RPC_HOSTS"); } #[test] fn test_get_block_private_ips() { let _lock = match ENV_MUTEX.lock() { Ok(guard) => guard, Err(poisoned) => poisoned.into_inner(), }; // Test default (not set) env::remove_var("BLOCK_PRIVATE_IPS"); assert!(!ServerConfig::get_block_private_ips()); // Test true env::set_var("BLOCK_PRIVATE_IPS", "true"); assert!(ServerConfig::get_block_private_ips()); // Test false env::set_var("BLOCK_PRIVATE_IPS", "false"); assert!(!ServerConfig::get_block_private_ips()); // Test case insensitive env::set_var("BLOCK_PRIVATE_IPS", "TRUE"); assert!(ServerConfig::get_block_private_ips()); // Test invalid value defaults to false env::set_var("BLOCK_PRIVATE_IPS", "invalid"); assert!(!ServerConfig::get_block_private_ips()); env::remove_var("BLOCK_PRIVATE_IPS"); }src/services/provider/evm/mod.rs (1)
188-194: Consider adding tests for SSRF protection.The new security validation logic (URL validation and redirect prevention) lacks specific test coverage. Consider adding tests that verify:
- Blocked hostnames are rejected (e.g., cloud metadata endpoints)
- Private IPs are blocked when
BLOCK_PRIVATE_IPS=true- Allow-list enforcement when
ALLOWED_RPC_HOSTSis set- Redirect policy prevents following redirects
Example test structure
#[test] fn test_initialize_provider_blocks_metadata_endpoint() { let _env_guard = setup_test_env(); std::env::set_var("BLOCK_PRIVATE_IPS", "true"); let provider = EvmProvider::new( vec![RpcConfig::new("http://localhost:8545".to_string())], 30, ).unwrap(); let result = provider.initialize_provider("http://169.254.169.254/latest/meta-data/"); assert!(result.is_err()); assert!(result.unwrap_err().to_string().contains("security validation failed")); std::env::remove_var("BLOCK_PRIVATE_IPS"); } #[test] fn test_initialize_provider_enforces_allowed_hosts() { let _env_guard = setup_test_env(); std::env::set_var("ALLOWED_RPC_HOSTS", "mainnet.infura.io"); let provider = EvmProvider::new( vec![RpcConfig::new("http://localhost:8545".to_string())], 30, ).unwrap(); // Should fail - not in allow-list let result = provider.initialize_provider("https://evil.com"); assert!(result.is_err()); std::env::remove_var("ALLOWED_RPC_HOSTS"); }Also applies to: 203-203
src/services/provider/solana/mod.rs (1)
572-580: Add test coverage for security validation.Similar to the EVM provider, the Solana provider needs tests for the SSRF protection logic. The existing
test_initialize_provider_invalid_url(lines 1320-1336) only tests URL format validation.src/models/relayer/mod.rs (1)
1153-1158: Enhance test coverage for SSRF protection.The existing test
test_relayer_validation_invalid_rpc_url(lines 2304-2323) only validates URL format. Add tests for the security validation:Example test cases
#[test] fn test_relayer_validation_rpc_url_blocks_metadata_endpoint() { std::env::set_var("BLOCK_PRIVATE_IPS", "true"); let relayer = Relayer::new( "valid-id".to_string(), "Valid Relayer".to_string(), "mainnet".to_string(), false, RelayerNetworkType::Evm, None, "valid-signer".to_string(), None, Some(vec![RpcConfig::new("http://169.254.169.254/".to_string())]), ); let result = relayer.validate(); assert!(result.is_err()); assert!(matches!(result.unwrap_err(), RelayerValidationError::InvalidRpcUrl(_))); std::env::remove_var("BLOCK_PRIVATE_IPS"); } #[test] fn test_relayer_validation_rpc_url_allows_valid_host() { std::env::set_var("ALLOWED_RPC_HOSTS", "mainnet.infura.io"); let relayer = Relayer::new( "valid-id".to_string(), "Valid Relayer".to_string(), "mainnet".to_string(), false, RelayerNetworkType::Evm, None, "valid-signer".to_string(), None, Some(vec![RpcConfig::new("https://mainnet.infura.io".to_string())]), ); assert!(relayer.validate().is_ok()); std::env::remove_var("ALLOWED_RPC_HOSTS"); }src/domain/relayer/solana/solana_relayer.rs (1)
1517-1518: Tests correctly updated forSwapInfo.fee_*: Option<String>The
Some(...)wrapping matches the new public model shape; looks consistent across both Jupiter Swap and Jupiter Ultra fixtures. Consider adding one fixture withfee_amount: None/fee_mint: Noneto ensure downstream code stays resilient to omitted fields.Also applies to: 1680-1682
src/domain/relayer/solana/rpc/methods/prepare_transaction.rs (1)
482-486: Test fixtures aligned with optional fee fieldsGood update to
fee_amount/fee_mintasOption<String>. I’d add a test variant where Jupiter returnsNonefor these fields (or omit them in JSON) to validate serde/default handling and any fee computation assumptions.src/config/config_file/network/common.rs (1)
258-270: Good coverage for rejecting non-HTTP schemesThis test matches the new SSRF posture (http/https only). If you keep env-driven validation, consider ensuring the test is robust when env vars are present (e.g., explicitly clear allowlist/block-private here).
src/domain/relayer/solana/rpc/methods/sign_and_send_transaction.rs (1)
454-458: Test fixtures correctly migrated to optional fee fieldsThe
Some(...)updates look consistent. Consider adding a test where the quote route plan omitsfee_amount/fee_mintto ensure no hidden unwrap/assumption exists in fee conversion logic.Also applies to: 630-634, 809-813
src/domain/relayer/solana/rpc/methods/fee_estimate.rs (1)
299-302: Tests aligned withSwapInfo.fee_*: Option<String>The fixture changes are consistent. I’d add one coverage point for
Nonefee fields to confirm fee estimation isn’t accidentally dependent on these being present.Also applies to: 468-472, 573-577, 673-676
src/domain/relayer/solana/dex/jupiter_ultra.rs (1)
160-162: Ultra test fixture correctly wraps fee fields inSome(...)Matches the updated public model. As a follow-up, a
None-fee fixture would help validate parsing/handling robustness.src/services/jupiter/mod.rs (1)
417-418: Consider usingNonein mocks where “fee fields absent” is the behavior you’re accommodating.
Right now mocks always populateSome("0")/Some("mock_fee_mint"), which won’t exercise the “missing fee fields” path that motivatedOption<>.Also applies to: 477-478, 684-685, 755-756
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
.env.exampleREADME.mdsrc/config/config_file/network/common.rssrc/config/server_config.rssrc/domain/relayer/solana/dex/jupiter_swap.rssrc/domain/relayer/solana/dex/jupiter_ultra.rssrc/domain/relayer/solana/rpc/methods/fee_estimate.rssrc/domain/relayer/solana/rpc/methods/prepare_transaction.rssrc/domain/relayer/solana/rpc/methods/sign_and_send_transaction.rssrc/domain/relayer/solana/rpc/methods/sign_transaction.rssrc/domain/relayer/solana/rpc/methods/transfer_transaction.rssrc/domain/relayer/solana/rpc/methods/utils.rssrc/domain/relayer/solana/solana_relayer.rssrc/models/relayer/mod.rssrc/services/jupiter/mod.rssrc/services/provider/evm/mod.rssrc/services/provider/solana/mod.rssrc/services/provider/stellar/mod.rssrc/utils/mocks.rssrc/utils/mod.rssrc/utils/url_security.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (.cursor/rules/rust_standards.mdc)
**/*.rs: Follow official Rust style guidelines using rustfmt (edition = 2021, max_width = 100)
Follow Rust naming conventions: snake_case for functions/variables, PascalCase for types, SCREAMING_SNAKE_CASE for constants and statics
Order imports alphabetically and group by: std, external crates, local crates
Include relevant doc comments (///) on public functions, structs, and modules. Use comments for 'why', not 'what'. Avoid redundant doc comments
Document lifetime parameters when they're not obvious
Avoid unsafe code unless absolutely necessary; justify its use in comments
Write idiomatic Rust: Prefer Result over panic, use ? operator for error propagation
Avoid unwrap; handle errors explicitly with Result and custom Error types for all async operations
Prefer header imports over function-level imports of dependencies
Prefer borrowing over cloning when possible
Use &str instead of &String for function parameters
Use &[T] instead of &Vec for function parameters
Avoid unnecessary .clone() calls
Use Vec::with_capacity() when size is known in advance
Use explicit lifetimes only when necessary; infer where possible
Always use Tokio runtime for async code. Await futures eagerly; avoid blocking calls in async contexts
Streams: Use futures::StreamExt for processing streams efficiently
Always use serde for JSON serialization/deserialization with #[derive(Serialize, Deserialize)]
Use type aliases for complex types to improve readability
Implement common traits (Debug, Clone, PartialEq) where appropriate, using derive macros when possible
Implement Display for user-facing types
Prefer defining traits when implementing services to make mocking and testing easier
For tests, prefer existing utils for creating mocks instead of duplicating code
Use assert_eq! and assert! macros appropriately
Keep tests minimal, deterministic, and fast
Use tracing for structured logging, e.g., tracing::info! for request and error logs
When optimizing, prefer clarity first, then performance
M...
Files:
src/services/provider/solana/mod.rssrc/utils/mocks.rssrc/config/config_file/network/common.rssrc/services/provider/evm/mod.rssrc/domain/relayer/solana/rpc/methods/transfer_transaction.rssrc/utils/mod.rssrc/config/server_config.rssrc/services/jupiter/mod.rssrc/utils/url_security.rssrc/domain/relayer/solana/rpc/methods/prepare_transaction.rssrc/domain/relayer/solana/rpc/methods/sign_and_send_transaction.rssrc/models/relayer/mod.rssrc/domain/relayer/solana/solana_relayer.rssrc/domain/relayer/solana/rpc/methods/utils.rssrc/domain/relayer/solana/rpc/methods/fee_estimate.rssrc/domain/relayer/solana/dex/jupiter_swap.rssrc/services/provider/stellar/mod.rssrc/domain/relayer/solana/rpc/methods/sign_transaction.rssrc/domain/relayer/solana/dex/jupiter_ultra.rs
🧠 Learnings (2)
📚 Learning: 2025-12-02T11:22:31.387Z
Learnt from: CR
Repo: OpenZeppelin/openzeppelin-relayer PR: 0
File: .cursor/rules/rust_standards.mdc:0-0
Timestamp: 2025-12-02T11:22:31.387Z
Learning: Applies to **/*.rs : For tests, prefer existing utils for creating mocks instead of duplicating code
Applied to files:
src/utils/mod.rs
📚 Learning: 2025-12-02T11:22:31.387Z
Learnt from: CR
Repo: OpenZeppelin/openzeppelin-relayer PR: 0
File: .cursor/rules/rust_standards.mdc:0-0
Timestamp: 2025-12-02T11:22:31.387Z
Learning: Applies to **/*.rs : Avoid unsafe code unless absolutely necessary; justify its use in comments
Applied to files:
src/utils/url_security.rs
🧬 Code graph analysis (5)
src/config/config_file/network/common.rs (3)
src/config/server_config.rs (2)
get_allowed_rpc_hosts(301-311)get_block_private_ips(314-318)src/utils/url_security.rs (1)
validate_rpc_url(24-84)src/config/config_file/network/test_utils.rs (1)
create_network_common(17-27)
src/services/provider/evm/mod.rs (2)
src/config/server_config.rs (2)
get_allowed_rpc_hosts(301-311)get_block_private_ips(314-318)src/utils/url_security.rs (1)
validate_rpc_url(24-84)
src/config/server_config.rs (2)
src/services/provider/stellar/mod.rs (1)
s(139-139)src/utils/url_security.rs (1)
host(71-71)
src/models/relayer/mod.rs (2)
src/config/server_config.rs (2)
get_allowed_rpc_hosts(301-311)get_block_private_ips(314-318)src/utils/url_security.rs (1)
validate_rpc_url(24-84)
src/services/provider/stellar/mod.rs (2)
src/config/server_config.rs (2)
get_allowed_rpc_hosts(301-311)get_block_private_ips(314-318)src/utils/url_security.rs (1)
validate_rpc_url(24-84)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Properties Tests
- GitHub Check: clippy
- GitHub Check: msrv
- GitHub Check: Unit Tests
- GitHub Check: Analyze (rust)
🔇 Additional comments (23)
.env.example (1)
17-28: LGTM! Clear SSRF protection documentation.The documentation clearly explains the new security features with helpful examples and default values. The note about cloud metadata endpoints being always blocked is particularly important for security awareness.
src/config/server_config.rs (3)
67-70: LGTM! Well-documented security configuration fields.The new fields are properly documented and follow the existing struct pattern. The documentation clearly explains their purpose for SSRF protection.
117-118: LGTM! Proper integration in from_env().The new fields are correctly populated using the getter methods, maintaining consistency with the existing pattern.
314-318: LGTM! Consistent boolean parsing.The implementation follows the same pattern as other boolean config getters in the file (e.g.,
get_enable_swagger,get_reset_storage_on_start).src/utils/mod.rs (1)
47-48: LGTM! Proper module integration.The URL security module is correctly declared and re-exported, following the established pattern in this file.
src/domain/relayer/solana/rpc/methods/transfer_transaction.rs (1)
540-543: LGTM! Test data updated for optional fee fields.The test data correctly reflects the change of
fee_amountandfee_mintfromStringtoOption<String>in theSwapInfostructure. The changes are consistent across test cases.Also applies to: 703-706
README.md (1)
574-594: LGTM! Comprehensive security documentation.The new section provides clear, actionable guidance on SSRF protection:
- Explains both configuration options
- Includes practical examples
- Highlights the always-blocked cloud metadata endpoints
- Provides recommended production configuration
This enhances security awareness and helps users configure the relayer safely.
src/services/provider/evm/mod.rs (2)
188-194: LGTM: Defense-in-depth URL security validation.The re-validation of RPC URL security during provider initialization is a solid defense-in-depth measure. This catches any URLs that might bypass earlier validation or are constructed dynamically at runtime.
203-203: LGTM: Proper redirect prevention configured.Setting
redirect(Policy::none())effectively prevents SSRF attacks via redirect chains. This is a critical security control for RPC URL handling.src/services/provider/solana/mod.rs (1)
572-580: LGTM: Consistent security validation pattern.The URL security validation follows the same pattern as the EVM provider, providing defense-in-depth. The error handling properly categorizes security validation failures as
NetworkConfigurationerrors.src/models/relayer/mod.rs (1)
1144-1147: LGTM: Efficient security configuration retrieval.Retrieving the security configuration once before the loop is efficient and avoids redundant environment variable access.
src/utils/mocks.rs (1)
329-330: Test ServerConfig updated for new SSRF fieldsInitializing
allowed_rpc_hostsandblock_private_ipskeeps tests deterministic (defaults). Looks consistent with the new config shape.src/domain/relayer/solana/rpc/methods/utils.rs (1)
1141-1144: Test fixtures updated correctly forSwapInfo.fee_*: Option<String>.
TheseSome(...)updates look consistent with the new model shape.Also applies to: 1821-1824, 2572-2575, 2917-2919
src/domain/relayer/solana/dex/jupiter_swap.rs (1)
204-205: Fixture aligns withSwapInfo.fee_amount/fee_mintnow being optional.src/services/provider/stellar/mod.rs (1)
1732-1736: Test relaxation for the new validation error path makes sense.src/services/jupiter/mod.rs (1)
54-58:SwapInfo.fee_amount/fee_mintoptional +#[serde(default)]is correct and forward-compatible.Jupiter returns both
feeAmountandfeeMintas strings in the API response, soOption<String>is the appropriate type. The addition of#[serde(default)]is good defensive programming given Jupiter's ongoing deprecation of these fields. No deserialization issues will occur.src/domain/relayer/solana/rpc/methods/sign_transaction.rs (2)
337-340: LGTM!Test fixture correctly updated to use
Option<String>forfee_amountandfee_mintfields, matching the data structure change.
570-573: LGTM!Consistent with the other test fixture update for the
Option<String>type change.src/utils/url_security.rs (5)
24-84: Well-structured SSRF validation with good defense-in-depth.The layered approach (scheme → allow-list → hostname → IP validation) is solid. The explicit comment explaining why DNS resolution is skipped (TOCTOU, latency, reliability) demonstrates good security reasoning. The handling of IPv4-mapped IPv6 addresses is a nice touch for bypass prevention.
110-178: LGTM! Comprehensive IP address validation.Good coverage of security-critical cases:
- IPv4-mapped IPv6 bypass prevention
- Unconditional blocking of unspecified addresses and metadata endpoints
- Conditional blocking of loopback, private, and link-local when
block_privateis enabledThe error messages are clear and informative.
220-230: LGTM! URL sanitization for logging.Stripping query parameters and fragments before logging prevents accidental exposure of API keys or other secrets that may be embedded in RPC URLs.
232-489: Good test coverage for the security validation logic.Tests cover the key scenarios: private IPs, loopback, metadata endpoints, allow-lists, scheme validation, dangerous hostnames, and edge cases. The acknowledgment of the IPv6 URL parsing limitation (lines 454-458) is appreciated.
193-199: No action needed. TheIpv6Addr::is_unicast_link_local()method was stabilized in Rust 1.84.0, and the project's MSRV is 1.88, which fully supports this method.Likely an incorrect or invalid review comment.
Codecov Report❌ Patch coverage is
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. @@ Coverage Diff @@
## main #605 +/- ##
==========================================
+ Coverage 91.31% 92.32% +1.01%
==========================================
Files 257 258 +1
Lines 91015 94337 +3322
==========================================
+ Hits 83111 87100 +3989
+ Misses 7904 7237 -667
🚀 New features to boost your workflow:
|
zeljkoX
left a comment
zeljkoX
left a comment
There was a problem hiding this comment.
LGTM
Added few minor comments
Thanks!
There was a problem hiding this comment.
Pull request overview
This pull request implements comprehensive SSRF (Server-Side Request Forgery) protection for custom RPC URLs in the OpenZeppelin Relayer. The changes add multi-layered security validation to prevent malicious actors from accessing internal network resources, cloud metadata endpoints, or other sensitive network locations.
Changes:
- Added new URL security validation module with configurable SSRF protection
- Integrated security checks across all provider types (EVM, Solana, Stellar) and configuration layers
- Made Jupiter SwapInfo fee fields optional for improved API flexibility
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/utils/url_security.rs |
New module implementing comprehensive URL validation with IP address checking, scheme validation, cloud metadata endpoint blocking, and configurable allowlists |
src/utils/mod.rs |
Exports the new URL security module |
src/config/server_config.rs |
Adds environment variable configuration for ALLOWED_RPC_HOSTS and BLOCK_PRIVATE_IPS |
src/config/config_file/network/common.rs |
Integrates SSRF validation into network configuration loading with comprehensive test coverage |
src/models/relayer/mod.rs |
Adds SSRF validation to custom RPC URL validation in relayer objects |
src/services/provider/evm/mod.rs |
Adds SSRF validation and HTTP redirect blocking to EVM provider initialization |
src/services/provider/stellar/mod.rs |
Adds SSRF validation and HTTP redirect blocking to Stellar provider initialization |
src/services/provider/solana/mod.rs |
Adds SSRF validation to Solana provider with documentation about redirect handling limitations |
src/services/jupiter/mod.rs |
Makes fee_amount and fee_mint fields optional in SwapInfo structure |
| Multiple test files | Updates test data to use optional fee fields in SwapInfo |
src/utils/mocks.rs |
Adds new security configuration fields to mock utilities |
README.md |
Documents new Custom RPC URL Security features with configuration examples |
.env.example |
Adds documentation and examples for new security environment variables |
Comments suppressed due to low confidence (2)
src/services/provider/stellar/mod.rs:398
- The error message includes the raw URL which may contain sensitive information like API keys in the path. Consider using
sanitize_url_for_errorto redact sensitive path information before including the URL in error messages.
.map_err(|e| {
ProviderError::NetworkConfiguration(format!(
"Failed to create HTTP client for raw RPC: {e} - URL: '{url}'"
))
})
src/services/provider/stellar/mod.rs:382
- The error message includes the raw URL which may contain sensitive information like API keys in the path. Consider using
sanitize_url_for_errorto redact sensitive path information before including the URL in error messages.
Client::new(url).map_err(|e| {
ProviderError::NetworkConfiguration(format!(
"Failed to create Stellar RPC client: {e} - URL: '{url}'"
))
})
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Implements SSRF (Server-Side Request Forgery) protection for custom RPC URLs, preventing malicious actors from accessing internal network resources or cloud metadata endpoints.
Key Security Features:
Configuration:
Two new optional environment variables:
Recommended production config:
Testing Process
Checklist
Note
If you are using Relayer in your stack, consider adding your team or organization to our list of Relayer Users in the Wild!
Summary by CodeRabbit
Release Notes
New Features
ALLOWED_RPC_HOSTS(comma-separated allowlist) andBLOCK_PRIVATE_IPS(defaults to false)Documentation
✏️ Tip: You can customize this high-level summary in your review settings.