fix(security): unify SSRF protection for WASM host calls#1060
Open
ferr079 wants to merge 1 commit intoRightNow-AI:mainfrom
Open
fix(security): unify SSRF protection for WASM host calls#1060ferr079 wants to merge 1 commit intoRightNow-AI:mainfrom
ferr079 wants to merge 1 commit intoRightNow-AI:mainfrom
Conversation
Member
|
Reviewed this end-to-end — this is exactly the kind of cleanup we want:
Status: mergeable but CI rollup is empty (no checks registered — likely branch pre-dates current workflow triggers). Can you rebase on latest |
6 tasks
The WASM sandbox host_net_fetch() had its own SSRF implementation (is_ssrf_target) that was incomplete compared to the canonical check_ssrf() in web_fetch.rs: - Missing 6 blocked hostnames (ip6-localhost, Alibaba/Azure IMDS, 0.0.0.0, ::1, [::1]) - Missing cloud metadata IP detection (is_metadata_ip) - Missing IPv6 bracket notation support - Ignoring ssrf_allowed_hosts from config.toml entirely - Duplicate is_private_ip() and extract_host_from_url() functions This meant a WASM agent could bypass SSRF protections that the builtin web_fetch tool correctly enforced. Changes: - Remove duplicated is_ssrf_target(), is_private_ip(), and extract_host_from_url() from host_functions.rs - Delegate to web_fetch::check_ssrf() which has the complete implementation with allowlist, CIDR matching, and metadata IP detection - Add ssrf_allowed_hosts to SandboxConfig and GuestState so the config propagates to WASM host calls - Make extract_host() pub(crate) for reuse - Update tests to exercise the unified code path, including new coverage for IPv6 and cloud metadata endpoints All 908 runtime tests pass. Zero clippy warnings.
ferr079
force-pushed
the
fix/unify-ssrf-protection
branch
from
a493df6 to
de8a692
Compare
Author
|
Rebased on latest Verified locally:
Ready for CI + merge when you're satisfied. |
pbranchu
pushed a commit
to pbranchu/openfang-upstream
that referenced
this pull request
Apr 18, 2026
- RwLock: replace Arc<Mutex<String>> with Arc<RwLock<String>> for
A2A_TASK_PROGRESS — eliminates exclusive lock in read-heavy path
- SSE parsing: extract pure parse_sse_data_line fn + SseLineOutcome enum;
refactor both send_task_streaming and send_task_streaming_with_progress
to use it; add timeout: Option<Duration> to both; 8 unit tests covering
all outcomes (final, update, empty, malformed, error, unknown)
- Bounded maps: add MAX_CONCURRENT_ASYNC_TASKS=256 cap with clear error;
AsyncTaskEntry{handle,inserted_at} tracks task age; OnceLock-based
background sweep (10 min interval, 2 h TTL) aborts stale handles;
TaskCleanupGuard RAII ensures cleanup on panic
- Prompt injection: rewrite inject_async_callback to deliver async results
via structural ToolUse+ToolResult pair instead of text framing; remote
agent content lands in a ToolResult block where LLM API semantics enforce
the data boundary; add prepend_turns: Option<Vec<Message>> to both
run_agent_loop and run_agent_loop_streaming so the synthetic ToolUse is
inserted AFTER validate_and_repair (prevents orphan removal of the
ToolResult user turn)
- Strip @default suffix from agent IDs in tool_agent_send (pre-existing fix)
- Update test to correctly verify prune_failed_tool_turns behavior
SSRF (RightNow-AI#1060): already using canonical crate::web_fetch::check_ssrf
Thread context (RightNow-AI#1054): context.thread_id passed through; will work
correctly once smart-thread sets it
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Author
|
Hey @jaberjaber23 — just a friendly ping. The rebase on latest |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The WASM sandbox
host_net_fetch()has its own SSRF implementation (is_ssrf_target) that diverged from the canonicalcheck_ssrf()inweb_fetch.rs. This creates a security gap where WASM agents can bypass protections that builtin tools correctly enforce.Issues found in
host_functions.rs::is_ssrf_target():ip6-localhost,[::1],::1,0.0.0.0, Alibaba100.100.100.200, Azure192.0.0.192)is_metadata_ip()check on resolved IPsssrf_allowed_hostsfrom config.toml completely ignored[::1]:8080not parsed correctlyis_private_ip()andextract_host_from_url()Changes
is_ssrf_target(),is_private_ip(), andextract_host_from_url()fromhost_functions.rsweb_fetch::check_ssrf()which has the complete implementationssrf_allowed_hoststhroughSandboxConfig→GuestState→ host callextract_host()topub(crate)for reuseResult
ssrf_allowed_hostsconfig now respected by WASM host callsTest plan
cargo check -p openfang-runtime -p openfang-kernel -p openfang-types— compiles cleancargo test -p openfang-runtime— 908 tests passcargo clippy -p openfang-runtime -p openfang-kernel -- -D warnings— zero warningsssrf_allowed_hosts = ["192.168.1.0/24"]and verify WASM agent can reach allowed internal services