From 6a463298213d63388704835601b6125fd3d5cded Mon Sep 17 00:00:00 2001 From: Brian H Date: Wed, 28 Aug 2024 14:17:43 -0600 Subject: [PATCH] A few fixes after yesterday's mergepalooza Signed-off-by: Brian H --- crates/compose/src/lib.rs | 6 +- crates/trigger/src/cli.rs | 7 +- crates/trigger/src/loader.rs | 293 ------------------ ...xx-spin-factors.md => 021-spin-factors.md} | 2 +- 4 files changed, 9 insertions(+), 299 deletions(-) delete mode 100644 crates/trigger/src/loader.rs rename docs/content/sips/{0xx-spin-factors.md => 021-spin-factors.md} (99%) diff --git a/crates/compose/src/lib.rs b/crates/compose/src/lib.rs index 81815a4f41..a9ee43fce7 100644 --- a/crates/compose/src/lib.rs +++ b/crates/compose/src/lib.rs @@ -26,7 +26,7 @@ use wac_graph::{CompositionGraph, NodeId}; /// dependent component. Finally, the composer will export all exports from the /// dependent component to its dependents. The composer will then encode the /// composition graph into a byte array and return it. -pub async fn compose<'a, L: LockedComponentSourceLoader>( +pub async fn compose<'a, L: ComponentSourceLoader>( loader: &'a L, component: &LockedComponent, ) -> Result, ComposeError> { @@ -35,7 +35,7 @@ pub async fn compose<'a, L: LockedComponentSourceLoader>( /// This trait is used to load component source code from a locked component source across various embdeddings. #[async_trait::async_trait] -pub trait LockedComponentSourceLoader { +pub trait ComponentSourceLoader { async fn load_component_source( &self, source: &locked::LockedComponentSource, @@ -97,7 +97,7 @@ struct Composer<'a, L> { loader: &'a L, } -impl<'a, L: LockedComponentSourceLoader> Composer<'a, L> { +impl<'a, L: ComponentSourceLoader> Composer<'a, L> { async fn compose(mut self, component: &LockedComponent) -> Result, ComposeError> { let source = self .loader diff --git a/crates/trigger/src/cli.rs b/crates/trigger/src/cli.rs index 55ce2a9035..e6267c6586 100644 --- a/crates/trigger/src/cli.rs +++ b/crates/trigger/src/cli.rs @@ -355,7 +355,7 @@ impl TriggerAppBuilder { struct SimpleComponentLoader; #[async_trait] - impl spin_compose::LockedComponentSourceLoader for SimpleComponentLoader { + impl spin_compose::ComponentSourceLoader for SimpleComponentLoader { async fn load_component_source( &self, source: &spin_app::locked::LockedComponentSource, @@ -399,7 +399,10 @@ impl TriggerAppBuilder { let composed = spin_compose::compose(self, component.locked) .await .with_context(|| { - format!("failed to compose component {:?}", component.locked.id) + format!( + "failed to resolve dependencies for component {:?}", + component.locked.id + ) })?; spin_core::Component::new(engine, composed) diff --git a/crates/trigger/src/loader.rs b/crates/trigger/src/loader.rs deleted file mode 100644 index b483c92c24..0000000000 --- a/crates/trigger/src/loader.rs +++ /dev/null @@ -1,293 +0,0 @@ -use std::path::{Path, PathBuf}; - -use anyhow::{ensure, Context, Result}; -use async_trait::async_trait; -use spin_app::{ - locked::{LockedApp, LockedComponent, LockedComponentSource}, - AppComponent, Loader, -}; -use spin_componentize::bugs::WasiLibc377Bug; -use spin_core::StoreBuilder; -use tokio::fs; - -use spin_common::{ui::quoted_path, url::parse_file_url}; - -/// Compilation status of all components of a Spin application -pub enum AotCompilationStatus { - /// No components are ahead of time (AOT) compiled. - Disabled, - #[cfg(feature = "unsafe-aot-compilation")] - /// All components are componentized and ahead of time (AOT) compiled to cwasm. - Enabled, -} - -/// Loader for the Spin runtime -pub struct TriggerLoader { - /// Working directory where files for mounting exist. - working_dir: PathBuf, - /// Set the static assets of the components in the temporary directory as writable. - allow_transient_write: bool, - /// Declares the compilation status of all components of a Spin application. - aot_compilation_status: AotCompilationStatus, -} - -impl TriggerLoader { - pub fn new(working_dir: impl Into, allow_transient_write: bool) -> Self { - Self { - working_dir: working_dir.into(), - allow_transient_write, - aot_compilation_status: AotCompilationStatus::Disabled, - } - } - - /// Updates the TriggerLoader to load AOT precompiled components - /// - /// **Warning: This feature may bypass important security guarantees of the - /// Wasmtime - // security sandbox if used incorrectly! Read this documentation - // carefully.** - /// - /// Usually, components are compiled just-in-time from portable Wasm - /// sources. This method causes components to instead be loaded - /// ahead-of-time as Wasmtime-precompiled native executable binaries. - /// Precompiled binaries must be produced with a compatible Wasmtime engine - /// using the same Wasmtime version and compiler target settings - typically - /// by a host with the same processor that will be executing them. See the - /// Wasmtime documentation for more information: - /// https://docs.rs/wasmtime/latest/wasmtime/struct.Module.html#method.deserialize - /// - /// # Safety - /// - /// This method is marked as `unsafe` because it enables potentially unsafe - /// behavior if - // used to load malformed or malicious precompiled binaries. Loading sources - // from an - /// incompatible Wasmtime engine will fail but is otherwise safe. This - /// method is safe if it can be guaranteed that `::load_component` will only ever be called with a trusted - /// `LockedComponentSource`. **Precompiled binaries must never be loaded - /// from untrusted sources.** - #[cfg(feature = "unsafe-aot-compilation")] - pub unsafe fn enable_loading_aot_compiled_components(&mut self) { - self.aot_compilation_status = AotCompilationStatus::Enabled; - } -} - -#[async_trait] -impl Loader for TriggerLoader { - async fn load_app(&self, url: &str) -> Result { - let path = parse_file_url(url)?; - let contents = std::fs::read(&path) - .with_context(|| format!("failed to read manifest at {}", quoted_path(&path)))?; - let app = - serde_json::from_slice(&contents).context("failed to parse app lock file JSON")?; - Ok(app) - } - - async fn load_component( - &self, - engine: &spin_core::wasmtime::Engine, - component: &LockedComponent, - ) -> Result { - let source = component - .source - .content - .source - .as_ref() - .context("LockedComponentSource missing source field")?; - let path = parse_file_url(source)?; - match self.aot_compilation_status { - #[cfg(feature = "unsafe-aot-compilation")] - AotCompilationStatus::Enabled => { - match engine.detect_precompiled_file(&path)?{ - Some(wasmtime::Precompiled::Component) => { - unsafe { - spin_core::Component::deserialize_file(engine, &path) - .with_context(|| format!("deserializing module {}", quoted_path(&path))) - } - }, - Some(wasmtime::Precompiled::Module) => anyhow::bail!("Spin loader is configured to load only AOT compiled components but an AOT compiled module provided at {}", quoted_path(&path)), - None => { - anyhow::bail!("Spin loader is configured to load only AOT compiled components, but {} is not precompiled", quoted_path(&path)) - } - } - } - AotCompilationStatus::Disabled => { - let component_loader = ComponentLoader; - let composed = spin_compose::compose(&component_loader, component) - .await - .with_context(|| format!("failed to resolve dependencies for component {:?}", component.id))?; - - spin_core::Component::new(engine, composed) - .with_context(|| format!("loading module {}", quoted_path(&path))) - } - } - } - - async fn load_module( - &self, - engine: &spin_core::wasmtime::Engine, - source: &LockedComponentSource, - ) -> Result { - let source = source - .content - .source - .as_ref() - .context("LockedComponentSource missing source field")?; - let path = parse_file_url(source)?; - check_uncomponentizable_module_deprecation(&path); - spin_core::Module::from_file(engine, &path) - .with_context(|| format!("loading module {}", quoted_path(&path))) - } - - async fn mount_files( - &self, - store_builder: &mut StoreBuilder, - component: &AppComponent, - ) -> Result<()> { - for content_dir in component.files() { - let source_uri = content_dir - .content - .source - .as_deref() - .with_context(|| format!("Missing 'source' on files mount {content_dir:?}"))?; - let source_path = self.working_dir.join(parse_file_url(source_uri)?); - ensure!( - source_path.is_dir(), - "TriggerLoader only supports directory mounts; {} is not a directory", - quoted_path(&source_path), - ); - let guest_path = content_dir.path.clone(); - if self.allow_transient_write { - store_builder.read_write_preopened_dir(source_path, guest_path)?; - } else { - store_builder.read_only_preopened_dir(source_path, guest_path)?; - } - } - Ok(()) - } -} - -// Check whether the given module is (likely) susceptible to a wasi-libc bug -// that makes it unsafe to componentize. If so, print a deprecation warning; -// this will turn into a hard error in a future release. -fn check_uncomponentizable_module_deprecation(module_path: &Path) { - let module = match std::fs::read(module_path) { - Ok(module) => module, - Err(err) => { - tracing::warn!("Failed to read {module_path:?}: {err:#}"); - return; - } - }; - match WasiLibc377Bug::detect(&module) { - Ok(WasiLibc377Bug::ProbablySafe) => {} - not_safe @ Ok(WasiLibc377Bug::ProbablyUnsafe | WasiLibc377Bug::Unknown) => { - println!( - "\n!!! DEPRECATION WARNING !!!\n\ - The Wasm module at {path}\n\ - {verbs} have been compiled with wasi-sdk version <19 and is likely to\n\ - contain a critical memory safety bug. Spin has deprecated execution of these\n\ - modules; they will stop working in a future release.\n\ - For more information, see: https://github.com/fermyon/spin/issues/2552\n", - path = module_path.display(), - verbs = if not_safe.unwrap() == WasiLibc377Bug::ProbablyUnsafe { - "appears to" - } else { - "may" - } - ); - } - Err(err) => { - tracing::warn!("Failed to apply wasi-libc bug heuristic on {module_path:?}: {err:#}"); - } - } -} - -struct ComponentLoader; - -#[async_trait] -impl spin_compose::ComponentLoader for ComponentLoader { - async fn load_component( - &self, - source: &spin_app::locked::LockedComponentSource, - ) -> anyhow::Result> { - let source = source - .content - .source - .as_ref() - .context("LockedComponentSource missing source field")?; - - let path = parse_file_url(source)?; - - let bytes: Vec = fs::read(&path).await.with_context(|| { - format!( - "failed to read component source from disk at path {}", - quoted_path(&path) - ) - })?; - - let component = spin_componentize::componentize_if_necessary(&bytes)?; - - Ok(component.into()) - } -} - -#[cfg(test)] -mod tests { - use super::*; - use spin_app::locked::ContentRef; - use std::io::Write; - use tempfile::NamedTempFile; - - fn precompiled_component(file: &mut NamedTempFile) -> LockedComponentSource { - let wasmtime_engine = wasmtime::Engine::default(); - let component = wasmtime::component::Component::new(&wasmtime_engine, "(component)") - .unwrap() - .serialize() - .unwrap(); - let file_uri = format!("file://{}", file.path().to_str().unwrap()); - file.write_all(&component).unwrap(); - LockedComponentSource { - content: ContentRef { - source: Some(file_uri), - ..Default::default() - }, - content_type: "application/wasm".to_string(), - } - } - - #[cfg(feature = "unsafe-aot-compilation")] - #[tokio::test] - async fn load_component_succeeds_for_precompiled_component() { - let mut file = NamedTempFile::new().unwrap(); - let source = precompiled_component(&mut file); - let mut loader = super::TriggerLoader::new("/unreferenced", false); - unsafe { - loader.enable_loading_aot_compiled_components(); - } - loader - .load_component(&spin_core::wasmtime::Engine::default(), &source) - .await - .unwrap(); - } - - #[tokio::test] - async fn load_component_fails_for_precompiled_component() { - let mut file = NamedTempFile::new().unwrap(); - let source = precompiled_component(&mut file); - let locked = spin_app::locked::LockedComponent { - id: "test".to_string(), - source, - metadata: Default::default(), - env: Default::default(), - files: Default::default(), - config: Default::default(), - dependencies: Default::default(), - }; - let loader = super::TriggerLoader::new("/unreferenced", false); - let result = loader - .load_component(&spin_core::wasmtime::Engine::default(), &locked) - .await; - assert!(result.is_err()); - } -} diff --git a/docs/content/sips/0xx-spin-factors.md b/docs/content/sips/021-spin-factors.md similarity index 99% rename from docs/content/sips/0xx-spin-factors.md rename to docs/content/sips/021-spin-factors.md index 9f06b74999..308428fc25 100644 --- a/docs/content/sips/0xx-spin-factors.md +++ b/docs/content/sips/021-spin-factors.md @@ -1,4 +1,4 @@ -title = "SIP 0XX - Spin Factors" +title = "SIP 021 - Spin Factors" template = "main" date = "2024-05-20T12:00:00Z"