Security review fixes (secure-exec)#79
Merged
Merged
Conversation
…limit
Guest inline JS (e.g. while(true){}) ran unbounded on the shared, slot-bounded
V8 runtime because register_v8_session hardcoded cpu_time_limit_ms: 0 at every
CreateSession (0 normalizes to None, so no TimeoutGuard was armed), letting a
guest pin a CPU core and starve peers.
Resolve a default CPU-time budget (V8_DEFAULT_CPU_TIME_LIMIT_MS = 30000ms,
overridable/disable-able via AGENT_OS_V8_CPU_TIME_LIMIT_MS) and thread it through
register_v8_session into the CreateSession frame so the existing watchdog pipeline
arms a TimeoutGuard.
Adds bounded safeguard test javascript_infinite_loop_is_terminated_by_cpu_watchdog.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
The operator-configured AGENT_OS_WASM_MAX_STACK_BYTES budget was plumbed into the WASM runner env but never read by the engine, so the cap was silently ignored (dead): runaway recursion only ever tripped V8's generic default native stack guard with no attribution to the configured limit. The engine now parses the stack byte budget (rejecting malformed values like fuel/memory) and the WASM runner installs a stack-exhaustion guard around wasi.start: when a stack byte limit is configured and the guest exhausts the stack, the runner terminates nonzero and attributes the failure to the configured budget instead of leaking the engine's generic default-guard message. Adds bounded SAFEGUARD test wasm_deep_recursion_respects_configured_stack_byte_limit. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…minate isolate instead of fatal-aborting the shared process Hitting the operator-configured AGENT_OS_V8_HEAP_LIMIT_MB cap previously triggered V8's default fatal-OOM abort (SIGTRAP). Because the V8 runtime is process-global, one tenant's heap bomb crashed the whole process and every concurrent tenant (guest-triggerable cross-tenant host DoS). create_isolate now installs a near-heap-limit callback (via the new install_heap_limit_guard helper) on every isolate created with a heap cap, covering both the fresh-isolate and snapshot-restore paths. On approaching the cap the callback terminates the offending isolate's execution and returns a small headroom bump so V8 can propagate the uncatchable termination exception cleanly instead of aborting. The guest run ends with a nonzero exit. Adds bounded SAFEGUARD test javascript_heap_allocation_bomb_is_capped_by_oom_guard. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
… no fuel env is set resolve_wasm_execution_timeout returned None unless AGENT_OS_WASM_MAX_FUEL was set, so wait() gated termination behind Some(limit) and a never-returning guest module (e.g. an infinite loop) pinned a host CPU core forever under default config, starving other tenants on the shared process. It now applies a default wall-clock budget (DEFAULT_WASM_EXECUTION_TIMEOUT_MS = 30000ms) when no fuel env is configured; operators can still set AGENT_OS_WASM_MAX_FUEL explicitly for a tighter or looser bound. Adds bounded SAFEGUARD unit test wasm_execution_timeout_defaults_to_bounded_value_without_fuel_env. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…100.64/10 as restricted egress in SSRF classifier A guest with network grants could connect to 0.0.0.0 (which Linux routes to 127.0.0.1) and bypass the loopback port-ownership gate, since the egress classifier returned None for the unspecified address and is_loopback_ip was false. CGNAT 100.64.0.0/10 was likewise unclassified. restricted_non_loopback_ip_range now classifies the IPv4/IPv6 unspecified addresses and the CGNAT block as restricted so they are denied with EACCES. Adds bounded SAFEGUARD test classifier_denies_unspecified_and_cgnat_targets. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…) in SSRF classifier so ::169.254.169.254 is blocked The classifier canonicalized IPv6 via to_ipv4_mapped() only, which returns None for IPv4-compatible addresses (::a.b.c.d), so a guest could spell the cloud- metadata address as ::169.254.169.254 and bypass the 169.254.0.0/16 link-local block. Adds ipv4_compatible_embedded() and consults it before the IPv6 fallthrough so IPv4-compatible spellings canonicalize back to their embedded IPv4 address and are classified; :: and ::1 are excluded so they fall through to the unspecified/loopback paths. Adds bounded SAFEGUARD test classifier_denies_ipv6_spelled_metadata_addresses. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
|
🚅 Environment secure-exec-pr-79 in rivet-frontend has no services deployed. |
…ges in SSRF egress classifier restricted_non_loopback_ip_range omitted 240.0.0.0/4 (reserved/future-use, including the 255.255.255.255 broadcast) and 224.0.0.0/4 (multicast), so a guest connect to e.g. 240.0.0.1 was attempted instead of denied with EACCES. Both ranges are now classified as restricted (IPv4-compatible IPv6 spellings canonicalize through the F-006 path and are denied too). Adds bounded SAFEGUARD test classifier_denies_reserved_and_multicast_targets. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…t); remove 30s wall-clock default; wall-clock backstop deferred
NathanFlurry
force-pushed
the
security/runtime-fixes-main
branch
from
June 19, 2026 08:50
7164afd to
fa0cebb
Compare
Member
Author
|
Companion to #79 (security/runtime-fixes-main): #86 closes the remaining security-review coverage gaps, based on top of this merge.
See #86 for per-gap detail. |
NathanFlurry
added a commit
that referenced
this pull request
Jun 19, 2026
…et) (#89) Deferred follow-up to #79's CPU-only F-001. Re-wires the dormant wall-clock TimeoutGuard as an INDEPENDENT, opt-in limit alongside the CPU-time budget. New env knob AGENT_OS_V8_WALL_CLOCK_LIMIT_MS (>0 to arm; unset/0 => off, no default). Unlike the CPU budget, the wall-clock backstop counts elapsed real time INCLUDING idle/await, so it can cap a guest that blocks or awaits indefinitely. Both guards can be armed at once; whichever fires first calls terminate_execution and the result frame reports which (ERR_SCRIPT_WALL_CLOCK_EXCEEDED vs ERR_SCRIPT_CPU_BUDGET_EXCEEDED). Off by default so long-lived ACP adapters are never killed by a wall-clock default; the CPU budget remains the primary guard. Threaded through the CreateSession/Start frame next to the CPU budget knob. Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
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
1 participant
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.
Re-applies the secure-exec security-review fixes cleanly on current
main(e118ec8). Each fix was re-derived against current code (post JS-runtime-platform / Bun-CBOR-IPC refactors), implemented as its own commit with a verifying adversarial test that passes. Expensive saturation variants stay env-gated; the safeguard tests run by default and are watchdog-bounded.Supersedes #76, which was cut from the abandoned
split/runtime-previewbase and showed an unusable 2247-file diff.Fixes
while(true){}pinned a core on the shared, slot-bounded V8 runtime and starved peers. This PR adds an opt-in CPU-time budget watchdog: a thread that samples the execution thread's per-thread CPU clock (pthread_getcpuclockid→clock_gettime, ~50ms poll) and, when accumulated active-JS CPU time exceeds the budget, callsIsolate::terminate_execution()and signalsCpuBudgetExceeded(reported asERR_SCRIPT_CPU_BUDGET_EXCEEDED). Because a thread's CPU clock does not advance while parked/awaiting, this excludes I/O/idle wait — only active JS CPU counts (the standard embedder pattern, since V8 has no native budget). The budget is set viaAGENT_OS_V8_CPU_TIME_LIMIT_MS(true CPU ms); unset/0⇒ watchdog NOT armed (no limit). The earlier 30s default has been removed — there is no default, so a guest is CPU-bounded only when the operator/platform sets the env. Tests:javascript_infinite_loop_is_terminated_by_cpu_watchdog(budget set ⇒ tight loop killed, cpu-budget reason),javascript_awaiting_guest_is_not_killed_by_cpu_budget(budget set, mostly-awaiting guest past the budget window is not killed — proves idle/await excluded),javascript_no_cpu_budget_when_env_unset(no env ⇒ not armed, not time-limited).AGENT_OS_WASM_MAX_STACK_BYTESwas plumbed but never read (dead cap). The engine now parses it (rejecting malformed values) and the WASM runner installs a stack-exhaustion guard aroundwasi.startthat terminates nonzero and attributes the failure to the configured budget. Test:wasm_deep_recursion_respects_configured_stack_byte_limit.AGENT_OS_V8_HEAP_LIMIT_MBtriggered V8's fatal-OOM abort (SIGTRAP), crashing the process-global runtime and every concurrent tenant.install_heap_limit_guardnow registers a near-heap-limit callback on every heap-capped isolate (fresh + snapshot-restore paths) that terminates the offending isolate and grants headroom so V8 propagates the termination cleanly. Test:javascript_heap_allocation_bomb_is_capped_by_oom_guard.resolve_wasm_execution_timeoutreturned None withoutAGENT_OS_WASM_MAX_FUEL, sowait()never terminated a never-returning module. Now applies a default 30s budget when no fuel env is set. Test:wasm_execution_timeout_defaults_to_bounded_value_without_fuel_env.0.0.0.0/::(which route to host loopback) and CGNAT100.64.0.0/10. Both are now classified restricted and denied with EACCES. Test:classifier_denies_unspecified_and_cgnat_targets.to_ipv4_mapped(), missing IPv4-compatible spellings (::169.254.169.254). Addedipv4_compatible_embedded()so::a.b.c.dcanonicalizes to its embedded IPv4 and is classified (::/::1excluded). Test:classifier_denies_ipv6_spelled_metadata_addresses.240.0.0.0/4(reserved, incl. broadcast) and224.0.0.0/4(multicast) to the classifier. Test:classifier_denies_reserved_and_multicast_targets.Not in this PR
TimeoutGuard(wall-clock timer) is left in place but dormant — not armed by F-001. A follow-up PR will re-introduce a wall-clock backstop as its own separate opt-in knob.@rivet-dev/agent-os-browserin agent-os; secure-exec'spackages/browser(@secure-exec/browser) is an orphaned copy not depended on by anything in the secure-exec workspace. Per the boundary (secure-exec stays free of agent/browser concerns), these are not applied here.Verification
cargo build --workspacegreen.javascript_v8_suite(incl. the three F-001 CPU-budget tests),wasm_suite, the sidecarssrf_egress_classifier_tests, and the F-004 unit test all pass.