diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 25c8a1bb0..540c504ba 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -12,6 +12,10 @@ cargo test # Run all tests (launcher auto-downloads on cargo test -p icp-cli # Tests for a specific package cargo test --test -- # Specific test cargo fmt && cargo clippy # Run after changes pass tests +# `icp-project`'s core must reach nothing only a host has — see the boundary below +cargo clippy -p icp-project --no-default-features --target wasm32-unknown-unknown +./scripts/check-no-host-reach.sh # ...which a build alone does not prove +cargo test -p icp-project --no-default-features ./scripts/generate-cli-docs.sh # Regenerate CLI docs when commands change ./scripts/generate-config-schemas.sh # Regenerate schema when manifest types change ``` @@ -52,14 +56,38 @@ reverse: - `network::Access` — a network's endpoints, root key and friendly domains - `canister::wasm::Fetch` — a wasm module a manifest names by URL - `canister::recipe::Resolve` — a recipe's Handlebars template +- `canister::build::Build` — running one build step - `canister::sync::plugin::Run` — running one sync plugin - `canister::sync::script::ScriptRunner` — running one sync script - `store_id::Access` / `store_artifact::Access` — the project's `.icp` stores +- `random::Random` — a choice that cannot be made by arithmetic - `host::Observe` — what resolution turned up, for telemetry -Host implementations of the first, the last two stores and the script runner -ship in `icp-project` itself behind the default-on `host` feature; the rest -have no implementation there at all. +Some of those have a host implementation in `icp-project` itself, behind the +default-on **`host`** feature: `HostFileSystem`, the two stores, `HostScripts`, +`HostRandom`, and `Builder`. The rest have none there at all. + +`host` must be **strictly additive** — it may add implementations, never change +what the rest of the crate does; if it changed behaviour rather than adding to +it, the checks below would prove nothing about the shipped binary. CI runs +three of them on the core with default features off: + +- a build for `wasm32-unknown-unknown`, which rejects a *dependency* that + reaches the host by gating that code on `cfg(unix)`/`cfg(windows)`, leaving + it with nothing to compile; +- `scripts/check-no-host-reach.sh`, which reads the imports back out of that + build's object files and rejects a call this crate itself makes into + `std::fs`, `std::process`, `std::env` and their neighbours — and, since a + dependency that reaches the host ungated compiles fine, the front doors onto + those that such a dependency offers, `camino`'s `Utf8Path::is_file` among + them. The build alone catches neither: `wasm32-unknown-unknown` ships a full + `std` whose host backends compile and fail at runtime; +- the crate's own tests, which are the only thing that exercises it with + `host` off, and so run in that configuration too. + +Whole modules that are irreducibly host-side sit behind it too — +`operations::bundle` writes a `.tar.gz` and walks a directory tree for +symlinks, which no seam over `FileSystem` reproduces faithfully. Because those are implemented across a crate boundary, their error types carry their cause boxed and pass it through with `#[snafu(transparent)]`, which leaves diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 076741383..cf486dc75 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -84,6 +84,89 @@ jobs: env: RUST_BACKTRACE: 1 + # `icp-project` is meant to end up runnable inside a canister, so its core + # must reach nothing that only a host has. Nothing else expresses this: a + # `--workspace` build unifies the `host` feature back on, because `icp-cli` + # enables it. So this builds that one crate, alone, for the target it would + # run on, and then reads back what the build left behind. + canister-target: + name: canister-target:required + needs: [changes, compile] + if: needs.changes.outputs.src == 'true' + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + + - name: Setup image (Linux) + run: ./.github/scripts/provision-linux-build.sh + + # rust-cache hashes all installed toolchains; the runner image's `stable` + # drifts as the image updates, which moves the cache key and causes misses. + # Remove it so only the rust-toolchain.toml-pinned version remains. + - name: Remove the runner's bundled Rust toolchain + run: rustup toolchain remove stable 2>/dev/null || true + + - uses: actions-rust-lang/setup-rust-toolchain@150fca883cd4034361b621bd4e6a9d34e5143606 # v1.15.4 + with: + target: wasm32-unknown-unknown + # `llvm-nm` reads the imports back out of the objects the build + # leaves behind; see scripts/check-no-host-reach.sh. + components: llvm-tools + cache-shared-key: ${{ runner.os }}-canister-target + cache-bin: false + + # Catches a dependency that reaches the host, which fails to compile + # here. Only the library: the dev-dependencies behind the tests serve a + # host by definition, and `httptest` does not build for wasm at all. + - name: Check icp-project without a host + run: | + cargo clippy -p icp-project --no-default-features \ + --target wasm32-unknown-unknown -- -D warnings + env: + RUST_BACKTRACE: 1 + + # Catches the crate's own code reaching the host, which compiles here: + # `wasm32-unknown-unknown` ships a full `std` whose host backends fail at + # runtime rather than at build time. + - name: Check the core reaches no host + run: ./scripts/check-no-host-reach.sh + + # The tests are the only thing that exercises the crate with `host` off, + # so they run in that configuration too — on this machine's target, + # since that is where a test can run at all. + - name: Test icp-project without a host + run: cargo test -p icp-project --no-default-features + env: + RUST_BACKTRACE: 1 + + # The checks above prove the core reaches no host. This proves it reaches + # no *crate above it* either, which no build would catch, because a cycle + # is what cargo would report and there is no cycle to report until + # someone writes one. + - name: Check the dependency direction + run: | + fail() { echo "::error::$1"; exit 1; } + # grep exits 1 when it found nothing and 2 or more when it could not + # read a file. Only the first is a pass: a guard that quietly stops + # guarding once a path moves is worse than no guard at all. + forbid() { + local message=$1 pattern=$2 + shift 2 + local matches status + matches=$(grep -rn "$pattern" "$@") && status=0 || status=$? + case $status in + 0) echo "$matches"; fail "$message" ;; + 1) ;; + *) fail "could not search $* for '$pattern'" ;; + esac + } + forbid "icp-project and icp-sync-plugin must not depend on icp-app" \ + 'icp-app' crates/icp-project/Cargo.toml crates/icp-sync-plugin/Cargo.toml + forbid "icp-app must not re-export icp-project: a command reaches for the crate that owns what it needs" \ + 'pub use icp_project' crates/icp-app/src + echo "the boundary holds" + format: name: fmt:required needs: [changes, compile] diff --git a/Cargo.lock b/Cargo.lock index 2eb4681e0..b64aac00a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3827,7 +3827,6 @@ dependencies = [ "schemars", "semver", "serde", - "serde_cbor", "serde_json", "serde_yaml", "sha2 0.11.0", diff --git a/Cargo.toml b/Cargo.toml index 903d4b944..6840e3254 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -109,7 +109,9 @@ tempfile = "3" test-tag = "0.1" time = { version = "0.3.47", features = ["formatting", "macros", "parsing"] } tiny-bip39 = "2.0.0" -tokio = { version = "1.45.0", features = ["macros", "rt-multi-thread"] } +# The most any crate here can have in common: `rt-multi-thread` is not +# available on wasm, so the crates that need it ask for it themselves. +tokio = { version = "1.45.0", features = ["macros"] } tracing = "0.1.41" tracing-subscriber = "0.3.20" url = { version = "2.5.4", features = ["serde"] } diff --git a/crates/icp-app/src/context/init.rs b/crates/icp-app/src/context/init.rs index fce33938c..b0dc0bf0d 100644 --- a/crates/icp-app/src/context/init.rs +++ b/crates/icp-app/src/context/init.rs @@ -167,6 +167,7 @@ pub fn initialize( syncer, wasm, network: netaccess.clone(), + random: Arc::new(icp_project::random::HostRandom), observer: telemetry_data.clone(), }, dirs, diff --git a/crates/icp-cli/Cargo.toml b/crates/icp-cli/Cargo.toml index 47215ff6a..319d5e1e3 100644 --- a/crates/icp-cli/Cargo.toml +++ b/crates/icp-cli/Cargo.toml @@ -67,7 +67,7 @@ snafu.workspace = true sysinfo.workspace = true tiny-bip39.workspace = true time.workspace = true -tokio.workspace = true +tokio = { workspace = true, features = ["rt-multi-thread"] } tracing-subscriber.workspace = true tracing.workspace = true url.workspace = true diff --git a/crates/icp-cli/src/commands/canister/create.rs b/crates/icp-cli/src/commands/canister/create.rs index 36d814f37..a4d533a0d 100644 --- a/crates/icp-cli/src/commands/canister/create.rs +++ b/crates/icp-cli/src/commands/canister/create.rs @@ -7,6 +7,7 @@ use clap::{ArgGroup, Args, Parser}; use ic_management_canister_types::CanisterSettings as MgmtCanisterSettings; use icp_app::context::{Context, NetworkSelection}; use icp_app::identity::IdentitySelection; +use icp_canister_interfaces::engine_canister::engine_canister_id; use icp_project::canister::resolve_controllers; use icp_project::host::EnvironmentSelection; use icp_project::parsers::{CyclesAmount, DurationAmount, MemoryAmount, parse_token_amount}; @@ -307,8 +308,14 @@ async fn create_canister(ctx: &Context, args: &CreateArgs) -> Result<(), anyhow: let calls = icp_app::calls::calls(agent.clone(), args.proxy)?; - let create_operation = - CreateOperation::new(calls, args.create_target(), args.funding(), vec![]); + let create_operation = CreateOperation::new( + calls, + ctx.host.random.clone(), + args.create_target(), + args.funding(), + vec![], + engine_canister_id().map_err(|message| anyhow!(message))?, + ); let canister_settings = args.canister_settings(); @@ -376,9 +383,11 @@ async fn create_project_canister(ctx: &Context, args: &CreateArgs) -> Result<(), let create_operation = CreateOperation::new( calls.clone(), + ctx.host.random.clone(), args.create_target(), args.funding(), ids.values().copied().collect(), + engine_canister_id().map_err(|message| anyhow!(message))?, ); let (canister_settings, unresolved) = diff --git a/crates/icp-cli/src/commands/deploy.rs b/crates/icp-cli/src/commands/deploy.rs index ffe2c5692..5d57438b0 100644 --- a/crates/icp-cli/src/commands/deploy.rs +++ b/crates/icp-cli/src/commands/deploy.rs @@ -1,9 +1,10 @@ -use anyhow::bail; +use anyhow::{anyhow, bail}; use candid::Principal; use clap::Args; use clap_complete::ArgValueCandidates; use icp_app::{context::Context, identity::IdentitySelection}; use icp_canister_interfaces::candid_ui::MAINNET_CANDID_UI_CID; +use icp_canister_interfaces::engine_canister::engine_canister_id; use icp_project::calls::{Call, CallError, CanisterCalls}; use icp_project::operations::deploy::{DeployParams, DeployReport, deploy, resolve_targets}; use icp_project::parsers::CyclesAmount; @@ -135,6 +136,7 @@ pub(crate) async fn exec(ctx: &Context, args: &DeployArgs) -> Result<(), anyhow: subnet: args.subnet, proxy: args.proxy, cycles: args.cycles.get(), + engine_registry: engine_canister_id().map_err(|message| anyhow!(message))?, no_create: args.no_create, yes: args.yes, args: args.args_opt.resolve_bytes()?, diff --git a/crates/icp-project/Cargo.toml b/crates/icp-project/Cargo.toml index ecc8c42ba..70abb7dfd 100644 --- a/crates/icp-project/Cargo.toml +++ b/crates/icp-project/Cargo.toml @@ -8,16 +8,16 @@ publish.workspace = true [features] default = ["host"] # Implementations of this crate's seams that use the machine it is running on: -# the filesystem and the project-local `.icp` stores. Turned off for a build -# that has to run somewhere without them, such as inside a canister. +# the filesystem, the project-local `.icp` stores, subprocesses, randomness. +# Turned off for a build that has to run somewhere without them, such as inside +# a canister — which CI checks by building this crate for +# `wasm32-unknown-unknown` with default features off, reading the host calls +# back out of that build with `scripts/check-no-host-reach.sh`, and running the +# tests in the same configuration. # -# It does not yet cover everything host-shaped here — the step runners still -# spawn subprocesses and drive a wasmtime sandbox unconditionally, `create` -# draws entropy from `rand`, and a bundle's plugin directories go through -# `tar`'s own directory walk. Each is a seam of its own to draw, and every -# dependency is still non-optional besides, so this is the boundary the feature -# claims rather than a canister target. -host = [] +# Strictly additive: it may add implementations, never change what the rest of +# the crate does, or those checks prove nothing about the shipped binary. +host = ["dep:camino-tempfile", "dep:dunce", "dep:rand", "dep:tar", "tokio/io-std", "tokio/process", "tokio/signal"] # Exposes this crate's mocks and fixtures so downstream crates can test against # the same seams. Off in a normal build, so none of it ships. test-util = [] @@ -26,11 +26,11 @@ test-util = [] async-trait = { workspace = true } bigdecimal = { workspace = true } camino = { workspace = true } -camino-tempfile = { workspace = true } +camino-tempfile = { workspace = true, optional = true } candid = { workspace = true } candid_parser = { workspace = true } clap = { workspace = true, optional = true } -dunce = { workspace = true } +dunce = { workspace = true, optional = true } flate2 = { workspace = true } futures = { workspace = true } glob = { workspace = true } @@ -48,22 +48,21 @@ num-bigint = { workspace = true } num-integer = { workspace = true } num-traits = { workspace = true } pathdiff = { workspace = true } -rand = { workspace = true } +rand = { workspace = true, optional = true } schemars = { workspace = true } semver = { workspace = true } serde = { workspace = true } -serde_cbor = { workspace = true } serde_json = { workspace = true } serde_yaml = { workspace = true } sha2 = { workspace = true } shellwords = { workspace = true } snafu = { workspace = true } strum = { workspace = true } -tar = { workspace = true } +tar = { workspace = true, optional = true } time = { workspace = true } -# `io-std` is listed for `tokio::io::stdout`/`stderr`; feature unification with other -# crates supplies it anyway, so dropping it would not fail the build. -tokio = { workspace = true, features = ["sync", "macros", "rt", "time", "io-util", "io-std", "process", "signal"] } +# Everything a canister build can have: no `process`, `signal` or `io-std`, +# which the `host` feature adds. +tokio = { workspace = true, features = ["sync", "macros", "rt", "time", "io-util"] } tracing = { workspace = true } url = { workspace = true } wasmparser = { workspace = true } @@ -72,5 +71,8 @@ wasmparser = { workspace = true } winreg = { workspace = true } [dev-dependencies] +# Also an optional dependency above, for `host`'s `scratch_dir`. Unconditional +# here so the tests build with `host` off, which is the configuration CI checks. +camino-tempfile = { workspace = true } httptest = { workspace = true } jsonschema = { workspace = true } diff --git a/crates/icp-project/src/canister/build/mod.rs b/crates/icp-project/src/canister/build/mod.rs index 7cefb961a..b51deec45 100644 --- a/crates/icp-project/src/canister/build/mod.rs +++ b/crates/icp-project/src/canister/build/mod.rs @@ -3,13 +3,18 @@ use async_trait::async_trait; use icp_events::StepReporter; use snafu::prelude::*; +#[cfg(feature = "host")] use std::sync::Arc; +#[cfg(feature = "host")] use crate::canister::wasm; use crate::manifest::canister::BuildStep; use crate::prelude::*; +// Both halves of the one implementation below, which is host-side. +#[cfg(feature = "host")] mod prebuilt; +#[cfg(feature = "host")] mod script; pub struct Params { @@ -20,10 +25,30 @@ pub struct Params { #[derive(Debug, Snafu)] pub enum BuildError { + #[cfg(feature = "host")] #[snafu(transparent)] Script { source: super::script::ScriptError }, + #[cfg(feature = "host")] #[snafu(transparent)] Prebuilt { source: prebuilt::PrebuiltError }, + /// A [`Build`] this crate cannot name failed. How it runs a step is its own + /// business, so the cause is carried whole and displayed as itself. + /// + /// This is the only variant a build off this machine has, and without it + /// the enum would be uninhabited there. + #[snafu(transparent)] + Other { + source: Box, + }, +} + +impl BuildError { + /// Wraps an implementation's own error for the trait boundary. + pub fn new(source: impl std::error::Error + Send + Sync + 'static) -> Self { + Self::Other { + source: Box::new(source), + } + } } #[async_trait] @@ -38,17 +63,23 @@ pub trait Build: Sync + Send { /// Runs each build step where it has to be run: a script step in a subprocess, /// a pre-built step by asking [`wasm::Fetch`] for the module. +/// +/// Only a host can run the script half, so this whole implementation is +/// host-side; somewhere without subprocesses supplies its own [`Build`]. +#[cfg(feature = "host")] pub struct Builder { wasm: Arc, files: Arc, } +#[cfg(feature = "host")] impl Builder { pub fn new(wasm: Arc, files: Arc) -> Self { Self { wasm, files } } } +#[cfg(feature = "host")] #[async_trait] impl Build for Builder { async fn build( diff --git a/crates/icp-project/src/canister/mod.rs b/crates/icp-project/src/canister/mod.rs index 1c7487ecb..2cfaabf07 100644 --- a/crates/icp-project/src/canister/mod.rs +++ b/crates/icp-project/src/canister/mod.rs @@ -15,6 +15,7 @@ pub mod recipe; pub mod sync; pub mod visibility; +#[cfg(feature = "host")] mod script; pub mod wasm; diff --git a/crates/icp-project/src/canister/sync/mod.rs b/crates/icp-project/src/canister/sync/mod.rs index f5bc22b29..d512969d5 100644 --- a/crates/icp-project/src/canister/sync/mod.rs +++ b/crates/icp-project/src/canister/sync/mod.rs @@ -16,7 +16,9 @@ pub mod declared; pub mod plugin; pub mod script; -use script::{HostScripts, ScriptInvocation, ScriptRunError, ScriptRunner}; +#[cfg(feature = "host")] +use script::HostScripts; +use script::{ScriptInvocation, ScriptRunError, ScriptRunner}; pub struct Params { pub path: PathBuf, @@ -75,6 +77,7 @@ pub struct Syncer { impl Syncer { /// A syncer that runs script steps as host subprocesses. + #[cfg(feature = "host")] pub fn host(wasm: Arc, plugins: Arc) -> Self { Self::new(Arc::new(HostScripts), wasm, plugins) } diff --git a/crates/icp-project/src/canister/sync/plugin.rs b/crates/icp-project/src/canister/sync/plugin.rs index 756194b0f..e91fac91c 100644 --- a/crates/icp-project/src/canister/sync/plugin.rs +++ b/crates/icp-project/src/canister/sync/plugin.rs @@ -187,6 +187,7 @@ pub enum PluginError { /// [`PLUGIN_COMPUTE_LIMIT_ENV`] override. Fails loudly on a malformed value so /// a typo doesn't silently fall back to the default and leave the caller /// wondering why their raised limit had no effect. +#[cfg(feature = "host")] fn resolve_compute_limit_secs() -> Result { match std::env::var(PLUGIN_COMPUTE_LIMIT_ENV) { Ok(value) => parse_compute_limit(&value), @@ -201,6 +202,13 @@ fn resolve_compute_limit_secs() -> Result { } } +/// The default, where there are no environment variables to override it with. +#[cfg(not(feature = "host"))] +fn resolve_compute_limit_secs() -> Result { + Ok(DEFAULT_PLUGIN_COMPUTE_LIMIT_SECS) +} + +#[cfg(feature = "host")] fn parse_compute_limit(value: &str) -> Result { match value.trim().parse::() { Ok(secs) if secs >= 1 => Ok(secs), @@ -357,6 +365,7 @@ fn resolve_callable( mod tests { use super::*; + #[cfg(feature = "host")] #[test] fn parse_compute_limit_accepts_positive_integers() { assert_eq!(parse_compute_limit("300").unwrap(), 300); @@ -364,6 +373,7 @@ mod tests { assert_eq!(parse_compute_limit(" 42 ").unwrap(), 42); } + #[cfg(feature = "host")] #[test] fn parse_compute_limit_rejects_invalid_values() { for bad in ["0", "abc", "30O", "-5", "1.5", ""] { diff --git a/crates/icp-project/src/canister/sync/script.rs b/crates/icp-project/src/canister/sync/script.rs index 82732d6a0..d0328393d 100644 --- a/crates/icp-project/src/canister/sync/script.rs +++ b/crates/icp-project/src/canister/sync/script.rs @@ -21,6 +21,7 @@ use crate::prelude::*; use super::Params; +#[cfg(feature = "host")] use super::super::script::execute_commands; /// A fully-resolved script sync step: the command(s), the working directory, and @@ -96,8 +97,10 @@ pub trait ScriptRunner: Sync + Send { } /// The [`ScriptRunner`] that spawns each command as a host subprocess. +#[cfg(feature = "host")] pub struct HostScripts; +#[cfg(feature = "host")] #[async_trait] impl ScriptRunner for HostScripts { async fn run_script( @@ -125,6 +128,7 @@ impl ScriptRunner for HostScripts { mod tests { use std::collections::BTreeMap; + #[cfg(feature = "host")] use tokio::sync::Mutex; use candid::Principal; @@ -136,6 +140,7 @@ mod tests { /// Serializes the tests here that mutate the process environment, since /// cargo runs tests in parallel threads. Async-aware because the variable /// has to stay set across the subprocess `await` that reads it. + #[cfg(feature = "host")] static ENV_MUTEX: Mutex<()> = Mutex::const_new(()); fn principal(byte: u8) -> Principal { @@ -206,6 +211,7 @@ mod tests { } /// The host runner passes the resolved environment through to the subprocess. + #[cfg(feature = "host")] #[tokio::test] async fn host_runner_applies_the_resolved_environment() { let out = camino_tempfile::NamedUtf8TempFile::new().unwrap(); @@ -234,6 +240,7 @@ mod tests { /// sets rather than `PATH`, because Git-for-Windows bash rewrites `PATH` /// into POSIX form, so its value there never equals the `PATH` the Rust side /// reads. + #[cfg(feature = "host")] #[tokio::test] async fn host_runner_overlays_rather_than_replaces_the_environment() { const AMBIENT: &str = "ICP_CLI_TEST_AMBIENT_VAR"; @@ -278,6 +285,7 @@ mod tests { /// A command that exits non-zero surfaces as a `ScriptRunError` whose source /// still names the command and its status, so the `caused by:` line the CLI /// prints stays specific. + #[cfg(feature = "host")] #[tokio::test] async fn host_runner_reports_a_failing_command() { let invocation = ScriptInvocation { diff --git a/crates/icp-project/src/host.rs b/crates/icp-project/src/host.rs index a432a0392..df5971078 100644 --- a/crates/icp-project/src/host.rs +++ b/crates/icp-project/src/host.rs @@ -77,6 +77,9 @@ pub struct Host { /// Network resolution: endpoints, root keys, friendly domains pub network: Arc, + /// Source of randomness, for picking a subnet out of several + pub random: Arc, + /// Where to report what resolution turned up. See [`Observe`]. pub observer: Arc, } @@ -117,6 +120,7 @@ impl Host { syncer: Arc::new(crate::canister::sync::UnimplementedMockSyncer), wasm: Arc::new(crate::canister::wasm::UnimplementedMockFetch), network: Arc::new(crate::network::MockNetworkAccessor::new()), + random: Arc::new(crate::random::FirstChoice), observer: Arc::new(Ignore), } } diff --git a/crates/icp-project/src/lib.rs b/crates/icp-project/src/lib.rs index 29d109d8b..ec17458fc 100644 --- a/crates/icp-project/src/lib.rs +++ b/crates/icp-project/src/lib.rs @@ -39,6 +39,7 @@ pub mod operations; pub mod parsers; pub mod prelude; pub mod project; +pub mod random; pub mod signal; pub mod store_artifact; pub mod store_id; @@ -249,12 +250,12 @@ static WORKSPACE_ROOT_ANNOUNCED: std::sync::atomic::AtomicBool = /// Warn once when the resolved workspace root differs from the sub-project the /// command is run in, so the upward resolution (§workspace model) is visible for /// every command, not just deploy. -fn announce_workspace_root_once(member: &Path, root: &Path) { +async fn announce_workspace_root_once(files: &dyn FileSystem, member: &Path, root: &Path) { let differs = match ( - dunce::canonicalize(member.as_std_path()), - dunce::canonicalize(root.as_std_path()), + files.canonicalize(member).await, + files.canonicalize(root).await, ) { - (Ok(m), Ok(r)) => m != r, + (Some(m), Some(r)) => m != r, _ => member != root, }; if differs && !WORKSPACE_ROOT_ANNOUNCED.swap(true, std::sync::atomic::Ordering::Relaxed) { @@ -277,7 +278,7 @@ impl ProjectLoad for ProjectLoadImpl { // Announce (once) when we resolved up to a workspace root above the // sub-project the command is run in, so this is visible for every command. if let Ok(member) = self.project_root_locate.locate_member() { - announce_workspace_root_once(&member, &pdir); + announce_workspace_root_once(self.files.as_ref(), &member, &pdir).await; } // Load project manifest @@ -706,7 +707,8 @@ impl ProjectLoad for NoProjectLoader { } } -#[cfg(test)] +// Every test here loads a project off a real directory through `HostFileSystem`. +#[cfg(all(test, feature = "host"))] mod tests { use super::*; use crate::canister::recipe::{Fetched, Resolve, ResolveError}; diff --git a/crates/icp-project/src/manifest/mod.rs b/crates/icp-project/src/manifest/mod.rs index 68e7ed7b1..e687b2bc3 100644 --- a/crates/icp-project/src/manifest/mod.rs +++ b/crates/icp-project/src/manifest/mod.rs @@ -321,7 +321,8 @@ where Ok(m) } -#[cfg(test)] +// Every test here drives `ProjectRootLocateImpl` over real directories. +#[cfg(all(test, feature = "host"))] mod tests { use super::*; use camino_tempfile::Utf8TempDir; diff --git a/crates/icp-project/src/operations/bundle.rs b/crates/icp-project/src/operations/bundle.rs index 43ed39bc8..e561f9641 100644 --- a/crates/icp-project/src/operations/bundle.rs +++ b/crates/icp-project/src/operations/bundle.rs @@ -379,6 +379,7 @@ impl Pruned<'_> { } } +/// Assemble a bundle from a workspace and write it out as a `.tar.gz`. pub async fn create_bundle( files: &dyn FileSystem, project_dir: &Path, diff --git a/crates/icp-project/src/operations/create.rs b/crates/icp-project/src/operations/create.rs index 42b065e57..eefe8b036 100644 --- a/crates/icp-project/src/operations/create.rs +++ b/crates/icp-project/src/operations/create.rs @@ -23,11 +23,10 @@ use icp_canister_interfaces::{ }, engine_canister::{ GET_ENGINE_OPERATOR_BY_SUBNET_METHOD, GetEngineOperatorBySubnetArgs, - GetEngineOperatorBySubnetResult, engine_canister_id, + GetEngineOperatorBySubnetResult, }, icp_ledger::{ICP_LEDGER_BLOCK_FEE_E8S, ICP_LEDGER_PRINCIPAL}, }; -use rand::seq::IndexedRandom; use snafu::{OptionExt, ResultExt, Snafu}; use tokio::{select, sync::OnceCell, time::sleep}; use tracing::{info, warn}; @@ -67,9 +66,6 @@ pub enum CreateOperationError { subnet: Principal, }, - #[snafu(display("invalid engine-canister id: {message}"))] - EngineCanisterId { message: String }, - #[snafu(display("failed to query the engine-canister registry"))] EngineCanisterQuery { source: crate::calls::CallError }, @@ -175,9 +171,14 @@ pub enum CreateTarget { struct CreateOperationInner { calls: Arc, + random: Arc, target: CreateTarget, funding: CreateFunding, existing_canisters: Vec, + /// The engine-canister registry to ask which engine-operator serves a + /// subnet. Resolved by the caller, which is where an override of it — an + /// environment variable — is something to read at all. + engine_registry: Principal, resolved_subnet: OnceCell>, } @@ -196,16 +197,20 @@ impl Clone for CreateOperation { impl CreateOperation { pub fn new( calls: Arc, + random: Arc, target: CreateTarget, funding: CreateFunding, existing_canisters: Vec, + engine_registry: Principal, ) -> Self { Self { inner: Arc::new(CreateOperationInner { calls, + random, target, funding, existing_canisters, + engine_registry, resolved_subnet: OnceCell::new(), }), } @@ -333,8 +338,7 @@ impl CreateOperation { &self, subnet: Principal, ) -> Result { - let engine_registry = engine_canister_id() - .map_err(|message| CreateOperationError::EngineCanisterId { message })?; + let engine_registry = self.inner.engine_registry; let arg = GetEngineOperatorBySubnetArgs { subnet_id: Some(subnet), @@ -646,11 +650,26 @@ impl CreateOperation { let subnets = get_available_subnets(self.inner.calls.as_ref()) .await .map_err(|e| e.to_string())?; + if subnets.is_empty() { + return Err("no available subnets found".to_string()); + } - subnets - .choose(&mut rand::rng()) - .copied() - .ok_or_else(|| "no available subnets found".to_string()) + let chosen = self + .inner + .random + .index_below(subnets.len()) + .await + .map_err(|e| e.to_string())?; + // `index_below` promises an index below the count, and the + // count is not zero, so a miss is the seam's bug and says so + // rather than posing as an empty list. + chosen.and_then(|i| subnets.get(i).copied()).ok_or_else(|| { + format!( + "randomness chose {chosen:?}, which is not one of the {} \ + available subnets", + subnets.len() + ) + }) } }) .await; diff --git a/crates/icp-project/src/operations/deploy.rs b/crates/icp-project/src/operations/deploy.rs index 4c23cc8bf..0c52ae9e6 100644 --- a/crates/icp-project/src/operations/deploy.rs +++ b/crates/icp-project/src/operations/deploy.rs @@ -184,6 +184,9 @@ pub struct DeployParams { pub subnet: Option, pub proxy: Option, pub cycles: u128, + /// The engine-canister registry a created canister's subnet is looked up + /// in, already resolved from the environment by the caller. + pub engine_registry: Principal, pub no_create: bool, /// Skip the Candid interface compatibility check. pub yes: bool, @@ -433,9 +436,11 @@ async fn create_canisters( }; let create_operation = CreateOperation::new( calls.clone(), + host.random.clone(), target, CreateFunding::Cycles(params.cycles), existing_ids, + params.engine_registry, ); let mut futs = FuturesOrdered::new(); diff --git a/crates/icp-project/src/operations/mod.rs b/crates/icp-project/src/operations/mod.rs index 0b444a533..3023f1e85 100644 --- a/crates/icp-project/src/operations/mod.rs +++ b/crates/icp-project/src/operations/mod.rs @@ -1,5 +1,9 @@ pub mod binding_env_vars; pub mod build; +// Host-side: a bundle is a `.tar.gz` on a disk, and a plugin's declared +// directory goes into it as a tree walked for symlinks, which no seam over +// `FileSystem` can reproduce faithfully. +#[cfg(feature = "host")] pub mod bundle; pub mod candid_compat; pub mod create; diff --git a/crates/icp-project/src/project.rs b/crates/icp-project/src/project.rs index 8f6085693..289df34fb 100644 --- a/crates/icp-project/src/project.rs +++ b/crates/icp-project/src/project.rs @@ -1672,7 +1672,7 @@ pub async fn consolidate_manifest( }) } -#[cfg(test)] +#[cfg(all(test, feature = "host"))] mod recipe_sync_tests { use super::*; use crate::canister::recipe::{Fetched, Resolve, ResolveError}; @@ -1780,7 +1780,7 @@ mod recipe_sync_tests { } } -#[cfg(test)] +#[cfg(all(test, feature = "host"))] mod dependency_tests { use super::*; use crate::canister::recipe::{Fetched, Resolve, ResolveError}; diff --git a/crates/icp-project/src/random.rs b/crates/icp-project/src/random.rs new file mode 100644 index 000000000..6744fe21d --- /dev/null +++ b/crates/icp-project/src/random.rs @@ -0,0 +1,73 @@ +//! Choosing at random. +//! +//! Only one thing here needs randomness — picking which subnet to create a +//! canister on when the manifest names a kind rather than a subnet — and it +//! cannot be done by arithmetic. On a host it is a syscall; inside a canister +//! it is a `raw_rand` call to the management canister, which is why this is +//! asked for rather than done. + +use async_trait::async_trait; +use snafu::Snafu; + +/// Randomness could not be obtained. +/// +/// Where it comes from is the implementation's business — a syscall, a +/// management-canister call — so the cause is carried whole and displayed as +/// itself. +#[derive(Debug, Snafu)] +#[snafu(transparent)] +pub struct RandomError { + pub source: Box, +} + +impl RandomError { + /// Wraps an implementation's own error for the trait boundary. + pub fn new(source: impl std::error::Error + Send + Sync + 'static) -> Self { + Self { + source: Box::new(source), + } + } +} + +/// A source of randomness. +/// +/// The method is a choice rather than a byte buffer because a choice is what +/// every caller wants, and turning bytes into an unbiased index is the kind of +/// thing each caller would get subtly wrong on its own. +#[async_trait] +pub trait Random: Send + Sync { + /// A uniformly distributed index below `count`, or `None` when `count` is + /// zero and there is nothing to choose. + async fn index_below(&self, count: usize) -> Result, RandomError>; +} + +#[cfg(feature = "host")] +/// [`Random`] from the operating system's entropy. +#[derive(Debug, Default, Clone, Copy)] +pub struct HostRandom; + +#[cfg(feature = "host")] +#[async_trait] +impl Random for HostRandom { + async fn index_below(&self, count: usize) -> Result, RandomError> { + use rand::RngExt; + match count { + 0 => Ok(None), + count => Ok(Some(rand::rng().random_range(0..count))), + } + } +} + +#[cfg(any(test, feature = "test-util"))] +/// [`Random`] that always chooses the first of anything, for a test that needs +/// a choice made but not an unpredictable one. +#[derive(Debug, Default, Clone, Copy)] +pub struct FirstChoice; + +#[cfg(any(test, feature = "test-util"))] +#[async_trait] +impl Random for FirstChoice { + async fn index_below(&self, count: usize) -> Result, RandomError> { + Ok((count > 0).then_some(0)) + } +} diff --git a/crates/icp-project/src/signal.rs b/crates/icp-project/src/signal.rs index 607fba7b6..6bc508d2a 100644 --- a/crates/icp-project/src/signal.rs +++ b/crates/icp-project/src/signal.rs @@ -1,4 +1,5 @@ /// Utilities for handling process termination signals across platforms. +#[cfg(feature = "host")] use tokio::select; /// Waits for a stop signal (Ctrl+C, SIGTERM on Unix, or window close on Windows). @@ -26,7 +27,7 @@ use tokio::select; /// # } /// # async fn do_work() {} /// ``` -#[cfg(unix)] +#[cfg(all(unix, feature = "host"))] pub async fn stop_signal() { use tokio::signal::unix::{SignalKind, signal}; let mut sigterm = signal(SignalKind::terminate()).unwrap(); @@ -61,7 +62,7 @@ pub async fn stop_signal() { /// # } /// # async fn do_work() {} /// ``` -#[cfg(windows)] +#[cfg(all(windows, feature = "host"))] pub async fn stop_signal() { use tokio::signal::windows::{ctrl_break, ctrl_close}; let mut ctrl_break = ctrl_break().unwrap(); @@ -72,3 +73,13 @@ pub async fn stop_signal() { _ = ctrl_close.recv() => {}, } } + +/// Waits for a stop signal — forever, where there is no process to signal. +/// +/// A caller that races this against its own work is asking to be interrupted +/// if anything interrupts the program. Nothing can, off a host, so the arm +/// simply never fires. +#[cfg(not(feature = "host"))] +pub async fn stop_signal() { + std::future::pending().await +} diff --git a/crates/icp-project/src/store_artifact.rs b/crates/icp-project/src/store_artifact.rs index 402fadc3c..707fe734c 100644 --- a/crates/icp-project/src/store_artifact.rs +++ b/crates/icp-project/src/store_artifact.rs @@ -261,7 +261,7 @@ impl Access for MockInMemoryArtifactStore { } } -#[cfg(test)] +#[cfg(all(test, feature = "host"))] mod tests { use super::{artifact_name_overflow, sanitize_artifact_name}; diff --git a/crates/icp-sync-plugin/Cargo.toml b/crates/icp-sync-plugin/Cargo.toml index 7cdcb33df..3a8d4a08b 100644 --- a/crates/icp-sync-plugin/Cargo.toml +++ b/crates/icp-sync-plugin/Cargo.toml @@ -19,7 +19,9 @@ icp-events.workspace = true icp-project = { path = "../icp-project", default-features = false } semver.workspace = true snafu.workspace = true -tokio.workspace = true +# `rt-multi-thread` for `block_in_place`, which the runtime needs because a +# wasm import cannot suspend while a canister call is in flight. +tokio = { workspace = true, features = ["rt-multi-thread"] } wasmtime.workspace = true wasmtime-wasi.workspace = true diff --git a/scripts/check-no-host-reach.sh b/scripts/check-no-host-reach.sh new file mode 100755 index 000000000..84c0e1be5 --- /dev/null +++ b/scripts/check-no-host-reach.sh @@ -0,0 +1,85 @@ +#!/usr/bin/env bash +# +# Fails if `icp-project`'s core reaches for something only a host has. +# +# Building the crate for `wasm32-unknown-unknown` does not prove this on its +# own: that target ships a full `std` whose `fs`, `process` and `env` backends +# compile and fail only at runtime, so a bare `std::fs::read_to_string` passes +# a build that was meant to reject it. What the build does leave behind is one +# relocatable wasm object per codegen unit, and there every symbol the crate +# calls but does not define is an import from the `env` module. So read those +# imports back and fail on any that names a corner of `std` only a host serves. +# +# This sees what `icp-project` itself calls, including whatever a dependency's +# generics monomorphise into it. What it cannot see is a dependency's own +# non-generic code, compiled once into that dependency's objects: a call that +# only arrives at the host a frame or two inside one of those is named here +# after the dependency rather than after `std`, and matches nothing in `std`. +# `camino`'s `Utf8Path::is_file` is such a front door, and the one this crate +# would reach for by accident, so those are denied by name too. + +set -euo pipefail + +cd "$(dirname "$0")/.." + +# Paths in `std` that only a host can serve, written as `llvm-nm` demangles +# them. `std::sys` is std's own platform layer, listed because a call that +# arrives there from an inlined front door would otherwise go unseen. +denied='std::(fs|net|os|env|process|time)::' +denied+='|std::io::stdio::' +denied+='|std::sys::(args|env|fd|fs|net|os|pal|process|stdio|thread|time)' + +# A dependency's own front door onto one of those. Found by reading these same +# imports out of every rlib in the `--no-default-features` closure and keeping +# the crates whose own objects reach a host corner of `std`: `camino`, `glob`, +# `handlebars`, `candid_parser`, `icp-canister-interfaces` and `snafu`. Every +# other crate there reaches the host only where the build already rejects it, +# gating the code on `cfg(unix)`/`cfg(windows)` — as `tokio`'s `process` and +# `signal` do, and as `rand`'s entropy source does. `snafu` earns no entry: its +# one reach is the `RUST_LIB_BACKTRACE` a captured `Backtrace` reads, and no +# error in this repo carries one. +denied+='|<(std::path::Path|camino::Utf8Path|camino::Utf8DirEntry)>::' +denied+='(try_exists|exists|is_file|is_dir|is_symlink|metadata|symlink_metadata' +denied+='|canonicalize|read_dir|read_link|file_type)' +denied+='|&2 + exit 1 +fi + +nm=$(find "$(rustc --print sysroot)" -name 'llvm-nm*' -type f -print -quit) +if [[ -z $nm ]]; then + echo "::error::llvm-nm not found; run 'rustup component add llvm-tools'" >&2 + exit 1 +fi + +# An rlib also carries cargo's metadata members, which `llvm-nm` reports as +# holding no symbols; that is expected, so keep it out of the log. +symbols=$("$nm" --demangle --undefined-only "$rlib" 2> >(grep -v ': no symbols$' >&2)) + +reaches=$(printf '%s\n' "$symbols" | sed 's/^ *U //' | sort -u | + grep -E "$denied" | grep -vxE "$allowed" || true) + +if [[ -n $reaches ]]; then + echo "::error::icp-project's core reaches the host; it must ask through a seam instead" + echo "$reaches" | sed 's/^/ /' + exit 1 +fi + +echo "icp-project's core reaches no host"