[git-webkit] Provide mechanism to exempt bugs from redaction #12382

JonWBedard · 2023-04-04T16:54:24Z

`e7a1365`

[git-webkit] Provide mechanism to exempt bugs from redaction
https://bugs.webkit.org/show_bug.cgi?id=254993
rdar://107615755

Reviewed by Geoffrey Garen.

It is often the case that we with to (eventually) publish changes which are
redacted when they are written. git-webkit should support a mechanism to mark
bugs (and by extension, the commits associated with those bug) as exempt from
redaction.

* Tools/Scripts/hooks/pre-push: Do not flag class 3 commits if an issue is
exempt from redaction.
* Tools/Scripts/libraries/webkitbugspy/setup.py: Bump version.
* Tools/Scripts/libraries/webkitbugspy/webkitbugspy/__init__.py: Ditto.
* Tools/Scripts/libraries/webkitbugspy/webkitbugspy/bugzilla.py:
(Tracker.__init__): Pass redact_exemption.
* Tools/Scripts/libraries/webkitbugspy/webkitbugspy/github.py:
(Tracker.__init__): Pass redact_exemption.
* Tools/Scripts/libraries/webkitbugspy/webkitbugspy/issue.py:
(Issue.redacted): Check if an issue is exempt from redaction.
* Tools/Scripts/libraries/webkitbugspy/webkitbugspy/radar.py:
(Tracker.__init__): Pass redact_exemption.
* Tools/Scripts/libraries/webkitbugspy/webkitbugspy/tests/bugzilla_unittest.py:
* Tools/Scripts/libraries/webkitbugspy/webkitbugspy/tests/github_unittest.py:
* Tools/Scripts/libraries/webkitbugspy/webkitbugspy/tests/radar_unittest.py:
* Tools/Scripts/libraries/webkitbugspy/webkitbugspy/tracker.py:
(Tracker.Redaction): Support defining a redaction exemption.
(Tracker.from_json): Pass redact_exemption.
(Tracker.__init__): Ditto.
* Tools/Scripts/libraries/webkitscmpy/setup.py: Bump version.
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/__init__.py: Ditto.
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/pull_request.py:
(PullRequest.create_pull_request):
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/test/pull_request_unittest.py:
* metadata/trackers.json: Exempt radars with the 'WebKit Cleared for Publication' keyword.

Canonical link: https://commits.webkit.org/262617@main

c9608ea

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	~~🛠 wincairo~~
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	~~🧪 api-mac~~	✅ 🛠 gtk
✅ 🧪 webkitpy	~~🧪 api-ios~~	~~🧪 mac-wk1~~	~~🧪 gtk-wk2~~
	~~🛠 tv~~	~~🧪 mac-wk2~~	~~🧪 api-gtk~~
	~~🛠 tv-sim~~	~~🧪 mac-AS-debug-wk2~~
✅ 🧪 services	~~🛠 watch~~	~~🧪 mac-wk2-stress~~
	✅ 🛠 watch-sim
✅ 🛠 🧪 unsafe-merge

webkit-early-warning-system · 2023-04-04T16:58:06Z

EWS run on previous version of this PR (hash 2401da9)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🛠 gtk
✅ 🧪 webkitpy	✅ 🧪 api-ios	✅ 🧪 mac-wk1	❌ 🧪 gtk-wk2
	✅ 🛠 tv	✅ 🧪 mac-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv-sim	✅ 🧪 mac-AS-debug-wk2
✅ 🧪 services	✅ 🛠 watch	✅ 🧪 mac-wk2-stress
	✅ 🛠 watch-sim

geoffreygaren

r=me with some suggestions

geoffreygaren · 2023-04-04T17:33:12Z

Tools/Scripts/libraries/webkitbugspy/webkitbugspy/tracker.py

+            if self.exemption:
+                if self.reason:
+                    return '{} and is exempt from redaction'.format(self.reason)
+                return 'is exempt from redaction for an unknown reason'


Why do we want to support the "unknown reason" case? Seems like that should be a hard error.

One potential way to implement this suggestion is to change exemption from a boolean with a separate reason field to a combined exemptionReason field, which is either a string or null. (Or maybe use an enum instead.)

We don't really support the "unknown reason" case, I was just being thorough in exhausting cases. I'll raise an exception in the constructor, in practice, this should be generated from metadata/trackers.json where we define reason in all cases.

geoffreygaren · 2023-04-04T17:38:01Z

metadata/trackers.json

@@ -23,6 +23,7 @@
        "redact" : {
            "classification:Security": true,
            "tentpole:.*Security.*": true
-        }
+        },
+        "redact_exemption" : {"keywords:[^:]*WebKit Cleared for Publication" : true}


If we can tie this keyword to the condition it hopes to express, then we can reduce the chances that someone will apply it blindly just to merge their patch. How about:

WebKit Security Does Not Repro in Shipping

Is there any condition where a Security bug does repro in shipping, but we want to support publishing it anyway?

We would need to use this same mechanism after shipping security fixes too during merge-back, because all merge-back commits will be referencing security commits which did reproduce in the latest shipping release when the bug was first filed.

We can also support multiple keywords here, so could have multiple supported keywords.

webkit-early-warning-system · 2023-04-04T20:18:29Z

EWS run on current version of this PR (hash c9608ea)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	~~🛠 wincairo~~
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	~~🧪 api-mac~~	✅ 🛠 gtk
✅ 🧪 webkitpy	~~🧪 api-ios~~	~~🧪 mac-wk1~~	~~🧪 gtk-wk2~~
	~~🛠 tv~~	~~🧪 mac-wk2~~	~~🧪 api-gtk~~
	~~🛠 tv-sim~~	~~🧪 mac-AS-debug-wk2~~
