Fail on bad check job job IDs

JobRunner/callback_server.py

@@ -184,6 +184,7 @@ def _handle_submit(app, module, method, data, token):
     _check_rpc_token(app, token)
     job_id = str(uuid.uuid4())
     data["method"] = "%s.%s" % (module, method)
+    outputs[job_id] = {"finished": 0}
     app.config["out_q"].put(["submit", job_id, data])
     app.config["jobcount"] += 1
     return {"result": [job_id]}
@@ -196,15 +197,33 @@ def _error(errstr, trace=None, code=-32000):
     return {"error": err}
 
 
+def _is_uuid(putative: str) -> bool:
+    """
+    Return True if `putative` is a valid UUID string, False otherwise.
+    """
+    try:
+        uuid.UUID(putative)
+    except ValueError:
+        return False
+    return True
+
+
 def _handle_checkjob(app, data):
-    if "params" not in data:
-        raise SanicException(status_code=404)
+    if (not data.get("params")
+        or not isinstance(data["params"], list)
+        or len(data["params"]) != 1
+        or not isinstance(data["params"][0], str)
+    ):
+        return _error("method params must be a list containing exactly one job ID string")
     job_id = data["params"][0]
+    if not _is_uuid(job_id):
+        return _error(f"Invalid job ID: {data['params'][0]}")
     _check_finished(app, f"Checkjob for {job_id}")
-    resp = {"finished": 0}
+    if job_id not in outputs:
+        return _error(f"No such job ID: {job_id}")
 
-    if job_id in outputs:
-        resp = outputs[job_id]
+    resp = outputs[job_id]
+    if resp.get("finished") != 0:
         resp["finished"] = 1
         if "error" in resp:
             return resp
@@ -246,7 +265,7 @@ async def _process_rpc(app, data, token):
         try:
             while True:
                 _check_finished(app, f'synk check for {data["method"]} for {job_id}')
-                if job_id in outputs:
+                if outputs[job_id].get("finished") != 0:
                     resp = outputs[job_id]
                     resp["finished"] = 1
                     return resp

test/test_callback_server_integration.py

@@ -8,11 +8,12 @@
 import os
 import pytest
 import requests
+import socket
 import time
 from typing import Any
+import uuid
 
 from JobRunner.Callback import Callback
-import socket
 
 # NOTE - the CBS is started once for this module. Don't assume it has any particular provenance
 # stored.
@@ -509,3 +510,46 @@ def test_submit_fail_max_jobs_limit(callback_ports):
        "id": "callback",
        "version": "1.1"
     }
+
+
+def test_check_job_fail_no_params(callback_ports):
+    port = callback_ports[0]
+    err = "method params must be a list containing exactly one job ID string"
+
+    resp = _post(port, {"method": "foo._check_job"})
+    j = resp.json()
+    assert j == {
+        "error": {
+            "code": -32000,
+            "name": "CallbackServerError",
+            "message": err,
+        },
+    }
+
+
+def test_check_job_fail_bad_params(callback_ports):
+    port = callback_ports[0]
+    err = "method params must be a list containing exactly one job ID string"
+    randomuuid = str(uuid.uuid4())
+
+    testset = [
+        (None, err),
+        ({}, err),
+        ("foo", err),
+        ([], err),
+        (["foo", "bar"], err),
+        ([[]], err),
+        ([{}], err),
+        ([randomuuid], f"No such job ID: {randomuuid}"),
+        ([randomuuid + "g"], f"Invalid job ID: {randomuuid}g"),
+    ]
+    for t, e in testset:
+        resp = _post(port, {"method": "foo._check_job", "params": t})
+        j = resp.json()
+        assert j == {
+            "error": {
+                "code": -32000,
+                "name": "CallbackServerError",
+                "message": e,
+            },
+        }