diff --git a/.agents/skills/ship/SKILL.md b/.agents/skills/ship/SKILL.md index 54940fd9e58..8d30b1b63d2 100644 --- a/.agents/skills/ship/SKILL.md +++ b/.agents/skills/ship/SKILL.md @@ -100,6 +100,16 @@ improvement(scope): description for enhancements chore(scope): description for maintenance ``` +## What to Omit + +The repo is public. Keep the title and description to the code change and its reasoning — never: + +- Customer, company, or user names; workspace/user/org IDs; email addresses +- Prod or staging operational data: log lines, DB rows, metrics, timestamps, incident details, canary/alert output +- Infrastructure specifics: hostnames, ARNs, internal URLs, env var values, secret names + +Describe the bug by its mechanism, not by how you found it. "Expired OAuth credentials fail to refresh in the worker" — not "the Sheets canary failed at 16:31Z for workspace abc-123". + ## PR Description Format Use this exact template in the user's voice (concise, bullet points): diff --git a/.claude/commands/ship.md b/.claude/commands/ship.md index 5380cbd23f0..5eb95285a76 100644 --- a/.claude/commands/ship.md +++ b/.claude/commands/ship.md @@ -99,6 +99,16 @@ improvement(scope): description for enhancements chore(scope): description for maintenance ``` +## What to Omit + +The repo is public. Keep the title and description to the code change and its reasoning — never: + +- Customer, company, or user names; workspace/user/org IDs; email addresses +- Prod or staging operational data: log lines, DB rows, metrics, timestamps, incident details, canary/alert output +- Infrastructure specifics: hostnames, ARNs, internal URLs, env var values, secret names + +Describe the bug by its mechanism, not by how you found it. "Expired OAuth credentials fail to refresh in the worker" — not "the Sheets canary failed at 16:31Z for workspace abc-123". + ## PR Description Format Use this exact template in the user's voice (concise, bullet points): diff --git a/.claude/rules/sim-architecture.md b/.claude/rules/sim-architecture.md index 6886710a61c..a8b25498eae 100644 --- a/.claude/rules/sim-architecture.md +++ b/.claude/rules/sim-architecture.md @@ -57,6 +57,27 @@ Use the `migrate-application-operation` skill before creating or migrating a pro Every export of a `'use client'` module becomes a *client reference* on the server — server-evaluated code (RSC pages/layouts, `prefetch.ts`, route handlers, block definitions, triggers) can only *render* it as a component or pass it as a prop, never *call* it (doing so throws at runtime, e.g. `tableKeys.list is not a function`; `next build` does not catch it). Keep server-importable query primitives (key factories, fetchers, mappers, constants) in non-`'use client'` modules — see `.claude/rules/sim-queries.md`. Enforced by `scripts/check-client-boundary-imports.ts`. +## The app/worker runtime boundary + +Server code runs in two runtimes with **different environments**. The app container loads the +full env from `SIM_ENV_SECRET_ID` (Secrets Manager). Trigger.dev workers — which execute +workflows, so every block handler and every tool call — get their env from the Trigger.dev +dashboard, and `trigger.config.ts` syncs only `DB_APP_NAME`. The repo cannot see what the +dashboard holds. + +So before replacing a worker's HTTP call to our own API with an in-process call, ask what env +that work reads *on the app side*. Anything gated by a `require*Capability` helper is the sharp +case: those **throw** when the variable is absent (`requireOAuthClientCapability` → +`EnvCapabilityConfigurationError`), and the throw may be caught and reported as something +unrelated. OAuth token refresh is the known example — moving it into the worker turns every +expired credential into `Failed to refresh access token`, while a still-valid token hides the +bug entirely, so it surfaces hours later and only for whoever's token lapsed first. + +An in-process conversion is safe when the same work already runs in that runtime (the agent +block has always called `executeProviderRequest` in-process, so router and evaluator joining it +is proven), or when the caller and the callee are both the app (a route calling a lib module, an +RSC prefetch reading the data layer). It is not safe on reasoning alone. + ## Feature Organization Features live under `app/workspace/[workspaceId]/`: diff --git a/.cursor/commands/ship.md b/.cursor/commands/ship.md index 7421187736d..bb437b52ccc 100644 --- a/.cursor/commands/ship.md +++ b/.cursor/commands/ship.md @@ -94,6 +94,16 @@ improvement(scope): description for enhancements chore(scope): description for maintenance ``` +## What to Omit + +The repo is public. Keep the title and description to the code change and its reasoning — never: + +- Customer, company, or user names; workspace/user/org IDs; email addresses +- Prod or staging operational data: log lines, DB rows, metrics, timestamps, incident details, canary/alert output +- Infrastructure specifics: hostnames, ARNs, internal URLs, env var values, secret names + +Describe the bug by its mechanism, not by how you found it. "Expired OAuth credentials fail to refresh in the worker" — not "the Sheets canary failed at 16:31Z for workspace abc-123". + ## PR Description Format Use this exact template in the user's voice (concise, bullet points): diff --git a/apps/sim/tools/index.ts b/apps/sim/tools/index.ts index 7a708b9db0b..0dcebc1ea23 100644 --- a/apps/sim/tools/index.ts +++ b/apps/sim/tools/index.ts @@ -1732,77 +1732,68 @@ async function executeToolImplementation( const callerUserId = userId && contextParams._context?.enforceCredentialAccess ? userId : undefined - let data: CredentialTokenPayload + const baseUrl = getInternalApiBaseUrl() + logger.info(`[${requestId}] Fetching access token from ${baseUrl}/api/auth/oauth/token`) - if (typeof window === 'undefined') { - // Server-side runs resolve the credential through the same application - // operation the route calls, rather than minting an internal JWT and - // POSTing to ourselves through the load balancer. The synthesized - // `AuthResult` is exactly what verifying that self-issued token would - // have produced, so authorization, refresh, and audit are unchanged — - // including failing closed when the run carries no user id. - const { resolveCredentialToken } = await import('@/lib/oauth/token-resolution') - const result = await resolveCredentialToken( - { success: true, authType: 'internal_jwt', userId }, - { - requestId, - credentialId: contextParams.credential as string, - workflowId, - scopes: tokenPayload.scopes, - impersonateEmail: tokenPayload.impersonateEmail, - callerUserId, - } - ) + const tokenUrlObj = new URL('/api/auth/oauth/token', baseUrl) + if (workflowId) { + tokenUrlObj.searchParams.set('workflowId', workflowId) + } + if (callerUserId) { + tokenUrlObj.searchParams.set('userId', callerUserId) + } - if (!result.ok) { - logger.error(`[${requestId}] Token fetch failed for ${toolId}:`, { - status: result.status, - error: result.error, - }) - const toolLabel = tool?.name || toolId - throw new Error(`Failed to obtain credential for ${toolLabel}: ${result.error}`) + /** + * Deliberately an HTTP hop rather than an in-process call to + * `resolveCredentialToken`, even though both run the same authorization rule. + * + * An OAuth refresh needs the provider's client id and secret + * (`requireOAuthClientCapability`, which THROWS when they are absent). Only the + * app container loads those, from `SIM_ENV_SECRET_ID`. Tool calls execute inside + * the Trigger.dev worker, whose environment does not carry them, so resolving + * in-process there turns every credential whose access token has expired into + * `Failed to refresh access token`. A still-valid token hides it — the refresh + * path is only reached once the token lapses. + * + * Moving this in-process requires the worker to hold the OAuth client config, + * not just a code change. + */ + const tokenHeaders: Record = { 'Content-Type': 'application/json' } + if (typeof window === 'undefined') { + try { + const internalToken = await generateInternalToken(userId) + tokenHeaders.Authorization = `Bearer ${internalToken}` + } catch (_e) { + // Swallow token generation errors; the request will fail and be reported upstream } + } - data = result.token - } else { - const baseUrl = getInternalApiBaseUrl() - logger.info(`[${requestId}] Fetching access token from ${baseUrl}/api/auth/oauth/token`) - - const tokenUrlObj = new URL('/api/auth/oauth/token', baseUrl) - if (workflowId) { - tokenUrlObj.searchParams.set('workflowId', workflowId) - } - if (callerUserId) { - tokenUrlObj.searchParams.set('userId', callerUserId) - } + // boundary-raw-fetch: same-origin token route, authenticated by internal JWT on the server and the session cookie in the browser + const response = await fetch(tokenUrlObj.toString(), { + method: 'POST', + headers: tokenHeaders, + body: JSON.stringify(tokenPayload), + }) - // boundary-raw-fetch: browser-side tool runs authenticate with the session cookie against the same-origin token route - const response = await fetch(tokenUrlObj.toString(), { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(tokenPayload), + if (!response.ok) { + const errorText = await response.text() + logger.error(`[${requestId}] Token fetch failed for ${toolId}:`, { + status: response.status, + error: errorText, }) - - if (!response.ok) { - const errorText = await response.text() - logger.error(`[${requestId}] Token fetch failed for ${toolId}:`, { - status: response.status, - error: errorText, - }) - let parsedError = errorText - try { - const parsed = JSON.parse(errorText) - if (parsed.error) parsedError = parsed.error - } catch { - // Use raw text - } - const toolLabel = tool?.name || toolId - throw new Error(`Failed to obtain credential for ${toolLabel}: ${parsedError}`) + let parsedError = errorText + try { + const parsed = JSON.parse(errorText) + if (parsed.error) parsedError = parsed.error + } catch { + // Use raw text } - - data = (await response.json()) as CredentialTokenPayload + const toolLabel = tool?.name || toolId + throw new Error(`Failed to obtain credential for ${toolLabel}: ${parsedError}`) } + const data = (await response.json()) as CredentialTokenPayload + contextParams.accessToken = data.accessToken if (data.idToken) { contextParams.idToken = data.idToken