Allowlist-based approach to VCS string sanitization

anatoliykmetyuk · eed3si9n · commit 3a474ab060df · 2026-03-23T17:33:58.000-04:00
diff --git a/main/src/main/scala/sbt/internal/VcsUriFragment.scala b/main/src/main/scala/sbt/internal/VcsUriFragment.scala
@@ -14,15 +14,19 @@ private[sbt] object VcsUriFragment {
   def validate(fragment: String): Unit = {
     if (fragment == null)
       throw new IllegalArgumentException("VCS URI fragment must not be null")
+    if (fragment.isEmpty)
+      throw new IllegalArgumentException("VCS URI fragment must not be empty")
     fragment.foreach { c =>
-      if (c == '&' || c == '|' || c == ';')
+      if (!isAllowed(c))
         throw new IllegalArgumentException(
-          "Invalid character in VCS URI fragment (shell metacharacters are not allowed)"
-        )
-      if (Character.isISOControl(c))
-        throw new IllegalArgumentException(
-          "Invalid character in VCS URI fragment (control characters are not allowed)"
+          "Invalid character in VCS URI fragment (only ASCII letters, digits, and - _ . / + are allowed)"
         )
     }
   }
+
+  private def isAllowed(c: Char): Boolean =
+    (c >= 'a' && c <= 'z') ||
+      (c >= 'A' && c <= 'Z') ||
+      (c >= '0' && c <= '9') ||
+      c == '-' || c == '_' || c == '.' || c == '/' || c == '+'
 }
diff --git a/main/src/test/scala/sbt/internal/VcsUriFragmentTest.scala b/main/src/test/scala/sbt/internal/VcsUriFragmentTest.scala
@@ -15,9 +15,14 @@ import hedgehog.runner.*
 object VcsUriFragmentTest extends Properties:
   override def tests: List[Test] = List(
     example("accepts typical branch and tag names", testAcceptsSafe),
+    example("accepts hex commit id fragment", testAcceptsHexSha),
+    example("rejects empty fragment", testRejectsEmpty),
     example("rejects ampersand", testRejectsAmpersand),
     example("rejects pipe", testRejectsPipe),
     example("rejects semicolon", testRejectsSemicolon),
+    example("rejects space", testRejectsSpace),
+    example("rejects percent", testRejectsPercent),
+    example("rejects greater-than", testRejectsGreaterThan),
     example("rejects newline", testRejectsNewline),
     example("rejects DEL", testRejectsDel),
   )
@@ -26,8 +31,16 @@ object VcsUriFragmentTest extends Properties:
     VcsUriFragment.validate("develop")
     VcsUriFragment.validate("v1.2.3")
     VcsUriFragment.validate("feature/foo-bar")
+    VcsUriFragment.validate("release/1.0.0+build")
     Result.success
 
+  def testAcceptsHexSha: Result =
+    VcsUriFragment.validate("abc123def4567890abcdef1234567890abcdef12")
+    Result.success
+
+  def testRejectsEmpty: Result =
+    interceptIllegal("")
+
   def testRejectsAmpersand: Result =
     interceptIllegal("a&b")
 
@@ -37,6 +50,15 @@ object VcsUriFragmentTest extends Properties:
   def testRejectsSemicolon: Result =
     interceptIllegal("a;b")
 
+  def testRejectsSpace: Result =
+    interceptIllegal("a b")
+
+  def testRejectsPercent: Result =
+    interceptIllegal("a%20b")
+
+  def testRejectsGreaterThan: Result =
+    interceptIllegal("a>b")
+
   def testRejectsNewline: Result =
     interceptIllegal("a\nb")
 
