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

Add python runner module S28 #1320

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Add python runner module S28 #1320

wants to merge 12 commits into from

Conversation

B1TC0R3
Copy link

@B1TC0R3 B1TC0R3 commented Sep 25, 2024

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    Feature.

  • What is the current behavior? (You can also link to an open issue here)
    EMBA cannot run Python scripts natively as modules.

  • What is the new behavior (if this is a feature change)? If possible add a screenshot.
    This PR adds a new module ("S28_python_run") with the capability of running user-supplied python scripts as
    modules. Related to issue Python runner module #1264.

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
    No. The module S28 is disabled in the default scan profile (default-scan.emba).
    In addition, when no Python scripts are manually specified, it will start and terminate without doing anything.

  • Other information:
    This is a continuation of pull request Add module S28_python_run #1277.
    As mentioned in my last commend there, I broke the Git history of my fork so badly that I had
    to delete the it...
    This new pull request also implements some changes I made to the logging mechanism.

@m-1-k-3
Copy link
Member

m-1-k-3 commented Dec 18, 2024

@B1TC0R3 are there any plans to finalize this PR?

@B1TC0R3
Copy link
Author

B1TC0R3 commented Dec 18, 2024

Hello and sorry for the long pause.
I have not forgotten about this yet, but had to prioritize other projects over this one after my paper was done.
Depending on how you want to handle this, we can close this PR and I will create a new one once implementation is finished or alternatively this one simply stays open as a draft. I currently don't want to make any predictions for how long I will take, as I was horribly wrong the last time already. -_-

@m-1-k-3
Copy link
Member

m-1-k-3 commented Dec 18, 2024

We can leave it open as draft or you can close it and improve it behind the scenes. Up to you.

@B1TC0R3
Copy link
Author

B1TC0R3 commented Dec 18, 2024

I will just leave it open then.

@B1TC0R3 B1TC0R3 reopened this Jan 29, 2025
@B1TC0R3
Copy link
Author

B1TC0R3 commented Jan 29, 2025

My remaining todo before this is pull request is ready:

  • [Done] Enforce proper linting and check for errors in S28 related code through check_project.sh.
  • [Done] Properly test the new code. It works fine in my dev environment, but that does not necessarily mean too much.
    • default-scan-python-runner.emba runs fine in strict mode on the current version of Kali Linux
    • default-scan-python-runner.emba runs fine in strict mode on Ubuntu 24.04
  • [Done] Update the documentation. Some things changed, will take care of this after everything else is done.

@B1TC0R3
Copy link
Author

B1TC0R3 commented Feb 7, 2025

@m-1-k-3 Here's the updated documentation for the new module. Let me know if I missed anything. ^_^

emba_python_modules.md (Outdated, updated version here)

@B1TC0R3 B1TC0R3 marked this pull request as ready for review February 7, 2025 23:07
@B1TC0R3 B1TC0R3 mentioned this pull request Feb 7, 2025
@m-1-k-3
Copy link
Member

m-1-k-3 commented Feb 8, 2025

The Codacy workflow uses Prospector and Pylint and has found multiple style issues. Could you take a look please.

@B1TC0R3
Copy link
Author

B1TC0R3 commented Feb 9, 2025

Since the pipeline uses prospector, I've also gone ahead and added that to check_project.sh in addition to flake8.

@m-1-k-3
Copy link
Member

m-1-k-3 commented Feb 9, 2025

I will do some further review and testing after #1452 ... in the meantime thank you for your work. Looking forward to dig into it

@B1TC0R3
Copy link
Author

B1TC0R3 commented Feb 9, 2025

Ah right, I almost forgot, there have been some minor updates to the documentation due to today's changes.
Here's the new version:
emba_python_modules.md

@B1TC0R3
Copy link
Author

B1TC0R3 commented Feb 10, 2025

I have reviewed the warnings identified by the pipeline and it can be broken down into two points:

  • The pipeline does not like the pylint disable statements. These exist on purpose to mute certain findings bound to intentional design choices. Each and every disable statement has an explanation attached to it. The disable statements are correctly parsed by check_project.sh as well as manual prospector runs.
  • The Format class is lacking public methods and certain attributes. This happens, because these attributes are dynamically generated from the EMBA environment variables during execution. It is done this way to remove the need to manually update the embaformatting.py script every time a color/format statement \033[...m is changed or added.

Since these warnings might be thrown on every following pipeline run, I do intent to fix them but am unsure of how to go about it. What is your opinion on this problem?

@m-1-k-3
Copy link
Member

m-1-k-3 commented Mar 11, 2025

Sorry for the delay ... as we were quite busy we needed to postpone this PR to the next release.

@@ -0,0 +1,158 @@
#!/usr/bin/python3
# pylint: disable=consider-using-with, no-member

Check warning

Code scanning / Pylint (reported by Codacy)

Bad option value 'consider-using-with' Warning

Bad option value 'consider-using-with'
@@ -0,0 +1,66 @@
#!/usr/bin/python3
# pylint: disable=broad-exception-caught

Check warning

Code scanning / Pylint (reported by Codacy)

Bad option value 'broad-exception-caught' Warning

Bad option value 'broad-exception-caught'
@@ -0,0 +1,158 @@
#!/usr/bin/python3
# pylint: disable=consider-using-with, no-member

Check warning

Code scanning / Pylintpython3 (reported by Codacy)

Bad option value 'consider-using-with' Warning

Bad option value 'consider-using-with'
@@ -0,0 +1,66 @@
#!/usr/bin/python3
# pylint: disable=broad-exception-caught

Check warning

Code scanning / Pylintpython3 (reported by Codacy)

Bad option value 'broad-exception-caught' Warning

Bad option value 'broad-exception-caught'
@@ -0,0 +1,158 @@
#!/usr/bin/python3
# pylint: disable=consider-using-with, no-member

Check warning

Code scanning / Prospector (reported by Codacy)

Bad option value 'consider-using-with' (bad-option-value) Warning

Bad option value 'consider-using-with' (bad-option-value)
@@ -0,0 +1,66 @@
#!/usr/bin/python3
# pylint: disable=broad-exception-caught

Check warning

Code scanning / Prospector (reported by Codacy)

Bad option value 'broad-exception-caught' (bad-option-value) Warning

Bad option value 'broad-exception-caught' (bad-option-value)
@B1TC0R3
Copy link
Author

B1TC0R3 commented Mar 18, 2025

Sorry for the delay ... as we were quite busy we needed to postpone this PR to the next release.

This is not an issue at all.

I am, however, still confused on why pylint is throwing these warnings, specially since bad-option-value is deprecated
in the current version of pylint (See documentation). Unfortunately I am still not able to reproduce these findings locally with pylint, which makes fixing them a bit difficult.

I am working on rewriting the Python code in a way where the disabled warnings would not be triggered in the first place,
but I am unsure of how to handle broad-exception-caught, as the whole point of that try-catch is to handle any error thrown by the module. This is required so the error is logged properly. I'm sure there is a solution though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants