fix(server): keep SQLite commits durable except SSH session issuance

WAL with synchronous=NORMAL can roll back acknowledged commits after a
power loss or kernel crash, including SSH session revocations and other
authorization-tightening writes. Run the main pool with synchronous=FULL
so every acknowledged write is durable; in WAL mode that is a single
fsync of the WAL per commit.

Add Store::create_relaxed for inserts that are safe to lose, and use it
only for SSH session issuance: a dropped token just fails validation.
On file-backed SQLite it runs on a dedicated single-connection pool with
synchronous=NORMAL. Both pools share one WAL, so the next FULL commit
also makes earlier relaxed commits durable. Postgres treats it as an
ordinary durable MustCreate insert.

Signed-off-by: Mrunal Patel <mrunalp@gmail.com>

Author: mrunalp

architecture/gateway.md

@@ -648,15 +648,20 @@ Gateway and Sandbox Protocol token responses follow the same convention: a
 present expiration timestamp carries the absolute deadline, while absence means
 the issued token does not expire.
 
-On-disk SQLite databases run in WAL journal mode with `synchronous=NORMAL`.
+On-disk SQLite databases run in WAL journal mode with `synchronous=FULL`.
 The adapter switches the file to WAL on a single connection before the pool
 opens, then applies both settings to every pooled connection. WAL lets readers
-proceed while a writer commits, and `NORMAL` removes the per-commit `fsync`,
+proceed while a writer commits and reduces each commit to one WAL `fsync`,
 which matters because gateway hot paths such as SSH session issuance and
-revocation are many small autocommit writes. The trade-off is that a power
-loss or kernel crash can roll back the most recent transactions; the database
-remains consistent. Deployments that need stronger durability or multiple
-replicas use Postgres. WAL requires a local filesystem with working shared
+revocation are many small autocommit writes. `synchronous` stays at `FULL`
+because some of those writes tighten authorization: under `NORMAL`, a power
+loss could roll back an acknowledged SSH session revocation and make the token
+valid again. Writes that are safe to lose, currently only SSH session issuance
+through `Store::create_relaxed`, use a second single-connection pool with
+`synchronous=NORMAL`. Losing a minted token only invalidates it, and because
+both pools share one WAL, the next `FULL` commit also makes earlier relaxed
+commits durable. Deployments that need multiple replicas use Postgres, where
+`create_relaxed` is an ordinary durable insert. WAL requires a local filesystem with working shared
 memory, so the SQLite file must not live on a network mount, and backups must
 use `sqlite3 .backup` or `VACUUM INTO` rather than copying the main file alone.
 

crates/openshell-server/src/grpc/sandbox.rs

@@ -2597,7 +2597,9 @@ pub(super) async fn handle_create_ssh_session(
     // Ensure metadata is valid (defense in depth - should always be true for server-constructed metadata)
     super::validation::validate_object_metadata(session.metadata.as_ref(), "ssh_session")?;
 
-    // Use MustCreate to atomically ensure the session token is unique
+    // `create_relaxed` fails if the token already exists, like MustCreate, but
+    // skips the per-commit fsync on SQLite. Losing a freshly minted token in a
+    // crash only makes it invalid; revocation stays on the durable `put_if`.
     let session_labels = session.object_labels();
     let session_labels_json = if session_labels.as_ref().is_none_or(HashMap::is_empty) {
         None
@@ -2609,14 +2611,13 @@ pub(super) async fn handle_create_ssh_session(
     };
     state
         .store
-        .put_if(
+        .create_relaxed(
             SshSession::object_type(),
             &token,
             session.object_name(),
             session.object_workspace(),
             &session.encode_to_vec(),
             session_labels_json.as_deref(),
-            WriteCondition::MustCreate,
         )
         .await
         .map_err(|e| Status::internal(format!("persist ssh session failed: {e}")))?;

crates/openshell-server/src/persistence/mod.rs

@@ -384,6 +384,38 @@ impl Store {
         ))
     }
 
+    /// Create an object that is safe to lose in a crash.
+    ///
+    /// Behaves like [`Self::put_if`] with [`WriteCondition::MustCreate`], but
+    /// the file-backed `SQLite` store commits it with `synchronous=NORMAL`, so
+    /// a power loss or kernel crash shortly after the call returns may roll
+    /// the insert back. Use it only for objects whose absence denies access,
+    /// such as newly minted SSH session tokens. Writes that revoke or tighten
+    /// anything must use [`Self::put_if`], which is always durable.
+    #[tracing::instrument(
+        name = "store",
+        skip_all,
+        fields(otel.name = "store.create_relaxed", otel.status_code = tracing::field::Empty,  object_type = %object_type, object.id = %id, object.name = %name, workspace = %workspace)
+    )]
+    pub async fn create_relaxed(
+        &self,
+        object_type: &str,
+        id: &str,
+        name: &str,
+        workspace: &str,
+        payload: &[u8],
+        labels: Option<&str>,
+    ) -> PersistenceResult<WriteResult> {
+        store_dispatch_traced!(self.create_relaxed(
+            object_type,
+            id,
+            name,
+            workspace,
+            payload,
+            labels
+        ))
+    }
+
     /// Delete an object by id with compare-and-swap support.
     ///
     /// # Arguments

