From 014596d2712f85890157f5e30c20337019d5edeb Mon Sep 17 00:00:00 2001 From: Iain Learmonth Date: Mon, 16 May 2022 12:47:40 +0100 Subject: [PATCH] security: fix all bandit issues --- app/alarms.py | 4 +++- app/terraform/proxy/__init__.py | 4 +++- app/terraform/proxy/cloudfront.py | 3 ++- app/terraform/proxy/fastly.py | 6 ++++-- app/terraform/terraform.py | 27 +++++++++++++++++++-------- 5 files changed, 31 insertions(+), 13 deletions(-) diff --git a/app/alarms.py b/app/alarms.py index 5cdf155..7f71bdf 100644 --- a/app/alarms.py +++ b/app/alarms.py @@ -42,5 +42,7 @@ def _get_alarm(target: str, def get_proxy_alarm(proxy_id: int, alarm_type: str) -> Alarm: alarm = _get_alarm("proxy", alarm_type, proxy_id=proxy_id) - assert(alarm is not None) + if alarm is None: + # mypy can't tell that this will never be reached + raise RuntimeError("Creating an alarm must have failed.") return alarm diff --git a/app/terraform/proxy/__init__.py b/app/terraform/proxy/__init__.py index 285ef4e..9c653c8 100644 --- a/app/terraform/proxy/__init__.py +++ b/app/terraform/proxy/__init__.py @@ -72,8 +72,10 @@ class ProxyAutomation(TerraformAutomation): proxy.origin_id = origin.id proxy.provider = self.provider proxy.psg = subgroup + # The random usage below is good enough for its purpose: to create a slug that + # hasn't been used before. proxy.slug = tldextract.extract(origin.domain_name).domain[:5] + ''.join( - random.choices(string.ascii_lowercase, k=12)) + random.choices(string.ascii_lowercase, k=12)) # nosec proxy.added = datetime.datetime.utcnow() proxy.updated = datetime.datetime.utcnow() db.session.add(proxy) diff --git a/app/terraform/proxy/cloudfront.py b/app/terraform/proxy/cloudfront.py index b473be0..f88bcb8 100644 --- a/app/terraform/proxy/cloudfront.py +++ b/app/terraform/proxy/cloudfront.py @@ -74,7 +74,8 @@ class ProxyCloudfrontAutomation(ProxyAutomation): """ def import_state(self, state: Any) -> None: - assert(isinstance(state, dict)) + if not isinstance(dict, state): + raise RuntimeError("The Terraform state object returned was not a dict.") if "child_modules" not in state['values']['root_module']: # There are no CloudFront proxies deployed to import state for return diff --git a/app/terraform/proxy/fastly.py b/app/terraform/proxy/fastly.py index 2749ff5..7607ed2 100644 --- a/app/terraform/proxy/fastly.py +++ b/app/terraform/proxy/fastly.py @@ -3,8 +3,8 @@ import datetime import os -import string import random +import string import jinja2 import tldextract @@ -97,8 +97,10 @@ def create_missing_proxies(): proxy = Proxy() proxy.origin_id = origin.id proxy.provider = "fastly" + # The random usage below is good enough for its purpose: to create a slug that + # hasn't been used before. proxy.slug = tldextract.extract(origin.domain_name).domain[:5] + ''.join( - random.choices(string.ascii_lowercase, k=random.randint(5, 10))) + random.choices(string.ascii_lowercase, 12)) # nosec proxy.added = datetime.datetime.utcnow() proxy.updated = datetime.datetime.utcnow() db.session.add(proxy) diff --git a/app/terraform/terraform.py b/app/terraform/terraform.py index 943e961..af75856 100644 --- a/app/terraform/terraform.py +++ b/app/terraform/terraform.py @@ -1,7 +1,7 @@ import json -import subprocess +import subprocess # nosec from abc import abstractmethod -from typing import Any, Dict, List, Optional, Tuple +from typing import Any, Optional, Tuple import jinja2 @@ -54,7 +54,10 @@ class TerraformAutomation(BaseAutomation): lock_timeout: int = 15) -> Tuple[int, str]: if not parallelism: parallelism = self.parallelism - tf = subprocess.run( + # The following subprocess call takes external input, but is providing + # the argument list as an array such that argument injection would be + # ineffective. + tf = subprocess.run( # nosec ['terraform', 'apply', '-auto-approve', @@ -73,8 +76,11 @@ class TerraformAutomation(BaseAutomation): def tf_init(self, *, lock_timeout: int = 15) -> None: - # The init command does not support JSON output - subprocess.run( + # The init command does not support JSON output. + # The following subprocess call takes external input, but is providing + # the argument list as an array such that argument injection would be + # ineffective. + subprocess.run( # nosec ['terraform', 'init', f'-lock-timeout={str(lock_timeout)}m', @@ -82,7 +88,8 @@ class TerraformAutomation(BaseAutomation): cwd=self.working_directory()) def tf_output(self) -> Any: - tf = subprocess.run( + # The following subprocess call does not take any user input. + tf = subprocess.run( # nosec ['terraform', 'output', '-json'], cwd=self.working_directory(), stdout=subprocess.PIPE) @@ -92,7 +99,10 @@ class TerraformAutomation(BaseAutomation): refresh: bool = True, parallelism: Optional[int] = None, lock_timeout: int = 15) -> Tuple[int, str]: - tf = subprocess.run( + # The following subprocess call takes external input, but is providing + # the argument list as an array such that argument injection would be + # ineffective. + tf = subprocess.run( # nosec ['terraform', 'plan', '-json', @@ -128,7 +138,8 @@ class TerraformAutomation(BaseAutomation): pass def tf_show(self) -> Any: - terraform = subprocess.run( + # This subprocess call doesn't take any user input. + terraform = subprocess.run( # nosec ['terraform', 'show', '-json'], cwd=self.working_directory(), stdout=subprocess.PIPE)