bzr-hookless-email doesn't work with bzr 2.6

[Glenn Morris]

Thanks a lot, this all looks very helpful!

Vincent Ladeuil wrote:

>     > I guess the only other option is to use the bzr-email plugin.
>
> Not sure about your use case here, but from reading bzr-hookless-email
> it seems to send one email by revision for all revisions since the last
> mail has been sent. Whereas bzr-email will send only one mail without
> tracking the revision. Does that still fit your use case ?

I think bzr-email would be fine. I'm not totally sure why Savannah went
with bzr-hookless-email originally. IIUC it was because of a dislike of
plugins. I can understand not wanting to let users install arbitrary
plugins, but I don't see the same problem with system plugins.

> Also, if bzr-hookless-email want to send mails from the last sent
> revision, may be you can just skip the problematic revision and set
> 'last_revision_mailed' to a newer one ? That may work around the issue
> while searching for the proper fix ?

Fantastic, yes, this worked!

> AFAIUI, these threads are about "Setting up bzr+ssh on Savannah" which
> seems to be a broader subject than just emailing about commits.

Sorry, I guess the only relevant bit is:

http://lists.gnu.org/archive/html/savannah-hackers/2009-12/msg00009.html

    For a highly secure environment like savannah, I'd be happy to
    provide an even less configurable bzr-email than normal, which users
    cannot configure at all beyond choosing to have commits sent or not.

http://lists.gnu.org/archive/html/savannah-hackers-public/2010-03/msg00028.html

   we'll want to give it an audit to make sure users won't be able to
   inject a command to run remotely

> I had a look at bzr-hookless-email but it uses too many pieces I don't
> know so I can't really comment on how or why it fails here.

Thanks for trying. :)

>     > Maybe the concern is that someone with write access to a branch's
>     > bazaar.conf
>
> Yeah, it's called branch.conf

(Sorry, just a typo on my part.)

>     > could eg set post_commit_mailer to something malicious?
>
> Not sure what, it has to be an external resource supposed to be a
> mailer.

This was just my guess from reading
http://doc.bazaar.canonical.com/plugins/en/email-plugin.html

  How emails are sent is determined by the value of the configuration
  option 'post_commit_mailer':
  [...]
  Any other value: Run the value expecting it to behave like /usr/bin/mail

So I wonder what happens if I set it "rm -rf /"...
Maybe it fails, but it's not obvious from the doc.

> Now, if post_commit_mailer is under the branch control, it can
> point to some code added by the branch.
>
> So, if there is need to counter this potential vulnerability,
> post_commit_mailer should remain under site admins control.

Yep.

> In bzr-email, the design is that a few hooks are defined and will
> received a "config stack" from the branch they are run for.
>
> This stack will search for the option values (in this order) defined:
>
> - on the command line,
> - in the locations.conf file
> - in the branch.conf (for the branch involved) file,
> - in the bazaar.conf file,
> - as default.
>
> So, it's unclear to me if/where you can define options in a way that an
> arbitrary user cannot override as it depends on what policy savannah
> enforces for the locations.conf and bazaar.conf files (if a user can
> modify a branch he generally can modify branch.conf too).

Yes, as an ordinary user I can modify a branch's branch.conf. This would
actually be useful, so that I can enable/disable commit emails for a
branch (using bzr config). Currently, we have to ask the admins to do
it.

I don't know about locations.conf or bazaar.conf. I don't think I can
modify those as an ordinary user, since (AFAIK) I have no shell access,
only access via bzr+ssh.

> If bzr is run as a specific user that only savannah admins have access
> to, defining options in locations.conf (with a section valid for all
> branches) will be a way to override whatever is defined in branch.conf.

Thanks, very useful to know. I will pass this on to the Savannah admins,
I don't know the details of how they have it implemented.

> If not, well, may be you can patch the plugin to use hard-coded values
> instead of getting them from a config stack ? Or getting them from a
> different config stack (which could be tailored to be as specific as
> needed).
>
> For reference, branch.conf (which is used by bzr-email-hooks,
> bzr-hookless-email is less clear but could be upgraded to use the newer
> API (the old one is still available for compatibility) is defined as:
>
> class BranchStack(Stack):
>     """Per-location options falling back to branch then global options stack.
>
>     The following sections are queried:
>
>     * command-line overrides,
>
>     * the sections matching ``location`` in ``locations.conf``, the order being
>       defined by the number of path components in the section glob, higher
>       numbers first (from most specific section to most generic),
>
>     * the no-name section in branch.conf,
>
>     * the ``DEFAULT`` section in ``bazaar.conf``.
>
>     This stack will use the no-name section in ``branch.conf`` as its
>     MutableSection.
>     """
>
>     def __init__(self, branch):
>         lstore = self.get_shared_store(LocationStore())
>         bstore = branch._get_config_store()
>         gstore = self.get_shared_store(GlobalStore())
>         super(BranchStack, self).__init__(
>             [self._get_overrides,
>              LocationMatcher(lstore, branch.base).get_sections,
>              NameMatcher(bstore, None).get_sections,
>              NameMatcher(gstore, 'DEFAULT').get_sections],
>             bstore)
>
> which for savannah could be simplified to be just (untested code):
>
> class SavannahStack():
>     """Savannah options stack.
>
>     The following sections are queried:
>
>     * the no-name section in /whatever/savannah.conf,
>
>     This stack will use the no-name section in ``savannah.conf`` as its
>     MutableSection.
>     """
>
>     def __init__(self):
>         sstore = self.get_shared_store(SavannahStore())
>         super(SavannahStack, self).__init__(
>             [NameMatcher(sstore, None).get_sections,
>             sstore)
>
> class SavannahStore(LockableIniFileStore):
>
>     def __init__(self, possible_transports=None):
>         t = transport.get_transport_from_path(
>             '/whatever, possible_transports=possible_transports)
>         super(SavannahStore, self).__init__(t, 'savannah.conf')
>
> '/whatever' should be a directory under admins control and can be
> read-only for everybody else.

Thanks very much for the hints!