diff --git a/clients/rust/examples/basic.rs b/clients/rust/examples/basic.rs index 83107ff..e039aa9 100644 --- a/clients/rust/examples/basic.rs +++ b/clients/rust/examples/basic.rs @@ -18,16 +18,12 @@ struct Hello { #[tokio::main] async fn main() -> Result<(), Box> { - let url = std::env::var("CLAWDFORGE_URL") - .unwrap_or_else(|_| "http://localhost:8800".to_string()); - let token = std::env::var("CLAWDFORGE_TOKEN").map_err(|_| { - "set CLAWDFORGE_TOKEN to a cf_... bearer minted via /admin/tokens" - })?; + let url = + std::env::var("CLAWDFORGE_URL").unwrap_or_else(|_| "http://localhost:8800".to_string()); + let token = std::env::var("CLAWDFORGE_TOKEN") + .map_err(|_| "set CLAWDFORGE_TOKEN to a cf_... bearer minted via /admin/tokens")?; - let client = Client::builder() - .base_url(url) - .token(token) - .build()?; + let client = Client::builder().base_url(url).token(token).build()?; // 1) liveness let h = client.healthz().await?; @@ -46,7 +42,10 @@ async fn main() -> Result<(), Box> { }) .await?; - println!("duration_ms={} stop_reason={:?}", r.duration_ms, r.stop_reason); + println!( + "duration_ms={} stop_reason={:?}", + r.duration_ms, r.stop_reason + ); match r.as_json::() { Ok(v) => println!("parsed: hello = {}", v.hello), diff --git a/clients/rust/src/client.rs b/clients/rust/src/client.rs index 2d93624..0923e0f 100644 --- a/clients/rust/src/client.rs +++ b/clients/rust/src/client.rs @@ -3,7 +3,7 @@ use std::path::Path; use std::time::Duration; -use reqwest::header::{HeaderMap, HeaderValue, AUTHORIZATION}; +use reqwest::header::{HeaderMap, HeaderValue}; use reqwest::multipart::{Form, Part}; use reqwest::{Body, Method, Response, StatusCode}; use serde::de::DeserializeOwned; @@ -24,7 +24,10 @@ const DEFAULT_TIMEOUT: Duration = Duration::from_secs(120); /// /// Construct via [`Client::builder`]. `Client` is cheap to clone — internally /// it wraps an `Arc`-backed `reqwest::Client`. -#[derive(Debug, Clone)] +/// +/// `Debug` is hand-written to redact bearer tokens — `format!("{:?}", client)` +/// will never expose `app_token` or `admin_token` plaintext. +#[derive(Clone)] pub struct Client { inner: reqwest::Client, base: Url, @@ -33,10 +36,28 @@ pub struct Client { app_token: Option, /// Admin bootstrap token (used for `/admin/*`). Optional. admin_token: Option, + /// Optional cap on `upload_file` size in bytes — `None` = no cap. + max_upload_bytes: Option, +} + +impl std::fmt::Debug for Client { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Client") + .field("base_url", &self.base.as_str()) + .field("app_token", &self.app_token.as_ref().map(|_| "")) + .field( + "admin_token", + &self.admin_token.as_ref().map(|_| ""), + ) + .field("max_upload_bytes", &self.max_upload_bytes) + .finish_non_exhaustive() + } } /// Builder for [`Client`]. -#[derive(Debug, Default)] +/// +/// `Debug` is hand-written to redact bearer tokens. +#[derive(Default)] pub struct ClientBuilder { base_url: Option, app_token: Option, @@ -44,6 +65,27 @@ pub struct ClientBuilder { timeout: Option, user_agent: Option, danger_accept_invalid_certs: bool, + max_upload_bytes: Option, +} + +impl std::fmt::Debug for ClientBuilder { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("ClientBuilder") + .field("base_url", &self.base_url) + .field("app_token", &self.app_token.as_ref().map(|_| "")) + .field( + "admin_token", + &self.admin_token.as_ref().map(|_| ""), + ) + .field("timeout", &self.timeout) + .field("user_agent", &self.user_agent) + .field( + "danger_accept_invalid_certs", + &self.danger_accept_invalid_certs, + ) + .field("max_upload_bytes", &self.max_upload_bytes) + .finish_non_exhaustive() + } } impl Client { @@ -99,6 +141,10 @@ impl Client { /// /// `ttl_secs` defaults to the server's 3600 if `None`. Server clamps to /// `60..=86400`. + /// + /// If [`ClientBuilder::max_upload_bytes`] was set and the file's size on + /// disk exceeds it, returns [`Error::Config`] before opening any network + /// connection. pub async fn upload_file( &self, path: impl AsRef, @@ -118,6 +164,14 @@ impl Client { let file = tokio::fs::File::open(path).await?; let len = file.metadata().await?.len(); + if let Some(max) = self.max_upload_bytes { + if len > max { + return Err(Error::Config(format!( + "file size {len} bytes exceeds max_upload_bytes={max}" + ))); + } + } + let stream = ReaderStream::new(file); let body = Body::wrap_stream(stream); @@ -143,10 +197,7 @@ impl Client { } /// `POST /admin/tokens`. Requires an admin token on the client. - pub async fn create_token( - &self, - body: TokenCreateRequest, - ) -> Result { + pub async fn create_token(&self, body: TokenCreateRequest) -> Result { let token = self.require_admin()?; let url = self.url("/admin/tokens")?; let resp = self @@ -163,27 +214,38 @@ impl Client { pub async fn list_tokens(&self) -> Result { let token = self.require_admin()?; let url = self.url("/admin/tokens")?; - let resp = self - .inner - .get(url) - .bearer_auth(token) - .send() - .await?; + let resp = self.inner.get(url).bearer_auth(token).send().await?; json_or_error(resp).await } /// `DELETE /admin/tokens/{name}`. Requires an admin token on the client. /// Returns `Ok(())` on success, [`Error::Api`] with status 404 if the /// token does not exist. + /// + /// `name` is validated client-side to match the server's + /// `[a-z0-9][a-z0-9_-]{0,63}` constraint — anything containing `/`, `?`, + /// `#`, `..`, or empty short-circuits with [`Error::Config`] before a + /// request is sent. This is defense-in-depth against path traversal via + /// `Url::join` (which honors RFC 3986 `..` resolution). pub async fn revoke_token(&self, name: &str) -> Result<(), Error> { + if name.is_empty() + || name.contains('/') + || name.contains('?') + || name.contains('#') + || name.contains("..") + { + return Err(Error::Config(format!("invalid token name: {name:?}"))); + } let token = self.require_admin()?; let url = self.url(&format!("/admin/tokens/{name}"))?; - let resp = self - .inner - .delete(url) - .bearer_auth(token) - .send() - .await?; + let resp = self.inner.delete(url).bearer_auth(token).send().await?; + // 2xx is success regardless of body — RFC-correct DELETE may return + // 204 No Content with no body. + if resp.status().is_success() { + return Ok(()); + } + // Non-2xx: route through json_or_error to get Auth/Api mapping. + // Discard the (already-non-success) deserialization slot. let _: serde_json::Value = json_or_error(resp).await?; Ok(()) } @@ -245,6 +307,15 @@ impl ClientBuilder { self } + /// Maximum file size (in bytes) accepted by [`Client::upload_file`]. + /// Files exceeding this cap fail with [`Error::Config`] before any + /// network I/O. Default `None` = no client-side cap (the server's own + /// limit still applies). + pub fn max_upload_bytes(mut self, max: u64) -> Self { + self.max_upload_bytes = Some(max); + self + } + /// Finalize. Errors if `base_url` is missing or unparseable. pub fn build(self) -> Result { let base_raw = self @@ -276,7 +347,6 @@ impl ClientBuilder { ); // We don't preset Authorization here — per-call helpers do it because // the right token depends on which endpoint is being hit. - let _ = AUTHORIZATION; // silence unused-import lint in some configs let inner = reqwest::Client::builder() .timeout(self.timeout.unwrap_or(DEFAULT_TIMEOUT)) @@ -290,6 +360,7 @@ impl ClientBuilder { base, app_token: self.app_token, admin_token: self.admin_token, + max_upload_bytes: self.max_upload_bytes, }) } } @@ -314,10 +385,15 @@ async fn json_or_error(resp: Response) -> Result } } +/// Truncate `s` to at most `max` bytes, snapping down to the nearest UTF-8 +/// codepoint boundary so we never panic on multibyte sequences. Appends `…` +/// to the truncated form. `str::floor_char_boundary` (stable 1.80+) does the +/// boundary math. fn truncate(s: &str, max: usize) -> String { if s.len() <= max { s.to_string() } else { - format!("{}…", &s[..max]) + let safe = s.floor_char_boundary(max); + format!("{}…", &s[..safe]) } } diff --git a/clients/rust/src/error.rs b/clients/rust/src/error.rs index 6cb0e66..74edaca 100644 --- a/clients/rust/src/error.rs +++ b/clients/rust/src/error.rs @@ -25,7 +25,7 @@ pub enum Error { /// Request never completed: DNS, connect, TLS, body-read, etc. #[error("transport error: {0}")] - Transport(#[from] reqwest::Error), + Transport(reqwest::Error), /// JSON decode failed on a successful HTTP response. #[error("json error: {0}")] @@ -37,8 +37,8 @@ pub enum Error { Io(#[from] std::io::Error), /// The configured request timeout elapsed before the server replied. - /// `reqwest` also surfaces timeouts via [`Error::Transport`]; this variant - /// is reserved for explicit deadlines in the client itself. + /// Mapped from `reqwest::Error::is_timeout()` so callers can match on + /// timeouts specifically without inspecting the inner transport error. #[error("timeout: {0}")] Timeout(String), @@ -47,6 +47,16 @@ pub enum Error { Config(String), } +impl From for Error { + fn from(e: reqwest::Error) -> Self { + if e.is_timeout() { + Self::Timeout(e.to_string()) + } else { + Self::Transport(e) + } + } +} + impl Error { /// Build an [`Error::Api`] from a status code and body string. pub(crate) fn api(status: u16, body: impl Into) -> Self { diff --git a/clients/rust/src/lib.rs b/clients/rust/src/lib.rs index 352550b..09eda47 100644 --- a/clients/rust/src/lib.rs +++ b/clients/rust/src/lib.rs @@ -42,7 +42,7 @@ //! container attribute. #![deny(rust_2018_idioms)] -#![warn(missing_docs)] +#![deny(missing_docs)] mod client; mod error; diff --git a/clients/rust/src/types.rs b/clients/rust/src/types.rs index fde026c..1774539 100644 --- a/clients/rust/src/types.rs +++ b/clients/rust/src/types.rs @@ -124,7 +124,10 @@ pub struct FileToken { /// Response body from `POST /admin/tokens`. /// /// `token` is the plaintext bearer — only returned at creation time. -#[derive(Debug, Clone, Deserialize, Serialize)] +/// +/// `Debug` is hand-written to redact `token` (the plaintext bearer); `name` +/// and `ip_cidrs` print verbatim. +#[derive(Clone, Deserialize, Serialize)] pub struct AppToken { /// App / consumer name. pub name: String, @@ -136,6 +139,16 @@ pub struct AppToken { pub ip_cidrs: Vec, } +impl std::fmt::Debug for AppToken { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("AppToken") + .field("name", &self.name) + .field("token", &"") + .field("ip_cidrs", &self.ip_cidrs) + .finish() + } +} + /// Request body for `POST /admin/tokens`. #[derive(Debug, Clone, Default, Serialize, Deserialize)] pub struct TokenCreateRequest { diff --git a/clients/rust/tests/client.rs b/clients/rust/tests/client.rs index 637319d..aefa741 100644 --- a/clients/rust/tests/client.rs +++ b/clients/rust/tests/client.rs @@ -106,7 +106,10 @@ async fn run_success_with_text_result() { .unwrap(); assert_eq!(r.as_text(), Some("plain string reply")); let json_attempt: Result, _> = r.as_json(); - assert!(json_attempt.is_err(), "string should not deserialize as map"); + assert!( + json_attempt.is_err(), + "string should not deserialize as map" + ); } #[tokio::test] @@ -293,7 +296,7 @@ async fn unauthorized_response_maps_to_auth_error() { }) .await .expect_err("should fail"); - matches!(err, Error::Auth(_)).then_some(()).unwrap(); + assert!(matches!(err, Error::Auth(_))); } #[tokio::test] @@ -320,7 +323,7 @@ async fn missing_app_token_short_circuits_run() { } #[tokio::test] -async fn transport_timeout_surfaces_as_transport_error() { +async fn error_timeout_constructed_on_reqwest_timeout() { let server = MockServer::start().await; Mock::given(method("GET")) .and(path("/healthz")) @@ -343,7 +346,7 @@ async fn transport_timeout_surfaces_as_transport_error() { .build() .unwrap(); let err = c.healthz().await.expect_err("should time out"); - assert!(matches!(err, Error::Transport(_)), "got {err:?}"); + assert!(matches!(err, Error::Timeout(_)), "got {err:?}"); } #[tokio::test] @@ -360,3 +363,155 @@ async fn builder_rejects_bad_scheme() { .expect_err("should fail"); assert!(matches!(err, Error::Config(_))); } + +// ---- audit-driven regression tests -------------------------------------- + +/// H1: 4xx body with multibyte char straddling the truncation cutoff must +/// not panic. Build a 503-byte string where `ü` (2 bytes UTF-8) lands at +/// offset 499..501, so byte 500 is mid-codepoint. +#[tokio::test] +async fn truncate_handles_multibyte_boundary() { + let server = MockServer::start().await; + + let mut body = String::new(); + for _ in 0..499 { + body.push('a'); + } + body.push('ü'); // bytes 499 and 500 + for _ in 0..2 { + body.push('b'); + } + assert_eq!(body.len(), 503); + assert!(!body.is_char_boundary(500)); + + Mock::given(method("POST")) + .and(path("/run")) + .respond_with(ResponseTemplate::new(401).set_body_string(body.clone())) + .mount(&server) + .await; + + let c = make_client(&server); + let err = c + .run(RunRequest { + prompt: "x".into(), + ..Default::default() + }) + .await + .expect_err("should fail"); + // Just having reached this line — without panicking — is the assertion. + assert!(matches!(err, Error::Auth(_)), "got {err:?}"); +} + +/// H2: `Debug` on `Client` must not leak app or admin tokens. +#[tokio::test] +async fn client_debug_redacts_bearer() { + let server = MockServer::start().await; + let c = Client::builder() + .base_url(server.uri()) + .token("cf_super_secret_app_bearer") + .admin_token("admin_super_secret_bearer") + .build() + .unwrap(); + let dbg = format!("{c:?}"); + assert!( + !dbg.contains("cf_super_secret_app_bearer"), + "app token leaked: {dbg}" + ); + assert!( + !dbg.contains("admin_super_secret_bearer"), + "admin token leaked: {dbg}" + ); + assert!(dbg.contains(""), "no redaction marker: {dbg}"); + + // ClientBuilder Debug also redacts. + let builder = Client::builder() + .base_url("http://x") + .token("cf_builder_secret"); + let bdbg = format!("{builder:?}"); + assert!( + !bdbg.contains("cf_builder_secret"), + "builder token leaked: {bdbg}" + ); + assert!(bdbg.contains(""), "no redaction marker: {bdbg}"); +} + +/// H2: `Debug` on `AppToken` must not leak the plaintext `token` field. +#[test] +fn app_token_debug_redacts_token() { + let t = clawdforge::AppToken { + name: "cauldron".into(), + token: "cf_should_not_appear".into(), + ip_cidrs: vec!["172.24.0.0/16".into()], + }; + let dbg = format!("{t:?}"); + assert!(!dbg.contains("cf_should_not_appear"), "leaked: {dbg}"); + assert!(dbg.contains(""), "no marker: {dbg}"); + // name + ip_cidrs are non-secret and should still print. + assert!(dbg.contains("cauldron")); + assert!(dbg.contains("172.24.0.0/16")); +} + +/// H3: `revoke_token` must reject path-traversal sequences before issuing +/// any HTTP request. +#[tokio::test] +async fn revoke_token_rejects_path_traversal() { + let server = MockServer::start().await; + // No mock — if a request escaped client-side validation, wiremock would + // 404 and we'd see Error::Api, not Error::Config. + let c = make_client(&server); + for bad in [ + "../foo", "..", "foo/bar", "foo?x=1", "foo#frag", "", "a/../b", + ] { + let err = c + .revoke_token(bad) + .await + .expect_err(&format!("revoke_token({bad:?}) should reject")); + assert!( + matches!(err, Error::Config(_)), + "{bad:?} produced wrong variant: {err:?}" + ); + } +} + +/// M2: a 204 No Content response from `revoke_token` must Ok-out. +#[tokio::test] +async fn revoke_token_accepts_204_no_content() { + let server = MockServer::start().await; + Mock::given(method("DELETE")) + .and(path_regex(r"^/admin/tokens/.+")) + .and(header("authorization", "Bearer admin_test_token")) + .respond_with(ResponseTemplate::new(204)) + .mount(&server) + .await; + + let c = make_client(&server); + c.revoke_token("cauldron") + .await + .expect("204 No Content should be Ok"); +} + +/// M4: `upload_file` with a `max_upload_bytes` cap rejects oversized files +/// before any network I/O. +#[tokio::test] +async fn upload_file_respects_max_upload_bytes() { + let server = MockServer::start().await; + // No /files mock — if the cap fails to short-circuit, the test will see + // a 404 from wiremock instead of Error::Config. + + let mut tmp = tempfile::NamedTempFile::new().unwrap(); + // Write 1024 bytes; cap at 512. + write!(tmp, "{}", "x".repeat(1024)).unwrap(); + tmp.flush().unwrap(); + + let c = Client::builder() + .base_url(server.uri()) + .token("cf_test_token") + .max_upload_bytes(512) + .build() + .unwrap(); + let err = c + .upload_file(tmp.path(), Some(1800)) + .await + .expect_err("should reject oversize"); + assert!(matches!(err, Error::Config(_)), "got {err:?}"); +}