Skip to content

fix(permission): scope system-model defaults to the binding's column (IKBA8U) - #2380

Open
yaojin3616 wants to merge 1 commit into
mainfrom
fix/ikba8u-view-file-leak
Open

fix(permission): scope system-model defaults to the binding's column (IKBA8U)#2380
yaojin3616 wants to merge 1 commit into
mainfrom
fix/ikba8u-view-file-leak

Conversation

@yaojin3616

Copy link
Copy Markdown
Collaborator

Summary

Fix IKBA8U: a user with only a knowledge-space viewer binding (i.e. view_space only) was getting view_file granted on every file inside the space, so the knowledge QA retrieval filter surfaced chunks the user could not browse from the listing UI.

Root cause

FineGrainedPermissionService._permission_ids_for_relation called default_permission_ids_for_relation for any system-model default. That helper flattens every level-1 permission across the three columns (space / folder / file). With nearest_binding_wins=True the lineage walk stopped at the space-level binding and handed the file the full level-1 set — including view_file. The QA retrieval post-filter then had no way to distinguish a real per-file grant from the inherited space-level grant, so the file leaked through.

Fix

  • Add column_permission_ids_for_relation on the knowledge-space permission template, which returns the matching column only (with the transitive folder→file grant preserved so the F036 listing UI keeps working).
  • Route the lineage-walk path (is_system model default) in _permission_ids_for_relation through it for knowledge_space / folder / knowledge_file.
  • default_permission_ids_for_relation is unchanged — the public-space / membership fallback (_public_space_viewer_permission_ids) still relies on the cross-column default.

Behavior matrix

binding before (buggy) after (fixed)
space-level viewer view_space, view_folder, view_file, download_* view_space only
folder-level viewer full level-1 set across all columns view_folder, download_folder, view_file, download_file (transitive)
file-level viewer full level-1 set across all columns view_file, download_file only
explicit-model binding respects permissions[] unchanged (still respects permissions[])

Verification

Added 13 tests across three files:

  • test/permission/test_ikba8u_view_file_column_scope.py — 10 tests: helper unit tests + real lineage-walk tests via InMemoryOpenFGA.
  • test/knowledge/test_ikba8u_bug_verify.py — 2 tests: the reproduction through the production KnowledgeSpaceService._get_child_item_effective_permission_ids path, asserting the leak is gone.
  • test/workstation/test_ikba8u_repro.py — 1 test: end-to-end queryChunksFromDB reproduction confirming the QA path now returns 0 docs for a space-only viewer.

Targeted tests (44 in total: F029 / F036 / IKBA8U) all green. Full test/knowledge/, test/permission/, test/workstation/ baselines unchanged: same set of pre-existing failures before and after the fix (the fix does not regress or fix any unrelated test).

Refs

IKBA8U — 日常模式-检索知识空间,只有知识空间权限,没有知识空间下的文件权限。检索出来了

…(IKBA8U)

A space-level ``viewer`` binding leaked ``view_file`` on every file in the
space, so the knowledge QA retrieval filter surfaced chunks the user
could not browse from the listing UI. Root cause:
``FineGrainedPermissionService._permission_ids_for_relation`` called
``default_permission_ids_for_relation`` for any system-model default,
which returns every level-1 permission across the three columns
(space / folder / file). With ``nearest_binding_wins=True`` the lineage
walk stopped at the space-level binding and handed the file the full
level-1 set, including ``view_file``.

Add ``column_permission_ids_for_relation`` on the knowledge-space
template, which returns the matching column only (with the transitive
folder->file grant preserved so the F036 listing UI keeps working), and
route the lineage-walk path through it. ``default_permission_ids_for_relation``
is unchanged -- the public-space / membership fallback still relies on
the cross-column default.

Refs: IKBA8U
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.

1 participant