sadpandajoe commented on code in PR #45342:
URL: https://github.com/apache/superset/pull/45342#discussion_r4283860391
##########
superset/commands/dataset/update.py:
##########
@@ -283,6 +283,7 @@ def _validate_sql_access(
sql=sql,
catalog=catalog,
schema=schema,
+ allow_dataset_editorship=False,
Review Comment:
The decoy can still be turned into a read of the real table through the
sibling check a few lines up. `_validate_table_access` (line 263) calls
`raise_for_access(database=db, table=table)` with the default
`allow_dataset_editorship=True`. A user with dataset write but no grants on
`public.customer_pii` creates a `SELECT 1` virtual dataset named `customer_pii`
in schema `public`, then sends `PUT /api/v1/dataset/<id>` with `{"sql": null}`.
The physical-conversion branch (around line 186) runs that check,
`query_datasources_by_name` returns the caller's own decoy, and the editorship
accept lets it through. The dataset is now physical, and chart-data or samples
authorize it via `is_editor` and return the real table's rows.
The same gap exists wherever user SQL or a table is validated without the
new flag: the legacy `/datasource/save/` path
(`views/datasource/views.py:210-220`) and dataset import
(`commands/dataset/importers/v1/utils.py:591`). Should the opt-out cover
`_validate_table_access` and those call sites too, so the new keyword isn't
only applied to the three paths in this diff?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
