Skip to content

Commit a26cd97

Browse files
committed
fix: improve socket security and refactor hook execution
1. Move package manager socket to dedicated directory /tmp/linglong/ 2. Set socket file permissions to 0700 (owner read/write only) 3. Clean up stale socket files from previous crashes on startup 4. Improve error logging to include socket path in failure messages 5. Refactor hook command execution to use Cmd class instead of std::system() 6. Remove custom shell escaping logic in favor of Cmd::exec() method Influence: 1. Test package manager daemon startup in no-dbus mode 2. Verify socket directory creation with correct permissions 3. Verify socket file has 0600 permissions after creation 4. Test daemon restart after crash to verify stale socket cleanup 5. Test package installation with pre/post install hooks 6. Verify hook commands execute correctly with environment variables 7. Test hook execution failure handling and error reporting 8. Verify socket cleanup on normal daemon shutdown
1 parent 6525cb7 commit a26cd97

3 files changed

Lines changed: 80 additions & 55 deletions

File tree

apps/ll-package-manager/src/main.cpp

Lines changed: 67 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,25 @@
1414
#include "linglong/repo/ostree_repo.h"
1515
#include "linglong/utils/error/error.h"
1616
#include "linglong/utils/file.h"
17+
#include "linglong/utils/finally/finally.h"
18+
#include "linglong/utils/log/log.h"
1719
#include "ocppi/cli/CLI.hpp"
1820
#include "ocppi/cli/crun/Crun.hpp"
1921

2022
#include <CLI/CLI.hpp>
2123

2224
#include <QCoreApplication>
2325

26+
#include <cerrno>
27+
#include <cstring>
2428
#include <filesystem>
2529
#include <memory>
2630
#include <string>
2731

