Phabricator and confidential reviews

[Mark Côté]

(Cross-posted to mozilla.tools)

Hi, I have an update and a request for comments regarding Phabricator and confidential reviews.

We've completed the functionality around limiting access to Differential revisions (i.e. code reviews) that are tied to confidential bugs. To recap the original plan, various security groups in BMO are mirrored to Phabricator as "projects", containing the same set of users. When a bug has such a security group added to it, e.g. dom-core-security, thus restricting its visibility largely to members of that group, a Phabricator "policy" is similarly set on any associated revisions, restricting their visibility to the same group of people (plus the author of the revision, if they are not in the project already).

However, users outside of the security group(s) can see confidential bugs if they are involved with them in some way. Frequently the CC field is used as a way to include outsiders in a bug.

Phabricator has a similar feature, called "subscribers", which, as with CCs, both grants visibility to confidential revisions and also sends email updates when the revision changes. It was suggested that we attempt to synchronize CC and subscriber lists.

First I want to double check that this is truly useful. I am not sure how often CCed users are involved with confidential bugs' patches (I might be able to ballpark this with some Bugzilla searches, but I don't think it would be easy to get a straight answer). Anecdotally I have been told that a lot of the time users are CCed just to be informed of the problem, e.g. a manager might want to be aware of a vulnerability. Given that adding subscribers to a revision is just as easy as CCing a user on a bug, if it is infrequent that outsiders need to be involved in reviewing confidential patches, I lean towards taking the simple route of making this manual.

However if this is more common than I suspect, then we must decide how to synchronize the lists. The most straightforward approach is one-way synchronization from BMO, that is, anyone CCed on the bug will automatically be added as a subscriber to any associated revisions, but anyone manually added to the subscribers list who is not CCed on the bug would be automatically removed by the BMO-Phabricator synchronization routine. The alternative is to keep track of who was manually added and who was automatically synchronized, which gets complicated rather quickly, both in terms of implementation and usability.

The second question that would come up is whether this synchronization should apply to all revisions or just confidential ones. Given the dual nature of CCs/subscribers, for both visibility and notifications, I lean towards only doing this synchronization for confidential revisions, where it is more important. A further justification for limiting the mirroring is that Phabricator has a much more powerful and fine-grained notification system (Herald) than BMO's product- and component-watching feature. Automatic mirroring everywhere would reduce the utility of the former.

If you have any thoughts on this, please reply. I'll answer any questions and summarize the feedback with a decision in a few days. Note that we can, of course, try a simple approach to start, and add in more complex functionality after an evaluation period.

To sum up, there are three questions:

1. Is mirroring a confidential bug's CC list to association Differential revisions' subscriber lists actually useful? That is, does the utility justify the cost of implementation and maintenance?

2. If yes, is one-way mirroring, from BMO to Differential, sufficient?

3. Again if #1 is yes, should such mirroring be limited to confidential bugs, given that Phabricator's notification system is more fine-grained, and thus more useful, than BMO's product- and component-watching feature?

Thanks,
Mark

[Nicolas B. Pierron]

On 08/09/2017 12:30 AM, Mark Côté wrote:
> Hi, I have an update and a request for comments regarding Phabricator and confidential reviews.

First of all, thanks for considering confidential bugs as part of this
process. This was my main reason for not using moz-review.

> We've completed the functionality around limiting access to Differential revisions (i.e. code reviews) that are tied to confidential bugs. […]

> However, users outside of the security group(s) can see confidential bugs if they are involved with them in some way. Frequently the CC field is used as a way to include outsiders in a bug.

Note that Bugzilla warns us against adding people who do not have s-s access
to s-s bug. (This is an awesome feature by the way)

> […]

>
> First I want to double check that this is truly useful. […]

I did that multiple time in the past. The main reason for doing it was to
CC the person who contributed the patch, such that at best they can
contribute a fix as well, and in the worst case they can contribute insight
for fixing the issue.

So, not only the CC-ed persons are asked to review, I might ask them to even
submit patches to these security bugs. This is a way to gradually empower
contributors, from my point of view.

