feat: Improve execute terminal command tool

This commit is contained in:
Petr Mironychev
2026-03-13 00:34:20 +01:00
parent 6910037e97
commit 37084bec59
5 changed files with 93 additions and 86 deletions

View File

@ -116,6 +116,7 @@ const char CA_ALLOWED_TERMINAL_COMMANDS[] = "QodeAssist.caAllowedTerminalCommand
const char CA_ALLOWED_TERMINAL_COMMANDS_LINUX[] = "QodeAssist.caAllowedTerminalCommandsLinux";
const char CA_ALLOWED_TERMINAL_COMMANDS_MACOS[] = "QodeAssist.caAllowedTerminalCommandsMacOS";
const char CA_ALLOWED_TERMINAL_COMMANDS_WINDOWS[] = "QodeAssist.caAllowedTerminalCommandsWindows";
const char CA_TERMINAL_COMMAND_TIMEOUT[] = "QodeAssist.caTerminalCommandTimeout";
const char QODE_ASSIST_GENERAL_OPTIONS_ID[] = "QodeAssist.GeneralOptions";
const char QODE_ASSIST_GENERAL_SETTINGS_PAGE_ID[] = "QodeAssist.1GeneralSettingsPageId";

View File

@ -128,6 +128,14 @@ ToolsSettings::ToolsSettings()
allowedTerminalCommandsWindows.setDisplayStyle(Utils::StringAspect::LineEditDisplay);
allowedTerminalCommandsWindows.setDefaultValue("git, dir, type, findstr, where");
terminalCommandTimeout.setSettingsKey(Constants::CA_TERMINAL_COMMAND_TIMEOUT);
terminalCommandTimeout.setLabelText(Tr::tr("Command Timeout (seconds)"));
terminalCommandTimeout.setToolTip(
Tr::tr("Maximum time in seconds to wait for a terminal command to complete. "
"Increase for long-running commands like builds."));
terminalCommandTimeout.setRange(5, 3600);
terminalCommandTimeout.setDefaultValue(30);
resetToDefaults.m_buttonText = Tr::tr("Reset Page to Defaults");
readSettings();
@ -167,6 +175,7 @@ ToolsSettings::ToolsSettings()
enableTerminalCommandTool,
enableTodoTool,
currentOsCommands,
terminalCommandTimeout,
autoApplyFileEdits}},
Stretch{1}};
});
@ -203,6 +212,7 @@ void ToolsSettings::resetSettingsToDefaults()
resetAspect(allowedTerminalCommandsLinux);
resetAspect(allowedTerminalCommandsMacOS);
resetAspect(allowedTerminalCommandsWindows);
resetAspect(terminalCommandTimeout);
writeSettings();
}
}

View File

@ -45,6 +45,7 @@ public:
Utils::StringAspect allowedTerminalCommandsLinux{this};
Utils::StringAspect allowedTerminalCommandsMacOS{this};
Utils::StringAspect allowedTerminalCommandsWindows{this};
Utils::IntegerAspect terminalCommandTimeout{this};
Utils::BoolAspect autoApplyFileEdits{this};
private:

View File

