From 2870664ee11358b0f35c8e83a33c39d1e86c72e5 Mon Sep 17 00:00:00 2001 From: ZacharyZcR Date: Tue, 28 Jul 2026 17:04:56 +0800 Subject: [PATCH] audit the remaining remote access paths (#1129) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Only SSH terminal sessions were audited. Opening a file manager session, an RDP, VNC or Telnet desktop, a Docker session or an SSH tunnel left no audit entry at all — which covers most of the ways data leaves a host or a foothold is established. Each of those four now writes an entry when the session is established, matching the existing ssh_connect: who, which host, from what address, and for tunnels the endpoint and local port being forwarded. Audit writes are fire-and-forget so they cannot delay or fail the connection, consistent with logAudit already swallowing its own errors. getAuditUsername was defined identically in two route files and is needed in four more, so it moves next to logAudit. --- src/backend/database/routes/credentials.ts | 12 ++-- src/backend/database/routes/host.ts | 11 ++-- src/backend/hosts/docker/routes.ts | 19 +++++++ src/backend/hosts/file-manager/index.ts | 23 ++++++++ src/backend/hosts/guacamole/routes.ts | 18 ++++++ src/backend/hosts/tunnel/routes.ts | 25 ++++++++ .../tests/utils/audit-username.test.ts | 57 +++++++++++++++++++ src/backend/utils/audit-logger.ts | 18 +++++- 8 files changed, 169 insertions(+), 14 deletions(-) create mode 100644 src/backend/tests/utils/audit-username.test.ts diff --git a/src/backend/database/routes/credentials.ts b/src/backend/database/routes/credentials.ts index bd5d49b3..3772e775 100644 --- a/src/backend/database/routes/credentials.ts +++ b/src/backend/database/routes/credentials.ts @@ -6,12 +6,15 @@ import { AuthManager } from "../../utils/auth-manager.js"; import { parseSSHKey } from "../../utils/ssh-key-utils.js"; import { registerCredentialKeyRoutes } from "./credential-key-routes.js"; import { registerCredentialDeployRoutes } from "./credential-deploy-routes.js"; -import { logAudit, getRequestMeta } from "../../utils/audit-logger.js"; +import { + logAudit, + getAuditUsername, + getRequestMeta, +} from "../../utils/audit-logger.js"; import { createCurrentCredentialRepository, createCurrentHostResolutionRepository, createCurrentHostRepository, - createCurrentUserRepository, createCurrentSyncTombstoneRepository, } from "../repositories/factory.js"; @@ -25,11 +28,6 @@ const authManager = AuthManager.getInstance(); const authenticateJWT = authManager.createAuthMiddleware(); const requireDataAccess = authManager.createDataAccessMiddleware(); -async function getAuditUsername(userId: string): Promise { - const actor = await createCurrentUserRepository().findById(userId); - return actor?.username ?? userId; -} - /** * @openapi * /credentials: diff --git a/src/backend/database/routes/host.ts b/src/backend/database/routes/host.ts index 37cbd992..83c3f90d 100644 --- a/src/backend/database/routes/host.ts +++ b/src/backend/database/routes/host.ts @@ -47,7 +47,11 @@ import { applyHostEnrollmentDefaults, requireHostEnrollmentAccessForPath, } from "./host-enrollment-auth.js"; -import { logAudit, getRequestMeta } from "../../utils/audit-logger.js"; +import { + logAudit, + getAuditUsername, + getRequestMeta, +} from "../../utils/audit-logger.js"; const router = express.Router(); @@ -55,11 +59,6 @@ const upload = multer({ storage: multer.memoryStorage() }); const STATS_SERVER_URL = "http://localhost:30005"; -async function getAuditUsername(userId: string): Promise { - const actor = await createCurrentUserRepository().findById(userId); - return actor?.username ?? userId; -} - function notifyStatsHostUpdated( hostId: number, headers: Pick, diff --git a/src/backend/hosts/docker/routes.ts b/src/backend/hosts/docker/routes.ts index ae013bb5..ff470b93 100644 --- a/src/backend/hosts/docker/routes.ts +++ b/src/backend/hosts/docker/routes.ts @@ -2,6 +2,11 @@ import express from "express"; import axios from "axios"; import { Client as SSHClient } from "ssh2"; import { logger } from "../../utils/logger.js"; +import { + logAudit, + getAuditUsername, + getRequestMeta, +} from "../../utils/audit-logger.js"; import { createCurrentCredentialRepository, createCurrentHostRepository, @@ -591,6 +596,20 @@ export function registerDockerSshRoutes(app: express.Express): void { } }); + void (async () => { + const { ipAddress, userAgent } = getRequestMeta(req); + await logAudit({ + userId, + username: await getAuditUsername(userId), + action: "docker_connect", + resourceType: "host", + resourceId: hostId ? String(hostId) : undefined, + ipAddress, + userAgent, + success: true, + }); + })(); + res.json({ success: true, message: "SSH connection established", diff --git a/src/backend/hosts/file-manager/index.ts b/src/backend/hosts/file-manager/index.ts index 9a18822a..a6980008 100644 --- a/src/backend/hosts/file-manager/index.ts +++ b/src/backend/hosts/file-manager/index.ts @@ -1,4 +1,9 @@ import express from "express"; +import { + logAudit, + getAuditUsername, + getRequestMeta, +} from "../../utils/audit-logger.js"; import { createCorsMiddleware } from "../../utils/cors-config.js"; import cookieParser from "cookie-parser"; import axios from "axios"; @@ -1165,6 +1170,24 @@ app.post("/ssh/file_manager/ssh/connect", async (req, res) => { scpLegacy: resolvedScpLegacy, }; scheduleSessionCleanup(sessionId); + + if (userId) { + const { ipAddress, userAgent } = getRequestMeta(req); + void (async () => { + await logAudit({ + userId, + username: await getAuditUsername(userId), + action: "file_manager_connect", + resourceType: "host", + resourceId: hostId ? String(hostId) : undefined, + resourceName: `${username}@${ip}:${port}`, + ipAddress, + userAgent, + success: true, + }); + })(); + } + res.json({ status: "success", message: "SSH connection established", diff --git a/src/backend/hosts/guacamole/routes.ts b/src/backend/hosts/guacamole/routes.ts index 71af3921..f6406fdf 100644 --- a/src/backend/hosts/guacamole/routes.ts +++ b/src/backend/hosts/guacamole/routes.ts @@ -15,6 +15,11 @@ import { import { resolveGuacdOptions } from "../../utils/guacd-config.js"; import { createJumpHostChain } from "../jump-host-chain.js"; import { waitForGuacdOpen } from "./guacamole-server.js"; +import { + logAudit, + getAuditUsername, + getRequestMeta, +} from "../../utils/audit-logger.js"; import { resolveJumpTunnelEndpoint } from "./jump-tunnel-endpoint.js"; const router = express.Router(); @@ -668,6 +673,19 @@ router.post( const sessionInfo = await waitForGuacdOpen(termixConnectId, 10000); + const { ipAddress, userAgent } = getRequestMeta(req); + await logAudit({ + userId, + username: await getAuditUsername(userId), + action: `${connectionType}_connect`, + resourceType: "host", + resourceId: String(hostId), + resourceName: `${hostname}:${port}`, + ipAddress, + userAgent, + success: true, + }); + res.json({ token, guacamoleConnectionId: sessionInfo?.guacamoleConnectionId ?? null, diff --git a/src/backend/hosts/tunnel/routes.ts b/src/backend/hosts/tunnel/routes.ts index 6eadcf9f..38b2006f 100644 --- a/src/backend/hosts/tunnel/routes.ts +++ b/src/backend/hosts/tunnel/routes.ts @@ -7,6 +7,11 @@ import type { AuthenticatedRequest, } from "../../../types/index.js"; import { CONNECTION_STATES } from "../../../types/index.js"; +import { + logAudit, + getAuditUsername, + getRequestMeta, +} from "../../utils/audit-logger.js"; import { tunnelLogger } from "../../utils/logger.js"; import { SystemCrypto } from "../../utils/system-crypto.js"; import { AuthManager } from "../../utils/auth-manager.js"; @@ -363,6 +368,26 @@ export function registerTunnelRoutes(app: express.Express): void { pendingTunnelOperations.set(tunnelName, operation); + const { ipAddress, userAgent } = getRequestMeta(req); + await logAudit({ + userId, + username: await getAuditUsername(userId), + action: "tunnel_connect", + resourceType: "tunnel", + resourceId: tunnelConfig.sourceHostId + ? String(tunnelConfig.sourceHostId) + : undefined, + resourceName: tunnelName, + details: JSON.stringify({ + endpointHost: tunnelConfig.endpointHost, + endpointPort: tunnelConfig.endpointPort, + sourcePort: tunnelConfig.sourcePort, + }), + ipAddress, + userAgent, + success: true, + }); + res.json({ message: "Connection request received", tunnelName }); operation diff --git a/src/backend/tests/utils/audit-username.test.ts b/src/backend/tests/utils/audit-username.test.ts new file mode 100644 index 00000000..db5e6b17 --- /dev/null +++ b/src/backend/tests/utils/audit-username.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, it, vi, beforeEach } from "vitest"; + +const findById = vi.hoisted(() => vi.fn()); + +vi.mock("../../database/repositories/factory.js", () => ({ + createCurrentAuditLogRepository: () => ({ create: vi.fn() }), + createCurrentUserRepository: () => ({ findById }), +})); + +const { getAuditUsername, getRequestMeta } = + await import("../../utils/audit-logger.js"); + +beforeEach(() => findById.mockReset()); + +describe("getAuditUsername", () => { + it("resolves the username to store alongside the entry", async () => { + findById.mockResolvedValueOnce({ id: "u-1", username: "alice" }); + + await expect(getAuditUsername("u-1")).resolves.toBe("alice"); + }); + + it("falls back to the id for an account that no longer exists", async () => { + findById.mockResolvedValueOnce(undefined); + + await expect(getAuditUsername("u-gone")).resolves.toBe("u-gone"); + }); + + it("never throws, so it cannot break the operation being audited", async () => { + findById.mockRejectedValueOnce(new Error("database unavailable")); + + await expect(getAuditUsername("u-1")).resolves.toBe("u-1"); + }); +}); + +describe("getRequestMeta", () => { + it("prefers the first x-forwarded-for hop", () => { + const meta = getRequestMeta({ + headers: { + "x-forwarded-for": "203.0.113.9, 10.0.0.1", + "user-agent": "Mozilla/5.0", + }, + ip: "10.0.0.1", + } as never); + + expect(meta).toEqual({ + ipAddress: "203.0.113.9", + userAgent: "Mozilla/5.0", + }); + }); + + it("falls back to the socket address", () => { + const meta = getRequestMeta({ headers: {}, ip: "192.0.2.5" } as never); + + expect(meta.ipAddress).toBe("192.0.2.5"); + expect(meta.userAgent).toBe(""); + }); +}); diff --git a/src/backend/utils/audit-logger.ts b/src/backend/utils/audit-logger.ts index c48c0380..88b0b39b 100644 --- a/src/backend/utils/audit-logger.ts +++ b/src/backend/utils/audit-logger.ts @@ -1,5 +1,21 @@ import type { Request } from "express"; -import { createCurrentAuditLogRepository } from "../database/repositories/factory.js"; +import { + createCurrentAuditLogRepository, + createCurrentUserRepository, +} from "../database/repositories/factory.js"; + +/** + * Resolves the display name to store alongside the entry. It is denormalised on + * purpose: the record has to stay readable after the account is gone. + */ +export async function getAuditUsername(userId: string): Promise { + try { + const actor = await createCurrentUserRepository().findById(userId); + return actor?.username ?? userId; + } catch { + return userId; + } +} export interface AuditLogParams { userId: string;