Also, some users can open s-s bugs, and contribute patches too. We should at
least accept people from the CC list / reporters to be able to submit new
patches.

> The second question that would come up is whether this synchronization should apply to all revisions or just confidential ones. […]

Currently Bugzilla has a "private" flag on attachments, and adding anybody
without s-s flags in the CC list of the bug should not have access to the
private attachments, but should have access to any non-private attachments.

A similar "private" flag could be used to prevent the synchronization of the
CC list / reporter which are out-side the s-s group.

--
Nicolas B. Pierron

[Axel Hecht]

To answer the question not asked ;-)
I think we should strive to have as few people as possible with general
access to security bugs. The concerns the folks have when crossing
borders is awful. And just from a general risk profile. Saying that as
someone that neither has nor wants access to security bugs in general.
So, in that sense, I think we should make this a general assumption,
that the folks writing patches and doing reviews are not in group of the
bug.
As to mirroring the information, I think it'd be good if there couldn't
be people on phabricator that are not on the bug. That way, the folks on
the bug that manage the security risk of it have one way of tracking the
visibility of the security issue.
I could see how not everybody involved in the bug automatically wants
all the bugmail. I personally wouldn't mind if I could opt-in to that,
but I'd be annoyed if I had to ask folks to manually add me again, after
I asked them to CC me to the bug. More a question if it's technically
feasible, could folks ask phabricator for access by following the link,
and it would go back and check bugzilla on-demand if it should do that?
Depends a bit on the additional private-attachment thing that Nicolas
mentioned.
Axel

Am 09.08.17 um 02:30 schrieb Mark Côté:

[Gijs Kruitbosch]

On 09/08/2017 01:30, Mark Côté wrote:
> If you have any thoughts on this, please reply. I'll answer any questions and summarize the feedback with a decision in a few days. Note that we can, of course, try a simple approach to start, and add in more complex functionality after an evaluation period.
>
> To sum up, there are three questions:
>
> 1. Is mirroring a confidential bug's CC list to association Differential revisions' subscriber lists actually useful? That is, does the utility justify the cost of implementation and maintenance?

Probably not as you're describing here, but read on.

> 2. If yes, is one-way mirroring, from BMO to Differential, sufficient?

I hit the case of "I need info / a review from person X on this security
bug that they can't currently see", and the associated modal warning
that :nbp mentioned, so often that I filed
https://bugzilla.mozilla.org/show_bug.cgi?id=1211377 (please read esp.
the bottom of comment #0).

So yes, people get CCd on sec bugs if/when required for
reviews/needinfo, and because we keep security groups relatively small
(for the reasons Axel stated), that means I end up using CCs heavily in
order to get work done on those bugs.

I think generally, what I would really want, rather than mirroring the
entire CC list, is making it painless to tell
bugzilla/mozreview/differential/whatever "here's a patch for
confidential bug X, request review from Y", and have whatever tool is
doing that not make me jump through 42 hoops to allow Y to see/do the
review. A warning would be acceptable and maybe useful, but I want to be
able to say "yes, I know X is a sec bug, and I know Y can't currently
see it, please do whatever is necessary to make Y see it anyway".

Oddly, that might mean that I actually want mirroring from differential
to BMO, but NOT the other way. That is, if I post a patch for a bug and
ask someone for review, they should be able to see the bug. :-)
I can always add people to the differential thing manually if there are
non-reviewers and non-sec-group-members who really need to see the
patch, right? (Seems much less likely!)

> 3. Again if #1 is yes, should such mirroring be limited to confidential bugs, given that Phabricator's notification system is more fine-grained, and thus more useful, than BMO's product- and component-watching feature?

Yes.

~ Gijs

[Ehsan Akhgari]

On 08/08/2017 08:30 PM, Mark Côté wrote:
> First I want to double check that this is truly useful. I am not sure how often CCed users are involved with confidential bugs' patches (I might be able to ballpark this with some Bugzilla searches, but I don't think it would be easy to get a straight answer).

