search

Mic92 / nixpkgs-review

Review pull-requests on https://github.com/NixOS/nixpkgs
MIT License
410 stars 66 forks source link

Sandbox = relaxed on NixOS? #205

Open Atemu opened 3 years ago

Atemu commented 3 years ago

When running nixpkgs-review today, I noticed that the nix CLI it runs in the process includes --option build-use-sandbox relaxed.

I'm not sure whether it does anything because I haven't set myself as a trusted user but it's safe to say that this is rather unexpected.

https://github.com/Mic92/nixpkgs-review/pull/161 was supposed to disable it on Darwin but it actually adds those options on Linux. Was that intended @SuperSandro2000?

SuperSandro2000 commented 3 years ago

Before that PR it was set on all platforms and after it the option is only set on Linux because it does not work reliable on Darwin.

Atemu commented 3 years ago

I see.

Why would nixpkgs-review want to weaken sandboxing on Linux though?

Mic92 commented 3 years ago

Right now it does not really change anything practically. We don't have the sandbox set to relaxed for any Linux nix package. There only 1-2 darwin builds in nixpkgs using it.

Atemu commented 3 years ago

Fixed-output drvs run without sandboxing when relaxed. This isn't really a concern for r13y since their output is indeed fixed but might break some and could be a potential security issue.

Why does the sandbox need to be set different from the environment in the first place though?

SuperSandro2000 commented 3 years ago

The sandbox on darwin is not ready yet to be always enabled.

Atemu commented 3 years ago

That's why it's disabled by default; in the environment. My question is why nixpkgs-review needs to change anything about that setting.

Mic92 commented 3 years ago

Fixed-output drvs run without sandboxing when relaxed. This isn't really a concern for r13y since their output is indeed fixed but might break some and could be a potential security issue.

Why does the sandbox need to be set different from the environment in the first place though?

afaik only if __noChroot is set https://github.com/NixOS/nix/blob/69eb65403ad3e877300a7c0c96411d9d9e27dc69/src/libstore/build/local-derivation-goal.cc#L416 which affects only two packages in nixpkgs.

Mic92 commented 3 years ago

That's why it's disabled by default; in the environment. My question is why nixpkgs-review needs to change anything about that setting.

It should be consistent for everyone. If someone happens to configured it's nix to use no sandbox it should be obvious. I am fine with enabling full sandbox on Linux again and keep it disabled on macOS. We only had the relaxed sandbox because someone wanted it on macOS in the first place.

Mic92 commented 3 years ago

Also please don't use nixpkgs-review on untrusted input. In a fixed input derivations it is possible to access the local network or do some other malicious stuff. Everything gets sourced in a nix-shell that also allows code execution without a sandbox.