crates/openshell-server/src/persistence/postgres.rs

@@ -180,6 +180,29 @@ ON CONFLICT (object_type, workspace, name) WHERE name IS NOT NULL DO UPDATE SET
         Ok(())
     }
 
+    /// Create an object; Postgres commits are always durable, so this is
+    /// [`Self::put_if`] with [`WriteCondition::MustCreate`].
+    pub async fn create_relaxed(
+        &self,
+        object_type: &str,
+        id: &str,
+        name: &str,
+        workspace: &str,
+        payload: &[u8],
+        labels: Option<&str>,
+    ) -> PersistenceResult<WriteResult> {
+        self.put_if(
+            object_type,
+            id,
+            name,
+            workspace,
+            payload,
+            labels,
+            WriteCondition::MustCreate,
+        )
+        .await
+    }
+
     #[allow(clippy::too_many_arguments)]
     pub async fn put_if(
         &self,

crates/openshell-server/src/persistence/sqlite.rs

@@ -41,6 +41,10 @@ use super::{DELETE_MANY_BATCH_SIZE, DRAFT_CHUNK_OBJECT_TYPE, POLICY_OBJECT_TYPE}
 #[derive(Debug, Clone)]
 pub struct SqliteStore {
     pool: SqlitePool,
+    /// Pool for writes whose loss after a crash is harmless; see
+    /// [`SqliteStore::create_relaxed`]. On-disk stores open it with
+    /// `synchronous=NORMAL`; in-memory stores share `pool`.
+    relaxed_pool: SqlitePool,
     #[cfg_attr(not(any(test, feature = "test-support")), allow(dead_code))]
     in_memory_keepalive: Option<Arc<Mutex<Option<SqliteConnection>>>>,
 }
@@ -80,12 +84,20 @@ pub(super) async fn replace_pool_connection(store: &SqliteStore) -> PersistenceR
 /// writes. `SQLite`'s default rollback journal makes each of those commits pay
 /// several `fsync` calls and blocks readers while a writer holds the lock, so
 /// under a burst of forwarded connections the whole store serializes on disk
-/// latency. WAL mode removes the reader/writer exclusion and, combined with
-/// `synchronous=NORMAL`, drops the per-commit `fsync`: a power loss or kernel
-/// crash may roll back the most recent transactions, but the database stays
-/// consistent. That is the standard WAL configuration and matches the
-/// single-node scope of the `SQLite` backend; deployments that need stronger
-/// durability guarantees use the Postgres backend.
+/// latency. WAL mode removes the reader/writer exclusion and cuts each commit
+/// to a single `fsync` of the WAL file.
+///
+/// The main pool keeps `synchronous=FULL` rather than the usual WAL pairing
+/// of `NORMAL`. Under `NORMAL` a power loss or kernel crash can roll back
+/// transactions that were already acknowledged, and several of those writes
+/// tighten authorization: an SSH session revoked just before the crash would
+/// come back valid for the rest of its lifetime. `FULL` keeps every
+/// acknowledged commit durable. Writes whose loss only ever denies access,
+/// such as minting a new SSH session token, go through a separate
+/// `synchronous=NORMAL` pool instead ([`SqliteStore::create_relaxed`]). Both
+/// pools append to the same WAL file, so the next `FULL` commit's `fsync` also
+/// makes every earlier relaxed commit durable, and a crash can never roll back
+/// a `FULL` commit.
 ///
 /// `journal_mode=WAL` is persistent in the database file, but switching into
 /// it needs exclusive access: if another connection holds the file open, the
@@ -105,7 +117,7 @@ async fn configure_on_disk_durability(
 ) -> PersistenceResult<SqliteConnectOptions> {
     let options = options
         .journal_mode(SqliteJournalMode::Wal)
-        .synchronous(SqliteSynchronous::Normal);
+        .synchronous(SqliteSynchronous::Full);
     let wal_error = |e: &sqlx::Error| {
         PersistenceError::Database(format!(
             "failed to switch SQLite database {} to WAL journal mode (the switch needs \
@@ -121,9 +133,56 @@ async fn configure_on_disk_durability(
     Ok(options)
 }
 
+/// Insert a new object at resource version 1, failing if it already exists.
+async fn insert_new_object(
+    pool: &SqlitePool,
+    object_type: &str,
+    id: &str,
+    name: &str,
+    workspace: &str,
+    payload: &[u8],
+    labels: Option<&str>,
+) -> PersistenceResult<WriteResult> {
+    let now_ms = current_time_ms();
+    sqlx::query(
+        r#"
+INSERT INTO "objects" ("object_type", "id", "name", "workspace", "payload", "created_at_ms", "updated_at_ms", "labels", "resource_version")
+VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?6, ?7, 1)
+"#,
+    )
+    .bind(object_type)
+    .bind(id)
+    .bind(name)
+    .bind(workspace)
+    .bind(payload)
+    .bind(now_ms)
+    .bind(labels.unwrap_or("{}"))
+    .execute(pool)
+    .await
+    .map_err(|e| map_db_error(&e))?;
+
+    Ok(WriteResult {
+        resource_version: 1,
+        created_at_ms: now_ms,
+        updated_at_ms: now_ms,
+    })
+}
+
 #[cfg(test)]
 pub(super) async fn journal_settings(store: &SqliteStore) -> PersistenceResult<(String, i64)> {
-    let mut connection = store.pool.acquire().await.map_err(|e| map_db_error(&e))?;
+    pool_journal_settings(&store.pool).await
+}
+
+#[cfg(test)]
+pub(super) async fn relaxed_journal_settings(
+    store: &SqliteStore,
+) -> PersistenceResult<(String, i64)> {
+    pool_journal_settings(&store.relaxed_pool).await
+}
+
+#[cfg(test)]
+async fn pool_journal_settings(pool: &SqlitePool) -> PersistenceResult<(String, i64)> {
+    let mut connection = pool.acquire().await.map_err(|e| map_db_error(&e))?;
     let journal_mode: String = sqlx::query_scalar("PRAGMA journal_mode")
         .fetch_one(&mut *connection)
         .await
@@ -185,18 +244,33 @@ impl SqliteStore {
             None
         };
 
+        let relaxed_options =
+            (!is_in_memory).then(|| options.clone().synchronous(SqliteSynchronous::Normal));
+
         let pool = pool_options
             .connect_with(options)
             .await
             .map_err(|e| map_db_error(&e))?;
 
+        // SQLite serializes writers, so one connection is enough for the
+        // relaxed pool.
+        let relaxed_pool = match relaxed_options {
+            Some(relaxed_options) => SqlitePoolOptions::new()
+                .max_connections(1)
+                .connect_with(relaxed_options)
+                .await
+                .map_err(|e| map_db_error(&e))?,
+            None => pool.clone(),
+        };
+
         // Tighten the permissions of the database file to owner-only access (0o600).
         if let Some(path) = db_path {
             restrict_db_file_permissions(&path)?;
         }
 
         Ok(Self {
             pool,
+            relaxed_pool,
             in_memory_keepalive,
         })
     }
@@ -251,6 +325,7 @@ impl SqliteStore {
     /// Do not call from runtime code; this tears down the active pool.
     #[cfg(any(test, feature = "test-support"))]
     pub async fn close(&self) {
+        self.relaxed_pool.close().await;
         self.pool.close().await;
         if let Some(keepalive) = &self.in_memory_keepalive {
             let connection = keepalive.lock().await.take();
@@ -294,6 +369,33 @@ ON CONFLICT ("object_type", "workspace", "name") WHERE "name" IS NOT NULL DO UPD
         Ok(())
     }
 
+    /// Create an object with `synchronous=NORMAL` durability.
+    ///
+    /// Same semantics as [`Self::put_if`] with [`WriteCondition::MustCreate`],
+    /// except that a power loss or kernel crash shortly after the call returns
+    /// may roll the insert back. Use it only for objects whose absence denies
+    /// access, never for writes that revoke or tighten anything.
+    pub async fn create_relaxed(
+        &self,
+        object_type: &str,
+        id: &str,
+        name: &str,
+        workspace: &str,
+        payload: &[u8],
+        labels: Option<&str>,
+    ) -> PersistenceResult<WriteResult> {
+        insert_new_object(
+            &self.relaxed_pool,
+            object_type,
+            id,
+            name,
+            workspace,
+            payload,
+            labels,
+        )
+        .await
+    }
+
     #[allow(clippy::too_many_arguments)]
     pub async fn put_if(
         &self,
@@ -309,29 +411,16 @@ ON CONFLICT ("object_type", "workspace", "name") WHERE "name" IS NOT NULL DO UPD
 
         match condition {
             WriteCondition::MustCreate => {
-                // Insert only - fail if object exists
-                sqlx::query(
-                    r#"
-INSERT INTO "objects" ("object_type", "id", "name", "workspace", "payload", "created_at_ms", "updated_at_ms", "labels", "resource_version")
-VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?6, ?7, 1)
-"#,
+                insert_new_object(
+                    &self.pool,
+                    object_type,
+                    id,
+                    name,
+                    workspace,
+                    payload,
+                    labels,
                 )
-                .bind(object_type)
-                .bind(id)
-                .bind(name)
-                .bind(workspace)
-                .bind(payload)
-                .bind(now_ms)
-                .bind(labels.unwrap_or("{}"))
-                .execute(&self.pool)
                 .await
-                .map_err(|e| map_db_error(&e))?;
-
-                Ok(WriteResult {
-                    resource_version: 1,
-                    created_at_ms: now_ms,
-                    updated_at_ms: now_ms,
-                })
             }
             WriteCondition::MatchResourceVersion(expected_version) => {
                 // Update with version check