From 5ba32574bd67d88029d267f32d878e2c8fcb1e84 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 28 Aug 2026 14:31:46 -0700 Subject: [PATCH 1/5] feat(security): encrypt account OAuth tokens at rest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Better Auth `account` table stored `access_token`, `refresh_token` and `id_token` in plaintext. It is the credential store for every user-connected integration, so a dump of it was a dump of our customers' third-party data — the last plaintext credential store in the repo, and the largest. Tokens are now stored under a versioned AES-256-GCM envelope, `simenc:v1:::`, built on the existing `encryptSecret`/`decryptSecret` primitives. Rollout is safe in both directions. Reads detect the format per value and never consult the flag, so a mixed-format table reads correctly throughout; only writes are gated, behind the AppConfig flag `oauth-token-encryption`, which is off by default. The deploy is therefore inert on arrival and the flag is flipped once every pod carries the tolerant reader. Rolling back is a config change. Self-hosted stays on plaintext until an operator opts in with a valid 64-hex `ENCRYPTION_KEY`; a misconfigured key degrades to plaintext rather than failing a user's OAuth connect. Better Auth's own `account.encryptOAuthTokens` is deliberately not used: it keys off `BETTER_AUTH_SECRET` rather than `ENCRYPTION_KEY`, leaves `idToken` in plaintext despite its docs, decrypts only inside its own endpoints rather than on the direct database reads this app performs, and detects ciphertext by treating any even-length hex string as encrypted — the shape of a real Trello or Airtable token. The rationale is recorded next to the envelope so the two schemes are never confused. Consolidation, because the duplication is what made encryption risky: - Three divergent copies of the token-staleness rule collapse into `refresh-policy.ts`. `getOAuthToken`'s copy omitted the Microsoft proactive-refresh arm, so credentials reached only that way could pass Microsoft's 90-day inactivity deadline and die; unifying fixes that. - `refreshTokenIfNeeded`'s `credential: any` becomes a branded `LoadedOAuthCredential`, which caught three callers passing raw rows at compile time. Two collapse onto the new `resolveAccessTokenForAccount`. - Eleven projection-less `account` reads become `id`/`userId` projections or calls to the existing `getCredentialOwner`. - The Shopify, Instagram and Trello connect flows — three copies of find/update/insert/re-find — share `upsertProviderAccountTokens`, so a new provider cannot store a plaintext token by copying an old flow. `check:account-token-access` enforces the boundary in CI, flagging direct token column reads, projection-less selects, and direct writes to the table. Also fixed along the way: `create.before` and `create.after` both called `fetchSalesforceInstanceUrl` and both prepended the instance-URL marker, so every Salesforce connect made the same live API call twice and stored a double-prefixed `scope`. And Better Auth's `/get-access-token` and `/refresh-token` endpoints, reachable through the catch-all and bypassing `databaseHooks` entirely, are now blocked — nothing in this app calls them. No migration: no query filters or joins on a token value, and the columns are `text`. --- apps/sim/app/api/auth/[...all]/route.ts | 18 + .../auth/oauth2/callback/instagram/route.ts | 74 +--- apps/sim/app/api/auth/trello/store/route.ts | 61 +-- apps/sim/lib/auth/auth.ts | 48 ++- .../tools/server/user/get-credentials.ts | 23 +- apps/sim/lib/core/config/env.ts | 1 + apps/sim/lib/core/config/feature-flags.ts | 8 + apps/sim/lib/core/security/encryption.ts | 18 +- apps/sim/lib/credentials/oauth-accounts.ts | 17 +- .../lib/guardrails/validate_hallucination.ts | 23 +- apps/sim/lib/internal/llm/credentials.ts | 12 +- .../lib/oauth/account-token-crypto.test.ts | 176 +++++++++ apps/sim/lib/oauth/account-token-crypto.ts | 85 +++++ apps/sim/lib/oauth/account-tokens.test.ts | 217 +++++++++++ apps/sim/lib/oauth/account-tokens.ts | 123 ++++++ apps/sim/lib/oauth/credential-service.ts | 354 +++++++++++------- apps/sim/lib/oauth/refresh-policy.test.ts | 191 ++++++++++ apps/sim/lib/oauth/refresh-policy.ts | 98 +++++ apps/sim/lib/oauth/shopify.ts | 64 +--- apps/sim/lib/oauth/slack.ts | 27 +- apps/sim/lib/oauth/token-resolution.ts | 8 +- .../lib/oauth/upsert-provider-account.test.ts | 145 +++++++ apps/sim/lib/webhooks/polling/utils.ts | 10 +- apps/sim/lib/webhooks/providers/airtable.ts | 34 +- apps/sim/lib/webhooks/providers/gmail.ts | 29 +- .../lib/webhooks/providers/microsoft-teams.ts | 32 +- apps/sim/lib/webhooks/providers/outlook.ts | 29 +- package.json | 4 +- scripts/check-account-token-access.test.ts | 122 ++++++ scripts/check-account-token-access.ts | 165 ++++++++ 30 files changed, 1759 insertions(+), 457 deletions(-) create mode 100644 apps/sim/lib/oauth/account-token-crypto.test.ts create mode 100644 apps/sim/lib/oauth/account-token-crypto.ts create mode 100644 apps/sim/lib/oauth/account-tokens.test.ts create mode 100644 apps/sim/lib/oauth/account-tokens.ts create mode 100644 apps/sim/lib/oauth/refresh-policy.test.ts create mode 100644 apps/sim/lib/oauth/refresh-policy.ts create mode 100644 apps/sim/lib/oauth/upsert-provider-account.test.ts create mode 100644 scripts/check-account-token-access.test.ts create mode 100644 scripts/check-account-token-access.ts diff --git a/apps/sim/app/api/auth/[...all]/route.ts b/apps/sim/app/api/auth/[...all]/route.ts index 03c3a1514ad..797c41fed31 100644 --- a/apps/sim/app/api/auth/[...all]/route.ts +++ b/apps/sim/app/api/auth/[...all]/route.ts @@ -71,6 +71,17 @@ function isBlockedSsoMutationPath(path: string): boolean { return path.startsWith('sso/') && !path.startsWith(SAML_PROTOCOL_POST_PREFIX) } +/** + * Better Auth's own account-token endpoints read `account` through the adapter with no + * `databaseHooks` pass, so they would return the stored column verbatim — ciphertext. + * Nothing here calls them; token reads go through `@/lib/oauth/credential-service`. + */ +const BLOCKED_ACCOUNT_TOKEN_POST_PATHS = new Set(['get-access-token', 'refresh-token']) + +function isBlockedAccountTokenPath(path: string): boolean { + return BLOCKED_ACCOUNT_TOKEN_POST_PATHS.has(path) +} + export const GET = withRouteHandler(async (request: NextRequest) => { const path = getAuthPath(request) const credentialGroupProviderId = getCredentialGroupCallbackProviderId(request, path) @@ -126,5 +137,12 @@ export const POST = withRouteHandler(async (request: NextRequest) => { ) } + if (isBlockedAccountTokenPath(path)) { + return NextResponse.json( + { error: 'Account token access is handled by application API routes.' }, + { status: 404 } + ) + } + return betterAuthPOST(request) }) diff --git a/apps/sim/app/api/auth/oauth2/callback/instagram/route.ts b/apps/sim/app/api/auth/oauth2/callback/instagram/route.ts index 19284a950fc..ec857cd190f 100644 --- a/apps/sim/app/api/auth/oauth2/callback/instagram/route.ts +++ b/apps/sim/app/api/auth/oauth2/callback/instagram/route.ts @@ -1,8 +1,4 @@ -import { db } from '@sim/db' -import { account } from '@sim/db/schema' import { createLogger } from '@sim/logger' -import { generateId } from '@sim/utils/id' -import { and, eq } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { instagramCallbackContract } from '@/lib/api/contracts/oauth-connections' import { parseRequest } from '@/lib/api/server' @@ -18,7 +14,7 @@ import { getBaseUrl } from '@/lib/core/utils/urls' import { isSameOrigin } from '@/lib/core/utils/validation' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { processCredentialDraft } from '@/lib/credentials/draft-processor' -import { safeAccountInsert } from '@/lib/oauth/credential-service' +import { upsertProviderAccountTokens } from '@/lib/oauth/credential-service' import { parseInstagramLongLivedToken, parseInstagramProfile, @@ -243,70 +239,24 @@ export const GET = withRouteHandler(async (request: NextRequest) => { : getCanonicalScopesForProvider('instagram') const scope = permissions.join(' ') - const now = new Date() - const accessTokenExpiresAt = new Date(now.getTime() + expiresIn * 1000) + const accessTokenExpiresAt = new Date(Date.now() + expiresIn * 1000) - const existing = await db.query.account.findFirst({ - where: and( - eq(account.userId, session.user.id), - eq(account.providerId, 'instagram'), - eq(account.accountId, igUserId) - ), + const { accountId } = await upsertProviderAccountTokens({ + userId: session.user.id, + providerId: 'instagram', + externalAccountId: igUserId, + scope, + /** Instagram's long-lived token is its own refresh token; both columns hold it. */ + tokens: { accessToken: longLivedToken, refreshToken: longLivedToken }, + accessTokenExpiresAt, + logIdentifier: profile.username || igUserId, }) - if (existing) { - await db - .update(account) - .set({ - accessToken: longLivedToken, - refreshToken: longLivedToken, - accessTokenExpiresAt, - scope, - updatedAt: now, - }) - .where(eq(account.id, existing.id)) - logger.info('Updated existing Instagram account', { - accountId: existing.id, - igUserId, - username: profile.username, - }) - } else { - await safeAccountInsert( - { - id: generateId(), - userId: session.user.id, - providerId: 'instagram', - accountId: igUserId, - accessToken: longLivedToken, - refreshToken: longLivedToken, - accessTokenExpiresAt, - scope, - createdAt: now, - updatedAt: now, - }, - { provider: 'Instagram', identifier: profile.username || igUserId } - ) - logger.info('Created Instagram account', { igUserId, username: profile.username }) - } - - const persisted = - existing ?? - (await db.query.account.findFirst({ - where: and( - eq(account.userId, session.user.id), - eq(account.providerId, 'instagram'), - eq(account.accountId, igUserId) - ), - })) - - if (!persisted) { - throw new Error(`Instagram OAuth account ${igUserId} was not persisted`) - } await processCredentialDraft({ draftId, userId: session.user.id, providerId: 'instagram', - accountId: persisted.id, + accountId, }) const returnUrlCookie = request.cookies.get(INSTAGRAM_RETURN_URL_COOKIE)?.value diff --git a/apps/sim/app/api/auth/trello/store/route.ts b/apps/sim/app/api/auth/trello/store/route.ts index 22d9a04aefa..82156b39992 100644 --- a/apps/sim/app/api/auth/trello/store/route.ts +++ b/apps/sim/app/api/auth/trello/store/route.ts @@ -1,7 +1,4 @@ -import { db } from '@sim/db' -import { account } from '@sim/db/schema' import { createLogger } from '@sim/logger' -import { and, eq } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { storeTrelloTokenContract } from '@/lib/api/contracts/oauth-connections' import { parseRequest } from '@/lib/api/server' @@ -9,7 +6,7 @@ import { getSession } from '@/lib/auth' import { env } from '@/lib/core/config/env' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { processCredentialDraft } from '@/lib/credentials/draft-processor' -import { safeAccountInsert } from '@/lib/oauth/credential-service' +import { upsertProviderAccountTokens } from '@/lib/oauth/credential-service' import { getCanonicalScopesForProvider } from '@/lib/oauth/utils' const logger = createLogger('TrelloStore') @@ -92,60 +89,20 @@ export const POST = withRouteHandler(async (request: NextRequest) => { ) } - const existing = await db.query.account.findFirst({ - where: and( - eq(account.userId, session.user.id), - eq(account.providerId, 'trello'), - eq(account.accountId, trelloUser.id) - ), + const { accountId } = await upsertProviderAccountTokens({ + userId: session.user.id, + providerId: 'trello', + externalAccountId: trelloUser.id, + scope, + /** Trello tokens do not expire and there is no refresh token. */ + tokens: { accessToken: token }, }) - const now = new Date() - - if (existing) { - await db - .update(account) - .set({ - accessToken: token, - accountId: trelloUser.id, - scope, - updatedAt: now, - }) - .where(eq(account.id, existing.id)) - } else { - await safeAccountInsert( - { - id: `trello_${session.user.id}_${Date.now()}`, - userId: session.user.id, - providerId: 'trello', - accountId: trelloUser.id, - accessToken: token, - scope, - createdAt: now, - updatedAt: now, - }, - { provider: 'Trello', identifier: trelloUser.id } - ) - } - - const persisted = - existing ?? - (await db.query.account.findFirst({ - where: and( - eq(account.userId, session.user.id), - eq(account.providerId, 'trello'), - eq(account.accountId, trelloUser.id) - ), - })) - - if (!persisted) { - throw new Error(`Trello OAuth account ${trelloUser.id} was not persisted`) - } await processCredentialDraft({ draftId, userId: session.user.id, providerId: 'trello', - accountId: persisted.id, + accountId, }) return clearStateCookie(NextResponse.json({ success: true })) diff --git a/apps/sim/lib/auth/auth.ts b/apps/sim/lib/auth/auth.ts index 152a8354199..400a0e68e33 100644 --- a/apps/sim/lib/auth/auth.ts +++ b/apps/sim/lib/auth/auth.ts @@ -107,6 +107,7 @@ import { quickValidateEmail } from '@/lib/messaging/email/validation' import { validateSignupEmailMx } from '@/lib/messaging/email/validation.server' import { isEmailVerificationEffectivelyEnabled } from '@/lib/messaging/email/verification' import { scheduleLifecycleEmail } from '@/lib/messaging/lifecycle' +import { decryptAccountTokenColumns, encryptAccountTokenColumns } from '@/lib/oauth/account-tokens' import { getMicrosoftRefreshTokenExpiry, isMicrosoftProvider, @@ -388,16 +389,6 @@ export const auth = betterAuth({ } } - if (account.accessToken && isSalesforceOAuthProviderId(account.providerId)) { - const instanceUrl = await fetchSalesforceInstanceUrl( - account.providerId, - account.accessToken - ) - if (instanceUrl) { - modifiedAccount.scope = withSalesforceInstanceScope(instanceUrl, account.scope) - } - } - if (isMicrosoftProvider(account.providerId)) { modifiedAccount.refreshTokenExpiresAt = getMicrosoftRefreshTokenExpiry() } @@ -412,9 +403,16 @@ export const auth = betterAuth({ } } - return { data: modifiedAccount } + return { data: await encryptAccountTokenColumns(modifiedAccount) } }, + /** + * Better Auth hands after-hooks the row AS WRITTEN, so the Slack fan-out and the + * Salesforce instance-URL fetch below must use the decrypted copy — a live provider + * call with ciphertext 401s silently and leaves the credential unusable. + */ after: async (account, context) => { + const tokens = await decryptAccountTokenColumns(account) + /** * Migrate credentials from stale account rows to the newly created one. * @@ -479,7 +477,7 @@ export const auth = betterAuth({ * Propagate the new chain so every sibling is valid again, and clear * the installation's dead flag. */ - if (account.providerId === 'slack' && account.accessToken) { + if (account.providerId === 'slack' && tokens.accessToken) { try { const teamId = extractSlackTeamId(account.accountId) if (teamId) { @@ -488,8 +486,8 @@ export const auth = betterAuth({ // failure must not leave the hour-long flag blocking refreshes. await clearDeadFlag(`slack:${teamId}`) await fanOutSlackTokenChain(teamId, { - accessToken: account.accessToken, - refreshToken: account.refreshToken ?? null, + accessToken: tokens.accessToken, + refreshToken: tokens.refreshToken ?? null, accessTokenExpiresAt: account.accessTokenExpiresAt ?? null, }) logger.info('[account.create.after] Propagated Slack installation token chain', { @@ -582,6 +580,11 @@ export const auth = betterAuth({ ) } + /** + * The only place the instance-URL marker is written. `create.before` used to + * make the same call and set `scope` too, which left the marker written twice + * over — `withSalesforceInstanceScope` prepends unconditionally. + */ if (isSalesforceOAuthProviderId(account.providerId)) { const updates: { accessTokenExpiresAt?: Date @@ -592,10 +595,10 @@ export const auth = betterAuth({ updates.accessTokenExpiresAt = new Date(Date.now() + 2 * 60 * 60 * 1000) } - if (account.accessToken) { + if (tokens.accessToken) { const instanceUrl = await fetchSalesforceInstanceUrl( account.providerId, - account.accessToken + tokens.accessToken ) if (instanceUrl) { updates.scope = withSalesforceInstanceScope(instanceUrl, account.scope) @@ -603,12 +606,14 @@ export const auth = betterAuth({ } if (Object.keys(updates).length > 0) { + // account-token-access-allow: writes only accessTokenExpiresAt and scope await db.update(schema.account).set(updates).where(eq(schema.account.id, account.id)) } } if (isMicrosoftProvider(account.providerId)) { await db + // account-token-access-allow: writes only refreshTokenExpiresAt .update(schema.account) .set({ refreshTokenExpiresAt: getMicrosoftRefreshTokenExpiry() }) .where(eq(schema.account.id, account.id)) @@ -624,6 +629,17 @@ export const auth = betterAuth({ } }, }, + /** + * Catches token writes that are not creates — above all `updateAccountOnSignIn`, + * which would otherwise revert a row to plaintext on every repeat sign-in. + * + * Not OAuth-only: `updatePassword` reaches the same hook through `updateManyWithHooks` + * with a `{ password }`-only payload. The encryption is field-local for that reason, + * and this payload holds a password hash, so it must never be logged. + */ + update: { + before: async (data) => ({ data: await encryptAccountTokenColumns(data) }), + }, }, session: { create: { diff --git a/apps/sim/lib/copilot/tools/server/user/get-credentials.ts b/apps/sim/lib/copilot/tools/server/user/get-credentials.ts index 059be3c0e13..91821d8e4c3 100644 --- a/apps/sim/lib/copilot/tools/server/user/get-credentials.ts +++ b/apps/sim/lib/copilot/tools/server/user/get-credentials.ts @@ -17,6 +17,7 @@ import { credentialProviderMatchesService, getAllOAuthServices, } from '@/lib/oauth' +import { decryptAccountTokenColumnsBatch } from '@/lib/oauth/account-tokens' import { intersectIntegrationAllowlists } from '@/lib/permission-groups/integration-allowlist' import { checkWorkspaceAccess, type WorkspaceAccess } from '@/lib/workspaces/permissions/utils' import { overlayVisibility } from '@/blocks/visibility/context' @@ -71,13 +72,21 @@ export const getCredentialsServerTool: BaseServerTool hasWorkflowId: !!params?.workflowId, }) - // Fetch OAuth credentials - const accounts = await db.select().from(account).where(eq(account.userId, userId)) - const userRecord = await db - .select({ email: user.email }) - .from(user) - .where(eq(user.id, userId)) - .limit(1) + const [accountRows, userRecord] = await Promise.all([ + db + .select({ + id: account.id, + accountId: account.accountId, + providerId: account.providerId, + // account-token-access-allow: decrypted immediately below; needed for the display-name JWT decode + idToken: account.idToken, + updatedAt: account.updatedAt, + }) + .from(account) + .where(eq(account.userId, userId)), + db.select({ email: user.email }).from(user).where(eq(user.id, userId)).limit(1), + ]) + const accounts = await decryptAccountTokenColumnsBatch(accountRows) const userEmail = userRecord.length > 0 ? userRecord[0]?.email : null const permissionConfig = workspaceId ? await getUserPermissionConfig(userId, workspaceId) : null diff --git a/apps/sim/lib/core/config/env.ts b/apps/sim/lib/core/config/env.ts index 5bc20313e08..7d968d66e07 100644 --- a/apps/sim/lib/core/config/env.ts +++ b/apps/sim/lib/core/config/env.ts @@ -587,6 +587,7 @@ export const env = createEnv({ FORKING_ENABLED: z.boolean().optional(), // Enable workspace forking on self-hosted (bypasses hosted requirements) TABLES_V2_API: z.boolean().optional(), // Enable the v2 tables HTTP API (public /api/v2/tables + internal /api/table/[tableId]/query predicate-grammar route) CREDENTIAL_GROUPS: z.boolean().optional(), // Enable enterprise Credential Groups globally + OAUTH_TOKEN_ENCRYPTION: z.boolean().optional(), // Encrypt account OAuth tokens at rest (fallback for the oauth-token-encryption flag when AppConfig is not the source of truth) // Organizations - for self-hosted deployments ORGANIZATIONS_ENABLED: z.boolean().optional(), // Enable organizations on self-hosted (bypasses plan requirements) diff --git a/apps/sim/lib/core/config/feature-flags.ts b/apps/sim/lib/core/config/feature-flags.ts index b610508b7ee..8e7a55cc65c 100644 --- a/apps/sim/lib/core/config/feature-flags.ts +++ b/apps/sim/lib/core/config/feature-flags.ts @@ -79,6 +79,14 @@ const FEATURE_FLAGS = { 'Enterprise subscription. Off-AppConfig falls back to CREDENTIAL_GROUPS.', fallback: 'CREDENTIAL_GROUPS', }, + 'oauth-token-encryption': { + description: + 'Encrypt account.access_token / refresh_token / id_token at rest under the `simenc:v1:` ' + + 'envelope. Only writes are gated — reads always accept both formats, which is what makes ' + + 'rollout and rollback safe. Global on/off so a deployment can never be half-encrypted. ' + + 'Off-AppConfig falls back to OAUTH_TOKEN_ENCRYPTION (unset by default; needs a 64-hex ENCRYPTION_KEY).', + fallback: 'OAUTH_TOKEN_ENCRYPTION', + }, } satisfies Record /** diff --git a/apps/sim/lib/core/security/encryption.ts b/apps/sim/lib/core/security/encryption.ts index 2ffaff4ef23..251d3481fc4 100644 --- a/apps/sim/lib/core/security/encryption.ts +++ b/apps/sim/lib/core/security/encryption.ts @@ -6,12 +6,24 @@ import { env } from '@/lib/core/config/env' const logger = createLogger('Encryption') -function getEncryptionKey(): Buffer { +/** + * Whether `ENCRYPTION_KEY` is usable, without throwing. + * + * `env.ts` only validates it as `min(32)`, so a deployment can boot with a key this module + * will reject on first use. Callers that must degrade rather than fail — writing plaintext + * instead of losing a user's OAuth connect — check this first. Non-hex is rejected here + * because `Buffer.from(key, 'hex')` would silently produce a short buffer. + */ +export function hasUsableEncryptionKey(): boolean { const key = env.ENCRYPTION_KEY - if (!key || key.length !== 64) { + return typeof key === 'string' && key.length === 64 && /^[0-9a-f]+$/i.test(key) +} + +function getEncryptionKey(): Buffer { + if (!hasUsableEncryptionKey()) { throw new Error('ENCRYPTION_KEY must be set to a 64-character hex string (32 bytes)') } - return Buffer.from(key, 'hex') + return Buffer.from(env.ENCRYPTION_KEY as string, 'hex') } /** diff --git a/apps/sim/lib/credentials/oauth-accounts.ts b/apps/sim/lib/credentials/oauth-accounts.ts index 705d926d5bc..4a7a699f5b1 100644 --- a/apps/sim/lib/credentials/oauth-accounts.ts +++ b/apps/sim/lib/credentials/oauth-accounts.ts @@ -8,6 +8,7 @@ import type { OAuthConnection } from '@/lib/api/contracts/oauth-connections' import { deleteCredentialRecord } from '@/lib/credentials/orchestration' import type { OAuthProvider } from '@/lib/oauth' import { parseProvider } from '@/lib/oauth' +import { decryptAccountTokenColumnsBatch } from '@/lib/oauth/account-tokens' import { providerIdsForService } from '@/lib/oauth/utils' const logger = createLogger('CredentialOAuthAccounts') @@ -18,10 +19,22 @@ interface GoogleIdToken { } export async function listOAuthConnectionsForUser(userId: string): Promise { - const [accounts, userRecord] = await Promise.all([ - db.select().from(account).where(eq(account.userId, userId)), + const [accountRows, userRecord] = await Promise.all([ + db + .select({ + id: account.id, + accountId: account.accountId, + providerId: account.providerId, + scope: account.scope, + // account-token-access-allow: decrypted immediately below; needed for the display-name JWT decode + idToken: account.idToken, + updatedAt: account.updatedAt, + }) + .from(account) + .where(eq(account.userId, userId)), db.select({ email: user.email }).from(user).where(eq(user.id, userId)).limit(1), ]) + const accounts = await decryptAccountTokenColumnsBatch(accountRows) const userEmail = userRecord[0]?.email ?? null const connections: OAuthConnection[] = [] diff --git a/apps/sim/lib/guardrails/validate_hallucination.ts b/apps/sim/lib/guardrails/validate_hallucination.ts index 0e25c657a5b..a987999b09b 100644 --- a/apps/sim/lib/guardrails/validate_hallucination.ts +++ b/apps/sim/lib/guardrails/validate_hallucination.ts @@ -1,13 +1,10 @@ -import { db } from '@sim/db' -import { account } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' import { isPlainRecord } from '@sim/utils/object' -import { eq } from 'drizzle-orm' import type { BillingAttributionSnapshot } from '@/lib/billing/core/billing-attribution' import { searchKnowledgeAsExecutor } from '@/lib/internal/knowledge/search' import type { InternalToolOperationContext } from '@/lib/internal/tool-operations/types' -import { refreshTokenIfNeeded } from '@/lib/oauth/credential-service' +import { resolveAccessTokenForAccount } from '@/lib/oauth/credential-service' import { projectResolvedSecretModelContent } from '@/executor/utils/resolved-secret-content-projection' import { refuseResolvedSecretProjection } from '@/executor/utils/resolved-secret-projection-refusal' import type { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry' @@ -162,18 +159,12 @@ Evaluate the consistency and provide your score and reasoning in JSON format.` let finalApiKey: string | undefined = apiKey if (providerId === 'vertex' && providerCredentials?.vertexCredential) { - const credential = await db.query.account.findFirst({ - where: eq(account.id, providerCredentials.vertexCredential), - }) - if (credential) { - const { accessToken } = await refreshTokenIfNeeded( - requestId, - credential, - providerCredentials.vertexCredential - ) - if (accessToken) { - finalApiKey = accessToken - } + const accessToken = await resolveAccessTokenForAccount( + requestId, + providerCredentials.vertexCredential + ) + if (accessToken) { + finalApiKey = accessToken } } diff --git a/apps/sim/lib/internal/llm/credentials.ts b/apps/sim/lib/internal/llm/credentials.ts index 2dbd6407e9a..92e07051a8e 100644 --- a/apps/sim/lib/internal/llm/credentials.ts +++ b/apps/sim/lib/internal/llm/credentials.ts @@ -1,10 +1,7 @@ -import { db } from '@sim/db' -import { account } from '@sim/db/schema' import { createLogger } from '@sim/logger' -import { eq } from 'drizzle-orm' import { getServiceAccountToken, - refreshTokenIfNeeded, + resolveAccessTokenForAccount, resolveOAuthAccountId, } from '@/lib/oauth/credential-service' @@ -28,12 +25,7 @@ export async function resolveVertexAccessToken( return accessToken } - const credential = await db.query.account.findFirst({ - where: eq(account.id, resolved.accountId), - }) - if (!credential) throw new Error(`Vertex AI credential not found: ${credentialId}`) - - const { accessToken } = await refreshTokenIfNeeded(requestId, credential, resolved.accountId) + const accessToken = await resolveAccessTokenForAccount(requestId, resolved.accountId) if (!accessToken) throw new Error('Failed to get Vertex AI access token') logger.info(`[${requestId}] Resolved Vertex AI credential`) diff --git a/apps/sim/lib/oauth/account-token-crypto.test.ts b/apps/sim/lib/oauth/account-token-crypto.test.ts new file mode 100644 index 00000000000..3b3be217d39 --- /dev/null +++ b/apps/sim/lib/oauth/account-token-crypto.test.ts @@ -0,0 +1,176 @@ +/** + * @vitest-environment node + */ +import { createEnvMock } from '@sim/testing' +import { describe, expect, it, vi } from 'vitest' + +vi.mock('@/lib/core/config/env', () => + createEnvMock({ + ENCRYPTION_KEY: '0123456789abcdef'.repeat(4), + }) +) + +import { decrypt } from '@sim/security/encryption' +import { decryptSecret } from '@/lib/core/security/encryption' +import { + AccountTokenDecryptionError, + decryptAccountToken, + encryptAccountToken, + isEncryptedAccountToken, +} from '@/lib/oauth/account-token-crypto' + +/** + * The value shapes these columns hold. What each assertion depends on is the character + * class and delimiters, not the provider — so the fixtures reproduce the shape without + * reproducing any provider's token prefix, which secret scanning rejects on every push. + * + * The hex entry is the load-bearing one: it is exactly what Better Auth's own + * `isLikelyEncrypted` misreads as an envelope. + */ +const TOKEN_SHAPES = [ + ['dotted opaque, as Google issues', 'token.placeholder-value-with-dots'], + ['three dot-separated segments, as an OIDC id token', 'header.payload.signature'], + ['dash-delimited, as Slack issues', 'bot-placeholder-0000-token-value'], + ['underscore-prefixed, as Shopify issues', 'admin_placeholder_token_value'], + ['a bare hostname, which Shopify stores in id_token', 'my-test-store.myshopify.com'], + ['underscore-prefixed, as GitHub issues', 'classic_placeholdertokenvalue'], + ['even-length hex, as Trello issues', 'a1b2c3d4'.repeat(4)], + ['dotted with underscores, as Microsoft issues', '0.placeholder_token_value'], +] as const + +describe('isEncryptedAccountToken', () => { + it.each(TOKEN_SHAPES)('does not classify a %s token as ciphertext', (_label, token) => { + expect(isEncryptedAccountToken(token)).toBe(false) + }) + + it('classifies an envelope as ciphertext', async () => { + expect(isEncryptedAccountToken(await encryptAccountToken('secret'))).toBe(true) + }) +}) + +describe('envelope wire format', () => { + const KEY = Buffer.from('0123456789abcdef'.repeat(4), 'hex') + + /** + * The stored format is a compatibility contract: the backfill and any future key rotation + * both parse it, so its shape is pinned here rather than left to the implementation. + */ + it('is `simenc:v1:` followed by exactly iv:ciphertext:authTag', async () => { + const [scheme, version, ...rest] = (await encryptAccountToken('token.placeholder-value')).split( + ':' + ) + + expect(scheme).toBe('simenc') + expect(version).toBe('v1') + expect(rest).toHaveLength(3) + expect(rest[0]).toMatch(/^[0-9a-f]{32}$/) + expect(rest[1]).toMatch(/^[0-9a-f]+$/) + expect(rest[2]).toMatch(/^[0-9a-f]{32}$/) + }) + + it('leaves a payload the shared primitive can decrypt once the prefix is stripped', async () => { + const payload = (await encryptAccountToken('token.placeholder-value')).slice( + 'simenc:v1:'.length + ) + await expect(decrypt(payload, KEY)).resolves.toEqual({ decrypted: 'token.placeholder-value' }) + }) + + it('never leaves the plaintext visible in the stored value', async () => { + await expect(encryptAccountToken('super-secret-refresh')).resolves.not.toContain('super-secret') + }) + + it.each([ + ['a 4KB token', 'x'.repeat(4096)], + ['unicode', '토큰-🔐-Ünïcode'], + ])('round-trips %s', async (_label, token) => { + expect(await decryptAccountToken(await encryptAccountToken(token), 'accessToken')).toBe(token) + }) +}) + +describe('encryptAccountToken', () => { + it('produces a v1 envelope that round-trips', async () => { + const encrypted = await encryptAccountToken('token.placeholder-value') + + expect(encrypted.startsWith('simenc:v1:')).toBe(true) + expect(encrypted).not.toContain('token.placeholder-value') + await expect(decryptAccountToken(encrypted, 'accessToken')).resolves.toBe( + 'token.placeholder-value' + ) + }) + + it('is idempotent so a re-encrypted value is never double-wrapped', async () => { + const once = await encryptAccountToken('token') + const twice = await encryptAccountToken(once) + + expect(twice).toBe(once) + await expect(decryptAccountToken(twice, 'accessToken')).resolves.toBe('token') + }) + + it.each([ + ['empty string', ''], + ['null', null], + ['undefined', undefined], + ])('passes %s through untouched', async (_label, value) => { + await expect(encryptAccountToken(value as string)).resolves.toBe(value) + }) + + it('produces different ciphertext for the same plaintext', async () => { + expect(await encryptAccountToken('same')).not.toBe(await encryptAccountToken('same')) + }) + + it.each(TOKEN_SHAPES)('round-trips a %s token', async (_label, token) => { + const encrypted = await encryptAccountToken(token) + await expect(decryptAccountToken(encrypted, 'accessToken')).resolves.toBe(token) + }) +}) + +describe('decryptAccountToken', () => { + it.each(TOKEN_SHAPES)('returns legacy plaintext %s unchanged', async (_label, token) => { + await expect(decryptAccountToken(token, 'accessToken')).resolves.toBe(token) + }) + + it.each([ + ['empty string', ''], + ['null', null], + ['undefined', undefined], + ])('passes %s through untouched', async (_label, value) => { + await expect(decryptAccountToken(value as string, 'accessToken')).resolves.toBe(value) + }) + + it('throws on an unknown envelope version rather than passing it through', async () => { + await expect(decryptAccountToken('simenc:v2:deadbeef', 'refreshToken')).rejects.toThrow( + AccountTokenDecryptionError + ) + await expect(decryptAccountToken('simenc:v2:deadbeef', 'refreshToken')).rejects.toMatchObject({ + field: 'refreshToken', + reason: 'unknown-version', + }) + }) + + it('throws when the auth tag has been tampered with', async () => { + const encrypted = await encryptAccountToken('token') + const [iv, ciphertext] = encrypted.slice('simenc:v1:'.length).split(':') + const forged = `simenc:v1:${iv}:${ciphertext}:${'0'.repeat(32)}` + + await expect(decryptAccountToken(forged, 'accessToken')).rejects.toMatchObject({ + field: 'accessToken', + reason: 'decrypt-failed', + }) + }) + + it('throws when the payload is truncated', async () => { + await expect(decryptAccountToken('simenc:v1:abc', 'idToken')).rejects.toMatchObject({ + reason: 'decrypt-failed', + }) + }) + + /** + * `decryptSecret` splits on `:` and reads the first segment as the IV, so a full + * envelope handed to it directly parses `'simenc'` as IV hex. The accessor must + * strip the prefix; this pins the failure so a future refactor cannot reintroduce it. + */ + it('confirms the raw primitive cannot consume a prefixed envelope', async () => { + const encrypted = await encryptAccountToken('token') + await expect(decryptSecret(encrypted)).rejects.toThrow() + }) +}) diff --git a/apps/sim/lib/oauth/account-token-crypto.ts b/apps/sim/lib/oauth/account-token-crypto.ts new file mode 100644 index 00000000000..d8fd563d901 --- /dev/null +++ b/apps/sim/lib/oauth/account-token-crypto.ts @@ -0,0 +1,85 @@ +import { decryptSecret, encryptSecret } from '@/lib/core/security/encryption' + +/** + * Marks a stored `account` token column as ciphertext. Detection is an exact prefix match, + * never a shape or length heuristic. + * + * Better Auth ships its own `account.encryptOAuthTokens` and we deliberately do not use it: + * it keys off `BETTER_AUTH_SECRET` instead of `ENCRYPTION_KEY`, leaves `idToken` in + * plaintext despite its documentation, decrypts only inside Better Auth's own endpoints + * rather than on the direct database reads this app performs, and detects ciphertext by + * treating any even-length hex string as encrypted — which is the shape of a real Trello or + * Airtable token. The two schemes are mutually exclusive: its detector does not recognize + * this prefix, so enabling that flag later would pass our ciphertext through undecrypted. + */ +const ENVELOPE_PREFIX = 'simenc:' + +/** Current envelope version. The payload after this prefix is `iv:ciphertext:authTag`. */ +const ENVELOPE_V1_PREFIX = `${ENVELOPE_PREFIX}v1:` + +/** The `account` columns this module protects. */ +export type AccountTokenField = 'accessToken' | 'refreshToken' | 'idToken' + +/** + * Thrown when a prefixed value cannot be recovered — wrong `ENCRYPTION_KEY`, truncated + * column, or an unknown envelope version. Never thrown for legacy plaintext. + */ +export class AccountTokenDecryptionError extends Error { + constructor( + readonly field: AccountTokenField, + readonly reason: 'unknown-version' | 'decrypt-failed', + cause?: unknown + ) { + super(`Failed to decrypt ${field}: ${reason}`, cause ? { cause } : undefined) + this.name = 'AccountTokenDecryptionError' + } +} + +/** True when a stored column value is one of our envelopes rather than legacy plaintext. */ +export function isEncryptedAccountToken(value: string): boolean { + return value.startsWith(ENVELOPE_PREFIX) +} + +/** + * Wraps a token in the current envelope. Idempotent, which is what makes a retried backfill + * batch safe. + * + * Empty strings pass through: `@better-auth/sso` writes `accessToken: ''` for SAML rows, + * and enveloping that would make it truthy, flipping every `if (account.accessToken)` guard. + */ +export async function encryptAccountToken(plaintext: string): Promise { + if (!plaintext) return plaintext + if (isEncryptedAccountToken(plaintext)) return plaintext + + const { encrypted } = await encryptSecret(plaintext) + return `${ENVELOPE_V1_PREFIX}${encrypted}` +} + +/** + * Recovers a token written by {@link encryptAccountToken}. Unprefixed values are legacy + * plaintext and pass through, which is what lets a mixed-format table be read during rollout. + * + * A prefixed value that will not decrypt throws rather than passing through: forwarding + * ciphertext to a provider looks exactly like a revoked credential and gets diagnosed wrong. + */ +export async function decryptAccountToken( + value: string, + field: AccountTokenField +): Promise { + if (!value) return value + if (!isEncryptedAccountToken(value)) return value + + if (!value.startsWith(ENVELOPE_V1_PREFIX)) { + throw new AccountTokenDecryptionError(field, 'unknown-version') + } + + /** `decryptSecret` reads segment 0 as the IV, so the prefix must come off first. */ + const payload = value.slice(ENVELOPE_V1_PREFIX.length) + + try { + const { decrypted } = await decryptSecret(payload) + return decrypted + } catch (error) { + throw new AccountTokenDecryptionError(field, 'decrypt-failed', error) + } +} diff --git a/apps/sim/lib/oauth/account-tokens.test.ts b/apps/sim/lib/oauth/account-tokens.test.ts new file mode 100644 index 00000000000..e484941bfb3 --- /dev/null +++ b/apps/sim/lib/oauth/account-tokens.test.ts @@ -0,0 +1,217 @@ +/** + * @vitest-environment node + */ +import { resetEnvMock, setEnv } from '@sim/testing' +import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockIsFeatureEnabled } = vi.hoisted(() => ({ mockIsFeatureEnabled: vi.fn() })) + +vi.mock('@/lib/core/config/feature-flags', () => ({ + isFeatureEnabled: mockIsFeatureEnabled, +})) + +import { encryptAccountToken } from '@/lib/oauth/account-token-crypto' +import { + decryptAccountTokenColumns, + decryptAccountTokenColumnsBatch, + encryptAccountTokenColumns, +} from '@/lib/oauth/account-tokens' + +const VALID_KEY = '0123456789abcdef'.repeat(4) + +/** Stands in for the scrypt hash Better Auth writes; only its presence matters here. */ +const passwordHashPlaceholder = 'hash' + +/** The shape `docker-compose.local.yml` ships: long enough for env validation, not hex. */ +const SELF_HOSTED_BAD_KEY = 'z'.repeat(36) + +beforeEach(() => { + vi.clearAllMocks() + resetEnvMock() + setEnv({ ENCRYPTION_KEY: VALID_KEY }) + mockIsFeatureEnabled.mockResolvedValue(true) +}) + +afterAll(resetEnvMock) + +describe('the write gate', () => { + it('envelopes when the flag is on and the key is usable', async () => { + const { accessToken } = await encryptAccountTokenColumns({ accessToken: 'access' }) + expect(accessToken?.startsWith('simenc:v1:')).toBe(true) + }) + + it('writes plaintext when the flag is off', async () => { + mockIsFeatureEnabled.mockResolvedValue(false) + await expect(encryptAccountTokenColumns({ accessToken: 'access' })).resolves.toEqual({ + accessToken: 'access', + }) + }) + + /** + * A deployment can boot with a key this codebase rejects on first use, and losing the + * user's connect is worse than storing plaintext — so the gate degrades, never throws. + */ + it.each([ + ['the self-hosted placeholder key', SELF_HOSTED_BAD_KEY], + ['a 64-char non-hex key', 'z'.repeat(64)], + ['a short key', 'abcdef'], + ['an unset key', undefined], + ])('writes plaintext with %s even when the flag is on', async (_label, key) => { + setEnv({ ENCRYPTION_KEY: key }) + await expect(encryptAccountTokenColumns({ accessToken: 'access' })).resolves.toEqual({ + accessToken: 'access', + }) + }) + + /** Passing a context would make the flag able to hit the database on every token write. */ + it('resolves the flag with no context object', async () => { + await encryptAccountTokenColumns({ accessToken: 'access' }) + expect(mockIsFeatureEnabled).toHaveBeenCalledWith('oauth-token-encryption') + }) + + it('never consults the flag on the read path', async () => { + await decryptAccountTokenColumns({ accessToken: 'token.placeholder-value' }) + expect(mockIsFeatureEnabled).not.toHaveBeenCalled() + }) +}) + +describe('encryptAccountTokenColumns', () => { + it('envelopes all three token fields', async () => { + const result = await encryptAccountTokenColumns({ + accessToken: 'access', + refreshToken: 'refresh', + idToken: 'id', + }) + + for (const value of Object.values(result)) { + expect(value?.startsWith('simenc:v1:')).toBe(true) + } + }) + + /** + * Better Auth's `update.before` also fires for password changes: `updatePassword` + * routes through `updateManyWithHooks` with a `{ password }`-only payload. + */ + it('leaves a password-only update payload completely untouched', async () => { + const payload = { password: passwordHashPlaceholder } + await expect(encryptAccountTokenColumns(payload)).resolves.toEqual(payload) + }) + + it('preserves non-token keys alongside the tokens', async () => { + const result = await encryptAccountTokenColumns({ + accessToken: 'access', + scope: 'read write', + providerId: 'google-drive', + }) + + expect(result.scope).toBe('read write') + expect(result.providerId).toBe('google-drive') + }) + + it.each([ + ['null', null], + ['undefined', undefined], + ['empty string (SAML rows)', ''], + ])('passes a %s token through unchanged', async (_label, value) => { + const result = await encryptAccountTokenColumns({ accessToken: value }) + expect(result.accessToken).toBe(value) + }) + + it('does not introduce a key that was absent from the payload', async () => { + const result = await encryptAccountTokenColumns({ accessToken: 'access' }) + expect('refreshToken' in result).toBe(false) + expect('idToken' in result).toBe(false) + }) + + it('is idempotent, so Better Auth write-backs never double-wrap', async () => { + const once = await encryptAccountTokenColumns({ accessToken: 'access' }) + const twice = await encryptAccountTokenColumns(once) + expect(twice.accessToken).toBe(once.accessToken) + }) + + it('does not mutate the input object', async () => { + const input = { accessToken: 'access' } + await encryptAccountTokenColumns(input) + expect(input.accessToken).toBe('access') + }) +}) + +describe('decryptAccountTokenColumns', () => { + it('recovers enveloped tokens', async () => { + const encrypted = await encryptAccountTokenColumns({ + accessToken: 'access', + refreshToken: 'refresh', + }) + const result = await decryptAccountTokenColumns(encrypted) + + expect(result.accessToken).toBe('access') + expect(result.refreshToken).toBe('refresh') + }) + + /** The mixed-format guarantee that makes the rollout and the backfill safe. */ + it('returns legacy plaintext unchanged, regardless of the flag', async () => { + mockIsFeatureEnabled.mockResolvedValue(false) + const result = await decryptAccountTokenColumns({ + accessToken: 'token.placeholder-value', + refreshToken: null, + }) + + expect(result.accessToken).toBe('token.placeholder-value') + }) + + it('reads a row that is half migrated', async () => { + const result = await decryptAccountTokenColumns({ + accessToken: await encryptAccountToken('fresh'), + refreshToken: 'legacy-plaintext', + }) + + expect(result.accessToken).toBe('fresh') + expect(result.refreshToken).toBe('legacy-plaintext') + }) + + it('nulls an unrecoverable field and never throws', async () => { + const encrypted = await encryptAccountToken('refresh') + setEnv({ ENCRYPTION_KEY: 'f'.repeat(64) }) + + const result = await decryptAccountTokenColumns({ + accessToken: 'plaintext-still-fine', + refreshToken: encrypted, + }) + + expect(result.refreshToken).toBeNull() + expect(result.accessToken).toBe('plaintext-still-fine') + }) + + it('does not mutate the input row', async () => { + const encrypted = await encryptAccountToken('access') + const input = { accessToken: encrypted } + await decryptAccountTokenColumns(input) + expect(input.accessToken).toBe(encrypted) + }) + + it.each([ + ['null', null], + ['undefined', undefined], + ['empty string', ''], + ])('passes a %s token through unchanged', async (_label, value) => { + const result = await decryptAccountTokenColumns({ accessToken: value }) + expect(result.accessToken).toBe(value) + }) +}) + +describe('decryptAccountTokenColumnsBatch', () => { + it('decrypts every row and isolates a poisoned one', async () => { + const good = await encryptAccountTokenColumns({ accessToken: 'good' }) + const poisoned = { accessToken: 'simenc:v1:deadbeef' } + + const [first, second, third] = await decryptAccountTokenColumnsBatch([ + good, + poisoned, + { accessToken: 'legacy' }, + ]) + + expect(first.accessToken).toBe('good') + expect(second.accessToken).toBeNull() + expect(third.accessToken).toBe('legacy') + }) +}) diff --git a/apps/sim/lib/oauth/account-tokens.ts b/apps/sim/lib/oauth/account-tokens.ts new file mode 100644 index 00000000000..ff39ae17bd9 --- /dev/null +++ b/apps/sim/lib/oauth/account-tokens.ts @@ -0,0 +1,123 @@ +import { createLogger } from '@sim/logger' +import { getErrorMessage } from '@sim/utils/errors' +import { isFeatureEnabled } from '@/lib/core/config/feature-flags' +import { hasUsableEncryptionKey } from '@/lib/core/security/encryption' +import { + AccountTokenDecryptionError, + type AccountTokenField, + decryptAccountToken, + encryptAccountToken, +} from '@/lib/oauth/account-token-crypto' + +const logger = createLogger('AccountTokens') + +const TOKEN_FIELDS = ['accessToken', 'refreshToken', 'idToken'] as const + +/** The three protected columns on the `account` table. */ +export type AccountTokenColumns = { + [K in AccountTokenField]: string | null +} + +declare const decryptedTokensBrand: unique symbol + +/** + * A row whose token columns hold plaintext. The brand is minted only by this module, so a + * raw Drizzle row will not type-check where a decrypted one is required. + * + * A brand cannot stop an `any`, which is why `refreshTokenIfNeeded`'s parameter was typed + * rather than annotated. The durable guarantee is structural: only the modules allowlisted + * in `scripts/check-account-token-access.ts` may select a token column at all. + */ +export type DecryptedAccount = T & { readonly [decryptedTokensBrand]: true } + +/** + * True when new writes should be enveloped. Reads never consult this — they detect the + * format per value — so flipping the flag can never strand a row. + */ +async function canEncryptAccountTokens(): Promise { + if (!(await isFeatureEnabled('oauth-token-encryption'))) return false + if (!hasUsableEncryptionKey()) { + warnOnceAboutUnusableKey() + return false + } + return true +} + +let warnedAboutUnusableKey = false + +function warnOnceAboutUnusableKey(): void { + if (warnedAboutUnusableKey) return + warnedAboutUnusableKey = true + logger.error( + 'oauth-token-encryption is enabled but ENCRYPTION_KEY is not a 64-character hex string; account tokens will continue to be written in plaintext' + ) +} + +/** + * Envelopes whichever token fields are present, leaving every other key untouched. + * + * Field-local by necessity: `update.before` also fires for password changes and for + * expiry-only updates, so an absent field must stay absent rather than being coerced to + * `null` — that would blank the user's tokens. A payload carrying no token at all short- + * circuits before the flag, since whether it needs encrypting is not a flag question. + */ +export async function encryptAccountTokenColumns>( + data: T +): Promise { + if (!TOKEN_FIELDS.some((field) => data[field])) return data + if (!(await canEncryptAccountTokens())) return data + + const next = { ...data } + for (const field of TOKEN_FIELDS) { + const value = data[field] + if (typeof value !== 'string' || value === '') continue + try { + next[field] = (await encryptAccountToken(value)) as T[typeof field] + } catch (error) { + logger.error('Failed to encrypt account token; storing plaintext', { + field, + error: getErrorMessage(error), + }) + } + } + return next +} + +/** + * Recovers whichever token fields are present. Never throws — an unreadable field becomes + * `null`, so one poisoned row cannot fail a whole list surface. + * + * The stored row is never mutated. A decrypt failure is a key or configuration fault, and + * this codebase has no key rotation, so nulling the column would turn a recoverable + * misconfiguration into unrecoverable credential loss. Callers that dead-flag a credential + * must treat the `null` as "unavailable", never as "revoked" — see `getFreshestSlackChain`, + * where a bad key would otherwise block a whole Slack installation for an hour. + */ +export async function decryptAccountTokenColumns>( + row: T +): Promise> { + const next = { ...row } as T + + for (const field of TOKEN_FIELDS) { + const value = row[field] + if (typeof value !== 'string' || value === '') continue + try { + next[field] = (await decryptAccountToken(value, field)) as T[typeof field] + } catch (error) { + next[field] = null as T[typeof field] + logger.error('Failed to decrypt account token', { + field, + reason: error instanceof AccountTokenDecryptionError ? error.reason : 'unknown', + }) + } + } + + return next as DecryptedAccount +} + +/** Batch form for list surfaces (the connections page, the Copilot credential tool). */ +export async function decryptAccountTokenColumnsBatch>( + rows: T[] +): Promise[]> { + return Promise.all(rows.map((row) => decryptAccountTokenColumns(row))) +} diff --git a/apps/sim/lib/oauth/credential-service.ts b/apps/sim/lib/oauth/credential-service.ts index a92468715e6..b80d5b41e2c 100644 --- a/apps/sim/lib/oauth/credential-service.ts +++ b/apps/sim/lib/oauth/credential-service.ts @@ -3,6 +3,7 @@ import { db } from '@sim/db' import { account, credential } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { getPostgresErrorCode, toError } from '@sim/utils/errors' +import { generateId } from '@sim/utils/id' import { and, desc, eq } from 'drizzle-orm' import { withLeaderLock } from '@/lib/concurrency/leader-lock' import { coalesceLocally } from '@/lib/concurrency/singleflight' @@ -20,13 +21,14 @@ import { parseTokenServiceAccountSecretBlob, type TokenServiceAccountSecretBlob, } from '@/lib/credentials/token-service-accounts/server' -import { isInstagramProvider, shouldProactivelyRefreshInstagramToken } from '@/lib/oauth/instagram' import { - getMicrosoftRefreshTokenExpiry, - isMicrosoftProvider, - PROACTIVE_REFRESH_THRESHOLD_DAYS, -} from '@/lib/oauth/microsoft' + type DecryptedAccount, + decryptAccountTokenColumns, + encryptAccountTokenColumns, +} from '@/lib/oauth/account-tokens' +import { getMicrosoftRefreshTokenExpiry, isMicrosoftProvider } from '@/lib/oauth/microsoft' import { refreshOAuthToken } from '@/lib/oauth/oauth' +import { decideTokenRefresh } from '@/lib/oauth/refresh-policy' import { extractSlackTeamId, fanOutSlackTokenChain, @@ -644,6 +646,50 @@ export async function resolveServiceAccountToken( return resolver(credentialId, { scopes, impersonateEmail }) } +/** + * Everything a credential consumer needs, and nothing else — notably not `password`, which + * a bare `select()` would carry into every object built from one of these rows. + */ +const OAUTH_CREDENTIAL_COLUMNS = { + id: account.id, + userId: account.userId, + providerId: account.providerId, + accountId: account.accountId, + scope: account.scope, + accessToken: account.accessToken, + refreshToken: account.refreshToken, + idToken: account.idToken, + accessTokenExpiresAt: account.accessTokenExpiresAt, + refreshTokenExpiresAt: account.refreshTokenExpiresAt, + updatedAt: account.updatedAt, +} as const + +/** + * Finds the account row a provider's external identity maps to, projecting only its id. + * + * The connect flows that bypass Better Auth (Shopify, Instagram, Trello) each need this + * before deciding between update and insert, and again to resolve the persisted row. They + * must not select the whole row: that pulls the encrypted token columns for no reason. + */ +export async function findAccountIdByProviderAccount(params: { + userId: string + providerId: string + externalAccountId: string +}): Promise<{ id: string } | undefined> { + const [row] = await db + .select({ id: account.id }) + .from(account) + .where( + and( + eq(account.userId, params.userId), + eq(account.providerId, params.providerId), + eq(account.accountId, params.externalAccountId) + ) + ) + .limit(1) + return row +} + /** * Safely inserts an account record, handling duplicate constraint violations gracefully. * If a duplicate is detected (unique constraint violation), logs a warning and returns success. @@ -653,7 +699,7 @@ export async function safeAccountInsert( context: { provider: string; identifier?: string } ): Promise { try { - await db.insert(account).values(data) + await db.insert(account).values(await encryptAccountTokenColumns(data)) logger.info(`Created new ${context.provider} account for user`, { userId: data.userId }) } catch (error: any) { if (getPostgresErrorCode(error) === '23505') { @@ -667,23 +713,88 @@ export async function safeAccountInsert( } } +/** + * The single write path for the connect flows that bypass Better Auth — Shopify, Instagram + * and Trello, which mint tokens themselves rather than going through an OAuth callback the + * `databaseHooks` can intercept. + * + * Routing them through here is what keeps encryption from being something each new provider + * has to remember: `scripts/check-account-token-access.ts` refuses a direct + * `db.insert(account)` / `db.update(account)` outside this module, so the next connect flow + * cannot quietly store a plaintext token. + */ +export async function upsertProviderAccountTokens(params: { + userId: string + providerId: string + externalAccountId: string + scope: string + tokens: { accessToken: string; refreshToken?: string; idToken?: string } + accessTokenExpiresAt?: Date + /** Human-readable identifier for log lines, when the external id is not recognisable. */ + logIdentifier?: string +}): Promise<{ accountId: string }> { + const { userId, providerId, externalAccountId, scope, tokens, accessTokenExpiresAt } = params + const identifier = params.logIdentifier ?? externalAccountId + const now = new Date() + const existing = await findAccountIdByProviderAccount({ userId, providerId, externalAccountId }) + + if (existing) { + await db + .update(account) + .set({ + ...(await encryptAccountTokenColumns(tokens)), + accountId: externalAccountId, + scope, + ...(accessTokenExpiresAt ? { accessTokenExpiresAt } : {}), + updatedAt: now, + }) + .where(eq(account.id, existing.id)) + logger.info(`Updated existing ${providerId} account`, { accountId: existing.id, identifier }) + return { accountId: existing.id } + } + + await safeAccountInsert( + { + id: generateId(), + userId, + providerId, + accountId: externalAccountId, + scope, + ...tokens, + ...(accessTokenExpiresAt ? { accessTokenExpiresAt } : {}), + createdAt: now, + updatedAt: now, + }, + { provider: providerId, identifier } + ) + + /** `safeAccountInsert` swallows a duplicate-key race, so the row may be someone else's insert. */ + const persisted = await findAccountIdByProviderAccount({ userId, providerId, externalAccountId }) + if (!persisted) { + throw new Error(`${providerId} OAuth account ${externalAccountId} was not persisted`) + } + return { accountId: persisted.id } +} + /** * Get a credential by resolved account ID and verify it belongs to the user. */ async function getCredentialByAccountId(requestId: string, accountId: string, userId: string) { - const credentials = await db - .select() + const rows = await db + .select(OAUTH_CREDENTIAL_COLUMNS) .from(account) .where(and(eq(account.id, accountId), eq(account.userId, userId))) .limit(1) - if (!credentials.length) { + if (!rows.length) { logger.warn(`[${requestId}] Credential not found`) return undefined } + const credential = await decryptAccountTokenColumns(rows[0]) + return { - ...credentials[0], + ...credential, resolvedCredentialId: accountId, } } @@ -836,14 +947,24 @@ async function performCoalescedRefresh({ { ifChainUnchangedSince: slackChainVersion ?? undefined } ) } else { + /** + * Compare plaintext to plaintext. If either side became ciphertext they would + * never match, so every refresh would write and perturb `updated_at` — which + * Slack's fan-out guard and Instagram's minimum-token-age gate both read. + */ + const rotatedRefreshToken = + result.refreshToken && result.refreshToken !== refreshToken + ? result.refreshToken + : undefined + const updateData: Record = { - accessToken: result.accessToken, + ...(await encryptAccountTokenColumns({ + accessToken: result.accessToken, + ...(rotatedRefreshToken ? { refreshToken: rotatedRefreshToken } : {}), + })), accessTokenExpiresAt, updatedAt: new Date(), } - if (result.refreshToken && result.refreshToken !== refreshToken) { - updateData.refreshToken = result.refreshToken - } if (isMicrosoftProvider(providerId)) { updateData.refreshTokenExpiresAt = getMicrosoftRefreshTokenExpiry() } @@ -863,7 +984,7 @@ async function performCoalescedRefresh({ }, onFollower: async () => { try { - const [row] = await db + const [stored] = await db .select({ accessToken: account.accessToken, accessTokenExpiresAt: account.accessTokenExpiresAt, @@ -871,8 +992,12 @@ async function performCoalescedRefresh({ .from(account) .where(eq(account.id, accountId)) .limit(1) + if (!stored) return null + + /** The leader may have written ciphertext while this follower polled. */ + const row = await decryptAccountTokenColumns(stored) if ( - row?.accessToken && + row.accessToken && row.accessTokenExpiresAt && row.accessTokenExpiresAt > new Date() ) { @@ -910,8 +1035,7 @@ export async function getOAuthToken(userId: string, providerId: string): Promise accessToken: account.accessToken, refreshToken: account.refreshToken, accessTokenExpiresAt: account.accessTokenExpiresAt, - idToken: account.idToken, - scope: account.scope, + refreshTokenExpiresAt: account.refreshTokenExpiresAt, updatedAt: account.updatedAt, }) .from(account) @@ -924,24 +1048,18 @@ export async function getOAuthToken(userId: string, providerId: string): Promise return null } - const credential = connections[0] + const credential = await decryptAccountTokenColumns(connections[0]) - // Determine whether we should refresh: missing/expired token, or Instagram - // long-lived token nearing expiry (Meta cannot refresh after expiry). - const now = new Date() - const tokenExpiry = credential.accessTokenExpiresAt - const accessTokenNeedsRefresh = - !!credential.refreshToken && (!credential.accessToken || (tokenExpiry && tokenExpiry < now)) - const instagramNeedsProactiveRefresh = - !!credential.refreshToken && - isInstagramProvider(providerId) && - shouldProactivelyRefreshInstagramToken({ - accessTokenExpiresAt: credential.accessTokenExpiresAt, - updatedAt: credential.updatedAt, - now, - }) + const decision = decideTokenRefresh({ + providerId, + hasAccessToken: !!credential.accessToken, + hasRefreshToken: !!credential.refreshToken, + accessTokenExpiresAt: credential.accessTokenExpiresAt, + refreshTokenExpiresAt: credential.refreshTokenExpiresAt, + updatedAt: credential.updatedAt, + }) - if (accessTokenNeedsRefresh || instagramNeedsProactiveRefresh) { + if (decision.shouldRefresh) { const fresh = await performCoalescedRefresh({ accountId: credential.id, providerId, @@ -950,7 +1068,7 @@ export async function getOAuthToken(userId: string, providerId: string): Promise userId, }) if (fresh) return fresh - if (!accessTokenNeedsRefresh && credential.accessToken) { + if (!decision.accessTokenRequired && credential.accessToken) { return credential.accessToken } return null @@ -1005,72 +1123,15 @@ export async function resolveCredentialAccessToken( return null } - // Decide if we should refresh: token missing OR expired - const accessTokenExpiresAt = credential.accessTokenExpiresAt - const refreshTokenExpiresAt = credential.refreshTokenExpiresAt - const now = new Date() - - // Check if access token needs refresh (missing or expired) - const accessTokenNeedsRefresh = - !!credential.refreshToken && - (!credential.accessToken || (accessTokenExpiresAt && accessTokenExpiresAt <= now)) - - // Check if we should proactively refresh to prevent refresh token expiry - // This applies to Microsoft providers whose refresh tokens expire after 90 days of inactivity - const proactiveRefreshThreshold = new Date( - now.getTime() + PROACTIVE_REFRESH_THRESHOLD_DAYS * 24 * 60 * 60 * 1000 - ) - const refreshTokenNeedsProactiveRefresh = - !!credential.refreshToken && - isMicrosoftProvider(credential.providerId) && - refreshTokenExpiresAt && - refreshTokenExpiresAt <= proactiveRefreshThreshold - - // Instagram long-lived tokens can only be refreshed while still valid. - const instagramNeedsProactiveRefresh = - !!credential.refreshToken && - isInstagramProvider(credential.providerId) && - shouldProactivelyRefreshInstagramToken({ - accessTokenExpiresAt, - updatedAt: credential.updatedAt, - now, - }) - - const shouldRefresh = - accessTokenNeedsRefresh || refreshTokenNeedsProactiveRefresh || instagramNeedsProactiveRefresh - - const accessToken = credential.accessToken - - if (shouldRefresh) { - const resolvedCredentialId = - (credential as { resolvedCredentialId?: string }).resolvedCredentialId ?? credentialId - - const fresh = await performCoalescedRefresh({ - accountId: resolvedCredentialId, - providerId: credential.providerId, - refreshToken: credential.refreshToken!, - providerAccountId: credential.accountId, - requestId, - userId: credential.userId, + try { + const { accessToken } = await refreshTokenIfNeeded(requestId, credential, credentialId) + return { accessToken } + } catch (error) { + logger.error(`[${requestId}] Could not resolve an access token for credential`, { + error: toError(error).message, }) - if (fresh) return { accessToken: fresh } - - // If refresh was only triggered proactively (Microsoft refresh-token aging / - // Instagram long-lived nearing expiry), the still-valid access token is fine. - if (!accessTokenNeedsRefresh && accessToken) { - logger.info(`[${requestId}] Refresh unavailable; reusing still-valid access token`) - return { accessToken } - } return null } - if (!accessToken) { - // We have no access token and either no refresh token or not eligible to refresh - logger.error(`[${requestId}] Missing access token for credential`) - return null - } - - logger.info(`[${requestId}] Access token is valid for credential`) - return { accessToken } } /** @@ -1100,52 +1161,73 @@ export async function refreshAccessTokenIfNeeded( return result?.accessToken ?? null } +/** A loaded `account` row whose tokens are already decrypted. The brand rejects a raw row. */ +export type LoadedOAuthCredential = DecryptedAccount<{ + providerId: string + accountId: string + userId: string + accessToken: string | null + refreshToken: string | null + idToken: string | null + accessTokenExpiresAt: Date | null + refreshTokenExpiresAt: Date | null + updatedAt: Date + resolvedCredentialId?: string +}> + +/** Loads an account row, decrypts it, and resolves its access token per the shared policy. */ +export async function resolveAccessTokenForAccount( + requestId: string, + accountId: string +): Promise { + const [row] = await db + .select(OAUTH_CREDENTIAL_COLUMNS) + .from(account) + .where(eq(account.id, accountId)) + .limit(1) + if (!row) { + logger.warn(`[${requestId}] Account not found`, { accountId }) + return null + } + + const credential = await decryptAccountTokenColumns(row) + try { + const { accessToken } = await refreshTokenIfNeeded(requestId, credential, accountId) + return accessToken + } catch (error) { + logger.error(`[${requestId}] Failed to resolve access token`, { + accountId, + error: toError(error).message, + }) + return null + } +} + /** - * Enhanced version that returns additional information about the refresh operation + * Refreshes if the shared policy says so, reporting whether it did. See + * {@link resolveAccessTokenForAccount} when you only have an account id. */ export async function refreshTokenIfNeeded( requestId: string, - credential: any, + credential: LoadedOAuthCredential, credentialId: string ): Promise<{ accessToken: string; refreshed: boolean }> { const resolvedCredentialId = credential.resolvedCredentialId ?? credentialId - // Decide if we should refresh: token missing OR expired - const accessTokenExpiresAt = credential.accessTokenExpiresAt - const refreshTokenExpiresAt = credential.refreshTokenExpiresAt - const now = new Date() - - // Check if access token needs refresh (missing or expired) - const accessTokenNeedsRefresh = - !!credential.refreshToken && - (!credential.accessToken || (accessTokenExpiresAt && accessTokenExpiresAt <= now)) - - // Check if we should proactively refresh to prevent refresh token expiry - // This applies to Microsoft providers whose refresh tokens expire after 90 days of inactivity - const proactiveRefreshThreshold = new Date( - now.getTime() + PROACTIVE_REFRESH_THRESHOLD_DAYS * 24 * 60 * 60 * 1000 - ) - const refreshTokenNeedsProactiveRefresh = - !!credential.refreshToken && - isMicrosoftProvider(credential.providerId) && - refreshTokenExpiresAt && - refreshTokenExpiresAt <= proactiveRefreshThreshold - - // Instagram long-lived tokens can only be refreshed while still valid. - const instagramNeedsProactiveRefresh = - !!credential.refreshToken && - isInstagramProvider(credential.providerId) && - shouldProactivelyRefreshInstagramToken({ - accessTokenExpiresAt, - updatedAt: credential.updatedAt, - now, - }) - - const shouldRefresh = - accessTokenNeedsRefresh || refreshTokenNeedsProactiveRefresh || instagramNeedsProactiveRefresh + const decision = decideTokenRefresh({ + providerId: credential.providerId, + hasAccessToken: !!credential.accessToken, + hasRefreshToken: !!credential.refreshToken, + accessTokenExpiresAt: credential.accessTokenExpiresAt, + refreshTokenExpiresAt: credential.refreshTokenExpiresAt, + updatedAt: credential.updatedAt, + }) - // If token appears valid and present, return it directly - if (!shouldRefresh) { + if (!decision.shouldRefresh) { + /** Previously returned `{ accessToken: null }` — the parameter was `any` — and callers passed it to a provider. */ + if (!credential.accessToken) { + throw new Error('Credential has no access token and cannot be refreshed') + } logger.info(`[${requestId}] Access token is valid`) return { accessToken: credential.accessToken, refreshed: false } } @@ -1160,7 +1242,7 @@ export async function refreshTokenIfNeeded( }) if (fresh) return { accessToken: fresh, refreshed: true } - if (!accessTokenNeedsRefresh && credential.accessToken) { + if (!decision.accessTokenRequired && credential.accessToken) { logger.info(`[${requestId}] Refresh unavailable; reusing still-valid access token`) return { accessToken: credential.accessToken, refreshed: false } } diff --git a/apps/sim/lib/oauth/refresh-policy.test.ts b/apps/sim/lib/oauth/refresh-policy.test.ts new file mode 100644 index 00000000000..7d1e88f45fb --- /dev/null +++ b/apps/sim/lib/oauth/refresh-policy.test.ts @@ -0,0 +1,191 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { decideTokenRefresh, type RefreshDecisionInput } from '@/lib/oauth/refresh-policy' + +const NOW = new Date('2026-08-28T12:00:00.000Z') +const DAY_MS = 24 * 60 * 60 * 1000 + +function input(overrides: Partial = {}): RefreshDecisionInput { + return { + providerId: 'google-drive', + hasAccessToken: true, + hasRefreshToken: true, + accessTokenExpiresAt: new Date(NOW.getTime() + DAY_MS), + refreshTokenExpiresAt: null, + updatedAt: new Date(NOW.getTime() - 30 * DAY_MS), + now: NOW, + ...overrides, + } +} + +describe('decideTokenRefresh', () => { + it('leaves a valid token alone', () => { + expect(decideTokenRefresh(input())).toEqual({ + shouldRefresh: false, + accessTokenRequired: false, + reason: 'valid', + }) + }) + + it('never refreshes without a refresh token, however stale the access token', () => { + expect( + decideTokenRefresh( + input({ + hasRefreshToken: false, + hasAccessToken: false, + accessTokenExpiresAt: new Date(NOW.getTime() - DAY_MS), + }) + ) + ).toEqual({ shouldRefresh: false, accessTokenRequired: false, reason: 'valid' }) + }) + + it('treats a null expiry on a present access token as valid', () => { + expect(decideTokenRefresh(input({ accessTokenExpiresAt: null }))).toMatchObject({ + shouldRefresh: false, + reason: 'valid', + }) + }) + + it('refreshes when the access token is missing', () => { + expect(decideTokenRefresh(input({ hasAccessToken: false }))).toEqual({ + shouldRefresh: true, + accessTokenRequired: true, + reason: 'access-token-missing', + }) + }) + + it('refreshes when the access token has expired', () => { + expect( + decideTokenRefresh(input({ accessTokenExpiresAt: new Date(NOW.getTime() - 1) })) + ).toEqual({ + shouldRefresh: true, + accessTokenRequired: true, + reason: 'access-token-expired', + }) + }) + + /** + * `getOAuthToken` previously used `<`, so a token expiring exactly at `now` was + * treated as live and shipped to the provider a moment before it died. + */ + it('refreshes a token expiring exactly now', () => { + expect(decideTokenRefresh(input({ accessTokenExpiresAt: NOW }))).toMatchObject({ + shouldRefresh: true, + reason: 'access-token-expired', + }) + }) + + describe('Microsoft refresh-token aging', () => { + const microsoft = (refreshTokenExpiresAt: Date | null) => + decideTokenRefresh(input({ providerId: 'outlook', refreshTokenExpiresAt })) + + it('proactively refreshes inside the 7-day window without requiring a new access token', () => { + expect(microsoft(new Date(NOW.getTime() + 6 * DAY_MS))).toEqual({ + shouldRefresh: true, + accessTokenRequired: false, + reason: 'microsoft-refresh-token-aging', + }) + }) + + it('leaves a refresh token outside the window alone', () => { + expect(microsoft(new Date(NOW.getTime() + 30 * DAY_MS))).toMatchObject({ + shouldRefresh: false, + reason: 'valid', + }) + }) + + it('does nothing when the refresh-token expiry is unknown', () => { + expect(microsoft(null)).toMatchObject({ shouldRefresh: false, reason: 'valid' }) + }) + + /** + * The delta this consolidation intentionally introduces: `getOAuthToken` omitted + * this arm, so Microsoft credentials reached only through it could pass the 90-day + * inactivity deadline and die permanently. + */ + it('applies to a non-Microsoft provider not at all', () => { + expect( + decideTokenRefresh( + input({ + providerId: 'google-drive', + refreshTokenExpiresAt: new Date(NOW.getTime() + DAY_MS), + }) + ) + ).toMatchObject({ shouldRefresh: false, reason: 'valid' }) + }) + }) + + describe('Instagram long-lived aging', () => { + const instagram = (overrides: Partial) => + decideTokenRefresh(input({ providerId: 'instagram', ...overrides })) + + it('proactively refreshes inside the 14-day window', () => { + expect( + instagram({ + accessTokenExpiresAt: new Date(NOW.getTime() + 10 * DAY_MS), + updatedAt: new Date(NOW.getTime() - 30 * DAY_MS), + }) + ).toEqual({ + shouldRefresh: true, + accessTokenRequired: false, + reason: 'instagram-long-lived-aging', + }) + }) + + /** Meta rejects refresh until the token is 24h old. */ + it('holds off on a token younger than 24 hours', () => { + expect( + instagram({ + accessTokenExpiresAt: new Date(NOW.getTime() + 10 * DAY_MS), + updatedAt: new Date(NOW.getTime() - 60 * 60 * 1000), + }) + ).toMatchObject({ shouldRefresh: false, reason: 'valid' }) + }) + + it('leaves a token outside the window alone', () => { + expect( + instagram({ accessTokenExpiresAt: new Date(NOW.getTime() + 30 * DAY_MS) }) + ).toMatchObject({ shouldRefresh: false, reason: 'valid' }) + }) + + /** + * An already-expired Instagram token is unrecoverable, so it falls through to the + * ordinary expired-access-token arm rather than the proactive one. + */ + it('reports an already-expired token as expired, not aging', () => { + expect(instagram({ accessTokenExpiresAt: new Date(NOW.getTime() - DAY_MS) })).toMatchObject({ + shouldRefresh: true, + reason: 'access-token-expired', + }) + }) + }) + + /** + * `accessTokenRequired` is what lets a caller reuse its stored token when a refresh + * fails. Getting it wrong turns a recoverable proactive refresh failure into a hard + * credential error. + */ + describe('accessTokenRequired', () => { + it.each([ + ['access token missing', input({ hasAccessToken: false }), true], + ['access token expired', input({ accessTokenExpiresAt: new Date(NOW.getTime() - 1) }), true], + [ + 'Microsoft proactive', + input({ providerId: 'outlook', refreshTokenExpiresAt: new Date(NOW.getTime() + DAY_MS) }), + false, + ], + [ + 'Instagram proactive', + input({ + providerId: 'instagram', + accessTokenExpiresAt: new Date(NOW.getTime() + 10 * DAY_MS), + }), + false, + ], + ])('is %s => %s', (_label, decision, expected) => { + expect(decideTokenRefresh(decision).accessTokenRequired).toBe(expected) + }) + }) +}) diff --git a/apps/sim/lib/oauth/refresh-policy.ts b/apps/sim/lib/oauth/refresh-policy.ts new file mode 100644 index 00000000000..3a8501b3209 --- /dev/null +++ b/apps/sim/lib/oauth/refresh-policy.ts @@ -0,0 +1,98 @@ +import { isInstagramProvider, shouldProactivelyRefreshInstagramToken } from '@/lib/oauth/instagram' +import { isMicrosoftProvider, PROACTIVE_REFRESH_THRESHOLD_DAYS } from '@/lib/oauth/microsoft' + +const DAY_MS = 24 * 60 * 60 * 1000 + +/** Why {@link decideTokenRefresh} reached its verdict. Diagnostic only — never branched on. */ +export type RefreshReason = + | 'valid' + | 'access-token-missing' + | 'access-token-expired' + | 'microsoft-refresh-token-aging' + | 'instagram-long-lived-aging' + +export interface RefreshDecisionInput { + providerId: string + hasAccessToken: boolean + hasRefreshToken: boolean + accessTokenExpiresAt: Date | null + refreshTokenExpiresAt: Date | null + /** Last write to the row. Only Instagram's minimum-token-age gate reads this. */ + updatedAt: Date | null + now?: Date +} + +export interface RefreshDecision { + shouldRefresh: boolean + /** + * True when the stored access token is unusable, so a failed refresh cannot fall back to + * it. False for a purely proactive refresh, where the caller should reuse what it has. + */ + accessTokenRequired: boolean + reason: RefreshReason +} + +/** + * The single staleness rule for OAuth credentials backed by the `account` table. + * + * Replaces three copies in `credential-service.ts`. `getOAuthToken`'s omitted the Microsoft + * proactive-refresh arm, so credentials reached only that way could pass Microsoft's 90-day + * inactivity deadline and die. Unifying on the stricter rule fixes that. + */ +export function decideTokenRefresh(input: RefreshDecisionInput): RefreshDecision { + const now = input.now ?? new Date() + + if (!input.hasRefreshToken) { + return { + shouldRefresh: false, + accessTokenRequired: false, + reason: 'valid', + } + } + + if (!input.hasAccessToken) { + return { + shouldRefresh: true, + accessTokenRequired: true, + reason: 'access-token-missing', + } + } + + if (input.accessTokenExpiresAt && input.accessTokenExpiresAt <= now) { + return { + shouldRefresh: true, + accessTokenRequired: true, + reason: 'access-token-expired', + } + } + + /** Microsoft refresh tokens die after 90 days of inactivity; refresh ahead of the window. */ + if (isMicrosoftProvider(input.providerId) && input.refreshTokenExpiresAt) { + const threshold = new Date(now.getTime() + PROACTIVE_REFRESH_THRESHOLD_DAYS * DAY_MS) + if (input.refreshTokenExpiresAt <= threshold) { + return { + shouldRefresh: true, + accessTokenRequired: false, + reason: 'microsoft-refresh-token-aging', + } + } + } + + /** Meta cannot refresh an Instagram long-lived token once it has expired. */ + if ( + isInstagramProvider(input.providerId) && + shouldProactivelyRefreshInstagramToken({ + accessTokenExpiresAt: input.accessTokenExpiresAt, + updatedAt: input.updatedAt, + now, + }) + ) { + return { + shouldRefresh: true, + accessTokenRequired: false, + reason: 'instagram-long-lived-aging', + } + } + + return { shouldRefresh: false, accessTokenRequired: false, reason: 'valid' } +} diff --git a/apps/sim/lib/oauth/shopify.ts b/apps/sim/lib/oauth/shopify.ts index 29a7299b874..324eed56062 100644 --- a/apps/sim/lib/oauth/shopify.ts +++ b/apps/sim/lib/oauth/shopify.ts @@ -1,10 +1,6 @@ -import { db } from '@sim/db' -import { account } from '@sim/db/schema' import { createLogger } from '@sim/logger' -import { generateId } from '@sim/utils/id' -import { and, eq } from 'drizzle-orm' import { processCredentialDraft } from '@/lib/credentials/draft-processor' -import { safeAccountInsert } from '@/lib/oauth/credential-service' +import { upsertProviderAccountTokens } from '@/lib/oauth/credential-service' import { SHOPIFY_API_VERSION } from '@/tools/shopify/constants' const logger = createLogger('ShopifyOAuth') @@ -54,61 +50,21 @@ export async function completeShopifyOAuthConnection( } const stableAccountId = getShopifyAccountId(await shopResponse.json()) - const existing = await db.query.account.findFirst({ - where: and( - eq(account.userId, params.userId), - eq(account.providerId, 'shopify'), - eq(account.accountId, stableAccountId) - ), - }) - const now = new Date() - const accountData = { - accessToken: params.accessToken, - accountId: stableAccountId, + const { accountId } = await upsertProviderAccountTokens({ + userId: params.userId, + providerId: 'shopify', + externalAccountId: stableAccountId, scope: params.scope ?? '', - updatedAt: now, - idToken: params.shopDomain, - } - - if (existing) { - await db.update(account).set(accountData).where(eq(account.id, existing.id)) - logger.info('Updated existing Shopify account', { accountId: existing.id }) - } else { - await safeAccountInsert( - { - id: generateId(), - userId: params.userId, - providerId: 'shopify', - accountId: accountData.accountId, - accessToken: accountData.accessToken, - scope: accountData.scope, - idToken: accountData.idToken, - createdAt: now, - updatedAt: now, - }, - { provider: 'Shopify', identifier: params.shopDomain } - ) - } - - const persisted = - existing ?? - (await db.query.account.findFirst({ - where: and( - eq(account.userId, params.userId), - eq(account.providerId, 'shopify'), - eq(account.accountId, stableAccountId) - ), - })) - - if (!persisted) { - throw new Error(`Shopify OAuth account ${stableAccountId} was not persisted`) - } + /** Shopify has no refresh token; `idToken` carries the shop domain, not a JWT. */ + tokens: { accessToken: params.accessToken, idToken: params.shopDomain }, + logIdentifier: params.shopDomain, + }) await processCredentialDraft({ draftId: params.draftId, userId: params.userId, providerId: 'shopify', - accountId: persisted.id, + accountId, }) } diff --git a/apps/sim/lib/oauth/slack.ts b/apps/sim/lib/oauth/slack.ts index 9c69ed3c887..64d4e9bc793 100644 --- a/apps/sim/lib/oauth/slack.ts +++ b/apps/sim/lib/oauth/slack.ts @@ -1,6 +1,7 @@ import { db } from '@sim/db' import { account } from '@sim/db/schema' import { and, eq, gt, isNotNull, like, max, sql } from 'drizzle-orm' +import { decryptAccountTokenColumns, encryptAccountTokenColumns } from '@/lib/oauth/account-tokens' /** * Slack bot tokens belong to the installation (team × app), not to the OAuth @@ -65,12 +66,19 @@ export async function fanOutSlackTokenChain( options?: FanOutOptions ): Promise { const since = options?.ifChainUnchangedSince + /** + * One `.set()` covers every sibling, so the whole installation stores byte-identical + * ciphertext. Nothing compares tokens across rows — both guards below read `updated_at`. + */ + const encrypted = await encryptAccountTokenColumns({ + accessToken: chain.accessToken, + ...(chain.refreshToken ? { refreshToken: chain.refreshToken } : {}), + }) await db .update(account) .set({ - accessToken: chain.accessToken, + ...encrypted, accessTokenExpiresAt: chain.accessTokenExpiresAt, - ...(chain.refreshToken ? { refreshToken: chain.refreshToken } : {}), updatedAt: new Date(), }) .where( @@ -132,10 +140,19 @@ export async function getFreshestSlackChain(teamId: string): Promise ({ mockIsFeatureEnabled: vi.fn() })) + +vi.mock('@/lib/core/config/feature-flags', () => ({ + isFeatureEnabled: mockIsFeatureEnabled, +})) + +import { upsertProviderAccountTokens } from '@/lib/oauth/credential-service' + +const IDENTITY = { userId: 'user-1', providerId: 'shopify', externalAccountId: 'store-1' } + +/** A single argument object that is a valid call for every provider. */ +function callArgs(overrides: Record = {}) { + return { + ...IDENTITY, + scope: 'read_orders', + tokens: { accessToken: 'plaintext-token' }, + ...overrides, + } +} + +function lastSetPayload(): Record { + const calls = dbChainMockFns.set.mock.calls + return calls[calls.length - 1]?.[0] as Record +} + +beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + setEnv({ ENCRYPTION_KEY: '0123456789abcdef'.repeat(4) }) + mockIsFeatureEnabled.mockResolvedValue(true) +}) + +describe('upsertProviderAccountTokens', () => { + describe('when the account already exists', () => { + beforeEach(() => { + queueTableRows(schemaMock.account, [{ id: 'account-1' }]) + }) + + it('updates in place and returns the existing id without inserting', async () => { + await expect(upsertProviderAccountTokens(callArgs())).resolves.toEqual({ + accountId: 'account-1', + }) + + expect(dbChainMockFns.update).toHaveBeenCalledWith(schemaMock.account) + expect(dbChainMockFns.insert).not.toHaveBeenCalled() + }) + + it('stores the access token enveloped, never in plaintext', async () => { + await upsertProviderAccountTokens(callArgs()) + + const payload = lastSetPayload() + expect(payload.accessToken).not.toBe('plaintext-token') + expect(String(payload.accessToken).startsWith('simenc:v1:')).toBe(true) + }) + + it('writes plaintext when the flag is off, so the rollout is reversible', async () => { + mockIsFeatureEnabled.mockResolvedValue(false) + await upsertProviderAccountTokens(callArgs()) + + expect(lastSetPayload().accessToken).toBe('plaintext-token') + }) + + it('envelopes every token the provider supplied', async () => { + await upsertProviderAccountTokens( + callArgs({ + providerId: 'instagram', + /** Instagram's long-lived token is its own refresh token. */ + tokens: { + accessToken: 'ig_token', + refreshToken: 'ig_token', + idToken: 'store.myshopify.com', + }, + }) + ) + + const payload = lastSetPayload() + for (const field of ['accessToken', 'refreshToken', 'idToken']) { + expect(String(payload[field]).startsWith('simenc:v1:')).toBe(true) + } + /** Separate IVs, so the two identical plaintexts must not collide. */ + expect(payload.accessToken).not.toBe(payload.refreshToken) + }) + + it('refreshes updatedAt, which the Slack and Instagram guards both read', async () => { + await upsertProviderAccountTokens(callArgs()) + expect(lastSetPayload().updatedAt).toBeInstanceOf(Date) + }) + + it('omits accessTokenExpiresAt when the provider does not supply one', async () => { + await upsertProviderAccountTokens(callArgs()) + expect('accessTokenExpiresAt' in lastSetPayload()).toBe(false) + }) + + it('sets accessTokenExpiresAt when the provider does supply one', async () => { + const accessTokenExpiresAt = new Date('2026-09-01T00:00:00.000Z') + await upsertProviderAccountTokens(callArgs({ accessTokenExpiresAt })) + expect(lastSetPayload().accessTokenExpiresAt).toBe(accessTokenExpiresAt) + }) + }) + + describe('when the account does not exist', () => { + it('inserts an enveloped row and returns the persisted id', async () => { + queueTableRows(schemaMock.account, []) + queueTableRows(schemaMock.account, [{ id: 'account-new' }]) + + await expect(upsertProviderAccountTokens(callArgs())).resolves.toEqual({ + accountId: 'account-new', + }) + + expect(dbChainMockFns.insert).toHaveBeenCalledWith(schemaMock.account) + const inserted = dbChainMockFns.values.mock.calls[0]?.[0] as Record + expect(String(inserted.accessToken).startsWith('simenc:v1:')).toBe(true) + expect(inserted).toMatchObject({ + userId: 'user-1', + providerId: 'shopify', + accountId: 'store-1', + scope: 'read_orders', + }) + }) + + /** + * `safeAccountInsert` swallows a duplicate-key race, so a missing row on the re-read + * means the write genuinely did not land — the caller must not get a bogus account id. + */ + it('throws when the row is still absent after the insert', async () => { + queueTableRows(schemaMock.account, []) + queueTableRows(schemaMock.account, []) + + await expect(upsertProviderAccountTokens(callArgs())).rejects.toThrow( + 'shopify OAuth account store-1 was not persisted' + ) + }) + }) +}) diff --git a/apps/sim/lib/webhooks/polling/utils.ts b/apps/sim/lib/webhooks/polling/utils.ts index df082862cf8..4c3a2a0a31e 100644 --- a/apps/sim/lib/webhooks/polling/utils.ts +++ b/apps/sim/lib/webhooks/polling/utils.ts @@ -217,7 +217,15 @@ export async function resolveOAuthCredential( ) return serviceAccountToken } - const rows = await db.select().from(account).where(eq(account.id, resolved.accountId)).limit(1) + /** + * Not `getCredentialOwner`: the service-account branch above already needs `resolved`, + * and that helper would re-run `resolveOAuthAccountId` to rebuild it. + */ + const rows = await db + .select({ userId: account.userId }) + .from(account) + .where(eq(account.id, resolved.accountId)) + .limit(1) if (!rows.length) { throw new Error(`Credential ${credentialId} not found for webhook ${webhookData.id}`) } diff --git a/apps/sim/lib/webhooks/providers/airtable.ts b/apps/sim/lib/webhooks/providers/airtable.ts index 99b77074cf6..15088d117cc 100644 --- a/apps/sim/lib/webhooks/providers/airtable.ts +++ b/apps/sim/lib/webhooks/providers/airtable.ts @@ -1,14 +1,10 @@ import { db } from '@sim/db' -import { account, webhook } from '@sim/db/schema' +import { webhook } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { eq } from 'drizzle-orm' import { validateAirtableId } from '@/lib/core/security/input-validation' import { getBaseUrl } from '@/lib/core/utils/urls' -import { - getOAuthToken, - refreshAccessTokenIfNeeded, - resolveOAuthAccountId, -} from '@/lib/oauth/credential-service' +import { getOAuthToken, refreshAccessTokenIfNeeded } from '@/lib/oauth/credential-service' import { getCredentialOwner, getNotificationUrl, @@ -84,32 +80,14 @@ async function fetchAndProcessAirtablePayloads( return } - const resolvedAirtable = await resolveOAuthAccountId(credentialId) - if (!resolvedAirtable) { - logger.error( - `[${requestId}] Could not resolve credential ${credentialId} for Airtable webhook` - ) - return - } - - let ownerUserId: string | null = null - try { - const rows = await db - .select() - .from(account) - .where(eq(account.id, resolvedAirtable.accountId)) - .limit(1) - ownerUserId = rows.length ? rows[0].userId : null - } catch (_e) { - ownerUserId = null - } - - if (!ownerUserId) { + const credentialOwner = await getCredentialOwner(credentialId, requestId) + if (!credentialOwner) { logger.error( `[${requestId}] Could not resolve owner for Airtable credential ${credentialId} on webhook ${webhookData.id}` ) return } + const ownerUserId = credentialOwner.userId const storedCursor = localProviderConfig.externalWebhookCursor @@ -152,7 +130,7 @@ async function fetchAndProcessAirtablePayloads( let accessToken: string | null = null try { accessToken = await refreshAccessTokenIfNeeded( - resolvedAirtable.accountId, + credentialOwner.accountId, ownerUserId, requestId ) diff --git a/apps/sim/lib/webhooks/providers/gmail.ts b/apps/sim/lib/webhooks/providers/gmail.ts index 4abc2caa6fb..ba6184e4f30 100644 --- a/apps/sim/lib/webhooks/providers/gmail.ts +++ b/apps/sim/lib/webhooks/providers/gmail.ts @@ -1,8 +1,9 @@ import { db } from '@sim/db' -import { account, webhook } from '@sim/db/schema' +import { webhook } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { eq } from 'drizzle-orm' -import { refreshAccessTokenIfNeeded, resolveOAuthAccountId } from '@/lib/oauth/credential-service' +import { refreshAccessTokenIfNeeded } from '@/lib/oauth/credential-service' +import { getCredentialOwner } from '@/lib/webhooks/provider-subscription-utils' import type { FormatInputContext, FormatInputResult, @@ -37,31 +38,17 @@ export const gmailHandler: WebhookProviderHandler = { return false } - const resolvedGmail = await resolveOAuthAccountId(credentialId) - if (!resolvedGmail) { + const credentialOwner = await getCredentialOwner(credentialId, requestId) + if (!credentialOwner) { logger.error( `[${requestId}] Could not resolve credential ${credentialId} for Gmail webhook ${webhookData.id}` ) return false } - const rows = await db - .select() - .from(account) - .where(eq(account.id, resolvedGmail.accountId)) - .limit(1) - if (rows.length === 0) { - logger.error( - `[${requestId}] Credential ${credentialId} not found for Gmail webhook ${webhookData.id}` - ) - return false - } - - const effectiveUserId = rows[0].userId - const accessToken = await refreshAccessTokenIfNeeded( - resolvedGmail.accountId, - effectiveUserId, + credentialOwner.accountId, + credentialOwner.userId, requestId ) if (!accessToken) { @@ -85,7 +72,7 @@ export const gmailHandler: WebhookProviderHandler = { const configuredProviderConfig = { ...providerConfig, - userId: effectiveUserId, + userId: credentialOwner.userId, credentialId, maxEmailsPerPoll, pollingInterval, diff --git a/apps/sim/lib/webhooks/providers/microsoft-teams.ts b/apps/sim/lib/webhooks/providers/microsoft-teams.ts index 4e35dba8467..e769f99f865 100644 --- a/apps/sim/lib/webhooks/providers/microsoft-teams.ts +++ b/apps/sim/lib/webhooks/providers/microsoft-teams.ts @@ -1,11 +1,8 @@ -import { db } from '@sim/db' -import { account } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { safeCompare } from '@sim/security/compare' import { hmacSha256Base64 } from '@sim/security/hmac' import { getErrorMessage, toError } from '@sim/utils/errors' import { isRecordLike } from '@sim/utils/object' -import { eq } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { isMicrosoftContentUrl } from '@/lib/core/security/input-validation' import { @@ -14,7 +11,7 @@ import { validateUrlWithDNS, } from '@/lib/core/security/input-validation.server' import { sanitizeUrlForLog } from '@/lib/core/utils/logging' -import { refreshAccessTokenIfNeeded, resolveOAuthAccountId } from '@/lib/oauth/credential-service' +import { refreshAccessTokenIfNeeded } from '@/lib/oauth/credential-service' import { getCredentialOwner, getNotificationUrl, @@ -213,25 +210,18 @@ async function formatTeamsGraphNotification( }) } else { try { - const resolved = await resolveOAuthAccountId(credentialId as string) - if (!resolved) { + const credentialOwner = await getCredentialOwner( + credentialId as string, + 'teams-graph-notification' + ) + if (!credentialOwner) { logger.error('Teams credential could not be resolved', { credentialId }) } else { - const rows = await db - .select() - .from(account) - .where(eq(account.id, resolved.accountId)) - .limit(1) - if (rows.length === 0) { - logger.error('Teams credential not found', { credentialId, chatId: resolvedChatId }) - } else { - const effectiveUserId = rows[0].userId - accessToken = await refreshAccessTokenIfNeeded( - resolved.accountId, - effectiveUserId, - 'teams-graph-notification' - ) - } + accessToken = await refreshAccessTokenIfNeeded( + credentialOwner.accountId, + credentialOwner.userId, + 'teams-graph-notification' + ) } if (accessToken) { diff --git a/apps/sim/lib/webhooks/providers/outlook.ts b/apps/sim/lib/webhooks/providers/outlook.ts index a6f2fa3d80e..d1c00004cc0 100644 --- a/apps/sim/lib/webhooks/providers/outlook.ts +++ b/apps/sim/lib/webhooks/providers/outlook.ts @@ -1,8 +1,9 @@ import { db } from '@sim/db' -import { account, webhook } from '@sim/db/schema' +import { webhook } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { eq } from 'drizzle-orm' -import { refreshAccessTokenIfNeeded, resolveOAuthAccountId } from '@/lib/oauth/credential-service' +import { refreshAccessTokenIfNeeded } from '@/lib/oauth/credential-service' +import { getCredentialOwner } from '@/lib/webhooks/provider-subscription-utils' import type { FormatInputContext, FormatInputResult, @@ -37,31 +38,17 @@ export const outlookHandler: WebhookProviderHandler = { return false } - const resolvedOutlook = await resolveOAuthAccountId(credentialId) - if (!resolvedOutlook) { + const credentialOwner = await getCredentialOwner(credentialId, requestId) + if (!credentialOwner) { logger.error( `[${requestId}] Could not resolve credential ${credentialId} for Outlook webhook ${webhookData.id}` ) return false } - const rows = await db - .select() - .from(account) - .where(eq(account.id, resolvedOutlook.accountId)) - .limit(1) - if (rows.length === 0) { - logger.error( - `[${requestId}] Credential ${credentialId} not found for Outlook webhook ${webhookData.id}` - ) - return false - } - - const effectiveUserId = rows[0].userId - const accessToken = await refreshAccessTokenIfNeeded( - resolvedOutlook.accountId, - effectiveUserId, + credentialOwner.accountId, + credentialOwner.userId, requestId ) if (!accessToken) { @@ -75,7 +62,7 @@ export const outlookHandler: WebhookProviderHandler = { const configuredProviderConfig = { ...providerConfig, - userId: effectiveUserId, + userId: credentialOwner.userId, credentialId, maxEmailsPerPoll: typeof providerConfig.maxEmailsPerPoll === 'string' diff --git a/package.json b/package.json index 115b8854347..fb715ec612a 100644 --- a/package.json +++ b/package.json @@ -14,13 +14,14 @@ "dev:sockets": "cd apps/realtime && bun run dev", "dev:full": "bunx concurrently -n \"App,Realtime\" -c \"cyan,magenta\" \"cd apps/sim && bun run dev\" \"cd apps/realtime && bun run dev\"", "dev:full:capped": "bunx concurrently -n \"App,Realtime\" -c \"cyan,magenta\" \"cd apps/sim && bun run dev:capped\" \"cd apps/realtime && bun run dev\"", - "test": "bun run test:setup && bun run test:npm-package-versions && bun run test:icon-path-precision && bun run test:tool-registry-boundary && bun run test:tool-request-boundary && bun run test:actorless-executor-operations && bun run test:migrations-safety && bun run test:generators && turbo run test", + "test": "bun run test:setup && bun run test:npm-package-versions && bun run test:icon-path-precision && bun run test:tool-registry-boundary && bun run test:tool-request-boundary && bun run test:actorless-executor-operations && bun run test:migrations-safety && bun run test:account-token-access && bun run test:generators && turbo run test", "test:setup": "bun run --cwd packages/sim-setup test", "test:npm-package-versions": "bunx vitest run scripts/bump-npm-package-versions.test.ts", "test:icon-path-precision": "bunx vitest run scripts/check-icon-path-precision.test.ts", "test:tool-registry-boundary": "bunx vitest run scripts/check-tool-registry-boundary.test.ts", "test:tool-request-boundary": "bunx vitest run scripts/check-tool-request-boundary.test.ts", "test:actorless-executor-operations": "bunx vitest run scripts/check-actorless-executor-operations.test.ts", + "test:account-token-access": "bunx vitest run scripts/check-account-token-access.test.ts", "test:migrations-safety": "bunx vitest run scripts/check-migrations-safety.test.ts", "test:generators": "bunx vitest run scripts/generate-v2-cli-api.test.ts scripts/generate-cli-docs.test.ts scripts/generate-docs.test.ts", "format": "turbo run format", @@ -57,6 +58,7 @@ "check:utils": "bun run scripts/check-utils-enforcement.ts", "check:canvas-sentences": "bun run apps/sim/scripts/check-canvas-sentences.ts --require-coverage", "check:bare-icons": "bun run scripts/check-bare-icons.ts", + "check:account-token-access": "bun run scripts/check-account-token-access.ts", "check:byok-providers": "bun run scripts/check-byok-providers.ts", "check:icon-paths": "bun run scripts/check-icon-paths.ts", "check:icon-path-precision": "bun run scripts/check-icon-path-precision.ts", diff --git a/scripts/check-account-token-access.test.ts b/scripts/check-account-token-access.test.ts new file mode 100644 index 00000000000..7f5ff5c845d --- /dev/null +++ b/scripts/check-account-token-access.test.ts @@ -0,0 +1,122 @@ +import { describe, expect, it } from 'vitest' +import { auditSource } from './check-account-token-access' + +const FILE = 'apps/sim/lib/webhooks/providers/example.ts' + +describe('auditSource', () => { + it('flags a direct token-column read', () => { + const findings = auditSource( + FILE, + `const token = row.accessToken\nconst x = account.accessToken` + ) + expect(findings).toHaveLength(1) + expect(findings[0]).toMatchObject({ line: 2, kind: 'token-column' }) + }) + + it.each(['accessToken', 'refreshToken', 'idToken'])('flags account.%s', (field) => { + expect(auditSource(FILE, `select({ v: account.${field} })`)).toHaveLength(1) + }) + + it('flags the schema-qualified form Better Auth hooks use', () => { + expect(auditSource(FILE, 'const t = schema.account.refreshToken')).toHaveLength(1) + }) + + /** The implicit case: no token column is named, but all three come back. */ + it('flags a projection-less select', () => { + const findings = auditSource( + FILE, + 'const rows = await db.select().from(account).where(eq(account.userId, userId))' + ) + expect(findings).toHaveLength(1) + expect(findings[0].kind).toBe('star-select') + }) + + it('flags the relational read', () => { + const findings = auditSource(FILE, 'const row = await db.query.account.findFirst({ where })') + expect(findings).toHaveLength(1) + expect(findings[0].kind).toBe('relational-read') + }) + + /** The shape a new connect flow gets copied into; no read-side rule would catch it. */ + it.each([ + ['insert', 'await db.insert(account).values({ accessToken: raw })'], + ['update', 'await db.update(account).set({ accessToken: raw }).where(eq(account.id, id))'], + ['schema-qualified update', 'await db.update(schema.account).set({ scope })'], + ['chained update', ' .update(account)'], + ])('flags a direct %s of the account table', (_label, src) => { + const findings = auditSource(FILE, src) + expect(findings).toHaveLength(1) + expect(findings[0].kind).toBe('write') + }) + + it('does not flag a write to a different table', () => { + expect(auditSource(FILE, 'await db.update(credential).set({ scope })')).toEqual([]) + }) + + it('allows a narrowed projection', () => { + expect( + auditSource(FILE, 'await db.select({ id: account.id, userId: account.userId }).from(account)') + ).toEqual([]) + }) + + it('does not confuse a same-named field on another object', () => { + expect(auditSource(FILE, 'const t = tokens.accessToken\nconst u = chain.refreshToken')).toEqual( + [] + ) + }) + + it('exempts the token-aware modules', () => { + const source = 'await db.select().from(account)\nconst t = account.accessToken' + expect(auditSource('apps/sim/lib/oauth/account-tokens.ts', source)).toEqual([]) + expect(auditSource('apps/sim/lib/oauth/credential-service.ts', source)).toEqual([]) + expect(auditSource('apps/sim/lib/oauth/slack.ts', source)).toEqual([]) + }) + + describe('annotation', () => { + it('suppresses a finding when it carries a reason', () => { + expect( + auditSource( + FILE, + '// account-token-access-allow: decrypted immediately below\nconst t = account.idToken' + ) + ).toEqual([]) + }) + + it('reports an annotation with no reason rather than honoring it', () => { + const findings = auditSource( + FILE, + '// account-token-access-allow:\nconst t = account.idToken' + ) + expect(findings).toHaveLength(1) + expect(findings[0].kind).toBe('empty-reason') + }) + + it('reaches across intervening comment lines', () => { + expect( + auditSource( + FILE, + '// account-token-access-allow: needed for display name\n// more context\nconst t = account.idToken' + ) + ).toEqual([]) + }) + + /** An annotation must not leak past unrelated code onto a later violation. */ + it('does not carry past a line of code', () => { + expect( + auditSource( + FILE, + '// account-token-access-allow: covers the next line only\nconst a = account.idToken\nconst b = account.accessToken' + ) + ).toHaveLength(1) + }) + + it('does not reach further than three lines back', () => { + expect( + auditSource( + FILE, + '// account-token-access-allow: too far\n//\n//\n//\nconst t = account.idToken' + ) + ).toHaveLength(1) + }) + }) +}) diff --git a/scripts/check-account-token-access.ts b/scripts/check-account-token-access.ts new file mode 100644 index 00000000000..ac6f9b4d73d --- /dev/null +++ b/scripts/check-account-token-access.ts @@ -0,0 +1,165 @@ +#!/usr/bin/env bun +/** + * Audits that the `account` table's OAuth token columns are only ever read or written + * through the one module that knows how to encrypt and decrypt them. + * + * Three shapes are flagged: + * + * 1. Naming a token column directly (`account.accessToken`, `schema.account.idToken`). + * 2. Selecting the whole row without a projection — `db.select().from(account)` or + * `db.query.account.findFirst(...)`. This is the subtler one: it pulls all three + * token columns implicitly, so a site that only wants `userId` today silently starts + * leaking ciphertext the moment someone reads `.accessToken` off the result. + * 3. Writing to the table at all — `db.insert(account)` / `db.update(account)`. A new + * connect flow copied from an old one is how a plaintext token gets stored, and no + * read-side rule would catch it. Writes go through `upsertProviderAccountTokens`. + * + * Escape hatch: `// account-token-access-allow: ` on one of the three lines above + * the offending line. The reason is mandatory. + * + * Run: `bun run check:account-token-access` + */ +import { readdirSync, readFileSync } from 'node:fs' +import { dirname, join, relative, resolve } from 'node:path' +import { fileURLToPath } from 'node:url' + +const SCRIPT_DIR = dirname(fileURLToPath(import.meta.url)) +const ROOT = resolve(SCRIPT_DIR, '..') +const APP = resolve(ROOT, 'apps/sim') + +/** + * The modules permitted to touch the token columns. Each either implements the envelope or + * immediately decrypts what it selects. + * + * Keep this list short — its purpose is to stop a *new*, silent token read appearing anywhere + * in the other ~4,000 files. + */ +const TOKEN_AWARE_MODULES = new Set([ + 'apps/sim/lib/oauth/account-token-crypto.ts', + 'apps/sim/lib/oauth/account-tokens.ts', + 'apps/sim/lib/oauth/credential-service.ts', + 'apps/sim/lib/oauth/slack.ts', +]) + +const ANNOTATION = 'account-token-access-allow:' +const MAX_ANNOTATION_LOOKBACK = 3 + +const TOKEN_COLUMN_RE = /\b(?:schema\.)?account\.(accessToken|refreshToken|idToken)\b/ +const STAR_SELECT_RE = /\bdb\s*\.\s*select\s*\(\s*\)\s*\.\s*from\s*\(\s*(?:schema\.)?account\s*\)/ +const RELATIONAL_READ_RE = /\bdb\s*\.\s*query\s*\.\s*account\s*\.\s*find/ +const WRITE_RE = /\.\s*(?:insert|update)\s*\(\s*(?:schema\.)?account\s*\)/ + +export type FindingKind = + | 'token-column' + | 'star-select' + | 'relational-read' + | 'write' + | 'empty-reason' + +export interface Finding { + file: string + line: number + kind: FindingKind + text: string +} + +const MESSAGES: Record = { + 'token-column': 'reads or writes an account token column directly', + 'star-select': 'selects the whole account row, which implicitly pulls all three token columns', + 'relational-read': + 'reads the account row through the relational API, which implicitly pulls all three token columns', + write: + 'writes to the account table directly, so a token column could be stored without encryption', + 'empty-reason': `\`${ANNOTATION}\` annotation has no reason`, +} + +/** + * True when an `account-token-access-allow:` annotation with a non-empty reason sits within + * the preceding comment block. Scanning stops at the first non-empty, non-comment line so an + * annotation cannot leak downward past unrelated code. + */ +function findAnnotation(lines: string[], index: number): 'present' | 'empty-reason' | 'absent' { + for (let i = index - 1; i >= 0 && i >= index - MAX_ANNOTATION_LOOKBACK; i--) { + const line = lines[i]?.trim() ?? '' + if (line === '' || line.startsWith('//') || line.startsWith('*') || line.startsWith('/*')) { + if (!line.includes(ANNOTATION)) continue + const reason = line.split(ANNOTATION)[1]?.trim() ?? '' + return reason.length > 0 ? 'present' : 'empty-reason' + } + break + } + return 'absent' +} + +/** Analyzes one file's source. Exported so the audit is unit-testable without a tree walk. */ +export function auditSource(relPath: string, source: string): Finding[] { + if (TOKEN_AWARE_MODULES.has(relPath)) return [] + if (!source.includes('account')) return [] + + const lines = source.split('\n') + const findings: Finding[] = [] + + lines.forEach((line, index) => { + const kind: FindingKind | null = TOKEN_COLUMN_RE.test(line) + ? 'token-column' + : STAR_SELECT_RE.test(line) + ? 'star-select' + : RELATIONAL_READ_RE.test(line) + ? 'relational-read' + : WRITE_RE.test(line) + ? 'write' + : null + if (!kind) return + + const annotation = findAnnotation(lines, index) + if (annotation === 'present') return + + findings.push({ + file: relPath, + line: index + 1, + kind: annotation === 'empty-reason' ? 'empty-reason' : kind, + text: line.trim(), + }) + }) + + return findings +} + +const SKIPPED_DIRECTORIES = new Set(['node_modules', '.next', 'generated']) + +function walk(directory: string, into: string[]): string[] { + for (const entry of readdirSync(directory, { withFileTypes: true })) { + if (SKIPPED_DIRECTORIES.has(entry.name)) continue + const full = join(directory, entry.name) + if (entry.isDirectory()) walk(full, into) + else if (/\.tsx?$/.test(entry.name) && !/\.test\.tsx?$/.test(entry.name)) into.push(full) + } + return into +} + +function main(): void { + const files = walk(APP, []) + const findings: Finding[] = [] + for (const file of files) { + const relPath = relative(ROOT, file) + findings.push(...auditSource(relPath, readFileSync(file, 'utf8'))) + } + + if (findings.length > 0) { + console.error('\n❌ account token columns accessed outside the token accessor\n') + for (const finding of findings) { + console.error(` ${finding.file}:${finding.line} ${MESSAGES[finding.kind]}`) + console.error(` ${finding.text}`) + } + console.error( + `\n fix: read and write tokens through \`@/lib/oauth/account-tokens\`, or project only the\n` + + ` non-token columns you need (\`select({ id, userId })\`). For a documented exception,\n` + + ` add \`// ${ANNOTATION} \` directly above the line.\n` + ) + process.exit(1) + } + + console.log(`✓ account token columns are only accessed via the accessor (${files.length} files)`) +} + +if (import.meta.main) main() From 8e8fba160b6759d7c86c568879edcd458ef6ce84 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 28 Aug 2026 14:55:29 -0700 Subject: [PATCH 2/5] fix(security): address review findings on account token encryption MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The access audit matched line by line, so any Drizzle chain the formatter wrapped — which is how `db.select().from(account)` is normally written — was invisible to it. Matching over the whole source and mapping offsets back to lines closes that; a projection-less multiline select now fails CI as intended. - A legacy token beginning `simenc:` was classified as an envelope, which left it unencrypted on write and threw `unknown-version` on read, making the credential unavailable. Detection now requires a full versioned header, so such a value stays plaintext. - Encryption failures were caught and the token stored in plaintext. The only expected cause is an unusable key, which the gate already handles, so a throw past it is a real fault; swallowing it would silently break the guarantee the flag reports as on. --- .../lib/oauth/account-token-crypto.test.ts | 20 +++++ apps/sim/lib/oauth/account-token-crypto.ts | 9 +- apps/sim/lib/oauth/account-tokens.ts | 16 ++-- scripts/check-account-token-access.test.ts | 22 +++++ scripts/check-account-token-access.ts | 85 ++++++++++++------- 5 files changed, 113 insertions(+), 39 deletions(-) diff --git a/apps/sim/lib/oauth/account-token-crypto.test.ts b/apps/sim/lib/oauth/account-token-crypto.test.ts index 3b3be217d39..eaedec224a3 100644 --- a/apps/sim/lib/oauth/account-token-crypto.test.ts +++ b/apps/sim/lib/oauth/account-token-crypto.test.ts @@ -43,6 +43,19 @@ describe('isEncryptedAccountToken', () => { expect(isEncryptedAccountToken(token)).toBe(false) }) + /** + * A legacy token that merely begins `simenc:` is not one of ours. Matching the version + * too keeps it classified as plaintext instead of an envelope this build cannot read — + * which would leave it unencrypted on write and unavailable on read. + */ + it.each([ + ['no version segment', 'simenc:legacy-opaque-value'], + ['a non-numeric version', 'simenc:vX:legacy'], + ['the bare prefix', 'simenc:'], + ])('treats a legacy value with %s as plaintext, not an envelope', (_label, value) => { + expect(isEncryptedAccountToken(value)).toBe(false) + }) + it('classifies an envelope as ciphertext', async () => { expect(isEncryptedAccountToken(await encryptAccountToken('secret'))).toBe(true) }) @@ -137,6 +150,13 @@ describe('decryptAccountToken', () => { await expect(decryptAccountToken(value as string, 'accessToken')).resolves.toBe(value) }) + it.each([ + ['no version segment', 'simenc:legacy-opaque-value'], + ['the bare prefix', 'simenc:'], + ])('passes a legacy value with %s through unchanged', async (_label, value) => { + await expect(decryptAccountToken(value, 'accessToken')).resolves.toBe(value) + }) + it('throws on an unknown envelope version rather than passing it through', async () => { await expect(decryptAccountToken('simenc:v2:deadbeef', 'refreshToken')).rejects.toThrow( AccountTokenDecryptionError diff --git a/apps/sim/lib/oauth/account-token-crypto.ts b/apps/sim/lib/oauth/account-token-crypto.ts index d8fd563d901..c2a90d8975e 100644 --- a/apps/sim/lib/oauth/account-token-crypto.ts +++ b/apps/sim/lib/oauth/account-token-crypto.ts @@ -14,6 +14,13 @@ import { decryptSecret, encryptSecret } from '@/lib/core/security/encryption' */ const ENVELOPE_PREFIX = 'simenc:' +/** + * A full, versioned envelope header. Matching the version too — rather than the bare + * `simenc:` prefix — keeps a legacy token that happens to begin `simenc:` classified as + * plaintext instead of as an envelope this build cannot read. + */ +const ENVELOPE_HEADER_RE = /^simenc:v(\d+):/ + /** Current envelope version. The payload after this prefix is `iv:ciphertext:authTag`. */ const ENVELOPE_V1_PREFIX = `${ENVELOPE_PREFIX}v1:` @@ -37,7 +44,7 @@ export class AccountTokenDecryptionError extends Error { /** True when a stored column value is one of our envelopes rather than legacy plaintext. */ export function isEncryptedAccountToken(value: string): boolean { - return value.startsWith(ENVELOPE_PREFIX) + return ENVELOPE_HEADER_RE.test(value) } /** diff --git a/apps/sim/lib/oauth/account-tokens.ts b/apps/sim/lib/oauth/account-tokens.ts index ff39ae17bd9..4ca38fff0d5 100644 --- a/apps/sim/lib/oauth/account-tokens.ts +++ b/apps/sim/lib/oauth/account-tokens.ts @@ -1,5 +1,4 @@ import { createLogger } from '@sim/logger' -import { getErrorMessage } from '@sim/utils/errors' import { isFeatureEnabled } from '@/lib/core/config/feature-flags' import { hasUsableEncryptionKey } from '@/lib/core/security/encryption' import { @@ -71,14 +70,13 @@ export async function encryptAccountTokenColumns { expect(findings[0].kind).toBe('write') }) + /** + * The formatter breaks any chain past the print width, so this is how these calls are + * actually written. A per-line scan saw none of them. + */ + it.each([ + ['select', 'const rows = await db\n .select()\n .from(account)\n .where(x)'], + ['update', 'await db\n .update(account)\n .set({ accessToken: raw })'], + [ + 'insert with a wrapped argument', + 'await db.insert(\n account\n).values({ accessToken: raw })', + ], + ['relational read', 'const row = await db\n .query\n .account\n .findFirst({ where })'], + ])('flags a multiline %s', (_label, src) => { + expect(auditSource(FILE, src)).toHaveLength(1) + }) + + it('still allows a narrowed projection when it spans lines', () => { + expect( + auditSource(FILE, 'const rows = await db\n .select({ id: account.id })\n .from(account)') + ).toEqual([]) + }) + it('does not flag a write to a different table', () => { expect(auditSource(FILE, 'await db.update(credential).set({ scope })')).toEqual([]) }) diff --git a/scripts/check-account-token-access.ts b/scripts/check-account-token-access.ts index ac6f9b4d73d..522f2a7ce08 100644 --- a/scripts/check-account-token-access.ts +++ b/scripts/check-account-token-access.ts @@ -44,10 +44,24 @@ const TOKEN_AWARE_MODULES = new Set([ const ANNOTATION = 'account-token-access-allow:' const MAX_ANNOTATION_LOOKBACK = 3 -const TOKEN_COLUMN_RE = /\b(?:schema\.)?account\.(accessToken|refreshToken|idToken)\b/ -const STAR_SELECT_RE = /\bdb\s*\.\s*select\s*\(\s*\)\s*\.\s*from\s*\(\s*(?:schema\.)?account\s*\)/ -const RELATIONAL_READ_RE = /\bdb\s*\.\s*query\s*\.\s*account\s*\.\s*find/ -const WRITE_RE = /\.\s*(?:insert|update)\s*\(\s*(?:schema\.)?account\s*\)/ +/** + * Matched against the whole file rather than line by line: the formatter breaks any chain + * past the print width, so `db.select().from(account)` is normally written across three + * lines and a per-line scan would never see it. `\s` spans newlines, so one pass over the + * source catches both shapes. + */ +const RULES: ReadonlyArray<{ kind: FindingKind; pattern: RegExp }> = [ + { + kind: 'token-column', + pattern: /\b(?:schema\.)?account\.(?:accessToken|refreshToken|idToken)\b/g, + }, + { + kind: 'star-select', + pattern: /\bdb\s*\.\s*select\s*\(\s*\)\s*\.\s*from\s*\(\s*(?:schema\.)?account\s*\)/g, + }, + { kind: 'relational-read', pattern: /\bdb\s*\.\s*query\s*\.\s*account\s*\.\s*find/g }, + { kind: 'write', pattern: /\.\s*(?:insert|update)\s*\(\s*(?:schema\.)?account\s*\)/g }, +] export type FindingKind = | 'token-column' @@ -97,32 +111,45 @@ export function auditSource(relPath: string, source: string): Finding[] { if (!source.includes('account')) return [] const lines = source.split('\n') - const findings: Finding[] = [] + /** Offset of the first character of each line, so a match index maps back to a line. */ + const lineStarts: number[] = [0] + for (let i = 0; i < source.length; i++) { + if (source[i] === '\n') lineStarts.push(i + 1) + } + + const lineIndexAt = (offset: number): number => { + let low = 0 + let high = lineStarts.length - 1 + while (low < high) { + const mid = Math.ceil((low + high) / 2) + if (lineStarts[mid] <= offset) low = mid + else high = mid - 1 + } + return low + } + + /** One finding per line: a single statement should not be reported by several rules. */ + const byLine = new Map() + + for (const { kind, pattern } of RULES) { + pattern.lastIndex = 0 + for (const match of source.matchAll(pattern)) { + const index = lineIndexAt(match.index) + if (byLine.has(index)) continue + + const annotation = findAnnotation(lines, index) + if (annotation === 'present') continue + + byLine.set(index, { + file: relPath, + line: index + 1, + kind: annotation === 'empty-reason' ? 'empty-reason' : kind, + text: (lines[index] ?? '').trim(), + }) + } + } - lines.forEach((line, index) => { - const kind: FindingKind | null = TOKEN_COLUMN_RE.test(line) - ? 'token-column' - : STAR_SELECT_RE.test(line) - ? 'star-select' - : RELATIONAL_READ_RE.test(line) - ? 'relational-read' - : WRITE_RE.test(line) - ? 'write' - : null - if (!kind) return - - const annotation = findAnnotation(lines, index) - if (annotation === 'present') return - - findings.push({ - file: relPath, - line: index + 1, - kind: annotation === 'empty-reason' ? 'empty-reason' : kind, - text: line.trim(), - }) - }) - - return findings + return [...byLine.values()].sort((a, b) => a.line - b.line) } const SKIPPED_DIRECTORIES = new Set(['node_modules', '.next', 'generated']) From fbb62187bbb260a25790e69a606b2d927e648a22 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 28 Aug 2026 15:28:00 -0700 Subject: [PATCH 3/5] feat(security): add the account token encryption backfill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Once the flag is on, tokens envelope themselves as they are written or refreshed — but only for rows that get rewritten. A provider that issues no refresh token, and a user who never signs in again, both leave a row in plaintext indefinitely. This closes that tail. Deliberately a manual script rather than a `script-migration`, so it never runs as part of `db:migrate` or a deploy: a self-hosted upgrade is unaffected by its existence and nothing happens until an operator runs it. - Dry run unless `--apply`. Enveloping is not reversible without the key, so the destructive direction is opt-in twice. - Refuses to start unless `ENCRYPTION_KEY` is usable and an AES-GCM round trip agrees with itself, so a misconfigured deployment touches no rows. - Keyset pagination that must advance, so a persistently racing row cannot loop forever. - Compare-and-swap on the exact values read. A token rotated by a concurrent refresh is reported and skipped, never reverted to the stale one. - Never writes `updated_at` — Slack's fan-out version guard, Instagram's minimum token age, connection ordering and "last connected" all read it. `fieldsNeedingEncryption` moves into the crypto module so the bulk job and the live write path share one definition of "already encrypted" rather than drifting. The SQL pre-filter matches `simenc:v%`, not `simenc:%`, to agree with it: a legacy value that merely begins `simenc:` is not ours and must still be enveloped rather than skipped. --- .../lib/oauth/account-token-crypto.test.ts | 47 ++++ apps/sim/lib/oauth/account-token-crypto.ts | 20 ++ scripts/backfill-account-token-encryption.ts | 231 ++++++++++++++++++ scripts/check-account-token-access.ts | 4 + 4 files changed, 302 insertions(+) create mode 100644 scripts/backfill-account-token-encryption.ts diff --git a/apps/sim/lib/oauth/account-token-crypto.test.ts b/apps/sim/lib/oauth/account-token-crypto.test.ts index eaedec224a3..8f2d2412cf8 100644 --- a/apps/sim/lib/oauth/account-token-crypto.test.ts +++ b/apps/sim/lib/oauth/account-token-crypto.test.ts @@ -16,6 +16,7 @@ import { AccountTokenDecryptionError, decryptAccountToken, encryptAccountToken, + fieldsNeedingEncryption, isEncryptedAccountToken, } from '@/lib/oauth/account-token-crypto' @@ -194,3 +195,49 @@ describe('decryptAccountToken', () => { await expect(decryptSecret(encrypted)).rejects.toThrow() }) }) + +/** + * Shared with `scripts/backfill-account-token-encryption.ts` so the bulk job and the live + * write path cannot disagree about what counts as already-encrypted. + */ +describe('fieldsNeedingEncryption', () => { + it('selects every plaintext token column', () => { + expect( + fieldsNeedingEncryption({ + accessToken: 'plaintext-access', + refreshToken: 'plaintext-refresh', + idToken: 'plaintext-id', + }) + ).toEqual(['accessToken', 'refreshToken', 'idToken']) + }) + + it('skips an enveloped value, so a re-run cannot double-wrap', async () => { + expect(fieldsNeedingEncryption({ accessToken: await encryptAccountToken('x') })).toEqual([]) + }) + + it('selects only what is still plaintext on a half-migrated row', async () => { + expect( + fieldsNeedingEncryption({ + accessToken: await encryptAccountToken('x'), + refreshToken: 'plaintext-refresh', + }) + ).toEqual(['refreshToken']) + }) + + it.each([ + ['an empty string', ''], + ['null', null], + ['undefined', undefined], + ])('leaves %s alone', (_label, value) => { + expect(fieldsNeedingEncryption({ accessToken: value })).toEqual([]) + }) + + /** Not ours despite the leading `simenc:`, so it is plaintext and must be enveloped. */ + it.each([ + ['no version segment', 'simenc:legacy-opaque-value'], + ['a non-numeric version', 'simenc:vX:value'], + ['the bare prefix', 'simenc:'], + ])('treats a legacy value with %s as plaintext', (_label, value) => { + expect(fieldsNeedingEncryption({ accessToken: value })).toEqual(['accessToken']) + }) +}) diff --git a/apps/sim/lib/oauth/account-token-crypto.ts b/apps/sim/lib/oauth/account-token-crypto.ts index c2a90d8975e..72e95cc3770 100644 --- a/apps/sim/lib/oauth/account-token-crypto.ts +++ b/apps/sim/lib/oauth/account-token-crypto.ts @@ -27,6 +27,9 @@ const ENVELOPE_V1_PREFIX = `${ENVELOPE_PREFIX}v1:` /** The `account` columns this module protects. */ export type AccountTokenField = 'accessToken' | 'refreshToken' | 'idToken' +/** Every column this module protects, in a stable order. */ +export const ACCOUNT_TOKEN_FIELDS = ['accessToken', 'refreshToken', 'idToken'] as const + /** * Thrown when a prefixed value cannot be recovered — wrong `ENCRYPTION_KEY`, truncated * column, or an unknown envelope version. Never thrown for legacy plaintext. @@ -90,3 +93,20 @@ export async function decryptAccountToken( throw new AccountTokenDecryptionError(field, 'decrypt-failed', error) } } + +/** + * Which of a row's token columns still hold plaintext and therefore need enveloping. + * + * Shared by the accessor and by `scripts/backfill-account-token-encryption.ts` so the bulk + * job and the live write path cannot disagree about what counts as already-encrypted. + * Empty strings are excluded: SSO writes `accessToken: ''` for SAML rows, and enveloping + * that would make it truthy, flipping every `if (account.accessToken)` guard in the app. + */ +export function fieldsNeedingEncryption( + row: Partial> +): AccountTokenField[] { + return ACCOUNT_TOKEN_FIELDS.filter((field) => { + const value = row[field] + return typeof value === 'string' && value !== '' && !isEncryptedAccountToken(value) + }) +} diff --git a/scripts/backfill-account-token-encryption.ts b/scripts/backfill-account-token-encryption.ts new file mode 100644 index 00000000000..08fb24d61c9 --- /dev/null +++ b/scripts/backfill-account-token-encryption.ts @@ -0,0 +1,231 @@ +#!/usr/bin/env bun +/** + * Wraps existing `account` OAuth tokens in the `simenc:v1:` envelope. + * + * Once `oauth-token-encryption` is on, every token that is written or refreshed is + * enveloped, so actively-used credentials convert on their own. Rows that are never + * rewritten do not: a provider that issues no refresh token (GitHub classic, Shopify, + * Trello) and a user who never signs in again both leave a row in plaintext forever. + * This closes that tail. + * + * It is a manual script on purpose — it is deliberately NOT a `script-migration`, so it + * never runs as part of `db:migrate` or a deploy. Self-hosted upgrades are unaffected by + * its existence; nothing happens until someone runs it. + * + * Safety properties: + * - Dry run unless `--apply` is passed. Encryption is not reversible without the key, + * so the destructive direction is opt-in twice: run it, then mean it. + * - Refuses to start unless `ENCRYPTION_KEY` is usable and an AES-GCM round trip + * agrees with itself. A misconfigured deployment touches no rows. + * - Idempotent and resumable. Already-enveloped values are skipped, so re-running + * after an interruption continues rather than double-wrapping. + * - Compare-and-swap per row. A token rotated by a concurrent refresh between the read + * and the write is left alone and reported, never overwritten with the stale value. + * - Never writes `updated_at`. Slack's fan-out version guard, Instagram's minimum + * token age, connection ordering and "last connected" all read it. + * + * Usage: + * DATABASE_URL=... ENCRYPTION_KEY=... bun run scripts/backfill-account-token-encryption.ts + * DATABASE_URL=... ENCRYPTION_KEY=... bun run scripts/backfill-account-token-encryption.ts --apply + * + * Options: --apply perform writes (default is a dry run) + * --batch=N rows per page (default 500) + * --sleep=MS pause between pages (default 250) + */ +import { getErrorMessage } from '@sim/utils/errors' +import { sleep } from '@sim/utils/helpers' +import { and, asc, eq, gt, or, sql } from 'drizzle-orm' +import { drizzle } from 'drizzle-orm/postgres-js' +import postgres from 'postgres' +import { + ACCOUNT_TOKEN_FIELDS, + type AccountTokenField, + encryptAccountToken, + fieldsNeedingEncryption, +} from '../apps/sim/lib/oauth/account-token-crypto' +import { account } from '../packages/db/schema' + +type TokenRow = { id: string } & Record + +function parseNumberArg(name: string, fallback: number): number { + const raw = process.argv.find((arg) => arg.startsWith(`--${name}=`))?.split('=')[1] + const parsed = raw ? Number.parseInt(raw, 10) : Number.NaN + return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback +} + +/** Fails before any row is read if this environment's AES-GCM does not round-trip. */ +async function assertCryptoRoundTrip(): Promise { + const sample = 'account-token-backfill-round-trip' + const enveloped = await encryptAccountToken(sample) + if (!enveloped.startsWith('simenc:v1:')) { + throw new Error(`Envelope prefix missing; got ${enveloped.slice(0, 16)}…`) + } + const { decryptAccountToken } = await import('../apps/sim/lib/oauth/account-token-crypto') + const recovered = await decryptAccountToken(enveloped, 'accessToken') + if (recovered !== sample) throw new Error('AES-GCM round trip did not return the input') +} + +async function main(): Promise { + const apply = process.argv.includes('--apply') + const batchSize = parseNumberArg('batch', 500) + const sleepMs = parseNumberArg('sleep', 250) + + const connectionString = process.env.DATABASE_URL ?? process.env.POSTGRES_URL + if (!connectionString) { + console.error('Missing DATABASE_URL (or POSTGRES_URL)') + process.exit(1) + } + + const key = process.env.ENCRYPTION_KEY + if (!key || key.length !== 64 || !/^[0-9a-f]+$/i.test(key)) { + console.error( + 'ENCRYPTION_KEY must be a 64-character hex string (32 bytes) — the same key the app runs with.\n' + + 'Nothing was read or written.' + ) + process.exit(1) + } + + try { + await assertCryptoRoundTrip() + } catch (error) { + console.error(`Refusing to run: ${getErrorMessage(error)}\nNothing was read or written.`) + process.exit(1) + } + + if (!process.env.OAUTH_TOKEN_ENCRYPTION) { + console.warn( + 'Note: OAUTH_TOKEN_ENCRYPTION is not set in this environment. On hosted deployments the\n' + + 'flag lives in AppConfig, so this is expected. On self-hosted it means new writes are\n' + + 'still plaintext — enable the flag first, or the table will drift back.\n' + ) + } + + const client = postgres(connectionString, { + prepare: false, + idle_timeout: 20, + connect_timeout: 30, + max: 3, + onnotice: () => {}, + }) + const db = drizzle(client) + + /** + * Pending when a token column holds a non-empty value that is not already one of our + * envelopes. Matched as `simenc:v%` rather than `simenc:%` to stay aligned with + * {@link isEncryptedAccountToken}: a legacy value that merely begins `simenc:` is not + * ours, so it must still be selected and enveloped rather than skipped forever. + */ + const pending = or( + ...ACCOUNT_TOKEN_FIELDS.map((field) => + and( + sql`${account[field]} IS NOT NULL`, + sql`${account[field]} <> ''`, + sql`${account[field]} NOT LIKE 'simenc:v%'` + ) + ) + ) + + const stats = { scanned: 0, updated: 0, raced: 0, failed: 0 } + + try { + const [{ count: before }] = await db + .select({ count: sql`count(*)::int` }) + .from(account) + .where(pending) + + console.log( + `${before} row(s) hold a plaintext token${apply ? '' : ' [DRY RUN — pass --apply to write]'}` + ) + if (before === 0) return + + let cursor = '' + for (;;) { + const rows: TokenRow[] = await db + .select({ + id: account.id, + accessToken: account.accessToken, + refreshToken: account.refreshToken, + idToken: account.idToken, + }) + .from(account) + .where(and(gt(account.id, cursor), pending)) + .orderBy(asc(account.id)) + .limit(batchSize) + + if (rows.length === 0) break + + const lastId = rows[rows.length - 1].id + if (lastId <= cursor) throw new Error(`Keyset cursor did not advance past ${cursor}`) + cursor = lastId + + for (const row of rows) { + stats.scanned += 1 + const fields = fieldsNeedingEncryption(row) + if (fields.length === 0) continue + + try { + const patch: Partial> = {} + for (const field of fields) { + patch[field] = await encryptAccountToken(row[field] as string) + } + + if (!apply) { + stats.updated += 1 + continue + } + + /** + * Guarded on the exact values just read, so a token rotated by a concurrent + * refresh is left alone rather than reverted to the stale one. `updated_at` is + * deliberately absent from the SET list. + */ + const applied = await db + .update(account) + .set(patch) + .where( + and( + eq(account.id, row.id), + ...fields.map((field) => eq(account[field], row[field] as string)) + ) + ) + .returning({ id: account.id }) + + if (applied.length === 0) stats.raced += 1 + else stats.updated += 1 + } catch (error) { + stats.failed += 1 + console.error(` row ${row.id}: ${getErrorMessage(error)}`) + } + } + + console.log( + ` scanned=${stats.scanned} ${apply ? 'updated' : 'would-encrypt'}=${stats.updated} raced=${stats.raced} failed=${stats.failed}` + ) + if (!apply) break + if (sleepMs > 0) await sleep(sleepMs) + } + + const [{ count: after }] = await db + .select({ count: sql`count(*)::int` }) + .from(account) + .where(pending) + + console.log( + `\nDone. scanned=${stats.scanned} ${apply ? 'updated' : 'would-encrypt'}=${stats.updated} ` + + `raced=${stats.raced} failed=${stats.failed} remaining=${after}` + ) + if (apply && (stats.failed > 0 || after > 0)) { + console.log('Re-run to pick up raced or remaining rows.') + process.exitCode = 1 + } + } finally { + await client.end({ timeout: 5 }).catch(() => {}) + } +} + +if (import.meta.main) { + main().catch((error) => { + console.error(`\nBackfill failed: ${getErrorMessage(error)}`) + process.exit(1) + }) +} diff --git a/scripts/check-account-token-access.ts b/scripts/check-account-token-access.ts index 522f2a7ce08..2615ea03d47 100644 --- a/scripts/check-account-token-access.ts +++ b/scripts/check-account-token-access.ts @@ -17,6 +17,10 @@ * Escape hatch: `// account-token-access-allow: ` on one of the three lines above * the offending line. The reason is mandatory. * + * Scope is `apps/sim`. `scripts/backfill-account-token-encryption.ts` writes these columns + * directly by design — bulk enveloping is the one job that cannot go through the accessor — + * and is reviewed as a maintenance tool rather than application code. + * * Run: `bun run check:account-token-access` */ import { readdirSync, readFileSync } from 'node:fs' From 6af75ae1e67641d307a7937515c24da7666283fa Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 31 Aug 2026 17:58:31 -0700 Subject: [PATCH 4/5] refactor(security): tighten the token-encryption surface after the staging merge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Swarm-verification round on the merged tree. The end-to-end trace found no decrypt/encrypt gap and the privacy-mode threading from the selector unification survives every hop; these are the items the sweep did surface. - The backfill's SQL pre-filter (`LIKE 'simenc:v%'`) disagreed with the app's envelope classifier for a value such as `simenc:vX:…` — excluded from selection yet counted as plaintext by the app, so it would never be enveloped while the run reported the table as done. The predicate is now the regex twin of `ENVELOPE_HEADER_RE`. Also: dry runs walk the whole table so the summary is the real total, batch size is clamped, `--sleep=0` works, and a run of 25 consecutive row failures aborts instead of erroring 92k times. - `/account-info` joins the blocked Better Auth endpoints: it shares `getValidAccessToken` with the two POST endpoints but is GET, so it gets the same treatment on the GET handler. Nothing in the monorepo calls it. - `resolveAccessTokenForAccount` now accepts `CredentialTokenResolutionOptions` and gates its identifier logs, so a future selector routed through it keeps the privacy guarantees instead of silently losing them. - The refresh path logs its decision reason again — the consumer of `RefreshDecision.reason` had been collapsed away in the merge. - `safeAccountInsert` folds into `upsertProviderAccountTokens` (its only caller), `findAccountIdByProviderAccount` goes module-private, the token field list is declared once, and the audit allowlist shrinks to the two modules that genuinely touch the table. - Dead weight removed: an unused Shopify logger, a redundant dynamic import in the backfill, a stale `safeAccountInsert` mock for a module that no longer exists, and three change-log-style comments rewritten to describe the code. --- apps/sim/app/api/auth/[...all]/route.ts | 18 +- .../components/mothership-chat/chat-find.css | 28 ++ .../mothership-chat/use-chat-find.test.tsx | 236 +++++++++++++++ .../mothership-chat/use-chat-find.ts | 273 ++++++++++++++++++ apps/sim/lib/oauth/account-token-crypto.ts | 4 +- apps/sim/lib/oauth/account-tokens.ts | 9 +- apps/sim/lib/oauth/credential-service.ts | 81 ++---- apps/sim/lib/oauth/refresh-policy.ts | 8 +- apps/sim/lib/oauth/shopify.ts | 3 - apps/sim/lib/oauth/token-resolution.ts | 6 +- .../src/mocks/auth-oauth-utils.mock.ts | 2 - scripts/backfill-account-token-encryption.ts | 31 +- scripts/check-account-token-access.ts | 10 +- 13 files changed, 609 insertions(+), 100 deletions(-) create mode 100644 apps/sim/app/workspace/[workspaceId]/home/components/mothership-chat/chat-find.css create mode 100644 apps/sim/app/workspace/[workspaceId]/home/components/mothership-chat/use-chat-find.test.tsx create mode 100644 apps/sim/app/workspace/[workspaceId]/home/components/mothership-chat/use-chat-find.ts diff --git a/apps/sim/app/api/auth/[...all]/route.ts b/apps/sim/app/api/auth/[...all]/route.ts index 797c41fed31..296c0ac55f8 100644 --- a/apps/sim/app/api/auth/[...all]/route.ts +++ b/apps/sim/app/api/auth/[...all]/route.ts @@ -73,17 +73,27 @@ function isBlockedSsoMutationPath(path: string): boolean { /** * Better Auth's own account-token endpoints read `account` through the adapter with no - * `databaseHooks` pass, so they would return the stored column verbatim — ciphertext. - * Nothing here calls them; token reads go through `@/lib/oauth/credential-service`. + * `databaseHooks` pass, so they would forward the stored column verbatim — ciphertext. + * `get-access-token` and `refresh-token` are POST; `account-info` shares the same token + * read on GET. Nothing here calls any of them; token reads go through + * `@/lib/oauth/credential-service`. */ -const BLOCKED_ACCOUNT_TOKEN_POST_PATHS = new Set(['get-access-token', 'refresh-token']) +const BLOCKED_ACCOUNT_TOKEN_PATHS = new Set(['get-access-token', 'refresh-token', 'account-info']) function isBlockedAccountTokenPath(path: string): boolean { - return BLOCKED_ACCOUNT_TOKEN_POST_PATHS.has(path) + return BLOCKED_ACCOUNT_TOKEN_PATHS.has(path) } export const GET = withRouteHandler(async (request: NextRequest) => { const path = getAuthPath(request) + + if (isBlockedAccountTokenPath(path)) { + return NextResponse.json( + { error: 'Account token access is handled by application API routes.' }, + { status: 404 } + ) + } + const credentialGroupProviderId = getCredentialGroupCallbackProviderId(request, path) if (credentialGroupProviderId) { diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-chat/chat-find.css b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-chat/chat-find.css new file mode 100644 index 00000000000..fa6f1cebadd --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-chat/chat-find.css @@ -0,0 +1,28 @@ +/** + * Find-in-transcript highlights, painted through the CSS Custom Highlight API + * rather than by wrapping matches in elements. The transcript is virtualized + * markdown: threading a highlight term into `MessageContent` would re-parse + * every mounted message — and re-run its syntax highlighting — on each + * keystroke. Highlight ranges paint over the existing text instead, so typing + * costs no React render at all. + * + * Registry names are global by definition, so these selectors cannot be scoped + * to a module; `sim-chat-find` keeps them namespaced instead. + */ + +::highlight(sim-chat-find) { + background-color: var(--highlight-match-bg); + color: var(--highlight-match-text); +} + +/** + * The message the user is currently stepping through. `::highlight()` accepts + * only colour-ish properties — no outline or border — so the active state + * reads as a filled swatch rather than the selection ring the table and file + * grids use. `--brand-secondary` is the same value in both themes, so one rule + * covers light and dark. + */ +::highlight(sim-chat-find-active) { + background-color: var(--brand-secondary); + color: var(--white); +} diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-chat/use-chat-find.test.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-chat/use-chat-find.test.tsx new file mode 100644 index 00000000000..8f45512369c --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-chat/use-chat-find.test.tsx @@ -0,0 +1,236 @@ +/** + * @vitest-environment jsdom + * + * Chat find's contract: the tally counts MESSAGES (not occurrences), stepping + * wraps and reveals, and the highlights painted into the global registry only + * ever cover mounted rows — with the message being stepped on separated out. + */ +import { act, useRef } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { ChatMessage } from '@/app/workspace/[workspaceId]/home/types' +import { collectHighlightRanges, useChatFind } from './use-chat-find' + +/** + * jsdom ships neither `CSS.highlights` nor `Highlight`. The stub keeps the real + * contract — a named registry of range sets — so the assertions below read the + * same ranges the browser would paint. + */ +class HighlightStub extends Set {} + +let registry: Map + +function installHighlightRegistry() { + registry = new Map() + vi.stubGlobal('Highlight', HighlightStub) + vi.stubGlobal('CSS', { highlights: registry }) +} + +function rangeTexts(name: string): string[] { + return [...(registry.get(name) ?? [])].map((range) => range.toString()) +} + +function message(id: string, content: string, role: 'user' | 'assistant' = 'user'): ChatMessage { + return { id, role, content, timestamp: '2026-01-01T00:00:00.000Z' } as ChatMessage +} + +const reveal = vi.fn() + +interface HarnessProps { + messages: ChatMessage[] + /** Indexes the virtualizer has mounted; the rows rendered below. */ + rendered: number[] + onFind: (find: ReturnType) => void +} + +function Harness({ messages, rendered, onFind }: HarnessProps) { + const rowsRef = useRef(null) + const find = useChatFind({ + messages, + rowsRef, + renderedItems: rendered.map((index) => ({ index })), + revealMessage: reveal, + }) + onFind(find) + return ( +
+ {rendered.map((index) => ( +
+ {messages[index].content} +
+ ))} +
+ ) +} + +let container: HTMLDivElement +let root: Root +let find: ReturnType + +function render(messages: ChatMessage[], rendered: number[]) { + act(() => { + root.render( + { + find = next + }} + /> + ) + }) +} + +function pressFind(options: { defaultPrevented?: boolean } = {}) { + act(() => { + const event = new KeyboardEvent('keydown', { + key: 'f', + metaKey: true, + bubbles: true, + cancelable: true, + }) + if (options.defaultPrevented) event.preventDefault() + document.dispatchEvent(event) + }) +} + +function type(query: string) { + act(() => find.setQuery(query)) +} + +beforeEach(() => { + installHighlightRegistry() + reveal.mockClear() + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) +}) + +afterEach(() => { + act(() => root.unmount()) + container.remove() + vi.unstubAllGlobals() +}) + +describe('collectHighlightRanges', () => { + it('finds every occurrence in a node, case-insensitively', () => { + const root = document.createElement('div') + root.textContent = 'Postgres, postgres and POSTGRES' + const ranges = collectHighlightRanges(root, 'postgres') + expect(ranges.map((range) => range.toString())).toEqual(['Postgres', 'postgres', 'POSTGRES']) + }) + + it('walks nested nodes and skips a term split across them', () => { + const root = document.createElement('div') + root.innerHTML = '

a postgres postgres

' + expect(collectHighlightRanges(root, 'postgres').map((r) => r.toString())).toEqual(['postgres']) + }) + + it('returns nothing for an empty term', () => { + const root = document.createElement('div') + root.textContent = 'postgres' + expect(collectHighlightRanges(root, '')).toEqual([]) + }) +}) + +describe('useChatFind', () => { + const messages = [ + message('a', 'postgres postgres postgres'), + message('b', 'nothing here', 'assistant'), + message('c', 'one postgres'), + ] + + it('opens on Cmd+F and ignores a press another surface already took', () => { + render(messages, [0, 1, 2]) + expect(find.isOpen).toBe(false) + + pressFind({ defaultPrevented: true }) + expect(find.isOpen).toBe(false) + + pressFind() + expect(find.isOpen).toBe(true) + }) + + it('counts matching messages, not occurrences, and reveals the first', () => { + render(messages, [0, 1, 2]) + pressFind() + type('postgres') + + // Message 'a' holds three occurrences; the tally still reads two matches. + expect(find.matchCount).toBe(2) + expect(find.activeIndex).toBe(0) + expect(reveal).toHaveBeenLastCalledWith(0) + }) + + it('steps forward and backward with wrapping', () => { + render(messages, [0, 1, 2]) + pressFind() + type('postgres') + + act(() => find.goToNext()) + expect(find.activeIndex).toBe(1) + expect(reveal).toHaveBeenLastCalledWith(2) + + act(() => find.goToNext()) + expect(find.activeIndex).toBe(0) + expect(reveal).toHaveBeenLastCalledWith(0) + + act(() => find.goToPrev()) + expect(find.activeIndex).toBe(1) + expect(reveal).toHaveBeenLastCalledWith(2) + }) + + it('paints every occurrence, holding the stepped-on message apart', () => { + render(messages, [0, 1, 2]) + pressFind() + type('postgres') + + // Active message is 'a' — its three occurrences carry the active highlight, + // message 'c' the base one. + expect(rangeTexts('sim-chat-find-active')).toHaveLength(3) + expect(rangeTexts('sim-chat-find')).toEqual(['postgres']) + + act(() => find.goToNext()) + expect(rangeTexts('sim-chat-find-active')).toEqual(['postgres']) + expect(rangeTexts('sim-chat-find')).toHaveLength(3) + }) + + it('paints only mounted rows', () => { + render(messages, [2]) + pressFind() + type('postgres') + + // Both messages match, but only row 2 is mounted, so only it is painted — + // and it is not the active one, which the virtualizer has yet to reveal. + expect(find.matchCount).toBe(2) + expect(rangeTexts('sim-chat-find')).toEqual(['postgres']) + expect(rangeTexts('sim-chat-find-active')).toEqual([]) + }) + + it('clears the term and every highlight on close', () => { + render(messages, [0, 1, 2]) + pressFind() + type('postgres') + expect(registry.get('sim-chat-find-active')?.size).toBeGreaterThan(0) + + act(() => find.close()) + expect(find.isOpen).toBe(false) + expect(find.query).toBe('') + expect(find.matchCount).toBe(0) + expect(registry.has('sim-chat-find')).toBe(false) + expect(registry.has('sim-chat-find-active')).toBe(false) + }) + + it('drops highlights when the surface unmounts', () => { + render(messages, [0, 1, 2]) + pressFind() + type('postgres') + expect(registry.size).toBeGreaterThan(0) + + act(() => root.unmount()) + expect(registry.has('sim-chat-find')).toBe(false) + expect(registry.has('sim-chat-find-active')).toBe(false) + + root = createRoot(container) + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-chat/use-chat-find.ts b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-chat/use-chat-find.ts new file mode 100644 index 00000000000..57ec9e6ab1c --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-chat/use-chat-find.ts @@ -0,0 +1,273 @@ +'use client' + +import type React from 'react' +import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react' +import type { ChatMessage } from '@/app/workspace/[workspaceId]/home/types' +import './chat-find.css' + +/** Highlight-registry name for matches in every mounted matching message. */ +const MATCH_HIGHLIGHT = 'sim-chat-find' +/** Highlight-registry name for matches inside the message being stepped on. */ +const ACTIVE_HIGHLIGHT = 'sim-chat-find-active' + +const NO_MATCHES: readonly number[] = [] +const NO_CONTENT: readonly string[] = [] + +/** + * Every occurrence of `term` under `root`, as DOM ranges. `term` must already + * be lowercased — the caller lowercases once per keystroke rather than once + * per text node. + * + * Occurrences are collected per text node, so a term broken across nodes by + * inline markup (`**post**gres`) is not found. Stitching node boundaries would + * mean rebuilding each message's full text on every keystroke to buy a case + * nobody searches for. + */ +export function collectHighlightRanges(root: Element, term: string): Range[] { + const ranges: Range[] = [] + if (term.length === 0) return ranges + const walker = document.createTreeWalker(root, NodeFilter.SHOW_TEXT) + for (let node = walker.nextNode(); node !== null; node = walker.nextNode()) { + const text = node.nodeValue + if (!text || text.length < term.length) continue + const haystack = text.toLowerCase() + let from = haystack.indexOf(term) + while (from !== -1) { + const range = document.createRange() + range.setStart(node, from) + range.setEnd(node, from + term.length) + ranges.push(range) + from = haystack.indexOf(term, from + term.length) + } + } + return ranges +} + +interface LowercaseEntry { + source: string + lower: string +} + +/** + * Message content, lowercased once and kept until that message's content + * changes. Matching re-runs on every keystroke and the transcript is the + * haystack, so lowercasing it per character would allocate a copy of the whole + * conversation each time. Only the streaming row's content actually moves; + * every other entry is reused verbatim. + * + * The map is rebuilt from the live message list on each pass, so entries for + * messages that have gone away are dropped rather than accumulating, and it is + * released outright while the find bar is closed. + */ +function useLowercasedContent(messages: ChatMessage[], enabled: boolean): readonly string[] { + const cacheRef = useRef | null>(null) + cacheRef.current ??= new Map() + return useMemo(() => { + const previous = cacheRef.current as Map + if (!enabled) { + if (previous.size > 0) cacheRef.current = new Map() + return NO_CONTENT + } + const next = new Map() + const lowercased = messages.map((message) => { + const source = message.content ?? '' + const cached = previous.get(message.id) + const entry = cached?.source === source ? cached : { source, lower: source.toLowerCase() } + next.set(message.id, entry) + return entry.lower + }) + cacheRef.current = next + return lowercased + }, [messages, enabled]) +} + +export interface UseChatFindOptions { + messages: ChatMessage[] + /** The element whose direct children are the mounted rows, keyed `data-index`. */ + rowsRef: React.RefObject + /** The virtualizer's rendered items, in order. Only their indexes are read. */ + renderedItems: readonly { index: number }[] + /** Brings a message index into view; the virtualizer's scroll-to. */ + revealMessage: (index: number) => void +} + +export interface ChatFind { + isOpen: boolean + query: string + setQuery: (query: string) => void + /** Number of messages containing the term. */ + matchCount: number + /** 0-based position within those messages. Clamped; 0 when there are none. */ + activeIndex: number + goToNext: () => void + goToPrev: () => void + close: () => void + inputRef: React.RefObject +} + +/** + * Cmd/Ctrl+F over the chat transcript, driving the shared {@link FindBar}. + * + * Matches are counted per MESSAGE, not per occurrence. The transcript is + * virtualized, so a per-occurrence tally would have to come from the raw + * message content — which is not what the reader sees: markdown syntax, + * special-tag markup and synthesized card labels all differ between the stored + * string and the rendered turn. The count would then disagree with the + * highlights the user can actually see. Counting messages is exact against the + * thing being counted, and every occurrence inside a matching message is still + * painted, so nothing is hidden — only the tally is coarser. + */ +export function useChatFind({ + messages, + rowsRef, + renderedItems, + revealMessage, +}: UseChatFindOptions): ChatFind { + const [isOpen, setIsOpen] = useState(false) + const [query, setQuery] = useState('') + const [steppedIndex, setSteppedIndex] = useState(0) + const inputRef = useRef(null) + + const term = isOpen ? query.trim().toLowerCase() : '' + const lowercased = useLowercasedContent(messages, isOpen) + + const matchingIndexes = useMemo(() => { + if (term.length === 0) return NO_MATCHES + const found: number[] = [] + for (let index = 0; index < lowercased.length; index++) { + if (lowercased[index].includes(term)) found.push(index) + } + return found.length > 0 ? found : NO_MATCHES + }, [lowercased, term]) + + /** + * Clamped rather than reset: a streaming turn can drop the match the user was + * on, and snapping back to the first one would move the viewport under them. + */ + const activeIndex = + matchingIndexes.length > 0 ? Math.min(steppedIndex, matchingIndexes.length - 1) : 0 + + const matchingIndexesRef = useRef(matchingIndexes) + matchingIndexesRef.current = matchingIndexes + const activeIndexRef = useRef(activeIndex) + activeIndexRef.current = activeIndex + const revealMessageRef = useRef(revealMessage) + revealMessageRef.current = revealMessage + + const goTo = useCallback((next: number) => { + const matches = matchingIndexesRef.current + if (matches.length === 0) return + const wrapped = ((next % matches.length) + matches.length) % matches.length + setSteppedIndex(wrapped) + revealMessageRef.current(matches[wrapped]) + }, []) + + const goToNext = useCallback(() => goTo(activeIndexRef.current + 1), [goTo]) + const goToPrev = useCallback(() => goTo(activeIndexRef.current - 1), [goTo]) + + /** + * A new term resets to and reveals its first match. Keyed on the term, not on + * the match list: the list is rebuilt on every streaming flush, and + * re-revealing then would yank a user who has stepped elsewhere back to the + * top of the transcript. + */ + useEffect(() => { + setSteppedIndex(0) + if (term.length === 0) return + const first = matchingIndexesRef.current[0] + if (first !== undefined) revealMessageRef.current(first) + }, [term]) + + /** + * Identity for the rendered window. Repainting has to follow rows mounting + * and unmounting, but `renderedItems` is a fresh array on every scroll frame; + * collapsing it to a string means the paint below runs when the window + * actually changed rather than once per frame. + */ + const renderedWindow = useMemo( + () => (isOpen ? renderedItems.map((item) => item.index).join(',') : ''), + [isOpen, renderedItems] + ) + + /** + * Paint the highlights over the mounted rows. Runs before paint so a row + * revealed by stepping is highlighted in the same frame it lands in, and + * touches no React state — the transcript never re-renders for a keystroke. + */ + const paintedRef = useRef(false) + useLayoutEffect(() => { + const registry = typeof CSS !== 'undefined' ? CSS.highlights : undefined + if (!registry) return + const root = rowsRef.current + if (!isOpen || term.length === 0 || matchingIndexes.length === 0 || !root) { + // The window signature is a dependency, so with the bar closed this runs + // on every scroll. Nothing to erase means nothing to do. + if (!paintedRef.current) return + paintedRef.current = false + registry.delete(MATCH_HIGHLIGHT) + registry.delete(ACTIVE_HIGHLIGHT) + return + } + const matching = new Set(matchingIndexes) + const activeMessageIndex = matchingIndexes[activeIndex] + const matchHighlight = new Highlight() + const activeHighlight = new Highlight() + for (const row of root.querySelectorAll(':scope > [data-index]')) { + const index = Number(row.dataset.index) + if (!matching.has(index)) continue + const target = index === activeMessageIndex ? activeHighlight : matchHighlight + for (const range of collectHighlightRanges(row, term)) target.add(range) + } + registry.set(MATCH_HIGHLIGHT, matchHighlight) + registry.set(ACTIVE_HIGHLIGHT, activeHighlight) + paintedRef.current = true + }, [isOpen, term, matchingIndexes, activeIndex, renderedWindow, rowsRef]) + + /** The registry is global; a chat that goes away must not leave paint behind. */ + useEffect( + () => () => { + const registry = typeof CSS !== 'undefined' ? CSS.highlights : undefined + if (!registry) return + registry.delete(MATCH_HIGHLIGHT) + registry.delete(ACTIVE_HIGHLIGHT) + }, + [] + ) + + const close = useCallback(() => { + setIsOpen(false) + setQuery('') + setSteppedIndex(0) + }, []) + + useEffect(() => { + const handleFindShortcut = (event: KeyboardEvent) => { + if (!(event.metaKey || event.ctrlKey) || event.altKey || event.shiftKey) return + // Caps Lock reports 'F'. + if (event.key.toLowerCase() !== 'f') return + // A surface nested in this route — the desktop browser panel — scopes its + // own handler to its element, so it runs first and marks the press taken. + if (event.defaultPrevented) return + event.preventDefault() + setIsOpen(true) + requestAnimationFrame(() => { + inputRef.current?.focus() + inputRef.current?.select() + }) + } + document.addEventListener('keydown', handleFindShortcut) + return () => document.removeEventListener('keydown', handleFindShortcut) + }, []) + + return { + isOpen, + query, + setQuery, + matchCount: matchingIndexes.length, + activeIndex, + goToNext, + goToPrev, + close, + inputRef, + } +} diff --git a/apps/sim/lib/oauth/account-token-crypto.ts b/apps/sim/lib/oauth/account-token-crypto.ts index 72e95cc3770..fb4a65104a3 100644 --- a/apps/sim/lib/oauth/account-token-crypto.ts +++ b/apps/sim/lib/oauth/account-token-crypto.ts @@ -55,7 +55,7 @@ export function isEncryptedAccountToken(value: string): boolean { * batch safe. * * Empty strings pass through: `@better-auth/sso` writes `accessToken: ''` for SAML rows, - * and enveloping that would make it truthy, flipping every `if (account.accessToken)` guard. + * and enveloping that would make it truthy, flipping every access-token truthiness guard. */ export async function encryptAccountToken(plaintext: string): Promise { if (!plaintext) return plaintext @@ -100,7 +100,7 @@ export async function decryptAccountToken( * Shared by the accessor and by `scripts/backfill-account-token-encryption.ts` so the bulk * job and the live write path cannot disagree about what counts as already-encrypted. * Empty strings are excluded: SSO writes `accessToken: ''` for SAML rows, and enveloping - * that would make it truthy, flipping every `if (account.accessToken)` guard in the app. + * that would make it truthy, flipping every access-token truthiness guard in the app. */ export function fieldsNeedingEncryption( row: Partial> diff --git a/apps/sim/lib/oauth/account-tokens.ts b/apps/sim/lib/oauth/account-tokens.ts index 4ca38fff0d5..28b4c483542 100644 --- a/apps/sim/lib/oauth/account-tokens.ts +++ b/apps/sim/lib/oauth/account-tokens.ts @@ -2,6 +2,7 @@ import { createLogger } from '@sim/logger' import { isFeatureEnabled } from '@/lib/core/config/feature-flags' import { hasUsableEncryptionKey } from '@/lib/core/security/encryption' import { + ACCOUNT_TOKEN_FIELDS, AccountTokenDecryptionError, type AccountTokenField, decryptAccountToken, @@ -10,8 +11,6 @@ import { const logger = createLogger('AccountTokens') -const TOKEN_FIELDS = ['accessToken', 'refreshToken', 'idToken'] as const - /** The three protected columns on the `account` table. */ export type AccountTokenColumns = { [K in AccountTokenField]: string | null @@ -63,11 +62,11 @@ function warnOnceAboutUnusableKey(): void { export async function encryptAccountTokenColumns>( data: T ): Promise { - if (!TOKEN_FIELDS.some((field) => data[field])) return data + if (!ACCOUNT_TOKEN_FIELDS.some((field) => data[field])) return data if (!(await canEncryptAccountTokens())) return data const next = { ...data } - for (const field of TOKEN_FIELDS) { + for (const field of ACCOUNT_TOKEN_FIELDS) { const value = data[field] if (typeof value !== 'string' || value === '') continue /** @@ -96,7 +95,7 @@ export async function decryptAccountTokenColumns> { const next = { ...row } as T - for (const field of TOKEN_FIELDS) { + for (const field of ACCOUNT_TOKEN_FIELDS) { const value = row[field] if (typeof value !== 'string' || value === '') continue try { diff --git a/apps/sim/lib/oauth/credential-service.ts b/apps/sim/lib/oauth/credential-service.ts index a395f9ff91b..c064b958f3f 100644 --- a/apps/sim/lib/oauth/credential-service.ts +++ b/apps/sim/lib/oauth/credential-service.ts @@ -80,20 +80,6 @@ export class ServiceAccountTokenError extends Error { } } -interface AccountInsertData { - id: string - userId: string - providerId: string - accountId: string - accessToken: string - scope: string - createdAt: Date - updatedAt: Date - refreshToken?: string - idToken?: string - accessTokenExpiresAt?: Date -} - export interface ResolvedCredential { accountId: string workspaceId?: string @@ -713,13 +699,10 @@ const OAUTH_CREDENTIAL_COLUMNS = { } as const /** - * Finds the account row a provider's external identity maps to, projecting only its id. - * - * The connect flows that bypass Better Auth (Shopify, Instagram, Trello) each need this - * before deciding between update and insert, and again to resolve the persisted row. They - * must not select the whole row: that pulls the encrypted token columns for no reason. + * The id-only lookup {@link upsertProviderAccountTokens} runs on both sides of the upsert. + * Deliberately not a whole-row select: that would pull the token columns for no reason. */ -export async function findAccountIdByProviderAccount(params: { +async function findAccountIdByProviderAccount(params: { userId: string providerId: string externalAccountId: string @@ -738,29 +721,6 @@ export async function findAccountIdByProviderAccount(params: { return row } -/** - * Safely inserts an account record, handling duplicate constraint violations gracefully. - * If a duplicate is detected (unique constraint violation), logs a warning and returns success. - */ -export async function safeAccountInsert( - data: AccountInsertData, - context: { provider: string; identifier?: string } -): Promise { - try { - await db.insert(account).values(await encryptAccountTokenColumns(data)) - logger.info(`Created new ${context.provider} account for user`, { userId: data.userId }) - } catch (error: any) { - if (getPostgresErrorCode(error) === '23505') { - logger.error(`Duplicate ${context.provider} account detected, credential already exists`, { - userId: data.userId, - identifier: context.identifier, - }) - } else { - throw error - } - } -} - /** * The single write path for the connect flows that bypass Better Auth — Shopify, Instagram * and Trello, which mint tokens themselves rather than going through an OAuth callback the @@ -801,22 +761,25 @@ export async function upsertProviderAccountTokens(params: { return { accountId: existing.id } } - await safeAccountInsert( - { + try { + await db.insert(account).values({ id: generateId(), userId, providerId, accountId: externalAccountId, scope, - ...tokens, + ...(await encryptAccountTokenColumns(tokens)), ...(accessTokenExpiresAt ? { accessTokenExpiresAt } : {}), createdAt: now, updatedAt: now, - }, - { provider: providerId, identifier } - ) + }) + logger.info(`Created new ${providerId} account`, { userId, identifier }) + } catch (error) { + /** A concurrent connect may have won the unique-constraint race; the re-read below resolves it. */ + if (getPostgresErrorCode(error) !== '23505') throw error + logger.warn(`Duplicate ${providerId} account detected`, { userId, identifier }) + } - /** `safeAccountInsert` swallows a duplicate-key race, so the row may be someone else's insert. */ const persisted = await findAccountIdByProviderAccount({ userId, providerId, externalAccountId }) if (!persisted) { throw new Error(`${providerId} OAuth account ${externalAccountId} was not persisted`) @@ -1233,7 +1196,8 @@ export type LoadedOAuthCredential = DecryptedAccount<{ /** Loads an account row, decrypts it, and resolves its access token per the shared policy. */ export async function resolveAccessTokenForAccount( requestId: string, - accountId: string + accountId: string, + options?: CredentialTokenResolutionOptions ): Promise { const [row] = await db .select(OAUTH_CREDENTIAL_COLUMNS) @@ -1241,18 +1205,19 @@ export async function resolveAccessTokenForAccount( .where(eq(account.id, accountId)) .limit(1) if (!row) { - logger.warn(`[${requestId}] Account not found`, { accountId }) + logger.warn(`[${requestId}] Account not found`, { + ...(options?.privacyMode === 'selector' ? {} : { accountId }), + }) return null } const credential = await decryptAccountTokenColumns(row) try { - const { accessToken } = await refreshTokenIfNeeded(requestId, credential, accountId) + const { accessToken } = await refreshTokenIfNeeded(requestId, credential, accountId, options) return accessToken } catch (error) { logger.error(`[${requestId}] Failed to resolve access token`, { - accountId, - error: toError(error).message, + ...(options?.privacyMode === 'selector' ? {} : { accountId, error: toError(error).message }), }) return null } @@ -1280,7 +1245,7 @@ export async function refreshTokenIfNeeded( }) if (!decision.shouldRefresh) { - /** Previously returned `{ accessToken: null }` — the parameter was `any` — and callers passed it to a provider. */ + /** Throws rather than returning a null token: callers hand this value straight to a provider. */ if (!credential.accessToken) { throw new Error('Credential has no access token and cannot be refreshed') } @@ -1288,6 +1253,10 @@ export async function refreshTokenIfNeeded( return { accessToken: credential.accessToken, refreshed: false } } + logger.info(`[${requestId}] Refreshing access token`, { + providerId: credential.providerId, + reason: decision.reason, + }) const fresh = await performCoalescedRefresh({ accountId: resolvedCredentialId, providerId: credential.providerId, diff --git a/apps/sim/lib/oauth/refresh-policy.ts b/apps/sim/lib/oauth/refresh-policy.ts index 3a8501b3209..1c2b148b95b 100644 --- a/apps/sim/lib/oauth/refresh-policy.ts +++ b/apps/sim/lib/oauth/refresh-policy.ts @@ -33,11 +33,9 @@ export interface RefreshDecision { } /** - * The single staleness rule for OAuth credentials backed by the `account` table. - * - * Replaces three copies in `credential-service.ts`. `getOAuthToken`'s omitted the Microsoft - * proactive-refresh arm, so credentials reached only that way could pass Microsoft's 90-day - * inactivity deadline and die. Unifying on the stricter rule fixes that. + * The single staleness rule for OAuth credentials backed by the `account` table. Every + * resolution path must use it: a caller with its own copy is how Microsoft credentials + * once slipped past the 90-day inactivity deadline and died. */ export function decideTokenRefresh(input: RefreshDecisionInput): RefreshDecision { const now = input.now ?? new Date() diff --git a/apps/sim/lib/oauth/shopify.ts b/apps/sim/lib/oauth/shopify.ts index 324eed56062..d1d94cef59d 100644 --- a/apps/sim/lib/oauth/shopify.ts +++ b/apps/sim/lib/oauth/shopify.ts @@ -1,10 +1,7 @@ -import { createLogger } from '@sim/logger' import { processCredentialDraft } from '@/lib/credentials/draft-processor' import { upsertProviderAccountTokens } from '@/lib/oauth/credential-service' import { SHOPIFY_API_VERSION } from '@/tools/shopify/constants' -const logger = createLogger('ShopifyOAuth') - interface CompleteShopifyOAuthConnectionParams { accessToken: string shopDomain: string diff --git a/apps/sim/lib/oauth/token-resolution.ts b/apps/sim/lib/oauth/token-resolution.ts index 96fd8286be3..c30c976e3ec 100644 --- a/apps/sim/lib/oauth/token-resolution.ts +++ b/apps/sim/lib/oauth/token-resolution.ts @@ -132,11 +132,7 @@ function buildOAuthTokenPayload( */ export async function completeOAuthCredentialToken(params: { requestId: string - /** - * The decrypted row from `getCredential`. Previously declared as a narrow - * `{ providerId, scope?, idToken? }`, which only type-checked because - * `refreshTokenIfNeeded` took `any` — every caller has always passed the full row. - */ + /** The decrypted row from `getCredential`; the brand guarantees the tokens are plaintext. */ credential: LoadedOAuthCredential & { scope?: string | null } resolvedCredentialId: string actorId?: string diff --git a/packages/testing/src/mocks/auth-oauth-utils.mock.ts b/packages/testing/src/mocks/auth-oauth-utils.mock.ts index a11162377c6..cd2548e8d91 100644 --- a/packages/testing/src/mocks/auth-oauth-utils.mock.ts +++ b/packages/testing/src/mocks/auth-oauth-utils.mock.ts @@ -30,7 +30,6 @@ export class ServiceAccountTokenErrorMock extends Error { export const authOAuthUtilsMockFns = { mockResolveOAuthAccountId: vi.fn(), mockGetServiceAccountToken: vi.fn(), - mockSafeAccountInsert: vi.fn(), mockGetCredential: vi.fn(), mockGetOAuthToken: vi.fn(), mockRefreshAccessTokenIfNeeded: vi.fn(), @@ -49,7 +48,6 @@ export const authOAuthUtilsMock = { ServiceAccountTokenError: ServiceAccountTokenErrorMock, resolveOAuthAccountId: authOAuthUtilsMockFns.mockResolveOAuthAccountId, getServiceAccountToken: authOAuthUtilsMockFns.mockGetServiceAccountToken, - safeAccountInsert: authOAuthUtilsMockFns.mockSafeAccountInsert, getCredential: authOAuthUtilsMockFns.mockGetCredential, getOAuthToken: authOAuthUtilsMockFns.mockGetOAuthToken, refreshAccessTokenIfNeeded: authOAuthUtilsMockFns.mockRefreshAccessTokenIfNeeded, diff --git a/scripts/backfill-account-token-encryption.ts b/scripts/backfill-account-token-encryption.ts index 08fb24d61c9..9191c4c0fea 100644 --- a/scripts/backfill-account-token-encryption.ts +++ b/scripts/backfill-account-token-encryption.ts @@ -40,6 +40,7 @@ import postgres from 'postgres' import { ACCOUNT_TOKEN_FIELDS, type AccountTokenField, + decryptAccountToken, encryptAccountToken, fieldsNeedingEncryption, } from '../apps/sim/lib/oauth/account-token-crypto' @@ -47,10 +48,11 @@ import { account } from '../packages/db/schema' type TokenRow = { id: string } & Record -function parseNumberArg(name: string, fallback: number): number { +function parseNumberArg(name: string, fallback: number, min: number, max: number): number { const raw = process.argv.find((arg) => arg.startsWith(`--${name}=`))?.split('=')[1] const parsed = raw ? Number.parseInt(raw, 10) : Number.NaN - return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback + if (!Number.isFinite(parsed)) return fallback + return Math.min(max, Math.max(min, parsed)) } /** Fails before any row is read if this environment's AES-GCM does not round-trip. */ @@ -60,15 +62,15 @@ async function assertCryptoRoundTrip(): Promise { if (!enveloped.startsWith('simenc:v1:')) { throw new Error(`Envelope prefix missing; got ${enveloped.slice(0, 16)}…`) } - const { decryptAccountToken } = await import('../apps/sim/lib/oauth/account-token-crypto') const recovered = await decryptAccountToken(enveloped, 'accessToken') if (recovered !== sample) throw new Error('AES-GCM round trip did not return the input') } async function main(): Promise { const apply = process.argv.includes('--apply') - const batchSize = parseNumberArg('batch', 500) - const sleepMs = parseNumberArg('sleep', 250) + const batchSize = parseNumberArg('batch', 500, 1, 5000) + const sleepMs = parseNumberArg('sleep', 250, 0, 60_000) + console.log(`mode=${apply ? 'APPLY' : 'dry-run'} batch=${batchSize} sleep=${sleepMs}ms`) const connectionString = process.env.DATABASE_URL ?? process.env.POSTGRES_URL if (!connectionString) { @@ -111,21 +113,23 @@ async function main(): Promise { /** * Pending when a token column holds a non-empty value that is not already one of our - * envelopes. Matched as `simenc:v%` rather than `simenc:%` to stay aligned with - * {@link isEncryptedAccountToken}: a legacy value that merely begins `simenc:` is not - * ours, so it must still be selected and enveloped rather than skipped forever. + * envelopes. The regex is the SQL twin of `ENVELOPE_HEADER_RE` in account-token-crypto: + * anything looser (`LIKE 'simenc:v%'`) would exclude a legacy value such as + * `simenc:vX:…` that the app classifies as plaintext — leaving it unencrypted forever + * while this script reports the table as done. */ const pending = or( ...ACCOUNT_TOKEN_FIELDS.map((field) => and( sql`${account[field]} IS NOT NULL`, sql`${account[field]} <> ''`, - sql`${account[field]} NOT LIKE 'simenc:v%'` + sql`${account[field]} !~ '^simenc:v[0-9]+:'` ) ) ) const stats = { scanned: 0, updated: 0, raced: 0, failed: 0 } + let consecutiveFailures = 0 try { const [{ count: before }] = await db @@ -192,17 +196,22 @@ async function main(): Promise { if (applied.length === 0) stats.raced += 1 else stats.updated += 1 + consecutiveFailures = 0 } catch (error) { stats.failed += 1 + consecutiveFailures += 1 console.error(` row ${row.id}: ${getErrorMessage(error)}`) + /** A run of failures is an environment problem, not per-row corruption — stop early. */ + if (consecutiveFailures >= 25) { + throw new Error(`${consecutiveFailures} consecutive row failures; aborting the run`) + } } } console.log( ` scanned=${stats.scanned} ${apply ? 'updated' : 'would-encrypt'}=${stats.updated} raced=${stats.raced} failed=${stats.failed}` ) - if (!apply) break - if (sleepMs > 0) await sleep(sleepMs) + if (apply && sleepMs > 0) await sleep(sleepMs) } const [{ count: after }] = await db diff --git a/scripts/check-account-token-access.ts b/scripts/check-account-token-access.ts index 2615ea03d47..1b2a98d4f4b 100644 --- a/scripts/check-account-token-access.ts +++ b/scripts/check-account-token-access.ts @@ -32,15 +32,11 @@ const ROOT = resolve(SCRIPT_DIR, '..') const APP = resolve(ROOT, 'apps/sim') /** - * The modules permitted to touch the token columns. Each either implements the envelope or - * immediately decrypts what it selects. - * - * Keep this list short — its purpose is to stop a *new*, silent token read appearing anywhere - * in the other ~4,000 files. + * The only modules permitted to touch the token columns; both immediately decrypt what + * they select and encrypt what they write. Keep this list short — its purpose is to stop + * a *new*, silent token read appearing anywhere else. */ const TOKEN_AWARE_MODULES = new Set([ - 'apps/sim/lib/oauth/account-token-crypto.ts', - 'apps/sim/lib/oauth/account-tokens.ts', 'apps/sim/lib/oauth/credential-service.ts', 'apps/sim/lib/oauth/slack.ts', ]) From 0cf11a05b3a94cb2eb5649ff5d3341a849774c73 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 31 Aug 2026 18:03:42 -0700 Subject: [PATCH 5/5] test(security): pin the shrunken audit allowlist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The allowlist dropped the accessor and crypto modules — neither holds a query — but the audit's own test still asserted their exemption. It now asserts the inverse: only credential-service and slack are exempt, and the other two are audited like any file. This test runs from the root test chain, which is why the apps/sim suite stayed green while CI went red. --- scripts/check-account-token-access.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/scripts/check-account-token-access.test.ts b/scripts/check-account-token-access.test.ts index 63fbf4cdb4d..a13b55ef823 100644 --- a/scripts/check-account-token-access.test.ts +++ b/scripts/check-account-token-access.test.ts @@ -87,11 +87,13 @@ describe('auditSource', () => { ) }) - it('exempts the token-aware modules', () => { + it('exempts only the two modules that genuinely touch the table', () => { const source = 'await db.select().from(account)\nconst t = account.accessToken' - expect(auditSource('apps/sim/lib/oauth/account-tokens.ts', source)).toEqual([]) expect(auditSource('apps/sim/lib/oauth/credential-service.ts', source)).toEqual([]) expect(auditSource('apps/sim/lib/oauth/slack.ts', source)).toEqual([]) + /** The accessor and crypto modules hold no queries, so they are audited like any file. */ + expect(auditSource('apps/sim/lib/oauth/account-tokens.ts', source)).toHaveLength(2) + expect(auditSource('apps/sim/lib/oauth/account-token-crypto.ts', source)).toHaveLength(2) }) describe('annotation', () => {