Ensure committers have thought about bug description for security bugs when landing



Developer Services
Mercurial: hg.mozilla.org
6 years ago
4 years ago


(Reporter: jduell, Unassigned)




I've done this one too many time, forgetful me...

We should automatically strip anything bug "bug 45677: r=bz, a=akeybl" from security bugs when they are landed.  Right now we tell devs to manually do this.  I've forgotten now a bunch of times, and each time I get a reprimand and the hackers still get the hint in our hg logs.

Should just be a matter of querying bugzilla for isSecure?
Where do you want this stripped from? Bugzilla?
Might be better than the status quo, but would prevent the use of comments that are useful and not revealing. For many fixes that's probably not possible (you can't obscure a simple bounds check without lying) but for more involved fixes it's often possible to describe the code restructuring without saying which part of it precisely led to some bad thing elsewhere.

In fact, comment-less check-ins might be even more of a red-flag than one with a comment :-(
Keywords: sec-want
I generally like to give descriptions that are essentially mechanical descriptions of bugs. For instance, for bug 708825, I just said "set mIdentity to null if compartment enter fails", which doesn't reveal what the horrible consequence of not doing it, nor the weird tricky way you hit that problem. I feel like that at least gives some information in the hg log for later archaeology. But maybe it doesn't help much.

You can always just never put a descriptive description in the patch, though maybe that interferes with your work flow.

Comment 4

6 years ago
> would prevent the use of comments

We could add some sort of DONTDELETE magic so that people could add comments if they want one.  If nothing else this would be a reminder that they should stop and think about what they write, if anything...

Comment 5

6 years ago
So I propose we morph this into the following:  for secure bugs we barf at push time unless the commit message contains (ends with?) "SECURE".   If the message does contain "SECURE" than we s/SECURE// and commit the rest of the message.

That will at least make sure that people don't land patches without thinking of the text that they're putting into the public record.
Summary: automatically strip off bug description of security bugs when landing on m-c/inbound → Ensure committers have thought about bug description for security bugs when landing

Comment 6

6 years ago
The barf message could also point to a URI that describes best practices for what sort of commit message to provide--on IRC I got the advice to use a blank message, and it seems that's not actually the best way to do it.
This sounds like an hg hook.
Assignee: server-ops-devservices → nobody
Component: Server Operations: Developer Services → Hg: Customizations
QA Contact: shyam
FYI I don't think we can change commit messages on the server without really breaking the hg workflow. Changing the commit message changes the changeset's hash value.

I don't really know how you'd programmatically determine that the commit message has been carefully chosen, so I'm not sure how we'd implement this.

Comment 9

6 years ago
> Changing the commit message changes the changeset's hash value.

Ah, that blows up my idea in comment 5.   I'm out of ideas here--if someone wants to pursue, reopen or file a new bug.
Last Resolved: 6 years ago
Resolution: --- → WONTFIX


5 years ago
Product: mozilla.org → Release Engineering


4 years ago
Product: Release Engineering → Developer Services
You need to log in before you can comment on or make changes to this bug.