Compare commits

...

2 commits

Author SHA1 Message Date
alois31 8543b3cc1a
libfetchers: allow automatic rejection of configuration options from flakes
The `allow-flake-configuration` option allows the user to control whether to
accept configuration options supplied by flakes. Unfortunately, setting this
to false really meant "ask each time" (with an option to remember the choice
for each specific option encountered). Let no mean no, and introduce (and
default to) a separate value for the "ask each time" behaviour.

Co-Authored-By: Jade Lovelace <lix@jade.fyi>
Change-Id: I7ccd67a95bfc92cffc1ebdc972d243f5191cc1b4
2024-06-30 11:35:02 +02:00
alois31 cb79437c1c
tree-wide: unify progress bar inactive and paused states
Previously, the progress bar had two subtly different states in which the bar
would not actually render, both with their own shortcomings: inactive (which
was irreversible) and paused (reversible, but swallowing logs). Furthermore,
there was no way of resetting the statistics, so a very bad solution was
implemented (243c0f18da) that would create a new
logger for each line of the repl, leaking the previous one and discarding the
value of printBuildLogs. Finally, if stderr was not attached to a TTY, the
update thread was started even though the logger was not active, violating the
invariant required by the destructor (which is not observed because the logger
is leaked).

In this commit, the two aforementioned states are unified into a single one,
which can be exited again, correctly upholds the invariant that the update
thread is only running while the progress bar is active, and does not swallow
logs. The latter change in behavior is not expected to be a problems in the
rare cases where the paused state was used before, since other loggers (like
the simple one) don't exhibit it anyway. The startProgressBar/stopProgressBar
API is removed due to being a footgun, and a new method for properly resetting
the progress is added.

Co-Authored-By: Qyriad <qyriad@qyriad.me>
Change-Id: I2b7c3eb17d439cd0c16f7b896cfb61239ac7ff3a
2024-06-29 19:18:08 +02:00
27 changed files with 169 additions and 120 deletions

View file

@ -0,0 +1,9 @@
---
synopsis: Allow automatic rejection of configuration options from flakes
cls: [1541]
credits: [alois31]
category: Improvements
---
Setting `accept-flake-config` to `false` now respects user choice by automatically rejecting configuration options set by flakes.
The old behaviour of asking each time is still available (and default) by setting it to the special value `ask`.

View file

