fix(dataset): stop the legacy permission gate from shadowing RBAC - #39569
Open
BenjaminX wants to merge 1 commit into
Open
fix(dataset): stop the legacy permission gate from shadowing RBAC#39569BenjaminX wants to merge 1 commit into
BenjaminX wants to merge 1 commit into
Conversation
When RBAC_ENABLED is true, per-dataset access is owned by the RBAC whitelist and the @rbac_permission_required decorators. get_datasets already reflects that: its RBAC branch skips the legacy visibility filter and documents that "permission control is enforced via permission_keys on each item and @rbac_permission_required decorators". check_dataset_permission and check_dataset_operator_permission never got the same treatment, so they keep applying the legacy only_me / partial_team rules on every read and write path. The RBAC access config UI only writes the RBAC whitelist and never touches dataset.permission, so the two models cannot agree: after granting a member access from the RBAC UI, the legacy gate still rejects them with "You do not have permission to access this dataset." before the whitelist is ever consulted. Only the creator gets through, via the maintainer short-circuit. Skip the legacy rules when RBAC is enabled, keeping the tenant check in front of the guard so cross-tenant access is still rejected. Behaviour without RBAC is unchanged. GetProcessRuleApi resolves its dataset from a query-string document id and therefore cannot use the rbac_permission_required decorator; it was relying on the legacy gate alone. Enforce RBAC explicitly there once the dataset is known, following the existing enforce_rbac_access pattern, so neutralising the legacy gate does not widen access on that route. Audited every caller of both helpers: the console routes all carry rbac_permission_required, the service_api routes authenticate as the tenant owner (the legacy rules were already skipped for them), and the remaining call sites are helpers reached from decorated routes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
BenjaminX
requested review from
JohnJyong,
QuantumGhost and
laipz8200
as code owners
July 25, 2026 08:41
dosubot
Bot
added
the
size:S
Contributor
Pyrefly Diffbase → PR--- /tmp/pyrefly_base.txt 2026-07-25 08:43:10.611934916 +0000
+++ /tmp/pyrefly_pr.txt 2026-07-25 08:42:58.419823393 +0000
@@ -7789,9 +7789,17 @@
ERROR Object of class `FakeAccount` has no attribute `current_tenant_id` [missing-attribute]
--> tests/unit_tests/services/test_dataset_service_dataset.py:1124:9
ERROR Argument `SimpleNamespace` is not assignable to parameter `user` with type `Account | None` in function `services.dataset_service.DatasetService.check_dataset_operator_permission` [bad-argument-type]
- --> tests/unit_tests/services/test_dataset_service_dataset.py:1422:22
+ --> tests/unit_tests/services/test_dataset_service_dataset.py:1454:22
ERROR Argument `SimpleNamespace` is not assignable to parameter `dataset` with type `Dataset | None` in function `services.dataset_service.DatasetService.check_dataset_operator_permission` [bad-argument-type]
- --> tests/unit_tests/services/test_dataset_service_dataset.py:1430:25
+ --> tests/unit_tests/services/test_dataset_service_dataset.py:1454:46
+ERROR Argument `SimpleNamespace` is not assignable to parameter `user` with type `Account | None` in function `services.dataset_service.DatasetService.check_dataset_operator_permission` [bad-argument-type]
+ --> tests/unit_tests/services/test_dataset_service_dataset.py:1463:22
+ERROR Argument `SimpleNamespace` is not assignable to parameter `dataset` with type `Dataset | None` in function `services.dataset_service.DatasetService.check_dataset_operator_permission` [bad-argument-type]
+ --> tests/unit_tests/services/test_dataset_service_dataset.py:1463:46
+ERROR Argument `SimpleNamespace` is not assignable to parameter `user` with type `Account | None` in function `services.dataset_service.DatasetService.check_dataset_operator_permission` [bad-argument-type]
+ --> tests/unit_tests/services/test_dataset_service_dataset.py:1471:22
+ERROR Argument `SimpleNamespace` is not assignable to parameter `dataset` with type `Dataset | None` in function `services.dataset_service.DatasetService.check_dataset_operator_permission` [bad-argument-type]
+ --> tests/unit_tests/services/test_dataset_service_dataset.py:1479:25
ERROR Object of class `FakeAccount` has no attribute `id` [missing-attribute]
--> tests/unit_tests/services/test_dataset_service_document.py:106:9
ERROR Object of class `FakeAccount` has no attribute `current_tenant_id` [missing-attribute]
|
Contributor
Pyrefly Type Coverage
|
Contributor
Author
|
Ref issue link |
6 tasks
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
1 participant
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.
When RBAC_ENABLED is true, per-dataset access is owned by the RBAC whitelist and the @rbac_permission_required decorators. get_datasets already reflects that: its RBAC branch skips the legacy visibility filter and documents that "permission control is enforced via permission_keys on each item and @rbac_permission_required decorators".
check_dataset_permission and check_dataset_operator_permission never got the same treatment, so they keep applying the legacy only_me / partial_team rules on every read and write path. The RBAC access config UI only writes the RBAC whitelist and never touches dataset.permission, so the two models cannot agree: after granting a member access from the RBAC UI, the legacy gate still rejects them with "You do not have permission to access this dataset." before the whitelist is ever consulted. Only the creator gets through, via the maintainer short-circuit.
Skip the legacy rules when RBAC is enabled, keeping the tenant check in front of the guard so cross-tenant access is still rejected. Behaviour without RBAC is unchanged.
GetProcessRuleApi resolves its dataset from a query-string document id and therefore cannot use the rbac_permission_required decorator; it was relying on the legacy gate alone. Enforce RBAC explicitly there once the dataset is known, following the existing enforce_rbac_access pattern, so neutralising the legacy gate does not widen access on that route.
Audited every caller of both helpers: the console routes all carry rbac_permission_required, the service_api routes authenticate as the tenant owner (the legacy rules were already skipped for them), and the remaining call sites are helpers reached from decorated routes.