Skip to content

Commit febbfef

Browse files
ericallamTrigger.dev RepoOps
authored andcommitted
fix(webapp): tighten session create and webhook endpoint permissions
Tighten three permission checks in the webapp: - The webhook endpoint page's generate secret, set secret and generate verify token actions now require `write:webhooks`, matching the API, and their buttons are disabled without it. - Session `externalId`s can no longer contain a `/channels` path segment or end in `:channels`, since both forms name a session's channels. Session create returns 422, and webhook session keys and playground chat ids follow the same rule. - Session create no longer stores the webhook-only `channelEvent` and `actionSource: "webhook"` fields from a client's base payload, matching how client `.in` appends are already handled. Mono-RevId: d1aa9aad641047f37d54396774d4dd4e84530e68
1 parent 46918b2 commit febbfef

13 files changed

Lines changed: 528 additions & 29 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: improvement
4+
---
5+
6+
Session external ids can no longer contain a `/channels` path segment (for example `chat-1/channels/support`) or end in `:channels`, since both forms name a session's channels. Creating a session with one now returns a 422 validation error.

‎apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.webhooks.endpoints.$endpointParam/route.tsx‎

Lines changed: 84 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import type { WebhookValueSource } from "@trigger.dev/core/v3";
1717
import { type MutableRefObject, Suspense, useEffect, useRef, useState } from "react";
1818
import { TypedAwait, typeddefer, useTypedLoaderData } from "remix-typedjson";
1919
import { z } from "zod";
20+
import type { RuntimeEnvironmentType } from "@trigger.dev/database";
2021
import { WebhookIcon } from "~/assets/icons/WebhookIcon";
2122
import { CodeBlock } from "~/components/code/CodeBlock";
2223
import { PageBody } from "~/components/layout/AppLayout";
@@ -127,6 +128,25 @@ async function canCancelWaiters(
127128
return auth.ok && auth.ability.can("write", { type: "waitpoints", envType: environment.type });
128129
}
129130

