`mix deps.get` when violating policy but already installed.

[maennchen]

• Add {:cowlib, "~> 2.17"} dependency
• HEX_POLICY="" mix deps.get - Installs, warning, exit 0
• HEX_POLICY="hexpm:erlefsecuritywg/strict" mix deps.get - Shows Policy Breakdown, Warning, exit 0

I believe we should have an exit 1 no matter if a dependency is locked already if it violates a policy.

[ericmj]

Maybe that should be setting on the policy? I have always been working under the assumption that mix.lock should be trusted.

If not your CI/CD could suddenly stop working because a package author retired a version you are using. This is something we generally want to avoid, see left_pad.

[maennchen]

@ericmj For cooldown I can follow that argument, but for retirement and vulnerabilities, it’s normal that those arise after an initial installation.

If not, what does the policy actually guarantee?

[ericmj]

The policy guarantees you don't move to a version that doesn't match your policy restrictions, currently it doesn't do anything if the package if the package is already locked.

I can see that you would want a hard blocker on security advisories, but for renamed/deprecated/invalid retirements I would want my CD to break because an author decided their package is deprecated.

I can think of two improvements:

1. Add a policy setting "Strict enforcement" for each restriction which also checks your lock
2. Add a flag to mix hex.audit --policy which check against the specified policy, mix hex.audit always checks against the lock.

[maennchen]

mix hex.audit currently always exits non-zero. So essentially the policy would make it more lax instead of more strict. But adding policies to it would be nice anyways.

Strict enforcement sounds more like something you would want to enable / disable in the client and not in the policy itself.

[josevalim]

There is a very practical concern here: if mix deps.get breaks all your builds because of the policy, we will see people immediately adding flags to remove the policy, because being unable to test or merge any other ongoing work until the violation is addressed is going to be counter-productive. For example, imagine the vulnerability is reported, someone on my team is already working on it, but now the designer cannot do any work because mix deps.get is failing on their machine (or worse, they remove the policy).

On the other hand, we do want to encourage people to run something like mix hex.audit or mix deps.get --enforce or whatever.

And I assume this only applies if you have a policy?

[ericmj]

What about HEX_POLICY_STRICT=1 along with mix hex.config setting?

[josevalim]

We could but which vectors do we want to address?

1. On local machines it should be strict?
2. On CI/etc we want mix hex.audit unless you want to explicitly fail a deployment midway?

[ericmj]

HEX_POLICY_STRICT is more for local users I think, on CI they can use mix hex.audit, but it of course requires that they call it which I would assume the majority doesn't?

The safer would be to default to strict with an option to disable it but if we are adding explicit overrides in #1194 then a full disable feels like it doesn't add a ton value. Explicit, specific overrides seems much safer and doesn't break for random users based on their HEX_POLICY_STRICT setting.

[josevalim]

I'd like to hear @maennchen's take, he is likely busy though, so I think we can wait a bit and revisit this altogether soon.

[maennchen]

I think policy violations should make mix deps.get fail even when the dependency is already locked.

The main reason is that advisories and retirements usually appear after a version has already been released and after users may already have it in their mix.lock. If policy enforcement only affects newly resolved dependencies, then it misses the most important advisory workflow: “this version used to be acceptable, but is now known to violate policy”.

Putting this only into mix hex.audit feels too weak to me, because not every project or CI setup runs it. mix deps.get is the command almost everyone runs, including in CI, so it is the better enforcement point.

I think the default behavior should be:

• mix deps.get fails with exit 1 if the lock contains a dependency that violates the configured policy.
• The error message should explain the violation and point to an explicit opt-out flag/env var for users who intentionally want to install anyway.
• Projects that already run mix hex.audit in CI could use that opt-out for deps.get, but the default should be safe.

That keeps the policy useful for advisories without completely blocking users who need to override it temporarily.

[josevalim]

So I think we should meet and discuss this carefully as I am worried about this halting development or pushing towards bad practices. For example, imagine there is a team with 10 devs. There is a security report in Phoenix. One dev will be responsible for fixing it, what will the other 9 devs do? They need to continue working and as such they will need to bypass it. If we don't guide them correctly, they may use a config or an env var to disable all future policies check. That's bad.

Or maybe they need to add a new dependency and that new dependency does have a new security issue, but because in order to even run the project at the moment requires them to bypass policies, now they accidentally allow both new and old vulnerable deps. So we need to be really careful to not push them towards worse practices. At the bare minimum we need locked deps and new deps to be controlled by separate flags.

And we should probably discuss all kinds of scenarios and what we want to stop. For example, the fact earmark was recently retired, should really indicate all development commands and CI fail? What happens when a project has a zero day vulnerability but you can't even upgrade it because all fixed versions violate your max age policy? It means you accept the dependency, regardless of the policy, but you still need to bypass the policy on every invocation, even after it is made part of your lock. So I worry that, by being too restrictive, we may actually teach people to carelessly disable the policy whenever something goes wrong.

I think there is at least one legit case where we want this to happen, which is on dependencies that can cause supply chain attacks, but perhaps those should be done via another mechanism, regardless of policies.