fix(profile): address post-recovery review gaps

Author: schickling

crates/agent-spec/src/profile.rs

@@ -81,7 +81,11 @@ pub enum ProfileSource {
     /// A wasm module implementing the `resolve` protocol (see [`crate::profile_wasm`] for the
     /// ABI), plus the notification class every carrier it resolves carries. Compilation results
     /// (success or failure) are cached by path and file identity; instances remain per-resolution.
-    Wasm { module: PathBuf, class: ProfileClass },
+    Wasm {
+        module: PathBuf,
+        class: ProfileClass,
+        containment_root: Option<PathBuf>,
+    },
 }
 
 impl ResourceProfile {
@@ -96,6 +100,26 @@ impl ResourceProfile {
             source: ProfileSource::Wasm {
                 module: module.into(),
                 class,
+                containment_root: None,
+            },
+        }
+    }
+
+    /// Wasm-module profile whose module must be opened beneath one trusted directory without
+    /// following symlinks in any relative path component.
+    pub fn wasm_contained(
+        scheme: impl Into<String>,
+        containment_root: impl Into<PathBuf>,
+        relative_module: impl AsRef<Path>,
+        class: ProfileClass,
+    ) -> Self {
+        let containment_root = containment_root.into();
+        Self {
+            scheme: scheme.into(),
+            source: ProfileSource::Wasm {
+                module: containment_root.join(relative_module),
+                class,
+                containment_root: Some(containment_root),
             },
         }
     }
@@ -228,11 +252,15 @@ impl ResourceProfileRefresh<'_> {
         let Some(profile) = self.registry.profiles.get(scheme) else {
             return Ok(None);
         };
-        let ProfileSource::Wasm { module, class } = &profile.source;
+        let ProfileSource::Wasm {
+            module,
+            class,
+            containment_root,
+        } = &profile.source;
 
         #[cfg(not(feature = "wasm-resolver"))]
         {
-            let _ = (module, class, agent_dir);
+            let _ = (module, class, containment_root, agent_dir);
             Err("profile resolver unavailable: st2 was built without the `wasm-resolver` feature"
                 .to_owned())
         }
@@ -243,7 +271,7 @@ impl ResourceProfileRefresh<'_> {
                 if let Some(result) = modules.get(module) {
                     result.clone()
                 } else {
-                    let result = self.registry.compiled(module);
+                    let result = self.registry.compiled(module, containment_root.as_deref());
                     modules.insert(module.clone(), result.clone());
                     result
                 }
