-
-
Notifications
You must be signed in to change notification settings - Fork 788
feat: introduce organization access tokens #2391
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
WalkthroughAdds organization access tokens and a unified authenticateRequest API auth flow (supporting personal, organization, and apiKey). Refactors multiple API routes and whoami v2 to use the centralized auth and environment resolution. Introduces organizationAccessToken server module (create/list/revoke/authenticate), new token utilities (encrypt/decrypt/hash), and moves personal access token crypto to the shared utilities. Enforces ENCRYPTION_KEY must be 32 bytes and updates test/CI/docker envs. Adds OrganizationAccessToken model and enum to Prisma schema and migration. CLI changes validate token type and propagate tokenType in sessions. Estimated code review effort🎯 5 (Critical) | ⏱️ ~90 minutes ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 13
🔭 Outside diff range comments (8)
packages/cli-v3/src/utilities/session.ts (3)
7-18
: LoginResultOk forces user fields even for organization tokens — make userId/email optional.whoAmI for organization tokens may not include userId/email. The current type enforces them, which can cause runtime mismatch. Make these fields optional or use a discriminated union on auth.tokenType.
Proposed shape (minimal change):
export type LoginResultOk = { ok: true; profile: string; userId?: string; // optional for organization tokens email?: string; // optional for organization tokens dashboardUrl: string; auth: { apiUrl: string; accessToken: string; tokenType: "personal" | "organization"; }; };
43-52
: tokenType is hard-coded to "personal" in the error branch — derive it from the token prefix.When whoAmI fails, we can still reliably infer token type using validateAccessToken. Otherwise, commands that depend on tokenType may misbehave for organization tokens.
Apply this diff:
return { ok: false as const, error: userData.error, auth: { apiUrl: config.apiUrl, accessToken: config.accessToken, - tokenType: "personal", + tokenType: + (validateAccessToken(config.accessToken).success && + validateAccessToken(config.accessToken).type) || + "personal", }, };Add the missing import at the top of the file:
import { validateAccessToken } from "./isPersonalAccessToken.js";
55-66
: session.ts — don't hard-code tokenType; detect it and gate user fieldsWhoAmI responses (WhoAmIResponseSchema) do not include tokenType today, and the server returns a dummy identity for org tokens. Replace the hard-coded "personal" in session.ts with detection (prefer a server-provided tokenType if/when added; otherwise use validateAccessToken) and only populate userId/email for personal tokens.
Files to update
- packages/cli-v3/src/utilities/session.ts — make userId/email optional, import validateAccessToken, detect tokenType, gate userId/email.
- packages/cli-v3/src/commands/login.ts — replace other places that hard-code tokenType: "personal" (search for "tokenType: "personal" as const") to use the same detection or rely on validateAccessToken.
- (Optional server change) packages/core/src/v3/schemas/api.ts & apps/webapp/app/routes/api.v2.whoami.ts — if you prefer the server to return tokenType, add it to WhoAmIResponse and return it from the endpoint; otherwise keep client-side detection.
Suggested patch for session.ts
*** Update File: packages/cli-v3/src/utilities/session.ts @@ import { recordSpanException } from "@trigger.dev/core/v3/workers"; import { CliApiClient } from "../apiClient.js"; import { readAuthConfigProfile } from "./configFiles.js"; import { logger } from "./logger.js"; import { GitMeta } from "@trigger.dev/core/v3"; +import { validateAccessToken } from "./isPersonalAccessToken.js"; export type LoginResultOk = { ok: true; profile: string; - userId: string; - email: string; + // Only set for personal tokens; keep optional otherwise + userId?: string; + email?: string; dashboardUrl: string; auth: { apiUrl: string; accessToken: string; tokenType: "personal" | "organization"; }; }; @@ - if (!userData.success) { - return { - ok: false as const, - error: userData.error, - auth: { - apiUrl: config.apiUrl, - accessToken: config.accessToken, - tokenType: "personal", - }, - }; - } + if (!userData.success) { + const validation = validateAccessToken(config.accessToken); + const detectedTokenType = + (userData as any)?.data?.tokenType || (validation.success ? validation.type : "personal"); + + return { + ok: false as const, + error: userData.error, + auth: { + apiUrl: config.apiUrl, + accessToken: config.accessToken, + tokenType: detectedTokenType, + }, + }; + } - return { - ok: true as const, - profile, - userId: userData.data.userId, - email: userData.data.email, - dashboardUrl: userData.data.dashboardUrl, - auth: { - apiUrl: config.apiUrl, - accessToken: config.accessToken, - tokenType: "personal", - }, - }; + const validation = validateAccessToken(config.accessToken); + const detectedTokenType = + (userData.data as any)?.tokenType || (validation.success ? validation.type : "personal"); + + return { + ok: true as const, + profile, + // Only populate real user info for personal tokens + userId: detectedTokenType === "personal" ? userData.data.userId : undefined, + email: detectedTokenType === "personal" ? userData.data.email : undefined, + dashboardUrl: userData.data.dashboardUrl, + auth: { + apiUrl: config.apiUrl, + accessToken: config.accessToken, + tokenType: detectedTokenType, + }, + };Follow-up: If the server adds tokenType to WhoAmIResponse, prefer that value instead of prefix detection. Until then, use validateAccessToken as shown and make userId/email optional and gated.
packages/cli-v3/src/commands/login.ts (1)
347-356
: Embedded login error handling misses NotAccessTokenError (regression risk).The try/catch currently only rethrows NotPersonalAccessTokenError, but the env-token branch now throws NotAccessTokenError. In embedded mode this changes behavior from rethrowing to swallowing and returning { ok: false }, which is likely unintended.
Apply this diff to handle both error types:
if (options?.embedded) { - if (e instanceof NotPersonalAccessTokenError) { + if (e instanceof NotAccessTokenError || e instanceof NotPersonalAccessTokenError) { throw e; } return { ok: false as const, error: e instanceof Error ? e.message : String(e), }; }apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.$name.ts (1)
65-71
: Guard against invalid JSON body to avoid 500srequest.json() will throw on invalid JSON. Wrap in try/catch and return 400 with a helpful error.
Apply this diff:
- const jsonBody = await request.json(); + let jsonBody: unknown; + try { + jsonBody = await request.json(); + } catch { + return json({ error: "Invalid JSON body" }, { status: 400 }); + }apps/webapp/app/services/personalAccessToken.server.ts (2)
236-243
: Bug: response property typo “obfuscateToken” should be “obfuscatedToken”This breaks consumers expecting obfuscatedToken and is inconsistent with the new-token path.
Apply this diff:
return { id: existingCliPersonalAccessToken.id, name: existingCliPersonalAccessToken.name, userId: existingCliPersonalAccessToken.userId, - obfuscateToken: existingCliPersonalAccessToken.obfuscatedToken, + obfuscatedToken: existingCliPersonalAccessToken.obfuscatedToken, };
158-165
: Use timing-safe equality when comparing tokensEven though the hashed lookup already validated the token, this equality check guards against misconfigured encryption keys. Use timingSafeEqual to avoid leaking timing differences.
Apply this diff (add import and replace the comparison):
@@ -import { logger } from "./logger.server"; +import { logger } from "./logger.server"; +import { timingSafeEqual } from "node:crypto"; @@ - if (decryptedToken !== token) { + { + const a = Buffer.from(decryptedToken); + const b = Buffer.from(token); + const equal = a.length === b.length && timingSafeEqual(a, b); + if (!equal) { logger.error( `PersonalAccessToken with id: ${personalAccessToken.id} was found in the database with hash ${hashedToken}, but the decrypted token did not match the provided token.` ); return; - } + } + }apps/webapp/app/services/organizationAccessToken.server.ts (1)
189-206
: Avoid throwing from decryptOrganizationAccessToken; let callers decide error handlingThrowing here bubbles a 500 to auth callers. Either return a Result type or undefined with logging, or keep this pure and ensure callers catch errors. If you keep it throwing, ensure all callers wrap in try/catch (see prior suggestion on lines 95-138).
Option to return Result:
-export function decryptOrganizationAccessToken(organizationAccessToken: OrganizationAccessToken) { +export function decryptOrganizationAccessToken( + organizationAccessToken: OrganizationAccessToken +): string { const encryptedData = EncryptedSecretValueSchema.safeParse( organizationAccessToken.encryptedToken ); if (!encryptedData.success) { - throw new Error( - `Unable to parse encrypted OrganizationAccessToken with id: ${organizationAccessToken.id}: ${encryptedData.error.message}` - ); + logger.error( + `Unable to parse encrypted OrganizationAccessToken`, + { id: organizationAccessToken.id, issues: encryptedData.error.issues } + ); + throw new Error("Invalid encrypted token payload"); } const decryptedToken = decryptToken( encryptedData.data.nonce, encryptedData.data.ciphertext, encryptedData.data.tag ); return decryptedToken; }Combined with try/catch at call-site (previous comment), this prevents 500s from malformed data.
🧹 Nitpick comments (16)
packages/cli-v3/src/utilities/isPersonalAccessToken.ts (2)
12-24
: Harden validateAccessToken with token.trim() and single-pass validation.Incoming tokens may include accidental whitespace (copy/paste). Normalizing once avoids duplicate work and subtle bugs.
Apply this diff:
export function validateAccessToken( token: string ): { success: true; type: "personal" | "organization" } | { success: false } { - if (isPersonalAccessToken(token)) { + const t = token.trim(); + if (isPersonalAccessToken(t)) { return { success: true, type: "personal" }; } - if (isOrganizationAccessToken(token)) { + if (isOrganizationAccessToken(t)) { return { success: true, type: "organization" }; } return { success: false }; }
33-38
: NotAccessTokenError is defined but not used.Either integrate it (e.g., optional throwing variant of validateAccessToken) or remove to avoid dead code.
Example usage:
export function assertAccessToken(token: string): "personal" | "organization" { const res = validateAccessToken(token); if (!res.success) throw new NotAccessTokenError("Invalid access token"); return res.type; }internal-packages/database/prisma/schema.prisma (1)
136-161
: Model looks solid; add indexes for common queries and consider ON DELETE behavior.Operational suggestions:
- Expected query patterns will often include:
- organizationId and revokedAt IS NULL
- expiresAt comparisons
- lastAccessedAt sorting
- Consider adding indexes to support those and making cleanup fast.
Suggested additions:
model OrganizationAccessToken { // ...existing fields... @@index([organizationId]) @@index([organizationId, revokedAt]) @@index([organizationId, expiresAt]) @@index([lastAccessedAt]) }Also consider whether you want ON DELETE CASCADE to purge tokens with their organization:
- Current relation omits onDelete; if organizations can be deleted, setting onDelete: Cascade will prevent orphans.
packages/cli-v3/src/commands/login.ts (1)
283-287
: Preserve the effective API URL when storing the profile.You’re writing opts.defaultApiUrl, which may not match the effective URL used (e.g., from an existing profile or self-hosted config). Prefer persisting the actual value used to authenticate.
- writeAuthConfigProfile( - { - accessToken: indexResult.token, - apiUrl: opts.defaultApiUrl, - }, - options?.profile - ); + const effectiveApiUrl = authConfig?.apiUrl ?? opts.defaultApiUrl; + writeAuthConfigProfile( + { + accessToken: indexResult.token, + apiUrl: effectiveApiUrl, + }, + options?.profile + );apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.$name.ts (1)
24-28
: Message wording may mislead now that PAT/OAT are supportedThe 401 message says “API key” only; this route now accepts PATs and OATs too. Consider a clearer message.
Apply this diff:
- return json({ error: "Invalid or Missing API key" }, { status: 401 }); + return json({ error: "Invalid or missing credentials" }, { status: 401 });apps/webapp/app/utils/tokens.ts (1)
35-39
: Optional: prefer HMAC for token lookup hashingTokens are high-entropy, so SHA-256 is acceptable. Using HMAC-SHA-256 with a server secret prevents length-extension attacks and standardizes keyed hashing for lookups.
Example:
-export function hashToken(token: string): string { - const hash = nodeCrypto.createHash("sha256"); - hash.update(token); - return hash.digest("hex"); -} +export function hashToken(token: string): string { + const mac = nodeCrypto.createHmac("sha256", ENCRYPTION_KEY); + mac.update(token); + return mac.digest("hex"); +}internal-packages/database/prisma/migrations/20250813103207_add_organization_access_token/migration.sql (2)
1-20
: Add indexes to support frequent queries (list, validate, cleanup)Consider adding:
- Index on organizationId for listing tokens per org.
- Composite or partial index for active tokens (organizationId, revokedAt) WHERE revokedAt IS NULL.
- Optional index on expiresAt for cleanup jobs.
Suggested additions:
+CREATE INDEX "OrganizationAccessToken_organizationId_idx" + ON "OrganizationAccessToken"("organizationId"); + +CREATE INDEX "OrganizationAccessToken_active_org_idx" + ON "OrganizationAccessToken"("organizationId", "revokedAt") + WHERE "revokedAt" IS NULL; + +CREATE INDEX "OrganizationAccessToken_expiresAt_idx" + ON "OrganizationAccessToken"("expiresAt");
19-19
: Review FK delete behaviorON DELETE RESTRICT will block org deletion if tokens exist. If tokens should be removed with the org, switch to CASCADE. Otherwise, ensure the delete flow explicitly revokes/deletes tokens first.
apps/webapp/app/routes/api.v2.whoami.ts (3)
54-61
: Prefer findUnique for primary key lookupsMinor optimization/readability: using findUnique({ where: { id: userId } }) communicates intent and can be marginally more efficient.
Apply this diff:
- const user = await prisma.user.findFirst({ + const user = await prisma.user.findUnique({ select: { email: true, }, where: { id: userId, }, });
80-91
: Reduce PAT project lookup to a single queryYou can avoid the orgs prefetch by joining membership in the project query.
Apply this diff:
- const orgs = await prisma.organization.findMany({ - select: { id: true }, - where: { - members: { some: { userId } }, - }, - }); - - if (orgs.length === 0) { - return { success: true, result: userDetails }; - } - - const project = await prisma.project.findFirst({ + const project = await prisma.project.findFirst({ select: { externalRef: true, name: true, slug: true, organization: { select: { slug: true, title: true, }, }, }, where: { externalRef: projectRef, - organizationId: { - in: orgs.map((org) => org.id), - }, + organization: { + members: { + some: { userId }, + }, + }, }, });Also applies to: 100-118
142-200
: OAT path looks solid; keep placeholder email documentedThe synthetic identity avoids leaking user data and scopes project resolution to the org. Consider documenting the placeholder email contract for clients.
I can add endpoint docs clarifying identity semantics for OATs. Want me to draft that?
apps/webapp/app/services/apiAuth.server.ts (3)
318-347
: Strong union typing and conditional return narrowing — consider adding a runtime guardThe AuthenticationResult union and AllowedAuthenticationMethods/FilteredAuthenticationResult conditional types are solid. One improvement: the types enforce “at least one method is true” at compile time, but callers can still cast or pass a computed object. A small runtime assertion can prevent silent misconfiguration:
type AllowedAuthenticationMethods = Record<AuthenticationMethod, boolean> & ({ personalAccessToken: true } | { organizationAccessToken: true } | { apiKey: true }); +function assertAtLeastOneStrategyEnabled(config: AllowedAuthenticationMethods) { + if (!config.personalAccessToken && !config.organizationAccessToken && !config.apiKey) { + throw new Error("authenticateRequest: At least one authentication method must be enabled"); + } +}
394-401
: Remove chained “satisfies … as …” on returned literals to avoid TS parsing quirksThese returns are correct in shape, but the chained “satisfies Extract<…> as FilteredAuthenticationResult” can be fragile. A simple explicit cast is clearer and avoids parser ambiguities:
- return { - type: "personalAccessToken", - result, - } satisfies Extract< - AuthenticationResult, - { type: "personalAccessToken" } - > as FilteredAuthenticationResult<T>; + return { type: "personalAccessToken", result } as FilteredAuthenticationResult<T>;- return { - type: "organizationAccessToken", - result, - } satisfies Extract< - AuthenticationResult, - { type: "organizationAccessToken" } - > as FilteredAuthenticationResult<T>; + return { type: "organizationAccessToken", result } as FilteredAuthenticationResult<T>;- return { - type: "apiKey", - result, - } satisfies Extract< - AuthenticationResult, - { type: "apiKey" } - > as FilteredAuthenticationResult<T>; + return { type: "apiKey", result } as FilteredAuthenticationResult<T>;Also applies to: 410-417, 426-433
541-610
: Organization token environment resolution mirrors PAT branch — factor shared logic to reduce duplication and driftThe organizationAccessToken branch duplicates most of the PAT logic (project lookup by ref, environment lookup with/without branch, parent env apiKey reassignment). Extracting a shared helper (e.g., getEnvironmentForProjectAndSlug(projectId, slug, branch)) will:
- Reduce duplication.
- Ensure consistent error messages/behavior.
- Make future changes (e.g., archived checks) safer.
I can draft this helper in a follow-up if helpful.
apps/webapp/app/services/organizationAccessToken.server.ts (2)
18-43
: Query filters for validity look good; consider indexing and result orderingThe validity filter (revokedAt: null and not-expired) is correct. Ensure there’s an index on (organizationId, revokedAt, expiresAt) for efficient list queries. Also consider returning tokens ordered by lastAccessedAt desc or createdAt desc for a stable UI.
144-171
: Creation flow looks correct; nit: ensure obfuscation length matches token length policyobfuscateToken shows 4 leading/4 trailing chars and 18 bullets. With tokenValueLength=40, that reveals 8 of 40 chars, masking 32. 18 bullets are for display only; consider matching the number of bullets with the number of masked characters (e.g., 32) for consistency and to avoid leaking the token length policy via UI.
- const obfuscated = `${withoutPrefix.slice(0, 4)}${"•".repeat(18)}${withoutPrefix.slice(-4)}`; + const maskedCount = Math.max(0, withoutPrefix.length - 8); + const obfuscated = `${withoutPrefix.slice(0, 4)}${"•".repeat(maskedCount)}${withoutPrefix.slice(-4)}`;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (17)
apps/webapp/app/routes/api.v1.projects.$projectRef.$env.ts
(2 hunks)apps/webapp/app/routes/api.v1.projects.$projectRef.background-workers.$envSlug.$version.ts
(2 hunks)apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.$name.ts
(3 hunks)apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.import.ts
(2 hunks)apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.ts
(3 hunks)apps/webapp/app/routes/api.v2.whoami.ts
(1 hunks)apps/webapp/app/services/apiAuth.server.ts
(6 hunks)apps/webapp/app/services/organizationAccessToken.server.ts
(1 hunks)apps/webapp/app/services/personalAccessToken.server.ts
(1 hunks)apps/webapp/app/utils/tokens.ts
(1 hunks)internal-packages/database/prisma/migrations/20250813103207_add_organization_access_token/migration.sql
(1 hunks)internal-packages/database/prisma/schema.prisma
(2 hunks)packages/cli-v3/src/commands/deploy.ts
(1 hunks)packages/cli-v3/src/commands/login.ts
(7 hunks)packages/cli-v3/src/utilities/isOrganizationAccessToken.ts
(1 hunks)packages/cli-v3/src/utilities/isPersonalAccessToken.ts
(2 hunks)packages/cli-v3/src/utilities/session.ts
(4 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{ts,tsx}
: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
packages/cli-v3/src/commands/deploy.ts
packages/cli-v3/src/utilities/isOrganizationAccessToken.ts
apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.ts
apps/webapp/app/utils/tokens.ts
apps/webapp/app/routes/api.v2.whoami.ts
apps/webapp/app/routes/api.v1.projects.$projectRef.background-workers.$envSlug.$version.ts
apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.import.ts
packages/cli-v3/src/utilities/session.ts
packages/cli-v3/src/utilities/isPersonalAccessToken.ts
apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.$name.ts
apps/webapp/app/services/apiAuth.server.ts
apps/webapp/app/services/personalAccessToken.server.ts
apps/webapp/app/routes/api.v1.projects.$projectRef.$env.ts
packages/cli-v3/src/commands/login.ts
apps/webapp/app/services/organizationAccessToken.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.ts
apps/webapp/app/utils/tokens.ts
apps/webapp/app/routes/api.v2.whoami.ts
apps/webapp/app/routes/api.v1.projects.$projectRef.background-workers.$envSlug.$version.ts
apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.import.ts
apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.$name.ts
apps/webapp/app/services/apiAuth.server.ts
apps/webapp/app/services/personalAccessToken.server.ts
apps/webapp/app/routes/api.v1.projects.$projectRef.$env.ts
apps/webapp/app/services/organizationAccessToken.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}
: In the webapp, all environment variables must be accessed through theenv
export ofenv.server.ts
, instead of directly accessingprocess.env
.
When importing from@trigger.dev/core
in the webapp, never import from the root@trigger.dev/core
path; always use one of the subpath exports as defined in the package's package.json.
Files:
apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.ts
apps/webapp/app/utils/tokens.ts
apps/webapp/app/routes/api.v2.whoami.ts
apps/webapp/app/routes/api.v1.projects.$projectRef.background-workers.$envSlug.$version.ts
apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.import.ts
apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.$name.ts
apps/webapp/app/services/apiAuth.server.ts
apps/webapp/app/services/personalAccessToken.server.ts
apps/webapp/app/routes/api.v1.projects.$projectRef.$env.ts
apps/webapp/app/services/organizationAccessToken.server.ts
apps/webapp/app/services/**/*.server.ts
📄 CodeRabbit Inference Engine (.cursor/rules/webapp.mdc)
For testable services, separate service logic and configuration, as exemplified by
realtimeClient.server.ts
(service) andrealtimeClientGlobal.server.ts
(configuration).
Files:
apps/webapp/app/services/apiAuth.server.ts
apps/webapp/app/services/personalAccessToken.server.ts
apps/webapp/app/services/organizationAccessToken.server.ts
🧠 Learnings (1)
📚 Learning: 2025-07-18T17:49:24.468Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-18T17:49:24.468Z
Learning: Applies to {packages/core,apps/webapp}/**/*.{ts,tsx} : We use zod a lot in packages/core and in the webapp
Applied to files:
apps/webapp/app/routes/api.v1.projects.$projectRef.background-workers.$envSlug.$version.ts
🧬 Code Graph Analysis (11)
apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.ts (1)
apps/webapp/app/services/apiAuth.server.ts (1)
authenticateRequest
(372-436)
apps/webapp/app/utils/tokens.ts (1)
apps/webapp/app/services/secrets/secretStore.server.ts (1)
nonce
(183-189)
apps/webapp/app/routes/api.v2.whoami.ts (4)
apps/webapp/app/services/apiAuth.server.ts (1)
authenticateRequest
(372-436)packages/core/src/v3/apps/http.ts (1)
json
(65-75)packages/core/src/v3/schemas/api.ts (1)
WhoAmIResponse
(28-28)apps/webapp/app/utils/pathBuilder.ts (1)
v3ProjectPath
(140-142)
apps/webapp/app/routes/api.v1.projects.$projectRef.background-workers.$envSlug.$version.ts (1)
apps/webapp/app/services/apiAuth.server.ts (1)
authenticateRequest
(372-436)
apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.import.ts (1)
apps/webapp/app/services/apiAuth.server.ts (1)
authenticateRequest
(372-436)
packages/cli-v3/src/utilities/isPersonalAccessToken.ts (2)
apps/webapp/app/services/personalAccessToken.server.ts (1)
isPersonalAccessToken
(172-174)apps/webapp/app/services/organizationAccessToken.server.ts (1)
isOrganizationAccessToken
(140-142)
apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.$name.ts (1)
apps/webapp/app/services/apiAuth.server.ts (1)
authenticateRequest
(372-436)
apps/webapp/app/services/apiAuth.server.ts (2)
apps/webapp/app/services/personalAccessToken.server.ts (2)
PersonalAccessTokenAuthenticationResult
(92-94)isPersonalAccessToken
(172-174)apps/webapp/app/services/organizationAccessToken.server.ts (2)
OrganizationAccessTokenAuthenticationResult
(60-62)authenticateApiRequestWithOrganizationAccessToken
(72-81)
apps/webapp/app/routes/api.v1.projects.$projectRef.$env.ts (2)
apps/webapp/app/services/apiAuth.server.ts (2)
authenticateRequest
(372-436)authenticatedEnvironmentForAuthentication
(438-616)packages/core/src/v3/schemas/api.ts (2)
GetProjectEnvResponse
(50-55)GetProjectEnvResponse
(57-57)
packages/cli-v3/src/commands/login.ts (1)
packages/cli-v3/src/utilities/isPersonalAccessToken.ts (2)
validateAccessToken
(12-24)NotAccessTokenError
(33-38)
apps/webapp/app/services/organizationAccessToken.server.ts (3)
apps/webapp/app/services/secrets/secretStore.server.ts (1)
EncryptedSecretValueSchema
(56-60)apps/webapp/app/utils/tokens.ts (3)
hashToken
(35-39)encryptToken
(4-18)decryptToken
(20-33)packages/cli-v3/src/utilities/isPersonalAccessToken.ts (1)
isOrganizationAccessToken
(8-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (11)
packages/cli-v3/src/commands/deploy.ts (1)
309-315
: userId is optional for initializeDeployment — no changes requiredInitializeDeploymentRequestBody declares userId as optional and the server only uses it when present; omitting userId for org tokens results in no triggeredById (which is nullable) — this matches the CLI change to send userId only for personal tokens.
Files checked:
- packages/core/src/v3/schemas/api.ts — InitializeDeploymentRequestBody: userId: z.string().optional()
- apps/webapp/app/v3/services/initializeDeployment.server.ts — conditional lookup of payload.userId and setting triggeredById: triggeredBy?.id when creating WorkerDeployment
- internal-packages/database/prisma/schema.prisma — WorkerDeployment.triggeredById is nullable
- apps/webapp/app/routes/api.v1.deployments.ts — body validated with InitializeDeploymentRequestBody
- packages/cli-v3/src/commands/deploy.ts — CLI sends userId only for personal tokens
Tag:
internal-packages/database/prisma/schema.prisma (1)
204-205
: Relationship on Organization is correct and consistent.The one-to-many link organizationAccessTokens OrganizationAccessToken[] is appropriate and matches the new model.
packages/cli-v3/src/commands/login.ts (2)
199-201
: Propagating tokenType for stored and newly created tokens looks good.Explicitly marking interactive/stored tokens as "personal" is consistent with the current login flow that only generates PATs.
Also applies to: 221-223, 323-326
121-133
: LoginResult.auth already includes tokenType — no change requiredConfirmed by repo search:
- packages/cli-v3/src/utilities/session.ts — LoginResultOk / LoginResult define auth.tokenType = "personal" | "organization".
- packages/cli-v3/src/commands/deploy.ts — consumer reads authorization.auth.tokenType (uses it when building deploy payload).
The added tokenType in packages/cli-v3/src/commands/login.ts matches the existing type and consumers.
apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.import.ts (1)
6-9
: Centralized auth adoption is correct; route now supports OATs consistently.Switching to authenticateRequest aligns with the new multi-credential auth flow and keeps the 401 handling intact. Passing the auth result into authenticatedEnvironmentForAuthentication with branchNameFromRequest ensures correct branch scoping.
Also applies to: 24-28
apps/webapp/app/routes/api.v1.projects.$projectRef.$env.ts (1)
38-43
: LGTM: response fields align with GetProjectEnvResponse and env access via env.server.ts.Using env from env.server.ts satisfies our webapp env policy; returning apiKey/name/apiUrl/projectId matches the schema.
apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.$name.ts (1)
6-8
: Good move: centralize auth via authenticateRequestSwitching to the unified authenticateRequest keeps this route aligned with the new token model (PAT/OAT/API key) and reduces duplication.
apps/webapp/app/services/personalAccessToken.server.ts (1)
268-277
: LGTM: centralized token crypto usageCreating encryptedToken and hashedToken via shared utils reduces duplication and risk of drift across token types.
apps/webapp/app/routes/api.v2.whoami.ts (1)
9-14
: LGTM: limit auth to PAT and OAT for whoamiExplicitly disabling apiKey here is correct and aligns with the endpoint’s semantics.
apps/webapp/app/services/apiAuth.server.ts (1)
20-24
: Imports for organization access tokens look correct and scopedThe new imports are minimal and only pull in what’s needed (type, authenticate, type guard). No concerns here.
apps/webapp/app/services/organizationAccessToken.server.ts (1)
140-143
: Token type guard is concise and aligns with the chosen prefixLGTM. Keeping this colocated with other OAT utilities is appropriate.
apps/webapp/app/routes/api.v1.projects.$projectRef.background-workers.$envSlug.$version.ts
Show resolved
Hide resolved
} satisfies Extract< | ||
AuthenticationResult, | ||
{ type: "personalAccessToken" } | ||
> as FilteredAuthenticationResult<T>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't find a way to avoid type assertions here. I'd still keep the generics though, it's useful to narrow the return type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
packages/cli-v3/src/commands/login.ts (1)
235-241
: CI guidance should mention Organization tokens tooSince OATs are supported for CLI/CI, update the guidance to avoid blocking users who use OATs.
- throw new Error( - `Authentication required in CI environment. Please set the TRIGGER_ACCESS_TOKEN environment variable with a Personal Access Token. + throw new Error( + `Authentication required in CI environment. Please set the TRIGGER_ACCESS_TOKEN environment variable with a Trigger Access Token (Personal or Organization). -You can generate one here: ${dashboardUrl}/account/tokens +You can generate one here: ${dashboardUrl}/account/tokens -For more information, see: ${links.docs.gitHubActions.personalAccessToken}` +For more information, see: ${links.docs.gitHubActions.personalAccessToken}` );Optionally, add a docs link for organization tokens when available.
apps/webapp/app/services/personalAccessToken.server.ts (1)
239-243
: Bug: property name mismatch (obfuscatedToken vs obfuscateToken)Elsewhere you consistently use obfuscatedToken. Returning obfuscateToken here will break consumers that expect obfuscatedToken (e.g., the CLI polling path expects token.obfuscatedToken).
- obfuscateToken: existingCliPersonalAccessToken.obfuscatedToken, + obfuscatedToken: existingCliPersonalAccessToken.obfuscatedToken,Please verify any UI/API handlers that consume this object.
♻️ Duplicate comments (1)
packages/cli-v3/src/commands/login.ts (1)
98-105
: Message and comment are out-of-date with OAT supportThe comment implies OATs are “internal,” and the error message mentions only PATs. Per this PR’s objectives and current behavior, OATs are valid for CLI via TRIGGER_ACCESS_TOKEN. Update both to reflect that we accept PATs and OATs and only reject non-access tokens.
- if (!validationResult.success) { - // We deliberately don't surface the existence of organization access tokens to the user for now, as they're only used internally. - // Once we expose them in the application, we should also communicate that option here. - throw new NotAccessTokenError( - "Your TRIGGER_ACCESS_TOKEN is not a Personal Access Token, they start with 'tr_pat_'. You can generate one here: https://cloud.trigger.dev/account/tokens" - ); - } + if (!validationResult.success) { + throw new NotAccessTokenError( + "Your TRIGGER_ACCESS_TOKEN is not a valid Trigger Access Token. It should start with 'tr_pat_' (Personal) or 'tr_oat_' (Organization). You can generate one here: https://cloud.trigger.dev/account/tokens" + ); + }
🧹 Nitpick comments (6)
apps/webapp/app/env.server.ts (1)
26-31
: Good tightening: enforce exact 32-byte key; minor clarity nitThe runtime check is correct for AES-256-GCM keys. Consider clarifying the error to “32 UTF-8 bytes” to avoid confusion with “32 characters,” which may not equal 32 bytes for non-ASCII input.
- "ENCRYPTION_KEY must be exactly 32 bytes" + "ENCRYPTION_KEY must be exactly 32 UTF-8 bytes"packages/cli-v3/src/utilities/accessTokens.ts (1)
26-38
: Custom errors are fine; consider adding minimal testsThe error classes are straightforward. Recommend adding unit tests covering:
- PAT, OAT, and invalid token inputs to validateAccessToken
- Error messaging for non-access tokens
packages/cli-v3/src/commands/login.ts (2)
32-35
: Unused import guard seems unnecessaryYou import NotPersonalAccessTokenError but never throw it in this file. Unless thrown upstream, the catch branch for this error is effectively dead code.
-import { - validateAccessToken, - NotPersonalAccessTokenError, - NotAccessTokenError, -} from "../utilities/accessTokens.js"; +import { validateAccessToken, NotAccessTokenError } from "../utilities/accessTokens.js";If another module throws NotPersonalAccessTokenError in this flow, keep the import and add a short comment to justify it.
281-285
: Config write: prefer resolved apiUrl for consistencyYou resolve apiUrl earlier (env.TRIGGER_API_URL ?? opts.defaultApiUrl ?? CLOUD_API_URL). Consider persisting that resolved value so subsequent commands read exactly what was used during login.
- writeAuthConfigProfile( - { - accessToken: indexResult.token, - apiUrl: opts.defaultApiUrl, - }, - options?.profile - ); + const resolvedApiUrl = authConfig?.apiUrl ?? opts.defaultApiUrl; + writeAuthConfigProfile({ accessToken: indexResult.token, apiUrl: resolvedApiUrl }, options?.profile);apps/webapp/app/utils/tokens.server.ts (1)
30-34
: Minor: be explicit about encoding in hash.updateExplicitly pass “utf8” to avoid ambiguity.
- hash.update(token); + hash.update(token, "utf8");apps/webapp/app/services/personalAccessToken.server.ts (1)
161-166
: Use constant-time comparison to avoid timing side-channelsComparing secrets with !== leaks timing. Use timingSafeEqual on Buffers.
- if (decryptedToken !== token) { + // Use constant-time comparison + const a = Buffer.from(decryptedToken, "utf8"); + const b = Buffer.from(token, "utf8"); + const matches = a.length === b.length && (await import("node:crypto")).then(({ timingSafeEqual }) => timingSafeEqual(a, b)); + if (!matches) { logger.error( `PersonalAccessToken with id: ${personalAccessToken.id} was found in the database with hash ${hashedToken}, but the decrypted token did not match the provided token.` ); return; }If you prefer static imports, import timingSafeEqual at the top instead of dynamic import.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (7)
apps/webapp/app/env.server.ts
(1 hunks)apps/webapp/app/services/apiAuth.server.ts
(6 hunks)apps/webapp/app/services/organizationAccessToken.server.ts
(1 hunks)apps/webapp/app/services/personalAccessToken.server.ts
(3 hunks)apps/webapp/app/utils/tokens.server.ts
(1 hunks)packages/cli-v3/src/commands/login.ts
(7 hunks)packages/cli-v3/src/utilities/accessTokens.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/webapp/app/services/organizationAccessToken.server.ts
- apps/webapp/app/services/apiAuth.server.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{ts,tsx}
: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
packages/cli-v3/src/utilities/accessTokens.ts
apps/webapp/app/utils/tokens.server.ts
apps/webapp/app/env.server.ts
apps/webapp/app/services/personalAccessToken.server.ts
packages/cli-v3/src/commands/login.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
apps/webapp/app/utils/tokens.server.ts
apps/webapp/app/env.server.ts
apps/webapp/app/services/personalAccessToken.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}
: In the webapp, all environment variables must be accessed through theenv
export ofenv.server.ts
, instead of directly accessingprocess.env
.
When importing from@trigger.dev/core
in the webapp, never import from the root@trigger.dev/core
path; always use one of the subpath exports as defined in the package's package.json.
Files:
apps/webapp/app/utils/tokens.server.ts
apps/webapp/app/env.server.ts
apps/webapp/app/services/personalAccessToken.server.ts
apps/webapp/app/services/**/*.server.ts
📄 CodeRabbit Inference Engine (.cursor/rules/webapp.mdc)
For testable services, separate service logic and configuration, as exemplified by
realtimeClient.server.ts
(service) andrealtimeClientGlobal.server.ts
(configuration).
Files:
apps/webapp/app/services/personalAccessToken.server.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2391
File: packages/cli-v3/src/commands/login.ts:99-0
Timestamp: 2025-08-14T10:35:38.306Z
Learning: Organization access tokens (OATs) can be used for CLI deployments via TRIGGER_ACCESS_TOKEN environment variable, functioning similarly to personal access tokens (PATs) for this use case.
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2391
File: packages/cli-v3/src/commands/login.ts:99-0
Timestamp: 2025-08-14T10:35:38.306Z
Learning: In the whoami v2 endpoint, organization access tokens (OATs) return the same schema structure as personal access tokens (PATs), so existing CLI flows that expect userId and email fields work correctly with both token types.
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2391
File: apps/webapp/app/services/organizationAccessToken.server.ts:50-0
Timestamp: 2025-08-14T10:53:54.505Z
Learning: In the Trigger.dev codebase, token service functions (like revokePersonalAccessToken and revokeOrganizationAccessToken) don't include tenant scoping in their database queries. Instead, authorization and tenant scoping happens at a higher level in the authentication flow (typically in route handlers) before these service functions are called. This is a consistent pattern across both Personal Access Tokens (PATs) and Organization Access Tokens (OATs).
📚 Learning: 2025-07-18T17:49:47.180Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-07-18T17:49:47.180Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : In the webapp, all environment variables must be accessed through the `env` export of `env.server.ts`, instead of directly accessing `process.env`.
Applied to files:
apps/webapp/app/env.server.ts
📚 Learning: 2025-08-14T10:35:38.306Z
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2391
File: packages/cli-v3/src/commands/login.ts:99-0
Timestamp: 2025-08-14T10:35:38.306Z
Learning: In the whoami v2 endpoint, organization access tokens (OATs) return the same schema structure as personal access tokens (PATs), so existing CLI flows that expect userId and email fields work correctly with both token types.
Applied to files:
packages/cli-v3/src/commands/login.ts
📚 Learning: 2025-08-14T10:53:54.505Z
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2391
File: apps/webapp/app/services/organizationAccessToken.server.ts:50-0
Timestamp: 2025-08-14T10:53:54.505Z
Learning: In the Trigger.dev codebase, token service functions (like revokePersonalAccessToken and revokeOrganizationAccessToken) don't include tenant scoping in their database queries. Instead, authorization and tenant scoping happens at a higher level in the authentication flow (typically in route handlers) before these service functions are called. This is a consistent pattern across both Personal Access Tokens (PATs) and Organization Access Tokens (OATs).
Applied to files:
packages/cli-v3/src/commands/login.ts
📚 Learning: 2025-08-14T10:35:38.306Z
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2391
File: packages/cli-v3/src/commands/login.ts:99-0
Timestamp: 2025-08-14T10:35:38.306Z
Learning: Organization access tokens (OATs) can be used for CLI deployments via TRIGGER_ACCESS_TOKEN environment variable, functioning similarly to personal access tokens (PATs) for this use case.
Applied to files:
packages/cli-v3/src/commands/login.ts
🧬 Code Graph Analysis (4)
packages/cli-v3/src/utilities/accessTokens.ts (2)
apps/webapp/app/services/personalAccessToken.server.ts (1)
isPersonalAccessToken
(173-175)apps/webapp/app/services/organizationAccessToken.server.ts (1)
isOrganizationAccessToken
(142-144)
apps/webapp/app/utils/tokens.server.ts (1)
apps/webapp/app/services/secrets/secretStore.server.ts (1)
nonce
(183-189)
apps/webapp/app/services/personalAccessToken.server.ts (1)
apps/webapp/app/utils/tokens.server.ts (1)
encryptToken
(3-17)
packages/cli-v3/src/commands/login.ts (1)
packages/cli-v3/src/utilities/accessTokens.ts (2)
validateAccessToken
(12-24)NotAccessTokenError
(33-38)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
packages/cli-v3/src/utilities/accessTokens.ts (1)
12-24
: LGTM: simple, deterministic token-type detectionThe discriminated union and prefix-based detection are clear and sufficient for CLI purposes.
apps/webapp/app/services/personalAccessToken.server.ts (3)
6-8
: LGTM: centralizing crypto and using env is consistent with guidelinesImporting from ~/utils/tokens.server and using env.ENCRYPTION_KEY aligns with the webapp convention.
269-270
: LGTM: encrypt with centralized utility and validated keyThe call is correct and uses the validated 32-byte key.
314-319
: LGTM: decrypt via shared utilityThe decryption path matches the encrypted shape and uses the same validated key.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (3)
.github/workflows/unit-tests-webapp.yml
(1 hunks)docker/dev-compose.yml
(1 hunks)docker/services-compose.yml
(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2391
File: packages/cli-v3/src/commands/login.ts:99-0
Timestamp: 2025-08-14T10:35:38.306Z
Learning: Organization access tokens (OATs) can be used for CLI deployments via TRIGGER_ACCESS_TOKEN environment variable, functioning similarly to personal access tokens (PATs) for this use case.
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2391
File: packages/cli-v3/src/commands/login.ts:99-0
Timestamp: 2025-08-14T10:35:38.306Z
Learning: In the whoami v2 endpoint, organization access tokens (OATs) return the same schema structure as personal access tokens (PATs), so existing CLI flows that expect userId and email fields work correctly with both token types.
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2391
File: apps/webapp/app/services/organizationAccessToken.server.ts:50-0
Timestamp: 2025-08-14T10:53:54.505Z
Learning: In the Trigger.dev codebase, token service functions (like revokePersonalAccessToken and revokeOrganizationAccessToken) don't include tenant scoping in their database queries. Instead, authorization and tenant scoping happens at a higher level in the authentication flow (typically in route handlers) before these service functions are called. This is a consistent pattern across both Personal Access Tokens (PATs) and Organization Access Tokens (OATs).
🪛 Gitleaks (8.27.2)
docker/services-compose.yml
40-40: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
59-59: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
.github/workflows/unit-tests-webapp.yml
88-88: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
docker/dev-compose.yml
114-114: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
internal-packages/database/prisma/migrations/20250814124843_add_organization_access_tokens/migration.sql (1)
23-23
: Stray literal "23" will break the migration — remove it.There’s an unexpected
23
on its own line which will cause a SQL syntax error when applying the migration.Apply this diff to remove the stray line:
ALTER TABLE "OrganizationAccessToken" ADD CONSTRAINT "OrganizationAccessToken_organizationId_fkey" FOREIGN KEY ("organizationId") REFERENCES "Organization"("id") ON DELETE RESTRICT ON UPDATE CASCADE; -23 +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (2)
internal-packages/database/prisma/migrations/20250814124843_add_organization_access_tokens/migration.sql
(1 hunks)internal-packages/database/prisma/schema.prisma
(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2391
File: packages/cli-v3/src/commands/login.ts:99-0
Timestamp: 2025-08-14T10:35:38.306Z
Learning: Organization access tokens (OATs) can be used for CLI deployments via TRIGGER_ACCESS_TOKEN environment variable, functioning similarly to personal access tokens (PATs) for this use case.
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2391
File: packages/cli-v3/src/commands/login.ts:99-0
Timestamp: 2025-08-14T10:35:38.306Z
Learning: In the whoami v2 endpoint, organization access tokens (OATs) return the same schema structure as personal access tokens (PATs), so existing CLI flows that expect userId and email fields work correctly with both token types.
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2391
File: apps/webapp/app/services/organizationAccessToken.server.ts:50-0
Timestamp: 2025-08-14T10:53:54.505Z
Learning: In the Trigger.dev codebase, token service functions (like revokePersonalAccessToken and revokeOrganizationAccessToken) don't include tenant scoping in their database queries. Instead, authorization and tenant scoping happens at a higher level in the authentication flow (typically in route handlers) before these service functions are called. This is a consistent pattern across both Personal Access Tokens (PATs) and Organization Access Tokens (OATs).
📚 Learning: 2025-08-14T10:35:38.306Z
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2391
File: packages/cli-v3/src/commands/login.ts:99-0
Timestamp: 2025-08-14T10:35:38.306Z
Learning: In the whoami v2 endpoint, organization access tokens (OATs) return the same schema structure as personal access tokens (PATs), so existing CLI flows that expect userId and email fields work correctly with both token types.
Applied to files:
internal-packages/database/prisma/schema.prisma
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
internal-packages/database/prisma/migrations/20250814124843_add_organization_access_tokens/migration.sql (1)
14-15
: Confirm updatedAt is set by the ORM on insert/update.In SQL, updatedAt is NOT NULL without a DB default; Prisma’s @updatedat should set it on both create and update. If any code writes to this table outside Prisma, consider a DB-level default/trigger.
If you need a DB-side default, we can add
DEFAULT CURRENT_TIMESTAMP
here and a simple trigger to auto-update on UPDATEs.internal-packages/database/prisma/schema.prisma (2)
136-139
: Enum addition looks correct and future-proof.
212-213
: Relation wiring into Organization looks good and consistent.Keeping onDelete behavior aligned with PersonalAccessToken (RESTRICT) is fine given Organization is soft-deleted via deletedAt.
...kages/database/prisma/migrations/20250814124843_add_organization_access_tokens/migration.sql
Outdated
Show resolved
Hide resolved
Co-authored-by: Matt Aitken <[email protected]>
…he DB at all It is a safer approach. Also we do not need to ever read the decrypted token value after creation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
internal-packages/database/prisma/schema.prisma (1)
136-166
: Model looks correct; consider adding two more indexes to match access patternsThe model maps cleanly to the migration and intended usage. To further optimize org-scoped queries and “valid tokens” filtering, add:
- @@index([organizationId]) for generic org listings
- @@index([organizationId, revokedAt, expiresAt]) to accelerate validity filters (Prisma can’t do partial indexes)
Apply this diff inside OrganizationAccessToken:
model OrganizationAccessToken { id String @id @default(cuid()) @@ createdAt DateTime @default(now()) updatedAt DateTime @updatedAt - @@index([organizationId, createdAt(sort: Desc)]) + @@index([organizationId, createdAt(sort: Desc)]) + @@index([organizationId]) + @@index([organizationId, revokedAt, expiresAt]) }
🧹 Nitpick comments (1)
apps/webapp/app/services/organizationAccessToken.server.ts (1)
17-31
: Order the listing to leverage the createdAt DESC index and ensure stable outputAdding an explicit orderBy improves determinism and uses the existing compound index.
const organizationAccessTokens = await prisma.organizationAccessToken.findMany({ select: { id: true, name: true, createdAt: true, lastAccessedAt: true, expiresAt: true, }, where: { organizationId, revokedAt: null, OR: [{ expiresAt: null }, { expiresAt: { gte: new Date() } }], }, + orderBy: { createdAt: "desc" }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/webapp/app/services/organizationAccessToken.server.ts
(1 hunks)internal-packages/database/prisma/migrations/20250814144649_add_organization_access_tokens/migration.sql
(1 hunks)internal-packages/database/prisma/schema.prisma
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{ts,tsx}
: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
apps/webapp/app/services/organizationAccessToken.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
apps/webapp/app/services/organizationAccessToken.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}
: In the webapp, all environment variables must be accessed through theenv
export ofenv.server.ts
, instead of directly accessingprocess.env
.
When importing from@trigger.dev/core
in the webapp, never import from the root@trigger.dev/core
path; always use one of the subpath exports as defined in the package's package.json.
Files:
apps/webapp/app/services/organizationAccessToken.server.ts
apps/webapp/app/services/**/*.server.ts
📄 CodeRabbit Inference Engine (.cursor/rules/webapp.mdc)
For testable services, separate service logic and configuration, as exemplified by
realtimeClient.server.ts
(service) andrealtimeClientGlobal.server.ts
(configuration).
Files:
apps/webapp/app/services/organizationAccessToken.server.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2391
File: packages/cli-v3/src/commands/login.ts:99-0
Timestamp: 2025-08-14T10:35:38.306Z
Learning: Organization access tokens (OATs) can be used for CLI deployments via TRIGGER_ACCESS_TOKEN environment variable, functioning similarly to personal access tokens (PATs) for this use case.
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2391
File: packages/cli-v3/src/commands/login.ts:99-0
Timestamp: 2025-08-14T10:35:38.306Z
Learning: In the whoami v2 endpoint, organization access tokens (OATs) return the same schema structure as personal access tokens (PATs), so existing CLI flows that expect userId and email fields work correctly with both token types.
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2391
File: apps/webapp/app/services/organizationAccessToken.server.ts:50-0
Timestamp: 2025-08-14T10:53:54.505Z
Learning: In the Trigger.dev codebase, token service functions (like revokePersonalAccessToken and revokeOrganizationAccessToken) don't include tenant scoping in their database queries. Instead, authorization and tenant scoping happens at a higher level in the authentication flow (typically in route handlers) before these service functions are called. This is a consistent pattern across both Personal Access Tokens (PATs) and Organization Access Tokens (OATs).
📚 Learning: 2025-08-14T10:35:38.306Z
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2391
File: packages/cli-v3/src/commands/login.ts:99-0
Timestamp: 2025-08-14T10:35:38.306Z
Learning: In the whoami v2 endpoint, organization access tokens (OATs) return the same schema structure as personal access tokens (PATs), so existing CLI flows that expect userId and email fields work correctly with both token types.
Applied to files:
internal-packages/database/prisma/schema.prisma
apps/webapp/app/services/organizationAccessToken.server.ts
📚 Learning: 2025-08-14T10:53:54.505Z
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2391
File: apps/webapp/app/services/organizationAccessToken.server.ts:50-0
Timestamp: 2025-08-14T10:53:54.505Z
Learning: In the Trigger.dev codebase, token service functions (like revokePersonalAccessToken and revokeOrganizationAccessToken) don't include tenant scoping in their database queries. Instead, authorization and tenant scoping happens at a higher level in the authentication flow (typically in route handlers) before these service functions are called. This is a consistent pattern across both Personal Access Tokens (PATs) and Organization Access Tokens (OATs).
Applied to files:
apps/webapp/app/services/organizationAccessToken.server.ts
📚 Learning: 2025-08-14T10:35:38.306Z
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2391
File: packages/cli-v3/src/commands/login.ts:99-0
Timestamp: 2025-08-14T10:35:38.306Z
Learning: Organization access tokens (OATs) can be used for CLI deployments via TRIGGER_ACCESS_TOKEN environment variable, functioning similarly to personal access tokens (PATs) for this use case.
Applied to files:
apps/webapp/app/services/organizationAccessToken.server.ts
🧬 Code Graph Analysis (1)
apps/webapp/app/services/organizationAccessToken.server.ts (2)
apps/webapp/app/utils/tokens.server.ts (1)
hashToken
(30-34)apps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts (1)
createToken
(40-54)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
internal-packages/database/prisma/migrations/20250814144649_add_organization_access_tokens/migration.sql (1)
1-22
: Solid migration: correct constraints and indexes for OATs
- Enum + table shape aligns with the Prisma model.
- Unique index on hashedToken is appropriate for constant-time lookup.
- The compound index on (organizationId, createdAt DESC) supports org-scoped listings/pagination.
No blocking issues from a DB perspective.
internal-packages/database/prisma/schema.prisma (1)
208-209
: Relation wiring LGTMAdding organizationAccessTokens to Organization completes the one-to-many relation and matches the service usage.
apps/webapp/app/services/organizationAccessToken.server.ts (1)
87-121
: Auth flow is minimal and secure (hash-only, no plaintext storage) — good
- Prefix check limits accidental cross-type usage.
- Constant-time safe hash comparison via DB equality avoids timing issues at app level.
- Updating lastAccessedAt post-auth is helpful for auditing.
No changes requested here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/webapp/app/services/organizationAccessToken.server.ts (1)
46-55
: Use updateMany and filter on revokedAt to avoid invalid WhereUniqueInput and make revocation idempotentThis keeps tenant scoping handled upstream (per established pattern), prevents re-revoking already revoked tokens, and avoids Prisma’s WhereUniqueInput constraint if you later add non-unique fields to the filter.
Apply this diff:
export async function revokeOrganizationAccessToken(tokenId: string) { - await prisma.organizationAccessToken.update({ - where: { - id: tokenId, - }, - data: { - revokedAt: new Date(), - }, - }); + await prisma.organizationAccessToken.updateMany({ + where: { + id: tokenId, + revokedAt: null, + }, + data: { + revokedAt: new Date(), + }, + }); }Optionally, return a boolean to signal whether a row was updated:
export async function revokeOrganizationAccessToken(tokenId: string): Promise<boolean> { const res = await prisma.organizationAccessToken.updateMany({ where: { id: tokenId, revokedAt: null }, data: { revokedAt: new Date() }, }); return res.count > 0; }
🧹 Nitpick comments (4)
apps/webapp/app/services/organizationAccessToken.server.ts (4)
94-115
: Minimize data fetched and use a single timestamp for consistency
- Only select the fields you need to reduce payload and parsing.
- Use a single now value to avoid micro-drifts between the read and the write.
const hashedToken = hashToken(token); - const organizationAccessToken = await prisma.organizationAccessToken.findFirst({ - where: { - hashedToken, - revokedAt: null, - OR: [{ expiresAt: null }, { expiresAt: { gte: new Date() } }], - }, - }); + const now = new Date(); + const organizationAccessToken = await prisma.organizationAccessToken.findFirst({ + select: { + id: true, + organizationId: true, + }, + where: { + hashedToken, + revokedAt: null, + OR: [{ expiresAt: null }, { expiresAt: { gte: now } }], + }, + }); if (!organizationAccessToken) { return; } await prisma.organizationAccessToken.update({ where: { id: organizationAccessToken.id, }, data: { - lastAccessedAt: new Date(), + lastAccessedAt: now, }, });Optional: make the lastAccessedAt write best-effort (don’t block auth result) and guard errors to avoid noisy failures:
void prisma.organizationAccessToken .update({ where: { id: organizationAccessToken.id }, data: { lastAccessedAt: now } }) .catch((e) => logger.debug("Failed to update OAT lastAccessedAt", { tokenId: organizationAccessToken.id, error: e }));
17-31
: Optional: add a deterministic ordering for token listingsFor a stable UI and API, consider ordering results (e.g., newest first).
const organizationAccessTokens = await prisma.organizationAccessToken.findMany({ + orderBy: { createdAt: "desc" }, select: { id: true, name: true, createdAt: true, lastAccessedAt: true, expiresAt: true, }, where: { organizationId, revokedAt: null, OR: [{ expiresAt: null }, { expiresAt: { gte: new Date() } }], }, });
86-93
: Reduce log noise for prefix mismatchPrefix mismatches are often user error. Consider downgrading to debug to keep warn/error logs focused on actionable issues.
- logger.warn(`OAT doesn't start with ${tokenPrefix}`); + logger.debug(`OAT doesn't start with ${tokenPrefix}`);
1-160
: Add tests for the OAT service behaviorsRecommend unit/integration tests for:
- Creation: returns plaintext once, stores only hash
- Authentication: valid, revoked, expired, wrong-prefix cases
- lastAccessedAt update side effect
- Revocation: idempotency when already revoked
I can scaffold a test suite exercising these cases against a test database. Want me to generate it?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/webapp/app/services/organizationAccessToken.server.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{ts,tsx}
: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
apps/webapp/app/services/organizationAccessToken.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
apps/webapp/app/services/organizationAccessToken.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}
: In the webapp, all environment variables must be accessed through theenv
export ofenv.server.ts
, instead of directly accessingprocess.env
.
When importing from@trigger.dev/core
in the webapp, never import from the root@trigger.dev/core
path; always use one of the subpath exports as defined in the package's package.json.
Files:
apps/webapp/app/services/organizationAccessToken.server.ts
apps/webapp/app/services/**/*.server.ts
📄 CodeRabbit Inference Engine (.cursor/rules/webapp.mdc)
For testable services, separate service logic and configuration, as exemplified by
realtimeClient.server.ts
(service) andrealtimeClientGlobal.server.ts
(configuration).
Files:
apps/webapp/app/services/organizationAccessToken.server.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2391
File: packages/cli-v3/src/commands/login.ts:99-0
Timestamp: 2025-08-14T10:35:38.306Z
Learning: Organization access tokens (OATs) can be used for CLI deployments via TRIGGER_ACCESS_TOKEN environment variable, functioning similarly to personal access tokens (PATs) for this use case.
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2391
File: packages/cli-v3/src/commands/login.ts:99-0
Timestamp: 2025-08-14T10:35:38.306Z
Learning: In the whoami v2 endpoint, organization access tokens (OATs) return the same schema structure as personal access tokens (PATs), so existing CLI flows that expect userId and email fields work correctly with both token types.
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2391
File: apps/webapp/app/services/organizationAccessToken.server.ts:50-0
Timestamp: 2025-08-14T10:53:54.505Z
Learning: In the Trigger.dev codebase, token service functions (like revokePersonalAccessToken and revokeOrganizationAccessToken) don't include tenant scoping in their database queries. Instead, authorization and tenant scoping happens at a higher level in the authentication flow (typically in route handlers) before these service functions are called. This is a consistent pattern across both Personal Access Tokens (PATs) and Organization Access Tokens (OATs).
📚 Learning: 2025-08-14T10:53:54.505Z
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2391
File: apps/webapp/app/services/organizationAccessToken.server.ts:50-0
Timestamp: 2025-08-14T10:53:54.505Z
Learning: In the Trigger.dev codebase, token service functions (like revokePersonalAccessToken and revokeOrganizationAccessToken) don't include tenant scoping in their database queries. Instead, authorization and tenant scoping happens at a higher level in the authentication flow (typically in route handlers) before these service functions are called. This is a consistent pattern across both Personal Access Tokens (PATs) and Organization Access Tokens (OATs).
Applied to files:
apps/webapp/app/services/organizationAccessToken.server.ts
📚 Learning: 2025-08-14T10:35:38.306Z
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2391
File: packages/cli-v3/src/commands/login.ts:99-0
Timestamp: 2025-08-14T10:35:38.306Z
Learning: In the whoami v2 endpoint, organization access tokens (OATs) return the same schema structure as personal access tokens (PATs), so existing CLI flows that expect userId and email fields work correctly with both token types.
Applied to files:
apps/webapp/app/services/organizationAccessToken.server.ts
📚 Learning: 2025-08-14T10:35:38.306Z
Learnt from: myftija
PR: triggerdotdev/trigger.dev#2391
File: packages/cli-v3/src/commands/login.ts:99-0
Timestamp: 2025-08-14T10:35:38.306Z
Learning: Organization access tokens (OATs) can be used for CLI deployments via TRIGGER_ACCESS_TOKEN environment variable, functioning similarly to personal access tokens (PATs) for this use case.
Applied to files:
apps/webapp/app/services/organizationAccessToken.server.ts
🧬 Code Graph Analysis (1)
apps/webapp/app/services/organizationAccessToken.server.ts (2)
apps/webapp/app/utils/tokens.server.ts (1)
hashToken
(30-34)apps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts (1)
createToken
(40-54)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
apps/webapp/app/services/organizationAccessToken.server.ts (4)
61-62
: Strict Bearer header parsing is OK for consistency with PATsKeeping the exact-case “Bearer ” requirement matches the PAT flow and ensures predictable clients. No action needed.
33-40
: LGTM on non-sensitive OAT listing shapeOnly returning id, name, createdAt, lastAccessedAt, and expiresAt is a sound “obfuscated” shape. No sensitive data is exposed.
122-124
: LGTM: isOrganizationAccessTokenSimple, fast prefix check that matches the create path.
1-10
: Token generation looks good40-char payload with a curated alphabet and a fixed prefix is a solid choice for UX and entropy.
This PR introduces organization access tokens (OATs). They work similarly to
the personal access tokens (PATs), with a couple of differences:
The main usecase for OATs is using the Trigger CLI in automation with short-lived tokens,
e.g., in the CI. This way one would no longer need to use a specific user's PAT to authenticate.
This PR does not expose OATs in the application, they are currently only used
internally for the build server. We will eventually also enable creating them
in the application and also introduce token scopes.