From 0da7b4e42481c692eb64602fd2bd04597d29213a Mon Sep 17 00:00:00 2001 From: Duncan Date: Thu, 13 Aug 2026 11:09:09 -0400 Subject: [PATCH] fix(archive): validate scope-age index collation and direction in M4 pragma_index_info reports key column names but not per-key collation or sort direction, so a named non-partial index with the exact ordered columns but COLLATE NOCASE on a key was certified by M4's shape check even though the binary-equality prune-age query cannot use it (the plan falls back to the PK autoindex plus a temp B-tree sort). Switch key-shape validation to pragma_index_xinfo as the single source of key semantics, asserting name + BINARY collation + ascending direction; index_list stays only for ownership and partiality. A wrong-collation index is dropped and rebuilt inside M4's BEGIN IMMEDIATE like any other wrong shape. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- desktop/src-tauri/src/archive/retention.rs | 57 +++++++++++-------- .../src-tauri/src/archive/retention_tests.rs | 52 +++++++++++++++++ 2 files changed, 86 insertions(+), 23 deletions(-) diff --git a/desktop/src-tauri/src/archive/retention.rs b/desktop/src-tauri/src/archive/retention.rs index ef809468c..10108e7b8 100644 --- a/desktop/src-tauri/src/archive/retention.rs +++ b/desktop/src-tauri/src/archive/retention.rs @@ -92,16 +92,24 @@ const RETENTION_POLICIES_SHAPE: &[ColumnShape] = &[ /// carries `not_null = false` with `pk_position = 1`. const ARCHIVE_META_SHAPE: &[ColumnShape] = &[("key", "TEXT", false, 1), ("value", "TEXT", true, 0)]; +/// One key column's expected shape: `(name, collation, descending)`. The +/// age-range seek needs binary-ordered ascending keys; a differing collation +/// (e.g. `NOCASE`) or sort direction produces an index the prune query planner +/// will not use even when the column names match. +type IndexKeyShape = (&'static str, &'static str, bool); + /// Expected key columns of [`SCOPE_AGE_INDEX_NAME`] in seqno order — the /// age-range access path Phase 2 depends on. Order is load-bearing: a covering -/// seek needs the scope keys before `archived_at`. -const SCOPE_AGE_INDEX_SHAPE: &[&str] = &[ - "identity_pubkey", - "relay_url", - "scope_type", - "scope_value", - "archived_at", - "id", +/// seek needs the scope keys before `archived_at`. Collation and direction are +/// load-bearing too: [`SCOPE_AGE_INDEX_DDL`] declares every key `BINARY` +/// ascending (SQLite's defaults), and only a matching shape serves the seek. +const SCOPE_AGE_INDEX_SHAPE: &[IndexKeyShape] = &[ + ("identity_pubkey", "BINARY", false), + ("relay_url", "BINARY", false), + ("scope_type", "BINARY", false), + ("scope_value", "BINARY", false), + ("archived_at", "BINARY", false), + ("id", "BINARY", false), ]; // ── Shape validation (used by migration M4) ──────────────────────────────────── @@ -138,10 +146,11 @@ fn table_shape_matches( } /// Whether the scope-age index exists on `archived_event_scopes` with exactly -/// the expected key columns in order. False when the index is absent, sits on -/// the wrong table, indexes the wrong columns, or is partial (carries a `WHERE` -/// predicate) — all of which M4 repairs by dropping and recreating it (an index -/// carries no data, so a rebuild is safe). +/// the expected key columns — name, collation, and sort direction — in order. +/// False when the index is absent, sits on the wrong table, indexes the wrong +/// columns, applies a non-`BINARY` collation or descending order to any key, or +/// is partial (carries a `WHERE` predicate) — all of which M4 repairs by +/// dropping and recreating it (an index carries no data, so a rebuild is safe). pub(super) fn scope_age_index_is_correct(conn: &Connection) -> Result { let table: Option = conn .query_row( @@ -178,20 +187,22 @@ pub(super) fn scope_age_index_is_correct(conn: &Connection) -> Result = stmt - .query_map([], |r| r.get::<_, String>(0)) - .map_err(|e| format!("shape check: query index_info: {e}"))? + .map_err(|e| format!("shape check: prepare index_xinfo: {e}"))?; + let keys: Vec<(String, String, i64)> = stmt + .query_map([], |r| Ok((r.get(0)?, r.get(1)?, r.get(2)?))) + .map_err(|e| format!("shape check: query index_xinfo: {e}"))? .collect::, _>>() - .map_err(|e| format!("shape check: read index_info: {e}"))?; + .map_err(|e| format!("shape check: read index_xinfo: {e}"))?; - Ok(columns.len() == SCOPE_AGE_INDEX_SHAPE.len() - && columns - .iter() - .zip(SCOPE_AGE_INDEX_SHAPE) - .all(|(actual, expected)| actual == expected)) + Ok(keys.len() == SCOPE_AGE_INDEX_SHAPE.len() + && keys.iter().zip(SCOPE_AGE_INDEX_SHAPE).all( + |((name, coll, desc), (exp_name, exp_coll, exp_desc))| { + name == exp_name && coll.eq_ignore_ascii_case(exp_coll) && (*desc != 0) == *exp_desc + }, + )) } /// Validate the COMPLETE expected shape of the M4 objects: both tables' columns diff --git a/desktop/src-tauri/src/archive/retention_tests.rs b/desktop/src-tauri/src/archive/retention_tests.rs index 6ad2c167e..c9d661277 100644 --- a/desktop/src-tauri/src/archive/retention_tests.rs +++ b/desktop/src-tauri/src/archive/retention_tests.rs @@ -510,6 +510,58 @@ fn test_m4_partial_age_index_dropped_and_rebuilt_non_partial() { assert_eq!(partial, 0, "the rebuilt index must not be partial"); } +#[test] +fn test_m4_wrong_collation_age_index_dropped_and_rebuilt_binary() { + let db = NamedTempFile::new().unwrap(); + build_pre_m4_db(db.path()); + // Precreate the scope-age index with the expected NAME, right table, exact + // key order, and non-partial — but `COLLATE NOCASE` on the first key. It + // holds the right column names, is non-partial, and passes both the earlier + // name/order and partiality probes; only the xinfo collation check catches + // it. A NOCASE key makes the binary-equality prune-age query fall back to + // the PK autoindex plus a temp B-tree sort, so M4 must rebuild it BINARY. + { + let conn = Connection::open(db.path()).unwrap(); + conn.execute_batch(&format!( + "CREATE INDEX {SCOPE_AGE_INDEX_NAME} + ON archived_event_scopes + (identity_pubkey COLLATE NOCASE, relay_url, scope_type, + scope_value, archived_at, id);" + )) + .unwrap(); + // The validator must reject the NOCASE index BEFORE M4 runs — this is + // the exact check that fails without the xinfo collation probe. + assert!( + !scope_age_index_is_correct(&conn).unwrap(), + "an index with a non-BINARY key collation must not be certified" + ); + } + // M4 rebuilds the wrong-collation index BINARY, then the marker lands. + let conn = fresh(&db); + assert_eq!(m4_marker_count(&conn), 1, "M4 completes after the rebuild"); + assert!( + scope_age_index_is_correct(&conn).unwrap(), + "the index was rebuilt with BINARY collation on every key" + ); + // Prove every rebuilt key carries the default BINARY collation. + let mut stmt = conn + .prepare(&format!( + "SELECT coll FROM pragma_index_xinfo('{SCOPE_AGE_INDEX_NAME}') \ + WHERE key = 1 ORDER BY seqno" + )) + .unwrap(); + let colls: Vec = stmt + .query_map([], |r| r.get::<_, String>(0)) + .unwrap() + .collect::, _>>() + .unwrap(); + assert_eq!(colls.len(), 6, "all six key columns are indexed"); + assert!( + colls.iter().all(|c| c.eq_ignore_ascii_case("BINARY")), + "every rebuilt key uses BINARY collation, got {colls:?}" + ); +} + // ── Concurrency ─────────────────────────────────────────────────────────────── #[test]