Spurious "secure" emails from phabricator when a revision is filed and then its related revisions change
Categories
(Conduit :: Phabricator, defect, P1)
Tracking
(Not tracked)
People
(Reporter: bzbarsky, Assigned: smacleod)
References
Details
(Keywords: conduit-triaged, conduit-upstream)
Attachments
(4 files)
I received two emails this morning about https://phabricator.services.mozilla.com/D25726
The first one has the subject [Differential] [Request] [++-- ] D25726: Bug 1540962 - Make nsICommandManager builtinclass and make users use nsCommandManager directly and the body indicates that the revision is being created:
masayuki created this revision.
masayuki added a reviewer: bzbarsky.
The second email has the subject [Differential] D25726: Revision Updated and the body says:
This secure message is notifying you of a change to this object:
https://phabricator.services.mozilla.com/D25726
The content for this message can only be transmitted over a secure channel. To view the message content, follow this link:
https://phabricator.services.mozilla.com/mail/detail/655195/
That last link just shows a child revision being added to D25726.
Why was that second message sent as a "secure" message that I had to go read in a browser instead of in my mail client?
Note: I also got two emails for https://phabricator.services.mozilla.com/D25727 in the same way: one mail that says it's been created, and a "secure" one pointing to https://phabricator.services.mozilla.com/mail/detail/655226/ which is again a notification that a child revision was added.
|
Assignee | |
Comment 1•6 years ago
|
||
:dkl, could this be due to the change in upstream phabricator around secure emails on linked revisions?
|
||
Comment 2•6 years ago
|
||
I think what happened is a timing issue. The child revision was created and then added to the parent while the child was still secured by the 'secure-revision' policy. So the email about the child revision being added to the parent was sent as a secured email. BMO then came around shortly after and unlocked the child revision. You can see this in the revision history of https://phabricator.services.mozilla.com/D25727 I assume these were created using moz-phab and mozphab is probably creating the revision chain faster then BMO can update the revisions so this may always be an unfortunate side-effect with how we do security policies for revisions.
|
Reporter | |
Comment 3•6 years ago
|
||
Can we fix moz-phab to not do the chaining until the revisions have been poked by BMO?
Alternately, of course, we could fix our secure mail setup to not have unreadable mails....
(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #3)
Can we fix moz-phab to not do the chaining until the revisions have been poked by BMO?
no, this isn't viable for many reasons (eg. bmo updates are async and may take a while to complete, moz-phab isn't the only client, ...).
|
|
Reporter | |
Comment 5•6 years ago
|
||
OK. I mean, fixing the secure mail setup would obviously be vastly preferable from my pov for all sorts of reasons. ;)
|
|
Reporter | |
Comment 6•6 years ago
|
||
Fwiw, I get a bunch of these any time someone posts a patch queue to phabricator, apparently. :( Got 4 for the patches in bug 1542932.
|
|
Assignee | |
Comment 7•6 years ago
|
||
:bzbarsky, any chance you could attach original emails for me? Both the created emails for the two revisions and the secure message about the link being added. I'm interested in more specific timing information.
|
||
Comment 8•6 years ago
|
||
https://phabricator.services.mozilla.com/mail/detail/685594/
https://phabricator.services.mozilla.com/mail/detail/685596/
https://phabricator.services.mozilla.com/mail/detail/685598/
|
|
Reporter | |
Comment 9•6 years ago
|
||
|
|
Reporter | |
Comment 10•6 years ago
|
||
|
|
Reporter | |
Comment 11•6 years ago
|
||
|
|
Assignee | |
Comment 12•6 years ago
|
||
I've engaged upstream on the best way forward here, awaiting a response.
|
|
Assignee | |
Comment 13•6 years ago
|
||
The current upstream plan is to remove these dependency change emails entirely, as they interact with other pieces of Phabricator in complex ways. I'm leaning towards not pushing back on this, but I'm open to hearing compelling reasons we really need these notifications still (assuming they'd be in the email, not behind a link). :bzbarksy, since you reported, how do you feel about this solution?
|
|
||
Comment 14•6 years ago
•
|
||
(In reply to Steven MacLeod [:smacleod] from comment #13)
The current upstream plan is to remove these dependency change emails entirely, as they interact with other pieces of Phabricator in complex ways. I'm leaning towards not pushing back on this, but I'm open to hearing compelling reasons we really need these notifications still (assuming they'd be in the email, not behind a link). :bzbarksy, since you reported, how do you feel about this solution?
@smacleod, what if we just have them remove the child/parent revision summary from the change emails? Just state that some revision id has become a child/parent of another revision id with a link the user can click on to go to either revision. It has been decided in other places such as BMO that IDs are not considered sensitive information.
|
|
Assignee | |
Comment 15•6 years ago
|
||
(In reply to David Lawrence [:dkl] from comment #14)
@smacleod, what if we just have them remove the child/parent revision summary from the change emails? Just state that some revision id has become a child/parent of another revision id with a link the user can click on to go to either revision. It has been decided in other places such as BMO that IDs are not considered sensitive information.
Yes, and that's exactly what I proposed to upstream, but they would much rather remove these entirely due to complexity and interaction with other features.
|
|
Reporter | |
Comment 16•6 years ago
|
||
I would have no problem with the dependency-change emails going away entirely, personally.
|
|
Assignee | |
Comment 17•6 years ago
|
||
This will go out with the next Phabricator production deploy.
Description
•