From cc72e65f9d9e7b5a7691ff87ed365f1b190cfd96 Mon Sep 17 00:00:00 2001 From: User Date: Sat, 22 Aug 2026 13:49:21 +0200 Subject: [PATCH] fix(workspaces): align postgres connection diagnostics --- backend/src/auth/diagnostics.ts | 10 ++- backend/src/workspaces/diagnostics.ts | 48 +++++++++--- backend/test/auth-diagnostics.test.ts | 34 +++++++++ backend/test/workspaces-diagnostics.test.ts | 85 ++++++++++++++++++++- 4 files changed, 165 insertions(+), 12 deletions(-) diff --git a/backend/src/auth/diagnostics.ts b/backend/src/auth/diagnostics.ts index 32858529..84744dc5 100644 --- a/backend/src/auth/diagnostics.ts +++ b/backend/src/auth/diagnostics.ts @@ -129,8 +129,14 @@ export function createAuthDiagnoser(deps: AuthDiagnoserDependencies): AuthDiagno : { ready: false, mode: deps.authMode, checks: result }; } if (deps.authMode === "upstream") { - checks.push(check("auth_config_incomplete", "The deprecated upstream authentication mode is not certifiable.")); - return { ready: false, mode: deps.authMode, checks: ordered(checks) }; + const result = ordered(checks); + return result.length === 0 + ? { + ready: true, + mode: "upstream", + checks: [{ level: "info", code: "auth_ready", message: "Authentication is ready." }], + } + : { ready: false, mode: "upstream", checks: result }; } let loaded; diff --git a/backend/src/workspaces/diagnostics.ts b/backend/src/workspaces/diagnostics.ts index 48e9b253..2921587f 100644 --- a/backend/src/workspaces/diagnostics.ts +++ b/backend/src/workspaces/diagnostics.ts @@ -1,5 +1,5 @@ import { readFile } from "node:fs/promises"; -import { Client } from "pg"; +import { Client, type ClientConfig } from "pg"; import { MAX_WORKSPACE_DIAGNOSTIC_TIMEOUT_MS } from "../config.js"; import { buildInstallationContract } from "./contracts.js"; import type { RuntimeBindings } from "./runtime-renderer.js"; @@ -111,9 +111,16 @@ export interface DatabaseDiagnosticClientFactory { }): Promise; } +export interface PostgreSqlDiagnosticWireClient { + connect(): Promise; + query(sql: string, values: readonly unknown[]): Promise<{ rows: Array> }>; + end(): Promise; +} + export interface ConcreteDiagnosticAdapterDependencies { directProtocol?: DirectProtocolFactory; databaseClient?: DatabaseDiagnosticClientFactory; + createPostgresClient?: (config: ClientConfig) => PostgreSqlDiagnosticWireClient; } /** Adapters retain only diagnostic metadata and never return credential contents or bodies. */ @@ -146,6 +153,15 @@ async function restHeaders( export function createConcreteDiagnosticAdapters( dependencies: ConcreteDiagnosticAdapterDependencies = {}, ): DiagnosticAdapters { + const createPostgresClient = dependencies.createPostgresClient + ?? ((config: ClientConfig): PostgreSqlDiagnosticWireClient => { + const client = new Client(config); + return { + connect: async () => { await client.connect(); }, + query: async (sql, values) => await client.query(sql, [...values]), + end: async () => await client.end(), + }; + }); const databaseClient = dependencies.databaseClient ?? { async connect(request: { host: string; @@ -157,17 +173,21 @@ export function createConcreteDiagnosticAdapters( tlsServername?: string; signal: AbortSignal; }) { - const client = new Client({ + const tlsConfigured = request.tlsCaFile !== undefined || request.tlsServername !== undefined; + const ssl: ClientConfig["ssl"] = tlsConfigured + ? { + ...(request.tlsCaFile ? { ca: await readFile(request.tlsCaFile, "utf8") } : {}), + ...(request.tlsServername ? { servername: request.tlsServername } : {}), + rejectUnauthorized: true, + } + : false; + const client = createPostgresClient({ host: request.host, port: request.port, database: request.database, user: request.user, password: (await readFile(request.credentialFile, "utf8")).trim(), - ssl: { - ...(request.tlsCaFile ? { ca: await readFile(request.tlsCaFile, "utf8") } : {}), - ...(request.tlsServername ? { servername: request.tlsServername } : {}), - rejectUnauthorized: true, - }, + ssl, connectionTimeoutMillis: 5_000, }); const abort = () => { void client.end(); }; @@ -209,8 +229,18 @@ export function createConcreteDiagnosticAdapters( }); try { const result = await client.query( - "SELECT current_database() AS database, current_schema() AS schema", - [], + `SELECT + current_database() AS database, + CASE + WHEN pg_catalog.has_schema_privilege( + current_user, + (SELECT oid FROM pg_catalog.pg_namespace WHERE nspname = $1), + 'USAGE' + ) + THEN $1 + ELSE NULL + END AS schema`, + [schema], ); const row = result.rows[0]; if (row?.database !== database || row.schema !== schema) throw new Error("direct probe failed"); diff --git a/backend/test/auth-diagnostics.test.ts b/backend/test/auth-diagnostics.test.ts index 894b589f..e124d236 100644 --- a/backend/test/auth-diagnostics.test.ts +++ b/backend/test/auth-diagnostics.test.ts @@ -60,6 +60,40 @@ function registryYaml(role: "user" | "admin", passwordHash = validPasswordHash): ].join("\n"); } +test("reports upstream authentication ready when its protected session root is valid", async () => { + const report = await createAuthDiagnoser({ + authMode: "upstream", + authStateRoot: "/safe/auth-state", + sessionRootValidator: acceptSessionRoot, + }).inspect({ live: true }); + + expect(report).toEqual({ + ready: true, + mode: "upstream", + checks: [{ level: "info", code: "auth_ready", message: "Authentication is ready." }], + }); +}); + +test("upstream authentication still fails when its protected session root is invalid", async () => { + const sentinel = "synthetic-upstream-session-root-secret"; + const report = await createAuthDiagnoser({ + authMode: "upstream", + authStateRoot: "/safe/auth-state", + sessionRootValidator: async () => { throw new Error(sentinel); }, + }).inspect({ live: true }); + + expect(report).toEqual({ + ready: false, + mode: "upstream", + checks: [{ + level: "error", + code: "auth_session_store_invalid", + message: "The authentication session store is invalid.", + }], + }); + expect(JSON.stringify(report)).not.toContain(sentinel); +}); + test("reports deterministic live OIDC checks and silently ignores unrelated groups", async () => { const oidcDiagnose = vi.fn(async () => undefined); const fetch = vi.fn(async (input) => { diff --git a/backend/test/workspaces-diagnostics.test.ts b/backend/test/workspaces-diagnostics.test.ts index 739c45d7..16c82087 100644 --- a/backend/test/workspaces-diagnostics.test.ts +++ b/backend/test/workspaces-diagnostics.test.ts @@ -205,8 +205,9 @@ test("concrete DWH direct diagnostics authenticate, verify resource identity, an const passwordFile = join(root, "password"); await writeFile(passwordFile, "password-value"); const end = vi.fn(async () => undefined); + const query = vi.fn(async () => ({ rows: [{ database: "warehouse", schema: "datawarehouse" }] })); const connect = vi.fn(async () => ({ - query: vi.fn(async () => ({ rows: [{ database: "warehouse", schema: "datawarehouse" }] })), + query, end, })); try { @@ -219,6 +220,88 @@ test("concrete DWH direct diagnostics authenticate, verify resource identity, an }); expect(result).toMatchObject({ resolved: true, authenticated: true, tlsVerified: true }); expect(connect).toHaveBeenCalledWith(expect.objectContaining({ credentialFile: passwordFile })); + expect(query).toHaveBeenCalledWith( + expect.stringContaining("pg_catalog.has_schema_privilege"), + ["datawarehouse"], + ); + expect(query.mock.calls[0]?.[0]).toContain("$1"); + expect(query.mock.calls[0]?.[0]).not.toContain('"datawarehouse"'); + expect(end).toHaveBeenCalledOnce(); + } finally { + await rm(root, { recursive: true, force: true }); + } +}); + +test("concrete DWH direct diagnostics disable TLS when no TLS binding is declared", async () => { + const root = await mkdtemp(join(tmpdir(), "thoth-diagnostic-plain-")); + const passwordFile = join(root, "password"); + await writeFile(passwordFile, "password-value"); + const connect = vi.fn(async () => undefined); + const query = vi.fn(async () => ({ rows: [{ database: "warehouse", schema: "datawarehouse" }] })); + const end = vi.fn(async () => undefined); + const createPostgresClient = vi.fn(() => ({ connect, query, end })); + try { + const adapter = createConcreteDiagnosticAdapters({ createPostgresClient }); + await adapter.probeConnector({ + role: "dwh", transport: "postgres_direct", host: "127.0.0.1", port: 5432, + user: "reader", credentialFile: passwordFile, + resource: { database: "warehouse", schema: "datawarehouse" }, + timeoutMs: 1_000, signal: new AbortController().signal, + }); + expect(createPostgresClient).toHaveBeenCalledWith(expect.objectContaining({ ssl: false })); + expect(connect).toHaveBeenCalledOnce(); + expect(end).toHaveBeenCalledOnce(); + } finally { + await rm(root, { recursive: true, force: true }); + } +}); + +test("concrete DWH direct diagnostics use strict TLS only for explicit TLS bindings", async () => { + const root = await mkdtemp(join(tmpdir(), "thoth-diagnostic-tls-")); + const passwordFile = join(root, "password"); + const caFile = join(root, "ca.pem"); + await writeFile(passwordFile, "password-value"); + await writeFile(caFile, "test-ca"); + const connect = vi.fn(async () => undefined); + const query = vi.fn(async () => ({ rows: [{ database: "warehouse", schema: "datawarehouse" }] })); + const end = vi.fn(async () => undefined); + const createPostgresClient = vi.fn(() => ({ connect, query, end })); + try { + const adapter = createConcreteDiagnosticAdapters({ createPostgresClient }); + await adapter.probeConnector({ + role: "dwh", transport: "postgres_direct", host: "dwh.example.test", port: 5432, + user: "reader", credentialFile: passwordFile, tlsCaFile: caFile, + tlsServername: "dwh.example.test", + resource: { database: "warehouse", schema: "datawarehouse" }, + timeoutMs: 1_000, signal: new AbortController().signal, + }); + expect(createPostgresClient).toHaveBeenCalledWith(expect.objectContaining({ + ssl: { ca: "test-ca", servername: "dwh.example.test", rejectUnauthorized: true }, + })); + expect(connect).toHaveBeenCalledOnce(); + expect(end).toHaveBeenCalledOnce(); + } finally { + await rm(root, { recursive: true, force: true }); + } +}); + +test("concrete DWH direct diagnostics fail closed when the declared schema is inaccessible", async () => { + const root = await mkdtemp(join(tmpdir(), "thoth-diagnostic-schema-")); + const passwordFile = join(root, "password"); + await writeFile(passwordFile, "CANARY-DATABASE-SECRET"); + const end = vi.fn(async () => undefined); + const connect = vi.fn(async () => ({ + query: vi.fn(async () => ({ rows: [{ database: "warehouse", schema: null }] })), + end, + })); + try { + const adapter = createConcreteDiagnosticAdapters({ databaseClient: { connect } }); + await expect(adapter.probeConnector({ + role: "dwh", transport: "postgres_direct", host: "127.0.0.1", port: 5432, + user: "reader", credentialFile: passwordFile, + resource: { database: "warehouse", schema: "datawarehouse" }, + timeoutMs: 1_000, signal: new AbortController().signal, + })).rejects.toThrow("direct probe failed"); expect(end).toHaveBeenCalledOnce(); } finally { await rm(root, { recursive: true, force: true });