wroclaw-git-backups/nix
1
0
Fork 0
mirror of https://github.com/NixOS/nix synced 2025-06-25 06:31:14 +02:00
Code Activity

Add a special type of context for the result of toString

Browse source 
When you apply `builtins.toString` to a path value representing a path
in the Nix store (as is the case with flake inputs), historically you
got a string without context (e.g. `/nix/store/...-source`). This is
broken, since it allows you to pass a store path to a
derivation/toFile without a proper store reference. This is especially
a problem with lazy trees, since the store path is a virtual path that
doesn't exist and can be different every time.

For backwards compatibility, and to warn users about this unsafe use
of `toString`, we now keep track of such strings as a special type of
context.
This commit is contained in:
Eelco Dolstra 2025-05-07 18:53:39 +02:00
parent 8c568277fd
commit 2a35d8f800
10 changed files with 129 additions and 10 deletions
Download patch file Download diff file Expand all files Collapse all files

5
src/libexpr-test-support/include/nix/expr/tests/value/context.hh
View file

@ -23,6 +23,11 @@ struct Arbitrary<NixStringContextElem::DrvDeep> {
static Gen<NixStringContextElem::DrvDeep> arbitrary();
};
template<>
struct Arbitrary<NixStringContextElem::Path> {
static Gen<NixStringContextElem::Path> arbitrary();
};
template<>
struct Arbitrary<NixStringContextElem> {
static Gen<NixStringContextElem> arbitrary();

12
src/libexpr-test-support/tests/value/context.cc
View file

@ -15,6 +15,15 @@ Gen<NixStringContextElem::DrvDeep> Arbitrary<NixStringContextElem::DrvDeep>::arb
});
}
Gen<NixStringContextElem::Path> Arbitrary<NixStringContextElem::Path>::arbitrary()
{
return gen::map(gen::arbitrary<StorePath>(), [](StorePath storePath) {
return NixStringContextElem::Path{
.storePath = storePath,
};
});
}
Gen<NixStringContextElem> Arbitrary<NixStringContextElem>::arbitrary()
{
return gen::mapcat(
@ -30,6 +39,9 @@ Gen<NixStringContextElem> Arbitrary<NixStringContextElem>::arbitrary()
case 2:
return gen::map(
gen::arbitrary<NixStringContextElem::Built>(), [](NixStringContextElem a) { return a; });
case 3:
return gen::map(
gen::arbitrary<NixStringContextElem::Path>(), [](NixStringContextElem a) { return a; });
default:
assert(false);
}

3
src/libexpr/eval-cache.cc
View file

@ -628,6 +628,9 @@ string_t AttrCursor::getStringWithContext()
[&](const NixStringContextElem::Opaque & o) -> const StorePath & {
return o.path;
},
[&](const NixStringContextElem::Path & p) -> const StorePath & {
abort(); // FIXME
},
}, c.raw);
if (!root->state.store->isValidPath(path)) {
valid = false;

23
src/libexpr/eval.cc
View file

@ -952,8 +952,8 @@ void EvalState::mkPos(Value & v, PosIdx p)
// FIXME: only do this for virtual store paths?
attrs.alloc(sFile).mkString(path->path.abs(),
{
NixStringContextElem::Opaque{
.path = store->toStorePath(path->path.abs()).first
NixStringContextElem::Path{
.storePath = store->toStorePath(path->path.abs()).first
}
});
else
@ -2277,7 +2277,10 @@ std::string_view EvalState::forceStringNoCtx(Value & v, const PosIdx pos, std::s
{
auto s = forceString(v, pos, errorCtx);
if (v.context()) {
error<EvalError>("the string '%1%' is not allowed to refer to a store path (such as '%2%')", v.string_view(), v.context()[0]).withTrace(pos, errorCtx).debugThrow();
NixStringContext context;
copyContext(v, context);
if (hasContext(context))
error<EvalError>("the string '%1%' is not allowed to refer to a store path (such as '%2%')", v.string_view(), v.context()[0]).withTrace(pos, errorCtx).debugThrow();
}
return s;
}
@ -2336,7 +2339,16 @@ BackedStringView EvalState::coerceToString(
v.payload.path.path
: copyToStore
? store->printStorePath(copyPathToStore(context, v.path()))
: std::string(v.path().path.abs());
: ({
auto path = v.path();
if (path.accessor == rootFS && store->isInStore(path.path.abs())) {
context.insert(
NixStringContextElem::Path{
.storePath = store->toStorePath(path.path.abs()).first
});
}
std::string(path.path.abs());
});
}
if (v.type() == nAttrs) {
@ -2499,6 +2511,9 @@ std::pair<SingleDerivedPath, std::string_view> EvalState::coerceToSingleDerivedP
[&](NixStringContextElem::Built && b) -> SingleDerivedPath {
return std::move(b);
},
[&](NixStringContextElem::Path && p) -> SingleDerivedPath {
abort(); // FIXME
},
}, ((NixStringContextElem &&) *context.begin()).raw);
return {
std::move(derivedPath),

33
src/libexpr/include/nix/expr/value/context.hh
View file

@ -54,10 +54,35 @@ struct NixStringContextElem {
*/
using Built = SingleDerivedPath::Built;
/**
* A store path that will not result in a store reference when
* used in a derivation or toFile.
*
* When you apply `builtins.toString` to a path value representing
* a path in the Nix store (as is the case with flake inputs),
* historically you got a string without context
* (e.g. `/nix/store/...-source`). This is broken, since it allows
* you to pass a store path to a derivation/toFile without a
* proper store reference. This is especially a problem with lazy
* trees, since the store path is a virtual path that doesn't
* exist.
*
* For backwards compatibility, and to warn users about this
* unsafe use of `toString`, we keep track of such strings as a
* special type of context.
*/
struct Path
{
StorePath storePath;
GENERATE_CMP(Path, me->storePath);
};
using Raw = std::variant<
Opaque,
DrvDeep,
Built
Built,
Path
>;
Raw raw;
@ -82,4 +107,10 @@ struct NixStringContextElem {
typedef std::set<NixStringContextElem> NixStringContext;
/**
* Returns false if `context` has no elements other than
* `NixStringContextElem::Path`.
*/
bool hasContext(const NixStringContext & context);
}

19
src/libexpr/primops.cc
View file

@ -89,6 +89,9 @@ StringMap EvalState::realiseContext(const NixStringContext & context, StorePathS
if (maybePathsOut)
maybePathsOut->emplace(d.drvPath);
},
[&](const NixStringContextElem::Path & p) {
// FIXME
},
}, c.raw);
}
@ -1438,6 +1441,9 @@ static void derivationStrictInternal(
[&](const NixStringContextElem::Opaque & o) {
drv.inputSrcs.insert(state.devirtualize(o.path, &rewrites));
},
[&](const NixStringContextElem::Path & p) {
// FIXME: do something
},
}, c.raw);
}
@ -2346,10 +2352,21 @@ static void prim_toFile(EvalState & state, const PosIdx pos, Value * * args, Val
std::string contents(state.forceString(*args[1], context, pos, "while evaluating the second argument passed to builtins.toFile"));
StorePathSet refs;
StringMap rewrites;
for (auto c : context) {
if (auto p = std::get_if<NixStringContextElem::Opaque>(&c.raw))
refs.insert(p->path);
else if (auto p = std::get_if<NixStringContextElem::Path>(&c.raw)) {
if (contents.find(p->storePath.to_string()) != contents.npos) {
warn(
"Using 'builtins.toFile' to create a file named '%s' that references the store path '%s' without a proper context. "
"The resulting file will not have a correct store reference, so this is unreliable and may stop working in the future.",
name,
state.store->printStorePath(p->storePath));
state.devirtualize(p->storePath, &rewrites);
}
}
else
state.error<EvalError>(
"files created by %1% may not reference derivations, but %2% references %3%",
@ -2359,6 +2376,8 @@ static void prim_toFile(EvalState & state, const PosIdx pos, Value * * args, Val
).atPos(pos).debugThrow();
}
contents = rewriteStrings(contents, rewrites);
auto storePath = settings.readOnlyMode
? state.store->makeFixedOutputPathFromCA(name, TextInfo {
.hash = hashString(HashAlgorithm::SHA256, contents),

27
src/libexpr/primops/context.cc
View file

@ -7,9 +7,15 @@ namespace nix {
static void prim_unsafeDiscardStringContext(EvalState & state, const PosIdx pos, Value * * args, Value & v)
{
NixStringContext context;
NixStringContext context, filtered;
auto s = state.coerceToString(pos, *args[0], context, "while evaluating the argument passed to builtins.unsafeDiscardStringContext");
v.mkString(*s);
for (auto & c : context)
if (auto * p = std::get_if<NixStringContextElem::Path>(&c.raw))
filtered.insert(*p);
v.mkString(*s, filtered);
}
static RegisterPrimOp primop_unsafeDiscardStringContext({
@ -21,12 +27,19 @@ static RegisterPrimOp primop_unsafeDiscardStringContext({
.fun = prim_unsafeDiscardStringContext,
});
bool hasContext(const NixStringContext & context)
{
for (auto & c : context)
if (!std::get_if<NixStringContextElem::Path>(&c.raw))
return true;
return false;
}
static void prim_hasContext(EvalState & state, const PosIdx pos, Value * * args, Value & v)
{
NixStringContext context;
state.forceString(*args[0], context, pos, "while evaluating the argument passed to builtins.hasContext");
v.mkBool(!context.empty());
v.mkBool(hasContext(context));
}
static RegisterPrimOp primop_hasContext({
@ -103,7 +116,7 @@ static void prim_addDrvOutputDependencies(EvalState & state, const PosIdx pos, V
NixStringContext context;
auto s = state.coerceToString(pos, *args[0], context, "while evaluating the argument passed to builtins.addDrvOutputDependencies");
auto contextSize = context.size();
auto contextSize = context.size();
if (contextSize != 1) {
state.error<EvalError>(
"context of string '%s' must have exactly one element, but has %d",
@ -136,6 +149,10 @@ static void prim_addDrvOutputDependencies(EvalState & state, const PosIdx pos, V
above does not make much sense. */
return std::move(c);
},
[&](const NixStringContextElem::Path & p) -> NixStringContextElem::DrvDeep {
// FIXME: don't know what to do here.
abort();
},
}, context.begin()->raw) }),
};
@ -206,6 +223,8 @@ static void prim_getContext(EvalState & state, const PosIdx pos, Value * * args,
[&](NixStringContextElem::Opaque && o) {
contextInfos[std::move(o.path)].path = true;
},
[&](NixStringContextElem::Path && p) {
},
}, ((NixStringContextElem &&) i).raw);
}

5
src/libexpr/value-to-json.cc
View file

@ -7,9 +7,10 @@
#include <iomanip>
#include <nlohmann/json.hpp>
namespace nix {
using json = nlohmann::json;
json printValueAsJSON(EvalState & state, bool strict,
Value & v, const PosIdx pos, NixStringContext & context, bool copyToStore)
{
@ -33,6 +34,8 @@ json printValueAsJSON(EvalState & state, bool strict,
copyContext(v, context);
// FIXME: only use the context from `v`.
// FIXME: make devirtualization configurable?
// FIXME: don't devirtualize here? It's redundant if
// 'toFile' or 'derivation' also do it.
out = state.devirtualize(v.c_str(), context);
break;

9
src/libexpr/value/context.cc
View file

@ -57,6 +57,11 @@ NixStringContextElem NixStringContextElem::parse(
.drvPath = StorePath { s.substr(1) },
};
}
case '@': {
return NixStringContextElem::Path {
.storePath = StorePath { s.substr(1) },
};
}
default: {
// Ensure no '!'
if (s.find("!") != std::string_view::npos) {
@ -100,6 +105,10 @@ std::string NixStringContextElem::to_string() const
res += '=';
res += d.drvPath.to_string();
},
[&](const NixStringContextElem::Path & p) {
res += '@';
res += p.storePath.to_string();
},
}, raw);
return res;

3
src/nix/app.cc
View file

@ -92,6 +92,9 @@ UnresolvedApp InstallableValue::toApp(EvalState & state)
.path = o.path,
};
},
[&](const NixStringContextElem::Path & p) -> DerivedPath {
abort(); // FIXME
},
}, c.raw));
}