From e98730f3431100b5403111b8dcc1a62137d60bd7 Mon Sep 17 00:00:00 2001 From: Raymond Khalife Date: Tue, 10 Nov 2026 16:26:16 +0000 Subject: [PATCH 1/8] feat(project): --untrusted, with the project read beneath a cap-std directory handle `--untrusted` on build, deploy and sync limits the steps a project may run to a pre-built module and a plugin, and reads everything through `files::Confined` at the project root. `Confined` opens the root once as a `cap_std::fs::Dir` and makes every read and write beneath that handle, so the operating system refuses a path that leaves the root in the same step that opens it (`openat2` on Linux). On top of that it refuses any path through a symlink, wherever the link leads. This is #866 and #871 on main, without #868, and with `Confined` built on cap-std in place of a path walk followed by a plain open. Co-Authored-By: Claude --- CHANGELOG.md | 1 + Cargo.lock | 33 +- Cargo.toml | 1 + crates/icp-app/src/context/init.rs | 62 ++- crates/icp-app/src/network/accessor.rs | 18 +- crates/icp-app/src/network/directory.rs | 15 + crates/icp-app/src/recipe.rs | 9 +- crates/icp-cli/src/commands/build.rs | 5 +- crates/icp-cli/src/commands/deploy.rs | 5 +- crates/icp-cli/src/commands/sync.rs | 5 +- crates/icp-cli/src/complete.rs | 1 + crates/icp-cli/src/main.rs | 16 + crates/icp-cli/src/options.rs | 13 + crates/icp-cli/tests/untrusted_tests.rs | 368 ++++++++++++++++ crates/icp-project/Cargo.toml | 3 +- crates/icp-project/src/files.rs | 520 +++++++++++++++++++++++ crates/icp-project/src/lib.rs | 7 + crates/icp-project/src/project.rs | 109 +++++ crates/icp-project/src/store_artifact.rs | 91 +++- crates/icp-project/src/store_id.rs | 76 +++- crates/icp-project/src/untrusted.rs | 428 +++++++++++++++++++ docs/reference/cli.md | 3 + 22 files changed, 1753 insertions(+), 36 deletions(-) create mode 100644 crates/icp-cli/tests/untrusted_tests.rs create mode 100644 crates/icp-project/src/untrusted.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index bd34d7523..309183e04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ air-gapped signing, extensions # Unreleased +* feat: `--untrusted`, on `build`, `deploy` and `sync`, holds a project someone else handed over to what it may read and run. Every file the project reads — manifests, argument and environment files, local recipe templates, modules — must be under the project root and reached through no symlink. Each build step must be a pre-built module and each sync step a plugin, taken from a regular file in the project; the steps a recipe renders to are held to the same list. Anything else is refused before the command acts on it, naming what was found. What the CLI keeps under `.icp` is held to the same rule, so a symlink planted there is refused rather than written through. * chore: macOS on Intel (`x86_64-apple-darwin`) is no longer a release target. Releases ship no Intel macOS archive, the shell installer refuses that platform, and npm no longer publishes `@icp-sdk/icp-cli-darwin-x64`, so installing `@icp-sdk/icp-cli` on an Intel Mac warns of an unsupported platform. Homebrew dropped Intel macOS in September 2026, and GitHub is retiring its Intel macOS runners. v1.8.0 is the last release with an Intel macOS build. ## Experimental diff --git a/Cargo.lock b/Cargo.lock index e2ec8f506..20006a7bd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1018,6 +1018,18 @@ dependencies = [ "winx", ] +[[package]] +name = "cap-std" +version = "4.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c1ec78e242cfa2cfe276807ac2ecc00315a6c97786977414bcd1c3963b6c91b8" +dependencies = [ + "cap-primitives", + "io-extras", + "io-lifetimes 3.0.1", + "rustix", +] + [[package]] name = "cargo-generate" version = "0.23.14" @@ -2223,7 +2235,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -2415,7 +2427,7 @@ checksum = "94e7099f6313ecacbe1256e8ff9d617b75d1bcb16a6fddef94866d225a01a14a" dependencies = [ "io-lifetimes 2.0.4", "rustix", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -3750,6 +3762,7 @@ dependencies = [ "camino-tempfile", "candid", "candid_parser", + "cap-std", "clap", "dunce", "flate2", @@ -4069,7 +4082,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "20fd6de4ccfcc187e38bc21cfa543cb5a302cb86a8b114eb7f0bf0dc9f8ac00f" dependencies = [ "io-lifetimes 3.0.1", - "windows-sys 0.52.0", + "windows-sys 0.60.2", ] [[package]] @@ -5763,7 +5776,7 @@ dependencies = [ "once_cell", "socket2", "tracing", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -6178,7 +6191,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -6245,7 +6258,7 @@ dependencies = [ "security-framework 3.7.0", "security-framework-sys", "webpki-root-certs", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -7101,10 +7114,10 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" dependencies = [ "fastrand", - "getrandom 0.3.4", + "getrandom 0.4.3", "once_cell", "rustix", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -8349,7 +8362,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -8757,7 +8770,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3f3fd376f71958b862e7afb20cfe5a22830e1963462f3a17f49d82a6c1d1f42d" dependencies = [ "bitflags 2.13.2", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 4fd449c1a..99c873181 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,6 +32,7 @@ bytes = "1.11" camino = { version = "1.1.9", features = ["serde1"] } cargo-generate = "0.23.7" camino-tempfile = "1" +cap-std = "4" candid = "0.10.19" candid_parser = "0.3.0" clap = { version = "4.5.3", features = ["derive", "env", "string", "unstable-ext"] } diff --git a/crates/icp-app/src/context/init.rs b/crates/icp-app/src/context/init.rs index b0dc0bf0d..40bb93b53 100644 --- a/crates/icp-app/src/context/init.rs +++ b/crates/icp-app/src/context/init.rs @@ -12,7 +12,13 @@ use icp_project::store_artifact::ArtifactStore; use std::time::Duration; use crate::{agent, identity, identity::PasswordFunc}; -use icp_project::{Lazy, ProjectLoadImpl, host::Host, manifest, store_id}; +use icp_project::{ + Lazy, ProjectLoad, ProjectLoadImpl, + files::{Confined, FileSystem, HostFileSystem, RootError}, + host::Host, + manifest::{self, ProjectRootLocate as _}, + store_id, untrusted, +}; #[derive(Debug, Snafu)] pub enum ContextInitError { @@ -36,13 +42,25 @@ pub enum ContextInitError { PackageCache { source: icp_project::fs::lock::LockError, }, + + #[snafu(display("failed to locate the project to treat as untrusted"))] + UntrustedRoot { + source: manifest::ProjectRootLocateError, + }, + + #[snafu(display("failed to confine the untrusted project to its directory"))] + Confine { source: RootError }, } +/// With `untrusted`, the project is read through a filesystem confined to +/// its root and vetted by [`icp_project::untrusted`] before any command acts +/// on it. The root is located up front for that, so there has to be a project. pub fn initialize( project_root_override: Option, debug: bool, password_func: PasswordFunc, pem_session_duration: Option, + untrusted: bool, ) -> Result { // Setup global directory structure let dirs = Arc::new(Directories::new().context(DirectoriesSnafu)?); @@ -75,16 +93,35 @@ pub fn initialize( let cwd: PathBuf = PathBuf::try_from(current_dir().context(CwdSnafu)?).context(Utf8PathSnafu)?; + // A relative override is taken from the working directory: the + // filesystem an untrusted project is confined to matches every path + // against the root as spelled, so the root has to be a whole path. + let project_root_override = project_root_override.map(|dir| cwd.join(dir)); let project_root_locate = Arc::new(manifest::ProjectRootLocateImpl::new( cwd, project_root_override, )); + // An untrusted project is confined to its directory: what it reads, and + // what the CLI keeps under `.icp` on its behalf. + let confined = if untrusted { + let root = project_root_locate.locate().context(UntrustedRootSnafu)?; + Some(Confined::new(&root).context(ConfineSnafu)?) + } else { + None + }; + // Canister ID Store - let ids = Arc::new(store_id::AccessImpl::new(project_root_locate.clone())); + let ids = Arc::new(store_id::AccessImpl::new( + project_root_locate.clone(), + confined.clone(), + )); // Canister Artifact Store (wasm) - let artifacts = Arc::new(ArtifactStore::new(project_root_locate.clone())); + let artifacts = Arc::new(ArtifactStore::new( + project_root_locate.clone(), + confined.clone(), + )); // Prepare http client let http_client = reqwest::Client::new(); @@ -100,15 +137,20 @@ pub fn initialize( pkg_cache.clone(), )); + // The project's files come from this machine: the whole of it for the + // user's own project, the project directory alone for an untrusted one. + let files: Arc = match &confined { + Some(confined) => Arc::new(confined.clone()), + None => Arc::new(HostFileSystem), + }; + // Recipes let recipe = Arc::new(RecipeFetcher { http_client, pkg_cache, + files: files.clone(), }); - // The project's files come from this machine. - let files = Arc::new(icp_project::files::HostFileSystem); - // Canister builder let builder = Arc::new(Builder::new(wasm.clone(), files.clone())); @@ -125,8 +167,11 @@ pub fn initialize( files: files.clone(), }; - let pload = Lazy::new(pload); - let pload = Arc::new(pload); + let pload: Arc = if untrusted { + Arc::new(Lazy::new(untrusted::Vetted::new(pload, files.clone()))) + } else { + Arc::new(Lazy::new(pload)) + }; // Telemetry data bag (written by subsystems, read at session finish) let telemetry_data = Arc::new(crate::telemetry_data::TelemetryData::default()); @@ -152,6 +197,7 @@ pub fn initialize( // Network accessor let netaccess = Arc::new(crate::network::Accessor { project_root_locate: project_root_locate.clone(), + confined, descriptors: dirs.port_descriptor(), agent: agent_creator.clone(), }); diff --git a/crates/icp-app/src/network/accessor.rs b/crates/icp-app/src/network/accessor.rs index a97d770b5..075a43ddf 100644 --- a/crates/icp-app/src/network/accessor.rs +++ b/crates/icp-app/src/network/accessor.rs @@ -8,6 +8,7 @@ use candid::Principal; use snafu::{ResultExt, Snafu}; use url::Url; +use icp_project::files::{Confined, FsError}; use icp_project::manifest::{ProjectRootLocate, ProjectRootLocateError}; use icp_project::network::{ Access, AccessError, CollectFriendlyDomains, Configuration, NetworkAccess, NetworkUrls, @@ -26,6 +27,9 @@ use crate::network::{ pub enum LocateNetworkDirectoryError { #[snafu(display("failed to find project root"))] ProjectRootLocate { source: ProjectRootLocateError }, + + #[snafu(display("network '{network}' keeps its state where an untrusted project may not"))] + Confine { source: FsError, network: String }, } /// Where a network keeps its on-disk state. @@ -57,6 +61,10 @@ pub struct Accessor { // Project root pub project_root_locate: Arc, + // What an untrusted project is confined to, which its network + // directories are then held to as well + pub confined: Option, + // Port descriptors dir pub descriptors: PathBuf, @@ -74,14 +82,20 @@ impl Accessor { .project_root_locate .locate() .context(ProjectRootLocateSnafu)?; - Ok(NetworkDirectory::new( + let nd = NetworkDirectory::new( &network.name, &dir.join(ICP_BASE) .join(CACHE_DIR) .join("networks") .join(&network.name), &self.descriptors, - )) + ); + if let Some(confined) = &self.confined { + nd.held_by(confined).context(ConfineSnafu { + network: &network.name, + })?; + } + Ok(nd) } } diff --git a/crates/icp-app/src/network/directory.rs b/crates/icp-app/src/network/directory.rs index 2018feec9..6344ea04b 100644 --- a/crates/icp-app/src/network/directory.rs +++ b/crates/icp-app/src/network/directory.rs @@ -168,6 +168,21 @@ impl NetworkDirectory { Ok(()) } + /// Refuse the files this directory is read through — its lock and its + /// descriptor — unless `confined`, the filesystem an untrusted project is + /// held to, would give them. + pub(crate) fn held_by( + &self, + confined: &icp_project::files::Confined, + ) -> Result<(), icp_project::files::FsError> { + let paths = NetworkRootPaths { + network_root: self.network_root.clone(), + }; + confined.resolve(&paths.lock_file())?; + confined.resolve(&paths.network_descriptor_path())?; + Ok(()) + } + /// Locked directory access for the local network root. pub fn root(&self) -> Result, LockError> { DirectoryStructureLock::open_or_create(NetworkRootPaths { diff --git a/crates/icp-app/src/recipe.rs b/crates/icp-app/src/recipe.rs index 37b278e91..3bc3b1571 100644 --- a/crates/icp-app/src/recipe.rs +++ b/crates/icp-app/src/recipe.rs @@ -14,7 +14,7 @@ use crate::package::{ read_cached_uri_recipe, }; use icp_project::{ - fs::read, + files::{FileSystem, FsError}, manifest::recipe::{Recipe, RecipeType}, prelude::*, }; @@ -30,6 +30,8 @@ pub struct RecipeFetcher { pub http_client: reqwest::Client, /// Package cache for caching downloaded recipe templates pub pkg_cache: std::sync::Arc, + /// The project's files, which a local template is read through + pub files: std::sync::Arc, } /// The result of the fetch stage, as this resolver sees it. @@ -76,7 +78,7 @@ enum TemplateSource { #[derive(Debug, Snafu)] pub enum RecipeFetchError { #[snafu(display("failed to read local recipe template file"))] - ReadFile { source: icp_project::fs::IoError }, + ReadFile { source: FsError }, #[snafu(display("failed to decode UTF-8 string"))] DecodeUtf8 { source: FromUtf8Error }, @@ -133,7 +135,7 @@ impl RecipeFetcher { // Retrieve the template, using cache for remote/registry sources let (tmpl, should_cache) = match &tmpl_source { TemplateSource::LocalPath(path) => { - let bytes = read(path).context(ReadFileSnafu)?; + let bytes = self.files.read(path).await.context(ReadFileSnafu)?; (parse_bytes_to_string(bytes)?, false) } @@ -360,6 +362,7 @@ mod tests { RecipeFetcher { http_client: reqwest::Client::new(), pkg_cache: std::sync::Arc::new(crate::package::open(cache_dir.to_owned()).unwrap()), + files: std::sync::Arc::new(icp_project::files::HostFileSystem), } } diff --git a/crates/icp-cli/src/commands/build.rs b/crates/icp-cli/src/commands/build.rs index e6dcfc876..85b226709 100644 --- a/crates/icp-cli/src/commands/build.rs +++ b/crates/icp-cli/src/commands/build.rs @@ -9,7 +9,7 @@ use tracing::info; use icp_project::operations::build::build_many; use crate::{ - options::{EnvironmentOpt, arg_struct_change_help}, + options::{EnvironmentOpt, UntrustedOpt, arg_struct_change_help}, render::rendered, }; @@ -22,6 +22,9 @@ pub(crate) struct BuildArgs { #[command(flatten)] pub(crate) environment: BuildEnvironmentOpt, + + #[command(flatten)] + pub(crate) untrusted: UntrustedOpt, } arg_struct_change_help!( diff --git a/crates/icp-cli/src/commands/deploy.rs b/crates/icp-cli/src/commands/deploy.rs index 781814947..8e6e6377e 100644 --- a/crates/icp-cli/src/commands/deploy.rs +++ b/crates/icp-cli/src/commands/deploy.rs @@ -20,7 +20,7 @@ use tracing::info; use crate::options::EnvironmentOpt; use crate::{ commands::{args::ArgsOpt, canister::create}, - options::{IdentityOpt, arg_struct_change_help}, + options::{IdentityOpt, UntrustedOpt, arg_struct_change_help}, render::rendered, }; @@ -97,6 +97,9 @@ pub(crate) struct DeployArgs { /// `upgrade_args` in the manifest. #[command(flatten)] pub(crate) args_opt: ArgsOpt, + + #[command(flatten)] + pub(crate) untrusted: UntrustedOpt, } arg_struct_change_help!( diff --git a/crates/icp-cli/src/commands/sync.rs b/crates/icp-cli/src/commands/sync.rs index a82eba23a..fcb28e048 100644 --- a/crates/icp-cli/src/commands/sync.rs +++ b/crates/icp-cli/src/commands/sync.rs @@ -12,7 +12,7 @@ use tracing::info; use icp_project::operations::{readiness::wait_until_serving_queries, sync::sync_many}; use crate::{ - options::{EnvironmentOpt, IdentityOpt}, + options::{EnvironmentOpt, IdentityOpt, UntrustedOpt}, render::rendered, }; @@ -32,6 +32,9 @@ pub(crate) struct SyncArgs { #[command(flatten)] pub(crate) identity: IdentityOpt, + + #[command(flatten)] + pub(crate) untrusted: UntrustedOpt, } pub(crate) async fn exec(ctx: &Context, args: &SyncArgs) -> Result<(), anyhow::Error> { diff --git a/crates/icp-cli/src/complete.rs b/crates/icp-cli/src/complete.rs index be3e877ee..d181d91ce 100644 --- a/crates/icp-cli/src/complete.rs +++ b/crates/icp-cli/src/complete.rs @@ -66,6 +66,7 @@ fn context() -> Option<&'static Context> { false, Arc::new(|| Err("cannot prompt while completing".to_string())), None, + false, ) .ok() }) diff --git a/crates/icp-cli/src/main.rs b/crates/icp-cli/src/main.rs index 8cf2e5e64..c449b1f1a 100644 --- a/crates/icp-cli/src/main.rs +++ b/crates/icp-cli/src/main.rs @@ -214,6 +214,7 @@ async fn run() -> Result<(), Error> { cli.debug, password_func, pem_session_duration, + command.untrusted(), )?; // Extension commands are left out of telemetry: telemetry parses against @@ -266,6 +267,21 @@ enum Invocation { Extension(icp_app::extension::Installed, ArgMatches), } +impl Invocation { + /// Whether the command was asked to treat the project as untrusted. The + /// flag belongs to the commands that act on what a project says — build, + /// deploy, sync — but it is the context every command runs in that + /// applies it, so it is read off the command before that is composed. + fn untrusted(&self) -> bool { + match self { + Invocation::BuiltIn(Command::Build(args)) => args.untrusted.untrusted, + Invocation::BuiltIn(Command::Deploy(args)) => args.untrusted.untrusted, + Invocation::BuiltIn(Command::Sync(args)) => args.untrusted.untrusted, + Invocation::BuiltIn(_) | Invocation::Extension(..) => false, + } + } +} + /// Dispatch the command to its handler. async fn dispatch(ctx: &icp_app::context::Context, command: Command) -> Result<(), Error> { match command { diff --git a/crates/icp-cli/src/options.rs b/crates/icp-cli/src/options.rs index 7a3bd4446..5a477a3d5 100644 --- a/crates/icp-cli/src/options.rs +++ b/crates/icp-cli/src/options.rs @@ -40,6 +40,19 @@ impl From for IdentitySelection { } } +/// `--untrusted`, for a command that acts on what a project says: build, +/// deploy, sync. A deploy service handed a project by an agent is what asks +/// for it, and those are the commands it runs. +#[derive(Args, Clone, Debug, Default)] +pub(crate) struct UntrustedOpt { + /// Hold the project to what one someone else handed over may do: every + /// file it reads is under the project root and reached through no + /// symlink, each build step is a pre-built module and each sync step a + /// plugin. Anything else is refused + #[arg(long)] + pub(crate) untrusted: bool, +} + #[derive(Args, Clone, Debug, Default)] #[clap(group(ArgGroup::new("environment-select").multiple(false)))] pub(crate) struct EnvironmentOpt { diff --git a/crates/icp-cli/tests/untrusted_tests.rs b/crates/icp-cli/tests/untrusted_tests.rs new file mode 100644 index 000000000..38587b163 --- /dev/null +++ b/crates/icp-cli/tests/untrusted_tests.rs @@ -0,0 +1,368 @@ +//! `--untrusted` holds a project to what one from elsewhere may read and +//! run: see `icp_project::untrusted`. Each test here sets up a tree the CLI +//! would act on as the user's own, and shows the flag refusing it, naming +//! what it refused. + +use indoc::indoc; +use predicates::{prelude::PredicateBooleanExt, str::contains}; + +use crate::common::TestContext; +use icp_project::fs::write_string; + +mod common; + +/// A project directory holding a module of its own, and beside it a file +/// outside the project: `outside.bin`, and `outside.wasm`, a module. +fn project(ctx: &TestContext, manifest: &str) -> camino::Utf8PathBuf { + let project_dir = ctx.create_project_dir("icp"); + let wasm = ctx.make_asset("example_icp_mo.wasm"); + let outside = project_dir.parent().unwrap(); + std::fs::copy(&wasm, project_dir.join("backend.wasm")).expect("copy wasm"); + std::fs::copy(&wasm, outside.join("outside.wasm")).expect("copy wasm outside"); + std::fs::write(outside.join("outside.bin"), b"secret").expect("write outside"); + write_string(&project_dir.join("icp.yaml"), manifest).expect("write manifest"); + project_dir +} + +const PREBUILT: &str = indoc! {r#" + canisters: + - name: backend + build: + steps: + - type: pre-built + path: backend.wasm +"#}; + +const SCRIPT: &str = indoc! {r#" + canisters: + - name: backend + build: + steps: + - type: script + command: cp backend.wasm "$ICP_WASM_OUTPUT_PATH" +"#}; + +#[test] +fn a_bundle_shaped_project_builds_as_untrusted() { + let ctx = TestContext::new(); + let project_dir = project(&ctx, PREBUILT); + + ctx.icp() + .current_dir(&project_dir) + .args(["build", "--untrusted"]) + .assert() + .success(); +} + +#[test] +fn a_script_step_is_refused_as_untrusted_but_not_otherwise() { + let ctx = TestContext::new(); + let project_dir = project(&ctx, SCRIPT); + + ctx.icp() + .current_dir(&project_dir) + .args(["build"]) + .assert() + .success(); + + ctx.icp() + .current_dir(&project_dir) + .args(["build", "--untrusted"]) + .assert() + .failure() + .stderr( + contains("more than an untrusted project may") + .and(contains( + "canister 'backend' has a build step that is not a pre-built module", + )) + .and(contains("(script) cp backend.wasm")), + ); +} + +/// A recipe is read and rendered like any other; it is the steps it renders +/// to that are held to the list. +#[test] +fn a_recipe_is_rendered_and_its_steps_held_to_the_list() { + let ctx = TestContext::new(); + let project_dir = project( + &ctx, + indoc! {r#" + canisters: + - name: backend + recipe: + type: file://recipe.hbs + "#}, + ); + let recipe = project_dir.join("recipe.hbs"); + + std::fs::write( + &recipe, + "build:\n steps:\n - type: pre-built\n path: backend.wasm\n", + ) + .expect("write recipe"); + ctx.icp() + .current_dir(&project_dir) + .args(["build", "--untrusted"]) + .assert() + .success(); + + std::fs::write( + &recipe, + "build:\n steps:\n - type: script\n command: cp backend.wasm \"$ICP_WASM_OUTPUT_PATH\"\n", + ) + .expect("write recipe"); + ctx.icp() + .current_dir(&project_dir) + .args(["build", "--untrusted"]) + .assert() + .failure() + .stderr(contains("not a pre-built module").and(contains("(script) cp backend.wasm"))); +} + +/// What the project reads is held to its root: an argument file outside it +/// is read for the user's own project and refused for an untrusted one. +#[test] +fn a_file_read_from_outside_the_project_is_refused_as_untrusted() { + let ctx = TestContext::new(); + let project_dir = project( + &ctx, + indoc! {r#" + canisters: + - name: backend + init_args: + path: ../outside.bin + format: bin + build: + steps: + - type: pre-built + path: backend.wasm + "#}, + ); + + ctx.icp() + .current_dir(&project_dir) + .args(["build"]) + .assert() + .success(); + + ctx.icp() + .current_dir(&project_dir) + .args(["build", "--untrusted"]) + .assert() + .failure() + .stderr(contains("outside.bin").and(contains("is outside"))); +} + +/// A local recipe template is read through the same filesystem. +#[test] +fn a_recipe_template_outside_the_project_is_refused_as_untrusted() { + let ctx = TestContext::new(); + let project_dir = project( + &ctx, + indoc! {r#" + canisters: + - name: backend + recipe: + type: file://../recipe.hbs + "#}, + ); + std::fs::write( + project_dir.parent().unwrap().join("recipe.hbs"), + "build:\n steps:\n - type: pre-built\n path: backend.wasm\n", + ) + .expect("write recipe"); + + ctx.icp() + .current_dir(&project_dir) + .args(["build", "--untrusted"]) + .assert() + .failure() + .stderr(contains("recipe.hbs").and(contains("is outside"))); +} + +#[test] +fn a_module_outside_the_project_is_refused_as_untrusted() { + let ctx = TestContext::new(); + let project_dir = project( + &ctx, + indoc! {r#" + canisters: + - name: backend + build: + steps: + - type: pre-built + path: ../outside.wasm + "#}, + ); + + ctx.icp() + .current_dir(&project_dir) + .args(["build"]) + .assert() + .success(); + + ctx.icp() + .current_dir(&project_dir) + .args(["build", "--untrusted"]) + .assert() + .failure() + .stderr( + contains("names '") + .and(contains("outside.wasm")) + .and(contains("not a regular file in the project")), + ); +} + +/// A symlink is refused wherever it leads, out of the project or within it: +/// as the module a step names, and as a file the project reads. +#[cfg(unix)] +#[test] +fn a_symlink_is_refused_as_untrusted_wherever_it_leads() { + let ctx = TestContext::new(); + let project_dir = project( + &ctx, + indoc! {r#" + canisters: + - name: backend + init_args: + path: args.bin + format: bin + build: + steps: + - type: pre-built + path: linked.wasm + "#}, + ); + let module = project_dir.join("linked.wasm"); + let args = project_dir.join("args.bin"); + let outside = project_dir.parent().unwrap().to_path_buf(); + std::fs::write(&args, b"args").expect("write args"); + + for target in [ + outside.join("outside.wasm"), + project_dir.join("backend.wasm"), + ] { + std::os::unix::fs::symlink(&target, &module).expect("symlink"); + ctx.icp() + .current_dir(&project_dir) + .args(["build"]) + .assert() + .success(); + ctx.icp() + .current_dir(&project_dir) + .args(["build", "--untrusted"]) + .assert() + .failure() + .stderr(contains("linked.wasm").and(contains("not a regular file in the project"))); + std::fs::remove_file(&module).expect("remove link"); + } + + std::fs::copy(project_dir.join("backend.wasm"), &module).expect("copy module"); + std::fs::remove_file(&args).expect("remove args"); + std::os::unix::fs::symlink(outside.join("outside.bin"), &args).expect("symlink"); + ctx.icp() + .current_dir(&project_dir) + .args(["build", "--untrusted"]) + .assert() + .failure() + .stderr(contains("args.bin").and(contains("is a symlink"))); +} + +/// A path that mentions a symlink is refused even where a `..` after it +/// would, by the spelling alone, step back to a file of the project's own: +/// the host follows the link and takes `..` from where it leads. +#[cfg(unix)] +#[test] +fn a_path_that_steps_back_out_of_a_symlink_is_refused_as_untrusted() { + let ctx = TestContext::new(); + let project_dir = project( + &ctx, + indoc! {r#" + canisters: + - name: backend + build: + steps: + - type: pre-built + path: jump/../backend.wasm + "#}, + ); + let elsewhere = project_dir.parent().unwrap().join("elsewhere/child"); + std::fs::create_dir_all(&elsewhere).expect("mkdir"); + std::os::unix::fs::symlink(&elsewhere, project_dir.join("jump")).expect("symlink"); + + ctx.icp() + .current_dir(&project_dir) + .args(["build", "--untrusted"]) + .assert() + .failure() + .stderr( + contains("jump/../backend.wasm").and(contains("not a regular file in the project")), + ); +} + +/// What the CLI keeps under `.icp` is held to the project too: a symlink +/// planted where the built module is stored is refused, not written through. +#[cfg(unix)] +#[test] +fn a_symlink_under_dot_icp_is_refused_as_untrusted() { + let ctx = TestContext::new(); + let project_dir = project(&ctx, PREBUILT); + let victim = project_dir.parent().unwrap().join("victim"); + std::fs::write(&victim, b"mine").expect("write victim"); + let artifacts = project_dir.join(".icp/cache/artifacts"); + std::fs::create_dir_all(&artifacts).expect("mkdir"); + std::os::unix::fs::symlink(&victim, artifacts.join("backend")).expect("symlink"); + + ctx.icp() + .current_dir(&project_dir) + .args(["build", "--untrusted"]) + .assert() + .failure() + .stderr(contains("failed to save wasm artifact")); + assert_eq!(std::fs::read(&victim).expect("read victim"), b"mine"); +} + +/// The root may be spelled relatively: it is taken from the working directory. +#[test] +fn a_relative_project_root_is_taken_from_the_working_directory() { + let ctx = TestContext::new(); + let project_dir = project(&ctx, PREBUILT); + let parent = project_dir.parent().unwrap(); + + ctx.icp() + .current_dir(parent) + .args(["build", "--untrusted", "--project-root-override", "icp"]) + .assert() + .success(); + + ctx.icp() + .current_dir(parent) + .env("ICP_PROJECT_ROOT", "icp") + .args(["build", "--untrusted"]) + .assert() + .success(); +} + +/// The flag is on the three commands that act on what a project says, and +/// each refuses before it reaches any network. It is on no other command. +#[test] +fn the_flag_is_on_build_deploy_and_sync_alone() { + let ctx = TestContext::new(); + let project_dir = project(&ctx, SCRIPT); + + for command in ["build", "deploy", "sync"] { + ctx.icp() + .current_dir(&project_dir) + .args([command, "--untrusted"]) + .assert() + .failure() + .stderr(contains("more than an untrusted project may")); + } + + ctx.icp() + .current_dir(&project_dir) + .args(["identity", "list", "--untrusted"]) + .assert() + .failure() + .stderr(contains("unexpected argument '--untrusted'")); +} diff --git a/crates/icp-project/Cargo.toml b/crates/icp-project/Cargo.toml index 70abb7dfd..447d4db12 100644 --- a/crates/icp-project/Cargo.toml +++ b/crates/icp-project/Cargo.toml @@ -17,7 +17,7 @@ default = ["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"] +host = ["dep:camino-tempfile", "dep:cap-std", "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 = [] @@ -28,6 +28,7 @@ bigdecimal = { workspace = true } camino = { workspace = true } camino-tempfile = { workspace = true, optional = true } candid = { workspace = true } +cap-std = { workspace = true, optional = true } candid_parser = { workspace = true } clap = { workspace = true, optional = true } dunce = { workspace = true, optional = true } diff --git a/crates/icp-project/src/files.rs b/crates/icp-project/src/files.rs index 860cee6a9..89751d25c 100644 --- a/crates/icp-project/src/files.rs +++ b/crates/icp-project/src/files.rs @@ -88,6 +88,303 @@ pub trait Scratch: Send + Sync { fn path(&self) -> &Path; } +/// This machine's filesystem, held to one directory, for a project that is +/// not the user's own tree: one handed over by someone else. +/// +/// The root is opened once, as a directory handle, and every read and write +/// is made beneath that handle through [`cap_std`]: the operating system +/// resolves the path and refuses one that leaves the directory, by `..`, by +/// an absolute path or by a symlink, in the same step that opens the file. +/// Where the kernel offers that as one call (`openat2` on Linux) it is one +/// call; elsewhere `cap_std` walks the path a handle at a time. +/// +/// On top of that, every path must reach what it names through no symlink. +/// A relative path is joined to the root, `.` is dropped, `..` is applied, +/// and a path that comes to rest outside the root is refused with +/// [`OutsideRoot`]. Each component below the root is looked at as it is on +/// disk, and a path that passes through a symlink is refused with +/// [`ThroughLink`], wherever the link leads, even where a later `..` steps +/// back out of it. The predicates answer `false` for a refused path, and +/// scratch directories are made under the root. +/// +/// The two differ in what a race can do to them. A link swapped in between +/// the look and the read is followed, since the look is a separate step, but +/// only to somewhere under the root: the handle does not let it lead out. +/// +/// This confines what is read and written through it, and nothing else: a +/// `script` build step in a manifest still runs any command, so whoever +/// holds a tree they do not trust must also refuse such steps or sandbox them. +/// +/// The loader reads every path through the [`FileSystem`] it is given, so +/// this holds all of them. A local recipe template is the exception: the +/// [`Resolve`](crate::canister::recipe::Resolve) implementation reads it, and +/// must read it through this filesystem or refuse it. +/// +/// Known issue: a predicate can only answer `true` or `false`, so the reason +/// a path was refused is lost there, and a caller that checks before it +/// reads reports a refused path as missing. The loader does this for a +/// canister directory and for a dependency's manifest: `canisters: +/// [../elsewhere]` fails as "not found" rather than as outside the root. +/// +/// The root is absolute, and is matched as the caller spelled it, not as it +/// canonicalizes. It and everything above it are the caller's, and may be +/// reached through a link; only what lies below is held to shape. +#[cfg(feature = "host")] +#[derive(Debug, Clone)] +pub struct Confined { + root: PathBuf, + dir: std::sync::Arc, +} + +/// A root a [`Confined`] filesystem could not be held to. +#[cfg(feature = "host")] +#[derive(Debug, Snafu)] +pub enum RootError { + #[snafu(display( + "'{root}' is not an absolute path, which the directory a project is confined to must be" + ))] + NotAbsolute { root: PathBuf }, + + #[snafu(display("failed to open '{root}', the directory to confine the project to"))] + Open { + source: std::io::Error, + root: PathBuf, + }, +} + +/// A path a [`Confined`] filesystem was asked for that is not under its root. +#[cfg(feature = "host")] +#[derive(Debug, Snafu)] +#[snafu(display("'{path}' is outside '{root}', the directory this project is confined to"))] +pub struct OutsideRoot { + pub path: PathBuf, + pub root: PathBuf, +} + +/// A path a [`Confined`] filesystem was asked for that passes through a +/// symlink. +#[cfg(feature = "host")] +#[derive(Debug, Snafu)] +#[snafu(display( + "'{path}' passes through '{link}', which is a symlink, and every path in this project must \ + name a regular file or directory under its root" +))] +pub struct ThroughLink { + pub path: PathBuf, + pub link: PathBuf, +} + +/// An operation beneath a [`Confined`] filesystem's root failed. +#[cfg(feature = "host")] +#[derive(Debug, Snafu)] +pub enum ConfinedError { + #[snafu(display("failed to read '{path}'"))] + Read { + source: std::io::Error, + path: PathBuf, + }, + + #[snafu(display("failed to write '{path}'"))] + Write { + source: std::io::Error, + path: PathBuf, + }, + + #[snafu(display("failed to create directory '{path}'"))] + CreateDir { + source: std::io::Error, + path: PathBuf, + }, + + #[snafu(display("failed to copy '{from}' to '{to}'"))] + Copy { + source: std::io::Error, + from: PathBuf, + to: PathBuf, + }, + + #[snafu(display("failed to list '{path}'"))] + ReadDir { + source: std::io::Error, + path: PathBuf, + }, +} + +#[cfg(feature = "host")] +impl Confined { + /// Confine the host filesystem to `root`, which must be absolute and a + /// directory that is there to open. + pub fn new(root: &Path) -> Result { + snafu::ensure!(root.is_absolute(), NotAbsoluteSnafu { root }); + let dir = cap_std::fs::Dir::open_ambient_dir(root, cap_std::ambient_authority()) + .context(OpenSnafu { root })?; + Ok(Self { + root: root.to_path_buf(), + dir: std::sync::Arc::new(dir), + }) + } + + /// The directory everything is held to. + pub fn root(&self) -> &Path { + &self.root + } + + /// Where `path` lies under the root, as a real path: refused if it would + /// come to rest outside, or passes through a symlink on the way. + /// + /// What a caller that needs a path for something that does not read + /// through the trait — a lock file, a runtime mounting a directory — + /// asks for. Such a caller opens the path itself, after this has looked, + /// so it has this filesystem's rule but not the handle's guarantee. + pub fn resolve(&self, path: &Path) -> Result { + Ok(self.root.join(self.beneath(path)?)) + } + + /// `path` relative to the root, with `.` dropped and `..` applied: + /// refused if it would come to rest outside the root, or passes through + /// a symlink on the way. Empty for the root itself. + fn beneath(&self, path: &Path) -> Result { + let outside = || { + FsError::new(OutsideRoot { + path: path.to_path_buf(), + root: self.root.clone(), + }) + }; + let relative = if path.is_absolute() { + path.strip_prefix(&self.root).map_err(|_| outside())? + } else { + path + }; + // Each component is looked at as it is on disk the moment it is + // named, before any `..` after it is applied: the host would follow a + // link and take `..` from where it leads, so a path that so much as + // mentions one is refused, dangling or not. + let mut beneath = PathBuf::new(); + for component in relative.components() { + match component { + Utf8Component::Normal(part) => { + beneath.push(part); + if self + .dir + .symlink_metadata(&beneath) + .is_ok_and(|m| m.file_type().is_symlink()) + { + return Err(FsError::new(ThroughLink { + path: path.to_path_buf(), + link: self.root.join(beneath), + })); + } + } + Utf8Component::CurDir => {} + Utf8Component::ParentDir => { + if !beneath.pop() { + return Err(outside()); + } + } + Utf8Component::RootDir | Utf8Component::Prefix(_) => return Err(outside()), + } + } + Ok(beneath) + } + + /// As [`beneath`](Self::beneath), spelled the way the directory handle + /// takes it: it has no name for itself but `.`. + fn at(&self, path: &Path) -> Result { + let beneath = self.beneath(path)?; + Ok(if beneath.as_str().is_empty() { + PathBuf::from(".") + } else { + beneath + }) + } +} + +#[cfg(feature = "host")] +#[async_trait] +impl FileSystem for Confined { + async fn read(&self, path: &Path) -> Result, FsError> { + self.dir + .read(self.at(path)?) + .context(ReadSnafu { path }) + .map_err(FsError::new) + } + + async fn read_to_string(&self, path: &Path) -> Result { + self.dir + .read_to_string(self.at(path)?) + .context(ReadSnafu { path }) + .map_err(FsError::new) + } + + async fn write(&self, path: &Path, contents: &[u8]) -> Result<(), FsError> { + self.dir + .write(self.at(path)?, contents) + .context(WriteSnafu { path }) + .map_err(FsError::new) + } + + async fn create_dir_all(&self, path: &Path) -> Result<(), FsError> { + self.dir + .create_dir_all(self.at(path)?) + .context(CreateDirSnafu { path }) + .map_err(FsError::new) + } + + async fn copy(&self, from: &Path, to: &Path) -> Result<(), FsError> { + self.dir + .copy(self.at(from)?, &self.dir, self.at(to)?) + .map(|_| ()) + .context(CopySnafu { from, to }) + .map_err(FsError::new) + } + + async fn exists(&self, path: &Path) -> bool { + self.at(path).is_ok_and(|at| self.dir.exists(at)) + } + + async fn is_file(&self, path: &Path) -> bool { + self.at(path) + .is_ok_and(|at| self.dir.metadata(at).is_ok_and(|m| m.is_file())) + } + + async fn is_dir(&self, path: &Path) -> bool { + self.at(path) + .is_ok_and(|at| self.dir.metadata(at).is_ok_and(|m| m.is_dir())) + } + + /// A name that is not UTF-8 is skipped rather than being an error, as the + /// host's own listing skips it. + async fn read_dir(&self, path: &Path) -> Result, FsError> { + let at = self.at(path)?; + let list = || -> std::io::Result> { + let mut out = Vec::new(); + for entry in self.dir.read_dir(at)? { + if let Ok(name) = entry?.file_name().into_string() { + out.push(path.join(name)); + } + } + out.sort(); + Ok(out) + }; + list().context(ReadDirSnafu { path }).map_err(FsError::new) + } + + /// The path under the root as the caller spelled the root, so that what + /// comes back can be asked for again. + async fn canonicalize(&self, path: &Path) -> Option { + let within = self.dir.canonicalize(self.at(path).ok()?).ok()?; + Some(self.root.join(PathBuf::from_path_buf(within).ok()?)) + } + + async fn scratch_dir(&self) -> Result, FsError> { + let dir = camino_tempfile::Builder::new() + .prefix(".scratch-") + .tempdir_in(&self.root) + .map_err(FsError::new)?; + Ok(Box::new(HostScratch(dir))) + } +} + #[cfg(feature = "host")] /// The [`FileSystem`] backed by this machine's filesystem. #[derive(Debug, Default, Clone, Copy)] @@ -385,6 +682,229 @@ async fn list( mod tests { use super::*; + /// Every spelling of a path resolves under the root or is refused: a + /// relative one is joined, `.` and `..` are applied, an absolute one is + /// taken only under the root, and none may pass through a symlink. + #[cfg(feature = "host")] + mod confined { + use super::*; + + fn root() -> (camino_tempfile::Utf8TempDir, Confined) { + let dir = camino_tempfile::Utf8TempDir::new().expect("temp dir"); + std::fs::create_dir_all(dir.path().join("project/canisters")).expect("mkdir"); + std::fs::write(dir.path().join("project/icp.yaml"), b"canisters: []").expect("write"); + std::fs::write(dir.path().join("project/canisters/a.wasm"), b"wasm").expect("write"); + std::fs::write(dir.path().join("outside.txt"), b"secret").expect("write"); + let fs = Confined::new(&dir.path().join("project")).expect("absolute"); + (dir, fs) + } + + #[tokio::test] + async fn paths_resolve_under_the_root_by_any_spelling_inside_it() { + let (_dir, fs) = root(); + let root = fs.root().to_path_buf(); + for spelling in [ + "canisters/a.wasm", + "./canisters/a.wasm", + "canisters/../canisters/a.wasm", + root.join("canisters/a.wasm").as_str(), + ] { + assert_eq!(fs.read(Path::new(spelling)).await.expect(spelling), b"wasm"); + assert!(fs.is_file(Path::new(spelling)).await, "{spelling}"); + assert_eq!( + fs.canonicalize(Path::new(spelling)).await, + Some(root.join("canisters/a.wasm")), + "{spelling}" + ); + } + assert!(fs.is_dir(Path::new("canisters")).await); + assert!(fs.is_dir(&root).await); + assert_eq!( + fs.read_dir(&root).await.expect("list"), + vec![root.join("canisters"), root.join("icp.yaml")] + ); + assert!( + fs.resolve(Path::new("made/later")) + .unwrap() + .starts_with(&root) + ); + } + + #[tokio::test] + async fn paths_outside_the_root_are_refused_however_spelled() { + let (dir, fs) = root(); + let outside = dir.path().join("outside.txt"); + assert!( + outside.is_file(), + "the file exists, for the host filesystem to read" + ); + for spelling in [ + "../outside.txt", + "canisters/../../outside.txt", + outside.as_str(), + "/etc/hostname", + "../../../../../../etc/hostname", + ] { + let err = fs + .read(Path::new(spelling)) + .await + .expect_err(spelling) + .to_string(); + assert!(err.contains("is outside"), "{spelling}: {err}"); + assert!(!fs.exists(Path::new(spelling)).await, "{spelling}"); + assert!(!fs.is_file(Path::new(spelling)).await, "{spelling}"); + assert!( + fs.canonicalize(Path::new(spelling)).await.is_none(), + "{spelling}" + ); + assert!(fs.resolve(Path::new(spelling)).is_err(), "{spelling}"); + } + // Nor may anything be written there. + let err = fs + .write(Path::new("../planted"), b"x") + .await + .unwrap_err() + .to_string(); + assert!(err.contains("is outside"), "{err}"); + assert!(!dir.path().join("planted").exists()); + } + + /// A temp dir on macOS is reached through a symlink (`/var`), so a + /// root compared by what it canonicalizes to would refuse every + /// absolute path spelled the way the caller got it. + #[cfg(unix)] + #[tokio::test] + async fn the_root_is_matched_as_the_caller_spells_it() { + let (dir, _) = root(); + std::os::unix::fs::symlink(dir.path().join("project"), dir.path().join("alias")) + .expect("symlink"); + let spelled = dir.path().join("alias"); + let fs = Confined::new(&spelled).expect("absolute"); + assert_eq!( + fs.read(&spelled.join("icp.yaml")).await.unwrap(), + b"canisters: []" + ); + assert_eq!( + fs.read(Path::new("icp.yaml")).await.unwrap(), + b"canisters: []" + ); + assert!(fs.is_dir(&spelled).await); + // What canonicalizing gives back is spelled the same way, so it + // can be asked for again. + let canonical = fs.canonicalize(Path::new("icp.yaml")).await.unwrap(); + assert_eq!(canonical, spelled.join("icp.yaml")); + assert_eq!(fs.read(&canonical).await.unwrap(), b"canisters: []"); + } + + #[test] + fn a_root_that_is_not_absolute_is_refused() { + let err = Confined::new(Path::new("project")).unwrap_err(); + assert!(err.to_string().contains("not an absolute path"), "{err}"); + } + + #[test] + fn a_root_that_is_not_a_directory_to_open_is_refused() { + let (dir, _) = root(); + for root in ["nowhere", "project/icp.yaml"] { + let err = Confined::new(&dir.path().join(root)).unwrap_err(); + assert!(err.to_string().contains("failed to open"), "{root}: {err}"); + } + } + + /// A path through a symlink is refused wherever the link leads: out + /// of the root, back inside it, or nowhere. A link in the middle of + /// a path counts as much as one at its end. + #[cfg(unix)] + #[tokio::test] + async fn a_path_through_a_symlink_is_refused_wherever_it_leads() { + use std::os::unix::fs::symlink; + let (dir, fs) = root(); + let project = fs.root().to_path_buf(); + symlink(dir.path().join("outside.txt"), project.join("out.txt")).expect("symlink"); + symlink(project.join("icp.yaml"), project.join("in.yaml")).expect("symlink"); + symlink(project.join("nowhere"), project.join("dangling")).expect("symlink"); + symlink(project.join("canisters"), project.join("dir-link")).expect("symlink"); + + for (spelling, link) in [ + ("out.txt", "out.txt"), + ("in.yaml", "in.yaml"), + ("dangling", "dangling"), + ("dir-link/a.wasm", "dir-link"), + ("./dir-link/../dir-link/a.wasm", "dir-link"), + // The host would take `..` from where the link leads; the + // spelling alone would cancel the link out and find the file. + ("dir-link/../canisters/a.wasm", "dir-link"), + ("dangling/../icp.yaml", "dangling"), + ] { + let err = fs + .read(Path::new(spelling)) + .await + .expect_err(spelling) + .to_string(); + assert!(err.contains("passes through"), "{spelling}: {err}"); + assert!(err.contains(link), "{spelling}: {err}"); + assert!(!fs.exists(Path::new(spelling)).await, "{spelling}"); + assert!(!fs.is_file(Path::new(spelling)).await, "{spelling}"); + assert!(!fs.is_dir(Path::new(spelling)).await, "{spelling}"); + assert!( + fs.canonicalize(Path::new(spelling)).await.is_none(), + "{spelling}" + ); + } + // Nor may a link be written through. + let err = fs + .write(Path::new("out.txt"), b"x") + .await + .unwrap_err() + .to_string(); + assert!(err.contains("passes through"), "{err}"); + assert_eq!( + std::fs::read(dir.path().join("outside.txt")).unwrap(), + b"secret" + ); + } + + /// The look for a link and the read are two steps, and the handle is + /// what holds if the first is got past: asked directly, it follows a + /// link that stays under the root and refuses one that leaves. + #[cfg(unix)] + #[test] + fn the_handle_refuses_a_link_out_of_the_root_by_itself() { + use std::os::unix::fs::symlink; + let (dir, fs) = root(); + let project = fs.root().to_path_buf(); + symlink(dir.path().join("outside.txt"), project.join("out.txt")).expect("symlink"); + symlink("../outside.txt", project.join("out-relative.txt")).expect("symlink"); + symlink("icp.yaml", project.join("in.yaml")).expect("symlink"); + + assert!(fs.dir.read("out.txt").is_err()); + assert!(fs.dir.read("out-relative.txt").is_err()); + assert!(fs.dir.read("canisters/../../outside.txt").is_err()); + assert!(fs.dir.write("out.txt", b"x").is_err()); + assert_eq!(fs.dir.read("in.yaml").unwrap(), b"canisters: []"); + assert_eq!( + std::fs::read(dir.path().join("outside.txt")).unwrap(), + b"secret" + ); + } + + #[tokio::test] + async fn scratch_is_made_under_the_root() { + let (_dir, fs) = root(); + let scratch = fs.scratch_dir().await.expect("scratch"); + assert!(scratch.path().starts_with(fs.root())); + let out = scratch.path().join("out/module.wasm"); + fs.create_dir_all(&scratch.path().join("out")) + .await + .expect("mkdir"); + fs.write(&out, b"built").await.expect("write"); + assert_eq!(fs.read(&out).await.unwrap(), b"built"); + let path = scratch.path().to_path_buf(); + drop(scratch); + assert!(!path.exists()); + } + } + /// Builds a tree of empty files, creating parents as needed, and returns its /// root. fn tree(paths: &[&str]) -> camino_tempfile::Utf8TempDir { diff --git a/crates/icp-project/src/lib.rs b/crates/icp-project/src/lib.rs index ec17458fc..831620667 100644 --- a/crates/icp-project/src/lib.rs +++ b/crates/icp-project/src/lib.rs @@ -43,6 +43,7 @@ pub mod random; pub mod signal; pub mod store_artifact; pub mod store_id; +pub mod untrusted; /// The per-project state directory. `cache` holds what can be thrown away and /// rebuilt (ids for managed networks, build artifacts); `data` holds what @@ -219,6 +220,12 @@ pub enum ProjectLoadError { Project { source: project::ConsolidateManifestError, }, + + #[snafu(display("the project asks for more than an untrusted project may"))] + Untrusted { + #[snafu(source(from(untrusted::Refused, Box::new)))] + source: Box, + }, } #[async_trait] diff --git a/crates/icp-project/src/project.rs b/crates/icp-project/src/project.rs index e8f28cfbc..83eb475f7 100644 --- a/crates/icp-project/src/project.rs +++ b/crates/icp-project/src/project.rs @@ -1690,6 +1690,115 @@ pub async fn consolidate_manifest( }) } +/// A project loaded through a [`Confined`] filesystem reads nothing outside +/// its root, whatever path its manifest names: the loader resolves every +/// path it reads itself through the filesystem it is given, so the one rule +/// holds them all. (A local recipe template is read by the [`Resolve`] +/// implementation instead, which is why these tests supply none.) +#[cfg(all(test, feature = "host"))] +mod confined_tests { + use super::*; + use crate::canister::recipe::{Fetched, Resolve, ResolveError}; + use crate::files::{Confined, HostFileSystem}; + use crate::manifest::recipe::Recipe; + use camino_tempfile::Utf8TempDir; + + struct NoRecipes; + + #[async_trait::async_trait] + impl Resolve for NoRecipes { + async fn resolve(&self, _recipe: &Recipe) -> Result { + unreachable!("no manifest here names a recipe") + } + } + + /// A tree with a project directory and, beside it, what the project must + /// not reach: an argument file, and a canister directory of its own. + fn tree(manifest: &str) -> Utf8TempDir { + let dir = Utf8TempDir::new().expect("temp dir"); + let project = dir.path().join("project"); + std::fs::create_dir_all(&project).expect("mkdir"); + std::fs::write(project.join(PROJECT_MANIFEST), manifest).expect("write"); + std::fs::write(dir.path().join("outside.bin"), b"secret").expect("write"); + std::fs::create_dir_all(dir.path().join("elsewhere")).expect("mkdir"); + std::fs::write( + dir.path().join("elsewhere").join(CANISTER_MANIFEST), + "name: stray\nbuild:\n steps:\n - type: pre-built\n path: stray.wasm\n", + ) + .expect("write"); + dir + } + + async fn load(files: &dyn FileSystem, pdir: &Path) -> Result { + let m: ProjectManifest = load_manifest_from_path(files, &pdir.join(PROJECT_MANIFEST)) + .await + .map_err(|e| chain(&e))?; + consolidate_manifest(files, pdir, &NoRecipes, &m) + .await + .map_err(|e| chain(&e)) + } + + fn chain(e: &(dyn std::error::Error + 'static)) -> String { + let mut text = e.to_string(); + let mut source = e.source(); + while let Some(cause) = source { + text.push_str(": "); + text.push_str(&cause.to_string()); + source = cause.source(); + } + text + } + + #[tokio::test] + async fn an_argument_file_outside_the_root_is_read_by_the_host_and_refused_confined() { + let dir = tree(indoc::indoc! {" + canisters: + - name: backend + init_args: + path: ../outside.bin + format: bin + build: + steps: + - type: pre-built + path: backend.wasm + "}); + let pdir = dir.path().join("project"); + + let project = load(&HostFileSystem, &pdir) + .await + .expect("the host reads anywhere"); + let (_, backend) = &project.canisters["backend"]; + assert_eq!( + backend.init_args, + Some(CanisterArgs::Binary(b"secret".to_vec())) + ); + + let confined = Confined::new(&pdir).expect("absolute"); + let err = load(&confined, &pdir) + .await + .expect_err("confined, the read is refused"); + assert!(err.contains("init_args"), "{err}"); + assert!(err.contains("is outside"), "{err}"); + } + + #[tokio::test] + async fn a_canister_directory_outside_the_root_is_found_by_the_host_and_not_confined() { + let dir = tree("canisters:\n - ../elsewhere\n"); + let pdir = dir.path().join("project"); + + let project = load(&HostFileSystem, &pdir) + .await + .expect("the host reads anywhere"); + assert!(project.canisters.contains_key("stray")); + + let confined = Confined::new(&pdir).expect("absolute"); + let err = load(&confined, &pdir) + .await + .expect_err("confined, the directory is not there"); + assert!(err.contains("../elsewhere"), "{err}"); + } +} + #[cfg(all(test, feature = "host"))] mod recipe_sync_tests { use super::*; diff --git a/crates/icp-project/src/store_artifact.rs b/crates/icp-project/src/store_artifact.rs index 707fe734c..30488a0db 100644 --- a/crates/icp-project/src/store_artifact.rs +++ b/crates/icp-project/src/store_artifact.rs @@ -6,6 +6,7 @@ use std::{collections::HashMap, sync::Mutex}; #[cfg(feature = "host")] use crate::{ CACHE_DIR, ICP_BASE, + files::{Confined, FsError}, fs::{ lock::{DirectoryStructureLock, PathsAccess}, read, write, @@ -66,6 +67,7 @@ pub enum LookupArtifactError { #[cfg(feature = "host")] pub struct ArtifactStore { project_root_locate: Arc, + confined: Option, } #[cfg(feature = "host")] @@ -124,23 +126,46 @@ impl PathsAccess for ArtifactPaths { #[cfg(feature = "host")] impl ArtifactStore { - pub fn new(project_root_locate: Arc) -> Self { + /// `confined` is given for a project that is not trusted: the store's + /// files are then held to it, so a symlink planted where the store keeps + /// one is refused rather than written or read through. + pub fn new( + project_root_locate: Arc, + confined: Option, + ) -> Self { Self { project_root_locate, + confined, } } - /// Locked directory access for the artifact store. It will create the directory if it does not exist. - fn lock(&self) -> Result, crate::fs::lock::LockError> { + fn paths(&self) -> ArtifactPaths { let project_root = self .project_root_locate .locate() .expect("failed to locate project root"); - let artifact_dir = project_root - .join(ICP_BASE) - .join(CACHE_DIR) - .join("artifacts"); - DirectoryStructureLock::open_or_create(ArtifactPaths { dir: artifact_dir }) + ArtifactPaths { + dir: project_root + .join(ICP_BASE) + .join(CACHE_DIR) + .join("artifacts"), + } + } + + /// Refuse the lock file and `name`'s artifact unless the filesystem the + /// project is confined to, if it is, would give them. + fn held(&self, name: &str) -> Result<(), FsError> { + if let Some(confined) = &self.confined { + let paths = self.paths(); + confined.resolve(&paths.lock_file())?; + confined.resolve(&paths.artifact_by_name(name))?; + } + Ok(()) + } + + /// Locked directory access for the artifact store. It will create the directory if it does not exist. + fn lock(&self) -> Result, crate::fs::lock::LockError> { + DirectoryStructureLock::open_or_create(self.paths()) } } @@ -181,6 +206,7 @@ impl Access for ArtifactStore { } .fail(); } + self.held(name).map_err(|e| save_store(name, e))?; self.lock() .map_err(|e| save_store(name, e))? .with_write(async |store| { @@ -198,6 +224,7 @@ impl Access for ArtifactStore { } .fail(); } + self.held(name).map_err(|e| lookup_store(name, e))?; self.lock() .map_err(|e| lookup_store(name, e))? .with_read(async |store| { @@ -301,3 +328,51 @@ mod tests { assert!(artifact_name_overflow(&deep).is_some()); } } + +#[cfg(all(test, unix, feature = "host"))] +mod confined_tests { + use super::*; + use crate::manifest::ProjectRootLocateImpl; + + /// A link planted where the store keeps an artifact, or its lock, is + /// refused when the project is confined, rather than written through. + #[tokio::test] + async fn an_artifact_behind_a_symlink_is_refused_when_confined() { + let dir = camino_tempfile::tempdir().unwrap(); + let root = dir.path().join("project"); + let artifacts = root.join(ICP_BASE).join(CACHE_DIR).join("artifacts"); + std::fs::create_dir_all(&artifacts).unwrap(); + std::fs::write(root.join("icp.yaml"), "canisters: []").unwrap(); + let victim = dir.path().join("victim"); + std::fs::write(&victim, "mine").unwrap(); + let locate = Arc::new(ProjectRootLocateImpl::new(root.clone(), Some(root.clone()))); + let store = ArtifactStore::new(locate, Some(Confined::new(&root).unwrap())); + + for planted in ["backend", ".lock"] { + let link = artifacts.join(planted); + std::os::unix::fs::symlink(&victim, &link).unwrap(); + let err = store.save("backend", b"wasm").await.unwrap_err(); + assert!( + matches!(err, SaveError::SaveStore { .. }), + "{planted}: {err}" + ); + let err = store.lookup("backend").await.unwrap_err(); + assert!( + matches!(err, LookupArtifactError::LookupStore { .. }), + "{planted}: {err}" + ); + assert_eq!( + std::fs::read_to_string(&victim).unwrap(), + "mine", + "{planted}" + ); + std::fs::remove_file(&link).unwrap(); + } + + store + .save("backend", b"wasm") + .await + .expect("no link, saved"); + assert_eq!(store.lookup("backend").await.unwrap(), b"wasm"); + } +} diff --git a/crates/icp-project/src/store_id.rs b/crates/icp-project/src/store_id.rs index f4e438df2..f63a84088 100644 --- a/crates/icp-project/src/store_id.rs +++ b/crates/icp-project/src/store_id.rs @@ -7,6 +7,7 @@ use crate::manifest::ProjectRootLocateError; #[cfg(feature = "host")] use crate::{ CACHE_DIR, DATA_DIR, ICP_BASE, + files::Confined, fs::{create_dir_all, json, remove_file}, manifest::ProjectRootLocate, prelude::*, @@ -161,17 +162,34 @@ impl std::error::Error for StoreCause { #[cfg(feature = "host")] pub struct AccessImpl { project_root_locate: Arc, + confined: Option, lock: Mutex<()>, } #[cfg(feature = "host")] impl AccessImpl { - pub fn new(project_root_locate: Arc) -> Self { + /// `confined` is given for a project that is not trusted: the store's + /// files are then held to it, so a symlink planted where the store keeps + /// one is refused rather than written or read through. + pub fn new( + project_root_locate: Arc, + confined: Option, + ) -> Self { Self { project_root_locate, + confined, lock: Mutex::new(()), } } + + /// Refuse `fpath` unless the filesystem the project is confined to, if + /// it is, would give it. + fn held(&self, fpath: &Path) -> Result<(), StoreCause> { + if let Some(confined) = &self.confined { + confined.resolve(fpath).map_err(StoreCause::new)?; + } + Ok(()) + } } #[cfg(feature = "host")] @@ -187,6 +205,11 @@ impl Access for AccessImpl { let _g = self.lock.lock().expect("failed to acquire id store lock"); let fpath = self.get_fpath_for_env(is_cache, env)?; + self.held(&fpath) + .map_err(|source| RegisterError::RegisterStore { + source, + env: env.to_owned(), + })?; create_dir_all(fpath.parent().unwrap()).map_err(|e| RegisterError::RegisterStore { source: StoreCause::new(e), env: env.to_owned(), @@ -227,6 +250,11 @@ impl Access for AccessImpl { let _g = self.lock.lock().expect("failed to acquire id store lock"); let fpath = self.get_fpath_for_env(is_cache, env)?; + self.held(&fpath) + .map_err(|source| UnregisterError::UnregisterStore { + source, + env: env.to_owned(), + })?; // Load the file let mut mapping = load_mapping(&fpath).map_err(|e| UnregisterError::UnregisterStore { @@ -254,6 +282,11 @@ impl Access for AccessImpl { ) -> Result { let _g = self.lock.lock().expect("failed to acquire id store lock"); let fpath = self.get_fpath_for_env(is_cache, env)?; + self.held(&fpath) + .map_err(|source| LookupIdError::LookupStore { + source, + env: env.to_owned(), + })?; load_mapping(&fpath) .map_err(|e| LookupIdError::LookupStore { source: StoreCause::new(e), @@ -270,6 +303,11 @@ impl Access for AccessImpl { fn lookup_by_environment(&self, is_cache: bool, env: &str) -> Result { let _g = self.lock.lock().expect("failed to acquire id store lock"); let fpath = self.get_fpath_for_env(is_cache, env)?; + self.held(&fpath) + .map_err(|source| LookupIdError::LookupStore { + source, + env: env.to_owned(), + })?; load_mapping(&fpath).map_err(|e| LookupIdError::LookupStore { source: StoreCause::new(e), env: env.to_owned(), @@ -279,6 +317,11 @@ impl Access for AccessImpl { fn cleanup(&self, is_cache: bool, env: &str) -> Result<(), CleanupError> { let _g = self.lock.lock().expect("failed to acquire id store lock"); let fpath = self.get_fpath_for_env(is_cache, env)?; + self.held(&fpath) + .map_err(|source| CleanupError::CleanupStore { + source, + env: env.to_owned(), + })?; if fpath.exists() { remove_file(&fpath).map_err(|e| CleanupError::CleanupStore { source: StoreCause::new(e), @@ -446,3 +489,34 @@ pub mod mock { } } } + +#[cfg(all(test, unix, feature = "host"))] +mod confined_tests { + use super::*; + use crate::manifest::ProjectRootLocateImpl; + + /// A link planted where the store keeps a mapping is refused when the + /// project is confined, rather than read or written through. + #[test] + fn a_mapping_behind_a_symlink_is_refused_when_confined() { + let dir = camino_tempfile::tempdir().unwrap(); + let root = dir.path().join("project"); + let mappings = root.join(ICP_BASE).join(DATA_DIR).join("mappings"); + std::fs::create_dir_all(&mappings).unwrap(); + std::fs::write(root.join("icp.yaml"), "canisters: []").unwrap(); + let victim = dir.path().join("victim.json"); + std::fs::write(&victim, "{}").unwrap(); + std::os::unix::fs::symlink(&victim, mappings.join("ic.ids.json")).unwrap(); + + let locate = Arc::new(ProjectRootLocateImpl::new(root.clone(), Some(root.clone()))); + let store = AccessImpl::new(locate, Some(Confined::new(&root).unwrap())); + + let err = store + .register(false, "ic", "backend", Principal::anonymous()) + .unwrap_err(); + assert!(matches!(err, RegisterError::RegisterStore { .. }), "{err}"); + let err = store.lookup(false, "ic", "backend").unwrap_err(); + assert!(matches!(err, LookupIdError::LookupStore { .. }), "{err}"); + assert_eq!(std::fs::read_to_string(&victim).unwrap(), "{}"); + } +} diff --git a/crates/icp-project/src/untrusted.rs b/crates/icp-project/src/untrusted.rs new file mode 100644 index 000000000..212db8f87 --- /dev/null +++ b/crates/icp-project/src/untrusted.rs @@ -0,0 +1,428 @@ +//! Holding a project someone else handed over to what it may read and run. +//! +//! The CLI trusts a project the way it trusts the user's own checkout: a +//! build step may run any command, a sync step any script, and a manifest +//! may name any path on the machine. A project from elsewhere — an archive +//! an agent passed to a deploy service, a checkout the user has not read — +//! may not. Two things hold it: +//! +//! - what it reads goes through a filesystem confined to the project root — +//! on a host, [`Confined`](crate::files::Confined) — so that every path a +//! manifest names is under the root and reached through no symlink. The +//! id and artifact stores under `.icp` are held to the same filesystem; +//! - what it runs is on a list, stated as what is allowed so that a kind of +//! step added later is refused until someone decides otherwise: a build +//! step is a pre-built module, a sync step is a plugin, and the file either +//! names is a regular file that filesystem will give. +//! +//! A module or plugin named by URL is not on the list, and the steps a recipe +//! renders to are held to it like any other. A checksum is not asked for: the +//! same untrusted hand wrote it and the module. [`vet`] applies the list to a +//! loaded [`Project`], and [`Vetted`] wraps a loader so that every load is +//! vetted before the project is handed out. + +use std::sync::Arc; + +use async_trait::async_trait; +use snafu::{ResultExt, Snafu, ensure}; + +use crate::{ + Project, ProjectLoad, ProjectLoadError, UntrustedSnafu, + files::FileSystem, + manifest::{ + adapter::prebuilt::{LocalSource, SourceField}, + canister::{BuildStep, SyncStep}, + }, + prelude::*, +}; + +/// Why a loaded project may not be acted on as untrusted. +#[derive(Debug, Snafu)] +pub enum Refused { + #[snafu(display( + "canister '{canister}' has a build step that is not a pre-built module, which is all an \ + untrusted project may have: {step}" + ))] + BuildStep { canister: String, step: String }, + + #[snafu(display( + "canister '{canister}' has a sync step that is not a plugin, which is all an untrusted \ + project may have: {step}" + ))] + SyncStep { canister: String, step: String }, + + #[snafu(display( + "canister '{canister}' names '{path}', which is not a regular file in the project: an \ + untrusted project runs only a file under its root, reached through no symlink" + ))] + NotAFile { canister: String, path: PathBuf }, +} + +/// The file `step` names, if it is the one build step an untrusted project +/// may have: a pre-built module from a file. +fn local_module(step: &BuildStep) -> Option<&LocalSource> { + match step { + BuildStep::Prebuilt(adapter) => local_source(&adapter.source), + BuildStep::Script(_) => None, + } +} + +/// The file `step` names, if it is the one sync step an untrusted project +/// may have: a plugin from a file. +fn local_plugin(step: &SyncStep) -> Option<&LocalSource> { + match step { + SyncStep::Plugin(adapter) => local_source(&adapter.source), + SyncStep::Script(_) => None, + } +} + +fn local_source(source: &SourceField) -> Option<&LocalSource> { + match source { + SourceField::Local(local) => Some(local), + SourceField::Remote(_) => None, + } +} + +/// Hold a loaded project to the list. +/// +/// `files` is the filesystem the project is confined to, and it is what +/// says whether the file a step names is one: under the root, through no +/// symlink, and a regular file. A plugin is opened by its runtime rather +/// than read through `files`, and a read of a pipe nobody writes to waits +/// forever, so the question is asked here, before anything opens the file. +pub async fn vet(project: &Project, files: &dyn FileSystem) -> Result<(), Refused> { + for (_, (canister_dir, canister)) in &project.canisters { + let build = canister.build.steps.iter().map(|step| { + local_module(step).ok_or_else(|| { + BuildStepSnafu { + canister: &canister.name, + step: one_line(step), + } + .build() + }) + }); + let sync = canister.sync.steps.iter().map(|step| { + local_plugin(step).ok_or_else(|| { + SyncStepSnafu { + canister: &canister.name, + step: one_line(step), + } + .build() + }) + }); + for local in build.chain(sync) { + let path = canister_dir.join(&local?.path); + ensure!( + files.is_file(&path).await, + NotAFileSnafu { + canister: &canister.name, + path, + } + ); + } + } + Ok(()) +} + +/// A step as it displays, on one line: its kind, then the lines after the +/// first run together with `;` between. +fn one_line(step: &impl std::fmt::Display) -> String { + let text = step.to_string(); + let mut lines = text.lines().map(str::trim).filter(|line| !line.is_empty()); + let kind = lines.next().unwrap_or_default(); + let rest = lines.collect::>().join("; "); + if rest.is_empty() { + kind.to_owned() + } else { + format!("{kind} {rest}") + } +} + +/// A [`ProjectLoad`] that hands out only a project [`vet`] accepts. +pub struct Vetted { + loader: L, + files: Arc, +} + +impl Vetted { + /// `files` is what the paths are resolved through: the filesystem the + /// project is on. + pub fn new(loader: L, files: Arc) -> Self { + Self { loader, files } + } +} + +#[async_trait] +impl ProjectLoad for Vetted { + async fn load(&self) -> Result { + let project = self.loader.load().await?; + vet(&project, self.files.as_ref()) + .await + .context(UntrustedSnafu)?; + Ok(project) + } + + async fn exists(&self) -> Result { + self.loader.exists().await + } + + fn member_dir(&self) -> Option { + self.loader.member_dir() + } +} + +#[cfg(test)] +mod tests { + use std::collections::HashSet; + + use super::*; + use crate::{ + MockProjectLoader, + files::{FsError, Scratch}, + manifest::{ + adapter::{prebuilt, script}, + canister::{BuildSteps, SyncSteps}, + }, + }; + + /// A filesystem that knows which paths are regular files, which is all + /// the vet asks of one. + struct Disk(HashSet); + + #[async_trait] + impl FileSystem for Disk { + async fn is_file(&self, path: &Path) -> bool { + self.0.contains(path) + } + async fn read(&self, _: &Path) -> Result, FsError> { + unreachable!("the vet reads nothing") + } + async fn read_to_string(&self, _: &Path) -> Result { + unreachable!("the vet reads nothing") + } + async fn write(&self, _: &Path, _: &[u8]) -> Result<(), FsError> { + unreachable!("the vet writes nothing") + } + async fn create_dir_all(&self, _: &Path) -> Result<(), FsError> { + unreachable!("the vet writes nothing") + } + async fn copy(&self, _: &Path, _: &Path) -> Result<(), FsError> { + unreachable!("the vet writes nothing") + } + async fn exists(&self, _: &Path) -> bool { + unreachable!("the vet asks only for a regular file") + } + async fn is_dir(&self, _: &Path) -> bool { + unreachable!("the vet asks only for a regular file") + } + async fn read_dir(&self, _: &Path) -> Result, FsError> { + unreachable!("the vet reads nothing") + } + async fn canonicalize(&self, _: &Path) -> Option { + unreachable!("the vet asks only for a regular file") + } + async fn scratch_dir(&self) -> Result, FsError> { + unreachable!("the vet writes nothing") + } + } + + /// The minimal project is at `/project`, and this is its disk: a module + /// and a plugin. + fn disk() -> Arc { + Arc::new(Disk(HashSet::from([ + "/project/backend.wasm".into(), + "/project/p.wasm".into(), + ]))) + } + + async fn project_with(build: Vec, sync: Vec) -> Project { + let mut project = MockProjectLoader::minimal().load().await.unwrap(); + let (_, canister) = project.canisters.get_mut("backend").unwrap(); + canister.build = BuildSteps { steps: build }; + canister.sync = SyncSteps { steps: sync }; + project + } + + async fn vetted(build: Vec, sync: Vec) -> Result<(), Refused> { + vet(&project_with(build, sync).await, disk().as_ref()).await + } + + fn local(path: &str) -> BuildStep { + BuildStep::Prebuilt(prebuilt::Adapter { + source: SourceField::Local(prebuilt::LocalSource { path: path.into() }), + sha256: None, + }) + } + + fn script(command: &str) -> script::Adapter { + script::Adapter { + command: script::CommandField::Command(command.to_owned()), + } + } + + fn plugin(source: &str) -> SyncStep { + SyncStep::Plugin(Box::new( + serde_yaml::from_str(&format!("{source}\nsha256: abc")).unwrap(), + )) + } + + #[tokio::test] + async fn a_bundle_shaped_project_is_accepted() { + vetted(vec![local("backend.wasm")], vec![plugin("path: p.wasm")]) + .await + .expect("pre-built from the project, plugin from the project"); + } + + /// A script step and a remote module are the two other build steps there + /// are. Neither is on the list, and the message says what the list is + /// and what the step was. + #[tokio::test] + async fn a_build_step_that_is_not_a_local_prebuilt_module_is_refused() { + let err = vetted(vec![BuildStep::Script(script("make"))], vec![]) + .await + .unwrap_err(); + assert!(matches!(err, Refused::BuildStep { ref canister, .. } if canister == "backend")); + let text = err.to_string(); + assert!(text.contains("not a pre-built module"), "{text}"); + assert!(text.contains("(script) make"), "{text}"); + + let several = script::Adapter { + command: script::CommandField::Commands(vec!["make".into(), "cp a b".into()]), + }; + let err = vetted(vec![BuildStep::Script(several)], vec![]) + .await + .unwrap_err(); + assert!(err.to_string().contains("(script) make; cp a b"), "{err}"); + + let remote = BuildStep::Prebuilt(prebuilt::Adapter { + source: SourceField::Remote(prebuilt::RemoteSource { + url: "https://example.com/backend.wasm".into(), + }), + sha256: Some("abc".into()), + }); + let err = vetted(vec![remote], vec![]).await.unwrap_err(); + assert!(matches!(err, Refused::BuildStep { .. })); + assert!( + err.to_string().contains("example.com/backend.wasm"), + "{err}" + ); + + // One good step does not carry a bad one. + let err = vetted( + vec![local("backend.wasm"), BuildStep::Script(script("make"))], + vec![], + ) + .await + .unwrap_err(); + assert!(matches!(err, Refused::BuildStep { .. })); + } + + #[tokio::test] + async fn a_sync_step_that_is_not_a_local_plugin_is_refused() { + let err = vetted( + vec![local("backend.wasm")], + vec![SyncStep::Script(script("upload"))], + ) + .await + .unwrap_err(); + assert!(matches!(err, Refused::SyncStep { .. })); + let text = err.to_string(); + assert!(text.contains("not a plugin"), "{text}"); + assert!(text.contains("script"), "{text}"); + + let err = vetted( + vec![local("backend.wasm")], + vec![plugin("url: https://example.com/plugin.wasm")], + ) + .await + .unwrap_err(); + assert!(matches!(err, Refused::SyncStep { .. })); + assert!(err.to_string().contains("example.com/plugin.wasm"), "{err}"); + } + + #[tokio::test] + async fn a_vetted_loader_refuses_what_vet_refuses() { + let project = project_with(vec![BuildStep::Script(script("make"))], vec![]).await; + let loader = Vetted::new(MockProjectLoader::new(project), disk()); + assert!(loader.exists().await.unwrap()); + let err = loader.load().await.unwrap_err(); + assert!(matches!(err, ProjectLoadError::Untrusted { .. }), "{err}"); + + let loader = Vetted::new(MockProjectLoader::minimal(), disk()); + loader + .load() + .await + .expect("the minimal project is pre-built from its own file"); + } + + /// What a step names must be a regular file the filesystem will give, + /// for a module and for a plugin alike. + #[tokio::test] + async fn what_a_step_names_must_be_a_regular_file() { + let err = vetted(vec![local("missing.wasm")], vec![]) + .await + .unwrap_err(); + assert!( + matches!(err, Refused::NotAFile { ref path, .. } if path == "/project/missing.wasm"), + "{err}" + ); + assert!(err.to_string().contains("not a regular file"), "{err}"); + + let err = vetted(vec![local("backend.wasm")], vec![plugin("path: gone.wasm")]) + .await + .unwrap_err(); + assert!(matches!(err, Refused::NotAFile { .. }), "{err}"); + } + + /// On a real disk, through the filesystem the CLI confines an untrusted + /// project to: a module of the project's own is accepted, with the root + /// spelled through a link as a macOS temp dir is, and one outside the + /// root, behind a link wherever it leads, or not a regular file is + /// refused, the pipe before anything tries to read it. + #[cfg(all(unix, feature = "host"))] + #[tokio::test] + async fn on_disk_a_step_names_a_regular_file_under_the_root_through_no_link() { + use crate::files::Confined; + use std::os::unix::fs::symlink; + + let dir = camino_tempfile::tempdir().unwrap(); + let real = dir.path().join("project"); + std::fs::create_dir(&real).unwrap(); + let root = dir.path().join("alias"); + symlink(&real, &root).unwrap(); + std::fs::write(root.join("backend.wasm"), b"module").unwrap(); + std::fs::write(dir.path().join("secret.wasm"), b"secret").unwrap(); + symlink(root.join("backend.wasm"), root.join("in.wasm")).unwrap(); + symlink(dir.path().join("secret.wasm"), root.join("out.wasm")).unwrap(); + symlink(dir.path(), root.join("outdir")).unwrap(); + std::fs::create_dir(root.join("dir.wasm")).unwrap(); + let made = std::process::Command::new("mkfifo") + .arg(root.join("pipe.wasm")) + .status() + .unwrap(); + assert!(made.success(), "mkfifo"); + let files = Confined::new(&root).unwrap(); + + let mut project = project_with(vec![local("backend.wasm")], vec![]).await; + project.dir = root.clone(); + project.canisters.get_mut("backend").unwrap().0 = root.clone(); + vet(&project, &files) + .await + .expect("the project's own module"); + + for path in [ + "../secret.wasm", + "in.wasm", + "out.wasm", + "outdir/secret.wasm", + "dir.wasm", + "pipe.wasm", + ] { + project.canisters.get_mut("backend").unwrap().1.build = BuildSteps { + steps: vec![local(path)], + }; + let err = vet(&project, &files).await.unwrap_err(); + assert!(matches!(err, Refused::NotAFile { .. }), "{path}: {err}"); + } + } +} diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 9c14edebb..53f370922 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -136,6 +136,7 @@ Build canisters ###### **Options:** * `-e`, `--environment ` — Override the environment to build for. By default, the local environment is used +* `--untrusted` — Hold the project to what one someone else handed over may do: every file it reads is under the project root and reached through no symlink, each build step is a pre-built module and each sync step a plugin. Anything else is refused @@ -1071,6 +1072,7 @@ using --args or --args-file: - `bin`: Raw binary (only valid for file references) +* `--untrusted` — Hold the project to what one someone else handed over may do: every file it reads is under the project root and reached through no symlink, each build step is a pre-built module and each sync step a plugin. Anything else is refused @@ -1919,6 +1921,7 @@ Synchronize canisters * `--proxy ` — Principal of a proxy canister to route sync plugin calls to the target canister through * `-e`, `--environment ` — Override the environment to connect to. By default, the local environment is used * `--identity ` — The user identity to run this command as +* `--untrusted` — Hold the project to what one someone else handed over may do: every file it reads is under the project root and reached through no symlink, each build step is a pre-built module and each sync step a plugin. Anything else is refused From 7a7872e4b1f7952d54934481f3a41f6858fa493d Mon Sep 17 00:00:00 2001 From: Raymond Khalife Date: Tue, 10 Nov 2026 17:22:00 +0000 Subject: [PATCH 2/8] fix(untrusted): refuse a pipe before it is read, vet what a plugin is handed, take a recipe path from the working directory - Confined refuses to read what is not a regular file, before the open: a pipe nobody writes to would otherwise hold an args, env or recipe read forever. - vet holds each dirs: and files: entry of a plugin step to a directory, or a regular file or directory, in the project. The plugin runtime opens those itself. - A relative file:// recipe path is joined to the working directory before it is read, as it resolves for a trusted project, rather than to the confined root. - Say that .icp is the tool's own state and is not vetted, and that the plugin runtime opens by path after the look. - Wrap the host feature list, which had passed taplo's column width. Co-Authored-By: Claude --- crates/icp-app/src/context/init.rs | 3 +- crates/icp-app/src/recipe.rs | 40 +++++++++++++- crates/icp-project/Cargo.toml | 11 +++- crates/icp-project/src/files.rs | 47 ++++++++++++++++- crates/icp-project/src/untrusted.rs | 82 ++++++++++++++++++++++++++++- 5 files changed, 176 insertions(+), 7 deletions(-) diff --git a/crates/icp-app/src/context/init.rs b/crates/icp-app/src/context/init.rs index 40bb93b53..abf61003a 100644 --- a/crates/icp-app/src/context/init.rs +++ b/crates/icp-app/src/context/init.rs @@ -98,7 +98,7 @@ pub fn initialize( // against the root as spelled, so the root has to be a whole path. let project_root_override = project_root_override.map(|dir| cwd.join(dir)); let project_root_locate = Arc::new(manifest::ProjectRootLocateImpl::new( - cwd, + cwd.clone(), project_root_override, )); @@ -149,6 +149,7 @@ pub fn initialize( http_client, pkg_cache, files: files.clone(), + cwd, }); // Canister builder diff --git a/crates/icp-app/src/recipe.rs b/crates/icp-app/src/recipe.rs index 3bc3b1571..11d304721 100644 --- a/crates/icp-app/src/recipe.rs +++ b/crates/icp-app/src/recipe.rs @@ -32,6 +32,9 @@ pub struct RecipeFetcher { pub pkg_cache: std::sync::Arc, /// The project's files, which a local template is read through pub files: std::sync::Arc, + /// Where a relative local template path is taken from: the working + /// directory, whatever `files` would otherwise join it to + pub cwd: PathBuf, } /// The result of the fetch stage, as this resolver sees it. @@ -135,7 +138,8 @@ impl RecipeFetcher { // Retrieve the template, using cache for remote/registry sources let (tmpl, should_cache) = match &tmpl_source { TemplateSource::LocalPath(path) => { - let bytes = self.files.read(path).await.context(ReadFileSnafu)?; + let path = self.cwd.join(path); + let bytes = self.files.read(&path).await.context(ReadFileSnafu)?; (parse_bytes_to_string(bytes)?, false) } @@ -363,6 +367,7 @@ mod tests { http_client: reqwest::Client::new(), pkg_cache: std::sync::Arc::new(crate::package::open(cache_dir.to_owned()).unwrap()), files: std::sync::Arc::new(icp_project::files::HostFileSystem), + cwd: PathBuf::new(), } } @@ -397,6 +402,39 @@ mod tests { ); } + /// A relative local path is taken from the working directory, the same + /// for a project read through a filesystem confined to its root as for + /// the user's own: not joined to that root instead. + #[tokio::test] + async fn a_relative_local_recipe_is_taken_from_the_working_directory() { + let tmp = camino_tempfile::Utf8TempDir::new().unwrap(); + let root = tmp.path().join("project"); + std::fs::create_dir(&root).unwrap(); + std::fs::write(root.join("recipe.hbs"), "inside").unwrap(); + std::fs::write(tmp.path().join("recipe.hbs"), "outside").unwrap(); + + let fetch = async |path: &str| { + let recipe = Recipe { + recipe_type: RecipeType::File(path.to_owned()), + configuration: Default::default(), + sha256: None, + }; + RecipeFetcher { + files: std::sync::Arc::new(icp_project::files::Confined::new(&root).unwrap()), + cwd: tmp.path().to_owned(), + ..fetcher(&tmp.path().join("pkg")) + } + .fetch_recipe(&recipe) + .await + }; + assert_eq!( + fetch("project/recipe.hbs").await.unwrap().template, + "inside" + ); + let err = icp_project::error::flatten(&fetch("recipe.hbs").await.err().unwrap()); + assert!(err.contains("is outside"), "{err}"); + } + /// A sha256 that does not match the template contents is rejected. #[tokio::test] async fn checksum_mismatch_is_rejected() { diff --git a/crates/icp-project/Cargo.toml b/crates/icp-project/Cargo.toml index 447d4db12..9ac549144 100644 --- a/crates/icp-project/Cargo.toml +++ b/crates/icp-project/Cargo.toml @@ -17,7 +17,16 @@ default = ["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:cap-std", "dep:dunce", "dep:rand", "dep:tar", "tokio/io-std", "tokio/process", "tokio/signal"] +host = [ + "dep:camino-tempfile", + "dep:cap-std", + "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 = [] diff --git a/crates/icp-project/src/files.rs b/crates/icp-project/src/files.rs index 89751d25c..e19cb8ea6 100644 --- a/crates/icp-project/src/files.rs +++ b/crates/icp-project/src/files.rs @@ -184,6 +184,11 @@ pub enum ConfinedError { path: PathBuf, }, + #[snafu(display( + "'{path}' is not a regular file, which is all that is read from this project" + ))] + NotAFile { path: PathBuf }, + #[snafu(display("failed to write '{path}'"))] Write { source: std::io::Error, @@ -287,6 +292,18 @@ impl Confined { Ok(beneath) } + /// As [`at`](Self::at), for a path about to be read: refused unless what + /// is there is a regular file. Opening a pipe nobody writes to waits + /// forever, so the question is asked before the open. A path with + /// nothing at it goes through, for the read to report as missing. + fn file_at(&self, path: &Path) -> Result { + let at = self.at(path)?; + if self.dir.metadata(&at).is_ok_and(|m| !m.is_file()) { + return Err(FsError::new(NotAFileSnafu { path }.build())); + } + Ok(at) + } + /// As [`beneath`](Self::beneath), spelled the way the directory handle /// takes it: it has no name for itself but `.`. fn at(&self, path: &Path) -> Result { @@ -304,14 +321,14 @@ impl Confined { impl FileSystem for Confined { async fn read(&self, path: &Path) -> Result, FsError> { self.dir - .read(self.at(path)?) + .read(self.file_at(path)?) .context(ReadSnafu { path }) .map_err(FsError::new) } async fn read_to_string(&self, path: &Path) -> Result { self.dir - .read_to_string(self.at(path)?) + .read_to_string(self.file_at(path)?) .context(ReadSnafu { path }) .map_err(FsError::new) } @@ -888,6 +905,32 @@ mod tests { ); } + /// A pipe passes for a path under the root through no link, and a + /// read of one nobody writes to would never come back. + #[cfg(unix)] + #[tokio::test] + async fn a_read_of_what_is_not_a_regular_file_is_refused_before_it_opens() { + let (_dir, fs) = root(); + let made = std::process::Command::new("mkfifo") + .arg(fs.root().join("pipe")) + .status() + .expect("mkfifo"); + assert!(made.success(), "mkfifo"); + for path in ["pipe", "canisters"] { + let err = fs.read(Path::new(path)).await.expect_err(path).to_string(); + assert!(err.contains("not a regular file"), "{path}: {err}"); + let err = fs + .read_to_string(Path::new(path)) + .await + .expect_err(path) + .to_string(); + assert!(err.contains("not a regular file"), "{path}: {err}"); + } + // What is not there is still reported as that. + let err = fs.read(Path::new("gone")).await.unwrap_err().to_string(); + assert!(err.contains("failed to read"), "{err}"); + } + #[tokio::test] async fn scratch_is_made_under_the_root() { let (_dir, fs) = root(); diff --git a/crates/icp-project/src/untrusted.rs b/crates/icp-project/src/untrusted.rs index 212db8f87..fa9166a33 100644 --- a/crates/icp-project/src/untrusted.rs +++ b/crates/icp-project/src/untrusted.rs @@ -15,6 +15,17 @@ //! step is a pre-built module, a sync step is a plugin, and the file either //! names is a regular file that filesystem will give. //! +//! What the tool keeps under `.icp` is not part of the project: it is state +//! an earlier run wrote and a later one reads back as its own, a network's +//! descriptor among it, which says where that network's files are. Whoever +//! takes a project from elsewhere discards any `.icp` that came with it, and +//! nothing here vets what one holds. +//! +//! A plugin's runtime opens the plugin, and each file and directory the step +//! hands it, by path, after [`vet`] has looked. So those have the rule but +//! not the guarantee a read through the confined filesystem has: a tree +//! rewritten between the look and the open is not caught. +//! //! A module or plugin named by URL is not on the list, and the steps a recipe //! renders to are held to it like any other. A checksum is not asked for: the //! same untrusted hand wrote it and the module. [`vet`] applies the list to a @@ -30,7 +41,10 @@ use crate::{ Project, ProjectLoad, ProjectLoadError, UntrustedSnafu, files::FileSystem, manifest::{ - adapter::prebuilt::{LocalSource, SourceField}, + adapter::{ + plugin::NamedPaths, + prebuilt::{LocalSource, SourceField}, + }, canister::{BuildStep, SyncStep}, }, prelude::*, @@ -56,6 +70,13 @@ pub enum Refused { untrusted project runs only a file under its root, reached through no symlink" ))] NotAFile { canister: String, path: PathBuf }, + + #[snafu(display( + "canister '{canister}' gives its sync plugin '{path}', which is not a regular file or \ + directory in the project: an untrusted project hands a plugin only what is under its \ + root, reached through no symlink" + ))] + PluginInput { canister: String, path: PathBuf }, } /// The file `step` names, if it is the one build step an untrusted project @@ -90,6 +111,9 @@ fn local_source(source: &SourceField) -> Option<&LocalSource> { /// symlink, and a regular file. A plugin is opened by its runtime rather /// than read through `files`, and a read of a pipe nobody writes to waits /// forever, so the question is asked here, before anything opens the file. +/// The same is asked of what a plugin step hands its plugin, which the +/// runtime also opens itself: a `dirs` entry is a directory, and a `files` +/// entry a regular file or a directory. pub async fn vet(project: &Project, files: &dyn FileSystem) -> Result<(), Refused> { for (_, (canister_dir, canister)) in &project.canisters { let build = canister.build.steps.iter().map(|step| { @@ -120,6 +144,28 @@ pub async fn vet(project: &Project, files: &dyn FileSystem) -> Result<(), Refuse } ); } + for step in &canister.sync.steps { + let SyncStep::Plugin(adapter) = step else { + continue; + }; + let dirs = adapter.dirs.iter().flat_map(NamedPaths::entries); + let named = adapter.files.iter().flat_map(NamedPaths::entries); + let inputs = dirs + .map(|entry| (entry.path, false)) + .chain(named.map(|entry| (entry.path, true))); + for (declared, may_be_a_file) in inputs { + let path = canister_dir.join(declared); + let held = + (may_be_a_file && files.is_file(&path).await) || files.is_dir(&path).await; + ensure!( + held, + PluginInputSnafu { + canister: &canister.name, + path, + } + ); + } + } } Ok(()) } @@ -213,7 +259,7 @@ mod tests { unreachable!("the vet asks only for a regular file") } async fn is_dir(&self, _: &Path) -> bool { - unreachable!("the vet asks only for a regular file") + false } async fn read_dir(&self, _: &Path) -> Result, FsError> { unreachable!("the vet reads nothing") @@ -424,5 +470,37 @@ mod tests { let err = vet(&project, &files).await.unwrap_err(); assert!(matches!(err, Refused::NotAFile { .. }), "{path}: {err}"); } + + // What a plugin is handed is held the same way: its runtime opens + // each entry itself, and would wait on the pipe. + std::fs::write(root.join("p.wasm"), b"plugin").unwrap(); + std::fs::write(root.join("seed.txt"), b"seed").unwrap(); + std::fs::create_dir(root.join("assets")).unwrap(); + let with_inputs = |inputs: &str| SyncSteps { + steps: vec![plugin(&format!("path: p.wasm\n{inputs}"))], + }; + let canister = &mut project.canisters.get_mut("backend").unwrap().1; + canister.build = BuildSteps { + steps: vec![local("backend.wasm")], + }; + canister.sync = with_inputs("files:\n seed: seed.txt\n assets: assets"); + vet(&project, &files) + .await + .expect("the project's own files"); + for inputs in [ + "files:\n seed: pipe.wasm", + "files:\n seed: out.wasm", + "files:\n seed: ../secret.wasm", + "files:\n seed: gone", + "dirs: [outdir]", + "dirs: [seed.txt]", + ] { + project.canisters.get_mut("backend").unwrap().1.sync = with_inputs(inputs); + let err = vet(&project, &files).await.unwrap_err(); + assert!( + matches!(err, Refused::PluginInput { .. }), + "{inputs}: {err}" + ); + } } } From fdc3ed8f48d6cb637dd994cfecf4ff1efa1e2bb5 Mon Sep 17 00:00:00 2001 From: Raymond Khalife Date: Tue, 10 Nov 2026 17:26:22 +0000 Subject: [PATCH 3/8] refactor(untrusted): leave .icp to the tool, unguarded What is under .icp is the tool's own state, and whoever takes a project from elsewhere discards any that came with it. So the id store, the artifact store and a network's directory go back to what they are on main, with no confined filesystem handed to them. Co-Authored-By: Claude --- CHANGELOG.md | 2 +- crates/icp-app/src/context/init.rs | 14 +--- crates/icp-app/src/network/accessor.rs | 18 +---- crates/icp-app/src/network/directory.rs | 15 ---- crates/icp-cli/tests/untrusted_tests.rs | 22 ------ crates/icp-project/src/files.rs | 2 +- crates/icp-project/src/store_artifact.rs | 91 +++--------------------- crates/icp-project/src/store_id.rs | 76 +------------------- crates/icp-project/src/untrusted.rs | 3 +- 9 files changed, 17 insertions(+), 226 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 309183e04..fc4e56a07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ air-gapped signing, extensions # Unreleased -* feat: `--untrusted`, on `build`, `deploy` and `sync`, holds a project someone else handed over to what it may read and run. Every file the project reads — manifests, argument and environment files, local recipe templates, modules — must be under the project root and reached through no symlink. Each build step must be a pre-built module and each sync step a plugin, taken from a regular file in the project; the steps a recipe renders to are held to the same list. Anything else is refused before the command acts on it, naming what was found. What the CLI keeps under `.icp` is held to the same rule, so a symlink planted there is refused rather than written through. +* feat: `--untrusted`, on `build`, `deploy` and `sync`, holds a project someone else handed over to what it may read and run. Every file the project reads — manifests, argument and environment files, local recipe templates, modules — must be under the project root and reached through no symlink. Each build step must be a pre-built module and each sync step a plugin, taken from a regular file in the project; the steps a recipe renders to are held to the same list. Anything else is refused before the command acts on it, naming what was found. * chore: macOS on Intel (`x86_64-apple-darwin`) is no longer a release target. Releases ship no Intel macOS archive, the shell installer refuses that platform, and npm no longer publishes `@icp-sdk/icp-cli-darwin-x64`, so installing `@icp-sdk/icp-cli` on an Intel Mac warns of an unsupported platform. Homebrew dropped Intel macOS in September 2026, and GitHub is retiring its Intel macOS runners. v1.8.0 is the last release with an Intel macOS build. ## Experimental diff --git a/crates/icp-app/src/context/init.rs b/crates/icp-app/src/context/init.rs index abf61003a..13b26ec22 100644 --- a/crates/icp-app/src/context/init.rs +++ b/crates/icp-app/src/context/init.rs @@ -102,8 +102,7 @@ pub fn initialize( project_root_override, )); - // An untrusted project is confined to its directory: what it reads, and - // what the CLI keeps under `.icp` on its behalf. + // An untrusted project is confined to its directory in what it reads. let confined = if untrusted { let root = project_root_locate.locate().context(UntrustedRootSnafu)?; Some(Confined::new(&root).context(ConfineSnafu)?) @@ -112,16 +111,10 @@ pub fn initialize( }; // Canister ID Store - let ids = Arc::new(store_id::AccessImpl::new( - project_root_locate.clone(), - confined.clone(), - )); + let ids = Arc::new(store_id::AccessImpl::new(project_root_locate.clone())); // Canister Artifact Store (wasm) - let artifacts = Arc::new(ArtifactStore::new( - project_root_locate.clone(), - confined.clone(), - )); + let artifacts = Arc::new(ArtifactStore::new(project_root_locate.clone())); // Prepare http client let http_client = reqwest::Client::new(); @@ -198,7 +191,6 @@ pub fn initialize( // Network accessor let netaccess = Arc::new(crate::network::Accessor { project_root_locate: project_root_locate.clone(), - confined, descriptors: dirs.port_descriptor(), agent: agent_creator.clone(), }); diff --git a/crates/icp-app/src/network/accessor.rs b/crates/icp-app/src/network/accessor.rs index 075a43ddf..a97d770b5 100644 --- a/crates/icp-app/src/network/accessor.rs +++ b/crates/icp-app/src/network/accessor.rs @@ -8,7 +8,6 @@ use candid::Principal; use snafu::{ResultExt, Snafu}; use url::Url; -use icp_project::files::{Confined, FsError}; use icp_project::manifest::{ProjectRootLocate, ProjectRootLocateError}; use icp_project::network::{ Access, AccessError, CollectFriendlyDomains, Configuration, NetworkAccess, NetworkUrls, @@ -27,9 +26,6 @@ use crate::network::{ pub enum LocateNetworkDirectoryError { #[snafu(display("failed to find project root"))] ProjectRootLocate { source: ProjectRootLocateError }, - - #[snafu(display("network '{network}' keeps its state where an untrusted project may not"))] - Confine { source: FsError, network: String }, } /// Where a network keeps its on-disk state. @@ -61,10 +57,6 @@ pub struct Accessor { // Project root pub project_root_locate: Arc, - // What an untrusted project is confined to, which its network - // directories are then held to as well - pub confined: Option, - // Port descriptors dir pub descriptors: PathBuf, @@ -82,20 +74,14 @@ impl Accessor { .project_root_locate .locate() .context(ProjectRootLocateSnafu)?; - let nd = NetworkDirectory::new( + Ok(NetworkDirectory::new( &network.name, &dir.join(ICP_BASE) .join(CACHE_DIR) .join("networks") .join(&network.name), &self.descriptors, - ); - if let Some(confined) = &self.confined { - nd.held_by(confined).context(ConfineSnafu { - network: &network.name, - })?; - } - Ok(nd) + )) } } diff --git a/crates/icp-app/src/network/directory.rs b/crates/icp-app/src/network/directory.rs index 6344ea04b..2018feec9 100644 --- a/crates/icp-app/src/network/directory.rs +++ b/crates/icp-app/src/network/directory.rs @@ -168,21 +168,6 @@ impl NetworkDirectory { Ok(()) } - /// Refuse the files this directory is read through — its lock and its - /// descriptor — unless `confined`, the filesystem an untrusted project is - /// held to, would give them. - pub(crate) fn held_by( - &self, - confined: &icp_project::files::Confined, - ) -> Result<(), icp_project::files::FsError> { - let paths = NetworkRootPaths { - network_root: self.network_root.clone(), - }; - confined.resolve(&paths.lock_file())?; - confined.resolve(&paths.network_descriptor_path())?; - Ok(()) - } - /// Locked directory access for the local network root. pub fn root(&self) -> Result, LockError> { DirectoryStructureLock::open_or_create(NetworkRootPaths { diff --git a/crates/icp-cli/tests/untrusted_tests.rs b/crates/icp-cli/tests/untrusted_tests.rs index 38587b163..aa8bc7363 100644 --- a/crates/icp-cli/tests/untrusted_tests.rs +++ b/crates/icp-cli/tests/untrusted_tests.rs @@ -300,28 +300,6 @@ fn a_path_that_steps_back_out_of_a_symlink_is_refused_as_untrusted() { ); } -/// What the CLI keeps under `.icp` is held to the project too: a symlink -/// planted where the built module is stored is refused, not written through. -#[cfg(unix)] -#[test] -fn a_symlink_under_dot_icp_is_refused_as_untrusted() { - let ctx = TestContext::new(); - let project_dir = project(&ctx, PREBUILT); - let victim = project_dir.parent().unwrap().join("victim"); - std::fs::write(&victim, b"mine").expect("write victim"); - let artifacts = project_dir.join(".icp/cache/artifacts"); - std::fs::create_dir_all(&artifacts).expect("mkdir"); - std::os::unix::fs::symlink(&victim, artifacts.join("backend")).expect("symlink"); - - ctx.icp() - .current_dir(&project_dir) - .args(["build", "--untrusted"]) - .assert() - .failure() - .stderr(contains("failed to save wasm artifact")); - assert_eq!(std::fs::read(&victim).expect("read victim"), b"mine"); -} - /// The root may be spelled relatively: it is taken from the working directory. #[test] fn a_relative_project_root_is_taken_from_the_working_directory() { diff --git a/crates/icp-project/src/files.rs b/crates/icp-project/src/files.rs index e19cb8ea6..8679677c0 100644 --- a/crates/icp-project/src/files.rs +++ b/crates/icp-project/src/files.rs @@ -238,7 +238,7 @@ impl Confined { /// come to rest outside, or passes through a symlink on the way. /// /// What a caller that needs a path for something that does not read - /// through the trait — a lock file, a runtime mounting a directory — + /// through the trait — a runtime mounting a directory — /// asks for. Such a caller opens the path itself, after this has looked, /// so it has this filesystem's rule but not the handle's guarantee. pub fn resolve(&self, path: &Path) -> Result { diff --git a/crates/icp-project/src/store_artifact.rs b/crates/icp-project/src/store_artifact.rs index 30488a0db..707fe734c 100644 --- a/crates/icp-project/src/store_artifact.rs +++ b/crates/icp-project/src/store_artifact.rs @@ -6,7 +6,6 @@ use std::{collections::HashMap, sync::Mutex}; #[cfg(feature = "host")] use crate::{ CACHE_DIR, ICP_BASE, - files::{Confined, FsError}, fs::{ lock::{DirectoryStructureLock, PathsAccess}, read, write, @@ -67,7 +66,6 @@ pub enum LookupArtifactError { #[cfg(feature = "host")] pub struct ArtifactStore { project_root_locate: Arc, - confined: Option, } #[cfg(feature = "host")] @@ -126,46 +124,23 @@ impl PathsAccess for ArtifactPaths { #[cfg(feature = "host")] impl ArtifactStore { - /// `confined` is given for a project that is not trusted: the store's - /// files are then held to it, so a symlink planted where the store keeps - /// one is refused rather than written or read through. - pub fn new( - project_root_locate: Arc, - confined: Option, - ) -> Self { + pub fn new(project_root_locate: Arc) -> Self { Self { project_root_locate, - confined, } } - fn paths(&self) -> ArtifactPaths { + /// Locked directory access for the artifact store. It will create the directory if it does not exist. + fn lock(&self) -> Result, crate::fs::lock::LockError> { let project_root = self .project_root_locate .locate() .expect("failed to locate project root"); - ArtifactPaths { - dir: project_root - .join(ICP_BASE) - .join(CACHE_DIR) - .join("artifacts"), - } - } - - /// Refuse the lock file and `name`'s artifact unless the filesystem the - /// project is confined to, if it is, would give them. - fn held(&self, name: &str) -> Result<(), FsError> { - if let Some(confined) = &self.confined { - let paths = self.paths(); - confined.resolve(&paths.lock_file())?; - confined.resolve(&paths.artifact_by_name(name))?; - } - Ok(()) - } - - /// Locked directory access for the artifact store. It will create the directory if it does not exist. - fn lock(&self) -> Result, crate::fs::lock::LockError> { - DirectoryStructureLock::open_or_create(self.paths()) + let artifact_dir = project_root + .join(ICP_BASE) + .join(CACHE_DIR) + .join("artifacts"); + DirectoryStructureLock::open_or_create(ArtifactPaths { dir: artifact_dir }) } } @@ -206,7 +181,6 @@ impl Access for ArtifactStore { } .fail(); } - self.held(name).map_err(|e| save_store(name, e))?; self.lock() .map_err(|e| save_store(name, e))? .with_write(async |store| { @@ -224,7 +198,6 @@ impl Access for ArtifactStore { } .fail(); } - self.held(name).map_err(|e| lookup_store(name, e))?; self.lock() .map_err(|e| lookup_store(name, e))? .with_read(async |store| { @@ -328,51 +301,3 @@ mod tests { assert!(artifact_name_overflow(&deep).is_some()); } } - -#[cfg(all(test, unix, feature = "host"))] -mod confined_tests { - use super::*; - use crate::manifest::ProjectRootLocateImpl; - - /// A link planted where the store keeps an artifact, or its lock, is - /// refused when the project is confined, rather than written through. - #[tokio::test] - async fn an_artifact_behind_a_symlink_is_refused_when_confined() { - let dir = camino_tempfile::tempdir().unwrap(); - let root = dir.path().join("project"); - let artifacts = root.join(ICP_BASE).join(CACHE_DIR).join("artifacts"); - std::fs::create_dir_all(&artifacts).unwrap(); - std::fs::write(root.join("icp.yaml"), "canisters: []").unwrap(); - let victim = dir.path().join("victim"); - std::fs::write(&victim, "mine").unwrap(); - let locate = Arc::new(ProjectRootLocateImpl::new(root.clone(), Some(root.clone()))); - let store = ArtifactStore::new(locate, Some(Confined::new(&root).unwrap())); - - for planted in ["backend", ".lock"] { - let link = artifacts.join(planted); - std::os::unix::fs::symlink(&victim, &link).unwrap(); - let err = store.save("backend", b"wasm").await.unwrap_err(); - assert!( - matches!(err, SaveError::SaveStore { .. }), - "{planted}: {err}" - ); - let err = store.lookup("backend").await.unwrap_err(); - assert!( - matches!(err, LookupArtifactError::LookupStore { .. }), - "{planted}: {err}" - ); - assert_eq!( - std::fs::read_to_string(&victim).unwrap(), - "mine", - "{planted}" - ); - std::fs::remove_file(&link).unwrap(); - } - - store - .save("backend", b"wasm") - .await - .expect("no link, saved"); - assert_eq!(store.lookup("backend").await.unwrap(), b"wasm"); - } -} diff --git a/crates/icp-project/src/store_id.rs b/crates/icp-project/src/store_id.rs index f63a84088..f4e438df2 100644 --- a/crates/icp-project/src/store_id.rs +++ b/crates/icp-project/src/store_id.rs @@ -7,7 +7,6 @@ use crate::manifest::ProjectRootLocateError; #[cfg(feature = "host")] use crate::{ CACHE_DIR, DATA_DIR, ICP_BASE, - files::Confined, fs::{create_dir_all, json, remove_file}, manifest::ProjectRootLocate, prelude::*, @@ -162,34 +161,17 @@ impl std::error::Error for StoreCause { #[cfg(feature = "host")] pub struct AccessImpl { project_root_locate: Arc, - confined: Option, lock: Mutex<()>, } #[cfg(feature = "host")] impl AccessImpl { - /// `confined` is given for a project that is not trusted: the store's - /// files are then held to it, so a symlink planted where the store keeps - /// one is refused rather than written or read through. - pub fn new( - project_root_locate: Arc, - confined: Option, - ) -> Self { + pub fn new(project_root_locate: Arc) -> Self { Self { project_root_locate, - confined, lock: Mutex::new(()), } } - - /// Refuse `fpath` unless the filesystem the project is confined to, if - /// it is, would give it. - fn held(&self, fpath: &Path) -> Result<(), StoreCause> { - if let Some(confined) = &self.confined { - confined.resolve(fpath).map_err(StoreCause::new)?; - } - Ok(()) - } } #[cfg(feature = "host")] @@ -205,11 +187,6 @@ impl Access for AccessImpl { let _g = self.lock.lock().expect("failed to acquire id store lock"); let fpath = self.get_fpath_for_env(is_cache, env)?; - self.held(&fpath) - .map_err(|source| RegisterError::RegisterStore { - source, - env: env.to_owned(), - })?; create_dir_all(fpath.parent().unwrap()).map_err(|e| RegisterError::RegisterStore { source: StoreCause::new(e), env: env.to_owned(), @@ -250,11 +227,6 @@ impl Access for AccessImpl { let _g = self.lock.lock().expect("failed to acquire id store lock"); let fpath = self.get_fpath_for_env(is_cache, env)?; - self.held(&fpath) - .map_err(|source| UnregisterError::UnregisterStore { - source, - env: env.to_owned(), - })?; // Load the file let mut mapping = load_mapping(&fpath).map_err(|e| UnregisterError::UnregisterStore { @@ -282,11 +254,6 @@ impl Access for AccessImpl { ) -> Result { let _g = self.lock.lock().expect("failed to acquire id store lock"); let fpath = self.get_fpath_for_env(is_cache, env)?; - self.held(&fpath) - .map_err(|source| LookupIdError::LookupStore { - source, - env: env.to_owned(), - })?; load_mapping(&fpath) .map_err(|e| LookupIdError::LookupStore { source: StoreCause::new(e), @@ -303,11 +270,6 @@ impl Access for AccessImpl { fn lookup_by_environment(&self, is_cache: bool, env: &str) -> Result { let _g = self.lock.lock().expect("failed to acquire id store lock"); let fpath = self.get_fpath_for_env(is_cache, env)?; - self.held(&fpath) - .map_err(|source| LookupIdError::LookupStore { - source, - env: env.to_owned(), - })?; load_mapping(&fpath).map_err(|e| LookupIdError::LookupStore { source: StoreCause::new(e), env: env.to_owned(), @@ -317,11 +279,6 @@ impl Access for AccessImpl { fn cleanup(&self, is_cache: bool, env: &str) -> Result<(), CleanupError> { let _g = self.lock.lock().expect("failed to acquire id store lock"); let fpath = self.get_fpath_for_env(is_cache, env)?; - self.held(&fpath) - .map_err(|source| CleanupError::CleanupStore { - source, - env: env.to_owned(), - })?; if fpath.exists() { remove_file(&fpath).map_err(|e| CleanupError::CleanupStore { source: StoreCause::new(e), @@ -489,34 +446,3 @@ pub mod mock { } } } - -#[cfg(all(test, unix, feature = "host"))] -mod confined_tests { - use super::*; - use crate::manifest::ProjectRootLocateImpl; - - /// A link planted where the store keeps a mapping is refused when the - /// project is confined, rather than read or written through. - #[test] - fn a_mapping_behind_a_symlink_is_refused_when_confined() { - let dir = camino_tempfile::tempdir().unwrap(); - let root = dir.path().join("project"); - let mappings = root.join(ICP_BASE).join(DATA_DIR).join("mappings"); - std::fs::create_dir_all(&mappings).unwrap(); - std::fs::write(root.join("icp.yaml"), "canisters: []").unwrap(); - let victim = dir.path().join("victim.json"); - std::fs::write(&victim, "{}").unwrap(); - std::os::unix::fs::symlink(&victim, mappings.join("ic.ids.json")).unwrap(); - - let locate = Arc::new(ProjectRootLocateImpl::new(root.clone(), Some(root.clone()))); - let store = AccessImpl::new(locate, Some(Confined::new(&root).unwrap())); - - let err = store - .register(false, "ic", "backend", Principal::anonymous()) - .unwrap_err(); - assert!(matches!(err, RegisterError::RegisterStore { .. }), "{err}"); - let err = store.lookup(false, "ic", "backend").unwrap_err(); - assert!(matches!(err, LookupIdError::LookupStore { .. }), "{err}"); - assert_eq!(std::fs::read_to_string(&victim).unwrap(), "{}"); - } -} diff --git a/crates/icp-project/src/untrusted.rs b/crates/icp-project/src/untrusted.rs index fa9166a33..89af9c489 100644 --- a/crates/icp-project/src/untrusted.rs +++ b/crates/icp-project/src/untrusted.rs @@ -8,8 +8,7 @@ //! //! - what it reads goes through a filesystem confined to the project root — //! on a host, [`Confined`](crate::files::Confined) — so that every path a -//! manifest names is under the root and reached through no symlink. The -//! id and artifact stores under `.icp` are held to the same filesystem; +//! manifest names is under the root and reached through no symlink; //! - what it runs is on a list, stated as what is allowed so that a kind of //! step added later is refused until someone decides otherwise: a build //! step is a pre-built module, a sync step is a plugin, and the file either From 9de20822ca7f0a1a87d280b3672e3076f2d02a6b Mon Sep 17 00:00:00 2001 From: Raymond Khalife Date: Tue, 10 Nov 2026 17:50:53 +0000 Subject: [PATCH 4/8] feat(untrusted): refuse a recipe, rather than read it and vet what it renders to A recipe is build instructions read from a file or fetched from a URL while the project loads. Under --untrusted the loader is now given untrusted::NoRecipes, a Resolve that fetches nothing, so a project naming a recipe fails to load, wherever the recipe is from. RecipeFetcher goes back to what it is on main: it no longer reads a local template through the project's filesystem, since it is never asked for one for an untrusted project. Co-Authored-By: Claude --- CHANGELOG.md | 2 +- crates/icp-app/src/context/init.rs | 20 +++--- crates/icp-app/src/recipe.rs | 47 +------------ crates/icp-cli/src/options.rs | 2 +- crates/icp-cli/tests/untrusted_tests.rs | 89 ++++++++----------------- crates/icp-project/src/files.rs | 3 +- crates/icp-project/src/untrusted.rs | 68 +++++++++++++++++-- docs/reference/cli.md | 6 +- 8 files changed, 110 insertions(+), 127 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc4e56a07..9ded5bf07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ air-gapped signing, extensions # Unreleased -* feat: `--untrusted`, on `build`, `deploy` and `sync`, holds a project someone else handed over to what it may read and run. Every file the project reads — manifests, argument and environment files, local recipe templates, modules — must be under the project root and reached through no symlink. Each build step must be a pre-built module and each sync step a plugin, taken from a regular file in the project; the steps a recipe renders to are held to the same list. Anything else is refused before the command acts on it, naming what was found. +* feat: `--untrusted`, on `build`, `deploy` and `sync`, holds a project someone else handed over to what it may read and run. Every file the project reads — manifests, argument and environment files, modules — must be under the project root and reached through no symlink. Each build step must be a pre-built module and each sync step a plugin, taken from a regular file in the project. A recipe is refused, wherever it is from. Anything else is refused before the command acts on it, naming what was found. * chore: macOS on Intel (`x86_64-apple-darwin`) is no longer a release target. Releases ship no Intel macOS archive, the shell installer refuses that platform, and npm no longer publishes `@icp-sdk/icp-cli-darwin-x64`, so installing `@icp-sdk/icp-cli` on an Intel Mac warns of an unsupported platform. Homebrew dropped Intel macOS in September 2026, and GitHub is retiring its Intel macOS runners. v1.8.0 is the last release with an Intel macOS build. ## Experimental diff --git a/crates/icp-app/src/context/init.rs b/crates/icp-app/src/context/init.rs index 13b26ec22..50c1b2dfe 100644 --- a/crates/icp-app/src/context/init.rs +++ b/crates/icp-app/src/context/init.rs @@ -14,6 +14,7 @@ use std::time::Duration; use crate::{agent, identity, identity::PasswordFunc}; use icp_project::{ Lazy, ProjectLoad, ProjectLoadImpl, + canister::recipe::Resolve, files::{Confined, FileSystem, HostFileSystem, RootError}, host::Host, manifest::{self, ProjectRootLocate as _}, @@ -98,7 +99,7 @@ pub fn initialize( // against the root as spelled, so the root has to be a whole path. let project_root_override = project_root_override.map(|dir| cwd.join(dir)); let project_root_locate = Arc::new(manifest::ProjectRootLocateImpl::new( - cwd.clone(), + cwd, project_root_override, )); @@ -137,13 +138,16 @@ pub fn initialize( None => Arc::new(HostFileSystem), }; - // Recipes - let recipe = Arc::new(RecipeFetcher { - http_client, - pkg_cache, - files: files.clone(), - cwd, - }); + // Recipes: fetched for the user's own project, refused for an untrusted + // one, which states its steps itself. + let recipe: Arc = if untrusted { + Arc::new(untrusted::NoRecipes) + } else { + Arc::new(RecipeFetcher { + http_client, + pkg_cache, + }) + }; // Canister builder let builder = Arc::new(Builder::new(wasm.clone(), files.clone())); diff --git a/crates/icp-app/src/recipe.rs b/crates/icp-app/src/recipe.rs index 11d304721..37b278e91 100644 --- a/crates/icp-app/src/recipe.rs +++ b/crates/icp-app/src/recipe.rs @@ -14,7 +14,7 @@ use crate::package::{ read_cached_uri_recipe, }; use icp_project::{ - files::{FileSystem, FsError}, + fs::read, manifest::recipe::{Recipe, RecipeType}, prelude::*, }; @@ -30,11 +30,6 @@ pub struct RecipeFetcher { pub http_client: reqwest::Client, /// Package cache for caching downloaded recipe templates pub pkg_cache: std::sync::Arc, - /// The project's files, which a local template is read through - pub files: std::sync::Arc, - /// Where a relative local template path is taken from: the working - /// directory, whatever `files` would otherwise join it to - pub cwd: PathBuf, } /// The result of the fetch stage, as this resolver sees it. @@ -81,7 +76,7 @@ enum TemplateSource { #[derive(Debug, Snafu)] pub enum RecipeFetchError { #[snafu(display("failed to read local recipe template file"))] - ReadFile { source: FsError }, + ReadFile { source: icp_project::fs::IoError }, #[snafu(display("failed to decode UTF-8 string"))] DecodeUtf8 { source: FromUtf8Error }, @@ -138,8 +133,7 @@ impl RecipeFetcher { // Retrieve the template, using cache for remote/registry sources let (tmpl, should_cache) = match &tmpl_source { TemplateSource::LocalPath(path) => { - let path = self.cwd.join(path); - let bytes = self.files.read(&path).await.context(ReadFileSnafu)?; + let bytes = read(path).context(ReadFileSnafu)?; (parse_bytes_to_string(bytes)?, false) } @@ -366,8 +360,6 @@ mod tests { RecipeFetcher { http_client: reqwest::Client::new(), pkg_cache: std::sync::Arc::new(crate::package::open(cache_dir.to_owned()).unwrap()), - files: std::sync::Arc::new(icp_project::files::HostFileSystem), - cwd: PathBuf::new(), } } @@ -402,39 +394,6 @@ mod tests { ); } - /// A relative local path is taken from the working directory, the same - /// for a project read through a filesystem confined to its root as for - /// the user's own: not joined to that root instead. - #[tokio::test] - async fn a_relative_local_recipe_is_taken_from_the_working_directory() { - let tmp = camino_tempfile::Utf8TempDir::new().unwrap(); - let root = tmp.path().join("project"); - std::fs::create_dir(&root).unwrap(); - std::fs::write(root.join("recipe.hbs"), "inside").unwrap(); - std::fs::write(tmp.path().join("recipe.hbs"), "outside").unwrap(); - - let fetch = async |path: &str| { - let recipe = Recipe { - recipe_type: RecipeType::File(path.to_owned()), - configuration: Default::default(), - sha256: None, - }; - RecipeFetcher { - files: std::sync::Arc::new(icp_project::files::Confined::new(&root).unwrap()), - cwd: tmp.path().to_owned(), - ..fetcher(&tmp.path().join("pkg")) - } - .fetch_recipe(&recipe) - .await - }; - assert_eq!( - fetch("project/recipe.hbs").await.unwrap().template, - "inside" - ); - let err = icp_project::error::flatten(&fetch("recipe.hbs").await.err().unwrap()); - assert!(err.contains("is outside"), "{err}"); - } - /// A sha256 that does not match the template contents is rejected. #[tokio::test] async fn checksum_mismatch_is_rejected() { diff --git a/crates/icp-cli/src/options.rs b/crates/icp-cli/src/options.rs index 5a477a3d5..a648c1cb7 100644 --- a/crates/icp-cli/src/options.rs +++ b/crates/icp-cli/src/options.rs @@ -48,7 +48,7 @@ pub(crate) struct UntrustedOpt { /// Hold the project to what one someone else handed over may do: every /// file it reads is under the project root and reached through no /// symlink, each build step is a pre-built module and each sync step a - /// plugin. Anything else is refused + /// plugin, and it uses no recipe. Anything else is refused #[arg(long)] pub(crate) untrusted: bool, } diff --git a/crates/icp-cli/tests/untrusted_tests.rs b/crates/icp-cli/tests/untrusted_tests.rs index aa8bc7363..346af2255 100644 --- a/crates/icp-cli/tests/untrusted_tests.rs +++ b/crates/icp-cli/tests/untrusted_tests.rs @@ -3,7 +3,7 @@ //! would act on as the user's own, and shows the flag refusing it, naming //! what it refused. -use indoc::indoc; +use indoc::{formatdoc, indoc}; use predicates::{prelude::PredicateBooleanExt, str::contains}; use crate::common::TestContext; @@ -79,44 +79,34 @@ fn a_script_step_is_refused_as_untrusted_but_not_otherwise() { ); } -/// A recipe is read and rendered like any other; it is the steps it renders -/// to that are held to the list. +/// A recipe is build instructions read while the project loads, so one is +/// refused before it is read, wherever it is from and whatever it renders to. #[test] -fn a_recipe_is_rendered_and_its_steps_held_to_the_list() { +fn a_recipe_is_refused_as_untrusted() { let ctx = TestContext::new(); - let project_dir = project( - &ctx, - indoc! {r#" - canisters: - - name: backend - recipe: - type: file://recipe.hbs - "#}, - ); - let recipe = project_dir.join("recipe.hbs"); - - std::fs::write( - &recipe, - "build:\n steps:\n - type: pre-built\n path: backend.wasm\n", - ) - .expect("write recipe"); - ctx.icp() - .current_dir(&project_dir) - .args(["build", "--untrusted"]) - .assert() - .success(); + for recipe_type in ["file://recipe.hbs", "https://example.invalid/recipe.hbs"] { + let project_dir = project( + &ctx, + &formatdoc! {r#" + canisters: + - name: backend + recipe: + type: {recipe_type} + "#}, + ); + std::fs::write( + project_dir.join("recipe.hbs"), + "build:\n steps:\n - type: pre-built\n path: backend.wasm\n", + ) + .expect("write recipe"); - std::fs::write( - &recipe, - "build:\n steps:\n - type: script\n command: cp backend.wasm \"$ICP_WASM_OUTPUT_PATH\"\n", - ) - .expect("write recipe"); - ctx.icp() - .current_dir(&project_dir) - .args(["build", "--untrusted"]) - .assert() - .failure() - .stderr(contains("not a pre-built module").and(contains("(script) cp backend.wasm"))); + ctx.icp() + .current_dir(&project_dir) + .args(["build", "--untrusted"]) + .assert() + .failure() + .stderr(contains(recipe_type).and(contains("may not use a recipe"))); + } } /// What the project reads is held to its root: an argument file outside it @@ -153,33 +143,6 @@ fn a_file_read_from_outside_the_project_is_refused_as_untrusted() { .stderr(contains("outside.bin").and(contains("is outside"))); } -/// A local recipe template is read through the same filesystem. -#[test] -fn a_recipe_template_outside_the_project_is_refused_as_untrusted() { - let ctx = TestContext::new(); - let project_dir = project( - &ctx, - indoc! {r#" - canisters: - - name: backend - recipe: - type: file://../recipe.hbs - "#}, - ); - std::fs::write( - project_dir.parent().unwrap().join("recipe.hbs"), - "build:\n steps:\n - type: pre-built\n path: backend.wasm\n", - ) - .expect("write recipe"); - - ctx.icp() - .current_dir(&project_dir) - .args(["build", "--untrusted"]) - .assert() - .failure() - .stderr(contains("recipe.hbs").and(contains("is outside"))); -} - #[test] fn a_module_outside_the_project_is_refused_as_untrusted() { let ctx = TestContext::new(); diff --git a/crates/icp-project/src/files.rs b/crates/icp-project/src/files.rs index 8679677c0..594c55110 100644 --- a/crates/icp-project/src/files.rs +++ b/crates/icp-project/src/files.rs @@ -118,7 +118,8 @@ pub trait Scratch: Send + Sync { /// The loader reads every path through the [`FileSystem`] it is given, so /// this holds all of them. A local recipe template is the exception: the /// [`Resolve`](crate::canister::recipe::Resolve) implementation reads it, and -/// must read it through this filesystem or refuse it. +/// must read it through this filesystem or refuse it, as +/// [`NoRecipes`](crate::untrusted::NoRecipes) does. /// /// Known issue: a predicate can only answer `true` or `false`, so the reason /// a path was refused is lost there, and a caller that checks before it diff --git a/crates/icp-project/src/untrusted.rs b/crates/icp-project/src/untrusted.rs index 89af9c489..476d54048 100644 --- a/crates/icp-project/src/untrusted.rs +++ b/crates/icp-project/src/untrusted.rs @@ -4,11 +4,15 @@ //! build step may run any command, a sync step any script, and a manifest //! may name any path on the machine. A project from elsewhere — an archive //! an agent passed to a deploy service, a checkout the user has not read — -//! may not. Two things hold it: +//! may not. Three things hold it: //! //! - what it reads goes through a filesystem confined to the project root — //! on a host, [`Confined`](crate::files::Confined) — so that every path a //! manifest names is under the root and reached through no symlink; +//! - it names no recipe. A recipe is build instructions read from a file or +//! fetched from a URL while the project loads, before anyone can look at +//! what they say, so the loader is given [`NoRecipes`] to ask and the load +//! fails; //! - what it runs is on a list, stated as what is allowed so that a kind of //! step added later is refused until someone decides otherwise: a build //! step is a pre-built module, a sync step is a plugin, and the file either @@ -25,11 +29,23 @@ //! not the guarantee a read through the confined filesystem has: a tree //! rewritten between the look and the open is not caught. //! -//! A module or plugin named by URL is not on the list, and the steps a recipe -//! renders to are held to it like any other. A checksum is not asked for: the -//! same untrusted hand wrote it and the module. [`vet`] applies the list to a -//! loaded [`Project`], and [`Vetted`] wraps a loader so that every load is -//! vetted before the project is handed out. +//! A module or plugin named by URL is not on the list. A checksum is not +//! asked for: the same untrusted hand wrote it and the module. [`vet`] +//! applies the list to a loaded [`Project`], and [`Vetted`] wraps a loader so +//! that every load is vetted before the project is handed out. A consumer +//! composing its own loader holds the three together: +//! +//! ```ignore +//! let files = Arc::new(Confined::new(&root)?); +//! let project = Vetted::new( +//! ProjectLoadImpl { +//! project_root_locate, +//! recipe: Arc::new(NoRecipes), +//! files: files.clone(), +//! }, +//! files, +//! ); +//! ``` use std::sync::Arc; @@ -38,6 +54,7 @@ use snafu::{ResultExt, Snafu, ensure}; use crate::{ Project, ProjectLoad, ProjectLoadError, UntrustedSnafu, + canister::recipe::{Fetched, Resolve, ResolveError}, files::FileSystem, manifest::{ adapter::{ @@ -45,10 +62,35 @@ use crate::{ prebuilt::{LocalSource, SourceField}, }, canister::{BuildStep, SyncStep}, + recipe::Recipe, }, prelude::*, }; +/// The [`Resolve`] for an untrusted project: it fetches nothing, so a +/// project that names a recipe fails to load. +#[derive(Debug, Default, Clone, Copy)] +pub struct NoRecipes; + +/// A recipe an untrusted project named. +#[derive(Debug, Snafu)] +#[snafu(display( + "recipe '{recipe}' is refused: an untrusted project may not use a recipe, and states its \ + build and sync steps itself" +))] +pub struct RecipeRefused { + pub recipe: String, +} + +#[async_trait] +impl Resolve for NoRecipes { + async fn resolve(&self, recipe: &Recipe) -> Result { + Err(ResolveError::new(RecipeRefused { + recipe: recipe.recipe_type.to_string(), + })) + } +} + /// Why a loaded project may not be acted on as untrusted. #[derive(Debug, Snafu)] pub enum Refused { @@ -311,6 +353,20 @@ mod tests { )) } + #[tokio::test] + async fn no_recipe_is_fetched_wherever_it_is_from() { + for recipe_type in [ + "file://recipe.hbs", + "https://example.com/recipe.hbs", + "@dfinity/rust@v3.0.0", + ] { + let recipe: Recipe = serde_yaml::from_str(&format!("type: \"{recipe_type}\"")).unwrap(); + let err = NoRecipes.resolve(&recipe).await.err().unwrap().to_string(); + assert!(err.contains(recipe_type), "{err}"); + assert!(err.contains("may not use a recipe"), "{err}"); + } + } + #[tokio::test] async fn a_bundle_shaped_project_is_accepted() { vetted(vec![local("backend.wasm")], vec![plugin("path: p.wasm")]) diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 53f370922..d567bcd61 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -136,7 +136,7 @@ Build canisters ###### **Options:** * `-e`, `--environment ` — Override the environment to build for. By default, the local environment is used -* `--untrusted` — Hold the project to what one someone else handed over may do: every file it reads is under the project root and reached through no symlink, each build step is a pre-built module and each sync step a plugin. Anything else is refused +* `--untrusted` — Hold the project to what one someone else handed over may do: every file it reads is under the project root and reached through no symlink, each build step is a pre-built module and each sync step a plugin, and it uses no recipe. Anything else is refused @@ -1072,7 +1072,7 @@ using --args or --args-file: - `bin`: Raw binary (only valid for file references) -* `--untrusted` — Hold the project to what one someone else handed over may do: every file it reads is under the project root and reached through no symlink, each build step is a pre-built module and each sync step a plugin. Anything else is refused +* `--untrusted` — Hold the project to what one someone else handed over may do: every file it reads is under the project root and reached through no symlink, each build step is a pre-built module and each sync step a plugin, and it uses no recipe. Anything else is refused @@ -1921,7 +1921,7 @@ Synchronize canisters * `--proxy ` — Principal of a proxy canister to route sync plugin calls to the target canister through * `-e`, `--environment ` — Override the environment to connect to. By default, the local environment is used * `--identity ` — The user identity to run this command as -* `--untrusted` — Hold the project to what one someone else handed over may do: every file it reads is under the project root and reached through no symlink, each build step is a pre-built module and each sync step a plugin. Anything else is refused +* `--untrusted` — Hold the project to what one someone else handed over may do: every file it reads is under the project root and reached through no symlink, each build step is a pre-built module and each sync step a plugin, and it uses no recipe. Anything else is refused From 23c0f19848b4795fd3711e9923d4cd81cd7c6f0d Mon Sep 17 00:00:00 2001 From: Raymond Khalife Date: Tue, 10 Nov 2026 21:09:35 +0000 Subject: [PATCH 5/8] fix(untrusted): the project is where the command stands, and no root is searched for Finding a workspace root means reading the manifests above the working directory and following the dependencies they name, with the host's own filesystem, before there is a root to confine those reads to. It also let a project above that declares this one be adopted as the root, which widened what the untrusted project could read to the whole of that project. Under --untrusted the root is now the working directory, or the one --project-root-override names, and the locator is pinned to it. A directory with no icp.yaml is refused, saying so. Co-Authored-By: Claude --- CHANGELOG.md | 2 +- crates/icp-app/src/context/init.rs | 16 ++++++- crates/icp-cli/src/options.rs | 4 +- crates/icp-cli/tests/untrusted_tests.rs | 56 +++++++++++++++++++++++++ docs/reference/cli.md | 6 +-- 5 files changed, 78 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ded5bf07..c3228ce15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ air-gapped signing, extensions # Unreleased -* feat: `--untrusted`, on `build`, `deploy` and `sync`, holds a project someone else handed over to what it may read and run. Every file the project reads — manifests, argument and environment files, modules — must be under the project root and reached through no symlink. Each build step must be a pre-built module and each sync step a plugin, taken from a regular file in the project. A recipe is refused, wherever it is from. Anything else is refused before the command acts on it, naming what was found. +* feat: `--untrusted`, on `build`, `deploy` and `sync`, holds a project someone else handed over to what it may read and run. Every file the project reads — manifests, argument and environment files, modules — must be under the project root and reached through no symlink. Each build step must be a pre-built module and each sync step a plugin, taken from a regular file in the project. A recipe is refused, wherever it is from. The project is the directory the command is run from, or the one `--project-root-override` names; no root is searched for above it. Anything else is refused before the command acts on it, naming what was found. * chore: macOS on Intel (`x86_64-apple-darwin`) is no longer a release target. Releases ship no Intel macOS archive, the shell installer refuses that platform, and npm no longer publishes `@icp-sdk/icp-cli-darwin-x64`, so installing `@icp-sdk/icp-cli` on an Intel Mac warns of an unsupported platform. Homebrew dropped Intel macOS in September 2026, and GitHub is retiring its Intel macOS runners. v1.8.0 is the last release with an Intel macOS build. ## Experimental diff --git a/crates/icp-app/src/context/init.rs b/crates/icp-app/src/context/init.rs index 50c1b2dfe..9ac9b7f1a 100644 --- a/crates/icp-app/src/context/init.rs +++ b/crates/icp-app/src/context/init.rs @@ -44,7 +44,10 @@ pub enum ContextInitError { source: icp_project::fs::lock::LockError, }, - #[snafu(display("failed to locate the project to treat as untrusted"))] + #[snafu(display( + "an untrusted project is run from the directory that holds its icp.yaml, or named with \ + --project-root-override" + ))] UntrustedRoot { source: manifest::ProjectRootLocateError, }, @@ -98,6 +101,17 @@ pub fn initialize( // filesystem an untrusted project is confined to matches every path // against the root as spelled, so the root has to be a whole path. let project_root_override = project_root_override.map(|dir| cwd.join(dir)); + + // An untrusted project is the directory the command stands in, or the + // one it was pointed at, and nothing is searched for. Finding a root + // otherwise means reading manifests above and following the dependencies + // they name, before there is a root to hold those reads to, and it lets + // a project above adopt this one and widen what it may read. + let project_root_override = if untrusted { + Some(project_root_override.unwrap_or_else(|| cwd.clone())) + } else { + project_root_override + }; let project_root_locate = Arc::new(manifest::ProjectRootLocateImpl::new( cwd, project_root_override, diff --git a/crates/icp-cli/src/options.rs b/crates/icp-cli/src/options.rs index a648c1cb7..c47456aef 100644 --- a/crates/icp-cli/src/options.rs +++ b/crates/icp-cli/src/options.rs @@ -48,7 +48,9 @@ pub(crate) struct UntrustedOpt { /// Hold the project to what one someone else handed over may do: every /// file it reads is under the project root and reached through no /// symlink, each build step is a pre-built module and each sync step a - /// plugin, and it uses no recipe. Anything else is refused + /// plugin, and it uses no recipe. Anything else is refused. The project + /// is the directory the command is run from, or the one + /// `--project-root-override` names: no project root is searched for #[arg(long)] pub(crate) untrusted: bool, } diff --git a/crates/icp-cli/tests/untrusted_tests.rs b/crates/icp-cli/tests/untrusted_tests.rs index 346af2255..d697e6709 100644 --- a/crates/icp-cli/tests/untrusted_tests.rs +++ b/crates/icp-cli/tests/untrusted_tests.rs @@ -284,6 +284,62 @@ fn a_relative_project_root_is_taken_from_the_working_directory() { .success(); } +/// The project is where the command stands or where it is pointed, and no +/// root is searched for: not from a directory under the project, and not up +/// to a project above that declares this one, which would widen what the +/// untrusted project may read to the whole of the one above. +#[test] +fn the_root_is_where_the_command_stands_and_is_not_searched_for() { + let ctx = TestContext::new(); + let project_dir = project( + &ctx, + indoc! {r#" + canisters: + - name: backend + init_args: + path: ../outside.bin + format: bin + build: + steps: + - type: pre-built + path: backend.wasm + "#}, + ); + let parent = project_dir.parent().unwrap(); + write_string( + &parent.join("icp.yaml"), + indoc! {r#" + dependencies: + - name: inner + path: ./icp + "#}, + ) + .expect("write parent manifest"); + + // Trusted, the project above is the root, and the file is read. + ctx.icp() + .current_dir(&project_dir) + .args(["build"]) + .assert() + .success(); + + ctx.icp() + .current_dir(&project_dir) + .args(["build", "--untrusted"]) + .assert() + .failure() + .stderr(contains("outside.bin").and(contains("is outside"))); + + let src = project_dir.join("src"); + std::fs::create_dir(&src).expect("mkdir"); + ctx.icp() + .current_dir(&src) + .args(["build", "--untrusted"]) + .assert() + .failure() + .stderr(contains("run from the directory that holds its icp.yaml")); +} + /// The flag is on the three commands that act on what a project says, and /// each refuses before it reaches any network. It is on no other command. #[test] diff --git a/docs/reference/cli.md b/docs/reference/cli.md index d567bcd61..66cde462a 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -136,7 +136,7 @@ Build canisters ###### **Options:** * `-e`, `--environment ` — Override the environment to build for. By default, the local environment is used -* `--untrusted` — Hold the project to what one someone else handed over may do: every file it reads is under the project root and reached through no symlink, each build step is a pre-built module and each sync step a plugin, and it uses no recipe. Anything else is refused +* `--untrusted` — Hold the project to what one someone else handed over may do: every file it reads is under the project root and reached through no symlink, each build step is a pre-built module and each sync step a plugin, and it uses no recipe. Anything else is refused. The project is the directory the command is run from, or the one `--project-root-override` names: no project root is searched for @@ -1072,7 +1072,7 @@ using --args or --args-file: - `bin`: Raw binary (only valid for file references) -* `--untrusted` — Hold the project to what one someone else handed over may do: every file it reads is under the project root and reached through no symlink, each build step is a pre-built module and each sync step a plugin, and it uses no recipe. Anything else is refused +* `--untrusted` — Hold the project to what one someone else handed over may do: every file it reads is under the project root and reached through no symlink, each build step is a pre-built module and each sync step a plugin, and it uses no recipe. Anything else is refused. The project is the directory the command is run from, or the one `--project-root-override` names: no project root is searched for @@ -1921,7 +1921,7 @@ Synchronize canisters * `--proxy ` — Principal of a proxy canister to route sync plugin calls to the target canister through * `-e`, `--environment ` — Override the environment to connect to. By default, the local environment is used * `--identity ` — The user identity to run this command as -* `--untrusted` — Hold the project to what one someone else handed over may do: every file it reads is under the project root and reached through no symlink, each build step is a pre-built module and each sync step a plugin, and it uses no recipe. Anything else is refused +* `--untrusted` — Hold the project to what one someone else handed over may do: every file it reads is under the project root and reached through no symlink, each build step is a pre-built module and each sync step a plugin, and it uses no recipe. Anything else is refused. The project is the directory the command is run from, or the one `--project-root-override` names: no project root is searched for From 0556a71a478cd834f8d9a63cd4743299ce1a638e Mon Sep 17 00:00:00 2001 From: Raymond Khalife Date: Tue, 10 Nov 2026 21:11:35 +0000 Subject: [PATCH 6/8] docs(untrusted): say that --untrusted is run from the directory that holds icp.yaml It takes that directory as the project root and does not travel up to a project above. Said in the flag's help, the CLI reference, the project model and project dependencies pages, the changelog and the module doc. Co-Authored-By: Claude --- CHANGELOG.md | 2 +- crates/icp-cli/src/options.rs | 7 ++++--- crates/icp-project/src/untrusted.rs | 7 +++++++ docs/concepts/project-dependencies.md | 2 ++ docs/concepts/project-model.md | 2 ++ docs/reference/cli.md | 6 +++--- 6 files changed, 19 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c3228ce15..cf93a6e1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ air-gapped signing, extensions # Unreleased -* feat: `--untrusted`, on `build`, `deploy` and `sync`, holds a project someone else handed over to what it may read and run. Every file the project reads — manifests, argument and environment files, modules — must be under the project root and reached through no symlink. Each build step must be a pre-built module and each sync step a plugin, taken from a regular file in the project. A recipe is refused, wherever it is from. The project is the directory the command is run from, or the one `--project-root-override` names; no root is searched for above it. Anything else is refused before the command acts on it, naming what was found. +* feat: `--untrusted`, on `build`, `deploy` and `sync`, holds a project someone else handed over to what it may read and run. Every file the project reads — manifests, argument and environment files, modules — must be under the project root and reached through no symlink. Each build step must be a pre-built module and each sync step a plugin, taken from a regular file in the project. A recipe is refused, wherever it is from. It expects to be run from the directory that holds `icp.yaml`, which it takes as the project root: it does not look in the directories above, and running from a subdirectory fails. `--project-root-override` names another directory. Anything else is refused before the command acts on it, naming what was found. * chore: macOS on Intel (`x86_64-apple-darwin`) is no longer a release target. Releases ship no Intel macOS archive, the shell installer refuses that platform, and npm no longer publishes `@icp-sdk/icp-cli-darwin-x64`, so installing `@icp-sdk/icp-cli` on an Intel Mac warns of an unsupported platform. Homebrew dropped Intel macOS in September 2026, and GitHub is retiring its Intel macOS runners. v1.8.0 is the last release with an Intel macOS build. ## Experimental diff --git a/crates/icp-cli/src/options.rs b/crates/icp-cli/src/options.rs index c47456aef..18aad2558 100644 --- a/crates/icp-cli/src/options.rs +++ b/crates/icp-cli/src/options.rs @@ -48,9 +48,10 @@ pub(crate) struct UntrustedOpt { /// Hold the project to what one someone else handed over may do: every /// file it reads is under the project root and reached through no /// symlink, each build step is a pre-built module and each sync step a - /// plugin, and it uses no recipe. Anything else is refused. The project - /// is the directory the command is run from, or the one - /// `--project-root-override` names: no project root is searched for + /// plugin, and it uses no recipe. Anything else is refused. Run it from + /// the directory that holds `icp.yaml`: that directory is the project + /// root, and no root is looked for in the directories above it. Name + /// another directory with `--project-root-override` #[arg(long)] pub(crate) untrusted: bool, } diff --git a/crates/icp-project/src/untrusted.rs b/crates/icp-project/src/untrusted.rs index 476d54048..25fe7fc56 100644 --- a/crates/icp-project/src/untrusted.rs +++ b/crates/icp-project/src/untrusted.rs @@ -24,6 +24,13 @@ //! takes a project from elsewhere discards any `.icp` that came with it, and //! nothing here vets what one holds. //! +//! The root is the directory that holds the project's `icp.yaml`, named by +//! whoever asks, and it is not looked for. The CLI takes the directory it is +//! run from, or the one it is pointed at, and does not travel up to a +//! project above that declares this one: finding such a root means reading +//! manifests before there is a root to hold the reads to, and adopting it +//! would let the untrusted project read the whole of the one above. +//! //! A plugin's runtime opens the plugin, and each file and directory the step //! hands it, by path, after [`vet`] has looked. So those have the rule but //! not the guarantee a read through the confined filesystem has: a tree diff --git a/docs/concepts/project-dependencies.md b/docs/concepts/project-dependencies.md index 2472996df..759925fd8 100644 --- a/docs/concepts/project-dependencies.md +++ b/docs/concepts/project-dependencies.md @@ -100,6 +100,8 @@ Because a member-scoped deploy does not redeploy the member's dependencies, it f Force the workspace root with the `--project-root-override` flag or the `ICP_PROJECT_ROOT` environment variable. This uses the given directory as the root with no upward walk — for example, to deploy a vendored project truly on its own. +`--untrusted` never walks up either. It expects to be run from the directory that holds `icp.yaml` and takes that directory as the root, so a vendored project run with `--untrusted` is treated as standalone even when a project above declares it, and its dependencies must lie inside it. Use `--project-root-override` to name a different directory. + ## Environments across a workspace The workspace root owns the **network** and the **canister-ID store** for every environment; a dependency's own network definitions are ignored when it is deployed as part of a workspace. diff --git a/docs/concepts/project-model.md b/docs/concepts/project-model.md index c75755cf1..44787e5cf 100644 --- a/docs/concepts/project-model.md +++ b/docs/concepts/project-model.md @@ -257,6 +257,8 @@ icp-cli looks for `icp.yaml` in the current directory and parent directories. Yo icp deploy --project-root-override /path/to/project ``` +With `--untrusted` (on `build`, `deploy` and `sync`), nothing is searched for. Run the command from the directory that holds `icp.yaml`: that directory is the project root, and icp-cli does not look in parent directories. Running from a subdirectory fails. To act on a project somewhere else, name its directory with `--project-root-override`. + ## Next Steps - [Build, Deploy, Sync](build-deploy-sync.md) — The deployment lifecycle diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 66cde462a..50241601c 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -136,7 +136,7 @@ Build canisters ###### **Options:** * `-e`, `--environment ` — Override the environment to build for. By default, the local environment is used -* `--untrusted` — Hold the project to what one someone else handed over may do: every file it reads is under the project root and reached through no symlink, each build step is a pre-built module and each sync step a plugin, and it uses no recipe. Anything else is refused. The project is the directory the command is run from, or the one `--project-root-override` names: no project root is searched for +* `--untrusted` — Hold the project to what one someone else handed over may do: every file it reads is under the project root and reached through no symlink, each build step is a pre-built module and each sync step a plugin, and it uses no recipe. Anything else is refused. Run it from the directory that holds `icp.yaml`: that directory is the project root, and no root is looked for in the directories above it. Name another directory with `--project-root-override` @@ -1072,7 +1072,7 @@ using --args or --args-file: - `bin`: Raw binary (only valid for file references) -* `--untrusted` — Hold the project to what one someone else handed over may do: every file it reads is under the project root and reached through no symlink, each build step is a pre-built module and each sync step a plugin, and it uses no recipe. Anything else is refused. The project is the directory the command is run from, or the one `--project-root-override` names: no project root is searched for +* `--untrusted` — Hold the project to what one someone else handed over may do: every file it reads is under the project root and reached through no symlink, each build step is a pre-built module and each sync step a plugin, and it uses no recipe. Anything else is refused. Run it from the directory that holds `icp.yaml`: that directory is the project root, and no root is looked for in the directories above it. Name another directory with `--project-root-override` @@ -1921,7 +1921,7 @@ Synchronize canisters * `--proxy ` — Principal of a proxy canister to route sync plugin calls to the target canister through * `-e`, `--environment ` — Override the environment to connect to. By default, the local environment is used * `--identity ` — The user identity to run this command as -* `--untrusted` — Hold the project to what one someone else handed over may do: every file it reads is under the project root and reached through no symlink, each build step is a pre-built module and each sync step a plugin, and it uses no recipe. Anything else is refused. The project is the directory the command is run from, or the one `--project-root-override` names: no project root is searched for +* `--untrusted` — Hold the project to what one someone else handed over may do: every file it reads is under the project root and reached through no symlink, each build step is a pre-built module and each sync step a plugin, and it uses no recipe. Anything else is refused. Run it from the directory that holds `icp.yaml`: that directory is the project root, and no root is looked for in the directories above it. Name another directory with `--project-root-override` From 4b0656f6722e2ef78941d6b10ce9f71a389fd414 Mon Sep 17 00:00:00 2001 From: Raymond Khalife Date: Tue, 10 Nov 2026 23:49:13 +0000 Subject: [PATCH 7/8] docs(untrusted): say in the flag's help that .icp is not checked and is to be deleted Co-Authored-By: Claude --- CHANGELOG.md | 2 +- crates/icp-cli/src/options.rs | 4 +++- docs/reference/cli.md | 6 +++--- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cf93a6e1f..8129f6385 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ air-gapped signing, extensions # Unreleased -* feat: `--untrusted`, on `build`, `deploy` and `sync`, holds a project someone else handed over to what it may read and run. Every file the project reads — manifests, argument and environment files, modules — must be under the project root and reached through no symlink. Each build step must be a pre-built module and each sync step a plugin, taken from a regular file in the project. A recipe is refused, wherever it is from. It expects to be run from the directory that holds `icp.yaml`, which it takes as the project root: it does not look in the directories above, and running from a subdirectory fails. `--project-root-override` names another directory. Anything else is refused before the command acts on it, naming what was found. +* feat: `--untrusted`, on `build`, `deploy` and `sync`, holds a project someone else handed over to what it may read and run. Every file the project reads — manifests, argument and environment files, modules — must be under the project root and reached through no symlink. Each build step must be a pre-built module and each sync step a plugin, taken from a regular file in the project. A recipe is refused, wherever it is from. It expects to be run from the directory that holds `icp.yaml`, which it takes as the project root: it does not look in the directories above, and running from a subdirectory fails. `--project-root-override` names another directory. Anything else is refused before the command acts on it, naming what was found. The `.icp` directory is the tool's own state and is neither confined nor checked, so delete one that came with the project before running. * chore: macOS on Intel (`x86_64-apple-darwin`) is no longer a release target. Releases ship no Intel macOS archive, the shell installer refuses that platform, and npm no longer publishes `@icp-sdk/icp-cli-darwin-x64`, so installing `@icp-sdk/icp-cli` on an Intel Mac warns of an unsupported platform. Homebrew dropped Intel macOS in September 2026, and GitHub is retiring its Intel macOS runners. v1.8.0 is the last release with an Intel macOS build. ## Experimental diff --git a/crates/icp-cli/src/options.rs b/crates/icp-cli/src/options.rs index 18aad2558..7588ccfe4 100644 --- a/crates/icp-cli/src/options.rs +++ b/crates/icp-cli/src/options.rs @@ -51,7 +51,9 @@ pub(crate) struct UntrustedOpt { /// plugin, and it uses no recipe. Anything else is refused. Run it from /// the directory that holds `icp.yaml`: that directory is the project /// root, and no root is looked for in the directories above it. Name - /// another directory with `--project-root-override` + /// another directory with `--project-root-override`. The `.icp` + /// directory is the tool's own state and is neither confined nor + /// checked: delete one that came with the project before running #[arg(long)] pub(crate) untrusted: bool, } diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 50241601c..3de5021c7 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -136,7 +136,7 @@ Build canisters ###### **Options:** * `-e`, `--environment ` — Override the environment to build for. By default, the local environment is used -* `--untrusted` — Hold the project to what one someone else handed over may do: every file it reads is under the project root and reached through no symlink, each build step is a pre-built module and each sync step a plugin, and it uses no recipe. Anything else is refused. Run it from the directory that holds `icp.yaml`: that directory is the project root, and no root is looked for in the directories above it. Name another directory with `--project-root-override` +* `--untrusted` — Hold the project to what one someone else handed over may do: every file it reads is under the project root and reached through no symlink, each build step is a pre-built module and each sync step a plugin, and it uses no recipe. Anything else is refused. Run it from the directory that holds `icp.yaml`: that directory is the project root, and no root is looked for in the directories above it. Name another directory with `--project-root-override`. The `.icp` directory is the tool's own state and is neither confined nor checked: delete one that came with the project before running @@ -1072,7 +1072,7 @@ using --args or --args-file: - `bin`: Raw binary (only valid for file references) -* `--untrusted` — Hold the project to what one someone else handed over may do: every file it reads is under the project root and reached through no symlink, each build step is a pre-built module and each sync step a plugin, and it uses no recipe. Anything else is refused. Run it from the directory that holds `icp.yaml`: that directory is the project root, and no root is looked for in the directories above it. Name another directory with `--project-root-override` +* `--untrusted` — Hold the project to what one someone else handed over may do: every file it reads is under the project root and reached through no symlink, each build step is a pre-built module and each sync step a plugin, and it uses no recipe. Anything else is refused. Run it from the directory that holds `icp.yaml`: that directory is the project root, and no root is looked for in the directories above it. Name another directory with `--project-root-override`. The `.icp` directory is the tool's own state and is neither confined nor checked: delete one that came with the project before running @@ -1921,7 +1921,7 @@ Synchronize canisters * `--proxy ` — Principal of a proxy canister to route sync plugin calls to the target canister through * `-e`, `--environment ` — Override the environment to connect to. By default, the local environment is used * `--identity ` — The user identity to run this command as -* `--untrusted` — Hold the project to what one someone else handed over may do: every file it reads is under the project root and reached through no symlink, each build step is a pre-built module and each sync step a plugin, and it uses no recipe. Anything else is refused. Run it from the directory that holds `icp.yaml`: that directory is the project root, and no root is looked for in the directories above it. Name another directory with `--project-root-override` +* `--untrusted` — Hold the project to what one someone else handed over may do: every file it reads is under the project root and reached through no symlink, each build step is a pre-built module and each sync step a plugin, and it uses no recipe. Anything else is refused. Run it from the directory that holds `icp.yaml`: that directory is the project root, and no root is looked for in the directories above it. Name another directory with `--project-root-override`. The `.icp` directory is the tool's own state and is neither confined nor checked: delete one that came with the project before running From 9a929eadb6cd0afaf4a68b0fdb163c37129d4270 Mon Sep 17 00:00:00 2001 From: Raymond Khalife Date: Wed, 11 Nov 2026 19:11:14 +0000 Subject: [PATCH 8/8] docs(untrusted): say that vet checks no containment and takes it from the filesystem it is given vet asks the filesystem whether a path is a file or a directory and takes its word, so Vetted::new with HostFileSystem accepts a path outside the root or behind a symlink without an error. Say so on both. Co-Authored-By: Claude --- crates/icp-project/src/untrusted.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/crates/icp-project/src/untrusted.rs b/crates/icp-project/src/untrusted.rs index 25fe7fc56..43fd8c660 100644 --- a/crates/icp-project/src/untrusted.rs +++ b/crates/icp-project/src/untrusted.rs @@ -162,6 +162,14 @@ fn local_source(source: &SourceField) -> Option<&LocalSource> { /// The same is asked of what a plugin step hands its plugin, which the /// runtime also opens itself: a `dirs` entry is a directory, and a `files` /// entry a regular file or a directory. +/// +/// Containment is not checked here. `vet` asks `files` whether a path is a +/// file or a directory and takes its word, so `files` has to be a filesystem +/// that itself refuses a path outside the root or reached through a symlink: +/// on a host, [`Confined`](crate::files::Confined). Given one that does not — +/// [`HostFileSystem`](crate::files::HostFileSystem) — a module at +/// `../../elsewhere.wasm`, or one behind a symlink, passes, and nothing says +/// that anything is missing. pub async fn vet(project: &Project, files: &dyn FileSystem) -> Result<(), Refused> { for (_, (canister_dir, canister)) in &project.canisters { let build = canister.build.steps.iter().map(|step| { @@ -241,6 +249,12 @@ pub struct Vetted { impl Vetted { /// `files` is what the paths are resolved through: the filesystem the /// project is on. + /// + /// It must be the one the project is confined to — on a host, + /// [`Confined`](crate::files::Confined) — and the one `loader` reads + /// through. [`vet`] checks no containment of its own, so with + /// [`HostFileSystem`](crate::files::HostFileSystem) here a path outside + /// the root or behind a symlink is accepted without an error. pub fn new(loader: L, files: Arc) -> Self { Self { loader, files } }