db: add STRICT tables with migration for old databases

Enables STRICT tables in developer mode, but old databases (~2019) may
have BLOB values in TEXT columns. Migration converts BLOB faildetail
to TEXT with UTF-8 validation, NULLs invalid data.

STRICT is only applied to fresh databases; existing databases being
upgraded skip STRICT to avoid type affinity issues with legacy data.

Also adds security pragmas in developer mode: trusted_schema=OFF,
cell_size_check=ON.

Fixes #7913.

Changelog-Added: Database: STRICT tables and security pragmas in developer mode
Changelog-Fixed: Database migration for old BLOB-typed faildetail values

Author: wqxoxo

db/common.h

@@ -66,6 +66,9 @@ struct db {
 
 	/* Fatal if we try to write to db */
 	bool readonly;
+
+	/* Set during migrations to prevent STRICT mode on table creation */
+	bool in_migration;
 };
 
 struct db_query {

db/db_sqlite3.c

@@ -203,16 +203,46 @@ static bool db_sqlite3_setup(struct db *db, bool create)
 			   "PRAGMA foreign_keys = ON;", -1, &stmt, NULL);
 	err = sqlite3_step(stmt);
 	sqlite3_finalize(stmt);
-	return err == SQLITE_DONE;
+
+	if (err != SQLITE_DONE)
+		return false;
+
+	if (db->developer) {
+		sqlite3_prepare_v2(conn2sql(db->conn),
+				   "PRAGMA trusted_schema = OFF;", -1, &stmt, NULL);
+		sqlite3_step(stmt);
+		sqlite3_finalize(stmt);
+
+		sqlite3_prepare_v2(conn2sql(db->conn),
+				   "PRAGMA cell_size_check = ON;", -1, &stmt, NULL);
+		sqlite3_step(stmt);
+		sqlite3_finalize(stmt);
+	}
+
+	return true;
 }
 
 static bool db_sqlite3_query(struct db_stmt *stmt)
 {
 	sqlite3_stmt *s;
 	sqlite3 *conn = conn2sql(stmt->db->conn);
 	int err;
+	const char *query = stmt->query->query;
+	char *modified_query = NULL;
+
+	/* STRICT tables for developer mode, and not during upgrades. */
+	if (stmt->db->developer &&
+	    !stmt->db->in_migration &&
+	    strncasecmp(query, "CREATE TABLE", 12) == 0 &&
+	    !strstr(query, "STRICT")) {
+		modified_query = tal_fmt(stmt, "%s STRICT", query);
+		query = modified_query;
+	}
+
+	err = sqlite3_prepare_v2(conn, query, -1, &s, NULL);
 
-	err = sqlite3_prepare_v2(conn, stmt->query->query, -1, &s, NULL);
+	if (modified_query)
+		tal_free(modified_query);
 
 	for (size_t i=0; i<stmt->query->placeholders; i++) {
 		struct db_binding *b = &stmt->bindings[i];

db/utils.c

@@ -364,6 +364,7 @@ struct db *db_open_(const tal_t *ctx, const char *filename,
 	db->in_transaction = NULL;
 	db->transaction_started = false;
 	db->changes = NULL;
+	db->in_migration = false;
 
 	/* This must be outside a transaction, so catch it */
 	assert(!db->in_transaction);

devtools/sql-rewrite.py

@@ -45,6 +45,8 @@ def rewrite_single(self, query):
             r'BIGINT': 'INTEGER',
             r'BIGINTEGER': 'INTEGER',
             r'BIGSERIAL': 'INTEGER',
+            r'VARCHAR(?:\(\d+\))?': 'TEXT',
+            r'\bINT\b': 'INTEGER',
             r'CURRENT_TIMESTAMP\(\)': "strftime('%s', 'now')",
             r'INSERT INTO[ \t]+(.*)[ \t]+ON CONFLICT.*DO NOTHING;': 'INSERT OR IGNORE INTO \\1;',
             # Rewrite "decode('abcd', 'hex')" to become "x'abcd'"

tests/test_db.py

@@ -163,6 +163,14 @@ def test_scid_upgrade(node_factory, bitcoind):
     assert l1.db_query('SELECT scid FROM channels;') == [{'scid': scid_to_int('103x1x1')}]
     assert l1.db_query('SELECT failscid FROM payments;') == [{'failscid': scid_to_int('103x1x1')}]
 
+    faildetail_types = l1.db_query(
+        "SELECT id, typeof(faildetail) as type "
+        "FROM payments WHERE faildetail IS NOT NULL"
+    )
+    for row in faildetail_types:
+        assert row['type'] == 'text', \
+            f"Payment {row['id']}: faildetail has type {row['type']}, expected 'text'"
+
 
 @unittest.skipIf(not COMPAT, "needs COMPAT to convert obsolete db")
 @unittest.skipIf(os.getenv('TEST_DB_PROVIDER', 'sqlite3') != 'sqlite3', "This test is based on a sqlite3 snapshot")
@@ -642,3 +650,48 @@ def test_channel_htlcs_id_change(bitcoind, node_factory):
     # Make some HTLCS
     for amt in (100, 500, 1000, 5000, 10000, 50000, 100000):
         l1.pay(l3, amt)
+
+
+@unittest.skipIf(os.getenv('TEST_DB_PROVIDER', 'sqlite3') != 'sqlite3', "STRICT tables are SQLite3 specific")
+def test_sqlite_strict_mode(node_factory):
+    """Test that STRICT is appended to CREATE TABLE in developer mode."""
+    l1 = node_factory.get_node(options={'developer': None})
+
+    tables = l1.db_query("SELECT name, sql FROM sqlite_master WHERE type='table' AND name NOT LIKE 'sqlite_%'")
+
+    strict_tables = [t for t in tables if t['sql'] and 'STRICT' in t['sql']]
+    assert len(strict_tables) > 0, f"Expected at least one STRICT table in developer mode, found none out of {len(tables)}"
+
+    known_strict_tables = ['version', 'forwards', 'payments', 'local_anchors', 'addresses']
+    for table_name in known_strict_tables:
+        table_sql = next((t['sql'] for t in tables if t['name'] == table_name), None)
+        if table_sql:
+            assert 'STRICT' in table_sql, f"Expected table '{table_name}' to be STRICT in developer mode"
+
+
+@unittest.skipIf(os.getenv('TEST_DB_PROVIDER', 'sqlite3') != 'sqlite3', "SQLite3-specific test")
+@unittest.skipIf(not COMPAT, "needs COMPAT to test old database upgrade")
+@unittest.skipIf(TEST_NETWORK != 'regtest', "The network must match the DB snapshot")
+def test_strict_mode_with_old_database(node_factory, bitcoind):
+    """Test old database upgrades work (STRICT not applied during migrations)."""
+    bitcoind.generate_block(1)
+
+    l1 = node_factory.get_node(dbfile='oldstyle-scids.sqlite3.xz',
+                               options={'database-upgrade': True,
+                                        'developer': None})
+
+    assert l1.rpc.getinfo()['id'] is not None
+
+    # Upgraded tables won't be STRICT (only fresh databases get STRICT).
+    strict_tables = l1.db_query(
+        "SELECT name FROM sqlite_master "
+        "WHERE type='table' AND sql LIKE '%STRICT%'"
+    )
+    assert len(strict_tables) == 0, "Upgraded database should not have STRICT tables"
+
+    # Verify BLOB->TEXT migration ran for faildetail cleanup.
+    result = l1.db_query(
+        "SELECT COUNT(*) as count FROM payments "
+        "WHERE typeof(faildetail) = 'blob'"
+    )
+    assert result[0]['count'] == 0, "Found BLOB-typed faildetail after migration"

wallet/db.c

@@ -35,6 +35,9 @@ static bool db_migrate(struct lightningd *ld, struct db *db,
 	available = num_migrations - 1;
 	orig = current = db_get_version(db);
 
+	/* Disable STRICT for upgrades: legacy data may have wrong type affinity. */
+	db->in_migration = (current != -1);
+
 	if (current == -1)
 		log_info(ld->log, "Creating database");
 	else if (available < current) {
@@ -112,6 +115,8 @@ struct db *db_setup(const tal_t *ctx, struct lightningd *ld,
 
 	db_commit_transaction(db);
 
+	db->in_migration = false;
+
 	/* This needs to be done outside a transaction, apparently.
 	 * It's a good idea to do this every so often, and on db
 	 * upgrade is a reasonable time. */
@@ -1069,3 +1074,4 @@ void migrate_fail_pending_payments_without_htlcs(struct lightningd *ld,
 	db_bind_int(stmt, payment_status_in_db(PAYMENT_PENDING));
 	db_exec_prepared_v2(take(stmt));
 }
+

wallet/migrations.c

@@ -3,6 +3,7 @@
 #include <ccan/tal/str/str.h>
 #include <common/channel_id.h>
 #include <common/htlc_state.h>
+#include <common/utils.h>
 #include <db/bindings.h>
 #include <db/common.h>
 #include <db/exec.h>
@@ -1079,10 +1080,57 @@ static const struct db_migration dbmigrations[] = {
      NULL, revert_withheld_column},
     /* ^v25.12 */
 
+    /* Fix BLOB→TEXT in payments.faildetail for old databases. */
+    {NULL, migrate_fix_payments_faildetail_type,
+     /* Fixing data types is idempotent, so no revert needed */
+     NULL, NULL},
 };
 
 const struct db_migration *get_db_migrations(size_t *num)
 {
 	*num = ARRAY_SIZE(dbmigrations);
 	return dbmigrations;
 }
+
+void migrate_fix_payments_faildetail_type(struct lightningd *ld UNUSED,
+					  struct db *db)
+{
+	/* Historical databases may have BLOB-typed faildetail data.
+	 * STRICT mode rejects this, so convert or NULL out invalid UTF-8. */
+	struct db_stmt *stmt;
+
+	stmt = db_prepare_v2(db, SQL("SELECT id, faildetail "
+				     "FROM payments "
+				     "WHERE typeof(faildetail) = 'blob'"));
+	db_query_prepared(stmt);
+
+	while (db_step(stmt)) {
+		u64 id = db_col_u64(stmt, "id");
+		const u8 *blob = db_col_blob(stmt, "faildetail");
+		size_t len = db_col_bytes(stmt, "faildetail");
+		struct db_stmt *upd;
+
+		if (!utf8_check(blob, len)) {
+			/* Invalid UTF-8, NULL it out */
+			upd = db_prepare_v2(db,
+				SQL("UPDATE payments "
+				    "SET faildetail = NULL "
+				    "WHERE id = ?"));
+			db_bind_u64(upd, id);
+			db_exec_prepared_v2(take(upd));
+			continue;
+		}
+
+		/* Convert BLOB to TEXT */
+		char *text = tal_strndup(tmpctx, (char *)blob, len);
+		upd = db_prepare_v2(db,
+			SQL("UPDATE payments "
+			    "SET faildetail = ? "
+			    "WHERE id = ?"));
+		db_bind_text(upd, text);
+		db_bind_u64(upd, id);
+		db_exec_prepared_v2(take(upd));
+	}
+
+	tal_free(stmt);
+}

wallet/migrations.h

@@ -62,4 +62,5 @@ void migrate_remove_chain_moves_duplicates(struct lightningd *ld, struct db *db)
 void migrate_from_account_db(struct lightningd *ld, struct db *db);
 void migrate_datastore_commando_runes(struct lightningd *ld, struct db *db);
 void migrate_runes_idfix(struct lightningd *ld, struct db *db);
+void migrate_fix_payments_faildetail_type(struct lightningd *ld, struct db *db);
 #endif /* LIGHTNING_WALLET_MIGRATIONS_H */