keisei/KeiSeiKit-1.0
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(entity-store): FTS delete-tx + archive FTS desync (HIGH audit)

Browse source 
2 HIGH security/correctness fixes from post-v0.28 audit:

- delete.rs: hard-delete now wraps FTS DELETE + table DELETE in
  unchecked_transaction. Prior: two separate execute calls could
  leave FTS orphan on second-stmt failure. Same pattern as C2 fix.

- archive.rs: archive-UPDATE + FTS DELETE now atomic in one tx.
  Prior: archived rows remained searchable (UX/privacy leak).
  Semantics documented: archive = hidden from FTS; unarchive must
  re-insert (caller responsibility).

+4 regression tests: delete_rollback_on_fts_sabotage,
archive_removes_from_fts, archive_rollback_on_sabotage,
delete_succeeds_when_no_fts_configured. All green (49/49 kes tests).

Constructor Pattern: all files <200 LOC, all functions <30 LOC.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Parfii-bot 2026-04-23 15:45:23 +08:00
parent f2358397c9
commit 0706733abf
3 changed files with 262 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -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

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

@ -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")
}

173
_primitives/_rust/kei-entity-store/tests/bug_fixes_smoke.rs
View file

@ -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");
}