-
Notifications
You must be signed in to change notification settings - Fork 23
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
Option to not include description in findingsDetails #49
Comments
Hey Chris, do you mean the generated JSON is invalid? Can you describe your
use case in more detail where length is an issue?
…On Fri, 9 Aug 2024, 11:37 pm ChrisPage-AT, ***@***.***> wrote:
Would it be possible to include an input field to disable including the
descriptions of the CVEs in the findingsDetails output? Sometimes the
descriptions of CVEs can be excessively long or contain special characters
that break automations. The description can still be found by following the
uri link, so having this as an option would be really nice.
—
Reply to this email directly, view it on GitHub
<#49>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAC4U5LS3KAS2BZWXLSOVNLZQTOZ5AVCNFSM6AAAAABMIWHCO2VHI2DSMVQWIX3LMV43ASLTON2WKOZSGQ2TQMJYHAZTMOA>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Of course! The JSON itself is fine, it just ends up not being usable in later steps when certain CVEs are present. I have some bash steps after I run ecr-scan-image that parse through the findingsDetails and generate a .sarif to upload to the Code Scanning section of the Security tab on GitHub for the repo. The length issue is when you get enough super long descriptions like this. That isn't the longest one I've seen and it doesn't take very many of those to push the findingDetails JSON's total length beyond the bash argument character limit. I can't find an example of a bad character CVE because we've since resolved that one, but I believe the description contained an extended ascii character that bash apparently doesn't support. In both situations it ends up causing the job to fail. I was initially thinking about asking if there could be a character limit on what findingDetails returns, but it seemed easier and cleaner just to have the option to not return the description fields in the JSON. If there's anything else you need or if you're interested in pulling the sarif logic into this action, please let me know! |
I see your point.
Github have a limit of 1mb per output:
https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions#outputs-for-docker-container-and-javascript-actions
I think it's reasonable to add an option to disable description fields. And
it's reasonable to make this the default for new users. Unfortunately that
requires a major version bump, so let's make the change in two parts. First
add the option to disable description fields. Release this as a minor
version bump. Then, change behaviour/option so that description field is
disabled by default and release as a major version bump.
Does this sound sensible to you?
…On Mon, 12 Aug 2024 at 21:46, ChrisPage-AT ***@***.***> wrote:
Of course! The JSON itself is fine, it just ends up not being usable in
later steps when certain CVEs are present. I have some bash steps after I
run ecr-scan-image that parse through the findingsDetails and generate a
.sarif to upload to the Code Scanning section of the Security tab on GitHub
for the repo.
The length issue is when you get enough super long descriptions like this
<https://security-tracker.debian.org/tracker/CVE-2024-26907>. That isn't
the longest one I've seen and it doesn't take very many of those to push
the findingDetails JSON's total length beyond the bash argument character
limit. I can't find an example of a bad character CVE because we've since
resolved that one, but I believe the description contained an extended
ascii character that bash apparently doesn't support.
In both situations it ends up causing the job to fail. I was initially
thinking about asking if there could be a character limit on what
findingDetails returns, but it seemed easier and cleaner just to have the
option to not return the description fields in the JSON. If there's
anything else you need or if you're interested in pulling the sarif logic
into this action, please let me know!
—
Reply to this email directly, view it on GitHub
<#49 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAC4U5JLX2EKW4TK3STLE3TZRC4DBAVCNFSM6AAAAABMIWHCO2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEOBUGAZTQOJYGU>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Awesome! And yeah that totally makes sense to me. Definitely don't want to change default behaviors on folks without proper warning. |
Would it be possible to include an input field to disable including the descriptions of the CVEs in the findingsDetails output? Sometimes the descriptions of CVEs can be excessively long or contain special characters that break automations. The description can still be found by following the uri link, so having this as an option would be really nice.
The text was updated successfully, but these errors were encountered: