[rmcdaniel]

Fixes #775.

Problem

An update with wait_for: completed holds its request open for the caller's wait_timeout_seconds. That field was validated only as nullable|integer|min:1, both on POST /api/workflows/{id}/update/{name} (and its run-targeted form) and on bridge update_workflow targets. PHP and native had the same gap, so one request could hold a server worker, or a native request task, for as long as it asked.

Change

Both servers now refuse wait_timeout_seconds above 60 with 422 validation_failed ("The wait timeout seconds field must not be greater than 60."). The refusal happens before admission, so no command, update or history is recorded.

• PHP: ControlPlaneRequestContract::UPDATE_WAIT_TIMEOUT_SECONDS_MAX is applied as a max: rule in WorkflowController::update and BridgeAdapterController.

• Native: command_requests::UPDATE_WAIT_TIMEOUT_SECONDS_MAX is applied in the update request rules, both the HTTP pre-check and the admission rules.

• Contract: the control-plane request contract publishes operations.update.fields.wait_timeout_seconds = {minimum: 1, maximum: 60} on both servers. The change is additive, so the contract version stays 1.

• Docs: docs/server-reference.md documents the bound and the behavior when a wait times out (the update continues and reports wait_timed_out: true).

• Update-bound service operations (Security review of 256a5ac, comment 6410282701): an operation whose binding is a workflow update (workflow_update or update_workflow) hands its wait_timeout_seconds to the update admission. The pinned Workflow 2.13.1 passes that value through unchanged. Both service-operation ingresses therefore refuse a wait above 60 for such an operation, with the same validation refusal, before the operation runs:

  • the execute route (ServiceCatalogController::executeOperation);
  • the workflow-task start_service_operation command (WorkerController::authorizeServiceOperationCommands, which runs before admission).
    The shared check is App\Support\ServiceOperationUpdateWait. Other bindings keep the service operation's own 0..86400 contract. Native runs no service operation: it answers the execute route 501 and refuses start_service_operation as unsupported_workflow_command. A native test pins both, and the port guide names the bound for the future native slice.

• Update admission: ServerWorkflowControlPlane::update, which every update admission reaches (the service control plane resolves it after its own catalog lookup), refuses a wait_timeout_seconds above 60 before admitting. The route checks still answer first. This is the boundary a catalog binding changed after those checks cannot pass (Security review of 5f78555). A deterministic regression flips the binding between the server's check and execution: 61 admits no workflow update command, update event or completion wait, and 60 admits one. The raced service call itself can still end durably as a failed call (an audit row), which is fail-closed. With the admission check removed, the raced 61 admits the update.

• Configured default: the effective DW_V2_UPDATE_WAIT_COMPLETION_TIMEOUT_SECONDS, used when the request omits the field, is clamped to 60. The pinned Workflow 2.13.1 takes the setting as given, so PHP clamps it at boot in App\Support\UpdateWaitConfiguration::apply(). Native's config_audit::update_wait_completion_timeout_seconds clamps the same value.

Why 60, and why refuse rather than clamp a caller's value:

• It matches the existing subscribe long-poll maximum.
• It covers the longest wait our own tools use (sample app: 45 s).
• Workflow main has the same 60-second refusal and default clamp in UpdateWaitPolicy (durable-workflow/workflow@2a7511501b). That commit is not released and is not in the pinned 2.13.1 artifact, so this PR does not depend on it.

Tests

• PHP (tests/Feature/WorkflowControlPlaneTest.php, tests/Feature/BridgeAdapterControllerTest.php): 61 is refused on the instance route, the run-targeted route and the bridge, with no update command recorded. 60 is admitted, and the contract field is published.
• PHP (tests/Feature/UpdateWaitConfigurationTest.php): a configured 600 becomes 60, while 1, 25 and 60 are kept. Setting the environment variable to 600 and rebooting the application yields 60.
• PHP counterfactuals:
  • With the max: rule removed, the route tests fail (202 and 200 instead of 422).
  • Without the boot-time clamp, the boot test fails (600 instead of 60).
• Native:
  • Rule unit test: 60 passes, 61 fails with PHP's message.
  • Config reader test: 60, 61 and 600 all read as 60.
  • update_wait_timeout_is_bounded_before_admission: both routes refuse 61, no UpdateAccepted follows, one follows after 60, and the contract field is published.
• Native counterfactual: with the cap removed from the update rules, the rule unit test fails.
• PHP (tests/Feature/ServiceOperationUpdateWaitTest.php): a real catalog operation bound to a live workflow's approve update. On the execute route and on the workflow-task command, 61 is refused with no service call, no update command and no UpdateAccepted, UpdateRejected or ServiceCallStarted event; 60 is admitted with one service call and one update command. A start_workflow-bound operation still takes 600, and 86401 is refused as before.
• PHP counterfactual: without the service-operation check, both 61 cases answer 200 and admit the update.
• Native: service_operations_cannot_carry_an_update_wait_past_the_ceiling checks that the command is refused as unsupported_workflow_command and that the execute route answers 501.

