Skip to content

fix(runner): 400 for a missing bundle (#119); drop orphaned mass-VM schemas (#121) - #135

Merged
pparage merged 1 commit into
devfrom
fix/resolver-400-and-dead-schemas
Aug 6, 2026
Merged

fix(runner): 400 for a missing bundle (#119); drop orphaned mass-VM schemas (#121)#135
pparage merged 1 commit into
devfrom
fix/resolver-400-and-dead-schemas

Conversation

@pparage

@pparage pparage commented Aug 6, 2026

Copy link
Copy Markdown
Member

Closes #119 and #121.

#119 — a typo'd bundle name returned 500

_resolve_file calls .resolve(strict=True), which raises FileNotFoundError before the if not main_filepath.exists() branch further down could run — so that 400 was dead code, and the docstring's :raises HTTPException: 400 ... or the file does not exist was a lie:

POST /v0/admin/run/bundles/nonexistent/run  ->  500

Pre-existing, but #113 sharpened it: with the hardcoded /core/* routes gone, every bundle invocation goes through the arbitrary-name lookup, where a typo is the normal failure — and a 500 gives the caller no way to tell "you named something that doesn't exist" from "the server broke".

Caught and translated to the 400 the code below was already written for. strict=True is kept deliberately: the traversal check downstream relies on its symlink resolution, so loosening it would change a security-relevant behaviour to fix a status code.

Note on the test: test_accepts_dotted_segment_format asserted FileNotFoundError specifically — that was what made it non-vacuous, since the old regex rejected dots with a 400. Both failures are 400 now, so it asserts on the detail instead: PLAYBOOK NOT FOUND means the name was accepted and only the file was missing, where the old regex would have said INVALID ACTION NAME. Without that distinction the test would pass either way again.

#121 — dead schemas from the vm_ids removal

MassDelete* / MassAction* and their Request_/Reply_ aliases were orphaned when #113 deleted the six vm_ids/* routes. Only tests referenced them, so those go too — along with the stray module-level vm = MassDeleteVmItem binding.

~100 lines of schema, 21 lines of test, no API surface change (the routes were already gone).

488 passed, ruff clean, openapi.json regenerated — and the new drift guard from #134 now enforces that automatically.

#119: _resolve_file called .resolve(strict=True) before the 'not found'
branch it documents, so a typo'd bundle raised an unhandled
FileNotFoundError — a 500 from an endpoint whose docstring promises 400,
and with no way for the caller to tell a bad name from a broken server.
Catch it and raise the 400 the code below was already written for. Kept
strict=True so the symlink semantics the traversal check relies on do
not change.

The dotted-name test asserted FileNotFoundError specifically, which was
what made it non-vacuous; both failures are 400 now, so it asserts on
the detail instead — NOT FOUND means the name was accepted.

#121: removed MassDelete*/MassAction* and their Request_/Reply_ aliases,
orphaned when #113 deleted the vm_ids routes. Only tests referenced
them, so those go too, along with the stray module-level 'vm =
MassDeleteVmItem' binding.
@pparage
pparage merged commit b2db25f into dev Aug 6, 2026
3 checks passed
@pparage
pparage deleted the fix/resolver-400-and-dead-schemas branch August 6, 2026 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant