Skip to content

Python: an invalid method token queued before the first run fails that run and silently drops every other queued allow_domain rule #228

Description

@antsok

In the Python binding, Sandbox builds the native sandbox lazily on the first run(). Until then allow_domain(target, methods) stores the raw strings:

#[pyo3(signature = (target, methods=None))]
fn allow_domain(&mut self, target: &str, methods: Option<Vec<String>>) -> PyResult<()> {
    if let Some(sandbox) = self.inner.as_mut() {
        let methods = HttpMethod::parse_list(methods)
            .map_err(|e| PyRuntimeError::new_err(format!("{e}")))?;
        sandbox
            .allow_domain(target, methods)
            .map_err(|e| PyRuntimeError::new_err(format!("{e}")))?;
    } else {
        self.pending_networks.push((target.to_string(), methods));
    }
    Ok(())
}

The first run() then drains the queue after building:

for (target, methods) in std::mem::take(&mut self.pending_networks) {
    let methods = HttpMethod::parse_list(methods)
        .map_err(|e| PyRuntimeError::new_err(format!("{e}")))?;
    sandbox
        .allow_domain(&target, methods)
        .map_err(|e| PyRuntimeError::new_err(format!("{e}")))?;
}
self.inner = Some(sandbox);

Three things follow when one queued token is not a valid method. The error surfaces from run(), two calls away from the allow_domain that caused it. The ? returns before self.inner is set, so the built sandbox is dropped and its build cost is paid again. And the queue was already taken, so every other rule queued before the first run, valid or not, is discarded: the next run() builds a sandbox with no network rules at all and never says so.

Reproduced on hyperlight-sandbox 0.7.0 with backend-wasm 0.7.0 and python-guest 0.7.0, Windows 11 x86-64, WHP, CPython 3.13:

from hyperlight_sandbox import Sandbox

GET = "print(http_get('https://example.com/')['status'])"

sb = Sandbox()
sb.allow_domain("https://example.com", ["GET"])
print(sb.run(GET).stdout.strip())                    # 200: the GET rule works on its own

sb = Sandbox()
sb.allow_domain("https://example.com", ["GET"])
sb.allow_domain("https://example.com", ["PROPFIND"])  # accepted, no error
try:
    sb.run("print('plain')")
except RuntimeError as exc:
    print("first run:", exc)                         # invalid HTTP method: PROPFIND
print(sb.run("print('plain')").stdout.strip())       # plain: the second run works
r = sb.run(GET)
print(r.stderr.strip(), r.exit_code)                 # Err: ErrorCode_HttpRequestDenied() 1

sb = Sandbox()
sb.run("print('warm')")
sb.allow_domain("https://example.com", ["PROPFIND"])  # raises at once: the initialised path is right
200
first run: invalid HTTP method: PROPFIND
plain
Err: ErrorCode_HttpRequestDenied() 1
Traceback (most recent call last):
  ...
RuntimeError: invalid HTTP method: PROPFIND

Expected: the queued branch validates like the initialised one. Parse in allow_domain in both branches and queue the parsed MethodFilter, so a bad token raises from the call that supplied it and cannot take its siblings with it:

let methods = HttpMethod::parse_list(methods)
    .map_err(|e| PyRuntimeError::new_err(format!("{e}")))?;
match self.inner.as_mut() {
    Some(sandbox) => sandbox.allow_domain(target, methods).map_err(...)?,
    None => self.pending_networks.push((target.to_string(), methods)),
}

with pending_networks: Vec<(String, MethodFilter)>. parse_list already enforces the list bound, so nothing else moves. If a drain failure should remain possible for some other reason, self.inner ought to be set before the loop, or the loop ought to run on the remaining entries, so one refused rule does not empty the policy. Happy to send the PR.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions