Correctly set allowed modules/resoures when external reader scheme contain regex control characters (#941)

pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/CliCommand.kt

@@ -1,5 +1,5 @@
 /*
- * Copyright © 2024 Apple Inc. and the Pkl project authors. All rights reserved.
+ * Copyright © 2024-2025 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.
@@ -88,7 +88,7 @@ abstract class CliCommand(protected val cliOptions: CliBaseOptions) {
         cliOptions.allowedModules ?: SecurityManagers.defaultAllowedModules,
         cliOptions.allowedResources ?: SecurityManagers.defaultAllowedResources,
         SecurityManagers.defaultTrustLevels,
-        cliOptions.normalizedRootDir
+        cliOptions.normalizedRootDir,
       )
     val envVars = cliOptions.environmentVariables ?: System.getenv()
     val stackFrameTransformer =
@@ -99,7 +99,7 @@ abstract class CliCommand(protected val cliOptions: CliBaseOptions) {
       securityManager,
       cliOptions.timeout,
       stackFrameTransformer,
-      envVars
+      envVars,
     )
   }
 
@@ -110,15 +110,15 @@ abstract class CliCommand(protected val cliOptions: CliBaseOptions) {
   protected val allowedModules: List<Pattern> by lazy {
     cliOptions.allowedModules
       ?: evaluatorSettings?.allowedModules
-        ?: (SecurityManagers.defaultAllowedModules +
+      ?: (SecurityManagers.defaultAllowedModules +
-        externalModuleReaders.keys.map { Pattern.compile("$it:") }.toList())
+        externalModuleReaders.keys.map { Pattern.compile(Pattern.quote("$it:")) }.toList())
   }
 
   protected val allowedResources: List<Pattern> by lazy {
     cliOptions.allowedResources
       ?: evaluatorSettings?.allowedResources
-        ?: (SecurityManagers.defaultAllowedResources +
+      ?: (SecurityManagers.defaultAllowedResources +
-        externalResourceReaders.keys.map { Pattern.compile("$it:") }.toList())
+        externalResourceReaders.keys.map { Pattern.compile(Pattern.quote("$it:")) }.toList())
   }
 
   protected val rootDir: Path? by lazy {
@@ -140,7 +140,7 @@ abstract class CliCommand(protected val cliOptions: CliBaseOptions) {
         ?: evaluatorSettings?.let { settings ->
           if (settings.noCache == true) null else settings.moduleCacheDir
         }
-          ?: IoUtils.getDefaultModuleCacheDir()
+        ?: IoUtils.getDefaultModuleCacheDir()
   }
 
   protected val modulePath: List<Path> by lazy {
@@ -160,7 +160,7 @@ abstract class CliCommand(protected val cliOptions: CliBaseOptions) {
       allowedModules,
       allowedResources,
       SecurityManagers.defaultTrustLevels,
-      rootDir
+      rootDir,
     )
   }
 
@@ -168,22 +168,24 @@ abstract class CliCommand(protected val cliOptions: CliBaseOptions) {
 
   private val proxyAddress by lazy {
     cliOptions.httpProxy
-      ?: project?.evaluatorSettings?.http?.proxy?.address ?: settings.http?.proxy?.address
+      ?: project?.evaluatorSettings?.http?.proxy?.address
+      ?: settings.http?.proxy?.address
   }
 
   private val noProxy by lazy {
     cliOptions.httpNoProxy
-      ?: project?.evaluatorSettings?.http?.proxy?.noProxy ?: settings.http?.proxy?.noProxy
+      ?: project?.evaluatorSettings?.http?.proxy?.noProxy
+      ?: settings.http?.proxy?.noProxy
   }
 
   private val externalModuleReaders by lazy {
-    (project?.evaluatorSettings?.externalModuleReaders
+    (project?.evaluatorSettings?.externalModuleReaders ?: emptyMap()) +
-      ?: emptyMap()) + cliOptions.externalModuleReaders
+      cliOptions.externalModuleReaders
   }
 
   private val externalResourceReaders by lazy {
-    (project?.evaluatorSettings?.externalResourceReaders
+    (project?.evaluatorSettings?.externalResourceReaders ?: emptyMap()) +
-      ?: emptyMap()) + cliOptions.externalResourceReaders
+      cliOptions.externalResourceReaders
   }
 
   private val externalProcesses by lazy {

pkl-commons-cli/src/test/kotlin/org/pkl/commons/cli/BaseCommandTest.kt

@@ -1,5 +1,5 @@
 /*
- * Copyright © 2024 Apple Inc. and the Pkl project authors. All rights reserved.
+ * Copyright © 2024-2025 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.
@@ -82,24 +82,30 @@ class BaseCommandTest {
         "scheme3=reader3",
         "--external-module-reader",
         "scheme4=reader4 with args",
+        "--external-module-reader",
+        "scheme+ext=reader5 with args",
         "--external-resource-reader",
         "scheme1=reader1",
         "--external-resource-reader",
-        "scheme2=reader2 with args"
+        "scheme2=reader2 with args",
+        "--external-resource-reader",
+        "scheme+ext=reader5 with args",
       )
     )
     assertThat(cmd.baseOptions.externalModuleReaders)
       .isEqualTo(
         mapOf(
           "scheme3" to ExternalReader("reader3", emptyList()),
-          "scheme4" to ExternalReader("reader4", listOf("with", "args"))
+          "scheme4" to ExternalReader("reader4", listOf("with", "args")),
+          "scheme+ext" to ExternalReader("reader5", listOf("with", "args")),
         )
       )
     assertThat(cmd.baseOptions.externalResourceReaders)
       .isEqualTo(
         mapOf(
           "scheme1" to ExternalReader("reader1", emptyList()),
-          "scheme2" to ExternalReader("reader2", listOf("with", "args"))
+          "scheme2" to ExternalReader("reader2", listOf("with", "args")),
+          "scheme+ext" to ExternalReader("reader5", listOf("with", "args")),
         )
       )
   }

pkl-commons-cli/src/test/kotlin/org/pkl/commons/cli/CliCommandTest.kt

@@ -0,0 +1,68 @@
+/*
+ * Copyright © 2025 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.
+ * You may obtain a copy of the License at
+ *
+ *     https://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.pkl.commons.cli
+
+import org.assertj.core.api.Assertions.assertThat
+import org.junit.jupiter.api.Test
+import org.pkl.commons.cli.commands.BaseCommand
+import org.pkl.core.SecurityManagers
+
+class CliCommandTest {
+
+  class CliTest(private val options: CliBaseOptions) : CliCommand(options) {
+    override fun doRun() = Unit
+
+    val _allowedResources = allowedResources
+    val _allowedModules = allowedModules
+  }
+
+  private val cmd =
+    object : BaseCommand("test", "") {
+      override fun run() = Unit
+    }
+
+  @Test
+  fun `--external-resource-reader and --external-module-reader populate allowed modules and resources`() {
+    cmd.parse(
+      arrayOf(
+        "--external-module-reader",
+        "scheme3=reader3",
+        "--external-module-reader",
+        "scheme4=reader4 with args",
+        "--external-module-reader",
+        "scheme+ext=reader5 with args",
+        "--external-resource-reader",
+        "scheme1=reader1",
+        "--external-resource-reader",
+        "scheme2=reader2 with args",
+        "--external-resource-reader",
+        "scheme+ext=reader5 with args",
+      )
+    )
+    val opts = cmd.baseOptions.baseOptions(emptyList(), null, true)
+    val cliTest = CliTest(opts)
+    assertThat(cliTest._allowedModules.map { it.pattern() })
+      .isEqualTo(
+        SecurityManagers.defaultAllowedModules.map { it.pattern() } +
+          listOf("\\Qscheme3:\\E", "\\Qscheme4:\\E", "\\Qscheme+ext:\\E")
+      )
+    assertThat(cliTest._allowedResources.map { it.pattern() })
+      .isEqualTo(
+        SecurityManagers.defaultAllowedResources.map { it.pattern() } +
+          listOf("\\Qscheme1:\\E", "\\Qscheme2:\\E", "\\Qscheme+ext:\\E")
+      )
+  }
+}