Re: [PR] SOLR-17104: Protect lib directories from being written to [solr]
github-actions[bot] commented on PR #2154: URL: https://github.com/apache/solr/pull/2154#issuecomment-4035305116 This PR is now closed due to 60 days of inactivity after being marked as stale. Re-opening this PR is still possible, in which case it will be marked as active again. -- 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]
Re: [PR] SOLR-17104: Protect lib directories from being written to [solr]
github-actions[bot] closed pull request #2154: SOLR-17104: Protect lib directories from being written to URL: https://github.com/apache/solr/pull/2154 -- 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]
Re: [PR] SOLR-17104: Protect lib directories from being written to [solr]
github-actions[bot] commented on PR #2154: URL: https://github.com/apache/solr/pull/2154#issuecomment-3731104372 This PR has had no activity for 60 days and is now labeled as stale. Any new activity will remove the stale label. To attract more reviewers, please tag people who might be familiar with the code area and/or notify the [email protected] mailing list. To exempt this PR from being marked as stale, make it a draft PR or add the label "exempt-stale". If left unattended, this PR will be closed after another 60 days of inactivity. Thank you for your contribution! -- 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]
Re: [PR] SOLR-17104: Protect lib directories from being written to [solr]
github-actions[bot] closed pull request #2154: SOLR-17104: Protect lib directories from being written to URL: https://github.com/apache/solr/pull/2154 -- 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]
Re: [PR] SOLR-17104: Protect lib directories from being written to [solr]
github-actions[bot] commented on PR #2154: URL: https://github.com/apache/solr/pull/2154#issuecomment-3383616395 This PR is now closed due to 60 days of inactivity after being marked as stale. Re-opening this PR is still possible, in which case it will be marked as active again. -- 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]
Re: [PR] SOLR-17104: Protect lib directories from being written to [solr]
github-actions[bot] commented on PR #2154: URL: https://github.com/apache/solr/pull/2154#issuecomment-3172247001 This PR has had no activity for 60 days and is now labeled as stale. Any new activity will remove the stale label. To attract more reviewers, please tag people who might be familiar with the code area and/or notify the [email protected] mailing list. To exempt this PR from being marked as stale, make it a draft PR or add the label "exempt-stale". If left unattended, this PR will be closed after another 60 days of inactivity. Thank you for your contribution! -- 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]
Re: [PR] SOLR-17104: Protect lib directories from being written to [solr]
github-actions[bot] commented on PR #2154: URL: https://github.com/apache/solr/pull/2154#issuecomment-1967935982 This PR had no visible activity in the past 60 days, labeling it as stale. Any new activity will remove the stale label. To attract more reviewers, please tag someone or notify the [email protected] mailing list. Thank you for your contribution! -- 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]
Re: [PR] SOLR-17104: Protect lib directories from being written to [solr]
gerlowskija commented on code in PR #2154:
URL: https://github.com/apache/solr/pull/2154#discussion_r1427948954
##
solr/core/src/java/org/apache/solr/handler/SnapShooter.java:
##
@@ -104,6 +104,7 @@ private void initialize(
this.snapshotName = snapshotName;
if ("file".equals(location.getScheme())) {
solrCore.getCoreContainer().assertPathAllowed(Paths.get(location));
+ solrCore.getCoreContainer().assertPathNotProtected(Paths.get(location));
Review Comment:
[0] I worry a bit that folks might forget one or the other of these calls.
Maybe it'd be worth refactoring into a single utility method that optionally
invokes the not-protected logic in the case of writes. e.g.
```
public enum AccessType { READ, WRITE, UNKNOWN; }
public void assertPathAllowed(Path toCheck, AccessType access) {
...
if (access == AccessType.WRITE) {
assertPathNotProtected(...);
}
}
// Note the 'private' here.
private void assertPathNotProtected(Path toCheck) {
}
```
##
solr/core/src/java/org/apache/solr/core/NodeConfig.java:
##
@@ -455,7 +460,32 @@ private void setupSharedLib() {
libDirs.addAll(sharedLibs);
}
-addFoldersToSharedLib(libDirs);
+List libDirPaths = new ArrayList<>(libDirs.size());
+for (String libDir : libDirs) {
+ libDirPaths.add(getSolrHome().resolve(libDir).toAbsolutePath());
+}
+return libDirPaths;
+ }
+
+ // Get the list of protected directories, that cannot have files created
within them
+ public List getProtectedDirectories() {
+List protectedDirectories = new ArrayList<>(sharedLibPaths.size() +
4);
+
+// Start with the shared lib directories
+protectedDirectories.addAll(sharedLibPaths);
+
+if (getSolrInstallDir() != null) {
+ // Add in modules and tools
+
protectedDirectories.add(ModuleUtils.getModulesPath(getSolrInstallDir()).toAbsolutePath());
+
protectedDirectories.add(getSolrInstallDir().resolve("prometheus-exporter").toAbsolutePath());
+
+ // Add in server library folders
+ protectedDirectories.add(
Review Comment:
[0] I'm a little surprised we don't have static constants for these
directories somewhere. Maybe we should...
--
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]
Re: [PR] SOLR-17104: Protect lib directories from being written to [solr]
dsmiley commented on code in PR #2154:
URL: https://github.com/apache/solr/pull/2154#discussion_r1427316295
##
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##
@@ -1645,6 +1650,34 @@ public Set getAllowPaths() {
return allowPaths;
}
+ /**
+ * Checks that the given path is not protected (i.e. can accept writes).
Protected paths generally
+ * contain library directories that Solr loads jars and classes from.
+ *
+ * @param pathToAssert path to check
+ * @throws SolrException if path is contained inside a library folder
+ */
+ public void assertPathNotProtected(Path pathToAssert) throws SolrException {
Review Comment:
Please move this to SolrPaths. CoreContainer has a ton of responsibilities;
it doesn't need yet another one.
##
solr/core/src/java/org/apache/solr/core/NodeConfig.java:
##
@@ -455,7 +460,32 @@ private void setupSharedLib() {
libDirs.addAll(sharedLibs);
}
-addFoldersToSharedLib(libDirs);
+List libDirPaths = new ArrayList<>(libDirs.size());
+for (String libDir : libDirs) {
+ libDirPaths.add(getSolrHome().resolve(libDir).toAbsolutePath());
+}
+return libDirPaths;
+ }
+
+ // Get the list of protected directories, that cannot have files created
within them
+ public List getProtectedDirectories() {
+List protectedDirectories = new ArrayList<>(sharedLibPaths.size() +
4);
+
+// Start with the shared lib directories
+protectedDirectories.addAll(sharedLibPaths);
+
+if (getSolrInstallDir() != null) {
+ // Add in modules and tools
+
protectedDirectories.add(ModuleUtils.getModulesPath(getSolrInstallDir()).toAbsolutePath());
+
protectedDirectories.add(getSolrInstallDir().resolve("prometheus-exporter").toAbsolutePath());
+
+ // Add in server library folders
+ protectedDirectories.add(
+
getSolrInstallDir().resolve("server").resolve("lib").toAbsolutePath());
+ protectedDirectories.add(
+
getSolrInstallDir().resolve("server").resolve("solr-webapp").toAbsolutePath());
+}
Review Comment:
maybe assert that these actually exist? If one day we change a name or move
something, the assert will help us keep this up to date.
##
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##
@@ -1645,6 +1650,34 @@ public Set getAllowPaths() {
return allowPaths;
}
+ /**
+ * Checks that the given path is not protected (i.e. can accept writes).
Protected paths generally
+ * contain library directories that Solr loads jars and classes from.
+ *
+ * @param pathToAssert path to check
+ * @throws SolrException if path is contained inside a library folder
+ */
+ public void assertPathNotProtected(Path pathToAssert) throws SolrException {
+pathToAssert = pathToAssert.toAbsolutePath();
+List protectedPathWithCores =
+new ArrayList<>(protectedPaths.size() + solrCores.getNumAllCores());
+protectedPathWithCores.addAll(protectedPaths);
+solrCores.getCoreDescriptors().stream()
+.map(CoreDescriptor::getInstanceDir)
+.map(p -> p.resolve("lib").toAbsolutePath())
+.forEach(protectedPathWithCores::add);
+
+for (Path path : protectedPathWithCores) {
Review Comment:
Why the separation here with intermediate "protectedPathWithCores list; why
not end with forEach and do the check?
--
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]
Re: [PR] SOLR-17104: Protect lib directories from being written to [solr]
HoustonPutman commented on PR #2154: URL: https://github.com/apache/solr/pull/2154#issuecomment-1856382634 Do you mean the restore code or the lib directories? For the restore code, there might be more places we are writing user provided files, but I can't think of it. As for the lib directories, this is all of them off of my head, but I could be missing a few. -- 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]
Re: [PR] SOLR-17104: Protect lib directories from being written to [solr]
epugh commented on PR #2154: URL: https://github.com/apache/solr/pull/2154#issuecomment-1856373651 Is this the only place where this is needed? Or are there lots of directories similar? -- 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]
