diff -Nru click-reviewers-tools-0.19/bin/click-check-bin-path click-reviewers-tools-0.20/bin/click-check-bin-path --- click-reviewers-tools-0.19/bin/click-check-bin-path 1970-01-01 00:00:00.000000000 +0000 +++ click-reviewers-tools-0.20/bin/click-check-bin-path 2015-01-05 15:57:17.000000000 +0000 @@ -0,0 +1,31 @@ +#!/usr/bin/python3 +'''click-check-bin-path: perform click bin-path checks''' +# +# Copyright (C) 2014 Canonical Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 3 of the License. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +from __future__ import print_function +import sys + +import clickreviews.cr_common as cr_common +import clickreviews.cr_bin_path as cr_bin_path + +if __name__ == "__main__": + if len(sys.argv) < 2: + cr_common.error("Must give path to click package") + + review = cr_bin_path.ClickReviewBinPath(sys.argv[1]) + review.do_checks() + rc = review.do_report() + sys.exit(rc) diff -Nru click-reviewers-tools-0.19/bin/click-check-framework click-reviewers-tools-0.20/bin/click-check-framework --- click-reviewers-tools-0.19/bin/click-check-framework 1970-01-01 00:00:00.000000000 +0000 +++ click-reviewers-tools-0.20/bin/click-check-framework 2015-01-05 15:57:17.000000000 +0000 @@ -0,0 +1,31 @@ +#!/usr/bin/python3 +'''click-check-framework: perform click framework checks''' +# +# Copyright (C) 2014 Canonical Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 3 of the License. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +from __future__ import print_function +import sys + +import clickreviews.cr_common as cr_common +import clickreviews.cr_framework as cr_framework + +if __name__ == "__main__": + if len(sys.argv) < 2: + cr_common.error("Must give path to click package") + + review = cr_framework.ClickReviewFramework(sys.argv[1]) + review.do_checks() + rc = review.do_report() + sys.exit(rc) diff -Nru click-reviewers-tools-0.19/bin/click-review click-reviewers-tools-0.20/bin/click-review --- click-reviewers-tools-0.19/bin/click-review 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/bin/click-review 2015-01-05 15:57:17.000000000 +0000 @@ -16,7 +16,7 @@ return '' print(description) print(''.center(len(description), '-')) - for key in results.keys(): + for key in sorted(results.keys()): print(' - %s' % key) print('\t%s' % results[key]['text']) if 'link' in results[key]: diff -Nru click-reviewers-tools-0.19/bin/clickreviews/cr_bin_path.py click-reviewers-tools-0.20/bin/clickreviews/cr_bin_path.py --- click-reviewers-tools-0.19/bin/clickreviews/cr_bin_path.py 1970-01-01 00:00:00.000000000 +0000 +++ click-reviewers-tools-0.20/bin/clickreviews/cr_bin_path.py 2015-01-05 15:57:17.000000000 +0000 @@ -0,0 +1,69 @@ +'''cr_bin_path.py: click bin-path''' +# +# Copyright (C) 2014 Canonical Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 3 of the License. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +from __future__ import print_function + +from clickreviews.cr_common import ClickReview +import os + + +class ClickReviewBinPath(ClickReview): + '''This class represents click lint reviews''' + def __init__(self, fn): + peer_hooks = dict() + my_hook = 'bin-path' + peer_hooks[my_hook] = dict() + peer_hooks[my_hook]['required'] = ["snappy-systemd", "apparmor"] + peer_hooks[my_hook]['allowed'] = peer_hooks[my_hook]['required'] + + ClickReview.__init__(self, fn, "bin-path", peer_hooks=peer_hooks) + + self.bin_paths = dict() + for app in self.manifest['hooks']: + if 'bin-path' not in self.manifest['hooks'][app]: + # msg("Skipped missing bin-path hook for '%s'" % app) + continue + self.bin_paths[app] = self._extract_bin_path(app) + + def _extract_bin_path(self, app): + '''Get bin-path for app''' + rel = self.manifest['hooks'][app]['bin-path'] + fn = os.path.join(self.unpack_dir, rel) + if not os.path.exists(fn): + error("Could not find '%s'" % rel) + return fn + + def _check_bin_path_executable(self, app): + '''Check that the provided path exists''' + rel = self.manifest['hooks'][app]['bin-path'] + fn = os.path.join(self.unpack_dir, rel) + return os.access(fn, os.X_OK) + + def check_path(self): + '''Check path exists''' + t = 'info' + n = 'path exists' + s = "OK" + + for app in sorted(self.bin_paths): + t = 'info' + n = 'path executable' + s = "OK" + if not self._check_bin_path_executable(app): + t = 'error' + s = "'%s' is not executable" % \ + (self.manifest['hooks'][app]['bin-path']) + self._add_result(t, n, s) diff -Nru click-reviewers-tools-0.19/bin/clickreviews/cr_common.py click-reviewers-tools-0.20/bin/clickreviews/cr_common.py --- click-reviewers-tools-0.19/bin/clickreviews/cr_common.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/bin/clickreviews/cr_common.py 2015-01-05 15:57:17.000000000 +0000 @@ -57,10 +57,33 @@ class ClickReview(object): '''This class represents click reviews''' - def __init__(self, fn, review_type): + # Convenience to break out common types of clicks (eg, app, scope, + # framework, click service) + app_allowed_peer_hooks = ["account-application", + "account-service", + "apparmor", + "content-hub", + "desktop", + "push-helper", + "urls", + ] + scope_allowed_peer_hooks = ["account-application", + "account-service", + "apparmor", + "scope", + ] + # FIXME: when apparmor-policy is implemented, use this + # framework_allowed_peer_hooks = ["apparmor-policy"] + framework_allowed_peer_hooks = [] + service_allowed_peer_hooks = ["bin-path", + "snappy-systemd", + ] + + def __init__(self, fn, review_type, peer_hooks=None): self.click_package = fn self._check_path_exists() - if not self.click_package.endswith(".click"): + if not self.click_package.endswith(".click") and \ + not self.click_package.endswith(".snap"): if self.click_package.endswith(".deb"): error("filename does not end with '.click', but '.deb' " "instead. See http://askubuntu.com/a/485544/94326 for " @@ -114,6 +137,8 @@ self.valid_frameworks = self._extract_click_frameworks() + self.peer_hooks = peer_hooks + def _extract_click_frameworks(self): '''Extract installed click frameworks''' # TODO: update to use libclick API when available @@ -179,6 +204,8 @@ optional = ["title", "description", "maintainer", "architecture", "installed-size", "icon"] + snappy_optional = ["ports"] + for f in optional: if f in self.manifest: if f != "architecture" and \ @@ -208,7 +235,7 @@ error("manifest malformed: hooks/%s is empty:\n%s" % (app, mp)) for k in sorted(self.manifest): - if k not in required + optional + ['hooks']: + if k not in required + optional + snappy_optional + ['hooks']: # click supports local extensions via 'x-...', ignore those # here but report in lint if k.startswith('x-'): @@ -216,6 +243,72 @@ error("manifest malformed: unsupported field '%s':\n%s" % (k, mp)) + def _verify_peer_hooks(self, my_hook): + '''Compare manifest for required and allowed hooks''' + d = dict() + if self.peer_hooks is None: + return d + + for app in self.manifest["hooks"]: + if my_hook not in self.manifest["hooks"][app]: + continue + for h in self.peer_hooks[my_hook]['required']: + if h == my_hook: + continue + if h not in self.manifest["hooks"][app]: + if 'missing' not in d: + d['missing'] = dict() + if app not in d['missing']: + d['missing'][app] = [] + d['missing'][app].append(h) + for h in self.manifest["hooks"][app]: + if h == my_hook: + continue + if h not in self.peer_hooks[my_hook]['allowed']: + if 'disallowed' not in d: + d['disallowed'] = dict() + if app not in d['disallowed']: + d['disallowed'][app] = [] + d['disallowed'][app].append(h) + + return d + + def check_peer_hooks(self, hooks_sublist=[]): + '''Check if peer hooks are valid''' + # Nothing to verify + if self.peer_hooks is None: + return + + for hook in self.peer_hooks: + if len(hooks_sublist) > 0 and hook not in hooks_sublist: + continue + d = self._verify_peer_hooks(hook) + t = 'info' + n = "peer_hooks_required_%s" % hook + s = "OK" + + if 'missing' in d and len(d['missing'].keys()) > 0: + t = 'error' + for app in d['missing']: + s = "Missing required hooks for '%s': %s" % ( + app, ", ".join(d['missing'][app])) + self._add_result(t, n, s, manual_review=True) + else: + self._add_result(t, n, s) + + t = 'info' + n = "peer_hooks_disallowed_with_%s" % hook + s = "OK" + + if 'disallowed' in d and len(d['disallowed'].keys()) > 0: + t = 'error' + for app in d['disallowed']: + s = "Disallowed with %s (%s): %s" % ( + hook, app, ", ".join(d['disallowed'][app])) + self._add_result(t, n, s, manual_review=True) + else: + self._add_result(t, n, s) + def set_review_type(self, name): '''Set review name''' self.review_type = name diff -Nru click-reviewers-tools-0.19/bin/clickreviews/cr_content_hub.py click-reviewers-tools-0.20/bin/clickreviews/cr_content_hub.py --- click-reviewers-tools-0.19/bin/clickreviews/cr_content_hub.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/bin/clickreviews/cr_content_hub.py 2015-01-05 15:57:17.000000000 +0000 @@ -24,7 +24,13 @@ class ClickReviewContentHub(ClickReview): '''This class represents click lint reviews''' def __init__(self, fn): - ClickReview.__init__(self, fn, "content_hub") + peer_hooks = dict() + my_hook = 'content-hub' + peer_hooks[my_hook] = dict() + peer_hooks[my_hook]['allowed'] = ClickReview.app_allowed_peer_hooks + peer_hooks[my_hook]['required'] = [] + + ClickReview.__init__(self, fn, "content_hub", peer_hooks=peer_hooks) self.valid_keys = ['destination', 'share', 'source'] diff -Nru click-reviewers-tools-0.19/bin/clickreviews/cr_desktop.py click-reviewers-tools-0.20/bin/clickreviews/cr_desktop.py --- click-reviewers-tools-0.19/bin/clickreviews/cr_desktop.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/bin/clickreviews/cr_desktop.py 2015-01-05 15:57:17.000000000 +0000 @@ -29,42 +29,21 @@ class ClickReviewDesktop(ClickReview): '''This class represents click lint reviews''' def __init__(self, fn): - ClickReview.__init__(self, fn, "desktop") + peer_hooks = dict() + my_hook = 'desktop' + peer_hooks[my_hook] = dict() + peer_hooks[my_hook]['allowed'] = ClickReview.app_allowed_peer_hooks + peer_hooks[my_hook]['required'] = ["apparmor"] + + ClickReview.__init__(self, fn, "desktop", peer_hooks=peer_hooks) self.desktop_files = dict() # click-show-files and a couple tests self.desktop_entries = dict() self.desktop_hook_entries = 0 for app in self.manifest['hooks']: if 'desktop' not in self.manifest['hooks'][app]: - if 'scope' in self.manifest['hooks'][app]: - # msg("Skipped missing desktop hook for scope '%s'" % app) - continue - elif 'push-helper' in self.manifest['hooks'][app]: - # msg("Skipped missing desktop hook for push-helper '%s'" % - # app) - continue - elif 'pay-ui' in self.manifest['hooks'][app]: - # msg("Skipped missing desktop hook for pay-ui '%s'" % app) - continue - elif 'account-provider' in self.manifest['hooks'][app]: - # msg("Skipped missing desktop hook for account-provider" - # " '%s'" % app) - continue - elif 'account-qml-plugin' in self.manifest['hooks'][app]: - # msg("Skipped missing desktop hook for account-qml-plugin" - # " '%s'" % app) - continue - elif 'systemd' in self.manifest['hooks'][app]: - # msg("Skipped missing desktop hook for systemd" - # " '%s'" % app) - continue - elif 'framework' in self.manifest['hooks'][app]: - # TODO: verify this is the long term hook name - # msg("Skipped missing desktop hook for framework" - # " '%s'" % app) - continue - else: - error("could not find desktop hook for '%s'" % app) + # msg("Skipped missing desktop hook for '%s'" % app) + continue if not isinstance(self.manifest['hooks'][app]['desktop'], str): error("manifest malformed: hooks/%s/desktop is not str" % app) self.desktop_hook_entries += 1 diff -Nru click-reviewers-tools-0.19/bin/clickreviews/cr_framework.py click-reviewers-tools-0.20/bin/clickreviews/cr_framework.py --- click-reviewers-tools-0.19/bin/clickreviews/cr_framework.py 1970-01-01 00:00:00.000000000 +0000 +++ click-reviewers-tools-0.20/bin/clickreviews/cr_framework.py 2015-01-05 15:57:17.000000000 +0000 @@ -0,0 +1,130 @@ +'''cr_framework.py: click framework''' +# +# Copyright (C) 2014 Canonical Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 3 of the License. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +from __future__ import print_function + +from clickreviews.cr_common import ClickReview, open_file_read +import os + + +class ClickReviewFramework(ClickReview): + '''This class represents click lint reviews''' + def __init__(self, fn): + peer_hooks = dict() + my_hook = 'framework' + peer_hooks[my_hook] = dict() + peer_hooks[my_hook]['allowed'] = ClickReview.framework_allowed_peer_hooks + peer_hooks[my_hook]['required'] = peer_hooks[my_hook]['allowed'] + ClickReview.__init__(self, fn, "framework", peer_hooks=peer_hooks) + + self.frameworks_file = dict() + self.frameworks = dict() + for app in self.manifest['hooks']: + if 'framework' not in self.manifest['hooks'][app]: + # msg("Skipped missing framework hook for '%s'" % app) + continue + if not isinstance(self.manifest['hooks'][app]['framework'], str): + error("manifest malformed: hooks/%s/framework is not str" % + app) + (full_fn, data) = self._extract_framework(app) + self.frameworks_file[app] = full_fn + self.frameworks[app] = data + + def _extract_framework(self, app): + '''Get framework for app''' + rel = self.manifest['hooks'][app]['framework'] + fn = os.path.join(self.unpack_dir, rel) + if not os.path.exists(fn): + error("Could not find '%s'" % rel) + + data = dict() + fh = open_file_read(fn) + for line in fh.readlines(): + tmp = line.split(':') + if len(tmp) != 2: + continue + data[tmp[0].strip()] = tmp[1].strip() + fh.close() + + return (fn, data) + + def check_single_framework(self): + '''Check only have one framework in the click''' + t = 'info' + n = 'single_framework' + s = "OK" + if len(self.frameworks.keys()) > 1: + t = 'error' + s = 'framework hook specified multiple times' + self._add_result(t, n, s) + + def check_framework_base_name(self): + '''Check framework Base-Name''' + for app in sorted(self.frameworks): + t = 'info' + n = "base_name_present '%s'" % app + s = "OK" + if 'Base-Name' not in self.frameworks[app]: + t = 'error' + s = "Could not find 'Base-Name' in '%s'" % \ + (self.frameworks_file[app]) + self._add_result(t, n, s) + continue + self._add_result(t, n, s) + + t = 'info' + n = "base_name_namespacing '%s'" % app + s = "OK" + if self.frameworks[app]['Base-Name'] != self.manifest['name']: + t = 'error' + s = "'%s' != '%s'" % (self.frameworks[app]['Base-Name'], + self.manifest['name']) + self._add_result(t, n, s) + + def check_framework_base_version(self): + '''Check framework Base-Version''' + for app in sorted(self.frameworks): + t = 'info' + n = "base_version_present '%s'" % app + s = "OK" + if 'Base-Version' not in self.frameworks[app]: + t = 'error' + s = "Could not find 'Base-Version' in '%s'" % \ + (self.frameworks_file[app]) + self._add_result(t, n, s) + continue + self._add_result(t, n, s) + + v = self.frameworks[app]['Base-Version'] + t = 'info' + n = "base_version_number '%s'" % app + s = "OK" + try: + float(v) + except ValueError: + t = 'error' + s = "'Base-Version' malformed: '%s' is not a number" % v + self._add_result(t, n, s) + continue + self._add_result(t, n, s) + + t = 'info' + n = "base_version_positive '%s'" % app + s = "OK" + if float(v) < 0: + t = 'error' + s = "'Base-Version' malformed: '%s' is negative" % v + self._add_result(t, n, s) diff -Nru click-reviewers-tools-0.19/bin/clickreviews/cr_lint.py click-reviewers-tools-0.20/bin/clickreviews/cr_lint.py --- click-reviewers-tools-0.19/bin/clickreviews/cr_lint.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/bin/clickreviews/cr_lint.py 2015-01-05 15:57:17.000000000 +0000 @@ -86,14 +86,19 @@ 'account-qml-plugin', 'account-service', 'apparmor', + 'bin-path', 'content-hub', 'desktop', + 'framework', 'pay-ui', 'push-helper', 'scope', + 'snappy-systemd', 'urls'] - self.redflagged_hooks = ['pay-ui'] + self.redflagged_hooks = ['framework', + 'pay-ui', + ] if overrides is None: overrides = {} self.overrides = overrides @@ -608,6 +613,10 @@ "(Your email domain needs to match the reverse package " \ "namespace.)" % (self.email, ".".join(pkg_domain_rev)) + # flat namespaces are ok now + if self.click_pkgname.count('.') < 1: + t = 'info' + s = 'OK (flat namespace)' self._add_result(t, n, s, manual_review=manual_review) def check_title(self): @@ -717,17 +726,23 @@ os.path.basename(self.click_package) self._add_result(t, n, s) - # handle $pkgname.click - pkgname = tmp[0].partition('\.click')[0] + # handle $pkgname.click or $pkgname.snap + if tmp[0].endswith('.snap'): + pkgname = tmp[0].partition('.snap')[0] + else: + pkgname = tmp[0].partition('.click')[0] t = 'info' n = 'package_filename_pkgname_match' s = 'OK' l = None if pkgname != self.click_pkgname: - t = 'error' - s = "'%s' != '%s' from DEBIAN/control" % (pkgname, - self.click_pkgname) - l = 'http://askubuntu.com/questions/417361/what-does-lint-package-filename-pkgname-match-mean' + if pkgname.startswith('com.ubuntu.snappy.'): + s = "OK (store snappy workaround)" + else: + t = 'error' + s = "'%s' != '%s' from DEBIAN/control" % (pkgname, + self.click_pkgname) + l = 'http://askubuntu.com/questions/417361/what-does-lint-package-filename-pkgname-match-mean' self._add_result(t, n, s, l) # check if namespaces matches with filename @@ -748,8 +763,11 @@ s = 'OK' l = None if len(tmp) >= 2: - # handle $pkgname_$version.click - version = tmp[1].partition('.click')[0] + # handle $pkgname_$version.click or $pkgname_$version.snap + if tmp[0].endswith('.snap'): + version = tmp[1].partition('.snap')[0] + else: + version = tmp[1].partition('.click')[0] if version != self.click_version: t = 'error' s = "'%s' != '%s' from DEBIAN/control" % (version, @@ -765,7 +783,10 @@ n = 'package_filename_arch_valid' s = 'OK' if len(tmp) >= 3: - arch = tmp[2].partition('.click')[0] + if tmp[0].endswith('.snap'): + arch = tmp[2].partition('.snap')[0] + else: + arch = tmp[2].partition('.click')[0] if arch == "unknown": # short-circuit here since the appstore doesn't determine # the version yet @@ -786,7 +807,10 @@ n = 'package_filename_arch_match' s = 'OK' if len(tmp) >= 3: - arch = tmp[2].partition('.click')[0] + if tmp[0].endswith('.snap'): + arch = tmp[2].partition('.snap')[0] + else: + arch = tmp[2].partition('.click')[0] if arch != self.click_arch: t = 'error' s = "'%s' != '%s' from DEBIAN/control" % (arch, diff -Nru click-reviewers-tools-0.19/bin/clickreviews/cr_online_accounts.py click-reviewers-tools-0.20/bin/clickreviews/cr_online_accounts.py --- click-reviewers-tools-0.19/bin/clickreviews/cr_online_accounts.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/bin/clickreviews/cr_online_accounts.py 2015-01-05 15:57:17.000000000 +0000 @@ -25,7 +25,36 @@ class ClickReviewAccounts(ClickReview): '''This class represents click lint reviews''' def __init__(self, fn): - ClickReview.__init__(self, fn, "online_accounts") + peer_hooks = dict() + peer_hooks['account-application'] = dict() + peer_hooks['account-application']['allowed'] = \ + ClickReview.app_allowed_peer_hooks + \ + ClickReview.scope_allowed_peer_hooks + peer_hooks['account-application']['required'] = ['apparmor'] + + peer_hooks['account-service'] = dict() + peer_hooks['account-service']['required'] = ['account-application', + 'apparmor' + ] + peer_hooks['account-service']['allowed'] = \ + ClickReview.app_allowed_peer_hooks + \ + ClickReview.scope_allowed_peer_hooks + + peer_hooks['account-provider'] = dict() + peer_hooks['account-provider']['required'] = ['account-qml-plugin', + # 'apparmor' + ] + peer_hooks['account-provider']['allowed'] = \ + peer_hooks['account-provider']['required'] + + peer_hooks['account-qml-plugin'] = dict() + peer_hooks['account-qml-plugin']['required'] = ['account-provider', + # 'apparmor' + ] + peer_hooks['account-qml-plugin']['allowed'] = \ + peer_hooks['account-qml-plugin']['required'] + + ClickReview.__init__(self, fn, "online_accounts", peer_hooks) self.accounts_files = dict() self.accounts = dict() @@ -88,29 +117,6 @@ self._add_result(t, n, s) continue - # account-application must always appear with apparmor - t = 'info' - n = '%s_%s_apparmor' % (app, account_type) - s = "OK" - if 'apparmor' not in self.manifest['hooks'][app]: - t = 'error' - s = "missing 'apparmor' entry" - self._add_result(t, n, s) - - # account-application must always appear with desktop or scope - t = 'info' - n = '%s_%s_desktop_or_scope' % (app, account_type) - s = "OK" - found = False - for k in ['desktop', 'scope']: - if k in self.manifest['hooks'][app]: - found = True - break - if not found and 'pay-ui' not in self.manifest['hooks'][app]: - t = 'error' - s = "missing 'desktop' or 'scope' entry" - self._add_result(t, n, s) - root_tag = self.accounts[app][account_type].tag.lower() if root_tag != "application": t = 'error' @@ -159,24 +165,6 @@ self._add_result(t, n, s) continue - # account service must always appear with account-application - t = 'info' - n = '%s_%s_account-application' % (app, account_type) - s = "OK" - if 'account-application' not in self.accounts[app]: - t = 'error' - s = "missing 'account-application' entry" - self._add_result(t, n, s) - - # account-service must always appear with apparmor - t = 'info' - n = '%s_%s_apparmor' % (app, account_type) - s = "OK" - if 'apparmor' not in self.manifest['hooks'][app]: - t = 'error' - s = "missing 'apparmor' entry" - self._add_result(t, n, s) - root_tag = self.accounts[app][account_type].tag.lower() if root_tag != "service": t = 'error' @@ -223,40 +211,6 @@ manual_review = True self._add_result(t, n, s, manual_review=manual_review) - # account-provider must always appear with apparmor -# t = 'info' -# n = '%s_%s_apparmor' % (app, account_type) -# s = "OK" -# if 'apparmor' not in self.manifest['hooks'][app]: -# t = 'error' -# s = "missing 'apparmor' entry" -# self._add_result(t, n, s) - - # account-provider must always appear with account-qml-plugin - t = 'info' - n = '%s_%s_account-qml-plugin' % (app, account_type) - s = "OK" - if 'account-qml-plugin' not in self.accounts[app]: - t = 'error' - s = "missing 'account-qml-plugin' entry" - self._add_result(t, n, s) - - # account-provider must never appear with account-application or - # account-service - t = 'info' - n = '%s_%s_account-application_or_account-service' % (app, - account_type) - s = "OK" - found = False - for i in ['account-application', 'account-service']: - if i in self.accounts[app]: - found = True - if found: - t = 'error' - s = "must not specify account-application or account-service" \ - " with %s" % account_type - self._add_result(t, n, s) - t = 'info' n = '%s_%s_root' % (app, account_type) s = "OK" @@ -293,38 +247,3 @@ s = "(MANUAL REVIEW) '%s' not allowed" % account_type manual_review = True self._add_result(t, n, s, manual_review=manual_review) - - # account-qml-plugin must always appear with apparmor -# t = 'info' -# n = '%s_%s_apparmor' % (app, account_type) -# s = "OK" -# if 'apparmor' not in self.manifest['hooks'][app]: -# t = 'error' -# t = 'info' -# s = "missing 'apparmor' entry" -# self._add_result(t, n, s) - - # account-qml-plugin must always appear with account-provider - t = 'info' - n = '%s_%s_account-provider' % (app, account_type) - s = "OK" - if 'account-provider' not in self.accounts[app]: - t = 'error' - s = "missing 'account-provider' entry" - self._add_result(t, n, s) - - # account-qml-plugin must never appear with account-application or - # account-service - t = 'info' - n = '%s_%s_account-application_or_account-service' % (app, - account_type) - s = "OK" - found = False - for i in ['account-application', 'account-service']: - if i in self.accounts[app]: - found = True - if found: - t = 'error' - s = "must not specify account-application or account-service" \ - " with %s" % account_type - self._add_result(t, n, s) diff -Nru click-reviewers-tools-0.19/bin/clickreviews/cr_push_helper.py click-reviewers-tools-0.20/bin/clickreviews/cr_push_helper.py --- click-reviewers-tools-0.19/bin/clickreviews/cr_push_helper.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/bin/clickreviews/cr_push_helper.py 2015-01-05 15:57:17.000000000 +0000 @@ -24,7 +24,13 @@ class ClickReviewPushHelper(ClickReview): '''This class represents click lint reviews''' def __init__(self, fn): - ClickReview.__init__(self, fn, "push_helper") + peer_hooks = dict() + my_hook = 'push-helper' + peer_hooks[my_hook] = dict() + peer_hooks[my_hook]['allowed'] = ['apparmor'] + peer_hooks[my_hook]['required'] = ['apparmor'] + + ClickReview.__init__(self, fn, "push_helper", peer_hooks=peer_hooks) self.required_keys = ['exec'] self.optional_keys = ['app_id'] @@ -36,7 +42,8 @@ # msg("Skipped missing push-helper hook for '%s'" % app) continue if not isinstance(self.manifest['hooks'][app]['push-helper'], str): - error("manifest malformed: hooks/%s/urls is not str" % app) + error("manifest malformed: hooks/%s/push-helper is not str" % + app) (full_fn, jd) = self._extract_push_helper(app) self.push_helper_files[app] = full_fn self.push_helper[app] = jd diff -Nru click-reviewers-tools-0.19/bin/clickreviews/cr_scope.py click-reviewers-tools-0.20/bin/clickreviews/cr_scope.py --- click-reviewers-tools-0.19/bin/clickreviews/cr_scope.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/bin/clickreviews/cr_scope.py 2015-01-05 15:57:17.000000000 +0000 @@ -29,7 +29,13 @@ class ClickReviewScope(ClickReview): '''This class represents click lint reviews''' def __init__(self, fn): - ClickReview.__init__(self, fn, "scope") + peer_hooks = dict() + my_hook = 'scope' + peer_hooks[my_hook] = dict() + peer_hooks[my_hook]['allowed'] = ClickReview.scope_allowed_peer_hooks + peer_hooks[my_hook]['required'] = ['apparmor'] + + ClickReview.__init__(self, fn, "scope", peer_hooks=peer_hooks) self.scopes = dict() for app in self.manifest['hooks']: diff -Nru click-reviewers-tools-0.19/bin/clickreviews/cr_security.py click-reviewers-tools-0.20/bin/clickreviews/cr_security.py --- click-reviewers-tools-0.19/bin/clickreviews/cr_security.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/bin/clickreviews/cr_security.py 2015-01-05 15:57:17.000000000 +0000 @@ -26,7 +26,17 @@ class ClickReviewSecurity(ClickReview): '''This class represents click lint reviews''' def __init__(self, fn, overrides=None): - ClickReview.__init__(self, fn, "security") + peer_hooks = dict() + my_hook = 'apparmor' + peer_hooks[my_hook] = dict() + # Basically, everything except frameworks + peer_hooks[my_hook]['allowed'] = ClickReview.app_allowed_peer_hooks + \ + ClickReview.scope_allowed_peer_hooks + \ + ClickReview.service_allowed_peer_hooks + \ + ['pay-ui'] + peer_hooks[my_hook]['required'] = [] + + ClickReview.__init__(self, fn, "security", peer_hooks=peer_hooks) local_copy = os.path.join(os.path.dirname(__file__), '../data/apparmor-easyprof-ubuntu.json') @@ -72,8 +82,9 @@ self.allowed_network_scope_policy_groups = ['accounts', 'networking'] self.redflag_templates = ['unconfined'] - self.extraneous_templates = ['ubuntu-sdk', - 'default'] + # TODO: how to deal with other vendors + self.extraneous_ubuntu_templates = ['ubuntu-sdk', + 'default'] # framework policy is based on major framework version. In 13.10, there # was only 'ubuntu-sdk-13.10', but in 14.04, there will be several, @@ -98,14 +109,8 @@ self.security_apps = [] for app in self.manifest['hooks']: if 'apparmor' not in self.manifest['hooks'][app]: - # TODO: when we have apparmor templates for these, we want - # to remove this - if 'account-provider' in self.manifest['hooks'][app] or \ - 'account-qml-plugin' in self.manifest['hooks'][app]: - # msg("Skipped missing desktop hook for account-provider " - # "and account-qml-plugin '%s'" % app) - continue - error("could not find apparmor hook for '%s'" % app) + # msg("Skipped missing apparmor hook for '%s'" % app) + continue if not isinstance(self.manifest['hooks'][app]['apparmor'], str): error("manifest malformed: hooks/%s/apparmor is not str" % app) rel_fn = self.manifest['hooks'][app]['apparmor'] @@ -256,7 +261,8 @@ t = 'info' n = 'policy_vendor (%s)' % f s = "OK" - if 'policy_vendor' in m and m['policy_vendor'] != "ubuntu": + if 'policy_vendor' in m and \ + m['policy_vendor'] not in self.aa_policy: t = 'error' s = "policy_vendor '%s' not found" % m['policy_vendor'] self._add_result(t, n, s) @@ -288,10 +294,12 @@ t = 'info' n = 'policy_version_is_highest (%s, %s)' % (str(highest), f) s = "OK" + l = None if float(m['policy_version']) != highest: t = 'info' + l = 'http://askubuntu.com/q/562116/94326' s = '%s != %s' % (str(m['policy_version']), str(highest)) - self._add_result(t, n, s) + self._add_result(t, n, s, l) t = 'info' n = 'policy_version_matches_framework (%s)' % (f) @@ -338,7 +346,8 @@ t = 'error' s = "(MANUAL REVIEW) '%s' not allowed" % m['template'] manual_review = True - elif m['template'] in self.extraneous_templates: + elif ('policy_vendor' not in m or m['policy_vendor'] == 'ubuntu') \ + and m['template'] in self.extraneous_ubuntu_templates: t = 'warn' s = "No need to specify '%s' template" % m['template'] self._add_result(t, n, s, manual_review=manual_review) @@ -548,6 +557,7 @@ t = 'info' n = 'policy_groups_safe_%s (%s)' % (app, i) s = 'OK' + l = None manual_review = False aa_type = self._get_policy_group_type(vendor, version, i) @@ -559,12 +569,14 @@ t = 'error' s = "(MANUAL REVIEW) %s policy group " % aa_type + \ "'%s': vetted applications only" % (i) + if i == "debug": + l = 'http://askubuntu.com/a/562123/94326' manual_review = True elif aa_type != "common": t = 'error' s = "policy group '%s' has" % i + \ "unknown type '%s'" % (aa_type) - self._add_result(t, n, s, manual_review=manual_review) + self._add_result(t, n, s, l, manual_review=manual_review) def check_ignored(self): '''Check ignored fields''' @@ -595,6 +607,9 @@ found = [] for i in self.redflag_fields: if i in m: + if i == 'policy_vendor' and \ + m[i] in ['ubuntu', 'ubuntu-snappy']: + continue found.append(i) if len(found) > 0: diff -Nru click-reviewers-tools-0.19/bin/clickreviews/cr_skeleton.py click-reviewers-tools-0.20/bin/clickreviews/cr_skeleton.py --- click-reviewers-tools-0.19/bin/clickreviews/cr_skeleton.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/bin/clickreviews/cr_skeleton.py 2015-01-05 15:57:17.000000000 +0000 @@ -22,7 +22,19 @@ class ClickReviewSkeleton(ClickReview): '''This class represents click lint reviews''' def __init__(self, fn): - ClickReview.__init__(self, fn, "skeleton") + # Many test classes are for verify click hooks. 'peer_hooks' is used + # to declare what hooks may be use with my_hook. When using this + # mechanism, ClickReview.check_peer_hooks() is run for you. + peer_hooks = dict() + my_hook = 'skeleton' + peer_hooks[my_hook] = dict() + peer_hooks[my_hook]['allowed'] = ["desktop", "apparmor", "urls"] + peer_hooks[my_hook]['required'] = ["desktop", "apparmor"] + + ClickReview.__init__(self, fn, "skeleton", peer_hooks) + + # If not a hooks test, skip the above and omit peer_hooks like so: + # ClickReview.__init__(self, fn, "skeleton") def check_foo(self): '''Check foo''' diff -Nru click-reviewers-tools-0.19/bin/clickreviews/cr_tests.py click-reviewers-tools-0.20/bin/clickreviews/cr_tests.py --- click-reviewers-tools-0.19/bin/clickreviews/cr_tests.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/bin/clickreviews/cr_tests.py 2015-01-05 15:57:17.000000000 +0000 @@ -40,6 +40,9 @@ TEST_ACCOUNTS_QML_PLUGIN = dict() TEST_ACCOUNTS_SERVICE = dict() TEST_PUSH_HELPER = dict() +TEST_BIN_PATH = dict() +TEST_FRAMEWORK = dict() +TEST_SNAPPY_SYSTEMD = dict() # @@ -94,7 +97,7 @@ def _get_security_supported_policy_versions(self): '''Pretend we read the contens of /usr/share/apparmor/easyprof''' - return [1.0, 1.1, 1.2] + return [1.0, 1.1, 1.2, 1.3] def _extract_desktop_entry(self, app): @@ -154,6 +157,28 @@ return ("%s.push-helper.json" % app, TEST_PUSH_HELPER[app]) +def _extract_bin_path(self, app): + '''Pretend we found the bin-path file''' + return ("%s" % TEST_BIN_PATH[app]) + + +def _check_bin_path_executable(self, app): + '''Pretend we found the bin-path file''' + if TEST_BIN_PATH[app].endswith('.nonexec'): + return False + return True + + +def _extract_framework(self, app): + '''Pretend we found the framework file''' + return ("%s.framework" % app, TEST_FRAMEWORK[app]) + + +def _extract_systemd(self, app): + '''Pretend we found the systemd file''' + return ("%s.click-service" % app, TEST_SNAPPY_SYSTEMD[app]) + + # http://docs.python.org/3.4/library/unittest.mock-examples.html # Mock patching. Don't use decorators but instead patch in setUp() of the # child. Set up a list of patches, but don't start them. Create the helper @@ -239,6 +264,24 @@ 'clickreviews.cr_push_helper.ClickReviewPushHelper._extract_push_helper', _extract_push_helper)) +# bin-path overrides +patches.append(patch( + 'clickreviews.cr_bin_path.ClickReviewBinPath._extract_bin_path', + _extract_bin_path)) +patches.append(patch( + 'clickreviews.cr_bin_path.ClickReviewBinPath._check_bin_path_executable', + _check_bin_path_executable)) + +# framework overrides +patches.append(patch( + 'clickreviews.cr_framework.ClickReviewFramework._extract_framework', + _extract_framework)) + +# systemd overrides +patches.append(patch( + 'clickreviews.cr_systemd.ClickReviewSystemd._extract_systemd', + _extract_systemd)) + def mock_patch(): '''Call in setup of child''' @@ -307,6 +350,9 @@ self.test_accounts_qml_plugin = dict() self.test_accounts_service = dict() self.test_push_helper = dict() + self.test_bin_path = dict() + self.test_framework = dict() + self.test_systemd = dict() for app in self.test_manifest["hooks"].keys(): # setup security manifest for each app self.set_test_security_manifest(app, 'policy_groups', @@ -336,15 +382,24 @@ # Reset to no content-hub entries in manifest self.set_test_content_hub(app, None, None) - # Reset to no content-hub entries in manifest + # Reset to no accounts entries in manifest self.set_test_account(app, "account-application", None) self.set_test_account(app, "account-provider", None) self.set_test_account(app, "account-qml-plugin", None) self.set_test_account(app, "account-service", None) - # Reset to no content-hub entries in manifest + # Reset to no push-helper entries in manifest self.set_test_push_helper(app, None, None) + # Reset to no bin-path entries in manifest + self.set_test_bin_path(app, None) + + # Reset to no framework entries in manifest + self.set_test_framework(app, None, None) + + # Reset to no systemd entries in manifest + self.set_test_systemd(app, None, None) + self._update_test_security_manifests() self._update_test_desktop_files() self._update_test_url_dispatcher() @@ -355,6 +410,9 @@ self._update_test_accounts_qml_plugin() self._update_test_accounts_service() self._update_test_push_helper() + self._update_test_bin_path() + self._update_test_framework() + self._update_test_systemd() # webapps manifests (leave empty for now) self.test_webapp_manifests = dict() @@ -469,6 +527,32 @@ "%s.push-helper.json" % app self._update_test_manifest() + def _update_test_bin_path(self): + global TEST_BIN_PATH + TEST_BIN_PATH = dict() + for app in self.test_bin_path.keys(): + TEST_BIN_PATH[app] = self.test_bin_path[app] + self.test_manifest["hooks"][app]["bin-path"] = \ + "%s" % TEST_BIN_PATH[app] + self._update_test_manifest() + + def _update_test_framework(self): + global TEST_FRAMEWORK + TEST_FRAMEWORK = dict() + for app in self.test_framework.keys(): + TEST_FRAMEWORK[app] = self.test_framework[app] + if app not in self.test_manifest["hooks"]: + self.test_manifest["hooks"][app] = dict() + self.test_manifest["hooks"][app]["framework"] = \ + "%s.framework" % TEST_FRAMEWORK[app] + self._update_test_manifest() + + def _update_test_systemd(self): + global TEST_SNAPPY_SYSTEMD + TEST_SNAPPY_SYSTEMD = dict() + for app in self.test_systemd.keys(): + TEST_SNAPPY_SYSTEMD[app] = self.test_systemd[app] + def _update_test_name(self): self.test_name = "%s_%s_%s.click" % (self.test_control['Package'], self.test_control['Version'], @@ -650,7 +734,7 @@ def set_test_push_helper(self, app, key, value): '''Set push-helper entries. If value is None, remove key, if key is - None, remove content-hub from manifest''' + None, remove push-helper from manifest''' if key is None: if app in self.test_push_helper: self.test_push_helper.pop(app) @@ -663,6 +747,46 @@ self.test_push_helper[app][key] = value self._update_test_push_helper() + def set_test_bin_path(self, app, value): + '''Set bin-path entries. If value is None, remove bin-path from + manifest''' + if value is None: + if app in self.test_bin_path: + self.test_bin_path.pop(app) + else: + if app not in self.test_bin_path: + self.test_bin_path[app] = dict() + self.test_bin_path[app] = value + self._update_test_bin_path() + + def set_test_framework(self, app, key, value): + '''Set framework entries. If value is None, remove key, if key is + None, remove framework from manifest''' + if key is None: + if app in self.test_framework: + self.test_framework.pop(app) + elif value is None: + if key in self.test_framework[app]: + self.test_framework[app].pop(key) + else: + if app not in self.test_framework: + self.test_framework[app] = dict() + self.test_framework[app][key] = value + self._update_test_framework() + + def set_test_systemd(self, app, key, value, append=False): + '''Set systemd entries. If value is None, remove''' + if app not in self.test_systemd: + self.test_systemd[app] = [] + + if value is None: + self.test_systemd[app] = [] + else: + if not append: + self.test_systemd[app] = [] + self.test_systemd[app].append({key: value}) + self._update_test_systemd() + def setUp(self): '''Make sure our patches are applied everywhere''' global patches @@ -695,6 +819,12 @@ TEST_ACCOUNTS_APPLICATION = dict() global TEST_PUSH_HELPER TEST_PUSH_HELPER = dict() + global TEST_BIN_PATH + TEST_BIN_PATH = dict() + global TEST_FRAMEWORK + TEST_FRAMEWORK = dict() + global TEST_SNAPPY_SYSTEMD + TEST_SNAPPY_SYSTEMD = dict() self._reset_test_data() cr_common.recursive_rm(self.desktop_tmpdir) diff -Nru click-reviewers-tools-0.19/bin/clickreviews/cr_url_dispatcher.py click-reviewers-tools-0.20/bin/clickreviews/cr_url_dispatcher.py --- click-reviewers-tools-0.19/bin/clickreviews/cr_url_dispatcher.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/bin/clickreviews/cr_url_dispatcher.py 2015-01-05 15:57:17.000000000 +0000 @@ -26,7 +26,13 @@ class ClickReviewUrlDispatcher(ClickReview): '''This class represents click lint reviews''' def __init__(self, fn): - ClickReview.__init__(self, fn, "url_dispatcher") + peer_hooks = dict() + my_hook = 'urls' + peer_hooks[my_hook] = dict() + peer_hooks[my_hook]['allowed'] = ClickReview.app_allowed_peer_hooks + peer_hooks[my_hook]['required'] = [] + + ClickReview.__init__(self, fn, "url_dispatcher", peer_hooks=peer_hooks) self.required_keys = ['protocol'] self.optional_keys = ['domain-suffix'] diff -Nru click-reviewers-tools-0.19/bin/clickreviews/tests/test_aaa_example_cr_skeleton.py click-reviewers-tools-0.20/bin/clickreviews/tests/test_aaa_example_cr_skeleton.py --- click-reviewers-tools-0.19/bin/clickreviews/tests/test_aaa_example_cr_skeleton.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/bin/clickreviews/tests/test_aaa_example_cr_skeleton.py 2015-01-05 15:57:17.000000000 +0000 @@ -81,3 +81,75 @@ == Mock manifest ==''' % (self.test_name, cr_tests.TEST_CONTROL)) pprint.pprint(json.loads(cr_tests.TEST_MANIFEST)) + + def test_check_peer_hooks(self): + '''Test check_peer_hooks()''' + c = ClickReviewSkeleton(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["skeleton"] = "foo.skeleton" + + # add any required peer hooks + tmp["desktop"] = "foo.desktop" + tmp["apparmor"] = "foo.apparmor" + + # update the manifest and test_manifest + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + # We should end up with 2 info + expected_counts = {'info': 2, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_disallowed(self): + '''Test check_peer_hooks() - disallowed''' + c = ClickReviewSkeleton(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["skeleton"] = "foo.skeleton" + + # add any required peer hooks + tmp["desktop"] = "foo.desktop" + tmp["apparmor"] = "foo.apparmor" + + # add something not allowed + tmp["nonexistent"] = "nonexistent-hook" + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_required(self): + '''Test check_peer_hooks() - required''' + c = ClickReviewSkeleton(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["skeleton"] = "foo.skeleton" + + # skip adding required hooks + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) diff -Nru click-reviewers-tools-0.19/bin/clickreviews/tests/test_cr_bin_path.py click-reviewers-tools-0.20/bin/clickreviews/tests/test_cr_bin_path.py --- click-reviewers-tools-0.19/bin/clickreviews/tests/test_cr_bin_path.py 1970-01-01 00:00:00.000000000 +0000 +++ click-reviewers-tools-0.20/bin/clickreviews/tests/test_cr_bin_path.py 2015-01-05 15:57:17.000000000 +0000 @@ -0,0 +1,118 @@ +'''test_cr_bin_path.py: tests for the cr_bin_path module''' +# +# Copyright (C) 2014 Canonical Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 3 of the License. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +from clickreviews.cr_bin_path import ClickReviewBinPath +import clickreviews.cr_tests as cr_tests + + +class TestClickReviewBinPath(cr_tests.TestClickReview): + """Tests for the lint review tool.""" + def setUp(self): + # Monkey patch various file access classes. stop() is handled with + # addCleanup in super() + cr_tests.mock_patch() + super() + + def test_check_path(self): + '''Test check_path()''' + self.set_test_bin_path(self.default_appname, "bin/foo.exe") + c = ClickReviewBinPath(self.test_name) + c.check_path() + r = c.click_report + # We should end up with 1 info + expected_counts = {'info': 1, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_path_nonexecutable(self): + '''Test check_path() - nonexecutable''' + self.set_test_bin_path(self.default_appname, "bin/foo.nonexec") + c = ClickReviewBinPath(self.test_name) + c.check_path() + r = c.click_report + # We should end up with 1 info + expected_counts = {'info': 0, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_check_peer_hooks(self): + '''Test check_peer_hooks()''' + c = ClickReviewBinPath(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["bin-path"] = "usr/bin/foo" + + # add any required peer hooks + tmp["snappy-systemd"] = "foo.systemd" + tmp["apparmor"] = "foo.apparmor" + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + # We should end up with 2 info + expected_counts = {'info': 2, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_disallowed(self): + '''Test check_peer_hooks() - disallowed''' + c = ClickReviewBinPath(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["bin-path"] = "usr/bin/foo" + + # add any required peer hooks + tmp["snappy-systemd"] = "foo.systemd" + tmp["apparmor"] = "foo.apparmor" + + # add something not allowed + tmp["nonexistent"] = "nonexistent-hook" + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_required(self): + '''Test check_peer_hooks() - required''' + c = ClickReviewBinPath(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["bin-path"] = "usr/bin/foo" + + # skip adding required hooks + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) diff -Nru click-reviewers-tools-0.19/bin/clickreviews/tests/test_cr_common.py click-reviewers-tools-0.20/bin/clickreviews/tests/test_cr_common.py --- click-reviewers-tools-0.19/bin/clickreviews/tests/test_cr_common.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/bin/clickreviews/tests/test_cr_common.py 2015-01-05 15:57:17.000000000 +0000 @@ -37,3 +37,43 @@ 'warn': {}, 'error': {}, }) + + def test_verify_peer_hooks_empty(self): + '''Check verify_peer_hooks() - empty''' + peer_hooks = dict() + my_hook = "foo" + peer_hooks[my_hook] = dict() + peer_hooks[my_hook]['allowed'] = [] + peer_hooks[my_hook]['required'] = [] + self.review.peer_hooks = peer_hooks + + d = self.review._verify_peer_hooks(my_hook) + self.assertEqual(0, len(d.keys())) + + def test_verify_peer_hooks_missing(self): + '''Check verify_peer_hooks() - missing required''' + peer_hooks = dict() + my_hook = "desktop" + peer_hooks[my_hook] = dict() + peer_hooks[my_hook]['allowed'] = ["apparmor", "urls"] + peer_hooks[my_hook]['required'] = ["nonexistent"] + self.review.peer_hooks = peer_hooks + + d = self.review._verify_peer_hooks(my_hook) + self.assertEqual(1, len(d.keys())) + self.assertTrue('missing' in d.keys()) + self.assertTrue('nonexistent' in d['missing'][self.default_appname]) + + def test_verify_peer_hooks_disallowed(self): + '''Check verify_peer_hooks() - disallowed''' + peer_hooks = dict() + my_hook = "desktop" + peer_hooks[my_hook] = dict() + peer_hooks[my_hook]['allowed'] = ["apparmor"] + peer_hooks[my_hook]['required'] = [] + self.review.peer_hooks = peer_hooks + + d = self.review._verify_peer_hooks(my_hook) + self.assertEqual(1, len(d.keys())) + self.assertTrue('disallowed' in d.keys()) + self.assertTrue('urls' in d['disallowed'][self.default_appname]) diff -Nru click-reviewers-tools-0.19/bin/clickreviews/tests/test_cr_content_hub.py click-reviewers-tools-0.20/bin/clickreviews/tests/test_cr_content_hub.py --- click-reviewers-tools-0.19/bin/clickreviews/tests/test_cr_content_hub.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/bin/clickreviews/tests/test_cr_content_hub.py 2015-01-05 15:57:17.000000000 +0000 @@ -109,3 +109,54 @@ r = c.click_report expected_counts = {'info': 1, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) + + def test_check_peer_hooks(self): + '''Test check_peer_hooks()''' + self.set_test_content_hub(self.default_appname, "destination", "pictures") + self.set_test_content_hub(self.default_appname, "share", "pictures") + self.set_test_content_hub(self.default_appname, "source", "pictures") + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["content-hub"] = \ + self.test_manifest["hooks"][self.default_appname]["content-hub"] + + self.test_manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c = ClickReviewContentHub(self.test_name) + + c.check_peer_hooks() + r = c.click_report + # We should end up with 2 info + expected_counts = {'info': 2, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_disallowed(self): + '''Test check_peer_hooks() - disallowed''' + self.set_test_content_hub(self.default_appname, "destination", "pictures") + self.set_test_content_hub(self.default_appname, "share", "pictures") + self.set_test_content_hub(self.default_appname, "source", "pictures") + c = ClickReviewContentHub(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["content-hub"] = \ + self.test_manifest["hooks"][self.default_appname]["content-hub"] + + # add something not allowed + tmp["nonexistent"] = "nonexistent-hook" + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) diff -Nru click-reviewers-tools-0.19/bin/clickreviews/tests/test_cr_desktop.py click-reviewers-tools-0.20/bin/clickreviews/tests/test_cr_desktop.py --- click-reviewers-tools-0.19/bin/clickreviews/tests/test_cr_desktop.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/bin/clickreviews/tests/test_cr_desktop.py 2015-01-05 15:57:17.000000000 +0000 @@ -767,3 +767,74 @@ r = c.click_report expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) + + def test_check_peer_hooks(self): + '''Test check_peer_hooks()''' + c = ClickReviewDesktop(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["desktop"] = "foo.desktop" + + # add any required peer hooks + tmp["apparmor"] = "foo.apparmor" + + # update the manifest and test_manifest + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + # We should end up with 2 info + expected_counts = {'info': 2, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_disallowed(self): + '''Test check_peer_hooks() - disallowed''' + c = ClickReviewDesktop(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["desktop"] = "foo.desktop" + + # add any required peer hooks + tmp["desktop"] = "foo.desktop" + tmp["apparmor"] = "foo.apparmor" + + # add something not allowed + tmp["nonexistent"] = "nonexistent-hook" + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_required(self): + '''Test check_peer_hooks() - required''' + c = ClickReviewDesktop(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["desktop"] = "foo.desktop" + + # skip adding required hooks + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) diff -Nru click-reviewers-tools-0.19/bin/clickreviews/tests/test_cr_framework.py click-reviewers-tools-0.20/bin/clickreviews/tests/test_cr_framework.py --- click-reviewers-tools-0.19/bin/clickreviews/tests/test_cr_framework.py 1970-01-01 00:00:00.000000000 +0000 +++ click-reviewers-tools-0.20/bin/clickreviews/tests/test_cr_framework.py 2015-01-05 15:57:17.000000000 +0000 @@ -0,0 +1,209 @@ +'''test_cr_framework.py: tests for the cr_framework module''' +# +# Copyright (C) 2014 Canonical Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 3 of the License. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +from clickreviews.cr_framework import ClickReviewFramework +import clickreviews.cr_tests as cr_tests + + +class TestClickReviewFramework(cr_tests.TestClickReview): + """Tests for the lint review tool.""" + def setUp(self): + # Monkey patch various file access classes. stop() is handled with + # addCleanup in super() + cr_tests.mock_patch() + super() + + def test_single_framework(self): + '''Test check_single_framework()''' + self.set_test_framework(self.default_appname, "", "") + c = ClickReviewFramework(self.test_name) + + c.check_single_framework() + r = c.click_report + # We should end up with 1 info + expected_counts = {'info': 1, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_single_framework_multiple(self): + '''Test check_single_framework() - multiple''' + self.set_test_framework(self.default_appname, "", "") + self.set_test_framework("test-alt", "", "") + c = ClickReviewFramework(self.test_name) + c.check_single_framework() + r = c.click_report + # We should end up with 1 info + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_framework_base_name(self): + '''Test check_framework_base_name()''' + self.set_test_framework(self.default_appname, "Base-Name", + self.test_manifest["name"]) + c = ClickReviewFramework(self.test_name) + c.check_framework_base_name() + r = c.click_report + # We should end up with 2 info + expected_counts = {'info': 2, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_framework_base_name_missing(self): + '''Test check_framework_base_name() - missing''' + self.set_test_framework(self.default_appname, "Base-Version", "") + c = ClickReviewFramework(self.test_name) + c.check_framework_base_name() + r = c.click_report + # We should end up with 1 info + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_framework_base_name_empty(self): + '''Test check_framework_base_name() - empty''' + self.set_test_framework(self.default_appname, "Base-Name", "") + c = ClickReviewFramework(self.test_name) + c.check_framework_base_name() + r = c.click_report + # We should end up with 1 info + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_framework_base_version(self): + '''Test check_framework_base_version()''' + self.set_test_framework(self.default_appname, "Base-Version", 0.1) + c = ClickReviewFramework(self.test_name) + c.check_framework_base_version() + r = c.click_report + # We should end up with 3 info + expected_counts = {'info': 3, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_framework_base_version_missing(self): + '''Test check_framework_base_version() - missing''' + self.set_test_framework(self.default_appname, "Base-Name", "") + c = ClickReviewFramework(self.test_name) + c.check_framework_base_version() + r = c.click_report + # We should end up with 1 info + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_framework_base_version_empty(self): + '''Test check_framework_base_version() - empty''' + self.set_test_framework(self.default_appname, "Base-Version", "") + c = ClickReviewFramework(self.test_name) + c.check_framework_base_version() + r = c.click_report + # We should end up with 1 info + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_framework_base_version_str(self): + '''Test check_framework_base_version() - str''' + self.set_test_framework(self.default_appname, "Base-Version", "abc") + c = ClickReviewFramework(self.test_name) + c.check_framework_base_version() + r = c.click_report + # We should end up with 1 info + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_framework_base_version_negative(self): + '''Test check_framework_base_version() - negative''' + self.set_test_framework(self.default_appname, "Base-Version", -1) + c = ClickReviewFramework(self.test_name) + c.check_framework_base_version() + r = c.click_report + # We should end up with 1 info + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_check_peer_hooks(self): + '''Test check_peer_hooks()''' + self.set_test_framework(self.default_appname, "Base-Version", 0.1) + c = ClickReviewFramework(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["framework"] = "foo.framework" + + # add any required peer hooks + # FIXME: + # tmp["apparmor-policy"] = "apparmor/" + + # update the manifest and test_manifest + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + # We should end up with 2 info + expected_counts = {'info': 2, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_disallowed(self): + '''Test check_peer_hooks() - disallowed''' + self.set_test_framework(self.default_appname, "Base-Version", 0.1) + c = ClickReviewFramework(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["framework"] = "foo.framework" + + # add any required peer hooks + # FIXME: + # tmp["apparmor-policy"] = "apparmor/" + + # add disallowed framework + tmp["nonexistent"] = "nonexistent-hook" + + # update the manifest and test_manifest + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_required(self): + '''Test check_peer_hooks() - required''' + self.set_test_framework(self.default_appname, "Base-Version", 0.1) + c = ClickReviewFramework(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["framework"] = "foo.framework" + + # skip adding required hooks + + # update the manifest and test_manifest + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + # FIXME: apparmor-policy is not defined yet, so no error, when it is + # adjust to 'error': 1 + expected_counts = {'info': None, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) diff -Nru click-reviewers-tools-0.19/bin/clickreviews/tests/test_cr_lint.py click-reviewers-tools-0.20/bin/clickreviews/tests/test_cr_lint.py --- click-reviewers-tools-0.19/bin/clickreviews/tests/test_cr_lint.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/bin/clickreviews/tests/test_cr_lint.py 2015-01-05 15:57:17.000000000 +0000 @@ -632,6 +632,25 @@ "'ubuntu-devel-discuss@lists.ubuntu.com' as email)"} self.check_results(r, expected=expected) + def test_check_maintainer_flat_namespace(self): + '''Test check_maintainer() - flat namespace''' + self.set_test_control("Package", "snapp") + self.set_test_manifest("maintainer", + "Foo User ") + c = ClickReviewLint(self.test_name) + c.check_maintainer() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + expected = dict() + expected['info'] = dict() + expected['warn'] = dict() + expected['error'] = dict() + expected['info']['lint_maintainer_domain'] = \ + {"text": "OK (flat namespace)"} + self.check_results(r, expected=expected) + def test_check_icon(self): '''Test check_icon()''' self.set_test_manifest("icon", "someicon") diff -Nru click-reviewers-tools-0.19/bin/clickreviews/tests/test_cr_online_accounts.py click-reviewers-tools-0.20/bin/clickreviews/tests/test_cr_online_accounts.py --- click-reviewers-tools-0.19/bin/clickreviews/tests/test_cr_online_accounts.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/bin/clickreviews/tests/test_cr_online_accounts.py 2015-01-05 15:57:17.000000000 +0000 @@ -94,46 +94,7 @@ c = ClickReviewAccounts(self.test_name) c.check_application() r = c.click_report - expected_counts = {'info': 5, 'warn': 0, 'error': 0} - self.check_results(r, expected_counts) - - def test_check_application_missing_apparmor(self): - '''Test check_application() - missing apparmor''' - xml = self._stub_application() - # print(etree.tostring(xml)) - self.set_test_account(self.default_appname, "account-application", xml) - c = ClickReviewAccounts(self.test_name) - del c.manifest['hooks'][self.default_appname]['apparmor'] - c.check_application() - r = c.click_report - expected_counts = {'info': None, 'warn': 0, 'error': 1} - self.check_results(r, expected_counts) - - def test_check_application_missing_desktop_and_scope(self): - '''Test check_application() - missing desktop and scope''' - xml = self._stub_application() - # print(etree.tostring(xml)) - self.set_test_account(self.default_appname, "account-application", xml) - c = ClickReviewAccounts(self.test_name) - # The stub manifest doesn't have scope already - del c.manifest['hooks'][self.default_appname]['desktop'] - c.check_application() - r = c.click_report - expected_counts = {'info': None, 'warn': 0, 'error': 1} - self.check_results(r, expected_counts) - - def test_check_application_missing_desktop_and_scope_with_payui(self): - '''Test check_application() - missing desktop and scope with pay-ui''' - xml = self._stub_application() - # print(etree.tostring(xml)) - self.set_test_account(self.default_appname, "account-application", xml) - c = ClickReviewAccounts(self.test_name) - # The stub manifest doesn't have scope already - del c.manifest['hooks'][self.default_appname]['desktop'] - c.manifest['hooks'][self.default_appname]['pay-ui'] = "foo.desktop" - c.check_application() - r = c.click_report - expected_counts = {'info': None, 'warn': 0, 'error': 0} + expected_counts = {'info': 4, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) def test_check_application_not_specified(self): @@ -206,30 +167,7 @@ c = ClickReviewAccounts(self.test_name) c.check_service() r = c.click_report - expected_counts = {'info': 6, 'warn': 0, 'error': 0} - self.check_results(r, expected_counts) - - def test_check_service_missing_application(self): - '''Test check_service() - missing account-application''' - xml = self._stub_service() - self.set_test_account(self.default_appname, "account-service", xml) - c = ClickReviewAccounts(self.test_name) - c.check_service() - r = c.click_report - expected_counts = {'info': None, 'warn': 0, 'error': 1} - self.check_results(r, expected_counts) - - def test_check_service_missing_apparmor(self): - '''Test check_service() - missing apparmor''' - xml = self._stub_service() - self.set_test_account(self.default_appname, "account-service", xml) - xml = self._stub_application() - self.set_test_account(self.default_appname, "account-application", xml) - c = ClickReviewAccounts(self.test_name) - del c.manifest['hooks'][self.default_appname]['apparmor'] - c.check_service() - r = c.click_report - expected_counts = {'info': None, 'warn': 0, 'error': 1} + expected_counts = {'info': 5, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) def test_check_service_not_specified(self): @@ -322,212 +260,248 @@ c.check_provider() r = c.click_report # provider prompts manual review, so for now, need to have error as 1 - expected_counts = {'info': 4, 'warn': 0, 'error': 1} + expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) check_name = "%s_%s_account-provider" % (c.review_type, self.default_appname) self.check_manual_review(r, check_name) -# TODO: when apparmor has policy for account-provider, undo this - def _test_check_provider_missing_apparmor(self): - '''Test check_provider() - missing apparmor''' - xml = self._stub_provider() + def test_check_provider_has_id(self): + '''Test check_provider() - has id''' + xml = self._stub_provider(id="%s_%s" % (self.test_manifest["name"], + self.default_appname)) self.set_test_account(self.default_appname, "account-provider", xml) self.set_test_account(self.default_appname, "account-qml-plugin", True) c = ClickReviewAccounts(self.test_name) - del c.manifest['hooks'][self.default_appname]['apparmor'] c.check_provider() r = c.click_report - # provider prompts manual review, so for now, need to have error as 2 - expected_counts = {'info': None, 'warn': 0, 'error': 2} + # provider prompts manual review, so for now, need to have error as +1 + expected_counts = {'info': None, 'warn': 1, 'error': 1} self.check_results(r, expected_counts) check_name = "%s_%s_account-provider" % (c.review_type, self.default_appname) self.check_manual_review(r, check_name) - def test_check_provider_missing_qml_plugin(self): - '''Test check_provider() - missing account-qml-plugin''' + def test_check_qml_plugin(self): + '''Test check_qml_plugin()''' + self.set_test_account(self.default_appname, "account-qml-plugin", True) xml = self._stub_provider() self.set_test_account(self.default_appname, "account-provider", xml) c = ClickReviewAccounts(self.test_name) - c.check_provider() + c.check_qml_plugin() r = c.click_report - # provider prompts manual review, so for now, need to have error as 2 - expected_counts = {'info': None, 'warn': 0, 'error': 2} + # provider prompts manual review, so for now, need to have error as 1 + expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) - check_name = "%s_%s_account-provider" % (c.review_type, self.default_appname) + check_name = "%s_%s_account-qml-plugin" % (c.review_type, self.default_appname) self.check_manual_review(r, check_name) - def test_check_provider_with_application(self): - '''Test check_provider() - with account-application''' - xml = self._stub_provider() - self.set_test_account(self.default_appname, "account-provider", xml) - self.set_test_account(self.default_appname, "account-qml-plugin", True) - xml = self._stub_application() - self.set_test_account(self.default_appname, "account-application", xml) + def test_check_peer_hooks_application(self): + '''Test check_peer_hooks() - application''' c = ClickReviewAccounts(self.test_name) - c.check_provider() - r = c.click_report - # provider prompts manual review, so for now, need to have error as 2 - expected_counts = {'info': None, 'warn': 0, 'error': 2} - self.check_results(r, expected_counts) - check_name = "%s_%s_account-provider" % (c.review_type, self.default_appname) - self.check_manual_review(r, check_name) - def test_check_provider_with_service(self): - '''Test check_provider() - with account-service''' - xml = self._stub_provider() - self.set_test_account(self.default_appname, "account-provider", xml) - self.set_test_account(self.default_appname, "account-qml-plugin", True) - xml = self._stub_service() - self.set_test_account(self.default_appname, "account-service", xml) - c = ClickReviewAccounts(self.test_name) - c.check_provider() - r = c.click_report - # provider prompts manual review, so for now, need to have error as 2 - expected_counts = {'info': None, 'warn': 0, 'error': 2} - self.check_results(r, expected_counts) - check_name = "%s_%s_account-provider" % (c.review_type, self.default_appname) - self.check_manual_review(r, check_name) + # create a new hooks database for our peer hooks tests + tmp = dict() - def test_check_provider_with_application_and_service(self): - '''Test check_provider() - with account-application/account-service''' - xml = self._stub_provider() - self.set_test_account(self.default_appname, "account-provider", xml) - self.set_test_account(self.default_appname, "account-qml-plugin", True) - xml = self._stub_service() - self.set_test_account(self.default_appname, "account-service", xml) - xml = self._stub_application() - self.set_test_account(self.default_appname, "account-application", xml) - c = ClickReviewAccounts(self.test_name) - c.check_provider() - r = c.click_report - # provider prompts manual review, so for now, need to have error as 2 - expected_counts = {'info': None, 'warn': 0, 'error': 2} - self.check_results(r, expected_counts) - check_name = "%s_%s_account-provider" % (c.review_type, self.default_appname) - self.check_manual_review(r, check_name) + # add our hook + tmp["account-application"] = "foo.application" - def test_check_provider_not_specified(self): - '''Test check_provider() - not specified''' - self.set_test_account(self.default_appname, "account-qml-plugin", True) - c = ClickReviewAccounts(self.test_name) - c.check_provider() + # add any required peer hooks + tmp["apparmor"] = "foo.apparmor" + + # update the manifest and test_manifest + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks(["account-application"]) r = c.click_report - expected_counts = {'info': 1, 'warn': 0, 'error': 0} + # We should end up with 8 info + expected_counts = {'info': 2, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) - def test_check_provider_has_id(self): - '''Test check_provider() - has id''' - xml = self._stub_provider(id="%s_%s" % (self.test_manifest["name"], - self.default_appname)) - self.set_test_account(self.default_appname, "account-provider", xml) - self.set_test_account(self.default_appname, "account-qml-plugin", True) + def test_check_peer_hooks_application_disallowed(self): + '''Test check_peer_hooks() - disallowed (application)''' c = ClickReviewAccounts(self.test_name) - c.check_provider() + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["account-application"] = "foo.application" + + # add any required peer hooks + tmp["apparmor"] = "foo.apparmor" + + # add something not allowed + tmp["nonexistent"] = "nonexistent-hook" + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks(["account-application"]) r = c.click_report - # provider prompts manual review, so for now, need to have error as +1 - expected_counts = {'info': None, 'warn': 1, 'error': 1} + expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) - check_name = "%s_%s_account-provider" % (c.review_type, self.default_appname) - self.check_manual_review(r, check_name) - def test_check_qml_plugin(self): - '''Test check_qml_plugin()''' - self.set_test_account(self.default_appname, "account-qml-plugin", True) - xml = self._stub_provider() - self.set_test_account(self.default_appname, "account-provider", xml) + def test_check_peer_hooks_application_required(self): + '''Test check_peer_hooks() - required (application)''' c = ClickReviewAccounts(self.test_name) - c.check_qml_plugin() + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["account-application"] = "foo.application" + + # skip adding required hooks + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks(["account-application"]) r = c.click_report - # provider prompts manual review, so for now, need to have error as 1 - expected_counts = {'info': 2, 'warn': 0, 'error': 1} + expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) - check_name = "%s_%s_account-qml-plugin" % (c.review_type, self.default_appname) - self.check_manual_review(r, check_name) -# TODO: when apparmor has policy for account-qml-plugin, undo this - def _test_check_qml_plugin_missing_apparmor(self): - '''Test check_qml_plugin() - missing apparmor''' - self.set_test_account(self.default_appname, "account-qml-plugin", True) - xml = self._stub_provider() - self.set_test_account(self.default_appname, "account-provider", xml) + def test_check_peer_hooks_service(self): + '''Test check_peer_hooks() - service''' c = ClickReviewAccounts(self.test_name) - del c.manifest['hooks'][self.default_appname]['apparmor'] - c.check_qml_plugin() + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["account-service"] = "foo.service" + + # add any required peer hooks + tmp["account-application"] = "foo.application" + tmp["apparmor"] = "foo.apparmor" + + # update the manifest and test_manifest + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks(["account-service"]) r = c.click_report - # provider prompts manual review, so for now, need to have error as 2 - expected_counts = {'info': None, 'warn': 0, 'error': 2} + # We should end up with 8 info + expected_counts = {'info': 2, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) - check_name = "%s_%s_account-qml-plugin" % (c.review_type, self.default_appname) - self.check_manual_review(r, check_name) - def test_check_qml_plugin_missing_provider(self): - '''Test check_qml_plugin() - missing account-provider''' - self.set_test_account(self.default_appname, "account-qml-plugin", True) + def test_check_peer_hooks_service_disallowed(self): + '''Test check_peer_hooks() - disallowed (service)''' c = ClickReviewAccounts(self.test_name) - c.check_qml_plugin() + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["account-service"] = "foo.service" + + # add any required peer hooks + tmp["account-application"] = "foo.application" + tmp["apparmor"] = "foo.apparmor" + + # add something not allowed + tmp["nonexistent"] = "nonexistent-hook" + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks(["account-service"]) r = c.click_report - # provider prompts manual review, so for now, need to have error as 2 - expected_counts = {'info': None, 'warn': 0, 'error': 2} + expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) - check_name = "%s_%s_account-qml-plugin" % (c.review_type, self.default_appname) - self.check_manual_review(r, check_name) - def test_check_qml_plugin_with_application(self): - '''Test check_qml_plugin() - with account-application''' - self.set_test_account(self.default_appname, "account-qml-plugin", True) - xml = self._stub_provider() - self.set_test_account(self.default_appname, "account-provider", xml) - xml = self._stub_application() - self.set_test_account(self.default_appname, "account-application", xml) + def test_check_peer_hooks_service_required(self): + '''Test check_peer_hooks() - required (service)''' c = ClickReviewAccounts(self.test_name) - c.check_qml_plugin() + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["account-service"] = "foo.service" + + # skip adding required hooks + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks(["account-service"]) r = c.click_report - # provider prompts manual review, so for now, need to have error as 2 - expected_counts = {'info': None, 'warn': 0, 'error': 2} + expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) - check_name = "%s_%s_account-qml-plugin" % (c.review_type, self.default_appname) - self.check_manual_review(r, check_name) - def test_check_qml_plugin_with_service(self): - '''Test check_qml_plugin() - with account-service''' - self.set_test_account(self.default_appname, "account-qml-plugin", True) - xml = self._stub_provider() - self.set_test_account(self.default_appname, "account-provider", xml) - xml = self._stub_service() - self.set_test_account(self.default_appname, "account-service", xml) + def test_check_peer_hooks_provider(self): + '''Test check_peer_hooks() - provider''' c = ClickReviewAccounts(self.test_name) - c.check_qml_plugin() + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["account-provider"] = "foo.provider" + + # add any required peer hooks + tmp["account-qml-plugin"] = "foo.qml_plugin" + + # update the manifest and test_manifest + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks(["account-provider"]) r = c.click_report - # provider prompts manual review, so for now, need to have error as 2 - expected_counts = {'info': None, 'warn': 0, 'error': 2} + # We should end up with 8 info + expected_counts = {'info': 2, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) - check_name = "%s_%s_account-qml-plugin" % (c.review_type, self.default_appname) - self.check_manual_review(r, check_name) - def test_check_qml_plugin_with_application_and_service(self): - '''Test check_qml_plugin() - with account-application/account-service''' - self.set_test_account(self.default_appname, "account-qml-plugin", True) - xml = self._stub_provider() - self.set_test_account(self.default_appname, "account-provider", xml) - xml = self._stub_service() - self.set_test_account(self.default_appname, "account-service", xml) - xml = self._stub_application() - self.set_test_account(self.default_appname, "account-application", xml) + def test_check_peer_hooks_provider_disallowed(self): + '''Test check_peer_hooks() - disallowed (provider)''' c = ClickReviewAccounts(self.test_name) - c.check_qml_plugin() + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["account-provider"] = "foo.provider" + + # add any required peer hooks + tmp["account-qml-plugin"] = "foo.qml_plugin" + + # add something not allowed + tmp["nonexistent"] = "nonexistent-hook" + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks(["account-provider"]) r = c.click_report - # provider prompts manual review, so for now, need to have error as 1 - expected_counts = {'info': None, 'warn': 0, 'error': 2} + expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) - check_name = "%s_%s_account-qml-plugin" % (c.review_type, self.default_appname) - self.check_manual_review(r, check_name) - def test_check_qml_plugin_not_specified(self): - '''Test check_qml_plugin() - not specified''' - xml = self._stub_provider() - self.set_test_account(self.default_appname, "account-provider", xml) + def test_check_peer_hooks_provider_required(self): + '''Test check_peer_hooks() - required (provider)''' c = ClickReviewAccounts(self.test_name) - c.check_qml_plugin() + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["account-provider"] = "foo.provider" + + # skip adding required hooks + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks(["account-provider"]) r = c.click_report - expected_counts = {'info': 1, 'warn': 0, 'error': 0} + expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) diff -Nru click-reviewers-tools-0.19/bin/clickreviews/tests/test_cr_push_helper.py click-reviewers-tools-0.20/bin/clickreviews/tests/test_cr_push_helper.py --- click-reviewers-tools-0.19/bin/clickreviews/tests/test_cr_push_helper.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/bin/clickreviews/tests/test_cr_push_helper.py 2015-01-05 15:57:17.000000000 +0000 @@ -136,3 +136,73 @@ r = c.click_report expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) + + def test_check_peer_hooks(self): + '''Test check_peer_hooks()''' + c = ClickReviewPushHelper(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["push-helper"] = "foo.push-helper" + + # add any required peer hooks + tmp["apparmor"] = "foo.apparmor" + + # update the manifest and test_manifest + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + # We should end up with 2 info + expected_counts = {'info': 2, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_disallowed(self): + '''Test check_peer_hooks() - disallowed''' + c = ClickReviewPushHelper(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["push-helper"] = "foo.push-helper" + + # add any required peer hooks + tmp["apparmor"] = "foo.apparmor" + + # add something not allowed + tmp["nonexistent"] = "nonexistent-hook" + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_required(self): + '''Test check_peer_hooks() - required''' + c = ClickReviewPushHelper(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["push-helper"] = "foo.push-helper" + + # skip adding required hooks + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) diff -Nru click-reviewers-tools-0.19/bin/clickreviews/tests/test_cr_scope.py click-reviewers-tools-0.20/bin/clickreviews/tests/test_cr_scope.py --- click-reviewers-tools-0.19/bin/clickreviews/tests/test_cr_scope.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/bin/clickreviews/tests/test_cr_scope.py 2015-01-05 15:57:17.000000000 +0000 @@ -174,3 +174,75 @@ r = c.click_report expected_counts = {'info': None, 'warn': 1, 'error': 0} self.check_results(r, expected_counts) + + def test_check_peer_hooks(self): + '''Test check_peer_hooks()''' + c = ClickReviewScope(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["scope"] = "scope.ini" + + # add any required peer hooks + tmp["apparmor"] = "foo.apparmor" + + # update the manifest and test_manifest + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + # We should end up with 2 info + expected_counts = {'info': 2, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_disallowed(self): + '''Test check_peer_hooks() - disallowed''' + c = ClickReviewScope(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["scope"] = "scope.ini" + + # add any required peer hooks + tmp["apparmor"] = "foo.apparmor" + + # add something not allowed + tmp["desktop"] = "foo.desktop" + + # update the manifest and test_manifest + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_required(self): + '''Test check_peer_hooks() - required''' + c = ClickReviewScope(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["scope"] = "scope.ini" + + # skip adding required hooks + + # update the manifest and test_manifest + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) diff -Nru click-reviewers-tools-0.19/bin/clickreviews/tests/test_cr_security.py click-reviewers-tools-0.20/bin/clickreviews/tests/test_cr_security.py --- click-reviewers-tools-0.19/bin/clickreviews/tests/test_cr_security.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/bin/clickreviews/tests/test_cr_security.py 2015-01-05 15:57:17.000000000 +0000 @@ -228,10 +228,10 @@ '''Test check_policy_version() - incorrectly override framework''' self.set_test_manifest("framework", "nonexistent") self.set_test_security_manifest(self.default_appname, - "policy_version", 1.3) + "policy_version", 999999999.3) overrides = {'nonexistent': {'state': 'available', 'policy_vendor': 'ubuntu', - 'policy_version': 1.3}} + 'policy_version': 999999999.3}} c = ClickReviewSecurity(self.test_name, overrides=overrides) c.check_policy_version() report = c.click_report @@ -257,6 +257,18 @@ expected_counts = {'info': 1, 'warn': 0, 'error': 0} self.check_results(report, expected_counts) + def test_check_policy_vendor_ubuntu_snappy(self): + '''Test check_policy_vendor() - ubuntu-snappy''' + c = ClickReviewSecurity(self.test_name) + self.set_test_security_manifest(self.default_appname, + "policy_vendor", "ubuntu-snappy") + self.set_test_security_manifest(self.default_appname, + "policy_version", 1.3) + c.check_policy_vendor() + report = c.click_report + expected_counts = {'info': 1, 'warn': 0, 'error': 0} + self.check_results(report, expected_counts) + def test_check_policy_vendor_nonexistent(self): '''Test check_policy_vendor() - nonexistent''' c = ClickReviewSecurity(self.test_name) @@ -297,6 +309,56 @@ {"text": "No need to specify 'ubuntu-sdk' template"} self.check_results(report, expected=expected) + def test_check_template_default(self): + '''Test check_template() - default specified with no vendor''' + c = ClickReviewSecurity(self.test_name) + self.set_test_security_manifest(self.default_appname, + "template", "default") + c.check_template() + report = c.click_report + expected_counts = {'info': None, 'warn': 1, 'error': 0} + self.check_results(report, expected_counts) + + def test_check_template_default_with_ubuntu(self): + '''Test check_template() - default specified with ubuntu vendor''' + c = ClickReviewSecurity(self.test_name) + self.set_test_security_manifest(self.default_appname, + "template", "default") + self.set_test_security_manifest(self.default_appname, + "policy_vendor", "ubuntu") + c.check_template() + report = c.click_report + expected_counts = {'info': None, 'warn': 1, 'error': 0} + self.check_results(report, expected_counts) + + def test_check_template_default_with_snappy(self): + '''Test check_template() - default specified with ubuntu-snappy vendor''' + c = ClickReviewSecurity(self.test_name) + self.set_test_security_manifest(self.default_appname, + "template", "default") + self.set_test_security_manifest(self.default_appname, + "policy_vendor", "ubuntu-snappy") + self.set_test_security_manifest(self.default_appname, + "policy_version", 1.3) + c.check_template() + report = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 0} + self.check_results(report, expected_counts) + + def test_check_template_nonexistent_with_snappy(self): + '''Test check_template() - nonexistent with ubuntu-snappy vendor''' + c = ClickReviewSecurity(self.test_name) + self.set_test_security_manifest(self.default_appname, + "template", "nonexistent") + self.set_test_security_manifest(self.default_appname, + "policy_vendor", "ubuntu-snappy") + self.set_test_security_manifest(self.default_appname, + "policy_version", 1.3) + c.check_template() + report = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(report, expected_counts) + def test_check_template_webapp(self): '''Test check_template() - webapp''' c = ClickReviewSecurity(self.test_name) @@ -758,3 +820,126 @@ report = c.click_report expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(report, expected_counts) + + def test_check_peer_hooks(self): + '''Test check_peer_hooks()''' + c = ClickReviewSecurity(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["apparmor"] = "foo.apparmor" + + # update the manifest and test_manifest + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + # We should end up with 2 info + expected_counts = {'info': 2, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_disallowed(self): + '''Test check_peer_hooks() - disallowed''' + c = ClickReviewSecurity(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["apparmor"] = "foo.apparmor" + + # add something not allowed + tmp["framework"] = "foo.framework" + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_check_redflag_policy_vendor_ubuntu(self): + '''Test check_redflag() - policy_vendor - ubuntu''' + c = ClickReviewSecurity(self.test_name) + self.set_test_security_manifest(self.default_appname, + "policy_vendor", "ubuntu") + c.check_redflag() + report = c.click_report + expected_counts = {'info': 1, 'warn': 0, 'error': 0} + self.check_results(report, expected_counts) + + def test_check_redflag_policy_vendor_ubuntu_snappy(self): + '''Test check_redflag() - policy_vendor - ubuntu-snappy''' + c = ClickReviewSecurity(self.test_name) + self.set_test_security_manifest(self.default_appname, + "policy_vendor", "ubuntu-snappy") + c.check_redflag() + report = c.click_report + expected_counts = {'info': 1, 'warn': 0, 'error': 0} + self.check_results(report, expected_counts) + + def test_check_redflag_policy_vendor_notubuntu(self): + '''Test check_redflag() - policy_vendor - notubuntu''' + c = ClickReviewSecurity(self.test_name) + self.set_test_security_manifest(self.default_appname, + "policy_vendor", "notubuntu") + c.check_redflag() + report = c.click_report + expected_counts = {'info': 0, 'warn': 0, 'error': 1} + self.check_results(report, expected_counts) + + def test_check_redflag_abstractions(self): + '''Test check_redflag() - abstractions''' + c = ClickReviewSecurity(self.test_name) + self.set_test_security_manifest(self.default_appname, + "abstractions", ["python"]) + c.check_redflag() + report = c.click_report + expected_counts = {'info': 0, 'warn': 0, 'error': 1} + self.check_results(report, expected_counts) + + def test_check_redflag_binary(self): + '''Test check_redflag() - binary''' + c = ClickReviewSecurity(self.test_name) + self.set_test_security_manifest(self.default_appname, + "binary", "/bin/foo") + c.check_redflag() + report = c.click_report + expected_counts = {'info': 0, 'warn': 0, 'error': 1} + self.check_results(report, expected_counts) + + def test_check_redflag_read_path(self): + '''Test check_redflag() - read_path''' + c = ClickReviewSecurity(self.test_name) + self.set_test_security_manifest(self.default_appname, + "read_path", ["/"]) + c.check_redflag() + report = c.click_report + expected_counts = {'info': 0, 'warn': 0, 'error': 1} + self.check_results(report, expected_counts) + + def test_check_redflag_template_variables(self): + '''Test check_redflag() - template_variables''' + c = ClickReviewSecurity(self.test_name) + self.set_test_security_manifest(self.default_appname, + "template_variables", {"FOO": "bar"}) + c.check_redflag() + report = c.click_report + expected_counts = {'info': 0, 'warn': 0, 'error': 1} + self.check_results(report, expected_counts) + + def test_check_redflag_write_path(self): + '''Test check_redflag() - write_path''' + c = ClickReviewSecurity(self.test_name) + self.set_test_security_manifest(self.default_appname, + "write_path", ["/"]) + c.check_redflag() + report = c.click_report + expected_counts = {'info': 0, 'warn': 0, 'error': 1} + self.check_results(report, expected_counts) diff -Nru click-reviewers-tools-0.19/bin/clickreviews/tests/test_cr_url_dispatcher.py click-reviewers-tools-0.20/bin/clickreviews/tests/test_cr_url_dispatcher.py --- click-reviewers-tools-0.19/bin/clickreviews/tests/test_cr_url_dispatcher.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/bin/clickreviews/tests/test_cr_url_dispatcher.py 2015-01-05 15:57:17.000000000 +0000 @@ -209,3 +209,55 @@ r = c.click_report expected_counts = {'info': 0, 'warn': 1, 'error': 0} self.check_results(r, expected_counts) + + def test_check_peer_hooks(self): + '''Test check_peer_hooks()''' + self.set_test_url_dispatcher(self.default_appname, + key="protocol", + value="some-protocol") + c = ClickReviewUrlDispatcher(self.test_name) + print(c.manifest["hooks"]) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["url-dispatcher"] = \ + self.test_manifest["hooks"][self.default_appname]["urls"] + + # update the manifest and test_manifest + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + # We should end up with 2 info + expected_counts = {'info': 2, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_disallowed(self): + '''Test check_peer_hooks() - disallowed''' + self.set_test_url_dispatcher(self.default_appname, + key="protocol", + value="some-protocol") + c = ClickReviewUrlDispatcher(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["urls"] = \ + self.test_manifest["hooks"][self.default_appname]["urls"] + + # add something not allowed + tmp["nonexistent"] = "nonexistent-hook" + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) diff -Nru click-reviewers-tools-0.19/bin/click-run-checks click-reviewers-tools-0.20/bin/click-run-checks --- click-reviewers-tools-0.19/bin/click-run-checks 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/bin/click-run-checks 2015-01-05 15:57:17.000000000 +0000 @@ -72,6 +72,14 @@ prefer_local click-check-url-dispatcher "$c" echo "" +echo "= click-check-bin-path =" +prefer_local click-check-bin-path "$c" + +echo "" +echo "= click-check-framework =" +prefer_local click-check-framework "$c" + +echo "" echo "" if [ "$rc" = "1" ]; then echo "** Warnings found **" diff -Nru click-reviewers-tools-0.19/bin/click-show-files click-reviewers-tools-0.20/bin/click-show-files --- click-reviewers-tools-0.19/bin/click-show-files 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/bin/click-show-files 2015-01-05 15:57:17.000000000 +0000 @@ -28,6 +28,9 @@ from clickreviews import cr_content_hub from clickreviews import cr_online_accounts from clickreviews import cr_push_helper +from clickreviews import cr_bin_path +from clickreviews import cr_framework +from clickreviews import cr_systemd # This script just dumps important files to stdout @@ -104,6 +107,23 @@ fh.close() print("") + review_framework = cr_framework.ClickReviewFramework(sys.argv[1]) + for app in sorted(review_framework.frameworks_file): + f = os.path.join(review_framework.unpack_dir, + review_framework.frameworks_file[app]) + fh = cr_common.open_file_read(os.path.join(review_framework.unpack_dir, f)) + print("== click .framework: %s ==" % os.path.basename(f)) + for line in fh.readlines(): + print(line, end="") + fh.close() + print("") + + review_bin_path = cr_bin_path.ClickReviewBinPath(sys.argv[1]) + for app in sorted(review_bin_path.bin_paths): + f = os.path.join(review_bin_path.unpack_dir, review_bin_path.bin_paths[app]) + print("== bin_path: %s ==" % os.path.relpath(f, review_bin_path.unpack_dir)) + print("") + review_apparmor = cr_security.ClickReviewSecurity(sys.argv[1]) for f in sorted(review_apparmor.security_manifests): fh = cr_common.open_file_read(os.path.join(review_apparmor.unpack_dir, @@ -112,6 +132,17 @@ for line in fh.readlines(): print(line, end="") fh.close() + print("") + + review_systemd = cr_systemd.ClickReviewSystemd(sys.argv[1]) + for app in sorted(review_systemd.systemd_files): + f = review_systemd.systemd_files[app] + fh = cr_common.open_file_read(os.path.join(review_systemd.unpack_dir, + f)) + print("== systemd: %s ==" % os.path.basename(f)) + for line in fh.readlines(): + print(line, end="") + fh.close() print("") review_url_dispatcher = cr_url_dispatcher.ClickReviewUrlDispatcher(sys.argv[1]) diff -Nru click-reviewers-tools-0.19/clickreviews/cr_bin_path.py click-reviewers-tools-0.20/clickreviews/cr_bin_path.py --- click-reviewers-tools-0.19/clickreviews/cr_bin_path.py 1970-01-01 00:00:00.000000000 +0000 +++ click-reviewers-tools-0.20/clickreviews/cr_bin_path.py 2015-01-05 15:57:17.000000000 +0000 @@ -0,0 +1,69 @@ +'''cr_bin_path.py: click bin-path''' +# +# Copyright (C) 2014 Canonical Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 3 of the License. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +from __future__ import print_function + +from clickreviews.cr_common import ClickReview +import os + + +class ClickReviewBinPath(ClickReview): + '''This class represents click lint reviews''' + def __init__(self, fn): + peer_hooks = dict() + my_hook = 'bin-path' + peer_hooks[my_hook] = dict() + peer_hooks[my_hook]['required'] = ["snappy-systemd", "apparmor"] + peer_hooks[my_hook]['allowed'] = peer_hooks[my_hook]['required'] + + ClickReview.__init__(self, fn, "bin-path", peer_hooks=peer_hooks) + + self.bin_paths = dict() + for app in self.manifest['hooks']: + if 'bin-path' not in self.manifest['hooks'][app]: + # msg("Skipped missing bin-path hook for '%s'" % app) + continue + self.bin_paths[app] = self._extract_bin_path(app) + + def _extract_bin_path(self, app): + '''Get bin-path for app''' + rel = self.manifest['hooks'][app]['bin-path'] + fn = os.path.join(self.unpack_dir, rel) + if not os.path.exists(fn): + error("Could not find '%s'" % rel) + return fn + + def _check_bin_path_executable(self, app): + '''Check that the provided path exists''' + rel = self.manifest['hooks'][app]['bin-path'] + fn = os.path.join(self.unpack_dir, rel) + return os.access(fn, os.X_OK) + + def check_path(self): + '''Check path exists''' + t = 'info' + n = 'path exists' + s = "OK" + + for app in sorted(self.bin_paths): + t = 'info' + n = 'path executable' + s = "OK" + if not self._check_bin_path_executable(app): + t = 'error' + s = "'%s' is not executable" % \ + (self.manifest['hooks'][app]['bin-path']) + self._add_result(t, n, s) diff -Nru click-reviewers-tools-0.19/clickreviews/cr_common.py click-reviewers-tools-0.20/clickreviews/cr_common.py --- click-reviewers-tools-0.19/clickreviews/cr_common.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/clickreviews/cr_common.py 2015-01-05 15:57:17.000000000 +0000 @@ -57,10 +57,33 @@ class ClickReview(object): '''This class represents click reviews''' - def __init__(self, fn, review_type): + # Convenience to break out common types of clicks (eg, app, scope, + # framework, click service) + app_allowed_peer_hooks = ["account-application", + "account-service", + "apparmor", + "content-hub", + "desktop", + "push-helper", + "urls", + ] + scope_allowed_peer_hooks = ["account-application", + "account-service", + "apparmor", + "scope", + ] + # FIXME: when apparmor-policy is implemented, use this + # framework_allowed_peer_hooks = ["apparmor-policy"] + framework_allowed_peer_hooks = [] + service_allowed_peer_hooks = ["bin-path", + "snappy-systemd", + ] + + def __init__(self, fn, review_type, peer_hooks=None): self.click_package = fn self._check_path_exists() - if not self.click_package.endswith(".click"): + if not self.click_package.endswith(".click") and \ + not self.click_package.endswith(".snap"): if self.click_package.endswith(".deb"): error("filename does not end with '.click', but '.deb' " "instead. See http://askubuntu.com/a/485544/94326 for " @@ -114,6 +137,8 @@ self.valid_frameworks = self._extract_click_frameworks() + self.peer_hooks = peer_hooks + def _extract_click_frameworks(self): '''Extract installed click frameworks''' # TODO: update to use libclick API when available @@ -179,6 +204,8 @@ optional = ["title", "description", "maintainer", "architecture", "installed-size", "icon"] + snappy_optional = ["ports"] + for f in optional: if f in self.manifest: if f != "architecture" and \ @@ -208,7 +235,7 @@ error("manifest malformed: hooks/%s is empty:\n%s" % (app, mp)) for k in sorted(self.manifest): - if k not in required + optional + ['hooks']: + if k not in required + optional + snappy_optional + ['hooks']: # click supports local extensions via 'x-...', ignore those # here but report in lint if k.startswith('x-'): @@ -216,6 +243,72 @@ error("manifest malformed: unsupported field '%s':\n%s" % (k, mp)) + def _verify_peer_hooks(self, my_hook): + '''Compare manifest for required and allowed hooks''' + d = dict() + if self.peer_hooks is None: + return d + + for app in self.manifest["hooks"]: + if my_hook not in self.manifest["hooks"][app]: + continue + for h in self.peer_hooks[my_hook]['required']: + if h == my_hook: + continue + if h not in self.manifest["hooks"][app]: + if 'missing' not in d: + d['missing'] = dict() + if app not in d['missing']: + d['missing'][app] = [] + d['missing'][app].append(h) + for h in self.manifest["hooks"][app]: + if h == my_hook: + continue + if h not in self.peer_hooks[my_hook]['allowed']: + if 'disallowed' not in d: + d['disallowed'] = dict() + if app not in d['disallowed']: + d['disallowed'][app] = [] + d['disallowed'][app].append(h) + + return d + + def check_peer_hooks(self, hooks_sublist=[]): + '''Check if peer hooks are valid''' + # Nothing to verify + if self.peer_hooks is None: + return + + for hook in self.peer_hooks: + if len(hooks_sublist) > 0 and hook not in hooks_sublist: + continue + d = self._verify_peer_hooks(hook) + t = 'info' + n = "peer_hooks_required_%s" % hook + s = "OK" + + if 'missing' in d and len(d['missing'].keys()) > 0: + t = 'error' + for app in d['missing']: + s = "Missing required hooks for '%s': %s" % ( + app, ", ".join(d['missing'][app])) + self._add_result(t, n, s, manual_review=True) + else: + self._add_result(t, n, s) + + t = 'info' + n = "peer_hooks_disallowed_with_%s" % hook + s = "OK" + + if 'disallowed' in d and len(d['disallowed'].keys()) > 0: + t = 'error' + for app in d['disallowed']: + s = "Disallowed with %s (%s): %s" % ( + hook, app, ", ".join(d['disallowed'][app])) + self._add_result(t, n, s, manual_review=True) + else: + self._add_result(t, n, s) + def set_review_type(self, name): '''Set review name''' self.review_type = name diff -Nru click-reviewers-tools-0.19/clickreviews/cr_content_hub.py click-reviewers-tools-0.20/clickreviews/cr_content_hub.py --- click-reviewers-tools-0.19/clickreviews/cr_content_hub.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/clickreviews/cr_content_hub.py 2015-01-05 15:57:17.000000000 +0000 @@ -24,7 +24,13 @@ class ClickReviewContentHub(ClickReview): '''This class represents click lint reviews''' def __init__(self, fn): - ClickReview.__init__(self, fn, "content_hub") + peer_hooks = dict() + my_hook = 'content-hub' + peer_hooks[my_hook] = dict() + peer_hooks[my_hook]['allowed'] = ClickReview.app_allowed_peer_hooks + peer_hooks[my_hook]['required'] = [] + + ClickReview.__init__(self, fn, "content_hub", peer_hooks=peer_hooks) self.valid_keys = ['destination', 'share', 'source'] diff -Nru click-reviewers-tools-0.19/clickreviews/cr_desktop.py click-reviewers-tools-0.20/clickreviews/cr_desktop.py --- click-reviewers-tools-0.19/clickreviews/cr_desktop.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/clickreviews/cr_desktop.py 2015-01-05 15:57:17.000000000 +0000 @@ -29,42 +29,21 @@ class ClickReviewDesktop(ClickReview): '''This class represents click lint reviews''' def __init__(self, fn): - ClickReview.__init__(self, fn, "desktop") + peer_hooks = dict() + my_hook = 'desktop' + peer_hooks[my_hook] = dict() + peer_hooks[my_hook]['allowed'] = ClickReview.app_allowed_peer_hooks + peer_hooks[my_hook]['required'] = ["apparmor"] + + ClickReview.__init__(self, fn, "desktop", peer_hooks=peer_hooks) self.desktop_files = dict() # click-show-files and a couple tests self.desktop_entries = dict() self.desktop_hook_entries = 0 for app in self.manifest['hooks']: if 'desktop' not in self.manifest['hooks'][app]: - if 'scope' in self.manifest['hooks'][app]: - # msg("Skipped missing desktop hook for scope '%s'" % app) - continue - elif 'push-helper' in self.manifest['hooks'][app]: - # msg("Skipped missing desktop hook for push-helper '%s'" % - # app) - continue - elif 'pay-ui' in self.manifest['hooks'][app]: - # msg("Skipped missing desktop hook for pay-ui '%s'" % app) - continue - elif 'account-provider' in self.manifest['hooks'][app]: - # msg("Skipped missing desktop hook for account-provider" - # " '%s'" % app) - continue - elif 'account-qml-plugin' in self.manifest['hooks'][app]: - # msg("Skipped missing desktop hook for account-qml-plugin" - # " '%s'" % app) - continue - elif 'systemd' in self.manifest['hooks'][app]: - # msg("Skipped missing desktop hook for systemd" - # " '%s'" % app) - continue - elif 'framework' in self.manifest['hooks'][app]: - # TODO: verify this is the long term hook name - # msg("Skipped missing desktop hook for framework" - # " '%s'" % app) - continue - else: - error("could not find desktop hook for '%s'" % app) + # msg("Skipped missing desktop hook for '%s'" % app) + continue if not isinstance(self.manifest['hooks'][app]['desktop'], str): error("manifest malformed: hooks/%s/desktop is not str" % app) self.desktop_hook_entries += 1 diff -Nru click-reviewers-tools-0.19/clickreviews/cr_framework.py click-reviewers-tools-0.20/clickreviews/cr_framework.py --- click-reviewers-tools-0.19/clickreviews/cr_framework.py 1970-01-01 00:00:00.000000000 +0000 +++ click-reviewers-tools-0.20/clickreviews/cr_framework.py 2015-01-05 15:57:17.000000000 +0000 @@ -0,0 +1,130 @@ +'''cr_framework.py: click framework''' +# +# Copyright (C) 2014 Canonical Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 3 of the License. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +from __future__ import print_function + +from clickreviews.cr_common import ClickReview, open_file_read +import os + + +class ClickReviewFramework(ClickReview): + '''This class represents click lint reviews''' + def __init__(self, fn): + peer_hooks = dict() + my_hook = 'framework' + peer_hooks[my_hook] = dict() + peer_hooks[my_hook]['allowed'] = ClickReview.framework_allowed_peer_hooks + peer_hooks[my_hook]['required'] = peer_hooks[my_hook]['allowed'] + ClickReview.__init__(self, fn, "framework", peer_hooks=peer_hooks) + + self.frameworks_file = dict() + self.frameworks = dict() + for app in self.manifest['hooks']: + if 'framework' not in self.manifest['hooks'][app]: + # msg("Skipped missing framework hook for '%s'" % app) + continue + if not isinstance(self.manifest['hooks'][app]['framework'], str): + error("manifest malformed: hooks/%s/framework is not str" % + app) + (full_fn, data) = self._extract_framework(app) + self.frameworks_file[app] = full_fn + self.frameworks[app] = data + + def _extract_framework(self, app): + '''Get framework for app''' + rel = self.manifest['hooks'][app]['framework'] + fn = os.path.join(self.unpack_dir, rel) + if not os.path.exists(fn): + error("Could not find '%s'" % rel) + + data = dict() + fh = open_file_read(fn) + for line in fh.readlines(): + tmp = line.split(':') + if len(tmp) != 2: + continue + data[tmp[0].strip()] = tmp[1].strip() + fh.close() + + return (fn, data) + + def check_single_framework(self): + '''Check only have one framework in the click''' + t = 'info' + n = 'single_framework' + s = "OK" + if len(self.frameworks.keys()) > 1: + t = 'error' + s = 'framework hook specified multiple times' + self._add_result(t, n, s) + + def check_framework_base_name(self): + '''Check framework Base-Name''' + for app in sorted(self.frameworks): + t = 'info' + n = "base_name_present '%s'" % app + s = "OK" + if 'Base-Name' not in self.frameworks[app]: + t = 'error' + s = "Could not find 'Base-Name' in '%s'" % \ + (self.frameworks_file[app]) + self._add_result(t, n, s) + continue + self._add_result(t, n, s) + + t = 'info' + n = "base_name_namespacing '%s'" % app + s = "OK" + if self.frameworks[app]['Base-Name'] != self.manifest['name']: + t = 'error' + s = "'%s' != '%s'" % (self.frameworks[app]['Base-Name'], + self.manifest['name']) + self._add_result(t, n, s) + + def check_framework_base_version(self): + '''Check framework Base-Version''' + for app in sorted(self.frameworks): + t = 'info' + n = "base_version_present '%s'" % app + s = "OK" + if 'Base-Version' not in self.frameworks[app]: + t = 'error' + s = "Could not find 'Base-Version' in '%s'" % \ + (self.frameworks_file[app]) + self._add_result(t, n, s) + continue + self._add_result(t, n, s) + + v = self.frameworks[app]['Base-Version'] + t = 'info' + n = "base_version_number '%s'" % app + s = "OK" + try: + float(v) + except ValueError: + t = 'error' + s = "'Base-Version' malformed: '%s' is not a number" % v + self._add_result(t, n, s) + continue + self._add_result(t, n, s) + + t = 'info' + n = "base_version_positive '%s'" % app + s = "OK" + if float(v) < 0: + t = 'error' + s = "'Base-Version' malformed: '%s' is negative" % v + self._add_result(t, n, s) diff -Nru click-reviewers-tools-0.19/clickreviews/cr_lint.py click-reviewers-tools-0.20/clickreviews/cr_lint.py --- click-reviewers-tools-0.19/clickreviews/cr_lint.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/clickreviews/cr_lint.py 2015-01-05 15:57:17.000000000 +0000 @@ -86,14 +86,19 @@ 'account-qml-plugin', 'account-service', 'apparmor', + 'bin-path', 'content-hub', 'desktop', + 'framework', 'pay-ui', 'push-helper', 'scope', + 'snappy-systemd', 'urls'] - self.redflagged_hooks = ['pay-ui'] + self.redflagged_hooks = ['framework', + 'pay-ui', + ] if overrides is None: overrides = {} self.overrides = overrides @@ -608,6 +613,10 @@ "(Your email domain needs to match the reverse package " \ "namespace.)" % (self.email, ".".join(pkg_domain_rev)) + # flat namespaces are ok now + if self.click_pkgname.count('.') < 1: + t = 'info' + s = 'OK (flat namespace)' self._add_result(t, n, s, manual_review=manual_review) def check_title(self): @@ -717,17 +726,23 @@ os.path.basename(self.click_package) self._add_result(t, n, s) - # handle $pkgname.click - pkgname = tmp[0].partition('\.click')[0] + # handle $pkgname.click or $pkgname.snap + if tmp[0].endswith('.snap'): + pkgname = tmp[0].partition('.snap')[0] + else: + pkgname = tmp[0].partition('.click')[0] t = 'info' n = 'package_filename_pkgname_match' s = 'OK' l = None if pkgname != self.click_pkgname: - t = 'error' - s = "'%s' != '%s' from DEBIAN/control" % (pkgname, - self.click_pkgname) - l = 'http://askubuntu.com/questions/417361/what-does-lint-package-filename-pkgname-match-mean' + if pkgname.startswith('com.ubuntu.snappy.'): + s = "OK (store snappy workaround)" + else: + t = 'error' + s = "'%s' != '%s' from DEBIAN/control" % (pkgname, + self.click_pkgname) + l = 'http://askubuntu.com/questions/417361/what-does-lint-package-filename-pkgname-match-mean' self._add_result(t, n, s, l) # check if namespaces matches with filename @@ -748,8 +763,11 @@ s = 'OK' l = None if len(tmp) >= 2: - # handle $pkgname_$version.click - version = tmp[1].partition('.click')[0] + # handle $pkgname_$version.click or $pkgname_$version.snap + if tmp[0].endswith('.snap'): + version = tmp[1].partition('.snap')[0] + else: + version = tmp[1].partition('.click')[0] if version != self.click_version: t = 'error' s = "'%s' != '%s' from DEBIAN/control" % (version, @@ -765,7 +783,10 @@ n = 'package_filename_arch_valid' s = 'OK' if len(tmp) >= 3: - arch = tmp[2].partition('.click')[0] + if tmp[0].endswith('.snap'): + arch = tmp[2].partition('.snap')[0] + else: + arch = tmp[2].partition('.click')[0] if arch == "unknown": # short-circuit here since the appstore doesn't determine # the version yet @@ -786,7 +807,10 @@ n = 'package_filename_arch_match' s = 'OK' if len(tmp) >= 3: - arch = tmp[2].partition('.click')[0] + if tmp[0].endswith('.snap'): + arch = tmp[2].partition('.snap')[0] + else: + arch = tmp[2].partition('.click')[0] if arch != self.click_arch: t = 'error' s = "'%s' != '%s' from DEBIAN/control" % (arch, diff -Nru click-reviewers-tools-0.19/clickreviews/cr_online_accounts.py click-reviewers-tools-0.20/clickreviews/cr_online_accounts.py --- click-reviewers-tools-0.19/clickreviews/cr_online_accounts.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/clickreviews/cr_online_accounts.py 2015-01-05 15:57:17.000000000 +0000 @@ -25,7 +25,36 @@ class ClickReviewAccounts(ClickReview): '''This class represents click lint reviews''' def __init__(self, fn): - ClickReview.__init__(self, fn, "online_accounts") + peer_hooks = dict() + peer_hooks['account-application'] = dict() + peer_hooks['account-application']['allowed'] = \ + ClickReview.app_allowed_peer_hooks + \ + ClickReview.scope_allowed_peer_hooks + peer_hooks['account-application']['required'] = ['apparmor'] + + peer_hooks['account-service'] = dict() + peer_hooks['account-service']['required'] = ['account-application', + 'apparmor' + ] + peer_hooks['account-service']['allowed'] = \ + ClickReview.app_allowed_peer_hooks + \ + ClickReview.scope_allowed_peer_hooks + + peer_hooks['account-provider'] = dict() + peer_hooks['account-provider']['required'] = ['account-qml-plugin', + # 'apparmor' + ] + peer_hooks['account-provider']['allowed'] = \ + peer_hooks['account-provider']['required'] + + peer_hooks['account-qml-plugin'] = dict() + peer_hooks['account-qml-plugin']['required'] = ['account-provider', + # 'apparmor' + ] + peer_hooks['account-qml-plugin']['allowed'] = \ + peer_hooks['account-qml-plugin']['required'] + + ClickReview.__init__(self, fn, "online_accounts", peer_hooks) self.accounts_files = dict() self.accounts = dict() @@ -88,29 +117,6 @@ self._add_result(t, n, s) continue - # account-application must always appear with apparmor - t = 'info' - n = '%s_%s_apparmor' % (app, account_type) - s = "OK" - if 'apparmor' not in self.manifest['hooks'][app]: - t = 'error' - s = "missing 'apparmor' entry" - self._add_result(t, n, s) - - # account-application must always appear with desktop or scope - t = 'info' - n = '%s_%s_desktop_or_scope' % (app, account_type) - s = "OK" - found = False - for k in ['desktop', 'scope']: - if k in self.manifest['hooks'][app]: - found = True - break - if not found and 'pay-ui' not in self.manifest['hooks'][app]: - t = 'error' - s = "missing 'desktop' or 'scope' entry" - self._add_result(t, n, s) - root_tag = self.accounts[app][account_type].tag.lower() if root_tag != "application": t = 'error' @@ -159,24 +165,6 @@ self._add_result(t, n, s) continue - # account service must always appear with account-application - t = 'info' - n = '%s_%s_account-application' % (app, account_type) - s = "OK" - if 'account-application' not in self.accounts[app]: - t = 'error' - s = "missing 'account-application' entry" - self._add_result(t, n, s) - - # account-service must always appear with apparmor - t = 'info' - n = '%s_%s_apparmor' % (app, account_type) - s = "OK" - if 'apparmor' not in self.manifest['hooks'][app]: - t = 'error' - s = "missing 'apparmor' entry" - self._add_result(t, n, s) - root_tag = self.accounts[app][account_type].tag.lower() if root_tag != "service": t = 'error' @@ -223,40 +211,6 @@ manual_review = True self._add_result(t, n, s, manual_review=manual_review) - # account-provider must always appear with apparmor -# t = 'info' -# n = '%s_%s_apparmor' % (app, account_type) -# s = "OK" -# if 'apparmor' not in self.manifest['hooks'][app]: -# t = 'error' -# s = "missing 'apparmor' entry" -# self._add_result(t, n, s) - - # account-provider must always appear with account-qml-plugin - t = 'info' - n = '%s_%s_account-qml-plugin' % (app, account_type) - s = "OK" - if 'account-qml-plugin' not in self.accounts[app]: - t = 'error' - s = "missing 'account-qml-plugin' entry" - self._add_result(t, n, s) - - # account-provider must never appear with account-application or - # account-service - t = 'info' - n = '%s_%s_account-application_or_account-service' % (app, - account_type) - s = "OK" - found = False - for i in ['account-application', 'account-service']: - if i in self.accounts[app]: - found = True - if found: - t = 'error' - s = "must not specify account-application or account-service" \ - " with %s" % account_type - self._add_result(t, n, s) - t = 'info' n = '%s_%s_root' % (app, account_type) s = "OK" @@ -293,38 +247,3 @@ s = "(MANUAL REVIEW) '%s' not allowed" % account_type manual_review = True self._add_result(t, n, s, manual_review=manual_review) - - # account-qml-plugin must always appear with apparmor -# t = 'info' -# n = '%s_%s_apparmor' % (app, account_type) -# s = "OK" -# if 'apparmor' not in self.manifest['hooks'][app]: -# t = 'error' -# t = 'info' -# s = "missing 'apparmor' entry" -# self._add_result(t, n, s) - - # account-qml-plugin must always appear with account-provider - t = 'info' - n = '%s_%s_account-provider' % (app, account_type) - s = "OK" - if 'account-provider' not in self.accounts[app]: - t = 'error' - s = "missing 'account-provider' entry" - self._add_result(t, n, s) - - # account-qml-plugin must never appear with account-application or - # account-service - t = 'info' - n = '%s_%s_account-application_or_account-service' % (app, - account_type) - s = "OK" - found = False - for i in ['account-application', 'account-service']: - if i in self.accounts[app]: - found = True - if found: - t = 'error' - s = "must not specify account-application or account-service" \ - " with %s" % account_type - self._add_result(t, n, s) diff -Nru click-reviewers-tools-0.19/clickreviews/cr_push_helper.py click-reviewers-tools-0.20/clickreviews/cr_push_helper.py --- click-reviewers-tools-0.19/clickreviews/cr_push_helper.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/clickreviews/cr_push_helper.py 2015-01-05 15:57:17.000000000 +0000 @@ -24,7 +24,13 @@ class ClickReviewPushHelper(ClickReview): '''This class represents click lint reviews''' def __init__(self, fn): - ClickReview.__init__(self, fn, "push_helper") + peer_hooks = dict() + my_hook = 'push-helper' + peer_hooks[my_hook] = dict() + peer_hooks[my_hook]['allowed'] = ['apparmor'] + peer_hooks[my_hook]['required'] = ['apparmor'] + + ClickReview.__init__(self, fn, "push_helper", peer_hooks=peer_hooks) self.required_keys = ['exec'] self.optional_keys = ['app_id'] @@ -36,7 +42,8 @@ # msg("Skipped missing push-helper hook for '%s'" % app) continue if not isinstance(self.manifest['hooks'][app]['push-helper'], str): - error("manifest malformed: hooks/%s/urls is not str" % app) + error("manifest malformed: hooks/%s/push-helper is not str" % + app) (full_fn, jd) = self._extract_push_helper(app) self.push_helper_files[app] = full_fn self.push_helper[app] = jd diff -Nru click-reviewers-tools-0.19/clickreviews/cr_scope.py click-reviewers-tools-0.20/clickreviews/cr_scope.py --- click-reviewers-tools-0.19/clickreviews/cr_scope.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/clickreviews/cr_scope.py 2015-01-05 15:57:17.000000000 +0000 @@ -29,7 +29,13 @@ class ClickReviewScope(ClickReview): '''This class represents click lint reviews''' def __init__(self, fn): - ClickReview.__init__(self, fn, "scope") + peer_hooks = dict() + my_hook = 'scope' + peer_hooks[my_hook] = dict() + peer_hooks[my_hook]['allowed'] = ClickReview.scope_allowed_peer_hooks + peer_hooks[my_hook]['required'] = ['apparmor'] + + ClickReview.__init__(self, fn, "scope", peer_hooks=peer_hooks) self.scopes = dict() for app in self.manifest['hooks']: diff -Nru click-reviewers-tools-0.19/clickreviews/cr_security.py click-reviewers-tools-0.20/clickreviews/cr_security.py --- click-reviewers-tools-0.19/clickreviews/cr_security.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/clickreviews/cr_security.py 2015-01-05 15:57:17.000000000 +0000 @@ -26,7 +26,17 @@ class ClickReviewSecurity(ClickReview): '''This class represents click lint reviews''' def __init__(self, fn, overrides=None): - ClickReview.__init__(self, fn, "security") + peer_hooks = dict() + my_hook = 'apparmor' + peer_hooks[my_hook] = dict() + # Basically, everything except frameworks + peer_hooks[my_hook]['allowed'] = ClickReview.app_allowed_peer_hooks + \ + ClickReview.scope_allowed_peer_hooks + \ + ClickReview.service_allowed_peer_hooks + \ + ['pay-ui'] + peer_hooks[my_hook]['required'] = [] + + ClickReview.__init__(self, fn, "security", peer_hooks=peer_hooks) local_copy = os.path.join(os.path.dirname(__file__), '../data/apparmor-easyprof-ubuntu.json') @@ -72,8 +82,9 @@ self.allowed_network_scope_policy_groups = ['accounts', 'networking'] self.redflag_templates = ['unconfined'] - self.extraneous_templates = ['ubuntu-sdk', - 'default'] + # TODO: how to deal with other vendors + self.extraneous_ubuntu_templates = ['ubuntu-sdk', + 'default'] # framework policy is based on major framework version. In 13.10, there # was only 'ubuntu-sdk-13.10', but in 14.04, there will be several, @@ -98,14 +109,8 @@ self.security_apps = [] for app in self.manifest['hooks']: if 'apparmor' not in self.manifest['hooks'][app]: - # TODO: when we have apparmor templates for these, we want - # to remove this - if 'account-provider' in self.manifest['hooks'][app] or \ - 'account-qml-plugin' in self.manifest['hooks'][app]: - # msg("Skipped missing desktop hook for account-provider " - # "and account-qml-plugin '%s'" % app) - continue - error("could not find apparmor hook for '%s'" % app) + # msg("Skipped missing apparmor hook for '%s'" % app) + continue if not isinstance(self.manifest['hooks'][app]['apparmor'], str): error("manifest malformed: hooks/%s/apparmor is not str" % app) rel_fn = self.manifest['hooks'][app]['apparmor'] @@ -256,7 +261,8 @@ t = 'info' n = 'policy_vendor (%s)' % f s = "OK" - if 'policy_vendor' in m and m['policy_vendor'] != "ubuntu": + if 'policy_vendor' in m and \ + m['policy_vendor'] not in self.aa_policy: t = 'error' s = "policy_vendor '%s' not found" % m['policy_vendor'] self._add_result(t, n, s) @@ -288,10 +294,12 @@ t = 'info' n = 'policy_version_is_highest (%s, %s)' % (str(highest), f) s = "OK" + l = None if float(m['policy_version']) != highest: t = 'info' + l = 'http://askubuntu.com/q/562116/94326' s = '%s != %s' % (str(m['policy_version']), str(highest)) - self._add_result(t, n, s) + self._add_result(t, n, s, l) t = 'info' n = 'policy_version_matches_framework (%s)' % (f) @@ -338,7 +346,8 @@ t = 'error' s = "(MANUAL REVIEW) '%s' not allowed" % m['template'] manual_review = True - elif m['template'] in self.extraneous_templates: + elif ('policy_vendor' not in m or m['policy_vendor'] == 'ubuntu') \ + and m['template'] in self.extraneous_ubuntu_templates: t = 'warn' s = "No need to specify '%s' template" % m['template'] self._add_result(t, n, s, manual_review=manual_review) @@ -548,6 +557,7 @@ t = 'info' n = 'policy_groups_safe_%s (%s)' % (app, i) s = 'OK' + l = None manual_review = False aa_type = self._get_policy_group_type(vendor, version, i) @@ -559,12 +569,14 @@ t = 'error' s = "(MANUAL REVIEW) %s policy group " % aa_type + \ "'%s': vetted applications only" % (i) + if i == "debug": + l = 'http://askubuntu.com/a/562123/94326' manual_review = True elif aa_type != "common": t = 'error' s = "policy group '%s' has" % i + \ "unknown type '%s'" % (aa_type) - self._add_result(t, n, s, manual_review=manual_review) + self._add_result(t, n, s, l, manual_review=manual_review) def check_ignored(self): '''Check ignored fields''' @@ -595,6 +607,9 @@ found = [] for i in self.redflag_fields: if i in m: + if i == 'policy_vendor' and \ + m[i] in ['ubuntu', 'ubuntu-snappy']: + continue found.append(i) if len(found) > 0: diff -Nru click-reviewers-tools-0.19/clickreviews/cr_skeleton.py click-reviewers-tools-0.20/clickreviews/cr_skeleton.py --- click-reviewers-tools-0.19/clickreviews/cr_skeleton.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/clickreviews/cr_skeleton.py 2015-01-05 15:57:17.000000000 +0000 @@ -22,7 +22,19 @@ class ClickReviewSkeleton(ClickReview): '''This class represents click lint reviews''' def __init__(self, fn): - ClickReview.__init__(self, fn, "skeleton") + # Many test classes are for verify click hooks. 'peer_hooks' is used + # to declare what hooks may be use with my_hook. When using this + # mechanism, ClickReview.check_peer_hooks() is run for you. + peer_hooks = dict() + my_hook = 'skeleton' + peer_hooks[my_hook] = dict() + peer_hooks[my_hook]['allowed'] = ["desktop", "apparmor", "urls"] + peer_hooks[my_hook]['required'] = ["desktop", "apparmor"] + + ClickReview.__init__(self, fn, "skeleton", peer_hooks) + + # If not a hooks test, skip the above and omit peer_hooks like so: + # ClickReview.__init__(self, fn, "skeleton") def check_foo(self): '''Check foo''' diff -Nru click-reviewers-tools-0.19/clickreviews/cr_tests.py click-reviewers-tools-0.20/clickreviews/cr_tests.py --- click-reviewers-tools-0.19/clickreviews/cr_tests.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/clickreviews/cr_tests.py 2015-01-05 15:57:17.000000000 +0000 @@ -40,6 +40,9 @@ TEST_ACCOUNTS_QML_PLUGIN = dict() TEST_ACCOUNTS_SERVICE = dict() TEST_PUSH_HELPER = dict() +TEST_BIN_PATH = dict() +TEST_FRAMEWORK = dict() +TEST_SNAPPY_SYSTEMD = dict() # @@ -94,7 +97,7 @@ def _get_security_supported_policy_versions(self): '''Pretend we read the contens of /usr/share/apparmor/easyprof''' - return [1.0, 1.1, 1.2] + return [1.0, 1.1, 1.2, 1.3] def _extract_desktop_entry(self, app): @@ -154,6 +157,28 @@ return ("%s.push-helper.json" % app, TEST_PUSH_HELPER[app]) +def _extract_bin_path(self, app): + '''Pretend we found the bin-path file''' + return ("%s" % TEST_BIN_PATH[app]) + + +def _check_bin_path_executable(self, app): + '''Pretend we found the bin-path file''' + if TEST_BIN_PATH[app].endswith('.nonexec'): + return False + return True + + +def _extract_framework(self, app): + '''Pretend we found the framework file''' + return ("%s.framework" % app, TEST_FRAMEWORK[app]) + + +def _extract_systemd(self, app): + '''Pretend we found the systemd file''' + return ("%s.click-service" % app, TEST_SNAPPY_SYSTEMD[app]) + + # http://docs.python.org/3.4/library/unittest.mock-examples.html # Mock patching. Don't use decorators but instead patch in setUp() of the # child. Set up a list of patches, but don't start them. Create the helper @@ -239,6 +264,24 @@ 'clickreviews.cr_push_helper.ClickReviewPushHelper._extract_push_helper', _extract_push_helper)) +# bin-path overrides +patches.append(patch( + 'clickreviews.cr_bin_path.ClickReviewBinPath._extract_bin_path', + _extract_bin_path)) +patches.append(patch( + 'clickreviews.cr_bin_path.ClickReviewBinPath._check_bin_path_executable', + _check_bin_path_executable)) + +# framework overrides +patches.append(patch( + 'clickreviews.cr_framework.ClickReviewFramework._extract_framework', + _extract_framework)) + +# systemd overrides +patches.append(patch( + 'clickreviews.cr_systemd.ClickReviewSystemd._extract_systemd', + _extract_systemd)) + def mock_patch(): '''Call in setup of child''' @@ -307,6 +350,9 @@ self.test_accounts_qml_plugin = dict() self.test_accounts_service = dict() self.test_push_helper = dict() + self.test_bin_path = dict() + self.test_framework = dict() + self.test_systemd = dict() for app in self.test_manifest["hooks"].keys(): # setup security manifest for each app self.set_test_security_manifest(app, 'policy_groups', @@ -336,15 +382,24 @@ # Reset to no content-hub entries in manifest self.set_test_content_hub(app, None, None) - # Reset to no content-hub entries in manifest + # Reset to no accounts entries in manifest self.set_test_account(app, "account-application", None) self.set_test_account(app, "account-provider", None) self.set_test_account(app, "account-qml-plugin", None) self.set_test_account(app, "account-service", None) - # Reset to no content-hub entries in manifest + # Reset to no push-helper entries in manifest self.set_test_push_helper(app, None, None) + # Reset to no bin-path entries in manifest + self.set_test_bin_path(app, None) + + # Reset to no framework entries in manifest + self.set_test_framework(app, None, None) + + # Reset to no systemd entries in manifest + self.set_test_systemd(app, None, None) + self._update_test_security_manifests() self._update_test_desktop_files() self._update_test_url_dispatcher() @@ -355,6 +410,9 @@ self._update_test_accounts_qml_plugin() self._update_test_accounts_service() self._update_test_push_helper() + self._update_test_bin_path() + self._update_test_framework() + self._update_test_systemd() # webapps manifests (leave empty for now) self.test_webapp_manifests = dict() @@ -469,6 +527,32 @@ "%s.push-helper.json" % app self._update_test_manifest() + def _update_test_bin_path(self): + global TEST_BIN_PATH + TEST_BIN_PATH = dict() + for app in self.test_bin_path.keys(): + TEST_BIN_PATH[app] = self.test_bin_path[app] + self.test_manifest["hooks"][app]["bin-path"] = \ + "%s" % TEST_BIN_PATH[app] + self._update_test_manifest() + + def _update_test_framework(self): + global TEST_FRAMEWORK + TEST_FRAMEWORK = dict() + for app in self.test_framework.keys(): + TEST_FRAMEWORK[app] = self.test_framework[app] + if app not in self.test_manifest["hooks"]: + self.test_manifest["hooks"][app] = dict() + self.test_manifest["hooks"][app]["framework"] = \ + "%s.framework" % TEST_FRAMEWORK[app] + self._update_test_manifest() + + def _update_test_systemd(self): + global TEST_SNAPPY_SYSTEMD + TEST_SNAPPY_SYSTEMD = dict() + for app in self.test_systemd.keys(): + TEST_SNAPPY_SYSTEMD[app] = self.test_systemd[app] + def _update_test_name(self): self.test_name = "%s_%s_%s.click" % (self.test_control['Package'], self.test_control['Version'], @@ -650,7 +734,7 @@ def set_test_push_helper(self, app, key, value): '''Set push-helper entries. If value is None, remove key, if key is - None, remove content-hub from manifest''' + None, remove push-helper from manifest''' if key is None: if app in self.test_push_helper: self.test_push_helper.pop(app) @@ -663,6 +747,46 @@ self.test_push_helper[app][key] = value self._update_test_push_helper() + def set_test_bin_path(self, app, value): + '''Set bin-path entries. If value is None, remove bin-path from + manifest''' + if value is None: + if app in self.test_bin_path: + self.test_bin_path.pop(app) + else: + if app not in self.test_bin_path: + self.test_bin_path[app] = dict() + self.test_bin_path[app] = value + self._update_test_bin_path() + + def set_test_framework(self, app, key, value): + '''Set framework entries. If value is None, remove key, if key is + None, remove framework from manifest''' + if key is None: + if app in self.test_framework: + self.test_framework.pop(app) + elif value is None: + if key in self.test_framework[app]: + self.test_framework[app].pop(key) + else: + if app not in self.test_framework: + self.test_framework[app] = dict() + self.test_framework[app][key] = value + self._update_test_framework() + + def set_test_systemd(self, app, key, value, append=False): + '''Set systemd entries. If value is None, remove''' + if app not in self.test_systemd: + self.test_systemd[app] = [] + + if value is None: + self.test_systemd[app] = [] + else: + if not append: + self.test_systemd[app] = [] + self.test_systemd[app].append({key: value}) + self._update_test_systemd() + def setUp(self): '''Make sure our patches are applied everywhere''' global patches @@ -695,6 +819,12 @@ TEST_ACCOUNTS_APPLICATION = dict() global TEST_PUSH_HELPER TEST_PUSH_HELPER = dict() + global TEST_BIN_PATH + TEST_BIN_PATH = dict() + global TEST_FRAMEWORK + TEST_FRAMEWORK = dict() + global TEST_SNAPPY_SYSTEMD + TEST_SNAPPY_SYSTEMD = dict() self._reset_test_data() cr_common.recursive_rm(self.desktop_tmpdir) diff -Nru click-reviewers-tools-0.19/clickreviews/cr_url_dispatcher.py click-reviewers-tools-0.20/clickreviews/cr_url_dispatcher.py --- click-reviewers-tools-0.19/clickreviews/cr_url_dispatcher.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/clickreviews/cr_url_dispatcher.py 2015-01-05 15:57:17.000000000 +0000 @@ -26,7 +26,13 @@ class ClickReviewUrlDispatcher(ClickReview): '''This class represents click lint reviews''' def __init__(self, fn): - ClickReview.__init__(self, fn, "url_dispatcher") + peer_hooks = dict() + my_hook = 'urls' + peer_hooks[my_hook] = dict() + peer_hooks[my_hook]['allowed'] = ClickReview.app_allowed_peer_hooks + peer_hooks[my_hook]['required'] = [] + + ClickReview.__init__(self, fn, "url_dispatcher", peer_hooks=peer_hooks) self.required_keys = ['protocol'] self.optional_keys = ['domain-suffix'] diff -Nru click-reviewers-tools-0.19/clickreviews/tests/test_aaa_example_cr_skeleton.py click-reviewers-tools-0.20/clickreviews/tests/test_aaa_example_cr_skeleton.py --- click-reviewers-tools-0.19/clickreviews/tests/test_aaa_example_cr_skeleton.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/clickreviews/tests/test_aaa_example_cr_skeleton.py 2015-01-05 15:57:17.000000000 +0000 @@ -81,3 +81,75 @@ == Mock manifest ==''' % (self.test_name, cr_tests.TEST_CONTROL)) pprint.pprint(json.loads(cr_tests.TEST_MANIFEST)) + + def test_check_peer_hooks(self): + '''Test check_peer_hooks()''' + c = ClickReviewSkeleton(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["skeleton"] = "foo.skeleton" + + # add any required peer hooks + tmp["desktop"] = "foo.desktop" + tmp["apparmor"] = "foo.apparmor" + + # update the manifest and test_manifest + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + # We should end up with 2 info + expected_counts = {'info': 2, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_disallowed(self): + '''Test check_peer_hooks() - disallowed''' + c = ClickReviewSkeleton(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["skeleton"] = "foo.skeleton" + + # add any required peer hooks + tmp["desktop"] = "foo.desktop" + tmp["apparmor"] = "foo.apparmor" + + # add something not allowed + tmp["nonexistent"] = "nonexistent-hook" + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_required(self): + '''Test check_peer_hooks() - required''' + c = ClickReviewSkeleton(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["skeleton"] = "foo.skeleton" + + # skip adding required hooks + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) diff -Nru click-reviewers-tools-0.19/clickreviews/tests/test_cr_bin_path.py click-reviewers-tools-0.20/clickreviews/tests/test_cr_bin_path.py --- click-reviewers-tools-0.19/clickreviews/tests/test_cr_bin_path.py 1970-01-01 00:00:00.000000000 +0000 +++ click-reviewers-tools-0.20/clickreviews/tests/test_cr_bin_path.py 2015-01-05 15:57:17.000000000 +0000 @@ -0,0 +1,118 @@ +'''test_cr_bin_path.py: tests for the cr_bin_path module''' +# +# Copyright (C) 2014 Canonical Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 3 of the License. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +from clickreviews.cr_bin_path import ClickReviewBinPath +import clickreviews.cr_tests as cr_tests + + +class TestClickReviewBinPath(cr_tests.TestClickReview): + """Tests for the lint review tool.""" + def setUp(self): + # Monkey patch various file access classes. stop() is handled with + # addCleanup in super() + cr_tests.mock_patch() + super() + + def test_check_path(self): + '''Test check_path()''' + self.set_test_bin_path(self.default_appname, "bin/foo.exe") + c = ClickReviewBinPath(self.test_name) + c.check_path() + r = c.click_report + # We should end up with 1 info + expected_counts = {'info': 1, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_path_nonexecutable(self): + '''Test check_path() - nonexecutable''' + self.set_test_bin_path(self.default_appname, "bin/foo.nonexec") + c = ClickReviewBinPath(self.test_name) + c.check_path() + r = c.click_report + # We should end up with 1 info + expected_counts = {'info': 0, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_check_peer_hooks(self): + '''Test check_peer_hooks()''' + c = ClickReviewBinPath(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["bin-path"] = "usr/bin/foo" + + # add any required peer hooks + tmp["snappy-systemd"] = "foo.systemd" + tmp["apparmor"] = "foo.apparmor" + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + # We should end up with 2 info + expected_counts = {'info': 2, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_disallowed(self): + '''Test check_peer_hooks() - disallowed''' + c = ClickReviewBinPath(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["bin-path"] = "usr/bin/foo" + + # add any required peer hooks + tmp["snappy-systemd"] = "foo.systemd" + tmp["apparmor"] = "foo.apparmor" + + # add something not allowed + tmp["nonexistent"] = "nonexistent-hook" + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_required(self): + '''Test check_peer_hooks() - required''' + c = ClickReviewBinPath(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["bin-path"] = "usr/bin/foo" + + # skip adding required hooks + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) diff -Nru click-reviewers-tools-0.19/clickreviews/tests/test_cr_common.py click-reviewers-tools-0.20/clickreviews/tests/test_cr_common.py --- click-reviewers-tools-0.19/clickreviews/tests/test_cr_common.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/clickreviews/tests/test_cr_common.py 2015-01-05 15:57:17.000000000 +0000 @@ -37,3 +37,43 @@ 'warn': {}, 'error': {}, }) + + def test_verify_peer_hooks_empty(self): + '''Check verify_peer_hooks() - empty''' + peer_hooks = dict() + my_hook = "foo" + peer_hooks[my_hook] = dict() + peer_hooks[my_hook]['allowed'] = [] + peer_hooks[my_hook]['required'] = [] + self.review.peer_hooks = peer_hooks + + d = self.review._verify_peer_hooks(my_hook) + self.assertEqual(0, len(d.keys())) + + def test_verify_peer_hooks_missing(self): + '''Check verify_peer_hooks() - missing required''' + peer_hooks = dict() + my_hook = "desktop" + peer_hooks[my_hook] = dict() + peer_hooks[my_hook]['allowed'] = ["apparmor", "urls"] + peer_hooks[my_hook]['required'] = ["nonexistent"] + self.review.peer_hooks = peer_hooks + + d = self.review._verify_peer_hooks(my_hook) + self.assertEqual(1, len(d.keys())) + self.assertTrue('missing' in d.keys()) + self.assertTrue('nonexistent' in d['missing'][self.default_appname]) + + def test_verify_peer_hooks_disallowed(self): + '''Check verify_peer_hooks() - disallowed''' + peer_hooks = dict() + my_hook = "desktop" + peer_hooks[my_hook] = dict() + peer_hooks[my_hook]['allowed'] = ["apparmor"] + peer_hooks[my_hook]['required'] = [] + self.review.peer_hooks = peer_hooks + + d = self.review._verify_peer_hooks(my_hook) + self.assertEqual(1, len(d.keys())) + self.assertTrue('disallowed' in d.keys()) + self.assertTrue('urls' in d['disallowed'][self.default_appname]) diff -Nru click-reviewers-tools-0.19/clickreviews/tests/test_cr_content_hub.py click-reviewers-tools-0.20/clickreviews/tests/test_cr_content_hub.py --- click-reviewers-tools-0.19/clickreviews/tests/test_cr_content_hub.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/clickreviews/tests/test_cr_content_hub.py 2015-01-05 15:57:17.000000000 +0000 @@ -109,3 +109,54 @@ r = c.click_report expected_counts = {'info': 1, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) + + def test_check_peer_hooks(self): + '''Test check_peer_hooks()''' + self.set_test_content_hub(self.default_appname, "destination", "pictures") + self.set_test_content_hub(self.default_appname, "share", "pictures") + self.set_test_content_hub(self.default_appname, "source", "pictures") + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["content-hub"] = \ + self.test_manifest["hooks"][self.default_appname]["content-hub"] + + self.test_manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c = ClickReviewContentHub(self.test_name) + + c.check_peer_hooks() + r = c.click_report + # We should end up with 2 info + expected_counts = {'info': 2, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_disallowed(self): + '''Test check_peer_hooks() - disallowed''' + self.set_test_content_hub(self.default_appname, "destination", "pictures") + self.set_test_content_hub(self.default_appname, "share", "pictures") + self.set_test_content_hub(self.default_appname, "source", "pictures") + c = ClickReviewContentHub(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["content-hub"] = \ + self.test_manifest["hooks"][self.default_appname]["content-hub"] + + # add something not allowed + tmp["nonexistent"] = "nonexistent-hook" + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) diff -Nru click-reviewers-tools-0.19/clickreviews/tests/test_cr_desktop.py click-reviewers-tools-0.20/clickreviews/tests/test_cr_desktop.py --- click-reviewers-tools-0.19/clickreviews/tests/test_cr_desktop.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/clickreviews/tests/test_cr_desktop.py 2015-01-05 15:57:17.000000000 +0000 @@ -767,3 +767,74 @@ r = c.click_report expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) + + def test_check_peer_hooks(self): + '''Test check_peer_hooks()''' + c = ClickReviewDesktop(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["desktop"] = "foo.desktop" + + # add any required peer hooks + tmp["apparmor"] = "foo.apparmor" + + # update the manifest and test_manifest + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + # We should end up with 2 info + expected_counts = {'info': 2, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_disallowed(self): + '''Test check_peer_hooks() - disallowed''' + c = ClickReviewDesktop(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["desktop"] = "foo.desktop" + + # add any required peer hooks + tmp["desktop"] = "foo.desktop" + tmp["apparmor"] = "foo.apparmor" + + # add something not allowed + tmp["nonexistent"] = "nonexistent-hook" + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_required(self): + '''Test check_peer_hooks() - required''' + c = ClickReviewDesktop(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["desktop"] = "foo.desktop" + + # skip adding required hooks + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) diff -Nru click-reviewers-tools-0.19/clickreviews/tests/test_cr_framework.py click-reviewers-tools-0.20/clickreviews/tests/test_cr_framework.py --- click-reviewers-tools-0.19/clickreviews/tests/test_cr_framework.py 1970-01-01 00:00:00.000000000 +0000 +++ click-reviewers-tools-0.20/clickreviews/tests/test_cr_framework.py 2015-01-05 15:57:17.000000000 +0000 @@ -0,0 +1,209 @@ +'''test_cr_framework.py: tests for the cr_framework module''' +# +# Copyright (C) 2014 Canonical Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 3 of the License. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +from clickreviews.cr_framework import ClickReviewFramework +import clickreviews.cr_tests as cr_tests + + +class TestClickReviewFramework(cr_tests.TestClickReview): + """Tests for the lint review tool.""" + def setUp(self): + # Monkey patch various file access classes. stop() is handled with + # addCleanup in super() + cr_tests.mock_patch() + super() + + def test_single_framework(self): + '''Test check_single_framework()''' + self.set_test_framework(self.default_appname, "", "") + c = ClickReviewFramework(self.test_name) + + c.check_single_framework() + r = c.click_report + # We should end up with 1 info + expected_counts = {'info': 1, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_single_framework_multiple(self): + '''Test check_single_framework() - multiple''' + self.set_test_framework(self.default_appname, "", "") + self.set_test_framework("test-alt", "", "") + c = ClickReviewFramework(self.test_name) + c.check_single_framework() + r = c.click_report + # We should end up with 1 info + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_framework_base_name(self): + '''Test check_framework_base_name()''' + self.set_test_framework(self.default_appname, "Base-Name", + self.test_manifest["name"]) + c = ClickReviewFramework(self.test_name) + c.check_framework_base_name() + r = c.click_report + # We should end up with 2 info + expected_counts = {'info': 2, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_framework_base_name_missing(self): + '''Test check_framework_base_name() - missing''' + self.set_test_framework(self.default_appname, "Base-Version", "") + c = ClickReviewFramework(self.test_name) + c.check_framework_base_name() + r = c.click_report + # We should end up with 1 info + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_framework_base_name_empty(self): + '''Test check_framework_base_name() - empty''' + self.set_test_framework(self.default_appname, "Base-Name", "") + c = ClickReviewFramework(self.test_name) + c.check_framework_base_name() + r = c.click_report + # We should end up with 1 info + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_framework_base_version(self): + '''Test check_framework_base_version()''' + self.set_test_framework(self.default_appname, "Base-Version", 0.1) + c = ClickReviewFramework(self.test_name) + c.check_framework_base_version() + r = c.click_report + # We should end up with 3 info + expected_counts = {'info': 3, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_framework_base_version_missing(self): + '''Test check_framework_base_version() - missing''' + self.set_test_framework(self.default_appname, "Base-Name", "") + c = ClickReviewFramework(self.test_name) + c.check_framework_base_version() + r = c.click_report + # We should end up with 1 info + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_framework_base_version_empty(self): + '''Test check_framework_base_version() - empty''' + self.set_test_framework(self.default_appname, "Base-Version", "") + c = ClickReviewFramework(self.test_name) + c.check_framework_base_version() + r = c.click_report + # We should end up with 1 info + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_framework_base_version_str(self): + '''Test check_framework_base_version() - str''' + self.set_test_framework(self.default_appname, "Base-Version", "abc") + c = ClickReviewFramework(self.test_name) + c.check_framework_base_version() + r = c.click_report + # We should end up with 1 info + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_framework_base_version_negative(self): + '''Test check_framework_base_version() - negative''' + self.set_test_framework(self.default_appname, "Base-Version", -1) + c = ClickReviewFramework(self.test_name) + c.check_framework_base_version() + r = c.click_report + # We should end up with 1 info + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_check_peer_hooks(self): + '''Test check_peer_hooks()''' + self.set_test_framework(self.default_appname, "Base-Version", 0.1) + c = ClickReviewFramework(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["framework"] = "foo.framework" + + # add any required peer hooks + # FIXME: + # tmp["apparmor-policy"] = "apparmor/" + + # update the manifest and test_manifest + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + # We should end up with 2 info + expected_counts = {'info': 2, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_disallowed(self): + '''Test check_peer_hooks() - disallowed''' + self.set_test_framework(self.default_appname, "Base-Version", 0.1) + c = ClickReviewFramework(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["framework"] = "foo.framework" + + # add any required peer hooks + # FIXME: + # tmp["apparmor-policy"] = "apparmor/" + + # add disallowed framework + tmp["nonexistent"] = "nonexistent-hook" + + # update the manifest and test_manifest + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_required(self): + '''Test check_peer_hooks() - required''' + self.set_test_framework(self.default_appname, "Base-Version", 0.1) + c = ClickReviewFramework(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["framework"] = "foo.framework" + + # skip adding required hooks + + # update the manifest and test_manifest + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + # FIXME: apparmor-policy is not defined yet, so no error, when it is + # adjust to 'error': 1 + expected_counts = {'info': None, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) diff -Nru click-reviewers-tools-0.19/clickreviews/tests/test_cr_lint.py click-reviewers-tools-0.20/clickreviews/tests/test_cr_lint.py --- click-reviewers-tools-0.19/clickreviews/tests/test_cr_lint.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/clickreviews/tests/test_cr_lint.py 2015-01-05 15:57:17.000000000 +0000 @@ -632,6 +632,25 @@ "'ubuntu-devel-discuss@lists.ubuntu.com' as email)"} self.check_results(r, expected=expected) + def test_check_maintainer_flat_namespace(self): + '''Test check_maintainer() - flat namespace''' + self.set_test_control("Package", "snapp") + self.set_test_manifest("maintainer", + "Foo User ") + c = ClickReviewLint(self.test_name) + c.check_maintainer() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + expected = dict() + expected['info'] = dict() + expected['warn'] = dict() + expected['error'] = dict() + expected['info']['lint_maintainer_domain'] = \ + {"text": "OK (flat namespace)"} + self.check_results(r, expected=expected) + def test_check_icon(self): '''Test check_icon()''' self.set_test_manifest("icon", "someicon") diff -Nru click-reviewers-tools-0.19/clickreviews/tests/test_cr_online_accounts.py click-reviewers-tools-0.20/clickreviews/tests/test_cr_online_accounts.py --- click-reviewers-tools-0.19/clickreviews/tests/test_cr_online_accounts.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/clickreviews/tests/test_cr_online_accounts.py 2015-01-05 15:57:17.000000000 +0000 @@ -94,46 +94,7 @@ c = ClickReviewAccounts(self.test_name) c.check_application() r = c.click_report - expected_counts = {'info': 5, 'warn': 0, 'error': 0} - self.check_results(r, expected_counts) - - def test_check_application_missing_apparmor(self): - '''Test check_application() - missing apparmor''' - xml = self._stub_application() - # print(etree.tostring(xml)) - self.set_test_account(self.default_appname, "account-application", xml) - c = ClickReviewAccounts(self.test_name) - del c.manifest['hooks'][self.default_appname]['apparmor'] - c.check_application() - r = c.click_report - expected_counts = {'info': None, 'warn': 0, 'error': 1} - self.check_results(r, expected_counts) - - def test_check_application_missing_desktop_and_scope(self): - '''Test check_application() - missing desktop and scope''' - xml = self._stub_application() - # print(etree.tostring(xml)) - self.set_test_account(self.default_appname, "account-application", xml) - c = ClickReviewAccounts(self.test_name) - # The stub manifest doesn't have scope already - del c.manifest['hooks'][self.default_appname]['desktop'] - c.check_application() - r = c.click_report - expected_counts = {'info': None, 'warn': 0, 'error': 1} - self.check_results(r, expected_counts) - - def test_check_application_missing_desktop_and_scope_with_payui(self): - '''Test check_application() - missing desktop and scope with pay-ui''' - xml = self._stub_application() - # print(etree.tostring(xml)) - self.set_test_account(self.default_appname, "account-application", xml) - c = ClickReviewAccounts(self.test_name) - # The stub manifest doesn't have scope already - del c.manifest['hooks'][self.default_appname]['desktop'] - c.manifest['hooks'][self.default_appname]['pay-ui'] = "foo.desktop" - c.check_application() - r = c.click_report - expected_counts = {'info': None, 'warn': 0, 'error': 0} + expected_counts = {'info': 4, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) def test_check_application_not_specified(self): @@ -206,30 +167,7 @@ c = ClickReviewAccounts(self.test_name) c.check_service() r = c.click_report - expected_counts = {'info': 6, 'warn': 0, 'error': 0} - self.check_results(r, expected_counts) - - def test_check_service_missing_application(self): - '''Test check_service() - missing account-application''' - xml = self._stub_service() - self.set_test_account(self.default_appname, "account-service", xml) - c = ClickReviewAccounts(self.test_name) - c.check_service() - r = c.click_report - expected_counts = {'info': None, 'warn': 0, 'error': 1} - self.check_results(r, expected_counts) - - def test_check_service_missing_apparmor(self): - '''Test check_service() - missing apparmor''' - xml = self._stub_service() - self.set_test_account(self.default_appname, "account-service", xml) - xml = self._stub_application() - self.set_test_account(self.default_appname, "account-application", xml) - c = ClickReviewAccounts(self.test_name) - del c.manifest['hooks'][self.default_appname]['apparmor'] - c.check_service() - r = c.click_report - expected_counts = {'info': None, 'warn': 0, 'error': 1} + expected_counts = {'info': 5, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) def test_check_service_not_specified(self): @@ -322,212 +260,248 @@ c.check_provider() r = c.click_report # provider prompts manual review, so for now, need to have error as 1 - expected_counts = {'info': 4, 'warn': 0, 'error': 1} + expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) check_name = "%s_%s_account-provider" % (c.review_type, self.default_appname) self.check_manual_review(r, check_name) -# TODO: when apparmor has policy for account-provider, undo this - def _test_check_provider_missing_apparmor(self): - '''Test check_provider() - missing apparmor''' - xml = self._stub_provider() + def test_check_provider_has_id(self): + '''Test check_provider() - has id''' + xml = self._stub_provider(id="%s_%s" % (self.test_manifest["name"], + self.default_appname)) self.set_test_account(self.default_appname, "account-provider", xml) self.set_test_account(self.default_appname, "account-qml-plugin", True) c = ClickReviewAccounts(self.test_name) - del c.manifest['hooks'][self.default_appname]['apparmor'] c.check_provider() r = c.click_report - # provider prompts manual review, so for now, need to have error as 2 - expected_counts = {'info': None, 'warn': 0, 'error': 2} + # provider prompts manual review, so for now, need to have error as +1 + expected_counts = {'info': None, 'warn': 1, 'error': 1} self.check_results(r, expected_counts) check_name = "%s_%s_account-provider" % (c.review_type, self.default_appname) self.check_manual_review(r, check_name) - def test_check_provider_missing_qml_plugin(self): - '''Test check_provider() - missing account-qml-plugin''' + def test_check_qml_plugin(self): + '''Test check_qml_plugin()''' + self.set_test_account(self.default_appname, "account-qml-plugin", True) xml = self._stub_provider() self.set_test_account(self.default_appname, "account-provider", xml) c = ClickReviewAccounts(self.test_name) - c.check_provider() + c.check_qml_plugin() r = c.click_report - # provider prompts manual review, so for now, need to have error as 2 - expected_counts = {'info': None, 'warn': 0, 'error': 2} + # provider prompts manual review, so for now, need to have error as 1 + expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) - check_name = "%s_%s_account-provider" % (c.review_type, self.default_appname) + check_name = "%s_%s_account-qml-plugin" % (c.review_type, self.default_appname) self.check_manual_review(r, check_name) - def test_check_provider_with_application(self): - '''Test check_provider() - with account-application''' - xml = self._stub_provider() - self.set_test_account(self.default_appname, "account-provider", xml) - self.set_test_account(self.default_appname, "account-qml-plugin", True) - xml = self._stub_application() - self.set_test_account(self.default_appname, "account-application", xml) + def test_check_peer_hooks_application(self): + '''Test check_peer_hooks() - application''' c = ClickReviewAccounts(self.test_name) - c.check_provider() - r = c.click_report - # provider prompts manual review, so for now, need to have error as 2 - expected_counts = {'info': None, 'warn': 0, 'error': 2} - self.check_results(r, expected_counts) - check_name = "%s_%s_account-provider" % (c.review_type, self.default_appname) - self.check_manual_review(r, check_name) - def test_check_provider_with_service(self): - '''Test check_provider() - with account-service''' - xml = self._stub_provider() - self.set_test_account(self.default_appname, "account-provider", xml) - self.set_test_account(self.default_appname, "account-qml-plugin", True) - xml = self._stub_service() - self.set_test_account(self.default_appname, "account-service", xml) - c = ClickReviewAccounts(self.test_name) - c.check_provider() - r = c.click_report - # provider prompts manual review, so for now, need to have error as 2 - expected_counts = {'info': None, 'warn': 0, 'error': 2} - self.check_results(r, expected_counts) - check_name = "%s_%s_account-provider" % (c.review_type, self.default_appname) - self.check_manual_review(r, check_name) + # create a new hooks database for our peer hooks tests + tmp = dict() - def test_check_provider_with_application_and_service(self): - '''Test check_provider() - with account-application/account-service''' - xml = self._stub_provider() - self.set_test_account(self.default_appname, "account-provider", xml) - self.set_test_account(self.default_appname, "account-qml-plugin", True) - xml = self._stub_service() - self.set_test_account(self.default_appname, "account-service", xml) - xml = self._stub_application() - self.set_test_account(self.default_appname, "account-application", xml) - c = ClickReviewAccounts(self.test_name) - c.check_provider() - r = c.click_report - # provider prompts manual review, so for now, need to have error as 2 - expected_counts = {'info': None, 'warn': 0, 'error': 2} - self.check_results(r, expected_counts) - check_name = "%s_%s_account-provider" % (c.review_type, self.default_appname) - self.check_manual_review(r, check_name) + # add our hook + tmp["account-application"] = "foo.application" - def test_check_provider_not_specified(self): - '''Test check_provider() - not specified''' - self.set_test_account(self.default_appname, "account-qml-plugin", True) - c = ClickReviewAccounts(self.test_name) - c.check_provider() + # add any required peer hooks + tmp["apparmor"] = "foo.apparmor" + + # update the manifest and test_manifest + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks(["account-application"]) r = c.click_report - expected_counts = {'info': 1, 'warn': 0, 'error': 0} + # We should end up with 8 info + expected_counts = {'info': 2, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) - def test_check_provider_has_id(self): - '''Test check_provider() - has id''' - xml = self._stub_provider(id="%s_%s" % (self.test_manifest["name"], - self.default_appname)) - self.set_test_account(self.default_appname, "account-provider", xml) - self.set_test_account(self.default_appname, "account-qml-plugin", True) + def test_check_peer_hooks_application_disallowed(self): + '''Test check_peer_hooks() - disallowed (application)''' c = ClickReviewAccounts(self.test_name) - c.check_provider() + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["account-application"] = "foo.application" + + # add any required peer hooks + tmp["apparmor"] = "foo.apparmor" + + # add something not allowed + tmp["nonexistent"] = "nonexistent-hook" + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks(["account-application"]) r = c.click_report - # provider prompts manual review, so for now, need to have error as +1 - expected_counts = {'info': None, 'warn': 1, 'error': 1} + expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) - check_name = "%s_%s_account-provider" % (c.review_type, self.default_appname) - self.check_manual_review(r, check_name) - def test_check_qml_plugin(self): - '''Test check_qml_plugin()''' - self.set_test_account(self.default_appname, "account-qml-plugin", True) - xml = self._stub_provider() - self.set_test_account(self.default_appname, "account-provider", xml) + def test_check_peer_hooks_application_required(self): + '''Test check_peer_hooks() - required (application)''' c = ClickReviewAccounts(self.test_name) - c.check_qml_plugin() + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["account-application"] = "foo.application" + + # skip adding required hooks + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks(["account-application"]) r = c.click_report - # provider prompts manual review, so for now, need to have error as 1 - expected_counts = {'info': 2, 'warn': 0, 'error': 1} + expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) - check_name = "%s_%s_account-qml-plugin" % (c.review_type, self.default_appname) - self.check_manual_review(r, check_name) -# TODO: when apparmor has policy for account-qml-plugin, undo this - def _test_check_qml_plugin_missing_apparmor(self): - '''Test check_qml_plugin() - missing apparmor''' - self.set_test_account(self.default_appname, "account-qml-plugin", True) - xml = self._stub_provider() - self.set_test_account(self.default_appname, "account-provider", xml) + def test_check_peer_hooks_service(self): + '''Test check_peer_hooks() - service''' c = ClickReviewAccounts(self.test_name) - del c.manifest['hooks'][self.default_appname]['apparmor'] - c.check_qml_plugin() + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["account-service"] = "foo.service" + + # add any required peer hooks + tmp["account-application"] = "foo.application" + tmp["apparmor"] = "foo.apparmor" + + # update the manifest and test_manifest + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks(["account-service"]) r = c.click_report - # provider prompts manual review, so for now, need to have error as 2 - expected_counts = {'info': None, 'warn': 0, 'error': 2} + # We should end up with 8 info + expected_counts = {'info': 2, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) - check_name = "%s_%s_account-qml-plugin" % (c.review_type, self.default_appname) - self.check_manual_review(r, check_name) - def test_check_qml_plugin_missing_provider(self): - '''Test check_qml_plugin() - missing account-provider''' - self.set_test_account(self.default_appname, "account-qml-plugin", True) + def test_check_peer_hooks_service_disallowed(self): + '''Test check_peer_hooks() - disallowed (service)''' c = ClickReviewAccounts(self.test_name) - c.check_qml_plugin() + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["account-service"] = "foo.service" + + # add any required peer hooks + tmp["account-application"] = "foo.application" + tmp["apparmor"] = "foo.apparmor" + + # add something not allowed + tmp["nonexistent"] = "nonexistent-hook" + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks(["account-service"]) r = c.click_report - # provider prompts manual review, so for now, need to have error as 2 - expected_counts = {'info': None, 'warn': 0, 'error': 2} + expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) - check_name = "%s_%s_account-qml-plugin" % (c.review_type, self.default_appname) - self.check_manual_review(r, check_name) - def test_check_qml_plugin_with_application(self): - '''Test check_qml_plugin() - with account-application''' - self.set_test_account(self.default_appname, "account-qml-plugin", True) - xml = self._stub_provider() - self.set_test_account(self.default_appname, "account-provider", xml) - xml = self._stub_application() - self.set_test_account(self.default_appname, "account-application", xml) + def test_check_peer_hooks_service_required(self): + '''Test check_peer_hooks() - required (service)''' c = ClickReviewAccounts(self.test_name) - c.check_qml_plugin() + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["account-service"] = "foo.service" + + # skip adding required hooks + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks(["account-service"]) r = c.click_report - # provider prompts manual review, so for now, need to have error as 2 - expected_counts = {'info': None, 'warn': 0, 'error': 2} + expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) - check_name = "%s_%s_account-qml-plugin" % (c.review_type, self.default_appname) - self.check_manual_review(r, check_name) - def test_check_qml_plugin_with_service(self): - '''Test check_qml_plugin() - with account-service''' - self.set_test_account(self.default_appname, "account-qml-plugin", True) - xml = self._stub_provider() - self.set_test_account(self.default_appname, "account-provider", xml) - xml = self._stub_service() - self.set_test_account(self.default_appname, "account-service", xml) + def test_check_peer_hooks_provider(self): + '''Test check_peer_hooks() - provider''' c = ClickReviewAccounts(self.test_name) - c.check_qml_plugin() + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["account-provider"] = "foo.provider" + + # add any required peer hooks + tmp["account-qml-plugin"] = "foo.qml_plugin" + + # update the manifest and test_manifest + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks(["account-provider"]) r = c.click_report - # provider prompts manual review, so for now, need to have error as 2 - expected_counts = {'info': None, 'warn': 0, 'error': 2} + # We should end up with 8 info + expected_counts = {'info': 2, 'warn': 0, 'error': 0} self.check_results(r, expected_counts) - check_name = "%s_%s_account-qml-plugin" % (c.review_type, self.default_appname) - self.check_manual_review(r, check_name) - def test_check_qml_plugin_with_application_and_service(self): - '''Test check_qml_plugin() - with account-application/account-service''' - self.set_test_account(self.default_appname, "account-qml-plugin", True) - xml = self._stub_provider() - self.set_test_account(self.default_appname, "account-provider", xml) - xml = self._stub_service() - self.set_test_account(self.default_appname, "account-service", xml) - xml = self._stub_application() - self.set_test_account(self.default_appname, "account-application", xml) + def test_check_peer_hooks_provider_disallowed(self): + '''Test check_peer_hooks() - disallowed (provider)''' c = ClickReviewAccounts(self.test_name) - c.check_qml_plugin() + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["account-provider"] = "foo.provider" + + # add any required peer hooks + tmp["account-qml-plugin"] = "foo.qml_plugin" + + # add something not allowed + tmp["nonexistent"] = "nonexistent-hook" + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks(["account-provider"]) r = c.click_report - # provider prompts manual review, so for now, need to have error as 1 - expected_counts = {'info': None, 'warn': 0, 'error': 2} + expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) - check_name = "%s_%s_account-qml-plugin" % (c.review_type, self.default_appname) - self.check_manual_review(r, check_name) - def test_check_qml_plugin_not_specified(self): - '''Test check_qml_plugin() - not specified''' - xml = self._stub_provider() - self.set_test_account(self.default_appname, "account-provider", xml) + def test_check_peer_hooks_provider_required(self): + '''Test check_peer_hooks() - required (provider)''' c = ClickReviewAccounts(self.test_name) - c.check_qml_plugin() + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["account-provider"] = "foo.provider" + + # skip adding required hooks + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks(["account-provider"]) r = c.click_report - expected_counts = {'info': 1, 'warn': 0, 'error': 0} + expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) diff -Nru click-reviewers-tools-0.19/clickreviews/tests/test_cr_push_helper.py click-reviewers-tools-0.20/clickreviews/tests/test_cr_push_helper.py --- click-reviewers-tools-0.19/clickreviews/tests/test_cr_push_helper.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/clickreviews/tests/test_cr_push_helper.py 2015-01-05 15:57:17.000000000 +0000 @@ -136,3 +136,73 @@ r = c.click_report expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(r, expected_counts) + + def test_check_peer_hooks(self): + '''Test check_peer_hooks()''' + c = ClickReviewPushHelper(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["push-helper"] = "foo.push-helper" + + # add any required peer hooks + tmp["apparmor"] = "foo.apparmor" + + # update the manifest and test_manifest + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + # We should end up with 2 info + expected_counts = {'info': 2, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_disallowed(self): + '''Test check_peer_hooks() - disallowed''' + c = ClickReviewPushHelper(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["push-helper"] = "foo.push-helper" + + # add any required peer hooks + tmp["apparmor"] = "foo.apparmor" + + # add something not allowed + tmp["nonexistent"] = "nonexistent-hook" + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_required(self): + '''Test check_peer_hooks() - required''' + c = ClickReviewPushHelper(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["push-helper"] = "foo.push-helper" + + # skip adding required hooks + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) diff -Nru click-reviewers-tools-0.19/clickreviews/tests/test_cr_scope.py click-reviewers-tools-0.20/clickreviews/tests/test_cr_scope.py --- click-reviewers-tools-0.19/clickreviews/tests/test_cr_scope.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/clickreviews/tests/test_cr_scope.py 2015-01-05 15:57:17.000000000 +0000 @@ -174,3 +174,75 @@ r = c.click_report expected_counts = {'info': None, 'warn': 1, 'error': 0} self.check_results(r, expected_counts) + + def test_check_peer_hooks(self): + '''Test check_peer_hooks()''' + c = ClickReviewScope(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["scope"] = "scope.ini" + + # add any required peer hooks + tmp["apparmor"] = "foo.apparmor" + + # update the manifest and test_manifest + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + # We should end up with 2 info + expected_counts = {'info': 2, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_disallowed(self): + '''Test check_peer_hooks() - disallowed''' + c = ClickReviewScope(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["scope"] = "scope.ini" + + # add any required peer hooks + tmp["apparmor"] = "foo.apparmor" + + # add something not allowed + tmp["desktop"] = "foo.desktop" + + # update the manifest and test_manifest + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_required(self): + '''Test check_peer_hooks() - required''' + c = ClickReviewScope(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["scope"] = "scope.ini" + + # skip adding required hooks + + # update the manifest and test_manifest + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) diff -Nru click-reviewers-tools-0.19/clickreviews/tests/test_cr_security.py click-reviewers-tools-0.20/clickreviews/tests/test_cr_security.py --- click-reviewers-tools-0.19/clickreviews/tests/test_cr_security.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/clickreviews/tests/test_cr_security.py 2015-01-05 15:57:17.000000000 +0000 @@ -228,10 +228,10 @@ '''Test check_policy_version() - incorrectly override framework''' self.set_test_manifest("framework", "nonexistent") self.set_test_security_manifest(self.default_appname, - "policy_version", 1.3) + "policy_version", 999999999.3) overrides = {'nonexistent': {'state': 'available', 'policy_vendor': 'ubuntu', - 'policy_version': 1.3}} + 'policy_version': 999999999.3}} c = ClickReviewSecurity(self.test_name, overrides=overrides) c.check_policy_version() report = c.click_report @@ -257,6 +257,18 @@ expected_counts = {'info': 1, 'warn': 0, 'error': 0} self.check_results(report, expected_counts) + def test_check_policy_vendor_ubuntu_snappy(self): + '''Test check_policy_vendor() - ubuntu-snappy''' + c = ClickReviewSecurity(self.test_name) + self.set_test_security_manifest(self.default_appname, + "policy_vendor", "ubuntu-snappy") + self.set_test_security_manifest(self.default_appname, + "policy_version", 1.3) + c.check_policy_vendor() + report = c.click_report + expected_counts = {'info': 1, 'warn': 0, 'error': 0} + self.check_results(report, expected_counts) + def test_check_policy_vendor_nonexistent(self): '''Test check_policy_vendor() - nonexistent''' c = ClickReviewSecurity(self.test_name) @@ -297,6 +309,56 @@ {"text": "No need to specify 'ubuntu-sdk' template"} self.check_results(report, expected=expected) + def test_check_template_default(self): + '''Test check_template() - default specified with no vendor''' + c = ClickReviewSecurity(self.test_name) + self.set_test_security_manifest(self.default_appname, + "template", "default") + c.check_template() + report = c.click_report + expected_counts = {'info': None, 'warn': 1, 'error': 0} + self.check_results(report, expected_counts) + + def test_check_template_default_with_ubuntu(self): + '''Test check_template() - default specified with ubuntu vendor''' + c = ClickReviewSecurity(self.test_name) + self.set_test_security_manifest(self.default_appname, + "template", "default") + self.set_test_security_manifest(self.default_appname, + "policy_vendor", "ubuntu") + c.check_template() + report = c.click_report + expected_counts = {'info': None, 'warn': 1, 'error': 0} + self.check_results(report, expected_counts) + + def test_check_template_default_with_snappy(self): + '''Test check_template() - default specified with ubuntu-snappy vendor''' + c = ClickReviewSecurity(self.test_name) + self.set_test_security_manifest(self.default_appname, + "template", "default") + self.set_test_security_manifest(self.default_appname, + "policy_vendor", "ubuntu-snappy") + self.set_test_security_manifest(self.default_appname, + "policy_version", 1.3) + c.check_template() + report = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 0} + self.check_results(report, expected_counts) + + def test_check_template_nonexistent_with_snappy(self): + '''Test check_template() - nonexistent with ubuntu-snappy vendor''' + c = ClickReviewSecurity(self.test_name) + self.set_test_security_manifest(self.default_appname, + "template", "nonexistent") + self.set_test_security_manifest(self.default_appname, + "policy_vendor", "ubuntu-snappy") + self.set_test_security_manifest(self.default_appname, + "policy_version", 1.3) + c.check_template() + report = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(report, expected_counts) + def test_check_template_webapp(self): '''Test check_template() - webapp''' c = ClickReviewSecurity(self.test_name) @@ -758,3 +820,126 @@ report = c.click_report expected_counts = {'info': None, 'warn': 0, 'error': 1} self.check_results(report, expected_counts) + + def test_check_peer_hooks(self): + '''Test check_peer_hooks()''' + c = ClickReviewSecurity(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["apparmor"] = "foo.apparmor" + + # update the manifest and test_manifest + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + # We should end up with 2 info + expected_counts = {'info': 2, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_disallowed(self): + '''Test check_peer_hooks() - disallowed''' + c = ClickReviewSecurity(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["apparmor"] = "foo.apparmor" + + # add something not allowed + tmp["framework"] = "foo.framework" + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) + + def test_check_redflag_policy_vendor_ubuntu(self): + '''Test check_redflag() - policy_vendor - ubuntu''' + c = ClickReviewSecurity(self.test_name) + self.set_test_security_manifest(self.default_appname, + "policy_vendor", "ubuntu") + c.check_redflag() + report = c.click_report + expected_counts = {'info': 1, 'warn': 0, 'error': 0} + self.check_results(report, expected_counts) + + def test_check_redflag_policy_vendor_ubuntu_snappy(self): + '''Test check_redflag() - policy_vendor - ubuntu-snappy''' + c = ClickReviewSecurity(self.test_name) + self.set_test_security_manifest(self.default_appname, + "policy_vendor", "ubuntu-snappy") + c.check_redflag() + report = c.click_report + expected_counts = {'info': 1, 'warn': 0, 'error': 0} + self.check_results(report, expected_counts) + + def test_check_redflag_policy_vendor_notubuntu(self): + '''Test check_redflag() - policy_vendor - notubuntu''' + c = ClickReviewSecurity(self.test_name) + self.set_test_security_manifest(self.default_appname, + "policy_vendor", "notubuntu") + c.check_redflag() + report = c.click_report + expected_counts = {'info': 0, 'warn': 0, 'error': 1} + self.check_results(report, expected_counts) + + def test_check_redflag_abstractions(self): + '''Test check_redflag() - abstractions''' + c = ClickReviewSecurity(self.test_name) + self.set_test_security_manifest(self.default_appname, + "abstractions", ["python"]) + c.check_redflag() + report = c.click_report + expected_counts = {'info': 0, 'warn': 0, 'error': 1} + self.check_results(report, expected_counts) + + def test_check_redflag_binary(self): + '''Test check_redflag() - binary''' + c = ClickReviewSecurity(self.test_name) + self.set_test_security_manifest(self.default_appname, + "binary", "/bin/foo") + c.check_redflag() + report = c.click_report + expected_counts = {'info': 0, 'warn': 0, 'error': 1} + self.check_results(report, expected_counts) + + def test_check_redflag_read_path(self): + '''Test check_redflag() - read_path''' + c = ClickReviewSecurity(self.test_name) + self.set_test_security_manifest(self.default_appname, + "read_path", ["/"]) + c.check_redflag() + report = c.click_report + expected_counts = {'info': 0, 'warn': 0, 'error': 1} + self.check_results(report, expected_counts) + + def test_check_redflag_template_variables(self): + '''Test check_redflag() - template_variables''' + c = ClickReviewSecurity(self.test_name) + self.set_test_security_manifest(self.default_appname, + "template_variables", {"FOO": "bar"}) + c.check_redflag() + report = c.click_report + expected_counts = {'info': 0, 'warn': 0, 'error': 1} + self.check_results(report, expected_counts) + + def test_check_redflag_write_path(self): + '''Test check_redflag() - write_path''' + c = ClickReviewSecurity(self.test_name) + self.set_test_security_manifest(self.default_appname, + "write_path", ["/"]) + c.check_redflag() + report = c.click_report + expected_counts = {'info': 0, 'warn': 0, 'error': 1} + self.check_results(report, expected_counts) diff -Nru click-reviewers-tools-0.19/clickreviews/tests/test_cr_url_dispatcher.py click-reviewers-tools-0.20/clickreviews/tests/test_cr_url_dispatcher.py --- click-reviewers-tools-0.19/clickreviews/tests/test_cr_url_dispatcher.py 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/clickreviews/tests/test_cr_url_dispatcher.py 2015-01-05 15:57:17.000000000 +0000 @@ -209,3 +209,55 @@ r = c.click_report expected_counts = {'info': 0, 'warn': 1, 'error': 0} self.check_results(r, expected_counts) + + def test_check_peer_hooks(self): + '''Test check_peer_hooks()''' + self.set_test_url_dispatcher(self.default_appname, + key="protocol", + value="some-protocol") + c = ClickReviewUrlDispatcher(self.test_name) + print(c.manifest["hooks"]) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["url-dispatcher"] = \ + self.test_manifest["hooks"][self.default_appname]["urls"] + + # update the manifest and test_manifest + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + # We should end up with 2 info + expected_counts = {'info': 2, 'warn': 0, 'error': 0} + self.check_results(r, expected_counts) + + def test_check_peer_hooks_disallowed(self): + '''Test check_peer_hooks() - disallowed''' + self.set_test_url_dispatcher(self.default_appname, + key="protocol", + value="some-protocol") + c = ClickReviewUrlDispatcher(self.test_name) + + # create a new hooks database for our peer hooks tests + tmp = dict() + + # add our hook + tmp["urls"] = \ + self.test_manifest["hooks"][self.default_appname]["urls"] + + # add something not allowed + tmp["nonexistent"] = "nonexistent-hook" + + c.manifest["hooks"][self.default_appname] = tmp + self._update_test_manifest() + + # do the test + c.check_peer_hooks() + r = c.click_report + expected_counts = {'info': None, 'warn': 0, 'error': 1} + self.check_results(r, expected_counts) diff -Nru click-reviewers-tools-0.19/data/apparmor-easyprof-ubuntu.json click-reviewers-tools-0.20/data/apparmor-easyprof-ubuntu.json --- click-reviewers-tools-0.19/data/apparmor-easyprof-ubuntu.json 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/data/apparmor-easyprof-ubuntu.json 2015-01-05 15:57:17.000000000 +0000 @@ -125,6 +125,69 @@ "video_files_read" ] } + }, + "1.3": { + "templates": { + "common": [ + "default", + "ubuntu-push-helper", + "ubuntu-scope-network", + "ubuntu-sdk", + "ubuntu-webapp" + ], + "reserved": [ + "unconfined" + ] + }, + "policy_groups": { + "common": [ + "accounts", + "audio", + "camera", + "connectivity", + "content_exchange", + "content_exchange_source", + "location", + "microphone", + "networking", + "push-notification-client", + "sensors", + "usermetrics", + "video", + "webview" + ], + "reserved": [ + "calendar", + "contacts", + "debug", + "history", + "music_files", + "music_files_read", + "picture_files", + "picture_files_read", + "video_files", + "video_files_read" + ] + } + } + }, + "ubuntu-snappy": { + "1.3": { + "templates": { + "common": [ + "default", + "service" + ], + "reserved": [ + "unconfined" + ] + }, + "policy_groups": { + "common": [ + "networking" + ], + "reserved": [] + } } } } diff -Nru click-reviewers-tools-0.19/debian/bzr-builder.manifest click-reviewers-tools-0.20/debian/bzr-builder.manifest --- click-reviewers-tools-0.19/debian/bzr-builder.manifest 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/debian/bzr-builder.manifest 2015-01-05 15:57:17.000000000 +0000 @@ -1,2 +1,2 @@ -# bzr-builder format 0.3 deb-version {debupstream}-0~300 -lp:click-reviewers-tools revid:jamie@ubuntu.com-20141121140457-0n9802z9wrcv3pcm +# bzr-builder format 0.3 deb-version {debupstream}-0~349 +lp:click-reviewers-tools revid:daniel.holbach@canonical.com-20150105155230-ub1u5v0jzu3u3er6 diff -Nru click-reviewers-tools-0.19/debian/changelog click-reviewers-tools-0.20/debian/changelog --- click-reviewers-tools-0.19/debian/changelog 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/debian/changelog 2015-01-05 15:57:17.000000000 +0000 @@ -1,10 +1,16 @@ -click-reviewers-tools (0.19-0~300~ubuntu14.04.1) trusty; urgency=low +click-reviewers-tools (0.20-0~349~ubuntu14.04.1) trusty; urgency=low * Auto build. - -- Daniel Holbach Fri, 21 Nov 2014 15:31:17 +0000 + -- Daniel Holbach Mon, 05 Jan 2015 15:57:17 +0000 -click-reviewers-tools (0.19) UNRELEASED; urgency=medium +click-reviewers-tools (0.20) UNRELEASED; urgency=medium + + * + + -- Daniel Holbach Mon, 05 Jan 2015 16:52:10 +0100 + +click-reviewers-tools (0.19) vivid; urgency=medium [ Ricardo Kirkner ] * fetch framework data before running framework related checks @@ -20,8 +26,26 @@ * open scopes .ini file as utf8 (LP: #1371692) * allow for translatable fields in the scopes .ini file (LP: #1392133) * don't require desktop hook with systemd or framework + * com.ubuntu.snappy can use ubuntu-devel-discuss@lists.ubuntu.com + (LP: #1395007) + * add bin-path click hook checks and tests (LP: #1395001) + * add preliminary framework hook checks and tests (LP: #1395004) + * refactor hooks checks into parent class (LP: #1395005) + * sort click-review results in print_findings + * add preliminary systemd hook checks and tests + * update apparmor policy json and adjust security checks to properly handle + different policy vendors + * update data/apparmor-easyprof-ubuntu.json for 1.3 + * don't warn if specifying 'default' with ubuntu-snappy vendor + * systemd hook renamed to snappy-systemd + * allow filenames to end with .snap + * allow flat namesapces in check_maintainer_email() + + [ Daniel Holbach ] + * Add askubuntu explanation for policy_version_is_highest. + * Add askubuntu explanation for debug builds. (LP: #1390163) - -- Jamie Strandboge Thu, 13 Nov 2014 15:00:07 -0600 + -- Daniel Holbach Tue, 16 Dec 2014 17:07:36 +0100 click-reviewers-tools (0.18) utopic; urgency=medium diff -Nru click-reviewers-tools-0.19/run-tests click-reviewers-tools-0.20/run-tests --- click-reviewers-tools-0.19/run-tests 2014-11-21 15:31:17.000000000 +0000 +++ click-reviewers-tools-0.20/run-tests 2015-01-05 15:57:17.000000000 +0000 @@ -15,10 +15,13 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . +import sys import unittest test_directory = 'clickreviews/tests/' test_filename = 'test_*' +if len(sys.argv) > 1: + test_filename = sys.argv[1] suite = unittest.TestLoader().discover(test_directory, pattern=test_filename) unittest.TextTestRunner(verbosity=2).run(suite)