No, it wouldn't be. Right now we're living in a world where the
discussions about patches happen in the bug, which means parts of the
patch often get quoted on the bug, which may make people want to look at
the patch while they're on the bug page, whereas if they were following
the same conversation on Phabricator perhaps they may find it sufficient
to follow code related discussions there. Or perhaps not, maybe old
culture turns out to be hard to change and code related conversations
still find their way back into bugs, which would make people reading the
bug naturally want to look at the code from the bug. It's impossible
for me to tell which way it will work.

> Anecdotally I have been told that a lot of the time users are CCed just to be informed of the problem, e.g. a manager might want to be aware of a vulnerability. Given that adding subscribers to a revision is just as easy as CCing a user on a bug, if it is infrequent that outsiders need to be involved in reviewing confidential patches, I lean towards taking the simple route of making this manua

My anecdotal experience is that I almost always look at the patch no
matter what the security bug is about in order to try to see if the
patch reveals a pattern that we can build a static analysis around in
order to prevent further occurrences of the bug, so the proposal of
requiring an extra step of finding someone to CC you on the review in
order to see the patch will introduce a hurdle for me when I look at
bugs in the components where I'm not in the security group for (aka,
most components.) That being said, I don't do this every day, and it's
not clear what the cost of implementing the syncing proposal is, so I'm
not sure what the right trade-off should be here.

(BTW, do we have data about this from Bugzilla server logs, for example
about how often patch attachments on security sensitive bugs are viewed
by people who are recently CCed?)

Cheers,
Ehsan

[Daniel Veditz]

On Tue, Aug 8, 2017 at 11:38 PM, Nicolas B. Pierron <
nicolas....@mozilla.com> wrote:

> However, users outside of the security group(s) can see confidential bugs
>> if they are involved with them in some way. Frequently the CC field is
>> used as a way to include outsiders in a bug.
>
>
> Note that Bugzilla warns us against adding people who do not have s-s
> access to s-s bug. (This is an awesome feature by the way)
>

​It really shouldn't. Unless we expand the group of people who can see
security bugs to thousands of people (everyone with an NDA? even that might
not be enough) there will always be people who need to see a bug who
weren't able to see it by default. Since we have the CC'ing mechanism we
can keep the "default" group small and then freely CC people as needed.

I only know of two such warnings.

1) when you needinfo? someone who can't see a bug.​ That's warning you that
they won't ever see your request, not that you shouldn't add them to the
bug. If it were the latter we'd also be warning every time you CC someone
on a hidden bug. Since a named request is obviously inviting that person
into the bug we should just automatically CC that person at the same time.

2) when duping a bug. Normally when you dupe a bug the reporter of the dupe
is silently CC'd to the active bug. For security bugs the warning makes
this a conscious choice. Most of the time I'd say "sure, go ahead": the
reporter already knows about the issue, they might as well continue to be
involved in the solution. There are cases, though, where that's not true so
it's good to have people make a conscious choice. We might not want to CC
the dupe reporter if the active bug is not an identical dupe but is instead
a broader issue, or if the dupe target has a more damaging example that the
dupe reporter hadn't thought of yet.

Sometimes people dupe bugs to one that will fix it, but isn't the same kind
of testcase. When it's a security bug I usually prefer that we mark those
as "Depends on" the other bug and leave them open so we can verify the fix
later. As a bonus, then the CC'ing issue doesn't come up.

-
​Dan Veditz​

[Daniel Veditz]

On Wed, Aug 9, 2017 at 12:20 AM, Axel Hecht <l1...@mozilla.com> wrote:

> I think we should strive to have as few people as possible with general
> access to security bugs.

​We do. We've reduced the number of people with access, and split the
"client" security group into ~10 sub groups so that any given person has
access to a smaller subset of security bugs that are more likely to be
relevant to them.

So, in that sense, I think we should make this a general assumption, that
> the folks writing patches and doing reviews are not in group of the bug.
>

We
​would get little done if we couldn't CC people to give them access. Many
of those people are crucial to the production of the patch itself (as the
author or reviewer) and will need access to the Phabricator version.

-
​Dan Veditz​

