From 8b0026312a4c2010c19b604461b97c8386fbeba6 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Tue, 16 Dec 2025 01:43:56 +0300 Subject: [PATCH] treewide: Get rid of PosixSourceAccessor::createAtRoot We'd like to split out the implementation into Unix/Windows-specific parts to more easily iterate on improving UNIX accessors to make use of dirfd-based operations (or even openat2). This should be hidden behind the appropriate interface and not exposed as a static member function of the PosixSourceAccessor. --- src/libutil/archive.cc | 3 +- .../include/nix/util/posix-source-accessor.hh | 30 ------------------- .../include/nix/util/source-accessor.hh | 2 +- src/libutil/posix-source-accessor.cc | 15 ++-------- src/nix/add-to-store.cc | 2 +- src/nix/hash.cc | 4 +-- src/nix/nix-store/nix-store.cc | 4 +-- src/perl/lib/Nix/Store.xs | 4 +-- 8 files changed, 11 insertions(+), 53 deletions(-) diff --git a/src/libutil/archive.cc b/src/libutil/archive.cc index 0291d6827..097678998 100644 --- a/src/libutil/archive.cc +++ b/src/libutil/archive.cc @@ -103,7 +103,8 @@ void SourceAccessor::dumpPath(const CanonPath & path, Sink & sink, PathFilter & time_t dumpPathAndGetMtime(const Path & path, Sink & sink, PathFilter & filter) { - auto path2 = PosixSourceAccessor::createAtRoot(path, /*trackLastModified=*/true); + SourcePath path2 = { + makeFSSourceAccessor(std::filesystem::path{}, /*trackLastModified=*/true), CanonPath(absPath(path))}; path2.dumpPath(sink, filter); return path2.accessor->getLastModified().value(); } diff --git a/src/libutil/include/nix/util/posix-source-accessor.hh b/src/libutil/include/nix/util/posix-source-accessor.hh index 29561a3da..37b8d9189 100644 --- a/src/libutil/include/nix/util/posix-source-accessor.hh +++ b/src/libutil/include/nix/util/posix-source-accessor.hh @@ -43,36 +43,6 @@ public: std::optional getPhysicalPath(const CanonPath & path) override; - /** - * Create a `PosixSourceAccessor` and `SourcePath` corresponding to - * some native path. - * - * @param Whether the accessor should return a non-null getLastModified. - * When true the accessor must be used only by a single thread. - * - * The `PosixSourceAccessor` is rooted as far up the tree as - * possible, (e.g. on Windows it could scoped to a drive like - * `C:\`). This allows more `..` parent accessing to work. - * - * @note When `path` is trusted user input, canonicalize it using - * `std::filesystem::canonical`, `makeParentCanonical`, `std::filesystem::weakly_canonical`, etc, - * as appropriate for the use case. At least weak canonicalization is - * required for the `SourcePath` to do anything useful at the location it - * points to. - * - * @note A canonicalizing behavior is not built in `createAtRoot` so that - * callers do not accidentally introduce symlink-related security vulnerabilities. - * Furthermore, `createAtRoot` does not know whether the file pointed to by - * `path` should be resolved if it is itself a symlink. In other words, - * `createAtRoot` can not decide between aforementioned `canonical`, `makeParentCanonical`, etc. for its callers. - * - * See - * [`std::filesystem::path::root_path`](https://en.cppreference.com/w/cpp/filesystem/path/root_path) - * and - * [`std::filesystem::path::relative_path`](https://en.cppreference.com/w/cpp/filesystem/path/relative_path). - */ - static SourcePath createAtRoot(const std::filesystem::path & path, bool trackLastModified = false); - std::optional getLastModified() override { return trackLastModified ? std::optional{mtime} : std::nullopt; diff --git a/src/libutil/include/nix/util/source-accessor.hh b/src/libutil/include/nix/util/source-accessor.hh index 1006895b3..6c68187a9 100644 --- a/src/libutil/include/nix/util/source-accessor.hh +++ b/src/libutil/include/nix/util/source-accessor.hh @@ -233,7 +233,7 @@ ref getFSSourceAccessor(); * elements, and that absolute symlinks are resolved relative to * `root`. */ -ref makeFSSourceAccessor(std::filesystem::path root); +ref makeFSSourceAccessor(std::filesystem::path root, bool trackLastModified = false); /** * Construct an accessor that presents a "union" view of a vector of diff --git a/src/libutil/posix-source-accessor.cc b/src/libutil/posix-source-accessor.cc index abbab45db..4e090e617 100644 --- a/src/libutil/posix-source-accessor.cc +++ b/src/libutil/posix-source-accessor.cc @@ -1,7 +1,5 @@ #include "nix/util/posix-source-accessor.hh" -#include "nix/util/source-path.hh" #include "nix/util/signals.hh" -#include "nix/util/sync.hh" #include @@ -20,15 +18,6 @@ PosixSourceAccessor::PosixSourceAccessor() { } -SourcePath PosixSourceAccessor::createAtRoot(const std::filesystem::path & path, bool trackLastModified) -{ - std::filesystem::path path2 = absPath(path); - return { - make_ref(path2.root_path(), trackLastModified), - CanonPath{path2.relative_path().string()}, - }; -} - std::filesystem::path PosixSourceAccessor::makeAbsPath(const CanonPath & path) { return root.empty() ? (std::filesystem::path{path.abs()}) @@ -219,8 +208,8 @@ ref getFSSourceAccessor() return rootFS; } -ref makeFSSourceAccessor(std::filesystem::path root) +ref makeFSSourceAccessor(std::filesystem::path root, bool trackLastModified) { - return make_ref(std::move(root)); + return make_ref(std::move(root), trackLastModified); } } // namespace nix diff --git a/src/nix/add-to-store.cc b/src/nix/add-to-store.cc index e87f49546..0c6a1f74b 100644 --- a/src/nix/add-to-store.cc +++ b/src/nix/add-to-store.cc @@ -38,7 +38,7 @@ struct CmdAddToStore : MixDryRun, StoreCommand if (!namePart) namePart = baseNameOf(path); - auto sourcePath = PosixSourceAccessor::createAtRoot(makeParentCanonical(path)); + SourcePath sourcePath = makeFSSourceAccessor(makeParentCanonical(path)); auto storePath = dryRun ? store->computeStorePath(*namePart, sourcePath, caMethod, hashAlgo, {}).first : store->addToStoreSlow(*namePart, sourcePath, caMethod, hashAlgo, {}).path; diff --git a/src/nix/hash.cc b/src/nix/hash.cc index 2945c672c..eeb9a81bc 100644 --- a/src/nix/hash.cc +++ b/src/nix/hash.cc @@ -85,9 +85,7 @@ struct CmdHashBase : Command return std::make_unique(hashAlgo); }; - auto makeSourcePath = [&]() -> SourcePath { - return PosixSourceAccessor::createAtRoot(makeParentCanonical(path)); - }; + auto makeSourcePath = [&]() -> SourcePath { return makeFSSourceAccessor(makeParentCanonical(path)); }; Hash h{HashAlgorithm::SHA256}; // throwaway def to appease C++ switch (mode) { diff --git a/src/nix/nix-store/nix-store.cc b/src/nix/nix-store/nix-store.cc index a2c0aaf3f..0f657cd64 100644 --- a/src/nix/nix-store/nix-store.cc +++ b/src/nix/nix-store/nix-store.cc @@ -191,7 +191,7 @@ static void opAdd(Strings opFlags, Strings opArgs) throw UsageError("unknown flag"); for (auto & i : opArgs) { - auto sourcePath = PosixSourceAccessor::createAtRoot(makeParentCanonical(i)); + SourcePath sourcePath = makeFSSourceAccessor(makeParentCanonical(i)); cout << fmt("%s\n", store->printStorePath(store->addToStore(std::string(baseNameOf(i)), sourcePath))); } } @@ -215,7 +215,7 @@ static void opAddFixed(Strings opFlags, Strings opArgs) opArgs.pop_front(); for (auto & i : opArgs) { - auto sourcePath = PosixSourceAccessor::createAtRoot(makeParentCanonical(i)); + SourcePath sourcePath = makeFSSourceAccessor(makeParentCanonical(i)); std::cout << fmt( "%s\n", store->printStorePath(store->addToStoreSlow(baseNameOf(i), sourcePath, method, hashAlgo).path)); } diff --git a/src/perl/lib/Nix/Store.xs b/src/perl/lib/Nix/Store.xs index 93e9f0f95..f87b5def8 100644 --- a/src/perl/lib/Nix/Store.xs +++ b/src/perl/lib/Nix/Store.xs @@ -256,7 +256,7 @@ hashPath(char * algo, int base32, char * path) PPCODE: try { Hash h = hashPath( - PosixSourceAccessor::createAtRoot(path), + SourcePath{getFSSourceAccessor(), CanonPath(absPath(Path(path)))}, FileIngestionMethod::NixArchive, parseHashAlgo(algo)).first; auto s = h.to_string(base32 ? HashFormat::Nix32 : HashFormat::Base16, false); XPUSHs(sv_2mortal(newSVpv(s.c_str(), 0))); @@ -336,7 +336,7 @@ StoreWrapper::addToStore(char * srcPath, int recursive, char * algo) auto method = recursive ? ContentAddressMethod::Raw::NixArchive : ContentAddressMethod::Raw::Flat; auto path = THIS->store->addToStore( std::string(baseNameOf(srcPath)), - PosixSourceAccessor::createAtRoot(srcPath), + SourcePath{getFSSourceAccessor(), CanonPath(absPath(Path(srcPath)))}, method, parseHashAlgo(algo)); XPUSHs(sv_2mortal(newSVpv(THIS->store->printStorePath(path).c_str(), 0))); } catch (Error & e) {