feat(sync-plugin): make plugin compute-time limit configurable via env var

CHANGELOG.md

@@ -7,6 +7,7 @@ bump. Currently experimental: project bundling, project dependencies
 
 # Unreleased
 
+* feat(sync-plugin): the compute-time limit for `plugin` sync steps is now configurable via the `ICP_CLI_PLUGIN_COMPUTE_LIMIT_SECS` environment variable (default `60`). Raise it for compute-heavy plugins (e.g. brotli-compressing a large asset bundle) that legitimately exceed the default, especially on slower CI runners. The limit-exceeded error now names the variable and the current limit, and a malformed value is rejected rather than silently ignored.
 * feat: `icp canister link` assigns an existing canister principal to a project canister
 * feat: `icp canister create --with-icp` (not supported in `icp deploy`) uses the CMC to create canisters. Only needed for deploying to restricted system subnets.
 * feat: `icp deploy --no-create` will error if any canisters do not exist, rather than creating them.

crates/icp-cli/tests/sync_tests.rs

@@ -465,6 +465,63 @@ async fn sync_plugin_registers_seed_data() {
         );
 }
 
+/// A malformed `ICP_CLI_PLUGIN_COMPUTE_LIMIT_SECS` must abort the sync with an
+/// actionable error rather than being silently ignored. This also exercises the
+/// end-to-end wiring: it proves the override is actually read on the real plugin
+/// sync path (if it weren't, the bogus value would be ignored and the sync would
+/// proceed), which the unit tests can't cover on their own.
+#[tokio::test]
+async fn sync_plugin_rejects_invalid_compute_limit_env() {
+    let ctx = TestContext::new();
+    let project_dir = ctx.create_project_dir("icp");
+
+    let (canister_wasm, plugin_wasm) = build_sync_plugin_example();
+
+    let seed_data = project_dir.join("seed-data");
+    create_dir_all(&seed_data).expect("failed to create seed-data");
+    write_string(&seed_data.join("fruit-01.txt"), "apple").expect("failed to write fruit-01.txt");
+
+    let pm = formatdoc! {r#"
+        canisters:
+          - name: my-canister
+            build:
+              steps:
+                - type: script
+                  command: cp '{canister_wasm}' "$ICP_WASM_OUTPUT_PATH"
+            sync:
+              steps:
+                - type: plugin
+                  path: {plugin_wasm}
+                  dirs:
+                    - seed-data
+
+        {NETWORK_RANDOM_PORT}
+        {ENVIRONMENT_RANDOM_PORT}
+    "#};
+    write_string(&project_dir.join("icp.yaml"), &pm).expect("failed to write project manifest");
+
+    let _g = ctx.start_network_in(&project_dir, "random-network").await;
+    ctx.ping_until_healthy(&project_dir, "random-network");
+
+    clients::icp(&ctx, &project_dir, Some("random-environment".to_string()))
+        .mint_cycles(10 * TRILLION);
+
+    // deploy runs the sync step, which reads ICP_CLI_PLUGIN_COMPUTE_LIMIT_SECS.
+    // A non-integer value must abort with a message that names the variable and
+    // echoes the offending value.
+    ctx.icp()
+        .current_dir(&project_dir)
+        .env("NO_COLOR", "1")
+        .env("ICP_CLI_PLUGIN_COMPUTE_LIMIT_SECS", "not-a-number")
+        .args(["deploy", "--environment", "random-environment"])
+        .assert()
+        .failure()
+        .stderr(
+            contains("invalid ICP_CLI_PLUGIN_COMPUTE_LIMIT_SECS value")
+                .and(contains("not-a-number")),
+        );
+}
+
 /// A `dirs:` entry that is a symlink (here pointing outside the project) is
 /// rejected before the plugin runs, so a preopen cannot escape the canister
 /// directory. Symlinks are forbidden outright for now — see

crates/icp-sync-plugin/src/lib.rs

@@ -1,4 +1,6 @@
 mod path;
 mod runtime;
 
-pub use runtime::{RunPluginError, run_plugin};
+pub use runtime::{
+    DEFAULT_PLUGIN_COMPUTE_LIMIT_SECS, PLUGIN_COMPUTE_LIMIT_ENV, RunPluginError, run_plugin,
+};

crates/icp-sync-plugin/src/runtime.rs

@@ -9,8 +9,16 @@ use std::time::{Duration, Instant};
 const MAX_PLUGIN_OUTPUT: usize = 1024 * 1024; // 1 MiB per stream
 // Maximum wasm call-stack depth (in bytes).
 const MAX_WASM_STACK: usize = 512 * 1024;
-// How many seconds of pure wasm compute a plugin may use (host-call latency is excluded).
-const PLUGIN_COMPUTE_LIMIT_SECS: u64 = 60;
+/// Default seconds of pure wasm compute a plugin may use (host-call latency is
+/// excluded). This is a runaway guard, not a security boundary: the plugin runs
+/// locally in a read-only WASI sandbox, so the limit only protects the machine
+/// running `icp sync` from a plugin that never terminates. Legitimately heavy
+/// plugins (e.g. brotli-compressing a large asset bundle) can exceed it,
+/// especially on slower CI runners, so it is overridable via the
+/// [`PLUGIN_COMPUTE_LIMIT_ENV`] environment variable.
+pub const DEFAULT_PLUGIN_COMPUTE_LIMIT_SECS: u64 = 60;
+/// Environment variable that overrides [`DEFAULT_PLUGIN_COMPUTE_LIMIT_SECS`].
+pub const PLUGIN_COMPUTE_LIMIT_ENV: &str = "ICP_CLI_PLUGIN_COMPUTE_LIMIT_SECS";
 
 use bytes::Bytes;
 use camino::Utf8PathBuf;
@@ -124,8 +132,12 @@ impl SyncPluginImports for HostState {
 // must return wasmtime::Error (= anyhow::Error). Snafu derives std::error::Error
 // so .into() converts it via anyhow's blanket From<impl StdError + Send + Sync>.
 #[derive(Debug, Snafu)]
-#[snafu(display("plugin exceeded the {PLUGIN_COMPUTE_LIMIT_SECS}s compute time limit"))]
-struct ComputeTimeLimitExceeded;
+#[snafu(display(
+    "plugin exceeded the {limit_secs}s compute-time limit. If this plugin legitimately needs more compute time (e.g. brotli-compressing a large asset bundle), raise the limit by setting {PLUGIN_COMPUTE_LIMIT_ENV} above {limit_secs}s."
+))]
+struct ComputeTimeLimitExceeded {
+    limit_secs: u64,
+}
 
 #[derive(Debug, Snafu)]
 pub enum RunPluginError {
@@ -189,6 +201,7 @@ pub enum RunPluginError {
     PluginFailed { message: String },
 }
 
+#[allow(clippy::too_many_arguments)]
 pub fn run_plugin(
     wasm_path: Utf8PathBuf,
     base_dir: Utf8PathBuf,
@@ -199,6 +212,7 @@ pub fn run_plugin(
     proxy: Option<Principal>,
     identity_principal: Principal,
     environment: String,
+    compute_limit_secs: u64,
     stdio: Option<Sender<String>>,
 ) -> Result<Vec<String>, RunPluginError> {
     use wasmtime::component::{Component, Linker};
@@ -312,13 +326,16 @@ pub fn run_plugin(
     )?;
 
     let mut store = Store::new(&engine, host_state);
-    store.set_epoch_deadline(PLUGIN_COMPUTE_LIMIT_SECS);
+    store.set_epoch_deadline(compute_limit_secs);
     store.epoch_deadline_callback(move |_| {
         let extra = epoch_extension.swap(0, Ordering::Relaxed);
         if extra > 0 {
             Ok(wasmtime::UpdateDeadline::Continue(extra))
         } else {
-            Err(ComputeTimeLimitExceeded.into())
+            Err(ComputeTimeLimitExceeded {
+                limit_secs: compute_limit_secs,
+            }
+            .into())
         }
     });
 
@@ -538,11 +555,25 @@ mod tests {
             None,
             anon(),
             "test".to_string(),
+            DEFAULT_PLUGIN_COMPUTE_LIMIT_SECS,
             None,
         );
         assert!(matches!(result, Err(RunPluginError::LoadComponent { .. })));
     }
 
+    #[test]
+    fn compute_time_limit_error_reflects_the_configured_limit() {
+        // The remediation must anchor to the actual limit (not a hardcoded
+        // literal), so it reads correctly whether the limit is the default or
+        // an env-var override. Use a distinctive value to catch a regression.
+        let msg = ComputeTimeLimitExceeded { limit_secs: 120 }.to_string();
+        assert!(msg.contains("exceeded the 120s"), "got: {msg}");
+        // The suggestion tells the user to go above the current limit — the
+        // value must flow into the remediation clause too.
+        assert!(msg.contains("above 120s"), "got: {msg}");
+        assert!(msg.contains(PLUGIN_COMPUTE_LIMIT_ENV), "got: {msg}");
+    }
+
     // -------------------------------------------------------------------------
     // Fixture-dependent tests
     // -------------------------------------------------------------------------
@@ -562,6 +593,7 @@ mod tests {
             None,
             anon(),
             "test".to_string(),
+            DEFAULT_PLUGIN_COMPUTE_LIMIT_SECS,
             None,
         );
         assert!(matches!(result, Err(RunPluginError::PreopenDir { .. })));
@@ -589,6 +621,7 @@ mod tests {
             None,
             anon(),
             "test".to_string(),
+            DEFAULT_PLUGIN_COMPUTE_LIMIT_SECS,
             None,
         );
         assert!(matches!(result, Err(RunPluginError::SymlinkDir { .. })));
@@ -609,6 +642,7 @@ mod tests {
             None,
             anon(),
             "test".to_string(),
+            DEFAULT_PLUGIN_COMPUTE_LIMIT_SECS,
             None,
         );
         assert!(matches!(result, Err(RunPluginError::ReadFile { .. })));
@@ -636,6 +670,7 @@ mod tests {
             None,
             anon(),
             "test".to_string(),
+            DEFAULT_PLUGIN_COMPUTE_LIMIT_SECS,
             None,
         );
         assert!(matches!(result, Err(RunPluginError::SymlinkFile { .. })));
@@ -656,6 +691,7 @@ mod tests {
             None,
             anon(),
             "ok".to_string(),
+            DEFAULT_PLUGIN_COMPUTE_LIMIT_SECS,
             None,
         );
         assert!(result.is_ok());
@@ -676,6 +712,7 @@ mod tests {
             None,
             anon(),
             "error".to_string(),
+            DEFAULT_PLUGIN_COMPUTE_LIMIT_SECS,
             None,
         );
         assert!(matches!(
@@ -684,6 +721,41 @@ mod tests {
         ));
     }
 
+    #[test]
+    fn plugin_exceeding_compute_limit_is_trapped() {
+        let Some(wasm_path) = option_env!("TEST_PLUGIN_WASM") else {
+            return;
+        };
+        // The "spin" fixture busy-loops forever; a 1-second limit keeps the
+        // test fast while still exercising the epoch-interruption trap.
+        let result = run_plugin(
+            wasm_path.into(),
+            ".".into(),
+            vec![],
+            vec![],
+            anon(),
+            dummy_agent(),
+            None,
+            anon(),
+            "spin".to_string(),
+            1,
+            None,
+        );
+        let err = result.expect_err("spinning plugin should hit the compute limit");
+        // The trap surfaces through the CallExec source chain, so walk it and
+        // assert the message names both the limit and the override env var.
+        let mut chain = err.to_string();
+        let mut cur: &dyn std::error::Error = &err;
+        while let Some(src) = cur.source() {
+            chain = format!("{chain}: {src}");
+            cur = src;
+        }
+        assert!(
+            chain.contains("compute-time limit") && chain.contains(PLUGIN_COMPUTE_LIMIT_ENV),
+            "unexpected error chain: {chain}"
+        );
+    }
+
     #[tokio::test(flavor = "multi_thread")]
     async fn plugin_stdout_forwarded_through_stdio_channel() {
         let Some(wasm_path) = option_env!("TEST_PLUGIN_WASM") else {
@@ -701,6 +773,7 @@ mod tests {
                 None,
                 anon(),
                 "print".to_string(),
+                DEFAULT_PLUGIN_COMPUTE_LIMIT_SECS,
                 Some(tx),
             )
         });
@@ -726,6 +799,7 @@ mod tests {
                 None,
                 anon(),
                 "hello".to_string(),
+                DEFAULT_PLUGIN_COMPUTE_LIMIT_SECS,
                 Some(tx),
             )
         });

