Skip to content

fix(database): extend shillelagh URI pattern to cover all driver variants#39995

Merged
dpgaspar merged 1 commit into
apache:masterfrom
sha174n:fix/shillelagh-uri-blocklist
May 15, 2026
Merged

fix(database): extend shillelagh URI pattern to cover all driver variants#39995
dpgaspar merged 1 commit into
apache:masterfrom
sha174n:fix/shillelagh-uri-blocklist

Conversation

@sha174n
Copy link
Copy Markdown
Contributor

@sha174n sha174n commented May 10, 2026

SUMMARY

The shillelagh URI blocklist in analytics_db_safety.py used two separate patterns (shillelagh$ and shillelagh\+apsw$) that only matched the bare driver and one specific variant. Other driver suffixes such as +csv, +json, and +gsheets were not matched, allowing those URIs to bypass the blocklist check.

This PR replaces both patterns with a single regex shillelagh(?:\+[^\s]*)?$ that blocks shillelagh regardless of the driver suffix, consistent with how the existing sqlite pattern already handles all its variants via sqlite(?:\+[^\s]*)?$.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A — backend-only change.

TESTING INSTRUCTIONS

  1. Run the updated integration test:

    pytest tests/integration_tests/security/analytics_db_safety_tests.py -v

    All 15 test cases should pass, including the new shillelagh+csv, shillelagh+json, and shillelagh+gsheets variants.

  2. Attempt to create a database connection with sqlalchemy_uri: shillelagh+csv:///etc/passwd via the API — should return a 422 error, not 201.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration
  • Introduces new feature or API
  • Removes existing feature or API

…ants

Replace two separate shillelagh regexes with a single pattern that
blocks shillelagh regardless of the driver suffix, preventing URI
variants like shillelagh+csv or shillelagh+json from bypassing the
blocklist.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented May 10, 2026

Code Review Agent Run #b8d2bb

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: 9cf70ba..9cf70ba
    • superset/security/analytics_db_safety.py
    • tests/integration_tests/security/analytics_db_safety_tests.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

@dosubot dosubot Bot added the change:backend Requires changing the backend label May 10, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 10, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.83%. Comparing base (f67dd4a) to head (9cf70ba).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #39995   +/-   ##
=======================================
  Coverage   63.83%   63.83%           
=======================================
  Files        2589     2589           
  Lines      137821   137821           
  Branches    31928    31928           
=======================================
  Hits        87978    87978           
  Misses      48327    48327           
  Partials     1516     1516           
Flag Coverage Δ
hive 39.36% <ø> (ø)
mysql 59.01% <ø> (ø)
postgres 59.09% <ø> (ø)
presto 41.06% <ø> (ø)
python 60.53% <ø> (ø)
sqlite 58.73% <ø> (ø)
unit 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@rusackas rusackas requested a review from betodealmeida May 12, 2026 03:21
@dpgaspar dpgaspar merged commit 407321e into apache:master May 15, 2026
68 of 69 checks passed
sha174n added a commit to sha174n/superset that referenced this pull request May 15, 2026
…ants (apache#39995)

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:backend Requires changing the backend size/S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants