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

chore: document ValidatePackageNames #119

Merged
merged 1 commit into from
Mar 13, 2022

Conversation

achrinza
Copy link
Contributor

@achrinza achrinza commented Mar 13, 2022

see: #114

Signed-off-by: Rifa Achrinza 25147899+achrinza@users.noreply.github.com

This pull request adds the initial documentation for ValidatePackageNames and --validate-package-names/-n.

Description

This newly-added feature was not documented in any README file, which meant that it was only discoverable through either reading the codebase or through the CLI help menu (e.g. lockfile-lint --help). This pull request adds the minimum documentation to make it more accessible for those who want to quickly implement this tool.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation :-)

Related Issue

Nil

Motivation and Context

At LoopBack, we are in the works of implementing lockfile-lint as a solution to protect our Node.js projects from malicious lockfiles. We realised that one major problem that the READMEs did not address were lockfile modifications which pointed malicious packages published to known hosts, similar to what's described in #113.

While browsing the codebase, we realised that this was already addressed by the newly-released ValidatePackageNames validator. Hence, this pull request is to document this validator in the READMEs so that others can more easily discover it.

How Has This Been Tested?

A passing yarn run lint (i.e. no errors or new warnings), yarn run test and visual review through GitHub Markdown preview.

Yarn output
sh-5.1$ yarn install --frozen-lockfile
yarn install v1.22.17
[1/4] Resolving packages...
[2/4] Fetching packages...
info There appears to be trouble with your network connection. Retrying...
[3/4] Linking dependencies...
warning "workspace-aggregator-cd5a2f14-8237-48bb-9620-4a1ccaca7f0b > lockfile-lint-api > doxdox > doxdox-parser-dox@2.0.0" has incorrect peer dependency "doxdox@~2.0.3".
warning "workspace-aggregator-cd5a2f14-8237-48bb-9620-4a1ccaca7f0b > lockfile-lint-api > doxdox > doxdox-plugin-bootstrap@2.0.0" has incorrect peer dependency "doxdox@~2.0.3".
warning "workspace-aggregator-cd5a2f14-8237-48bb-9620-4a1ccaca7f0b > lockfile-lint-api > doxdox > doxdox-plugin-handlebars@2.0.0" has incorrect peer dependency "doxdox@~2.0.3".
warning "workspace-aggregator-cd5a2f14-8237-48bb-9620-4a1ccaca7f0b > lockfile-lint-api > doxdox > doxdox-plugin-markdown@2.0.0" has incorrect peer dependency "doxdox@~2.0.3".
[4/4] Building fresh packages...
Done in 217.25s.
sh-5.1$ yarn run lint
yarn run v1.22.17
$ lerna exec yarn lint
lerna notice cli v3.14.1
lerna info versioning independent
lerna info Executing command in 2 packages: "yarn lint"
$ eslint .
$ eslint .

/home/user/Documents/git-repos/github.com/lirantal/lockfile-lint/packages/lockfile-lint/bin/lockfile-lint.js
   36:3  warning  Don't use process.exit(); throw an error instead  no-process-exit
   92:3  warning  Don't use process.exit(); throw an error instead  no-process-exit
  103:3  warning  Don't use process.exit(); throw an error instead  no-process-exit

✖ 3 problems (0 errors, 3 warnings)

lerna success exec Executed command in 2 packages: "yarn lint"
Done in 8.37s.
sh-5.1$ yarn run test
yarn run v1.22.17
$ lerna exec yarn test
lerna notice cli v3.14.1
lerna info versioning independent
lerna info Executing command in 2 packages: "yarn test"
$ jest
 PASS  __tests__/validators.url.test.js
  Validator: Url
    ✓ validator should throw an error when provided a string (74ms)
    ✓ validator should throw an error when provided null (21ms)
    ✓ validator should throw an error when provided array (133ms)
    ✓ validator should throw an error instantiated with no value (2ms)
    ✓ validator should fail if not allowed url is used for a resource (7ms)
    ✓ validator should succeed if all resources are from an allowed URL (2ms)
    ✓ validator should fail if validate method receives a non-array value (4ms)
    ✓ validator should not fail even if one of the packages has no `resolved` field (1ms)
    ✓ validator should return true for a single package with a valid URL (2ms)
    ✓ validator should return false for a single package with an invalid URL (2ms)
    ✓ validator should return true for a single package that does not have a resolved URL (2ms)

 PASS  __tests__/validators.scheme.test.js (5.063s)
  Validator: Protocol
    ✓ validator should throw an error when provided a string (27ms)
    ✓ validator should throw an error when provided null (4ms)
    ✓ validator should throw an error when provided an object (5ms)
    ✓ validator should throw error if not provided array of values (4ms)
    ✓ validator should fail if finding a non-https resource (8ms)
    ✓ validator should succeed if resources match provided protocols (10ms)
    ✓ validator should succeed if package has no `resolved` field (7ms)

 PASS  __tests__/validators.host.test.js (5.377s)
  Validator: Host
    ✓ validator should throw an error when provided a string (30ms)
    ✓ validator should throw an error when provided null (5ms)
    ✓ validator should throw an error when provided array (4ms)
    ✓ validator should throw an error instantiated with no value (3ms)
    ✓ validator should fail if not allowed host is used for a resource (12ms)
    ✓ validator should succeed if all resources are from a valid host alias (4ms)
    ✓ validator should succeed if all resources are from a valid list of host aliases (4ms)
    ✓ validator should succeed if all resources are matching a host address (2ms)
    ✓ validator should succeed if all resources are matching a host address but input is a full URL (5ms)
    ✓ validator should fail if validate method receives a non-array value (4ms)
    ✓ validator should allow for git-based URLs (3ms)
    ✓ validator should not throw if emptyHostnames are allowed (3ms)
    ✓ validator should return errors if emptyHostnames are not allowed (3ms)
    ✓ validator should not throw if package has no `resolved` field (2ms)
    ✓ validator should return true for a single package on a valid host (2ms)
    ✓ validator should return true for a single package that does not have a resolved URL (1ms)
    ✓ validator should succeed if resources match a host:port address as input (2ms)
    ✓ validator should succeed if a resource matches a host:port address as input (1ms)

 PASS  __tests__/validators.packageNames.test.js
  Validator: PackageName
    ✓ validator should throw an error when provided a string (39ms)
    ✓ validator should throw an error when provided null (3ms)
    ✓ validator should throw an error when provided array (11ms)
    ✓ validator should fail if a resolved URL for a different package is found (9ms)
    ✓ validator should succeed if all resource URLs are for correct packages (5ms)
    ✓ validator should succeed if package has no `resolved` field (4ms)

 PASS  __tests__/validators.https.test.js
  Validator: HTTPS
    ✓ validator should throw an error when provided a string (39ms)
    ✓ validator should throw an error when provided null (9ms)
    ✓ validator should throw an error when provided array (6ms)
    ✓ validator should fail if finding a non-https resource (11ms)
    ✓ validator should succeed if all resources are https (4ms)
    ✓ validator should succeed if package has no `resolved` field (2ms)

 PASS  __tests__/parseNpmLockfile.test.js
  ParseLockfile Npm
    ✓ parsing an npm lockfile returns an object with packages (42ms)
    ✓ parsing an npm lockfile with nested deps returns an object with packages (13ms)
    ✓ parsing an npm lockfile with invalid content throws an error (45ms)

 PASS  __tests__/parseLockfile.test.js
  ParseLockfile
    ✓ instantiating a parser correctly works (13ms)
    ✓ instantiating a parser with invalid object throws an error (44ms)
    ✓ instantiating a parser with string type throws an error (4ms)
    ✓ using a parser with an invalid option type throws an error (4ms)
    ✓ using a parser with a path option for a lockfile of unknown type throws an error (4ms)
    ✓ using a parser with a path option for a lockfile that does not exist throws an error (18ms)

 PASS  __tests__/parseYarnLockfile.test.js
  ParseLockfile Yarn
    ✓ parsing a yarn lockfile returns an object with packages (26ms)
    ✓ parsing a yarn lockfile with invalid content throws an error (50ms)

 PASS  __tests__/app.test.js
  Validators
    library should export available validators
      ✓ validator ValidateHost.js should be available in index.js (13ms)
      ✓ validator ValidateHttps.js should be available in index.js (2ms)
      ✓ validator ValidatePackageNames.js should be available in index.js (2ms)
      ✓ validator ValidateScheme.js should be available in index.js (2ms)
      ✓ validator ValidateUrl.js should be available in index.js (1ms)
  Utils
    ✓ library should export ParseLockFile util (2ms)

----------------------------------|----------|----------|----------|----------|-------------------|
File                              |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
----------------------------------|----------|----------|----------|----------|-------------------|
All files                         |      100 |    92.93 |      100 |      100 |                   |
 lockfile-lint-api                |      100 |      100 |      100 |      100 |                   |
  index.js                        |      100 |      100 |      100 |      100 |                   |
 lockfile-lint-api/src            |      100 |     91.3 |      100 |      100 |                   |
  ParseLockfile.js                |      100 |     91.3 |      100 |      100 |           117,132 |
 lockfile-lint-api/src/common     |      100 |    83.33 |      100 |      100 |                   |
  ParsingError.js                 |      100 |    83.33 |      100 |      100 |                26 |
  constants.js                    |      100 |      100 |      100 |      100 |                   |
 lockfile-lint-api/src/validators |      100 |    94.29 |      100 |      100 |                   |
  ValidateHost.js                 |      100 |    95.83 |      100 |      100 |                86 |
  ValidateHttps.js                |      100 |    88.89 |      100 |      100 |                 8 |
  ValidatePackageNames.js         |      100 |    90.91 |      100 |      100 |                 6 |
  ValidateScheme.js               |      100 |    92.31 |      100 |      100 |                 6 |
  ValidateUrl.js                  |      100 |      100 |      100 |      100 |                   |
