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

BUG: --npm wrong input does not throw error #3166

Closed
gabibguti opened this issue Jun 14, 2023 · 2 comments · Fixed by #4118
Closed

BUG: --npm wrong input does not throw error #3166

gabibguti opened this issue Jun 14, 2023 · 2 comments · Fixed by #4118
Labels
area/cli Command Line Interface good first issue Good for newcomers kind/bug Something isn't working

Comments

@gabibguti
Copy link
Contributor

Describe the bug
Scorecard can receive as input the name of the package from npm, pypi and rubygems ecosystems as per the documentation. Reading the documentation, it was not clear to me that I needed to provide the package name and providing a package URL does not throw an error but runs the evaluation with a weird behavior.

Reproduction steps
Steps to reproduce the behavior:

  1. Run Scorecard v4.10.2 with --npm=https://github.com/airbnb/lottie-web
  2. See output results for repo: name: github.com/perrmadiafrrian/react-lottie-light

Expected behavior
I expected Scorecard would warn me I made a mistake in the --npm flag input and not run the evaluation for an unexpected repository.

Additional context
None.

@gabibguti gabibguti added the kind/bug Something isn't working label Jun 14, 2023
@spencerschrock
Copy link
Contributor

spencerschrock commented Jun 14, 2023

Has to do with the implementation using npm's search functionality:
https://www.npmjs.com/search?q=https%3A%2F%2Fgithub.com%2Fairbnb%2Flottie-web

func fetchGitRepositoryFromNPM(packageName string, packageManager packageManagerClient) (string, error) {
npmSearchURL := "https://registry.npmjs.org/-/v1/search?text=%s&size=1"
resp, err := packageManager.Get(npmSearchURL, packageName)
if err != nil {

If we do a different endpoint, could probably catch this sort of thing:
https://registry.npmjs.org/<package>

https://registry.npmjs.org/lottie-web works
https://registry.npmjs.org/https://github.com/airbnb/lottie-web doesn't work

@spencerschrock spencerschrock added the area/cli Command Line Interface label Jun 14, 2023
@spencerschrock spencerschrock added the good first issue Good for newcomers label Apr 15, 2024
@spencerschrock
Copy link
Contributor

Also related to #2441

aklevans added a commit to aklevans/scorecard that referenced this issue May 20, 2024
aklevans added a commit to aklevans/scorecard that referenced this issue May 20, 2024
Signed-off-by: aklevans <alexklevans@gmail.com>
aklevans added a commit to aklevans/scorecard that referenced this issue May 21, 2024
Signed-off-by: aklevans <alexklevans@gmail.com>
spencerschrock pushed a commit that referenced this issue Jun 5, 2024
…pm database (#4118)

* Update endpoint used when getting repo from npm to solve #3166

Signed-off-by: aklevans <alexklevans@gmail.com>

* Update test files to account for endpoint change when getting repo from npm

Signed-off-by: aklevans <alexklevans@gmail.com>

* Fix linter issues

Signed-off-by: aklevans <alexklevans@gmail.com>

* Added unit tests for #3166 and #2441

Signed-off-by: aklevans <alexklevans@gmail.com>

* fix linter issues and reduce mock json output in package_manager_test to only include necessary data

Signed-off-by: aklevans <alexklevans@gmail.com>

* fix linter issues in package_managers.go

Signed-off-by: aklevans <alexklevans@gmail.com>

* convert windows line breaks to linux

Signed-off-by: aklevans <alexklevans@gmail.com>

* reduce test case size, still has windows line breaks

Signed-off-by: aklevans <alexklevans@gmail.com>

* Fix unit tests

Signed-off-by: aklevans <alexklevans@gmail.com>

* attempt linter fix

Signed-off-by: aklevans <alexklevans@gmail.com>

* Fix linter issues stemming from windows line breaks

Signed-off-by: aklevans <alexklevans@gmail.com>

* Remove magic number and rename variable to be more accurate

Signed-off-by: aklevans <alexklevans@gmail.com>

---------

Signed-off-by: aklevans <alexklevans@gmail.com>
Signed-off-by: aklevans <105876795+aklevans@users.noreply.github.com>
balteravishay pushed a commit to balteravishay/scorecard that referenced this issue Jun 12, 2024
…pm database (ossf#4118)

* Update endpoint used when getting repo from npm to solve ossf#3166

Signed-off-by: aklevans <alexklevans@gmail.com>

* Update test files to account for endpoint change when getting repo from npm

Signed-off-by: aklevans <alexklevans@gmail.com>

* Fix linter issues

Signed-off-by: aklevans <alexklevans@gmail.com>

* Added unit tests for ossf#3166 and ossf#2441

Signed-off-by: aklevans <alexklevans@gmail.com>

* fix linter issues and reduce mock json output in package_manager_test to only include necessary data

Signed-off-by: aklevans <alexklevans@gmail.com>

* fix linter issues in package_managers.go

Signed-off-by: aklevans <alexklevans@gmail.com>

* convert windows line breaks to linux

Signed-off-by: aklevans <alexklevans@gmail.com>

* reduce test case size, still has windows line breaks

Signed-off-by: aklevans <alexklevans@gmail.com>

* Fix unit tests

Signed-off-by: aklevans <alexklevans@gmail.com>

* attempt linter fix

Signed-off-by: aklevans <alexklevans@gmail.com>

* Fix linter issues stemming from windows line breaks

Signed-off-by: aklevans <alexklevans@gmail.com>

* Remove magic number and rename variable to be more accurate

Signed-off-by: aklevans <alexklevans@gmail.com>

---------

Signed-off-by: aklevans <alexklevans@gmail.com>
Signed-off-by: aklevans <105876795+aklevans@users.noreply.github.com>
Signed-off-by: balteraivshay <avishay.balter@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cli Command Line Interface good first issue Good for newcomers kind/bug Something isn't working
Projects
Status: Done
2 participants