Previously, system call filtering (to prevent builders from storing files with
setuid/setgid permission bits or extended attributes) was performed using a
blocklist. While this looks simple at first, it actually carries significant
security and maintainability risks: after all, the kernel may add new syscalls
to achieve the same functionality one is trying to block, and it can even be
hard to actually add the syscall to the blocklist when building against a C
library that doesn't know about it yet. For a recent demonstration of this
happening in practice to Nix, see the introduction of fchmodat2 [0] [1].
The allowlist approach does not share the same drawback. While it does require
a rather large list of harmless syscalls to be maintained in the codebase,
failing to update this list (and roll out the update to all users) in time has
rather benign effects; at worst, very recent programs that already rely on new
syscalls will fail with an error the same way they would on a slightly older
kernel that doesn't support them yet. Most importantly, no unintended new ways
of performing dangerous operations will be silently allowed.
Another possible drawback is reduced system call performance due to the larger
filter created by the allowlist requiring more computation [2]. However, this
issue has not convincingly been demonstrated yet in practice, for example in
systemd or various browsers.
This commit tries to keep the behavior as close to unchanged as possible. Only
newer syscalls that are not supported by glibc 2.38 (as found in NixOS 23.11)
are blocked. Since this includes fchmodat2, the compatibility code added for
handling this syscall can be removed too.
[0] https://github.com/NixOS/nixpkgs/issues/300635
[1] https://github.com/NixOS/nix/issues/10424
[2] https://github.com/flatpak/flatpak/pull/4462#issuecomment-1061690607
Change-Id: I541be3ea9b249bcceddfed6a5a13ac10b11e16ad
In f047e4357b, I missed the behavior that if
building without a dedicated build user (i.e. in single-user setups), seccomp
setup failures are silently ignored. This was introduced without explanation 7
years ago (ff6becafa8). Hopefully the only
use-case nowadays is causing spurious test suite successes when messing up the
seccomp filter during development. Let's try removing it.
Change-Id: Ibe51416d9c7a6dd635c2282990224861adf1ceab
The `allow-flake-configuration` option allows the user to control whether to
accept configuration options supplied by flakes. Unfortunately, setting this
to false really meant "ask each time" (with an option to remember the choice
for each specific option encountered). Let no mean no, and introduce (and
default to) a separate value for the "ask each time" behaviour.
Co-Authored-By: Jade Lovelace <lix@jade.fyi>
Change-Id: I7ccd67a95bfc92cffc1ebdc972d243f5191cc1b4
this most notably affects `nix eval`: if there is no progress bar to be
shown and no activities going on we should not print anything at all. a
progress bar with no activities would print a bunch of terminal escapes
*and a space*, which is not helpful in simple cases like nix eval -E 1.
notably this does *not* affect nix eval called on non-terminal outputs,
but it is slightly confusing nevertheless (and not difficult to avoid).
fixes lix-project/lix#424
Change-Id: Iee793c79ba5a485d6606e0d292ed2eae6dfb7216
MAKEFLAGS hasn't been relevant since we switched off the Make
buildsystem and using the clang environment by default gives you clangd
by default which most developers will want.
Change-Id: I9c11d0613577047e6c908f049c1ffaca5fb5ff67
this gives about 20% performance improvements on pure parsing. obviously
it will be less on full eval, but depending on how much parsing is to be
done (e.g. including hackage-packages.nix or not) it's more like 4%-10%.
this has been tested (with thousands of core hours of fuzzing) to ensure
that the ASTs produced by the new parser are exactly the same as the old
one would have produced. error messages will change (sometimes by a lot)
and are not yet perfect, but we would rather leave this as is for later.
test results for running only the parser (excluding the variable binding
code) in a tight loop with inputs and parameters as given are promising:
- 40% faster on lix's package.nix at 10000 iterations
- 1.3% faster on nixpkgs all-packages.nix at 1000 iterations
- equivalent on all of nixpkgs concatenated at 100 iterations
(excluding invalid files, each file surrounded with parens)
more realistic benchmarks are somewhere in between the extremes, parsing
once again getting the largest uplift. other realistic workloads improve
by a few percentage points as well, notably system builds are 4% faster.
Benchmarks summary (from ./bench/summarize.jq bench/bench-*.json)
old/bin/nix --extra-experimental-features 'nix-command flakes' eval -f bench/nixpkgs/pkgs/development/haskell-modules/hackage-packages.nix
mean: 0.408s ± 0.025s
user: 0.355s | system: 0.033s
median: 0.389s
range: 0.388s ... 0.442s
relative: 1
new/bin/nix --extra-experimental-features 'nix-command flakes' eval -f bench/nixpkgs/pkgs/development/haskell-modules/hackage-packages.nix
mean: 0.332s ± 0.024s
user: 0.279s | system: 0.033s
median: 0.314s
range: 0.313s ... 0.361s
relative: 0.814
---
old/bin/nix --extra-experimental-features 'nix-command flakes' eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'
mean: 6.133s ± 0.022s
user: 5.395s | system: 0.437s
median: 6.128s
range: 6.099s ... 6.183s
relative: 1
new/bin/nix --extra-experimental-features 'nix-command flakes' eval --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'
mean: 5.925s ± 0.025s
user: 5.176s | system: 0.456s
median: 5.934s
range: 5.861s ... 5.943s
relative: 0.966
---
GC_INITIAL_HEAP_SIZE=10g old/bin/nix eval --extra-experimental-features 'nix-command flakes' --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'
mean: 4.503s ± 0.027s
user: 3.731s | system: 0.547s
median: 4.499s
range: 4.478s ... 4.541s
relative: 1
GC_INITIAL_HEAP_SIZE=10g new/bin/nix eval --extra-experimental-features 'nix-command flakes' --raw --impure --expr 'with import <nixpkgs/nixos> {}; system'
mean: 4.285s ± 0.031s
user: 3.504s | system: 0.571s
median: 4.281s
range: 4.221s ... 4.328s
relative: 0.951
---
old/bin/nix --extra-experimental-features 'nix-command flakes' search --no-eval-cache github:nixos/nixpkgs/e1fa12d4f6c6fe19ccb59cac54b5b3f25e160870 hello
mean: 16.475s ± 0.07s
user: 14.088s | system: 1.572s
median: 16.495s
range: 16.351s ... 16.536s
relative: 1
new/bin/nix --extra-experimental-features 'nix-command flakes' search --no-eval-cache github:nixos/nixpkgs/e1fa12d4f6c6fe19ccb59cac54b5b3f25e160870 hello
mean: 15.973s ± 0.013s
user: 13.558s | system: 1.615s
median: 15.973s
range: 15.946s ... 15.99s
relative: 0.97
---
Change-Id: Ie66ec2d045dec964632c6541e25f8f0797319ee2
These are totally available and you can just turn them on, but they have
very bad dependency tracking and thus bloat incremental change times,
which is not really ok.
Change-Id: Iaa63ed18a789e74fcb757248cd24c3b194afcc80
This reverts commit 35eec921af.
Reason for revert: Regressed nix-eval-jobs, and it appears to be this change is buggy/missing a case. It just needs another pass.
Code causing the problem in n-e-j, when invoked with `nix-eval-jobs --flake '.#hydraJobs'`:
```
n-e-j/tests/assets » ../../build/src/nix-eval-jobs --meta --workers 1 --flake .#hydraJobs
warning: unknown setting 'trusted-users'
warning: `--gc-roots-dir' not specified
error: unsupported Git input attribute 'dir'
error: worker error: error: unsupported Git input attribute 'dir'
```
```
nix::Value *vRoot = [&]() {
if (args.flake) {
auto [flakeRef, fragment, outputSpec] =
nix::parseFlakeRefWithFragmentAndExtendedOutputsSpec(
args.releaseExpr, nix::absPath("."));
nix::InstallableFlake flake{
{}, state, std::move(flakeRef), fragment, outputSpec,
{}, {}, args.lockFlags};
return flake.toValue(*state).first;
} else {
return releaseExprTopLevelValue(*state, autoArgs, args);
}
}();
```
Inspecting the program behaviour reveals that `dir` was in fact set in the URL going into the fetcher. This is in turn because unlike in the case changed in this commit, it was not erased before handing it to libfetchers, which is probably just a mistake.
```
(rr) up
3 0x00007ffff60262ae in nix::fetchers::Input::fromURL (url=..., requireTree=requireTree@entry=true) at src/libfetchers/fetchers.cc:39
warning: Source file is more recent than executable.
39 auto res = inputScheme->inputFromURL(url, requireTree);
(rr) p url
$1 = (const nix::ParsedURL &) @0x7fffdc874190: {url = "git+file:///home/jade/lix/nix-eval-jobs",
base = "git+file:///home/jade/lix/nix-eval-jobs", scheme = "git+file", authority = std::optional<std::string> = {[contained value] = ""},
path = "/home/jade/lix/nix-eval-jobs", query = std::map with 1 element = {["dir"] = "tests/assets"}, fragment = ""}
(rr) up
4 0x00007ffff789d904 in nix::parseFlakeRefWithFragment (url=".#hydraJobs", baseDir=std::optional<std::string> = {...},
allowMissing=allowMissing@entry=false, isFlake=isFlake@entry=true) at src/libexpr/flake/flakeref.cc:179
warning: Source file is more recent than executable.
179 FlakeRef(Input::fromURL(parsedURL, isFlake), getOr(parsedURL.query, "dir", "")),
(rr) p parsedURL
$2 = {url = "git+file:///home/jade/lix/nix-eval-jobs", base = "git+file:///home/jade/lix/nix-eval-jobs", scheme = "git+file",
authority = std::optional<std::string> = {[contained value] = ""}, path = "/home/jade/lix/nix-eval-jobs", query = std::map with 1 element = {
["dir"] = "tests/assets"}, fragment = ""}
(rr) list
174
175 if (pathExists(flakeRoot + "/.git/shallow"))
176 parsedURL.query.insert_or_assign("shallow", "1");
177
178 return std::make_pair(
179 FlakeRef(Input::fromURL(parsedURL, isFlake), getOr(parsedURL.query, "dir", "")),
180 fragment);
181 }
```
Change-Id: Ib55a882eaeb3e59228857761dc1e3b2e366b0f5e
On operating systems where /bin/sh is not Bash, some scripts are invalid
because of bashisms, and building Lix fails with errors like this:
`render-manpage.sh: 3: set: Illegal option -o pipefail`
This modifies all scripts that use a `/bin/sh` shebang to `/usr/bin/env
bash`, including currently POSIX-compliant ones, to prevent any future
confusion.
Change-Id: Ia074cc6db42d40fc59a63726f6194ea0149ea5e0
I was working on nix-eval-jobs with a dev shell with some shenanigans to
run against a locally built Lix and it was getting really annoying when
`nix develop ../lix#` was messing up my other git repo's hooks.
This is a fix via blunt force, but it is at least obvious how it works.
Change-Id: Ia29eeb5be57ab6a2c88451c00ea18a51e4dfe65e
This is a squash of upstream PRs #10303, #10312 and #10883.
fix: Treat empty TMPDIR as unset
Fixes an instance of
nix: src/libutil/util.cc:139: nix::Path nix::canonPath(PathView, bool): Assertion `path != ""' failed.
... which I've been getting in one of my shells for some reason.
I have yet to find out why TMPDIR was empty, but it's no reason for
Nix to break.
(cherry picked from commit c3fb2aa1f9d1fa756dac38d3588c836c5a5395dc)
fix: Treat empty XDG_RUNTIME_DIR as unset
See preceding commit. Not observed in the wild, but is sensible
and consistent with TMPDIR behavior.
(cherry picked from commit b9e7f5aa2df3f0e223f5c44b8089cbf9b81be691)
local-derivation-goal.cc: Reuse defaultTempDir()
(cherry picked from commit fd31945742710984de22805ee8d97fbd83c3f8eb)
fix: remove usage of XDG_RUNTIME_DIR for TMP
(cherry picked from commit 1363f51bcb24ab9948b7b5093490a009947f7453)
tests/functional: Add count()
(cherry picked from commit 6221770c9de4d28137206bdcd1a67eea12e1e499)
Remove uncalled for message
(cherry picked from commit b1fe388d33530f0157dcf9f461348b61eda13228)
Add build-dir setting
(cherry picked from commit 8b16cced18925aa612049d08d5e78eccbf0530e4)
Change-Id: Ic7b75ff0b6a3b19e50a4ac8ff2d70f15c683c16a
The stdenv phases don’t actually do anything (at least not anymore),
and our justfile doesn’t behave the same as our docs.
This patch removes the stdenv phases from the docs, documents our
usage of just, and makes `just setup` heed `$mesonFlags`.
Fixes#413.
Fixes#414.
Change-Id: Ieb0b2a8ae420526238b5f9a73d7849ec6919995d
Following the latest hacking.md currently fails because of a missing
include in upstream editline. This patch fixes the build by adding
that missing include.
Fixes#410.
Change-Id: Iefd4cb687ed3da71ccda9fe9624f38e6ef4623e5
this was only used in one place, and that place has been rewritten to
use a temporary file instead. keeping this around is not very helpful
at this time, and in any case we'd be better off rewriting subprocess
handling in rust where we not only have a much safer library for such
things but also async frameworks necessary for this easily available.
Change-Id: I6f8641b756857c84ae2602cdf41f74ee7a1fda02
we want to remove runProgram's ability to provide stdin to a process
because the concurrency issues of handling both stdin and stdout are
much more pronounced once runProgram returns not is collected output
but a source. this is possible in the current c++ framework, however
it isn't necessary in almost all cases (as demonstrated by only this
single user existing) and in much better handled with a proper async
concurrency model that lets the caller handle both at the same time.
Change-Id: I29da1e1ad898d45e2e33a7320b246d5003e7712b
This is primarily for readability, but iwrc chaining std::string's
operator+ is also pretty scuffed performance-wise, and this was doing a
lot of operator+ chainging.
Change-Id: I9f25235df153eb2bbb491f1ff7ebbeed9a8ec985
copy-constructing or assigning from pid_t can easily lead to duplicate
Pid instances for the same process if a pid_t was used carelessly, and
Pid itself was copy-constructible. both could cause surprising results
such as killing processes twice (which could become very problemantic,
but luckily modern systems don't reuse PIDs all that quickly), or more
than one piece of the code believing it owns a process when neither do
Change-Id: Ifea7445f84200b34c1a1d0acc2cdffe0f01e20c6
this is only used in one place, and only to set a nicer error message on
EndOfFile. the only caller that actually *catches* this exception should
provide an error message in that catch block rather than forcing support
for setting error message so deep into the stack. copyStorePath is never
called outside of PathSubstitutionGoal anyway, which catches everything.
Change-Id: Ifbae8706d781c388737706faf4c8a8b7917ca278
LocalDerivationGoal includes a large number of low-level sandboxing
primitives for Darwin and Linux, intermingled with ifdefs.
Start creating platform-specific classes to make it easier to add new
platforms and review platform-specific code.
This change only creates support infrastructure and moves two function,
more functions will be moved in future changes.
Change-Id: I9fc29fa2a7345107d4fc96c46fa90b4eabf6bb89
This comes quite often when the available job slots on all remote
builders are exhausted and this is pretty spammy.
This isn't really an issue, but expected behavior.
A better way to display this is a nom-like approach where all scheduled
builds are shown in a tree and pending builds are being marked as such
IMHO.
Change-Id: I6bc14e6054f84e3eb0768127b490e263d8cdcf89
The original idea was to fix lix#174, but for a user friendly solution,
I figured that we'd need more consistency:
* Invalid query params will cause an error, just like invalid
attributes. This has the following two consequences:
* The `?dir=`-param from flakes will be removed before the URL to be
fetched is passed to libfetchers.
* The tarball fetcher doesn't allow URLs with custom query params
anymore. I think this was questionable anyways given that an
arbitrary set of query params was silently removed from the URL you
wanted to fetch. The correct way is to use an attribute-set
with a key `url` that contains the tarball URL to fetch.
* Same for the git & mercurial fetchers: in that case it doesn't even
matter though: both fetchers added unused query params to the URL
that's passed from the input scheme to the fetcher (`url2` in the code).
It turns out that this was never used since the query parameters were
erased again in `getActualUrl`.
* Validation happens for both attributes and URLs. Previously, a lot of
fetchers validated e.g. refs/revs only when specified in a URL and
the validity of attribute names only in `inputFromAttrs`.
Now, all the validation is done in `inputFromAttrs` and `inputFromURL`
constructs attributes that will be passed to `inputFromAttrs`.
* Accept all attributes as URL query parameters. That also includes
lesser used ones such as `narHash`.
And "output" attributes like `lastModified`: these could be declared
already when declaring inputs as attribute rather than URL. Now the
behavior is at least consistent.
Personally, I think we should differentiate in the future between
"fetched input" (basically the attr-set that ends up in the lock-file)
and "unfetched input" earlier: both inputFrom{Attrs,URL} entrypoints
are probably OK for unfetched inputs, but for locked/fetched inputs
a custom entrypoint should be used. Then, the current entrypoints
wouldn't have to allow these attributes anymore.
Change-Id: I1be1992249f7af8287cfc37891ab505ddaa2e8cd
Add the log-formats `multiline` and `multiline-with-logs` which offer
multiple current active building status lines.
Change-Id: Idd8afe62f8591b5d8b70e258c5cefa09be4cab03
alois31
jade
eldritch horrors
Lunaphied
Robert Hensing
Delan Azabani
Qyriad
Maximilian Bosch
Artemis Tosini
kloenk