Hi,

On Sat, 12 Dec 2015, Ansgar Burchardt wrote:
> > There's a new configuration entry DInstall::UploadMailRecipients that
> > lists the recipients of the mails, either static ones (like
> > d...@security.debian.org) or dynamic ones (like "maintainer", "changed_by"
> > or "signer").
> 
> I prefer something like "mail:f...@example.com" over using the "@" do
> discriminate between e-mail addresses and keywords.

Done.

> > +            fpr_addresses = gpg_get_key_addresses(fingerprint)
> > +            for fpr_addr in fpr_addresses:
> > +                if fpr_addr in emails_added:
> > +                    break  # The signer already gets a copy via another 
> > email
> > +            else:
>                ^^^^
> Wrong indent?

No. It's the "else" clause of the for loop:
https://docs.python.org/2/tutorial/controlflow.html#break-and-continue-statements-and-else-clauses-on-loops

It's executed only when we come out of the loop without having hit any
break.

> Python also has a `any` function for this type of loops which should be
> a bit less verbose.

But the else is gone with this syntax now.

> That looks like `emails_added` should be a `set`.  As the order of
> recipients shouldn't matter, we can probably just use a single set for
> the mail addresses.

Changed them to set() but we need two because we retain the name
associated to the email but for the duplicate check we want to check
only on the email.

> Also the new logic to include the GPG mail address only when no other
> address from the key is already used depends on `signer` being the last
> keyword.

Right, I added a bit of logic to ensure this is always the case.

Thanks for the review! An updated patch is attached.

Thorsten, can you do some tests with the updated patch?

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Support Debian LTS: http://www.freexian.com/services/debian-lts.html
Learn to master Debian: http://debian-handbook.info/get/
>From 8da7b953f0353d0f67c6434e25c4f7eae415eb72 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rapha=C3=ABl=20Hertzog?= <hert...@debian.org>
Date: Tue, 1 Dec 2015 16:20:22 +0100
Subject: [PATCH] Implement DInstall::UploadMailRecipients to control
 recipients of upload mails

This new setting lets you configure the list of recipients of upload
mails (Accepted/Rejected), you can mix harcoded emails and the special
keywords "maintainer", "changed_by" and "signer" which get replaced
by the corresponding address extracted from the upload data.

The goal is that the security archive only sends mails to the security
team and to the person who signed the upload to not leak any information
about embargoed uploads.

Closes: #796784
---
 config/debian-security/dak.conf |  5 ++++-
 daklib/utils.py                 | 47 +++++++++++++++++++++++++++++++++++------
 2 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/config/debian-security/dak.conf b/config/debian-security/dak.conf
index ad47e33..1bd45e1 100644
--- a/config/debian-security/dak.conf
+++ b/config/debian-security/dak.conf
@@ -18,7 +18,10 @@ Dinstall
    BXANotify "false";
    DefaultSuite "stable";
    SuiteSuffix "updates/";
-   OverrideMaintainer "d...@security.debian.org";
+   UploadMailRecipients {
+     "mail:d...@security.debian.org";
+     "signer";
+   };
    LegacyStableHasNoSections "false";
    AllowSourceOnlyUploads "true";
 };
diff --git a/daklib/utils.py b/daklib/utils.py
index 518d66e..9d743b1 100644
--- a/daklib/utils.py
+++ b/daklib/utils.py
@@ -1086,15 +1086,48 @@ def mail_addresses_for_upload(maintainer, changed_by, fingerprint):
     @return: list of RFC 2047-encoded mail addresses to contact regarding
              this upload
     """
-    addresses = [maintainer]
-    if changed_by != maintainer:
-        addresses.append(changed_by)
+    recipients = Cnf.value_list('Dinstall::UploadMailRecipients')
+    if not recipients:
+        recipients = [
+            'maintainer',
+            'changed_by',
+            'signer'
+        ]
+
+    # Ensure signer is last if present
+    try:
+        recipients.pop(recipients.index('signer'))
+        recipients.append('signer')
+    except ValueError:
+        pass
+
+    # Compute the set of addresses of the recipients
+    addresses = set()  # Name + email
+    emails = set()     # Email only
+    for recipient in recipients:
+        if recipient.startswith('mail:'):  # Email hardcoded in config
+            address = recipient[5:]
+        elif recipient == 'maintainer':
+            address = maintainer
+        elif recipient == 'changed_by':
+            address = changed_by
+        elif recipient == 'signer':
+            fpr_addresses = gpg_get_key_addresses(fingerprint)
+            address = fpr_addresses[0] if len(fpr_addresses) > 0 else None
+            if any([x in emails for x in fpr_addresses]):
+                # The signer already gets a copy via another email
+                address = None
+        else:
+            raise Exception('Unsupported entry in {0}: {1}'.format(
+                'Dinstall::UploadMailRecipients', recipient))
 
-    fpr_addresses = gpg_get_key_addresses(fingerprint)
-    if len(fpr_addresses) > 0 and fix_maintainer(changed_by)[3] not in fpr_addresses and fix_maintainer(maintainer)[3] not in fpr_addresses:
-        addresses.append(fpr_addresses[0])
+        if address is not None:
+            email = fix_maintainer(address)[3]
+            if email not in emails:
+                addresses.add(address)
+                emails.add(email)
 
-    encoded_addresses = [ fix_maintainer(e)[1] for e in addresses ]
+    encoded_addresses = [fix_maintainer(e)[1] for e in addresses]
     return encoded_addresses
 
 ################################################################################
-- 
2.6.4

Reply via email to