plugins/sql: add a delete_statement to toplevel tables with created_index values.

Public commit record

What the developer wrote

Authored by Rusty Russell

73/100 · Adequate

plugins/sql: add a delete_statement to toplevel tables with created_index values.

This will let us (efficiently) delete a single entry, so we can wean
tables with created_index off the default "delete all and reload" behavior.

Change insert_stmt to a const, too (and use a temporary when we're
building it).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context

The short version

What changed, and why it matters

This is a small internal cleanup in Core Lightning's SQL plugin. It adds a prepared 'DELETE' statement for tables that have a created_index column, and makes the existing insert statement constant. The change is meant to enable future efficient single-row deletion instead of deleting and reloading whole tables. There is no direct security bug visible in the diff, and no exploit path is shown.

Recommended action

No immediate action required. Treat as routine refactoring. If reviewing the broader feature, verify that future commits actually bind parameters correctly when delete_stmt is executed and that created_index values are unique and stable.

Security signals we found

SQL statement construction using tal_fmt with internal table names

Use of parameterized placeholder '?' for created_index in delete statement

No user-controlled input concatenated into generated SQL

Refactoring only; no functional deletion logic added in this commit

Technical analysis

Evidence from the diff

The commit modifies plugins/sql.c’s table_desc structure to add a delete_stmt field and populate it with ‘DELETE FROM

WHERE created_index = ?;’ only when td->has_created_index is true. It also refactors insert_stmt construction to use a temporary char* before assigning to a now-const insert_stmt member. No SQL injection vulnerability is introduced: table names come from internal schema definitions, and the statement uses a bound parameter for the created_index value. The change is preparatory and does not itself alter runtime behavior beyond creating an additional prepared statement.

Changed components

plugins/sql.cstruct table_descfinish_td() function

Inspect captured patch +14 / −8

diff --git a/plugins/sql.c b/plugins/sql.c
index 5f9a6bc9..313b16e5 100644
--- a/plugins/sql.c
+++ b/plugins/sql.c
@@ -126,7 +126,9 @@ struct table_desc {
 	/* name if we need to wait for changes */
 	const char *waitname;
 	struct column **columns;
-	char *insert_stmt;
+	const char *insert_stmt;
+	/* If we have create_index, we can delete by it */
+	const char *delete_stmt;
 	/* If we are a subtable */
 	struct table_desc *parent;
 	/* Is this a sub object (otherwise, subarray if parent is true) */
@@ -1558,7 +1560,7 @@ static const char *primary_key_name(const struct table_desc *td)
 static void finish_td(struct plugin *plugin, struct table_desc *td)
 {
 	struct sql *sql = sql_of(plugin);
-	char *create_stmt;
+	char *create_stmt, *insert_stmt;
 	int err;
 	char *errmsg;
 	const char *sep = "";
@@ -1569,11 +1571,14 @@ static void finish_td(struct plugin *plugin, struct table_desc *td)
 		goto do_subtables;
 
 	create_stmt = tal_fmt(tmpctx, "CREATE TABLE %s (", td->name);
-	td->insert_stmt = tal_fmt(td, "INSERT INTO %s VALUES (", td->name);
+	insert_stmt = tal_fmt(td, "INSERT INTO %s VALUES (", td->name);
 	/* If no created_index, create explicit rowid */
 	if (!td->has_created_index) {
 		tal_append_fmt(&create_stmt, "rowid INTEGER PRIMARY KEY, ");
-		tal_append_fmt(&td->insert_stmt, "?, ");
+		tal_append_fmt(&insert_stmt, "?, ");
+		td->delete_stmt = NULL;
+	} else {
+		td->delete_stmt = tal_fmt(td, "DELETE FROM %s WHERE created_index = ?;", td->name);
 	}
 
 	/* If we're a child array, we reference the parent column */
@@ -1586,7 +1591,7 @@ static void finish_td(struct plugin *plugin, struct table_desc *td)
 			       "row INTEGER REFERENCES %s(%s) ON DELETE CASCADE,"
 			       " arrindex INTEGER",
 			       parent->name, primary_key_name(parent));
-		tal_append_fmt(&td->insert_stmt, "?,?");
+		tal_append_fmt(&insert_stmt, "?,?");
 		sep = ",";
 	}
 
@@ -1594,11 +1599,11 @@ static void finish_td(struct plugin *plugin, struct table_desc *td)
 		const struct column *col = td->columns[i];
 
 		if (col->sub) {
-			add_sub_object(&td->insert_stmt, &create_stmt,
+			add_sub_object(&insert_stmt, &create_stmt,
 				       &sep, col->sub);
 			continue;
 		}
-		tal_append_fmt(&td->insert_stmt, "%s?", sep);
+		tal_append_fmt(&insert_stmt, "%s?", sep);
 		tal_append_fmt(&create_stmt, "%s%s %s",
 			       sep,
 			       col->dbname,
@@ -1609,7 +1614,8 @@ static void finish_td(struct plugin *plugin, struct table_desc *td)
 		sep = ",";
 	}
 	tal_append_fmt(&create_stmt, ");");
-	tal_append_fmt(&td->insert_stmt, ");");
+	tal_append_fmt(&insert_stmt, ");");
+	td->insert_stmt = insert_stmt;
 
 	err = sqlite3_exec(sql->db, create_stmt, NULL, NULL, &errmsg);
 	if (err != SQLITE_OK)

Risk score

Why this scored 17/100

Our methodology →

Potential impact 2/30

Exploitability 1/25

Stealth signal 1/15

Affected reach 2/15

Confidence 8/10

Evidence quality 3/5

Human-validated context

Community notes

Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.

No validated notes yet.

The AI analysis stands alone for now. Submit a note if you can add evidence or important context.

What the developer wrote

What changed, and why it matters

Security signals we found

Evidence from the diff

Changed components

Why this scored 17/100

Community notes

Related watches

contrib: add jaonoctus's signing key

wallet: fixed timestamps for invented bookkeeper deposits

docs: document missing and fix erroneous JSON-RPC error codes