Skip to content

sqlite-inproc: Add allow_attach_file runtime config option #3503

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lann wants to merge 1 commit into
base: main
Choose a base branch
from
Open

sqlite-inproc: Add allow_attach_file runtime config option #3503

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/sqlite-inproc/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
19 changes: 18 additions & 1 deletion crates/sqlite-inproc/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,19 @@ impl InProcDatabaseLocation {
/// A connection to a sqlite database
pub struct InProcConnection {
location: InProcDatabaseLocation,
allow_attach_file: bool,
connection: OnceLock<Arc<Mutex<rusqlite::Connection>>>,
}

impl InProcConnection {
pub fn new(location: InProcDatabaseLocation) -> Result<Self, sqlite::Error> {
pub fn new(
location: InProcDatabaseLocation,
allow_attach_file: bool,
) -> Result<Self, sqlite::Error> {
let connection = OnceLock::new();
Ok(Self {
location,
allow_attach_file,
connection,
})
}
Expand All @@ -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)))
}
}
Expand Down
14 changes: 12 additions & 2 deletions crates/sqlite/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -140,6 +140,13 @@ const DEFAULT_SQLITE_DB_FILENAME: &str = "sqlite_db.db";
#[serde(deny_unknown_fields)]
pub struct InProcDatabase {
pub path: Option<PathBuf>,

/// 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 {
Expand All @@ -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)
Expand Down
18 changes: 16 additions & 2 deletions tests/test-components/components/sqlite/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 <file> 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") => {}
Copy link
Copy Markdown
Collaborator Author

@lann lann May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Annoyingly the ATTACH and VACUUM errors are sliiiightly different, leading to this awkward check.

Err(e) => bail!("unexpected error for {forbidden_stmt:?}: {e:?}"),
}
}

Ok(())
Expand Down
6 changes: 3 additions & 3 deletions tests/test-components/helper/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ pub fn outgoing_body(body: OutgoingBody, buffer: Vec<u8>) -> 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}"))));
}
}
}
Expand Down Expand Up @@ -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)*)?)
Copy link
Copy Markdown
Collaborator Author

@lann lann May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This drive-by refactor makes the trailing comma optional when no extra args are passed, e.g. https://github.com/spinframework/spin/pull/3503/changes#diff-980855598b2b312f034dcd583093cd525c7d808c00097b4e62424c35a801d89eL56-R56

));
}};
}
Expand Down
Loading