Skip to content

Commit 677ce4c

Browse files
carderned-csclaude
authored
fix(webapp,core): harden CLI auth-code login, socket-auth (#53)
* fix(webapp): gate CLI auth-code PAT minting behind explicit consent + rate limit The `/account/authorization-code/:code` loader minted and bound a Personal Access Token as a pure side effect of a GET, so any authenticated browser that merely landed on the URL (phished link, prefetch) silently issued a CLI PAT bound to an attacker-supplied code, which the attacker then harvested from the unauthenticated, unthrottled `/api/v1/token` endpoint. - Move the mint out of the loader into an `action` behind an explicit "Authorize" POST from a logged-in human. The loader now only renders a consent screen (read-only `isAuthorizationCodeMintable`). The CLI contract is unchanged: it never calls this route and keeps polling `/api/v1/token`, which already returns `{ token: null }` until consent is given. - Add IP rate limiting to `/api/v1/authorization-code` (mint) and per-code rate limiting to `/api/v1/token` (poll). The poll limiter is keyed by the code, not the IP, so the CLI's ~1/s poll loop isn't broken behind a shared NAT. - Remove both endpoints from the global rate-limit allowlist. - Shorten the unconsumed-code TTL from 10 minutes to 2. Response shapes are unchanged; the only new behavior is a 429 on limit breach. Addresses the auth-code half of GHSA-58mc (TRI-9770). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 92ef2c3cd68fc6c5d3e7b01ad553bae5fcf121b6) * fix(webapp): require ADMIN role to rename or delete a project verifyProjectMembership only proved org membership, so any MEMBER-role invitee could rename or permanently delete a project and all its runs, schedules, and environments. Gate the rename/delete intents on OrgMember.role === ADMIN. Addresses TRI-9870. Co-authored-by: Daniel Sutton <dansutton@trigger.dev> * fix(webapp): keep auth-code/token endpoints allowlisted (don't break CLI login) The global apiRateLimiter keys on the Authorization header and returns 401 for any matched-but-unauthenticated /api path BEFORE the route runs. The CLI auth-code/token endpoints are intentionally unauthenticated, so removing them from the allowlist 401s them and breaks CLI login outright. Restore the allowlist entries (skipping the auth-keyed global limiter) — the dedicated authCodeRateLimiter in the route actions still provides the throttle. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit b823a8750b570802f245cbbab3d948b509db3cef) * fix(webapp): require ADMIN for org delete/rename and non-DEV key regen Add an isOrgAdmin helper (legacy OrgMember.role, independent of the RBAC ability layer so it holds in OSS deployments) and gate on it for: - organization rename/delete (settings index action) - regenerating non-development API keys (production/staging/preview are org-wide; DEV self-service is unchanged) Both previously checked only org membership, letting any MEMBER perform them. Addresses TRI-9870. Co-authored-by: Daniel Sutton <dansutton@trigger.dev> * fix(webapp): bound auth-code minting even without X-Forwarded-For + record deferral - The mint rate limiter previously no-op'd when extractClientIp returned null (no X-Forwarded-For, e.g. non-ALB/direct deploys), leaving minting unbounded there. Fall back to a shared key so it's always throttled. - Add a server-changes note documenting the auth-code/org-admin hardening and recording the intentional inviteMembers-baseline deferral so it isn't lost. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit d532795a28ac76b52b14d6a0ef72ac401535c7e1) * fix(webapp): restore auth-code TTL to original 10 minutes The 2-minute TTL was imported from the third-party report's recommendation, not grounded in a real need. The consent gate is what closes the phishing vector; the TTL length is immaterial to it. Restore the original 10-minute window (keep the shared constant purely to stop the mint/read paths drifting). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 70951ddb57faf9ffb96f032110fda1668ea4ed62) * fix(cli): widen login poll window to fit the new consent click The auth-code login page now requires an explicit "Authorize" click before a PAT is minted (it no longer mints as a side effect of loading the page). The CLI/MCP poll loop was sized (~60s) for the old auto-mint flow, so a human who takes longer than ~60s to approve would get "Failed to get access token" even though the code is valid for 10 minutes. Widen the poll to ~5 minutes — within the code TTL and the per-code poll rate limit (~1/s). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 80c39d6f0d63c89a6b50fcf8b80c02c1801472ff) * docs(webapp): correct stale CLI poll-count in auth-code rate-limiter comment The CLI poll window was widened to ~5 min, so "up to ~61 times/min / 60 retries" no longer describes it. The steady cadence is ~1/s (~60/min), still under the 100/min/code cap regardless of total poll count. Comment-only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 3f87584db9c7e91474977ae9431b3b8b71c2e6d2) * refactor(webapp): use findFirst over findUnique in auth-code lookups Code-review cleanup: findFirst avoids the findUnique constraint on the non-unique lookup shape used by the auth-code mint/consent path. Co-authored-by: Daniel Sutton <dansutton@trigger.dev> * fix(webapp): skip auth-code mint limit when there's no trustworthy client IP The shared "no-forwarded-for" fallback collapsed every X-Forwarded-For-less request into one 30/min bucket, letting a single client DoS login for a whole non-ALB instance. Skip the limit when there's no client IP instead — matching the existing magicLinkRateLimiter pattern. Our cloud is behind an ALB so the per-IP limit always applies there; the consent gate (not this limit) is what closes the PAT-theft vector, so leaving non-proxied self-host minting unbounded is low-risk row churn rather than a login outage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit e5b9e109384b163b397ad879c9c0ae3b7e4d1853) * docs: scope server-changes note to shipped fixes * format * fix(webapp): use conform submission.reply for ADMIN-denied 403s The ADMIN gate returned the pre-parseWithZod `submission.error[""]` shape, which no longer typechecks (TS2339) and would not surface in the form. Use `submission.reply({ formErrors })` so the denial message renders and the types line up with the parseWithZod migration on the base branch. * chore: consolidate changeset and tighten comments * fix(cli): retry auth-code polling on 429 instead of aborting The login poll loop threw an AbortError for any failed token response, which pRetry treats as fatal. A 429 from the per-code poll rate limiter would therefore abandon the whole login ("Failed to get access token") even though the auth code is still valid and the user may not have approved the consent screen yet. Expose the HTTP status on wrapZodFetch failures and, in the poll path, treat a 429 as a retryable error so the loop backs off and keeps polling. Covers both the CLI login command and the MCP auth flow, which share the same getPersonalAccessToken helper. * chore(webapp): scope changes to auth-code login * chore: align auth-code login release note --------- Co-authored-by: Daniel Sutton <dansutton@trigger.dev> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 7cb35f1 commit 677ce4c