[Daniel Veditz]

On Tue, Aug 8, 2017 at 5:30 PM, Mark Côté <mc...@mozilla.com> wrote:

> I am not sure how often CCed users are involved with confidential bugs'
> patches

> ​[​
> ​....] Anecdotally I have been told that a lot of the time users are CCed

> just to be informed of the problem, e.g. a manager might want to be aware
> of a vulnerability.
>

​People are CC'd for both reasons quite often. Couldn't tell you if it's
50-50 or 30-70.

​

> Given that adding subscribers to a revision is just as easy as CCing a
> user on a bug, if it is infrequent that outsiders need to be involved in
> reviewing confidential patches, I lean towards taking the simple route of
> making this manual.
>

​Can someone who is the assignee of a confidential bug, but is not in the
security group, create a confidential Phabricator patch for that bug? That
case happens quite often as some simpler security bugs are taken by the
reporter or assigned to newer team members. If so then I guess we could try
the manual approach as a MVP until it annoys people too much.

The second question that would come up is whether this synchronization
> should apply to all revisions or just confidential ones.

​For a confidential bug, why would some revisions be confidential and some
not? It's not uncommon for a bug to be marked confidential after work has
started on it and security ramifications become clear. We'd want that
confidentiality to apply to the patches that already exist because they may
provide outsiders the same hints about the problem they gave us.​

1. Is mirroring a confidential bug's CC list to association Differential
> revisions' subscriber lists actually useful? That is, does the utility
> justify the cost of implementation and maintenance?
>

How much work is it? Hard to know how much work reducing the annoyance and
friction is worth.

2. If yes, is one-way mirroring, from BMO to Differential, sufficient?
>

​Probably not, especially if you don't block subscribing people for
confidential bugs. People will be blindsided if they've subscribed people
and then it gets wiped out.​ Being told you have to go to BMO for that bug
will be an annoyance, but at least you won't think you've given someone
access when you really haven't and then wonder why they never review your
patch.

3. Again if #1 is yes, should such mirroring be limited to confidential
> bugs, given that Phabricator's notification system is more fine-grained,
> and thus more useful, than BMO's product- and component-watching feature?
>

​I'm on the fence about #1 (I lean toward 'yes'), but definitely only
needed for confidential bugs.
​
-
​Dan Veditz​

[Mark Côté]

For brevity and clarity I'm just replying to Dan here, but I am attempting to address other points raised so far in this thread.

On Wednesday, 9 August 2017 13:07:08 UTC-4, Daniel Veditz wrote:
> On Tue, Aug 8, 2017 at 5:30 PM, Mark Côté <mc...@mozilla.com> wrote:
>
> > I am not sure how often CCed users are involved with confidential bugs'
> > patches
> > ​[​
> > ​....] Anecdotally I have been told that a lot of the time users are CCed
> > just to be informed of the problem, e.g. a manager might want to be aware
> > of a vulnerability.
> >
>
> ​People are CC'd for both reasons quite often. Couldn't tell you if it's
> 50-50 or 30-70.

I think Gijs's post is quite interesting. It furthers my idea that there are two main reasons you get involved in a confidential bug: either someone figures you should know about the issue, or you are pulled in to review the patch. (Although there are some outliers, like Ehsan, who fall under both.) I actually like Gijs's proposal, to mirror *from* Phabricator *to* BMO. That way, if you're looking at the bug and want to pull someone in, you CC them; if you're looking at the fix and want to involve someone, you add them as a subscriber which then incidentally lets them view the bug, for background and updates and such. We can also add a clear comment ("user X has been CCed to this bug via Differential revision Y" or such).

I think we'd want to simplify here, though, and make such mirroing "one time", meaning you get CCed when added as a subscriber, but you wouldn't be removed from the CC list of the bug if removed as a subscriber, since people may be being directly CCed to the bug as well.

