Skip to content

Feat/fixing downstream kiro vulnerability scans#19

Open
AntonBarashSnyk wants to merge 4 commits intomainfrom
feat/updating-kiro-vulnerability-scan
Open

Feat/fixing downstream kiro vulnerability scans#19
AntonBarashSnyk wants to merge 4 commits intomainfrom
feat/updating-kiro-vulnerability-scan

Conversation

@AntonBarashSnyk
Copy link
Copy Markdown
Contributor

JIRA ticket:
https://snyksec.atlassian.net/browse/AG-132

This PR fixes an issue in the Kiro SCA scanner which didn't flag packages as vulnerable if they had downstream packages that were vulnerable. This was an issue because the scanner would only cache and check the issues that were directly connected to the specific package placed in package.json, but wouldn't check the dependency tree of the package for downstream dependencies that are vulnerable. This PR makes it so that the scanner now checks the entire dependency tree of each package to scan and cache whether the package is vulnerable

@AntonBarashSnyk AntonBarashSnyk requested review from a team as code owners March 10, 2026 15:38
Copy link
Copy Markdown

@sathvi-k sathvi-k left a comment

Choose a reason for hiding this comment

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

a few questions about things i wanted to understand / things that might need to be changed

@@ -46,6 +46,40 @@
LOG_DIR = os.environ.get("SNYK_HOOK_LOG_DIR", "/tmp")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

just curious: where is SNYK_HOOK_LOG_DIR set?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not set anywhere explicitly right now, so presumably this line will always set LOG_DIR to /tmp unless there is an environment variable set for SNYK_HOOK_LOG_DIR by the user. I think having this as the default is fine since it'll fallback to tmp

Copy link
Copy Markdown

@ifeanyiecheruo ifeanyiecheruo left a comment

Choose a reason for hiding this comment

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

One non-blocking comment about moving code out of the pre-commit hook. Otherwise looks good

"fixed_version": v.fixed_version,
"cve": v.cve
})
# Find the top-level package (first package after project name in dependency path)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Non blocknig.

The pre-commit is getting kinda big. Maybe we should move this chunk into one of the existing files or make a new file for it

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.

3 participants