@ -32,7 +32,6 @@
#include "local-fs-store.hh"
#include "signals.hh"
#include "print.hh"
#include "progress-bar.hh"
#include "gc-small-vector.hh"
#include "users.hh"
@ -300,7 +299,7 @@ ReplExitStatus NixRepl::mainLoop()
/* Stop the progress bar because it interferes with the display of
the repl. */
stopProgressBar();
logger->pause();
std::string input;
@ -684,9 +683,10 @@ ProcessLineResult NixRepl::processLine(std::string line)
// TODO: this only shows a progress bar for explicitly initiated builds,
// not eval-time fetching or builds performed for IFD.
// But we can't just show it everywhere, since that would erase partial output from evaluation.
startProgressBar();
logger->resetProgress();
logger->resume();
Finally stopLogger([&]() {
stopProgressBar();
logger->pause();
});
state->store->buildPaths({

View file

@ -51,30 +51,44 @@ void ConfigFile::apply()
else
assert(false);
if (!whitelist.count(baseName) && !nix::fetchSettings.acceptFlakeConfig) {
bool trusted = false;
auto trustedList = readTrustedList();
auto tlname = get(trustedList, name);
if (auto saved = tlname ? get(*tlname, valueS) : nullptr) {
trusted = *saved;
printInfo("Using saved setting for '%s = %s' from ~/.local/share/nix/trusted-settings.json.", name, valueS);
} else {
// FIXME: filter ANSI escapes, newlines, \r, etc.
if (std::tolower(logger->ask(fmt("do you want to allow configuration setting '%s' to be set to '" ANSI_RED "%s" ANSI_NORMAL "' (y/N)?", name, valueS)).value_or('n')) == 'y') {
trusted = true;
}
if (std::tolower(logger->ask(fmt("do you want to permanently mark this value as %s (y/N)?", trusted ? "trusted": "untrusted" )).value_or('n')) == 'y') {
trustedList[name][valueS] = trusted;
writeTrustedList(trustedList);
}
bool trusted = whitelist.count(baseName);
if (!trusted) {
switch (nix::fetchSettings.acceptFlakeConfig) {
case AcceptFlakeConfig::True: {
trusted = true;
break;
}
if (!trusted) {
warn("ignoring untrusted flake configuration setting '%s'.\nPass '%s' to trust it", name, "--accept-flake-config");
continue;
case AcceptFlakeConfig::Ask: {
auto trustedList = readTrustedList();
auto tlname = get(trustedList, name);
if (auto saved = tlname ? get(*tlname, valueS) : nullptr) {
trusted = *saved;
printInfo("Using saved setting for '%s = %s' from ~/.local/share/nix/trusted-settings.json.", name, valueS);
} else {
// FIXME: filter ANSI escapes, newlines, \r, etc.
if (std::tolower(logger->ask(fmt("Do you want to allow configuration setting '%s' to be set to '" ANSI_RED "%s" ANSI_NORMAL "' (y/N)? This may allow the flake to gain root, see the nix.conf manual page.", name, valueS)).value_or('n')) == 'y') {
trusted = true;
}
if (std::tolower(logger->ask(fmt("do you want to permanently mark this value as %s (y/N)?", trusted ? "trusted": "untrusted" )).value_or('n')) == 'y') {
trustedList[name][valueS] = trusted;
writeTrustedList(trustedList);
}
}
break;
}
case nix::AcceptFlakeConfig::False: {
trusted = false;
break;
};
}
}
globalConfig.set(name, valueS);
if (trusted) {
debug("accepting trusted flake configuration setting '%s'", name);
globalConfig.set(name, valueS);
} else {
warn("ignoring untrusted flake configuration setting '%s', pass '%s' to trust it (may allow the flake to gain root, see the nix.conf manual page)", name, "--accept-flake-config");
}
}
}

View file

@ -1,7 +1,50 @@
#include "abstract-setting-to-json.hh"
#include "args.hh"
#include "config-impl.hh"
#include "fetch-settings.hh"
#include <nlohmann/json.hpp>
namespace nix {
template<> AcceptFlakeConfig BaseSetting<AcceptFlakeConfig>::parse(const std::string & str) const
{
if (str == "true") return AcceptFlakeConfig::True;
else if (str == "ask") return AcceptFlakeConfig::Ask;
else if (str == "false") return AcceptFlakeConfig::False;
else throw UsageError("option '%s' has invalid value '%s'", name, str);
}
template<> std::string BaseSetting<AcceptFlakeConfig>::to_string() const
{
if (value == AcceptFlakeConfig::True) return "true";
else if (value == AcceptFlakeConfig::Ask) return "ask";
else if (value == AcceptFlakeConfig::False) return "false";
else abort();
}
template<> void BaseSetting<AcceptFlakeConfig>::convertToArg(Args & args, const std::string & category)
{
args.addFlag({
.longName = name,
.description = "Accept Lix configuration options from flakes without confirmation. This allows flakes to gain root access to your machine if you are a trusted user; see the nix.conf manual page for more details.",
.category = category,
.handler = {[this]() { override(AcceptFlakeConfig::True); }}
});
args.addFlag({
.longName = "ask-" + name,
.description = "Ask whether to accept Lix configuration options from flakes.",
.category = category,
.handler = {[this]() { override(AcceptFlakeConfig::Ask); }}
});
args.addFlag({
.longName = "no-" + name,
.description = "Reject Lix configuration options from flakes.",
.category = category,
.handler = {[this]() { override(AcceptFlakeConfig::False); }}
});
}
FetchSettings::FetchSettings()
{
}

View file

@ -11,6 +11,8 @@
namespace nix {
enum class AcceptFlakeConfig { True, Ask, False };
struct FetchSettings : public Config
{
FetchSettings();
@ -86,15 +88,21 @@ struct FetchSettings : public Config
"Whether to use flake registries to resolve flake references.",
{}, true, Xp::Flakes};
Setting<bool> acceptFlakeConfig{this, false, "accept-flake-config",
Setting<AcceptFlakeConfig> acceptFlakeConfig{
this, AcceptFlakeConfig::Ask, "accept-flake-config",
R"(
Whether to accept Lix configuration from the `nixConfig` attribute of
a flake without prompting. This is almost always a very bad idea.
Setting this setting as a trusted user allows Nix flakes to gain root
a flake. Doing so as a trusted user allows Nix flakes to gain root
access on your machine if they set one of the several
trusted-user-only settings that execute commands as root.
If set to `true`, such configuration will be accepted without asking;
this is almost always a very bad idea. Setting this to `ask` will
prompt the user each time whether to allow a certain configuration
option set this way, and offer to optionally remember their choice.
When set to `false`, the configuration will be automatically
declined.
See [multi-user installations](@docroot@/installation/multi-user.md)
for more details on the Lix security model.
)",

View file

@ -403,11 +403,8 @@ struct GitInputScheme : InputScheme
AutoDelete const _delete{msgPath};
writeFile(msgPath, *commitMsg);
// Pause the logger to allow for user input (such as a gpg passphrase) in `git commit`
logger->pause();
Finally restoreLogger([]() { logger->resume(); });
runProgram("git", true,
{ "-C", *root, "--git-dir", gitDir, "commit", std::string(path.rel()), "-F", msgPath });
{ "-C", *root, "--git-dir", gitDir, "commit", std::string(path.rel()), "-F", msgPath }, true);
}
}
}

View file

@ -44,50 +44,55 @@ static std::string_view storePathToName(std::string_view path)
ProgressBar::ProgressBar(bool isTTY)
: isTTY(isTTY)
{
state_.lock()->active = isTTY;
updateThread = std::thread([&]() {
auto state(state_.lock());
auto nextWakeup = A_LONG_TIME;
while (state->active) {
if (!state->haveUpdate)
state.wait_for(updateCV, nextWakeup);
nextWakeup = draw(*state, {});
state.wait_for(quitCV, std::chrono::milliseconds(50));
}
});
resume();
}
ProgressBar::~ProgressBar()
{
stop();
pause();
}
/* Called by destructor, can't be overridden */
void ProgressBar::stop()
void ProgressBar::pause()
{
{
auto state(state_.lock());
if (!state->active) return;
state->active = false;
writeToStderr("\r\e[K");
updateCV.notify_one();
quitCV.notify_one();
}
updateThread.join();
}
void ProgressBar::pause()
void ProgressBar::resetProgress()
{
state_.lock()->paused = true;
writeToStderr("\r\e[K");
auto state(state_.lock());
auto prevActive = state->active;
*state = ProgressBar::State {
.active = prevActive,
};
update(*state);
}
void ProgressBar::resume()
{
state_.lock()->paused = false;
writeToStderr("\r\e[K");
state_.lock()->haveUpdate = true;
updateCV.notify_one();
if (isTTY) {
auto state(state_.lock());
if (state->active) return;
state->active = true;
state->haveUpdate = true;
updateThread = std::thread([&]() {
auto state(state_.lock());
auto nextWakeup = A_LONG_TIME;
for (;;) {
if (!state->haveUpdate)
state.wait_for(updateCV, nextWakeup);
if (!state->active)
break;
nextWakeup = std::max(draw(*state, {}), std::chrono::milliseconds(50));
}
writeToStderr("\r\e[K");
});
}
}
bool ProgressBar::isVerbose()
@ -318,7 +323,7 @@ std::chrono::milliseconds ProgressBar::draw(State & state, const std::optional<s
auto nextWakeup = A_LONG_TIME;
state.haveUpdate = false;
if (state.paused || !state.active) return nextWakeup;
if (!state.active) return nextWakeup;
auto windowSize = getWindowSize();
auto width = windowSize.second;
@ -559,16 +564,4 @@ Logger * makeProgressBar()
return new ProgressBar(shouldANSI());
}
void startProgressBar()
{
logger = makeProgressBar();
}
void stopProgressBar()
{
auto progressBar = dynamic_cast<ProgressBar *>(logger);
if (progressBar) progressBar->stop();
}
}

View file

@ -48,16 +48,15 @@ struct ProgressBar : public Logger
uint64_t corruptedPaths = 0, untrustedPaths = 0;
bool active = true;
bool paused = false;
bool haveUpdate = true;
bool active = false;
bool haveUpdate = false;
};
Sync<State> state_;
std::thread updateThread;
std::condition_variable quitCV, updateCV;
std::condition_variable updateCV;
bool printBuildLogs = false;
bool printMultiline = false;
@ -67,10 +66,10 @@ struct ProgressBar : public Logger
~ProgressBar();
void stop() override final;
void pause() override;
void resetProgress() override;
void resume() override;
bool isVerbose() override;
@ -113,8 +112,4 @@ struct ProgressBar : public Logger
Logger * makeProgressBar();
void startProgressBar();
void stopProgressBar();
}

