From 032eabb3a447f46b88bafd85d232f20dbc786460 Mon Sep 17 00:00:00 2001 From: Glauber Costa Date: Fri, 29 Aug 2025 20:02:49 -0500 Subject: [PATCH] prevent modification to system tables. SQLite does not allow us to modify system tables, but we do. Let's fix it. --- core/schema.rs | 6 ++++++ core/translate/alter.rs | 6 ++++++ core/translate/delete.rs | 6 ++++++ core/translate/insert.rs | 6 ++++++ core/translate/update.rs | 12 ++++++++++-- 5 files changed, 34 insertions(+), 2 deletions(-) diff --git a/core/schema.rs b/core/schema.rs index a4f129645..382e53fa7 100644 --- a/core/schema.rs +++ b/core/schema.rs @@ -41,6 +41,12 @@ use turso_parser::{ const SCHEMA_TABLE_NAME: &str = "sqlite_schema"; const SCHEMA_TABLE_NAME_ALT: &str = "sqlite_master"; +/// Check if a table name refers to a system table that should be protected from direct writes +pub fn is_system_table(table_name: &str) -> bool { + let normalized = table_name.to_lowercase(); + normalized == SCHEMA_TABLE_NAME || normalized == SCHEMA_TABLE_NAME_ALT +} + #[derive(Debug)] pub struct Schema { pub tables: HashMap>, diff --git a/core/translate/alter.rs b/core/translate/alter.rs index 0990e90ac..a72af5cb2 100644 --- a/core/translate/alter.rs +++ b/core/translate/alter.rs @@ -28,6 +28,12 @@ pub fn translate_alter_table( body: alter_table, } = alter; let table_name = table_name.name.as_str(); + + // Check if someone is trying to ALTER a system table + if crate::schema::is_system_table(table_name) { + crate::bail_parse_error!("table {} may not be modified", table_name); + } + if schema.table_has_indexes(table_name) && !schema.indexes_enabled() { // Let's disable altering a table with indices altogether instead of checking column by // column to be extra safe. diff --git a/core/translate/delete.rs b/core/translate/delete.rs index 5dd26ee8c..86eb069d9 100644 --- a/core/translate/delete.rs +++ b/core/translate/delete.rs @@ -23,6 +23,12 @@ pub fn translate_delete( connection: &Arc, ) -> Result { let tbl_name = normalize_ident(tbl_name.name.as_str()); + + // Check if this is a system table that should be protected from direct writes + if crate::schema::is_system_table(&tbl_name) { + crate::bail_parse_error!("table {} may not be modified", tbl_name); + } + if schema.table_has_indexes(&tbl_name) && !schema.indexes_enabled() { // Let's disable altering a table with indices altogether instead of checking column by // column to be extra safe. diff --git a/core/translate/insert.rs b/core/translate/insert.rs index a4b9e7bf0..451fdac60 100644 --- a/core/translate/insert.rs +++ b/core/translate/insert.rs @@ -72,6 +72,12 @@ pub fn translate_insert( ); } let table_name = &tbl_name.name; + + // Check if this is a system table that should be protected from direct writes + if crate::schema::is_system_table(table_name.as_str()) { + crate::bail_parse_error!("table {} may not be modified", table_name); + } + let table = match schema.get_table(table_name.as_str()) { Some(table) => table, None => crate::bail_parse_error!("no such table: {}", table_name), diff --git a/core/translate/update.rs b/core/translate/update.rs index f94ebe118..3ceb2d6c7 100644 --- a/core/translate/update.rs +++ b/core/translate/update.rs @@ -59,7 +59,7 @@ pub fn translate_update( mut program: ProgramBuilder, connection: &Arc, ) -> crate::Result { - let mut plan = prepare_update_plan(&mut program, schema, body, connection)?; + let mut plan = prepare_update_plan(&mut program, schema, body, connection, false)?; optimize_plan(&mut plan, schema)?; // TODO: freestyling these numbers let opts = ProgramBuilderOpts { @@ -81,7 +81,7 @@ pub fn translate_update_for_schema_change( ddl_query: &str, after: impl FnOnce(&mut ProgramBuilder), ) -> crate::Result { - let mut plan = prepare_update_plan(&mut program, schema, body, connection)?; + let mut plan = prepare_update_plan(&mut program, schema, body, connection, true)?; if let Plan::Update(plan) = &mut plan { if program.capture_data_changes_mode().has_updates() { @@ -106,6 +106,7 @@ pub fn prepare_update_plan( schema: &Schema, body: &mut ast::Update, connection: &Arc, + is_internal_schema_change: bool, ) -> crate::Result { if body.with.is_some() { bail_parse_error!("WITH clause is not supported in UPDATE"); @@ -121,6 +122,13 @@ pub fn prepare_update_plan( bail_parse_error!("INDEXED BY clause is not supported in UPDATE"); } let table_name = &body.tbl_name.name; + + // Check if this is a system table that should be protected from direct writes + // Skip this check for internal schema change operations (like ALTER TABLE) + if !is_internal_schema_change && crate::schema::is_system_table(table_name.as_str()) { + bail_parse_error!("table {} may not be modified", table_name); + } + if schema.table_has_indexes(&table_name.to_string()) && !schema.indexes_enabled() { // Let's disable altering a table with indices altogether instead of checking column by // column to be extra safe.