refactor(BA-6922): model fragment scoped search as a scope action#13024
Closed
jopemachine wants to merge 4 commits into
Closed
Conversation
The scoped fragment search was a `BaseBulkAction[SearchableActionTarget]` carrying a list of domain/user targets OR-combined into one query. A search is not a bulk mutation over entities: it acts at one RBAC scope and returns whatever lives under it. Modelled as a bulk action it also reached the processor with no RBAC validator at all, so any authenticated caller could search any domain's or user's fragments. Follow the scoped-search pattern the rest of the codebase already uses (`SearchVFoldersInProjectAction`, `SearchKernelScopedHistoryAction`): one `BaseScopeAction` per scope kind, holding the repository `SearchScope` directly, wired through `ScopeActionProcessor` with the scope RBAC validator. - actions: `scoped_search.py` split into `domain_scoped_search.py` and `user_scoped_search.py`, one Action + ActionResult pair each - repository: `scoped_search(querier, scope)` takes a single scope - REST: `POST /scoped-search` split into `POST /domains/search` and `POST /users/search`, both auth + RBAC gated - DTO: `AppConfigFragmentScope` (domain/user id lists) replaced by `AppConfigFragmentDomainScope` / `AppConfigFragmentUserScope` Searching several scopes at once is no longer a single request. Nothing consumed the removed `queried_refs` / `element_refs()` on the result. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
size:XL
jopemachine
marked this pull request as draft
July 22, 2026 06:40
Splitting the request DTO per scope kind duplicated eight order/pagination fields to vary one submodel. The convention is one scoped-search input carrying a scope submodel (`ScopedSearchKernelHistoriesInput.scope`), so the scope kind belongs inside that submodel, not in the input's name. `ScopedSearchAppConfigFragmentInput.scope` is an `AppConfigFragmentScope` discriminated by `AppConfigFragmentSearchScopeType` (domain | user), with `scope_id` typed as the polymorphic `AppConfigScopeID`. `POST /domains/search` and `POST /users/search` collapse back into `POST /scoped/search`. The actions stay split: each scope is its own `BaseScopeAction` with its own RBAC scope validation, so the adapter matches on the scope type to pick one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Splitting per scope kind duplicated the action, the processor, the service method and the repository scope to vary one enum value. The fragment row is `(scope_type, scope_id)`, so one scope expresses the query exactly — the same shape `AppConfigFragmentCreatorSpec` already uses for writes. - repository: `DomainAppConfigFragmentSearchScope` / `UserAppConfigFragmentSearchScope` become one `AppConfigFragmentSearchScope` carrying `scope_type` + `scope_id` - action: one `ScopedSearchAppConfigFragmentAction` on `BaseScopeAction`, deriving its RBAC facets through `AppConfigScopeType.to_rbac_*` exactly as `CreateAppConfigFragmentAction` does - service / processor / adapter / route: one `scoped_search` each - DTO: `AppConfigFragmentScope` takes `AppConfigScopeType` + `AppConfigScopeID`, rejecting `public` until public reachability is decided Repository scoped-search tests are parametrized over the scope instead of one near-identical test per kind; the OR-across-scopes case is dropped with the capability. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jopemachine
changed the title
Member
Author
|
Superseded by #13032, which puts the same refactor on |
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.
📚 Stacked PRs
BA-6922feat: AppConfig fragment search REST v2 API (admin + scoped)BA-6922refactor: model the fragment scoped search as a scope action ← you are hereMerge in order, bottom-up. Each PR is based on the one above it, so its diff shows only its own layer.
Related: #12929 (BA-6922), resolved by #13020.
Summary
ScopedSearchAppConfigFragmentActionwas aBaseBulkAction[SearchableActionTarget]holding a list of domain/user targets OR-combined into one query. It is now aBaseScopeActionacting at the single scope the request names, deriving its RBAC facets throughAppConfigScopeType.to_rbac_*exactly as the siblingCreateAppConfigFragmentActiondoes.ScopeActionProcessorwithvalidators.rbac.scope. As a bulk action it reached the processor with no RBAC validator at all, so any authenticated caller could read any domain's or user's fragments by naming them in the request body.DomainAppConfigFragmentSearchScope/UserAppConfigFragmentSearchScopecollapse into oneAppConfigFragmentSearchScope(scope_type, scope_id)— the fragment row is(scope_type, scope_id), so one scope expresses the query exactly.scoped_search(querier, scope)takes a single scope.AppConfigFragmentScopebecomesscope_type: AppConfigScopeType+scope_id: AppConfigScopeID, mirroringCreateAppConfigFragmentInput. The route staysPOST /v2/app-config-fragments/scoped/search.Why: a search is not a bulk mutation.
BaseBulkActionexists to authorize an operation per entity across a batch (bulk_update/bulk_purgeover fragment ids); a scoped search acts at one RBAC scope and returns whatever lives under it.Tradeoff: searching several scopes in one request is gone — that was the only thing the list bought, and it cannot be authorized as a single scope action.
Open item, unchanged: the DTO rejects
scope_type: public. Public maps to theGLOBALRBAC scope, which has no scope element to authorize against, so whether a scoped search should reach public fragments still needs its own decision. The repository scope models it (scope_id=None), so lifting the restriction is a validator change.Test plan
pants fix fmt lint checkover the six touched packages — ruff, visibility, mypy all cleanopenapi.jsonregenerated with./backend.ai mgr api dump-openapi; final delta vs feat(BA-6922): AppConfig fragment search REST v2 API (admin + scoped) #13020 is/scoped-search→/scoped/searchplus the reshapedAppConfigFragmentScopequeried_refs/element_refs()removal verified to have no consumer anywhere in the tree/scoped/searchfor admin and non-admin🤖 Generated with Claude Code