From c60e92e09bf36ed32ea268bc2f411ae0584a155e Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 17 Aug 2026 17:07:39 -0700 Subject: [PATCH 1/3] feat(secrets): add optional descriptions to workspace secrets Workspace secrets already have a backing credential row with a description column, but nothing surfaced it. Teammates had no way to record what a secret is for. - Add a Description field to the secret detail page, matching the integrations credential page, gated on workspace-secret admin - Fold the value and description editors into one Save/Discard pair and one unsaved-changes guard; two guards cannot coexist, since each seeds its own same-URL history entry - Match descriptions in the secrets settings search - Expose description on GET/PUT /api/v2/secrets and in the CLI Descriptions are workspace-only: env_personal credential rows are per-workspace mirrors of one user-global secret, so one saved there would exist in a single workspace, and a personal secret has no teammates to inform. The API rejects a description on personal scope rather than silently dropping it, and omitting it on PUT leaves any existing description untouched so a value rotation cannot erase it. --- apps/docs/content/docs/en/cli/reference.mdx | 1 + apps/docs/content/docs/en/cli/secrets.mdx | 1 + apps/docs/openapi-v2-resources.json | 27 +++++++- .../app/api/v2/secrets/[name]/route.test.ts | 49 ++++++++++++++ apps/sim/app/api/v2/secrets/route.test.ts | 33 ++++++++++ apps/sim/app/api/v2/secrets/utils.ts | 1 + .../hooks/use-credential-detail-form.ts | 48 ++++++++++++-- .../connected-credential-detail.tsx | 2 +- .../secrets-manager/secrets-manager.tsx | 14 +++- .../secrets/hooks/use-secret-value.ts | 9 ++- .../secrets/[credentialId]/secret-detail.tsx | 64 +++++++++++++------ .../lib/api/contracts/v2/openapi/resources.ts | 1 + apps/sim/lib/api/contracts/v2/secrets.ts | 23 +++++++ apps/sim/lib/credentials/secret-values.ts | 10 ++- apps/sim/lib/secrets/application/use-cases.ts | 13 +++- packages/sim-cli/src/commands/secrets.ts | 29 +++++++++ packages/sim-cli/src/contract/commands.ts | 1 + packages/sim-cli/src/generated/v2-api.ts | 8 +++ 18 files changed, 299 insertions(+), 35 deletions(-) diff --git a/apps/docs/content/docs/en/cli/reference.mdx b/apps/docs/content/docs/en/cli/reference.mdx index 51250137abb..0edf783ac28 100644 --- a/apps/docs/content/docs/en/cli/reference.mdx +++ b/apps/docs/content/docs/en/cli/reference.mdx @@ -1787,6 +1787,7 @@ sim secrets set [options] | --- | --- | --- | | `--scope ` | Yes | Secret ownership scope. Accepted values: `workspace`, `personal`. | | `--value ` | No | Secret value; visible to shell history when supplied directly. | +| `--description ` | No | What the secret is for, shown to teammates; workspace scope only. Omit to leave an existing description unchanged. | diff --git a/apps/docs/content/docs/en/cli/secrets.mdx b/apps/docs/content/docs/en/cli/secrets.mdx index b13dfb1751f..36d4a73c31c 100644 --- a/apps/docs/content/docs/en/cli/secrets.mdx +++ b/apps/docs/content/docs/en/cli/secrets.mdx @@ -80,5 +80,6 @@ sim secrets set [options] | --- | --- | --- | | `--scope ` | Yes | Secret ownership scope. Accepted values: `workspace`, `personal`. | | `--value ` | No | Secret value; visible to shell history when supplied directly. | +| `--description ` | No | What the secret is for, shown to teammates; workspace scope only. Omit to leave an existing description unchanged. | diff --git a/apps/docs/openapi-v2-resources.json b/apps/docs/openapi-v2-resources.json index a9b3861e23c..68a997a9f83 100644 --- a/apps/docs/openapi-v2-resources.json +++ b/apps/docs/openapi-v2-resources.json @@ -5052,6 +5052,17 @@ "enum": ["workspace", "personal"], "description": "Whether the secret belongs to the workspace or to the caller. A personal secret belongs to the caller across every workspace, not to one workspace." }, + "description": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "description": "What the secret is for, as set on the workspace secret. Always null for a personal secret, which has no shared audience." + }, "role": { "type": "string", "enum": ["admin", "member"], @@ -5070,7 +5081,7 @@ "description": "ISO 8601 timestamp when the secret was last updated." } }, - "required": ["name", "scope", "role", "createdAt", "updatedAt"], + "required": ["name", "scope", "description", "role", "createdAt", "updatedAt"], "additionalProperties": false, "title": "Secret metadata", "description": "Public secret metadata without the stored secret value." @@ -5107,6 +5118,7 @@ { "name": "STRIPE_API_KEY", "scope": "workspace", + "description": "Production billing key — rotate quarterly.", "role": "admin", "createdAt": "2026-06-01T09:14:00.000Z", "updatedAt": "2026-06-20T14:02:11.000Z" @@ -5133,6 +5145,7 @@ "data": { "name": "STRIPE_API_KEY", "scope": "workspace", + "description": "Production billing key — rotate quarterly.", "role": "admin", "createdAt": "2026-06-01T09:14:00.000Z", "updatedAt": "2026-06-20T14:02:11.000Z" @@ -5160,6 +5173,18 @@ "maxLength": 65536, "description": "Write-only secret value. It is never returned.", "writeOnly": true + }, + "description": { + "description": "What the secret is for, shown to teammates. Omit to leave an existing description untouched; pass null to clear it. Workspace scope only.", + "anyOf": [ + { + "type": "string", + "maxLength": 500 + }, + { + "type": "null" + } + ] } }, "required": ["workspaceId", "scope", "value"], diff --git a/apps/sim/app/api/v2/secrets/[name]/route.test.ts b/apps/sim/app/api/v2/secrets/[name]/route.test.ts index 7f49d462fb8..7ed0a5a580c 100644 --- a/apps/sim/app/api/v2/secrets/[name]/route.test.ts +++ b/apps/sim/app/api/v2/secrets/[name]/route.test.ts @@ -135,6 +135,55 @@ describe('/api/v2/secrets/[name]', () => { }) }) + it('forwards a workspace description to the set operation', async () => { + const response = await PUT( + request('PUT', { + workspaceId: WORKSPACE_ID, + scope: 'workspace', + value: 'secret-value', + description: ' Prod billing key ', + }), + context + ) + + expect(response.status).toBe(201) + expect(mocks.set).toHaveBeenCalledWith({ + principal: PRINCIPAL, + input: { + workspaceId: WORKSPACE_ID, + name: SECRET_NAME, + scope: 'workspace', + value: 'secret-value', + description: 'Prod billing key', + }, + request: expect.anything(), + }) + }) + + it('omits description entirely when unset so a rotation cannot erase it', async () => { + await PUT( + request('PUT', { workspaceId: WORKSPACE_ID, scope: 'workspace', value: 'rotated' }), + context + ) + + expect(mocks.set.mock.calls[0][0].input).not.toHaveProperty('description') + }) + + it('rejects a description on a personal secret rather than dropping it', async () => { + const response = await PUT( + request('PUT', { + workspaceId: WORKSPACE_ID, + scope: 'personal', + value: 'secret-value', + description: 'has no shared audience', + }), + context + ) + + expect(response.status).toBe(400) + expect(mocks.set).not.toHaveBeenCalled() + }) + it('returns 200 when replacing an existing secret', async () => { mocks.set.mockResolvedValueOnce({ secret, userId: 'user-1', created: false }) diff --git a/apps/sim/app/api/v2/secrets/route.test.ts b/apps/sim/app/api/v2/secrets/route.test.ts index ef740920143..7de65d992ba 100644 --- a/apps/sim/app/api/v2/secrets/route.test.ts +++ b/apps/sim/app/api/v2/secrets/route.test.ts @@ -113,6 +113,7 @@ describe('GET /api/v2/secrets', () => { { name: 'STRIPE_API_KEY', scope: 'workspace', + description: null, role: 'admin', createdAt: '2026-01-01T00:00:00.000Z', updatedAt: '2026-01-02T00:00:00.000Z', @@ -142,6 +143,38 @@ describe('GET /api/v2/secrets', () => { * `mapInput` — because the contract-level sweep only checks a hand-maintained * map of param names and stays green when a route drops the stamp entirely. */ + it('reports a workspace secret description and never a personal one', async () => { + mocks.list.mockResolvedValue({ + secrets: [ + { ...secret, description: 'Prod billing key' }, + { + ...secret, + id: 'secret-2', + type: 'env_personal' as const, + displayName: 'MY_TEST_KEY', + envKey: 'MY_TEST_KEY', + envOwnerUserId: 'user-1', + description: 'leaked from a workspace mirror', + }, + ], + userId: 'user-1', + nextCursorKeys: null, + sortBy: 'name', + sortOrder: 'asc', + }) + + const response = await GET( + new NextRequest(`http://localhost:3000/api/v2/secrets?workspaceId=${WORKSPACE_ID}`, { + headers: { 'x-api-key': 'key' }, + }) + ) + const body = await response.json() + + expect(response.status).toBe(200) + expect(body.data[0].description).toBe('Prod billing key') + expect(body.data[1].description).toBeNull() + }) + it('refuses a cursor minted under a different filter', async () => { mocks.list.mockResolvedValue({ secrets: [secret], diff --git a/apps/sim/app/api/v2/secrets/utils.ts b/apps/sim/app/api/v2/secrets/utils.ts index 498040cd920..ba2d370c780 100644 --- a/apps/sim/app/api/v2/secrets/utils.ts +++ b/apps/sim/app/api/v2/secrets/utils.ts @@ -13,6 +13,7 @@ export function toV2Secret(row: VisibleWorkspaceCredential, userId: string): V2S return { name: row.envKey, scope: row.type === 'env_workspace' ? 'workspace' : 'personal', + description: row.type === 'env_workspace' ? row.description : null, role: row.role, createdAt: row.createdAt.toISOString(), updatedAt: row.updatedAt.toISOString(), diff --git a/apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-credential-detail-form.ts b/apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-credential-detail-form.ts index 6ebda086901..c62b712f6f0 100644 --- a/apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-credential-detail-form.ts +++ b/apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-credential-detail-form.ts @@ -9,22 +9,44 @@ import { useUnsavedChangesGuard } from './use-unsaved-changes-guard' const logger = createLogger('CredentialDetailForm') +/** + * A second editable section rendered on the same detail page (e.g. a secret's + * value), whose lifecycle is folded into the form's. + */ +export interface CredentialDetailFormSection { + isDirty: boolean + isSaving: boolean + /** Resolves false when the write failed, which stops the metadata save from committing alone. */ + save: () => Promise + discard: () => void +} + interface UseCredentialDetailFormParams { credential: WorkspaceCredential | null isAdmin: boolean /** Where the back link / discard navigates to. */ backHref: string + /** + * An additional editable section on the page, folded into one dirty state, one + * save, and one unsaved-changes guard. Two independent guards on a page cannot + * coexist: each seeds its own same-URL history entry while dirty, so Back would + * pop only one of them and leave the other stranded. + */ + section?: CredentialDetailFormSection } /** * Shared editable-metadata controller for a credential detail page: Display Name * and Description drafts seeded from the credential, dirty tracking, an - * admin-only save, and the shared unsaved-changes guard. + * admin-only save, and the shared unsaved-changes guard. An optional + * {@link CredentialDetailFormSection} folds a second editor on the same page + * into that one save and one guard. */ export function useCredentialDetailForm({ credential, isAdmin, backHref, + section, }: UseCredentialDetailFormParams) { const updateCredential = useUpdateWorkspaceCredential() @@ -50,12 +72,21 @@ export function useCredentialDetailForm({ const isDescriptionDirty = credential ? descriptionDraft !== (credential.description || '') : false - const isDirty = isDisplayNameDirty || isDescriptionDirty + const isMetadataDirty = isDisplayNameDirty || isDescriptionDirty + const isSectionDirty = section?.isDirty ?? false + const isDirty = isMetadataDirty || isSectionDirty + const isSaving = updateCredential.isPending || (section?.isSaving ?? false) const guard = useUnsavedChangesGuard({ isDirty, backHref }) const save = useCallback(async () => { - if (!credential || !isAdmin || !isDirty || updateCredential.isPending) return + if (!credential || isSaving) return + const savesMetadata = isAdmin && isMetadataDirty + if (!savesMetadata && !isSectionDirty) return + + if (isSectionDirty && !(await section?.save())) return + if (!savesMetadata) return + try { await updateCredential.mutateAsync({ credentialId: credential.id, @@ -73,18 +104,21 @@ export function useCredentialDetailForm({ }, [ credential, isAdmin, - isDirty, + isMetadataDirty, + isSectionDirty, + isSaving, + section, isDisplayNameDirty, isDescriptionDirty, displayNameDraft, descriptionDraft, updateCredential.mutateAsync, - updateCredential.isPending, ]) const discard = useCallback(() => { if (credential) seedDrafts(credential) - }, [credential, seedDrafts]) + section?.discard() + }, [credential, section, seedDrafts]) return { displayNameDraft, @@ -94,7 +128,7 @@ export function useCredentialDetailForm({ isDirty, save, discard, - isSaving: updateCredential.isPending, + isSaving, handleBackClick: guard.handleBackClick, showUnsavedAlert: guard.showUnsavedAlert, setShowUnsavedAlert: guard.setShowUnsavedAlert, diff --git a/apps/sim/app/workspace/[workspaceId]/integrations/connected/[credentialId]/connected-credential-detail.tsx b/apps/sim/app/workspace/[workspaceId]/integrations/connected/[credentialId]/connected-credential-detail.tsx index f4da0081688..08982ee5442 100644 --- a/apps/sim/app/workspace/[workspaceId]/integrations/connected/[credentialId]/connected-credential-detail.tsx +++ b/apps/sim/app/workspace/[workspaceId]/integrations/connected/[credentialId]/connected-credential-detail.tsx @@ -267,7 +267,7 @@ export function ConnectedCredentialDetail({ maxLength={500} autoComplete='off' data-lpignore='true' - disabled={!isAdmin} + viewOnly={!isAdmin} /> diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/secrets/components/secrets-manager/secrets-manager.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/secrets/components/secrets-manager/secrets-manager.tsx index 2ad13f3d873..8e200edf9f7 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/secrets/components/secrets-manager/secrets-manager.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/secrets/components/secrets-manager/secrets-manager.tsx @@ -419,12 +419,22 @@ export function SecretsManager() { return mapped.filter(({ envVar }) => envVar.key.toLowerCase().includes(term)) }, [envVars, searchTerm]) + /** + * Matches description as well as key, so a secret documented as "prod billing" + * is findable by that wording. The row has no description column, so a match on + * it is only legible on the secret's detail page. Personal secrets carry no + * shared description and stay key-only. + */ const filteredWorkspaceEntries = useMemo(() => { const entries = Object.entries(workspaceVars) if (!searchTerm.trim()) return entries const term = searchTerm.toLowerCase() - return entries.filter(([key]) => key.toLowerCase().includes(term)) - }, [workspaceVars, searchTerm]) + return entries.filter( + ([key]) => + key.toLowerCase().includes(term) || + Boolean(workspaceEnvKeyToCredential.get(key)?.description?.toLowerCase().includes(term)) + ) + }, [workspaceVars, searchTerm, workspaceEnvKeyToCredential]) const filteredNewWorkspaceRows = useMemo(() => { const mapped = newWorkspaceRows.map((row, index) => ({ row, originalIndex: index })) diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/secrets/hooks/use-secret-value.ts b/apps/sim/app/workspace/[workspaceId]/settings/components/secrets/hooks/use-secret-value.ts index 8e246fb6de4..824ba4bb13c 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/secrets/hooks/use-secret-value.ts +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/secrets/hooks/use-secret-value.ts @@ -69,8 +69,9 @@ export function useSecretValue({ workspaceId, credential }: UseSecretValueParams const isDirty = draft !== currentValue const isSaving = savePersonal.isPending || upsertWorkspace.isPending - const save = async () => { - if (!credential || !canEdit || isConflicted || !isDirty || isSaving) return + /** Resolves false when the write failed, so a combined save can stop before its next step. */ + const save = async (): Promise => { + if (!credential || !canEdit || isConflicted || !isDirty || isSaving) return true try { if (isPersonal) { const { data: latest } = await refetchPersonal() @@ -79,7 +80,7 @@ export function useSecretValue({ workspaceId, credential }: UseSecretValueParams description: 'Could not load your latest secrets. Please try again in a moment.', }) logger.warn('Aborted personal secret save: latest environment unavailable') - return + return false } const merged: Record = Object.fromEntries( Object.entries(latest).map(([key, entry]) => [key, entry.value]) @@ -89,11 +90,13 @@ export function useSecretValue({ workspaceId, credential }: UseSecretValueParams } else { await upsertWorkspace.mutateAsync({ workspaceId, variables: { [envKey]: draft } }) } + return true } catch (error) { toast.error("Couldn't save value", { description: getErrorMessage(error, 'Please try again in a moment.'), }) logger.error('Failed to save secret value', error) + return false } } diff --git a/apps/sim/app/workspace/[workspaceId]/settings/secrets/[credentialId]/secret-detail.tsx b/apps/sim/app/workspace/[workspaceId]/settings/secrets/[credentialId]/secret-detail.tsx index 985b35649fa..a1c62aff44c 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/secrets/[credentialId]/secret-detail.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/secrets/[credentialId]/secret-detail.tsx @@ -1,8 +1,8 @@ 'use client' import { useState } from 'react' -import { Chip, ChipCopyInput, ChipLink, Send } from '@sim/emcn' -import { ArrowLeft, Key } from '@sim/emcn/icons' +import { Chip, ChipCopyInput, ChipLink, ChipTextarea } from '@sim/emcn' +import { ArrowLeft, Key, Send } from '@sim/emcn/icons' import { SaveDiscardChips } from '@/components/settings/save-discard-actions' import { ResourceTile } from '@/app/workspace/[workspaceId]/components' import { @@ -12,7 +12,7 @@ import { CredentialMembersSection, DetailSection, UnsavedChangesModal, - useUnsavedChangesGuard, + useCredentialDetailForm, } from '@/app/workspace/[workspaceId]/components/credential-detail' import { SecretValueField } from '@/app/workspace/[workspaceId]/settings/components/secrets/components/secret-value-field' import { useSecretValue } from '@/app/workspace/[workspaceId]/settings/components/secrets/hooks/use-secret-value' @@ -34,32 +34,44 @@ export function SecretDetail({ workspaceId, credentialId }: SecretDetailProps) { const [isShareModalOpen, setIsShareModalOpen] = useState(false) const valueField = useSecretValue({ workspaceId, credential }) - const guard = useUnsavedChangesGuard({ isDirty: valueField.isDirty, backHref: secretsHref }) + + const form = useCredentialDetailForm({ + credential, + isAdmin, + backHref: secretsHref, + section: valueField, + }) const back = ( - + Secrets ) const canEditValue = valueField.canEdit && !valueField.isConflicted + /** + * Gates workspace-secret administration — Share and Description alike. + * Description is workspace-only because `env_personal` credentials are + * per-workspace mirrors of one user-global secret, so one saved here would + * exist in this workspace alone — and a personal secret has no teammates to + * inform. + */ + const isWorkspaceSecretAdmin = isAdmin && !isPersonal const actions = - credential && ((isAdmin && !isPersonal) || canEditValue) ? ( + credential && (isWorkspaceSecretAdmin || canEditValue) ? ( <> - {isAdmin && !isPersonal && ( + {isWorkspaceSecretAdmin && ( setIsShareModalOpen(true)}> Share )} - {canEditValue && ( - - )} + ) : null @@ -109,6 +121,22 @@ export function SecretDetail({ workspaceId, credentialId }: SecretDetailProps) { /> + {!isPersonal && ( + + form.setDescriptionDraft(event.target.value)} + placeholder='Add a description...' + maxLength={500} + autoComplete='off' + data-lpignore='true' + viewOnly={!isWorkspaceSecretAdmin} + /> + + )} + {!isPersonal && } @@ -121,9 +149,9 @@ export function SecretDetail({ workspaceId, credentialId }: SecretDetailProps) { )} ) diff --git a/apps/sim/lib/api/contracts/v2/openapi/resources.ts b/apps/sim/lib/api/contracts/v2/openapi/resources.ts index 23db1528093..3c0a0cc3d3f 100644 --- a/apps/sim/lib/api/contracts/v2/openapi/resources.ts +++ b/apps/sim/lib/api/contracts/v2/openapi/resources.ts @@ -232,6 +232,7 @@ const CREDENTIAL_CONNECTION_EXAMPLE = { const SECRET_EXAMPLE = { name: 'STRIPE_API_KEY', scope: 'workspace', + description: 'Production billing key — rotate quarterly.', role: 'admin', createdAt: '2026-06-01T09:14:00.000Z', updatedAt: '2026-06-20T14:02:11.000Z', diff --git a/apps/sim/lib/api/contracts/v2/secrets.ts b/apps/sim/lib/api/contracts/v2/secrets.ts index 067bdc81ff5..c797875a1e1 100644 --- a/apps/sim/lib/api/contracts/v2/secrets.ts +++ b/apps/sim/lib/api/contracts/v2/secrets.ts @@ -33,6 +33,12 @@ export const v2SecretSchema = z .object({ name: v2SecretNameSchema, scope: v2SecretScopeSchema, + description: z + .string() + .nullable() + .describe( + 'What the secret is for, as set on the workspace secret. Always null for a personal secret, which has no shared audience.' + ), role: workspaceCredentialRoleSchema.describe('Caller role for the secret.'), createdAt: v2TimestampSchema.describe('ISO 8601 timestamp when the secret was created.'), updatedAt: v2TimestampSchema.describe('ISO 8601 timestamp when the secret was last updated.'), @@ -88,8 +94,25 @@ export const v2SetSecretBodySchema = z .max(65_536, 'value is too long') .describe('Write-only secret value. It is never returned.') .meta({ writeOnly: true }), + description: z + .string() + .trim() + .max(500, 'description must be at most 500 characters') + .nullish() + .describe( + 'What the secret is for, shown to teammates. Omit to leave an existing description untouched; pass null to clear it. Workspace scope only.' + ), }) .strict() + .superRefine((data, ctx) => { + if (data.scope === 'personal' && data.description !== undefined) { + ctx.addIssue({ + code: 'custom', + path: ['description'], + message: 'description is only supported for a workspace secret', + }) + } + }) export type V2SetSecretBody = z.input export const v2DeleteSecretQuerySchema = z diff --git a/apps/sim/lib/credentials/secret-values.ts b/apps/sim/lib/credentials/secret-values.ts index 197f1e127f8..e5ab714adaa 100644 --- a/apps/sim/lib/credentials/secret-values.ts +++ b/apps/sim/lib/credentials/secret-values.ts @@ -32,8 +32,14 @@ export async function setWorkspaceSecret(params: { name: string value: string userId: string + /** + * Teammate-facing note on the credential row. `undefined` leaves any existing + * description untouched so rotating a value can't silently erase it; `null` + * clears it. + */ + description?: string | null }): Promise { - const { workspaceId, name, value, userId } = params + const { workspaceId, name, value, userId, description } = params const { encrypted } = await encryptSecret(value) const updatedAt = new Date() @@ -76,7 +82,7 @@ export async function setWorkspaceSecret(params: { }) await tx .update(credential) - .set({ updatedAt }) + .set(description === undefined ? { updatedAt } : { updatedAt, description }) .where( and( eq(credential.workspaceId, workspaceId), diff --git a/apps/sim/lib/secrets/application/use-cases.ts b/apps/sim/lib/secrets/application/use-cases.ts index 1cd0daadd80..680f6042a4d 100644 --- a/apps/sim/lib/secrets/application/use-cases.ts +++ b/apps/sim/lib/secrets/application/use-cases.ts @@ -250,6 +250,12 @@ export interface SetSecretInput { name: string scope: SecretScope value: string + /** + * Workspace scope only, and rejected at the contract for personal scope: an + * `env_personal` row is a per-workspace mirror of one user-global secret, so a + * description written here would exist in this workspace alone. + */ + description?: string | null } export const setSecretUseCase = defineAuthorizedWorkspaceUseCase({ @@ -273,6 +279,7 @@ export const setSecretUseCase = defineAuthorizedWorkspaceUseCase({ name: input.name, value: input.value, userId, + description: input.description, }) const secret = await getWorkspaceSecretMetadata({ workspaceId: context.workspaceId, @@ -297,7 +304,11 @@ export const setSecretUseCase = defineAuthorizedWorkspaceUseCase({ resourceId: `${input.scope}:${input.name}`, resourceName: input.name, description: `Set ${input.scope} secret "${input.name}"`, - metadata: { scope: input.scope, name: input.name }, + metadata: { + scope: input.scope, + name: input.name, + ...(input.description !== undefined ? { descriptionUpdated: true } : {}), + }, }), }) diff --git a/packages/sim-cli/src/commands/secrets.ts b/packages/sim-cli/src/commands/secrets.ts index 5c24291a3ee..73bfa41b995 100644 --- a/packages/sim-cli/src/commands/secrets.ts +++ b/packages/sim-cli/src/commands/secrets.ts @@ -14,13 +14,17 @@ const SECRET_RESULT: CommandSpec = { { header: 'name' }, { header: 'scope' }, { header: 'role' }, + { header: 'description' }, { header: 'updated', path: 'updatedAt', format: 'timestamp' }, ], } +const MAX_DESCRIPTION_LENGTH = 500 + interface SetSecretOptions { scope: (typeof SECRET_SCOPES)[number] value?: string + description?: string } function validateSecretValue(value: string): string { @@ -31,7 +35,27 @@ function validateSecretValue(value: string): string { return value } +/** + * A description belongs to the workspace secret teammates share; a personal + * secret has none, and the API rejects one. Failing here names the flag rather + * than surfacing a validation error against the request body. + */ +function validateDescription(options: SetSecretOptions): string | undefined { + if (options.description === undefined) return undefined + if (options.scope === 'personal') { + throw new SimApiError('--description is only supported for a workspace secret.', 0) + } + if (options.description.length > MAX_DESCRIPTION_LENGTH) { + throw new SimApiError( + `Secret description cannot exceed ${MAX_DESCRIPTION_LENGTH} characters.`, + 0 + ) + } + return options.description +} + async function setSecret(name: string, options: SetSecretOptions, command: Command): Promise { + const description = validateDescription(options) const value = validateSecretValue(options.value ?? (await promptSecret())) const { client, profile } = clientFrom(command) const operation = V2_OPERATIONS.setSecret @@ -41,6 +65,7 @@ async function setSecret(name: string, options: SetSecretOptions, command: Comma workspaceId: client.requireWorkspace(), scope: options.scope, value, + ...(description !== undefined ? { description } : {}), }, }) @@ -62,6 +87,10 @@ export function attachSecretCommands(program: Command): void { .makeOptionMandatory() ) .option('--value ', 'Secret value; visible to shell history when supplied directly') + .option( + '--description ', + 'What the secret is for, shown to teammates; workspace scope only. Omit to leave an existing description unchanged' + ) .action((name: string, options: SetSecretOptions, command: Command) => setSecret(name, options, command) ) diff --git a/packages/sim-cli/src/contract/commands.ts b/packages/sim-cli/src/contract/commands.ts index 66622e9d38a..885cef60582 100644 --- a/packages/sim-cli/src/contract/commands.ts +++ b/packages/sim-cli/src/contract/commands.ts @@ -460,6 +460,7 @@ export const CLI_CONTRACT: CliContract = { { header: 'name' }, { header: 'scope' }, { header: 'role' }, + { header: 'description' }, { header: 'updated', path: 'updatedAt', format: 'timestamp' }, ], }, diff --git a/packages/sim-cli/src/generated/v2-api.ts b/packages/sim-cli/src/generated/v2-api.ts index cac9ffe9f72..920ba5c7b78 100644 --- a/packages/sim-cli/src/generated/v2-api.ts +++ b/packages/sim-cli/src/generated/v2-api.ts @@ -3794,6 +3794,7 @@ export type ListSecretsQuery = { type ListSecretsResponseRef0 = { name: string scope: 'workspace' | 'personal' + description: string | null role: 'admin' | 'member' createdAt: string updatedAt: string @@ -4790,11 +4791,13 @@ export type SetSecretBody = { workspaceId: string scope: 'workspace' | 'personal' value: string + description?: string | null } type SetSecretResponseRef0 = { name: string scope: 'workspace' | 'personal' + description: string | null role: 'admin' | 'member' createdAt: string updatedAt: string @@ -8535,6 +8538,11 @@ export const V2_OPERATIONS = { required: true, describe: 'Write-only secret value. It is never returned.', }, + description: { + kind: 'string', + describe: + 'What the secret is for, shown to teammates. Omit to leave an existing description untouched; pass null to clear it. Workspace scope only.', + }, }, }, tableExportDownload: { From c3ff520165591245d540e80b63703e97b85207d4 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 17 Aug 2026 17:25:24 -0700 Subject: [PATCH 2/3] fix(secrets): address review findings on secret descriptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Patch the credential detail cache optimistically on update. `onMutate` cancelled the detail query but only patched the lists, so a detail-backed editor stayed dirty after a successful save until the refetch landed — long enough for Discard to restore the pre-save value over the committed one, and for Back to open the unsaved-changes guard. - Memoize `useSecretValue`'s returned callbacks and object, per the hook convention, so the composed form's save/discard stop churning per render. - Reject a description on a personal secret in the domain layer rather than only at the v2 boundary. The internal credential update path accepted one for any type, writing data every reader hides. - Normalize an empty description to null so the API and UI agree. - Correct the secrets documentation, which described a Display Name field the detail view does not have and omitted the scope rule. - Drop the CLI's copy of the 500-character bound; it can't import the contract, so a copy only drifts from the message the API already returns. - Collapse a redundant save guard and align the description write gate with the render gate. Leaves the integrations credential page byte-identical to staging. --- .../content/docs/en/platform/credentials.mdx | 5 +- apps/docs/openapi-v2-resources.json | 11 +-- .../app/api/v2/secrets/[name]/route.test.ts | 14 +++ .../hooks/use-credential-detail-form.ts | 11 ++- .../connected-credential-detail.tsx | 2 +- .../secrets-manager/secrets-manager.tsx | 7 +- .../secrets/hooks/use-secret-value.ts | 47 ++++++--- .../secrets/[credentialId]/secret-detail.tsx | 18 ++-- apps/sim/hooks/queries/credentials.test.ts | 98 +++++++++++++++++++ apps/sim/hooks/queries/credentials.ts | 48 ++++++--- apps/sim/lib/api/contracts/v2/secrets.ts | 3 +- .../credentials/orchestration/index.test.ts | 36 +++++++ .../lib/credentials/orchestration/index.ts | 6 +- .../lib/secrets/application/use-cases.test.ts | 32 ++++++ apps/sim/lib/secrets/application/use-cases.ts | 6 ++ packages/sim-cli/src/commands/secrets.ts | 27 +++-- packages/sim-cli/src/generated/v2-api.ts | 2 +- 17 files changed, 297 insertions(+), 76 deletions(-) create mode 100644 apps/sim/hooks/queries/credentials.test.ts diff --git a/apps/docs/content/docs/en/platform/credentials.mdx b/apps/docs/content/docs/en/platform/credentials.mdx index ef1425bd3c8..1f8c7017250 100644 --- a/apps/docs/content/docs/en/platform/credentials.mdx +++ b/apps/docs/content/docs/en/platform/credentials.mdx @@ -95,14 +95,15 @@ Click **Details** on any secret row to open its detail view. Secret details view showing Display Name, Description, and Members sections From here you can: -- Edit the **Display Name** and **Description** +- View the **Key** and edit the **Value** +- Edit the **Description** — an optional note telling teammates what the secret is for. Workspace secrets only; a personal secret is not shared, so it has none - Manage **Members** — invite teammates by email and assign them an **Admin** or **Member** role Click **Save** to apply changes, or **Back** to return to the list. diff --git a/apps/docs/openapi-v2-resources.json b/apps/docs/openapi-v2-resources.json index 68a997a9f83..d75d0e45559 100644 --- a/apps/docs/openapi-v2-resources.json +++ b/apps/docs/openapi-v2-resources.json @@ -5175,7 +5175,7 @@ "writeOnly": true }, "description": { - "description": "What the secret is for, shown to teammates. Omit to leave an existing description untouched; pass null to clear it. Workspace scope only.", + "description": "What the secret is for, shown to teammates. Omit to leave an existing description untouched; pass null or an empty string to clear it. Workspace scope only.", "anyOf": [ { "type": "string", @@ -5190,14 +5190,7 @@ "required": ["workspaceId", "scope", "value"], "additionalProperties": false, "title": "Set secret request", - "description": "Ownership scope and write-only value for the secret.", - "examples": [ - { - "workspaceId": "a91c4b2e-6d3f-4e8a-b5c7-0d9e2f1a8c64", - "scope": "workspace", - "value": "YOUR_SECRET_VALUE" - } - ] + "description": "Ownership scope and write-only value for the secret." }, "V2SecretDeleteData": { "type": "object", diff --git a/apps/sim/app/api/v2/secrets/[name]/route.test.ts b/apps/sim/app/api/v2/secrets/[name]/route.test.ts index 7ed0a5a580c..2d47fb464de 100644 --- a/apps/sim/app/api/v2/secrets/[name]/route.test.ts +++ b/apps/sim/app/api/v2/secrets/[name]/route.test.ts @@ -169,6 +169,20 @@ describe('/api/v2/secrets/[name]', () => { expect(mocks.set.mock.calls[0][0].input).not.toHaveProperty('description') }) + it('normalizes an empty description to null so it matches the UI clear path', async () => { + await PUT( + request('PUT', { + workspaceId: WORKSPACE_ID, + scope: 'workspace', + value: 'secret-value', + description: ' ', + }), + context + ) + + expect(mocks.set.mock.calls[0][0].input.description).toBeNull() + }) + it('rejects a description on a personal secret rather than dropping it', async () => { const response = await PUT( request('PUT', { diff --git a/apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-credential-detail-form.ts b/apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-credential-detail-form.ts index c62b712f6f0..fc36c039bad 100644 --- a/apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-credential-detail-form.ts +++ b/apps/sim/app/workspace/[workspaceId]/components/credential-detail/hooks/use-credential-detail-form.ts @@ -16,7 +16,11 @@ const logger = createLogger('CredentialDetailForm') export interface CredentialDetailFormSection { isDirty: boolean isSaving: boolean - /** Resolves false when the write failed, which stops the metadata save from committing alone. */ + /** + * Resolves true when the caller may proceed — including when there was nothing + * to write. False only when a write was attempted and failed, which stops the + * metadata save from committing alone. + */ save: () => Promise discard: () => void } @@ -81,11 +85,8 @@ export function useCredentialDetailForm({ const save = useCallback(async () => { if (!credential || isSaving) return - const savesMetadata = isAdmin && isMetadataDirty - if (!savesMetadata && !isSectionDirty) return - if (isSectionDirty && !(await section?.save())) return - if (!savesMetadata) return + if (!isAdmin || !isMetadataDirty) return try { await updateCredential.mutateAsync({ diff --git a/apps/sim/app/workspace/[workspaceId]/integrations/connected/[credentialId]/connected-credential-detail.tsx b/apps/sim/app/workspace/[workspaceId]/integrations/connected/[credentialId]/connected-credential-detail.tsx index 08982ee5442..f4da0081688 100644 --- a/apps/sim/app/workspace/[workspaceId]/integrations/connected/[credentialId]/connected-credential-detail.tsx +++ b/apps/sim/app/workspace/[workspaceId]/integrations/connected/[credentialId]/connected-credential-detail.tsx @@ -267,7 +267,7 @@ export function ConnectedCredentialDetail({ maxLength={500} autoComplete='off' data-lpignore='true' - viewOnly={!isAdmin} + disabled={!isAdmin} /> diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/secrets/components/secrets-manager/secrets-manager.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/secrets/components/secrets-manager/secrets-manager.tsx index 8e200edf9f7..835ea5a57cb 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/secrets/components/secrets-manager/secrets-manager.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/secrets/components/secrets-manager/secrets-manager.tsx @@ -420,10 +420,9 @@ export function SecretsManager() { }, [envVars, searchTerm]) /** - * Matches description as well as key, so a secret documented as "prod billing" - * is findable by that wording. The row has no description column, so a match on - * it is only legible on the secret's detail page. Personal secrets carry no - * shared description and stay key-only. + * The row has no description column, so a description-only match is legible + * only on the secret's detail page. Personal secrets carry no shared + * description and stay key-only. */ const filteredWorkspaceEntries = useMemo(() => { const entries = Object.entries(workspaceVars) diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/secrets/hooks/use-secret-value.ts b/apps/sim/app/workspace/[workspaceId]/settings/components/secrets/hooks/use-secret-value.ts index 824ba4bb13c..4f841fd1bf5 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/secrets/hooks/use-secret-value.ts +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/secrets/hooks/use-secret-value.ts @@ -1,6 +1,6 @@ 'use client' -import { useState } from 'react' +import { useCallback, useMemo, useState } from 'react' import { toast } from '@sim/emcn' import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' @@ -69,8 +69,7 @@ export function useSecretValue({ workspaceId, credential }: UseSecretValueParams const isDirty = draft !== currentValue const isSaving = savePersonal.isPending || upsertWorkspace.isPending - /** Resolves false when the write failed, so a combined save can stop before its next step. */ - const save = async (): Promise => { + const save = useCallback(async (): Promise => { if (!credential || !canEdit || isConflicted || !isDirty || isSaving) return true try { if (isPersonal) { @@ -98,18 +97,40 @@ export function useSecretValue({ workspaceId, credential }: UseSecretValueParams logger.error('Failed to save secret value', error) return false } - } - - const discard = () => setDraft(currentValue) - - return { - value: draft, - setValue: setDraft, + }, [ + credential, canEdit, isConflicted, isDirty, - save, - discard, isSaving, - } + isPersonal, + envKey, + draft, + workspaceId, + refetchPersonal, + savePersonal.mutateAsync, + upsertWorkspace.mutateAsync, + ]) + + const discard = useCallback(() => setDraft(currentValue), [currentValue]) + + /** + * Memoized so the object itself is stable, not just its callbacks: consumers + * pass the whole value as one unit into {@link useCredentialDetailForm}'s + * `section`, where a fresh object each render would churn the combined + * save/discard identities regardless of the callbacks inside it. + */ + return useMemo( + () => ({ + value: draft, + setValue: setDraft, + canEdit, + isConflicted, + isDirty, + save, + discard, + isSaving, + }), + [draft, canEdit, isConflicted, isDirty, save, discard, isSaving] + ) } diff --git a/apps/sim/app/workspace/[workspaceId]/settings/secrets/[credentialId]/secret-detail.tsx b/apps/sim/app/workspace/[workspaceId]/settings/secrets/[credentialId]/secret-detail.tsx index a1c62aff44c..90649218644 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/secrets/[credentialId]/secret-detail.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/secrets/[credentialId]/secret-detail.tsx @@ -35,9 +35,17 @@ export function SecretDetail({ workspaceId, credentialId }: SecretDetailProps) { const valueField = useSecretValue({ workspaceId, credential }) + /** + * Description is workspace-only because `env_personal` credentials are + * per-workspace mirrors of one user-global secret, so one saved here would + * exist in this workspace alone — and a personal secret has no teammates to + * inform. Gates the write and the render alike, so the two cannot disagree. + */ + const isWorkspaceSecretAdmin = isAdmin && !isPersonal + const form = useCredentialDetailForm({ credential, - isAdmin, + isAdmin: isWorkspaceSecretAdmin, backHref: secretsHref, section: valueField, }) @@ -49,14 +57,6 @@ export function SecretDetail({ workspaceId, credentialId }: SecretDetailProps) { ) const canEditValue = valueField.canEdit && !valueField.isConflicted - /** - * Gates workspace-secret administration — Share and Description alike. - * Description is workspace-only because `env_personal` credentials are - * per-workspace mirrors of one user-global secret, so one saved here would - * exist in this workspace alone — and a personal secret has no teammates to - * inform. - */ - const isWorkspaceSecretAdmin = isAdmin && !isPersonal const actions = credential && (isWorkspaceSecretAdmin || canEditValue) ? ( diff --git a/apps/sim/hooks/queries/credentials.test.ts b/apps/sim/hooks/queries/credentials.test.ts new file mode 100644 index 00000000000..2afec62d3cf --- /dev/null +++ b/apps/sim/hooks/queries/credentials.test.ts @@ -0,0 +1,98 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { queryClient } = vi.hoisted(() => ({ + queryClient: { + cancelQueries: vi.fn().mockResolvedValue(undefined), + invalidateQueries: vi.fn().mockResolvedValue(undefined), + getQueryData: vi.fn(), + getQueriesData: vi.fn(() => []), + setQueryData: vi.fn(), + setQueriesData: vi.fn(), + }, +})) + +vi.mock('@tanstack/react-query', () => ({ + keepPreviousData: {}, + useQuery: vi.fn(), + useQueryClient: vi.fn(() => queryClient), + useMutation: vi.fn((options) => options), +})) + +vi.mock('@/lib/api/client/request', () => ({ requestJson: vi.fn() })) + +import { useUpdateWorkspaceCredential } from '@/hooks/queries/credentials' + +const CREDENTIAL_ID = 'cred-1' + +const existing = { + id: CREDENTIAL_ID, + workspaceId: 'workspace-1', + type: 'env_workspace' as const, + displayName: 'STRIPE_API_KEY', + description: 'old description', + providerId: null, + accountId: null, + envKey: 'STRIPE_API_KEY', + envOwnerUserId: null, + createdBy: 'user-1', + createdAt: '2026-01-01T00:00:00.000Z', + updatedAt: '2026-01-02T00:00:00.000Z', + role: 'admin' as const, +} + +/** Replays the detail-cache updater the mutation hands to `setQueryData`. */ +function detailAfterMutate(cached: typeof existing | null) { + const detailCall = queryClient.setQueryData.mock.calls.find( + ([key]) => Array.isArray(key) && key.includes('detail') + ) + const updater = detailCall?.[1] as (old: unknown) => unknown + return updater(cached) +} + +describe('useUpdateWorkspaceCredential optimistic detail cache', () => { + beforeEach(() => { + vi.clearAllMocks() + queryClient.getQueryData.mockReturnValue(existing) + queryClient.getQueriesData.mockReturnValue([]) + }) + + it('patches the detail cache so a detail-backed editor stops being dirty after save', async () => { + const mutation = useUpdateWorkspaceCredential() as any + await mutation.onMutate({ credentialId: CREDENTIAL_ID, description: 'new description' }) + + expect(detailAfterMutate(existing)).toMatchObject({ description: 'new description' }) + }) + + it('clears the detail description when the edit passes null', async () => { + const mutation = useUpdateWorkspaceCredential() as any + await mutation.onMutate({ credentialId: CREDENTIAL_ID, description: null }) + + expect(detailAfterMutate(existing)).toMatchObject({ description: null }) + }) + + it('leaves untouched fields alone when only displayName changes', async () => { + const mutation = useUpdateWorkspaceCredential() as any + await mutation.onMutate({ credentialId: CREDENTIAL_ID, displayName: 'RENAMED' }) + + expect(detailAfterMutate(existing)).toMatchObject({ + displayName: 'RENAMED', + description: 'old description', + }) + }) + + it('rolls the detail cache back when the update fails', async () => { + const mutation = useUpdateWorkspaceCredential() as any + const context = await mutation.onMutate({ + credentialId: CREDENTIAL_ID, + description: 'new description', + }) + queryClient.setQueryData.mockClear() + + mutation.onError(new Error('boom'), { credentialId: CREDENTIAL_ID }, context) + + expect(queryClient.setQueryData).toHaveBeenCalledWith( + expect.arrayContaining(['detail']), + existing + ) + }) +}) diff --git a/apps/sim/hooks/queries/credentials.ts b/apps/sim/hooks/queries/credentials.ts index 13a6e782422..e5cc2f98d2f 100644 --- a/apps/sim/hooks/queries/credentials.ts +++ b/apps/sim/hooks/queries/credentials.ts @@ -156,35 +156,53 @@ export function useUpdateWorkspaceCredential() { const previousLists = queryClient.getQueriesData({ queryKey: workspaceCredentialKeys.lists(), }) + const previousDetail = queryClient.getQueryData( + workspaceCredentialKeys.detail(variables.credentialId) + ) + + /** Applies the in-flight edit to one cached credential. */ + const withEdit = (cred: WorkspaceCredential): WorkspaceCredential => ({ + ...cred, + ...(variables.displayName !== undefined ? { displayName: variables.displayName } : {}), + ...(variables.description !== undefined + ? { description: variables.description ?? null } + : {}), + }) + + /* + * The detail cache is patched alongside the lists, not just cancelled: a + * detail-backed editor compares its drafts against this entry to decide + * whether it is dirty, so leaving it stale keeps the surface dirty after a + * successful save until the `onSettled` refetch lands — long enough for + * Discard to restore the pre-save value over the committed one. + */ + queryClient.setQueryData( + workspaceCredentialKeys.detail(variables.credentialId), + (old) => (old ? withEdit(old) : old) + ) queryClient.setQueriesData( { queryKey: workspaceCredentialKeys.lists() }, (old) => { if (!old) return old - return old.map((cred) => - cred.id === variables.credentialId - ? { - ...cred, - ...(variables.displayName !== undefined - ? { displayName: variables.displayName } - : {}), - ...(variables.description !== undefined - ? { description: variables.description ?? null } - : {}), - } - : cred - ) + return old.map((cred) => (cred.id === variables.credentialId ? withEdit(cred) : cred)) } ) - return { previousLists } + return { previousLists, previousDetail } }, - onError: (_err, _variables, context) => { + onError: (_err, variables, context) => { if (context?.previousLists) { for (const [queryKey, data] of context.previousLists) { queryClient.setQueryData(queryKey, data) } } + if (context?.previousDetail !== undefined) { + queryClient.setQueryData( + workspaceCredentialKeys.detail(variables.credentialId), + context.previousDetail + ) + } }, onSettled: (_data, _error, variables) => { queryClient.invalidateQueries({ diff --git a/apps/sim/lib/api/contracts/v2/secrets.ts b/apps/sim/lib/api/contracts/v2/secrets.ts index c797875a1e1..19a76fe6101 100644 --- a/apps/sim/lib/api/contracts/v2/secrets.ts +++ b/apps/sim/lib/api/contracts/v2/secrets.ts @@ -99,8 +99,9 @@ export const v2SetSecretBodySchema = z .trim() .max(500, 'description must be at most 500 characters') .nullish() + .transform((value) => (value === '' ? null : value)) .describe( - 'What the secret is for, shown to teammates. Omit to leave an existing description untouched; pass null to clear it. Workspace scope only.' + 'What the secret is for, shown to teammates. Omit to leave an existing description untouched; pass null or an empty string to clear it. Workspace scope only.' ), }) .strict() diff --git a/apps/sim/lib/credentials/orchestration/index.test.ts b/apps/sim/lib/credentials/orchestration/index.test.ts index 47064858db5..88be9f0395d 100644 --- a/apps/sim/lib/credentials/orchestration/index.test.ts +++ b/apps/sim/lib/credentials/orchestration/index.test.ts @@ -441,6 +441,42 @@ describe('performUpdateCredential — service-account secret rotation', () => { }) }) +describe('performUpdateCredential — description scope', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + mockIsClientCredentialAccountProviderId.mockReturnValue(false) + mockGetClientCredentialAccountDescriptor.mockReturnValue(undefined) + }) + + it('applies a description to a workspace secret', async () => { + mockCredential({ type: 'env_workspace', envKey: 'STRIPE_API_KEY', providerId: null }) + + const result = await performUpdateCredential({ + credentialId: 'cred-1', + userId: 'user-1', + description: 'Prod billing key', + }) + + expect(result.success).toBe(true) + expect(updatePayload().description).toBe('Prod billing key') + }) + + it('rejects a description on a personal secret instead of writing dead data', async () => { + mockCredential({ type: 'env_personal', envKey: 'MY_TEST_KEY', providerId: null }) + + const result = await performUpdateCredential({ + credentialId: 'cred-1', + userId: 'user-1', + description: 'invisible dead data', + }) + + // Dropping the only field leaves nothing to update, so the existing + // empty-update guard turns this into an explicit rejection. + expect(result).toMatchObject({ success: false, errorCode: 'validation' }) + }) +}) + describe('createServiceAccountCredential', () => { beforeEach(() => { vi.clearAllMocks() diff --git a/apps/sim/lib/credentials/orchestration/index.ts b/apps/sim/lib/credentials/orchestration/index.ts index 2ca86cb8b07..0cd930c4ad1 100644 --- a/apps/sim/lib/credentials/orchestration/index.ts +++ b/apps/sim/lib/credentials/orchestration/index.ts @@ -194,7 +194,11 @@ export async function updateCredentialRecord( ): Promise { try { const updates: Record = {} - if (params.description !== undefined) { + // A description is teammate-facing, so it is meaningless on `env_personal`: + // those rows are per-workspace mirrors of one user-global secret, and every + // reader already hides or nulls the field for them. Enforced here rather than + // at one adapter so no surface can write dead data behind the invariant. + if (params.description !== undefined && params.credential.type !== 'env_personal') { updates.description = params.description ?? null } if ( diff --git a/apps/sim/lib/secrets/application/use-cases.test.ts b/apps/sim/lib/secrets/application/use-cases.test.ts index c7b708c06aa..e9b5071d319 100644 --- a/apps/sim/lib/secrets/application/use-cases.test.ts +++ b/apps/sim/lib/secrets/application/use-cases.test.ts @@ -163,6 +163,38 @@ describe('secret application use cases', () => { expect(JSON.stringify(mocks.audit.mock.calls)).not.toContain('secret-value') }) + it('forwards a workspace description to the manager', async () => { + await setSecretUseCase.execute({ + principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' }, + input: { + workspaceId: workspace.workspaceId, + name: secret.envKey, + scope: 'workspace', + value: 'secret-value', + description: 'Prod billing key', + }, + }) + + expect(mocks.setWorkspace).toHaveBeenCalledWith( + expect.objectContaining({ description: 'Prod billing key' }) + ) + }) + + it('refuses a description on a personal secret in the use case, not just the contract', async () => { + await expect( + setSecretUseCase.execute({ + principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' }, + input: { + workspaceId: workspace.workspaceId, + name: secret.envKey, + scope: 'personal', + value: 'secret-value', + description: 'has no shared audience', + }, + }) + ).rejects.toThrow(/only supported for a workspace secret/) + }) + it('still fails a workspace write whose metadata never materialized', async () => { mocks.listCredentials.mockResolvedValue({ data: [], nextCursorKeys: null }) diff --git a/apps/sim/lib/secrets/application/use-cases.ts b/apps/sim/lib/secrets/application/use-cases.ts index 680f6042a4d..74f1242cb8f 100644 --- a/apps/sim/lib/secrets/application/use-cases.ts +++ b/apps/sim/lib/secrets/application/use-cases.ts @@ -265,6 +265,12 @@ export const setSecretUseCase = defineAuthorizedWorkspaceUseCase({ authorizationOptions, async execute({ principal, input, context }) { const userId = principalUserId(principal) + if (input.scope === 'personal' && input.description !== undefined) { + throw new OrchestrationError( + 'validation', + 'description is only supported for a workspace secret' + ) + } if (input.scope === 'workspace') { await requireWorkspaceSecretMutationAccess({ workspaceId: context.workspaceId, diff --git a/packages/sim-cli/src/commands/secrets.ts b/packages/sim-cli/src/commands/secrets.ts index 73bfa41b995..aae967d22d5 100644 --- a/packages/sim-cli/src/commands/secrets.ts +++ b/packages/sim-cli/src/commands/secrets.ts @@ -19,8 +19,6 @@ const SECRET_RESULT: CommandSpec = { ], } -const MAX_DESCRIPTION_LENGTH = 500 - interface SetSecretOptions { scope: (typeof SECRET_SCOPES)[number] value?: string @@ -38,24 +36,23 @@ function validateSecretValue(value: string): string { /** * A description belongs to the workspace secret teammates share; a personal * secret has none, and the API rejects one. Failing here names the flag rather - * than surfacing a validation error against the request body. + * than surfacing a validation error against the request body, and does so before + * the interactive value prompt. The length bound is left to the API, whose + * message already names the field — a copy here would silently drift from it. */ -function validateDescription(options: SetSecretOptions): string | undefined { - if (options.description === undefined) return undefined - if (options.scope === 'personal') { +function validateDescriptionScope( + description: string | undefined, + scope: SetSecretOptions['scope'] +): string | undefined { + if (description === undefined) return undefined + if (scope === 'personal') { throw new SimApiError('--description is only supported for a workspace secret.', 0) } - if (options.description.length > MAX_DESCRIPTION_LENGTH) { - throw new SimApiError( - `Secret description cannot exceed ${MAX_DESCRIPTION_LENGTH} characters.`, - 0 - ) - } - return options.description + return description } async function setSecret(name: string, options: SetSecretOptions, command: Command): Promise { - const description = validateDescription(options) + const description = validateDescriptionScope(options.description, options.scope) const value = validateSecretValue(options.value ?? (await promptSecret())) const { client, profile } = clientFrom(command) const operation = V2_OPERATIONS.setSecret @@ -65,7 +62,7 @@ async function setSecret(name: string, options: SetSecretOptions, command: Comma workspaceId: client.requireWorkspace(), scope: options.scope, value, - ...(description !== undefined ? { description } : {}), + description, }, }) diff --git a/packages/sim-cli/src/generated/v2-api.ts b/packages/sim-cli/src/generated/v2-api.ts index 920ba5c7b78..6f410188006 100644 --- a/packages/sim-cli/src/generated/v2-api.ts +++ b/packages/sim-cli/src/generated/v2-api.ts @@ -8541,7 +8541,7 @@ export const V2_OPERATIONS = { description: { kind: 'string', describe: - 'What the secret is for, shown to teammates. Omit to leave an existing description untouched; pass null to clear it. Workspace scope only.', + 'What the secret is for, shown to teammates. Omit to leave an existing description untouched; pass null or an empty string to clear it. Workspace scope only.', }, }, }, From 5d35f396e292d7c47d3bbb92c7097e19544e738c Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 17 Aug 2026 17:27:26 -0700 Subject: [PATCH 3/3] fix(secrets): keep the API docs example and CLI column order stable Backward-compatibility fixes for anyone who never sets a description. - Move the blank-to-null normalization out of the contract and into the route. A Zod `.transform()` on any property drops the whole request schema's OpenAPI examples, which had silently removed the Set Secret request example from the published docs. - Append the CLI `description` column instead of inserting it before `updated`. `--output text` is positional, so inserting would shift every field an existing script cuts. - Reject a description on a personal secret with a message that says so, rather than dropping the field and falling through to the generic "no updatable fields" error. --- apps/docs/openapi-v2-resources.json | 11 +++++++++-- apps/sim/app/api/v2/secrets/[name]/route.ts | 11 ++++++++++- apps/sim/lib/api/contracts/v2/secrets.ts | 3 +-- .../credentials/orchestration/index.test.ts | 3 +-- .../sim/lib/credentials/orchestration/index.ts | 18 ++++++++++++++---- packages/sim-cli/src/commands/secrets.ts | 2 +- packages/sim-cli/src/contract/commands.ts | 4 +++- packages/sim-cli/src/generated/v2-api.ts | 2 +- 8 files changed, 40 insertions(+), 14 deletions(-) diff --git a/apps/docs/openapi-v2-resources.json b/apps/docs/openapi-v2-resources.json index d75d0e45559..8caf0604abb 100644 --- a/apps/docs/openapi-v2-resources.json +++ b/apps/docs/openapi-v2-resources.json @@ -5175,7 +5175,7 @@ "writeOnly": true }, "description": { - "description": "What the secret is for, shown to teammates. Omit to leave an existing description untouched; pass null or an empty string to clear it. Workspace scope only.", + "description": "What the secret is for, shown to teammates. Workspace scope only — sending it for a personal secret is rejected. Omit it to leave an existing description untouched; send null or an empty string to clear one.", "anyOf": [ { "type": "string", @@ -5190,7 +5190,14 @@ "required": ["workspaceId", "scope", "value"], "additionalProperties": false, "title": "Set secret request", - "description": "Ownership scope and write-only value for the secret." + "description": "Ownership scope and write-only value for the secret.", + "examples": [ + { + "workspaceId": "a91c4b2e-6d3f-4e8a-b5c7-0d9e2f1a8c64", + "scope": "workspace", + "value": "YOUR_SECRET_VALUE" + } + ] }, "V2SecretDeleteData": { "type": "object", diff --git a/apps/sim/app/api/v2/secrets/[name]/route.ts b/apps/sim/app/api/v2/secrets/[name]/route.ts index f7c8db59e83..35cfd3d42f4 100644 --- a/apps/sim/app/api/v2/secrets/[name]/route.ts +++ b/apps/sim/app/api/v2/secrets/[name]/route.ts @@ -19,7 +19,16 @@ export const PUT = defineV2JsonRoute({ auth: v2ApiKeyAuth, rateLimit: v2RateLimits.publicApi, errorPolicy: v2OrchestrationErrorPolicy, - mapInput: ({ params, body }) => ({ ...body, name: params.name }), + /** + * Normalizes a blank description to an explicit clear here rather than in the + * contract: a Zod `.transform()` on any property drops the whole request + * schema's OpenAPI examples, silently removing them from the published docs. + */ + mapInput: ({ params, body }) => ({ + ...body, + name: params.name, + ...(body.description === '' ? { description: null } : {}), + }), useCase: setSecretUseCase, statusForResult: ({ created }) => (created ? 201 : 200), present: ({ secret, userId }) => ({ data: toV2Secret(secret, userId) }), diff --git a/apps/sim/lib/api/contracts/v2/secrets.ts b/apps/sim/lib/api/contracts/v2/secrets.ts index 19a76fe6101..5fc59908262 100644 --- a/apps/sim/lib/api/contracts/v2/secrets.ts +++ b/apps/sim/lib/api/contracts/v2/secrets.ts @@ -99,9 +99,8 @@ export const v2SetSecretBodySchema = z .trim() .max(500, 'description must be at most 500 characters') .nullish() - .transform((value) => (value === '' ? null : value)) .describe( - 'What the secret is for, shown to teammates. Omit to leave an existing description untouched; pass null or an empty string to clear it. Workspace scope only.' + 'What the secret is for, shown to teammates. Workspace scope only — sending it for a personal secret is rejected. Omit it to leave an existing description untouched; send null or an empty string to clear one.' ), }) .strict() diff --git a/apps/sim/lib/credentials/orchestration/index.test.ts b/apps/sim/lib/credentials/orchestration/index.test.ts index 88be9f0395d..e9519cc2206 100644 --- a/apps/sim/lib/credentials/orchestration/index.test.ts +++ b/apps/sim/lib/credentials/orchestration/index.test.ts @@ -471,9 +471,8 @@ describe('performUpdateCredential — description scope', () => { description: 'invisible dead data', }) - // Dropping the only field leaves nothing to update, so the existing - // empty-update guard turns this into an explicit rejection. expect(result).toMatchObject({ success: false, errorCode: 'validation' }) + expect(result.success ? '' : result.error).toMatch(/cannot have a description/) }) }) diff --git a/apps/sim/lib/credentials/orchestration/index.ts b/apps/sim/lib/credentials/orchestration/index.ts index 0cd930c4ad1..d31a670ffb4 100644 --- a/apps/sim/lib/credentials/orchestration/index.ts +++ b/apps/sim/lib/credentials/orchestration/index.ts @@ -193,12 +193,22 @@ export async function updateCredentialRecord( params: UpdateCredentialRecordParams ): Promise { try { - const updates: Record = {} // A description is teammate-facing, so it is meaningless on `env_personal`: // those rows are per-workspace mirrors of one user-global secret, and every - // reader already hides or nulls the field for them. Enforced here rather than - // at one adapter so no surface can write dead data behind the invariant. - if (params.description !== undefined && params.credential.type !== 'env_personal') { + // reader already hides or nulls the field for them. Rejected here rather than + // at one adapter, so no surface can write data every reader hides — and said + // plainly, since dropping the field would fall through to the generic + // "no updatable fields" error and explain nothing. + if (params.description !== undefined && params.credential.type === 'env_personal') { + return { + success: false, + error: 'A personal secret cannot have a description; it is not shared with teammates.', + errorCode: 'validation', + } + } + + const updates: Record = {} + if (params.description !== undefined) { updates.description = params.description ?? null } if ( diff --git a/packages/sim-cli/src/commands/secrets.ts b/packages/sim-cli/src/commands/secrets.ts index aae967d22d5..6b864a8cc0c 100644 --- a/packages/sim-cli/src/commands/secrets.ts +++ b/packages/sim-cli/src/commands/secrets.ts @@ -14,8 +14,8 @@ const SECRET_RESULT: CommandSpec = { { header: 'name' }, { header: 'scope' }, { header: 'role' }, - { header: 'description' }, { header: 'updated', path: 'updatedAt', format: 'timestamp' }, + { header: 'description' }, ], } diff --git a/packages/sim-cli/src/contract/commands.ts b/packages/sim-cli/src/contract/commands.ts index 885cef60582..f32146cfeb2 100644 --- a/packages/sim-cli/src/contract/commands.ts +++ b/packages/sim-cli/src/contract/commands.ts @@ -456,12 +456,14 @@ export const CLI_CONTRACT: CliContract = { ], }, listSecrets: { + // `description` trails the existing columns: `--output text` is positional, + // so inserting ahead of `updated` would shift every field a script already cuts. columns: [ { header: 'name' }, { header: 'scope' }, { header: 'role' }, - { header: 'description' }, { header: 'updated', path: 'updatedAt', format: 'timestamp' }, + { header: 'description' }, ], }, getWorkspace: { diff --git a/packages/sim-cli/src/generated/v2-api.ts b/packages/sim-cli/src/generated/v2-api.ts index 6f410188006..2cae5e43b6d 100644 --- a/packages/sim-cli/src/generated/v2-api.ts +++ b/packages/sim-cli/src/generated/v2-api.ts @@ -8541,7 +8541,7 @@ export const V2_OPERATIONS = { description: { kind: 'string', describe: - 'What the secret is for, shown to teammates. Omit to leave an existing description untouched; pass null or an empty string to clear it. Workspace scope only.', + 'What the secret is for, shown to teammates. Workspace scope only — sending it for a personal secret is rejected. Omit it to leave an existing description untouched; send null or an empty string to clear one.', }, }, },