-
-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
python3Packages.matrix-nio: permit insecure Olm during check phase #336694
python3Packages.matrix-nio: permit insecure Olm during check phase #336694
Conversation
Olm has a known vulnerability (NixOS#334638) but is only an optional dependency of nio, so in theory nio should by default be unaffected. nio’s tests, however, cover its full suite of extra features, so Olm is still evaluated as a dependency of the check phase. Since the check phase doesn’t process user data or access the network this vulnerability isn’t relevant and can be ignored, allowing nio to evaluate and ultimately be run without Olm.
2b520d6
to
9a3501b
Compare
We could also just do one of these:
I think I’d prefer any one of them, since they’re simpler than the construction here. |
Can we handle #336052 here too since it’s the exact same issue? |
Also I think instead of Anyway I lean toward just disabling the checks. |
Thanks, how’s that for a simpler implementation? Please also feel free to just edit yourself if that’s easier. |
Can we please stop such hick and just ignore the effected tests? |
Yes. |
Description of changes
Olm has a known vulnerability but is only an optional dependency of nio, so in theory nio should by default be unaffected. nio’s tests, however, cover its full suite of extra features, so Olm is still evaluated as a dependency of the check phase. Since the check phase doesn’t process user data or access the network this vulnerability isn’t relevant and can be ignored, allowing nio to evaluate and ultimately be run without Olm.
I’m not confident in this implementation. Is there a tidier way to do it? Should the permission be narrowed to specifically this vulnerability? I haven’t found any precedent for this and wonder if something like a
checkPhasePermittedInsecurePackages
would be warranted.Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
git cherry-pick NixOS/#336651 && nixpkgs-review rev HEAD
../result/bin/
)Add a 👍 reaction to pull requests you find important.