Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion crates/fetchkit/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,21 @@ pub enum FetchError {

impl FetchError {
/// Create an error from a reqwest error
///
// THREAT[TM-LEAK-001]: Sanitize reqwest errors to avoid leaking internal hostnames/IPs
pub fn from_reqwest(err: reqwest::Error) -> Self {
if err.is_timeout() {
FetchError::FirstByteTimeout
} else if err.is_connect() {
FetchError::ConnectError(err)
} else if err.is_redirect() {
FetchError::RequestError("redirect error".to_string())
} else if err.is_body() {
FetchError::RequestError("error reading response body".to_string())
} else if err.is_decode() {
FetchError::RequestError("error decoding response".to_string())
} else {
FetchError::RequestError(err.to_string())
FetchError::RequestError("request failed".to_string())
}
}
}
Expand Down
45 changes: 45 additions & 0 deletions crates/fetchkit/tests/ssrf_security.rs
Original file line number Diff line number Diff line change
Expand Up @@ -458,3 +458,48 @@ async fn test_input_007_subdomain_not_matched_by_host_prefix() {
let result = tool.execute(req).await;
assert!(!matches!(result, Err(FetchError::BlockedUrl)));
}

// ============================================================================
// TM-LEAK-001: Error messages must not leak internal hostnames or IPs
// ============================================================================

#[test]
fn test_leak_001_request_error_variants_are_generic() {
// THREAT[TM-LEAK-001]: Verify all RequestError messages are generic and
// don't leak hostnames or IPs from reqwest errors.
// The from_reqwest fallback classifies by error kind, not raw message.
let generic_messages = [
"redirect error",
"error reading response body",
"error decoding response",
"request failed",
];
for msg in &generic_messages {
let err = FetchError::RequestError(msg.to_string());
let display = err.to_string();
assert!(
!display.contains("127.0.0.1"),
"Error should not contain IPs: {display}"
);
assert!(
!display.contains("internal"),
"Error should not contain hostnames: {display}"
);
}
}

#[test]
fn test_leak_001_timeout_and_connect_display_are_generic() {
// Verify that timeout and connect error Display messages are static
// strings that don't contain any dynamic content
let timeout_msg = FetchError::FirstByteTimeout.to_string();
assert_eq!(
timeout_msg,
"Request timed out: server did not respond within 1 second"
);

// ConnectError uses "Failed to connect to server" as Display, hiding
// the reqwest source error from the user-facing message
let blocked_msg = FetchError::BlockedUrl.to_string();
assert_eq!(blocked_msg, "Blocked URL: not allowed by policy");
}
6 changes: 4 additions & 2 deletions specs/threat-model.md
Original file line number Diff line number Diff line change
Expand Up @@ -284,8 +284,10 @@ against unbounded responses (TM-DOS-001).

**TM-LEAK-001 — Error message detail (MITIGATED):**
FetchKit's error types (`FetchError`) use generic messages that don't include
resolved IP addresses. Connect errors say "Failed to connect to server" without
revealing the specific IP or port that was attempted.
resolved IP addresses or internal hostnames. Connect errors say "Failed to connect
to server" and the `from_reqwest()` fallback path classifies errors by type
(redirect, body, decode) instead of passing through raw reqwest error strings
which could contain hostnames or URL details.

## 6. Content Conversion (TM-CONV)

Expand Down
Loading