YAML files don't support inline whitelisting #50
Conversation
|
Does this only fail with YAML files? Or does it fail on other file types too? |
|
It works fine with python files |
|
@LouisTrezzini, please open an issue to report this bug, or complete this pull request to fix it. Pull requests should be used for code wanting to be merged in with the master branch. |
domanchi
changed the title
| @@ -38,17 +38,17 @@ def analyze_string(self, string, line_num, filename): # pragma: no cover | |||
|
|
|||
| NOTE: line_num and filename are used for PotentialSecret creation only. | |||
| """ | |||
| pass | |||
| raise NotImplementedError | |||
There was a problem hiding this comment.
This isn't really needed, because abc.abstractmethod will prevent it from even being initialized.
| filename, | ||
| ), | ||
| ) | ||
| if not item['__line__'] in ignored_lines: |
There was a problem hiding this comment.
Use and instead, to avoid hadouken code?
There was a problem hiding this comment.
if you look more attentively, the continue statement is shared
but I agree it's not very elegant
| data = YamlLineInjector(file).json() | ||
| parser = YamlFileParser(file) | ||
| data = parser.json() | ||
| ignored_lines = parser.get_ignored_lines() |
There was a problem hiding this comment.
Any reason why we want to do this all at one time, as compared to scanning as needed?
e.g.
if '__line__' in item and not WHITELIST_REGEX.search(item['__value__']):
pass
There was a problem hiding this comment.
the current behavior is doing what you suggest:
if '__line__' in item:
potential_secrets.update(
self.analyze_string(
item['__value__'],
item['__line__'],
filename,
),
)
But value is actually the value, not the full line, so the comment is dropped
pyYAML drops comments during preprocessing so we can't use it
I decided to scan the file once to identify all ignored lines and later do a simple O(1) line in ignored_lines check
There was a problem hiding this comment.
Ahh. Right. Good point.
Can you explain this in the docstring for get_ignored_lines? Specifically regarding the fact that the parser drops the comments, and thus, we need to parse the file separately from yaml parsing.
| filename, | ||
| ), | ||
| ) | ||
| if not item['__line__'] in ignored_lines: |
| data = YamlLineInjector(file).json() | ||
| parser = YamlFileParser(file) | ||
| data = parser.json() | ||
| ignored_lines = parser.get_ignored_lines() |
There was a problem hiding this comment.
Ahh. Right. Good point.
Can you explain this in the docstring for get_ignored_lines? Specifically regarding the fact that the parser drops the comments, and thus, we need to parse the file separately from yaml parsing.
|
I pushed the changes you asked for, feel free to merge |
No description provided.