Skip to content

add --ignore-scripts to npm build - #34

Merged
zwolf merged 1 commit into
mainfrom
build-ignore-scripts
Jun 5, 2023
Merged

add --ignore-scripts to npm build#34
zwolf merged 1 commit into
mainfrom
build-ignore-scripts

Conversation

@camallen

@camallen camallen commented Oct 5, 2022

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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

camallen commented Oct 5, 2022

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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
Copy Markdown
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.

@zwolf zwolf left a comment

Copy link
Copy Markdown
Member

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