feat: require explicit registration of absolute database paths by onehumandev · Pull Request #50 · silvermine/tauri-plugin-sqlite

onehumandev · 2026-06-09T16:31:31Z

Problem:
Previously, only databases that lived in app_config_dir could be opened with this plugin.
The user passed in the path relative to app_config_dir to load the database.
This meant that there was no support for databases that may live anywhere else within the app, which is a reasonable requirement. For example, some databases may need to live the Library folder on an iOS app so the database will be kept as part of the app backup.

Solution:
Databases can now only be opened by an exact, pre-registered absolute path (in-memory databases are the sole exception). Paths are registered via Builder::allow_path or, more commonly, in the new Builder::on_setup hook, which runs during plugin setup once app exists — so paths derived from app.path().app_data_dir() and friends can be computed and
registered at startup.
Registered paths are canonicalized once at setup so the equality check is symlink-safe.
The resolver canonicalizes each requested path and requires an exact allowlist match.
.. segments and null bytes are always rejected.
Migration registration flows through the same resolver, so a migration's key must be a registered absolute path.

Why no relative paths:
A relative-path default (app_config_dir) is an implicit grant — every file under that dir becomes reachable without the developer opting in. Removing it makes the set of reachable databases exactly the set the developer registered, with no ambient authority, and gives exactly one path to accessing a db.

Why no registered root folders:
Root allow-listing grants a whole subtree, including files created later that the developer never considered. Prefix matching is also easy to get subtly wrong against symlinks and traversal. Exact-path allow-listing is the least-privilege equivalent: explicit, auditable, and unambiguous.

BREAKING CHANGE: relative paths are removed.
load() now requires an absolute path registered via allow_path / on_setup. Previously-relative callers must register the absolute path and pass it from the frontend.
It is the responsibility of the caller to ensure a consistent passing down of the needed path.

onehumandev · 2026-06-09T16:31:46Z

@velocitysystems For your review :) Thanks!

jjhafer · 2026-06-09T18:33:51Z

@onehumandev Added some comments.

velocitysystems · 2026-06-10T15:03:23Z

