feat(reborn): add shared runtime HTTP egress#3098
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces several core crates for the 'Reborn' architecture, including ironclaw_network for network policy and HTTP transport, ironclaw_secrets for scoped secret leasing, and a hardened ironclaw_wasm runtime. It also establishes a shared runtime HTTP egress service in ironclaw_host_runtime and updates the MCP runtime to utilize these new boundaries. Feedback focuses on improving error semantics in the MCP crate by using specific variants for unsupported transports and simplifying a redundant timeout check within the WASM host import thread limiter.
|
One test gap I would close before this lands: the host-runtime egress tests prove lease consumption and redaction, but they should also assert that the actual outbound Suggested coverage: use the existing recording network around That gives us caller-level proof that the external call receives the leased secret, not just helper-level proof that a lease was fetched and later redacted. |
|
Core authority-boundary question for this slice: where is the decision made that a given external HTTP call should receive a given The host-runtime path looks like it can safely lease, consume, inject, and redact a requested secret. What I want to verify is that The invariant I would expect before
If that policy source is intentionally out of scope for this PR, can we document the expected upstream owner/contract so |
|
Addressed the review comments in ef4f2de.
|
henrypark133
left a comment
There was a problem hiding this comment.
Review: hold on runtime egress boundary
The overall direction is strong: the PR creates a clearer split where ironclaw_network owns URL/DNS/private-IP/transport enforcement, ironclaw_secrets owns scoped one-shot lease mechanics, and ironclaw_host_runtime composes the two for runtime HTTP egress.
What looks good:
- The low-level ownership boundaries are mostly clean and backed by crate boundary tests.
- Focused local tests pass for the new runtime, network, secrets, script, MCP, and WASM paths.
- MCP stdio remains fail-closed until process-level egress controls exist, which is the right default.
Concerning: credential-injection authority boundary is still unresolved
I left a separate thread asking where the decision is made that a specific external HTTP call should receive a specific RuntimeCredentialInjection. The host-runtime path can safely lease, consume, inject, and redact a requested secret, but we need the upstream contract that proves the injection request is host-derived from an approved capability/obligation rather than invented by runtime/plugin code.
Before this lands, I would expect the contract to make clear that secret injection only happens after the host verifies the capability declared the secret, the caller was authorized/approved to use it, the outbound URL matches the allowed destination/pattern, the injection shape is host-approved, and the final request still passes network policy.
Concerning: host-runtime injection test should assert the actual outbound request
I also left a separate test-gap comment. The current host-runtime egress tests prove lease consumption and redaction, but they should also assert that the recorded outbound NetworkHttpRequest contains the injected header/query param. That gives caller-level proof that the external request receives the leased secret, not only helper-level proof that a lease was fetched.
Low-priority notes:
docs/reborn/contracts/network.mdsays response body limiting is a non-goal, but the code and tests implementresponse_body_limit. That non-goal should be corrected.git diff --check origin/reborn-integration...HEADcurrently fails oncrates/ironclaw_wasm/src/constants.rs:25due a blank line at EOF.- The PR is draft, merge-conflicting, and currently only has lightweight scope/classify checks, so I would hold until it has meaningful CI after conflict resolution.
Local verification:
- Passed:
cargo test -p ironclaw_host_runtime -p ironclaw_network -p ironclaw_secrets -p ironclaw_scripts -p ironclaw_mcp -p ironclaw_wasm - Passed:
cargo test -p ironclaw_architecture reborn_runtime_http_egress_has_single_network_boundary - Failed:
git diff --check origin/reborn-integration...HEAD
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef4f2dea1e
ℹ️ 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".
|
Addressed the latest review follow-ups in 73a7447.
The earlier credential-injection authority boundary and outbound-injection test concerns are covered by ef4f2de and remain present after the merge-resolution commit. |
There was a problem hiding this comment.
Pull request overview
This PR introduces a shared, host-mediated HTTP egress path for Reborn runtimes, with common contracts in ironclaw_host_api, a composed implementation in ironclaw_host_runtime, and runtime-lane adapters/tests to ensure all outbound HTTP is routed through a single enforced boundary.
Changes:
- Add
RuntimeHttpEgresscontracts toironclaw_host_apiand implementHostHttpEgressServiceinironclaw_host_runtime(composing secrets + network). - Extend
ironclaw_networkto include host-mediated HTTP egress (URL parsing, DNS resolution, private-IP denial, redirect-disabled transport, response streaming limits, and request/response byte accounting). - Add WASM/Script/MCP thin adapters and contract tests, plus architecture ratchets to prevent direct HTTP/DNS usage outside the boundary.
Reviewed changes
Copilot reviewed 40 out of 41 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/reborn/contracts/secrets.md | Updates secrets contract text to reflect host-runtime injection composition responsibilities. |
| docs/reborn/contracts/network.md | Updates network contract to include HTTP egress boundary behavior and test expectations. |
| docs/reborn/README.md | Adds pointers to new/expanded Reborn crates (host_runtime, network, secrets, wasm, scripts, mcp). |
| docs/plans/2026-04-29-reborn-wasm-wit-compatibility.md | Adds a detailed WIT-compatibility plan aligned with shared egress/secrets boundaries. |
| crates/ironclaw_host_api/src/lib.rs | Exposes new shared HTTP egress contract module. |
| crates/ironclaw_host_api/src/http.rs | Adds RuntimeHttpEgress* request/response/errors and credential injection plan types. |
| crates/ironclaw_host_runtime/src/lib.rs | Implements shared host egress service composing ironclaw_network + ironclaw_secrets with redaction. |
| crates/ironclaw_network/src/lib.rs | Adds network-owned HTTP egress types, DNS resolver, policy wrapper, and reqwest-based transport. |
| crates/ironclaw_network/Cargo.toml | Adds reqwest + url dependencies for transport and parsing. |
| crates/ironclaw_wasm/src/* + tests/* | Adds WASM runtime substrate + host import plumbing + contract tests, including HTTP adapter wiring. |
| crates/ironclaw_scripts/src/lib.rs + tests/script_http_adapter_contract.rs | Adds Script HTTP adapter that delegates to shared runtime egress. |
| crates/ironclaw_mcp/src/lib.rs + tests/mcp_adapter_contract.rs | Adds MCP HTTP adapter and fails closed for external stdio transport until process controls exist. |
| crates/ironclaw_architecture/tests/reborn_dependency_boundaries.rs | Adds ratchet test to forbid direct reqwest/DNS/SSRF helpers in runtime crates. |
| Cargo.toml / Cargo.lock | Wires new crates into the workspace and lockfile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
75711af to
15a400d
Compare
72cf5a1 to
3b6fdd8
Compare
Summary
RuntimeHttpEgresscontracts inironclaw_host_apiandHostHttpEgressServiceinironclaw_host_runtime.ironclaw_networkfrom policy-only substrate to host-mediated HTTP egress with URL parsing, policy checks, DNS/private-IP protection, redirect-disabled transport, streaming response limits, and separate request/response byte accounting.Review notes
crates/ironclaw_wasmon this stack. The WASM adapter is wired through the transitional JSON import harness for coverage; the WIT rewrite follow-up must route generatedhttp-requestimports throughWasmRuntimeHttpAdapter.ResourceUsage.network_egress_bytesremains request-only. Pre-transport policy/DNS/private-IP rejections now report zero outbound request bytes; response bytes stay separate.Verification
cargo test -p ironclaw_networkcargo test -p ironclaw_host_runtimecargo test -p ironclaw_host_apicargo test -p ironclaw_scripts -p ironclaw_mcpcargo test -p ironclaw_wasmcargo test -p ironclaw_architecturecargo clippy -p ironclaw_host_api -p ironclaw_network -p ironclaw_host_runtime -p ironclaw_scripts -p ironclaw_mcp -p ironclaw_wasm -p ironclaw_architecture --all-targets -- -D warningscargo fmt --all -- --checkgit diff --check