Skip to content

refactor(chart): improve dashboard validation consistency in chart operations#40336

Draft
sha174n wants to merge 8 commits into
apache:masterfrom
sha174n:fix/chart-dashboard-ownership-validation
Draft

refactor(chart): improve dashboard validation consistency in chart operations#40336
sha174n wants to merge 8 commits into
apache:masterfrom
sha174n:fix/chart-dashboard-ownership-validation

Conversation

@sha174n
Copy link
Copy Markdown
Contributor

@sha174n sha174n commented May 21, 2026

Standardizes dashboard relationship validation logic between chart creation and update operations. Consolidates permission checking patterns for improved code consistency.

This PR ensures that both chart creation and update operations apply equivalent validation logic when establishing dashboard relationships.

sha174n and others added 5 commits May 21, 2026 14:00
Add clarifications for common vulnerability report categories that fall
outside our security scope to help manage high-volume reporting and
align with other Apache projects:

- User enumeration through API responses or timing differences
- Low-impact information disclosure (versions, generic errors)
- Resource exhaustion requiring authentication
- Missing security headers without demonstrable exploit

This reduces triage overhead while focusing on genuine architectural risks.
Define vulnerabilities as issues with meaningful impact beyond intended
security model, allowing low-impact technical edge cases to be classified
as hardening rather than vulnerabilities. This provides clearer triage
criteria for boundary variations and access control edge cases.
Add missing ownership validation in UpdateChartCommand._validate_new_dashboard_access()
to match CreateChartCommand behavior. Previously only checked visibility via access
filter, now requires ownership to prevent chart injection into non-owned dashboards.
The previous fix appended DashboardsForbiddenError (a ForbiddenError, not
a ValidationError) to the exceptions list, causing AttributeError in
ChartInvalidError.normalized_messages() → HTTP 500 instead of 403.

Fix: raise DashboardsForbiddenError directly (matching CreateChartCommand)
and add a DashboardsForbiddenError except clause to the put() handler,
consistent with the existing post() handler.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented May 21, 2026

Codecov Report

❌ Patch coverage is 40.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.18%. Comparing base (558ff44) to head (220afb9).

Files with missing lines Patch % Lines
superset/commands/chart/update.py 33.33% 1 Missing and 1 partial ⚠️
superset/charts/api.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #40336      +/-   ##
==========================================
- Coverage   64.18%   64.18%   -0.01%     
==========================================
  Files        2592     2592              
  Lines      139036   139041       +5     
  Branches    32282    32284       +2     
==========================================
+ Hits        89244    89246       +2     
- Misses      48260    48262       +2     
- Partials     1532     1533       +1     
Flag Coverage Δ
hive 39.28% <0.00%> (-0.01%) ⬇️
mysql 58.79% <40.00%> (-0.01%) ⬇️
postgres 58.87% <40.00%> (-0.01%) ⬇️
presto 40.95% <0.00%> (-0.01%) ⬇️
python 60.42% <40.00%> (-0.01%) ⬇️
sqlite 58.51% <40.00%> (-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 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.

@github-actions github-actions Bot added the api Related to the REST API label May 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api Related to the REST API size/S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant