From b57a5eef57f22ce0d9e9dabf8d76bbaf29774756 Mon Sep 17 00:00:00 2001 From: Severin Siffert Date: Fri, 11 Apr 2025 12:54:26 +0200 Subject: [PATCH 1/2] feat!: confirm short freezing threshold --- CHANGELOG.md | 4 ++++ docs/cli-reference/dfx-canister.mdx | 3 ++- e2e/tests-dfx/update_settings.bash | 10 ++++++++++ src/dfx/src/commands/canister/update_settings.rs | 11 +++++++++++ 4 files changed, 27 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 08bcba8ed0..7216ad9d07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ # UNRELEASED +### feat!: Add safeguard to very short freezing threshold + +Similar to very long freezing thresholds, setting a freezing threshold below 1 week now requires confirmation with `--confirm-very-short-freezing-threshold` so that unexpected canister deletion is less likely. + ### fix: clear state when switching from shared to project network dfx would try to reuse canister ids when switching from a shared network to a project network, diff --git a/docs/cli-reference/dfx-canister.mdx b/docs/cli-reference/dfx-canister.mdx index aca3203460..0ce987df17 100644 --- a/docs/cli-reference/dfx-canister.mdx +++ b/docs/cli-reference/dfx-canister.mdx @@ -1189,10 +1189,11 @@ You can specify the following options for the `dfx canister update-settings` com | `--add-log-viewer ` | Add a principal to the list of log viewers of the canister. Can be specified more than once to add multiple log viewers. If current log visibility is `public` or `controllers`, it will be changed to the custom allowed viewer list. | | `-c`, `--compute-allocation ` | Specifies the canister's compute allocation. This should be a percent in the range [0..100]. | | `--confirm-very-long-freezing-threshold` | Freezing thresholds above ~1.5 years require this option as confirmation. | +| `--confirm-very-short-freezing-threshold` | Freezing thresholds below 1 week require this option as confirmation. | | `--impersonate ` | Specifies a principal on behalf of which requests to a local PocketIC instance are sent. | | `--set-controller ` | Specifies the identity name or the principal of the new controller. Can be specified more than once, indicating the canister will have multiple controllers. If any controllers are set with this parameter, any other controllers will be removed. | | `--set-log-viewer ` | Specifies the the principal of the log viewer of the canister. Can be specified more than once, indicating the canister will have multiple log viewers. If any log viewers are set with this parameter, any other log viewers will be removed. If current log visibility is `public` or `controllers`, it will be changed to the custom allowed viewer list. | -| `--memory-allocation ` | Specifies how much memory the canister is allowed to use in total. This should be a value in the range [0..12 GiB]. A setting of 0 means the canister will have access to memory on a “best-effort” basis: It will only be charged for the memory it uses, but at any point in time may stop running if it tries to allocate more memory when there isn’t space available on the subnet. | +| `--memory-allocation ` | Specifies how much memory the canister is allowed to use in total. This should be a value in the range [0..12 GiB]. A setting of 0 means the canister will have access to memory on a "best-effort" basis: It will only be charged for the memory it uses, but at any point in time may stop running if it tries to allocate more memory when there isn't space available on the subnet. | | `--reserved-cycles-limit ` | Specifies the upper limit of the canister's reserved cycles. | | `--wasm-memory-limit ` | Specifies a soft upper limit for the canister's heap memory. | | `--log-visibility ` | Specifies who is allowed to read the canister's logs. Can be either "controllers" or "public". For custom allowed viewers, use `--set-log-viewer` or `--add-log-viewer`. | diff --git a/e2e/tests-dfx/update_settings.bash b/e2e/tests-dfx/update_settings.bash index c96b82e663..98f84ff909 100644 --- a/e2e/tests-dfx/update_settings.bash +++ b/e2e/tests-dfx/update_settings.bash @@ -40,6 +40,16 @@ teardown() { assert_command dfx ledger fabricate-cycles --canister hello_backend --t 100 assert_command dfx canister status hello_backend assert_match "Freezing threshold: 100_000_000_000" + + # trying to set threshold to 1 day, which should not work without confirmation + assert_command_fail dfx canister update-settings hello_backend --freezing-threshold 86400 + assert_match "The freezing threshold is very short at less than 1 week" # error message pointing to the error + + # with manual override it's ok + assert_command dfx canister update-settings hello_backend --freezing-threshold 86400 --confirm-very-short-freezing-threshold + + assert_command dfx canister status hello_backend + assert_match "Freezing threshold: 86_400" } @test "set wasm memory limit" { diff --git a/src/dfx/src/commands/canister/update_settings.rs b/src/dfx/src/commands/canister/update_settings.rs index 748086129c..bba209f3c9 100644 --- a/src/dfx/src/commands/canister/update_settings.rs +++ b/src/dfx/src/commands/canister/update_settings.rs @@ -111,6 +111,10 @@ pub struct UpdateSettingsOpts { #[arg(long)] confirm_very_long_freezing_threshold: bool, + /// Freezing thresholds below 1 week require this flag as confirmation. + #[arg(long)] + confirm_very_short_freezing_threshold: bool, + /// Skips yes/no checks by answering 'yes'. Such checks can result in loss of control, /// so this is not recommended outside of CI. #[arg(long, short)] @@ -141,6 +145,13 @@ pub async fn exec( "If you truly want to set a freezing threshold that is longer than a year, please run the same command, but with the flag --confirm-very-long-freezing-threshold to confirm you want to do this.", )).context("Misunderstanding is very likely."); } + if threshold_in_seconds < 604_800 /* 1 week */ && !opts.confirm_very_short_freezing_threshold + { + return Err(DiagnosedError::new( + "The freezing threshold is very short at less than 1 week. This may lead to canisters getting deleted with no or too little warning when cycles run out.", + "If you truly want to set a freezing threshold that is shorter than a week, please run the same command, but with the flag --confirm-very-short-freezing-threshold to confirm you want to do this.", + )).context("Dangerous operation requires confirmation."); + } } fetch_root_key_if_needed(env).await?; From 59f700701fe76ae3f0148e7de8f1c4e5844b0987 Mon Sep 17 00:00:00 2001 From: Severin Siffert Date: Fri, 11 Apr 2025 13:00:09 +0200 Subject: [PATCH 2/2] reword --- CHANGELOG.md | 2 +- src/dfx/src/commands/canister/update_settings.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7216ad9d07..f5d1cb1834 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### feat!: Add safeguard to very short freezing threshold -Similar to very long freezing thresholds, setting a freezing threshold below 1 week now requires confirmation with `--confirm-very-short-freezing-threshold` so that unexpected canister deletion is less likely. +Similar to very long freezing thresholds, setting a freezing threshold below 1 week now requires confirmation with `--confirm-very-short-freezing-threshold` so that unexpected canister uninstallation is less likely. ### fix: clear state when switching from shared to project network diff --git a/src/dfx/src/commands/canister/update_settings.rs b/src/dfx/src/commands/canister/update_settings.rs index bba209f3c9..2162e71481 100644 --- a/src/dfx/src/commands/canister/update_settings.rs +++ b/src/dfx/src/commands/canister/update_settings.rs @@ -148,7 +148,7 @@ pub async fn exec( if threshold_in_seconds < 604_800 /* 1 week */ && !opts.confirm_very_short_freezing_threshold { return Err(DiagnosedError::new( - "The freezing threshold is very short at less than 1 week. This may lead to canisters getting deleted with no or too little warning when cycles run out.", + "The freezing threshold is very short at less than 1 week. This may lead to canisters getting uninstalled with no or too little warning when cycles run out.", "If you truly want to set a freezing threshold that is shorter than a week, please run the same command, but with the flag --confirm-very-short-freezing-threshold to confirm you want to do this.", )).context("Dangerous operation requires confirmation."); }