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
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
Attribute names containing special characters like @ or . need to be
quoted, so we need to do our own tokenization of the command line for
completion, and quote the attribute names when we provide the completion.
Fixes: https://git.lix.systems/lix-project/lix/issues/450
Change-Id: I55a30dd272880c89445d9ded49b3f2c90cb19326
This is a useful piece of functionality to being able to eat URL
hyperlinks, for instance, which is a bug that Lix has while dealing with
terminal output today.
Change-Id: I77b2de107b2525cad7ea5dea28bfba2cc78b9e6d
This commit makes Lix include the summarized content of the value being
indexed when it is bad.
lix/lix2 » nix eval --expr '{x.y = 2;}' 'x.y.z'
error: the value being indexed in the selection path 'x.y.z' at 'x.y' should be a set but is an integer: 2
lix/lix2 » nix eval --expr '{x.y = { a = 3; };}' 'x.y.z'
error: attribute 'z' in selection path 'x.y.z' not found inside path 'x.y', whose contents are: { a = 3; }
Did you mean a?
lix/lix2 » nix eval --expr '{x.y = { a = 3; };}' 'x.y.1'
error: the expression selected by the selection path 'x.y.1' should be a list but is a set: { a = 3; }
Change-Id: I3202aba0e437e00b4c6d3ee287a2d9a7c6892dbf
I want this for being able to write reasonable expect-test style tests
for oneliners. We will still probably want something like insta for more
complicated test cases where you actually *want* the output in a
different file, but for now this will do.
cc: https://git.lix.systems/lix-project/lix/issues/595
Change-Id: I6ddc42963cc49177762cfca206fe9a9efe1ae65d
I don't know what the heck the xonsh module is doing but its obviously
crimes so it has to go. It was never intended to be running here anyway.
Fixes: https://git.lix.systems/lix-project/lix/issues/593
Change-Id: I1877698469392f85884945aaa60987c68c4e0ebc
Lix requires a non-antiquated macOS SDK, and 24.11 does not yet have a
non-antiquated one as default.
Fixes: https://git.lix.systems/lix-project/lix/issues/588
Change-Id: Iad19c06d7fefe3a736cdcb39ced185e52dcfcbb8
nixpkgs 24.11 changes how we access xonsh yet again
and updates clang.
Unfortunately, clang 18 produces significantly more
warnings on existing code that is challenging to fix.
Make sure that doesn't error when we're running
`-Werror` builds.
n.b. I had to change the "SSL certificate problem: self-signed
certificate" to the old error prior to the improved libcurl errors,
since what is presumably a difference in which TLS library is used has
cropped up between releases? Either way the curl error buffer is empty.
Seems like we aggressively cannot do anything about this.
Change-Id: If0141a46a8b445a0e7d6f86f939e8c8e03569bf5
It being overridable was an intended feature with good use cases, and
should not have been removed. However, this feature is generally in a
bad state and needs revisiting in the future.
Fixup for 81d5f0a7d9Fixes#599
Change-Id: I2d93e012caa65aa795bce3a71d8e56d7052ef9df
* changes:
libexpr: Rework error messages on ExprSelct::eval
libexpr: Track position information in attrpaths
libexpr: Assert: Don't print assertion in error message
libexpr: Split ExprAttrs and ExprLet
Calls to `show` have been removed. To counter the loss of information,
the error positions have been improved and now correctly point to the
current selector instead of the entire select expression.
Change-Id: I4771fe874af1ac15828a9863550cd4369a8f0e94
This is a pretty small change because the parser already has all
necessary information (needed for parse errors), now we want to keep it
to also provide better eval errors.
Change-Id: Ifc7a9516b9b0c8d9698f1899a6912ae91f6696ab
The `show` functionality needs to be removed because it is deeply
flawed, and given that we already print position information in the
error message (which probably wasn't always the case in the past) the
assertion printing is redundant anyways.
Change-Id: I1f5e05ab73aaa0ec92994c2211463260fd374898
ExprLet was previously inheriting from ExprAttrs for the data, while
ignoring all
set-specific operations on it. The set specific code has now been split
off so that
let doesn't inherit it anymore:
- ExprAttrs (not an Expr), containing the attributes and the related
logic
- ExprLet : Expr, ExprAttrs
- ExprSet : Expr, ExprAttrs
Co-authored-by: eldritch horrors <pennae@lix.systems>
Change-Id: I63f2fbcd1e790b3cffb56eec1e7565ee3cdbf964
Mostly these are bugprone-unused-local-non-trivial-variable.
Also fix instances of:
- bugprone-optional-value-conversion
- bugprone-inc-dec-in-conditions (please check this loop is correct, it
is the only non trivial code change in here)
- bugprone-unused-return-value (well, by fixing the lint config)
There are three notable changes relating to undefined vars:
- openLogFile ignoring the result. This is because openLogFile does a
whole bunch of mutation of member variables
- hiliteMatches: i am guessing this is because showing the derivation
name was unhelpful and it just got changed
- canonPath in NarAccessor: canonPath inside of a thing that is supposed
to be vfs based cannot possibly be correct, so let's delete it given
it is unused.
Fixes: https://git.lix.systems/lix-project/lix/issues/584
Change-Id: I887adc9ff28b61f726dcfed197e6796b414c2fcf
these must be tampered with before the evaluator is created, *never*
after. doing it any other way leads to interesting things like #596.
fixes#596
Change-Id: Iea253ccce44b94b1243833837a3df93c795967d9
Calling a custom type size_t is incredibly sneaky and makes all the code
around this extremely context dependent.
Change-Id: Idae684781f45fe615020d8642f12a656ab2c15ad
this finally gives us a witness type we can use to prove that a certain
call graph subtree can't be used in kj promises using only a single new
assumption: if EvalState& is never held as a reference member of a type
and instead only ever passes as an argument or held on the stack we can
be certain that anything that has access to en EvalState ref must never
be run inside a promise and, crucially, that anything that doesn't have
access to an EvalState& *can* be run inside a promise without problems.
Change-Id: I6c15ada479175ad7e6cd3e4a729a5586b3ba30d6
Jade Lovelace