fix(embedded): allow guest users to sort by visible columns by YuriyKrasilnikov · Pull Request #37371 · apache/superset

YuriyKrasilnikov · 2026-01-22T22:06:44Z

Summary

Fixes #37061: Guest users in embedded dashboards can now sort table columns.

Before: Clicking any column header → "Guest user cannot modify chart payload"
After: Sorting by visible columns works; hidden columns and SQL injection still blocked.

Problem Analysis

Root Cause

query_context_modified() treated orderby identically to metrics/columns:

for key, equivalent in [
    ("metrics", ["metrics"]),
    ("columns", ["columns", "groupby"]),
    ("orderby", ["orderby"]),  # ← Same logic as metrics
]:
    if not requested.issubset(stored):
        return True  # BLOCKED

Issue: User clicks column → frontend sends new orderby → orderby ≠ stored_orderby → blocked.

Why This Is Wrong

Field	Expected Behavior
metrics	Strict subset of stored (security)
columns	Strict subset of stored (security)
orderby	Can be ANY visible column (UX requirement)

Sorting is navigation, not data access expansion.

Security Research

Before implementing, analyzed attack vectors:

Attack Vector Analysis

Vector	Risk	Example	Our Mitigation
SQL Injection	HIGH	`ORDER BY random()`, `sleep(5)`	Block `expressionType: "SQL"`
Data Exfiltration	HIGH	`ORDER BY credit_card_number` reveals ranking	Whitelist visible columns only
RLS Bypass	MEDIUM	Sort by filtered column reveals existence	Whitelist prevents column probing
DoS	LOW	Expensive sort on unindexed column	Existing query timeouts apply

Industry Standard

Platform	Approach
Looker	Whitelist via LookML
Metabase	Visible fields only
Tableau	Pre-configured sorts
Our solution	Whitelist by visible columns

Mathematical Model

V = visible columns (columns ∪ groupby ∪ metrics)
S = requested sort columns

Rule: S ⊆ V (sort only by what user can see)

Architecture Decision

Why Not Just Remove orderby Check?

# Option A: Remove orderby from loop (REJECTED)
for key in ["metrics", "columns", "groupby"]:  # no orderby

Rejected because: Opens SQL injection and data exfiltration vectors.

Why Not Frontend-Only Sorting?

Rejected because: Client-side sort on 1000 rows ≠ database sort on 100000 rows. Wrong results.

Why Comparator Pattern?

During implementation, discovered a bug in existing code:

# EXISTING BUG (lines 260-264):
for key, equivalent in [...]:
    for key in equivalent:  # ← key OVERWRITTEN!
        stored_values.update(...)
# After inner loop: key = last value of equivalent, not original key

This bug affects iterations 2-4 of the outer loop.

Solution: Refactor to explicit variables + custom comparator for orderby:

for mapping in _FIELD_MAPPINGS:
    field_name = mapping["field"]      # explicit, not reused
    equiv_fields = mapping["equivalent"]
    comparator = mapping.get("comparator", default_compare)

    for equiv_field in equiv_fields:   # different variable name
        ...

Why TypedDict?

class FieldMapping(TypedDict):
    field: str
    equivalent: list[str]
    comparator: NotRequired[Callable[...]]

Type safety: MyPy catches errors
Extensibility: Add new comparators without changing loop
Documentation: Structure is self-documenting

Implementation Details

New Functions

Function	Purpose	Returns
`_get_visible_columns(chart)`	Extract columns ∪ groupby ∪ metrics	`set[str]`
`_extract_orderby_column_name(item)`	Normalize orderby format; None = unsafe SQL	`str \| None`
`_orderby_whitelist_compare(ctx, chart, visible)`	Whitelist validation for orderby	`bool` (True = block)
`_default_field_compare(...)`	Default subset comparison (extracted for complexity)	`bool`

Why `_extract_orderby_column_name` Returns None for SQL?

if orderby_item.get("expressionType") == "SQL":
    return None  # Triggers block

Reason: SQL expressions can contain:

random() — non-deterministic, DoS potential
pg_sleep(5) — timing attack
(SELECT password FROM users LIMIT 1) — data exfiltration

Why Check Both form_data AND queries?

# Check form_data.orderby
for orderby_tuple in form_data.get("orderby") or []:
    ...

# Check query_context.queries[].orderby
for query in query_context.queries:
    for orderby_tuple in getattr(query, "orderby", None) or []:
        ...

Reason: Attacker could send valid form_data.orderby but inject malicious query.orderby. Both must be validated.

