if a build directory is accessible to other users it is possible to
smuggle data in and out of build directories. usually this ins only
a build purity problem, but in combination with other issues it can
be used to break out of a build sandbox. to prevent this we default
to using a subdirectory of nixStateDir (which is more restrictive).
Fixes CVE-2025-52991.
Change-Id: Iacfc9b50534de158618c815f9fb99d7dae1be4d0
This allows using a userspace program, pasta, to handle comms between
the build sandbox, and the outside world; allowing for full isolation
including the network namespace, closing the "fixed-output derivation
talks to the host over an abstract domain socket" hole for good.
Fixes CVE-2025-46416.
Co-Authored-By: Puck Meerburg <puck@puckipedia.com>
Change-Id: Ifd499b7dbb3784600a6e842fede65fc031ff9f15
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)
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 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
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
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
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
"persistent" here means state that outlives any particular evaluation,
i.e. everything that exist outside of a call to eval() and friends. do
note that this leave EvalState nearly empty, containing only things of
importance to evaluation *itself*, e.g. the call depth tracker. do not
worry about the self-reference ctx member. nothing to see here, go on.
Change-Id: Ie8d0af3f09d33902f81e0c36a1096dd9f334a537
for the same reason the eval caches should not keep them as references,
only worse: installables cannot fully lose their reference member since
some types of installable (flakes -sigh-) need access to caches held by
their member reference. passing that around as well at all times is not
feasible; passing in EvalState& all the time is quite bad enough as is.
Change-Id: I05d438623b5ef10fc22e7e256b89447e61a0657d
this is not necessary in any way, but it will make the following changes
smaller and easier to review. the aliases could also be added piecemeal,
but doing it here lets us lean heavily on our compilers for correctness.
(teacher notes: here the author foreshadows the shape of things to come.
not all names change, and only the names unchanged are those which will,
over time, become ever more unrecognizable. note especially nix/main.cc,
where `state` is not only cloned, but itself changes pointerness. it can
be seen as a nod to the trans community, but more realistically it is no
more than foreshadowing the future where `state` is only seen by proxy.)
Change-Id: I7732025e58df089b7f8e564fc63960cd91729d09
it's not used for anything any more. it was only passed on to the AttrDb
constructor, which itself only stashed it away to an unused member field
Change-Id: I334fde751a3754e6580f573c21ae3d04be00345a
and for the same reason DrvInfo shouldn't, only even more so. the eval
cache infrastructure wants to be a wrapper around evaluation, with its
lifetimes fully decoupled from the lifetime of the wrapped evaluation.
this can't not create problems when eval lifetime must become bounded.
Change-Id: Ib8eec649995b4decd7290c2266322f67d73b6b46
in the future eval states will be asyncio root contexts. proving that
they are used correctly is impossible if the are referenced by things
with weakly scoped lifetimes, or entirely unscoped lifetimes, as we'd
have to deal with in the case of `DrvInfos` passed as out parameters.
Change-Id: I9ca5c46a714c6c914a77c7c7500cb4939ac9aff5
it's all strings anyway. the db stores strings, the cli wants to
interact with attr paths as strings, so we will just use strings
Change-Id: Id9ea07d92343de77e8d47af8fec1e86ae225e9a1
there's no reason for these to be private. much more important things
that are now part of EvalState are not private, and having statistics
be private forces us to add otherwise unnecessary friend declarations
Change-Id: Ib37097d94a9f55c2b21969fb6c51049b1c914515
thunk creation doesn't have to be called that. thunks are more of an
implementation detail, calling it evalLazily seems a lot more clear.
Change-Id: I64bdac422eadb629002195cd6b6a89dce1b4a60f
this will let us pass the capability to create debuggable eval errors
without having to pass an entire EvalState. we could pass symbols and
debug states around just as easily, but if we add new capabilities to
our debugger we might have to change many more places than with this.
Change-Id: I2f8893012e5d98a986ef1fc888234c2dd8d5e096
if we hold the error being built behind a pointer we no longer need to
allocate the error builder itself to avoid impacting eval performance.
adding && ref qualifiers to builder functions and adding the nodiscard
attribute to the class itself also makes the nodiscard attributes kept
on the builder functions unnecessary. we do keep the noinlines though,
removing them still regresses eval performance even after this change.
Change-Id: Ibc14a66955ac32142d97fb3680b0a7e14db250cd
this is necessary to share a debug state between multiple eval states.
while doings so makes little sense at present it will be necessary for
async io support since eval states must be async io roots, which means
we must create them as needed from a shared evaluation context object.
Change-Id: Id9d4b37aae40706f65c741e3b961855582e035ab
if we define a TraceableAllocator at all times and use that in places
that want maybe-traceable allocation we can simplify things a lot. we
also unconditionally allocate cache root pointers for Value and Env-1
caches, even though we don't need them without gc (they're so cheap).
defaulting to `std::allocator` without gc recovers previous behavior.
Change-Id: I236da8c3b0669b40cdfe355ec3ec4e764d096074
that way we don't need to eval while initializing the evaluator. this
will make a lot more sense once we start pushing asyncio through here
Change-Id: I3663052438ed97d48e213b71395ad1dd5e1318bb
currently only used to hide the nix code of derivation from stack traces
and debug frames, but perhaps we'll find it useful for other things too.
Change-Id: Ie5667873d8858d25dd4113bdf454e800b59082d7
this doesn't belong here, getFlake should be handling this instead. we
do lose the debug frame for loading a root flake, but that seems fine.
there's no point in starting a debugger if the root isn't even a flake
Change-Id: I24ad32b6716baee81a1a0f8bf9ce26814d97c7aa
this was only used to enable an optimization we no longer need such
strong guarantees for: SmallVector can be used with a dynamic size.
Change-Id: I4513fb7e665827fe363ce6308448656e6c5badb7
Error::addTrace exists and is used far more often already. let's
standardize on the variant that doesn't need yet more templates.
Change-Id: If66b69ca02dbb546ce98cf385181bd13ce7ad9b5
the callback is always called immediately, which defeats the purpose of
the callback and the purpose of the template itself. there seems to not
be any performance impact of this. optimizing Value::determinePos would
be nice, but it's not used nearly often enough to matter at this point.
Change-Id: I2aec6a38103630652112f4b273653f11d2404c04
nothing needs these any more. the CLI calls evalFile, but only in places
where it'll only be called *once* per process lifetime. __import does so
too, but import doesn't care about the parse tree, only the eval result.
interestingly this improves eval performance by 6% on system eval unless
GC never occurs. no idea why this makes a GCing eval faster, but it does
Change-Id: I8289528550244e0a8b5ebc7284d8fb9aaac59e20
only generate-manpage.nix uses it any more, so we can inline it there
instead of keeping it around as a separate generated header. doing so
will also allow us to remove caching functions needed *only for this*
Change-Id: I97ee91f1dd7140ecb69dbafd8479b82fba7981b8
eval caches are not used by actual eval at all, only by the flake-shaped
wrappers around evaluation. moving caches into a subclass both clarifies
that eval caches and eval states are coupled and separates concerns that
should not have been intermixed as they were here. in the future we will
want to split up and decouple things even further. that'll have to wait.
Change-Id: I7b69510c0f8b212f05fae62e7b992d9475b4841f
installables already have a ref<EvalState>. why are we passing the same
eval state in again, by reference, everywhere? that's just unnecessary.
Change-Id: I8225ea2575146edc55d283c0b5173b804553ceec
this belongs to lazy trees, which we neither have nor intend to have.
we will keep SourcePath as that may come in handy at some later date.
Change-Id: I44b8f1dd6c435d7486c393fabdcd272766b2b56b
this also fixes a debugger bug where leaving the debugger does not clean
up old debugger state completely. in such cases the fake frame withFrame
created was left behind after the corresponding caller frame was unwound
Change-Id: I45adcd116276b03b2f87076518c9eae6fe844e06
checking whether a repl hook is set does not tell whether it's running,
which in turn means we will enter a debug repl even when not debugging.
this is not very useful when we have no debug frames to inspect at all,
as commonly happens when starting a new repl with --debugger specified.
Change-Id: I7065dc4ec29743bdd53ed99c29d6592e2ceea89c
just to keep debug-related state closer together. eventually this will
also allow us to not allocate debugger state altogether unless needed.
Change-Id: Id83fea75d96d0ecbe21683cb1b57dd5b11b13535
not all of these are even used by eval itself. notably sWith wasn't used
at all, sEpsilon was only used by the eval cache one layer up, and other
attributes are used in places even further away from eval itself. we can
keep this commingling for now, but eventually we should clean it up too.
Change-Id: I5684ac614361bf008e04472130c6c02082b4c2d7
these do not rely on the GC being initialized. there's no reason for
them to not be statics, shared between all eval states in a process.
Change-Id: Ib9675f3945d3a0a7097d6c85096adcbd6f441d83
currently ref<>s are used to share eval states between whatever created
that state and the various repl instances. repls however do not own any
eval states, not even partially. they only provide interactive use of a
previously allocated state that is owned by something else, notably for
debug repls which are run from *within* an evaluation which may already
be wrapped by another repl instance. presumably this was originally set
up like this to guarantee memory safety, even though it's never needed.
Change-Id: I29a2cd1d4245c077f9270452a0873d4c47448729
there's no reason to ever *own* a repl instance. everything either wants
to run a repl and receive the result of that run, or run a repl and then
totally ignore the result. allowing non-transient repl instances doesn't
do anything useful for us, unless making ownership analysis hard counts.
this also fixes a bug in which a repl could ignore the store it was told
to use during construction, using the one returned by openStore instead.
this never showed up in lix because openStore() was always passed anyway
Change-Id: If2b1ad24ab74377340199b36af1dd629d7ce3f25
give legacy commands their program name and c++-converted argv directly
instead of passing on the (argc, argv) pair untouched. all are required
to process these things, and all of them do it in exactly the same way.
we can also remove a few old helpers only used to make this less awful.
Change-Id: I4ecd02343bca0cf85faf6fe043031d4f64c5f29c
it's a trap. it's always a trap. non-default ctors of virtual base
classes must be called with the same parameters every time or very
surprising things will happen. good luck if you virtually derive a
class more than once with different ctor arguments. that'll break.
Change-Id: I1345963e69e98f37c3cbd070bae164d89eade455
this does nothing on classes that are never used as base classes and
have only a single base class of their own. all of these classes are
also made final to prove that they are indeed in this category. once
we need to derive any of them we can erase their `final` specifiers.
Change-Id: I4bd8e50a58815227392b5a5a762fda4542d15bae
there are no uses of plain MultiCommand as a base class *except* in
NixArgs, which is the only one that does not implement run(). there
is not much of a reason not to implement a run member there though,
so let's just do that and get rid of this weird intermediate class.
Change-Id: Ie84e3acd071b43bc186a2bac87646cbfb3aff845
the very slight speedup in config setting access is not worth the
maintenance overhead of conflating concers like this. the virtual
inheritance scheme used for configs requires too much duplication
of base class constructor arguments to be worth doing. perhaps we
should get rid of all virtual inheritance of data-membered bases?
Change-Id: I4acf5ceaedb4ed7476efe1114c2e065ec72d2c6d
clangd broke because it can't look through symlinks. compile_commands
manipulation does not fix it, clangd configuration does not fix it, a
vfs overlay does not fix it, and while a combination of those can fix
it with a bind mount in place that's just too cursed to even consider
clangd bug: https://github.com/llvm/llvm-project/issues/116877
Change-Id: I8e3e8489548eb3a7aa65ac9d12a5ec8abf814aec
give the transfer progress callback a chance to register all transfers
as aborted when an interrupt event occurs. checkInterrupt() will throw
an Interrupted exception if an interrupt signal was caught, which will
immediately break out of the curl loop. it isn't even necessary within
the curl main loop as curl guarantees to call progress callbacks about
once per second (or more often), and we already abort transfers from a
progress update if a signal was caught. this may delay shutdown a bit,
but "about one second" as should not be noticeable in most situations.
fixes#577
Change-Id: Ida4e72732562bdf6560e4f182ecdcdb663c6dda5
transferComplete has outlived its usefulness, and the other two phases
can be a boolean flag telling us whether we're still receiving headers
Change-Id: Ia943f95dcf0ce8ee2cc6d0f44ddf13151aef4346
things can be much simpler if the read loop knows whether the transfer
is done or not. this information is available, let's pass it back down
Change-Id: Idb99afeb06a0d7a0999c3f4a1c6ce5adeab5f054
file:// urls are no longer handled by curl itself, and this is supposed
to test the curl wrapper. use http to force the wrapper to be involved.
Change-Id: Ib13087db07b3b2f1ae44ce8e3ec7a96d935b1bab
they don't need these any more if we consider unpausing and cancelling
of transfers a cooperative affair between transfers and the curl_multi
handler. we can also remove an enable_shared_from_this base class now.
Change-Id: Iefa380df60c0cada53719cdaa500bab953892319
wrap all curl pointers in unique pointers for move correctness. the
destructor of TransferItem can go away completely since it does not
do anything special any more: memory bookkeeping is done using raii
wrappers, and its effects on the transfer state are not observable.
only TransferSource inspects the result, and even it does not check
for transfer results in its own destructor (only during transfers).
Change-Id: Ibb58484703854d5d5425e7c63cf2985d93920e97
keep the download buffer in TransferItem. keep only *one* download
buffer (not one for success returns and one for failures). we also
remove the asynchronous callbacks since they are no longer needed.
Change-Id: I9b1c1e4c59d5333602aea1c36e477c481eaaf98e
