fix: properly construct concurrency key

[pyranota]

No description provided.

[claude]

Claude finished @pyranota's task in 2m 3s —— View job

---

PR Review: Enforce Workspace Isolation in Custom Concurrency Keys (Cloud Mode)

• Gather context and understand the changes
• Review code quality and best practices
• Check for potential bugs or issues
• Analyze performance considerations
• Evaluate security implications
• Provide feedback with inline comments
• Add reproduction instructions for testing

---

Summary

This PR adds workspace isolation for custom concurrency keys in cloud mode. When running on cloud infrastructure, if a user specifies a custom concurrency key that doesn't include $workspace, the system automatically prefixes it with the workspace ID. This prevents cross-workspace concurrency conflicts where users in different workspaces could inadvertently (or maliciously) share concurrency limits.

What Changed

In insert_concurrency_key():

• In cloud mode (#[cfg(feature = "cloud")]): If the custom key doesn't contain $workspace, prefix the interpolated key with {workspace_id}/
• In non-cloud mode: Behavior unchanged (just returns the interpolated key)

Review Feedback

Positive:

• ✅ The security fix is important for multi-tenant cloud environments
• ✅ The conditional compilation approach is clean and follows existing patterns in the codebase
• ✅ The key format {workspace_id}/{interpolated} is consistent with fullpath_with_workspace
• ✅ The check happens before database insertion, preventing the vulnerability at the right layer

Suggestions (see inline comments):

1. Minor optimization: The x.clone() on line 5361 creates an unnecessary allocation. Consider checking x.contains("$workspace") before calling interpolate_args to avoid the clone.

2. Documentation note: The $workspace check is case-sensitive. This is consistent with how interpolation works, but worth being aware of.

No Major Issues Found

The implementation is correct and addresses the security concern appropriately. The only suggestion is a minor optimization to avoid an unnecessary string clone.

---

Testing Instructions

To verify this change works correctly on cloud deployments:

Navigate to any script or flow settings and configure a custom concurrency key (under Advanced > Concurrency settings). Set a key like my-custom-key without using $workspace. When the script/flow runs, the actual concurrency key stored in the database should be prefixed with your workspace ID (e.g., my-workspace/my-custom-key). You can verify this by checking the concurrency_key table or observing that jobs in different workspaces with the same custom key no longer share concurrency limits. If you use $workspace in your key (e.g., $workspace/my-key), no additional prefix should be added.

[claude]

Minor optimization: The x.clone() here creates an unnecessary allocation. Since interpolate_args takes ownership of the string, we could avoid the clone by checking the condition first.

Suggested change

	let interpolated = interpolate_args(x.clone(), args, workspace_id);
	let has_workspace = x.contains("$workspace");
	let interpolated = interpolate_args(x, args, workspace_id);

Then update line 5366 to use has_workspace instead of x.contains("$workspace").

[cloudflare-workers-and-pages]

Deploying windmill with    Cloudflare Pages

Latest commit:	e39fca5
Status:	✅  Deploy successful!
Preview URL:	https://9dd5761a.windmill.pages.dev
Branch Preview URL:	https://pyra-win-1656-enforce-worksp.windmill.pages.dev

View logs

[claude]

Consistency: This format "{workspace_id}/{interpolated}" is consistent with how fullpath_with_workspace constructs keys (line 3357), which uses "{workspace_id}/{type}/{path}". Good choice for consistency in the key namespace.