fix: validate training progress identifiers early
This commit is contained in:
+28
-1
@@ -54,10 +54,15 @@ def route_request(
|
||||
method: str = "GET",
|
||||
body: Optional[dict] = None,
|
||||
) -> tuple[int, dict]:
|
||||
deps = dependencies or build_api_dependencies()
|
||||
method = method.upper()
|
||||
path_parts = _path_parts(path)
|
||||
|
||||
if _is_training_progress_post(method, path):
|
||||
_validate_progress_identifier_body(body or {})
|
||||
return 404, {"error": "not_found"}
|
||||
|
||||
deps = dependencies or build_api_dependencies()
|
||||
|
||||
if method == "GET" and path == "/health":
|
||||
return 200, deps["health"]()
|
||||
if method == "GET" and path == "/candles":
|
||||
@@ -245,6 +250,10 @@ def _get_single(query: dict, key: str, default: Optional[str] = None) -> Optiona
|
||||
return values[0]
|
||||
|
||||
|
||||
def _is_training_progress_post(method: str, path: str) -> bool:
|
||||
return method == "POST" and path == "/training/progress"
|
||||
|
||||
|
||||
def _path_parts(path: str) -> list[str]:
|
||||
return [part for part in path.strip("/").split("/") if part]
|
||||
|
||||
@@ -269,6 +278,24 @@ def _parse_path_int(value: str, key: str) -> int:
|
||||
return parsed
|
||||
|
||||
|
||||
def _validate_progress_identifier_body(body: dict) -> None:
|
||||
for key in ("attempt_id", "case_id"):
|
||||
if key in body:
|
||||
_parse_body_identifier(body[key], key)
|
||||
|
||||
|
||||
def _parse_body_identifier(value, key: str) -> int:
|
||||
if isinstance(value, bool):
|
||||
raise ValueError(f"invalid_body_field:{key}")
|
||||
try:
|
||||
parsed = int(value)
|
||||
except (TypeError, ValueError) as exc:
|
||||
raise ValueError(f"invalid_body_field:{key}") from exc
|
||||
if parsed < 1:
|
||||
raise ValueError(f"invalid_body_field:{key}")
|
||||
return parsed
|
||||
|
||||
|
||||
def _read_json_body(handler: BaseHTTPRequestHandler) -> dict:
|
||||
try:
|
||||
length = int(handler.headers.get("Content-Length", "0"))
|
||||
|
||||
@@ -51,6 +51,25 @@ class _TrainingRepositoryStub:
|
||||
self.calls.append(("add_task", kwargs))
|
||||
|
||||
|
||||
class _ProgressRepositoryProbe:
|
||||
def __init__(self) -> None:
|
||||
self.calls = []
|
||||
|
||||
def update_progress(self, **kwargs):
|
||||
self.calls.append(("update_progress", kwargs))
|
||||
return {"status": "recorded"}
|
||||
|
||||
|
||||
class _ProgressServiceProbe:
|
||||
def __init__(self, repository: _ProgressRepositoryProbe) -> None:
|
||||
self.repository = repository
|
||||
self.calls = []
|
||||
|
||||
def update_progress(self, body: dict) -> dict:
|
||||
self.calls.append(("update_progress", body))
|
||||
return self.repository.update_progress(**body)
|
||||
|
||||
|
||||
def _training_dependencies(service: TrainingCaseService) -> dict:
|
||||
return {
|
||||
"training_cases": lambda **kwargs: [],
|
||||
@@ -67,6 +86,13 @@ def _training_dependencies(service: TrainingCaseService) -> dict:
|
||||
}
|
||||
|
||||
|
||||
def _progress_dependencies(service: _ProgressServiceProbe) -> dict:
|
||||
return {
|
||||
"training_progress": lambda **kwargs: [],
|
||||
"training_record_progress": lambda body: service.update_progress(body),
|
||||
}
|
||||
|
||||
|
||||
class _HandlerHarness(server.APIRequestHandler):
|
||||
def __init__(self, path: str, dependencies: dict, method: str = "GET", body: bytes = b"") -> None:
|
||||
self.path = path
|
||||
@@ -196,6 +222,53 @@ class TrainingApiErrorContractM9Tests(unittest.TestCase):
|
||||
with self.assertRaisesRegex(ValueError, "invalid_limit_query_param:limit"):
|
||||
server.route_request(path, {"limit": [""]}, dependencies=self.dependencies)
|
||||
|
||||
def test_post_training_progress_rejects_invalid_identifiers_before_side_effects(self) -> None:
|
||||
post_cases = (
|
||||
({"attempt_id": "not-an-id", "case_id": 7}, "invalid_body_field:attempt_id"),
|
||||
({"attempt_id": "", "case_id": 7}, "invalid_body_field:attempt_id"),
|
||||
({"attempt_id": 0, "case_id": 7}, "invalid_body_field:attempt_id"),
|
||||
({"attempt_id": 101, "case_id": "not-an-id"}, "invalid_body_field:case_id"),
|
||||
({"attempt_id": 101, "case_id": ""}, "invalid_body_field:case_id"),
|
||||
({"attempt_id": 101, "case_id": 0}, "invalid_body_field:case_id"),
|
||||
({"attempt_id": "", "case_id": ""}, "invalid_body_field:attempt_id"),
|
||||
)
|
||||
|
||||
for payload, expected_error in post_cases:
|
||||
repository = _ProgressRepositoryProbe()
|
||||
service = _ProgressServiceProbe(repository)
|
||||
dependencies = _progress_dependencies(service)
|
||||
with self.subTest(payload=payload):
|
||||
with self.assertRaises(ValueError) as raised:
|
||||
server.route_request(
|
||||
"/training/progress",
|
||||
{},
|
||||
dependencies=dependencies,
|
||||
method="POST",
|
||||
body=payload,
|
||||
)
|
||||
|
||||
error = str(raised.exception)
|
||||
serialized = json.dumps({"error": error}, sort_keys=True)
|
||||
self.assertEqual(error, expected_error)
|
||||
for detail in PRIVATE_DIAGNOSTIC_TERMS:
|
||||
with self.subTest(detail=detail):
|
||||
self.assertNotIn(detail.lower(), serialized.lower())
|
||||
self.assertEqual(service.calls, [])
|
||||
self.assertEqual(repository.calls, [])
|
||||
|
||||
handler = _HandlerHarness(
|
||||
"/training/progress",
|
||||
dependencies,
|
||||
method="POST",
|
||||
body=json.dumps(payload).encode("utf-8"),
|
||||
)
|
||||
|
||||
handler.dispatch()
|
||||
|
||||
self._assert_public_error(handler, 400, expected_error)
|
||||
self.assertEqual(service.calls, [])
|
||||
self.assertEqual(repository.calls, [])
|
||||
|
||||
def test_missing_and_invalid_attempt_fields_return_public_400(self) -> None:
|
||||
post_cases = (
|
||||
("/training/attempts", {}, "missing_required_json_field:case_id"),
|
||||
|
||||
Reference in New Issue
Block a user