11 files changed

Lines changed: 370 additions & 64 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@trigger.dev/core": patch
3+
"trigger.dev": patch
4+
---
5+
6+
Require explicit browser approval for CLI and MCP login, with resilient polling while approval is pending.

apps/webapp/app/routes/account.authorization-code.$authorizationCode/route.tsx

Lines changed: 121 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,19 @@
11
import { CheckCircleIcon } from "@heroicons/react/24/solid";
2-
import type { LoaderFunctionArgs } from "@remix-run/server-runtime";
3-
import { typedjson, useTypedLoaderData } from "remix-typedjson";
2+
import { Form } from "@remix-run/react";
3+
import type { ActionFunctionArgs, LoaderFunctionArgs } from "@remix-run/server-runtime";
4+
import { typedjson, useTypedActionData, useTypedLoaderData } from "remix-typedjson";
45
import { z } from "zod";
56
import { AppContainer, MainCenteredContainer } from "~/components/layout/AppLayout";
7+
import { Button } from "~/components/primitives/Buttons";
68
import { Callout } from "~/components/primitives/Callout";
79
import { Header1 } from "~/components/primitives/Headers";
810
import { Icon } from "~/components/primitives/Icon";
911
import { Paragraph } from "~/components/primitives/Paragraph";
1012
import { logger } from "~/services/logger.server";
11-
import { createPersonalAccessTokenFromAuthorizationCode } from "~/services/personalAccessToken.server";
13+
import {
14+
createPersonalAccessTokenFromAuthorizationCode,
15+
isAuthorizationCodeMintable,
16+
} from "~/services/personalAccessToken.server";
1217
import { requireUserId } from "~/services/session.server";
1318

