fix(ssh): invalidate stale sessions after credential updates
- bind pooled SSH connections to credential fingerprints - close active terminal sessions when SSH settings change - enforce the configured authentication method - add regression coverage for credential refresh behavior
This commit is contained in:
1 parent
73b4b6255d
commit
60ae2dde51
4 files changed
+142
-22
No files matched your search
@@ -11,6 +11,7 @@ import {
|
||||
userCanAccessServer,
|
||||
} from "../services/server-access.service";
|
||||
import * as ssh from "../services/ssh.service";
|
||||
import { closeTerminalSessionsForServer } from "./terminal";
|
||||
|
||||
function serializeMetric(
|
||||
metric: {
|
||||
@@ -308,6 +309,23 @@ export async function serverRoutes(app: FastifyInstance) {
|
||||
});
|
||||
}
|
||||
|
||||
const currentServer = await prisma.server.findFirst({
|
||||
where: { id, organizationId: req.organizationId! },
|
||||
});
|
||||
if (!currentServer) {
|
||||
return reply
|
||||
.status(404)
|
||||
.send({ success: false, error: "Server not found" });
|
||||
}
|
||||
|
||||
const sshConfigChanged =
|
||||
(rest.ip !== undefined && rest.ip !== currentServer.ip) ||
|
||||
(rest.sshPort !== undefined && rest.sshPort !== currentServer.sshPort) ||
|
||||
(rest.username !== undefined && rest.username !== currentServer.username) ||
|
||||
(rest.authType !== undefined && rest.authType !== currentServer.authType) ||
|
||||
sshKey !== undefined ||
|
||||
password !== undefined;
|
||||
|
||||
const server = await prisma.server.update({
|
||||
where: { id },
|
||||
data: {
|
||||
@@ -317,6 +335,11 @@ export async function serverRoutes(app: FastifyInstance) {
|
||||
},
|
||||
});
|
||||
|
||||
if (sshConfigChanged) {
|
||||
closeTerminalSessionsForServer(id);
|
||||
ssh.closeConnection(id);
|
||||
}
|
||||
|
||||
await auditLog({
|
||||
userId: req.userId,
|
||||
organizationId: req.organizationId,
|
||||
|
||||
@@ -89,14 +89,35 @@ function detachSocket(session: TerminalSession) {
|
||||
session.attachedSocket = undefined;
|
||||
}
|
||||
|
||||
function destroySession(sessionId: string) {
|
||||
function destroySession(sessionId: string, message?: string) {
|
||||
const session = terminalSessions.get(sessionId);
|
||||
if (!session) return;
|
||||
const attachedSocket = session.attachedSocket;
|
||||
detachSocket(session);
|
||||
session.shellStream?.end?.();
|
||||
session.shellStream?.close?.();
|
||||
session.shellStream?.destroy?.();
|
||||
terminalSessions.delete(sessionId);
|
||||
|
||||
if (attachedSocket?.readyState === 1) {
|
||||
if (message) {
|
||||
attachedSocket.send(JSON.stringify({ type: "error", data: message }));
|
||||
}
|
||||
attachedSocket.close();
|
||||
}
|
||||
}
|
||||
|
||||
export function closeTerminalSessionsForServer(serverId: string): number {
|
||||
let closedSessions = 0;
|
||||
for (const [sessionId, session] of terminalSessions.entries()) {
|
||||
if (session.serverId !== serverId) continue;
|
||||
destroySession(
|
||||
sessionId,
|
||||
"SSH settings changed. Open a new terminal session to use the updated account.",
|
||||
);
|
||||
closedSessions += 1;
|
||||
}
|
||||
return closedSessions;
|
||||
}
|
||||
|
||||
function pruneExpiredSessions() {
|
||||
|
||||
@@ -2,25 +2,46 @@ import { NodeSSH } from "node-ssh";
|
||||
import { Server } from "@prisma/client";
|
||||
import { decrypt } from "../../lib/crypto";
|
||||
|
||||
// Pool: serverId → active SSH connection
|
||||
const pool = new Map<string, NodeSSH>();
|
||||
const connectionPromises = new Map<string, Promise<NodeSSH>>();
|
||||
type ConnectionEntry = {
|
||||
fingerprint: string;
|
||||
ssh: NodeSSH;
|
||||
};
|
||||
|
||||
type PendingConnection = {
|
||||
fingerprint: string;
|
||||
promise: Promise<NodeSSH>;
|
||||
};
|
||||
|
||||
// A server can keep the same id while its SSH account changes. Keep the
|
||||
// credential fingerprint beside the connection so a stale account is never
|
||||
// reused after an update.
|
||||
const pool = new Map<string, ConnectionEntry>();
|
||||
const connectionPromises = new Map<string, PendingConnection>();
|
||||
const connectionGenerations = new Map<string, number>();
|
||||
|
||||
function getConnectionFingerprint(server: Server): string {
|
||||
return JSON.stringify([
|
||||
server.ip,
|
||||
server.sshPort,
|
||||
server.username,
|
||||
server.authType,
|
||||
server.sshKeyEnc,
|
||||
server.passwordEnc,
|
||||
]);
|
||||
}
|
||||
|
||||
function attachConnectionLifecycle(serverId: string, ssh: NodeSSH): void {
|
||||
const connection = ssh.connection;
|
||||
if (!connection) return;
|
||||
|
||||
connection.on("error", () => {
|
||||
closeConnection(serverId);
|
||||
});
|
||||
const removeIfCurrent = () => {
|
||||
if (pool.get(serverId)?.ssh !== ssh) return;
|
||||
pool.delete(serverId);
|
||||
};
|
||||
|
||||
connection.on("close", () => {
|
||||
closeConnection(serverId);
|
||||
});
|
||||
|
||||
connection.on("end", () => {
|
||||
closeConnection(serverId);
|
||||
});
|
||||
connection.on("error", removeIfCurrent);
|
||||
connection.on("close", removeIfCurrent);
|
||||
connection.on("end", removeIfCurrent);
|
||||
}
|
||||
|
||||
type SshConnectOptions = {
|
||||
@@ -48,7 +69,10 @@ async function connectSsh(
|
||||
): Promise<void> {
|
||||
const baseOpts = buildConnectionOptions(server, options);
|
||||
|
||||
if (server.authType === "SSH_KEY" && server.sshKeyEnc) {
|
||||
if (server.authType === "SSH_KEY") {
|
||||
if (!server.sshKeyEnc) {
|
||||
throw new Error("No SSH key configured for this server");
|
||||
}
|
||||
await ssh.connect({
|
||||
...baseOpts,
|
||||
privateKey: decrypt(server.sshKeyEnc),
|
||||
@@ -56,7 +80,10 @@ async function connectSsh(
|
||||
return;
|
||||
}
|
||||
|
||||
if (server.passwordEnc) {
|
||||
if (server.authType === "PASSWORD") {
|
||||
if (!server.passwordEnc) {
|
||||
throw new Error("No SSH password configured for this server");
|
||||
}
|
||||
await ssh.connect({
|
||||
...baseOpts,
|
||||
password: decrypt(server.passwordEnc),
|
||||
@@ -71,25 +98,41 @@ async function connectSsh(
|
||||
* Get or create an SSH connection for a server
|
||||
*/
|
||||
export async function getConnection(server: Server): Promise<NodeSSH> {
|
||||
// Return existing connection if alive
|
||||
const fingerprint = getConnectionFingerprint(server);
|
||||
|
||||
// Return an existing connection only when it was created from the exact
|
||||
// same host and credentials.
|
||||
const existing = pool.get(server.id);
|
||||
if (existing?.isConnected()) return existing;
|
||||
if (existing?.fingerprint === fingerprint && existing.ssh.isConnected()) {
|
||||
return existing.ssh;
|
||||
}
|
||||
if (existing) closeConnection(server.id);
|
||||
|
||||
const pending = connectionPromises.get(server.id);
|
||||
if (pending) return pending;
|
||||
if (pending?.fingerprint === fingerprint) return pending.promise;
|
||||
if (pending) closeConnection(server.id);
|
||||
|
||||
const generation = connectionGenerations.get(server.id) ?? 0;
|
||||
|
||||
const connectPromise = (async () => {
|
||||
const ssh = new NodeSSH();
|
||||
await connectSsh(ssh, server);
|
||||
|
||||
if ((connectionGenerations.get(server.id) ?? 0) !== generation) {
|
||||
ssh.dispose();
|
||||
throw new Error("SSH configuration changed while connecting; retry the request");
|
||||
}
|
||||
|
||||
attachConnectionLifecycle(server.id, ssh);
|
||||
pool.set(server.id, ssh);
|
||||
pool.set(server.id, { fingerprint, ssh });
|
||||
return ssh;
|
||||
})().finally(() => {
|
||||
if (connectionPromises.get(server.id)?.promise === connectPromise) {
|
||||
connectionPromises.delete(server.id);
|
||||
}
|
||||
});
|
||||
|
||||
connectionPromises.set(server.id, connectPromise);
|
||||
connectionPromises.set(server.id, { fingerprint, promise: connectPromise });
|
||||
return connectPromise;
|
||||
}
|
||||
|
||||
@@ -97,9 +140,13 @@ export async function getConnection(server: Server): Promise<NodeSSH> {
|
||||
* Close and remove a connection from the pool
|
||||
*/
|
||||
export function closeConnection(serverId: string): void {
|
||||
const conn = pool.get(serverId);
|
||||
const conn = pool.get(serverId)?.ssh;
|
||||
pool.delete(serverId);
|
||||
connectionPromises.delete(serverId);
|
||||
connectionGenerations.set(
|
||||
serverId,
|
||||
(connectionGenerations.get(serverId) ?? 0) + 1,
|
||||
);
|
||||
|
||||
try {
|
||||
conn?.dispose();
|
||||
|
||||
@@ -0,0 +1,29 @@
|
||||
import assert from "node:assert/strict";
|
||||
import { readFileSync } from "node:fs";
|
||||
import test from "node:test";
|
||||
|
||||
test("server SSH updates invalidate terminal sessions and pooled connections", () => {
|
||||
const source = readFileSync("src/server/routes/servers.ts", "utf8");
|
||||
const updateRoute = source.slice(
|
||||
source.indexOf('// PUT /servers/:id'),
|
||||
source.indexOf('// DELETE /servers/:id'),
|
||||
);
|
||||
|
||||
assert.match(updateRoute, /const sshConfigChanged =/);
|
||||
assert.match(updateRoute, /closeTerminalSessionsForServer\(id\)/);
|
||||
assert.match(updateRoute, /ssh\.closeConnection\(id\)/);
|
||||
});
|
||||
|
||||
test("pooled SSH connections are bound to their credential fingerprint", () => {
|
||||
const source = readFileSync(
|
||||
"src/server/services/ssh-services/connection.ts",
|
||||
"utf8",
|
||||
);
|
||||
|
||||
assert.match(source, /function getConnectionFingerprint/);
|
||||
assert.match(source, /server\.username/);
|
||||
assert.match(source, /server\.authType/);
|
||||
assert.match(source, /server\.sshKeyEnc/);
|
||||
assert.match(source, /server\.passwordEnc/);
|
||||
assert.match(source, /existing\?\.fingerprint === fingerprint/);
|
||||
});
|
||||
Reference in new issue
Block a user