diff --git a/packages/control-plane/src/automation/authorization-guard.test.ts b/packages/control-plane/src/automation/authorization-guard.test.ts new file mode 100644 index 000000000..9fc5d67f1 --- /dev/null +++ b/packages/control-plane/src/automation/authorization-guard.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, it } from "vitest"; +import type { SqlDatabase } from "../db/sql-database"; +import { isAutomationExecutionAuthorized, isPrincipalAuthorized } from "./authorization-guard"; + +function recordingDb(): { db: SqlDatabase; bindings: unknown[][]; queries: string[] } { + const bindings: unknown[][] = []; + const queries: string[] = []; + const statement = { + bind(...values: unknown[]) { + bindings.push(values); + return statement; + }, + first: async () => ({ authorized: 1 }), + }; + return { + db: { + prepare: (query: string) => { + queries.push(query); + return statement; + }, + } as unknown as SqlDatabase, + bindings, + queries, + }; +} + +describe("automation execution authorization", () => { + it("queries owner and target-use permissions with stable bindings", async () => { + const { db, bindings, queries } = recordingDb(); + + await expect( + isAutomationExecutionAuthorized(db, { + automationId: "automation-1", + requiresRepositoryUse: true, + requiresEnvironmentUse: true, + }) + ).resolves.toBe(true); + + expect(bindings).toHaveLength(1); + expect(bindings[0]?.[0]).toBe("automation-1"); + expect(queries[0]).toContain("a.id = ? AND a.deleted_at IS NULL"); + expect(queries[0]).not.toContain("automation_repositories"); + expect(queries[0]).not.toContain("automation_environments"); + }); + + it("authorizes an explicit execution user instead of the stored owner", async () => { + const { db, bindings, queries } = recordingDb(); + + await expect( + isAutomationExecutionAuthorized(db, { + automationId: "automation-1", + executionUserId: "requester-1", + requiresRepositoryUse: false, + requiresEnvironmentUse: false, + }) + ).resolves.toBe(true); + + expect(bindings[0]?.slice(0, 2)).toEqual(["requester-1", "automation-1"]); + expect(queries[0]).toContain("JOIN users u ON u.id = ?"); + }); + + it("authorizes collaboration without requiring automation launch permissions", async () => { + const { db, bindings, queries } = recordingDb(); + + await expect(isPrincipalAuthorized(db, "actor-1", "sessions.collaborate")).resolves.toBe(true); + + expect(bindings[0]?.[0]).toBe("actor-1"); + expect(queries[0]).not.toContain("automations"); + }); +}); diff --git a/packages/control-plane/src/automation/authorization-guard.ts b/packages/control-plane/src/automation/authorization-guard.ts new file mode 100644 index 000000000..e99aa6275 --- /dev/null +++ b/packages/control-plane/src/automation/authorization-guard.ts @@ -0,0 +1,90 @@ +import { type PermissionId } from "@open-inspect/shared/rbac"; +import { rolePermissionPredicate } from "../authorization/permission-sql"; +import type { SqlDatabase } from "../db/sql-database"; + +interface SqlPredicate { + sql: string; + values: readonly unknown[]; +} + +/** Immutable execution requirements derived from the targets selected for one firing. */ +export interface AutomationExecutionAuthorizationRequest { + automationId: string; + executionUserId?: string; + requiresRepositoryUse: boolean; + requiresEnvironmentUse: boolean; +} + +function executionPredicate(request: AutomationExecutionAuthorizationRequest): SqlPredicate { + const createGuard = rolePermissionPredicate("sessions.create"); + const repositoryGuard = rolePermissionPredicate("repositories.use"); + const environmentGuard = rolePermissionPredicate("environments.use"); + return { + sql: `EXISTS ( + SELECT 1 FROM automations a + JOIN users u ON u.id = ${request.executionUserId ? "?" : "a.user_id"} + JOIN user_role_assignments ura ON ura.user_id = u.id + JOIN roles r ON r.id = ura.role_id + WHERE a.id = ? AND a.deleted_at IS NULL AND u.suspended_at IS NULL + AND ${createGuard.sql} + ${request.requiresRepositoryUse ? `AND ${repositoryGuard.sql}` : ""} + ${request.requiresEnvironmentUse ? `AND ${environmentGuard.sql}` : ""} + )`, + values: [ + ...(request.executionUserId ? [request.executionUserId] : []), + request.automationId, + ...createGuard.values, + ...(request.requiresRepositoryUse ? repositoryGuard.values : []), + ...(request.requiresEnvironmentUse ? environmentGuard.values : []), + ], + }; +} + +function principalPredicate(userId: string, permission: PermissionId): SqlPredicate { + const permissionGuard = rolePermissionPredicate(permission); + return { + sql: `EXISTS ( + SELECT 1 FROM users u + JOIN user_role_assignments ura ON ura.user_id = u.id + JOIN roles r ON r.id = ura.role_id + WHERE u.id = ? AND u.suspended_at IS NULL AND ${permissionGuard.sql} + )`, + values: [userId, ...permissionGuard.values], + }; +} + +/** + * Revalidates that an automation's execution principal may create its session and use its targets. + * + * The caller derives repository/environment requirements from the immutable target selection that + * will execute, so a concurrent edit to the automation tables cannot weaken this decision. Missing + * users, roles, automations, or suspended users fail closed. + * + * This does not decide whether a caller may manage or manually trigger the automation. The route's + * ownership-scoped authorization performs that admission before execution begins. + */ +export async function isAutomationExecutionAuthorized( + db: SqlDatabase, + request: AutomationExecutionAuthorizationRequest +): Promise { + const predicate = executionPredicate(request); + const row = await db + .prepare(`SELECT CASE WHEN (${predicate.sql}) THEN 1 ELSE 0 END AS authorized`) + .bind(...predicate.values) + .first<{ authorized: number }>(); + return row?.authorized === 1; +} + +/** Check one canonical principal for a permission without imposing automation-launch grants. */ +export async function isPrincipalAuthorized( + db: SqlDatabase, + userId: string, + permission: PermissionId +): Promise { + const predicate = principalPredicate(userId, permission); + const row = await db + .prepare(`SELECT CASE WHEN (${predicate.sql}) THEN 1 ELSE 0 END AS authorized`) + .bind(...predicate.values) + .first<{ authorized: number }>(); + return row?.authorized === 1; +} diff --git a/packages/control-plane/src/db/automation-store.test.ts b/packages/control-plane/src/db/automation-store.test.ts index 73ab7ec0d..fd4b8adca 100644 --- a/packages/control-plane/src/db/automation-store.test.ts +++ b/packages/control-plane/src/db/automation-store.test.ts @@ -91,7 +91,7 @@ const sampleRow: AutomationRow = { next_run_at: now + 86400000, consecutive_failures: 0, created_by: "user-1", - user_id: null, + user_id: "11111111111111111111111111111111", created_at: now, updated_at: now, deleted_at: null, @@ -152,6 +152,7 @@ describe("toAutomation", () => { expect(automation.triggerConfig).toBeNull(); expect(automation.consecutiveFailures).toBe(0); expect(automation.createdBy).toBe("user-1"); + expect(automation.userId).toBe("11111111111111111111111111111111"); expect(automation.environmentIds).toEqual([]); }); @@ -497,7 +498,9 @@ describe("AutomationStore", () => { advanceSchedule: { fromSlot: now, nextRunAt: now + 60_000 }, }); - const advance = statements.at(-1)!; + const advance = statements.find((statement) => + statement.sql.includes("SET next_run_at = ?") + )!; // Compare-and-set on the claimed slot, not a monotonic timestamp guard: // "any later value wins" lets a loser advance again from the winner's // successor and skip a slot entirely. diff --git a/packages/control-plane/src/db/automation-store.ts b/packages/control-plane/src/db/automation-store.ts index 8ecb0df39..69f4a7e34 100644 --- a/packages/control-plane/src/db/automation-store.ts +++ b/packages/control-plane/src/db/automation-store.ts @@ -22,6 +22,7 @@ import { } from "./automation-model-provider-auth"; import type { SqlDatabase, SqlStatement } from "./sql-database"; import type { AutomationListCursor } from "./automation-list-cursor"; +import { UserStore } from "./user-store"; function escapeLikePattern(value: string): string { return value.replace(/[\\%_]/g, "\\$&"); @@ -206,6 +207,7 @@ export function toAutomation( nextRunAt: row.next_run_at, consecutiveFailures: row.consecutive_failures, createdBy: row.created_by, + userId: row.user_id, createdAt: row.created_at, updatedAt: row.updated_at, deletedAt: row.deleted_at, @@ -314,6 +316,7 @@ function toAutomationInvocation( // ─── Store ─────────────────────────────────────────────────────────────────── +/** Persists automations, invocations, runs, and composable lifecycle mutations. */ export class AutomationStore { constructor(private readonly db: SqlDatabase) {} @@ -367,6 +370,29 @@ export class AutomationStore { .first(); } + /** + * Repair a legacy SCM-only owner with a compare-and-set and return the canonical row. + * Every ownership admission path calls this before comparing `user_id`, so repair is + * a storage invariant rather than a side effect of starting an invocation. + */ + async resolveCanonicalOwner(automation: AutomationRow): Promise { + if (automation.user_id || !automation.created_by || automation.created_by === "anonymous") { + return automation; + } + + const identity = await new UserStore(this.db).getIdentity("github", automation.created_by); + if (!identity) return automation; + + const result = await this.db + .prepare("UPDATE automations SET user_id = ? WHERE id = ? AND user_id IS NULL") + .bind(identity.userId, automation.id) + .run(); + if ((result.meta?.changes ?? 0) > 0) { + return { ...automation, user_id: identity.userId }; + } + return (await this.getById(automation.id)) ?? automation; + } + async list(options: { limit: number; cursor?: AutomationListCursor | null; @@ -512,36 +538,48 @@ export class AutomationStore { return this.getById(id); } - async softDelete(id: string): Promise { - const now = Date.now(); - const result = await this.db + /** Build a soft-delete statement for composition in an atomic batch. */ + bindSoftDelete(id: string, now = Date.now()): SqlStatement { + return this.db .prepare( "UPDATE automations SET deleted_at = ?, next_run_at = NULL, updated_at = ? WHERE id = ? AND deleted_at IS NULL" ) - .bind(now, now, id) - .run(); + .bind(now, now, id); + } + + /** Soft-delete an automation and report whether a live row changed. */ + async softDelete(id: string): Promise { + const result = await this.bindSoftDelete(id).run(); return (result.meta?.changes ?? 0) > 0; } - async pause(id: string): Promise { - const now = Date.now(); - const result = await this.db + /** Build a pause statement for composition in an atomic batch. */ + bindPause(id: string, now = Date.now()): SqlStatement { + return this.db .prepare( "UPDATE automations SET enabled = 0, next_run_at = NULL, updated_at = ? WHERE id = ? AND deleted_at IS NULL" ) - .bind(now, id) - .run(); + .bind(now, id); + } + + /** Pause an automation and report whether a live row changed. */ + async pause(id: string): Promise { + const result = await this.bindPause(id).run(); return (result.meta?.changes ?? 0) > 0; } - async resume(id: string, nextRunAt: number | null): Promise { - const now = Date.now(); - const result = await this.db + /** Build a resume statement for composition in an atomic batch. */ + bindResume(id: string, nextRunAt: number | null, now = Date.now()): SqlStatement { + return this.db .prepare( "UPDATE automations SET enabled = 1, next_run_at = ?, consecutive_failures = 0, updated_at = ? WHERE id = ? AND deleted_at IS NULL" ) - .bind(nextRunAt, now, id) - .run(); + .bind(nextRunAt, now, id); + } + + /** Resume an automation and report whether a live row changed. */ + async resume(id: string, nextRunAt: number | null): Promise { + const result = await this.bindResume(id, nextRunAt).run(); return (result.meta?.changes ?? 0) > 0; } @@ -855,7 +893,7 @@ export class AutomationStore { /** * Per-source overlap predicate, used both as the cheap pre-check and inside - * the guarded insert (same SQL, one definition). Schedule/manual firings + * the conditional insert (same SQL, one definition). Schedule/manual firings * block on ANY active run of the automation (main parity with * getActiveRunForAutomation); event firings block per concurrency key only — * an automation-wide guard would serialize unrelated events. @@ -911,7 +949,6 @@ export class AutomationStore { const invocation = params.invocation; const overlap = this.overlapPredicate(invocation.automation_id, params.overlapScope); const statements: SqlStatement[] = []; - statements.push( this.db .prepare( @@ -1046,6 +1083,46 @@ export class AutomationStore { return { inserted: (results[0]?.meta?.changes ?? 0) > 0 }; } + /** Atomically record a denied cron slot and pause it so overdue denial cannot starve the queue. */ + async recordAuthorizationDenied( + invocation: AutomationInvocationRow, + fromSlot: number + ): Promise<{ inserted: boolean; paused: boolean }> { + const results = await this.db.batch([ + this.db + .prepare( + `INSERT OR IGNORE INTO automation_invocations + (id, automation_id, source, scheduled_at, trigger_key, concurrency_key, + trigger_metadata, skip_reason, failure_counted_at, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)` + ) + .bind( + invocation.id, + invocation.automation_id, + invocation.source, + invocation.scheduled_at, + invocation.trigger_key, + invocation.concurrency_key, + invocation.trigger_metadata, + invocation.skip_reason, + invocation.failure_counted_at, + invocation.created_at, + invocation.updated_at + ), + this.db + .prepare( + `UPDATE automations + SET enabled = 0, next_run_at = NULL, updated_at = ? + WHERE id = ? AND deleted_at IS NULL AND enabled = 1 AND next_run_at = ?` + ) + .bind(Date.now(), invocation.automation_id, fromSlot), + ]); + return { + inserted: (results[0]?.meta?.changes ?? 0) > 0, + paused: (results[1]?.meta?.changes ?? 0) > 0, + }; + } + async getInvocationById(invocationId: string): Promise { return this.db .prepare(`SELECT * FROM automation_invocations WHERE id = ?`) diff --git a/packages/control-plane/src/router.ts b/packages/control-plane/src/router.ts index 5b406dca0..e13e0a4ce 100644 --- a/packages/control-plane/src/router.ts +++ b/packages/control-plane/src/router.ts @@ -472,8 +472,10 @@ async function enforceAutomationRequirement( try { const authorization = ctx.authorization; if (!authorization) throw new Error("Missing request authorization"); - const automation = await new AutomationStore(ctx.db).getById(automationId); - if (!automation) return error("Automation not found", 404); + const store = new AutomationStore(ctx.db); + const storedAutomation = await store.getById(automationId); + if (!storedAutomation) return error("Automation not found", 404); + const automation = await store.resolveCanonicalOwner(storedAutomation); const permissionStem = `automations.${requirement.operation}` as const; const permissionScope = resolveScopedPermission(permissionStem, authorization.permissions); @@ -488,6 +490,7 @@ async function enforceAutomationRequirement( ); } + ctx.automationAdmission = { automation }; return null; } catch { return json({ error: "Authorization unavailable", code: "authorization_unavailable" }, 503); diff --git a/packages/control-plane/src/routes/automations.test.ts b/packages/control-plane/src/routes/automations.test.ts index 769ca4953..1a7adff02 100644 --- a/packages/control-plane/src/routes/automations.test.ts +++ b/packages/control-plane/src/routes/automations.test.ts @@ -13,14 +13,28 @@ import type { Principal } from "../auth/principal"; import type { SqlDatabase } from "../db/sql-database"; import type { Env } from "../types"; import { TEST_BACKGROUND_TASK_CONTEXT } from "../router.test-support"; -import { AutomationTriggerBlockedError } from "../scheduler/scheduler"; +import { + AutomationExecutionUnauthorizedError, + AutomationTriggerBlockedError, +} from "../scheduler/scheduler"; +import { PERMISSION_IDS, type PermissionId } from "@open-inspect/shared/rbac"; const mockProviderAdapterGet = vi.hoisted(() => vi.fn()); +const mockResolveGitHubCredentialAuthority = vi.hoisted(() => vi.fn()); +const mockResolveGitHubEnrichmentForRequest = vi.hoisted(() => vi.fn()); vi.mock("../auth/model-provider-account-default-adapters", () => ({ modelProviderAccountAdapterRegistry: { get: mockProviderAdapterGet }, })); +vi.mock("../source-control/github-credential-authority", () => ({ + resolveGitHubCredentialAuthority: mockResolveGitHubCredentialAuthority, +})); + +vi.mock("../session/identity", () => ({ + resolveGitHubEnrichmentForRequest: mockResolveGitHubEnrichmentForRequest, +})); + // ─── Mocks ────────────────────────────────────────────────────────────────── const mockStore = { @@ -38,6 +52,9 @@ const mockStore = { getEnvironmentsForAutomationIds: vi.fn(), bindAutomationInsert: vi.fn(), bindAutomationUpdate: vi.fn(), + bindSoftDelete: vi.fn(), + bindPause: vi.fn(), + bindResume: vi.fn(), bindRepositoryInserts: vi.fn(), bindReplaceRepositories: vi.fn(), bindEnvironmentInserts: vi.fn(), @@ -85,8 +102,18 @@ const MockAutomationTriggerBlockedError = vi.hoisted( } } ); +const MockAutomationExecutionUnauthorizedError = vi.hoisted( + () => + class AutomationExecutionUnauthorizedError extends Error { + constructor() { + super("Automation owner is not authorized to execute"); + this.name = "AutomationExecutionUnauthorizedError"; + } + } +); vi.mock("../scheduler/scheduler", () => ({ + AutomationExecutionUnauthorizedError: MockAutomationExecutionUnauthorizedError, AutomationTriggerBlockedError: MockAutomationTriggerBlockedError, Scheduler: vi.fn().mockImplementation(function () { return { trigger: mockSchedulerTrigger }; @@ -142,17 +169,6 @@ vi.mock("./shared", async (importOriginal) => { // ─── Helpers ──────────────────────────────────────────────────────────────── -/** Find the handler for a given method + path from automationRoutes. */ -function getHandler(method: string, path: string) { - for (const route of automationRoutes) { - if (route.method === method && route.pattern.test(path)) { - const match = path.match(route.pattern)!; - return { handler: route.handler, match }; - } - } - throw new Error(`No route found for ${method} ${path}`); -} - function createEnv(): Env { return { DB: { batch: mockBatch } as unknown as D1Database, @@ -178,21 +194,30 @@ const SLACK_BOT_PRINCIPAL: Principal = { }, }; -function createCtx(principal: Principal = USER_PRINCIPAL): RequestContext { +function createCtx( + principal: Principal = USER_PRINCIPAL, + permissions: readonly PermissionId[] = PERMISSION_IDS +): RequestContext { const statement = { - bind: vi.fn(), - first: vi.fn(async () => ({ active: 1 })), + bind: vi.fn(() => statement), + first: vi.fn(async () => ({ satisfied: 1 })), + all: vi.fn(async () => ({ results: [] })), }; - statement.bind.mockReturnValue(statement); - return { trace_id: "trace-1", request_id: "req-1", principal, - db: { - batch: mockBatch, - prepare: vi.fn(() => statement), - } as unknown as SqlDatabase, + ...(principal.kind === "user" + ? { + authorization: { + userId: principal.userId, + suspendedAt: null, + role: { id: "role_builtin_owner", key: "owner" as const, name: "Owner" }, + permissions: [...permissions], + }, + } + : {}), + db: { batch: mockBatch, prepare: vi.fn(() => statement) } as unknown as SqlDatabase, executionCtx: TEST_BACKGROUND_TASK_CONTEXT, metrics: { d1Queries: [], @@ -210,9 +235,14 @@ async function callRoute( body?: unknown; query?: Record; principal?: Principal; + permissions?: readonly PermissionId[]; } ): Promise { - const { handler, match } = getHandler(method, path); + const route = automationRoutes.find( + (candidate) => candidate.method === method && candidate.pattern.test(path) + ); + if (!route) throw new Error(`No route found for ${method} ${path}`); + const match = path.match(route.pattern)!; const url = new URL(`https://test.local${path}`); if (options?.query) { for (const [k, v] of Object.entries(options.query)) { @@ -226,7 +256,20 @@ async function callRoute( init.headers = { "Content-Type": "application/json" }; init.body = JSON.stringify(options.body); } - return handler(new Request(url, init), createEnv(), match, createCtx(options?.principal)); + const ctx = createCtx(options?.principal, options?.permissions); + const automationRequirement = + route.authorization.kind === "active-user" + ? route.authorization.allOf.find((requirement) => requirement.kind === "automation") + : undefined; + if (automationRequirement) { + const automation = await mockStore.getById( + match.groups?.[automationRequirement.automationIdParam] + ); + if (!automation) + return new Response(JSON.stringify({ error: "Automation not found" }), { status: 404 }); + ctx.automationAdmission = { automation }; + } + return route.handler(new Request(url, init), createEnv(), match, ctx); } // ─── Sample data ──────────────────────────────────────────────────────────── @@ -267,13 +310,16 @@ describe("automation route handlers", () => { mockProviderAuthStore.listForAutomationIds.mockResolvedValue(new Map()); mockStore.bindAutomationInsert.mockReturnValue({ sql: "insert-automation" }); mockStore.bindAutomationUpdate.mockReturnValue({ sql: "update-automation" }); + mockStore.bindSoftDelete.mockReturnValue({ sql: "delete-automation" }); + mockStore.bindPause.mockReturnValue({ sql: "pause-automation" }); + mockStore.bindResume.mockReturnValue({ sql: "resume-automation" }); mockStore.bindRepositoryInserts.mockReturnValue([{ sql: "insert-repositories" }]); mockStore.bindReplaceRepositories.mockReturnValue([{ sql: "replace-repositories" }]); mockStore.bindEnvironmentInserts.mockReturnValue([{ sql: "insert-environments" }]); mockStore.bindReplaceEnvironments.mockReturnValue([{ sql: "replace-environments" }]); mockProviderAuthStore.bindInserts.mockReturnValue([{ sql: "insert-provider-auth" }]); mockProviderAuthStore.bindReplace.mockReturnValue([{ sql: "replace-provider-auth" }]); - mockBatch.mockResolvedValue([]); + mockBatch.mockResolvedValue([{ meta: { changes: 1 }, results: [] }]); mockSchedulerTrigger.mockResolvedValue({ invocationId: "inv-1", runs: [{ id: "run-1" }], @@ -286,6 +332,8 @@ describe("automation route handlers", () => { archivedAt: null, }); mockProviderAdapterGet.mockReturnValue({}); + mockResolveGitHubCredentialAuthority.mockResolvedValue({ kind: "legacy" }); + mockResolveGitHubEnrichmentForRequest.mockResolvedValue(null); vi.mocked(resolveRepoOrError).mockResolvedValue({ repoId: 12345, repoOwner: "acme", @@ -699,6 +747,28 @@ describe("automation route handlers", () => { expect(await res.json()).toEqual({ error: "Environment not found: env_a, env_b" }); }); + it("checks environment-use permission before disclosing whether an environment exists", async () => { + mockEnvironmentStore.getById.mockResolvedValue(null); + + const res = await callRoute("POST", "/automations", { + body: { + name: "Workspace sync", + scheduleCron: "0 9 * * *", + scheduleTz: "UTC", + instructions: "Run tests", + environmentIds: ["env_missing"], + }, + permissions: PERMISSION_IDS.filter((permission) => permission !== "environments.use"), + }); + + expect(res.status).toBe(403); + await expect(res.json()).resolves.toMatchObject({ + code: "permission_required", + permission: "environments.use", + }); + expect(mockEnvironmentStore.getById).not.toHaveBeenCalled(); + }); + it("rejects malformed environment ids", async () => { const res = await callRoute("POST", "/automations", { body: { @@ -1004,6 +1074,51 @@ describe("automation route handlers", () => { }); describe("PUT /automations/:id (update)", () => { + it.each([ + ["repository", { repositories: [] }, "repositories.use"], + ["environment", { environmentIds: [] }, "environments.use"], + ] as const)( + "allows clearing a %s replacement without target-use permission", + async (_target, body, permission) => { + mockStore.getById.mockResolvedValue(sampleRow); + + const res = await callRoute("PUT", "/automations/auto-1", { + body, + permissions: PERMISSION_IDS.filter((candidate) => candidate !== permission), + }); + + expect(res.status).toBe(200); + expect(mockBatch).toHaveBeenCalled(); + } + ); + + it.each([ + [ + "repository", + { repositories: [{ repoOwner: "acme", repoName: "api" }] }, + "repositories.use", + ], + ["environment", { environmentIds: ["env_1"] }, "environments.use"], + ] as const)( + "requires target-use permission for a non-empty %s replacement", + async (_target, body, permission) => { + mockStore.getById.mockResolvedValue(sampleRow); + + const res = await callRoute("PUT", "/automations/auto-1", { + body, + permissions: PERMISSION_IDS.filter((candidate) => candidate !== permission), + }); + + expect(res.status).toBe(403); + await expect(res.json()).resolves.toEqual({ + error: "Forbidden", + code: "permission_required", + permission, + }); + expect(mockBatch).not.toHaveBeenCalled(); + } + ); + it("updates automation fields", async () => { mockStore.getById.mockResolvedValue(sampleRow); @@ -1560,8 +1675,6 @@ describe("automation route handlers", () => { describe("DELETE /automations/:id", () => { it("soft-deletes automation", async () => { - mockStore.softDelete.mockResolvedValue(true); - const res = await callRoute("DELETE", "/automations/auto-1"); expect(res.status).toBe(200); @@ -1570,7 +1683,7 @@ describe("automation route handlers", () => { }); it("returns 404 when not found", async () => { - mockStore.softDelete.mockResolvedValue(false); + mockBatch.mockResolvedValue([{ meta: { changes: 0 }, results: [] }]); const res = await callRoute("DELETE", "/automations/missing"); expect(res.status).toBe(404); @@ -1579,16 +1692,15 @@ describe("automation route handlers", () => { describe("POST /automations/:id/pause", () => { it("pauses automation", async () => { - mockStore.pause.mockResolvedValue(true); mockStore.getById.mockResolvedValue({ ...sampleRow, enabled: 0 }); const res = await callRoute("POST", "/automations/auto-1/pause"); expect(res.status).toBe(200); - expect(mockStore.pause).toHaveBeenCalledWith("auto-1"); + expect(mockStore.bindPause).toHaveBeenCalledWith("auto-1"); }); it("returns 404 when not found", async () => { - mockStore.pause.mockResolvedValue(false); + mockBatch.mockResolvedValue([{ meta: { changes: 0 }, results: [] }]); const res = await callRoute("POST", "/automations/missing/pause"); expect(res.status).toBe(404); @@ -1598,11 +1710,10 @@ describe("automation route handlers", () => { describe("POST /automations/:id/resume", () => { it("resumes automation and recomputes next_run_at", async () => { mockStore.getById.mockResolvedValue({ ...sampleRow, enabled: 0 }); - mockStore.resume.mockResolvedValue(true); const res = await callRoute("POST", "/automations/auto-1/resume"); expect(res.status).toBe(200); - expect(mockStore.resume).toHaveBeenCalledWith("auto-1", expect.any(Number)); + expect(mockStore.bindResume).toHaveBeenCalledWith("auto-1", expect.any(Number)); }); it("returns 404 when not found", async () => { @@ -1640,12 +1751,28 @@ describe("automation route handlers", () => { expect(mockStore.update).not.toHaveBeenCalled(); } ); + + it("returns 404 when the key update affects no current automation", async () => { + mockStore.getById.mockResolvedValue({ ...sampleRow, trigger_type: "webhook" }); + mockBatch.mockResolvedValue([{ meta: { changes: 0 }, results: [] }]); + + const res = await callRoute("POST", "/automations/auto-1/regenerate-key"); + + expect(res.status).toBe(404); + await expect(res.json()).resolves.toEqual({ error: "Automation not found" }); + }); }); describe("POST /automations/:id/trigger", () => { it("triggers automation via the scheduler", async () => { mockStore.getById.mockResolvedValue(sampleRow); mockStore.getActiveRunForAutomation.mockResolvedValue(null); + const enrichment = { + scmUserId: "123", + scmLogin: "requester", + accessTokenEncrypted: "encrypted-access", + }; + mockResolveGitHubEnrichmentForRequest.mockResolvedValue(enrichment); const res = await callRoute("POST", "/automations/auto-1/trigger"); expect(res.status).toBe(201); @@ -1653,6 +1780,7 @@ describe("automation route handlers", () => { invocationId: "inv-1", runs: [{ id: "run-1" }], }); + expect(mockSchedulerTrigger).toHaveBeenCalledWith("auto-1", "user-1", enrichment); }); it("returns 404 when automation not found", async () => { @@ -1665,20 +1793,25 @@ describe("automation route handlers", () => { it("returns 409 when the scheduler reports an active run", async () => { mockStore.getById.mockResolvedValue(sampleRow); - const env = createEnv(); mockSchedulerTrigger.mockRejectedValue(new AutomationTriggerBlockedError()); - const { handler, match } = getHandler("POST", "/automations/auto-1/trigger"); - const request = new Request("https://test.local/automations/auto-1/trigger", { - method: "POST", - }); - const res = await handler(request, env, match, createCtx()); + const res = await callRoute("POST", "/automations/auto-1/trigger"); expect(res.status).toBe(409); expect(await res.json()).toEqual({ error: "A run is already active for this automation", }); }); + it("returns 403 when the owner is unauthorized to execute", async () => { + mockStore.getById.mockResolvedValue(sampleRow); + mockSchedulerTrigger.mockRejectedValue(new AutomationExecutionUnauthorizedError()); + + const res = await callRoute("POST", "/automations/auto-1/trigger"); + + expect(res.status).toBe(403); + expect(await res.json()).toEqual({ error: "Execution authorization required" }); + }); + it("returns 500 when the scheduler cannot launch the automation", async () => { mockStore.getById.mockResolvedValue(sampleRow); mockSchedulerTrigger.mockRejectedValue(new Error("launch failed")); diff --git a/packages/control-plane/src/routes/automations.ts b/packages/control-plane/src/routes/automations.ts index fc638ed7f..5bfd9b0f0 100644 --- a/packages/control-plane/src/routes/automations.ts +++ b/packages/control-plane/src/routes/automations.ts @@ -19,6 +19,7 @@ import { } from "@open-inspect/shared/types/automations"; import type { ModelProviderSelections } from "@open-inspect/shared/types/provider-accounts"; import { listChannels } from "@open-inspect/shared/slack"; +import type { PermissionId } from "@open-inspect/shared/rbac"; import { getValidModelOrDefault, isValidModel, @@ -48,7 +49,11 @@ import { generateId } from "../auth/crypto"; import { applyIdentityEnforcement, resolveCanonicalUserId } from "../auth/identity-enforcement"; import { generateWebhookApiKey, hashApiKey, encryptSentrySecret } from "../auth/webhook-key"; import { createLogger } from "../logger"; -import { AutomationTriggerBlockedError, Scheduler } from "../scheduler/scheduler"; +import { + AutomationExecutionUnauthorizedError, + AutomationTriggerBlockedError, + Scheduler, +} from "../scheduler/scheduler"; import { hydrateAutomation } from "../automation/hydrate"; import { MAX_AUTOMATION_REPOSITORIES } from "@open-inspect/shared/types/automations"; import { @@ -63,14 +68,40 @@ import { resolveRepoOrError, requireAutomation, requirePermission, + type AutomationRouteAdmission, } from "./shared"; import type { Env } from "../types"; import type { SqlDatabase, SqlStatement } from "../db/sql-database"; import { z } from "zod"; import { ProviderAccountSelectionPolicyError } from "../model-provider-accounts/selection-policy"; +import { resolveGitHubCredentialAuthority } from "../source-control/github-credential-authority"; +import { resolveGitHubEnrichmentForRequest } from "../session/identity"; const logger = createLogger("router:automations"); +function requireTargetPermissions( + ctx: RequestContext, + requiredPermissions: readonly PermissionId[] +): Response | null { + const authorization = ctx.authorization; + if (!authorization) return json({ error: "Authorization unavailable" }, 503); + const missingPermission = requiredPermissions.find( + (permission) => !authorization.permissions.includes(permission) + ); + if (missingPermission) { + return json( + { error: "Forbidden", code: "permission_required", permission: missingPermission }, + 403 + ); + } + return null; +} + +function admittedAutomation(ctx: RequestContext): AutomationRouteAdmission { + if (!ctx.automationAdmission) throw new Error("Missing automation route admission"); + return ctx.automationAdmission; +} + /** Minimum cron interval in minutes. */ const MIN_CRON_INTERVAL_MINUTES = 15; @@ -563,6 +594,18 @@ async function handleCreateAutomation( requestedEnvironmentIds = environmentSelection.kind === "replace" ? environmentSelection.environmentIds : []; validateTargetCounts(triggerType, requestedRepositories.length, requestedEnvironmentIds.length); + } catch (e) { + if (e instanceof TargetSelectionError) return error(e.message, 400); + throw e; + } + if (ctx.principal?.kind === "user") { + const targetAuthorizationError = requireTargetPermissions(ctx, [ + ...(requestedRepositories.length > 0 ? (["repositories.use"] as const) : []), + ...(requestedEnvironmentIds.length > 0 ? (["environments.use"] as const) : []), + ]); + if (targetAuthorizationError) return targetAuthorizationError; + } + try { await resolveEnvironmentSelection(ctx.db, requestedEnvironmentIds); } catch (e) { if (e instanceof TargetSelectionError) return error(e.message, 400); @@ -709,7 +752,7 @@ async function handleCreateAutomation( ...slackStore.bindChannelStatements(row.id, extractSlackChannels(body.triggerConfig)) ); } - await db.batch(createStatements); + await ctx.db.batch(createStatements); const automation = await hydrateAutomation(db, (await store.getById(id))!); @@ -776,8 +819,8 @@ async function handleUpdateAutomation( const db: SqlDatabase = ctx.db; const store = new AutomationStore(db); const providerAuthStore = new AutomationModelProviderAuthStore(db); - const existing = await store.getById(id); - if (!existing) return error("Automation not found", 404); + const admission = admittedAutomation(ctx); + const { automation: existing } = admission; const rawBody = await parseJsonBody(request); if (rawBody instanceof Response) return rawBody; @@ -876,6 +919,18 @@ async function handleUpdateAutomation( // it simply applies from the next invocation. const selection = getRepositorySelection(body); const environmentSelection = getEnvironmentSelection(body); + const requiredTargetPermissions: PermissionId[] = [ + ...(selection.kind === "replace" && selection.repositories.length > 0 + ? (["repositories.use"] as const) + : []), + ...(environmentSelection.kind === "replace" && environmentSelection.environmentIds.length > 0 + ? (["environments.use"] as const) + : []), + ]; + if (requiredTargetPermissions.length > 0) { + const targetAuthorizationError = requireTargetPermissions(ctx, requiredTargetPermissions); + if (targetAuthorizationError) return targetAuthorizationError; + } // The count rules span both selections, so when EITHER is replaced they are // validated against the automation's FINAL state (the replacement plus the @@ -1038,7 +1093,7 @@ async function handleUpdateAutomation( ); } if (statements.length > 0) { - await db.batch(statements); + await ctx.db.batch(statements); } const updated = await store.getById(id); if (!updated) return error("Automation not found", 404); @@ -1063,7 +1118,9 @@ async function handleDeleteAutomation( if (!id) return error("Automation ID required", 400); const store = new AutomationStore(ctx.db); - const deleted = await store.softDelete(id); + admittedAutomation(ctx); + const result = await ctx.db.batch([store.bindSoftDelete(id)]); + const deleted = result[0]?.meta.changes === 1; if (!deleted) return error("Automation not found", 404); logger.info("automation.deleted", { @@ -1086,7 +1143,9 @@ async function handlePauseAutomation( if (!id) return error("Automation ID required", 400); const store = new AutomationStore(ctx.db); - const paused = await store.pause(id); + admittedAutomation(ctx); + const result = await ctx.db.batch([store.bindPause(id)]); + const paused = result[0]?.meta.changes === 1; if (!paused) return error("Automation not found", 404); logger.info("automation.paused", { @@ -1112,8 +1171,7 @@ async function handleResumeAutomation( if (!id) return error("Automation ID required", 400); const store = new AutomationStore(ctx.db); - const existing = await store.getById(id); - if (!existing) return error("Automation not found", 404); + const { automation: existing } = admittedAutomation(ctx); // For schedule automations, compute the next run time. // For event-driven automations, resume with null next_run_at. @@ -1127,7 +1185,8 @@ async function handleResumeAutomation( nextRunAt = null; } - const resumed = await store.resume(id, nextRunAt); + const result = await ctx.db.batch([store.bindResume(id, nextRunAt)]); + const resumed = result[0]?.meta.changes === 1; if (!resumed) return error("Automation not found", 404); logger.info("automation.resumed", { @@ -1145,7 +1204,7 @@ async function handleResumeAutomation( } async function handleTriggerAutomation( - _request: Request, + request: Request, env: Env, match: RegExpMatchArray, ctx: RequestContext @@ -1153,14 +1212,37 @@ async function handleTriggerAutomation( const id = match.groups?.id; if (!id) return error("Automation ID required", 400); - const store = new AutomationStore(ctx.db); - const automation = await store.getById(id); - if (!automation) return error("Automation not found", 404); + admittedAutomation(ctx); + const requesterUserId = ctx.authorization?.userId; + if (!requesterUserId) return error("Authorization unavailable", 503); + + let requesterEnrichment; + try { + requesterEnrichment = await resolveGitHubEnrichmentForRequest( + env, + ctx.db, + new UserStore(ctx.db), + requesterUserId, + await resolveGitHubCredentialAuthority(ctx, request.headers) + ); + } catch (enrichmentError) { + logger.warn("Failed to enrich manual automation trigger with GitHub identity", { + error: + enrichmentError instanceof Error ? enrichmentError : new Error(String(enrichmentError)), + automation_id: id, + request_id: ctx.request_id, + trace_id: ctx.trace_id, + }); + } // The scheduler performs the authoritative D1-backed concurrency check. let triggerResult; try { - triggerResult = await new Scheduler(ctx.db, env, ctx.executionCtx).trigger(id); + triggerResult = await new Scheduler(ctx.db, env, ctx.executionCtx).trigger( + id, + requesterUserId, + requesterEnrichment ?? undefined + ); } catch (triggerError) { logger.error("automation.trigger_failed", { event: "automation.trigger_failed", @@ -1172,6 +1254,9 @@ async function handleTriggerAutomation( if (triggerError instanceof AutomationTriggerBlockedError) { return error("A run is already active for this automation", 409); } + if (triggerError instanceof AutomationExecutionUnauthorizedError) { + return json({ error: "Execution authorization required" }, 403); + } return error("Failed to trigger automation", 500); } @@ -1242,8 +1327,7 @@ async function handleRegenerateKey( if (!id) return error("Automation ID required", 400); const store = new AutomationStore(ctx.db); - const automation = await store.getById(id); - if (!automation) return error("Automation not found", 404); + const { automation } = admittedAutomation(ctx); const workerUrl = env.WORKER_URL || ""; @@ -1262,7 +1346,12 @@ async function handleRegenerateKey( parsedBody.data.sentryClientSecret, env.REPO_SECRETS_ENCRYPTION_KEY ); - await store.update(id, { trigger_auth_data: encrypted } as Record); + const statement = store.bindAutomationUpdate(id, { + trigger_auth_data: encrypted, + } as Record); + if (!statement) return error("Automation not found", 404); + const result = await ctx.db.batch([statement]); + if ((result[0]?.meta.changes ?? 0) === 0) return error("Automation not found", 404); logger.info("automation.secret_updated", { event: "automation.secret_updated", @@ -1284,7 +1373,12 @@ async function handleRegenerateKey( const apiKey = generateWebhookApiKey(); const hash = await hashApiKey(apiKey); - await store.update(id, { trigger_auth_data: hash } as Record); + const statement = store.bindAutomationUpdate(id, { + trigger_auth_data: hash, + } as Record); + if (!statement) return error("Automation not found", 404); + const result = await ctx.db.batch([statement]); + if ((result[0]?.meta.changes ?? 0) === 0) return error("Automation not found", 404); logger.info("automation.key_regenerated", { event: "automation.key_regenerated", diff --git a/packages/control-plane/src/routes/shared.ts b/packages/control-plane/src/routes/shared.ts index 1696f07b6..df2555306 100644 --- a/packages/control-plane/src/routes/shared.ts +++ b/packages/control-plane/src/routes/shared.ts @@ -17,6 +17,7 @@ import type { ScopedPermissionStem, } from "@open-inspect/shared/rbac"; import type { ServiceName } from "@open-inspect/shared/service-auth"; +import type { AutomationRow } from "../db/automation-store"; import { createSourceControlProviderFromEnv, SourceControlProviderError, @@ -50,8 +51,15 @@ export type RequestContext = CorrelationContext & { authentication?: AuthenticationContext; /** Effective human authorization loaded once by the router for this request. */ authorization?: EffectiveAuthorization; + /** Resource admission populated by the router for automation mutation routes. */ + automationAdmission?: AutomationRouteAdmission; }; +/** Automation resource admitted by the router for the current mutation. */ +export interface AutomationRouteAdmission { + automation: AutomationRow; +} + /** Route matching, authorization, and handler configuration. */ export interface RouteDefinition { method: string; diff --git a/packages/control-plane/src/scheduler/scheduler.test.ts b/packages/control-plane/src/scheduler/scheduler.test.ts index 66071cf5b..355627edb 100644 --- a/packages/control-plane/src/scheduler/scheduler.test.ts +++ b/packages/control-plane/src/scheduler/scheduler.test.ts @@ -20,6 +20,8 @@ const mockResolveSessionProviderAuth = vi.hoisted(() => { provider: "xai", authMode: "api_key", selectionSource: "unattended_policy" }, ]) ); +const mockIsAutomationExecutionAuthorized = vi.hoisted(() => vi.fn().mockResolvedValue(true)); +const mockIsPrincipalAuthorized = vi.hoisted(() => vi.fn().mockResolvedValue(true)); vi.mock("../source-control", () => ({ createSourceControlProviderFromEnv: vi.fn(() => ({ @@ -31,6 +33,15 @@ vi.mock("../session/provider-account-resolution", () => ({ resolveSessionProviderAuth: mockResolveSessionProviderAuth, })); +vi.mock("../automation/authorization-guard", async (importOriginal) => { + const actual = (await importOriginal()) as Record; + return { + ...actual, + isAutomationExecutionAuthorized: mockIsAutomationExecutionAuthorized, + isPrincipalAuthorized: mockIsPrincipalAuthorized, + }; +}); + vi.mock("../session/skill-resolution", () => ({ resolveManagedSkills: vi.fn(async () => ({ selection: { mode: "all" }, @@ -41,7 +52,7 @@ vi.mock("../session/skill-resolution", () => ({ })), })); -const { Scheduler } = await import("./scheduler"); +const { AutomationExecutionUnauthorizedError, Scheduler } = await import("./scheduler"); // ─── Mock factories ────────────────────────────────────────────────────────── @@ -75,6 +86,7 @@ function createMockStore() { getRepositoriesForAutomationIds: vi.fn().mockResolvedValue(new Map()), getEnvironmentsForAutomation: vi.fn().mockResolvedValue([]), getEnvironmentsForAutomationIds: vi.fn().mockResolvedValue(new Map()), + resolveCanonicalOwner: vi.fn(async (automation: unknown) => automation), insertInvocationGuarded: vi.fn().mockImplementation(async (params: unknown) => { capturedInvocationParams.push( structuredClone(params) as { children: Array> } @@ -82,6 +94,7 @@ function createMockStore() { return { inserted: true }; }), insertSkippedInvocation: vi.fn().mockResolvedValue({ inserted: true }), + recordAuthorizationDenied: vi.fn().mockResolvedValue({ inserted: true, paused: true }), getInvocationById: vi.fn().mockResolvedValue(null), getInvocationRunAggregate: vi.fn().mockResolvedValue(aggregate()), tryMarkInvocationFailureCounted: vi.fn().mockResolvedValue(true), @@ -202,6 +215,7 @@ function createEmptyDbMock(): D1Database { prepare: vi.fn(() => ({ bind: vi.fn(() => ({ first: vi.fn(async () => null), + run: vi.fn(async () => undefined), })), })), } as unknown as D1Database; @@ -349,7 +363,7 @@ const sampleAutomation = { next_run_at: now - 60000, consecutive_failures: 0, created_by: "user-1", - user_id: null as string | null, + user_id: "user-1" as string | null, created_at: now - 86400000, updated_at: now - 86400000, deleted_at: null, @@ -475,6 +489,11 @@ describe("Scheduler", () => { { provider: "xai", authMode: "api_key", selectionSource: "unattended_policy" }, ]); mockProviderAuthList.mockResolvedValue([]); + mockIsAutomationExecutionAuthorized.mockResolvedValue(true); + mockIsPrincipalAuthorized.mockResolvedValue(true); + mockUserStoreGetIdentity.mockImplementation(async (provider: string) => + provider === "slack" ? { userId: "slack-actor-user" } : null + ); capturedInvocationParams = []; mockStore = createMockStore(); mockGetSlackAutomationsForChannel.mockResolvedValue([]); @@ -488,7 +507,8 @@ describe("Scheduler", () => { describe("tick", () => { it("returns empty summary when no overdue automations", async () => { - const scheduler = createScheduler(); + const env = createEnv(); + const scheduler = createScheduler(env); const result = await scheduler.tick(); expect(result).toEqual({ processed: 0, skipped: 0, failed: 0 }); @@ -498,7 +518,9 @@ describe("Scheduler", () => { mockStore.getOverdueAutomations.mockResolvedValue([sampleAutomation]); selectRepositories("auto-1", [repositoryRow("auto-1")]); - const scheduler = createScheduler(); + const env = createEnv(); + const fetchMock = vi.mocked(env.SESSION.get(env.SESSION.idFromName("auto-1")).fetch); + const scheduler = createScheduler(env); const result = await scheduler.tick(); expect(result).toMatchObject({ processed: 1 }); @@ -522,6 +544,39 @@ describe("Scheduler", () => { expect.any(String), expect.any(Number) ); + await expect(getInitBody(fetchMock)).resolves.toMatchObject({ + userId: sampleAutomation.created_by, + canonicalUserId: sampleAutomation.user_id, + }); + await expect(getPromptBody(fetchMock)).resolves.toMatchObject({ + authorId: sampleAutomation.created_by, + canonicalUserId: sampleAutomation.user_id, + }); + }); + + it("rejects unattended execution before invocation work when the owner is unauthorized", async () => { + mockStore.getOverdueAutomations.mockResolvedValue([sampleAutomation]); + selectRepositories("auto-1", [repositoryRow("auto-1")]); + mockIsAutomationExecutionAuthorized.mockResolvedValue(false); + + const result = await createScheduler().tick(); + + expect(result).toEqual({ processed: 0, skipped: 1, failed: 0 }); + expect(mockIsAutomationExecutionAuthorized).toHaveBeenCalledWith(expect.anything(), { + automationId: "auto-1", + executionUserId: "user-1", + requiresRepositoryUse: true, + requiresEnvironmentUse: false, + }); + expect(mockStore.insertInvocationGuarded).not.toHaveBeenCalled(); + expect(mockResolveSessionProviderAuth).not.toHaveBeenCalled(); + expect(mockStore.recordAuthorizationDenied).toHaveBeenCalledWith( + expect.objectContaining({ + automation_id: "auto-1", + skip_reason: "execution_authorization_denied", + }), + sampleAutomation.next_run_at + ); }); it("does not enqueue a prompt when recovery wins the launch transition", async () => { @@ -1329,31 +1384,38 @@ describe("Scheduler", () => { ); }); - it("falls back to identity lookup for legacy automations without user_id", async () => { - mockStore.getOverdueAutomations.mockResolvedValue([sampleAutomation]); + it("repairs legacy automation identity before invocation admission", async () => { + const legacyAutomation = { ...sampleAutomation, user_id: null }; + mockStore.getOverdueAutomations.mockResolvedValue([legacyAutomation]); selectRepositories("auto-1", [repositoryRow("auto-1")]); - mockUserStoreGetIdentity.mockResolvedValue({ userId: "looked-up-user" }); + mockStore.resolveCanonicalOwner.mockResolvedValue({ + ...legacyAutomation, + user_id: "looked-up-user", + }); const scheduler = createScheduler(); await scheduler.tick(); - expect(mockUserStoreGetIdentity).toHaveBeenCalledWith("github", "user-1"); + expect(mockStore.resolveCanonicalOwner).toHaveBeenCalledWith(legacyAutomation); + expect(mockStore.resolveCanonicalOwner.mock.invocationCallOrder[0]).toBeLessThan( + mockIsAutomationExecutionAuthorized.mock.invocationCallOrder[0] + ); expect(mockSessionStoreCreate).toHaveBeenCalledWith( expect.objectContaining({ userId: "looked-up-user" }) ); }); - it("creates session with null userId when identity lookup finds nothing", async () => { - mockStore.getOverdueAutomations.mockResolvedValue([sampleAutomation]); + it("rejects a legacy automation when identity lookup finds nothing", async () => { + const legacyAutomation = { ...sampleAutomation, user_id: null }; + mockStore.getOverdueAutomations.mockResolvedValue([legacyAutomation]); selectRepositories("auto-1", [repositoryRow("auto-1")]); - mockUserStoreGetIdentity.mockResolvedValue(null); + mockStore.resolveCanonicalOwner.mockResolvedValue(legacyAutomation); - const scheduler = createScheduler(); - await scheduler.tick(); + const result = await createScheduler().tick(); - expect(mockSessionStoreCreate).toHaveBeenCalledWith( - expect.objectContaining({ userId: null }) - ); + expect(result).toEqual({ processed: 0, skipped: 1, failed: 0 }); + expect(mockStore.recordAuthorizationDenied).toHaveBeenCalled(); + expect(mockSessionStoreCreate).not.toHaveBeenCalled(); }); it("swallows launch-failure tracking errors and logs scheduler.fail_track_error", async () => { @@ -2011,7 +2073,9 @@ describe("Scheduler", () => { mockStore.getById.mockResolvedValue(null); const scheduler = createScheduler(); - await expect(scheduler.trigger("nonexistent")).rejects.toThrow("Automation not found"); + await expect(scheduler.trigger("nonexistent", "user-1")).rejects.toThrow( + "Automation not found" + ); }); it("rejects when active run exists, recording nothing", async () => { @@ -2019,18 +2083,41 @@ describe("Scheduler", () => { mockStore.getActiveRunForAutomation.mockResolvedValue({ id: "run-active" }); const scheduler = createScheduler(); - await expect(scheduler.trigger("auto-1")).rejects.toThrow("An active run already exists"); + await expect(scheduler.trigger("auto-1", "user-1")).rejects.toThrow( + "An active run already exists" + ); expect(mockStore.insertSkippedInvocation).not.toHaveBeenCalled(); expect(mockStore.insertInvocationGuarded).not.toHaveBeenCalled(); }); + it("rejects with a purpose-specific error when the owner cannot execute", async () => { + mockStore.getById.mockResolvedValue(sampleAutomation); + mockIsAutomationExecutionAuthorized.mockResolvedValue(false); + + const scheduler = createScheduler(); + await expect(scheduler.trigger("auto-1", "user-1")).rejects.toBeInstanceOf( + AutomationExecutionUnauthorizedError + ); + expect(mockStore.insertInvocationGuarded).not.toHaveBeenCalled(); + }); + it("creates an invocation and launches runs on successful trigger", async () => { mockStore.getById.mockResolvedValue(sampleAutomation); mockStore.getActiveRunForAutomation.mockResolvedValue(null); mockStore.getRepositoriesForAutomation.mockResolvedValue([repositoryRow("auto-1")]); - const scheduler = createScheduler(); - const result = await scheduler.trigger("auto-1"); + const env = createEnv(); + const fetchMock = vi.mocked(env.SESSION.get(env.SESSION.idFromName("auto-1")).fetch); + const scheduler = createScheduler(env); + const result = await scheduler.trigger("auto-1", "user-1", { + scmUserId: "123", + scmLogin: "requester", + displayName: "Requester", + email: "123+requester@users.noreply.github.com", + accessTokenEncrypted: "encrypted-access", + refreshTokenEncrypted: "encrypted-refresh", + tokenExpiresAt: 123456, + }); expect(result).toEqual({ invocationId: expect.any(String), @@ -2048,6 +2135,15 @@ describe("Scheduler", () => { expect.any(String), expect.any(Number) ); + await expect(getInitBody(fetchMock)).resolves.toMatchObject({ + scmUserId: "123", + scmLogin: "requester", + scmName: "Requester", + scmEmail: "123+requester@users.noreply.github.com", + scmTokenEncrypted: "encrypted-access", + scmRefreshTokenEncrypted: "encrypted-refresh", + scmTokenExpiresAt: 123456, + }); }); it("rejects when every launch fails, still recording the failed children", async () => { @@ -2071,7 +2167,9 @@ describe("Scheduler", () => { .spyOn((scheduler as unknown as { log: Logger }).log, "error") .mockImplementation(() => {}); - await expect(scheduler.trigger("auto-1")).rejects.toThrow("Failed to trigger automation"); + await expect(scheduler.trigger("auto-1", "user-1")).rejects.toThrow( + "Failed to trigger automation" + ); const failTrackCall = errorSpy.mock.calls.find( ([, data]) => @@ -2207,6 +2305,31 @@ describe("Scheduler", () => { expect(threadContextCalls(slackFetch)).toHaveLength(1); }); + it("continues fan-out after one matching automation is unauthorized", async () => { + mockGetSlackAutomationsForChannel.mockResolvedValue([ + sampleSlackAutomation, + { ...sampleSlackAutomation, id: "auto-slack-2" }, + ]); + mockStore.getLatestSteerableRunForThread.mockResolvedValue(null); + mockIsAutomationExecutionAuthorized + .mockResolvedValueOnce(false) + .mockResolvedValueOnce(true); + const { env } = threadContextEnv(); + + expect(await createScheduler(env).event(makeSlackEvent())).toEqual({ + triggered: 1, + skipped: 1, + steered: 0, + }); + + expect(mockStore.insertInvocationGuarded).toHaveBeenCalledTimes(1); + expect(mockStore.insertInvocationGuarded).toHaveBeenCalledWith( + expect.objectContaining({ + invocation: expect.objectContaining({ automation_id: "auto-slack-2" }), + }) + ); + }); + it("launches without history when the context request fails", async () => { mockGetSlackAutomationsForChannel.mockResolvedValue([sampleSlackAutomation]); mockStore.getLatestSteerableRunForThread.mockResolvedValue(null); @@ -2351,6 +2474,25 @@ describe("Scheduler", () => { expect(mockStore.insertSkippedInvocation).not.toHaveBeenCalled(); }); + it("resolves and authorizes the Slack actor once across several steering candidates", async () => { + mockGetSlackAutomationsForChannel.mockResolvedValue([ + sampleSlackAutomation, + { ...sampleSlackAutomation, id: "auto-slack-2" }, + ]); + mockStore.getLatestSteerableRunForThread.mockResolvedValue( + sampleRunRow({ id: "active-run", session_id: "sess-running" }) + ); + + expect(await createScheduler().event(makeSlackEvent({ text: "follow up" }))).toEqual({ + triggered: 0, + skipped: 0, + steered: 2, + }); + + expect(mockUserStoreGetIdentity).toHaveBeenCalledTimes(1); + expect(mockIsPrincipalAuthorized).toHaveBeenCalledTimes(1); + }); + it("continues the same session on a reply after the run has completed", async () => { mockGetSlackAutomationsForChannel.mockResolvedValue([sampleSlackAutomation]); // The thread's run finished, but its session is still steerable within the @@ -2441,7 +2583,9 @@ describe("Scheduler", () => { mockStore.getLatestSteerableRunForThread.mockResolvedValue(null); mockStore.getActiveRunForKey.mockResolvedValue(null); - const scheduler = createScheduler(); + const env = createEnv(); + const fetchMock = vi.mocked(env.SESSION.get(env.SESSION.idFromName("auto-slack")).fetch); + const scheduler = createScheduler(env); // Matching text so the trigger conditions pass. const result = await scheduler.event(makeSlackEvent()); @@ -2464,6 +2608,14 @@ describe("Scheduler", () => { automation_id: "auto-slack", status: "starting", }); + await expect(getInitBody(fetchMock)).resolves.toMatchObject({ + userId: sampleSlackAutomation.created_by, + canonicalUserId: sampleSlackAutomation.user_id, + }); + await expect(getPromptBody(fetchMock)).resolves.toMatchObject({ + authorId: sampleSlackAutomation.created_by, + canonicalUserId: sampleSlackAutomation.user_id, + }); }); it("appends workspace session instructions to a new Slack automation session", async () => { diff --git a/packages/control-plane/src/scheduler/scheduler.ts b/packages/control-plane/src/scheduler/scheduler.ts index de07e6351..5ae9e034a 100644 --- a/packages/control-plane/src/scheduler/scheduler.ts +++ b/packages/control-plane/src/scheduler/scheduler.ts @@ -70,8 +70,13 @@ import { resolveManagedSkills } from "../session/skill-resolution"; import type { EnqueuePromptRequest } from "../session/enqueue-prompt-contract"; import { resolveAutomationRepositories } from "../automation/repository"; import { resolveAutomationSessionTarget } from "../automation/session-target"; +import { + isAutomationExecutionAuthorized, + isPrincipalAuthorized, +} from "../automation/authorization-guard"; import type { RequestContext } from "../routes/shared"; import { deliverWithRetry } from "../session/callback-delivery"; +import type { GitHubEnrichment } from "../session/identity"; /** Max automations to process per tick (backpressure). */ const MAX_PER_TICK = 25; @@ -188,9 +193,20 @@ export class AutomationTriggerBlockedError extends Error { } } +/** Raised when an automation's execution principal lacks required authorization. */ +export class AutomationExecutionUnauthorizedError extends Error { + /** Create an error for an unauthorized automation execution principal. */ + constructor() { + super("Automation execution principal is not authorized"); + this.name = "AutomationExecutionUnauthorizedError"; + } +} + interface StartInvocationParams { automation: AutomationRow; source: AutomationInvocationSource; + /** Human authority used for a manual firing; unattended sources use the automation owner. */ + executionPrincipal?: ExecutionPrincipal; /** Cron slot being served — becomes scheduled_at and the idempotency key (schedule source only). */ scheduledAt?: number; /** Next cron slot, advanced atomically with the insert (schedule source only). */ @@ -215,6 +231,12 @@ interface StartInvocationParams { instructionsOverrideFactory?: () => Promise; } +interface ExecutionPrincipal { + platformUserId: string; + participantUserId: string; + scmEnrichment?: GitHubEnrichment; +} + type StartInvocationResult = /** Invocation inserted; children launched (some may have pre-failed). */ | { outcome: "started"; invocationId: string; runs: AutomationRunRow[]; launched: number } @@ -223,7 +245,9 @@ type StartInvocationResult = /** Overlap on a manual firing — nothing recorded; the caller answers 409. */ | { outcome: "blocked" } /** Idempotency/dedup collision — another firing owns this slot or event. */ - | { outcome: "deduplicated" }; + | { outcome: "deduplicated" } + /** The execution principal cannot launch the immutable target snapshot. */ + | { outcome: "unauthorized" }; type SchedulerPromptRequest = Pick< EnqueuePromptRequest, @@ -255,6 +279,7 @@ export function composeAutomationPrompt(contextBlock: string, instructions: stri return `${instructions}\n---\n\n${contextBlock}`; } +/** Coordinates authorized automation scheduling, dispatch, and completion handling. */ export class Scheduler { private readonly log: Logger; @@ -323,7 +348,17 @@ export class Scheduler { store: AutomationStore, params: StartInvocationParams ): Promise { - const { automation, source } = params; + const { source } = params; + const automation = await store.resolveCanonicalOwner(params.automation); + const executionPrincipal = + params.executionPrincipal ?? + (automation.user_id + ? { + platformUserId: automation.user_id, + participantUserId: automation.created_by, + } + : null); + if (!executionPrincipal) return { outcome: "unauthorized" }; const now = Date.now(); const concurrencyKey = params.concurrencyKey ?? null; @@ -335,7 +370,7 @@ export class Scheduler { ? { kind: "concurrencyKey", concurrencyKey } : { kind: "automation" }; - // Cheap pre-check; the guarded insert below re-applies the same predicate + // Cheap pre-check; the conditional insert below re-applies the same predicate // atomically, so a race here only costs a wasted child build. const activeRun = overlapScope.kind === "concurrencyKey" @@ -345,10 +380,20 @@ export class Scheduler { return this.recordOverlapSkip(store, params, { advanceSchedule: true }); } - const selection = - params.repositories ?? (await store.getRepositoriesForAutomation(automation.id)); - const environmentSelection = - params.environments ?? (await store.getEnvironmentsForAutomation(automation.id)); + const [selection, environmentSelection] = await Promise.all([ + params.repositories ?? store.getRepositoriesForAutomation(automation.id), + params.environments ?? store.getEnvironmentsForAutomation(automation.id), + ]); + if ( + !(await isAutomationExecutionAuthorized(this.db, { + automationId: automation.id, + executionUserId: executionPrincipal.platformUserId, + requiresRepositoryUse: selection.length > 0, + requiresEnvironmentUse: environmentSelection.length > 0, + })) + ) { + return { outcome: "unauthorized" }; + } const resolutions = await resolveAutomationRepositories(this.env, selection); const invocationId = generateId(); @@ -406,7 +451,7 @@ export class Scheduler { const launchCandidates = children.filter((child) => child.status === "starting"); // Resolve provider routing before admission, alongside the already-built // target children. Together these values are the immutable launch snapshot - // for this firing: edits made after the guarded insert cannot change which + // for this firing: edits made after the conditional insert cannot change which // account an admitted child uses. let providerAuthSnapshot: | { providerAuth: SessionModelProviderAuthInput[] } @@ -501,9 +546,16 @@ export class Scheduler { automation, child, providerAuthSnapshot.providerAuth, - sessionId + sessionId, + executionPrincipal + ); + await this.sendPromptToSession( + sessionId, + automation, + child.id, + executionPrincipal, + instructionsOverride ); - await this.sendPromptToSession(sessionId, automation, child.id, instructionsOverride); child.status = "running"; child.session_id = sessionId; } catch (e) { @@ -678,6 +730,32 @@ export class Scheduler { case "blocked": skipped++; break; + case "unauthorized": { + const deniedAt = Date.now(); + await store.recordAuthorizationDenied( + { + id: generateId(), + automation_id: automation.id, + source: "schedule", + scheduled_at: automation.next_run_at, + trigger_key: null, + concurrency_key: null, + trigger_metadata: null, + skip_reason: "execution_authorization_denied", + failure_counted_at: null, + created_at: deniedAt, + updated_at: deniedAt, + }, + automation.next_run_at! + ); + this.log.warn("Paused scheduled automation after execution authorization denial", { + event: "scheduler.authorization_denied", + automation_id: automation.id, + scheduled_at: automation.next_run_at, + }); + skipped++; + break; + } } } catch (e) { this.log.error("Unexpected error processing automation", { @@ -862,7 +940,7 @@ export class Scheduler { // ─── Event handler ─────────────────────────────────────────────────────── - /** Match an inbound event to automations and start or steer their invocations. */ + /** Match an inbound event to authorized automations and start or steer their invocations. */ async event(event: AutomationEvent): Promise { const store = new AutomationStore(this.db); @@ -910,6 +988,29 @@ export class Scheduler { slackContextPromise ??= this.buildSlackContextWithThread(slackEvent); return slackContextPromise; }; + let slackSteeringActorPromise: Promise | undefined; + const slackSteeringActor = (slackEvent: SlackAutomationEvent): Promise => { + slackSteeringActorPromise ??= (async () => { + try { + const identity = await new UserStore(this.db).getIdentity( + "slack", + slackEvent.actorUserId + ); + if (!identity) return null; + return (await isPrincipalAuthorized(this.db, identity.userId, "sessions.collaborate")) + ? identity.userId + : null; + } catch (error) { + this.log.warn("Failed to authorize slack actor for session steering", { + event: "scheduler.slack_steer_authorization_failed", + slack_actor_id: slackEvent.actorUserId, + error: error instanceof Error ? error : new Error(String(error)), + }); + return null; + } + })(); + return slackSteeringActorPromise; + }; let triggered = 0; let skipped = 0; @@ -938,9 +1039,21 @@ export class Scheduler { event.concurrencyKey, now - SLACK_THREAD_CONTINUITY_WINDOW_MS ); - if (steerable?.session_id && (await this.steerSession(steerable, automation, event))) { - steered++; - continue; + if (steerable?.session_id) { + const actorUserId = await slackSteeringActor(event); + if (!actorUserId) { + this.log.warn("Blocked slack steering for unauthorized actor", { + event: "scheduler.slack_steer_unauthorized", + automation_id: automation.id, + session_id: steerable.session_id, + slack_actor_id: event.actorUserId, + }); + continue; + } + if (await this.steerSession(steerable, automation, event, actorUserId)) { + steered++; + continue; + } } // No steerable session (outside the window, no session yet, or a rare // enqueue error) → fall through. Like the @mention path's stale-session @@ -1004,6 +1117,14 @@ export class Scheduler { case "blocked": skipped++; break; + case "unauthorized": + this.log.warn("Skipped event automation after execution authorization denial", { + event: "scheduler.authorization_denied", + automation_id: automation.id, + source: event.source, + }); + skipped++; + break; } } @@ -1027,15 +1148,31 @@ export class Scheduler { // ─── Manual trigger ────────────────────────────────────────────────────── - async trigger(automationId: string): Promise { + /** Manually trigger an automation under the requesting user's authority. */ + async trigger( + automationId: string, + requesterUserId: string, + requesterEnrichment?: GitHubEnrichment + ): Promise { const store = new AutomationStore(this.db); const automation = await store.getById(automationId); if (!automation) { throw new Error("Automation not found"); } - const result = await this.startInvocation(store, { automation, source: "manual" }); + const result = await this.startInvocation(store, { + automation, + source: "manual", + executionPrincipal: { + platformUserId: requesterUserId, + participantUserId: requesterUserId, + scmEnrichment: requesterEnrichment, + }, + }); + if (result.outcome === "unauthorized") { + throw new AutomationExecutionUnauthorizedError(); + } if (result.outcome !== "started") { // Manual overlap (pre-check or lost race) records nothing. throw new AutomationTriggerBlockedError(); @@ -1303,28 +1440,9 @@ export class Scheduler { automation: AutomationRow, run: AutomationRunRow, providerAuth: SessionModelProviderAuthInput[], - sessionId: string + sessionId: string, + executionPrincipal: ExecutionPrincipal ): Promise { - // Resolve the canonical user_id for the session index. - // Automations created through the web UI populate user_id at creation time - // (handleCreateAutomation resolves it for both GitHub and Google users), so this - // lookup is skipped for them. The fallback below only covers legacy rows with - // user_id = NULL: those predate Google login and store the GitHub numeric user ID - // in created_by (from the canonical browser principal), so a GitHub-only identity lookup - // recovers the canonical user. It becomes dead code once legacy rows are backfilled. - let userId = automation.user_id; - if (!userId && automation.created_by && automation.created_by !== "anonymous") { - try { - const userStore = new UserStore(this.db); - const identity = await userStore.getIdentity("github", automation.created_by); - if (identity) { - userId = identity.userId; - } - } catch { - // Best-effort — proceed without user_id - } - } - const ctx: RequestContext = { trace_id: `automation:${automation.id}`, request_id: run.id, @@ -1361,7 +1479,7 @@ export class Scheduler { environmentId: target.environmentId, }, { mode: "all" }, - userId + executionPrincipal.platformUserId ); const sessionInput: SessionInitInput = { @@ -1370,10 +1488,15 @@ export class Scheduler { title: `[Auto] ${automation.name}`, model: automation.model, reasoningEffort: automation.reasoning_effort, - participantUserId: automation.created_by, - platformUserId: userId, - scmTokenEncrypted: null, - scmRefreshTokenEncrypted: null, + participantUserId: executionPrincipal.participantUserId, + platformUserId: executionPrincipal.platformUserId, + scmUserId: executionPrincipal.scmEnrichment?.scmUserId, + scmLogin: executionPrincipal.scmEnrichment?.scmLogin, + scmName: executionPrincipal.scmEnrichment?.displayName, + scmEmail: executionPrincipal.scmEnrichment?.email, + scmTokenEncrypted: executionPrincipal.scmEnrichment?.accessTokenEncrypted ?? null, + scmRefreshTokenEncrypted: executionPrincipal.scmEnrichment?.refreshTokenEncrypted ?? null, + scmTokenExpiresAt: executionPrincipal.scmEnrichment?.tokenExpiresAt, codeServerEnabled, vncEnabled, sandboxSettings, @@ -1392,6 +1515,7 @@ export class Scheduler { sessionId: string, automation: AutomationRow, runId: string, + executionPrincipal: ExecutionPrincipal, instructionsOverride?: string ): Promise { const callbackContext: AutomationCallbackContext = { @@ -1403,8 +1527,8 @@ export class Scheduler { await this.enqueueSessionPrompt(sessionId, { content: instructionsOverride ?? automation.instructions, - authorId: automation.created_by, - canonicalUserId: automation.user_id, + authorId: executionPrincipal.participantUserId, + canonicalUserId: executionPrincipal.platformUserId, source: "automation", callbackContext, }); @@ -1423,7 +1547,8 @@ export class Scheduler { private async steerSession( run: AutomationRunRow, automation: AutomationRow, - event: SlackAutomationEvent + event: SlackAutomationEvent, + actorUserId: string ): Promise { const sessionId = run.session_id!; const callbackContext: SlackCallbackContext = { @@ -1442,11 +1567,10 @@ export class Scheduler { }; try { - const identity = await new UserStore(this.db).getIdentity("slack", event.actorUserId); await this.enqueueSessionPrompt(sessionId, { content: event.text, authorId: `slack:${event.actorUserId}`, - canonicalUserId: identity?.userId, + canonicalUserId: actorUserId, source: "slack", callbackContext, }); diff --git a/packages/control-plane/test/integration/automation-authorization.test.ts b/packages/control-plane/test/integration/automation-authorization.test.ts new file mode 100644 index 000000000..ff7c2fac4 --- /dev/null +++ b/packages/control-plane/test/integration/automation-authorization.test.ts @@ -0,0 +1,183 @@ +import { env } from "cloudflare:test"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { AutomationStore, type AutomationRow } from "../../src/db/automation-store"; +import { UserStore } from "../../src/db/user-store"; +import { cleanD1Tables } from "./cleanup"; +import { serviceFetch, sqlDatabase } from "./helpers"; + +const BROWSER_USER_ID = "11111111111111111111111111111111"; + +function automation(id: string, userId: string): AutomationRow { + return { + id, + name: id, + instructions: "Run tests", + trigger_type: "schedule", + schedule_cron: "0 9 * * *", + schedule_tz: "UTC", + event_type: null, + trigger_config: null, + trigger_auth_data: null, + model: "anthropic/claude-sonnet-4-6", + reasoning_effort: null, + enabled: 1, + next_run_at: null, + consecutive_failures: 0, + created_by: userId, + user_id: userId, + created_at: 1, + updated_at: 1, + deleted_at: null, + }; +} + +async function seedBrowser(role: "owner" | "member" = "member"): Promise { + const response = await serviceFetch("https://cp.test/me/authorization", { + initialUserRole: role, + }); + expect(response.status).toBe(200); +} + +describe("automation router authorization", () => { + beforeEach(cleanD1Tables); + afterEach(cleanD1Tables); + + it("returns 404 for a missing automation before authority disclosure", async () => { + await seedBrowser("member"); + + const response = await serviceFetch("https://cp.test/automations/missing", { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ name: "Updated" }), + initialUserRole: "member", + }); + + expect(response.status).toBe(404); + await expect(response.json()).resolves.toEqual({ error: "Automation not found" }); + + const store = new AutomationStore(env.DB); + await store.create(automation("deleted-automation", BROWSER_USER_ID)); + await store.softDelete("deleted-automation"); + const deleted = await serviceFetch("https://cp.test/automations/deleted-automation", { + method: "DELETE", + initialUserRole: "member", + }); + expect(deleted.status).toBe(404); + await expect(deleted.json()).resolves.toEqual({ error: "Automation not found" }); + }); + + it("allows own management and denies another user's automation", async () => { + await seedBrowser("member"); + const other = await new UserStore(sqlDatabase(env.DB)).createUser({ displayName: "Other" }); + const store = new AutomationStore(env.DB); + await store.create(automation("own-automation", BROWSER_USER_ID)); + await store.create(automation("other-automation", other.id)); + + const own = await serviceFetch("https://cp.test/automations/own-automation", { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ name: "Updated own" }), + initialUserRole: "member", + }); + const denied = await serviceFetch("https://cp.test/automations/other-automation", { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ name: "Updated other" }), + initialUserRole: "member", + }); + + expect(own.status).toBe(200); + expect(denied.status).toBe(403); + await expect(denied.json()).resolves.toMatchObject({ + code: "permission_required", + permission: "automations.manage.own", + }); + expect((await store.getById("other-automation"))?.name).toBe("other-automation"); + }); + + it("repairs a legacy GitHub owner before own-scope route admission", async () => { + await seedBrowser("member"); + const store = new AutomationStore(env.DB); + await store.create({ + ...automation("legacy-own-automation", BROWSER_USER_ID), + created_by: "583231", + user_id: null, + }); + + const response = await serviceFetch("https://cp.test/automations/legacy-own-automation", { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ name: "Updated legacy own" }), + initialUserRole: "member", + }); + + expect(response.status).toBe(200); + expect((await store.getById("legacy-own-automation"))?.user_id).toBe(BROWSER_USER_ID); + }); + + it("denies users without manage permission", async () => { + await seedBrowser("member"); + await new AutomationStore(env.DB).create(automation("viewer-target", BROWSER_USER_ID)); + await env.DB.prepare( + "UPDATE user_role_assignments SET role_id = 'role_builtin_viewer' WHERE user_id = ?" + ) + .bind(BROWSER_USER_ID) + .run(); + + const response = await serviceFetch("https://cp.test/automations/viewer-target", { + method: "DELETE", + }); + + expect(response.status).toBe(403); + await expect(response.json()).resolves.toMatchObject({ + code: "permission_required", + permission: "automations.manage.own", + }); + }); + + it("retains body-derived target permission checks after route admission", async () => { + await seedBrowser("member"); + await new AutomationStore(env.DB).create(automation("target-permission", BROWSER_USER_ID)); + await env.DB.batch([ + env.DB.prepare( + `INSERT INTO roles + (id, key, name, normalized_name, description, is_system) + VALUES ('role_manage_only', NULL, 'Manage Only', 'manage only', NULL, 0)` + ), + env.DB.prepare( + `INSERT INTO role_permissions (role_id, permission_id) + VALUES ('role_manage_only', 'automations.manage.own')` + ), + env.DB.prepare( + "UPDATE user_role_assignments SET role_id = 'role_manage_only' WHERE user_id = ?" + ).bind(BROWSER_USER_ID), + ]); + + const response = await serviceFetch("https://cp.test/automations/target-permission", { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ repositories: [{ repoOwner: "acme", repoName: "api" }] }), + }); + + expect(response.status).toBe(403); + await expect(response.json()).resolves.toMatchObject({ + code: "permission_required", + permission: "repositories.use", + }); + }); + + it("denies bot services before handler dispatch", async () => { + await seedBrowser("owner"); + await new AutomationStore(env.DB).create(automation("service-target", BROWSER_USER_ID)); + + const response = await serviceFetch("https://cp.test/automations/service-target", { + method: "DELETE", + service: "slack-bot", + actor: "slack:U-AUTOMATION", + }); + + expect(response.status).toBe(403); + await expect(response.json()).resolves.toMatchObject({ code: "service_capability_required" }); + expect(await new AutomationStore(env.DB).getById("service-target")).not.toBeNull(); + }); +}); diff --git a/packages/control-plane/test/integration/automation-invocations.test.ts b/packages/control-plane/test/integration/automation-invocations.test.ts index 5e0a1bbe5..08c4b9811 100644 --- a/packages/control-plane/test/integration/automation-invocations.test.ts +++ b/packages/control-plane/test/integration/automation-invocations.test.ts @@ -8,6 +8,7 @@ import { type AutomationRow, type AutomationRunRow, } from "../../src/db/automation-store"; +import { isAutomationExecutionAuthorized } from "../../src/automation/authorization-guard"; import { cleanD1Tables } from "./cleanup"; // ─── Fixtures ──────────────────────────────────────────────────────────────── @@ -27,7 +28,7 @@ function makeAutomation(overrides?: Partial): AutomationRow { next_run_at: now + 86_400_000, consecutive_failures: 0, created_by: "user-1", - user_id: null, + user_id: "user-1", created_at: now, updated_at: now, deleted_at: null, @@ -90,7 +91,73 @@ async function countRows(table: string, where = "1=1"): Promise { } describe("automation invocations (D1 integration)", () => { - beforeEach(cleanD1Tables); + beforeEach(async () => { + await cleanD1Tables(); + await env.DB.batch([ + env.DB.prepare( + `INSERT INTO users + (id, display_name, email, email_verified, avatar_url, created_at, updated_at) + VALUES ('user-1', 'Execution Owner', NULL, 0, NULL, 1, 1)` + ), + env.DB.prepare( + "UPDATE user_role_assignments SET role_id = 'role_builtin_owner' WHERE user_id = 'user-1'" + ), + ]); + }); + + it("checks owner and target-use permissions for unattended execution", async () => { + const ownerId = "11111111111111111111111111111111"; + const roleId = "role_execution_test"; + const store = new AutomationStore(env.DB); + const automation = makeAutomation({ user_id: ownerId, created_by: ownerId }); + + await env.DB.batch([ + env.DB.prepare( + `INSERT INTO users + (id, display_name, email, email_verified, avatar_url, created_at, updated_at) + VALUES (?, 'Execution Owner', NULL, 0, NULL, 1, 1)` + ).bind(ownerId), + env.DB.prepare( + `INSERT INTO roles + (id, key, name, normalized_name, description, is_system) + VALUES (?, NULL, 'Execution Test', 'execution test', NULL, 0)` + ).bind(roleId), + env.DB.prepare("UPDATE user_role_assignments SET role_id = ? WHERE user_id = ?").bind( + roleId, + ownerId + ), + ]); + await store.create(automation); + await Promise.all( + [ + ...store.bindRepositoryInserts( + automation.id, + [{ repo_owner: "acme", repo_name: "app", repo_id: 1, base_branch: "main" }], + Date.now() + ), + ...store.bindEnvironmentInserts(automation.id, ["env_1"], Date.now()), + ].map((statement) => statement.run()) + ); + + const authorized = () => + isAutomationExecutionAuthorized(env.DB, { + automationId: automation.id, + requiresRepositoryUse: true, + requiresEnvironmentUse: true, + }); + const grant = (permission: string) => + env.DB.prepare("INSERT INTO role_permissions (role_id, permission_id) VALUES (?, ?)") + .bind(roleId, permission) + .run(); + + await expect(authorized()).resolves.toBe(false); + await grant("sessions.create"); + await expect(authorized()).resolves.toBe(false); + await grant("repositories.use"); + await expect(authorized()).resolves.toBe(false); + await grant("environments.use"); + await expect(authorized()).resolves.toBe(true); + }); // ─── Derived status ──────────────────────────────────────────────────────── diff --git a/packages/control-plane/test/integration/scheduler-events.test.ts b/packages/control-plane/test/integration/scheduler-events.test.ts index ab406235b..4c48e04ea 100644 --- a/packages/control-plane/test/integration/scheduler-events.test.ts +++ b/packages/control-plane/test/integration/scheduler-events.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, beforeEach } from "vitest"; import { env } from "cloudflare:test"; -import { sqlDatabase } from "./helpers"; +import { seedActiveUser, sqlDatabase } from "./helpers"; import { AutomationStore, type AutomationRow } from "../../src/db/automation-store"; import type { SentryAutomationEvent, WebhookAutomationEvent } from "@open-inspect/shared/triggers"; import { cleanD1Tables } from "./cleanup"; @@ -23,7 +23,7 @@ function makeAutomation(overrides?: Partial): AutomationRow { next_run_at: now + 86400000, consecutive_failures: 0, created_by: "user-1", - user_id: null, + user_id: "user-1", created_at: now, updated_at: now, deleted_at: null, @@ -74,7 +74,10 @@ function makeWebhookEvent( } describe("Scheduler event handling (integration)", () => { - beforeEach(cleanD1Tables); + beforeEach(async () => { + await cleanD1Tables(); + await seedActiveUser("user-1"); + }); // ─── Sentry event matching ─────────────────────────────────────────────── diff --git a/packages/control-plane/test/integration/scheduler-slack-events.test.ts b/packages/control-plane/test/integration/scheduler-slack-events.test.ts index 08632bfdc..996ccc38a 100644 --- a/packages/control-plane/test/integration/scheduler-slack-events.test.ts +++ b/packages/control-plane/test/integration/scheduler-slack-events.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, beforeEach } from "vitest"; import { env } from "cloudflare:test"; -import { sqlDatabase } from "./helpers"; +import { seedActiveUser, sqlDatabase } from "./helpers"; import { AutomationStore, type AutomationRow } from "../../src/db/automation-store"; import { SlackChannelStore } from "../../src/db/slack-channel-store"; import type { SlackAutomationEvent } from "@open-inspect/shared/triggers"; @@ -24,7 +24,7 @@ function makeAutomation(overrides?: Partial): AutomationRow { next_run_at: null, consecutive_failures: 0, created_by: "user-1", - user_id: null, + user_id: "user-1", created_at: now, updated_at: now, deleted_at: null, @@ -80,7 +80,17 @@ async function fetchInvocations(store: AutomationStore, automationId: string) { } describe("Scheduler slack event handling (integration)", () => { - beforeEach(cleanD1Tables); + beforeEach(async () => { + await cleanD1Tables(); + await seedActiveUser("user-1"); + await seedActiveUser("slack-actor-1"); + await env.DB.prepare( + `INSERT INTO user_identities + (id, user_id, provider, provider_user_id, provider_issuer, created_at, updated_at) + VALUES ('slack-identity-1', 'slack-actor-1', 'slack', 'U1', + 'https://slack.com', 1, 1)` + ).run(); + }); it("triggers a matching slack automation and records thread coordinates", async () => { const store = new AutomationStore(env.DB); @@ -174,7 +184,7 @@ describe("Scheduler slack event handling (integration)", () => { expect(JSON.parse(invocationRow!.trigger_metadata!).channel).toBe("C1"); }); - it("steers the running session on a follow-up reply instead of dropping it", async () => { + it("steers the running session when the Slack actor may collaborate", async () => { const store = new AutomationStore(env.DB); const id = await seedSlackAutomation(store); @@ -203,6 +213,118 @@ describe("Scheduler slack event handling (integration)", () => { ).toBeUndefined(); }); + it.each([ + [ + "suspended", + async () => { + await env.DB.prepare("UPDATE users SET suspended_at = 1 WHERE id = ?") + .bind("slack-actor-1") + .run(); + }, + ], + [ + "revoked", + async () => { + await env.DB.prepare("UPDATE user_role_assignments SET role_id = ? WHERE user_id = ?") + .bind("role_builtin_viewer", "slack-actor-1") + .run(); + }, + ], + [ + "missing collaboration permission", + async () => { + await env.DB.batch([ + env.DB.prepare( + `INSERT INTO roles + (id, key, name, normalized_name, description, is_system) + VALUES ('role_no_collaboration', NULL, 'No Collaboration', 'no collaboration', + NULL, 0)` + ), + env.DB.prepare( + `INSERT INTO role_permissions (role_id, permission_id) + VALUES ('role_no_collaboration', 'sessions.create')` + ), + env.DB.prepare( + "UPDATE user_role_assignments SET role_id = 'role_no_collaboration' WHERE user_id = 'slack-actor-1'" + ), + ]); + }, + ], + ])( + "does not steer when the Slack actor's collaboration authority is %s", + async (authorityState, revoke) => { + const store = new AutomationStore(env.DB); + const id = await seedSlackAutomation(store); + const concurrencyKey = `slack:C1:thread-${authorityState}`; + expect( + await sendEvent( + makeSlackEvent({ + text: "deploy the api", + concurrencyKey, + triggerKey: `slack:msg:C1:root-${authorityState}`, + }) + ) + ).toMatchObject({ triggered: 1 }); + + await revoke(); + + expect( + await sendEvent( + makeSlackEvent({ + text: "also update the changelog", + concurrencyKey, + triggerKey: `slack:msg:C1:reply-${authorityState}`, + }) + ) + ).toEqual({ triggered: 0, skipped: 0, steered: 0 }); + expect(await fetchRuns(id)).toHaveLength(1); + expect(await fetchInvocations(store, id)).toHaveLength(1); + } + ); + + it("does not impose automation-launch grants on the Slack actor while steering", async () => { + const store = new AutomationStore(env.DB); + const id = await seedSlackAutomation(store); + const concurrencyKey = "slack:C1:thread-actor-only"; + expect( + await sendEvent( + makeSlackEvent({ + text: "deploy the api", + concurrencyKey, + triggerKey: "slack:msg:C1:root-actor-only", + }) + ) + ).toMatchObject({ triggered: 1 }); + + await env.DB.batch([ + env.DB.prepare( + `INSERT INTO roles + (id, key, name, normalized_name, description, is_system) + VALUES ('role_collaboration_only', NULL, 'Collaboration Only', + 'collaboration only', NULL, 0)` + ), + env.DB.prepare( + `INSERT INTO role_permissions (role_id, permission_id) + VALUES ('role_collaboration_only', 'sessions.collaborate')` + ), + env.DB.prepare( + `UPDATE user_role_assignments + SET role_id = 'role_collaboration_only' WHERE user_id = 'slack-actor-1'` + ), + ]); + + expect( + await sendEvent( + makeSlackEvent({ + text: "also update the changelog", + concurrencyKey, + triggerKey: "slack:msg:C1:reply-actor-only", + }) + ) + ).toEqual({ triggered: 0, skipped: 0, steered: 1 }); + expect(await fetchRuns(id)).toHaveLength(1); + }); + it("continues the same session on a reply after the run has completed", async () => { const store = new AutomationStore(env.DB); const id = await seedSlackAutomation(store); diff --git a/packages/control-plane/test/integration/scheduler.test.ts b/packages/control-plane/test/integration/scheduler.test.ts index 702430ea7..60d45315f 100644 --- a/packages/control-plane/test/integration/scheduler.test.ts +++ b/packages/control-plane/test/integration/scheduler.test.ts @@ -1,11 +1,15 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; import { env } from "cloudflare:test"; -import { sqlDatabase } from "./helpers"; +import { seedActiveUser, sqlDatabase } from "./helpers"; import { AutomationStore, type AutomationRow } from "../../src/db/automation-store"; import type { AutomationRunStatus } from "@open-inspect/shared/types/automations"; import { cleanD1Tables } from "./cleanup"; import { makeRunRow, seedRun, fetchRuns } from "./run-helpers"; -import { Scheduler, resolveAutomationProviderAuth } from "../../src/scheduler/scheduler"; +import { + AutomationExecutionUnauthorizedError, + Scheduler, + resolveAutomationProviderAuth, +} from "../../src/scheduler/scheduler"; import { AutomationModelProviderAuthStore } from "../../src/db/automation-model-provider-auth"; import { ModelProviderAccountStore } from "../../src/db/model-provider-accounts"; import { ProviderDefaultStore } from "../../src/db/provider-account-defaults"; @@ -30,7 +34,7 @@ function makeAutomation(overrides?: Partial): AutomationRow { next_run_at: now + 86400000, consecutive_failures: 0, created_by: "user-1", - user_id: null, + user_id: "user-1", created_at: now, updated_at: now, deleted_at: null, @@ -44,6 +48,7 @@ function makeAutomation(overrides?: Partial): AutomationRow { describe("Scheduler (integration)", () => { beforeEach(async () => { await cleanD1Tables(); + await seedActiveUser("user-1"); await env.DB.exec( "DELETE FROM model_provider_account_defaults; DELETE FROM model_provider_accounts;" ); @@ -423,6 +428,41 @@ describe("Scheduler (integration)", () => { expect(automation!.next_run_at!).toBeGreaterThan(now); }); + it("records and pauses an authorization-denied schedule so it is not repeatedly overdue", async () => { + const store = new AutomationStore(env.DB); + const scheduledAt = Date.now() - 60_000; + await store.create( + makeAutomation({ id: "auto-denied-schedule", next_run_at: scheduledAt, enabled: 1 }) + ); + await env.DB.prepare( + "UPDATE user_role_assignments SET role_id = 'role_builtin_viewer' WHERE user_id = ?" + ) + .bind("user-1") + .run(); + + expect(await createScheduler().tick()).toMatchObject({ skipped: 1, failed: 0 }); + const denied = await store.getById("auto-denied-schedule"); + expect(denied).toMatchObject({ enabled: 0, next_run_at: null }); + const firstInvocations = await store.listInvocations("auto-denied-schedule", { + limit: 20, + offset: 0, + }); + expect(firstInvocations.invocations).toEqual([ + expect.objectContaining({ + status: "skipped", + skipReason: "execution_authorization_denied", + scheduledAt, + }), + ]); + + expect(await createScheduler().tick()).toEqual({ processed: 0, skipped: 0, failed: 0 }); + const secondInvocations = await store.listInvocations("auto-denied-schedule", { + limit: 20, + offset: 0, + }); + expect(secondInvocations.invocations).toHaveLength(1); + }); + it("auto-pauses after recovery sweep detects 3rd consecutive failure", async () => { const store = new AutomationStore(env.DB); const now = Date.now(); @@ -527,8 +567,8 @@ describe("Scheduler (integration)", () => { ]; const [triggerA, triggerB, tick] = await Promise.allSettled([ - schedulers[0]!.trigger("auto-concurrent-admission"), - schedulers[1]!.trigger("auto-concurrent-admission"), + schedulers[0]!.trigger("auto-concurrent-admission", "user-1"), + schedulers[1]!.trigger("auto-concurrent-admission", "user-1"), schedulers[2]!.tick(), ]); @@ -629,7 +669,7 @@ describe("Scheduler (integration)", () => { }); it("rejects when automation is not found", async () => { - await expect(createScheduler().trigger("nonexistent")).rejects.toThrow( + await expect(createScheduler().trigger("nonexistent", "user-1")).rejects.toThrow( "Automation not found" ); }); @@ -648,14 +688,85 @@ describe("Scheduler (integration)", () => { }) ); - await expect(createScheduler().trigger("auto-trig1")).rejects.toThrow( + await expect(createScheduler().trigger("auto-trig1", "user-1")).rejects.toThrow( "An active run already exists" ); }); - it("creates a run record when triggered", async () => { + it("requires the requester to have session execution authority", async () => { + const requesterId = "manual-trigger-requester"; + await seedActiveUser(requesterId); + const roleId = "role_manual_trigger_only"; + await env.DB.batch([ + env.DB.prepare( + `INSERT INTO roles (id, key, name, normalized_name, description, is_system) + VALUES (?, NULL, 'Manual Trigger Only', 'manual trigger only', NULL, 0)` + ).bind(roleId), + env.DB.prepare( + `INSERT INTO role_permissions (role_id, permission_id) + VALUES (?, 'automations.trigger.any')` + ).bind(roleId), + env.DB.prepare(`UPDATE user_role_assignments SET role_id = ? WHERE user_id = ?`).bind( + roleId, + requesterId + ), + ]); const store = new AutomationStore(env.DB); - await store.create(makeAutomation({ id: "auto-trig2" })); + await store.create(makeAutomation({ id: "auto-trigger-only" })); + + await expect( + createScheduler().trigger("auto-trigger-only", requesterId) + ).rejects.toBeInstanceOf(AutomationExecutionUnauthorizedError); + expect(await fetchRuns("auto-trigger-only")).toEqual([]); + }); + + it("creates manual-trigger sessions as the requester", async () => { + const requesterId = "manual-trigger-requester"; + await seedActiveUser(requesterId); + const store = new AutomationStore(env.DB); + await store.create(makeAutomation({ id: "auto-requester-principal" })); + const promptBodies: Array> = []; + const sessionFetch = vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const request = input instanceof Request ? input : new Request(input, init); + const path = new URL(request.url).pathname; + if (path === "/internal/init") return Response.json({ status: "ok" }); + if (path === "/internal/prompt") { + promptBodies.push(await request.json>()); + return Response.json({ messageId: "msg-requester", status: "queued" }); + } + return new Response("Not Found", { status: 404 }); + }); + const schedulerEnv = { + ...(env as Env), + SESSION: { + idFromName: vi.fn((name: string) => name), + get: vi.fn(() => ({ fetch: sessionFetch })), + } as unknown as DurableObjectNamespace, + }; + + await createScheduler(schedulerEnv).trigger("auto-requester-principal", requesterId); + + expect( + await env.DB.prepare( + `SELECT user_id FROM sessions WHERE automation_id = 'auto-requester-principal'` + ).first() + ).toEqual({ user_id: requesterId }); + expect(promptBodies).toContainEqual( + expect.objectContaining({ authorId: requesterId, canonicalUserId: requesterId }) + ); + }); + + it("repairs a legacy owner before creating a triggered run", async () => { + const store = new AutomationStore(env.DB); + await env.DB.prepare( + `INSERT INTO user_identities + (id, user_id, provider, provider_user_id, provider_issuer, created_at, updated_at) + VALUES ('legacy-identity', 'user-1', 'github', 'legacy-github-id', + 'https://github.com', 1, 1)` + ).run(); + await store.create( + makeAutomation({ id: "auto-trig2", created_by: "legacy-github-id", user_id: null }) + ); const sessionFetch = vi.fn(async (input: RequestInfo | URL) => { const path = new URL( @@ -675,7 +786,7 @@ describe("Scheduler (integration)", () => { } as unknown as DurableObjectNamespace, }; - const result = await createScheduler(schedulerEnv).trigger("auto-trig2"); + const result = await createScheduler(schedulerEnv).trigger("auto-trig2", "user-1"); expect(result).toEqual({ invocationId: expect.any(String), runs: [expect.objectContaining({ status: "running" })], @@ -684,13 +795,14 @@ describe("Scheduler (integration)", () => { const runs = await fetchRuns("auto-trig2"); expect(runs).toHaveLength(1); expect(runs[0]!.invocation_id).not.toBeNull(); + expect((await store.getById("auto-trig2"))!.user_id).toBe("user-1"); }); }); // ─── Invocation finalization (D2) ───────────────────────────────────────── describe("invocation finalization", () => { - /** Seed an invocation with N children in the given statuses via the real guarded insert. */ + /** Seed an invocation with N children in the given statuses via the real conditional insert. */ async function seedInvocation( store: AutomationStore, automationId: string, diff --git a/packages/control-plane/test/integration/webhooks-slack.test.ts b/packages/control-plane/test/integration/webhooks-slack.test.ts index 0036b4be8..9bde8b843 100644 --- a/packages/control-plane/test/integration/webhooks-slack.test.ts +++ b/packages/control-plane/test/integration/webhooks-slack.test.ts @@ -7,6 +7,8 @@ import { serviceFetch, sqlDatabase } from "./helpers"; // ─── Helpers ────────────────────────────────────────────────────────────────── +const AUTOMATION_OWNER_ID = "11111111111111111111111111111111"; + function makeSlackEventBody(overrides?: Record): Record { const ts = `${Date.now()}.${Math.floor(Math.random() * 1e6)}`; return { @@ -38,8 +40,8 @@ function makeSlackAutomation(overrides?: Partial): AutomationRow enabled: 1, next_run_at: null, consecutive_failures: 0, - created_by: "user-1", - user_id: null, + created_by: AUTOMATION_OWNER_ID, + user_id: AUTOMATION_OWNER_ID, created_at: now, updated_at: now, deleted_at: null, @@ -56,6 +58,13 @@ function makeSlackAutomation(overrides?: Partial): AutomationRow } async function seedSlackAutomation(): Promise { + await env.DB.prepare( + `INSERT INTO users + (id, display_name, email, email_verified, avatar_url, created_at, updated_at) + VALUES (?, 'Slack Owner', NULL, 0, NULL, ?, ?)` + ) + .bind(AUTOMATION_OWNER_ID, Date.now(), Date.now()) + .run(); const store = new AutomationStore(env.DB); const automation = makeSlackAutomation(); await store.create(automation); diff --git a/packages/control-plane/test/integration/webhooks.test.ts b/packages/control-plane/test/integration/webhooks.test.ts index dfb9427e0..288f0da4f 100644 --- a/packages/control-plane/test/integration/webhooks.test.ts +++ b/packages/control-plane/test/integration/webhooks.test.ts @@ -5,6 +5,7 @@ import { hashApiKey } from "../../src/auth/webhook-key"; import { encryptToken } from "../../src/auth/crypto"; import { cleanD1Tables } from "./cleanup"; import { fetchRuns } from "./run-helpers"; +import { seedActiveUser } from "./helpers"; // ─── Helpers ────────────────────────────────────────────────────────────────── @@ -36,7 +37,7 @@ function makeAutomation(overrides: Partial = {}): AutomationRow { next_run_at: null, consecutive_failures: 0, created_by: "test-user", - user_id: null, + user_id: "test-user", created_at: Date.now(), updated_at: Date.now(), deleted_at: null, @@ -134,7 +135,10 @@ const sentryMetricWarningPayload = { // ─── Sentry webhook tests (per-automation) ─────────────────────────────────── describe("POST /webhooks/sentry/:id", () => { - beforeEach(cleanD1Tables); + beforeEach(async () => { + await cleanD1Tables(); + await seedActiveUser("test-user"); + }); it("creates an automation run for a current Sentry issue.created webhook", async () => { const automation = await createSentryAutomation(); @@ -382,7 +386,10 @@ describe("POST /webhooks/sentry/:id", () => { // ─── Automation webhook tests ───────────────────────────────────────────────── describe("POST /webhooks/automation/:id", () => { - beforeEach(cleanD1Tables); + beforeEach(async () => { + await cleanD1Tables(); + await seedActiveUser("test-user"); + }); const TEST_API_KEY = "test-webhook-api-key-abc123"; diff --git a/packages/shared/src/types/automations.test.ts b/packages/shared/src/types/automations.test.ts index 101de3180..c752c742f 100644 --- a/packages/shared/src/types/automations.test.ts +++ b/packages/shared/src/types/automations.test.ts @@ -6,6 +6,7 @@ import { } from "./automations"; const ACCOUNT_ID = "0123456789abcdef0123456789abcdef"; +const USER_ID = "11111111111111111111111111111111"; const automation = { id: "auto-1", @@ -20,6 +21,7 @@ const automation = { nextRunAt: 123, consecutiveFailures: 0, createdBy: "user-1", + userId: USER_ID, createdAt: 1, updatedAt: 2, deletedAt: null, @@ -67,6 +69,17 @@ describe("listAutomationsResponseSchema", () => { ).toBe(false); }); + it("requires a canonical owner ID when ownership is present", () => { + const response = (userId: string | null) => ({ + automations: [{ ...automation, userId }], + hasMore: false as const, + nextCursor: null, + }); + + expect(listAutomationsResponseSchema.safeParse(response(null)).success).toBe(true); + expect(listAutomationsResponseSchema.safeParse(response("user-1")).success).toBe(false); + }); + it("validates recent execution summaries", () => { const result = listAutomationsResponseSchema.parse({ automations: [ diff --git a/packages/shared/src/types/automations.ts b/packages/shared/src/types/automations.ts index 36cf8ff29..c14dbcc31 100644 --- a/packages/shared/src/types/automations.ts +++ b/packages/shared/src/types/automations.ts @@ -8,6 +8,7 @@ import { import type { RepositoryInput, RepositoryRef } from "./repositories"; import { modelProviderSelectionsSchema } from "./provider-accounts"; import { isEnvironmentId } from "./environments"; +import { isCanonicalUserId } from "../user-id"; export type AutomationRunStatus = "starting" | "running" | "completed" | "failed" | "skipped"; @@ -80,6 +81,7 @@ const automationSchema = z.object({ nextRunAt: z.number().nullable(), consecutiveFailures: z.number(), createdBy: z.string(), + userId: z.string().refine(isCanonicalUserId, "Invalid canonical user ID").nullable(), createdAt: z.number(), updatedAt: z.number(), deletedAt: z.number().nullable(), diff --git a/packages/web/src/components/automations/automations-list.test.tsx b/packages/web/src/components/automations/automations-list.test.tsx index b28960020..5c20c2bbb 100644 --- a/packages/web/src/components/automations/automations-list.test.tsx +++ b/packages/web/src/components/automations/automations-list.test.tsx @@ -41,6 +41,7 @@ function makeAutomation(overrides: Partial = {}): Automation nextRunAt: null, consecutiveFailures: 0, createdBy: "user-1", + userId: "11111111111111111111111111111111", createdAt: Date.now(), updatedAt: Date.now(), deletedAt: null, diff --git a/packages/web/src/hooks/use-automations.test.tsx b/packages/web/src/hooks/use-automations.test.tsx index 5d1068117..c944029e6 100644 --- a/packages/web/src/hooks/use-automations.test.tsx +++ b/packages/web/src/hooks/use-automations.test.tsx @@ -25,6 +25,7 @@ function automation(id: string, name: string): AutomationListItem { nextRunAt: null, consecutiveFailures: 0, createdBy: "user-1", + userId: "11111111111111111111111111111111", createdAt: 1, updatedAt: 1, deletedAt: null,