aminghadersohi commented on code in PR #40357: URL: https://github.com/apache/superset/pull/40357#discussion_r3328079029
########## superset/mcp_service/plugin/tool/create_plugin.py: ########## @@ -0,0 +1,108 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import logging + +from fastmcp import Context +from superset_core.mcp.decorators import tool, ToolAnnotations + +from superset.extensions import event_logger +from superset.mcp_service.plugin.schemas import ( + CreatePluginRequest, + CreatePluginResponse, +) + +logger = logging.getLogger(__name__) + + +@tool( + tags=["mutate"], + class_permission_name="DynamicPlugin", + method_permission_name="write", + annotations=ToolAnnotations( + title="Register a dynamic plugin", + readOnlyHint=False, + destructiveHint=False, + ), +) +async def create_plugin( + request: CreatePluginRequest, ctx: Context +) -> CreatePluginResponse: + """Register a new dynamic (custom) plugin in Superset. Review Comment: Fixed: Added `tests/unit_tests/mcp_service/plugin/tool/test_plugin_tools.py` covering schema validation (blank/too-long/invalid-URL inputs), feature-flag gating, success path, IntegrityError/duplicate handling, not-found handling, and unexpected-error re-raise for both `create_plugin` and `update_plugin`. ########## superset/mcp_service/plugin/schemas.py: ########## @@ -0,0 +1,127 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Pydantic schemas for plugin-related MCP tool requests and responses.""" + +from pydantic import BaseModel, Field, field_validator, model_validator + + +class CreatePluginRequest(BaseModel): + """Request schema for create_plugin.""" + + name: str = Field( + ..., + min_length=1, + description="Human-friendly name for the plugin.", + ) + key: str = Field( + ..., + min_length=1, + description=( + "Unique plugin key. Should match the package name from the plugin's " + "package.json (e.g. '@my-org/my-chart-plugin')." + ), + ) + bundle_url: str = Field( + ..., + min_length=1, + description=( + "Full URL pointing to the built plugin bundle " + "(e.g. a CDN-hosted JavaScript file)." + ), + ) Review Comment: Fixed: `CreatePluginRequest` now has a `validate_bundle_url` field validator that rejects any `bundle_url` that doesn't start with `http://` or `https://`, preventing javascript:, data:, ftp:, and other invalid schemes from being stored. ########## superset/mcp_service/plugin/schemas.py: ########## @@ -0,0 +1,127 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Pydantic schemas for plugin-related MCP tool requests and responses.""" + +from pydantic import BaseModel, Field, field_validator, model_validator + + +class CreatePluginRequest(BaseModel): + """Request schema for create_plugin.""" + + name: str = Field( + ..., + min_length=1, + description="Human-friendly name for the plugin.", + ) + key: str = Field( + ..., + min_length=1, + description=( + "Unique plugin key. Should match the package name from the plugin's " + "package.json (e.g. '@my-org/my-chart-plugin')." + ), + ) + bundle_url: str = Field( + ..., + min_length=1, + description=( + "Full URL pointing to the built plugin bundle " + "(e.g. a CDN-hosted JavaScript file)." + ), + ) + + @field_validator("name", "key", "bundle_url") + @classmethod + def must_not_be_blank(cls, v: str) -> str: + if not v.strip(): + raise ValueError("Field must not be blank") + return v.strip() + + +class CreatePluginResponse(BaseModel): + """Response schema for create_plugin.""" + + id: int | None = Field( + None, + description="ID of the newly created plugin. None if creation failed.", + ) + name: str | None = Field(None, description="Plugin name.") + key: str | None = Field(None, description="Plugin key.") + bundle_url: str | None = Field(None, description="Plugin bundle URL.") + error: str | None = Field( + None, + description="Error message if creation failed, otherwise null.", + ) + + +class UpdatePluginRequest(BaseModel): + """Request schema for update_plugin.""" + + id: int = Field(..., description="ID of the plugin to update.") + name: str | None = Field( + None, + min_length=1, + description="New human-friendly name for the plugin.", + ) + key: str | None = Field( + None, + min_length=1, + description=( + "New unique plugin key. Should match the package name from the " + "plugin's package.json (e.g. '@my-org/my-chart-plugin')." + ), + ) + bundle_url: str | None = Field( + None, + min_length=1, + description=( + "New full URL pointing to the built plugin bundle " + "(e.g. a CDN-hosted JavaScript file)." + ), + ) Review Comment: Fixed: `UpdatePluginRequest` has the same `validate_bundle_url` field validator as the create schema, rejecting non-http/https bundle URLs before they reach the database. -- 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]
