Fast-fail oversize SA key/value and operation names at request boundary

durable-workflow-ops · durable-workflow-ops · commit fc1e2ea31a7b · 2026-04-21T00:38:35.000Z
Closes the last remaining gaps in #462 audit:
- signal/update/query name length + control-char validation before DB
- per-key regex + length cap for search_attributes at start
- per-value byte cap for SA string values (and array elements)
- configurable via DW_MAX_OPERATION_NAME_LENGTH,
  DW_MAX_SEARCH_ATTRIBUTE_KEY_LENGTH,
  DW_MAX_SEARCH_ATTRIBUTE_VALUE_BYTES
- /api/cluster/info exposes the new limits so clients can discover them
diff --git a/app/Http/Controllers/Api/HealthController.php b/app/Http/Controllers/Api/HealthController.php
@@ -92,6 +92,9 @@ public function clusterInfo(Request $request): JsonResponse
                 'max_payload_bytes' => (int) config('server.limits.max_payload_bytes', 2 * 1024 * 1024),
                 'max_memo_bytes' => (int) config('server.limits.max_memo_bytes', 256 * 1024),
                 'max_search_attributes' => (int) config('server.limits.max_search_attributes', 100),
+                'max_search_attribute_key_length' => (int) config('server.limits.max_search_attribute_key_length', 128),
+                'max_search_attribute_value_bytes' => (int) config('server.limits.max_search_attribute_value_bytes', 2048),
+                'max_operation_name_length' => (int) config('server.limits.max_operation_name_length', 256),
                 'max_pending_activities' => (int) config('server.limits.max_pending_activities', 2000),
                 'max_pending_children' => (int) config('server.limits.max_pending_children', 2000),
             ],
diff --git a/app/Http/Controllers/Api/WorkflowController.php b/app/Http/Controllers/Api/WorkflowController.php
@@ -149,6 +149,10 @@ public function start(Request $request): JsonResponse
             }
         }
 
+        if (isset($validated['search_attributes'])) {
+            $this->validateSearchAttributes($validated['search_attributes']);
+        }
+
         $workflowId = $validated['workflow_id'] ?? null;
 
         if ($workflowId !== null && $this->workflowIdReservedElsewhere($namespace, $workflowId)) {
@@ -302,6 +306,8 @@ public function signal(Request $request, string $workflowId, string $signalName)
             return $response;
         }
 
+        $this->validateOperationName($signalName, 'signal');
+
         $namespace = $request->attributes->get('namespace');
 
         if (! NamespaceWorkflowScope::workflowBound($namespace, $workflowId)) {
@@ -352,6 +358,8 @@ public function query(Request $request, string $workflowId, string $queryName):
             return $response;
         }
 
+        $this->validateOperationName($queryName, 'query');
+
         $namespace = $request->attributes->get('namespace');
 
         if (! NamespaceWorkflowScope::workflowBound($namespace, $workflowId)) {
@@ -408,6 +416,8 @@ public function update(Request $request, string $workflowId, string $updateName)
             return $response;
         }
 
+        $this->validateOperationName($updateName, 'update');
+
         $namespace = $request->attributes->get('namespace');
 
         if (! NamespaceWorkflowScope::workflowBound($namespace, $workflowId)) {
@@ -839,4 +849,103 @@ private function rejectLegacyUpdateFields(Request $request): void
             ]);
         }
     }
