Skip to content

fix: permissions check for duplicating workflow#706

Merged
emir-karabeg merged 1 commit intostagingfrom
fix/duplicate-permissions
Jul 16, 2025
Merged

fix: permissions check for duplicating workflow#706
emir-karabeg merged 1 commit intostagingfrom
fix/duplicate-permissions

Conversation

@emir-karabeg
Copy link
Collaborator

@emir-karabeg emir-karabeg commented Jul 16, 2025

Description

Fixed the 404 error for duplicating a workflow where the user is not the owner. Now users with admin or write permissions can duplicate.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Security enhancement
  • Performance improvement
  • Code refactoring (no functional changes)

How Has This Been Tested?

Test in staging.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • All tests pass locally and in CI (bun run test)
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • I have updated version numbers as needed (if needed)
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Security Considerations:

  • My changes do not introduce any new security vulnerabilities
  • I have considered the security implications of my changes

@vercel
Copy link

vercel bot commented Jul 16, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sim ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 16, 2025 8:51pm
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
docs ⬜️ Skipped (Inspect) Jul 16, 2025 8:51pm

greptile-apps[bot]
greptile-apps bot reviewed Jul 16, 2025
View reviewed changes
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Summary

This PR fixes a permission-related issue in the workflow duplication functionality. Previously, only workflow owners could duplicate workflows, which was too restrictive. The changes now allow users with admin or write permissions in a workspace to duplicate workflows within that workspace, even if they don't own them directly.

The key changes include:

  1. Removing the user ID restriction from the initial workflow query
  2. Adding a proper permission check that validates either workflow ownership or workspace permissions
  3. Improving error handling to properly distinguish between 404 (not found) and 403 (access denied) cases

The changes align with the workspace-based permission model and improve the collaborative workflow experience.

PR Description Notes:

  • The PR description has marked all types of changes which seems excessive - this appears to be primarily a bug fix with some security enhancement
  • The testing section could be more detailed about specific test cases

Confidence score: 4/5

  1. This PR is safe to merge with proper testing in staging
  2. The code changes are well-structured with clear permission checks and error handling
  3. The route.ts file needs careful testing to verify all permission scenarios work as expected

1 file reviewed, 1 comment
Edit PR Review Bot Settings | Greptile

apps/sim/app/api/workflows/[id]/duplicate/route.ts
Comment on lines +81 to +83
if (!canAccessSource) {
throw new Error('Source workflow not found or access denied')
}
Copy link
Contributor

@greptile-apps greptile-apps bot Jul 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider moving error message to a const variable at the top of the file since it's referenced in two places (here and error handling)

@delve-auditor
Copy link

delve-auditor bot commented Jul 16, 2025

✅ No security or compliance issues detected. Reviewed everything up to 3c7342a.

Security Overview
  • 🔎 Scanned files: 1 changed file(s)
Detected Code Changes
Change Type Relevant files
Bug Fix ► apps/sim/app/api/workflows/[id]/duplicate/route.ts
    Add proper permissions check for workflow duplication
► apps/sim/app/chat/[subdomain]/chat-client.tsx
    Remove excessive logging
► apps/sim/app/chat/[subdomain]/components/voice-interface/voice-interface.tsx
    Refactor voice interface for improved reliability

Reply to this PR with @delve-auditor followed by a description of what change you want and we'll auto-submit a change to this PR to implement it.

@emir-karabeg emir-karabeg merged commit 4c6c727 into staging Jul 16, 2025
6 checks passed
@waleedlatif1 waleedlatif1 mentioned this pull request Jul 16, 2025
Merged
@emir-karabeg emir-karabeg deleted the fix/duplicate-permissions branch July 16, 2025 23:16
arenadeveloper02 pushed a commit to arenadeveloper02/p2-sim that referenced this pull request Sep 19, 2025
@emir-karabeg
fix: permissions check for duplicating workflow (simstudioai#706)
0eb8e8a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@emir-karabeg