This is an automated email from the ASF dual-hosted git repository.
jongyoul pushed a commit to branch branch-0.12
in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/branch-0.12 by this push:
new c5353cbc33 [HOTFIX] Validate interpreter library lookup
c5353cbc33 is described below
commit c5353cbc33b17eb340f97ab064de73c487ad36bd
Author: Jongyoul Lee <[email protected]>
AuthorDate: Tue Aug 18 17:55:22 2026 +0900
[HOTFIX] Validate interpreter library lookup
This PR makes interpreter library lookup more robust and predictable.
The previous lookup used the interpreter setting name directly as the
on-disk directory, while dependency repositories are stored by setting ID. The
updated lookup resolves the registered interpreter setting first, uses its ID
for the repository, and returns only direct, regular JAR entries expected by
the library synchronization flow. Missing and unsupported entries are ignored.
This also preserves compatibility when an interpreter setting name differs
from its setting ID.
Hot Fix
* [x] Validate interpreter and library lookup inputs
* [x] Resolve repositories through registered interpreter settings
* [x] Preserve setting-name-to-setting-ID compatibility
* [x] Add unit tests for accepted and ignored entries
N/A
```bash
./mvnw -pl zeppelin-server \
-Dtest=RemoteInterpreterEventServerTest,RemoteInterpreterEventServerLibraryTest
test
./mvnw -pl zeppelin-server -DskipTests \
org.apache.rat:apache-rat-plugin:check -Prat
```
The unit tests cover registered JAR retrieval and metadata,
setting-name-to-setting-ID resolution, unsupported lookup inputs, unregistered
settings, non-JAR files, directories, and symbolic links.
N/A
* Does the license files need to update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No
Closes #5423 from jongyoul/codex/security-getlibrary-path.
Signed-off-by: Jongyoul Lee <[email protected]>
(cherry picked from commit 8adeb6a402e9aa7ad228afee5324fdc7eee1de96)
Signed-off-by: Jongyoul Lee <[email protected]>
---
.../interpreter/RemoteInterpreterEventServer.java | 138 ++++++++++++-----
.../RemoteInterpreterEventServerLibraryTest.java | 170 +++++++++++++++++++++
2 files changed, 271 insertions(+), 37 deletions(-)
diff --git
a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/RemoteInterpreterEventServer.java
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/RemoteInterpreterEventServer.java
index bab3ee7b2a..cc72cc15f8 100644
---
a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/RemoteInterpreterEventServer.java
+++
b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/RemoteInterpreterEventServer.java
@@ -19,10 +19,7 @@ package org.apache.zeppelin.interpreter;
import com.google.gson.Gson;
import com.google.gson.reflect.TypeToken;
-
-import java.util.OptionalInt;
import org.apache.commons.io.FileUtils;
-import org.apache.commons.lang3.StringUtils;
import org.apache.thrift.TException;
import org.apache.thrift.server.TThreadPoolServer;
import org.apache.thrift.transport.TServerSocket;
@@ -59,15 +56,19 @@ import org.apache.zeppelin.user.AuthenticationInfo;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import java.io.File;
import java.io.IOException;
import java.nio.ByteBuffer;
+import java.nio.file.DirectoryStream;
+import java.nio.file.Files;
+import java.nio.file.InvalidPathException;
+import java.nio.file.LinkOption;
+import java.nio.file.Path;
import java.util.ArrayList;
-import java.util.Collection;
import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
@@ -583,31 +584,33 @@ public class RemoteInterpreterEventServer implements
RemoteInterpreterEventServi
@Override
public List<LibraryMetadata> getAllLibraryMetadatas(String interpreter)
throws TException {
- if (StringUtils.isBlank(interpreter)) {
- LOGGER.warn("Interpreter is blank");
- return Collections.emptyList();
- }
- File interpreterLocalRepo = new File(
-
zConf.getAbsoluteDir(ZeppelinConfiguration.ConfVars.ZEPPELIN_DEP_LOCALREPO)
- + File.separator
- + interpreter);
- if (!interpreterLocalRepo.exists()) {
- LOGGER.warn("Local interpreter repository {} for interpreter {} doesn't
exists", interpreterLocalRepo,
- interpreter);
- return Collections.emptyList();
- }
- if (!interpreterLocalRepo.isDirectory()) {
- LOGGER.warn("Local interpreter repository {} is no folder",
interpreterLocalRepo);
+ Optional<Path> interpreterLocalRepo =
resolveInterpreterRepository(interpreter);
+ if (interpreterLocalRepo.isEmpty()) {
+ LOGGER.warn("Unable to resolve local repository for requested
interpreter");
return Collections.emptyList();
}
- Collection<File> files = FileUtils.listFiles(interpreterLocalRepo, new
String[] { "jar" }, false);
- List<LibraryMetadata> metaDatas = new ArrayList<>(files.size());
- for (File file : files) {
- try {
- metaDatas.add(new LibraryMetadata(file.getName(),
FileUtils.checksumCRC32(file)));
- } catch (IOException e) {
- LOGGER.warn(e.getMessage(), e);
+
+ Path repository = interpreterLocalRepo.get();
+ List<LibraryMetadata> metaDatas = new ArrayList<>();
+ try (DirectoryStream<Path> libraries =
+ Files.newDirectoryStream(repository, "*.jar")) {
+ for (Path entry : libraries) {
+ Optional<Path> library = resolveLibrary(repository,
entry.getFileName().toString());
+ if (library.isEmpty()) {
+ continue;
+ }
+
+ Path libraryPath = library.get();
+ try {
+ metaDatas.add(new LibraryMetadata(
+ libraryPath.getFileName().toString(),
+ FileUtils.checksumCRC32(libraryPath.toFile())));
+ } catch (IOException e) {
+ LOGGER.warn("Unable to calculate interpreter library checksum", e);
+ }
}
+ } catch (IOException e) {
+ LOGGER.warn("Unable to list libraries for requested interpreter", e);
}
return metaDatas;
}
@@ -615,23 +618,84 @@ public class RemoteInterpreterEventServer implements
RemoteInterpreterEventServi
@Override
public ByteBuffer getLibrary(String interpreter, String libraryName) throws
TException {
- if (StringUtils.isAnyBlank(interpreter, libraryName)) {
- LOGGER.warn("Interpreter \"{}\" or libraryName \"{}\" is blank",
interpreter, libraryName);
- return null;
- }
- File library = new
File(zConf.getAbsoluteDir(ZeppelinConfiguration.ConfVars.ZEPPELIN_DEP_LOCALREPO)
- + File.separator + interpreter + File.separator + libraryName);
- if (!library.exists()) {
- LOGGER.warn("Library {} doesn't exists", library);
+ Optional<Path> library = resolveInterpreterRepository(interpreter)
+ .flatMap(repository -> resolveLibrary(repository, libraryName));
+ if (library.isEmpty()) {
+ LOGGER.warn("Unable to resolve requested interpreter library");
return null;
}
try {
- return ByteBuffer.wrap(FileUtils.readFileToByteArray(library));
+ return ByteBuffer.wrap(Files.readAllBytes(library.get()));
} catch (IOException e) {
- LOGGER.error("Unable to read library {}", library, e);
+ LOGGER.error("Unable to read requested interpreter library", e);
}
return null;
}
+ private Optional<Path> resolveInterpreterRepository(String interpreter) {
+ if (interpreter == null || interpreter.isBlank()) {
+ return Optional.empty();
+ }
+
+ try {
+ InterpreterSetting interpreterSetting =
+ interpreterSettingManager.getInterpreterSettingByName(interpreter);
+ if (interpreterSetting == null || interpreterSetting.getId() == null) {
+ return Optional.empty();
+ }
+
+ Path repositoryName = Path.of(interpreterSetting.getId());
+ if (repositoryName.isAbsolute() || repositoryName.getNameCount() != 1) {
+ return Optional.empty();
+ }
+
+ Path repositoryRoot = Path.of(
+
zConf.getAbsoluteDir(ZeppelinConfiguration.ConfVars.ZEPPELIN_DEP_LOCALREPO))
+ .toRealPath();
+ Path interpreterRepository =
repositoryRoot.resolve(repositoryName).normalize();
+ if (!repositoryRoot.equals(interpreterRepository.getParent())
+ || !Files.isDirectory(interpreterRepository,
LinkOption.NOFOLLOW_LINKS)) {
+ return Optional.empty();
+ }
+
+ Path realInterpreterRepository = interpreterRepository.toRealPath();
+ if (!repositoryRoot.equals(realInterpreterRepository.getParent())) {
+ return Optional.empty();
+ }
+ return Optional.of(realInterpreterRepository);
+ } catch (IOException | InvalidPathException e) {
+ LOGGER.debug("Unable to resolve interpreter repository", e);
+ return Optional.empty();
+ }
+ }
+
+ private Optional<Path> resolveLibrary(Path interpreterRepository, String
libraryName) {
+ if (libraryName == null || !libraryName.endsWith(".jar")) {
+ return Optional.empty();
+ }
+
+ try {
+ Path libraryPath = Path.of(libraryName);
+ if (libraryPath.isAbsolute() || libraryPath.getNameCount() != 1) {
+ return Optional.empty();
+ }
+
+ Path library = interpreterRepository.resolve(libraryPath).normalize();
+ if (!interpreterRepository.equals(library.getParent())
+ || !Files.isRegularFile(library, LinkOption.NOFOLLOW_LINKS)) {
+ return Optional.empty();
+ }
+
+ Path realLibrary = library.toRealPath();
+ if (!interpreterRepository.equals(realLibrary.getParent())) {
+ return Optional.empty();
+ }
+ return Optional.of(realLibrary);
+ } catch (IOException | InvalidPathException e) {
+ LOGGER.debug("Unable to resolve interpreter library", e);
+ return Optional.empty();
+ }
+ }
+
}
diff --git
a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/RemoteInterpreterEventServerLibraryTest.java
b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/RemoteInterpreterEventServerLibraryTest.java
new file mode 100644
index 0000000000..80cb7c9f68
--- /dev/null
+++
b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/RemoteInterpreterEventServerLibraryTest.java
@@ -0,0 +1,170 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zeppelin.interpreter;
+
+import org.apache.zeppelin.conf.ZeppelinConfiguration;
+import org.apache.zeppelin.conf.ZeppelinConfiguration.ConfVars;
+import org.apache.zeppelin.interpreter.thrift.LibraryMetadata;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.List;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+class RemoteInterpreterEventServerLibraryTest {
+
+ private static final String INTERPRETER = "spark";
+
+ @TempDir
+ Path temporaryDirectory;
+
+ private Path dependencyRepository;
+ private InterpreterSettingManager interpreterSettingManager;
+ private RemoteInterpreterEventServer server;
+
+ @BeforeEach
+ void setUp() throws IOException {
+ dependencyRepository =
Files.createDirectory(temporaryDirectory.resolve("dependencies"));
+ Files.createDirectory(dependencyRepository.resolve(INTERPRETER));
+
+ ZeppelinConfiguration zConf = ZeppelinConfiguration.load();
+ zConf.setProperty(
+ ConfVars.ZEPPELIN_DEP_LOCALREPO.getVarName(),
dependencyRepository.toString());
+
+ interpreterSettingManager = mock(InterpreterSettingManager.class);
+ InterpreterSetting interpreterSetting = mock(InterpreterSetting.class);
+ when(interpreterSetting.getId()).thenReturn(INTERPRETER);
+ when(interpreterSettingManager.getInterpreterSettingByName(INTERPRETER))
+ .thenReturn(interpreterSetting);
+ server = new RemoteInterpreterEventServer(zConf,
interpreterSettingManager);
+ }
+
+ @Test
+ void readsRegisteredInterpreterJarAndListsItsMetadata() throws Exception {
+ byte[] expected = {1, 2, 3, 4};
+
Files.write(dependencyRepository.resolve(INTERPRETER).resolve("library.jar"),
expected);
+
+ ByteBuffer library = server.getLibrary(INTERPRETER, "library.jar");
+ assertArrayEquals(expected, library.array());
+
+ List<LibraryMetadata> metadata =
server.getAllLibraryMetadatas(INTERPRETER);
+ assertEquals(1, metadata.size());
+ assertEquals("library.jar", metadata.get(0).getName());
+ }
+
+ @Test
+ void resolvesRepositoryFromRegisteredSettingId() throws Exception {
+ String settingName = "spark-display-name";
+ String settingId = "spark-setting-id";
+ Path settingRepository =
Files.createDirectory(dependencyRepository.resolve(settingId));
+ byte[] expected = {5, 6, 7};
+ Files.write(settingRepository.resolve("library.jar"), expected);
+ InterpreterSetting interpreterSetting = mock(InterpreterSetting.class);
+ when(interpreterSetting.getId()).thenReturn(settingId);
+ when(interpreterSettingManager.getInterpreterSettingByName(settingName))
+ .thenReturn(interpreterSetting);
+
+ ByteBuffer library = server.getLibrary(settingName, "library.jar");
+ assertArrayEquals(expected, library.array());
+ }
+
+ @Test
+ void rejectsTraversalAbsoluteAndMultiSegmentInterpreterPaths() throws
Exception {
+ Path outsideDirectory =
Files.createDirectory(temporaryDirectory.resolve("outside"));
+ Files.write(outsideDirectory.resolve("library.jar"), new byte[] {9});
+
+ assertNull(server.getLibrary("../../../../etc", "passwd"));
+
+ List<String> invalidInterpreters = List.of(
+ "../outside",
+ outsideDirectory.toString(),
+ "spark/child",
+ "spark\\child",
+ ".",
+ "..");
+ for (String invalidInterpreter : invalidInterpreters) {
+ assertNull(server.getLibrary(invalidInterpreter, "library.jar"),
invalidInterpreter);
+ assertTrue(
+ server.getAllLibraryMetadatas(invalidInterpreter).isEmpty(),
invalidInterpreter);
+ }
+ }
+
+ @Test
+ void rejectsTraversalAbsoluteAndMultiSegmentLibraryPaths() throws Exception {
+ Path outsideLibrary = Files.write(
+ temporaryDirectory.resolve("outside.jar"), new byte[] {9});
+
+ List<String> invalidLibraries = List.of(
+ "../outside.jar",
+ outsideLibrary.toString(),
+ "nested/library.jar",
+ "nested\\library.jar",
+ ".",
+ "..");
+ for (String invalidLibrary : invalidLibraries) {
+ assertNull(server.getLibrary(INTERPRETER, invalidLibrary),
invalidLibrary);
+ }
+ }
+
+ @Test
+ void rejectsUnregisteredInterpreterNonJarAndDirectory() throws Exception {
+ Path interpreterRepository = dependencyRepository.resolve(INTERPRETER);
+ Files.write(interpreterRepository.resolve("library.txt"), new byte[] {1});
+ Files.createDirectory(interpreterRepository.resolve("directory.jar"));
+ Path unregisteredRepository = Files.createDirectory(
+ dependencyRepository.resolve("unregistered"));
+ Files.write(unregisteredRepository.resolve("library.jar"), new byte[] {1});
+
+ assertNull(server.getLibrary("unregistered", "library.jar"));
+ assertTrue(server.getAllLibraryMetadatas("unregistered").isEmpty());
+ assertNull(server.getLibrary(INTERPRETER, "library.txt"));
+ assertNull(server.getLibrary(INTERPRETER, "directory.jar"));
+ assertTrue(server.getAllLibraryMetadatas(INTERPRETER).isEmpty());
+ }
+
+ @Test
+ void rejectsLibraryAndInterpreterSymlinkEscapes() throws Exception {
+ Path outsideDirectory =
Files.createDirectory(temporaryDirectory.resolve("outside"));
+ Path outsideLibrary = Files.write(outsideDirectory.resolve("outside.jar"),
new byte[] {9});
+ Files.createSymbolicLink(
+ dependencyRepository.resolve(INTERPRETER).resolve("library.jar"),
outsideLibrary);
+
+ assertNull(server.getLibrary(INTERPRETER, "library.jar"));
+ assertTrue(server.getAllLibraryMetadatas(INTERPRETER).isEmpty());
+
+ String linkedInterpreter = "linked-interpreter";
+ Files.createSymbolicLink(dependencyRepository.resolve(linkedInterpreter),
outsideDirectory);
+ InterpreterSetting interpreterSetting = mock(InterpreterSetting.class);
+ when(interpreterSetting.getId()).thenReturn(linkedInterpreter);
+
when(interpreterSettingManager.getInterpreterSettingByName(linkedInterpreter))
+ .thenReturn(interpreterSetting);
+ assertNull(server.getLibrary(linkedInterpreter, "outside.jar"));
+ assertTrue(server.getAllLibraryMetadatas(linkedInterpreter).isEmpty());
+ }
+}
