Skip to content

feat: require explicit registration of absolute database paths#50

Open
onehumandev wants to merge 1 commit into
silvermine:masterfrom
onehumandev:aldewaal/paths_support
Open

feat: require explicit registration of absolute database paths#50
onehumandev wants to merge 1 commit into
silvermine:masterfrom
onehumandev:aldewaal/paths_support

Conversation

@onehumandev

Copy link
Copy Markdown

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

Copy link
Copy Markdown
Author

@velocitysystems For your review :) Thanks!

Comment thread src/resolve.rs Outdated
Comment thread src/resolve.rs Outdated
Comment thread src/resolve.rs Outdated
Comment thread src/lib.rs Outdated
@jjhafer

jjhafer commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

@onehumandev Added some comments.

@onehumandev onehumandev force-pushed the aldewaal/paths_support branch from 4af4219 to 096fe5f Compare June 10, 2026 13:41
Comment thread src/resolve.rs
Comment thread src/resolve.rs
Comment thread src/resolve.rs Outdated
Comment thread README.md Outdated
Comment thread src/lib.rs Outdated
Comment thread README.md
tauri::Builder::default()
.plugin(
Builder::new()
.on_setup(|app, reg| {

@velocitysystems velocitysystems Jun 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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:

  1. 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.

  2. 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 Aadd_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?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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.

@velocitysystems velocitysystems Jun 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

  1. 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.
  2. Not all keys are paths. :memory: and file:…?mode=memory URIs share the same maps, and PathBuf is the wrong type for those.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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!

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants