Skip to content

Commit 36f03ba

Browse files
committed
refactor(rbac): make API key policy methods optional on the controller
The API-key policy methods are capability extensions, so declare them optional on RoleBaseAccessController and normalize the surface in LazyController rather than requiring every implementation to carry them. An authorization extension is compiled against whichever core commit its base image ships, so a required additive method forces both sides to move together and turns rolling an extension back into a build failure instead of a graceful degradation. Absence fails closed: no preset catalogue, and prepareApiKeyPolicy refuses outright rather than defaulting to full access, so an extension below this contract cannot mint a credential. Keys already issued are unaffected — they authorize from the scopes persisted on their row. loader.create() now returns HostRbacController, the total surface, so host callers neither guard nor invent their own absent-extension default.
1 parent d20a5af commit 36f03ba

3 files changed

Lines changed: 131 additions & 11 deletions

File tree

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
import type { PrismaClient } from "@trigger.dev/database";
2+
import { describe, expect, it, vi } from "vitest";
3+
4+
// The API-key policy methods are OPTIONAL on RoleBaseAccessController so a
5+
// plugin compiled against an older OSS commit still satisfies the contract
6+
// (the plugin is built against whichever OSS source its base image carries).
7+
// LazyController is what turns that partial surface into a total one, and these
8+
// tests pin the defaults it substitutes — in particular that a missing
9+
// prepareApiKeyPolicy fails CLOSED rather than resolving to full access.
10+
//
11+
// A stand-in for the cloud plugin, which isn't installed in this repo. The
12+
// factory supplies the specifier, so no real module has to resolve.
13+
vi.mock("@triggerdotdev/plugins/rbac", () => ({
14+
default: {
15+
create: () => ({
16+
// Deliberately omits apiKeyPresets / prepareApiKeyPolicy /
17+
// describeApiKeyPolicy — this is a pre-contract plugin.
18+
isUsingPlugin: async () => true,
19+
}),
20+
},
21+
}));
22+
23+
const prismaPlaceholder = {} as unknown as PrismaClient;
24+
25+
describe("LazyController API-key policy defaults (plugin predates the contract)", () => {
26+
async function controller() {
27+
const loader = (await import("./index.js")).default;
28+
const instance = loader.create(prismaPlaceholder);
29+
// Guard against a silent fallback: if the mock didn't take, these
30+
// assertions would be checking the fallback's real implementations.
31+
await expect(instance.isUsingPlugin()).resolves.toBe(true);
32+
return instance;
33+
}
34+
35+
it("reports no preset catalogue rather than throwing", async () => {
36+
await expect((await controller()).apiKeyPresets("org_123")).resolves.toBeNull();
37+
});
38+
39+
it("refuses to prepare a policy — including FULL_ACCESS", async () => {
40+
const result = await (
41+
await controller()
42+
).prepareApiKeyPolicy({
43+
organizationId: "org_123",
44+
presetId: "FULL_ACCESS",
45+
});
46+
47+
// The critical assertion: absence must never resolve to `{ ok: true }` with
48+
// an admin scope. A plugin below the contract cannot mint any credential.
49+
expect(result.ok).toBe(false);
50+
expect(result).not.toHaveProperty("policy");
51+
});
52+
53+
it("describes a policy as having nothing extra to show", async () => {
54+
await expect(
55+
(await controller()).describeApiKeyPolicy({ presetId: "TRIGGER_ONLY", scopes: ["read:runs"] })
56+
).resolves.toEqual({});
57+
});
58+
});

internal-packages/rbac/src/index.ts

