From 1d26f820c6fd7d5a6508de20249e0bf00f72a8a5 Mon Sep 17 00:00:00 2001 From: ZacharyZcR Date: Tue, 28 Jul 2026 01:49:01 +0800 Subject: [PATCH] honour per-host recording flags and explain a missing recording (#1121) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The session recording section offers a recording path, a filename template and four content toggles, but the backend overwrote five of the six on every connection. A host could set none of them and get no indication why. Location and filename genuinely are not the host's to choose — recordings are indexed by them for playback and the backend refuses to read outside its recordings directory — so drop those two inputs rather than keep pretending they apply. The content flags are a host-level decision, so default them instead of forcing them. That still leaves the reported case, where guacd writes the file somewhere the backend cannot see it. The warning now reports both paths and names the two env vars that align them, which is otherwise guesswork for a split-container setup. Closes Termix-SSH/Support#1041 --- .../hosts/guacamole/guacamole-server.ts | 6 ++ .../hosts/guacamole/recording-settings.ts | 22 ++++++ src/backend/hosts/guacamole/routes.ts | 12 ++-- src/backend/hosts/guacamole/token-service.ts | 3 + .../guacamole/recording-settings.test.ts | 68 +++++++++++++++++++ src/ui/sidebar/HostEditorGuacamoleTabs.tsx | 60 ---------------- 6 files changed, 106 insertions(+), 65 deletions(-) create mode 100644 src/backend/hosts/guacamole/recording-settings.ts create mode 100644 src/backend/tests/hosts/guacamole/recording-settings.test.ts diff --git a/src/backend/hosts/guacamole/guacamole-server.ts b/src/backend/hosts/guacamole/guacamole-server.ts index efc8654d..6b63e8e9 100644 --- a/src/backend/hosts/guacamole/guacamole-server.ts +++ b/src/backend/hosts/guacamole/guacamole-server.ts @@ -104,10 +104,16 @@ async function persistGuacamoleRecording( await new Promise((resolve) => setTimeout(resolve, 100)); } if (!fs.existsSync(resolvedPath)) { + const guacdPath = recording.guacdPath ?? GUACAMOLE_RECORDINGS_DIR; guacLogger.warn("Guacamole recording file was not found", { operation: "guac_recording_missing", hostId: recording.hostId, path: resolvedPath, + guacdPath, + hint: + "guacd writes the recording to guacdPath, the backend reads it from path. " + + "When guacd runs in its own container these must be the same volume — set " + + "GUACD_RECORDING_PATH to guacd's mount point and GUACD_RECORDING_BACKEND_PATH to this one.", }); return; } diff --git a/src/backend/hosts/guacamole/recording-settings.ts b/src/backend/hosts/guacamole/recording-settings.ts new file mode 100644 index 00000000..32369169 --- /dev/null +++ b/src/backend/hosts/guacamole/recording-settings.ts @@ -0,0 +1,22 @@ +/** + * Merges Termix's recording bookkeeping into a host's guacd settings. + * + * Location and filename are not the host's to choose: recordings are indexed by + * them for playback, and the backend refuses to read anything outside its + * recordings directory. What a recording *contains* is a host-level decision, so + * those flags are only defaulted, never overwritten. + */ +export function withRecordingSettings( + guacConfig: Record, + recordingPath: string, + recordingName: string, +): Record { + return { + ...guacConfig, + "recording-path": recordingPath, + "recording-name": recordingName, + "create-recording-path": true, + "recording-exclude-output": guacConfig["recording-exclude-output"] ?? false, + "recording-include-keys": guacConfig["recording-include-keys"] ?? true, + }; +} diff --git a/src/backend/hosts/guacamole/routes.ts b/src/backend/hosts/guacamole/routes.ts index 3a3882b7..453da94d 100644 --- a/src/backend/hosts/guacamole/routes.ts +++ b/src/backend/hosts/guacamole/routes.ts @@ -1,5 +1,6 @@ import express from "express"; import { GuacamoleTokenService } from "./token-service.js"; +import { withRecordingSettings } from "./recording-settings.js"; import { guacLogger } from "../../utils/logger.js"; import { AuthManager } from "../../utils/auth-manager.js"; import { PermissionManager } from "../../utils/permission-manager.js"; @@ -615,15 +616,16 @@ router.post( userId, protocol: connectionType as "rdp" | "vnc" | "telnet", path: recordingName, + guacdPath: recordingPath, startedAt: new Date().toISOString(), } : undefined; if (recordingEnabled) { - guacConfig["recording-path"] = recordingPath; - guacConfig["recording-name"] = recordingName; - guacConfig["create-recording-path"] = true; - guacConfig["recording-exclude-output"] = false; - guacConfig["recording-include-keys"] = true; + guacConfig = withRecordingSettings( + guacConfig, + recordingPath, + recordingName, + ); } const termixConnectId = crypto.randomUUID(); diff --git a/src/backend/hosts/guacamole/token-service.ts b/src/backend/hosts/guacamole/token-service.ts index a77e6203..8bd34c4b 100644 --- a/src/backend/hosts/guacamole/token-service.ts +++ b/src/backend/hosts/guacamole/token-service.ts @@ -48,6 +48,9 @@ export interface GuacamoleRecordingMetadata { userId: string; protocol: "rdp" | "vnc" | "telnet"; path: string; + /** Directory guacd was told to write into; differs from the backend's view + * when guacd runs in its own container. */ + guacdPath?: string; startedAt: string; } diff --git a/src/backend/tests/hosts/guacamole/recording-settings.test.ts b/src/backend/tests/hosts/guacamole/recording-settings.test.ts new file mode 100644 index 00000000..68b19c53 --- /dev/null +++ b/src/backend/tests/hosts/guacamole/recording-settings.test.ts @@ -0,0 +1,68 @@ +import { describe, expect, it } from "vitest"; +import { withRecordingSettings } from "../../../hosts/guacamole/recording-settings.js"; + +const PATH = "/app/data/session_recordings/guacamole"; +const NAME = "b7e6c0f2-0000-4000-8000-000000000000.guac"; + +describe("withRecordingSettings", () => { + it("takes ownership of the location and filename", () => { + const merged = withRecordingSettings( + { + "recording-path": "/var/lib/termix/recordings", + "recording-name": "${GUAC_USERNAME}-${GUAC_DATE}", + "create-recording-path": false, + }, + PATH, + NAME, + ); + + expect(merged).toMatchObject({ + "recording-path": PATH, + "recording-name": NAME, + "create-recording-path": true, + }); + }); + + it("defaults the content flags when the host has no opinion", () => { + expect(withRecordingSettings({}, PATH, NAME)).toMatchObject({ + "recording-exclude-output": false, + "recording-include-keys": true, + }); + }); + + it("keeps the host's content flags, including the falsy ones", () => { + const merged = withRecordingSettings( + { + "recording-exclude-output": true, + "recording-include-keys": false, + }, + PATH, + NAME, + ); + + expect(merged).toMatchObject({ + "recording-exclude-output": true, + "recording-include-keys": false, + }); + }); + + it("leaves unrelated settings alone", () => { + const merged = withRecordingSettings( + { "recording-exclude-mouse": true, width: "1920" }, + PATH, + NAME, + ); + + expect(merged).toMatchObject({ + "recording-exclude-mouse": true, + width: "1920", + }); + }); + + it("does not mutate the settings it was given", () => { + const original = { "recording-path": "/tmp/mine" }; + withRecordingSettings(original, PATH, NAME); + + expect(original).toEqual({ "recording-path": "/tmp/mine" }); + }); +}); diff --git a/src/ui/sidebar/HostEditorGuacamoleTabs.tsx b/src/ui/sidebar/HostEditorGuacamoleTabs.tsx index b25e5af0..e2807472 100644 --- a/src/ui/sidebar/HostEditorGuacamoleTabs.tsx +++ b/src/ui/sidebar/HostEditorGuacamoleTabs.tsx @@ -859,26 +859,6 @@ export function HostEditorRdpTab({ } >
-
- - setGuacField("recording-path", e.target.value)} - /> -
-
- - setGuacField("recording-name", e.target.value)} - /> -
-
- - setGuacField("recording-path", e.target.value)} - /> -
-
- - setGuacField("recording-name", e.target.value)} - /> -
-
- - setGuacField("recording-path", e.target.value)} - /> -
-
- - setGuacField("recording-name", e.target.value)} - /> -