-
Notifications
You must be signed in to change notification settings - Fork 39
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
ZAP import accepts types other than "url" #269
ZAP import accepts types other than "url" #269
Conversation
config: ``` importUrlsFromFile: type: one of 'har', 'modsec2', 'url' (default), 'zap_messages' fileName: path to file ```
@@ -118,8 +118,10 @@ scanners: | |||
apiUrl: "<URL to openAPI>" | |||
# alternative to apiURL: apiFile: "<local path to openAPI file>" | |||
|
|||
# A list of URLs can also be provided, from a text file (1 URL per line) | |||
importUrlsFromFile: "<path to import URL>" | |||
# A list of URLs can also be provided, type supported: 'har', 'modsec2', 'url' (default), 'zap_messages' |
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.
Does this require a config schema version 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.
not if we keep backward compatibility: we can simply suggest only the new method, while keeping the old one valid for historical purpose.
[quick additional note: backward compatibility was removed based on next conversation]
scanners/zap/zap.py
Outdated
Prepare a URL import job. All ZAP's import job are supported: 'har', 'modsec2', 'url' (default), 'zap_messages' | ||
|
||
2 possibilities: | ||
1- [for backward compatibility] if importUrlsFromFile is a string: |
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.
Would it be better to migrate between config versions, rather than try to handle two different schemas? I believe there is migration code already but I haven't looked closely
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.
I added a new commit for this purpose
Updated to config schema version 6 (including adding converter from v5 to v6 & test) and removed backward compatibility |
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.
LGTM
config:
default remains "url", and previous config style (
importUrlsFromFile
is a string) remains supported