search

NYULibraries / primo-explore-views

A consolidated monorepo of NYU Consortium views
MIT License
2 stars 1 forks source link

Clear dependabot security vulnerability alerts #335

Open da70 opened 2 years ago

da70 commented 2 years ago

Branch: https://github.com/NYULibraries/primo-explore-views/tree/chore/clear-dependabot-security-vulnerability-alerts_335

Looked through all alerts. This is the initial list of minimum patch versions:

ajv: 6.12.3 engine.io: 4.0.0 ini: 1.3.6 lodash: 4.17.21 minimist: 1.2.6 moment: 2.29.2 shelljs: 0.8.5 tar: 6.1.9 trim-newlines: 3.0.1

Other NYULibraries packages that are imported as dependencies by this project, which might need to be cleared also:

Current status

da70 commented 2 years ago

Was going to configure dependabot to target development branch for automatic security vulnerability PRs, but it's not possible. See (closed) ticket #336.

da70 commented 2 years ago

Read through the alerts that didn't not have PRs attached to them because they seemed to be the ones containing information on dependency conflicts that prevent dependabot from updating (and creating a PR). Many of the conflicts require an upgrade of the cypress package for direct resolution. There are other conflicts caused by cypress transitive dependencies. Will try upgrading cypress first to the latest stable version.

da70 commented 2 years ago