Lines changed: 50 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import type {
2+
ApiKeyPolicyDescription,
3+
ApiKeyPreset,
24
Permission,
5+
PrepareApiKeyPolicyResult,
36
RbacAbility,
47
RbacDatabaseConfig,
58
Role,
@@ -12,6 +15,18 @@ import type {
1215
import type { PrismaClient } from "@trigger.dev/database";
1316
import { RoleBaseAccessFallback } from "./fallback.js";
1417
export type { RoleBaseAccessController, RbacAbility, RbacResource } from "@trigger.dev/plugins";
18+
19+
/**
20+
* The controller surface as the HOST sees it, after LazyController has filled in
21+
* defaults for the optional capability methods a plugin may not implement.
22+
*
23+
* `RoleBaseAccessController` is the *plugin-facing* contract, where capability
24+
* extensions are optional so an older plugin still satisfies it. Host code
25+
* always talks to the LazyController singleton (`rbac`), which never omits a
26+
* method — so host consumers should depend on this type, not on the plugin
27+
* contract, and get a total surface without writing their own guards.
28+
*/
29+
export type HostRbacController = Required<RoleBaseAccessController>;
1530
export type { UserActorAuthResult, UserActorClaims } from "@trigger.dev/plugins";
1631
export { buildJwtAbility, scopesWithinAbility } from "./ability.js";
1732
export { FULL_ACCESS_PRESET_ID, scopesGrantFullAccess } from "@trigger.dev/plugins";
@@ -215,18 +230,39 @@ class LazyController implements RoleBaseAccessController {
215230
return (await this.c()).systemRoles(...args);
216231
}
217232

218-
async apiKeyPresets(...args: Parameters<RoleBaseAccessController["apiKeyPresets"]>) {
219-
return (await this.c()).apiKeyPresets(...args);
233+
// The API-key policy methods are optional on the controller contract (see the
234+
// note on RoleBaseAccessController) so a plugin compiled against an older OSS
235+
// commit still satisfies it. LazyController is where that optional surface is
236+
// normalized into a total one: every host caller goes through `rbac`, so the
237+
// absent-plugin default lives here once instead of at each call site.
238+
async apiKeyPresets(
239+
...args: Parameters<NonNullable<RoleBaseAccessController["apiKeyPresets"]>>
240+
): Promise<ApiKeyPreset[] | null> {
241+
const controller = await this.c();
242+
// Same meaning as the no-plugin fallback: no catalogue to offer.
243+
return controller.apiKeyPresets ? controller.apiKeyPresets(...args) : null;
220244
}
221245

222-
async prepareApiKeyPolicy(...args: Parameters<RoleBaseAccessController["prepareApiKeyPolicy"]>) {
223-
return (await this.c()).prepareApiKeyPolicy(...args);
246+
async prepareApiKeyPolicy(
247+
...args: Parameters<NonNullable<RoleBaseAccessController["prepareApiKeyPolicy"]>>
248+
): Promise<PrepareApiKeyPolicyResult> {
249+
const controller = await this.c();
250+
if (!controller.prepareApiKeyPolicy) {
251+
// Fail closed. A plugin that predates this contract must not be able to
252+
// mint a credential — least of all a full-access one — so creation stops
253+
// outright. Keys already issued are unaffected: they authorize from the
254+
// scopes persisted on their row, compiled by the host bearer resolver.
255+
return { ok: false, error: "API key access presets are not available" };
256+
}
257+
return controller.prepareApiKeyPolicy(...args);
224258
}
225259

226260
async describeApiKeyPolicy(
227-
...args: Parameters<RoleBaseAccessController["describeApiKeyPolicy"]>
228-
) {
229-
return (await this.c()).describeApiKeyPolicy(...args);
261+
...args: Parameters<NonNullable<RoleBaseAccessController["describeApiKeyPolicy"]>>
262+
): Promise<ApiKeyPolicyDescription> {
263+
const controller = await this.c();
264+
// Presentation only — an undescribed policy renders from its stored scopes.
265+
return controller.describeApiKeyPolicy ? controller.describeApiKeyPolicy(...args) : {};
230266
}
231267

232268
async allPermissions(
@@ -309,7 +345,13 @@ class LazyController implements RoleBaseAccessController {
309345
class RoleBaseAccess {
310346
// Synchronous — returns a lazy controller that resolves any installed
311347
// plugin on first call.
312-
create(prisma: RbacPrismaInput, options?: RbacCreateOptions): RoleBaseAccessController {
348+
//
349+
// Returns HostRbacController, not RoleBaseAccessController: the latter is the
350+
// plugin-facing contract whose capability methods are optional, and
351+
// LazyController has already substituted defaults for any the installed plugin
352+
// omits. Handing back the total surface is what keeps host callers from having
353+
// to guard (or, worse, from inventing their own absent-plugin default).
354+
create(prisma: RbacPrismaInput, options?: RbacCreateOptions): HostRbacController {
313355
return new LazyController(prisma, options);
314356
}
315357
}

packages/plugins/src/rbac.ts

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -470,17 +470,37 @@ export interface RoleBaseAccessController {
470470
// Plugin-owned API-key policy catalogue and policy generation. A null
471471
// catalogue means no plugin is installed. The host owns credentials and
472472
// persists the effective policy returned by prepareApiKeyPolicy().
473-
apiKeyPresets(organizationId: string): Promise<ApiKeyPreset[] | null>;
473+
//
474+
// These three are OPTIONAL, and are the first optional members of this
475+
// interface — the distinction is deliberate. Methods the host cannot serve a
476+
// request without (authenticateBearer, authenticatePat, authenticateSession)
477+
// stay required. Capability extensions the host can degrade past are
478+
// optional, so a plugin built against an older contract still satisfies this
479+
// interface. That matters because the plugin is compiled against whichever
480+
// OSS commit its base image carries: making an additive capability required
481+
// turns every such addition into a lockstep two-repo merge, and turns a
482+
// plugin rollback into an image build failure instead of a graceful
483+
// degradation.
484+
//
485+
// Absence must fail CLOSED, and each default is chosen so it does:
486+
// - apiKeyPresets -> null (identical to "no plugin installed")
487+
// - prepareApiKeyPolicy -> { ok: false } — NEVER a full-access default;
488+
// key creation stops, while already-issued keys
489+
// keep authorizing from their persisted scopes
490+
// - describeApiKeyPolicy -> {} (presentation only)
491+
// LazyController applies these defaults, so host callers going through
492+
// `rbac` see a total surface and cannot forget the guard.
493+
apiKeyPresets?(organizationId: string): Promise<ApiKeyPreset[] | null>;
474494
// `presetId` is deliberately required: an authorization function must not
475495
// have an implicit default, least of all a full-access one. A caller that
476496
// omits the field should fail to compile rather than silently mint an admin
477497
// credential. Installs with no preset catalogue pass "FULL_ACCESS".
478-
prepareApiKeyPolicy(params: {
498+
prepareApiKeyPolicy?(params: {
479499
organizationId: string;
480500
presetId: string;
481501
taskIdentifiers?: string[];
482502
}): Promise<PrepareApiKeyPolicyResult>;
483-
describeApiKeyPolicy(policy: ApiKeyPolicy): Promise<ApiKeyPolicyDescription>;
503+
describeApiKeyPolicy?(policy: ApiKeyPolicy): Promise<ApiKeyPolicyDescription>;
484504

485505
// Role introspection. The fallback returns []; a plugin may return
486506
// its own role catalogue.

0 commit comments

Comments
 (0)