feat: add safety controls for controller modification commands

CHANGELOG.md

@@ -1,5 +1,8 @@
 # Unreleased
 
+* feat: Add safety controls for `--set-controller` and `--remove-controller`
+  * Warn and prompt for confirmation when removing yourself from controllers
+  * Add `-f/--force` flag to skip confirmation prompts
 * feat: Show `name` in `canister status` command
 * feat: `icp canister metadata <canister> <metadata section>` now fetches metadata sections from specified canisters
 * fix: Validate explicit canister paths and throw an error if `canister.yaml` is not found

crates/icp-cli/src/commands/canister/settings/update.rs

@@ -2,6 +2,8 @@ use anyhow::bail;
 use byte_unit::{Byte, Unit};
 use clap::{ArgAction, Args};
 use console::Term;
+use dialoguer::Confirm;
+use ic_agent::Identity;
 use ic_agent::export::Principal;
 use ic_management_canister_types::{CanisterStatusResult, EnvironmentVariable, LogVisibility};
 use icp::ProjectLoadError;
@@ -13,12 +15,20 @@ use crate::commands::args;
 
 #[derive(Clone, Debug, Default, Args)]
 pub(crate) struct ControllerOpt {
+    /// Add one or more principals to the canister's controller list.
     #[arg(long, action = ArgAction::Append, conflicts_with("set_controller"))]
     add_controller: Option<Vec<Principal>>,
 
+    /// Remove one or more principals from the canister's controller list.
+    ///
+    /// Warning: Removing yourself will cause you to lose control of the canister.
     #[arg(long, action = ArgAction::Append, conflicts_with("set_controller"))]
     remove_controller: Option<Vec<Principal>>,
 
+    /// Replace the canister's controller list with the specified principals.
+    ///
+    /// Warning: This removes all existing controllers not in the new list.
+    /// If you don't include yourself, you will lose control of the canister.
     #[arg(long, action = ArgAction::Append)]
     set_controller: Option<Vec<Principal>>,
 }
@@ -76,6 +86,10 @@ pub(crate) struct UpdateArgs {
     #[command(flatten)]
     pub(crate) cmd_args: args::CanisterCommandArgs,
 
+    /// Force the operation without confirmation prompts
+    #[arg(short = 'f', long)]
+    force: bool,
+
     #[command(flatten)]
     controllers: Option<ControllerOpt>,
 
@@ -106,6 +120,11 @@ pub(crate) struct UpdateArgs {
 
 pub(crate) async fn exec(ctx: &Context, args: &UpdateArgs) -> Result<(), anyhow::Error> {
     let selections = args.cmd_args.selections();
+    let identity = ctx.get_identity(&selections.identity).await?;
+    let caller_principal = identity
+        .sender()
+        .map_err(|e| anyhow::anyhow!("failed to get caller principal: {e}"))?;
+
     let agent = ctx
         .get_agent(
             &selections.identity,
@@ -139,14 +158,34 @@ pub(crate) async fn exec(ctx: &Context, args: &UpdateArgs) -> Result<(), anyhow:
         current_status = Some(mgmt.canister_status(&cid).await?.0);
     }
 
-    // TODO(VZ): Ask for consent
-    // - if the freezing threshold is too long or too short.
-    // - if trying to remove the caller itself from the controllers.
+    // TODO(VZ): Ask for consent if the freezing threshold is too long or too short.
 
     // Handle controllers.
     let mut controllers: Option<Vec<Principal>> = None;
     if let Some(controllers_opt) = &args.controllers {
         controllers = get_controllers(controllers_opt, current_status.as_ref());
+
+        // Check if the caller is being removed from controllers
+        if let Some(new_controllers) = &controllers
+            && !new_controllers.contains(&caller_principal)
+            && !args.force
+        {
+            ctx.term.write_line(
+                "Warning: You are about to remove yourself from the controllers list.",
+            )?;
+            ctx.term.write_line(
+                "This will cause you to lose control of the canister and cannot be undone.",
+            )?;
+
+            let confirmed = Confirm::new()
+                .with_prompt("Do you want to proceed?")
+                .default(false)
+                .interact()?;
+
+            if !confirmed {
+                bail!("Operation cancelled by user");
+            }
+        }
     }
 
     // Handle log visibility.

crates/icp-cli/tests/canister_settings_tests.rs

@@ -271,7 +271,7 @@ async fn canister_settings_update_controllers() {
                 .and(contains(principal_bob.as_str()).not()),
         );
 
-    // Set multiple controllers
+    // Set multiple controllers (uses --force since we're removing ourselves as controller)
     ctx.icp()
         .current_dir(&project_dir)
         .args([
@@ -281,6 +281,7 @@ async fn canister_settings_update_controllers() {
             "my-canister",
             "--environment",
             "random-environment",
+            "--force",
             "--set-controller",
             principal_alice.as_str(),
             "--set-controller",

docs/reference/cli.md

@@ -290,9 +290,14 @@ Change a canister's settings to specified values
 * `-n`, `--network <NETWORK>` — Name of the network to target, conflicts with environment argument
 * `-e`, `--environment <ENVIRONMENT>` — Override the environment to connect to. By default, the local environment is used
 * `--identity <IDENTITY>` — The user identity to run this command as
-* `--add-controller <ADD_CONTROLLER>`
-* `--remove-controller <REMOVE_CONTROLLER>`
-* `--set-controller <SET_CONTROLLER>`
+* `-f`, `--force` — Force the operation without confirmation prompts
+* `--add-controller <ADD_CONTROLLER>` — Add one or more principals to the canister's controller list
+* `--remove-controller <REMOVE_CONTROLLER>` — Remove one or more principals from the canister's controller list.
+
+   Warning: Removing yourself will cause you to lose control of the canister.
+* `--set-controller <SET_CONTROLLER>` — Replace the canister's controller list with the specified principals.
+
+   Warning: This removes all existing controllers not in the new list. If you don't include yourself, you will lose control of the canister.
 * `--compute-allocation <COMPUTE_ALLOCATION>`
 * `--memory-allocation <MEMORY_ALLOCATION>`
 * `--freezing-threshold <FREEZING_THRESHOLD>`