From d47ce9d1292fd7530d784a30fe9bb7f1cbc5e646 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 19 Oct 2026 22:20:32 +0000 Subject: [PATCH] Apply remaining changes Co-authored-by: rakshasa <475125+rakshasa@users.noreply.github.com> --- doc/rtorrent.rc | 7 +++++++ src/command_local.cc | 3 +++ src/session/download_storer.cc | 17 ++++++++++++----- src/session/download_storer.h | 3 ++- src/session/session_manager.cc | 18 +++++++++++++++++- src/session/session_manager.h | 9 +++++++++ test/src/test_session_commit.cc | 2 +- test/src/test_session_storer.cc | 19 +++++++++++++++++-- test/src/test_session_storer.h | 2 ++ 9 files changed, 70 insertions(+), 10 deletions(-) diff --git a/doc/rtorrent.rc b/doc/rtorrent.rc index 831ce302..042dcb32 100644 --- a/doc/rtorrent.rc +++ b/doc/rtorrent.rc @@ -45,6 +45,13 @@ # #session.path.set = ./session +# Umask for files written to the session directory, default 0077 (files +# are mode 0600). Use 0007 for 0660 or 0027 for 0640. The owner must keep +# read/write access. Session files contain tracker announce URLs +# (potentially passkeys), so loosening permissions is a user choice. +# +#session.umask.set = 0077 + # Watch a directory for new torrents, and stop those that have been # deleted. Use directory.watch.ready for network shares or other watch # directories where files may be copied or written directly into place. diff --git a/src/command_local.cc b/src/command_local.cc index 505a138b..63ae04c3 100644 --- a/src/command_local.cc +++ b/src/command_local.cc @@ -123,6 +123,8 @@ initialize_command_local() { CMD_ANY_STRING_V("session.path.set", [](auto, auto& str) { return session_thread::manager()->set_path(str); }); CMD_ANY ("session.use_lock", [](auto, auto) { return session_thread::manager()->use_lock(); }); CMD_ANY_VALUE_V ("session.use_lock.set", [](auto, auto& value) { return session_thread::manager()->set_use_lock(value); }); + CMD_ANY ("session.umask", [](auto, auto) { return session_thread::manager()->umask(); }); + CMD_ANY_VALUE_V ("session.umask.set", [](auto, auto& value) { return session_thread::manager()->set_umask(value); }); CMD_VAR_BOOL ("session.on_completion", true); CMD_ANY_V ("session.save", [dList](auto, auto) { return dList->session_save(); }); @@ -171,6 +173,7 @@ initialize_command_local() { rpc::rpc.mark_safe("directory.default.realpath.or_empty"); rpc::rpc.mark_safe("directory.default.realpath.or_throw"); rpc::rpc.mark_safe("session.use_lock"); + rpc::rpc.mark_safe("session.umask"); rpc::rpc.mark_safe("session.on_completion"); rpc::rpc.mark_safe("pieces.sync.always_safe"); diff --git a/src/session/download_storer.cc b/src/session/download_storer.cc index 459f6df1..48e686aa 100644 --- a/src/session/download_storer.cc +++ b/src/session/download_storer.cc @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -123,7 +124,7 @@ is_correct_format(const std::string& f) { } void -save_stream(const std::string& path, bool use_fsyncdisk, const std::stringstream& stream) { +save_stream(const std::string& path, bool use_fsyncdisk, mode_t file_mode, const std::stringstream& stream) { // Remove any leftover temporary file first so that O_EXCL only ever fails on // an entry that appeared after the unlink, and O_NOFOLLOW keeps a symlink // planted in the session directory from redirecting the write. @@ -136,6 +137,12 @@ save_stream(const std::string& path, bool use_fsyncdisk, const std::stringstream if (fd < 0) throw torrent::storage_error("failed to open file for writing : " + path); + // open() applies the process umask, so set the configured mode explicitly. + if (::fchmod(fd, file_mode) == -1) { + ::close(fd); + throw torrent::storage_error("failed to set file permissions : " + path); + } + const auto data = stream.view(); std::size_t remaining = data.size(); const char* cursor = data.data(); @@ -196,7 +203,7 @@ sync_directory_of(const std::string& path) { } // namespace anonymous void -DownloadStorer::save_and_move_streams(const std::string& path, bool use_fsyncdisk, +DownloadStorer::save_and_move_streams(const std::string& path, bool use_fsyncdisk, mode_t file_mode, const std::stringstream* torrent_stream, const std::stringstream* rtorrent_stream, const std::stringstream* libtorrent_stream) { @@ -205,10 +212,10 @@ DownloadStorer::save_and_move_streams(const std::string& path, bool use_fsyncdis auto rtorrent_path = path + ".rtorrent"; if (torrent_stream) - save_stream(torrent_path + ".new", use_fsyncdisk, *torrent_stream); + save_stream(torrent_path + ".new", use_fsyncdisk, file_mode, *torrent_stream); - save_stream(libtorrent_path + ".new", use_fsyncdisk, *libtorrent_stream); - save_stream(rtorrent_path + ".new", use_fsyncdisk, *rtorrent_stream); + save_stream(libtorrent_path + ".new", use_fsyncdisk, file_mode, *libtorrent_stream); + save_stream(rtorrent_path + ".new", use_fsyncdisk, file_mode, *rtorrent_stream); if (torrent_stream) { if (::rename((torrent_path + ".new").c_str(), torrent_path.c_str()) == -1) diff --git a/src/session/download_storer.h b/src/session/download_storer.h index 9472ce21..1ed2a9e8 100644 --- a/src/session/download_storer.h +++ b/src/session/download_storer.h @@ -3,6 +3,7 @@ #include #include +#include #include #include @@ -33,7 +34,7 @@ public: auto rtorrent_stream() { return std::move(m_rtorrent_stream); } auto libtorrent_stream() { return std::move(m_libtorrent_stream); } - static void save_and_move_streams(const std::string& path, bool use_fsyncdisk, + static void save_and_move_streams(const std::string& path, bool use_fsyncdisk, mode_t file_mode, const std::stringstream* torrent_stream, const std::stringstream* rtorrent_stream, const std::stringstream* libtorrent_stream); diff --git a/src/session/session_manager.cc b/src/session/session_manager.cc index 2536d133..b9301d53 100644 --- a/src/session/session_manager.cc +++ b/src/session/session_manager.cc @@ -65,6 +65,22 @@ SessionManager::set_use_lock(bool use_lock) { m_use_lock = use_lock; } +void +SessionManager::set_umask(int64_t umask) { + assert(torrent::this_thread::thread() == torrent::main_thread::thread()); + + if (m_freeze_info) + throw torrent::input_error("Session umask option cannot be changed after startup."); + + if (umask < 0 || umask > 0777) + throw torrent::input_error("Session umask must be in the range 0 to 0777."); + + if ((umask & 0600) != 0) + throw torrent::input_error("Session umask must not mask owner read/write permissions (0600)."); + + m_umask = umask; +} + void SessionManager::save_resume_download(core::Download* download) { assert(torrent::this_thread::thread() == torrent::main_thread::thread()); @@ -379,7 +395,7 @@ SessionManager::process_next_save_request_unsafe() { }; try { - DownloadStorer::save_and_move_streams(itr->second.path, m_use_fsyncdisk, + DownloadStorer::save_and_move_streams(itr->second.path, m_use_fsyncdisk, file_mode(), itr->second.torrent_stream.get(), itr->second.rtorrent_stream.get(), itr->second.libtorrent_stream.get()); diff --git a/src/session/session_manager.h b/src/session/session_manager.h index 4c49e647..cbce1adf 100644 --- a/src/session/session_manager.h +++ b/src/session/session_manager.h @@ -6,6 +6,8 @@ #include #include #include +#include +#include #include #include #include @@ -59,6 +61,10 @@ public: bool use_lock() const; void set_use_lock(bool use_lock); + int umask() const; + void set_umask(int64_t umask); + mode_t file_mode() const; + void save_full_download(core::Download* download); void save_resume_download(core::Download* download); void remove_download(core::Download* download); @@ -99,6 +105,7 @@ private: std::string m_path; bool m_use_fsyncdisk{true}; bool m_use_lock{true}; + int m_umask{0077}; align_cacheline @@ -133,6 +140,8 @@ inline bool SessionManager::is_used() const { return !m_path.e inline std::string SessionManager::path() const { return m_path; } inline bool SessionManager::use_fsyncdisk() const { return m_use_fsyncdisk; } inline bool SessionManager::use_lock() const { return m_use_lock; } +inline int SessionManager::umask() const { return m_umask; } +inline mode_t SessionManager::file_mode() const { return 0666 & ~m_umask; } inline void SessionManager::flush_all_pending_builds() { process_pending_builds(true); } } // namespace session diff --git a/test/src/test_session_commit.cc b/test/src/test_session_commit.cc index c057c9cc..d6d39d49 100644 --- a/test/src/test_session_commit.cc +++ b/test/src/test_session_commit.cc @@ -73,7 +73,7 @@ TestSessionCommit::commit_and_verify(bool use_fsyncdisk) { std::stringstream rtorrent_stream("rtorrent-data"); std::stringstream libtorrent_stream("libtorrent-data"); - session::DownloadStorer::save_and_move_streams(path, use_fsyncdisk, &torrent_stream, &rtorrent_stream, &libtorrent_stream); + session::DownloadStorer::save_and_move_streams(path, use_fsyncdisk, 0600, &torrent_stream, &rtorrent_stream, &libtorrent_stream); CPPUNIT_ASSERT_EQUAL(std::string("torrent-data"), read_file(path)); CPPUNIT_ASSERT_EQUAL(std::string("rtorrent-data"), read_file(path + ".rtorrent")); diff --git a/test/src/test_session_storer.cc b/test/src/test_session_storer.cc index 1febdb4f..8db76068 100644 --- a/test/src/test_session_storer.cc +++ b/test/src/test_session_storer.cc @@ -39,12 +39,12 @@ read_file(const std::string& path) { } void -save_session_files(const std::string& path) { +save_session_files(const std::string& path, mode_t file_mode = 0600) { std::stringstream torrent_stream("torrent-data"); std::stringstream rtorrent_stream("rtorrent-data"); std::stringstream libtorrent_stream("libtorrent-data"); - session::DownloadStorer::save_and_move_streams(path, false, &torrent_stream, &rtorrent_stream, &libtorrent_stream); + session::DownloadStorer::save_and_move_streams(path, false, file_mode, &torrent_stream, &rtorrent_stream, &libtorrent_stream); } unsigned int @@ -130,6 +130,21 @@ TestSessionStorer::test_saved_files_are_owner_only() { CPPUNIT_ASSERT_EQUAL(0600u, permissions_of(path + ".libtorrent_resume")); } +void +TestSessionStorer::test_saved_files_use_configured_mode() { + for (mode_t mode : {0660, 0640}) { + auto path = m_session_dir + "/" + entry_name; + auto prev_umask = ::umask(0); + + save_session_files(path, mode); + ::umask(prev_umask); + + CPPUNIT_ASSERT_EQUAL((unsigned int)mode, permissions_of(path)); + CPPUNIT_ASSERT_EQUAL((unsigned int)mode, permissions_of(path + ".rtorrent")); + CPPUNIT_ASSERT_EQUAL((unsigned int)mode, permissions_of(path + ".libtorrent_resume")); + } +} + // Entries listed for loading must report their real type so that a symlink in // the session directory is skipped instead of loaded. void diff --git a/test/src/test_session_storer.h b/test/src/test_session_storer.h index 34079950..591d2f41 100644 --- a/test/src/test_session_storer.h +++ b/test/src/test_session_storer.h @@ -7,6 +7,7 @@ class TestSessionStorer : public test_fixture { CPPUNIT_TEST(test_temp_file_symlink_is_not_followed); CPPUNIT_TEST(test_saved_files_are_owner_only); + CPPUNIT_TEST(test_saved_files_use_configured_mode); CPPUNIT_TEST(test_symlinked_entry_is_not_a_file); CPPUNIT_TEST_SUITE_END(); @@ -17,6 +18,7 @@ public: void test_temp_file_symlink_is_not_followed(); void test_saved_files_are_owner_only(); + void test_saved_files_use_configured_mode(); void test_symlinked_entry_is_not_a_file(); private: