diff --git a/apps/dev-playground/client/src/routes/database.route.tsx b/apps/dev-playground/client/src/routes/database.route.tsx index 9b041cef6..daa3e1251 100644 --- a/apps/dev-playground/client/src/routes/database.route.tsx +++ b/apps/dev-playground/client/src/routes/database.route.tsx @@ -293,7 +293,7 @@ function DatabaseRoute() {
diff --git a/docs/docs/plugins/database.md b/docs/docs/plugins/database.md index dd0b9db74..6468b9e25 100644 --- a/docs/docs/plugins/database.md +++ b/docs/docs/plugins/database.md @@ -31,8 +31,12 @@ server routes. App admission alone does not provide row-level isolation. Configure a Lakebase `postgres` resource and its connection environment variables as described in [Lakebase configuration](./lakebase.md#environment-variables). -The database tables must already exist and match the declared schema. This plugin -checks connectivity during setup; it does not create or migrate tables. + +When a query fails at runtime, the client receives only a stable message such as +`Database operation failed`. The server log records the SQLSTATE and only +identifier-only messages for known missing-column or missing-table errors +(for example `column notes.board_id does not exist`). Other driver messages, +details, and hints are omitted because they may contain row values. Apps scaffolded with the Database plugin selected include an empty `config/database/schema.ts`, so `database()` can start without requiring sample diff --git a/docs/docs/plugins/lakebase.md b/docs/docs/plugins/lakebase.md index cf1fa9cb2..c887073da 100644 --- a/docs/docs/plugins/lakebase.md +++ b/docs/docs/plugins/lakebase.md @@ -93,6 +93,8 @@ env: For local development, the `.env` file is automatically generated by `databricks apps init` with the correct values for your Lakebase project. +For OAuth pools, `PGHOST` must be a host of `LAKEBASE_ENDPOINT`. Credentials are issued for the endpoint, but the pool connects to `PGHOST`, so a host left over from another branch can serve that branch's tables. At startup, `lakebase()` and `database()` look the endpoint up; a confirmed mismatch fails setup and names both hosts. Native password pools skip this check. If the lookup fails or takes more than three seconds, AppKit warns that it could not verify the host but does not block startup. A missing endpoint produces a warning. When you switch branches, update both variables; `databricks postgres list-endpoints projects/{project}/branches/{branch}` shows the endpoint name and its host. Direct `createLakebasePool()` calls do not perform this asynchronous check. + For the full configuration reference (SSL, pool size, timeouts, logging, ORM examples), see the [`@databricks/lakebase` README](https://github.com/databricks/appkit/blob/main/packages/lakebase/README.md). ### Pool configuration diff --git a/packages/appkit/src/connectors/lakebase/endpoint-host.ts b/packages/appkit/src/connectors/lakebase/endpoint-host.ts new file mode 100644 index 000000000..a3020901d --- /dev/null +++ b/packages/appkit/src/connectors/lakebase/endpoint-host.ts @@ -0,0 +1,149 @@ +import type { LakebasePoolConfig } from "@databricks/lakebase"; + +import { ConfigurationError } from "../../errors"; +import { createLogger } from "../../logging/logger"; +import { contextFromAbortSignal } from "../context"; + +const logger = createLogger("connectors:lakebase"); + +/** An unavailable lookup must not delay startup indefinitely. */ +const HOST_CHECK_TIMEOUT_MS = 3_000; +const ENDPOINT_NAME = + /^projects\/[A-Za-z0-9._-]+\/branches\/[A-Za-z0-9._-]+\/endpoints\/[A-Za-z0-9._-]+$/; +const HOST_NAME = /^[A-Za-z0-9](?:[A-Za-z0-9.-]{0,251}[A-Za-z0-9])?$/; + +/** Share in-flight lookups only between pools using the same workspace identity. */ +type Client = NonNullable; +const checks = new WeakMap>>(); + +type HostCheckConfig = Pick< + Partial, + "endpoint" | "host" | "workspaceClient" | "password" +>; + +/** + * Refuse a confirmed OAuth host mismatch before the pool can access another branch. + * An endpoint that cannot be read is not proof of a mismatch, so it skips the + * check without blocking startup. + */ +export function assertEndpointHostMatches( + config: HostCheckConfig, +): Promise { + const endpoint = config.endpoint ?? process.env.LAKEBASE_ENDPOINT; + const host = config.host ?? process.env.PGHOST; + const client = config.workspaceClient; + if ( + !endpoint || + !host || + !client || + config.password !== undefined || + !ENDPOINT_NAME.test(endpoint) || + !HOST_NAME.test(host) + ) { + return Promise.resolve(); + } + const key = `${endpoint}\n${host.toLowerCase()}`; + let clientChecks = checks.get(client); + if (!clientChecks) { + clientChecks = new Map(); + checks.set(client, clientChecks); + } + let check = clientChecks.get(key); + if (!check) { + check = compareHosts(client, endpoint, host).finally(() => { + clientChecks.delete(key); + }); + clientChecks.set(key, check); + } + return check; +} + +async function compareHosts( + client: NonNullable, + endpoint: string, + host: string, +): Promise { + let timer: ReturnType | undefined; + const controller = new AbortController(); + let hosts: string[]; + const warnUnverified = (reason: string) => + logger.warn( + "Could not verify PGHOST %s against LAKEBASE_ENDPOINT %s (%s). Check the endpoint host manually before using this pool.", + host, + endpoint, + reason, + ); + try { + const response = await Promise.race([ + client.apiClient.request( + { + path: `/api/2.0/postgres/${endpoint}`, + method: "GET", + headers: new Headers({ Accept: "application/json" }), + raw: false, + }, + contextFromAbortSignal(controller.signal), + ), + new Promise((resolve) => { + timer = setTimeout(() => { + controller.abort(); + resolve(undefined); + }, HOST_CHECK_TIMEOUT_MS); + }), + ]); + hosts = endpointHosts(response); + } catch (error) { + // An endpoint that no longer exists is itself the misconfiguration. + if ( + typeof error === "object" && + error !== null && + "statusCode" in error && + error.statusCode === 404 + ) { + logger.warn( + "LAKEBASE_ENDPOINT %s was not found. Check the project, branch, and endpoint names; `databricks postgres list-endpoints projects/{project}/branches/{branch}` lists them with their hosts.", + endpoint, + ); + return; + } + warnUnverified( + controller.signal.aborted ? "lookup timed out" : "lookup failed", + ); + return; + } finally { + clearTimeout(timer); + } + if (hosts.length === 0) { + warnUnverified( + controller.signal.aborted + ? "lookup timed out" + : "endpoint returned no hosts", + ); + return; + } + if (hosts.includes(host.toLowerCase())) return; + logger.error( + "PGHOST %s is not a host of LAKEBASE_ENDPOINT %s (expected %s). Refusing to connect to the wrong branch.", + host, + endpoint, + hosts.join(" or "), + ); + throw new ConfigurationError( + `PGHOST ${host} is not a host of LAKEBASE_ENDPOINT ${endpoint} (expected ${hosts.join(" or ")}). Set PGHOST to a host of the configured endpoint before starting the app.`, + ); +} + +/** Read every host the endpoint serves, read-write and read-only. */ +function endpointHosts(response: unknown): string[] { + if (!response || typeof response !== "object") return []; + const status = Reflect.get(response, "status"); + if (!status || typeof status !== "object") return []; + const hosts = Reflect.get(status, "hosts"); + if (!hosts || typeof hosts !== "object") return []; + return Object.values(hosts) + .filter( + (value): value is string => + typeof value === "string" && HOST_NAME.test(value), + ) + .map((value) => value.toLowerCase()); +} diff --git a/packages/appkit/src/connectors/lakebase/index.ts b/packages/appkit/src/connectors/lakebase/index.ts index b7bf2e1d4..08e169d14 100644 --- a/packages/appkit/src/connectors/lakebase/index.ts +++ b/packages/appkit/src/connectors/lakebase/index.ts @@ -10,6 +10,7 @@ import { ServiceContext } from "../../context/service-context"; import { ConfigurationError } from "../../errors"; import { createLogger } from "../../logging/logger"; import { createWorkspaceClient } from "../../workspace-client"; +import { assertEndpointHostMatches } from "./endpoint-host"; /** * Create a Lakebase pool with appkit's logger integration. @@ -47,7 +48,10 @@ export async function initializeLakebasePool( : createWorkspaceClient({ clientOptions: getClientOptions() }); resolved.workspaceClient = client.toLegacyWorkspaceClient(); } - const user = await getUsernameWithApiLookup(resolved); + const [user] = await Promise.all([ + getUsernameWithApiLookup(resolved), + assertEndpointHostMatches(resolved), + ]); if (!user) { throw ConfigurationError.invalidConnection( "Lakebase", @@ -72,6 +76,7 @@ export { type RequestedResource, } from "@databricks/lakebase"; +export { assertEndpointHostMatches } from "./endpoint-host"; export { createLakebasePoolManager, type LakebasePoolManager, diff --git a/packages/appkit/src/connectors/lakebase/tests/endpoint-host.test.ts b/packages/appkit/src/connectors/lakebase/tests/endpoint-host.test.ts new file mode 100644 index 000000000..b66c0567d --- /dev/null +++ b/packages/appkit/src/connectors/lakebase/tests/endpoint-host.test.ts @@ -0,0 +1,318 @@ +import type { LakebasePoolConfig } from "@databricks/lakebase"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; + +import { assertEndpointHostMatches } from "../endpoint-host"; + +type Client = NonNullable; + +// Unique pairs keep concurrent checks from different tests independent. +let sequence = 0; +function names() { + sequence += 1; + return { + endpoint: `projects/p${sequence}/branches/b/endpoints/primary`, + host: `ep-configured-${sequence}.database.example.com`, + }; +} + +function clientAnswering(response: unknown | Promise) { + const request = vi.fn(async () => response); + return { request, client: { apiClient: { request } } as unknown as Client }; +} + +const endpointWith = (hosts: Record) => ({ + name: "ignored", + status: { hosts }, +}); + +let warn: ReturnType; +const warnings = () => warn.mock.calls.flat().map(String).join(" "); + +beforeEach(() => { + vi.stubEnv("LAKEBASE_ENDPOINT", ""); + vi.stubEnv("PGHOST", ""); + warn = vi.spyOn(console, "warn").mockImplementation(() => undefined); +}); +afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); + vi.unstubAllEnvs(); +}); + +describe("assertEndpointHostMatches", () => { + test("rejects a confirmed mismatch with both hosts before a pool is allocated", async () => { + const { endpoint, host } = names(); + const { request, client } = clientAnswering( + endpointWith({ host: "ep-expected.database.example.com" }), + ); + const errorLog = vi + .spyOn(console, "error") + .mockImplementation(() => undefined); + + await expect( + assertEndpointHostMatches({ endpoint, host, workspaceClient: client }), + ).rejects.toThrow(host); + + expect(request).toHaveBeenCalledWith( + expect.objectContaining({ + path: `/api/2.0/postgres/${endpoint}`, + method: "GET", + }), + expect.anything(), + ); + expect(warn).not.toHaveBeenCalled(); + const output = errorLog.mock.calls.flat().map(String).join(" "); + expect(output).toContain(host); + expect(output).toContain(endpoint); + expect(output).toContain("ep-expected.database.example.com"); + }); + + test("reads the endpoint and host from the environment", async () => { + const { endpoint, host } = names(); + vi.stubEnv("LAKEBASE_ENDPOINT", endpoint); + vi.stubEnv("PGHOST", host); + const { request, client } = clientAnswering( + endpointWith({ host: "ep-other.database.example.com" }), + ); + + await expect( + assertEndpointHostMatches({ workspaceClient: client }), + ).rejects.toThrow("ep-other.database.example.com"); + + expect(request).toHaveBeenCalledOnce(); + }); + + test.each([ + ["the read-write host", (host: string) => ({ host: host.toUpperCase() })], + [ + "a read-only host", + (host: string) => ({ + host: "ep-primary.database.example.com", + read_only_host: host, + }), + ], + ])("stays quiet when PGHOST is %s", async (_label, hostsFor) => { + const { endpoint, host } = names(); + const { client } = clientAnswering(endpointWith(hostsFor(host))); + + await assertEndpointHostMatches({ + endpoint, + host, + workspaceClient: client, + }); + + expect(warn).not.toHaveBeenCalled(); + }); + + test.each([ + ["no endpoint", { endpoint: undefined }], + ["no host", { host: undefined }], + ["no workspace client", { workspaceClient: undefined }], + [ + "an endpoint that is not a resource name", + { endpoint: "../../jobs/list" }, + ], + [ + "an endpoint with query characters", + { endpoint: "projects/p/branches/b/endpoints/primary?token=private" }, + ], + [ + "an endpoint with a trailing newline", + { endpoint: "projects/p/branches/b/endpoints/primary\n" }, + ], + ["a host with a newline", { host: "ep.example.com\nsecret" }], + ["a host with a trailing newline", { host: "ep.example.com\n" }], + ])("sends no request with %s", async (_label, override) => { + const { request, client } = clientAnswering(endpointWith({})); + + await assertEndpointHostMatches({ + ...names(), + workspaceClient: client, + ...override, + }); + + expect(request).not.toHaveBeenCalled(); + }); + + test.each([ + ["the lookup fails", () => Promise.reject(new Error("403 Forbidden"))], + ["the endpoint lists no hosts", () => endpointWith({})], + ["the response has no status", () => ({ name: "x" })], + ])("continues startup but warns when %s", async (_label, answer) => { + const { endpoint, host } = names(); + const request = vi.fn(async () => answer()); + const client = { apiClient: { request } } as unknown as Client; + + await expect( + assertEndpointHostMatches({ endpoint, host, workspaceClient: client }), + ).resolves.toBeUndefined(); + expect(warnings()).toContain("Could not verify PGHOST"); + }); + + test("does not log or trust a malformed host in the endpoint response", async () => { + const { endpoint, host } = names(); + const { client } = clientAnswering( + endpointWith({ host: "ep-expected.database.example.com\nprivate row" }), + ); + const errorLog = vi + .spyOn(console, "error") + .mockImplementation(() => undefined); + + await expect( + assertEndpointHostMatches({ endpoint, host, workspaceClient: client }), + ).resolves.toBeUndefined(); + expect(errorLog).not.toHaveBeenCalled(); + expect(warnings()).toContain("Could not verify PGHOST"); + expect(warnings()).not.toContain("private row"); + }); + + test("warns when the endpoint no longer exists", async () => { + const { endpoint, host } = names(); + const request = vi.fn(async () => { + throw Object.assign(new Error("branch id not found"), { + statusCode: 404, + }); + }); + const client = { apiClient: { request } } as unknown as Client; + + await assertEndpointHostMatches({ + endpoint, + host, + workspaceClient: client, + }); + + expect(warnings()).toContain(`${endpoint} was not found`); + expect(warnings()).not.toContain("branch id not found"); + }); + + test("gives up on a slow lookup without holding startup", async () => { + vi.useFakeTimers(); + const { endpoint, host } = names(); + const { client } = clientAnswering(new Promise(() => undefined)); + + const check = assertEndpointHostMatches({ + endpoint, + host, + workspaceClient: client, + }); + await vi.advanceTimersByTimeAsync(3_000); + + await expect(check).resolves.toBeUndefined(); + expect(warnings()).toContain("Could not verify PGHOST"); + }); + + test("aborts the outbound SDK request when the three-second deadline expires", async () => { + vi.useFakeTimers(); + const { endpoint, host } = names(); + const request = vi.fn( + async (_request: unknown, _context?: unknown) => + new Promise(() => undefined), + ); + const client = { apiClient: { request } } as unknown as Client; + + const check = assertEndpointHostMatches({ + endpoint, + host, + workspaceClient: client, + }); + await vi.advanceTimersByTimeAsync(3_000); + await check; + + const context = request.mock.calls[0]?.[1] as + | { cancellationToken?: { isCancellationRequested: boolean } } + | undefined; + expect(context?.cancellationToken?.isCancellationRequested).toBe(true); + }); + + test("retries after an API failure instead of caching an unverified host", async () => { + const { endpoint, host } = names(); + const request = vi + .fn() + .mockRejectedValueOnce(new Error("403 Forbidden")) + .mockResolvedValueOnce( + endpointWith({ host: "ep-expected.database.example.com" }), + ); + const client = { apiClient: { request } } as unknown as Client; + + await assertEndpointHostMatches({ + endpoint, + host, + workspaceClient: client, + }); + await expect( + assertEndpointHostMatches({ endpoint, host, workspaceClient: client }), + ).rejects.toThrow("ep-expected.database.example.com"); + expect(request).toHaveBeenCalledTimes(2); + }); + + test("rechecks after a successful lookup in case the endpoint host changes", async () => { + const { endpoint, host } = names(); + const request = vi + .fn() + .mockResolvedValueOnce(endpointWith({ host })) + .mockResolvedValueOnce( + endpointWith({ host: "ep-new.database.example.com" }), + ); + const client = { apiClient: { request } } as unknown as Client; + + await assertEndpointHostMatches({ + endpoint, + host, + workspaceClient: client, + }); + await expect( + assertEndpointHostMatches({ endpoint, host, workspaceClient: client }), + ).rejects.toThrow("ep-new.database.example.com"); + expect(request).toHaveBeenCalledTimes(2); + }); + + test("does not reuse an inaccessible client's check for another identity", async () => { + const { endpoint, host } = names(); + const denied = vi.fn(async () => { + throw new Error("403 Forbidden"); + }); + const { request: allowed, client: allowedClient } = clientAnswering( + endpointWith({ host: "ep-expected.database.example.com" }), + ); + const deniedClient = { + apiClient: { request: denied }, + } as unknown as Client; + + const results = await Promise.allSettled([ + assertEndpointHostMatches({ + endpoint, + host, + workspaceClient: deniedClient, + }), + assertEndpointHostMatches({ + endpoint, + host, + workspaceClient: allowedClient, + }), + ]); + + expect(results.map(({ status }) => status)).toEqual([ + "fulfilled", + "rejected", + ]); + expect(allowed).toHaveBeenCalledOnce(); + }); + + test("shares one lookup between pools for the same endpoint and host", async () => { + const { endpoint, host } = names(); + const { request, client } = clientAnswering( + endpointWith({ host: "ep-expected.database.example.com" }), + ); + + const results = await Promise.allSettled([ + assertEndpointHostMatches({ endpoint, host, workspaceClient: client }), + assertEndpointHostMatches({ endpoint, host, workspaceClient: client }), + ]); + + expect(request).toHaveBeenCalledOnce(); + expect(results.map(({ status }) => status)).toEqual([ + "rejected", + "rejected", + ]); + }); +}); diff --git a/packages/appkit/src/connectors/lakebase/tests/initialize-pool.test.ts b/packages/appkit/src/connectors/lakebase/tests/initialize-pool.test.ts index 4f16b9af9..2c4f5989b 100644 --- a/packages/appkit/src/connectors/lakebase/tests/initialize-pool.test.ts +++ b/packages/appkit/src/connectors/lakebase/tests/initialize-pool.test.ts @@ -8,9 +8,11 @@ import type { UserContext } from "../../../context/user-context"; const mocks = vi.hoisted(() => { const me = vi.fn(); - const client = { currentUser: { me } }; + const request = vi.fn(); + const client = { currentUser: { me }, apiClient: { request } }; return { me, + request, client, createPool: vi.fn(), createWorkspaceClient: vi.fn(() => ({ @@ -190,6 +192,23 @@ describe("AppKit Lakebase connector initialization", () => { expect(mocks.me).not.toHaveBeenCalled(); }); + test("does not validate a Lakebase endpoint for native password authentication", async () => { + mocks.request.mockResolvedValue({ + status: { hosts: { host: "ep-other.database.example.com" } }, + }); + + expect( + await initializeLakebasePool({ + user: "native-user", + password: "test-only-password", + endpoint: "projects/native/branches/test/endpoints/primary", + host: "ep-native.database.example.com", + workspaceClient: mocks.client as unknown as Client, + }), + ).toBe(pool); + expect(mocks.request).not.toHaveBeenCalled(); + }); + test("does not allocate a pool when identity cannot be resolved", async () => { mocks.me.mockRejectedValueOnce(new Error("Private auth detail")); await expect(initializeLakebasePool()).rejects.toMatchObject({ @@ -200,4 +219,25 @@ describe("AppKit Lakebase connector initialization", () => { }); expect(mocks.createPool).not.toHaveBeenCalled(); }); + + test("refuses a pool when PGHOST is not a host of the endpoint", async () => { + vi.stubEnv( + "LAKEBASE_ENDPOINT", + "projects/p/branches/fresh/endpoints/primary", + ); + vi.stubEnv("PGHOST", "ep-stale.database.example.test"); + mocks.request.mockResolvedValue({ + status: { hosts: { host: "ep-fresh.database.example.test" } }, + }); + await expect(initializeLakebasePool()).rejects.toThrow( + "ep-fresh.database.example.test", + ); + + expect(mocks.request.mock.calls[0]?.[0]).toEqual( + expect.objectContaining({ + path: "/api/2.0/postgres/projects/p/branches/fresh/endpoints/primary", + }), + ); + expect(mocks.createPool).not.toHaveBeenCalled(); + }); }); diff --git a/packages/appkit/src/database/runtime/engine/drizzle-data-path.ts b/packages/appkit/src/database/runtime/engine/drizzle-data-path.ts index b89bcc208..5695a7140 100644 --- a/packages/appkit/src/database/runtime/engine/drizzle-data-path.ts +++ b/packages/appkit/src/database/runtime/engine/drizzle-data-path.ts @@ -159,27 +159,85 @@ function upsertUpdateValues( // nested `cause` rather than the thrown error. Walk a bounded chain to find it. const MAX_CAUSE_DEPTH = 5; -function sqlStateOf(error: unknown): string | undefined { +// Even schema and authorization errors can carry values in message/detail/hint. +// Admit only the exact identifier-only forms needed to diagnose schema drift. +const SAFE_COLUMN_ERROR = + /^column (?:[A-Za-z_][A-Za-z0-9_]*\.)?[A-Za-z_][A-Za-z0-9_]* does not exist$/; +const SAFE_RELATION_ERROR = + /^relation "[A-Za-z_][A-Za-z0-9_]*(?:\.[A-Za-z_][A-Za-z0-9_]*)?" does not exist$/; +const SAFE_DNS_ERROR = /^getaddrinfo ENOTFOUND [A-Za-z0-9.-]+$/; +const MAX_DIAGNOSTIC_LENGTH = 200; +const SYSTEM_ERROR_CODES = new Set([ + "ENOTFOUND", + "EAI_AGAIN", + "ECONNREFUSED", + "ETIMEDOUT", + "EHOSTUNREACH", + "ENETUNREACH", + "EPIPE", +]); + +interface DriverFailure { + readonly sqlState?: string; + /** Server-log-only driver text; it never reaches the thrown error. */ + readonly diagnostic?: string; +} + +/** Never log unvalidated driver text, even for an otherwise known SQLSTATE. */ +function safeDriverText(carrier: object, code: string): string | undefined { + try { + const message = Reflect.get(carrier, "message"); + if (typeof message !== "string" || message.length > MAX_DIAGNOSTIC_LENGTH) + return undefined; + if (code === "42703" && SAFE_COLUMN_ERROR.test(message)) return message; + if (code === "42P01" && SAFE_RELATION_ERROR.test(message)) return message; + if ( + code === "ENOTFOUND" && + Reflect.get(carrier, "syscall") === "getaddrinfo" && + SAFE_DNS_ERROR.test(message) + ) + return message; + return undefined; + } catch { + return undefined; + } +} + +function driverFailureOf(error: unknown): DriverFailure { let current = error; for (let depth = 0; depth < MAX_CAUSE_DEPTH; depth++) { - if (!current || typeof current !== "object") return undefined; + if (!current || typeof current !== "object") return {}; try { const candidate = Reflect.get(current, "code"); + // Node system errors are untrusted too; only a well-formed DNS host is safe. + if ( + typeof candidate === "string" && + SYSTEM_ERROR_CODES.has(candidate) && + typeof Reflect.get(current, "syscall") === "string" + ) { + return { diagnostic: safeDriverText(current, candidate) ?? candidate }; + } // SQLSTATE is always a five-character alphanumeric class code. if (typeof candidate === "string" && /^[0-9A-Z]{5}$/.test(candidate)) { - return candidate; + return { + sqlState: candidate, + diagnostic: safeDriverText(current, candidate), + }; } current = Reflect.get(current, "cause"); } catch { - return undefined; + return {}; } } - return undefined; + return {}; } -/** Classify SQLSTATE without retaining the driver error or its properties. */ +/** + * Classify SQLSTATE without retaining the driver error or its properties. + * Only allowlisted identifier-only messages enter the server log. + */ function classifyDriverError(error: unknown): DatabasePluginError { - const code = sqlStateOf(error); + const { sqlState: code, diagnostic } = driverFailureOf(error); const category: DatabaseErrorCategory = code === "40001" || code === "40P01" || code === "57014" ? "TRANSIENT" @@ -189,9 +247,10 @@ function classifyDriverError(error: unknown): DatabasePluginError { ? "CONFLICT" : "INTERNAL"; logger.error( - "Database driver error classified as %s (SQLSTATE %s)", + "Database driver error classified as %s (SQLSTATE %s)%s", category, code ?? "unknown", + diagnostic ? `: ${diagnostic}` : "", ); return new DatabasePluginError(category, "runtime"); } diff --git a/packages/appkit/src/database/runtime/tests/drizzle-data-path.test.ts b/packages/appkit/src/database/runtime/tests/drizzle-data-path.test.ts index fbf223733..b9d35409b 100644 --- a/packages/appkit/src/database/runtime/tests/drizzle-data-path.test.ts +++ b/packages/appkit/src/database/runtime/tests/drizzle-data-path.test.ts @@ -815,6 +815,209 @@ describe("database failures", () => { } }); + it.each(["42501", "57014", "42601", "42703", "42P01", "28000"])( + "does not log arbitrary driver text for SQLSTATE %s", + async (code) => { + const fake = makeFakeDb(); + const query = fake.db.query as unknown as Record< + string, + { findMany: () => Promise } + >; + query.users.findMany = async () => { + throw { + code, + message: "patient@example.test must not reach the log", + detail: "private row value patient@example.test", + hint: "try patient@example.test", + }; + }; + const errorLog = vi + .spyOn(console, "error") + .mockImplementation(() => undefined); + + try { + await createDrizzleDataPath(fake.db, schema) + .select(users, {}) + .catch(() => undefined); + + const output = errorLog.mock.calls.flat().map(String).join(" "); + expect(output).toContain(code); + expect(output).not.toContain("patient@example.test"); + } finally { + errorLog.mockRestore(); + } + }, + ); + + it("does not admit trailing newlines in otherwise allowed diagnostics", async () => { + const fake = makeFakeDb(); + const query = fake.db.query as unknown as Record< + string, + { findMany: () => Promise } + >; + query.users.findMany = async () => { + throw { code: "42703", message: "column notes.body does not exist\n" }; + }; + const errorLog = vi + .spyOn(console, "error") + .mockImplementation(() => undefined); + + try { + await createDrizzleDataPath(fake.db, schema) + .select(users, {}) + .catch(() => undefined); + expect(errorLog.mock.calls.flat().map(String).join(" ")).not.toContain( + "\n", + ); + } finally { + errorLog.mockRestore(); + } + }); + + it("bounds untrusted system error codes in server logs", async () => { + const fake = makeFakeDb(); + const query = fake.db.query as unknown as Record< + string, + { findMany: () => Promise } + >; + const oversizedCode = `E${"A".repeat(500)}`; + query.users.findMany = async () => { + throw { code: oversizedCode, syscall: "connect", message: "sensitive" }; + }; + const errorLog = vi + .spyOn(console, "error") + .mockImplementation(() => undefined); + + try { + await createDrizzleDataPath(fake.db, schema) + .select(users, {}) + .catch(() => undefined); + expect(errorLog.mock.calls.flat().map(String).join(" ")).not.toContain( + oversizedCode, + ); + } finally { + errorLog.mockRestore(); + } + }); + + // Schema drift used to surface as a bare SQLSTATE, with no object named. + it.each([ + [ + "42703", + { + message: "column notes.board_id does not exist", + hint: 'Perhaps you meant to reference the column "notes.body".', + }, + ["column notes.board_id does not exist"], + ], + [ + "42P01", + { message: 'relation "public.boards" does not exist' }, + ['relation "public.boards" does not exist'], + ], + ] as const)( + "logs safe schema identifiers for SQLSTATE %s, never free-form text", + async (code, fields, expected) => { + const fake = makeFakeDb(); + const query = fake.db.query as unknown as Record< + string, + { findMany: () => Promise } + >; + query.users.findMany = async () => { + const driver = Object.assign(new Error(fields.message), { + code, + ...fields, + }); + throw Object.assign( + new Error("Failed query: select * from users\nparams: alice@x.com"), + { cause: driver }, + ); + }; + const errorLog = vi + .spyOn(console, "error") + .mockImplementation(() => undefined); + + try { + const error = await createDrizzleDataPath(fake.db, schema) + .select(users, {}) + .catch((caught) => caught); + + const output = errorLog.mock.calls.flat().map(String).join(" "); + for (const text of expected) expect(output).toContain(text); + expect(output).not.toContain("Perhaps you meant"); + expect(output).not.toContain("Failed query"); + expect(output).not.toContain("alice@x.com"); + // The diagnostic stays in the log; callers still get the stable error. + expect(error.message).toBe("Database operation failed"); + expect(JSON.stringify(error)).not.toContain(fields.message); + } finally { + errorLog.mockRestore(); + } + }, + ); + + it.each([ + ["22P02", 'invalid input syntax for type integer: "alice@x.com"'], + ["23502", "null value in column alice@x.com"], + ["P0001", "raised for alice@x.com"], + ])( + "never logs driver text for value-bearing SQLSTATE %s", + async (code, message) => { + const fake = makeFakeDb(); + const query = fake.db.query as unknown as Record< + string, + { findMany: () => Promise } + >; + query.users.findMany = async () => { + throw { code, message, detail: "Key (email)=(alice@x.com)" }; + }; + const errorLog = vi + .spyOn(console, "error") + .mockImplementation(() => undefined); + + try { + await createDrizzleDataPath(fake.db, schema) + .select(users, {}) + .catch(() => undefined); + + const output = errorLog.mock.calls.flat().map(String).join(" "); + expect(output).toContain(code); + expect(output).not.toContain("alice@x.com"); + } finally { + errorLog.mockRestore(); + } + }, + ); + + it("logs a connection failure's system error text", async () => { + const fake = makeFakeDb(); + const query = fake.db.query as unknown as Record< + string, + { findMany: () => Promise } + >; + query.users.findMany = async () => { + throw Object.assign( + new Error("getaddrinfo ENOTFOUND ep-stale.database.example.com"), + { code: "ENOTFOUND", syscall: "getaddrinfo" }, + ); + }; + const errorLog = vi + .spyOn(console, "error") + .mockImplementation(() => undefined); + + try { + const error = await createDrizzleDataPath(fake.db, schema) + .select(users, {}) + .catch((caught) => caught); + + const output = errorLog.mock.calls.flat().map(String).join(" "); + expect(output).toContain("ENOTFOUND ep-stale.database.example.com"); + expect(error).toMatchObject({ category: "INTERNAL" }); + } finally { + errorLog.mockRestore(); + } + }); + it("stops walking an error cause cycle", async () => { const fake = makeFakeDb(); const query = fake.db.query as unknown as Record< diff --git a/packages/appkit/src/plugins/database/lifecycle.ts b/packages/appkit/src/plugins/database/lifecycle.ts index 60fa80066..c50badf6f 100644 --- a/packages/appkit/src/plugins/database/lifecycle.ts +++ b/packages/appkit/src/plugins/database/lifecycle.ts @@ -23,6 +23,7 @@ import type { SqlTag, TransactionClient, } from "./entity-types"; +import { assertSchemaMatchesDatabase } from "./schema-check"; import { createMutationScope, type MutationScope } from "./scope"; import type { DatabaseHooks } from "./types"; @@ -161,7 +162,10 @@ function buildDatabaseExports(context: ExportContext): DatabaseExports { return result; } -/** Validate the schema, create one pool-backed API, and verify connectivity. */ +/** + * Validate the schema, create one pool-backed API, and verify connectivity and + * that every declared table and column exists. + */ export async function createDatabaseState( schema: TSchema, execute: EntityExecute, @@ -200,6 +204,7 @@ export async function createDatabaseState( }); // Do not publish exports until an authenticated statement succeeds. await dataPath.raw`select 1`; + await assertSchemaMatchesDatabase(dataPath, schema); return { pool, exports, @@ -208,9 +213,16 @@ export async function createDatabaseState( }, }; } catch (error) { - logger.error("Database setup failed: %O", error); active = false; await pool?.end().catch(() => undefined); + // A setup failure names its own reason from schema metadata; anything + // else may carry connector or driver details and is replaced. + if (error instanceof DatabasePluginError && error.phase === "setup") { + logger.error("%s", error.message); + throw error; + } + // Unknown driver and SDK errors may carry SQL, parameters, and credentials. + logger.error("Database setup failed (connector or driver error)"); throw new DatabasePluginError("SETUP_FAILED", "setup"); } } diff --git a/packages/appkit/src/plugins/database/schema-check.ts b/packages/appkit/src/plugins/database/schema-check.ts new file mode 100644 index 000000000..f1334ecda --- /dev/null +++ b/packages/appkit/src/plugins/database/schema-check.ts @@ -0,0 +1,82 @@ +import { databaseSetupFailed } from "../../database/errors"; +import type { DataPath } from "../../database/runtime"; +import type { Schema } from "../../database/schema-builder"; + +interface CatalogColumn { + readonly table_name: string; + readonly column_name: string | null; + readonly relation_kind: string; +} + +/** + * Confirm every declared table and column exists before routes are published. + * The plugin never migrates, so drift would otherwise surface on each request + * as an undefined-table or undefined-column failure. pg_catalog is read rather + * than information_schema, which hides objects the role cannot access and + * would report a privilege gap as a missing column. + */ +export async function assertSchemaMatchesDatabase( + dataPath: DataPath, + schema: Schema, +): Promise { + const tables = Object.values(schema.$tables); + if (tables.length === 0) return; + const schemaName = schema.$schemaName; + const tableNames = tables.map((table) => table.$name); + // Include non-tables so setup can report their kind instead of claiming they are absent. + const rows = await dataPath.raw` + select c.relname::text as table_name, a.attname::text as column_name, + c.relkind::text as relation_kind + from pg_catalog.pg_class c + join pg_catalog.pg_namespace n on n.oid = c.relnamespace + left join pg_catalog.pg_attribute a + on a.attrelid = c.oid and a.attnum > 0 and not a.attisdropped + where n.nspname::text = ${schemaName} + and c.relname::text = any(${tableNames}::text[]) + and c.relkind in ('r', 'p', 'v', 'm', 'f')`; + + const found = new Map }>(); + for (const row of rows) { + const relation = found.get(row.table_name) ?? { + kind: row.relation_kind, + columns: new Set(), + }; + if (row.column_name) relation.columns.add(row.column_name); + found.set(row.table_name, relation); + } + + const problems: string[] = []; + for (const table of tables) { + const qualified = `${schemaName}.${table.$name}`; + const relation = found.get(table.$name); + if (!relation) { + problems.push(`table ${qualified} does not exist`); + continue; + } + if (relation.kind !== "r" && relation.kind !== "p") { + const kind = + relation.kind === "v" + ? "view" + : relation.kind === "m" + ? "materialized view" + : relation.kind === "f" + ? "foreign table" + : "non-table relation"; + problems.push(`table ${qualified} is a ${kind}, not a table`); + continue; + } + const missing = Object.values(table.$columns) + .map((meta) => meta.columnName) + .filter((name) => !relation.columns.has(name)); + if (missing.length > 0) { + problems.push( + `table ${qualified} is missing ${missing.length === 1 ? "column" : "columns"} ${missing.join(", ")}`, + ); + } + } + if (problems.length > 0) { + throw databaseSetupFailed( + `the declared schema does not match the database: ${problems.join("; ")}. Create or migrate these before starting the app; the plugin does not.`, + ); + } +} diff --git a/packages/appkit/src/plugins/database/tests/crud.integration.test.ts b/packages/appkit/src/plugins/database/tests/crud.integration.test.ts index 7a2b6fe6f..6088a4315 100644 --- a/packages/appkit/src/plugins/database/tests/crud.integration.test.ts +++ b/packages/appkit/src/plugins/database/tests/crud.integration.test.ts @@ -20,6 +20,10 @@ vi.mock("../../../database/runtime/engine/drizzle-data-path", () => ({ createDrizzleDb: mocks.createDrizzleDb, createDrizzleDataPath: mocks.createDrizzleDataPath, })); +// The catalog comparison has its own suite; this fake answers no catalog. +vi.mock("../schema-check", () => ({ + assertSchemaMatchesDatabase: async () => undefined, +})); import { DatabasePlugin } from "../database"; diff --git a/packages/appkit/src/plugins/database/tests/lifecycle.test.ts b/packages/appkit/src/plugins/database/tests/lifecycle.test.ts index 20bbb04de..aae7c04d2 100644 --- a/packages/appkit/src/plugins/database/tests/lifecycle.test.ts +++ b/packages/appkit/src/plugins/database/tests/lifecycle.test.ts @@ -8,6 +8,7 @@ const mocks = vi.hoisted(() => ({ initializeLakebasePool: vi.fn(), createDrizzleDb: vi.fn(), createDrizzleDataPath: vi.fn(), + assertSchemaMatchesDatabase: vi.fn(), })); vi.mock("../../../connectors/lakebase", () => ({ initializeLakebasePool: mocks.initializeLakebasePool, @@ -16,6 +17,10 @@ vi.mock("../../../database/runtime/engine/drizzle-data-path", () => ({ createDrizzleDb: mocks.createDrizzleDb, createDrizzleDataPath: mocks.createDrizzleDataPath, })); +// The catalog comparison has its own suite; these fakes answer no catalog. +vi.mock("../schema-check", () => ({ + assertSchemaMatchesDatabase: mocks.assertSchemaMatchesDatabase, +})); import { IDLE_IN_TRANSACTION_TIMEOUT_MS, @@ -156,6 +161,9 @@ describe("createDatabaseState", () => { async (stage) => { const { pool, execute } = arrange(); const raw = new Error("secret constraint detail"); + const errorLog = vi + .spyOn(console, "error") + .mockImplementation(() => undefined); if (stage === "drizzle") mocks.createDrizzleDb.mockImplementationOnce(() => { throw raw; @@ -179,11 +187,40 @@ describe("createDatabaseState", () => { expect(error.message).toBe("Database setup failed"); expect(error.cause).toBeUndefined(); expect(pool.end).toHaveBeenCalledTimes(1); + expect(errorLog.mock.calls.flat().map(String).join(" ")).not.toContain( + raw.message, + ); }, ); + test("checks the catalog after readiness and keeps a drift failure's reason", async () => { + const { pool, path, execute } = arrange(); + const drift = new DatabasePluginError( + "SETUP_FAILED", + "setup", + "Database setup failed: table public.tags does not exist", + ); + mocks.assertSchemaMatchesDatabase.mockRejectedValueOnce(drift); + + const error = await createDatabaseState(schema, execute).catch( + (caught) => caught, + ); + + expect(path.raw).toHaveBeenCalledTimes(1); + expect(mocks.assertSchemaMatchesDatabase).toHaveBeenCalledWith( + path, + schema, + ); + expect(error).toBe(drift); + expect(error.message).toContain("table public.tags does not exist"); + expect(pool.end).toHaveBeenCalledTimes(1); + }); + test("sanitizes connector initialization failures", async () => { const { execute } = arrange(); + const errorLog = vi + .spyOn(console, "error") + .mockImplementation(() => undefined); mocks.initializeLakebasePool.mockRejectedValueOnce( new Error("secret host and credential details"), ); @@ -195,6 +232,9 @@ describe("createDatabaseState", () => { expect(error).toMatchObject({ category: "SETUP_FAILED", phase: "setup" }); expect(error.message).toBe("Database setup failed"); expect(error.cause).toBeUndefined(); + expect(errorLog.mock.calls.flat().map(String).join(" ")).not.toContain( + "secret host and credential details", + ); }); test("runs root SQL directly, maps failures safely, and rejects after deactivation", async () => { diff --git a/packages/appkit/src/plugins/database/tests/mvp.integration.test.ts b/packages/appkit/src/plugins/database/tests/mvp.integration.test.ts index 7aab7a0ed..514f60bc0 100644 --- a/packages/appkit/src/plugins/database/tests/mvp.integration.test.ts +++ b/packages/appkit/src/plugins/database/tests/mvp.integration.test.ts @@ -22,6 +22,10 @@ vi.mock("../../../database/runtime/engine/drizzle-data-path", () => ({ createDrizzleDb: mocks.createDrizzleDb, createDrizzleDataPath: mocks.createDrizzleDataPath, })); +// The catalog comparison has its own suite; this fake answers no catalog. +vi.mock("../schema-check", () => ({ + assertSchemaMatchesDatabase: async () => undefined, +})); import { DatabasePlugin } from "../database"; diff --git a/packages/appkit/src/plugins/database/tests/schema-check.test.ts b/packages/appkit/src/plugins/database/tests/schema-check.test.ts new file mode 100644 index 000000000..6f10836a8 --- /dev/null +++ b/packages/appkit/src/plugins/database/tests/schema-check.test.ts @@ -0,0 +1,145 @@ +import { describe, expect, test, vi } from "vitest"; + +import type { DataPath } from "../../../database/runtime"; +import { defineSchema, fk, id, text } from "../../../database/schema-builder"; +import { assertSchemaMatchesDatabase } from "../schema-check"; + +const schema = defineSchema(({ table }) => { + const boards = table("boards", { id: id(), title: text().notNull() }); + const notes = table("notes", { + id: id(), + board_id: fk(() => boards.id).notNull(), + author_email: text().private(), + body: text().notNull(), + }); + return { boards, notes }; +}); + +type CatalogRow = { + table_name: string; + column_name: string | null; + relation_kind: string; +}; + +/** Answer the catalog query with the given tables and columns. */ +function catalog( + tables: Record, + kinds: Record = {}, +) { + const rows: CatalogRow[] = Object.entries(tables).flatMap( + ([name, columns]): CatalogRow[] => + columns.length === 0 + ? [ + { + table_name: name, + column_name: null, + relation_kind: kinds[name] ?? "r", + }, + ] + : columns.map((column) => ({ + table_name: name, + column_name: column, + relation_kind: kinds[name] ?? "r", + })), + ); + const raw = vi.fn(async () => rows); + return { raw, path: { raw } as unknown as DataPath }; +} + +describe("assertSchemaMatchesDatabase", () => { + test("accepts a database with every declared column, and extra ones", async () => { + const { path } = catalog({ + boards: ["id", "title", "archived_at"], + notes: ["id", "board_id", "author_email", "body"], + }); + await expect( + assertSchemaMatchesDatabase(path, schema), + ).resolves.toBeUndefined(); + }); + + test("names every missing table and column in one setup failure", async () => { + // The shape a same-named table from another app leaves behind. + const { path } = catalog({ notes: ["id", "case_id", "author", "content"] }); + + const error = await assertSchemaMatchesDatabase(path, schema).catch( + (caught) => caught, + ); + + expect(error).toMatchObject({ category: "SETUP_FAILED", phase: "setup" }); + expect(error.message).toContain("table public.boards does not exist"); + expect(error.message).toContain( + "table public.notes is missing columns board_id, author_email, body", + ); + // Only the stable message crosses a request boundary. + expect(error.clientMessage).toBe("Database setup failed"); + }); + + test("uses the singular for one missing column", async () => { + const { path } = catalog({ + boards: ["id", "title"], + notes: ["id", "board_id", "body"], + }); + await expect(assertSchemaMatchesDatabase(path, schema)).rejects.toThrow( + "table public.notes is missing column author_email", + ); + }); + + test("reports a table with no readable columns as missing them all", async () => { + const { path } = catalog({ + boards: [], + notes: ["id", "board_id", "author_email", "body"], + }); + await expect(assertSchemaMatchesDatabase(path, schema)).rejects.toThrow( + "table public.boards is missing columns id, title", + ); + }); + + test.each([ + ["v", "view"], + ["m", "materialized view"], + ["f", "foreign table"], + ])( + "rejects %s as a CRUD table even when all columns exist", + async (kind, description) => { + const { path } = catalog( + { + boards: ["id", "title"], + notes: ["id", "board_id", "author_email", "body"], + }, + { boards: kind }, + ); + + await expect(assertSchemaMatchesDatabase(path, schema)).rejects.toThrow( + `public.boards is a ${description}, not a table`, + ); + }, + ); + + test("passes the schema and table names as parameter values", async () => { + const scoped = defineSchema( + ({ table }) => ({ tags: table("tags", { id: id() }) }), + { schemaName: "playground" }, + ); + const { raw, path } = catalog({ tags: ["id"] }); + + await assertSchemaMatchesDatabase(path, scoped); + + expect(raw).toHaveBeenCalledTimes(1); + const [strings, ...values] = raw.mock.calls[0] as unknown as [ + TemplateStringsArray, + ...unknown[], + ]; + expect(strings.join("?")).toContain("pg_catalog.pg_attribute"); + expect(strings.join("?")).toContain("c.relkind::text as relation_kind"); + expect(values).toEqual(["playground", ["tags"]]); + }); + + test("skips the catalog query for an empty schema", async () => { + const { raw, path } = catalog({}); + await assertSchemaMatchesDatabase( + path, + defineSchema(() => ({})), + ); + expect(raw).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/appkit/src/plugins/lakebase/lakebase.ts b/packages/appkit/src/plugins/lakebase/lakebase.ts index 5f351d827..dd4fe5a92 100644 --- a/packages/appkit/src/plugins/lakebase/lakebase.ts +++ b/packages/appkit/src/plugins/lakebase/lakebase.ts @@ -11,6 +11,7 @@ import { type LakebasePool, type LakebasePoolManager, RoutingPool, + assertEndpointHostMatches, } from "../../connectors/lakebase"; import { getClientOptions } from "../../context/client-options"; import { getCallerContext } from "../../context/execution-context"; @@ -85,7 +86,10 @@ export class LakebasePlugin extends Plugin implements ToolProvider { clientOptions: getClientOptions(), }).toLegacyWorkspaceClient(), }; - const user = await getUsernameWithApiLookup(poolConfig); + const [user] = await Promise.all([ + getUsernameWithApiLookup(poolConfig), + assertEndpointHostMatches(poolConfig), + ]); const spPool = createLakebasePool({ ...poolConfig, user }); logger.info("Lakebase SP pool initialized"); diff --git a/packages/appkit/src/plugins/lakebase/tests/lakebase-agent-tool.test.ts b/packages/appkit/src/plugins/lakebase/tests/lakebase-agent-tool.test.ts index b33eaef5c..214d236ee 100644 --- a/packages/appkit/src/plugins/lakebase/tests/lakebase-agent-tool.test.ts +++ b/packages/appkit/src/plugins/lakebase/tests/lakebase-agent-tool.test.ts @@ -1,3 +1,4 @@ +import type { LakebasePoolConfig } from "@databricks/lakebase"; import { beforeEach, describe, expect, test, vi } from "vitest"; /** * Tests the agent-tool surface of the Lakebase plugin. @@ -106,6 +107,55 @@ describe("LakebasePlugin - agent tool opt-in", () => { }); }); +test("LakebasePlugin rejects a confirmed host mismatch before creating either pool", async () => { + const { createLakebasePool, createLakebasePoolManager } = + await import("../../../connectors/lakebase"); + vi.mocked(createLakebasePool).mockClear(); + vi.mocked(createLakebasePoolManager).mockClear(); + const request = vi.fn(async () => ({ + status: { hosts: { host: "ep-expected.database.example.com" } }, + })); + const workspaceClient = { apiClient: { request } } as unknown as NonNullable< + LakebasePoolConfig["workspaceClient"] + >; + const plugin = makePlugin({ + pool: { + endpoint: "projects/plugin/branches/test/endpoints/primary", + host: "ep-other.database.example.com", + workspaceClient, + }, + }); + + await expect(plugin.setup()).rejects.toThrow( + "ep-expected.database.example.com", + ); + expect(createLakebasePool).not.toHaveBeenCalled(); + expect(createLakebasePoolManager).not.toHaveBeenCalled(); +}); + +test("LakebasePlugin permits a separate host when using native password authentication", async () => { + const { createLakebasePool } = await import("../../../connectors/lakebase"); + const request = vi.fn(async () => ({ + status: { hosts: { host: "ep-expected.database.example.com" } }, + })); + const workspaceClient = { apiClient: { request } } as unknown as NonNullable< + LakebasePoolConfig["workspaceClient"] + >; + const plugin = makePlugin({ + pool: { + user: "native-user", + password: "test-only-password", + endpoint: "projects/native/branches/test/endpoints/primary", + host: "ep-native.database.example.com", + workspaceClient, + }, + }); + + await expect(plugin.setup()).resolves.toBeUndefined(); + expect(request).not.toHaveBeenCalled(); + expect(createLakebasePool).toHaveBeenCalled(); +}); + describe("LakebasePlugin - readOnly enforcement", () => { let plugin: LakebasePlugin;