> > Given that adding subscribers to a revision is just as easy as CCing a
> > user on a bug, if it is infrequent that outsiders need to be involved in
> > reviewing confidential patches, I lean towards taking the simple route of
> > making this manual.
> >
>
> ​Can someone who is the assignee of a confidential bug, but is not in the
> security group, create a confidential Phabricator patch for that bug? That
> case happens quite often as some simpler security bugs are taken by the
> reporter or assigned to newer team members. If so then I guess we could try
> the manual approach as a MVP until it annoys people too much.

Yes. You won't be able to submit a revision associated with a bug you don't have access to, but as long as you can attach a file to the bug, you can create an associated revision.

> The second question that would come up is whether this synchronization
> > should apply to all revisions or just confidential ones.
>
>
> ​For a confidential bug, why would some revisions be confidential and some
> not? It's not uncommon for a bug to be marked confidential after work has
> started on it and security ramifications become clear. We'd want that
> confidentiality to apply to the patches that already exist because they may
> provide outsiders the same hints about the problem they gave us.​

Sorry, I was not clear here. I meant whether we'd want to do any mirroring of CC/subscriber lists for public bugs. (This was rephrased as question 3 in my summary). Seems that the answer continues to be "no" here.

> 1. Is mirroring a confidential bug's CC list to association Differential
> > revisions' subscriber lists actually useful? That is, does the utility
> > justify the cost of implementation and maintenance?
> >
>
> How much work is it? Hard to know how much work reducing the annoyance and
> friction is worth.

Yeah that's going to depend on what solution we come up with. :) But my proposed solution above wouldn't be too difficult, as we are already doing some revision->bug updates anyway.

> 2. If yes, is one-way mirroring, from BMO to Differential, sufficient?
> >
>
> ​Probably not, especially if you don't block subscribing people for
> confidential bugs. People will be blindsided if they've subscribed people
> and then it gets wiped out.​ Being told you have to go to BMO for that bug
> will be an annoyance, but at least you won't think you've given someone
> access when you really haven't and then wonder why they never review your
> patch.

I think my above proposal addresses this, particularly with comments indicating when someone has been automatically added to the CC list.

Thanks everyone for your input so far.

Mark

[Daniel Veditz]

On Wed, Aug 9, 2017 at 11:32 AM, Mark Côté <mc...@mozilla.com> wrote:

> I actually like Gijs's proposal, to mirror *from* Phabricator *to* BMO.
> That way, if you're looking at the bug and want to pull someone in, you CC
> them; if you're looking at the fix and want to involve someone, you add
> them as a subscriber which then incidentally lets them view the bug, for
> background and updates and such.

​What if there's not "the" fix but multiple patches? This is quite common
for security bugs where a testcase that reveals the vulnerability is done
in a separate patch so it can be checked in after we release the fix. Or
occasionally bugs like https://bugzilla.mozilla.org/show_bug.cgi?id=1371259
that had 9 separate patches. Would the same people have to be separately
subscribed to each?​

​There's a case to be made for adding in both directions. How much work
would it be to (add all CCs to subscriber list) when attaching a new patch,
and (subscribe person to all existing patches)​ when someone new is CC'd to
a bug? This was my attempt to match your "one time mirroring" approach to
going the other way, which makes sense to me.

And don't forget reporter and assignees. Occasionally a reporter not in the
security group will notice that a patch is insufficient which is nicer to
find before the patch is committed than after the commit link is added to
the bug. Sometimes someone other than the official assignee adds a patch
for an alternate approach to a fix and asks the assignee for feedback,
though that's uncommon and the assignee could just be manually subscribed
to the patch at that point.

We can wait and see how common the "please subscribe me to the patch"
requests are, but I suspect they'll be common enough.

-
​Dan Veditz​

[Frederik Braun]

Having both reported, fixed and reviewed security bugs, I feel an
uni-directional sync from Phabricator to BMO is not going to cut it. I
think it will be unexpected for most users and might just lead to
additional "why can I not see the patch" bug comments.

I understand that it's more work, but I really think we should aim for
"perfect" rather than "good enough", when it comes to developer experience.

I don't see a strong "security reason" not to include people copied in
the bug to see the patch. Quite contrary: It's also what distinguishes
our bug bounty program from others. We're more open, we have outside
reporters work with us on the patch and see all the comments and
iterations that lead to its perfection. This encourages early feedback
and improves the outcome.

