From aa19d7e9e113af2561759fbffef5ecc16c54885e Mon Sep 17 00:00:00 2001 From: reionwong Date: Sat, 17 Oct 2026 10:03:03 +0800 Subject: [PATCH] fix(networkmanager): free the unlock prompt state once it completes --- clients/networkmanager/secretreader.cpp | 9 +- clients/networkmanager/tests/CMakeLists.txt | 7 ++ .../tests/secretreader_test.cpp | 109 ++++++++++++++++++ 3 files changed, 122 insertions(+), 3 deletions(-) create mode 100644 clients/networkmanager/tests/secretreader_test.cpp diff --git a/clients/networkmanager/secretreader.cpp b/clients/networkmanager/secretreader.cpp index 523fe2a..1a747de 100644 --- a/clients/networkmanager/secretreader.cpp +++ b/clients/networkmanager/secretreader.cpp @@ -175,8 +175,11 @@ void SecretReader::prompt(const std::string& prompt, std::function(); state->done = std::move(done); std::weak_ptr alive = m_alive; - const auto finish = [this, id, state, alive](bool dismissed, const Paths& result) { - if (alive.expired() || !state->done) + // The match owns the state; the timer it holds must not, or neither goes. + std::weak_ptr weakState = state; + const auto finish = [this, id, weakState, alive](bool dismissed, const Paths& result) { + const std::shared_ptr state = weakState.lock(); + if (!state || alive.expired() || !state->done) return; auto callback = std::move(state->done); state->done = nullptr; @@ -190,7 +193,7 @@ void SecretReader::prompt(const std::string& prompt, std::functionsubscribe(rule, [finish](const Message& message) { + m_prompts.emplace(id, m_bus->subscribe(rule, [finish, state](const Message& message) { bool dismissed = true; Variant result; message.read(dismissed, result); diff --git a/clients/networkmanager/tests/CMakeLists.txt b/clients/networkmanager/tests/CMakeLists.txt index 481784c..4781586 100644 --- a/clients/networkmanager/tests/CMakeLists.txt +++ b/clients/networkmanager/tests/CMakeLists.txt @@ -20,3 +20,10 @@ target_link_libraries(cutefish-networkmanager-client-bus-tests PRIVATE cutefish- # The client and the fake NetworkManager talk over a private session bus. add_test(NAME cutefish-networkmanager-client-bus-tests COMMAND ${DBUS_RUN_SESSION} -- $) + +add_executable(cutefish-networkmanager-secretreader-tests secretreader_test.cpp) +set_target_properties(cutefish-networkmanager-secretreader-tests PROPERTIES AUTOMOC OFF AUTORCC OFF) +target_link_libraries(cutefish-networkmanager-secretreader-tests PRIVATE cutefish-networkmanager-client GTest::gtest) +# A fake Secret Service on a private session bus. +add_test(NAME cutefish-networkmanager-secretreader-tests + COMMAND ${DBUS_RUN_SESSION} -- $) diff --git a/clients/networkmanager/tests/secretreader_test.cpp b/clients/networkmanager/tests/secretreader_test.cpp new file mode 100644 index 0000000..bb7f9aa --- /dev/null +++ b/clients/networkmanager/tests/secretreader_test.cpp @@ -0,0 +1,109 @@ +// SPDX-FileCopyrightText: 2026 Reion Wong +// SPDX-License-Identifier: GPL-3.0-or-later + +#include + +#include +#include +#include +#include +#include +#include +#include + +#include "base/dbus.h" +#include "base/event_loop.h" +#include "base/timer.h" +#include "networkmanager/secretreader.h" + +using namespace std::chrono_literals; +using namespace cutefish::base; +using cutefish::networkmanager::SecretReader; + +using Paths = std::vector; + +static EventLoop* loop = nullptr; + +static bool waitFor(const std::function& condition, std::chrono::milliseconds timeout = 3000ms) +{ + if (condition()) + return true; + bool met = false; + Timer poll; + poll.onTimeout = [&] { + if (condition()) { + met = true; + loop->quit(); + } + }; + poll.start(2ms); + Timer limit; + limit.setSingleShot(true); + limit.onTimeout = [&] { loop->quit(); }; + limit.start(timeout); + loop->run(); + return met || condition(); +} + +// A keyring whose only item is locked, and whose unlock prompt the user dismisses. +class FakeSecretService +{ +public: + FakeSecretService() + : m_bus(dbus::Connection::open(dbus::Connection::Bus::Session)) + , m_service(m_bus.get(), "/org/freedesktop/secrets", "org.freedesktop.Secret.Service") + , m_prompt(m_bus.get(), "/prompt/1", "org.freedesktop.Secret.Prompt") + { + m_service.addMethod("OpenSession", [](const std::string&, const dbus::Variant&) { + return std::tuple{dbus::Variant(std::string()), + dbus::ObjectPath{"/session/1"}}; + }); + m_service.addMethod("SearchItems", [](const std::map&) { + return std::tuple{Paths(), Paths{dbus::ObjectPath{"/item/1"}}}; + }); + m_service.addMethod("Unlock", [](const Paths&) { + return std::tuple{Paths(), dbus::ObjectPath{"/prompt/1"}}; + }); + m_service.publish(); + + m_prompt.addMethod("Prompt", [this](const std::string&) { + ++prompts; + m_bus->emitSignal("/prompt/1", "org.freedesktop.Secret.Prompt", "Completed", true, dbus::Variant(Paths())); + }); + m_prompt.addSignal("Completed", "bv"); + m_prompt.publish(); + } + + bool serve() { return m_bus->requestName("org.freedesktop.secrets"); } + + int prompts = 0; + +private: + std::unique_ptr m_bus; + dbus::Object m_service; + dbus::Object m_prompt; +}; + +TEST(SecretReaderTest, DismissedPromptAnswersWithNothing) +{ + FakeSecretService service; + ASSERT_TRUE(service.serve()); + + std::optional> loaded; + { + SecretReader reader; + reader.load("uuid", "vpn", [&](const std::map& secrets) { loaded = secrets; }); + ASSERT_TRUE(waitFor([&] { return loaded.has_value(); })); + } + + EXPECT_EQ(service.prompts, 1); + EXPECT_TRUE(loaded->empty()); +} + +int main(int argc, char** argv) +{ + testing::InitGoogleTest(&argc, argv); + EventLoop eventLoop; + loop = &eventLoop; + return RUN_ALL_TESTS(); +}