A code audit surfaced one functional bug, two config-schema gaps, and two low-severity security-hardening items. This issue fixes them together (each small and self-contained) with regression coverage. Written so another contributor can pick it up and verify independently.
1. Bug (functional) - task result ignores result_scope, always written token-scoped
Files: src/WorkflowExecutor.php, methods recordResult() (~line 826) and recordOutcome() (~line 860).
Cause: resultTarget() returns [?string name, VariableScope scope]. Both callers destructured the second element into a variable named "scoped" and used it as a boolean (scoped ? token : NULL). VariableScope is a PHP backed enum, and every enum case is truthy, so the ternary always took the token branch. VariableResolver::setVariable() treats a non-NULL token as a token-scoped write.
Effect: a node configured result_scope: instance had its result variable (and its message/reason variable in recordOutcome) written token-scoped anyway. Token-scoped variables are visible only to the writing branch and its descendants, so the result was invisible to sibling branches and dropped when a synchronizing join re-parented the continuing token. Affects any task recording a result via this path, including the ECA event task (modules/orchestra_eca/src/Plugin/TaskType/EcaEventTask.php) and all outcome-exception results/messages.
Fix: compare the enum explicitly, matching the pattern already correct in resumeParked() (~line 725): scope === VariableScope::Token ? token : NULL. Renamed the destructured var from "scoped" to "scope". No import change needed (same namespace).
2. Config schema - missing interaction settings schema
The node schema in config/schema/orchestra.schema.yml types interaction settings as orchestra.interaction.[%parent.plugin] (~line 258) with no wildcard fallback. Two shipped plugins had no schema, so any workflow using them fails strict config-schema validation (enforced by core in Kernel tests and on config save):
- orchestra.interaction.comment - plugin
modules/orchestra_interaction/src/Plugin/Interaction/CommentInteraction.php, settingsrequired(bool),comment_variable(string). Added tomodules/orchestra_interaction/config/schema/orchestra_interaction.schema.yml. - orchestra.interaction.entity_form - plugin
modules/orchestra_content/src/Plugin/Interaction/EntityInteraction.php, settingsform_operation(string),attachment_key(string). orchestra_content had no config/schema dir; addedmodules/orchestra_content/config/schema/orchestra_content.schema.yml.
3. Security (low) - capability token leak via Referer
File: modules/orchestra_interaction/src/Controller/InteractionController.php, methods step() and signal().
The public dispatcher routes (/interaction/{orchestra_instance} and /interaction/{orchestra_instance}/signal/{outcome}) are gated by a signed capability token in the "token" query arg (see InteractionToken). Since the token sits in the URL, any off-site link/resource on the rendered page would leak it via the Referer header. Added a private withNoReferrer() helper attaching Referrer-Policy: no-referrer to both render-array responses (#attached[http_header]) and redirect responses. Same-site navigation, which the token still authorizes, is unaffected.
4. Security (low) - client base URL allowed cleartext
File: modules/orchestra_client/src/Form/SettingsForm.php.
The base URL was a plain url element, so http:// was accepted; the OAuth client secret and bearer token travel there. Added validateForm() requiring https, excepting loopback hosts (localhost, 127.0.0.1, ::1, *.localhost) for local dev. Validation is form-only, so programmatic config writes (and existing tests) are unaffected.
Reviewed, intentionally not changed
- subprocess retry node setting riding the
orchestra.node_setting.*string fallback is correct: node config is a free map stored as strings by design and cast on read (SubprocessTask::retryLimit does an int cast). Declaring it integer would create a validation failure. - Remote API error messages in
ApiController.phpare a deliberate contract for authenticated, tenant-scoped consumers (not stack traces; 500s already sanitized). - Instance-scoped token 30-day TTL / non-revocation is a documented design trade-off; the Referer hardening addresses the main leak vector.
Tests
Added two Kernel tests to tests/src/Kernel/EngineOutcomeTest.php: testOutcomeResultHonorsInstanceScope() and testOutcomeResultHonorsTokenScope(), asserting the stored variable owning-token column is NULL for instance scope and non-NULL for token scope (new variableTokenId() helper). Pre-existing tests asserted only value-by-name and did not catch the bug. outcomeWorkflow() gained a result_scope param (defaults to instance).
Verify
- EngineOutcomeTest: new tests pass with the fix, the instance-scope one fails if WorkflowExecutor is reverted.
- phpcs (Drupal + DrupalPractice) and php -l clean on all changed files.
Issue fork orchestra-3610442
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 #4
mably commented