Skip to content

Commit

Permalink
Check for security vulnerabilities (#599)
Browse files Browse the repository at this point in the history
* Check for security vulnerabilities

https://testdriven.io/tips/dfbe77b2-6912-41ff-9d33-6d7a6a5a8e26/

* Fix security issue in JSON encoder monkey patch

* Unit test monkey patch

* Choose more descriptive variable names
  • Loading branch information
brainix authored Jan 17, 2022
1 parent c5520a9 commit 182748e
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 11 deletions.
7 changes: 5 additions & 2 deletions .github/workflows/python-package.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,12 @@ jobs:
coverage3 run -m unittest discover --start-directory tests --verbose
coverage3 report
- name: Type check with Mypy
run: |
mypy
run: mypy
- name: Lint with Flake8 and isort
run: |
flake8 *\.py pottery/*\.py tests/*\.py --count --max-complexity=10 --statistics
isort *\.py pottery/*\.py tests/*\.py --check-only --diff
- name: Check for security vulnerabilities with Bandit and Safety
run: |
bandit -r pottery
safety check
21 changes: 15 additions & 6 deletions pottery/monkey.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@

import logging
from typing import Any
from typing import Callable
from typing import Dict
from typing import List
from typing import Union
from typing import cast

# TODO: When we drop support for Python 3.7, change the following import to:
# from typing import Final
Expand All @@ -37,12 +39,19 @@
# already knows how to JSONify dicts, lists, and strings).

def _default(self: Any, obj: Any) -> Union[Dict[str, Any], List[Any], str]:
func_names = {'to_dict', 'to_list', 'to_str'}
funcs = {getattr(obj.__class__, name, None) for name in func_names}
funcs.discard(None)
assert len(funcs) <= 1
func = funcs.pop() if any(funcs) else _default.default # type: ignore
return_value = func(obj) # type: ignore
method_names = ('to_dict', 'to_list', 'to_str')
methods = tuple(getattr(obj.__class__, name, None) for name in method_names)
methods = tuple(method for method in methods if method is not None)
if len(methods) > 1:
methods_defined = ', '.join(
cast(Callable, method).__qualname__ + '()' for method in methods
)
raise TypeError(
f"{methods_defined} defined; "
f"don't know how to JSONify {obj.__class__.__name__} objects"
)
method = methods[0] if methods else _default.default # type: ignore
return_value = method(obj) # type: ignore
return return_value

import json # isort: skip
Expand Down
2 changes: 1 addition & 1 deletion pottery/queue.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ def get(self,
except (WatchError, IndexError):
if not block or timer.elapsed() / 1000 >= (timeout or 0):
raise QueueEmptyError(redis=self.redis, key=self.key)
delay = random.uniform(0, self.RETRY_DELAY/1000)
delay = random.uniform(0, self.RETRY_DELAY/1000) # nosec
time.sleep(delay)

__get = get
Expand Down
2 changes: 1 addition & 1 deletion pottery/redlock.py
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ def log_time_enqueued(timer: ContextTimer, acquired: bool) -> None:
log_time_enqueued(timer, True)
return True
enqueued = True
delay = random.uniform(0, self.RETRY_DELAY/1000)
delay = random.uniform(0, self.RETRY_DELAY/1000) # nosec
time.sleep(delay)
if enqueued:
log_time_enqueued(timer, False)
Expand Down
3 changes: 3 additions & 0 deletions requirements-to-freeze.txt
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ types-redis
flake8
isort

bandit
safety

twine


Expand Down
11 changes: 11 additions & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
bandit==1.7.1
bleach==4.1.0
certifi==2021.10.8
charset-normalizer==2.0.10
click==8.0.3
colorama==0.4.4
coverage==6.2
Deprecated==1.2.13
docutils==0.18.1
dparse==0.5.1
flake8==4.0.1
gitdb==4.0.9
GitPython==3.1.26
hiredis==2.0.0
idna==3.3
importlib-metadata==4.2.0
Expand All @@ -16,17 +21,23 @@ mmh3==3.0.0
mypy==0.931
mypy-extensions==0.4.3
packaging==21.3
pbr==5.8.0
pkginfo==1.8.2
pycodestyle==2.8.0
pyflakes==2.4.0
Pygments==2.11.2
pyparsing==3.0.6
PyYAML==6.0
readme-renderer==32.0
redis==4.1.0
requests==2.27.1
requests-toolbelt==0.9.1
rfc3986==2.0.0
safety==1.10.3
six==1.16.0
smmap==5.0.0
stevedore==3.5.0
toml==0.10.2
tomli==2.0.0
tqdm==4.62.3
twine==3.7.1
Expand Down
30 changes: 29 additions & 1 deletion tests/test_monkey.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,37 @@
from tests.base import TestCase


class Incorrect:
def to_dict(self):
return {}

def to_list(self):
return []


class Correct:
def to_dict(self):
return {}


class MonkeyPatchTests(TestCase):
def test_json_encoder(self):
def test_typeerror_not_jsonifyable(self):
try:
json.dumps(object())
except TypeError as error:
assert str(error) == 'Object of type object is not JSON serializable'

def test_typeerror_multiple_methods(self):
try:
json.dumps(Incorrect())
except TypeError as error:
assert str(error) == (
"Incorrect.to_dict(), Incorrect.to_list() defined; "
"don't know how to JSONify Incorrect objects"
)

def test_dict(self):
assert json.dumps({}) == '{}'

def test_to_dict(self):
assert json.dumps(Correct()) == '{}'

0 comments on commit 182748e

Please sign in to comment.