From 5d7ebc5b010192b57742b0397fba578a6f7c38a8 Mon Sep 17 00:00:00 2001 From: mptyl Date: Mon, 3 Aug 2026 22:10:09 +0200 Subject: [PATCH] fix: harden workspace runtime snapshots --- backend/src/app.ts | 2 + backend/src/tht/tht-runner.ts | 162 +++++++++++++++++- backend/src/workspaces/runtime-renderer.ts | 11 +- backend/test/tht-runner.test.ts | 113 +++++++++++- .../test/workspace-runtime-renderer.test.ts | 26 +++ backend/test/workspaces-bindings.test.ts | 1 + 6 files changed, 297 insertions(+), 18 deletions(-) diff --git a/backend/src/app.ts b/backend/src/app.ts index 190da83a..b2d5d074 100644 --- a/backend/src/app.ts +++ b/backend/src/app.ts @@ -1,5 +1,6 @@ import Fastify, { type FastifyInstance } from "fastify"; import cors from "@fastify/cors"; +import { join } from "node:path"; import type { AppConfig } from "./config.js"; import { ThtRunner } from "./tht/tht-runner.js"; import { PiProcessManager } from "./pi/pi-process-manager.js"; @@ -40,6 +41,7 @@ export function buildApp(config: AppConfig, deps?: BuildAppDeps): FastifyInstanc harnessDir: config.harnessDir, configPath: process.env.THT_CONFIG ?? "config/tht.yaml", dataRoot: config.dataRoot, + runtimeSnapshotRoot: join(config.workspaceRegistry.root, "snapshots", "runtime"), secretsFile: config.secretsFile, secretFiles: config.secretFiles, }); diff --git a/backend/src/tht/tht-runner.ts b/backend/src/tht/tht-runner.ts index 1b64f9a7..ed112a5c 100644 --- a/backend/src/tht/tht-runner.ts +++ b/backend/src/tht/tht-runner.ts @@ -1,5 +1,9 @@ import { spawn } from "node:child_process"; -import { existsSync } from "node:fs"; +import { createHash, randomUUID } from "node:crypto"; +import { + closeSync, constants as fsConstants, existsSync, fchmodSync, fstatSync, fsyncSync, lstatSync, mkdirSync, + openSync, readFileSync, readSync, realpathSync, statSync, unlinkSync, writeFileSync, +} from "node:fs"; import { isAbsolute, join } from "node:path"; import { clearPrincipalEnvironment, principalEnvironment, type PrincipalContext } from "../auth/principal.js"; import { secretValue, type SecretBundleConfig } from "../config/secret-bundle.js"; @@ -9,6 +13,7 @@ export interface ThtConfig extends SecretBundleConfig { harnessDir: string; configPath: string; dataRoot?: string; + runtimeSnapshotRoot?: string; } export interface SessionRow { @@ -38,7 +43,18 @@ export interface OllamaEnsureResult { model_name?: string; } +interface RuntimeSnapshot { + path: string; + dev: number; + ino: number; + size: number; + mode: number; + digest: string; +} + export class ThtRunner { + private readonly runtimeSnapshots = new Map(); + constructor(private cfg: ThtConfig, private principal?: PrincipalContext) {} /** Bind one trusted request principal to every child spawned by this runner. */ @@ -51,7 +67,10 @@ export class ThtRunner { */ private configArg(workspaceConfigPath?: string): string[] { if (workspaceConfigPath) { - if (isAbsolute(workspaceConfigPath)) return ["-c", workspaceConfigPath]; + if (isAbsolute(workspaceConfigPath)) { + this.assertTrustedRuntimeSnapshot(workspaceConfigPath); + return ["-c", workspaceConfigPath]; + } if (workspaceConfigPath.includes("/")) { throw new Error("workspace snapshot config path must be absolute"); } @@ -64,6 +83,115 @@ export class ThtRunner { return ["-c", this.cfg.configPath]; } + private runtimeSnapshotDirectory(): string { + if (!this.cfg.runtimeSnapshotRoot) throw new Error("runtime snapshot root is not configured"); + if (!isAbsolute(this.cfg.runtimeSnapshotRoot)) throw new Error("runtime snapshot root must be absolute"); + mkdirSync(this.cfg.runtimeSnapshotRoot, { recursive: true, mode: 0o700 }); + const directory = lstatSync(this.cfg.runtimeSnapshotRoot); + if (!directory.isDirectory() || directory.isSymbolicLink() || (directory.mode & 0o077) !== 0) { + throw new Error("runtime snapshot root is not trusted"); + } + return realpathSync(this.cfg.runtimeSnapshotRoot); + } + + private static isRestrictiveMode(mode: number): boolean { + const permissions = mode & 0o777; + return (permissions & 0o400) !== 0 && (permissions & ~0o600) === 0; + } + + private assertTrustedRuntimeSnapshot(path: string): RuntimeSnapshot { + const snapshot = this.runtimeSnapshots.get(path); + if (!snapshot) throw new Error("config path is not a trusted runtime snapshot"); + try { + const entry = lstatSync(path); + const stat = statSync(path); + if ( + !entry.isFile() || entry.isSymbolicLink() + || stat.dev !== snapshot.dev || stat.ino !== snapshot.ino || stat.size !== snapshot.size + || (stat.mode & 0o777) !== snapshot.mode || !ThtRunner.isRestrictiveMode(stat.mode) + || createHash("sha256").update(readFileSync(path)).digest("hex") !== snapshot.digest + ) throw new Error("changed runtime snapshot"); + return snapshot; + } catch { + throw new Error("config path is not a trusted runtime snapshot"); + } + } + + /** Create an opaque, backend-owned temporary config that is safe to hand to `tht`. */ + createRuntimeSnapshot(config: string): string { + const directory = this.runtimeSnapshotDirectory(); + const path = join(directory, `runtime-${randomUUID()}.yaml`); + const fd = openSync( + path, + fsConstants.O_WRONLY | fsConstants.O_CREAT | fsConstants.O_EXCL | fsConstants.O_NOFOLLOW, + 0o600, + ); + try { + writeFileSync(fd, config, "utf8"); + fsyncSync(fd); + fchmodSync(fd, 0o400); + const stat = fstatSync(fd); + this.runtimeSnapshots.set(path, { + path, + dev: stat.dev, + ino: stat.ino, + size: stat.size, + mode: stat.mode & 0o777, + digest: createHash("sha256").update(config, "utf8").digest("hex"), + }); + return path; + } catch (error) { + try { unlinkSync(path); } catch { /* creation did not produce a removable file */ } + throw error; + } finally { + closeSync(fd); + } + } + + cleanupRuntimeSnapshot(path: string): void { + const snapshot = this.runtimeSnapshots.get(path); + if (!snapshot) return; + this.runtimeSnapshots.delete(path); + try { unlinkSync(snapshot.path); } catch { /* a changed path is never removed recursively */ } + } + + private openTrustedRuntimeSnapshot(path: string): number { + const snapshot = this.assertTrustedRuntimeSnapshot(path); + const fd = openSync(path, fsConstants.O_RDONLY | fsConstants.O_NOFOLLOW); + try { + const stat = fstatSync(fd); + if ( + !stat.isFile() || stat.dev !== snapshot.dev || stat.ino !== snapshot.ino || stat.size !== snapshot.size + || (stat.mode & 0o777) !== snapshot.mode || !ThtRunner.isRestrictiveMode(stat.mode) + ) throw new Error("changed runtime snapshot"); + const contents = Buffer.alloc(snapshot.size); + let offset = 0; + while (offset < contents.length) { + const bytes = readSync(fd, contents, offset, contents.length - offset, offset); + if (bytes === 0) throw new Error("truncated runtime snapshot"); + offset += bytes; + } + if (createHash("sha256").update(contents).digest("hex") !== snapshot.digest) { + throw new Error("changed runtime snapshot"); + } + return fd; + } catch { + closeSync(fd); + throw new Error("config path is not a trusted runtime snapshot"); + } + } + + async runWithRuntimeSnapshot( + args: string[], config: string, timeoutMs: number = ThtRunner.DEFAULT_TIMEOUT_MS, + ): Promise<{ code: number; stdout: string; stderr: string }> { + const snapshot = this.createRuntimeSnapshot(config); + try { + return await this.run(args, snapshot, timeoutMs); + } finally { + this.cleanupRuntimeSnapshot(snapshot); + } + } + /** * Build the full argv for a `tht` invocation. `--config`/`-c` is a PER-COMMAND * option in the `tht` CLI (there is NO global `-c`), so it MUST be appended @@ -80,7 +208,7 @@ export class ThtRunner { static readonly DWH_TIMEOUT_MS = 120_000; run( - args: string[], workspace?: string, timeoutMs: number = ThtRunner.DEFAULT_TIMEOUT_MS, + args: string[], workspaceConfigPath?: string, timeoutMs: number = ThtRunner.DEFAULT_TIMEOUT_MS, ): Promise<{ code: number; stdout: string; stderr: string }> { return new Promise((resolve) => { const env: NodeJS.ProcessEnv = { ...process.env }; @@ -99,10 +227,26 @@ export class ThtRunner { env.THT_CA = ca; env.THT_SSL_CA = ca; } - const ch = spawn(this.cfg.thtBin, this.buildArgv(args, workspace), { - cwd: this.cfg.harnessDir, - env, - }); + let snapshotFd: number | undefined; + let ch; + try { + snapshotFd = workspaceConfigPath && isAbsolute(workspaceConfigPath) + ? this.openTrustedRuntimeSnapshot(workspaceConfigPath) + : undefined; + ch = spawn( + this.cfg.thtBin, + snapshotFd === undefined + ? this.buildArgv(args, workspaceConfigPath) + : [...args, "-c", "/dev/fd/3"], + { + cwd: this.cfg.harnessDir, + env, + ...(snapshotFd === undefined ? {} : { stdio: ["ignore", "pipe", "pipe", snapshotFd] }), + }, + ); + } finally { + if (snapshotFd !== undefined) closeSync(snapshotFd); + } let stdout = ""; let stderr = ""; let settled = false; @@ -119,8 +263,8 @@ export class ThtRunner { finish({ code: 124, stdout, stderr: stderr || `timed out after ${timeoutMs}ms` }); }, timeoutMs); } - ch.stdout.on("data", (d: Buffer) => (stdout += d)); - ch.stderr.on("data", (d: Buffer) => (stderr += d)); + ch.stdout?.on("data", (d: Buffer) => (stdout += d)); + ch.stderr?.on("data", (d: Buffer) => (stderr += d)); ch.on("error", (error) => finish({ code: 1, stdout, stderr: stderr || error.message })); ch.on("close", (code) => finish({ code: code ?? 0, stdout, stderr })); }); diff --git a/backend/src/workspaces/runtime-renderer.ts b/backend/src/workspaces/runtime-renderer.ts index f1cf8429..e6db2960 100644 --- a/backend/src/workspaces/runtime-renderer.ts +++ b/backend/src/workspaces/runtime-renderer.ts @@ -31,10 +31,10 @@ function requireBinding(binding: ResolvedBinding, name: string): string { function legacyDirectConnection( binding: ResolvedBinding, - names: { host: string; port: string; user: string; passwordFile: string }, + names: { host: string; port: string; user: string; passwordFile: string; tlsCaFile: string }, identity: { database: string; schema: string }, ): Record { - return { + const connection: Record = { host: requireBinding(binding, names.host), port: Number(requireBinding(binding, names.port)), database: identity.database, @@ -42,6 +42,9 @@ function legacyDirectConnection( user: requireBinding(binding, names.user), password_file: requireBinding(binding, names.passwordFile), }; + const tlsCaFile = bindingValue(binding, names.tlsCaFile); + if (tlsCaFile !== undefined) connection.ssl_ca_file = tlsCaFile; + return connection; } function legacyRestEndpoint( @@ -93,13 +96,13 @@ export function renderRuntimeConfig( const database = dwhDirect ? { ...legacyDirectConnection(bindings.dwh, { host: name("DWH", "HOST"), port: name("DWH", "PORT"), user: name("DWH", "USER"), - passwordFile: name("DWH", "PASSWORD_FILE"), + passwordFile: name("DWH", "PASSWORD_FILE"), tlsCaFile: name("DWH", "TLS_CA_FILE"), }, dwhIdentity), transport: "direct" } : placeholderConnection(dwhIdentity); const vectorDb = vectorDirect ? legacyDirectConnection(bindings.vector, { host: name("VECTOR", "HOST"), port: name("VECTOR", "PORT"), user: name("VECTOR", "USER"), - passwordFile: name("VECTOR", "PASSWORD_FILE"), + passwordFile: name("VECTOR", "PASSWORD_FILE"), tlsCaFile: name("VECTOR", "TLS_CA_FILE"), }, vectorIdentity) : placeholderConnection(vectorIdentity); const embedding: Record = { diff --git a/backend/test/tht-runner.test.ts b/backend/test/tht-runner.test.ts index 4a618606..62c58ea2 100644 --- a/backend/test/tht-runner.test.ts +++ b/backend/test/tht-runner.test.ts @@ -1,6 +1,9 @@ import { test, expect, vi } from "vitest"; import { EventEmitter } from "node:events"; -import { chmodSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { + chmodSync, lstatSync, mkdirSync, mkdtempSync, readdirSync, rmSync, symlinkSync, + writeFileSync, +} from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { ThtRunner } from "../src/tht/tht-runner.js"; @@ -170,10 +173,110 @@ test("buildArgv appends -c AFTER the subcommand (never a global -c)", () => { }); test("buildArgv passes an absolute immutable snapshot after the tht subcommand", () => { - const r = new ThtRunner({ thtBin: "tht", harnessDir: "/nope", configPath: "config/tht.yaml" }); - expect(r.buildArgv(["session", "new"], "/data/workspace-registry/snapshots/a/psd-clinical.yaml")).toEqual([ - "session", "new", "-c", "/data/workspace-registry/snapshots/a/psd-clinical.yaml", - ]); + const root = mkdtempSync(join(tmpdir(), "tht-runner-snapshot-")); + const snapshotRoot = join(root, "snapshots", "runtime"); + mkdirSync(snapshotRoot, { recursive: true, mode: 0o700 }); + const r = new ThtRunner({ + thtBin: "tht", harnessDir: "/nope", configPath: "config/tht.yaml", runtimeSnapshotRoot: snapshotRoot, + }); + try { + const snapshot = r.createRuntimeSnapshot("language: en\n"); + expect(lstatSync(snapshot).isFile()).toBe(true); + expect(lstatSync(snapshot).mode & 0o777).toBe(0o400); + expect(r.buildArgv(["session", "new"], snapshot)).toEqual([ + "session", "new", "-c", snapshot, + ]); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +test("absolute config paths must be unmodified runner-created snapshots", () => { + const root = mkdtempSync(join(tmpdir(), "tht-runner-snapshot-")); + const snapshotRoot = join(root, "snapshots", "runtime"); + const outside = join(root, "outside.yaml"); + mkdirSync(snapshotRoot, { recursive: true, mode: 0o700 }); + writeFileSync(outside, "language: en\n"); + const r = new ThtRunner({ + thtBin: "tht", harnessDir: "/nope", configPath: "config/tht.yaml", runtimeSnapshotRoot: snapshotRoot, + }); + try { + expect(() => r.buildArgv(["session", "new"], "/tmp/untrusted.yaml")) + .toThrow(/trusted runtime snapshot/i); + expect(() => r.buildArgv(["session", "new"], outside)) + .toThrow(/trusted runtime snapshot/i); + + const snapshot = r.createRuntimeSnapshot("language: en\n"); + chmodSync(snapshot, 0o600); + writeFileSync(snapshot, "language: it\n"); + chmodSync(snapshot, 0o400); + expect(() => r.buildArgv(["session", "new"], snapshot)) + .toThrow(/trusted runtime snapshot/i); + + const symlink = join(snapshotRoot, "symlink.yaml"); + symlinkSync(outside, symlink); + expect(() => r.buildArgv(["session", "new"], symlink)) + .toThrow(/trusted runtime snapshot/i); + + const directory = join(snapshotRoot, "directory.yaml"); + mkdirSync(directory); + expect(() => r.buildArgv(["session", "new"], directory)) + .toThrow(/trusted runtime snapshot/i); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +test("runtime snapshots require an absolute configured root", () => { + const relativeRoot = `tht-runner-relative-${Date.now()}`; + const r = new ThtRunner({ + thtBin: "tht", harnessDir: "/nope", configPath: "config/tht.yaml", runtimeSnapshotRoot: relativeRoot, + }); + try { + expect(() => r.createRuntimeSnapshot("language: en\n")).toThrow(/runtime snapshot root/i); + } finally { + rmSync(join(process.cwd(), relativeRoot), { recursive: true, force: true }); + } +}); + +test("runtime snapshots are consumed through a read-only descriptor and cleaned after success", async () => { + const root = mkdtempSync(join(tmpdir(), "tht-runner-snapshot-")); + const snapshotRoot = join(root, "snapshots", "runtime"); + const r = new ThtRunner({ + thtBin: "tht", harnessDir: "/nope", configPath: "config/tht.yaml", runtimeSnapshotRoot: snapshotRoot, + }); + try { + (spawn as any).mockClear(); + await r.runWithRuntimeSnapshot(["session", "list", "--json"], "language: en\n"); + const [, argv, options] = (spawn as any).mock.calls[0]; + expect(argv.slice(-2)).toEqual(["-c", "/dev/fd/3"]); + expect(options.stdio).toHaveLength(4); + expect(readdirSync(snapshotRoot)).toEqual([]); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +test("runtime snapshots are cleaned after a failed child", async () => { + const root = mkdtempSync(join(tmpdir(), "tht-runner-snapshot-")); + const snapshotRoot = join(root, "snapshots", "runtime"); + const r = new ThtRunner({ + thtBin: "tht", harnessDir: "/nope", configPath: "config/tht.yaml", runtimeSnapshotRoot: snapshotRoot, + }); + try { + (spawn as any).mockImplementationOnce(() => { + const ch: any = new EventEmitter(); + ch.stdout = new EventEmitter(); + ch.stderr = new EventEmitter(); + queueMicrotask(() => ch.emit("close", 1)); + return ch; + }); + const result = await r.runWithRuntimeSnapshot(["session", "list", "--json"], "language: en\n"); + expect(result.code).toBe(1); + expect(readdirSync(snapshotRoot)).toEqual([]); + } finally { + rmSync(root, { recursive: true, force: true }); + } }); test("sqlPreview argv has no positional file — uses --session to resolve path", async () => { diff --git a/backend/test/workspace-runtime-renderer.test.ts b/backend/test/workspace-runtime-renderer.test.ts index f7dcbc48..e0a047fd 100644 --- a/backend/test/workspace-runtime-renderer.test.ts +++ b/backend/test/workspace-runtime-renderer.test.ts @@ -42,6 +42,7 @@ const directBindings: RuntimeBindings = { THT_WS_PSD_CLINICAL_DWH_PORT: "5432", THT_WS_PSD_CLINICAL_DWH_USER: "thoth_reader", THT_WS_PSD_CLINICAL_DWH_PASSWORD_FILE: "/run/secrets/dwh-password", + THT_WS_PSD_CLINICAL_DWH_TLS_CA_FILE: "/run/secrets/dwh-ca.pem", }, }, vector: { @@ -52,6 +53,7 @@ const directBindings: RuntimeBindings = { THT_WS_PSD_CLINICAL_VECTOR_PORT: "5432", THT_WS_PSD_CLINICAL_VECTOR_USER: "vector_reader", THT_WS_PSD_CLINICAL_VECTOR_PASSWORD_FILE: "/run/secrets/vector-password", + THT_WS_PSD_CLINICAL_VECTOR_TLS_CA_FILE: "/run/secrets/vector-ca.pem", }, }, embedding: { @@ -74,12 +76,14 @@ test("renders a direct PostgreSQL binding to the legacy harness shape", () => { schema: "datawarehouse", user: "thoth_reader", password_file: "/run/secrets/dwh-password", + ssl_ca_file: "/run/secrets/dwh-ca.pem", transport: "direct", }, vector_db: { host: "vector.internal", schema: "datawarehouse", password_file: "/run/secrets/vector-password", + ssl_ca_file: "/run/secrets/vector-ca.pem", }, embeddings: { base_url: "http://embedding.internal:11434", @@ -92,6 +96,28 @@ test("renders a direct PostgreSQL binding to the legacy harness shape", () => { expect(yaml).toContain("schema: datawarehouse"); }); +test("omits direct TLS fields when binding validation did not retain a file path", () => { + const dwhValues = { ...directBindings.dwh.values }; + const vectorValues = { ...directBindings.vector.values }; + delete dwhValues.THT_WS_PSD_CLINICAL_DWH_TLS_CA_FILE; + delete vectorValues.THT_WS_PSD_CLINICAL_VECTOR_TLS_CA_FILE; + const yaml = renderRuntimeConfig(workspace, { + ...directBindings, + dwh: { + ...directBindings.dwh, + values: dwhValues, + }, + vector: { + ...directBindings.vector, + values: vectorValues, + }, + }, paths); + const rendered = parse(yaml); + + expect(rendered.database).not.toHaveProperty("ssl_ca_file"); + expect(rendered.vector_db).not.toHaveProperty("ssl_ca_file"); +}); + test("renders REST bindings through the legacy rest sections without secret values", () => { const yaml = renderRuntimeConfig(workspace, { ...directBindings, diff --git a/backend/test/workspaces-bindings.test.ts b/backend/test/workspaces-bindings.test.ts index 68fbe6a7..fbfee982 100644 --- a/backend/test/workspaces-bindings.test.ts +++ b/backend/test/workspaces-bindings.test.ts @@ -102,6 +102,7 @@ test("reports an invalid optional secret file instead of silently dropping it", }, [password.root]); expect(result.missing).toContain("THT_WS_PSD_CLINICAL_DWH_TLS_CA_FILE"); + expect(result.values).not.toHaveProperty("THT_WS_PSD_CLINICAL_DWH_TLS_CA_FILE"); }); test("rejects a selected transport that the canonical workspace does not support", () => {