View file

@ -4,7 +4,6 @@
#include "gc-store.hh"
#include "signals.hh"
#include "loggers.hh"
#include "progress-bar.hh"
#include "current-process.hh"
#include <algorithm>
@ -349,7 +348,7 @@ RunPager::RunPager()
if (!pager) pager = getenv("PAGER");
if (pager && ((std::string) pager == "" || (std::string) pager == "cat")) return;
stopProgressBar();
logger->pause();
Pipe toPager;
toPager.create();

View file

@ -117,9 +117,8 @@ public:
virtual ~Logger() { }
virtual void stop() { };
virtual void pause() { };
virtual void resetProgress() { };
virtual void resume() { };
// Whether the logger prints the whole build log

View file

@ -543,7 +543,7 @@ static void main_nix_build(int argc, char * * argv)
restoreProcessContext();
logger->stop();
logger->pause();
execvp(shell->c_str(), argPtrs.data());
@ -606,7 +606,7 @@ static void main_nix_build(int argc, char * * argv)
outPaths.push_back(outputPath);
}
logger->stop();
logger->pause();
for (auto & path : outPaths)
std::cout << store->printStorePath(path) << '\n';

View file

@ -3,7 +3,6 @@
#include "shared.hh"
#include "store-api.hh"
#include "local-fs-store.hh"
#include "progress-bar.hh"
#include <nlohmann/json.hpp>
@ -143,7 +142,7 @@ struct CmdBuild : InstallablesCommand, MixDryRun, MixJSON, MixProfile
createOutLinks(outLink, buildables, *store2);
if (printOutputPaths) {
stopProgressBar();
logger->pause();
for (auto & buildable : buildables) {
std::visit(overloaded {
[&](const BuiltPath::Opaque & bo) {

View file

@ -2,7 +2,6 @@
#include "store-api.hh"
#include "fs-accessor.hh"
#include "nar-accessor.hh"
#include "progress-bar.hh"
using namespace nix;
@ -20,7 +19,7 @@ struct MixCat : virtual Args
auto file = accessor->readFile(path);
stopProgressBar();
logger->pause();
writeFull(STDOUT_FILENO, file);
}
};

View file

@ -6,7 +6,6 @@
#include "store-api.hh"
#include "outputs-spec.hh"
#include "derivations.hh"
#include "progress-bar.hh"
#include "run.hh"
#include <iterator>
@ -690,7 +689,7 @@ struct CmdPrintDevEnv : Common, MixJSON
{
auto buildEnvironment = getBuildEnvironment(store, installable).first;
stopProgressBar();
logger->pause();
if (json) {
logger->writeToStdout(buildEnvironment.toJSON());

View file

@ -1,7 +1,6 @@
#include "command.hh"
#include "store-api.hh"
#include "archive.hh"
#include "progress-bar.hh"
using namespace nix;
@ -21,7 +20,7 @@ struct CmdDumpPath : StorePathCommand
void run(ref<Store> store, const StorePath & storePath) override
{
stopProgressBar();
logger->pause();
FdSink sink(STDOUT_FILENO);
store->narFromPath(storePath, sink);
sink.flush();
@ -57,7 +56,7 @@ struct CmdDumpPath2 : Command
void run() override
{
stopProgressBar();
logger->pause();
FdSink sink(STDOUT_FILENO);
dumpPath(path, sink);
sink.flush();

View file

@ -2,7 +2,6 @@
#include "shared.hh"
#include "eval.hh"
#include "attr-path.hh"
#include "progress-bar.hh"
#include "editor-for.hh"
#include "current-process.hh"
@ -42,7 +41,7 @@ struct CmdEdit : InstallableCommand
}
}();
stopProgressBar();
logger->pause();
auto args = editorFor(file, line);

View file

@ -6,7 +6,6 @@
#include "eval.hh"
#include "eval-inline.hh"
#include "value-to-json.hh"
#include "progress-bar.hh"
#include <nlohmann/json.hpp>
@ -76,7 +75,7 @@ struct CmdEval : MixJSON, InstallableCommand, MixReadOnlyOption
}
if (writeTo) {
stopProgressBar();
logger->pause();
if (pathExists(*writeTo))
throw Error("path '%s' already exists", *writeTo);
@ -114,7 +113,7 @@ struct CmdEval : MixJSON, InstallableCommand, MixReadOnlyOption
}
else if (raw) {
stopProgressBar();
logger->pause();
writeFull(STDOUT_FILENO, *state->coerceToString(noPos, *v, context, "while generating the eval command output"));
}

View file

@ -3,7 +3,6 @@
#include "shared.hh"
#include "store-api.hh"
#include "log-store.hh"
#include "progress-bar.hh"
using namespace nix;
@ -55,7 +54,7 @@ struct CmdLog : InstallableCommand
auto log = logSub.getBuildLog(path);
if (!log) continue;
stopProgressBar();
logger->pause();
printInfo("got build log for '%s' from '%s'", installable->what(), logSub.getUri());
writeFull(STDOUT_FILENO, *log);
return;

View file

@ -346,8 +346,6 @@ void mainWrapped(int argc, char * * argv)
}
#endif
Finally f([] { logger->stop(); });
programPath = argv[0];
auto programName = std::string(baseNameOf(programPath));
@ -363,7 +361,8 @@ void mainWrapped(int argc, char * * argv)
evalSettings.pureEval = true;
setLogFormat("bar");
setLogFormat(LogFormat::bar);
Finally f([] { logger->pause(); });
settings.verboseBuild = false;
if (isatty(STDERR_FILENO)) {
verbosity = lvlNotice;

View file

@ -1,10 +1,10 @@
#include "command.hh"
#include "common-args.hh"
#include "loggers.hh"
#include "shared.hh"
#include "store-api.hh"
#include "filetransfer.hh"
#include "finally.hh"
#include "progress-bar.hh"
#include "tarfile.hh"
#include "attr-path.hh"
#include "eval-inline.hh"
@ -180,10 +180,8 @@ static int main_nix_prefetch_url(int argc, char * * argv)
if (args.size() > 2)
throw UsageError("too many arguments");
Finally f([]() { stopProgressBar(); });
if (isatty(STDERR_FILENO))
startProgressBar();
setLogFormat(LogFormat::bar);
auto store = openStore();
auto state = std::make_unique<EvalState>(myArgs.searchPath, store);
@ -237,7 +235,7 @@ static int main_nix_prefetch_url(int argc, char * * argv)
auto [storePath, hash] = prefetchFile(
store, resolveMirrorUrl(*state, url), name, ht, expectedHash, unpack, executable);
stopProgressBar();
logger->pause();
if (!printPath)
printInfo("path is '%s'", store->printStorePath(storePath));

View file

@ -8,7 +8,6 @@
#include "local-store.hh"
#include "finally.hh"
#include "fs-accessor.hh"
#include "progress-bar.hh"
#include "eval.hh"
#include "build/personality.hh"
#include "current-process.hh"
@ -31,7 +30,7 @@ void runProgramInStore(ref<Store> store,
const Strings & args,
std::optional<std::string_view> system)
{
stopProgressBar();
logger->pause();
restoreProcessContext();

View file

@ -3,7 +3,6 @@
#include "store-api.hh"
#include "thread-pool.hh"
#include "signals.hh"
#include "progress-bar.hh"
#include <atomic>
@ -222,7 +221,7 @@ struct CmdKey : NixMultiCommand
if (!command)
throw UsageError("'nix key' requires a sub-command.");
stopProgressBar();
logger->pause();
command->second->run();
}
};

View file

@ -13,7 +13,6 @@
#include "eval-settings.hh"
#include "attr-path.hh"
#include "names.hh"
#include "progress-bar.hh"
using namespace nix;
@ -88,7 +87,7 @@ struct CmdUpgradeNix : MixDryRun, EvalCommand
auto version = DrvName(storePath.name()).version;
if (dryRun) {
stopProgressBar();
logger->pause();
warn("would upgrade to version %s", version);
return;
}
@ -106,7 +105,7 @@ struct CmdUpgradeNix : MixDryRun, EvalCommand
throw Error("could not verify that '%s' works", program);
}
stopProgressBar();
logger->pause();
auto const fullStorePath = store->printStorePath(storePath);

View file

@ -1,6 +1,5 @@
#include "command.hh"
#include "store-api.hh"
#include "progress-bar.hh"
#include "fs-accessor.hh"
#include "shared.hh"
@ -110,7 +109,7 @@ struct CmdWhyDepends : SourceExprCommand, MixOperateOnOptions
auto dependencyPath = *optDependencyPath;
auto dependencyPathHash = dependencyPath.hashPart();
stopProgressBar(); // FIXME
logger->pause(); // FIXME
auto accessor = store->getFSAccessor();

View file

@ -32,7 +32,7 @@ NIX_CONFIG='
experimental-features = nix-command
accept-flake-config = true
' nix config show accept-flake-config 1>$TEST_ROOT/stdout 2>$TEST_ROOT/stderr
grepQuiet "false" $TEST_ROOT/stdout
grepQuiet "ask" $TEST_ROOT/stdout
grepQuiet "Ignoring setting 'accept-flake-config' because experimental feature 'flakes' is not enabled" $TEST_ROOT/stderr
# 'flakes' experimental-feature is disabled after, ignore and warn
@ -40,7 +40,7 @@ NIX_CONFIG='
accept-flake-config = true
experimental-features = nix-command
' nix config show accept-flake-config 1>$TEST_ROOT/stdout 2>$TEST_ROOT/stderr
grepQuiet "false" $TEST_ROOT/stdout
grepQuiet "ask" $TEST_ROOT/stdout
grepQuiet "Ignoring setting 'accept-flake-config' because experimental feature 'flakes' is not enabled" $TEST_ROOT/stderr
# 'flakes' experimental-feature is enabled before, process

View file

@ -28,6 +28,11 @@ nix build < /dev/null
(! [[ -f post-hook-ran ]])
clearStore
# likewise with no-accept-flake-config
nix build --no-accept-flake-config
(! [[ -f post-hook-ran ]])
clearStore
nix build --accept-flake-config
test -f post-hook-ran || fail "The post hook should have ran"

View file

@ -2,6 +2,7 @@
#include "eval.hh"
#include "progress-bar.hh"
#include "loggers.hh"
#include "logging.hh"
#include "shared.hh"
@ -23,7 +24,7 @@ namespace nix
initNix();
initGC();
startProgressBar();
setLogFormat(LogFormat::bar);
ASSERT_NE(dynamic_cast<ProgressBar *>(logger), nullptr);
ProgressBar & progressBar = dynamic_cast<ProgressBar &>(*logger);