fix(functions): don't reuse a JS context left with a queued job (#14980)

Co-authored-by: Hengfei Yang <hengfei.yang@gmail.com>

src/transform/src/js.rs

@@ -106,9 +106,6 @@ fn compile_js_function_inner(
         return Err(std::io::Error::other("JavaScript function cannot be empty"));
     }
 
-    // Phase 1 + Phase 2 Security: Comprehensive pattern blocking
-    // Use centrally-defined security patterns from o2-enterprise
-    // These patterns are statically initialized and reused across all compilations
     #[cfg(feature = "enterprise")]
     {
         use o2_enterprise::enterprise::auth::js_security;
@@ -429,12 +426,16 @@ fn cached_js_context(org_id: &str, function: &str) -> Result<Context, String> {
     })
 }
 
-/// Records what the context just used holds, then evicts least recently used ones over budget.
+/// Records what the just-used context holds, or drops it for a queued job, then evicts over budget.
 fn account_js_context(org_id: &str, function: &str, context: &Context) {
     let heap_bytes = usize::try_from(context.runtime().memory_usage().malloc_size).unwrap_or(0);
+    // Nothing ever runs the job queue, so a context left with a job would only keep growing.
+    let job_pending = context.runtime().is_job_pending();
     JS_CONTEXTS.with(|cache| {
         let mut cache = cache.borrow_mut();
-        if let Some(entry) = cache
+        if job_pending {
+            cache.retain(|c| !(c.org_id == org_id && c.function == function));
+        } else if let Some(entry) = cache
             .iter_mut()
             .find(|c| c.org_id == org_id && c.function == function)
         {
@@ -898,10 +899,6 @@ for (var i = 0; i < filtered.length; i++) {
         assert_eq!(output_array[1]["value"], 80);
     }
 
-    // ============================================================================
-    // Phase 2 Security Hardening Tests
-    // ============================================================================
-
     #[test]
     fn test_security_block_globalthis() {
         let func = r#"globalThis.escape = function() { return "hacked"; };"#;
@@ -1171,4 +1168,44 @@ for (var i = 0; i < filtered.length; i++) {
             assert!(held <= bound, "after {} functions: {held} > {bound}", i + 1);
         }
     }
+
+    #[test]
+    fn test_org_context_is_reused_and_not_seen_by_another_org() {
+        let f = compile_js_function(
+            "Math.calls = (Math.calls || 0) + 1; row.calls = Math.calls;",
+            "org_a",
+        )
+        .unwrap();
+        let calls = |org: &str| {
+            let (out, err) = apply_js_fn(&f, json!({}), org, &[]);
+            assert!(err.is_none(), "{org}: {err:?}");
+            out["calls"].clone()
+        };
+        assert_eq!(calls("reuse_a"), json!(1));
+        assert_eq!(
+            calls("reuse_b"),
+            json!(1),
+            "org B must not see org A's changes"
+        );
+        assert_eq!(calls("reuse_a"), json!(2), "org A's context must be reused");
+    }
+
+    #[test]
+    fn test_context_left_with_a_queued_job_is_not_reused() {
+        let f = compile_js_function(
+            "Promise.resolve().then(function () {}); \
+             Math.calls = (Math.calls || 0) + 1; row.calls = Math.calls;",
+            "jobs_org",
+        )
+        .unwrap();
+        for _ in 0..2 {
+            let (out, err) = apply_js_fn(&f, json!({}), "jobs_org", &[]);
+            assert!(err.is_none(), "{err:?}");
+            assert_eq!(
+                out["calls"],
+                json!(1),
+                "the next call must get a fresh context"
+            );
+        }
+    }
 }

tests/api-testing/tests/pipelines/test_js_function_isolation.py

@@ -0,0 +1,75 @@
+"""API regression tests: a JavaScript function never sees another organization's state (#14868)."""
+
+import uuid
+
+import pytest
+
+SECRET = "SECRET_XYZ_14868"
+# a request runs on whichever blocking-pool thread is free, so repeat to make the two orgs share threads
+ROUNDS = 20
+
+
+@pytest.fixture(scope="module")
+def two_orgs(create_session, base_url):
+    """Two new orgs, created once per module because `DELETE /api/organizations/{id}` answers 404."""
+    orgs = []
+    for role in ("a", "b"):
+        name = f"jsiso_{role}_{uuid.uuid4().hex[:8]}"
+        resp = create_session.post(f"{base_url}api/organizations", json={"name": name})
+        assert resp.status_code in (200, 201), f"could not create org {name}: {resp.status_code} {resp.text[:200]}"
+        orgs.append(resp.json().get("identifier", name))
+    return orgs
+
+
+def _run(session, base_url, org_id, function, event):
+    resp = session.post(
+        f"{base_url}api/{org_id}/functions/test",
+        json={"function": function, "events": [event], "trans_type": 1},
+    )
+    assert resp.status_code == 200, f"{org_id}: expected 200, got {resp.status_code}: {resp.content}"
+    results = resp.json()["results"]
+    assert len(results) == 1, f"{org_id}: expected one result, got: {results}"
+    # a function that failed or never ran has nothing to leak, so the leak checks would pass vacuously
+    assert not results[0].get("message"), f"{org_id}: the function failed: {results[0]!r}"
+    assert results[0]["event"].get("ran") is True, f"{org_id}: the function did not run: {results[0]!r}"
+    return results[0]["event"]
+
+
+def test_js_builtin_replacement_does_not_leak_to_another_org(create_session, base_url, two_orgs):
+    """A `JSON.parse` replaced by one org's function must never see another org's rows."""
+    org_a, org_b = two_orgs
+    hook = (
+        "var _o = JSON.parse;"
+        "if (!JSON.hooked) { JSON.hooked = true;"
+        " JSON.parse = function (s) { JSON.grab = (JSON.grab || '') + s + '|'; return _o(s); }; }"
+        "row.ran = true;"
+        "row.leaked = JSON.grab || '';"
+    )
+    for _ in range(ROUNDS):
+        _run(create_session, base_url, org_a, hook, {"n": 1})
+    for _ in range(ROUNDS):
+        _run(create_session, base_url, org_b, hook, {"secret": SECRET})
+    later = [_run(create_session, base_url, org_a, hook, {"n": 2}) for _ in range(ROUNDS)]
+
+    for event in later:
+        assert SECRET not in event["leaked"], f"org A's JSON.parse hook saw org B's row: {event!r}"
+    # unless the hook outlived a request, no two requests shared a thread and the check above proved nothing
+    assert any(event["leaked"] for event in later), f"org A's hook never outlived a request: {later!r}"
+
+
+def test_js_global_object_property_does_not_leak_to_another_org(create_session, base_url, two_orgs):
+    """A property one org's function adds to a global object must never be visible to another org."""
+    org_a, org_b = two_orgs
+    fn = (
+        "if (row.secret) { JSON.stash = row.secret; }"
+        "row.ran = true;"
+        "row.leaked = (typeof JSON.stash !== 'undefined') ? JSON.stash : '';"
+    )
+    for _ in range(ROUNDS):
+        _run(create_session, base_url, org_a, fn, {"secret": SECRET})
+    for _ in range(ROUNDS):
+        event = _run(create_session, base_url, org_b, fn, {"n": 1})
+        assert SECRET not in str(event.get("leaked", "")), f"org B saw a property set by org A: {event!r}"
+    # unless org A reads its own property back, no two requests shared a thread and the check above proved nothing
+    own = [_run(create_session, base_url, org_a, fn, {"n": 1}).get("leaked") for _ in range(ROUNDS)]
+    assert SECRET in own, f"org A's property never outlived a request: {own!r}"