From 528e4d9af63fcecb13f3d20c9961278a6dff8e80 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Wed, 21 Oct 2026 12:06:56 +0200 Subject: [PATCH 1/5] feat(appkit): operations-first routes for the database plugin (POC) Add defineQuery/defineMutation/definePolicy and a database({ routes }) option, auto-discovered from config/database/routes/*.ts. Every route runs one pipeline: actor from x-forwarded-user, zod input validation, strict authorize (only literal true allows; throw or non-decision is a 500), then execute with the service-principal db. Mutations run authorize and execute in one transaction. Routes that overlap generated CRUD fail setup. Co-authored-by: Isaac Signed-off-by: MarioCadenas --- docs/docs/api/appkit/Class.AppKitError.md | 1 + docs/docs/api/appkit/Class.OperationError.md | 198 +++++++++ .../api/appkit/Function.defineMutation.md | 24 ++ docs/docs/api/appkit/Function.definePolicy.md | 23 ++ docs/docs/api/appkit/Function.defineQuery.md | 24 ++ docs/docs/api/appkit/Function.runOperation.md | 35 ++ docs/docs/api/appkit/Interface.Actor.md | 19 + docs/docs/api/appkit/Interface.Operation.md | 76 ++++ .../api/appkit/Interface.OperationContext.md | 33 ++ .../api/appkit/TypeAlias.DatabaseRoutes.md | 7 + .../api/appkit/TypeAlias.IDatabaseConfig.md | 18 + docs/docs/api/appkit/TypeAlias.Policy.md | 24 ++ .../docs/api/appkit/TypeAlias.PolicyResult.md | 11 + docs/docs/api/appkit/index.md | 11 + docs/docs/api/appkit/typedoc-sidebar.ts | 55 +++ packages/appkit/src/beta.ts | 15 +- .../appkit/src/plugins/database/database.ts | 124 ++++++ packages/appkit/src/plugins/database/index.ts | 13 + .../appkit/src/plugins/database/lifecycle.ts | 9 +- .../appkit/src/plugins/database/operations.ts | 285 +++++++++++++ .../plugins/database/tests/operations.test.ts | 389 ++++++++++++++++++ packages/appkit/src/plugins/database/types.ts | 12 + 22 files changed, 1404 insertions(+), 2 deletions(-) create mode 100644 docs/docs/api/appkit/Class.OperationError.md create mode 100644 docs/docs/api/appkit/Function.defineMutation.md create mode 100644 docs/docs/api/appkit/Function.definePolicy.md create mode 100644 docs/docs/api/appkit/Function.defineQuery.md create mode 100644 docs/docs/api/appkit/Function.runOperation.md create mode 100644 docs/docs/api/appkit/Interface.Actor.md create mode 100644 docs/docs/api/appkit/Interface.Operation.md create mode 100644 docs/docs/api/appkit/Interface.OperationContext.md create mode 100644 docs/docs/api/appkit/TypeAlias.DatabaseRoutes.md create mode 100644 docs/docs/api/appkit/TypeAlias.Policy.md create mode 100644 docs/docs/api/appkit/TypeAlias.PolicyResult.md create mode 100644 packages/appkit/src/plugins/database/operations.ts create mode 100644 packages/appkit/src/plugins/database/tests/operations.test.ts diff --git a/docs/docs/api/appkit/Class.AppKitError.md b/docs/docs/api/appkit/Class.AppKitError.md index 3cb3a0a5b..a057098d5 100644 --- a/docs/docs/api/appkit/Class.AppKitError.md +++ b/docs/docs/api/appkit/Class.AppKitError.md @@ -27,6 +27,7 @@ console.error(error.toJSON()); // Safe for logging, sensitive values redacted ## Extended by +- [`OperationError`](Class.OperationError.md) - [`AuthenticationError`](Class.AuthenticationError.md) - [`ConfigurationError`](Class.ConfigurationError.md) - [`ConnectionError`](Class.ConnectionError.md) diff --git a/docs/docs/api/appkit/Class.OperationError.md b/docs/docs/api/appkit/Class.OperationError.md new file mode 100644 index 000000000..d39cde742 --- /dev/null +++ b/docs/docs/api/appkit/Class.OperationError.md @@ -0,0 +1,198 @@ +# Class: OperationError + +A failure whose message is written for the caller: denial, bad input, or a business conflict. + +## Extends + +- [`AppKitError`](Class.AppKitError.md) + +## Constructors + +### Constructor + +```ts +new OperationError( + statusCode: 400 | 401 | 403 | 404 | 409 | 500, + message: string, + details?: readonly { + message: string; + path: string; +}[]): OperationError; +``` + +#### Parameters + +| Parameter | Type | +| ------ | ------ | +| `statusCode` | `400` \| `401` \| `403` \| `404` \| `409` \| `500` | +| `message` | `string` | +| `details?` | readonly \{ `message`: `string`; `path`: `string`; \}[] | + +#### Returns + +`OperationError` + +#### Overrides + +[`AppKitError`](Class.AppKitError.md).[`constructor`](Class.AppKitError.md#constructor) + +## Properties + +### \_clientMessage? + +```ts +protected readonly optional _clientMessage: string; +``` + +Client-safe error message. When set, callers serializing the error to +a client (SSE, HTTP body) MUST prefer `clientMessage` over `message`. +`message` may contain raw upstream / SDK text including statement +fragments, internal object names, and correlation IDs. + +Subclasses can set this in their constructor for a fixed sanitized +string. When unset, `clientMessage` defaults to a generic per-code +string (see the getter), and the raw `message` is kept server-side +only. + +#### Inherited from + +[`AppKitError`](Class.AppKitError.md).[`_clientMessage`](Class.AppKitError.md#_clientmessage) + +*** + +### cause? + +```ts +readonly optional cause: Error; +``` + +Optional cause of the error + +#### Inherited from + +[`AppKitError`](Class.AppKitError.md).[`cause`](Class.AppKitError.md#cause) + +*** + +### code + +```ts +readonly code: "OPERATION_ERROR" = "OPERATION_ERROR"; +``` + +Error code for programmatic error handling + +#### Overrides + +[`AppKitError`](Class.AppKitError.md).[`code`](Class.AppKitError.md#code) + +*** + +### context? + +```ts +readonly optional context: Record; +``` + +Additional context for the error + +#### Inherited from + +[`AppKitError`](Class.AppKitError.md).[`context`](Class.AppKitError.md#context) + +*** + +### details? + +```ts +readonly optional details: readonly { + message: string; + path: string; +}[]; +``` + +*** + +### isRetryable + +```ts +readonly isRetryable: false = false; +``` + +Whether this error type is generally safe to retry + +#### Overrides + +[`AppKitError`](Class.AppKitError.md).[`isRetryable`](Class.AppKitError.md#isretryable) + +*** + +### statusCode + +```ts +readonly statusCode: 400 | 401 | 403 | 404 | 409 | 500; +``` + +HTTP status code suggestion (can be overridden) + +#### Inherited from + +[`AppKitError`](Class.AppKitError.md).[`statusCode`](Class.AppKitError.md#statuscode) + +## Accessors + +### clientMessage + +#### Get Signature + +```ts +get clientMessage(): string; +``` + +Sanitized message safe to forward to clients. Override in subclasses +if a more specific default is appropriate. + +##### Returns + +`string` + +#### Inherited from + +[`AppKitError`](Class.AppKitError.md).[`clientMessage`](Class.AppKitError.md#clientmessage) + +## Methods + +### toJSON() + +```ts +toJSON(): Record; +``` + +Convert error to JSON for logging/serialization. +Sensitive values in context are automatically redacted. + +#### Returns + +`Record`\<`string`, `unknown`\> + +#### Inherited from + +[`AppKitError`](Class.AppKitError.md).[`toJSON`](Class.AppKitError.md#tojson) + +*** + +### toString() + +```ts +toString(): string; +``` + +Create a human-readable string representation + +#### Returns + +`string` + +#### Inherited from + +[`AppKitError`](Class.AppKitError.md).[`toString`](Class.AppKitError.md#tostring) diff --git a/docs/docs/api/appkit/Function.defineMutation.md b/docs/docs/api/appkit/Function.defineMutation.md new file mode 100644 index 000000000..d8d5b2f57 --- /dev/null +++ b/docs/docs/api/appkit/Function.defineMutation.md @@ -0,0 +1,24 @@ +# Function: defineMutation() + +```ts +function defineMutation(definition: OperationDefinition): Operation; +``` + +A state change. `authorize` and `execute` share one transaction, so a refusal or failure rolls back. + +## Type Parameters + +| Type Parameter | +| ------ | +| `S` *extends* `ZodType`\<`unknown`, `unknown`, `$ZodTypeInternals`\<`unknown`, `unknown`\>\> | +| `R` | + +## Parameters + +| Parameter | Type | +| ------ | ------ | +| `definition` | `OperationDefinition`\<`S`, `R`\> | + +## Returns + +[`Operation`](Interface.Operation.md)\<`S`, `R`\> diff --git a/docs/docs/api/appkit/Function.definePolicy.md b/docs/docs/api/appkit/Function.definePolicy.md new file mode 100644 index 000000000..9a04c4ef2 --- /dev/null +++ b/docs/docs/api/appkit/Function.definePolicy.md @@ -0,0 +1,23 @@ +# Function: definePolicy() + +```ts +function definePolicy(policy: Policy): Policy; +``` + +Type a reusable authorization check; the input type is what the policy reads. + +## Type Parameters + +| Type Parameter | +| ------ | +| `I` | + +## Parameters + +| Parameter | Type | +| ------ | ------ | +| `policy` | [`Policy`](TypeAlias.Policy.md)\<`I`\> | + +## Returns + +[`Policy`](TypeAlias.Policy.md)\<`I`\> diff --git a/docs/docs/api/appkit/Function.defineQuery.md b/docs/docs/api/appkit/Function.defineQuery.md new file mode 100644 index 000000000..c204ab66a --- /dev/null +++ b/docs/docs/api/appkit/Function.defineQuery.md @@ -0,0 +1,24 @@ +# Function: defineQuery() + +```ts +function defineQuery(definition: OperationDefinition): Operation; +``` + +A read. Runs `authorize` then `execute` outside a transaction. + +## Type Parameters + +| Type Parameter | +| ------ | +| `S` *extends* `ZodType`\<`unknown`, `unknown`, `$ZodTypeInternals`\<`unknown`, `unknown`\>\> | +| `R` | + +## Parameters + +| Parameter | Type | +| ------ | ------ | +| `definition` | `OperationDefinition`\<`S`, `R`\> | + +## Returns + +[`Operation`](Interface.Operation.md)\<`S`, `R`\> diff --git a/docs/docs/api/appkit/Function.runOperation.md b/docs/docs/api/appkit/Function.runOperation.md new file mode 100644 index 000000000..751df6f8d --- /dev/null +++ b/docs/docs/api/appkit/Function.runOperation.md @@ -0,0 +1,35 @@ +# Function: runOperation() + +```ts +function runOperation( + operation: Operation, + call: { + actor: Actor | undefined; + input: unknown; +}, +db: DatabaseExports): Promise; +``` + +The one execution path for an operation, whoever calls it: validate the +input, authorize, then execute with the service-principal `db`. + +## Type Parameters + +| Type Parameter | +| ------ | +| `S` *extends* `ZodType`\<`unknown`, `unknown`, `$ZodTypeInternals`\<`unknown`, `unknown`\>\> | +| `R` | + +## Parameters + +| Parameter | Type | +| ------ | ------ | +| `operation` | [`Operation`](Interface.Operation.md)\<`S`, `R`\> | +| `call` | \{ `actor`: [`Actor`](Interface.Actor.md) \| `undefined`; `input`: `unknown`; \} | +| `call.actor` | [`Actor`](Interface.Actor.md) \| `undefined` | +| `call.input` | `unknown` | +| `db` | [`DatabaseExports`](TypeAlias.DatabaseExports.md) | + +## Returns + +`Promise`\<`R`\> diff --git a/docs/docs/api/appkit/Interface.Actor.md b/docs/docs/api/appkit/Interface.Actor.md new file mode 100644 index 000000000..edb4ca558 --- /dev/null +++ b/docs/docs/api/appkit/Interface.Actor.md @@ -0,0 +1,19 @@ +# Interface: Actor + +The authenticated user a request acts for. Never the database identity. + +## Properties + +### userEmail? + +```ts +readonly optional userEmail: string; +``` + +*** + +### userId + +```ts +readonly userId: string; +``` diff --git a/docs/docs/api/appkit/Interface.Operation.md b/docs/docs/api/appkit/Interface.Operation.md new file mode 100644 index 000000000..6708d443f --- /dev/null +++ b/docs/docs/api/appkit/Interface.Operation.md @@ -0,0 +1,76 @@ +# Interface: Operation\ + +## Extends + +- `OperationDefinition`\<`S`, `R`\> + +## Type Parameters + +| Type Parameter | Default type | +| ------ | ------ | +| `S` *extends* `z.ZodType` | `z.ZodType` | +| `R` | `unknown` | + +## Properties + +### authorize + +```ts +readonly authorize: + | Policy> + | readonly Policy>[]; +``` + +Required: an operation without an authorization decision is never exposed. + +#### Inherited from + +```ts +OperationDefinition.authorize +``` + +*** + +### execute() + +```ts +readonly execute: (context: OperationContext>) => Promise; +``` + +#### Parameters + +| Parameter | Type | +| ------ | ------ | +| `context` | [`OperationContext`](Interface.OperationContext.md)\<`output`\<`S`\>\> | + +#### Returns + +`Promise`\<`R`\> + +#### Inherited from + +```ts +OperationDefinition.execute +``` + +*** + +### input + +```ts +readonly input: S; +``` + +#### Inherited from + +```ts +OperationDefinition.input +``` + +*** + +### kind + +```ts +readonly kind: "query" | "mutation"; +``` diff --git a/docs/docs/api/appkit/Interface.OperationContext.md b/docs/docs/api/appkit/Interface.OperationContext.md new file mode 100644 index 000000000..cae20948e --- /dev/null +++ b/docs/docs/api/appkit/Interface.OperationContext.md @@ -0,0 +1,33 @@ +# Interface: OperationContext\ + +What `authorize` and `execute` receive. `db` always runs as the app's service principal. + +## Type Parameters + +| Type Parameter | +| ------ | +| `I` | + +## Properties + +### actor + +```ts +readonly actor: Actor; +``` + +*** + +### db + +```ts +readonly db: TransactionClient; +``` + +*** + +### input + +```ts +readonly input: I; +``` diff --git a/docs/docs/api/appkit/TypeAlias.DatabaseRoutes.md b/docs/docs/api/appkit/TypeAlias.DatabaseRoutes.md new file mode 100644 index 000000000..445e8e3f4 --- /dev/null +++ b/docs/docs/api/appkit/TypeAlias.DatabaseRoutes.md @@ -0,0 +1,7 @@ +# Type Alias: DatabaseRoutes + +```ts +type DatabaseRoutes = Readonly>; +``` + +Operations exposed over HTTP, keyed `"METHOD /path"` under `/api/database`. diff --git a/docs/docs/api/appkit/TypeAlias.IDatabaseConfig.md b/docs/docs/api/appkit/TypeAlias.IDatabaseConfig.md index 2461127b6..93cb7ccfe 100644 --- a/docs/docs/api/appkit/TypeAlias.IDatabaseConfig.md +++ b/docs/docs/api/appkit/TypeAlias.IDatabaseConfig.md @@ -4,6 +4,7 @@ type IDatabaseConfig = { api?: DatabaseApiConfig; hooks?: { readonly [TTable in SchemaTableName]?: EntityHooks }; + routes?: DatabaseRoutes; schema?: TSchema; }; ``` @@ -49,6 +50,23 @@ readonly optional hooks: { readonly [TTable in SchemaTableName]?: Entit *** +### routes? + +```ts +readonly optional routes: DatabaseRoutes; +``` + +Operations exposed over HTTP under `/api/database`, keyed +`"METHOD /path"`. Each runs through the same pipeline: the actor comes from +the Apps proxy (`x-forwarded-user`), the input is validated, `authorize` +must return `true`, then `execute` runs as the service principal. + +When omitted, setup merges the default exports of +`config/database/routes/*.ts`. Passing `routes` disables that discovery. +A route that overlaps another or a generated CRUD route fails setup. + +*** + ### schema? ```ts diff --git a/docs/docs/api/appkit/TypeAlias.Policy.md b/docs/docs/api/appkit/TypeAlias.Policy.md new file mode 100644 index 000000000..ac0e05ae9 --- /dev/null +++ b/docs/docs/api/appkit/TypeAlias.Policy.md @@ -0,0 +1,24 @@ +# Type Alias: Policy()\ + +```ts +type Policy = (context: OperationContext) => + | PolicyResult +| Promise; +``` + +## Type Parameters + +| Type Parameter | +| ------ | +| `I` | + +## Parameters + +| Parameter | Type | +| ------ | ------ | +| `context` | [`OperationContext`](Interface.OperationContext.md)\<`I`\> | + +## Returns + + \| [`PolicyResult`](TypeAlias.PolicyResult.md) + \| `Promise`\<[`PolicyResult`](TypeAlias.PolicyResult.md)\> diff --git a/docs/docs/api/appkit/TypeAlias.PolicyResult.md b/docs/docs/api/appkit/TypeAlias.PolicyResult.md new file mode 100644 index 000000000..421218ad9 --- /dev/null +++ b/docs/docs/api/appkit/TypeAlias.PolicyResult.md @@ -0,0 +1,11 @@ +# Type Alias: PolicyResult + +```ts +type PolicyResult = + | boolean + | { + deny: string; +}; +``` + +Only a literal `true` allows; `false` or `{ deny }` refuses with 403. diff --git a/docs/docs/api/appkit/index.md b/docs/docs/api/appkit/index.md index ee6db8b67..4c4d931c5 100644 --- a/docs/docs/api/appkit/index.md +++ b/docs/docs/api/appkit/index.md @@ -26,6 +26,7 @@ surface with `@databricks/appkit/beta`. Not meant for application imports. | [IdentityExpiredError](Class.IdentityExpiredError.md) | The downstream service rejected the active caller's credentials. | | [InitializationError](Class.InitializationError.md) | Error thrown when a service or component is not properly initialized. Use when accessing services before they are ready. | | [MlflowClient](Class.MlflowClient.md) | A thin client over the Databricks workspace REST API, owning the host + bearer token so callers (eval-run creation, assessment writes, the judge's serving endpoint) don't each re-derive URLs or re-attach auth. The host is normalized once at construction. | +| [OperationError](Class.OperationError.md) | A failure whose message is written for the caller: denial, bad input, or a business conflict. | | [Plugin](Class.Plugin.md) | Base abstract class for creating AppKit plugins. | | [PolicyDeniedError](Class.PolicyDeniedError.md) | Thrown when a policy denies an action. | | [ResourceRegistry](Class.ResourceRegistry.md) | Central registry for tracking plugin resource requirements. Deduplication uses type + resourceKey (machine-stable); alias is for display only. | @@ -39,6 +40,7 @@ surface with `@databricks/appkit/beta`. Not meant for application imports. | Interface | Description | | ------ | ------ | +| [Actor](Interface.Actor.md) | The authenticated user a request acts for. Never the database identity. | | [AgentAdapter](Interface.AgentAdapter.md) | - | | [AgentDefinition](Interface.AgentDefinition.md) | - | | [AgentInput](Interface.AgentInput.md) | - | @@ -95,6 +97,8 @@ surface with `@databricks/appkit/beta`. Not meant for application imports. | [MatchResult](Interface.MatchResult.md) | Result of a deterministic matcher run against a value. | | [McpConnectAllResult](Interface.McpConnectAllResult.md) | Per-endpoint outcome of [AppKitMcpClient.connectAll](Class.AppKitMcpClient.md#connectall). Callers (the agents plugin in particular) use the split to warn at startup when some MCP servers are unreachable without aborting boot for the rest. | | [Message](Interface.Message.md) | - | +| [Operation](Interface.Operation.md) | - | +| [OperationContext](Interface.OperationContext.md) | What `authorize` and `execute` receive. `db` always runs as the app's service principal. | | [PluginManifest](Interface.PluginManifest.md) | Plugin manifest that declares metadata and resource requirements. Attached to plugin classes as a static property. Extends the shared PluginManifest with strict resource types. | | [PluginToolkitProvider](Interface.PluginToolkitProvider.md) | Minimum shape every entry in the [Plugins](TypeAlias.Plugins.md) map must expose. Core plugins (analytics, files, genie, lakebase) implement this directly via their `.toolkit()` method. The agents plugin and standalone `runAgent` synthesize this shape for any registered plugin that doesn't implement `.toolkit()` directly (falling back to `getAgentTools()` walking). | | [PostResult](Interface.PostResult.md) | Structured result for a best-effort POST that must not throw. | @@ -153,6 +157,7 @@ surface with `@databricks/appkit/beta`. Not meant for application imports. | [DatabaseApiWriteOperation](TypeAlias.DatabaseApiWriteOperation.md) | Generated HTTP write operations. | | [DatabaseApiWritesConfig](TypeAlias.DatabaseApiWritesConfig.md) | All writes by default; false keeps reads only, and an object narrows writes. | | [DatabaseExports](TypeAlias.DatabaseExports.md) | Typed database API published by the plugin. | +| [DatabaseRoutes](TypeAlias.DatabaseRoutes.md) | Operations exposed over HTTP, keyed `"METHOD /path"` under `/api/database`. | | [EntityHooks](TypeAlias.EntityHooks.md) | Response shaping and mutation lifecycle declared for one table. | | [EvalProgress](TypeAlias.EvalProgress.md) | - | | [ExecutionContext](TypeAlias.ExecutionContext.md) | - | @@ -168,6 +173,8 @@ surface with `@databricks/appkit/beta`. Not meant for application imports. | [Matcher](TypeAlias.Matcher.md) | A deterministic matcher: inspects a string value and returns a result. | | [PluginData](TypeAlias.PluginData.md) | Tuple of plugin class, config, and name. Created by `toPlugin()` and passed to `createApp()`. | | [Plugins](TypeAlias.Plugins.md) | Plugin map passed to the function form of [AgentDefinition.tools](Interface.AgentDefinition.md#tools). Each entry exposes a `.toolkit(opts?)` method that returns a record of [ToolkitEntry](Interface.ToolkitEntry.md) markers ready to be spread into a tool record. | +| [Policy](TypeAlias.Policy.md) | - | +| [PolicyResult](TypeAlias.PolicyResult.md) | Only a literal `true` allows; `false` or `{ deny }` refuses with 403. | | [ReadSerializer](TypeAlias.ReadSerializer.md) | Shape one already private-safe row before it reaches the wire. A `Promise` is not assignable to the return type, so an async callback fails to compile: serializers run inside the response path and must not add latency there. | | [ResolvedToolEntry](TypeAlias.ResolvedToolEntry.md) | Internal tool-index entry after a tool record has been resolved to a dispatchable form. | | [ResourceFieldEntry](TypeAlias.ResourceFieldEntry.md) | - | @@ -218,6 +225,9 @@ surface with `@databricks/appkit/beta`. Not meant for application imports. | [defineEval](Function.defineEval.md) | Define an agent eval. Default-export the result from a `server/agents//evals/*.eval.ts` file. | | [defineEvalConfig](Function.defineEvalConfig.md) | Define per-directory eval config. Default-export from `evals.config.ts`. | | [defineManifest](Function.defineManifest.md) | Validates a raw manifest (typically a `manifest.json` import) against the canonical Zod schema and returns it as a strict [PluginManifest](Interface.PluginManifest.md). | +| [defineMutation](Function.defineMutation.md) | A state change. `authorize` and `execute` share one transaction, so a refusal or failure rolls back. | +| [definePolicy](Function.definePolicy.md) | Type a reusable authorization check; the input type is what the policy reads. | +| [defineQuery](Function.defineQuery.md) | A read. Runs `authorize` then `execute` outside a transaction. | | [defineSchema](Function.defineSchema.md) | Compile one declared schema. The returned type keeps the table names the builder returned, so `api.tables` and `hooks` can name only real tables. | | [defineTool](Function.defineTool.md) | Defines a single tool entry for a plugin's internal registry. | | [discoverEvalConfigs](Function.discoverEvalConfigs.md) | Discover the per-agent `evals.config.ts` (from [defineEvalConfig](Function.defineEvalConfig.md)) at `/server/agents//evals/evals.config.ts`. Config is per-agent: each agent's config applies only to that agent's evals. Agents without a config file are omitted. Returns a stable, sorted list. | @@ -281,6 +291,7 @@ surface with `@databricks/appkit/beta`. Not meant for application imports. | [runEvalsInDir](Function.runEvalsInDir.md) | Discover, load, and run every eval under each agent's `evals/` dir, driving the agents on a running app. Never throws for an individual eval — load/run failures become non-passing [EvalResult](Interface.EvalResult.md)s. | | [runInCallerContext](Function.runInCallerContext.md) | Run a function with an immutable snapshot of the caller context. Nested and concurrent scopes keep their own identities. | | [~~runInUserContext~~](Function.runInUserContext.md) | - | +| [runOperation](Function.runOperation.md) | The one execution path for an operation, whoever calls it: validate the input, authorize, then execute with the service-principal `db`. | | [runWithRetries](Function.runWithRetries.md) | Run `attempt` up to `1 + retries` times, stopping as soon as it returns a result that is neither a thrown error / per-eval timeout (`error`) nor a transport/agent turn failure (`infraFailure`). Assertion failures set neither, so a failed-but-completed eval is returned on the first try and never retried. Returns the last result when every attempt failed on infra. | | [summarize](Function.summarize.md) | - | | [text](Function.text.md) | - | diff --git a/docs/docs/api/appkit/typedoc-sidebar.ts b/docs/docs/api/appkit/typedoc-sidebar.ts index dca9af23b..048bd759d 100644 --- a/docs/docs/api/appkit/typedoc-sidebar.ts +++ b/docs/docs/api/appkit/typedoc-sidebar.ts @@ -81,6 +81,11 @@ const typedocSidebar: SidebarsConfig = { id: "api/appkit/Class.MlflowClient", label: "MlflowClient" }, + { + type: "doc", + id: "api/appkit/Class.OperationError", + label: "OperationError" + }, { type: "doc", id: "api/appkit/Class.Plugin", @@ -127,6 +132,11 @@ const typedocSidebar: SidebarsConfig = { type: "category", label: "Interfaces", items: [ + { + type: "doc", + id: "api/appkit/Interface.Actor", + label: "Actor" + }, { type: "doc", id: "api/appkit/Interface.AgentAdapter", @@ -409,6 +419,16 @@ const typedocSidebar: SidebarsConfig = { id: "api/appkit/Interface.Message", label: "Message" }, + { + type: "doc", + id: "api/appkit/Interface.Operation", + label: "Operation" + }, + { + type: "doc", + id: "api/appkit/Interface.OperationContext", + label: "OperationContext" + }, { type: "doc", id: "api/appkit/Interface.PluginManifest", @@ -680,6 +700,11 @@ const typedocSidebar: SidebarsConfig = { id: "api/appkit/TypeAlias.DatabaseExports", label: "DatabaseExports" }, + { + type: "doc", + id: "api/appkit/TypeAlias.DatabaseRoutes", + label: "DatabaseRoutes" + }, { type: "doc", id: "api/appkit/TypeAlias.EntityHooks", @@ -755,6 +780,16 @@ const typedocSidebar: SidebarsConfig = { id: "api/appkit/TypeAlias.Plugins", label: "Plugins" }, + { + type: "doc", + id: "api/appkit/TypeAlias.Policy", + label: "Policy" + }, + { + type: "doc", + id: "api/appkit/TypeAlias.PolicyResult", + label: "PolicyResult" + }, { type: "doc", id: "api/appkit/TypeAlias.ReadSerializer", @@ -968,6 +1003,21 @@ const typedocSidebar: SidebarsConfig = { id: "api/appkit/Function.defineManifest", label: "defineManifest" }, + { + type: "doc", + id: "api/appkit/Function.defineMutation", + label: "defineMutation" + }, + { + type: "doc", + id: "api/appkit/Function.definePolicy", + label: "definePolicy" + }, + { + type: "doc", + id: "api/appkit/Function.defineQuery", + label: "defineQuery" + }, { type: "doc", id: "api/appkit/Function.defineSchema", @@ -1287,6 +1337,11 @@ const typedocSidebar: SidebarsConfig = { label: "runInUserContext", className: "typedoc-sidebar-item-deprecated" }, + { + type: "doc", + id: "api/appkit/Function.runOperation", + label: "runOperation" + }, { type: "doc", id: "api/appkit/Function.runWithRetries", diff --git a/packages/appkit/src/beta.ts b/packages/appkit/src/beta.ts index 03d574653..687bbc16b 100644 --- a/packages/appkit/src/beta.ts +++ b/packages/appkit/src/beta.ts @@ -121,8 +121,21 @@ export type { SearchResult, } from "./plugins/ai-search/types"; export * from "./plugins/beta-exports.generated"; -export { database } from "./plugins/database"; +export { + database, + defineMutation, + definePolicy, + defineQuery, + OperationError, + runOperation, +} from "./plugins/database"; export type { + Actor, + DatabaseRoutes, + Operation, + OperationContext, + Policy, + PolicyResult, DatabaseApiConfig, DatabaseApiWriteOperation, DatabaseApiWritesConfig, diff --git a/packages/appkit/src/plugins/database/database.ts b/packages/appkit/src/plugins/database/database.ts index ff4a49ac4..f283e9beb 100644 --- a/packages/appkit/src/plugins/database/database.ts +++ b/packages/appkit/src/plugins/database/database.ts @@ -28,6 +28,17 @@ import type { DatabaseExports } from "./entity-types"; import { createDatabaseState, type DatabaseState } from "./lifecycle"; import { loadDefaultDatabaseSchema } from "./load-schema"; import manifest from "./manifest.json"; +import { + type AnyOperation, + isOperation, + loadDefaultDatabaseRoutes, + operationErrorResponse, + parseRouteKey, + type RouteMethod, + resolveActor, + routeSignature, + runOperation, +} from "./operations"; import type { DatabaseHooks, DefaultDatabaseSchema, @@ -48,6 +59,12 @@ export class DatabasePlugin< private exposure: CrudExposure = { tables: [], writes: new Map() }; private resolvedSchema: Schema | null = null; private crudTables: Map | null = null; + private operationRoutes: { + key: string; + method: RouteMethod; + path: string; + operation: AnyOperation; + }[] = []; constructor(config: IDatabaseConfig = {}) { assertDatabaseConfig(config); @@ -56,6 +73,7 @@ export class DatabasePlugin< schema: config.schema, api: config.api, hooks: config.hooks, + routes: config.routes, }; } @@ -93,6 +111,17 @@ export class DatabasePlugin< (operation, options) => this.execute(operation, options), this.hooks(), ); + let operationRoutes: DatabasePlugin["operationRoutes"]; + try { + operationRoutes = resolveOperationRoutes( + this.config.routes ?? (await loadDefaultDatabaseRoutes()), + exposure, + ); + } catch (error) { + candidate.deactivate(); + await candidate.pool.end().catch(() => undefined); + throw error; + } if (this.draining) { // Setup may finish while shutdown is waiting; never publish that state. candidate.deactivate(); @@ -101,6 +130,7 @@ export class DatabasePlugin< } this.resolvedSchema = schema; this.exposure = exposure; + this.operationRoutes = operationRoutes; this.state = candidate; this.printTables(schema); })(); @@ -124,6 +154,15 @@ export class DatabasePlugin< /** Register generated CRUD, subject to the configured table and write restrictions. */ injectRoutes(router: express.Router): void { + // Before CRUD, so a static operation path is not captured by `/:id`. + for (const route of this.operationRoutes) { + this.route(router, { + name: route.key, + method: route.method, + path: route.path, + handler: (req, res) => this.handleOperation(route, req, res), + }); + } if (this.exposure.tables.length === 0) return; const tables = this.exposedTables(); const hooks = this.hooks(); @@ -183,6 +222,51 @@ export class DatabasePlugin< } } + /** Run one operation route: the actor comes from the proxy headers, never the request data. */ + private async handleOperation( + route: (typeof this.operationRoutes)[number], + req: express.Request, + res: express.Response, + ): Promise { + const source = route.method === "get" ? req.query : req.body; + const input = { + ...(typeof source === "object" && source !== null ? source : {}), + // Path parameters address the resource, so the body cannot replace them. + ...req.params, + }; + await this.telemetry.startActiveSpan( + "database.operation.route", + { + attributes: { + operation: route.key, + "http.route": `/api/${this.name}${route.path}`, + }, + }, + async (span) => { + try { + const result = await runOperation( + route.operation, + { actor: resolveActor(req), input }, + this.exports(), + ); + span.setAttribute("outcome", "success"); + res.status(200).setHeader("Cache-Control", "no-store"); + res.json(result ?? null); + } catch (error) { + const { status, body } = operationErrorResponse(error); + span.setAttribute("outcome", status < 500 ? "rejected" : "failed"); + span.setAttribute("http.status_code", status); + if (!res.headersSent) { + res.status(status).setHeader("Cache-Control", "no-store"); + res.json(body); + } + } finally { + span.end(); + } + }, + ); + } + /** Name every declared table once at boot, beside what reaches it over HTTP. */ private printTables(schema: Schema): void { const names = Object.keys(schema.$tables).sort(); @@ -312,6 +396,46 @@ export class DatabasePlugin< } } +/** + * Validate operation routes against each other and against the generated CRUD + * they share `/api/database` with: an exact overlap fails setup instead of + * letting one silently shadow the other. + */ +function resolveOperationRoutes( + routes: Readonly>, + exposure: CrudExposure, +) { + const taken = new Set(); + for (const table of exposure.tables) { + // Conservative: assume every keyed CRUD route, whatever the writes config. + for (const [method, routePath] of [ + ["get", `/${table}`], + ["post", `/${table}`], + ["get", `/${table}/:id`], + ["patch", `/${table}/:id`], + ["delete", `/${table}/:id`], + ]) { + taken.add(routeSignature(method, routePath)); + } + } + return Object.entries(routes).map(([key, operation]) => { + const { method, path } = parseRouteKey(key); + if (!isOperation(operation)) { + throw databaseSetupFailed( + `Route ${JSON.stringify(key)} must map to an operation created with defineQuery() or defineMutation().`, + ); + } + const signature = routeSignature(method, path); + if (taken.has(signature)) { + throw databaseSetupFailed( + `Route ${JSON.stringify(key)} collides with another route or a generated CRUD route. Exclude the table with api.tables or change the path.`, + ); + } + taken.add(signature); + return { key, method, path, operation }; + }); +} + type DatabaseRegistration = { plugin: PluginConstructor>; config: IDatabaseConfig; diff --git a/packages/appkit/src/plugins/database/index.ts b/packages/appkit/src/plugins/database/index.ts index 5a5a88748..095556f41 100644 --- a/packages/appkit/src/plugins/database/index.ts +++ b/packages/appkit/src/plugins/database/index.ts @@ -10,3 +10,16 @@ export type { ReadSerializer, ReadSerializerContext, } from "./types"; +export { + type Actor, + type DatabaseRoutes, + defineMutation, + definePolicy, + defineQuery, + type Operation, + type OperationContext, + OperationError, + type Policy, + type PolicyResult, + runOperation, +} from "./operations"; diff --git a/packages/appkit/src/plugins/database/lifecycle.ts b/packages/appkit/src/plugins/database/lifecycle.ts index c50badf6f..b02135532 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 { OperationError } from "./operations"; import { assertSchemaMatchesDatabase } from "./schema-check"; import { createMutationScope, type MutationScope } from "./scope"; import type { DatabaseHooks } from "./types"; @@ -155,7 +156,13 @@ function buildDatabaseExports(context: ExportContext): DatabaseExports { try { return await runTransaction(context, callback); } catch (error) { - if (error instanceof DatabaseValidationError) throw error; + // Both carry a message written for the caller, not driver detail. + if ( + error instanceof DatabaseValidationError || + error instanceof OperationError + ) { + throw error; + } throw classifyDatabaseError(error, "transaction"); } }; diff --git a/packages/appkit/src/plugins/database/operations.ts b/packages/appkit/src/plugins/database/operations.ts new file mode 100644 index 000000000..c495170be --- /dev/null +++ b/packages/appkit/src/plugins/database/operations.ts @@ -0,0 +1,285 @@ +import { readdir } from "node:fs/promises"; +import path from "node:path"; + +import type { Request } from "express"; +import type { z } from "zod"; + +import { + DatabasePluginError, + databaseSetupFailed, +} from "../../database/errors"; +import { AppKitError } from "../../errors"; +import { createLogger } from "../../logging/logger"; +import type { DatabaseExports, TransactionClient } from "./entity-types"; + +const logger = createLogger("database"); + +/** The authenticated user a request acts for. Never the database identity. */ +export interface Actor { + readonly userId: string; + readonly userEmail?: string; +} + +/** What `authorize` and `execute` receive. `db` always runs as the app's service principal. */ +export interface OperationContext { + readonly actor: Actor; + readonly input: I; + readonly db: TransactionClient; +} + +/** Only a literal `true` allows; `false` or `{ deny }` refuses with 403. */ +export type PolicyResult = boolean | { readonly deny: string }; + +export type Policy = ( + context: OperationContext, +) => PolicyResult | Promise; + +interface OperationDefinition { + readonly input: S; + /** Required: an operation without an authorization decision is never exposed. */ + readonly authorize: Policy> | readonly Policy>[]; + readonly execute: (context: OperationContext>) => Promise; +} + +export interface Operation< + S extends z.ZodType = z.ZodType, + R = unknown, +> extends OperationDefinition { + readonly kind: "query" | "mutation"; +} + +// Variance: a registry holds operations of every input and result type. +export type AnyOperation = Operation; + +/** Operations exposed over HTTP, keyed `"METHOD /path"` under `/api/database`. */ +export type DatabaseRoutes = Readonly>; + +/** Only objects built by the define helpers are runnable, so `authorize` was checked. */ +const defined = new WeakSet(); + +function defineOperation( + kind: Operation["kind"], + definition: OperationDefinition, +): Operation { + const policies = [definition.authorize].flat(); + if ( + policies.length === 0 || + policies.some((policy) => typeof policy !== "function") + ) { + throw databaseSetupFailed( + "An operation requires authorize: a policy function or a non-empty array of them.", + ); + } + const operation = Object.freeze({ ...definition, kind }); + defined.add(operation); + return operation; +} + +/** A read. Runs `authorize` then `execute` outside a transaction. */ +export function defineQuery( + definition: OperationDefinition, +): Operation { + return defineOperation("query", definition); +} + +/** A state change. `authorize` and `execute` share one transaction, so a refusal or failure rolls back. */ +export function defineMutation( + definition: OperationDefinition, +): Operation { + return defineOperation("mutation", definition); +} + +/** Type a reusable authorization check; the input type is what the policy reads. */ +export function definePolicy(policy: Policy): Policy { + return policy; +} + +export function isOperation(value: unknown): value is AnyOperation { + return typeof value === "object" && value !== null && defined.has(value); +} + +/** A failure whose message is written for the caller: denial, bad input, or a business conflict. */ +export class OperationError extends AppKitError { + readonly code = "OPERATION_ERROR"; + readonly isRetryable = false; + + constructor( + readonly statusCode: 400 | 401 | 403 | 404 | 409 | 500, + message: string, + readonly details?: readonly { path: string; message: string }[], + ) { + super(message, { clientMessage: message }); + } +} + +const AUTHORIZATION_FAILED = "Authorization check failed"; + +/** Every policy must return exactly `true`; anything else is a refusal or an error. */ +async function authorize( + operation: AnyOperation, + context: OperationContext, +): Promise { + for (const policy of [operation.authorize].flat() as Policy[]) { + let result: unknown; + try { + result = await policy(context); + } catch (error) { + // A failed permission lookup is never a decision either way. + logger.error("Operation policy threw: %O", describe(error)); + throw new OperationError(500, AUTHORIZATION_FAILED); + } + if (result === true) continue; + if (result === false) throw new OperationError(403, "Forbidden"); + if ( + typeof result === "object" && + result !== null && + typeof (result as { deny?: unknown }).deny === "string" + ) { + throw new OperationError(403, (result as { deny: string }).deny); + } + // A forgotten `return` must not read as permission. + logger.error("Operation policy returned a non-decision"); + throw new OperationError(500, AUTHORIZATION_FAILED); + } +} + +/** Log a name, never a message: driver errors can carry SQL, values, or credentials. */ +function describe(error: unknown): string { + return error instanceof Error ? error.name : typeof error; +} + +/** + * The one execution path for an operation, whoever calls it: validate the + * input, authorize, then execute with the service-principal `db`. + */ +export async function runOperation( + operation: Operation, + call: { readonly actor: Actor | undefined; readonly input: unknown }, + db: DatabaseExports, +): Promise { + if (!isOperation(operation)) { + throw new OperationError(500, "Operation failed"); + } + const actor = call.actor; + if (!actor?.userId) throw new OperationError(401, "Authentication required"); + const parsed = operation.input.safeParse(call.input); + if (!parsed.success) { + throw new OperationError( + 400, + "Invalid input", + parsed.error.issues.map((issue) => ({ + path: issue.path.join("."), + message: issue.message, + })), + ); + } + const run = async (client: TransactionClient) => { + const context = { actor, input: parsed.data, db: client }; + await authorize(operation, context); + return operation.execute(context); + }; + if (operation.kind === "query") return run(db); + + return db.transaction(run); +} + +/** The caller a safe response may describe, with no internal detail. */ +export function operationErrorResponse(error: unknown): { + status: number; + body: { error: string; details?: readonly unknown[] }; +} { + if (error instanceof OperationError) { + return { + status: error.statusCode, + body: error.details + ? { error: error.clientMessage, details: error.details } + : { error: error.clientMessage }, + }; + } + if (error instanceof DatabasePluginError) { + return { status: error.statusCode, body: { error: error.clientMessage } }; + } + logger.error("Operation failed: %s", describe(error)); + return { status: 500, body: { error: "Operation failed" } }; +} + +/** + * The Apps proxy sets `x-forwarded-user` after authenticating the user; it is + * the same header the rest of AppKit trusts. A body or query user id never is. + */ +export function resolveActor(req: Request): Actor | undefined { + const userId = req.header("x-forwarded-user")?.trim(); + const userEmail = req.header("x-forwarded-email")?.trim() || undefined; + if (userId) return { userId, userEmail }; + if (process.env.NODE_ENV === "development") return { userId: "dev-user" }; + return undefined; +} + +const METHODS = ["get", "post", "put", "patch", "delete"] as const; +export type RouteMethod = (typeof METHODS)[number]; + +/** Parse `"POST /orders/:id/approve"`, failing setup on anything else. */ +export function parseRouteKey(key: string): { + method: RouteMethod; + path: string; +} { + const match = /^([A-Z]+) (\/[A-Za-z0-9_\-/:]*)$/.exec(key); + const method = match?.[1].toLowerCase() as RouteMethod | undefined; + if (!match || !method || !METHODS.includes(method)) { + throw databaseSetupFailed( + `Route ${JSON.stringify(key)} must look like "GET /orders" or "POST /orders/:id/approve".`, + ); + } + return { method, path: match[2] }; +} + +/** Identity of a route for collision checks: parameter names do not matter to Express. */ +export function routeSignature(method: string, routePath: string): string { + return `${method.toLowerCase()} ${routePath.replace(/:[^/]+/g, ":")}`; +} + +/** Same application-relative convention as `config/database/schema.ts`. */ +const DEFAULT_ROUTES_DIR = "config/database/routes"; + +/** + * Merge every `config/database/routes/*.ts` default export. A missing folder + * means no routes; a repeated route in two files fails setup. + */ +export async function loadDefaultDatabaseRoutes( + root = process.cwd(), +): Promise { + const dir = path.resolve(root, DEFAULT_ROUTES_DIR); + let files: string[]; + try { + files = (await readdir(dir)).filter((file) => /\.(ts|js|mjs)$/.test(file)); + } catch { + return {}; + } + const { createJiti } = await import("jiti"); + const jiti = createJiti(import.meta.url, { moduleCache: false }); + const routes: Record = {}; + for (const file of files.sort()) { + const label = JSON.stringify(path.join(DEFAULT_ROUTES_DIR, file)); + let exported: unknown; + try { + exported = await jiti.import(path.join(dir, file), { default: true }); + } catch { + // Import errors can contain credentials or application internals. + throw databaseSetupFailed(`Could not load the routes module ${label}.`); + } + if (typeof exported !== "object" || exported === null) { + throw databaseSetupFailed( + `The routes module ${label} must default-export a { "METHOD /path": operation } record.`, + ); + } + for (const [key, operation] of Object.entries(exported)) { + if (Object.hasOwn(routes, key)) { + throw databaseSetupFailed( + `Route ${JSON.stringify(key)} is declared more than once.`, + ); + } + routes[key] = operation; + } + } + return routes; +} diff --git a/packages/appkit/src/plugins/database/tests/operations.test.ts b/packages/appkit/src/plugins/database/tests/operations.test.ts new file mode 100644 index 000000000..159e34404 --- /dev/null +++ b/packages/appkit/src/plugins/database/tests/operations.test.ts @@ -0,0 +1,389 @@ +import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; + +import { createMockTelemetry, mockServiceContext } from "@tools/test-helpers"; +import type { Request, RequestHandler, Response } from "express"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +import { z } from "zod"; + +import type { DataPath } from "../../../database/runtime"; +import { defineSchema, id, text } from "../../../database/schema-builder"; +import type { ITelemetry } from "../../../telemetry"; +import type { DatabaseExports } from "../entity-types"; +import { + defineMutation, + definePolicy, + defineQuery, + loadDefaultDatabaseRoutes, + type OperationError, + operationErrorResponse, + runOperation, +} from "../operations"; +import type { IDatabaseConfig } from "../types"; + +const mocks = vi.hoisted(() => ({ + initializeLakebasePool: vi.fn(), + createDrizzleDb: vi.fn(), + createDrizzleDataPath: vi.fn(), +})); + +vi.mock("../../../connectors/lakebase", () => ({ + initializeLakebasePool: mocks.initializeLakebasePool, +})); +vi.mock("../../../database/runtime/engine/drizzle-data-path", () => ({ + createDrizzleDb: mocks.createDrizzleDb, + createDrizzleDataPath: mocks.createDrizzleDataPath, +})); +vi.mock("../schema-check", () => ({ + assertSchemaMatchesDatabase: async () => undefined, +})); + +import { DatabasePlugin } from "../database"; + +const alice = { userId: "alice" }; + +/** A transaction that restores the state it started from when its callback throws. */ +function fakeDb(state: { value: number }) { + const db = { + sql: vi.fn(async () => []), + transaction: async (callback: (tx: unknown) => Promise) => { + const before = state.value; + try { + return await callback(db); + } catch (error) { + state.value = before; + throw error; + } + }, + }; + return db as unknown as DatabaseExports & { sql: ReturnType }; +} + +async function failure(promise: Promise): Promise { + try { + await promise; + } catch (error) { + return error as OperationError; + } + throw new Error("expected the operation to fail"); +} + +describe("runOperation", () => { + const state = { value: 0 }; + const execute = vi.fn(async () => { + state.value += 1; + return { value: state.value }; + }); + const increment = ( + authorize: Parameters[0]["authorize"], + ) => + defineMutation({ input: z.object({ by: z.number() }), authorize, execute }); + + beforeEach(() => { + state.value = 0; + execute.mockClear(); + }); + + test("runs an authorized operation and returns its typed result", async () => { + const op = increment(() => true); + await expect( + runOperation(op, { actor: alice, input: { by: 1 } }, fakeDb(state)), + ).resolves.toEqual({ value: 1 }); + }); + + test("rejects a missing actor before validation or authorization", async () => { + const authorize = vi.fn(() => true); + const error = await failure( + runOperation( + increment(authorize), + { actor: undefined, input: { by: 1 } }, + fakeDb(state), + ), + ); + expect(error.statusCode).toBe(401); + expect(authorize).not.toHaveBeenCalled(); + expect(execute).not.toHaveBeenCalled(); + }); + + test("rejects invalid input before authorization or execution", async () => { + const authorize = vi.fn(() => true); + const error = await failure( + runOperation( + increment(authorize), + { actor: alice, input: { by: "1" } }, + fakeDb(state), + ), + ); + expect(error.statusCode).toBe(400); + expect(error.details?.[0].path).toBe("by"); + expect(authorize).not.toHaveBeenCalled(); + expect(execute).not.toHaveBeenCalled(); + }); + + test.each([ + [false, "Forbidden"], + [{ deny: "Not on this team" }, "Not on this team"], + ])("refuses with 403 when a policy returns %j", async (result, message) => { + const error = await failure( + runOperation( + increment(() => result), + { actor: alice, input: { by: 1 } }, + fakeDb(state), + ), + ); + expect([error.statusCode, error.clientMessage]).toEqual([403, message]); + expect(state.value).toBe(0); + }); + + test("requires every policy in an array to allow", async () => { + const second = vi.fn(() => true); + const error = await failure( + runOperation( + increment([() => ({ deny: "no" }), second]), + { actor: alice, input: { by: 1 } }, + fakeDb(state), + ), + ); + expect(error.statusCode).toBe(403); + expect(second).not.toHaveBeenCalled(); + }); + + test.each([ + ["a thrown lookup", () => Promise.reject(new Error("password=hunter2"))], + ["a forgotten return", () => undefined as never], + ["a truthy non-decision", () => ({ ok: true }) as never], + ])("treats %s as a 500, never an allow", async (_name, policy) => { + const error = await failure( + runOperation( + increment(policy), + { actor: alice, input: { by: 1 } }, + fakeDb(state), + ), + ); + expect([error.statusCode, error.clientMessage]).toEqual([ + 500, + "Authorization check failed", + ]); + expect(JSON.stringify(operationErrorResponse(error))).not.toContain( + "hunter2", + ); + expect(execute).not.toHaveBeenCalled(); + }); + + test("rolls back a mutation whose execution fails after writing", async () => { + const op = defineMutation({ + input: z.object({}), + authorize: () => true, + execute: async () => { + state.value = 99; + throw new Error("boom"); + }, + }); + await expect( + runOperation(op, { actor: alice, input: {} }, fakeDb(state)), + ).rejects.toThrow("boom"); + expect(state.value).toBe(0); + }); + + test("reports an unknown failure without its message", () => { + const response = operationErrorResponse( + new Error("connect postgres://sp:token@host failed"), + ); + expect(response).toEqual({ + status: 500, + body: { error: "Operation failed" }, + }); + }); + + test("refuses to define or run an operation without authorization", async () => { + expect(() => + defineQuery({ + input: z.object({}), + authorize: [], + execute: async () => 1, + }), + ).toThrow(/requires authorize/); + const forged = { + kind: "query", + input: z.object({}), + authorize: () => true, + execute: async () => 1, + } as never; + const error = await failure( + runOperation(forged, { actor: alice, input: {} }, fakeDb(state)), + ); + expect(error.statusCode).toBe(500); + }); + + test("definePolicy keeps the policy callable as-is", () => { + const policy = definePolicy<{ teamId: string }>(() => true); + expect( + policy({ actor: alice, input: { teamId: "x" }, db: {} as never }), + ).toBe(true); + }); +}); + +describe("loadDefaultDatabaseRoutes", () => { + let root: string; + beforeEach(async () => { + root = await mkdtemp(path.join(tmpdir(), "appkit-routes-")); + }); + afterEach(async () => { + await rm(root, { recursive: true, force: true }); + }); + + test("returns no routes when the folder is missing", async () => { + await expect(loadDefaultDatabaseRoutes(root)).resolves.toEqual({}); + }); + + test("merges default exports and rejects a repeated route", async () => { + const dir = path.join(root, "config/database/routes"); + await mkdir(dir, { recursive: true }); + await writeFile(path.join(dir, "a.ts"), 'export default { "GET /a": 1 };'); + await writeFile(path.join(dir, "b.ts"), 'export default { "GET /b": 2 };'); + await expect(loadDefaultDatabaseRoutes(root)).resolves.toEqual({ + "GET /a": 1, + "GET /b": 2, + }); + await writeFile(path.join(dir, "c.ts"), 'export default { "GET /a": 3 };'); + await expect(loadDefaultDatabaseRoutes(root)).rejects.toThrow( + /declared more than once/, + ); + }); +}); + +const schema = defineSchema((builder) => ({ + notes: builder.table("notes", { id: id(), body: text().notNull() }), +})); + +const seen = vi.fn(); +const echo = defineMutation({ + input: z.object({ id: z.string(), actorId: z.string().optional() }), + authorize: ({ actor }) => actor.userId === "alice" || { deny: "Only alice" }, + execute: async ({ actor, input }) => { + seen(actor, input); + return { id: input.id, by: actor.userId }; + }, +}); + +async function mount(config: Omit, "schema">) { + const path = { + raw: async () => [], + transaction: async (callback: (tx: DataPath) => Promise) => + callback(path), + } as unknown as DataPath; + mocks.initializeLakebasePool.mockResolvedValue({ + end: vi.fn(async () => undefined), + }); + mocks.createDrizzleDb.mockReturnValue({}); + mocks.createDrizzleDataPath.mockReturnValue(path); + const plugin = new DatabasePlugin({ schema, ...config }); + (plugin as unknown as { telemetry: ITelemetry }).telemetry = + createMockTelemetry(); + await plugin.setup(); + const handlers = new Map(); + const record = (method: string) => (route: string, handler: RequestHandler) => + handlers.set(`${method} ${route}`, handler); + plugin.injectRoutes({ + get: record("get"), + post: record("post"), + patch: record("patch"), + delete: record("delete"), + } as never); + return handlers; +} + +async function call( + handler: RequestHandler | undefined, + req: { headers?: Record; params?: object; body?: object }, +) { + const sent: { status?: number; body?: unknown } = {}; + const res = { + headersSent: false, + status(code: number) { + sent.status = code; + return res; + }, + setHeader: () => res, + json(body: unknown) { + sent.body = body; + return res; + }, + }; + const headers = req.headers ?? {}; + await (handler as unknown as (req: Request, res: Response) => Promise)( + { + params: req.params ?? {}, + body: req.body, + query: {}, + header: (name: string) => headers[name], + } as unknown as Request, + res as unknown as Response, + ); + return sent; +} + +describe("database routes", () => { + let context: Awaited>; + beforeEach(async () => { + vi.stubEnv("NODE_ENV", "production"); + seen.mockClear(); + context = await mockServiceContext(); + }); + afterEach(() => { + context.restore(); + vi.unstubAllEnvs(); + }); + + test("serves an operation with the actor from the proxy header, not the body", async () => { + const handlers = await mount({ + api: false, + routes: { "POST /notes/:id/echo": echo }, + }); + const sent = await call(handlers.get("post /notes/:id/echo"), { + headers: { "x-forwarded-user": "alice" }, + params: { id: "7" }, + // Neither a body id nor a body identity can replace the trusted ones. + body: { id: "8", userId: "mallory", actorId: "mallory" }, + }); + expect(sent).toEqual({ status: 200, body: { id: "7", by: "alice" } }); + }); + + test("answers 401 without the proxy header and 403 for a refused actor", async () => { + const handlers = await mount({ + api: false, + routes: { "POST /notes/:id/echo": echo }, + }); + const handler = handlers.get("post /notes/:id/echo"); + expect(await call(handler, { params: { id: "7" } })).toEqual({ + status: 401, + body: { error: "Authentication required" }, + }); + expect( + await call(handler, { + headers: { "x-forwarded-user": "bob" }, + params: { id: "7" }, + }), + ).toEqual({ status: 403, body: { error: "Only alice" } }); + expect(seen).not.toHaveBeenCalled(); + }); + + test("fails setup when a route overlaps generated CRUD", async () => { + await expect( + mount({ routes: { "GET /notes/:noteId": echo } }), + ).rejects.toThrow(/collides/); + }); + + test("fails setup for a malformed key or a value that is not an operation", async () => { + await expect( + mount({ api: false, routes: { "/notes": echo } }), + ).rejects.toThrow(/must look like/); + await expect( + mount({ + api: false, + routes: { "GET /x": { execute: () => 1 } as never }, + }), + ).rejects.toThrow(/must map to an operation/); + }); +}); diff --git a/packages/appkit/src/plugins/database/types.ts b/packages/appkit/src/plugins/database/types.ts index 80824ffe6..d93610f32 100644 --- a/packages/appkit/src/plugins/database/types.ts +++ b/packages/appkit/src/plugins/database/types.ts @@ -1,6 +1,7 @@ import type { DatabaseRegistry } from "../../database/contract"; import type { Schema } from "../../database/schema-builder"; import type { EntityMutationHooks } from "./hooks"; +import type { DatabaseRoutes } from "./operations"; type RegistryTableName = Extract; @@ -103,4 +104,15 @@ export type IDatabaseConfig = { readonly hooks?: { readonly [TTable in SchemaTableName]?: EntityHooks; }; + /** + * Operations exposed over HTTP under `/api/database`, keyed + * `"METHOD /path"`. Each runs through the same pipeline: the actor comes from + * the Apps proxy (`x-forwarded-user`), the input is validated, `authorize` + * must return `true`, then `execute` runs as the service principal. + * + * When omitted, setup merges the default exports of + * `config/database/routes/*.ts`. Passing `routes` disables that discovery. + * A route that overlaps another or a generated CRUD route fails setup. + */ + readonly routes?: DatabaseRoutes; }; From fa0f7178581524526fd6312e2f9ba3f237614bc6 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Wed, 21 Oct 2026 12:07:02 +0200 Subject: [PATCH 2/5] feat(playground): orders operations example orders.list and orders.approve under config/database/routes, with an example team_members permission table, setup SQL, and tests for team scoping, approval limits, non-pending state, and concurrent approval. Generated CRUD stays on for the note tables only. Co-authored-by: Isaac Signed-off-by: MarioCadenas --- .../dev-playground/config/database/orders.sql | 28 +++ .../config/database/policies.ts | 39 +++ .../config/database/routes/orders.ts | 48 ++++ apps/dev-playground/config/database/schema.ts | 24 +- apps/dev-playground/server/database.test.ts | 7 +- apps/dev-playground/server/index.ts | 11 +- .../server/orders-operations.test.ts | 237 ++++++++++++++++++ .../shared/appkit-types/database.d.ts | 128 ++++++++++ 8 files changed, 516 insertions(+), 6 deletions(-) create mode 100644 apps/dev-playground/config/database/orders.sql create mode 100644 apps/dev-playground/config/database/policies.ts create mode 100644 apps/dev-playground/config/database/routes/orders.ts create mode 100644 apps/dev-playground/server/orders-operations.test.ts diff --git a/apps/dev-playground/config/database/orders.sql b/apps/dev-playground/config/database/orders.sql new file mode 100644 index 000000000..69be255b6 --- /dev/null +++ b/apps/dev-playground/config/database/orders.sql @@ -0,0 +1,28 @@ +-- Tables and seed data for the operations example (config/database/routes/orders.ts). +create table if not exists orders ( + id serial primary key, + team_id text not null, + created_by text not null, + status varchar(16) not null, + total_cents integer not null, + approved_by text +); + +create table if not exists team_members ( + id serial primary key, + user_id text not null, + team_id text not null, + role varchar(16) not null, + approval_limit_cents integer not null, + unique (user_id, team_id) +); + +insert into team_members (user_id, team_id, role, approval_limit_cents) values + ('alice', 'red', 'approver', 100000), + ('bob', 'red', 'member', 0), + ('carol', 'blue', 'approver', 100000); + +insert into orders (team_id, created_by, status, total_cents) values + ('red', 'bob', 'pending', 25000), + ('red', 'bob', 'pending', 500000), + ('blue', 'carol', 'pending', 9900); diff --git a/apps/dev-playground/config/database/policies.ts b/apps/dev-playground/config/database/policies.ts new file mode 100644 index 000000000..7329edea2 --- /dev/null +++ b/apps/dev-playground/config/database/policies.ts @@ -0,0 +1,39 @@ +import { definePolicy } from "@databricks/appkit/beta"; + +/** Listing one team requires membership; listing without a team is scoped in the query. */ +export const canReadTeam = definePolicy<{ teamId?: string }>( + async ({ actor, input, db }) => { + if (input.teamId === undefined) return true; + const rows = await db.sql` + select 1 from team_members + where user_id = ${actor.userId} and team_id = ${input.teamId}`; + return rows.length > 0 || { deny: "Not a member of this team" }; + }, +); + +/** An approver of the order's team, within their limit, who did not create it. */ +export const canApproveOrder = definePolicy<{ id: number }>( + async ({ actor, input, db }) => { + const [row] = await db.sql<{ + created_by: string; + total_cents: number; + role: string | null; + approval_limit_cents: number | null; + }>` + select o.created_by, o.total_cents, m.role, m.approval_limit_cents + from orders o + left join team_members m + on m.team_id = o.team_id and m.user_id = ${actor.userId} + where o.id = ${input.id}`; + // Same answer for "missing" and "another team's": existence is not leaked. + if (!row?.role) return { deny: "Order not found" }; + if (row.role !== "approver") return { deny: "Approver role required" }; + if (row.created_by === actor.userId) { + return { deny: "You cannot approve your own order" }; + } + if (row.total_cents > (row.approval_limit_cents ?? 0)) { + return { deny: "Order total exceeds your approval limit" }; + } + return true; + }, +); diff --git a/apps/dev-playground/config/database/routes/orders.ts b/apps/dev-playground/config/database/routes/orders.ts new file mode 100644 index 000000000..016df2168 --- /dev/null +++ b/apps/dev-playground/config/database/routes/orders.ts @@ -0,0 +1,48 @@ +import { + defineMutation, + defineQuery, + OperationError, +} from "@databricks/appkit/beta"; +import { z } from "zod"; + +import { canApproveOrder, canReadTeam } from "../policies"; + +interface Order { + id: number; + team_id: string; + created_by: string; + status: string; + total_cents: number; + approved_by: string | null; +} + +export const listOrders = defineQuery({ + input: z.object({ teamId: z.string().min(1).optional() }), + authorize: canReadTeam, + // The membership join is the access rule: rows of other teams are never read. + execute: ({ actor, input, db }) => db.sql` + select o.* from orders o + join team_members m on m.team_id = o.team_id and m.user_id = ${actor.userId} + where ${input.teamId ?? null}::text is null or o.team_id = ${input.teamId ?? null} + order by o.id`, +}); + +export const approveOrder = defineMutation({ + input: z.object({ id: z.coerce.number().int().positive() }), + authorize: canApproveOrder, + execute: async ({ actor, input, db }) => { + // The state check is part of the write: of two concurrent approvals, the + // second re-reads the committed row, matches nothing, and gets a 409. + const [order] = await db.sql` + update orders set status = 'approved', approved_by = ${actor.userId} + where id = ${input.id} and status = 'pending' + returning *`; + if (!order) throw new OperationError(409, "Order is not pending"); + return order; + }, +}); + +export default { + "GET /orders": listOrders, + "POST /orders/:id/approve": approveOrder, +}; diff --git a/apps/dev-playground/config/database/schema.ts b/apps/dev-playground/config/database/schema.ts index 2bc9a0126..b7aa68c5e 100644 --- a/apps/dev-playground/config/database/schema.ts +++ b/apps/dev-playground/config/database/schema.ts @@ -6,6 +6,7 @@ import { defineSchema, fk, id, + integer, text, timestamp, varchar, @@ -40,5 +41,26 @@ export const schema = defineSchema(({ table }) => { created_at: timestamp({ withTimezone: true }).defaultNow().notNull(), }); - return { boards, notes, note_events }; + // Operations example (config/database/routes/orders.ts). Exposed only through + // operations, never generated CRUD; see orders.sql for the tables and seed. + const orders = table("orders", { + id: id(), + team_id: text().notNull(), + created_by: text().notNull(), + status: varchar(16).notNull(), + total_cents: integer().notNull(), + approved_by: text(), + }); + + // Example permission data. AppKit has no team or role source; an app owns + // whatever its policies read. + const team_members = table("team_members", { + id: id(), + user_id: text().notNull(), + team_id: text().notNull(), + role: varchar(16).notNull(), + approval_limit_cents: integer().notNull(), + }); + + return { boards, notes, note_events, orders, team_members }; }); diff --git a/apps/dev-playground/server/database.test.ts b/apps/dev-playground/server/database.test.ts index 2840d54b2..ada5764cd 100644 --- a/apps/dev-playground/server/database.test.ts +++ b/apps/dev-playground/server/database.test.ts @@ -86,11 +86,13 @@ afterEach(() => { const beforeCreate = () => hooks.beforeCreate(values, context); describe("inline playground database registration", () => { - test("declares three tables in the conventional schema file", () => { + test("declares the note and order tables in the conventional schema file", () => { expect(Object.keys(schema.$tables)).toEqual([ "boards", "notes", "note_events", + "orders", + "team_members", ]); }); @@ -98,8 +100,9 @@ describe("inline playground database registration", () => { expect(databaseConfig.schema).toBeUndefined(); }); - test("enables reads for all tables and HTTP writes only for boards and notes", () => { + test("generates CRUD only for the note tables and writes only for boards and notes", () => { expect(databaseConfig.api).toEqual({ + tables: ["boards", "notes", "note_events"], writes: { tables: ["boards", "notes"] }, }); }); diff --git a/apps/dev-playground/server/index.ts b/apps/dev-playground/server/index.ts index f2c22a7ca..9abd82be7 100644 --- a/apps/dev-playground/server/index.ts +++ b/apps/dev-playground/server/index.ts @@ -76,9 +76,14 @@ createApp({ ? [ // The schema is loaded from config/database/schema.ts. database({ - // Reads are generated for all three tables. Only boards and notes - // accept HTTP writes; the audit trail is written by the hook. - api: { writes: { tables: ["boards", "notes"] } }, + // Reads are generated for the three note tables. Only boards and + // notes accept HTTP writes; the audit trail is written by the hook. + // orders and team_members get no generated routes: they are reached + // only through the authorized operations in config/database/routes. + api: { + tables: ["boards", "notes", "note_events"], + writes: { tables: ["boards", "notes"] }, + }, // Example of Hooks integration between Database and Agents hooks: { // Hook per entity diff --git a/apps/dev-playground/server/orders-operations.test.ts b/apps/dev-playground/server/orders-operations.test.ts new file mode 100644 index 000000000..6537a6b96 --- /dev/null +++ b/apps/dev-playground/server/orders-operations.test.ts @@ -0,0 +1,237 @@ +import { runOperation } from "@databricks/appkit/beta"; +import { beforeEach, describe, expect, test } from "vitest"; + +import { approveOrder, listOrders } from "../config/database/routes/orders"; + +interface Order { + id: number; + team_id: string; + created_by: string; + status: string; + total_cents: number; + approved_by: string | null; +} + +const members = [ + { + user_id: "alice", + team_id: "red", + role: "approver", + approval_limit_cents: 100_000, + }, + { user_id: "bob", team_id: "red", role: "member", approval_limit_cents: 0 }, + { + user_id: "carol", + team_id: "blue", + role: "approver", + approval_limit_cents: 100_000, + }, +]; + +let orders: Order[]; + +/** + * Answers the example's four statements from memory. The conditional update + * checks and writes in one synchronous step, as Postgres does under its row + * lock, so two concurrent approvals cannot both match `status = 'pending'`. + */ +function fakeDb() { + const sql = async (strings: TemplateStringsArray, ...values: unknown[]) => { + const text = strings.join("?"); + await Promise.resolve(); + if (text.includes("select 1 from team_members")) { + const [user, team] = values; + return members.filter((m) => m.user_id === user && m.team_id === team); + } + if (text.includes("select o.* from orders o")) { + const [user, team] = values; + const teams = members + .filter((m) => m.user_id === user) + .map((m) => m.team_id); + return orders.filter( + (o) => + teams.includes(o.team_id) && (team === null || o.team_id === team), + ); + } + if (text.includes("select o.created_by")) { + const [user, id] = values; + const order = orders.find((o) => o.id === id); + if (!order) return []; + const member = members.find( + (m) => m.team_id === order.team_id && m.user_id === user, + ); + return [ + { + created_by: order.created_by, + total_cents: order.total_cents, + role: member?.role ?? null, + approval_limit_cents: member?.approval_limit_cents ?? null, + }, + ]; + } + if (text.includes("update orders set status")) { + const [user, id] = values; + const order = orders.find((o) => o.id === id && o.status === "pending"); + if (!order) return []; + Object.assign(order, { status: "approved", approved_by: user }); + return [{ ...order }]; + } + throw new Error(`unexpected statement: ${text}`); + }; + const db = { + sql, + transaction: async (callback: (tx: unknown) => Promise) => { + const snapshot = structuredClone(orders); + try { + return await callback(db); + } catch (error) { + orders = snapshot; + throw error; + } + }, + }; + return db as unknown as Parameters[2]; +} + +const as = (userId: string) => ({ userId }); + +async function statusOf(promise: Promise): Promise { + try { + await promise; + return 200; + } catch (error) { + return (error as { statusCode: number }).statusCode; + } +} + +beforeEach(() => { + orders = [ + { + id: 1, + team_id: "red", + created_by: "bob", + status: "pending", + total_cents: 25_000, + approved_by: null, + }, + { + id: 2, + team_id: "red", + created_by: "bob", + status: "pending", + total_cents: 500_000, + approved_by: null, + }, + { + id: 3, + team_id: "blue", + created_by: "carol", + status: "pending", + total_cents: 9_900, + approved_by: null, + }, + ]; +}); + +describe("orders.list", () => { + test("returns only the caller's teams, filtered in the query", async () => { + const rows = await runOperation( + listOrders, + { actor: as("bob"), input: {} }, + fakeDb(), + ); + expect(rows.map((o) => o.id)).toEqual([1, 2]); + }); + + test("denies another team's orders when the caller changes teamId", async () => { + expect( + await statusOf( + runOperation( + listOrders, + { actor: as("bob"), input: { teamId: "blue" } }, + fakeDb(), + ), + ), + ).toBe(403); + }); +}); + +describe("orders.approve", () => { + test("approves a pending order within the approver's limit", async () => { + const order = await runOperation( + approveOrder, + { actor: as("alice"), input: { id: "1" } }, + fakeDb(), + ); + expect(order).toMatchObject({ + id: 1, + status: "approved", + approved_by: "alice", + }); + }); + + test.each([ + ["a member without the approver role", "bob", 1], + ["an approver over their limit", "alice", 2], + ["an approver of another team", "carol", 1], + ["the order's own creator", "carol", 3], + ])("denies %s and leaves the order pending", async (_name, user, id) => { + expect( + await statusOf( + runOperation( + approveOrder, + { actor: as(user), input: { id } }, + fakeDb(), + ), + ), + ).toBe(403); + expect(orders.find((o) => o.id === id)?.status).toBe("pending"); + }); + + test("refuses an order that is no longer pending", async () => { + orders[0].status = "rejected"; + expect( + await statusOf( + runOperation( + approveOrder, + { actor: as("alice"), input: { id: 1 } }, + fakeDb(), + ), + ), + ).toBe(409); + expect(orders[0]).toMatchObject({ status: "rejected", approved_by: null }); + }); + + test("lets exactly one of two concurrent approvals win", async () => { + const db = fakeDb(); + const statuses = await Promise.all([ + statusOf( + runOperation( + approveOrder, + { actor: as("alice"), input: { id: 1 } }, + db, + ), + ), + statusOf( + runOperation( + approveOrder, + { actor: as("alice"), input: { id: 1 } }, + db, + ), + ), + ]); + expect(statuses.sort()).toEqual([200, 409]); + }); + + test("rejects a non-numeric id before any statement runs", async () => { + expect( + await statusOf( + runOperation( + approveOrder, + { actor: as("alice"), input: { id: "1; drop table" } }, + fakeDb(), + ), + ), + ).toBe(400); + }); +}); diff --git a/apps/dev-playground/shared/appkit-types/database.d.ts b/apps/dev-playground/shared/appkit-types/database.d.ts index a388108d0..68d0b5fc0 100644 --- a/apps/dev-playground/shared/appkit-types/database.d.ts +++ b/apps/dev-playground/shared/appkit-types/database.d.ts @@ -179,6 +179,134 @@ interface GeneratedDatabaseRegistry { key: "id"; }; }; + "orders": { + row: { + "id": number; + "team_id": string; + "created_by": string; + "status": string; + "total_cents": number; + "approved_by": string | null; + }; + publicRow: { + "id": number; + "team_id": string; + "created_by": string; + "status": string; + "total_cents": number; + "approved_by": string | null; + }; + insert: { + "team_id": string; + "created_by": string; + "status": string; + "total_cents": number; + "approved_by"?: string | null; + }; + update: { + "team_id"?: string; + "created_by"?: string; + "status"?: string; + "total_cents"?: number; + "approved_by"?: string | null; + }; + filters: DatabaseLogicalFilter<{ + "id"?: number | readonly (number)[] | { eq?: number; neq?: number; in?: readonly (number)[]; gt?: number; gte?: number; lt?: number; lte?: number; }; + "team_id"?: string | readonly (string)[] | { eq?: string; neq?: string; in?: readonly (string)[]; like?: string; ilike?: string; }; + "created_by"?: string | readonly (string)[] | { eq?: string; neq?: string; in?: readonly (string)[]; like?: string; ilike?: string; }; + "status"?: string | readonly (string)[] | { eq?: string; neq?: string; in?: readonly (string)[]; like?: string; ilike?: string; }; + "total_cents"?: number | readonly (number)[] | { eq?: number; neq?: number; in?: readonly (number)[]; gt?: number; gte?: number; lt?: number; lte?: number; }; + "approved_by"?: string | readonly (string)[] | { eq?: string; neq?: string; in?: readonly (string)[]; like?: string; ilike?: string; is?: null; }; + }>; + includes: {}; + hasPrimaryKey: true; + api: { + insert: { + "team_id": string; + "created_by": string; + "status": string; + "total_cents": number; + "approved_by"?: string | null; + }; + update: { + "team_id"?: string; + "created_by"?: string; + "status"?: string; + "total_cents"?: number; + "approved_by"?: string | null; + }; + filters: DatabaseLogicalFilter<{ + "id"?: number | { eq?: number; neq?: number; in?: readonly (number)[]; gt?: number; gte?: number; lt?: number; lte?: number; }; + "team_id"?: string | { eq?: string; neq?: string; in?: readonly (string)[]; like?: string; ilike?: string; }; + "created_by"?: string | { eq?: string; neq?: string; in?: readonly (string)[]; like?: string; ilike?: string; }; + "status"?: string | { eq?: string; neq?: string; in?: readonly (string)[]; like?: string; ilike?: string; }; + "total_cents"?: number | { eq?: number; neq?: number; in?: readonly (number)[]; gt?: number; gte?: number; lt?: number; lte?: number; }; + "approved_by"?: string | { eq?: string; neq?: string; in?: readonly (string)[]; like?: string; ilike?: string; is?: null; }; + }>; + orderable: "id" | "team_id" | "created_by" | "status" | "total_cents" | "approved_by"; + key: "id"; + }; + }; + "team_members": { + row: { + "id": number; + "user_id": string; + "team_id": string; + "role": string; + "approval_limit_cents": number; + }; + publicRow: { + "id": number; + "user_id": string; + "team_id": string; + "role": string; + "approval_limit_cents": number; + }; + insert: { + "user_id": string; + "team_id": string; + "role": string; + "approval_limit_cents": number; + }; + update: { + "user_id"?: string; + "team_id"?: string; + "role"?: string; + "approval_limit_cents"?: number; + }; + filters: DatabaseLogicalFilter<{ + "id"?: number | readonly (number)[] | { eq?: number; neq?: number; in?: readonly (number)[]; gt?: number; gte?: number; lt?: number; lte?: number; }; + "user_id"?: string | readonly (string)[] | { eq?: string; neq?: string; in?: readonly (string)[]; like?: string; ilike?: string; }; + "team_id"?: string | readonly (string)[] | { eq?: string; neq?: string; in?: readonly (string)[]; like?: string; ilike?: string; }; + "role"?: string | readonly (string)[] | { eq?: string; neq?: string; in?: readonly (string)[]; like?: string; ilike?: string; }; + "approval_limit_cents"?: number | readonly (number)[] | { eq?: number; neq?: number; in?: readonly (number)[]; gt?: number; gte?: number; lt?: number; lte?: number; }; + }>; + includes: {}; + hasPrimaryKey: true; + api: { + insert: { + "user_id": string; + "team_id": string; + "role": string; + "approval_limit_cents": number; + }; + update: { + "user_id"?: string; + "team_id"?: string; + "role"?: string; + "approval_limit_cents"?: number; + }; + filters: DatabaseLogicalFilter<{ + "id"?: number | { eq?: number; neq?: number; in?: readonly (number)[]; gt?: number; gte?: number; lt?: number; lte?: number; }; + "user_id"?: string | { eq?: string; neq?: string; in?: readonly (string)[]; like?: string; ilike?: string; }; + "team_id"?: string | { eq?: string; neq?: string; in?: readonly (string)[]; like?: string; ilike?: string; }; + "role"?: string | { eq?: string; neq?: string; in?: readonly (string)[]; like?: string; ilike?: string; }; + "approval_limit_cents"?: number | { eq?: number; neq?: number; in?: readonly (number)[]; gt?: number; gte?: number; lt?: number; lte?: number; }; + }>; + orderable: "id" | "user_id" | "team_id" | "role" | "approval_limit_cents"; + key: "id"; + }; + }; } declare module "@databricks/appkit" { From 86ea1d631d948834df3bd65cfb6ac9d67b199d16 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Wed, 21 Oct 2026 13:40:22 +0200 Subject: [PATCH 3/5] fix(appkit): detect route collisions the way Express matches routes Letter case and a trailing slash no longer tell two routes apart, so GET /Notes or GET /notes/ can't shadow generated CRUD or each other. Co-authored-by: Isaac Signed-off-by: MarioCadenas --- .../appkit/src/plugins/database/operations.ts | 12 ++++++++++-- .../plugins/database/tests/operations.test.ts | 16 ++++++++++++++-- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/packages/appkit/src/plugins/database/operations.ts b/packages/appkit/src/plugins/database/operations.ts index c495170be..3aa31b5c5 100644 --- a/packages/appkit/src/plugins/database/operations.ts +++ b/packages/appkit/src/plugins/database/operations.ts @@ -233,9 +233,17 @@ export function parseRouteKey(key: string): { return { method, path: match[2] }; } -/** Identity of a route for collision checks: parameter names do not matter to Express. */ +/** + * Identity of a route for collision checks, matching Express's defaults: + * parameter names, letter case, and a trailing slash do not tell routes apart. + */ export function routeSignature(method: string, routePath: string): string { - return `${method.toLowerCase()} ${routePath.replace(/:[^/]+/g, ":")}`; + const path = + routePath + .replace(/:[^/]+/g, ":") + .toLowerCase() + .replace(/\/+$/, "") || "/"; + return `${method.toLowerCase()} ${path}`; } /** Same application-relative convention as `config/database/schema.ts`. */ diff --git a/packages/appkit/src/plugins/database/tests/operations.test.ts b/packages/appkit/src/plugins/database/tests/operations.test.ts index 159e34404..b0988b89d 100644 --- a/packages/appkit/src/plugins/database/tests/operations.test.ts +++ b/packages/appkit/src/plugins/database/tests/operations.test.ts @@ -369,9 +369,21 @@ describe("database routes", () => { expect(seen).not.toHaveBeenCalled(); }); - test("fails setup when a route overlaps generated CRUD", async () => { + test.each(["GET /notes/:noteId", "GET /Notes", "GET /notes/"])( + "fails setup when %s overlaps generated CRUD", + async (key) => { + await expect(mount({ routes: { [key]: echo } })).rejects.toThrow( + /collides/, + ); + }, + ); + + test("fails setup for routes Express treats as one", async () => { await expect( - mount({ routes: { "GET /notes/:noteId": echo } }), + mount({ + api: false, + routes: { "GET /status": echo, "GET /STATUS/": echo }, + }), ).rejects.toThrow(/collides/); }); From f575870e1932cb131a902d3504baff0b2dbaf231 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Wed, 21 Oct 2026 13:40:30 +0200 Subject: [PATCH 4/5] fix(playground): lock the facts an order approval is authorized on canApproveOrder now reads the order FOR UPDATE and the membership FOR SHARE inside the mutation's transaction, so total, team, creator and role can't change between the decision and the commit. The fake transaction undoes only its own writes, and the concurrency test asserts the winning approval persists. Co-authored-by: Isaac Signed-off-by: MarioCadenas --- .../config/database/policies.ts | 34 +++-- .../config/database/routes/orders.ts | 4 +- .../server/orders-operations.test.ts | 120 +++++++++++------- 3 files changed, 94 insertions(+), 64 deletions(-) diff --git a/apps/dev-playground/config/database/policies.ts b/apps/dev-playground/config/database/policies.ts index 7329edea2..d33eae143 100644 --- a/apps/dev-playground/config/database/policies.ts +++ b/apps/dev-playground/config/database/policies.ts @@ -11,27 +11,35 @@ export const canReadTeam = definePolicy<{ teamId?: string }>( }, ); -/** An approver of the order's team, within their limit, who did not create it. */ +/** + * An approver of the order's team, within their limit, who did not create it. + * Runs inside the mutation's transaction, and locks what it reads: `for update` + * pins the order's team, creator and total, `for share` pins the membership, + * so neither can change between this decision and the approval's commit. + */ export const canApproveOrder = definePolicy<{ id: number }>( async ({ actor, input, db }) => { - const [row] = await db.sql<{ + const [order] = await db.sql<{ + team_id: string; created_by: string; total_cents: number; - role: string | null; - approval_limit_cents: number | null; }>` - select o.created_by, o.total_cents, m.role, m.approval_limit_cents - from orders o - left join team_members m - on m.team_id = o.team_id and m.user_id = ${actor.userId} - where o.id = ${input.id}`; + select team_id, created_by, total_cents from orders + where id = ${input.id} + for update`; + const [member] = order + ? await db.sql<{ role: string; approval_limit_cents: number }>` + select role, approval_limit_cents from team_members + where user_id = ${actor.userId} and team_id = ${order.team_id} + for share` + : []; // Same answer for "missing" and "another team's": existence is not leaked. - if (!row?.role) return { deny: "Order not found" }; - if (row.role !== "approver") return { deny: "Approver role required" }; - if (row.created_by === actor.userId) { + if (!order || !member) return { deny: "Order not found" }; + if (member.role !== "approver") return { deny: "Approver role required" }; + if (order.created_by === actor.userId) { return { deny: "You cannot approve your own order" }; } - if (row.total_cents > (row.approval_limit_cents ?? 0)) { + if (order.total_cents > member.approval_limit_cents) { return { deny: "Order total exceeds your approval limit" }; } return true; diff --git a/apps/dev-playground/config/database/routes/orders.ts b/apps/dev-playground/config/database/routes/orders.ts index 016df2168..a00a0d913 100644 --- a/apps/dev-playground/config/database/routes/orders.ts +++ b/apps/dev-playground/config/database/routes/orders.ts @@ -31,8 +31,8 @@ export const approveOrder = defineMutation({ input: z.object({ id: z.coerce.number().int().positive() }), authorize: canApproveOrder, execute: async ({ actor, input, db }) => { - // The state check is part of the write: of two concurrent approvals, the - // second re-reads the committed row, matches nothing, and gets a 409. + // canApproveOrder holds the order's row lock, so a concurrent approval + // waits for this commit, then finds the order no longer pending: a 409. const [order] = await db.sql` update orders set status = 'approved', approved_by = ${actor.userId} where id = ${input.id} and status = 'pending' diff --git a/apps/dev-playground/server/orders-operations.test.ts b/apps/dev-playground/server/orders-operations.test.ts index 6537a6b96..6bf77c03b 100644 --- a/apps/dev-playground/server/orders-operations.test.ts +++ b/apps/dev-playground/server/orders-operations.test.ts @@ -30,62 +30,64 @@ const members = [ let orders: Order[]; +/** Every statement the fake answered, to check what the policies lock. */ +let statements: string[]; + /** - * Answers the example's four statements from memory. The conditional update - * checks and writes in one synchronous step, as Postgres does under its row - * lock, so two concurrent approvals cannot both match `status = 'pending'`. + * Answers the example's statements from memory. The conditional update checks + * and writes in one synchronous step, as Postgres does under its row lock, so + * two concurrent approvals cannot both match `status = 'pending'`. Each + * transaction undoes only its own writes, so one rollback cannot erase + * another's commit. */ function fakeDb() { - const sql = async (strings: TemplateStringsArray, ...values: unknown[]) => { - const text = strings.join("?"); - await Promise.resolve(); - if (text.includes("select 1 from team_members")) { - const [user, team] = values; - return members.filter((m) => m.user_id === user && m.team_id === team); - } - if (text.includes("select o.* from orders o")) { - const [user, team] = values; - const teams = members - .filter((m) => m.user_id === user) - .map((m) => m.team_id); - return orders.filter( - (o) => - teams.includes(o.team_id) && (team === null || o.team_id === team), - ); - } - if (text.includes("select o.created_by")) { - const [user, id] = values; - const order = orders.find((o) => o.id === id); - if (!order) return []; - const member = members.find( - (m) => m.team_id === order.team_id && m.user_id === user, - ); - return [ - { - created_by: order.created_by, - total_cents: order.total_cents, - role: member?.role ?? null, - approval_limit_cents: member?.approval_limit_cents ?? null, - }, - ]; - } - if (text.includes("update orders set status")) { - const [user, id] = values; - const order = orders.find((o) => o.id === id && o.status === "pending"); - if (!order) return []; - Object.assign(order, { status: "approved", approved_by: user }); - return [{ ...order }]; - } - throw new Error(`unexpected statement: ${text}`); - }; + const sqlFor = + (undo?: (() => void)[]) => + async (strings: TemplateStringsArray, ...values: unknown[]) => { + const text = strings.join("?"); + statements.push(text); + await Promise.resolve(); + if (text.includes("select 1 from team_members")) { + const [user, team] = values; + return members.filter((m) => m.user_id === user && m.team_id === team); + } + if (text.includes("select o.* from orders o")) { + const [user, team] = values; + const teams = members + .filter((m) => m.user_id === user) + .map((m) => m.team_id); + return orders.filter( + (o) => + teams.includes(o.team_id) && (team === null || o.team_id === team), + ); + } + if (text.includes("select team_id, created_by, total_cents")) { + const [id] = values; + return orders.filter((o) => o.id === id).map((o) => ({ ...o })); + } + if (text.includes("select role, approval_limit_cents")) { + const [user, team] = values; + return members.filter((m) => m.user_id === user && m.team_id === team); + } + if (text.includes("update orders set status")) { + const [user, id] = values; + const order = orders.find((o) => o.id === id && o.status === "pending"); + if (!order) return []; + const before = { ...order }; + Object.assign(order, { status: "approved", approved_by: user }); + undo?.push(() => Object.assign(order, before)); + return [{ ...order }]; + } + throw new Error(`unexpected statement: ${text}`); + }; const db = { - sql, + sql: sqlFor(), transaction: async (callback: (tx: unknown) => Promise) => { - const snapshot = structuredClone(orders); + const undo: (() => void)[] = []; try { - return await callback(db); + return await callback({ sql: sqlFor(undo) }); } catch (error) { - orders = snapshot; + for (const step of undo.reverse()) step(); throw error; } }, @@ -105,6 +107,7 @@ async function statusOf(promise: Promise): Promise { } beforeEach(() => { + statements = []; orders = [ { id: 1, @@ -221,6 +224,25 @@ describe("orders.approve", () => { ), ]); expect(statuses.sort()).toEqual([200, 409]); + // The losing call's rollback must not take the winner's write with it. + expect(orders[0]).toMatchObject({ + status: "approved", + approved_by: "alice", + }); + }); + + test("locks the order and the membership it authorizes against", async () => { + await runOperation( + approveOrder, + { actor: as("alice"), input: { id: 1 } }, + fakeDb(), + ); + expect(statements.find((s) => s.includes("from orders"))).toMatch( + /for update\s*$/, + ); + expect(statements.find((s) => s.includes("from team_members"))).toMatch( + /for share\s*$/, + ); }); test("rejects a non-numeric id before any statement runs", async () => { From 764bcaeda45decba89e3250f2f7cdc31d68edda5 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Wed, 21 Oct 2026 14:36:35 +0200 Subject: [PATCH 5/5] fix(appkit): keep hook validation and dev-loaded operations working in routes Map a DatabaseValidationError from an operation to 422 like generated CRUD instead of a generic 500. Recognize operations by a registry symbol and OperationError by shape, so route files that jiti loads with their own module copy (development export) still run. Co-authored-by: Isaac Signed-off-by: MarioCadenas --- .../appkit/src/plugins/database/lifecycle.ts | 7 +- .../appkit/src/plugins/database/operations.ts | 76 ++++++++++++++++--- .../plugins/database/tests/operations.test.ts | 76 +++++++++++++++++++ 3 files changed, 142 insertions(+), 17 deletions(-) diff --git a/packages/appkit/src/plugins/database/lifecycle.ts b/packages/appkit/src/plugins/database/lifecycle.ts index b02135532..743a17af7 100644 --- a/packages/appkit/src/plugins/database/lifecycle.ts +++ b/packages/appkit/src/plugins/database/lifecycle.ts @@ -23,7 +23,7 @@ import type { SqlTag, TransactionClient, } from "./entity-types"; -import { OperationError } from "./operations"; +import { isOperationError } from "./operations"; import { assertSchemaMatchesDatabase } from "./schema-check"; import { createMutationScope, type MutationScope } from "./scope"; import type { DatabaseHooks } from "./types"; @@ -157,10 +157,7 @@ function buildDatabaseExports(context: ExportContext): DatabaseExports { return await runTransaction(context, callback); } catch (error) { // Both carry a message written for the caller, not driver detail. - if ( - error instanceof DatabaseValidationError || - error instanceof OperationError - ) { + if (error instanceof DatabaseValidationError || isOperationError(error)) { throw error; } throw classifyDatabaseError(error, "transaction"); diff --git a/packages/appkit/src/plugins/database/operations.ts b/packages/appkit/src/plugins/database/operations.ts index 3aa31b5c5..05f1fe13d 100644 --- a/packages/appkit/src/plugins/database/operations.ts +++ b/packages/appkit/src/plugins/database/operations.ts @@ -54,25 +54,31 @@ export type AnyOperation = Operation; /** Operations exposed over HTTP, keyed `"METHOD /path"` under `/api/database`. */ export type DatabaseRoutes = Readonly>; -/** Only objects built by the define helpers are runnable, so `authorize` was checked. */ -const defined = new WeakSet(); +/** + * Marks objects built by the define helpers. A registry symbol, not a + * module-local WeakSet: a dev loader can evaluate this module twice, and an + * operation from one copy must still run in the other. + */ +const OPERATION = Symbol.for("@databricks/appkit/database.operation"); + +function hasPolicies(authorize: unknown): boolean { + const policies = [authorize].flat(); + return ( + policies.length > 0 && + policies.every((policy) => typeof policy === "function") + ); +} function defineOperation( kind: Operation["kind"], definition: OperationDefinition, ): Operation { - const policies = [definition.authorize].flat(); - if ( - policies.length === 0 || - policies.some((policy) => typeof policy !== "function") - ) { + if (!hasPolicies(definition.authorize)) { throw databaseSetupFailed( "An operation requires authorize: a policy function or a non-empty array of them.", ); } - const operation = Object.freeze({ ...definition, kind }); - defined.add(operation); - return operation; + return Object.freeze({ ...definition, kind, [OPERATION]: true }); } /** A read. Runs `authorize` then `execute` outside a transaction. */ @@ -94,8 +100,16 @@ export function definePolicy(policy: Policy): Policy { return policy; } +/** The mark alone is not trusted: the policies are checked again here. */ export function isOperation(value: unknown): value is AnyOperation { - return typeof value === "object" && value !== null && defined.has(value); + if (typeof value !== "object" || value === null) return false; + const candidate = value as Record; + return ( + candidate[OPERATION] === true && + (candidate.kind === "query" || candidate.kind === "mutation") && + typeof candidate.execute === "function" && + hasPolicies(candidate.authorize) + ); } /** A failure whose message is written for the caller: denial, bad input, or a business conflict. */ @@ -112,6 +126,31 @@ export class OperationError extends AppKitError { } } +const OPERATION_STATUSES = new Set([400, 401, 403, 404, 409, 500]); + +/** By shape, not `instanceof`: route files may load another copy of this module. */ +export function isOperationError(error: unknown): error is OperationError { + if (!(error instanceof Error)) return false; + const candidate = error as Partial; + return ( + candidate.code === "OPERATION_ERROR" && + OPERATION_STATUSES.has(candidate.statusCode as number) && + typeof candidate.clientMessage === "string" + ); +} + +/** A hook's deliberate rejection, by shape for the same reason. */ +function isValidationError(error: unknown): error is Error & { + clientMessage: string; + issues: readonly { path: readonly unknown[]; message: string }[]; +} { + return ( + error instanceof Error && + (error as { code?: unknown }).code === "DATABASE_VALIDATION_ERROR" && + Array.isArray((error as { issues?: unknown }).issues) + ); +} + const AUTHORIZATION_FAILED = "Authorization check failed"; /** Every policy must return exactly `true`; anything else is a refusal or an error. */ @@ -188,7 +227,7 @@ export function operationErrorResponse(error: unknown): { status: number; body: { error: string; details?: readonly unknown[] }; } { - if (error instanceof OperationError) { + if (isOperationError(error)) { return { status: error.statusCode, body: error.details @@ -196,6 +235,19 @@ export function operationErrorResponse(error: unknown): { : { error: error.clientMessage }, }; } + if (isValidationError(error)) { + // Hook-authored issues, the same signal generated CRUD answers with 422. + const details = error.issues + .filter((issue) => issue.path.length > 0) + .map((issue) => ({ path: issue.path.join("."), message: issue.message })); + return { + status: 422, + body: + details.length > 0 + ? { error: error.clientMessage, details } + : { error: error.clientMessage }, + }; + } if (error instanceof DatabasePluginError) { return { status: error.statusCode, body: { error: error.clientMessage } }; } diff --git a/packages/appkit/src/plugins/database/tests/operations.test.ts b/packages/appkit/src/plugins/database/tests/operations.test.ts index b0988b89d..a2c75d255 100644 --- a/packages/appkit/src/plugins/database/tests/operations.test.ts +++ b/packages/appkit/src/plugins/database/tests/operations.test.ts @@ -9,6 +9,7 @@ import { z } from "zod"; import type { DataPath } from "../../../database/runtime"; import { defineSchema, id, text } from "../../../database/schema-builder"; +import { DatabaseValidationError } from "../../../errors"; import type { ITelemetry } from "../../../telemetry"; import type { DatabaseExports } from "../entity-types"; import { @@ -17,6 +18,7 @@ import { defineQuery, loadDefaultDatabaseRoutes, type OperationError, + OperationError as OperationErrorClass, operationErrorResponse, runOperation, } from "../operations"; @@ -196,6 +198,51 @@ describe("runOperation", () => { }); }); + test("answers a hook's validation rejection with 422, like generated CRUD", () => { + const response = operationErrorResponse( + new DatabaseValidationError("title is empty", [ + { path: ["title"], message: "Title is required" }, + ]), + ); + expect(response).toEqual({ + status: 422, + body: { + error: "Database request failed validation", + details: [{ path: "title", message: "Title is required" }], + }, + }); + }); + + test("runs operations and errors from another copy of the module", async () => { + // A dev loader (jiti, --conditions=development) can evaluate the module twice. + vi.resetModules(); + const other = await import("../operations"); + expect(other.OperationError).not.toBe(OperationErrorClass); + const op = other.defineMutation({ + input: z.object({}), + authorize: () => true, + execute: async () => { + throw new other.OperationError(409, "Order is not pending"); + }, + }); + const error = await failure( + runOperation(op, { actor: alice, input: {} }, fakeDb(state)), + ); + expect(operationErrorResponse(error)).toEqual({ + status: 409, + body: { error: "Order is not pending" }, + }); + // The shared mark alone does not make an object runnable. + const forged = { ...op, authorize: [] } as never; + expect( + ( + await failure( + runOperation(forged, { actor: alice, input: {} }, fakeDb(state)), + ) + ).statusCode, + ).toBe(500); + }); + test("refuses to define or run an operation without authorization", async () => { expect(() => defineQuery({ @@ -350,6 +397,35 @@ describe("database routes", () => { expect(sent).toEqual({ status: 200, body: { id: "7", by: "alice" } }); }); + test("answers a hook's validation rejection inside a mutation with 422", async () => { + const handlers = await mount({ + api: false, + routes: { + "POST /notes": defineMutation({ + input: z.object({}), + authorize: () => true, + execute: async () => { + throw new DatabaseValidationError("body is empty", [ + { path: ["body"], message: "Body is required" }, + ]); + }, + }), + }, + }); + expect( + await call(handlers.get("post /notes"), { + headers: { "x-forwarded-user": "alice" }, + body: {}, + }), + ).toEqual({ + status: 422, + body: { + error: "Database request failed validation", + details: [{ path: "body", message: "Body is required" }], + }, + }); + }); + test("answers 401 without the proxy header and 403 for a refused actor", async () => { const handlers = await mount({ api: false,