From 0256e5578e97a11db66207e1f8e231db115c91f8 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sat, 23 Apr 2022 23:04:35 +0200 Subject: [PATCH 1/2] libfetchers/git: hardcode `--git-dir` To demonstrate the problem: * You need a `git` at 2.33.3 in your $PATH * An expression like this in a git repository: ``` nix { outputs = { self, nixpkgs }: { packages.foo.x86_64-linux = with nixpkgs.legacyPackages.x86_64-linux; runCommand "snens" { } '' echo ${(builtins.fetchGit ./.).lastModifiedDate} > $out ''; }; } ``` Now, when instantiating the package via `builtins.getFlake`, it fails on Nix 2.7 like this: $ nix-instantiate -E '(builtins.getFlake "'"$(pwd)"'").packages.foo.x86_64-linux' fatal: unsafe repository ('/nix/store/a7j3125km4h8l0p71q6ssfkxamfh5d61-source' is owned by someone else) To add an exception for this directory, call: git config --global --add safe.directory /nix/store/a7j3125km4h8l0p71q6ssfkxamfh5d61-source error: program 'git' failed with exit code 128 (use '--show-trace' to show detailed location information) This breaks e.g. `nixops`-deployments using flakes with similar expressions as shown above. The cause for this is that `git(1)` tries to find the highest `.git`-directory in the directory tree and if it finds a such a directory, but with another owning user (root vs. the user who evaluates the expression), it fails as above. This was changed recently to fix CVE-2022-24765[1]. By explicitly specifying `--git-dir`, Git assumes to be in the top-level directory and doesn't attempt to look for a `.git`-directory in the parent directories and thus the code-path leading to said error is never reached. [1] https://lore.kernel.org/git/xmqqv8veb5i6.fsf@gitster.g/ --- src/libfetchers/git.cc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 34b1342a0..219a5ca7a 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -23,7 +23,7 @@ const std::string gitInitialBranch = "__nix_dummy_branch"; static std::string readHead(const Path & path) { - return chomp(runProgram("git", true, { "-C", path, "rev-parse", "--abbrev-ref", "HEAD" })); + return chomp(runProgram("git", true, { "-C", path, "--git-dir", ".git", "rev-parse", "--abbrev-ref", "HEAD" })); } static bool isNotDotGitDirectory(const Path & path) @@ -152,11 +152,11 @@ struct GitInputScheme : InputScheme assert(sourcePath); runProgram("git", true, - { "-C", *sourcePath, "add", "--force", "--intent-to-add", "--", std::string(file) }); + { "-C", *sourcePath, "--git-dir", ".git", "add", "--force", "--intent-to-add", "--", std::string(file) }); if (commitMsg) runProgram("git", true, - { "-C", *sourcePath, "commit", std::string(file), "-m", *commitMsg }); + { "-C", *sourcePath, "--git-dir", ".git", "commit", std::string(file), "-m", *commitMsg }); } std::pair getActualUrl(const Input & input) const @@ -259,7 +259,7 @@ struct GitInputScheme : InputScheme if (hasHead) { // Using git diff is preferrable over lower-level operations here, // because its conceptually simpler and we only need the exit code anyways. - auto gitDiffOpts = Strings({ "-C", actualUrl, "diff", "HEAD", "--quiet"}); + auto gitDiffOpts = Strings({ "-C", actualUrl, "--git-dir", ".git", "diff", "HEAD", "--quiet"}); if (!submodules) { // Changes in submodules should only make the tree dirty // when those submodules will be copied as well. @@ -284,7 +284,7 @@ struct GitInputScheme : InputScheme if (fetchSettings.warnDirty) warn("Git tree '%s' is dirty", actualUrl); - auto gitOpts = Strings({ "-C", actualUrl, "ls-files", "-z" }); + auto gitOpts = Strings({ "-C", actualUrl, "--git-dir", ".git", "ls-files", "-z" }); if (submodules) gitOpts.emplace_back("--recurse-submodules"); @@ -314,7 +314,7 @@ struct GitInputScheme : InputScheme // modified dirty file? input.attrs.insert_or_assign( "lastModified", - hasHead ? std::stoull(runProgram("git", true, { "-C", actualPath, "log", "-1", "--format=%ct", "--no-show-signature", "HEAD" })) : 0); + hasHead ? std::stoull(runProgram("git", true, { "-C", actualPath, "--git-dir", ".git", "log", "-1", "--format=%ct", "--no-show-signature", "HEAD" })) : 0); return {std::move(storePath), input}; } @@ -335,7 +335,7 @@ struct GitInputScheme : InputScheme if (!input.getRev()) input.attrs.insert_or_assign("rev", - Hash::parseAny(chomp(runProgram("git", true, { "-C", actualUrl, "rev-parse", *input.getRef() })), htSHA1).gitRev()); + Hash::parseAny(chomp(runProgram("git", true, { "-C", actualUrl, "--git-dir", ".git", "rev-parse", *input.getRef() })), htSHA1).gitRev()); repoDir = actualUrl; From d1f5356311bff3cb7840a82e6b35aeb66a1f6416 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sun, 24 Apr 2022 18:06:36 +0200 Subject: [PATCH 2/2] libfetchers/git: fix for nixos-rebuild The `--git-dir=` must be `.` in some cases (for cached repos that are "bare" repos in `~/.cache/nix/gitv3`). With this fix we can add `--git-dir` to each `git`-invokation needed for `nixos-rebuild`. --- src/libfetchers/git.cc | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 219a5ca7a..af40990e5 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -21,6 +21,15 @@ namespace nix::fetchers { // old version of git, which will ignore unrecognized `-c` options. const std::string gitInitialBranch = "__nix_dummy_branch"; +static std::string getGitDir() +{ + auto gitDir = getEnv("GIT_DIR"); + if (!gitDir) { + return ".git"; + } + return *gitDir; +} + static std::string readHead(const Path & path) { return chomp(runProgram("git", true, { "-C", path, "--git-dir", ".git", "rev-parse", "--abbrev-ref", "HEAD" })); @@ -150,13 +159,14 @@ struct GitInputScheme : InputScheme { auto sourcePath = getSourcePath(input); assert(sourcePath); + auto gitDir = getGitDir(); runProgram("git", true, - { "-C", *sourcePath, "--git-dir", ".git", "add", "--force", "--intent-to-add", "--", std::string(file) }); + { "-C", *sourcePath, "--git-dir", gitDir, "add", "--force", "--intent-to-add", "--", std::string(file) }); if (commitMsg) runProgram("git", true, - { "-C", *sourcePath, "--git-dir", ".git", "commit", std::string(file), "-m", *commitMsg }); + { "-C", *sourcePath, "--git-dir", gitDir, "commit", std::string(file), "-m", *commitMsg }); } std::pair getActualUrl(const Input & input) const @@ -175,6 +185,7 @@ struct GitInputScheme : InputScheme std::pair fetch(ref store, const Input & _input) override { Input input(_input); + auto gitDir = getGitDir(); std::string name = input.getName(); @@ -237,7 +248,7 @@ struct GitInputScheme : InputScheme since that is the refrence we want to use later on. */ auto result = runProgram(RunOptions { .program = "git", - .args = { "-C", actualUrl, "--git-dir=.git", "rev-parse", "--verify", "--no-revs", "HEAD^{commit}" }, + .args = { "-C", actualUrl, "--git-dir", gitDir, "rev-parse", "--verify", "--no-revs", "HEAD^{commit}" }, .environment = env, .mergeStderrToStdout = true }); @@ -259,7 +270,7 @@ struct GitInputScheme : InputScheme if (hasHead) { // Using git diff is preferrable over lower-level operations here, // because its conceptually simpler and we only need the exit code anyways. - auto gitDiffOpts = Strings({ "-C", actualUrl, "--git-dir", ".git", "diff", "HEAD", "--quiet"}); + auto gitDiffOpts = Strings({ "-C", actualUrl, "--git-dir", gitDir, "diff", "HEAD", "--quiet"}); if (!submodules) { // Changes in submodules should only make the tree dirty // when those submodules will be copied as well. @@ -284,7 +295,7 @@ struct GitInputScheme : InputScheme if (fetchSettings.warnDirty) warn("Git tree '%s' is dirty", actualUrl); - auto gitOpts = Strings({ "-C", actualUrl, "--git-dir", ".git", "ls-files", "-z" }); + auto gitOpts = Strings({ "-C", actualUrl, "--git-dir", gitDir, "ls-files", "-z" }); if (submodules) gitOpts.emplace_back("--recurse-submodules"); @@ -314,7 +325,7 @@ struct GitInputScheme : InputScheme // modified dirty file? input.attrs.insert_or_assign( "lastModified", - hasHead ? std::stoull(runProgram("git", true, { "-C", actualPath, "--git-dir", ".git", "log", "-1", "--format=%ct", "--no-show-signature", "HEAD" })) : 0); + hasHead ? std::stoull(runProgram("git", true, { "-C", actualPath, "--git-dir", gitDir, "log", "-1", "--format=%ct", "--no-show-signature", "HEAD" })) : 0); return {std::move(storePath), input}; } @@ -335,7 +346,7 @@ struct GitInputScheme : InputScheme if (!input.getRev()) input.attrs.insert_or_assign("rev", - Hash::parseAny(chomp(runProgram("git", true, { "-C", actualUrl, "--git-dir", ".git", "rev-parse", *input.getRef() })), htSHA1).gitRev()); + Hash::parseAny(chomp(runProgram("git", true, { "-C", actualUrl, "--git-dir", gitDir, "rev-parse", *input.getRef() })), htSHA1).gitRev()); repoDir = actualUrl; @@ -351,6 +362,7 @@ struct GitInputScheme : InputScheme Path cacheDir = getCacheDir() + "/nix/gitv3/" + hashString(htSHA256, actualUrl).to_string(Base32, false); repoDir = cacheDir; + gitDir = "."; createDirs(dirOf(cacheDir)); PathLocks cacheDirLock({cacheDir + ".lock"}); @@ -427,7 +439,7 @@ struct GitInputScheme : InputScheme // cache dir lock is removed at scope end; we will only use read-only operations on specific revisions in the remainder } - bool isShallow = chomp(runProgram("git", true, { "-C", repoDir, "rev-parse", "--is-shallow-repository" })) == "true"; + bool isShallow = chomp(runProgram("git", true, { "-C", repoDir, "--git-dir", gitDir, "rev-parse", "--is-shallow-repository" })) == "true"; if (isShallow && !shallow) throw Error("'%s' is a shallow Git repository, but a non-shallow repository is needed", actualUrl);