This is an automated email from the ASF dual-hosted git repository. EnxDev pushed a commit to branch test/chatbot-local in repository https://gitbox.apache.org/repos/asf/superset.git
commit df6e0095dcf1f0f016bf0c0b2cc0c2e2e86e759a Author: Enzo Martellucci <[email protected]> AuthorDate: Thu May 28 10:05:43 2026 +0200 fix(extensions): sync PR review fixes to test/chatbot-local - ExtensionsList.tsx: unique Tooltip ids per row; onKeyDown Enter/Space for star and delete role=button spans; data-test attrs; remove stale loading dep - ExtensionsList.test.tsx: 10 unit tests (import validation, delete confirm, star toggle, keyboard a11y, file upload) - api.py: path-traversal validation, upload size limit, LOCAL_EXTENSIONS 409 - test_api.py: 19 Python unit tests for POST and DELETE endpoints Co-Authored-By: Claude Sonnet 4.6 <[email protected]> --- .../src/extensions/ExtensionsList.test.tsx | 299 +++++++++++++--- .../src/extensions/ExtensionsList.tsx | 25 +- superset/extensions/api.py | 52 ++- tests/unit_tests/extensions/test_api.py | 381 +++++++++++++++++++++ 4 files changed, 700 insertions(+), 57 deletions(-) diff --git a/superset-frontend/src/extensions/ExtensionsList.test.tsx b/superset-frontend/src/extensions/ExtensionsList.test.tsx index ee2be4691d1..c48bfc174f0 100644 --- a/superset-frontend/src/extensions/ExtensionsList.test.tsx +++ b/superset-frontend/src/extensions/ExtensionsList.test.tsx @@ -16,73 +16,284 @@ * specific language governing permissions and limitations * under the License. */ -import { render, waitFor } from 'spec/helpers/testing-library'; +import userEvent from '@testing-library/user-event'; +import { fireEvent, render, screen, waitFor, within } from 'spec/helpers/testing-library'; +import { SupersetClient } from '@superset-ui/core'; import ExtensionsList from './ExtensionsList'; -import fetchMock from 'fetch-mock'; - -beforeAll(() => fetchMock.unmockGlobal()); - -// Mock initial state for the store -const mockInitialState = { - extensions: { - loading: false, - resourceCount: 2, - resourceCollection: [ - { - id: 1, - name: 'Test Extension 1', - enabled: true, - }, - { - id: 2, - name: 'Test Extension 2', - enabled: false, - }, - ], - bulkSelectEnabled: false, + +// --------------------------------------------------------------------------- +// Module-level mocks +// --------------------------------------------------------------------------- + +jest.mock('src/views/CRUD/hooks', () => ({ + useListViewResource: jest.fn(), +})); + +jest.mock('src/components', () => ({ + ListView: ({ columns, data }: any) => ( + <table> + <tbody> + {(data ?? []).map((row: any) => + columns.map((col: any) => ( + <td key={`${row.id}-${col.id}`}> + {col.Cell ? col.Cell({ row: { original: row } }) : row[col.accessor]} + </td> + )), + )} + </tbody> + </table> + ), +})); + +// Stub SubMenu so tests aren't coupled to the navigation menu rendering chain. +jest.mock('src/features/home/SubMenu', () => ({ + __esModule: true, + default: ({ buttons }: any) => ( + <div data-test="submenu"> + {(buttons ?? []).map((btn: any, i: number) => ( + // eslint-disable-next-line react/no-array-index-key + <button key={i} type="button" onClick={btn.onClick}> + {btn.name} + </button> + ))} + </div> + ), +})); + +// withToasts is the outermost HOC — pass through so callers can inject toast fns. +jest.mock('src/components/MessageToasts/withToasts', () => (C: any) => C); + +jest.mock('src/views/contributions', () => ({ + CHATBOT_LOCATION: 'superset.chatbot', +})); + +jest.mock('src/core/views', () => ({ + getRegisteredViewIds: jest.fn(() => []), + subscribeToRegistry: jest.fn(() => () => undefined), + getRegistryVersion: jest.fn(() => 0), +})); + +jest.mock('src/core/extensions', () => ({ + notifyExtensionSettingsChanged: jest.fn(), +})); + +jest.mock('@superset-ui/core', () => { + const actual = jest.requireActual('@superset-ui/core'); + return { + ...actual, + SupersetClient: { + get: jest.fn(), + post: jest.fn(), + put: jest.fn(), + delete: jest.fn(), + }, + }; +}); + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +const { useListViewResource } = jest.requireMock('src/views/CRUD/hooks'); +const mockGet = SupersetClient.get as jest.Mock; +const mockPost = SupersetClient.post as jest.Mock; +const mockPut = SupersetClient.put as jest.Mock; +const mockDelete = SupersetClient.delete as jest.Mock; + +const EXTENSIONS = [ + { + id: 'acme.chatbot', + name: 'chatbot', + publisher: 'acme', + enabled: true, + deletable: true, }, -}; + { + id: 'acme.widget', + name: 'widget', + publisher: 'acme', + enabled: true, + deletable: false, + }, +]; + +const mockFetchData = jest.fn(); +const mockRefreshData = jest.fn(); + +function setupHook(extensions = EXTENSIONS) { + useListViewResource.mockReturnValue({ + state: { + loading: false, + resourceCount: extensions.length, + resourceCollection: extensions, + }, + fetchData: mockFetchData, + refreshData: mockRefreshData, + }); +} const defaultProps = { addDangerToast: jest.fn(), addSuccessToast: jest.fn(), }; -const renderWithStore = (props = {}) => - render(<ExtensionsList {...defaultProps} {...props} />, { +function renderList(props = {}) { + return render(<ExtensionsList {...defaultProps} {...props} />, { useRedux: true, useQueryParams: true, useRouter: true, useTheme: true, - initialState: mockInitialState, }); +} + +function uploadFile(input: HTMLInputElement, file: File) { + Object.defineProperty(input, 'files', { value: [file], configurable: true }); + fireEvent.change(input); +} -test('renders extensions list with basic structure', async () => { - renderWithStore(); +// --------------------------------------------------------------------------- +// Setup / teardown +// --------------------------------------------------------------------------- - // Check that the component renders - expect(document.body).toBeInTheDocument(); +beforeEach(() => { + jest.clearAllMocks(); + mockGet.mockResolvedValue({ + json: { result: { active_chatbot_id: null, enabled: {} } }, + }); + setupHook(); }); -test('displays extension names in the list', async () => { - renderWithStore(); +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +test('renders the import button in the submenu', () => { + renderList(); + expect(screen.getByTestId('submenu')).toBeInTheDocument(); +}); +test('renders extension names in the table', async () => { + renderList(); await waitFor(() => { - // These texts should appear somewhere in the rendered component - expect(document.body).toHaveTextContent(/Extensions/); + expect(screen.getByText('chatbot')).toBeInTheDocument(); + expect(screen.getByText('widget')).toBeInTheDocument(); }); }); -test('calls toast functions when provided', () => { - const addDangerToast = jest.fn(); - const addSuccessToast = jest.fn(); +test('renders delete button only for deletable extensions', async () => { + renderList(); + await waitFor(() => { + // Only acme.chatbot has deletable: true + expect(screen.getAllByTestId('delete-extension')).toHaveLength(1); + }); +}); + +test('clicking delete opens confirmation dialog', async () => { + renderList(); + await waitFor(() => screen.getByText('chatbot')); - renderWithStore({ - addDangerToast, - addSuccessToast, + await userEvent.click(screen.getByTestId('delete-extension')); + + await waitFor(() => { + expect(screen.getByText(/are you sure you want to delete/i)).toBeInTheDocument(); }); +}); + +test('typing DELETE in confirmation modal triggers delete API call', async () => { + mockDelete.mockResolvedValue({}); + renderList(); + await waitFor(() => screen.getByText('chatbot')); + + await userEvent.click(screen.getByTestId('delete-extension')); + + const confirmInput = await screen.findByTestId('delete-modal-input'); + fireEvent.change(confirmInput, { target: { value: 'DELETE' } }); - // The component should accept these props without error - expect(addDangerToast).toBeDefined(); - expect(addSuccessToast).toBeDefined(); + const modal = screen.getByRole('dialog'); + const confirmBtn = within(modal) + .getAllByRole('button', { name: /^delete$/i }) + .pop()!; + await userEvent.click(confirmBtn); + + await waitFor(() => { + expect(mockDelete).toHaveBeenCalledWith( + expect.objectContaining({ endpoint: '/api/v1/extensions/acme/chatbot' }), + ); + expect(mockRefreshData).toHaveBeenCalled(); + }); +}); + +test('star button shown only for extensions registered as chatbot views', async () => { + const { getRegisteredViewIds } = jest.requireMock('src/core/views'); + (getRegisteredViewIds as jest.Mock).mockReturnValue(['acme.chatbot']); + + renderList(); + await waitFor(() => screen.getByText('chatbot')); + + expect(screen.getAllByTestId('set-default-chatbot')).toHaveLength(1); +}); + +test('clicking star calls PUT settings with the extension id', async () => { + const { getRegisteredViewIds } = jest.requireMock('src/core/views'); + (getRegisteredViewIds as jest.Mock).mockReturnValue(['acme.chatbot']); + mockPut.mockResolvedValue({ json: {} }); + + renderList(); + await waitFor(() => screen.getByText('chatbot')); + + await userEvent.click(screen.getByTestId('set-default-chatbot')); + + await waitFor(() => { + expect(mockPut).toHaveBeenCalledWith( + expect.objectContaining({ + endpoint: '/api/v1/extensions/settings', + jsonPayload: expect.objectContaining({ + active_chatbot_id: 'acme.chatbot', + }), + }), + ); + }); +}); + +test('pressing Enter on star span triggers set-default action', async () => { + const { getRegisteredViewIds } = jest.requireMock('src/core/views'); + (getRegisteredViewIds as jest.Mock).mockReturnValue(['acme.chatbot']); + mockPut.mockResolvedValue({ json: {} }); + + renderList(); + await waitFor(() => screen.getByText('chatbot')); + + fireEvent.keyDown(screen.getByTestId('set-default-chatbot'), { key: 'Enter' }); + + await waitFor(() => { + expect(mockPut).toHaveBeenCalled(); + }); +}); + +test('uploading a non-.supx file shows danger toast without calling API', async () => { + const addDangerToast = jest.fn(); + renderList({ addDangerToast }); + + const input = document.querySelector<HTMLInputElement>('input[type="file"]')!; + uploadFile(input, new File(['x'], 'evil.zip', { type: 'application/zip' })); + + expect(addDangerToast).toHaveBeenCalledWith( + expect.stringMatching(/\.supx/i), + ); + expect(mockPost).not.toHaveBeenCalled(); +}); + +test('uploading a .supx file calls POST endpoint and refreshes list', async () => { + mockPost.mockResolvedValue({}); + renderList(); + + const input = document.querySelector<HTMLInputElement>('input[type="file"]')!; + uploadFile(input, new File(['PK'], 'my.supx', { type: 'application/octet-stream' })); + + await waitFor(() => { + expect(mockPost).toHaveBeenCalledWith( + expect.objectContaining({ endpoint: '/api/v1/extensions/' }), + ); + expect(mockRefreshData).toHaveBeenCalled(); + }); }); diff --git a/superset-frontend/src/extensions/ExtensionsList.tsx b/superset-frontend/src/extensions/ExtensionsList.tsx index 1b8083c2e55..3902fd1ee19 100644 --- a/superset-frontend/src/extensions/ExtensionsList.tsx +++ b/superset-frontend/src/extensions/ExtensionsList.tsx @@ -246,7 +246,7 @@ const ExtensionsList: FunctionComponent<ExtensionsListProps> = ({ </Tooltip> {isChatbot && ( <Tooltip - id="set-default-chatbot-tooltip" + id={`set-chatbot-tooltip-${id}`} title={ isDefault ? t('Remove default') @@ -257,8 +257,14 @@ const ExtensionsList: FunctionComponent<ExtensionsListProps> = ({ <span role="button" tabIndex={0} + data-test="set-default-chatbot" className="action-button" onClick={() => setDefaultChatbot(id)} + onKeyDown={(e: React.KeyboardEvent) => { + if (e.key === 'Enter' || e.key === ' ') { + setDefaultChatbot(id); + } + }} > {isDefault ? ( <Icons.StarFilled iconSize="l" /> @@ -281,15 +287,21 @@ const ExtensionsList: FunctionComponent<ExtensionsListProps> = ({ > {(confirmDelete: () => void) => ( <Tooltip - id="delete-extension-tooltip" + id={`delete-extension-tooltip-${id}`} title={t('Delete')} placement="bottom" > <span role="button" tabIndex={0} + data-test="delete-extension" className="action-button" onClick={confirmDelete} + onKeyDown={(e: React.KeyboardEvent) => { + if (e.key === 'Enter' || e.key === ' ') { + confirmDelete(); + } + }} > <Icons.DeleteOutlined iconSize="l" /> </span> @@ -302,14 +314,7 @@ const ExtensionsList: FunctionComponent<ExtensionsListProps> = ({ }, }, ], - [ - loading, - settings, - chatbotIds, - toggleEnabled, - setDefaultChatbot, - handleDelete, - ], + [settings, chatbotIds, toggleEnabled, setDefaultChatbot, handleDelete], ); const menuData: SubMenuProps = { diff --git a/superset/extensions/api.py b/superset/extensions/api.py index 380e2c94165..cf841e86aaf 100644 --- a/superset/extensions/api.py +++ b/superset/extensions/api.py @@ -15,6 +15,7 @@ # specific language governing permissions and limitations # under the License. import mimetypes +import re from io import BytesIO from pathlib import Path from typing import Any @@ -37,6 +38,19 @@ from superset.extensions.utils import ( ) from superset.utils.core import check_is_safe_zip +# Allowlist for publisher and name path parameters — alphanumeric, hyphens, +# underscores only. Rejects path-traversal attempts (../), URL-encoded slashes, +# and any other characters that could escape EXTENSIONS_PATH. +_SEGMENT_RE = re.compile(r"^[A-Za-z0-9_-]+$") + +# Default 10 MB server-side upload limit; can be overridden via config. +_DEFAULT_MAX_UPLOAD_BYTES = 10 * 1024 * 1024 + + +def _validate_segment(value: str) -> bool: + """Return True if *value* is a safe publisher or name segment.""" + return bool(_SEGMENT_RE.match(value)) + class ExtensionsRestApi(BaseApi): allow_browser_login = True @@ -180,7 +194,8 @@ class ExtensionsRestApi(BaseApi): 500: $ref: '#/components/responses/500' """ - # Reconstruct composite ID from publisher and name + if not _validate_segment(publisher) or not _validate_segment(name): + return self.response(400, message="Invalid publisher or name.") composite_id = f"{publisher}.{name}" extensions = get_extensions() extension = extensions.get(composite_id) @@ -249,7 +264,19 @@ class ExtensionsRestApi(BaseApi): if not upload.filename or not upload.filename.endswith(".supx"): return self.response(400, message="File must have a .supx extension.") - stream = BytesIO(upload.read()) + max_bytes: int = current_app.config.get( + "EXTENSIONS_MAX_UPLOAD_SIZE", _DEFAULT_MAX_UPLOAD_BYTES + ) + raw = upload.read(max_bytes + 1) + if len(raw) > max_bytes: + return self.response( + 400, + message=( + f"File exceeds the maximum allowed size of {max_bytes} bytes." + ), + ) + + stream = BytesIO(raw) if not is_zipfile(stream): return self.response(400, message="File is not a valid ZIP archive.") @@ -262,7 +289,22 @@ class ExtensionsRestApi(BaseApi): except Exception as ex: # pylint: disable=broad-except return self.response(400, message=f"Invalid extension bundle: {ex}") + # Reject bundles whose manifest id collides with a LOCAL_EXTENSIONS entry. + local_ids = { + Path(p).name for p in current_app.config.get("LOCAL_EXTENSIONS", []) + } + if extension.manifest.id in local_ids: + return self.response( + 409, + message=( + f"Extension '{extension.manifest.id}' is already installed as a " + "local extension. Remove it from LOCAL_EXTENSIONS before uploading." + ), + ) + # Persist to EXTENSIONS_PATH so the extension survives restarts. + # Destination path is derived from the validated manifest id, not the + # uploaded filename, so the upload filename cannot escape EXTENSIONS_PATH. dest_dir = Path(extensions_path) dest_dir.mkdir(parents=True, exist_ok=True) dest_file = dest_dir / f"{extension.manifest.id}.supx" @@ -304,6 +346,9 @@ class ExtensionsRestApi(BaseApi): if not security_manager.is_admin(): return self.response(403, message="Admin access required.") + if not _validate_segment(publisher) or not _validate_segment(name): + return self.response(400, message="Invalid publisher or name.") + composite_id = f"{publisher}.{name}" extensions = get_extensions() extension = extensions.get(composite_id) @@ -429,7 +474,8 @@ class ExtensionsRestApi(BaseApi): 500: $ref: '#/components/responses/500' """ - # Reconstruct composite ID from publisher and name + if not _validate_segment(publisher) or not _validate_segment(name): + return self.response(400, message="Invalid publisher or name.") composite_id = f"{publisher}.{name}" extensions = get_extensions() extension = extensions.get(composite_id) diff --git a/tests/unit_tests/extensions/test_api.py b/tests/unit_tests/extensions/test_api.py new file mode 100644 index 00000000000..c78f3953e95 --- /dev/null +++ b/tests/unit_tests/extensions/test_api.py @@ -0,0 +1,381 @@ +# 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. + +"""Unit tests for the extensions REST API (POST and DELETE endpoints).""" + +from __future__ import annotations + +import io +import zipfile +from pathlib import Path +from typing import Any +from unittest.mock import MagicMock + +from superset.extensions.api import _validate_segment + +# --------------------------------------------------------------------------- +# _validate_segment helper +# --------------------------------------------------------------------------- + + +def test_validate_segment_accepts_alphanumeric() -> None: + assert _validate_segment("acme") is True + assert _validate_segment("my-ext") is True + assert _validate_segment("my_ext") is True + assert _validate_segment("Ext123") is True + + +def test_validate_segment_rejects_traversal() -> None: + assert _validate_segment("..") is False + assert _validate_segment("../etc") is False + assert _validate_segment("acme/bad") is False + assert _validate_segment("acme%2Fbad") is False + assert _validate_segment("") is False + + +def test_validate_segment_rejects_dots() -> None: + assert _validate_segment("acme.corp") is False + + +# --------------------------------------------------------------------------- +# Helpers for building fake .supx payloads +# --------------------------------------------------------------------------- + + +def _make_supx(manifest_id: str = "acme.chatbot") -> bytes: + """Return minimal valid .supx (zip) bytes with a manifest.""" + buf = io.BytesIO() + manifest_json = ( + f'{{"id": "{manifest_id}", "name": "Chatbot", "version": "1.0.0",' + f'"publisher": "acme", "description": "test"}}' + ) + with zipfile.ZipFile(buf, "w") as zf: + zf.writestr("manifest.json", manifest_json) + return buf.getvalue() + + +def _make_fake_extension(manifest_id: str = "acme.chatbot") -> MagicMock: + ext = MagicMock() + ext.manifest.id = manifest_id + ext.source_base_path = "upload://" + ext.frontend = {} + ext.backend = {} + ext.version = "1.0.0" + ext.name = "Chatbot" + return ext + + +# --------------------------------------------------------------------------- +# POST /api/v1/extensions/ — upload and install +# --------------------------------------------------------------------------- + + +class TestPostEndpoint: + def _post(self, client: Any, data: dict[str, Any], full_api_access: None) -> Any: + return client.post( + "/api/v1/extensions/", + data=data, + content_type="multipart/form-data", + ) + + def test_non_admin_rejected( + self, client: Any, full_api_access: None, mocker: Any + ) -> None: + mocker.patch( + "superset.extensions.api.security_manager.is_admin", return_value=False + ) + resp = client.post("/api/v1/extensions/", data={}) + assert resp.status_code == 403 + + def test_missing_extensions_path_returns_400( + self, client: Any, full_api_access: None, mocker: Any + ) -> None: + mocker.patch( + "superset.extensions.api.security_manager.is_admin", return_value=True + ) + mocker.patch.dict("flask.current_app.config", {"EXTENSIONS_PATH": None}) + resp = client.post("/api/v1/extensions/", data={}) + assert resp.status_code == 400 + assert "EXTENSIONS_PATH" in resp.json["message"] + + def test_missing_bundle_field_returns_400( + self, client: Any, full_api_access: None, mocker: Any, tmp_path: Path + ) -> None: + mocker.patch( + "superset.extensions.api.security_manager.is_admin", return_value=True + ) + mocker.patch.dict( + "flask.current_app.config", {"EXTENSIONS_PATH": str(tmp_path)} + ) + resp = client.post( + "/api/v1/extensions/", + data={}, + content_type="multipart/form-data", + ) + assert resp.status_code == 400 + assert "bundle" in resp.json["message"] + + def test_wrong_extension_rejected( + self, client: Any, full_api_access: None, mocker: Any, tmp_path: Path + ) -> None: + mocker.patch( + "superset.extensions.api.security_manager.is_admin", return_value=True + ) + mocker.patch.dict( + "flask.current_app.config", {"EXTENSIONS_PATH": str(tmp_path)} + ) + resp = client.post( + "/api/v1/extensions/", + data={"bundle": (io.BytesIO(b"data"), "evil.zip")}, + content_type="multipart/form-data", + ) + assert resp.status_code == 400 + assert ".supx" in resp.json["message"] + + def test_oversize_upload_rejected( + self, client: Any, full_api_access: None, mocker: Any, tmp_path: Path + ) -> None: + mocker.patch( + "superset.extensions.api.security_manager.is_admin", return_value=True + ) + mocker.patch.dict( + "flask.current_app.config", + {"EXTENSIONS_PATH": str(tmp_path), "EXTENSIONS_MAX_UPLOAD_SIZE": 10}, + ) + big = io.BytesIO(b"x" * 20) + resp = client.post( + "/api/v1/extensions/", + data={"bundle": (big, "big.supx")}, + content_type="multipart/form-data", + ) + assert resp.status_code == 400 + assert "maximum" in resp.json["message"] + + def test_not_a_zip_returns_400( + self, client: Any, full_api_access: None, mocker: Any, tmp_path: Path + ) -> None: + mocker.patch( + "superset.extensions.api.security_manager.is_admin", return_value=True + ) + mocker.patch.dict( + "flask.current_app.config", {"EXTENSIONS_PATH": str(tmp_path)} + ) + resp = client.post( + "/api/v1/extensions/", + data={"bundle": (io.BytesIO(b"not a zip"), "ext.supx")}, + content_type="multipart/form-data", + ) + assert resp.status_code == 400 + assert "ZIP" in resp.json["message"] + + def test_zip_slip_rejected( + self, client: Any, full_api_access: None, mocker: Any, tmp_path: Path + ) -> None: + """check_is_safe_zip raises on path-traversal entries inside the zip.""" + mocker.patch( + "superset.extensions.api.security_manager.is_admin", return_value=True + ) + mocker.patch.dict( + "flask.current_app.config", {"EXTENSIONS_PATH": str(tmp_path)} + ) + mocker.patch( + "superset.extensions.api.check_is_safe_zip", + side_effect=Exception("zip-slip detected"), + ) + supx = _make_supx() + resp = client.post( + "/api/v1/extensions/", + data={"bundle": (io.BytesIO(supx), "ext.supx")}, + content_type="multipart/form-data", + ) + assert resp.status_code == 400 + assert "zip-slip" in resp.json["message"] + + def test_local_extensions_collision_returns_409( + self, client: Any, full_api_access: None, mocker: Any, tmp_path: Path + ) -> None: + mocker.patch( + "superset.extensions.api.security_manager.is_admin", return_value=True + ) + mocker.patch.dict( + "flask.current_app.config", + { + "EXTENSIONS_PATH": str(tmp_path), + "LOCAL_EXTENSIONS": ["/opt/superset/ext/acme.chatbot"], + }, + ) + fake_ext = _make_fake_extension("acme.chatbot") + mocker.patch( + "superset.extensions.api.get_bundle_files_from_zip", return_value=[] + ) + mocker.patch( + "superset.extensions.api.get_loaded_extension", return_value=fake_ext + ) + supx = _make_supx("acme.chatbot") + resp = client.post( + "/api/v1/extensions/", + data={"bundle": (io.BytesIO(supx), "ext.supx")}, + content_type="multipart/form-data", + ) + assert resp.status_code == 409 + assert "local extension" in resp.json["message"] + + def test_happy_path_returns_201( + self, client: Any, full_api_access: None, mocker: Any, tmp_path: Path + ) -> None: + mocker.patch( + "superset.extensions.api.security_manager.is_admin", return_value=True + ) + mocker.patch.dict( + "flask.current_app.config", + {"EXTENSIONS_PATH": str(tmp_path), "LOCAL_EXTENSIONS": []}, + ) + fake_ext = _make_fake_extension("acme.chatbot") + mocker.patch( + "superset.extensions.api.get_bundle_files_from_zip", return_value=[] + ) + mocker.patch( + "superset.extensions.api.get_loaded_extension", return_value=fake_ext + ) + mocker.patch( + "superset.extensions.api.build_extension_data", + return_value={"id": "acme.chatbot"}, + ) + supx = _make_supx("acme.chatbot") + resp = client.post( + "/api/v1/extensions/", + data={"bundle": (io.BytesIO(supx), "ext.supx")}, + content_type="multipart/form-data", + ) + assert resp.status_code == 201 + assert resp.json["result"]["id"] == "acme.chatbot" + assert (tmp_path / "acme.chatbot.supx").exists() + + +# --------------------------------------------------------------------------- +# DELETE /api/v1/extensions/<publisher>/<name> +# --------------------------------------------------------------------------- + + +class TestDeleteEndpoint: + def test_non_admin_rejected( + self, client: Any, full_api_access: None, mocker: Any + ) -> None: + mocker.patch( + "superset.extensions.api.security_manager.is_admin", return_value=False + ) + resp = client.delete("/api/v1/extensions/acme/chatbot") + assert resp.status_code == 403 + + def test_path_traversal_publisher_rejected( + self, client: Any, full_api_access: None, mocker: Any + ) -> None: + mocker.patch( + "superset.extensions.api.security_manager.is_admin", return_value=True + ) + # Use percent-encoded dots so Flask routing passes the segment to the + # handler as the string ".." — literal slashes in the path would be + # intercepted by the router before reaching the view. + resp = client.delete("/api/v1/extensions/%2E%2E/passwd") + assert resp.status_code == 400 + assert "Invalid" in resp.json["message"] + + def test_invalid_name_returns_400( + self, client: Any, full_api_access: None, mocker: Any + ) -> None: + mocker.patch( + "superset.extensions.api.security_manager.is_admin", return_value=True + ) + resp = client.delete("/api/v1/extensions/acme/bad.name") + assert resp.status_code == 400 + assert "Invalid" in resp.json["message"] + + def test_unknown_extension_returns_404( + self, client: Any, full_api_access: None, mocker: Any + ) -> None: + mocker.patch( + "superset.extensions.api.security_manager.is_admin", return_value=True + ) + mocker.patch("superset.extensions.api.get_extensions", return_value={}) + resp = client.delete("/api/v1/extensions/acme/chatbot") + assert resp.status_code == 404 + + def test_local_extension_cannot_be_deleted( + self, client: Any, full_api_access: None, mocker: Any, tmp_path: Path + ) -> None: + local_base = str(tmp_path / "local-ext" / "dist") + fake_ext = _make_fake_extension("acme.chatbot") + fake_ext.source_base_path = local_base + mocker.patch( + "superset.extensions.api.security_manager.is_admin", return_value=True + ) + mocker.patch( + "superset.extensions.api.get_extensions", + return_value={"acme.chatbot": fake_ext}, + ) + mocker.patch.dict( + "flask.current_app.config", + {"LOCAL_EXTENSIONS": [str(tmp_path / "local-ext")]}, + ) + resp = client.delete("/api/v1/extensions/acme/chatbot") + assert resp.status_code == 400 + assert "LOCAL_EXTENSIONS" in resp.json["message"] + + def test_happy_path_deletes_file( + self, client: Any, full_api_access: None, mocker: Any, tmp_path: Path + ) -> None: + supx_file = tmp_path / "acme.chatbot.supx" + supx_file.write_bytes(b"fake") + + fake_ext = _make_fake_extension("acme.chatbot") + fake_ext.source_base_path = "upload://" + mocker.patch( + "superset.extensions.api.security_manager.is_admin", return_value=True + ) + mocker.patch( + "superset.extensions.api.get_extensions", + return_value={"acme.chatbot": fake_ext}, + ) + mocker.patch.dict( + "flask.current_app.config", + { + "LOCAL_EXTENSIONS": [], + "EXTENSIONS_PATH": str(tmp_path), + }, + ) + resp = client.delete("/api/v1/extensions/acme/chatbot") + assert resp.status_code == 200 + assert not supx_file.exists() + + def test_supx_file_missing_returns_404( + self, client: Any, full_api_access: None, mocker: Any, tmp_path: Path + ) -> None: + fake_ext = _make_fake_extension("acme.chatbot") + fake_ext.source_base_path = "upload://" + mocker.patch( + "superset.extensions.api.security_manager.is_admin", return_value=True + ) + mocker.patch( + "superset.extensions.api.get_extensions", + return_value={"acme.chatbot": fake_ext}, + ) + mocker.patch.dict( + "flask.current_app.config", + {"LOCAL_EXTENSIONS": [], "EXTENSIONS_PATH": str(tmp_path)}, + ) + resp = client.delete("/api/v1/extensions/acme/chatbot") + assert resp.status_code == 404
