fix: resolve checkstyle and CodeQL security issues

- Fix import ordering by moving static imports to the end
- Add path traversal validation to prevent security vulnerability
- Normalize paths and validate against resource directory to prevent directory traversal attacks
- Handle null returns from getPathToCheck for invalid paths

Co-authored-by: chirag-madlani <[email protected]>

Author: 2 people

openmetadata-service/src/main/java/org/openmetadata/service/socket/OpenMetadataAssetServlet.java

@@ -13,20 +13,19 @@
 
 package org.openmetadata.service.socket;
 
-import java.net.URL;
-import jakarta.servlet.ServletRequest;
-import jakarta.servlet.ServletResponse;
-import jakarta.servlet.http.HttpServletRequestWrapper;
-import jakarta.servlet.http.HttpServletResponseWrapper;
-
 import static org.openmetadata.service.exception.OMErrorPageHandler.setSecurityHeader;
 
 import io.dropwizard.servlets.assets.AssetServlet;
 import jakarta.servlet.ServletException;
 import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletRequestWrapper;
 import jakarta.servlet.http.HttpServletResponse;
+import jakarta.servlet.http.HttpServletResponseWrapper;
 import java.io.IOException;
+import java.net.URL;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import org.jetbrains.annotations.Nullable;
 import org.openmetadata.service.config.OMWebConfiguration;
 import org.openmetadata.service.resources.system.IndexResource;
@@ -66,27 +65,31 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp)
 
     // 1. Check for Brotli (br)
     if (acceptEncoding != null && acceptEncoding.contains("br")) {
-       try {
-         String fullResourcePath = getPathToCheck(req, requestUri, ".br");
-         URL url = this.getClass().getResource(fullResourcePath);
-         if (url != null) {
-           serveCompressed(req, resp, requestUri, "br", "br");
-           return;
-         }
-       } catch (Exception e) {
-         // Ignore and try next
-       }
+      try {
+        String fullResourcePath = getPathToCheck(req, requestUri, ".br");
+        if (fullResourcePath != null) {
+          URL url = this.getClass().getResource(fullResourcePath);
+          if (url != null) {
+            serveCompressed(req, resp, requestUri, "br", "br");
+            return;
+          }
+        }
+      } catch (Exception e) {
+        // Ignore and try next
+      }
     }
 
     // 2. Check for Gzip
     if (acceptEncoding != null && acceptEncoding.contains("gzip")) {
       try {
         String fullResourcePath = getPathToCheck(req, requestUri, ".gz");
-        URL url = this.getClass().getResource(fullResourcePath);
-        
-        if (url != null) {
-          serveCompressed(req, resp, requestUri, "gzip", "gz");
-          return;
+        if (fullResourcePath != null) {
+          URL url = this.getClass().getResource(fullResourcePath);
+
+          if (url != null) {
+            serveCompressed(req, resp, requestUri, "gzip", "gz");
+            return;
+          }
         }
       } catch (Exception e) {
         // Fallback to default behavior
@@ -109,43 +112,67 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp)
   }
 
   private String getPathToCheck(HttpServletRequest req, String requestUri, String extension) {
-      String pathToCheck = requestUri;
-      String contextPath = req.getContextPath();
-      if (contextPath != null && requestUri.startsWith(contextPath)) {
-        pathToCheck = requestUri.substring(contextPath.length());
-      }
-      return this.resourcePath + (pathToCheck.startsWith("/") ? "" : "/") + pathToCheck + extension;
-  }
+    String pathToCheck = requestUri;
+    String contextPath = req.getContextPath();
+    if (contextPath != null && requestUri.startsWith(contextPath)) {
+      pathToCheck = requestUri.substring(contextPath.length());
+    }
 
-  private void serveCompressed(HttpServletRequest req, HttpServletResponse resp, String requestUri, String contentEncoding, String extension) throws ServletException, IOException {
-      resp.setHeader("Content-Encoding", contentEncoding);
-      String mimeType = req.getServletContext().getMimeType(requestUri);
+    String fullPath =
+        this.resourcePath + (pathToCheck.startsWith("/") ? "" : "/") + pathToCheck + extension;
 
-      HttpServletRequestWrapper compressedReq = new HttpServletRequestWrapper(req) {
-        @Override
-        public String getPathInfo() {
-           String pathInfo = super.getPathInfo();
-           return pathInfo != null ? pathInfo + "." + extension : null;
-        }
+    // Validate against path traversal attacks
+    try {
+      Path normalizedPath = Paths.get(fullPath).normalize();
+      Path baseResourcePath = Paths.get(this.resourcePath).normalize();
 
-        @Override
-        public String getRequestURI() {
-          return super.getRequestURI() + "." + extension;
-        }
-      };
+      if (!normalizedPath.startsWith(baseResourcePath)) {
+        return null;
+      }
+    } catch (Exception e) {
+      return null;
+    }
+
+    return fullPath;
+  }
 
-      HttpServletResponseWrapper compressedResp = new HttpServletResponseWrapper(resp) {
-        @Override
-        public void setContentType(String type) {
-           if (mimeType != null) {
+  private void serveCompressed(
+      HttpServletRequest req,
+      HttpServletResponse resp,
+      String requestUri,
+      String contentEncoding,
+      String extension)
+      throws ServletException, IOException {
+    resp.setHeader("Content-Encoding", contentEncoding);
+    String mimeType = req.getServletContext().getMimeType(requestUri);
+
+    HttpServletRequestWrapper compressedReq =
+        new HttpServletRequestWrapper(req) {
+          @Override
+          public String getPathInfo() {
+            String pathInfo = super.getPathInfo();
+            return pathInfo != null ? pathInfo + "." + extension : null;
+          }
+
+          @Override
+          public String getRequestURI() {
+            return super.getRequestURI() + "." + extension;
+          }
+        };
+
+    HttpServletResponseWrapper compressedResp =
+        new HttpServletResponseWrapper(resp) {
+          @Override
+          public void setContentType(String type) {
+            if (mimeType != null) {
               super.setContentType(mimeType);
-           } else {
+            } else {
               super.setContentType(type);
-           }
-        }
-      };
+            }
+          }
+        };
 
-      super.doGet(compressedReq, compressedResp);
+    super.doGet(compressedReq, compressedResp);
   }
 
   /**

openmetadata-service/src/test/java/org/openmetadata/service/socket/OpenMetadataAssetServletTest.java

@@ -8,8 +8,6 @@
 import jakarta.servlet.ServletOutputStream;
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
-import java.net.URL;
-import java.util.Collections;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.mockito.Mock;
@@ -31,7 +29,7 @@ public void setup() throws Exception {
     MockitoAnnotations.openMocks(this);
     when(request.getServletContext()).thenReturn(servletContext);
     when(response.getOutputStream()).thenReturn(outputStream);
-    
+
     // Initialize servlet with /assets as resource path
     servlet = new OpenMetadataAssetServlet("/", "/assets", "/", "index.html", webConfiguration);
   }
@@ -122,8 +120,8 @@ public void testServeBrotliAsset() throws Exception {
     try {
       servlet.doGet(request, response);
     } catch (Exception e) {
-       e.printStackTrace();
-       throw e;
+      e.printStackTrace();
+      throw e;
     }
 
     // Verify Content-Encoding is br