Why `equiv_field` Instead of `key`?

Reason: Python loop variables leak into outer scope. Reusing key corrupted subsequent iterations.

Changes Summary

File	Change
`superset/security/manager.py`	+FieldMapping TypedDict, +4 functions, refactor loop
`tests/unit_tests/security/manager_test.py`	+3 tests for whitelist behavior

New Tests

Test	Scenario	Expected
`test_..._visible_column_allowed`	Sort by column in chart	✅ Allowed
`test_..._hidden_column_blocked`	Sort by `credit_card_number`	❌ Blocked
`test_..._direction_change_allowed`	Change ASC↔DESC	✅ Allowed

Existing test_query_context_modified_orderby (SQL injection) continues to pass.

Backward Compatibility

Aspect	Impact
API	No change
Behavior for non-guest users	No change
Behavior for guest users	Sorting now works
Existing tests	All pass (28/28)

How To Test

Create embedded dashboard with Table chart
Get guest token
Click column header to sort
Before: Error "Guest user cannot modify chart payload"
After: Table sorts correctly

Checklist

Security research completed (4 attack vectors analyzed)
Architecture decision documented
Bug fix: variable shadowing in loop
Unit tests for whitelist behavior
Existing tests pass
MyPy types added (TypedDict, type hints)
ruff/pylint pass

bito-code-review · 2026-01-22T22:06:55Z

Code Review Agent Run #d2ccfe

Actionable Suggestions - 0

Review Details

Files reviewed - 2 · Commit Range: 10637f4..10637f4
- superset/security/manager.py
- tests/unit_tests/security/manager_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

YuriyKrasilnikov · 2026-01-23T16:26:10Z

Response to codeant-ai bot review

The bot's suggestion is incorrect

The bot suggested supporting {"column": "string"} format. After thorough investigation of Superset's type system, this format does not exist:

# superset/superset_typing.py:51-57
class AdhocMetric(TypedDict, total=False):
    column: AdhocMetricColumn | None  # ← DICT or None, NEVER string!

# superset-frontend/packages/superset-ui-core/src/query/types/Metric.ts
export interface AdhocMetricSimple {
  column: { column_name?: string, ... };  // ← Always DICT
}

Data Flow Analysis

Frontend (formData)
    ↓
saveModalActions.ts:172 → JSON.stringify(formData)
    ↓
/api/v1/chart/{id} (PUT/POST)
    ↓
charts/schemas.py → fields.Raw() ← NO STRUCTURE VALIDATION
    ↓
DATABASE: slices.params (MediumText)
    ↓
[Query Request]
    ↓
query_context_factory.py → ChartDAO.find_by_id()
    ↓
stored_chart.params_dict → json_to_dict(self.params)
    ↓
security/manager.py:_orderby_whitelist_compare() ← OUR BARRIERS

Key point: fields.Raw() in schema accepts ANY input without validation. Barriers must be in security code.

However, added defensive barriers anyway

Even though the bot's specific scenario is invalid, applied fail-closed defensive coding. This protects against malformed API requests, 500 errors, and potential bypasses.

What added

1. Validate orderby is a list (_orderby_whitelist_compare):

if form_orderby is not None and not isinstance(form_orderby, list):
    return True  # block invalid format

Attack prevented:

form_data = {"orderby": "malicious"}  # string, not list
# Without barrier: iterates chars "m","a","l"... all skip → PASSES!
# With barrier: blocked immediately

2. Validate each element is tuple/list
3. Validate nested column is dict (already existed)

Similar patterns in Superset codebase

Our barriers follow existing Superset patterns:

Location	Pattern	Code
`security/manager.py:812`	isinstance before access	`if isinstance(datasource, BaseDatasource):`
`charts/client_processing.py:112`	Type check before use	`if not isinstance(df, pd.DataFrame):`
`models/slice.py:374`	Safe conversion	`if id_or_uuid.isdigit(): return int()`
`common/query_object.py:279`	Validation method	`def validate(): ... raise QueryObjectValidationError`
`views/base.py:337`	try/except with fallback	`except json.JSONDecodeError: return fallback`

Why defensive barriers even if bot was wrong

