You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "mikebridge (via GitHub)" <gi...@apache.org> on 2026/09/18 21:28:40 UTC
[PR] fix: avoid provider calls when rendering datasource access denials [superset]
mikebridge opened a new pull request, #44432:
URL: https://github.com/apache/superset/pull/44432
### SUMMARY
Follow-up to #44416 and [SC-121388](https://app.shortcut.com/preset/story/121388), addressing [Richard's immediate follow-up request in his September 18, 18:21 UTC review](https://github.com/apache/superset/pull/44416#pullrequestreview-5251162269). Denied semantic-view chart saves and Explore form_data requests loaded provider metadata just to construct an access-denied payload. When that provider failed, the denial became a server error. Read the persisted datasource id directly in both denial builders so the request returns 403 without contacting the provider. Table ids and denial disclosure protections remain unchanged.
The new Gamma tests use actual permissions, including chart editorship for PUT, and a provider mocked to raise; they verify 403, zero provider calls and no saved changes.
Deliberate scope deferral: chart save still accepts `semantic_view` when `SEMANTIC_LAYERS` is off, while semantic discovery returns 404. A consistent flag-off policy across save, Explore and existing-chart use is deferred; this change fixes denial rendering only. Query-context datasource identity is checked on create and query-context-only update; the full PUT path does not perform that binding check. This follow-up does not expand that contract.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Backend only. Before: all three tested denial paths returned 500 after provider metadata failure. After: 403 with no provider access.
### TESTING INSTRUCTIONS
- 113 tests passed across semantic chart-save and complete chart API suites on isolated SQLite.
- 262 security manager/permission-link, chart-command and Explore unit tests passed.
- Red-first Gamma chart POST/PUT and form_data POST controls failed before the fix.
- Required branch-scoped hooks passed, including MyPy, pylint and Ruff. Independent review approved the exact staged tree with zero findings.
### ADDITIONAL INFORMATION
- [x] Has associated issue: SC-121388; follows #44416 and Richard's review.
- [ ] Required feature flags: denial rendering is independent of feature-flag state.
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] 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
Codex authored; independent security-path review approved with zero findings. No live-provider/browser validation claimed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: avoid provider calls when rendering datasource access denials [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi merged PR #44432:
URL: https://github.com/apache/superset/pull/44432
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: avoid provider calls when rendering datasource access denials [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44432:
URL: https://github.com/apache/superset/pull/44432#issuecomment-5736403496
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: avoid provider calls when rendering datasource access denials [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #44432:
URL: https://github.com/apache/superset/pull/44432#issuecomment-5736604316
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/44432?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 66.02%. Comparing base ([`062bf94`](https://app.codecov.io/gh/apache/superset/commit/062bf948e59849397fe12ea9b03d255c1002eda3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`0eb74a1`](https://app.codecov.io/gh/apache/superset/commit/0eb74a14905b1987289df59cf781281fd71cd914?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
> :exclamation: There is a different number of reports uploaded between BASE (062bf94) and HEAD (0eb74a1). Click for more details.
>
> <details><summary>HEAD has 5 uploads less than BASE</summary>
>
>| Flag | BASE (062bf94) | HEAD (0eb74a1) |
>|------|------|------|
>|python|8|3|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #44432 +/- ##
===========================================
- Coverage 80.62% 66.02% -14.60%
===========================================
Files 2942 2942
Lines 175617 175617
Branches 40755 40755
===========================================
- Hits 141584 115948 -25636
- Misses 31371 57205 +25834
+ Partials 2662 2464 -198
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/44432/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/44432/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `37.16% <ø> (ø)` | |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/44432/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/44432/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/44432/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.04% <ø> (ø)` | |
| [python](https://app.codecov.io/gh/apache/superset/pull/44432/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.25% <ø> (-28.70%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/44432/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.12% <ø> (+<0.01%)` | :arrow_up: |
| [unit](https://app.codecov.io/gh/apache/superset/pull/44432/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/44432?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org