fix: harden security, reduce duplication, and improve robustness
- Fix SQL injection in data.rs by wrapping get_table_data in READ ONLY transaction - Fix SQL injection in docker.rs CREATE DATABASE via escape_ident - Fix command injection in docker.rs by validating pg_version/container_name and escaping shell-interpolated values - Fix UTF-8 panic on stderr truncation with char_indices - Wrap delete_rows in a transaction for atomicity - Replace .expect() with proper error propagation in lib.rs - Cache AI settings in AppState to avoid repeated disk reads - Cap JSONB column discovery at 50 to prevent unbounded queries - Fix ERD colorMode to respect system theme via useTheme() - Extract AppState::get_pool() replacing ~19 inline pool patterns - Extract shared AiSettingsFields component (DRY popover + sheet) - Make get_connections_path pub(crate) and reuse from docker.rs - Deduplicate check_docker by delegating to check_docker_internal Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -21,10 +21,7 @@ pub async fn get_table_data(
|
||||
sort_direction: Option<String>,
|
||||
filter: Option<String>,
|
||||
) -> TuskResult<PaginatedQueryResult> {
|
||||
let pools = state.pools.read().await;
|
||||
let pool = pools
|
||||
.get(&connection_id)
|
||||
.ok_or(TuskError::NotConnected(connection_id))?;
|
||||
let pool = state.get_pool(&connection_id).await?;
|
||||
|
||||
let qualified = format!("{}.{}", escape_ident(&schema), escape_ident(&table));
|
||||
|
||||
@@ -56,11 +53,24 @@ pub async fn get_table_data(
|
||||
|
||||
let start = Instant::now();
|
||||
|
||||
let (rows, count_row) = tokio::try_join!(
|
||||
sqlx::query(&data_sql).fetch_all(pool),
|
||||
sqlx::query(&count_sql).fetch_one(pool),
|
||||
)
|
||||
.map_err(TuskError::Database)?;
|
||||
// Always run table data queries in a read-only transaction to prevent
|
||||
// writable CTEs or other mutation via the raw filter parameter.
|
||||
let mut tx = (&pool).begin().await.map_err(TuskError::Database)?;
|
||||
sqlx::query("SET TRANSACTION READ ONLY")
|
||||
.execute(&mut *tx)
|
||||
.await
|
||||
.map_err(TuskError::Database)?;
|
||||
|
||||
let rows = sqlx::query(&data_sql)
|
||||
.fetch_all(&mut *tx)
|
||||
.await
|
||||
.map_err(TuskError::Database)?;
|
||||
let count_row = sqlx::query(&count_sql)
|
||||
.fetch_one(&mut *tx)
|
||||
.await
|
||||
.map_err(TuskError::Database)?;
|
||||
|
||||
tx.rollback().await.map_err(TuskError::Database)?;
|
||||
|
||||
let execution_time_ms = start.elapsed().as_millis();
|
||||
let total_rows: i64 = count_row.get(0);
|
||||
@@ -134,10 +144,7 @@ pub async fn update_row(
|
||||
return Err(TuskError::ReadOnly);
|
||||
}
|
||||
|
||||
let pools = state.pools.read().await;
|
||||
let pool = pools
|
||||
.get(&connection_id)
|
||||
.ok_or(TuskError::NotConnected(connection_id))?;
|
||||
let pool = state.get_pool(&connection_id).await?;
|
||||
|
||||
let qualified = format!("{}.{}", escape_ident(&schema), escape_ident(&table));
|
||||
|
||||
@@ -155,7 +162,7 @@ pub async fn update_row(
|
||||
let mut query = sqlx::query(&sql);
|
||||
query = bind_json_value(query, &value);
|
||||
query = query.bind(ctid_val);
|
||||
query.execute(pool).await.map_err(TuskError::Database)?;
|
||||
query.execute(&pool).await.map_err(TuskError::Database)?;
|
||||
} else {
|
||||
let where_parts: Vec<String> = pk_columns
|
||||
.iter()
|
||||
@@ -174,7 +181,7 @@ pub async fn update_row(
|
||||
for pk_val in &pk_values {
|
||||
query = bind_json_value(query, pk_val);
|
||||
}
|
||||
query.execute(pool).await.map_err(TuskError::Database)?;
|
||||
query.execute(&pool).await.map_err(TuskError::Database)?;
|
||||
}
|
||||
|
||||
Ok(())
|
||||
@@ -193,10 +200,7 @@ pub async fn insert_row(
|
||||
return Err(TuskError::ReadOnly);
|
||||
}
|
||||
|
||||
let pools = state.pools.read().await;
|
||||
let pool = pools
|
||||
.get(&connection_id)
|
||||
.ok_or(TuskError::NotConnected(connection_id))?;
|
||||
let pool = state.get_pool(&connection_id).await?;
|
||||
|
||||
let qualified = format!("{}.{}", escape_ident(&schema), escape_ident(&table));
|
||||
|
||||
@@ -215,7 +219,7 @@ pub async fn insert_row(
|
||||
query = bind_json_value(query, val);
|
||||
}
|
||||
|
||||
query.execute(pool).await.map_err(TuskError::Database)?;
|
||||
query.execute(&pool).await.map_err(TuskError::Database)?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -234,14 +238,14 @@ pub async fn delete_rows(
|
||||
return Err(TuskError::ReadOnly);
|
||||
}
|
||||
|
||||
let pools = state.pools.read().await;
|
||||
let pool = pools
|
||||
.get(&connection_id)
|
||||
.ok_or(TuskError::NotConnected(connection_id))?;
|
||||
let pool = state.get_pool(&connection_id).await?;
|
||||
|
||||
let qualified = format!("{}.{}", escape_ident(&schema), escape_ident(&table));
|
||||
let mut total_affected: u64 = 0;
|
||||
|
||||
// Wrap all deletes in a transaction for atomicity
|
||||
let mut tx = (&pool).begin().await.map_err(TuskError::Database)?;
|
||||
|
||||
if pk_columns.is_empty() {
|
||||
// Fallback: use ctids for row identification
|
||||
let ctid_list = ctids.ok_or_else(|| {
|
||||
@@ -250,7 +254,7 @@ pub async fn delete_rows(
|
||||
for ctid_val in &ctid_list {
|
||||
let sql = format!("DELETE FROM {} WHERE ctid = $1::tid", qualified);
|
||||
let query = sqlx::query(&sql).bind(ctid_val);
|
||||
let result = query.execute(pool).await.map_err(TuskError::Database)?;
|
||||
let result = query.execute(&mut *tx).await.map_err(TuskError::Database)?;
|
||||
total_affected += result.rows_affected();
|
||||
}
|
||||
} else {
|
||||
@@ -269,11 +273,13 @@ pub async fn delete_rows(
|
||||
query = bind_json_value(query, val);
|
||||
}
|
||||
|
||||
let result = query.execute(pool).await.map_err(TuskError::Database)?;
|
||||
let result = query.execute(&mut *tx).await.map_err(TuskError::Database)?;
|
||||
total_affected += result.rows_affected();
|
||||
}
|
||||
}
|
||||
|
||||
tx.commit().await.map_err(TuskError::Database)?;
|
||||
|
||||
Ok(total_affected)
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user