Add the universal WASIp2 Resource executor - #410
Conversation
schickling-assistant
changed the title
|
@codex please review this PR. Posted on behalf of @schickling
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48c9fe4e41
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
agent-identity: dev3.direct.omp.2cshu64q agent-persona: generalist agent-supervisor: unavailable agent-tool: OMP agent-tool-version: 18.0.9 agent-runtime: OMP 18.0.9 tooling-profile: dotfiles@b607597
agent-identity: dev3.direct.omp.2cshu64q agent-persona: generalist agent-supervisor: unavailable agent-tool: OMP agent-tool-version: 18.0.9 agent-runtime: OMP 18.0.9 tooling-profile: dotfiles@b607597
48c9fe4 to
ce39603
Compare
|
@codex review Posted on behalf of @schickling
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce39603dec
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for path in [&manifest_path, &artifact_path] { | ||
| if let Err(error) = | ||
| validate_owned_cache_entry_with(cache.root(), path, effective_uid(), &oracle) |
There was a problem hiding this comment.
When the cache root is beneath a directory another UID can rename entries in, these pathname checks do not authenticate the bytes being deserialized: both read_regular_bounded calls have already closed their file descriptors, so an attacker can substitute a malicious cache tree for the reads and restore the trusted tree before this loop runs. The malicious manifest supplies its own matching artifact digest, allowing attacker-chosen AOT bytes to reach Component::deserialize; validate ownership and mode on the exact opened descriptors and traverse from a retained root descriptor. Fresh evidence after the ownership fix is that both cache reads still complete before this pathname-based validation.
Useful? React with 👍 / 👎.
| let selector_schema = serde_json::from_str(&descriptor.selector_schema_json) | ||
| .map_err(DescriptorValidationError::SelectorSchemaJson)?; | ||
| let default_selector = serde_json::from_str(&descriptor.default_selector_json) | ||
| .map_err(DescriptorValidationError::DefaultSelectorJson)?; |
There was a problem hiding this comment.
A provider can return a syntactically valid schema such as {"type":"integer"} with the default selector "not-an-integer", and describe accepts both because this only parses JSON. A binding that omits its selector can therefore activate with a default that violates the provider's own schema; compile/validate the supported schema definition and validate the default against it before returning the descriptor.
Useful? React with 👍 / 👎.
| let component = Arc::new(component); | ||
| components.insert(digest, Arc::clone(&component)); |
There was a problem hiding this comment.
In a long-running executor that observes component upgrades or many distinct catalog components, every successful load permanently retains another compiled Component in this HashMap; there is no eviction or removal path. Compiled Wasmtime artifacts can be substantially larger than their admitted input, so repeated reloads can grow process memory without bound despite the per-invocation resource limits; use a bounded cache or release entries no longer referenced by callers.
Useful? React with 👍 / 👎.
1 participant
Problem
Resource providers need one sandboxed execution path. A provider-specific native runtime would duplicate lifecycle, limits, cancellation, and artifact trust rules.
Goal
Execute any admitted Resource observer component through one WASIp2 Component Model runtime with fresh per-observation state and reusable immutable artifacts.
Decisions
Verification
CI=1 nix develop -c cargo test -p st2-resource-wasip2 --features runtimenix build .#checks.x86_64-linux.wasip2-resource-executor --no-link --print-build-logsBoth passed locally: 5 library tests and 13 executor integration tests. Coverage includes import admission, fresh Store state, fuel/epoch cancellation, resource limits, traps, corrupt cache rejection, concurrent writers, and AOT reuse.
Complexity
Wasmtime and the verified AOT cache are substantial, but they replace per-provider execution frameworks with one bounded runtime and one artifact trust boundary.
Concerns
The provider-enabled binary has a larger closure. The default
st2package does not enable the provider runtime feature.Friction & bottlenecks
Nix auto-GC removed an input during one validation attempt. The exact check passed on retry. Component compilation is the dominant local build cost.
Follow-ups
The next PR in stack #412 supplies GitHub Issue and PTY stats capability modules and production components.
References
Posted on behalf of @schickling
agent_identitysessionagent_personaagent_supervisoragent_toolagent_tool_versionagent_runtimetooling_profile