fix(secrets): make the identifier join reversible and admit the listing route

Two gaps in the route boundary, both found in review.

Percent-encoding a component does not make the join reversible. It leaves the
unreserved characters alone, and `.`, `-` and `_` are both unreserved and
plausible delimiters -- so under `id_delimiter="."` the one component
`prod.vision` and the two components `prod` and `vision` join to the same
string, and two distinct Secrets share one route. An alter or a drop then
reaches whichever exists. The delimiter is now escaped inside each component,
which is what keeps the boundaries the caller drew. The default `$` is not
unreserved, so it was already escaped inside a component and only ever appeared
raw as the separator; nothing on that path changes shape.

The listing route took a namespace identifier without the admission the Secret
identifier gets, so `list_secrets(&[".."])` resolved to `/v1/secret/list` and
left the route it was built for. Both identifiers now go through one
`require_addressable`, which is also why it is no longer named for Secrets
alone.

Tests: distinct identities stay distinct under a configured delimiter, and a
dot-only namespace is refused before a request is built.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UfmeJ533rQDnPBkMtjerV6

rust/lancedb/src/remote/db.rs

@@ -374,14 +374,14 @@ impl<S: HttpSend> RemoteDatabase<S> {
     /// against a charset that does not exist yet.
     fn secret_id(&self, name: &str, namespace_path: &[String]) -> Result<String> {
         let identifier = build_secret_identifier(name, namespace_path, &self.client.id_delimiter);
-        if !secret_identifier_is_addressable(&identifier) {
-            return Err(Error::InvalidInput {
-                message: format!(
-                    "Secret identifier {identifier:?} cannot be addressed: a path segment of                      only dots is resolved as a relative path and never reaches the Secret                      route"
-                ),
-            });
-        }
-        Ok(identifier)
+        require_addressable(identifier)
+    }
+
+    fn secret_namespace_id(&self, namespace_path: &[String]) -> Result<String> {
+        require_addressable(build_secret_namespace_identifier(
+            namespace_path,
+            &self.client.id_delimiter,
+        ))
     }
 
     async fn post_secret_write<T: serde::Serialize>(&self, route: &str, body: &T) -> Result<()> {
@@ -565,14 +565,39 @@ fn build_secret_identifier(name: &str, namespace: &[String], delimiter: &str) ->
         .iter()
         .map(String::as_str)
         .chain(std::iter::once(name))
-        .map(encode_identifier_component)
+        .map(|component| encode_identifier_component(component, delimiter))
         .collect::<Vec<_>>()
         .join(delimiter)
 }
 
-/// One component of an identifier, escaped for a path segment.
-fn encode_identifier_component(component: &str) -> String {
-    urlencoding::encode(component).into_owned()
+/// One component of an identifier, escaped for a path segment and for the
+/// delimiter that will join it to the others.
+///
+/// Percent-encoding alone is not enough to make the join reversible. It leaves
+/// the unreserved characters untouched, and `.`, `-` and `_` are both
+/// unreserved and plausible delimiters -- so under `id_delimiter="."` the
+/// component `prod.vision` would join to the same string as the two components
+/// `prod` and `vision`, and the two distinct Secrets would share one route.
+/// Escaping the delimiter inside the component is what keeps the boundaries
+/// the caller drew.
+///
+/// The default `$` needs no special handling: it is not unreserved, so it is
+/// already escaped inside a component and only ever appears raw as the
+/// separator. Nothing on that path changes shape.
+fn encode_identifier_component(component: &str, delimiter: &str) -> String {
+    let encoded = urlencoding::encode(component).into_owned();
+    if delimiter.is_empty() || !encoded.contains(delimiter) {
+        return encoded;
+    }
+    encoded.replace(delimiter, &percent_encode_every_byte(delimiter))
+}
+
+/// Percent-encode every byte, whether or not it needs it.
+///
+/// Used for the delimiter, which has to be escaped inside a component precisely
+/// when `urlencoding` would leave it alone.
+fn percent_encode_every_byte(value: &str) -> String {
+    value.bytes().map(|byte| format!("%{byte:02X}")).collect()
 }
 
 /// Refuse an identifier that cannot be a path segment at all.
@@ -588,9 +613,17 @@ fn encode_identifier_component(component: &str) -> String {
 /// reporting that it cannot express the request. The alternative is a URL that
 /// silently resolves to a different route and delivers the body -- on a write
 /// verb, a credential -- to whatever handler is left there.
-fn secret_identifier_is_addressable(identifier: &str) -> bool {
+fn require_addressable(identifier: String) -> Result<String> {
     let decoded = identifier.replace("%2E", ".").replace("%2e", ".");
-    !decoded.chars().all(|character| character == '.')
+    if !decoded.is_empty() && decoded.chars().all(|character| character == '.') {
+        return Err(Error::InvalidInput {
+            message: format!(
+                "identifier {identifier:?} cannot be addressed: a path segment of only dots is \
+                 resolved as a relative path and never reaches the route it was built for"
+            ),
+        });
+    }
+    Ok(identifier)
 }
 
 /// The namespace a Secret listing is scoped to, encoded the same way.
@@ -602,7 +635,7 @@ fn build_secret_namespace_identifier(namespace: &[String], delimiter: &str) -> S
     }
     namespace
         .iter()
-        .map(|segment| encode_identifier_component(segment))
+        .map(|segment| encode_identifier_component(segment, delimiter))
         .collect::<Vec<_>>()
         .join(delimiter)
 }
@@ -988,8 +1021,7 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
     }
 
     async fn list_secrets(&self, namespace_path: &[String]) -> Result<Vec<String>> {
-        let namespace_id =
-            build_secret_namespace_identifier(namespace_path, &self.client.id_delimiter);
+        let namespace_id = self.secret_namespace_id(namespace_path)?;
         let path = format!("/v1/namespace/{namespace_id}/secret/list");
         let mut names = Vec::new();
         let mut page_token: Option<String> = None;
@@ -3333,6 +3365,65 @@ mod tests {
         }
     }
 
+    /// A delimiter that is an unreserved character survives percent-encoding, so
+    /// it has to be escaped inside a component or two different identities join
+    /// to the same string.
+    #[tokio::test]
+    async fn test_a_configured_delimiter_cannot_alias_two_identities() {
+        use std::sync::{Arc, Mutex};
+        let seen = Arc::new(Mutex::new(Vec::new()));
+        let captured = seen.clone();
+        let conn = Connection::new_with_handler_and_config(
+            move |request| {
+                captured
+                    .lock()
+                    .unwrap()
+                    .push(request.url().path().to_string());
+                http::Response::builder().status(200).body("{}").unwrap()
+            },
+            ClientConfig {
+                id_delimiter: Some(".".to_string()),
+                ..Default::default()
+            },
+        );
+        conn.drop_secret("openai", &["prod.vision".to_string()])
+            .await
+            .unwrap();
+        conn.drop_secret("openai", &["prod".to_string(), "vision".to_string()])
+            .await
+            .unwrap();
+        let seen = seen.lock().unwrap();
+        assert_ne!(seen[0], seen[1], "identities collided: {seen:?}");
+    }
+
+    /// Listing is addressed by a namespace identifier and needs the same
+    /// admission the Secret identifier gets: a dot-only namespace resolves away
+    /// and leaves the listing route entirely.
+    #[tokio::test]
+    async fn test_a_dot_only_namespace_cannot_leave_the_listing_route() {
+        use std::sync::{Arc, Mutex};
+        for namespace in [".", "..", "..."] {
+            let reached = Arc::new(Mutex::new(false));
+            let flag = reached.clone();
+            let conn = Connection::new_with_handler(move |_| {
+                *flag.lock().unwrap() = true;
+                http::Response::builder()
+                    .status(200)
+                    .body(r#"{"secrets":[]}"#)
+                    .unwrap()
+            });
+            let error = conn
+                .list_secrets(&[namespace.to_string()])
+                .await
+                .expect_err("a dot-only namespace must not be addressable");
+            assert!(error.to_string().contains("cannot be addressed"), "{error}");
+            assert!(
+                !*reached.lock().unwrap(),
+                "{namespace:?} reached the transport"
+            );
+        }
+    }
+
     /// Two different Secrets must not share one URL. Encoding the components
     /// rather than the joined string is what keeps a segment containing the
     /// delimiter from reading as two segments.