Fix nix-build --check -K in sandbox w/o root

Temporarily add user-write permission to build directory so that it
can be moved out of the sandbox to the store with a .check suffix.

This is necessary because the build directory has already had its
permissions set read-only, but write permission is required
to update the directory's parent link to move it out of the sandbox.

Updated the related --check "derivation may not be deterministic"
messages to consistently use the real store paths.

Added test for non-root sandbox nix-build --check -K to demonstrate
issue and help prevent regressions.
This commit is contained in:
Bruce Toll 2019-02-17 14:34:31 -05:00
parent 3abf6d03c6
commit 8132d0a12e
2 changed files with 33 additions and 4 deletions

View file

@ -3537,6 +3537,29 @@ StorePathSet parseReferenceSpecifiers(Store & store, const BasicDerivation & drv
} }
static void moveCheckToStore(const Path & src, const Path & dst)
{
/* For the rename of directory to succeed, we must be running as root or
the directory must be made temporarily writable (to update the
directory's parent link ".."). */
struct stat st;
if (lstat(src.c_str(), &st) == -1) {
throw SysError(format("getting attributes of path '%1%'") % src);
}
bool changePerm = (geteuid() && S_ISDIR(st.st_mode) && !(st.st_mode & S_IWUSR));
if (changePerm)
chmod_(src, st.st_mode | S_IWUSR);
if (rename(src.c_str(), dst.c_str()))
throw SysError(format("renaming '%1%' to '%2%'") % src % dst);
if (changePerm)
chmod_(dst, st.st_mode);
}
void DerivationGoal::registerOutputs() void DerivationGoal::registerOutputs()
{ {
/* When using a build hook, the build hook can register the output /* When using a build hook, the build hook can register the output
@ -3715,8 +3738,7 @@ void DerivationGoal::registerOutputs()
if (settings.runDiffHook || settings.keepFailed) { if (settings.runDiffHook || settings.keepFailed) {
Path dst = worker.store.toRealPath(path + checkSuffix); Path dst = worker.store.toRealPath(path + checkSuffix);
deletePath(dst); deletePath(dst);
if (rename(actualPath.c_str(), dst.c_str())) moveCheckToStore(actualPath, dst);
throw SysError(format("renaming '%1%' to '%2%'") % actualPath % dst);
handleDiffHook( handleDiffHook(
buildUser ? buildUser->getUID() : getuid(), buildUser ? buildUser->getUID() : getuid(),
@ -3724,10 +3746,10 @@ void DerivationGoal::registerOutputs()
path, dst, worker.store.printStorePath(drvPath), tmpDir); path, dst, worker.store.printStorePath(drvPath), tmpDir);
throw NotDeterministic("derivation '%s' may not be deterministic: output '%s' differs from '%s'", throw NotDeterministic("derivation '%s' may not be deterministic: output '%s' differs from '%s'",
worker.store.printStorePath(drvPath), path, dst); worker.store.printStorePath(drvPath), worker.store.toRealPath(path), dst);
} else } else
throw NotDeterministic("derivation '%s' may not be deterministic: output '%s' differs", throw NotDeterministic("derivation '%s' may not be deterministic: output '%s' differs",
worker.store.printStorePath(drvPath), path); worker.store.printStorePath(drvPath), worker.store.toRealPath(path));
} }
/* Since we verified the build, it's now ultimately trusted. */ /* Since we verified the build, it's now ultimately trusted. */

View file

@ -28,3 +28,10 @@ nix cat-store $outPath/foobar | grep FOOBAR
# Test --check without hash rewriting. # Test --check without hash rewriting.
nix-build dependencies.nix --no-out-link --check --sandbox-paths /nix/store nix-build dependencies.nix --no-out-link --check --sandbox-paths /nix/store
# Test that sandboxed builds with --check and -K can move .check directory to store
nix-build check.nix -A nondeterministic --sandbox-paths /nix/store --no-out-link
(! nix-build check.nix -A nondeterministic --sandbox-paths /nix/store --no-out-link --check -K 2> $TEST_ROOT/log)
if grep -q 'error: renaming' $TEST_ROOT/log; then false; fi
grep -q 'may not be deterministic' $TEST_ROOT/log