Fix follows paths in subordinate lockfiles

This commit is contained in:
Alex Zero 2021-07-12 02:07:53 +01:00
parent 2cd1a5b8f3
commit b3c424f5a6
No known key found for this signature in database
GPG key ID: 2E54D6D25B06F1DB
4 changed files with 82 additions and 20 deletions

View file

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

View file

@ -43,7 +43,6 @@ struct FlakeInput
std::optional<FlakeRef> ref; std::optional<FlakeRef> ref;
bool isFlake = true; // true = process flake to get outputs, false = (fetched) static source path bool isFlake = true; // true = process flake to get outputs, false = (fetched) static source path
std::optional<InputPath> follows; std::optional<InputPath> follows;
bool absolute = false; // whether 'follows' is relative to the flake root
FlakeInputs overrides; FlakeInputs overrides;
}; };
@ -125,6 +124,15 @@ struct LockFlags
std::set<InputPath> inputUpdates; 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( LockedFlake lockFlake(
EvalState & state, EvalState & state,
const FlakeRef & flakeRef, const FlakeRef & flakeRef,

View file

@ -38,6 +38,9 @@ struct Input
bool immutable = false; bool immutable = false;
bool direct = true; bool direct = true;
/* path of the parent of this input, used for relative path resolution */
std::optional<Path> parent;
public: public:
static Input fromURL(const std::string & url); 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::pair<Tree, Input> fetch(ref<Store> store, const Input & input) override
{ {
std::string absPath;
auto path = getStrAttr(input.attrs, "path"); 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) if (storePath)
store->addTempRoot(*storePath); store->addTempRoot(*storePath);
if (!storePath || storePath->name() != "source" || !store->isValidPath(*storePath)) if (!storePath || storePath->name() != "source" || !store->isValidPath(*storePath))
// FIXME: try to substitute storePath. // FIXME: try to substitute storePath.
storePath = store->addToStore("source", path); storePath = store->addToStore("source", absPath);
return { return {
Tree(store->toRealPath(*storePath), std::move(*storePath)), Tree(store->toRealPath(*storePath), std::move(*storePath)),