Skip to content

Commit ff61e23

Browse files
committed
fix(search): align review limits and isolate OAuth token refresh
1 parent 5fc21fc commit ff61e23

10 files changed

Lines changed: 114 additions & 20 deletions

File tree

‎apps/desktop/e2e/source-connect.spec.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ test('source authorization returns to its desktop screen and refreshes live', as
9494
json({
9595
credentialGroup: null,
9696
availableProviders: [],
97+
assistantProviders: [],
9798
availableMcpConnectors: [],
9899
canManage: false,
99100
indexingAvailable: true,
2.92 KB
Loading

‎apps/sim/app/api/organizations/[id]/chats/[chatId]/actions/route.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,10 @@ export const GET = defineInternalJsonRoute({
1212
contract: listAssistantActionsContract,
1313
auth: internalSessionAuth,
1414
operation: assistantActionOperations.list,
15-
rateLimit: internalRateLimits.user({ bucketName: 'assistant-actions-review' }),
15+
rateLimit: internalRateLimits.user({
16+
bucketName: 'assistant-actions-review',
17+
config: { maxTokens: 120, refillRate: 120, refillIntervalMs: 60_000 },
18+
}),
1619
errorPolicy: internalOrchestrationErrorPolicy,
1720
mapInput: ({ params, query }) => ({
1821
organizationId: params.id,

‎apps/sim/components/assistant-actions/review.tsx‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ import { Hand } from '@sim/emcn/icons'
1515
import { getErrorMessage } from '@sim/utils/errors'
1616
import { omit } from '@sim/utils/object'
1717
import type { AssistantActionsResponse } from '@/lib/api/contracts/assistant-actions'
18+
import {
19+
ASSISTANT_ACTION_MAX_BYTES,
20+
ASSISTANT_REVIEW_FIELD_MAX_CHARS,
21+
} from '@/lib/assistant-actions/limits'
1822
import {
1923
useAssistantActionReview,
2024
useDecideAssistantAction,
@@ -116,7 +120,8 @@ export function AssistantActionReviewProvider({
116120
</ChipModalHeader>
117121
<ChipModalBody>
118122
<p className='px-2 text-[var(--text-muted)] text-small'>
119-
Review each action before it runs. Allow and Cancel apply only to these proposals.
123+
Review each action before it runs. Allow and Cancel apply only to these proposals. Each
124+
action has a {ASSISTANT_ACTION_MAX_BYTES / 1024} KB total limit across all fields.
120125
</p>
121126
{actions.map((action) => (
122127
<ActionReview
@@ -208,7 +213,7 @@ function ActionReview({ action, edits, disabled, onEdit, onDecide }: ActionRevie
208213
title={field.label}
209214
value={edits[field.name] ?? field.value}
210215
disabled={disabled}
211-
maxLength={65_536}
216+
maxLength={ASSISTANT_REVIEW_FIELD_MAX_CHARS}
212217
onChange={(value) => onEdit(field.name, value)}
213218
rows={field.format === 'multiline' ? 4 : 2}
214219
/>

‎apps/sim/hooks/queries/assistant-actions.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ const assistantActionKeys = {
1919
}
2020

2121
const ASSISTANT_PERMISSIONS_STALE_TIME = 10_000
22-
const ASSISTANT_REVIEW_STALE_TIME = 1_000
22+
const ASSISTANT_REVIEW_STALE_TIME = 3_000
23+
const ASSISTANT_REVIEW_POLL_INTERVAL = 3_000
2324

2425
export function useAssistantToolPermissions(organizationId: string, credentialId: string) {
2526
return useQuery({
@@ -51,7 +52,7 @@ export function useAssistantActionReview(
5152
enabled: enabled && Boolean(organizationId && chatId),
5253
staleTime: ASSISTANT_REVIEW_STALE_TIME,
5354
refetchInterval: (query) =>
54-
active || query.state.data?.actions.length ? ASSISTANT_REVIEW_STALE_TIME : false,
55+
active || query.state.data?.actions.length ? ASSISTANT_REVIEW_POLL_INTERVAL : false,
5556
})
5657
}
5758

‎apps/sim/lib/api/contracts/assistant-actions.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { z } from 'zod'
22
import { organizationIdSchema } from '@/lib/api/contracts/primitives'
33
import { defineRouteContract } from '@/lib/api/contracts/types'
4+
import { ASSISTANT_REVIEW_FIELD_MAX_CHARS } from '@/lib/assistant-actions/limits'
45

56
const policySchema = z.enum(['allow', 'ask', 'block'])
67
const toolIdSchema = z.string().min(1).max(256)
@@ -113,7 +114,7 @@ export const listAssistantActionsContract = defineRouteContract({
113114
target: z.boolean(),
114115
editable: z.boolean(),
115116
format: z.enum(['text', 'multiline', 'json']),
116-
value: z.string().max(65_536),
117+
value: z.string().max(ASSISTANT_REVIEW_FIELD_MAX_CHARS),
117118
})
118119
)
119120
.max(256),
@@ -131,7 +132,7 @@ const decisionBodySchema = z
131132
proposalVersion: z.string().regex(/^[a-f0-9]{64}$/),
132133
decision: z.enum(['allow', 'skip']),
133134
edits: z
134-
.record(z.string().min(1).max(256), z.string().max(65_536))
135+
.record(z.string().min(1).max(256), z.string().max(ASSISTANT_REVIEW_FIELD_MAX_CHARS))
135136
.refine((value) => Object.keys(value).length <= 32, 'Too many edited fields')
136137
.optional(),
137138
})

‎apps/sim/lib/assistant-actions/binding.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { AssistantActionBinding } from '@sim/db/schema'
2+
import { ASSISTANT_ACTION_MAX_BYTES } from '@/lib/assistant-actions/limits'
23
import { OrchestrationError } from '@/lib/core/orchestration/types'
34
import { assertAssistantIntegrationCall } from '@/lib/mothership/assistant/tool-policy'
45
import { getToolMetadata } from '@/tools/metadata'
@@ -8,10 +9,10 @@ export function resolveAssistantActionBinding(
89
toolId: string,
910
args: Record<string, unknown>
1011
): AssistantActionBinding {
11-
if (Buffer.byteLength(JSON.stringify(args), 'utf8') > 65_536)
12+
if (Buffer.byteLength(JSON.stringify(args), 'utf8') > ASSISTANT_ACTION_MAX_BYTES)
1213
throw new OrchestrationError(
1314
'payload_too_large',
14-
'This action is too large to review. Ask Assistant to split it into smaller actions.'
15+
`This action exceeds the ${ASSISTANT_ACTION_MAX_BYTES / 1024} KB total review limit. Shorten the text or ask Assistant to split it into smaller actions.`
1516
)
1617
const { credentialId, operation } = assertAssistantIntegrationCall(getToolMetadata(toolId), args)
1718
return { credentialId, operationVersion: operation.version, effect: operation.effect }
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
export const ASSISTANT_ACTION_MAX_BYTES = 64 * 1024
2+
export const ASSISTANT_REVIEW_FIELD_MAX_CHARS = 64 * 1024

‎apps/sim/lib/assistant-actions/tool-permissions.integration.ts‎

Lines changed: 76 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,11 @@ import {
4545
import * as providerTransport from '@/lib/core/security/input-validation.server'
4646
import { credentialGroupScopePolicyVersion } from '@/lib/credential-groups/provider-adapter'
4747
import { getCredentialGroupProviderAdapter } from '@/lib/credential-groups/provider-registry'
48-
import { encryptManagedOAuthTokenSet } from '@/lib/credentials/managed-oauth'
48+
import {
49+
decryptManagedOAuthTokenSet,
50+
encryptManagedOAuthTokenSet,
51+
rejectManagedOAuthToken,
52+
} from '@/lib/credentials/managed-oauth'
4953
import {
5054
claimToolExecution,
5155
recordToolPermissionDecision,
@@ -1113,7 +1117,7 @@ describe('Assistant Google Docs delivery', () => {
11131117
})
11141118

11151119
describe('Assistant writes cannot escape their admitted invocation', () => {
1116-
it.each(['missing', 'changed', 'consumed'] as const)(
1120+
it.each(['missing', 'workspace', 'unscoped', 'changed', 'consumed'] as const)(
11171121
'refuses %s execution authority before reaching the provider',
11181122
async (attempt) => {
11191123
const provider = vi.spyOn(globalThis, 'fetch').mockImplementation(async () => {
@@ -1130,7 +1134,11 @@ describe('Assistant writes cannot escape their admitted invocation', () => {
11301134
const context = {
11311135
userId: ownerId,
11321136
workflowId: '',
1133-
organizationId,
1137+
...(attempt === 'workspace'
1138+
? { workspaceId: generateId() }
1139+
: attempt === 'unscoped'
1140+
? {}
1141+
: { organizationId }),
11341142
chatId,
11351143
toolCallId,
11361144
requestMode: 'assistant' as const,
@@ -1143,7 +1151,7 @@ describe('Assistant writes cannot escape their admitted invocation', () => {
11431151
{ operationContext: context }
11441152
)
11451153
try {
1146-
if (attempt === 'missing') {
1154+
if (attempt === 'missing' || attempt === 'workspace' || attempt === 'unscoped') {
11471155
expect(await execute()).toMatchObject({
11481156
success: false,
11491157
error: expect.stringContaining('approved, durable'),
@@ -1317,6 +1325,70 @@ describe('Assistant Jira sprint delivery', () => {
13171325
})
13181326

13191327
describe('Assistant Salesforce reviewed updates', () => {
1328+
it('preserves a refreshed grant during rejection without resolving its API destination', async () => {
1329+
const [saved] = await db
1330+
.select({
1331+
encryptedOauthTokenSet: credential.encryptedOauthTokenSet,
1332+
accessTokenExpiresAt: credential.accessTokenExpiresAt,
1333+
refreshTokenExpiresAt: credential.refreshTokenExpiresAt,
1334+
lastRefreshedAt: credential.lastRefreshedAt,
1335+
managedOauthStatus: credential.managedOauthStatus,
1336+
updatedAt: credential.updatedAt,
1337+
})
1338+
.from(credential)
1339+
.where(eq(credential.id, salesforceCredentialId))
1340+
if (!saved?.encryptedOauthTokenSet) throw new Error('Expected a saved Salesforce grant')
1341+
const policy = await getCredentialGroupProviderAdapter('salesforce').getPolicy(undefined, {})
1342+
const requests: string[] = []
1343+
const originalFetch = globalThis.fetch
1344+
const provider = vi.spyOn(globalThis, 'fetch').mockImplementation(async (input, init) => {
1345+
const request = new Request(input, init)
1346+
const url = new URL(request.url)
1347+
if (url.hostname !== 'login.salesforce.com') return originalFetch(input, init)
1348+
requests.push(url.pathname)
1349+
if (url.pathname === '/services/oauth2/token') {
1350+
expect(request.method).toBe('POST')
1351+
expect(new URLSearchParams(await request.text()).get('refresh_token')).toBe(
1352+
'assistant-salesforce-refresh'
1353+
)
1354+
return Response.json({
1355+
access_token: 'refreshed-salesforce-token',
1356+
refresh_token: 'rotated-salesforce-refresh',
1357+
expires_in: 3600,
1358+
token_type: 'Bearer',
1359+
})
1360+
}
1361+
return new Response(null, { status: 503 })
1362+
})
1363+
try {
1364+
await expect(
1365+
rejectManagedOAuthToken({
1366+
organizationId,
1367+
credentialId: salesforceCredentialId,
1368+
expectedProviderId: 'salesforce',
1369+
requiredScopes: policy.requiredScopes,
1370+
rejectedAccessToken: 'assistant-salesforce-token',
1371+
})
1372+
).resolves.toBe(false)
1373+
expect(requests).toEqual(['/services/oauth2/token'])
1374+
const [current] = await db
1375+
.select()
1376+
.from(credential)
1377+
.where(eq(credential.id, salesforceCredentialId))
1378+
if (!current?.encryptedOauthTokenSet) throw new Error('Expected the refreshed grant')
1379+
expect(current.managedOauthStatus).toBe('active')
1380+
expect(current.lastRefreshedAt).toBeInstanceOf(Date)
1381+
expect(current.accessTokenExpiresAt?.getTime()).toBeGreaterThan(Date.now())
1382+
expect(await decryptManagedOAuthTokenSet(current.encryptedOauthTokenSet)).toMatchObject({
1383+
accessToken: 'refreshed-salesforce-token',
1384+
refreshToken: 'rotated-salesforce-refresh',
1385+
})
1386+
} finally {
1387+
provider.mockRestore()
1388+
await db.update(credential).set(saved).where(eq(credential.id, salesforceCredentialId))
1389+
}
1390+
})
1391+
13201392
it.each(['clear', 'blank_status', 'blank_priority'] as const)(
13211393
'honors reviewed edits with a real-shaped managed grant: %s',
13221394
async (scenario) => {

‎apps/sim/lib/credentials/managed-oauth.ts‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,7 @@ export async function rejectManagedOAuthToken(input: {
349349
if (updated.length === 0) return false
350350
if (!tokenSet.refreshToken) return true
351351
try {
352-
await resolveManagedOAuthToken({
352+
await resolveManagedOAuthAccessToken({
353353
credentialId: input.credentialId,
354354
workspaceId: input.workspaceId,
355355
organizationId: input.organizationId,
@@ -374,10 +374,10 @@ export async function rejectManagedOAuthToken(input: {
374374
}
375375

376376
async function resolveProviderTokenContext(
377-
providerId: string | null,
377+
providerId: string,
378378
token: ResolvedManagedOAuthToken
379379
): Promise<ResolvedManagedOAuthToken> {
380-
if (!providerId || !isSalesforceOAuthProviderId(providerId)) return token
380+
if (!isSalesforceOAuthProviderId(providerId)) return token
381381
const instanceUrl = await fetchSalesforceInstanceUrl(providerId, token.accessToken)
382382
if (!instanceUrl)
383383
throw new ManagedOAuthCredentialError(
@@ -391,6 +391,15 @@ async function resolveProviderTokenContext(
391391
/** Resolves a managed credential ID into a usable token without exposing it to list APIs. */
392392
export async function resolveManagedOAuthToken(
393393
params: ResolveManagedOAuthTokenParams
394+
): Promise<ResolvedManagedOAuthToken> {
395+
return resolveProviderTokenContext(
396+
params.expectedProviderId,
397+
await resolveManagedOAuthAccessToken(params)
398+
)
399+
}
400+
401+
async function resolveManagedOAuthAccessToken(
402+
params: ResolveManagedOAuthTokenParams
394403
): Promise<ResolvedManagedOAuthToken> {
395404
const initial = await getManagedCredential(db, params.credentialId, params)
396405
if (!initial) {
@@ -423,11 +432,11 @@ export async function resolveManagedOAuthToken(
423432
const initialTokenSet = await decryptManagedOAuthTokenSet(initial.encryptedOauthTokenSet!)
424433
const now = new Date()
425434
if (hasFreshAccessToken(initial.accessTokenExpiresAt, now)) {
426-
return resolveProviderTokenContext(initial.providerId, {
435+
return {
427436
accessToken: initialTokenSet.accessToken,
428437
...(initialTokenSet.idToken ? { idToken: initialTokenSet.idToken } : {}),
429438
refreshed: false,
430-
})
439+
}
431440
}
432441

433442
const refreshOutcome = await db.transaction(async (tx) => {
@@ -549,8 +558,7 @@ export async function resolveManagedOAuthToken(
549558
})
550559

551560
if ('error' in refreshOutcome && refreshOutcome.error) throw refreshOutcome.error
552-
if ('token' in refreshOutcome && refreshOutcome.token)
553-
return resolveProviderTokenContext(initial.providerId, refreshOutcome.token)
561+
if ('token' in refreshOutcome && refreshOutcome.token) return refreshOutcome.token
554562
throw new ManagedOAuthCredentialError(
555563
'MANAGED_CREDENTIAL_REFRESH_FAILED',
556564
'Managed credential refresh returned no token',

0 commit comments

Comments
 (0)