snyk-tech-services / snyk-pnpm-github-action

Other
2 stars 2 forks source link

pnpmLockfilePath also looking for `package.json` #10

Open shailygupta opened 2 years ago

shailygupta commented 2 years ago

Issue

We are currently attempting to utilize this action to scan and test our pnpm project, however, we are running into the following issue:

Error: Error: Error: package.json not found at location: ./common/config/rush/package.json

.github/workflows/snyk.yaml

name: Snyk

on:
  push:
    branches: [ $default-branch ]
  pull_request:

jobs:
  pnpm_snyk_check_dev:
    runs-on: ubuntu-latest
    name: Snyk Dev Check
    steps:
      - uses: actions/checkout@v2
      - name: Snyk API pnpm scan
        id: pnpm-scan-api-tool
        uses: snyk-tech-services/snyk-pnpm-github-action@master
        with:
          snykToken: ${{ secrets.SNYK_TOKEN}}
          pnpmLockfilePath: "./common/config/rush" 
          snykOrganization: "Engineering"

Directory structure:

.
├── README.md
├── apps
│   ├── developer-example
│   │   ├── README.md
│   │   ├── build.js
│   │   ├── cdk
│   │   │   ├── accountProps.ts
│   │   │   ├── deployDeveloperExampleStack.ts
│   │   │   └── developerExampleStack.ts
│   │   ├── cdk.json
│   │   ├── deploy.sh
│   │   ├── package.json
│   │   ├── src
│   │   │   └── handlers
│   │   │       └── helloWorldLambda.ts
│   │   ├── tests
│   │   │   ├── e2e
│   │   │   │   └── api.test.ts
│   │   │   └── unit
│   │   │       └── handlers
│   │   │           └── helloworldLambda.test.ts
│   │   └── tsconfig.json
├── common
│   ├── config
│   │   └── rush
│   │       ├── artifactory.json
│   │       ├── build-cache.json
│   │       ├── command-line.json
│   │       ├── common-versions.json
│   │       ├── experiments.json
│   │       ├── pnpm-lock.yaml
│   │      └── version-policies.json

Expected

An optional variable that allows the specification of the package.json path as well in the action

mathild3r commented 2 years ago

Hello,

I am Mathilde from technical services team at Snyk. Currently we don't support this configuration, I've created a ticket on our side to support this. We should be able to start working on this sometimes next week, I will keep you updated.

Thank you, Mathilde

collinroth commented 2 years ago

Hi Mathilde,

I work with @shailygupta. One thing to keep in mind with this request is the context of why we have these files in two locations:

The reason that we are asking for the ability to specify the pnpm-lockfile.yaml location separately from the package.json location is that we are using Rushjs as a monorepo orchestrator in combination with pnpm as a package manager. In this combination, each project within the monorepo will have its own package.json, and rush will maintain a centralized pnpm-lockfile.yaml. We are trying to get Snyk to work in this environment. Initially, we were hopeful that your support for pnpm via this github-action would align with this use case.

For example, as a crude experiment before your proposed modifications, I copied the pnpm-lockfile.yaml into a project subdir and tried running snyk-pnpm-deptree-api-tool - which ultimately I believe this github-action is using. Sadly, we see an exception reported from the underlying tool.

✗ snyk-pnpm-deptree-api-tool --root `pwd` --orgId <org> --snykToken <token>
(node:23525) UnhandledPromiseRejectionWarning: Error: Error: Fail to generate a depGraph TypeError: Cannot read property 'dev' of undefined
    at main (/Users/collinroth/.nvm/versions/node/v14.18.0/lib/node_modules/snyk-pnpm-deptree-api-tool/src/lib/index.ts:291:11)
(Use `node --trace-warnings ...` to show where the warning was created)
(node:23525) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:23525) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

Thus, assuming my test is valid, it appears there's more to the story than just a new argument for this github action (and the underlying snyk-pnpm-deptree-api-tool). We really are looking for the snyk-pnpm-github-action to support pnpm when used within the context of a monorepo being managed by Rushjs. It would be great if your suggested changes would accomplish that goal.

Please note that I've also raised a support request with Snyk Support on this subject.

Thanks, Collin

mathild3r commented 2 years ago

Hi @collinroth @shailygupta ,

Thank you for the clarification.

If you hit this error it might be because you lockfile link package together and having move the lockfile the link is not valid anymore.

Are you able to share more information about your configuration? and maybe share some files?

In the meantime I will start looking into pnpm monorepo configuration.

Thank you, Mathilde

mathild3r commented 2 years ago

Hi,

Apologies I just saw that you already gave me the file structure in the previous message. I will need to see the pnpm-lockfile and package.json to answer your question about the exception. I've asked our CSM to see if we can setup a meeting so we can have a look.

Thank you, Mathilde

mathild3r commented 2 years ago

Hello,

I have setup my own rush project and was able to reproduce the error. it look like Rush is adding lines in the pnpm lockfile ie :

dependencies:
  '@rush-temp/goof': file:projects/goof.tgz
  adm-zip: 0.4.7

and then later:

 file:projects/goof.tgz:
    resolution: {integrity: sha512-7LWISx3BwBuxoxCOaBf/Mhucb9T+XCU/3t8VfcPZV5E9c6FY/XaPbWoWLtDswR/Ju7poHaA7RFGplHR7/ZMoRw==, tarball: file:projects/goof.tgz}
    name: '@rush-temp/goof'
    version: 0.0.0
    dependencies:
      optional: 0.1.4

For a dependency like this ( '@rush-temp/goof': file:projects/goof.tgz) our parser will then look for : @rush-temp/goof/file:projects/goof.tgz to get the details and subdependencies but as you can see in my example the name doesn't correspond which make the parser fail.

I am looking to add support for this configuration and will then look at updating the api tool and github action. I think Nora will be in touch with you to setup a meeting and have a look at your configuration.

Thank you, Mathilde

mathild3r commented 2 years ago

Hi @collinroth @shailygupta ,

I wanted to let you know I've released a new version of the snyk-pnpm-deptree-api-tool today with support for monorepo (new optionnal parameter --manifestFilePath ) if you would like to try.

I am working on the github action and will update you when available.

Thank you, Mathilde

collinroth commented 2 years ago

Hello @mathild3r ,

I've tried your new version of the snyk-pnpm-deptree-api-tool and it seems to go into an infinite loop. CPU stays at 100%, and it doesn't return.

Any suggestions? -collin