✅ 🧪 services	~~🛠 watch~~	~~🧪 mac-wk2-stress~~
	✅ 🛠 watch-sim
✅ 🛠 🧪 unsafe-merge

https://bugs.webkit.org/show_bug.cgi?id=254993 rdar://107615755 Reviewed by Geoffrey Garen. It is often the case that we with to (eventually) publish changes which are redacted when they are written. git-webkit should support a mechanism to mark bugs (and by extension, the commits associated with those bug) as exempt from redaction. * Tools/Scripts/hooks/pre-push: Do not flag class 3 commits if an issue is exempt from redaction. * Tools/Scripts/libraries/webkitbugspy/setup.py: Bump version. * Tools/Scripts/libraries/webkitbugspy/webkitbugspy/__init__.py: Ditto. * Tools/Scripts/libraries/webkitbugspy/webkitbugspy/bugzilla.py: (Tracker.__init__): Pass redact_exemption. * Tools/Scripts/libraries/webkitbugspy/webkitbugspy/github.py: (Tracker.__init__): Pass redact_exemption. * Tools/Scripts/libraries/webkitbugspy/webkitbugspy/issue.py: (Issue.redacted): Check if an issue is exempt from redaction. * Tools/Scripts/libraries/webkitbugspy/webkitbugspy/radar.py: (Tracker.__init__): Pass redact_exemption. * Tools/Scripts/libraries/webkitbugspy/webkitbugspy/tests/bugzilla_unittest.py: * Tools/Scripts/libraries/webkitbugspy/webkitbugspy/tests/github_unittest.py: * Tools/Scripts/libraries/webkitbugspy/webkitbugspy/tests/radar_unittest.py: * Tools/Scripts/libraries/webkitbugspy/webkitbugspy/tracker.py: (Tracker.Redaction): Support defining a redaction exemption. (Tracker.from_json): Pass redact_exemption. (Tracker.__init__): Ditto. * Tools/Scripts/libraries/webkitscmpy/setup.py: Bump version. * Tools/Scripts/libraries/webkitscmpy/webkitscmpy/__init__.py: Ditto. * Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/pull_request.py: (PullRequest.create_pull_request): * Tools/Scripts/libraries/webkitscmpy/webkitscmpy/test/pull_request_unittest.py: * metadata/trackers.json: Exempt radars with the 'WebKit Cleared for Publication' keyword. Canonical link: https://commits.webkit.org/262617@main

webkit-commit-queue · 2023-04-05T14:32:33Z

Committed 262617@main (e7a1365): https://commits.webkit.org/262617@main

Reviewed commits have been landed. Closing PR #12382 and removing active labels.

JonWBedard requested a review from facetothefate as a code owner April 4, 2023 16:54

JonWBedard self-assigned this Apr 4, 2023

JonWBedard added the Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases label Apr 4, 2023

JonWBedard requested review from aj062, dewei-zhu and geoffreygaren April 4, 2023 17:03

geoffreygaren approved these changes Apr 4, 2023

View reviewed changes

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Apr 4, 2023

JonWBedard removed the merging-blocked Applied to prevent a change from being merged label Apr 4, 2023

JonWBedard force-pushed the eng/git-webkit-Provide-mechanism-to-exempt-bugs-from-redaction branch from 2401da9 to c9608ea Compare April 4, 2023 20:14

JonWBedard added skip-ews Applied to prevent a change from being run on EWS unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing and removed skip-ews Applied to prevent a change from being run on EWS labels Apr 4, 2023

webkit-commit-queue force-pushed the eng/git-webkit-Provide-mechanism-to-exempt-bugs-from-redaction branch from c9608ea to e7a1365 Compare April 5, 2023 14:32

webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Apr 5, 2023

webkit-commit-queue merged commit e7a1365 into WebKit:main Apr 5, 2023

JonWBedard deleted the eng/git-webkit-Provide-mechanism-to-exempt-bugs-from-redaction branch April 5, 2023 22:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[git-webkit] Provide mechanism to exempt bugs from redaction #12382

[git-webkit] Provide mechanism to exempt bugs from redaction #12382

JonWBedard commented Apr 4, 2023 •

edited by webkit-early-warning-system

webkit-early-warning-system commented Apr 4, 2023 •

edited

geoffreygaren left a comment

geoffreygaren Apr 4, 2023

geoffreygaren Apr 4, 2023

JonWBedard Apr 4, 2023

geoffreygaren Apr 4, 2023

JonWBedard Apr 4, 2023

webkit-early-warning-system commented Apr 4, 2023 •

edited

webkit-commit-queue commented Apr 5, 2023

[git-webkit] Provide mechanism to exempt bugs from redaction #12382

[git-webkit] Provide mechanism to exempt bugs from redaction #12382

Conversation

JonWBedard commented Apr 4, 2023 • edited by webkit-early-warning-system

e7a1365

webkit-early-warning-system commented Apr 4, 2023 • edited

geoffreygaren left a comment

Choose a reason for hiding this comment

geoffreygaren Apr 4, 2023

Choose a reason for hiding this comment

geoffreygaren Apr 4, 2023

Choose a reason for hiding this comment

JonWBedard Apr 4, 2023

Choose a reason for hiding this comment

geoffreygaren Apr 4, 2023

Choose a reason for hiding this comment

JonWBedard Apr 4, 2023

Choose a reason for hiding this comment

webkit-early-warning-system commented Apr 4, 2023 • edited

webkit-commit-queue commented Apr 5, 2023

JonWBedard commented Apr 4, 2023 •

edited by webkit-early-warning-system

`e7a1365`

webkit-early-warning-system commented Apr 4, 2023 •

edited

webkit-early-warning-system commented Apr 4, 2023 •

edited