From e0af60677ae1433a24feee1fe86f8504a6063b58 Mon Sep 17 00:00:00 2001 From: deacon Date: Tue, 17 Mar 2026 23:17:41 -0400 Subject: [PATCH] Fix copilot review feedback on security tests - Normalize PLUGIN_DIR with os.path.abspath - Use pytest.importorskip('yaml') instead of bare import - Guard ATOMICS_DIR test with pytest.skip when directory is missing - Add encoding='utf-8' to all file open() calls - Handle None keyword.arg in AST keyword scanning --- tests/test_atomic_security.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/tests/test_atomic_security.py b/tests/test_atomic_security.py index d593d81..03db0d5 100644 --- a/tests/test_atomic_security.py +++ b/tests/test_atomic_security.py @@ -2,9 +2,10 @@ import os import pytest -import yaml -PLUGIN_DIR = os.path.join(os.path.dirname(__file__), '..') +yaml = pytest.importorskip("yaml") + +PLUGIN_DIR = os.path.abspath(os.path.join(os.path.dirname(__file__), '..')) ATOMIC_SVC_PATH = os.path.join(PLUGIN_DIR, 'app', 'atomic_svc.py') PAYLOADS_DIR = os.path.join(PLUGIN_DIR, 'payloads') DATA_DIR = os.path.join(PLUGIN_DIR, 'data') @@ -16,7 +17,7 @@ class TestAtomicSvcMD5Security: @pytest.fixture(autouse=True) def _load_source(self): - with open(ATOMIC_SVC_PATH, 'r') as f: + with open(ATOMIC_SVC_PATH, 'r', encoding='utf-8') as f: self.source = f.read() self.tree = ast.parse(self.source) @@ -53,7 +54,7 @@ def test_md5_calls_use_usedforsecurity_false(self): """ md5_calls = self._find_md5_calls() for call_node in md5_calls: - keyword_names = [kw.arg for kw in call_node.keywords] + keyword_names = [kw.arg for kw in call_node.keywords if kw.arg is not None] has_usedforsecurity = 'usedforsecurity' in keyword_names if has_usedforsecurity: for kw in call_node.keywords: @@ -93,6 +94,11 @@ def _get_atomic_yaml_files(self): def test_yaml_files_exist(self): """The atomic data directory should contain YAML test definitions.""" + if not os.path.isdir(ATOMICS_DIR): + pytest.skip( + f"Atomics directory not found at {ATOMICS_DIR} — " + f"run 'git submodule update --init' to populate data" + ) yaml_files = self._get_atomic_yaml_files() assert len(yaml_files) > 0, ( f"No YAML files found in {ATOMICS_DIR} — " @@ -103,7 +109,7 @@ def test_yaml_files_are_parseable(self): """All YAML files should parse without error.""" yaml_files = self._get_atomic_yaml_files() for fpath in yaml_files: - with open(fpath, 'r') as f: + with open(fpath, 'r', encoding='utf-8') as f: try: data = yaml.safe_load(f) except yaml.YAMLError as e: @@ -122,7 +128,7 @@ def test_yaml_files_have_required_fields(self): basename = os.path.basename(fpath) if not basename.startswith('T'): continue - with open(fpath, 'r') as f: + with open(fpath, 'r', encoding='utf-8') as f: data = yaml.safe_load(f) assert 'attack_technique' in data, ( f"Missing 'attack_technique' in {fpath}"