Internal
Public Access
Add SshSessionBackend test coverage (#1)
Adds two kinds of coverage, continuing #1's remaining scope: 1. Pure-function tests for mapSshError() and escapeForShellSingleQuotes(), promoted from private members to public statics purely so tests can call them without spinning up a process. escapeForShellSingleQuotes() is the actual security boundary for password auth (it's what stops a password containing a single quote from breaking out of the askpass script's quoting), so it gets a real adversarial test, not just a happy-path one. 2. State-machine tests (connect -> Connected, auth failure -> Failed with the right mapped message, connection refused -> Failed, input round-tripping, reconnect) driven against tests/fixtures/fake_ssh.sh, a small controllable stand-in for the real ssh binary, instead of a real network/SSH server. This needed one small testability seam: a new constructor overload that overrides the launched program ("ssh" in production, the fixture script in tests). POSIX-only for now: the fixture is a shell script, so the state-machine tests QSKIP on Windows until an equivalent fixture exists there; the pure-function tests run everywhere. RdpSessionBackend coverage is still open -- it's a bigger lift again (FreeRDP's own event loop, not just a QProcess), left for a follow-up. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
+16
-10
@@ -7,16 +7,14 @@
|
||||
#include <QTextStream>
|
||||
#include <QUuid>
|
||||
|
||||
namespace {
|
||||
QString escapeForShellSingleQuotes(const QString& value)
|
||||
SshSessionBackend::SshSessionBackend(const Profile& profile, QObject* parent)
|
||||
: SshSessionBackend(profile, QStringLiteral("ssh"), parent)
|
||||
{
|
||||
QString escaped = value;
|
||||
escaped.replace(QStringLiteral("'"), QStringLiteral("'\"'\"'"));
|
||||
return escaped;
|
||||
}
|
||||
}
|
||||
|
||||
SshSessionBackend::SshSessionBackend(const Profile& profile, QObject* parent)
|
||||
SshSessionBackend::SshSessionBackend(const Profile& profile,
|
||||
const QString& sshProgramOverride,
|
||||
QObject* parent)
|
||||
: SessionBackend(profile, parent),
|
||||
m_process(new QProcess(this)),
|
||||
m_connectedProbeTimer(new QTimer(this)),
|
||||
@@ -27,7 +25,8 @@ SshSessionBackend::SshSessionBackend(const Profile& profile, QObject* parent)
|
||||
m_waitingForHostKeyConfirmation(false),
|
||||
m_passwordSubmitted(false),
|
||||
m_terminalColumns(0),
|
||||
m_terminalRows(0)
|
||||
m_terminalRows(0),
|
||||
m_sshProgram(sshProgramOverride)
|
||||
{
|
||||
m_connectedProbeTimer->setSingleShot(true);
|
||||
|
||||
@@ -395,7 +394,7 @@ bool SshSessionBackend::startSshProcess(const SessionConnectOptions& options)
|
||||
args << target;
|
||||
|
||||
m_process->setProcessEnvironment(environment);
|
||||
m_process->setProgram(QStringLiteral("ssh"));
|
||||
m_process->setProgram(m_sshProgram);
|
||||
m_process->setArguments(args);
|
||||
m_process->setProcessChannelMode(QProcess::SeparateChannels);
|
||||
|
||||
@@ -471,7 +470,7 @@ void SshSessionBackend::cleanupAskPassScript()
|
||||
}
|
||||
}
|
||||
|
||||
QString SshSessionBackend::mapSshError(const QString& rawError) const
|
||||
QString SshSessionBackend::mapSshError(const QString& rawError)
|
||||
{
|
||||
const QString raw = rawError.trimmed();
|
||||
if (raw.contains(QStringLiteral("Permission denied"), Qt::CaseInsensitive)) {
|
||||
@@ -510,6 +509,13 @@ QString SshSessionBackend::mapSshError(const QString& rawError) const
|
||||
return QStringLiteral("SSH connection failed.");
|
||||
}
|
||||
|
||||
QString SshSessionBackend::escapeForShellSingleQuotes(const QString& value)
|
||||
{
|
||||
QString escaped = value;
|
||||
escaped.replace(QStringLiteral("'"), QStringLiteral("'\"'\"'"));
|
||||
return escaped;
|
||||
}
|
||||
|
||||
QString SshSessionBackend::knownHostsFileForNullDevice() const
|
||||
{
|
||||
#ifdef Q_OS_WIN
|
||||
|
||||
@@ -13,8 +13,16 @@ class SshSessionBackend : public SessionBackend
|
||||
|
||||
public:
|
||||
explicit SshSessionBackend(const Profile& profile, QObject* parent = nullptr);
|
||||
// Test-only: overrides the executable launched instead of "ssh", so
|
||||
// tests can point it at a controllable fixture script.
|
||||
SshSessionBackend(const Profile& profile, const QString& sshProgramOverride, QObject* parent);
|
||||
~SshSessionBackend() override;
|
||||
|
||||
// Pure, state-free helpers exposed as public statics purely so tests
|
||||
// can exercise them directly without spinning up a real ssh process.
|
||||
static QString mapSshError(const QString& rawError);
|
||||
static QString escapeForShellSingleQuotes(const QString& value);
|
||||
|
||||
public slots:
|
||||
void connectSession(const SessionConnectOptions& options) override;
|
||||
void disconnectSession() override;
|
||||
@@ -46,6 +54,7 @@ private:
|
||||
bool m_passwordSubmitted;
|
||||
int m_terminalColumns;
|
||||
int m_terminalRows;
|
||||
QString m_sshProgram;
|
||||
|
||||
void setState(SessionState state, const QString& message);
|
||||
bool startSshProcess(const SessionConnectOptions& options);
|
||||
@@ -53,7 +62,6 @@ private:
|
||||
QProcessEnvironment& environment,
|
||||
QString& error);
|
||||
void cleanupAskPassScript();
|
||||
QString mapSshError(const QString& rawError) const;
|
||||
QString knownHostsFileForNullDevice() const;
|
||||
void applyTerminalSizeIfAvailable();
|
||||
};
|
||||
|
||||
@@ -5,3 +5,15 @@ add_executable(test_profile_repository
|
||||
target_include_directories(test_profile_repository PRIVATE ${CMAKE_SOURCE_DIR}/src)
|
||||
target_link_libraries(test_profile_repository PRIVATE Qt6::Core Qt6::Sql Qt6::Test)
|
||||
add_test(NAME test_profile_repository COMMAND test_profile_repository)
|
||||
|
||||
add_executable(test_ssh_session_backend
|
||||
test_ssh_session_backend.cpp
|
||||
${CMAKE_SOURCE_DIR}/src/ssh_session_backend.cpp
|
||||
${CMAKE_SOURCE_DIR}/src/session_backend.h
|
||||
)
|
||||
target_include_directories(test_ssh_session_backend PRIVATE ${CMAKE_SOURCE_DIR}/src)
|
||||
target_link_libraries(test_ssh_session_backend PRIVATE Qt6::Core Qt6::Gui Qt6::Test)
|
||||
target_compile_definitions(test_ssh_session_backend PRIVATE
|
||||
ORBITHUB_TEST_FIXTURES_DIR="${CMAKE_CURRENT_SOURCE_DIR}/fixtures"
|
||||
)
|
||||
add_test(NAME test_ssh_session_backend COMMAND test_ssh_session_backend)
|
||||
|
||||
+29
@@ -0,0 +1,29 @@
|
||||
#!/bin/sh
|
||||
# Minimal, deterministic stand-in for the real `ssh` binary, used by
|
||||
# SshSessionBackend's state-machine tests so they never touch a real
|
||||
# network or SSH server. Behavior is selected by which fixture hostname
|
||||
# appears among argv (SshSessionBackend always passes the profile's
|
||||
# host, optionally as user@host, as the final argument).
|
||||
for arg in "$@"; do
|
||||
case "$arg" in
|
||||
*@succeed|succeed)
|
||||
echo "Welcome to the fake host."
|
||||
# Stay alive echoing stdin back (simulates an interactive
|
||||
# session) until the backend terminates us.
|
||||
while IFS= read -r line; do
|
||||
echo "$line"
|
||||
done
|
||||
exit 0
|
||||
;;
|
||||
*@fail-auth|fail-auth)
|
||||
echo "Permission denied (publickey,password)." >&2
|
||||
exit 255
|
||||
;;
|
||||
*@refuse|refuse)
|
||||
echo "ssh: connect to host refuse port 22: Connection refused" >&2
|
||||
exit 255
|
||||
;;
|
||||
esac
|
||||
done
|
||||
echo "fake_ssh.sh: no recognized fixture host in arguments: $*" >&2
|
||||
exit 1
|
||||
@@ -0,0 +1,236 @@
|
||||
#include "ssh_session_backend.h"
|
||||
|
||||
#include <QTest>
|
||||
|
||||
#include <memory>
|
||||
|
||||
#ifndef ORBITHUB_TEST_FIXTURES_DIR
|
||||
#error "ORBITHUB_TEST_FIXTURES_DIR must be defined by the build"
|
||||
#endif
|
||||
|
||||
namespace {
|
||||
Profile makeProfile(const QString& fixtureHost)
|
||||
{
|
||||
Profile profile;
|
||||
profile.name = QStringLiteral("Test Profile");
|
||||
profile.host = fixtureHost;
|
||||
profile.port = 22;
|
||||
profile.username = QStringLiteral("tester");
|
||||
profile.protocol = QStringLiteral("SSH");
|
||||
profile.authMode = QStringLiteral("Password");
|
||||
return profile;
|
||||
}
|
||||
|
||||
SessionConnectOptions makeOptions()
|
||||
{
|
||||
SessionConnectOptions options;
|
||||
options.password = QStringLiteral("dummy-password");
|
||||
return options;
|
||||
}
|
||||
}
|
||||
|
||||
class TestSshSessionBackend : public QObject
|
||||
{
|
||||
Q_OBJECT
|
||||
|
||||
private slots:
|
||||
// Pure-function coverage -- no process involved.
|
||||
void mapSshErrorRecognizesKnownPatterns();
|
||||
void mapSshErrorFallsBackForUnknownText();
|
||||
void mapSshErrorHandlesEmptyInput();
|
||||
void escapeForShellSingleQuotesNeutralizesQuotes();
|
||||
void escapeForShellSingleQuotesLeavesPlainTextAlone();
|
||||
|
||||
// State-machine coverage, driven against tests/fixtures/fake_ssh.sh
|
||||
// instead of a real ssh binary or network.
|
||||
void init();
|
||||
void cleanup();
|
||||
void successfulConnectReachesConnectedThenDisconnects();
|
||||
void authFailureReachesFailedStateWithMappedMessage();
|
||||
void connectionRefusedReachesFailedState();
|
||||
void sendInputEchoesThroughOutputReceived();
|
||||
void reconnectRestartsAndReachesConnectedAgain();
|
||||
|
||||
private:
|
||||
QString fixturePath() const;
|
||||
void createBackend(const QString& fixtureHost);
|
||||
|
||||
std::unique_ptr<SshSessionBackend> m_backend;
|
||||
SessionState m_lastState = SessionState::Disconnected;
|
||||
QString m_lastErrorDisplay;
|
||||
QString m_lastErrorRaw;
|
||||
QString m_receivedOutput;
|
||||
};
|
||||
|
||||
QString TestSshSessionBackend::fixturePath() const
|
||||
{
|
||||
return QStringLiteral(ORBITHUB_TEST_FIXTURES_DIR "/fake_ssh.sh");
|
||||
}
|
||||
|
||||
void TestSshSessionBackend::createBackend(const QString& fixtureHost)
|
||||
{
|
||||
m_backend =
|
||||
std::make_unique<SshSessionBackend>(makeProfile(fixtureHost), fixturePath(), nullptr);
|
||||
connect(m_backend.get(),
|
||||
&SessionBackend::stateChanged,
|
||||
this,
|
||||
[this](SessionState state, const QString&) { m_lastState = state; });
|
||||
connect(m_backend.get(),
|
||||
&SessionBackend::connectionError,
|
||||
this,
|
||||
[this](const QString& display, const QString& raw) {
|
||||
m_lastErrorDisplay = display;
|
||||
m_lastErrorRaw = raw;
|
||||
});
|
||||
connect(m_backend.get(),
|
||||
&SessionBackend::outputReceived,
|
||||
this,
|
||||
[this](const QString& chunk) { m_receivedOutput += chunk; });
|
||||
}
|
||||
|
||||
void TestSshSessionBackend::mapSshErrorRecognizesKnownPatterns()
|
||||
{
|
||||
QCOMPARE(SshSessionBackend::mapSshError(QStringLiteral("Permission denied (publickey,password).")),
|
||||
QStringLiteral("Authentication failed. Check username and credentials."));
|
||||
QCOMPARE(SshSessionBackend::mapSshError(QStringLiteral("Host key verification failed.")),
|
||||
QStringLiteral("Host key verification failed."));
|
||||
QCOMPARE(SshSessionBackend::mapSshError(QStringLiteral("ssh: Could not resolve hostname bogus")),
|
||||
QStringLiteral("Host could not be resolved."));
|
||||
QCOMPARE(SshSessionBackend::mapSshError(QStringLiteral("ssh: connect to host x port 22: Connection timed out")),
|
||||
QStringLiteral("Connection timed out."));
|
||||
QCOMPARE(SshSessionBackend::mapSshError(QStringLiteral("ssh: connect to host x port 22: Connection refused")),
|
||||
QStringLiteral("Connection refused by remote host."));
|
||||
QCOMPARE(SshSessionBackend::mapSshError(QStringLiteral("ssh: connect to host x port 22: No route to host")),
|
||||
QStringLiteral("No route to host."));
|
||||
QCOMPARE(SshSessionBackend::mapSshError(QStringLiteral("Identity file /nope not accessible: No such file.")),
|
||||
QStringLiteral("Private key file is not accessible."));
|
||||
QCOMPARE(SshSessionBackend::mapSshError(QStringLiteral("posix_spawn: /usr/bin/ssh-askpass: No such file or directory")),
|
||||
QStringLiteral("SSH password helper is missing or failed to launch."));
|
||||
QCOMPARE(SshSessionBackend::mapSshError(QStringLiteral("open /some/other/path: No such file or directory")),
|
||||
QStringLiteral("Required file was not found."));
|
||||
}
|
||||
|
||||
void TestSshSessionBackend::mapSshErrorFallsBackForUnknownText()
|
||||
{
|
||||
QCOMPARE(SshSessionBackend::mapSshError(QStringLiteral("some completely novel ssh error text")),
|
||||
QStringLiteral("SSH connection failed."));
|
||||
}
|
||||
|
||||
void TestSshSessionBackend::mapSshErrorHandlesEmptyInput()
|
||||
{
|
||||
QCOMPARE(SshSessionBackend::mapSshError(QString()),
|
||||
QStringLiteral("SSH connection failed for an unknown reason."));
|
||||
QCOMPARE(SshSessionBackend::mapSshError(QStringLiteral(" ")),
|
||||
QStringLiteral("SSH connection failed for an unknown reason."));
|
||||
}
|
||||
|
||||
void TestSshSessionBackend::escapeForShellSingleQuotesNeutralizesQuotes()
|
||||
{
|
||||
// A password containing a single quote must not be able to break out
|
||||
// of the single-quoted printf argument in the askpass script -- this
|
||||
// is the actual security boundary, not just cosmetic escaping.
|
||||
const QString malicious = QStringLiteral("pw' ; rm -rf ~ ; echo '");
|
||||
const QString escaped = SshSessionBackend::escapeForShellSingleQuotes(malicious);
|
||||
const QString reconstructedScriptArg = QStringLiteral("'") + escaped + QStringLiteral("'");
|
||||
// Every single quote in the reconstructed argument must be either the
|
||||
// outer boundary quote (open at index 0, close at the very end) or the
|
||||
// start of a full '"'"' re-opening sequence -- never a bare, unescaped
|
||||
// quote that could close the argument early.
|
||||
int index = 0;
|
||||
while (index < reconstructedScriptArg.length()) {
|
||||
if (reconstructedScriptArg.at(index) != QChar::fromLatin1('\'')) {
|
||||
++index;
|
||||
continue;
|
||||
}
|
||||
if (index == 0 || index == reconstructedScriptArg.length() - 1) {
|
||||
++index;
|
||||
continue;
|
||||
}
|
||||
QCOMPARE(reconstructedScriptArg.mid(index, 5), QStringLiteral("'\"'\"'"));
|
||||
index += 5;
|
||||
}
|
||||
}
|
||||
|
||||
void TestSshSessionBackend::escapeForShellSingleQuotesLeavesPlainTextAlone()
|
||||
{
|
||||
QCOMPARE(SshSessionBackend::escapeForShellSingleQuotes(QStringLiteral("plain-password-123")),
|
||||
QStringLiteral("plain-password-123"));
|
||||
}
|
||||
|
||||
void TestSshSessionBackend::init()
|
||||
{
|
||||
#ifdef Q_OS_WIN
|
||||
// fixtures/fake_ssh.sh is a POSIX shell script; there's no Windows
|
||||
// fixture yet, so skip only the tests that actually launch it. The
|
||||
// pure-function tests above (mapSshError*, escapeForShellSingleQuotes*)
|
||||
// don't touch the fixture and still run everywhere.
|
||||
const QByteArray currentTest = QTest::currentTestFunction();
|
||||
if (!currentTest.startsWith("mapSshError") && !currentTest.startsWith("escapeForShellSingleQuotes")) {
|
||||
QSKIP("No Windows equivalent of tests/fixtures/fake_ssh.sh yet");
|
||||
}
|
||||
#endif
|
||||
|
||||
m_lastState = SessionState::Disconnected;
|
||||
m_lastErrorDisplay.clear();
|
||||
m_lastErrorRaw.clear();
|
||||
m_receivedOutput.clear();
|
||||
// Individual tests call createBackend() with the fixture host they
|
||||
// need; most want "succeed", so provide it as the default here.
|
||||
createBackend(QStringLiteral("succeed"));
|
||||
}
|
||||
|
||||
void TestSshSessionBackend::cleanup()
|
||||
{
|
||||
if (m_backend) {
|
||||
m_backend->disconnectSession();
|
||||
}
|
||||
m_backend.reset();
|
||||
}
|
||||
|
||||
void TestSshSessionBackend::successfulConnectReachesConnectedThenDisconnects()
|
||||
{
|
||||
m_backend->connectSession(makeOptions());
|
||||
QTRY_COMPARE(m_lastState, SessionState::Connected);
|
||||
|
||||
m_backend->disconnectSession();
|
||||
QTRY_COMPARE(m_lastState, SessionState::Disconnected);
|
||||
}
|
||||
|
||||
void TestSshSessionBackend::authFailureReachesFailedStateWithMappedMessage()
|
||||
{
|
||||
createBackend(QStringLiteral("fail-auth"));
|
||||
m_backend->connectSession(makeOptions());
|
||||
QTRY_COMPARE(m_lastState, SessionState::Failed);
|
||||
QCOMPARE(m_lastErrorDisplay, QStringLiteral("Authentication failed. Check username and credentials."));
|
||||
QVERIFY(m_lastErrorRaw.contains(QStringLiteral("Permission denied")));
|
||||
}
|
||||
|
||||
void TestSshSessionBackend::connectionRefusedReachesFailedState()
|
||||
{
|
||||
createBackend(QStringLiteral("refuse"));
|
||||
m_backend->connectSession(makeOptions());
|
||||
QTRY_COMPARE(m_lastState, SessionState::Failed);
|
||||
QCOMPARE(m_lastErrorDisplay, QStringLiteral("Connection refused by remote host."));
|
||||
}
|
||||
|
||||
void TestSshSessionBackend::sendInputEchoesThroughOutputReceived()
|
||||
{
|
||||
m_backend->connectSession(makeOptions());
|
||||
QTRY_COMPARE(m_lastState, SessionState::Connected);
|
||||
|
||||
m_backend->sendInput(QStringLiteral("hello-from-test\n"));
|
||||
QTRY_VERIFY(m_receivedOutput.contains(QStringLiteral("hello-from-test")));
|
||||
}
|
||||
|
||||
void TestSshSessionBackend::reconnectRestartsAndReachesConnectedAgain()
|
||||
{
|
||||
m_backend->connectSession(makeOptions());
|
||||
QTRY_COMPARE(m_lastState, SessionState::Connected);
|
||||
|
||||
m_lastState = SessionState::Connecting;
|
||||
m_backend->reconnectSession(makeOptions());
|
||||
QTRY_COMPARE(m_lastState, SessionState::Connected);
|
||||
}
|
||||
|
||||
QTEST_GUILESS_MAIN(TestSshSessionBackend)
|
||||
#include "test_ssh_session_backend.moc"
|
||||
Reference in New Issue
Block a user