+
+    /**
+     * Fast-fail oversize or malformed signal/update/query names at the
+     * request boundary so the control plane never dispatches a command
+     * row the downstream path would later reject.
+     */
+    private function validateOperationName(string $name, string $kind): void
+    {
+        $max = (int) config('server.limits.max_operation_name_length', 256);
+        $field = $kind.'_name';
+        $bytes = strlen($name);
+
+        if ($bytes === 0) {
+            throw ValidationException::withMessages([
+                $field => [sprintf('The %s name must not be empty.', $kind)],
+            ]);
+        }
+
+        if ($max > 0 && $bytes > $max) {
+            throw ValidationException::withMessages([
+                $field => [sprintf(
+                    'The %s name exceeds the maximum length of %d bytes.',
+                    $kind,
+                    $max,
+                )],
+            ]);
+        }
+
+        if (preg_match('/[\x00-\x1F\x7F]/', $name) === 1) {
+            throw ValidationException::withMessages([
+                $field => [sprintf(
+                    'The %s name must not contain control characters.',
+                    $kind,
+                )],
+            ]);
+        }
+    }
+
+    /**
+     * Validate each search-attribute key and string value against the
+     * per-key and per-value limits so large or malformed entries are
+     * rejected before being written to the DB or forwarded to the
+     * control plane.
+     *
+     * @param  array<int|string, mixed>  $searchAttributes
+     */
+    private function validateSearchAttributes(array $searchAttributes): void
+    {
+        $maxKeyLength = (int) config('server.limits.max_search_attribute_key_length', 128);
+        $maxValueBytes = (int) config('server.limits.max_search_attribute_value_bytes', 2048);
+        $messages = [];
+
+        foreach ($searchAttributes as $key => $value) {
+            if (! is_string($key) || $key === '') {
+                $messages[] = 'Search attribute keys must be non-empty strings.';
+
+                continue;
+            }
+
+            if ($maxKeyLength > 0 && strlen($key) > $maxKeyLength) {
+                $messages[] = sprintf(
+                    'Search attribute key [%s] exceeds the maximum length of %d bytes.',
+                    $key,
+                    $maxKeyLength,
+                );
+
+                continue;
+            }
+
+            if (preg_match('/^[a-zA-Z][a-zA-Z0-9_]*$/', $key) !== 1) {
+                $messages[] = sprintf(
+                    'Search attribute key [%s] must start with a letter and contain only letters, numbers, and underscores.',
+                    $key,
+                );
+
+                continue;
+            }
+
+            $atoms = is_array($value) ? $value : [$value];
+
+            foreach ($atoms as $atom) {
+                if (is_string($atom) && $maxValueBytes > 0 && strlen($atom) > $maxValueBytes) {
+                    $messages[] = sprintf(
+                        'Search attribute [%s] value exceeds the maximum of %d bytes.',
+                        $key,
+                        $maxValueBytes,
+                    );
+
+                    break;
+                }
+            }
+        }
+
+        if ($messages !== []) {
+            throw ValidationException::withMessages([
+                'search_attributes' => $messages,
+            ]);
+        }
+    }
 }
diff --git a/config/dw-contract.php b/config/dw-contract.php
@@ -325,6 +325,24 @@
             'since' => '2.0.0',
             'legacy' => 'WORKFLOW_MAX_SEARCH_ATTRIBUTES',
         ],
