From af7176be127d879d18cee780b879becfff5c8eeb Mon Sep 17 00:00:00 2001 From: Jim Schaff Date: Wed, 23 Sep 2026 13:32:10 -0400 Subject: [PATCH] feat: registry manifest, and refuse unregistered process addresses at submission Strategy decision 5, phase A. compose_api/registry/manifest.yaml is the registry of record: pinned entries with module roots, a curation level and the evidence for it. /simulation/run now reads every composite document in the upload, nested bundle archives included, and refuses any address that is not local:. under an entry at level tested or higher in the registry_env bundle, with a 400 naming each one. Until now nothing was checked: the upload went to the cluster byte for byte, the allow-list was built and ignored, and the container honours local:!module (arbitrary import) and the rest/parallel protocols. Those unsafe forms are refused even under address_policy=warn. Also: an unsupported file suffix is a 400, not a 500; the unused PBAllowList and hardcoded allow_list are removed. Co-Authored-By: Claude Opus 5.5 (1M context) --- CLAUDE.md | 9 + compose_api/api/routers/simulation.py | 49 +++-- .../api/spec/openapi_3_1_0_generated.yaml | 6 + compose_api/common/gateway/utils.py | 22 +-- compose_api/config.py | 6 + compose_api/registry/__init__.py | 0 compose_api/registry/manifest.py | 82 ++++++++ compose_api/registry/manifest.yaml | 39 ++++ compose_api/registry/validation.py | 175 ++++++++++++++++ compose_api/simulation/handlers.py | 4 - compose_api/simulation/models.py | 4 - docs/strategy.md | 2 +- tests/api/test_submission_rejection.py | 55 ++++++ tests/registry/__init__.py | 0 tests/registry/test_manifest.py | 67 +++++++ tests/registry/test_validation.py | 186 ++++++++++++++++++ tests/simulation/test_simulation.py | 38 ---- 17 files changed, 665 insertions(+), 79 deletions(-) create mode 100644 compose_api/registry/__init__.py create mode 100644 compose_api/registry/manifest.py create mode 100644 compose_api/registry/manifest.yaml create mode 100644 compose_api/registry/validation.py create mode 100644 tests/api/test_submission_rejection.py create mode 100644 tests/registry/__init__.py create mode 100644 tests/registry/test_manifest.py create mode 100644 tests/registry/test_validation.py diff --git a/CLAUDE.md b/CLAUDE.md index 88bf21ff..989cc317 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -107,6 +107,15 @@ inline as f-string heredocs in `simulation_service.py` (one for simulation runs, The container definition is generated by `pbest`, hashed (`get_singularity_hash`, md5 of the def file), and that hash is the identity of a `SimulatorVersion` — an unseen hash triggers a container build job before the simulation runs. +**Registry and submission checks.** `compose_api/registry/manifest.yaml` is the registry of record (strategy +decision 5): pinned entries, each with `module_roots`, a curation `level` and `evidence`. `/simulation/run` calls +`registry.validation.validate_submission` before anything is recorded: it opens every composite document in the +upload (nested bundle archives included) and refuses any process address that is not `local:.` under +a registered entry at level `tested` or higher in the `registry_env` bundle, with a 400 listing each address. +Non-`local` protocols and the arbitrary-import `local:!` form are refused even under `address_policy=warn`. The +container would otherwise run whatever it is sent, so this is the only check. Adding a simulator to what the service +runs means a manifest entry *and* the library in `simulation/simulator_registry.json`. + **Job tracking.** An `HpcRun` row links a SLURM job id, a `correlation_id`, and a `JobType` (`SIMULATION` / `BUILD_CONTAINER`). `JobMonitor` updates status two ways: a 5-second polling loop reconciling `squeue`/`sacct` against running `HpcRun` rows, and (only when `hpc_has_messaging` is true) a NATS subscription on diff --git a/compose_api/api/routers/simulation.py b/compose_api/api/routers/simulation.py index 621e366a..4228a565 100644 --- a/compose_api/api/routers/simulation.py +++ b/compose_api/api/routers/simulation.py @@ -3,17 +3,18 @@ from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, UploadFile from compose_api.common.gateway.models import RouterConfig -from compose_api.common.gateway.utils import allow_list, get_simulation_request_from_uploaded_file +from compose_api.common.gateway.utils import get_simulation_request_from_uploaded_file +from compose_api.config import get_settings from compose_api.dependencies import ( get_database_service, get_job_monitor, get_simulation_service, ) +from compose_api.registry.validation import AddressPolicy, SubmissionRejectedError, validate_submission from compose_api.simulation.handlers import ( run_simulation, ) from compose_api.simulation.models import ( - PBAllowList, SimulationExperiment, ) @@ -24,6 +25,12 @@ config = RouterConfig(router=APIRouter(), prefix="/simulation", dependencies=[]) +REJECTED_RESPONSE = { + "description": "The submission cannot run. For a problem with the request itself (empty file, unsupported type, " + "interval out of range) `detail` is a string. For a problem with its contents (unreadable archive, or processes " + "the registry does not allow) `detail` is an object with a `message` and a `violations` list naming each address." +} + @config.router.post( path="/run", @@ -32,6 +39,7 @@ tags=["Simulation"], dependencies=[Depends(get_simulation_service), Depends(get_database_service)], summary="Run a simulation", + responses={400: REJECTED_RESPONSE}, ) async def submit_simulation( background_tasks: BackgroundTasks, @@ -39,6 +47,28 @@ async def submit_simulation( interval_time: float = 1.0, batch_submission: bool = False, ) -> SimulationExperiment: + if interval_time < 0 or interval_time > 1000: + raise HTTPException(status_code=400, detail="Invalid interval time, it has to be between 0 and 1000") + + simulation_request = await get_simulation_request_from_uploaded_file( + uploaded_file=uploaded_file, batch_submission=batch_submission + ) + simulation_request.end_time_point = interval_time + # Checked before anything is recorded or submitted: the container resolves addresses itself, and will import + # whatever module it is told to, so this is the only point a submission is checked. + try: + validate_submission( + simulation_request.request_file_path, + simulation_request.simulation_file_type, + AddressPolicy(get_settings().address_policy), + ) + except SubmissionRejectedError as e: + simulation_request.request_file_path.unlink(missing_ok=True) + raise HTTPException( + status_code=400, + detail={"message": e.message, "violations": [v.model_dump() for v in e.violations]}, + ) from e + sim_service = get_simulation_service() if sim_service is None: logger.error("Simulation service is not initialized") @@ -52,19 +82,6 @@ async def submit_simulation( logger.error("Job Monitor service is not initialized") raise HTTPException(status_code=500, detail="Job Monitor service is not initialized") - # TODO ################################################################################ - # !!! Input validation for Omex file, preferably converting it to an internal type !!!# - ####################################################################################### - logger.warning("NO VALIDATION YET") - # Tmp file for future implementation - if interval_time < 0 or interval_time > 1000: - raise HTTPException(status_code=400, detail="Invalid interval time, it has to be between 0 and 1000") - - simulation_request = await get_simulation_request_from_uploaded_file( - uploaded_file=uploaded_file, batch_submission=batch_submission - ) - simulation_request.end_time_point = interval_time - try: return await run_simulation( simulation_request=simulation_request, @@ -72,8 +89,6 @@ async def submit_simulation( simulation_service_slurm=sim_service, background_tasks=background_tasks, job_monitor=job_monitor, - # TODO: Put/Get actual allow list - pb_allow_list=PBAllowList(allow_list=allow_list), ) except Exception as e: logger.exception("Error running simulation") diff --git a/compose_api/api/spec/openapi_3_1_0_generated.yaml b/compose_api/api/spec/openapi_3_1_0_generated.yaml index b4ba7125..24175fa1 100644 --- a/compose_api/api/spec/openapi_3_1_0_generated.yaml +++ b/compose_api/api/spec/openapi_3_1_0_generated.yaml @@ -125,6 +125,12 @@ paths: application/json: schema: $ref: '#/components/schemas/SimulationExperiment' + '400': + description: The submission cannot run. For a problem with the request itself + (empty file, unsupported type, interval out of range) `detail` is a string. + For a problem with its contents (unreadable archive, or processes the + registry does not allow) `detail` is an object with a `message` and a + `violations` list naming each address. '422': description: Validation Error content: diff --git a/compose_api/common/gateway/utils.py b/compose_api/common/gateway/utils.py index b0734493..0428328c 100644 --- a/compose_api/common/gateway/utils.py +++ b/compose_api/common/gateway/utils.py @@ -55,25 +55,17 @@ async def get_simulation_request_from_uploaded_file( raise HTTPException(status_code=400, detail="Empty uploaded file") suffix = Path(uploaded_file.filename).suffix + try: + file_type = SimulationFileType.get_file_type(suffix) + except ValueError as e: + raise HTTPException( + status_code=400, detail=f"Unsupported file type `{suffix}`; send .omex, .pbg or .sbml" + ) from e with tempfile.NamedTemporaryFile(delete=False, suffix=suffix) as tmp_file: contents = await uploaded_file.read() tmp_file.write(contents) return SimulationRequest( request_file_path=Path(tmp_file.name), - simulation_file_type=SimulationFileType.get_file_type(suffix), + simulation_file_type=file_type, is_batch=batch_submission, ) - - -allow_list = [ - "pypi::git+https://github.com/biosimulators/bspil-basico.git@initial_work", - "pypi::cobra", - "pypi::tellurium", - "pypi::copasi-basico", - "pypi::smoldyn", - "pypi::numpy", - "pypi::matplotlib", - "pypi::scipy", - "pypi::pb_multiscale_actin", - "conda::readdy", -] diff --git a/compose_api/config.py b/compose_api/config.py index a477a588..84658448 100644 --- a/compose_api/config.py +++ b/compose_api/config.py @@ -96,6 +96,12 @@ class Settings(BaseSettings): # behaviour when a download fails, not an error. simulator_image_repository: str = "ghcr.io/biosimulations/registry_env" + # What /simulation/run does with a process address the registry manifest does not allow + # (compose_api/registry/manifest.yaml). "enforce" rejects the submission with a 400; "warn" logs and runs it, + # as an escape hatch if enforcement turns out to block a real user. Unsafe addresses -- non-local protocols + # and the arbitrary-import `local:!` form -- are rejected under either policy. + address_policy: Literal["enforce", "warn"] = "enforce" + @lru_cache def _load_settings() -> Settings: diff --git a/compose_api/registry/__init__.py b/compose_api/registry/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/compose_api/registry/manifest.py b/compose_api/registry/manifest.py new file mode 100644 index 00000000..8514361b --- /dev/null +++ b/compose_api/registry/manifest.py @@ -0,0 +1,82 @@ +"""The simulator registry manifest: the components this service will run, and how far each has been checked. + +The manifest is a versioned file in this package, reviewed by pull request, and read at call time. It is the +registry of record described by strategy decision 5; the database tables that predate it are not consulted. +""" + +import enum +from pathlib import Path + +import yaml +from pydantic import BaseModel, Field, model_validator + +_MANIFEST_PATH = Path(__file__).parent / "manifest.yaml" + +# The bundle the service currently runs every composite in. See strategy decision 5, "registry_env is the first +# bundle, and a stopgap". +SERVICE_BUNDLE = "registry_env" + + +class CurationLevel(enum.StrEnum): + """The curation ladder, lowest first. Order matters: comparisons use the declaration order.""" + + LISTED = "listed" + INSTALLABLE = "installable" + TESTED = "tested" + VALIDATED = "validated" + CURATED = "curated" + + @property + def rank(self) -> int: + return list(CurationLevel).index(self) + + def at_least(self, other: "CurationLevel") -> bool: + return self.rank >= other.rank + + +# The lowest level the service will run on HPC (goal G6). +HPC_MINIMUM_LEVEL = CurationLevel.TESTED + + +class RegistryEntry(BaseModel): + id: str + package: str + source: str + version: str | None = None + commit: str | None = None + module_roots: list[str] = Field(min_length=1) + level: CurationLevel + bundles: list[str] = Field(default_factory=list) + evidence: list[str] = Field(default_factory=list) + reviewer: str | None = None + + @model_validator(mode="after") + def _pinned(self) -> "RegistryEntry": + if self.version is None and self.commit is None: + raise ValueError(f"registry entry {self.id!r} must be pinned by version or commit") + return self + + +class Manifest(BaseModel): + entries: list[RegistryEntry] + + @model_validator(mode="after") + def _unique_ids(self) -> "Manifest": + ids = [entry.id for entry in self.entries] + duplicates = sorted({i for i in ids if ids.count(i) > 1}) + if duplicates: + raise ValueError(f"duplicate registry entry ids: {duplicates}") + return self + + def entry_for_module(self, module: str) -> RegistryEntry | None: + """The entry whose longest module root contains `module`, so a nested root wins over its parent.""" + best: tuple[int, RegistryEntry] | None = None + for entry in self.entries: + for root in entry.module_roots: + if (module == root or module.startswith(root + ".")) and (best is None or len(root) > best[0]): + best = (len(root), entry) + return None if best is None else best[1] + + +def load_manifest(path: Path = _MANIFEST_PATH) -> Manifest: + return Manifest.model_validate(yaml.safe_load(path.read_text())) diff --git a/compose_api/registry/manifest.yaml b/compose_api/registry/manifest.yaml new file mode 100644 index 00000000..20be7670 --- /dev/null +++ b/compose_api/registry/manifest.yaml @@ -0,0 +1,39 @@ +# The simulator registry of record: which process-bigraph components this service will run, pinned, and how far +# each has been checked. Strategy decision 5 (docs/strategy.md) defines the fields and the curation ladder. +# +# Changes land by pull request. Raising an entry's level needs the evidence for the new level in `evidence`. +# An address is accepted on HPC only if its module sits under one of an entry's `module_roots`, that entry is at +# level `tested` or higher, and the entry is part of the bundle the service runs (`registry_env`). + +entries: + - id: pbsim-common + package: pbsim-common + source: https://github.com/biosimulations/biosim-registry + version: "0.1.2" + module_roots: + - pbsim_common + level: tested + bundles: + - registry_env + evidence: + - >- + Grandfathered at tested on 2026-09-23. The gate is this repository's own cluster tests + (tests/simulators/test_copasi.py, tests/simulators/test_tellurium.py, and the phase_cycle.omex fixture), + which run CopasiUTCStep and TelluriumUTCStep inside the built registry_env image, not the package's own + test suite. Phase B of decision 5 replaces this with a computed level. + + - id: pb-multiscale-actin + package: pb-multiscale-actin + source: https://pypi.org/project/pb-multiscale-actin/1.3.1/ + version: "1.3.1" + module_roots: + - pb_multiscale_actin + level: tested + bundles: + - registry_env + evidence: + - >- + Grandfathered at tested on 2026-09-23. The gate is this repository's own cluster test + (tests/simulators/test_readdy.py with the readdy.omex fixture), which runs ReaddyActinMembrane and + SimulariumEmitter inside the built registry_env image, not the package's own test suite. Phase B of + decision 5 replaces this with a computed level. diff --git a/compose_api/registry/validation.py b/compose_api/registry/validation.py new file mode 100644 index 00000000..3a137a14 --- /dev/null +++ b/compose_api/registry/validation.py @@ -0,0 +1,175 @@ +"""Check a submitted composite against the registry before it reaches the cluster (goal G6). + +The container resolves process addresses itself, and will import any module it is told to, so this is the only +place a submission is checked. It reads every composite document in the upload -- in an OMEX archive, including +archives nested in a batch bundle -- and checks every process address against the manifest. +""" + +import enum +import io +import json +import logging +import zipfile +from collections.abc import Iterator +from pathlib import Path +from typing import Any + +from pydantic import BaseModel + +from compose_api.registry.manifest import HPC_MINIMUM_LEVEL, SERVICE_BUNDLE, Manifest, load_manifest +from compose_api.simulation.models import SimulationFileType + +logger = logging.getLogger(__name__) + +_DOCUMENT_SUFFIXES = (".pbg", ".json") +_NESTED_ARCHIVE_SUFFIX = ".omex" +# A bundle nests archives one level deep; anything deeper is not something a client produces. +_MAX_NESTING = 3 + + +class AddressPolicy(enum.StrEnum): + ENFORCE = "enforce" + WARN = "warn" + + +class AddressViolation(BaseModel): + document: str + address: str + reason: str + # Rejected under every policy, because the container would do something unsafe with it. + always_rejected: bool = False + + +class SubmissionRejectedError(Exception): + def __init__(self, message: str, violations: list[AddressViolation] | None = None) -> None: + super().__init__(message) + self.message = message + self.violations = violations or [] + + +def _documents_in_archive(data: bytes, name: str, depth: int = 0) -> Iterator[tuple[str, Any]]: + if depth > _MAX_NESTING: + raise SubmissionRejectedError(f"{name}: archives are nested more than {_MAX_NESTING} deep") + try: + archive = zipfile.ZipFile(io.BytesIO(data)) + except zipfile.BadZipFile as e: + raise SubmissionRejectedError(f"{name} is not a valid OMEX (zip) archive") from e + with archive: + for member in archive.infolist(): + if member.is_dir(): + continue + member_name = f"{name}/{member.filename}" + lowered = member.filename.lower() + if lowered.endswith(_DOCUMENT_SUFFIXES): + yield member_name, _parse_document(archive.read(member), member_name) + elif lowered.endswith(_NESTED_ARCHIVE_SUFFIX): + yield from _documents_in_archive(archive.read(member), member_name, depth + 1) + + +def _parse_document(data: bytes, name: str) -> Any: + try: + return json.loads(data) + except (UnicodeDecodeError, json.JSONDecodeError) as e: + raise SubmissionRejectedError(f"{name} is not a valid JSON composite document") from e + + +def _addresses(node: Any) -> Iterator[Any]: + """Every process address in a document, in string or {protocol, data} form. + + A dict under an `address` key without a `protocol` is a type declaration, such as a quoted default, not an + address the container will resolve, so it is not collected. + """ + if isinstance(node, dict): + for key, value in node.items(): + if key == "address" and (isinstance(value, str) or (isinstance(value, dict) and "protocol" in value)): + yield value + else: + yield from _addresses(value) + elif isinstance(node, list): + for item in node: + yield from _addresses(item) + + +def _split_address(address: Any) -> tuple[str, str | None]: + """(protocol, target) for an address in string or {protocol, data} form; a bare string defaults to `local`.""" + if isinstance(address, dict): + target = address.get("data") + return str(address.get("protocol")), target if isinstance(target, str) else None + if ":" in address: + protocol, target = address.split(":", 1) + return protocol, target + return "local", address + + +def _unsafe_reason(protocol: str, target: str | None) -> str | None: + """Why the container would do something unsafe with this address, if it would.""" + if protocol != "local": + return f"protocol `{protocol}` is not accepted; only `local` runs on the service" + if target is None: + return "address data must be a string naming a registered class" + if target.startswith("!"): + return "the `local:!` form imports arbitrary modules and is never accepted" + return None + + +def _registry_reason(target: str, manifest: Manifest) -> str | None: + """Why the registry does not allow this class on the service, if it does not.""" + module, _, class_name = target.rpartition(".") + if not module or not class_name: + return "short names are ambiguous across packages; use the qualified form `local:.`" + entry = manifest.entry_for_module(module) + if entry is None: + return f"module `{module}` is not provided by any registered simulator" + if not entry.level.at_least(HPC_MINIMUM_LEVEL): + return ( + f"registry entry `{entry.id}` is at level `{entry.level}`; the service runs `{HPC_MINIMUM_LEVEL}` or higher" + ) + if SERVICE_BUNDLE not in entry.bundles: + return f"registry entry `{entry.id}` is not installed in the service image `{SERVICE_BUNDLE}`" + return None + + +def check_address(address: Any, document: str, manifest: Manifest) -> AddressViolation | None: + protocol, target = _split_address(address) + unsafe = _unsafe_reason(protocol, target) + if unsafe is not None or target is None: + reason = unsafe or "address data must be a string naming a registered class" + return AddressViolation(document=document, address=str(address), reason=reason, always_rejected=True) + refused = _registry_reason(target, manifest) + if refused is not None: + return AddressViolation(document=document, address=str(address), reason=refused) + return None + + +def find_violations(path: Path, file_type: SimulationFileType, manifest: Manifest) -> list[AddressViolation]: + """Check every address in a submitted file. Raises SubmissionRejectedError if the file cannot be read.""" + data = path.read_bytes() + match file_type: + case SimulationFileType.OMEX: + documents = list(_documents_in_archive(data, path.name)) + if not documents: + raise SubmissionRejectedError(f"{path.name} contains no composite document (.pbg or .json)") + case SimulationFileType.PBG: + documents = [(path.name, _parse_document(data, path.name))] + case SimulationFileType.SBML: + # A bare model carries no process addresses; the service chooses the simulator. + return [] + return [ + violation + for name, document in documents + for address in _addresses(document) + if (violation := check_address(address, name, manifest)) is not None + ] + + +def validate_submission(path: Path, file_type: SimulationFileType, policy: AddressPolicy) -> list[AddressViolation]: + """Raise SubmissionRejectedError if the submission may not run; return any violations tolerated under `warn`.""" + violations = find_violations(path, file_type, load_manifest()) + if not violations: + return [] + blocking = violations if policy == AddressPolicy.ENFORCE else [v for v in violations if v.always_rejected] + if blocking: + raise SubmissionRejectedError("the composite names processes this service will not run", blocking) + for v in violations: + logger.warning("Accepted under address_policy=warn: %s in %s: %s", v.address, v.document, v.reason) + return violations diff --git a/compose_api/simulation/handlers.py b/compose_api/simulation/handlers.py index 42cdc60f..6fe14157 100644 --- a/compose_api/simulation/handlers.py +++ b/compose_api/simulation/handlers.py @@ -13,7 +13,6 @@ ContainerizationEngine, ) -from compose_api.common.gateway.utils import allow_list from compose_api.db.database_service import DatabaseService from compose_api.db.services.hpc_db import HPCDatabaseService from compose_api.dependencies import ( @@ -28,7 +27,6 @@ HpcRun, JobStatus, JobType, - PBAllowList, RegisteredSimulators, RemoteContainerImage, Simulation, @@ -64,7 +62,6 @@ async def run_simulation( database_service: DatabaseService, simulation_service_slurm: SimulationService, job_monitor: JobMonitor, - pb_allow_list: PBAllowList, background_tasks: BackgroundTasks, ) -> SimulationExperiment: with tempfile.TemporaryDirectory(delete=False) as tmp_dir: @@ -146,7 +143,6 @@ async def run_curated_pbif( database_service=db_service, simulation_service_slurm=sim_service, job_monitor=job_monitor, - pb_allow_list=PBAllowList(allow_list=allow_list), background_tasks=background_tasks, ) except Exception as e: diff --git a/compose_api/simulation/models.py b/compose_api/simulation/models.py index f1e96db5..725f0848 100644 --- a/compose_api/simulation/models.py +++ b/compose_api/simulation/models.py @@ -249,10 +249,6 @@ class SubmittedSimulation(BaseModel): hpc_run: HpcRun | None -class PBAllowList(BaseModel): - allow_list: list[str] - - class SimulationExperiment(BaseModel): simulation_database_id: int simulator_database_id: int diff --git a/docs/strategy.md b/docs/strategy.md index 4393a2d4..bae862b6 100644 --- a/docs/strategy.md +++ b/docs/strategy.md @@ -288,7 +288,7 @@ exists: `viva-basic-processes`' expression step and `spatio-flux`'s count/concen | Phase | What | Gate | |---|---|---| -| A | Manifest schema; ingest the catalog; every wrapper appears at *listed*. Enforce the allow-list the service already receives. | Service rejects unlisted addresses (G6); registry lists every catalog wrapper. | +| A | Manifest schema; ingest the catalog; every wrapper appears at *listed*. Enforce the allow-list the service already receives. | Service rejects unlisted addresses (G6); registry lists every catalog wrapper. **Manifest and enforcement landed 2026-09-23 (`compose_api/registry/`); catalog ingest follows.** | | B | CI that builds each entry's environment and runs its tests; levels 2 and 3 computed, not asserted. | A nightly report of every entry's level, and why. | | C | The per-composite resolver, after digest identity (decision 2). | Two wrappers from different entries run in one composite on HPC. | | D | Container splitting for conflicting components. | One composite with processes in two containers (`A3.4.docker`). | diff --git a/tests/api/test_submission_rejection.py b/tests/api/test_submission_rejection.py new file mode 100644 index 00000000..62e112c9 --- /dev/null +++ b/tests/api/test_submission_rejection.py @@ -0,0 +1,55 @@ +"""/simulation/run refuses what the registry does not allow, with a 400 that names each address (goal G6). + +Rejection happens before any service is consulted, so these need no database, scheduler or cluster. +""" + +import json + +import httpx +import pytest + +from compose_api.config import override_settings + + +def pbg(*addresses: str) -> bytes: + return json.dumps({"state": {f"s{i}": {"_type": "step", "address": a} for i, a in enumerate(addresses)}}).encode() + + +@pytest.mark.asyncio +async def test_unregistered_and_unsafe_addresses_are_a_400_listing_each(http_api_client: httpx.AsyncClient) -> None: + response = await http_api_client.post( + "/simulation/run", + files={"uploaded_file": ("doc.pbg", pbg("local:numpy.Solver", "local:!os.system"), "application/json")}, + ) + assert response.status_code == 400 + detail = response.json()["detail"] + assert {v["address"] for v in detail["violations"]} == {"local:numpy.Solver", "local:!os.system"} + + +@pytest.mark.asyncio +async def test_warn_policy_still_refuses_unsafe_forms(http_api_client: httpx.AsyncClient) -> None: + with override_settings(address_policy="warn"): + response = await http_api_client.post( + "/simulation/run", + files={"uploaded_file": ("doc.pbg", pbg("rest:http://example.org/p"), "application/json")}, + ) + assert response.status_code == 400 + assert response.json()["detail"]["violations"][0]["always_rejected"] is True + + +@pytest.mark.asyncio +async def test_unreadable_archive_is_a_400(http_api_client: httpx.AsyncClient) -> None: + response = await http_api_client.post( + "/simulation/run", files={"uploaded_file": ("x.omex", b"not a zip", "application/zip")} + ) + assert response.status_code == 400 + assert "not a valid OMEX" in response.json()["detail"]["message"] + + +@pytest.mark.asyncio +async def test_unknown_file_type_is_a_400_not_a_500(http_api_client: httpx.AsyncClient) -> None: + response = await http_api_client.post( + "/simulation/run", files={"uploaded_file": ("model.txt", b"hello", "text/plain")} + ) + assert response.status_code == 400 + assert "Unsupported file type" in response.text diff --git a/tests/registry/__init__.py b/tests/registry/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/registry/test_manifest.py b/tests/registry/test_manifest.py new file mode 100644 index 00000000..31bd756b --- /dev/null +++ b/tests/registry/test_manifest.py @@ -0,0 +1,67 @@ +import pytest +from pydantic import ValidationError + +from compose_api.registry.manifest import ( + HPC_MINIMUM_LEVEL, + SERVICE_BUNDLE, + CurationLevel, + Manifest, + RegistryEntry, + load_manifest, +) + + +def entry(entry_id: str, roots: list[str], **fields: object) -> RegistryEntry: + return RegistryEntry.model_validate({ + "id": entry_id, + "package": entry_id, + "source": "https://example.org", + "version": "1.0", + "module_roots": roots, + "level": "listed", + **fields, + }) + + +def test_shipped_manifest_loads_and_every_entry_has_evidence_for_its_level() -> None: + manifest = load_manifest() + assert manifest.entries + for e in manifest.entries: + if e.level.at_least(CurationLevel.INSTALLABLE): + assert e.evidence, f"{e.id} is at {e.level} with no evidence recorded" + + +def test_the_service_bundle_is_runnable() -> None: + runnable = [ + e for e in load_manifest().entries if SERVICE_BUNDLE in e.bundles and e.level.at_least(HPC_MINIMUM_LEVEL) + ] + assert {e.id for e in runnable} >= {"pbsim-common", "pb-multiscale-actin"} + + +def test_levels_are_ordered() -> None: + assert CurationLevel.CURATED.at_least(CurationLevel.TESTED) + assert not CurationLevel.LISTED.at_least(CurationLevel.TESTED) + assert CurationLevel.TESTED.at_least(CurationLevel.TESTED) + + +def test_an_entry_must_be_pinned() -> None: + with pytest.raises(ValidationError, match="must be pinned"): + RegistryEntry.model_validate({ + "id": "x", + "package": "x", + "source": "https://example.org", + "module_roots": ["x"], + "level": "listed", + }) + + +def test_duplicate_ids_are_refused() -> None: + with pytest.raises(ValidationError, match="duplicate"): + Manifest(entries=[entry("a", ["a"]), entry("a", ["b"])]) + + +def test_longest_module_root_wins() -> None: + manifest = Manifest(entries=[entry("outer", ["pkg"]), entry("inner", ["pkg.sub"])]) + assert (found := manifest.entry_for_module("pkg.sub.mod")) is not None and found.id == "inner" + assert (found := manifest.entry_for_module("pkg.other")) is not None and found.id == "outer" + assert manifest.entry_for_module("pkgx.mod") is None diff --git a/tests/registry/test_validation.py b/tests/registry/test_validation.py new file mode 100644 index 00000000..625620ad --- /dev/null +++ b/tests/registry/test_validation.py @@ -0,0 +1,186 @@ +"""What the service accepts and refuses at submission (goal G6), with no cluster and no database.""" + +import io +import json +import zipfile +from pathlib import Path +from typing import Any + +import pytest +from jinja2 import Template + +from compose_api.registry.manifest import CurationLevel, Manifest, RegistryEntry, load_manifest +from compose_api.registry.validation import ( + AddressPolicy, + SubmissionRejectedError, + find_violations, + validate_submission, +) +from compose_api.simulation.models import SimulationFileType + +RESOURCES = Path(__file__).parent.parent / "fixtures" / "resources" +TEMPLATES = Path(__file__).parent.parent.parent / "compose_api" / "api" / "routers" / "templates" +COPASI = "local:pbsim_common.simulators.copasi_process.CopasiUTCStep" + + +def composite(*addresses: Any) -> dict[str, Any]: + return {"state": {f"step_{i}": {"_type": "step", "address": a} for i, a in enumerate(addresses)}} + + +def omex(tmp_path: Path, members: dict[str, bytes], name: str = "experiment.omex") -> Path: + path = tmp_path / name + with zipfile.ZipFile(path, "w") as archive: + for member, data in members.items(): + archive.writestr(member, data) + return path + + +def omex_bytes(members: dict[str, bytes]) -> bytes: + buffer = io.BytesIO() + with zipfile.ZipFile(buffer, "w") as archive: + for member, data in members.items(): + archive.writestr(member, data) + return buffer.getvalue() + + +def document(*addresses: Any) -> bytes: + return json.dumps(composite(*addresses)).encode() + + +@pytest.mark.parametrize("fixture", ["phase_cycle.omex", "readdy.omex"]) +def test_existing_fixtures_are_accepted(fixture: str) -> None: + assert find_violations(RESOURCES / fixture, SimulationFileType.OMEX, load_manifest()) == [] + + +@pytest.mark.parametrize("template", ["copasi.jinja", "tellurium.jinja"]) +def test_curated_templates_are_accepted(tmp_path: Path, template: str) -> None: + rendered = Template((TEMPLATES / template).read_text()).render( + start_time=0, duration=10, end_time=10, num_data_points=11, output_dir="/experiment/output" + ) + path = tmp_path / "curated.pbg" + path.write_text(rendered) + assert find_violations(path, SimulationFileType.PBG, load_manifest()) == [] + + +def test_quoted_address_type_default_is_not_an_address(tmp_path: Path) -> None: + doc = composite(COPASI) + doc["state"]["schema"] = {"address": {"_type": "quote", "_default": "local:time_course"}} + path = tmp_path / "doc.pbg" + path.write_text(json.dumps(doc)) + assert find_violations(path, SimulationFileType.PBG, load_manifest()) == [] + + +@pytest.mark.parametrize( + ("address", "reason", "always"), + [ + ("local:!os.system", "imports arbitrary modules", True), + ("rest:http://example.org/process", "protocol `rest`", True), + ({"protocol": "parallel", "data": COPASI}, "protocol `parallel`", True), + ({"protocol": "local", "data": {"not": "a string"}}, "must be a string", True), + ("local:CopasiUTCStep", "short names", False), + ("CopasiUTCStep", "short names", False), + ("local:numpy.linalg.Solver", "not provided by any registered simulator", False), + ("local:pbsim_commonplace.Step", "not provided by any registered simulator", False), + ], +) +def test_refused_addresses(tmp_path: Path, address: Any, reason: str, always: bool) -> None: + path = tmp_path / "doc.pbg" + path.write_bytes(document(COPASI, address)) + (violation,) = find_violations(path, SimulationFileType.PBG, load_manifest()) + assert reason in violation.reason + assert violation.always_rejected is always + + +def test_entry_below_tested_is_refused(tmp_path: Path) -> None: + manifest = Manifest( + entries=[ + RegistryEntry( + id="viva-example", + package="viva-example", + source="https://github.com/vivarium-collective/viva-example", + commit="0" * 40, + module_roots=["viva_example"], + level=CurationLevel.LISTED, + bundles=["registry_env"], + ) + ] + ) + path = tmp_path / "doc.pbg" + path.write_bytes(document("local:viva_example.processes.Example")) + (violation,) = find_violations(path, SimulationFileType.PBG, manifest) + assert "level `listed`" in violation.reason + + +def test_entry_outside_the_service_bundle_is_refused(tmp_path: Path) -> None: + manifest = Manifest( + entries=[ + RegistryEntry( + id="viva-example", + package="viva-example", + source="https://github.com/vivarium-collective/viva-example", + commit="0" * 40, + module_roots=["viva_example"], + level=CurationLevel.CURATED, + ) + ] + ) + path = tmp_path / "doc.pbg" + path.write_bytes(document("local:viva_example.processes.Example")) + (violation,) = find_violations(path, SimulationFileType.PBG, manifest) + assert "not installed in the service image" in violation.reason + + +def test_bad_address_in_a_nested_bundle_is_found(tmp_path: Path) -> None: + inner_good = omex_bytes({"a.pbg": document(COPASI)}) + inner_bad = omex_bytes({"b.pbg": document("local:!subprocess.run")}) + path = omex(tmp_path, {"omex_0.omex": inner_good, "omex_1.omex": inner_bad}) + (violation,) = find_violations(path, SimulationFileType.OMEX, load_manifest()) + assert violation.document == "experiment.omex/omex_1.omex/b.pbg" + + +def test_every_document_in_an_archive_is_checked(tmp_path: Path) -> None: + """The container runs whichever document it finds first, so all of them must pass.""" + path = omex(tmp_path, {"a.pbg": document(COPASI), "z.json": document("local:evil.Process")}) + (violation,) = find_violations(path, SimulationFileType.OMEX, load_manifest()) + assert violation.document.endswith("z.json") + + +@pytest.mark.parametrize( + ("members", "message"), + [ + ({"model.sbml": b""}, "contains no composite document"), + ({"doc.pbg": b"not json"}, "not a valid JSON composite document"), + ], +) +def test_unreadable_archives_are_rejected(tmp_path: Path, members: dict[str, bytes], message: str) -> None: + with pytest.raises(SubmissionRejectedError, match=message): + find_violations(omex(tmp_path, members), SimulationFileType.OMEX, load_manifest()) + + +def test_not_a_zip_is_rejected(tmp_path: Path) -> None: + path = tmp_path / "experiment.omex" + path.write_bytes(b"plain text") + with pytest.raises(SubmissionRejectedError, match="not a valid OMEX"): + find_violations(path, SimulationFileType.OMEX, load_manifest()) + + +def test_warn_policy_tolerates_registry_refusals_but_not_unsafe_forms(tmp_path: Path) -> None: + unregistered = tmp_path / "unregistered.pbg" + unregistered.write_bytes(document("local:numpy.Solver")) + tolerated = validate_submission(unregistered, SimulationFileType.PBG, AddressPolicy.WARN) + assert len(tolerated) == 1 + + with pytest.raises(SubmissionRejectedError): + validate_submission(unregistered, SimulationFileType.PBG, AddressPolicy.ENFORCE) + + unsafe = tmp_path / "unsafe.pbg" + unsafe.write_bytes(document("local:!os.system")) + with pytest.raises(SubmissionRejectedError) as raised: + validate_submission(unsafe, SimulationFileType.PBG, AddressPolicy.WARN) + assert raised.value.violations[0].always_rejected + + +def test_sbml_carries_no_addresses(tmp_path: Path) -> None: + path = tmp_path / "model.sbml" + path.write_text("") + assert find_violations(path, SimulationFileType.SBML, load_manifest()) == [] diff --git a/tests/simulation/test_simulation.py b/tests/simulation/test_simulation.py index 9286ccbd..6d19819f 100644 --- a/tests/simulation/test_simulation.py +++ b/tests/simulation/test_simulation.py @@ -13,7 +13,6 @@ ContainerizationFileRepr, ) -from compose_api.common.gateway.utils import allow_list from compose_api.common.hpc.models import SlurmJob from compose_api.common.ssh.ssh_service import SSHService from compose_api.db.database_service import DatabaseServiceSQL @@ -27,7 +26,6 @@ from compose_api.simulation.models import ( JobStatus, JobType, - PBAllowList, RemoteContainerImage, SimulationRequest, SimulatorVersion, @@ -198,7 +196,6 @@ async def test_simulate( simulation_service_slurm=simulation_service_slurm, job_monitor=job_monitor, background_tasks=test_bg_tasks, - pb_allow_list=PBAllowList(allow_list=allow_list), ) assert sim_experiement is not None await test_bg_tasks.call_tasks() @@ -234,38 +231,3 @@ async def test_simulate( await ssh_service.scp_download(archive_result, remote_experiment_result) report_csv_file = Path(os.path.join(test_dir, "fixtures/resources/report.csv")) assert_test_sim_results(archive_result, report_csv_file, temp_dir_path, difference_tolerance=1e-4) - - -# @pytest.mark.skipif(len(get_settings().slurm_submit_key_path) == 0, reason="slurm ssh key file not supplied") -# @pytest.mark.asyncio -# async def test_simulator_not_in_allowlist( -# simulation_service_slurm: SimulationServiceHpc, -# database_service: DatabaseServiceSQL, -# simulation_request: SimulationRequest, -# job_monitor: JobMonitor, -# simulator: SimulatorVersion, -# ) -> None: -# # insert the latest commit into the database -# test_bg_tasks = TestBackgroundTask() -# experiement_id = get_experiment_id(simulator, "".join(random.choices(string.hexdigits, k=7))) -# -# simulation = await database_service.get_simulator_db().insert_simulation( -# sim_request=simulation_request, experiment_id=experiement_id, simulator_version=simulator -# ) -# -# print(f"Reimplement allow list in the future: {allow_list}") - -# with pytest.raises(ValueError): -# await handlers.run_simulation( -# simulation_request, -# database_service, -# simulation_service_slurm, -# job_monitor=job_monitor, -# background_tasks=test_bg_tasks, -# pb_allow_list=PBAllowList(allow_list=["pypi:bspil"]), -# ) -# await test_bg_tasks.call_tasks() -# await simulation_service_slurm.submit_simulation_job( -# simulation=simulation, -# experiment_id=experiement_id, -# )