From b5a9c42df147b1bd24342fcb04a23ef6948f2408 Mon Sep 17 00:00:00 2001 From: Anish Lakhwara Date: Thu, 27 Jul 2023 06:29:37 +1000 Subject: [PATCH] feat: add pre-commit to check we don't have real passwords in yml files (#990) * feat: use existing pre-commit framework * feat(ci): add github action for password_check * feat: add some simple tests to password_check.py * fix: set `backend_password_secret` in default values.yaml to an allowed password --- .github/workflows/password-check.yaml | 40 ++++++++++++++ .pre-commit-config.yaml | 7 +++ chart/values.yaml | 2 +- scripts/check_passwords.py | 47 ++++++++++++++++ scripts/test/test_check_passwords.py | 79 +++++++++++++++++++++++++++ 5 files changed, 174 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/password-check.yaml create mode 100644 scripts/check_passwords.py create mode 100644 scripts/test/test_check_passwords.py diff --git a/.github/workflows/password-check.yaml b/.github/workflows/password-check.yaml new file mode 100644 index 00000000..e7821b22 --- /dev/null +++ b/.github/workflows/password-check.yaml @@ -0,0 +1,40 @@ +name: Password Check + +on: + push: + paths: + - '*.yaml' + - '*.yml' + pull_request: + paths: + - '*.yaml' + - '*.yml' + +jobs: + check: + runs-on: ubuntu-latest + steps: + - name: checkout + uses: actions/checkout@v3 + with: + fetch-depth: 3 + + - name: Set up Python + uses: actions/setup-python@v4 + with: + python-version: '3.10' + + - name: Install dependencies + run: | + cd backend/ + python -m pip install --upgrade pip + pip install pyyaml + + - name: Password Check + run: | + CHANGED_FILES=$(git diff --name-only HEAD^..HEAD) + echo $CHANGED_FILES + YML_FILES=$(echo "$CHANGED_FILES" | { grep ".yml$\|.yaml$" || true; }) + if [[ -n "$YML_FILES" ]]; then + python3 scripts/check_passwords.py $YML_FILES + fi diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f8104ce9..e1c5ef93 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -19,3 +19,10 @@ repos: language: system entry: frontend/.husky/pre-commit pass_filenames: false +- repo: local + hooks: + - id: password-check + name: password-check + language: system + types: [yaml] + entry: python3 scripts/check_passwords.py diff --git a/chart/values.yaml b/chart/values.yaml index e9060023..310077f7 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -91,7 +91,7 @@ default_org: "My Organization" backend_image: "docker.io/webrecorder/browsertrix-backend:latest" backend_pull_policy: "Always" -backend_password_secret: "c9085f33ecce4347aa1d69339e16c499" +backend_password_secret: "PASSWORD!" # number of backend pods backend_num_replicas: 1 diff --git a/scripts/check_passwords.py b/scripts/check_passwords.py new file mode 100644 index 00000000..dbf51c55 --- /dev/null +++ b/scripts/check_passwords.py @@ -0,0 +1,47 @@ +"A small dirty script to check that none of the password config options have been set to real passwords" +from collections.abc import Generator +import yaml +import sys + + +ALLOWED_PASSWORDS = ["PassW0rd!", "password", "PASSWORD@", "PASSW0RD!", "PASSWORD!"] + +def key_finder(d: dict, key: str = "password", top_level = None) -> Generator: + """This recursive function yields all the keys in {d} that _contains_ the string {key} + + :param dict d: The dictionary to dive through + :param str key: The phrase we are going to match keys against + :return: A generator that creates tuples containing Optional[top_level_key], key, value + :rtype Union[tuple[str, str], tuple[str, str, str]] + """ + if d is None: + return {} + for k, v in d.items(): + if isinstance(v, dict): + if top_level is None: + yield from key_finder(v, key, k) # Pass the top level name into the recursive descent + else: + yield from key_finder(v, key, top_level) # name isn't the top level key + if key in str(k): # Sometimes yaml gets parsed with key True + if top_level is None: + yield k, v # Key is already top level + else: + yield top_level, k, v # Use the top level name + +WE_DUN_GOOFED: bool = False + +changed_files = sys.argv[1:] # Ignore filename of this script +for file in changed_files: + with open(file, 'r') as f: + yml = yaml.safe_load(f) + gen = key_finder(yml) + for password_keys in gen: + if password_keys[-1] not in ALLOWED_PASSWORDS: + if len(password_keys) == 2: + print(f"top level key '{password_keys[0]}' in {file} contains a real password!") + else: + print(f"top level key '{password_keys[0]}' with subkey '{password_keys[1]}' in {file} contains a real password!") + WE_DUN_GOOFED = True + +if WE_DUN_GOOFED: + exit(1) diff --git a/scripts/test/test_check_passwords.py b/scripts/test/test_check_passwords.py new file mode 100644 index 00000000..e632877a --- /dev/null +++ b/scripts/test/test_check_passwords.py @@ -0,0 +1,79 @@ +import pytest +import yaml +from os import listdir +# Import hacking for script +import sys +sys.path.insert(0, '.') +import check_passwords + +@pytest.fixture +def yaml_files(tmp_path): + with_password = """ + nested: + deep: + in_the_land: + is_a_password: thisislegit! + not_nested_password: uh_oh_i_commited_creds + """ + with_allowed_password = """ + nested: + deep: + in_the_land: + is_a_password: PassW0rd! + not_nested_password: password + """ + example_yaml = """ + doe: "a deer, a female deer" + ray: "a drop of golden sun" + pi: 3.14159 + xmas: true + french-hens: 3 + calling-birds: + - huey + - dewey + - louie + - fred + xmas-fifth-day: + calling-birds: four + french-hens: 3 + golden-rings: 5 + partridges: + count: 1 + location: "a pear tree" + turtle-doves: two + """ + with open(tmp_path / "with_password.yaml", 'w') as fobj: + fobj.write(with_password) + + with open(tmp_path / "with_allowed_password.yaml", 'w') as fobj: + fobj.write(with_allowed_password) + + with open(tmp_path / "example.yaml", 'w') as fobj: + fobj.write(example_yaml) + return tmp_path + +class TestCheckPasswords: + def test_find_passwords(self, yaml_files): + with open(yaml_files / "with_password.yaml", 'r') as fobj: + yml = yaml.safe_load(fobj) + gen = check_passwords.key_finder(yml) + assert ('nested', 'is_a_password', "thisislegit!") == next(gen) + assert ('not_nested_password', 'uh_oh_i_commited_creds') == next(gen) + + def test_dont_find_passwords(self, yaml_files): + with open(yaml_files / "with_allowed_password.yaml", 'r') as fobj: + yml = yaml.safe_load(fobj) + gen = check_passwords.key_finder(yml) + (_, _, password) = next(gen) + assert password in ["PassW0rd!", "password"] + (_, password) = next(gen) + assert password in ["PassW0rd!", "password"] + with pytest.raises(StopIteration): + next(gen) + + def test_parsing_yaml(self, yaml_files): + with open(yaml_files / "example.yaml", 'r') as fobj: + yml = yaml.safe_load(fobj) + gen = check_passwords.key_finder(yml) + with pytest.raises(StopIteration): + next(gen)