Skip to content

Conversation

mastersans
Copy link
Member

  • added commands for vex input and output
  • generation support shifted to use vex_manager
  • the test would fail due to mismatch of pypi and github version of lib4sbom
  • errors are quick to fix, but I thought to draft a PR

cc @anthonyharrison @terriko

@mastersans mastersans marked this pull request as draft June 28, 2024 20:28
@mastersans mastersans marked this pull request as ready for review June 29, 2024 04:59
@mastersans
Copy link
Member Author

also I'll include spdx vex support in later commits once the project is more towards the completion.

Copy link
Contributor

@anthonyharrison anthonyharrison left a comment

Choose a reason for hiding this comment

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

The documentation (REDME and MANUAL) needs updating to include the new command line options.

input_group.add_argument(
"--vex",
action="store",
choices=["cyclonedx", "csaf", "open"],
Copy link
Contributor

Choose a reason for hiding this comment

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

The choices should be cyclonedx, csaf, openvex and auto.

However, do we need this if we assume auto?

Copy link
Member Author

Choose a reason for hiding this comment

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

@anthonyharrison I think its better this way so that lib4vex don't have to figure out on auto and auto being handy if no value for --vex is passed, should i remove it so, for parser to always work on auto?

Copy link
Contributor

Choose a reason for hiding this comment

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

If auto is accurate enough (and I'm fairly sure it is?) we should probably use auto and get rid of the option. It'll take a milisecond for the computer to figure it out and a lot longer than that for the human if they don't already know the answer. ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

@terriko i have shifted the manual defining of vex to auto ; )

# vulnerability.set_remediation(cve.response)
# set_remediation is not available for lib4sbom pypi version for now
# if cve.response:
# vulnerability.set_remediation(cve.response[0])
Copy link
Contributor

Choose a reason for hiding this comment

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

The vulnerability class has a set_value function which allows the setting of any attribute.

e.g. vulnerability.set_value("remediation","Some interesting remediation description")

Copy link
Member Author

Choose a reason for hiding this comment

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

@anthonyharrison i thought the same of using set_value as the Vulnerability class do not include set_remediation in pypi version but even if set it the cyclonedx generator in lib4sbom latest pypi version do not take remediation into account for now so i thought to use it in comment, although i am aware that csaf takes it into account, I'll shift it to set_value()

@mastersans
Copy link
Member Author

Hey @anthonyharrison I have made the requested changes can you checkout them, Thanks

Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

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

Re-running the windows tests just in case.

If you're planning to bring back those tests once the lib4sbom version mismatch is fixed, could you disable them rather than deleting here? And if you aren't planning to bring those particular ones back, can we get a new test to replace them here?

@mastersans
Copy link
Member Author

@terriko im not sure which one you are talking about, but the removal in the test_vex of test_triage file the 2 lines removed is due to the code logic in lib4vex generation(that one actually works with GitHub version of lib4sbom so in future we just need to enable it) and if it's about the generate_vex test, that test is already shifted into test_vex file so older one is not needed also I didn't include the test regarding the schema of cyclonedx vex because there will be a minor fix that I open PR for in lib4sbom so that test would fail too, I hope I didn't missed something ; )

@terriko
Copy link
Contributor

terriko commented Jul 8, 2024

Pinging @mastersans : you'll need to do some conflict resolution on this before it can be merged.

@terriko terriko added the awaiting submitter Need more information from submitter label Jul 8, 2024
@mastersans
Copy link
Member Author

mastersans commented Jul 8, 2024

Pinging @mastersans : you'll need to do some conflict resolution on this before it can be merged.

In flash speed......

edit: And here goes the bad cache dayyy ; (

@terriko
Copy link
Contributor

terriko commented Jul 8, 2024

Yeah, this is blocked until those tests clear up. If you or @inosmeet can make a PR that just temporarily disables those 24 tests, that would probably be a good idea so we can keep the gsoc things moving.

@terriko
Copy link
Contributor

terriko commented Jul 8, 2024

(this is what I get for turning on branch protection so certain tests have to pass or the merge button disappears...)

@terriko terriko removed the awaiting submitter Need more information from submitter label Jul 16, 2024
@terriko
Copy link
Contributor

terriko commented Jul 16, 2024

yay for tests behaving again. Going to merge this now.

@terriko terriko merged commit 361f5fd into intel:main Jul 16, 2024
@mastersans mastersans deleted the command-line branch July 17, 2024 11:02
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