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

This commit is contained in:
Daniel Chao
2025-10-09 10:16:33 -07:00
committed by GitHub
parent 3a29ea8998
commit 42dcad25c6
4 changed files with 43 additions and 30 deletions

View File

@@ -145,17 +145,17 @@ public final class SecurityManagers {
@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 final class SecurityManagers {
}
}
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 @@ public final class SecurityManagers {
}
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);
}
}

View File

@@ -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.

View File

@@ -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`")
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)
.hasMessageContaining(
"Refusing to read resource `file:///bar.txt` because it is not within the root directory (`--root-dir`)."
)
}
@Test

View File

@@ -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