Merge 'prevent modification to system tables.' from Glauber Costa

SQLite does not allow us to modify system tables, but we do. Let's fix
it.

Reviewed-by: Preston Thorpe <preston@turso.tech>
Reviewed-by: Avinash Sajjanshetty (@avinassh)

Closes #2855
This commit is contained in:
Preston Thorpe
2025-09-04 19:57:04 -04:00
committed by GitHub
5 changed files with 34 additions and 2 deletions

View File

@@ -43,6 +43,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<String, Arc<Table>>,

View File

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

View File

@@ -23,6 +23,12 @@ pub fn translate_delete(
connection: &Arc<crate::Connection>,
) -> Result<ProgramBuilder> {
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.

View File

@@ -75,6 +75,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),

View File

@@ -59,7 +59,7 @@ pub fn translate_update(
mut program: ProgramBuilder,
connection: &Arc<crate::Connection>,
) -> crate::Result<ProgramBuilder> {
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<ProgramBuilder> {
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<crate::Connection>,
is_internal_schema_change: bool,
) -> crate::Result<Plan> {
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.