diff --git a/.superpowers/sdd/2026-08-03-diagnostic-contract-extension/task-3-report.md b/.superpowers/sdd/2026-08-03-diagnostic-contract-extension/task-3-report.md index afe0c419..246794e4 100644 --- a/.superpowers/sdd/2026-08-03-diagnostic-contract-extension/task-3-report.md +++ b/.superpowers/sdd/2026-08-03-diagnostic-contract-extension/task-3-report.md @@ -73,3 +73,33 @@ failed and no diagnostic secret was emitted. ## Blockers None. + +## Round 2 remediation + +The final review found two remaining contract gaps. The binding resolver already treated +`auth: none` as credential-free, but the runtime renderer and diagnostic connector still required +the API-key file. Rendering and connector construction now make that requirement conditional on +the declared REST authentication mode, so a DWH/vector `auth: none` workspace passes resolver, +runtime rendering, and diagnostics with no API-key file. + +SSH forwarding previously changed the PostgreSQL connection host to `127.0.0.1` without retaining +the original target for TLS hostname validation. Forwarded probes now carry `SSH_TARGET_HOST` as +`tlsServername` into the PostgreSQL TLS options; private CA and verified system trust behavior are +unchanged. + +TDD RED: the new end-to-end no-key test failed at the unconditional runtime +`API_KEY_FILE` requirement, while the SSH test showed no `tlsServername` on the loopback probe or +database-client request. TDD GREEN: the focused backend workspace tests passed `40/40`. + +Round 2 final verification: + +```text +backend: npx vitest run +31 test files passed; 332 tests passed + +backend: npx tsc --noEmit -p . +exit 0 + +repository: git diff --check +exit 0 +``` diff --git a/backend/src/workspaces/diagnostics.ts b/backend/src/workspaces/diagnostics.ts index b678dc32..945c08bd 100644 --- a/backend/src/workspaces/diagnostics.ts +++ b/backend/src/workspaces/diagnostics.ts @@ -50,6 +50,7 @@ export interface ConnectorDiagnosticRequest { user?: string; credentialFile?: string; tlsCaFile?: string; + tlsServername?: string; resource: DiagnosticResource; timeoutMs: number; signal: AbortSignal; @@ -87,6 +88,7 @@ export interface VectorDiagnosticRequest { baseUrl?: string; credentialFile?: string; tlsCaFile?: string; + tlsServername?: string; diagnostic?: RestDiagnosticRequest & { response: { collection: string; dimensions: string; distance: string }; }; @@ -147,7 +149,7 @@ export interface DatabaseDiagnosticClient { export interface DatabaseDiagnosticClientFactory { connect(request: { - host: string; port: number; database: string; user: string; credentialFile: string; tlsCaFile?: string; signal: AbortSignal; + host: string; port: number; database: string; user: string; credentialFile: string; tlsCaFile?: string; tlsServername?: string; signal: AbortSignal; }): Promise; } @@ -298,13 +300,15 @@ export function createConcreteDiagnosticAdapters( }, }; const databaseClient = dependencies.databaseClient ?? { - async connect(request: { host: string; port: number; database: string; user: string; credentialFile: string; tlsCaFile?: string; signal: AbortSignal }) { + async connect(request: { host: string; port: number; database: string; user: string; credentialFile: string; tlsCaFile?: string; tlsServername?: string; signal: AbortSignal }) { const client = new Client({ 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"), rejectUnauthorized: true } - : { rejectUnauthorized: true }, + ssl: { + ...(request.tlsCaFile ? { ca: await readFile(request.tlsCaFile, "utf8") } : {}), + ...(request.tlsServername ? { servername: request.tlsServername } : {}), + rejectUnauthorized: true, + }, connectionTimeoutMillis: 5_000, }); const abort = () => { void client.end(); }; @@ -330,7 +334,8 @@ export function createConcreteDiagnosticAdapters( if (!database || !schema) throw new Error("direct probe failed"); const client = await databaseClient.connect({ host: request.host, port: request.port, database, user: request.user, - credentialFile: request.credentialFile, tlsCaFile: request.tlsCaFile, signal: request.signal, + credentialFile: request.credentialFile, tlsCaFile: request.tlsCaFile, + tlsServername: request.tlsServername, signal: request.signal, }); try { const result = await client.query("SELECT current_database() AS database, current_schema() AS schema", []); @@ -398,7 +403,8 @@ export function createConcreteDiagnosticAdapters( } const client = await databaseClient.connect({ host: request.host, port: request.port, database: resource.database, user: request.user, - credentialFile: request.credentialFile, tlsCaFile: request.tlsCaFile, signal: request.signal, + credentialFile: request.credentialFile, tlsCaFile: request.tlsCaFile, + tlsServername: request.tlsServername, signal: request.signal, }); try { const metadata = await client.query( @@ -584,15 +590,14 @@ function connectorRequest( collection: workspace.semantic_index.vector_store.collection, }; const field = (suffix: string) => bindingName(workspace, contractRole, suffix); - const credentialFile = values[field(binding.transport === "rest_api" ? "API_KEY_FILE" : "PASSWORD_FILE")]; - if (credentialFile === undefined) return undefined; - if (binding.transport === "rest_api") { const baseUrl = values[field("BASE_URL")]; const diagnostic = role === "dwh" ? workspace.diagnostics?.dwh_rest : workspace.diagnostics?.vector_rest?.metadata; if (baseUrl === undefined || diagnostic === undefined) return undefined; + const credentialFile = diagnostic.auth === "none" ? undefined : values[field("API_KEY_FILE")]; + if (diagnostic.auth !== "none" && credentialFile === undefined) return undefined; return { role, transport: "rest_api", @@ -621,6 +626,9 @@ function connectorRequest( }; } + const credentialFile = values[field("PASSWORD_FILE")]; + if (credentialFile === undefined) return undefined; + const host = values[field("HOST")]; const port = numericBinding(values, field("PORT")); const user = values[field("USER")]; @@ -660,6 +668,7 @@ function tunnelProbeRequest( user, credentialFile: password, tlsCaFile: binding.values[bindingName(workspace, contractRole, "TLS_CA_FILE")], + tlsServername: binding.values[bindingName(workspace, contractRole, "SSH_TARGET_HOST")], resource: role === "dwh" ? { database: workspace.dwh.database, schema: workspace.dwh.schema } : { @@ -710,7 +719,8 @@ export function createWorkspaceDiagnoser( tunneledVectorMetadata = await adapters.inspectVector({ transport: "ssh_tunnel", host: tunneledRequest.host, port: tunneledRequest.port, user: tunneledRequest.user, credentialFile: tunneledRequest.credentialFile, - tlsCaFile: tunneledRequest.tlsCaFile, resource: tunneledRequest.resource, + tlsCaFile: tunneledRequest.tlsCaFile, tlsServername: tunneledRequest.tlsServername, + resource: tunneledRequest.resource, collection: canonical.semantic_index.vector_store.collection, dimensions: canonical.semantic_index.vector_store.dimensions, distance: canonical.semantic_index.vector_store.distance, diff --git a/backend/src/workspaces/runtime-renderer.ts b/backend/src/workspaces/runtime-renderer.ts index d5a5094f..d922361a 100644 --- a/backend/src/workspaces/runtime-renderer.ts +++ b/backend/src/workspaces/runtime-renderer.ts @@ -45,11 +45,12 @@ function legacyDirectConnection( function legacyRestEndpoint( binding: ResolvedBinding, names: { baseUrl: string; apiKeyFile: string; tlsCaFile: string }, + requiresCredential: boolean, ): Record { const endpoint: Record = { base_url: requireBinding(binding, names.baseUrl), - api_key_file: requireBinding(binding, names.apiKeyFile), }; + if (requiresCredential) endpoint.api_key_file = requireBinding(binding, names.apiKeyFile); const tlsCaFile = bindingValue(binding, names.tlsCaFile); if (tlsCaFile !== undefined) endpoint.ssl_ca_file = tlsCaFile; return endpoint; @@ -124,7 +125,7 @@ export function renderRuntimeConfig( const rest = legacyRestEndpoint(bindings.dwh, { baseUrl: name("DWH", "BASE_URL"), apiKeyFile: name("DWH", "API_KEY_FILE"), tlsCaFile: name("DWH", "TLS_CA_FILE"), - }); + }, canonical.diagnostics?.dwh_rest?.auth !== "none"); rendered.rest = rest; rendered.dwh = { type: "thoth_rest", database: dwhIdentity, endpoint: rest }; } else { @@ -136,7 +137,7 @@ export function renderRuntimeConfig( const vectorRest = legacyRestEndpoint(bindings.vector, { baseUrl: name("VECTOR", "BASE_URL"), apiKeyFile: name("VECTOR", "API_KEY_FILE"), tlsCaFile: name("VECTOR", "TLS_CA_FILE"), - }); + }, canonical.diagnostics?.vector_rest?.metadata.auth !== "none"); rendered.vector_rest = vectorRest; rendered.vectors = { type: "thoth_vector_http", reader: vectorRest }; } else { diff --git a/backend/test/workspaces-diagnostics.test.ts b/backend/test/workspaces-diagnostics.test.ts index 1f8cc0d4..cae90433 100644 --- a/backend/test/workspaces-diagnostics.test.ts +++ b/backend/test/workspaces-diagnostics.test.ts @@ -10,7 +10,7 @@ import { type DiagnosticAdapters, } from "../src/workspaces/diagnostics.js"; import { resolveRuntimeBindings } from "../src/workspaces/bindings.js"; -import type { RuntimeBindings } from "../src/workspaces/runtime-renderer.js"; +import { renderRuntimeConfig, type RuntimeBindings } from "../src/workspaces/runtime-renderer.js"; import { parseWorkspaceYaml } from "../src/workspaces/schema.js"; const workspace = parseWorkspaceYaml(`workspace: @@ -268,6 +268,74 @@ test("checks direct and REST resolution, TLS, authentication, and resource metad expect(JSON.stringify(result)).not.toContain("/run/secrets/dwh-api-key"); }); +test("carries auth-none REST bindings from resolver through runtime rendering to diagnostics without a key", async () => { + const unauthenticatedWorkspace = parseWorkspaceYaml(`workspace: + schema_version: 2 + id: psd-clinical + name: Policlinico San Donato + language: it +dwh: + engine: postgres + database: warehouse + schema: datawarehouse + supported_transports: [rest_api] +semantic_index: + vector_store: + engine: pgvector + database: postgres + schema: vectors + collection: clinical_documents + dimensions: 768 + distance: cosine + supported_transports: [rest_api] + embedding: + provider: ollama_compatible + model: nomic-embed-text-v2-moe + dimensions: 768 +diagnostics: + dwh_rest: + method: POST + path: /rpc/ping + auth: none + response: { database: database, schema: schema } + vector_rest: + metadata: + method: GET + path: /vector/metadata + auth: none + response: { collection: collection, dimensions: dimensions, distance: distance } + embedding: + method: GET + path: /models + auth: none + response: { model: model, dimensions: dimensions } +llm_policy: + allowed: [zai/glm-5.2] +`); + const resolved = resolveRuntimeBindings(unauthenticatedWorkspace, { + THT_WS_PSD_CLINICAL_DWH_TRANSPORT: "rest_api", + THT_WS_PSD_CLINICAL_DWH_BASE_URL: "https://dwh.example.test", + THT_WS_PSD_CLINICAL_VECTOR_TRANSPORT: "rest_api", + THT_WS_PSD_CLINICAL_VECTOR_BASE_URL: "https://vector.example.test", + THT_WS_PSD_CLINICAL_EMBEDDING_BASE_URL: "https://embedding.example.test", + }, ["/run/secrets"]); + const adapters = successfulAdapters(); + + const runtime = renderRuntimeConfig(unauthenticatedWorkspace, resolved, { + sessions: "/data/sessions", artifacts: "/data/artifacts", indexes: "/data/indexes", + }); + const result = await diagnose(adapters)(unauthenticatedWorkspace, resolved, { writeProbe: false }); + + expect(runtime).not.toContain("api_key_file"); + expect(result.activatable).toBe(true); + expect(adapters.probeConnector).toHaveBeenCalledWith(expect.objectContaining({ + role: "dwh", credentialFile: undefined, + })); + expect(adapters.probeConnector).toHaveBeenCalledWith(expect.objectContaining({ + role: "vector", credentialFile: undefined, + })); +}); + test("uses a loopback-only SSH tunnel for the bounded connector probe", async () => { const adapters = successfulAdapters(); const sshBindings: RuntimeBindings = { @@ -305,6 +373,35 @@ test("uses a loopback-only SSH tunnel for the bounded connector probe", async () })); }); +test("retains the SSH target hostname for forwarded PostgreSQL TLS validation", async () => { + const adapters = successfulAdapters(); + const sshBindings: RuntimeBindings = { + ...bindings, + dwh: { + transport: "ssh_tunnel", + missing: [], + values: { + THT_WS_PSD_CLINICAL_DWH_USER: "reader", + THT_WS_PSD_CLINICAL_DWH_PASSWORD_FILE: "/run/secrets/dwh-password", + THT_WS_PSD_CLINICAL_DWH_SSH_HOST: "bastion.example.test", + THT_WS_PSD_CLINICAL_DWH_SSH_PORT: "22", + THT_WS_PSD_CLINICAL_DWH_SSH_USER: "tunnel", + THT_WS_PSD_CLINICAL_DWH_SSH_PRIVATE_KEY_FILE: "/run/secrets/ssh-key", + THT_WS_PSD_CLINICAL_DWH_SSH_KNOWN_HOSTS_FILE: "/run/secrets/known-hosts", + THT_WS_PSD_CLINICAL_DWH_SSH_TARGET_HOST: "dwh.internal", + THT_WS_PSD_CLINICAL_DWH_SSH_TARGET_PORT: "5432", + }, + }, + }; + + await diagnose(adapters)(workspace, sshBindings, { writeProbe: false }); + + expect(adapters.probeConnector).toHaveBeenCalledWith(expect.objectContaining({ + host: "127.0.0.1", + tlsServername: "dwh.internal", + })); +}); + test("passes the declared vector database and schema to direct diagnostics", async () => { const adapters = successfulAdapters(); @@ -622,6 +719,31 @@ test("uses system trust for direct and SSH PostgreSQL diagnostics when no CA bin } }); +test("passes the original target hostname to the PostgreSQL TLS client", async () => { + const directory = await mkdtemp(join(tmpdir(), "thothii-diagnostic-")); + const passwordFile = join(directory, "password"); + await writeFile(passwordFile, "password\n", { mode: 0o600 }); + const connect = vi.fn(async () => ({ + query: vi.fn(async () => ({ rows: [{ database: "warehouse", schema: "datawarehouse" }] })), + end: vi.fn(async () => undefined), + })); + try { + await createConcreteDiagnosticAdapters({ databaseClient: { connect } } as any).probeConnector({ + role: "dwh", transport: "ssh_tunnel", host: "127.0.0.1", port: 5432, user: "reader", + credentialFile: passwordFile, tlsServername: "dwh.internal", + resource: { database: "warehouse", schema: "datawarehouse" }, timeoutMs: 5000, + signal: new AbortController().signal, + }); + + expect(connect).toHaveBeenCalledWith(expect.objectContaining({ + host: "127.0.0.1", + tlsServername: "dwh.internal", + })); + } finally { + await rm(directory, { recursive: true, force: true }); + } +}); + test("selects the vector index containing the declared vector column for direct metadata", async () => { const directory = await mkdtemp(join(tmpdir(), "thothii-diagnostic-")); const passwordFile = join(directory, "password"); diff --git a/docs/superpowers/specs/2026-08-03-git-workspace-registry-design.md b/docs/superpowers/specs/2026-08-03-git-workspace-registry-design.md index 69a2083f..5714168c 100644 --- a/docs/superpowers/specs/2026-08-03-git-workspace-registry-design.md +++ b/docs/superpowers/specs/2026-08-03-git-workspace-registry-design.md @@ -220,6 +220,9 @@ the model/dimensions response fields. Only `GET` and `POST`, `none`/`bearer`/`x- authentication, origin-relative paths without a query or fragment, and identifier-shaped response field names are accepted. +For `auth: none`, the binding resolver, runtime renderer, and diagnostic connector all omit the +API-key requirement. Credential-backed declarations retain their local secret-file requirement. + `vector_rest.reversible_probe`, when present, is an authenticated POST with a declared response field that must echo each requested `create`/`remove` operation. It is called with a generated diagnostic record create request and a matching remove request, with cleanup retried in `finally`. @@ -330,7 +333,9 @@ trusted TLS-termination boundary. SSH adapters verify the remote host against an explicit known-hosts file, open a temporary local tunnel, and pass the resulting endpoint to the corresponding direct adapter, including its -verified private-CA-or-system-trust policy. Host-key checking cannot be disabled by the form. +verified private-CA-or-system-trust policy. The direct adapter connects to loopback but uses the +original `SSH_TARGET_HOST` as the TLS server name, so certificate hostname validation remains +bound to the remote target. Host-key checking cannot be disabled by the form. Transport selection is installation-specific because a production server may connect directly while a laptop reaches the same logical resource through REST or SSH. diff --git a/docs/workspace-diagnostic-protocol.md b/docs/workspace-diagnostic-protocol.md index 3ab1d13e..8e620b6e 100644 --- a/docs/workspace-diagnostic-protocol.md +++ b/docs/workspace-diagnostic-protocol.md @@ -19,8 +19,8 @@ belongs in the descriptor, this document, a generated `.env.example`, or diagnos fragment. The client may use only the declared method, path, auth mode, and response-field names. - `auth: none` sends no credential; `auth: bearer` reads a local file and sends `Authorization: Bearer `; `auth: x-api-key` sends `x-api-key: `. - The resolver does not require or read an API-key file for an `auth: none` diagnostic. File - content is never logged or returned. + The resolver, rendered runtime endpoint, and diagnoser do not require or read an API-key file + for an `auth: none` diagnostic. File content is never logged or returned. ## Canonical descriptor additions @@ -117,6 +117,10 @@ Both returned values must equal the descriptor's DWH database and schema. with certificate verification; when absent, the native client still requires a valid certificate chain from the runtime system trust store. Absence never disables TLS verification. +An SSH tunnel changes only the TCP peer to loopback. The forwarded PostgreSQL TLS connection sets +its server name to `_SSH_TARGET_HOST`, so certificate hostname validation remains against the +declared remote target rather than `127.0.0.1`. + For REST, the descriptor above declares the exact ping: ```text @@ -142,7 +146,8 @@ collection, integer `dimensions`, and `distance` (`cosine`, `l2`, or `inner_prod `semantic_index.vector_store`. Their optional `*_TLS_CA_FILE` follows the same verified private-CA-or-system-trust rule as the -DWH diagnostic. +DWH diagnostic. For an SSH tunnel, their TLS server name is likewise the declared vector +`SSH_TARGET_HOST`, not the loopback listener. For REST, the exact descriptor-declared request is, for example: