-
-
Notifications
You must be signed in to change notification settings - Fork 602
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
New check: B113: TrojanSource - Bidirectional control characters #757
Conversation
Could you rebase please? |
e59ae35
to
75d527b
Compare
@sigmavirus24: Sure, done |
I added a commit to please the |
a676318
to
ffe205a
Compare
I just fixed the typo I made in the last commit |
I added the missing "SPDX-License-Identifier: Apache-2.0" comment to please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But I'd like another person to take a gander
bandit/plugins/trojansource.py
Outdated
'\u2066', '\u2067', '\u2068', '\u2069') | ||
|
||
|
||
@test.test_id('B113') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR #743 is also using B113. First to merge would get it, other would need to change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I'm fine with that 😄
Just checking: this is pending approval from @lukehinds and/or @ghugo ? |
Or @ericwb |
Ping @lukehinds / @ghugo / @sigmavirus24 / @ericwb: |
FYI it has been added to Pylint already: pylint-dev/pylint#5311 |
78d5a23
to
cde6449
Compare
fd80a31
to
229aa95
Compare
I have made some fixups to please the At least it does on my environment, with Python 3.7:
Could you please approve the pipeline execution @ericwb? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you rebase on main and resolve conflicts? Thanks
Also, since this seems more closely related to a type of injection, I
think bandit ID of 613 would be more appropriate.
I see you've added commits, is a rebase still needed @ericwb ? |
The bandit-baseline fails in the pep8 build job. I suspect this is normal since a new plugin was added, so the baseline comparison would be different. I'll merge and see if it resolves, otherwise I might have to revert. |
Close #749