rusackas commented on code in PR #43390:
URL: https://github.com/apache/superset/pull/43390#discussion_r3833767725
##########
superset/commands/tag/delete.py:
##########
@@ -134,10 +134,34 @@ def run(self) -> None:
TagDAO.delete_tags(self._tags)
def validate(self) -> None:
- exceptions = []
- # Validate tag exists
- for tag in self._tags:
- if not TagDAO.find_by_name(tag):
- exceptions.append(TagNotFoundError(tag))
+ exceptions: list[TagNotFoundError | TagDeleteFailedError] = []
+ for tag_name in self._tags:
+ tag_name = tag_name.strip()
+ tag = TagDAO.find_by_name(tag_name)
+ # Validate tag exists
+ if not tag:
+ exceptions.append(TagNotFoundError(tag_name))
+ continue
+ # System-generated tags (type:*, editor:*, favorited_by:*) are
+ # maintained by Superset itself and must not be deletable through
+ # the bulk route.
+ if tag.type is not None and tag.type != TagType.custom:
+ exceptions.append(
+ TagDeleteFailedError(
+ f"Tag {tag_name} is a system tag and cannot be deleted"
+ )
+ )
+ continue
+ # Deleting a tag cascades removal of all of its associations
+ # org-wide, so existence is not enough: require the user to be an
+ # admin or the tag's creator (the single-association route
+ # enforces per-object access in DeleteTaggedObjectCommand).
+ if not (
Review Comment:
Good catch, fixed in d0861c387e: the pk route now runs through
DeleteTagsCommand so it gets the same ownership/system-tag checks as
bulk_delete.
##########
superset/reports/filters.py:
##########
@@ -43,6 +43,26 @@ def _apply_editors(self, query: Query) -> Query:
return query.filter(ReportSchedule.id.in_(editor_ids_query))
+class ReportExecutionLogFilter(BaseFilter): # pylint:
disable=too-few-public-methods
+ """
+ Scope execution logs to report schedules the user can edit, mirroring
+ ``ReportScheduleFilter`` on the schedule API. Logs carry evaluated alert
+ values and database error messages, so they must not be readable across
+ ownership boundaries via an attacker-chosen schedule id.
+ """
+
+ def apply(self, query: Query, value: Any) -> Query:
+ if security_manager.can_access_all_datasources():
Review Comment:
Good catch, fixed in 0ceacdedf4/816b98f876: execution-log reads are now
gated on can_access_all_queries instead, scoped to editable schedules otherwise.
##########
superset/security/api.py:
##########
@@ -423,6 +424,11 @@ class UserRegistrationsRestAPI(BaseSupersetModelRestApi):
resource_name = "security/user_registrations"
datamodel = SQLAInterface(RegisterUser)
allow_browser_login = True
+ # This API is read-only by design: restricting the exposed routes keeps
+ # the FAB default POST/PUT/DELETE handlers from ever being registered,
+ # so a mis-granted role cannot create, alter, or silently cancel a
+ # pending registration.
+ include_route_methods = {RouteMethod.GET, RouteMethod.GET_LIST,
RouteMethod.INFO}
Review Comment:
Good catch, fixed in bf4a7817eb: restored DELETE (POST/PUT stay excluded),
still gated Admin-only via ADMIN_ONLY_VIEW_MENUS.
##########
superset/datasets/schemas.py:
##########
@@ -480,9 +480,13 @@ class DatasetColumnDrillInfoSchema(Schema):
class UserSchema(Schema):
+ # Deliberately excludes ``email``: drill_info is reachable by any user
Review Comment:
Good catch, fixed in b8ffd8db78: drill_info now uses an editor schema
without secondary_label instead of the shared SubjectResponseSchema.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
