Report transitive dependency vulnerability errors for npm, yarn, and pnpm

[kbukum1]

Description:

What are you trying to accomplish?

This PR aims to fix a bug for the npm_and_yarn ecosystem. The changes include:

1. Implementing accurate reporting for transitive dependencies by returning nil when there are conflicting dependencies and the dependency is not top-level.
2. Ensuring that the correct security fix version is identified for both top-level and transitive dependencies within the npm_and_yarn ecosystem.

This change is necessary to ensure that our system correctly handles and reports vulnerable transitive dependencies within the npm_and_yarn ecosystem, improving our ability to maintain a clear and accurate dependency relationship.

Anything you want to highlight for special attention from reviewers?

This change specifically addresses the bug related to conflicting transitive dependencies in the npm_and_yarn ecosystem.

How will you know you've accomplished your goal?

• The system accurately identifies and reports the correct security fix versions for both top-level and transitive dependencies within the npm_and_yarn ecosystem.
• The method handles cases with conflicting dependencies correctly by returning nil for transitive dependencies with conflicts.
• Additional tests are added to verify the changes, and all tests pass successfully.

Checklist

• I have run the complete test suite to ensure all tests and linters pass.
• I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
• I have written clear and descriptive commit messages.
• I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
• I have ensured that the code is well-documented and easy to understand.

[kbukum1]

@jakecoffman, @jurre

This change affects the core functionality of Dependabot. @abdulapopoola has already approved the PR. It would be greatly appreciated if one of you could also review it to ensure its accuracy and stability before we proceed with merging.

Thank you!

[jakecoffman]

Thanks for working on fixing these NoChangeError exceptions.

This solution looks like you're on the right track, but I think we can push the check for a conflicting transitive dependency down into the ecosystem.

dependabot-core/npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker.rb

Lines 67 to 76 in 85a6dac

	def lowest_resolvable_security_fix_version
	raise "Dependency not vulnerable!" unless vulnerable?
	# NOTE: we currently don't resolve transitive/sub-dependencies as
	# npm/yarn don't provide any control over updating to a specific
	# sub-dependency version
	return latest_resolvable_transitive_security_fix_version_with_no_unlock unless dependency.top_level?
	# TODO: Might want to check resolvability here?
	lowest_security_fix_version
	end

I tested returning nil from lowest_resolvable_security_fix_version and it responded with

{"data":{"error-type":"security_update_not_possible","error-details":{"conflicting-dependencies":[{"explanation":"dependency@0.2.0 requires webpack@~5.55.1","name":"dependency","requirement":"~5.55.1","version":"0.2.0"},{"dependency_name":"webpack","explanation":"No patched version available for webpack","fix_available":false,"fix_updates":[],"top_level_ancestors":[]}],"dependency-name":"webpack","latest-resolvable-version":"5.55.1","lowest-non-vulnerable-version":"5.76.0"}},"type":"record_update_job_error"}

So we're already setup to report these as expected errors including the conflicting dependency data. I think the root cause here is lowest_resolvable_security_fix_version should not return an unresolvable version.

In summary, you should be able to add something like return nil if !dependency.top_level? && conflicting_dependencies.any? to the lowest_resolvable_security_fix_version to have the same effect, and not have to change the updater code.

[kbukum1]

Thanks for working on fixing these NoChangeError exceptions.

This solution looks like you're on the right track, but I think we can push the check for a conflicting transitive dependency down into the ecosystem.

dependabot-core/npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker.rb

Lines 67 to 76 in 85a6dac

	def lowest_resolvable_security_fix_version
	raise "Dependency not vulnerable!" unless vulnerable?
	# NOTE: we currently don't resolve transitive/sub-dependencies as
	# npm/yarn don't provide any control over updating to a specific
	# sub-dependency version
	return latest_resolvable_transitive_security_fix_version_with_no_unlock unless dependency.top_level?
	# TODO: Might want to check resolvability here?
	lowest_security_fix_version
	end

I tested returning nil from lowest_resolvable_security_fix_version and it responded with

{"data":{"error-type":"security_update_not_possible","error-details":{"conflicting-dependencies":[{"explanation":"dependency@0.2.0 requires webpack@~5.55.1","name":"dependency","requirement":"~5.55.1","version":"0.2.0"},{"dependency_name":"webpack","explanation":"No patched version available for webpack","fix_available":false,"fix_updates":[],"top_level_ancestors":[]}],"dependency-name":"webpack","latest-resolvable-version":"5.55.1","lowest-non-vulnerable-version":"5.76.0"}},"type":"record_update_job_error"}

So we're already setup to report these as expected errors including the conflicting dependency data. I think the root cause here is lowest_resolvable_security_fix_version should not return an unresolvable version.

In summary, you should be able to add something like return nil if !dependency.top_level? && conflicting_dependencies.any? to the lowest_resolvable_security_fix_version to have the same effect, and not have to change the updater code.

Thanks @jakecoffman. I see. I will check it and if it relating this error to the it's main dependency I will make changes. I think here only I have a concern regarding we are trying to create PR for webpack main dependency and we don't need that since it is already up-to-date and not vulnerable. But because another unrelated webpack transitive dependency we are throwing error. If this error shows security update not possible relating it to the main webpack dependency then I think that is not going to work since it is kind of confussion and unrelated error with main webpack dependency. But really thanks for showing that.

[kbukum1]

Thank you @jakecoffman. Just checked and the solution is going to work. Just need to tested it. Thank you.

[kbukum1]

Thank you @jakecoffman. Just checked and the solution is going to work. Just need to tested it. Thank you.

@jakecoffman , @abdulapopoola the problem is fixed in the way mentioned in the comment. It is ready for review.

[jakecoffman]

It's a bad practice to mock the class you're testing.

I would suggest looking at another test in this file that uses @dependabot-fixtures. Create a minimal reproduction of the NoChangeError in that org and use it in this test to verify lowest_resolvable_security_fix_version returns nil.

[kbukum1]

fixed

[jakecoffman]

This is great, nice work!

[kbukum1]

This is great, nice work!

Thanks for all the feedback.

[kbukum1]

The error is going to show like following in the log.

2024/08/02 17:34:26 INFO Starting job processing
2024/08/02 17:34:26 INFO Starting security update job for example/repository
2024/08/02 17:34:26 INFO Checking if webpack 5.55.1 needs updating
2024/08/02 17:34:27 INFO Latest version is 5.93.0
2024/08/02 17:34:28 INFO VulnerabilityAuditor: starting audit
2024/08/02 17:34:36 INFO VulnerabilityAuditor: audit result not viable: downgrades_dependencies
2024/08/02 17:34:36 INFO Requirements to unlock update_not_possible
2024/08/02 17:34:36 INFO Requirements update strategy bump_versions_if_necessary
2024/08/02 17:34:38 INFO The latest possible version that can be installed is 5.55.1 because of the following conflicting dependencies:

  parent-dependency@0.2.0 requires webpack@~5.55.1
  No patched version available for webpack
2024/08/02 17:34:38 INFO The earliest fixed version is 5.76.0.
2024/08/02 17:34:38 INFO Finished job processing
2024/08/02 17:34:38 INFO Results:
Dependabot encountered '1' error(s) during execution, please check the logs for more details.
+------------------------------+
|            Errors            |
+------------------------------+
| security_update_not_possible |
+------------------------------+