Skip to content

fix: enhance query filtering by including workspace_id in various model lookups#6515

Closed
shaohuzhang1 wants to merge 2 commits into
v2from
pr@v2@fix_email_sm
Closed

fix: enhance query filtering by including workspace_id in various model lookups#6515
shaohuzhang1 wants to merge 2 commits into
v2from
pr@v2@fix_email_sm

Conversation

@shaohuzhang1

Copy link
Copy Markdown
Contributor

fix: enhance query filtering by including workspace_id in various model lookups

Copilot AI review requested due to automatic review settings July 27, 2026 05:04
@shaohuzhang1

Copy link
Copy Markdown
Contributor Author

Seems you are using me but didn't get OPENAI_API_KEY seted in Variables/Secrets for this repo. you could follow readme for more information

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR tightens multi-tenant data access by scoping a variety of lookups and mutations to workspace_id (and, for chat records, to application_id) to avoid cross-workspace/application operations.

Changes:

  • Add workspace_id scoping to batch deletes/edits across triggers, tools, models, folders, applications, and knowledge-related entities (problems/termbase/tags/workflow actions).
  • Add additional permission decorators on select user/workspace endpoints and file deletion endpoint.
  • Scope chat record voting and chat record queries to the authenticated application_id.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
apps/users/views/user.py Adds missing permission checks to workspace user list/member GET endpoints.
apps/trigger/serializers/trigger.py Restricts batch trigger deletion operations to the provided workspace_id.
apps/tools/serializers/tool.py Scopes tool edit/delete/update-store operations to workspace_id and adds URL safety checks.
apps/oss/views/file.py Adds permission gating to file deletion endpoint.
apps/models_provider/serializers/model_serializer.py Scopes model pause/delete/edit operations to workspace_id.
apps/knowledge/serializers/termbase.py Scopes termbase delete/export/one operations to knowledge_id.
apps/knowledge/serializers/tag.py Scopes tag edit/delete/batch-delete operations to knowledge_id.
apps/knowledge/serializers/problem.py Scopes problem deletes/reads to knowledge_id.
apps/knowledge/serializers/knowledge_workflow.py Adds workspace-aware validation for knowledge workflow actions and action operations.
apps/folders/serializers/folder.py Adds workspace-aware folder existence validation and scopes folder reads/updates/deletes.
apps/chat/views/chat_record.py Passes application_id into vote requests for stronger scoping.
apps/chat/serializers/chat_record.py Requires application_id and enforces it in vote lookups.
apps/application/serializers/application.py Scopes application batch delete to workspace_id before deleting related records.
apps/application/serializers/application_chat_record.py Ensures chat record queries are constrained by chat__application_id.
Comments suppressed due to low confidence (2)

apps/knowledge/serializers/tag.py:200

  • BatchDelete.batch_delete() deletes all Tag rows in the knowledge base that match keys_to_delete, but it only deletes DocumentTag rows for the originally provided tag_ids. This can leave orphan DocumentTag rows referencing tags that were deleted (and/or fail to remove document-tag relations for tags removed via key-based deletion). Delete the matching DocumentTag rows for all tags that will be deleted (before deleting the tags).
            # 获取要删除的标签的key
            tags_to_delete = QuerySet(Tag).filter(id__in=tag_ids, knowledge_id=self.data.get('knowledge_id'))
            keys_to_delete = set(tags_to_delete.values_list('key', flat=True))

            # 删除具有相同key的所有标签
            QuerySet(Tag).filter(
                knowledge_id=self.data.get('knowledge_id'),
                key__in=keys_to_delete
            ).delete()

            # 删除关联的DocumentTag
            QuerySet(DocumentTag).filter(tag_id__in=tag_ids).delete()

apps/trigger/serializers/trigger.py:474

  • Batch.batch_delete() filters trigger_id_list by workspace_id, but then re-fetches each trigger by ID only and immediately serializes it. If a trigger is deleted concurrently (or otherwise not found), TriggerModelSerializer(trigger).data will error. Also, adding workspace_id to the per-item fetch keeps the intent consistent and avoids accidentally operating on a trigger from a different workspace if IDs were ever reused/misrouted.
            trigger_id_list = list(
                QuerySet(Trigger)
                .filter(id__in=trigger_id_list, workspace_id=workspace_id)
                .values_list("id", flat=True)
            )
            for trigger_id in trigger_id_list:
                trigger = QuerySet(Trigger).filter(id=trigger_id).first()
                undeploy(TriggerModelSerializer(trigger).data, **{})


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread apps/oss/views/file.py
Comment on lines 68 to 70
@log(menu='file', operate='Delete file')
@has_permissions(RoleConstants.ADMIN, RoleConstants.WORKSPACE_MANAGE, RoleConstants.USER)
def delete(self, request: Request, file_id: str):
Comment thread apps/knowledge/serializers/tag.py Outdated
Comment on lines 163 to 166
else:
# 仅删除当前标签
QuerySet(Tag).filter(id=self.data.get('tag_id')).delete()
QuerySet(Tag).filter(id=self.data.get('tag_id'), knowledge_id=self.data.get('knowledge_id')).delete()
QuerySet(DocumentTag).filter(tag_id=self.data.get('tag_id')).delete()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fixed in commit 5a3d1b6. The single-tag delete path now fetches and validates the tag against the given knowledge_id first, raising an error if it doesn't exist, and then deletes both Tag and DocumentTag using the resolved tag.id.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fixed in 5a3d1b6. The else branch now fetches and validates the tag scoped to both tag_id and knowledge_id first. If the tag doesn't belong to that knowledge base, an error is raised before any deletion occurs. Both Tag and DocumentTag rows are then deleted using the resolved tag.id.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

apps/knowledge/serializers/tag.py:205

  • batch_delete deletes DocumentTag rows by the raw tag_ids request list, which can remove associations for tags outside the provided knowledge_id. It also fails to remove DocumentTag rows for other tags that get deleted by key__in=keys_to_delete, leaving orphans (because on_delete=DO_NOTHING). Derive the actual tag ids to delete within this knowledge/key set, then delete DocumentTag by that list.
            tags_to_delete = QuerySet(Tag).filter(id__in=tag_ids, knowledge_id=self.data.get('knowledge_id'))
            keys_to_delete = set(tags_to_delete.values_list('key', flat=True))

            # 删除具有相同key的所有标签
            QuerySet(Tag).filter(
                knowledge_id=self.data.get('knowledge_id'),
                key__in=keys_to_delete
            ).delete()

            # 删除关联的DocumentTag
            QuerySet(DocumentTag).filter(tag_id__in=tag_ids).delete()

Comment on lines +466 to 473
trigger_id_list = list(
QuerySet(Trigger)
.filter(id__in=trigger_id_list, workspace_id=workspace_id)
.values_list("id", flat=True)
)
for trigger_id in trigger_id_list:
trigger = QuerySet(Trigger).filter(id=trigger_id).first()
undeploy(TriggerModelSerializer(trigger).data, **{})
Comment on lines +153 to 157
tag = QuerySet(Tag).filter(
id=self.data.get('tag_id'), knowledge_id=self.data.get('knowledge_id')
).first()
if tag is None:
raise AppApiException(500, _('Tag id does not exist'))
@liuruibin liuruibin closed this Jul 27, 2026
@liuruibin
liuruibin deleted the pr@v2@fix_email_sm branch July 27, 2026 05:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants