diff --git a/crates/tinytools-std/src/network/gate.rs b/crates/tinytools-std/src/network/gate.rs index 49d38d3..c30bbc8 100644 --- a/crates/tinytools-std/src/network/gate.rs +++ b/crates/tinytools-std/src/network/gate.rs @@ -12,6 +12,26 @@ //! already holding, so a host maps its own policy onto it without translating //! a vocabulary. +/// What these tools call themselves on the wire. +/// +/// reqwest sends no `User-Agent` unless told to, and a missing one is not a +/// cosmetic omission. GitHub's REST API refuses the request outright: +/// +/// ```text +/// 403 Request forbidden by administrative rules. +/// Please make sure your request has a User-Agent header +/// ``` +/// +/// Reproducible on demand — the same URL in the same second answers 403 with +/// no header and 200 with one — so every `api.github.com` call through these +/// tools failed, always, and the 403 was then read as a credentials problem. +/// Several other APIs require one too, and anonymous traffic is the first a +/// rate limiter penalises. +/// +/// Identifying rather than disguised: a server that wants to throttle or block +/// this traffic should be able to name it. +pub(super) const USER_AGENT: &str = concat!("tinytools/", env!("CARGO_PKG_VERSION")); + /// The host policy a network tool consults before it acts. /// /// Implementations must be cheap to call: the tools ask on every invocation. diff --git a/crates/tinytools-std/src/network/http_request.rs b/crates/tinytools-std/src/network/http_request.rs index 39aac65..3307b6b 100644 --- a/crates/tinytools-std/src/network/http_request.rs +++ b/crates/tinytools-std/src/network/http_request.rs @@ -184,6 +184,7 @@ impl HttpRequestTool { let builder = reqwest::Client::builder() .timeout(Duration::from_secs(self.timeout_secs)) .connect_timeout(Duration::from_secs(10)) + .user_agent(super::gate::USER_AGENT) .redirect(reqwest::redirect::Policy::none()); let builder = self.gate.prepare_client("tool.http_request", builder); let client = builder.build()?; diff --git a/crates/tinytools-std/src/network/http_request_tests.rs b/crates/tinytools-std/src/network/http_request_tests.rs index 520cc33..8ee3f7e 100644 --- a/crates/tinytools-std/src/network/http_request_tests.rs +++ b/crates/tinytools-std/src/network/http_request_tests.rs @@ -544,3 +544,41 @@ fn the_test_gate_builds_a_client_with_the_requested_timeouts() { let gate = TestNetGate::supervised(); let _client = gate.timeout_client("svc", 5, 2); } + +/// Both network tools must name themselves on the wire. +/// +/// Not cosmetic: GitHub's REST API answers 403 to a request with no +/// `User-Agent`, so every `api.github.com` call through these tools failed +/// until this was set. `serve` records the raw request, which is the only way +/// to assert an outgoing header actually left the process. +#[tokio::test] +async fn an_outgoing_request_carries_a_user_agent() -> anyhow::Result<()> { + let (addr, seen) = serve(vec![ + "HTTP/1.1 200 OK\r\nContent-Length: 2\r\nConnection: close\r\n\r\nok".to_string(), + ]) + .await; + let tool = test_tool(vec![]); + let _ = tool + .execute_request( + &format!("http://{addr}/"), + reqwest::Method::GET, + vec![], + None, + ) + .await?; + + let request = seen + .lock() + .map_err(|error| anyhow::anyhow!("request log mutex poisoned: {error}"))?[0] + .clone(); + let lower = request.to_ascii_lowercase(); + assert!( + lower.contains("user-agent:"), + "no User-Agent was sent:\n{request}" + ); + assert!( + lower.contains("user-agent: tinytools/"), + "the header must identify this crate:\n{request}" + ); + Ok(()) +} diff --git a/crates/tinytools-std/src/network/web_fetch.rs b/crates/tinytools-std/src/network/web_fetch.rs index 1775e0b..25b8572 100644 --- a/crates/tinytools-std/src/network/web_fetch.rs +++ b/crates/tinytools-std/src/network/web_fetch.rs @@ -11,7 +11,7 @@ //! budgets or retries on tool errors sees blocked and rate-limited pages for //! what they are. 3xx responses are not followed and stay successful reports. -use super::gate::{HttpLimits, NetGate, host_of}; +use super::gate::{HttpLimits, NetGate, USER_AGENT, host_of}; use crate::url_guard::{normalize_allowed_domains, validate_url_with_dns_check}; use async_trait::async_trait; use serde_json::json; @@ -237,6 +237,7 @@ impl WebFetchTool { // the caller so they can decide whether to refetch the new URL. let client = match reqwest::Client::builder() .timeout(Duration::from_secs(self.timeout_secs)) + .user_agent(USER_AGENT) .redirect(reqwest::redirect::Policy::none()) .build() { diff --git a/crates/tinytools-std/src/network/web_fetch_tests.rs b/crates/tinytools-std/src/network/web_fetch_tests.rs index 239b5e9..e6d36f8 100644 --- a/crates/tinytools-std/src/network/web_fetch_tests.rs +++ b/crates/tinytools-std/src/network/web_fetch_tests.rs @@ -253,6 +253,50 @@ async fn serve_once(response: &str) -> String { format!("http://{addr}/page") } +#[tokio::test] +async fn an_outgoing_request_carries_a_user_agent() { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + let seen = Arc::new(std::sync::Mutex::new(String::new())); + let request_log = Arc::clone(&seen); + let server = tokio::spawn(async move { + let (mut socket, _) = listener.accept().await.unwrap(); + let mut request = Vec::new(); + let mut buf = [0u8; 1024]; + while !request.windows(4).any(|window| window == b"\r\n\r\n") { + let n = socket.read(&mut buf).await.unwrap(); + assert!( + n != 0, + "peer closed before sending the complete HTTP headers" + ); + request.extend_from_slice(&buf[..n]); + } + *request_log.lock().unwrap() = String::from_utf8_lossy(&request).to_string(); + socket + .write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\nConnection: close\r\n\r\nok") + .await + .unwrap(); + }); + + let tool = fetch(test_security(), vec![], None, None); + let result = tool + .fetch_validated(&format!("http://{addr}/"), 1_000_000, false) + .await + .unwrap(); + assert!(!result.is_error, "got: {}", result.output()); + server.await.unwrap(); + + let request = seen.lock().unwrap().to_ascii_lowercase(); + let user_agent = request + .lines() + .find_map(|line| line.strip_prefix("user-agent: ")); + assert_eq!( + user_agent, + Some(concat!("tinytools/", env!("CARGO_PKG_VERSION"))) + ); +} + fn http_response(status_line: &str, headers: &str, body: &str) -> String { format!( "HTTP/1.1 {status_line}\r\n{headers}Content-Length: {}\r\nConnection: close\r\n\r\n{body}",