Skip to content
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

Added identity and occurrences to evidence. Updated test cases. #199

Merged
merged 3 commits into from
Apr 24, 2023

Conversation

stevespringett
Copy link
Member

Closes #129

@stevespringett stevespringett added this to the 1.5 milestone Mar 25, 2023
@stevespringett stevespringett self-assigned this Mar 25, 2023
@stevespringett stevespringett linked an issue Mar 25, 2023 that may be closed by this pull request
@stevespringett stevespringett added the RFC notice sent A public RFC notice was distributed to the CycloneDX mailing list for consideration label Mar 25, 2023
@@ -19,6 +19,40 @@
],
"purl": "pkg:maven/com.google.code.findbugs/[email protected]",
"evidence": {
"identity": {
"field": "purl",
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sold on the idea that this "field" needs to even be there. The original idea was to be able to separate out individual evidence of group/name/version into individual pieces of evidence. But for some things, it actually doesn't make sense (like purl).

Copy link
Member Author

Choose a reason for hiding this comment

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

Same would apply fo "field" in the XML and protobuf.

"uniqueItems": true,
"additionalItems": false,
"items": {
"$ref": "#/definitions/refType"
Copy link
Member

Choose a reason for hiding this comment

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

👍 this is in line with #198

@jkowalleck
Copy link
Member

jkowalleck commented Mar 29, 2023

according to

"confidence": {
"type": "number",
"minimum": 0,
"maximum": 1,
"title": "Confidence",
"description": "The overall confidence of the evidence from 0 - 1, where 1 is 100% confidence."
},

and
"confidence": {
"type": "number",
"minimum": 0,
"maximum": 1,
"title": "Confidence",
"description": "The confidence of the evidence from 0 - 1, where 1 is 100% confidence. Confidence is specific to the technique used. Each technique of analysis can have independent confidence."
},

We have an overall confidence, and we have multiple specific confidence values.
One could argue that an overall confidence might be derived of (e.g. meridian over all) all specific confidences. This would be true if all specifics were weighted equally.
The mere existence of an overall confidence value indicates that this is not the case.

Therefore, i would argue, that the weighing of each specific confidence value should be published.
And if this was published, then an overall value would no longer make sense, since it could be calculated by each tool that digested this document.

❌ Therefore, Ii do not agree to the existence of such property.


PS: here are examples:

weights unknown

specific confidence A: 0.5 
specific confidence B: 0.75
specific confidence C: 1.0
overall confidence: 0.8  << how? what weighting was used to justify this value? has no value to me, as details are missing

weights are equal - per definition

specific confidence A: 0.5
specific confidence B: 0.75
specific confidence C: 1.0
overall confidence: 0.75  <<  has no value to me, as i could calculate it myself: (0.5 + 0.75 + 1.0) / 3

individual weights published

specific confidence A: 0.5 (at weight 0.1)
specific confidence B: 0.75 (at weight 0.2)
specific confidence C: 1.0 (at weight 1.0)
overall confidence: 0.4  <<  has no value to me, as i could calculate it myself: ((0.5* 0.1) + (0.75 * 0.2) + (1.0 * 1.0)) / 3

@stevespringett
Copy link
Member Author

@jkowalleck Thanks for your input. Based on today's IWG meeting, it's my understanding that weight is no longer being considered, as its up to the consumer to place weight on what they believe are the most important techniques for analysis. Is that correct? Is weight being withdrawn? If so, then perhaps some documentation improvements to describe consumer weighting?

@stevespringett
Copy link
Member Author

Per #129 (comment) callstack is being added in. Will update when complete.

@stevespringett
Copy link
Member Author

Ok, callstack support has been added back into this PR and test cases

@stevespringett stevespringett merged commit 6771127 into v1.5-dev Apr 24, 2023
@stevespringett stevespringett deleted the v1.5-dev-evidence branch April 24, 2023 02:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposed core enhancement request for comment RFC notice sent A public RFC notice was distributed to the CycloneDX mailing list for consideration RFC vote accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add evidence used to determine inclusion of a component.
2 participants