From 3c9d91b4b73ba6e8b7e162c2391647355cb737f6 Mon Sep 17 00:00:00 2001 From: Matteo Mazzola Date: Fri, 31 Oct 2025 17:58:55 +0000 Subject: [PATCH] Error if installed plugin is inside plugins folder (#137398) * Error if installed plugin is inside plugins folder If a non directory file is in the plugins folder during installation, there will be a non descriptive error during findBundle, as it assumes everything is a directory. This new preemptive check adds a descripive error message to avoid this scenario. Also, for consistency and to not use a deprecated URL constructor, in the download method we now use URI. Closes #27401 (cherry picked from commit 941f3022a1325a6ca3b813d878a53574ab073300) --- .../plugins/cli/InstallPluginAction.java | 29 +++++++++++++++++-- .../plugins/cli/InstallPluginActionTests.java | 28 ++++++++++++++---- docs/changelog/137398.yaml | 6 ++++ 3 files changed, 54 insertions(+), 9 deletions(-) create mode 100644 docs/changelog/137398.yaml diff --git a/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/cli/InstallPluginAction.java b/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/cli/InstallPluginAction.java index 2798b3353259b..0aaa19846f6d0 100644 --- a/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/cli/InstallPluginAction.java +++ b/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/cli/InstallPluginAction.java @@ -63,6 +63,7 @@ import java.nio.file.FileVisitResult; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; import java.nio.file.SimpleFileVisitor; import java.nio.file.StandardCopyOption; import java.nio.file.attribute.BasicFileAttributes; @@ -329,10 +330,31 @@ private Path download(InstallablePlugin plugin, Path tmpDir) throws Exception { } throw new UserException(ExitCodes.USAGE, msg); } + + verifyLocationNotInPluginsDirectory(pluginLocation); + terminal.println(logPrefix + "Downloading " + URLDecoder.decode(pluginLocation, StandardCharsets.UTF_8)); return downloadZip(pluginLocation, tmpDir); } + @SuppressForbidden(reason = "Need to use Paths#get") + private void verifyLocationNotInPluginsDirectory(String pluginLocation) throws URISyntaxException, IOException, UserException { + if (pluginLocation == null) { + return; + } + URI uri = new URI(pluginLocation); + if ("file".equalsIgnoreCase(uri.getScheme())) { + Path pluginRealPath = Paths.get(uri).toRealPath(); + Path pluginsDirectory = env.pluginsDir().toRealPath(); + if (pluginRealPath.startsWith(pluginsDirectory)) { + throw new UserException( + ExitCodes.USAGE, + "Installation of plugin in location [" + pluginLocation + "] from inside the plugins directory is not permitted." + ); + } + } + } + @SuppressForbidden(reason = "Need to use PathUtils#get") private Path getPluginArchivePath(String pluginId, String pluginArchiveDir) throws UserException { final Path path = PathUtils.get(pluginArchiveDir); @@ -462,9 +484,9 @@ private static List checkMisspelledPlugin(String pluginId) { /** Downloads a zip from the url, into a temp file under the given temp dir. */ // pkg private for tests @SuppressForbidden(reason = "We use getInputStream to download plugins") - Path downloadZip(String urlString, Path tmpDir) throws IOException { + Path downloadZip(String urlString, Path tmpDir) throws IOException, URISyntaxException { terminal.println(VERBOSE, "Retrieving zip from " + urlString); - URL url = new URL(urlString); + URL url = new URI(urlString).toURL(); Path zip = Files.createTempFile(tmpDir, null, ".zip"); URLConnection urlConnection = this.proxy == null ? url.openConnection() : url.openConnection(this.proxy); urlConnection.addRequestProperty("User-Agent", "elasticsearch-plugin-installer"); @@ -548,9 +570,10 @@ private InputStream urlOpenStream(final URL url) throws IOException { * @throws IOException if an I/O exception occurs download or reading files and resources * @throws PGPException if an exception occurs verifying the downloaded ZIP signature * @throws UserException if checksum validation fails + * @throws URISyntaxException is the url is invalid */ private Path downloadAndValidate(final String urlString, final Path tmpDir, final boolean officialPlugin) throws IOException, - PGPException, UserException { + PGPException, UserException, URISyntaxException { Path zip = downloadZip(urlString, tmpDir); pathsToDeleteOnShutdown.add(zip); String checksumUrlString = urlString + ".sha512"; diff --git a/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/cli/InstallPluginActionTests.java b/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/cli/InstallPluginActionTests.java index a4e503a894ab4..ead08787269c3 100644 --- a/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/cli/InstallPluginActionTests.java +++ b/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/cli/InstallPluginActionTests.java @@ -66,18 +66,18 @@ import java.io.BufferedReader; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; -import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.io.StringReader; -import java.net.MalformedURLException; import java.net.URI; +import java.net.URISyntaxException; import java.net.URL; import java.nio.charset.StandardCharsets; import java.nio.file.DirectoryStream; import java.nio.file.FileAlreadyExistsException; import java.nio.file.FileSystem; import java.nio.file.Files; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.nio.file.attribute.GroupPrincipal; @@ -552,8 +552,8 @@ public void testTransaction() throws Exception { pluginZip.getId() + "-does-not-exist", pluginZip.getLocation() + "-does-not-exist" ); - final FileNotFoundException e = expectThrows( - FileNotFoundException.class, + final NoSuchFileException e = expectThrows( + NoSuchFileException.class, () -> installPlugins(List.of(pluginZip, nonexistentPluginZip), env.v1()) ); assertThat(e.getMessage(), containsString("does-not-exist")); @@ -586,11 +586,27 @@ public void testSpaceInUrl() throws Exception { assertPlugin("fake", pluginDir, env.v2()); } + public void testCannotInstallFromInsidePluginsDirectory() throws Exception { + InstallablePlugin pluginZip = createPluginZip("fake", pluginDir); + Path pluginZipInsidePlugins = env.v2().pluginsDir().resolve("fake.zip"); + try (InputStream in = FileSystemUtils.openFileURLStream(new URL(pluginZip.getLocation()))) { + Files.copy(in, pluginZipInsidePlugins, StandardCopyOption.REPLACE_EXISTING); + } + String location = pluginZipInsidePlugins.toUri().toURL().toString(); + assumeTrue("requires file URL scheme", location.startsWith("file:")); + InstallablePlugin modifiedPlugin = new InstallablePlugin("fake", location); + UserException e = expectThrows(UserException.class, () -> installPlugin(modifiedPlugin)); + assertThat( + e.getMessage(), + startsWith("Installation of plugin in location [" + location + "] from inside the plugins directory is not permitted.") + ); + } + public void testMalformedUrlNotMaven() { // has two colons, so it appears similar to maven coordinates InstallablePlugin plugin = new InstallablePlugin("fake", "://host:1234"); - MalformedURLException e = expectThrows(MalformedURLException.class, () -> installPlugin(plugin)); - assertThat(e.getMessage(), containsString("no protocol")); + URISyntaxException e = expectThrows(URISyntaxException.class, () -> installPlugin(plugin)); + assertThat(e.getMessage(), containsString("Expected scheme name")); } public void testFileNotMaven() { diff --git a/docs/changelog/137398.yaml b/docs/changelog/137398.yaml new file mode 100644 index 0000000000000..4d4a25257fbd1 --- /dev/null +++ b/docs/changelog/137398.yaml @@ -0,0 +1,6 @@ +pr: 137398 +summary: Error if installed plugin is inside plugins folder +area: Infra/Plugins +type: enhancement +issues: + - 27401