From 55b7f4e412cab4d259f58ac935dcf2a60b5aa3cc Mon Sep 17 00:00:00 2001 From: Cleto Martin Date: Mon, 29 Jul 2024 14:20:12 +0200 Subject: [PATCH] Remove deprecated IBM finding plugin BREAKING CHANGES: this notifier will be removed. Seems like ibm_cloud_security_advisor is [now deprecated](https://github.com/ibm-cloud-security/security-advisor-sdk-python/). It is having issues compatibility issues with Python 3.12. --- .flake8 | 8 + .pre-commit-config.yaml | 8 +- CHANGES.md | 4 + compliance/__init__.py | 2 +- compliance/config.py | 3 - compliance/fetch.py | 6 +- compliance/locker.py | 2 +- compliance/notify.py | 81 +--------- compliance/utils/services/pagerduty.py | 12 +- doc-source/notifiers.rst | 32 +--- setup.cfg | 7 +- test/t_compliance/t_controls/test_controls.py | 4 +- test/t_compliance/t_evidence/test_signing.py | 4 +- .../t_notify/test_findings_notifier.py | 141 ------------------ 14 files changed, 41 insertions(+), 273 deletions(-) create mode 100644 .flake8 delete mode 100644 test/t_compliance/t_notify/test_findings_notifier.py diff --git a/.flake8 b/.flake8 new file mode 100644 index 00000000..01d2c6bc --- /dev/null +++ b/.flake8 @@ -0,0 +1,8 @@ +[flake8] +max-line-length = 120 +exclude = + .venv + .build + __pycache__ + .idea + build diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index e5e0b027..5ed8518e 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,22 +1,22 @@ repos: - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v4.4.0 + rev: v4.6.0 hooks: - id: trailing-whitespace - id: check-yaml - id: fix-encoding-pragma args: ["--remove"] # Not needed on python3 - repo: https://github.com/ambv/black - rev: 22.12.0 + rev: 24.4.2 hooks: - id: black - repo: https://github.com/PyCQA/flake8 - rev: 6.0.0 + rev: 7.1.0 hooks: - id: flake8 files: "^(compliance|test|demo)" - repo: https://github.com/PyCQA/bandit - rev: 1.7.4 + rev: 1.7.9 hooks: - id: bandit args: [--recursive] diff --git a/CHANGES.md b/CHANGES.md index 53b68f29..a12d46ef 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,7 @@ +# [3.0.0](https://github.com/ComplianceAsCode/auditree-framework/releases/tag/v3.0.0) + +- [CHANGED] Remove IBM findings notifier. + # [2.0.1](https://github.com/ComplianceAsCode/auditree-framework/releases/tag/v2.0.1) - [FIXED] Enable universal newlines when executing local commands. diff --git a/compliance/__init__.py b/compliance/__init__.py index fefc1045..6c875fee 100644 --- a/compliance/__init__.py +++ b/compliance/__init__.py @@ -13,4 +13,4 @@ # limitations under the License. """Compliance automation package.""" -__version__ = "2.0.1" +__version__ = "3.0.0" diff --git a/compliance/config.py b/compliance/config.py index 916c0095..41aa4f22 100644 --- a/compliance/config.py +++ b/compliance/config.py @@ -49,9 +49,6 @@ class ComplianceConfig(object): # Pagerduty service id to use for an accreditation # E.g. {"mycompany.soc2": "ABCDEFG"} "pagerduty": {}, - # Security Advisor FindingsAPI endpoint to use for an accreditation - # E.g. {"mycompany.soc2": "https://my.findings.api/findings"} - "findings": {}, }, "org": {"name": "YOUR_ORG", "settings": {}}, } diff --git a/compliance/fetch.py b/compliance/fetch.py index a4e99fbf..7b97aae9 100644 --- a/compliance/fetch.py +++ b/compliance/fetch.py @@ -90,7 +90,7 @@ def fetchURL(self, url, params=None, creds=None): # noqa: N802 org = self.config.raw_config.get("org", {}).get("name", "") ua = f'{org.lower().replace(" ", "-")}-compliance-checks' response = requests.get( - url, params=params, auth=creds, headers={"User-Agent": ua} + url, params=params, auth=creds, headers={"User-Agent": ua}, timeout=3600 ) response.raise_for_status() return response.content @@ -151,7 +151,7 @@ def fetchLocalCommands( # noqa: N802 if not cwd: cwd = os.path.expanduser("~") stdin = "\n".join(commands) + "\n" - return check_output( + return check_output( # nosec: B603 The input command can be anything. cmd, cwd=cwd, env=env, input=stdin, timeout=timeout, universal_newlines=True ).rstrip() @@ -165,7 +165,7 @@ def fetch(url, name): :returns: the path to the file. """ - r = requests.get(url) + r = requests.get(url, timeout=3600) r.raise_for_status() path = Path(tempfile.gettempdir(), name) with path.open("wb") as f: diff --git a/compliance/locker.py b/compliance/locker.py index d25b5a10..04fec69a 100644 --- a/compliance/locker.py +++ b/compliance/locker.py @@ -654,7 +654,7 @@ def get_locker_repo(self, locker="evidence locker"): def init_config(self): """Apply the git configuration.""" with self.repo.config_writer() as cw: - for (section, cfg) in self.gitconfig.items(): + for section, cfg in self.gitconfig.items(): for key, value in cfg.items(): cw.set_value(section, key, value) diff --git a/compliance/notify.py b/compliance/notify.py index 99ecc86e..5c8fda43 100644 --- a/compliance/notify.py +++ b/compliance/notify.py @@ -27,11 +27,6 @@ from compliance.utils.services.github import Github from compliance.utils.test import parse_test_id -from ibm_cloud_sdk_core.api_exception import ApiException -from ibm_cloud_sdk_core.authenticators import IAMAuthenticator - -from ibm_cloud_security_advisor import FindingsApiV1 - import requests @@ -861,7 +856,9 @@ def _send_message(self, message, channels): retries = self._config.get("retries", 3) retry = 0 while retry < retries: - response = requests.post(url, headers=headers, data=json.dumps(msg)) + response = requests.post( + url, headers=headers, data=json.dumps(msg), timeout=180 + ) if response.status_code == 429: time.sleep(int(response.headers.get("Retry-After", retry)) + 1) retry += 1 @@ -1050,77 +1047,6 @@ def _resolve_alert(self, test_id, test_desc, msg, accreditation): ) -class FindingsNotifier(_BaseNotifier): - """ - Findings notifier class. - - Notifications are sent using the Findings API. This notifier is - configurable via :class:`compliance.config.ComplianceConfig`. - """ - - def __init__(self, results, controls, push_error=False): - """ - Construct and initialize the Findings notifier object. - - :param results: dictionary generated by - :py:class:`compliance.runners.CheckMode` at the end of the execution. - :param controls: the control descriptor that manages accreditations. - """ - super(FindingsNotifier, self).__init__(results, controls, push_error) - self._config = get_config().get("notify.findings") - self._creds = get_config().creds - api_key = self._creds["findings"].api_key - authenticator = IAMAuthenticator(apikey=api_key) - self.findings_api = FindingsApiV1(authenticator=authenticator) - - def notify(self): - """Send notifications to the Findings API.""" - if self._push_error: - self.logger.error( - "Remote locker push failed. Findings notifier not triggered." - ) - return - self.logger.info("Running the Findings notifier...") - if not self._config: - self.logger.warning("Using findings notification without config") - - messages = list(self._messages_by_accreditations().items()) - messages.sort(key=lambda x: x[0]) - for accreditation, desc in messages: - if accreditation not in self._config: - continue - findings_api_endpoint = self._config[accreditation] - self.findings_api.set_service_url(findings_api_endpoint) - - passed, failed, warned, errored = self._split_by_status(desc) - for _, _, msg in failed + errored + passed + warned: - self._create_findings(msg["body"]) - - def _create_findings(self, data): - occurrence_list = data["occurrence_list"] - account_id = data["account_id"] - provider_id = data["provider_id"] - status = 0 - - for occurrence in occurrence_list: - try: - response = self.findings_api.create_occurrence( - account_id=account_id, provider_id=provider_id, **occurrence - ) - self.logger.info(response.status_code) - except ApiException as e: - status = e.code - self.logger.error( - "Finding creation failed " - f'for occurrence id {occurrence["id"]} ' - f"with {str(e.code)}: {str(e)}" - ) - except Exception as e: - status = -1 - self.logger.error(f"Unexpected error occurred: {str(e)}") - return status - - def get_notifiers(): """ Provide a dictionary of all notifier class objects. @@ -1137,5 +1063,4 @@ def get_notifiers(): "pagerduty": PagerDutyNotifier, "gh_issues": GHIssuesNotifier, "locker": LockerNotifier, - "findings": FindingsNotifier, } diff --git a/compliance/utils/services/pagerduty.py b/compliance/utils/services/pagerduty.py index 52a6ff34..7f0f3cbb 100644 --- a/compliance/utils/services/pagerduty.py +++ b/compliance/utils/services/pagerduty.py @@ -56,7 +56,7 @@ def get(path, params=None, headers=None, creds=None): params.update({"limit": PAGES_LIMIT, "offset": offset}) more = True while more: - r = requests.get(url, headers=hdrs, params=params) + r = requests.get(url, headers=hdrs, params=params, timeout=180) yield r more = r.json().get("more", False) if more: @@ -77,7 +77,7 @@ def delete(path, params=None, headers=None, creds=None): :param creds: a Config object with PagerDuty credentials """ url, params, hdrs = _init_request(path, params, headers, creds) - return requests.delete(url, headers=hdrs, params=params) + return requests.delete(url, headers=hdrs, params=params, timeout=180) def put(path, params=None, headers=None, creds=None): @@ -93,7 +93,7 @@ def put(path, params=None, headers=None, creds=None): :param creds: a Config object with PagerDuty credentials """ url, params, hdrs = _init_request(path, params, headers, creds) - return requests.put(url, headers=hdrs, params=params) + return requests.put(url, headers=hdrs, params=params, timeout=180) def post(path, params=None, headers=None, creds=None): @@ -109,7 +109,7 @@ def post(path, params=None, headers=None, creds=None): :param creds: a Config object with PagerDuty credentials """ url, params, hdrs = _init_request(path, params, headers, creds) - return requests.post(url, headers=hdrs, params=params) + return requests.post(url, headers=hdrs, params=params, timeout=180) def send_event( @@ -132,7 +132,9 @@ def send_event( } headers = {"Content-Type": "application/json"} - response = requests.post(PD_EVENTS_V2_URL, headers=headers, data=json.dumps(msg)) + response = requests.post( + PD_EVENTS_V2_URL, headers=headers, data=json.dumps(msg), timeout=180 + ) response.raise_for_status() if response.json().get("status") != "success": raise RuntimeError("PagerDuty Error: " + response.json()) diff --git a/doc-source/notifiers.rst b/doc-source/notifiers.rst index 11fbee8f..dfe24fa5 100644 --- a/doc-source/notifiers.rst +++ b/doc-source/notifiers.rst @@ -8,7 +8,7 @@ Notifiers The last phase in a typical framework check run is the notification system. Multiple notifiers can be targeted as part of this phase by using the ``--notify`` option on the ``compliance --check`` command. Valid -notifier options are ``stdout``, ``slack``, ``pagerduty``, ``findings``, +notifier options are ``stdout``, ``slack``, ``pagerduty``, ``gh_issues`` and, ``locker``. The general idea behind the notification system is that each ``test_`` can generate a short notification that has the following components: @@ -347,33 +347,3 @@ locker. The summary markdown file will **only** be pushed to the remote evidence locker if the ``full-remote`` argument is applied to the ``evidence`` option when executing your checks otherwise the file will remain in the local evidence locker. No additional configuration is required for this notifier. - -Security Advisor Findings -------------------------- - -This configurable notifier will post findings to Security Advisor Findings API -per accreditation. The following is an example configuration for this notifier -to be added to a configuration file and used with the ``-C`` option when -executing your compliance checks:: - - { - "notify": { - "findings": { - "accr1": "https://us-south.secadvisor.cloud.ibm.com/findings", - "accr2": "https://eu-gb.secadvisor.cloud.ibm.com/findings" - } - } - } - -Supported regions for Security Advisor Findings API - - us-south: https://us-south.secadvisor.cloud.ibm.com/findings - - eu-gb: https://eu-gb.secadvisor.cloud.ibm.com/findings - -This notifier also needs to know the credentials for sending findings -to Security Advisor Findings API. Include the following in your credentials -file:: - - [findings] - api_key=platform-api-key - -``api_key`` is your IBM Cloud Platform API Key. diff --git a/setup.cfg b/setup.cfg index ba9f9472..58a76d31 100644 --- a/setup.cfg +++ b/setup.cfg @@ -7,10 +7,11 @@ author_email = al.finkelstein@ibm.com url = https://auditree.github.io/ license = Apache License 2.0 classifiers = - Programming Language :: Python :: 3.6 - Programming Language :: Python :: 3.7 Programming Language :: Python :: 3.8 Programming Language :: Python :: 3.9 + Programming Language :: Python :: 3.10 + Programming Language :: Python :: 3.11 + Programming Language :: Python :: 3.12 License :: OSI Approved :: Apache Software License Operating System :: MacOS :: MacOS X Operating System :: POSIX :: Linux @@ -24,9 +25,9 @@ install_requires = inflection>=0.3.1 GitPython>=2.1.3 jinja2>=2.10 - ibm_cloud_security_advisor>=2.0.0 ilcli>=0.3.1 cryptography>=35.0.0 + requests>=2.30.0 [options.packages.find] exclude = diff --git a/test/t_compliance/t_controls/test_controls.py b/test/t_compliance/t_controls/test_controls.py index 9395c636..7ffddcdd 100644 --- a/test/t_compliance/t_controls/test_controls.py +++ b/test/t_compliance/t_controls/test_controls.py @@ -14,6 +14,7 @@ """Compliance automation control descriptor tests module.""" import unittest +import pytest from compliance.controls import ControlDescriptor @@ -55,9 +56,8 @@ def test_contructor_and_base_properties(self): def test_as_dict_immutability(self): """Ensure that control content cannot be changed through as_dict.""" - with self.assertRaises(AttributeError) as ar: + with pytest.raises(AttributeError): self.cd.as_dict = {"foo": "bar"} - self.assertTrue(str(ar.exception).startswith("can't set attribute")) controls_copy = self.cd.as_dict self.assertEqual(controls_copy, self.cd.as_dict) controls_copy.update({"foo": "bar"}) diff --git a/test/t_compliance/t_evidence/test_signing.py b/test/t_compliance/t_evidence/test_signing.py index d3a87ba2..7d50ec44 100644 --- a/test/t_compliance/t_evidence/test_signing.py +++ b/test/t_compliance/t_evidence/test_signing.py @@ -42,7 +42,8 @@ def setUp(self): self.agent = ComplianceAgent(name="auditree.local") self.unknown_agent = ComplianceAgent(name="unknown.local") - self.agent.private_key = self.unknown_agent.private_key = """ + self.agent.private_key = self.unknown_agent.private_key = ( + """ -----BEGIN RSA PRIVATE KEY----- MIIEpAIBAAKCAQEAxYosRYnahnSuH3SmNupnzQhxJsDEhqChKjrcyN19L8+vcjUU iMSaKRoAHuUKp5Pfwkoylryd4AyXIU9UnXZgdIOl2+r5xzXqfdLwi+PAU/eEWPLA @@ -71,6 +72,7 @@ def setUp(self): YLmlUVKZU7mt43D8aj8l4l11jWDBkvOba/wJ7CjTQ15ik4ntl9TRzg== -----END RSA PRIVATE KEY----- """.encode() + ) self.pub_key = """ -----BEGIN PUBLIC KEY----- diff --git a/test/t_compliance/t_notify/test_findings_notifier.py b/test/t_compliance/t_notify/test_findings_notifier.py deleted file mode 100644 index e07a60cd..00000000 --- a/test/t_compliance/t_notify/test_findings_notifier.py +++ /dev/null @@ -1,141 +0,0 @@ -# Copyright (c) 2020 IBM Corp. All rights reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -"""Compliance automation findings notifier tests module.""" - -import time -import unittest -from unittest import mock -from unittest.mock import MagicMock, Mock, create_autospec, patch - -from compliance.config import ComplianceConfig -from compliance.controls import ControlDescriptor -from compliance.notify import FindingsNotifier - -from ibm_cloud_sdk_core.api_exception import ApiException -from ibm_cloud_sdk_core.authenticators import IAMAuthenticator - -from requests.models import Response - -from .. import build_compliance_check_obj - - -class FindingsNotifierTest(unittest.TestCase): - """FindingsNotifier test class.""" - - def setUp(self): - """Initialize each test.""" - self.results = {} - for status in ["pass", "fail", "error", "warn"]: - name, result = self._generate_result(status) - self.results[name] = result - note_name = "account_id/providers/provider_id/notes/note_id" - self.data = { - "occurrence_list": [ - { - "id": "test-id", - "kind": "FINDING", - "note_name": note_name, - "context": { - "region": "region", - "resource_id": "resource_id", - "resource_type": "resource_type", - "resource_name": "resource_name", - "service_name": "Auditree", - }, - "finding": {"severity": "severity"}, - } - ], - "account_id": "account_id", - "provider_id": "provider_id", - } - - @patch("compliance.notify.get_config") - @patch("compliance.notify.IAMAuthenticator") - def test_notify_findings_success(self, get_config_mock, iam_mock): - """Test Findings notifier sends content to SA findings.""" - config_mock = create_autospec(ComplianceConfig) - config_mock.get.return_value = {"audit": "https://localhost"} - get_config_mock.return_value = config_mock - controls_mock = create_autospec(ControlDescriptor) - controls_mock.get_accreditations.return_value = ["audit"] - iam_mock = create_autospec(IAMAuthenticator) - iam_mock.validate.return_value = True - with mock.patch("compliance.notify.FindingsApiV1") as findings_mock: - the_response = Mock(spec=Response) - the_response.json.return_value = {} - the_response.status_code = 200 - mock_findings = findings_mock.return_value - mock_findings.create_occurrence.return_value = the_response - notifier = FindingsNotifier(self.results, controls_mock) - notifier.notify() - result = notifier._create_findings(self.data) - - self.assertEqual(result, 0) - - @patch("compliance.notify.get_config") - @patch("compliance.notify.IAMAuthenticator") - def test_notify_findings_409_exception(self, get_config_mock, iam_mock): - """Test Findings notifier handles 409 conflicts.""" - config_mock = create_autospec(ComplianceConfig) - config_mock.get.return_value = {"audit": "https://localhost"} - get_config_mock.return_value = config_mock - controls_mock = create_autospec(ControlDescriptor) - controls_mock.get_accreditations.return_value = ["audit"] - iam_mock = create_autospec(IAMAuthenticator) - iam_mock.validate.return_value = True - with mock.patch("compliance.notify.FindingsApiV1") as findings_mock: - mock_findings = findings_mock.return_value - mock_findings.create_occurrence.side_effect = ApiException(code=409) - notifier = FindingsNotifier(self.results, controls_mock) - notifier.notify() - result = notifier._create_findings(self.data) - - self.assertEqual(result, 409) - - @patch("compliance.notify.get_config") - @patch("compliance.notify.IAMAuthenticator") - def test_notify_findings_unexpected_error(self, get_config_mock, iam_mock): - """Test Findings notifier handles unexpected errors.""" - config_mock = create_autospec(ComplianceConfig) - config_mock.get.return_value = {"audit": "https://localhost"} - get_config_mock.return_value = config_mock - controls_mock = create_autospec(ControlDescriptor) - controls_mock.get_accreditations.return_value = ["audit"] - iam_mock = create_autospec(IAMAuthenticator) - iam_mock.validate.return_value = True - with mock.patch("compliance.notify.FindingsApiV1") as findings_mock: - mock_findings = findings_mock.return_value - mock_findings.create_occurrence.side_effect = Exception( - {"status_code": 500} - ) - notifier = FindingsNotifier(self.results, controls_mock) - notifier.notify() - result = notifier._create_findings(self.data) - - self.assertEqual(result, -1) - - def _build_check_mock(self, name): - check_mock = MagicMock() - check_mock.test = build_compliance_check_obj(name, name, name, [name]) - return check_mock - - def _generate_result(self, status): - return ( - f"compliance.test.{status}_example", - { - "status": status, - "timestamp": time.time(), - "test": self._build_check_mock(f"{status}_example"), - }, - )