131+
const NO_WEBHOOKS_WRITE = "You don't have permission to change webhooks in this environment.";
132+
133+
async function canWriteWebhooks(
134+
request: Request,
135+
scope: {
136+
userId: string;
137+
organizationId: string;
138+
projectId: string;
139+
environmentType: RuntimeEnvironmentType;
140+
}
141+
): Promise<boolean> {
142+
const auth = await rbac.authenticateSession(request, {
143+
userId: scope.userId,
144+
organizationId: scope.organizationId,
145+
projectId: scope.projectId,
146+
});
147+
return auth.ok && auth.ability.can("write", { type: "webhooks", envType: scope.environmentType });
148+
}
149+
130150
export const loader = async ({ request, params }: LoaderFunctionArgs) => {
131151
const { user, project, environment, endpointParam } = await requireWebhookAccess(request, params);
132152

@@ -203,6 +223,12 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
203223

204224
return typeddefer({
205225
canCancelWaiters: await canCancelWaiters(request, { user, project, environment }),
226+
canWrite: await canWriteWebhooks(request, {
227+
userId: user.id,
228+
organizationId: project.organizationId,
229+
projectId: project.id,
230+
environmentType: environment.type,
231+
}),
206232
endpoint,
207233
ingestUrl,
208234
routing,
@@ -268,6 +294,14 @@ export const action = async ({ request, params }: ActionFunctionArgs) => {
268294
};
269295
}
270296

297+
const canWrite = await canWriteWebhooks(request, {
298+
userId: user.id,
299+
organizationId: project.organizationId,
300+
projectId: project.id,
301+
environmentType: environment.type,
302+
});
303+
if (!canWrite) throwPermissionDenied(NO_WEBHOOKS_WRITE);
304+
271305
// Generate (integrator-supplied secret): the UI reveals it ONCE for pasting into the provider.
272306
if (intent === "generate-secret") {
273307
const ttl = PreviousSecretTtlFormField.safeParse(Object.fromEntries(formData));
@@ -324,6 +358,7 @@ export default function Page() {
324358
waiters,
325359
composerEndpoints,
326360
canCancelWaiters: canCancel,
361+
canWrite,
327362
} = useTypedLoaderData<typeof loader>();
328363
const organization = useOrganization();
329364
const project = useProject();
@@ -485,6 +520,7 @@ export default function Page() {
485520
routing={routing}
486521
verifier={verifier}
487522
hasVerifyToken={hasVerifyToken}
523+
canWrite={canWrite}
488524
/>
489525
</ResizablePanel>
490526
</ResizablePanelGroup>
@@ -501,13 +537,18 @@ function EndpointSidebar({
501537
routing,
502538
verifier,
503539
hasVerifyToken,
540+
canWrite,
504541
}: {
505542
endpoint: WebhookEndpointDetail;
506543
ingestUrl: string;
507544
hasVerifyToken: boolean;
508545
routing: LoaderData["routing"];
509546
verifier: LoaderData["verifier"];
547+
canWrite: boolean;
510548
}) {
549+
const disabledReason = canWrite
550+
? undefined
551+
: "You don't have permission to change this endpoint.";
511552
const metadataJson =
512553
endpoint.metadata != null && Object.keys(endpoint.metadata as object).length > 0
513554
? JSON.stringify(endpoint.metadata, null, 2)
@@ -569,12 +610,16 @@ function EndpointSidebar({
569610
)}
570611
<div className="flex flex-wrap items-center gap-2">
571612
{canGenerate ? (
572-
<GenerateSecretDialog hasSigningSecret={endpoint.hasSigningSecret} />
613+
<GenerateSecretDialog
614+
hasSigningSecret={endpoint.hasSigningSecret}
615+
disabledReason={disabledReason}
616+
/>
573617
) : null}
574618
<SetSecretDialog
575619
hasSigningSecret={endpoint.hasSigningSecret}
576620
credentialNoun={credentialNoun}
577621
variant={canGenerate ? "tertiary/small" : "secondary/small"}
622+
disabledReason={disabledReason}
578623
/>
579624
</div>
580625
</div>
@@ -595,7 +640,10 @@ function EndpointSidebar({
595640
Not set, the provider cannot verify this URL yet
596641
</span>
597642
)}
598-
<GenerateVerifyTokenDialog hasVerifyToken={hasVerifyToken} />
643+
<GenerateVerifyTokenDialog
644+
hasVerifyToken={hasVerifyToken}
645+
disabledReason={disabledReason}
646+
/>
599647
<Hint>
600648
The provider sends a GET with <code>{getHandshake.tokenParam}</code> when you
601649
save the URL; it must carry this token.
@@ -841,10 +889,12 @@ function SetSecretDialog({
841889
hasSigningSecret,
842890
credentialNoun,
843891
variant = "secondary/small",
892+
disabledReason,
844893
}: {
845894
hasSigningSecret: boolean;
846895
credentialNoun: string;
847896
variant?: "secondary/small" | "tertiary/small";
897+
disabledReason?: string;
848898
}) {
849899
const fetcher = useFetcher<typeof action>();
850900
const [open, setOpen] = useState(false);
@@ -863,7 +913,12 @@ function SetSecretDialog({
863913
return (
864914
<Dialog open={open} onOpenChange={setOpen}>
865915
<DialogTrigger asChild>
866-
<Button variant={variant} LeadingIcon={KeyIcon}>
916+
<Button
917+
variant={variant}
918+
LeadingIcon={KeyIcon}
919+
disabled={disabledReason !== undefined}
920+
tooltip={disabledReason}
921+
>
867922
{verb} {credentialNoun}
868923
</Button>
869924
</DialogTrigger>
@@ -916,7 +971,13 @@ function SetSecretDialog({
916971

917972
// For integrator-supplied secrets (GitHub/GitLab/standard): mint a strong secret server-side,
918973
// store it, and reveal it ONCE so the user can paste it into their provider.
919-
function GenerateSecretDialog({ hasSigningSecret }: { hasSigningSecret: boolean }) {
974+
function GenerateSecretDialog({
975+
hasSigningSecret,
976+
disabledReason,
977+
}: {
978+
hasSigningSecret: boolean;
979+
disabledReason?: string;
980+
}) {
920981
const fetcher = useFetcher<typeof action>();
921982
const [open, setOpen] = useState(false);
922983
const isSubmitting = fetcher.state !== "idle";
@@ -930,7 +991,12 @@ function GenerateSecretDialog({ hasSigningSecret }: { hasSigningSecret: boolean
930991
return (
931992
<Dialog open={open} onOpenChange={setOpen}>
932993
<DialogTrigger asChild>
933-
<Button variant="secondary/small" LeadingIcon={SparklesIcon}>
994+
<Button
995+
variant="secondary/small"
996+
LeadingIcon={SparklesIcon}
997+
disabled={disabledReason !== undefined}
998+
tooltip={disabledReason}
999+
>
9341000
{hasSigningSecret ? "Regenerate secret" : "Generate secret"}
9351001
</Button>
9361002
</DialogTrigger>
@@ -995,7 +1061,13 @@ function GenerateSecretDialog({ hasSigningSecret }: { hasSigningSecret: boolean
9951061
* signing secret: minted server-side, stored encrypted, revealed once for the provider's Verify
9961062
* Token field. Regenerating replaces it and the URL must be re-verified in the provider.
9971063
*/
998-
function GenerateVerifyTokenDialog({ hasVerifyToken }: { hasVerifyToken: boolean }) {
1064+
function GenerateVerifyTokenDialog({
1065+
hasVerifyToken,
1066+
disabledReason,
1067+
}: {
1068+
hasVerifyToken: boolean;
1069+
disabledReason?: string;
1070+
}) {
9991071
const fetcher = useFetcher<typeof action>();
10001072
const [open, setOpen] = useState(false);
10011073
const [dismissedToken, setDismissedToken] = useState<string | undefined>(undefined);
@@ -1029,7 +1101,12 @@ function GenerateVerifyTokenDialog({ hasVerifyToken }: { hasVerifyToken: boolean
10291101
return (
10301102
<Dialog open={open} onOpenChange={onOpenChange}>
10311103
<DialogTrigger asChild>
1032-
<Button variant="secondary/small" LeadingIcon={SparklesIcon}>
1104+
<Button
1105+
variant="secondary/small"
1106+
LeadingIcon={SparklesIcon}
1107+
disabled={disabledReason !== undefined}
1108+
tooltip={disabledReason}
1109+
>
10331110
{hasVerifyToken ? "Regenerate verify token" : "Generate verify token"}
10341111
</Button>
10351112
</DialogTrigger>

‎apps/webapp/app/routes/api.v1.sessions.ts‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,10 @@ import { $replica, prisma, type PrismaClient } from "~/db.server";
1212
import { clickhouseFactory } from "~/services/clickhouse/clickhouseFactoryInstance.server";
1313
import { logger } from "~/services/logger.server";
1414
import { mintSessionToken } from "~/services/realtime/mintSessionToken.server";
15+
import { stripClientIngressFieldsFromBasePayload } from "~/services/realtime/sanitizeSessionInput.server";
1516
import {
1617
isSafeSessionExternalId,
17-
SESSION_CHANNEL_SCOPE_INFIX,
18+
SESSION_EXTERNAL_ID_FOLD_ERROR,
1819
} from "~/services/realtime/sessionChannels.server";
1920
import {
2021
ensureRunForSession,
@@ -163,12 +164,7 @@ const { action } = createActionApiRoute(
163164
async ({ authentication, body, ability }) => {
164165
try {
165166
if (body.externalId && !isSafeSessionExternalId(body.externalId)) {
166-
return json(
167-
{
168-
error: `externalId cannot contain "${SESSION_CHANNEL_SCOPE_INFIX}" or end in ":out" or ":in"`,
169-
},
170-
{ status: 422 }
171-
);
167+
return json({ error: SESSION_EXTERNAL_ID_FOLD_ERROR }, { status: 422 });
172168
}
173169

174170
const sessionIds = body.externalId ? [body.externalId] : [];
@@ -189,13 +185,18 @@ const { action } = createActionApiRoute(
189185
return json({ error: "Unauthorized" }, { status: 403 });
190186
}
191187

188+
const triggerConfig = {
189+
...body.triggerConfig,
190+
basePayload: stripClientIngressFieldsFromBasePayload(body.triggerConfig.basePayload),
191+
};
192+
192193
// Defer cached config changes until the stored task has been authorized.
193194
const { session, isCached } = await findOrCreateSession({
194195
environment: authentication.environment,
195196
externalId: body.externalId,
196197
type: body.type,
197198
taskIdentifier: body.taskIdentifier,
198-
triggerConfig: body.triggerConfig,
199+
triggerConfig,
199200
tags: body.tags,
200201
metadata: body.metadata as Record<string, unknown> | undefined,
201202
expiresAt: body.expiresAt,
@@ -241,7 +242,7 @@ const { action } = createActionApiRoute(
241242
session,
242243
await prisma.session.update({
243244
where: { id: session.id },
244-
data: { triggerConfig: body.triggerConfig as unknown as Prisma.InputJsonValue },
245+
data: { triggerConfig: triggerConfig as unknown as Prisma.InputJsonValue },
245246
select: { triggerConfig: true, updatedAt: true },
246247
})
247248
);

‎apps/webapp/app/routes/realtime.v1.sessions.$session.$io.append.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
resolveSessionWithWriterFallback,
77
} from "~/services/realtime/sessions.server";
88
import { stripClientIngressFields } from "~/services/realtime/sanitizeSessionInput.server";
9+
import { isSafeSessionExternalId } from "~/services/realtime/sessionChannels.server";
910
import { ensureRunThenAppend } from "~/services/realtime/sessionChannelAppend.server";
1011
import { anyResource, createActionApiRoute } from "~/services/routeBuilders/apiBuilder.server";
1112

@@ -57,7 +58,9 @@ const { action, loader } = createActionApiRoute(
5758
ids.add(session.friendlyId);
5859
if (session.externalId) ids.add(session.externalId);
5960
}
60-
return anyResource([...ids].map((id) => ({ type: "sessions", id })));
61+
return anyResource(
62+
[...ids].filter(isSafeSessionExternalId).map((id) => ({ type: "sessions", id }))
63+
);
6164
},
6265
},
6366
},

‎apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.playground.action.tsx‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@ import { findEnvironmentBySlug } from "~/models/runtimeEnvironment.server";
99
import { requireUserId } from "~/services/session.server";
1010
import { EnvironmentParamSchema } from "~/utils/pathBuilder";
1111
import { mintSessionToken } from "~/services/realtime/mintSessionToken.server";
12+
import {
13+
isSafeSessionExternalId,
14+
SESSION_EXTERNAL_ID_FOLD_ERROR,
15+
} from "~/services/realtime/sessionChannels.server";
16+
import { stripClientIngressFieldsFromBasePayload } from "~/services/realtime/sanitizeSessionInput.server";
1217
import { ensureRunForSession } from "~/services/realtime/sessionRunManager.server";
1318
import { runStore } from "~/v3/runStore.server";
1419

@@ -95,6 +100,9 @@ export const action = async ({ request, params }: ActionFunctionArgs) => {
95100
if (!chatId) {
96101
return json({ error: "chatId is required" }, { status: 400 });
97102
}
103+
if (!isSafeSessionExternalId(chatId)) {
104+
return json({ error: SESSION_EXTERNAL_ID_FOLD_ERROR }, { status: 400 });
105+
}
98106

99107
// Parse the optional initial payload — used as the basePayload
100108
// for the first run trigger. After session create, the agent
@@ -126,7 +134,7 @@ export const action = async ({ request, params }: ActionFunctionArgs) => {
126134
].slice(0, 10);
127135

128136
const triggerConfig = {
129-
basePayload: {
137+
basePayload: stripClientIngressFieldsFromBasePayload({
130138
// The first run boots before the user's first message lands on
131139
// `.in/append`, so it sees `messages: []` and `trigger: "preload"`.
132140
// Mirrors the defaults in `chat.createStartSessionAction` —
@@ -137,7 +145,7 @@ export const action = async ({ request, params }: ActionFunctionArgs) => {
137145
...payload,
138146
chatId,
139147
...(parsedClientData ? { metadata: parsedClientData } : {}),
140-
},
148+
}),
141149
...(machine ? { machine } : {}),
142150
tags,
143151
...(maxAttempts ? { maxAttempts: parseInt(maxAttempts, 10) } : {}),

‎apps/webapp/app/services/realtime/sanitizeSessionInput.server.ts‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,22 @@ export function stripClientIngressFields(part: string): string {
2424
const payload = (record as { payload?: unknown }).payload as Record<string, unknown> | undefined;
2525
if (!payload || typeof payload !== "object") return part;
2626

27+
return removeClientIngressFields(payload) ? JSON.stringify(record) : part;
28+
}
29+
30+
export function stripClientIngressFieldsFromBasePayload<T extends Record<string, unknown>>(
31+
basePayload: T
32+
): T {
33+
const copy = { ...basePayload };
34+
removeClientIngressFields(copy);
35+
return copy;
36+
}
37+
38+
function removeClientIngressFields(payload: Record<string, unknown>): boolean {
2739
const forgedWebhookAction =
2840
Object.hasOwn(payload, "actionSource") && payload.actionSource === "webhook";
2941
const forgedChannelEvent = Object.hasOwn(payload, "channelEvent");
30-
if (!forgedWebhookAction && !forgedChannelEvent) return part;
31-
3242
if (forgedWebhookAction) delete payload.actionSource;
3343
if (forgedChannelEvent) delete payload.channelEvent;
34-
return JSON.stringify(record);
44+
return forgedWebhookAction || forgedChannelEvent;
3545
}

0 commit comments

Comments
 (0)