diff --git a/.gitignore b/.gitignore index 028c4292..c0aacd91 100644 --- a/.gitignore +++ b/.gitignore @@ -22,3 +22,6 @@ docs-site/.astro/ docs-site/dist/ docs-site/dist-test/ docs-site/node_modules/ + +# Python bytecode from the test assets +__pycache__/ diff --git a/CHANGELOG.md b/CHANGELOG.md index c600bf18..d804de51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,11 +9,14 @@ air-gapped signing # Unreleased * feat: a canister can now declare `upgrade_args` alongside `init_args`, in its own manifest and as a per-canister environment override. It is passed when `icp deploy` upgrades the canister, where `init_args` is passed when it installs or reinstalls it. It takes exactly the forms `init_args` does (inline Candid string, or `{ value | path, format }`), and paths resolve against the canister's own directory the same way. A canister that declares no `upgrade_args` is upgraded with its `init_args`, as before, and `--args` / `--args-file` still override whichever applies. +* feat: a new `status_visibility` canister setting controls who may read a canister's status (its running state, cycles, memory usage, and settings) through the management canister. It takes the same forms as `log_visibility` — `controllers` (the default), `public`, or `{ allowed_viewers: [...] }` — and can be set in a manifest's `settings:` block or with `icp canister settings update --status-visibility / --add-status-viewer / --remove-status-viewer / --set-status-viewer`. `icp canister status` and `icp canister settings show` now report it. See the [canister settings reference](docs/reference/canister-settings.md#status_visibility). + * `icp canister status` and `icp canister settings show` now list principals one per line under their label rather than comma-separated on the label's line: `log viewer:` / `status viewer: ` for the allowed viewers of a visibility setting, `controller: ` for a canister's controllers. This applies to `log_visibility` and to the controller list as well, so scripts matching those lines need updating. Both lists are sorted, which the controller list previously was not. + * `icp canister settings update` no longer lets a relative viewer edit silently revoke public access. `--add-log-viewer` / `--remove-log-viewer` and their `--*-status-viewer` counterparts are now rejected while that setting is `public`, which carries no allowed-viewers list for them to be relative to — previously they started one from empty, so adding a viewer to a public canister took access away from everyone else. The error points at `--set-*-viewer`, which states the new list outright, and at `--*-visibility controllers`, which revokes public access on its own. Both remain allowed and now warn about what they cost: replacing a `public` policy with a list, and removing the last viewer. This is breaking for the pre-existing log flags. + * This raises the minimum replica version: reading a canister's status now requires one that reports `status_visibility`, and against an older replica `canister_status` fails to decode — which affects `icp deploy`, `icp canister status`, and `icp canister settings show`/`sync`, not just the new setting. Every mainnet subnet reports it. A `managed` network resolves the launcher to `latest` unless it pins `version:`, so only a pinned launcher older than `15.0.0-2026-08-13-03-55` is affected; raise the pin to that version or later. * feat: `script` build steps now receive `ICP_CLI_ENVIRONMENT`, the name of the environment the canisters are being built for, so a build can vary by environment the way a sync step already could. * feat: `icp completions ` prints a shell completion script for `bash`, `zsh`, `fish`, `powershell`, or `elvish` to stdout. See the [installation guide](docs/guides/installation.md#shell-completions) for where to put it. * fix: canister settings from the manifest are no longer silently discarded when a canister is created through the legacy management-canister fallback (a CloudEngine subnet with no registered engine operator). That path went through `ic-utils`, which encodes `create_canister`'s argument as a bare `canister_settings` record rather than the `record { settings : opt canister_settings; ... }` the interface spec defines, so the replica read no settings at all and created the canister with defaults. `icp deploy` masked this by syncing settings afterwards; `icp canister create` does not, and left the canister unconfigured. * fix: `icp canister logs` output formats are corrected. `--json` now emits machine-readable JSON and the default emits the human-readable lines (the two were swapped), and `--follow --json` emits newline-delimited JSON, one record per line, streamed as each record arrives. This is breaking for scripts: parsing the default output as JSON now requires `--json`, and consumers of `--follow --json` must read one JSON object per line. -* fix: `icp canister status` again falls back on the publicly readable state-tree information when the caller may not read the status. Replicas now reject those calls with `IC0542`, which the fallback did not recognise, so the command failed with `Error looking up canister ` instead of printing the controllers and module hash. `IC0541`, returned on subnets with administrators, is now recognised too, and the fallback no longer depends on whether the rejection arrives certified or uncertified. ## Experimental diff --git a/Cargo.lock b/Cargo.lock index 9d5c30dd..f208b369 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -120,7 +120,7 @@ version = "1.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "40c48f72fd53cd289104fc64099abca73db4166ad86ea0b4341abe65af83dadc" dependencies = [ - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -131,7 +131,7 @@ checksum = "291e6a250ff86cd4a820112fb8898808a366d8f9f58ce16d1f538353ad55747d" dependencies = [ "anstyle", "once_cell_polyfill", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -170,6 +170,12 @@ dependencies = [ "rustversion", ] +[[package]] +name = "array-init" +version = "2.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3d62b7694a562cdf5a74227903507c56ab2cc8bdd1f781ed5cb4cf9c9f810bfc" + [[package]] name = "arrayvec" version = "0.5.2" @@ -626,26 +632,26 @@ dependencies = [ ] [[package]] -name = "binread" -version = "2.2.0" +name = "binrw" +version = "0.15.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "16598dfc8e6578e9b597d9910ba2e73618385dc9f4b1d43dd92c349d6be6418f" +checksum = "6ad120d555272286c1017d25165ab8bd74806f13fc85b258484ec7e4ce75458f" dependencies = [ - "binread_derive", - "lazy_static", - "rustversion", + "array-init", + "binrw_derive", + "bytemuck", ] [[package]] -name = "binread_derive" -version = "2.1.0" +name = "binrw_derive" +version = "0.15.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1d9672209df1714ee804b1f4d4f68c8eb2a90b1f7a07acf472f88ce198ef1fed" +checksum = "6df92e0e9baae4dc82c7bad7715ca40c0a5c71539057bf2ea04a5c29c980410b" dependencies = [ "either", "proc-macro2", "quote", - "syn 1.0.109", + "syn 2.0.118", ] [[package]] @@ -980,12 +986,12 @@ dependencies = [ [[package]] name = "candid" -version = "0.10.29" +version = "0.10.35" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f8f781afa4a1303e3eab4ada0720a874942bcfa936ce01b816ac6378945c43a9" +checksum = "5cdba86ff862a2cc6b244f273a8713ffdf82b84254470e90bd029ee87c283f25" dependencies = [ "anyhow", - "binread", + "binrw", "byteorder", "candid_derive", "hex", @@ -1003,9 +1009,9 @@ dependencies = [ [[package]] name = "candid_derive" -version = "0.10.29" +version = "0.10.35" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ad6ae8e7944dd0035651bc0e7b3a3e4cb16f5fc43f8ae4fd76b36ff2cd52759f" +checksum = "13501edc1f9c9f057d5d84171a13ee1fe4255705f9027a3fac64b5949cbdd170" dependencies = [ "lazy_static", "proc-macro2", @@ -1047,7 +1053,7 @@ dependencies = [ "cap-primitives", "cap-std", "io-lifetimes 3.0.1", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -1064,7 +1070,7 @@ dependencies = [ "maybe-owned", "rustix", "rustix-linux-procfs", - "windows-sys 0.60.2", + "windows-sys 0.61.2", "winx", ] @@ -2024,7 +2030,7 @@ dependencies = [ "libc", "option-ext", "redox_users 0.5.2", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -2274,7 +2280,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -3504,6 +3510,17 @@ dependencies = [ "serde_bytes", ] +[[package]] +name = "ic-management-canister-types" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c4cdeb533e0ed67b4fd6b7f2c396e6b84cfe0f875e38911bed49a1f527c65fb3" +dependencies = [ + "candid", + "serde", + "serde_bytes", +] + [[package]] name = "ic-stable-structures" version = "0.6.9" @@ -3635,7 +3652,7 @@ dependencies = [ "ic-ed25519", "ic-identity-hsm", "ic-ledger-types", - "ic-management-canister-types 0.8.0", + "ic-management-canister-types 0.9.0", "ic-utils", "icp-canister-interfaces", "icp-events", @@ -3690,7 +3707,7 @@ version = "1.3.0" dependencies = [ "bigdecimal", "candid", - "ic-management-canister-types 0.8.0", + "ic-management-canister-types 0.9.0", "serde", ] @@ -3727,7 +3744,7 @@ dependencies = [ "ic-agent", "ic-ed25519", "ic-ledger-types", - "ic-management-canister-types 0.8.0", + "ic-management-canister-types 0.9.0", "ic-utils", "icp", "icp-canister-interfaces", @@ -4880,7 +4897,7 @@ version = "0.50.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" dependencies = [ - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -6214,7 +6231,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -6281,7 +6298,7 @@ dependencies = [ "security-framework 3.7.0", "security-framework-sys", "webpki-root-certs", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -6934,7 +6951,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "52d1cfed4120b4d927bf7c0f86d2087a4a7d6027c906d9f9d525a80573b9be51" dependencies = [ "libc", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -6963,7 +6980,7 @@ dependencies = [ "cfg-if", "libc", "psm", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -7146,7 +7163,7 @@ dependencies = [ "getrandom 0.4.3", "once_cell", "rustix", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -7186,7 +7203,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "230a1b821ccbd75b185820a1f1ff7b14d21da1e442e22c0863ea5f08771a8874" dependencies = [ "rustix", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -7643,7 +7660,7 @@ checksum = "f2f6fb2847f6742cd76af783a2a2c49e9375d0a111c7bef6f71cd9e738c72d6e" dependencies = [ "memoffset", "tempfile", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -8374,7 +8391,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -8772,7 +8789,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7d6f32a0ff4a9f6f01231eb2059cc85479330739333e0e58cadf03b6af2cca10" dependencies = [ "cfg-if", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 01934100..5ca927f5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -56,7 +56,7 @@ httptest = "0.16.3" ic-agent = { version = "0.49.1" } ic-ed25519 = "0.6.0" ic-ledger-types = "0.16.0" -ic-management-canister-types = { version = "0.8.0" } +ic-management-canister-types = { version = "0.9.0" } ic-utils = { version = "0.49.1" } icp = { path = "crates/icp" } icp-canister-interfaces = { path = "crates/icp-canister-interfaces" } diff --git a/crates/icp-cli/src/commands/canister/create.rs b/crates/icp-cli/src/commands/canister/create.rs index 6155ce82..38022554 100644 --- a/crates/icp-cli/src/commands/canister/create.rs +++ b/crates/icp-cli/src/commands/canister/create.rs @@ -163,7 +163,12 @@ impl CreateArgs { .or(default.settings.reserved_cycles_limit.clone()) .map(|c| Nat::from(c.get())), // TODO This should be configurable from the CLI - log_visibility: default.settings.log_visibility.clone().map(Into::into), + log_visibility: default.settings.log_visibility.clone().map(|v| v.0.into()), + status_visibility: default + .settings + .status_visibility + .clone() + .map(|v| v.0.into()), memory_allocation: self .settings .memory_allocation @@ -254,6 +259,7 @@ impl CreateArgs { .map(|c| Nat::from(c.get())), // TODO This should be configurable from the CLI log_visibility: None, + status_visibility: None, memory_allocation: self .settings .memory_allocation diff --git a/crates/icp-cli/src/commands/canister/mod.rs b/crates/icp-cli/src/commands/canister/mod.rs index 191d7d5c..4863143b 100644 --- a/crates/icp-cli/src/commands/canister/mod.rs +++ b/crates/icp-cli/src/commands/canister/mod.rs @@ -1,4 +1,5 @@ use clap::Subcommand; +use icp::canister::Visibility; pub(crate) mod call; pub(crate) mod create; @@ -16,6 +17,59 @@ pub(crate) mod status; pub(crate) mod stop; pub(crate) mod top_up; +/// Lists principals one per line below a label, indented two spaces past +/// `indent` — the indent of the label itself — so they nest the way the other +/// lists in `canister status` and `canister settings show` do. `noun` names what +/// each line holds, and stands in for the list when it is empty. Principals are +/// sorted so repeated calls print the same order. +fn format_principal_list( + principals: impl IntoIterator, + noun: &str, + indent: &str, +) -> String { + let mut principals: Vec = principals.into_iter().collect(); + principals.sort(); + + if principals.is_empty() { + return format!("\n{indent} {noun} list is empty"); + } + + principals + .iter() + .map(|principal| format!("\n{indent} {noun}: {principal}")) + .collect() +} + +/// Renders a visibility setting for `canister status` and `canister settings show`. +/// +/// The policy goes on the label's own line, with any allowed viewers listed +/// below it. `viewer` names what the setting grants — "log viewer", "status +/// viewer" — since a report carries one line per setting and the entries would +/// otherwise not say which they belong to. +pub(crate) fn format_visibility(visibility: &Visibility, viewer: &str, indent: &str) -> String { + match visibility { + Visibility::Controllers => "Controllers".to_string(), + Visibility::Public => "Public".to_string(), + Visibility::AllowedViewers(viewers) => format!( + "Allowed viewers{}", + format_principal_list(viewers.iter().map(|p| p.to_string()), viewer, indent) + ), + } +} + +/// Renders a canister's controllers for the same two reports. Unlike a +/// visibility setting, the list stands alone rather than qualifying a policy, so +/// the label is part of what this returns. +pub(crate) fn format_controllers( + controllers: impl IntoIterator, + indent: &str, +) -> String { + format!( + "{indent}Controllers:{}", + format_principal_list(controllers, "controller", indent) + ) +} + /// Perform canister operations against a network #[derive(Debug, Subcommand)] #[allow(clippy::large_enum_variant)] @@ -38,3 +92,87 @@ pub(crate) enum Command { Stop(stop::StopArgs), TopUp(top_up::TopUpArgs), } + +#[cfg(test)] +mod tests { + use candid::Principal; + + use super::*; + + fn principal(text: &str) -> Principal { + Principal::from_text(text).unwrap() + } + + /// Allowed viewers are listed one per line, sorted, and nested two spaces + /// past the label — which sits at a different indent in `canister status` + /// than in `canister settings show`. + #[test] + fn allowed_viewers_are_listed_one_per_line() { + let viewers = Visibility::AllowedViewers(vec![ + principal("ryjl3-tyaaa-aaaaa-aaaba-cai"), + principal("aaaaa-aa"), + ]); + + // `settings show`, where the label is not indented. + assert_eq!( + format!( + "Status visibility: {}", + format_visibility(&viewers, "status viewer", "") + ), + "Status visibility: Allowed viewers\n \ + status viewer: aaaaa-aa\n \ + status viewer: ryjl3-tyaaa-aaaaa-aaaba-cai" + ); + // `canister status`, where it sits two spaces in. + assert_eq!( + format!( + " Log visibility: {}", + format_visibility(&viewers, "log viewer", " ") + ), + " Log visibility: Allowed viewers\n \ + log viewer: aaaaa-aa\n \ + log viewer: ryjl3-tyaaa-aaaaa-aaaba-cai" + ); + } + + /// The fixed policies stay on the label's line, and an empty list says so + /// where its entries would have gone. + #[test] + fn fixed_policies_are_rendered_inline() { + assert_eq!( + format_visibility(&Visibility::Controllers, "log viewer", " "), + "Controllers" + ); + assert_eq!( + format_visibility(&Visibility::Public, "log viewer", " "), + "Public" + ); + assert_eq!( + format_visibility(&Visibility::AllowedViewers(vec![]), "log viewer", " "), + "Allowed viewers\n log viewer list is empty" + ); + } + + /// Controllers are listed the same way, but carry their own label. + #[test] + fn controllers_are_listed_one_per_line() { + let controllers = ["ryjl3-tyaaa-aaaaa-aaaba-cai", "aaaaa-aa"].map(str::to_string); + + assert_eq!( + format_controllers(controllers.clone(), " "), + " Controllers:\n \ + controller: aaaaa-aa\n \ + controller: ryjl3-tyaaa-aaaaa-aaaba-cai" + ); + assert_eq!( + format_controllers(controllers, ""), + "Controllers:\n \ + controller: aaaaa-aa\n \ + controller: ryjl3-tyaaa-aaaaa-aaaba-cai" + ); + assert_eq!( + format_controllers([], " "), + " Controllers:\n controller list is empty" + ); + } +} diff --git a/crates/icp-cli/src/commands/canister/settings/show.rs b/crates/icp-cli/src/commands/canister/settings/show.rs index a12628d3..8f17b4e6 100644 --- a/crates/icp-cli/src/commands/canister/settings/show.rs +++ b/crates/icp-cli/src/commands/canister/settings/show.rs @@ -1,17 +1,21 @@ use clap::Args; use ic_agent::export::Principal; -use ic_management_canister_types::{CanisterIdRecord, DefiniteCanisterSettings, LogVisibility}; +use ic_management_canister_types::{CanisterIdRecord, DefiniteCanisterSettings}; use icp::context::Context; use std::fmt::Write; use icp::operations::proxy_management; -use crate::commands::args::CanisterCommandArgs; +use crate::commands::{ + args::CanisterCommandArgs, + canister::{format_controllers, format_visibility}, +}; /// Show the settings of a canister. /// /// Queries the canister_status endpoint of the management canister and -/// displays only the settings fields. Requires the caller to be a controller. +/// displays only the settings fields. Requires the caller to be allowed to read +/// the canister's status, which by default means being one of its controllers. #[derive(Debug, Args)] pub(crate) struct ShowArgs { #[command(flatten)] @@ -66,12 +70,8 @@ fn build_output(s: &DefiniteCanisterSettings) -> String { let mut buf = String::new(); writeln!( &mut buf, - "Controllers: {}", - s.controllers - .iter() - .map(|p| p.to_string()) - .collect::>() - .join(", ") + "{}", + format_controllers(s.controllers.iter().map(|p| p.to_string()), "") ) .unwrap(); writeln!(&mut buf, "Compute allocation: {}", s.compute_allocation).unwrap(); @@ -92,20 +92,18 @@ fn build_output(s: &DefiniteCanisterSettings) -> String { .unwrap(); writeln!(&mut buf, "Log memory limit: {}", s.log_memory_limit).unwrap(); - let log_visibility = match &s.log_visibility { - LogVisibility::Controllers => "Controllers".to_string(), - LogVisibility::Public => "Public".to_string(), - LogVisibility::AllowedViewers(viewers) => { - if viewers.is_empty() { - "Allowed viewers list is empty".to_string() - } else { - let mut v: Vec = viewers.iter().map(|p| p.to_string()).collect(); - v.sort(); - format!("Allowed viewers: {}", v.join(", ")) - } - } - }; - writeln!(&mut buf, "Log visibility: {log_visibility}").unwrap(); + writeln!( + &mut buf, + "Log visibility: {}", + format_visibility(&s.log_visibility.clone().into(), "log viewer", "") + ) + .unwrap(); + writeln!( + &mut buf, + "Status visibility: {}", + format_visibility(&s.status_visibility.clone().into(), "status viewer", "") + ) + .unwrap(); if s.environment_variables.is_empty() { writeln!(&mut buf, "Environment variables: N/A").unwrap(); diff --git a/crates/icp-cli/src/commands/canister/settings/update.rs b/crates/icp-cli/src/commands/canister/settings/update.rs index 71e45808..f708fc0b 100644 --- a/crates/icp-cli/src/commands/canister/settings/update.rs +++ b/crates/icp-cli/src/commands/canister/settings/update.rs @@ -5,10 +5,11 @@ use dialoguer::Confirm; use ic_agent::Identity; use ic_agent::export::Principal; use ic_management_canister_types::{ - CanisterIdRecord, CanisterSettings, CanisterStatusResult, EnvironmentVariable, LogVisibility, + CanisterIdRecord, CanisterSettings, CanisterStatusResult, EnvironmentVariable, UpdateSettingsArgs, }; use icp::ProjectLoadError; +use icp::canister::Visibility; use icp::context::{CanisterSelection, Context}; use icp::parsers::{CyclesAmount, DurationAmount, MemoryAmount}; use std::collections::{HashMap, HashSet}; @@ -51,18 +52,24 @@ pub(crate) struct LogVisibilityOpt { /// Use --add-log-viewer / --set-log-viewer to grant access to specific principals instead. #[arg( long, - value_parser = log_visibility_parser, + value_parser = visibility_parser, conflicts_with("add_log_viewer"), conflicts_with("remove_log_viewer"), conflicts_with("set_log_viewer"), )] - log_visibility: Option, + log_visibility: Option, - /// Add a principal to the allowed log viewers list + /// Add a principal to the allowed log viewers list. + /// + /// Rejected while log visibility is public, which has no viewers list to + /// add to; use --set-log-viewer to replace the public policy with a list. #[arg(long, action = ArgAction::Append, conflicts_with("set_log_viewer"))] add_log_viewer: Option>, - /// Remove a principal from the allowed log viewers list + /// Remove a principal from the allowed log viewers list. + /// + /// Rejected while log visibility is public, which has no viewers list to + /// remove from; use --log-visibility controllers to revoke public access. #[arg(long, action = ArgAction::Append, conflicts_with("set_log_viewer"))] remove_log_viewer: Option>, @@ -72,11 +79,186 @@ pub(crate) struct LogVisibilityOpt { } impl LogVisibilityOpt { + fn flags(&self) -> VisibilityFlags<'_> { + VisibilityFlags { + label: "Log visibility", + stem: "log", + fixed: self.log_visibility.as_ref(), + add: self.add_log_viewer.as_deref(), + remove: self.remove_log_viewer.as_deref(), + set: self.set_log_viewer.as_deref(), + } + } + pub(crate) fn require_current_settings(&self) -> bool { - self.add_log_viewer.is_some() || self.remove_log_viewer.is_some() + self.flags().require_current_settings() } } +#[derive(Clone, Debug, Default, Args)] +pub(crate) struct StatusVisibilityOpt { + /// Set status visibility to a fixed policy [possible values: controllers, public]. + /// Conflicts with --add-status-viewer, --remove-status-viewer, and --set-status-viewer. + /// Use --add-status-viewer / --set-status-viewer to grant access to specific principals instead. + #[arg( + long, + value_parser = visibility_parser, + conflicts_with("add_status_viewer"), + conflicts_with("remove_status_viewer"), + conflicts_with("set_status_viewer"), + )] + status_visibility: Option, + + /// Add a principal to the allowed status viewers list. + /// + /// Rejected while status visibility is public, which has no viewers list to + /// add to; use --set-status-viewer to replace the public policy with a list. + #[arg(long, action = ArgAction::Append, conflicts_with("set_status_viewer"))] + add_status_viewer: Option>, + + /// Remove a principal from the allowed status viewers list. + /// + /// Rejected while status visibility is public, which has no viewers list to + /// remove from; use --status-visibility controllers to revoke public access. + #[arg(long, action = ArgAction::Append, conflicts_with("set_status_viewer"))] + remove_status_viewer: Option>, + + /// Replace the allowed status viewers list with the specified principals + #[arg(long, action = ArgAction::Append)] + set_status_viewer: Option>, +} + +impl StatusVisibilityOpt { + fn flags(&self) -> VisibilityFlags<'_> { + VisibilityFlags { + label: "Status visibility", + stem: "status", + fixed: self.status_visibility.as_ref(), + add: self.add_status_viewer.as_deref(), + remove: self.remove_status_viewer.as_deref(), + set: self.set_status_viewer.as_deref(), + } + } + + pub(crate) fn require_current_settings(&self) -> bool { + self.flags().require_current_settings() + } +} + +/// The flags of one visibility group, borrowed so the resolution below is +/// written once for every setting that has such a group. +struct VisibilityFlags<'a> { + /// How the setting reads in prose, e.g. `Log visibility`. + label: &'static str, + /// The stem its flags share, e.g. `log` for `--add-log-viewer`. + stem: &'static str, + fixed: Option<&'a Visibility>, + add: Option<&'a [Principal]>, + remove: Option<&'a [Principal]>, + set: Option<&'a [Principal]>, +} + +impl VisibilityFlags<'_> { + /// Any viewer edit is resolved against the canister's current policy, so it + /// has to be fetched first: `--add` and `--remove` build on the current + /// list, and `--set` needs it only to warn about what it replaces. + fn require_current_settings(&self) -> bool { + self.add.is_some() || self.remove.is_some() || self.set.is_some() + } + + /// Turns the flags of one group into the policy to send, rejecting the + /// edits the group cannot express. Pure: it neither reads the network nor + /// prints, and what a legal edit costs the caller is warned about by + /// [`maybe_warn_on_lost_access`]. + fn resolve(&self, current: Option<&Visibility>) -> Result { + if let Some(fixed) = self.fixed { + return Ok(fixed.clone()); + } + + if let Some(viewers) = self.set { + return Ok(Visibility::AllowedViewers(viewers.to_vec())); + } + + let mut viewers = match current { + Some(Visibility::AllowedViewers(viewers)) => viewers.clone(), + Some(Visibility::Public) if self.add.is_some() || self.remove.is_some() => { + return Err(self.public_has_no_viewers_list()); + } + // `controllers` has no list either, but building one from it only + // ever grants access on top, so the edits mean what they say. + _ => vec![], + }; + + if let Some(to_be_added) = self.add { + for principal in to_be_added { + if !viewers.contains(principal) { + viewers.push(*principal); + } + } + } + + if let Some(to_be_removed) = self.remove { + viewers.retain(|principal| !to_be_removed.contains(principal)); + } + + Ok(Visibility::AllowedViewers(viewers)) + } + + /// `public` grants access to everyone, so it carries no viewers list for a + /// relative edit to be relative to. The only way to honour one would be to + /// start a list from empty, which revokes everyone else's access — a policy + /// change these flags do not name, so it is refused in favour of the flags + /// that do. + fn public_has_no_viewers_list(&self) -> anyhow::Error { + let (label, stem) = (self.label, self.stem); + let edits = match (self.add.is_some(), self.remove.is_some()) { + (true, true) => format!("--add-{stem}-viewer / --remove-{stem}-viewer"), + (true, false) => format!("--add-{stem}-viewer"), + _ => format!("--remove-{stem}-viewer"), + }; + + anyhow::anyhow!( + "{label} is currently public, so there is no allowed viewers list for {edits} to edit. \ + Use `--set-{stem}-viewer ` to replace the public policy with an explicit \ + list, or `--{stem}-visibility controllers` to revoke public access on its own." + ) + } +} + +/// Warns when a legal viewer edit still takes access away rather than granting +/// it: `--set-*-viewer` on a `public` canister revokes everyone else's access, +/// and removing the last viewer leaves the controllers alone with it. +/// +/// A warning rather than a prompt or a refusal: both edits state outright what +/// the new list is, so unlike the relative edits [`VisibilityFlags::resolve`] +/// refuses on a `public` canister, they say what they do. Either is reversible +/// by any controller, and a prompt would break scripted use. +fn maybe_warn_on_lost_access(label: &str, current: Option<&Visibility>, resolved: &Visibility) { + let Visibility::AllowedViewers(viewers) = resolved else { + return; + }; + + if current == Some(&Visibility::Public) { + warn!( + "{label} is currently public; listing allowed viewers revokes access for everyone else" + ); + } + if viewers.is_empty() { + warn!("{label} is left with no allowed viewers; only the controllers keep access"); + } +} + +/// Resolves one visibility group against the policy the canister carries now, +/// warning about what the result costs. +fn resolve_visibility( + flags: VisibilityFlags<'_>, + current: Option, +) -> Result { + let resolved = flags.resolve(current.as_ref())?; + maybe_warn_on_lost_access(flags.label, current.as_ref(), &resolved); + Ok(resolved) +} + #[derive(Clone, Debug, Default, Args)] pub(crate) struct EnvironmentVariableOpt { /// Add a canister environment variable in KEY=VALUE format @@ -143,6 +325,9 @@ pub(crate) struct UpdateArgs { #[command(flatten)] log_visibility: Option, + #[command(flatten)] + status_visibility: Option, + #[command(flatten)] environment_variables: Option, @@ -233,11 +418,27 @@ pub(crate) async fn exec(ctx: &Context, args: &UpdateArgs) -> Result<(), anyhow: } } - // Handle log visibility. - let mut log_visibility: Option = None; - if let Some(log_visibility_opt) = args.log_visibility.clone() { - log_visibility = get_log_visibility(&log_visibility_opt, current_status.as_ref()); - } + // Handle log and status visibility. + let log_visibility = args + .log_visibility + .as_ref() + .map(|opt| { + let current = current_status + .as_ref() + .map(|status| Visibility::from(status.settings.log_visibility.clone())); + resolve_visibility(opt.flags(), current) + }) + .transpose()?; + let status_visibility = args + .status_visibility + .as_ref() + .map(|opt| { + let current = current_status + .as_ref() + .map(|status| Visibility::from(status.settings.status_visibility.clone())); + resolve_visibility(opt.flags(), current) + }) + .transpose()?; // Handle environment variables. let mut environment_variables: Option> = None; @@ -288,6 +489,11 @@ pub(crate) async fn exec(ctx: &Context, args: &UpdateArgs) -> Result<(), anyhow: "Log visibility is already set in icp.yaml; this new value will be overridden on next settings sync" ); } + if status_visibility.is_some() && configured_settings.status_visibility.is_some() { + warn!( + "Status visibility is already set in icp.yaml; this new value will be overridden on next settings sync" + ); + } let settings = CanisterSettings { controllers, @@ -304,12 +510,12 @@ pub(crate) async fn exec(ctx: &Context, args: &UpdateArgs) -> Result<(), anyhow: .as_ref() .map(|m| Nat::from(m.get())), log_memory_limit: args.log_memory_limit.as_ref().map(|m| Nat::from(m.get())), - log_visibility, + log_visibility: log_visibility.map(Into::into), + status_visibility: status_visibility.map(Into::into), environment_variables, - // TODO: expose snapshot_visibility as a `settings update` flag (with - // set/add/remove-viewer sub-flags), mirroring log_visibility. Tracked for - // a follow-up PR; until then, leave it unchanged. + // Not exposed as flags yet; `None` leaves them unchanged. snapshot_visibility: None, + minimum_incoming_canister_call_cycles: None, }; proxy_management::update_settings( @@ -335,10 +541,10 @@ fn compute_allocation_parser(compute_allocation: &str) -> Result { Err("Must be a percent between 0 and 100".to_string()) } -fn log_visibility_parser(log_visibility: &str) -> Result { - match log_visibility { - "public" => Ok(LogVisibility::Public), - "controllers" => Ok(LogVisibility::Controllers), +fn visibility_parser(visibility: &str) -> Result { + match visibility { + "public" => Ok(Visibility::Public), + "controllers" => Ok(Visibility::Controllers), _ => Err("Must be `controllers` or `public`.".to_string()), } } @@ -366,6 +572,12 @@ fn require_current_settings(args: &UpdateArgs) -> bool { return true; } + if let Some(status_visibility) = &args.status_visibility + && status_visibility.require_current_settings() + { + return true; + } + if let Some(environment_variables) = &args.environment_variables && environment_variables.require_current_settings() { @@ -406,48 +618,6 @@ fn get_controllers( } } -fn get_log_visibility( - log_visibility: &LogVisibilityOpt, - current_status: Option<&CanisterStatusResult>, -) -> Option { - if let Some(log_visibility) = log_visibility.log_visibility.as_ref() { - return Some(log_visibility.clone()); - } - - if let Some(viewer) = log_visibility.set_log_viewer.as_ref() { - // TODO(VZ): Warn for switching from public to viewers. - return Some(LogVisibility::AllowedViewers(viewer.clone())); - } - - let mut log_viewers: Vec = match current_status { - Some(status) => match &status.settings.log_visibility { - LogVisibility::AllowedViewers(viewers) => viewers.clone(), - _ => vec![], - }, - None => vec![], - }; - - if let Some(to_be_added) = log_visibility.add_log_viewer.as_ref() { - // TODO(VZ): Warn for switching from public to viewers. - for principal in to_be_added { - if !log_viewers.iter().any(|x| x == principal) { - log_viewers.push(*principal); - } - } - } - - if let Some(removed) = log_visibility.remove_log_viewer.as_ref() { - // TODO(VZ): Warn for removing from if log visibility is public and controllers. - for principal in removed { - if let Some(idx) = log_viewers.iter().position(|x| x == principal) { - log_viewers.swap_remove(idx); - } - } - } - - Some(LogVisibility::AllowedViewers(log_viewers)) -} - fn get_environment_variables( environment_variables: &EnvironmentVariableOpt, current_status: Option<&CanisterStatusResult>, @@ -511,3 +681,209 @@ fn maybe_warn_on_env_vars_change( } } } + +#[cfg(test)] +mod tests { + use super::*; + + fn principal(text: &str) -> Principal { + Principal::from_text(text).unwrap() + } + + /// The three principals below, in an order no sort would produce, so the + /// tests can tell a preserved list from a reordered one. + fn alice() -> Principal { + principal("ryjl3-tyaaa-aaaaa-aaaba-cai") + } + + fn bob() -> Principal { + principal("2vxsx-fae") + } + + fn carol() -> Principal { + principal("aaaaa-aa") + } + + /// Builds a group of flags the way [`LogVisibilityOpt::flags`] does, from + /// the values clap would have parsed. + fn flags<'a>( + fixed: Option<&'a Visibility>, + add: Option<&'a [Principal]>, + remove: Option<&'a [Principal]>, + set: Option<&'a [Principal]>, + ) -> VisibilityFlags<'a> { + VisibilityFlags { + label: "Log visibility", + stem: "log", + fixed, + add, + remove, + set, + } + } + + /// A fixed policy conflicts with the viewer flags in clap, so this can only + /// happen if that wiring breaks; resolution still has to pick one, and the + /// explicit policy is it. + #[test] + fn fixed_policy_wins_over_viewer_edits() { + let viewers = [alice()]; + let resolved = flags( + Some(&Visibility::Public), + Some(&viewers), + None, + Some(&viewers), + ) + .resolve(Some(&Visibility::Controllers)) + .unwrap(); + + assert_eq!(resolved, Visibility::Public); + } + + #[test] + fn set_replaces_the_current_list() { + let current = Visibility::AllowedViewers(vec![alice(), bob()]); + let viewers = [carol()]; + + assert_eq!( + flags(None, None, None, Some(&viewers)) + .resolve(Some(¤t)) + .unwrap(), + Visibility::AllowedViewers(vec![carol()]) + ); + } + + #[test] + fn add_appends_without_duplicating() { + let current = Visibility::AllowedViewers(vec![alice(), bob()]); + let to_add = [bob(), carol()]; + + assert_eq!( + flags(None, Some(&to_add), None, None) + .resolve(Some(¤t)) + .unwrap(), + Visibility::AllowedViewers(vec![alice(), bob(), carol()]) + ); + } + + /// `controllers` carries no list to build on, but a list built from it + /// grants access on top of the controllers rather than taking any away, so + /// the edit means what it says and starts from empty. + #[test] + fn viewer_edits_against_controllers_start_from_empty() { + let to_add = [alice()]; + assert_eq!( + flags(None, Some(&to_add), None, None) + .resolve(Some(&Visibility::Controllers)) + .unwrap(), + Visibility::AllowedViewers(vec![alice()]) + ); + + let to_remove = [alice()]; + assert_eq!( + flags(None, None, Some(&to_remove), None) + .resolve(Some(&Visibility::Controllers)) + .unwrap(), + Visibility::AllowedViewers(vec![]) + ); + } + + /// `public` carries no list either, and starting one would revoke everyone + /// else's access — which a relative edit does not say to do, so it is + /// refused, pointing at the two flags that state a policy outright. + #[test] + fn relative_viewer_edits_are_refused_while_public() { + let viewers = [alice()]; + + for (add, remove, named) in [ + (Some(&viewers[..]), None, "--add-log-viewer"), + (None, Some(&viewers[..]), "--remove-log-viewer"), + ( + Some(&viewers[..]), + Some(&viewers[..]), + "--add-log-viewer / --remove-log-viewer", + ), + ] { + let err = flags(None, add, remove, None) + .resolve(Some(&Visibility::Public)) + .unwrap_err() + .to_string(); + + assert!(err.contains("Log visibility is currently public"), "{err}"); + assert!(err.contains(named), "{err}"); + assert!(err.contains("--set-log-viewer"), "{err}"); + assert!(err.contains("--log-visibility controllers"), "{err}"); + } + } + + /// Stating the new list outright is still allowed while public: it says + /// what the policy becomes, so nothing about it is a surprise. + #[test] + fn set_is_allowed_while_public() { + let viewers = [alice()]; + + assert_eq!( + flags(None, None, None, Some(&viewers)) + .resolve(Some(&Visibility::Public)) + .unwrap(), + Visibility::AllowedViewers(vec![alice()]) + ); + } + + /// Removal keeps the order of the viewers it leaves behind, since that + /// order is what `settings show` and `canister status` print. + #[test] + fn remove_drops_named_viewers_and_keeps_the_rest_in_order() { + let current = Visibility::AllowedViewers(vec![alice(), bob(), carol()]); + let to_remove = [bob()]; + + assert_eq!( + flags(None, None, Some(&to_remove), None) + .resolve(Some(¤t)) + .unwrap(), + Visibility::AllowedViewers(vec![alice(), carol()]) + ); + } + + /// Removing every viewer leaves the list empty rather than falling back to + /// `controllers` — the same policy in effect, and what the canister keeps. + #[test] + fn remove_can_empty_the_list() { + let current = Visibility::AllowedViewers(vec![alice()]); + let to_remove = [alice(), bob()]; + + assert_eq!( + flags(None, None, Some(&to_remove), None) + .resolve(Some(¤t)) + .unwrap(), + Visibility::AllowedViewers(vec![]) + ); + } + + /// Adding and removing in one command applies both, in that order. + #[test] + fn add_and_remove_apply_together() { + let current = Visibility::AllowedViewers(vec![alice()]); + let to_add = [bob(), carol()]; + let to_remove = [alice(), bob()]; + + assert_eq!( + flags(None, Some(&to_add), Some(&to_remove), None) + .resolve(Some(¤t)) + .unwrap(), + Visibility::AllowedViewers(vec![carol()]) + ); + } + + /// Only a fixed policy stands on its own; every viewer edit is resolved + /// against the current one, so it has to be fetched. + #[test] + fn only_a_fixed_policy_needs_no_current_settings() { + let viewers = [alice()]; + + assert!(!flags(Some(&Visibility::Public), None, None, None).require_current_settings()); + assert!(flags(None, Some(&viewers), None, None).require_current_settings()); + assert!(flags(None, None, Some(&viewers), None).require_current_settings()); + assert!(flags(None, None, None, Some(&viewers)).require_current_settings()); + } +} diff --git a/crates/icp-cli/src/commands/canister/status.rs b/crates/icp-cli/src/commands/canister/status.rs index 79cb4c7d..05415c87 100644 --- a/crates/icp-cli/src/commands/canister/status.rs +++ b/crates/icp-cli/src/commands/canister/status.rs @@ -2,10 +2,9 @@ use anyhow::{anyhow, bail}; use clap::Args; use clap_complete::ArgValueCandidates; use ic_agent::{Agent, AgentError, agent::RejectResponse, export::Principal}; -use ic_management_canister_types::{ - CanisterIdRecord, CanisterStatusResult, EnvironmentVariable, LogVisibility, -}; +use ic_management_canister_types::{CanisterIdRecord, CanisterStatusResult, EnvironmentVariable}; use icp::{ + canister::Visibility, context::{CanisterSelection, Context, EnvironmentSelection, NetworkSelection}, identity::IdentitySelection, }; @@ -15,7 +14,13 @@ use tracing::debug; use icp::operations::{proxy::UpdateOrProxyError, proxy_management}; -use crate::{commands::args, options}; +use crate::{ + commands::{ + args, + canister::{format_controllers, format_visibility}, + }, + options, +}; /// Error code returned by the replica if the target canister is not found const E_CANISTER_NOT_FOUND: &str = "IC0301"; @@ -371,13 +376,19 @@ struct SerializableCanisterSettings { wasm_memory_limit: String, wasm_memory_threshold: String, log_memory_limit: String, - log_visibility: SerializableLogVisibility, + log_visibility: SerializableVisibility, + status_visibility: SerializableVisibility, environment_variables: Vec, } -#[derive(Serialize, Clone)] +/// `--json` renders a visibility setting as `{"type": ..., "value": ...}`, +/// which differs from the manifest form [`Visibility`] serializes to. +#[derive(Clone)] +struct SerializableVisibility(Visibility); + +#[derive(Serialize)] #[serde(tag = "type", content = "value")] -enum SerializableLogVisibility { +enum VisibilityRepr { Controllers, Public, AllowedViewers(Vec), @@ -424,21 +435,23 @@ impl SerializableCanisterSettings { wasm_memory_limit: settings.wasm_memory_limit.to_string(), wasm_memory_threshold: settings.wasm_memory_threshold.to_string(), log_memory_limit: settings.log_memory_limit.to_string(), - log_visibility: SerializableLogVisibility::from(&settings.log_visibility), + log_visibility: SerializableVisibility(settings.log_visibility.clone().into()), + status_visibility: SerializableVisibility(settings.status_visibility.clone().into()), environment_variables: settings.environment_variables.clone(), } } } -impl SerializableLogVisibility { - fn from(visibility: &LogVisibility) -> Self { - match visibility { - LogVisibility::Controllers => Self::Controllers, - LogVisibility::Public => Self::Public, - LogVisibility::AllowedViewers(viewers) => { - Self::AllowedViewers(viewers.iter().map(|p| p.to_string()).collect()) +impl Serialize for SerializableVisibility { + fn serialize(&self, serializer: S) -> Result { + let repr = match &self.0 { + Visibility::Controllers => VisibilityRepr::Controllers, + Visibility::Public => VisibilityRepr::Public, + Visibility::AllowedViewers(viewers) => { + VisibilityRepr::AllowedViewers(viewers.iter().map(|p| p.to_string()).collect()) } - } + }; + repr.serialize(serializer) } } @@ -461,7 +474,11 @@ fn build_public_output(result: &PublicCanisterStatusResult) -> Result Result Result "Controllers".to_string(), - SerializableLogVisibility::Public => "Public".to_string(), - SerializableLogVisibility::AllowedViewers(mut viewers) => { - if viewers.is_empty() { - "Allowed viewers list is empty".to_string() - } else { - viewers.sort(); - format!("Allowed viewers: {}", viewers.join(", ")) - } - } - }; - writeln!(&mut buf, " Log visibility: {log_visibility}")?; + writeln!( + &mut buf, + " Log visibility: {}", + format_visibility(&settings.log_visibility.0, "log viewer", " ") + )?; + writeln!( + &mut buf, + " Status visibility: {}", + format_visibility(&settings.status_visibility.0, "status viewer", " ") + )?; // Display environment variables configured for this canister // Environment variables are key-value pairs that can be accessed within the canister @@ -625,4 +639,21 @@ mod tests { .is_none() ); } + + /// `--json` renders visibility as a tagged `{"type", "value"}` object, which + /// is a different shape from the manifest form `Visibility` serializes to, + /// so it is pinned here rather than left to a derive. + #[test] + fn json_visibility_shape() { + let json = |v: Visibility| serde_json::to_string(&SerializableVisibility(v)).unwrap(); + + assert_eq!(json(Visibility::Controllers), r#"{"type":"Controllers"}"#); + assert_eq!(json(Visibility::Public), r#"{"type":"Public"}"#); + assert_eq!( + json(Visibility::AllowedViewers(vec![ + Principal::from_text("aaaaa-aa").unwrap() + ])), + r#"{"type":"AllowedViewers","value":["aaaaa-aa"]}"# + ); + } } diff --git a/crates/icp-cli/tests/canister_create_tests.rs b/crates/icp-cli/tests/canister_create_tests.rs index 9db9ff21..00b99f40 100644 --- a/crates/icp-cli/tests/canister_create_tests.rs +++ b/crates/icp-cli/tests/canister_create_tests.rs @@ -973,7 +973,7 @@ async fn canister_create_with_unresolved_canister_controller_warns_and_syncs() { ]) .assert() .success() - .stdout(contains("Controllers: 2vxsx-fae")); + .stdout(contains("controller: 2vxsx-fae")); // Creating "b" triggers sync_controller_dependents, which updates "a"'s controller list. ctx.icp() diff --git a/crates/icp-cli/tests/canister_info_tests.rs b/crates/icp-cli/tests/canister_info_tests.rs index 2b22ef76..24322486 100644 --- a/crates/icp-cli/tests/canister_info_tests.rs +++ b/crates/icp-cli/tests/canister_info_tests.rs @@ -61,7 +61,7 @@ async fn canister_status() { ]) .assert() .success() - .stdout(contains("Controllers: 2vxsx-fae").and(contains( + .stdout(contains("controller: 2vxsx-fae").and(contains( "Module hash: 0x17a05e36278cd04c7ae6d3d3226c136267b9df7525a0657521405e22ec96be7a", ))); } diff --git a/crates/icp-cli/tests/canister_settings_tests.rs b/crates/icp-cli/tests/canister_settings_tests.rs index f3ece0c0..659908a1 100644 --- a/crates/icp-cli/tests/canister_settings_tests.rs +++ b/crates/icp-cli/tests/canister_settings_tests.rs @@ -75,7 +75,7 @@ async fn canister_settings_update_controllers() { ]) .assert() .success() - .stdout(contains("Controllers: 2vxsx-fae").and(contains(principal_alice.as_str()).not())); + .stdout(contains("controller: 2vxsx-fae").and(contains(principal_alice.as_str()).not())); // Add controller ctx.icp() @@ -106,7 +106,7 @@ async fn canister_settings_update_controllers() { ]) .assert() .success() - .stdout(contains("Controllers: 2vxsx-fae").and(contains(principal_alice.as_str()))); + .stdout(contains("controller: 2vxsx-fae").and(contains(principal_alice.as_str()))); // Add and remove controller. ctx.icp() @@ -140,7 +140,7 @@ async fn canister_settings_update_controllers() { .assert() .success() .stdout( - contains("Controllers: 2vxsx-fae") + contains("controller: 2vxsx-fae") .and(contains(principal_alice.as_str()).not()) .and(contains(principal_bob.as_str())), ); @@ -174,7 +174,7 @@ async fn canister_settings_update_controllers() { ]) .assert() .success() - .stdout(contains("Controllers: 2vxsx-fae").and(contains(principal_bob.as_str()).not())); + .stdout(contains("controller: 2vxsx-fae").and(contains(principal_bob.as_str()).not())); // Add multiple controllers ctx.icp() @@ -208,7 +208,7 @@ async fn canister_settings_update_controllers() { .assert() .success() .stdout( - contains("Controllers: 2vxsx-fae") + contains("controller: 2vxsx-fae") .and(contains(principal_alice.as_str())) .and(contains(principal_bob.as_str())), ); @@ -245,7 +245,7 @@ async fn canister_settings_update_controllers() { .assert() .success() .stdout( - contains("Controllers: 2vxsx-fae") + contains("controller: 2vxsx-fae") .and(contains(principal_alice.as_str()).not()) .and(contains(principal_bob.as_str()).not()), ); @@ -473,7 +473,8 @@ async fn canister_settings_update_log_visibility() { .success() .stdout(contains("Log visibility: Public")); - // Add log viewer. + // Public carries no viewers list, so adding to one is refused rather than + // silently revoking public access. ctx.icp() .current_dir(&project_dir) .args([ @@ -487,7 +488,30 @@ async fn canister_settings_update_log_visibility() { "random-environment", ]) .assert() - .success(); + .failure() + .stderr(contains( + "Log visibility is currently public, so there is no allowed viewers list for --add-log-viewer to edit", + )); + + // Setting the list outright is the way to say it, and warns about what the + // public policy it replaces used to grant. + ctx.icp() + .current_dir(&project_dir) + .args([ + "canister", + "settings", + "update", + "my-canister", + "--set-log-viewer", + principal_alice.as_str(), + "--environment", + "random-environment", + ]) + .assert() + .success() + .stderr(contains( + "Log visibility is currently public; listing allowed viewers revokes access for everyone else", + )); // Query settings ctx.icp() @@ -503,7 +527,7 @@ async fn canister_settings_update_log_visibility() { .assert() .success() .stdout( - contains("Log visibility: Allowed viewers:").and(contains(principal_alice.as_str())), + contains("Log visibility: Allowed viewers").and(contains(principal_alice.as_str())), ); // Add and remove log viewer. @@ -538,7 +562,7 @@ async fn canister_settings_update_log_visibility() { .assert() .success() .stdout( - contains("Log visibility: Allowed viewers:") + contains("Log visibility: Allowed viewers") .and(contains(principal_alice.as_str()).not()) .and(contains(principal_bob.as_str())), ); @@ -572,7 +596,9 @@ async fn canister_settings_update_log_visibility() { ]) .assert() .success() - .stdout(contains("Log visibility: Allowed viewers list is empty")); + .stdout(contains( + "Log visibility: Allowed viewers\n log viewer list is empty", + )); // Add multiple log viewers. ctx.icp() @@ -606,7 +632,7 @@ async fn canister_settings_update_log_visibility() { .assert() .success() .stdout( - contains("Log visibility: Allowed viewers:") + contains("Log visibility: Allowed viewers") .and(contains(principal_alice.as_str())) .and(contains(principal_bob.as_str())), ); @@ -642,7 +668,9 @@ async fn canister_settings_update_log_visibility() { ]) .assert() .success() - .stdout(contains("Log visibility: Allowed viewers list is empty")); + .stdout(contains( + "Log visibility: Allowed viewers\n log viewer list is empty", + )); // Set multiple log viewers. ctx.icp() @@ -676,7 +704,7 @@ async fn canister_settings_update_log_visibility() { .assert() .success() .stdout( - contains("Log visibility: Allowed viewers:") + contains("Log visibility: Allowed viewers") .and(contains(principal_alice.as_str())) .and(contains(principal_bob.as_str())), ); @@ -954,7 +982,7 @@ async fn canister_settings_update_environment_variables() { .assert() .success() .stdout( - contains("Controllers: 2vxsx-fae") + contains("controller: 2vxsx-fae") .and(contains("Environment variables:")) .and(contains("PUBLIC_CANISTER_ID:my-canister")), ); @@ -1394,7 +1422,281 @@ async fn canister_settings_sync_log_visibility() { write_string(&project_dir.join("icp.yaml"), &pm_with_allowed_viewers) .expect("failed to write project manifest"); sync(&ctx, &project_dir); - confirm_log_visibility(&ctx, &project_dir, "Allowed viewers: 2vxsx-fae, aaaaa-aa"); + confirm_log_visibility( + &ctx, + &project_dir, + "Allowed viewers\n log viewer: 2vxsx-fae\n log viewer: aaaaa-aa", + ); + + // status_visibility takes the same manifest forms, and a single sync has to + // apply both settings: either change alone would satisfy the "settings + // already match" check that decides whether to send an update at all. + let pm_with_both = formatdoc! {r#" + canisters: + - name: my-canister + build: + steps: + - type: script + command: cp '{wasm}' "$ICP_WASM_OUTPUT_PATH" + settings: + log_visibility: public + status_visibility: + allowed_viewers: + - "aaaaa-aa" + - "2vxsx-fae" + + {NETWORK_RANDOM_PORT} + {ENVIRONMENT_RANDOM_PORT} + "#}; + + write_string(&project_dir.join("icp.yaml"), &pm_with_both) + .expect("failed to write project manifest"); + sync(&ctx, &project_dir); + confirm_log_visibility(&ctx, &project_dir, "Public"); + ctx.icp() + .current_dir(&project_dir) + .args([ + "canister", + "settings", + "show", + "my-canister", + "--environment", + "random-environment", + ]) + .assert() + .success() + .stdout(contains( + "Status visibility: Allowed viewers\n status viewer: 2vxsx-fae\n status viewer: aaaaa-aa", + )); +} + +/// Drives the `--*-status-viewer` / `--status-visibility` flags against a live +/// replica, checking each one both in `settings show` and in what it actually +/// grants: whether a non-controller may read the status, or falls back to the +/// public state-tree information. +/// +/// The flag-resolution matrix itself is unit-tested in +/// `commands::canister::settings::update`; what this adds is that clap wires the +/// flags to the right group and that the replica honours the result. +#[tokio::test] +async fn canister_settings_update_status_visibility() { + let ctx = TestContext::new(); + + let project_dir = ctx.create_project_dir("icp"); + + let client = clients::icp(&ctx, &project_dir, None); + let principal_alice = get_principal(&client, "alice"); + let principal_bob = get_principal(&client, "bob"); + + let wasm = ctx.make_asset("example_icp_mo.wasm"); + + let pm = formatdoc! {r#" + canisters: + - name: my-canister + build: + steps: + - type: script + command: cp '{wasm}' "$ICP_WASM_OUTPUT_PATH" + + {NETWORK_RANDOM_PORT} + {ENVIRONMENT_RANDOM_PORT} + "#}; + + write_string(&project_dir.join("icp.yaml"), &pm).expect("failed to write project manifest"); + + let _g = ctx.start_network_in(&project_dir, "random-network").await; + ctx.ping_until_healthy(&project_dir, "random-network"); + + clients::icp(&ctx, &project_dir, Some("random-environment".to_string())) + .mint_cycles(10 * TRILLION); + + ctx.icp() + .current_dir(&project_dir) + .args(["deploy", "--environment", "random-environment"]) + .assert() + .success(); + + fn update(ctx: &TestContext, project_dir: &Path, args: &[&str]) -> assert_cmd::assert::Assert { + let mut all = vec![ + "canister", + "settings", + "update", + "my-canister", + "--environment", + "random-environment", + ]; + all.extend_from_slice(args); + ctx.icp() + .current_dir(project_dir) + .args(all) + .assert() + .success() + } + + fn update_fails( + ctx: &TestContext, + project_dir: &Path, + args: &[&str], + ) -> assert_cmd::assert::Assert { + let mut all = vec![ + "canister", + "settings", + "update", + "my-canister", + "--environment", + "random-environment", + ]; + all.extend_from_slice(args); + ctx.icp() + .current_dir(project_dir) + .args(all) + .assert() + .failure() + } + + fn confirm(ctx: &TestContext, project_dir: &Path) -> assert_cmd::assert::Assert { + ctx.icp() + .current_dir(project_dir) + .args([ + "canister", + "settings", + "show", + "my-canister", + "--environment", + "random-environment", + ]) + .assert() + .success() + } + + /// `canister status` as alice, who is not a controller: the full report only + /// when she may read the status, the public fallback otherwise. + fn status_as_alice(ctx: &TestContext, project_dir: &Path) -> assert_cmd::assert::Assert { + ctx.icp() + .current_dir(project_dir) + .args([ + "canister", + "status", + "my-canister", + "--identity", + "alice", + "--environment", + "random-environment", + ]) + .assert() + .success() + } + + // The default is controllers, reported separately from log visibility. + confirm(&ctx, &project_dir).stdout( + contains("Status visibility: Controllers").and(contains("Log visibility: Controllers")), + ); + status_as_alice(&ctx, &project_dir).stdout(contains("Status:").not()); + + // --add-status-viewer grants it to alice, relative to the current list. + update( + &ctx, + &project_dir, + &["--add-status-viewer", principal_alice.as_str()], + ); + status_as_alice(&ctx, &project_dir) + .stdout(contains("Status: Running").and(contains("Status visibility: Allowed viewers"))); + + // Add and remove in one call, again relative to the current list. Alice + // loses access, so the fallback comes back. + update( + &ctx, + &project_dir, + &[ + "--add-status-viewer", + principal_bob.as_str(), + "--remove-status-viewer", + principal_alice.as_str(), + ], + ); + confirm(&ctx, &project_dir).stdout( + contains("Status visibility: Allowed viewers") + .and(contains(principal_bob.as_str())) + .and(contains(principal_alice.as_str()).not()), + ); + status_as_alice(&ctx, &project_dir).stdout(contains("Status:").not()); + + // --set-status-viewer replaces the list outright. + update( + &ctx, + &project_dir, + &["--set-status-viewer", principal_alice.as_str()], + ); + confirm(&ctx, &project_dir).stdout( + contains("Status visibility: Allowed viewers") + .and(contains(principal_alice.as_str())) + .and(contains(principal_bob.as_str()).not()), + ); + + // Public grants it to everyone, and leaves log visibility alone. + update(&ctx, &project_dir, &["--status-visibility", "public"]); + confirm(&ctx, &project_dir) + .stdout(contains("Status visibility: Public").and(contains("Log visibility: Controllers"))); + status_as_alice(&ctx, &project_dir).stdout(contains("Status: Running")); + + // A relative viewer edit has no list to be relative to while the status is + // public, and is refused rather than silently revoking public access. + update_fails( + &ctx, + &project_dir, + &["--add-status-viewer", principal_bob.as_str()], + ) + .stderr(contains( + "Status visibility is currently public, so there is no allowed viewers list for --add-status-viewer to edit", + )); + update_fails( + &ctx, + &project_dir, + &["--remove-status-viewer", principal_bob.as_str()], + ) + .stderr(contains("--remove-status-viewer")); + // Refused, so the canister is untouched and alice still reads the status. + status_as_alice(&ctx, &project_dir).stdout(contains("Status: Running")); + + // Stating the list outright is allowed, and warns about what it revokes. + update( + &ctx, + &project_dir, + &["--set-status-viewer", principal_bob.as_str()], + ) + .stderr(contains( + "Status visibility is currently public; listing allowed viewers revokes access for everyone else", + )); + status_as_alice(&ctx, &project_dir).stdout(contains("Status:").not()); + + // Removing the last viewer leaves the controllers alone with it, which is + // warned about but not refused: the edit says what it does. + update( + &ctx, + &project_dir, + &["--remove-status-viewer", principal_bob.as_str()], + ) + .stderr(contains( + "Status visibility is left with no allowed viewers; only the controllers keep access", + )); + + // Revoking it puts the fallback back in place. + update(&ctx, &project_dir, &["--status-visibility", "controllers"]); + status_as_alice(&ctx, &project_dir).stdout(contains("Status:").not()); + + // An update naming neither visibility group must leave both alone, rather + // than resetting them to an empty allowed-viewers list. + update( + &ctx, + &project_dir, + &["--set-log-viewer", principal_alice.as_str()], + ); + update(&ctx, &project_dir, &["--freezing-threshold", "7d"]); + confirm(&ctx, &project_dir).stdout( + contains("Status visibility: Controllers") + .and(contains("Log visibility: Allowed viewers")) + .and(contains(principal_alice.as_str())), + ); } #[tokio::test] @@ -1572,6 +1874,7 @@ async fn canister_settings_show() { .and(contains(r#""wasm_memory_threshold""#)) .and(contains(r#""log_memory_limit""#)) .and(contains(r#""log_visibility""#)) + .and(contains(r#""status_visibility""#)) .and(contains(r#""environment_variables""#)), ); } diff --git a/crates/icp-cli/tests/canister_start_tests.rs b/crates/icp-cli/tests/canister_start_tests.rs index 96fd707a..00f7d6d8 100644 --- a/crates/icp-cli/tests/canister_start_tests.rs +++ b/crates/icp-cli/tests/canister_start_tests.rs @@ -80,7 +80,7 @@ async fn canister_start() { .stdout( starts_with("Canister Id:") .and(contains("Status: Stopped")) - .and(contains("Controllers: 2vxsx-fae")), + .and(contains("controller: 2vxsx-fae")), ); // Start canister @@ -111,7 +111,7 @@ async fn canister_start() { .stdout( starts_with("Canister Id:") .and(contains("Status: Running")) - .and(contains("Controllers: 2vxsx-fae")), + .and(contains("controller: 2vxsx-fae")), ); } diff --git a/crates/icp-cli/tests/canister_status_tests.rs b/crates/icp-cli/tests/canister_status_tests.rs index e6f65f7f..fe6098da 100644 --- a/crates/icp-cli/tests/canister_status_tests.rs +++ b/crates/icp-cli/tests/canister_status_tests.rs @@ -67,7 +67,7 @@ async fn canister_status() { .stdout( starts_with("Canister Id:") .and(contains("Status: Running")) - .and(contains("Controllers: 2vxsx-fae")), + .and(contains("controller: 2vxsx-fae")), ); } @@ -203,7 +203,7 @@ async fn canister_status_falls_back_when_access_is_denied() { .assert() .success() .stdout( - contains(format!("Controllers: {principal_alice}")) + contains(format!("controller: {principal_alice}")) .and(contains("Module hash:")) .and(contains("Status:").not()), ); diff --git a/crates/icp-cli/tests/canister_stop_tests.rs b/crates/icp-cli/tests/canister_stop_tests.rs index 4696eec2..f81168da 100644 --- a/crates/icp-cli/tests/canister_stop_tests.rs +++ b/crates/icp-cli/tests/canister_stop_tests.rs @@ -79,7 +79,7 @@ async fn canister_stop() { .stdout( starts_with("Canister Id:") .and(contains("Status: Stopped")) - .and(contains("Controllers: 2vxsx-fae")), + .and(contains("controller: 2vxsx-fae")), ); } diff --git a/crates/icp/src/canister/mod.rs b/crates/icp/src/canister/mod.rs index e5277333..065f7e0f 100644 --- a/crates/icp/src/canister/mod.rs +++ b/crates/icp/src/canister/mod.rs @@ -1,7 +1,7 @@ use std::collections::HashMap; -use candid::{Nat, Principal}; -use ic_management_canister_types::{CanisterSettings, LogVisibility}; +use candid::Nat; +use ic_management_canister_types::CanisterSettings; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -13,136 +13,12 @@ use crate::{ pub mod build; pub mod recipe; pub mod sync; +pub mod visibility; mod script; pub mod wasm; -/// Controls who can read canister logs. -/// Supports both string format ("controllers", "public") and object format ({ allowed_viewers: [...] }). -#[derive(Clone, Debug, PartialEq, Serialize)] -#[serde(untagged)] -pub enum LogVisibilityDef { - /// Simple string variants for controllers or public - Simple(LogVisibilitySimple), - /// Object format with allowed_viewers list - AllowedViewers { allowed_viewers: Vec }, -} - -#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] -#[serde(rename_all = "snake_case")] -pub enum LogVisibilitySimple { - Controllers, - Public, -} - -impl<'de> Deserialize<'de> for LogVisibilityDef { - fn deserialize(deserializer: D) -> Result - where - D: serde::Deserializer<'de>, - { - use serde::de::{Error, MapAccess, Visitor}; - use std::fmt; - - struct LogVisibilityVisitor; - - impl<'de> Visitor<'de> for LogVisibilityVisitor { - type Value = LogVisibilityDef; - - fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - formatter.write_str("'controllers', 'public', or object with 'allowed_viewers'") - } - - fn visit_str(self, value: &str) -> Result { - LogVisibilitySimple::deserialize( - serde::de::value::StrDeserializer::::new(value), - ) - .map(LogVisibilityDef::Simple) - .map_err(|_| { - E::custom(format!( - "unknown log_visibility value: '{}', expected 'controllers' or 'public'", - value - )) - }) - } - - fn visit_map(self, mut map: M) -> Result - where - M: MapAccess<'de>, - { - let mut allowed_viewers: Option> = None; - - while let Some(key) = map.next_key::()? { - match key.as_str() { - "allowed_viewers" => { - if allowed_viewers.is_some() { - return Err(Error::duplicate_field("allowed_viewers")); - } - allowed_viewers = Some(map.next_value()?); - } - _ => { - return Err(Error::unknown_field(&key, &["allowed_viewers"])); - } - } - } - - allowed_viewers - .map(|v| LogVisibilityDef::AllowedViewers { allowed_viewers: v }) - .ok_or_else(|| Error::missing_field("allowed_viewers")) - } - } - - deserializer.deserialize_any(LogVisibilityVisitor) - } -} - -impl JsonSchema for LogVisibilityDef { - fn schema_name() -> std::borrow::Cow<'static, str> { - std::borrow::Cow::Borrowed("LogVisibility") - } - - fn json_schema(_generator: &mut schemars::SchemaGenerator) -> schemars::Schema { - schemars::json_schema!({ - "description": "Controls who can read canister logs.", - "oneOf": [ - { - "type": "string", - "enum": ["controllers", "public"], - "description": "Simple log visibility: 'controllers' (only controllers can view) or 'public' (anyone can view)" - }, - { - "type": "object", - "properties": { - "allowed_viewers": { - "type": "array", - "items": { - "type": "string", - "description": "A principal ID that can view logs" - }, - "description": "List of principal IDs that can view canister logs" - } - }, - "required": ["allowed_viewers"], - "additionalProperties": false, - "description": "Specific principals that can view logs" - } - ] - }) - } -} - -impl From for LogVisibility { - fn from(value: LogVisibilityDef) -> Self { - match value { - LogVisibilityDef::Simple(LogVisibilitySimple::Controllers) => { - LogVisibility::Controllers - } - LogVisibilityDef::Simple(LogVisibilitySimple::Public) => LogVisibility::Public, - LogVisibilityDef::AllowedViewers { allowed_viewers } => { - LogVisibility::AllowedViewers(allowed_viewers) - } - } - } -} +pub use visibility::{LogVisibilityDef, StatusVisibilityDef, Visibility}; /// A reference to a controller: either an explicit principal or a canister name in this project. /// @@ -256,6 +132,10 @@ pub struct Settings { #[serde(skip_serializing_if = "Option::is_none")] pub log_visibility: Option, + /// Controls who can read the canister's status. + #[serde(skip_serializing_if = "Option::is_none")] + pub status_visibility: Option, + /// Compute allocation (0 to 100). Represents guaranteed compute capacity. #[serde(skip_serializing_if = "Option::is_none")] pub compute_allocation: Option, @@ -309,6 +189,7 @@ impl From for ManifestSettings { fn from(settings: Settings) -> Self { let Settings { log_visibility, + status_visibility, compute_allocation, memory_allocation, freezing_threshold, @@ -322,6 +203,7 @@ impl From for ManifestSettings { Self { log_visibility, + status_visibility, compute_allocation, memory_allocation, freezing_threshold, @@ -345,7 +227,8 @@ impl From for CanisterSettings { freezing_threshold: settings.freezing_threshold.map(|d| Nat::from(d.get())), controllers: None, reserved_cycles_limit: settings.reserved_cycles_limit.map(|c| Nat::from(c.get())), - log_visibility: settings.log_visibility.map(Into::into), + log_visibility: settings.log_visibility.map(|v| v.0.into()), + status_visibility: settings.status_visibility.map(|v| v.0.into()), memory_allocation: settings.memory_allocation.map(|m| Nat::from(m.get())), compute_allocation: settings.compute_allocation.map(Nat::from), ..Default::default() @@ -355,112 +238,11 @@ impl From for CanisterSettings { #[cfg(test)] mod tests { + use candid::Principal; use indoc::indoc; use super::*; - #[test] - fn log_visibility_deserialize_controllers() { - let yaml = "controllers"; - let result: LogVisibilityDef = serde_yaml::from_str(yaml).unwrap(); - assert_eq!( - result, - LogVisibilityDef::Simple(LogVisibilitySimple::Controllers) - ); - } - - #[test] - fn log_visibility_deserialize_public() { - let yaml = "public"; - let result: LogVisibilityDef = serde_yaml::from_str(yaml).unwrap(); - assert_eq!( - result, - LogVisibilityDef::Simple(LogVisibilitySimple::Public) - ); - } - - #[test] - fn log_visibility_deserialize_allowed_viewers() { - let yaml = r#" -allowed_viewers: - - "aaaaa-aa" - - "2vxsx-fae" -"#; - let result: LogVisibilityDef = serde_yaml::from_str(yaml).unwrap(); - match result { - LogVisibilityDef::AllowedViewers { allowed_viewers } => { - assert_eq!(allowed_viewers.len(), 2); - assert_eq!( - allowed_viewers[0], - Principal::from_text("aaaaa-aa").unwrap() - ); - assert_eq!( - allowed_viewers[1], - Principal::from_text("2vxsx-fae").unwrap() - ); - } - _ => panic!("Expected AllowedViewers variant"), - } - } - - #[test] - fn log_visibility_deserialize_allowed_viewers_empty() { - let yaml = "allowed_viewers: []"; - let result: LogVisibilityDef = serde_yaml::from_str(yaml).unwrap(); - match result { - LogVisibilityDef::AllowedViewers { allowed_viewers } => { - assert!(allowed_viewers.is_empty()); - } - _ => panic!("Expected AllowedViewers variant"), - } - } - - #[test] - fn log_visibility_deserialize_invalid_string() { - let yaml = "invalid"; - let result: Result = serde_yaml::from_str(yaml); - assert!(result.is_err()); - let err = result.unwrap_err().to_string(); - assert!(err.contains("unknown log_visibility value")); - } - - #[test] - fn log_visibility_deserialize_invalid_field() { - let yaml = "unknown_field: []"; - let result: Result = serde_yaml::from_str(yaml); - assert!(result.is_err()); - let err = result.unwrap_err().to_string(); - assert!(err.contains("unknown field")); - } - - #[test] - fn log_visibility_serialize_controllers() { - let log_vis = LogVisibilityDef::Simple(LogVisibilitySimple::Controllers); - let yaml = serde_yaml::to_string(&log_vis).unwrap(); - assert_eq!(yaml.trim(), "controllers"); - } - - #[test] - fn log_visibility_serialize_public() { - let log_vis = LogVisibilityDef::Simple(LogVisibilitySimple::Public); - let yaml = serde_yaml::to_string(&log_vis).unwrap(); - assert_eq!(yaml.trim(), "public"); - } - - #[test] - fn log_visibility_serialize_allowed_viewers() { - let log_vis = LogVisibilityDef::AllowedViewers { - allowed_viewers: vec![ - Principal::from_text("aaaaa-aa").unwrap(), - Principal::from_text("2vxsx-fae").unwrap(), - ], - }; - let yaml = serde_yaml::to_string(&log_vis).unwrap(); - assert!(yaml.contains("allowed_viewers")); - assert!(yaml.contains("aaaaa-aa")); - assert!(yaml.contains("2vxsx-fae")); - } - #[test] fn settings_reserved_cycles_limit_parses_suffix() { let yaml = "reserved_cycles_limit: 4.3t"; @@ -664,26 +446,4 @@ controllers: ControllerRef::CanisterName("my_other_canister".to_owned()) ); } - - #[test] - fn log_visibility_conversion_to_ic_type() { - let controllers = LogVisibilityDef::Simple(LogVisibilitySimple::Controllers); - let ic_controllers: LogVisibility = controllers.into(); - assert!(matches!(ic_controllers, LogVisibility::Controllers)); - - let public = LogVisibilityDef::Simple(LogVisibilitySimple::Public); - let ic_public: LogVisibility = public.into(); - assert!(matches!(ic_public, LogVisibility::Public)); - - let viewers = LogVisibilityDef::AllowedViewers { - allowed_viewers: vec![Principal::from_text("aaaaa-aa").unwrap()], - }; - let ic_viewers: LogVisibility = viewers.into(); - match ic_viewers { - LogVisibility::AllowedViewers(v) => { - assert_eq!(v.len(), 1); - } - _ => panic!("Expected AllowedViewers"), - } - } } diff --git a/crates/icp/src/canister/visibility.rs b/crates/icp/src/canister/visibility.rs new file mode 100644 index 00000000..bc756c8c --- /dev/null +++ b/crates/icp/src/canister/visibility.rs @@ -0,0 +1,293 @@ +use std::fmt; + +use candid::Principal; +use ic_management_canister_types::{LogVisibility, StatusVisibility}; +use serde::{Deserialize, Serialize, Serializer, de}; + +/// Who may read a visibility-gated part of a canister. +/// +/// The management canister exposes several structurally identical visibility +/// settings, each with its own Candid type. This is the single form the CLI +/// parses, compares, and renders; conversions to and from the Candid types are +/// generated below. +/// +/// The variants say who a setting grants access to, not the whole access rule: +/// a setting may admit callers of its own on top, as `status_visibility` does +/// for subnet administrators and for a canister reading its own status. Each +/// manifest newtype documents its own rule. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum Visibility { + /// The canister's controllers. + Controllers, + /// Anyone. + Public, + /// The canister's controllers plus the listed principals. + AllowedViewers(Vec), +} + +impl Visibility { + /// Deserializes the manifest form: either `controllers` / `public`, or a + /// `{ allowed_viewers: [...] }` mapping. `setting` names the manifest field + /// so a bad value points at the setting that carried it. + fn deserialize_setting<'de, D>(deserializer: D, setting: &'static str) -> Result + where + D: serde::Deserializer<'de>, + { + struct VisibilityVisitor(&'static str); + + impl<'de> de::Visitor<'de> for VisibilityVisitor { + type Value = Visibility; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str("'controllers', 'public', or object with 'allowed_viewers'") + } + + fn visit_str(self, value: &str) -> Result { + match value { + "controllers" => Ok(Visibility::Controllers), + "public" => Ok(Visibility::Public), + _ => Err(E::custom(format!( + "unknown {} value: '{}', expected 'controllers' or 'public'", + self.0, value + ))), + } + } + + fn visit_map(self, mut map: M) -> Result + where + M: de::MapAccess<'de>, + { + let mut allowed_viewers: Option> = None; + + while let Some(key) = map.next_key::()? { + match key.as_str() { + "allowed_viewers" => { + if allowed_viewers.is_some() { + return Err(de::Error::duplicate_field("allowed_viewers")); + } + allowed_viewers = Some(map.next_value()?); + } + _ => return Err(de::Error::unknown_field(&key, &["allowed_viewers"])), + } + } + + allowed_viewers + .map(Visibility::AllowedViewers) + .ok_or_else(|| de::Error::missing_field("allowed_viewers")) + } + } + + deserializer.deserialize_any(VisibilityVisitor(setting)) + } +} + +impl Serialize for Visibility { + fn serialize(&self, serializer: S) -> Result { + match self { + Visibility::Controllers => serializer.serialize_str("controllers"), + Visibility::Public => serializer.serialize_str("public"), + Visibility::AllowedViewers(allowed_viewers) => { + use serde::ser::SerializeMap; + let mut map = serializer.serialize_map(Some(1))?; + map.serialize_entry("allowed_viewers", allowed_viewers)?; + map.end() + } + } + } +} + +macro_rules! candid_conversions { + ($candid:ident) => { + impl From for $candid { + fn from(value: Visibility) -> Self { + match value { + Visibility::Controllers => Self::Controllers, + Visibility::Public => Self::Public, + Visibility::AllowedViewers(viewers) => Self::AllowedViewers(viewers), + } + } + } + + impl From<$candid> for Visibility { + fn from(value: $candid) -> Self { + match value { + $candid::Controllers => Self::Controllers, + $candid::Public => Self::Public, + $candid::AllowedViewers(viewers) => Self::AllowedViewers(viewers), + } + } + } + }; +} + +candid_conversions!(LogVisibility); +candid_conversions!(StatusVisibility); + +fn visibility_schema(description: &str, subject: &str, controllers: &str) -> schemars::Schema { + schemars::json_schema!({ + "description": description, + "oneOf": [ + { + "type": "string", + "enum": ["controllers", "public"], + "description": format!("'controllers' ({controllers}) or 'public' (anyone can {subject})"), + }, + { + "type": "object", + "properties": { + "allowed_viewers": { + "type": "array", + "items": { + "type": "string", + "description": "A principal ID", + }, + "description": format!("Principal IDs that can {subject}, in addition to the controllers"), + } + }, + "required": ["allowed_viewers"], + "additionalProperties": false, + "description": format!("Specific principals that can {subject}"), + } + ] + }) +} + +/// Declares a manifest-level visibility setting: a [`Visibility`] newtype that +/// names itself in parse errors and in the generated JSON schema. +macro_rules! visibility_setting { + ( + $name:ident, + setting = $setting:literal, + schema = $schema:literal, + description = $description:literal, + subject = $subject:literal, + controllers = $controllers:literal $(,)? + ) => { + #[doc = $description] + #[derive(Clone, Debug, PartialEq, Eq, Serialize)] + #[serde(transparent)] + pub struct $name(pub Visibility); + + impl<'de> Deserialize<'de> for $name { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + Visibility::deserialize_setting(deserializer, $setting).map(Self) + } + } + + impl schemars::JsonSchema for $name { + fn schema_name() -> std::borrow::Cow<'static, str> { + std::borrow::Cow::Borrowed($schema) + } + + fn json_schema(_generator: &mut schemars::SchemaGenerator) -> schemars::Schema { + visibility_schema($description, $subject, $controllers) + } + } + + impl From<$name> for Visibility { + fn from(value: $name) -> Self { + value.0 + } + } + }; +} + +visibility_setting!( + LogVisibilityDef, + setting = "log_visibility", + schema = "LogVisibility", + description = "Controls who can read canister logs.", + subject = "read the logs", + controllers = "only the canister's controllers can read the logs", +); + +visibility_setting!( + StatusVisibilityDef, + setting = "status_visibility", + schema = "StatusVisibility", + description = "Controls who can read the canister's status.", + subject = "read the status", + controllers = "the canister's controllers, plus any subnet administrators and the canister itself, can read the status", +); + +#[cfg(test)] +mod tests { + use super::*; + + fn principal(text: &str) -> Principal { + Principal::from_text(text).unwrap() + } + + #[test] + fn deserialize_allowed_viewers() { + let yaml = r#" +allowed_viewers: + - "aaaaa-aa" + - "2vxsx-fae" +"#; + let parsed: LogVisibilityDef = serde_yaml::from_str(yaml).unwrap(); + assert_eq!( + parsed.0, + Visibility::AllowedViewers(vec![principal("aaaaa-aa"), principal("2vxsx-fae")]) + ); + + let parsed: LogVisibilityDef = serde_yaml::from_str("allowed_viewers: []").unwrap(); + assert_eq!(parsed.0, Visibility::AllowedViewers(vec![])); + } + + #[test] + fn deserialize_rejects_bad_input_naming_the_setting() { + let err = serde_yaml::from_str::("invalid") + .unwrap_err() + .to_string(); + assert!(err.contains("unknown log_visibility value"), "{err}"); + + let err = serde_yaml::from_str::("invalid") + .unwrap_err() + .to_string(); + assert!(err.contains("unknown status_visibility value"), "{err}"); + + let err = serde_yaml::from_str::("unknown_field: []") + .unwrap_err() + .to_string(); + assert!(err.contains("unknown field"), "{err}"); + } + + #[test] + fn serialize_round_trips_the_manifest_form() { + for value in [ + Visibility::Controllers, + Visibility::Public, + Visibility::AllowedViewers(vec![principal("aaaaa-aa"), principal("2vxsx-fae")]), + ] { + let yaml = serde_yaml::to_string(&StatusVisibilityDef(value.clone())).unwrap(); + assert_eq!( + serde_yaml::from_str::(&yaml) + .unwrap() + .0, + value + ); + } + + let yaml = serde_yaml::to_string(&LogVisibilityDef(Visibility::Controllers)).unwrap(); + assert_eq!(yaml.trim(), "controllers"); + } + + #[test] + fn converts_to_and_from_candid_types() { + for value in [ + Visibility::Controllers, + Visibility::Public, + Visibility::AllowedViewers(vec![principal("aaaaa-aa")]), + ] { + let log: LogVisibility = value.clone().into(); + assert_eq!(Visibility::from(log), value); + + let status: StatusVisibility = value.clone().into(); + assert_eq!(Visibility::from(status), value); + } + } +} diff --git a/crates/icp/src/operations/settings.rs b/crates/icp/src/operations/settings.rs index 8105e786..6dc690fb 100644 --- a/crates/icp/src/operations/settings.rs +++ b/crates/icp/src/operations/settings.rs @@ -5,7 +5,7 @@ use std::{ use crate::{ Canister, - canister::{Settings, resolve_controllers}, + canister::{Settings, Visibility, resolve_controllers}, context::{Context, EnvironmentSelection}, store_id::IdMapping, }; @@ -13,7 +13,7 @@ use candid::{Nat, Principal}; use futures::{StreamExt, stream::FuturesOrdered}; use ic_agent::Agent; use ic_management_canister_types::{ - CanisterIdRecord, CanisterSettings, EnvironmentVariable, LogVisibility, UpdateSettingsArgs, + CanisterIdRecord, CanisterSettings, EnvironmentVariable, UpdateSettingsArgs, }; use icp_events::TaskOutcome; @@ -47,13 +47,13 @@ pub struct SyncSettingsManyError { names: Vec, } -/// Compare two LogVisibility values in an order-insensitive manner. -/// For AllowedViewers, the principal lists are compared as sets. -fn log_visibility_eq(a: &LogVisibility, b: &LogVisibility) -> bool { +/// Compare two visibility settings, treating the allowed-viewers list as a set +/// so a reordering from the replica does not look like a pending change. +fn visibility_eq(a: &Visibility, b: &Visibility) -> bool { match (a, b) { - (LogVisibility::Controllers, LogVisibility::Controllers) => true, - (LogVisibility::Public, LogVisibility::Public) => true, - (LogVisibility::AllowedViewers(va), LogVisibility::AllowedViewers(vb)) => { + (Visibility::Controllers, Visibility::Controllers) => true, + (Visibility::Public, Visibility::Public) => true, + (Visibility::AllowedViewers(va), Visibility::AllowedViewers(vb)) => { let set_a: HashSet<_> = va.iter().collect(); let set_b: HashSet<_> = vb.iter().collect(); set_a == set_b @@ -86,6 +86,7 @@ pub async fn sync_settings( .context(FetchCurrentSettingsSnafu { canister: *cid })?; let &Settings { ref log_visibility, + ref status_visibility, compute_allocation, ref memory_allocation, ref freezing_threshold, @@ -98,9 +99,10 @@ pub async fn sync_settings( } = &canister.settings; let current_settings = status.settings; - // Convert our log_visibility to IC type for comparison and update - let log_visibility_setting: Option = - log_visibility.clone().map(LogVisibility::from); + let desired_log_visibility = log_visibility.clone().map(Visibility::from); + let desired_status_visibility = status_visibility.clone().map(Visibility::from); + let current_log_visibility = Visibility::from(current_settings.log_visibility.clone()); + let current_status_visibility = Visibility::from(current_settings.status_visibility.clone()); let environment_variable_setting = if let Some(configured_environment_variables) = &environment_variables { @@ -147,9 +149,12 @@ pub async fn sync_settings( desired_sorted != current_sorted }); - if log_visibility_setting + if desired_log_visibility .as_ref() - .is_none_or(|s| log_visibility_eq(s, ¤t_settings.log_visibility)) + .is_none_or(|s| visibility_eq(s, ¤t_log_visibility)) + && desired_status_visibility + .as_ref() + .is_none_or(|s| visibility_eq(s, ¤t_status_visibility)) && compute_allocation.is_none_or(|s| s == current_settings.compute_allocation) && memory_allocation .as_ref() @@ -184,7 +189,8 @@ pub async fn sync_settings( } let settings = CanisterSettings { - log_visibility: log_visibility_setting, + log_visibility: desired_log_visibility.map(Into::into), + status_visibility: desired_status_visibility.map(Into::into), compute_allocation: compute_allocation.map(Nat::from), memory_allocation: memory_allocation.as_ref().map(|m| Nat::from(m.get())), freezing_threshold: freezing_threshold.as_ref().map(|d| Nat::from(d.get())), @@ -194,10 +200,9 @@ pub async fn sync_settings( log_memory_limit: log_memory_limit.as_ref().map(|m| Nat::from(m.get())), environment_variables: environment_variable_setting, controllers: controllers_setting, - // TODO: make snapshot_visibility configurable from the manifest and synced - // here, mirroring log_visibility (Controllers/Public/AllowedViewers). - // Tracked for a follow-up PR; until then, leave it unchanged. + // Not configurable from the manifest yet; `None` leaves them unchanged. snapshot_visibility: None, + minimum_incoming_canister_call_cycles: None, }; proxy_management::update_settings( @@ -333,90 +338,87 @@ mod tests { use super::*; #[test] - fn log_visibility_eq_controllers() { - assert!(log_visibility_eq( - &LogVisibility::Controllers, - &LogVisibility::Controllers + fn visibility_eq_controllers() { + assert!(visibility_eq( + &Visibility::Controllers, + &Visibility::Controllers )); } #[test] - fn log_visibility_eq_public() { - assert!(log_visibility_eq( - &LogVisibility::Public, - &LogVisibility::Public - )); + fn visibility_eq_public() { + assert!(visibility_eq(&Visibility::Public, &Visibility::Public)); } #[test] - fn log_visibility_eq_different_variants() { - assert!(!log_visibility_eq( - &LogVisibility::Controllers, - &LogVisibility::Public + fn visibility_eq_different_variants() { + assert!(!visibility_eq( + &Visibility::Controllers, + &Visibility::Public )); - assert!(!log_visibility_eq( - &LogVisibility::Public, - &LogVisibility::Controllers + assert!(!visibility_eq( + &Visibility::Public, + &Visibility::Controllers )); } #[test] - fn log_visibility_eq_allowed_viewers_same_order() { + fn visibility_eq_allowed_viewers_same_order() { let p1 = Principal::from_text("aaaaa-aa").unwrap(); let p2 = Principal::from_text("2vxsx-fae").unwrap(); - assert!(log_visibility_eq( - &LogVisibility::AllowedViewers(vec![p1, p2]), - &LogVisibility::AllowedViewers(vec![p1, p2]) + assert!(visibility_eq( + &Visibility::AllowedViewers(vec![p1, p2]), + &Visibility::AllowedViewers(vec![p1, p2]) )); } #[test] - fn log_visibility_eq_allowed_viewers_different_order() { + fn visibility_eq_allowed_viewers_different_order() { let p1 = Principal::from_text("aaaaa-aa").unwrap(); let p2 = Principal::from_text("2vxsx-fae").unwrap(); // Order should not matter - assert!(log_visibility_eq( - &LogVisibility::AllowedViewers(vec![p1, p2]), - &LogVisibility::AllowedViewers(vec![p2, p1]) + assert!(visibility_eq( + &Visibility::AllowedViewers(vec![p1, p2]), + &Visibility::AllowedViewers(vec![p2, p1]) )); } #[test] - fn log_visibility_eq_allowed_viewers_different_principals() { + fn visibility_eq_allowed_viewers_different_principals() { let p1 = Principal::from_text("aaaaa-aa").unwrap(); let p2 = Principal::from_text("2vxsx-fae").unwrap(); let p3 = Principal::from_text("ryjl3-tyaaa-aaaaa-aaaba-cai").unwrap(); - assert!(!log_visibility_eq( - &LogVisibility::AllowedViewers(vec![p1, p2]), - &LogVisibility::AllowedViewers(vec![p1, p3]) + assert!(!visibility_eq( + &Visibility::AllowedViewers(vec![p1, p2]), + &Visibility::AllowedViewers(vec![p1, p3]) )); } #[test] - fn log_visibility_eq_allowed_viewers_different_length() { + fn visibility_eq_allowed_viewers_different_length() { let p1 = Principal::from_text("aaaaa-aa").unwrap(); let p2 = Principal::from_text("2vxsx-fae").unwrap(); - assert!(!log_visibility_eq( - &LogVisibility::AllowedViewers(vec![p1]), - &LogVisibility::AllowedViewers(vec![p1, p2]) + assert!(!visibility_eq( + &Visibility::AllowedViewers(vec![p1]), + &Visibility::AllowedViewers(vec![p1, p2]) )); } #[test] - fn log_visibility_eq_allowed_viewers_vs_other() { + fn visibility_eq_allowed_viewers_vs_other() { let p1 = Principal::from_text("aaaaa-aa").unwrap(); - assert!(!log_visibility_eq( - &LogVisibility::AllowedViewers(vec![p1]), - &LogVisibility::Controllers + assert!(!visibility_eq( + &Visibility::AllowedViewers(vec![p1]), + &Visibility::Controllers )); - assert!(!log_visibility_eq( - &LogVisibility::AllowedViewers(vec![p1]), - &LogVisibility::Public + assert!(!visibility_eq( + &Visibility::AllowedViewers(vec![p1]), + &Visibility::Public )); } diff --git a/crates/icp/src/project.rs b/crates/icp/src/project.rs index 88adfcc3..b24dd0a4 100644 --- a/crates/icp/src/project.rs +++ b/crates/icp/src/project.rs @@ -241,6 +241,7 @@ fn resolve_manifest_settings( ) -> Result<(Settings, BTreeMap), ConsolidateManifestError> { let ManifestSettings { log_visibility, + status_visibility, compute_allocation, memory_allocation, freezing_threshold, @@ -280,6 +281,7 @@ fn resolve_manifest_settings( let settings = Settings { log_visibility: log_visibility.clone(), + status_visibility: status_visibility.clone(), compute_allocation: *compute_allocation, memory_allocation: memory_allocation.clone(), freezing_threshold: freezing_threshold.clone(), diff --git a/docs/reference/canister-settings.md b/docs/reference/canister-settings.md index 31f748cd..7e7cf387 100644 --- a/docs/reference/canister-settings.md +++ b/docs/reference/canister-settings.md @@ -159,6 +159,48 @@ settings: - "2vxsx-fae" ``` +### status_visibility + +Controls who can read the canister's status through the management canister's +`canister_status` endpoint — the report `icp canister status` prints, covering +the running state, cycles balance, memory usage, and the settings themselves. + +| Property | Value | +|----------|-------| +| Type | String or Object | +| Values | `controllers`, `public`, or `allowed_viewers` object | +| Default | `controllers` | + +```yaml +# The canister's controllers can read the status (default) +settings: + status_visibility: controllers + +# Anyone can read the status +settings: + status_visibility: public + +# Specific principals can read the status, in addition to the controllers +settings: + status_visibility: + allowed_viewers: + - "aaaaa-aa" + - "2vxsx-fae" +``` + +Two callers are always allowed, whatever the setting says: the administrators of +the subnet the canister runs on, and the canister itself reading its own status. +Unlike [`log_visibility`](#log_visibility), which grants access to the +controllers and listed viewers alone, `controllers` here is a floor rather than +an exhaustive list. + +A caller that is not allowed to read the status still sees the canister's +controllers and module hash, which the replica publishes in the state tree and +`icp canister status` falls back to. Granting status access does not grant any +control over the canister. + +The replica accepts at most 10 principals in `allowed_viewers`. + ### environment_variables Runtime environment variables accessible to the canister. @@ -227,6 +269,7 @@ canisters: wasm_memory_limit: 1gib wasm_memory_threshold: 512mib log_visibility: controllers + status_visibility: controllers log_memory_limit: 2mib environment_variables: ENV: "production" diff --git a/docs/reference/cli.md b/docs/reference/cli.md index c391b7ec..672927c0 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -481,7 +481,7 @@ Commands to manage canister settings Show the settings of a canister. -Queries the canister_status endpoint of the management canister and displays only the settings fields. Requires the caller to be a controller. +Queries the canister_status endpoint of the management canister and displays only the settings fields. Requires the caller to be allowed to read the canister's status, which by default means being one of its controllers. **Usage:** `icp canister settings show [OPTIONS] ` @@ -532,9 +532,21 @@ Change a canister's settings to specified values * `--wasm-memory-threshold ` — Wasm memory threshold in bytes. Supports suffixes: kb, kib, mb, mib, gb, gib (e.g. "4gib" or "2.5kb") * `--log-memory-limit ` — Log memory limit in bytes (max 2 MiB). Oldest logs are purged when usage exceeds this value. Supports suffixes: kb, kib, mb, mib (e.g. "2mib" or "256kib"). Canister default is 4096 bytes * `--log-visibility ` — Set log visibility to a fixed policy [possible values: controllers, public]. Conflicts with --add-log-viewer, --remove-log-viewer, and --set-log-viewer. Use --add-log-viewer / --set-log-viewer to grant access to specific principals instead -* `--add-log-viewer ` — Add a principal to the allowed log viewers list -* `--remove-log-viewer ` — Remove a principal from the allowed log viewers list +* `--add-log-viewer ` — Add a principal to the allowed log viewers list. + + Rejected while log visibility is public, which has no viewers list to add to; use --set-log-viewer to replace the public policy with a list. +* `--remove-log-viewer ` — Remove a principal from the allowed log viewers list. + + Rejected while log visibility is public, which has no viewers list to remove from; use --log-visibility controllers to revoke public access. * `--set-log-viewer ` — Replace the allowed log viewers list with the specified principals +* `--status-visibility ` — Set status visibility to a fixed policy [possible values: controllers, public]. Conflicts with --add-status-viewer, --remove-status-viewer, and --set-status-viewer. Use --add-status-viewer / --set-status-viewer to grant access to specific principals instead +* `--add-status-viewer ` — Add a principal to the allowed status viewers list. + + Rejected while status visibility is public, which has no viewers list to add to; use --set-status-viewer to replace the public policy with a list. +* `--remove-status-viewer ` — Remove a principal from the allowed status viewers list. + + Rejected while status visibility is public, which has no viewers list to remove from; use --status-visibility controllers to revoke public access. +* `--set-status-viewer ` — Replace the allowed status viewers list with the specified principals * `--add-environment-variable ` — Add a canister environment variable in KEY=VALUE format * `--remove-environment-variable ` — Remove a canister environment variable by key name * `--proxy ` — Principal of a proxy canister to route the management canister calls through diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 27733cba..ad75cf50 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -388,6 +388,7 @@ settings: wasm_memory_limit: 1gib wasm_memory_threshold: 512mib log_visibility: controllers + status_visibility: controllers environment_variables: KEY: "value" # A value may also be read from a file, relative to the canister directory diff --git a/docs/schemas/canister-yaml-schema.json b/docs/schemas/canister-yaml-schema.json index 593211c2..e05c42f1 100644 --- a/docs/schemas/canister-yaml-schema.json +++ b/docs/schemas/canister-yaml-schema.json @@ -213,7 +213,7 @@ "description": "Controls who can read canister logs.", "oneOf": [ { - "description": "Simple log visibility: 'controllers' (only controllers can view) or 'public' (anyone can view)", + "description": "'controllers' (only the canister's controllers can read the logs) or 'public' (anyone can read the logs)", "enum": [ "controllers", "public" @@ -222,12 +222,12 @@ }, { "additionalProperties": false, - "description": "Specific principals that can view logs", + "description": "Specific principals that can read the logs", "properties": { "allowed_viewers": { - "description": "List of principal IDs that can view canister logs", + "description": "Principal IDs that can read the logs, in addition to the controllers", "items": { - "description": "A principal ID that can view logs", + "description": "A principal ID", "type": "string" }, "type": "array" @@ -438,6 +438,17 @@ ], "description": "Upper limit on cycles reserved for future resource payments.\nMemory allocations that would push the reserved balance above this limit will fail.\nSupports suffixes in YAML: k, m, b, t (e.g. \"4t\" or \"4.3t\")." }, + "status_visibility": { + "anyOf": [ + { + "$ref": "#/$defs/StatusVisibility" + }, + { + "type": "null" + } + ], + "description": "Controls who can read the canister's status." + }, "wasm_memory_limit": { "anyOf": [ { @@ -463,6 +474,37 @@ }, "type": "object" }, + "StatusVisibility": { + "description": "Controls who can read the canister's status.", + "oneOf": [ + { + "description": "'controllers' (the canister's controllers, plus any subnet administrators and the canister itself, can read the status) or 'public' (anyone can read the status)", + "enum": [ + "controllers", + "public" + ], + "type": "string" + }, + { + "additionalProperties": false, + "description": "Specific principals that can read the status", + "properties": { + "allowed_viewers": { + "description": "Principal IDs that can read the status, in addition to the controllers", + "items": { + "description": "A principal ID", + "type": "string" + }, + "type": "array" + } + }, + "required": [ + "allowed_viewers" + ], + "type": "object" + } + ] + }, "SyncStep": { "description": "Identifies the type of adapter used to sync the canister,\nalong with its configuration.\n\nThe adapter type is specified via the `type` field in the YAML file.\nFor example:\n\n```yaml\ntype: script\ncommand: echo \"synchronizing canister\"\n```", "oneOf": [ diff --git a/docs/schemas/environment-yaml-schema.json b/docs/schemas/environment-yaml-schema.json index 2ae424b2..96c8ee04 100644 --- a/docs/schemas/environment-yaml-schema.json +++ b/docs/schemas/environment-yaml-schema.json @@ -54,7 +54,7 @@ "description": "Controls who can read canister logs.", "oneOf": [ { - "description": "Simple log visibility: 'controllers' (only controllers can view) or 'public' (anyone can view)", + "description": "'controllers' (only the canister's controllers can read the logs) or 'public' (anyone can read the logs)", "enum": [ "controllers", "public" @@ -63,12 +63,12 @@ }, { "additionalProperties": false, - "description": "Specific principals that can view logs", + "description": "Specific principals that can read the logs", "properties": { "allowed_viewers": { - "description": "List of principal IDs that can view canister logs", + "description": "Principal IDs that can read the logs, in addition to the controllers", "items": { - "description": "A principal ID that can view logs", + "description": "A principal ID", "type": "string" }, "type": "array" @@ -244,6 +244,17 @@ ], "description": "Upper limit on cycles reserved for future resource payments.\nMemory allocations that would push the reserved balance above this limit will fail.\nSupports suffixes in YAML: k, m, b, t (e.g. \"4t\" or \"4.3t\")." }, + "status_visibility": { + "anyOf": [ + { + "$ref": "#/$defs/StatusVisibility" + }, + { + "type": "null" + } + ], + "description": "Controls who can read the canister's status." + }, "wasm_memory_limit": { "anyOf": [ { @@ -268,6 +279,37 @@ } }, "type": "object" + }, + "StatusVisibility": { + "description": "Controls who can read the canister's status.", + "oneOf": [ + { + "description": "'controllers' (the canister's controllers, plus any subnet administrators and the canister itself, can read the status) or 'public' (anyone can read the status)", + "enum": [ + "controllers", + "public" + ], + "type": "string" + }, + { + "additionalProperties": false, + "description": "Specific principals that can read the status", + "properties": { + "allowed_viewers": { + "description": "Principal IDs that can read the status, in addition to the controllers", + "items": { + "description": "A principal ID", + "type": "string" + }, + "type": "array" + } + }, + "required": [ + "allowed_viewers" + ], + "type": "object" + } + ] } }, "$id": "EnvironmentManifest", diff --git a/docs/schemas/icp-yaml-schema.json b/docs/schemas/icp-yaml-schema.json index 05774f60..213804fe 100644 --- a/docs/schemas/icp-yaml-schema.json +++ b/docs/schemas/icp-yaml-schema.json @@ -493,7 +493,7 @@ "description": "Controls who can read canister logs.", "oneOf": [ { - "description": "Simple log visibility: 'controllers' (only controllers can view) or 'public' (anyone can view)", + "description": "'controllers' (only the canister's controllers can read the logs) or 'public' (anyone can read the logs)", "enum": [ "controllers", "public" @@ -502,12 +502,12 @@ }, { "additionalProperties": false, - "description": "Specific principals that can view logs", + "description": "Specific principals that can read the logs", "properties": { "allowed_viewers": { - "description": "List of principal IDs that can view canister logs", + "description": "Principal IDs that can read the logs, in addition to the controllers", "items": { - "description": "A principal ID that can view logs", + "description": "A principal ID", "type": "string" }, "type": "array" @@ -970,6 +970,17 @@ ], "description": "Upper limit on cycles reserved for future resource payments.\nMemory allocations that would push the reserved balance above this limit will fail.\nSupports suffixes in YAML: k, m, b, t (e.g. \"4t\" or \"4.3t\")." }, + "status_visibility": { + "anyOf": [ + { + "$ref": "#/$defs/StatusVisibility" + }, + { + "type": "null" + } + ], + "description": "Controls who can read the canister's status." + }, "wasm_memory_limit": { "anyOf": [ { @@ -995,6 +1006,37 @@ }, "type": "object" }, + "StatusVisibility": { + "description": "Controls who can read the canister's status.", + "oneOf": [ + { + "description": "'controllers' (the canister's controllers, plus any subnet administrators and the canister itself, can read the status) or 'public' (anyone can read the status)", + "enum": [ + "controllers", + "public" + ], + "type": "string" + }, + { + "additionalProperties": false, + "description": "Specific principals that can read the status", + "properties": { + "allowed_viewers": { + "description": "Principal IDs that can read the status, in addition to the controllers", + "items": { + "description": "A principal ID", + "type": "string" + }, + "type": "array" + } + }, + "required": [ + "allowed_viewers" + ], + "type": "object" + } + ] + }, "SubnetKind": { "enum": [ "application",