-
Notifications
You must be signed in to change notification settings - Fork 687
feat: [CMPA-603] Add npm support for SBOM component metadata being FF #6950
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ export interface Options { | |
| scanAllUnmanaged?: boolean; | ||
| showNpmScope?: boolean; | ||
| allProjects?: boolean; | ||
| includeComponentMetadata?: boolean; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can this hold values such as null and undefined? Just a sanity question |
||
| } | ||
|
|
||
| export interface Plugin { | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| { | ||
| "name": "npm-component-metadata-v1", | ||
| "version": "1.0.0", | ||
| "dependencies": { | ||
| "lodash": "4.17.15" | ||
| } | ||
| } |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| { | ||
| "name": "npm-component-metadata-v2", | ||
| "version": "1.0.0", | ||
| "dependencies": { | ||
| "lodash": "4.17.15" | ||
| } | ||
| } |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| { | ||
| "name": "npm-component-metadata-v3", | ||
| "version": "1.0.0", | ||
| "dependencies": { | ||
| "lodash": "4.17.15" | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| import { createProjectFromFixture } from '../../util/createProject'; | ||
| import { runSnykCLI } from '../../util/runSnykCLI'; | ||
|
|
||
| jest.setTimeout(1000 * 60); | ||
|
|
||
| // `--include-component-metadata` makes the npm plugin forward the flag to | ||
| // snyk-nodejs-lockfile-parser, which reads the install-time `integrity` and | ||
| // `resolved` fields already recorded in the lockfile and surfaces them as | ||
| // `hash:<algorithm>` and `distribution:url` labels on the dep-graph nodes. | ||
| // Unlike maven there is nothing to resolve first — the metadata lives in the | ||
| // lockfile — so these fixtures need no `npm install`. | ||
| // | ||
| // Covers npm lockfile v1 (legacy depTree path, converted to a dep-graph for | ||
| // printing) and v2/v3 (native dep-graph path). | ||
| describe('`snyk test --include-component-metadata` (npm)', () => { | ||
| const labelKeys = (printGraphStdout: string, prefix: string): string[] => { | ||
| const jsonDG = JSON.parse( | ||
| printGraphStdout.split('DepGraph data:')[1].split('DepGraph target:')[0], | ||
| ); | ||
| return jsonDG.graph.nodes | ||
| .flatMap((node) => Object.keys(node.info?.labels ?? {})) | ||
| .filter((key) => key.startsWith(prefix)); | ||
| }; | ||
|
|
||
| const fixtures = [ | ||
| ['v1', 'npm-include-component-metadata/lock-v1'], | ||
| ['v2', 'npm-include-component-metadata/lock-v2'], | ||
| ['v3', 'npm-include-component-metadata/lock-v3'], | ||
| ]; | ||
|
|
||
| describe.each(fixtures)('lockfile %s', (_version, fixture) => { | ||
| it('attaches hash and distribution:url labels with the flag', async () => { | ||
| const project = await createProjectFromFixture(fixture); | ||
|
|
||
| const { code, stdout } = await runSnykCLI( | ||
| 'test --include-component-metadata --print-graph --file=package-lock.json', | ||
| { cwd: project.path() }, | ||
| ); | ||
|
|
||
| expect(code).toEqual(0); | ||
| expect(stdout).toContain('DepGraph data:'); | ||
| expect(labelKeys(stdout, 'hash:').length).toBeGreaterThan(0); | ||
| expect(labelKeys(stdout, 'distribution:url').length).toBeGreaterThan(0); | ||
| }); | ||
|
|
||
| // Control: without the flag the same project must not produce the labels, | ||
| // proving they are driven by `--include-component-metadata`. | ||
| it('does not attach the labels without the flag', async () => { | ||
| const project = await createProjectFromFixture(fixture); | ||
|
|
||
| const { code, stdout } = await runSnykCLI( | ||
| 'test --print-graph --file=package-lock.json', | ||
| { cwd: project.path() }, | ||
| ); | ||
|
Comment on lines
+48
to
+54
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since this test is in a loop, I'm wondering if we should add some timeout here or if default is enough |
||
|
|
||
| expect(code).toEqual(0); | ||
| expect(stdout).toContain('DepGraph data:'); | ||
| expect(labelKeys(stdout, 'hash:')).toHaveLength(0); | ||
| expect(labelKeys(stdout, 'distribution:url')).toHaveLength(0); | ||
|
Comment on lines
+56
to
+59
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is there any chance that changes in the responses from runSnykCLI would cause these values to fail the test?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (maybe my comment is hard to understand the way I wrote - just wondering if this could be flaky) |
||
| }); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Optional] instead of setting false as default in both of these includeComponentMetadata, I would suggest saving to a constant such as defaultIncludeComponentMetadata