Compare commits

..

2 Commits

Author SHA1 Message Date
Claude Code
97b6465ece fix(embedded-sdk): validate supersetDomain and surface relaxing sandbox tokens
Builds on the dashboard id validation in this PR with two more
defense-in-depth input checks on embedDashboard params:

- supersetDomain is validated as a parseable absolute URL (it must carry a
  protocol), and the postMessage targetOrigin now uses the normalized
  `new URL(supersetDomain).origin` rather than the raw domain. Sub-path
  deployments keep working because the iframe src still uses the full
  domain; only the targetOrigin is normalized to a clean origin.
- iframeSandboxExtras tokens that relax the iframe's isolation are still
  honored (the option is an intentional escape hatch) but are now logged
  via console.warn so they aren't enabled unintentionally.

Adds unit tests for validateSupersetDomain and findUnsafeSandboxExtras.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-01 15:34:06 -07:00
Claude Code
4f2fe42044 fix(embedded-sdk): validate embedded dashboard id format before use
Add defense-in-depth format validation for the embedded dashboard id
before it is interpolated into the iframe URL. The id is a
Superset-generated UUID (hexadecimal characters and hyphens), so a new
exported `validateEmbeddedDashboardId` helper rejects any value that does
not match that format.

This is a minor behavior change: a malformed id now throws instead of
being used as-is. A unit test file covers the helper, and a jest config
is added so the switchboard ES module is transformed during testing.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-01 15:25:54 -07:00
50 changed files with 327 additions and 3594 deletions

75
.github/SECURITY.md vendored Normal file
View File