1419
const ParamsSchema = z.object({
@@ -20,49 +25,57 @@ const SearchParamsSchema = z.object({
2025
clientName: z.string().optional(),
2126
});
2227

23-
export const loader = async ({ request, params }: LoaderFunctionArgs) => {
24-
const userId = await requireUserId(request);
25-
28+
function parseParams(params: unknown) {
2629
const parsedParams = ParamsSchema.safeParse(params);
27-
2830
if (!parsedParams.success) {
2931
logger.info("Invalid params", { params });
30-
throw new Response(undefined, {
31-
status: 400,
32-
statusText: "Invalid params",
33-
});
32+
throw new Response(undefined, { status: 400, statusText: "Invalid params" });
3433
}
34+
return parsedParams.data;
35+
}
3536

37+
function parseSearch(request: Request) {
3638
const url = new URL(request.url);
3739
const searchObject = Object.fromEntries(url.searchParams.entries());
38-
3940
const searchParams = SearchParamsSchema.safeParse(searchObject);
40-
4141
const source = (searchParams.success ? searchParams.data.source : undefined) ?? "cli";
4242
const clientName = (searchParams.success ? searchParams.data.clientName : undefined) ?? "unknown";
43+
return { source, clientName };
44+
}
45+
46+
// The loader only renders a consent screen; minting/binding a PAT happens in
47+
// the `action`, behind an explicit "Authorize" POST.
48+
export const loader = async ({ request, params }: LoaderFunctionArgs) => {
49+
await requireUserId(request);
50+
51+
const { authorizationCode } = parseParams(params);
52+
const { source, clientName } = parseSearch(request);
53+
54+
const mintable = await isAuthorizationCodeMintable(authorizationCode);
55+
56+
return typedjson({
57+
status: mintable ? ("consent" as const) : ("invalid" as const),
58+
source,
59+
clientName,
60+
});
61+
};
62+
63+
export const action = async ({ request, params }: ActionFunctionArgs) => {
64+
const userId = await requireUserId(request);
65+
66+
const { authorizationCode } = parseParams(params);
67+
const { source, clientName } = parseSearch(request);
4368

4469
try {
45-
const _personalAccessToken = await createPersonalAccessTokenFromAuthorizationCode(
46-
parsedParams.data.authorizationCode,
47-
userId
48-
);
49-
return typedjson({
50-
success: true as const,
51-
source,
52-
clientName,
53-
});
70+
await createPersonalAccessTokenFromAuthorizationCode(authorizationCode, userId);
71+
return typedjson({ success: true as const, source, clientName });
5472
} catch (error) {
5573
if (error instanceof Response) {
5674
throw error;
5775
}
5876

5977
if (error instanceof Error) {
60-
return typedjson({
61-
success: false as const,
62-
error: error.message,
63-
source,
64-
clientName,
65-
});
78+
return typedjson({ success: false as const, error: error.message, source, clientName });
6679
}
6780

6881
logger.error(JSON.stringify(error));
@@ -74,32 +87,78 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
7487
};
7588

7689
export default function Page() {
77-
const result = useTypedLoaderData<typeof loader>();
90+
const loaderData = useTypedLoaderData<typeof loader>();
91+
const actionData = useTypedActionData<typeof action>();
92+
93+
// After the consent POST: success or failure.
94+
if (actionData) {
95+
return (
96+
<AuthShell>
97+
{actionData.success ? (
98+
<div>
99+
<Header1 className="mb-2 flex items-center gap-1">
100+
<Icon icon={CheckCircleIcon} className="h-6 w-6 text-emerald-500" /> Successfully
101+
authenticated
102+
</Header1>
103+
<Paragraph>
104+
{getInstructionsForSource(actionData.source, actionData.clientName)}
105+
</Paragraph>
106+
</div>
107+
) : (
108+
<div>
109+
<Header1 className="mb-2">Authentication failed</Header1>
110+
<Callout variant="error" className="my-2">
111+
{actionData.error}
112+
</Callout>
113+
<Paragraph spacing>
114+
There was a problem authenticating you, please try logging in with your CLI again.
115+
</Paragraph>
116+
</div>
117+
)}
118+
</AuthShell>
119+
);
120+
}
121+
122+
// Initial GET: invalid/expired code, or the consent prompt.
123+
if (loaderData.status === "invalid") {
124+
return (
125+
<AuthShell>
126+
<div>
127+
<Header1 className="mb-2">Authentication failed</Header1>
128+
<Callout variant="error" className="my-2">
129+
This login link is invalid or has expired.
130+
</Callout>
131+
<Paragraph spacing>
132+
Please try logging in with your CLI again to get a fresh link.
133+
</Paragraph>
134+
</div>
135+
</AuthShell>
136+
);
137+
}
78138

139+
return (
140+
<AuthShell>
141+
<div className="flex flex-col gap-4">
142+
<Header1>Authorize login</Header1>
143+
<Paragraph>{getConsentPrompt(loaderData.source, loaderData.clientName)}</Paragraph>
144+
<Form method="post">
145+
<Button type="submit" variant="primary/medium" fullWidth>
146+
Authorize
147+
</Button>
148+
</Form>
149+
<Paragraph variant="extra-small">
150+
Only authorize if you started this login yourself. If you didn't, close this page.
151+
</Paragraph>
152+
</div>
153+
</AuthShell>
154+
);
155+
}
156+
157+
function AuthShell({ children }: { children: React.ReactNode }) {
79158
return (
80159
<AppContainer>
81160
<MainCenteredContainer className="max-w-88">
82-
<div className="flex flex-col items-center space-y-4">
83-
{result.success ? (
84-
<div>
85-
<Header1 className="mb-2 flex items-center gap-1">
86-
<Icon icon={CheckCircleIcon} className="h-6 w-6 text-emerald-500" /> Successfully
87-
authenticated
88-
</Header1>
89-
<Paragraph>{getInstructionsForSource(result.source, result.clientName)}</Paragraph>
90-
</div>
91-
) : (
92-
<div>
93-
<Header1 className="mb-2">Authentication failed</Header1>
94-
<Callout variant="error" className="my-2">
95-
{result.error}
96-
</Callout>
97-
<Paragraph spacing>
98-
There was a problem authenticating you, please try logging in with your CLI again.
99-
</Paragraph>
100-
</div>
101-
)}
102-
</div>
161+
<div className="flex flex-col items-center space-y-4">{children}</div>
103162
</MainCenteredContainer>
104163
</AppContainer>
105164
);
@@ -113,6 +172,18 @@ const prettyClientNames: Record<string, string> = {
113172
"claude-ai": "Claude Desktop",
114173
};
115174

175+
function getConsentPrompt(source: string, clientName: string) {
176+
if (source === "mcp") {
177+
const pretty = prettyClientNames[clientName] ?? clientName;
178+
if (pretty && pretty !== "unknown") {
179+
return `Authorize ${pretty} to access your Trigger.dev account?`;
180+
}
181+
return `Authorize this MCP client to access your Trigger.dev account?`;
182+
}
183+
184+
return `Authorize the Trigger.dev CLI to access your account?`;
185+
}
186+
116187
function getInstructionsForSource(source: string, clientName: string) {
117188
if (source === "mcp") {
118189
if (clientName) {

apps/webapp/app/routes/api.v1.authorization-code.ts

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,12 @@ import { json } from "@remix-run/server-runtime";
33
import type { CreateAuthorizationCodeResponse } from "@trigger.dev/core/v3";
44
import { env } from "~/env.server";
55
import { logger } from "~/services/logger.server";
6+
import {
7+
AuthorizationCodeRateLimitError,
8+
checkAuthorizationCodeMintRateLimit,
9+
} from "~/services/authCodeRateLimiter.server";
610
import { createAuthorizationCode } from "~/services/personalAccessToken.server";
11+
import { extractClientIp } from "~/utils/extractClientIp.server";
712

813
/** Used to create an AuthorizationCode, that can then be used to obtain a Personal Access Token by logging in with the provided URL */
914
export async function action({ request }: ActionFunctionArgs) {
@@ -14,8 +19,24 @@ export async function action({ request }: ActionFunctionArgs) {
1419
return { status: 405, body: "Method Not Allowed" };
1520
}
1621

17-
//there is no authentication on this endpoint, anyone can create an AuthorizationCode.
18-
//they're only used to allow a user to login, when they'll then receive a Personal Access Token
22+
//this endpoint is unauthenticated (codes only allow a user to log in), so it's
23+
//rate-limited per client IP. Keyed by X-Forwarded-For; if there's no trustworthy
24+
//client IP we skip the limit rather than bucket everyone together. Self-hosters
25+
//wanting per-IP limiting should front the app with a proxy that sets X-Forwarded-For.
26+
const clientIp = extractClientIp(request.headers.get("x-forwarded-for"));
27+
if (clientIp) {
28+
try {
29+
await checkAuthorizationCodeMintRateLimit(clientIp);
30+
} catch (error) {
31+
if (error instanceof AuthorizationCodeRateLimitError) {
32+
return json(
33+
{ error: "Too many requests, please try again later." },
34+
{ status: 429, headers: { "Retry-After": Math.ceil(error.retryAfter / 1000).toString() } }
35+
);
36+
}
37+
throw error;
38+
}
39+
}
1940

2041
try {
2142
const authorizationCode = await createAuthorizationCode();

apps/webapp/app/routes/api.v1.token.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ import type { GetPersonalAccessTokenResponse } from "@trigger.dev/core/v3";
44
import { GetPersonalAccessTokenRequestSchema } from "@trigger.dev/core/v3";
55
import { generateErrorMessage } from "zod-error";
66
import { logger } from "~/services/logger.server";
7+
import {
8+
AuthorizationCodeRateLimitError,
9+
checkAuthorizationCodeTokenPollRateLimit,
10+
} from "~/services/authCodeRateLimiter.server";
711
import { getPersonalAccessTokenFromAuthorizationCode } from "~/services/personalAccessToken.server";
812
import { clientSafeErrorMessage } from "~/utils/prismaErrors";
913

@@ -25,6 +29,20 @@ export async function action({ request }: ActionFunctionArgs) {
2529
return json({ error: generateErrorMessage(body.error.issues) }, { status: 422 });
2630
}
2731

32+
// Per-code rate limit (keyed by the code, not the IP, so the CLI's poll loop
33+
// isn't broken behind a shared NAT).
34+
try {
35+
await checkAuthorizationCodeTokenPollRateLimit(body.data.authorizationCode);
36+
} catch (error) {
37+
if (error instanceof AuthorizationCodeRateLimitError) {
38+
return json(
39+
{ error: "Too many requests, please try again later." },
40+
{ status: 429, headers: { "Retry-After": Math.ceil(error.retryAfter / 1000).toString() } }
41+
);
42+
}
43+
throw error;
44+
}
45+
2846
try {
2947
const personalAccessToken = await getPersonalAccessTokenFromAuthorizationCode(
3048
body.data.authorizationCode

apps/webapp/app/services/apiRateLimit.server.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ export const apiRateLimiter = authorizationRateLimitMiddleware({
4848
// Allow /api/v1/tasks/:id/callback/:secret
4949
pathWhiteList: [
5050
"/api/internal/stripe_webhooks",
51+
// Keep allowlisted: these CLI endpoints are intentionally unauthenticated,
52+
// so this Authorization-header-keyed limiter would 401 them. They are
53+
// throttled separately by authCodeRateLimiter.server.ts.
5154
"/api/v1/authorization-code",
5255
"/api/v1/token",
5356
"/api/v1/usage/ingest",

0 commit comments

Comments
 (0)