On 10.08.2017 00:57, Daniel Veditz wrote:

> On Wed, Aug 9, 2017 at 11:32 AM, Mark Côté <mc...@mozilla.com> wrote:
>
>> I actually like Gijs's proposal, to mirror *from* Phabricator *to* BMO.
>> That way, if you're looking at the bug and want to pull someone in, you CC
>> them; if you're looking at the fix and want to involve someone, you add
>> them as a subscriber which then incidentally lets them view the bug, for
>> background and updates and such.
>
>

> ​What if there's not "the" fix but multiple patches? This is quite common
> for security bugs where a testcase that reveals the vulnerability is done
> in a separate patch so it can be checked in after we release the fix. Or
> occasionally bugs like https://bugzilla.mozilla.org/show_bug.cgi?id=1371259
> that had 9 separate patches. Would the same people have to be separately
> subscribed to each?​
>
> ​There's a case to be made for adding in both directions. How much work
> would it be to (add all CCs to subscriber list) when attaching a new patch,
> and (subscribe person to all existing patches)​ when someone new is CC'd to
> a bug? This was my attempt to match your "one time mirroring" approach to
> going the other way, which makes sense to me.
>
> And don't forget reporter and assignees. Occasionally a reporter not in the
> security group will notice that a patch is insufficient which is nicer to
> find before the patch is committed than after the commit link is added to
> the bug. Sometimes someone other than the official assignee adds a patch
> for an alternate approach to a fix and asks the assignee for feedback,
> though that's uncommon and the assignee could just be manually subscribed
> to the patch at that point.
>
> We can wait and see how common the "please subscribe me to the patch"
> requests are, but I suspect they'll be common enough.
>
> -
> ​Dan Veditz​

> _______________________________________________
> dev-platform mailing list
> dev-pl...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>

[Randell Jesup]

>On Wed, Aug 9, 2017 at 11:32 AM, Mark Côté <mc...@mozilla.com> wrote:
>
>> I actually like Gijs's proposal, to mirror *from* Phabricator *to* BMO.
>> That way, if you're looking at the bug and want to pull someone in, you CC
>> them; if you're looking at the fix and want to involve someone, you add
>> them as a subscriber which then incidentally lets them view the bug, for
>> background and updates and such.
>
>
>​What if there's not "the" fix but multiple patches? This is quite common
>for security bugs where a testcase that reveals the vulnerability is done
>in a separate patch so it can be checked in after we release the fix. Or
>occasionally bugs like https://bugzilla.mozilla.org/show_bug.cgi?id=1371259
>that had 9 separate patches. Would the same people have to be separately
>subscribed to each?​