Upgraded cypress from 3.4.0 to 9.5.4. If I am understanding how dependabot works, clicking "Try again" on an alert would probably not be helpful since the upgrade is not yet in master. I'm not sure what the frequency of dependabot security checks are. For now I'll just manually check the versions of the packages in the current alerts (note that there could security vulnerabilities in the upgraded packages as well, which we won't know about until the upgrades make it to master and dependabot runs again).

da70 commented 2 years ago

primo-explore-e2e-cypress: OK

primo-explore-e2e-cypress> for package in ajv engine.io ini lodash minimist moment shelljs tar trim-newlines; do version=$( grep '"version":' node_modules/${package}/package.json ); echo "$package: $version"; done
grep: node_modules/ajv/package.json: No such file or directory
ajv: 
grep: node_modules/engine.io/package.json: No such file or directory
engine.io: 
ini:   "version": "2.0.0",
lodash:   "version": "4.17.21",
minimist:   "version": "1.2.6",
grep: node_modules/moment/package.json: No such file or directory
moment: 
grep: node_modules/shelljs/package.json: No such file or directory
shelljs: 
grep: node_modules/tar/package.json: No such file or directory
tar: 
grep: node_modules/trim-newlines/package.json: No such file or directory
trim-newlines: 
primo-explore-e2e-cypress>

Note that the moment package is no longer a dependency. This is not surprising given that the moment team has declared Moment.js a legacy project: Project Status

da70 commented 2 years ago

Upgraded primo-explore-e2e-cypress/Dockerfile base image to cypress/browsers:node16.13.2-chrome100-ff98 -- Cypress requires Node 12.0.0 or higher.

da70 commented 2 years ago

The four remaining packages that need to be upgraded likely require the other primo NPM packages to be cleared first. Will start on those. In the meantime, will want to see if the tests pass using the new Cypress.

da70 commented 2 years ago

I notice that there only appear to be dependabot alerts for top-level and primo-explore-e2e-cypress/, not for the custom/ subdirectories. The custom/ subdirectories have package.json files but no yarn.lock files -- I'm guessing that's why.

da70 commented 2 years ago

run-e2e-NYU (7363) -- FAIL

[This is from 4/22/22 -- copying notes from my personal issue tracker]

Run step in new tab: https://app.circleci.com/pipelines/github/NYULibraries/primo-explore-views/1007/workflows/325236fa-32a4-4e99-a7a0-9af4f055fa22/jobs/7363/parallel-runs/0/steps/0-105

Raw: https://circleci.com/api/v1.1/project/github/NYULibraries/primo-explore-views/7363/output/105/0?file=true&allocation-id=62633829eb41c9465bb0d930-0-build%2F756D31

Online search turned up references to this error:

FROM docker:20-git

Docker for project: .circleci/config.yml:

    - image: cimg/base:2021.07

https://circleci.com/developer/images/image/cimg/base:

2021.07 2021.07-20.04 build-essential 12.8ubuntu1.1, curl 7.68.0, docker 20.10.7, docker-compose /usr/local/bin/docker-compose 1.29.2, dockerize v0.6.1, git 2.32.0, jq 1.6, ubuntu 20.04.2 LTS, wget 1.20.3 Show less 332.18 MiB Jul 2, 2021

Could be because the Dockerfiles for the top-level project and for primo-explore-e2e-cypress have clashing Node versions? Top-level Dockerfile base image -- https://github.com/NYULibraries/primo-explore-views/blob/dd3dacbd5fb80985cd9a6c8cf372280e7a549d88/Dockerfile#L1:

FROM quay.io/nyulibraries/primo-explore-devenv:2.0.0

Here's the manifest: https://quay.io/repository/nyulibraries/primo-explore-devenv/manifest/sha256:7d47064d2b69dd7bb6e1982d0190ae689fedff927dd5cd6450e2d0ad13688163:

NODE_VERSION=10.24.1

Also: primo-explore-views/docker-compose.yml:

  yarn:
    image: node:12
    working_dir: /app
    entrypoint: ["yarn"]
    volumes:
      - ./:/app

Also in container /app/ is actually NYULibraries / primo-explore-devenv, which has its own set of npm dependencies. Currently dependabot alerts are disabled for that repo, perhaps because it's a fork.

da70 commented 2 years ago

Cypress tests running against updated local: FAIL

EDIT: Forgot that vid=[VIEW] query param doesn't work on local, have to specify the view using VIEW environment variable when issuing the docker-compose up command.

Saw lots of test failures just for Home page functionality. Looks like vid=[VIEW] is not working on local for some reason:

image

...should look like this (screenshot of dev):

image
da70 commented 2 years ago

Eric directed me to this Web Services wiki page: CircleCI yarn build failures.

da70 commented 2 years ago

Upgraded CircleCI docker to 20.10.14: ca50fbb8555a7bb922983f10287bb3bfb20bb503. This is the latest available in CircleCI: Docker version.

da70 commented 2 years ago

Got the error again: https://app.circleci.com/pipelines/github/NYULibraries/primo-explore-views/1021/workflows/66acdebd-ce3a-400d-bc73-93e70bd7cb7c/jobs/7509/parallel-runs/0/steps/0-105

error An unexpected error occurred: "EPERM: operation not permitted, copyfile '/usr/local/share/.cache/yarn/v6/npm-aggregate-error-3.1.0-92670ff50f5359bdb7a3e0d40d0ec30c5737687a-integrity/node_modules/aggregate-error/index.d.ts' -> '/app/node_modules/aggregate-error/index.d.ts'".

Node:

Step 1/9 : FROM cypress/browsers:node16.13.2-chrome100-ff98
da70 commented 2 years ago

Testing 21 different combinations of CircleCI remote Docker version and cypress/browser image version (2 of these tests were already done previously):

CircleCI remote Docker versions:

20.10.14 20.10.12 20.10.11 20.10.7 20.10.6 20.10.2 19.03.13

cypress/browser image versions:

node16.13.2-chrome100-ff98 node14.17.6-chrome100-ff98 node12.18.3-chrome89-ff86

da70 commented 2 years ago

Results of the Docker vs. Node version tests

[EDIT: updated table and results of the temp-branch-to-run-tests-for-commit-before-dependency-upgrades test]

node12.18.3-chrome89-ff86 node14.17.6-chrome100-ff98 node16.13.2-chrome100-ff98
19.03.13 OK, but with Cypress tests failures FAIL FAIL
20.10.2 OK, but with Cypress tests failures FAIL FAIL
20.10.6 OK, but with Cypress tests failures FAIL FAIL
20.10.7 OK, but with Cypress tests failures FAIL FAIL
20.10.11 OK, but with Cypress tests failures FAIL FAIL
20.10.12 OK, but with Cypress tests failures FAIL FAIL
20.10.14 OK, but with Cypress tests failures FAIL FAIL

OK = Most e2e tests passed, and a spot check of one of the few failed jobs showed that failure was due to Cypress failed test, not the CircleCI yarn build failures error FAIL = All e2e tests failed, and at least job was observed to have the CircleCI yarn build failures error in its output

Looks like Node 12 is currently our only possible option for the updated Cypress (which requires 12 or higher). At first glance it looks like it's the same few tests that are failing for each Docker version. I'll have to take a closer look.

Created a temporary branch from the commit before the dependency upgrades and pushed to see if there are any tests failures in CircleCI -- temp-branch-to-run-tests-for-commit-before-dependency-upgrades. All tests passed, which strongly suggests that the dependency upgrades themselves are causing test failures.

da70 commented 2 years ago

Reverted primo-explore-e2e-cypress/package.json and primo-explore-e2e-cypress/yarn.lock files to state before did any dependency upgrades, but maintained the Docker version of 20.10.14 and cypress/browser image version of node12.18.3-chrome89-ff86. Tests are running: https://app.circleci.com/pipelines/github/NYULibraries/primo-explore-views/1078/workflows/ab0267fb-8063-41eb-90a2-7725e868ecb1

da70 commented 2 years ago

All tests passed. Upgraded cypress to 10.1.0. Tests are running: https://app.circleci.com/pipelines/github/NYULibraries/primo-explore-views/1079/workflows/ed2a350e-b1f7-4273-a2c2-a31d7e4299bd

da70 commented 2 years ago

Tests for 10.1.0 commit failed: https://app.circleci.com/pipelines/github/NYULibraries/primo-explore-views/1079/workflows/ed2a350e-b1f7-4273-a2c2-a31d7e4299bd

Reason:

There is a cypress.json file at the path: /app

Cypress version 10.0.0 no longer supports cypress.json.

Please run cypress open to launch the migration tool to migrate to cypress.config.{js,ts,mjs,cjs}.

https://on.cypress.io/migration-guide

error Command failed with exit code 1.

Note that documentation for earlier versions is no longer available: Allow selecting older version of cypress documentation on the site #4050. Unless the documentation is available elsewhere, we should consider upgrading to 10.

Pushed a commit for cypress 9.7.0 -- tests failed, apparently with the same errors seen with the other Node 12 cypress image commits: https://app.circleci.com/pipelines/github/NYULibraries/primo-explore-views/1080/workflows/3adc8c69-d1cb-46e3-a94b-791957fdd2ef/jobs/8088

I checked out cypress 9.7.0 on local. Indeed, the local Primo breaks in the Chrome browser running in the cypress environment during a test run:

image

...but works fine in Brave outside of the cypress environment:

image

...and also in a new, separate Chrome window:

image
da70 commented 2 years ago

While there don't appear to be any full documentation sets for previous cypress versions, there is on the live doc site a separate page for cypress.json configuration: Configuration (Legacy).