----------------------------------|----------|----------|----------|----------|-------------------|
Test Suites: 9 passed, 9 total
Tests:       65 passed, 65 total
Snapshots:   0 total
Time:        13.297s
Ran all test suites.
$ jest
 PASS  __tests__/config.test.js (6.55s)
  config
    ✓ running without parameters should display help (303ms)
    ✓ running with -h should display help (89ms)
    ✓ running without parameters should display a requirement for the p option (321ms)
    ✓ providing conflicting arguments should display an error (113ms)
    ✓ providing valid minimal arguments should return correct config (62ms)
    ✓ providing valid complete arguments should return correct config (44ms)
    cosmiconfig integration
      ✓ options are loaded from cosmiconfig files (28ms)
      ✓ command-line options take precedence (34ms)
      ✓ invalid config files are ignored (306ms)

  console.error src/main.js:48
    detected invalid protocol for package: debug@^4.1.1
        expected: https:
        actual: http:
    

  console.error src/main.js:48
    detected invalid protocol for package: ms@^2.1.1
        expected: https:
        actual: http:
    

  console.error src/main.js:48
    detected invalid protocol for package: debug@^4.1.1
        expected: https:
        actual: http:
    

  console.error src/main.js:48
    detected invalid protocol for package: ms@^2.1.1
        expected: https:
        actual: http:
    

  console.error src/main.js:48
    detected invalid host(s) for package: debug@^4.1.1
        expected: registry.npmjs.org
        actual: registry.yarnpkg.com
    

  console.error src/main.js:48
    detected invalid host(s) for package: ms@^2.1.1
        expected: registry.npmjs.org
        actual: registry.yarnpkg.com
    

  console.error src/main.js:48
    detected invalid host(s) for package: metalsmith-permalinks@github:XhmikosR/metalsmith-permalinks#432843d5823a292b2e47397ba46fd761d03eb9d3-23e3afb87f94f6fbb4a2a4dee0e9ad5ae3345557
        expected: registry.npmjs.org
        actual: 
    

  console.error src/main.js:48
    detected invalid host(s) for package: substitute@https://github.com/segment-boneyard/substitute/archive/0.1.0.tar.gz-cac7fab347605af7808679c31e920d7da07e6b0c
        expected: registry.npmjs.org
        actual: github.com
    

  console.error src/main.js:48
    detected invalid scheme(s) for package: debug@4.1.1-031b0fadad70d901aa76ca1028682c7fc8ed370c
        expected: https
        actual: https:
    

  console.error src/main.js:48
    detected invalid scheme(s) for package: ms@git+https://github.com/zeit/ms.git#adf1eb282d29fe3c405d205a3854177b86a97c1f-0f7c6a877d7eeaa579752be37c7fd80b9bd02a4a
        expected: https
        actual: git+https:
    

  console.error src/main.js:48
    detected invalid scheme(s) for package: debug@^4.1.1
        expected: https
        actual: https:
    

  console.error src/main.js:48
    detected invalid scheme(s) for package: ms@^2.1.1
        expected: https
        actual: https:
    

 PASS  __tests__/main.test.js (6.958s)
  Main CLI logic
    Invoking validators should handle errors and defaults
      ✓ when no validator function is provided expect to fail (97ms)
      ✓ should handle exceptions when validators arent provided for runValidators method (44ms)
      ✓ should handle unknown validator function name gracefully (9ms)
    validateHttp
      ✓ a failing validator should return proper validation failed object (67ms)
      ✓ a successful validator should return proper validation object (19ms)
    validateHosts
      ✓ a failing validator should return proper validation failed object (62ms)
      ✓ a failing validator should throw an error if an empty host is not allowed (27ms)
      ✓ a successful validator should return proper validation object (6ms)
    validateSchemes
      ✓ should fail validating allowed schemes for a package-lock.json file (39ms)
      ✓ should succeed validating allowed schemes for a package-lock.json file (7ms)
      ✓ should fail validating allowed schemes for a yarn.lock file (38ms)
      ✓ should succeed validating allowed schemes for a yarn.lock file (5ms)

  console.error src/main.js:48
    detected invalid scheme(s) for package: ms@git+https://github.com/zeit/ms#master
        expected: https
        actual: git+https:
    

  console.log __tests__/cli.test.js:164
    

 PASS  __tests__/cli.test.js (11.612s)
  CLI tests
    ✓ Running without parameters should display help (1172ms)
    ✓ Running without parameters should display a requirement for the p option (577ms)
    ✓ Linting a file that has wrong host should display an error message and use exit code 1 (535ms)
    ✓ Linting a file that has wrong host should return exit code 1 (480ms)
    ✓ Linting a file that has incorrect package name in resolved url should return exit code 1 (503ms)
    ✓ Providing conflicting arguments should display an error (576ms)
    ✓ Allowed hosts and allowed urls flags should work together (432ms)
    cosmiconfig integration
      ✓ options are loaded from cosmiconfig files (347ms)
      ✓ command-line options take precedence (329ms)
      ✓ invalid config files are ignored (328ms)
  Validator managers:
    ✓ Host manager should work together with URL manager (16ms)
    ✓ Host manager should return errors for lock file with packages on other hosts (2ms)
    ✓ URL manager should return errors for lock file with packages on other URLs (2ms)

----------------|----------|----------|----------|----------|-------------------|
File            |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
----------------|----------|----------|----------|----------|-------------------|
All files       |       92 |    88.24 |       90 |       92 |                   |
 src            |      100 |    90.91 |      100 |      100 |                   |
  config.js     |      100 |       75 |      100 |      100 |                 7 |
  main.js       |      100 |      100 |      100 |      100 |                   |
 src/validators |       85 |    83.33 |    83.33 |       85 |                   |
  index.js      |       85 |    83.33 |    83.33 |       85 | 84,90,95,96,97,99 |
----------------|----------|----------|----------|----------|-------------------|
Test Suites: 3 passed, 3 total
Tests:       34 passed, 34 total
Snapshots:   0 total
Time:        13.68s

Screenshots (if appropriate):

Nil.

Checklist:

  • I have updated the documentation (if required).
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I added a picture of a cute animal cause it's fun

see: lirantal#114

Signed-off-by: Rifa Achrinza <25147899+achrinza@users.noreply.github.com>
@lirantal lirantal self-requested a review March 13, 2022 14:27
@lirantal lirantal added the documentation Improvements or additions to documentation label Mar 13, 2022
@lirantal
Copy link
Owner

Thank you Rifa, this looks great and was indeed overlooked.

@lirantal lirantal merged commit 5ce8330 into lirantal:master Mar 13, 2022
@achrinza achrinza deleted the docs/validate-package-names branch March 14, 2022 03:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants