diff --git a/crates/sqlite-inproc/Cargo.toml b/crates/sqlite-inproc/Cargo.toml index 2cb06ae4f3..30f6b77056 100644 --- a/crates/sqlite-inproc/Cargo.toml +++ b/crates/sqlite-inproc/Cargo.toml @@ -8,7 +8,7 @@ edition = { workspace = true } anyhow = { workspace = true } async-trait = { workspace = true } futures = { workspace = true } -rusqlite = { workspace = true, features = ["bundled"] } +rusqlite = { workspace = true, features = ["bundled", "hooks"] } spin-factor-sqlite = { path = "../factor-sqlite" } spin-wasi-async = { path = "../wasi-async" } spin-world = { path = "../world" } diff --git a/crates/sqlite-inproc/src/lib.rs b/crates/sqlite-inproc/src/lib.rs index dc9cedf79e..9272847dea 100644 --- a/crates/sqlite-inproc/src/lib.rs +++ b/crates/sqlite-inproc/src/lib.rs @@ -46,14 +46,19 @@ impl InProcDatabaseLocation { /// A connection to a sqlite database pub struct InProcConnection { location: InProcDatabaseLocation, + allow_attach_file: bool, connection: OnceLock>>, } impl InProcConnection { - pub fn new(location: InProcDatabaseLocation) -> Result { + pub fn new( + location: InProcDatabaseLocation, + allow_attach_file: bool, + ) -> Result { let connection = OnceLock::new(); Ok(Self { location, + allow_attach_file, connection, }) } @@ -74,6 +79,18 @@ impl InProcConnection { InProcDatabaseLocation::Path(path) => rusqlite::Connection::open(path), } .map_err(|e| sqlite::Error::Io(e.to_string()))?; + if !self.allow_attach_file { + connection.authorizer(Some(|ctx: rusqlite::hooks::AuthContext<'_>| { + use rusqlite::hooks::{AuthAction, Authorization}; + match ctx.action { + // Deny attaching files except tempfile ("") and in-memory (":memory:") databases + AuthAction::Attach { filename } if !matches!(filename, "" | ":memory:") => { + Authorization::Deny + } + _ => Authorization::Allow, + } + })); + } Ok(Arc::new(Mutex::new(connection))) } } diff --git a/crates/sqlite/src/lib.rs b/crates/sqlite/src/lib.rs index 35467c6925..f39696e686 100644 --- a/crates/sqlite/src/lib.rs +++ b/crates/sqlite/src/lib.rs @@ -126,7 +126,7 @@ impl RuntimeConfigResolver { .map(|p| p.join(DEFAULT_SQLITE_DB_FILENAME)); let factory = move || { let location = InProcDatabaseLocation::from_path(path.clone())?; - let connection = spin_sqlite_inproc::InProcConnection::new(location)?; + let connection = spin_sqlite_inproc::InProcConnection::new(location, false)?; Ok(Arc::new(connection) as _) }; Arc::new(factory) @@ -140,6 +140,13 @@ const DEFAULT_SQLITE_DB_FILENAME: &str = "sqlite_db.db"; #[serde(deny_unknown_fields)] pub struct InProcDatabase { pub path: Option, + + /// If `false` (the default), disallows `ATTACH`ing an existing file to a + /// database connection. + /// + /// Note: Attaching a new tempfile or `:memory:` database is always allowed. + #[serde(default)] + pub allow_attach_file: bool, } impl InProcDatabase { @@ -156,7 +163,10 @@ impl InProcDatabase { .map(|p| resolve_relative_path(p, base_dir)); let location = InProcDatabaseLocation::from_path(path)?; let factory = move || { - let connection = spin_sqlite_inproc::InProcConnection::new(location.clone())?; + let connection = spin_sqlite_inproc::InProcConnection::new( + location.clone(), + self.allow_attach_file, + )?; Ok(Arc::new(connection) as _) }; Ok(factory) diff --git a/tests/test-components/components/sqlite/src/lib.rs b/tests/test-components/components/sqlite/src/lib.rs index 302653145e..f7c7cc30db 100644 --- a/tests/test-components/components/sqlite/src/lib.rs +++ b/tests/test-components/components/sqlite/src/lib.rs @@ -53,9 +53,23 @@ impl Component { // This should exceed the 128MB query result limit: match conn.execute("SELECT * FROM test_data", &[]) { - Ok(_) => bail!("large select should not have succeeded",), + Ok(_) => bail!("large select should not have succeeded"), Err(Error::Io(s)) if s.contains("query result exceeds limit") => {} - Err(e) => bail!("unexpected error: {e}",), + Err(e) => bail!("unexpected error: {e}"), + } + + // ATTACH tempfile and in-memory are allowed + for allowed_stmt in ["ATTACH '' AS tempfile", "ATTACH ':memory:' AS inmemory"] { + conn.execute(allowed_stmt, &[]) + .map_err(|e| format!("{allowed_stmt:?} failed: {e:?}"))?; + } + // ATTACH forbidden by default; VACUUM INTO uses ATTACH under the hood + for forbidden_stmt in ["ATTACH 'any_file' AS attach_file", "VACUUM INTO 'any_file'"] { + match conn.execute(forbidden_stmt, &[]) { + Ok(_) => bail!("{forbidden_stmt:?} should fail"), + Err(Error::Io(s)) if s.contains("authoriz") => {} + Err(e) => bail!("unexpected error for {forbidden_stmt:?}: {e:?}"), + } } Ok(()) diff --git a/tests/test-components/helper/src/lib.rs b/tests/test-components/helper/src/lib.rs index 20d5209dcb..f810898c2f 100644 --- a/tests/test-components/helper/src/lib.rs +++ b/tests/test-components/helper/src/lib.rs @@ -109,7 +109,7 @@ pub fn outgoing_body(body: OutgoingBody, buffer: Vec) -> Result<(), ErrorCod offset += count; } Err(e) => { - return Err(ErrorCode::InternalError(Some(format!("I/O error: {e}")))) + return Err(ErrorCode::InternalError(Some(format!("I/O error: {e}")))); } } } @@ -174,12 +174,12 @@ macro_rules! ensure_eq { #[macro_export] macro_rules! bail { - ($fmt:expr, $($arg:tt)*) => {{ + ($fmt:expr $(, $($arg:tt)*)?) => {{ let krate = module_path!().split("::").next().unwrap(); let file = file!(); let line = line!(); return Err(format!( - "{krate}#({file}:{line}) {}", format_args!($fmt, $($arg)*) + "{krate}#({file}:{line}) {}", format_args!($fmt $(, $($arg)*)?) )); }}; }