[MINOR] Reject traversal segments in note and folder paths

zeppelin-plugins/notebookrepo/filesystem/src/main/java/org/apache/zeppelin/notebook/repo/FileSystemNotebookRepo.java

@@ -103,6 +103,8 @@ public void move(String noteId,
   @Override
   public void move(String folderPath, String newFolderPath, AuthenticationInfo subject)
       throws IOException {
+    NotebookPathValidator.rejectTraversalSegments(folderPath);
+    NotebookPathValidator.rejectTraversalSegments(newFolderPath);
     // [ZEPPELIN-4195] newFolderPath parent path maybe not exist
     this.fs.tryMkDir(new Path(notebookDir, folderPath.substring(1)).getParent());
     this.fs.move(new Path(notebookDir, folderPath.substring(1)),
@@ -119,6 +121,7 @@ public void remove(String noteId, String notePath, AuthenticationInfo subject)
 
   @Override
   public void remove(String folderPath, AuthenticationInfo subject) throws IOException {
+    NotebookPathValidator.rejectTraversalSegments(folderPath);
     if (!this.fs.delete(new Path(notebookDir, folderPath.substring(1)))) {
       LOGGER.warn("Fail to remove folder: {}", folderPath);
     }

zeppelin-server/src/main/java/org/apache/zeppelin/notebook/repo/NotebookPathValidator.java

@@ -0,0 +1,87 @@
+/*
+ * 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.notebook.repo;
+
+import java.io.IOException;
+import java.net.URLDecoder;
+import java.nio.charset.StandardCharsets;
+import java.util.regex.Pattern;
+
+/**
+ * Note-path validation helpers shared by {@link NotebookRepo} implementations
+ * and the service layer. A {@code final} class with {@code static} methods
+ * (rather than {@link NotebookRepo} default methods) prevents an
+ * implementation from accidentally — or intentionally — overriding the
+ * checks and bypassing them.
+ */
+public final class NotebookPathValidator {
+
+  private static final Pattern PATH_SEGMENT_SPLIT = Pattern.compile("/+");
+  private static final String PARENT_SEGMENT = "..";
+  private static final String CURRENT_SEGMENT = ".";
+  private static final int MAX_DECODE_LAYERS = 5;
+
+  private NotebookPathValidator() {
+  }
+
+  /**
+   * Refuses any {@code ..} or {@code .} segment in {@code notePath}. The
+   * input is URL-decoded repeatedly first so that variants such as
+   * {@code %2e%2e} or {@code %252e%252e} cannot bypass the check.
+   *
+   * @throws IOException if the path is null, contains a traversal segment,
+   *     or has more URL-encoding layers than {@value #MAX_DECODE_LAYERS}
+   */
+  public static void rejectTraversalSegments(String notePath) throws IOException {
+    if (notePath == null) {
+      throw new IOException("Path must not be null");
+    }
+    String decoded = decodeRepeatedly(notePath);
+    String stripped = decoded.startsWith("/") ? decoded.substring(1) : decoded;
+    for (String segment : PATH_SEGMENT_SPLIT.split(stripped)) {
+      if (PARENT_SEGMENT.equals(segment) || CURRENT_SEGMENT.equals(segment)) {
+        throw new IOException("Path traversal segments are not allowed: " + notePath);
+      }
+    }
+  }
+
+  /**
+   * Repeatedly URL-decodes {@code encoded} until it stabilises, capped at
+   * {@value #MAX_DECODE_LAYERS} layers. Detecting stability after {@code N}
+   * layers of encoding requires {@code N + 1} decode passes — the final pass
+   * confirms the result is unchanged — so the loop runs one extra iteration.
+   *
+   * @throws IOException if the input is malformed or has more URL-encoding
+   *     layers than {@value #MAX_DECODE_LAYERS}
+   */
+  public static String decodeRepeatedly(String encoded) throws IOException {
+    String previous = encoded;
+    for (int pass = 0; pass <= MAX_DECODE_LAYERS; pass++) {
+      String decoded;
+      try {
+        decoded = URLDecoder.decode(previous, StandardCharsets.UTF_8);
+      } catch (IllegalArgumentException e) {
+        throw new IOException("Malformed URL-encoded path: " + encoded, e);
+      }
+      if (decoded.equals(previous)) {
+        return decoded;
+      }
+      previous = decoded;
+    }
+    throw new IOException("Exceeded maximum decode attempts. Possible malicious input.");
+  }
+}

zeppelin-server/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepo.java

@@ -146,6 +146,7 @@ default String buildNoteFileName(String noteId, String notePath) throws IOExcept
     if (!notePath.startsWith("/")) {
       throw new IOException("Invalid notePath: " + notePath);
     }
+    NotebookPathValidator.rejectTraversalSegments(notePath);
     return (notePath + "_" + noteId + ".zpln").substring(1);
   }
 

zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java

@@ -25,8 +25,6 @@
 
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
-import java.net.URLDecoder;
-import java.nio.charset.StandardCharsets;
 import java.text.ParseException;
 import java.text.SimpleDateFormat;
 import java.time.Instant;
@@ -57,6 +55,7 @@
 import org.apache.zeppelin.notebook.AuthorizationService;
 import org.apache.zeppelin.notebook.exception.CorruptedNoteException;
 import org.apache.zeppelin.notebook.exception.NotePathAlreadyExistsException;
+import org.apache.zeppelin.notebook.repo.NotebookPathValidator;
 import org.apache.zeppelin.notebook.repo.NotebookRepoWithVersionControl;
 import org.apache.zeppelin.notebook.scheduler.SchedulerService;
 import org.apache.zeppelin.common.Message;
@@ -240,7 +239,7 @@ String normalizeNotePath(String notePath) throws IOException {
 
     notePath = notePath.replace("\r", " ").replace("\n", " ");
 
-    notePath = decodeRepeatedly(notePath);
+    notePath = NotebookPathValidator.decodeRepeatedly(notePath);
     if (notePath.endsWith("/")) {
       throw new IOException("Note name shouldn't end with '/'");
     }
@@ -315,6 +314,7 @@ public void renameNote(String noteId,
           }
         }
         try {
+          newNotePathReal = normalizeNotePath(newNotePathReal);
           notebook.moveNote(noteId, newNotePathReal, context.getAutheInfo());
           callback.onSuccess(readNote, context);
         } catch (NotePathAlreadyExistsException e) {
@@ -1567,20 +1567,4 @@ private <T> boolean checkPermission(String noteId,
     }
   }
 
-  private static String decodeRepeatedly(final String encoded) throws IOException {
-    String previous = encoded;
-    int maxDecodeAttempts = 5;
-    int attempts = 0;
-
-    while (attempts < maxDecodeAttempts) {
-      String decoded = URLDecoder.decode(previous, StandardCharsets.UTF_8);
-      attempts++;
-      if (decoded.equals(previous)) {
-        return decoded;
-      }
-      previous = decoded;
-    }
-
-    throw new IOException("Exceeded maximum decode attempts. Possible malicious input.");
-  }
 }

zeppelin-server/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoPathValidationTest.java

@@ -0,0 +1,113 @@
+/*
+ * 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.notebook.repo;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.io.IOException;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+class NotebookRepoPathValidationTest {
+
+  @ParameterizedTest
+  @ValueSource(strings = {
+      "/../etc/passwd",
+      "/foo/../../etc/passwd",
+      "/foo/../bar",
+      "/foo/./bar",
+      "/./bar",
+      "/foo/..",
+      "/..",
+      "/.",
+      // URL-encoded variants must be rejected after decoding.
+      "/%2e%2e/etc/passwd",
+      "/foo/%2E%2E/bar",
+      "/%2e/foo",
+      // Double-encoded.
+      "/%252e%252e/etc/passwd",
+  })
+  void rejectTraversalSegments_rejects_traversal(String malicious) {
+    assertThrows(IOException.class,
+        () -> NotebookPathValidator.rejectTraversalSegments(malicious),
+        "expected rejection for: " + malicious);
+  }
+
+  @ParameterizedTest
+  @ValueSource(strings = {
+      "/MyNote",
+      "/folder/MyNote",
+      "/folder/sub-folder/My Note With Spaces",
+      "/한글노트",
+      "/foo.bar.baz",
+      // ".." and "." are rejected only as exact segments — names containing
+      // dots remain valid.
+      "/...",
+      "/foo/...",
+      "/foo..bar",
+  })
+  void rejectTraversalSegments_accepts_normal_paths(String safe) throws IOException {
+    NotebookPathValidator.rejectTraversalSegments(safe);
+  }
+
+  @Test
+  void rejectTraversalSegments_rejects_null() {
+    assertThrows(IOException.class, () -> NotebookPathValidator.rejectTraversalSegments(null));
+  }
+
+  @Test
+  void rejectTraversalSegments_rejects_excessive_encoding_layers() {
+    // Six layers of "..", past the 5-layer decode cap.
+    String payload = "/%252525252e%252525252e/etc";
+    assertThrows(IOException.class, () -> NotebookPathValidator.rejectTraversalSegments(payload));
+  }
+
+  @Test
+  void decodeRepeatedly_returns_input_when_already_decoded() throws IOException {
+    assertEquals("/foo bar", NotebookPathValidator.decodeRepeatedly("/foo bar"));
+  }
+
+  @Test
+  void decodeRepeatedly_decodes_until_stable() throws IOException {
+    assertEquals("/..", NotebookPathValidator.decodeRepeatedly("/%252e%252e"));
+  }
+
+  @Test
+  void decodeRepeatedly_throws_on_too_many_layers() {
+    String payload = "/%2525252525252525252e";
+    assertThrows(IOException.class, () -> NotebookPathValidator.decodeRepeatedly(payload));
+  }
+
+  @Test
+  void decodeRepeatedly_wraps_malformed_percent_encoding_as_io_exception() {
+    // Trailing "%" without two hex digits makes URLDecoder.decode raise
+    // IllegalArgumentException; the validator must convert it to IOException
+    // so callers get a single, declared failure mode.
+    assertThrows(IOException.class, () -> NotebookPathValidator.decodeRepeatedly("/foo%"));
+    assertThrows(IOException.class, () -> NotebookPathValidator.decodeRepeatedly("/foo%ZZ"));
+  }
+
+  @Test
+  void decodeRepeatedly_accepts_max_decode_layers() throws IOException {
+    // Exactly five layers of "%2e" wrapping ("%252525252e") must decode
+    // cleanly; the constant means *layers*, not raw loop iterations.
+    assertEquals("/..", NotebookPathValidator.decodeRepeatedly("/%252525252e%252525252e"));
+  }
+}