+        'DW_MAX_SEARCH_ATTRIBUTE_KEY_LENGTH' => [
+            'description' => 'Maximum length in bytes for a single search-attribute key at the request boundary.',
+            'default' => '128',
+            'since' => '2.0.0',
+            'legacy' => 'WORKFLOW_MAX_SEARCH_ATTRIBUTE_KEY_LENGTH',
+        ],
+        'DW_MAX_SEARCH_ATTRIBUTE_VALUE_BYTES' => [
+            'description' => 'Maximum size in bytes for a single search-attribute string value at the request boundary.',
+            'default' => '2048',
+            'since' => '2.0.0',
+            'legacy' => 'WORKFLOW_MAX_SEARCH_ATTRIBUTE_VALUE_BYTES',
+        ],
+        'DW_MAX_OPERATION_NAME_LENGTH' => [
+            'description' => 'Maximum length in bytes for a signal, update, or query name accepted at the request boundary.',
+            'default' => '256',
+            'since' => '2.0.0',
+            'legacy' => 'WORKFLOW_MAX_OPERATION_NAME_LENGTH',
+        ],
         'DW_MAX_PENDING_ACTIVITIES' => [
             'description' => 'Maximum pending activities per workflow run before the server rejects a command batch.',
             'default' => '2000',
diff --git a/config/server.php b/config/server.php
@@ -350,6 +350,21 @@
         'max_payload_bytes' => (int) EnvAuditor::env('DW_MAX_PAYLOAD_BYTES', 'WORKFLOW_MAX_PAYLOAD_BYTES', 2 * 1024 * 1024),
         'max_memo_bytes' => (int) EnvAuditor::env('DW_MAX_MEMO_BYTES', 'WORKFLOW_MAX_MEMO_BYTES', 256 * 1024),
         'max_search_attributes' => (int) EnvAuditor::env('DW_MAX_SEARCH_ATTRIBUTES', 'WORKFLOW_MAX_SEARCH_ATTRIBUTES', 100),
+        'max_search_attribute_key_length' => (int) EnvAuditor::env(
+            'DW_MAX_SEARCH_ATTRIBUTE_KEY_LENGTH',
+            'WORKFLOW_MAX_SEARCH_ATTRIBUTE_KEY_LENGTH',
+            128,
+        ),
+        'max_search_attribute_value_bytes' => (int) EnvAuditor::env(
+            'DW_MAX_SEARCH_ATTRIBUTE_VALUE_BYTES',
+            'WORKFLOW_MAX_SEARCH_ATTRIBUTE_VALUE_BYTES',
+            2048,
+        ),
+        'max_operation_name_length' => (int) EnvAuditor::env(
+            'DW_MAX_OPERATION_NAME_LENGTH',
+            'WORKFLOW_MAX_OPERATION_NAME_LENGTH',
+            256,
+        ),
         'max_pending_activities' => (int) EnvAuditor::env('DW_MAX_PENDING_ACTIVITIES', 'WORKFLOW_MAX_PENDING_ACTIVITIES', 2000),
         'max_pending_children' => (int) EnvAuditor::env('DW_MAX_PENDING_CHILDREN', 'WORKFLOW_MAX_PENDING_CHILDREN', 2000),
     ],
diff --git a/tests/Feature/PayloadLimitsTest.php b/tests/Feature/PayloadLimitsTest.php
@@ -314,4 +314,188 @@ public function test_search_attribute_limit_is_scoped_to_namespace(): void
         ], $this->apiHeaders('default'))
             ->assertCreated();
     }
