Skip to content

Commit 49c951e

Browse files
d-cscarderne
authored andcommitted
fix(webapp): run/batch/trigger tenant scoping & batch-resume authorization (#43)
1 parent 7fa8f95 commit 49c951e

15 files changed

Lines changed: 620 additions & 10 deletions
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
area: webapp
3+
type: fix
4+
---
5+
6+
Scope run, batch, and trigger lookups to the caller's tenant

apps/webapp/app/routes/resources.batches.$batchId.check-completion.ts

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,13 @@ import type { ActionFunction } from "@remix-run/node";
33
import { json } from "@remix-run/node";
44
import { assertExhaustive } from "@trigger.dev/core/utils";
55
import { z } from "zod";
6+
import { prisma } from "~/db.server";
67
import { redirectWithErrorMessage, redirectWithSuccessMessage } from "~/models/message.server";
78
import { logger } from "~/services/logger.server";
9+
import { requireUserId } from "~/services/session.server";
10+
import { sanitizeRedirectPath } from "~/utils";
11+
import { runStore } from "~/v3/runStore.server";
12+
import { findBatchRunIdForUser } from "~/v3/services/batchRunAccess.server";
813
import { ResumeBatchRunService } from "~/v3/services/resumeBatchRun.server";
914

1015
export const checkCompletionSchema = z.object({
@@ -16,6 +21,8 @@ const ParamSchema = z.object({
1621
});
1722

1823
export const action: ActionFunction = async ({ request, params }) => {
24+
// Require a logged-in user; org membership is checked below before resuming.
25+
const userId = await requireUserId(request);
1926
const { batchId } = ParamSchema.parse(params);
2027

2128
const formData = await request.formData();
@@ -25,9 +32,22 @@ export const action: ActionFunction = async ({ request, params }) => {
2532
return json(submission.reply());
2633
}
2734

35+
// Keep the post-action redirect same-origin.
36+
const safeRedirectUrl = sanitizeRedirectPath(submission.value.redirectUrl);
37+
38+
// Only act on a batch in an org the caller belongs to. Accepts either the
39+
// friendlyId or the internal id; both forms stay org-scoped.
40+
const ownedBatchRunId = await findBatchRunIdForUser(prisma, runStore, batchId, userId);
41+
42+
if (!ownedBatchRunId) {
43+
return redirectWithErrorMessage(safeRedirectUrl, request, "Batch not found");
44+
}
45+
2846
try {
2947
const resumeBatchRunService = new ResumeBatchRunService();
30-
const resumeResult = await resumeBatchRunService.call(batchId);
48+
// Resume by the resolved internal id: the service looks up strictly by
49+
// `{ id }`, so passing a friendlyId param would resolve to nothing.
50+
const resumeResult = await resumeBatchRunService.call(ownedBatchRunId);
3151

3252
let message: string | undefined;
3353

@@ -52,7 +72,7 @@ export const action: ActionFunction = async ({ request, params }) => {
5272
}
5373
}
5474

55-
return redirectWithSuccessMessage(submission.value.redirectUrl, request, message);
75+
return redirectWithSuccessMessage(safeRedirectUrl, request, message);
5676
} catch (error) {
5777
if (error instanceof Error) {
5878
logger.error("Failed to check batch completion", {
@@ -62,10 +82,10 @@ export const action: ActionFunction = async ({ request, params }) => {
6282
stack: error.stack,
6383
},
6484
});
65-
return redirectWithErrorMessage(submission.value.redirectUrl, request, error.message);
85+
return redirectWithErrorMessage(safeRedirectUrl, request, error.message);
6686
} else {
6787
logger.error("Failed to check batch completion", { error });
68-
return redirectWithErrorMessage(submission.value.redirectUrl, request, "Unknown error");
88+
return redirectWithErrorMessage(safeRedirectUrl, request, "Unknown error");
6989
}
7090
}
7191
};

apps/webapp/app/runEngine/concerns/idempotencyKeys.server.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,22 @@ export class IdempotencyKeyConcern {
248248

249249
//We're using `andWait` so we need to block the parent run with a waitpoint
250250
if (resumeParentOnCompletion && parentRunId) {
251+
// `parentRunId` comes from the request body and isn't re-validated
252+
// here, so confirm the parent run is in the caller's environment
253+
// before wiring a waitpoint against it.
254+
const parentRunInternalId = RunId.fromFriendlyId(parentRunId);
255+
const parentRunInCallerEnv = await runStore.findRun(
256+
{
257+
id: parentRunInternalId,
258+
runtimeEnvironmentId: request.environment.id,
259+
},
260+
{ select: { id: true } },
261+
this.prisma
262+
);
263+
if (!parentRunInCallerEnv) {
264+
throw new ServiceValidationError("Parent run not found in the calling environment", 404);
265+
}
266+
251267
// Get or create waitpoint lazily (existing run may not have one if it was standalone)
252268
let associatedWaitpoint = existingRun.associatedWaitpoint;
253269
if (!associatedWaitpoint) {
@@ -276,7 +292,7 @@ export class IdempotencyKeyConcern {
276292
: event.spanId;
277293

278294
await this.engine.blockRunWithWaitpoint({
279-
runId: RunId.fromFriendlyId(parentRunId),
295+
runId: parentRunInternalId,
280296
waitpoints: associatedWaitpoint!.id,
281297
spanIdToComplete: spanId,
282298
batch: request.options?.batchId
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import type { RunStore } from "@internal/run-store";
2+
import { BatchId } from "@trigger.dev/core/v3/isomorphic";
3+
import type { PrismaClientOrTransaction } from "@trigger.dev/database";
4+
5+
/**
6+
* Resolve the BatchTaskRun id for `batchId` (accepting either the friendlyId or
7+
* the internal id) only if `userId` is a member of the batch's owning
8+
* organization. Returns null otherwise. Batch lookup goes through runStore so
9+
* batches resident in either run-store database are visible.
10+
*/
11+
export async function findBatchRunIdForUser(
12+
prisma: PrismaClientOrTransaction,
13+
store: RunStore,
14+
batchId: string,
15+
userId: string
16+
): Promise<string | null> {
17+
const batchRunId = toBatchRunId(batchId);
18+
if (!batchRunId) return null;
19+
20+
const batchRun = await store.findBatchTaskRunById(batchRunId);
21+
if (!batchRun) return null;
22+
23+
return (await userCanAccessEnvironment(prisma, batchRun.runtimeEnvironmentId, userId))
24+
? batchRun.id
25+
: null;
26+
}
27+
28+
function toBatchRunId(batchId: string): string | null {
29+
try {
30+
return BatchId.toId(batchId);
31+
} catch {
32+
return null;
33+
}
34+
}
35+
36+
async function userCanAccessEnvironment(
37+
prisma: PrismaClientOrTransaction,
38+
runtimeEnvironmentId: string,
39+
userId: string
40+
): Promise<boolean> {
41+
const environment = await prisma.runtimeEnvironment.findFirst({
42+
where: {
43+
id: runtimeEnvironmentId,
44+
organization: { members: { some: { userId } } },
45+
},
46+
select: { id: true },
47+
});
48+
49+
return !!environment;
50+
}

apps/webapp/app/v3/services/batchTriggerV3.server.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { env } from "~/env.server";
2020
import { findEnvironmentById } from "~/models/runtimeEnvironment.server";
2121
import { batchTaskRunItemStatusForRunStatus } from "~/models/taskRun.server";
2222
import type { AuthenticatedEnvironment } from "~/services/apiAuth.server";
23+
import { dependentAttemptWhere } from "./dependentAttemptScope";
2324
import { logger } from "~/services/logger.server";
2425
import { getEntitlement } from "~/services/platform.v3.server";
2526
import { controlPlaneResolver } from "~/v3/runOpsMigration/controlPlaneResolver.server";
@@ -193,7 +194,8 @@ export class BatchTriggerV3Service extends BaseService {
193194

194195
const dependentAttempt = body?.dependentAttempt
195196
? await this._prisma.taskRunAttempt.findFirst({
196-
where: { friendlyId: body.dependentAttempt },
197+
// Scope to the caller's environment (see dependentAttemptWhere).
198+
where: dependentAttemptWhere(body.dependentAttempt, environment.id),
197199
include: {
198200
taskRun: {
199201
select: {
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import type { Prisma } from "@trigger.dev/database";
2+
3+
/**
4+
* Where-clause for resolving a dependent/parent TaskRunAttempt by friendlyId,
5+
* scoped to the caller's environment via the related run. The env scope keeps a
6+
* foreign friendlyId from resolving onto the new batch. Standalone builder so
7+
* the scope can be asserted directly in tests.
8+
*/
9+
export function dependentAttemptWhere(
10+
friendlyId: string,
11+
environmentId: string
12+
): Prisma.TaskRunAttemptWhereInput {
13+
return {
14+
friendlyId,
15+
taskRun: { runtimeEnvironmentId: environmentId },
16+
};
17+
}

apps/webapp/app/v3/services/triggerTaskV1.server.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import { isFinalAttemptStatus, isFinalRunStatus } from "../taskStatus";
3131
import { startActiveSpan } from "../tracer.server";
3232
import { clampMaxDuration } from "../utils/maxDuration";
3333
import { BaseService, ServiceValidationError } from "./baseService.server";
34+
import { attemptInEnvironmentWhere, batchRunInEnvironmentWhere } from "./triggerV1Scoping";
3435
import { EnqueueDelayedRunService } from "./enqueueDelayedRun.server";
3536
import { enqueueRun } from "./enqueueRun.server";
3637
import { ExpireEnqueuedRunService } from "./expireEnqueuedRun.server";
@@ -165,7 +166,7 @@ export class TriggerTaskServiceV1 extends BaseService {
165166

166167
const dependentAttempt = body.options?.dependentAttempt
167168
? await this._prisma.taskRunAttempt.findFirst({
168-
where: { friendlyId: body.options.dependentAttempt },
169+
where: attemptInEnvironmentWhere(body.options.dependentAttempt, environment.id),
169170
include: {
170171
taskRun: {
171172
select: {
@@ -205,7 +206,7 @@ export class TriggerTaskServiceV1 extends BaseService {
205206

206207
const parentAttempt = body.options?.parentAttempt
207208
? await this._prisma.taskRunAttempt.findFirst({
208-
where: { friendlyId: body.options.parentAttempt },
209+
where: attemptInEnvironmentWhere(body.options.parentAttempt, environment.id),
209210
include: {
210211
taskRun: {
211212
select: {
@@ -223,7 +224,7 @@ export class TriggerTaskServiceV1 extends BaseService {
223224

224225
const dependentBatchRun = body.options?.dependentBatch
225226
? await this._prisma.batchTaskRun.findFirst({
226-
where: { friendlyId: body.options.dependentBatch },
227+
where: batchRunInEnvironmentWhere(body.options.dependentBatch, environment.id),
227228
include: {
228229
dependentTaskAttempt: {
229230
include: {
@@ -270,7 +271,7 @@ export class TriggerTaskServiceV1 extends BaseService {
270271

271272
const parentBatchRun = body.options?.parentBatch
272273
? await this._prisma.batchTaskRun.findFirst({
273-
where: { friendlyId: body.options.parentBatch },
274+
where: batchRunInEnvironmentWhere(body.options.parentBatch, environment.id),
274275
include: {
275276
dependentTaskAttempt: {
276277
include: {
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import type { Prisma } from "@trigger.dev/database";
2+
3+
// Where-clauses for resolving caller-supplied parent/dependent attempt & batch
4+
// friendlyIds in the V1 trigger path, scoped to the caller's environment so a
5+
// foreign friendlyId can't be wired onto the new run/batch. Standalone builders
6+
// so the scope can be asserted directly in tests.
7+
8+
export function attemptInEnvironmentWhere(
9+
friendlyId: string,
10+
environmentId: string
11+
): Prisma.TaskRunAttemptWhereInput {
12+
return { friendlyId, taskRun: { runtimeEnvironmentId: environmentId } };
13+
}
14+
15+
export function batchRunInEnvironmentWhere(
16+
friendlyId: string,
17+
environmentId: string
18+
): Prisma.BatchTaskRunWhereInput {
19+
return { friendlyId, runtimeEnvironmentId: environmentId };
20+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import { WorkerInstanceGroupType } from "@trigger.dev/database";
2+
3+
/**
4+
* Whether a worker group may be used by the calling project.
5+
*
6+
* MANAGED groups are shared across projects. UNMANAGED groups are per-project
7+
* (masterQueue is `${projectId}-${name}`), so a project may only use an
8+
* UNMANAGED group whose `projectId` matches it. Dependency-free so it can be
9+
* unit-tested directly.
10+
*/
11+
export function isWorkerGroupAllowedForProject(
12+
workerGroup: { type: WorkerInstanceGroupType; projectId: string | null },
13+
projectId: string
14+
): boolean {
15+
if (workerGroup.type === WorkerInstanceGroupType.UNMANAGED) {
16+
return workerGroup.projectId === projectId;
17+
}
18+
return true;
19+
}

apps/webapp/app/v3/services/worker/workerGroupService.server.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { WorkerInstanceGroup, WorkloadType } from "@trigger.dev/database";
22
import { WorkerInstanceGroupType } from "@trigger.dev/database";
33
import { WithRunEngine } from "../baseService.server";
4+
import { isWorkerGroupAllowedForProject } from "./workerGroupAccess";
45
import { WorkerGroupTokenService } from "./workerGroupTokenService.server";
56
import { logger } from "~/services/logger.server";
67
import { FEATURE_FLAG } from "~/v3/featureFlags";
@@ -252,6 +253,13 @@ export class WorkerGroupService extends WithRunEngine {
252253
throw new Error(`The region you specified doesn't exist ("${regionOverride}").`);
253254
}
254255

256+
// The masterQueue-only lookup above can resolve another project's
257+
// UNMANAGED group, so reject groups not usable by this project
258+
// (see isWorkerGroupAllowedForProject).
259+
if (!isWorkerGroupAllowedForProject(workerGroup, project.id)) {
260+
throw new Error(`The region you specified isn't available to you ("${regionOverride}").`);
261+
}
262+
255263
// If they're restricted, check they have access
256264
if (project.allowedWorkerQueues.length > 0) {
257265
if (project.allowedWorkerQueues.includes(workerGroup.masterQueue)) {

0 commit comments

Comments
 (0)