fix(adapters): post-set_path normalization invariant check (FWD-13)
- assemble_request_url now verifies after Url::set_path that the decoded URL path segments are byte-identical to base_dir + decoded rendered segments; a mismatch (any future normalizer rewrite in the url crate) fails loudly with INTERNAL instead of silently re-routing an authenticated request - property-style corpus test over dot/percent/binary values: accepted values must survive byte-identical as one literal segment with no lone dot segments; failures may only be INVALID_INPUT Verification: scripts/verify.sh (383 passed) and --all-features (499 passed), clippy -D warnings, fmt --check all pass.
This commit is contained in:
+116
-1
@@ -118,7 +118,7 @@ use alkcall::registry::context::OperationContext;
|
|||||||
use alkcall::registry::registration::ResponseStream;
|
use alkcall::registry::registration::ResponseStream;
|
||||||
use futures::stream;
|
use futures::stream;
|
||||||
use futures::StreamExt;
|
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::header::{HeaderMap, HeaderName, HeaderValue, ACCEPT, AUTHORIZATION, CONTENT_TYPE};
|
||||||
use reqwest::Method;
|
use reqwest::Method;
|
||||||
use serde_json::Value;
|
use serde_json::Value;
|
||||||
@@ -655,6 +655,7 @@ fn assemble_request_url(base_url: &str, rendered_path: &str) -> Result<Url, Call
|
|||||||
full_path.push_str(&request_path);
|
full_path.push_str(&request_path);
|
||||||
let mut url = base.clone();
|
let mut url = base.clone();
|
||||||
url.set_path(&full_path);
|
url.set_path(&full_path);
|
||||||
|
assert_untouched_by_normalization(&url, base_dir, rendered_path, base_url)?;
|
||||||
let same_origin = url.scheme() == base.scheme()
|
let same_origin = url.scheme() == base.scheme()
|
||||||
&& url.host() == base.host()
|
&& url.host() == base.host()
|
||||||
&& url.port_or_known_default() == base.port_or_known_default();
|
&& url.port_or_known_default() == base.port_or_known_default();
|
||||||
@@ -668,6 +669,48 @@ fn assemble_request_url(base_url: &str, rendered_path: &str) -> Result<Url, Call
|
|||||||
Ok(url)
|
Ok(url)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Post-`set_path` invariant (FWD-13): the decoded segments of the
|
||||||
|
/// final URL path must equal `base_dir`'s segments plus the decoded
|
||||||
|
/// rendered segments, byte-identical. `set_path` is the one step this
|
||||||
|
/// crate does not control, and its parser normalizes lone dot segments
|
||||||
|
/// away (`/../x` → `/x`); this check turns any future normalizer
|
||||||
|
/// surprise into a loud `INTERNAL` error instead of a silently
|
||||||
|
/// re-routed authenticated request.
|
||||||
|
fn assert_untouched_by_normalization(
|
||||||
|
url: &Url,
|
||||||
|
base_dir: &str,
|
||||||
|
rendered_path: &str,
|
||||||
|
base_url: &str,
|
||||||
|
) -> 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<String> = base_segments
|
||||||
|
.iter()
|
||||||
|
.copied()
|
||||||
|
.map(str::to_string)
|
||||||
|
.chain(rendered_segments)
|
||||||
|
.collect();
|
||||||
|
let actual: Vec<String> = 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)]
|
#[derive(Debug, thiserror::Error)]
|
||||||
pub(crate) enum BodyReadError {
|
pub(crate) enum BodyReadError {
|
||||||
#[error("upstream response body exceeds the {RESPONSE_BODY_CAP}-byte response cap")]
|
#[error("upstream response body exceeds the {RESPONSE_BODY_CAP}-byte response cap")]
|
||||||
@@ -1823,6 +1866,78 @@ mod tests {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// 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<String> = 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 {
|
fn ctx_with_capability(namespace: &str, value: String) -> OperationContext {
|
||||||
let mut ctx = noop_context();
|
let mut ctx = noop_context();
|
||||||
ctx.capabilities = Capabilities::new().with_http_token(namespace, value);
|
ctx.capabilities = Capabilities::new().with_http_token(namespace, value);
|
||||||
|
|||||||
Reference in New Issue
Block a user