Support relative paths in js modules (#1481)

python/ccf/proposal_generator.py

@@ -8,6 +8,7 @@ import json
 import os
 import sys
 import functools
+from pathlib import PurePosixPath
 from typing import Union, Optional, Any
 
 from loguru import logger as LOG  # type: ignore
@@ -268,7 +269,12 @@ def set_js_app(app_script_path: str, **kwargs):
 
 @cli_proposal
 def set_module(module_name, module_path, **kwargs):
-    if module_name.endswith(".js"):
+    module_name_ = PurePosixPath(module_name)
+    if not module_name_.is_absolute():
+        raise ValueError("module name must be an absolute path")
+    if any(folder in [".", ".."] for folder in module_name_.parents):
+        raise ValueError("module name must not contain . or .. components")
+    if module_name_.suffix == ".js":
         with open(module_path) as f:
             js = f.read()
         proposal_args = {"name": module_name, "module": {"js": js}}

src/apps/js_generic/js_generic.cpp

@@ -152,12 +152,19 @@ namespace ccfapp
   static JSModuleDef* js_module_loader(
     JSContext* ctx, const char* module_name, void* opaque)
   {
-    LOG_INFO_FMT("Loading module '{}'", module_name);
+    // QuickJS resolves relative paths but in some cases omits leading slashes.
+    std::string module_name_kv(module_name);
+    if (module_name_kv[0] != '/')
+    {
+      module_name_kv.insert(0, "/");
+    }
+
+    LOG_INFO_FMT("Loading module '{}'", module_name_kv);
 
     auto arg = (JSModuleLoaderArg*)opaque;
 
     const auto modules = arg->tx->get_view(arg->network->modules);
-    auto module = modules->get(std::string(module_name));
+    auto module = modules->get(module_name_kv);
     if (!module.has_value())
     {
       JS_ThrowReferenceError(ctx, "module '%s' not found in kv", module_name);
@@ -291,7 +298,7 @@ namespace ccfapp
 
         // Compile module
         std::string code = handler_script.value().text.value();
-        auto path = fmt::format("app_scripts::{}", local_method);
+        auto path = fmt::format("/__endpoint__{}.js", local_method);
         JSValue module = JS_Eval(
           ctx,
           code.c_str(),

src/node/rpc/member_frontend.h

@@ -104,10 +104,16 @@ namespace ccf
       }
     }
 
-    void set_module(kv::Tx& tx, std::string name, Module module)
+    bool set_module(kv::Tx& tx, std::string name, Module module)
     {
+      if (name.empty() || name[0] != '/')
+      {
+        LOG_FAIL_FMT("module names must start with /");
+        return false;
+      }
       auto tx_modules = tx.get_view(network.modules);
       tx_modules->put(name, module);
+      return true;
     }
 
     bool remove_module(kv::Tx& tx, std::string name)
@@ -160,8 +166,7 @@ namespace ccf
         {"set_module",
          [this](ObjectId, kv::Tx& tx, const nlohmann::json& args) {
            const auto parsed = args.get<SetModule>();
-           set_module(tx, parsed.name, parsed.module);
-           return true;
+           return set_module(tx, parsed.name, parsed.module);
          }},
         // remove a module
         {"remove_module",

tests/modules.py

@@ -13,49 +13,66 @@ import ccf.proposal_generator
 
 from loguru import logger as LOG
 
-MODULE_RETURN = "Hello world!"
-MODULE_CONTENT = f"""
+MODULE_PATH_1 = "/app/foo.js"
+MODULE_RETURN_1 = "Hello world!"
+MODULE_CONTENT_1 = f"""
 export function foo() {{
-    return "{MODULE_RETURN}";
+    return "{MODULE_RETURN_1}";
 }}
 """
 
+MODULE_PATH_2 = "/app/bar.js"
+MODULE_CONTENT_2 = """
+import {foo} from "./foo.js"
+export function bar() {
+    return foo();
+}
+"""
+
+# For the purpose of resolving relative import paths,
+# app script modules are currently assumed to be located at /.
+# This will likely change.
 APP_SCRIPT = """
 return {
   ["POST test_module"] = [[
-    import {foo} from "foo.js";
+    import {bar} from "./app/bar.js";
     export default function()
     {
-      return foo();
+      return bar();
     }
   ]]
 }
 """
 
 
+def make_module_set_proposal(path, content, network):
+    primary, _ = network.find_nodes()
+    with tempfile.NamedTemporaryFile("w") as f:
+        f.write(content)
+        f.flush()
+        proposal_body, _ = ccf.proposal_generator.set_module(path, f.name)
+    proposal = network.consortium.get_any_active_member().propose(
+        primary, proposal_body
+    )
+    network.consortium.vote_using_majority(primary, proposal)
+
+
 @reqs.description("Test module set and remove")
 def test_module_set_and_remove(network, args):
     primary, _ = network.find_nodes()
 
     LOG.info("Member makes a module update proposal")
-    with tempfile.NamedTemporaryFile("w") as f:
-        f.write(MODULE_CONTENT)
-        f.flush()
-        proposal_body, _ = ccf.proposal_generator.set_module("foo.js", f.name)
-    proposal = network.consortium.get_any_active_member().propose(
-        primary, proposal_body
-    )
-    network.consortium.vote_using_majority(primary, proposal)
+    make_module_set_proposal(MODULE_PATH_1, MODULE_CONTENT_1, network)
 
     with primary.client(
         f"member{network.consortium.get_any_active_member().member_id}"
     ) as c:
-        r = c.post("/gov/read", {"table": "ccf.modules", "key": "foo.js"})
+        r = c.post("/gov/read", {"table": "ccf.modules", "key": MODULE_PATH_1})
         assert r.status_code == http.HTTPStatus.OK, r.status_code
-        assert r.body["js"] == MODULE_CONTENT, r.body
+        assert r.body["js"] == MODULE_CONTENT_1, r.body
 
     LOG.info("Member makes a module remove proposal")
-    proposal_body, _ = ccf.proposal_generator.remove_module("foo.js")
+    proposal_body, _ = ccf.proposal_generator.remove_module(MODULE_PATH_1)
     proposal = network.consortium.get_any_active_member().propose(
         primary, proposal_body
     )
@@ -64,7 +81,7 @@ def test_module_set_and_remove(network, args):
     with primary.client(
         f"member{network.consortium.get_any_active_member().member_id}"
     ) as c:
-        r = c.post("/gov/read", {"table": "ccf.modules", "key": "foo.js"})
+        r = c.post("/gov/read", {"table": "ccf.modules", "key": MODULE_PATH_1})
         assert r.status_code == http.HTTPStatus.BAD_REQUEST, r.status_code
     return network
 
@@ -73,15 +90,9 @@ def test_module_set_and_remove(network, args):
 def test_module_import(network, args):
     primary, _ = network.find_nodes()
 
-    # Add module
-    with tempfile.NamedTemporaryFile("w") as f:
-        f.write(MODULE_CONTENT)
-        f.flush()
-        proposal_body, _ = ccf.proposal_generator.set_module("foo.js", f.name)
-    proposal = network.consortium.get_any_active_member().propose(
-        primary, proposal_body
-    )
-    network.consortium.vote_using_majority(primary, proposal)
+    # Add modules
+    make_module_set_proposal(MODULE_PATH_1, MODULE_CONTENT_1, network)
+    make_module_set_proposal(MODULE_PATH_2, MODULE_CONTENT_2, network)
 
     # Update JS app which imports module
     with tempfile.NamedTemporaryFile("w") as f:
@@ -92,7 +103,7 @@ def test_module_import(network, args):
     with primary.client("user0") as c:
         r = c.post("/app/test_module", {})
         assert r.status_code == http.HTTPStatus.OK, r.status_code
-        assert r.body == MODULE_RETURN
+        assert r.body == MODULE_RETURN_1
 
     return network