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

add --ignore-scripts to npm build #34

Merged
merged 1 commit into from
Jun 5, 2023
Merged

add --ignore-scripts to npm build #34

merged 1 commit into from
Jun 5, 2023

Conversation

camallen
Copy link
Contributor

@camallen camallen commented Oct 5, 2022

Related to #33 (comment)

Run npm ci system with --ignore-scripts to secure the npm builds from package supply chain attacks via shell access in pre / post scripts. See https://docs.npmjs.com/cli/v7/commands/npm-install#ignore-scripts

use `--ignore-scripts` to secure the npm builds from package supply chain attacks via shell access in pre / post scripts. See https://docs.npmjs.com/cli/v7/commands/npm-install#ignore-scripts
@eatyourgreens
Copy link
Contributor

Sounds good to me. It's possible we have projects with dependencies that run post-install scripts, but I think the risks of running them outweigh the benefits. I'll double-check whether npm ci ignores them by default. I know that you have to explicitly disable them for npm install.

@camallen
Copy link
Contributor Author

camallen commented Oct 5, 2022

I'll double-check whether npm ci ignores them by default. I know that you have to explicitly disable them for npm install.

Looks like npm ci allows them to run by default https://docs.npmjs.com/cli/v8/commands/npm-ci#ignore-scripts

@eatyourgreens
Copy link
Contributor

On the other hand, we can also protect ourselves against malicious scripts by running installs with restricted permissions, which I think the Docker build already does? Maybe the best solution is a combination of restricted node user account and no pre- or post-install scripts.

@eatyourgreens
Copy link
Contributor

Oh, and I think the only package that might need to run a post-install script is new-relic, but we don't use that in any of the projects that use this workflow.

Copy link
Member

@zwolf zwolf left a comment

Choose a reason for hiding this comment

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

If nothing currently uses post-install scripts functionality, then this looks fine to me.

Noting that the FE team will have to keep this in mind as things are migrated or newly added. FEM won't be, due to its idiosyncratic deploys, but worth remembering.

@zwolf zwolf merged commit c0dbea0 into main Jun 5, 2023
@zwolf zwolf deleted the build-ignore-scripts branch June 5, 2023 16:46
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.

3 participants