Skip to content

Commit 161b21f

Browse files
authored
feat(pi): require the user's own API key for Create PR even on hosted models (#6056)
1 parent 1a23438 commit 161b21f

6 files changed

Lines changed: 212 additions & 40 deletions

File tree

apps/sim/blocks/blocks/pi.ts

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@ import { getEnv, isTruthy } from '@/lib/core/config/env'
33
import type { BlockConfig } from '@/blocks/types'
44
import { AuthMode, IntegrationType } from '@/blocks/types'
55
import {
6+
getApiKeyCondition,
67
getPiModelOptions,
78
getProviderCredentialSubBlocks,
89
PROVIDER_CREDENTIAL_INPUTS,
910
} from '@/blocks/utils'
11+
import { isPiByokOnlyMode } from '@/providers/pi-providers'
1012
import type { ToolResponse } from '@/tools/types'
1113

1214
interface PiResponse extends ToolResponse {
@@ -110,6 +112,20 @@ function getSearchApiKeyCondition() {
110112
}
111113
}
112114

115+
const hostedModelApiKeyCondition = getApiKeyCondition()
116+
117+
/**
118+
* API Key visibility for the Pi block.
119+
*
120+
* Create PR hands the model key to the sandbox as an environment variable, so
121+
* Sim never supplies a hosted key there — the field is shown for every model,
122+
* including ones that are hosted elsewhere in Sim. Review Code and Local Dev
123+
* keep the model client inside Sim, so they follow the standard hosted-model
124+
* rule and hide the field when Sim covers the key.
125+
*/
126+
const piApiKeyCondition = (values?: Record<string, unknown>) =>
127+
isPiByokOnlyMode(values?.mode) ? CLOUD : hostedModelApiKeyCondition(values)
128+
113129
export const PiBlock: BlockConfig<PiResponse> = {
114130
type: 'pi',
115131
name: 'Pi Coding Agent',
@@ -122,7 +138,7 @@ export const PiBlock: BlockConfig<PiResponse> = {
122138
- Enable Babysit Mode on Create PR when trusted review bots and required checks should be monitored and fixed in bounded rounds.
123139
- Use Review Code to analyze an existing PR and leave summary + inline review comments.
124140
- Use Local Dev to edit a repo on your own machine; expose the machine on a public hostname/tunnel so Sim can reach it over SSH.
125-
- Create PR requires your own provider API key because the model runs in the sandbox. Review Code keeps the model key in Sim and can use either BYOK or a hosted key.
141+
- Create PR requires your own provider API key for every model, including ones Sim hosts, because the model runs in the sandbox. Review Code and Local Dev keep the model key in Sim and can use either BYOK or a hosted key.
126142
- Internet Search is off by default and always needs your own key for the selected provider, entered on the block. There is no workspace BYOK fallback and no hosted key. Leave it on None unless the task genuinely needs external information.
127143
`,
128144
category: 'blocks',
@@ -178,8 +194,15 @@ export const PiBlock: BlockConfig<PiResponse> = {
178194
options: getPiModelOptions,
179195
commandSearchable: true,
180196
},
181-
182-
...getProviderCredentialSubBlocks(),
197+
...getProviderCredentialSubBlocks().map((subBlock) =>
198+
subBlock.id === 'apiKey'
199+
? {
200+
...subBlock,
201+
placeholder: 'Enter your API key for the selected model',
202+
condition: piApiKeyCondition,
203+
}
204+
: subBlock
205+
),
183206

184207
{
185208
id: 'searchProvider',
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { resetEnvFlagsMock, setEnvFlags } from '@sim/testing'
5+
import { afterAll, afterEach, beforeEach, describe, expect, it } from 'vitest'
6+
import { evaluateSubBlockCondition } from '@/lib/workflows/subblocks/visibility'
7+
import { PiBlock } from '@/blocks/blocks/pi'
8+
import { getHostedModels } from '@/providers/utils'
9+
10+
const apiKeySubBlock = PiBlock.subBlocks.find((subBlock) => subBlock.id === 'apiKey')
11+
const hostedModel = getHostedModels()[0]
12+
13+
function isApiKeyVisible(values: Record<string, unknown>): boolean {
14+
return evaluateSubBlockCondition(apiKeySubBlock?.condition, values)
15+
}
16+
17+
describe('Pi API Key visibility', () => {
18+
beforeEach(() => {
19+
setEnvFlags({ isHosted: true })
20+
})
21+
22+
afterEach(() => {
23+
setEnvFlags({ isHosted: false })
24+
})
25+
26+
afterAll(resetEnvFlagsMock)
27+
28+
it('exposes an apiKey subblock that is required when visible', () => {
29+
expect(apiKeySubBlock).toBeDefined()
30+
expect(apiKeySubBlock?.required).toBe(true)
31+
})
32+
33+
// The bug this guards: the field used to hide for hosted models in every mode,
34+
// and Create PR then failed at execution demanding a BYOK key the user was
35+
// never asked for.
36+
it('shows the field in Create PR even for a model Sim hosts', () => {
37+
expect(hostedModel).toBeDefined()
38+
expect(isApiKeyVisible({ mode: 'cloud', model: hostedModel })).toBe(true)
39+
})
40+
41+
it('shows the field in Create PR for a model Sim does not host', () => {
42+
expect(isApiKeyVisible({ mode: 'cloud', model: 'some-unhosted-model' })).toBe(true)
43+
})
44+
45+
it.each([['local'], ['cloud_review']])(
46+
'hides the field in %s mode for a model Sim hosts',
47+
(mode) => {
48+
expect(isApiKeyVisible({ mode, model: hostedModel })).toBe(false)
49+
}
50+
)
51+
52+
it.each([['local'], ['cloud_review']])(
53+
'shows the field in %s mode for a model Sim does not host',
54+
(mode) => {
55+
expect(isApiKeyVisible({ mode, model: 'some-unhosted-model' })).toBe(true)
56+
}
57+
)
58+
})

apps/sim/executor/handlers/pi/keys.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import type { PiSupportedProvider } from '@/providers/pi-provider-configs'
1818
import {
1919
getPiProviderApiKeyEnvVar,
2020
getPiWorkspaceBYOKProviderId,
21+
isPiByokOnlyMode,
2122
isPiSupportedProvider,
2223
} from '@/providers/pi-providers'
2324

@@ -45,7 +46,7 @@ export async function resolvePiModelKey(params: ResolvePiModelKeyParams): Promis
4546
return { apiKey: params.apiKey, isBYOK: true }
4647
}
4748

48-
if (params.mode === 'cloud') {
49+
if (isPiByokOnlyMode(params.mode)) {
4950
const workspaceBYOKProviderId = getPiWorkspaceBYOKProviderId(providerId)
5051
if (params.workspaceId && workspaceBYOKProviderId) {
5152
const byok = await getBYOKKey(params.workspaceId, workspaceBYOKProviderId)

apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.test.ts

Lines changed: 98 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,14 @@ const {
1111
mockGetTool,
1212
mockGetCustomToolById,
1313
mockGetSkillById,
14+
mockGetHostedModels,
1415
} = vi.hoisted(() => ({
1516
mockValidateSelectorIds: vi.fn(),
1617
mockGetModelOptions: vi.fn(() => []),
1718
mockGetTool: vi.fn(),
1819
mockGetCustomToolById: vi.fn(),
1920
mockGetSkillById: vi.fn(),
21+
mockGetHostedModels: vi.fn(() => [] as string[]),
2022
}))
2123

2224
const conditionBlockConfig = {
@@ -55,6 +57,18 @@ const agentBlockConfig = {
5557
],
5658
}
5759

60+
const piBlockConfig = {
61+
type: 'pi',
62+
name: 'Pi Coding Agent',
63+
outputs: {},
64+
subBlocks: [
65+
{ id: 'mode', type: 'dropdown' },
66+
{ id: 'model', type: 'combobox', options: mockGetModelOptions },
67+
{ id: 'apiKey', type: 'short-input' },
68+
],
69+
tools: { access: [] },
70+
}
71+
5872
const huggingfaceBlockConfig = {
5973
type: 'huggingface',
6074
name: 'HuggingFace',
@@ -175,35 +189,25 @@ const toolsByIdMock: Record<string, unknown> = {
175189
},
176190
}
177191

192+
const blockConfigsByType: Record<string, unknown> = {
193+
condition: conditionBlockConfig,
194+
slack: oauthBlockConfig,
195+
router_v2: routerBlockConfig,
196+
agent: agentBlockConfig,
197+
pi: piBlockConfig,
198+
huggingface: huggingfaceBlockConfig,
199+
knowledge: knowledgeBlockConfig,
200+
canonicalcred: canonicalCredBlockConfig,
201+
video_generator_v3: videoBlockConfig,
202+
custom_key_block: customKeyBlockConfig,
203+
image_generator_v2: imageBlockConfig,
204+
throw_gate_block: throwGateBlockConfig,
205+
throw_selector_block: throwSelectorBlockConfig,
206+
generic_webhook: genericWebhookBlockConfig,
207+
}
208+
178209
vi.mock('@/blocks/registry', () => ({
179-
getBlock: (type: string) =>
180-
type === 'condition'
181-
? conditionBlockConfig
182-
: type === 'slack'
183-
? oauthBlockConfig
184-
: type === 'router_v2'
185-
? routerBlockConfig
186-
: type === 'agent'
187-
? agentBlockConfig
188-
: type === 'huggingface'
189-
? huggingfaceBlockConfig
190-
: type === 'knowledge'
191-
? knowledgeBlockConfig
192-
: type === 'canonicalcred'
193-
? canonicalCredBlockConfig
194-
: type === 'video_generator_v3'
195-
? videoBlockConfig
196-
: type === 'custom_key_block'
197-
? customKeyBlockConfig
198-
: type === 'image_generator_v2'
199-
? imageBlockConfig
200-
: type === 'throw_gate_block'
201-
? throwGateBlockConfig
202-
: type === 'throw_selector_block'
203-
? throwSelectorBlockConfig
204-
: type === 'generic_webhook'
205-
? genericWebhookBlockConfig
206-
: undefined,
210+
getBlock: (type: string) => blockConfigsByType[type],
207211
}))
208212

209213
vi.mock('@/blocks/utils', () => ({
@@ -227,7 +231,7 @@ vi.mock('@/lib/workflows/skills/operations', () => ({
227231
}))
228232

229233
vi.mock('@/providers/utils', () => ({
230-
getHostedModels: () => [],
234+
getHostedModels: mockGetHostedModels,
231235
}))
232236

233237
import {
@@ -238,6 +242,8 @@ import {
238242
validateWorkflowSelectorIds,
239243
} from './validation'
240244

245+
const CTX = { userId: 'user-1', workspaceId: 'workspace-1' }
246+
241247
afterAll(resetEnvFlagsMock)
242248

243249
describe('validateInputsForBlock', () => {
@@ -919,7 +925,69 @@ describe('preValidateCredentialInputs (hosted-tool blocks)', () => {
919925
})
920926
})
921927

922-
const CTX = { userId: 'user-1', workspaceId: 'workspace-1' }
928+
describe('preValidateCredentialInputs (hosted models)', () => {
929+
beforeEach(() => {
930+
vi.clearAllMocks()
931+
mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: [] })
932+
mockGetHostedModels.mockReturnValue(['claude-sonnet-4-6'])
933+
setEnvFlags({ isHosted: true })
934+
})
935+
936+
afterEach(() => {
937+
mockGetHostedModels.mockReset()
938+
setEnvFlags({ isHosted: false })
939+
})
940+
941+
const piAddOperation = (mode: string) => [
942+
{
943+
operation_type: 'add' as const,
944+
block_id: 'pi-1',
945+
params: {
946+
type: 'pi',
947+
inputs: { mode, model: 'claude-sonnet-4-6', apiKey: 'user-anthropic-key' },
948+
},
949+
},
950+
]
951+
952+
it('strips apiKey for a hosted model on a normal LLM block', async () => {
953+
const operations = [
954+
{
955+
operation_type: 'add' as const,
956+
block_id: 'agent-1',
957+
params: {
958+
type: 'agent',
959+
inputs: { model: 'claude-sonnet-4-6', apiKey: 'user-anthropic-key' },
960+
},
961+
},
962+
]
963+
964+
const result = await preValidateCredentialInputs(operations, CTX)
965+
966+
expect(result.filteredOperations[0]?.params?.inputs?.apiKey).toBeUndefined()
967+
expect(result.errors).toHaveLength(1)
968+
expect(result.errors[0]?.error).toContain('hosted model')
969+
})
970+
971+
// Create PR hands the key to the sandbox, so Sim never covers it with a hosted
972+
// key -- stripping it would leave the copilot authoring a block that cannot run.
973+
it('preserves apiKey on a Create PR Pi block when the model is hosted', async () => {
974+
const result = await preValidateCredentialInputs(piAddOperation('cloud'), CTX)
975+
976+
expect(result.filteredOperations[0]?.params?.inputs?.apiKey).toBe('user-anthropic-key')
977+
expect(result.errors).toHaveLength(0)
978+
})
979+
980+
// Local Dev and Review Code keep the model client in Sim, so the hosted key applies.
981+
it.each([['local'], ['cloud_review']])(
982+
'strips apiKey on a Pi block in %s mode when the model is hosted',
983+
async (mode) => {
984+
const result = await preValidateCredentialInputs(piAddOperation(mode), CTX)
985+
986+
expect(result.filteredOperations[0]?.params?.inputs?.apiKey).toBeUndefined()
987+
expect(result.errors).toHaveLength(1)
988+
}
989+
)
990+
})
923991

924992
describe('validateWorkflowSelectorIds (credential inclusion)', () => {
925993
beforeEach(() => {

apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.ts

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,9 @@ import {
1515
import { getBlock } from '@/blocks/registry'
1616
import type { SubBlockConfig } from '@/blocks/types'
1717
import { getModelOptions } from '@/blocks/utils'
18-
import { EDGE, normalizeName } from '@/executor/constants'
18+
import { BlockType, EDGE, normalizeName } from '@/executor/constants'
1919
import { isKnownModelId, suggestModelIdsForUnknownModel } from '@/providers/models'
20+
import { isPiByokOnlyMode } from '@/providers/pi-providers'
2021
import { getTool } from '@/tools/utils'
2122
import { TRIGGER_RUNTIME_SUBBLOCK_IDS, TRIGGER_WEBHOOK_URL_FIELD } from '@/triggers/constants'
2223
import type {
@@ -1248,7 +1249,8 @@ export async function collectUnresolvedAgentToolReferences(
12481249
* - Filters out apiKey inputs when isHosted is true and the key is platform-managed: either a
12491250
* hosted LLM model (model in getHostedModels) or a block whose active tool declares
12501251
* `hosting` (e.g. Fal-backed video/image generators) - the canonical signal also used by
1251-
* injectHostedKeyIfNeeded at execution
1252+
* injectHostedKeyIfNeeded at execution. The Pi Coding Agent block in Create PR mode is
1253+
* exempt from the hosted-model rule because it always runs on the user's own key
12521254
* - Also validates credentials and apiKeys in nestedNodes (blocks inside loop/parallel)
12531255
* Returns validation errors for any removed inputs.
12541256
*/
@@ -1316,17 +1318,24 @@ export async function preValidateCredentialInputs(
13161318
}
13171319

13181320
/**
1319-
* Check if apiKey should be filtered for a block with the given model
1321+
* Check if apiKey should be filtered for a block with the given model.
1322+
*
1323+
* The Pi Coding Agent in Create PR mode is exempt: it hands the model key to
1324+
* a sandbox, so Sim never covers it with a hosted key and the block needs the
1325+
* user's own key even for hosted models. Its other modes keep the model
1326+
* client in Sim and follow the normal rule.
13201327
*/
13211328
function collectHostedApiKeyInput(
13221329
inputs: Record<string, unknown>,
1323-
modelValue: string | undefined,
1330+
toolParams: Record<string, unknown>,
13241331
opIndex: number,
13251332
blockId: string,
13261333
blockType: string,
13271334
nestedBlockId?: string
13281335
) {
13291336
if (!hostedModelsLower || !inputs.apiKey) return
1337+
if (blockType === BlockType.PI && isPiByokOnlyMode(toolParams.mode)) return
1338+
const modelValue = toolParams.model
13301339
if (!modelValue || typeof modelValue !== 'string') return
13311340

13321341
if (hostedModelsLower.has(modelValue.toLowerCase())) {
@@ -1525,10 +1534,9 @@ export async function preValidateCredentialInputs(
15251534
// Hosted collectors no-op off hosted Sim, so only resolve the effective state when it matters.
15261535
if (isHosted) {
15271536
const toolParams = finalValues.get(stateKey) ?? inputs
1528-
const modelValue = toolParams.model as string | undefined
15291537
collectHostedApiKeyInput(
15301538
inputs,
1531-
modelValue,
1539+
toolParams,
15321540
opIndex,
15331541
reportBlockId,
15341542
blockType,

apps/sim/providers/pi-providers.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,20 @@ export function isPiSupportedProvider(providerId: string): providerId is PiSuppo
2727
return PI_PROVIDER_CONFIG_BY_ID.has(providerId)
2828
}
2929

30+
/**
31+
* Whether a Pi block mode hands the model API key into the sandbox and
32+
* therefore always requires the user's own key. Create PR ('cloud') runs the
33+
* model client inside the sandbox, so Sim never supplies a hosted key for it:
34+
* the block always shows the API Key field, copilot validation never strips
35+
* it, and execution requires BYOK. Review Code and Local Dev keep the model
36+
* client in Sim and follow the normal hosted-key rules. All three enforcement
37+
* sites (block condition, edit-workflow validation, key resolution) consume
38+
* this predicate so they cannot drift.
39+
*/
40+
export function isPiByokOnlyMode(mode: unknown): boolean {
41+
return mode === 'cloud'
42+
}
43+
3044
/** Returns Pi's provider ID for a supported Sim provider. */
3145
export function getPiProviderId(providerId: PiSupportedProvider): PiProviderConfig['piProviderId'] {
3246
const config = PI_PROVIDER_CONFIG_BY_ID.get(providerId)

0 commit comments

Comments
 (0)