Skip to content

Conversation

@claude
Copy link
Contributor

@claude claude bot commented Dec 29, 2025

Summary

  • Add check_tag_available_for_workspace_internal call when pushing jobs from worker_flow as flow substeps
  • This prevents abuse by ensuring tags used in flow substeps are validated against workspace permissions
  • Add test test_flow_substep_tag_availability_check to validate that flows fail when using tags not available for the workspace

Test plan

  • Added test test_flow_substep_tag_availability_check that:
    • Sets up a restricted tag only available to other-workspace (not test-workspace)
    • Creates a flow with a substep using the restricted tag
    • Verifies the flow fails with an appropriate error
  • cargo check -p windmill-worker passes
  • cargo check -p windmill --features deno_core --test worker passes

Closes #7467

🤖 Generated with Claude Code

Add check_tag_available_for_workspace_internal call when pushing jobs
from worker_flow as flow substeps. This prevents abuse by ensuring
tags used in flow substeps are validated against workspace permissions.

Also adds a test to validate that flows fail when using tags that
are not available for the workspace.

Closes #7467

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com>
@cloudflare-workers-and-pages
Copy link

Deploying windmill with  Cloudflare Pages  Cloudflare Pages

Latest commit: a42818b
Status: ✅  Deploy successful!
Preview URL: https://3ec261f5.windmill.pages.dev
Branch Preview URL: https://claude-issue-7467-20251229-1.windmill.pages.dev

View logs

@rubenfiszel rubenfiszel marked this pull request as ready for review December 29, 2025 17:14
@claude
Copy link
Contributor Author

claude bot commented Dec 29, 2025

Claude finished @claude[bot]'s task in 3m 14s —— View job


PR #7468 Review Complete

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

Summary

This PR adds a security check for tag availability when pushing flow substeps. The implementation follows the established pattern used in windmill-queue/src/schedule.rs.

Code Review

✅ Implementation (worker_flow.rs)

The implementation at lines 3357-3371 is correct and follows the established patterns:

// Check tag availability for flow substeps to prevent abuse
if let Some(tag_str) = tag.as_deref().filter(|t| !t.is_empty()) {
    check_tag_available_for_workspace_internal(
        &db,
        &flow_job.workspace_id,
        tag_str,
        email,
        None, // no token for flow substeps so no scopes so no scope_tags
    )
    .warn_after_seconds_with_sql(
        1,
        "check_tag_available_for_workspace_internal".to_string(),
    )
    .await?;
}

Positives:

  • Correctly uses .filter(|t| !t.is_empty()) to avoid checking empty tags
  • Properly uses warn_after_seconds_with_sql for performance monitoring (matching schedule.rs pattern)
  • Uses ? for proper error propagation
  • Passes None for scope_tags since flow substeps don't have token scopes (correctly documented with comment)

✅ Test (worker.rs)

The test at lines 2972-3043 is well-structured:

Positives:

  • Uses SpecificTagType::NoneExcept correctly to restrict the tag to only other-workspace
  • Properly cleans up the custom tags state after the test
  • Verifies both failure condition and error message content

Minor suggestions (non-blocking):

  1. The error message assertion at line 3031 is quite loose - it accepts either "restricted-tag" or just "tag" in the message. This could pass even if the error message changes significantly.
  2. Consider using RAII pattern for cleanup (though the current approach works fine for tests).

⚠️ CI Status

The CI failure is not related to this PR. The failure is a git authentication issue during checkout:

fatal: could not read Username for 'https://github.com': terminal prompts disabled

This is an infrastructure issue with the CI runner, not a code problem.

Security Implications

This change improves security by preventing users from bypassing workspace tag restrictions through flow substeps. Previously, a user could potentially use restricted tags in flow step definitions even when those tags weren't available to their workspace. Now the check happens at runtime when the substep job is pushed.

Performance Considerations

The check_tag_available_for_workspace_internal call is appropriately wrapped with warn_after_seconds_with_sql(1, ...) which will log a warning if the check takes longer than 1 second. This is consistent with how the function is called in schedule.rs.

Testing Instructions

To verify the tag availability check for flow substeps:

  1. Navigate to the Instance Settings > Core > Custom Tags section
  2. Add a specific tag (e.g., "restricted-tag") and configure it with "None except" rule, specifying a workspace other than your current one
  3. Create a new Flow in your workspace
  4. Add a Script step and in the step configuration, set the Tag field to "restricted-tag"
  5. Run the flow - it should fail with an error message indicating the tag is not available to the workspace

@rubenfiszel rubenfiszel merged commit bfe35a8 into main Dec 29, 2025
17 of 20 checks passed
@rubenfiszel rubenfiszel deleted the claude/issue-7467-20251229-1654 branch December 29, 2025 17:24
@github-actions github-actions bot locked and limited conversation to collaborators Dec 29, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: check tag available for flow substeps

2 participants