From bbbb9e190f535b580072aee7fef07d89fa457a1b Mon Sep 17 00:00:00 2001 From: "Rodrigo Rodriguez (Pragmatismo)" Date: Fri, 2 Jan 2026 18:20:04 -0300 Subject: [PATCH] Allow dynamic tables from app_generator in db_api - Added table_exists_in_database() to check if table exists in PostgreSQL - Updated validate_table_name() to allow valid identifiers (not just whitelist) - Added validate_table_name_with_conn() for full validation with DB check - Added is_table_allowed_with_conn() for handlers to verify table existence - Updated list_records_handler and count_records_handler to use dynamic check - Uses parameterized query for table existence check (SQL injection safe) --- src/basic/keywords/db_api.rs | 34 ++++++++++- src/security/sql_guard.rs | 108 ++++++++++++++++++++++++++++++++++- 2 files changed, 137 insertions(+), 5 deletions(-) diff --git a/src/basic/keywords/db_api.rs b/src/basic/keywords/db_api.rs index 452c0f879..a48c266d2 100644 --- a/src/basic/keywords/db_api.rs +++ b/src/basic/keywords/db_api.rs @@ -6,7 +6,7 @@ use crate::core::shared::sanitize_identifier; use crate::core::urls::ApiUrls; use crate::security::error_sanitizer::log_and_sanitize; use crate::security::sql_guard::{ - build_safe_count_query, build_safe_select_query, validate_table_name, + build_safe_count_query, build_safe_select_query, is_table_allowed_with_conn, validate_table_name, }; use axum::{ extract::{Path, Query, State}, @@ -126,7 +126,7 @@ pub async fn list_records_handler( let limit = params.limit.unwrap_or(20).min(100); let offset = params.offset.unwrap_or(0); - // Validate table name against whitelist + // Validate table name (basic check - no SQL injection) if let Err(e) = validate_table_name(&table_name) { warn!("Invalid table name attempted: {} - {}", table_name, e); return ( @@ -150,6 +150,16 @@ pub async fn list_records_handler( } }; + // Check if table actually exists in database (supports dynamic tables from app_generator) + if !is_table_allowed_with_conn(&mut conn, &table_name) { + warn!("Table not found in database: {}", table_name); + return ( + StatusCode::NOT_FOUND, + Json(json!({ "error": format!("Table '{}' not found", table_name) })), + ) + .into_response(); + } + // Check table-level read access let access_info = match check_table_access(&mut conn, &table_name, &user_roles, AccessType::Read) { @@ -631,6 +641,16 @@ pub async fn count_records_handler( let table_name = sanitize_identifier(&table); let user_roles = user_roles_from_headers(&headers); + // Validate table name (basic check - no SQL injection) + if let Err(e) = validate_table_name(&table_name) { + warn!("Invalid table name attempted: {} - {}", table_name, e); + return ( + StatusCode::BAD_REQUEST, + Json(json!({ "error": "Invalid table name" })), + ) + .into_response(); + } + let mut conn = match state.conn.get() { Ok(c) => c, Err(e) => { @@ -639,6 +659,16 @@ pub async fn count_records_handler( } }; + // Check if table actually exists in database (supports dynamic tables from app_generator) + if !is_table_allowed_with_conn(&mut conn, &table_name) { + warn!("Table not found in database: {}", table_name); + return ( + StatusCode::NOT_FOUND, + Json(json!({ "error": format!("Table '{}' not found", table_name) })), + ) + .into_response(); + } + // Check table-level read access (count requires read permission) if let Err(e) = check_table_access(&mut conn, &table_name, &user_roles, AccessType::Read) { return (StatusCode::FORBIDDEN, Json(json!({ "error": e }))).into_response(); diff --git a/src/security/sql_guard.rs b/src/security/sql_guard.rs index 0db51c94a..741e381c9 100644 --- a/src/security/sql_guard.rs +++ b/src/security/sql_guard.rs @@ -1,7 +1,8 @@ use std::collections::HashSet; use std::sync::LazyLock; -static ALLOWED_TABLES: LazyLock> = LazyLock::new(|| { +/// System tables that are always allowed (core application tables) +static SYSTEM_TABLES: LazyLock> = LazyLock::new(|| { HashSet::from([ "automations", "bots", @@ -33,6 +34,35 @@ static ALLOWED_TABLES: LazyLock> = LazyLock::new(|| { ]) }); +/// Check if a table exists in the database +/// This allows dynamically created tables (from app generator) to be accessed +pub fn table_exists_in_database(conn: &mut diesel::PgConnection, table_name: &str) -> bool { + use diesel::prelude::*; + use diesel::sql_query; + use diesel::sql_types::Text; + + #[derive(diesel::QueryableByName)] + struct TableExists { + #[diesel(sql_type = diesel::sql_types::Bool)] + exists: bool, + } + + let result: Result = sql_query( + "SELECT EXISTS ( + SELECT FROM information_schema.tables + WHERE table_schema = 'public' + AND table_name = $1 + ) as exists" + ) + .bind::(table_name) + .get_result(conn); + + match result { + Ok(r) => r.exists, + Err(_) => false, + } +} + static ALLOWED_ORDER_COLUMNS: LazyLock> = LazyLock::new(|| { HashSet::from([ "id", @@ -75,10 +105,59 @@ impl std::fmt::Display for SqlGuardError { impl std::error::Error for SqlGuardError {} +/// Validate table name - checks system tables whitelist only +/// For full validation including dynamic tables, use validate_table_name_with_conn pub fn validate_table_name(table: &str) -> Result<&str, SqlGuardError> { let sanitized = sanitize_identifier(table); - if ALLOWED_TABLES.contains(sanitized.as_str()) { + // Check basic identifier validity (no SQL injection) + if sanitized.is_empty() || sanitized.len() > 63 { + return Err(SqlGuardError::InvalidTableName(table.to_string())); + } + + // Check for dangerous patterns + if sanitized.contains(';') || sanitized.contains("--") || sanitized.contains("/*") { + return Err(SqlGuardError::PotentialInjection(table.to_string())); + } + + // System tables are always allowed + if SYSTEM_TABLES.contains(sanitized.as_str()) { + return Ok(table); + } + + // For non-system tables, we allow them if they look safe + // The actual existence check should be done with validate_table_name_with_conn + // This allows dynamically created tables from app_generator to work + if sanitized.chars().all(|c| c.is_alphanumeric() || c == '_') { + Ok(table) + } else { + Err(SqlGuardError::InvalidTableName(table.to_string())) + } +} + +/// Validate table name with database connection - checks if table actually exists +pub fn validate_table_name_with_conn<'a>( + conn: &mut diesel::PgConnection, + table: &'a str, +) -> Result<&'a str, SqlGuardError> { + let sanitized = sanitize_identifier(table); + + // First do basic validation + if sanitized.is_empty() || sanitized.len() > 63 { + return Err(SqlGuardError::InvalidTableName(table.to_string())); + } + + if sanitized.contains(';') || sanitized.contains("--") || sanitized.contains("/*") { + return Err(SqlGuardError::PotentialInjection(table.to_string())); + } + + // System tables are always allowed + if SYSTEM_TABLES.contains(sanitized.as_str()) { + return Ok(table); + } + + // Check if table exists in database (for dynamically created tables) + if table_exists_in_database(conn, &sanitized) { Ok(table) } else { Err(SqlGuardError::InvalidTableName(table.to_string())) @@ -229,9 +308,32 @@ pub fn register_dynamic_table(table_name: &'static str) { log::info!("Dynamic table registration requested for: {}", table_name); } +/// Check if table is in the system tables whitelist +pub fn is_system_table(table: &str) -> bool { + let sanitized = sanitize_identifier(table); + SYSTEM_TABLES.contains(sanitized.as_str()) +} + +/// Check if table is allowed (system table or valid identifier) +/// For full check including database existence, use is_table_allowed_with_conn pub fn is_table_allowed(table: &str) -> bool { let sanitized = sanitize_identifier(table); - ALLOWED_TABLES.contains(sanitized.as_str()) + if SYSTEM_TABLES.contains(sanitized.as_str()) { + return true; + } + // Allow valid identifiers (actual existence checked elsewhere) + !sanitized.is_empty() + && sanitized.len() <= 63 + && sanitized.chars().all(|c| c.is_alphanumeric() || c == '_') +} + +/// Check if table is allowed with database connection +pub fn is_table_allowed_with_conn(conn: &mut diesel::PgConnection, table: &str) -> bool { + let sanitized = sanitize_identifier(table); + if SYSTEM_TABLES.contains(sanitized.as_str()) { + return true; + } + table_exists_in_database(conn, &sanitized) } #[cfg(test)]