crates/icp-sync-plugin/tests/fixtures/test-plugin/src/lib.rs

@@ -19,6 +19,17 @@ impl Guest for TestPlugin {
                 println!("stdout from plugin");
                 Ok(())
             }
+            "spin" => {
+                // Busy-loop forever to exercise the host's compute-time limit.
+                // The epoch-interruption check at the loop back-edge traps this,
+                // so it never returns. `black_box` keeps the loop from being
+                // optimized away.
+                let mut x: u64 = 0;
+                loop {
+                    x = x.wrapping_add(1);
+                    std::hint::black_box(x);
+                }
+            }
             _ => Ok(()),
         }
     }

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

@@ -1,7 +1,9 @@
 use camino::Utf8PathBuf;
 use candid::Principal;
 use ic_agent::Agent;
-use icp_sync_plugin::{RunPluginError, run_plugin};
+use icp_sync_plugin::{
+    DEFAULT_PLUGIN_COMPUTE_LIMIT_SECS, PLUGIN_COMPUTE_LIMIT_ENV, RunPluginError, run_plugin,
+};
 use snafu::prelude::*;
 use tokio::sync::mpsc::Sender;
 
@@ -17,10 +19,36 @@ pub enum PluginError {
     #[snafu(display("failed to get identity principal: {err}"))]
     GetIdentityPrincipal { err: String },
 
+    #[snafu(display(
+        "invalid {PLUGIN_COMPUTE_LIMIT_ENV} value '{value}': expected a positive integer number of seconds"
+    ))]
+    InvalidComputeLimit { value: String },
+
     #[snafu(display("failed to run plugin"))]
     Run { source: RunPluginError },
 }
 
