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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

deps: Update audit-ci implementation #1045

Open
wants to merge 11 commits into
base: dev
Choose a base branch
from

Conversation

quinnturner
Copy link

@quinnturner quinnturner commented Apr 8, 2022

I am the author of audit-ci. I noticed this project was using it, and I wanted to help out!

  • Install latest audit-ci devDependency
  • Use yarn dlx instead of installed version in CI
  • Use .audit-ci.jsonc for better advisory management
  • Remove unused yarn install during the pipeline
  • Updates settings.json in VSCode to hide .audit-ci.jsonc
  • Removes unused advisory 1005059

I do have a separate approach for you to consider: run audit-ci immediately after running actions/setup-nodeand before the yarn install with yarn dlx in the release workflow. That way, if there's a compromised dependency that runs a postinstall in the release, you will catch it before it installs.

馃懏馃徎馃懏馃徎馃懏馃徎 !!!! REFERENCE THE PROBLEM YOUR ARE SOLVING IN THE PR TITLE AND DESCRIBE YOUR SOLUTION HERE !!!! DO NOT FORGET !!!! 馃懏馃徎馃懏馃徎馃懏馃徎

PR Checklist:

  • Have you read the CONTRIBUTING.md?
  • Does your PR follow the C4 patch requirements?
  • Have you rebased your work on top of the latest master?
  • Have you checked your code compiles? (yarn build)
  • Have you included tests for any non-trivial functionality?
  • Have you checked your code passes the unit tests? (yarn test)
  • Have you checked your code formatting is correct? (yarn lint:js)
  • If you added any dependencies, have you checked they do not contain any known vulnerabilities? (yarn audit)
  • If your changes affect public APIs, does your PR follow the C4 evolution of public contracts?
  • If your code changes public APIs, have you incremented the crate version numbers and documented your changes in the CHANGELOG.md?
  • If you are contributing for the first time, please read the agreement in CONTRIBUTING.md now and add a comment to this pull request stating that your PR is in accordance with the Developer's Certificate of Origin.

Thank you for your code, it's appreciated! :)

- Install latest audit-ci devDependency
- Use yarn dlx instead of installed version in CI
- Use .audit-ci.jsonc for better advisory management
- Remove unused yarn install during the pipeline
- Updates settings.json in VSCode to hide .audit-ci.jsonc
- Removes unused advisory 1005059
@lgtm-com
Copy link

lgtm-com bot commented May 19, 2022

This pull request introduces 27 alerts when merging 6960446 into cc73b5c - view on LGTM.com

new alerts:

  • 16 for Unused variable, import, function or class
  • 8 for Useless assignment to local variable
  • 1 for Useless conditional
  • 1 for Comparison between inconvertible types
  • 1 for Incomplete string escaping or encoding

@lgtm-com
Copy link

lgtm-com bot commented May 19, 2022

This pull request introduces 27 alerts when merging 4f8fc44 into cc73b5c - view on LGTM.com

new alerts:

  • 16 for Unused variable, import, function or class
  • 8 for Useless assignment to local variable
  • 1 for Useless conditional
  • 1 for Comparison between inconvertible types
  • 1 for Incomplete string escaping or encoding

@lgtm-com
Copy link

lgtm-com bot commented May 19, 2022

This pull request introduces 27 alerts when merging f84e60d into cc73b5c - view on LGTM.com

new alerts:

  • 16 for Unused variable, import, function or class
  • 8 for Useless assignment to local variable
  • 1 for Useless conditional
  • 1 for Comparison between inconvertible types
  • 1 for Incomplete string escaping or encoding

@lgtm-com
Copy link

lgtm-com bot commented May 19, 2022

This pull request introduces 27 alerts when merging 42df653 into cc73b5c - view on LGTM.com

new alerts:

  • 16 for Unused variable, import, function or class
  • 8 for Useless assignment to local variable
  • 1 for Useless conditional
  • 1 for Comparison between inconvertible types
  • 1 for Incomplete string escaping or encoding

Comment on lines +7 to +8
/.yarn/*
!/.yarn/releases
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/.yarn/*
!/.yarn/releases
.yarn/*
# Uncomment !.yarn/cache/ if you want Zero-Installs
# !.yarn/cache
!.yarn/patches
!.yarn/plugins
!.yarn/releases
!.yarn/sdks
!.yarn/versions

with:
node-version: 14

- name: Upgrade to Yarn 3.0
run: |
yarn set version 3.0
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't recommend doing this. You should add the Yarn release to git based on the gitignore I suggested in this review. Yarn with automatically pick it up.


- name: Remove any cached files
run: |
rm -Rf ./node_modules && rm -Rf ./.yarn/cache && rm -Rf ./.yarn/unplugged
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should be necessary.

run: |
yarn run-audit
yarn dlx audit-ci@^6 --config .audit-ci.jsonc
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also just use npx if you want. It doesn't matter which dependency manager's task runner you use, audit-ci will look at lock files. I usually use npx regardless of the dependency manager tbh since it seems quicker.

@@ -22,11 +22,11 @@ jobs:

- name: Install deps with big timeout
run: |
yarn install --network-timeout 600000
yarn config set httpTimeout 600000 && yarn install
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I usually use this for Yarn Berry pipeline installations for caching purposes:

      - name: Prepare for install
        run: |
          echo -e "logFilters:\n  - code: YN0013\n    level: discard\n" > ~/.yarnrc.yml
      - name: Get yarn cache directory path
        id: yarn-cache-dir-path
        run: |
          echo "::set-output name=dir::$(yarn config get cacheFolder)"
          echo -e "Yarn cache directory: $(yarn config get cacheFolder)"
      - name: Cache Yarn
        uses: actions/cache@v3
        id: yarn-cache # use this to check for `cache-hit` (`steps.yarn-cache.outputs.cache-hit != 'true'`)
        with:
          path: ${{ steps.yarn-cache-dir-path.outputs.dir }}
          key: yarn-cache-folder-${{ hashFiles('yarn.lock', '.yarnrc.yml') }}
          restore-keys: |
            yarn-cache-folder-
      - name: Install dependencies
        run: yarn install --immutable

"postcss": "7.0.36",
"web3": "1.6.0"
},
"packageManager": "yarn@3.0.2"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd recommend going with yarn@3.2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants