mistercrunch commented on PR #33084: URL: https://github.com/apache/superset/pull/33084#issuecomment-2797897614
Guessing `master` was broken at the time of branch creation, please rebase. About the feature, I'm not sure whether/how we should be in the business of disabling SQL functionality for security purposes. I think where I'd draw the line is in areas where a SQL feature can compromise the integrity of the OS and Superset instance itself. I don't see any issue with `current_schema` for instance, it's a useful feature AFAIC, not a security concern - unless I'm missing somehting. Also noting that much of these database features can be disabled at the database user account level. Seems good DBA hygiene is key here, not BI tool stewardship. Now. Here we're modifying a local config setting, so it's designed to be managed/altered at the environment level. You can do this on your envrionment today. It can certainly be good to prevent certain things that we know are bad. "secure by default" is great, though I wouldn't want to provide the impression that it's Superset's responsibility to secure your database or give the impression of a false sense of safety from it. It's not scalable for us (and I imagine for other BI tools) to be on top of all the potential security issues different database engines may expose. In any case, whatever we decide here, those are breaking changes and would need an entry in UPDATING.md -- 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]
