Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor the CLI dynamic command resolution #362

Merged
merged 6 commits into from
Mar 6, 2024
Merged

Refactor the CLI dynamic command resolution #362

merged 6 commits into from
Mar 6, 2024

Conversation

tasansal
Copy link
Collaborator

@tasansal tasansal commented Mar 6, 2024

The dynamic module resolution was making some CLI failures on CI/CD (not sure why).

However, this PR refactors the CLI plugin resolution with a much safer approach.

Changes

  • Replaced eval with importlib for secure dynamic command module loading, better mitigating arbitrary code execution risks.
  • Defined a plugin folder and a specified list of known modules for tighter execution control, enhancing CLI security.
  • Enhanced error handling for clearer feedback on command module loading issues.
  • Shifted to pathlib for more robust path handling, replacing os.path and string manipulations.
  • Adopted importlib.metadata for fetching package versions, offering a fallback for unavailable versions.

@tasansal tasansal added the ci Continuous Integration label Mar 6, 2024
@tasansal tasansal self-assigned this Mar 6, 2024
This class now dynamically loads command logic from the given plugin_folder, replacing the previous implementation. The new approach uses importlib to load command modules, which should improve security and maintainability. The "commands" command has also been updated to use the new implementation.
The __main__.py file now better handles command execution by listing known safe-to-execute modules instead of ignoring certain modules. Additionally, the retrieval of the package version has been refactored to use a function. This provides more robust error handling by returning a default value if the package version can't be retrieved for any reason. Cleanup changes have also been made to __init__.py.
@tasansal tasansal changed the title Fix mysterious CI/CD failure Refactor the CLI dynamic command resolution Mar 6, 2024
@tasansal tasansal added refactoring Refactoring and removed ci Continuous Integration labels Mar 6, 2024
@TGSAI TGSAI deleted a comment from codecov-commenter Mar 6, 2024
@tasansal tasansal merged commit c3ba558 into main Mar 6, 2024
20 checks passed
@tasansal tasansal deleted the fix-cicd branch March 6, 2024 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant