diff --git a/src/adapters/forward.rs b/src/adapters/forward.rs index dee1a15..cb5a4b6 100644 --- a/src/adapters/forward.rs +++ b/src/adapters/forward.rs @@ -127,7 +127,7 @@ use alkcall::registry::context::OperationContext; use alkcall::registry::registration::ResponseStream; use futures::stream; use futures::StreamExt; -use percent_encoding::{utf8_percent_encode, AsciiSet, CONTROLS}; +use percent_encoding::{percent_decode_str, utf8_percent_encode, AsciiSet, CONTROLS}; use reqwest::header::{HeaderMap, HeaderName, HeaderValue, ACCEPT, AUTHORIZATION, CONTENT_TYPE}; use reqwest::Method; use serde_json::Value; @@ -486,12 +486,27 @@ fn scalar_value_to_string(value: &Value) -> String { /// containing `/`, `?`, `#`, or `\` — and any raw `%` it carries — is /// encoded so that a rendered value stays one literal path segment /// (FWD-01 traversal/ smuggled-segments gate). Scalars are never -/// rejected: they are rendered safely instead. Object/array values -/// under a placeholder key are rejected with `INVALID_INPUT` (FWD-18): -/// a path placeholder designates exactly one literal segment, so a -/// structural value has no faithful rendering — the pre-decision -/// behavior spliced the minified JSON into the path *and* emitted the -/// key as a query parameter (double-routed, neither faithful). +/// rejected: they are rendered safely instead — with the lone-dot +/// exception below (FWD-13). Object/array values under a placeholder +/// key are rejected with `INVALID_INPUT` (FWD-18): a path placeholder +/// designates exactly one literal segment, so a structural value has no +/// faithful rendering — the pre-decision behavior spliced the minified +/// JSON into the path *and* emitted the key as a query parameter +/// (double-routed, neither faithful). +/// +/// # Lone-dot values (FWD-13) +/// +/// A scalar whose *decoded* form is exactly `.` or `..` — including the +/// `%2e`/`%2E` spellings, which `Url::set_path` normalizes away after +/// decoding regardless of what the encode set preserves — is rejected +/// with `INVALID_INPUT`. `Url::set_path` removes lone dot segments +/// case-insensitively (`/../x` → `/x`, `/./x` → `/x`, `/%2e%2e/x` → +/// `/x`), so no encode-set fix can keep such a value faithful: the +/// upstream would receive a *different* endpoint than the template +/// describes, with the namespace's credentials attached. The rejection +/// is exact-match on the full decoded segment; values like `v1.2.3` or +/// `.hidden-file` render normally. The parameter name is quoted but the +/// value is never echoed. pub(crate) fn value_to_path_segment(value: &Value) -> Result { let raw = match value { Value::String(_) | Value::Number(_) | Value::Bool(_) | Value::Null => { @@ -504,9 +519,29 @@ pub(crate) fn value_to_path_segment(value: &Value) -> Result ))) } }; + reject_lone_dot_value(value, &raw)?; Ok(utf8_percent_encode(&raw, PATH_VALUE_ENCODE_SET).to_string()) } +/// FWD-13 gate: a decoded path-value of exactly `.` or `..` cannot be +/// rendered faithfully — `Url::set_path` silently normalizes lone dot +/// segments away, so the upstream would receive a different endpoint +/// than the template describes. The parameter is named but the value is +/// not echoed. Called with both the original [`Value`] (for type-naming) +/// and its raw scalar string. +fn reject_lone_dot_value(value: &Value, raw: &str) -> Result<(), CallError> { + let type_label = type_name_of(value); + let is_lone_dot = matches!(raw, "." | "..") + || raw.eq_ignore_ascii_case("%2e") + || raw.eq_ignore_ascii_case("%2e%2e"); + if is_lone_dot { + return Err(CallError::invalid_input(format!( + "path placeholder value of type {type_label} decodes to a lone dot segment, which cannot appear in a rendered path (Url::set_path would silently normalize it away); pass a concrete non-dot value" + ))); + } + Ok(()) +} + /// Raw form of a scalar input value for query emission: `&`/`=` /// separation, escaping, and space encoding are handled by /// `url::query_pairs_mut` downstream, so the value itself must be raw — @@ -632,6 +667,7 @@ fn assemble_request_url(base_url: &str, rendered_path: &str) -> Result Result Result<(), CallError> { + let rendered_segments = rendered_path + .trim_matches('/') + .split('/') + .filter(|s| !s.is_empty()) + .map(|s| percent_decode_str(s).decode_utf8_lossy().into_owned()); + let base_segments: Vec<&str> = base_dir.split('/').filter(|s| !s.is_empty()).collect(); + let expected: Vec = base_segments + .iter() + .copied() + .map(str::to_string) + .chain(rendered_segments) + .collect(); + let actual: Vec = url + .path() + .split('/') + .filter(|s| !s.is_empty()) + .map(|s| percent_decode_str(s).decode_utf8_lossy().into_owned()) + .collect(); + let matches = actual.len() == expected.len() + && std::iter::Iterator::zip(actual.iter(), expected.iter()).all(|(a, e)| a == e); + if matches { + Ok(()) + } else { + Err(CallError::internal(format!( + "request path `{rendered_path}` resolved against `{base_url}` was rewritten by URL normalization: expected segments {expected:?}, got {actual:?}" + ))) + } +} + #[derive(Debug, thiserror::Error)] pub(crate) enum BodyReadError { #[error("upstream response body exceeds the {RESPONSE_BODY_CAP}-byte response cap")] @@ -1699,6 +1777,179 @@ mod tests { assert_eq!(url.path(), "/files/a%252Fb"); } + /// Empirical pin of the failure mode FWD-13 describes, against the + /// locked `url` crate: lone `.`/`..` (and their percent-escaped + /// spellings) are silently *normalized away* by `Url::set_path`, so + /// the encode set alone cannot keep the rendered path faithful. + #[test] + fn url_set_path_normalizes_lone_dot_and_dot_dot_path_values() { + let mut url = Url::parse("https://api.example.com").expect("parses"); + for (spliced, normalized) in [ + ("/tenants/../resources", "/resources"), + ("/tenants/./resources", "/tenants/resources"), + ("/files/..", "/"), + ("/repos/%2E%2E/x", "/x"), + ] { + url.set_path(spliced); + assert_eq!( + url.path(), + normalized, + "set_path silently normalized `{spliced}`" + ); + } + } + + #[test] + fn lone_dot_dot_path_value_is_rejected() { + let err = request_url( + "https://api.example.com", + "/tenants/{tenant}/resources", + json!({"tenant": ".."}), + ) + .expect_err("lone `..` value must be rejected"); + assert_eq!(err.code, "INVALID_INPUT"); + assert!( + err.message.contains("lone dot segment"), + "message must explain the lone-dot rejection: {}", + err.message + ); + assert!( + !err.message.contains(".."), + "error must not echo the raw value: {}", + err.message + ); + } + + #[test] + fn lone_dot_path_value_is_rejected() { + let err = request_url( + "https://api.example.com", + "/tenants/{tenant}/resources", + json!({"tenant": "."}), + ) + .expect_err("lone `.` value must be rejected"); + assert_eq!(err.code, "INVALID_INPUT"); + } + + #[test] + fn percent_escapes_spellings_of_lone_dots_are_rejected() { + for value in ["\u{2e}\u{2e}", "%2e%2e", "%2E%2e", "%2e%2E"] { + let err = request_url( + "https://api.example.com", + "/tenants/{tenant}/resources", + json!({ "tenant": value }), + ) + .expect_err("escaped lone-dot spellings must be rejected"); + assert_eq!(err.code, "INVALID_INPUT", "value was: {value}"); + } + + for value in ["%2e", "%2E"] { + let err = request_url( + "https://api.example.com", + "/tenants/{tenant}/resources", + json!({ "tenant": value }), + ) + .expect_err("escaped lone-dot spellings must be rejected"); + assert_eq!(err.code, "INVALID_INPUT", "value was: {value}"); + } + } + + #[test] + fn dotted_but_not_lone_dot_values_still_render() { + for value in [ + "v1.2.3", + ".hidden-file", + "..hidden", + "hidden..", + "a..b", + ".a.b.", + "...", + ] { + let url = request_url( + "https://api.example.com", + "/files/{name}", + json!({ "name": value }), + ) + .unwrap_or_else(|e| panic!("value `{value}` must render: {e:?}")); + assert!( + url.path().starts_with("/files/"), + "value `{value}` rendered into the path" + ); + } + } + + /// FWD-13 belt-and-braces: across a dot/percent/binary corpus the + /// post-`set_path` invariant holds — the decoded URL path segments + /// equal the base dir plus the decoded rendered segments, + /// byte-identical — so a normalizer rewrite (today's lone-dot + /// removal, tomorrow's regression in the `url` crate) can never + /// silently re-route an authenticated request. Lone-dot values are + /// rejected outright; values containing `/` or `\` are rejected as + /// smuggled separators only if the encode-set layer ever regressed. + #[test] + fn post_set_path_invariant_holds_across_dot_percent_binary_corpus() { + let corpus = [ + "v1.2.3", + ".hidden-file", + "..", + ".", + "%2e", + "%2E", + "%2e%2e", + "%2E%2E", + "%252e", + "hidden..", + "a..b", + "...", + "a%2Fb", + "a b", + "h\\éllo→世界", + "line\nbreak", + "tab\tchar", + "\u{7f}\u{1f600}", + ]; + for value in corpus { + let outcome = request_url( + "https://api.example.com/v1", + "/files/{name}", + json!({ "name": value }), + ); + match outcome { + Ok(url) => { + assert_eq!(url.host_str(), Some("api.example.com")); + let segments: Vec = url + .path_segments() + .map(|s| { + s.map(|seg| percent_decode_str(seg).decode_utf8_lossy().into_owned()) + .collect() + }) + .unwrap_or_default(); + assert_eq!( + segments.len(), + 3, + "value `{value:?}` must render as one literal segment under /v1/files/" + ); + assert_eq!( + segments.get(2).map(String::as_str), + Some(value), + "value `{value:?}` must survive byte-identical as the final segment" + ); + assert!( + !segments.iter().any(|s| s == "." || s == ".."), + "value `{value:?}` must not leave lone dot segments: {:?}", + segments + ); + } + Err(err) => { + assert_eq!( + err.code, "INVALID_INPUT", + "value `{value:?}` may only fail as INVALID_INPUT" + ); + } + } + } + } + fn ctx_with_capability(namespace: &str, value: String) -> OperationContext { let mut ctx = noop_context(); ctx.capabilities = Capabilities::new().with_http_token(namespace, value);