-
Notifications
You must be signed in to change notification settings - Fork 1
Description
Problem Statement
The setup_dev.py script uses subprocess.run with a command list (not shell=True), which is safe from shell injection. However, the script modifies sys.path by inserting the repository root at position 0, which could allow a malicious ergodic_insurance package in the parent directory to shadow the legitimate one.
More importantly, the migrate_configs.py script at line 9 does sys.path.insert(0, str(Path(__file__).parent.parent.parent)) which inserts an arbitrary directory into the Python import path. If the project is cloned into a directory alongside malicious packages, this path manipulation could load attacker-controlled code.
v1.0 Impact
Low -- this requires the attacker to have write access to the parent directory of the project checkout, which is unlikely in most deployment scenarios. This is a defense-in-depth improvement.
Affected Code
ergodic_insurance/scripts/migrate_configs.py:L9--sys.path.insert(0, str(Path(__file__).parent.parent.parent))
Current Behavior
The script inserts the project's grandparent directory into sys.path[0], which takes priority over all other import paths. If a malicious ergodic_insurance/ package exists in that directory, it would be imported instead of the legitimate one.
Expected Behavior
Use relative imports or importlib to import from the known package location instead of manipulating sys.path. Alternatively, validate that the path being inserted actually contains the expected package.
Alternative Solutions Evaluated
- Remove sys.path manipulation: Use proper package installation (
pip install -e .) before running migration scripts. Pros: Standard Python practice. Cons: Requires installation step. - Validate the inserted path: Check that the directory contains
ergodic_insurance/__init__.pybefore inserting. Pros: Simple validation. Cons: Still manipulates sys.path.
Recommended Approach
Option 1: Document that migration scripts require the package to be installed. Remove sys.path.insert and rely on proper package installation.
Acceptance Criteria
-
sys.path.insertremoved from migration scripts - Scripts documented as requiring package installation
- All existing tests continue to pass