Merge feat/wave13-fts-fix — HIGH audit fixes

_primitives/_rust/kei-entity-store/src/verbs/archive.rs

@@ -9,6 +9,15 @@
 //! fall back to legacy `archived` heuristics (those remain in
 //! `delete.rs` soft-path only).
 //!
+//! FTS semantics: archiving means "hidden from active listing". The verb
+//! therefore DELETEs the row from `fts_<table>` inside the same
+//! transaction as the UPDATE, so `search` will no longer return the
+//! archived row. If a future caller flips the column back to active
+//! (unarchive), it MUST reinsert the row into the FTS index — the
+//! current contract does not auto-reindex on unarchive. "Keep
+//! searchable while archived" is a future feature (`search
+//! --include-archived`), NOT today.
+//!
 //! Input: `{ id: <int|string> }`.
 //! Output: `{ id, archived_at }` — `archived_at` is the stamped
 //! timestamp when a `<field>_at` column exists, else `null`.
@@ -36,7 +45,7 @@ pub fn run(
     let has_ts = schema.fields.iter().any(|f| f.name == ts_col);
     let now: i64 = chrono::Utc::now().timestamp();
 
-    let rows = execute_archive(conn, schema, field_name, &ts_col, has_ts, &id, now)?;
+    let rows = archive_tx(conn, schema, field_name, &ts_col, has_ts, &id, now)?;
     if rows == 0 {
         return Err(VerbError::not_found_text(schema.name, id.as_string()));
     }
@@ -54,7 +63,9 @@ fn guard_enabled(schema: &EntitySchema) -> Result<(), VerbError> {
     Ok(())
 }
 
-fn execute_archive(
+/// UPDATE the archived column (+ stamp) and DELETE the row from FTS (if
+/// configured) in one transaction. Either both persist or neither does.
+fn archive_tx(
     conn: &Connection,
     schema: &EntitySchema,
     field_name: &str,
@@ -62,11 +73,35 @@ fn execute_archive(
     has_ts: bool,
     id: &PkValue,
     now: i64,
+) -> Result<usize, VerbError> {
+    let tx = conn.unchecked_transaction()?;
+    let rows = execute_archive(&tx, schema, field_name, ts_col, has_ts, id, now)?;
+    if rows > 0 && schema.fts_columns.is_some() {
+        tx.execute(
+            &format!(
+                "DELETE FROM fts_{t} WHERE {t}_id=?1",
+                t = schema.table
+            ),
+            rusqlite::params![id.as_sql()],
+        )?;
+    }
+    tx.commit()?;
+    Ok(rows)
+}
+
+fn execute_archive(
+    tx: &rusqlite::Transaction<'_>,
+    schema: &EntitySchema,
+    field_name: &str,
+    ts_col: &str,
+    has_ts: bool,
+    id: &PkValue,
+    now: i64,
 ) -> Result<usize, VerbError> {
     let marker = archive_marker(schema, field_name);
     let pk_name = schema.pk().name;
     let rows = if has_ts {
-        conn.execute(
+        tx.execute(
             &format!(
                 "UPDATE {t} SET {field_name} = ?1, {ts_col} = ?2 WHERE {pk_name} = ?3",
                 t = schema.table
@@ -74,7 +109,7 @@ fn execute_archive(
             rusqlite::params![marker, now, id.as_sql()],
         )?
     } else {
-        conn.execute(
+        tx.execute(
             &format!(
                 "UPDATE {t} SET {field_name} = ?1 WHERE {pk_name} = ?2",
                 t = schema.table

_primitives/_rust/kei-entity-store/src/verbs/delete.rs

@@ -1,9 +1,13 @@
 //! `delete` verb — hard DELETE by id, OR soft (if schema has an
 //! `archived` integer field, flips it to 1).
+//!
+//! The hard-delete path wraps the `fts_<table>` DELETE and the base-table
+//! DELETE in a single `unchecked_transaction`, so a mid-flight FTS failure
+//! rolls back the row delete too (mirrors create/update C2 fix).
 
 use crate::error::VerbError;
 use crate::schema::EntitySchema;
-use crate::verbs::pk;
+use crate::verbs::pk::{self, PkValue};
 use rusqlite::Connection;
 use serde_json::{json, Value};
 
@@ -22,29 +26,9 @@ pub fn run(
     let soft = input.get("soft").and_then(|v| v.as_bool()).unwrap_or(false);
 
     let rows = if soft && has_archived_field(schema) {
-        conn.execute(
-            &format!(
-                "UPDATE {} SET archived = 1 WHERE {}=?1",
-                schema.table,
-                schema.pk().name
-            ),
-            rusqlite::params![id.as_sql()],
-        )?
+        soft_delete(conn, schema, &id)?
     } else {
-        if schema.fts_columns.is_some() {
-            conn.execute(
-                &format!("DELETE FROM fts_{} WHERE {}_id=?1", schema.table, schema.table),
-                rusqlite::params![id.as_sql()],
-            )?;
-        }
-        conn.execute(
-            &format!(
-                "DELETE FROM {} WHERE {}=?1",
-                schema.table,
-                schema.pk().name
-            ),
-            rusqlite::params![id.as_sql()],
-        )?
+        hard_delete_tx(conn, schema, &id)?
     };
     if rows == 0 {
         return Err(VerbError::not_found_text(schema.name, id.as_string()));
@@ -52,6 +36,51 @@ pub fn run(
     Ok(json!({ "ok": true, "id": id.as_json() }))
 }
 
+fn soft_delete(
+    conn: &Connection,
+    schema: &EntitySchema,
+    id: &PkValue,
+) -> Result<usize, VerbError> {
+    let rows = conn.execute(
+        &format!(
+            "UPDATE {} SET archived = 1 WHERE {}=?1",
+            schema.table,
+            schema.pk().name
+        ),
+        rusqlite::params![id.as_sql()],
+    )?;
+    Ok(rows)
+}
+
+/// FTS DELETE + base-table DELETE in one transaction. If either fails,
+/// neither persists.
+fn hard_delete_tx(
+    conn: &Connection,
+    schema: &EntitySchema,
+    id: &PkValue,
+) -> Result<usize, VerbError> {
+    let tx = conn.unchecked_transaction()?;
+    if schema.fts_columns.is_some() {
+        tx.execute(
+            &format!(
+                "DELETE FROM fts_{t} WHERE {t}_id=?1",
+                t = schema.table
+            ),
+            rusqlite::params![id.as_sql()],
+        )?;
+    }
+    let rows = tx.execute(
+        &format!(
+            "DELETE FROM {} WHERE {}=?1",
+            schema.table,
+            schema.pk().name
+        ),
+        rusqlite::params![id.as_sql()],
+    )?;
+    tx.commit()?;
+    Ok(rows)
+}
+
 fn has_archived_field(schema: &EntitySchema) -> bool {
     schema.fields.iter().any(|f| f.name == "archived")
 }

_primitives/_rust/kei-entity-store/tests/bug_fixes_smoke.rs

@@ -2,8 +2,8 @@
 //! injection/M3/TEXT-cap/M2). Each test names the finding it pins.
 
 use kei_entity_store::error::VerbError;
-use kei_entity_store::schema::{EntitySchema, FieldDef};
-use kei_entity_store::verbs::{create, search, update};
+use kei_entity_store::schema::{EdgeKeyKind, EntitySchema, FieldDef};
+use kei_entity_store::verbs::{archive, create, delete, search, update};
 use kei_entity_store::verbs::validate::MAX_TEXT_BYTES;
 use kei_entity_store::Store;
 use rusqlite::Connection;
@@ -190,3 +190,172 @@ fn m2_user_version_applied_once_idempotent() {
         assert_eq!(v, kei_entity_store::engine::CURRENT_USER_VERSION);
     }
 }
+
+// ---------- delete/archive transaction semantics ----------
+//
+// Two audit findings pinned here:
+//   (1) delete.rs hard-path wraps FTS DELETE + table DELETE in one tx
+//       (was two separate execs — partial failure orphaned FTS rows).
+//   (2) archive.rs removes the row from FTS inside the same tx as the
+//       UPDATE (was UPDATE only — archived rows stayed searchable).
+
+static ARCHIVABLE_FIELDS: &[FieldDef] = &[
+    FieldDef::pk("id"),
+    FieldDef::text_nn("title"),
+    FieldDef::text("description"),
+    FieldDef::integer("status"),
+    FieldDef::integer("status_at"),
+    FieldDef::created_at(),
+];
+
+/// FTS + archived_field both configured — required for
+/// `archive_removes_from_fts` and `archive_rollback_on_sabotage`.
+static ARCHIVABLE_FTS: EntitySchema = EntitySchema {
+    name: "item",
+    table: "items",
+    fields: ARCHIVABLE_FIELDS,
+    enabled_verbs: &["create", "get", "search", "archive"],
+    fts_columns: Some(&["title", "description"]),
+    edge_table: None,
+    edge_key_kind: EdgeKeyKind::IntegerPair,
+    archived_field: Some("status"),
+    custom_migrations: &[],
+};
+
+static NO_FTS_FIELDS: &[FieldDef] = &[
+    FieldDef::pk("id"),
+    FieldDef::text_nn("title"),
+    FieldDef::created_at(),
+];
+
+/// No FTS — required for `delete_succeeds_when_no_fts_configured`.
+static NO_FTS_SCHEMA: EntitySchema = EntitySchema {
+    name: "item",
+    table: "plain_items",
+    fields: NO_FTS_FIELDS,
+    enabled_verbs: &["create", "get", "delete"],
+    fts_columns: None,
+    edge_table: None,
+    edge_key_kind: EdgeKeyKind::IntegerPair,
+    archived_field: None,
+    custom_migrations: &[],
+};
+
+#[test]
+fn delete_rollback_on_fts_sabotage() {
+    // Hard-delete path must roll back when the FTS DELETE fails — the
+    // row itself must survive. Pins: delete.rs tx wrapping (finding 1).
+    let s = mk();
+    let id = create::run(s.conn(), &SCHEMA, json!({ "title": "keepme" }))
+        .unwrap()["id"]
+        .as_i64()
+        .unwrap();
+
+    s.conn().execute_batch("DROP TABLE fts_items;").unwrap();
+
+    let result = delete::run(s.conn(), &SCHEMA, json!({ "id": id }));
+    assert!(result.is_err(), "delete must fail when FTS is missing");
+
+    let count: i64 = s
+        .conn()
+        .query_row(
+            "SELECT COUNT(*) FROM items WHERE id=?1",
+            rusqlite::params![id],
+            |r| r.get(0),
+        )
+        .unwrap();
+    assert_eq!(count, 1, "row must survive FTS-failed delete (rollback)");
+}
+
+#[test]
+fn archive_removes_from_fts() {
+    // Archive must remove the row from FTS so `search` no longer returns
+    // it. Pins: archive.rs fts-delete-in-tx (finding 2).
+    let s = Store::open_memory(&[&ARCHIVABLE_FTS]).unwrap();
+    let id = create::run(
+        s.conn(),
+        &ARCHIVABLE_FTS,
+        json!({ "title": "uniqword42", "description": "findme" }),
+    )
+    .unwrap()["id"]
+    .as_i64()
+    .unwrap();
+
+    // Before archive: search finds the row.
+    let before =
+        search::run(s.conn(), &ARCHIVABLE_FTS, json!({ "query": "uniqword42" })).unwrap();
+    assert_eq!(
+        before["results"].as_array().unwrap().len(),
+        1,
+        "row must be searchable before archive"
+    );
+
+    archive::run(s.conn(), &ARCHIVABLE_FTS, json!({ "id": id })).unwrap();
+
+    // After archive: search returns zero hits.
+    let after =
+        search::run(s.conn(), &ARCHIVABLE_FTS, json!({ "query": "uniqword42" })).unwrap();
+    assert_eq!(
+        after["results"].as_array().unwrap().len(),
+        0,
+        "archived row must not be searchable"
+    );
+}
+
+#[test]
+fn archive_rollback_on_sabotage() {
+    // Archive wraps UPDATE + FTS DELETE in one tx. If the FTS delete
+    // fails, the UPDATE must roll back too. Pins: archive.rs tx
+    // wrapping (finding 2).
+    let s = Store::open_memory(&[&ARCHIVABLE_FTS]).unwrap();
+    let id = create::run(
+        s.conn(),
+        &ARCHIVABLE_FTS,
+        json!({ "title": "stable", "description": "x" }),
+    )
+    .unwrap()["id"]
+    .as_i64()
+    .unwrap();
+
+    s.conn().execute_batch("DROP TABLE fts_items;").unwrap();
+
+    let result = archive::run(s.conn(), &ARCHIVABLE_FTS, json!({ "id": id }));
+    assert!(result.is_err(), "archive must fail when FTS is missing");
+
+    // Status column must still be 0 (unarchived) — the UPDATE rolled back.
+    let status: i64 = s
+        .conn()
+        .query_row(
+            "SELECT status FROM items WHERE id=?1",
+            rusqlite::params![id],
+            |r| r.get(0),
+        )
+        .unwrap();
+    assert_eq!(
+        status, 0,
+        "archived_field must be unchanged after FTS-failed archive"
+    );
+}
+
+#[test]
+fn delete_succeeds_when_no_fts_configured() {
+    // Delete on a schema with `fts_columns: None` must work — the tx
+    // wrapping must not introduce a spurious FTS DELETE.
+    let s = Store::open_memory(&[&NO_FTS_SCHEMA]).unwrap();
+    let id = create::run(s.conn(), &NO_FTS_SCHEMA, json!({ "title": "bye" }))
+        .unwrap()["id"]
+        .as_i64()
+        .unwrap();
+
+    delete::run(s.conn(), &NO_FTS_SCHEMA, json!({ "id": id })).unwrap();
+
+    let count: i64 = s
+        .conn()
+        .query_row(
+            "SELECT COUNT(*) FROM plain_items WHERE id=?1",
+            rusqlite::params![id],
+            |r| r.get(0),
+        )
+        .unwrap();
+    assert_eq!(count, 0, "row must be gone after hard delete on no-FTS schema");
+}