@@ -0,0 +1,75 @@
# Security Policy
This is a project of the [Apache Software Foundation](https://apache.org) and follows the
ASF [vulnerability handling process](https://apache.org/security/#vulnerability-handling).
## Reporting Vulnerabilities
**⚠️ Please do not file GitHub issues for security vulnerabilities as they are public! ⚠️**
Apache Software Foundation takes a rigorous standpoint in annihilating the security issues
in its software projects. Apache Superset is highly sensitive and forthcoming to issues
pertaining to its features and functionality.
If you have any concern or believe you have found a vulnerability in Apache Superset,
please get in touch with the Apache Superset Security Team privately at
e-mail address [security@superset.apache.org](mailto:security@superset.apache.org).
More details can be found on the ASF website at
[ASF vulnerability reporting process](https://apache.org/security/#reporting-a-vulnerability)
**Submission Standards & AI Policy**
To ensure engineering focus remains on verified risks and to manage high reporting volumes, all reports must meet the following criteria:
- Plain Text Format: In accordance with Apache guidelines, please provide all details in plain text within the email body. Avoid sending PDFs, Word documents, or password-protected archives.
- Mandatory AI Disclosure: If you utilized Large Language Models (LLMs) or AI tools to identify a flaw or assist in writing a report, you must disclose this in your submission so our triage team can contextualize the findings.
- Human-Verified PoC: All submissions must include a manual, step-by-step Proof of Concept (PoC) performed on a supported release. Raw AI outputs, hypothetical chat transcripts, or unverified scanner logs will be closed as Invalid.
We kindly ask you to include the following information in your report to assist our developers in triaging and remediating issues efficiently:
- Version/Commit: The specific version of Apache Superset or the Git commit hash you are using.
- Configuration: A sanitized copy of your `superset_config.py` file or any config overrides.
- Environment: Your deployment method (e.g., Docker Compose, Helm, or source) and relevant OS/Browser details.
- Impacted Component: Identification of the affected area (e.g., Python backend, React frontend, or a specific database connector).
- Expected vs. Actual Behavior: A clear description of the intended system behavior versus the observed vulnerability.
- Detailed Reproduction Steps: Clear, manual steps to reproduce the vulnerability.
**Vulnerability Definition**
Apache Superset considers a security vulnerability to be a demonstrable issue that has meaningful impact on confidentiality, integrity, or availability beyond the intended security model. Low-impact boundary variations or technical edge cases in existing access controls may be classified as hardening improvements rather than vulnerabilities, even if exploitable.
**Out of Scope Vulnerabilities**
To prioritize engineering efforts on genuine architectural risks, the following scenarios are explicitly out of scope and will not be issued a CVE:
- **Attacks requiring Admin privileges**: (e.g., CSS injection, template manipulation, dashboard ownership overrides, or modifying global system settings). Per the CVE vulnerability definition in CNA Operational Rules 4.1, a qualifying vulnerability must allow violation of a security policy. The Admin role is a fully trusted operational boundary defined by Apache Superset's security policy; actions within this boundary do not violate that policy and are therefore considered intended capabilities 'by design,' not vulnerabilities.
- **Brute Force and Rate Limiting**: Reports targeting a lack of resource exhaustion protections, generic rate-limiting, or volumetric Denial of Service (DoS) attempts.
- **Theoretical attack vectors**: Issues without a demonstrable, reproducible exploit path.
- **Non-Exploitable Findings**: Missing security headers, generic banner disclosures, or descriptive error messages that do not lead to a direct, documented exploit.
- **User enumeration**: API responses, timing differences, or error messages that reveal whether user accounts, IDs, dashboards, or datasets exist.
- **Information disclosure (low impact)**: Software version disclosure, generic error messages, stack traces without sensitive data exposure, or system configuration details that don't enable further exploitation.
- **Resource exhaustion requiring authentication**: Denial of Service attacks that require valid user credentials and don't bypass rate limiting or resource controls.
- **Missing security headers**: Without demonstration of a concrete exploit scenario that leverages the missing header.
**Outcome of Reports**
Reports that are deemed out-of-scope for a CVE but represent valid security best practices or hardening opportunities may be converted into public GitHub issues. This allows the community to contribute to the general hardening of the platform even when a specific vulnerability threshold is not met.
Note that Apache Superset is not responsible for any third-party dependencies that may
have security issues. Any vulnerabilities found in third-party dependencies should be
reported to the maintainers of those projects. Results from security scans of Apache
Superset dependencies found on its official Docker image can be remediated at release time
by extending the image itself.
**Vulnerability Aggregation & CVE Attribution**
In accordance with MITRE CNA Operational Rules (4.1.10, 4.1.11, and 4.2.13), Apache Superset issues CVEs based on the underlying architectural root cause rather than the number of affected endpoints or exploit payloads.
- Aggregation: If multiple exploit vectors stem from the same programmatic failure or shared vulnerable code, they must be aggregated into a single, comprehensive report.
- Independent Fixes: Separate CVEs will only be assigned if the vulnerabilities reside in decoupled architectural modules and can be fixed independently of one another.
Reports that fail to aggregate related findings will be merged during triage to ensure an accurate and defensible CVE record.
**Your responsible disclosure and collaboration are invaluable.**
## Extra Information
- [Apache Superset documentation](https://superset.apache.org/docs/security)
- [Common Vulnerabilities and Exposures by release](https://superset.apache.org/docs/security/cves)
- [How Security Vulnerabilities are Reported & Handled in Apache Superset (Blog)](https://preset.io/blog/how-security-vulnerabilities-are-reported-and-handled-in-apache-superset/)

View File

@@ -24,41 +24,32 @@ runs:
- name: Interpret Python Version
id: set-python-version
shell: bash
env:
INPUT_PYTHON_VERSION: ${{ inputs.python-version }}
run: |
if [ "$INPUT_PYTHON_VERSION" = "current" ]; then
RESOLVED_VERSION="3.11"
elif [ "$INPUT_PYTHON_VERSION" = "next" ]; then
if [ "${{ inputs.python-version }}" = "current" ]; then
echo "PYTHON_VERSION=3.11" >> $GITHUB_ENV
elif [ "${{ inputs.python-version }}" = "next" ]; then
# currently disabled in GHA matrixes because of library compatibility issues
RESOLVED_VERSION="3.12"
elif [ "$INPUT_PYTHON_VERSION" = "previous" ]; then
RESOLVED_VERSION="3.10"
elif printf '%s' "$INPUT_PYTHON_VERSION" | grep -Eq '^[0-9]+\.[0-9]+(\.[0-9]+)?$'; then
RESOLVED_VERSION="$INPUT_PYTHON_VERSION"
echo "PYTHON_VERSION=3.12" >> $GITHUB_ENV
elif [ "${{ inputs.python-version }}" = "previous" ]; then
echo "PYTHON_VERSION=3.10" >> $GITHUB_ENV
else
echo "Invalid python-version: '$INPUT_PYTHON_VERSION'" >&2
exit 1
echo "PYTHON_VERSION=${{ inputs.python-version }}" >> $GITHUB_ENV
fi
echo "python-version=$RESOLVED_VERSION" >> "$GITHUB_OUTPUT"
- name: Set up Python ${{ steps.set-python-version.outputs.python-version }}
- name: Set up Python ${{ env.PYTHON_VERSION }}
uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5
with:
python-version: ${{ steps.set-python-version.outputs.python-version }}
python-version: ${{ env.PYTHON_VERSION }}
cache: ${{ inputs.cache }}
- name: Install dependencies
env:
INPUT_INSTALL_SUPERSET: ${{ inputs.install-superset }}
INPUT_REQUIREMENTS_TYPE: ${{ inputs.requirements-type }}
run: |
if [ "$INPUT_INSTALL_SUPERSET" = "true" ]; then
if [ "${{ inputs.install-superset }}" = "true" ]; then
sudo apt-get update && sudo apt-get -y install libldap2-dev libsasl2-dev
pip install --upgrade pip setuptools wheel uv
if [ "$INPUT_REQUIREMENTS_TYPE" = "dev" ]; then
if [ "${{ inputs.requirements-type }}" = "dev" ]; then
uv pip install --system -r requirements/development.txt
elif [ "$INPUT_REQUIREMENTS_TYPE" = "base" ]; then
elif [ "${{ inputs.requirements-type }}" = "base" ]; then
uv pip install --system -r requirements/base.txt
fi

View File

@@ -73,21 +73,20 @@ jobs:
shell: bash
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
BUILD_PRESET: ${{ matrix.build_preset }}
run: |
# Single platform builds in pull_request context to speed things up
if [ "$GITHUB_EVENT_NAME" = "push" ]; then
if [ "${{ github.event_name }}" = "push" ]; then
PLATFORM_ARG="--platform linux/arm64 --platform linux/amd64"
# can only --load images in single-platform builds
PUSH_OR_LOAD="--push"
elif [ "$GITHUB_EVENT_NAME" = "pull_request" ]; then
elif [ "${{ github.event_name }}" = "pull_request" ]; then
PLATFORM_ARG="--platform linux/amd64"
PUSH_OR_LOAD="--load"
fi
supersetbot docker \
$PUSH_OR_LOAD \
--preset "$BUILD_PRESET" \
--preset ${{ matrix.build_preset }} \
--context "$EVENT" \
--context-ref "$RELEASE" $FORCE_LATEST \
--extra-flags "--build-arg INCLUDE_CHROMIUM=false --tag $IMAGE_TAG" \
@@ -113,10 +112,8 @@ jobs:
- name: docker-compose sanity check
if: (steps.check.outputs.python || steps.check.outputs.frontend || steps.check.outputs.docker) && matrix.build_preset == 'dev'
shell: bash
env:
BUILD_PRESET: ${{ matrix.build_preset }}
run: |
export SUPERSET_BUILD_TARGET=$BUILD_PRESET
export SUPERSET_BUILD_TARGET=${{ matrix.build_preset }}
# This should reuse the CACHED image built in the previous steps
docker compose build superset-init --build-arg DEV_MODE=false --build-arg INCLUDE_CHROMIUM=false
docker compose up superset-init --exit-code-from superset-init

View File

@@ -19,10 +19,10 @@ jobs:
- name: Check for latest tag
id: latest-tag
env:
RELEASE_TAG_NAME: ${{ github.event.release.tag_name }}
run: |
source ./scripts/tag_latest_release.sh "$RELEASE_TAG_NAME" --dry-run
source ./scripts/tag_latest_release.sh $(echo ${GITHUB_EVENT_RELEASE_TAG_NAME}) --dry-run
env:
GITHUB_EVENT_RELEASE_TAG_NAME: ${{ github.event.release.tag_name }}
- name: Configure Git
run: |

View File

@@ -71,12 +71,10 @@ jobs:
output: ' '
- name: pre-commit
env:
CHANGED_FILES: ${{ steps.changed_files.outputs.files }}
run: |
set +e # Don't exit immediately on failure
export SKIP=type-checking-frontend
pre-commit run --files $CHANGED_FILES
pre-commit run --files ${{ steps.changed_files.outputs.files }}
PRE_COMMIT_EXIT_CODE=$?
git diff --quiet --exit-code
GIT_DIFF_EXIT_CODE=$?

View File

@@ -2,7 +2,6 @@ name: 🎪 Superset Showtime
# Ultra-simple: just sync on any PR state change
on:
# zizmor: ignore[dangerous-triggers] - required to react to PR label changes; this workflow does not check out or execute PR-provided code
pull_request_target:
types: [labeled, unlabeled, synchronize, closed]
@@ -103,7 +102,7 @@ jobs:
- name: Install Superset Showtime
if: steps.auth.outputs.authorized == 'true'
run: |
echo "::notice::Maintainer $GITHUB_ACTOR triggered deploy for PR ${PULL_REQUEST_NUMBER}"
echo "::notice::Maintainer ${GITHUB_ACTOR} triggered deploy for PR ${PULL_REQUEST_NUMBER}"
pip install --upgrade superset-showtime
showtime version
@@ -174,11 +173,9 @@ jobs:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
DOCKERHUB_USER: ${{ secrets.DOCKERHUB_USER }}
DOCKERHUB_TOKEN: ${{ secrets.DOCKERHUB_TOKEN }}
CHECK_PR_NUMBER: ${{ steps.check.outputs.pr_number }}
CHECK_TARGET_SHA: ${{ steps.check.outputs.target_sha }}
run: |
PR_NUM="$CHECK_PR_NUMBER"
TARGET_SHA="$CHECK_TARGET_SHA"
PR_NUM="${{ steps.check.outputs.pr_number }}"
TARGET_SHA="${{ steps.check.outputs.target_sha }}"
if [[ -n "$TARGET_SHA" ]]; then
python -m showtime sync $PR_NUM --sha "$TARGET_SHA"
else

View File

@@ -2,7 +2,6 @@ name: Docs Deployment
on:
# Deploy after integration tests complete on master
# zizmor: ignore[dangerous-triggers] - runs in base-branch context after a trusted upstream workflow; scoped to master
workflow_run:
workflows: ["Python-Integration"]
types: [completed]

View File

@@ -7,7 +7,6 @@ on:
- "superset/db_engine_specs/**"
- ".github/workflows/superset-docs-verify.yml"
types: [synchronize, opened, reopened, ready_for_review]
# zizmor: ignore[dangerous-triggers] - runs in base-branch context and only consumes artifacts from the trusted upstream workflow
workflow_run:
workflows: ["Python-Integration"]
types: [completed]

View File

@@ -141,9 +141,8 @@ jobs:
- name: Set safe app root
if: failure()
id: set-safe-app-root
env:
APP_ROOT: ${{ matrix.app_root }}
run: |
APP_ROOT="${{ matrix.app_root }}"
SAFE_APP_ROOT=${APP_ROOT//\//_}
echo "safe_app_root=$SAFE_APP_ROOT" >> $GITHUB_OUTPUT
- name: Upload Artifacts
@@ -255,9 +254,8 @@ jobs:
- name: Set safe app root
if: failure()
id: set-safe-app-root
env:
APP_ROOT: ${{ matrix.app_root }}
run: |
APP_ROOT="${{ matrix.app_root }}"
SAFE_APP_ROOT=${APP_ROOT//\//_}
echo "safe_app_root=$SAFE_APP_ROOT" >> $GITHUB_OUTPUT
- name: Upload Playwright Artifacts

View File

@@ -62,8 +62,6 @@ jobs:
run: echo "branch_name=helm-publish-${GITHUB_SHA:0:7}" >> $GITHUB_ENV
- name: Force recreate branch from gh-pages
env:
BRANCH_NAME: ${{ env.branch_name }}
run: |
# Ensure a clean working directory
git reset --hard
@@ -75,13 +73,13 @@ jobs:
git fetch origin gh-pages
# Check out and reset the target branch based on gh-pages
git checkout -B "$BRANCH_NAME" origin/gh-pages
git checkout -B ${{ env.branch_name }} origin/gh-pages
# Remove submodules from the branch
git submodule deinit -f --all
# Force push to the remote branch
git push origin "$BRANCH_NAME" --force
git push origin ${{ env.branch_name }} --force
# Return to the original branch
git checkout local_gha_temp
@@ -106,7 +104,7 @@ jobs:
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
with:
script: |
const branchName = process.env.BRANCH_NAME;
const branchName = '${{ env.branch_name }}';
const [owner, repo] = process.env.GITHUB_REPOSITORY.split('/');
if (!branchName) {

View File

@@ -1,7 +1,6 @@
name: Translation Regression Comment
on:
# zizmor: ignore[dangerous-triggers] - runs in base-branch context and only consumes the uploaded artifact; never checks out PR code (see note below)
workflow_run:
workflows: ["Translations"]
types: [completed]

View File

@@ -84,15 +84,13 @@ jobs:
# drift on the base branch.
- name: Fetch base ref and create comparison worktree
if: steps.check.outputs.python == 'true' || steps.check.outputs.frontend == 'true'
env:
PR_BASE_REF: ${{ github.event.pull_request.base.ref }}
run: |
# For PRs use the base branch; for direct pushes compare against the previous commit.
BASE_REF="$PR_BASE_REF"
BASE_REF="${{ github.event.pull_request.base.ref }}"
if [ -n "$BASE_REF" ]; then
git fetch --depth=1 origin "$BASE_REF"
else
git fetch --depth=2 origin "$GITHUB_REF"
git fetch --depth=2 origin "${{ github.ref }}"
fi
git worktree add /tmp/base-worktree FETCH_HEAD

View File

@@ -68,11 +68,9 @@ jobs:
build: "true"
- name: Use Node.js 20
# zizmor: ignore[cache-poisoning] - node only runs the supersetbot CLI; no dependency cache is enabled
uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6
with:
node-version: 20
package-manager-cache: false
- name: Setup supersetbot
uses: ./.github/actions/setup-supersetbot/
@@ -127,11 +125,9 @@ jobs:
fetch-depth: 0
- name: Use Node.js 20
# zizmor: ignore[cache-poisoning] - node only runs the supersetbot CLI; no dependency cache is enabled
uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6
with:
node-version: 20
package-manager-cache: false
- name: Setup supersetbot
uses: ./.github/actions/setup-supersetbot/

View File

@@ -1,14 +1,12 @@
name: Welcome New Contributor
on:
# zizmor: ignore[dangerous-triggers] - posts a welcome comment only; does not check out or execute PR-provided code
pull_request_target:
types: [opened]
jobs:
welcome:
runs-on: ubuntu-24.04
if: github.event.pull_request.author_association == 'FIRST_TIME_CONTRIBUTOR'
permissions:
pull-requests: write

View File

@@ -52,43 +52,6 @@ Common pre-commit failures:
- **External API exposure** - Use UUIDs in public APIs instead of internal integer IDs
- **Existing models** - Add UUID fields alongside integer IDs for gradual migration
## Security and Threat Model
Before evaluating any code path for security issues, read [`SECURITY.md`](SECURITY.md). It is the canonical, authoritative source for Apache Superset's security model and is referenced by both human reporters and automated scanners.
In short, the test for whether a finding is in scope is one question:
> *Does it let a principal perform an action the role and capability matrix in `SECURITY.md` does not entitle them to?*
If yes, it is in scope. If no, it is not.
The three trust boundaries are:
1. **The Admin role** is a fully trusted operational principal. Anything an Admin can do through documented configuration, API, or UI is an intended capability, not a vulnerability.
2. **The operator** owns deployment-time decisions (secrets, network exposure, feature-flag selection, connector and codec choices, notification destinations, third-party plugins). Misconfiguration at this layer is a deployment defect, not a Superset vulnerability.
3. **The codebase** is responsible for enforcing the role and capability matrix wherever it exposes functionality to a principal: API routes, command and DAO layers, UI handlers, background jobs, and any other entry point. A missing or incorrect enforcement check is in scope no matter where it lives.
The security model assumes that operator-controlled infrastructure, including the metadata database, cache backends, message brokers, secret stores, and deployment environment, remains within the operator's trust boundary. Vulnerabilities must demonstrate a security boundary violation by an attacker who does not already control those systems.
Route-level authorization in this codebase uses one of three Flask-AppBuilder decorators depending on the route type:
- `@protect()` for REST API routes (`ModelRestApi` / `BaseApi`)
- `@has_access_api` for legacy view routes
- `@has_access` for legacy HTML view routes
Object-level authorization via `security_manager.raise_for_access(...)` applies to data-bearing resources: dashboards, charts, datasets and datasources, queries, database and table access, and query contexts. Other resources (annotations, tags, CSS templates, reports, RLS rules, and similar) rely on the route-level decorator plus DAO `base_filters` for ownership scoping; the absence of `raise_for_access` on these resources is by design, not a finding. Code that omits the per-object gate on a route that returns or mutates a specific data-bearing object is in scope; code that follows the correct pattern for its resource class can still contain injection, SSRF, XSS, or other classes of finding unrelated to authorization, which are evaluated separately.
The full role and capability matrix, in-scope and out-of-scope class lists, and CVE aggregation rules are in [`SECURITY.md`](SECURITY.md). Defer to that document for any specifics.
**Requirements for findings filed by automated tooling**
Automated scanners (LLM-based code scanners, static analyzers, dependency tools) that file findings against this codebase must, in each finding, name:
1. The specific role and capability matrix row in [`SECURITY.md`](SECURITY.md) the finding believes is violated.
2. The principal the finding assumes the attacker holds (Public, Gamma, sql_lab, Alpha, Admin, Embedded guest token, or a custom role with explicit capability grants).
Findings that cannot identify both should be filed as questions, not vulnerabilities. This requirement exists to ensure every reported issue is testable against the published security model and to keep speculative or pattern-match-only reports out of the triage queue.
## Key Directories
```
@@ -165,7 +128,6 @@ The Developer Portal auto-generates MDX documentation from Storybook stories. **
## Architecture Patterns
### Security & Features
- **Security model**: see the top-level [Security and Threat Model](#security-and-threat-model) section and [`SECURITY.md`](SECURITY.md)
- **RBAC**: Role-based access via Flask-AppBuilder
- **Feature flags**: Control feature rollouts
- **Row-level security**: SQL-based data access control

View File

@@ -1,145 +0,0 @@
<!--
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.
-->
# Security Policy
This is a project of the [Apache Software Foundation](https://apache.org) and follows the
ASF [vulnerability handling process](https://apache.org/security/#vulnerability-handling).
## Reporting Vulnerabilities
**⚠️ Please do not file GitHub issues for security vulnerabilities as they are public! ⚠️**
Apache Software Foundation takes a rigorous standpoint in resolving the security issues
in its software projects. Apache Superset is highly sensitive and forthcoming to issues
pertaining to its features and functionality.
If you have any concern or believe you have found a vulnerability in Apache Superset,
please get in touch with the Apache Superset Security Team privately at
e-mail address [security@superset.apache.org](mailto:security@superset.apache.org).
More details can be found on the ASF website at
[ASF vulnerability reporting process](https://apache.org/security/#reporting-a-vulnerability)
**Submission Standards & AI Policy**
To ensure engineering focus remains on verified risks and to manage high reporting volumes, all reports must meet the following criteria:
- Plain Text Format: In accordance with Apache guidelines, please provide all details in plain text within the email body. Avoid sending PDFs, Word documents, or password-protected archives.
- Mandatory AI Disclosure: If you utilized Large Language Models (LLMs) or AI tools to identify a flaw or assist in writing a report, you must disclose this in your submission so our triage team can contextualize the findings.
- Human-Verified PoC: All submissions must include a manual, step-by-step Proof of Concept (PoC) performed on a supported release. Raw AI outputs, hypothetical chat transcripts, or unverified scanner logs will be closed as Invalid.
We kindly ask you to include the following information in your report to assist our developers in triaging and remediating issues efficiently:
- Version/Commit: The specific version of Apache Superset or the Git commit hash you are using.
- Configuration: A sanitized copy of your `superset_config.py` file or any config overrides.
- Environment: Your deployment method (e.g., Docker Compose, Helm, or source) and relevant OS/Browser details.
- Impacted Component: Identification of the affected area (e.g., Python backend, React frontend, or a specific database connector).
- Expected vs. Actual Behavior: A clear description of the intended system behavior versus the observed vulnerability.
- Detailed Reproduction Steps: Clear, manual steps to reproduce the vulnerability.
## Security Model
This section defines what Apache Superset considers a security issue and what it does not. It is the canonical reference for reporters, the Apache Superset Security Team, and any automated tool (LLM-based scanner, static analyzer, dependency tool) that needs to constrain its hypotheses to behaviors that genuinely violate the project's security policy.
The model is intentionally written in terms of principals, trust boundaries, and capability surface rather than in terms of specific files, functions, or libraries. New code paths inherit the model automatically.
### Trust Boundaries
Apache Superset's threat model assumes three trust boundaries.
1. *The Admin role* is a fully trusted operational principal. Anything an Admin can do through the documented user interface, REST API, or configuration system is an intended capability, not a vulnerability, even if individually powerful or destructive. The Admin role is, by policy, equivalent to operating-system-level trust over the Apache Superset application. This is unavoidable rather than aspirational: an Admin can, for example, register new database connections of arbitrary type, execute arbitrary SQL through SQL Lab, render Jinja templates that resolve to SQL or rendered HTML, and override application configuration. Granting Admin is functionally equivalent to granting shell access on the host, which is the reasoning behind treating it as a trust boundary in the sense of MITRE CNA Operational Rules 4.1.
2. *The operator* is whoever deploys, configures, and runs Apache Superset. Behaviors that depend on deployment-time decisions are the operator's responsibility, not Apache Superset's. This includes the values of secrets, the network reachability of the application and its data sources, the choice of database connectors and cache backends, the selection of feature flags, the destinations of notifications, and the trust placed in third-party plugins. Defaults that fail closed are the responsibility of the Apache Superset codebase. Defaults that fail open must be accompanied by a documented hardening requirement; applying that hardening is the operator's responsibility, while shipping an undocumented or unflagged fail-open default is a codebase issue.
3. *The Apache Superset codebase* is responsible for enforcing the role and capability matrix below across its product surface. A failure to enforce, anywhere in that surface, is in scope. The codebase's commitments are limited to the role and capability matrix and to controls Apache Superset's own documentation (this file and the linked Security documentation) explicitly positions as security boundaries; configurable hardening that operators can layer on top is treated separately under *Vulnerability Scope* below.
### Roles and Capabilities
Apache Superset ships with the following first-class principals. Detailed permission definitions live in the [Security documentation](https://superset.apache.org/docs/security).
| Principal | Read data | Write objects | Execute SQL | Manage databases | Manage users, roles, RLS |
|---|---|---|---|---|---|
| Public (anonymous) | none by default | no | no | no | no |
| Gamma | only granted datasets | own charts and dashboards on granted datasets | no by default (requires the `sql_lab` role) | no | no |
| Alpha | all data sources | own charts, dashboards, and datasets | no by default (requires the `sql_lab` role) | data upload to existing databases only | no |
| Admin | all | all | yes | yes | yes |
| Embedded guest token | data sources reachable through the embedded dashboards the token authorizes | no | no | no | no |
The `sql_lab` role is *additive*: it grants the SQL Lab permission set on top of the base role above, and is the only path by which Gamma or Alpha gain SQL execution capability. Database access is still scoped per the base role's grants. Admin includes SQL Lab access by default.
Deployments may grant or revoke individual view-menu permissions, which shifts the boundary for that deployment but does not redefine the model. Any custom role created by an operator inherits the same principle: its capabilities are whatever the operator has explicitly granted it. The Public principal follows the same rule: operators may grant the Public role read access to specific datasets or dashboards (typically for anonymous reporting use cases), which shifts the boundary for that deployment without redefining the model.
### Vulnerability Scope
The test for whether a finding is in scope is a single question:
> *Does this finding let a principal perform an action the role and capability matrix above does not entitle them to?*
If yes, it is in scope. If no, it is out of scope. The lists below apply that test to the classes Apache Superset most commonly receives reports about; they are illustrative, not exhaustive.
*In Scope*
- A user, embedded guest, or anonymous visitor reads, modifies, or deletes data outside their granted set. Includes object-level access bypass on charts, dashboards, datasets, saved queries, tags, annotations, and similar per-object endpoints, and row-level-security rule bypass.
- A user supplies input that the codebase should sanitize or parameterize but does not, causing arbitrary SQL, template code, or scripts to execute. Includes injection through Jinja templates, SQL-construction paths, and any field the codebase passes to a query engine or template engine.
- A user bypasses authentication, fixates or reuses another user's session, or reaches an authenticated endpoint without logging in.
- An embedded guest token authorizes actions outside the dashboard it was issued for, or can be forged, replayed, or escalated to a higher principal.
- Apache Superset, acting on behalf of an unprivileged user, fetches an outbound URL the user controls in a feature where Apache Superset itself, not the operator, controls the outbound destination set (server-side request forgery).
- An Apache Superset default fails open without an accompanying documented hardening requirement. The codebase is responsible for shipping fail-closed defaults or for documenting the hardening required when a default fails open; failures of that responsibility are in scope (see *Trust Boundaries*).
- A user bypasses a control Apache Superset documents specifically as a security boundary. This includes row-level security, the access checks tied to the role and capability matrix above, and any feature whose documentation positions it as security-relevant. The codebase commits to enforcing those controls; bypasses are in scope regardless of which principal triggers them.
- A user causes a script to execute in another user's browser through a field the codebase renders to that other user (cross-site scripting), or causes cross-origin leakage of authenticated session state or data.
- A user reaches a route, page, or API endpoint that requires a role they do not have.
*Out of Scope*
- Any action an Admin role can perform through documented configuration, API, or UI. The Admin role is a trusted operational principal by policy. Per MITRE CNA Operational Rules 4.1, a qualifying vulnerability must violate a security policy; behavior within a documented trust boundary does not.
- Deployment or operator decisions: the values of secrets and tokens, whether internal networks are reachable from the server, which database connectors or cache backends are enabled, which feature flags are set, where notifications are delivered, and which third-party plugins are loaded.
- Compromise, modification, or malicious control of trusted backend infrastructure. Apache Superset assumes the integrity of its metastore, cache backends (for example Redis or Memcached), message brokers, secret stores, and other operator-managed infrastructure. Findings that require an attacker to read from, write to, or otherwise tamper with these systems, including injecting malicious state, serialized objects, cache entries, task metadata, configuration, or database records, are post-compromise scenarios and do not constitute vulnerabilities in Apache Superset itself. A finding remains in scope only if an unprivileged user can cause such modification through a vulnerability in Apache Superset.
- Code paths whose intended purpose is example data, demos, fixtures, local development, or documentation, rather than the production runtime.
- How a downstream application (spreadsheet program, email client, browser handling user-downloaded files) interprets output Apache Superset produced for it.
- Findings without a reproducible proof of concept against a supported release. The burden of demonstrating exploitability rests with the reporter; findings closed for lack of a proof of concept may be refiled if one is later produced.
- Brute force, rate limiting, denial of service, or resource exhaustion that does not bypass a documented control.
- Missing security headers, banner or version disclosure, user or object enumeration through error messages or timing, and similar low-impact information disclosure that does not enable a further concrete exploit.
- Bypasses of configurable defense-in-depth hardening that Apache Superset does not document as a security boundary. Apache Superset is not a SQL or database firewall: operator-deployable filters such as SQL function or table denylists, URI restrictions on already-authorized database connectors, and similar belt-and-braces controls are provided to let operators layer hardening on top of the role and capability matrix, not as firewall-grade guarantees the codebase commits to. Findings against such hardening are improvements, not vulnerabilities, unless the documentation positions the specific control as security-relevant.
- Hardening suggestions that improve defense in depth but do not violate the security model.
Findings in third-party dependencies fall into two cases. A finding in a transitive dependency, or in an operator-selected dependency that Apache Superset does not ship, is out of scope and should be reported to the dependency's maintainers. A finding caused by Apache Superset pinning a known-vulnerable version of a direct dependency it ships, or using a dependency in a way that creates a vulnerability the dependency itself does not have, remains in scope. Dependency findings in the official Apache Superset Docker image that fall into the first case can be remediated by extending the image at release time.
When uncertain whether a finding falls in scope, please file it through the reporting process above. The triage team will classify it and explain the reasoning if it is closed as out of scope.
**Outcome of Reports**
Reports that are deemed out of scope for a CVE but represent valid security best practices or hardening opportunities are typically converted into public GitHub issues, where the community can contribute fixes alongside the maintainers. The triage decision and reasoning are communicated back to the reporter in either case.
**Vulnerability Aggregation & CVE Attribution**
In accordance with MITRE CNA Operational Rules (4.1.10, 4.1.11, and 4.2.13), Apache Superset issues CVEs based on the underlying architectural root cause rather than the number of affected endpoints or exploit payloads.
- Aggregation: If multiple exploit vectors stem from the same programmatic failure or shared vulnerable code, they must be aggregated into a single, comprehensive report.
- Independent Fixes: Separate CVEs will only be assigned if the vulnerabilities reside in decoupled architectural modules and can be fixed independently of one another.
Reports that fail to aggregate related findings will be merged during triage to ensure an accurate and defensible CVE record.
**Your responsible disclosure and collaboration are invaluable.**
## Extra Information
- [Apache Superset documentation](https://superset.apache.org/docs/security)
- [Common Vulnerabilities and Exposures by release](https://superset.apache.org/docs/security/cves)
- [How Security Vulnerabilities are Reported & Handled in Apache Superset (Blog)](https://preset.io/blog/how-security-vulnerabilities-are-reported-and-handled-in-apache-superset/)
**For LLM agents and automated scanners**
This file is the canonical source of truth for what Apache Superset considers a security vulnerability. Automated tooling (LLM-based code scanners, static analyzers, dependency tools) should treat the Security Model section as authoritative when classifying findings. The repository's [AGENTS.md](AGENTS.md) file contains a short pointer to this document for agents that read AGENTS.md as their entry point.

View File

@@ -132,9 +132,7 @@
"lodash": "4.18.1",
"lodash-es": "4.18.1",
"yaml": "1.10.3",
"uuid": "11.1.1",
"serialize-javascript": "7.0.5",
"d3-color": "3.1.0"
"uuid": "11.1.1"
},
"packageManager": "yarn@1.22.22+sha1.ac34549e6aa8e7ead463a7407e1c7390f61a6610"
}

View File

@@ -6538,9 +6538,14 @@ d3-chord@3:
dependencies:
d3-path "1 - 3"
"d3-color@1 - 2", "d3-color@1 - 3", d3-color@3, d3-color@3.1.0:
"d3-color@1 - 2":
version "2.0.0"
resolved "https://registry.npmjs.org/d3-color/-/d3-color-2.0.0.tgz"
integrity sha512-SPXi0TSKPD4g9tw0NMZFnR95XVgUZiBH+uUTqQuDu1OsE2zomHU7ho0FISciaPvosimixwHFl3WHLGabv6dDgQ==
"d3-color@1 - 3", d3-color@3:
version "3.1.0"
resolved "https://registry.yarnpkg.com/d3-color/-/d3-color-3.1.0.tgz#395b2833dfac71507f12ac2f7af23bf819de24e2"
resolved "https://registry.npmjs.org/d3-color/-/d3-color-3.1.0.tgz"
integrity sha512-zg/chbXyeBtMQ1LbD/WSoW2DpC3I0mpmPdW+ynRTj/x2DAWYrIY7qeZIHidozwV24m4iavr15lNwIwLxRmOxhA==
d3-contour@4:
@@ -13357,10 +13362,12 @@ serialize-error@^8.1.0:
dependencies:
type-fest "^0.20.2"
serialize-javascript@7.0.5, serialize-javascript@^6.0.0, serialize-javascript@^6.0.1:
version "7.0.5"
resolved "https://registry.yarnpkg.com/serialize-javascript/-/serialize-javascript-7.0.5.tgz#c798cc0552ffbb08981914a42a8756e339d0d5b1"
integrity sha512-F4LcB0UqUl1zErq+1nYEEzSHJnIwb3AF2XWB94b+afhrekOUijwooAYqFyRbjYkm2PAKBabx6oYv/xDxNi8IBw==
serialize-javascript@^6.0.0, serialize-javascript@^6.0.1:
version "6.0.2"
resolved "https://registry.yarnpkg.com/serialize-javascript/-/serialize-javascript-6.0.2.tgz#defa1e055c83bf6d59ea805d8da862254eb6a6c2"
integrity sha512-Saa1xPByTTq2gdeFZYLLo+RFE35NHZkAbqZeWNd3BpzppeVisAqpDjcp8dyf6uIvEqJRd46jemmyA4iFIeVk8g==
dependencies:
randombytes "^2.1.0"
serve-handler@^6.1.7:
version "6.1.7"

View File

@@ -71,7 +71,7 @@ dependencies = [
"marshmallow>=3.0, <4",
"marshmallow-union>=0.1",
"msgpack>=1.0.0, <1.2",
"nh3>=0.3.5, <0.4",
"nh3>=0.2.11, <0.4",
"numpy>1.23.5, <2.3",
"packaging",
# --------------------------
@@ -123,7 +123,7 @@ bigquery = [
]
clickhouse = ["clickhouse-connect>=0.13.0, <2.0"]
cockroachdb = ["cockroachdb>=0.3.5, <0.4"]
crate = ["sqlalchemy-cratedb>=0.41.0, <1"]
crate = ["sqlalchemy-cratedb>=0.40.1, <1"]
d1 = [
"superset-engine-d1>=0.1.0",
"sqlalchemy-d1>=0.1.0",
@@ -135,7 +135,7 @@ databricks = [
"databricks-sqlalchemy==1.0.5",
]
db2 = ["ibm-db-sa>0.3.8, <=0.4.4"]
denodo = ["denodo-sqlalchemy>=2.0.5,<2.1.0"]
denodo = ["denodo-sqlalchemy>=1.0.6,<2.1.0"]
dremio = ["sqlalchemy-dremio>=1.2.1, <4"]
drill = ["sqlalchemy-drill>=1.1.10, <2"]
druid = ["pydruid>=0.6.5,<0.7"]
@@ -190,7 +190,7 @@ risingwave = ["sqlalchemy-risingwave"]
shillelagh = ["shillelagh[all]>=1.4.4, <2"]
singlestore = ["sqlalchemy-singlestoredb>=1.1.1, <2"]
snowflake = ["snowflake-sqlalchemy>=1.2.4, <2"]
sqlite = ["syntaqlite>=0.1.0,<0.5.0"]
sqlite = ["syntaqlite>=0.1.0"]
spark = [
"pyhive[hive]>=0.6.5;python_version<'3.11'",
"pyhive[hive_pure_sasl]>=0.7",
@@ -235,7 +235,7 @@ development = [
"ruff",
"sqloxide",
"statsd",
"syntaqlite>=0.4.2,<0.5.0",
"syntaqlite>=0.1.0",
]
[project.urls]

View File

@@ -176,7 +176,7 @@ holidays==0.82
# via apache-superset (pyproject.toml)
humanize==4.12.3
# via apache-superset (pyproject.toml)
idna==3.15
idna==3.10
# via
# email-validator
# requests
@@ -241,7 +241,7 @@ msgpack==1.0.8
# via apache-superset (pyproject.toml)
msgspec==0.19.0
# via flask-session
nh3==0.3.5
nh3==0.2.21
# via apache-superset (pyproject.toml)
numexpr==2.10.2
# via -r requirements/base.in

View File

@@ -52,7 +52,7 @@ attrs==25.3.0
# referencing
# requests-cache
# trio
authlib==1.6.12
authlib==1.6.9
# via fastmcp
babel==2.17.0
# via
@@ -317,7 +317,7 @@ flask-wtf==1.2.2
# -c requirements/base-constraint.txt
# apache-superset
# flask-appbuilder
fonttools==4.60.2
fonttools==4.55.0
# via matplotlib
freezegun==1.5.1
# via apache-superset
@@ -422,7 +422,7 @@ humanize==4.12.3
# apache-superset
identify==2.5.36
# via pre-commit
idna==3.15
idna==3.10
# via
# -c requirements/base-constraint.txt
# anyio
@@ -566,7 +566,7 @@ msgspec==0.19.0
# flask-session
mysqlclient==2.2.6
# via apache-superset
nh3==0.3.5
nh3==0.2.21
# via
# -c requirements/base-constraint.txt
# apache-superset
@@ -838,7 +838,7 @@ python-dotenv==1.2.2
# apache-superset
# fastmcp
# pydantic-settings
python-ldap==3.4.5
python-ldap==3.4.4
# via apache-superset
python-multipart==0.0.29
# via mcp
@@ -1005,7 +1005,7 @@ starlette==0.49.1
# via mcp
statsd==4.0.1
# via apache-superset
syntaqlite==0.4.2
syntaqlite==0.1.0
# via apache-superset
tabulate==0.10.0
# via
@@ -1090,7 +1090,7 @@ vine==5.1.0
# amqp
# celery
# kombu
virtualenv==20.36.1
virtualenv==20.29.2
# via pre-commit
watchdog==6.0.0
# via

View File

@@ -0,0 +1,24 @@
/*
* 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.
*/
module.exports = {
// @superset-ui/switchboard ships ES module syntax, so it must be
// transformed by babel rather than ignored as a node_modules dependency.
transformIgnorePatterns: ["/node_modules/(?!@superset-ui/switchboard)"],
};

View File

@@ -0,0 +1,91 @@
/**
* 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 {
validateEmbeddedDashboardId,
validateSupersetDomain,
findUnsafeSandboxExtras,
} from "./index";
describe("validateEmbeddedDashboardId", () => {
it("accepts a canonical uuid", () => {
expect(() =>
validateEmbeddedDashboardId("f4787a4f-2541-4f8a-9b5e-1e2d3c4b5a6f")
).not.toThrow();
});
it("accepts a simple hexadecimal id", () => {
expect(() => validateEmbeddedDashboardId("abc123")).not.toThrow();
});
it.each([
["../../evil"],
["a/b"],
["x?foo=bar"],
["x#frag"],
["a@b.com"],
["foo bar"],
["http://evil.com"],
[""],
["%2e%2e"],
])("rejects an id with an unexpected format: %p", (id) => {
expect(() => validateEmbeddedDashboardId(id)).toThrow();
});
});
describe("validateSupersetDomain", () => {
it.each([
["https://superset.example.com"],
["http://localhost:8088"],
// sub-path deployments are valid; the origin is what matters downstream
["https://example.com/superset"],
])("accepts a valid absolute URL: %p", (domain) => {
expect(() => validateSupersetDomain(domain)).not.toThrow();
});
it.each([
["superset.example.com"], // missing protocol
["not a url"],
[""],
["/relative/path"],
])("rejects a malformed domain: %p", (domain) => {
expect(() => validateSupersetDomain(domain)).toThrow(
"Invalid supersetDomain format"
);
});
});
describe("findUnsafeSandboxExtras", () => {
it("returns the tokens that relax iframe isolation", () => {
expect(
findUnsafeSandboxExtras([
"allow-forms",
"allow-top-navigation",
"allow-popups",
"allow-top-navigation-by-user-activation",
])
).toEqual(["allow-top-navigation", "allow-top-navigation-by-user-activation"]);
});
it("returns an empty array when all tokens are safe", () => {
expect(
findUnsafeSandboxExtras(["allow-forms", "allow-popups", "allow-downloads"])
).toEqual([]);
});
});

View File

@@ -50,7 +50,9 @@ export type UiConfigType = {
};
export type EmbedDashboardParams = {
/** The id provided by the embed configuration UI in Superset */
/** The id provided by the embed configuration UI in Superset.
* This is the UUID generated by Superset's embed configuration and is
* expected to contain only hexadecimal characters and hyphens. */
id: string;
/** The domain where Superset can be located, with protocol, such as: https://superset.example.com */
supersetDomain: string;
@@ -112,6 +114,48 @@ export type EmbeddedDashboard = {
setThemeMode: (mode: ThemeMode) => void;
};
/**
* Validates that an embedded dashboard id has the expected format
* (the UUID produced by Superset's embed configuration). Throws on
* anything containing characters that are not part of that format.
*/
export function validateEmbeddedDashboardId(id: string): void {
if (typeof id !== 'string' || !/^[a-f0-9-]+$/i.test(id)) {
throw new Error('Invalid dashboard id format');
}
}
/**
* Validates that supersetDomain is a parseable absolute URL (it must include
* a protocol, e.g. https://superset.example.com). Throws otherwise. The
* domain's origin is what gets used as the postMessage targetOrigin, so it
* has to resolve to a well-formed origin.
*/
export function validateSupersetDomain(supersetDomain: string): void {
try {
// eslint-disable-next-line no-new
new URL(supersetDomain);
} catch {
throw new Error('Invalid supersetDomain format');
}
}
// Sandbox tokens that materially relax the iframe's isolation (for example,
// letting the embedded frame navigate the top-level page). They remain
// supported via iframeSandboxExtras for callers that genuinely need them.
const UNSAFE_SANDBOX_EXTRAS = [
'allow-top-navigation',
'allow-top-navigation-by-user-activation',
];
/**
* Returns any caller-provided sandbox tokens that relax the iframe's
* isolation, so they can be surfaced and not enabled unintentionally.
*/
export function findUnsafeSandboxExtras(extras: string[]): string[] {
return extras.filter(token => UNSAFE_SANDBOX_EXTRAS.includes(token));
}
/**
* Embeds a Superset dashboard into the page using an iframe.
*/
@@ -128,6 +172,8 @@ export async function embedDashboard({
referrerPolicy,
resolvePermalinkUrl,
}: EmbedDashboardParams): Promise<EmbeddedDashboard> {
validateEmbeddedDashboardId(id);
function log(...info: unknown[]) {
if (debug) {
console.debug(`[superset-embedded-sdk][dashboard ${id}]`, ...info);
@@ -139,6 +185,7 @@ export async function embedDashboard({
if (supersetDomain.endsWith('/')) {
supersetDomain = supersetDomain.slice(0, -1);
}
validateSupersetDomain(supersetDomain);
function calculateConfig() {
let configNumber = 0;
@@ -195,6 +242,13 @@ export async function embedDashboard({
iframe.sandbox.add('allow-forms'); // for forms to submit
iframe.sandbox.add('allow-popups'); // for exporting charts as csv
// additional sandbox props
const unsafeSandboxExtras = findUnsafeSandboxExtras(iframeSandboxExtras);
if (unsafeSandboxExtras.length > 0) {
console.warn(
'[superset-embedded-sdk] iframeSandboxExtras includes tokens that ' +
`relax the iframe's isolation: ${unsafeSandboxExtras.join(', ')}`,
);
}
iframeSandboxExtras.forEach((key: string) => {
iframe.sandbox.add(key);
});
@@ -216,7 +270,9 @@ export async function embedDashboard({
// we know the content window isn't null because we are in the load event handler.
iframe.contentWindow!.postMessage(
{ type: IFRAME_COMMS_MESSAGE_TYPE, handshake: 'port transfer' },
supersetDomain,
// Use the normalized origin (not the raw domain, which may carry a
// sub-path for sub-path deployments) as the postMessage targetOrigin.
new URL(supersetDomain).origin,
[theirPort],
);
log('sent message channel to the iframe');

View File

@@ -1,85 +0,0 @@
/**
* 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.
*/
/**
* Tests for the color threshold formatter logic in BigNumberViz.
*
* The key fix: bigNumber === 0 is falsy, so the original code
* `const result = bigNumber ? formatter.getColorFromValue(bigNumber) : false`
* would skip formatting for zero. The fix uses an explicit type check instead.
*/
// describe block makes applyColorFormatters block-scoped, avoiding TS2451
// when TypeScript's root tsconfig includes this file as a global script.
describe('BigNumberViz color formatters', () => {
const applyColorFormatters = (
bigNumber: number | null | undefined,
formatters: Array<{ getColorFromValue: (v: number) => string | undefined }>,
): string | undefined => {
let numberColor: string | undefined;
const hasFormatters = Array.isArray(formatters) && formatters.length > 0;
if (hasFormatters) {
formatters.forEach(formatter => {
// Fixed: use explicit type check instead of falsy check
if (typeof bigNumber === 'number' && !isNaN(bigNumber)) {
numberColor = formatter.getColorFromValue(bigNumber);
}
});
}
return numberColor;
};
test('applies color formatter when bigNumber is 0', () => {
const getColorFromValue = jest.fn(() => 'red');
const color = applyColorFormatters(0, [{ getColorFromValue }]);
expect(getColorFromValue).toHaveBeenCalledWith(0);
expect(color).toBe('red');
});
test('applies color formatter when bigNumber is positive', () => {
const getColorFromValue = jest.fn(() => 'green');
const color = applyColorFormatters(42, [{ getColorFromValue }]);
expect(getColorFromValue).toHaveBeenCalledWith(42);
expect(color).toBe('green');
});
test('applies color formatter when bigNumber is negative', () => {
const getColorFromValue = jest.fn(() => 'blue');
const color = applyColorFormatters(-5, [{ getColorFromValue }]);
expect(getColorFromValue).toHaveBeenCalledWith(-5);
expect(color).toBe('blue');
});
test('does not call color formatter when bigNumber is null', () => {
const getColorFromValue = jest.fn();
applyColorFormatters(null, [{ getColorFromValue }]);
expect(getColorFromValue).not.toHaveBeenCalled();
});
test('does not call color formatter when bigNumber is undefined', () => {
const getColorFromValue = jest.fn();
applyColorFormatters(undefined, [{ getColorFromValue }]);
expect(getColorFromValue).not.toHaveBeenCalled();
});
});

View File

@@ -204,8 +204,11 @@ function BigNumberVis({
let numberColor;
if (hasThresholdColorFormatter) {
colorThresholdFormatters!.forEach(formatter => {
if (typeof bigNumber === 'number' && !isNaN(bigNumber)) {
numberColor = formatter.getColorFromValue(bigNumber);
const formatterResult = bigNumber
? formatter.getColorFromValue(bigNumber as number)
: false;
if (formatterResult) {
numberColor = formatterResult;
}
});
} else {

View File

@@ -17,39 +17,20 @@
* under the License.
*/
import { FC, memo, useCallback, useEffect, useMemo, useState } from 'react';
import { FC, memo, useMemo } from 'react';
import { t } from '@apache-superset/core/translation';
import {
DataMaskStateWithId,
QueryObjectFilterClause,
} from '@superset-ui/core';
import { DataMaskStateWithId } from '@superset-ui/core';
import { styled } from '@apache-superset/core/theme';
import { Loading } from '@superset-ui/core/components';
import { Icons } from '@superset-ui/core/components/Icons';
import { FilterBarOrientation, RootState } from 'src/dashboard/types';
import { RootState } from 'src/dashboard/types';
import { useChartLayoutItems } from 'src/dashboard/util/useChartLayoutItems';
import { useChartIds } from 'src/dashboard/util/charts/useChartIds';
import { useDispatch, useSelector } from 'react-redux';
import { useHistory, useLocation } from 'react-router-dom';
import { removeDataMask, updateDataMask } from 'src/dataMask/actions';
import {
getRisonFilterParam,
parseRisonFilters,
RISON_UNMATCHED_DATAMASK_ID,
risonFiltersToExtraFormDataFilters,
updateUrlWithUnmatchedFilters,
} from 'src/dashboard/util/risonFilters';
import { useSelector } from 'react-redux';
import FilterControls from './FilterControls/FilterControls';
import { useChartsVerboseMaps, getFilterBarTestId } from './utils';
import { HorizontalBarProps } from './types';
import FilterBarSettings from './FilterBarSettings';
import crossFiltersSelector from './CrossFilters/selectors';
import {
getUrlFilterIndicators,
getUrlFilterIdentity,
UrlFilterIndicator,
} from './UrlFilters/urlFilterUtils';
import UrlFilterTag from './UrlFilters/UrlFilterTag';
const HorizontalBar = styled.div`
${({ theme }) => `
@@ -84,28 +65,6 @@ const FilterBarEmptyStateContainer = styled.div`
`}
`;
const UrlFiltersContainer = styled.div`
${({ theme }) => `
display: flex;
flex-direction: row;
align-items: center;
gap: ${theme.sizeUnit * 2}px;
padding: 0 ${theme.sizeUnit * 2}px;
margin-right: ${theme.sizeUnit * 2}px;
border-right: 1px solid ${theme.colorBorder};
`}
`;
const UrlFilterTitle = styled.div`
${({ theme }) => `
display: flex;
align-items: center;
gap: ${theme.sizeUnit}px;
font-weight: ${theme.fontWeightStrong};
font-size: ${theme.fontSizeSM}px;
`}
`;
const HorizontalFilterBar: FC<HorizontalBarProps> = ({
actions,
dataMaskSelected,
@@ -120,9 +79,6 @@ const HorizontalFilterBar: FC<HorizontalBarProps> = ({
const dataMask = useSelector<RootState, DataMaskStateWithId>(
state => state.dataMask,
);
const dispatch = useDispatch();
const history = useHistory();
const location = useLocation();
const chartIds = useChartIds();
const chartLayoutItems = useChartLayoutItems();
const verboseMaps = useChartsVerboseMaps();
@@ -138,71 +94,9 @@ const HorizontalFilterBar: FC<HorizontalBarProps> = ({
[chartIds, chartLayoutItems, dataMask, verboseMaps],
);
const [activeUrlFilters, setActiveUrlFilters] = useState<
UrlFilterIndicator[]
>(() => getUrlFilterIndicators());
// Re-read chips whenever the URL changes (back/forward navigation, or a
// programmatic history.replace).
useEffect(() => {
setActiveUrlFilters(getUrlFilterIndicators());
}, [location.search]);
const handleRemoveUrlFilter = useCallback(
(filterToRemove: UrlFilterIndicator) => {
const risonParam = getRisonFilterParam();
if (!risonParam) return;
const removeId = getUrlFilterIdentity(filterToRemove.filter);
const currentFilters = parseRisonFilters(risonParam);
const remaining = currentFilters.filter(
f => getUrlFilterIdentity(f) !== removeId,
);
updateUrlWithUnmatchedFilters(remaining, history);
setActiveUrlFilters(prev =>
prev.filter(f => getUrlFilterIdentity(f.filter) !== removeId),
);
if (remaining.length === 0) {
dispatch(removeDataMask(RISON_UNMATCHED_DATAMASK_ID));
} else {
const extraFormDataFilters: QueryObjectFilterClause[] =
risonFiltersToExtraFormDataFilters(remaining);
dispatch(
updateDataMask(RISON_UNMATCHED_DATAMASK_ID, {
extraFormData: { filters: extraFormDataFilters },
}),
);
}
},
[dispatch, history],
);
const urlFiltersComponent = useMemo(() => {
if (activeUrlFilters.length === 0) return null;
return (
<UrlFiltersContainer>
<UrlFilterTitle>
<Icons.LinkOutlined iconSize="s" />
{t('URL Filters')}
</UrlFilterTitle>
{activeUrlFilters.map(filter => (
<UrlFilterTag
key={getUrlFilterIdentity(filter.filter)}
filter={filter}
orientation={FilterBarOrientation.Horizontal}
onRemove={handleRemoveUrlFilter}
/>
))}
</UrlFiltersContainer>
);
}, [activeUrlFilters, handleRemoveUrlFilter]);
const hasFilters =
filterValues.length > 0 ||
selectedCrossFilters.length > 0 ||
activeUrlFilters.length > 0 ||
chartCustomizationValues.length > 0;
return (
@@ -219,19 +113,16 @@ const HorizontalFilterBar: FC<HorizontalBarProps> = ({
</FilterBarEmptyStateContainer>
)}
{hasFilters && (
<>
{urlFiltersComponent}
<FilterControls
dataMaskSelected={dataMaskSelected}
onFilterSelectionChange={onSelectionChange}
onPendingCustomizationDataMaskChange={
onPendingCustomizationDataMaskChange
}
chartCustomizationValues={chartCustomizationValues}
clearAllTriggers={clearAllTriggers}
onClearAllComplete={onClearAllComplete}
/>
</>
<FilterControls
dataMaskSelected={dataMaskSelected}
onFilterSelectionChange={onSelectionChange}
onPendingCustomizationDataMaskChange={
onPendingCustomizationDataMaskChange
}
chartCustomizationValues={chartCustomizationValues}
clearAllTriggers={clearAllTriggers}
onClearAllComplete={onClearAllComplete}
/>
)}
{actions}
</>

View File

@@ -36,7 +36,6 @@ const renderWrapper = (overrideProps?: Record<string, any>) =>
waitFor(() =>
render(<HorizontalBar {...defaultProps} {...overrideProps} />, {
useRedux: true,
useRouter: true,
initialState: {
dashboardState: {
sliceIds: [],

View File

@@ -1,84 +0,0 @@
/**
* 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 { useCSSTextTruncation } from '@superset-ui/core';
import { styled, css, useTheme } from '@apache-superset/core/theme';
import { Tag } from 'src/components/Tag';
import { Tooltip } from '@superset-ui/core/components';
import { FilterBarOrientation } from 'src/dashboard/types';
import { ellipsisCss } from '../CrossFilters/styles';
import { UrlFilterIndicator } from './urlFilterUtils';
const StyledValue = styled.b`
${({ theme }) => `
max-width: ${theme.sizeUnit * 25}px;
`}
${ellipsisCss}
`;
const StyledColumn = styled('span')`
${({ theme }) => `
max-width: ${theme.sizeUnit * 25}px;
padding-right: ${theme.sizeUnit}px;
`}
${ellipsisCss}
`;
const StyledTag = styled(Tag)`
${({ theme }) => `
border: 1px solid ${theme.colorBorder};
border-radius: 2px;
.anticon-close {
vertical-align: middle;
}
`}
`;
const UrlFilterTag = (props: {
filter: UrlFilterIndicator;
orientation: FilterBarOrientation;
onRemove: (filter: UrlFilterIndicator) => void;
}) => {
const { filter, orientation, onRemove } = props;
const theme = useTheme();
const [columnRef, columnIsTruncated] =
useCSSTextTruncation<HTMLSpanElement>();
const [valueRef, valueIsTruncated] = useCSSTextTruncation<HTMLSpanElement>();
return (
<StyledTag
css={css`
${orientation === FilterBarOrientation.Vertical
? `margin-top: ${theme.sizeUnit * 2}px;`
: `margin-left: ${theme.sizeUnit * 2}px;`}
`}
closable
onClose={() => onRemove(filter)}
>
<Tooltip title={columnIsTruncated ? filter.subject : null}>
<StyledColumn ref={columnRef}>{filter.subject}</StyledColumn>
</Tooltip>
<Tooltip title={valueIsTruncated ? filter.value : null}>
<StyledValue ref={valueRef}>{filter.value}</StyledValue>
</Tooltip>
</StyledTag>
);
};
export default UrlFilterTag;

View File

@@ -1,139 +0,0 @@
/**
* 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.
*/
/**
* Validates PR review items #2 and #7:
* - chip removal must dispatch a dataMask update so charts re-query
* (otherwise the URL changes but the synthetic Rison filter keeps
* influencing chart results until full reload).
* - the chip list must react to URL changes (back/forward navigation or
* a programmatic history.replace), not snapshot the URL at mount.
*/
import { createMemoryHistory } from 'history';
import { Router } from 'react-router-dom';
import { act, render, screen, userEvent } from 'spec/helpers/testing-library';
import { REMOVE_DATA_MASK, UPDATE_DATA_MASK } from 'src/dataMask/actions';
import { RISON_UNMATCHED_DATAMASK_ID } from 'src/dashboard/util/risonFilters';
import UrlFiltersVertical from './Vertical';
const mockDispatch = jest.fn();
jest.mock('react-redux', () => ({
...jest.requireActual('react-redux'),
useDispatch: () => mockDispatch,
}));
const seedUrl = (search: string) => {
// jsdom doesn't navigate, so set both window.location (read by
// getRisonFilterParam) and react-router's in-memory history.
window.history.replaceState({}, '', `/superset/dashboard/1/${search}`);
};
const renderAt = (search: string) => {
seedUrl(search);
const history = createMemoryHistory({
initialEntries: [`/superset/dashboard/1/${search}`],
});
const utils = render(
<Router history={history}>
<UrlFiltersVertical />
</Router>,
{ useRedux: true },
);
return { ...utils, history };
};
beforeEach(() => {
mockDispatch.mockReset();
window.history.replaceState({}, '', '/');
});
test('renders chips parsed from the f= URL param', () => {
renderAt('?f=(region:EMEA,channel:web)');
expect(screen.getByText('URL Filters')).toBeInTheDocument();
expect(screen.getByText('region')).toBeInTheDocument();
expect(screen.getByText('EMEA')).toBeInTheDocument();
expect(screen.getByText('channel')).toBeInTheDocument();
expect(screen.getByText('web')).toBeInTheDocument();
});
test('renders nothing when there is no f= param', () => {
renderAt('');
expect(screen.queryByText('URL Filters')).not.toBeInTheDocument();
});
test('removing a chip dispatches updateDataMask with the remaining filters', async () => {
renderAt('?f=(region:EMEA,channel:web)');
const closeButtons = screen.getAllByRole('img', { name: /close/i });
expect(closeButtons).toHaveLength(2);
await userEvent.click(closeButtons[0]);
// The remaining filter must still apply to charts, so updateDataMask
// (not removeDataMask) is dispatched with one filter left.
const updateCalls = mockDispatch.mock.calls
.map(([action]) => action)
.filter(action => action?.type === UPDATE_DATA_MASK);
expect(updateCalls).toHaveLength(1);
expect(updateCalls[0].filterId).toBe(RISON_UNMATCHED_DATAMASK_ID);
expect(updateCalls[0].dataMask.extraFormData.filters).toHaveLength(1);
expect(updateCalls[0].dataMask.extraFormData.filters[0]).toMatchObject({
col: 'channel',
val: ['web'],
});
// The chip we clicked is gone from the UI.
expect(screen.queryByText('region')).not.toBeInTheDocument();
});
test('removing the last chip dispatches removeDataMask, not an empty update', async () => {
renderAt('?f=(region:EMEA)');
await userEvent.click(screen.getByRole('img', { name: /close/i }));
const removeCalls = mockDispatch.mock.calls
.map(([action]) => action)
.filter(action => action?.type === REMOVE_DATA_MASK);
expect(removeCalls).toHaveLength(1);
expect(removeCalls[0].filterId).toBe(RISON_UNMATCHED_DATAMASK_ID);
// No stray updateDataMask with empty filters.
const updateCalls = mockDispatch.mock.calls
.map(([action]) => action)
.filter(action => action?.type === UPDATE_DATA_MASK);
expect(updateCalls).toHaveLength(0);
});
test('chip list re-renders when the URL changes (popstate/programmatic nav)', () => {
const { history } = renderAt('?f=(region:EMEA)');
expect(screen.getByText('region')).toBeInTheDocument();
expect(screen.queryByText('priority')).not.toBeInTheDocument();
// Simulate a programmatic URL change (e.g. via history.push or a back/
// forward nav). The component must re-read the URL filters.
act(() => {
seedUrl('?f=(priority:high)');
history.replace('/superset/dashboard/1/?f=(priority:high)');
});
expect(screen.getByText('priority')).toBeInTheDocument();
expect(screen.getByText('high')).toBeInTheDocument();
expect(screen.queryByText('region')).not.toBeInTheDocument();
});

View File

@@ -1,96 +0,0 @@
/**
* 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 { useCallback, useEffect, useState } from 'react';
import { useDispatch } from 'react-redux';
import { useHistory, useLocation } from 'react-router-dom';
import { QueryObjectFilterClause } from '@superset-ui/core';
import { removeDataMask, updateDataMask } from 'src/dataMask/actions';
import {
getRisonFilterParam,
parseRisonFilters,
RISON_UNMATCHED_DATAMASK_ID,
risonFiltersToExtraFormDataFilters,
updateUrlWithUnmatchedFilters,
} from 'src/dashboard/util/risonFilters';
import {
getUrlFilterIdentity,
getUrlFilterIndicators,
UrlFilterIndicator,
} from './urlFilterUtils';
import UrlFiltersVerticalCollapse from './VerticalCollapse';
const UrlFiltersVertical = () => {
const dispatch = useDispatch();
const history = useHistory();
const location = useLocation();
const [urlFilters, setUrlFilters] = useState<UrlFilterIndicator[]>(() =>
getUrlFilterIndicators(),
);
// Re-read chips whenever the URL changes (back/forward navigation, or a
// programmatic history.replace).
useEffect(() => {
setUrlFilters(getUrlFilterIndicators());
}, [location.search]);
const handleRemoveFilter = useCallback(
(filterToRemove: UrlFilterIndicator) => {
const risonParam = getRisonFilterParam();
if (!risonParam) return;
const removeId = getUrlFilterIdentity(filterToRemove.filter);
const currentFilters = parseRisonFilters(risonParam);
const remaining = currentFilters.filter(
f => getUrlFilterIdentity(f) !== removeId,
);
updateUrlWithUnmatchedFilters(remaining, history);
setUrlFilters(prev =>
prev.filter(f => getUrlFilterIdentity(f.filter) !== removeId),
);
if (remaining.length === 0) {
dispatch(removeDataMask(RISON_UNMATCHED_DATAMASK_ID));
} else {
const extraFormDataFilters: QueryObjectFilterClause[] =
risonFiltersToExtraFormDataFilters(remaining);
dispatch(
updateDataMask(RISON_UNMATCHED_DATAMASK_ID, {
extraFormData: { filters: extraFormDataFilters },
}),
);
}
},
[dispatch, history],
);
if (!urlFilters.length) {
return null;
}
return (
<UrlFiltersVerticalCollapse
urlFilters={urlFilters}
onRemoveFilter={handleRemoveFilter}
/>
);
};
export default UrlFiltersVertical;

View File

@@ -1,131 +0,0 @@
/**
* 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 { useCallback, useMemo, useState } from 'react';
import { t } from '@apache-superset/core/translation';
import { css, useTheme, SupersetTheme } from '@apache-superset/core/theme';
import { Icons } from '@superset-ui/core/components/Icons';
import { FilterBarOrientation } from 'src/dashboard/types';
import UrlFilterTag from './UrlFilterTag';
import { UrlFilterIndicator, getUrlFilterIdentity } from './urlFilterUtils';
const sectionContainerStyle = (theme: SupersetTheme) => css`
margin-bottom: ${theme.sizeUnit * 3}px;
padding: 0 ${theme.sizeUnit * 4}px;
`;
const sectionHeaderStyle = (theme: SupersetTheme) => css`
display: flex;
align-items: center;
justify-content: space-between;
padding: ${theme.sizeUnit * 2}px 0;
cursor: pointer;
user-select: none;
&:hover {
background: ${theme.colorBgTextHover};
margin: 0 -${theme.sizeUnit * 2}px;
padding: ${theme.sizeUnit * 2}px;
border-radius: ${theme.borderRadius}px;
}
`;
const sectionTitleStyle = (theme: SupersetTheme) => css`
margin: 0;
font-size: ${theme.fontSize}px;
font-weight: ${theme.fontWeightStrong};
color: ${theme.colorText};
line-height: 1.3;
display: flex;
align-items: center;
gap: ${theme.sizeUnit}px;
`;
const sectionContentStyle = (theme: SupersetTheme) => css`
padding: ${theme.sizeUnit * 2}px 0;
`;
const dividerStyle = (theme: SupersetTheme) => css`
height: 1px;
background: ${theme.colorSplit};
margin: ${theme.sizeUnit * 2}px 0;
`;
const iconStyle = (open: boolean, theme: SupersetTheme) => css`
transform: ${open ? 'rotate(0deg)' : 'rotate(180deg)'};
transition: transform 0.2s ease;
color: ${theme.colorTextSecondary};
`;
const UrlFiltersVerticalCollapse = (props: {
urlFilters: UrlFilterIndicator[];
onRemoveFilter: (filter: UrlFilterIndicator) => void;
}) => {
const { urlFilters, onRemoveFilter } = props;
const theme = useTheme();
const [isOpen, setIsOpen] = useState(true);
const toggleSection = useCallback(() => {
setIsOpen(prev => !prev);
}, []);
const filterIndicators = useMemo(
() =>
urlFilters.map(filter => (
<UrlFilterTag
key={getUrlFilterIdentity(filter.filter)}
filter={filter}
orientation={FilterBarOrientation.Vertical}
onRemove={onRemoveFilter}
/>
)),
[urlFilters, onRemoveFilter],
);
if (!urlFilters.length) {
return null;
}
return (
<div css={sectionContainerStyle}>
<div
css={sectionHeaderStyle}
onClick={toggleSection}
onKeyDown={e => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
toggleSection();
}
}}
role="button"
tabIndex={0}
>
<h4 css={sectionTitleStyle}>
<Icons.LinkOutlined iconSize="s" />
{t('URL Filters')}
</h4>
<Icons.UpOutlined iconSize="m" css={iconStyle(isOpen, theme)} />
</div>
{isOpen && <div css={sectionContentStyle}>{filterIndicators}</div>}
{isOpen && <div css={dividerStyle} data-test="url-filters-divider" />}
</div>
);
};
export default UrlFiltersVerticalCollapse;

View File

@@ -1,71 +0,0 @@
/**
* 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 {
getRisonFilterParam,
parseRisonFilters,
RisonFilter,
} from 'src/dashboard/util/risonFilters';
export interface UrlFilterIndicator {
subject: string;
operator: string;
value: string;
filter: RisonFilter;
}
/**
* Build a stable identity string for a URL filter so duplicates on the same
* column (e.g., multiple conditions on `country`) get distinct React keys and
* can be removed individually.
*/
export function getUrlFilterIdentity(filter: RisonFilter): string {
return `${filter.subject}|${filter.operator}|${JSON.stringify(
filter.comparator,
)}`;
}
function formatFilterValue(filter: RisonFilter): string {
const { comparator, operator } = filter;
if (operator === 'BETWEEN' && Array.isArray(comparator)) {
return `${comparator[0]} ${comparator[1]}`;
}
if (Array.isArray(comparator)) {
return comparator.join(', ');
}
return String(comparator);
}
export function getUrlFilterIndicators(): UrlFilterIndicator[] {
const risonParam = getRisonFilterParam();
if (!risonParam) {
return [];
}
const filters = parseRisonFilters(risonParam);
return filters.map(filter => ({
subject: filter.subject,
operator: filter.operator,
value: formatFilterValue(filter),
filter,
}));
}

View File

@@ -45,7 +45,6 @@ import Header from './Header';
import FilterControls from './FilterControls/FilterControls';
import CrossFiltersVertical from './CrossFilters/Vertical';
import crossFiltersSelector from './CrossFilters/selectors';
import UrlFiltersVertical from './UrlFilters/Vertical';
enum SectionType {
Filters = 'filters',
@@ -302,7 +301,6 @@ const VerticalFilterBar: FC<VerticalBarProps> = ({
) : (
<div css={tabPaneStyle} onScroll={onScroll}>
<>
<UrlFiltersVertical />
<CrossFiltersVertical hideHeader={hasOnlyOneSectionType} />
{filterControls}
</>

View File

@@ -107,14 +107,8 @@ const publishDataMask = debounce(
const previousParams = new URLSearchParams(search);
const newParams = new URLSearchParams();
let dataMaskKey: string | null;
// Capture the raw, still-URL-encoded `f=` payload from the query string
// directly. URLSearchParams decodes values (and turns `+` into space), which
// would corrupt the Rison payload if we re-inserted it without re-encoding.
const rawRisonMatch = search.match(/[?&]f=([^&]*)/);
const rawRisonFilterValue = rawRisonMatch ? rawRisonMatch[1] : null;
previousParams.forEach((value, key) => {
if (!EXCLUDED_URL_PARAMS.includes(key) && key !== 'f') {
if (!EXCLUDED_URL_PARAMS.includes(key)) {
newParams.append(key, value);
}
});
@@ -154,16 +148,9 @@ const publishDataMask = debounce(
if (appRoot !== '/' && replacementPathname.startsWith(appRoot)) {
replacementPathname = replacementPathname.substring(appRoot.length);
}
// Manually reconstruct the search string to preserve Rison filter encoding
let searchString = newParams.toString();
if (rawRisonFilterValue) {
const separator = searchString ? '&' : '';
searchString = `${searchString}${separator}f=${rawRisonFilterValue}`;
}
history.replace({
pathname: replacementPathname,
search: searchString,
search: newParams.toString(),
});
}
},

View File

@@ -64,18 +64,6 @@ import SyncDashboardState, {
getDashboardContextLocalStorage,
} from '../components/SyncDashboardState';
import { AutoRefreshProvider } from '../contexts/AutoRefreshContext';
import { Filter, PartialFilters } from '@superset-ui/core';
import {
parseRisonFilters,
risonFiltersToExtraFormDataFilters,
getRisonFilterParam,
prettifyRisonFilterUrl,
injectRisonFiltersIntelligently,
updateUrlWithUnmatchedFilters,
RISON_UNMATCHED_DATAMASK_ID,
} from '../util/risonFilters';
type NativeFilterConfigEntry = Partial<Filter> & { id: string };
export const DashboardPageIdContext = createContext('');
@@ -207,63 +195,6 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }: PageProps) => {
dataMask = isOldRison;
}
// Parse Rison URL filters with intelligent native filter injection
const risonFilterParam = getRisonFilterParam();
if (risonFilterParam) {
const risonFilters = parseRisonFilters(risonFilterParam);
if (risonFilters.length > 0) {
// Convert native filter config array to keyed object for lookup
const filterConfigArray = (dashboard?.metadata
?.native_filter_configuration ?? []) as NativeFilterConfigEntry[];
const nativeFilters: PartialFilters = {};
filterConfigArray.forEach(filter => {
nativeFilters[filter.id] = filter;
});
const injectionResult = injectRisonFiltersIntelligently(
risonFilters,
nativeFilters,
dataMask,
);
dataMask = injectionResult.updatedDataMask;
// Unmatched filters apply via a synthetic dataMask entry: because no
// entry in `nativeFilters` claims this id, `getAllActiveFilters`
// falls through to `allSliceIds` and the filters scope to every chart.
if (injectionResult.unmatchedFilters.length > 0) {
const extraFormDataFilters = risonFiltersToExtraFormDataFilters(
injectionResult.unmatchedFilters,
);
dataMask = {
...dataMask,
[RISON_UNMATCHED_DATAMASK_ID]: {
id: RISON_UNMATCHED_DATAMASK_ID,
extraFormData: { filters: extraFormDataFilters },
filterState: {},
ownState: {},
},
};
}
// Rewrite the URL to drop matched filters in a single step, keeping
// only unmatched ones (and prettifying their encoding). Going
// through react-router's history keeps `history.location.search` in
// sync so `publishDataMask` doesn't re-emit the original `f=`.
const matchedCount =
risonFilters.length - injectionResult.unmatchedFilters.length;
if (matchedCount > 0) {
updateUrlWithUnmatchedFilters(
injectionResult.unmatchedFilters,
history,
);
}
if (injectionResult.unmatchedFilters.length > 0) {
prettifyRisonFilterUrl();
}
}
}
if (readyToRender) {
if (!isDashboardHydrated.current) {
isDashboardHydrated.current = true;

View File

@@ -1,110 +0,0 @@
/**
* 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.
*/
/**
* Pinning test for the unmatched-Rison-filter wiring (PR review item #1).
*
* The reviewer flagged that unmatched URL filters were being written to
* dataMask under a key (`__rison_filters__`) that no downstream code reads —
* so unmatched filters silently dropped instead of applying to charts.
*
* The fix uses a synthetic dataMask id (`RISON_UNMATCHED_DATAMASK_ID`) whose
* `extraFormData.filters` falls through to `getAllActiveFilters`'s
* `allSliceIds` scope fallback, hitting every chart on the dashboard. This
* test pins down that contract so a future refactor of `getAllActiveFilters`
* can't silently break the wiring again.
*/
import { DataMaskStateWithId } from '@superset-ui/core';
import { getAllActiveFilters } from './activeAllDashboardFilters';
import {
RISON_UNMATCHED_DATAMASK_ID,
risonFiltersToExtraFormDataFilters,
} from './risonFilters';
test('synthetic Rison-unmatched dataMask entry scopes to every chart', () => {
const allSliceIds = [101, 202, 303];
const dataMask: DataMaskStateWithId = {
[RISON_UNMATCHED_DATAMASK_ID]: {
id: RISON_UNMATCHED_DATAMASK_ID,
extraFormData: {
filters: risonFiltersToExtraFormDataFilters([
{ subject: 'region', operator: '==', comparator: 'EMEA' },
]),
},
filterState: {},
ownState: {},
},
};
const activeFilters = getAllActiveFilters({
chartConfiguration: {},
nativeFilters: {}, // no native filter claims the synthetic id
dataMask,
allSliceIds,
});
const entry = activeFilters[RISON_UNMATCHED_DATAMASK_ID];
expect(entry).toBeDefined();
// Scope MUST equal allSliceIds — that's the whole reason this works.
expect(entry.scope).toEqual(allSliceIds);
// And the filters must be in the {col, op, val} shape getExtraFormData merges.
expect(entry.values).toEqual({
filters: [{ col: 'region', op: 'IN', val: ['EMEA'] }],
});
});
test('synthetic entry coexists with real native filters without overlap', () => {
const allSliceIds = [1, 2, 3];
const dataMask: DataMaskStateWithId = {
NATIVE_FILTER_country: {
id: 'NATIVE_FILTER_country',
extraFormData: {
filters: [{ col: 'country', op: 'IN', val: ['USA'] }],
},
filterState: { value: ['USA'] },
ownState: {},
},
[RISON_UNMATCHED_DATAMASK_ID]: {
id: RISON_UNMATCHED_DATAMASK_ID,
extraFormData: {
filters: [{ col: 'region', op: 'IN', val: ['EMEA'] }],
},
filterState: {},
ownState: {},
},
};
const activeFilters = getAllActiveFilters({
chartConfiguration: {},
nativeFilters: {
NATIVE_FILTER_country: {
id: 'NATIVE_FILTER_country',
chartsInScope: [1], // native filter is narrow
targets: [{ column: { name: 'country' } }],
filterType: 'filter_select',
},
},
dataMask,
allSliceIds,
});
// Native filter keeps its narrow scope.
expect(activeFilters.NATIVE_FILTER_country.scope).toEqual([1]);
// Synthetic filter scopes to all slices.
expect(activeFilters[RISON_UNMATCHED_DATAMASK_ID].scope).toEqual(allSliceIds);
});

View File

@@ -1,411 +0,0 @@
/**
* 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 { PartialFilters, DataMaskStateWithId } from '@superset-ui/core';
import {
injectRisonFiltersIntelligently,
RisonFilter,
parseRisonFilters,
risonFiltersToExtraFormDataFilters,
risonFiltersToString,
risonToAdhocFilters,
updateUrlWithUnmatchedFilters,
} from './risonFilters';
const mockNativeFilters: PartialFilters = {
filter_1: {
id: 'filter_1',
targets: [
{
column: { name: 'country' },
datasetId: 1,
},
],
filterType: 'filter_select',
},
filter_2: {
id: 'filter_2',
targets: [
{
column: { name: 'year' },
datasetId: 1,
},
],
filterType: 'filter_range',
},
filter_3: {
id: 'filter_3',
targets: [
{
column: { name: 'Country Code' },
datasetId: 1,
},
],
filterType: 'filter_select',
},
};
const mockDataMask: DataMaskStateWithId = {
filter_1: {
id: 'filter_1',
filterState: { value: undefined },
ownState: {},
},
};
test('should parse simple Rison filters', () => {
const risonString = '(country:USA,year:2024)';
const result = parseRisonFilters(risonString);
expect(result).toHaveLength(2);
expect(result[0]).toEqual({
subject: 'country',
operator: '==',
comparator: 'USA',
});
expect(result[1]).toEqual({
subject: 'year',
operator: '==',
comparator: 2024,
});
});
test('should parse IN operator with array syntax', () => {
const result = parseRisonFilters('(country:!(USA,Canada))');
expect(result).toHaveLength(1);
expect(result[0]).toEqual({
subject: 'country',
operator: 'IN',
comparator: ['USA', 'Canada'],
});
});
test('should parse BETWEEN operator', () => {
const result = parseRisonFilters('(msrp:(between:!(35,200)))');
expect(result).toHaveLength(1);
expect(result[0]).toEqual({
subject: 'msrp',
operator: 'BETWEEN',
comparator: [35, 200],
});
});
test('should parse NOT operator', () => {
const result = parseRisonFilters('(NOT:(country:USA))');
expect(result).toHaveLength(1);
expect(result[0].operator).toBe('!=');
expect(result[0].comparator).toBe('USA');
});
test('should parse comparison operators', () => {
expect(parseRisonFilters('(sales:(gt:100000))')[0].operator).toBe('>');
expect(parseRisonFilters('(age:(gte:18))')[0].operator).toBe('>=');
expect(parseRisonFilters('(temp:(lt:32))')[0].operator).toBe('<');
expect(parseRisonFilters('(price:(lte:1000))')[0].operator).toBe('<=');
});
test('should return empty array for invalid Rison', () => {
expect(parseRisonFilters('invalid rison')).toEqual([]);
expect(parseRisonFilters('(unclosed')).toEqual([]);
});
test('should match Rison filter to native filter by column name', () => {
const risonFilters: RisonFilter[] = [
{ subject: 'country', operator: '==', comparator: 'USA' },
];
const result = injectRisonFiltersIntelligently(
risonFilters,
mockNativeFilters,
mockDataMask,
);
expect(result.updatedDataMask.filter_1.filterState?.value).toEqual(['USA']);
expect(result.unmatchedFilters).toHaveLength(0);
});
test('should match column names with spaces (case-insensitive)', () => {
const risonFilters: RisonFilter[] = [
{ subject: 'Country Code', operator: '==', comparator: 'USA' },
];
const result = injectRisonFiltersIntelligently(
risonFilters,
mockNativeFilters,
mockDataMask,
);
expect(result.updatedDataMask.filter_3.filterState?.value).toEqual(['USA']);
expect(result.unmatchedFilters).toHaveLength(0);
});
test('should match column names case-insensitively', () => {
const risonFilters: RisonFilter[] = [
{ subject: 'country code', operator: '==', comparator: 'USA' },
];
const result = injectRisonFiltersIntelligently(
risonFilters,
mockNativeFilters,
mockDataMask,
);
expect(result.updatedDataMask.filter_3.filterState?.value).toEqual(['USA']);
expect(result.unmatchedFilters).toHaveLength(0);
});
test('should handle unmatched filters with fallback', () => {
const risonFilters: RisonFilter[] = [
{ subject: 'region', operator: '==', comparator: 'North America' },
];
const result = injectRisonFiltersIntelligently(
risonFilters,
mockNativeFilters,
mockDataMask,
);
expect(result.unmatchedFilters).toHaveLength(1);
expect(result.unmatchedFilters[0].subject).toBe('region');
});
test('should convert values correctly for different filter types', () => {
const risonFilters: RisonFilter[] = [
{ subject: 'country', operator: '==', comparator: 'USA' },
{ subject: 'year', operator: 'BETWEEN', comparator: [2020, 2024] },
];
const result = injectRisonFiltersIntelligently(
risonFilters,
mockNativeFilters,
mockDataMask,
);
// Select filter should be array
expect(result.updatedDataMask.filter_1.filterState?.value).toEqual(['USA']);
// Range filter should be min/max object
expect(result.updatedDataMask.filter_2.filterState?.value).toEqual({
min: 2020,
max: 2024,
});
expect(result.unmatchedFilters).toHaveLength(0);
});
test('should set extraFormData for auto-application on select filters', () => {
const risonFilters: RisonFilter[] = [
{ subject: 'country', operator: '==', comparator: 'USA' },
];
const result = injectRisonFiltersIntelligently(
risonFilters,
mockNativeFilters,
mockDataMask,
);
expect(result.updatedDataMask.filter_1.extraFormData).toEqual({
filters: [{ col: 'country', op: 'IN', val: ['USA'] }],
});
});
test('should set extraFormData for auto-application on IN filters', () => {
const risonFilters: RisonFilter[] = [
{ subject: 'country', operator: 'IN', comparator: ['USA', 'Canada'] },
];
const result = injectRisonFiltersIntelligently(
risonFilters,
mockNativeFilters,
mockDataMask,
);
expect(result.updatedDataMask.filter_1.filterState?.value).toEqual([
'USA',
'Canada',
]);
expect(result.updatedDataMask.filter_1.extraFormData).toEqual({
filters: [{ col: 'country', op: 'IN', val: ['USA', 'Canada'] }],
});
});
test('should set extraFormData for auto-application on BETWEEN filters', () => {
const risonFilters: RisonFilter[] = [
{ subject: 'year', operator: 'BETWEEN', comparator: [2020, 2024] },
];
const result = injectRisonFiltersIntelligently(
risonFilters,
mockNativeFilters,
mockDataMask,
);
expect(result.updatedDataMask.filter_2.filterState?.value).toEqual({
min: 2020,
max: 2024,
});
expect(result.updatedDataMask.filter_2.extraFormData).toEqual({
filters: [
{ col: 'year', op: '>=', val: 2020 },
{ col: 'year', op: '<=', val: 2024 },
],
});
});
test('should handle mixed matched and unmatched filters', () => {
const risonFilters: RisonFilter[] = [
{ subject: 'country', operator: '==', comparator: 'USA' },
{ subject: 'category', operator: '==', comparator: 'Sales' },
];
const result = injectRisonFiltersIntelligently(
risonFilters,
mockNativeFilters,
mockDataMask,
);
expect(result.updatedDataMask.filter_1.filterState?.value).toEqual(['USA']);
expect(result.unmatchedFilters).toHaveLength(1);
expect(result.unmatchedFilters[0].subject).toBe('category');
});
test('should convert filters to adhoc format', () => {
const risonFilters: RisonFilter[] = [
{ subject: 'country', operator: '==', comparator: 'USA' },
];
const adhocFilters = risonToAdhocFilters(risonFilters);
expect(adhocFilters).toHaveLength(1);
expect(adhocFilters[0]).toMatchObject({
expressionType: 'SIMPLE',
clause: 'WHERE',
subject: 'country',
operator: '==',
comparator: 'USA',
});
});
test('should convert filters to Rison string', () => {
const filters: RisonFilter[] = [
{ subject: 'country', operator: '==', comparator: 'USA' },
];
const result = risonFiltersToString(filters);
expect(result).toBe('(country:USA)');
});
test('should convert IN filters to Rison string', () => {
const filters: RisonFilter[] = [
{ subject: 'country', operator: 'IN', comparator: ['USA', 'Canada'] },
];
const result = risonFiltersToString(filters);
expect(result).toBe('(country:!(USA,Canada))');
});
test('should return empty string for empty filters', () => {
expect(risonFiltersToString([])).toBe('');
});
test('risonFiltersToExtraFormDataFilters expands BETWEEN into two clauses', () => {
const filters: RisonFilter[] = [
{ subject: 'country', operator: '==', comparator: 'USA' },
{ subject: 'year', operator: 'BETWEEN', comparator: [2020, 2024] },
];
expect(risonFiltersToExtraFormDataFilters(filters)).toEqual([
{ col: 'country', op: 'IN', val: ['USA'] },
{ col: 'year', op: '>=', val: 2020 },
{ col: 'year', op: '<=', val: 2024 },
]);
});
test('updateUrlWithUnmatchedFilters goes through history when supplied', () => {
const replace = jest.fn();
const history = { replace };
// Seed the URL so the function has something to read.
const originalLocation = window.location.href;
window.history.replaceState({}, '', '/superset/dashboard/1/?f=(country:USA)');
updateUrlWithUnmatchedFilters(
[{ subject: 'region', operator: '==', comparator: 'EMEA' }],
history,
);
expect(replace).toHaveBeenCalledTimes(1);
const call = replace.mock.calls[0][0];
expect(call.pathname).toBe('/superset/dashboard/1/');
expect(call.search).toContain('f=');
expect(call.search).toContain('region');
// Restore.
window.history.replaceState({}, '', originalLocation);
});
test('updateUrlWithUnmatchedFilters drops f= when no unmatched remain', () => {
const replace = jest.fn();
const originalLocation = window.location.href;
window.history.replaceState({}, '', '/superset/dashboard/1/?f=(country:USA)');
updateUrlWithUnmatchedFilters([], { replace });
expect(replace).toHaveBeenCalledTimes(1);
expect(replace.mock.calls[0][0].search).toBe('');
window.history.replaceState({}, '', originalLocation);
});
test('updateUrlWithUnmatchedFilters cleanup is observable by history readers', () => {
// Validates PR review item #3: the URL cleanup must go through a path
// that downstream history-readers (e.g. publishDataMask) observe. The
// raw window.history.replaceState fallback alone left react-router's
// history.location.search stale, causing publishDataMask to re-append
// the original f= on the next interaction.
//
// Stand in for react-router's history with a fake whose `.location`
// updates synchronously when .replace is called — same contract as
// react-router-dom's history.replace.
const fakeHistory = {
location: {
pathname: '/superset/dashboard/1/',
search: '?f=(country:USA)',
},
replace(next: { pathname: string; search: string }) {
this.location = next;
},
};
const originalLocation = window.location.href;
window.history.replaceState({}, '', '/superset/dashboard/1/?f=(country:USA)');
updateUrlWithUnmatchedFilters(
[{ subject: 'sales', operator: '>', comparator: 1000 }],
fakeHistory,
);
// After cleanup, a reader of history.location.search (the same path
// publishDataMask uses) must NOT see the original matched filter.
expect(fakeHistory.location.search).not.toContain('country');
expect(fakeHistory.location.search).toContain('sales');
window.history.replaceState({}, '', originalLocation);
});

View File

@@ -1,569 +0,0 @@
/**
* 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 {
QueryObjectFilterClause,
PartialFilters,
DataMaskStateWithId,
} from '@superset-ui/core';
import rison from 'rison';
/**
* Synthetic dataMask key for URL Rison filters that don't match any native
* filter on the dashboard. Because no entry in `nativeFilters` claims this id,
* `getAllActiveFilters` falls through to `allSliceIds`, so the attached
* `extraFormData.filters` apply to every chart on the dashboard.
*/
export const RISON_UNMATCHED_DATAMASK_ID = '__rison_unmatched__';
export interface RisonFilter {
subject: string;
operator: string;
comparator: string | number | boolean | (string | number)[];
}
export interface IntelligentRisonInjectionResult {
updatedDataMask: DataMaskStateWithId;
unmatchedFilters: RisonFilter[];
}
/**
* Parse individual filter condition
*/
function parseFilterCondition(key: string, value: unknown): RisonFilter {
// Handle comparison operators: (gt:100), (between:!(1,10))
if (typeof value === 'object' && value !== null && !Array.isArray(value)) {
const [operator, operatorValue] = Object.entries(
value as Record<string, unknown>,
)[0];
switch (operator) {
case 'gt':
return {
subject: key,
operator: '>',
comparator: operatorValue as string | number,
};
case 'gte':
return {
subject: key,
operator: '>=',
comparator: operatorValue as string | number,
};
case 'lt':
return {
subject: key,
operator: '<',
comparator: operatorValue as string | number,
};
case 'lte':
return {
subject: key,
operator: '<=',
comparator: operatorValue as string | number,
};
case 'between':
return {
subject: key,
operator: 'BETWEEN',
comparator: operatorValue as (string | number)[],
};
case 'like':
return {
subject: key,
operator: 'LIKE',
comparator: operatorValue as string,
};
default:
return {
subject: key,
operator: '==',
comparator: value as unknown as string | number,
};
}
}
// Handle IN operator: !(value1,value2)
if (Array.isArray(value)) {
return {
subject: key,
operator: 'IN',
comparator: value as (string | number)[],
};
}
// Handle simple equality
return {
subject: key,
operator: '==',
comparator: value as string | number | boolean,
};
}
/**
* Parse Rison filter syntax from URL parameter.
* Supports formats like: (country:USA,year:2024)
*/
export function parseRisonFilters(risonString: string): RisonFilter[] {
try {
const parsed = rison.decode(risonString);
const filters: RisonFilter[] = [];
if (!parsed || typeof parsed !== 'object') {
return filters;
}
const parsedObj = parsed as Record<string, unknown>;
// Handle OR operator: OR:!(condition1,condition2)
if (parsedObj.OR && Array.isArray(parsedObj.OR)) {
(parsedObj.OR as Record<string, unknown>[]).forEach(condition => {
if (typeof condition === 'object') {
Object.entries(condition).forEach(([key, value]) => {
filters.push(parseFilterCondition(key, value));
});
}
});
return filters;
}
// Handle NOT operator: NOT:(condition). Falls through so regular keys at the
// same level are still picked up below (supports mixed payloads like
// `(country:USA,NOT:(status:archived))`).
if (parsedObj.NOT && typeof parsedObj.NOT === 'object') {
Object.entries(parsedObj.NOT as Record<string, unknown>).forEach(
([key, value]) => {
const filter = parseFilterCondition(key, value);
if (filter.operator === '==') {
filter.operator = '!=';
} else if (filter.operator === 'IN') {
filter.operator = 'NOT IN';
}
filters.push(filter);
},
);
}
// Handle regular filters
Object.entries(parsedObj).forEach(([key, value]) => {
if (key !== 'OR' && key !== 'NOT') {
filters.push(parseFilterCondition(key, value));
}
});
return filters;
} catch (error) {
console.warn('Failed to parse Rison filters:', error);
return [];
}
}
/**
* Convert Rison filters to Superset adhoc filter format
*/
export function risonToAdhocFilters(
risonFilters: RisonFilter[],
): QueryObjectFilterClause[] {
return risonFilters.map(
filter =>
({
expressionType: 'SIMPLE' as const,
clause: 'WHERE' as const,
subject: filter.subject,
operator: filter.operator,
comparator: filter.comparator,
}) as unknown as QueryObjectFilterClause,
);
}
/**
* Prettify Rison filter URL by replacing encoded characters.
* Uses browser history API to update URL without page reload.
*/
export function prettifyRisonFilterUrl(): void {
try {
const currentUrl = window.location.href;
if (!currentUrl.includes('&f=') && !currentUrl.includes('?f=')) {
return;
}
const urlMatch = currentUrl.match(/([?&])f=([^&]*)/);
if (!urlMatch) {
return;
}
const separator = urlMatch[1];
let risonValue = urlMatch[2];
if (!risonValue.includes('%') && !risonValue.includes('+')) {
return;
}
let previousValue = '';
let decodeAttempts = 0;
while (risonValue !== previousValue && decodeAttempts < 5) {
previousValue = risonValue;
try {
if (risonValue.includes('%')) {
risonValue = decodeURIComponent(risonValue);
}
} catch {
break;
}
decodeAttempts += 1;
}
risonValue = risonValue.replace(/\+/g, ' ');
const matchIndex = urlMatch.index ?? 0;
const beforeRison = currentUrl.substring(0, matchIndex);
const afterRison = currentUrl.substring(matchIndex + urlMatch[0].length);
const prettifiedUrl = `${beforeRison}${separator}f=${risonValue}${afterRison}`;
if (prettifiedUrl !== currentUrl) {
window.history.replaceState(window.history.state, '', prettifiedUrl);
}
} catch (error) {
console.warn('Failed to prettify Rison URL:', error);
}
}
/**
* Get Rison filter parameter from URL
*/
export function getRisonFilterParam(): string | null {
const params = new URLSearchParams(window.location.search);
return params.get('f');
}
/**
* Convert an array of RisonFilter back to Rison string format
*/
export function risonFiltersToString(filters: RisonFilter[]): string {
if (filters.length === 0) {
return '';
}
type RisonValue =
| string
| number
| boolean
| (string | number)[]
| Record<string, unknown>;
const risonObject: Record<string, RisonValue> = {};
const notObject: Record<string, RisonValue> = {};
const encodePositive = (
target: Record<string, RisonValue>,
filter: RisonFilter,
op: string,
) => {
if (op === 'IN' && Array.isArray(filter.comparator)) {
target[filter.subject] = filter.comparator;
} else if (op === '==') {
target[filter.subject] = filter.comparator;
} else {
const operatorMap: Record<string, string> = {
'>': 'gt',
'>=': 'gte',
'<': 'lt',
'<=': 'lte',
BETWEEN: 'between',
LIKE: 'like',
ILIKE: 'ilike',
};
const risonOp = operatorMap[op] || op;
target[filter.subject] = { [risonOp]: filter.comparator };
}
};
filters.forEach(filter => {
if (filter.operator === '!=') {
// Re-emit as NOT:(col:value) so parseRisonFilters can read it back.
encodePositive(notObject, filter, '==');
} else if (filter.operator === 'NOT IN') {
encodePositive(notObject, filter, 'IN');
} else {
encodePositive(risonObject, filter, filter.operator);
}
});
if (Object.keys(notObject).length > 0) {
risonObject.NOT = notObject;
}
try {
return rison.encode(risonObject);
} catch (error) {
console.warn('Failed to encode Rison filters:', error);
return '';
}
}
interface ReplaceHistory {
replace(location: { pathname: string; search: string }): void;
}
/**
* Update the URL to remove successfully matched filters, keeping only unmatched ones.
* When a react-router history is supplied, the update goes through it so that
* components reading from `history.location` (e.g. `publishDataMask` in the
* filter bar) see the new search string. Otherwise falls back to a raw
* `window.history.replaceState`.
*/
export function updateUrlWithUnmatchedFilters(
unmatchedFilters: RisonFilter[],
history?: ReplaceHistory,
): void {
try {
const currentUrl = new URL(window.location.href);
if (unmatchedFilters.length === 0) {
currentUrl.searchParams.delete('f');
} else {
const newRisonString = risonFiltersToString(unmatchedFilters);
if (newRisonString) {
currentUrl.searchParams.set('f', newRisonString);
} else {
currentUrl.searchParams.delete('f');
}
}
// Always keep window.history in sync so callers that read
// `window.location.search` (e.g. `getRisonFilterParam`) see the update.
// With a real `BrowserRouter`, `history.replace` would do this too — but
// under a `createMemoryHistory` (used in tests, or in some embedded
// contexts) it does not, and we'd leak the stale URL into the next
// `getRisonFilterParam()` call.
window.history.replaceState(
window.history.state,
'',
currentUrl.toString(),
);
if (history) {
history.replace({
pathname: currentUrl.pathname,
search: currentUrl.search,
});
}
} catch (error) {
console.warn('Failed to update URL with unmatched filters:', error);
}
}
/**
* Find a native filter that matches a Rison filter by column name.
* Uses case-insensitive, trimmed comparison to handle column names with spaces.
*/
function findMatchingNativeFilter(
risonFilter: RisonFilter,
nativeFilters: PartialFilters,
): string | null {
const normalizedSubject = risonFilter.subject.trim().toLowerCase();
for (const [filterId, nativeFilter] of Object.entries(nativeFilters)) {
if (!nativeFilter?.targets) continue;
const hasMatchingTarget = nativeFilter.targets.some(target => {
if (typeof target === 'object' && target && 'column' in target) {
return target.column?.name?.trim().toLowerCase() === normalizedSubject;
}
return false;
});
if (hasMatchingTarget) {
return filterId;
}
}
return null;
}
/**
* Build extraFormData filters for a given rison filter and column name
*/
function buildExtraFormDataFilters(
risonFilter: RisonFilter,
columnName: string,
): QueryObjectFilterClause[] {
const { operator, comparator } = risonFilter;
if (operator === 'IN' || (operator === '==' && Array.isArray(comparator))) {
return [
{
col: columnName,
op: 'IN',
val: Array.isArray(comparator) ? comparator : [comparator],
},
];
}
if (operator === '==' && !Array.isArray(comparator)) {
return [{ col: columnName, op: 'IN', val: [comparator] }];
}
if (
operator === 'BETWEEN' &&
Array.isArray(comparator) &&
comparator.length === 2
) {
return [
{ col: columnName, op: '>=', val: comparator[0] },
{ col: columnName, op: '<=', val: comparator[1] },
];
}
return [
{
col: columnName,
op: operator,
val: comparator,
} as QueryObjectFilterClause,
];
}
/**
* Convert a list of Rison filters into the `{col, op, val}` clauses expected
* by `dataMask[id].extraFormData.filters`. Each filter uses its own subject
* as the column name.
*/
export function risonFiltersToExtraFormDataFilters(
filters: RisonFilter[],
): QueryObjectFilterClause[] {
return filters.flatMap(filter =>
buildExtraFormDataFilters(filter, filter.subject),
);
}
/**
* Convert a Rison filter value to the format expected by a native filter.
* Also returns extraFormData for auto-application.
*/
function convertRisonToNativeValue(
risonFilter: RisonFilter,
nativeFilter: { filterType?: string },
): unknown {
const { comparator, operator } = risonFilter;
const filterType = nativeFilter?.filterType;
switch (filterType) {
case 'filter_select':
if (operator === 'IN' || Array.isArray(comparator)) {
return Array.isArray(comparator) ? comparator : [comparator];
}
return [comparator];
case 'filter_range':
if (
operator === 'BETWEEN' &&
Array.isArray(comparator) &&
comparator.length === 2
) {
return { min: comparator[0], max: comparator[1] };
}
return comparator;
case 'filter_time_range':
case 'filter_timecolumn':
return comparator;
default:
return Array.isArray(comparator) ? comparator : [comparator];
}
}
/**
* Build a complete DataMask entry for a rison filter matched to a native filter.
* Sets both filterState.value AND extraFormData so the filter auto-applies.
*/
function buildDataMaskForFilter(
risonFilter: RisonFilter,
nativeFilter: {
id: string;
filterType?: string;
targets?: { column?: { name?: string } }[];
},
columnName: string,
) {
const convertedValue = convertRisonToNativeValue(risonFilter, nativeFilter);
return {
id: nativeFilter.id,
filterState: {
value: convertedValue,
},
extraFormData: {
filters: buildExtraFormDataFilters(risonFilter, columnName),
},
ownState: {},
};
}
/**
* Intelligently inject Rison filters into native filters where possible,
* falling back to brute-force injection for unmatched filters
*/
export function injectRisonFiltersIntelligently(
risonFilters: RisonFilter[],
nativeFilters: PartialFilters,
currentDataMask: DataMaskStateWithId,
): IntelligentRisonInjectionResult {
const updatedDataMask = { ...currentDataMask };
const unmatchedFilters: RisonFilter[] = [];
risonFilters.forEach(risonFilter => {
const matchingFilterId = findMatchingNativeFilter(
risonFilter,
nativeFilters,
);
if (matchingFilterId) {
const matchedFilter = nativeFilters[matchingFilterId];
const filterId = matchedFilter?.id ?? matchingFilterId;
if (matchedFilter && filterId) {
const columnName =
matchedFilter.targets?.[0]?.column?.name ?? risonFilter.subject;
const dataMaskEntry = buildDataMaskForFilter(
risonFilter,
{ ...matchedFilter, id: filterId } as {
id: string;
filterType?: string;
targets?: { column?: { name?: string } }[];
},
columnName,
);
updatedDataMask[filterId] = {
...updatedDataMask[filterId],
...dataMaskEntry,
};
return;
}
}
unmatchedFilters.push(risonFilter);
});
return {
updatedDataMask,
unmatchedFilters,
};
}

View File

@@ -34,7 +34,6 @@ from superset.commands.chart.exceptions import (
from superset.commands.utils import get_datasource_by_id
from superset.daos.chart import ChartDAO
from superset.daos.dashboard import DashboardDAO
from superset.utils import json
from superset.utils.decorators import on_error, transaction
logger = logging.getLogger(__name__)
@@ -44,13 +43,6 @@ class CreateChartCommand(CreateMixin, BaseCommand):
def __init__(self, data: dict[str, Any]):
self._properties = data.copy()
if params_str := self._properties.get("params"):
params = json.loads(params_str)
if isinstance(params, dict) and "viz_type" in params:
# Only fall back to params when no top-level viz_type was supplied;
# an explicit top-level field takes precedence.
self._properties.setdefault("viz_type", params["viz_type"])
@transaction(on_error=partial(on_error, reraise=ChartCreateFailedError))
def run(self) -> Model:
self.validate()

View File

@@ -365,164 +365,3 @@ def serialize_annotation(obj: Any) -> AnnotationInfo | None:
layer_id=getattr(obj, "layer_id", None),
)
)
class CreateLayerAnnotationRequest(BaseModel):
"""Request schema for create_layer_annotation."""
model_config = ConfigDict(populate_by_name=True)
layer_id: int = Field(
...,
description="ID of the annotation layer to add the annotation to.",
)
short_descr: str = Field(
...,
min_length=1,
max_length=500,
description="Short description / title of the annotation. "
"Must be unique within the annotation layer.",
)
start_dttm: datetime = Field(
...,
description="Annotation start time in ISO 8601 format "
"(e.g. '2024-01-15T08:00:00').",
)
end_dttm: datetime = Field(
...,
description="Annotation end time in ISO 8601 format "
"(e.g. '2024-01-15T09:00:00'). Must be >= start_dttm.",
)
long_descr: str | None = Field(
None,
description="Detailed description of the annotation (optional).",
)
json_metadata: str | None = Field(
None,
description="Optional JSON metadata string for the annotation.",
)
@field_validator("json_metadata")
@classmethod
def validate_json_metadata(cls, v: str | None) -> str | None:
if v is None:
return v
try:
json_utils.loads(v)
except (ValueError, TypeError) as exc:
raise ValueError("json_metadata must be valid JSON") from exc
return v
class CreateLayerAnnotationResponse(BaseModel):
"""Response schema for create_layer_annotation."""
id: int | None = Field(
None,
description="ID of the created annotation. None if creation failed.",
)
layer_id: int = Field(
...,
description="ID of the annotation layer the annotation was added to.",
)
short_descr: str = Field(
...,
description="Short description / title of the annotation.",
)
start_dttm: datetime | None = Field(
None,
description="Annotation start time.",
)
end_dttm: datetime | None = Field(
None,
description="Annotation end time.",
)
long_descr: str | None = Field(
None,
description="Detailed description of the annotation.",
)
error: str | None = Field(
None,
description="Error message if creation failed, otherwise null.",
)
class UpdateLayerAnnotationRequest(BaseModel):
"""Request schema for update_layer_annotation."""
model_config = ConfigDict(populate_by_name=True)
layer_id: int = Field(
...,
description="ID of the annotation layer the annotation belongs to.",
)
annotation_id: int = Field(
...,
description="ID of the annotation to update.",
)
short_descr: str | None = Field(
None,
min_length=1,
max_length=500,
description="New short description / title. "
"Must be unique within the annotation layer.",
)
start_dttm: datetime | None = Field(
None,
description="New annotation start time in ISO 8601 format.",
)
end_dttm: datetime | None = Field(
None,
description="New annotation end time in ISO 8601 format. "
"Must be >= start_dttm.",
)
long_descr: str | None = Field(
None,
description="New detailed description (optional).",
)
json_metadata: str | None = Field(
None,
description="New JSON metadata string (optional).",
)
@field_validator("json_metadata")
@classmethod
def validate_json_metadata(cls, v: str | None) -> str | None:
if v is None:
return v
try:
json.loads(v)
except (ValueError, TypeError) as exc:
raise ValueError("json_metadata must be valid JSON") from exc
return v
class UpdateLayerAnnotationResponse(BaseModel):
"""Response schema for update_layer_annotation."""
id: int | None = Field(
None,
description="ID of the updated annotation. None if update failed.",
)
layer_id: int = Field(
...,
description="ID of the annotation layer.",
)
short_descr: str | None = Field(
None,
description="Short description / title of the annotation.",
)
start_dttm: datetime | None = Field(
None,
description="Annotation start time.",
)
end_dttm: datetime | None = Field(
None,
description="Annotation end time.",
)
long_descr: str | None = Field(
None,
description="Detailed description of the annotation.",
)
error: str | None = Field(
None,
description="Error message if update failed, otherwise null.",
)

View File

@@ -15,18 +15,14 @@
# specific language governing permissions and limitations
# under the License.
from .create_layer_annotation import create_layer_annotation
from .get_annotation_layer_info import get_annotation_layer_info
from .get_layer_annotation_info import get_layer_annotation_info
from .list_annotation_layers import list_annotation_layers
from .list_layer_annotations import list_layer_annotations
from .update_layer_annotation import update_layer_annotation
__all__ = [
"list_annotation_layers",
"get_annotation_layer_info",
"list_layer_annotations",
"get_layer_annotation_info",
"create_layer_annotation",
"update_layer_annotation",
]

View File

@@ -1,140 +0,0 @@
# 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 typing import Any
from fastmcp import Context
from superset_core.mcp.decorators import tool, ToolAnnotations
from superset.extensions import event_logger
from superset.mcp_service.annotation_layer.schemas import (
CreateLayerAnnotationRequest,
CreateLayerAnnotationResponse,
)
logger = logging.getLogger(__name__)
@tool(
tags=["mutate"],
class_permission_name="Annotation",
method_permission_name="write",
annotations=ToolAnnotations(
title="Add annotation to an annotation layer",
readOnlyHint=False,
destructiveHint=False,
),
)
async def create_layer_annotation(
request: CreateLayerAnnotationRequest, ctx: Context
) -> CreateLayerAnnotationResponse:
"""Add a new annotation to an existing annotation layer.
Use this tool when a user wants to mark a specific time range on charts
with a label or note — for example, to flag a deployment, an outage, or
a campaign period.
Workflow:
1. Identify the target annotation layer (layer_id). Use list tools if needed.
2. Call this tool with the layer_id, a short description, and the time range.
3. The annotation will appear on charts that reference that layer.
"""
await ctx.info(
"Creating annotation: layer_id=%s, short_descr=%r"
% (request.layer_id, request.short_descr)
)
try:
from superset.commands.annotation_layer.annotation.create import (
CreateAnnotationCommand,
)
from superset.commands.annotation_layer.annotation.exceptions import (
AnnotationCreateFailedError,
AnnotationInvalidError,
)
from superset.commands.annotation_layer.exceptions import (
AnnotationLayerNotFoundError,
)
properties: dict[str, Any] = {
"layer": request.layer_id,
"short_descr": request.short_descr,
"start_dttm": request.start_dttm,
"end_dttm": request.end_dttm,
}
if request.long_descr is not None:
properties["long_descr"] = request.long_descr
if request.json_metadata is not None:
properties["json_metadata"] = request.json_metadata
with event_logger.log_context(action="mcp.create_layer_annotation.create"):
annotation = CreateAnnotationCommand(properties).run()
await ctx.info(
"Annotation created: id=%s, layer_id=%s, short_descr=%r"
% (annotation.id, request.layer_id, request.short_descr)
)
return CreateLayerAnnotationResponse(
id=annotation.id,
layer_id=request.layer_id,
short_descr=annotation.short_descr,
start_dttm=annotation.start_dttm,
end_dttm=annotation.end_dttm,
long_descr=getattr(annotation, "long_descr", None),
)
except AnnotationLayerNotFoundError as exc:
await ctx.warning(
"Annotation layer not found: layer_id=%s" % (request.layer_id,)
)
return CreateLayerAnnotationResponse(
id=None,
layer_id=request.layer_id,
short_descr=request.short_descr,
start_dttm=request.start_dttm,
end_dttm=request.end_dttm,
error=f"Annotation layer {request.layer_id} not found: {exc}",
)
except AnnotationInvalidError as exc:
messages = exc.normalized_messages()
await ctx.warning("Annotation validation failed: %s" % (messages,))
return CreateLayerAnnotationResponse(
id=None,
layer_id=request.layer_id,
short_descr=request.short_descr,
start_dttm=request.start_dttm,
end_dttm=request.end_dttm,
error=str(messages),
)
except AnnotationCreateFailedError as exc:
await ctx.error("Annotation creation failed: %s" % (str(exc),))
return CreateLayerAnnotationResponse(
id=None,
layer_id=request.layer_id,
short_descr=request.short_descr,
start_dttm=request.start_dttm,
end_dttm=request.end_dttm,
error=f"Failed to create annotation: {exc}",
)
except Exception as exc:
await ctx.error(
"Unexpected error creating annotation: %s: %s"
% (type(exc).__name__, str(exc))
)
raise

View File

@@ -1,153 +0,0 @@
# 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 typing import Any
from fastmcp import Context
from superset_core.mcp.decorators import tool, ToolAnnotations
from superset.extensions import event_logger
from superset.mcp_service.annotation_layer.schemas import (
UpdateLayerAnnotationRequest,
UpdateLayerAnnotationResponse,
)
logger = logging.getLogger(__name__)
def _build_update_properties(request: UpdateLayerAnnotationRequest) -> dict[str, Any]:
"""Build the properties dict for UpdateAnnotationCommand from the request."""
properties: dict[str, Any] = {"layer": request.layer_id}
if request.short_descr is not None:
properties["short_descr"] = request.short_descr
if request.start_dttm is not None:
properties["start_dttm"] = request.start_dttm
if request.end_dttm is not None:
properties["end_dttm"] = request.end_dttm
if request.long_descr is not None:
properties["long_descr"] = request.long_descr
if request.json_metadata is not None:
properties["json_metadata"] = request.json_metadata
return properties
@tool(
tags=["mutate"],
class_permission_name="Annotation",
method_permission_name="write",
annotations=ToolAnnotations(
title="Update an annotation in an annotation layer",
readOnlyHint=False,
destructiveHint=True,
),
)
async def update_layer_annotation(
request: UpdateLayerAnnotationRequest, ctx: Context
) -> UpdateLayerAnnotationResponse:
"""Update an existing annotation in an annotation layer.
Use this tool to change the time range, description, or metadata of an
existing annotation — for example to correct a deployment window or extend
an outage marker.
All fields except layer_id and annotation_id are optional; only the
fields provided will be updated.
Workflow:
1. Identify the annotation layer (layer_id) and annotation (annotation_id).
2. Call this tool with the fields you want to change.
3. The annotation will be updated in place on all charts that reference
that layer.
"""
await ctx.info(
"Updating annotation: layer_id=%s, annotation_id=%s"
% (request.layer_id, request.annotation_id)
)
try:
from superset.commands.annotation_layer.annotation.exceptions import (
AnnotationInvalidError,
AnnotationNotFoundError,
AnnotationUpdateFailedError,
)
from superset.commands.annotation_layer.annotation.update import (
UpdateAnnotationCommand,
)
from superset.commands.annotation_layer.exceptions import (
AnnotationLayerNotFoundError,
)
properties = _build_update_properties(request)
with event_logger.log_context(action="mcp.update_layer_annotation.update"):
annotation = UpdateAnnotationCommand(
request.annotation_id, properties
).run()
await ctx.info(
"Annotation updated: id=%s, layer_id=%s" % (annotation.id, request.layer_id)
)
return UpdateLayerAnnotationResponse(
id=annotation.id,
layer_id=request.layer_id,
short_descr=annotation.short_descr,
start_dttm=annotation.start_dttm,
end_dttm=annotation.end_dttm,
long_descr=getattr(annotation, "long_descr", None),
)
except AnnotationNotFoundError as exc:
await ctx.warning(
"Annotation not found: annotation_id=%s" % (request.annotation_id,)
)
return UpdateLayerAnnotationResponse(
id=None,
layer_id=request.layer_id,
error=f"Annotation {request.annotation_id} not found: {exc}",
)
except AnnotationLayerNotFoundError as exc:
await ctx.warning(
"Annotation layer not found: layer_id=%s" % (request.layer_id,)
)
return UpdateLayerAnnotationResponse(
id=None,
layer_id=request.layer_id,
error=f"Annotation layer {request.layer_id} not found: {exc}",
)
except AnnotationInvalidError as exc:
messages = exc.normalized_messages()
await ctx.warning("Annotation validation failed: %s" % (messages,))
return UpdateLayerAnnotationResponse(
id=None,
layer_id=request.layer_id,
error=str(messages),
)
except AnnotationUpdateFailedError as exc:
await ctx.error("Annotation update failed: %s" % (str(exc),))
return UpdateLayerAnnotationResponse(
id=None,
layer_id=request.layer_id,
error=f"Failed to update annotation: {exc}",
)
except Exception as exc:
await ctx.error(
"Unexpected error updating annotation: %s: %s"
% (type(exc).__name__, str(exc))
)
raise

View File

@@ -673,12 +673,10 @@ from superset.mcp_service.action_log.tool import ( # noqa: F401, E402
list_action_logs,
)
from superset.mcp_service.annotation_layer.tool import ( # noqa: F401, E402
create_layer_annotation,
get_annotation_layer_info,
get_layer_annotation_info,
list_annotation_layers,
list_layer_annotations,
update_layer_annotation,
)
from superset.mcp_service.chart import ( # noqa: F401, E402
prompts as chart_prompts,

View File

@@ -30,10 +30,6 @@ BLOCKLIST = {
re.compile(r"sqlite(?:\+[^\s]*)?$"),
# shillelagh allows opening local files (eg, 'SELECT * FROM "csv:///etc/passwd"')
re.compile(r"shillelagh(?:\+[^\s]*)?$"),
# duckdb exposes filesystem table-valued functions (read_csv_auto, read_text,
# read_parquet, glob, etc.) that grant arbitrary local file access. The
# motherduck cloud variant rides on the same dialect and is covered too.
re.compile(r"duckdb(?:\+[^\s]*)?$"),
}

View File

@@ -1,274 +0,0 @@
# 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.
"""
Parser for Rison URL filters that converts simplified filter syntax
to Superset's adhoc_filters format.
"""
from __future__ import annotations
import logging
import re
from typing import Any, Optional, Union
import prison
from flask import has_request_context, request
logger = logging.getLogger(__name__)
# SQL identifiers permitted in URL-supplied filter columns. Restricted to
# alphanumeric/underscore (optionally schema-qualified) so OR-path SQL
# generation never interpolates an attacker-controlled identifier verbatim.
_SAFE_IDENTIFIER_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*(\.[A-Za-z_][A-Za-z0-9_]*)?$")
def _safe_identifier(name: Any) -> Optional[str]:
if isinstance(name, str) and _SAFE_IDENTIFIER_RE.match(name):
return name
return None
def _quote_sql_literal(value: Any) -> Optional[str]:
"""Quote a scalar value safely for SQL string interpolation.
- Strings: single-quoted with `'` escaped as `''` (SQL standard).
- Numeric / bool: rendered bare.
- Anything else (None, dict, list, etc.): rejected.
"""
if isinstance(value, bool):
return "TRUE" if value else "FALSE"
if isinstance(value, (int, float)):
return str(value)
if isinstance(value, str):
return "'" + value.replace("'", "''") + "'"
return None
class RisonFilterParser:
"""
Parse Rison filter syntax from URL parameter 'f' and convert to adhoc_filters.
Supports:
- Simple equality: f=(country:USA)
- Lists (IN): f=(country:!(USA,Canada))
- NOT operator: f=(NOT:(country:USA))
- OR operator: f=(OR:!(condition1,condition2))
- Comparison operators: f=(sales:(gt:100000))
- BETWEEN: f=(date:(between:!(2024-01-01,2024-12-31)))
- LIKE: f=(name:(like:'%smith%'))
"""
OPERATORS: dict[str, str] = {
"gt": ">",
"gte": ">=",
"lt": "<",
"lte": "<=",
"between": "BETWEEN",
"like": "LIKE",
"ilike": "ILIKE",
"ne": "!=",
"eq": "==",
}
def parse(self, filter_string: Optional[str] = None) -> list[dict[str, Any]]:
"""
Parse Rison filter string and convert to adhoc_filters format.
Args:
filter_string: Rison-encoded filter string, or None to get from request
Returns:
List of adhoc_filter dictionaries
"""
if filter_string is None:
# Callers outside a Flask request context (CLI, async tasks, tests)
# would otherwise hit a RuntimeError on `request.args` access.
if not has_request_context():
return []
filter_string = request.args.get("f")
if not filter_string:
return []
try:
filters_obj = prison.loads(filter_string)
return self._convert_to_adhoc_filters(filters_obj)
except Exception:
logger.warning(
"Failed to parse Rison filters: %s", filter_string, exc_info=True
)
return []
def _convert_to_adhoc_filters(
self, filters_obj: Union[dict[str, Any], list[Any], Any]
) -> list[dict[str, Any]]:
if not isinstance(filters_obj, dict):
return []
adhoc_filters: list[dict[str, Any]] = []
for key, value in filters_obj.items():
if key == "OR":
adhoc_filters.extend(self._handle_or_operator(value))
elif key == "NOT":
adhoc_filters.extend(self._handle_not_operator(value))
else:
filter_dict = self._create_filter(key, value)
if filter_dict:
adhoc_filters.append(filter_dict)
return adhoc_filters
def _create_filter(
self, column: str, value: Any, negate: bool = False
) -> Optional[dict[str, Any]]:
filter_dict: dict[str, Any] = {
"expressionType": "SIMPLE",
"clause": "WHERE",
"subject": column,
}
if isinstance(value, list):
filter_dict["operator"] = "NOT IN" if negate else "IN"
filter_dict["comparator"] = value
elif isinstance(value, dict):
operator_info = self._parse_operator_dict(value)
if operator_info:
operator, comparator = operator_info
if negate and operator == "==":
operator = "!="
elif negate and operator == "IN":
operator = "NOT IN"
filter_dict["operator"] = operator
filter_dict["comparator"] = comparator
else:
return None
else:
filter_dict["operator"] = "!=" if negate else "=="
filter_dict["comparator"] = value
return filter_dict
def _parse_operator_dict(
self, op_dict: dict[str, Any]
) -> Optional[tuple[str, Any]]:
if not op_dict:
return None
for op_key, op_value in op_dict.items():
if op_key in self.OPERATORS:
operator = self.OPERATORS[op_key]
if (
operator == "BETWEEN"
and isinstance(op_value, list)
and len(op_value) == 2
):
return operator, op_value
return operator, op_value
if op_key == "in":
return "IN", op_value if isinstance(op_value, list) else [op_value]
if op_key == "nin":
return "NOT IN", op_value if isinstance(op_value, list) else [op_value]
return None
def _handle_or_operator(self, or_value: Any) -> list[dict[str, Any]]:
if not isinstance(or_value, list):
return []
sql_parts: list[str] = []
for item in or_value:
if isinstance(item, dict):
for col, val in item.items():
if col not in ("OR", "NOT"):
sql_part = self._build_sql_condition(col, val)
if sql_part:
sql_parts.append(sql_part)
if sql_parts:
return [
{
"expressionType": "SQL",
"clause": "WHERE",
"sqlExpression": f"({' OR '.join(sql_parts)})",
}
]
return []
def _build_sql_condition(self, column: str, value: Any) -> Optional[str]:
# URL-supplied columns flow directly into a raw SQL expression in the
# OR path, so the identifier must match a strict whitelist or we drop
# the whole condition. Likewise, string literals get `'` escaped to
# `''` and unrecognised value types are rejected outright.
safe_column = _safe_identifier(column)
if safe_column is None:
logger.warning("Rejecting URL filter with unsafe column: %r", column)
return None
if isinstance(value, list):
quoted = [_quote_sql_literal(v) for v in value]
if any(q is None for q in quoted):
return None
return f"{safe_column} IN ({', '.join(quoted)})" # type: ignore[arg-type]
if isinstance(value, dict):
operator_info = self._parse_operator_dict(value)
if operator_info:
op, comp = operator_info
if op == "BETWEEN" and isinstance(comp, list) and len(comp) == 2:
lo = _quote_sql_literal(comp[0])
hi = _quote_sql_literal(comp[1])
if lo is None or hi is None:
return None
return f"{safe_column} BETWEEN {lo} AND {hi}"
comp_str = _quote_sql_literal(comp)
if comp_str is None:
return None
return f"{safe_column} {op} {comp_str}"
return None
val_str = _quote_sql_literal(value)
if val_str is None:
return None
return f"{safe_column} = {val_str}"
def _handle_not_operator(self, not_value: Any) -> list[dict[str, Any]]:
if isinstance(not_value, dict):
filters: list[dict[str, Any]] = []
for col, val in not_value.items():
if col not in ("OR", "NOT"):
filter_dict = self._create_filter(col, val, negate=True)
if filter_dict:
filters.append(filter_dict)
return filters
return []
def merge_rison_filters(form_data: dict[str, Any]) -> None:
"""
Merge Rison filters from 'f' parameter into form_data.
Modifies form_data in place.
"""
parser = RisonFilterParser()
if rison_filters := parser.parse():
existing_filters = form_data.get("adhoc_filters", [])
form_data["adhoc_filters"] = existing_filters + rison_filters
logger.info("Added %d filters from Rison parameter", len(rison_filters))

View File

@@ -97,26 +97,6 @@ from superset.security.analytics_db_safety import check_sqlalchemy_uri
True,
"shillelagh cannot be used as a data source for security reasons.",
),
(
"duckdb:///:memory:",
True,
"duckdb cannot be used as a data source for security reasons.",
),
(
"duckdb:////tmp/local.db",
True,
"duckdb cannot be used as a data source for security reasons.",
),
(
"duckdb+duckdb_engine:////tmp/local.db",
True,
"duckdb cannot be used as a data source for security reasons.",
),
(
"duckdb:///md:my_db?motherduck_token=tok",
True,
"duckdb cannot be used as a data source for security reasons.",
),
],
)
def test_check_sqlalchemy_uri(

View File

@@ -1,467 +0,0 @@
# 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.
from datetime import datetime, timezone
from unittest.mock import MagicMock, patch
import pytest
from fastmcp import Client
from superset.mcp_service.annotation_layer.schemas import (
CreateLayerAnnotationRequest,
)
from superset.mcp_service.app import mcp
from superset.utils import json
@pytest.fixture
def mcp_server():
return mcp
@pytest.fixture(autouse=True)
def mock_auth():
"""Mock authentication for all tests."""
from unittest.mock import Mock
with patch("superset.mcp_service.auth.get_user_from_request") as mock_get_user:
mock_user = Mock()
mock_user.id = 1
mock_user.username = "admin"
mock_get_user.return_value = mock_user
yield mock_get_user
def _make_request(**kwargs) -> CreateLayerAnnotationRequest:
defaults = {
"layer_id": 1,
"short_descr": "Deploy v2.0",
"start_dttm": datetime(2024, 1, 15, 8, 0, tzinfo=timezone.utc),
"end_dttm": datetime(2024, 1, 15, 9, 0, tzinfo=timezone.utc),
}
defaults.update(kwargs)
return CreateLayerAnnotationRequest(**defaults)
def _make_mock_annotation(
id: int = 42,
short_descr: str = "Deploy v2.0",
long_descr: str | None = None,
) -> MagicMock:
annotation = MagicMock()
annotation.id = id
annotation.short_descr = short_descr
annotation.long_descr = long_descr
annotation.start_dttm = datetime(2024, 1, 15, 8, 0, tzinfo=timezone.utc)
annotation.end_dttm = datetime(2024, 1, 15, 9, 0, tzinfo=timezone.utc)
return annotation
# --- Schema tests ---
def test_request_valid() -> None:
req = _make_request()
assert req.layer_id == 1
assert req.short_descr == "Deploy v2.0"
assert req.long_descr is None
assert req.json_metadata is None
def test_request_short_descr_too_long() -> None:
from pydantic import ValidationError
with pytest.raises(ValidationError):
_make_request(short_descr="x" * 501)
def test_request_end_before_start_is_allowed_at_schema_level() -> None:
# Date ordering is enforced by the command, not the Pydantic schema
req = _make_request(
start_dttm=datetime(2024, 1, 15, 10, 0),
end_dttm=datetime(2024, 1, 15, 8, 0),
)
assert req.start_dttm > req.end_dttm
def test_request_invalid_json_metadata_fails() -> None:
from pydantic import ValidationError
with pytest.raises(ValidationError, match="json_metadata must be valid JSON"):
_make_request(json_metadata="not-json{")
def test_request_valid_json_metadata() -> None:
req = _make_request(json_metadata='{"key": "value"}')
assert req.json_metadata == '{"key": "value"}'
# --- Tool logic tests ---
@pytest.mark.asyncio
async def test_create_layer_annotation_success(mcp_server: object) -> None:
"""Happy path: annotation created, id and fields returned."""
mock_annotation = _make_mock_annotation(id=42, short_descr="Deploy v2.0")
mock_command = MagicMock()
mock_command.run.return_value = mock_annotation
with patch(
"superset.commands.annotation_layer.annotation.create.CreateAnnotationCommand",
return_value=mock_command,
):
async with Client(mcp_server) as client:
request = _make_request()
result = await client.call_tool(
"create_layer_annotation", {"request": request.model_dump()}
)
data = json.loads(result.content[0].text)
assert data["id"] == 42
assert data["layer_id"] == 1
assert data["short_descr"] == "Deploy v2.0"
assert data["error"] is None
@pytest.mark.asyncio
async def test_create_layer_annotation_layer_not_found(mcp_server: object) -> None:
"""AnnotationLayerNotFoundError returns structured error response."""
from superset.commands.annotation_layer.exceptions import (
AnnotationLayerNotFoundError,
)
mock_command = MagicMock()
mock_command.run.side_effect = AnnotationLayerNotFoundError()
with patch(
"superset.commands.annotation_layer.annotation.create.CreateAnnotationCommand",
return_value=mock_command,
):
async with Client(mcp_server) as client:
request = _make_request(layer_id=999)
result = await client.call_tool(
"create_layer_annotation", {"request": request.model_dump()}
)
data = json.loads(result.content[0].text)
assert data["id"] is None
assert data["layer_id"] == 999
assert data["error"] is not None
assert "999" in data["error"]
@pytest.mark.asyncio
async def test_create_layer_annotation_invalid_error(mcp_server: object) -> None:
"""AnnotationInvalidError (e.g. duplicate short_descr) returns structured error."""
from superset.commands.annotation_layer.annotation.exceptions import (
AnnotationInvalidError,
AnnotationUniquenessValidationError,
)
invalid_exc = AnnotationInvalidError()
invalid_exc.append(AnnotationUniquenessValidationError())
mock_command = MagicMock()
mock_command.run.side_effect = invalid_exc
with patch(
"superset.commands.annotation_layer.annotation.create.CreateAnnotationCommand",
return_value=mock_command,
):
async with Client(mcp_server) as client:
request = _make_request()
result = await client.call_tool(
"create_layer_annotation", {"request": request.model_dump()}
)
data = json.loads(result.content[0].text)
assert data["id"] is None
assert data["error"] is not None
@pytest.mark.asyncio
async def test_create_layer_annotation_create_failed(mcp_server: object) -> None:
"""AnnotationCreateFailedError is caught and returned as error response."""
from superset.commands.annotation_layer.annotation.exceptions import (
AnnotationCreateFailedError,
)
mock_command = MagicMock()
mock_command.run.side_effect = AnnotationCreateFailedError()
with patch(
"superset.commands.annotation_layer.annotation.create.CreateAnnotationCommand",
return_value=mock_command,
):
async with Client(mcp_server) as client:
request = _make_request()
result = await client.call_tool(
"create_layer_annotation", {"request": request.model_dump()}
)
data = json.loads(result.content[0].text)
assert data["id"] is None
assert data["error"] is not None
assert "Failed to create annotation" in data["error"]
@pytest.mark.asyncio
async def test_create_layer_annotation_optional_fields_forwarded(
mcp_server: object,
) -> None:
"""long_descr and json_metadata are forwarded to CreateAnnotationCommand."""
mock_annotation = _make_mock_annotation(
id=5, short_descr="Outage", long_descr="DB connectivity lost"
)
mock_command_instance = MagicMock()
mock_command_instance.run.return_value = mock_annotation
mock_command_cls = MagicMock(return_value=mock_command_instance)
with patch(
"superset.commands.annotation_layer.annotation.create.CreateAnnotationCommand",
mock_command_cls,
):
async with Client(mcp_server) as client:
request = _make_request(
short_descr="Outage",
long_descr="DB connectivity lost",
json_metadata='{"severity": "high"}',
)
await client.call_tool(
"create_layer_annotation", {"request": request.model_dump()}
)
props = mock_command_cls.call_args[0][0]
assert props["long_descr"] == "DB connectivity lost"
assert props["json_metadata"] == '{"severity": "high"}'
# ---------------------------------------------------------------------------
# update_layer_annotation tests
# ---------------------------------------------------------------------------
from superset.mcp_service.annotation_layer.schemas import ( # noqa: E402
UpdateLayerAnnotationRequest,
)
def _make_update_request(**kwargs) -> UpdateLayerAnnotationRequest:
defaults: dict[str, object] = {
"layer_id": 1,
"annotation_id": 42,
}
defaults.update(kwargs)
return UpdateLayerAnnotationRequest(**defaults)
# --- Schema tests ---
def test_update_request_valid() -> None:
req = _make_update_request()
assert req.layer_id == 1
assert req.annotation_id == 42
assert req.short_descr is None
def test_update_request_short_descr_too_long() -> None:
from pydantic import ValidationError
with pytest.raises(ValidationError):
_make_update_request(short_descr="x" * 501)
def test_update_request_invalid_json_metadata_fails() -> None:
from pydantic import ValidationError
with pytest.raises(ValidationError, match="json_metadata must be valid JSON"):
_make_update_request(json_metadata="not-json{")
def test_update_request_valid_json_metadata() -> None:
req = _make_update_request(json_metadata='{"key": "value"}')
assert req.json_metadata == '{"key": "value"}'
# --- Tool logic tests ---
@pytest.mark.asyncio
async def test_update_layer_annotation_success(mcp_server: object) -> None:
"""Happy path: annotation updated, id and fields returned."""
mock_annotation = _make_mock_annotation(id=42, short_descr="Fixed title")
mock_command = MagicMock()
mock_command.run.return_value = mock_annotation
with patch(
"superset.commands.annotation_layer.annotation.update.UpdateAnnotationCommand",
return_value=mock_command,
):
async with Client(mcp_server) as client:
request = _make_update_request(short_descr="Fixed title")
result = await client.call_tool(
"update_layer_annotation", {"request": request.model_dump()}
)
data = json.loads(result.content[0].text)
assert data["id"] == 42
assert data["layer_id"] == 1
assert data["short_descr"] == "Fixed title"
assert data["error"] is None
@pytest.mark.asyncio
async def test_update_layer_annotation_not_found(mcp_server: object) -> None:
"""AnnotationNotFoundError returns structured error response."""
from superset.commands.annotation_layer.annotation.exceptions import (
AnnotationNotFoundError,
)
mock_command = MagicMock()
mock_command.run.side_effect = AnnotationNotFoundError()
with patch(
"superset.commands.annotation_layer.annotation.update.UpdateAnnotationCommand",
return_value=mock_command,
):
async with Client(mcp_server) as client:
request = _make_update_request(annotation_id=999)
result = await client.call_tool(
"update_layer_annotation", {"request": request.model_dump()}
)
data = json.loads(result.content[0].text)
assert data["id"] is None
assert data["error"] is not None
assert "999" in data["error"]
@pytest.mark.asyncio
async def test_update_layer_annotation_layer_not_found(mcp_server: object) -> None:
"""AnnotationLayerNotFoundError returns structured error response."""
from superset.commands.annotation_layer.exceptions import (
AnnotationLayerNotFoundError,
)
mock_command = MagicMock()
mock_command.run.side_effect = AnnotationLayerNotFoundError()
with patch(
"superset.commands.annotation_layer.annotation.update.UpdateAnnotationCommand",
return_value=mock_command,
):
async with Client(mcp_server) as client:
request = _make_update_request(layer_id=999)
result = await client.call_tool(
"update_layer_annotation", {"request": request.model_dump()}
)
data = json.loads(result.content[0].text)
assert data["id"] is None
assert data["error"] is not None
assert "999" in data["error"]
@pytest.mark.asyncio
async def test_update_layer_annotation_invalid_error(mcp_server: object) -> None:
"""AnnotationInvalidError (e.g. duplicate short_descr) returns structured error."""
from superset.commands.annotation_layer.annotation.exceptions import (
AnnotationInvalidError,
AnnotationUniquenessValidationError,
)
invalid_exc = AnnotationInvalidError()
invalid_exc.append(AnnotationUniquenessValidationError())
mock_command = MagicMock()
mock_command.run.side_effect = invalid_exc
with patch(
"superset.commands.annotation_layer.annotation.update.UpdateAnnotationCommand",
return_value=mock_command,
):
async with Client(mcp_server) as client:
request = _make_update_request(short_descr="Duplicate")
result = await client.call_tool(
"update_layer_annotation", {"request": request.model_dump()}
)
data = json.loads(result.content[0].text)
assert data["id"] is None
assert data["error"] is not None
@pytest.mark.asyncio
async def test_update_layer_annotation_update_failed(mcp_server: object) -> None:
"""AnnotationUpdateFailedError is caught and returned as error response."""
from superset.commands.annotation_layer.annotation.exceptions import (
AnnotationUpdateFailedError,
)
mock_command = MagicMock()
mock_command.run.side_effect = AnnotationUpdateFailedError()
with patch(
"superset.commands.annotation_layer.annotation.update.UpdateAnnotationCommand",
return_value=mock_command,
):
async with Client(mcp_server) as client:
request = _make_update_request()
result = await client.call_tool(
"update_layer_annotation", {"request": request.model_dump()}
)
data = json.loads(result.content[0].text)
assert data["id"] is None
assert data["error"] is not None
assert "Failed to update annotation" in data["error"]
@pytest.mark.asyncio
async def test_update_layer_annotation_only_provided_fields_forwarded(
mcp_server: object,
) -> None:
"""Only non-None fields are forwarded to UpdateAnnotationCommand."""
mock_annotation = _make_mock_annotation(id=42)
mock_command_instance = MagicMock()
mock_command_instance.run.return_value = mock_annotation
mock_command_cls = MagicMock(return_value=mock_command_instance)
with patch(
"superset.commands.annotation_layer.annotation.update.UpdateAnnotationCommand",
mock_command_cls,
):
async with Client(mcp_server) as client:
request = _make_update_request(
short_descr="New title",
long_descr="Updated description",
)
await client.call_tool(
"update_layer_annotation", {"request": request.model_dump()}
)
# annotation_id is the first positional arg, properties is the second
call_args = mock_command_cls.call_args
annotation_id_arg = call_args[0][0]
props = call_args[0][1]
assert annotation_id_arg == 42
assert props["short_descr"] == "New title"
assert props["long_descr"] == "Updated description"
# Fields not provided should not be in properties
assert "start_dttm" not in props
assert "end_dttm" not in props
assert "json_metadata" not in props

View File

@@ -1,183 +0,0 @@
# 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 Rison filter parser."""
from superset.utils.rison_filters import RisonFilterParser
def test_simple_equality():
parser = RisonFilterParser()
result = parser.parse("(country:USA)")
assert len(result) == 1
assert result[0]["expressionType"] == "SIMPLE"
assert result[0]["clause"] == "WHERE"
assert result[0]["subject"] == "country"
assert result[0]["operator"] == "=="
assert result[0]["comparator"] == "USA"
def test_multiple_filters_and():
parser = RisonFilterParser()
result = parser.parse("(country:USA,year:2024)")
assert len(result) == 2
assert result[0]["subject"] == "country"
assert result[0]["comparator"] == "USA"
assert result[1]["subject"] == "year"
assert result[1]["comparator"] == 2024
def test_list_in_operator():
parser = RisonFilterParser()
result = parser.parse("(country:!(USA,Canada))")
assert len(result) == 1
assert result[0]["subject"] == "country"
assert result[0]["operator"] == "IN"
assert result[0]["comparator"] == ["USA", "Canada"]
def test_not_operator():
parser = RisonFilterParser()
result = parser.parse("(NOT:(country:USA))")
assert len(result) == 1
assert result[0]["subject"] == "country"
assert result[0]["operator"] == "!="
assert result[0]["comparator"] == "USA"
def test_not_in_operator():
parser = RisonFilterParser()
result = parser.parse("(NOT:(country:!(USA,Canada)))")
assert len(result) == 1
assert result[0]["subject"] == "country"
assert result[0]["operator"] == "NOT IN"
assert result[0]["comparator"] == ["USA", "Canada"]
def test_or_operator():
parser = RisonFilterParser()
result = parser.parse("(OR:!((status:active),(priority:high)))")
assert len(result) == 1
assert result[0]["expressionType"] == "SQL"
assert result[0]["clause"] == "WHERE"
assert "status = 'active' OR priority = 'high'" in result[0]["sqlExpression"]
def test_comparison_operators():
parser = RisonFilterParser()
result = parser.parse("(sales:(gt:100000))")
assert result[0]["operator"] == ">"
assert result[0]["comparator"] == 100000
result = parser.parse("(age:(gte:18))")
assert result[0]["operator"] == ">="
assert result[0]["comparator"] == 18
result = parser.parse("(temp:(lt:32))")
assert result[0]["operator"] == "<"
assert result[0]["comparator"] == 32
result = parser.parse("(price:(lte:1000))")
assert result[0]["operator"] == "<="
assert result[0]["comparator"] == 1000
def test_between_operator():
parser = RisonFilterParser()
result = parser.parse("(date:(between:!('2024-01-01','2024-12-31')))")
assert len(result) == 1
assert result[0]["operator"] == "BETWEEN"
assert result[0]["comparator"] == ["2024-01-01", "2024-12-31"]
def test_like_operator():
parser = RisonFilterParser()
result = parser.parse("(name:(like:'%smith%'))")
assert len(result) == 1
assert result[0]["operator"] == "LIKE"
assert result[0]["comparator"] == "%smith%"
def test_empty_filter():
parser = RisonFilterParser()
assert parser.parse("") == []
assert parser.parse("()") == []
def test_invalid_rison():
parser = RisonFilterParser()
assert parser.parse("invalid rison") == []
assert parser.parse("(unclosed") == []
def test_or_branch_escapes_string_literals():
"""OR-branch SQL must escape apostrophes; otherwise a value like
`O'Brien'); DROP TABLE x; --` could close out a literal and inject.
Bypasses the Rison parser (whose quoting rules complicate expressing
apostrophes in test inputs) and exercises `_handle_or_operator` with
a hand-built decoded payload — which is exactly what `_convert_to_adhoc_filters`
feeds it after a successful prison.loads.
"""
parser = RisonFilterParser()
result = parser._handle_or_operator(
[{"name": "O'Brien"}, {"role": "admin"}],
)
assert len(result) == 1
sql = result[0]["sqlExpression"]
# Single quote inside the literal is doubled per SQL standard.
assert "'O''Brien'" in sql
assert "'admin'" in sql
def test_or_branch_rejects_unsafe_identifiers():
"""Columns that don't match a strict identifier whitelist drop the
condition rather than getting interpolated into SQL."""
parser = RisonFilterParser()
result = parser._handle_or_operator(
[{"col; DROP TABLE x": 1}, {"role": "admin"}],
)
# The unsafe column is dropped, the safe one remains.
assert len(result) == 1
sql = result[0]["sqlExpression"]
assert "DROP" not in sql
assert "role = 'admin'" in sql
def test_or_branch_all_conditions_unsafe_returns_nothing():
parser = RisonFilterParser()
result = parser._handle_or_operator(
[{"col;1": 1}, {"col;2": 2}],
)
assert result == []
def test_or_branch_between_quotes_both_bounds():
parser = RisonFilterParser()
result = parser.parse("(OR:!((date:(between:!('2024-01-01','2024-12-31')))))")
sql = result[0]["sqlExpression"]
assert "date BETWEEN '2024-01-01' AND '2024-12-31'" in sql