fix: Scope group vfolders by group_id in the REST folder list#12367
Draft
rapsealk wants to merge 3 commits into
Draft
fix: Scope group vfolders by group_id in the REST folder list#12367rapsealk wants to merge 3 commits into
group_id in the REST folder list#12367rapsealk wants to merge 3 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes the REST vfolder listing behavior so that GET /folders?group_id=<project_uuid> actually scopes group-owned vfolders to the requested project, instead of always returning the full “accessible union” regardless of scope. The change aligns REST behavior with the already-correctly-scoped GraphQL vfolder_nodes behavior described in the linked issue.
Changes:
- Apply a project-scope filter in
VFolderService.list()by forwarding agroup_scope_idwhen the action isScopeType.PROJECT. - Thread
group_scope_idthroughVfolderRepository.list_accessible_vfolders()intoquery_accessible_vfolders()viaextra_vf_group_conds. - Add unit tests to ensure
group_scope_idis forwarded for PROJECT scope and omitted for USER scope, plus a Towncrier fragment.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/ai/backend/manager/services/vfolder/services/vfolder.py |
Derives group_scope_id for PROJECT scope and forwards it to the repository to enforce project scoping for group vfolders. |
src/ai/backend/manager/repositories/vfolder/repository.py |
Adds group_scope_id parameter and translates it into extra_vf_group_conds for the underlying accessible-vfolders query. |
tests/unit/manager/services/vfolder/test_vfolder_crud_service.py |
Adds unit tests verifying the service forwards group_scope_id only for PROJECT scope. |
changes/12363.fix.md |
Documents the bugfix for release notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
`GET /folders` accepts a `group_id` query parameter and the handler resolves it into a PROJECT scope, but `VFolderService.list` ignored the scope — it called `list_accessible_vfolders` without it and only echoed `_scope_type`/`_scope_id` back in the result. So the listing always returned the full accessible union (every MODEL_STORE project, plus every domain project for admins), making `group_id` a silent no-op. Thread a `group_scope_id` through `VFolderRepository.list_accessible_vfolders` into `query_accessible_vfolders` as `extra_vf_group_conds` (group == scope_id), applied only when the request is project-scoped. User-owned and invited vfolders are unaffected, so a project-scoped request returns the caller's own folders plus that project's group folders — matching the `vfolder_nodes` GraphQL connection. Fixes #12363. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
rapsealk
changed the title
group_id in the REST folder list
rapsealk
force-pushed
the
fix/folders-honor-group-id
branch
from
6fa1491 to
d4aafd7
Compare
12363.fix.md -> 12367.fix.md Co-authored-by: octodog <mu001@lablup.com>
seedspirit
requested changes
Jun 23, 2026
Per review feedback: prefer "project" over "group" for the scope argument, and `project_id` is sufficient (no `_scope_` suffix needed). Renames the new `group_scope_id` parameter on `VFolderService.list` / `VFolderRepository.list_accessible_vfolders` to `project_id`, matching the `project_id` naming already used elsewhere in the vfolder tests. The underlying `VFolderRow.group` column and the REST `group_id` query parameter are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GET /folders(the vfolder list REST API) accepts agroup_idquery parameterand the handler resolves it into a PROJECT scope, but
VFolderService.listnever used the scope: it called
list_accessible_vfolderswithout it and onlyechoed
_scope_type/_scope_idback in the result. So the listing alwaysreturned the full accessible union — every
MODEL_STOREproject, plus everydomain project for admins, and all group vfolders for superadmins — regardless
of the requested scope.
group_idwas a silent no-op.Verified on 26.4.4:
/folders==/folders?group_id=<project>for both asuperadmin and a regular user.
Change
Thread a
group_scope_idthroughVFolderRepository.list_accessible_vfoldersinto
query_accessible_vfoldersasextra_vf_group_conds(group == scope_id),applied only when the request is project-scoped:
project's group folders — matching the
vfolder_nodesGraphQL connection(
scope_id: "project:<id>"), which already scopes correctly and is what theWebUI uses.
group_id(USER scope) is unchanged — back-compatible.Tests
tests/unit/manager/services/vfolder/test_vfolder_crud_service.py:test_project_scope_restricts_group_vfolders— PROJECT scope forwardsgroup_scope_id.test_user_scope_does_not_restrict_group_vfolders— USER scope forwardsNone.Both pass via
pants test.ruff/pants fmtclean.Fixes #12363.