1417229 - Enable Triage Lead on a component to see security bugs in that component

Status: RESOLVED FIXED

(bugzilla.mozilla.org :: General, enhancement)

Description

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

Per discussion with wleung and dveditz we want triage leads in a component to be able to see security bugs in that component. 

Two ways to proceed:

1. Add that capability to the Triage Owner field.
2. Re-purpose the QA Contact field, making it the Triage Owner (and removing the existing triage owner field.)

Also allow the Triage Owner field to be optionally excluded from seeing a bug in a security group.

Comment 1

[Dylan Hardison [:dylan] (he/him)]

A third option is to always consider the triage lead to be CC'd.


But I think QA contact and Triage lead are very confuseable. Maybe they should be unified?

Comment 2

[Daniel Veditz [:dveditz]]

There are current behavioral differences: QA Contact is an editable field, and in the current UI the Triage Owner cannot be changed. On the other hand I believe the QA Contact already has the security access we want. If the field remains editable then access can be revoked by taking the person out of the field. If we stick with the original "Triage Owner" request then we'd have to make its access follow the same checkbox used for CC folks.

If the simplest hack is to just auto-CC the triage owner on every new bug I'm all for that.

Comment 3

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

The Triage Owner field, we've learned is pretty volatile. If we CC the triage owner on bugs, we'd need to remove them when we change it. Also since bugs change components, we'd have some security bugs without the triage owner being CCed. 

If I understand correctly, a new bug gets it's QA contact from the default for the component, but that does not change if the default QA Contact changes. I've had to clean up a few bugs due to departures because of that. 

What I'm looking for is that the triage owner updates for all bugs in the component when I change it, and the triage owner is part of a group who is automatically included into the security group. 

The piece we need is a dynamically defined group, which I've asked for in another bug which I can't find at the moment.

Comment 4

[Daniel Veditz [:dveditz]]

(In reply to Emma Humphries, Bugmaster ☕️🎸🧞‍♀️✨ (she/her) [:emceeaich] (UTC-8) needinfo? me from comment #3)
> The piece we need is a dynamically defined group, which I've asked for in
> another bug which I can't find at the moment.

We manage to give automatic access to the assignee, reporter, QA contact, and CCs without dynamically defined groups. Why can't we treat the Triage Owner like that?

Comment 5

[Dylan Hardison [:dylan] (he/him)]

(In reply to Daniel Veditz [:dveditz] from comment #4)
> (In reply to Emma Humphries, Bugmaster ☕️🎸🧞‍♀️✨ (she/her) [:emceeaich]
> (UTC-8) needinfo? me from comment #3)
> > The piece we need is a dynamically defined group, which I've asked for in
> > another bug which I can't find at the moment.
> 
> We manage to give automatic access to the assignee, reporter, QA contact,
> and CCs without dynamically defined groups. Why can't we treat the Triage
> Owner like that?

Mostly because those checks are not centralized in the code, and there is greater risk changing them.

Comment 6

[Daniel Veditz [:dveditz]]

Dylan mentioned over IRC last week that a possibility is to change the handful of places that treat QA Contact special so that is uses Triage Owner ==instead==. Adding a new check is harder, but finding the already-identified QA Contact places is straigtforward.

Comment 9

[David Lawrence [:dkl]]

Attachment: GitHub Pull Request

Comment 10

[David Lawrence [:dkl]]

I went with the approach that a triage owner can see a secure bug the same as a reporter, qa contact, assignee or cc member can see the bug so this will happen automatically for each bug. Instead of adding the triage owner to the cc list which is not needed.

Comment 11

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

I think that's the right approach. No cleanup needed when the triage owner changes.

Comment 12

[Daniel Veditz [:dveditz]]

Adding triage owner to the CC list was a suggested hack (that we could even script ourselves if necessary) but not the preferred solution in any case. This looks good.

Comment 13

[Kohei Yoshino [:kohei]]

Merged to master.