Fix error message when reading a resource/module past root dir (#1234)

Author: bioball

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

@@ -145,17 +145,17 @@ private static class Standard implements SecurityManager {
 
     @Override
     public void checkResolveModule(URI uri) throws SecurityManagerException {
-      checkRead(uri, allowedModules, "moduleNotInAllowList");
+      checkRead(uri, allowedModules, false);
     }
 
     @Override
     public void checkResolveResource(URI resource) throws SecurityManagerException {
-      checkRead(resource, allowedResources, "resourceNotInAllowList");
+      checkRead(resource, allowedResources, true);
     }
 
     @Override
     public void checkReadResource(URI uri) throws SecurityManagerException {
-      checkRead(uri, allowedResources, "resourceNotInAllowList");
+      checkRead(uri, allowedResources, true);
     }
 
     @Override
@@ -185,21 +185,21 @@ public void checkImportModule(URI importingModule, URI importedModule)
       }
     }
 
-    private void checkRead(URI uri, List<Pattern> allowedPatterns, String errorMessageKey)
+    private void checkRead(URI uri, List<Pattern> allowedPatterns, boolean isResource)
         throws SecurityManagerException {
       for (var pattern : allowedPatterns) {
         if (pattern.matcher(uri.toString()).lookingAt()) {
-          checkIsUnderRootDir(uri, errorMessageKey);
+          checkIsUnderRootDir(uri, isResource);
           return;
         }
       }
 
-      var message = ErrorMessages.create(errorMessageKey, uri);
+      var messageKey = isResource ? "resourceNotInAllowList" : "moduleNotInAllowList";
+      var message = ErrorMessages.create(messageKey, uri);
       throw new SecurityManagerException(message);
     }
 
-    private void checkIsUnderRootDir(URI uri, String errorMessageKey)
-        throws SecurityManagerException {
+    private void checkIsUnderRootDir(URI uri, boolean isResource) throws SecurityManagerException {
       if (!uri.isAbsolute()) {
         throw new AssertionError("Expected absolute URI but got: " + uri);
       }
@@ -220,7 +220,8 @@ private void checkIsUnderRootDir(URI uri, String errorMessageKey)
       }
 
       if (!path.startsWith(rootDir)) {
-        var message = ErrorMessages.create(errorMessageKey, uri);
+        var errorMessageKey = isResource ? "resourcePastRootDir" : "modulePastRootDir";
+        var message = ErrorMessages.create(errorMessageKey, uri, rootDir);
         throw new SecurityManagerException(message);
       }
     }

pkl-core/src/main/resources/org/pkl/core/errorMessages.properties

@@ -690,6 +690,12 @@ Refusing to read resource `{0}` because it does not match any entry in the resou
 moduleNotInAllowList=\
 Refusing to load module `{0}` because it does not match any entry in the module allowlist (`--allowed-modules`).
 
+resourcePastRootDir=\
+Refusing to read resource `{0}` because it is not within the root directory (`--root-dir`).
+
+modulePastRootDir=\
+Refusing to load module `{0}` because it is not within the root directory (`--root-dir`).
+
 insufficientModuleTrustLevel=\
 Refusing to import module `{0}` because importing module `{1}` has an insufficient trust level.
 

pkl-core/src/test/kotlin/org/pkl/core/EvaluatorTest.kt

@@ -269,27 +269,35 @@ class EvaluatorTest {
   @Test
   fun `cannot import module located outside root dir`(@TempDir tempDir: Path) {
     val evaluator =
-      EvaluatorBuilder.preconfigured()
-        .setSecurityManager(
-          SecurityManagers.standard(
-            SecurityManagers.defaultAllowedModules,
-            SecurityManagers.defaultAllowedResources,
-            SecurityManagers.defaultTrustLevels,
-            tempDir,
-          )
-        )
-        .build()
+      with(EvaluatorBuilder.preconfigured()) {
+        rootDir = tempDir
+        build()
+      }
 
-    val module = tempDir.resolve("test.pkl")
-    module.writeString(
-      """
-      amends "/non/existing.pkl"
-    """
-        .trimIndent()
-    )
+    val module = tempDir.resolve("test.pkl").writeString("amends \"/non/existing.pkl\"")
+
+    val e = assertThrows<PklException> { evaluator.evaluate(path(module)) }
+    assertThat(e.message)
+      .contains(
+        "Refusing to load module `file:///non/existing.pkl` because it is not within the root directory (`--root-dir`)."
+      )
+  }
+
+  @Test
+  fun `cannot read resource located outside root dir`(@TempDir tempDir: Path) {
+    val evaluator =
+      with(EvaluatorBuilder.preconfigured()) {
+        rootDir = tempDir
+        build()
+      }
+
+    val module = tempDir.resolve("test.pkl").writeString("res = read(\"/bar.txt\")")
 
     val e = assertThrows<PklException> { evaluator.evaluate(path(module)) }
-    assertThat(e.message).contains("Refusing to load module `file:///non/existing.pkl`")
+    assertThat(e)
+      .hasMessageContaining(
+        "Refusing to read resource `file:///bar.txt` because it is not within the root directory (`--root-dir`)."
+      )
   }
 
   @Test

pkl-gradle/src/test/kotlin/org/pkl/gradle/EvaluatorsTest.kt

@@ -306,9 +306,7 @@ class EvaluatorsTest : AbstractTest() {
     val result = runTask("evalTest", expectFailure = true)
     assertThat(result.output).contains("Refusing to load module")
     assertThat(result.output)
-      .contains(
-        "because it does not match any entry in the module allowlist (`--allowed-modules`)."
-      )
+      .contains("because it is not within the root directory (`--root-dir`).")
   }
 
   @Test