Reason	Explanation
Security-critical code	Must be fail-closed (block unknown, don't pass)
Schema accepts Raw()	No structure validation at API level
Defense in depth	Don't rely on "frontend won't send this"
Prevents 500 errors	Graceful block instead of crash

Tests added (20 new tests)

test_extract_orderby_column_name_* - unit tests for extraction
test_get_visible_columns_* - unit tests for whitelist
test_query_context_modified_orderby_* - integration tests for barriers
Including: test_query_context_modified_orderby_string_instead_of_list_blocked

netlify · 2026-01-23T16:27:04Z

✅ Deploy Preview for superset-docs-preview ready!

Name	Link
🔨 Latest commit	`9f1aacc`
🔍 Latest deploy log	https://app.netlify.com/projects/superset-docs-preview/deploys/6a45b0407b404e00082306ce
😎 Deploy Preview	https://deploy-preview-37371--superset-docs-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

bito-code-review · 2026-01-23T17:27:21Z

Code Review Agent Run #442218

Actionable Suggestions - 0

Review Details

Files reviewed - 2 · Commit Range: 10637f4..32990fb
- superset/security/manager.py
- tests/unit_tests/security/manager_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

codecov · 2026-01-28T19:44:23Z

Codecov Report

❌ Patch coverage is 52.06612% with 58 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.59%. Comparing base (246bbeb) to head (d68eece).

Files with missing lines	Patch %	Lines
superset/security/manager.py	54.08%	35 Missing and 10 partials ⚠️
superset/models/helpers.py	43.75%	8 Missing and 1 partial ⚠️
superset/connectors/sqla/models.py	42.85%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #37371      +/-   ##
==========================================
- Coverage   64.60%   64.59%   -0.01%     
==========================================
  Files        2684     2684              
  Lines      148192   148285      +93     
  Branches    34138    34167      +29     
==========================================
+ Hits        95738    95787      +49     
- Misses      50711    50743      +32     
- Partials     1743     1755      +12

Flag	Coverage Δ
hive	`39.19% <10.74%> (-0.04%)`	⬇️
mysql	`57.83% <52.06%> (-0.01%)`	⬇️
postgres	`57.89% <52.06%> (-0.01%)`	⬇️
presto	`40.73% <13.22%> (-0.04%)`	⬇️
python	`59.28% <52.06%> (-0.01%)`	⬇️
sqlite	`57.47% <52.06%> (-0.01%)`	⬇️
unit	`100.00% <ø> (ø)`

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

☔ View full report in Codecov by Harness.
📢 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.

YuriyKrasilnikov · 2026-01-28T21:10:12Z

Fixed in `81e9859`

ruff C901: Function too complex

Problem: _orderby_whitelist_compare had cyclomatic complexity 12 (max 10).

Fix: Extracted helper function _validate_orderby_list to handle orderby validation logic, reducing complexity of the main function.

YuriyKrasilnikov · 2026-01-28T21:27:43Z

Response to codeant-ai bot suggestions

Both suggestions are incorrect per Superset's type system (see previous analysis).

1. `{"column": "string"}` format

Does not exist. AdhocMetric.column is AdhocMetricColumn (dict) or None, never a plain string. Test at line 1360-1375 explicitly documents this as INVALID.

2. `tuple` as outer orderby container

Also incorrect. Type is list[OrderBy] (superset_typing.py:127), schema is fields.List (schemas.py:1345). The outer container is always list, not tuple. tuple is the inner element (OrderBy = tuple[Metric | Column, bool]).

No changes needed.

bito-code-review · 2026-01-28T22:11:34Z

Code Review Agent Run #5fc97b

Actionable Suggestions - 0

Review Details

Files reviewed - 2 · Commit Range: 32990fb..81e9859
- superset/security/manager.py
- tests/unit_tests/security/manager_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

rusackas · 2026-06-13T18:15:55Z

Thanks for the thorough root-cause writeup @YuriyKrasilnikov. It's gone conflicting (again, sorry), so we'll need a rebase to get it merged, but we're getting better at staying on top of all these PRs these days. Since this loosens guest-user query validation it wants a careful security pass, so CC @dpgaspar @mistercrunch, and myself once it's green, and/or reach out on Slack.

rusackas · 2026-06-19T17:03:27Z

Sorry about the confusion on the Issue... I'll try to rebase and resolve conflicts here, so we can get this through and close the issue for real!

Fixes apache#37061 Guest users in embedded dashboards can now sort table columns that are visible in the chart. Sorting by hidden columns or SQL expressions remains blocked for security. Changes: - Add FieldMapping TypedDict for extensible field configuration - Add _get_visible_columns() to extract columns/groupby/metrics - Add _extract_orderby_column_name() to normalize orderby format - Add _orderby_whitelist_compare() for whitelist validation - Add _default_field_compare() to reduce function complexity - Refactor query_context_modified() loop to use explicit variables - Fix variable shadowing bug (key reused in nested loop) - Add 3 tests for whitelist behavior

Add fail-closed defensive barriers to prevent malformed input from bypassing security checks or causing 500 errors. Changes: - _orderby_whitelist_compare(): validate orderby is list, each element is tuple/list, and not empty before processing - _extract_orderby_column_name(): validate nested column is dict - Added 20 unit tests covering valid formats and invalid edge cases Why barriers are needed: 1. Security-critical code should be fail-closed (block unknown, not pass) 2. Schema accepts fields.Raw() which doesn't validate structure 3. Prevents 500 errors on malformed input from API Example attack prevented: orderby: "malicious_string" # string instead of list Without barrier: iterates over characters, all skip, check passes With barrier: blocked immediately (fail-closed) Follows Superset defensive coding patterns: - isinstance checks before usage - Early return on invalid data - Comments explaining barrier purpose - No logging (consistent with security/ style)

Extract helper function to reduce cyclomatic complexity of _orderby_whitelist_compare from 12 to below 10 (ruff C901).

Two fixes made while rebasing this PR onto current master and reviewing it against the issue: 1. Reconcile the orderby visible-column whitelist with the native-filter time-grain handling on master. An orderby entry is allowed when it sorts by a visible column OR matches an entry the chart already sorts by (compared via freeze_value, which strips the guest-overridable timeGrain). Without this, a guest re-sending the chart's temporal x-axis orderby with a different grain was flagged as tampering (regressed test_query_context_modified_time_grain_in_orderby). 2. Include `all_columns` in the visible-column set. The Table plugin's raw "Query mode" stores its columns under `all_columns`, so raw-records tables (a common trigger of the reported guest sort error) would otherwise have all sorting blocked. Hidden columns and adhoc SQL expressions remain blocked (fail-closed). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Honor Table column_config visibility when deriving guest-sortable columns. Use Superset label helpers for column and metric labels so adhoc SIMPLE metrics without custom labels match their result keys. Require new guest orderby entries to be [term, bool] and block malformed SIMPLE metric dicts.

bito-code-review · 2026-06-22T01:21:01Z

Code Review Agent Run #ca7b5d

Actionable Suggestions - 0

Review Details

Files reviewed - 2 · Commit Range: aef22bf..b68d882
- superset/security/manager.py
- tests/unit_tests/security/manager_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

YuriyKrasilnikov · 2026-06-22T03:46:43Z

@rusackas thanks again for rebasing this and for the security-review guidance.

I pushed one more pass in b68d882 and the PR is green now, including pre-commit (current), unit-tests (current), and unit-tests-required.

I re-traced the guest sorting path from the Table frontend down to backend validation and tightened the security boundary a bit more:

the guest orderby whitelist now respects Table column_config[<columnKey>].visible === false, so a manually modified guest payload cannot sort by a selected-but-hidden Table column;
label extraction now uses the backend get_column_name() / get_metric_name() helpers, which keeps adhoc SIMPLE metrics without custom labels aligned with actual result keys like SUM(sales);
new guest orderby entries now have to be strict [term, bool] pairs, and newly supplied SQL expression objects / malformed SIMPLE metric dicts fail closed;
saved/frozen owner-defined orderby entries are still allowed as before.

Local targeted validation: .venv/bin/pytest tests/unit_tests/security/manager_test.py -q passes with 89 passed.

Also asking @dpgaspar @mistercrunch to take a look at this PR now that CI is green, since it may loosens guest-user query validation.

sha174n · 2026-07-01T17:15:31Z

Thanks for tackling this! Guest column sorting for embedded dashboards was since merged in #41218, which looks to supersede this (and the branch now conflicts with master), so I think it can be closed. Appreciate the work — the sortable-column handling here was nicely defensive.

bito-code-review · 2026-07-02T03:28:05Z

Code Review Agent Run #029dc3

Actionable Suggestions - 0

Filtered by Review Rules

Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.

superset/connectors/sqla/models.py - 1
- Incomplete None validation for aggregate · Line 1681-1693

Review Details

Files reviewed - 5 · Commit Range: b68d882..b2523e3
- superset/connectors/sqla/models.py
- superset/models/helpers.py
- superset/security/manager.py
- tests/unit_tests/models/helpers_test.py
- tests/unit_tests/security/manager_test.py
Files skipped - 0
Tools
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ 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-code-review · 2026-07-02T09:10:33Z

Code Review Agent Run #bcb008

Actionable Suggestions - 0

Review Details

Files reviewed - 5 · Commit Range: b2523e3..d68eece
- superset/models/helpers.py
- tests/unit_tests/models/helpers_test.py
- superset/security/manager.py
- tests/unit_tests/security/manager_test.py
- superset/connectors/sqla/models.py
Files skipped - 0
Tools
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ 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-code-review · 2026-07-02T09:22:35Z

Code Review Agent Run #bcb008

Actionable Suggestions - 0

Filtered by Review Rules

Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.

tests/unit_tests/security/manager_test.py - 1
- Incorrect comment in test · Line 1618-1618

Review Details

Files reviewed - 5 · Commit Range: b2523e3..d68eece
- superset/models/helpers.py
- tests/unit_tests/models/helpers_test.py
- superset/security/manager.py
- tests/unit_tests/security/manager_test.py
- superset/connectors/sqla/models.py
Files skipped - 0
Tools
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ 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

sha174n

Following up on my earlier comment: please disregard the "close as superseded" note. You've rebased onto the merged #41218 and reworked this to limit guest-initiated sorting to the chart's visible result columns (excluding those marked column_config: {visible: false}), while still replaying an owner's stored orderby. That's a solid defense-in-depth improvement on top of #41218, and the distinction between replaying a stored sort versus a guest changing its direction is nicely handled. Coverage looks thorough and CI is green. LGTM, and thanks for pushing it forward.

pull-request-size Bot added the size/L label Jan 22, 2026

dosubot Bot added authentication:access-control Rlated to access control change:backend Requires changing the backend labels Jan 22, 2026

codeant-ai-for-open-source Bot reviewed Jan 22, 2026

View reviewed changes

Comment thread superset/security/manager.py Outdated

pull-request-size Bot added size/XL and removed size/L labels Jan 23, 2026

rusackas requested review from Vitor-Avila, dpgaspar and mistercrunch January 23, 2026 19:18

dosubot Bot mentioned this pull request Jan 28, 2026

Unknown column used in orderby for SQL expression columns (server-side pagination) #37520

Closed

3 tasks

Vitor-Avila requested a review from betodealmeida January 28, 2026 19:37

codeant-ai-for-open-source Bot reviewed Jan 28, 2026

View reviewed changes

Comment thread superset/security/manager.py Outdated

Comment thread superset/security/manager.py Outdated

YuriyKrasilnikov added a commit to YuriyKrasilnikov/superset that referenced this pull request Feb 19, 2026

Merge fix/guest-user-sorting (PR apache#37371)

54485db

HJulio mentioned this pull request Jun 19, 2026

[6.0.0] Embedded dashboard (guest): sorting a table triggers “Data error: Guest user cannot modify chart payload” #37061

Closed

3 tasks

YuriyKrasilnikov added 3 commits June 19, 2026 10:08

refactor(security): extract _validate_orderby_list to reduce complexity

df08c1c

Extract helper function to reduce cyclomatic complexity of _orderby_whitelist_compare from 12 to below 10 (ruff C901).

rusackas force-pushed the fix/guest-user-sorting-37061 branch from 81e9859 to 0207a89 Compare June 19, 2026 17:19

rusackas force-pushed the fix/guest-user-sorting-37061 branch from 0207a89 to aef22bf Compare June 19, 2026 17:24

pull-request-size Bot added size/XXL and removed size/XL labels Jun 21, 2026

fix(security): harden guest chart sort controls

9f1aacc

YuriyKrasilnikov force-pushed the fix/guest-user-sorting-37061 branch from fd250de to 9f1aacc Compare July 2, 2026 00:26

codeant-ai-for-open-source Bot reviewed Jul 2, 2026

View reviewed changes

Comment thread superset/connectors/sqla/models.py Outdated

Comment thread superset/connectors/sqla/models.py Outdated

Comment thread superset/models/helpers.py Outdated

Comment thread tests/unit_tests/models/helpers_test.py Outdated

codeant-ai-for-open-source Bot reviewed Jul 2, 2026

View reviewed changes

Comment thread tests/unit_tests/security/manager_test.py

Comment thread tests/unit_tests/security/manager_test.py

YuriyKrasilnikov added 2 commits July 2, 2026 01:05

chore(security): address guest sort review feedback

b2523e3

fix(query): prefer selected adhoc column labels in orderby

eeeacb2

codeant-ai-for-open-source Bot reviewed Jul 2, 2026

View reviewed changes

Comment thread superset/security/manager.py Outdated

Comment thread superset/security/manager.py Outdated

Comment thread superset/security/manager.py Outdated

Comment thread tests/unit_tests/models/helpers_test.py Outdated

Comment thread tests/unit_tests/security/manager_test.py Outdated

codeant-ai-for-open-source Bot reviewed Jul 2, 2026

View reviewed changes

Comment thread superset/connectors/sqla/models.py Outdated

Comment thread superset/connectors/sqla/models.py Outdated

Comment thread tests/unit_tests/security/manager_test.py Outdated

YuriyKrasilnikov and others added 3 commits July 2, 2026 06:44

chore(security): satisfy guest sort lint feedback

9338f55

fix(query): harden adhoc metric validation

c01229f

Merge branch 'master' into fix/guest-user-sorting-37061

d68eece

sha174n approved these changes Jul 2, 2026

View reviewed changes

rusackas merged commit ab0e77c into apache:master Jul 2, 2026
59 checks passed

devin-ai-integration Bot mentioned this pull request Jul 3, 2026

[regression] Adhoc column orderby resolution priority changed — now shadows stored metrics and physical columns (PR #37371) breno-helf/superset#38

Closed

YuriyKrasilnikov deleted the fix/guest-user-sorting-37061 branch July 3, 2026 05:42

aminghadersohi mentioned this pull request Jul 20, 2026

fix(query): resolve search filter against adhoc column labels in Table chart #42216

Closed

6 tasks

luizotavio32 mentioned this pull request Aug 5, 2026

fix(embedded): load guest charts with a BASE_AXIS x-axis (backport #42150 to 6.2) #42766

Closed

9 tasks

Uh oh!

Conversation

YuriyKrasilnikov commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Problem Analysis

Root Cause

Why This Is Wrong

Security Research

Attack Vector Analysis

Industry Standard

Mathematical Model

Architecture Decision

Why Not Just Remove orderby Check?

Why Not Frontend-Only Sorting?

Why Comparator Pattern?

Why TypedDict?

Implementation Details

New Functions

Why _extract_orderby_column_name Returns None for SQL?

Why Check Both form_data AND queries?

Why equiv_field Instead of key?

Changes Summary

New Tests

Backward Compatibility

How To Test

Checklist

Uh oh!

bito-code-review Bot commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #d2ccfe

Uh oh!

Uh oh!

YuriyKrasilnikov commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Response to codeant-ai bot review

The bot's suggestion is incorrect

Data Flow Analysis

However, added defensive barriers anyway

What added

Similar patterns in Superset codebase

Why defensive barriers even if bot was wrong

Tests added (20 new tests)

Uh oh!

netlify Bot commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for superset-docs-preview ready!

Uh oh!

bito-code-review Bot commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #442218

Uh oh!

codecov Bot commented Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

YuriyKrasilnikov commented Jan 28, 2026

Fixed in 81e9859

ruff C901: Function too complex

Uh oh!

Uh oh!

Uh oh!

YuriyKrasilnikov commented Jan 28, 2026

Response to codeant-ai bot suggestions

1. {"column": "string"} format

2. tuple as outer orderby container

Uh oh!

bito-code-review Bot commented Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #5fc97b

Uh oh!

rusackas commented Jun 13, 2026

Uh oh!

rusackas commented Jun 19, 2026

Uh oh!

bito-code-review Bot commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #ca7b5d

Uh oh!

YuriyKrasilnikov commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

YuriyKrasilnikov commented Jan 22, 2026 •

edited

Loading

Why `_extract_orderby_column_name` Returns None for SQL?

Why `equiv_field` Instead of `key`?

bito-code-review Bot commented Jan 22, 2026 •

edited

Loading

YuriyKrasilnikov commented Jan 23, 2026 •

edited

Loading

netlify Bot commented Jan 23, 2026 •

edited

Loading

bito-code-review Bot commented Jan 23, 2026 •

edited

Loading

codecov Bot commented Jan 28, 2026 •

edited

Loading

Fixed in `81e9859`

1. `{"column": "string"}` format

2. `tuple` as outer orderby container

bito-code-review Bot commented Jan 28, 2026 •

edited

Loading

bito-code-review Bot commented Jun 22, 2026 •

edited

Loading

YuriyKrasilnikov commented Jun 22, 2026 •

edited

Loading

bito-code-review Bot commented Jul 2, 2026 •

edited

Loading

bito-code-review Bot commented Jul 2, 2026 •

edited

Loading

bito-code-review Bot commented Jul 2, 2026 •

edited

Loading