feat(BA-6859): bind app_config fragments to their RBAC scope on write (repository layer)#12801
Closed
jopemachine wants to merge 13 commits into
Closed
feat(BA-6859): bind app_config fragments to their RBAC scope on write (repository layer)#12801jopemachine wants to merge 13 commits into
jopemachine wants to merge 13 commits into
Conversation
jopemachine
commented
Jul 13, 2026
1b707ed to
19f19b7
Compare
This was referenced Jul 13, 2026
19f19b7 to
f9ec19e
Compare
bbd0e93 to
269eb29
Compare
jopemachine
commented
Jul 13, 2026
… (repository layer) Run the app_config_fragment repository on RBACOpsProvider and make every fragment write flow through the RBAC scope machinery, branch-free: - The shared RBAC creator/unbinder specs now model *global-scoped* entities: RBACEntityCreator.scope_ref and RBACScopeEntityUnbinder.scope_ref accept None, meaning the entity lives outside the RBAC scope hierarchy and carries no scope association. - AppConfigScopeType.to_rbac_element_type() maps a fragment scope to its RBAC scope element (public -> None), next to its to_rbac_scope_type sibling. - create binds a user/domain fragment to its scope via association_scopes_entities in the same tx; purge unbinds it through RBACWriteOps.unbind_scope_entities (implemented directly on the ops provider — the standalone execute_rbac_* helpers are transitional), deleting the row and its association atomically. A public fragment takes the same single code path with scope_ref=None. - Registers APP_CONFIG_FRAGMENT as an owner-accessible RBAC resource type. - Repository/db_source create now takes AppConfigFragmentCreatorSpec directly. Data-layer foundation for RBAC-gating fragment writes; service-layer validators + DI registration follow (BA-6860). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
269eb29 to
d0a0c05
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds repository-layer RBAC scope bindings for AppConfig fragments.
Changes:
- Binds user/domain fragments to RBAC scopes; public fragments remain global.
- Atomically removes fragment bindings during purge.
- Extends RBAC resource registration and repository tests.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
changes/12801.feature.md |
Adds the feature changelog. |
src/ai/backend/common/data/app_config/types.py |
Maps AppConfig scopes to RBAC elements. |
src/ai/backend/common/data/permission/types.py |
Registers fragments as RBAC resources. |
src/ai/backend/manager/repositories/app_config_fragment/db_source/db_source.py |
Adds scoped creation and unbinding. |
src/ai/backend/manager/repositories/app_config_fragment/repositories.py |
Instantiates the RBAC ops provider. |
src/ai/backend/manager/repositories/app_config_fragment/repository.py |
Updates provider and create interfaces. |
src/ai/backend/manager/repositories/app_config_fragment/scope_binders.py |
Defines fragment scope-binding helpers. |
src/ai/backend/manager/repositories/base/rbac/entity_creator.py |
Supports global entities without associations. |
src/ai/backend/manager/repositories/base/rbac/scope_unbinder.py |
Supports global entity unbinding. |
src/ai/backend/manager/repositories/ops/rbac/provider.py |
Adds transactional scope unbinding. |
src/ai/backend/manager/services/app_config_fragment/service.py |
Passes creator specs directly. |
tests/unit/manager/repositories/app_config_fragment/test_repository.py |
Tests fragment RBAC associations. |
tests/unit/manager/services/app_config_fragment/test_service.py |
Updates creation delegation assertions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+112
to
+114
| all_scope_refs = [ | ||
| ref for ref in (creator.scope_ref, *creator.additional_scope_refs) if ref is not None | ||
| ] |
Comment on lines
+269
to
+271
| all_scope_refs = [ | ||
| ref for ref in (creator.scope_ref, *creator.additional_scope_refs) if ref is not None | ||
| ] |
…ding - rename db_source field `_ops` -> `_rbac_ops_provider` for consistency - purge: check unbind `deleted_count` and report not-found on a concurrent delete instead of returning a stale success - tighten the news fragment wording Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Condense the multi-line prose comments in the db_source and repositories to the project's lead-with-the-conclusion style; no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Wire the RBAC validators into the app_config_fragment write path so each write is authorized at the fragment's scope (BEP-1052): - create: target_element now returns the fragment's user/domain scope so the scope-chain check runs there; a public fragment is global-scoped and superadmin- only, enforced by the new PublicAppConfigFragmentWriteValidator - update/purge: single-entity RBAC validator (scope resolved via the scope binding) - bulk_update/bulk_purge: bulk RBAC validator - register the repository/service/processors in the DI containers Reads stay on the allow-list read tiers, so get/search carry no RBAC validator; bulk_create has no pre-existing targets, so the target-based bulk validator cannot authorize it. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
jopemachine
added a commit
that referenced
this pull request
Jul 14, 2026
…osure The processor-level RBAC validator wiring is consolidated into #12801 (BA-6859), which now owns the fragment write-authorization path end to end. Revert this PR's processors.py / factory.py validator wiring to base so it no longer duplicates #12801, leaving only the API-route change: fragment POST/PATCH/DELETE move from superadmin-only to authenticated (RBAC gates the write at #12801). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
- add RBACWriteOps.create_scoped (single scoped insert) and use it in the app_config_fragment db_source instead of bulk_create_scoped with a one-item list - remove the create/update/purge comments that framed authorization/write-gates in the DB layer; authz is enforced by the RBAC validators at the processor layer Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
jopemachine
commented
Jul 14, 2026
jopemachine
commented
Jul 14, 2026
jopemachine
commented
Jul 14, 2026
…pe ref - add RBACWriteOps.purge_scoped: delete the row and clear its scope association (by entity_id) in one pass, returning None when the row is already gone so a concurrent purge reports not-found — replaces the read-then-unbind flow - app_config_fragment purge uses purge_scoped; create computes its scope ref inline - drop the now-unused scope_binders module (fragment_rbac_scope_ref helper and the AppConfigFragmentScopeUnbinder it fed) Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
cc5214f to
335e962
Compare
Fragment purge switched to purge_scoped, and no other caller uses the provider's unbind_scope_entities, so drop the dead method and its scope_unbinder imports. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…test Address review comments: drop the _create_action helper (construct the action inline per case) and type user ids as UserID. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…ponsibility The bundled chain hardcoded the processor's validator order and tested nothing the two validators don't cover on their own. Split into TestScopeAuthorization (ScopeActionRBACValidator: own scope allowed, another's denied) and TestPublicWriteGuard (PublicAppConfigFragmentWriteValidator, no DB needed: non-superadmin denied, superadmin allowed). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Register APP_CONFIG_FRAGMENT as an RBAC scope and finish the write-authorization path on top of it: - ScopeType: add APP_CONFIG_FRAGMENT so to_scope_type() resolves for the fragment in both the scope-chain check and the entity-as-scope purge - ops: add single create_scoped / purge_scoped; purge clears the fragment row, its scope associations, and any permissions granted at its own scope - authorization: public (global-scoped) writes stay superadmin-only through the generic scope validator, so the bespoke PublicAppConfigFragmentWriteValidator is removed and create wires just the scope validator - migration: backfill APP_CONFIG_FRAGMENT permissions onto existing roles - tests: create/public authorization + entity-as-scope purge (with_tables gains the role/permission tables) Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Wire the bulk RBAC validator into scoped_search so a caller cannot read another user's fragments: each queried user/domain scope is checked for the caller's read permission, matching the audit_log scoped_search pattern. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Add BulkScopeActionRBACValidator (the bulk analog of ScopeActionRBACValidator: subject is the action's own entity type, targets are scopes) and wire it into bulk_create. bulk_create's action now exposes each spec's scope as a target, so a caller may bulk-create only in scopes it can write (own user / domain; public is superadmin-only). The generic bulk validator cannot authorize this — it would check CREATE on the USER/DOMAIN scope type, which users lack. The validator is added to the RBACValidators bundle as an optional field (like legacy_rbac) so existing test fixtures keep working; the composer always sets it. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
… up scope permissions Purge now takes an AppConfigFragmentPurgerSpec and removes the fragment row, its RBAC scope associations, and the permissions granted at its own scope together. Refine the backfill migration docstring to clarify that only permissions rows are backfilled (scope associations are per-fragment and written at creation time). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Member
Author
|
Superseded by a 3-PR split (each ≤ ~400 lines, migration + tests bundled with their feature):
Stack order: #12825 → #12826 → #12827. Bulk-write authorization moves to the API PR. Closing this in favor of the split. |
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
Runs the
app_config_fragmentrepository onRBACOpsProviderand binds each fragment to its RBAC scope on write — the data-layer foundation for RBAC-gating fragment writes. This is the base of the AppConfig-fragment-write-authz stack; the service-layer processor validators + DI registration land in a follow-up PR on top.What it does
RBACEntityCreator.scope_refandRBACScopeEntityUnbinder.scope_refnow acceptNone, meaning the entity lives outside the RBAC scope hierarchy (e.g. apublicapp-config fragment) and carries no scope association — the executors simply skip the association rows. Behavior for every existing caller (always non-None) is unchanged.AppConfigScopeType.to_rbac_element_type()maps a fragment scope to its RBAC scope element (user→USER,domain→DOMAIN,public→None), living on the enum next to itsto_rbac_scope_typesibling — no ad-hoc mapping tables at call sites.RBACEntityCreatorwithscope_ref = fragment_rbac_scope_ref(...). Auser/domainfragment is bound to its scope via anassociation_scopes_entitiesrow in the same transaction; apublicfragment getsscope_ref=None(no association). No scope branching in the db_source.AppConfigFragmentScopeUnbinder) is executed byRBACWriteOps.unbind_scope_entities, implemented directly on the ops provider (the standaloneexecute_rbac_*helpers are transitional and their logic is being folded into the ops providers), deleting the row and its scope association atomically; apublicfragment'sscope_refresolves toNone, so only the row is deleted. (The genericexecute_rbac_entity_purgerdoes not apply — it also clears entity-as-scope rows and requires a scope-convertible element type, which a leaf resource likeAPP_CONFIG_FRAGMENTis not.)APP_CONFIG_FRAGMENTas an owner-accessible RBAC resource type (_resource_types()), so a user's system role covers their own user-scope fragment.repository.createnow takesAppConfigFragmentCreatorSpecdirectly (the RBAC creator is assembled inside the repository layer).Why the split
The allow-list stays a read allowlist (existence registers a readable layer +
rank); write authorization moves to RBAC. This PR is only the repository/association plumbing so it can sit at the base. The follow-up wires the RBAC validators into the fragment write processors and drops the interim_may_writecheck.Notes
bulk_create/bulk_purgedo not yet write/remove associations (no API-reachable bulk-write path today); handled with the service wiring if/when needed.USER:<id>binding, a public create writes none, and purge removes it.📚 Stacked PRs
Part of the AppConfigFragment / AppConfig stack under BEP-1052 (epic BA-5781). Merge in order:
feat(BA-6552): app_config_fragments DB model and Alembic migrationfeat(BA-6553): repository layerrefactor(BA-6619): consolidate AppConfigScopeType into common.datarefactor(BA-6620): ExistsQuerier + AppConfigAllowList.existsfeat(BA-6554): AppConfigFragment service layerfeat(BA-6702): move fragment rank to the allow list with scope defaultsfeat(BA-6701): expose allow-list rank on the v2 API surfacefeat(BA-6704): cascade app config subtree deletion from the definitionfeat(BA-6626): app_config_fragment bulk repository layerfeat(BA-6618): app_config_fragment bulk CRUD service layerfeat(BA-6810): AppConfigFragment visible-fragments query (repository layer)feat(BA-6859): bind fragments to their RBAC scope on write (repository layer) ← you are herefeat(BA-6555): AppConfig merge engine + resolve service (service layer)feat(BA-6556): AppConfig REST v2 APIfeat(BA-6860): RBAC-gate fragment writes at the processorsAccess model: fragment
create/update/purgeare gated by RBAC (a user writes their ownuser-scope fragment, a domain admin their domain's, a superadmin any —publicis superadmin-only); reads are gated by the allow list (existence = readable layer,rank= merge order) plus scope visibility, with no RBAC on the read path;app_config_definitions/app_config_allow_listthemselves are superadmin-only (REST + GQL). Closed / superseded: #12794 · #12756 · #12764.