Apps: validate Windows profile dir names and reject reserved app IDs at install (M/L)

crates/sage-apps/src/lifecycle/install.rs

@@ -17,10 +17,10 @@ use tauri::{AppHandle, Manager, State};
 use uuid::Uuid;
 
 use crate::{
-    AppsHostState, SageAppCommon, SageAppIdentity, SageAppPackageManifest, SageAppSnapshot,
-    SageAppStorage, SageAppWalletScope, SageGrantedPermissionsInput, UserSageApp,
-    UserSageAppSource, allocate_new_storage, apps_root, emit_listed_apps_changed,
-    fresh_snapshot_dir, write_snapshot_manifest,
+    AppsHostState, RUNTIME_ID_PREFIX, SANDBOX_TEST_ID_PREFIX, SageAppCommon, SageAppIdentity,
+    SageAppPackageManifest, SageAppSnapshot, SageAppStorage, SageAppWalletScope,
+    SageGrantedPermissionsInput, UserSageApp, UserSageAppSource, allocate_new_storage, apps_root,
+    builtin_system_app_spec, emit_listed_apps_changed, fresh_snapshot_dir, write_snapshot_manifest,
 };
 
 #[async_trait]
@@ -100,6 +100,8 @@ where
 
     let (app_id, app_dir) = source.resolve_target(&root, base_path, &prepared_artifact)?;
 
+    ensure_installable_app_id(&app_id)?;
+
     if host_state.db.app_exists(&app_id).await? {
         anyhow::bail!("App is already installed");
     }
@@ -225,6 +227,25 @@ pub fn fresh_origin_id(app_id: &str) -> String {
     format!("{}.{}", Uuid::new_v4(), app_id)
 }
 
