Refactor PrivacyModule (#752)

* Refactor PrivacyModule; support sensitive text patterns in config

Refactors the PrivacyModule to:
- allow specifying sensitive text patterns in the config instead of
  hardcoding them
- support regex patterns for sensitive file names

Additionally the test class PrivacyModuleTest is refactored to cover more
cases and to reuse test code for email address and sensitive text detection.

* Fail on unknown config entries

This helps detecting mistyped config entries.

Author: Marcono1234

config/config.yml

@@ -176,14 +176,23 @@ arisa:
 
         ----
         Restricted by PrivacyModule ??[~arisabot]??
-      allowedEmailRegex:
-        - '^(mailer-daemon|postmaster|nobody|noreply|no-reply|hostmaster|usenet|news|webmaster|www|abuse|noc|security|info|support|uucp|ftp|news|admin|root)@.*'
-        - '.*@(mojang.com|microsoft.com|minecraft.net|zendesk.com)$'
-      sensitiveFileNames:
-        - launcher_accounts.json
-        - launcher_msa_credentials.json
-        - launcher_profiles.json
-        - .jfr
+      # Important: These email regex patterns must match the email address completely
+      allowedEmailRegexes:
+        - '(mailer-daemon|postmaster|nobody|noreply|no-reply|hostmaster|usenet|news|webmaster|www|abuse|noc|security|info|support|uucp|ftp|news|admin|root)@.*'
+        - '.*@(mojang\.com|microsoft\.com|minecraft\.net|zendesk\.com)'
+      sensitiveTextRegexes:
+        - '\(Session ID is token:'
+        - '--accessToken ey'
+        - '(?<![^\s])(?=[^\s]*[A-Z])(?=[^\s]*[0-9])[A-Z0-9]{17}(?![^\s])'
+        # At the moment braintree transaction IDs seem to have 8 chars, but to be future-proof
+        # match if there are more chars as well
+        - '\bbraintree:[a-f0-9]{6,12}\b'
+      sensitiveFileNameRegexes:
+        - 'launcher_accounts\.json'
+        - 'launcher_msa_credentials\.json'
+        - 'launcher_profiles\.json'
+        # Java Flight Recorder files contain session token in some Minecraft versions
+        - '.*\.jfr'
 
     removeTriagedMeqs:
       resolutions:

docs/Modules.md

@@ -239,13 +239,18 @@ Hides privacy information like Email addresses in tickets or comments.
 ### Checks
 - The ticket is not set to private.
 #### For Setting Tickets to Private
-- Any of the fields and/or text attachments added after last run contains session ID or Email.
-- Or any of the attachments has a name specified by `sensitiveFileNames` in the [config](../config/config.yml)
-  (defaults to empty list)
+- Any of the following matches one of the regex patterns specified by `sensitiveTextRegexes` in the
+  [config](../config/config.yml), or contains an email address which does not match one of the `allowedEmailRegexes`:
+  - summary, environment, description (if ticket was created after last run)
+  - text attachment (if it was created after last run)
+  - changelog entry string value (if it was created after last run)
+- Or any of the attachments created after the last run has a name which matches one of `sensitiveFileNameRegexes` in
+  the [config](../config/config.yml).
 #### For Restricting Comments to `staff`
 - The comment was added after last run.
 - The comment is not restricted.
-- The comment contains session ID or Email.
+- The comment matches one of the regex patterns specified by `sensitiveTextRegexes` in the
+  [config](../config/config.yml), or contains an email address which does not match one of the `allowedEmailRegexes`.
 
 ## PrivateDuplicate
 | Entry | Value                                                                               |

src/main/kotlin/io/github/mojira/arisa/ConfigService.kt

@@ -1,13 +1,17 @@
 package io.github.mojira.arisa
 
 import com.uchuhimo.konf.Config
+import com.uchuhimo.konf.Feature
 import com.uchuhimo.konf.source.yaml
 import io.github.mojira.arisa.infrastructure.config.Arisa
 
 class ConfigService {
     val config: Config = Config { addSpec(Arisa) }
-        .from.yaml.watchFile("config/config.yml")
-        .from.yaml.watchFile("config/local.yml")
+        // Only enable strict config parsing for YAML files; environment variables and system properties
+        // likely contain entries completely unrelated to Arisa
+        .from.enabled(Feature.FAIL_ON_UNKNOWN_PATH).yaml.watchFile("config/config.yml")
+        .from.enabled(Feature.FAIL_ON_UNKNOWN_PATH).yaml.watchFile("config/local.yml")
         .from.env()
         .from.systemProperties()
+        .validateRequired()
 }

src/main/kotlin/io/github/mojira/arisa/infrastructure/config/ArisaConfig.kt

@@ -160,13 +160,21 @@ object Arisa : ConfigSpec() {
                 "",
                 description = "The text which will be appended at the comments that are restricted by this module."
             )
-            val allowedEmailRegex by optional<List<String>>(
+            val allowedEmailRegexes by optional<List<String>>(
                 default = emptyList(),
-                description = "List of regex for allowed emails"
+                description = "Regex patterns matching allowed email addresses, that is, email addresses which may" +
+                    " be public. The patterns must match the complete email address; partial matches won't have any" +
+                    " effect."
             )
-            val sensitiveFileNames by optional<List<String>>(
+            val sensitiveTextRegexes by optional<List<String>>(
                 default = emptyList(),
-                description = "Names of attachment files containing sensitive information"
+                description = "Regex patterns matching sensitive text. The patterns do not have to match the" +
+                    " complete text; partial matches will be detected as well."
+            )
+            val sensitiveFileNameRegexes by optional<List<String>>(
+                default = emptyList(),
+                description = "Regex patterns matching names of attachment files containing sensitive information." +
+                    " The patterns must match the complete file name; partial matches won't have any effect."
             )
         }
 

src/main/kotlin/io/github/mojira/arisa/modules/PrivacyModule.kt

@@ -7,21 +7,16 @@ import io.github.mojira.arisa.domain.CommentOptions
 import io.github.mojira.arisa.domain.Issue
 import java.time.Instant
 
+private fun Iterable<Regex>.anyMatches(string: String) = any { it.matches(string) }
+
 class PrivacyModule(
     private val message: String,
     private val commentNote: String,
-    private val allowedEmailsRegex: List<Regex>,
-    private val sensitiveFileNames: List<String>
+    private val allowedEmailRegexes: List<Regex>,
+    private val sensitiveTextRegexes: List<Regex>,
+    private val sensitiveFileNameRegexes: List<Regex>
 ) : Module {
-    private val patterns: List<Regex> = listOf(
-        """.*\(Session ID is token:.*""".toRegex(),
-        """.*--accessToken ey.*""".toRegex(),
-        """.*(?<![^\s])(?=[^\s]*[A-Z])(?=[^\s]*[0-9])[A-Z0-9]{17}(?![^\s]).*""".toRegex(),
-        // At the moment braintree transaction IDs seem to have 8 chars, but to be future-proof
-        // match if there are more chars as well
-        """.*\bbraintree:[a-f0-9]{6,12}\b.*""".toRegex()
-    )
-
+    // Matches an email address, which is not part of a user mention ([~name])
     private val emailRegex = "(?<!\\[~)\\b[a-zA-Z0-9.\\-_]+@[a-zA-Z.\\-_]+\\.[a-zA-Z.\\-]{2,15}\\b".toRegex()
 
     override fun invoke(issue: Issue, lastRun: Instant): Either<ModuleError, ModuleResponse> = with(issue) {
@@ -34,9 +29,9 @@ class PrivacyModule(
                 string += "$summary $environment $description "
             }
 
-            attachments
+            val newAttachments = attachments.filter { it.created.isAfter(lastRun) }
+            newAttachments
                 .asSequence()
-                .filter { it.created.isAfter(lastRun) }
                 .filter { it.mimeType.startsWith("text/") }
                 .forEach { string += "${String(it.getContent())} " }
 
@@ -46,19 +41,19 @@ class PrivacyModule(
                 .filter { it.changedFromString == null }
                 .forEach { string += "${it.changedToString} " }
 
-            val doesStringMatchPatterns = string.matches(patterns)
+            val doesStringMatchPatterns = string.containsMatch(sensitiveTextRegexes)
             val doesEmailMatches = matchesEmail(string)
 
-            val doesAttachmentNameMatch = attachments
+            val doesAttachmentNameMatch = newAttachments
                 .asSequence()
                 .map(Attachment::name)
-                .any(sensitiveFileNames::contains)
+                .any(sensitiveFileNameRegexes::anyMatches)
 
             val restrictCommentFunctions = comments
                 .asSequence()
                 .filter { it.created.isAfter(lastRun) }
                 .filter { it.visibilityType == null }
-                .filter { it.body?.matches(patterns) ?: false || matchesEmail(it.body ?: "") }
+                .filter { it.body?.containsMatch(sensitiveTextRegexes) ?: false || matchesEmail(it.body ?: "") }
                 .filterNot {
                     it.getAuthorGroups()?.any { group ->
                         listOf("helper", "global-moderators", "staff").contains(group)
@@ -86,9 +81,10 @@ class PrivacyModule(
     private fun matchesEmail(string: String): Boolean {
         return emailRegex
             .findAll(string)
-            .filterNot { email -> allowedEmailsRegex.any { regex -> regex.matches(email.value) } }
+            .map(MatchResult::value)
+            .filterNot(allowedEmailRegexes::anyMatches)
             .any()
     }
 
-    private fun String.matches(patterns: List<Regex>) = patterns.any { it.containsMatchIn(this) }
+    private fun String.containsMatch(patterns: List<Regex>) = patterns.any { it.containsMatchIn(this) }
 }

src/main/kotlin/io/github/mojira/arisa/registry/InstantModuleRegistry.kt

@@ -156,8 +156,9 @@ class InstantModuleRegistry(config: Config) : ModuleRegistry(config) {
             PrivacyModule(
                 config[Arisa.Modules.Privacy.message],
                 config[Arisa.Modules.Privacy.commentNote],
-                config[Arisa.Modules.Privacy.allowedEmailRegex].map(String::toRegex),
-                config[Arisa.Modules.Privacy.sensitiveFileNames]
+                config[Arisa.Modules.Privacy.allowedEmailRegexes].map(String::toRegex),
+                config[Arisa.Modules.Privacy.sensitiveTextRegexes].map(String::toRegex),
+                config[Arisa.Modules.Privacy.sensitiveFileNameRegexes].map(String::toRegex)
             )
         )
 

src/test/kotlin/io/github/mojira/arisa/infrastructure/IntegrationTest.kt

@@ -1,6 +1,7 @@
 package io.github.mojira.arisa.infrastructure
 
 import com.uchuhimo.konf.Config
+import com.uchuhimo.konf.Feature
 import com.uchuhimo.konf.source.yaml
 import io.github.mojira.arisa.infrastructure.config.Arisa
 import io.kotest.core.spec.style.StringSpec
@@ -10,14 +11,16 @@ import java.io.File
 class IntegrationTest : StringSpec({
     "should be able to read the main config file correctly" {
         val config = Config { addSpec(Arisa) }
-            .from.map.flat(
+            // Only enable strict config parsing for map and YAML files; environment variables and system properties
+            // likely contain entries completely unrelated to Arisa
+            .from.enabled(Feature.FAIL_ON_UNKNOWN_PATH).map.flat(
                 mapOf(
                     "arisa.credentials.username" to "test",
                     "arisa.credentials.password" to "test",
                     "arisa.credentials.dandelionToken" to "test"
                 )
             )
-            .from.yaml.watchFile("config/config.yml")
+            .from.enabled(Feature.FAIL_ON_UNKNOWN_PATH).yaml.watchFile("config/config.yml")
             .from.env()
             .from.systemProperties()