fix(permissions): stop wildcards from granting negative permissions

common/src/main/kotlin/gg/grounds/permissions/PermissionChecker.kt

@@ -64,16 +64,23 @@ class InMemoryPermissionSnapshots(initialSnapshots: Map<UUID, PermissionSnapshot
     }
 }
 
+/**
+ * [negativePermissions] are the keys the loaded plugins declared as inverted — holding one takes
+ * something away instead of granting it. They are never handed out by a wildcard: an operator with
+ * `ALLOW *` means "give this player everything", not "mute them".
+ */
 class SnapshotPermissions(
     private val snapshots: InMemoryPermissionSnapshots,
     private val defaultScope: PermissionCheckScope = PermissionCheckScope.global(),
     private val clock: Clock = Clock.systemUTC(),
+    private val negativePermissions: Set<String> = emptySet(),
 ) : Permissions {
     constructor(
         snapshots: Map<UUID, PermissionSnapshot>,
         defaultScope: PermissionCheckScope = PermissionCheckScope.global(),
         clock: Clock = Clock.systemUTC(),
-    ) : this(InMemoryPermissionSnapshots(snapshots), defaultScope, clock)
+        negativePermissions: Set<String> = emptySet(),
+    ) : this(InMemoryPermissionSnapshots(snapshots), defaultScope, clock, negativePermissions)
 
     override fun hasPermission(playerId: UUID, permission: String): Boolean {
         return hasPermission(playerId, permission, defaultScope)
@@ -100,11 +107,13 @@ class SnapshotPermissions(
             return false
         }
 
+        val negative = permission in negativePermissions
+
         val candidate =
             (snapshot.allowPatterns + snapshot.denyPatterns)
                 .asSequence()
                 .filterNot { it.isExpired(now) }
-                .filter { PermissionPattern.matches(it.pattern, permission) }
+                .filter { PermissionPattern.matches(it.pattern, permission, negative) }
                 .mapNotNull { grant -> grant.toCandidate(scope) }
                 .maxWithOrNull(
                     compareBy<PermissionCandidate> { it.scopeSpecificity }
@@ -147,15 +156,24 @@ class SnapshotPermissions(
 }
 
 private object PermissionPattern {
-    fun matches(pattern: String, permission: String): Boolean =
-        when {
+    fun matches(pattern: String, permission: String, negative: Boolean = false): Boolean {
+        if (pattern == permission) {
+            return true
+        }
+        // A wildcard says "everything", which is never meant to include a permission that takes
+        // something away. Those have to be granted by name.
+        if (negative) {
+            return false
+        }
+        return when {
             pattern == "*" -> true
             pattern.endsWith(".*") -> {
                 val prefix = pattern.removeSuffix("*")
                 permission.startsWith(prefix) && permission.length > prefix.length
             }
-            else -> pattern == permission
+            else -> false
         }
+    }
 
     fun specificity(pattern: String): Int =
         when {

common/src/main/kotlin/gg/grounds/permissions/catalog/PermissionManifest.kt

@@ -60,6 +60,15 @@ data class PermissionManifestEntry(
     val label: String,
     val description: String,
     val supportedScopes: List<PermissionManifestScope>,
+    /**
+     * Marks a permission whose meaning is inverted: holding it takes something away rather than
+     * granting it (a mute, a ban from a channel).
+     *
+     * A wildcard grant means "give this player everything", which must never be read as "mute
+     * them". Negative permissions are therefore only ever matched by an exact grant — see
+     * `SnapshotPermissions`.
+     */
+    val negative: Boolean = false,
 )
 
 enum class PermissionManifestScope {

common/src/main/kotlin/gg/grounds/permissions/catalog/PermissionManifestCollector.kt

@@ -9,7 +9,21 @@ data class PermissionManifestCollectionFailure(val origin: ManifestOrigin, val r
 data class PermissionManifestCollection(
     val manifests: List<CollectedPermissionManifest>,
     val failures: List<PermissionManifestCollectionFailure>,
-)
+) {
+    /**
+     * The permission keys the loaded plugins declared as negative.
+     *
+     * Only plugins running in this JVM can be asked about a permission, so their own manifests are
+     * the complete authority here — no catalog round-trip needed.
+     */
+    fun negativePermissionKeys(): Set<String> =
+        manifests
+            .asSequence()
+            .flatMap { it.manifest.permissions.asSequence() }
+            .filter { it.negative }
+            .map { it.key }
+            .toSet()
+}
 
 class PermissionManifestCollector {
     fun collect(origins: Iterable<ManifestOrigin>): PermissionManifestCollection {

common/src/test/kotlin/gg/grounds/permissions/PermissionCheckerTest.kt

@@ -201,15 +201,69 @@ class PermissionCheckerTest {
         assertFalse(permissions.hasPermission(playerId, "chat.send"))
     }
 
+    // An operator holds ALLOW *. That says "give this player everything" — it must not hand them
+    // grounds.chat.muted, which takes chat away. This muted every admin on the network.
+    @Test
+    fun globalWildcardDoesNotGrantANegativePermission() {
+        val permissions =
+            permissions(
+                allowPatterns = listOf(allow("*")),
+                negativePermissions = setOf("grounds.chat.muted"),
+            )
+
+        assertFalse(permissions.hasPermission(playerId, "grounds.chat.muted"))
+        assertTrue(permissions.hasPermission(playerId, "grounds.chat.staff"))
+    }
+
+    @Test
+    fun prefixWildcardDoesNotGrantANegativePermission() {
+        val permissions =
+            permissions(
+                allowPatterns = listOf(allow("grounds.chat.*")),
+                negativePermissions = setOf("grounds.chat.muted"),
+            )
+
+        assertFalse(permissions.hasPermission(playerId, "grounds.chat.muted"))
+        assertTrue(permissions.hasPermission(playerId, "grounds.chat.staff"))
+    }
+
+    // Muting somebody still has to work — by name.
+    @Test
+    fun exactGrantStillAppliesANegativePermission() {
+        val permissions =
+            permissions(
+                allowPatterns = listOf(allow("grounds.chat.muted")),
+                negativePermissions = setOf("grounds.chat.muted"),
+            )
+
+        assertTrue(permissions.hasPermission(playerId, "grounds.chat.muted"))
+    }
+
+    // A wildcard cannot grant it, so a DENY next to it has nothing to overrule — but an exact
+    // grant that was later revoked must still lose.
+    @Test
+    fun denyBeatsAnExactGrantOfANegativePermission() {
+        val permissions =
+            permissions(
+                allowPatterns = listOf(allow("grounds.chat.muted")),
+                denyPatterns = listOf(deny("grounds.chat.muted")),
+                negativePermissions = setOf("grounds.chat.muted"),
+            )
+
+        assertFalse(permissions.hasPermission(playerId, "grounds.chat.muted"))
+    }
+
     private fun permissions(
         allowPatterns: List<PermissionGrant> = emptyList(),
         denyPatterns: List<PermissionGrant> = emptyList(),
         scope: PermissionCheckScope = PermissionCheckScope.global(),
+        negativePermissions: Set<String> = emptySet(),
     ): Permissions =
         SnapshotPermissions(
             mapOf(playerId to snapshot(allowPatterns = allowPatterns, denyPatterns = denyPatterns)),
             defaultScope = scope,
             clock = clock,
+            negativePermissions = negativePermissions,
         )
 
     private fun snapshot(

minestom/src/main/kotlin/gg/grounds/permissions/minestom/GroundsPermissionsModule.kt

@@ -7,6 +7,7 @@ import gg.grounds.permissions.PermissionSnapshotRefreshSweep
 import gg.grounds.permissions.Permissions
 import gg.grounds.permissions.SnapshotPermissions
 import gg.grounds.permissions.catalog.CollectedPermissionManifest
+import gg.grounds.permissions.catalog.PermissionManifestCollection
 import gg.grounds.runtime.GroundsModule
 import gg.grounds.runtime.GroundsServerContext
 import java.time.Clock
@@ -46,11 +47,17 @@ class GroundsPermissionsModule(private val clock: Clock = Clock.systemUTC()) : G
             Executors.newSingleThreadExecutor { runnable ->
                 Thread(runnable, "grounds-permissions-manifest-catalog").apply { isDaemon = true }
             }
+        // Collected before the checker is built: it has to know which permissions are negative
+        // before it answers its first question. Registration with the catalog service is the slow
+        // part and stays off-thread below.
+        val manifests = collectActivePermissionManifests(ctx.activeModuleProviders)
+
         val permissions =
             SnapshotPermissions(
                 snapshots = snapshots,
                 defaultScope = config.context.toCheckScope(),
                 clock = clock,
+                negativePermissions = manifests.negativePermissionKeys(),
             )
         val loader =
             MinestomPermissionSnapshotLoader(
@@ -105,7 +112,7 @@ class GroundsPermissionsModule(private val clock: Clock = Clock.systemUTC()) : G
         ctx.onShutdown { stop() }
 
         registerActivePermissionManifests(
-            activeProviders = ctx.activeModuleProviders,
+            manifests = manifests,
             manifestClient = manifestClient,
             manifestExecutor = manifestExecutor,
             context = config.context,
@@ -133,15 +140,14 @@ class GroundsPermissionsModule(private val clock: Clock = Clock.systemUTC()) : G
     }
 
     private fun registerActivePermissionManifests(
-        activeProviders: Iterable<gg.grounds.runtime.ActiveGroundsModuleProvider>,
+        manifests: PermissionManifestCollection,
         manifestClient: PermissionCatalogClient,
         manifestExecutor: ExecutorService,
         context: PermissionSnapshotContext,
     ) {
         try {
             manifestExecutor.execute {
-                val collection = collectActivePermissionManifests(activeProviders)
-                collection.failures.forEach { failure ->
+                manifests.failures.forEach { failure ->
                     logger.warn(
                         "Skipped malformed permission manifest (originId={}, originVersion={}, reason={})",
                         failure.origin.id,
@@ -151,7 +157,7 @@ class GroundsPermissionsModule(private val clock: Clock = Clock.systemUTC()) : G
                 }
                 val registration =
                     MinestomPermissionManifestRegistrar(manifestClient, context)
-                        .register(collection.manifests)
+                        .register(manifests.manifests)
                 registration.registered.forEach(::logManifestRegistration)
                 registration.failures.forEach { failure ->
                     logger.warn(

velocity/src/main/kotlin/gg/grounds/permissions/velocity/GroundsPermissionsPlugin.kt

@@ -17,6 +17,7 @@ import gg.grounds.permissions.PermissionSnapshotRefreshSweep
 import gg.grounds.permissions.Permissions
 import gg.grounds.permissions.SnapshotPermissions
 import gg.grounds.permissions.catalog.PermissionManifest
+import gg.grounds.permissions.catalog.PermissionManifestCollection
 import gg.grounds.permissions.catalog.PermissionManifestCollector
 import io.grpc.LoadBalancerRegistry
 import io.grpc.NameResolverRegistry
@@ -101,8 +102,17 @@ constructor(
                 .repeat(config.refreshIntervalSeconds, TimeUnit.SECONDS)
                 .schedule()
 
+        // Collected before the checker is built: it has to know which permissions are negative
+        // before it answers its first question. Registration with the catalog service is the slow
+        // part and stays on the scheduler below.
+        val manifests = collectPermissionManifests()
+
         val permissions =
-            SnapshotPermissions(snapshots, defaultScope = config.context.toCheckScope())
+            SnapshotPermissions(
+                snapshots,
+                defaultScope = config.context.toCheckScope(),
+                negativePermissions = manifests.negativePermissionKeys(),
+            )
         this.permissions = permissions
         loadCommandPermissions()?.let { commandPermissions ->
             val router =
@@ -154,7 +164,10 @@ constructor(
         }
 
         proxy.scheduler
-            .buildTask(this, Runnable { registerActivePermissionManifests(manifestClient, config) })
+            .buildTask(
+                this,
+                Runnable { registerActivePermissionManifests(manifestClient, config, manifests) },
+            )
             .schedule()
 
         logger.info(
@@ -201,10 +214,7 @@ constructor(
             null
         }
 
-    private fun registerActivePermissionManifests(
-        manifestClient: PermissionCatalogClient,
-        config: VelocityPermissionsConfig,
-    ) {
+    private fun collectPermissionManifests(): PermissionManifestCollection {
         val collection =
             PermissionManifestCollector()
                 .collect(discoverPermissionManifestOrigins(proxy.pluginManager.plugins))
@@ -216,6 +226,14 @@ constructor(
                 failure.reason,
             )
         }
+        return collection
+    }
+
+    private fun registerActivePermissionManifests(
+        manifestClient: PermissionCatalogClient,
+        config: VelocityPermissionsConfig,
+        collection: PermissionManifestCollection,
+    ) {
         val registration =
             PermissionManifestRegistrar(manifestClient, config.context)
                 .register(collection.manifests)