Skip to content

Commit 7c94cf6

Browse files
SSRF hardening-related API ergonomics
1 parent 5ccf52e commit 7c94cf6

15 files changed

+838
-35
lines changed

AGENTS.md

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
## What is This Codebase?
44

55
This is a Rust client for the [RabbitMQ HTTP API](https://www.rabbitmq.com/docs/http-api-reference).
6+
See [CONTRIBUTING.md](./CONTRIBUTING.md) for test running instructions and development setup.
67

78
## Build System
89

@@ -45,15 +46,26 @@ They have very similar APIs except that all functions of the async client are, w
4546
* `tests/*proptests.rs` are property-based tests
4647
* `tests/test_helpers.rs` contains helper functions shared by multiple test modules
4748

48-
Use `cargo nextest run --profile default --all-features '--' --exact [test module name]` to run
49+
### `nextest` Test Filters
50+
51+
Key [`nextest` filterset predicates](https://nexte.st/docs/filtersets/reference/):
52+
53+
* `test(pattern)`: matches test names using a substring (e.g. `test(list_nodes)`)
54+
* `binary(pattern)`: matches the test binary name (e.g. `binary(async_tls_tests)`)
55+
* `package(name)`: matches by package (e.g. `package(rabbitmq_http_client)`)
56+
* `test(=exact_name)`: an exact test name match
57+
* `test(/regex/)`: like `test(pattern)` but uses regular expression matching
58+
* Set operations: `expr1 + expr2` (union), `expr1 - expr2` (difference), `not expr` (negation)
59+
60+
For example, use `cargo nextest run --all-features -E 'binary(=test_module_name)'` to run
4961
all tests in a specific module.
5062

5163
### Property-based Tests
5264

5365
Property-based tests are written using [proptest](https://docs.rs/proptest/latest/proptest/) and
5466
use a naming convention: they begin with `prop_`.
5567

56-
To run the property-based tests specifically, use `cargo nextest run --all-features 'prop_'`.
68+
To run the property-based tests specifically, use `cargo nextest run --all-features -E 'test(~prop_)'`.
5769

5870
## Source of Domain Knowledge
5971

CHANGELOG.md

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,15 @@
22

33
## v0.82.0 (in development)
44

5-
(no changes yet)
5+
### Breaking Changes
6+
7+
* SSRF hardening: `ClientBuilder#build` now returns `Result<Client, EndpointValidationError>`
8+
and rejects endpoints that do not use an `http` or `https` scheme
9+
### Enhancements
10+
11+
* `ClientBuilder#with_redirect_policy` controls the HTTP redirect policy.
12+
`with_recommended_defaults` now also disables redirects
13+
* `EndpointValidationError` and `RedirectPolicy` are re-exported from both `api` and `blocking_api`
614

715

816
## v0.81.0 (Feb 17, 2026)

CONTRIBUTING.md

Lines changed: 54 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,26 @@
11
# Contributing
22

3+
See also [AGENTS.md](./AGENTS.md) for a high-level codebase overview and conventions.
4+
35
## Running Tests
46

57
While tests support the standard `cargo test` option, another option
68
for running tests is [cargo-nextest](https://nexte.st/).
79

10+
### `nextest` Test Filters
11+
12+
Key [`nextest` filterset predicates](https://nexte.st/docs/filtersets/reference/):
13+
14+
* `test(pattern)`: matches test names using a substring (e.g. `test(list_nodes)`)
15+
* `binary(pattern)`: matches the test binary name (e.g. `binary(async_tls_tests)`)
16+
* `package(name)`: matches by package (e.g. `package(rabbitmq_http_client)`)
17+
* `test(=exact_name)`: an exact test name match
18+
* `test(/regex/)`: like `test(pattern)` but uses regular expression matching
19+
* Set operations: `expr1 + expr2` (union), `expr1 - expr2` (difference), `not expr` (negation)
20+
21+
For example, use `cargo nextest run --all-features -E 'binary(=test_module_name)'` to run
22+
all tests in a specific module.
23+
824
### Run All Tests
925

1026
``` bash
@@ -14,21 +30,52 @@ NEXTEST_RETRIES=3 cargo nextest run --all-features
1430
### Run All Async Client Tests
1531

1632
```bash
17-
cargo test async --all-features
33+
cargo nextest run --all-features -E 'binary(~async)'
34+
```
35+
36+
### Run All Blocking Client Tests
37+
38+
```bash
39+
cargo nextest run --all-features -E 'binary(~blocking)'
40+
```
41+
42+
### Run All Unit Tests
43+
44+
```bash
45+
cargo nextest run --all-features -E 'binary(~unit)'
1846
```
1947

20-
### Run All Sync Client Tests
48+
### Run All Tests in a Specific Module
49+
50+
Use the `binary(=name)` predicate with the test module name:
2151

2252
```bash
23-
cargo test blocking --all-features
53+
cargo nextest run --all-features -E 'binary(=unit_endpoint_validation_tests)'
2454
```
2555

26-
### Run a Specific Test
56+
### Run a Specific Test by Name
57+
58+
Use the `test(=name)` predicate for an exact match:
2759

2860
``` bash
29-
NEXTEST_RETRIES=3 cargo nextest run -E "test(test_list_all_vhost_limits)"
61+
cargo nextest run --all-features -E 'test(=test_list_all_vhost_limits)'
62+
```
63+
64+
Or use `test(~substring)` for a substring match:
65+
66+
```bash
67+
cargo nextest run --all-features -E 'test(~list_vhost)'
68+
```
69+
70+
### Run Property-based Tests
71+
72+
```bash
73+
cargo nextest run --all-features -E 'test(~prop_)'
3074
```
3175

76+
See the [nextest filtersets documentation](https://nexte.st/docs/selecting/) for more
77+
filter expression predicates and operators.
78+
3279
## Running TLS Integration Tests
3380

3481
TLS-enabled tests require a set of certificate and private key pairs, plus
@@ -51,7 +98,7 @@ export TLS_CERTS_DIR=/path/to/tls-gen/basic/result
5198
Finally, run the tests in question only:
5299

53100
```shell
54-
cargo nextest run -E 'binary(async_tls_tests)' --run-ignored=only --all-features
101+
cargo nextest run --all-features --run-ignored=only -E 'binary(=async_tls_tests)'
55102

56-
cargo nextest run -E 'binary(blocking_tls_tests)' --run-ignored=only --all-features
103+
cargo nextest run --all-features --run-ignored=only -E 'binary(=blocking_tls_tests)'
57104
```

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ blocking = [
6363
tabled = ["dep:tabled"]
6464
default-tls = ["reqwest?/default-tls"]
6565
native-tls = ["reqwest?/native-tls"]
66-
rustls-tls = ["reqwest?/rustls"]
66+
rustls = ["reqwest?/rustls"]
6767
hickory-dns = ["reqwest?/hickory-dns"]
6868

6969
[profile.ci]

src/api/client.rs

Lines changed: 49 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,14 @@
1515

1616
use crate::commons::RetrySettings;
1717
use crate::error::{
18+
EndpointValidationError,
1819
Error::{ClientErrorResponse, NotFound, ServerErrorResponse},
1920
ErrorDetails,
2021
};
2122
use crate::responses;
2223
use backtrace::Backtrace;
2324
use log::trace;
24-
use reqwest::{Client as HttpClient, StatusCode, header::HeaderMap};
25+
use reqwest::{Client as HttpClient, StatusCode, header::HeaderMap, redirect};
2526
use serde::Serialize;
2627
use serde::de::DeserializeOwned;
2728
use std::fmt::Display;
@@ -44,7 +45,11 @@ pub type Result<T> = result::Result<T, HttpClientError>;
4445
/// let endpoint = "http://localhost:15672/api";
4546
/// let username = "username";
4647
/// let password = "password";
47-
/// let rc = ClientBuilder::new().with_endpoint(&endpoint).with_basic_auth_credentials(&username, &password).build();
48+
/// let rc = ClientBuilder::new()
49+
/// .with_endpoint(&endpoint)
50+
/// .with_basic_auth_credentials(&username, &password)
51+
/// .build()
52+
/// .unwrap();
4853
/// // list cluster nodes
4954
/// rc.list_nodes().await;
5055
/// // list user connections
@@ -60,6 +65,7 @@ pub struct ClientBuilder<E = &'static str, U = &'static str, P = &'static str> {
6065
retry_settings: RetrySettings,
6166
connect_timeout: Option<Duration>,
6267
request_timeout: Option<Duration>,
68+
redirect_policy: Option<redirect::Policy>,
6369
}
6470

6571
impl Default for ClientBuilder {
@@ -85,6 +91,7 @@ impl ClientBuilder {
8591
retry_settings: RetrySettings::default(),
8692
connect_timeout: None,
8793
request_timeout: None,
94+
redirect_policy: None,
8895
}
8996
}
9097
}
@@ -96,14 +103,16 @@ where
96103
P: Display,
97104
{
98105
/// Applies recommended default settings: 60 second request timeout,
99-
/// 3 retry attempts with 1 second delay between retries.
106+
/// 3 retry attempts with 1 second delay between retries,
107+
/// and no redirects (the RabbitMQ HTTP API does not use redirects).
100108
pub fn with_recommended_defaults(self) -> Self {
101109
Self {
102110
request_timeout: Some(Duration::from_secs(60)),
103111
retry_settings: RetrySettings {
104112
max_attempts: 3,
105113
delay_ms: 1000,
106114
},
115+
redirect_policy: Some(redirect::Policy::none()),
107116
..self
108117
}
109118
}
@@ -126,6 +135,7 @@ where
126135
retry_settings: self.retry_settings,
127136
connect_timeout: self.connect_timeout,
128137
request_timeout: self.request_timeout,
138+
redirect_policy: self.redirect_policy,
129139
}
130140
}
131141

@@ -145,6 +155,7 @@ where
145155
retry_settings: self.retry_settings,
146156
connect_timeout: self.connect_timeout,
147157
request_timeout: self.request_timeout,
158+
redirect_policy: self.redirect_policy,
148159
}
149160
}
150161

@@ -194,7 +205,8 @@ where
194205
/// .with_endpoint("http://localhost:15672/api")
195206
/// .with_basic_auth_credentials("user", "password")
196207
/// .with_request_timeout(Duration::from_secs(30))
197-
/// .build();
208+
/// .build()
209+
/// .unwrap();
198210
/// ```
199211
pub fn with_request_timeout(self, timeout: Duration) -> Self {
200212
ClientBuilder {
@@ -213,10 +225,34 @@ where
213225
}
214226
}
215227

228+
/// Sets the redirect policy for HTTP requests.
229+
///
230+
/// By default, `reqwest` follows up to 10 redirects. Use
231+
/// `reqwest::redirect::Policy::none()` to disable redirects entirely,
232+
/// which is recommended when the endpoint comes from a potentially untrusted source.
233+
///
234+
/// This setting is ignored if a custom HTTP client is used
235+
/// via [`Self::with_client`].
236+
pub fn with_redirect_policy(self, policy: redirect::Policy) -> Self {
237+
ClientBuilder {
238+
redirect_policy: Some(policy),
239+
..self
240+
}
241+
}
242+
216243
/// Builds and returns a configured `Client`.
217244
///
245+
/// Returns an error if the endpoint scheme is not `http` or `https`.
246+
///
218247
/// This consumes the `ClientBuilder`.
219-
pub fn build(self) -> Client<E, U, P> {
248+
pub fn build(self) -> result::Result<Client<E, U, P>, EndpointValidationError> {
249+
let endpoint_str = self.endpoint.to_string();
250+
if !endpoint_str.starts_with("http://") && !endpoint_str.starts_with("https://") {
251+
return Err(EndpointValidationError::UnsupportedScheme {
252+
endpoint: endpoint_str,
253+
});
254+
}
255+
220256
let client = match self.client {
221257
Some(c) => c,
222258
None => {
@@ -227,17 +263,22 @@ where
227263
if let Some(timeout) = self.request_timeout {
228264
builder = builder.timeout(timeout);
229265
}
230-
builder.build().unwrap()
266+
if let Some(policy) = self.redirect_policy {
267+
builder = builder.redirect(policy);
268+
}
269+
builder
270+
.build()
271+
.map_err(|e| EndpointValidationError::ClientBuildError { source: e })?
231272
}
232273
};
233274

234-
Client::from_http_client_with_retry(
275+
Ok(Client::from_http_client_with_retry(
235276
client,
236277
self.endpoint,
237278
self.username,
238279
self.password,
239280
self.retry_settings,
240-
)
281+
))
241282
}
242283
}
243284

src/api/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,6 @@ mod vhosts;
4444

4545
// Re-export for backwards compatibility.
4646
pub use crate::commons::RetrySettings;
47+
pub use crate::error::EndpointValidationError;
4748
pub use client::{Client, ClientBuilder, HttpClientError, HttpClientResponse, Result};
49+
pub use reqwest::redirect::Policy as RedirectPolicy;

0 commit comments

Comments
 (0)