mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
feat(secrets): let mship add secret descriptions (#6814)
* feat(copilot): support workspace secret descriptions * feat(copilot): save secret card descriptions * test(copilot): cover secret card descriptions * fix(copilot): update secret descriptions without values
This commit is contained in:
+83
@@ -14,9 +14,11 @@ const {
|
||||
mockSendBrowserPanelAction,
|
||||
mockUpsertWorkspaceEnvironment,
|
||||
mockUseUserPermissionsContext,
|
||||
mockUpdateWorkspaceCredential,
|
||||
mockUseWorkspaceCredential,
|
||||
mockUseWorkspaceCredentials,
|
||||
} = vi.hoisted(() => ({
|
||||
mockUpdateWorkspaceCredential: vi.fn(async () => undefined),
|
||||
mockRefetchPersonalEnvironment: vi.fn(async () => ({ data: {} })),
|
||||
mockRefetchWorkspaceCredentials: vi.fn(async () => ({ data: [] })),
|
||||
mockIsBrowserAgentAvailable: vi.fn(() => false),
|
||||
@@ -37,6 +39,7 @@ vi.mock('next/navigation', () => ({
|
||||
}))
|
||||
|
||||
vi.mock('@/hooks/queries/credentials', () => ({
|
||||
useUpdateWorkspaceCredential: () => ({ mutateAsync: mockUpdateWorkspaceCredential }),
|
||||
useWorkspaceCredential: mockUseWorkspaceCredential,
|
||||
useWorkspaceCredentials: mockUseWorkspaceCredentials,
|
||||
}))
|
||||
@@ -1144,6 +1147,86 @@ describe('CredentialDisplay link tag', () => {
|
||||
act(() => root.unmount())
|
||||
})
|
||||
|
||||
it('attaches an agent-authored description after the secret value is saved', async () => {
|
||||
mockUseUserPermissionsContext.mockReturnValue({ canEdit: true })
|
||||
mockRefetchWorkspaceCredentials.mockResolvedValueOnce({
|
||||
data: [{ id: 'cred-1', envKey: 'WORKSPACE_KEY' }],
|
||||
})
|
||||
const container = document.createElement('div')
|
||||
const root = createRoot(container)
|
||||
const data: CredentialItemData[] = [
|
||||
{
|
||||
type: 'secret_input',
|
||||
name: 'WORKSPACE_KEY',
|
||||
scope: 'workspace',
|
||||
description: ' Stripe live key for billing ',
|
||||
},
|
||||
]
|
||||
|
||||
act(() => {
|
||||
root.render(<SpecialTags segment={{ type: 'credential', data }} onOptionSelect={vi.fn()} />)
|
||||
})
|
||||
|
||||
const input = container.querySelector('input')
|
||||
act(() => {
|
||||
if (!input) return
|
||||
const valueSetter = Object.getOwnPropertyDescriptor(
|
||||
window.HTMLInputElement.prototype,
|
||||
'value'
|
||||
)?.set
|
||||
valueSetter?.call(input, 'sk-live-123')
|
||||
input.dispatchEvent(new Event('input', { bubbles: true }))
|
||||
})
|
||||
const submitButton = Array.from(container.querySelectorAll('button')).find(
|
||||
(button) => button.textContent === 'Submit'
|
||||
)
|
||||
await act(async () => submitButton?.click())
|
||||
|
||||
expect(mockUpsertWorkspaceEnvironment).toHaveBeenCalledWith({
|
||||
workspaceId: 'workspace-1',
|
||||
variables: { WORKSPACE_KEY: 'sk-live-123' },
|
||||
})
|
||||
// The description rides the credential row the value save just created, so
|
||||
// the trimmed note lands without the user ever seeing the field.
|
||||
expect(mockUpdateWorkspaceCredential).toHaveBeenCalledWith({
|
||||
credentialId: 'cred-1',
|
||||
description: 'Stripe live key for billing',
|
||||
})
|
||||
act(() => root.unmount())
|
||||
})
|
||||
|
||||
it('leaves a personal secret undescribed', async () => {
|
||||
mockUseUserPermissionsContext.mockReturnValue({ canEdit: true })
|
||||
const container = document.createElement('div')
|
||||
const root = createRoot(container)
|
||||
const data: CredentialItemData[] = [
|
||||
{ type: 'secret_input', name: 'PERSONAL_KEY', scope: 'personal', description: 'my key' },
|
||||
]
|
||||
|
||||
act(() => {
|
||||
root.render(<SpecialTags segment={{ type: 'credential', data }} onOptionSelect={vi.fn()} />)
|
||||
})
|
||||
|
||||
const input = container.querySelector('input')
|
||||
act(() => {
|
||||
if (!input) return
|
||||
const valueSetter = Object.getOwnPropertyDescriptor(
|
||||
window.HTMLInputElement.prototype,
|
||||
'value'
|
||||
)?.set
|
||||
valueSetter?.call(input, 'personal-secret')
|
||||
input.dispatchEvent(new Event('input', { bubbles: true }))
|
||||
})
|
||||
const submitButton = Array.from(container.querySelectorAll('button')).find(
|
||||
(button) => button.textContent === 'Submit'
|
||||
)
|
||||
await act(async () => submitButton?.click())
|
||||
|
||||
expect(mockSavePersonalEnvironment).toHaveBeenCalled()
|
||||
expect(mockUpdateWorkspaceCredential).not.toHaveBeenCalled()
|
||||
act(() => root.unmount())
|
||||
})
|
||||
|
||||
it('renders one status recap from a transcript submission', () => {
|
||||
const container = document.createElement('div')
|
||||
const root: Root = createRoot(container)
|
||||
|
||||
+74
-2
@@ -1,6 +1,6 @@
|
||||
'use client'
|
||||
|
||||
import { createElement, lazy, Suspense, useEffect, useMemo, useState } from 'react'
|
||||
import { createElement, lazy, Suspense, useCallback, useEffect, useMemo, useState } from 'react'
|
||||
import {
|
||||
ArrowRight,
|
||||
Check,
|
||||
@@ -61,7 +61,11 @@ import type {
|
||||
import { useServiceAccountConnectTarget } from '@/app/workspace/[workspaceId]/integrations/components/connect-service-account-modal/use-service-account-connect'
|
||||
import { useWorkspaceHostContext } from '@/app/workspace/[workspaceId]/providers/workspace-host-provider'
|
||||
import { useUserPermissionsContext } from '@/app/workspace/[workspaceId]/providers/workspace-permissions-provider'
|
||||
import { useWorkspaceCredential } from '@/hooks/queries/credentials'
|
||||
import {
|
||||
useUpdateWorkspaceCredential,
|
||||
useWorkspaceCredential,
|
||||
useWorkspaceCredentials,
|
||||
} from '@/hooks/queries/credentials'
|
||||
import {
|
||||
usePersonalEnvironment,
|
||||
useSavePersonalEnvironment,
|
||||
@@ -137,6 +141,12 @@ export interface CredentialItemData {
|
||||
name?: string
|
||||
/** Where a secret_input value is persisted. Defaults to "workspace". */
|
||||
scope?: SecretInputScope
|
||||
/**
|
||||
* What the secret is for (secret_input, workspace scope only), written by the
|
||||
* agent that asked for it. Never shown or editable in the card — it exists so
|
||||
* the saved secret carries its purpose into workspace settings.
|
||||
*/
|
||||
description?: string
|
||||
/**
|
||||
* Existing credential to reconnect in place (service_account only). Present =
|
||||
* rotate the secret on this credential; absent = create a new one.
|
||||
@@ -1751,6 +1761,63 @@ interface CredentialControlProps {
|
||||
onConnected?: () => void
|
||||
}
|
||||
|
||||
/**
|
||||
* Attaches the agent-authored descriptions to workspace secrets once their values
|
||||
* are saved, reusing the credential update endpoint the secrets settings page
|
||||
* calls. It runs after the value write because that write is what mints the
|
||||
* credential row a description hangs on, and it is best-effort: the value is the
|
||||
* point of the card, so a failed note never fails the save. Personal rows are
|
||||
* skipped — their credential rows are per-workspace mirrors of one user-global
|
||||
* secret, so no single row can own a description.
|
||||
*/
|
||||
function useWorkspaceSecretDescriptions(items: CredentialItemData[]) {
|
||||
const { workspaceId } = useParams<{ workspaceId: string }>()
|
||||
const describedByName = useMemo(() => {
|
||||
const entries = new Map<string, string>()
|
||||
for (const item of items) {
|
||||
if (item.type !== 'secret_input' || item.scope === 'personal') continue
|
||||
const name = item.name?.trim()
|
||||
const description = item.description?.trim()
|
||||
if (name && description) entries.set(name, description)
|
||||
}
|
||||
return entries
|
||||
}, [items])
|
||||
|
||||
const credentialsQuery = useWorkspaceCredentials({
|
||||
workspaceId,
|
||||
type: 'env_workspace',
|
||||
enabled: describedByName.size > 0,
|
||||
})
|
||||
const updateCredential = useUpdateWorkspaceCredential()
|
||||
const refetchCredentials = credentialsQuery.refetch
|
||||
|
||||
return useCallback(
|
||||
async (savedNames: string[]) => {
|
||||
const pending = savedNames.filter((name) => describedByName.has(name))
|
||||
if (pending.length === 0) return
|
||||
|
||||
try {
|
||||
const { data } = await refetchCredentials()
|
||||
const idByEnvKey = new Map((data ?? []).map((row) => [row.envKey, row.id]))
|
||||
await Promise.all(
|
||||
pending.map(async (name) => {
|
||||
const credentialId = idByEnvKey.get(name)
|
||||
if (!credentialId) return
|
||||
await updateCredential.mutateAsync({
|
||||
credentialId,
|
||||
description: describedByName.get(name),
|
||||
})
|
||||
})
|
||||
)
|
||||
} catch {
|
||||
// Swallowed deliberately: the secret is stored, and the card must not
|
||||
// report failure over a missing note.
|
||||
}
|
||||
},
|
||||
[describedByName, refetchCredentials, updateCredential.mutateAsync]
|
||||
)
|
||||
}
|
||||
|
||||
function SecretInputDisplay({ data, divided = false, onSaved }: CredentialControlProps) {
|
||||
const { workspaceId } = useParams<{ workspaceId: string }>()
|
||||
const secretName = (data.name ?? '').trim()
|
||||
@@ -1765,6 +1832,7 @@ function SecretInputDisplay({ data, divided = false, onSaved }: CredentialContro
|
||||
const personalQuery = usePersonalEnvironment()
|
||||
const personalEnv = personalQuery.data
|
||||
const { canEdit } = useUserPermissionsContext()
|
||||
const attachDescriptions = useWorkspaceSecretDescriptions(useMemo(() => [data], [data]))
|
||||
|
||||
// Setting a workspace var needs write/admin (same gate as the secrets manager);
|
||||
// personal vars are the user's own, so any member may set them.
|
||||
@@ -1790,6 +1858,7 @@ function SecretInputDisplay({ data, divided = false, onSaved }: CredentialContro
|
||||
await savePersonal.mutateAsync({ variables: merged })
|
||||
} else {
|
||||
await upsertWorkspace.mutateAsync({ workspaceId, variables: { [secretName]: value } })
|
||||
await attachDescriptions([secretName])
|
||||
}
|
||||
setValue('')
|
||||
setSaved(true)
|
||||
@@ -2361,6 +2430,7 @@ function CredentialInputCard({
|
||||
const upsertWorkspace = useUpsertWorkspaceEnvironment()
|
||||
const savePersonal = useSavePersonalEnvironment()
|
||||
const personalQuery = usePersonalEnvironment()
|
||||
const attachDescriptions = useWorkspaceSecretDescriptions(data)
|
||||
const [secretDrafts, setSecretDrafts] = useState<Record<number, string>>({})
|
||||
const [savedSecretRows, setSavedSecretRows] = useState<Set<number>>(() => new Set())
|
||||
const [connectedIntegrationRows, setConnectedIntegrationRows] = useState<Set<number>>(
|
||||
@@ -2519,6 +2589,8 @@ function CredentialInputCard({
|
||||
return false
|
||||
}
|
||||
|
||||
await attachDescriptions(Object.keys(workspaceVariables))
|
||||
|
||||
const nextSavedSecretRows = new Set(savedSecretRows)
|
||||
for (const index of enteredSecretIndexes) nextSavedSecretRows.add(index)
|
||||
setSavedSecretRows(nextSavedSecretRows)
|
||||
|
||||
@@ -295,13 +295,25 @@ export const Browser: ToolCatalogEntry = {
|
||||
mode: 'async',
|
||||
parameters: {
|
||||
properties: {
|
||||
sessionId: {
|
||||
description:
|
||||
'Reusable session ID returned by an earlier browser call in this chat. Supply it only on a later user message that continues the same browsing objective, and at most once per user message.',
|
||||
type: 'string',
|
||||
},
|
||||
task: {
|
||||
description:
|
||||
'The web task to complete, in plain language (include the target site/URL if known).',
|
||||
"Optional brief scoping instruction that the conversation does not already convey. Do not restate the user's request.",
|
||||
type: 'string',
|
||||
},
|
||||
title: {
|
||||
description:
|
||||
"Required private orchestration label (3–8 words) for this Browser Agent session's stable objective. When resuming with sessionId, copy the registry title unchanged.",
|
||||
maxLength: 120,
|
||||
minLength: 1,
|
||||
type: 'string',
|
||||
},
|
||||
},
|
||||
required: ['task'],
|
||||
required: ['title'],
|
||||
type: 'object',
|
||||
},
|
||||
subagentId: 'browser',
|
||||
@@ -1248,16 +1260,14 @@ export const Cp: ToolCatalogEntry = {
|
||||
properties: {
|
||||
destination: {
|
||||
type: 'string',
|
||||
maxLength: 4096,
|
||||
description:
|
||||
'Target path under workflows/. An existing folder (or a path ending in "/") duplicates sources into it keeping their names; otherwise the last segment names the copy and the preceding segments are the target folder (created automatically when missing).',
|
||||
},
|
||||
sources: {
|
||||
type: 'array',
|
||||
maxItems: 100,
|
||||
description:
|
||||
'Canonical workflow VFS paths to duplicate, e.g. ["workflows/My%20Workflow"]. Copy paths verbatim from glob/grep/read output.',
|
||||
items: { type: 'string', maxLength: 4096 },
|
||||
items: { type: 'string' },
|
||||
},
|
||||
toolTitle: {
|
||||
type: 'string',
|
||||
@@ -3716,10 +3726,9 @@ export const Mkdir: ToolCatalogEntry = {
|
||||
properties: {
|
||||
paths: {
|
||||
type: 'array',
|
||||
maxItems: 100,
|
||||
description:
|
||||
'Canonical folder VFS paths to create, e.g. ["files/Reports/2026"]. Missing parent segments are created automatically.',
|
||||
items: { type: 'string', maxLength: 4096 },
|
||||
items: { type: 'string' },
|
||||
},
|
||||
toolTitle: {
|
||||
type: 'string',
|
||||
@@ -3742,16 +3751,14 @@ export const Mv: ToolCatalogEntry = {
|
||||
properties: {
|
||||
destination: {
|
||||
type: 'string',
|
||||
maxLength: 4096,
|
||||
description:
|
||||
'Target path. A path ending in "/" (or naming an existing folder) moves sources into it keeping their names — always use the trailing "/" form when targeting a folder. Otherwise the last segment is the new name and the preceding segments are the target folder (created automatically when missing).',
|
||||
},
|
||||
sources: {
|
||||
type: 'array',
|
||||
maxItems: 100,
|
||||
description:
|
||||
'Canonical VFS paths to move or rename, e.g. ["files/draft.md"]. All sources must share one category. Copy paths verbatim from glob/grep/read output.',
|
||||
items: { type: 'string', maxLength: 4096 },
|
||||
items: { type: 'string' },
|
||||
},
|
||||
toolTitle: {
|
||||
type: 'string',
|
||||
@@ -4184,10 +4191,9 @@ export const Rm: ToolCatalogEntry = {
|
||||
properties: {
|
||||
paths: {
|
||||
type: 'array',
|
||||
maxItems: 100,
|
||||
description:
|
||||
'Canonical VFS paths to delete, e.g. ["files/Reports/draft.md"]. Copy paths verbatim from glob/grep/read output. Paths from different categories may be mixed in one call.',
|
||||
items: { type: 'string', maxLength: 4096 },
|
||||
items: { type: 'string' },
|
||||
},
|
||||
toolTitle: {
|
||||
type: 'string',
|
||||
@@ -4752,10 +4758,19 @@ export const SetEnvironmentVariables: ToolCatalogEntry = {
|
||||
items: {
|
||||
type: 'object',
|
||||
properties: {
|
||||
description: {
|
||||
type: 'string',
|
||||
description:
|
||||
'What the variable is for, in one short phrase — aim for under 80 characters, like "Stripe live key for the billing workflow". Not a sentence, and never a restatement of the name. Workspace scope only; sending it with scope personal is rejected. Omit it on an existing variable to leave its current description untouched; send an empty string to clear one. You may send it alone, without a value, to describe a secret that already exists.',
|
||||
},
|
||||
name: { type: 'string', description: 'Variable name' },
|
||||
value: { type: 'string', description: 'Variable value' },
|
||||
value: {
|
||||
type: 'string',
|
||||
description:
|
||||
"Variable value. Omit it to leave an existing variable's value untouched and change only its description — never invent or guess a value you were not given, which would overwrite the real secret.",
|
||||
},
|
||||
},
|
||||
required: ['name', 'value'],
|
||||
required: ['name'],
|
||||
},
|
||||
},
|
||||
},
|
||||
|
||||
@@ -39,13 +39,25 @@ export const TOOL_RUNTIME_SCHEMAS: Record<string, ToolRuntimeSchemaEntry> = {
|
||||
browser: {
|
||||
parameters: {
|
||||
properties: {
|
||||
sessionId: {
|
||||
description:
|
||||
'Reusable session ID returned by an earlier browser call in this chat. Supply it only on a later user message that continues the same browsing objective, and at most once per user message.',
|
||||
type: 'string',
|
||||
},
|
||||
task: {
|
||||
description:
|
||||
'The web task to complete, in plain language (include the target site/URL if known).',
|
||||
"Optional brief scoping instruction that the conversation does not already convey. Do not restate the user's request.",
|
||||
type: 'string',
|
||||
},
|
||||
title: {
|
||||
description:
|
||||
"Required private orchestration label (3–8 words) for this Browser Agent session's stable objective. When resuming with sessionId, copy the registry title unchanged.",
|
||||
maxLength: 120,
|
||||
minLength: 1,
|
||||
type: 'string',
|
||||
},
|
||||
},
|
||||
required: ['task'],
|
||||
required: ['title'],
|
||||
type: 'object',
|
||||
},
|
||||
resultSchema: undefined,
|
||||
@@ -1112,18 +1124,15 @@ export const TOOL_RUNTIME_SCHEMAS: Record<string, ToolRuntimeSchemaEntry> = {
|
||||
properties: {
|
||||
destination: {
|
||||
type: 'string',
|
||||
maxLength: 4096,
|
||||
description:
|
||||
'Target path under workflows/. An existing folder (or a path ending in "/") duplicates sources into it keeping their names; otherwise the last segment names the copy and the preceding segments are the target folder (created automatically when missing).',
|
||||
},
|
||||
sources: {
|
||||
type: 'array',
|
||||
maxItems: 100,
|
||||
description:
|
||||
'Canonical workflow VFS paths to duplicate, e.g. ["workflows/My%20Workflow"]. Copy paths verbatim from glob/grep/read output.',
|
||||
items: {
|
||||
type: 'string',
|
||||
maxLength: 4096,
|
||||
},
|
||||
},
|
||||
toolTitle: {
|
||||
@@ -3596,12 +3605,10 @@ export const TOOL_RUNTIME_SCHEMAS: Record<string, ToolRuntimeSchemaEntry> = {
|
||||
properties: {
|
||||
paths: {
|
||||
type: 'array',
|
||||
maxItems: 100,
|
||||
description:
|
||||
'Canonical folder VFS paths to create, e.g. ["files/Reports/2026"]. Missing parent segments are created automatically.',
|
||||
items: {
|
||||
type: 'string',
|
||||
maxLength: 4096,
|
||||
},
|
||||
},
|
||||
toolTitle: {
|
||||
@@ -3620,18 +3627,15 @@ export const TOOL_RUNTIME_SCHEMAS: Record<string, ToolRuntimeSchemaEntry> = {
|
||||
properties: {
|
||||
destination: {
|
||||
type: 'string',
|
||||
maxLength: 4096,
|
||||
description:
|
||||
'Target path. A path ending in "/" (or naming an existing folder) moves sources into it keeping their names — always use the trailing "/" form when targeting a folder. Otherwise the last segment is the new name and the preceding segments are the target folder (created automatically when missing).',
|
||||
},
|
||||
sources: {
|
||||
type: 'array',
|
||||
maxItems: 100,
|
||||
description:
|
||||
'Canonical VFS paths to move or rename, e.g. ["files/draft.md"]. All sources must share one category. Copy paths verbatim from glob/grep/read output.',
|
||||
items: {
|
||||
type: 'string',
|
||||
maxLength: 4096,
|
||||
},
|
||||
},
|
||||
toolTitle: {
|
||||
@@ -4067,12 +4071,10 @@ export const TOOL_RUNTIME_SCHEMAS: Record<string, ToolRuntimeSchemaEntry> = {
|
||||
properties: {
|
||||
paths: {
|
||||
type: 'array',
|
||||
maxItems: 100,
|
||||
description:
|
||||
'Canonical VFS paths to delete, e.g. ["files/Reports/draft.md"]. Copy paths verbatim from glob/grep/read output. Paths from different categories may be mixed in one call.',
|
||||
items: {
|
||||
type: 'string',
|
||||
maxLength: 4096,
|
||||
},
|
||||
},
|
||||
toolTitle: {
|
||||
@@ -4597,16 +4599,22 @@ export const TOOL_RUNTIME_SCHEMAS: Record<string, ToolRuntimeSchemaEntry> = {
|
||||
items: {
|
||||
type: 'object',
|
||||
properties: {
|
||||
description: {
|
||||
type: 'string',
|
||||
description:
|
||||
'What the variable is for, in one short phrase — aim for under 80 characters, like "Stripe live key for the billing workflow". Not a sentence, and never a restatement of the name. Workspace scope only; sending it with scope personal is rejected. Omit it on an existing variable to leave its current description untouched; send an empty string to clear one. You may send it alone, without a value, to describe a secret that already exists.',
|
||||
},
|
||||
name: {
|
||||
type: 'string',
|
||||
description: 'Variable name',
|
||||
},
|
||||
value: {
|
||||
type: 'string',
|
||||
description: 'Variable value',
|
||||
description:
|
||||
"Variable value. Omit it to leave an existing variable's value untouched and change only its description — never invent or guess a value you were not given, which would overwrite the real secret.",
|
||||
},
|
||||
},
|
||||
required: ['name', 'value'],
|
||||
required: ['name'],
|
||||
},
|
||||
},
|
||||
},
|
||||
|
||||
@@ -12,12 +12,27 @@ const {
|
||||
|
||||
afterAll(resetEnvironmentUtilsMock)
|
||||
|
||||
const { ensureWorkflowAccessMock, ensureWorkspaceAccessMock, getDefaultWorkspaceIdMock } =
|
||||
vi.hoisted(() => ({
|
||||
ensureWorkflowAccessMock: vi.fn(),
|
||||
ensureWorkspaceAccessMock: vi.fn(),
|
||||
getDefaultWorkspaceIdMock: vi.fn(),
|
||||
}))
|
||||
const {
|
||||
ensureWorkflowAccessMock,
|
||||
ensureWorkspaceAccessMock,
|
||||
getDefaultWorkspaceIdMock,
|
||||
listCredentialsMock,
|
||||
performUpdateCredentialMock,
|
||||
} = vi.hoisted(() => ({
|
||||
ensureWorkflowAccessMock: vi.fn(),
|
||||
ensureWorkspaceAccessMock: vi.fn(),
|
||||
getDefaultWorkspaceIdMock: vi.fn(),
|
||||
listCredentialsMock: vi.fn(),
|
||||
performUpdateCredentialMock: vi.fn(),
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/credentials/queries', () => ({
|
||||
listVisibleWorkspaceCredentials: listCredentialsMock,
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/credentials/orchestration', () => ({
|
||||
performUpdateCredential: performUpdateCredentialMock,
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/copilot/tools/handlers/access', () => ({
|
||||
ensureWorkflowAccess: ensureWorkflowAccessMock,
|
||||
@@ -37,6 +52,13 @@ describe('setEnvironmentVariablesServerTool', () => {
|
||||
getDefaultWorkspaceIdMock.mockResolvedValue('ws-default')
|
||||
upsertPersonalEnvVarsMock.mockResolvedValue({ added: ['API_KEY'], updated: [] })
|
||||
upsertWorkspaceEnvVarsMock.mockResolvedValue(['API_KEY'])
|
||||
listCredentialsMock.mockResolvedValue({
|
||||
data: [
|
||||
{ id: 'cred-api', envKey: 'API_KEY' },
|
||||
{ id: 'cred-other', envKey: 'OTHER_KEY' },
|
||||
],
|
||||
})
|
||||
performUpdateCredentialMock.mockResolvedValue({ success: true })
|
||||
})
|
||||
|
||||
it('defaults to workspace scope and uses the current workspace context', async () => {
|
||||
@@ -92,4 +114,111 @@ describe('setEnvironmentVariablesServerTool', () => {
|
||||
'user-1'
|
||||
)
|
||||
})
|
||||
|
||||
it('describes a workspace secret through the credential update handler, never rewriting its value', async () => {
|
||||
await setEnvironmentVariablesServerTool.execute(
|
||||
{
|
||||
variables: [
|
||||
{ name: 'API_KEY', value: 'secret', description: ' Stripe live key ' },
|
||||
{ name: 'OTHER_KEY', value: 'other' },
|
||||
],
|
||||
},
|
||||
{ userId: 'user-1', workspaceId: 'ws-1' }
|
||||
)
|
||||
|
||||
expect(performUpdateCredentialMock).toHaveBeenCalledTimes(1)
|
||||
expect(performUpdateCredentialMock).toHaveBeenCalledWith({
|
||||
credentialId: 'cred-api',
|
||||
userId: 'user-1',
|
||||
description: 'Stripe live key',
|
||||
allowedTypes: ['env_workspace'],
|
||||
})
|
||||
// The access-checked value write runs first: it authorizes the caller and
|
||||
// mints the credential row a new key's description hangs on.
|
||||
expect(upsertWorkspaceEnvVarsMock.mock.invocationCallOrder[0]).toBeLessThan(
|
||||
performUpdateCredentialMock.mock.invocationCallOrder[0]
|
||||
)
|
||||
})
|
||||
|
||||
it('describes a secret that already exists without touching its value', async () => {
|
||||
const result = await setEnvironmentVariablesServerTool.execute(
|
||||
{ variables: [{ name: 'API_KEY', description: 'Stripe live key' }] },
|
||||
{ userId: 'user-1', workspaceId: 'ws-1' }
|
||||
)
|
||||
|
||||
// Nothing is written to the secret itself: coercing the absent value to ''
|
||||
// would blank the very secret the model is annotating.
|
||||
expect(upsertWorkspaceEnvVarsMock).toHaveBeenCalledWith('ws-1', {}, 'user-1')
|
||||
expect(performUpdateCredentialMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ credentialId: 'cred-api', description: 'Stripe live key' })
|
||||
)
|
||||
expect(result.describedVariables).toEqual(['API_KEY'])
|
||||
})
|
||||
|
||||
it('keeps a stored value reported when its description fails', async () => {
|
||||
performUpdateCredentialMock.mockResolvedValue({ success: false, error: 'Forbidden' })
|
||||
|
||||
const result = await setEnvironmentVariablesServerTool.execute(
|
||||
{ variables: [{ name: 'API_KEY', value: 'secret', description: 'Stripe live key' }] },
|
||||
{ userId: 'user-1', workspaceId: 'ws-1' }
|
||||
)
|
||||
|
||||
expect(result.workspaceUpdatedVariables).toEqual(['API_KEY'])
|
||||
expect(result.describedVariables).toEqual([])
|
||||
expect(result.message).toContain('API_KEY: Forbidden')
|
||||
})
|
||||
|
||||
it('fails a describe-only call that saved nothing', async () => {
|
||||
performUpdateCredentialMock.mockResolvedValue({ success: false, error: 'Forbidden' })
|
||||
upsertWorkspaceEnvVarsMock.mockResolvedValue([])
|
||||
|
||||
await expect(
|
||||
setEnvironmentVariablesServerTool.execute(
|
||||
{ variables: [{ name: 'API_KEY', description: 'Stripe live key' }] },
|
||||
{ userId: 'user-1', workspaceId: 'ws-1' }
|
||||
)
|
||||
).rejects.toThrow('Could not describe: API_KEY: Forbidden')
|
||||
})
|
||||
|
||||
it('clears a description sent blank and leaves an omitted one alone', async () => {
|
||||
await setEnvironmentVariablesServerTool.execute(
|
||||
{
|
||||
variables: [
|
||||
{ name: 'API_KEY', value: 'secret', description: ' ' },
|
||||
{ name: 'OTHER_KEY', value: 'other' },
|
||||
],
|
||||
},
|
||||
{ userId: 'user-1', workspaceId: 'ws-1' }
|
||||
)
|
||||
|
||||
expect(performUpdateCredentialMock).toHaveBeenCalledTimes(1)
|
||||
expect(performUpdateCredentialMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ credentialId: 'cred-api', description: null })
|
||||
)
|
||||
})
|
||||
|
||||
it('rejects a description on a personal secret', async () => {
|
||||
await expect(
|
||||
setEnvironmentVariablesServerTool.execute(
|
||||
{
|
||||
scope: 'personal',
|
||||
variables: [{ name: 'API_KEY', value: 'secret', description: 'my key' }],
|
||||
},
|
||||
{ userId: 'user-1', workspaceId: 'ws-1' }
|
||||
)
|
||||
).rejects.toThrow('description is only supported for a workspace secret')
|
||||
|
||||
expect(upsertPersonalEnvVarsMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('rejects a description longer than the secret detail form allows', async () => {
|
||||
await expect(
|
||||
setEnvironmentVariablesServerTool.execute(
|
||||
{ variables: [{ name: 'API_KEY', value: 'secret', description: 'a'.repeat(501) }] },
|
||||
{ userId: 'user-1', workspaceId: 'ws-1' }
|
||||
)
|
||||
).rejects.toThrow('description for API_KEY must be at most 500 characters')
|
||||
|
||||
expect(upsertWorkspaceEnvVarsMock).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -7,6 +7,8 @@ import {
|
||||
getDefaultWorkspaceId,
|
||||
} from '@/lib/copilot/tools/handlers/access'
|
||||
import type { BaseServerTool, ServerToolContext } from '@/lib/copilot/tools/server/base-tool'
|
||||
import { performUpdateCredential } from '@/lib/credentials/orchestration'
|
||||
import { listVisibleWorkspaceCredentials } from '@/lib/credentials/queries'
|
||||
import { upsertPersonalEnvVars, upsertWorkspaceEnvVars } from '@/lib/environment/utils'
|
||||
|
||||
type EnvironmentVariableInputValue = string | number | boolean | null | undefined
|
||||
@@ -14,8 +16,12 @@ type EnvironmentVariableInputValue = string | number | boolean | null | undefine
|
||||
interface EnvironmentVariableInput {
|
||||
name: string
|
||||
value: EnvironmentVariableInputValue
|
||||
description?: string | null
|
||||
}
|
||||
|
||||
/** Matches the secret detail form and `PUT /api/v2/secrets`. */
|
||||
const DESCRIPTION_MAX_LENGTH = 500
|
||||
|
||||
interface SetEnvironmentVariablesParams {
|
||||
variables: Record<string, EnvironmentVariableInputValue> | EnvironmentVariableInput[]
|
||||
scope?: 'personal' | 'workspace'
|
||||
@@ -32,17 +38,56 @@ interface SetEnvironmentVariablesResult {
|
||||
addedVariables: string[]
|
||||
updatedVariables: string[]
|
||||
workspaceUpdatedVariables: string[]
|
||||
describedVariables: string[]
|
||||
}
|
||||
|
||||
const EnvVarSchema = z.object({ variables: z.record(z.string(), z.string()) })
|
||||
|
||||
/** A row that only annotates a secret that already exists — it sends no value. */
|
||||
function isDescriptionOnly(item: EnvironmentVariableInput): boolean {
|
||||
return (
|
||||
(item.value === undefined || item.value === null) &&
|
||||
typeof item.description === 'string' &&
|
||||
item.description.trim().length > 0
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Collects the descriptions the model actually sent. A variable that omits the
|
||||
* field is absent from the result, so an existing description survives a value
|
||||
* rotation; a blank one clears it. The object form of `variables` carries none.
|
||||
*/
|
||||
function normalizeDescriptions(
|
||||
input: Record<string, EnvironmentVariableInputValue> | EnvironmentVariableInput[]
|
||||
): Record<string, string | null> {
|
||||
if (!Array.isArray(input)) return {}
|
||||
|
||||
const descriptions: Record<string, string | null> = {}
|
||||
for (const item of input) {
|
||||
if (!item || typeof item.name !== 'string' || item.description === undefined) continue
|
||||
const description = item.description?.trim() ?? ''
|
||||
if (description.length > DESCRIPTION_MAX_LENGTH) {
|
||||
throw new Error(
|
||||
`description for ${item.name} must be at most ${DESCRIPTION_MAX_LENGTH} characters`
|
||||
)
|
||||
}
|
||||
descriptions[item.name] = description === '' ? null : description
|
||||
}
|
||||
return descriptions
|
||||
}
|
||||
|
||||
/**
|
||||
* Values to write. An array item that carries a description but no value is a
|
||||
* description-only edit and is deliberately absent here: coercing its missing
|
||||
* value to `''` would blank the very secret the model is trying to annotate.
|
||||
*/
|
||||
function normalizeVariables(
|
||||
input: Record<string, EnvironmentVariableInputValue> | EnvironmentVariableInput[]
|
||||
): Record<string, string> {
|
||||
if (Array.isArray(input)) {
|
||||
return input.reduce(
|
||||
(acc, item) => {
|
||||
if (item && typeof item.name === 'string') {
|
||||
if (item && typeof item.name === 'string' && !isDescriptionOnly(item)) {
|
||||
acc[item.name] = String(item.value ?? '')
|
||||
}
|
||||
return acc
|
||||
@@ -55,6 +100,57 @@ function normalizeVariables(
|
||||
) as Record<string, string>
|
||||
}
|
||||
|
||||
/**
|
||||
* Writes descriptions onto secrets, never their values. Resolves each key to its
|
||||
* credential row and hands it to `performUpdateCredential` — the same handler the
|
||||
* secrets settings page calls — so credential-admin access, the `env_personal`
|
||||
* refusal, and the audit record are all decided in one place.
|
||||
*
|
||||
* Only the `description` column is touched. Re-sending the value to attach a note
|
||||
* would clobber a rotation that landed between the two writes, and the note is
|
||||
* never worth losing someone else's secret over.
|
||||
*/
|
||||
async function describeSecrets(params: {
|
||||
workspaceId: string
|
||||
userId: string
|
||||
descriptions: Record<string, string | null>
|
||||
}): Promise<{ described: string[]; failures: string[] }> {
|
||||
const names = Object.keys(params.descriptions)
|
||||
if (names.length === 0) return { described: [], failures: [] }
|
||||
|
||||
const { data: credentials } = await listVisibleWorkspaceCredentials({
|
||||
workspaceId: params.workspaceId,
|
||||
userId: params.userId,
|
||||
workspaceAccess: { canAdmin: false },
|
||||
types: ['env_workspace'],
|
||||
})
|
||||
const idByEnvKey = new Map(
|
||||
credentials.flatMap((row) => (row.envKey ? [[row.envKey, row.id] as const] : []))
|
||||
)
|
||||
|
||||
const described: string[] = []
|
||||
const failures: string[] = []
|
||||
for (const name of names) {
|
||||
const credentialId = idByEnvKey.get(name)
|
||||
if (!credentialId) {
|
||||
failures.push(`no workspace secret named ${name}`)
|
||||
continue
|
||||
}
|
||||
const result = await performUpdateCredential({
|
||||
credentialId,
|
||||
userId: params.userId,
|
||||
description: params.descriptions[name],
|
||||
allowedTypes: ['env_workspace'],
|
||||
})
|
||||
if (result.success) {
|
||||
described.push(name)
|
||||
} else {
|
||||
failures.push(`${name}: ${result.error ?? 'could not be described'}`)
|
||||
}
|
||||
}
|
||||
return { described, failures }
|
||||
}
|
||||
|
||||
async function resolveWorkspaceId(
|
||||
params: SetEnvironmentVariablesParams,
|
||||
context: ServerToolContext | undefined,
|
||||
@@ -100,11 +196,21 @@ export const setEnvironmentVariablesServerTool: BaseServerTool<
|
||||
const scope = params.scope === 'personal' ? 'personal' : 'workspace'
|
||||
|
||||
const normalized = normalizeVariables(variables || {})
|
||||
const descriptions = normalizeDescriptions(variables || {})
|
||||
// Rejected rather than dropped, matching `PUT /api/v2/secrets` and the domain
|
||||
// layer: a personal secret's value is user-global, but its credential rows are
|
||||
// per-workspace mirrors, so there is no single row to hold its description —
|
||||
// one written here would exist in this workspace alone.
|
||||
if (scope === 'personal' && Object.keys(descriptions).length > 0) {
|
||||
throw new Error('description is only supported for a workspace secret')
|
||||
}
|
||||
const { variables: validatedVariables } = EnvVarSchema.parse({ variables: normalized })
|
||||
const variableNames = Object.keys(validatedVariables)
|
||||
const added: string[] = []
|
||||
const updated: string[] = []
|
||||
let workspaceUpdated: string[] = []
|
||||
let described: string[] = []
|
||||
let descriptionFailures: string[] = []
|
||||
|
||||
let resolvedWorkspaceId: string | undefined
|
||||
if (scope === 'workspace') {
|
||||
@@ -114,6 +220,15 @@ export const setEnvironmentVariablesServerTool: BaseServerTool<
|
||||
validatedVariables,
|
||||
authenticatedUserId
|
||||
)
|
||||
// Runs after the value write, which is what mints the credential row a
|
||||
// brand-new key's description hangs on.
|
||||
const outcome = await describeSecrets({
|
||||
workspaceId: resolvedWorkspaceId,
|
||||
userId: authenticatedUserId,
|
||||
descriptions,
|
||||
})
|
||||
described = outcome.described
|
||||
descriptionFailures = outcome.failures
|
||||
} else {
|
||||
const result = await upsertPersonalEnvVars(authenticatedUserId, validatedVariables)
|
||||
added.push(...result.added)
|
||||
@@ -131,11 +246,20 @@ export const setEnvironmentVariablesServerTool: BaseServerTool<
|
||||
workspaceId: resolvedWorkspaceId,
|
||||
})
|
||||
|
||||
// A failed description never fails a stored value — but a describe-only call
|
||||
// has nothing else to report, so its failure is the result.
|
||||
if (descriptionFailures.length > 0 && workspaceUpdated.length === 0) {
|
||||
throw new Error(`Could not describe: ${descriptionFailures.join('; ')}`)
|
||||
}
|
||||
|
||||
const parts: string[] = []
|
||||
if (added.length > 0) parts.push(`${added.length} personal secret(s) added`)
|
||||
if (updated.length > 0) parts.push(`${updated.length} personal secret(s) updated`)
|
||||
if (workspaceUpdated.length > 0)
|
||||
parts.push(`${workspaceUpdated.length} workspace secret(s) updated`)
|
||||
if (described.length > 0) parts.push(`${described.length} description(s) saved`)
|
||||
if (descriptionFailures.length > 0)
|
||||
parts.push(`descriptions not saved (${descriptionFailures.join('; ')})`)
|
||||
|
||||
return {
|
||||
message: `Successfully processed ${totalProcessed} secret(s): ${parts.join(', ')}`,
|
||||
@@ -146,6 +270,7 @@ export const setEnvironmentVariablesServerTool: BaseServerTool<
|
||||
addedVariables: added,
|
||||
updatedVariables: updated,
|
||||
workspaceUpdatedVariables: workspaceUpdated,
|
||||
describedVariables: described,
|
||||
}
|
||||
},
|
||||
}
|
||||
|
||||
@@ -527,4 +527,20 @@ describe('serializeCredentials — type distinguishes reconnect flow', () => {
|
||||
)
|
||||
expect(json[0].type).toBeUndefined()
|
||||
})
|
||||
|
||||
it('shows what a workspace secret is for, and omits the field when nothing was recorded', () => {
|
||||
const json = JSON.parse(
|
||||
serializeCredentials([
|
||||
{
|
||||
providerId: 'STRIPE_KEY',
|
||||
description: 'Stripe live key for billing',
|
||||
scope: 'workspace',
|
||||
createdAt: now,
|
||||
},
|
||||
{ providerId: 'OPENAI_API_KEY', description: null, scope: 'workspace', createdAt: now },
|
||||
])
|
||||
)
|
||||
expect(json[0].description).toBe('Stripe live key for billing')
|
||||
expect(json[1]).not.toHaveProperty('description')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -715,6 +715,8 @@ export function serializeCredentials(
|
||||
id?: string
|
||||
providerId: string
|
||||
displayName?: string | null
|
||||
/** What a workspace secret is for, when one has been recorded. */
|
||||
description?: string | null
|
||||
role?: string | null
|
||||
scope: string | null
|
||||
/** 'service_account' for a shared app credential; omitted/undefined for a personal OAuth connection. */
|
||||
@@ -727,6 +729,7 @@ export function serializeCredentials(
|
||||
id: a.id || undefined,
|
||||
provider: a.providerId,
|
||||
displayName: a.displayName || undefined,
|
||||
description: a.description || undefined,
|
||||
role: a.role || undefined,
|
||||
scope: a.scope || undefined,
|
||||
// 'oauth' (personal connection) vs 'service_account' (shared app
|
||||
|
||||
@@ -2521,6 +2521,7 @@ export class WorkspaceVFS {
|
||||
serializeCredentials([
|
||||
...visibleEnvCredentials.map((c) => ({
|
||||
providerId: c.envKey,
|
||||
description: c.description,
|
||||
scope: c.type === 'env_workspace' ? 'workspace' : 'personal',
|
||||
createdAt: c.updatedAt,
|
||||
})),
|
||||
|
||||
@@ -217,6 +217,8 @@ interface AccessibleEnvCredential {
|
||||
type: 'env_workspace' | 'env_personal'
|
||||
envKey: string
|
||||
envOwnerUserId: string | null
|
||||
/** Always null on `env_personal`: a mirror row cannot own a user-global secret's note. */
|
||||
description: string | null
|
||||
updatedAt: Date
|
||||
}
|
||||
|
||||
@@ -740,6 +742,7 @@ export async function getAccessibleEnvCredentials(
|
||||
type: credential.type,
|
||||
envKey: credential.envKey,
|
||||
envOwnerUserId: credential.envOwnerUserId,
|
||||
description: credential.description,
|
||||
updatedAt: credential.updatedAt,
|
||||
})
|
||||
.from(credential)
|
||||
@@ -772,6 +775,7 @@ export async function getAccessibleEnvCredentials(
|
||||
type: row.type,
|
||||
envKey: row.envKey,
|
||||
envOwnerUserId: row.envOwnerUserId,
|
||||
description: row.type === 'env_workspace' ? row.description : null,
|
||||
updatedAt: row.updatedAt,
|
||||
}))
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user