Support secret interpolation in streamable HTTP extension URLs

crates/goose/src/agents/extension.rs

@@ -474,7 +474,7 @@ impl ExtensionConfig {
                 Ok(Self::StreamableHttp {
                     name,
                     description,
-                    uri,
+                    uri: substitute_env_vars(&uri, &merged),
                     envs: Envs::new(merged),
                     env_keys: vec![],
                     headers,
@@ -785,6 +785,35 @@ available_tools: []
         }
         ; "http_env_key_and_header_substitution"
     )]
+    #[test_case(
+        ExtensionConfig::StreamableHttp {
+            name: "test".into(),
+            description: String::new(),
+            uri: "https://example.com/mcp?api_key=$MY_SECRET".into(),
+            envs: extension::Envs::default(),
+            env_keys: vec!["MY_SECRET".into()],
+            headers: std::collections::HashMap::new(),
+            timeout: None,
+            bundled: None,
+            available_tools: vec![],
+        },
+        ExtensionConfig::StreamableHttp {
+            name: "test".into(),
+            description: String::new(),
+            uri: "https://example.com/mcp?api_key=secret_value".into(),
+            envs: extension::Envs::new({
+                let mut m = std::collections::HashMap::new();
+                m.insert("MY_SECRET".to_string(), "secret_value".to_string());
+                m
+            }),
+            env_keys: vec![],
+            headers: std::collections::HashMap::new(),
+            timeout: None,
+            bundled: None,
+            available_tools: vec![],
+        }
+        ; "http_env_key_uri_substitution"
+    )]
     #[test_case(
         ExtensionConfig::Stdio {
             name: "test".into(),

crates/goose/src/agents/extension_manager.rs

@@ -60,6 +60,11 @@ static RE_ENV_SIMPLE: Lazy<regex::Regex> =
 
 struct Extension {
     pub config: ExtensionConfig,
+    /// Resolved config snapshot (with secrets from keyring substituted)
+    /// captured at client-creation time. Used to detect secret rotation
+    /// without re-reading the keyring on every comparison. Only held in
+    /// memory — never serialized to disk.
+    resolved_config: ExtensionConfig,
 
     client: McpClientBox,
     server_info: Option<ServerInfo>,
@@ -69,13 +74,15 @@ struct Extension {
 impl Extension {
     fn new(
         config: ExtensionConfig,
+        resolved_config: ExtensionConfig,
         client: McpClientBox,
         server_info: Option<ServerInfo>,
         temp_dir: Option<tempfile::TempDir>,
     ) -> Self {
         Self {
             client,
             config,
+            resolved_config,
             server_info,
             _temp_dir: temp_dir,
         }
@@ -377,8 +384,9 @@ pub(crate) fn substitute_env_vars(value: &str, env_map: &HashMap<String, String>
         }
     }
 
-    let snapshot = result.clone();
-    for cap in RE_ENV_SIMPLE.captures_iter(&snapshot) {
+    // Scan the original input for $VAR patterns (not the post-substitution result)
+    // to avoid recursive expansion when a substituted value contains $OTHER_VAR.
+    for cap in RE_ENV_SIMPLE.captures_iter(value) {
         if let Some(var_name) = cap.get(1) {
             if !value.contains(&format!("${{{}}}", var_name.as_str())) {
                 if let Some(env_value) = env_map.get(var_name.as_str()) {
@@ -538,8 +546,14 @@ impl ExtensionManager {
     ) -> ExtensionResult<()> {
         let sanitized_name = config.key();
 
+        // Compare both the unresolved config (to detect structural changes like
+        // migrating from plaintext envs to env_keys) and the resolved config (to
+        // detect secret rotation where only keyring values changed). Only skip
+        // restart if both match.
+        let resolved_config = config.clone().resolve(Config::global()).await?;
+
         if let Some(existing) = self.extensions.lock().await.get(&sanitized_name) {
-            if existing.config == config {
+            if existing.config == config && existing.resolved_config == resolved_config {
                 return Ok(());
             }
             tracing::debug!(
@@ -567,6 +581,7 @@ impl ExtensionManager {
             } => {
                 let config = Config::global();
                 let all_envs = merge_environments(envs, env_keys, &sanitized_name, config).await?;
+                let resolved_uri = substitute_env_vars(uri, &all_envs);
                 let resolved_headers = headers
                     .iter()
                     .map(|(k, v)| (k.clone(), substitute_env_vars(v, &all_envs)))
@@ -576,7 +591,7 @@ impl ExtensionManager {
                 };
 
                 create_streamable_http_client(
-                    uri,
+                    &resolved_uri,
                     *timeout,
                     &resolved_headers,
                     name,
@@ -787,7 +802,13 @@ impl ExtensionManager {
         let mut extensions = self.extensions.lock().await;
         extensions.insert(
             sanitized_name,
-            Extension::new(config, Arc::from(client), server_info, temp_dir),
+            Extension::new(
+                config,
+                resolved_config,
+                Arc::from(client),
+                server_info,
+                temp_dir,
+            ),
         );
         drop(extensions);
         self.invalidate_tools_cache_and_bump_version().await;
@@ -804,10 +825,10 @@ impl ExtensionManager {
         temp_dir: Option<TempDir>,
     ) {
         let normalized = name_to_key(&name);
-        self.extensions
-            .lock()
-            .await
-            .insert(normalized, Extension::new(config, client, info, temp_dir));
+        self.extensions.lock().await.insert(
+            normalized,
+            Extension::new(config.clone(), config.clone(), client, info, temp_dir),
+        );
         self.invalidate_tools_cache_and_bump_version().await;
     }
 
@@ -1708,7 +1729,7 @@ mod tests {
                 bundled: None,
                 available_tools,
             };
-            let extension = Extension::new(config, client, None, None);
+            let extension = Extension::new(config.clone(), config.clone(), client, None, None);
             self.extensions
                 .lock()
                 .await
@@ -2071,6 +2092,20 @@ mod tests {
         assert_eq!(result, "Authorization: Bearer secret123 and API key456");
     }
 
+    #[tokio::test]
+    async fn test_substitute_env_vars_no_recursive_expansion() {
+        let mut env_map = HashMap::new();
+        env_map.insert("TOKEN".to_string(), "abc$KEY".to_string());
+        env_map.insert("KEY".to_string(), "xyz".to_string());
+
+        // A substituted value containing $KEY should NOT be re-expanded
+        let result = substitute_env_vars("${TOKEN}", &env_map);
+        assert_eq!(result, "abc$KEY");
+
+        let result = substitute_env_vars("$TOKEN", &env_map);
+        assert_eq!(result, "abc$KEY");
+    }
+
     #[tokio::test]
     async fn test_collect_moim_uses_minute_granularity() {
         let temp_dir = tempfile::tempdir().unwrap();

ui/desktop/src/components/settings/extensions/modal/ExtensionModal.tsx

@@ -387,7 +387,7 @@ export default function ExtensionModal({
                 />
               </div>
 
-              {formData.type === 'stdio' && (
+              {(formData.type === 'stdio' || formData.type === 'streamable_http') && (
                 <>
                   <hr className="border-t border-border-primary" />