From 1f087c15c6c20a607b29f4758dba1cbd3a62550f Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 24 Oct 2024 11:54:49 +0200 Subject: [PATCH 1/4] refactor: Funnel libexpr fetchToStore through EvalState This allows a bit more control. EvalState can now augment all fetchToStore calls. --- src/libexpr/eval.cc | 15 +++++++++++++-- src/libexpr/eval.hh | 9 +++++++++ src/libexpr/primops.cc | 3 +-- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index f17753415fc..25354fb1f92 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -2358,7 +2358,6 @@ StorePath EvalState::copyPathToStore(NixStringContext & context, const SourcePat ? *dstPathCached : [&]() { auto dstPath = fetchToStore( - *store, path.resolveSymlinks(), settings.readOnlyMode ? FetchMode::DryRun : FetchMode::Copy, path.baseName(), @@ -2377,6 +2376,18 @@ StorePath EvalState::copyPathToStore(NixStringContext & context, const SourcePat return dstPath; } +StorePath EvalState::fetchToStore( + const SourcePath & path, + FetchMode mode, + std::string_view name, + ContentAddressMethod method, + PathFilter * filter, + RepairFlag repair) +{ + return ::nix::fetchToStore(*store, path, mode, name, method, filter, repair); +} + + SourcePath EvalState::coerceToPath(const PosIdx pos, Value & v, NixStringContext & context, std::string_view errorCtx) { @@ -3055,7 +3066,7 @@ std::optional EvalState::resolveLookupPathPath(const LookupPath::Pa store, fetchSettings, EvalSettings::resolvePseudoUrl(value)); - auto storePath = fetchToStore(*store, SourcePath(accessor), FetchMode::Copy); + auto storePath = fetchToStore(SourcePath(accessor), FetchMode::Copy); return finish(store->toRealPath(storePath)); } catch (Error & e) { logWarning({ diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index f7ed6be83af..2a9bbdbbdeb 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -3,6 +3,7 @@ #include "attr-set.hh" #include "eval-error.hh" +#include "fetch-to-store.hh" #include "types.hh" #include "value.hh" #include "nixexpr.hh" @@ -804,6 +805,14 @@ public: DocComment getDocCommentForPos(PosIdx pos); + StorePath fetchToStore( + const SourcePath & path, + FetchMode mode, + std::string_view name = "source", + ContentAddressMethod method = ContentAddressMethod::Raw::NixArchive, + PathFilter * filter = nullptr, + RepairFlag repair = NoRepair); + private: /** diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 203d109324f..5f5fb4d8653 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -2473,8 +2473,7 @@ static void addPath( {})); if (!expectedHash || !state.store->isValidPath(*expectedStorePath)) { - auto dstPath = fetchToStore( - *state.store, + auto dstPath = state.fetchToStore( path.resolveSymlinks(), settings.readOnlyMode ? FetchMode::DryRun : FetchMode::Copy, name, From ed3edf2de5510a9fd864f0c640d6055b2adfe5da Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 24 Oct 2024 13:18:06 +0200 Subject: [PATCH 2/4] Add option disallow-copy-paths --- src/libexpr/eval-settings.hh | 9 +++++++++ src/libexpr/eval.cc | 3 +++ tests/functional/disallow-copy-paths.sh | 16 ++++++++++++++++ tests/functional/local.mk | 1 + tests/functional/meson.build | 1 + 5 files changed, 30 insertions(+) create mode 100755 tests/functional/disallow-copy-paths.sh diff --git a/src/libexpr/eval-settings.hh b/src/libexpr/eval-settings.hh index 115e3ee501b..ec6b609f422 100644 --- a/src/libexpr/eval-settings.hh +++ b/src/libexpr/eval-settings.hh @@ -247,6 +247,15 @@ struct EvalSettings : Config This option can be enabled by setting `NIX_ABORT_ON_WARN=1` in the environment. )"}; + + Setting> disallowCopyPaths{this, {}, "disallow-copy-paths", + R"( + A list of paths that are not allowed to be copied. + + This is useful for finding expressions which copy sources, which can slow down evaluation. + You may find copied sources by running `nix` commands with increased verbosity, such as `nix build -vvvv 2>&1 | grep /nix/store`. + After identifying one more more paths, run `nix build --option disallow-copy-paths /nix/store/... --show-trace` to find the expression that copies the path, or add `--debugger`. + )"}; }; /** diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 25354fb1f92..640ca777e04 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -2384,6 +2384,9 @@ StorePath EvalState::fetchToStore( PathFilter * filter, RepairFlag repair) { + if (path.accessor == rootFS && settings.disallowCopyPaths.get().contains(path.path.abs())) { + error("not allowed to copy '%1%' due to option '%2%'", path.path.abs(), settings.disallowCopyPaths.name).debugThrow(); + } return ::nix::fetchToStore(*store, path, mode, name, method, filter, repair); } diff --git a/tests/functional/disallow-copy-paths.sh b/tests/functional/disallow-copy-paths.sh new file mode 100755 index 00000000000..4cd1ff827f1 --- /dev/null +++ b/tests/functional/disallow-copy-paths.sh @@ -0,0 +1,16 @@ +#!/usr/bin/env bash + +source common.sh + +clearStoreIfPossible + +# shellcheck disable=SC2016 +path="$(nix eval --raw --impure --expr '"${./disallow-copy-paths.sh}"')" + +# shellcheck disable=SC2016 +expectStderr 1 nix-instantiate \ + --disallow-copy-paths "$path" \ + --expr --strict \ + --argstr path "$path" \ + '{ path }: "${/. + path}" + "bla bla"' \ + | grepQuiet "error.*not allowed to copy.*$path.* due to option.*disallow-copy-paths" diff --git a/tests/functional/local.mk b/tests/functional/local.mk index e50b5eaf1df..26c6a5644bf 100644 --- a/tests/functional/local.mk +++ b/tests/functional/local.mk @@ -1,5 +1,6 @@ nix_tests = \ test-infra.sh \ + disallow-copy-paths.sh \ gc.sh \ nix-collect-garbage-d.sh \ remote-store.sh \ diff --git a/tests/functional/meson.build b/tests/functional/meson.build index 54f3e7a0158..8f3a2cae90c 100644 --- a/tests/functional/meson.build +++ b/tests/functional/meson.build @@ -69,6 +69,7 @@ suites = [ 'deps': [], 'tests': [ 'test-infra.sh', + 'disallow-copy-paths.sh', 'gc.sh', 'nix-collect-garbage-d.sh', 'remote-store.sh', From c7cd714169e97d480a43b9c86c12f463751a1f57 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 24 Oct 2024 13:30:11 +0200 Subject: [PATCH 3/4] refactor: Extract EvalState::checkDisallowedCopyPath --- src/libexpr/eval.cc | 10 +++++++--- src/libexpr/eval.hh | 3 +++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 640ca777e04..fa4661b1102 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -2376,6 +2376,12 @@ StorePath EvalState::copyPathToStore(NixStringContext & context, const SourcePat return dstPath; } +void EvalState::checkDisallowCopyPath(const SourcePath & path) { + if (path.accessor == rootFS && settings.disallowCopyPaths.get().contains(path.path.abs())) { + error("not allowed to copy '%1%' due to option '%2%'", path.path.abs(), settings.disallowCopyPaths.name).debugThrow(); + } +} + StorePath EvalState::fetchToStore( const SourcePath & path, FetchMode mode, @@ -2384,9 +2390,7 @@ StorePath EvalState::fetchToStore( PathFilter * filter, RepairFlag repair) { - if (path.accessor == rootFS && settings.disallowCopyPaths.get().contains(path.path.abs())) { - error("not allowed to copy '%1%' due to option '%2%'", path.path.abs(), settings.disallowCopyPaths.name).debugThrow(); - } + checkDisallowCopyPath(path); return ::nix::fetchToStore(*store, path, mode, name, method, filter, repair); } diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 2a9bbdbbdeb..095a567630a 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -813,6 +813,9 @@ public: PathFilter * filter = nullptr, RepairFlag repair = NoRepair); + /** Throws if path occurs in the disallow-copy-path option. */ + void checkDisallowCopyPath(const SourcePath & path); + private: /** From e95a4ba6bb83161f0fa1af38b23112f2daf25439 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 24 Oct 2024 13:55:05 +0200 Subject: [PATCH 4/4] Support builtins.path with disallow-copy-paths --- src/libexpr/eval-settings.hh | 2 ++ src/libexpr/eval.cc | 3 +- src/libexpr/primops.cc | 6 +++- tests/functional/disallow-copy-paths.sh | 48 +++++++++++++++++++++---- 4 files changed, 50 insertions(+), 9 deletions(-) diff --git a/src/libexpr/eval-settings.hh b/src/libexpr/eval-settings.hh index ec6b609f422..fdf253c88e7 100644 --- a/src/libexpr/eval-settings.hh +++ b/src/libexpr/eval-settings.hh @@ -255,6 +255,8 @@ struct EvalSettings : Config This is useful for finding expressions which copy sources, which can slow down evaluation. You may find copied sources by running `nix` commands with increased verbosity, such as `nix build -vvvv 2>&1 | grep /nix/store`. After identifying one more more paths, run `nix build --option disallow-copy-paths /nix/store/... --show-trace` to find the expression that copies the path, or add `--debugger`. + + A filtering copy is always allowed, such as `builtins.filterSource` or `builtins.path { filter = ...; }`. )"}; }; diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index fa4661b1102..8ffe69355ba 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -2390,7 +2390,8 @@ StorePath EvalState::fetchToStore( PathFilter * filter, RepairFlag repair) { - checkDisallowCopyPath(path); + if (!filter) + checkDisallowCopyPath(path); return ::nix::fetchToStore(*store, path, mode, name, method, filter, repair); } diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 5f5fb4d8653..9db6a199127 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -2486,8 +2486,12 @@ static void addPath( path ).atPos(pos).debugThrow(); state.allowAndSetStorePathString(dstPath, v); - } else + } else { + if (!filterFun) + state.checkDisallowCopyPath(path); + state.allowAndSetStorePathString(*expectedStorePath, v); + } } catch (Error & e) { e.addTrace(state.positions[pos], "while adding path '%s'", path); throw; diff --git a/tests/functional/disallow-copy-paths.sh b/tests/functional/disallow-copy-paths.sh index 4cd1ff827f1..fb662adb0ea 100755 --- a/tests/functional/disallow-copy-paths.sh +++ b/tests/functional/disallow-copy-paths.sh @@ -7,10 +7,44 @@ clearStoreIfPossible # shellcheck disable=SC2016 path="$(nix eval --raw --impure --expr '"${./disallow-copy-paths.sh}"')" -# shellcheck disable=SC2016 -expectStderr 1 nix-instantiate \ - --disallow-copy-paths "$path" \ - --expr --strict \ - --argstr path "$path" \ - '{ path }: "${/. + path}" + "bla bla"' \ - | grepQuiet "error.*not allowed to copy.*$path.* due to option.*disallow-copy-paths" +all_tests() { + + # shellcheck disable=SC2016 + expectStderr 1 nix-instantiate \ + --disallow-copy-paths "$path" \ + --expr --strict \ + --argstr path "$path" \ + '{ path }: "${/. + path}" + "bla bla"' \ + "$@" \ + | grepQuiet "error.*not allowed to copy.*$path.* due to option.*disallow-copy-paths" + + # shellcheck disable=SC2016 + expectStderr 1 nix-instantiate \ + --disallow-copy-paths "$path" \ + --expr --strict \ + --argstr path "$path" \ + "$@" \ + '{ path }: builtins.path { path = /. + path; name = "source"; } + "bla bla"' \ + | grepQuiet "error.*not allowed to copy.*$path.* due to option.*disallow-copy-paths" + + # shellcheck disable=SC2016 + expectStderr 1 nix-instantiate \ + --disallow-copy-paths "$path" \ + --expr --strict \ + --argstr path "$path" \ + "$@" \ + '{ path }: builtins.path { path = path; name = "source"; } + "bla bla"' \ + | grepQuiet "error.*not allowed to copy.*$path.* due to option.*disallow-copy-paths" + + # shellcheck disable=SC2016 + nix-instantiate \ + --disallow-copy-paths "$path" \ + --expr --eval --strict \ + "$@" \ + --argstr path "$path" \ + '{ path }: builtins.path { path = path; name = "source"; filter = _: _: true; } + "bla bla"' \ + +} + +all_tests +all_tests --readonly-mode