Skip to content

fix(api): enforce ownership check for conversation delete#32686

Merged
laipz8200 merged 1 commit intomainfrom
check-delete-conv-auth
Mar 1, 2026
Merged

fix(api): enforce ownership check for conversation delete#32686
laipz8200 merged 1 commit intomainfrom
check-delete-conv-auth

Conversation

@laipz8200
Copy link
Copy Markdown
Member

@laipz8200 laipz8200 commented Feb 27, 2026

Important

  1. Make sure you have read our contribution guidelines
  2. Ensure there is an associated issue and you have been assigned to it
  3. Use the correct syntax to link this PR: Fixes #<issue number>.

Summary

Follow-up #23591

  • Fix unauthorized conversation deletion in console installed-app conversations.
  • Root cause: ConversationService.delete deleted by Conversation.id directly, without ownership checks.
  • Restore authorization boundary by resolving the conversation through get_conversation(...) (scoped by app, source, and current user) before deletion.
  • Add regression test covering cross-account delete attempts to ensure it raises ConversationNotExistsError, preserves the conversation row, and does not trigger async cleanup.

Screenshots

Before After
N/A (backend-only security fix) N/A (backend-only security fix)

Checklist

  • This change requires a documentation update, included: Dify Document
  • I understand that this PR may be closed in case there was no previous discussion or issues. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.
  • I ran make lint and make type-check (backend) and cd web && npx lint-staged (frontend) to appease the lint gods

@gemini-code-assist
Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@github-actions
Copy link
Copy Markdown
Contributor

Pyrefly Diff

No changes detected.

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

Pyrefly Diff

No changes detected.

@laipz8200 laipz8200 force-pushed the check-delete-conv-auth branch 2 times, most recently from f684f02 to fb6088c Compare February 27, 2026 07:00
@github-actions
Copy link
Copy Markdown
Contributor

Pyrefly Diff

No changes detected.

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

Pyrefly Diff

No changes detected.

@laipz8200 laipz8200 force-pushed the check-delete-conv-auth branch from fb6088c to a501643 Compare February 27, 2026 07:27
@laipz8200 laipz8200 marked this pull request as ready for review February 27, 2026 09:47
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Feb 27, 2026
@laipz8200 laipz8200 force-pushed the check-delete-conv-auth branch from a501643 to 75d2ab4 Compare February 28, 2026 20:59
@github-actions
Copy link
Copy Markdown
Contributor

Pyrefly Diff

No changes detected.

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

Pyrefly Diff

No changes detected.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Mar 1, 2026
@laipz8200 laipz8200 merged commit 53c62fd into main Mar 1, 2026
16 checks passed
@laipz8200 laipz8200 deleted the check-delete-conv-auth branch March 1, 2026 09:53
saumyatalwani pushed a commit to infocusp-fullstack/dify that referenced this pull request Mar 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants