From af8318bbd618789ec87b9ffb2572bc0893055e02 Mon Sep 17 00:00:00 2001 From: Adam Spofford Date: Thu, 25 Jun 2026 03:46:41 -0700 Subject: [PATCH 1/3] Fix bundle build ordering --- crates/icp-cli/src/operations/bundle.rs | 9 +++- crates/icp-cli/tests/bundle_tests.rs | 65 +++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/crates/icp-cli/src/operations/bundle.rs b/crates/icp-cli/src/operations/bundle.rs index 386f6d13..0e3156ef 100644 --- a/crates/icp-cli/src/operations/bundle.rs +++ b/crates/icp-cli/src/operations/bundle.rs @@ -238,9 +238,11 @@ pub(crate) async fn create_bundle( ) -> Result<(), BundleError> { validate_canisters(&canisters)?; let canonical_project_dir = canonicalize(project_dir)?; - let canonical_sync_dirs = validate_source_paths(&canisters, &canonical_project_dir)?; - validate_output_path(output, &canonical_sync_dirs)?; + // Build before validating source paths: a canister's synced directories + // (e.g. a frontend `dist`) are commonly produced by its own build step, so + // they may not exist on disk until the build has run. Canonicalizing them + // first would fail with "No such file or directory" for those build outputs. build_many_with_progress_bar( canisters.clone(), builder, @@ -250,6 +252,9 @@ pub(crate) async fn create_bundle( ) .await?; + let canonical_sync_dirs = validate_source_paths(&canisters, &canonical_project_dir)?; + validate_output_path(output, &canonical_sync_dirs)?; + // Re-read the raw manifest to preserve networks and environments verbatim. let raw_manifest: ProjectManifest = load_manifest_from_path(&project_dir.join(PROJECT_MANIFEST)) diff --git a/crates/icp-cli/tests/bundle_tests.rs b/crates/icp-cli/tests/bundle_tests.rs index d693edbd..cb114f83 100644 --- a/crates/icp-cli/tests/bundle_tests.rs +++ b/crates/icp-cli/tests/bundle_tests.rs @@ -523,6 +523,71 @@ fn bundle_rejects_source_outside_project() { .stderr(contains("my-canister").and(contains("outside the project directory"))); } +/// A plugin's synced directory is often an output of the canister's own build +/// step (e.g. a frontend `dist` produced by `npm run build`), so it does not +/// exist when bundling starts. Bundling must run the build before it validates +/// and archives the sync sources — otherwise canonicalizing the not-yet-built +/// directory fails with "No such file or directory". +#[test] +fn bundle_builds_synced_dir_before_validating() { + let ctx = TestContext::new(); + let project_dir = ctx.create_project_dir("icp"); + let wasm_src = ctx.make_asset("example_icp_mo.wasm"); + + write(&project_dir.join("plugin.wasm"), b"\x00asm\x01\x00\x00\x00") + .expect("failed to write plugin wasm"); + + // `generated-assets` is created by the build step; it does not exist on disk + // before `icp project bundle` runs. + let pm = formatdoc! {r#" + canisters: + - name: my-canister + build: + steps: + - type: script + commands: + - mkdir -p generated-assets + - printf hello > generated-assets/index.html + - cp '{wasm_src}' "$ICP_WASM_OUTPUT_PATH" + sync: + steps: + - type: plugin + path: plugin.wasm + dirs: + - generated-assets + "#}; + + write_string(&project_dir.join("icp.yaml"), &pm).expect("failed to write project manifest"); + + let bundle_path = project_dir.join("bundle.tar.gz"); + ctx.icp() + .current_dir(&project_dir) + .args(["project", "bundle", "--output", bundle_path.as_str()]) + .assert() + .success(); + + // The build-produced directory is archived under the plugin's `dirs/` area. + let bundle_bytes = fs::read(bundle_path.as_std_path()).expect("failed to read bundle"); + let gz = GzDecoder::new(BufReader::new(bundle_bytes.as_slice())); + let mut archive = Archive::new(gz); + let mut found_asset = false; + for entry in archive.entries().expect("failed to read archive entries") { + let entry = entry.expect("failed to read archive entry"); + let path = entry + .path() + .expect("failed to get entry path") + .to_string_lossy() + .into_owned(); + if path == "plugins/my-canister/0/dirs/generated-assets/index.html" { + found_asset = true; + } + } + assert!( + found_asset, + "build-produced synced dir was not archived in the bundle" + ); +} + /// Bundle a canister with two plugin sync steps and verify the archive layout: per-plugin /// wasm at `plugins/{canister}/{idx}.wasm`, preopened dirs at `plugins/{canister}/{idx}/dirs/`, /// input files at `plugins/{canister}/{idx}/files/`. Also verify the rewritten manifest From 978be2bc320cb719540594a0baa2fbc735c54cca Mon Sep 17 00:00:00 2001 From: Adam Spofford Date: Thu, 25 Jun 2026 03:57:50 -0700 Subject: [PATCH 2/3] Revert ordering and instead avoid hard canonicalization --- crates/icp-cli/src/operations/bundle.rs | 71 +++++++++++++++++++++---- crates/icp-cli/tests/bundle_tests.rs | 8 +-- 2 files changed, 65 insertions(+), 14 deletions(-) diff --git a/crates/icp-cli/src/operations/bundle.rs b/crates/icp-cli/src/operations/bundle.rs index 0e3156ef..f834a956 100644 --- a/crates/icp-cli/src/operations/bundle.rs +++ b/crates/icp-cli/src/operations/bundle.rs @@ -238,11 +238,10 @@ pub(crate) async fn create_bundle( ) -> Result<(), BundleError> { validate_canisters(&canisters)?; let canonical_project_dir = canonicalize(project_dir)?; + let canonical_sync_dirs = + validate_source_paths(project_dir, &canisters, &canonical_project_dir)?; + validate_output_path(output, &canonical_sync_dirs)?; - // Build before validating source paths: a canister's synced directories - // (e.g. a frontend `dist`) are commonly produced by its own build step, so - // they may not exist on disk until the build has run. Canonicalizing them - // first would fail with "No such file or directory" for those build outputs. build_many_with_progress_bar( canisters.clone(), builder, @@ -252,9 +251,6 @@ pub(crate) async fn create_bundle( ) .await?; - let canonical_sync_dirs = validate_source_paths(&canisters, &canonical_project_dir)?; - validate_output_path(output, &canonical_sync_dirs)?; - // Re-read the raw manifest to preserve networks and environments verbatim. let raw_manifest: ProjectManifest = load_manifest_from_path(&project_dir.join(PROJECT_MANIFEST)) @@ -778,6 +774,7 @@ fn validate_canisters(canisters: &[(PathBuf, Canister)]) -> Result<(), BundleErr /// project directory. Returns the canonical sync-directory paths for use in output-overlap /// detection. fn validate_source_paths( + project_dir: &Path, canisters: &[(PathBuf, Canister)], canonical_project_dir: &Path, ) -> Result, BundleError> { @@ -790,19 +787,21 @@ fn validate_source_paths( if let Some(dirs) = &adapter.dirs { for d in dirs { let src = canister_path.join(d); - let canon = canonicalize_within_project( + let resolved = resolve_within_project( &src, + project_dir, canonical_project_dir, &canister.name, )?; - canonical_sync_dirs.push(canon); + canonical_sync_dirs.push(resolved); } } if let Some(files) = &adapter.files { for f in files { let src = canister_path.join(f); - canonicalize_within_project( + resolve_within_project( &src, + project_dir, canonical_project_dir, &canister.name, )?; @@ -815,6 +814,58 @@ fn validate_source_paths( Ok(canonical_sync_dirs) } +/// Resolves a sync source path to its absolute location under the canonical +/// project directory, rejecting paths that escape the project — without touching +/// the filesystem. +/// +/// Unlike [`canonicalize_within_project`], this performs no syscalls and does not +/// require the path to exist. Sync directories are frequently produced by a +/// canister's own build step (e.g. a frontend `dist` from `npm run build`), so +/// they do not exist yet when this validation runs, before the build. Validating +/// lexically keeps that feedback early instead of deferring it until after a +/// potentially slow build. +/// +/// Every canister path is rooted at `project_dir`, so the path's project-relative +/// portion is recovered with `strip_prefix` and its `.`/`..` components resolved +/// textually; a `..` that rises above the root — or an absolute/sibling path that +/// `strip_prefix` rejects — is an escape. Symlinks are deliberately not resolved: +/// the archive step records them verbatim (`follow_symlinks(false)`), and a +/// not-yet-built directory cannot be a symlink anyway. +fn resolve_within_project( + src: &Path, + project_dir: &Path, + canonical_project_dir: &Path, + canister: &str, +) -> Result { + let escapes = || { + SourceEscapesProjectSnafu { + canister: canister.to_owned(), + path: src.to_path_buf(), + root: canonical_project_dir.to_path_buf(), + } + .build() + }; + + let rel = src.strip_prefix(project_dir).map_err(|_| escapes())?; + + let mut components: Vec<&str> = Vec::new(); + for component in rel.components() { + match component { + Utf8Component::Normal(c) => components.push(c), + Utf8Component::CurDir => {} + // A `..` with nothing left to pop rises above the project root. + Utf8Component::ParentDir => { + components.pop().ok_or_else(escapes)?; + } + Utf8Component::RootDir | Utf8Component::Prefix(_) => return Err(escapes()), + } + } + + let mut resolved = canonical_project_dir.to_path_buf(); + resolved.extend(components); + Ok(resolved) +} + /// Refuse to write the bundle output into a directory we are about to recursively archive — /// otherwise the partial bundle file would be included in itself. fn validate_output_path(output: &Path, canonical_sync_dirs: &[PathBuf]) -> Result<(), BundleError> { diff --git a/crates/icp-cli/tests/bundle_tests.rs b/crates/icp-cli/tests/bundle_tests.rs index cb114f83..8a23d75b 100644 --- a/crates/icp-cli/tests/bundle_tests.rs +++ b/crates/icp-cli/tests/bundle_tests.rs @@ -525,11 +525,11 @@ fn bundle_rejects_source_outside_project() { /// A plugin's synced directory is often an output of the canister's own build /// step (e.g. a frontend `dist` produced by `npm run build`), so it does not -/// exist when bundling starts. Bundling must run the build before it validates -/// and archives the sync sources — otherwise canonicalizing the not-yet-built -/// directory fails with "No such file or directory". +/// exist when bundling validates the sync sources, before the build. Validation +/// must resolve sync paths lexically (no canonicalization) so a not-yet-built +/// directory is accepted; the build then creates it before it is archived. #[test] -fn bundle_builds_synced_dir_before_validating() { +fn bundle_accepts_synced_dir_created_by_build_step() { let ctx = TestContext::new(); let project_dir = ctx.create_project_dir("icp"); let wasm_src = ctx.make_asset("example_icp_mo.wasm"); From 16733de633a144d385afd560e295fb598e0a9975 Mon Sep 17 00:00:00 2001 From: Adam Spofford Date: Thu, 25 Jun 2026 10:25:30 -0700 Subject: [PATCH 3/3] fix relative path prefix issue --- crates/icp-cli/src/operations/bundle.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/icp-cli/src/operations/bundle.rs b/crates/icp-cli/src/operations/bundle.rs index f834a956..c725ec8d 100644 --- a/crates/icp-cli/src/operations/bundle.rs +++ b/crates/icp-cli/src/operations/bundle.rs @@ -770,7 +770,7 @@ fn validate_canisters(canisters: &[(PathBuf, Canister)]) -> Result<(), BundleErr Ok(()) } -/// Canonicalize every asset/plugin source path and confirm it lives inside the canonical +/// Make every asset/plugin source path absolute and confirm it lives inside the /// project directory. Returns the canonical sync-directory paths for use in output-overlap /// detection. fn validate_source_paths( @@ -846,6 +846,7 @@ fn resolve_within_project( .build() }; + let project_dir = project_dir.strip_prefix(".").unwrap_or(project_dir); let rel = src.strip_prefix(project_dir).map_err(|_| escapes())?; let mut components: Vec<&str> = Vec::new();