Problem
The remote API and its OpenAPI document disagree in several places, and the client has gaps a caller cannot see: a documented size cap not applied on one path, a permission the document does not name, and unbounded reads described as bounded. Nothing here is high severity on its own; together they are a published contract that does not hold, which a beta would freeze.
17 findings were reported: 10 med, 7 low. Two did not survive verification and are answered below rather than fixed. One further defect, not among the 17, is fixed here as well: the OpenAPI document was not valid YAML.
The rest
One line each: what it is, and where. The failing scenario and the fix for each are carried by its own commit.
- med NOT A DEFECT.
listInstances()promises "newest first" in three published places, and it delivers it:EntityStorageBase::loadMultiple()re-keys its result to the IDs it was passed, so the query's sort reaches the caller whatever the entity static cache held. The code and the documents are both right. What was missing is a test, since the one pagination test compares canonically and so passes on any order. That pin is added.modules/orchestra_api/src/LocalOrchestraClient.php:152-178 - med Neither HTTP call restricts redirects. On the token request that leaks the client secret, which travels in the request body: a 307 preserves the body, so a same-host redirect downgrading the scheme sends the secret in clear text. On an API call it is an integrity problem rather than a leak, because Guzzle strips the Authorization header across origins and a scheme change counts as cross-origin, but the redirect target's body still comes back as the answer. Both now refuse redirects.
modules/orchestra_client/src/RemoteTokenProvider.php:111-117,modules/orchestra_client/src/RemoteOrchestraClient.php:133-147 - med The documented 64 KB variable cap is enforced only in
setVariable();startProcess()seeds variables straight into the engine, so a start request with a 10 MB value returns 201 and stores it. The sibling guard on reserved names is mirrored on both paths.modules/orchestra_api/src/LocalOrchestraClient.php:59-74against:217-244 - med The published documents name only the read permission; every write endpoint requires a second, separate permission, and no path documents a 403. An integrator provisioned exactly as the document says gets 403 on start, signal and set-variable with nothing pointing at the cause.
docs/openapi.yaml:29-31and:52-53,docs/distributed.md:38-59,modules/orchestra_server_api/orchestra_server_api.routing.yml:7,37,57 - med
getInstanceis documented as returning "live tokens" and returns every token ever created on the instance, unbounded.modules/orchestra_api/src/LocalOrchestraClient.php:102-112,docs/openapi.yaml:176 - med A variable name longer than 255 characters reaches storage unvalidated and yields a 500 rather than a 4xx.
modules/orchestra_api/src/LocalOrchestraClient.php:217-244,src/Entity/Variable.php:58-60,modules/orchestra_server_api/src/Controller/ApiController.php:167-171 - med The OpenAPI omits both limits
setVariableactually enforces: the 64 KB cap and the reserved name prefix.docs/openapi.yaml:243-267againstmodules/orchestra_api/src/LocalOrchestraClient.php:222-236 - med An empty variable map serializes as a JSON array rather than an object, so a typed client decoding into a map breaks on the empty case.
modules/orchestra_server_api/src/Controller/ApiController.php:141,docs/openapi.yaml:224-230 - med Changing the client id or the base URL leaves the previous consumer's bearer token cached, so the site keeps acting as the old consumer, and its tenant, for up to 24 hours.
modules/orchestra_client/src/Form/SettingsForm.php:81-88,modules/orchestra_client/src/RemoteTokenProvider.php:31,72-80 - med A malformed JSON body on start is silently read as "no variables" and returns 201.
modules/orchestra_server_api/src/Controller/ApiController.php:181-188and:46-48 - med
docs/openapi.yamlis not valid YAML, which none of the 17 findings covered because nothing read the file. Six lines put an unquoted comma-bearing scalar inside a flow mapping: Symfony's parser refuses the document outright, and a lenient parser splits on each comma, truncating a description and turning the rest of the sentence into three invented schema keys on that property.docs/openapi.yaml - low The remote adapter validates only the start response's shape; the four read methods return whatever decoded.
modules/orchestra_client/src/RemoteOrchestraClient.php:79-104,184-187 - low An internal issue label is baked into a shipped test's docblocks.
modules/orchestra_server_api/tests/src/Functional/ApiRouteAccessTest.php:25and:126 - low The permission-denial test claims to cover every endpoint but silently skips all three write endpoints. Same file,
:100-120 - low The two page-size numbers the OpenAPI publishes as contract, a default of 50 and a maximum of 200, are pinned by no test.
modules/orchestra_api/src/LocalOrchestraClient.php:28-29,docs/openapi.yaml:148-154 - low A non-integer limit is silently read as one row rather than rejected.
modules/orchestra_server_api/src/Controller/ApiController.php:78,modules/orchestra_api/src/LocalOrchestraClient.php:165 - low NOT A DEFECT. The 422 branch on
getVariablesis reachable: the controller depends onOrchestraClientInterface, and the remote implementation maps a transport failure, or a non-2xx from the far side, ontoOrchestraClientException. Removing the branch would turn that into a 500. What is wrong is the interface, which declared no@throwsongetInstance(),listInstances()orgetVariables()although every implementation of them can raise, and that is what makes the branch read as dead. The three now declare it.modules/orchestra_server_api/src/Controller/ApiController.php:143-145 - low The distributed guide's contract table omits that
listInstancesreturns one bounded page.docs/distributed.md:31
Note on the API surface
getInstance() now returns only the live tokens of an instance, so a caller reading terminal ones sees fewer. That is what the OpenAPI document and the interface docblock always described, and it is the one behavioural change here that a consumer of the @api interface could notice.
Remaining tasks
None.
This issue summary was drafted with the assistance of an AI agent (Claude). The analysis and the wording were reviewed by me before posting, and accountability for the content is mine.
Issue fork orchestra-3620616
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
mably commentedComment #4
mably commentedComment #6
mably commented