From 2d830be7eece7e47dd329258242d0b2f099f4be7 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Wed, 23 Sep 2026 17:59:35 +0200 Subject: [PATCH 01/10] feat(appkit): establish backward-compatible user execution scopes Inherit ambient callers and retain fail-closed request-based tool dispatch. Preserve deprecated context exports and warehouse access for existing callers. Signed-off-by: MarioCadenas --- docs/docs/plugins/execution-context.md | 88 +++--- packages/appkit/src/cache/index.ts | 4 +- packages/appkit/src/context/caller-context.ts | 8 +- packages/appkit/src/context/index.ts | 4 +- packages/appkit/src/context/request-scope.ts | 53 ++++ packages/appkit/src/context/scoped-api.ts | 122 +++++++ .../src/context/tests/legacy-context.test.ts | 56 ++++ packages/appkit/src/context/user-context.ts | 11 +- packages/appkit/src/core/appkit.ts | 49 ++- packages/appkit/src/core/plugin-context.ts | 109 ++++--- .../src/core/tests/appkit-user-scope.test.ts | 299 ++++++++++++++++++ .../src/core/tests/plugin-context.test.ts | 76 ++++- packages/appkit/src/index.ts | 12 + packages/appkit/src/plugin/plugin.ts | 192 +---------- packages/appkit/src/plugins/agents/agents.ts | 6 +- .../agents/tests/dispatch-tool-call.test.ts | 3 +- .../plugins/ai-search/tests/ai-search.test.ts | 10 + .../plugins/analytics/tests/analytics.test.ts | 24 +- .../appkit/src/testing/create-test-app.ts | 6 +- .../appkit/src/testing/test-plugin-context.ts | 87 +++-- .../testing/tests/test-plugin-context.test.ts | 142 ++++++--- packages/shared/src/plugin.ts | 33 ++ 22 files changed, 989 insertions(+), 405 deletions(-) create mode 100644 packages/appkit/src/context/request-scope.ts create mode 100644 packages/appkit/src/context/scoped-api.ts create mode 100644 packages/appkit/src/context/tests/legacy-context.test.ts create mode 100644 packages/appkit/src/core/tests/appkit-user-scope.test.ts diff --git a/docs/docs/plugins/execution-context.md b/docs/docs/plugins/execution-context.md index bb2400492..837101b60 100644 --- a/docs/docs/plugins/execution-context.md +++ b/docs/docs/plugins/execution-context.md @@ -4,62 +4,70 @@ sidebar_position: 5 # Execution context -AppKit manages Databricks authentication via two contexts: +AppKit uses the service principal by default. Open a caller scope when an +operation should use the requesting user's Databricks permissions. -- **ServiceContext** (singleton): Initialized at app startup with service principal credentials -- **ExecutionContext**: Determined at runtime - either service principal or user context +## User-scoped operations -## Headers for user context - -- `x-forwarded-user`: required in production; identifies the user -- `x-forwarded-access-token`: required for user token passthrough - -## Using `asUser(req)` for user-scoped operations - -The `asUser(req)` pattern allows plugins to execute operations using the requesting user's credentials: +Prefer a scope block for handlers that perform multiple operations: ```ts -// In a custom plugin route handler -router.post("/users/me/data", async (req, res) => { - // Execute as the user (uses their Databricks permissions) - const result = await this.asUser(req).query("SELECT ..."); - res.json(result); +const result = await appkit.asUser(req).run(async (kit) => { + const orders = await kit.analytics.query("orders"); + return orders; }); -// Service principal execution (default) -router.post("/system/data", async (req, res) => { - const result = await this.query("SELECT ..."); - res.json(result); -}); +// Shorthand for one operation: +await appkit.asUser(req).analytics.query("orders"); + +// With no caller scope, a plain call uses the service principal: +await appkit.analytics.query("metrics"); ``` -## Context helper functions +One immutable caller context is shared by the block and its plugin handles. +Nested operations, tools, and plain app handles used inside the block inherit +that context. Async work and lazy stream consumption preserve the caller. +There is no public `asApp()` and scoped handles cannot chain another `asUser()`. +Group principals are deferred. -Exported from `@databricks/appkit`: +The Apps proxy supplies `x-forwarded-access-token` and `x-forwarded-user`, both +required in production. `x-forwarded-email` is optional. Only trust these headers +behind the authenticated Apps proxy or your own trusted authentication boundary. -- `getCurrentUserId()`: Returns user ID in user context, service user ID otherwise -- `getWorkspaceClient()`: Returns the appropriate WorkspaceClient for current context -- `getWarehouseId()`: `Promise` (from `DATABRICKS_WAREHOUSE_ID` or auto-selected in dev) -- `getWorkspaceId()`: `Promise` (from `DATABRICKS_WORKSPACE_ID` or fetched) +`plugin.asUser(req)` remains available with a one-time deprecation warning. +New code should use `appkit.asUser(req)`. -## Telemetry span attributes +## Tools and agents -The `plugin.execute` span created by the execution interceptor chain includes these attributes: +`PluginContext.executeTool` inherits an existing caller scope. Without one, it +establishes user scope from the request, preserving the original fail-closed OBO +default. Missing credentials reject before the provider executes. An existing +caller takes precedence over conflicting or absent request credentials. -| Attribute | Type | Description | -|-----------|------|-------------| -| `execution.context` | `"user"` \| `"service"` | Whether the operation runs as a user (OBO) or service principal | -| `caller.id` | `string` | The user ID (OBO) or service principal ID | -| `execution.obo_dev_fallback` | `boolean` | Set to `true` when an OBO call falls back to service principal in development mode | +The agents HTTP execution routes (`/invocations`, `/responses`, and +`/api/agents/chat`) establish user scope at entry by default, including all nested +tool calls. Missing credentials reject the request before the agent runs. +The marked development fallback below is the only missing-token exception. -These attributes are automatically added when your plugin uses `execute()` or `executeStream()`. All built-in plugins use these methods for their OBO operations. Custom plugins should do the same to get automatic telemetry instrumentation. +## Context helpers and cache isolation -## Lakebase per-user connections +Exported from `@databricks/appkit`: -The Lakebase plugin uses a different mechanism for `asUser(req)`: instead of swapping the `WorkspaceClient` via AsyncLocalStorage, it creates a **separate `pg.Pool` per user**, each with its own OAuth token refresh. This is necessary because PostgreSQL connections are authenticated at connection time — the pool itself is the authentication boundary. +- `getCurrentPrincipalKey()`: `app` or `user:`, used to partition cache entries. +- `getCurrentActorId()`: initiating user ID, when available, for audit and telemetry. +- `getWorkspaceClient()`: workspace client for the current execution. +- `getWarehouseId()`: resource configuration, separate from execution identity. +- `getWorkspaceId()`: workspace ID as a promise. -See [Lakebase plugin — per-user connections](./lakebase.md#on-behalf-of-obo--per-user-connections) for details. +The old context helpers remain as deprecated compatibility aliases. Cache keys +now include the principal namespace even when an explicit legacy user key is +supplied. Existing stored entries will have a cold miss after upgrading; users +and SP continue to have separate cache entries and in-flight work. -## Development mode behavior +## Development fallback -In local development (`NODE_ENV=development`), if `asUser(req)` is called without a user token, it logs a warning and skips user impersonation — the operation runs with the default credentials configured for the app instead. The telemetry span will show `execution.context: "service"` with `execution.obo_dev_fallback: true` to distinguish these from regular service principal calls. +With `NODE_ENV=development`, `asUser(req)` without a token logs a warning and +runs with default app credentials, marked `DEV_OBO_FALLBACK`. If a caller scope +is already open, fallback retains it instead of widening to SP. The marker does +not leak outside the scope. Production never falls back when credentials are +missing. diff --git a/packages/appkit/src/cache/index.ts b/packages/appkit/src/cache/index.ts index 486bd8133..fcaffc637 100644 --- a/packages/appkit/src/cache/index.ts +++ b/packages/appkit/src/cache/index.ts @@ -4,6 +4,7 @@ import type { CacheConfig, CacheEntry, CacheStorage } from "shared"; import { createLakebasePool } from "../connectors/lakebase"; import { getClientOptions } from "../context/client-options"; +import { getCurrentPrincipalKey } from "../context/execution-context"; import { AppKitError, ExecutionError, InitializationError } from "../errors"; import { createLogger } from "../logging/logger"; import type { Counter, TelemetryProvider } from "../telemetry"; @@ -551,7 +552,8 @@ export class CacheManager { * @returns Cache key */ generateKey(parts: (string | number | object)[], userKey: string): string { - const allParts = [userKey, ...parts]; + // Legacy partitions remain supported, but cannot override the principal namespace. + const allParts = [getCurrentPrincipalKey(), userKey, ...parts]; const serialized = JSON.stringify(allParts); return createHash("sha256").update(serialized).digest("hex"); } diff --git a/packages/appkit/src/context/caller-context.ts b/packages/appkit/src/context/caller-context.ts index c99d05258..5fa59dc01 100644 --- a/packages/appkit/src/context/caller-context.ts +++ b/packages/appkit/src/context/caller-context.ts @@ -1,4 +1,5 @@ import type { ServiceContextState } from "./service-context"; +import type { UserContext } from "./user-context"; /** The caller identity whose permissions authorize execution, not its resources. */ export type CallerPrincipal = Readonly<{ @@ -18,6 +19,8 @@ export interface CallerContext { /** Truncated SHA-256 hash of the caller token, used to detect rotation. */ readonly tokenFingerprint?: string; readonly workspaceId: Promise; + /** @deprecated Use getWarehouseId(). Only legacy context access exposes this field. */ + readonly warehouseId?: Promise; } const snapshots = new WeakSet(); @@ -35,7 +38,10 @@ export function snapshotCallerContext(ctx: CallerContext): CallerContext { return snapshot; } -export type ExecutionContext = ServiceContextState | CallerContext; +export type ExecutionContext = + | ServiceContextState + | CallerContext + | UserContext; export function isCallerContext(ctx: ExecutionContext): ctx is CallerContext { return "principal" in ctx && ctx.principal.type === "user"; diff --git a/packages/appkit/src/context/index.ts b/packages/appkit/src/context/index.ts index e1b849689..c17ba4fb0 100644 --- a/packages/appkit/src/context/index.ts +++ b/packages/appkit/src/context/index.ts @@ -4,6 +4,8 @@ export { getCurrentPrincipalKey, getCurrentUserId, getExecutionContext, + getUserContext, + isInUserContext, getWarehouseId, getWorkspaceClient, getWorkspaceId, @@ -18,4 +20,4 @@ export { isCallerContext, } from "./caller-context"; export { ServiceContext } from "./service-context"; -export type { UserContext } from "./user-context"; +export { type UserContext, isUserContext } from "./user-context"; diff --git a/packages/appkit/src/context/request-scope.ts b/packages/appkit/src/context/request-scope.ts new file mode 100644 index 000000000..8c58ba17c --- /dev/null +++ b/packages/appkit/src/context/request-scope.ts @@ -0,0 +1,53 @@ +import { createContextKey, context as otelContext } from "@opentelemetry/api"; +import type { Request } from "express"; + +import { AuthenticationError } from "../errors"; +import { createLogger } from "../logging/logger"; +import { runInCallerContext } from "./execution-context"; +import { ServiceContext } from "./service-context"; + +const logger = createLogger("execution-context"); +const DEV_OBO_FALLBACK_KEY = createContextKey("appkit.devOboFallback"); + +/** Internal scope shared by shorthand calls and the whole run block. */ +export interface RequestScope { + run(fn: () => T): T; +} + +export function isDevOboFallback(): boolean { + return otelContext.active().getValue(DEV_OBO_FALLBACK_KEY) === true; +} + +/** Build the caller once from the trusted Apps proxy headers. */ +export function createRequestScope( + req: Request, + createCaller = ServiceContext.createCallerContext, +): RequestScope { + const token = req.header("x-forwarded-access-token")?.trim(); + const userId = req.header("x-forwarded-user")?.trim(); + const userEmail = req.header("x-forwarded-email"); + const isDev = process.env.NODE_ENV === "development"; + + if (!token && isDev) { + logger.warn( + "asUser() called without user token in development mode. Skipping user impersonation.", + ); + return { + run: (fn) => + otelContext.with( + otelContext.active().setValue(DEV_OBO_FALLBACK_KEY, true), + fn, + ), + }; + } + if (!token) throw AuthenticationError.missingToken("user token"); + if (!userId && !isDev) throw AuthenticationError.missingUserId(); + + const caller = createCaller( + token, + userId || "dev-user", + undefined, + userEmail, + ); + return { run: (fn) => runInCallerContext(caller, fn) }; +} diff --git a/packages/appkit/src/context/scoped-api.ts b/packages/appkit/src/context/scoped-api.ts new file mode 100644 index 000000000..c4e88f3aa --- /dev/null +++ b/packages/appkit/src/context/scoped-api.ts @@ -0,0 +1,122 @@ +import type { RequestScope } from "./request-scope"; + +const EXCLUDED_FROM_PROXY = new Set([ + "setup", + "shutdown", + "attachContext", + "injectRoutes", + "getEndpoints", + "getSkipBodyParsingPaths", + "abortActiveOperations", + "clientConfig", + "asUser", + "asApp", + "asGroup", + "constructor", +]); + +export function isPlainObject( + value: unknown, +): value is Record { + if (typeof value !== "object" || value === null) return false; + const prototype = Object.getPrototypeOf(value); + return prototype === Object.prototype || prototype === null; +} + +function isIdentityMethod(key: PropertyKey): boolean { + return typeof key === "string" && /^as[A-Z]/.test(key); +} + +/** Preserve identity across callable exports, returned handles, and lazy streams. */ +export function scopeApi( + value: T, + scope: RequestScope, + receiver?: unknown, +): T { + if (typeof value === "function") { + return new Proxy(value, { + apply: (fn, thisArg, args) => + scopeApi( + scope.run(() => Reflect.apply(fn, receiver ?? thisArg, args)), + scope, + ), + get: (fn, key) => + isIdentityMethod(key) + ? undefined + : scopeApi(Reflect.get(fn, key), scope, fn), + }); + } + if (value instanceof Promise) { + return value.then((result) => scopeApi(result, scope)) as T; + } + if (value && typeof value === "object" && Symbol.asyncIterator in value) { + const iterable = value as AsyncIterable; + const wrapIterator = ( + iterator: AsyncIterator, + ): AsyncIterableIterator => { + const finish = iterator.return?.bind(iterator); + const fail = iterator.throw?.bind(iterator); + return { + [Symbol.asyncIterator]() { + return this; + }, + next: (...args: [] | [unknown]) => + scope.run(() => iterator.next(...args)), + ...(finish && { + return: (result?: unknown) => scope.run(() => finish(result)), + }), + ...(fail && { + throw: (error?: unknown) => scope.run(() => fail(error)), + }), + }; + }; + if ("next" in value && typeof value.next === "function") { + return wrapIterator(value as AsyncIterableIterator) as T; + } + return { + [Symbol.asyncIterator]: () => + wrapIterator(scope.run(() => iterable[Symbol.asyncIterator]())), + } as T; + } + if (isPlainObject(value)) { + const result = Object.create(Object.getPrototypeOf(value)); + for (const key of Reflect.ownKeys(value)) { + if (isIdentityMethod(key)) continue; + Object.defineProperty(result, key, { + enumerable: Object.getOwnPropertyDescriptor(value, key)?.enumerable, + configurable: true, + get: () => + scopeApi( + scope.run(() => Reflect.get(value, key)), + scope, + receiver ?? value, + ), + }); + } + return result; + } + return value; +} + +/** Compatibility adapter for direct Plugin.asUser callers. */ +export function scopePlugin( + plugin: T, + scope: RequestScope, +): T { + return new Proxy(plugin, { + get(target, key) { + if (isIdentityMethod(key)) return undefined; + const value = Reflect.get(target, key, target); + if (typeof key === "string" && EXCLUDED_FROM_PROXY.has(key)) return value; + if (key === "exports" && typeof value === "function") { + return () => + scopeApi(scope.run(() => value.call(target)) ?? {}, scope, target); + } + if (typeof value === "function") { + return (...args: unknown[]) => + scope.run(() => Reflect.apply(value, target, args)); + } + return scopeApi(value, scope, target); + }, + }); +} diff --git a/packages/appkit/src/context/tests/legacy-context.test.ts b/packages/appkit/src/context/tests/legacy-context.test.ts new file mode 100644 index 000000000..27a25bf5d --- /dev/null +++ b/packages/appkit/src/context/tests/legacy-context.test.ts @@ -0,0 +1,56 @@ +import { + type ExecutionContext, + type UserContext, + getCurrentUserId, + getExecutionContext, + getUserContext, + getWarehouseId, + isUserContext, + runInUserContext, + ServiceContext, +} from "@databricks/appkit"; +import { afterEach, expect, test, vi } from "vitest"; + +import { mockServiceContext } from "../../testing/fixtures"; + +afterEach(() => vi.restoreAllMocks()); + +test("legacy public types and helpers retain identity and warehouse access", async () => { + const mock = mockServiceContext(); + const warehouseId = Promise.resolve("legacy-warehouse"); + const legacy: UserContext = { + client: mock.serviceContext.client, + userId: "legacy-user", + isUserContext: true, + warehouseId, + workspaceId: mock.serviceContext.workspaceId, + }; + const execution: ExecutionContext = legacy; + expect(isUserContext(execution)).toBe(true); + await runInUserContext(legacy, async () => { + const active = getExecutionContext(); + expect(isUserContext(active)).toBe(true); + if (isUserContext(active)) { + const userId: string = active.userId; + expect(userId).toBe("legacy-user"); + expect(active.isUserContext).toBe(true); + } + const resource: Promise | undefined = active.warehouseId; + expect(resource).toBe(warehouseId); + expect(getUserContext()?.warehouseId).toBe(warehouseId); + expect(getWarehouseId()).toBe(warehouseId); + expect(getCurrentUserId()).toBe("legacy-user"); + }); + expect(getCurrentUserId()).toBe(mock.serviceContext.serviceUserId); +}); + +test("the deprecated user factory remains callable through ServiceContext", () => { + const mock = mockServiceContext(); + const legacy: UserContext = ServiceContext.createUserContext( + "test-token", + "alice", + ); + expect(legacy.userId).toBe("alice"); + expect(legacy.isUserContext).toBe(true); + expect(legacy.warehouseId).toBe(mock.serviceContext.warehouseId); +}); diff --git a/packages/appkit/src/context/user-context.ts b/packages/appkit/src/context/user-context.ts index b854f578e..6c7d8a03b 100644 --- a/packages/appkit/src/context/user-context.ts +++ b/packages/appkit/src/context/user-context.ts @@ -9,11 +9,13 @@ import type { ServiceContextState } from "./service-context"; export type { ExecutionContext } from "./caller-context"; +const immutableCallers = new WeakSet(); + /** * @deprecated Use CallerContext and its principal field. Kept for callers * that construct the legacy shape or read its flat identity fields. */ -export interface UserContext { +export type UserContext = { /** WorkspaceClient authenticated as the user */ client: ServiceContextState["client"]; /** The user's ID (from request headers) */ @@ -30,7 +32,7 @@ export interface UserContext { workspaceId: Promise; /** Flag indicating this is a user context */ isUserContext: true; -} +}; /** * @deprecated Use snapshotCallerContext for identity-only snapshots. @@ -46,9 +48,10 @@ export function immutableCallerContext( function legacyIdentityContext( ctx: CallerContext, ): CallerContext & UserContext { + if (immutableCallers.has(ctx)) return ctx as CallerContext & UserContext; const caller = snapshotCallerContext(ctx); const { principal } = caller; - return Object.freeze({ + const snapshot = Object.freeze({ ...caller, get userId() { warnContextDeprecation( @@ -79,6 +82,8 @@ function legacyIdentityContext( return true; }, }); + immutableCallers.add(snapshot); + return snapshot; } /** Expose the old resource field only through the deprecated context APIs. */ diff --git a/packages/appkit/src/core/appkit.ts b/packages/appkit/src/core/appkit.ts index aa29602b2..ce8105a93 100644 --- a/packages/appkit/src/core/appkit.ts +++ b/packages/appkit/src/core/appkit.ts @@ -1,16 +1,18 @@ import type { + AppKitApi, BasePlugin, CacheConfig, InputPluginMap, OptionalConfigPluginDef, PluginConstructor, PluginData, - PluginMap, } from "shared"; import { version as productVersion } from "../../package.json"; import { CacheManager } from "../cache"; import { ServiceContext } from "../context"; +import { createRequestScope } from "../context/request-scope"; +import { scopeApi } from "../context/scoped-api"; import { isInternalTelemetryEnabled, TelemetryReporter, @@ -43,6 +45,29 @@ export class AppKit { /** Owns the shutdown sequence; assigned once every plugin has started. */ #lifecycle: LifecycleManager | undefined; + /** Execute a block or a single plugin call as the requesting user. */ + asUser(req: import("express").Request) { + const scope = createRequestScope(req); + const kit: Record = Object.create(null); + for (const [name, plugin] of Object.entries(this.#pluginInstances)) { + Object.defineProperty(kit, name, { + enumerable: true, + get: () => + scopeApi( + scope.run(() => plugin.exports?.() ?? {}), + scope, + plugin, + ), + }); + } + Object.defineProperty(kit, "run", { + value: async ( + fn: (kit: Record) => T | Promise, + ): Promise => scope.run(() => fn(kit)), + }); + return Object.freeze(kit); + } + private constructor(config: { plugins: TPlugins }) { const { plugins, ...globalConfig } = config; @@ -91,6 +116,11 @@ export class AppKit { pluginData: OptionalConfigPluginDef, extraData?: Record, ) { + if (name === "run" || /^as[A-Z]/.test(name)) { + throw new Error( + `Plugin name "${name}" is reserved for execution scoping`, + ); + } const { plugin: Plugin, config: pluginConfig } = pluginData; const baseConfig = { ...config, @@ -157,10 +187,8 @@ export class AppKit { * since the plugin manages its own `asUser` per-call (e.g. files plugin). * When it returns a plain object, the standard `asUser` wrapper is added. * - * The OBO-side wrapping lives inside `Plugin.asUser` — calling - * `plugin.asUser(req).exports()` returns exports whose functions already - * run inside the user's AsyncLocalStorage scope. AppKit only adapts the - * shape; it does not own the user-context concept. + * This preserves the deprecated per-plugin API. Both entry points delegate + * to the shared request scope; new code should use the app-level API. */ private wrapWithAsUser(plugin: T) { // If plugin doesn't implement exports(), return empty object @@ -182,6 +210,7 @@ export class AppKit { return { ...objExports, /** + * @deprecated Use appkit.asUser(req) instead. * Execute operations using the user's identity from the request. * Returns user-scoped exports where all methods execute with the * user's Databricks credentials instead of the service principal. @@ -199,7 +228,7 @@ export class AppKit { telemetry?: TelemetryConfig; cache?: CacheConfig; client?: WorkspaceClient; - onPluginsReady?: (appkit: PluginMap) => void | Promise; + onPluginsReady?: (appkit: AppKitApi) => void | Promise; disableInternalTelemetry?: boolean; /** * Skip installing the SIGTERM/SIGINT handlers. Internal, and not exposed @@ -209,7 +238,7 @@ export class AppKit { */ installSignalHandlers?: boolean; } = {}, - ): Promise> { + ): Promise> { // Initialize core services TelemetryManager.initialize(config?.telemetry); await CacheManager.getInstance(config?.cache); @@ -253,7 +282,7 @@ export class AppKit { await instance.#context.emitLifecycle("setup:complete"); - const app = instance as unknown as PluginMap; + const app = instance as typeof instance & AppKitApi; if (config.onPluginsReady) { logger.debug("Running onPluginsReady hook"); @@ -426,9 +455,9 @@ export async function createApp< cache?: CacheConfig; client?: WorkspaceClient; /** Runs after plugin setup but **before** the server starts. */ - onPluginsReady?: (appkit: PluginMap) => void | Promise; + onPluginsReady?: (appkit: AppKitApi) => void | Promise; disableInternalTelemetry?: boolean; } = {}, -): Promise> { +): Promise> { return AppKit._createApp(config); } diff --git a/packages/appkit/src/core/plugin-context.ts b/packages/appkit/src/core/plugin-context.ts index 4f86a5189..ea3c1cd56 100644 --- a/packages/appkit/src/core/plugin-context.ts +++ b/packages/appkit/src/core/plugin-context.ts @@ -1,6 +1,9 @@ import type express from "express"; -import type { BasePlugin, IAppRequest, ToolProvider } from "shared"; +import type { BasePlugin, ToolProvider } from "shared"; +import { getCallerContext } from "../context/execution-context"; +import { createRequestScope } from "../context/request-scope"; +import { ServiceContext } from "../context/service-context"; import { createLogger } from "../logging/logger"; import { type ITelemetry, @@ -22,13 +25,9 @@ interface RouteTarget { } /** - * A tool-provider plugin that also exposes user-scoped execution. Plugins - * derived from {@link Plugin} satisfy this implicitly because `asUser` lives - * on the base class. {@link isToolProvider} narrows to this shape so - * `executeTool` can call `asUser` without an unsafe cast. + * Tool execution inherits the caller scope established by the entry point. */ -type ToolProviderPlugin = BasePlugin & - ToolProvider & { asUser: (req: IAppRequest) => ToolProvider }; +type ToolProviderPlugin = BasePlugin & ToolProvider; /** * Lifecycle events emitted through {@link PluginContext.emitLifecycle}. @@ -68,18 +67,29 @@ export class PluginContext { Set<() => void | Promise> >(); private telemetry: ITelemetry; + private createCallerContext: typeof ServiceContext.createCallerContext; /** * @param deps.telemetry - Telemetry provider used for `executeTool` spans. - * Defaults to the shared `"plugin-context"` provider — the production + * Defaults to the shared `"plugin-context"` provider, the production * path. Injectable so the testing kit can pass a mock provider and run - * `executeTool` without a live OpenTelemetry pipeline. This is the only - * seam the mock context needs; route buffering and the tool registry are + * `executeTool` without a live OpenTelemetry pipeline. + * @param deps.createCallerContext - Offline credential factory for tests. + * Header validation and ALS remain real; route buffering and the registry are * exercised through the existing public API. */ - constructor(deps: { telemetry?: ITelemetry } = {}) { + constructor( + deps: { + telemetry?: ITelemetry; + /** @internal Credential factory seam for offline testing. */ + createCallerContext?: typeof ServiceContext.createCallerContext; + } = {}, + ) { this.telemetry = deps.telemetry ?? TelemetryManager.getProvider("plugin-context"); + this.createCallerContext = + deps.createCallerContext ?? + ((...args) => ServiceContext.createCallerContext(...args)); } /** @@ -190,15 +200,15 @@ export class PluginContext { } /** - * Execute a tool on a ToolProvider plugin with automatic user scoping + * Execute a tool on a ToolProvider plugin with the ambient principal * and telemetry. * * The context: * 1. Resolves the plugin by name - * 2. Calls `asUser(req)` for user-scoped execution + * 2. Inherits the caller, or establishes user scope from the request * 3. Wraps the call in a telemetry span with a configurable timeout * - * @param timeoutMs Per-call timeout. Defaults to 5 minutes — the floor + * @param timeoutMs Per-call timeout. Defaults to 5 minutes, the floor * for cold SQL Warehouse round-trips, long Genie conversations, and * busy serverless Lakebase queries. The agents plugin overrides this * per-app via `agents({ limits: { toolCallTimeoutMs } })`. @@ -221,35 +231,41 @@ export class PluginContext { const tracer = this.telemetry.getTracer(); const operationName = `executeTool:${pluginName}.${toolName}`; - return tracer.startActiveSpan(operationName, async (span) => { - const timeoutSignal = AbortSignal.timeout(timeoutMs); - const combinedSignal = signal - ? AbortSignal.any([signal, timeoutSignal]) - : timeoutSignal; + const execute = () => + tracer.startActiveSpan(operationName, async (span) => { + const timeoutSignal = AbortSignal.timeout(timeoutMs); + const combinedSignal = signal + ? AbortSignal.any([signal, timeoutSignal]) + : timeoutSignal; - try { - const userScoped = provider.asUser(req); - const result = await userScoped.executeAgentTool( - toolName, - args, - combinedSignal, - ); - span.setStatus({ code: SpanStatusCode.OK }); - return result; - } catch (error) { - span.setStatus({ - code: SpanStatusCode.ERROR, - message: - error instanceof Error ? error.message : "Tool execution failed", - }); - span.recordException( - error instanceof Error ? error : new Error(String(error)), - ); - throw error; - } finally { - span.end(); - } - }); + try { + const result = await provider.executeAgentTool( + toolName, + args, + combinedSignal, + ); + span.setStatus({ code: SpanStatusCode.OK }); + return result; + } catch (error) { + span.setStatus({ + code: SpanStatusCode.ERROR, + message: + error instanceof Error ? error.message : "Tool execution failed", + }); + span.recordException( + error instanceof Error ? error : new Error(String(error)), + ); + throw error; + } finally { + span.end(); + } + }); + + // Direct request dispatch retains the original fail-closed OBO default. + // An existing caller wins, even when request credentials differ or are absent. + return getCallerContext() + ? execute() + : createRequestScope(req, this.createCallerContext).run(execute); } /** @@ -342,10 +358,7 @@ export class PluginContext { } /** - * Type guard: checks whether a plugin implements the ToolProvider interface - * and exposes the user-scoped `asUser` helper that the {@link Plugin} base - * class provides. Narrowing to {@link ToolProviderPlugin} lets `executeTool` - * call `asUser` without an unsafe cast. + * Type guard for the tool-provider methods. Identity comes from the ALS scope. */ export function isToolProvider(plugin: unknown): plugin is ToolProviderPlugin { return ( @@ -354,8 +367,6 @@ export function isToolProvider(plugin: unknown): plugin is ToolProviderPlugin { "getAgentTools" in plugin && typeof (plugin as ToolProvider).getAgentTools === "function" && "executeAgentTool" in plugin && - typeof (plugin as ToolProvider).executeAgentTool === "function" && - "asUser" in plugin && - typeof (plugin as { asUser?: unknown }).asUser === "function" + typeof (plugin as ToolProvider).executeAgentTool === "function" ); } diff --git a/packages/appkit/src/core/tests/appkit-user-scope.test.ts b/packages/appkit/src/core/tests/appkit-user-scope.test.ts new file mode 100644 index 000000000..5aa244a5b --- /dev/null +++ b/packages/appkit/src/core/tests/appkit-user-scope.test.ts @@ -0,0 +1,299 @@ +import { context as otelContext } from "@opentelemetry/api"; +import { AsyncLocalStorageContextManager } from "@opentelemetry/context-async-hooks"; +import type { AgentAdapter, AgentToolDefinition, ToolProvider } from "shared"; +import { describe, expect, expectTypeOf, test, vi } from "vitest"; + +import { CacheManager } from "../../cache"; +import { + getCallerContext, + getCurrentPrincipalKey, + ServiceContext, +} from "../../context"; +import { isDevOboFallback } from "../../context/request-scope"; +import { Plugin, toPlugin } from "../../plugin"; +import { agents } from "../../plugins/agents"; +import { createMockRequest, createTestApp } from "../../testing"; + +class IdentityPlugin extends Plugin implements ToolProvider { + static manifest = { + name: "identity" as const, + displayName: "Identity probe", + description: "Captures the principal in tests", + resources: { required: [], optional: [] }, + }; + private marker = "bound"; + identify() { + return `${this.marker}:${getCurrentPrincipalKey()}`; + } + getAgentTools(): AgentToolDefinition[] { + return [ + { + name: "read", + description: "Read identity", + parameters: { type: "object", properties: {} }, + }, + ]; + } + async executeAgentTool() { + await Promise.resolve(); + return getCurrentPrincipalKey(); + } + exports() { + return { + read: this.identify, + nested: { read: () => getCurrentPrincipalKey() }, + stream: async function* () { + await Promise.resolve(); + yield getCurrentPrincipalKey(); + await Promise.resolve(); + yield getCurrentPrincipalKey(); + }, + fail: () => { + throw new Error("scope failure"); + }, + asOther: () => "must not be reachable", + }; + } +} +class CallablePlugin extends Plugin { + static manifest = { ...IdentityPlugin.manifest, name: "callable" as const }; + exports() { + return (_key: string) => ({ read: () => getCurrentPrincipalKey() }); + } +} +const identity = toPlugin(IdentityPlugin); +const callable = toPlugin(CallablePlugin); +const request = (userId: string) => + createMockRequest({ obo: { userId, token: `${userId}-token` } }); + +describe("app-level caller scope", () => { + test("keeps the plugin alias with one deprecation warning", async () => { + await using app = await createTestApp({ + plugins: [identity()], + server: false, + }); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + expect(app.plugins.identity.asUser(request("alice")).read()).toBe( + "bound:user:alice", + ); + expect(app.plugins.identity.asUser(request("bob")).read()).toBe( + "bound:user:bob", + ); + expect( + warn.mock.calls.filter((args) => + args.some( + (arg) => + typeof arg === "string" && + arg.includes("Plugin.asUser is deprecated"), + ), + ), + ).toHaveLength(1); + } finally { + warn.mockRestore(); + } + }); + test("shares one caller across shorthand, nested exports, callable handles, and run", async () => { + await using app = await createTestApp({ + plugins: [identity(), callable()], + server: false, + }); + const kit = app.plugins; + const createCaller = vi.mocked(ServiceContext.createCallerContext); + createCaller.mockClear(); + const scoped = kit.asUser(request("alice")); + expect(scoped.identity.read()).toBe("bound:user:alice"); + expect(scoped.identity.nested.read()).toBe("user:alice"); + expect(scoped.callable("a").read()).toBe("user:alice"); + const result = scoped.run(async (userKit) => { + const caller = getCallerContext(); + await Promise.resolve(); + expect(userKit.identity.read()).toBe("bound:user:alice"); + expect(kit.identity.read()).toBe("bound:user:alice"); + expect(getCallerContext()).toBe(caller); + return 42; + }); + expectTypeOf(result).toEqualTypeOf>(); + expect(await result).toBe(42); + expect(createCaller).toHaveBeenCalledTimes(1); + expect(kit.identity.read()).toBe("bound:app"); + }); + + test("restores the parent scope after failures and isolates concurrent users", async () => { + await using app = await createTestApp({ + plugins: [identity()], + server: false, + }); + const alice = app.plugins.asUser(request("alice")); + const bob = app.plugins.asUser(request("bob")); + await alice.run(async () => { + await expect(bob.run((kit) => kit.identity.fail())).rejects.toThrow( + "scope failure", + ); + expect(getCurrentPrincipalKey()).toBe("user:alice"); + }); + const values = await Promise.all( + [alice, bob].map((scope) => + scope.run(async (kit) => { + await new Promise((resolve) => setImmediate(resolve)); + return kit.identity.read(); + }), + ), + ); + expect(values).toEqual(["bound:user:alice", "bound:user:bob"]); + expect(getCurrentPrincipalKey()).toBe("app"); + }); + + test("keeps shorthand streams scoped when consumed outside the original call", async () => { + await using app = await createTestApp({ + plugins: [identity()], + server: false, + }); + const stream = app.plugins.asUser(request("alice")).identity.stream(); + const keys: string[] = []; + for await (const key of stream) keys.push(key); + expect(keys).toEqual(["user:alice", "user:alice"]); + expect(getCurrentPrincipalKey()).toBe("app"); + const direct = app.plugins.asUser(request("bob")).identity.stream(); + expect(await direct.next()).toEqual({ done: false, value: "user:bob" }); + await direct.return(); + expect(getCurrentPrincipalKey()).toBe("app"); + }); + + test("marks dev fallback without widening an existing caller scope", async () => { + await using app = await createTestApp({ + plugins: [identity()], + server: false, + }); + vi.stubEnv("NODE_ENV", "development"); + const manager = new AsyncLocalStorageContextManager().enable(); + otelContext.setGlobalContextManager(manager); + try { + const fallback = app.plugins.asUser(createMockRequest()); + await fallback.run(() => { + expect(getCurrentPrincipalKey()).toBe("app"); + expect(isDevOboFallback()).toBe(true); + }); + await app.plugins.asUser(request("alice")).run(() => + fallback.run(() => { + expect(getCurrentPrincipalKey()).toBe("user:alice"); + expect(isDevOboFallback()).toBe(true); + }), + ); + expect(isDevOboFallback()).toBe(false); + } finally { + vi.unstubAllEnvs(); + otelContext.disable(); + manager.disable(); + } + }); + + test("blocks principal chaining and has no public asApp", async () => { + await using app = await createTestApp({ + plugins: [identity()], + server: false, + }); + const scoped = app.plugins.asUser(request("alice")); + expect(scoped).not.toHaveProperty("asUser"); + expect(scoped).not.toHaveProperty("asApp"); + expect(scoped.identity).not.toHaveProperty("asUser"); + expect(scoped.identity).not.toHaveProperty("asOther"); + expect(app.plugins).not.toHaveProperty("asApp"); + expectTypeOf(scoped).not.toHaveProperty("asUser"); + expectTypeOf(scoped.identity).not.toHaveProperty("asUser"); + }); + + test("fails closed for missing token or user identity outside development", async () => { + await using app = await createTestApp({ + plugins: [identity()], + server: false, + }); + expect(() => app.plugins.asUser(createMockRequest())).toThrow("token"); + expect(() => + app.plugins.asUser( + createMockRequest({ headers: { "x-forwarded-access-token": "token" } }), + ), + ).toThrow(); + }); + + test("partitions cache results and in-flight work by principal even with identical legacy keys", async () => { + await using app = await createTestApp({ + plugins: [identity()], + server: false, + }); + const cache = CacheManager.getInstanceSync(); + const execute = vi.fn(async () => getCurrentPrincipalKey()); + const read = () => + cache.getOrExecute(["identity"], execute, "same-legacy-key"); + expect(await read()).toBe("app"); + const results = await Promise.all( + ["alice", "bob", "alice"].map((user) => + app.plugins.asUser(request(user)).run(read), + ), + ); + expect(results).toEqual(["user:alice", "user:bob", "user:alice"]); + expect(execute).toHaveBeenCalledTimes(3); + expect(await read()).toBe("app"); + }); +}); + +describe("agents HTTP identity boundary", () => { + const adapter: AgentAdapter = { + async *run(_input, ctx) { + const principal = getCurrentPrincipalKey(); + const toolPrincipal = await ctx.executeTool("identity.read", {}); + yield { type: "message_delta", content: `${principal}/${toolPrincipal}` }; + yield { type: "status", status: "complete" }; + }, + }; + + test.each(["/invocations", "/responses", "/api/agents/chat"])( + "%s defaults to user identity, including tool dispatch", + async (path) => { + await using app = await createTestApp({ + plugins: [ + identity(), + agents({ + agents: { + probe: { + instructions: "Identify the caller", + model: adapter, + tools: (plugins) => plugins.identity.toolkit(), + }, + }, + }), + ], + }); + const response = await app.post(path, { + body: path.endsWith("chat") ? { message: "hello" } : { input: "hello" }, + obo: { userId: "alice" }, + }); + expect(response.status).toBe(200); + expect(await response.text()).toContain("user:alice/user:alice"); + expect(getCurrentPrincipalKey()).toBe("app"); + }, + ); + + test.each(["/invocations", "/responses", "/api/agents/chat"])( + "%s cannot execute as SP by omitting the token", + async (path) => { + const run = vi.fn(adapter.run); + await using app = await createTestApp({ + plugins: [ + identity(), + agents({ + agents: { + probe: { instructions: "Identify the caller", model: { run } }, + }, + }), + ], + }); + const response = await app.post(path, { + body: path.endsWith("chat") ? { message: "hello" } : { input: "hello" }, + headers: { "x-forwarded-user": "alice" }, + }); + expect(response.status).toBe(401); + expect(run).not.toHaveBeenCalled(); + }, + ); +}); diff --git a/packages/appkit/src/core/tests/plugin-context.test.ts b/packages/appkit/src/core/tests/plugin-context.test.ts index 194056635..cf6be16d6 100644 --- a/packages/appkit/src/core/tests/plugin-context.test.ts +++ b/packages/appkit/src/core/tests/plugin-context.test.ts @@ -1,6 +1,8 @@ import type { AgentToolDefinition } from "shared"; -import { beforeEach, describe, expect, test, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +import { getCurrentPrincipalKey, runInCallerContext } from "../../context"; +import { createMockRequest, mockServiceContext } from "../../testing/fixtures"; import { isToolProvider, PluginContext } from "../plugin-context"; /** @@ -63,10 +65,13 @@ function createMockToolProvider(tools: AgentToolDefinition[] = []) { describe("PluginContext", () => { let ctx: PluginContext; + let service: ReturnType; beforeEach(() => { + service = mockServiceContext(); ctx = new PluginContext(); }); + afterEach(() => service.restore()); describe("route buffering", () => { test("addRoute buffers when no route target exists", () => { @@ -259,14 +264,19 @@ describe("PluginContext", () => { }); describe("executeTool", () => { - test("calls asUser(req).executeAgentTool on the correct plugin", async () => { + test("defaults direct dispatch to the forwarded user", async () => { const provider = createMockToolProvider(); + provider.executeAgentTool.mockImplementation(async () => + getCurrentPrincipalKey(), + ); ctx.registerToolProvider("analytics", provider); - const mockReq = { headers: {} } as any; - await ctx.executeTool(mockReq, "analytics", "query", { sql: "SELECT 1" }); + const mockReq = createMockRequest({ obo: { userId: "alice" } }); + await expect( + ctx.executeTool(mockReq, "analytics", "query", { sql: "SELECT 1" }), + ).resolves.toBe("user:alice"); - expect(provider.asUser).toHaveBeenCalledWith(mockReq); + expect(provider.asUser).not.toHaveBeenCalled(); expect(provider.executeAgentTool).toHaveBeenCalledWith( "query", { sql: "SELECT 1" }, @@ -274,8 +284,36 @@ describe("PluginContext", () => { ); }); + test("rejects missing credentials before executing the provider", async () => { + const provider = createMockToolProvider(); + ctx.registerToolProvider("analytics", provider); + await expect( + ctx.executeTool(createMockRequest(), "analytics", "query", {}), + ).rejects.toThrow(/token/i); + expect(provider.executeAgentTool).not.toHaveBeenCalled(); + }); + + test("inherits an ambient caller without reconstructing credentials", async () => { + const provider = createMockToolProvider(); + provider.executeAgentTool.mockImplementation(async () => + getCurrentPrincipalKey(), + ); + ctx.registerToolProvider("analytics", provider); + await expect( + runInCallerContext( + { + principal: { type: "user", userId: "injected" }, + client: service.serviceContext.client, + workspaceId: service.serviceContext.workspaceId, + }, + () => ctx.executeTool(createMockRequest(), "analytics", "query", {}), + ), + ).resolves.toBe("user:injected"); + expect(service.createUserContextSpy).not.toHaveBeenCalled(); + }); + test("throws for unknown plugin name", async () => { - const mockReq = { headers: {} } as any; + const mockReq = createMockRequest({ obo: true }); await expect( ctx.executeTool(mockReq, "nonexistent", "query", {}), @@ -289,7 +327,7 @@ describe("PluginContext", () => { ); ctx.registerToolProvider("analytics", provider); - const mockReq = { headers: {} } as any; + const mockReq = createMockRequest({ obo: true }); await expect( ctx.executeTool(mockReq, "analytics", "query", {}), @@ -301,7 +339,12 @@ describe("PluginContext", () => { const provider = createMockToolProvider(); ctx.registerToolProvider("analytics", provider); - await ctx.executeTool({ headers: {} } as any, "analytics", "query", {}); + await ctx.executeTool( + createMockRequest({ obo: true }), + "analytics", + "query", + {}, + ); expect(lastSpan.setStatus).toHaveBeenCalledWith({ code: SpanStatusCode.OK, @@ -318,7 +361,12 @@ describe("PluginContext", () => { ctx.registerToolProvider("analytics", provider); await expect( - ctx.executeTool({ headers: {} } as any, "analytics", "query", {}), + ctx.executeTool( + createMockRequest({ obo: true }), + "analytics", + "query", + {}, + ), ).rejects.toThrow("Query failed"); expect(lastSpan.setStatus).toHaveBeenCalledWith({ @@ -334,7 +382,7 @@ describe("PluginContext", () => { ctx.registerToolProvider("analytics", provider); const controller = new AbortController(); - const mockReq = { headers: {} } as any; + const mockReq = createMockRequest({ obo: true }); await ctx.executeTool( mockReq, @@ -369,7 +417,7 @@ describe("PluginContext", () => { ctx.registerToolProvider("analytics", provider); const pending = ctx.executeTool( - { headers: {} } as any, + createMockRequest({ obo: true }), "analytics", "query", {}, @@ -520,14 +568,12 @@ describe("isToolProvider", () => { expect(isToolProvider(undefined)).toBe(false); }); - test("returns false for objects missing asUser", () => { - // ToolProvider plugins must also expose user-scoped execution; the - // guard ensures executeTool can call asUser without an unsafe cast. + test("does not require the deprecated asUser method", () => { expect( isToolProvider({ getAgentTools: vi.fn(), executeAgentTool: vi.fn(), }), - ).toBe(false); + ).toBe(true); }); }); diff --git a/packages/appkit/src/index.ts b/packages/appkit/src/index.ts index 7e7476e2a..1873381c5 100644 --- a/packages/appkit/src/index.ts +++ b/packages/appkit/src/index.ts @@ -7,6 +7,9 @@ // Types from shared export type { + AppKitApi, + ScopedPluginMap, + UserScopedApp, BasePluginConfig, CacheConfig, IAppRouter, @@ -41,9 +44,18 @@ export { type CallerPrincipal, type ExecutionContext, type Principal, + type UserContext, + ServiceContext, + getCallerContext, getCurrentActorId, getCurrentPrincipalKey, + getCurrentUserId, getExecutionContext, + getUserContext, + isInUserContext, + isUserContext, + runInCallerContext, + runInUserContext, } from "./context"; export { createApp } from "./core"; export { getWarehouseId } from "./resources"; diff --git a/packages/appkit/src/plugin/plugin.ts b/packages/appkit/src/plugin/plugin.ts index a9ea78cf9..c435327a5 100644 --- a/packages/appkit/src/plugin/plugin.ts +++ b/packages/appkit/src/plugin/plugin.ts @@ -1,4 +1,4 @@ -import { createContextKey, context as otelContext } from "@opentelemetry/api"; +import { context as otelContext } from "@opentelemetry/api"; import type express from "express"; import type { BasePlugin, @@ -16,11 +16,10 @@ import { camelToKebab } from "shared"; import { AppManager } from "../app"; import { CacheManager } from "../cache"; -import { - getCurrentUserId, - runInCallerContext, - ServiceContext, -} from "../context"; +import { getCurrentUserId } from "../context"; +import { warnContextDeprecation } from "../context/deprecation"; +import { createRequestScope } from "../context/request-scope"; +import { scopePlugin } from "../context/scoped-api"; import type { PluginContext } from "../core/plugin-context"; import { AppKitError, AuthenticationError } from "../errors"; import { createLogger } from "../logging/logger"; @@ -45,62 +44,8 @@ import type { const logger = createLogger("plugin"); -/** - * OTel context key for marking OBO dev mode fallback. - * Set when asUser() is called in development mode without a user token. - */ -const DEV_OBO_FALLBACK_KEY = createContextKey("appkit.devOboFallback"); - -/** - * Returns true if `value` is a plain object literal (not an array, Date, - * class instance, etc.). Used to decide whether to recurse into nested - * export shapes when wrapping functions. - * - * @internal exported so the AppKit core can reuse the same predicate for - * its `bindExportMethods` walk; not part of the public package surface. - */ -export function isPlainObject( - value: unknown, -): value is Record { - if (typeof value !== "object" || value === null) return false; - const proto = Object.getPrototypeOf(value); - return proto === Object.prototype || proto === null; -} - -/** - * Returns a deep copy of `exports` where every function has been replaced - * with `wrap(fn)`, walking into nested plain objects. - * - * Used by the asUser proxy to make the user context follow function - * references that escape the proxy via `exports()`. The original input is - * not mutated, so plugins that memoize `exports()` are safe — each call - * through the proxy yields an independent, freshly wrapped view. - */ -function wrapExportFunctions( - exports: Record, - wrap: (fn: (...a: unknown[]) => unknown) => (...a: unknown[]) => unknown, -): Record { - const result: Record = {}; - for (const key of Object.keys(exports)) { - const val = exports[key]; - if (typeof val === "function") { - result[key] = wrap(val as (...a: unknown[]) => unknown); - } else if (isPlainObject(val)) { - result[key] = wrapExportFunctions(val, wrap); - } else { - result[key] = val; - } - } - return result; -} - -/** - * Returns true if the current execution is an OBO dev mode fallback - * (asUser() was called but fell back to service principal due to missing token). - */ -export function isDevOboFallback(): boolean { - return otelContext.active().getValue(DEV_OBO_FALLBACK_KEY) === true; -} +export { isPlainObject } from "../context/scoped-api"; +export { isDevOboFallback } from "../context/request-scope"; /** * Narrow an unknown thrown value to an Error that carries a numeric @@ -116,27 +61,6 @@ function hasHttpStatusCode( ); } -/** - * Methods that should not be proxied by asUser(). - * These are lifecycle/internal methods that don't make sense - * to execute in a user context. - */ -const EXCLUDED_FROM_PROXY = new Set([ - // Lifecycle methods - "setup", - "shutdown", - "attachContext", - "injectRoutes", - "getEndpoints", - "getSkipBodyParsingPaths", - "abortActiveOperations", - "clientConfig", - // asUser itself - prevent chaining like .asUser().asUser() - "asUser", - // Internal methods - "constructor", -]); - /** * Base abstract class for creating AppKit plugins. * @@ -422,106 +346,10 @@ export abstract class Plugin< throw AuthenticationError.missingUserId(); } - /** - * Execute operations using the user's identity from the request. - * Returns a proxy of this plugin where all method calls execute - * with the user's Databricks credentials instead of the service principal. - * - * @param req - The Express request containing the user token in headers - * @returns A proxied plugin instance that executes as the user - * @throws AuthenticationError if user token is not available in request headers (production only). - * In development mode (`NODE_ENV=development`), skips user impersonation instead of throwing. - */ + /** @deprecated Use appkit.asUser(req) to scope the whole app. */ asUser(req: express.Request): this { - const token = req.header("x-forwarded-access-token")?.trim(); - const userId = req.header("x-forwarded-user")?.trim(); - const userEmail = req.header("x-forwarded-email"); - const isDev = process.env.NODE_ENV === "development"; - - // In local development, skip user impersonation since there's no user - // token available. Mark execution as OBO dev fallback via OTel context - // so telemetry can distinguish intended OBO calls from regular SP calls. - if (!token && isDev) { - logger.warn( - "asUser() called without user token in development mode. Skipping user impersonation.", - ); - - return this._createAsUserProxy((fn) => (...args) => { - const ctx = otelContext.active().setValue(DEV_OBO_FALLBACK_KEY, true); - return otelContext.with(ctx, () => fn(...args)); - }); - } - - if (!token) { - throw AuthenticationError.missingToken("user token"); - } - - if (!userId && !isDev) { - throw AuthenticationError.missingUserId(); - } - - const effectiveUserId = userId || "dev-user"; - - const userContext = ServiceContext.createCallerContext( - token, - effectiveUserId, - undefined, - userEmail ?? undefined, - ); - - return this._createAsUserProxy( - (fn) => - (...args) => - runInCallerContext(userContext, () => fn(...args)), - ); - } - - /** - * Creates a proxy of `this` where every method call — and every function - * in the result of `exports()` — runs inside `wrapCall`. - * - * `wrapCall` decides the per-call scope. Two strategies are used today: - * - real OBO: fn => (...args) => runInCallerContext(userContext, () => fn(...args)) - * - dev fallback: fn => (...args) => otelContext.with(DEV_OBO_FALLBACK_KEY=true, () => fn(...args)) - * - * `exports` is intercepted because methods captured in the returned - * exports object never re-enter the proxy's `get` trap. Wrapping them - * here is the only way to make the user context follow function - * references back out of the plugin. - */ - private _createAsUserProxy( - wrapCall: ( - fn: (...a: unknown[]) => unknown, - ) => (...a: unknown[]) => unknown, - ): this { - return new Proxy(this, { - get: (target, prop, receiver) => { - const value = Reflect.get(target, prop, receiver); - - if (typeof value !== "function") return value; - if (typeof prop === "string" && EXCLUDED_FROM_PROXY.has(prop)) - return value; - - if (prop === "exports") { - return () => { - const raw = (value as () => unknown).call(target); - if (raw == null) return {}; - // Callable exports (e.g. files, jobs) manage per-call asUser - // themselves; leave them untouched. - if (typeof raw === "function") return raw; - if (isPlainObject(raw)) { - return wrapExportFunctions(raw, (fn) => - wrapCall(fn.bind(target)), - ); - } - return raw; - }; - } - - const fn = (value as (...a: unknown[]) => unknown).bind(target); - return wrapCall(fn); - }, - }) as this; + warnContextDeprecation("Plugin.asUser", "appkit.asUser(req)"); + return scopePlugin(this, createRequestScope(req)); } // streaming execution with interceptors diff --git a/packages/appkit/src/plugins/agents/agents.ts b/packages/appkit/src/plugins/agents/agents.ts index db06f80eb..4eb65a530 100644 --- a/packages/appkit/src/plugins/agents/agents.ts +++ b/packages/appkit/src/plugins/agents/agents.ts @@ -19,6 +19,7 @@ import type { import { isSupervisorTool } from "../../agents/supervisor-api"; import { AppKitMcpClient, buildMcpHostPolicy } from "../../connectors/mcp"; import { getWorkspaceClient } from "../../context"; +import { createRequestScope } from "../../context/request-scope"; import { consumeAdapterStream } from "../../core/agent/consume-adapter-stream"; import { loadAgentsFromDir } from "../../core/agent/load-agents"; import { CODE_AGENTS_SOURCE_DIR } from "../../core/agent/load-code-agents"; @@ -909,7 +910,7 @@ export class AgentsPlugin extends Plugin implements ToolProvider { // Return the promise so the forwardAsyncErrors wrapper applied by // PluginContext.addRoute can forward rejections to the error middleware. const handler = (req: express.Request, res: express.Response) => - this._handleInvoke(req, res); + createRequestScope(req).run(() => this._handleInvoke(req, res)); this.context.addRoute("post", "/invocations", handler); this.context.addRoute("post", "/responses", handler); } @@ -919,7 +920,8 @@ export class AgentsPlugin extends Plugin implements ToolProvider { name: "chat", method: "post", path: "/chat", - handler: async (req, res) => this._handleChat(req, res), + handler: async (req, res) => + createRequestScope(req).run(() => this._handleChat(req, res)), }); this.route(router, { name: "cancel", diff --git a/packages/appkit/src/plugins/agents/tests/dispatch-tool-call.test.ts b/packages/appkit/src/plugins/agents/tests/dispatch-tool-call.test.ts index 0fe6fa67c..9fa979843 100644 --- a/packages/appkit/src/plugins/agents/tests/dispatch-tool-call.test.ts +++ b/packages/appkit/src/plugins/agents/tests/dispatch-tool-call.test.ts @@ -369,8 +369,7 @@ describe("dispatchToolCall — toolkit timeout plumbing", () => { expect(call[2]).toBe("query"); expect(call[5]).toBe(90_000); - // The stub could never prove this: the real executeTool routed the call - // through the analytics provider's on-behalf-of (asUser) path. + // Direct dispatch establishes the forwarded user's scope. expect(mock.toolCalls).toHaveLength(1); expect(mock.toolCalls[0]).toMatchObject({ plugin: "analytics", diff --git a/packages/appkit/src/plugins/ai-search/tests/ai-search.test.ts b/packages/appkit/src/plugins/ai-search/tests/ai-search.test.ts index 3f38539a5..b692c1d67 100644 --- a/packages/appkit/src/plugins/ai-search/tests/ai-search.test.ts +++ b/packages/appkit/src/plugins/ai-search/tests/ai-search.test.ts @@ -22,6 +22,16 @@ vi.mock("../../../context", () => ({ }, })); +vi.mock("../../../context/service-context", () => ({ + ServiceContext: { + createCallerContext: (_token: string, userId: string) => ({ + principal: { type: "user", userId }, + client: mockWorkspaceClient, + workspaceId: Promise.resolve("test-workspace"), + }), + }, +})); + vi.mock("../../../logging/logger", () => ({ createLogger: () => ({ debug: vi.fn(), diff --git a/packages/appkit/src/plugins/analytics/tests/analytics.test.ts b/packages/appkit/src/plugins/analytics/tests/analytics.test.ts index bb42cb830..a8cbf3692 100644 --- a/packages/appkit/src/plugins/analytics/tests/analytics.test.ts +++ b/packages/appkit/src/plugins/analytics/tests/analytics.test.ts @@ -21,7 +21,9 @@ import type express from "express"; import { sql } from "shared"; import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +import { runInCallerContext } from "../../../context"; import { ServiceContext } from "../../../context/service-context"; +import { createMockWorkspaceClient } from "../../../testing"; import { useTestCache } from "../../../testing/test-cache"; import { AnalyticsPlugin, analytics, writeChunk } from "../analytics"; import type { IAnalyticsConfig } from "../types"; @@ -1893,7 +1895,7 @@ describe("Analytics Plugin", () => { describe("analytics as a cross-plugin tool provider", () => { // A consumer plugin (e.g. agents) resolves analytics' tools through the // shared PluginContext. These drive that dispatch and assert the on-behalf-of - // identity the real executeTool resolves — coverage a bare stub can't give. + // identity inherited from the caller scope. test("dispatches analytics.query on behalf of the user", async () => { const rows = [{ customer: "Acme", revenue: 1_000_000 }]; const mock = createTestPluginContext({ @@ -1903,9 +1905,17 @@ describe("analytics as a cross-plugin tool provider", () => { const req = createMockRequest({ obo: { userId: "analyst@example.com" }, }) as unknown as express.Request; - const result = await mock.ctx.executeTool(req, "analytics", "query", { - sql: "SELECT * FROM top_customers", - }); + const result = await runInCallerContext( + { + principal: { type: "user", userId: "analyst@example.com" }, + client: createMockWorkspaceClient(), + workspaceId: Promise.resolve("workspace"), + }, + () => + mock.ctx.executeTool(req, "analytics", "query", { + sql: "SELECT * FROM top_customers", + }), + ); expect(result).toEqual({ rows, @@ -1920,7 +1930,7 @@ describe("analytics as a cross-plugin tool provider", () => { }); }); - test("rejects a token-less request before the tool runs", async () => { + test("rejects token-less direct dispatch without an ambient caller", async () => { const mock = createTestPluginContext({ analytics: { query: () => ({ rows: [] }) }, }); @@ -1928,8 +1938,8 @@ describe("analytics as a cross-plugin tool provider", () => { await expect( mock.ctx.executeTool(req, "analytics", "query", {}), - ).rejects.toThrow(/Missing user token/); - expect(mock.toolCalls).toHaveLength(0); + ).rejects.toThrow(/token/i); + expect(mock.toolCalls).toEqual([]); }); test("forwards the per-call timeout so a slow tool is aborted", async () => { diff --git a/packages/appkit/src/testing/create-test-app.ts b/packages/appkit/src/testing/create-test-app.ts index fb4decdee..b54edbdcd 100644 --- a/packages/appkit/src/testing/create-test-app.ts +++ b/packages/appkit/src/testing/create-test-app.ts @@ -9,7 +9,7 @@ import type { CacheConfig, PluginConstructor, PluginData, - PluginMap, + AppKitApi, } from "shared"; import { vi } from "vitest"; @@ -117,7 +117,7 @@ export interface TestApp { * Plugin exports by manifest name. Nested rather than spread because `get` and * `delete` are plausible plugin names and would collide with the request methods. */ - plugins: PluginMap; + plugins: AppKitApi; /** The same object a handler resolves at runtime. */ client: WorkspaceClient; /** e.g. `http://127.0.0.1:54321`. Throws when `server: false`. */ @@ -389,7 +389,7 @@ export async function createTestApp( }; return { - plugins: bootedApp as unknown as PluginMap, + plugins: bootedApp, client, get baseUrl() { if (baseUrl === undefined) { diff --git a/packages/appkit/src/testing/test-plugin-context.ts b/packages/appkit/src/testing/test-plugin-context.ts index 037235c2b..97a433224 100644 --- a/packages/appkit/src/testing/test-plugin-context.ts +++ b/packages/appkit/src/testing/test-plugin-context.ts @@ -9,11 +9,17 @@ import { afterEach, onTestFinished } from "vitest"; import { CacheManager } from "../cache"; import { InMemoryStorage } from "../cache/storage"; +import { getCallerContext } from "../context"; +import { createRequestScope } from "../context/request-scope"; import { isToolProvider, PluginContext } from "../core/plugin-context"; -import { AuthenticationError } from "../errors"; import type { Plugin } from "../plugin"; import type { ITelemetry } from "../telemetry"; -import { applyEnv, createMockTelemetry, mockServiceContext } from "./fixtures"; +import { + applyEnv, + createMockTelemetry, + fakeUserContext, + mockServiceContext, +} from "./fixtures"; import { createMockWorkspaceClient } from "./mock-workspace-client"; /** @@ -89,17 +95,8 @@ export interface RecordedToolCall { /** The abort signal `executeTool` composed (timeout ∘ caller). */ signal?: AbortSignal; /** - * Whether the dispatch was resolved through the on-behalf-of (`asUser`) - * path. `PluginContext.executeTool` always calls `provider.asUser(req)`, so - * for a tool reached through `executeTool` this is `true` — and, because the - * fake `asUser` enforces the same token precondition as the real - * {@link Plugin.asUser}, a request with no `x-forwarded-access-token` makes - * that call **throw** rather than record `asUser: true`. The meaningful - * assertions are therefore: a well-formed request records `asUser: true` - * with {@link userId} set, and a token-less request rejects. - * - * The fake replicates the token precondition only, not the real dev-mode - * OTel `isDevOboFallback()` marker — assert OBO here, not via that flag. + * Whether dispatch ran in a caller scope, inherited or established from + * the forwarded request credentials. */ asUser: boolean; /** @@ -185,7 +182,7 @@ export interface TestPluginContext { * workspace, no OpenTelemetry pipeline, no network. * * The context is the *real* class, so route buffering, the tool registry, - * timeout composition, and the on-behalf-of (`asUser`) path all run for real. + * timeout composition, and ambient caller inheritance all run for real. * Only three edges are faked, matching the seams the class actually has: * * - **Telemetry** is a mock provider injected into the context (the one @@ -230,9 +227,15 @@ export function createTestPluginContext( return createTestPluginContextWithOptions(fakes, options); } -function createTestPluginContextSync(fakes: FakeProviders): TestPluginContext { +function createTestPluginContextSync( + fakes: FakeProviders, + client = createMockWorkspaceClient(), +): TestPluginContext { const telemetry = createMockTelemetry(); - const ctx = new PluginContext({ telemetry }); + const createCallerContext = fakeUserContext(client, { + workspaceId: Promise.resolve("test-workspace-id"), + }); + const ctx = new PluginContext({ telemetry, createCallerContext }); const toolCalls: RecordedToolCall[] = []; const routes: RecordedRoute[] = []; @@ -307,48 +310,36 @@ function createTestPluginContextSync(fakes: FakeProviders): TestPluginContext { const base: ToolProvider = { getAgentTools: () => record.tools, - executeAgentTool: (toolName, args, signal) => - resolve(toolName, args, signal, false, undefined), + executeAgentTool: (toolName, args, signal) => { + const caller = getCallerContext(); + return resolve( + toolName, + args, + signal, + !!caller, + caller?.principal.userId, + ); + }, }; - // Mirror the real `Plugin.asUser` token precondition (plugin.ts) so the - // recorded `asUser` flag reflects genuine user-scope resolution rather than - // being unconditionally true: a request with no `x-forwarded-access-token` - // throws `missingToken` (production behavior), except in development where - // the real code skips impersonation. This is edge-faking of asUser's - // *contract*, not a reimplementation of `runInUserContext`/`ServiceContext`. - // - // Deliberately NOT reproduced: the real dev-mode path sets an OTel - // `DEV_OBO_FALLBACK_KEY` marker (read by `isDevOboFallback()`). That key is - // module-private telemetry plumbing; assert OBO via the recorded - // `asUser`/`userId` fields, not `isDevOboFallback()`. + // Reuse production header validation and ALS. Only the client is faked. const asUser = (req: IAppRequest): ToolProvider => { record.asUserRequests.push(req as express.Request); - const token = (req as express.Request) - .header?.("x-forwarded-access-token") - ?.trim(); - const userId = (req as express.Request) - .header?.("x-forwarded-user") - ?.trim(); - const isDev = process.env.NODE_ENV === "development"; - - if (!token && !isDev) { - throw AuthenticationError.missingToken("user token"); - } - if (token && !userId && !isDev) { - throw AuthenticationError.missingUserId(); - } + const scope = createRequestScope( + req as express.Request, + createCallerContext, + ); return { ...base, executeAgentTool: (toolName, args, signal) => - resolve(toolName, args, signal, true, userId), + scope.run(() => base.executeAgentTool(toolName, args, signal)), }; }; - // `registerToolProvider` expects the full ToolProviderPlugin shape - // (BasePlugin & ToolProvider & { asUser }). executeTool only ever calls - // `asUser` and `executeAgentTool`; the remaining BasePlugin surface is + // executeTool calls executeAgentTool under the ambient caller scope. + // Keep asUser for older tests that call the provider directly. + // The remaining BasePlugin surface is // never touched for a registered provider, so a focused fake plus a cast // is sufficient and avoids reimplementing a plugin. const provider = { @@ -427,7 +418,7 @@ function createTestPluginContextWithOptions( const restoreEnv = applyEnv(envVars); // Create the base context (without options this time, since we're handling everything) - const base = createTestPluginContextSync(fakes); + const base = createTestPluginContextSync(fakes, client); // Restore function: restores env and service context (idempotent) let hasRestored = false; diff --git a/packages/appkit/src/testing/tests/test-plugin-context.test.ts b/packages/appkit/src/testing/tests/test-plugin-context.test.ts index fe276a90b..4a1d4d5d3 100644 --- a/packages/appkit/src/testing/tests/test-plugin-context.test.ts +++ b/packages/appkit/src/testing/tests/test-plugin-context.test.ts @@ -1,10 +1,16 @@ import type express from "express"; import { describe, expect, test, vi } from "vitest"; +import { + getCallerContext, + getCurrentPrincipalKey, + runInCallerContext, +} from "../../context"; import { PluginContext } from "../../core/plugin-context"; import { Plugin } from "../../plugin"; import type { PluginManifest } from "../../registry"; import { createMockRequest } from "../fixtures"; +import { createMockWorkspaceClient } from "../mock-workspace-client"; import { createTestPluginContext } from "../test-plugin-context"; // A minimal real plugin for exercising attach() end-to-end. @@ -33,8 +39,7 @@ class ProbePlugin extends Plugin { * timeout, route recording) rather than a reimplementation. */ -// Default to a well-formed OBO request (user token + user id) so executeTool's -// asUser path resolves. Pass `{}` explicitly to model a token-less request. +// Direct dispatch uses forwarded identity unless a caller is already open. function mockReq( headers: Record = { "x-forwarded-access-token": "user-token", @@ -61,21 +66,28 @@ describe("createTestPluginContext — construction", () => { }); }); -describe("createTestPluginContext — executeTool runs the REAL user-scoping path", () => { - test("dispatches through asUser and returns the canned static response", async () => { +function asCaller(userId: string, fn: () => T): T { + return runInCallerContext( + { + principal: { type: "user", userId }, + client: createMockWorkspaceClient(), + workspaceId: Promise.resolve("test-workspace"), + }, + fn, + ); +} + +describe("createTestPluginContext ambient tool identity", () => { + test("inherits the caller and returns the canned static response", async () => { const rows = [{ user: "alice", n: 3 }]; const mock = createTestPluginContext({ analytics: { top_users: rows } }); - const result = await mock.ctx.executeTool( - mockReq(), - "analytics", - "top_users", - { limit: 10 }, + const result = await asCaller("alice", () => + mock.ctx.executeTool(mockReq(), "analytics", "top_users", { limit: 10 }), ); expect(result).toEqual(rows); - // executeTool resolves the user scope via provider.asUser(req), and the - // fake resolves the user id from the request headers. + // The fake observes the real ambient scope, not the request headers. expect(mock.toolCalls).toHaveLength(1); expect(mock.toolCalls[0]).toMatchObject({ plugin: "analytics", @@ -84,24 +96,19 @@ describe("createTestPluginContext — executeTool runs the REAL user-scoping pat asUser: true, userId: "alice", }); - // The OBO request object is the one we passed in. - expect(mock.providers.get("analytics")?.asUserRequests).toHaveLength(1); + expect(mock.providers.get("analytics")?.asUserRequests).toHaveLength(0); }); - test("rejects a token-less request the way the real asUser does", async () => { - // The fake asUser enforces the same token precondition as Plugin.asUser, - // so a header-less request must reject rather than silently record - // asUser: true — this is what makes the OBO assertion meaningful. + test("rejects missing credentials without an ambient scope", async () => { const mock = createTestPluginContext({ analytics: { top_users: [] } }); await expect( mock.ctx.executeTool(mockReq({}), "analytics", "top_users", {}), - ).rejects.toThrow(/Missing user token/); - // The dispatch never reached the tool. - expect(mock.toolCalls).toHaveLength(0); + ).rejects.toThrow(/token/i); + expect(mock.toolCalls).toEqual([]); }); - test("rejects a request with a token but no user id", async () => { + test("rejects a token without a forwarded user", async () => { const mock = createTestPluginContext({ analytics: { top_users: [] } }); await expect( @@ -111,22 +118,79 @@ describe("createTestPluginContext — executeTool runs the REAL user-scoping pat "top_users", {}, ), - ).rejects.toThrow(/Missing user id|user id/i); - expect(mock.toolCalls).toHaveLength(0); + ).rejects.toThrow(/user/i); + expect(mock.toolCalls).toEqual([]); + }); + + test("establishes real ALS from the request and restores it after dispatch", async () => { + const mock = createTestPluginContext({ + analytics: { + identity: async () => { + await Promise.resolve(); + return getCurrentPrincipalKey(); + }, + }, + }); + await expect( + mock.ctx.executeTool(mockReq(), "analytics", "identity", {}), + ).resolves.toBe("user:alice"); + expect(mock.toolCalls[0]).toMatchObject({ asUser: true, userId: "alice" }); + expect(getCallerContext()).toBeUndefined(); + }); + + test("inherits an existing caller even without request credentials", async () => { + const mock = createTestPluginContext({ + analytics: { identity: () => getCurrentPrincipalKey() }, + }); + await expect( + asCaller("injected", () => + mock.ctx.executeTool(mockReq({}), "analytics", "identity", {}), + ), + ).resolves.toBe("user:injected"); + }); + + test("isolates concurrent request scopes and restores them after failure", async () => { + const mock = createTestPluginContext({ + analytics: { + identity: async () => { + await new Promise((resolve) => setTimeout(resolve, 1)); + const principal = getCurrentPrincipalKey(); + if (principal === "user:bob") throw new Error(principal); + return principal; + }, + }, + }); + const results = await Promise.allSettled( + ["alice", "bob"].map((userId) => + mock.ctx.executeTool( + createMockRequest({ obo: { userId } }), + "analytics", + "identity", + {}, + ), + ), + ); + expect(results).toEqual([ + { status: "fulfilled", value: "user:alice" }, + { status: "rejected", reason: new Error("user:bob") }, + ]); + expect(getCallerContext()).toBeUndefined(); }); - test("records the resolved user id so a test can assert who the tool ran as", async () => { + test("ambient identity wins over a conflicting request user", async () => { const mock = createTestPluginContext({ analytics: { top_users: [] } }); - await mock.ctx.executeTool( - mockReq({ - "x-forwarded-access-token": "tok", - "x-forwarded-user": "bob", - }), - "analytics", - "top_users", - {}, + await asCaller("alice", () => + mock.ctx.executeTool( + mockReq({ + "x-forwarded-access-token": "tok", + "x-forwarded-user": "bob", + }), + "analytics", + "top_users", + {}, + ), ); - expect(mock.toolCalls[0]).toMatchObject({ asUser: true, userId: "bob" }); + expect(mock.toolCalls[0]).toMatchObject({ asUser: true, userId: "alice" }); }); test("invokes a function response with the args and the composed signal", async () => { @@ -211,16 +275,13 @@ describe("createTestPluginContext — executeTool runs the REAL user-scoping pat describe("createTestPluginContext — asUser dev-mode branch", () => { test("in development, a token-less request is allowed through (no throw)", async () => { - // The fake asUser mirrors Plugin.asUser's dev-mode behavior: under - // NODE_ENV=development a missing token skips impersonation instead of - // throwing. The rest of the suite runs under NODE_ENV=test, so this is the - // only place that branch is exercised. + // Direct dispatch without a caller scope stays SP in development too. const prev = process.env.NODE_ENV; process.env.NODE_ENV = "development"; try { const mock = createTestPluginContext({ analytics: { top_users: [] } }); - // No forwarded headers at all — would reject in production. + // The development-only fallback is also used at the dispatch boundary. const result = await mock.ctx.executeTool( mockReq({}), "analytics", @@ -229,10 +290,9 @@ describe("createTestPluginContext — asUser dev-mode branch", () => { ); expect(result).toEqual([]); - // It still records the dispatch as an OBO call; userId is unset because - // no user header was present (dev skips impersonation, does not invent one). + // No caller was established. expect(mock.toolCalls).toHaveLength(1); - expect(mock.toolCalls[0]).toMatchObject({ asUser: true }); + expect(mock.toolCalls[0]).toMatchObject({ asUser: false }); expect(mock.toolCalls[0]?.userId).toBeUndefined(); } finally { process.env.NODE_ENV = prev; diff --git a/packages/shared/src/plugin.ts b/packages/shared/src/plugin.ts index 8c7c13c6c..1a4e1306f 100644 --- a/packages/shared/src/plugin.ts +++ b/packages/shared/src/plugin.ts @@ -251,6 +251,7 @@ export type WithAsUser = SDK extends (...args: any[]) => any ? SDK : SDK & { /** + * @deprecated Use appkit.asUser(req) instead. * Execute operations using the user's identity from the request. * Returns a user-scoped SDK where all methods execute with the * user's Databricks credentials instead of the service principal. @@ -276,6 +277,38 @@ export type PluginMap< >; }; +/** A scoped SDK cannot change the execution principal through chaining. */ +export type ScopedExports = T extends (...args: any[]) => any + ? T + : T extends Promise + ? Promise> + : T extends object + ? { + [K in keyof T as K extends `as${Capitalize}` + ? never + : K]: ScopedExports; + } + : T; + +export type ScopedPluginMap< + U extends readonly PluginData[], +> = { + [P in U[number] as P["name"]]: ScopedExports< + PluginExports> + >; +}; + +export type UserScopedApp< + U extends readonly PluginData[], +> = ScopedPluginMap & { + run(fn: (kit: ScopedPluginMap) => T | Promise): Promise; +}; + +/** App instance with plugin exports and an explicit caller-scoped entry point. */ +export type AppKitApi< + U extends readonly PluginData[], +> = PluginMap & { asUser(req: IAppRequest): UserScopedApp }; + /** Tuple of plugin class, config, and name. Created by `toPlugin()` and passed to `createApp()`. */ export type PluginData = { plugin: T; config: U; name: N }; /** Factory function type returned by `toPlugin()`. Accepts optional config and returns a PluginData tuple. */ From b6050dc1637196d33e32ea1ba441b1c9b0b368f1 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Thu, 24 Sep 2026 18:02:46 +0200 Subject: [PATCH 02/10] refactor(appkit): simplify scoped execution helpers Share the plain-object helper while preserving existing exports and object semantics. Remove redundant proxy exclusions and clarify fail-closed tool scope selection. Signed-off-by: MarioCadenas --- packages/appkit/src/context/scoped-api.ts | 14 ++---- packages/appkit/src/core/appkit.ts | 2 +- packages/appkit/src/core/plugin-context.ts | 11 +++-- packages/appkit/src/plugin/plugin.ts | 2 +- .../src/plugin/tests/asUser-proxy.test.ts | 7 +++ .../src/plugins/database/crud/contract.ts | 4 +- .../plugins/server/client-config-sanitizer.ts | 6 +-- packages/appkit/src/utils/is-plain-object.ts | 8 +++ .../src/utils/tests/is-plain-object.test.ts | 49 +++++++++++++++++++ 9 files changed, 78 insertions(+), 25 deletions(-) create mode 100644 packages/appkit/src/utils/is-plain-object.ts create mode 100644 packages/appkit/src/utils/tests/is-plain-object.test.ts diff --git a/packages/appkit/src/context/scoped-api.ts b/packages/appkit/src/context/scoped-api.ts index c4e88f3aa..53446ac34 100644 --- a/packages/appkit/src/context/scoped-api.ts +++ b/packages/appkit/src/context/scoped-api.ts @@ -1,5 +1,8 @@ +import { isPlainObject } from "../utils/is-plain-object"; import type { RequestScope } from "./request-scope"; +export { isPlainObject } from "../utils/is-plain-object"; + const EXCLUDED_FROM_PROXY = new Set([ "setup", "shutdown", @@ -9,20 +12,9 @@ const EXCLUDED_FROM_PROXY = new Set([ "getSkipBodyParsingPaths", "abortActiveOperations", "clientConfig", - "asUser", - "asApp", - "asGroup", "constructor", ]); -export function isPlainObject( - value: unknown, -): value is Record { - if (typeof value !== "object" || value === null) return false; - const prototype = Object.getPrototypeOf(value); - return prototype === Object.prototype || prototype === null; -} - function isIdentityMethod(key: PropertyKey): boolean { return typeof key === "string" && /^as[A-Z]/.test(key); } diff --git a/packages/appkit/src/core/appkit.ts b/packages/appkit/src/core/appkit.ts index ce8105a93..305996486 100644 --- a/packages/appkit/src/core/appkit.ts +++ b/packages/appkit/src/core/appkit.ts @@ -18,12 +18,12 @@ import { TelemetryReporter, } from "../internal-telemetry"; import { createLogger } from "../logging/logger"; -import { isPlainObject } from "../plugin/plugin"; import { uiVariants } from "../plugins/ui-variants"; import { ResourceRegistry, ResourceType } from "../registry"; import { getConfiguredWarehouseId } from "../resources/warehouse"; import type { TelemetryConfig } from "../telemetry"; import { TelemetryManager } from "../telemetry"; +import { isPlainObject } from "../utils/is-plain-object"; import type { WorkspaceClient } from "../workspace-client"; import { LifecycleManager } from "./lifecycle-manager"; import { isToolProvider, PluginContext } from "./plugin-context"; diff --git a/packages/appkit/src/core/plugin-context.ts b/packages/appkit/src/core/plugin-context.ts index ea3c1cd56..173fb965f 100644 --- a/packages/appkit/src/core/plugin-context.ts +++ b/packages/appkit/src/core/plugin-context.ts @@ -231,7 +231,7 @@ export class PluginContext { const tracer = this.telemetry.getTracer(); const operationName = `executeTool:${pluginName}.${toolName}`; - const execute = () => + const executeInCurrentScope = () => tracer.startActiveSpan(operationName, async (span) => { const timeoutSignal = AbortSignal.timeout(timeoutMs); const combinedSignal = signal @@ -261,11 +261,12 @@ export class PluginContext { } }); - // Direct request dispatch retains the original fail-closed OBO default. - // An existing caller wins, even when request credentials differ or are absent. + // Inherit the caller or establish request user scope before the span and tool run. return getCallerContext() - ? execute() - : createRequestScope(req, this.createCallerContext).run(execute); + ? executeInCurrentScope() + : createRequestScope(req, this.createCallerContext).run( + executeInCurrentScope, + ); } /** diff --git a/packages/appkit/src/plugin/plugin.ts b/packages/appkit/src/plugin/plugin.ts index c435327a5..474284c1b 100644 --- a/packages/appkit/src/plugin/plugin.ts +++ b/packages/appkit/src/plugin/plugin.ts @@ -44,7 +44,7 @@ import type { const logger = createLogger("plugin"); -export { isPlainObject } from "../context/scoped-api"; +export { isPlainObject } from "../utils/is-plain-object"; export { isDevOboFallback } from "../context/request-scope"; /** diff --git a/packages/appkit/src/plugin/tests/asUser-proxy.test.ts b/packages/appkit/src/plugin/tests/asUser-proxy.test.ts index 36f9e4f49..ddbda3fe0 100644 --- a/packages/appkit/src/plugin/tests/asUser-proxy.test.ts +++ b/packages/appkit/src/plugin/tests/asUser-proxy.test.ts @@ -269,6 +269,13 @@ describe("Plugin.asUser proxy", () => { // ── Real OBO: method-call proxy ──────────────────────────────────── describe("real OBO — method calls", () => { + test("blocks identity chaining on the legacy proxy", () => { + const plugin = new ProbePlugin(config); + const proxied = plugin.asUser(createReqWithObo()); + + expect(proxied.asUser).toBeUndefined(); + }); + test("async method runs inside the user's AsyncLocalStorage scope", async () => { const plugin = new ProbePlugin(config); const proxied = plugin.asUser(createReqWithObo()); diff --git a/packages/appkit/src/plugins/database/crud/contract.ts b/packages/appkit/src/plugins/database/crud/contract.ts index 97b4d884d..bdd9d9d9d 100644 --- a/packages/appkit/src/plugins/database/crud/contract.ts +++ b/packages/appkit/src/plugins/database/crud/contract.ts @@ -2,6 +2,7 @@ import { DatabasePluginError } from "../../../database/errors"; import type { Row } from "../../../database/runtime"; import type { AppKitTable } from "../../../database/schema-builder"; import { filterOperatorsForKind } from "../../../database/schema-builder/types"; +import { isPlainObject as hasPlainObjectPrototype } from "../../../utils/is-plain-object"; import { MAX_SERIALIZED_DEPTH, MAX_SERIALIZED_NODES } from "../defaults"; import { type CompiledColumn, compileColumn, type JsonValue } from "./codecs"; @@ -45,8 +46,7 @@ export function isPlainObject( if (value === null || typeof value !== "object" || Array.isArray(value)) { return false; } - const prototype = Object.getPrototypeOf(value); - return prototype === Object.prototype || prototype === null; + return hasPlainObjectPrototype(value); } /** A serializer that breaks its contract is trusted code failing, not input. */ diff --git a/packages/appkit/src/plugins/server/client-config-sanitizer.ts b/packages/appkit/src/plugins/server/client-config-sanitizer.ts index a491c3934..9a0a28c2e 100644 --- a/packages/appkit/src/plugins/server/client-config-sanitizer.ts +++ b/packages/appkit/src/plugins/server/client-config-sanitizer.ts @@ -1,6 +1,7 @@ import pc from "picocolors"; import { createLogger } from "../../logging/logger"; +import { isPlainObject } from "../../utils/is-plain-object"; const logger = createLogger("server:config"); @@ -78,11 +79,6 @@ function isSecretCoveredByPublicValue( ); } -function isPlainObject(value: object): boolean { - const proto = Object.getPrototypeOf(value); - return proto === Object.prototype || proto === null; -} - function invalidClientConfig( pluginName: string, path: string, diff --git a/packages/appkit/src/utils/is-plain-object.ts b/packages/appkit/src/utils/is-plain-object.ts new file mode 100644 index 000000000..0501386fe --- /dev/null +++ b/packages/appkit/src/utils/is-plain-object.ts @@ -0,0 +1,8 @@ +/** Test for an object whose direct prototype is Object.prototype or null. */ +export function isPlainObject( + value: unknown, +): value is Record { + if (typeof value !== "object" || value === null) return false; + const prototype = Object.getPrototypeOf(value); + return prototype === Object.prototype || prototype === null; +} diff --git a/packages/appkit/src/utils/tests/is-plain-object.test.ts b/packages/appkit/src/utils/tests/is-plain-object.test.ts new file mode 100644 index 000000000..f3ea1b663 --- /dev/null +++ b/packages/appkit/src/utils/tests/is-plain-object.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, test } from "vitest"; + +import { isPlainObject as scopedIsPlainObject } from "../../context/scoped-api"; +import { isPlainObject as pluginIsPlainObject } from "../../plugin/plugin"; +import { isPlainObject as crudIsPlainObject } from "../../plugins/database/crud/contract"; +import { isPlainObject } from "../is-plain-object"; + +describe.each([ + { name: "utility", check: isPlainObject }, + { name: "scoped API export", check: scopedIsPlainObject }, + { name: "plugin export", check: pluginIsPlainObject }, + { name: "CRUD export", check: crudIsPlainObject }, +])("$name", ({ check }) => { + test("accepts object literals and null-prototype records", () => { + expect(check({ value: 1 })).toBe(true); + expect(check(Object.create(null))).toBe(true); + }); + + test("rejects non-objects and objects with other prototypes", () => { + class Instance {} + for (const value of [ + null, + undefined, + false, + 0, + "text", + Symbol("value"), + 1n, + () => {}, + [], + new Date(), + new Map(), + new Instance(), + Object.create({ value: 1 }), + ]) { + expect(check(value)).toBe(false); + } + }); +}); + +test("CRUD keeps its explicit array rejection regardless of the prototype", () => { + for (const prototype of [Object.prototype, null]) { + const array = Object.setPrototypeOf([], prototype); + expect(isPlainObject(array)).toBe(true); + expect(scopedIsPlainObject(array)).toBe(true); + expect(pluginIsPlainObject(array)).toBe(true); + expect(crudIsPlainObject(array)).toBe(false); + } +}); From 5565c74333c479e21e454f0b28c85ef521165bda Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Fri, 25 Sep 2026 10:28:36 +0200 Subject: [PATCH 03/10] docs(appkit): sync user scope API reference Signed-off-by: MarioCadenas --- docs/docs/api/appkit/Class.Plugin.md | 19 +- docs/docs/api/appkit/Class.ServiceContext.md | 165 ++++++++++++++++++ docs/docs/api/appkit/Function.createApp.md | 10 +- .../api/appkit/Function.getCallerContext.md | 13 ++ .../api/appkit/Function.getCurrentUserId.md | 14 ++ .../appkit/Function.getExecutionContext.md | 2 +- .../api/appkit/Function.getUserContext.md | 16 ++ .../api/appkit/Function.isInUserContext.md | 11 ++ .../docs/api/appkit/Function.isUserContext.md | 20 +++ .../api/appkit/Function.runInCallerContext.md | 27 +++ .../api/appkit/Function.runInUserContext.md | 28 +++ .../api/appkit/Interface.CallerContext.md | 12 ++ docs/docs/api/appkit/TypeAlias.AppKitApi.md | 33 ++++ .../api/appkit/TypeAlias.ExecutionContext.md | 5 +- .../api/appkit/TypeAlias.ScopedPluginMap.md | 11 ++ docs/docs/api/appkit/TypeAlias.UserContext.md | 101 +++++++++++ .../api/appkit/TypeAlias.UserScopedApp.md | 37 ++++ docs/docs/api/appkit/index.md | 12 ++ docs/docs/api/appkit/typedoc-sidebar.ts | 65 +++++++ 19 files changed, 581 insertions(+), 20 deletions(-) create mode 100644 docs/docs/api/appkit/Class.ServiceContext.md create mode 100644 docs/docs/api/appkit/Function.getCallerContext.md create mode 100644 docs/docs/api/appkit/Function.getCurrentUserId.md create mode 100644 docs/docs/api/appkit/Function.getUserContext.md create mode 100644 docs/docs/api/appkit/Function.isInUserContext.md create mode 100644 docs/docs/api/appkit/Function.isUserContext.md create mode 100644 docs/docs/api/appkit/Function.runInCallerContext.md create mode 100644 docs/docs/api/appkit/Function.runInUserContext.md create mode 100644 docs/docs/api/appkit/TypeAlias.AppKitApi.md create mode 100644 docs/docs/api/appkit/TypeAlias.ScopedPluginMap.md create mode 100644 docs/docs/api/appkit/TypeAlias.UserContext.md create mode 100644 docs/docs/api/appkit/TypeAlias.UserScopedApp.md diff --git a/docs/docs/api/appkit/Class.Plugin.md b/docs/docs/api/appkit/Class.Plugin.md index b4f9e5221..a783c2c92 100644 --- a/docs/docs/api/appkit/Class.Plugin.md +++ b/docs/docs/api/appkit/Class.Plugin.md @@ -228,32 +228,25 @@ BasePlugin.abortActiveOperations *** -### asUser() +### ~~asUser()~~ ```ts asUser(req: Request): this; ``` -Execute operations using the user's identity from the request. -Returns a proxy of this plugin where all method calls execute -with the user's Databricks credentials instead of the service principal. - #### Parameters -| Parameter | Type | Description | -| ------ | ------ | ------ | -| `req` | `Request` | The Express request containing the user token in headers | +| Parameter | Type | +| ------ | ------ | +| `req` | `Request` | #### Returns `this` -A proxied plugin instance that executes as the user - -#### Throws +#### Deprecated -AuthenticationError if user token is not available in request headers (production only). - In development mode (`NODE_ENV=development`), skips user impersonation instead of throwing. +Use appkit.asUser(req) to scope the whole app. *** diff --git a/docs/docs/api/appkit/Class.ServiceContext.md b/docs/docs/api/appkit/Class.ServiceContext.md new file mode 100644 index 000000000..7ca9d1f92 --- /dev/null +++ b/docs/docs/api/appkit/Class.ServiceContext.md @@ -0,0 +1,165 @@ +# Class: ServiceContext + +ServiceContext is a singleton that manages the service principal's +WorkspaceClient and workspace ID. WarehouseResource owns warehouse bindings. + +It's initialized once at app startup and provides the foundation +for both service principal and user context execution. + +## Constructors + +### Constructor + +```ts +new ServiceContext(): ServiceContext; +``` + +#### Returns + +`ServiceContext` + +## Methods + +### createCallerContext() + +```ts +static createCallerContext( + token: string, + userId: string, + userName?: string, + userEmail?: string): CallerContext; +``` + +Create an immutable caller context from the existing user request headers. + +#### Parameters + +| Parameter | Type | Description | +| ------ | ------ | ------ | +| `token` | `string` | The user's access token from x-forwarded-access-token header | +| `userId` | `string` | The user's ID from x-forwarded-user header | +| `userName?` | `string` | Optional user name | +| `userEmail?` | `string` | Optional email from x-forwarded-email | + +#### Returns + +[`CallerContext`](Interface.CallerContext.md) + +#### Throws + +Error if token is not provided + +*** + +### ~~createUserContext()~~ + +```ts +static createUserContext( + token: string, + userId: string, + userName?: string, + userEmail?: string): CallerContext & UserContext; +``` + +#### Parameters + +| Parameter | Type | +| ------ | ------ | +| `token` | `string` | +| `userId` | `string` | +| `userName?` | `string` | +| `userEmail?` | `string` | + +#### Returns + +[`CallerContext`](Interface.CallerContext.md) & [`UserContext`](TypeAlias.UserContext.md) + +#### Deprecated + +Use ServiceContext.createCallerContext. + +*** + +### get() + +```ts +static get(): ServiceContextState; +``` + +Get the initialized service context. + +#### Returns + +`ServiceContextState` + +#### Throws + +Error if not initialized + +*** + +### getClientOptions() + +```ts +static getClientOptions(): ClientOptions; +``` + +Get the client options for WorkspaceClient. +Exposed for testing purposes. + +#### Returns + +`ClientOptions` + +*** + +### initialize() + +```ts +static initialize(options?: { + warehouseId?: boolean; +}, client?: WorkspaceClient): Promise; +``` + +Initialize the service context. Should be called once at app startup. +Safe to call multiple times - will return the same instance. + +#### Parameters + +| Parameter | Type | Description | +| ------ | ------ | ------ | +| `options?` | \{ `warehouseId?`: `boolean`; \} | Which shared resources to resolve (derived from plugin manifests). | +| `options.warehouseId?` | `boolean` | - | +| `client?` | [`WorkspaceClient`](Interface.WorkspaceClient.md) | Optional pre-configured WorkspaceClient to use instead of creating one from environment credentials. | + +#### Returns + +`Promise`\<`ServiceContextState`\> + +*** + +### isInitialized() + +```ts +static isInitialized(): boolean; +``` + +Check if the service context has been initialized. + +#### Returns + +`boolean` + +*** + +### reset() + +```ts +static reset(): void; +``` + +Reset the service context. Only for testing purposes. + +#### Returns + +`void` diff --git a/docs/docs/api/appkit/Function.createApp.md b/docs/docs/api/appkit/Function.createApp.md index b7cd2fc3a..1611d5aa8 100644 --- a/docs/docs/api/appkit/Function.createApp.md +++ b/docs/docs/api/appkit/Function.createApp.md @@ -5,10 +5,10 @@ function createApp(config: { cache?: CacheConfig; client?: WorkspaceClient; disableInternalTelemetry?: boolean; - onPluginsReady?: (appkit: PluginMap) => void | Promise; + onPluginsReady?: (appkit: AppKitApi) => void | Promise; plugins?: T; telemetry?: TelemetryConfig; -}): Promise>; +}): Promise>; ``` Bootstraps AppKit with the provided configuration. @@ -31,17 +31,17 @@ with an `asUser(req)` method for user-scoped execution. | Parameter | Type | Description | | ------ | ------ | ------ | -| `config` | \{ `cache?`: [`CacheConfig`](Interface.CacheConfig.md); `client?`: [`WorkspaceClient`](Interface.WorkspaceClient.md); `disableInternalTelemetry?`: `boolean`; `onPluginsReady?`: (`appkit`: `PluginMap`\<`T`\>) => `void` \| `Promise`\<`void`\>; `plugins?`: `T`; `telemetry?`: [`TelemetryConfig`](Interface.TelemetryConfig.md); \} | - | +| `config` | \{ `cache?`: [`CacheConfig`](Interface.CacheConfig.md); `client?`: [`WorkspaceClient`](Interface.WorkspaceClient.md); `disableInternalTelemetry?`: `boolean`; `onPluginsReady?`: (`appkit`: [`AppKitApi`](TypeAlias.AppKitApi.md)\<`T`\>) => `void` \| `Promise`\<`void`\>; `plugins?`: `T`; `telemetry?`: [`TelemetryConfig`](Interface.TelemetryConfig.md); \} | - | | `config.cache?` | [`CacheConfig`](Interface.CacheConfig.md) | - | | `config.client?` | [`WorkspaceClient`](Interface.WorkspaceClient.md) | - | | `config.disableInternalTelemetry?` | `boolean` | - | -| `config.onPluginsReady?` | (`appkit`: `PluginMap`\<`T`\>) => `void` \| `Promise`\<`void`\> | Runs after plugin setup but **before** the server starts. | +| `config.onPluginsReady?` | (`appkit`: [`AppKitApi`](TypeAlias.AppKitApi.md)\<`T`\>) => `void` \| `Promise`\<`void`\> | Runs after plugin setup but **before** the server starts. | | `config.plugins?` | `T` | - | | `config.telemetry?` | [`TelemetryConfig`](Interface.TelemetryConfig.md) | - | ## Returns -`Promise`\<`PluginMap`\<`T`\>\> +`Promise`\<[`AppKitApi`](TypeAlias.AppKitApi.md)\<`T`\>\> A `PluginMap` keyed by plugin name with typed exports diff --git a/docs/docs/api/appkit/Function.getCallerContext.md b/docs/docs/api/appkit/Function.getCallerContext.md new file mode 100644 index 000000000..807844ddb --- /dev/null +++ b/docs/docs/api/appkit/Function.getCallerContext.md @@ -0,0 +1,13 @@ +# Function: getCallerContext() + +```ts +function getCallerContext(): CallerContext | undefined; +``` + +Get the caller context if one is active, otherwise `undefined`. +Unlike `getExecutionContext()`, this does not require `ServiceContext` +to be initialized and never throws. + +## Returns + +[`CallerContext`](Interface.CallerContext.md) \| `undefined` diff --git a/docs/docs/api/appkit/Function.getCurrentUserId.md b/docs/docs/api/appkit/Function.getCurrentUserId.md new file mode 100644 index 000000000..477338729 --- /dev/null +++ b/docs/docs/api/appkit/Function.getCurrentUserId.md @@ -0,0 +1,14 @@ +# ~~Function: getCurrentUserId()~~ + +```ts +function getCurrentUserId(): string; +``` + +## Returns + +`string` + +## Deprecated + +Use getCurrentPrincipalKey for new cache keys or getCurrentActorId +for audit. Preserves the bare user or service ID for existing callers. diff --git a/docs/docs/api/appkit/Function.getExecutionContext.md b/docs/docs/api/appkit/Function.getExecutionContext.md index 14751649d..54dd7f307 100644 --- a/docs/docs/api/appkit/Function.getExecutionContext.md +++ b/docs/docs/api/appkit/Function.getExecutionContext.md @@ -14,7 +14,7 @@ Get the current execution context. ## Returns \| `ServiceContextState` - \| [`CallerContext`](Interface.CallerContext.md) & `UserContext` + \| [`CallerContext`](Interface.CallerContext.md) & [`UserContext`](TypeAlias.UserContext.md) ## Throws diff --git a/docs/docs/api/appkit/Function.getUserContext.md b/docs/docs/api/appkit/Function.getUserContext.md new file mode 100644 index 000000000..3bcbdaca3 --- /dev/null +++ b/docs/docs/api/appkit/Function.getUserContext.md @@ -0,0 +1,16 @@ +# ~~Function: getUserContext()~~ + +```ts +function getUserContext(): + | CallerContext & UserContext + | undefined; +``` + +## Returns + + \| [`CallerContext`](Interface.CallerContext.md) & [`UserContext`](TypeAlias.UserContext.md) + \| `undefined` + +## Deprecated + +Use getCallerContext and its principal field. diff --git a/docs/docs/api/appkit/Function.isInUserContext.md b/docs/docs/api/appkit/Function.isInUserContext.md new file mode 100644 index 000000000..3e61bc19f --- /dev/null +++ b/docs/docs/api/appkit/Function.isInUserContext.md @@ -0,0 +1,11 @@ +# Function: isInUserContext() + +```ts +function isInUserContext(): boolean; +``` + +Check if currently running in a user context. + +## Returns + +`boolean` diff --git a/docs/docs/api/appkit/Function.isUserContext.md b/docs/docs/api/appkit/Function.isUserContext.md new file mode 100644 index 000000000..63d692ce5 --- /dev/null +++ b/docs/docs/api/appkit/Function.isUserContext.md @@ -0,0 +1,20 @@ +# ~~Function: isUserContext()~~ + +```ts +function isUserContext(ctx: ExecutionContext): ctx is UserContext & Partial; +``` + +## Parameters + +| Parameter | Type | +| ------ | ------ | +| `ctx` | [`ExecutionContext`](TypeAlias.ExecutionContext.md) | + +## Returns + +`ctx is UserContext & Partial` + +## Deprecated + +Use isCallerContext. Active caller contexts retain the legacy +identity accessors for callers narrowed by this guard. diff --git a/docs/docs/api/appkit/Function.runInCallerContext.md b/docs/docs/api/appkit/Function.runInCallerContext.md new file mode 100644 index 000000000..b3279f0c2 --- /dev/null +++ b/docs/docs/api/appkit/Function.runInCallerContext.md @@ -0,0 +1,27 @@ +# Function: runInCallerContext() + +```ts +function runInCallerContext(callerContext: CallerContext, fn: () => T): T; +``` + +Run a function with an immutable snapshot of the caller context. +Nested and concurrent scopes keep their own identities. + +## Type Parameters + +| Type Parameter | +| ------ | +| `T` | + +## Parameters + +| Parameter | Type | Description | +| ------ | ------ | ------ | +| `callerContext` | [`CallerContext`](Interface.CallerContext.md) | The caller context to use | +| `fn` | () => `T` | The function to run | + +## Returns + +`T` + +The result of the function diff --git a/docs/docs/api/appkit/Function.runInUserContext.md b/docs/docs/api/appkit/Function.runInUserContext.md new file mode 100644 index 000000000..6998cfbe4 --- /dev/null +++ b/docs/docs/api/appkit/Function.runInUserContext.md @@ -0,0 +1,28 @@ +# ~~Function: runInUserContext()~~ + +```ts +function runInUserContext(userContext: + | UserContext + | CallerContext & Pick, fn: () => T): T; +``` + +## Type Parameters + +| Type Parameter | +| ------ | +| `T` | + +## Parameters + +| Parameter | Type | +| ------ | ------ | +| `userContext` | \| [`UserContext`](TypeAlias.UserContext.md) \| [`CallerContext`](Interface.CallerContext.md) & `Pick`\<[`UserContext`](TypeAlias.UserContext.md), `"warehouseId"`\> | +| `fn` | () => `T` | + +## Returns + +`T` + +## Deprecated + +Use runInCallerContext. diff --git a/docs/docs/api/appkit/Interface.CallerContext.md b/docs/docs/api/appkit/Interface.CallerContext.md index f9890e18a..d12322176 100644 --- a/docs/docs/api/appkit/Interface.CallerContext.md +++ b/docs/docs/api/appkit/Interface.CallerContext.md @@ -30,6 +30,18 @@ Truncated SHA-256 hash of the caller token, used to detect rotation. *** +### ~~warehouseId?~~ + +```ts +readonly optional warehouseId: Promise; +``` + +#### Deprecated + +Use getWarehouseId(). Only legacy context access exposes this field. + +*** + ### workspaceId ```ts diff --git a/docs/docs/api/appkit/TypeAlias.AppKitApi.md b/docs/docs/api/appkit/TypeAlias.AppKitApi.md new file mode 100644 index 000000000..21f3cd4c0 --- /dev/null +++ b/docs/docs/api/appkit/TypeAlias.AppKitApi.md @@ -0,0 +1,33 @@ +# Type Alias: AppKitApi\ + +```ts +type AppKitApi = PluginMap & { + asUser: UserScopedApp; +}; +``` + +App instance with plugin exports and an explicit caller-scoped entry point. + +## Type Declaration + +### asUser() + +```ts +asUser(req: IAppRequest): UserScopedApp; +``` + +#### Parameters + +| Parameter | Type | +| ------ | ------ | +| `req` | `IAppRequest` | + +#### Returns + +[`UserScopedApp`](TypeAlias.UserScopedApp.md)\<`U`\> + +## Type Parameters + +| Type Parameter | +| ------ | +| `U` *extends* readonly [`PluginData`](TypeAlias.PluginData.md)\<`PluginConstructor`, `unknown`, `string`\>[] | diff --git a/docs/docs/api/appkit/TypeAlias.ExecutionContext.md b/docs/docs/api/appkit/TypeAlias.ExecutionContext.md index 4cf921385..3ba93d701 100644 --- a/docs/docs/api/appkit/TypeAlias.ExecutionContext.md +++ b/docs/docs/api/appkit/TypeAlias.ExecutionContext.md @@ -1,5 +1,8 @@ # Type Alias: ExecutionContext ```ts -type ExecutionContext = ServiceContextState | CallerContext; +type ExecutionContext = + | ServiceContextState + | CallerContext + | UserContext; ``` diff --git a/docs/docs/api/appkit/TypeAlias.ScopedPluginMap.md b/docs/docs/api/appkit/TypeAlias.ScopedPluginMap.md new file mode 100644 index 000000000..e512b8a84 --- /dev/null +++ b/docs/docs/api/appkit/TypeAlias.ScopedPluginMap.md @@ -0,0 +1,11 @@ +# Type Alias: ScopedPluginMap\ + +```ts +type ScopedPluginMap = { [P in U[number] as P["name"]]: ScopedExports>> }; +``` + +## Type Parameters + +| Type Parameter | +| ------ | +| `U` *extends* readonly [`PluginData`](TypeAlias.PluginData.md)\<`PluginConstructor`, `unknown`, `string`\>[] | diff --git a/docs/docs/api/appkit/TypeAlias.UserContext.md b/docs/docs/api/appkit/TypeAlias.UserContext.md new file mode 100644 index 000000000..b4784ebe7 --- /dev/null +++ b/docs/docs/api/appkit/TypeAlias.UserContext.md @@ -0,0 +1,101 @@ +# ~~Type Alias: UserContext~~ + +```ts +type UserContext = { + client: ServiceContextState["client"]; + isUserContext: true; + tokenFingerprint?: string; + userEmail?: string; + userId: string; + userName?: string; + warehouseId?: Promise; + workspaceId: Promise; +}; +``` + +## Deprecated + +Use CallerContext and its principal field. Kept for callers +that construct the legacy shape or read its flat identity fields. + +## Properties + +### ~~client~~ + +```ts +client: ServiceContextState["client"]; +``` + +WorkspaceClient authenticated as the user + +*** + +### ~~isUserContext~~ + +```ts +isUserContext: true; +``` + +Flag indicating this is a user context + +*** + +### ~~tokenFingerprint?~~ + +```ts +optional tokenFingerprint: string; +``` + +Truncated SHA-256 hash of the user's OBO token, used to detect token rotation + +*** + +### ~~userEmail?~~ + +```ts +optional userEmail: string; +``` + +The user's email (from `x-forwarded-email` header) + +*** + +### ~~userId~~ + +```ts +userId: string; +``` + +The user's ID (from request headers) + +*** + +### ~~userName?~~ + +```ts +optional userName: string; +``` + +The user's name (from request headers) + +*** + +### ~~warehouseId?~~ + +```ts +optional warehouseId: Promise; +``` + +#### Deprecated + +Use getWarehouseId() from @databricks/appkit. + +*** + +### ~~workspaceId~~ + +```ts +workspaceId: Promise; +``` + +Promise that resolves to the workspace ID (inherited from service context) diff --git a/docs/docs/api/appkit/TypeAlias.UserScopedApp.md b/docs/docs/api/appkit/TypeAlias.UserScopedApp.md new file mode 100644 index 000000000..54155f4f6 --- /dev/null +++ b/docs/docs/api/appkit/TypeAlias.UserScopedApp.md @@ -0,0 +1,37 @@ +# Type Alias: UserScopedApp\ + +```ts +type UserScopedApp = ScopedPluginMap & { + run: Promise; +}; +``` + +## Type Declaration + +### run() + +```ts +run(fn: (kit: ScopedPluginMap) => T | Promise): Promise; +``` + +#### Type Parameters + +| Type Parameter | +| ------ | +| `T` | + +#### Parameters + +| Parameter | Type | +| ------ | ------ | +| `fn` | (`kit`: [`ScopedPluginMap`](TypeAlias.ScopedPluginMap.md)\<`U`\>) => `T` \| `Promise`\<`T`\> | + +#### Returns + +`Promise`\<`T`\> + +## Type Parameters + +| Type Parameter | +| ------ | +| `U` *extends* readonly [`PluginData`](TypeAlias.PluginData.md)\<`PluginConstructor`, `unknown`, `string`\>[] | diff --git a/docs/docs/api/appkit/index.md b/docs/docs/api/appkit/index.md index a3271ef5f..2c2fba246 100644 --- a/docs/docs/api/appkit/index.md +++ b/docs/docs/api/appkit/index.md @@ -28,6 +28,7 @@ surface with `@databricks/appkit/beta`. Not meant for application imports. | [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. | | [ServerError](Class.ServerError.md) | Error thrown when server lifecycle operations fail. Use for server start/stop issues, configuration conflicts, etc. | +| [ServiceContext](Class.ServiceContext.md) | ServiceContext is a singleton that manages the service principal's WorkspaceClient and workspace ID. WarehouseResource owns warehouse bindings. | | [SupervisorApiAdapter](Class.SupervisorApiAdapter.md) | Adapter that calls the Databricks AI Gateway Responses API (`/ai-gateway/mlflow/v1/responses`). | | [TunnelError](Class.TunnelError.md) | Error thrown when remote tunnel operations fail. Use for tunnel connection issues, message parsing failures, etc. | | [ValidationError](Class.ValidationError.md) | Error thrown when input validation fails. Use for invalid parameters, missing required fields, or type mismatches. | @@ -140,6 +141,7 @@ surface with `@databricks/appkit/beta`. Not meant for application imports. | [AgentTool](TypeAlias.AgentTool.md) | Any tool an agent can invoke: inline function tools (`tool()`), hosted MCP tools (`mcpServer()` / raw hosted), toolkit references from plugins (`analytics().toolkit()`), or adapter-hosted Supervisor-API tools (`supervisorTools.*`). | | [AgentTools](TypeAlias.AgentTools.md) | Per-agent tool record. String keys map to inline tools, toolkit entries, hosted tools, etc. | | [AgentToolsFn](TypeAlias.AgentToolsFn.md) | Function form of `AgentDefinition.tools`. Receives the typed [Plugins](TypeAlias.Plugins.md) map and returns a tool record. Invoked exactly once at setup (or once per `runAgent` call in standalone mode); the result is cached as the agent's resolved tool record. | +| [AppKitApi](TypeAlias.AppKitApi.md) | App instance with plugin exports and an explicit caller-scoped entry point. | | [BaseSystemPromptOption](TypeAlias.BaseSystemPromptOption.md) | - | | [CallerPrincipal](TypeAlias.CallerPrincipal.md) | The caller identity whose permissions authorize execution, not its resources. | | [ConfigSchema](TypeAlias.ConfigSchema.md) | Configuration schema definition for plugin config. Re-exported from the standard JSON Schema Draft 7 types. | @@ -165,6 +167,7 @@ surface with `@databricks/appkit/beta`. Not meant for application imports. | [ResolvedToolEntry](TypeAlias.ResolvedToolEntry.md) | Internal tool-index entry after a tool record has been resolved to a dispatchable form. | | [ResourceFieldEntry](TypeAlias.ResourceFieldEntry.md) | - | | [ResourcePermission](TypeAlias.ResourcePermission.md) | Union of all possible permission levels across all resource types. | +| [ScopedPluginMap](TypeAlias.ScopedPluginMap.md) | - | | [SearchFilters](TypeAlias.SearchFilters.md) | - | | [ServingFactory](TypeAlias.ServingFactory.md) | Factory function returned by `AppKit.serving`. | | [Severity](TypeAlias.Severity.md) | Whether an assertion fails the eval (`gate`) or is tracked only (`soft`). | @@ -172,6 +175,8 @@ surface with `@databricks/appkit/beta`. Not meant for application imports. | [ToolRegistry](TypeAlias.ToolRegistry.md) | - | | [ToPlugin](TypeAlias.ToPlugin.md) | Factory function type returned by `toPlugin()`. Accepts optional config and returns a PluginData tuple. | | [TransactionClient](TypeAlias.TransactionClient.md) | Entity and SQL capabilities bound to one transaction. | +| [~~UserContext~~](TypeAlias.UserContext.md) | - | +| [UserScopedApp](TypeAlias.UserScopedApp.md) | - | ## Variables @@ -228,13 +233,16 @@ surface with `@databricks/appkit/beta`. Not meant for application imports. | [fromSupervisorApi](Function.fromSupervisorApi.md) | Creates an [AgentAdapter](Interface.AgentAdapter.md) backed by the Databricks AI Gateway Responses API (`/ai-gateway/mlflow/v1/responses`). | | [functionToolToDefinition](Function.functionToolToDefinition.md) | - | | [generateDatabaseCredential](Function.generateDatabaseCredential.md) | Generate OAuth credentials for Postgres database connection using the proper Postgres API. | +| [getCallerContext](Function.getCallerContext.md) | Get the caller context if one is active, otherwise `undefined`. Unlike `getExecutionContext()`, this does not require `ServiceContext` to be initialized and never throws. | | [getCurrentActorId](Function.getCurrentActorId.md) | The initiating user in a caller scope; no user actor exists in service scope. | | [getCurrentPrincipalKey](Function.getCurrentPrincipalKey.md) | Get the principal key for future cache keying: `app` or `user:`. | +| [~~getCurrentUserId~~](Function.getCurrentUserId.md) | - | | [getExecutionContext](Function.getExecutionContext.md) | Get the current execution context. | | [getLakebaseOrmConfig](Function.getLakebaseOrmConfig.md) | Get Lakebase connection configuration for ORMs that don't accept pg.Pool directly. | | [getLakebasePgConfig](Function.getLakebasePgConfig.md) | Get Lakebase connection configuration for PostgreSQL clients. | | [getPluginManifest](Function.getPluginManifest.md) | Loads and validates the manifest from a plugin constructor. Normalizes string type/permission to strict ResourceType/ResourcePermission. | | [getResourceRequirements](Function.getResourceRequirements.md) | Gets the resource requirements from a plugin's manifest. | +| [~~getUserContext~~](Function.getUserContext.md) | - | | [getUsernameWithApiLookup](Function.getUsernameWithApiLookup.md) | Resolves the PostgreSQL username for a Lakebase connection. | | [getWarehouseId](Function.getWarehouseId.md) | Get the configured SQL warehouse ID after app initialization. The warehouse is an app resource; SP and caller executions use the same binding. Deprecated user-context scopes retain support for explicit warehouse overrides. | | [getWorkspaceClient](Function.getWorkspaceClient.md) | Get workspace client from config or SDK default auth chain | @@ -243,10 +251,12 @@ surface with `@databricks/appkit/beta`. Not meant for application imports. | [integer](Function.integer.md) | - | | [isFunctionTool](Function.isFunctionTool.md) | - | | [isHostedTool](Function.isHostedTool.md) | - | +| [isInUserContext](Function.isInUserContext.md) | Check if currently running in a user context. | | [isJudgeConfigured](Function.isJudgeConfigured.md) | - | | [isSQLTypeMarker](Function.isSQLTypeMarker.md) | Type guard to check if a value is a SQL type marker | | [isSupervisorTool](Function.isSupervisorTool.md) | Type guard for [HostedSupervisorTool](Interface.HostedSupervisorTool.md). Used by the agents plugin (`buildToolIndex`) and standalone `runAgent` (`classifyTool`) to route supervisor-hosted tools to the extensions payload rather than the adapter's `tools` array. | | [isToolkitEntry](Function.isToolkitEntry.md) | Type guard for `ToolkitEntry` — used by the agents plugin to differentiate toolkit references from inline tools in a mixed `tools` record. | +| [~~isUserContext~~](Function.isUserContext.md) | - | | [jsonb](Function.jsonb.md) | - | | [loadAgentFromFile](Function.loadAgentFromFile.md) | Loads a single markdown agent file and resolves its frontmatter against registered plugin toolkits + ambient tool library. | | [loadAgentsFromDir](Function.loadAgentsFromDir.md) | Scans a directory for one subdirectory per agent, each containing `agent.md` (frontmatter + body). Produces an `AgentDefinition` record keyed by agent id (folder name). Throws on frontmatter errors or unresolved references. Returns an empty map if the directory does not exist. | @@ -263,6 +273,8 @@ surface with `@databricks/appkit/beta`. Not meant for application imports. | [runAgent](Function.runAgent.md) | Standalone agent execution without `createApp`. Resolves the adapter, binds inline tools, and drives the adapter's `run()` loop to completion. | | [runEval](Function.runEval.md) | Run a single eval against a driver. Never throws for assertion or agent failures — those become a non-passing [EvalResult](Interface.EvalResult.md). Only a malformed eval definition surfaces as `result.error`. | | [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) | - | | [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 ecfdaa3f4..8da3816b6 100644 --- a/docs/docs/api/appkit/typedoc-sidebar.ts +++ b/docs/docs/api/appkit/typedoc-sidebar.ts @@ -91,6 +91,11 @@ const typedocSidebar: SidebarsConfig = { id: "api/appkit/Class.ServerError", label: "ServerError" }, + { + type: "doc", + id: "api/appkit/Class.ServiceContext", + label: "ServiceContext" + }, { type: "doc", id: "api/appkit/Class.SupervisorApiAdapter", @@ -613,6 +618,11 @@ const typedocSidebar: SidebarsConfig = { id: "api/appkit/TypeAlias.AgentToolsFn", label: "AgentToolsFn" }, + { + type: "doc", + id: "api/appkit/TypeAlias.AppKitApi", + label: "AppKitApi" + }, { type: "doc", id: "api/appkit/TypeAlias.BaseSystemPromptOption", @@ -739,6 +749,11 @@ const typedocSidebar: SidebarsConfig = { id: "api/appkit/TypeAlias.ResourcePermission", label: "ResourcePermission" }, + { + type: "doc", + id: "api/appkit/TypeAlias.ScopedPluginMap", + label: "ScopedPluginMap" + }, { type: "doc", id: "api/appkit/TypeAlias.SearchFilters", @@ -773,6 +788,17 @@ const typedocSidebar: SidebarsConfig = { type: "doc", id: "api/appkit/TypeAlias.TransactionClient", label: "TransactionClient" + }, + { + type: "doc", + id: "api/appkit/TypeAlias.UserContext", + label: "UserContext", + className: "typedoc-sidebar-item-deprecated" + }, + { + type: "doc", + id: "api/appkit/TypeAlias.UserScopedApp", + label: "UserScopedApp" } ] }, @@ -1016,6 +1042,11 @@ const typedocSidebar: SidebarsConfig = { id: "api/appkit/Function.generateDatabaseCredential", label: "generateDatabaseCredential" }, + { + type: "doc", + id: "api/appkit/Function.getCallerContext", + label: "getCallerContext" + }, { type: "doc", id: "api/appkit/Function.getCurrentActorId", @@ -1026,6 +1057,12 @@ const typedocSidebar: SidebarsConfig = { id: "api/appkit/Function.getCurrentPrincipalKey", label: "getCurrentPrincipalKey" }, + { + type: "doc", + id: "api/appkit/Function.getCurrentUserId", + label: "getCurrentUserId", + className: "typedoc-sidebar-item-deprecated" + }, { type: "doc", id: "api/appkit/Function.getExecutionContext", @@ -1051,6 +1088,12 @@ const typedocSidebar: SidebarsConfig = { id: "api/appkit/Function.getResourceRequirements", label: "getResourceRequirements" }, + { + type: "doc", + id: "api/appkit/Function.getUserContext", + label: "getUserContext", + className: "typedoc-sidebar-item-deprecated" + }, { type: "doc", id: "api/appkit/Function.getUsernameWithApiLookup", @@ -1091,6 +1134,11 @@ const typedocSidebar: SidebarsConfig = { id: "api/appkit/Function.isHostedTool", label: "isHostedTool" }, + { + type: "doc", + id: "api/appkit/Function.isInUserContext", + label: "isInUserContext" + }, { type: "doc", id: "api/appkit/Function.isJudgeConfigured", @@ -1111,6 +1159,12 @@ const typedocSidebar: SidebarsConfig = { id: "api/appkit/Function.isToolkitEntry", label: "isToolkitEntry" }, + { + type: "doc", + id: "api/appkit/Function.isUserContext", + label: "isUserContext", + className: "typedoc-sidebar-item-deprecated" + }, { type: "doc", id: "api/appkit/Function.jsonb", @@ -1191,6 +1245,17 @@ const typedocSidebar: SidebarsConfig = { id: "api/appkit/Function.runEvalsInDir", label: "runEvalsInDir" }, + { + type: "doc", + id: "api/appkit/Function.runInCallerContext", + label: "runInCallerContext" + }, + { + type: "doc", + id: "api/appkit/Function.runInUserContext", + label: "runInUserContext", + className: "typedoc-sidebar-item-deprecated" + }, { type: "doc", id: "api/appkit/Function.runWithRetries", From 41e6d8dab83a02a16e2acf6bc6d090ceb4172e76 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Tue, 29 Sep 2026 18:01:06 +0200 Subject: [PATCH 04/10] docs(appkit): sync manifest warehouse selection reference Signed-off-by: MarioCadenas --- docs/docs/api/appkit/Class.ServiceContext.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/docs/api/appkit/Class.ServiceContext.md b/docs/docs/api/appkit/Class.ServiceContext.md index 7ca9d1f92..1935b45af 100644 --- a/docs/docs/api/appkit/Class.ServiceContext.md +++ b/docs/docs/api/appkit/Class.ServiceContext.md @@ -117,7 +117,7 @@ Exposed for testing purposes. ```ts static initialize(options?: { - warehouseId?: boolean; + warehouseId?: string | boolean; }, client?: WorkspaceClient): Promise; ``` @@ -128,8 +128,8 @@ Safe to call multiple times - will return the same instance. | Parameter | Type | Description | | ------ | ------ | ------ | -| `options?` | \{ `warehouseId?`: `boolean`; \} | Which shared resources to resolve (derived from plugin manifests). | -| `options.warehouseId?` | `boolean` | - | +| `options?` | \{ `warehouseId?`: `string` \| `boolean`; \} | A resolved warehouse ID, or a boolean enabling discovery. | +| `options.warehouseId?` | `string` \| `boolean` | - | | `client?` | [`WorkspaceClient`](Interface.WorkspaceClient.md) | Optional pre-configured WorkspaceClient to use instead of creating one from environment credentials. | #### Returns From 4941a73df74d28b366dd050b8973ddfeb74c3e2b Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Thu, 1 Oct 2026 11:10:35 +0200 Subject: [PATCH 05/10] fix(appkit): keep hand-rolled agent tools on the app principal The agents HTTP routes wrapped the whole run in user scope, so hand-rolled tool({ execute }) calls ran as the user. On main they run as the app service principal, and an app relying on that would break. User scope now applies only where plugin-toolkit tools dispatch: executeTool opens it per call and still rejects without user credentials, so a plugin tool never silently runs as the service principal. The model call and hand-rolled tools run in the app context, as on main. Sub-agent tool calls go through the same dispatch. Tests cover all three execution routes and sub-agents. Docs describe hand-rolled tools as running as the app service principal. Co-authored-by: Isaac Signed-off-by: MarioCadenas --- docs/docs/plugins/execution-context.md | 16 +-- .../src/core/tests/appkit-user-scope.test.ts | 104 +++++++++++++++--- packages/appkit/src/plugins/agents/agents.ts | 8 +- 3 files changed, 102 insertions(+), 26 deletions(-) diff --git a/docs/docs/plugins/execution-context.md b/docs/docs/plugins/execution-context.md index 0f140b128..8fec3c77a 100644 --- a/docs/docs/plugins/execution-context.md +++ b/docs/docs/plugins/execution-context.md @@ -44,10 +44,12 @@ establishes user scope from the request, preserving the original fail-closed OBO default. Missing credentials reject before the provider executes. An existing caller takes precedence over conflicting or absent request credentials. -The agents HTTP execution routes (`/invocations`, `/responses`, and -`/api/agents/chat`) establish user scope at entry by default, including all nested -tool calls. Missing credentials reject the request before the agent runs. -The marked development fallback below is the only missing-token exception. +On the agents HTTP execution routes (`/invocations`, `/responses`, and +`/api/agents/chat`), each plugin-toolkit tool call runs in user scope through +`executeTool`, including tool calls made by sub-agents. A plugin tool call without +usable user credentials rejects; it never runs as the service principal. The +marked development fallback below is the only missing-token exception. The agent's +model call and hand-rolled tools are not wrapped in user scope. ## Which built-in surfaces run as the user @@ -59,9 +61,9 @@ The default is the **service principal**. Work runs on behalf of the user only i | Serving plugin (deprecated) routes | signed-in user (OBO) | the built-in route runs every call in user scope; prefer the agents plugin | | Analytics `.obo.sql` queries | signed-in user (OBO) | the `.obo.sql` file name selects the user lane | | Analytics `.sql` queries, Files, and other plugin calls | app service principal | user only inside `appkit.asUser(req)`, or for Files volumes configured with `auth: "on-behalf-of-user"` | -| Agents HTTP routes: the model (LLM) call | app service principal | the routes open user scope, but the model adapter builds its own service-principal client, so the model call does not use the user token | -| Agents HTTP routes: plugin-toolkit tool calls (`plugin:`) | signed-in user (OBO) | `executeTool` inherits the route's user scope | -| Agents HTTP routes: hand-rolled `tool({ execute })` | signed-in user (OBO), for AppKit calls inside `execute` | `execute` runs inside the route's user scope, so plugin handles and `getWorkspaceClient()` resolve to the user | +| Agents HTTP routes: the model (LLM) call | app service principal | the model adapter builds its own service-principal client, and the route does not open user scope | +| Agents HTTP routes: plugin-toolkit tool calls (`plugin:`) | signed-in user (OBO) | `executeTool` opens user scope for each call; without user credentials the call rejects | +| Agents HTTP routes: hand-rolled `tool({ execute })` | app service principal | `execute` receives only the validated arguments and runs in the app context, as before | | Standalone `runAgent` (no HTTP request) | app service principal | there is no request, so no user scope | So an agent's **model inference runs as the service principal**, while the tools it calls over the built-in HTTP routes run on behalf of the user. See the [agents plugin](./agents.md) for the tool-level detail. diff --git a/packages/appkit/src/core/tests/appkit-user-scope.test.ts b/packages/appkit/src/core/tests/appkit-user-scope.test.ts index 5aa244a5b..5afe1bca3 100644 --- a/packages/appkit/src/core/tests/appkit-user-scope.test.ts +++ b/packages/appkit/src/core/tests/appkit-user-scope.test.ts @@ -2,6 +2,7 @@ import { context as otelContext } from "@opentelemetry/api"; import { AsyncLocalStorageContextManager } from "@opentelemetry/context-async-hooks"; import type { AgentAdapter, AgentToolDefinition, ToolProvider } from "shared"; import { describe, expect, expectTypeOf, test, vi } from "vitest"; +import { z } from "zod"; import { CacheManager } from "../../cache"; import { @@ -13,6 +14,8 @@ import { isDevOboFallback } from "../../context/request-scope"; import { Plugin, toPlugin } from "../../plugin"; import { agents } from "../../plugins/agents"; import { createMockRequest, createTestApp } from "../../testing"; +import { tool } from "../agent/tools/tool"; +import type { AgentDefinition } from "../agent/types"; class IdentityPlugin extends Plugin implements ToolProvider { static manifest = { @@ -238,17 +241,32 @@ describe("app-level caller scope", () => { }); describe("agents HTTP identity boundary", () => { + const paths = ["/invocations", "/responses", "/api/agents/chat"]; + const body = (path: string) => + path.endsWith("chat") ? { message: "hello" } : { input: "hello" }; + + // The model call and hand-rolled tools see the ambient principal; plugin + // tools get user scope per call from executeTool. const adapter: AgentAdapter = { async *run(_input, ctx) { - const principal = getCurrentPrincipalKey(); - const toolPrincipal = await ctx.executeTool("identity.read", {}); - yield { type: "message_delta", content: `${principal}/${toolPrincipal}` }; + const model = getCurrentPrincipalKey(); + const plugin = await ctx.executeTool("identity.read", {}); + const handRolled = await ctx.executeTool("whoami", {}); + yield { + type: "message_delta", + content: `model=${model} plugin=${plugin} handRolled=${handRolled}`, + }; yield { type: "status", status: "complete" }; }, }; + const whoami = tool({ + description: "Report the principal seen by a hand-rolled tool", + schema: z.object({}), + execute: () => getCurrentPrincipalKey(), + }); - test.each(["/invocations", "/responses", "/api/agents/chat"])( - "%s defaults to user identity, including tool dispatch", + test.each(paths)( + "%s runs plugin tools as the user and the model and hand-rolled tools as the app", async (path) => { await using app = await createTestApp({ plugins: [ @@ -258,42 +276,98 @@ describe("agents HTTP identity boundary", () => { probe: { instructions: "Identify the caller", model: adapter, - tools: (plugins) => plugins.identity.toolkit(), + tools: (plugins) => ({ + ...plugins.identity.toolkit(), + whoami, + }), }, }, }), ], }); const response = await app.post(path, { - body: path.endsWith("chat") ? { message: "hello" } : { input: "hello" }, + body: body(path), obo: { userId: "alice" }, }); expect(response.status).toBe(200); - expect(await response.text()).toContain("user:alice/user:alice"); + expect(await response.text()).toContain( + "model=app plugin=user:alice handRolled=app", + ); expect(getCurrentPrincipalKey()).toBe("app"); }, ); - test.each(["/invocations", "/responses", "/api/agents/chat"])( - "%s cannot execute as SP by omitting the token", + test("sub-agents follow the same split", async () => { + const parent: AgentAdapter = { + async *run(_input, ctx) { + const child = await ctx.executeTool("agent-child", { input: "go" }); + yield { type: "message_delta", content: `child[${child}]` }; + yield { type: "status", status: "complete" }; + }, + }; + const child: AgentDefinition = { + instructions: "Identify the caller", + model: adapter, + tools: (plugins) => ({ + ...plugins.identity.toolkit(), + whoami, + }), + }; + await using app = await createTestApp({ + plugins: [ + identity(), + agents({ + agents: { + probe: { + default: true, + instructions: "Delegate", + model: parent, + agents: { child }, + }, + child, + }, + }), + ], + }); + const response = await app.post("/api/agents/chat", { + body: { message: "hello" }, + obo: { userId: "alice" }, + }); + expect(response.status).toBe(200); + expect(await response.text()).toContain( + "model=app plugin=user:alice handRolled=app", + ); + }); + + test.each(paths)( + "%s rejects a plugin tool call without a user token and never runs it as the app", async (path) => { - const run = vi.fn(adapter.run); + const executeAgentTool = vi.spyOn( + IdentityPlugin.prototype, + "executeAgentTool", + ); await using app = await createTestApp({ plugins: [ identity(), agents({ agents: { - probe: { instructions: "Identify the caller", model: { run } }, + probe: { + instructions: "Identify the caller", + model: adapter, + tools: (plugins) => plugins.identity.toolkit(), + }, }, }), ], }); const response = await app.post(path, { - body: path.endsWith("chat") ? { message: "hello" } : { input: "hello" }, + body: body(path), headers: { "x-forwarded-user": "alice" }, }); - expect(response.status).toBe(401); - expect(run).not.toHaveBeenCalled(); + const text = await response.text(); + expect(executeAgentTool).not.toHaveBeenCalled(); + expect(text).not.toContain("plugin=app"); + executeAgentTool.mockRestore(); }, ); }); diff --git a/packages/appkit/src/plugins/agents/agents.ts b/packages/appkit/src/plugins/agents/agents.ts index 4eb65a530..059136cb9 100644 --- a/packages/appkit/src/plugins/agents/agents.ts +++ b/packages/appkit/src/plugins/agents/agents.ts @@ -19,7 +19,6 @@ import type { import { isSupervisorTool } from "../../agents/supervisor-api"; import { AppKitMcpClient, buildMcpHostPolicy } from "../../connectors/mcp"; import { getWorkspaceClient } from "../../context"; -import { createRequestScope } from "../../context/request-scope"; import { consumeAdapterStream } from "../../core/agent/consume-adapter-stream"; import { loadAgentsFromDir } from "../../core/agent/load-agents"; import { CODE_AGENTS_SOURCE_DIR } from "../../core/agent/load-code-agents"; @@ -909,8 +908,10 @@ export class AgentsPlugin extends Plugin implements ToolProvider { if (!this.context) return; // Return the promise so the forwardAsyncErrors wrapper applied by // PluginContext.addRoute can forward rejections to the error middleware. + // No route-wide user scope: plugin tools get it per call in executeTool, + // while the model call and hand-rolled tools stay on the app principal. const handler = (req: express.Request, res: express.Response) => - createRequestScope(req).run(() => this._handleInvoke(req, res)); + this._handleInvoke(req, res); this.context.addRoute("post", "/invocations", handler); this.context.addRoute("post", "/responses", handler); } @@ -920,8 +921,7 @@ export class AgentsPlugin extends Plugin implements ToolProvider { name: "chat", method: "post", path: "/chat", - handler: async (req, res) => - createRequestScope(req).run(() => this._handleChat(req, res)), + handler: async (req, res) => this._handleChat(req, res), }); this.route(router, { name: "cancel", From 2e7276415a2b74e34241ecdcb7b6b0a7326e5712 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Thu, 1 Oct 2026 14:07:52 +0200 Subject: [PATCH 06/10] refactor(appkit): drop the scoped-api isPlainObject re-export scoped-api.ts re-exported isPlainObject from utils only so the agreement test could import it from here. Remove the re-export and point the test at the utility directly; the dedicated util remains the single source. Co-authored-by: Isaac Signed-off-by: MarioCadenas --- packages/appkit/src/context/scoped-api.ts | 2 -- packages/appkit/src/utils/tests/is-plain-object.test.ts | 3 --- 2 files changed, 5 deletions(-) diff --git a/packages/appkit/src/context/scoped-api.ts b/packages/appkit/src/context/scoped-api.ts index 53446ac34..06f4d1fc0 100644 --- a/packages/appkit/src/context/scoped-api.ts +++ b/packages/appkit/src/context/scoped-api.ts @@ -1,8 +1,6 @@ import { isPlainObject } from "../utils/is-plain-object"; import type { RequestScope } from "./request-scope"; -export { isPlainObject } from "../utils/is-plain-object"; - const EXCLUDED_FROM_PROXY = new Set([ "setup", "shutdown", diff --git a/packages/appkit/src/utils/tests/is-plain-object.test.ts b/packages/appkit/src/utils/tests/is-plain-object.test.ts index f3ea1b663..0fb4e9ef5 100644 --- a/packages/appkit/src/utils/tests/is-plain-object.test.ts +++ b/packages/appkit/src/utils/tests/is-plain-object.test.ts @@ -1,13 +1,11 @@ import { describe, expect, test } from "vitest"; -import { isPlainObject as scopedIsPlainObject } from "../../context/scoped-api"; import { isPlainObject as pluginIsPlainObject } from "../../plugin/plugin"; import { isPlainObject as crudIsPlainObject } from "../../plugins/database/crud/contract"; import { isPlainObject } from "../is-plain-object"; describe.each([ { name: "utility", check: isPlainObject }, - { name: "scoped API export", check: scopedIsPlainObject }, { name: "plugin export", check: pluginIsPlainObject }, { name: "CRUD export", check: crudIsPlainObject }, ])("$name", ({ check }) => { @@ -42,7 +40,6 @@ test("CRUD keeps its explicit array rejection regardless of the prototype", () = for (const prototype of [Object.prototype, null]) { const array = Object.setPrototypeOf([], prototype); expect(isPlainObject(array)).toBe(true); - expect(scopedIsPlainObject(array)).toBe(true); expect(pluginIsPlainObject(array)).toBe(true); expect(crudIsPlainObject(array)).toBe(false); } From 004033e12c9abe77a073543cc50b9b961c8fd34e Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Thu, 1 Oct 2026 16:25:37 +0200 Subject: [PATCH 07/10] perf(appkit): memoize per-plugin scoped export on the asUser handle Repeated property access on one asUser(req) handle rebuilt the scoped proxy on every read (measured ~854 ns per access on a cached handle). Memoize each plugin's wrapped export in a Map created inside the asUser call, so repeated access reuses the wrapper (~12 ns) and a method call drops from ~1500 ns to ~490 ns. The memo lives only on the per-request handle: the Map is created per asUser(req) call and captured solely by that handle's getters, so it dies with the request and is never shared across principals. A new test proves two handles get different wrappers and each resolves to its own principal. Fail-closed and per-principal cache partitioning are unchanged. Co-authored-by: Isaac Signed-off-by: MarioCadenas --- packages/appkit/src/core/appkit.ts | 25 +++++++++++++----- .../src/core/tests/appkit-user-scope.test.ts | 26 +++++++++++++++++++ 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/packages/appkit/src/core/appkit.ts b/packages/appkit/src/core/appkit.ts index 305996486..3a02fc2a0 100644 --- a/packages/appkit/src/core/appkit.ts +++ b/packages/appkit/src/core/appkit.ts @@ -49,15 +49,28 @@ export class AppKit { asUser(req: import("express").Request) { const scope = createRequestScope(req); const kit: Record = Object.create(null); + // Memoize each plugin's scoped export for the life of THIS handle only. + // The map is created per asUser(req) call and captured solely by this + // handle's getters, so it dies with the request and never crosses + // principals. Repeated access (e.g. kit.analytics twice) reuses the same + // wrapper instead of rebuilding the proxy on every read. + const scopedExports = new Map(); for (const [name, plugin] of Object.entries(this.#pluginInstances)) { Object.defineProperty(kit, name, { enumerable: true, - get: () => - scopeApi( - scope.run(() => plugin.exports?.() ?? {}), - scope, - plugin, - ), + get: () => { + if (!scopedExports.has(name)) { + scopedExports.set( + name, + scopeApi( + scope.run(() => plugin.exports?.() ?? {}), + scope, + plugin, + ), + ); + } + return scopedExports.get(name); + }, }); } Object.defineProperty(kit, "run", { diff --git a/packages/appkit/src/core/tests/appkit-user-scope.test.ts b/packages/appkit/src/core/tests/appkit-user-scope.test.ts index 5afe1bca3..53070b5d3 100644 --- a/packages/appkit/src/core/tests/appkit-user-scope.test.ts +++ b/packages/appkit/src/core/tests/appkit-user-scope.test.ts @@ -122,6 +122,32 @@ describe("app-level caller scope", () => { expect(kit.identity.read()).toBe("bound:app"); }); + test("memoizes a plugin's scoped export per handle without crossing principals", async () => { + await using app = await createTestApp({ + plugins: [identity()], + server: false, + }); + const kit = app.plugins; + const alice = kit.asUser(request("alice")); + const bob = kit.asUser(request("bob")); + + // Same handle: repeated access returns the identical wrapper (memoized, + // not rebuilt on every read). + expect(alice.identity).toBe(alice.identity); + + // Different handles: different wrappers, so one request's scoped export is + // never shared with another principal. + expect(alice.identity).not.toBe(bob.identity); + + // Each memoized wrapper still resolves to its own principal. + expect(alice.identity.read()).toBe("bound:user:alice"); + expect(bob.identity.read()).toBe("bound:user:bob"); + // And again, proving the cached wrapper did not latch the first caller. + expect(alice.identity.read()).toBe("bound:user:alice"); + expect(bob.identity.read()).toBe("bound:user:bob"); + expect(getCurrentPrincipalKey()).toBe("app"); + }); + test("restores the parent scope after failures and isolates concurrent users", async () => { await using app = await createTestApp({ plugins: [identity()], From 33451c3b5ae4a0fbb2fff815820d44917ea1e3ed Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Mon, 5 Oct 2026 16:29:46 +0200 Subject: [PATCH 08/10] fix(appkit): return native streams unchanged from scoped plugin APIs scopeApi rewrapped any async-iterable value in an iterator-only object. A web ReadableStream is async-iterable in Node 18+, so a scoped Files download returned `contents` that was no longer a ReadableStream: getReader, cancel, pipeTo and locked were gone, and only for-await still worked. This also hit the deprecated plugin.asUser(req) path, breaking existing code that pipes or reads a download. Return web ReadableStream and Node stream.Readable values unchanged. The authenticated request already ran inside the caller scope, so reading the body is plain data with no deferred identity work. Lazy generators and custom async iterators keep their scoped wrapping. Co-authored-by: Isaac Signed-off-by: MarioCadenas --- packages/appkit/src/context/scoped-api.ts | 12 ++++ .../src/core/tests/appkit-user-scope.test.ts | 63 +++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/packages/appkit/src/context/scoped-api.ts b/packages/appkit/src/context/scoped-api.ts index 06f4d1fc0..2cf4f3ce7 100644 --- a/packages/appkit/src/context/scoped-api.ts +++ b/packages/appkit/src/context/scoped-api.ts @@ -1,3 +1,5 @@ +import { Readable } from "node:stream"; + import { isPlainObject } from "../utils/is-plain-object"; import type { RequestScope } from "./request-scope"; @@ -39,6 +41,16 @@ export function scopeApi( if (value instanceof Promise) { return value.then((result) => scopeApi(result, scope)) as T; } + // Native streams pass through unchanged. The authenticated request already + // ran inside the caller scope; reading the body is pure data with no + // deferred identity work, and wrapping would strip getReader/pipe/cancel. + if ( + (typeof ReadableStream !== "undefined" && + value instanceof ReadableStream) || + value instanceof Readable + ) { + return value; + } if (value && typeof value === "object" && Symbol.asyncIterator in value) { const iterable = value as AsyncIterable; const wrapIterator = ( diff --git a/packages/appkit/src/core/tests/appkit-user-scope.test.ts b/packages/appkit/src/core/tests/appkit-user-scope.test.ts index 53070b5d3..1dee6d1c0 100644 --- a/packages/appkit/src/core/tests/appkit-user-scope.test.ts +++ b/packages/appkit/src/core/tests/appkit-user-scope.test.ts @@ -1,3 +1,5 @@ +import { Readable } from "node:stream"; + import { context as otelContext } from "@opentelemetry/api"; import { AsyncLocalStorageContextManager } from "@opentelemetry/context-async-hooks"; import type { AgentAdapter, AgentToolDefinition, ToolProvider } from "shared"; @@ -66,6 +68,26 @@ class CallablePlugin extends Plugin { } const identity = toPlugin(IdentityPlugin); const callable = toPlugin(CallablePlugin); +const BYTES = [104, 105]; +class DownloadPlugin extends Plugin { + static manifest = { ...IdentityPlugin.manifest, name: "download" as const }; + exports() { + return { + // Files-style download: the authenticated request already ran, the + // body is a native web stream. + download: async () => ({ + contents: new ReadableStream({ + start(controller) { + controller.enqueue(new Uint8Array(BYTES)); + controller.close(); + }, + }), + }), + nodeStream: () => Readable.from([Buffer.from(BYTES)]), + }; + } +} +const download = toPlugin(DownloadPlugin); const request = (userId: string) => createMockRequest({ obo: { userId, token: `${userId}-token` } }); @@ -173,6 +195,47 @@ describe("app-level caller scope", () => { expect(getCurrentPrincipalKey()).toBe("app"); }); + test.each([ + [ + "appkit.asUser(req)", + (app: any) => app.plugins.asUser(request("alice")).download, + ], + [ + "deprecated plugin.asUser(req)", + (app: any) => app.plugins.download.asUser(request("alice")), + ], + ])( + "returns native ReadableStream downloads unchanged through %s", + async (_, scoped) => { + vi.spyOn(console, "warn").mockImplementation(() => {}); + await using app = await createTestApp({ + plugins: [download()], + server: false, + }); + const { contents } = await scoped(app).download(); + expect(contents).toBeInstanceOf(ReadableStream); + const reader = contents.getReader(); + const { value } = await reader.read(); + expect(Array.from(value)).toEqual(BYTES); + reader.releaseLock(); + + const { contents: second } = await scoped(app).download(); + await expect(second.cancel()).resolves.toBeUndefined(); + }, + ); + + test("returns a Node Readable unchanged through the scoped API", async () => { + await using app = await createTestApp({ + plugins: [download()], + server: false, + }); + const stream = app.plugins.asUser(request("alice")).download.nodeStream(); + expect(stream).toBeInstanceOf(Readable); + const chunks: Buffer[] = []; + for await (const chunk of stream) chunks.push(chunk as Buffer); + expect(Array.from(Buffer.concat(chunks))).toEqual(BYTES); + }); + test("keeps shorthand streams scoped when consumed outside the original call", async () => { await using app = await createTestApp({ plugins: [identity()], From b6b57ca3c31e7ed8783ee60a4db6fdbe9d77aa83 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Mon, 5 Oct 2026 16:46:35 +0200 Subject: [PATCH 09/10] fix(appkit): resolve the Files default policy user from the active scope FilesPlugin.exports() built the default-surface policy user with `isServicePrincipal: true` hardcoded. That was right when the per-volume `.asUser(req)` adapter was the only programmatic user path, but `appkit.asUser(req)` runs the default surface inside the user scope: the SDK call ran as the user while policies still saw a service principal. A policy such as `user.isServicePrincipal || path.startsWith(user.id)` let the user through as if it were the SP. The default-surface policy user now reads `isServicePrincipal` and `id` from the active scope at call time, and `files.auth_mode` is tagged from the same scope ("on-behalf-of-user" inside a caller scope). The per-volume `.asUser(req)` adapter is unchanged. Co-authored-by: Isaac Signed-off-by: MarioCadenas --- packages/appkit/src/plugins/files/plugin.ts | 55 ++++++++++--- .../src/plugins/files/tests/plugin.test.ts | 81 ++++++++++++++++++- 2 files changed, 125 insertions(+), 11 deletions(-) diff --git a/packages/appkit/src/plugins/files/plugin.ts b/packages/appkit/src/plugins/files/plugin.ts index cc3dc4ee8..8136055df 100644 --- a/packages/appkit/src/plugins/files/plugin.ts +++ b/packages/appkit/src/plugins/files/plugin.ts @@ -19,6 +19,8 @@ import { validateCustomContentTypes, } from "../../connectors/files"; import { + getCallerContext, + getCurrentActorId, getCurrentUserId, getExecutionContext, getWorkspaceClient, @@ -1503,6 +1505,37 @@ export class FilesPlugin extends Plugin implements ToolProvider { * additional parent span is opened, so each call produces exactly one * `files.` span instead of two. */ + /** + * Like `_wrapVolumeAPIWithSPSpan`, but picks `files.auth_mode` from the + * active scope at call time: "on-behalf-of-user" inside a caller scope + * (for example `appkit.asUser(req)`), "service-principal" otherwise. + */ + private _wrapVolumeAPIWithScopeSpan(api: VolumeAPI): VolumeAPI { + const wrap = + ( + operation: string, + fn: (...args: Args) => Promise, + ): ((...args: Args) => Promise) => + (...args: Args) => + this._withAuthModeAttributes( + operation, + getCallerContext() ? "on-behalf-of-user" : "service-principal", + () => fn(...args), + ); + + return { + list: wrap("list", api.list), + read: wrap("read", api.read), + download: wrap("download", api.download), + exists: wrap("exists", api.exists), + metadata: wrap("metadata", api.metadata), + upload: wrap("upload", api.upload), + createDirectory: wrap("createDirectory", api.createDirectory), + delete: wrap("delete", api.delete), + preview: wrap("preview", api.preview), + }; + } + private _wrapVolumeAPIWithSPSpan(api: VolumeAPI): VolumeAPI { const wrap = ( @@ -1803,19 +1836,21 @@ export class FilesPlugin extends Plugin implements ToolProvider { ); } - // Lazy user resolution: getCurrentUserId() is called when a method - // is invoked (policy check), not when exports() is called. - const spUser: FilePolicyUser = { + // Identity is resolved at call time from the active scope, not at + // export time: the default surface is also reached inside + // appkit.asUser(req), where the SDK call runs as the user, so the + // policy must see the user too (not a hardcoded service principal). + const policyUser: FilePolicyUser = { get id() { - return getCurrentUserId(); + return getCurrentActorId() ?? ServiceContext.get().serviceUserId; + }, + get isServicePrincipal() { + return getCallerContext() === undefined; }, - isServicePrincipal: true, }; - // Default (non-asUser) programmatic surface: every call is tagged - // with `files.auth_mode = "service-principal"` for telemetry parity - // with the HTTP route path. - const spApi = this._wrapVolumeAPIWithSPSpan( - this.createVolumeAPI(volumeKey, spUser), + // Tag `files.auth_mode` from the same scope at call time. + const spApi = this._wrapVolumeAPIWithScopeSpan( + this.createVolumeAPI(volumeKey, policyUser), ); return { diff --git a/packages/appkit/src/plugins/files/tests/plugin.test.ts b/packages/appkit/src/plugins/files/tests/plugin.test.ts index ad0cbc4aa..b1e306dec 100644 --- a/packages/appkit/src/plugins/files/tests/plugin.test.ts +++ b/packages/appkit/src/plugins/files/tests/plugin.test.ts @@ -3,6 +3,8 @@ import { PassThrough, Readable } from "node:stream"; import { mockServiceContext, setupDatabricksEnv } from "@tools/test-helpers"; import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +import { createRequestScope } from "../../../context/request-scope"; +import { scopeApi } from "../../../context/scoped-api"; import { ServiceContext } from "../../../context/service-context"; import { createApp } from "../../../core"; import { AuthenticationError } from "../../../errors"; @@ -14,7 +16,7 @@ import { FILES_WRITE_DEFAULTS, } from "../defaults"; import { FilesPlugin, files } from "../plugin"; -import { PolicyDeniedError, policy } from "../policy"; +import { PolicyDeniedError, policy, type FilePolicyUser } from "../policy"; const { mockClient, MockApiError } = await vi.hoisted(async () => { const mockFilesApi = { @@ -3830,6 +3832,83 @@ describe("FilesPlugin", () => { ); expect(sp).toBeDefined(); }); + + // appkit.asUser(req) scopes the default surface with these primitives. + const oboHeaders = { + "x-forwarded-access-token": "alice-token", + "x-forwarded-user": "alice@example.com", + }; + const appkitAsUser = (value: T) => + scopeApi( + value, + createRequestScope(mockReq("uploads", oboHeaders) as any), + ); + + test("policy user reflects the active scope on the default surface", async () => { + const seen: FilePolicyUser[] = []; + const recorder = (_a: unknown, _r: unknown, user: FilePolicyUser) => { + seen.push({ id: user.id, isServicePrincipal: user.isServicePrincipal }); + return true; + }; + const plugin = new FilesPlugin({ + volumes: { uploads: { policy: recorder }, exports: {} }, + }); + mockClient.files.listDirectoryContents.mockImplementation( + async function* () {}, + ); + const handle = plugin.exports()("uploads"); + + await handle.list("d"); + await appkitAsUser(handle).list("d"); + await handle + .asUser({ + header: (n: string) => (oboHeaders as any)[n.toLowerCase()], + } as any) + .list("d"); + + expect(seen[0].isServicePrincipal).toBe(true); + expect(seen[1]).toEqual({ + id: "alice@example.com", + isServicePrincipal: false, + }); + expect(seen[2]).toEqual({ + id: "alice@example.com", + isServicePrincipal: false, + }); + }); + + test("an SP-only policy denies appkit.asUser(req) but allows the default surface", async () => { + const spOnly = (_a: unknown, _r: unknown, user: FilePolicyUser) => + user.isServicePrincipal === true; + const plugin = new FilesPlugin({ + volumes: { uploads: { policy: spOnly }, exports: {} }, + }); + mockClient.files.listDirectoryContents.mockImplementation( + async function* () {}, + ); + const handle = plugin.exports()("uploads"); + + await expect(handle.list("d")).resolves.toBeDefined(); + await expect(appkitAsUser(handle).list("d")).rejects.toBeInstanceOf( + PolicyDeniedError, + ); + }); + + test("appkit.asUser(req) tags files.auth_mode as on-behalf-of-user", async () => { + const plugin = new FilesPlugin(VOLUMES_CONFIG); + const calls = spyOnTelemetry(plugin); + mockClient.files.listDirectoryContents.mockImplementation( + async function* () {}, + ); + + await appkitAsUser(plugin.exports()("uploads")).list("d"); + + const modes = calls + .map((c) => c.attributes?.["files.auth_mode"]) + .filter(Boolean); + expect(modes).toContain("on-behalf-of-user"); + expect(modes).not.toContain("service-principal"); + }); }); describe("Upload Stream Size Limiter", () => { From e136455b1e56c3f4b54cc3df8e7fb08e44d4ca7c Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Mon, 5 Oct 2026 17:40:01 +0200 Subject: [PATCH 10/10] fix(appkit): keep scoped results as plain data and narrow the identity guard Two scopeApi regressions on the new appkit.asUser(req) surface and the deprecated plugin.asUser(req) path. Results were rebuilt as getter-only copies, so assigning a field threw in strict mode and nested objects were re-wrapped on every read (`r.meta === r.meta` was false). Values now only get scoped wrapping when they can run code later: a function, a Promise, a non-native async iterable, or a plain object reaching one of those through its own properties (getters count and are not invoked). Plain data comes back unchanged. Objects mixing data and methods keep the wrapper, return data members as-is, and forward assignments to the original. The identity guard hid every `as[A-Z]` name, so exports like asCsv or asArrow disappeared under scoped surfaces. It now hides only asUser, the one identity method the stack defines. Co-authored-by: Isaac Signed-off-by: MarioCadenas --- packages/appkit/src/context/scoped-api.ts | 58 ++++++++++++++---- .../src/core/tests/appkit-user-scope.test.ts | 61 ++++++++++++++++++- 2 files changed, 106 insertions(+), 13 deletions(-) diff --git a/packages/appkit/src/context/scoped-api.ts b/packages/appkit/src/context/scoped-api.ts index 2cf4f3ce7..143203617 100644 --- a/packages/appkit/src/context/scoped-api.ts +++ b/packages/appkit/src/context/scoped-api.ts @@ -15,8 +15,40 @@ const EXCLUDED_FROM_PROXY = new Set([ "constructor", ]); +// Methods that would re-scope a scoped surface to another identity. Only +// these are hidden; other `as*` names (asCsv, asArrow) are ordinary exports. +const IDENTITY_METHODS: ReadonlySet = new Set(["asUser"]); + function isIdentityMethod(key: PropertyKey): boolean { - return typeof key === "string" && /^as[A-Z]/.test(key); + return IDENTITY_METHODS.has(key); +} + +function isNativeStream(value: unknown): boolean { + return ( + (typeof ReadableStream !== "undefined" && + value instanceof ReadableStream) || + value instanceof Readable + ); +} + +/** + * Whether a value can run code later and so must run in the caller scope: + * a function, a Promise, a non-native async iterable, or a plain object + * reaching one of those through its own properties. Getter properties count, + * and are not invoked here. Arrays and other objects are plain data, as before. + */ +function needsScope(value: unknown, seen = new Set()): boolean { + if (typeof value === "function" || value instanceof Promise) return true; + if (!value || typeof value !== "object" || isNativeStream(value)) { + return false; + } + if (Symbol.asyncIterator in value) return true; + if (!isPlainObject(value) || seen.has(value)) return false; + seen.add(value); + return Reflect.ownKeys(value).some((key) => { + const descriptor = Object.getOwnPropertyDescriptor(value, key); + return descriptor?.get !== undefined || needsScope(descriptor?.value, seen); + }); } /** Preserve identity across callable exports, returned handles, and lazy streams. */ @@ -44,11 +76,7 @@ export function scopeApi( // Native streams pass through unchanged. The authenticated request already // ran inside the caller scope; reading the body is pure data with no // deferred identity work, and wrapping would strip getReader/pipe/cancel. - if ( - (typeof ReadableStream !== "undefined" && - value instanceof ReadableStream) || - value instanceof Readable - ) { + if (isNativeStream(value)) { return value; } if (value && typeof value === "object" && Symbol.asyncIterator in value) { @@ -81,18 +109,24 @@ export function scopeApi( } as T; } if (isPlainObject(value)) { + // Plain data results come back unchanged: mutable and identity-stable. + if (!needsScope(value)) return value; const result = Object.create(Object.getPrototypeOf(value)); for (const key of Reflect.ownKeys(value)) { if (isIdentityMethod(key)) continue; Object.defineProperty(result, key, { enumerable: Object.getOwnPropertyDescriptor(value, key)?.enumerable, configurable: true, - get: () => - scopeApi( - scope.run(() => Reflect.get(value, key)), - scope, - receiver ?? value, - ), + get: () => { + const member = scope.run(() => Reflect.get(value, key)); + // Data members are returned as-is so repeated reads are identical. + return needsScope(member) + ? scopeApi(member, scope, receiver ?? value) + : member; + }, + set: (next: unknown) => { + Reflect.set(value, key, next); + }, }); } return result; diff --git a/packages/appkit/src/core/tests/appkit-user-scope.test.ts b/packages/appkit/src/core/tests/appkit-user-scope.test.ts index 1dee6d1c0..f96405b76 100644 --- a/packages/appkit/src/core/tests/appkit-user-scope.test.ts +++ b/packages/appkit/src/core/tests/appkit-user-scope.test.ts @@ -88,6 +88,24 @@ class DownloadPlugin extends Plugin { } } const download = toPlugin(DownloadPlugin); +class DataPlugin extends Plugin { + static manifest = { ...IdentityPlugin.manifest, name: "data" as const }; + exports() { + return { + query: () => ({ rows: [{ id: 1 }], meta: { total: 1 } }), + handle: () => ({ id: "h", read: () => getCurrentPrincipalKey() }), + asCsv: () => "id\n1", + }; + } +} +const data = toPlugin(DataPlugin); +const scopedSurfaces: Array<[string, (app: any) => any]> = [ + ["appkit.asUser(req)", (app) => app.plugins.asUser(request("alice")).data], + [ + "deprecated appkit..asUser(req)", + (app) => app.plugins.data.asUser(request("alice")), + ], +]; const request = (userId: string) => createMockRequest({ obo: { userId, token: `${userId}-token` } }); @@ -236,6 +254,48 @@ describe("app-level caller scope", () => { expect(Array.from(Buffer.concat(chunks))).toEqual(BYTES); }); + test.each(scopedSurfaces)( + "returns plain data results unchanged through %s", + async (_, scoped) => { + vi.spyOn(console, "warn").mockImplementation(() => {}); + await using app = await createTestApp({ + plugins: [data()], + server: false, + }); + const result = scoped(app).query(); + expect(result).toEqual(app.plugins.data.query()); + expect(result.meta).toBe(result.meta); + result.rows = [{ id: 2 }]; + expect(result.rows).toEqual([{ id: 2 }]); + }, + ); + + test.each(scopedSurfaces)( + "still runs returned handle methods in the caller scope through %s", + async (_, scoped) => { + vi.spyOn(console, "warn").mockImplementation(() => {}); + await using app = await createTestApp({ + plugins: [data()], + server: false, + }); + expect(scoped(app).handle().read()).toBe("user:alice"); + expect(app.plugins.data.handle().read()).toBe("app"); + }, + ); + + test.each(scopedSurfaces)( + "keeps non-identity as* exports callable but blocks asUser through %s", + async (_, scoped) => { + vi.spyOn(console, "warn").mockImplementation(() => {}); + await using app = await createTestApp({ + plugins: [data()], + server: false, + }); + expect(scoped(app).asCsv()).toBe("id\n1"); + expect(scoped(app).asUser).toBeUndefined(); + }, + ); + test("keeps shorthand streams scoped when consumed outside the original call", async () => { await using app = await createTestApp({ plugins: [identity()], @@ -289,7 +349,6 @@ describe("app-level caller scope", () => { expect(scoped).not.toHaveProperty("asUser"); expect(scoped).not.toHaveProperty("asApp"); expect(scoped.identity).not.toHaveProperty("asUser"); - expect(scoped.identity).not.toHaveProperty("asOther"); expect(app.plugins).not.toHaveProperty("asApp"); expectTypeOf(scoped).not.toHaveProperty("asUser"); expectTypeOf(scoped.identity).not.toHaveProperty("asUser");