+/// Resolve the plugin compute-time limit, honoring the
+/// [`PLUGIN_COMPUTE_LIMIT_ENV`] override. Fails loudly on a malformed value so
+/// a typo doesn't silently fall back to the default and leave the caller
+/// wondering why their raised limit had no effect.
+fn resolve_compute_limit_secs() -> Result<u64, PluginError> {
+    match std::env::var(PLUGIN_COMPUTE_LIMIT_ENV) {
+        Err(_) => Ok(DEFAULT_PLUGIN_COMPUTE_LIMIT_SECS),
+        Ok(value) => parse_compute_limit(&value),
+    }
+}
+
+fn parse_compute_limit(value: &str) -> Result<u64, PluginError> {
+    match value.trim().parse::<u64>() {
+        Ok(secs) if secs >= 1 => Ok(secs),
+        _ => InvalidComputeLimitSnafu {
+            value: value.to_owned(),
+        }
+        .fail(),
+    }
+}
+
 pub(super) async fn sync(
     adapter: &Adapter,
     params: &Params,
@@ -30,6 +58,11 @@ pub(super) async fn sync(
     stdio: Option<Sender<String>>,
     pkg_cache: &PackageCache,
 ) -> Result<Vec<String>, PluginError> {
+    // 0. Resolve the compute-time limit up front so a malformed
+    //    ICP_CLI_PLUGIN_COMPUTE_LIMIT_SECS fails fast — before downloading the
+    //    wasm or touching the network — rather than after doing that work.
+    let compute_limit_secs = resolve_compute_limit_secs()?;
+
     // 1. Determine the on-disk path for the wasm. run_plugin needs a path, not raw bytes.
     //    - Local: sha256 is verified if present, then the original path is returned.
     //    - Remote: downloaded to cache (sha256 required, enforced at parse time) and the
@@ -71,8 +104,33 @@ pub(super) async fn sync(
             proxy,
             identity_principal,
             environment_owned,
+            compute_limit_secs,
             stdio_clone,
         )
     })
     .context(RunSnafu)
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn parse_compute_limit_accepts_positive_integers() {
+        assert_eq!(parse_compute_limit("300").unwrap(), 300);
+        // Surrounding whitespace is tolerated.
+        assert_eq!(parse_compute_limit("  42 ").unwrap(), 42);
+    }
+
+    #[test]
+    fn parse_compute_limit_rejects_invalid_values() {
+        for bad in ["0", "abc", "30O", "-5", "1.5", ""] {
+            let err =
+                parse_compute_limit(bad).expect_err(&format!("expected '{bad}' to be rejected"));
+            assert!(
+                matches!(err, PluginError::InvalidComputeLimit { .. }),
+                "unexpected error for '{bad}': {err}"
+            );
+        }
+    }
+}