32+
#include <fcntl.h>
33+
#include <sys/stat.h>
34+
#include <unistd.h>
35+
2836
namespace {
2937

3038
struct CommandLineOptions
@@ -162,18 +170,69 @@ auto runDaemonMode(ocppi::cli::CLI &cli, const CommandLineOptions &options) -> i
162170
} else {
163171
LogI("Running linglong package manager without dbus daemon...");
164172

165-
auto *server = new QDBusServer("unix:path=/tmp/linglong-package-manager.socket",
173+
auto socketDir = std::filesystem::path("/tmp/linglong");
174+
175+
if (mkdir(socketDir.c_str(), S_IRWXU) != 0) {
176+
if (errno == EEXIST) {
177+
int fd = open(socketDir.c_str(), O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC);
178+
if (fd < 0) {
179+
LogE("Failed to open /tmp/linglong safely: {}", strerror(errno));
180+
return -1;
181+
}
182+
struct stat st;
183+
if (fstat(fd, &st) != 0) {
184+
LogE("Failed to fstat /tmp/linglong: {}", strerror(errno));
185+
return -1;
186+
}
187+
if (st.st_uid != getuid()) {
188+
LogE(
189+
"Failed to create /tmp/linglong: owner mismatch (expected uid {}, got {}).",
190+
getuid(),
191+
st.st_uid);
192+
return -1;
193+
}
194+
if (fchmod(fd, S_IRWXU) != 0) {
195+
LogE("Failed to set permissions on /tmp/linglong: {}", strerror(errno));
196+
return -1;
197+
}
198+
199+
auto close_fd = linglong::utils::finally::finally([fd] {
200+
close(fd);
201+
});
202+
} else {
203+
LogE("Failed to create /tmp/linglong: {}", strerror(errno));
204+
return -1;
205+
}
206+
}
207+
208+
auto socketPath = socketDir / "linglong-package-manager.socket";
209+
210+
// Clean up stale socket from previous crash
211+
std::error_code ec;
212+
std::filesystem::remove(socketPath, ec);
213+
214+
auto *server = new QDBusServer(QString("unix:path=%1").arg(socketPath.c_str()),
166215
QCoreApplication::instance());
167216
if (!server->isConnected()) {
168-
LogE("listen on socket: {}", server->lastError().message().toStdString());
217+
LogE("listen on socket {}: {}",
218+
socketPath.c_str(),
219+
server->lastError().message().toStdString());
169220
return -1;
170221
}
171-
QObject::connect(QCoreApplication::instance(), &QCoreApplication::aboutToQuit, []() {
172-
if (QDir::root().remove("/tmp/linglong-package-manager.socket")) {
173-
return;
174-
}
175-
LogE("failed to remove /tmp/linglong-package-manager.socket.");
176-
});
222+
223+
// Set socket file permissions: only owner can read/write (0600)
224+
if (chmod(socketPath.c_str(), S_IRUSR | S_IWUSR) != 0) {
225+
LogE("Failed to set socket permissions: {}", strerror(errno));
226+
return -1;
227+
}
228+
229+
// Clean up on exit
230+
QObject::connect(QCoreApplication::instance(),
231+
&QCoreApplication::aboutToQuit,
232+
[socketPath]() {
233+
std::error_code ec;
234+
std::filesystem::remove(socketPath, ec);
235+
});
177236

178237
QObject::connect(server,
179238
&QDBusServer::newConnection,

libs/linglong/src/linglong/cli/cli.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,8 @@ utils::error::Result<api::dbus::v1::PackageManager *> Cli::getPkgMan()
470470
}
471471

472472
LogW("some subcommands will failed in --no-dbus mode.");
473-
const auto pkgManAddress = QString("unix:path=/tmp/linglong-package-manager.socket");
473+
const auto pkgManAddress =
474+
QString("unix:path=/tmp/linglong/linglong-package-manager.socket");
474475
QProcess::startDetached("sudo",
475476
{ "--user",
476477
LINGLONG_USERNAME,

libs/utils/src/linglong/utils/hooks.cpp

Lines changed: 11 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,21 @@
11
/*
2-
* SPDX-FileCopyrightText: 2025 UnionTech Software Technology Co., Ltd.
2+
* SPDX-FileCopyrightText: 2025 - 2026 UnionTech Software Technology Co., Ltd.
33
*
44
* SPDX-License-Identifier: LGPL-3.0-or-later
55
*/
66

77
#include "hooks.h"
88

9+
#include "cmd.h"
910
#include "configure.h"
1011
#include "linglong/common/error.h"
11-
#include "linglong/utils/env.h"
1212
#include "linglong/utils/error/error.h"
1313
#include "linglong/utils/log/log.h"
1414

1515
#include <fmt/format.h>
1616

1717
#include <array>
1818
#include <climits>
19-
#include <cstdlib>
2019
#include <cstring>
2120
#include <filesystem>
2221
#include <fstream>
@@ -33,58 +32,24 @@ constexpr std::string_view PRE_INSTALL_ACTION_PREFIX = "ll-pre-install=";
3332
constexpr std::string_view POST_INSTALL_ACTION_PREFIX = "ll-post-install=";
3433
constexpr std::string_view POST_UNINSTALL_ACTION_PREFIX = "ll-post-uninstall=";
3534

36-
// This function ensures the command string is safely wrapped for 'sh -c'.
37-
static std::string escapeAndWrapCommandForShell(const std::string &command)
38-
{
39-
std::string escapedCommand = command;
40-
size_t pos = 0;
41-
// Replace ' with '\'' (close current quote, add literal single quote, open new quote)
42-
while ((pos = escapedCommand.find('\'', pos)) != std::string::npos) {
43-
escapedCommand.replace(pos, 1, "'\\''");
44-
pos += 4;
45-
}
46-
47-
return "sh -c " + escapedCommand;
48-
}
49-
5035
using CommandList = const std::vector<std::string> &;
5136

5237
utils::error::Result<void> executeHookCommands(
5338
CommandList commands, const std::vector<std::pair<std::string, std::string>> &envVars) noexcept
5439
{
55-
LINGLONG_TRACE("Executing command");
40+
LINGLONG_TRACE("Executing hook commands");
5641

57-
std::vector<std::unique_ptr<EnvironmentVariableGuard>> envVarGuards;
58-
envVarGuards.reserve(envVars.size());
59-
for (const auto &pair : envVars) {
60-
envVarGuards.emplace_back(
61-
std::make_unique<EnvironmentVariableGuard>(pair.first, pair.second));
62-
}
42+
for (const auto &command : commands) {
43+
Cmd cmd("/bin/sh");
6344

64-
for (const auto &command_raw : commands) {
65-
std::string fullCommand = escapeAndWrapCommandForShell(command_raw);
66-
67-
int ret = std::system(fullCommand.c_str());
68-
69-
if (ret == -1) {
70-
return LINGLONG_ERR(fmt::format("Failed to execute command: '{}'. System error: {}.",
71-
fullCommand,
72-
common::error::errorString(errno)));
45+
for (const auto &[name, value] : envVars) {
46+
cmd.setEnv(name, value);
7347
}
7448

75-
if (!WIFEXITED(ret)) {
76-
int signalNum = WTERMSIG(ret);
77-
return LINGLONG_ERR(fmt::format("Command '{}' terminated by signal {} ({}).",
78-
fullCommand,
79-
signalNum,
80-
strsignal(signalNum)));
81-
}
82-
83-
int exitStatus = WEXITSTATUS(ret);
84-
if (exitStatus != 0) {
85-
return LINGLONG_ERR(fmt::format("Command '{}' exited with non-zero status: {}.",
86-
fullCommand,
87-
exitStatus));
49+
auto result = cmd.exec({ "-c", command });
50+
if (!result.has_value()) {
51+
return LINGLONG_ERR(
52+
fmt::format("Hook command '{}' failed: {}.", command, result.error()));
8853
}
8954
}
9055
return LINGLONG_OK;

0 commit comments

Comments
 (0)