From 565e93a4561bc97f472e11017bb3a1e1ca6c7dee Mon Sep 17 00:00:00 2001 From: mptyl Date: Tue, 4 Aug 2026 00:37:57 +0200 Subject: [PATCH] fix: align workspace diagnostic contracts --- .../task-3-report.md | 75 ++++++++++++ backend/src/workspaces/bindings.ts | 32 +++++- backend/src/workspaces/diagnostics.ts | 28 +++-- backend/src/workspaces/runtime-renderer.ts | 9 +- backend/src/workspaces/schema.ts | 12 +- .../test/workspace-runtime-renderer.test.ts | 1 + backend/test/workspaces-bindings.test.ts | 60 +++++++++- backend/test/workspaces-contracts.test.ts | 1 + backend/test/workspaces-diagnostics.test.ts | 108 ++++++++++++++++-- backend/test/workspaces-schema.test.ts | 19 +++ ...026-08-03-git-workspace-registry-design.md | 15 ++- docs/workspace-diagnostic-protocol.md | 28 +++-- 12 files changed, 346 insertions(+), 42 deletions(-) create mode 100644 .superpowers/sdd/2026-08-03-diagnostic-contract-extension/task-3-report.md 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 new file mode 100644 index 00000000..afe0c419 --- /dev/null +++ b/.superpowers/sdd/2026-08-03-diagnostic-contract-extension/task-3-report.md @@ -0,0 +1,75 @@ +# Task 3 — Diagnostic contract remediation report + +Date: 2026-08-04 + +## Scope + +This remediation is limited to the four approved review findings for the workspace diagnostic +extension. It does not add registry routes, change workspace publication, alter session startup, +or expand transport support. + +## Changes + +1. `RuntimeBindings` now has an explicit `vectorWriter` binding. The new + `resolveRuntimeBindings()` resolves DWH, vector reader, vector writer, and embedding bindings + together. The diagnoser takes the writer credential only from `bindings.vectorWriter`, never + from vector-reader values. +2. Direct PostgreSQL and SSH-tunnelled direct probes accept an absent CA binding while retaining + certificate verification through the runtime system trust store. A supplied CA still uses + verified private-CA trust. REST private-CA refusal is unchanged. +3. A reversible vector probe now requires an authenticated POST declaration with a response map + containing `operation`. The adapter requires the successful JSON response to echo `create` or + `remove` respectively, so an arbitrary 2xx or an upsert-only response cannot activate the + write probe. +4. For DWH and vector REST diagnostics declared with `auth: none`, the resolver no longer + requires an API-key file and the adapter sends no credential. Credential-backed diagnostics + continue to require their local secret file. + +## TDD evidence + +The first focused RED run failed for the intended missing behavior: + +- `resolveRuntimeBindings is not a function` for unauthenticated resolver bindings; +- schema accepted a reversible probe without a response contract; and +- existing diagnostic fixtures rejected the new `response` declaration until schema support was + implemented. + +The focused GREEN run passed `43/43` tests across: + +- `test/workspaces-bindings.test.ts` +- `test/workspaces-schema.test.ts` +- `test/workspaces-diagnostics.test.ts` + +The regression coverage includes resolver-to-diagnoser writer propagation without manually +inserting the writer key into vector-reader bindings, no-CA direct/SSH system-trust requests, +operation-echo validation for create/remove, and `auth: none` bindings without secret files. + +## Documentation and design + +- `docs/workspace-diagnostic-protocol.md` now documents the verified system-trust fallback, + no-secret `auth: none` behavior, and required reversible response contract. +- `docs/superpowers/specs/2026-08-03-git-workspace-registry-design.md` now records the same + response, CA, SSH, and authentication rules. + +## Final verification + +The initial sandboxed full suite could not bind its local SSE listener (`listen EPERM: +operation not permitted 127.0.0.1`). It was rerun unchanged with local-listener permission. + +```text +backend: npx vitest run +31 test files passed; 329 tests passed + +backend: npx tsc --noEmit -p . +exit 0 + +repository: git diff --check +exit 0 +``` + +Expected test harness stderr from existing Pi/process failure-path tests remained present; no test +failed and no diagnostic secret was emitted. + +## Blockers + +None. diff --git a/backend/src/workspaces/bindings.ts b/backend/src/workspaces/bindings.ts index e5e5e0fd..f991960b 100644 --- a/backend/src/workspaces/bindings.ts +++ b/backend/src/workspaces/bindings.ts @@ -10,6 +10,13 @@ import { type WorkspaceDescriptor, } from "./schema.js"; +export interface RuntimeBindings { + dwh: ResolvedBinding; + vector: ResolvedBinding; + vectorWriter: ResolvedBinding; + embedding: ResolvedBinding; +} + export interface ResolvedBinding { transport: DwhTransport | VectorTransport; values: Record; @@ -63,12 +70,19 @@ function isSafeSecretFile(path: string, secretRoots: readonly string[]): boolean } function requiredSuffixes( + workspace: WorkspaceDescriptor, role: InstallationRole, transport: DwhTransport | VectorTransport, ): readonly InstallationSuffix[] { if (role === "EMBEDDING") return EMBEDDING_REQUIRED_SUFFIXES; if (role === "VECTOR_WRITER") return ["API_KEY_FILE"]; - return REQUIRED_SUFFIXES[role][transport] ?? []; + const required = REQUIRED_SUFFIXES[role][transport] ?? []; + const diagnostic = role === "DWH" + ? workspace.diagnostics?.dwh_rest + : workspace.diagnostics?.vector_rest?.metadata; + return transport === "rest_api" && diagnostic?.auth === "none" + ? required.filter((suffix) => suffix !== "API_KEY_FILE") + : required; } /** @@ -98,7 +112,7 @@ export function resolveBinding( missing.push(transportVariable.name); } - const required = new Set(requiredSuffixes(role, selectedTransport)); + const required = new Set(requiredSuffixes(canonical, role, selectedTransport)); const values: Record = {}; for (const variable of variables) { if (variable.suffix === "TRANSPORT") continue; @@ -115,3 +129,17 @@ export function resolveBinding( return { transport: selectedTransport, values, missing }; } + +/** Resolve all runtime roles together so optional writer credentials cannot be smuggled into reader bindings. */ +export function resolveRuntimeBindings( + workspace: WorkspaceDescriptor, + env: NodeJS.ProcessEnv, + secretRoots: readonly string[], +): RuntimeBindings { + return { + dwh: resolveBinding(workspace, "DWH", env, secretRoots), + vector: resolveBinding(workspace, "VECTOR", env, secretRoots), + vectorWriter: resolveBinding(workspace, "VECTOR_WRITER", env, secretRoots), + embedding: resolveBinding(workspace, "EMBEDDING", env, secretRoots), + }; +} diff --git a/backend/src/workspaces/diagnostics.ts b/backend/src/workspaces/diagnostics.ts index 271ea179..b678dc32 100644 --- a/backend/src/workspaces/diagnostics.ts +++ b/backend/src/workspaces/diagnostics.ts @@ -131,7 +131,9 @@ export interface WriteDiagnosticRecordRequest { credentialFile?: string; tlsCaFile?: string; baseUrl?: string; - diagnostic?: RestDiagnosticRequest; + diagnostic?: RestDiagnosticRequest & { + response: { operation: string }; + }; } export interface DirectProtocolFactory { @@ -145,7 +147,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; signal: AbortSignal; }): Promise; } @@ -296,11 +298,13 @@ 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; 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: { ca: await readFile(request.tlsCaFile, "utf8"), rejectUnauthorized: true }, + ssl: request.tlsCaFile + ? { ca: await readFile(request.tlsCaFile, "utf8"), rejectUnauthorized: true } + : { rejectUnauthorized: true }, connectionTimeoutMillis: 5_000, }); const abort = () => { void client.end(); }; @@ -317,7 +321,7 @@ export function createConcreteDiagnosticAdapters( }; const directProtocol = dependencies.directProtocol ?? { async probe(request: ConnectorDiagnosticRequest): Promise { - if (!request.host || !request.port || !request.user || !request.credentialFile || !request.tlsCaFile + if (!request.host || !request.port || !request.user || !request.credentialFile || !(await secretPresent(request.credentialFile))) { throw new Error("direct probe failed"); } @@ -388,7 +392,7 @@ export function createConcreteDiagnosticAdapters( async inspectVector(request) { if (request.transport === "pgvector_direct" || request.transport === "ssh_tunnel") { const resource = request.resource; - if (!request.host || !request.port || !request.user || !request.credentialFile || !request.tlsCaFile + if (!request.host || !request.port || !request.user || !request.credentialFile || !resource?.database || !resource.schema || !(await secretPresent(request.credentialFile))) { throw new Error("vector metadata adapter is unavailable"); } @@ -454,7 +458,10 @@ export function createConcreteDiagnosticAdapters( signal: request.signal, redirect: "error", }); - if (!response.ok) throw new Error("vector write adapter is unavailable"); + const payload = await response.json().catch(() => undefined) as Record | undefined; + if (!response.ok || !payload || payload[request.diagnostic.response.operation] !== "create") { + throw new Error("vector write adapter is unavailable"); + } }, async removeDiagnosticRecord(request) { if (!request.baseUrl || !request.diagnostic || request.tlsCaFile) throw new Error("vector write adapter is unavailable"); @@ -468,7 +475,10 @@ export function createConcreteDiagnosticAdapters( signal: request.signal, redirect: "error", }); - if (!response.ok) throw new Error("vector write adapter is unavailable"); + const payload = await response.json().catch(() => undefined) as Record | undefined; + if (!response.ok || !payload || payload[request.diagnostic.response.operation] !== "remove") { + throw new Error("vector write adapter is unavailable"); + } }, }; } @@ -789,7 +799,7 @@ export function createWorkspaceDiagnoser( && bindings.vector.transport === "rest_api" && !diagnostics.some((diagnostic) => diagnostic.level === "error") ) { - const credentialFile = bindings.vector.values[bindingName(canonical, "VECTOR_WRITER", "API_KEY_FILE")]; + const credentialFile = bindings.vectorWriter.values[bindingName(canonical, "VECTOR_WRITER", "API_KEY_FILE")]; if (!credentialFile) return { activatable: true, diagnostics }; const readerCredentialFile = bindings.vector.values[bindingName(canonical, "VECTOR", "API_KEY_FILE")]; if (readerCredentialFile && await sameSecretFile(credentialFile, readerCredentialFile)) { diff --git a/backend/src/workspaces/runtime-renderer.ts b/backend/src/workspaces/runtime-renderer.ts index 5695503d..d5a5094f 100644 --- a/backend/src/workspaces/runtime-renderer.ts +++ b/backend/src/workspaces/runtime-renderer.ts @@ -1,13 +1,8 @@ import { stringify } from "yaml"; import { buildInstallationContract } from "./contracts.js"; import { validateCanonicalWorkspace, type WorkspaceDescriptor } from "./schema.js"; -import type { ResolvedBinding } from "./bindings.js"; - -export interface RuntimeBindings { - dwh: ResolvedBinding; - vector: ResolvedBinding; - embedding: ResolvedBinding; -} +import type { ResolvedBinding, RuntimeBindings } from "./bindings.js"; +export type { RuntimeBindings } from "./bindings.js"; export interface RuntimePaths { sessions: string; diff --git a/backend/src/workspaces/schema.ts b/backend/src/workspaces/schema.ts index ce99bb96..e0754fc7 100644 --- a/backend/src/workspaces/schema.ts +++ b/backend/src/workspaces/schema.ts @@ -25,7 +25,11 @@ export interface CanonicalDiagnostics { metadata: RestDiagnosticRequest & { response: { collection: string; dimensions: string; distance: string }; }; - reversible_probe?: RestDiagnosticRequest & { method: "POST" }; + reversible_probe?: RestDiagnosticRequest & { + method: "POST"; + auth: Exclude; + response: { operation: string }; + }; }; embedding?: RestDiagnosticRequest & { response: { model: string; dimensions: string } }; } @@ -124,7 +128,11 @@ const vectorMetadataDiagnostic = restDiagnosticRequest.extend({ distance: responseField, }).strict(), }).strict(); -const reversibleVectorProbe = restDiagnosticRequest.extend({ method: z.literal("POST") }).strict(); +const reversibleVectorProbe = restDiagnosticRequest.extend({ + method: z.literal("POST"), + auth: z.enum(["bearer", "x-api-key"]), + response: z.object({ operation: responseField }).strict(), +}).strict(); const embeddingDiagnostic = restDiagnosticRequest.extend({ response: z.object({ model: responseField, dimensions: responseField }).strict(), }).strict(); diff --git a/backend/test/workspace-runtime-renderer.test.ts b/backend/test/workspace-runtime-renderer.test.ts index 2c682f54..c677312b 100644 --- a/backend/test/workspace-runtime-renderer.test.ts +++ b/backend/test/workspace-runtime-renderer.test.ts @@ -82,6 +82,7 @@ const directBindings: RuntimeBindings = { THT_WS_PSD_CLINICAL_VECTOR_TLS_CA_FILE: "/run/secrets/vector-ca.pem", }, }, + vectorWriter: { transport: "rest_api", missing: [], values: {} }, embedding: { transport: "rest_api", missing: [], diff --git a/backend/test/workspaces-bindings.test.ts b/backend/test/workspaces-bindings.test.ts index 2418e334..feee8c0e 100644 --- a/backend/test/workspaces-bindings.test.ts +++ b/backend/test/workspaces-bindings.test.ts @@ -2,7 +2,7 @@ import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, expect, test } from "vitest"; -import { resolveBinding } from "../src/workspaces/bindings.js"; +import { resolveBinding, resolveRuntimeBindings } from "../src/workspaces/bindings.js"; import { parseWorkspaceYaml } from "../src/workspaces/schema.js"; const workspace = parseWorkspaceYaml(`workspace: @@ -56,6 +56,64 @@ test("marks a portable workspace non-activatable when its local REST key file is expect(result.missing).toContain("THT_WS_PSD_CLINICAL_DWH_API_KEY_FILE"); }); +test("does not require a REST secret file when its declared diagnostic uses auth none", () => { + const unauthenticatedWorkspace = parseWorkspaceYaml(`workspace: + schema_version: 2 + id: psd-clinical + name: Policlinico San Donato + language: it +dwh: + engine: postgres + database: postgres + 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 bindings = 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"]); + + expect(bindings.dwh.missing).toEqual([]); + expect(bindings.vector.missing).toEqual([]); + expect(bindings.embedding.missing).toEqual([]); +}); + test("resolves direct bindings from the stable workspace namespace", () => { const password = secretPath("dwh-password"); const result = resolveBinding(workspace, "DWH", { diff --git a/backend/test/workspaces-contracts.test.ts b/backend/test/workspaces-contracts.test.ts index a51361bc..357fd499 100644 --- a/backend/test/workspaces-contracts.test.ts +++ b/backend/test/workspaces-contracts.test.ts @@ -147,6 +147,7 @@ llm_policy: THT_WS_PSD_CLINICAL_VECTOR_PASSWORD_FILE: "/run/secrets/vector-reader", }, }, + vectorWriter: { transport: "rest_api", missing: [], values: {} }, embedding: { transport: "rest_api", missing: [], diff --git a/backend/test/workspaces-diagnostics.test.ts b/backend/test/workspaces-diagnostics.test.ts index 0f150e2b..1f8cc0d4 100644 --- a/backend/test/workspaces-diagnostics.test.ts +++ b/backend/test/workspaces-diagnostics.test.ts @@ -9,6 +9,7 @@ import { createWorkspaceDiagnoser, type DiagnosticAdapters, } from "../src/workspaces/diagnostics.js"; +import { resolveRuntimeBindings } from "../src/workspaces/bindings.js"; import type { RuntimeBindings } from "../src/workspaces/runtime-renderer.js"; import { parseWorkspaceYaml } from "../src/workspaces/schema.js"; @@ -56,6 +57,7 @@ diagnostics: method: POST path: /vector/diagnostic-probe auth: bearer + response: { operation: operation } `); const writerWorkspace = parseWorkspaceYaml(`workspace: @@ -88,6 +90,11 @@ semantic_index: llm_policy: allowed: [zai/glm-5.2] diagnostics: + dwh_rest: + method: POST + path: /rpc/ping + auth: bearer + response: { database: database, schema: schema } vector_rest: metadata: method: GET @@ -98,6 +105,7 @@ diagnostics: method: POST path: /vector/diagnostic-probe auth: bearer + response: { operation: operation } `); const bindings: RuntimeBindings = { @@ -123,6 +131,7 @@ const bindings: RuntimeBindings = { THT_WS_PSD_CLINICAL_VECTOR_TLS_CA_FILE: "/run/secrets/vector-ca", }, }, + vectorWriter: { transport: "rest_api", missing: [], values: {} }, embedding: { transport: "rest_api", missing: [], @@ -142,9 +151,13 @@ const writerBindings: RuntimeBindings = { values: { THT_WS_PSD_CLINICAL_VECTOR_BASE_URL: "https://vector.example.test", THT_WS_PSD_CLINICAL_VECTOR_API_KEY_FILE: "/run/secrets/vector-reader-key", - THT_WS_PSD_CLINICAL_VECTOR_WRITER_API_KEY_FILE: "/run/secrets/vector-writer-key", }, }, + vectorWriter: { + transport: "rest_api", + missing: [], + values: { THT_WS_PSD_CLINICAL_VECTOR_WRITER_API_KEY_FILE: "/run/secrets/vector-writer-key" }, + }, }; function successfulAdapters(overrides: Partial = {}): DiagnosticAdapters { @@ -448,6 +461,40 @@ test("requires a matching embedding model vector and removes its unique write pr })); }); +test("passes the resolver's distinct vector-writer binding to the diagnoser", async () => { + const directory = await mkdtemp(join(tmpdir(), "thothii-diagnostic-bindings-")); + const readerKey = join(directory, "reader-key"); + const writerKey = join(directory, "writer-key"); + const dwhKey = join(directory, "dwh-key"); + await Promise.all([ + writeFile(readerKey, "reader\n", { mode: 0o600 }), + writeFile(writerKey, "writer\n", { mode: 0o600 }), + writeFile(dwhKey, "dwh\n", { mode: 0o600 }), + ]); + const adapters = successfulAdapters(); + try { + const resolved = resolveRuntimeBindings(writerWorkspace, { + THT_WS_PSD_CLINICAL_DWH_TRANSPORT: "rest_api", + THT_WS_PSD_CLINICAL_DWH_BASE_URL: "https://dwh.example.test", + THT_WS_PSD_CLINICAL_DWH_API_KEY_FILE: dwhKey, + THT_WS_PSD_CLINICAL_VECTOR_TRANSPORT: "rest_api", + THT_WS_PSD_CLINICAL_VECTOR_BASE_URL: "https://vector.example.test", + THT_WS_PSD_CLINICAL_VECTOR_API_KEY_FILE: readerKey, + THT_WS_PSD_CLINICAL_VECTOR_WRITER_API_KEY_FILE: writerKey, + THT_WS_PSD_CLINICAL_EMBEDDING_BASE_URL: "https://embedding.example.test", + }, [directory]); + + await diagnose(adapters)(writerWorkspace, resolved, { writeProbe: true }); + + expect(resolved.vectorWriter.values).toEqual({ + THT_WS_PSD_CLINICAL_VECTOR_WRITER_API_KEY_FILE: writerKey, + }); + expect(adapters.writeDiagnosticRecord).toHaveBeenCalledWith(expect.objectContaining({ credentialFile: writerKey })); + } finally { + await rm(directory, { recursive: true, force: true }); + } +}); + test("keeps a reader-only workspace activatable without a vector write probe", async () => { const adapters = successfulAdapters(); @@ -476,11 +523,8 @@ test("rejects a writer credential that aliases the reader credential", async () const adapters = successfulAdapters(); const aliasedBindings: RuntimeBindings = { ...writerBindings, - vector: { ...writerBindings.vector, values: { - ...writerBindings.vector.values, - THT_WS_PSD_CLINICAL_VECTOR_API_KEY_FILE: readerKey, - THT_WS_PSD_CLINICAL_VECTOR_WRITER_API_KEY_FILE: writerAlias, - } }, + vector: { ...writerBindings.vector, values: { ...writerBindings.vector.values, THT_WS_PSD_CLINICAL_VECTOR_API_KEY_FILE: readerKey } }, + vectorWriter: { ...writerBindings.vectorWriter, values: { THT_WS_PSD_CLINICAL_VECTOR_WRITER_API_KEY_FILE: writerAlias } }, }; try { @@ -555,6 +599,29 @@ test("requires an authenticated TLS database query before direct diagnostics suc } }); +test("uses system trust for direct and SSH PostgreSQL diagnostics when no CA binding exists", async () => { + const directory = await mkdtemp(join(tmpdir(), "thothii-diagnostic-")); + const passwordFile = join(directory, "password"); + await writeFile(passwordFile, "password\n", { mode: 0o600 }); + const query = vi.fn(async () => ({ rows: [{ database: "warehouse", schema: "datawarehouse" }] })); + const connect = vi.fn(async () => ({ query, end: vi.fn(async () => undefined) })); + const adapter = createConcreteDiagnosticAdapters({ databaseClient: { connect } } as any); + try { + for (const transport of ["postgres_direct", "ssh_tunnel"] as const) { + await expect(adapter.probeConnector({ + role: "dwh", transport, host: "127.0.0.1", port: 5432, user: "reader", credentialFile: passwordFile, + resource: { database: "warehouse", schema: "datawarehouse" }, timeoutMs: 5000, + signal: new AbortController().signal, + })).resolves.toMatchObject({ tlsVerified: true, authenticated: true }); + } + expect(connect).toHaveBeenCalledTimes(2); + expect(connect).toHaveBeenNthCalledWith(1, expect.objectContaining({ tlsCaFile: undefined })); + expect(connect).toHaveBeenNthCalledWith(2, expect.objectContaining({ tlsCaFile: undefined })); + } 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"); @@ -597,20 +664,41 @@ test("applies declared auth modes and rejects private CA files across vector RES const keyFile = join(directory, "api-key"); const caFile = join(directory, "ca.pem"); await Promise.all([writeFile(keyFile, "writer-key\n", { mode: 0o600 }), writeFile(caFile, "private-ca\n")]); - const fetchSpy = vi.fn(async () => new Response(JSON.stringify({ collection: "clinical_documents", dimensions: 768, distance: "cosine", model: "embed" }), { status: 200, headers: { "content-type": "application/json" } })); + const fetchSpy = vi.fn(async () => new Response(JSON.stringify({ collection: "clinical_documents", dimensions: 768, distance: "cosine", model: "embed", operation: "create" }), { status: 200, headers: { "content-type": "application/json" } })); vi.stubGlobal("fetch", fetchSpy); const adapter = createConcreteDiagnosticAdapters(); const signal = new AbortController().signal; try { await adapter.inspectVector({ transport: "rest_api", baseUrl: "https://vector.example.test", collection: "clinical_documents", timeoutMs: 1, signal, diagnostic: { method: "GET", path: "/metadata", auth: "none", response: { collection: "collection", dimensions: "dimensions", distance: "distance" } } }); await adapter.probeEmbedding({ baseUrl: "https://embed.example.test", model: "embed", timeoutMs: 1, signal, credentialFile: keyFile, diagnostic: { method: "POST", path: "/embed", auth: "x-api-key", response: { model: "model", dimensions: "dimensions" } } }); - await adapter.writeDiagnosticRecord({ baseUrl: "https://vector.example.test", credentialFile: keyFile, collection: "clinical_documents", dimensions: 768, id: "diagnostic:test", timeoutMs: 1, signal, diagnostic: { method: "POST", path: "/probe", auth: "none" } }); expect(fetchSpy.mock.calls[0]?.[1]).toMatchObject({ headers: {} }); expect(fetchSpy.mock.calls[1]?.[1]).toMatchObject({ headers: { "x-api-key": "writer-key" } }); - expect(fetchSpy.mock.calls[2]?.[1]).toMatchObject({ headers: expect.not.objectContaining({ authorization: expect.anything() }) }); await expect(adapter.inspectVector({ transport: "rest_api", baseUrl: "https://vector.example.test", credentialFile: keyFile, tlsCaFile: caFile, collection: "clinical_documents", timeoutMs: 1, signal, diagnostic: { method: "GET", path: "/metadata", auth: "bearer", response: { collection: "collection", dimensions: "dimensions", distance: "distance" } } })).rejects.toThrow("vector metadata adapter is unavailable"); await expect(adapter.probeEmbedding({ baseUrl: "https://embed.example.test", credentialFile: keyFile, tlsCaFile: caFile, model: "embed", timeoutMs: 1, signal, diagnostic: { method: "POST", path: "/embed", auth: "bearer", response: { model: "model", dimensions: "dimensions" } } })).rejects.toThrow("embedding probe failed"); - await expect(adapter.removeDiagnosticRecord({ baseUrl: "https://vector.example.test", credentialFile: keyFile, tlsCaFile: caFile, collection: "clinical_documents", id: "diagnostic:test", dimensions: 768, timeoutMs: 1, signal, diagnostic: { method: "POST", path: "/probe", auth: "bearer" } })).rejects.toThrow("vector write adapter is unavailable"); + await expect(adapter.removeDiagnosticRecord({ baseUrl: "https://vector.example.test", credentialFile: keyFile, tlsCaFile: caFile, collection: "clinical_documents", id: "diagnostic:test", dimensions: 768, timeoutMs: 1, signal, diagnostic: { method: "POST", path: "/probe", auth: "bearer", response: { operation: "operation" } } })).rejects.toThrow("vector write adapter is unavailable"); + } finally { + vi.unstubAllGlobals(); + await rm(directory, { recursive: true, force: true }); + } +}); + +test("validates that the reversible writer response confirms each requested operation", async () => { + const directory = await mkdtemp(join(tmpdir(), "thothii-diagnostic-")); + const keyFile = join(directory, "writer-key"); + await writeFile(keyFile, "writer\n", { mode: 0o600 }); + const fetchSpy = vi.fn(async (_url: string, init: RequestInit) => new Response(JSON.stringify({ + operation: JSON.parse(String(init.body)).operation === "create" ? "create" : "not-removed", + }), { status: 200, headers: { "content-type": "application/json" } })); + vi.stubGlobal("fetch", fetchSpy); + const request = { + baseUrl: "https://vector.example.test", credentialFile: keyFile, collection: "clinical_documents", + dimensions: 768, id: "diagnostic:test", timeoutMs: 5000, signal: new AbortController().signal, + diagnostic: { method: "POST" as const, path: "/probe", auth: "bearer" as const, response: { operation: "operation" } }, + }; + try { + const adapter = createConcreteDiagnosticAdapters(); + await expect(adapter.writeDiagnosticRecord(request)).resolves.toBeUndefined(); + await expect(adapter.removeDiagnosticRecord(request)).rejects.toThrow("vector write adapter is unavailable"); } finally { vi.unstubAllGlobals(); await rm(directory, { recursive: true, force: true }); diff --git a/backend/test/workspaces-schema.test.ts b/backend/test/workspaces-schema.test.ts index 145796d5..5aa28ac8 100644 --- a/backend/test/workspaces-schema.test.ts +++ b/backend/test/workspaces-schema.test.ts @@ -98,6 +98,7 @@ test("requires explicit vector database and schema identities with strict diagno + " method: POST\n" + " path: /rpc/diagnostic_vector_probe\n" + " auth: bearer\n" + + " response: { operation: operation }\n" + " embedding:\n" + " method: GET\n" + " path: /models\n" @@ -134,6 +135,24 @@ test("requires explicit vector database and schema identities with strict diagno .toThrow(/response field/i); }); +test("requires a reversible writer probe to declare the response operation it verifies", () => { + const writerProbe = validYaml.replace("llm_policy:\n", `diagnostics: + vector_rest: + metadata: + method: GET + path: /metadata + auth: bearer + response: { collection: collection, dimensions: dimensions, distance: distance } + reversible_probe: + method: POST + path: /diagnostic-probe + auth: bearer +llm_policy: +`); + + expect(() => parseWorkspaceYaml(writerProbe)).toThrow(/response|operation/i); +}); + test("keeps v1 descriptors readable but requires explicit migration before v2 operations", () => { const v1WithoutVectorIdentity = validYaml.replace("schema_version: 2", "schema_version: 1").replace( " database: postgres\n schema: vectors\n", "", 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 4cdfe65a..69a2083f 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 @@ -163,6 +163,7 @@ diagnostics: method: POST path: /vector/diagnostic-probe auth: bearer + response: { operation: operation } embedding: method: GET path: /models @@ -219,7 +220,8 @@ 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. -`vector_rest.reversible_probe`, when present, is POST-only. It is called with a generated +`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`. An upsert-only service cannot be declared as this probe. All ordinary diagnostics remain read-only. The complete request, response, timeout, reader-only fallback, SSH, and private-CA limitations are @@ -310,7 +312,10 @@ Transport behavior is encapsulated behind connector adapters. ### 8.1 Direct -Direct adapters connect to the configured host and port with the native protocol. PostgreSQL direct access supports TLS modes and CA files. Vector direct access uses the native vector-store protocol or database driver. +Direct adapters connect to the configured host and port with the native protocol. PostgreSQL +direct access uses a supplied CA file when present and otherwise requires runtime system trust; +certificate verification is never disabled. Vector direct access uses the native vector-store +protocol or database driver. ### 8.2 REST API @@ -323,7 +328,9 @@ trusted TLS-termination boundary. ### 8.3 SSH tunnel -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. Host-key checking cannot be disabled by the form. +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. Transport selection is installation-specific because a production server may connect directly while a laptop reaches the same logical resource through REST or SSH. @@ -592,6 +599,8 @@ Legacy sessions without workspace revision use the existing compatibility resolu - Git SSH uses explicit known-hosts verification. - REST and direct TLS validation cannot be disabled silently. - Diagnostics sanitize provider errors before returning them to the browser. +- `auth: none` diagnostics neither require nor read an API-key file; authenticated REST + diagnostics still require the declared local secret file. - Production CORS remains same-origin; absence of embedded authentication does not imply cross-origin write access. - The first release allows every user who can access the ThothII application to publish workspace changes. This limitation is documented until an authorization layer is introduced. diff --git a/docs/workspace-diagnostic-protocol.md b/docs/workspace-diagnostic-protocol.md index 59da0500..3ab1d13e 100644 --- a/docs/workspace-diagnostic-protocol.md +++ b/docs/workspace-diagnostic-protocol.md @@ -19,7 +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 file content is never logged or returned. + The resolver does not require or read an API-key file for an `auth: none` diagnostic. File + content is never logged or returned. ## Canonical descriptor additions @@ -55,6 +56,7 @@ diagnostics: method: POST path: /vector/diagnostic-probe auth: bearer + response: { operation: operation } embedding: method: GET path: /models @@ -63,8 +65,9 @@ diagnostics: ``` `diagnostics.dwh_rest` requires DWH `rest_api`; `diagnostics.vector_rest` requires vector -`rest_api`. `reversible_probe` is optional, but when present it must be `POST`. Response-map -values are JSON object field names, not values to be put in Git. +`rest_api`. `reversible_probe` is optional, but when present it must be authenticated `POST` and +declare the response field that echoes the requested `operation`. Response-map values are JSON +object field names, not values to be put in Git. ## Installation-local variable contract @@ -77,11 +80,11 @@ approved local secret root; it is never the secret itself. | --- | --- | | DWH selection | `THT_WS__DWH_TRANSPORT` | | DWH `postgres_direct` | `THT_WS__DWH_HOST`, `THT_WS__DWH_PORT`, `THT_WS__DWH_USER`, `THT_WS__DWH_PASSWORD_FILE`; optional `THT_WS__DWH_TLS_CA_FILE` | -| DWH `rest_api` | `THT_WS__DWH_BASE_URL`, `THT_WS__DWH_API_KEY_FILE`; optional `THT_WS__DWH_TLS_CA_FILE` | +| DWH `rest_api` | `THT_WS__DWH_BASE_URL`; `THT_WS__DWH_API_KEY_FILE` only for `bearer`/`x-api-key`; optional `THT_WS__DWH_TLS_CA_FILE` | | DWH `ssh_tunnel` | `THT_WS__DWH_USER`, `THT_WS__DWH_PASSWORD_FILE`, `THT_WS__DWH_SSH_HOST`, `THT_WS__DWH_SSH_PORT`, `THT_WS__DWH_SSH_USER`, `THT_WS__DWH_SSH_PRIVATE_KEY_FILE`, `THT_WS__DWH_SSH_KNOWN_HOSTS_FILE`, `THT_WS__DWH_SSH_TARGET_HOST`, `THT_WS__DWH_SSH_TARGET_PORT`; optional `THT_WS__DWH_TLS_CA_FILE` | | Vector selection | `THT_WS__VECTOR_TRANSPORT` | | Vector `pgvector_direct` | `THT_WS__VECTOR_HOST`, `THT_WS__VECTOR_PORT`, `THT_WS__VECTOR_USER`, `THT_WS__VECTOR_PASSWORD_FILE`; optional `THT_WS__VECTOR_TLS_CA_FILE` | -| Vector `rest_api` | `THT_WS__VECTOR_BASE_URL`, `THT_WS__VECTOR_API_KEY_FILE`; optional `THT_WS__VECTOR_TLS_CA_FILE` | +| Vector `rest_api` | `THT_WS__VECTOR_BASE_URL`; `THT_WS__VECTOR_API_KEY_FILE` only for `bearer`/`x-api-key`; optional `THT_WS__VECTOR_TLS_CA_FILE` | | Vector `ssh_tunnel` | `THT_WS__VECTOR_USER`, `THT_WS__VECTOR_PASSWORD_FILE`, `THT_WS__VECTOR_SSH_HOST`, `THT_WS__VECTOR_SSH_PORT`, `THT_WS__VECTOR_SSH_USER`, `THT_WS__VECTOR_SSH_PRIVATE_KEY_FILE`, `THT_WS__VECTOR_SSH_KNOWN_HOSTS_FILE`, `THT_WS__VECTOR_SSH_TARGET_HOST`, `THT_WS__VECTOR_SSH_TARGET_PORT`; optional `THT_WS__VECTOR_TLS_CA_FILE` | | Optional vector writer | `THT_WS__VECTOR_WRITER_API_KEY_FILE` | | Embedding service | `THT_WS__EMBEDDING_BASE_URL`; optional `THT_WS__EMBEDDING_API_KEY_FILE`, `THT_WS__EMBEDDING_TLS_CA_FILE` | @@ -110,6 +113,10 @@ SELECT current_database() AS database, current_schema() AS schema Both returned values must equal the descriptor's DWH database and schema. +`*_TLS_CA_FILE` is optional for direct and SSH PostgreSQL diagnostics. When provided, it is used +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. + For REST, the descriptor above declares the exact ping: ```text @@ -134,6 +141,9 @@ identity. They inspect the declared collection's vector column and index and mus collection, integer `dimensions`, and `distance` (`cosine`, `l2`, or `inner_product`) declared in `semantic_index.vector_store`. +Their optional `*_TLS_CA_FILE` follows the same verified private-CA-or-system-trust rule as the +DWH diagnostic. + For REST, the exact descriptor-declared request is, for example: ```text @@ -152,7 +162,8 @@ Ordinary validation is reader-only. A write probe runs only when all of the foll 1. The operator explicitly requests it. 2. The descriptor has `semantic_index.vector_writer: {}`. -3. The descriptor declares `diagnostics.vector_rest.reversible_probe`. +3. The descriptor declares an authenticated `diagnostics.vector_rest.reversible_probe` with an + `operation` response field. 4. The selected vector transport is `rest_api`. 5. `THT_WS__VECTOR_WRITER_API_KEY_FILE` exists locally and is distinct from the reader API-key file. @@ -169,8 +180,9 @@ then: { "operation": "remove", "id": "diagnostic:", "collection": "" } ``` -Both requests require a 2xx response. Cleanup is attempted in `finally`, including after a write -timeout or error. The endpoint must implement both operations as a bounded, reversible diagnostic +Both requests require a 2xx JSON response whose declared `operation` field equals the requested +`create` or `remove` operation. Cleanup is attempted in `finally`, including after a write timeout +or error. The endpoint must implement both operations as a bounded, reversible diagnostic operation; an upsert-only endpoint is prohibited. It must not retain, index, or expose diagnostic records. If the writer capability or its local binding is absent, validation remains reader-only and no write request is sent.