I've seen landings (for non-security bugs) that involved up to 100
patches on one bug... or even more. While that's probably never
happened for a sec bug, multiple patches is not unusual (and in fact
common where there's a test added).

>And don't forget reporter and assignees. Occasionally a reporter not in the
>security group will notice that a patch is insufficient which is nicer to
>find before the patch is committed than after the commit link is added to
>the bug. Sometimes someone other than the official assignee adds a patch
>for an alternate approach to a fix and asks the assignee for feedback,
>though that's uncommon and the assignee could just be manually subscribed
>to the patch at that point.

In fact it's quite common for people who are cc'd other than the patch
writer and reviewer(s) to look at the bug and comment on it, or to
submit an alternate or modified version of a patch previously uploaded.
And as Dan points out, frequently the reporter (when an external
'researcher') will weigh in on the patches being posted or use them to
verify if they fix the problem they found (since many times only they
can reliably reproduce the problem - private test setups, drivers, HW, etc).

>We can wait and see how common the "please subscribe me to the patch"
>requests are, but I suspect they'll be common enough.

Bite the bullet and at least make all CC'd people able to see all
patches, always. It's needed.

Another aspect of all this that needs to be thought out and verified is
what happens when a non-sec bug becomes a sec bug (and vice-versa,
though I'm far less concerned about that). When a bug becomes a sec
bug, all patches associated with that bug must become confidential. And
when a bug moves to be open (not sec-restricted), the patches should
also become open.

--
Randell Jesup, Mozilla Corp
remove "news" for personal email

[Mark Côté]

On Saturday, 26 August 2017 00:40:08 UTC-4, Randell Jesup wrote:
> >And don't forget reporter and assignees. Occasionally a reporter not in the
> >security group will notice that a patch is insufficient which is nicer to
> >find before the patch is committed than after the commit link is added to
> >the bug. Sometimes someone other than the official assignee adds a patch
> >for an alternate approach to a fix and asks the assignee for feedback,
> >though that's uncommon and the assignee could just be manually subscribed
> >to the patch at that point.
>
> In fact it's quite common for people who are cc'd other than the patch
> writer and reviewer(s) to look at the bug and comment on it, or to
> submit an alternate or modified version of a patch previously uploaded.
> And as Dan points out, frequently the reporter (when an external
> 'researcher') will weigh in on the patches being posted or use them to
> verify if they fix the problem they found (since many times only they
> can reliably reproduce the problem - private test setups, drivers, HW, etc).

Right, it's easy to add the reporter since that person won't change. The assignee will also automatically have access if they are the patch author.

I also got some data from BMO that strongly suggests that, indeed, many people who aren't the author or reviewer look at sec-bug patches, and most of those aren't in the sec group; they have access by being CCed in. That's surprising to me, but it definitely suggests the need for syncing the bug's CC list to the revision's subscriber list.

> >We can wait and see how common the "please subscribe me to the patch"
> >requests are, but I suspect they'll be common enough.
>
> Bite the bullet and at least make all CC'd people able to see all
> patches, always. It's needed.

Yeah, that's the direction I think we should take.

For now, we will implement exact syncing of the CC list + reporter as the revision's subscriber list. This means that anyone being added as a subscriber who isn't CCed will unfortunately be quickly removed (we may even prevent manual additions to the subscriber list). The reasoning here is that I believe it's important that anyone looking at the bug knows exactly who can see the patch. If the two lists are kept in sync, it will be obvious. However, if we let people add subscribers directly, then it may turn out that a bug with only, say, 10 CCed people might have a patch that 100 people can view, and that's probably something you don't want to hide.

We'll also investigate what bidirectional syncing would take. Due to race conditions and circular references, though, it won't be straightforward, and I suspect our time would be best spent elsewhere.

We can and will revisit this after Phabricator goes live for everyone.

> Another aspect of all this that needs to be thought out and verified is
> what happens when a non-sec bug becomes a sec bug (and vice-versa,
> though I'm far less concerned about that). When a bug becomes a sec
> bug, all patches associated with that bug must become confidential. And
> when a bug moves to be open (not sec-restricted), the patches should
> also become open.

Of course. This is already implemented.

Mark

[Randell Jesup]

>> Bite the bullet and at least make all CC'd people able to see all
>> patches, always. It's needed.
>
>Yeah, that's the direction I think we should take.

Good, thanks.

>For now, we will implement exact syncing of the CC list + reporter as the
>revision's subscriber list. This means that anyone being added as a
>subscriber who isn't CCed will unfortunately be quickly removed (we may
>even prevent manual additions to the subscriber list). The reasoning here
>is that I believe it's important that anyone looking at the bug knows
>exactly who can see the patch. If the two lists are kept in sync, it will
>be obvious. However, if we let people add subscribers directly, then it
>may turn out that a bug with only, say, 10 CCed people might have a patch
>that 100 people can view, and that's probably something you don't want to
>hide.
>
>We'll also investigate what bidirectional syncing would take. Due to race
>conditions and circular references, though, it won't be straightforward,
>and I suspect our time would be best spent elsewhere.

Don't bother with bidirectional syncing - your point about being able to
look at the bug and know who can see it is important, but I don't see
any advantage in subscribe mirroring into cc. I would (as you suggest)
block subscribe on sec issues, to avoid confusion (if you can, tell
people to use cc, but that's just gravy).