From ad99241640e1e130f775947224b7640bfcec05aa Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Tue, 18 Aug 2026 15:10:06 -0400 Subject: [PATCH 1/3] feat: submit a signed message with `icp message send` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The other half of the air-gapped workflow: `icp canister call --sign-only` writes a file on the machine that holds the key, and this submits it from the machine that has the network. A separate command rather than a mode of `call`, because it shares none of `call`'s inputs — canister, method, args, candid, proxy, cycles and query all come from the file — and because `call` resolves an identity and may unlock a key, which is exactly what the submitting machine must not do. It builds an anonymous agent and never looks for a default identity. It reports where now falls in the submission window before anything else, and before anything touches the network, so "not yet valid" reads as the state the signer asked for rather than as a puzzle; a message outside its window is refused with the window named, and with a note that being only just outside it usually means the signing machine's clock has drifted. Everything displayed is decoded from the signed envelope, never read from the file's metadata. `--dry-run` prints the same summary and stops without an agent, a root key, or an interface fetch, so inspecting a file is entirely offline. Submission goes through `update_signed`, falling back to `wait_signed` with the signer's pre-signed status check when the call does not answer synchronously — that fallback is what lets this machine await an outcome with no key, and is why no `poll` or `status` command is needed. Both failure paths carry the recovery advice: re-run on the same file, do not re-sign, since only an identical request id is de-duplicated by the IC. `canister call`'s response rendering moves to `operations/call_output.rs` so both commands decode and print a reply the same way. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 5 +- crates/icp-cli/src/commands/canister/call.rs | 300 +-------------- crates/icp-cli/src/commands/message/mod.rs | 9 + crates/icp-cli/src/commands/message/send.rs | 365 +++++++++++++++++++ crates/icp-cli/src/commands/mod.rs | 3 + crates/icp-cli/src/main.rs | 7 + crates/icp-cli/src/operations/call_output.rs | 305 ++++++++++++++++ crates/icp-cli/src/operations/mod.rs | 1 + crates/icp-cli/tests/message_send_tests.rs | 354 ++++++++++++++++++ docs/reference/cli.md | 54 ++- 10 files changed, 1115 insertions(+), 288 deletions(-) create mode 100644 crates/icp-cli/src/commands/message/mod.rs create mode 100644 crates/icp-cli/src/commands/message/send.rs create mode 100644 crates/icp-cli/src/operations/call_output.rs create mode 100644 crates/icp-cli/tests/message_send_tests.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 0eb1ed2c8..b711daf3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,10 @@ air-gapped signing ## Experimental -* feat(signing): `icp canister call --sign-only ` composes and signs a call and writes it to a JSON file instead of submitting it, so a machine that holds the key can prepare a call with no network at all and a machine with network can submit it without holding the key. `-` writes to stdout. Nothing is fetched while signing: the Candid interface comes from `--candid` or from the canister's local build artifact rather than from the canister itself, and `--root-key` must name a key (`mainnet` or a hex-encoded key) rather than `fetch`. `--proxy` is not supported. The command that submits the file, `icp message send`, follows separately. +* feat(signing): a canister call can now be signed on one machine and submitted from another, restoring what `dfx canister sign` / `dfx canister send` covered. `icp canister call --sign-only ` composes and signs a call and writes it to a JSON file instead of submitting it; `icp message send ` submits that file and prints the reply. So a machine that holds the key needs no network, and the machine with the network needs no key — it never resolves an identity at all. `-` writes to stdout and reads from stdin respectively. + * Nothing is fetched while signing: the Candid interface comes from `--candid` or from the canister's local build artifact rather than from the canister itself, and `--root-key` must name a key (`mainnet` or a hex-encoded key) rather than `fetch`. `--proxy` is not supported. + * `icp message send` shows what the message contains — sender, canister, method, decoded argument, window, and destination — and asks before submitting. `--yes` skips the prompt, and a non-TTY proceeds without one so a scripted courier works. `--dry-run` prints the same summary and stops without touching the network at all, which makes it the file-inspection command. `--network` / `--root-key` override where the file says to submit, and `--candid`, `--output` and `--json` render the reply exactly as `icp canister call` does. + * If sending fails after the message may already have gone out, re-run `icp message send` **on the same file**: the request id is a hash of the signed content, so resubmitting the identical message is de-duplicated by the IC and cannot execute twice. Signing again produces a new expiry, hence a different request id, which is *not* de-duplicated — for a transfer, a double spend. Every post-submission failure says so. * `--valid-from ` places the message's submission window, as a duration from now (`55m`, `2h`) or an RFC 3339 timestamp; it defaults to now. The window is always five minutes wide, because the IC rejects an ingress message whose expiry is further ahead than that — so this places the window rather than sizing it. Note that this is not dfx's `--expire-after`, which names the window's *end* and leaves you to subtract the five minutes yourself. * The file records the signed envelope, where to submit it, a tagged canister-or-subnet destination, the Candid interface, and a human-readable summary of what was signed. An update also carries a pre-signed `request_status` read, so the submitting machine can await the outcome with no key of its own; it shares the call's expiry, so both live in the same window. diff --git a/crates/icp-cli/src/commands/canister/call.rs b/crates/icp-cli/src/commands/canister/call.rs index 2790cc001..4de112681 100644 --- a/crates/icp-cli/src/commands/canister/call.rs +++ b/crates/icp-cli/src/commands/canister/call.rs @@ -1,12 +1,8 @@ use anyhow::{Context as _, anyhow, bail}; -use candid::types::{Type, TypeInner}; -use candid::{IDLArgs, Principal, TypeEnv, types::Function}; +use candid::{IDLArgs, Principal}; use candid_parser::assist; use candid_parser::parse_idl_args; -use candid_parser::utils::CandidSource; -use clap::{Args, ValueEnum, ValueHint}; -use dialoguer::console::Term; -use ic_agent::Agent; +use clap::{Args, ValueHint}; use ic_agent::agent::EffectiveId; use icp::context::{Context, EnvironmentSelection, NetworkSelection}; use icp::manifest::ArgsFormat; @@ -16,34 +12,21 @@ use icp::prelude::*; use icp::signed_message::{ self, CallType, Destination, Request, SignedMessage, Summary, WindowState, }; -use serde::Serialize; use std::io::{self, Write}; use std::str::FromStr; use time::{OffsetDateTime, format_description::well_known::Rfc3339}; -use tracing::{error, warn}; +use tracing::warn; use url::Url; use crate::{ commands::args::{self, load_args}, - operations::misc::fetch_canister_metadata, + operations::call_output::{ + CallOutputMode, CanisterInterface, get_candid_type, load_candid_from_file, print_response, + }, operations::proxy::update_or_proxy_raw, operations::wasm::extract_candid_service, }; -/// How to interpret and display the call response blob. -#[derive(Debug, Clone, Copy, Default, ValueEnum)] -pub(crate) enum CallOutputMode { - /// Try Candid, then UTF-8, then fall back to hex. - #[default] - Auto, - /// Parse as Candid and pretty-print; error if parsing fails. - Candid, - /// Parse as UTF-8 text; error if invalid. - Text, - /// Print raw response as hex. - Hex, -} - /// Make a canister call #[derive(Args, Debug)] pub(crate) struct CallArgs { @@ -104,9 +87,9 @@ pub(crate) struct CallArgs { #[arg(long)] pub(crate) json: bool, - /// Sign the call and write it to FILE instead of submitting it, so it can be - /// submitted later from a machine that has network access but not your key. - /// `-` writes to stdout. + /// Sign the call and write it to FILE instead of submitting it, for + /// submission from another machine with `icp message send`. `-` writes to + /// stdout. /// /// Nothing is sent, and nothing is fetched: the interface comes from /// `--candid` or the local build artifact rather than from the canister, so @@ -327,38 +310,7 @@ pub(crate) async fn exec(ctx: &Context, args: &CallArgs) -> Result<(), anyhow::E .await? }; - let mut term = Term::buffered_stdout(); - let decoded = decode_response(&res, args.output, declared_method.as_ref()); - - if args.json { - let envelope = JsonCallResponse::build(&res, decoded.as_ref().ok()); - let write_result = serde_json::to_writer(&term, &envelope); - match (write_result, decoded) { - (Ok(()), decode_result) => { - decode_result?; - } - (Err(write_err), Err(decode_err)) => { - // Prefer the decode error; the write failure is incidental. - error!("failed to write JSON response: {write_err}"); - return Err(decode_err); - } - (Err(write_err), Ok(_)) => { - return Err(write_err).context("failed to write JSON response"); - } - } - } else { - match decoded? { - Decoded::Candid(ret) => print_candid_for_term(&mut term, &ret) - .context("failed to print candid return value")?, - Decoded::Text(s) => writeln!(term, "{s}")?, - Decoded::Bytes => writeln!(term, "{}", hex::encode(&res))?, - } - } - - // term is buffered; this single flush covers all output paths (json and non-json). - term.flush()?; - - Ok(()) + print_response(&res, args.output, declared_method.as_ref(), args.json) } /// Signs the call and writes it out for another machine to submit, instead of @@ -508,7 +460,7 @@ async fn sign_only( } eprintln!( - "Signed a {} call to '{method}' on {cid}, as {sender}.", + "Signed the {} call to '{method}' on {cid}, as {sender}.", call_type.as_str(), ); eprintln!( @@ -516,8 +468,9 @@ async fn sign_only( signed_message::format_timestamp(valid_from), signed_message::format_timestamp(valid_until), ); - if out != "-" { - eprintln!("Written to {out}."); + match out.as_str() { + "-" => eprintln!("Submit it with: icp message send "), + path => eprintln!("Written to {path}. Submit it with: icp message send {path}"), } if interface.is_none() { warn!( @@ -583,107 +536,6 @@ fn floor_to_minute(t: OffsetDateTime) -> OffsetDateTime { .and_then(|t| t.replace_second(0)) .expect("0 is a valid second and nanosecond") } - -/// A response decoded according to the requested `CallOutputMode`. -enum Decoded { - Candid(IDLArgs), - Text(String), - /// No decoding was attempted or all attempts failed; emit raw bytes as hex. - Bytes, -} - -fn decode_response( - res: &[u8], - mode: CallOutputMode, - method: Option<&(TypeEnv, Function)>, -) -> Result { - let res_hex = || format!("response (hex): {}", hex::encode(res)); - match mode { - CallOutputMode::Auto => { - if let Ok(args) = try_decode_candid(res, method) { - Ok(Decoded::Candid(args)) - } else if let Ok(s) = std::str::from_utf8(res) { - Ok(Decoded::Text(s.to_string())) - } else { - Ok(Decoded::Bytes) - } - } - CallOutputMode::Candid => try_decode_candid(res, method) - .map(Decoded::Candid) - .with_context(res_hex), - CallOutputMode::Text => std::str::from_utf8(res) - .map(|s| Decoded::Text(s.to_string())) - .with_context(res_hex) - .context("response is not valid UTF-8"), - CallOutputMode::Hex => Ok(Decoded::Bytes), - } -} - -#[derive(Serialize)] -struct JsonCallResponse { - response_bytes: String, - response_text: Option, - response_candid: Option, -} - -impl JsonCallResponse { - fn build(res: &[u8], decoded: Option<&Decoded>) -> Self { - Self { - response_bytes: hex::encode(res), - response_text: match decoded { - Some(Decoded::Text(s)) => Some(s.clone()), - _ => None, - }, - response_candid: match decoded { - Some(Decoded::Candid(args)) => Some(format!("{args}")), - _ => None, - }, - } - } -} - -/// Tries to decode the response as Candid. Returns `None` if decoding fails. -fn try_decode_candid( - res: &[u8], - candid_types: Option<&(TypeEnv, Function)>, -) -> Result { - match candid_types { - Some((type_env, func)) => IDLArgs::from_bytes_with_types(res, type_env, &func.rets) - .map_err(|e| anyhow!("failed to parse Candid: {e}")), - None => IDLArgs::from_bytes(res).map_err(|e| anyhow!("failed to parse Candid: {e}")), - } -} - -/// Pretty-prints IDLArgs detecting the terminal's width to avoid the 80-column default. -pub(crate) fn print_candid_for_term(term: &mut Term, args: &IDLArgs) -> io::Result<()> { - if term.is_term() { - let width = term.size().1 as usize; - let pp_args = candid_parser::pretty::candid::value::pp_args(args); - match pp_args.render(width, term) { - Ok(()) => { - writeln!(term)?; - } - Err(_) => { - writeln!(term, "{args}")?; - } - } - } else { - writeln!(term, "{args}")?; - } - Ok(()) -} - -/// Gets the Candid type of a method on a canister by fetching its Candid interface. -/// -/// This is a best effort function: it will succeed if -/// - the canister exposes its Candid interface in its metadata; -/// - the IDL file can be parsed and type checked in Rust parser; -/// - has an actor in the IDL file. If anything fails, it returns None. -async fn get_candid_type(agent: &Agent, canister_id: Principal) -> Option { - let candid_interface = fetch_canister_metadata(agent, canister_id, "candid:service").await?; - CanisterInterface::from_text(candid_interface).ok() -} - /// Gets the Candid interface a project canister was last built with, from the /// `candid:service` metadata of its build artifact. /// @@ -700,127 +552,3 @@ async fn local_candid_type( let wasm = ctx.artifacts.lookup(name).await.ok()?; CanisterInterface::from_text(extract_candid_service(&wasm)?).ok() } - -/// Loads a Candid interface from a local `.did` file. -/// -/// Unlike [`get_candid_type`], failures are surfaced to the caller because the -/// user explicitly asked for this file to be used. -fn load_candid_from_file(path: &Path) -> Result { - // Parsed from the path rather than from the text below, so that a `.did` - // file importing another one still resolves. - let candid_source = CandidSource::File(path.as_std_path()); - let (type_env, ty) = candid_source - .load() - .with_context(|| format!("failed to load Candid interface from {path}"))?; - let actor = - ty.ok_or_else(|| anyhow!("Candid file {path} does not declare a service interface"))?; - Ok(CanisterInterface { - env: type_env, - ty: actor, - source: icp::fs::read_to_string(path)?, - }) -} - -struct CanisterInterface { - env: TypeEnv, - ty: Type, - - /// The `.did` text this was parsed from. `--sign-only` embeds it in the - /// message file, since the machine that submits the call has no project to - /// resolve an interface from. - source: String, -} - -impl CanisterInterface { - fn from_text(source: String) -> Result { - let (env, ty) = CandidSource::Text(&source) - .load() - .context("failed to parse Candid interface")?; - let ty = ty.context("Candid interface does not declare a service")?; - Ok(CanisterInterface { env, ty, source }) - } - - fn methods(&self) -> impl Iterator { - let ty = if let TypeInner::Class(_, t) = &*self.ty.0 { - t - } else { - &self.ty - }; - let TypeInner::Service(methods) = &*ty.0 else { - unreachable!("check_prog should verify service type") - }; - methods.iter().map(|(name, _)| name.as_str()) - } - fn get_method<'a>(&'a self, method_name: &'a str) -> Option<&'a Function> { - self.env.get_method(&self.ty, method_name).ok() - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn typed_decoding_preserves_record_field_names() { - // Encode a record — field names become hashes in the Candid binary format - let args = candid_parser::parse_idl_args( - r#"(record { network = "regtest"; bitcoin_canister_id = "abc" })"#, - ) - .unwrap(); - let bytes = args.to_bytes().unwrap(); - - // Without types: field names are lost, displayed as hash numbers - let untyped = IDLArgs::from_bytes(&bytes).unwrap(); - let untyped_str = format!("{untyped}"); - assert!( - !untyped_str.contains("network"), - "untyped decoding should not contain field names: {untyped_str}" - ); - - // With types: field names are restored from the type environment - let did = r#" - type config = record { network : text; bitcoin_canister_id : text }; - service : { "get_config" : () -> (config) query } - "#; - let source = CandidSource::Text(did); - let (type_env, ty) = source.load().unwrap(); - let actor = ty.unwrap(); - let func = type_env.get_method(&actor, "get_config").unwrap().clone(); - - let typed = IDLArgs::from_bytes_with_types(&bytes, &type_env, &func.rets).unwrap(); - let typed_str = format!("{typed}"); - assert!( - typed_str.contains("network"), - "typed decoding should contain 'network': {typed_str}" - ); - assert!( - typed_str.contains("bitcoin_canister_id"), - "typed decoding should contain 'bitcoin_canister_id': {typed_str}" - ); - } - - #[test] - fn is_query_detects_method_types() { - let did = r#" - service : { - "get_value" : () -> (text) query; - "set_value" : (text) -> () - } - "#; - let source = CandidSource::Text(did); - let (type_env, ty) = source.load().unwrap(); - let actor = ty.unwrap(); - - let query_func = type_env.get_method(&actor, "get_value").unwrap(); - assert!( - query_func.is_query(), - "get_value should be detected as query" - ); - - let update_func = type_env.get_method(&actor, "set_value").unwrap(); - assert!( - !update_func.is_query(), - "set_value should be detected as update" - ); - } -} diff --git a/crates/icp-cli/src/commands/message/mod.rs b/crates/icp-cli/src/commands/message/mod.rs new file mode 100644 index 000000000..34b1fb7b5 --- /dev/null +++ b/crates/icp-cli/src/commands/message/mod.rs @@ -0,0 +1,9 @@ +use clap::Subcommand; + +pub(crate) mod send; + +/// Work with signed messages +#[derive(Subcommand, Debug)] +pub(crate) enum Command { + Send(send::SendArgs), +} diff --git a/crates/icp-cli/src/commands/message/send.rs b/crates/icp-cli/src/commands/message/send.rs new file mode 100644 index 000000000..703f38ddf --- /dev/null +++ b/crates/icp-cli/src/commands/message/send.rs @@ -0,0 +1,365 @@ +use anyhow::{Context as _, bail}; +use candid::{IDLArgs, TypeEnv, types::Function}; +use clap::{Args, ValueHint}; +use ic_agent::agent::CallResponse; +use icp::context::{Context, IC_ROOT_KEY, NetworkSelection}; +use icp::identity::IdentitySelection; +use icp::network::RootKeySpec; +use icp::prelude::*; +use icp::signed_message::{ + CallType, Destination, SUBMISSION_WINDOW, SignedMessage, Validated, WindowState, + format_timestamp, +}; +use std::io::{self, IsTerminal, Read}; +use time::{Duration, OffsetDateTime}; +use tracing::warn; +use url::Url; + +use crate::operations::call_output::{ + CallOutputMode, CanisterInterface, get_candid_type, load_candid_from_file, print_response, +}; +use crate::options::NetworkOpt; + +/// Submit a message signed on another machine +/// +/// Takes a file written by `icp canister call --sign-only`, shows what it +/// contains, submits it, and waits for the reply. No identity is used and none +/// is needed: the message was already signed by whoever composed it, so this +/// machine only has to carry it to the network. +#[derive(Args, Debug)] +pub(crate) struct SendArgs { + /// The signed message file. `-` reads stdin. + #[arg(value_hint = ValueHint::FilePath)] + pub(crate) file: PathBuf, + + /// Where to submit, overriding the network recorded in the file. + /// + /// Useful when the signing machine recorded a URL this one cannot reach. + /// The envelope is signed and carries no URL of its own, so redirecting it + /// cannot change what executes — only whether it is accepted. + #[command(flatten)] + pub(crate) network: NetworkOpt, + + /// Show what the message contains and exit without submitting it. + #[arg(long)] + pub(crate) dry_run: bool, + + /// Submit without asking for confirmation. + #[arg(long, short)] + pub(crate) yes: bool, + + /// Path to a Candid (`.did`) file describing the canister's interface, + /// overriding the one embedded in the message. + #[arg(long, value_name = "PATH", value_hint = ValueHint::FilePath)] + pub(crate) candid: Option, + + /// How to interpret and display the response. + #[arg(long, short, default_value = "auto")] + pub(crate) output: CallOutputMode, + + /// Output command results as JSON + #[arg(long)] + pub(crate) json: bool, +} + +pub(crate) async fn exec(ctx: &Context, args: &SendArgs) -> Result<(), anyhow::Error> { + let message = load(&args.file)?; + // Everything shown or acted upon below comes from here, decoded out of the + // signed envelope. The file's own metadata is never trusted for a decision. + let validated = message + .validate(OffsetDateTime::now_utc()) + .with_context(|| format!("{} is not a message this can submit", args.file))?; + + // Before anything else, including anything that would touch the network: a + // message outside its window cannot be submitted, and saying so first makes + // "not yet valid" read as the state the signer asked for. + report_window(&validated); + if !args.dry_run { + refuse_unsubmittable(&validated)?; + } + + let (url, root_key) = resolve_network(ctx, args, &message).await?; + + // `--dry-run` is the file-inspection command, so it stays entirely offline: + // no agent, no root key, and no fetching an interface the file did not carry. + let agent = match args.dry_run { + true => None, + false => { + let agent = ctx + .get_agent_for_url(&IdentitySelection::Anonymous, &url) + .await?; + apply_root_key(&agent, &root_key).await?; + Some(agent) + } + }; + + let interface = resolve_interface(args, &message, &validated, agent.as_ref()).await?; + let declared_method = interface + .as_ref() + .and_then(|i| Some((i.env.clone(), i.get_method(&validated.method)?.clone()))); + + print_summary(&message, &validated, &url, declared_method.as_ref()); + + if args.dry_run { + eprintln!("Not submitted: this was a --dry-run."); + return Ok(()); + } + let agent = agent.expect("an agent is built whenever the message is submitted"); + + if !args.yes && !confirm()? { + eprintln!("Not submitted."); + return Ok(()); + } + + let effective_id = message.destination.to_effective_id(); + let response = match validated.call_type { + CallType::Query => agent + .query_signed(effective_id, message.request.envelope.clone()) + .await + .context("the query was rejected")?, + CallType::Update => { + let submitted = agent + .update_signed(effective_id, message.request.envelope.clone()) + .await + .with_context(|| resubmit_advice(&args.file))?; + match submitted { + // The synchronous call path already returned a certified reply. + CallResponse::Response(reply) => reply, + // Otherwise await the outcome with the status check the signer + // pre-signed — which is what lets this machine wait without a key. + CallResponse::Poll(request_id) => { + let status_check = message + .request + .status_check + .clone() + .context("an update message must carry a status_check")?; + agent + .wait_signed(&request_id, effective_id, status_check) + .await + .map(|(reply, _cert)| reply) + .with_context(|| resubmit_advice(&args.file))? + } + } + } + }; + + print_response(&response, args.output, declared_method.as_ref(), args.json) +} + +/// Refuses a message that is outside its submission window, naming the window so +/// the operator knows whether to wait or to go back to the signing machine. +fn refuse_unsubmittable(validated: &Validated) -> Result<(), anyhow::Error> { + match validated.window { + WindowState::Valid => Ok(()), + WindowState::Expired => bail!( + "the submission window closed at {}, so this message can no longer be submitted. \ + It has to be signed again on the machine that holds the key.", + format_timestamp(validated.valid_until), + ), + WindowState::NotYetValid => bail!( + "the submission window does not open until {} ({} from now). Run this again then; \ + the message stays good until it expires at {}.", + format_timestamp(validated.valid_from), + describe_gap(validated.valid_from - OffsetDateTime::now_utc()), + format_timestamp(validated.valid_until), + ), + } +} + +/// Reads the message from `path`, or from stdin for `-`. +fn load(path: &Path) -> Result { + if path != "-" { + return Ok(SignedMessage::load(path)?); + } + let mut buf = String::new(); + io::stdin() + .read_to_string(&mut buf) + .context("failed to read the signed message from stdin")?; + serde_json::from_str(&buf).context("failed to parse the signed message read from stdin") +} + +/// Where to submit: the file's network, unless this machine was told otherwise. +async fn resolve_network( + ctx: &Context, + args: &SendArgs, + message: &SignedMessage, +) -> Result<(Url, RootKeySpec), anyhow::Error> { + let selection: NetworkSelection = args.network.clone().into(); + if selection == NetworkSelection::Default { + return Ok(( + message.network.url.clone(), + message.network.root_key.clone(), + )); + } + let network = ctx.get_network(&selection).await?; + let access = ctx.network.access(&network).await?; + Ok((access.api_url, RootKeySpec::Explicit(access.root_key))) +} + +/// The reply's certificate is verified against this, so it is the one piece of +/// configuration that decides whether an answer can be believed. +async fn apply_root_key( + agent: &ic_agent::Agent, + root_key: &RootKeySpec, +) -> Result<(), anyhow::Error> { + match root_key { + RootKeySpec::Mainnet => agent.set_root_key(IC_ROOT_KEY.to_vec()), + RootKeySpec::Explicit(bytes) => agent.set_root_key(bytes.clone()), + RootKeySpec::Fetch => { + warn!( + "fetching the root key from the network; its provenance is not verified \ + (trust-on-first-use), so the reply's certificate proves less than a pinned key would" + ); + agent + .fetch_root_key() + .await + .context("failed to fetch the root key")?; + } + } + Ok(()) +} + +/// `--candid` → the interface embedded by the signer → what the canister +/// publishes → nothing. +/// +/// The embedded interface sorts above the fetch deliberately: it is the one the +/// signer used to *encode* the argument, so decoding with it is self-consistent, +/// needs no round trip, and works for a canister exposing no metadata. The fetch +/// stays a real fallback because this machine, unlike the signer, is online — +/// and is skipped entirely when there is no agent, i.e. under `--dry-run`. +async fn resolve_interface( + args: &SendArgs, + message: &SignedMessage, + validated: &Validated, + agent: Option<&ic_agent::Agent>, +) -> Result, anyhow::Error> { + if let Some(path) = &args.candid { + return Ok(Some(load_candid_from_file(path)?)); + } + if let Some(embedded) = &message.candid { + match CanisterInterface::from_text(embedded.clone()) { + Ok(interface) => return Ok(Some(interface)), + // Display only, so a stale or broken interface degrades rather than + // stopping a message that is otherwise perfectly submittable. + Err(e) => warn!("ignoring the interface embedded in the message: {e}"), + } + } + match agent { + Some(agent) => Ok(get_candid_type(agent, validated.canister_id).await), + None => Ok(None), + } +} + +/// Says where now falls in the window before anything else happens, so "not yet +/// valid" reads as the state the signer asked for rather than as a puzzle. +fn report_window(validated: &Validated) { + let now = OffsetDateTime::now_utc(); + match validated.window { + WindowState::NotYetValid => eprintln!( + "Not yet submittable. The window opens at {} — {} from now — and closes at {}.", + format_timestamp(validated.valid_from), + describe_gap(validated.valid_from - now), + format_timestamp(validated.valid_until), + ), + WindowState::Valid => eprintln!( + "Submittable for another {}, until {}.", + describe_gap(validated.valid_until - now), + format_timestamp(validated.valid_until), + ), + WindowState::Expired => eprintln!( + "Expired. The window closed at {}, {} ago.", + format_timestamp(validated.valid_until), + describe_gap(now - validated.valid_until), + ), + } + // The window is computed from the signing machine's clock, and an offline + // machine is exactly the kind that drifts. + if matches!( + validated.window, + WindowState::NotYetValid | WindowState::Expired + ) && (validated.valid_from - now).abs() < SUBMISSION_WINDOW + { + eprintln!( + "It is only just outside the window, which usually means the signing machine's \ + clock is off rather than that you are early or late." + ); + } +} + +fn print_summary( + message: &SignedMessage, + validated: &Validated, + url: &Url, + declared_method: Option<&(TypeEnv, Function)>, +) { + eprintln!(); + eprintln!(" Sender: {}", validated.sender); + eprintln!(" Canister: {}", validated.canister_id); + eprintln!( + " Method: {} ({})", + validated.method, + validated.call_type.as_str() + ); + eprintln!( + " Argument: {}", + render_argument(&validated.arg, declared_method) + ); + eprintln!(" Network: {url}"); + // Unauthenticated, and for a subnet-scoped canister creation the destination + // *is* the choice of subnet — so it is shown rather than assumed. + match message.destination { + Destination::Canister(id) => eprintln!(" Destination: canister {id}"), + Destination::Subnet(id) => eprintln!(" Destination: subnet {id}"), + } + eprintln!(); +} + +/// The argument decoded, or hex if there is no interface to decode it with. +/// A prompt showing a hex blob is not something anyone can review. +fn render_argument(arg: &[u8], declared_method: Option<&(TypeEnv, Function)>) -> String { + let decoded = match declared_method { + Some((env, func)) => IDLArgs::from_bytes_with_types(arg, env, &func.args).ok(), + None => IDLArgs::from_bytes(arg).ok(), + }; + match decoded { + Some(args) => format!("{args}"), + None => format!("{} (hex, could not decode)", hex::encode(arg)), + } +} + +fn confirm() -> Result { + // A courier is expected to be scripted, and blocking on a prompt nobody can + // answer is worse than submitting a message its author already reviewed. + if !io::stdin().is_terminal() { + return Ok(true); + } + dialoguer::Confirm::new() + .with_prompt("Submit this message?") + .default(false) + .interact() + .context("failed to read confirmation") +} + +/// What to do when a submission fails after the message may already have been +/// sent. Re-running is safe; re-signing is not. +fn resubmit_advice(file: &Path) -> String { + format!( + "the message may already have reached the network. Re-run `icp message send {file}` on \ + the same file — the request id is a hash of the signed content, so resubmitting the \ + identical message is de-duplicated by the IC and cannot execute twice. Do NOT sign it \ + again: a new signature means a new expiry, hence a different request id, which is NOT \ + de-duplicated. Two limits worth knowing: once the reply is pruned from the ingress \ + history the outcome can no longer be recovered (the call still executed), and once the \ + window closes the message cannot be submitted at all" + ) +} + +/// A rough, readable gap, for telling someone how long they have or how long to wait. +fn describe_gap(gap: Duration) -> String { + let seconds = gap.whole_seconds().abs(); + match seconds { + 0..=90 => format!("{seconds}s"), + 91..=5400 => format!("{}m", (seconds + 30) / 60), + _ => format!("{}h{}m", seconds / 3600, (seconds % 3600) / 60), + } +} diff --git a/crates/icp-cli/src/commands/mod.rs b/crates/icp-cli/src/commands/mod.rs index dcd64626a..27a26b5a5 100644 --- a/crates/icp-cli/src/commands/mod.rs +++ b/crates/icp-cli/src/commands/mod.rs @@ -8,6 +8,7 @@ pub(crate) mod cycles; pub(crate) mod deploy; pub(crate) mod environment; pub(crate) mod identity; +pub(crate) mod message; pub(crate) mod network; pub(crate) mod new; pub(crate) mod parsers; @@ -31,6 +32,8 @@ pub(crate) enum Command { #[command(subcommand)] Identity(identity::Command), #[command(subcommand)] + Message(message::Command), + #[command(subcommand)] Network(network::Command), New(new::IcpGenerateArgs), #[command(subcommand)] diff --git a/crates/icp-cli/src/main.rs b/crates/icp-cli/src/main.rs index 6fe636038..66047c977 100644 --- a/crates/icp-cli/src/main.rs +++ b/crates/icp-cli/src/main.rs @@ -416,6 +416,13 @@ async fn dispatch(ctx: &icp::context::Context, command: Command) -> Result<(), E }, // Network + // Message + Command::Message(cmd) => match cmd { + commands::message::Command::Send(args) => { + commands::message::send::exec(ctx, &args).await? + } + }, + Command::Network(cmd) => match cmd { commands::network::Command::List(args) => { commands::network::list::exec(ctx, &args).await? diff --git a/crates/icp-cli/src/operations/call_output.rs b/crates/icp-cli/src/operations/call_output.rs new file mode 100644 index 000000000..953931971 --- /dev/null +++ b/crates/icp-cli/src/operations/call_output.rs @@ -0,0 +1,305 @@ +//! Rendering a canister call's response. +//! +//! Shared by `icp canister call` and `icp message send`: the two resolve an +//! interface from different places — a project and the network, versus the text +//! embedded in a signed message — but a reply must print the same either way. + +use anyhow::{Context as _, anyhow}; +use candid::types::{Type, TypeInner}; +use candid::{IDLArgs, Principal, TypeEnv, types::Function}; +use candid_parser::utils::CandidSource; +use clap::ValueEnum; +use dialoguer::console::Term; +use ic_agent::Agent; +use icp::prelude::*; +use serde::Serialize; +use std::io::{self, Write}; +use tracing::error; + +use crate::operations::misc::fetch_canister_metadata; + +/// How to interpret and display the call response blob. +#[derive(Debug, Clone, Copy, Default, ValueEnum)] +pub(crate) enum CallOutputMode { + /// Try Candid, then UTF-8, then fall back to hex. + #[default] + Auto, + /// Parse as Candid and pretty-print; error if parsing fails. + Candid, + /// Parse as UTF-8 text; error if invalid. + Text, + /// Print raw response as hex. + Hex, +} + +/// Writes a call's response to stdout, in whichever of the two shapes was asked +/// for. +/// +/// Lifted out of `canister call` unchanged so that `message send` renders a +/// reply identically: same `--output` modes, same `--json` envelope, same single +/// flush of the buffered terminal covering both paths. +pub(crate) fn print_response( + res: &[u8], + mode: CallOutputMode, + method: Option<&(TypeEnv, Function)>, + json: bool, +) -> Result<(), anyhow::Error> { + let mut term = Term::buffered_stdout(); + let decoded = decode_response(res, mode, method); + + if json { + let envelope = JsonCallResponse::build(res, decoded.as_ref().ok()); + let write_result = serde_json::to_writer(&term, &envelope); + match (write_result, decoded) { + (Ok(()), decode_result) => { + decode_result?; + } + (Err(write_err), Err(decode_err)) => { + // Prefer the decode error; the write failure is incidental. + error!("failed to write JSON response: {write_err}"); + return Err(decode_err); + } + (Err(write_err), Ok(_)) => { + return Err(write_err).context("failed to write JSON response"); + } + } + } else { + match decoded? { + Decoded::Candid(ret) => print_candid_for_term(&mut term, &ret) + .context("failed to print candid return value")?, + Decoded::Text(s) => writeln!(term, "{s}")?, + Decoded::Bytes => writeln!(term, "{}", hex::encode(res))?, + } + } + + // term is buffered; this single flush covers all output paths (json and non-json). + term.flush()?; + Ok(()) +} + +/// A response decoded according to the requested `CallOutputMode`. +pub(crate) enum Decoded { + Candid(IDLArgs), + Text(String), + /// No decoding was attempted or all attempts failed; emit raw bytes as hex. + Bytes, +} + +pub(crate) fn decode_response( + res: &[u8], + mode: CallOutputMode, + method: Option<&(TypeEnv, Function)>, +) -> Result { + let res_hex = || format!("response (hex): {}", hex::encode(res)); + match mode { + CallOutputMode::Auto => { + if let Ok(args) = try_decode_candid(res, method) { + Ok(Decoded::Candid(args)) + } else if let Ok(s) = std::str::from_utf8(res) { + Ok(Decoded::Text(s.to_string())) + } else { + Ok(Decoded::Bytes) + } + } + CallOutputMode::Candid => try_decode_candid(res, method) + .map(Decoded::Candid) + .with_context(res_hex), + CallOutputMode::Text => std::str::from_utf8(res) + .map(|s| Decoded::Text(s.to_string())) + .with_context(res_hex) + .context("response is not valid UTF-8"), + CallOutputMode::Hex => Ok(Decoded::Bytes), + } +} + +#[derive(Serialize)] +struct JsonCallResponse { + response_bytes: String, + response_text: Option, + response_candid: Option, +} + +impl JsonCallResponse { + fn build(res: &[u8], decoded: Option<&Decoded>) -> Self { + Self { + response_bytes: hex::encode(res), + response_text: match decoded { + Some(Decoded::Text(s)) => Some(s.clone()), + _ => None, + }, + response_candid: match decoded { + Some(Decoded::Candid(args)) => Some(format!("{args}")), + _ => None, + }, + } + } +} + +/// Tries to decode the response as Candid. Returns `None` if decoding fails. +fn try_decode_candid( + res: &[u8], + candid_types: Option<&(TypeEnv, Function)>, +) -> Result { + match candid_types { + Some((type_env, func)) => IDLArgs::from_bytes_with_types(res, type_env, &func.rets) + .map_err(|e| anyhow!("failed to parse Candid: {e}")), + None => IDLArgs::from_bytes(res).map_err(|e| anyhow!("failed to parse Candid: {e}")), + } +} + +/// Pretty-prints IDLArgs detecting the terminal's width to avoid the 80-column default. +pub(crate) fn print_candid_for_term(term: &mut Term, args: &IDLArgs) -> io::Result<()> { + if term.is_term() { + let width = term.size().1 as usize; + let pp_args = candid_parser::pretty::candid::value::pp_args(args); + match pp_args.render(width, term) { + Ok(()) => { + writeln!(term)?; + } + Err(_) => { + writeln!(term, "{args}")?; + } + } + } else { + writeln!(term, "{args}")?; + } + Ok(()) +} + +/// Gets the Candid type of a method on a canister by fetching its Candid interface. +/// +/// This is a best effort function: it will succeed if +/// - the canister exposes its Candid interface in its metadata; +/// - the IDL file can be parsed and type checked in Rust parser; +/// - has an actor in the IDL file. If anything fails, it returns None. +pub(crate) async fn get_candid_type( + agent: &Agent, + canister_id: Principal, +) -> Option { + let candid_interface = fetch_canister_metadata(agent, canister_id, "candid:service").await?; + CanisterInterface::from_text(candid_interface).ok() +} + +/// Loads a Candid interface from a local `.did` file. +/// +/// Unlike [`get_candid_type`], failures are surfaced to the caller because the +/// user explicitly asked for this file to be used. +pub(crate) fn load_candid_from_file(path: &Path) -> Result { + // Parsed from the path rather than from the text below, so that a `.did` + // file importing another one still resolves. + let candid_source = CandidSource::File(path.as_std_path()); + let (type_env, ty) = candid_source + .load() + .with_context(|| format!("failed to load Candid interface from {path}"))?; + let actor = + ty.ok_or_else(|| anyhow!("Candid file {path} does not declare a service interface"))?; + Ok(CanisterInterface { + env: type_env, + ty: actor, + source: icp::fs::read_to_string(path)?, + }) +} + +pub(crate) struct CanisterInterface { + pub(crate) env: TypeEnv, + pub(crate) ty: Type, + + /// The `.did` text this was parsed from. `--sign-only` embeds it in the + /// message file, since the machine that submits the call has no project to + /// resolve an interface from. + pub(crate) source: String, +} + +impl CanisterInterface { + pub(crate) fn from_text(source: String) -> Result { + let (env, ty) = CandidSource::Text(&source) + .load() + .context("failed to parse Candid interface")?; + let ty = ty.context("Candid interface does not declare a service")?; + Ok(CanisterInterface { env, ty, source }) + } + + pub(crate) fn methods(&self) -> impl Iterator { + let ty = if let TypeInner::Class(_, t) = &*self.ty.0 { + t + } else { + &self.ty + }; + let TypeInner::Service(methods) = &*ty.0 else { + unreachable!("check_prog should verify service type") + }; + methods.iter().map(|(name, _)| name.as_str()) + } + pub(crate) fn get_method<'a>(&'a self, method_name: &'a str) -> Option<&'a Function> { + self.env.get_method(&self.ty, method_name).ok() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn typed_decoding_preserves_record_field_names() { + // Encode a record — field names become hashes in the Candid binary format + let args = candid_parser::parse_idl_args( + r#"(record { network = "regtest"; bitcoin_canister_id = "abc" })"#, + ) + .unwrap(); + let bytes = args.to_bytes().unwrap(); + + // Without types: field names are lost, displayed as hash numbers + let untyped = IDLArgs::from_bytes(&bytes).unwrap(); + let untyped_str = format!("{untyped}"); + assert!( + !untyped_str.contains("network"), + "untyped decoding should not contain field names: {untyped_str}" + ); + + // With types: field names are restored from the type environment + let did = r#" + type config = record { network : text; bitcoin_canister_id : text }; + service : { "get_config" : () -> (config) query } + "#; + let source = CandidSource::Text(did); + let (type_env, ty) = source.load().unwrap(); + let actor = ty.unwrap(); + let func = type_env.get_method(&actor, "get_config").unwrap().clone(); + + let typed = IDLArgs::from_bytes_with_types(&bytes, &type_env, &func.rets).unwrap(); + let typed_str = format!("{typed}"); + assert!( + typed_str.contains("network"), + "typed decoding should contain 'network': {typed_str}" + ); + assert!( + typed_str.contains("bitcoin_canister_id"), + "typed decoding should contain 'bitcoin_canister_id': {typed_str}" + ); + } + + #[test] + fn is_query_detects_method_types() { + let did = r#" + service : { + "get_value" : () -> (text) query; + "set_value" : (text) -> () + } + "#; + let source = CandidSource::Text(did); + let (type_env, ty) = source.load().unwrap(); + let actor = ty.unwrap(); + + let query_func = type_env.get_method(&actor, "get_value").unwrap(); + assert!( + query_func.is_query(), + "get_value should be detected as query" + ); + + let update_func = type_env.get_method(&actor, "set_value").unwrap(); + assert!( + !update_func.is_query(), + "set_value should be detected as update" + ); + } +} diff --git a/crates/icp-cli/src/operations/mod.rs b/crates/icp-cli/src/operations/mod.rs index 5ce6546d0..918b80613 100644 --- a/crates/icp-cli/src/operations/mod.rs +++ b/crates/icp-cli/src/operations/mod.rs @@ -1,6 +1,7 @@ pub(crate) mod binding_env_vars; pub(crate) mod build; pub(crate) mod bundle; +pub(crate) mod call_output; pub(crate) mod candid_compat; pub(crate) mod canister_migration; pub(crate) mod create; diff --git a/crates/icp-cli/tests/message_send_tests.rs b/crates/icp-cli/tests/message_send_tests.rs new file mode 100644 index 000000000..97f715a5d --- /dev/null +++ b/crates/icp-cli/tests/message_send_tests.rs @@ -0,0 +1,354 @@ +//! `icp message send`: submitting a message that was signed elsewhere. +//! +//! The round-trip tests run the whole two-machine workflow against a local +//! network — sign with no network in reach, then submit — since that pairing is +//! the only thing that proves either half works. + +use indoc::formatdoc; +use predicates::prelude::PredicateBooleanExt; +use predicates::str::contains; +use serde_json::Value; + +use crate::common::{ChildGuard, ENVIRONMENT_RANDOM_PORT, NETWORK_RANDOM_PORT, TestContext}; +use icp::fs::write_string; +use icp::prelude::*; + +mod common; + +const GREET_DID: &str = r#"service : { "greet" : (text) -> (text) query }"#; + +/// A project with a canister deployed to a running local network. +/// +/// The returned guard owns the network process: the caller has to hold it for +/// as long as it needs the network, and dropping it shuts the network down. +async fn deployed(ctx: &TestContext) -> (PathBuf, ChildGuard) { + let project_dir = ctx.create_project_dir("icp"); + 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("write manifest"); + + let guard = ctx.start_network_in(&project_dir, "random-network").await; + ctx.ping_until_healthy(&project_dir, "random-network"); + + ctx.icp() + .current_dir(&project_dir) + .args([ + "deploy", + "my-canister", + "--environment", + "random-environment", + ]) + .assert() + .success(); + + (project_dir, guard) +} + +/// The whole point: sign on one machine, submit from another, get the reply. +#[tokio::test] +async fn round_trip_update() { + let ctx = TestContext::new(); + let (project_dir, _network) = deployed(&ctx).await; + let msg = project_dir.join("update.json"); + + ctx.icp() + .current_dir(&project_dir) + .args([ + "canister", + "call", + "--environment", + "random-environment", + "--sign-only", + msg.as_str(), + "my-canister", + "greet", + "(\"world\")", + ]) + .assert() + .success(); + + // Submitting needs no identity, no project and no interface of its own — + // only the file. Run it from outside the project to prove that. + ctx.icp() + .args(["message", "send", msg.as_str(), "--yes"]) + .assert() + .success() + .stdout(contains("Hello, world!")) + .stderr(contains("Submittable for another")); +} + +/// A signed query takes the other endpoint and carries no status check. +#[tokio::test] +async fn round_trip_query() { + let ctx = TestContext::new(); + let (project_dir, _network) = deployed(&ctx).await; + let msg = project_dir.join("query.json"); + + ctx.icp() + .current_dir(&project_dir) + .args([ + "canister", + "call", + "--environment", + "random-environment", + "--query", + "--sign-only", + msg.as_str(), + "my-canister", + "greet", + "(\"world\")", + ]) + .assert() + .success(); + + ctx.icp() + .args(["message", "send", msg.as_str(), "--yes"]) + .assert() + .success() + .stdout(contains("Hello, world!")); +} + +/// Re-running `send` on the same file is the documented recovery when a +/// submission fails after the message may already have gone out. It has to be +/// accepted and yield the same answer — that is what the advice depends on. +/// (It does not, and cannot from here, observe how many times the call ran; the +/// IC's de-duplication of an identical request id is what guarantees that.) +#[tokio::test] +async fn resending_the_same_file_is_accepted() { + let ctx = TestContext::new(); + let (project_dir, _network) = deployed(&ctx).await; + let msg = project_dir.join("resend.json"); + + ctx.icp() + .current_dir(&project_dir) + .args([ + "canister", + "call", + "--environment", + "random-environment", + "--sign-only", + msg.as_str(), + "my-canister", + "greet", + "(\"once\")", + ]) + .assert() + .success(); + + for attempt in 0..2 { + ctx.icp() + .args(["message", "send", msg.as_str(), "--yes"]) + .assert() + .success() + .stdout(contains("Hello, once!")); + eprintln!("submission {} accepted", attempt + 1); + } +} + +/// `--dry-run` inspects the file and stops. It stays entirely offline, which is +/// what makes it safe to point at a message you have not decided to send yet — +/// the network here does not exist at all. +#[test] +fn dry_run_inspects_without_sending() { + let ctx = TestContext::new(); + let did = ctx.home_path().join("service.did"); + write_string(&did, GREET_DID).expect("write candid"); + let msg = ctx.home_path().join("message.json"); + + ctx.icp() + .args([ + "canister", + "call", + "--network", + "http://127.0.0.1:1", + "--root-key", + "mainnet", + "--candid", + did.as_str(), + "--sign-only", + msg.as_str(), + "ryjl3-tyaaa-aaaaa-aaaba-cai", + "greet", + "(\"world\")", + ]) + .assert() + .success(); + + ctx.icp() + .args(["message", "send", msg.as_str(), "--dry-run"]) + .assert() + .success() + // The argument is decoded with the interface the signer embedded, so the + // summary is reviewable rather than a hex blob. + .stderr(contains("(\"world\")")) + .stderr(contains("greet (update)")) + .stderr(contains( + "Destination: canister ryjl3-tyaaa-aaaaa-aaaba-cai", + )) + .stderr(contains("Not submitted")); +} + +/// The summary is display-only, so a file whose summary disagrees with its +/// signed envelope is refused rather than quietly believed either way. +#[test] +fn tampered_file_is_refused() { + let ctx = TestContext::new(); + let did = ctx.home_path().join("service.did"); + write_string(&did, GREET_DID).expect("write candid"); + let msg = ctx.home_path().join("tampered.json"); + + ctx.icp() + .args([ + "canister", + "call", + "--network", + "http://127.0.0.1:1", + "--root-key", + "mainnet", + "--candid", + did.as_str(), + "--sign-only", + msg.as_str(), + "ryjl3-tyaaa-aaaaa-aaaba-cai", + "greet", + "(\"world\")", + ]) + .assert() + .success(); + + let mut file: Value = + serde_json::from_str(&icp::fs::read_to_string(&msg).expect("read")).expect("JSON"); + file["summary"]["method"] = Value::String("transfer".into()); + write_string( + &msg, + &serde_json::to_string_pretty(&file).expect("serialize"), + ) + .expect("write"); + + ctx.icp() + .args(["message", "send", msg.as_str(), "--yes"]) + .assert() + .failure() + .stderr(contains("summary does not match the signed request")); +} + +/// A message whose window has not opened is refused with the opening time, so +/// the operator knows to wait rather than to re-sign. Refused before anything +/// touches the network: the URL here is unreachable. +#[test] +fn not_yet_valid_file_is_refused() { + let ctx = TestContext::new(); + let did = ctx.home_path().join("service.did"); + write_string(&did, GREET_DID).expect("write candid"); + let msg = ctx.home_path().join("later.json"); + + ctx.icp() + .args([ + "canister", + "call", + "--network", + "http://127.0.0.1:1", + "--root-key", + "mainnet", + "--candid", + did.as_str(), + "--sign-only", + msg.as_str(), + "--valid-from", + "1h", + "ryjl3-tyaaa-aaaaa-aaaba-cai", + "greet", + "(\"world\")", + ]) + .assert() + .success(); + + ctx.icp() + .args(["message", "send", msg.as_str(), "--yes"]) + .assert() + .failure() + .stderr(contains("Not yet submittable")) + .stderr(contains("does not open until").and(contains("Run this again then"))); +} + +/// An expired message is refused with the time the window closed. Built here +/// rather than signed, because `--sign-only` will not produce a window that has +/// already passed — a query, since it needs no status check to keep in step. +#[test] +fn expired_file_is_refused() { + use ic_agent::{Agent, identity::AnonymousIdentity}; + use icp::signed_message::{ + CallType, Destination, Network, Request, SUBMISSION_WINDOW, SignedMessage, Summary, + format_timestamp, + }; + use time::OffsetDateTime; + + let ctx = TestContext::new(); + let canister = candid::Principal::from_text("ryjl3-tyaaa-aaaaa-aaaba-cai").expect("principal"); + + // A window that closed ten minutes ago. + let valid_until = (OffsetDateTime::now_utc() - time::Duration::minutes(10)) + .replace_nanosecond(0) + .and_then(|t| t.replace_second(0)) + .expect("0 is a valid second and nanosecond"); + let valid_from = valid_until - SUBMISSION_WINDOW; + + let agent = Agent::builder() + .with_url("http://127.0.0.1:1") + .with_identity(AnonymousIdentity) + .build() + .expect("building an agent makes no request"); + let signed = agent + .query(&canister, "greet") + .with_arg(b"arg".to_vec()) + .expire_at(valid_until) + .sign() + .expect("signing makes no request"); + + let message = SignedMessage { + format: icp::signed_message::FORMAT.to_string(), + version: icp::signed_message::VERSION, + request: Request { + call_type: CallType::Query, + envelope: signed.signed_query, + request_id: None, + status_check: None, + }, + network: Network { + url: "http://127.0.0.1:1".parse().expect("url"), + root_key: icp::network::RootKeySpec::Mainnet, + }, + destination: Destination::Canister(canister), + candid: None, + summary: Summary { + sender: signed.sender, + canister_id: canister, + method: "greet".to_string(), + arg: b"arg".to_vec(), + signed_at: format_timestamp(valid_from), + valid_from: format_timestamp(valid_from), + valid_until: format_timestamp(valid_until), + }, + }; + + let msg = ctx.home_path().join("expired.json"); + message.save(&msg).expect("save"); + + ctx.icp() + .args(["message", "send", msg.as_str(), "--yes"]) + .assert() + .failure() + .stderr(contains("Expired")) + .stderr(contains(&format_timestamp(valid_until)[..])) + .stderr(contains("signed again")); +} diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 2b9e49ea0..f6feebac1 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -60,6 +60,8 @@ This document contains the help content for the `icp` command-line program. * [`icp identity principal`↴](#icp-identity-principal) * [`icp identity reauth`↴](#icp-identity-reauth) * [`icp identity rename`↴](#icp-identity-rename) +* [`icp message`↴](#icp-message) +* [`icp message send`↴](#icp-message-send) * [`icp network`↴](#icp-network) * [`icp network list`↴](#icp-network-list) * [`icp network ping`↴](#icp-network-ping) @@ -95,6 +97,7 @@ This document contains the help content for the `icp` command-line program. * `deploy` — Deploy a project to an environment * `environment` — Show information about the current project environments * `identity` — Manage your identities +* `message` — Work with signed messages * `network` — Launch and manage local test networks * `new` — Create a new ICP project from a template * `project` — Manage the current project @@ -214,7 +217,7 @@ Make a canister call Print raw response as hex * `--json` — Output command results as JSON -* `--sign-only ` — Sign the call and write it to FILE instead of submitting it, so it can be submitted later from a machine that has network access but not your key. `-` writes to stdout. +* `--sign-only ` — Sign the call and write it to FILE instead of submitting it, for submission from another machine with `icp message send`. `-` writes to stdout. Nothing is sent, and nothing is fetched: the interface comes from `--candid` or the local build artifact rather than from the canister, so this works with no network at all. `--root-key` must name a key rather than `fetch`, and `--proxy` is not supported. * `--valid-from ` — When the signed message's five-minute submission window opens: a duration from now (`55m`, `2h`) or an RFC 3339 timestamp (`2026-08-17T10:07:00Z`). Defaults to now. @@ -1343,6 +1346,55 @@ Rename an identity +## `icp message` + +Work with signed messages + +**Usage:** `icp message ` + +###### **Subcommands:** + +* `send` — Submit a message signed on another machine + + + +## `icp message send` + +Submit a message signed on another machine + +Takes a file written by `icp canister call --sign-only`, shows what it contains, submits it, and waits for the reply. No identity is used and none is needed: the message was already signed by whoever composed it, so this machine only has to carry it to the network. + +**Usage:** `icp message send [OPTIONS] ` + +###### **Arguments:** + +* `` — The signed message file. `-` reads stdin + +###### **Options:** + +* `-n`, `--network ` — Name or URL of the network to target, conflicts with environment argument +* `-k`, `--root-key ` — The root key to use if connecting to a network by URL. Required when using `--network `. One of `mainnet`, `fetch`, or a 266-character hex-encoded root key +* `--dry-run` — Show what the message contains and exit without submitting it +* `-y`, `--yes` — Submit without asking for confirmation +* `--candid ` — Path to a Candid (`.did`) file describing the canister's interface, overriding the one embedded in the message +* `-o`, `--output ` — How to interpret and display the response + + Default value: `auto` + + Possible values: + - `auto`: + Try Candid, then UTF-8, then fall back to hex + - `candid`: + Parse as Candid and pretty-print; error if parsing fails + - `text`: + Parse as UTF-8 text; error if invalid + - `hex`: + Print raw response as hex + +* `--json` — Output command results as JSON + + + ## `icp network` Launch and manage local test networks From 5a21a62834f5bd94135304495fb83f95c8b43b6e Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Thu, 20 Aug 2026 10:39:21 -0400 Subject: [PATCH 2/3] refactor: let only the message file say where to submit it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `icp message send` no longer takes `--network` / `--root-key`. Overriding is a rare case, and a courier who genuinely has to redirect a message can edit `network` in the file — those fields are unauthenticated whether a flag writes them or an editor does, so that is inside the trust model rather than a way around it, and the envelope carries no URL, so where a message goes cannot change what executes. Removing them also closes a footgun. `--network` inherits `env = "ICP_NETWORK"`, which is right for every command that chooses where to act but wrong for one whose artifact already decided: a stray shell variable silently redirected a message signed for a local network to mainnet, and with `--yes` in a scripted courier nothing would have caught it. The new test pins that, and that an edited `network` is honoured. The convenience lost is small in a way the design already guarantees: the only realistic reason a recorded URL goes stale is the network moving, and a five-minute submission window means such a message has almost certainly expired anyway. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 2 +- crates/icp-cli/src/commands/message/send.rs | 49 +++++++------------ crates/icp-cli/tests/message_send_tests.rs | 52 +++++++++++++++++++++ docs/reference/cli.md | 4 +- 4 files changed, 72 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b711daf3f..54afa1527 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ air-gapped signing * feat(signing): a canister call can now be signed on one machine and submitted from another, restoring what `dfx canister sign` / `dfx canister send` covered. `icp canister call --sign-only ` composes and signs a call and writes it to a JSON file instead of submitting it; `icp message send ` submits that file and prints the reply. So a machine that holds the key needs no network, and the machine with the network needs no key — it never resolves an identity at all. `-` writes to stdout and reads from stdin respectively. * Nothing is fetched while signing: the Candid interface comes from `--candid` or from the canister's local build artifact rather than from the canister itself, and `--root-key` must name a key (`mainnet` or a hex-encoded key) rather than `fetch`. `--proxy` is not supported. - * `icp message send` shows what the message contains — sender, canister, method, decoded argument, window, and destination — and asks before submitting. `--yes` skips the prompt, and a non-TTY proceeds without one so a scripted courier works. `--dry-run` prints the same summary and stops without touching the network at all, which makes it the file-inspection command. `--network` / `--root-key` override where the file says to submit, and `--candid`, `--output` and `--json` render the reply exactly as `icp canister call` does. + * `icp message send` shows what the message contains — sender, canister, method, decoded argument, window, and destination — and asks before submitting. `--yes` skips the prompt, and a non-TTY proceeds without one so a scripted courier works. `--dry-run` prints the same summary and stops without touching the network at all, which makes it the file-inspection command. `--candid`, `--output` and `--json` render the reply exactly as `icp canister call` does. Where the message is submitted comes from the file alone — there is no `--network` override, so no environment variable can silently redirect a signed message; a courier who has to change it edits the file's `network` field, which is unauthenticated in either case. * If sending fails after the message may already have gone out, re-run `icp message send` **on the same file**: the request id is a hash of the signed content, so resubmitting the identical message is de-duplicated by the IC and cannot execute twice. Signing again produces a new expiry, hence a different request id, which is *not* de-duplicated — for a transfer, a double spend. Every post-submission failure says so. * `--valid-from ` places the message's submission window, as a duration from now (`55m`, `2h`) or an RFC 3339 timestamp; it defaults to now. The window is always five minutes wide, because the IC rejects an ingress message whose expiry is further ahead than that — so this places the window rather than sizing it. Note that this is not dfx's `--expire-after`, which names the window's *end* and leaves you to subtract the five minutes yourself. * The file records the signed envelope, where to submit it, a tagged canister-or-subnet destination, the Candid interface, and a human-readable summary of what was signed. An update also carries a pre-signed `request_status` read, so the submitting machine can await the outcome with no key of its own; it shares the call's expiry, so both live in the same window. diff --git a/crates/icp-cli/src/commands/message/send.rs b/crates/icp-cli/src/commands/message/send.rs index 703f38ddf..c3aa8d741 100644 --- a/crates/icp-cli/src/commands/message/send.rs +++ b/crates/icp-cli/src/commands/message/send.rs @@ -2,7 +2,7 @@ use anyhow::{Context as _, bail}; use candid::{IDLArgs, TypeEnv, types::Function}; use clap::{Args, ValueHint}; use ic_agent::agent::CallResponse; -use icp::context::{Context, IC_ROOT_KEY, NetworkSelection}; +use icp::context::{Context, IC_ROOT_KEY}; use icp::identity::IdentitySelection; use icp::network::RootKeySpec; use icp::prelude::*; @@ -18,7 +18,6 @@ use url::Url; use crate::operations::call_output::{ CallOutputMode, CanisterInterface, get_candid_type, load_candid_from_file, print_response, }; -use crate::options::NetworkOpt; /// Submit a message signed on another machine /// @@ -26,20 +25,17 @@ use crate::options::NetworkOpt; /// contains, submits it, and waits for the reply. No identity is used and none /// is needed: the message was already signed by whoever composed it, so this /// machine only has to carry it to the network. +/// +/// It is submitted to the network the file names. If that has to change — the +/// signing machine recorded a URL this one cannot reach, say — edit `network` in +/// the file: the envelope is signed and carries no URL of its own, so where it +/// goes cannot change what executes. #[derive(Args, Debug)] pub(crate) struct SendArgs { /// The signed message file. `-` reads stdin. #[arg(value_hint = ValueHint::FilePath)] pub(crate) file: PathBuf, - /// Where to submit, overriding the network recorded in the file. - /// - /// Useful when the signing machine recorded a URL this one cannot reach. - /// The envelope is signed and carries no URL of its own, so redirecting it - /// cannot change what executes — only whether it is accepted. - #[command(flatten)] - pub(crate) network: NetworkOpt, - /// Show what the message contains and exit without submitting it. #[arg(long)] pub(crate) dry_run: bool, @@ -78,7 +74,14 @@ pub(crate) async fn exec(ctx: &Context, args: &SendArgs) -> Result<(), anyhow::E refuse_unsubmittable(&validated)?; } - let (url, root_key) = resolve_network(ctx, args, &message).await?; + // Where to submit comes from the file and nowhere else. There is deliberately + // no `--network` override: the signer already decided, a courier who genuinely + // has to redirect can edit `network` in the file (it is unauthenticated + // either way, so that is within the trust model rather than around it), and a + // flag here would inherit `ICP_NETWORK` from the environment — which would let + // a stray shell variable silently send a message somewhere else. + let url = &message.network.url; + let root_key = &message.network.root_key; // `--dry-run` is the file-inspection command, so it stays entirely offline: // no agent, no root key, and no fetching an interface the file did not carry. @@ -86,9 +89,9 @@ pub(crate) async fn exec(ctx: &Context, args: &SendArgs) -> Result<(), anyhow::E true => None, false => { let agent = ctx - .get_agent_for_url(&IdentitySelection::Anonymous, &url) + .get_agent_for_url(&IdentitySelection::Anonymous, url) .await?; - apply_root_key(&agent, &root_key).await?; + apply_root_key(&agent, root_key).await?; Some(agent) } }; @@ -98,7 +101,7 @@ pub(crate) async fn exec(ctx: &Context, args: &SendArgs) -> Result<(), anyhow::E .as_ref() .and_then(|i| Some((i.env.clone(), i.get_method(&validated.method)?.clone()))); - print_summary(&message, &validated, &url, declared_method.as_ref()); + print_summary(&message, &validated, url, declared_method.as_ref()); if args.dry_run { eprintln!("Not submitted: this was a --dry-run."); @@ -178,24 +181,6 @@ fn load(path: &Path) -> Result { serde_json::from_str(&buf).context("failed to parse the signed message read from stdin") } -/// Where to submit: the file's network, unless this machine was told otherwise. -async fn resolve_network( - ctx: &Context, - args: &SendArgs, - message: &SignedMessage, -) -> Result<(Url, RootKeySpec), anyhow::Error> { - let selection: NetworkSelection = args.network.clone().into(); - if selection == NetworkSelection::Default { - return Ok(( - message.network.url.clone(), - message.network.root_key.clone(), - )); - } - let network = ctx.get_network(&selection).await?; - let access = ctx.network.access(&network).await?; - Ok((access.api_url, RootKeySpec::Explicit(access.root_key))) -} - /// The reply's certificate is verified against this, so it is the one piece of /// configuration that decides whether an answer can be believed. async fn apply_root_key( diff --git a/crates/icp-cli/tests/message_send_tests.rs b/crates/icp-cli/tests/message_send_tests.rs index 97f715a5d..1393ada6e 100644 --- a/crates/icp-cli/tests/message_send_tests.rs +++ b/crates/icp-cli/tests/message_send_tests.rs @@ -198,6 +198,58 @@ fn dry_run_inspects_without_sending() { .stderr(contains("Not submitted")); } +/// Where a message goes is the file's business. A courier who genuinely has to +/// redirect one edits the file — `network` is unauthenticated either way, so that +/// is inside the trust model. Nothing outside the file can change it, and in +/// particular `ICP_NETWORK` cannot: a stray shell variable silently sending a +/// signed message to mainnet is exactly the accident there is no flag for. +#[test] +fn only_the_file_says_where_to_submit() { + let ctx = TestContext::new(); + let did = ctx.home_path().join("service.did"); + write_string(&did, GREET_DID).expect("write candid"); + let msg = ctx.home_path().join("redirect.json"); + + ctx.icp() + .args([ + "canister", + "call", + "--network", + "http://127.0.0.1:9999", + "--root-key", + "mainnet", + "--candid", + did.as_str(), + "--sign-only", + msg.as_str(), + "ryjl3-tyaaa-aaaaa-aaaba-cai", + "greet", + "(\"world\")", + ]) + .assert() + .success(); + + // An edited network is honoured, and the message still validates: the + // envelope is signed and carries no URL, so this cannot change what executes. + let mut file: Value = + serde_json::from_str(&icp::fs::read_to_string(&msg).expect("read")).expect("JSON"); + file["network"]["url"] = Value::String("http://127.0.0.1:1234/".into()); + write_string( + &msg, + &serde_json::to_string_pretty(&file).expect("serialize"), + ) + .expect("write"); + + ctx.icp() + .args(["message", "send", msg.as_str(), "--dry-run"]) + .env("ICP_NETWORK", "ic") + .assert() + .success() + .stderr(contains("Network: http://127.0.0.1:1234/")) + // Not mainnet, which is where `ICP_NETWORK=ic` would have pointed it. + .stderr(contains("icp-api.io").not()); +} + /// The summary is display-only, so a file whose summary disagrees with its /// signed envelope is refused rather than quietly believed either way. #[test] diff --git a/docs/reference/cli.md b/docs/reference/cli.md index f6feebac1..6aaf27b3d 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -1364,6 +1364,8 @@ Submit a message signed on another machine Takes a file written by `icp canister call --sign-only`, shows what it contains, submits it, and waits for the reply. No identity is used and none is needed: the message was already signed by whoever composed it, so this machine only has to carry it to the network. +It is submitted to the network the file names. If that has to change — the signing machine recorded a URL this one cannot reach, say — edit `network` in the file: the envelope is signed and carries no URL of its own, so where it goes cannot change what executes. + **Usage:** `icp message send [OPTIONS] ` ###### **Arguments:** @@ -1372,8 +1374,6 @@ Takes a file written by `icp canister call --sign-only`, shows what it contains, ###### **Options:** -* `-n`, `--network ` — Name or URL of the network to target, conflicts with environment argument -* `-k`, `--root-key ` — The root key to use if connecting to a network by URL. Required when using `--network `. One of `mainnet`, `fetch`, or a 266-character hex-encoded root key * `--dry-run` — Show what the message contains and exit without submitting it * `-y`, `--yes` — Submit without asking for confirmation * `--candid ` — Path to a Candid (`.did`) file describing the canister's interface, overriding the one embedded in the message From 3acb7f684010e45bc440eda98ea64010807099e3 Mon Sep 17 00:00:00 2001 From: Linwei Shang Date: Thu, 20 Aug 2026 10:49:41 -0400 Subject: [PATCH 3/3] fix: do not let a message's own interface understate its argument MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found that "display only" was doing more work than §4.4 claimed. The embedded Candid interface is unauthenticated, and Candid's record subtyping lets a narrower one decode the very same bytes while dropping fields — so a doctored `.did` renders `(record { to = "alice" })` for an argument that also says `amount = 1_000`, with the envelope, summary and request id all still validating. The operator approves what they were shown, which is not what was signed. `send` now cross-checks against the untyped decode. That reads the argument's own type table, which is part of the signed bytes, so it cannot be tampered with and cannot omit anything; when the readable rendering surfaces fewer values, both are shown and the readable one is flagged. The honest case is unchanged and unadorned. Four smaller findings from the same review: - The confirmation prompt can sit open past the window, which then spent a round trip on a message known to be stale. Re-checked after the prompt. - The clock-drift hint was unreachable when expired: measured against `valid_from`, the expired side is always more than a whole window away. It now measures whichever edge was missed. - A reply that arrived but could not be rendered left the riskiest case — an executed transfer — without the do-not-re-sign warning that the changelog promises for every post-submission failure. - Recovery advice told a `-` caller to re-run a command whose stdin is already consumed; it now asks for the same bytes instead. The pasteable path in `--sign-only`'s output goes through `shell_quote`. A fifth was already answered by dropping `--network`: nothing on the `--dry-run` path can resolve a root key any more, so it cannot fetch one. Co-Authored-By: Claude Opus 5 (1M context) --- crates/icp-cli/src/commands/canister/call.rs | 7 +- crates/icp-cli/src/commands/message/send.rs | 131 +++++++++++++++---- crates/icp-cli/tests/message_send_tests.rs | 71 +++++++++- 3 files changed, 178 insertions(+), 31 deletions(-) diff --git a/crates/icp-cli/src/commands/canister/call.rs b/crates/icp-cli/src/commands/canister/call.rs index 4de112681..4b8ea59c9 100644 --- a/crates/icp-cli/src/commands/canister/call.rs +++ b/crates/icp-cli/src/commands/canister/call.rs @@ -23,6 +23,7 @@ use crate::{ operations::call_output::{ CallOutputMode, CanisterInterface, get_candid_type, load_candid_from_file, print_response, }, + operations::create::shell_quote, operations::proxy::update_or_proxy_raw, operations::wasm::extract_candid_service, }; @@ -470,7 +471,11 @@ async fn sign_only( ); match out.as_str() { "-" => eprintln!("Submit it with: icp message send "), - path => eprintln!("Written to {path}. Submit it with: icp message send {path}"), + // Quoted: a path with a space in it would otherwise paste as two arguments. + path => eprintln!( + "Written to {path}. Submit it with: icp message send {}", + shell_quote(path) + ), } if interface.is_none() { warn!( diff --git a/crates/icp-cli/src/commands/message/send.rs b/crates/icp-cli/src/commands/message/send.rs index c3aa8d741..29f0c63d5 100644 --- a/crates/icp-cli/src/commands/message/send.rs +++ b/crates/icp-cli/src/commands/message/send.rs @@ -18,6 +18,7 @@ use url::Url; use crate::operations::call_output::{ CallOutputMode, CanisterInterface, get_candid_type, load_candid_from_file, print_response, }; +use crate::operations::create::shell_quote; /// Submit a message signed on another machine /// @@ -114,6 +115,11 @@ pub(crate) async fn exec(ctx: &Context, args: &SendArgs) -> Result<(), anyhow::E return Ok(()); } + // The prompt above can sit open for as long as it likes, so the window is + // checked again rather than spending a round trip on a message that expired + // while it waited. + refuse_unsubmittable(&message.validate(OffsetDateTime::now_utc())?)?; + let effective_id = message.destination.to_effective_id(); let response = match validated.call_type { CallType::Query => agent @@ -124,7 +130,12 @@ pub(crate) async fn exec(ctx: &Context, args: &SendArgs) -> Result<(), anyhow::E let submitted = agent .update_signed(effective_id, message.request.envelope.clone()) .await - .with_context(|| resubmit_advice(&args.file))?; + .with_context(|| { + resubmit_advice( + &args.file, + "the message may already have reached the network", + ) + })?; match submitted { // The synchronous call path already returned a certified reply. CallResponse::Response(reply) => reply, @@ -140,13 +151,29 @@ pub(crate) async fn exec(ctx: &Context, args: &SendArgs) -> Result<(), anyhow::E .wait_signed(&request_id, effective_id, status_check) .await .map(|(reply, _cert)| reply) - .with_context(|| resubmit_advice(&args.file))? + .with_context(|| { + resubmit_advice( + &args.file, + "the call was submitted; waiting for its outcome failed", + ) + })? } } } }; - print_response(&response, args.output, declared_method.as_ref(), args.json) + print_response(&response, args.output, declared_method.as_ref(), args.json).map_err(|e| { + match validated.call_type { + // Nothing executed, so a rendering failure is just that. + CallType::Query => e, + // The call ran and its reply is in hand — this is the case where + // someone is most likely to reach for signing it again. + CallType::Update => e.context(resubmit_advice( + &args.file, + "the call executed and its reply arrived, but could not be rendered", + )), + } + }) } /// Refuses a message that is outside its submission window, naming the window so @@ -258,12 +285,15 @@ fn report_window(validated: &Validated) { ), } // The window is computed from the signing machine's clock, and an offline - // machine is exactly the kind that drifts. - if matches!( - validated.window, - WindowState::NotYetValid | WindowState::Expired - ) && (validated.valid_from - now).abs() < SUBMISSION_WINDOW - { + // machine is exactly the kind that drifts. Measured against whichever edge + // was missed: against `valid_from` the expired side is always more than a + // whole window away, so this would never have fired there. + let overshoot = match validated.window { + WindowState::NotYetValid => Some(validated.valid_from - now), + WindowState::Expired => Some(now - validated.valid_until), + WindowState::Valid => None, + }; + if overshoot.is_some_and(|by| by < SUBMISSION_WINDOW) { eprintln!( "It is only just outside the window, which usually means the signing machine's \ clock is off rather than that you are early or late." @@ -299,19 +329,51 @@ fn print_summary( eprintln!(); } -/// The argument decoded, or hex if there is no interface to decode it with. -/// A prompt showing a hex blob is not something anyone can review. +/// The argument as the operator should see it before approving anything. +/// +/// Rendered through the interface when there is one, because a hex blob makes the +/// prompt useless for review. But the interface usually comes out of the message +/// itself and is not authenticated, and Candid's record subtyping lets a narrower +/// one decode the very same bytes while silently dropping fields — a doctored +/// `.did` can show `(record { to = "alice" })` for an argument that also says +/// `amount = 1_000`. +/// +/// So the untyped decode is used as a cross-check. It reads the argument's own +/// type table, which is part of the signed bytes and so cannot be tampered with, +/// meaning it can never omit anything. When the two disagree about how much is +/// there, both are shown and the readable one is not to be trusted. fn render_argument(arg: &[u8], declared_method: Option<&(TypeEnv, Function)>) -> String { - let decoded = match declared_method { - Some((env, func)) => IDLArgs::from_bytes_with_types(arg, env, &func.args).ok(), - None => IDLArgs::from_bytes(arg).ok(), - }; - match decoded { - Some(args) => format!("{args}"), - None => format!("{} (hex, could not decode)", hex::encode(arg)), + let untyped = IDLArgs::from_bytes(arg).ok(); + let typed = declared_method + .and_then(|(env, func)| IDLArgs::from_bytes_with_types(arg, env, &func.args).ok()); + + match (typed, untyped) { + (Some(typed), Some(untyped)) if values_in(&typed) < values_in(&untyped) => format!( + "{typed}\n WARNING: the interface in this message renders less than the \ + signed argument contains.\n The signed bytes say: {untyped}" + ), + (Some(typed), _) => format!("{typed}"), + (None, Some(untyped)) => format!("{untyped}"), + (None, None) => format!("{} (hex, could not decode)", hex::encode(arg)), } } +/// Counts the values a decode actually surfaced, so a rendering that drops record +/// fields can be told apart from one that shows everything. +fn values_in(args: &IDLArgs) -> usize { + fn walk(value: &candid::IDLValue) -> usize { + use candid::IDLValue; + 1 + match value { + IDLValue::Record(fields) => fields.iter().map(|f| walk(&f.val)).sum(), + IDLValue::Vec(values) => values.iter().map(walk).sum(), + IDLValue::Opt(inner) => walk(inner), + IDLValue::Variant(v) => walk(&v.0.val), + _ => 0, + } + } + args.args.iter().map(walk).sum() +} + fn confirm() -> Result { // A courier is expected to be scripted, and blocking on a prompt nobody can // answer is worse than submitting a message its author already reviewed. @@ -325,17 +387,30 @@ fn confirm() -> Result { .context("failed to read confirmation") } -/// What to do when a submission fails after the message may already have been -/// sent. Re-running is safe; re-signing is not. -fn resubmit_advice(file: &Path) -> String { +/// What to do when something fails after the message may already have reached +/// the network. Re-submitting the identical message is safe; signing a new one is +/// not. +/// +/// `situation` says what is known to have happened, because the two cases differ: +/// a failed submission may or may not have executed, while a reply that could not +/// be rendered certainly did. +fn resubmit_advice(file: &Path, situation: &str) -> String { + // `-` has already consumed its input, so telling the caller to re-run the + // same command would send them at a pipe that is empty — or leave them + // waiting on one that never closes. + let rerun = match file.as_str() { + "-" => "feed the identical bytes to `icp message send -` again (you need the message \ + you piped in; save it to a file if you no longer have it)" + .to_string(), + path => format!("re-run `icp message send {}`", shell_quote(path)), + }; format!( - "the message may already have reached the network. Re-run `icp message send {file}` on \ - the same file — the request id is a hash of the signed content, so resubmitting the \ - identical message is de-duplicated by the IC and cannot execute twice. Do NOT sign it \ - again: a new signature means a new expiry, hence a different request id, which is NOT \ - de-duplicated. Two limits worth knowing: once the reply is pruned from the ingress \ - history the outcome can no longer be recovered (the call still executed), and once the \ - window closes the message cannot be submitted at all" + "{situation}. To recover, {rerun} — the request id is a hash of the signed content, so \ + resubmitting the identical message is de-duplicated by the IC and cannot execute twice. \ + Do NOT sign it again: a new signature means a new expiry, hence a different request id, \ + which is NOT de-duplicated. Two limits worth knowing: once the reply is pruned from the \ + ingress history the outcome can no longer be recovered (the call still executed), and \ + once the window closes the message cannot be submitted at all" ) } diff --git a/crates/icp-cli/tests/message_send_tests.rs b/crates/icp-cli/tests/message_send_tests.rs index 1393ada6e..13de7114e 100644 --- a/crates/icp-cli/tests/message_send_tests.rs +++ b/crates/icp-cli/tests/message_send_tests.rs @@ -294,6 +294,71 @@ fn tampered_file_is_refused() { .stderr(contains("summary does not match the signed request")); } +/// The interface a message carries is unauthenticated, and Candid's record +/// subtyping lets a narrower one decode the same bytes while dropping fields — so +/// a courier could otherwise show an operator an argument that understates what +/// they are approving. The untyped decode comes from the argument's own type +/// table, which is part of the signed bytes, so it cannot be made to omit +/// anything and is used to catch exactly this. +#[test] +fn a_doctored_interface_cannot_hide_the_signed_argument() { + let ctx = TestContext::new(); + let did = ctx.home_path().join("transfer.did"); + write_string( + &did, + r#"service : { "transfer" : (record { to : text; amount : nat }) -> () }"#, + ) + .expect("write candid"); + let msg = ctx.home_path().join("transfer.json"); + + ctx.icp() + .args([ + "canister", + "call", + "--network", + "http://127.0.0.1:1", + "--root-key", + "mainnet", + "--candid", + did.as_str(), + "--sign-only", + msg.as_str(), + "ryjl3-tyaaa-aaaaa-aaaba-cai", + "transfer", + "(record { to = \"alice\"; amount = 1000 : nat })", + ]) + .assert() + .success(); + + // Honest file: the amount is shown, and nothing is flagged. + ctx.icp() + .args(["message", "send", msg.as_str(), "--dry-run"]) + .assert() + .success() + .stderr(contains("amount = 1_000")) + .stderr(contains("WARNING").not()); + + // Swap in an interface that declares only `to`. It still decodes, and the + // envelope and summary still validate — nothing about the file is invalid. + let mut file: Value = + serde_json::from_str(&icp::fs::read_to_string(&msg).expect("read")).expect("JSON"); + file["candid"] = + Value::String(r#"service : { "transfer" : (record { to : text }) -> () }"#.into()); + write_string( + &msg, + &serde_json::to_string_pretty(&file).expect("serialize"), + ) + .expect("write"); + + ctx.icp() + .args(["message", "send", msg.as_str(), "--dry-run"]) + .assert() + .success() + .stderr(contains("renders less than the signed argument contains")) + // The hidden value is surfaced from the signed bytes, by field hash. + .stderr(contains("1_000")); +} + /// A message whose window has not opened is refused with the opening time, so /// the operator knows to wait rather than to re-sign. Refused before anything /// touches the network: the URL here is unreachable. @@ -348,8 +413,9 @@ fn expired_file_is_refused() { let ctx = TestContext::new(); let canister = candid::Principal::from_text("ryjl3-tyaaa-aaaaa-aaaba-cai").expect("principal"); - // A window that closed ten minutes ago. - let valid_until = (OffsetDateTime::now_utc() - time::Duration::minutes(10)) + // A window that closed two minutes ago: close enough that the drift note + // applies, which measured against the wrong edge it never was. + let valid_until = (OffsetDateTime::now_utc() - time::Duration::minutes(2)) .replace_nanosecond(0) .and_then(|t| t.replace_second(0)) .expect("0 is a valid second and nanosecond"); @@ -402,5 +468,6 @@ fn expired_file_is_refused() { .failure() .stderr(contains("Expired")) .stderr(contains(&format_timestamp(valid_until)[..])) + .stderr(contains("clock is off")) .stderr(contains("signed again")); }