From 0cad504c8a826ed174b0221fa20d53407cc18138 Mon Sep 17 00:00:00 2001 From: blokyk Date: Mon, 25 May 2026 20:16:54 +0200 Subject: [PATCH] libexpr/flake-config: improve untrusted flake config prompt ux the case-sensitiveness of the "accept all options" prompt (and the fact it silently defaults to no) was confusing a few users, so by rewording it slightly we can avoid that case-sensitiveness and make the prompt clearer. for consistency, i also rewrote the individual accept prompt to make it case-insensitive. at reviewers' requests, i also reworked the code so it re-prompts the user on invalid answers. Change-Id: Ie786c0f26a0aa752b22d5a5e8af1b2c7ca2846b5 --- lix/libexpr/flake/config.cc | 182 +++++++++++++++++++++++------------- lix/libmain/progress-bar.cc | 6 +- lix/libmain/progress-bar.hh | 2 +- lix/libutil/logging.hh | 6 +- 4 files changed, 128 insertions(+), 68 deletions(-) diff --git a/lix/libexpr/flake/config.cc b/lix/libexpr/flake/config.cc index 6d66c6fac..9fd14cfef 100644 --- a/lix/libexpr/flake/config.cc +++ b/lix/libexpr/flake/config.cc @@ -4,6 +4,7 @@ #include "lix/libutil/json.hh" #include "lix/libutil/users.hh" #include "lix/libfetchers/fetch-settings.hh" +#include namespace nix::flake { @@ -30,6 +31,64 @@ static void writeTrustedList(const TrustedList & trustedList) writeFile(path, JSON(trustedList).dump()); } +static bool +askForEachSetting(TrustedList & trustedList, std::map & untrustedSettings) +{ + // clang-format off + constexpr auto prompt = + "[" ANSI_BOLD "y" ANSI_NORMAL "]es for now/" + "[" ANSI_BOLD "N" ANSI_NORMAL "]o for now/" + "[" ANSI_BOLD "a" ANSI_NORMAL "]lways allow"; + // clang-format on + + auto didTrustedListChange = false; + int acceptedCount = 0; + for (const auto & [name, valueS] : untrustedSettings) { + auto reply = + logger + ->ask( + fmt("Do you want to allow setting '" ANSI_MAGENTA "%s = %s" ANSI_NORMAL "'? (%s) ", + name, + valueS, + prompt) + ) + .value_or("n"); + + reply = toLower(reply); + + static const std::string yes = "yes for now"; + static const std::string no = "no for now"; + static const std::string always = "always allow"; + + while (true) { + if (no.starts_with(reply)) { + break; + } + + if (yes.starts_with(reply) || always.starts_with(reply)) { + if (reply[0] == 'a') { + trustedList[name][valueS] = true; + didTrustedListChange = true; + } + + globalConfig.set(name, valueS); + acceptedCount++; + break; + } + + // if the reply wasn't a prefix of any answer, ask the user again + reply = logger->ask(fmt("Couldn't understand reply.\n%s: ", prompt)).value_or("n"); + } + } + + if (didTrustedListChange) { + writeTrustedList(trustedList); + } + + // return false if *none* of the settings were accepted + return acceptedCount > 0; +} + static bool batchAskForSetting( bool & negativeTrustOverride, TrustedList & trustedList, @@ -43,77 +102,74 @@ static bool batchAskForSetting( printWarning("%s", warning); - auto reply = logger - ->ask( - fmt("Do you want to allow configuration settings to be applied?\nThis may allow the " - "flake to gain root, see the nix.conf manual page (" ANSI_BOLD "y" ANSI_NORMAL - "es for now/" ANSI_BOLD "A" ANSI_NORMAL "llow always/" ANSI_BOLD "n" ANSI_NORMAL - "o/" ANSI_BOLD "N" ANSI_NORMAL "o to all) ") - ) - .value_or('n'); + // clang-format off + constexpr auto globalPrompt = + "[" ANSI_BOLD "y" ANSI_NORMAL "]es for now/" + "[" ANSI_BOLD "n" ANSI_NORMAL "]o for now/" + "[" ANSI_BOLD "a" ANSI_NORMAL "]lways allow/" + "[" ANSI_BOLD "I" ANSI_NORMAL "]ndividually review"; + // clang-format on - if (reply == 'N') { - printWarning("Rejecting all untrusted nix.conf entries"); - printTaggedWarning( - "you can set '%s' to '%b' to automatically reject configuration options supplied by " - "flakes", - "accept-flake-config", - false - ); - negativeTrustOverride = true; - return false; - } + auto reply = + logger + ->ask( + fmt("Do you want to allow these configuration settings to be applied?\n" ANSI_BOLD + "This may allow the flake to gain root" ANSI_NORMAL ", see the nix.conf manual page.\n" + "(%s) ", + globalPrompt) + ) + .value_or("I"); // if the answer is empty (or there is no interactive prompt), + // just default to reviewing each individually + + reply = toLower(reply); + + static const std::string yes = "yes for now"; + static const std::string no = "no for now"; + static const std::string always = "always allow"; + static const std::string review = "individually review"; + + // when interactive, loops and reprompts until the reply is one of y/n/a/i (or any prefix of the answers) + while (true) { + if (no.starts_with(reply)) { + printWarning("Rejecting all untrusted nix.conf entries"); + printTaggedWarning( + "you can set '%s' to '%b' to automatically reject configuration options supplied by " + "flakes", + "accept-flake-config", + false + ); + negativeTrustOverride = true; + return false; + } + + if (yes.starts_with(reply) || always.starts_with(reply)) { + auto alwaysAllow = reply[0] == 'a'; + for (const auto & [name, valueS] : untrustedSettings) { + if (alwaysAllow) { + trustedList[name][valueS] = true; + } + globalConfig.set(name, valueS); + } - if (reply == 'y' || reply == 'A') { - auto alwaysAllow = reply == 'A'; - for (const auto & [name, valueS] : untrustedSettings) { if (alwaysAllow) { - trustedList[name][valueS] = true; - } - globalConfig.set(name, valueS); - } - - if (alwaysAllow) { - writeTrustedList(trustedList); - } - - return true; - } else { - printTaggedWarning( - "you can set '%s' to '%b' to automatically reject configuration options supplied " - "by flakes", - "accept-flake-config", - false - ); - } - - auto didTrustedListChange = false; - for (const auto & [name, valueS] : untrustedSettings) { - auto individualReply = logger - ->ask( - fmt("Do you want to allow setting '%s = %s'? (" ANSI_BOLD - "y" ANSI_NORMAL "es for now/" ANSI_BOLD "A" ANSI_NORMAL - "llow always/" ANSI_BOLD "n" ANSI_NORMAL "o for now) ", - name, - valueS) - ) - .value_or('n'); - - if (individualReply == 'y' || individualReply == 'A') { - if (individualReply == 'A') { - trustedList[name][valueS] = true; - didTrustedListChange = true; + printTaggedWarning( + "adding these configuration settings to the trusted list at %s, " + "edit it if you want to remove them in the future", + trustedListPath() + ); + writeTrustedList(trustedList); } - globalConfig.set(name, valueS); + return true; } - } - if (didTrustedListChange) { - writeTrustedList(trustedList); - } + if (review.starts_with(reply)) { + return askForEachSetting(trustedList, untrustedSettings); + } - return false; + // if the reply wasn't a prefix of any, ask the user again + reply = logger->ask(fmt("Couldn't understand reply.\n%s: ", globalPrompt)).value_or("n"); + } } void ConfigFile::apply() diff --git a/lix/libmain/progress-bar.cc b/lix/libmain/progress-bar.cc index 54a580a73..39af11c20 100644 --- a/lix/libmain/progress-bar.cc +++ b/lix/libmain/progress-bar.cc @@ -562,7 +562,7 @@ void ProgressBar::writeToStdout(std::string_view s) restoreProgressDisplay(*state); } -std::optional ProgressBar::ask(std::string_view msg) +std::optional ProgressBar::ask(std::string_view msg) { auto state(state_.lock()); if (state->paused > 0 || !isatty(STDIN_FILENO)) return {}; @@ -571,9 +571,9 @@ std::optional ProgressBar::ask(std::string_view msg) std::cerr << msg; auto s = trim(readLine(STDIN_FILENO)); writeLogsToStderr("\e[?2026h"); // begin synchronized update - if (s.size() != 1) return {}; restoreProgressDisplay(*state); - return s[0]; + // only return the string if it's not empty + return s.size() != 0 ? s : std::optional{}; } void ProgressBar::setPrintBuildLogs(bool printBuildLogs) diff --git a/lix/libmain/progress-bar.hh b/lix/libmain/progress-bar.hh index a3f2b580d..a577aa58c 100644 --- a/lix/libmain/progress-bar.hh +++ b/lix/libmain/progress-bar.hh @@ -103,7 +103,7 @@ struct ProgressBar : public Logger void writeToStdout(std::string_view s) override; - std::optional ask(std::string_view msg) override; + std::optional ask(std::string_view msg) override; void setPrintBuildLogs(bool printBuildLogs) override; diff --git a/lix/libutil/logging.hh b/lix/libutil/logging.hh index 4fb4768cf..0b93936a7 100644 --- a/lix/libutil/logging.hh +++ b/lix/libutil/logging.hh @@ -215,7 +215,11 @@ public: writeToStdout(fmt(args...)); } - virtual std::optional ask(std::string_view s) + /** + * Try to interactively ask the given question to the user. + * If not interactive, or if the answer is an empty string, return std::optional{} + */ + virtual std::optional ask(std::string_view s) { return {}; } virtual void setPrintBuildLogs(bool printBuildLogs)