Qualification at 378bb18

The native lines below are from 5f78555; 378bb18 changes PHP only.

• SQLite, all targets: lib 310 and execution 522 passed.
• Execution tests command_requests, updates_ and update_wait: 32/32 on each of PostgreSQL 17, MySQL 8 and MariaDB 10.11.
• Shared fixtures update-validation, state-updates and visibility-current-runs: pass on all four engines at 256a5ac; the successor adds no change they cover.
• PHPUnit (pinned server image, PHP 8.3):
  • ServiceOperationUpdateWaitTest 4/4
  • ExternalWorkflowUpdateRequestIdTest 47/47
  • WorkflowUpdateWorkerRoutingTest 13/13
  • WorkflowUpdateValidationTaskTest 13/13
  • ServiceExecutionRoutesTest 16/16
  • WorkerServiceOperationTest 35/35
  • ServiceCatalogControllerTest 30/30
  • WorkflowWorkerProtocolTest 108/108
  • WorkflowControlPlaneTest 54/54
  • BridgeAdapterControllerTest 9/9
  • ClusterInfoTest 73/73
  • UpdateWaitConfigurationTest 3/3

Pint passes on every touched PHP file. Hosted CI at this head: 60 passed, 1 skipped.

🤖 Generated with Claude Code

[rmcdaniel]

Security review of exact head 256a5ac8279fcca767e6ab06d16c6573d8650315, tree e9081f73fdb6899a878476fba92a66ed52ab02b8: held; not cleared for merge.

The direct instance/run routes, bridge route, PHP configured-default clamp, native request/admission rules, native configured-default clamp, published request contract, and their 60/61 regressions satisfy the named paths. One accepted PHP update alias remains outside the bound.

POST /api/service-endpoints/{endpointName}/services/{serviceName}/operations/{operationName}/execute is an operator-facing runtime route. At this head, ServiceCatalogController::executeOperation() resolves the catalog operation, then permits wait_timeout_seconds through 86,400 and passes it unchanged to ServiceControlPlane::execute():

