Merge branch 'fix/subflake-follows-fix' of https://github.com/ArctarusLimited/nix

This commit is contained in:
Eelco Dolstra 2021-08-23 12:47:09 +02:00
commit c6b063c31a
5 changed files with 175 additions and 22 deletions

View file

@ -329,21 +329,22 @@ LockedFlake lockFlake(
const FlakeInputs & flakeInputs,
std::shared_ptr<Node> node,
const InputPath & inputPathPrefix,
std::shared_ptr<const Node> oldNode)>
std::shared_ptr<const Node> oldNode,
const LockParent parent, const Path parentPath)>
computeLocks;
computeLocks = [&](
const FlakeInputs & flakeInputs,
std::shared_ptr<Node> node,
const InputPath & inputPathPrefix,
std::shared_ptr<const Node> oldNode)
std::shared_ptr<const Node> oldNode,
const LockParent parent, const Path parentPath)
{
debug("computing lock file node '%s'", printInputPath(inputPathPrefix));
/* Get the overrides (i.e. attributes of the form
'inputs.nixops.inputs.nixpkgs.url = ...'). */
// FIXME: check this
for (auto & [id, input] : flake.inputs) {
for (auto & [id, input] : flakeInputs) {
for (auto & [idOverride, inputOverride] : input.overrides) {
auto inputPath(inputPathPrefix);
inputPath.push_back(id);
@ -379,15 +380,23 @@ LockedFlake lockFlake(
path we haven't processed yet. */
if (input.follows) {
InputPath target;
if (hasOverride || input.absolute)
/* 'follows' from an override is relative to the
root of the graph. */
if (parent.absolute && !hasOverride) {
target = *input.follows;
else {
/* Otherwise, it's relative to the current flake. */
} else {
if (hasOverride)
{
target = inputPathPrefix;
target.pop_back();
}
else
{
target = parent.path;
}
for (auto & i : *input.follows) target.push_back(i);
}
debug("input '%s' follows '%s'", inputPathS, printInputPath(target));
node->inputs.insert_or_assign(id, target);
continue;
@ -433,7 +442,7 @@ LockedFlake lockFlake(
if (hasChildUpdate) {
auto inputFlake = getFlake(
state, oldLock->lockedRef, false, flakeCache);
computeLocks(inputFlake.inputs, childNode, inputPath, oldLock);
computeLocks(inputFlake.inputs, childNode, inputPath, oldLock, parent, parentPath);
} else {
/* No need to fetch this flake, we can be
lazy. However there may be new overrides on the
@ -450,12 +459,11 @@ LockedFlake lockFlake(
} else if (auto follows = std::get_if<1>(&i.second)) {
fakeInputs.emplace(i.first, FlakeInput {
.follows = *follows,
.absolute = true
});
}
}
computeLocks(fakeInputs, childNode, inputPath, oldLock);
computeLocks(fakeInputs, childNode, inputPath, oldLock, parent, parentPath);
}
} else {
@ -467,7 +475,18 @@ LockedFlake lockFlake(
throw Error("cannot update flake input '%s' in pure mode", inputPathS);
if (input.isFlake) {
auto inputFlake = getFlake(state, *input.ref, useRegistries, flakeCache);
Path localPath = parentPath;
FlakeRef localRef = *input.ref;
// If this input is a path, recurse it down.
// This allows us to resolve path inputs relative to the current flake.
if (localRef.input.getType() == "path")
{
localRef.input.parent = parentPath;
localPath = canonPath(parentPath + "/" + *input.ref->input.getSourcePath());
}
auto inputFlake = getFlake(state, localRef, useRegistries, flakeCache);
/* Note: in case of an --override-input, we use
the *original* ref (input2.ref) for the
@ -488,6 +507,13 @@ LockedFlake lockFlake(
parents.push_back(*input.ref);
Finally cleanup([&]() { parents.pop_back(); });
// Follows paths from existing inputs in the top-level lockfile are absolute,
// whereas paths in subordinate lockfiles are relative to those lockfiles.
LockParent newParent {
.path = inputPath,
.absolute = oldLock ? true : false
};
/* Recursively process the inputs of this
flake. Also, unless we already have this flake
in the top-level lock file, use this flake's
@ -497,7 +523,8 @@ LockedFlake lockFlake(
oldLock
? std::dynamic_pointer_cast<const Node>(oldLock)
: LockFile::read(
inputFlake.sourceInfo->actualPath + "/" + inputFlake.lockedRef.subdir + "/flake.lock").root);
inputFlake.sourceInfo->actualPath + "/" + inputFlake.lockedRef.subdir + "/flake.lock").root,
newParent, localPath);
}
else {
@ -515,9 +542,17 @@ LockedFlake lockFlake(
}
};
LockParent parent {
.path = {},
.absolute = true
};
// Bring in the current ref for relative path resolution if we have it
auto parentPath = canonPath(flake.sourceInfo->actualPath + "/" + flake.lockedRef.subdir);
computeLocks(
flake.inputs, newLockFile.root, {},
lockFlags.recreateLockFile ? nullptr : oldLockFile.root);
lockFlags.recreateLockFile ? nullptr : oldLockFile.root, parent, parentPath);
for (auto & i : lockFlags.inputOverrides)
if (!overridesUsed.count(i.first))

View file

@ -43,7 +43,6 @@ struct FlakeInput
std::optional<FlakeRef> ref;
bool isFlake = true; // true = process flake to get outputs, false = (fetched) static source path
std::optional<InputPath> follows;
bool absolute = false; // whether 'follows' is relative to the flake root
FlakeInputs overrides;
};
@ -125,6 +124,15 @@ struct LockFlags
std::set<InputPath> inputUpdates;
};
struct LockParent {
/* The path to this parent */
InputPath path;
/* Whether we are currently inside a top-level lockfile (inputs absolute)
or subordinate lockfile (inputs relative) */
bool absolute;
};
LockedFlake lockFlake(
EvalState & state,
const FlakeRef & flakeRef,

View file

@ -38,6 +38,9 @@ struct Input
bool immutable = false;
bool direct = true;
/* path of the parent of this input, used for relative path resolution */
std::optional<Path> parent;
public:
static Input fromURL(const std::string & url);

View file

@ -82,18 +82,34 @@ struct PathInputScheme : InputScheme
std::pair<Tree, Input> fetch(ref<Store> store, const Input & input) override
{
std::string absPath;
auto path = getStrAttr(input.attrs, "path");
// FIXME: check whether access to 'path' is allowed.
if (path[0] != '/' && input.parent)
{
auto parent = canonPath(*input.parent);
auto storePath = store->maybeParseStorePath(path);
// the path isn't relative, prefix it
absPath = canonPath(parent + "/" + path);
// for security, ensure that if the parent is a store path, it's inside it
if (!parent.rfind(store->storeDir, 0) && absPath.rfind(store->storeDir, 0))
throw BadStorePath("relative path '%s' points outside of its parent's store path %s, this is a security violation", path, parent);
}
else
{
absPath = path;
}
// FIXME: check whether access to 'path' is allowed.
auto storePath = store->maybeParseStorePath(absPath);
if (storePath)
store->addTempRoot(*storePath);
if (!storePath || storePath->name() != "source" || !store->isValidPath(*storePath))
// FIXME: try to substitute storePath.
storePath = store->addToStore("source", path);
storePath = store->addToStore("source", absPath);
return {
Tree(store->toRealPath(*storePath), std::move(*storePath)),

View file

@ -26,10 +26,15 @@ nonFlakeDir=$TEST_ROOT/nonFlake
flakeA=$TEST_ROOT/flakeA
flakeB=$TEST_ROOT/flakeB
flakeGitBare=$TEST_ROOT/flakeGitBare
flakeFollowsA=$TEST_ROOT/follows/flakeA
flakeFollowsB=$TEST_ROOT/follows/flakeA/flakeB
flakeFollowsC=$TEST_ROOT/follows/flakeA/flakeB/flakeC
flakeFollowsD=$TEST_ROOT/follows/flakeA/flakeD
flakeFollowsE=$TEST_ROOT/follows/flakeA/flakeE
for repo in $flake1Dir $flake2Dir $flake3Dir $flake7Dir $templatesDir $nonFlakeDir $flakeA $flakeB; do
for repo in $flake1Dir $flake2Dir $flake3Dir $flake7Dir $templatesDir $nonFlakeDir $flakeA $flakeB $flakeFollowsA; do
rm -rf $repo $repo.tmp
mkdir $repo
mkdir -p $repo
git -C $repo init
git -C $repo config user.email "foobar@example.com"
git -C $repo config user.name "Foobar"
@ -681,3 +686,89 @@ git -C $flakeB commit -a -m 'Foo'
# Test list-inputs with circular dependencies
nix flake metadata $flakeA
# Test flake follow paths
mkdir -p $flakeFollowsB
mkdir -p $flakeFollowsC
mkdir -p $flakeFollowsD
mkdir -p $flakeFollowsE
cat > $flakeFollowsA/flake.nix <<EOF
{
description = "Flake A";
inputs = {
B = {
url = "path:./flakeB";
inputs.foobar.follows = "D";
};
D.url = "path:./flakeD";
foobar.url = "path:./flakeE";
};
outputs = { ... }: {};
}
EOF
cat > $flakeFollowsB/flake.nix <<EOF
{
description = "Flake B";
inputs = {
foobar.url = "path:./../flakeE";
C = {
url = "path:./flakeC";
inputs.foobar.follows = "foobar";
};
};
outputs = { ... }: {};
}
EOF
cat > $flakeFollowsC/flake.nix <<EOF
{
description = "Flake C";
inputs = {
foobar.url = "path:./../../flakeE";
};
outputs = { ... }: {};
}
EOF
cat > $flakeFollowsD/flake.nix <<EOF
{
description = "Flake D";
inputs = {};
outputs = { ... }: {};
}
EOF
cat > $flakeFollowsE/flake.nix <<EOF
{
description = "Flake D";
inputs = {};
outputs = { ... }: {};
}
EOF
git -C $flakeFollowsA add flake.nix flakeB/flake.nix \
flakeB/flakeC/flake.nix flakeD/flake.nix flakeE/flake.nix
nix flake lock $flakeFollowsA
[[ $(jq -c .nodes.B.inputs.C $flakeFollowsA/flake.lock) = '"C"' ]]
[[ $(jq -c .nodes.B.inputs.foobar $flakeFollowsA/flake.lock) = '["D"]' ]]
[[ $(jq -c .nodes.C.inputs.foobar $flakeFollowsA/flake.lock) = '["B","foobar"]' ]]
# Ensure a relative path is not allowed to go outside the store path
cat > $flakeFollowsA/flake.nix <<EOF
{
description = "Flake A";
inputs = {
B.url = "path:./../../flakeB";
};
outputs = { ... }: {};
}
EOF
git -C $flakeFollowsA add flake.nix
nix flake lock $flakeFollowsA 2>&1 | grep 'this is a security violation'