Skip to content

fix(AlertsReports): validate anchor_list is a list#38723

Merged
msyavuz merged 1 commit into
masterfrom
msyavuz/fix/alert-report-filters-anchor
Mar 23, 2026
Merged

fix(AlertsReports): validate anchor_list is a list#38723
msyavuz merged 1 commit into
masterfrom
msyavuz/fix/alert-report-filters-anchor

Conversation

@msyavuz
Copy link
Copy Markdown
Member

@msyavuz msyavuz commented Mar 18, 2026

User description

SUMMARY

This pr fixes an edge case where anchor_link is not a list and split into individual characters. We now check for it and raise an error if so.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

  1. Create a dashboard report with the ALERT_REPORT_TABS feature flag enabled
  2. Manually set the report's extra.dashboard.anchor to a JSON-encoded string value (e.g., '"TAB-A"' — note the double quoting)
  3. Trigger the report execution
  4. Before fix: the code iterates over "TAB-A" character by character, generating invalid URLs for "T", "A", "B", "-", "A"
  5. After fix: the code detects the parsed value isn't a list, raises JSONDecodeError, and falls back to the single-tab URL path — same behavior as a plain non-JSON string

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

CodeAnt-AI Description

Fix dashboard report anchor handling to avoid iterating over non-list values

What Changed

  • When a report's dashboard anchor is JSON-encoded but not a list (e.g., a JSON string), the code now detects that and falls back to the single-tab dashboard URL instead of treating the string as a list of characters and generating invalid tab URLs.
  • Added a unit test that verifies a JSON scalar anchor (a JSON-encoded string) falls back to the single-tab URL path.

Impact

✅ Fewer invalid dashboard tab URLs in alert/report exports
✅ Clearer single-tab fallback when anchor value is malformed
✅ Fewer broken alert report executions caused by malformed anchor data

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

@dosubot dosubot Bot added the alert-reports Namespace | Anything related to the Alert & Reports feature label Mar 18, 2026
@codeant-ai-for-open-source codeant-ai-for-open-source Bot added the size:S This PR changes 10-29 lines, ignoring generated files label Mar 18, 2026
@codeant-ai-for-open-source
Copy link
Copy Markdown
Contributor

Sequence Diagram

This PR adds a type check after parsing the dashboard anchor so only list values are treated as tab anchors. If the parsed anchor is not a list, execution now follows the existing fallback path and returns a single dashboard URL.

sequenceDiagram
    participant Scheduler
    participant ReportExecutor
    participant JsonParser
    participant UrlBuilder

    Scheduler->>ReportExecutor: Execute dashboard report
    ReportExecutor->>JsonParser: Parse dashboard anchor JSON

    alt Parsed value is a list
        JsonParser-->>ReportExecutor: Return tab anchor list
        ReportExecutor->>UrlBuilder: Build URLs for selected tabs
        UrlBuilder-->>ReportExecutor: Return tab URLs
    else Parsed value is not a list or invalid JSON
        JsonParser-->>ReportExecutor: Return decode error
        ReportExecutor->>UrlBuilder: Build single dashboard URL
        UrlBuilder-->>ReportExecutor: Return single URL
    end

    ReportExecutor-->>Scheduler: Return dashboard URLs
Loading

Generated by CodeAnt AI

