Remove explicit ACL permissions file, fix misconfigured entries#13655
Remove explicit ACL permissions file, fix misconfigured entries#13655mtsgrd wants to merge 2 commits intobump-tauri-patch-versionsfrom
Conversation
This includes a security fix that enforces ACL checks for IPC requests from remote origins even when no AppManifest is configured. Previously, custom commands bypassed ACL entirely without an AppManifest. This means we can remove our explicit default.toml ACL permissions which have been a recurring source of bugs when overlooked (see #13076, #13384, and #13653).
Detected missing `commit_squash` ACL entry and stale entries for removed commands (`forge_branch_chat`, `forge_provider`). With the tauri bump in the parent commit, custom commands no longer require an explicit permissions manifest for local origins. This lets us delete `permissions/default.toml` entirely, eliminating a recurring source of bugs where new commands silently fail because they were not added to the ACL list (see #13076, #13384, #13653).
There was a problem hiding this comment.
Pull request overview
Removes the Tauri custom-command ACL permission manifest and its capability reference, relying on the newer Tauri behavior where local origins no longer need an explicit permissions list for custom commands. This reduces maintenance overhead and prevents regressions where new commands silently fail due to missing ACL entries.
Changes:
- Deleted
permissions/default.toml(the “allow-custom-commands” aggregated ACL list). - Removed
"allow-custom-commands"from the main capability’s permissions list. - Simplified the Tauri build script to use
tauri_build::build().
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
crates/gitbutler-tauri/permissions/default.toml |
Removed the explicit aggregated ACL permissions manifest for custom commands. |
crates/gitbutler-tauri/capabilities/main.json |
Dropped the now-obsolete allow-custom-commands permission reference from the main capability. |
crates/gitbutler-tauri/build.rs |
Simplified build script by removing explicit manifest/ACL wiring and calling tauri_build::build(). |
e5fa2a7 to
6e0e0e9
Compare
slarse
left a comment
There was a problem hiding this comment.
It's great if we can get rid of the ACL.
With Tauri 2.11.0, custom commands are allowed for local origins by default, so we no longer need the explicit manifest.
This comment has me a bit concerned that this is missing the point. The problem the ACL solved wasn't to allow local origins, it was to deny non-local ones. Counter-intuitive as that may seem. Can you validate that, with this patch, we still deny non-local origins from accessing the IPC bridge?
To be able to validate, you need to first remove the on_navigate() navigation block in crates/gitbutler-tauri/src/window.rs. After that you can navigate to a different origin by just setting location.href = "whatever" in the console, and execute an IPC command with e.g. __TAURI_INTERNALS__.invoke("list_projects");. It should be denied.
6e0e0e9 to
47ef7af
Compare
Summary
Removes the hand-maintained
permissions/default.tomlACL file and its references, now that the Tauri bump in #13654 makes it unnecessary.Why
Every new
#[tauri::command]had to be manually added topermissions/default.toml. Forgetting an entry caused the command to silently fail at runtime with no compile-time error. This has bitten us multiple times:forge_providerpermissionWith Tauri 2.11.0, custom commands are allowed for local origins by default, so we no longer need the explicit manifest.
Changes
crates/gitbutler-tauri/permissions/default.toml"allow-custom-commands"fromcapabilities/main.jsonbuild.rsto usetauri_build::build()instead ofAppManifestBugs found & fixed
commit_squashwas registered ingenerate_handler!but absent from the ACLforge_branch_chatandforge_providerwere in the ACL but their commands had been removedThis is part 1 of 1 in a stack made with GitButler: