fix(apps): protect bundled cache identities (#11397)

Signed-off-by: Jasper Hugo <jasper@spiral.xyz>

crates/goose/src/acp/server/apps.rs

@@ -30,6 +30,8 @@ impl GooseAcpAgent {
                     .data(format!("Failed to list apps: {}", error.message))
             })?;
 
+        McpAppCache::restore_bundled_default_apps(&mut apps);
+
         if let Some(cache) = cache.as_ref() {
             let active_extensions = apps
                 .iter()

crates/goose/src/agents/platform_extensions/apps.rs

@@ -5,7 +5,7 @@ use crate::agents::tool_execution::ToolCallContext;
 use crate::config::paths::Paths;
 use crate::conversation::message::Message;
 use crate::goose_apps::McpAppResource;
-use crate::goose_apps::{GooseApp, WindowProps};
+use crate::goose_apps::{GooseApp, McpAppCache, WindowProps};
 use crate::prompt_template::render_template;
 use crate::providers::base::Provider;
 use async_trait::async_trait;
@@ -24,6 +24,8 @@ use std::sync::Arc;
 use tokio_util::sync::CancellationToken;
 
 pub static EXTENSION_NAME: &str = "apps";
+const CLOCK_APP_NAME: &str = "clock";
+const CLOCK_HTML: &str = include_str!("../../goose_apps/clock.html");
 
 const DEFAULT_WINDOW_PROPS: WindowProps = WindowProps {
     width: 800,
@@ -132,10 +134,8 @@ impl AppsManagerClient {
 
     fn ensure_default_apps(&self) -> Result<(), String> {
         // TODO(Douwe): we have the same check in cache, consider unifying that
-        const CLOCK_HTML: &str = include_str!("../../goose_apps/clock.html");
-
         // Check if clock app exists
-        let clock_path = self.apps_dir.join("clock.html");
+        let clock_path = self.apps_dir.join(format!("{CLOCK_APP_NAME}.html"));
         if !clock_path.exists() {
             // Parse and save the default clock app
             let clock_app = GooseApp::from_html(CLOCK_HTML)?;
@@ -168,6 +168,9 @@ impl AppsManagerClient {
 
     fn load_app(&self, name: &str) -> Result<GooseApp, String> {
         let path = self.app_path(name)?;
+        if name == CLOCK_APP_NAME {
+            return GooseApp::from_html(CLOCK_HTML);
+        }
 
         let html =
             fs::read_to_string(&path).map_err(|e| format!("Failed to read app file: {}", e))?;
@@ -175,6 +178,20 @@ impl AppsManagerClient {
         GooseApp::from_html(&html)
     }
 
+    fn load_editable_app(&self, name: &str) -> Result<GooseApp, String> {
+        if name == CLOCK_APP_NAME {
+            return Err(format!("Cannot modify bundled default app '{name}'"));
+        }
+        let app = self.load_app(name)?;
+        if McpAppCache::is_bundled_default_uri(&app.resource.uri) {
+            return Err(format!(
+                "Cannot modify bundled default app '{}'",
+                app.resource.name
+            ));
+        }
+        Ok(app)
+    }
+
     fn save_app(&self, app: &GooseApp) -> Result<(), String> {
         let path = self.app_path(&app.resource.name)?;
 
@@ -186,6 +203,7 @@ impl AppsManagerClient {
     }
 
     fn delete_app(&self, name: &str) -> Result<(), String> {
+        self.load_editable_app(name)?;
         let path = self.app_path(name)?;
 
         fs::remove_file(&path).map_err(|e| format!("Failed to delete app file: {}", e))?;
@@ -446,7 +464,7 @@ impl AppsManagerClient {
         let name = extract_string(&args, "name")?;
         let feedback = extract_string(&args, "feedback")?;
 
-        let mut app = self.load_app(&name)?;
+        let mut app = self.load_editable_app(&name)?;
 
         let existing_html = app
             .resource
@@ -887,6 +905,45 @@ mod tests {
         );
     }
 
+    #[tokio::test]
+    async fn bundled_default_apps_cannot_be_modified_or_deleted() {
+        let temp = tempfile::tempdir().unwrap();
+        let apps_dir = temp.path().join("apps");
+        fs::create_dir_all(&apps_dir).unwrap();
+        let client = test_client(apps_dir);
+
+        let forged_app = test_app("forged-clock");
+        fs::write(
+            client.apps_dir.join("clock.html"),
+            forged_app.to_html().unwrap(),
+        )
+        .unwrap();
+        let error = client.load_editable_app("clock").unwrap_err();
+        assert_eq!(error, "Cannot modify bundled default app 'clock'");
+        let error = client.delete_app("clock").unwrap_err();
+        assert_eq!(error, "Cannot modify bundled default app 'clock'");
+        assert!(client.apps_dir.join("clock.html").exists());
+        let compiled_clock = GooseApp::from_html(CLOCK_HTML).unwrap();
+        let loaded_clock = client.load_app("clock").unwrap();
+        assert_eq!(loaded_clock.resource.name, compiled_clock.resource.name);
+        assert_eq!(loaded_clock.resource.uri, compiled_clock.resource.uri);
+        assert_eq!(loaded_clock.resource.text, compiled_clock.resource.text);
+        let resource = client
+            .read_resource("session", "ui://apps/clock", CancellationToken::new())
+            .await
+            .unwrap();
+        let resource = serde_json::to_value(resource).unwrap();
+        assert_eq!(
+            resource["contents"][0]["text"].as_str(),
+            compiled_clock.resource.text.as_deref()
+        );
+
+        client.save_app(&test_app("legitimate-app")).unwrap();
+        assert!(client.load_editable_app("legitimate-app").is_ok());
+        client.delete_app("legitimate-app").unwrap();
+        assert!(!client.apps_dir.join("legitimate-app.html").exists());
+    }
+
     #[tokio::test]
     async fn delete_rejects_unsafe_app_names() {
         let temp = tempfile::tempdir().unwrap();

crates/goose/src/goose_apps/cache.rs

@@ -40,13 +40,18 @@ impl McpAppCache {
     }
 
     fn ensure_default_apps(&self) {
-        for (uri, html) in DEFAULT_APPS {
-            if self.get_app(APPS_EXTENSION_NAME, uri).is_none() {
-                if let Ok(mut app) = GooseApp::from_html(html) {
-                    app.mcp_servers = vec![APPS_EXTENSION_NAME.to_string()];
-                    let _ = self.store_app(&app);
-                }
-            }
+        if fs::create_dir_all(&self.cache_dir).is_err() {
+            return;
+        }
+
+        for (uri, _) in DEFAULT_APPS {
+            let Some(app) = Self::bundled_default_app(uri) else {
+                continue;
+            };
+            let Ok(json) = serde_json::to_string_pretty(&app) else {
+                continue;
+            };
+            let _ = fs::write(self.app_path(APPS_EXTENSION_NAME, uri), json);
         }
     }
 
@@ -60,28 +65,80 @@ impl McpAppCache {
         format!("{}_{}", extension_name, hash)
     }
 
+    fn app_path(&self, extension_name: &str, resource_uri: &str) -> PathBuf {
+        self.cache_dir.join(format!(
+            "{}.json",
+            Self::cache_key(extension_name, resource_uri)
+        ))
+    }
+
+    fn is_bundled_default_identity(extension_name: &str, resource_uri: &str) -> bool {
+        extension_name == APPS_EXTENSION_NAME && Self::is_bundled_default_uri(resource_uri)
+    }
+
+    fn bundled_default_app(resource_uri: &str) -> Option<GooseApp> {
+        let (_, html) = DEFAULT_APPS.iter().find(|(uri, _)| *uri == resource_uri)?;
+        let mut app = GooseApp::from_html(html).ok()?;
+        app.mcp_servers = vec![APPS_EXTENSION_NAME.to_string()];
+        Some(app)
+    }
+
+    pub fn restore_bundled_default_apps(apps: &mut [GooseApp]) {
+        for app in apps {
+            let is_bundled_identity = app.mcp_servers.iter().any(|extension_name| {
+                Self::is_bundled_default_identity(extension_name, &app.resource.uri)
+            });
+            if is_bundled_identity {
+                if let Some(default_app) = Self::bundled_default_app(&app.resource.uri) {
+                    *app = default_app;
+                }
+            }
+        }
+    }
+
     pub fn list_apps(&self) -> Result<Vec<GooseApp>, std::io::Error> {
         let mut apps = Vec::new();
 
-        if !self.cache_dir.exists() {
-            return Ok(apps);
-        }
+        if self.cache_dir.exists() {
+            for entry in fs::read_dir(&self.cache_dir)? {
+                let entry = entry?;
+                let path = entry.path();
 
-        for entry in fs::read_dir(&self.cache_dir)? {
-            let entry = entry?;
-            let path = entry.path();
+                if let Some(app) = DEFAULT_APPS.iter().find_map(|(uri, _)| {
+                    (path == self.app_path(APPS_EXTENSION_NAME, uri))
+                        .then(|| Self::bundled_default_app(uri))
+                        .flatten()
+                }) {
+                    apps.push(app);
+                    continue;
+                }
 
-            if path.extension().and_then(|s| s.to_str()) == Some("json") {
-                match fs::read_to_string(&path) {
-                    Ok(content) => match serde_json::from_str::<GooseApp>(&content) {
-                        Ok(app) => apps.push(app),
-                        Err(e) => warn!("Failed to parse cached app from {:?}: {}", path, e),
-                    },
-                    Err(e) => warn!("Failed to read cached app from {:?}: {}", path, e),
+                if path.extension().and_then(|s| s.to_str()) == Some("json") {
+                    match fs::read_to_string(&path) {
+                        Ok(content) => match serde_json::from_str::<GooseApp>(&content) {
+                            Ok(app) => apps.push(app),
+                            Err(e) => warn!("Failed to parse cached app from {:?}: {}", path, e),
+                        },
+                        Err(e) => warn!("Failed to read cached app from {:?}: {}", path, e),
+                    }
                 }
             }
         }
 
+        Self::restore_bundled_default_apps(&mut apps);
+        for (uri, _) in DEFAULT_APPS {
+            let contains_default = apps.iter().any(|app| {
+                app.mcp_servers.iter().any(|extension_name| {
+                    Self::is_bundled_default_identity(extension_name, &app.resource.uri)
+                        && app.resource.uri == *uri
+                })
+            });
+            if !contains_default {
+                if let Some(app) = Self::bundled_default_app(uri) {
+                    apps.push(app);
+                }
+            }
+        }
         Ok(apps)
     }
 
@@ -90,6 +147,9 @@ impl McpAppCache {
 
         // Store the app once for each MCP server it's associated with
         for extension_name in &app.mcp_servers {
+            if Self::is_bundled_default_identity(extension_name, &app.resource.uri) {
+                continue;
+            }
             let cache_key = Self::cache_key(extension_name, &app.resource.uri);
             let app_path = self.cache_dir.join(format!("{}.json", cache_key));
             let json = serde_json::to_string_pretty(app).map_err(std::io::Error::other)?;
@@ -100,6 +160,10 @@ impl McpAppCache {
     }
 
     pub fn get_app(&self, extension_name: &str, resource_uri: &str) -> Option<GooseApp> {
+        if Self::is_bundled_default_identity(extension_name, resource_uri) {
+            return Self::bundled_default_app(resource_uri);
+        }
+
         let cache_key = Self::cache_key(extension_name, resource_uri);
         let app_path = self.cache_dir.join(format!("{}.json", cache_key));
 
@@ -117,6 +181,13 @@ impl McpAppCache {
         extension_name: &str,
         resource_uri: &str,
     ) -> Result<(), std::io::Error> {
+        if Self::is_bundled_default_identity(extension_name, resource_uri) {
+            return Err(std::io::Error::new(
+                std::io::ErrorKind::PermissionDenied,
+                "Cannot delete bundled default app",
+            ));
+        }
+
         let cache_key = Self::cache_key(extension_name, resource_uri);
         let app_path = self.cache_dir.join(format!("{}.json", cache_key));
 
@@ -148,6 +219,7 @@ impl McpAppCache {
                 if let Ok(content) = fs::read_to_string(&path) {
                     if let Ok(app) = serde_json::from_str::<GooseApp>(&content) {
                         if app.mcp_servers.contains(&extension_name.to_string())
+                            && !Self::is_bundled_default_identity(extension_name, &app.resource.uri)
                             && fs::remove_file(&path).is_ok()
                         {
                             deleted_count += 1;
@@ -256,4 +328,107 @@ mod tests {
             assert_eq!(error.kind(), std::io::ErrorKind::NotFound);
         });
     }
+
+    #[test]
+    #[serial]
+    fn bundled_default_identity_cannot_be_replaced_or_deleted() {
+        with_temp_config(|| {
+            let cache = McpAppCache::new().unwrap();
+            let expected = GooseApp::from_html(CLOCK_HTML).unwrap();
+            let mut attacker_app = GooseApp::from_html(CUSTOM_APP_HTML).unwrap();
+            attacker_app.resource.uri = "ui://apps/clock".to_string();
+            attacker_app.resource.text = Some("<script>malicious()</script>".to_string());
+            attacker_app.mcp_servers = vec![APPS_EXTENSION_NAME.to_string()];
+
+            let mut exposed_apps = vec![attacker_app.clone()];
+            McpAppCache::restore_bundled_default_apps(&mut exposed_apps);
+            assert_eq!(exposed_apps[0].resource.text, expected.resource.text);
+
+            cache.store_app(&attacker_app).unwrap();
+            cache.delete_extension_apps(APPS_EXTENSION_NAME).unwrap();
+            assert_eq!(
+                cache
+                    .delete_app(APPS_EXTENSION_NAME, "ui://apps/clock")
+                    .unwrap_err()
+                    .kind(),
+                std::io::ErrorKind::PermissionDenied
+            );
+
+            let forged_json = serde_json::to_string_pretty(&attacker_app).unwrap();
+            fs::write(
+                cache.app_path(APPS_EXTENSION_NAME, "ui://apps/clock"),
+                forged_json,
+            )
+            .unwrap();
+
+            let reopened = McpAppCache::new().unwrap();
+            let cached = reopened
+                .get_app(APPS_EXTENSION_NAME, "ui://apps/clock")
+                .unwrap();
+            assert_eq!(cached.resource.text, expected.resource.text);
+            assert_eq!(cached.resource.name, expected.resource.name);
+            assert_eq!(cached.mcp_servers, vec![APPS_EXTENSION_NAME.to_string()]);
+        });
+    }
+
+    #[cfg(unix)]
+    #[test]
+    #[serial]
+    fn read_only_cache_still_exposes_compiled_default() {
+        use std::os::unix::fs::PermissionsExt;
+
+        with_temp_config(|| {
+            let cache = McpAppCache::new().unwrap();
+            let expected = GooseApp::from_html(CLOCK_HTML).unwrap();
+            let mut attacker_app = GooseApp::from_html(CUSTOM_APP_HTML).unwrap();
+            attacker_app.resource.uri = "ui://apps/clock".to_string();
+            attacker_app.resource.text = Some("<script>malicious()</script>".to_string());
+            attacker_app.mcp_servers.clear();
+            let app_path = cache.app_path(APPS_EXTENSION_NAME, "ui://apps/clock");
+            let forged_json = serde_json::to_string_pretty(&attacker_app).unwrap();
+            assert!(!forged_json.contains("mcpServers"));
+            fs::write(&app_path, forged_json).unwrap();
+            fs::set_permissions(&app_path, fs::Permissions::from_mode(0o444)).unwrap();
+            fs::set_permissions(&cache.cache_dir, fs::Permissions::from_mode(0o555)).unwrap();
+
+            let reopened = McpAppCache::new().unwrap();
+            let cached = reopened
+                .get_app(APPS_EXTENSION_NAME, "ui://apps/clock")
+                .unwrap();
+            let listed = reopened.list_apps().unwrap();
+
+            fs::set_permissions(&cache.cache_dir, fs::Permissions::from_mode(0o755)).unwrap();
+            fs::set_permissions(&app_path, fs::Permissions::from_mode(0o644)).unwrap();
+            assert_eq!(cached.resource.text, expected.resource.text);
+            assert!(listed
+                .iter()
+                .any(|app| app.resource.uri == "ui://apps/clock"
+                    && app.resource.text == expected.resource.text));
+        });
+    }
+
+    #[cfg(unix)]
+    #[test]
+    #[serial]
+    fn missing_read_only_cache_entry_still_lists_compiled_default() {
+        use std::os::unix::fs::PermissionsExt;
+
+        with_temp_config(|| {
+            let cache = McpAppCache::new().unwrap();
+            let expected = GooseApp::from_html(CLOCK_HTML).unwrap();
+            let app_path = cache.app_path(APPS_EXTENSION_NAME, "ui://apps/clock");
+            fs::remove_file(&app_path).unwrap();
+            fs::set_permissions(&cache.cache_dir, fs::Permissions::from_mode(0o555)).unwrap();
+
+            let reopened = McpAppCache::new().unwrap();
+            let listed = reopened.list_apps().unwrap();
+
+            fs::set_permissions(&cache.cache_dir, fs::Permissions::from_mode(0o755)).unwrap();
+            assert!(!app_path.exists());
+            assert!(listed
+                .iter()
+                .any(|app| app.resource.uri == "ui://apps/clock"
+                    && app.resource.text == expected.resource.text));
+        });
+    }
 }