regression_set_status_flags: avoid leaking regressing bug number if i… (

#999) * Test fix Not sure how this ever worked, we expect flags for esr to exist. * regression_set_status_flags: avoid leaking regressing bug number if it's private
mozilla · Jul 31, 2020 · e24ad54 · e24ad54
1 parent cfaeced
commit e24ad54
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 5 deletions.
diff --git a/auto_nag/scripts/regression_set_status_flags.py b/auto_nag/scripts/regression_set_status_flags.py
@@ -143,8 +143,13 @@ def get_status_changes(self, bugs):
                 filtered_bugs[bugid] = info
 
         for bugid in filtered_bugs:
+            regressor = bugs[bugid]["regressed_by"]
             self.status_changes[bugid]["comment"] = {
-                "body": f'{self.description()} {bugs[bugid]["regressed_by"]}'
+                "body": f"{self.description()} {regressor}",
+                # if the regressing bug is private (security or otherwise
+                # confidential), don't leak its number through our comment (the
+                # regressed_by field is not public in that case)
+                "is_private": bool(data[regressor].get("groups")),
             }
         return filtered_bugs
 

diff --git a/auto_nag/tests/test_regression_set_status_flags.py b/auto_nag/tests/test_regression_set_status_flags.py
@@ -22,7 +22,9 @@ def mock_get_checked_versions():
 def mock_get_bugs(self, *args, **kwargs):
     return {
         "1111": {
-            "id": 100,
+            "id": 1111,
+            "cf_status_firefox_esr2": "---",
+            "cf_status_firefox_esr3": "---",
             "cf_status_firefox2": "---",
             "cf_status_firefox3": "affected",
             "cf_status_firefox4": "fixed",
@@ -35,18 +37,33 @@ def mock_get_bugs(self, *args, **kwargs):
             "cf_status_firefox4": "---",
             "regressed_by": 222,
         },
+        "3333": {
+            "id": 3333,
+            "cf_status_firefox_esr2": "---",
+            "cf_status_firefox_esr3": "---",
+            "cf_status_firefox2": "---",
+            "cf_status_firefox3": "affected",
+            "cf_status_firefox4": "fixed",
+            "regressed_by": 333,
+        },
     }
 
 
 def mock_get_flags_from_regressing_bugs(self, bugids):
-    assert sorted(bugids) == [111, 222]
+    assert sorted(bugids) == [111, 222, 333]
     return {
         111: {
             "id": 111,
             "cf_status_firefox_esr3": "fixed",
             "cf_status_firefox3": "fixed",
         },
         222: {"id": 222, "cf_status_firefox1": "fixed",},
+        333: {
+            "id": 333,
+            "cf_status_firefox_esr3": "fixed",
+            "cf_status_firefox3": "fixed",
+            "groups": ["core-security-release"],
+        },
     }
 
 
@@ -74,8 +91,8 @@ def test_status_changes(self):
         r = RegressionSetStatusFlags()
         bugs = r.get_bugs()
         # 2222 is left unchanged because it regressed too long ago
-        self.assertEqual(sorted(bugs), ["1111"])
-        self.assertEqual(list(r.status_changes), ["1111"])
+        self.assertEqual(sorted(bugs), ["1111", "3333"])
+        self.assertEqual(list(r.status_changes), ["1111", "3333"])
         self.assertEqual(
             sorted(r.status_changes["1111"]),
             [
@@ -90,3 +107,5 @@ def test_status_changes(self):
             r.status_changes["1111"]["cf_status_firefox_esr2"], "unaffected"
         )
         self.assertEqual(r.status_changes["1111"]["cf_status_firefox_esr3"], "affected")
+        self.assertFalse(r.status_changes["1111"]["comment"]["is_private"])
+        self.assertTrue(r.status_changes["3333"]["comment"]["is_private"])