fix: prevent invalid external job input/output group names (#101)

Only allow alphanumeric names and underscores

Author: nfrasser

cryosparc/job.py

@@ -4,6 +4,8 @@
 
 import json
 import math
+import re
+import urllib.parse
 from contextlib import contextmanager
 from io import BytesIO
 from pathlib import PurePath, PurePosixPath
@@ -39,6 +41,12 @@
     from .tools import CryoSPARC
 
 
+GROUP_NAME_PATTERN = r"^[A-Za-z][0-9A-Za-z_]*$"
+"""
+Input and output result groups may only contain, letters, numbers and underscores.
+"""
+
+
 class Job(MongoController[JobDocument]):
     """
     Accessor class to a job in CryoSPARC with ability to load inputs and
@@ -1233,6 +1241,11 @@ def add_input(
             ... )
             "input_micrographs"
         """
+        if name and not re.fullmatch(GROUP_NAME_PATTERN, name):
+            raise ValueError(
+                f'Invalid input name "{name}"; may only contain letters, numbers and underscores, '
+                "and must start with a letter"
+            )
         try:
             self.cs.vis.add_external_job_input(  # type: ignore
                 project_uid=self.project_uid,
@@ -1354,6 +1367,11 @@ def add_output(
             ... )
             "particle_alignments"
         """
+        if name and not re.fullmatch(GROUP_NAME_PATTERN, name):
+            raise ValueError(
+                f'Invalid output name "{name}"; may only contain letters, numbers and underscores, '
+                "and must start with a letter"
+            )
         try:
             self.cs.vis.add_external_job_output(  # type: ignore
                 project_uid=self.project_uid,
@@ -1519,7 +1537,8 @@ def save_output(self, name: str, dataset: Dataset, *, refresh: bool = True):
             >>> job.save_output("picked_particles", particles)
 
         """
-        url = f"/external/projects/{self.project_uid}/jobs/{self.uid}/outputs/{name}/dataset"
+
+        url = f"/external/projects/{self.project_uid}/jobs/{self.uid}/outputs/{urllib.parse.quote_plus(name)}/dataset"
         with make_request(self.cs.vis, url=url, data=dataset.stream(compression="lz4")) as res:
             result = res.read().decode()
             assert res.status >= 200 and res.status < 400, f"Save output failed with message: {result}"

tests/conftest.py

@@ -98,9 +98,13 @@ def shuffle(self):
 # fmt: on
 
 
-def request_callback_core(request, uri, response_headers):
-    body = json.loads(request.body)
-    procs = {
+@pytest.fixture
+def mock_jsonrpc_procs_core() -> Dict[str, Any]:
+    """
+    Dictionary of JSON RPC method names and their return values. Can override
+    existing values in subfixtures.
+    """
+    return {
         "hello_world": {"hello": "world"},
         "get_running_version": "develop",
         "get_id_by_email_password": "6372a35e821ed2b71d9fe4e3",
@@ -253,17 +257,36 @@ def request_callback_core(request, uri, response_headers):
         "job_connect_group": True,
         "job_set_param": True,
     }
-    procs["system.describe"] = {"procs": [{"name": m} for m in procs]}
-    response_headers["content-type"] = "application/json"
-    return [200, response_headers, json.dumps({"result": procs[body["method"]]})]
 
 
-def request_callback_vis(request, uri, response_headers):
-    body = json.loads(request.body)
-    procs: Dict[str, Any] = {"hello_world": {"hello": "world"}}
-    procs["system.describe"] = {"procs": [{"name": m} for m in procs]}
-    response_headers["content-type"] = "application/json"
-    return [200, response_headers, json.dumps({"result": procs[body["method"]]})]
+@pytest.fixture
+def request_callback_core(mock_jsonrpc_procs_core):
+    def request_callback_core_fn(request, uri, response_headers):
+        body = json.loads(request.body)
+        mock_jsonrpc_procs_core["system.describe"] = {"procs": [{"name": m} for m in mock_jsonrpc_procs_core]}
+        response_headers["content-type"] = "application/json"
+        return [200, response_headers, json.dumps({"result": mock_jsonrpc_procs_core[body["method"]]})]
+
+    return request_callback_core_fn
+
+
+@pytest.fixture
+def mock_jsonrpc_procs_vis() -> Dict[str, Any]:
+    return {
+        "hello_world": {"hello": "world"},
+    }
+
+
+@pytest.fixture
+def request_callback_vis(mock_jsonrpc_procs_vis):
+    def request_callback_vis_fn(request, uri, response_headers):
+        body = json.loads(request.body)
+
+        mock_jsonrpc_procs_vis["system.describe"] = {"procs": [{"name": m} for m in mock_jsonrpc_procs_vis]}
+        response_headers["content-type"] = "application/json"
+        return [200, response_headers, json.dumps({"result": mock_jsonrpc_procs_vis[body["method"]]})]
+
+    return request_callback_vis_fn
 
 
 def request_callback_vis_get_project_file(request, uri, response_headers):
@@ -404,7 +427,7 @@ def t20s_particles_passthrough():
 
 
 @pytest.fixture
-def cs():
+def cs(request_callback_core, request_callback_vis):
     httpretty.enable(verbose=False, allow_net_connect=False)
     httpretty.register_uri(httpretty.POST, "http://localhost:39002/api", body=request_callback_core)  # type: ignore
     httpretty.register_uri(httpretty.POST, "http://localhost:39003/api", body=request_callback_vis)  # type: ignore

tests/test_job.py

@@ -4,15 +4,60 @@
 import pytest
 
 from cryosparc.dataset import Dataset
-from cryosparc.job import Job
+from cryosparc.job import ExternalJob, Job
 from cryosparc.project import Project
+from cryosparc.tools import CryoSPARC
+
+from .conftest import T20S_PARTICLES
 
 
 @pytest.fixture
 def job(cs, project: Project):
     return project.find_job("J1")
 
 
+@pytest.fixture
+def mock_external_job_doc():
+    return {
+        "_id": "67292e95282b26b45d0e8fee",
+        "uid": "J2",
+        "uid_num": 2,
+        "project_uid": "P1",
+        "project_uid_num": 1,
+        "type": "snowflake",
+        "job_type": "snowflake",
+        "title": "Recenter Particles",
+        "description": "Enter a description.",
+        "status": "building",
+        "created_at": "Mon, 04 Nov 2024 20:29:09 GMT",
+        "created_by_user_id": "61f0383552d791f286b796ef",
+        "parents": [],
+        "children": [],
+        "input_slot_groups": [],
+        "output_result_groups": [],
+        "output_results": [],
+        "params_base": {},
+        "params_spec": {},
+        "params_secs": {},
+        "workspace_uids": ["W1"],
+    }
+
+
+@pytest.fixture
+def external_job(
+    mock_jsonrpc_procs_vis,
+    mock_jsonrpc_procs_core,
+    mock_external_job_doc,
+    cs: CryoSPARC,
+    project: Project,
+):
+    mock_jsonrpc_procs_vis["create_external_job"] = "J2"
+    mock_jsonrpc_procs_core["get_job"] = mock_external_job_doc
+    cs.cli()
+    cs.vis()
+    return project.create_external_job("W1", title="Recenter Particles")
+
+
 def test_queue(job: Job):
     job.queue()
     queue_request = httpretty.latest_requests()[-3]
@@ -104,3 +149,108 @@ def test_job_subprocess_io(job: Job):
     opt1 = {"project_uid": "P1", "job_uid": "J1", "message": "error", "error": False}
     opt2 = {"project_uid": "P1", "job_uid": "J1", "message": "world", "error": False}
     assert params == opt1 or params == opt2
+
+
+def test_create_external_job(cs: CryoSPARC, external_job: ExternalJob):
+    requests = httpretty.latest_requests()
+    create_external_job_request = requests[-3]
+    create_external_job_body = create_external_job_request.parsed_body
+    find_external_job_request = requests[-1]
+    find_external_job_body = find_external_job_request.parsed_body
+
+    assert create_external_job_body["method"] == "create_external_job"
+    assert create_external_job_body["params"] == {
+        "project_uid": "P1",
+        "workspace_uid": "W1",
+        "user": cs.user_id,
+        "title": "Recenter Particles",
+        "desc": None,
+    }
+    assert find_external_job_body["method"] == "get_job"
+    assert find_external_job_body["params"] == ["P1", "J2"]
+
+
+@pytest.fixture
+def external_job_output(mock_jsonrpc_procs_vis, mock_external_job_doc, cs: CryoSPARC, external_job: ExternalJob):
+    mock_external_job_doc["output_result_groups"] = [
+        {
+            "uid": "J2-G1",
+            "type": "particle",
+            "name": "particles",
+            "title": "Particles",
+            "description": "",
+            "contains": [
+                {
+                    "uid": "J2-R1",
+                    "type": "particle.blob",
+                    "group_name": "particles",
+                    "name": "blob",
+                    "passthrough": False,
+                },
+                {
+                    "uid": "J2-R2",
+                    "type": "particle.ctf",
+                    "group_name": "particles",
+                    "name": "ctf",
+                    "passthrough": False,
+                },
+            ],
+            "passthrough": False,
+        }
+    ]
+    mock_external_job_doc["output_results"] = [
+        {
+            "uid": "J2-R1",
+            "type": "particle.blob",
+            "group_name": "particles",
+            "name": "blob",
+            "title": "",
+            "description": "",
+            "min_fields": [["path", "O"], ["idx", "u4"], ["shape", "2u4"], ["psize_A", "f4"], ["sign", "f4"]],
+            "versions": [0],
+            "metafiles": ["J2/particles.cs"],
+            "num_items": [10],
+            "passthrough": False,
+        },
+        {
+            "uid": "J2-R2",
+            "type": "particle.ctf",
+            "group_name": "particles",
+            "name": "ctf",
+            "title": "",
+            "description": "",
+            "min_fields": [["type", "O"], ["exp_group_id", "u4"], ["accel_kv", "f4"], ["cs_mm", "f4"]],
+            "versions": [0],
+            "metafiles": ["J2/particles.cs"],
+            "num_items": [10],
+            "passthrough": False,
+        },
+    ]
+    mock_jsonrpc_procs_vis["add_external_job_output"] = "particles"
+    httpretty.register_uri(
+        httpretty.POST,
+        "http://localhost:39003/external/projects/P1/jobs/J2/outputs/particles/dataset",
+        body='"particles"',
+    )
+
+    cs.vis()
+    external_job.add_output("particle", name="particles", slots=["blob", "ctf"])
+    external_job.save_output("particles", T20S_PARTICLES)
+    return T20S_PARTICLES
+
+
+def test_external_job_output(external_job_output):
+    requests = httpretty.latest_requests()
+    create_output_request = requests[-3]
+    find_external_job_request = requests[-1]
+    find_external_job_body = find_external_job_request.parsed_body
+
+    assert len(external_job_output) > 0
+    assert create_output_request.url == "http://localhost:39003/external/projects/P1/jobs/J2/outputs/particles/dataset"
+    assert find_external_job_body["method"] == "get_job"
+    assert find_external_job_body["params"] == ["P1", "J2"]
+
+
+def test_invalid_external_job_output(external_job):
+    with pytest.raises(ValueError, match="Invalid output name"):
+        external_job.add_output("particle", name="particles/1", slots=["blob", "ctf"])