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

Fix generating dependency trees info for dockerized NodeJS projects #927

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

grgau
Copy link

@grgau grgau commented Mar 19, 2024

Solve issue [NodeJs][Docker] Error getting Dependency Tree in NodeJS Docker images by removing premature return when generating SBOM from dockerized NodeJS images. Because of this return the lock files are not being read and the dependency trees are staying empty.

@prabhu
Copy link
Collaborator

prabhu commented Mar 19, 2024

Thanks @grgau for the fix. Could you kindly sign the commit by following the below instructions?

https://github.com/CycloneDX/cdxgen/pull/927/checks?check_run_id=22859077739

@grgau grgau force-pushed the fix/nodejs-dependency-tree-docker branch 2 times, most recently from b0d578a to 3ee6427 Compare March 19, 2024 23:08
…d app

Signed-off-by: grgau <pedro.ferracini@alumni.usp.br>
@grgau grgau force-pushed the fix/nodejs-dependency-tree-docker branch from 3ee6427 to 6d0e498 Compare March 19, 2024 23:10
@@ -1850,12 +1850,6 @@ export async function createNodejsBom(path, options) {
pkgList = pkgList.concat(dlist);
}
}
return buildBomNSData(options, pkgList, "npm", {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should have included a comment here. I think the early return is required to only collect information from package.json alone. This is because for containers we are interested in the post-build lifecycle whereas the lock file could represent the pre-build or build lifecycle and, therefore, might include dev dependencies that may not eventually get included in the image.

As far as the dependency tree goes, I think we can come up with a new function that constructs a tree based on the lock file but only for the components from the package.json files. Such a tree might be useful but not accurate since the dependency tree after tree-shaking could look completely different to build.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry for the delay, I'll take a look. Is this tree construction currently done in the same function? in the process of calling getAllFiles and then processing the dependencies variable?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@grgau, best to do some research to see how others are implementing this. For example, npm cli has an sbom command nowadays. Not sure if it can recover the dependency tree from node_modules directory alone using arborist.

@prabhu
Copy link
Collaborator

prabhu commented Jun 11, 2024

@grgau any thoughts on how this is handled by other tools?

@grgau
Copy link
Author

grgau commented Jun 24, 2024

I'm sorry @prabhu , but I'm without time to see this issue until the end of the month at least :(

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.

2 participants