Comment on lines +271 to +275
anchor_list = json.loads(anchor)
if not isinstance(anchor_list, list):
raise json.JSONDecodeError(
"Anchor value is not a list", anchor, 0
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: json.loads is called directly on anchor without checking its runtime type. If stored report metadata contains a non-string anchor (for example a legacy/prevalidated list), json.loads raises TypeError, which is not caught by the except json.JSONDecodeError block and can fail report execution instead of falling back safely. [type error]

Severity Level: Major ⚠️
- ❌ Dashboard report execution can crash on malformed anchor type.
- ❌ Scheduled report emails/screenshots may not be generated.
- ⚠️ Celery reports.execute task logs unexpected execution errors.
Suggested change
anchor_list = json.loads(anchor)
if not isinstance(anchor_list, list):
raise json.JSONDecodeError(
"Anchor value is not a list", anchor, 0
)
if isinstance(anchor, list):
anchor_list = anchor
elif isinstance(anchor, str):
anchor_list = json.loads(anchor)
else:
raise json.JSONDecodeError(
"Anchor value is not a list", str(anchor), 0
)
if not isinstance(anchor_list, list):
raise json.JSONDecodeError(
"Anchor value is not a list", anchor, 0
)
Steps of Reproduction ✅
1. Update an existing report via `PUT /api/v1/report/<id>` (API class
`ReportScheduleRestApi` in `superset/reports/api.py:66`, using `ReportSchedulePutSchema`
at `superset/reports/api.py:194`), setting `extra.dashboard.anchor` to a JSON array/list
value instead of a string.

2. This payload is accepted because `extra` is an untyped dict (`fields.Dict`) in
`superset/reports/schemas.py:371`, and `UpdateReportScheduleCommand.validate()`
(`superset/commands/report/update.py:58`) does not call `_validate_report_extra` (only
create does in `superset/commands/report/create.py:143`).

3. When the schedule runs (`reports.execute` Celery task in
`superset/tasks/scheduler.py:116-134`), execution reaches dashboard screenshot generation,
which calls `get_dashboard_urls()` from `_get_screenshots()`
(`superset/commands/report/execute.py:385`).

4. In `get_dashboard_urls()` (`superset/commands/report/execute.py:271`),
`json.loads(anchor)` is called on a Python list, raising `TypeError`; the handler only
catches `json.JSONDecodeError` at `superset/commands/report/execute.py:282`, so execution
fails instead of graceful fallback.
Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** superset/commands/report/execute.py
**Line:** 271:275
**Comment:**
	*Type Error: `json.loads` is called directly on `anchor` without checking its runtime type. If stored report metadata contains a non-string `anchor` (for example a legacy/prevalidated list), `json.loads` raises `TypeError`, which is not caught by the `except json.JSONDecodeError` block and can fail report execution instead of falling back safely.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

Copy link
Copy Markdown
Contributor

@bito-code-review bito-code-review Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Agent Run #ee0d0e

Actionable Suggestions - 1
  • superset/commands/report/execute.py - 1
Review Details
  • Files reviewed - 2 · Commit Range: c31eb44..c31eb44
    • superset/commands/report/execute.py
    • tests/unit_tests/commands/report/execute_test.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

Comment on lines +271 to +275
anchor_list = json.loads(anchor)
if not isinstance(anchor_list, list):
raise json.JSONDecodeError(
"Anchor value is not a list", anchor, 0
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incomplete input validation

The validation checks if anchor_list is a list but doesn't verify that all elements are strings, as expected by _get_tabs_urls(tab_anchors: list[str]). If the JSON contains a list of non-strings (e.g., integers), it will proceed and likely cause a runtime error later when creating permalinks. Consider adding a check for element types to match the intended list[str] type.

Code Review Run #ee0d0e


Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them

@msyavuz msyavuz added the hold:testing! On hold for testing label Mar 18, 2026
@sadpandajoe sadpandajoe removed the hold:testing! On hold for testing label Mar 20, 2026
@msyavuz msyavuz merged commit 100ad7d into master Mar 23, 2026
85 of 91 checks passed
@msyavuz msyavuz deleted the msyavuz/fix/alert-report-filters-anchor branch March 23, 2026 15:20
michael-s-molina pushed a commit that referenced this pull request Mar 24, 2026
qfcwell pushed a commit to qfcwell/superset that referenced this pull request May 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

alert-reports Namespace | Anything related to the Alert & Reports feature preset-io size/S size:S This PR changes 10-29 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants