diff --git a/app/schemas/vms.py b/app/schemas/vms.py index d5b0237..8f1201d 100644 --- a/app/schemas/vms.py +++ b/app/schemas/vms.py @@ -524,106 +524,6 @@ class VmActionReply(BaseModel): } -# --------------------------------------------------------------------------- -# Mass Delete -# --------------------------------------------------------------------------- - - -class MassDeleteVmItem(BaseModel): - id: str = Field(..., description="Virtual machine id", pattern=r"^[0-9]+$") - - name: str = Field( - ..., - description="Virtual machine meta name", - pattern="^[A-Za-z0-9-]+$", # deny void name - # pattern=r"^[A-Za-z0-9-]*$", - ) - - -class MassDeleteRequest(BaseModel): - proxmox_node: str = Field( - ..., - # default= "px-testing", - description="Proxmox node name", - pattern=r"^[A-Za-z0-9-]*$", - ) - - as_json: bool = Field( - default=True, description="If true : JSON output else : raw output" - ) - - vms: List[MassDeleteVmItem] = Field( - ..., - description="List of virtual machine (vm_id + vm_name)", - min_length=1, - ) - - model_config = { - "json_schema_extra": { - "example": { - "proxmox_node": "px-testing", - "as_json": True, - "vms": [ - {"id": "4000", "name": "vuln-box-00"}, - {"id": "4001", "name": "vuln-box-01"}, - {"id": "4002", "name": "vuln-box-02"}, - ], - } - } - } - - -class MassDeleteItemReply(BaseModel): - action: Literal["vm_start", "vm_stop", "vm_resume", "vm_pause", "vm_stop_force"] - source: Literal["proxmox"] - - proxmox_node: str - vm_id: str # int = Field(..., ge=1) - # vm_new_id : str # int = Field(..., ge=1) - vm_name: str - vm_status: Literal["running", "stopped", "paused"] - - -class MassDeleteReply(BaseModel): - rc: int = Field(0, description="RETURN code (0 = OK)") - result: list[MassDeleteItemReply] - - -# --------------------------------------------------------------------------- -# Mass Start / Stop / Resume / Pause -# --------------------------------------------------------------------------- - - -class MassActionRequest(BaseModel): - proxmox_node: str = Field( - ..., - # default= "px-testing", - description="Proxmox node name", - pattern=r"^[A-Za-z0-9-]*$", - ) - - as_json: bool = Field( - default=True, description="If true : JSON output else : raw output" - ) - # - - vm_ids: List[str] = Field( - ..., - description="Virtual machine id", - min_length=1, - ) - - model_config = { - "json_schema_extra": { - "example": { - "proxmox_node": "px-testing", - "as_json": True, - "vm_ids": ["4000", "4001"], - } - } - } - - # --------------------------------------------------------------------------- # Backward compatibility -- old names used by current routes # --------------------------------------------------------------------------- @@ -661,11 +561,4 @@ class MassActionRequest(BaseModel): Reply_ProxmoxVmsVMID_StartStopPauseResumeItem = VmActionItemReply Reply_ProxmoxVmsVMID_StartStopPauseResume = VmActionReply -# vm_ids/mass_delete.py -vm = MassDeleteVmItem -Request_ProxmoxVmsVmIds_MassDelete = MassDeleteRequest -Reply_ProxmoxVmsVMID_MasseDeleteItem = MassDeleteItemReply -Reply_ProxmoxVmsVmIds_MassDelete = MassDeleteReply -# vm_ids/mass_start_stop_resume_pause.py -Request_ProxmoxVmsVmIds_MassStartStopPauseResume = MassActionRequest diff --git a/app/utils/checks_playbooks.py b/app/utils/checks_playbooks.py index fce85af..75c5d51 100644 --- a/app/utils/checks_playbooks.py +++ b/app/utils/checks_playbooks.py @@ -225,10 +225,17 @@ def _resolve_file( # # if not is_init_yaml: - if is_init_yaml is False: - main_filepath = (actions_dir / action_name / "main.yml").resolve(strict=True) - else: - main_filepath = (actions_dir / action_name / "init.yml").resolve(strict=True) + filename = "init.yml" if is_init_yaml else "main.yml" + try: + # strict=True keeps the symlink semantics the traversal check below + # relies on, but it raises before the "not found" branch further down + # could ever run — so a typo'd bundle surfaced as an unhandled + # FileNotFoundError, i.e. a 500 from an endpoint documented as 400. + main_filepath = (actions_dir / action_name / filename).resolve(strict=True) + except FileNotFoundError as e: + err = f":: err - PLAYBOOK NOT FOUND : {action_name}/{filename}" + logger.error(err) + raise HTTPException(status_code=400, detail=err) from e # # checks - attempt to avoid file - path traversal injections + symlinks injections diff --git a/tests/test_checks_playbooks.py b/tests/test_checks_playbooks.py index 5b381d4..4a5eae5 100644 --- a/tests/test_checks_playbooks.py +++ b/tests/test_checks_playbooks.py @@ -65,13 +65,17 @@ def test_accepts_dotted_segment_format(self): """Dotted .. names pass format validation (range42-playbooks#133) and fail only on the missing file. - Asserting FileNotFoundError specifically is what makes this test - meaningful: the old regex rejected dots with HTTPException(400), so - a `raises((HTTPException, FileNotFoundError))` would pass either way. + Both failures are now HTTPException(400), so the assertion has to be + on the *detail*: NOT FOUND means the name was accepted and only the + file was missing, where the old regex would have said INVALID FORMAT. + Without that distinction this test passes either way. """ - with pytest.raises(FileNotFoundError): + with pytest.raises(HTTPException) as exc_info: resolve_bundles_playbook( "generic/systems.baseline.docker_host", "www_app") + assert exc_info.value.status_code == 400 + assert "PLAYBOOK NOT FOUND" in exc_info.value.detail + assert "INVALID ACTION NAME" not in exc_info.value.detail def test_rejects_dot_segment(self): with pytest.raises(HTTPException) as exc_info: diff --git a/tests/test_route_runner.py b/tests/test_route_runner.py index c8f73eb..7fbe652 100644 --- a/tests/test_route_runner.py +++ b/tests/test_route_runner.py @@ -73,3 +73,18 @@ def test_rejects_malformed_names(self, client, name): """ resp = client.post(f"/v0/admin/run/bundles/{name}/run", json=BODY) assert resp.status_code == 400 + + +class TestMissingBundle: + """A typo'd bundle name is a user error, not a server fault (#119).""" + + def test_missing_bundle_is_400_not_500(self, client): + resp = client.post( + "/v0/admin/run/bundles/generic/does.not.exist/run", json=BODY) + assert resp.status_code == 400, resp.text + assert "PLAYBOOK NOT FOUND" in str(resp.json()) + + def test_missing_scenario_is_400_not_500(self, client): + resp = client.post( + "/v0/admin/run/scenarios/no_such_scenario/run", json=BODY) + assert resp.status_code == 400, resp.text diff --git a/tests/test_schemas.py b/tests/test_schemas.py index 5439f6a..9822b7b 100644 --- a/tests/test_schemas.py +++ b/tests/test_schemas.py @@ -87,23 +87,6 @@ def test_vm_clone_request(): assert req.vm_description == "cloned-vm" # default -def test_mass_delete_request(): - from app.schemas.vms import MassDeleteRequest, MassDeleteVmItem - - req = MassDeleteRequest( - proxmox_node="px-testing", - vms=[MassDeleteVmItem(id="4000", name="vuln-box-00")], - ) - assert len(req.vms) == 1 - - -def test_mass_delete_request_rejects_empty_vms(): - from app.schemas.vms import MassDeleteRequest - - with pytest.raises(ValidationError): - MassDeleteRequest(proxmox_node="px-testing", vms=[]) - - # =========================================================================== # vm_config.py # =========================================================================== @@ -313,10 +296,6 @@ def test_backward_compat_aliases_vms(): assert Request_ProxmoxVmsVMID_StartStopPauseResume is VmActionRequest - from app.schemas.vms import MassDeleteRequest, Request_ProxmoxVmsVmIds_MassDelete - - assert Request_ProxmoxVmsVmIds_MassDelete is MassDeleteRequest - def test_backward_compat_aliases_vm_config(): from app.schemas.vm_config import (