+
+    // ── Per-attribute SA validation at request boundary ─────────────
+
+    public function test_workflow_start_rejects_oversized_search_attribute_value(): void
+    {
+        config(['server.limits.max_search_attribute_value_bytes' => 64]);
+
+        $this->configureWorkflowTypes([
+            ExternalGreetingWorkflow::class,
+        ]);
+
+        $this->postJson('/api/workflows', [
+            'workflow_type' => 'ExternalGreetingWorkflow',
+            'search_attributes' => ['Region' => str_repeat('x', 200)],
+        ], $this->apiHeaders())
+            ->assertStatus(422)
+            ->assertJsonPath(
+                'validation_errors.search_attributes.0',
+                fn (string $msg): bool => str_contains($msg, 'Region') && str_contains($msg, '64'),
+            );
+    }
+
+    public function test_workflow_start_rejects_oversized_search_attribute_key(): void
+    {
+        config(['server.limits.max_search_attribute_key_length' => 8]);
+
+        $this->configureWorkflowTypes([
+            ExternalGreetingWorkflow::class,
+        ]);
+
+        $longKey = str_repeat('K', 32);
+
+        $this->postJson('/api/workflows', [
+            'workflow_type' => 'ExternalGreetingWorkflow',
+            'search_attributes' => [$longKey => 'small'],
+        ], $this->apiHeaders())
+            ->assertStatus(422)
+            ->assertJsonPath(
+                'validation_errors.search_attributes.0',
+                fn (string $msg): bool => str_contains($msg, $longKey) && str_contains($msg, '8'),
+            );
+    }
+
+    public function test_workflow_start_rejects_malformed_search_attribute_key(): void
+    {
+        $this->configureWorkflowTypes([
+            ExternalGreetingWorkflow::class,
+        ]);
+
+        $this->postJson('/api/workflows', [
+            'workflow_type' => 'ExternalGreetingWorkflow',
+            'search_attributes' => ['123invalid' => 'small'],
+        ], $this->apiHeaders())
+            ->assertStatus(422)
+            ->assertJsonPath(
+                'validation_errors.search_attributes.0',
+                fn (string $msg): bool => str_contains($msg, '123invalid'),
+            );
+    }
+
+    public function test_workflow_start_rejects_search_attribute_array_with_oversized_element(): void
+    {
+        config(['server.limits.max_search_attribute_value_bytes' => 32]);
+
+        $this->configureWorkflowTypes([
+            ExternalGreetingWorkflow::class,
+        ]);
+
+        $this->postJson('/api/workflows', [
+            'workflow_type' => 'ExternalGreetingWorkflow',
+            'search_attributes' => ['Tags' => ['short', str_repeat('y', 200)]],
+        ], $this->apiHeaders())
+            ->assertStatus(422)
+            ->assertJsonPath(
+                'validation_errors.search_attributes.0',
+                fn (string $msg): bool => str_contains($msg, 'Tags') && str_contains($msg, '32'),
+            );
+    }
+
+    public function test_workflow_start_accepts_valid_search_attributes(): void
+    {
+        $this->configureWorkflowTypes([
+            ExternalGreetingWorkflow::class,
+        ]);
+
+        $this->postJson('/api/workflows', [
+            'workflow_type' => 'ExternalGreetingWorkflow',
+            'search_attributes' => [
+                'Region' => 'us-east-1',
+                'Priority' => 5,
+                'Tags' => ['alpha', 'beta'],
+            ],
+        ], $this->apiHeaders())
+            ->assertSuccessful();
+    }
+
+    // ── Signal / update / query name length validation ─────────────
+
+    public function test_workflow_signal_rejects_oversized_name(): void
+    {
+        config(['server.limits.max_operation_name_length' => 16]);
+
+        $longName = str_repeat('A', 64);
+
+        $this->postJson(
+            "/api/workflows/wf-any/signal/{$longName}",
+            ['input' => []],
+            $this->apiHeaders(),
+        )
+            ->assertStatus(422)
+            ->assertJsonPath(
+                'validation_errors.signal_name.0',
+                fn (string $msg): bool => str_contains($msg, '16'),
+            );
+    }
+
+    public function test_workflow_query_rejects_oversized_name(): void
+    {
+        config(['server.limits.max_operation_name_length' => 16]);
+
+        $longName = str_repeat('Q', 64);
+
+        $this->postJson(
+            "/api/workflows/wf-any/query/{$longName}",
+            ['input' => []],
+            $this->apiHeaders(),
+        )
+            ->assertStatus(422)
+            ->assertJsonPath(
+                'validation_errors.query_name.0',
+                fn (string $msg): bool => str_contains($msg, '16'),
+            );
+    }
+
+    public function test_workflow_update_rejects_oversized_name(): void
+    {
+        config(['server.limits.max_operation_name_length' => 16]);
+
+        $longName = str_repeat('U', 64);
+
+        $this->postJson(
+            "/api/workflows/wf-any/update/{$longName}",
+            ['input' => []],
+            $this->apiHeaders(),
+        )
+            ->assertStatus(422)
+            ->assertJsonPath(
+                'validation_errors.update_name.0',
+                fn (string $msg): bool => str_contains($msg, '16'),
+            );
+    }
+
+    public function test_workflow_signal_rejects_control_characters_in_name(): void
+    {
+        $badName = rawurlencode("my\x01signal");
+
+        $this->postJson(
+            "/api/workflows/wf-any/signal/{$badName}",
+            ['input' => []],
+            $this->apiHeaders(),
+        )
+            ->assertStatus(422)
+            ->assertJsonPath(
+                'validation_errors.signal_name.0',
+                fn (string $msg): bool => str_contains($msg, 'control characters'),
+            );
+    }
+
+    public function test_workflow_signal_name_validation_runs_before_workflow_lookup(): void
+    {
+        // Even when the workflow does not exist, the name-validation
+        // failure must surface as 422 (not 404) so clients learn the
+        // name itself is the problem.
+        config(['server.limits.max_operation_name_length' => 8]);
+
+        $longName = str_repeat('A', 16);
+
+        $this->postJson(
+            "/api/workflows/does-not-exist/signal/{$longName}",
+            ['input' => []],
+            $this->apiHeaders(),
+        )
+            ->assertStatus(422);
+    }
 }
