bugfix(cli): parse requirements.txt first and map packages to technique #2030
Conversation
a5571ed to
2e8c244
Compare
|
Can you please run the linter ( |
@malice00 |
lib/cli/index.js
Outdated
| const directDependencies = await parseReqFile(reqData, true); | ||
| if (directDependencies?.length) { | ||
| // Mark direct dependencies from requirements.txt as manifest-analysis | ||
| directDependencies.forEach((pkg) => { |
There was a problem hiding this comment.
Shall we add this evidence creation in the parseReqFile method instead? This way the index.js could remain a bit lightweight.
lib/cli/index.js
Outdated
| // Also mark the evidence | ||
| if (!pkg.evidence) { | ||
| pkg.evidence = { | ||
| identity: { field: "purl", confidence: 1, methods: [] }, |
There was a problem hiding this comment.
Use a lower confidence value such as 0.5 or even 0.3 when there are no version.s
| @@ -3858,6 +3886,35 @@ export async function createPythonBom(path, options) { | |||
| parentComponent, | |||
| ); | |||
| if (pkgMap.pkgList?.length) { | |||
| pkgMap.pkgList.forEach((pkg) => { | |||
There was a problem hiding this comment.
Does this logic overwrite and set a single manifest-analysis technique or add to the existing array?
There was a problem hiding this comment.
If the package has already been found, it will use it's technique. Otherwise it will set it as manifest-analysis
There was a problem hiding this comment.
How can index.js know that those packages were found using manifest-analysis? Can we move this to utils.js as well?
There was a problem hiding this comment.
Using the packageTechniqueMap. I can create a function for this in the utils.js if that is what you mean
lib/helpers/utils.js
Outdated
| pkg.evidence = { | ||
| identity: { | ||
| field: "purl", | ||
| confidence: pkg.version ? 1 : 0.3, |
There was a problem hiding this comment.
Let's use 0.5 instead of 1.
lib/helpers/utils.js
Outdated
| } | ||
| pkg.evidence.identity.methods.push({ | ||
| technique: "manifest-analysis", | ||
| confidence: 1, |
There was a problem hiding this comment.
Same confidence logic as line 5792
Signed-off-by: Omri Yoffe <oyoffe@paloaltonetworks.com>
dbad80c to
93b314c
Compare
|
Some test failures. Please take a look at your convenience. |
Signed-off-by: Omri Yoffe <oyoffe@paloaltonetworks.com>
Signed-off-by: Omri Yoffe <oyoffe@paloaltonetworks.com>
Signed-off-by: Omri Yoffe <oyoffe@paloaltonetworks.com>
93b314c to
eb581c9
Compare
Fixed |
|
Thank you so much. Give us some time since we are running behind with testing, as everything is quite manual. |
|
@prabhu Thank you, do you have an estimate when will you be able to test this? |
| @@ -3847,6 +3848,10 @@ export async function createPythonBom(path, options) { | |||
| const basePath = dirname(f); | |||
| let reqData; | |||
| let frozen = false; | |||
|
|
|||
| reqData = readFileSync(f, { encoding: "utf-8" }); | |||
| await parseReqFile(reqData, true, packageTechniqueMap); | |||
There was a problem hiding this comment.
@omriyoffe-panw This line is dangling and isn't doing anything. What was the idea behind this?
| pkg.evidence.identity.methods = | ||
| pkg.evidence.identity.methods.map((method) => ({ | ||
| ...method, | ||
| technique: "manifest-analysis", |
There was a problem hiding this comment.
This line is also not correct since it overwrites technique instead of adding to the array.
For example, in the below the technique is getting overwritten to manifest-analysis.
{
"group": "",
"name": "starlette",
"version": "0.46.2",
"purl": "pkg:pypi/starlette@0.46.2",
"type": "library",
"bom-ref": "pkg:pypi/starlette@0.46.2",
"properties": [
{
"name": "SrcFile",
"value": "requirements.txt"
}
],
"evidence": {
"identity": [
{
"field": "purl",
"confidence": 1,
"methods": [
{
"technique": "manifest-analysis",
"confidence": 1,
"value": "/Users/prabhu/miniconda3/envs/2069"
}
],
"concludedValue": "/Users/prabhu/miniconda3/envs/2069"
}
]
}
}
|
This PR needs some cleaning up. Will work on it separately. |
A suggested fix for
#1891