Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ bump. Currently experimental: project bundling
# Unreleased

* feat: `icp canister delete` will now send the canister's remaining cycles to the caller
* feat: The root key of a `connected` network is now optional. When you connect to a local (loopback) network by URL, the root key is fetched from the network automatically, so `--network <URL>` no longer requires `--root-key`. A remote network must still provide its root key (via the manifest `root-key` or `--root-key`); previously an omitted key silently defaulted to the mainnet key, which now surfaces a clear error instead. The built-in `ic` network is unchanged.

# v1.0.2

Expand Down
21 changes: 9 additions & 12 deletions crates/icp-cli/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,9 @@ pub(crate) struct NetworkOptInner {
#[arg(long, short = 'n', env = "ICP_NETWORK", group = "network-select", help_heading = heading::NETWORK_PARAMETERS, value_parser = parse_network_target)]
network: Option<NetworkTarget>,

/// The root key to use if connecting to a network by URL.
/// Required when using `--network <URL>`.
/// The root key to use when connecting to a network by URL.
/// Optional: for a local (loopback) URL it is fetched from the network;
/// for a remote URL it is required.
#[arg(long, short = 'k', requires = "network", help_heading = heading::NETWORK_PARAMETERS, value_parser = parse_root_key)]
root_key: Option<RootKey>,
}
Expand All @@ -109,7 +110,7 @@ pub(crate) struct NetworkOptInner {
// validation to only allow --root-key when the network is a url.
#[derive(Clone, Debug, Default)]
pub(crate) enum NetworkOpt {
Url(Url, RootKey),
Url(Url, Option<RootKey>),

Name(String),

Expand All @@ -122,14 +123,10 @@ impl FromArgMatches for NetworkOpt {
let inner = NetworkOptInner::from_arg_matches(matches)?;

match (inner.network, inner.root_key) {
// Case: We have a URL, so we REQUIRE the root key
(Some(NetworkTarget::Url(url)), Some(key)) => Ok(NetworkOpt::Url(url, key)),

// ERROR Case: URL provided but missing root key
(Some(NetworkTarget::Url(_)), None) => Err(clap::Error::raw(
ErrorKind::MissingRequiredArgument,
"`--root-key` is required when `--network` is a URL.\n",
)),
// Case: URL, with or without a root key. A missing key is resolved
// when the network is accessed (fetched for a local network, error
// for remote).
(Some(NetworkTarget::Url(url)), key) => Ok(NetworkOpt::Url(url, key)),

// Case: Named network (root key should be empty)
(Some(NetworkTarget::Named(name)), None) => Ok(NetworkOpt::Name(name)),
Expand Down Expand Up @@ -169,7 +166,7 @@ impl Args for NetworkOpt {
impl From<NetworkOpt> for NetworkSelection {
fn from(v: NetworkOpt) -> Self {
match v {
NetworkOpt::Url(url, RootKey(key)) => NetworkSelection::Url(url, key),
NetworkOpt::Url(url, key) => NetworkSelection::Url(url, key.map(|RootKey(k)| k)),
NetworkOpt::Name(name) => NetworkSelection::Named(name),
NetworkOpt::None => NetworkSelection::Default,
}
Expand Down
41 changes: 41 additions & 0 deletions crates/icp-cli/tests/canister_call_root_key_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,22 @@ async fn canister_call_with_url_and_root_key() {
.success()
.stdout(eq("(\"Hello, world!\")").trim());

// Test calling with url from external directory WITHOUT a root key: the
// network is local (loopback), so the root key is fetched automatically.
ctx.icp()
.args([
"canister",
"call",
"--network",
gateway_url,
canister_id,
"greet",
"(\"world\")",
])
.assert()
.success()
.stdout(eq("(\"Hello, world!\")").trim());

// Test calling with with url from external directory with bad root key
ctx.icp()
.args([
Expand All @@ -137,3 +153,28 @@ async fn canister_call_with_url_and_root_key() {
.failure()
.stderr(contains("invalid value 'badbadbad' for '--root-key"));
}

/// Connecting to a *remote* URL without a root key must fail fast with a clear
/// message, rather than silently defaulting to the mainnet key and failing
/// later with a cryptic certificate-verification error. The loopback check
/// short-circuits before any network request, so this needs no live server.
#[tokio::test]
async fn canister_call_remote_url_without_root_key_errors() {
let ctx = TestContext::new();

ctx.icp()
.args([
"canister",
"call",
"--network",
"https://example.com",
"aaaaa-aa",
"greet",
"(\"world\")",
])
.assert()
.failure()
.stderr(contains(
"a root key is required to connect to remote network",
));
}
33 changes: 32 additions & 1 deletion crates/icp-cli/tests/network_status_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,14 +127,17 @@ fn status_connected_network() {
let ctx = TestContext::new();
let project_dir = ctx.create_project_dir("icp");

// Project manifest with connected network
// Project manifest with a connected network. A remote network must specify
// its root key explicitly (only a local/loopback network has its key
// fetched automatically).
write_string(
&project_dir.join("icp.yaml"),
r#"
networks:
- name: connected-network
mode: connected
url: https://ic0.app
root-key: 308182301d060d2b0601040182dc7c0503010201060c2b0601040182dc7c05030201036100814c0e6ec71fab583b08bd81373c255c3c371b2e84863c98a4f1e08b74235d14fb5d9c0cd546d9685f913a0c0b2cc5341583bf4b4392e467db96d65b9bb4cb717112f8472e0d5a4d14505ffd7484b01291091c5f87b98883463f98091a0baaae
"#,
)
.expect("failed to write project manifest");
Expand All @@ -147,6 +150,34 @@ networks:
.stdout(contains("Url: https://ic0.app"));
}

/// A connected network at a remote URL must specify its root key; accessing one
/// without a key fails fast rather than silently assuming the mainnet key.
#[test]
fn status_connected_network_remote_without_root_key_errors() {
let ctx = TestContext::new();
let project_dir = ctx.create_project_dir("icp");

write_string(
&project_dir.join("icp.yaml"),
r#"
networks:
- name: connected-network
mode: connected
url: https://ic0.app
"#,
)
.expect("failed to write project manifest");

ctx.icp()
.current_dir(&project_dir)
.args(["network", "status", "connected-network"])
.assert()
.failure()
.stderr(contains(
"a root key is required to connect to remote network",
));
}

#[test]
fn status_not_in_project() {
let ctx = TestContext::new();
Expand Down
9 changes: 5 additions & 4 deletions crates/icp/src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@ pub enum NetworkSelection {
Default,
/// Use a named network
Named(String),
/// Use a network by URL
Url(Url, Vec<u8>),
/// Use a network by URL. The root key is optional: `None` is resolved when
/// the network is accessed (fetched for a local network, error for remote).
Url(Url, Option<Vec<u8>>),
}

/// Selection type for environments - similar to IdentitySelection
Expand Down Expand Up @@ -180,7 +181,7 @@ impl Context {
http_gateway_url: Some(
IC_MAINNET_NETWORK_GATEWAY_URL.parse().unwrap(),
),
root_key: IC_ROOT_KEY.to_vec(),
root_key: Some(IC_ROOT_KEY.to_vec()),
},
},
}
Expand All @@ -197,7 +198,7 @@ impl Context {
connected: crate::network::Connected {
api_url: url.clone(),
http_gateway_url: Some(url.clone()),
root_key: root_key.to_vec(),
root_key: root_key.clone(),
},
},
},
Expand Down
2 changes: 1 addition & 1 deletion crates/icp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ impl MockProjectLoader {
connected: Connected {
api_url: IC_MAINNET_NETWORK_API_URL.parse().unwrap(),
http_gateway_url: Some(IC_MAINNET_NETWORK_GATEWAY_URL.parse().unwrap()),
root_key: IC_ROOT_KEY.to_vec(),
root_key: Some(IC_ROOT_KEY.to_vec()),
},
},
};
Expand Down
62 changes: 61 additions & 1 deletion crates/icp/src/network/access.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,25 @@ pub enum GetNetworkAccessError {

#[snafu(display("failed to load network descriptor"))]
LoadNetworkDescriptor { source: LoadNetworkFileError },

#[snafu(display(
"a root key is required to connect to remote network `{url}`; pass `--root-key` (or the manifest `root-key`), or use `--network ic` for mainnet"
))]
RootKeyRequiredForRemote { url: Url },

#[snafu(display("failed to build agent to fetch the root key from local network `{url}`"))]
BuildRootKeyAgent {
url: Url,
#[snafu(source(from(ic_agent::AgentError, Box::new)))]
source: Box<ic_agent::AgentError>,
},

#[snafu(display("failed to fetch the root key from local network `{url}`"))]
FetchRootKey {
url: Url,
#[snafu(source(from(ic_agent::AgentError, Box::new)))]
source: Box<ic_agent::AgentError>,
},
}

pub async fn get_managed_network_access(
Expand Down Expand Up @@ -90,7 +109,10 @@ pub async fn get_managed_network_access(
pub async fn get_connected_network_access(
connected: &Connected,
) -> Result<NetworkAccess, GetNetworkAccessError> {
let root_key = connected.root_key.clone();
let root_key = match &connected.root_key {
Some(rk) => rk.clone(),
None => resolve_root_key(&connected.api_url).await?,
};

Ok(NetworkAccess {
root_key,
Expand All @@ -99,3 +121,41 @@ pub async fn get_connected_network_access(
use_friendly_domains: false,
})
}

/// Whether `url`'s host is a loopback address (or `localhost`).
fn is_loopback(url: &Url) -> bool {
match url.host() {
Some(url::Host::Domain(d)) => d.eq_ignore_ascii_case("localhost"),
Some(url::Host::Ipv4(ip)) => ip.is_loopback(),
Some(url::Host::Ipv6(ip)) => ip.is_loopback(),
None => false,
}
}

/// Resolve the root key for a connected network whose key was omitted.
///
/// For a local (loopback) network the key is fetched from the network's status
/// endpoint — safe because there is no meaningful man-in-the-middle surface on
/// loopback. For a remote network we refuse rather than silently defaulting to
/// the mainnet key: a genuine remote network never shares mainnet's key, so
/// defaulting would only defer the failure to the first certified call with a
/// cryptic verification error.
async fn resolve_root_key(api_url: &Url) -> Result<Vec<u8>, GetNetworkAccessError> {
if !is_loopback(api_url) {
return RootKeyRequiredForRemoteSnafu {
url: api_url.clone(),
}
.fail();
}

let agent = ic_agent::Agent::builder()
.with_url(api_url.as_str())
.build()
.context(BuildRootKeyAgentSnafu {
url: api_url.clone(),
})?;
agent.fetch_root_key().await.context(FetchRootKeySnafu {
url: api_url.clone(),
})?;
Ok(agent.read_root_key())
}
35 changes: 28 additions & 7 deletions crates/icp/src/network/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,25 @@ pub struct ManagedImageConfig {
pub extra_hosts: Vec<String>,
}

/// Serde helper for an optional hex-encoded byte string (`hex::serde` does not
/// cover `Option`).
mod hex_opt {
use serde::{Deserialize, Deserializer, Serializer};

pub fn serialize<S: Serializer>(v: &Option<Vec<u8>>, s: S) -> Result<S::Ok, S::Error> {
match v {
Some(bytes) => s.serialize_some(&hex::encode(bytes)),
None => s.serialize_none(),
}
}

pub fn deserialize<'de, D: Deserializer<'de>>(d: D) -> Result<Option<Vec<u8>>, D::Error> {
Option::<String>::deserialize(d)?
.map(|s| hex::decode(s).map_err(serde::de::Error::custom))
.transpose()
}
}

#[derive(Clone, Debug, PartialEq, JsonSchema, Deserialize, Serialize)]
#[serde(rename_all = "kebab-case")]
pub struct Connected {
Expand All @@ -172,10 +191,12 @@ pub struct Connected {
/// The URL this network's HTTP gateway can be reached at.
pub http_gateway_url: Option<Url>,

/// The root key of this network
#[serde(with = "hex::serde")]
#[schemars(with = "String")]
pub root_key: Vec<u8>,
/// The root key of this network. `None` means the key is resolved when the
/// network is accessed: fetched from a local (loopback) network, or an
/// error for a remote network.
#[serde(with = "hex_opt", default, skip_serializing_if = "Option::is_none")]
#[schemars(with = "Option<String>")]
pub root_key: Option<Vec<u8>>,
}

#[derive(Clone, Debug, Deserialize, PartialEq, JsonSchema, Serialize)]
Expand Down Expand Up @@ -232,9 +253,9 @@ impl From<ManifestGateway> for Gateway {

impl From<ManifestConnected> for Connected {
fn from(value: ManifestConnected) -> Self {
let root_key = value
.root_key
.map_or_else(|| crate::context::IC_ROOT_KEY.to_vec(), |rk| rk.0);
// Absent keys are resolved at access time; we do not default to the
// mainnet key here.
let root_key = value.root_key.map(|rk| rk.0);
match value.endpoints {
Endpoints::Implicit { url } => Connected {
api_url: url.clone(),
Expand Down
2 changes: 1 addition & 1 deletion crates/icp/src/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ pub async fn consolidate_manifest(
connected: Connected {
api_url: IC_MAINNET_NETWORK_API_URL.parse().unwrap(),
http_gateway_url: Some(IC_MAINNET_NETWORK_GATEWAY_URL.parse().unwrap()),
root_key: IC_ROOT_KEY.to_vec(),
root_key: Some(IC_ROOT_KEY.to_vec()),
},
},
},
Expand Down
Loading
Loading