@ -32,6 +32,8 @@
#include <QSharedPointer>
#include <QTimer>
#include <atomic>
namespace QodeAssist::Tools {
ExecuteTerminalCommandTool::ExecuteTerminalCommandTool(QObject *parent)
@ -188,54 +190,66 @@ QFuture<QString> ExecuteTerminalCommandTool::executeAsync(const QJsonObject &inp
QFuture<QString> future = promise->future();
promise->start();
auto resolved = std::make_shared<std::atomic<bool>>(false);
QProcess *process = new QProcess();
process->setWorkingDirectory(workingDir);
process->setProcessChannelMode(QProcess::MergedChannels);
process->setReadChannel(QProcess::StandardOutput);
const int timeoutMs = commandTimeoutMs();
QTimer *timeoutTimer = new QTimer();
timeoutTimer->setSingleShot(true);
timeoutTimer->setInterval(COMMAND_TIMEOUT_MS);
auto outputSize = QSharedPointer<qint64>::create(0);
timeoutTimer->setInterval(timeoutMs);
QObject::connect(timeoutTimer, &QTimer::timeout, [process, promise, resolved, command, args, timeoutTimer, timeoutMs]() {
if (*resolved)
return;
*resolved = true;
QObject::connect(timeoutTimer, &QTimer::timeout, [process, promise, command, args, timeoutTimer]() {
LOG_MESSAGE(QString("ExecuteTerminalCommandTool: Command '%1 %2' timed out after %3ms")
.arg(command)
.arg(args)
.arg(COMMAND_TIMEOUT_MS));
.arg(timeoutMs));
process->terminate();
QTimer::singleShot(1000, process, [process]() {
if (process->state() == QProcess::Running) {
if (process->state() != QProcess::NotRunning) {
LOG_MESSAGE("ExecuteTerminalCommandTool: Forcefully killing process after timeout");
process->kill();
}
process->deleteLater();
});
promise->addResult(QString("Error: Command '%1 %2' timed out after %3 seconds. "
"The process has been terminated.")
.arg(command)
.arg(args.isEmpty() ? "" : args)
.arg(COMMAND_TIMEOUT_MS / 1000));
.arg(timeoutMs / 1000));
promise->finish();
process->deleteLater();
timeoutTimer->deleteLater();
});
QObject::connect(
process,
QOverload<int, QProcess::ExitStatus>::of(&QProcess::finished),
[this, process, promise, command, args, timeoutTimer, outputSize](
[this, process, promise, resolved, command, args, timeoutTimer](
int exitCode, QProcess::ExitStatus exitStatus) {
if (*resolved) {
process->deleteLater();
return;
}
*resolved = true;
timeoutTimer->stop();
timeoutTimer->deleteLater();
const QByteArray rawOutput = process->readAll();
*outputSize += rawOutput.size();
const QString output = sanitizeOutput(QString::fromUtf8(rawOutput), *outputSize);
const qint64 outputSize = rawOutput.size();
const QString output = sanitizeOutput(QString::fromUtf8(rawOutput), outputSize);
const QString fullCommand = args.isEmpty() ? command : QString("%1 %2").arg(command).arg(args);
@ -244,7 +258,7 @@ QFuture<QString> ExecuteTerminalCommandTool::executeAsync(const QJsonObject &inp
LOG_MESSAGE(QString("ExecuteTerminalCommandTool: Command '%1' completed "
"successfully (output size: %2 bytes)")
.arg(fullCommand)
.arg(*outputSize));
.arg(outputSize));
promise->addResult(
QString("Command '%1' executed successfully.\n\nOutput:\n%2")
.arg(fullCommand)
@ -254,7 +268,7 @@ QFuture<QString> ExecuteTerminalCommandTool::executeAsync(const QJsonObject &inp
"exit code %2 (output size: %3 bytes)")
.arg(fullCommand)
.arg(exitCode)
.arg(*outputSize));
.arg(outputSize));
promise->addResult(
QString("Command '%1' failed with exit code %2.\n\nOutput:\n%3")
.arg(fullCommand)
@ -265,7 +279,7 @@ QFuture<QString> ExecuteTerminalCommandTool::executeAsync(const QJsonObject &inp
LOG_MESSAGE(QString("ExecuteTerminalCommandTool: Command '%1' crashed or was "
"terminated (output size: %2 bytes)")
.arg(fullCommand)
.arg(*outputSize));
.arg(outputSize));
const QString error = process->errorString();
promise->addResult(
QString("Command '%1' crashed or was terminated.\n\nError: %2\n\nOutput:\n%3")
@ -278,11 +292,13 @@ QFuture<QString> ExecuteTerminalCommandTool::executeAsync(const QJsonObject &inp
process->deleteLater();
});
QObject::connect(process, &QProcess::errorOccurred, [process, promise, command, args, timeoutTimer](
QObject::connect(process, &QProcess::errorOccurred, [process, promise, resolved, command, args, timeoutTimer](
QProcess::ProcessError error) {
if (promise->future().isFinished()) {
if (*resolved) {
process->deleteLater();
return;
}
*resolved = true;
timeoutTimer->stop();
timeoutTimer->deleteLater();
@ -292,7 +308,7 @@ QFuture<QString> ExecuteTerminalCommandTool::executeAsync(const QJsonObject &inp
.arg(fullCommand)
.arg(error)
.arg(process->errorString()));
QString errorMessage;
switch (error) {
case QProcess::FailedToStart:
@ -318,71 +334,46 @@ QFuture<QString> ExecuteTerminalCommandTool::executeAsync(const QJsonObject &inp
.arg(process->errorString());
break;
}
promise->addResult(QString("Error: %1").arg(errorMessage));
promise->finish();
process->deleteLater();
});
QString fullCommand = command;
QStringList argsList;
if (!args.isEmpty()) {
fullCommand += " " + args;
argsList = QProcess::splitCommand(args);
}
#ifdef Q_OS_WIN
static const QStringList windowsBuiltinCommands = {
"dir", "type", "del", "copy", "move", "ren", "rename",
"dir", "type", "del", "copy", "move", "ren", "rename",
"md", "mkdir", "rd", "rmdir", "cd", "chdir", "cls", "echo",
"set", "path", "prompt", "ver", "vol", "date", "time"
};
const QString lowerCommand = command.toLower();
const bool isBuiltin = windowsBuiltinCommands.contains(lowerCommand);
if (isBuiltin) {
LOG_MESSAGE(QString("ExecuteTerminalCommandTool: Executing Windows builtin command '%1' via cmd.exe")
.arg(command));
process->start("cmd.exe", QStringList() << "/c" << fullCommand);
QStringList cmdArgs;
cmdArgs << "/c" << command;
cmdArgs.append(argsList);
process->start("cmd.exe", cmdArgs);
} else {
#endif
QStringList splitCommand = QProcess::splitCommand(fullCommand);
if (splitCommand.isEmpty()) {
LOG_MESSAGE("ExecuteTerminalCommandTool: Failed to parse command");
promise->addResult(QString("Error: Failed to parse command '%1'").arg(fullCommand));
promise->finish();
process->deleteLater();
timeoutTimer->deleteLater();
return future;
}
const QString program = splitCommand.takeFirst();
process->start(program, splitCommand);
#ifdef Q_OS_WIN
process->start(command, argsList);
}
#else
process->start(command, argsList);
#endif
if (!process->waitForStarted(PROCESS_START_TIMEOUT_MS)) {
LOG_MESSAGE(QString("ExecuteTerminalCommandTool: Failed to start command '%1' within %2ms")
.arg(fullCommand)
.arg(PROCESS_START_TIMEOUT_MS));
const QString errorString = process->errorString();
promise->addResult(QString("Error: Failed to start command '%1': %2\n\n"
"Possible reasons:\n"
"- Command not found in PATH\n"
"- Insufficient permissions\n"
"- Invalid command syntax")
.arg(fullCommand)
.arg(errorString));
promise->finish();
process->deleteLater();
timeoutTimer->deleteLater();
return future;
}
LOG_MESSAGE(QString("ExecuteTerminalCommandTool: Process started successfully (PID: %1)")
.arg(process->processId()));
timeoutTimer->start();
LOG_MESSAGE(QString("ExecuteTerminalCommandTool: Process start requested for '%1'")
.arg(command));
return future;
}
@ -414,19 +405,27 @@ bool ExecuteTerminalCommandTool::areArgumentsSafe(const QString &args) const
return true;
}
// Check for null bytes
if (args.contains(QChar('\0'))) {
LOG_MESSAGE("ExecuteTerminalCommandTool: Null byte found in args");
return false;
}
static const QStringList dangerousPatterns = {
";", // Command separator
"&&", // AND operator
"||", // OR operator
"&", // Command separator / background execution
"|", // Pipe operator
">", // Output redirection
">>", // Append redirection
"<", // Input redirection
"`", // Command substitution
"$(", // Command substitution
"$()", // Command substitution
"\\n", // Newline (could start new command)
"\\r" // Carriage return
"${", // Variable expansion
"\n", // Newline (could start new command)
"\r", // Carriage return
#ifdef Q_OS_WIN
"^", // Escape character in cmd.exe (can bypass other checks)
"%", // Environment variable expansion on Windows
#endif
};
for (const QString &pattern : dangerousPatterns) {
@ -456,9 +455,6 @@ QString ExecuteTerminalCommandTool::sanitizeOutput(const QString &output, qint64
QStringList ExecuteTerminalCommandTool::getAllowedCommands() const
{
static QString cachedCommandsStr;
static QStringList cachedCommands;
QString commandsStr;
#ifdef Q_OS_LINUX
@ -471,28 +467,27 @@ QStringList ExecuteTerminalCommandTool::getAllowedCommands() const
commandsStr = Settings::toolsSettings().allowedTerminalCommandsLinux().trimmed(); // fallback
#endif
if (commandsStr == cachedCommandsStr && !cachedCommands.isEmpty()) {
return cachedCommands;
}
cachedCommandsStr = commandsStr;
cachedCommands.clear();
if (commandsStr.isEmpty()) {
return QStringList();
}
QStringList result;
const QStringList rawCommands = commandsStr.split(',', Qt::SkipEmptyParts);
cachedCommands.reserve(rawCommands.size());
result.reserve(rawCommands.size());
for (const QString &cmd : rawCommands) {
const QString trimmed = cmd.trimmed();
if (!trimmed.isEmpty()) {
cachedCommands.append(trimmed);
result.append(trimmed);
}
}
return cachedCommands;
return result;
}
int ExecuteTerminalCommandTool::commandTimeoutMs() const
{
return Settings::toolsSettings().terminalCommandTimeout() * 1000;
}
QString ExecuteTerminalCommandTool::getCommandDescription() const
@ -518,7 +513,7 @@ QString ExecuteTerminalCommandTool::getCommandDescription() const
"Commands have a %2 second timeout. "
"Returns the command output (stdout and stderr) or an error message if the command fails.%3")
.arg(allowedList)
.arg(COMMAND_TIMEOUT_MS / 1000)
.arg(commandTimeoutMs() / 1000)
.arg(osInfo);
}

View File

@ -46,12 +46,12 @@ private:
QString getCommandDescription() const;
QString sanitizeOutput(const QString &output, qint64 maxSize) const;
int commandTimeoutMs() const;
// Constants for production safety
static constexpr int COMMAND_TIMEOUT_MS = 30000; // 30 seconds
static constexpr qint64 MAX_OUTPUT_SIZE = 10 * 1024 * 1024; // 10 MB
static constexpr int MAX_COMMAND_LENGTH = 1024;
static constexpr int MAX_ARGS_LENGTH = 4096;
static constexpr int PROCESS_START_TIMEOUT_MS = 3000;
};
} // namespace QodeAssist::Tools