Add validation for reserved characters in FeatureType names and categories (#1991)

Copilot · vsct-jburet · web-flow · commit f75d7417fc7d · 2026-02-03T21:45:27.000+01:00
* Initial plan

* Add validation for forbidden characters in feature names and categories

Co-authored-by: vsct-jburet &lt;12413454+vsct-jburet@users.noreply.github.com&gt;

* Add validation tests for enable and disable methods

Co-authored-by: vsct-jburet &lt;12413454+vsct-jburet@users.noreply.github.com&gt;

* Simplify validation tests as requested

Co-authored-by: vsct-jburet &lt;12413454+vsct-jburet@users.noreply.github.com&gt;

* Add missing comma validation test for category

Co-authored-by: vsct-jburet &lt;12413454+vsct-jburet@users.noreply.github.com&gt;

---------

Co-authored-by: copilot-swe-agent[bot] &lt;198982749+Copilot@users.noreply.github.com&gt;
Co-authored-by: vsct-jburet &lt;12413454+vsct-jburet@users.noreply.github.com&gt;
diff --git a/bot/engine/src/main/kotlin/engine/feature/FeatureType.kt b/bot/engine/src/main/kotlin/engine/feature/FeatureType.kt
@@ -19,6 +19,13 @@ package ai.tock.bot.engine.feature
 /**
  * A feature with a name and a category.
  * Usually implemented by an enum.
+ *
+ * **Important:** Feature names and categories must not contain the following characters:
+ * - `+` (plus sign) - reserved for internal use as applicationId separator
+ * - `,` (comma) - reserved for internal use as category/name separator
+ *
+ * Attempting to use these characters will result in an [IllegalArgumentException]
+ * when the feature is added or modified.
  */
 interface FeatureType {
     val name: String
diff --git a/bot/engine/src/main/kotlin/engine/feature/FeatureValidation.kt b/bot/engine/src/main/kotlin/engine/feature/FeatureValidation.kt
@@ -0,0 +1,57 @@
+/*
+ * Copyright (C) 2017/2025 SNCF Connect & Tech
+ *
+ * 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
+ *
+ * http://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 ai.tock.bot.engine.feature
+
+/**
+ * Characters that are forbidden in feature names and categories.
+ * These characters are used internally as separators in the storage layer:
+ * - '+' is used to separate the applicationId from the feature identifier
+ * - ',' is used to separate the category from the name
+ */
+private val FORBIDDEN_CHARACTERS = setOf('+', ',')
+
+/**
+ * Validates that a feature name does not contain forbidden characters.
+ *
+ * @param name the feature name to validate
+ * @throws IllegalArgumentException if the name contains forbidden characters
+ */
+fun validateFeatureName(name: String) {
+    val invalidChars = name.filter { it in FORBIDDEN_CHARACTERS }
+    if (invalidChars.isNotEmpty()) {
+        throw IllegalArgumentException(
+            "Feature name must not contain the following characters: ${FORBIDDEN_CHARACTERS.joinToString(", ")}. " +
+                "Found: ${invalidChars.toSet().joinToString(", ")}",
+        )
+    }
+}
+
+/**
+ * Validates that a feature category does not contain forbidden characters.
+ *
+ * @param category the feature category to validate
+ * @throws IllegalArgumentException if the category contains forbidden characters
+ */
+fun validateFeatureCategory(category: String) {
+    val invalidChars = category.filter { it in FORBIDDEN_CHARACTERS }
+    if (invalidChars.isNotEmpty()) {
+        throw IllegalArgumentException(
+            "Feature category must not contain the following characters: ${FORBIDDEN_CHARACTERS.joinToString(", ")}. " +
+                "Found: ${invalidChars.toSet().joinToString(", ")}",
+        )
+    }
+}
diff --git a/bot/engine/src/test/kotlin/engine/feature/FeatureValidationTest.kt b/bot/engine/src/test/kotlin/engine/feature/FeatureValidationTest.kt
@@ -0,0 +1,57 @@
+/*
+ * Copyright (C) 2017/2025 SNCF Connect & Tech
+ *
+ * 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
+ *
+ * http://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 ai.tock.bot.engine.feature
+
+import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.assertThrows
+
+class FeatureValidationTest {
+    @Test
+    fun `validateFeatureName accepts valid names`() {
+        // Should not throw
+        validateFeatureName("validName")
+        validateFeatureName("valid_name")
+        validateFeatureName("valid-name")
+    }
+
+    @Test
+    fun `validateFeatureName rejects forbidden characters`() {
+        assertThrows<IllegalArgumentException> {
+            validateFeatureName("invalid+name")
+        }
+        assertThrows<IllegalArgumentException> {
+            validateFeatureName("invalid,name")
+        }
+    }
+
+    @Test
+    fun `validateFeatureCategory accepts valid categories`() {
+        // Should not throw
+        validateFeatureCategory("valid.category")
+        validateFeatureCategory("ai.tock.bot.Feature")
+    }
+
+    @Test
+    fun `validateFeatureCategory rejects forbidden characters`() {
+        assertThrows<IllegalArgumentException> {
+            validateFeatureCategory("invalid+category")
+        }
+        assertThrows<IllegalArgumentException> {
+            validateFeatureCategory("invalid,category")
+        }
+    }
+}
diff --git a/bot/storage-mongo/src/main/kotlin/FeatureMongoDAO.kt b/bot/storage-mongo/src/main/kotlin/FeatureMongoDAO.kt
@@ -18,6 +18,8 @@ package ai.tock.bot.mongo
 
 import ai.tock.bot.engine.feature.FeatureDAO
 import ai.tock.bot.engine.feature.FeatureState
+import ai.tock.bot.engine.feature.validateFeatureCategory
+import ai.tock.bot.engine.feature.validateFeatureName
 import ai.tock.bot.mongo.Feature_.Companion.BotId
 import ai.tock.bot.mongo.Feature_.Companion.Namespace
 import ai.tock.bot.mongo.Feature_.Companion._id
@@ -160,6 +162,8 @@ internal class FeatureMongoDAO(
         applicationId: String?,
         graduation: Int?,
     ) {
+        validateFeatureCategory(category)
+        validateFeatureName(name)
         val id = calculateId(botId, namespace, category, name, applicationId)
         val feature = Feature(id, "$category,$name", true, botId, namespace, startDate, endDate, graduation)
 
@@ -173,6 +177,8 @@ internal class FeatureMongoDAO(
         name: String,
         applicationId: String?,
     ) {
+        validateFeatureCategory(category)
+        validateFeatureName(name)
         val id = calculateId(botId, namespace, category, name, applicationId)
         val feature = Feature(id, "$category,$name", false, botId, namespace)
         col.save(feature)
@@ -209,6 +215,8 @@ internal class FeatureMongoDAO(
         applicationId: String?,
         graduation: Int?,
     ) {
+        validateFeatureCategory(category)
+        validateFeatureName(name)
         val id = calculateId(botId, namespace, category, name, applicationId)
         val feature = Feature(id, "$category,$name", enabled, botId, namespace, startDate, endDate, graduation)
         col.save(feature)
diff --git a/bot/storage-mongo/src/test/kotlin/FeatureMongoDAOTest.kt b/bot/storage-mongo/src/test/kotlin/FeatureMongoDAOTest.kt
@@ -38,6 +38,7 @@ import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.DisplayName
 import org.junit.jupiter.api.Nested
 import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.assertThrows
 import org.litote.kmongo.coroutine.CoroutineCollection
 import org.litote.kmongo.coroutine.CoroutineFindPublisher
 import org.litote.kmongo.coroutine.coroutine
@@ -314,6 +315,30 @@ internal class FeatureMongoDAOTest {
                     `assert that feature is persisted with`(id, false)
                 }
             }
+
+            @Test
+            fun `addFeature rejects forbidden characters in name`() {
+                runBlocking {
+                    assertThrows<IllegalArgumentException> {
+                        featureDAO.addFeature(botId, namespace, true, "category", "name+invalid", null, null)
+                    }
+                    assertThrows<IllegalArgumentException> {
+                        featureDAO.addFeature(botId, namespace, true, "category", "name,invalid", null, null)
+                    }
+                }
+            }
+
+            @Test
+            fun `addFeature rejects forbidden characters in category`() {
+                runBlocking {
+                    assertThrows<IllegalArgumentException> {
+                        featureDAO.addFeature(botId, namespace, true, "category+invalid", "name", null, null)
+                    }
+                    assertThrows<IllegalArgumentException> {
+                        featureDAO.addFeature(botId, namespace, true, "category,invalid", "name", null, null)
+                    }
+                }
+            }
         }
 
         @Test