• server/routes/api.php

  Lines 406 to 414 in 256a5ac

  	// Service-execution surface — runtime call to a registered operation.
  	// The workflowBootstrap gate is included because dispatch may end up
  	// creating a workflow, signal, update, or activity row, and those
  	// surfaces refuse traffic during schema rollout.
  	Route::prefix('service-endpoints')->middleware([$operator, $cpv, $httpControl, $workflowBootstrap, $ns, $namespaceAdmission])->group(function () {
  	Route::post(
  	'/{endpointName}/services/{serviceName}/operations/{operationName}/execute',
  	[ServiceCatalogController::class, 'executeOperation'],
  	);

• server/app/Http/Controllers/Api/ServiceCatalogController.php

  Lines 634 to 705 in 256a5ac

  	$validated = $request->validate([
  	'arguments' => ['nullable'],
  	'payload_codec' => ['nullable', 'string', 'max:191'],
  	'mode_override' => ['nullable', 'string', 'in:'.implode(',', self::OPERATION_MODES)],
  	'wait_for' => ['nullable', 'string', 'in:accepted,completed'],
  	'wait_timeout_seconds' => ['nullable', 'integer', 'min:0', 'max:86400'],
  	'idempotency_key' => ['nullable', 'string', 'max:191'],
  	'caller_namespace' => ['nullable', 'string', 'max:255'],
  	'caller_workflow_instance_id' => ['nullable', 'string', 'max:191'],
  	'caller_workflow_run_id' => ['nullable', 'string', 'max:26'],
  	'target_workflow_instance_id' => ['nullable', 'string', 'max:191'],
  	'target_workflow_run_id' => ['nullable', 'string', 'max:26'],
  	'connection' => ['nullable', 'string', 'max:191'],
  	'queue' => ['nullable', 'string', 'max:191'],
  	'business_key' => ['nullable', 'string', 'max:191'],
  	'labels' => ['nullable', 'array'],
  	'memo' => ['nullable', 'array'],
  	'search_attributes' => ['nullable', 'array'],
  	'duplicate_start_policy' => [
  	'nullable',
  	'string',
  	'in:reject_duplicate,return_existing_active',
  	],
  	]);
  	try {
  	$validated['payload_codec'] = PayloadCodecContract::canonicalize(
  	$validated['payload_codec'] ?? null,
  	);
  	} catch (\InvalidArgumentException $exception) {
  	throw ValidationException::withMessages([
  	'payload_codec' => [$exception->getMessage()],
  	]);
  	}
  	$arguments = $validated['arguments'] ?? null;
  	if (is_string($arguments)) {
  	AvroPayloadEnvelopeResolver::assertSerializedPayload($arguments, 'arguments');
  	} elseif (is_array($arguments) && array_key_exists('codec', $arguments)) {
  	$resolved = AvroPayloadEnvelopeResolver::resolveCommandPayloadWithCodec(
  	$arguments,
  	'arguments',
  	app(NamespaceExternalPayloadStorage::class)->driverFor($this->namespace($request)),
  	true, // Retain the namespace's existing immutable external bytes.
  	);
  	if (! is_string($resolved['payload']) || $resolved['codec'] === null) {
  	throw ValidationException::withMessages([
  	'arguments' => ['The arguments envelope must contain only codec and blob, or codec and external storage.'],
  	]);
  	}
  	$validated['arguments'] = $resolved['payload'];
  	$validated['payload_codec'] = $resolved['codec'];
  	}
  	if (isset($validated['search_attributes'])) {
  	$validated['search_attribute_types'] = $this->searchAttributeValues()->validateForNamespace(
  	$this->namespace($request),
  	$validated['search_attributes'],
  	);
  	}
  	$callerNamespace = $validated['caller_namespace'] ?? $this->namespace($request);
  	$executionOptions = array_filter(
  	[
  	'namespace' => $this->namespace($request),
  	'arguments' => $validated['arguments'] ?? null,
  	'payload_blob' => is_string($validated['arguments'] ?? null) ? $validated['arguments'] : null,
  	'payload_codec' => $validated['payload_codec'] ?? null,
  	'mode_override' => $validated['mode_override'] ?? null,
  	'wait_for' => $validated['wait_for'] ?? null,
  	'wait_timeout_seconds' => $validated['wait_timeout_seconds'] ?? null,

For an update_workflow / workflow-update binding, the actually pinned Workflow 2.13.1 source forwards that explicit value through workflowCommandOptions() into WorkflowControlPlane::update():

• https://github.com/durable-workflow/workflow/blob/b2a3369110b8cbdf060802ff91e475110e66e333/src/V2/Support/DefaultServiceControlPlane.php#L1191-L1225
• https://github.com/durable-workflow/workflow/blob/b2a3369110b8cbdf060802ff91e475110e66e333/src/V2/Support/DefaultServiceControlPlane.php#L1980-L1996

The pinned DefaultWorkflowControlPlane uses the explicit value for completion waits, and pinned requestedTimeoutSeconds() returns every positive integer unchanged. The new application config clamp is therefore not a backstop for this path; it applies only when an explicit request value is absent:

• https://github.com/durable-workflow/workflow/blob/b2a3369110b8cbdf060802ff91e475110e66e333/src/V2/Support/DefaultWorkflowControlPlane.php#L833-L847
• https://github.com/durable-workflow/workflow/blob/b2a3369110b8cbdf060802ff91e475110e66e333/src/V2/Support/UpdateWaitPolicy.php#L76-L96

There is a second form of the same path to audit: PHP workflow-task start_service_operation commands accept a nonnegative wait_timeout_seconds without this ceiling, then synchronously call the same service control plane. A catalog binding can again resolve to workflow update:

• server/app/Support/WorkflowTaskCompletionValidation.php

  Lines 112 to 121 in 256a5ac

  	'commands.*.namespace' => ['nullable', 'string', 'max:191'],
  	'commands.*.caller_namespace' => ['nullable', 'string', 'max:191'],
  	'commands.*.service_call_id' => ['nullable', 'string', 'max:191'],
  	'commands.*.idempotency_key' => ['nullable', 'string', 'max:191'],
  	'commands.*.mode_override' => ['nullable', 'string', 'in:sync,async'],
  	'commands.*.wait_for' => ['nullable', 'string', 'in:accepted,completed'],
  	'commands.*.wait_timeout_seconds' => ['nullable', 'integer', 'min:0'],
  	'commands.*.target_workflow_instance_id' => ['nullable', 'string', 'max:191'],
  	'commands.*.target_workflow_run_id' => ['nullable', 'string', 'max:191'],
  	'commands.*.business_key' => ['nullable', 'string', 'max:191'],

• https://github.com/durable-workflow/workflow/blob/b2a3369110b8cbdf060802ff91e475110e66e333/src/V2/Support/DefaultWorkflowTaskBridge.php#L3511-L3535
• https://github.com/durable-workflow/workflow/blob/b2a3369110b8cbdf060802ff91e475110e66e333/src/V2/Support/DefaultWorkflowTaskBridge.php#L3577-L3603
• https://github.com/durable-workflow/workflow/blob/b2a3369110b8cbdf060802ff91e475110e66e333/src/V2/Support/DefaultWorkflowTaskBridge.php#L6519-L6558

Required successor:

1. Refuse 61 before executing/admitting an update for every service-operation path whose resolved binding is workflow update, including the HTTP execute route and the PHP workflow-task command path. Preserve the generic non-update service-operation timeout contract unless a separate deliberate contract change is made.
2. Add 60/61 regressions for an actual update-bound service operation. Prove 61 returns the canonical validation refusal before ServiceControlPlane::execute() / update admission and records no workflow update command or update event; prove 60 is admitted. Cover the workflow-task command form too.
3. State native non-applicability if native does not yet serve/execute these service-operation paths; otherwise apply the same bound there. Ensure the eventual native service-operation slice cannot reintroduce this alias.
4. Return the successor SHA/tree, complete delta from this head, counterfactual results, and fresh exact-head hosted gates.

Green CI on this head cannot clear the uncovered ingress. The older #776 and #773 activations remain separately held.

[rmcdaniel]

Security re-review of exact successor 5f78555: the direct update-bound service-operation paths are now covered, but this head remains held for an admission-time TOCTOU gap.

Both added guards inspect WorkflowServiceOperation before authoritative execution. The HTTP path subsequently calls pinned DefaultServiceControlPlane::execute(), which resolves the operation again. The worker path checks at WorkerController::authorizeServiceOperationCommands, then later enters its storage transaction and the pinned Workflow 2.13.1 control plane resolves the operation again. A catalog binding change between those reads (or a missing operation created between the worker precheck and execution) can make the guard see non-update/null while authoritative execution sees update_workflow, allowing a wait above 60 into update admission. The worker guard is neither repeated nor locked inside that admission transaction.

Required before Security clearance: enforce the ceiling against the same operation resolution used by authoritative service-call admission/execution, or lock and revalidate the exact catalog chain within the same transaction; preserve the 0..86400 contract for other bindings. Add deterministic regression/counterfactual evidence that a binding change cannot bypass the ceiling. Fresh exact-head hosted gates remain required. The prior 256a5ac hold and this successor hold remain in force.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[rmcdaniel]

Security source re-review of exact head 378bb18a285cfa4bfb8da979f3a67af620163e51, tree 63d9425486a85331d81eaf11d3e7e3f64e05dd85: the prior TOCTOU source hold is resolved. Final Security clearance remains pending terminal hosted gates at this exact head.

• ServerWorkflowControlPlane::update() checks the 60-second ceiling before entering the mutation wrapper or the pinned inner update, after DefaultServiceControlPlane has authoritatively resolved the current service binding.
• The HTTP and worker ingresses already validate/normalize the value as an integer. The pinned UpdateWaitPolicy rejects other timeout shapes, so the defensive cast does not expose a type path to a longer wait.
• The deterministic binding-change regression covers both service ingresses: 61 creates no workflow update command/event or completion wait, while 60 is admitted.

Precision note: in the forced concurrent-catalog race, the service call may already have been admitted under the old binding and then durably end as a failed call with 409. That audit row is fail-closed; it is not a workflow update admission or an unbounded wait. Do not extend the ordinary precheck's “no service call” claim to that raced case.

No further source change is required by this Security review. This does not authorize merge/deploy and does not clear the separate open CodeQL #138 source finding.

[rmcdaniel]

Security final clearance for exact head 378bb18a285cfa4bfb8da979f3a67af620163e51, tree 63d9425486a85331d81eaf11d3e7e3f64e05dd85.

The exact-source TOCTOU review is recorded in the preceding Security comment. The candidate remains byte-identical. Hosted evidence is now terminal:

• GitHub reports 61 checks, with no pending or adverse conclusion.
• Shared Server fixtures run 40288383872 completed successfully at this exact SHA: all 43 jobs completed and none failed.
• PHP feature, repository contract, performance, CodeQL, Sonar, public-boundary, and replay/query topology gates all passed.

The holds on 256a5ac8 and 5f785556 are superseded for this immutable successor. Security accepts #778 for merge under the normal owner/repository rules. Any head change requires applicable source and gate revalidation.

This clearance is source/merge scope only. It does not authorize deployment, does not clear #773 or #776, and does not dismiss the separate pre-existing CodeQL #138 finding now tracked in #779.

[rmcdaniel]

Merged as b0375645fc150739f9c436fc833d69cea46e94c9.

• Tree: 63d9425486a85331d81eaf11d3e7e3f64e05dd85, identical to the Security-cleared head 378bb18a285cfa4bfb8da979f3a67af620163e51 (clearance 6411351308).
• Parents: edc8036debe6113dbf82fb02eef8a0c9514a1e6c (main) and 378bb18a285cfa4bfb8da979f3a67af620163e51.
• Hosted checks at the head: 60 passed, 1 skipped.
• The branch fix/775-update-wait-maximum is deleted, and Update completion wait_timeout_seconds is caller-unbounded #775 is closed by this merge.