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.

Sign up for GitHub

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

Jump to bottom

Feat/add snyk code test support #1640

Closed
wants to merge 9 commits into from
Closed

Feat/add snyk code test support #1640

wants to merge 9 commits into from

Conversation

ArturSnyk
Copy link
Contributor

@ArturSnyk ArturSnyk commented Feb 16, 2021 •
edited
Loading

  • Ready for review
  • Follows CONTRIBUTING rules
  • Reviewed by Snyk internal team

What does this PR do?

This introduces an mpv usage for snyk code.

Where should the reviewer start?

you should have the snykcode cli's ff,
and run it with snyk code test or snyk code test <project_path>

How should this be manually tested?

snyk code test or snyk code test <project_path>

Any background context you want to provide?

we will be adding more functionality around this flow, more error handling, analytics, and output functionality, later on

What are the relevant tickets?

https://snyksec.atlassian.net/browse/COD-123

Screenshots

image

Additional questions

@ArturSnyk ArturSnyk self-assigned this Feb 16, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Feb 16, 2021 •
edited
Loading

Warnings
⚠️

Looks like you added a new Tap test. Consider making it a Jest test instead. See files like test/*.spec.ts for examples. Files found:

  • test/fixtures/code/sample-analyze-folders-response.json
  • test/fixtures/code/sample-sarif.json

Generated by 🚫 dangerJS against 2fa4e6b

@ArturSnyk ArturSnyk force-pushed the feat/add-snyk-code-test-support branch from a6ffbaa to fd522b3 Compare February 16, 2021 08:37
dkontorovskyy
dkontorovskyy reviewed Feb 16, 2021
View reviewed changes
Copy link
Contributor

@dkontorovskyy dkontorovskyy left a comment

Choose a reason for hiding this comment

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

@ArturSnyk ArturSnyk force-pushed the feat/add-snyk-code-test-support branch 4 times, most recently from bb2d63f to 9b0df5c Compare February 16, 2021 12:18
@ArturSnyk
feat: add snyk code support for test flow
09a61c2 
* code-client support
* configs to use the code-client proxy
@ArturSnyk
feat: add analyze folder functionality
2deb886 
* we call snyk codes to analize our project and expecting
to get a response that includes sarif object.
@ArturSnyk
feat: format code output
7261642 
* creating new formating schema for snyk code scanning
@ArturSnyk ArturSnyk force-pushed the feat/add-snyk-code-test-support branch 2 times, most recently from aba5d88 to 82ce5c8 Compare February 17, 2021 16:58
@ArturSnyk
Copy link
Contributor Author

The test Is failing because of a security issue we have in the library that was added to this PR code-client.

It is ok to review this or. we won't merge before this issue is fixed :) and we are working on it

@ArturSnyk ArturSnyk marked this pull request as ready for review February 17, 2021 17:17
@ArturSnyk ArturSnyk requested review from a team as code owners February 17, 2021 17:17
@ArturSnyk
feat: add snyk code functionality to test flow
f091346 
* adjusting the flow so we won't have redundant data
manipulations in case we are in test code flow
@ArturSnyk
fix: add security rule filtering on code scaning
5020a9d 
* we want to filter our sarif response by security only
@ArturSnyk
fix: add missing code project type
f3ab9bf
@ArturSnyk
chore: error handling for critical severity for snyk code
77a1533
@ArturSnyk
test: tests for snyk code
c2de354
@ArturSnyk
feat: upgrade code-client
2fa4e6b 
* the upgrade removed the usage of lodash within code-client.
* updating analyzeFolders's arguments
* adjust tests
@ArturSnyk ArturSnyk force-pushed the feat/add-snyk-code-test-support branch from 82ce5c8 to 2fa4e6b Compare February 17, 2021 17:56
@github-actions
Copy link
Contributor

Expected release notes (by @ArturSnyk)

features:
upgrade code-client (2fa4e6b)
add snyk code functionality to test flow (f091346)
format code output (7261642)
add analyze folder functionality (2deb886)
add snyk code support for test flow (09a61c2)

fixes:
add missing code project type (f3ab9bf)
add security rule filtering on code scaning (5020a9d)

others (will not be included in Semantic-Release notes):
tests for snyk code (c2de354)
error handling for critical severity for snyk code (77a1533)

  • I hereby acknowledge these release notes are 🥙 AWESOME 🥙

robcresswell
robcresswell suggested changes Feb 17, 2021
View reviewed changes
Copy link
Contributor

@robcresswell robcresswell left a comment

Choose a reason for hiding this comment

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

Just putting a blocker for ecosys arch to review

gitphill
gitphill reviewed Feb 19, 2021
View reviewed changes
config.default.json
@@ -2,5 +2,6 @@
"API": "https://snyk.io/api/v1",
"devDeps": false,
"PRUNE_DEPS_THRESHOLD": 40000,
"MAX_PATH_COUNT": 1500000
"MAX_PATH_COUNT": 1500000,
"CODE_CLIENT_PROXY_URL": "http://deeproxy.dev.snyk.io"
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't reference dev in this file? (I see API is pointing at production not dev)

JackuB
JackuB suggested changes Feb 19, 2021
View reviewed changes
Copy link
Contributor

@JackuB JackuB left a comment

Choose a reason for hiding this comment

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

There is still a vulnerability in coming from the fast-glob. We can't merge it if it's there

Screen Shot 2021-02-19 at 16 07 51

@ArturSnyk ArturSnyk closed this Mar 2, 2021
@ArturSnyk
Copy link
Contributor Author

closed in favor of #1664

@ArturSnyk ArturSnyk deleted the feat/add-snyk-code-test-support branch March 2, 2021 14:18
This was referenced Apr 26, 2021
Open
Open
Open
Open
Open
@nathan-snyk nathan-snyk mentioned this pull request Jun 21, 2023
Open
@MaxMood96 MaxMood96 mentioned this pull request Jun 21, 2023
Open
@aliscco aliscco mentioned this pull request Jun 22, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dkontorovskyy dkontorovskyy dkontorovskyy left review comments

@gitphill gitphill gitphill left review comments

@robcresswell robcresswell robcresswell requested changes

@JackuB JackuB JackuB requested changes

Assignees

@ArturSnyk ArturSnyk

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ArturSnyk @JackuB @robcresswell @gitphill @dkontorovskyy