+/// Rejects app IDs that are reserved for builtin apps.
+///
+/// IDs with the sandbox-test or runtime prefixes get special treatment (for
+/// example, `__sage_test_` apps bypass the sandbox launch gate), so an
+/// installed app must never be able to claim one. Current ID generators can't
+/// produce these, but this must hold structurally rather than by coincidence
+/// of slugification.
+fn ensure_installable_app_id(app_id: &str) -> AnyResult<()> {
+    if app_id.starts_with(SANDBOX_TEST_ID_PREFIX) || app_id.starts_with(RUNTIME_ID_PREFIX) {
+        anyhow::bail!("app id uses a reserved prefix: {app_id}");
+    }
+
+    if builtin_system_app_spec(app_id).is_some() {
+        anyhow::bail!("app id collides with a builtin system app: {app_id}");
+    }
+
+    Ok(())
+}
+
 pub fn create_app_dir(app_dir: &Path) -> AnyResult<()> {
     if app_dir.exists() {
         anyhow::bail!("app directory already exists, cannot create");
@@ -401,6 +422,35 @@ mod tests {
         assert_eq!(installed.source(), &UserSageAppSource::Zip);
     }
 
+    #[test]
+    fn ensure_installable_app_id_rejects_reserved_ids() {
+        for app_id in [
+            "__sage_test_storage_isolation_persistent",
+            "__sage_test_anything",
+            "__sage_runtime_origin_cleanup",
+            "task-manager",
+            "app-update",
+            "bridge-approval",
+        ] {
+            assert!(
+                ensure_installable_app_id(app_id).is_err(),
+                "expected app id {app_id:?} to be rejected"
+            );
+        }
+    }
+
+    #[test]
+    fn ensure_installable_app_id_accepts_generated_ids() {
+        for app_id in [
+            "my-app-8c8532e1-14d8-4d5f-b652-4ff29fc35a19",
+            "url-my-app-0123456789abcdef",
+            "task-manager-8c8532e1-14d8-4d5f-b652-4ff29fc35a19",
+        ] {
+            ensure_installable_app_id(app_id)
+                .unwrap_or_else(|err| panic!("expected app id {app_id:?} to be accepted: {err}"));
+        }
+    }
+
     #[test]
     fn create_app_dir_rejects_existing_directory() {
         let dir = tempdir().unwrap();

crates/sage-apps/src/sandbox/builtin_apps.rs

@@ -25,6 +25,7 @@ macro_rules! runtime_id_prefix {
 }
 
 pub const SANDBOX_TEST_ID_PREFIX: &str = sandbox_test_id_prefix!();
+pub const RUNTIME_ID_PREFIX: &str = runtime_id_prefix!();
 
 pub const BUILTIN_STORAGE_ISOLATION_PERSISTENT_ID: &str =
     concat!(sandbox_test_id_prefix!(), "storage_isolation_persistent");

crates/sage-apps/src/types/storage.rs

@@ -1,10 +1,96 @@
-use serde::{Deserialize, Serialize};
+use serde::{Deserialize, Deserializer, Serialize};
 use specta::Type;
 
 #[derive(Debug, Clone, Serialize, Deserialize, Type, PartialEq, Eq)]
 #[serde(tag = "kind", rename_all = "camelCase")]
 pub enum SageAppStorage {
-    AppleDataStore { identifier_hex: String },
-    WindowsProfile { directory_name: String },
+    AppleDataStore {
+        identifier_hex: String,
+    },
+    WindowsProfile {
+        #[serde(deserialize_with = "deserialize_profile_directory_name")]
+        directory_name: String,
+    },
     Unmanaged,
 }
+
+/// Validates a Windows profile directory name loaded from persisted state.
+///
+/// The name is later joined onto the app data directory (including for
+/// `remove_dir_all` during storage cleanup), so a tampered database row like
+/// `".."` or `"..\\.."` must never be accepted: it could escape the profiles
+/// directory and delete arbitrary data. Legitimate names are only ever
+/// `profile-<uuid>` or `builtin-profile-<builtin app id>`.
+pub(crate) fn validate_profile_directory_name(name: &str) -> Result<(), String> {
+    if name.is_empty() {
+        return Err("profile directory name must not be empty".to_string());
+    }
+
+    if !name
+        .chars()
+        .all(|c| c.is_ascii_alphanumeric() || matches!(c, '-' | '_'))
+    {
+        return Err(format!("invalid profile directory name: {name}"));
+    }
+
+    Ok(())
+}
+
+fn deserialize_profile_directory_name<'de, D>(deserializer: D) -> Result<String, D::Error>
+where
+    D: Deserializer<'de>,
+{
+    let name = String::deserialize(deserializer)?;
+    validate_profile_directory_name(&name).map_err(serde::de::Error::custom)?;
+    Ok(name)
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn windows_profile_json(directory_name: &str) -> String {
+        let valid = serde_json::to_string(&SageAppStorage::WindowsProfile {
+            directory_name: "PLACEHOLDER".to_string(),
+        })
+        .unwrap();
+
+        valid.replace("PLACEHOLDER", directory_name)
+    }
+
+    #[test]
+    fn windows_profile_accepts_expected_directory_names() {
+        for name in [
+            "profile-8c8532e1-14d8-4d5f-b652-4ff29fc35a19",
+            "builtin-profile-__sage_test_storage_isolation_persistent",
+        ] {
+            let storage: SageAppStorage =
+                serde_json::from_str(&windows_profile_json(name)).unwrap();
+            assert_eq!(
+                storage,
+                SageAppStorage::WindowsProfile {
+                    directory_name: name.to_string()
+                }
+            );
+        }
+    }
+
+    #[test]
+    fn windows_profile_rejects_traversal_and_separator_directory_names() {
+        for name in [
+            "..",
+            "../evil",
+            "..\\\\evil",
+            "profiles/../..",
+            "a/b",
+            ".",
+            "profile-abc.def",
+            "",
+        ] {
+            assert!(
+                serde_json::from_str::<SageAppStorage>(&windows_profile_json(name)).is_err(),
+                "expected directory name {name:?} to be rejected"
+            );
+        }
+    }
+}