Fix possible race condition during module and resource reading (#1426)

Author: stackoverflow

pkl-core/src/main/java/org/pkl/core/SecurityManager.java

@@ -1,5 +1,5 @@
 /*
- * Copyright © 2024 Apple Inc. and the Pkl project authors. All rights reserved.
+ * Copyright © 2024-2026 Apple Inc. and the Pkl project authors. All rights reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -15,7 +15,10 @@
  */
 package org.pkl.core;
 
+import java.io.IOException;
 import java.net.URI;
+import java.nio.file.Path;
+import org.pkl.core.util.Nullable;
 
 /**
  * Enforces a security model during {@link Evaluator evaluation}.
@@ -40,4 +43,20 @@ public interface SecurityManager {
    * to access the given URI.
    */
   void checkResolveResource(URI resource) throws SecurityManagerException;
+
+  /**
+   * Resolves the given {@code file:} URI to a secure, symlink-free path that has been verified to
+   * be within the root directory (if one is configured). The returned path can be opened with
+   * {@link java.nio.file.LinkOption#NOFOLLOW_LINKS}.
+   *
+   * <p>Returns {@code null} for non-{@code file:} URIs or if no root directory is configured.
+   *
+   * @param uri the URI to resolve
+   * @return the resolved, symlink-free path under root directory, or {@code null}
+   * @throws SecurityManagerException if the resolved path is not within the root directory
+   * @throws IOException if the path cannot be resolved
+   */
+  default @Nullable Path resolveSecurePath(URI uri) throws SecurityManagerException, IOException {
+    return null;
+  }
 }

pkl-core/src/main/java/org/pkl/core/SecurityManagers.java

@@ -1,5 +1,5 @@
 /*
- * Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved.
+ * Copyright © 2024-2026 Apple Inc. and the Pkl project authors. All rights reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -171,6 +171,19 @@ public void checkImportModule(URI importingModule, URI importedModule)
       }
     }
 
+    @Override
+    public @Nullable Path resolveSecurePath(URI uri) throws SecurityManagerException, IOException {
+      if (rootDir == null || !uri.isAbsolute() || !uri.getScheme().equals("file")) {
+        return null;
+      }
+      var path = Path.of(uri);
+      var realPath = path.toRealPath();
+      if (!realPath.startsWith(rootDir)) {
+        throw new SecurityManagerException(ErrorMessages.create("modulePastRootDir", uri, rootDir));
+      }
+      return realPath;
+    }
+
     private @Nullable Path normalizePath(@Nullable Path path) {
       if (path == null) {
         return null;

pkl-core/src/main/java/org/pkl/core/module/ModuleKeys.java

@@ -1,5 +1,5 @@
 /*
- * Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved.
+ * Copyright © 2024-2026 Apple Inc. and the Pkl project authors. All rights reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -345,10 +345,18 @@ public ResolvedModuleKey resolve(SecurityManager securityManager)
       if (java.io.File.separatorChar == '\\' && uriPath != null && uriPath.contains("\\")) {
         throw new FileNotFoundException();
       }
-      var realPath = IoUtils.pathOf(uri).toRealPath();
+      // Use resolveSecurePath to atomically resolve symlinks and verify under rootDir.
+      // The returned path is symlink-free, so it can be opened with NOFOLLOW_LINKS.
+      var securePath = securityManager.resolveSecurePath(uri);
+      Path realPath;
+      if (securePath != null) {
+        realPath = securePath;
+      } else {
+        realPath = IoUtils.pathOf(uri).toRealPath();
+      }
       var resolvedUri = realPath.toUri();
       securityManager.checkResolveModule(resolvedUri);
-      return ResolvedModuleKeys.file(this, resolvedUri, realPath);
+      return ResolvedModuleKeys.file(this, resolvedUri, realPath, securePath != null);
     }
 
     @Override
@@ -413,8 +421,14 @@ public ResolvedModuleKey resolve(SecurityManager securityManager)
         throws IOException, SecurityManagerException {
       securityManager.checkResolveModule(uri);
 
-      var path = resolver.resolve(uri).toRealPath();
-      return ResolvedModuleKeys.file(this, path.toUri(), path);
+      var securePath = securityManager.resolveSecurePath(uri);
+      Path path;
+      if (securePath != null) {
+        path = securePath;
+      } else {
+        path = resolver.resolve(uri).toRealPath();
+      }
+      return ResolvedModuleKeys.file(this, path.toUri(), path, securePath != null);
     }
   }
 

pkl-core/src/main/java/org/pkl/core/module/ResolvedModuleKeys.java

@@ -1,5 +1,5 @@
 /*
- * Copyright © 2024 Apple Inc. and the Pkl project authors. All rights reserved.
+ * Copyright © 2024-2026 Apple Inc. and the Pkl project authors. All rights reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -15,26 +15,36 @@
  */
 package org.pkl.core.module;
 