@@ -420,13 +448,15 @@ impl ResourceProfileRegistry {
     fn compiled(
         &self,
         module_path: &Path,
+        containment_root: Option<&Path>,
     ) -> Result<Arc<crate::profile_wasm::WasmResolver>, String> {
         #[cfg(test)]
         {
             self.wasm_cache.lock().snapshot_attempts += 1;
         }
         let snapshot = crate::profile_wasm::read_module_snapshot(
             module_path,
+            containment_root,
             DEFAULT_MODULE_LIMIT_BYTES,
         )
         .map_err(|error| error.to_string())?;

crates/agent-spec/src/profile_wasm.rs

@@ -29,6 +29,8 @@ pub const DEFAULT_MEMORY_LIMIT_BYTES: usize = 64 * 1024 * 1024;
 pub const DEFAULT_TABLE_ELEMENT_LIMIT: usize = 10_000;
 /// Maximum resolver module bytes admitted before Wasmtime validation and compilation.
 pub use crate::profile::DEFAULT_MODULE_LIMIT_BYTES;
+/// Maximum JSON payload accepted from one resolver call before UTF-8 or Serde decoding.
+pub const DEFAULT_OUTPUT_LIMIT_BYTES: usize = 64 * 1024;
 
 /// One resolution result as produced by a wasm resolver module.
 #[derive(Debug, Clone, PartialEq, Eq, Deserialize)]
@@ -101,7 +103,7 @@ impl Clone for WasmResolver {
 impl WasmResolver {
     /// Compile a `.wasm` module from disk with default budgets.
     pub fn load(path: &std::path::Path) -> Result<Self, WasmResolveError> {
-        let snapshot = read_module_snapshot(path, DEFAULT_MODULE_LIMIT_BYTES)?;
+        let snapshot = read_module_snapshot(path, None, DEFAULT_MODULE_LIMIT_BYTES)?;
         Self::from_bytes(&snapshot.bytes)
     }
 
@@ -204,11 +206,12 @@ pub(crate) struct ModuleSnapshot {
 
 pub(crate) fn read_module_snapshot(
     path: &std::path::Path,
+    containment_root: Option<&std::path::Path>,
     limit: usize,
 ) -> Result<ModuleSnapshot, WasmResolveError> {
     use std::io::Read as _;
 
-    let file = open_module_file(path)?;
+    let file = open_module_file(path, containment_root)?;
     let declared_len = file
         .metadata()
         .map_err(|error| WasmResolveError::Instantiation(error.to_string()))?
@@ -235,31 +238,82 @@ pub(crate) fn read_module_snapshot(
 }
 
 #[cfg(unix)]
-fn open_module_file(path: &std::path::Path) -> Result<std::fs::File, WasmResolveError> {
+fn open_module_file(
+    path: &std::path::Path,
+    containment_root: Option<&std::path::Path>,
+) -> Result<std::fs::File, WasmResolveError> {
+    use std::ffi::CString;
+    use std::os::fd::{AsRawFd as _, FromRawFd as _};
+    use std::os::unix::ffi::OsStrExt as _;
     use std::os::unix::fs::OpenOptionsExt as _;
-
-    let file = std::fs::OpenOptions::new()
-        .read(true)
-        .custom_flags(libc::O_CLOEXEC | libc::O_NOFOLLOW | libc::O_NONBLOCK)
-        .open(path)
-        .map_err(|error| WasmResolveError::Instantiation(error.to_string()))?;
-    if !file
-        .metadata()
-        .map_err(|error| WasmResolveError::Instantiation(error.to_string()))?
-        .file_type()
-        .is_file()
-    {
+    use std::path::Component;
+
+    let Some(root) = containment_root else {
+        return validate_module_file(
+            std::fs::OpenOptions::new()
+                .read(true)
+                .custom_flags(libc::O_CLOEXEC | libc::O_NOFOLLOW | libc::O_NONBLOCK)
+                .open(path)
+                .map_err(|error| WasmResolveError::Instantiation(error.to_string()))?,
+        );
+    };
+    let relative = path.strip_prefix(root).map_err(|_| {
+        WasmResolveError::Instantiation(format!(
+            "resolver module {} escapes containment root {}",
+            path.display(),
+            root.display()
+        ))
+    })?;
+    let mut components = relative.components().peekable();
+    if components.peek().is_none() {
         return Err(WasmResolveError::Instantiation(
-            "resolver module is not a regular file".to_owned(),
+            "resolver module path is empty".to_owned(),
         ));
     }
-    Ok(file)
+    let mut directory = std::fs::OpenOptions::new()
+        .read(true)
+        .custom_flags(libc::O_CLOEXEC | libc::O_NOFOLLOW | libc::O_DIRECTORY)
+        .open(root)
+        .map_err(|error| WasmResolveError::Instantiation(error.to_string()))?;
+    while let Some(component) = components.next() {
+        let Component::Normal(name) = component else {
+            return Err(WasmResolveError::Instantiation(
+                "resolver module path contains an unsafe component".to_owned(),
+            ));
+        };
+        let name = CString::new(name.as_bytes()).map_err(|_| {
+            WasmResolveError::Instantiation(
+                "resolver module path contains an interior NUL".to_owned(),
+            )
+        })?;
+        let is_last = components.peek().is_none();
+        let flags = libc::O_RDONLY
+            | libc::O_CLOEXEC
+            | libc::O_NOFOLLOW
+            | if is_last {
+                libc::O_NONBLOCK
+            } else {
+                libc::O_DIRECTORY
+            };
+        // SAFETY: `directory` is a live descriptor and `name` is a NUL-terminated single path
+        // component. The returned descriptor is immediately owned by `File`.
+        let fd = unsafe { libc::openat(directory.as_raw_fd(), name.as_ptr(), flags) };
+        if fd < 0 {
+            return Err(WasmResolveError::Instantiation(
+                std::io::Error::last_os_error().to_string(),
+            ));
+        }
+        // SAFETY: `openat` returned a fresh owned descriptor.
+        let opened = unsafe { std::fs::File::from_raw_fd(fd) };
+        if is_last {
+            return validate_module_file(opened);
+        }
+        directory = opened;
+    }
+    unreachable!("a non-empty component iterator returns its final file")
 }
 
-#[cfg(not(unix))]
-fn open_module_file(path: &std::path::Path) -> Result<std::fs::File, WasmResolveError> {
-    let file = std::fs::File::open(path)
-        .map_err(|error| WasmResolveError::Instantiation(error.to_string()))?;
+fn validate_module_file(file: std::fs::File) -> Result<std::fs::File, WasmResolveError> {
     if !file
         .metadata()
         .map_err(|error| WasmResolveError::Instantiation(error.to_string()))?
@@ -273,6 +327,17 @@ fn open_module_file(path: &std::path::Path) -> Result<std::fs::File, WasmResolve
     Ok(file)
 }
 
+#[cfg(not(unix))]
+fn open_module_file(
+    path: &std::path::Path,
+    _containment_root: Option<&std::path::Path>,
+) -> Result<std::fs::File, WasmResolveError> {
+    validate_module_file(
+        std::fs::File::open(path)
+            .map_err(|error| WasmResolveError::Instantiation(error.to_string()))?,
+    )
+}
+
 fn reject_symlink_components(
     agent_dir: &std::path::Path,
     path: &std::path::Path,
@@ -420,6 +485,11 @@ impl WasmInstance {
 
         let ret_ptr = (packed >> 32) as u32 as usize;
         let ret_len = (packed as u32) as usize;
+        if ret_len > DEFAULT_OUTPUT_LIMIT_BYTES {
+            return Err(WasmResolveError::BadReturn(format!(
+                "return payload is {ret_len} bytes; limit is {DEFAULT_OUTPUT_LIMIT_BYTES} bytes"
+            )));
+        }
         let bytes = self.read_guest_bytes(ret_ptr, ret_len)?;
         let text = std::str::from_utf8(bytes)
             .map_err(|e| WasmResolveError::BadReturn(format!("return payload is not UTF-8: {e}")))?;

crates/agent-spec/tests/profile_wasm.rs

@@ -10,7 +10,8 @@
 
 use agent_spec::profile::{ProfileClass, ResourceProfile, ResourceProfileRegistry};
 use agent_spec::profile_wasm::{
-    DEFAULT_MODULE_LIMIT_BYTES, DEFAULT_TABLE_ELEMENT_LIMIT, WasmResolveError, WasmResolver,
+    DEFAULT_MODULE_LIMIT_BYTES, DEFAULT_OUTPUT_LIMIT_BYTES, DEFAULT_TABLE_ELEMENT_LIMIT,
+    WasmResolveError, WasmResolver,
 };
 use std::path::{Path, PathBuf};
 use wasmtime::Trap;
@@ -293,6 +294,26 @@ fn garbage_return_payload_is_reported_not_crashed_on() {
     }
 }
 
+#[test]
+fn oversized_valid_return_range_is_rejected_before_decoding() {
+    let oversized = WasmResolver::from_wat(
+        r#"(module
+  (memory (export "memory") 2)
+  (func (export "alloc") (param i32) (result i32) (i32.const 1024))
+  (func (export "resolve") (param i32 i32 i32 i32) (result i64)
+    (i64.extend_i32_u (i32.const 65537)))
+)"#,
+    )
+    .expect("oversized-return module compiles");
+    match oversized.resolve_once("dev.schickling.agent-goal://x", "/a") {
+        Err(WasmResolveError::BadReturn(error)) => {
+            assert!(error.contains("limit"), "got: {error}");
+            assert!(error.contains(&(DEFAULT_OUTPUT_LIMIT_BYTES + 1).to_string()));
+        }
+        other => panic!("expected output-limit rejection, got {other:?}"),
+    }
+}
+
 #[test]
 fn wild_return_pointer_is_caught_before_memory_access() {
     let wild = WasmResolver::from_wat(&format!(
@@ -389,6 +410,34 @@ fn oversized_module_is_rejected_before_wasmtime_compilation() {
     }
 }
 
+#[cfg(unix)]
+#[test]
+fn catalog_relative_module_rejects_symlinked_path_ancestors() {
+    let catalog = tempfile::tempdir().expect("catalog directory");
+    let outside = tempfile::tempdir().expect("outside directory");
+    std::fs::copy(DEMO_WASM_PATH, outside.path().join("demo.wasm"))
+        .expect("outside module is copied");
+    std::os::unix::fs::symlink(outside.path(), catalog.path().join("resolvers"))
+        .expect("resolver ancestor symlink is created");
+    let registry = ResourceProfileRegistry::empty().with_profile(
+        ResourceProfile::wasm_contained(
+            "dev.schickling.agent-goal",
+            catalog.path(),
+            "resolvers/demo.wasm",
+            ProfileClass::Immediate,
+        ),
+    );
+
+    let error = registry
+        .try_resolve(
+            catalog.path(),
+            "dev.schickling.agent-goal://host/worker",
+        )
+        .expect_err("a symlinked module ancestor must be rejected");
+    assert!(!error.is_empty());
+}
+
+
 #[cfg(unix)]
 #[test]
 fn special_file_module_is_rejected_without_blocking() {

docs/vrs/07-resource-profile/spec.md

@@ -66,9 +66,13 @@ There is one source variant:
 ProfileSource::Wasm {
     module: PathBuf,
     class: ProfileClass,
+    containment_root: Option<PathBuf>,
 }
 ```
 
+`containment_root` is the trusted descriptor-traversal root for catalog-relative
+modules and is absent for explicitly external absolute modules.
+
 There is no template or exec variant. `ResourceProfileRegistry::builtin()` is
 empty. `with_profile` and `with_profiles` inject catalog-owned registrations;
 a later programmatic insertion for the same exact scheme replaces the prior
@@ -219,6 +223,8 @@ metadata alone is never treated as a durable proof.
 
 Each module is opened nonblocking and no-follow, accepted only as a regular
 file, and read through a 16 MiB admission cap before validation or compilation.
+Catalog-relative modules are traversed descriptor-relative from the catalog
+root with `O_NOFOLLOW` on every ancestor and the final component.
 The bounded 32-entry LRU cache stores both successful modules and compilation
 failures by module path plus byte digest and stable file metadata. Registry
 clones and concurrent subscribers therefore coalesce one compilation attempt
@@ -231,10 +237,11 @@ instance receives one fresh allowance before each later call:
 
 | Boundary | Contract |
 | --- | --- |
-| Module file | regular, no-follow, nonblocking open; 16 MiB maximum before Wasmtime compilation |
+| Module file | regular, nonblocking; catalog-relative paths use descriptor-relative no-follow traversal for every component; 16 MiB maximum before Wasmtime compilation |
 | Imports | none; import-requiring modules fail instantiation |
 | Fuel | 5,000,000 fuel units for start + first call; same budget per later call |
 | Linear memory | 64 MiB maximum |
+| Resolver return | memory range must be valid and at most 64 KiB before UTF-8/JSON decoding |
 | Memories | at most 1 |
 | Tables | at most 4, with at most 10,000 elements each |
 | Instance state | fresh per registry resolution |
@@ -248,7 +255,7 @@ Failure taxonomy:
 | missing `memory`, `alloc`, or `resolve` | `MissingExport` | same |
 | unreachable/stack/memory trap | `Trap` | same |
 | infinite start function or call | `FuelExhausted` | same |
-| invalid pointer, UTF-8, JSON, empty/escaped path, symlink, or special-file read | `BadReturn` or unreadable carrier | same |
+| invalid pointer, oversized return, UTF-8, JSON, empty/escaped path, symlink, or special-file read | `BadReturn` or unreadable carrier | same |
 | feature disabled | registered-profile error | same; no alternate resolver |
 
 All wasmtime code and dependencies are gated by `wasm-resolver`, forwarded from

src/agents.rs

@@ -59,6 +59,8 @@ pub fn roster_from_discovered(
     this_host: &str,
 ) -> Vec<AgentRow> {
     let pty_root = probe_pty_root(catalog_root);
+    let profiles = crate::catalog::declared_profiles(catalog_root).unwrap_or_default();
+    let profile_refresh = profiles.begin_refresh();
     let mut rows: Vec<AgentRow> = found
         .specs
         .iter()
@@ -76,7 +78,13 @@ pub fn roster_from_discovered(
                 resource_resync: s
                     .resources
                     .iter()
-                    .map(|resource| crate::resync::resource_coverage(agent_dir, resource))
+                    .map(|resource| {
+                        crate::resync::resource_coverage_with_profiles(
+                            agent_dir,
+                            resource,
+                            &profile_refresh,
+                        )
+                    })
                     .collect(),
                 last_activity_ms: newest_activity_ms(agent_dir),
                 inbox: inbox_count(agent_dir),