From 5e9d57a9bac31667ce3d2a0d2d2945ac48dc7b4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garci=CC=81a?= Date: Sun, 8 Jun 2025 21:15:05 +0200 Subject: [PATCH 01/12] Implement SDK managed state # Conflicts: # Cargo.lock # crates/bitwarden-state/Cargo.toml # crates/bitwarden-state/src/registry.rs # crates/bitwarden-state/src/repository.rs # crates/bitwarden-vault/src/cipher/cipher.rs # Conflicts: # crates/bitwarden-vault/src/vault_client.rs # Conflicts: # Cargo.lock # crates/bitwarden-state/Cargo.toml Use bundled sqlite to solve compile issues Update readme Don't stringify the data in indexeddb Remove the comment Update readme --- Cargo.lock | 95 +++++++++++ .../src/platform/state_client.rs | 26 ++- crates/bitwarden-state/Cargo.toml | 13 ++ crates/bitwarden-state/README.md | 63 ++++++- crates/bitwarden-state/src/lib.rs | 2 + crates/bitwarden-state/src/registry.rs | 76 ++++++++- crates/bitwarden-state/src/repository.rs | 40 ++++- .../src/sdk_managed/indexed_db.rs | 159 ++++++++++++++++++ crates/bitwarden-state/src/sdk_managed/mod.rs | 95 +++++++++++ .../bitwarden-state/src/sdk_managed/sqlite.rs | 102 +++++++++++ crates/bitwarden-threading/src/lib.rs | 2 +- .../src/thread_bound_runner.rs | 10 +- crates/bitwarden-uniffi/src/error.rs | 3 + crates/bitwarden-uniffi/src/platform/mod.rs | 20 ++- crates/bitwarden-vault/src/cipher/cipher.rs | 2 +- .../src/platform/mod.rs | 21 ++- 16 files changed, 712 insertions(+), 17 deletions(-) create mode 100644 crates/bitwarden-state/src/sdk_managed/indexed_db.rs create mode 100644 crates/bitwarden-state/src/sdk_managed/mod.rs create mode 100644 crates/bitwarden-state/src/sdk_managed/sqlite.rs diff --git a/Cargo.lock b/Cargo.lock index 3e25d4cec..180258f6a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -605,8 +605,16 @@ name = "bitwarden-state" version = "1.0.0" dependencies = [ "async-trait", + "bitwarden-error", + "bitwarden-threading", + "indexed-db", + "js-sys", + "rusqlite", + "serde", + "serde_json", "thiserror 1.0.69", "tokio", + "tsify-next", ] [[package]] @@ -1572,6 +1580,18 @@ dependencies = [ "once_cell", ] +[[package]] +name = "fallible-iterator" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2acce4a10f12dc2fb14a218589d4f1f62ef011b2d0cc4b3cb1bba8e94da14649" + +[[package]] +name = "fallible-streaming-iterator" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7360491ce676a36bf9bb3c56c1aa791658183a54d2744120f27285738d90465a" + [[package]] name = "fancy-regex" version = "0.13.0" @@ -1605,6 +1625,12 @@ version = "1.0.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" +[[package]] +name = "foldhash" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d9c4f5dac5e15c24eb999c26181a6ca40b39fe946cbe4c263c7209467bc83af2" + [[package]] name = "form_urlencoded" version = "1.2.1" @@ -1870,6 +1896,18 @@ name = "hashbrown" version = "0.15.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5971ac85611da7067dbfcabef3c70ebb5606018acd9e2a3903a0da507521e0d5" +dependencies = [ + "foldhash", +] + +[[package]] +name = "hashlink" +version = "0.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7382cf6263419f2d8df38c55d7da83da5c18aef87fc7a7fc1fb1e344edfe14c1" +dependencies = [ + "hashbrown 0.15.4", +] [[package]] name = "heck" @@ -2173,6 +2211,20 @@ version = "0.3.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ce23b50ad8242c51a442f3ff322d56b02f08852c77e4c0b4d3fd684abc89c683" +[[package]] +name = "indexed-db" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "78f4ecbb6cd50773303683617a93fc2782267d2c94546e9545ec4190eb69aa1a" +dependencies = [ + "futures-channel", + "futures-util", + "pin-project-lite", + "scoped-tls", + "thiserror 2.0.12", + "web-sys", +] + [[package]] name = "indexmap" version = "1.9.3" @@ -2370,6 +2422,17 @@ version = "0.2.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f9fbbcab51052fe104eb5e5d351cf728d30a5be1fe14d9be8a3b097481fb97de" +[[package]] +name = "libsqlite3-sys" +version = "0.34.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "91632f3b4fb6bd1d72aa3d78f41ffecfcf2b1a6648d8c241dbe7dbfaf4875e15" +dependencies = [ + "cc", + "pkg-config", + "vcpkg", +] + [[package]] name = "linux-raw-sys" version = "0.4.15" @@ -2824,6 +2887,12 @@ dependencies = [ "spki", ] +[[package]] +name = "pkg-config" +version = "0.3.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7edddbd0b52d732b21ad9a5fab5c704c14cd949e5e9a1ec5929a24fded1b904c" + [[package]] name = "plain" version = "0.2.3" @@ -3296,6 +3365,20 @@ dependencies = [ "zeroize", ] +[[package]] +name = "rusqlite" +version = "0.36.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3de23c3319433716cf134eed225fe9986bc24f63bed9be9f20c329029e672dc7" +dependencies = [ + "bitflags 2.9.1", + "fallible-iterator", + "fallible-streaming-iterator", + "hashlink", + "libsqlite3-sys", + "smallvec", +] + [[package]] name = "rustc-demangle" version = "0.1.25" @@ -3481,6 +3564,12 @@ dependencies = [ "syn", ] +[[package]] +name = "scoped-tls" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e1cf6437eb19a8f4a6cc0f7dca544973b0b78843adbfeb3683d1a94a0024a294" + [[package]] name = "scopeguard" version = "1.2.0" @@ -4628,6 +4717,12 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ba73ea9cf16a25df0c8caa16c51acb937d5712a8429db78a3ee29d5dcacd3a65" +[[package]] +name = "vcpkg" +version = "0.2.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "accd4ea62f7bb7a82fe23066fb0957d48ef677f6eeb8215f372f52e48bb32426" + [[package]] name = "version_check" version = "0.9.5" diff --git a/crates/bitwarden-core/src/platform/state_client.rs b/crates/bitwarden-core/src/platform/state_client.rs index 566b9dea7..33a6807f5 100644 --- a/crates/bitwarden-core/src/platform/state_client.rs +++ b/crates/bitwarden-core/src/platform/state_client.rs @@ -1,6 +1,9 @@ use std::sync::Arc; -use bitwarden_state::repository::{Repository, RepositoryItem}; +use bitwarden_state::{ + registry::StateRegistryError, + repository::{Repository, RepositoryItem, RepositoryItemData}, +}; use crate::Client; @@ -25,4 +28,25 @@ impl StateClient { pub fn get_client_managed(&self) -> Option>> { self.client.internal.repository_map.get_client_managed() } + + /// Initialize the database for SDK managed repositories. + pub async fn initialize_database( + &self, + repositories: Vec, + ) -> Result<(), StateRegistryError> { + self.client + .internal + .repository_map + .initialize_database(repositories) + .await + } + + /// Get a SDK managed state repository for a specific type, if it exists. + pub fn get_sdk_managed< + T: RepositoryItem + serde::ser::Serialize + serde::de::DeserializeOwned, + >( + &self, + ) -> Result, StateRegistryError> { + self.client.internal.repository_map.get_sdk_managed() + } } diff --git a/crates/bitwarden-state/Cargo.toml b/crates/bitwarden-state/Cargo.toml index 0593aad1f..c602313ed 100644 --- a/crates/bitwarden-state/Cargo.toml +++ b/crates/bitwarden-state/Cargo.toml @@ -15,7 +15,20 @@ wasm = [] [dependencies] async-trait = { workspace = true } +bitwarden-error = { workspace = true } +bitwarden-threading = { workspace = true } +serde = { workspace = true } +serde_json = { workspace = true } thiserror = { workspace = true } +tokio = { workspace = true } + +[target.'cfg(target_arch="wasm32")'.dependencies] +indexed-db = ">=0.4.2, <0.5" +js-sys = { workspace = true } +tsify-next = { workspace = true } + +[target.'cfg(not(target_arch="wasm32"))'.dependencies] +rusqlite = { version = ">=0.36.0, <0.37", features = ["bundled"] } [dev-dependencies] tokio = { workspace = true, features = ["rt"] } diff --git a/crates/bitwarden-state/README.md b/crates/bitwarden-state/README.md index cc135e002..82f4358f7 100644 --- a/crates/bitwarden-state/README.md +++ b/crates/bitwarden-state/README.md @@ -14,7 +14,7 @@ struct Cipher { // Register `Cipher` for use with a `Repository`. // This should be done in the crate where `Cipher` is defined. -bitwarden_state::register_repository_item!(Cipher, "Cipher"); +bitwarden_state::register_repository_item!(Cipher, "Cipher", version: 1); ``` With the registration complete, the next important decision is to select where will the data be @@ -173,3 +173,64 @@ class CipherStoreImpl: CipherStore { getClient(userId = userId).platform().store().registerCipherStore(CipherStoreImpl()); ``` + +## SDK-Managed State + +With `SDK-Managed State`, the SDK will be exclusively responsible for the data storage. This means +that the clients don't need to make any changes themselves, as the implementation is internal to the +SDK. To add support for an SDK managed `Repository`, it needs to be added to the initialization code +for WASM and UniFFI. This example shows how to add support for `Cipher`s. + +### WASM + +Go to `crates/bitwarden-wasm-internal/src/platform/mod.rs` and add a line with your type, as shown: + +```rust,ignore + pub async fn initialize_state( + &self, + cipher_repository: CipherRepository, + ) -> Result<(), bitwarden_state::registry::StateRegistryError> { + let cipher = cipher_repository.into_channel_impl(); + self.0.platform().state().register_client_managed(cipher); + + let sdk_managed_repositories = vec![ + // This should list all the SDK-managed repositories + ::data(), + // Add your type here + ]; + + self.0 + .platform() + .state() + .initialize_database(sdk_managed_repositories) + .await + } +``` + +### UniFFI + +Go to `crates/bitwarden-uniffi/src/platform/mod.rs` and add a line with your type, as shown: + +```rust,ignore + pub async fn initialize_state( + &self, + cipher_repository: Arc, + ) -> Result<()> { + let cipher = UniffiRepositoryBridge::new(cipher_repository); + self.0.platform().state().register_client_managed(cipher); + + let sdk_managed_repositories = vec![ + // This should list all the SDK-managed repositories + ::data(), + // Add your type here + ]; + + self.0 + .platform() + .state() + .initialize_database(sdk_managed_repositories) + .await + .map_err(Error::StateRegistry)?; + Ok(()) + } +``` diff --git a/crates/bitwarden-state/src/lib.rs b/crates/bitwarden-state/src/lib.rs index 825a46e39..fe982ee46 100644 --- a/crates/bitwarden-state/src/lib.rs +++ b/crates/bitwarden-state/src/lib.rs @@ -5,3 +5,5 @@ pub mod repository; /// This module provides a registry for managing repositories of different types. pub mod registry; + +pub(crate) mod sdk_managed; diff --git a/crates/bitwarden-state/src/registry.rs b/crates/bitwarden-state/src/registry.rs index 47e9fb832..1645aaf0c 100644 --- a/crates/bitwarden-state/src/registry.rs +++ b/crates/bitwarden-state/src/registry.rs @@ -1,15 +1,25 @@ use std::{ any::{Any, TypeId}, collections::HashMap, - sync::{Arc, RwLock}, + sync::{Arc, OnceLock, RwLock}, }; -use crate::repository::{Repository, RepositoryItem}; +use bitwarden_error::bitwarden_error; +use serde::{de::DeserializeOwned, Serialize}; +use thiserror::Error; + +use crate::{ + repository::{Repository, RepositoryItem, RepositoryItemData}, + sdk_managed::{Database, SystemDatabase}, +}; /// A registry that contains repositories for different types of items. /// These repositories can be either managed by the client or by the SDK itself. pub struct StateRegistry { + sdk_managed: RwLock>, client_managed: RwLock>>, + + database: OnceLock, } impl std::fmt::Debug for StateRegistry { @@ -18,13 +28,58 @@ impl std::fmt::Debug for StateRegistry { } } +#[allow(missing_docs)] +#[bitwarden_error(flat)] +#[derive(Debug, Error)] +pub enum StateRegistryError { + #[error("Database is already initialized")] + DatabaseAlreadyInitialized, + #[error("Database is not initialized")] + DatabaseNotInitialized, + + #[error(transparent)] + Database(#[from] crate::sdk_managed::DatabaseError), +} + impl StateRegistry { /// Creates a new empty `StateRegistry`. #[allow(clippy::new_without_default)] pub fn new() -> Self { StateRegistry { client_managed: RwLock::new(HashMap::new()), + database: OnceLock::new(), + sdk_managed: RwLock::new(Vec::new()), + } + } + + // TODO: Ideally we'd do this in new, but that would mean making the client initialization + // async. + // TODO: This function needs to be provided some configuration to know where to open the + // database. For Sqlite: + // - A folder path where the files will be stored. + // - A user ID to create a unique database file per user? + // + // For WASM indexedDB: + // - A database name to use for the indexedDB (Some prefix to avoid conflicts + user ID?) + + /// Initializes the database used for sdk-managed repositories. + pub async fn initialize_database( + &self, + repositories: Vec, + ) -> Result<(), StateRegistryError> { + if self.database.get().is_some() { + return Err(StateRegistryError::DatabaseAlreadyInitialized); } + let _ = self + .database + .set(SystemDatabase::initialize(&repositories).await?); + + *self + .sdk_managed + .write() + .expect("RwLock should not be poisoned") = repositories.clone(); + + Ok(()) } /// Registers a client-managed repository into the map, associating it with its type. @@ -44,6 +99,17 @@ impl StateRegistry { .and_then(|boxed| boxed.downcast_ref::>>()) .map(Arc::clone) } + + /// Retrieves a SDK-managed repository from the database. + pub fn get_sdk_managed( + &self, + ) -> Result, StateRegistryError> { + if let Some(db) = self.database.get() { + Ok(db.get_repository::()?) + } else { + Err(StateRegistryError::DatabaseNotInitialized) + } + } } #[cfg(test)] @@ -83,9 +149,9 @@ mod tests { #[derive(PartialEq, Eq, Debug)] struct TestItem(T); - register_repository_item!(TestItem, "TestItem"); - register_repository_item!(TestItem, "TestItem"); - register_repository_item!(TestItem>, "TestItem>"); + register_repository_item!(TestItem, "TestItem", version: 1); + register_repository_item!(TestItem, "TestItem", version: 1); + register_repository_item!(TestItem>, "TestItem>", version: 1); impl_repository!(TestA, TestItem); impl_repository!(TestB, TestItem); diff --git a/crates/bitwarden-state/src/repository.rs b/crates/bitwarden-state/src/repository.rs index bf19092b2..bd0006c61 100644 --- a/crates/bitwarden-state/src/repository.rs +++ b/crates/bitwarden-state/src/repository.rs @@ -6,6 +6,14 @@ pub enum RepositoryError { /// An internal unspecified error. #[error("Internal error: {0}")] Internal(String), + + /// A serialization or deserialization error. + #[error(transparent)] + Serde(#[from] serde_json::Error), + + /// An internal database error. + #[error(transparent)] + Database(#[from] crate::sdk_managed::DatabaseError), } /// This trait represents a generic repository interface, capable of storing and retrieving @@ -28,21 +36,51 @@ pub trait Repository: Send + Sync { pub trait RepositoryItem: Internal + Send + Sync + 'static { /// The name of the type implementing this trait. const NAME: &'static str; + + /// The version of the repository type implementing this trait. + const VERSION: u32; + /// Returns the `TypeId` of the type implementing this trait. fn type_id() -> TypeId { TypeId::of::() } + + /// Returns metadata about the repository item type. + fn data() -> RepositoryItemData { + RepositoryItemData::new::() + } +} + +/// This struct holds metadata about a registered repository item type. +#[allow(dead_code)] +#[derive(Debug, Clone, Copy)] +pub struct RepositoryItemData { + pub(crate) type_id: TypeId, + pub(crate) name: &'static str, + pub(crate) version: u32, +} + +impl RepositoryItemData { + /// Create a new `RepositoryItemData` from a type that implements `RepositoryItem`. + pub fn new() -> Self { + Self { + type_id: TypeId::of::(), + name: T::NAME, + version: T::VERSION, + } + } } /// Register a type for use in a repository. The type must only be registered once in the crate /// where it's defined. The provided name must be unique and not be changed. #[macro_export] macro_rules! register_repository_item { - ($ty:ty, $name:literal) => { + ($ty:ty, $name:literal, version: $version:literal) => { const _: () = { impl $crate::repository::___internal::Internal for $ty {} impl $crate::repository::RepositoryItem for $ty { const NAME: &'static str = $name; + const VERSION: u32 = $version; } }; }; diff --git a/crates/bitwarden-state/src/sdk_managed/indexed_db.rs b/crates/bitwarden-state/src/sdk_managed/indexed_db.rs new file mode 100644 index 000000000..6e734ef03 --- /dev/null +++ b/crates/bitwarden-state/src/sdk_managed/indexed_db.rs @@ -0,0 +1,159 @@ +use js_sys::JsString; +use serde::{de::DeserializeOwned, ser::Serialize}; + +use crate::{ + repository::{RepositoryItem, RepositoryItemData}, + sdk_managed::{Database, DatabaseError}, +}; + +#[derive(Debug, thiserror::Error)] +#[error("IndexedDB internal error: {0}")] +pub struct IndexedDbInternalError(String); +impl From for IndexedDbInternalError { + fn from(err: tsify_next::serde_wasm_bindgen::Error) -> Self { + IndexedDbInternalError(err.to_string()) + } +} + +#[derive(Clone)] +pub struct IndexedDbDatabase( + bitwarden_threading::ThreadBoundRunner>, +); +impl Database for IndexedDbDatabase { + async fn initialize(registrations: &[RepositoryItemData]) -> Result { + let factory = indexed_db::Factory::get()?; + + let registrations = registrations.to_vec(); + + // Sum all the versions of the registrations to determine the database version + // TODO: We should do a better versioning strategy, as this won't work if one repository is + // removed + let version: u32 = registrations.iter().map(|reg| reg.version).sum(); + + // Open the database, creating it if needed + let db = factory + .open("bitwarden-sdk-test-db", version, async move |evt| { + let db = evt.database(); + + for reg in registrations { + db.build_object_store(reg.name).create()?; + } + + Ok(()) + }) + .await?; + + let runner = bitwarden_threading::ThreadBoundRunner::new(db); + Ok(IndexedDbDatabase(runner)) + } + + async fn get( + &self, + namespace: &str, + key: &str, + ) -> Result, DatabaseError> { + let namespace = namespace.to_string(); + let key = key.to_string(); + + let result = self + .0 + .run_in_thread(move |db| async move { + db.transaction(&[&namespace]) + .run(|t| async move { + let store = t.object_store(&namespace)?; + let response = store.get(&JsString::from(key)).await?; + + if let Some(value) = response { + Ok(::tsify_next::serde_wasm_bindgen::from_value(value) + .map_err(IndexedDbInternalError::from)?) + } else { + Ok(None) + } + }) + .await + }) + .await??; + + Ok(result) + } + + async fn list( + &self, + namespace: &str, + ) -> Result, DatabaseError> { + let namespace = namespace.to_string(); + + let results = self + .0 + .run_in_thread(move |db| async move { + db.transaction(&[&namespace]) + .run(|t| async move { + let store = t.object_store(&namespace)?; + let results = store.get_all(None).await?; + + let mut items: Vec = Vec::new(); + + for value in results { + let item: T = ::tsify_next::serde_wasm_bindgen::from_value(value) + .map_err(IndexedDbInternalError::from)?; + items.push(item); + } + + Ok(items) + }) + .await + }) + .await??; + + Ok(results) + } + + async fn set( + &self, + namespace: &str, + key: &str, + value: T, + ) -> Result<(), DatabaseError> { + let namespace = namespace.to_string(); + let key = key.to_string(); + + self.0 + .run_in_thread(move |db| async move { + db.transaction(&[&namespace]) + .rw() + .run(|t| async move { + let store = t.object_store(&namespace)?; + + let value = ::tsify_next::serde_wasm_bindgen::to_value(&value) + .map_err(IndexedDbInternalError::from)?; + + store.put_kv(&JsString::from(key), &value).await?; + Ok(()) + }) + .await + }) + .await??; + + Ok(()) + } + + async fn remove(&self, namespace: &str, key: &str) -> Result<(), DatabaseError> { + let namespace = namespace.to_string(); + let key = key.to_string(); + + self.0 + .run_in_thread(move |db| async move { + db.transaction(&[&namespace]) + .rw() + .run(|t| async move { + let store = t.object_store(&namespace)?; + store.delete(&JsString::from(key)).await?; + Ok(()) + }) + .await + }) + .await??; + + Ok(()) + } +} diff --git a/crates/bitwarden-state/src/sdk_managed/mod.rs b/crates/bitwarden-state/src/sdk_managed/mod.rs new file mode 100644 index 000000000..cfd4417be --- /dev/null +++ b/crates/bitwarden-state/src/sdk_managed/mod.rs @@ -0,0 +1,95 @@ +use bitwarden_error::bitwarden_error; +use serde::{de::DeserializeOwned, ser::Serialize}; +use thiserror::Error; + +use crate::repository::{Repository, RepositoryError, RepositoryItem, RepositoryItemData}; + +#[cfg(target_arch = "wasm32")] +mod indexed_db; +#[cfg(target_arch = "wasm32")] +pub(super) type SystemDatabase = indexed_db::IndexedDbDatabase; +#[cfg(target_arch = "wasm32")] +type InternalError = ::indexed_db::Error; + +#[cfg(not(target_arch = "wasm32"))] +mod sqlite; +#[cfg(not(target_arch = "wasm32"))] +pub(super) type SystemDatabase = sqlite::SqliteDatabase; +#[cfg(not(target_arch = "wasm32"))] +type InternalError = ::rusqlite::Error; + +#[bitwarden_error(flat)] +#[derive(Debug, Error)] +pub enum DatabaseError { + #[error(transparent)] + ThreadBoundRunner(#[from] bitwarden_threading::CallError), + + #[error("Serialization error: {0}")] + Serialization(#[from] serde_json::Error), + + #[error("JS error: {0}")] + JSError(String), + + #[error(transparent)] + Internal(#[from] InternalError), +} + +pub trait Database { + async fn initialize(registrations: &[RepositoryItemData]) -> Result + where + Self: Sized; + + async fn get( + &self, + namespace: &str, + key: &str, + ) -> Result, DatabaseError>; + + async fn list( + &self, + namespace: &str, + ) -> Result, DatabaseError>; + + async fn set( + &self, + namespace: &str, + key: &str, + value: T, + ) -> Result<(), DatabaseError>; + + async fn remove(&self, namespace: &str, key: &str) -> Result<(), DatabaseError>; +} + +struct DBRepository { + database: SystemDatabase, + _marker: std::marker::PhantomData, +} + +#[async_trait::async_trait] +impl Repository for DBRepository { + async fn get(&self, key: String) -> Result, RepositoryError> { + let value = self.database.get(V::NAME, &key).await?; + Ok(value) + } + async fn list(&self) -> Result, RepositoryError> { + let values = self.database.list(V::NAME).await?; + Ok(values) + } + async fn set(&self, key: String, value: V) -> Result<(), RepositoryError> { + Ok(self.database.set(V::NAME, &key, value).await?) + } + async fn remove(&self, key: String) -> Result<(), RepositoryError> { + Ok(self.database.remove(V::NAME, &key).await?) + } +} + +impl SystemDatabase { + pub(super) fn get_repository( + &self, + ) -> Result, DatabaseError> { + Ok(DBRepository { + database: self.clone(), + _marker: std::marker::PhantomData, + }) + } +} diff --git a/crates/bitwarden-state/src/sdk_managed/sqlite.rs b/crates/bitwarden-state/src/sdk_managed/sqlite.rs new file mode 100644 index 000000000..32a62f41c --- /dev/null +++ b/crates/bitwarden-state/src/sdk_managed/sqlite.rs @@ -0,0 +1,102 @@ +use std::sync::Arc; + +use serde::{de::DeserializeOwned, ser::Serialize}; +use tokio::sync::Mutex; + +use crate::{ + repository::{RepositoryItem, RepositoryItemData}, + sdk_managed::{Database, DatabaseError}, +}; + +// TODO: Use connection pooling with r2d2 and r2d2_sqlite? +#[derive(Clone)] +pub struct SqliteDatabase(Arc>); +impl Database for SqliteDatabase { + async fn initialize(registrations: &[RepositoryItemData]) -> Result { + let mut db = rusqlite::Connection::open_in_memory()?; + + // Set WAL mode for better concurrency + db.execute("PRAGMA journal_mode = WAL;", [])?; + + let transaction = db.transaction()?; + + for reg in registrations { + transaction.execute( + "CREATE TABLE IF NOT EXISTS ?1 (key TEXT PRIMARY KEY, value TEXT NOT NULL);", + [reg.name], + )?; + } + + transaction.commit()?; + Ok(SqliteDatabase(Arc::new(Mutex::new(db)))) + } + + async fn get( + &self, + namespace: &str, + key: &str, + ) -> Result, DatabaseError> { + let conn = self.0.lock().await; + let mut stmt = conn.prepare("SELECT value FROM ?1 WHERE key = ?2")?; + let mut rows = stmt.query(rusqlite::params![namespace, key])?; + + if let Some(row) = rows.next()? { + let value = row.get::<_, String>(0)?; + + Ok(Some(serde_json::from_str(&value)?)) + } else { + Ok(None) + } + } + + async fn list( + &self, + namespace: &str, + ) -> Result, DatabaseError> { + let conn = self.0.lock().await; + let mut stmt = conn.prepare("SELECT key, value FROM ?1")?; + let rows = stmt.query_map(rusqlite::params![namespace], |row| row.get(1))?; + + let mut results = Vec::new(); + for row in rows { + let value: String = row?; + let value: T = serde_json::from_str(&value)?; + results.push(value); + } + + Ok(results) + } + + async fn set( + &self, + namespace: &str, + key: &str, + value: T, + ) -> Result<(), DatabaseError> { + let mut conn = self.0.lock().await; + let transaction = conn.transaction()?; + + let value = serde_json::to_string(&value)?; + + transaction.execute( + "INSERT OR REPLACE INTO ?1 (key, value) VALUES (?2, ?3)", + rusqlite::params![namespace, key, value], + )?; + + transaction.commit()?; + Ok(()) + } + + async fn remove(&self, namespace: &str, key: &str) -> Result<(), DatabaseError> { + let mut conn = self.0.lock().await; + let transaction = conn.transaction()?; + + transaction.execute( + "DELETE FROM ?1 WHERE key = ?2", + rusqlite::params![namespace, key], + )?; + + transaction.commit()?; + Ok(()) + } +} diff --git a/crates/bitwarden-threading/src/lib.rs b/crates/bitwarden-threading/src/lib.rs index b86b47214..fd0e8bc1a 100644 --- a/crates/bitwarden-threading/src/lib.rs +++ b/crates/bitwarden-threading/src/lib.rs @@ -11,4 +11,4 @@ mod thread_bound_runner; #[allow(missing_docs)] pub mod time; -pub use thread_bound_runner::ThreadBoundRunner; +pub use thread_bound_runner::{CallError, ThreadBoundRunner}; diff --git a/crates/bitwarden-threading/src/thread_bound_runner.rs b/crates/bitwarden-threading/src/thread_bound_runner.rs index ff3154f89..6366561cb 100644 --- a/crates/bitwarden-threading/src/thread_bound_runner.rs +++ b/crates/bitwarden-threading/src/thread_bound_runner.rs @@ -80,11 +80,19 @@ pub struct CallError(String); /// This pattern is useful for interacting with APIs or data structures that must remain /// on the same thread, such as GUI toolkits, WebAssembly contexts, or other thread-bound /// environments. -#[derive(Clone)] pub struct ThreadBoundRunner { call_channel_tx: tokio::sync::mpsc::Sender>, } +// This is not implemented using derive to remove the implicit bound on `ThreadState: Clone` +impl Clone for ThreadBoundRunner { + fn clone(&self) -> Self { + ThreadBoundRunner { + call_channel_tx: self.call_channel_tx.clone(), + } + } +} + impl ThreadBoundRunner where ThreadState: 'static, diff --git a/crates/bitwarden-uniffi/src/error.rs b/crates/bitwarden-uniffi/src/error.rs index 3e0ca8db3..7193a2e5a 100644 --- a/crates/bitwarden-uniffi/src/error.rs +++ b/crates/bitwarden-uniffi/src/error.rs @@ -66,6 +66,9 @@ pub enum Error { #[error(transparent)] Crypto(#[from] bitwarden_crypto::CryptoError), + #[error(transparent)] + StateRegistry(#[from] bitwarden_state::registry::StateRegistryError), + // Generators #[error(transparent)] Username(#[from] UsernameError), diff --git a/crates/bitwarden-uniffi/src/platform/mod.rs b/crates/bitwarden-uniffi/src/platform/mod.rs index 798307967..b0e4fc074 100644 --- a/crates/bitwarden-uniffi/src/platform/mod.rs +++ b/crates/bitwarden-uniffi/src/platform/mod.rs @@ -2,6 +2,7 @@ use std::sync::Arc; use bitwarden_core::{platform::FingerprintRequest, Client}; use bitwarden_fido::ClientFido2Ext; +use bitwarden_state::repository::RepositoryItem; use bitwarden_vault::Cipher; use repository::UniffiRepositoryBridge; @@ -56,11 +57,24 @@ repository::create_uniffi_repository!(CipherRepository, Cipher); #[uniffi::export] impl StateClient { - pub fn register_cipher_repository(&self, store: Arc) { - let store_internal = UniffiRepositoryBridge::new(store); + pub async fn initialize_state( + &self, + cipher_repository: Arc, + ) -> Result<()> { + let cipher = UniffiRepositoryBridge::new(cipher_repository); + self.0.platform().state().register_client_managed(cipher); + + let sdk_managed_repositories = vec![ + // This should list all the SDK-managed repositories + ::data(), + ]; + self.0 .platform() .state() - .register_client_managed(store_internal) + .initialize_database(sdk_managed_repositories) + .await + .map_err(Error::StateRegistry)?; + Ok(()) } } diff --git a/crates/bitwarden-vault/src/cipher/cipher.rs b/crates/bitwarden-vault/src/cipher/cipher.rs index 4c9c1bdb1..b43d2f0b8 100644 --- a/crates/bitwarden-vault/src/cipher/cipher.rs +++ b/crates/bitwarden-vault/src/cipher/cipher.rs @@ -135,7 +135,7 @@ pub struct Cipher { pub revision_date: DateTime, } -bitwarden_state::register_repository_item!(Cipher, "Cipher"); +bitwarden_state::register_repository_item!(Cipher, "Cipher", version: 1); #[allow(missing_docs)] #[derive(Serialize, Deserialize, Debug, Clone)] diff --git a/crates/bitwarden-wasm-internal/src/platform/mod.rs b/crates/bitwarden-wasm-internal/src/platform/mod.rs index b2b977659..29d4fc004 100644 --- a/crates/bitwarden-wasm-internal/src/platform/mod.rs +++ b/crates/bitwarden-wasm-internal/src/platform/mod.rs @@ -1,4 +1,5 @@ use bitwarden_core::Client; +use bitwarden_state::repository::RepositoryItem; use bitwarden_vault::Cipher; use wasm_bindgen::prelude::wasm_bindgen; @@ -33,8 +34,22 @@ repository::create_wasm_repository!(CipherRepository, Cipher, "Repository Result<(), bitwarden_state::registry::StateRegistryError> { + let cipher = cipher_repository.into_channel_impl(); + self.0.platform().state().register_client_managed(cipher); + + let sdk_managed_repositories = vec![ + // This should list all the SDK-managed repositories + ::data(), + ]; + + self.0 + .platform() + .state() + .initialize_database(sdk_managed_repositories) + .await } } From e5cab0d87d644da0e169348421ff0ac3757b3964 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Thu, 26 Jun 2025 13:21:00 +0200 Subject: [PATCH 02/12] [PM-22593] improve initialization process for database and repositories (#329) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 🎟ī¸ Tracking ## 📔 Objective ## ⏰ Reminders before review - Contributor guidelines followed - All formatters and local linters executed and passed - Written new unit and / or integration tests where applicable - Protected functional changes with optionality (feature flags) - Used internationalization (i18n) for all UI strings - CI builds passed - Communicated to DevOps any deployment requirements - Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team ## đŸĻŽ Reviewer guidelines - 👍 (`:+1:`) or similar for great changes - 📝 (`:memo:`) or ℹī¸ (`:information_source:`) for notes or general info - ❓ (`:question:`) for questions - 🤔 (`:thinking:`) or 💭 (`:thought_balloon:`) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion - 🎨 (`:art:`) for suggestions / improvements - ❌ (`:x:`) or ⚠ī¸ (`:warning:`) for more significant problems or concerns needing attention - 🌱 (`:seedling:`) or â™ģī¸ (`:recycle:`) for future improvements or indications of technical debt - ⛏ (`:pick:`) for minor or nitpick changes --- .../src/platform/state_client.rs | 4 +++- crates/bitwarden-state/src/lib.rs | 2 ++ crates/bitwarden-state/src/registry.rs | 5 +++-- .../src/sdk_managed/configuration.rs | 21 +++++++++++++++++++ .../src/sdk_managed/indexed_db.rs | 11 ++++++++-- crates/bitwarden-state/src/sdk_managed/mod.rs | 11 +++++++++- .../bitwarden-state/src/sdk_managed/sqlite.rs | 18 +++++++++++++--- crates/bitwarden-uniffi/src/platform/mod.rs | 20 ++++++++++++++++-- .../src/platform/mod.rs | 21 +++++++++++++++++-- 9 files changed, 100 insertions(+), 13 deletions(-) create mode 100644 crates/bitwarden-state/src/sdk_managed/configuration.rs diff --git a/crates/bitwarden-core/src/platform/state_client.rs b/crates/bitwarden-core/src/platform/state_client.rs index 33a6807f5..a5a0daef2 100644 --- a/crates/bitwarden-core/src/platform/state_client.rs +++ b/crates/bitwarden-core/src/platform/state_client.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use bitwarden_state::{ registry::StateRegistryError, repository::{Repository, RepositoryItem, RepositoryItemData}, + DatabaseConfiguration, }; use crate::Client; @@ -32,12 +33,13 @@ impl StateClient { /// Initialize the database for SDK managed repositories. pub async fn initialize_database( &self, + configuration: DatabaseConfiguration, repositories: Vec, ) -> Result<(), StateRegistryError> { self.client .internal .repository_map - .initialize_database(repositories) + .initialize_database(configuration, repositories) .await } diff --git a/crates/bitwarden-state/src/lib.rs b/crates/bitwarden-state/src/lib.rs index fe982ee46..ec172cdd1 100644 --- a/crates/bitwarden-state/src/lib.rs +++ b/crates/bitwarden-state/src/lib.rs @@ -7,3 +7,5 @@ pub mod repository; pub mod registry; pub(crate) mod sdk_managed; + +pub use sdk_managed::DatabaseConfiguration; diff --git a/crates/bitwarden-state/src/registry.rs b/crates/bitwarden-state/src/registry.rs index 1645aaf0c..407f9c758 100644 --- a/crates/bitwarden-state/src/registry.rs +++ b/crates/bitwarden-state/src/registry.rs @@ -10,7 +10,7 @@ use thiserror::Error; use crate::{ repository::{Repository, RepositoryItem, RepositoryItemData}, - sdk_managed::{Database, SystemDatabase}, + sdk_managed::{Database, DatabaseConfiguration, SystemDatabase}, }; /// A registry that contains repositories for different types of items. @@ -65,6 +65,7 @@ impl StateRegistry { /// Initializes the database used for sdk-managed repositories. pub async fn initialize_database( &self, + configuration: DatabaseConfiguration, repositories: Vec, ) -> Result<(), StateRegistryError> { if self.database.get().is_some() { @@ -72,7 +73,7 @@ impl StateRegistry { } let _ = self .database - .set(SystemDatabase::initialize(&repositories).await?); + .set(SystemDatabase::initialize(configuration, &repositories).await?); *self .sdk_managed diff --git a/crates/bitwarden-state/src/sdk_managed/configuration.rs b/crates/bitwarden-state/src/sdk_managed/configuration.rs new file mode 100644 index 000000000..622e476af --- /dev/null +++ b/crates/bitwarden-state/src/sdk_managed/configuration.rs @@ -0,0 +1,21 @@ +use std::path::PathBuf; + +#[derive(Debug)] +/// Configuration for the database used by the SDK. +pub enum DatabaseConfiguration { + /// SQLite configuration, used on native platforms + Sqlite { + /// The name of the SQLite database. Different users should have different database + /// names to avoid conflicts. + db_name: String, + /// The path to the folder in which the SQLite database should be stored. + folder_path: PathBuf, + }, + + /// IndexedDB configuration, used on WebAssembly platforms + IndexedDb { + /// The name of the IndexedDB database. Different users should have different database + /// names to avoid conflicts. + db_name: String, + }, +} diff --git a/crates/bitwarden-state/src/sdk_managed/indexed_db.rs b/crates/bitwarden-state/src/sdk_managed/indexed_db.rs index 6e734ef03..ec5c127a8 100644 --- a/crates/bitwarden-state/src/sdk_managed/indexed_db.rs +++ b/crates/bitwarden-state/src/sdk_managed/indexed_db.rs @@ -3,7 +3,7 @@ use serde::{de::DeserializeOwned, ser::Serialize}; use crate::{ repository::{RepositoryItem, RepositoryItemData}, - sdk_managed::{Database, DatabaseError}, + sdk_managed::{Database, DatabaseConfiguration, DatabaseError}, }; #[derive(Debug, thiserror::Error)] @@ -20,7 +20,14 @@ pub struct IndexedDbDatabase( bitwarden_threading::ThreadBoundRunner>, ); impl Database for IndexedDbDatabase { - async fn initialize(registrations: &[RepositoryItemData]) -> Result { + async fn initialize( + configuration: DatabaseConfiguration, + registrations: &[RepositoryItemData], + ) -> Result { + let DatabaseConfiguration::IndexedDb { db_name: _db_name } = configuration else { + return Err(DatabaseError::UnsupportedConfiguration(configuration)); + }; + let factory = indexed_db::Factory::get()?; let registrations = registrations.to_vec(); diff --git a/crates/bitwarden-state/src/sdk_managed/mod.rs b/crates/bitwarden-state/src/sdk_managed/mod.rs index cfd4417be..637230848 100644 --- a/crates/bitwarden-state/src/sdk_managed/mod.rs +++ b/crates/bitwarden-state/src/sdk_managed/mod.rs @@ -4,6 +4,9 @@ use thiserror::Error; use crate::repository::{Repository, RepositoryError, RepositoryItem, RepositoryItemData}; +mod configuration; +pub use configuration::DatabaseConfiguration; + #[cfg(target_arch = "wasm32")] mod indexed_db; #[cfg(target_arch = "wasm32")] @@ -21,6 +24,9 @@ type InternalError = ::rusqlite::Error; #[bitwarden_error(flat)] #[derive(Debug, Error)] pub enum DatabaseError { + #[error("Database not supported on this platform: {0:?}")] + UnsupportedConfiguration(DatabaseConfiguration), + #[error(transparent)] ThreadBoundRunner(#[from] bitwarden_threading::CallError), @@ -35,7 +41,10 @@ pub enum DatabaseError { } pub trait Database { - async fn initialize(registrations: &[RepositoryItemData]) -> Result + async fn initialize( + configuration: DatabaseConfiguration, + registrations: &[RepositoryItemData], + ) -> Result where Self: Sized; diff --git a/crates/bitwarden-state/src/sdk_managed/sqlite.rs b/crates/bitwarden-state/src/sdk_managed/sqlite.rs index 32a62f41c..2ead3fd9b 100644 --- a/crates/bitwarden-state/src/sdk_managed/sqlite.rs +++ b/crates/bitwarden-state/src/sdk_managed/sqlite.rs @@ -5,15 +5,27 @@ use tokio::sync::Mutex; use crate::{ repository::{RepositoryItem, RepositoryItemData}, - sdk_managed::{Database, DatabaseError}, + sdk_managed::{Database, DatabaseConfiguration, DatabaseError}, }; // TODO: Use connection pooling with r2d2 and r2d2_sqlite? #[derive(Clone)] pub struct SqliteDatabase(Arc>); impl Database for SqliteDatabase { - async fn initialize(registrations: &[RepositoryItemData]) -> Result { - let mut db = rusqlite::Connection::open_in_memory()?; + async fn initialize( + configuration: DatabaseConfiguration, + registrations: &[RepositoryItemData], + ) -> Result { + let DatabaseConfiguration::Sqlite { + db_name, + folder_path: mut path, + } = configuration + else { + return Err(DatabaseError::UnsupportedConfiguration(configuration)); + }; + path.set_file_name(format!("{}.sqlite", db_name)); + + let mut db = rusqlite::Connection::open(path)?; // Set WAL mode for better concurrency db.execute("PRAGMA journal_mode = WAL;", [])?; diff --git a/crates/bitwarden-uniffi/src/platform/mod.rs b/crates/bitwarden-uniffi/src/platform/mod.rs index b0e4fc074..6050733a4 100644 --- a/crates/bitwarden-uniffi/src/platform/mod.rs +++ b/crates/bitwarden-uniffi/src/platform/mod.rs @@ -2,7 +2,7 @@ use std::sync::Arc; use bitwarden_core::{platform::FingerprintRequest, Client}; use bitwarden_fido::ClientFido2Ext; -use bitwarden_state::repository::RepositoryItem; +use bitwarden_state::{repository::RepositoryItem, DatabaseConfiguration}; use bitwarden_vault::Cipher; use repository::UniffiRepositoryBridge; @@ -55,10 +55,17 @@ pub struct StateClient(Client); repository::create_uniffi_repository!(CipherRepository, Cipher); +#[derive(uniffi::Record)] +pub struct SqliteConfiguration { + db_name: String, + folder_path: String, +} + #[uniffi::export] impl StateClient { pub async fn initialize_state( &self, + configuration: SqliteConfiguration, cipher_repository: Arc, ) -> Result<()> { let cipher = UniffiRepositoryBridge::new(cipher_repository); @@ -72,9 +79,18 @@ impl StateClient { self.0 .platform() .state() - .initialize_database(sdk_managed_repositories) + .initialize_database(configuration.into(), sdk_managed_repositories) .await .map_err(Error::StateRegistry)?; Ok(()) } } + +impl From for DatabaseConfiguration { + fn from(config: SqliteConfiguration) -> Self { + DatabaseConfiguration::Sqlite { + db_name: config.db_name, + folder_path: config.folder_path.into(), + } + } +} diff --git a/crates/bitwarden-wasm-internal/src/platform/mod.rs b/crates/bitwarden-wasm-internal/src/platform/mod.rs index 29d4fc004..6a5c26d29 100644 --- a/crates/bitwarden-wasm-internal/src/platform/mod.rs +++ b/crates/bitwarden-wasm-internal/src/platform/mod.rs @@ -1,6 +1,8 @@ use bitwarden_core::Client; -use bitwarden_state::repository::RepositoryItem; +use bitwarden_state::{repository::RepositoryItem, DatabaseConfiguration}; use bitwarden_vault::Cipher; +use serde::{Deserialize, Serialize}; +use tsify_next::Tsify; use wasm_bindgen::prelude::wasm_bindgen; mod repository; @@ -32,10 +34,17 @@ impl StateClient { repository::create_wasm_repository!(CipherRepository, Cipher, "Repository"); +#[derive(Serialize, Deserialize, Tsify)] +#[tsify(into_wasm_abi, from_wasm_abi)] +pub struct IndexedDbConfiguration { + pub db_name: String, +} + #[wasm_bindgen] impl StateClient { pub async fn initialize_state( &self, + configuration: IndexedDbConfiguration, cipher_repository: CipherRepository, ) -> Result<(), bitwarden_state::registry::StateRegistryError> { let cipher = cipher_repository.into_channel_impl(); @@ -49,7 +58,15 @@ impl StateClient { self.0 .platform() .state() - .initialize_database(sdk_managed_repositories) + .initialize_database(configuration.into(), sdk_managed_repositories) .await } } + +impl From for DatabaseConfiguration { + fn from(config: IndexedDbConfiguration) -> Self { + bitwarden_state::DatabaseConfiguration::IndexedDb { + db_name: config.db_name, + } + } +} From 934625af2d514b2bdfbc038739399c426987e435 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garci=CC=81a?= Date: Mon, 25 Aug 2025 14:32:37 +0200 Subject: [PATCH 03/12] Split database init from client managed repo for now --- crates/bitwarden-uniffi/src/platform/mod.rs | 10 ++++------ crates/bitwarden-wasm-internal/src/platform/mod.rs | 9 +++++---- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/crates/bitwarden-uniffi/src/platform/mod.rs b/crates/bitwarden-uniffi/src/platform/mod.rs index 6050733a4..439bada77 100644 --- a/crates/bitwarden-uniffi/src/platform/mod.rs +++ b/crates/bitwarden-uniffi/src/platform/mod.rs @@ -63,14 +63,12 @@ pub struct SqliteConfiguration { #[uniffi::export] impl StateClient { - pub async fn initialize_state( - &self, - configuration: SqliteConfiguration, - cipher_repository: Arc, - ) -> Result<()> { - let cipher = UniffiRepositoryBridge::new(cipher_repository); + pub fn register_cipher_repository(&self, repository: Arc) { + let cipher = UniffiRepositoryBridge::new(repository); self.0.platform().state().register_client_managed(cipher); + } + pub async fn initialize_state(&self, configuration: SqliteConfiguration) -> Result<()> { let sdk_managed_repositories = vec![ // This should list all the SDK-managed repositories ::data(), diff --git a/crates/bitwarden-wasm-internal/src/platform/mod.rs b/crates/bitwarden-wasm-internal/src/platform/mod.rs index b44e6df97..316275660 100644 --- a/crates/bitwarden-wasm-internal/src/platform/mod.rs +++ b/crates/bitwarden-wasm-internal/src/platform/mod.rs @@ -59,14 +59,15 @@ pub struct IndexedDbConfiguration { #[wasm_bindgen] impl StateClient { + pub fn register_cipher_repository(&self, cipher_repository: CipherRepository) { + let cipher = cipher_repository.into_channel_impl(); + self.0.platform().state().register_client_managed(cipher); + } + pub async fn initialize_state( &self, configuration: IndexedDbConfiguration, - cipher_repository: CipherRepository, ) -> Result<(), bitwarden_state::registry::StateRegistryError> { - let cipher = cipher_repository.into_channel_impl(); - self.0.platform().state().register_client_managed(cipher); - let sdk_managed_repositories = vec![ // This should list all the SDK-managed repositories ::data(), From 4717df8338b3344aa538c9a243f211282b56d1e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garci=CC=81a?= Date: Mon, 25 Aug 2025 15:05:08 +0200 Subject: [PATCH 04/12] Update dependency --- Cargo.lock | 8 ++++---- crates/bitwarden-state/Cargo.toml | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b07baa0b5..9cea51638 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2531,9 +2531,9 @@ checksum = "f9fbbcab51052fe104eb5e5d351cf728d30a5be1fe14d9be8a3b097481fb97de" [[package]] name = "libsqlite3-sys" -version = "0.34.0" +version = "0.35.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "91632f3b4fb6bd1d72aa3d78f41ffecfcf2b1a6648d8c241dbe7dbfaf4875e15" +checksum = "133c182a6a2c87864fe97778797e46c7e999672690dc9fa3ee8e241aa4a9c13f" dependencies = [ "cc", "pkg-config", @@ -3474,9 +3474,9 @@ dependencies = [ [[package]] name = "rusqlite" -version = "0.36.0" +version = "0.37.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3de23c3319433716cf134eed225fe9986bc24f63bed9be9f20c329029e672dc7" +checksum = "165ca6e57b20e1351573e3729b958bc62f0e48025386970b6e4d29e7a7e71f3f" dependencies = [ "bitflags 2.9.1", "fallible-iterator", diff --git a/crates/bitwarden-state/Cargo.toml b/crates/bitwarden-state/Cargo.toml index 349a6a6df..11ec5eb1a 100644 --- a/crates/bitwarden-state/Cargo.toml +++ b/crates/bitwarden-state/Cargo.toml @@ -28,7 +28,7 @@ js-sys = { workspace = true } tsify = { workspace = true } [target.'cfg(not(target_arch="wasm32"))'.dependencies] -rusqlite = { version = ">=0.36.0, <0.37", features = ["bundled"] } +rusqlite = { version = ">=0.37.0, <0.38", features = ["bundled"] } [dev-dependencies] tokio = { workspace = true, features = ["rt"] } From 1275e7038fb8347ec3fc36798b8dd91b292d2ff8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garci=CC=81a?= Date: Tue, 26 Aug 2025 12:34:02 +0200 Subject: [PATCH 05/12] Remove versioning --- crates/bitwarden-state/README.md | 2 +- crates/bitwarden-state/src/registry.rs | 6 +++--- crates/bitwarden-state/src/repository.rs | 8 +------- crates/bitwarden-state/src/sdk_managed/indexed_db.rs | 7 +++---- crates/bitwarden-vault/src/cipher/cipher.rs | 2 +- crates/bitwarden-vault/src/folder/folder_models.rs | 2 +- 6 files changed, 10 insertions(+), 17 deletions(-) diff --git a/crates/bitwarden-state/README.md b/crates/bitwarden-state/README.md index 82f4358f7..42e091af3 100644 --- a/crates/bitwarden-state/README.md +++ b/crates/bitwarden-state/README.md @@ -14,7 +14,7 @@ struct Cipher { // Register `Cipher` for use with a `Repository`. // This should be done in the crate where `Cipher` is defined. -bitwarden_state::register_repository_item!(Cipher, "Cipher", version: 1); +bitwarden_state::register_repository_item!(Cipher, "Cipher"); ``` With the registration complete, the next important decision is to select where will the data be diff --git a/crates/bitwarden-state/src/registry.rs b/crates/bitwarden-state/src/registry.rs index 215dcb289..bd213e702 100644 --- a/crates/bitwarden-state/src/registry.rs +++ b/crates/bitwarden-state/src/registry.rs @@ -158,9 +158,9 @@ mod tests { #[derive(PartialEq, Eq, Debug)] struct TestItem(T); - register_repository_item!(TestItem, "TestItem", version: 1); - register_repository_item!(TestItem, "TestItem", version: 1); - register_repository_item!(TestItem>, "TestItem>", version: 1); + register_repository_item!(TestItem, "TestItem"); + register_repository_item!(TestItem, "TestItem"); + register_repository_item!(TestItem>, "TestItem>"); impl_repository!(TestA, TestItem); impl_repository!(TestB, TestItem); diff --git a/crates/bitwarden-state/src/repository.rs b/crates/bitwarden-state/src/repository.rs index c0e73d0ec..4df46e9a7 100644 --- a/crates/bitwarden-state/src/repository.rs +++ b/crates/bitwarden-state/src/repository.rs @@ -43,9 +43,6 @@ pub trait RepositoryItem: Internal + Send + Sync + 'static { /// The name of the type implementing this trait. const NAME: &'static str; - /// The version of the repository type implementing this trait. - const VERSION: u32; - /// Returns the `TypeId` of the type implementing this trait. fn type_id() -> TypeId { TypeId::of::() @@ -63,7 +60,6 @@ pub trait RepositoryItem: Internal + Send + Sync + 'static { pub struct RepositoryItemData { pub(crate) type_id: TypeId, pub(crate) name: &'static str, - pub(crate) version: u32, } impl RepositoryItemData { @@ -72,7 +68,6 @@ impl RepositoryItemData { Self { type_id: TypeId::of::(), name: T::NAME, - version: T::VERSION, } } } @@ -81,12 +76,11 @@ impl RepositoryItemData { /// where it's defined. The provided name must be unique and not be changed. #[macro_export] macro_rules! register_repository_item { - ($ty:ty, $name:literal, version: $version:literal) => { + ($ty:ty, $name:literal) => { const _: () = { impl $crate::repository::___internal::Internal for $ty {} impl $crate::repository::RepositoryItem for $ty { const NAME: &'static str = $name; - const VERSION: u32 = $version; } }; }; diff --git a/crates/bitwarden-state/src/sdk_managed/indexed_db.rs b/crates/bitwarden-state/src/sdk_managed/indexed_db.rs index 5ab9dec00..be31e470f 100644 --- a/crates/bitwarden-state/src/sdk_managed/indexed_db.rs +++ b/crates/bitwarden-state/src/sdk_managed/indexed_db.rs @@ -32,10 +32,9 @@ impl Database for IndexedDbDatabase { let registrations = registrations.to_vec(); - // Sum all the versions of the registrations to determine the database version - // TODO: We should do a better versioning strategy, as this won't work if one repository is - // removed - let version: u32 = registrations.iter().map(|reg| reg.version).sum(); + // TODO: This version will be replaced by a proper migration system in a followup PR: + // https://github.com/bitwarden/sdk-internal/pull/410 + let version: u32 = 1; // Open the database, creating it if needed let db = factory diff --git a/crates/bitwarden-vault/src/cipher/cipher.rs b/crates/bitwarden-vault/src/cipher/cipher.rs index 9dda01d04..cc96a028e 100644 --- a/crates/bitwarden-vault/src/cipher/cipher.rs +++ b/crates/bitwarden-vault/src/cipher/cipher.rs @@ -141,7 +141,7 @@ pub struct Cipher { pub revision_date: DateTime, } -bitwarden_state::register_repository_item!(Cipher, "Cipher", version: 1); +bitwarden_state::register_repository_item!(Cipher, "Cipher"); #[allow(missing_docs)] #[derive(Serialize, Deserialize, Debug, Clone)] diff --git a/crates/bitwarden-vault/src/folder/folder_models.rs b/crates/bitwarden-vault/src/folder/folder_models.rs index 3c8ebc3ac..d82dcde4c 100644 --- a/crates/bitwarden-vault/src/folder/folder_models.rs +++ b/crates/bitwarden-vault/src/folder/folder_models.rs @@ -28,7 +28,7 @@ pub struct Folder { pub revision_date: DateTime, } -bitwarden_state::register_repository_item!(Folder, "Folder", version: 1); +bitwarden_state::register_repository_item!(Folder, "Folder"); #[allow(missing_docs)] #[derive(Serialize, Deserialize, Debug, PartialEq, Eq)] From 84c36e01a9d89a056619561c2fcba8931f2a883a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garci=CC=81a?= Date: Tue, 2 Sep 2025 16:29:25 +0200 Subject: [PATCH 06/12] Replace sqlite table placeholder with format, validate the names at compile time, use indexeddb db name --- crates/bitwarden-state/Cargo.toml | 2 +- crates/bitwarden-state/src/registry.rs | 6 +- crates/bitwarden-state/src/repository.rs | 55 ++++++++- .../src/sdk_managed/indexed_db.rs | 35 +++--- crates/bitwarden-state/src/sdk_managed/mod.rs | 16 +-- .../bitwarden-state/src/sdk_managed/sqlite.rs | 113 +++++++++++++----- 6 files changed, 164 insertions(+), 63 deletions(-) diff --git a/crates/bitwarden-state/Cargo.toml b/crates/bitwarden-state/Cargo.toml index 11ec5eb1a..c1c548792 100644 --- a/crates/bitwarden-state/Cargo.toml +++ b/crates/bitwarden-state/Cargo.toml @@ -11,7 +11,7 @@ keywords.workspace = true [features] uniffi = [] -wasm = [] +wasm = ["bitwarden-threading/wasm"] [dependencies] async-trait = { workspace = true } diff --git a/crates/bitwarden-state/src/registry.rs b/crates/bitwarden-state/src/registry.rs index bd213e702..b2c8950d5 100644 --- a/crates/bitwarden-state/src/registry.rs +++ b/crates/bitwarden-state/src/registry.rs @@ -158,9 +158,9 @@ mod tests { #[derive(PartialEq, Eq, Debug)] struct TestItem(T); - register_repository_item!(TestItem, "TestItem"); - register_repository_item!(TestItem, "TestItem"); - register_repository_item!(TestItem>, "TestItem>"); + register_repository_item!(TestItem, "TestItem_usize"); + register_repository_item!(TestItem, "TestItem_String"); + register_repository_item!(TestItem>, "TestItem_Vec"); impl_repository!(TestA, TestItem); impl_repository!(TestB, TestItem); diff --git a/crates/bitwarden-state/src/repository.rs b/crates/bitwarden-state/src/repository.rs index 4df46e9a7..8f218340e 100644 --- a/crates/bitwarden-state/src/repository.rs +++ b/crates/bitwarden-state/src/repository.rs @@ -58,8 +58,8 @@ pub trait RepositoryItem: Internal + Send + Sync + 'static { #[allow(dead_code)] #[derive(Debug, Clone, Copy)] pub struct RepositoryItemData { - pub(crate) type_id: TypeId, - pub(crate) name: &'static str, + type_id: TypeId, + name: &'static str, } impl RepositoryItemData { @@ -70,6 +70,32 @@ impl RepositoryItemData { name: T::NAME, } } + + /// Get the `TypeId` of the registered type. + pub fn type_id(&self) -> TypeId { + self.type_id + } + /// Get the name of the registered type. + /// This name is guaranteed to be a valid identifier. + pub fn name(&self) -> &'static str { + self.name + } +} + +/// Validate that the provided name will be a valid identifier at compile time. +/// This is intentionally limited to ensure compatibility with various storage backends. +pub const fn validate_registry_name(name: &str) -> bool { + let bytes = name.as_bytes(); + let mut i = 0; + while i < bytes.len() { + let byte = bytes[i]; + // Check if character is alphabetic (a-z, A-Z) or underscore + if !((byte >= b'a' && byte <= b'z') || (byte >= b'A' && byte <= b'Z') || byte == b'_') { + return false; + } + i += 1; + } + true } /// Register a type for use in a repository. The type must only be registered once in the crate @@ -82,6 +108,14 @@ macro_rules! register_repository_item { impl $crate::repository::RepositoryItem for $ty { const NAME: &'static str = $name; } + assert!( + $crate::repository::validate_registry_name($name), + concat!( + "Repository name '", + $name, + "' must contain only alphabetic characters and underscores" + ) + ) }; }; } @@ -91,7 +125,22 @@ macro_rules! register_repository_item { #[doc(hidden)] pub mod ___internal { - // This trait is just to try to discourage users from implementing `RepositoryItem` directly. + // This trait is forbid users from implementing `RepositoryItem` directly. pub trait Internal {} } pub(crate) use ___internal::Internal; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_validate_name() { + assert!(validate_registry_name("valid")); + assert!(validate_registry_name("Valid_Name")); + assert!(!validate_registry_name("Invalid-Name")); + assert!(!validate_registry_name("Invalid Name")); + assert!(!validate_registry_name("Invalid.Name")); + assert!(!validate_registry_name("Invalid123")); + } +} diff --git a/crates/bitwarden-state/src/sdk_managed/indexed_db.rs b/crates/bitwarden-state/src/sdk_managed/indexed_db.rs index be31e470f..ab75f746a 100644 --- a/crates/bitwarden-state/src/sdk_managed/indexed_db.rs +++ b/crates/bitwarden-state/src/sdk_managed/indexed_db.rs @@ -24,7 +24,7 @@ impl Database for IndexedDbDatabase { configuration: DatabaseConfiguration, registrations: &[RepositoryItemData], ) -> Result { - let DatabaseConfiguration::IndexedDb { db_name: _db_name } = configuration else { + let DatabaseConfiguration::IndexedDb { db_name } = configuration else { return Err(DatabaseError::UnsupportedConfiguration(configuration)); }; @@ -38,11 +38,11 @@ impl Database for IndexedDbDatabase { // Open the database, creating it if needed let db = factory - .open("bitwarden-sdk-test-db", version, async move |evt| { + .open(&db_name, version, async move |evt| { let db = evt.database(); for reg in registrations { - db.build_object_store(reg.name).create()?; + db.build_object_store(reg.name()).create()?; } Ok(()) @@ -55,18 +55,16 @@ impl Database for IndexedDbDatabase { async fn get( &self, - namespace: &str, key: &str, ) -> Result, DatabaseError> { - let namespace = namespace.to_string(); let key = key.to_string(); let result = self .0 .run_in_thread(move |db| async move { - db.transaction(&[&namespace]) + db.transaction(&[T::NAME]) .run(|t| async move { - let store = t.object_store(&namespace)?; + let store = t.object_store(T::NAME)?; let response = store.get(&JsString::from(key)).await?; if let Some(value) = response { @@ -85,16 +83,13 @@ impl Database for IndexedDbDatabase { async fn list( &self, - namespace: &str, ) -> Result, DatabaseError> { - let namespace = namespace.to_string(); - let results = self .0 .run_in_thread(move |db| async move { - db.transaction(&[&namespace]) + db.transaction(&[T::NAME]) .run(|t| async move { - let store = t.object_store(&namespace)?; + let store = t.object_store(T::NAME)?; let results = store.get_all(None).await?; let mut items: Vec = Vec::new(); @@ -116,19 +111,17 @@ impl Database for IndexedDbDatabase { async fn set( &self, - namespace: &str, key: &str, value: T, ) -> Result<(), DatabaseError> { - let namespace = namespace.to_string(); let key = key.to_string(); self.0 .run_in_thread(move |db| async move { - db.transaction(&[&namespace]) + db.transaction(&[T::NAME]) .rw() .run(|t| async move { - let store = t.object_store(&namespace)?; + let store = t.object_store(T::NAME)?; let value = ::tsify::serde_wasm_bindgen::to_value(&value) .map_err(IndexedDbInternalError::from)?; @@ -143,16 +136,18 @@ impl Database for IndexedDbDatabase { Ok(()) } - async fn remove(&self, namespace: &str, key: &str) -> Result<(), DatabaseError> { - let namespace = namespace.to_string(); + async fn remove( + &self, + key: &str, + ) -> Result<(), DatabaseError> { let key = key.to_string(); self.0 .run_in_thread(move |db| async move { - db.transaction(&[&namespace]) + db.transaction(&[T::NAME]) .rw() .run(|t| async move { - let store = t.object_store(&namespace)?; + let store = t.object_store(T::NAME)?; store.delete(&JsString::from(key)).await?; Ok(()) }) diff --git a/crates/bitwarden-state/src/sdk_managed/mod.rs b/crates/bitwarden-state/src/sdk_managed/mod.rs index 637230848..a5e6b9ddf 100644 --- a/crates/bitwarden-state/src/sdk_managed/mod.rs +++ b/crates/bitwarden-state/src/sdk_managed/mod.rs @@ -50,23 +50,23 @@ pub trait Database { async fn get( &self, - namespace: &str, key: &str, ) -> Result, DatabaseError>; async fn list( &self, - namespace: &str, ) -> Result, DatabaseError>; async fn set( &self, - namespace: &str, key: &str, value: T, ) -> Result<(), DatabaseError>; - async fn remove(&self, namespace: &str, key: &str) -> Result<(), DatabaseError>; + async fn remove( + &self, + key: &str, + ) -> Result<(), DatabaseError>; } struct DBRepository { @@ -77,18 +77,18 @@ struct DBRepository { #[async_trait::async_trait] impl Repository for DBRepository { async fn get(&self, key: String) -> Result, RepositoryError> { - let value = self.database.get(V::NAME, &key).await?; + let value = self.database.get::(&key).await?; Ok(value) } async fn list(&self) -> Result, RepositoryError> { - let values = self.database.list(V::NAME).await?; + let values = self.database.list::().await?; Ok(values) } async fn set(&self, key: String, value: V) -> Result<(), RepositoryError> { - Ok(self.database.set(V::NAME, &key, value).await?) + Ok(self.database.set::(&key, value).await?) } async fn remove(&self, key: String) -> Result<(), RepositoryError> { - Ok(self.database.remove(V::NAME, &key).await?) + Ok(self.database.remove::(&key).await?) } } diff --git a/crates/bitwarden-state/src/sdk_managed/sqlite.rs b/crates/bitwarden-state/src/sdk_managed/sqlite.rs index 702353d77..a85d15a38 100644 --- a/crates/bitwarden-state/src/sdk_managed/sqlite.rs +++ b/crates/bitwarden-state/src/sdk_managed/sqlite.rs @@ -11,6 +11,36 @@ use crate::{ // TODO: Use connection pooling with r2d2 and r2d2_sqlite? #[derive(Clone)] pub struct SqliteDatabase(Arc>); + +impl SqliteDatabase { + fn initialize_internal( + mut db: rusqlite::Connection, + registrations: &[RepositoryItemData], + ) -> Result { + // Set WAL mode for better concurrency + db.pragma_update(None, "journal_mode", "WAL")?; + + let transaction = db.transaction()?; + + for reg in registrations { + // SAFETY: SQLite tables cannot use ?, but `reg.name()` is not user controlled and + // is validated to only contain valid characters, so it's safe to + // interpolate here. + + transaction.execute( + &format!( + "CREATE TABLE IF NOT EXISTS {} (key TEXT PRIMARY KEY, value TEXT NOT NULL);", + reg.name(), + ), + [], + )?; + } + + transaction.commit()?; + Ok(SqliteDatabase(Arc::new(Mutex::new(db)))) + } +} + impl Database for SqliteDatabase { async fn initialize( configuration: DatabaseConfiguration, @@ -25,32 +55,20 @@ impl Database for SqliteDatabase { }; path.set_file_name(format!("{db_name}.sqlite")); - let mut db = rusqlite::Connection::open(path)?; - - // Set WAL mode for better concurrency - db.execute("PRAGMA journal_mode = WAL;", [])?; - - let transaction = db.transaction()?; - - for reg in registrations { - transaction.execute( - "CREATE TABLE IF NOT EXISTS ?1 (key TEXT PRIMARY KEY, value TEXT NOT NULL);", - [reg.name], - )?; - } - - transaction.commit()?; - Ok(SqliteDatabase(Arc::new(Mutex::new(db)))) + let db = rusqlite::Connection::open(path)?; + Self::initialize_internal(db, registrations) } async fn get( &self, - namespace: &str, key: &str, ) -> Result, DatabaseError> { let conn = self.0.lock().await; - let mut stmt = conn.prepare("SELECT value FROM ?1 WHERE key = ?2")?; - let mut rows = stmt.query(rusqlite::params![namespace, key])?; + + // SAFETY: SQLite tables cannot use ?, but `T::NAME` is not user controlled and is + // validated to only contain valid characters, so it's safe to interpolate here. + let mut stmt = conn.prepare(&format!("SELECT value FROM {} WHERE key = ?1", T::NAME))?; + let mut rows = stmt.query(rusqlite::params![key])?; if let Some(row) = rows.next()? { let value = row.get::<_, String>(0)?; @@ -63,11 +81,13 @@ impl Database for SqliteDatabase { async fn list( &self, - namespace: &str, ) -> Result, DatabaseError> { let conn = self.0.lock().await; - let mut stmt = conn.prepare("SELECT key, value FROM ?1")?; - let rows = stmt.query_map(rusqlite::params![namespace], |row| row.get(1))?; + + // SAFETY: SQLite tables cannot use ?, but `T::NAME` is not user controlled and is + // validated to only contain valid characters, so it's safe to interpolate here. + let mut stmt = conn.prepare(&format!("SELECT key, value FROM {}", T::NAME))?; + let rows = stmt.query_map([], |row| row.get(1))?; let mut results = Vec::new(); for row in rows { @@ -81,7 +101,6 @@ impl Database for SqliteDatabase { async fn set( &self, - namespace: &str, key: &str, value: T, ) -> Result<(), DatabaseError> { @@ -90,25 +109,63 @@ impl Database for SqliteDatabase { let value = serde_json::to_string(&value)?; + // SAFETY: SQLite tables cannot use ?, but `T::NAME` is not user controlled and is + // validated to only contain valid characters, so it's safe to interpolate here. transaction.execute( - "INSERT OR REPLACE INTO ?1 (key, value) VALUES (?2, ?3)", - rusqlite::params![namespace, key, value], + &format!( + "INSERT OR REPLACE INTO {} (key, value) VALUES (?1, ?2)", + T::NAME, + ), + rusqlite::params![key, value], )?; transaction.commit()?; Ok(()) } - async fn remove(&self, namespace: &str, key: &str) -> Result<(), DatabaseError> { + async fn remove( + &self, + key: &str, + ) -> Result<(), DatabaseError> { let mut conn = self.0.lock().await; let transaction = conn.transaction()?; + // SAFETY: SQLite tables cannot use ?, but `T::NAME` is not user controlled and is + // validated to only contain valid characters, so it's safe to interpolate here. transaction.execute( - "DELETE FROM ?1 WHERE key = ?2", - rusqlite::params![namespace, key], + &format!("DELETE FROM {} WHERE key = ?1", T::NAME), + rusqlite::params![key], )?; transaction.commit()?; Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::register_repository_item; + + #[tokio::test] + async fn test_sqlite_integration() { + let db = rusqlite::Connection::open_in_memory().unwrap(); + + #[derive(Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)] + struct TestA(usize); + register_repository_item!(TestA, "TestItem_A"); + + let registrations = vec![TestA::data()]; + + let db = SqliteDatabase::initialize_internal(db, ®istrations).unwrap(); + + assert_eq!(db.list::().await.unwrap(), Vec::::new()); + + db.set("key1", TestA(42)).await.unwrap(); + assert_eq!(db.get::("key1").await.unwrap(), Some(TestA(42))); + + db.remove::("key1").await.unwrap(); + + assert_eq!(db.get::("key1").await.unwrap(), None); + } +} From 269c2d388cb195d457fe2e95127fcb85ee9ca219 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garci=CC=81a?= Date: Tue, 2 Sep 2025 16:36:02 +0200 Subject: [PATCH 07/12] Remove unnecessary params macro --- crates/bitwarden-state/src/sdk_managed/sqlite.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/crates/bitwarden-state/src/sdk_managed/sqlite.rs b/crates/bitwarden-state/src/sdk_managed/sqlite.rs index a85d15a38..4e98340b3 100644 --- a/crates/bitwarden-state/src/sdk_managed/sqlite.rs +++ b/crates/bitwarden-state/src/sdk_managed/sqlite.rs @@ -68,7 +68,7 @@ impl Database for SqliteDatabase { // SAFETY: SQLite tables cannot use ?, but `T::NAME` is not user controlled and is // validated to only contain valid characters, so it's safe to interpolate here. let mut stmt = conn.prepare(&format!("SELECT value FROM {} WHERE key = ?1", T::NAME))?; - let mut rows = stmt.query(rusqlite::params![key])?; + let mut rows = stmt.query([key])?; if let Some(row) = rows.next()? { let value = row.get::<_, String>(0)?; @@ -116,7 +116,7 @@ impl Database for SqliteDatabase { "INSERT OR REPLACE INTO {} (key, value) VALUES (?1, ?2)", T::NAME, ), - rusqlite::params![key, value], + [key, &value], )?; transaction.commit()?; @@ -132,10 +132,7 @@ impl Database for SqliteDatabase { // SAFETY: SQLite tables cannot use ?, but `T::NAME` is not user controlled and is // validated to only contain valid characters, so it's safe to interpolate here. - transaction.execute( - &format!("DELETE FROM {} WHERE key = ?1", T::NAME), - rusqlite::params![key], - )?; + transaction.execute(&format!("DELETE FROM {} WHERE key = ?1", T::NAME), [key])?; transaction.commit()?; Ok(()) From a5635630abf19a975b4818f04384cd22dcdbadff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garci=CC=81a?= Date: Tue, 2 Sep 2025 16:58:43 +0200 Subject: [PATCH 08/12] Fix comment and add quotes to sqlite --- crates/bitwarden-state/src/repository.rs | 3 ++- crates/bitwarden-state/src/sdk_managed/sqlite.rs | 10 +++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/bitwarden-state/src/repository.rs b/crates/bitwarden-state/src/repository.rs index 8f218340e..81ca7a8a4 100644 --- a/crates/bitwarden-state/src/repository.rs +++ b/crates/bitwarden-state/src/repository.rs @@ -125,7 +125,8 @@ macro_rules! register_repository_item { #[doc(hidden)] pub mod ___internal { - // This trait is forbid users from implementing `RepositoryItem` directly. + // This trait is in an internal module to try to forbid users from implementing `RepositoryItem` + // directly. pub trait Internal {} } pub(crate) use ___internal::Internal; diff --git a/crates/bitwarden-state/src/sdk_managed/sqlite.rs b/crates/bitwarden-state/src/sdk_managed/sqlite.rs index 4e98340b3..18d9e6217 100644 --- a/crates/bitwarden-state/src/sdk_managed/sqlite.rs +++ b/crates/bitwarden-state/src/sdk_managed/sqlite.rs @@ -29,7 +29,7 @@ impl SqliteDatabase { transaction.execute( &format!( - "CREATE TABLE IF NOT EXISTS {} (key TEXT PRIMARY KEY, value TEXT NOT NULL);", + "CREATE TABLE IF NOT EXISTS \"{}\" (key TEXT PRIMARY KEY, value TEXT NOT NULL);", reg.name(), ), [], @@ -67,7 +67,7 @@ impl Database for SqliteDatabase { // SAFETY: SQLite tables cannot use ?, but `T::NAME` is not user controlled and is // validated to only contain valid characters, so it's safe to interpolate here. - let mut stmt = conn.prepare(&format!("SELECT value FROM {} WHERE key = ?1", T::NAME))?; + let mut stmt = conn.prepare(&format!("SELECT value FROM \"{}\" WHERE key = ?1", T::NAME))?; let mut rows = stmt.query([key])?; if let Some(row) = rows.next()? { @@ -86,7 +86,7 @@ impl Database for SqliteDatabase { // SAFETY: SQLite tables cannot use ?, but `T::NAME` is not user controlled and is // validated to only contain valid characters, so it's safe to interpolate here. - let mut stmt = conn.prepare(&format!("SELECT key, value FROM {}", T::NAME))?; + let mut stmt = conn.prepare(&format!("SELECT key, value FROM \"{}\"", T::NAME))?; let rows = stmt.query_map([], |row| row.get(1))?; let mut results = Vec::new(); @@ -113,7 +113,7 @@ impl Database for SqliteDatabase { // validated to only contain valid characters, so it's safe to interpolate here. transaction.execute( &format!( - "INSERT OR REPLACE INTO {} (key, value) VALUES (?1, ?2)", + "INSERT OR REPLACE INTO \"{}\" (key, value) VALUES (?1, ?2)", T::NAME, ), [key, &value], @@ -132,7 +132,7 @@ impl Database for SqliteDatabase { // SAFETY: SQLite tables cannot use ?, but `T::NAME` is not user controlled and is // validated to only contain valid characters, so it's safe to interpolate here. - transaction.execute(&format!("DELETE FROM {} WHERE key = ?1", T::NAME), [key])?; + transaction.execute(&format!("DELETE FROM \"{}\" WHERE key = ?1", T::NAME), [key])?; transaction.commit()?; Ok(()) From e82dd8f4023fed47abee751f7d704039695d348d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garci=CC=81a?= Date: Tue, 2 Sep 2025 17:03:39 +0200 Subject: [PATCH 09/12] Add explicit validation --- .../bitwarden-state/src/sdk_managed/sqlite.rs | 35 +++++++++++++++---- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/crates/bitwarden-state/src/sdk_managed/sqlite.rs b/crates/bitwarden-state/src/sdk_managed/sqlite.rs index 18d9e6217..190ace8e5 100644 --- a/crates/bitwarden-state/src/sdk_managed/sqlite.rs +++ b/crates/bitwarden-state/src/sdk_managed/sqlite.rs @@ -4,7 +4,7 @@ use serde::{de::DeserializeOwned, ser::Serialize}; use tokio::sync::Mutex; use crate::{ - repository::{RepositoryItem, RepositoryItemData}, + repository::{validate_registry_name, RepositoryItem, RepositoryItemData}, sdk_managed::{Database, DatabaseConfiguration, DatabaseError}, }; @@ -12,6 +12,16 @@ use crate::{ #[derive(Clone)] pub struct SqliteDatabase(Arc>); +fn validate_identifier(name: &'static str) -> Result<&'static str, DatabaseError> { + if validate_registry_name(name) { + Ok(name) + } else { + Err(DatabaseError::Internal( + rusqlite::Error::InvalidParameterName(name.to_string()), + )) + } +} + impl SqliteDatabase { fn initialize_internal( mut db: rusqlite::Connection, @@ -26,11 +36,10 @@ impl SqliteDatabase { // SAFETY: SQLite tables cannot use ?, but `reg.name()` is not user controlled and // is validated to only contain valid characters, so it's safe to // interpolate here. - transaction.execute( &format!( "CREATE TABLE IF NOT EXISTS \"{}\" (key TEXT PRIMARY KEY, value TEXT NOT NULL);", - reg.name(), + validate_identifier(reg.name())?, ), [], )?; @@ -67,7 +76,10 @@ impl Database for SqliteDatabase { // SAFETY: SQLite tables cannot use ?, but `T::NAME` is not user controlled and is // validated to only contain valid characters, so it's safe to interpolate here. - let mut stmt = conn.prepare(&format!("SELECT value FROM \"{}\" WHERE key = ?1", T::NAME))?; + let mut stmt = conn.prepare(&format!( + "SELECT value FROM \"{}\" WHERE key = ?1", + validate_identifier(T::NAME)? + ))?; let mut rows = stmt.query([key])?; if let Some(row) = rows.next()? { @@ -86,7 +98,10 @@ impl Database for SqliteDatabase { // SAFETY: SQLite tables cannot use ?, but `T::NAME` is not user controlled and is // validated to only contain valid characters, so it's safe to interpolate here. - let mut stmt = conn.prepare(&format!("SELECT key, value FROM \"{}\"", T::NAME))?; + let mut stmt = conn.prepare(&format!( + "SELECT key, value FROM \"{}\"", + validate_identifier(T::NAME)? + ))?; let rows = stmt.query_map([], |row| row.get(1))?; let mut results = Vec::new(); @@ -114,7 +129,7 @@ impl Database for SqliteDatabase { transaction.execute( &format!( "INSERT OR REPLACE INTO \"{}\" (key, value) VALUES (?1, ?2)", - T::NAME, + validate_identifier(T::NAME)?, ), [key, &value], )?; @@ -132,7 +147,13 @@ impl Database for SqliteDatabase { // SAFETY: SQLite tables cannot use ?, but `T::NAME` is not user controlled and is // validated to only contain valid characters, so it's safe to interpolate here. - transaction.execute(&format!("DELETE FROM \"{}\" WHERE key = ?1", T::NAME), [key])?; + transaction.execute( + &format!( + "DELETE FROM \"{}\" WHERE key = ?1", + validate_identifier(T::NAME)? + ), + [key], + )?; transaction.commit()?; Ok(()) From 76b9f2d5b3d0af56ea7c21b46a13efcbd0e2b731 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garci=CC=81a?= Date: Tue, 2 Sep 2025 17:15:29 +0200 Subject: [PATCH 10/12] Expand validate comment --- crates/bitwarden-state/src/repository.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/bitwarden-state/src/repository.rs b/crates/bitwarden-state/src/repository.rs index 81ca7a8a4..7f956dc3e 100644 --- a/crates/bitwarden-state/src/repository.rs +++ b/crates/bitwarden-state/src/repository.rs @@ -83,7 +83,9 @@ impl RepositoryItemData { } /// Validate that the provided name will be a valid identifier at compile time. -/// This is intentionally limited to ensure compatibility with various storage backends. +/// This is intentionally limited to ensure compatibility with current and future storage backends. +/// For example, SQLite tables must not begin with a number or contain special characters. +/// Valid characters are a-z, A-Z, and underscore (_). pub const fn validate_registry_name(name: &str) -> bool { let bytes = name.as_bytes(); let mut i = 0; From a763767e6546e3bcf29f20ffddc60dd8b1d900f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garci=CC=81a?= Date: Wed, 3 Sep 2025 14:28:47 +0200 Subject: [PATCH 11/12] Add clone comment and document that client/sdk state are not mutually exclusive --- crates/bitwarden-state/README.md | 14 +++++++++----- .../bitwarden-threading/src/thread_bound_runner.rs | 5 +++++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/crates/bitwarden-state/README.md b/crates/bitwarden-state/README.md index 42e091af3..90b8029ee 100644 --- a/crates/bitwarden-state/README.md +++ b/crates/bitwarden-state/README.md @@ -27,6 +27,10 @@ stored: - If the SDK itself will handle data storage, we call that approach `SDK-Managed State`. The implementation of this is will a work in progress. +Note that these approaches aren't mutually exclusive: a repository item can use both client and SDK +managed state at the same time. However, this mixed approach is only recommended during migration +scenarios to avoid potential confusion. + ## Client-Managed State With `Client-Managed State` the application and SDK will both access the same data pool, which @@ -53,7 +57,7 @@ impl StateClient { } ``` -#### How to use it on web clients +#### How to initialize Client-Managed State on the web clients Once we have the function defined in `bitwarden-wasm-internal`, we can use it from the web clients. For that, the first thing we need to do is create a mapper between the client and SDK types. This @@ -113,7 +117,7 @@ impl StateClient { } ``` -#### How to use it on iOS +#### How to initialize Client-Managed State on iOS Once we have the function defined in `bitwarden-uniffi`, we can use it from the iOS application: @@ -144,7 +148,7 @@ let store = CipherStoreImpl(cipherDataStore: self.cipherDataStore, userId: userI try await self.clientService.platform().store().registerCipherStore(store: store); ``` -### How to use it on Android +### How to initialize Client-Managed State on Android Once we have the function defined in `bitwarden-uniffi`, we can use it from the Android application: @@ -181,7 +185,7 @@ that the clients don't need to make any changes themselves, as the implementatio SDK. To add support for an SDK managed `Repository`, it needs to be added to the initialization code for WASM and UniFFI. This example shows how to add support for `Cipher`s. -### WASM +### How to initialize SDK-Managed State on WASM Go to `crates/bitwarden-wasm-internal/src/platform/mod.rs` and add a line with your type, as shown: @@ -207,7 +211,7 @@ Go to `crates/bitwarden-wasm-internal/src/platform/mod.rs` and add a line with y } ``` -### UniFFI +### How to initialize SDK-Managed State on UniFFI Go to `crates/bitwarden-uniffi/src/platform/mod.rs` and add a line with your type, as shown: diff --git a/crates/bitwarden-threading/src/thread_bound_runner.rs b/crates/bitwarden-threading/src/thread_bound_runner.rs index 6366561cb..1fcf514ec 100644 --- a/crates/bitwarden-threading/src/thread_bound_runner.rs +++ b/crates/bitwarden-threading/src/thread_bound_runner.rs @@ -84,6 +84,11 @@ pub struct ThreadBoundRunner { call_channel_tx: tokio::sync::mpsc::Sender>, } +/// Makes a clone of the runner handle. +/// +/// This creates another handle to the same underlying runner object. +/// The underlying state is not duplicated; all clones refer to the same +/// instance. // This is not implemented using derive to remove the implicit bound on `ThreadState: Clone` impl Clone for ThreadBoundRunner { fn clone(&self) -> Self { From 6d9a0955cd3e61dc7a287c8f581c1fd4eb6f6ddb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garci=CC=81a?= Date: Wed, 3 Sep 2025 15:54:54 +0200 Subject: [PATCH 12/12] More doc updates --- crates/bitwarden-state/README.md | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/bitwarden-state/README.md b/crates/bitwarden-state/README.md index 90b8029ee..4ae324985 100644 --- a/crates/bitwarden-state/README.md +++ b/crates/bitwarden-state/README.md @@ -187,7 +187,9 @@ for WASM and UniFFI. This example shows how to add support for `Cipher`s. ### How to initialize SDK-Managed State on WASM -Go to `crates/bitwarden-wasm-internal/src/platform/mod.rs` and add a line with your type, as shown: +Go to `crates/bitwarden-wasm-internal/src/platform/mod.rs` and add a line with your type, as shown +below. In this example we're registering `Cipher` as both client and SDK managed to show how both +are done, but you can also just do one or the other. ```rust,ignore pub async fn initialize_state( @@ -195,6 +197,7 @@ Go to `crates/bitwarden-wasm-internal/src/platform/mod.rs` and add a line with y cipher_repository: CipherRepository, ) -> Result<(), bitwarden_state::registry::StateRegistryError> { let cipher = cipher_repository.into_channel_impl(); + // Register the provided repository as client managed state self.0.platform().state().register_client_managed(cipher); let sdk_managed_repositories = vec![ @@ -213,7 +216,9 @@ Go to `crates/bitwarden-wasm-internal/src/platform/mod.rs` and add a line with y ### How to initialize SDK-Managed State on UniFFI -Go to `crates/bitwarden-uniffi/src/platform/mod.rs` and add a line with your type, as shown: +Go to `crates/bitwarden-uniffi/src/platform/mod.rs` and add a line with your type, as shown below. +In this example we're registering `Cipher` as both client and SDK managed to show how both are done, +but you can also just do one or the other. ```rust,ignore pub async fn initialize_state( @@ -221,6 +226,7 @@ Go to `crates/bitwarden-uniffi/src/platform/mod.rs` and add a line with your typ cipher_repository: Arc, ) -> Result<()> { let cipher = UniffiRepositoryBridge::new(cipher_repository); + // Register the provided repository as client managed state self.0.platform().state().register_client_managed(cipher); let sdk_managed_repositories = vec![