-import java.io.File;
 import java.io.IOException;
 import java.net.URI;
 import java.net.URL;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.AccessDeniedException;
 import java.nio.file.Files;
+import java.nio.file.LinkOption;
 import java.nio.file.Path;
 import org.pkl.core.util.IoUtils;
 
 /** Utilities for obtaining and using resolved module keys. */
 public final class ResolvedModuleKeys {
   private ResolvedModuleKeys() {}
 
+  /**
+   * Creates a resolved module key backed by the given file path. The resulting module will be
+   * loaded from that file path and cached using the given URI as cache key.
+   *
+   * @param nofollow if true, the file will be opened with {@link LinkOption#NOFOLLOW_LINKS}.
+   */
+  public static ResolvedModuleKey file(ModuleKey original, URI uri, Path path, boolean nofollow) {
+    return new FileKey(original, uri, path, nofollow);
+  }
+
   /**
    * Creates a resolved module key backed by the given file path. The resulting module will be
    * loaded from that file path and cached using the given URI as cache key.
    */
   public static ResolvedModuleKey file(ModuleKey original, URI uri, Path path) {
-    return new File(original, uri, path);
+    return new FileKey(original, uri, path, false);
   }
 
   /**
@@ -62,15 +72,17 @@ public static ResolvedModuleKey delegated(ResolvedModuleKey delegate, ModuleKey
     return new Delegated(delegate, original);
   }
 
-  private static class File implements ResolvedModuleKey {
+  private static class FileKey implements ResolvedModuleKey {
     final ModuleKey original;
     final URI uri;
     final Path path;
+    final boolean nofollow;
 
-    File(ModuleKey original, URI uri, Path path) {
+    FileKey(ModuleKey original, URI uri, Path path, boolean nofollow) {
       this.original = original;
       this.uri = uri;
       this.path = path;
+      this.nofollow = nofollow;
     }
 
     @Override
@@ -86,6 +98,11 @@ public URI getUri() {
     @Override
     public String loadSource() throws IOException {
       try {
+        if (nofollow) {
+          try (var in = Files.newInputStream(path, LinkOption.NOFOLLOW_LINKS)) {
+            return new String(in.readAllBytes(), StandardCharsets.UTF_8);
+          }
+        }
         return Files.readString(path, StandardCharsets.UTF_8);
       } catch (AccessDeniedException e) {
         // Windows throws `AccessDeniedException` when reading directories.

pkl-core/src/main/java/org/pkl/core/resource/ResourceReaders.java

@@ -1,5 +1,5 @@
 /*
- * Copyright © 2024-2025 Apple Inc. and the Pkl project authors. All rights reserved.
+ * Copyright © 2024-2026 Apple Inc. and the Pkl project authors. All rights reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -22,6 +22,8 @@
 import java.net.http.HttpRequest;
 import java.net.http.HttpResponse.BodyHandlers;
 import java.nio.file.Files;
+import java.nio.file.LinkOption;
+import java.nio.file.NoSuchFileException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
@@ -257,6 +259,21 @@ private static final class FileResource extends UrlResource {
     @Override
     public Optional<Object> read(URI uri) throws IOException, URISyntaxException {
       IoUtils.validateFileUri(uri);
+      // Use resolveSecurePath to get a symlink-free path verified under rootDir.
+      var securityManager = VmContext.get(null).getSecurityManager();
+      try {
+        var securePath = securityManager.resolveSecurePath(uri);
+        if (securePath != null) {
+          try (var in = Files.newInputStream(securePath, LinkOption.NOFOLLOW_LINKS)) {
+            var content = in.readAllBytes();
+            return Optional.of(new Resource(uri, content));
+          } catch (NoSuchFileException e) {
+            return Optional.empty();
+          }
+        }
+      } catch (SecurityManagerException e) {
+        throw new IOException(e);
+      }
       return super.read(uri);
     }