+   tauri::Builder::default()
+      .plugin(
+         Builder::new()
+            .on_setup(|app, reg| {


The dynamic-path setup reads as more boilerplate than the old add_migrations("main.db", …) one-liner, and some of that looks avoidable without giving up the closure's flexibility (arbitrary, app-derived absolute paths).

Two things stand out in the example:

We register the same database twice — allow_path(&db) and add_migrations(...) — and have to keep the keys in sync by hand. That's the same coupling behind the byte-identical-key concern: if the two spellings ever drift, migrations silently attach to a key load() never matches.

The two calls take different types for the same value: allow_path takes &db (a path) while add_migrations takes db.to_string_lossy().into_owned() (a String).

Two ways to collapse this to a singlecall, both keeping &Path and canonicalizing to the same key internally:

Option A — add_migrations auto-allowlists. Have it take impl AsRef<Path> and register the path on the allowlist itself. You can't migrate a database you're not allowed to open, so allow-listing a migration target is implied:

.on_setup(|app, reg| { let db = app.path().app_data_dir()?.join("main.db"); reg.add_migrations(&db, sqlx::migrate!("./migrations")); // allows + migrates Ok(()) })

allow_path(&db) stays for databases without migrations. Downside: add_migrations now has an allowlist side effect, which is slightly less obvious from its name.

Option B — chained handle. Keep allow_path as the single allowlist entry point and have it return a per-path handle you attach migrations to:

.on_setup(|app, reg| { let db = app.path().app_data_dir()?.join("main.db"); reg.allow_path(&db).with_migrations(sqlx::migrate!("./migrations")); Ok(()) })

This keeps add_migrations/allow_path responsibilities cleanly separated (allow-listing stays the one explicit gate) while still guaranteeing the migration key is the allowed path. allow_path(&db) alone still works for the no-migrations case.

Either removes the stringified key and the hand-synced double registration. What are your thoughts @onehumandev @jjhafer?

Option A makes more sense to me; when we register the path, we register the migrations with it as well.
I'll make that change.

If we go with Option A, we'll still need to retain the allow_path(..) API for databases that don't require migrations. I'm still somewhat hesitant about this approach, though, since it introduces implicit behavior into the add_migrations(..) method.

That said, the builder pattern in Option B is appealing because it keeps the API surface unchanged while making it more composable. Perhaps @jjhafer may have some thoughts before we commit to either path?

Sorry, I realized I didn't actually read option B :) That is a better option than A; I'll await @jjhafer's thoughts.

And one other question for my own edification, as it relates to this; is there a particular reason we chose to pass down and cache the raw strings as the path key instead of using PathBuf?

It's not an issue either way, but given that we are keying against a string that we are requiring to be a path, I was curious why we didn't use PathBuf, even if just for semantic reasons.

Good question — it was deliberate for a couple of specific reasons:

The key is string-shaped at both ends. It arrives from the frontend as a JSON string and ends up as a SQLite connection string for sqlx. Where path semantics matter — .. checks, canonicalization, the allowlist match — the code does use PathBuf; the String only reappears after validation, as an opaque cache key.

Not all keys are paths. :memory: and file:…?mode=memory URIs share the same maps, and PathBuf is the wrong type for those.

After looking at this more, I think there's a simpler way to do this, which is a single register_database method that takes the db path and an optional Migrator as a parameter. That solves the possible duplication issue, and allows creating a DB without a migrator.

I've updated this MR with this change; let me know if this is acceptable, or if another solution would be preferred. Thanks!

Thanks @onehumandev. I like it; nice! Two small follow-ups:

Could both methods take impl AsRef<Path> (dropping the to_string_lossy().into_owned() boilerplate)?

Could Builder (&str) and SetupRegistrar (impl Into<String>) share the same signature?

I like it with the above follow ups that @velocitysystems mentioned.

After discussion with @velocitysystems , there is a larger simplification that can be made here. To avoid having a polluted MR, I'll be closing down this one and opening a new one with the changed approach, as the changes will be significantly different from what this MR currently has.
Thanks!

Problem: Previously, only databases that lived in `app_config_dir` could be opened with this plugin. The user passed in the path relative to `app_config_dir` to load the datbase. This meant that there was no support for databases that may live anywhere else within the app, which is a reasonable requirement. For example, some databases may need to live the `Library` folder on an iOS app so the database will be kept as part of the app backup. Solution: Databases can now only be opened by an exact, pre-registered absolute path (in-memory databases are the sole exception). Paths are registered via Builder::allow_path or, more commonly, in the new Builder::on_setup hook, which runs during plugin setup once `app` exists — so paths derived from app.path().app_data_dir() and friends can be computed and registered at startup. Registered paths are canonicalized once at setup so the equality check is symlink-safe. The resolver canonicalizes each requested path and requires an exact allowlist match. `..` segments and null bytes are always rejected. Migration registration flows through the same resolver, so a migration's key must be a registered absolute path. Why no relative paths: A relative-path default (app_config_dir) is an implicit grant — every file under that dir becomes reachable without the developer opting in. Removing it makes the set of reachable databases exactly the set the developer registered, with no ambient authority, and gives exactly one path to accessing a db. Why no registered root folders: Root allow-listing grants a whole subtree, including files created later that the developer never considered. Prefix matching is also easy to get subtly wrong against symlinks and traversal. Exact-path allow-listing is the least-privilege equivalent: explicit, auditable, and unambiguous. BREAKING CHANGE: relative paths are removed. load() now requires an absolute path registered via allow_path / on_setup. Previously-relative callers must register the absolute path and pass it from the frontend. It is the responsibility of the caller to ensure a consistent passing down of the needed path.

velocitysystems · 2026-06-11T15:46:45Z

-   /// Builder::new()
-   ///     .add_migrations("main.db", sqlx::migrate!("./doc-test-fixtures/migrations"))
-   ///     .build::<tauri::Wry>();
+   /// Builder::<tauri::Wry>::new()


Nit: Use Builder::<R>::new() and remove = Wry from comment.

velocitysystems · 2026-06-11T15:46:56Z

+   /// use tauri::Manager;
+   ///
+   /// # fn example() {
+   /// Builder::<tauri::Wry>::new()


Nit: Use Builder::::new() and remove = Wry from comment.

velocitysystems · 2026-06-11T15:48:20Z

-   Ok(joined)
+   // Path is allowed — create parent directories if needed, then return the canonical path
+   // so the opened file matches the value that passed validation.
+   canonicalize_database_path(candidate, true)


This re-canonicalizes candidate independently of the canonical value that passed the allowlist check on line 129, and the second result is never re-verified against the allowlist. If the path's final component changes between the two calls (file created, symlink swapped), the value we connect to can differ from the value we validated.

Should we either: (a) reuse canonical here (creating its parent directory directly), or (b) re-check the second canonicalization result against allowed_files before returning?

velocitysystems · 2026-06-11T15:48:56Z

+fn validate_and_resolve(path: &str, allowed_files: &[PathBuf]) -> Result<PathBuf, Error> {
   // Pass through in-memory database paths unchanged — they don't touch the filesystem.
   // Matches the same patterns as `is_memory_database` in sqlx-sqlite-conn-mgr.
   if is_memory_path(path) {


Nit: The in-memory classification runs before the null-byte and .. checks, so e.g. file::memory:\0… is passed through to sqlx unvalidated.

velocitysystems · 2026-06-11T15:50:21Z

    *
    * @example
    * ```ts
    * const db = await Database.load('mydb.db')


Nit: Example needs to be updated to use the new API.

velocitysystems · 2026-06-11T15:53:28Z

+   }

-   create_dir_all(&app_path)?;
+   canonicalize_database_path(Path::new(key), true).map(|p| p.to_string_lossy().into_owned())


Registration accepts relative paths (no is_absolute() check here), so a relative key gets allowlisted at a location that depends on the current working directory — should registration enforce the same path rules as load()?

jjhafer reviewed Jun 9, 2026

View reviewed changes

Comment thread src/resolve.rs Outdated

Comment thread src/resolve.rs Outdated

Comment thread src/resolve.rs Outdated

Comment thread src/lib.rs Outdated

onehumandev force-pushed the aldewaal/paths_support branch from 4af4219 to 096fe5f Compare June 10, 2026 13:41

velocitysystems suggested changes Jun 10, 2026

View reviewed changes

velocitysystems mentioned this pull request Jun 11, 2026

refactor: support closing all connections to a db in Rust #51

Open

onehumandev force-pushed the aldewaal/paths_support branch from 096fe5f to e0b8d48 Compare June 11, 2026 13:55

onehumandev force-pushed the aldewaal/paths_support branch from e0b8d48 to 0f92369 Compare June 11, 2026 14:39

onehumandev requested review from jjhafer and velocitysystems June 11, 2026 14:43

velocitysystems suggested changes Jun 11, 2026

View reviewed changes

onehumandev closed this Jun 11, 2026

velocitysystems mentioned this pull request Jun 15, 2026

feat: register databases by key and validate paths at setup #52

Open

Conversation

onehumandev commented Jun 9, 2026

Uh oh!

onehumandev commented Jun 9, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jjhafer commented Jun 9, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

velocitysystems Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

velocitysystems Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

velocitysystems Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

velocitysystems Jun 10, 2026 •

edited

Loading

velocitysystems Jun 11, 2026 •

edited

Loading

velocitysystems Jun 11, 2026 •

edited

Loading