Skip to content

Commit ba7b6a8

Browse files
committed
fix: improve socket security and refactor hook execution
1. Move package manager socket to dedicated directory /tmp/linglong/ 2. Create socket directory with secure permissions (rwxr-xr-x) 3. Set socket file permissions to 0600 (owner read/write only) 4. Clean up stale socket files from previous crashes on startup 5. Improve error logging to include socket path in failure messages 6. Refactor hook command execution to use Cmd class instead of std::system() 7. 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 ba7b6a8

3 files changed

Lines changed: 75 additions & 55 deletions

File tree

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

Lines changed: 62 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,23 @@
1414
#include "linglong/repo/ostree_repo.h"
1515
#include "linglong/utils/error/error.h"
1616
#include "linglong/utils/file.h"
17+
#include "linglong/utils/log/log.h"
1718
#include "ocppi/cli/CLI.hpp"
1819
#include "ocppi/cli/crun/Crun.hpp"
1920

2021
#include <CLI/CLI.hpp>
2122

2223
#include <QCoreApplication>
2324

25+
#include <cerrno>
26+
#include <cstring>
2427
#include <filesystem>
2528
#include <memory>
2629
#include <string>
2730

31+
#include <sys/stat.h>
32+
#include <unistd.h>
33+
2834
namespace {
2935

3036
struct CommandLineOptions
@@ -162,18 +168,66 @@ auto runDaemonMode(ocppi::cli::CLI &cli, const CommandLineOptions &options) -> i
162168
} else {
163169
LogI("Running linglong package manager without dbus daemon...");
164170

165-
auto *server = new QDBusServer("unix:path=/tmp/linglong-package-manager.socket",
171+
// Create socket directory with secure permissions
172+
auto socketDir = std::filesystem::path("/tmp/linglong");
173+
std::error_code ec;
174+
175+
struct stat st;
176+
if (lstat(socketDir.c_str(), &st) == 0) {
177+
if (S_ISLNK(st.st_mode)) {
178+
LogE("failed to create socket directory: /tmp/linglong is a symbolic link. Please "
179+
"remove it.");
180+
return -1;
181+
}
182+
if (!S_ISDIR(st.st_mode)) {
183+
LogE("failed to create socket directory: /tmp/linglong exists but is not a "
184+
"directory. Please remove it.");
185+
return -1;
186+
}
187+
} else if (errno != ENOENT) {
188+
LogE("Failed to check /tmp/linglong: {}", strerror(errno));
189+
return -1;
190+
}
191+
192+
std::filesystem::create_directory(socketDir, ec);
193+
if (ec && ec != std::errc::file_exists) {
194+
LogE("Failed to create socket directory /tmp/linglong: {}", ec.message());
195+
return -1;
196+
}
197+
198+
// Set directory permissions to 0700 (deepin-linglong only)
199+
if (chmod(socketDir.c_str(), S_IRWXU) != 0) {
200+
LogE("Failed to set permissions on /tmp/linglong: {}", strerror(errno));
201+
return -1;
202+
}
203+
204+
auto socketPath = socketDir / "linglong-package-manager.socket";
205+
206+
// Clean up stale socket from previous crash
207+
std::filesystem::remove(socketPath, ec);
208+
209+
auto *server = new QDBusServer(QString("unix:path=%1").arg(socketPath.c_str()),
166210
QCoreApplication::instance());
167211
if (!server->isConnected()) {
168-
LogE("listen on socket: {}", server->lastError().message().toStdString());
212+
LogE("listen on socket {}: {}",
213+
socketPath.c_str(),
214+
server->lastError().message().toStdString());
169215
return -1;
170216
}
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-
});
217+
218+
// Set socket file permissions: only owner can read/write (0600)
219+
if (chmod(socketPath.c_str(), S_IRUSR | S_IWUSR) != 0) {
220+
LogE("Failed to set socket permissions: {}", strerror(errno));
221+
return -1;
222+
}
223+
224+
// Clean up on exit
225+
QObject::connect(QCoreApplication::instance(),
226+
&QCoreApplication::aboutToQuit,
227+
[socketPath]() {
228+
std::error_code ec;
229+
std::filesystem::remove(socketPath, ec);
230+
});
177231

178232
QObject::connect(server,
179233
&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)