fix(tauri): enforce ACL for remote origins even without AppManifest#15266
Conversation
Package Changes Through 7a2672fThere are 11 changes which include tauri with minor, @tauri-apps/api with minor, tauri-build with minor, tauri-macos-sign with patch, tauri-bundler with minor, @tauri-apps/cli with minor, tauri-cli with minor, tauri-runtime with minor, tauri-runtime-wry with minor, tauri-utils with minor, tauri-plugin with minor Planned Package VersionsThe following package releases are the planned based on the context of changes in this pull request.
Add another change file through the GitHub UI by following this link. Read about change files or the docs at github.com/jbolda/covector |
tweidinger
left a comment
There was a problem hiding this comment.
Just reviewed the code without testing. Breaking should be fine since unintentional and security related. LGTM and thanks!
Maybe next time via the disclosure feature though 😅
|
Please fix tests/fmt/clippy before merge @mtsgrd or @FabianLars . I'm on mobile atm. |
6c38293 to
2476cae
Compare
Custom (non-plugin) commands registered via `invoke_handler` currently bypass ACL checks entirely when no `AppManifest` is configured in `build.rs`. This means if a webview navigates to a remote URL, that remote content can invoke any custom IPC command without restriction. Add `|| !is_local` to the existing ACL enforcement condition so that requests from non-local origins always go through `resolve_access()`. Behavior changes: - Local origin + no AppManifest: unchanged (permissive, commands work) - Remote origin + no AppManifest: now enforced, rejected (no ACL entries) - Remote origin + explicit `remote` capability: allowed (resolve_access matches the configured capability) This is a **breaking change** for apps that rely on remote origins being able to call custom commands without configuring an AppManifest. Such apps will need to add an AppManifest with explicit `remote` capabilities to restore the previous behavior. Normalize test URLs to tauri scheme and tidy formatting Update test expectations to use the tauri://localhost URL scheme instead of http://tauri.localhost to reflect the app's custom URL scheme handling. Also reformat some test code for readability by collapsing and simplifying builder expressions and replace a match-based error check with an if-let to make the intent clearer and reduce indentation.
2476cae to
7a2672f
Compare
|
@tweidinger thanks for reviewing! Let me know if there's anything else I can do?
Indeed, that would have been better. |
|
@tweidinger how can we move this pr forward? |
|
Hi, The new unit test hard-codes Severity: action required | Category: correctness How to fix: Use platform-specific local URL Agent prompt to fix - you can give this to your LLM of choice:
We noticed a couple of other issues in this PR as well - happy to share if helpful. Found by Qodo code review |
|
Hey, I am traveling atm. Poking @FabianLars for getting this merged in dev. |
The `forge_provider` backend command was missing from the Tauri ACL permissions list (fixed in an earlier commit), which meant the query silently failed for all desktop users since 0.19.4. Regular github.com users were unaffected because the frontend had redundant domain-matching logic that masked the failure. GitHub Enterprise Server users (e.g. github.ourinternaldomain.net) had no such fallback and lost their forge integration (PR button, reviews, etc.). ACLs are going away soon (tauri-apps/tauri#15266), so rather than papering over backend failures with frontend fallbacks, this commit: - Removes the redundant frontend domain-matching in forgeFactory that was masking the broken backend query - Simplifies forge type resolution to: backend detection → manual override → default - Adds Rust tests confirming GHES subdomain detection works for both SSH and HTTPS URL forms
The `forge_provider` backend command was missing from the Tauri ACL permissions list (fixed in an earlier commit), which meant the query silently failed for all desktop users since 0.19.4. Regular github.com users were unaffected because the frontend had redundant domain-matching logic that masked the failure. GitHub Enterprise Server users (e.g. github.ourinternaldomain.net) had no such fallback and lost their forge integration (PR button, reviews, etc.). ACLs are going away soon (tauri-apps/tauri#15266), so rather than papering over backend failures with frontend fallbacks, this commit: - Removes the redundant frontend domain-matching in forgeFactory that was masking the broken backend query - Simplifies forge type resolution to: backend detection → manual override → default - Adds Rust tests confirming GHES subdomain detection works for both SSH and HTTPS URL forms
The `forge_provider` backend command was missing from the Tauri ACL permissions list (fixed in an earlier commit), which meant the query silently failed for all desktop users since 0.19.4. Regular github.com users were unaffected because the frontend had redundant domain-matching logic that masked the failure. GitHub Enterprise Server users (e.g. github.ourinternaldomain.net) had no such fallback and lost their forge integration (PR button, reviews, etc.). ACLs are going away soon (tauri-apps/tauri#15266), so rather than papering over backend failures with frontend fallbacks, this commit: - Removes the redundant frontend domain-matching in forgeFactory that was masking the broken backend query - Simplifies forge type resolution to: manual override → backend detection → default - Adds Rust tests confirming GHES subdomain detection works for both SSH and HTTPS URL forms
Root cause of v0.3.22 release bug ('Command install_vigem_driver not
allowed by ACL'): src-tauri/Cargo.lock was gitignored, so CI pulled
tauri 2.11.x while local builds had older tauri without strict ACL
enforcement. Tauri 2.11 (PR tauri-apps/tauri#15266) enforces capability
ACL for IPC calls from any remote origin even without an explicit
AppManifest; previously custom (non-plugin) commands bypassed ACL
entirely. Sunshine GUI loads its real UI from https://localhost:47990
which tauri classifies as remote (the URL is not relative to the
packaged frontendDist), so every custom command got rejected.
This commit ships both the dep-hygiene fix (so CI is reproducible) and
the proper ACL adaptation. Supersedes the previous rust-toolchain.toml
pin workaround (v0.3.23) which was based on a misdiagnosis (rustc
version drift) - the toolchain file is removed here.
Changes:
* Track src-tauri/Cargo.lock (binary crate). Adds .gitignore comment
documenting why. cargo update bumped tauri 2.10.3 -> 2.11.1.
* Remove src-tauri/rust-toolchain.toml (obsolete v0.3.23 workaround).
* build.rs now parses src/main.rs to extract every command listed in
the tauri::generate_handler block at compile time, then calls
AppManifest::commands so tauri-build auto-generates allow-/deny-
permission TOMLs under permissions/autogenerated/ (gitignored). It
also writes capabilities/app-commands.generated.json (gitignored)
binding every allow-* permission to sunshine backend remote URLs
(localhost and 127.0.0.1, both schemes). Tauri auto-loads
./capabilities/**/* alongside the inline default capability in
tauri.conf.json, which keeps managing windows / plugin perms /
external whitelist URLs by hand.
* Rename windows::_webview_heartbeat -> webview_heartbeat to avoid the
ugly auto-generated allow--webview-heartbeat double-dash slug.
Adding a new tauri command now requires only a line in main.rs;
build.rs regenerates ACL on next compile - single source of truth.
tauri 2.11.1 includes tauri-apps/tauri#15266, which enforces ACL for non-local origins even when no AppManifest is configured. The hand-rolled `allow-custom-commands` permission existed solely to flip `has_app_acl_manifest` so custom command invocations from local origins would not bypass the ACL gate (and, by side effect, also be checked for remote origins). With the upstream fix in place, remote origins are checked regardless, and local origins remain permissive without an AppManifest. The capability declares `local: true`, so the workaround is no longer load-bearing. Drop `permissions/default.toml`, the `allow-custom-commands` entry from `capabilities/main.json`, and the explicit `try_build(... AppManifest ...)` plumbing in `build.rs` -- back to plain `tauri_build::build()`.
Summary
Custom (non-plugin) commands registered via
invoke_handlerbypass ACL checks entirely when noAppManifestis configured inbuild.rs. This means if a webview navigates away from the bundled frontend to a remote URL, that remote page can invoke any custom IPC command without restriction.This PR adds
|| !is_localto the existing ACL enforcement condition inWebview::on_message(), so that requests from non-local origins always go throughresolve_access():Behavior after this change
remotecapabilityresolve_accessmatches, allowedBreaking change
Apps that intentionally allow remote origins to call custom commands without configuring an
AppManifestwill need to add anAppManifestwith explicitremotecapabilities to restore the previous behavior. This is expected to be uncommon, since most apps either use local-only webviews or already configure anAppManifestwhen working with remote content.Test plan
remote_origin_blocked_for_custom_commands_without_app_manifestunit test that verifies:https://some-hostile-domain.comis rejectedtauri://localhostis not blocked by the remote-origin guardtaurilib tests continue to pass