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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions crates/icp-cli/src/operations/bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,7 @@ async fn prepare_canister(
canister_path,
&path_name,
idx,
local_names,
pkg_cache,
out,
)
Expand Down Expand Up @@ -710,6 +711,24 @@ fn localize_controllers<EnvVar>(
settings
}

/// Rewrite a plugin's declared call targets from workspace store keys back to the
/// local names of the instance being written, on the same grounds as
/// [`localize_controllers`].
fn localize_call_targets(
canisters: Option<&[String]>,
local_names: &HashMap<&str, &str>,
) -> Option<Vec<String>> {
canisters.map(|canisters| {
canisters
.iter()
.map(|target| match local_names.get(target.as_str()) {
Some(local) => (*local).to_owned(),
None => target.clone(),
})
.collect()
})
}

#[allow(clippy::too_many_arguments)]
async fn prepare_plugin_step(
adapter: &plugin::Adapter,
Expand All @@ -718,6 +737,7 @@ async fn prepare_plugin_step(
canister_path: &Path,
path_name: &str,
idx: usize,
local_names: &HashMap<&str, &str>,
pkg_cache: &PackageCache,
out: &mut BundleArtifacts,
) -> Result<SyncStep, BundleError> {
Expand Down Expand Up @@ -788,6 +808,7 @@ async fn prepare_plugin_step(
sha256: Some(plugin_sha256),
dirs: bundle_dirs,
files: bundle_files,
canisters: localize_call_targets(adapter.canisters.as_deref(), local_names),
}))
}

Expand Down
128 changes: 128 additions & 0 deletions crates/icp-cli/tests/bundle_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1088,6 +1088,134 @@ fn bundle_packages_plugin_sync_steps() {
);
}

