Starting with commit 0dbfa7b26e access would also
be allowed to ancestors of allowed paths. This is (ironically) a significant
purity regression, since several users of the purity checks will themselves
assume that arbitrary descent is allowed. For example, `builtins.readDir` and
`builtins.path` could now refer to the filesystem root, breaking purity
entirely in the latter case by allowing to read arbitrary files. Restore the
previous behaviour of only allowing access to explicitly allowed paths.
Change-Id: Ie64180733ab735da9873255e1ccbf95ba7c9161c
(cherry picked from commit 9d99a7c2cf)
signals could cause lockFile to return without having locked the file.
the garbage collector didn't check for this, and then hilarity ensued.
Change-Id: If86d33595e8bf5510d2b032139342261dc6e07c9
(cherry picked from commit d186064c3d)
lockFile is currently interruptible by signals like SIGCHLD. which
happen a lot in daemons. now imagine that daemon enabled automatic
garbage collection. observe that the local store does not actually
check whether its lock operations have succeeded ... get the idea?
Change-Id: Ibfd7ee786c4fee3add72d4456a7e95e73e09c73e
(cherry picked from commit 79f9c39e36)
propagating the "unlock" lock type through this high-level api is
nonsense. it doesn't make sense to treat locking and unlocking as
similar operations; unlocking *must* not not interruptible by our
checkInterrupt machinery or it will just leave locks lying around
for a potentially very long time. unlock operations should not be
taking long enough to *want* them interrupted anyway. even on nfs
this makes very little sense because nfs waits *uninterruptibly*.
Change-Id: I10d605c8fe6c651bee64466eee1f8e20251d39f4
(cherry picked from commit aa87c8aa93)
a65e9e5828 did not inform `tryEval` that
(as far as it's concerned) `state.debug` moved to `state.errors.debug`
and changed types. this resulted in the REPL erroneously coming up, that
REPL having a non-debug state, and segfaulting after that REPL exited.
it's probably good that `state.debug` isn't mutated by `--ignore-try`
anymore.
Change-Id: I1918e93edacd626452aa423fc2eb825080738835
Fixes: a65e9e5828 ("libexpr: extract eval error creation into new type")
Signed-off-by: Dusk Banks <me@bb010g.com>
(cherry picked from commit 6a583136b7)
Original-Author: picnoir <picnoir@alternativebit.fr>
Inspired from
https://github.com/NixOS/nix/pull/11922/commits/ced8d311a593fcf9c3823e4e118474ac132d8e60
and adapted for Lix needs.
TL;DR: The topological sort should ensure that it is possible to delete
the path iterated upon. Nonetheless, in some cases,
`invalidatePathChecked` can still throw `PathInUse`, the exception
bubbles up and cancel the garbage collection procedure, leaving the rest
of the paths untouched. This change ensure that the error is logged for
further investigation but doesn't prevent the GC to continue when it
can.
After code review, we decided to make it a `printInfo` to inform the
user about sudden "in use" dependencies during garbage collection and
let them re-run garbage collection if they care about this.
References: https://github.com/NixOS/nix/issues/11923
References: https://git.lix.systems/lix-project/lix/issues/621
Change-Id: I5606c9afd16b5faa747b713fde2dc24016990ba3
Signed-off-by: Raito Bezarius <raito@lix.systems>
(cherry picked from commit 6a41dae49a)
There's a race condition where awaitData could early-return for data
coming from a 404 response or similar and thus not rethrow the exception
that is forthcoming, and a related race during transfer setup (which
could retry a transfer *twice* per retry round).
This would then cause substitution failures like below since the exception
isn't caught in HttpBinaryCacheStore::getFile as intended, but instead
by an exception handler downstream of `drain()` which would error out
the entire operation.
Symptom:
» nix-build ./docs-service.nix -o "docs-service-result"
error: unable to download 'https://cache.nixos.org/7mr3fy8w66gi5inmf0jkkkl90lxy4jyg.narinfo': HTTP e
rror 404 ()
response body:
This is kind of a hack in how it is implemented: it assumes that you
can't intentionally be receiving a large unsuccessful response since in
such a case, `awaitData` will wait for finish() to be called to throw an
exception and will never escape until the download finishes, while
continuing to buffer the entire response into memory, which could be bad
if an error response had a large payload.
That said, nobody is sending Lix 1GiB of 404, so meh I guess, and this
is how it is seemingly intended to work. That was a design flaw of the
thing before any of the Lix team got our paws on it.
I tested this by adding _exit(0) inside the expected exception catch and
then running the offending command repeatedly to see if the symptom ever
appeared again, and it did not.
Needs cherry-pick to 2.92 and a 2.92.1 release once reviewed.
Fixes: https://git.lix.systems/lix-project/lix/issues/635
Co-Authored-By: lix@jade.fyi
Change-Id: If54f6eeaad60b5ca9d5b77d4d9232da1d295e7d1
(cherry picked from commit de58cd6e80)
This merge commit returns to the previous state prior to the release but leaves the tag in the branch history.
Release created with releng/create_release.xsh
Change-Id: I4e4650d96de82b46c35171b3a9fc1e3a6ca8e6a1
This commit remove a check for the log level in the `printVersion()`
function that was making `nix --version` behaving weirdly and
inconsistently compared to other `nix-*` commands.
The root cause is the following code in `lix/nix/main.cc` that change
the log level at runtime if nix is call interactively:
```cpp
if (isatty(STDERR_FILENO)) {
verbosity = lvlNotice;
} else {
verbosity = lvlInfo;
}
```
This should be removed, but since it has more implication it's gonna be
done in another CL.
Fix: https://git.lix.systems/lix-project/lix/issues/620
Change-Id: Id2c83c51d7ef799ee2f9b8dbdd2bfaeaf2df6188
We have obliterated the support across the build system for these
include paths anywhere, so the codemod is now redundant and can go.
Change-Id: I54082f39752c6aac6429e1c24026211adae8221a
This is a breaking change for non-migrated external clients. External
users always need to use <lix/libcmd/foo.hh> type include paths now.
This is as was always planned with the include rearrangement.
Change-Id: I269be91ff9f9cc94d5d3043cf3e0bdf8db1d8edb
This fixes a bug where flakes do not actually do purity path checks
correctly.
Tested-By: Jade Lovelace <lix@jade.fyi>
Change-Id: If7d131a8e73a5874fb15cfaa0dea3b8811ba35d2
this is more of a theoretical problem, but it does allow changing the
behavior of a flake depending on mutable machine state. it's unlikely
that this could be used to reliably do anything bad, but it does lead
to even more non-determinstic evaluation of (notionally) pure flakes.
Change-Id: I5bac7ed045046da08a36c764ab887bc9c7551542
This fixes segfaults with nix copy when there was an error processing
addMultipleToStore.
Running with ASAN/TSAN pointed at an use-after-free with threads from
the pool accessing the graph declared in processGraph after the function
was exiting and destructing the variables.
It turns out that if there is an error before pool.process() is called,
for example while we are still enqueueing tasks, then pool.process()
isn't called and threads are still left to run.
By creating the pool last we ensure that it is stopped first before
running other destructors even if an exception happens early.
fixes#618
Change-Id: I42a355f632aa0354df94c5d5d8cbe7ab5196c9a6
SourcePath only manipulates path names now. all accesses must go through
a checked path going forward to ensure we don't escape restriction lists
of pure and restricted evaluation. if a directory path is checked it can
safely be assumed that the directory itself is allowed, and its contents
will likewise be safe to access. it is tempting to assumed that contents
will also be fine, but that's only true if the content is not a symlink.
Change-Id: Icec3098d53fe9dce50997954ba958fe4f304d59b
like earlier, anything accessed during eval must be checked against the
list of path restrictions. this notably excludes `Pos::getSource` which
is run only from an unrestricted context (resolving line/column numbers
for expressions), but since positions require the parser to run and the
parser requires a checked input to produce positions this is not a leak
Change-Id: I337859e9c780590d4434885125a3ef70a11f6e93
the purpose of resolveExprPath is to produce a parser input path. parser
input paths must be validated against the path allow list so they do not
escape the restricted/pure eval sandbox. checking the input path and any
intermediate paths during resolving makes this a lot harder to do badly.
Change-Id: Ib31b5bca63fe26a5e08458a871cdc9f92f9b6a10
in pure mode it is entirely useless. in impure mode it's mostly useless
since the way in which it is used is either equivalent to not being run
at all, or is equivalent to turning the following lstat into a stat. we
add a stat method instead for all those who need final symlinks stat'd.
Change-Id: I801886d18eb34b26e62b4c05d53318c6421a69bf
all files are physical, so this doesn't have to be optional. if it only
returns a copy of a member it's not useful either, but performance cost
Change-Id: Ib2f935ae247d96418d55bc100e04765dc586528b
after startTransfer the transfer, and thus downloadState, is gone. asan
hasn't caught this, presumably because the access is in libc somewhere.
we don't even need access to the old state; the assertion is not useful
here and clearing the previous exception is invisible to the new round.
Change-Id: I32dc1a487b96cbaabcf5061c9b1f96dbc5aaae49
Nixpkgs issues / PRs:
* https://github.com/NixOS/nixpkgs/pull/368091
* https://github.com/NixOS/nixpkgs/issues/369366
This can be triggered with the postgresql_14 derivation from nixpkgs rev
19305d94dacca226ca048b78e6de00f599c65858
(/nix/store/bxp6g57limvwiga61vdlyvhy7i8rp6wd-postgresql-14.15.drv on
x86_64-linux): for reasons unknown to me, only the `man` and `lib` outputs
are cached on cache.nixos.org:
$ nix derivation show /nix/store/bxp6g57limvwiga61vdlyvhy7i8rp6wd-postgresql-14.15.drv | jq '.[].outputs.[].path' -r | xargs nix path-info --store https://cache.nixos.org
warning: The interpretation of store paths arguments ending in `.drv` recently changed. If this command is now failing try again with '/nix/store/bxp6g57limvwiga61vdlyvhy7i8rp6wd-postgresql-14.15.drv^*'
don't know how to build these paths:
/nix/store/m9vb40xxr6gckjzpfxnqcmjqsks2gx03-postgresql-14.15
/nix/store/nm1415wa53iawar9axwxy0an6ximhayn-postgresql-14.15-dev
/nix/store/v9vrvfhiw9gk8hj9895sb15fxvxnyylj-postgresql-14.15-debug
/nix/store/zi12g1p99g2173i8093ixbqkfh9ng87b-postgresql-14.15-doc
/nix/store/3i3fpz0xss9inampf51gp3pkx24ypxpj-postgresql-14.15-man
/nix/store/db8797h2cp4rm1cnsqrf87apkkxwwdff-postgresql-14.15-lib
error: path '/nix/store/m9vb40xxr6gckjzpfxnqcmjqsks2gx03-postgresql-14.15' does not exist in the store
Also, the derivation uses the `outputChecks` feature (and thus `__structuredAttrs`)
to make sure that e.g. the `out` output doesn't reference the `man`
output:
__structuredAttrs = true;
outputs = [ "out" "dev" "doc" "lib" "man" ];
outputChecks.out.disallowedReferences = [ "dev" "doc" "man" ];
With all that in place, the following error was hit on all CppNix / Lix
versions currently supported when trying to build the derivation above:
error: derivation contains an illegal reference specifier 'man'
The following happened here:
* The `man` & `lib` outputs were substituted at some point.
* When register outputs, the reference checks are made.
* `LocalDerivationGoal::checkOutputs` gets a map of all outputs that
were built and are NOT already registered in the store. In the example
above this means `out`, `dev`, `debug` and `doc`.
* `checkOutputs` tries to resolve the `man` output and fails to do so
because it's a store-path that's already registered and thus not part
of the map passed to `checkOutputs`.
Since the map passed to `checkOutputs` is used in various other places
that appear to assume that the paths aren't registered already, I didn't
write the already registered paths into it. Instead, I created a second
map that contains all already registered outputs and pass it as third
argument to `checkOutputs`. If the other lookups fail, this map will be
now checked before the "illegal reference specifier"-error is thrown.
This fixes the problem with `postgresql_14` for me.
Also wrote a small regression test that fails locally without the patch
in place.
Change-Id: Ieacca80c001fcfbebf6f5fe97e25c49d2724c3ff
in a daemon all calls to the logger can throw an Interrupted exception,
which so far has silently stopped the curl thread without notifying its
transfers and leaving them stuck as a result. ensuring that the loggers
can never throw Interrupted will have very unpleasant side-effects, and
throwing depending on context requires large amount of bookkeeping. for
now it is easiest to abort all transfers on Interrupted during cleanup.
the test for this is extremely sketchy because we want to hit a single,
very specifically chosen, loger call in TransferItem::finish(). the bug
was triggered by the `act.progress` further down from what we're aiming
for, but that one is much harder to select for than the debug log here.
fixes#613
Change-Id: Id72efa64dd30cbbf256d2ab2a328457a0b095c6a
the flake input bump broke these due to packages being moved. also fix
renamed options while we are here, before they inevitably break later.
Change-Id: If16bb0a221e63ae7394af6b8d904c5ecd8994e6f
After gathering more community feedback, more non-trivial use cases for
overriding `__findFile` emerged. Unlike the use case of Tvix mentioned
in #599, these can't easily be worked around by overriding `nixPath`
instead.
This is the second fixup/partial revert for
81d5f0a7d9.
See also #599.
Change-Id: I7ca75e1a2b196c0da341969c61f4c168b5f657f9
