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

some fixes for et #2589

Open
wants to merge 4 commits into
base: stable
Choose a base branch
from
Open

some fixes for et #2589

wants to merge 4 commits into from

Conversation

ReimarBauer
Copy link
Member

Purpose of PR?:

Fixes #542

some more to replace

@ReimarBauer ReimarBauer requested a review from joernu76 December 13, 2024 14:55
@ReimarBauer ReimarBauer requested a review from matrss December 16, 2024 06:40
@@ -39,8 +39,9 @@
import os

import fs
import xml.dom.minidom
import xml.parsers.expat
import xml.dom.minidom # nosec
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like that the nosec marker is on the import, instead of on the actual usage that was deemed unproblematic. This would disable warnings for new places using the module, if any were to be added in the future. Is it possible to instead mark every line using xml.dom.minidom as nosec instead, with an explanation why they are safe?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

explanation moved. At the moment we add a sec scanner we maybe have to configure that differently.

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