[Reportbug-maint] Bug#878088: Bug#878088: reportbug: please inform security and lts teams about security update regressions

[Nis Martensen]

On 29-01-2018 00:11, Markus Koschany wrote:
> 
> I noticed that you had to import apt but reportbug does not depend on
> python3-apt. After I had installed this package it worked. I also
> believe you don't need to check for the upstream changelog.gz file, the
> Debian changelog should be sufficient.

There is already a patch in some other bug report that adds a Depends:
on python3-apt.  You are right that this is also required if the new
function gets accepted.

We need to look for both changelog files, since native packages (like
reportbug) do not have a separate Debian one.


Your patch looks good to me now. Only minor nits:

> reportbug.debdiff
> 
> 
> diff -Nru reportbug-7.1.8/bin/reportbug reportbug-7.1.8+nmu1/bin/reportbug
> --- reportbug-7.1.8/bin/reportbug	2017-12-29 05:25:43.000000000 +0100
> +++ reportbug-7.1.8+nmu1/bin/reportbug	2018-01-23 20:43:14.000000000 +0100
> @@ -32,6 +32,8 @@
>  import optparse
>  import re
>  import locale
> +import requests
> +import json
>  import subprocess
>  import shlex
>  import email
> @@ -1926,6 +1928,37 @@
>              listcc += ui.get_multiline(
>                  'Enter any additional addresses this report should be sent to; press ENTER after each address.')
>  
> +        # If the bug is reported against a package with a version that
> +        # indicates a security update add the security or lts team to CC
> +        # after user confirmation
> +        if pkgversion and package and not self.options.offline and not self.options.mode == 'novice':

Instead of "not self.options.mode == 'novice', please use
"mode > MODE_NOVICE"

> +            if utils.is_security_update(package, pkgversion):
> +                if ui.yes_no('Do you want to report a regression because of a security update? ',
> +                             'Yes, please inform the LTS and security teams.',
> +                             'No or I am not sure.', True):
> +                    regex = re.compile('(\+|~)deb(\d+)u(\d+)')
> +                    secversion = regex.search(pkgversion)
> +                    distnumber = secversion.group(2)

shorter: distnumber = re.search('[+~]deb(\d+)u\d+', pkgversion).group(1)

> +                    support = 'none'
> +                    email_address = []

email_address = 'none'

> +                    try:
> +                        r = requests.get('https://security-tracker.debian.org/tracker/distributions.json',
> +                                    timeout=self.options.timeout)
> +                        data = r.json()
> +                        for key, value in data.items():
> +                            if distnumber == value['major-version']:
> +                                if value['support']:
> +                                    support = value['support']
> +                                if value['contact']:
> +                                    email_address = value['contact']

If we can trust that no fields are null, then the last two ifs are not
needed and we can drop them to simplify the code. If we don't trust the
input, should we protect against other errors as well? We'd get
TypeError if some `value` is not a dict, and KeyError if any key is not
there.

> +
> +                        if support != 'none':

if support != 'none' and utils.check_email_addr(email_address)


> +                            listcc += [email_address]

else:
    ewrite('No support team contact address could be identified.\n')

> +
> +                    except requests.exceptions.RequestException:

If we want to also catch TypeError and KeyError, do it here or in a
separate except clause?

> +                        ewrite('Unable to connect to security-tracker.debian.org.\n'
> +                               'Please try again later or contact the LTS or security team via email directly.\n')
> +
>          if severity and rtype:
>              severity = debbugs.convert_severity(severity, rtype)
>