diff --git a/libsql-server/src/error.rs b/libsql-server/src/error.rs index c3a9227752..2bb829be89 100644 --- a/libsql-server/src/error.rs +++ b/libsql-server/src/error.rs @@ -297,6 +297,8 @@ pub enum LoadDumpError { NoCommit, #[error("Path is not a file")] NotAFile, + #[error("The passed dump sql is invalid: {0}")] + InvalidSqlInput(String), } impl ResponseError for LoadDumpError {} @@ -315,7 +317,8 @@ impl IntoResponse for &LoadDumpError { | NoTxn | NoCommit | NotAFile - | DumpFilePathNotAbsolute => self.format_err(StatusCode::BAD_REQUEST), + | DumpFilePathNotAbsolute + | InvalidSqlInput(_) => self.format_err(StatusCode::BAD_REQUEST), } } } diff --git a/libsql-server/src/namespace/configurator/helpers.rs b/libsql-server/src/namespace/configurator/helpers.rs index 00c4cbc4ff..081eeaef80 100644 --- a/libsql-server/src/namespace/configurator/helpers.rs +++ b/libsql-server/src/namespace/configurator/helpers.rs @@ -11,6 +11,7 @@ use futures::Stream; use libsql_sys::EncryptionConfig; use libsql_wal::io::StdIO; use libsql_wal::registry::WalRegistry; +use rusqlite::hooks::{AuthAction, AuthContext, Authorization}; use tokio::io::AsyncBufReadExt as _; use tokio::task::JoinSet; use tokio_util::io::StreamReader; @@ -328,8 +329,31 @@ where line = tokio::task::spawn_blocking({ let conn = conn.clone(); move || -> crate::Result { - conn.with_raw(|conn| conn.execute(&line, ())).map_err(|e| { - LoadDumpError::Internal(format!("line: {}, error: {}", line_id, e)) + conn.with_raw(|conn| { + conn.authorizer(Some(|auth: AuthContext<'_>| match auth.action { + AuthAction::Attach { filename: _ } => Authorization::Deny, + _ => Authorization::Allow, + })); + conn.execute(&line, ()) + }) + .map_err(|e| match e { + rusqlite::Error::SqlInputError { + msg, sql, offset, .. + } => { + let msg = if sql.to_lowercase().contains("attach") { + format!( + "attach statements are not allowed in dumps, msg: {}, sql: {}, offset: {}", + msg, + sql, + offset + ) + } else { + format!("msg: {}, sql: {}, offset: {}", msg, sql, offset) + }; + + LoadDumpError::InvalidSqlInput(msg) + } + e => LoadDumpError::Internal(format!("line: {}, error: {}", line_id, e)), })?; Ok(line) } diff --git a/libsql-server/tests/namespaces/dumps.rs b/libsql-server/tests/namespaces/dumps.rs index 93b7e1676c..1ef1870d12 100644 --- a/libsql-server/tests/namespaces/dumps.rs +++ b/libsql-server/tests/namespaces/dumps.rs @@ -279,3 +279,149 @@ fn export_dump() { sim.run().unwrap(); } + +#[test] +fn load_dump_with_attach_rejected() { + const DUMP: &str = r#" + PRAGMA foreign_keys=OFF; + BEGIN TRANSACTION; + CREATE TABLE test (x); + INSERT INTO test VALUES(42); + ATTACH foo/bar.sql + COMMIT;"#; + + let mut sim = Builder::new() + .simulation_duration(Duration::from_secs(1000)) + .build(); + let tmp = tempdir().unwrap(); + let tmp_path = tmp.path().to_path_buf(); + + std::fs::write(tmp_path.join("dump.sql"), DUMP).unwrap(); + + make_primary(&mut sim, tmp.path().to_path_buf()); + + sim.client("client", async move { + let client = Client::new(); + + // path is not absolute is an error + let resp = client + .post( + "http://primary:9090/v1/namespaces/foo/create", + json!({ "dump_url": "file:dump.sql"}), + ) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + + // path doesn't exist is an error + let resp = client + .post( + "http://primary:9090/v1/namespaces/foo/create", + json!({ "dump_url": "file:/dump.sql"}), + ) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + + let resp = client + .post( + "http://primary:9090/v1/namespaces/foo/create", + json!({ "dump_url": format!("file:{}", tmp_path.join("dump.sql").display())}), + ) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::BAD_REQUEST, + "{}", + resp.json::().await.unwrap_or_default() + ); + + assert_snapshot!(resp.body_string().await?); + + let foo = + Database::open_remote_with_connector("http://foo.primary:8080", "", TurmoilConnector)?; + let foo_conn = foo.connect()?; + + let res = foo_conn.query("select count(*) from test", ()).await; + // This should error since the dump should have failed! + assert!(res.is_err()); + + Ok(()) + }); + + sim.run().unwrap(); +} + +#[test] +fn load_dump_with_invalid_sql() { + const DUMP: &str = r#" + PRAGMA foreign_keys=OFF; + BEGIN TRANSACTION; + CREATE TABLE test (x); + INSERT INTO test VALUES(42); + SELECT abs(-9223372036854775808) + COMMIT;"#; + + let mut sim = Builder::new() + .simulation_duration(Duration::from_secs(1000)) + .build(); + let tmp = tempdir().unwrap(); + let tmp_path = tmp.path().to_path_buf(); + + std::fs::write(tmp_path.join("dump.sql"), DUMP).unwrap(); + + make_primary(&mut sim, tmp.path().to_path_buf()); + + sim.client("client", async move { + let client = Client::new(); + + // path is not absolute is an error + let resp = client + .post( + "http://primary:9090/v1/namespaces/foo/create", + json!({ "dump_url": "file:dump.sql"}), + ) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + + // path doesn't exist is an error + let resp = client + .post( + "http://primary:9090/v1/namespaces/foo/create", + json!({ "dump_url": "file:/dump.sql"}), + ) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + + let resp = client + .post( + "http://primary:9090/v1/namespaces/foo/create", + json!({ "dump_url": format!("file:{}", tmp_path.join("dump.sql").display())}), + ) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::BAD_REQUEST, + "{}", + resp.json::().await.unwrap_or_default() + ); + + assert_snapshot!(resp.body_string().await?); + + let foo = + Database::open_remote_with_connector("http://foo.primary:8080", "", TurmoilConnector)?; + let foo_conn = foo.connect()?; + + let res = foo_conn.query("select count(*) from test", ()).await; + // This should error since the dump should have failed! + assert!(res.is_err()); + + Ok(()) + }); + + sim.run().unwrap(); +} diff --git a/libsql-server/tests/namespaces/snapshots/tests__namespaces__dumps__load_dump_with_attach_rejected.snap b/libsql-server/tests/namespaces/snapshots/tests__namespaces__dumps__load_dump_with_attach_rejected.snap new file mode 100644 index 0000000000..57809be5f1 --- /dev/null +++ b/libsql-server/tests/namespaces/snapshots/tests__namespaces__dumps__load_dump_with_attach_rejected.snap @@ -0,0 +1,6 @@ +--- +source: libsql-server/tests/namespaces/dumps.rs +expression: resp.body_string().await? +snapshot_kind: text +--- +{"error":"The passed dump sql is invalid: attach statements are not allowed in dumps, msg: near \"COMMIT\": syntax error, sql: ATTACH foo/bar.sql\n COMMIT;, offset: 28"} diff --git a/libsql-server/tests/namespaces/snapshots/tests__namespaces__dumps__load_dump_with_invalid_sql.snap b/libsql-server/tests/namespaces/snapshots/tests__namespaces__dumps__load_dump_with_invalid_sql.snap new file mode 100644 index 0000000000..e1f600116f --- /dev/null +++ b/libsql-server/tests/namespaces/snapshots/tests__namespaces__dumps__load_dump_with_invalid_sql.snap @@ -0,0 +1,6 @@ +--- +source: libsql-server/tests/namespaces/dumps.rs +expression: resp.body_string().await? +snapshot_kind: text +--- +{"error":"The passed dump sql is invalid: msg: near \"COMMIT\": syntax error, sql: SELECT abs(-9223372036854775808) \n COMMIT;, offset: 43"}