dsmiley commented on code in PR #4193:
URL: https://github.com/apache/solr/pull/4193#discussion_r2955050434
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkMaintenanceUtils.java:
##########
@@ -443,13 +433,9 @@ public static void downloadFromZK(SolrZkClient zkClient,
String zkPath, Path fil
if (children.size() == 0) {
// If we didn't copy data down, then we also didn't create the file.
But we still need a
// marker on the local disk so create an empty file.
- if (isFileForbiddenInConfigSets(zkPath)) {
Review Comment:
> this also generally just prevents us from having RCE issues with injecting
code in ZK
Huh? I'd ask, how did that code get into ZK in the first place? We should
prevent ConfigSetService (which is API endpoint connected) from doing so. But
otherwise... it's by-design / intentional / operator-driven (user knows what
they are doing). It's not definitively wrong. You could put fun stuff in an
XSLT say. We have to trust **admins** (not users generally) to populate
configSets with whatever they want, wherever that configset may exist (e.g. ZK).
Without repeating myself I can't add much more than I didn't already say. I
guess we will disagree, but you approved the PR. An additional argument I
didn't say, is that by having this code here, we pretend we are somehow
protecting ourselves, and it can make code kind of un-touchable for fear you
are breaking Solr security. Same thing with SolrPaths... or AllowPaths etc.
which can become kind of toxic/no-go land or else one is potentially painted as
someone who doesn't care about security.
--
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]
