Makes enableScripts: false the default

[arcanis]

What's the problem this PR addresses?

I was planning to wait until the next major to land this, but considering the regularity of package compromissions, I think we need to address it sooner than that.

How did you fix it?

Disables running scripts by default.

Checklist

• I have read the Contributing Guide.

• I have set the packages that need to be released for my changes to be effective.

• I will check that all automated PR checks pass before the PR gets reviewed.

[clemyan]

I think adding a migration that sets enableScripts: true on existing projects will make this change non-breaking?

[arcanis]

I think adding a migration that sets enableScripts: true on existing projects will make this change non-breaking?

I'm a bit thorned. On one hand yes it'd make the change non-breaking. On the other I feel like this change is important enough that it's worth being breaking rather than hope people notice it in their changelogs.

The error messages should hopefully be clear enough that any failed CI can quickly surface what changed and how to address it - and since our ecosystem tends to lock package manager versions anyway I think it should be ok.

[clemyan]

I'm a bit thorned.

Same here. I feel like however we slice this ends up being a tradeoff between correctness and security

But I do think that local file changes that shows up on git status/git diff is a much better place to surface this information than the changelog, which users may or may not read

And, as far as I understand, making a migration like this bumps the lockfile version, so yarn install won't silently pass on CI anyway. The committer must explicitly commit or discard the config change and commit the new lockfile.

[CptnFizzbin]

But I do think that local file changes that shows up on git status/git diff is a much better place to surface this information than the changelog, which users may or may not read

The yarnrc.yml diff was how I found out about this new flag/default, and was immediately curious.

[GNUGradyn]

Interesting idea, on one hand there have been alot of supply chain attacks on NPM recently and people probably expect simply installing a package to be a safe operation, and since most modern setups lock the yarn version this shouldn't be breaking. But on the other hand this would pretty much completely break installing packages from git out of the box and the supply chain attack would still succeed as soon as the library is actually used. Is there some sort of warning like WARNING: Package example123 wants to run a build script, but scripts are disabled in this project for security. This package will likely not work properly when you install the package so people will know why some packages are suddenly dead in the latest version?

[arcanis]

Git repositories use a separate mechanism: #7091

[GNUGradyn]

Ah cool. That makes sense. Presumably this is trying to prevent issues where the wrong package name is specified which is not really an issue when you are providing a specific git URL so the risk profile is different there.

[arcanis]

My concern was that a threat actor could use a often-allowlisted package name pointing to a bogus repo:

{
  "dependencies": {
    "sharp": "git@github.com:malicious/pwnd"
  }
}

Hence requiring explicit source aknowledgement.

[onigoetz]

I welcome this change but am puzzled at how to use this feature.

I would like to enable postinstall only for some packages and disallow for all others.
How should I approach doing this ?

The documentation mentions dependenciesMeta but doesn't provide an example.

I was hoping to find something similar to https://pnpm.io/settings#allowbuilds

[arcanis]

It's documented here: https://yarnpkg.com/configuration/manifest#dependenciesMeta.built

In a future version it'll be moved to the yarnrc.yml.

[wickkidd]

I realize I'm late to the party, but this is definitely a breaking change. Now I have to go modify my renovate centralized configs to account for this anomaly and go manually update over 50 repos. This makes semver meaningless. Not trying to be the squeaky wheel, but as a long time yarn fan this is discouraging.

[skeet70]

Agreed with @wickkidd that this was a breaking change, and breaks many of our builds. At the very least this should have been called out as an emergency breaking change in the changelog and the repo changelog (not only in the github release), if deemed necessary for security to break semver and put it into a minor version.

[arcanis]

Can you share details about why the mechanisms we implemented in this PR to avoid it being a breaking change didn't work?

Yarn should auto-apply the previous settings in your yarnrc when you update an existing project. Did it fail to do so?

[naugtur]

Congrats!

defaulting approvedGitRepositories: [] when a .yarnrc.yml is created would be a great next step

[arcanis]

Yes, it will be in the next major release

[arcanis]

Actually I'm not sure I understand, can you clarify just in case?

New projects are already configured to rejects git dependencies by default, with repos having to be explicitly allowed. It's only existing projects that have the ** default, got backward compatibility.

[naugtur]

I didn't know that. Gotta rethink my plan to put an empty array in there by default when setting up @lavamoat/allow-scripts then.

Where can I read up on the specifics of the behavior?
How is "existing" project being determined?

[arcanis]

Can you share details about why the mechanisms we implemented in this PR to avoid it being a breaking change didn't work?

Please manifest yourself if you had problems during the previous migration - I'm strongly considering following that same process again to make npmMinimalAgeGate: 1d the default on new projects, see #7135 (existing ones will keep using the same value, which will be hardcoded in their .yarnrc.yml during migration).

How is "existing" project being determined?

All lockfiles have a lockfile version. If your version is lower than the one used by your Yarn binary we apply the relevant migration patches to your config to maintain your configuration even as our defaults become stricter.

The only case I can imagine for that not working would be if a project somehow didn't have a lockfile checked-in, but we've always been very clear that lockfiles must always be checked-in.

[naugtur]

You did the right thing. If making something more secure becomes a breaking change in a niche configuration it's still totally worth it.

One outcome I don't like is users on existing projects hear the announcement of a new default and are happy they're being protected but they got migrated into a less secure option. I'd consider giving them a nudge when the migration happens. Drop a big multiline comment in the config that's hard to miss in PR review or something.

---

Mandatory XKCD https://xkcd.com/1172/

[leo60228]

All lockfiles have a lockfile version. If your version is lower than the one used by your Yarn binary we apply the relevant migration patches to your config to maintain your configuration even as our defaults become stricter.

Weren't yarn-zpm lockfiles already using version 9?

[arcanis]

zpm is still experimental, so its lockfile version isn't final yet