refactor(permissions): collapse to one authorized set, drop install args#73
Merged
Conversation
Replace the three-level permission model (Commit/Prepare/ManagePermissions) with two rules: - Canister controllers can do anything. - A single set of "authorized" principals may sync assets; only controllers can change it. The set holds only the extra, non-controller principals — controllers are always allowed without being stored. Canister: - Drop the Permission enum and grant_permission/revoke_permission/ list_permitted/take_ownership; keep authorize/deauthorize/list_authorized. - Collapse the can_commit/can_prepare guards into one guard_can_sync; set management is guarded by guard_is_controller. - Add a can_sync query so a caller can check its own access (true if authorized or a controller) up front. - Simplify init/upgrade args to `authorized: vec principal` (empty = none; no args means empty on init, untouched on upgrade). Drop the legacy StableStatePermissions blob; stable state stores one authorized set. Sync plugin: - ensure_can_sync runs before any scan/diff and uses can_sync, so an unauthorized run fails fast. In proxy mode it grants via the proxy; in direct mode it errors. Controllers are no longer falsely blocked. Verified: canister-core + sync-core unit tests, candid compatibility, and the e2e suite all pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… endpoints
Remove AssetCanisterArgs/InitArgs/UpgradeArgs entirely. The canister now
installs with no args; the authorized set is managed exclusively through the
controller-guarded authorize/deauthorize endpoints.
- init() clears state; post_upgrade(stable_state) restores it untouched
- drop the Init-vs-Upgrade trap branches
- remove State::set_authorized (only existed to apply the args) and its test
- service signature becomes `service: () -> {`
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Refactors the assets canister permission model into a single controller-managed “authorized” set (with controllers always implicitly allowed) and removes install/upgrade-time args from the canister interface. Sync tooling is updated to preflight authorization via a new can_sync query and to authorize via proxy when available.
Changes:
- Replace multi-level permissions with a single authorized set + controller-only management; add
can_syncquery for preflight checks. - Update sync-core to fail fast using
can_sync, and toauthorizevia proxy mode when needed. - Remove install/upgrade args from the canister’s candid and Rust entrypoints; simplify stable state to store only the authorized set.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/sync-core/tests/bench_sync.rs | Updates bench mock to support the new can_sync preflight call. |
| crates/sync-core/src/sync.rs | Adds ensure_can_sync fast-fail authorization preflight + proxy authorization path; updates tests accordingly. |
| crates/sync-core/src/canister.rs | Removes legacy permission call wrappers; adds can_sync and authorize_via_proxy wrappers. |
| crates/canister/src/lib.rs | Drops init/upgrade args, replaces guards with guard_can_sync/guard_is_controller, and exposes can_sync query. |
| crates/canister/assets.did | Removes legacy permission and install-arg types; updates service signature to service: () -> { ... } and adds can_sync. |
| crates/canister-core/src/types.rs | Removes legacy permission/install-arg wire types from the core types module. |
| crates/canister-core/src/tests.rs | Updates stable-state construction and adds tests for authorize/deauthorize + stable roundtrip of the authorized set. |
| crates/canister-core/src/state.rs | Replaces per-permission principal sets with a single authorized set and updates stable-state restoration accordingly. |
| crates/canister-core/src/stable.rs | Simplifies stable state to persist only the authorized set (drops legacy permissions blob). |
| crates/canister-core/src/lib.rs | Implements new authorization API (authorize/deauthorize/can_sync) and new guards; removes install arg handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Simplifies the canister's permission model down to two rules and removes the install-time arguments that configured it.
Permission model (
14d6ac2)Replaces the three-level model (Commit/Prepare/ManagePermissions) with:
Canister changes:
Permissionenum andgrant_permission/revoke_permission/list_permitted/take_ownership; keepauthorize/deauthorize/list_authorized.can_commit/can_prepareguards into oneguard_can_sync; set management is guarded byguard_is_controller.can_syncquery so a caller can check its own access up front.StableStatePermissionsblob; stable state stores one authorized set.Sync plugin:
ensure_can_syncruns before any scan/diff and usescan_sync, so an unauthorized run fails fast. Proxy mode grants via the proxy; direct mode errors. Controllers are no longer falsely blocked.Drop install args (
3cf7f7c)Removes
AssetCanisterArgs/InitArgs/UpgradeArgsentirely. The canister now installs with no args; the authorized set is managed exclusively through the controller-guardedauthorize/deauthorizeendpoints.init()clears state;post_upgrade(stable_state)restores it untouched.State::set_authorized(which only existed to apply the args).service: () -> {.Testing
cargo test -p canister-core— passescargo test -p sync-core— passescandid_interface_compatibility— exported interface matchesassets.didcargo test -p e2e— full suite passes (live deploy with no install args)cargo clippy— clean🤖 Generated with Claude Code