/// A plugin's declared call targets must survive bundling — dropping them would turn a
/// working project into a bundle whose cross-canister calls are all rejected. Names of
/// the writing instance's own canisters come back out as local names; principals and
/// names that already resolved against the workspace are left alone.
#[test]
fn bundle_preserves_plugin_call_targets() {
let ctx = TestContext::new();
let project_dir = ctx.create_project_dir("icp");
let wasm_src = ctx.make_asset("example_icp_mo.wasm");

let build_step = formatdoc! {r#"
build:
steps:
- type: script
command: cp '{wasm_src}' "$ICP_WASM_OUTPUT_PATH"
"#};

// Bundling only repackages the plugin wasm bytes, so any non-empty content works.
write(&project_dir.join("plugin.wasm"), b"\x00asm\x01\x00\x00\x00")
.expect("failed to write plugin wasm");

let dep_dir = project_dir.join("vendor/openemail");
create_dir_all(&dep_dir).expect("failed to create dependency dir");
write(&dep_dir.join("plugin.wasm"), b"\x00asm\x01\x00\x00\x00")
.expect("failed to write dependency plugin wasm");

// The dependency's plugin names its own sibling, both bare and by store key.
write_string(
&dep_dir.join("icp.yaml"),
&formatdoc! {r#"
canisters:
- name: backend
{build_step}
sync:
steps:
- type: plugin
path: plugin.wasm
canisters:
- helper
- vendor/openemail:helper
- name: helper
{build_step}
"#},
)
.expect("failed to write dependency manifest");

// The root's plugin names a root sibling and a dependency canister by store key.
write_string(
&project_dir.join("icp.yaml"),
&formatdoc! {r#"
canisters:
- name: frontend
{build_step}
sync:
steps:
- type: plugin
path: plugin.wasm
canisters:
- api
- vendor/openemail:backend
- name: api
{build_step}

dependencies:
- name: openemail
path: ./vendor/openemail
canisters: [backend]
"#},
)
.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();

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 manifests: std::collections::HashMap<String, String> = std::collections::HashMap::new();
for entry in archive.entries().expect("failed to read archive entries") {
let mut 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.ends_with("icp.yaml") {
let mut yaml = String::new();
entry
.read_to_string(&mut yaml)
.expect("failed to read manifest");
manifests.insert(path, yaml);
}
}

let plugin_targets = |yaml: &str, canister: &str| -> Vec<String> {
let parsed: serde_yaml::Value =
serde_yaml::from_str(yaml).expect("manifest yaml is invalid");
let canisters = parsed["canisters"]
.as_sequence()
.expect("manifest has no canisters");
let entry = canisters
.iter()
.find(|c| c["name"].as_str() == Some(canister))
.unwrap_or_else(|| panic!("{canister} not found in bundled manifest: {yaml}"));
entry["sync"]["steps"][0]["canisters"]
.as_sequence()
.unwrap_or_else(|| panic!("{canister} plugin step lost its canisters: {yaml}"))
.iter()
.map(|t| t.as_str().expect("call target is not a string").to_owned())
.collect()
};

assert_eq!(
plugin_targets(&manifests["icp.yaml"], "frontend"),
["api", "vendor/openemail:backend"],
);
// Both spellings of the dependency's own sibling come out as its local name.
assert_eq!(
plugin_targets(&manifests["vendor/openemail/icp.yaml"], "backend"),
["helper", "helper"],
);
}

/// An `icp_appmanifest.yaml` next to the project manifest must be included in the bundle, with its
/// top-level `images` paths relocated under a top-level `images/` folder and the
/// referenced image files copied alongside. Unrelated metadata is preserved.
Expand Down
55 changes: 35 additions & 20 deletions crates/icp-sync-plugin/DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,19 @@ docs; the *reasons* behind those choices are recorded here.
- **Raw Candid bytes at the boundary** — `canister-call-request.arg` is
`list<u8>`. The plugin owns Candid encoding/decoding; the host forwards bytes
unchanged. This keeps the host free of any per-canister type knowledge.
- **`canister-call` takes no canister ID** — the host always calls the canister
from `sync-exec-input.canister-id`. There is deliberately no field for a
different target, so the single-canister restriction is *structural* rather
than a policy the plugin could bypass.
- **`canister-call` takes an explicit `target`** — the plugin selects the
canister being synced (`host`) or a canister from the step's `canisters:`
list, by name. The host resolves the target and *enforces* the list: a target
absent from it is rejected without a call. Names are the only way to address
another canister: the name→principal mapping is the host's to make, since it
varies per environment, and a plugin that hardcodes a principal is pinned to
one deployment. (In the earlier `@0.1.0` interface
`canister-call` had no target and always reached the canister being synced; see
*Interface versioning* below.)
- **`sync-exec-input` carries the canister ID table** — `canister-ids` exposes
the project's name→principal map for the environment, so a plugin can resolve
canister names it knows about. It is informational only: `canister-call`
still targets the canister being synced, so the table grants no ability to
call other canisters.
canister names it knows about. It is informational only; calling still
requires an entry in `canisters:`.
- **Filesystem access via WASI, not a host import** — plugins use standard
language APIs (`std::fs`); the host preopens the declared `dirs` read-only. No
bespoke `read-file`/`list-dir` import is needed.
Expand Down Expand Up @@ -71,9 +75,12 @@ pub fn run_plugin(invocation: PluginInvocation) -> Result<Vec<String>, RunPlugin
```

`PluginInvocation` bundles the inputs: `wasm_path`, `base_dir`, `dirs`, `files`,
`target_canister_id` (the canister being synced), `agent`, `proxy`,
`identity_principal`, `environment`, `compute_limit_secs`, and the exposed
`canister_ids` table, plus `stdio`.
`host_canister_id` (the canister being synced), `agent`, `proxy`,
`identity_principal`, `environment`, `compute_limit_secs`, the exposed
`canister_ids` table, the `callable: CallableCanisters` enforcement set, and
`stdio`. The CLI resolves the manifest's declared `canisters:` into
`CallableCanisters` before calling; this crate stays free of any manifest
knowledge.

`dirs` and `files` are the manifest-relative path strings, straight from the
adapter. The runtime owns *all* filesystem access anchored at `base_dir`: it
Expand Down Expand Up @@ -112,7 +119,8 @@ mod v2 { wasmtime::component::bindgen!({ world: "sync-plugin", path: "sync-plugi
mod v1 { wasmtime::component::bindgen!({ world: "sync-plugin", path: "sync-plugin-v1.wit" }); }

struct HostState {
target_canister_id: Principal,
host_canister_id: Principal,
callable: CallableCanisters, // name → principal, from the manifest
agent: Arc<Agent>,
proxy: Option<Principal>,
wasi_ctx: wasmtime_wasi::WasiCtx,
Expand All @@ -121,16 +129,18 @@ struct HostState {
}

// Implemented for both v1::SyncPluginImports and v2::SyncPluginImports; both
// delegate to one shared `do_canister_call(...)`.
// delegate to one shared `do_canister_call(target, ...)`.
```

`HostState` implements `WasiView` so wasmtime_wasi can access the WASI context.
`canister_call` uses `tokio::runtime::Handle::current().block_on(...)` because
the caller already wraps the synchronous `run_plugin` in
`tokio::task::block_in_place`. Both interface versions call the canister being
synced. When a proxy is configured and the call is a non-`direct` update, it is
encoded as `ProxyArgs` and routed through the proxy's `proxy` method; otherwise
it goes straight to the target via `ic-agent`.
`tokio::task::block_in_place`. For a v0.2.0 plugin the target is resolved from
the request's `call-target` by `resolve_call_target`, which enforces the
`callable` set; for a v0.1.0 plugin the target is always `host_canister_id`.
When a proxy is configured and the call is a non-`direct` update, it is encoded
as `ProxyArgs` and routed through the proxy's `proxy` method; otherwise it goes
straight to the resolved target via `ic-agent`.

### Interface versioning (parallel v0.1.0 / v0.2.0 support)

Expand Down Expand Up @@ -174,21 +184,26 @@ Deserializes the `canister.yaml` fields into:

```rust
pub struct Adapter {
pub source: SourceField, // path: or url:
pub source: SourceField, // path: or url:
pub sha256: Option<String>,
pub dirs: Option<Vec<String>>,
pub files: Option<Vec<String>>,
pub canisters: Option<Vec<String>>, // extra callable canisters, by name
}
```

`Deserialize` is hand-written to reject a `url` source without a `sha256`.
Each `canisters:` entry is a canister name resolved against the project's ID
table for the environment being synced. `Deserialize` is hand-written to reject a
`url` source without a `sha256`.

### `crates/icp/src/canister/sync/plugin.rs`

Resolves the wasm (local read or remote HTTP fetch into the package cache),
verifies sha256, builds the exposed canister ID table, then calls
verifies sha256, builds the exposed canister ID table and the `CallableCanisters`
enforcement set (resolving `canisters:` against the project's IDs), then calls
`icp_sync_plugin::run_plugin(...)` with a `PluginInvocation`. The runtime — not
the CLI — opens the declared paths and enforces the path-safety checks, so the
CLI no longer touches the plugin's input files itself. `exposed_canister_ids`
adds a bare-local-name duplicate for every canister in the same subproject as
the one being synced.
the one being synced; `resolve_callable` fails the step if a name in
`canisters:` does not resolve.
4 changes: 2 additions & 2 deletions crates/icp-sync-plugin/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ mod path;
mod runtime;

pub use runtime::{
DEFAULT_PLUGIN_COMPUTE_LIMIT_SECS, PLUGIN_COMPUTE_LIMIT_ENV, PluginInvocation, RunPluginError,
run_plugin,
CallableCanisters, DEFAULT_PLUGIN_COMPUTE_LIMIT_SECS, PLUGIN_COMPUTE_LIMIT_ENV,
PluginInvocation, RunPluginError, run_plugin,
};
Loading
Loading