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

Standardize with module template #351

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

mcmire
Copy link

@mcmire mcmire commented Nov 15, 2023

  • Bring GitHub workflows up to date
    • Bump our actions
    • Bump gh-pages action, which now requires setting PUBLISH_DOCS_TOKEN
    • Send release candidate notifications to Slack, which requires SLACK_WEBHOOK_URL
  • Update dotfiles
  • Update .nvmrc so we're always using the current LTS
  • Add Yarn constraints
  • Update build script to publish ESM and CJS versions, and split tsconfig.json into development and build versions
  • Downgrade Yarn to 3.2.1 to match module template
  • Upgrade ESLint packages and fix lint violations
  • Upgrade other dev dependencies

@mcmire mcmire force-pushed the standardize-nov-2023 branch from 1307906 to f55d0cf Compare November 15, 2023 20:11
Copy link

socket-security bot commented Nov 15, 2023

Copy link

socket-security bot commented Nov 15, 2023

🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎

To accept the risk, merge this PR and you will not be notified again.

Issue Package Version Note Source
Invalid package.json @jest/globals 28.1.3
Invalid package.json jest-watcher 28.1.3
Network access esbuild 0.18.20
Shell access esbuild 0.18.20
New author react-is 18.2.0
New author global-prefix 1.0.2
New author resolve-dir 1.0.1
New author expand-tilde 2.0.2
New author cosmiconfig 7.1.0
No README @inquirer/core 0.0.15-alpha.0

Next steps

What is an invalid package.json?

Package has an invalid package.json and can cause installation problems if you try to use it.

Fix syntax errors in the invalid package.json and publish a new version with a valid package.json. Consumers can use npm overrides to force a version that does not have this problem if one exists.

What is network access?

This module accesses the network.

Packages should remove all network access that is functionally unnecessary. Consumers should audit network access to ensure legitimate use.

What is shell access?

This module accesses the system shell. Accessing the system shell increases the risk of executing arbitrary code.

Packages should avoid accessing the shell which can reduce portability, and make it easier for malicious shell access to be introduced.

What is new author?

A new npm collaborator published a version of the package for the first time. New collaborators are usually benign additions to a project, but do indicate a change to the security surface area of a package.

Scrutinize new collaborator additions to packages because they now have the ability to publish code into your dependency tree. Packages should avoid frequent or unnecessary additions or changes to publishing rights.

Why are READMEs important?

Package does not have a README. This may indicate a failed publish or a low quality package.

Add a README to to the package and publish a new version.

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore [email protected] bar@* or ignore all packages with @SocketSecurity ignore-all

"./encryption": {
"import": "./dist/encryption.mjs",
"require": "./dist/encryption.js",
"types": "./dist/types/encryption.d.ts"
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 confirm that these are created by running rm -rf dist && yarn build.

"sourceMap": true,
"moduleResolution": "node",
"noEmit": true,
"noErrorTruncation": true,
"strictNullChecks": true,
Copy link
Author

Choose a reason for hiding this comment

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

I've added a ticket to change this to strict: true here: #352

"strictNullChecks": true,
"target": "ES2017",
"typeRoots": ["./node_modules/@types"]
"target": "es2017"
Copy link
Author

Choose a reason for hiding this comment

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

Worth noting that in the module template we use a target of es2020. I've added a ticket here: #353

// ],

// Indicates which provider should be used to instrument code for coverage
coverageProvider: 'babel',
Copy link
Author

Choose a reason for hiding this comment

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

This is a new setting and may be the cause of the coverage threshold bumping.

"@metamask/eslint-config-nodejs": "^12.0.0",
"@metamask/eslint-config-typescript": "^12.0.0",
"@types/jest": "^28.1.6",
"@types/node": "^16",
Copy link
Author

Choose a reason for hiding this comment

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

@mcmire mcmire force-pushed the standardize-nov-2023 branch from f55d0cf to a7efba4 Compare November 15, 2023 20:44
- Bring GitHub workflows up to date
  - Bump our actions
  - Bump `gh-pages` action, which now requires setting
    PUBLISH_DOCS_TOKEN
  - Send release candidate notifications to Slack, which requires
    SLACK_WEBHOOK_URL
- Update dotfiles
- Update `.nvmrc` so we're always using the current LTS
- Add Yarn constraints
- Update build script to publish ESM and CJS versions, and split
  `tsconfig.json` into development and build versions
- Downgrade Yarn to 3.2.1 to match module template
- Upgrade ESLint packages and fix lint violations
- Upgrade other dev dependencies
@mcmire mcmire force-pushed the standardize-nov-2023 branch from a7efba4 to 55dbc77 Compare November 15, 2023 20:47
"@lavamoat/preinstall-always-fail",
"@metamask/auto-changelog",
"@types/*",
"@yarnpkg/*",
Copy link
Author

Choose a reason for hiding this comment

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

The addition of clipanion and @yarnpkg/* is different from the module template.

It seems that newer versions of depcheck (at the time of this writing, I was testing with 1.4.7) cause depcheck to mistakenly think that none of the packages being referred to in the .yarn/plugins directory are installed:

Missing dependencies
* @yarnpkg/core: ./.yarn/plugins/@yarnpkg/plugin-constraints.cjs
* @yarnpkg/cli: ./.yarn/plugins/@yarnpkg/plugin-constraints.cjs
* clipanion: ./.yarn/plugins/@yarnpkg/plugin-constraints.cjs
* @yarnpkg/fslib: ./.yarn/plugins/@yarnpkg/plugin-constraints.cjs

We probably need to add these lines to the module template.

@mcmire
Copy link
Author

mcmire commented Nov 15, 2023

Invalid package json, network access, shell access, new authors, no README ok.

@SocketSecurity ignore @inquirer/[email protected]
@SocketSecurity ignore [email protected]
@SocketSecurity ignore [email protected]
@SocketSecurity ignore [email protected]
@SocketSecurity ignore [email protected]
@SocketSecurity ignore [email protected]
@SocketSecurity ignore [email protected]
@SocketSecurity ignore @jest/[email protected]
@SocketSecurity ignore [email protected]

(edit: I'm not sure why these comment don't do anything for me, but whatever)

@legobeat

This comment was marked as off-topic.

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.

2 participants