fix: close catalog review gaps
Publish documentation / publish (push) Successful in 38s

This commit is contained in:
Codex
2026-08-31 16:28:42 +02:00
parent 64fbe642ef
commit f586152636
7 changed files with 301 additions and 48 deletions
+14 -9
View File
@@ -3,6 +3,18 @@ export interface ConfiguredTransportUrlOptions {
originOnly?: boolean;
}
export function parseCredentialFreeHttpUrl(value: string): URL | undefined {
let url: URL;
try {
url = new URL(value);
} catch {
return undefined;
}
if (!["http:", "https:"].includes(url.protocol)
|| url.username || url.password || url.search || url.hash) return undefined;
return url;
}
function canonicalLoopbackAuthority(value: string): boolean {
const match = /^http:\/\/([^/?#]+)(?:[/?#]|$)/.exec(value);
if (!match) return false;
@@ -27,15 +39,8 @@ export function parseConfiguredTransportUrl(
value: string,
options: ConfiguredTransportUrlOptions,
): URL | undefined {
let url: URL;
try {
url = new URL(value);
} catch {
return undefined;
}
if (url.username || url.password || url.search || url.hash || (options.originOnly && url.pathname !== "/")) {
return undefined;
}
const url = parseCredentialFreeHttpUrl(value);
if (!url || (options.originOnly && url.pathname !== "/")) return undefined;
if (url.protocol === "https:") return url;
if (options.allowLoopbackHttp && url.protocol === "http:" && canonicalLoopbackAuthority(value)) return url;
return undefined;
+5 -3
View File
@@ -159,7 +159,7 @@ export class CatalogService {
for (const id of Object.values(CATALOG_SECRET_IDS)) this.secretStore.forget(workspaceId, id);
}
async test(database: WorkspaceDatabase): Promise<DatabaseTestResult> {
async test(database: WorkspaceDatabase): Promise<WorkspaceDatabase | undefined> {
return await this.operations.run(database.id, async () => {
const testedAt = new Date().toISOString();
const controller = new AbortController();
@@ -168,6 +168,7 @@ export class CatalogService {
? database.binding.restAuth === "none" ? [] : [CATALOG_SECRET_IDS.apiKey]
: [];
const materialized = this.secretStore.materialize(database.workspaceId, required);
let result: DatabaseTestResult;
try {
if (database.binding.transport !== "rest_api") {
const client = await this.postgres.connect(database, controller.signal);
@@ -205,13 +206,13 @@ export class CatalogService {
},
});
}
return {
result = {
connectionStatus: "reachable",
testedVersion: database.version,
lastTestedAt: testedAt,
};
} catch {
return {
result = {
connectionStatus: "failed",
testedVersion: database.version,
lastTestedAt: testedAt,
@@ -223,6 +224,7 @@ export class CatalogService {
controller.abort();
materialized.release();
}
return await this.repository.recordTest(database.id, database.version, result);
});
}
}
+5 -2
View File
@@ -1,6 +1,7 @@
import type { FastifyInstance, FastifyReply, FastifyRequest } from "fastify";
import { z } from "zod";
import { isPrincipalContext, requirePermission } from "../auth/authorization.js";
import { parseCredentialFreeHttpUrl } from "../auth/url-policy.js";
import { CatalogService, type CatalogSecretName } from "../catalog/service.js";
import { WorkspaceRegistryError } from "../workspaces/git-repository.js";
import {
@@ -26,7 +27,9 @@ const bindingSchema = z.object({
host: optionalText,
port: port.optional(),
username: optionalText,
baseUrl: z.url().max(2048).optional(),
baseUrl: z.string().max(2048)
.refine((value) => parseCredentialFreeHttpUrl(value) !== undefined)
.optional(),
restPath: z.string().regex(/^\/(?!\/)[^?#\\\u0000-\u001f]*$/).max(512).optional(),
restAuth: z.enum(["none", "bearer", "x-api-key"]).optional(),
tlsServername: optionalText,
@@ -209,7 +212,7 @@ export function catalogDatabaseRoutes(
const database = await deps.repository.get(id);
if (!database) return reply.code(404).send({ code: "database_not_found", message: "Database configuration was not found." });
if (database.version !== version) return reply.code(409).send({ code: "database_stale", message: "Database configuration changed. Reload and try again." });
const tested = await deps.repository.recordTest(id, version, await deps.service.test(database));
const tested = await deps.service.test(database);
if (!tested) return reply.code(409).send({ code: "database_stale", message: "Database configuration changed. Reload and try again." });
return { ...tested, configured: true, secrets: deps.service.configuredSecrets(tested.workspaceId) };
} catch (error) { return safeError(reply, error); }
+71 -1
View File
@@ -5,6 +5,8 @@ import { afterEach, expect, test, vi } from "vitest";
import { buildApp } from "../src/app.js";
import { loadConfig } from "../src/config.js";
import { MemoryCatalogRepository } from "../src/catalog/memory-repository.js";
import { CatalogOperationCoordinator } from "../src/catalog/operation-coordinator.js";
import type { CatalogPostgresAccess } from "../src/catalog/postgres-access.js";
import type { ObservedSchemaSnapshot } from "../src/catalog/types.js";
import { WorkspaceSecretStore } from "../src/workspaces/secret-store.js";
import type { WorkspaceRegistry, WorkspaceRevision } from "../src/workspaces/registry.js";
@@ -28,7 +30,13 @@ const workspace: WorkspaceDescriptor = {
};
const revision: WorkspaceRevision = { id: "psd-clinical", commit: "a".repeat(40), blob: "b".repeat(40), snapshotPath: "/tmp/psd.yaml" };
function setup(environment: Record<string, string> = {}) {
function setup(
environment: Record<string, string> = {},
catalogDependencies: {
catalogOperationCoordinator?: CatalogOperationCoordinator;
catalogPostgresAccess?: CatalogPostgresAccess;
} = {},
) {
const secretRoot = mkdtempSync(join(tmpdir(), "catalog-secret-"));
const runtimeRoot = mkdtempSync(join(tmpdir(), "catalog-secret-runtime-"));
roots.push(secretRoot, runtimeRoot);
@@ -49,6 +57,7 @@ function setup(environment: Record<string, string> = {}) {
workspaceSecretStore: secretStore,
catalogRepository: repository,
workspaceDiagnoser: vi.fn(),
...catalogDependencies,
});
return { app, secretStore, repository };
}
@@ -130,6 +139,29 @@ test("lists orphaned records and takes the REST diagnostic path from workspace Y
]));
});
test.each([
"https://reader:secret@psd.example/api",
"https://psd.example/api?token=secret",
"https://psd.example/api#secret",
"ftp://psd.example/api",
])("rejects unsafe REST base URL %s before persistence", async (baseUrl) => {
const { app, repository } = setup();
const response = await app.inject({
method: "POST",
url: "/catalog/databases",
payload: {
...direct,
binding: { transport: "rest_api", baseUrl, restPath: "/health", restAuth: "bearer" },
},
});
expect(response.statusCode).toBe(400);
expect(response.json()).toEqual({
code: "database_invalid",
message: "Database configuration is invalid.",
});
expect(await repository.getByWorkspace("psd-clinical")).toBeUndefined();
});
test("uses optimistic versions, keeps secrets write-only, and hard-deletes only local configuration", async () => {
const { app, secretStore } = setup();
const created = (await app.inject({ method: "POST", url: "/catalog/databases", payload: direct })).json();
@@ -151,6 +183,44 @@ test("uses optimistic versions, keeps secrets write-only, and hard-deletes only
expect((await app.inject({ method: "GET", url: "/catalog/databases" })).json()).toMatchObject([{ configured: false }]);
});
test("rejects a connection test while another catalog operation owns the database", async () => {
const coordinator = new CatalogOperationCoordinator();
const postgres: CatalogPostgresAccess = {
connect: vi.fn(async () => { throw new Error("connection must not start"); }),
};
const { app } = setup({}, {
catalogOperationCoordinator: coordinator,
catalogPostgresAccess: postgres,
});
const created = (await app.inject({
method: "POST",
url: "/catalog/databases",
payload: direct,
})).json();
const release = coordinator.reserve(created.id);
try {
const response = await app.inject({
method: "POST",
url: `/catalog/databases/${created.id}/test`,
payload: { version: created.version },
});
expect(response.statusCode).toBe(409);
expect(response.json()).toEqual({
code: "database_operation_in_progress",
message: "A database operation is already in progress.",
});
expect(postgres.connect).not.toHaveBeenCalled();
expect((await app.inject({
method: "GET",
url: `/catalog/databases/${created.id}`,
})).json()).toMatchObject({ connectionStatus: "untested" });
} finally {
release();
}
});
test("returns exact global and per-database fleet metrics", async () => {
const { app, repository } = setup();
const database = await repository.create(direct);
+111
View File
@@ -0,0 +1,111 @@
import { mkdtempSync, rmSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { expect, test } from "vitest";
import { MemoryCatalogRepository } from "../src/catalog/memory-repository.js";
import { CatalogOperationCoordinator } from "../src/catalog/operation-coordinator.js";
import type { CatalogPostgresAccess } from "../src/catalog/postgres-access.js";
import { CatalogService } from "../src/catalog/service.js";
import type { DatabaseTestResult, WorkspaceDatabase } from "../src/catalog/types.js";
import type { WorkspaceRegistry } from "../src/workspaces/registry.js";
import { WorkspaceSecretStore } from "../src/workspaces/secret-store.js";
interface Deferred {
promise: Promise<void>;
resolve: () => void;
}
function deferred(): Deferred {
let resolve!: () => void;
const promise = new Promise<void>((done) => { resolve = done; });
return { promise, resolve };
}
class PausingRecordRepository extends MemoryCatalogRepository {
constructor(
private readonly recordStarted: Deferred,
private readonly continueRecord: Deferred,
) {
super();
}
override async recordTest(
id: string,
expectedVersion: number,
result: DatabaseTestResult,
): Promise<WorkspaceDatabase | undefined> {
this.recordStarted.resolve();
await this.continueRecord.promise;
return await super.recordTest(id, expectedVersion, result);
}
}
test("holds the database reservation until the connection result is recorded", async () => {
const secretRoot = mkdtempSync(join(tmpdir(), "catalog-service-secret-"));
const runtimeRoot = mkdtempSync(join(tmpdir(), "catalog-service-runtime-"));
const recordStarted = deferred();
const continueRecord = deferred();
const repository = new PausingRecordRepository(recordStarted, continueRecord);
const coordinator = new CatalogOperationCoordinator();
const secretStore = new WorkspaceSecretStore({
root: secretRoot,
runtimeRoot,
installationId: "test",
});
const postgres: CatalogPostgresAccess = {
connect: async () => ({
query: async () => ({ rows: [{ database: "warehouse", schema: "datawarehouse" }] }),
end: async () => undefined,
}),
};
try {
const database = await repository.create({
workspaceId: "psd-clinical",
engine: "postgres",
databaseName: "warehouse",
schema: "datawarehouse",
binding: {
transport: "postgres_direct",
host: "db.internal",
port: 5432,
username: "reader",
},
});
const service = new CatalogService(
repository,
{} as WorkspaceRegistry,
secretStore,
[],
1_000,
postgres,
coordinator,
);
const connectionTest = service.test(database);
const firstCompletedPhase = await Promise.race([
recordStarted.promise.then(() => "recording" as const),
connectionTest.then(() => "returned" as const),
]);
expect(firstCompletedPhase).toBe("recording");
try {
await expect(coordinator.run(database.id, async () => "overlapped"))
.rejects.toThrow("A database operation is already in progress");
} finally {
continueRecord.resolve();
}
expect(await connectionTest).toMatchObject({
id: database.id,
version: 1,
connectionStatus: "reachable",
testedVersion: 1,
});
expect(await coordinator.run(database.id, async () => "released")).toBe("released");
} finally {
continueRecord.resolve();
rmSync(secretRoot, { recursive: true, force: true });
rmSync(runtimeRoot, { recursive: true, force: true });
}
});
@@ -496,7 +496,7 @@ test.each(synchronizationScopes)(
},
);
test("starts Generate Missing for one configured database without confirmation", async () => {
test("discloses source sampling before starting Generate Missing", async () => {
const user = userEvent.setup();
const queuedRun = makeDescriptionGenerationRun({ scope: "missing", total: 3 });
let startBody: unknown;
@@ -519,6 +519,15 @@ test("starts Generate Missing for one configured database without confirmation",
await user.click(screen.getByRole("button", { name: "Actions" }));
await user.click(await screen.findByRole("menuitem", { name: "Generate Missing" }));
expect(screen.getByText("Generate missing descriptions for Policlinico San Donato?")).toBeVisible();
expect(screen.getByRole("note", {
name: "Metadata generation source data disclosure",
})).toHaveTextContent(
"When available, up to five real source rows and five representative values from columns not marked sensitive are sent to the selected provider.",
);
expect(startBody).toBeUndefined();
await user.click(screen.getByRole("button", { name: "Generate Missing" }));
await waitFor(() => expect(startBody).toEqual({
modelId: "local-qwen",
scope: "missing",
@@ -527,6 +536,44 @@ test("starts Generate Missing for one configured database without confirmation",
expect(await screen.findByRole("dialog", { name: "Description generation" })).toBeVisible();
});
test("keeps Fleet Generate Missing behind the source-data disclosure", async () => {
const user = userEvent.setup();
let generationStarts = 0;
server.use(
http.get("/api/catalog/metadata-generation/models", () => HttpResponse.json({
models: [{ id: "local-qwen", label: "Local Qwen" }],
default: "local-qwen",
})),
http.post("/api/catalog/databases/:databaseId/description-generation-runs", () => {
generationStarts += 1;
return HttpResponse.json(makeDescriptionGenerationRun({ scope: "missing" }), { status: 202 });
}),
);
renderPage({ rows: [makeDatabase()], presentation: "fleet" });
const databaseRow = await screen.findByRole("row", { name: /Policlinico San Donato/ });
await user.click(within(databaseRow).getByRole("checkbox", { name: /toggle row selection/i }));
await user.selectOptions(
screen.getByRole("combobox", { name: "Batch action" }),
"generate-missing",
);
await waitFor(() => expect(screen.getByRole("combobox", {
name: "Metadata description model",
})).toHaveValue("local-qwen"));
await user.click(screen.getByRole("button", { name: "Run action" }));
expect(screen.getByRole("note", {
name: "Metadata generation source data disclosure",
})).toBeVisible();
expect(generationStarts).toBe(0);
await user.click(screen.getByRole("button", { name: "Cancel" }));
expect(screen.queryByRole("note", {
name: "Metadata generation source data disclosure",
})).not.toBeInTheDocument();
expect(generationStarts).toBe(0);
});
test("confirms Generate All replacement, supports cancel, and sends the database-wide scope", async () => {
const user = userEvent.setup();
const queuedRun = makeDescriptionGenerationRun({ scope: "all", total: 6 });
@@ -552,6 +599,9 @@ test("confirms Generate All replacement, supports cancel, and sends the database
expect(screen.getByText("Replace generated descriptions for Policlinico San Donato?")).toBeVisible();
expect(screen.getByText(/existing generated descriptions for eligible tables and columns will be replaced/i)).toBeVisible();
expect(screen.getByRole("note", {
name: "Metadata generation source data disclosure",
})).toHaveTextContent(/real source rows.*selected provider/i);
expect(startBody).toBeUndefined();
await user.click(screen.getByRole("button", { name: "Cancel" }));
@@ -587,6 +637,7 @@ test("keeps the database selected and safely explains when no descriptions are e
await user.click(within(databaseRow).getByRole("checkbox", { name: /toggle row selection/i }));
await user.click(screen.getByRole("button", { name: "Actions" }));
await user.click(await screen.findByRole("menuitem", { name: "Generate Missing" }));
await user.click(screen.getByRole("button", { name: "Generate Missing" }));
expect(await screen.findByText(
"No eligible catalog tables or columns need description generation.",
@@ -648,7 +699,9 @@ test.each([
await user.click(await screen.findByRole("menuitem", {
name: scope === "all" ? "Generate All" : "Generate Missing",
}));
if (scope === "all") await user.click(screen.getByRole("button", { name: "Generate All" }));
await user.click(screen.getByRole("button", {
name: scope === "all" ? "Generate All" : "Generate Missing",
}));
expect(await screen.findByRole("heading", {
name: status === "completed" ? "Completed" : "Completed with errors",
@@ -752,6 +805,7 @@ test("disables database-wide generation while a description generation is active
await user.click(within(databaseRow).getByRole("checkbox", { name: /toggle row selection/i }));
await user.click(screen.getByRole("button", { name: "Actions" }));
await user.click(await screen.findByRole("menuitem", { name: "Generate Missing" }));
await user.click(screen.getByRole("button", { name: "Generate Missing" }));
expect(await screen.findByRole("dialog", { name: "Description generation" })).toBeVisible();
databaseRow = await screen.findByRole("row", { name: /Policlinico San Donato/ });
@@ -214,7 +214,9 @@ export function DatabaseGrid({
const [selectedRows, setSelectedRows] = useState<CatalogDatabase[]>([]);
const [action, setAction] = useState<"test" | "sync" | "generate" | "suggest" | "delete" | null>(null);
const [pendingDelete, setPendingDelete] = useState<CatalogDatabaseMetadataDeleteTarget | null>(null);
const [pendingGenerateAll, setPendingGenerateAll] = useState(false);
const [pendingGenerationScope, setPendingGenerationScope] = useState<
Extract<DescriptionGenerationScope, "all" | "missing"> | null
>(null);
const context = useMemo<DatabaseGridContext>(
() => ({ canManage, presentation, onView, onOpenTables, onEdit, onDelete, onOpenSync }),
[canManage, presentation, onView, onOpenTables, onEdit, onDelete, onOpenSync],
@@ -376,10 +378,10 @@ export function DatabaseGrid({
return () => window.clearTimeout(timer);
}, [pendingDelete]);
useEffect(() => {
if (!pendingGenerateAll) return;
const timer = window.setTimeout(() => document.getElementById("database-generate-all-confirm-button")?.focus(), 0);
if (!pendingGenerationScope) return;
const timer = window.setTimeout(() => document.getElementById("database-generation-confirm-button")?.focus(), 0);
return () => window.clearTimeout(timer);
}, [pendingGenerateAll]);
}, [pendingGenerationScope]);
const closeDeleteConfirmation = () => {
setPendingDelete(null);
window.setTimeout(() => {
@@ -387,8 +389,8 @@ export function DatabaseGrid({
else actionsTriggerRef.current?.focus();
}, 0);
};
const closeGenerateAllConfirmation = () => {
setPendingGenerateAll(false);
const closeGenerationConfirmation = () => {
setPendingGenerationScope(null);
window.setTimeout(() => {
if (presentation === "fleet") actionSelectRef.current?.focus();
else actionsTriggerRef.current?.focus();
@@ -407,16 +409,17 @@ export function DatabaseGrid({
setSelectedRows([]);
}
setPendingDelete(null);
setPendingGenerateAll(false);
setPendingGenerationScope(null);
if (kind === "delete") window.setTimeout(() => searchInputRef.current?.focus(), 0);
} catch {
// The page-level operation owns safe error feedback. Preserve the selection for retry.
if (kind === "generate") setPendingGenerationScope(null);
} finally { setAction(null); }
};
const runFleetAction = async (selectedAction: DatabaseFleetAction) => {
if (selectedAction === "generate-all") {
setPendingGenerateAll(true);
if (selectedAction === "generate-all" || selectedAction === "generate-missing") {
setPendingGenerationScope(selectedAction === "generate-all" ? "all" : "missing");
return;
}
if (selectedAction === "clear-tables" || selectedAction === "clear-relationships") {
@@ -432,10 +435,6 @@ export function DatabaseGrid({
await perform("sync", () => onSyncSelected(selectedRows, scope));
return;
}
if (selectedAction === "generate-missing") {
await perform("generate", () => onGenerateDescriptions(selectedRows, "missing"));
return;
}
await perform("suggest", () => onSuggestSensitive(selectedRows), false);
};
@@ -452,35 +451,47 @@ export function DatabaseGrid({
: "flex min-h-12 flex-wrap items-center gap-3 border-b border-border px-3 py-2"}
>
{selectedRows.length > 0 ? (
pendingGenerateAll ? (
pendingGenerationScope ? (
<div
role="group"
aria-labelledby="database-generate-all-confirmation"
aria-labelledby="database-generation-confirmation"
className="flex min-w-0 flex-1 flex-wrap items-center gap-2"
onKeyDown={(event) => {
if (event.key === "Escape" && action === null) closeGenerateAllConfirmation();
if (event.key === "Escape" && action === null) closeGenerationConfirmation();
}}
>
<div className="mr-auto min-w-56">
<p id="database-generate-all-confirmation" className="text-sm font-semibold">
Replace generated descriptions for {selectedRows[0]?.workspaceName}?
<p id="database-generation-confirmation" className="text-sm font-semibold">
{pendingGenerationScope === "all"
? `Replace generated descriptions for ${selectedRows[0]?.workspaceName}?`
: `Generate missing descriptions for ${selectedRows[0]?.workspaceName}?`}
</p>
<p className="text-xs text-muted-foreground">
Existing generated descriptions for eligible tables and columns will be replaced.
{pendingGenerationScope === "all" ? (
<p className="text-xs text-muted-foreground">
Existing generated descriptions for eligible tables and columns will be replaced.
</p>
) : null}
<p
role="note"
aria-label="Metadata generation source data disclosure"
className="mt-1 text-xs font-medium text-amber-800 dark:text-amber-300"
>
When available, up to five real source rows and five representative values from
columns not marked sensitive are sent to the selected provider.
</p>
</div>
<Button type="button" variant="ghost" disabled={action !== null} onClick={closeGenerateAllConfirmation}>Cancel</Button>
<Button type="button" variant="ghost" disabled={action !== null} onClick={closeGenerationConfirmation}>Cancel</Button>
<Button
id="database-generate-all-confirm-button"
id="database-generation-confirm-button"
type="button"
variant="destructive"
variant={pendingGenerationScope === "all" ? "destructive" : "default"}
disabled={action !== null}
onClick={() => void perform(
"generate",
() => onGenerateDescriptions(selectedRows, "all"),
() => onGenerateDescriptions(selectedRows, pendingGenerationScope),
)}
>
Generate All
{pendingGenerationScope === "all" ? "Generate All" : "Generate Missing"}
</Button>
</div>
) : pendingDelete ? (
@@ -564,17 +575,14 @@ export function DatabaseGrid({
<Menu.Item
className="rounded-md px-3 py-2 text-sm outline-none data-[highlighted]:bg-muted data-[disabled]:opacity-45"
disabled={!canGenerateDescriptions}
onClick={() => setPendingGenerateAll(true)}
onClick={() => setPendingGenerationScope("all")}
>
Generate All
</Menu.Item>
<Menu.Item
className="rounded-md px-3 py-2 text-sm outline-none data-[highlighted]:bg-muted data-[disabled]:opacity-45"
disabled={!canGenerateDescriptions}
onClick={() => void perform(
"generate",
() => onGenerateDescriptions(selectedRows, "missing"),
)}
onClick={() => setPendingGenerationScope("missing")}
>
Generate Missing
</Menu.Item>
@@ -649,7 +657,7 @@ export function DatabaseGrid({
selectionColumnDef={{ width: 44, maxWidth: 44, pinned: "left" }}
onSelectionChanged={({ api }) => {
if (pendingDelete && action === null) setPendingDelete(null);
if (pendingGenerateAll && action === null) setPendingGenerateAll(false);
if (pendingGenerationScope && action === null) setPendingGenerationScope(null);
setSelectedRows(api.getSelectedRows());
}}
rowHeight={44}