makepad/platform/script/tests/fn_ref_callback.rs
Kevin Boos 6dd0b2c133
Splash: a host-services bridge for mini-apps, plus three VM/parser fixes (#1181)
* Splash: a host-services bridge so isolates can ask for brokered capabilities

Mini-apps are sandboxed hard (fs/run/res stripped, net gated), which also
means they can't do anything real. This adds the one doorway back: a
mod.host module in every isolate whose host.request(service, args, cb)
queues {app_tag, heap_key, req_id, service, args_json} on a thread-local
the EMBEDDING HOST drains and answers (splash_host_respond re-enters the
isolate under the normal budget and calls the callback with {ok, data,
error}). No policy lives in makepad: an undrained request never resolves,
tags are host-assigned (Splash::set_host_tag) so scripts can't spoof who
they are, and host.capabilities() just echoes whatever grant list the
host last pushed (set_host_caps). Callbacks are rooted ScriptFnRefs keyed
by heap, GC'd with the isolate alongside the storage-jail roots.

Also: call_script_fn_with_strings (string args must be minted in the
callee's own heap), 'let host = mod.host' in both Splash prefixes (line
offsets documented per prefix; the net prefix was already one line off),
and mod.cx.quit is now nil'd in isolates - a mini-app could quit the
whole host process with one call.

* script: stop validation from blessing scripts that failed to parse

The parser RECOVERS from errors (dangling else, missing expression), logs
them, sets had_error - which nothing ever read - and hands back a runnable
module. Nothing enters the trap queue, so a host validating with a
captured_errors sink + take_errors() got an empty list and reported
success; three freshly-written mini-apps shipped real parse errors straight
through host_launcher's validate this way, visible only as stray [E] log
lines.

report_error now also records the formatted message on the parser
(ScriptParser::errors), and both eval paths (eval_with_source and the
streaming eval_with_append_source) drain that into bx.captured_errors when
a sink is installed. No sink = logs only, exactly as before. Regression
tests in tests/parse_error_capture.rs, including the exact fn-final
if/else shape that slipped through.

* splash_host: review fixes — is_ok result field, silent surfaces, JSON hardening

Three classes of fixes from an adversarial review of the bridge:

- The result object's success field is now is_ok. 'ok' is the script
  dialect's ok-test KEYWORD, so r.ok never parsed as a field access — every
  callback that read it silently died. A pure-VM regression test
  (fn_ref_callback.rs) now exercises the exact store-callback-then-answer
  flow the bridge uses.

- SplashHostRequest carries may_prompt, set per isolate via
  Splash::set_host_prompts: background surfaces (home-screen widget tiles)
  are marked silent so a host can fail their permission-needing requests
  instead of popping consent dialogs nobody asked for. splash_host_respond
  also reports an outcome now (Delivered / NoCallback / IsolateGone) so
  hosts can log undeliverable answers, and Splash::isolate_heap_key lets a
  host relate a request to a specific widget (IPC fan-out skips the
  sender's own isolate with it).

- heap.to_json hardening: a cyclic object graph (script-buildable, host-
  serialized on every bridge request) recursed to a stack overflow — now a
  depth cap emits null leaves; backslashes were mis-escaped as a single
  backslash (invalid JSON downstream), tab and other control chars weren't
  escaped at all, and a handle serialized as unquoted junk.

* script: a closure's captured varargs must not shadow its own parameters

A call binds positional args by INDEXING the fn object's vec, which holds
declared parameters — but also, past that, any varargs the call received
(unnamed_fn_arg pushes them with a NIL key). A closure captures the scope
it was minted in, so those leftovers ride along ahead of the closure's own
parameters.

Concretely: script timers invoke their callback with one number (the time).
Hand start_timeout a zero-arg closure and that number lands in the scope as
a NIL-keyed vararg; any closure created in that body then binds its FIRST
parameter against the leftover — first failing the typecheck ("arg 0 (nil)
type mismatch: expected number, got object"), and once that was relaxed,
binding the value under the NIL key so the real parameter stayed nil. It
cost a full debug cycle in host_launcher, where every host-service callback
created inside a boot timer silently never ran.

Both binding paths now walk the DECLARED (named) entries in order, so
captured varargs can never be mistaken for a parameter. Regression test in
tests/extra_arg_typecheck.rs reproduces the timer shape exactly.

(Pre-existing and unrelated: widget_tree's test_observe_and_find_single_node
and test_property_patch_no_structural_rebuild fail on upstream dev too.)

* script: stop parse_json silently dropping negative numbers

The tokenizer emits a leading `-` as its own Operator token, and none of
the three JSON value positions (object value, array element, root) had a
case for it. The sign was swallowed — and inside an object the KEY went
with it, because the minus consumed the value slot and the parser resynced
on the next token.

So `{"lat":37.7,"lon":-122.4}` parsed to `{"lat":37.7}`. No error, no
warning, just a missing field. That is how it was found: a mini-app asked
the host where it was, got coordinates with no longitude, and quietly fell
back to a default city. Sub-zero temperatures and negative UTC offsets
(New York is -14400) were being dropped the same way.

A pending-sign flag is applied to the next number in all three positions.
Bare scalar roots stay unsupported ("42" never parsed either) — separate
pre-existing gap, not touched here. Tests in
platform/script/tests/json_negative_numbers.rs.

* splash_storage: let the host raise a single isolate's jail quota

The jail's 16MB whole-app cap is a constant, so "this app may keep more
than the standard amount" had nowhere to live. A per-heap quota map beside
SANDBOX_ROOTS gives the host one, set through Splash::set_storage_quota
and cleared with the isolate like every other per-isolate binding. Script
still can't see or raise its own cap.

Lowering a quota never deletes anything — it just stops further growth —
so revoking the grant is safe on an app that already wrote past the
default.

host_launcher uses this for a `storage-large` permission (64MB), which is
the point: a capability the user can revoke and have it actually mean
something.
2026-08-19 20:06:01 +02:00

159 lines
5.6 KiB
Rust

//! Mirrors the splash_host bridge: a native fn stores a script closure as a
//! ScriptFnRef; the host later builds a result object and calls it.
use std::cell::RefCell;
use makepad_script::*;
fn test_vm() -> ScriptVm<'static> {
let host = Box::leak(Box::new(0i32));
let std = Box::leak(Box::new(0i32));
ScriptVm {
host,
std,
bx: Box::new(ScriptVmBase::new()),
}
}
std::thread_local! {
static STORED: RefCell<Option<ScriptFnRef>> = RefCell::new(None);
}
#[test]
fn stored_fn_ref_calls_back_with_built_object() {
let mut vm = test_vm();
let m = vm.new_module(id!(hostx));
vm.add_method(
m,
id_lut!(request),
script_args_def!(service = NIL, args = NIL, on_result = NIL),
|vm, args| {
let on_result = script_value!(vm, args.on_result);
let Some(obj) = on_result.as_object() else {
panic!("callback is not an object");
};
assert!(vm.bx.heap.is_fn(obj), "callback object is not fn-tagged");
let fnref = vm.bx.heap.new_fn_ref(obj);
STORED.with(|s| *s.borrow_mut() = Some(fnref));
1.0.into()
},
);
vm.bx.captured_errors = Some(Vec::new());
vm.with_instruction_limit(500_000, |vm| {
vm.eval(ScriptMod {
cargo_manifest_path: String::new(),
module_path: String::new(),
file: "fn_ref_callback".to_string(),
line: 0,
column: 0,
// `is_ok`, not `ok`: `ok` is the ok-test keyword and `r.ok` does
// not parse as field access — the exact bug that motivated the
// bridge's result-field name.
code: "let got = {value: -1}\nlet _rid = mod.hostx.request(\"svc\", {}, fn(r) { got.value = r.is_ok })\n(got)"
.to_string(),
values: vec![],
})
});
// The host answers later: build {ok: true, ...} and invoke the callback,
// exactly as splash_host_respond does.
let callback = STORED.with(|s| s.borrow_mut().take()).expect("stored");
let obj = vm.bx.heap.new_object();
let trap = vm.bx.threads.cur().trap.pass();
vm.bx.heap.set_value(obj, id!(is_ok).into(), true.into(), trap);
vm.bx.heap.set_value(obj, id!(data).into(), NIL, trap);
vm.with_instruction_limit(500_000, |vm| {
vm.call(callback.as_object().into(), &[obj.into()]);
});
let errors = vm.take_errors();
assert!(errors.is_empty(), "callback errored: {errors:?}");
// The closure wrote r.ok into module state.
let scope = {
let bodies = vm.bx.code.bodies.borrow();
bodies
.iter()
.find_map(|body| match &body.source {
ScriptSource::Mod(m) if m.file == "fn_ref_callback" => {
Some(body.scope.as_object())
}
_ => None,
})
.expect("body scope")
};
let got = vm.bx.heap.scope_value(scope, id!(got), vm.trap());
let got_obj = got.as_object().expect("got object");
let value = vm.bx.heap.value(got_obj, id!(value).into(), vm.trap());
assert_eq!(value.as_bool(), Some(true), "callback did not run: {value:?}");
}
/// The isolation_probe shape: the request is made inside a closure, the
/// callback is a `fn(r)` expression argument, and the result lands in a
/// module scalar through a named fn.
#[test]
fn nested_fn_arg_callback_mutates_module_scalar() {
let mut vm = test_vm();
let m = vm.new_module(id!(hostx));
vm.add_method(
m,
id_lut!(request),
script_args_def!(service = NIL, args = NIL, on_result = NIL),
|vm, args| {
let on_result = script_value!(vm, args.on_result);
let obj = on_result.as_object().expect("callback object");
assert!(vm.bx.heap.is_fn(obj), "callback object is not fn-tagged");
let fnref = vm.bx.heap.new_fn_ref(obj);
STORED.with(|s| *s.borrow_mut() = Some(fnref));
1.0.into()
},
);
vm.bx.captured_errors = Some(Vec::new());
let code = "\
let svc_text = \"PENDING\"
fn svc_result(r){
if r.is_ok {
svc_text = \"ALLOWED\"
return nil
}
svc_text = \"DENIED\"
}
let run = || {
let _rid = mod.hostx.request(\"svc\", {}, fn(r) { svc_result(r) })
}
run()";
vm.with_instruction_limit(500_000, |vm| {
vm.eval(ScriptMod {
cargo_manifest_path: String::new(),
module_path: String::new(),
file: "nested_fn_cb".to_string(),
line: 0,
column: 0,
code: code.to_string(),
values: vec![],
})
});
let callback = STORED.with(|s| s.borrow_mut().take()).expect("stored");
let obj = vm.bx.heap.new_object();
let trap = vm.bx.threads.cur().trap.pass();
vm.bx.heap.set_value(obj, id!(is_ok).into(), false.into(), trap);
vm.with_instruction_limit(500_000, |vm| {
vm.call(callback.as_object().into(), &[obj.into()]);
});
let errors = vm.take_errors();
assert!(errors.is_empty(), "callback errored: {errors:?}");
let scope = {
let bodies = vm.bx.code.bodies.borrow();
bodies
.iter()
.find_map(|body| match &body.source {
ScriptSource::Mod(m) if m.file == "nested_fn_cb" => Some(body.scope.as_object()),
_ => None,
})
.expect("body scope")
};
let text = vm.bx.heap.scope_value(scope, id!(svc_text), vm.trap());
let mut out = String::new();
vm.string_with(text, |_vm, s| out = s.to_string());
assert_eq!(out, "DENIED", "callback chain did not update the module var");
}