feat(operator)!: Add UrlExt::join_safe function

Cargo.toml

@@ -137,6 +137,7 @@ derive_partial_eq_without_eq = "deny"
 unnecessary_struct_initialization = "deny"
 
 # Additional restriction lints we enforce
+disallowed_methods = "deny"
 unwrap_in_result = "deny"
 unwrap_used = "deny"
 panic = "deny"

clippy.toml

@@ -1,2 +1,5 @@
 allow-unwrap-in-tests = true
 allow-panic-in-tests = true
+disallowed-methods = [
+  { path = "url::Url::join", reason = "the function resolves its input as a URL, allowing bad actors to craft malicious inputs which replace the URL entirely or traverse the path; use UrlExt::join_safe instead" },
+]

crates/stackable-operator/CHANGELOG.md

@@ -6,17 +6,23 @@ All notable changes to this project will be documented in this file.
 
 ### Added
 
+- Add `UrlExt::join_safe` function to safely append path segments to a base URL ([#1319]).
 - Add `crd::containerdebug.rs` module with the `ContainerdebugConfig` CRD struct as well as helper functions on `ValidatedContainerdebugConfig` ([#1308])
 
-[#1308]: https://github.com/stackabletech/operator-rs/pull/1308
-
 ### Changed
 
 - BREAKING: Bump the RustCrypto dependencies to their new generation, which affects the re-exported
   `certs` and `webhook` modules. See the changelogs of `stackable-certs` and `stackable-webhook`
   for details ([#1315]).
 
+### Removed
+
+- BREAKING: Remove the unsafe and no longer needed `UrlExt::join_many` function. Use
+  `UrlExt::join_safe` instead ([#1319]).
+
+[#1308]: https://github.com/stackabletech/operator-rs/pull/1308
 [#1315]: https://github.com/stackabletech/operator-rs/pull/1315
+[#1319]: https://github.com/stackabletech/operator-rs/pull/1319
 
 ## [0.121.0] - 2026-10-14
 

crates/stackable-operator/src/utils/mod.rs

@@ -19,7 +19,10 @@ pub use self::bash::COMMON_BASH_TRAP_FUNCTIONS;
     since = "0.61.1"
 )]
 pub use self::logging::print_startup_string;
-pub use self::{option::OptionExt, url::UrlExt};
+pub use self::{
+    option::OptionExt,
+    url::{UrlError, UrlExt},
+};
 
 /// Returns the fully qualified controller name, which should be used when a single controller needs to be referred to uniquely.
 ///

crates/stackable-operator/src/utils/url.rs

@@ -1,55 +1,142 @@
+use snafu::{Snafu, ensure};
+
+#[derive(Debug, PartialEq, Eq, Snafu)]
+pub enum UrlError {
+    #[snafu(display(
+        "failed to use {segments} as url path segment{plural}",
+        segments = segments.join(", "),
+        plural = if segments.len() == 1 { "" } else { "s" }
+    ))]
+    InvalidSegments { segments: Vec<String> },
+
+    #[snafu(display("failed to append path segments to url as it cannot be a base"))]
+    CannotBeABase,
+}
+
+/// An extension trait for [`url::Url`], most notably providing a safe alternative for
+/// [`url::Url::join`]: [`UrlExt::join_safe`].
 pub trait UrlExt: Sized {
-    /// Joins many path segments to the [`Url`][url::Url]. Note: Passing
-    /// segments without a trailing slash while not being the final segment
-    /// will **allocate** on the heap to append the missing trailing slash.
-    /// See <https://docs.rs/url/latest/url/struct.Url.html#method.join> for
-    /// more information about why the trailing slash is important.
-    ///
-    /// ### Example
+    /// Returns a clone of this URL with each of `segments` appended as exactly one URL path segment
+    /// to the existing path segments.
     ///
-    /// ```
-    /// use stackable_operator::utils::UrlExt;
+    /// Compared to [`url::Url::join`], it protects users against (accidental) misuse (especially
+    /// when dealing with untrusted input) in two ways:
     ///
-    /// let url = url::Url::parse("http://example.com").unwrap();
-    /// let url = url.join_many(vec!["realms/", "master/", "myuser"]).unwrap();
-    ///
-    /// assert_eq!(url.as_str(), "http://example.com/realms/master/myuser");
-    /// ```
-    fn join_many<'a>(
-        self,
-        inputs: impl IntoIterator<Item = &'a str>,
-    ) -> Result<Self, url::ParseError>;
+    /// 1. It rejects invalid segments: Empty segments, `.`, `..`, and segments containing any of
+    ///    `\t`, `\n`, `\r`.
+    /// 2. It only appends segments, the base URL is never replaced.
+    fn join_safe<T>(&self, segments: &[T]) -> Result<Self, UrlError>
+    where
+        T: AsRef<str>;
 }
 
 impl UrlExt for url::Url {
-    fn join_many<'a>(
-        self,
-        inputs: impl IntoIterator<Item = &'a str>,
-    ) -> Result<Self, url::ParseError> {
-        let mut iter = inputs.into_iter().peekable();
-        let mut url = self;
-
-        while let Some(input) = iter.next() {
-            url = if !input.ends_with('/') && iter.peek().is_some() {
-                url.join(&format!("{input}/"))?
-            } else {
-                url.join(input)?
-            };
-        }
+    fn join_safe<T>(&self, segments: &[T]) -> Result<Self, UrlError>
+    where
+        T: AsRef<str>,
+    {
+        let invalid_segments = segments
+            .iter()
+            .map(AsRef::as_ref)
+            .filter(|s| matches!(*s, "" | "." | "..") || s.contains(['\t', '\n', '\r']))
+            .map(ToOwned::to_owned)
+            .collect::<Vec<_>>();
+
+        ensure!(
+            invalid_segments.is_empty(),
+            InvalidSegmentsSnafu {
+                segments: invalid_segments
+            }
+        );
+
+        let mut url = self.clone();
+        url.path_segments_mut()
+            // A URL cannot be a base when:
+            //
+            // - its scheme is not one of the "special" schemes (http, https, ws, wss, ftp, file), and
+            // - it has no authority (no // after the scheme)
+            //
+            // Examples: mailto:user@example.com, javascript:alert(1)
+            //
+            // See https://url.spec.whatwg.org/#cannot-be-a-base-url-path-state
+            .map_err(|()| CannotBeABaseSnafu.build())?
+            .pop_if_empty()
+            .extend(segments);
 
         Ok(url)
     }
 }
 
 #[cfg(test)]
-mod test {
+mod tests {
+    use rstest::rstest;
+
     use super::*;
 
+    #[rstest]
+    #[case::path_slash("https://example.org/a/", "https://example.org/a/foo/bar/baz")]
+    #[case::path("https://example.org/a", "https://example.org/a/foo/bar/baz")]
+    #[case::root_slash("https://example.org/", "https://example.org/foo/bar/baz")]
+    #[case::root("https://example.org", "https://example.org/foo/bar/baz")]
+    fn join_safe_keeps_base_path(#[case] base: &str, #[case] expected: &str) {
+        let url = url::Url::parse(base).expect("base URL must parse");
+        let url = url
+            .join_safe(&["foo", "bar", "baz"])
+            .expect("must join, all segments are valid");
+
+        assert_eq!(url.as_str(), expected);
+    }
+
+    #[rstest]
+    #[case::absolute_url("https://evil.example.com/", "https:%2F%2Fevil.example.com%2F")]
+    #[case::scheme_relative("//evil.example.com", "%2F%2Fevil.example.com")]
+    #[case::traversal("../../realms/master", "..%2F..%2Frealms%2Fmaster")]
+    #[case::percent_encoded_dots("%2e%2e", "%252e%252e")]
+    #[case::backslash("..\\admin", "..%5Cadmin")]
+    #[case::query("alice?exact=false", "alice%3Fexact=false")]
+    #[case::fragment("alice#", "alice%23")]
+    #[case::guest_principal_name(
+        "alice_example.com#EXT#@example.org",
+        "alice_example.com%23EXT%23@example.org"
+    )]
+    fn join_safe_appends_malicious_segment(#[case] segment: &str, #[case] expected: &str) {
+        let url = url::Url::parse("http://example.org").expect("base URL must parse");
+        let url = url
+            .join_safe(&[segment])
+            .expect("must join, all segments are valid");
+
+        assert_eq!(url.as_str(), format!("http://example.org/{expected}"));
+    }
+
+    #[rstest]
+    #[case::multiple(&["", ".", ".."])]
+    #[case::empty(&[""])]
+    #[case::current(&["."])]
+    #[case::parent(&[".."])]
+    #[case::tab(&["ad\tmin"])]
+    #[case::line_feed(&["ad\nmin"])]
+    #[case::carriage_return(&["ad\rmin"])]
+    fn join_safe_rejects_invalid_segment(#[case] segments: &[&str]) {
+        let url = url::Url::parse("http://example.org").expect("base URL must parse");
+        let error = url
+            .join_safe(segments)
+            .expect_err("must be error because invalid segments are not appended");
+
+        assert_eq!(
+            error,
+            UrlError::InvalidSegments {
+                segments: segments.iter().map(|s| (*s).to_owned()).collect()
+            }
+        );
+    }
+
     #[test]
-    fn url_join_many() {
-        let url = url::Url::parse("http://example.com").unwrap();
-        let url = url.join_many(vec!["realms/", "master/", "myuser"]).unwrap();
+    fn join_safe_rejects_invalid_base() {
+        let url = url::Url::parse("mailto:user@example.org").expect("base URL must parse");
+        let error = url
+            .join_safe(&["foo", "bar", "baz"])
+            .expect_err("must be error because the base URL cannot be a base (opaque path state)");
 
-        assert_eq!(url.as_str(), "http://example.com/realms/master/myuser");
+        assert_eq!(error, UrlError::CannotBeABase);
     }
 }