Skip to content

Commit 5396edb

Browse files
committed
fix(secrets): let a workspace secret change its metadata without resending the value
Restoring redaction cost more than removing it. The only way to flip a secret back to redacted was to re-send the plaintext, because the write required a value and omitting it fell into an interactive prompt that cannot run in CI. A workspace secret can now change its description or visibility on its own; the stored value is never re-encrypted or rewritten, a write that names no existing secret answers not-found rather than creating one, and a personal secret still requires a value because it has no other writable field. The path parameter was also one shared schema across the write and the delete, so a single description had to cover both and the delete documented an argument that could create and replace. Split, mirroring the credentials pair. The metadata write is a new update against the credentials table, so its scope is asserted by composition and by condition count: an unscoped update would let one workspace flip another workspace's identically-named secret out of redaction, and the cache invalidation would then carry that flag into the other workspace's runtime catalog.
1 parent ca2d0bb commit 5396edb

9 files changed

Lines changed: 544 additions & 23 deletions

File tree

apps/sim/app/api/v2/secrets/[name]/route.test.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,59 @@ describe('/api/v2/secrets/[name]', () => {
206206
expect(response.status).toBe(200)
207207
})
208208

209+
it('sends a value-less workspace write through as a metadata-only update at 200', async () => {
210+
mocks.set.mockResolvedValueOnce({ secret, userId: 'user-1', created: false })
211+
212+
const response = await PUT(
213+
request('PUT', { workspaceId: WORKSPACE_ID, scope: 'workspace', unredacted: false }),
214+
context
215+
)
216+
217+
expect(response.status).toBe(200)
218+
expect(mocks.set).toHaveBeenCalledWith({
219+
principal: PRINCIPAL,
220+
input: {
221+
workspaceId: WORKSPACE_ID,
222+
name: SECRET_NAME,
223+
scope: 'workspace',
224+
unredacted: false,
225+
},
226+
request: expect.anything(),
227+
})
228+
expect(mocks.set.mock.calls[0][0].input).not.toHaveProperty('value')
229+
})
230+
231+
it('answers 404 rather than creating when a metadata-only write names no secret', async () => {
232+
mocks.set.mockRejectedValueOnce(new OrchestrationError('not_found', 'Secret not found'))
233+
234+
const response = await PUT(
235+
request('PUT', { workspaceId: WORKSPACE_ID, scope: 'workspace', unredacted: true }),
236+
context
237+
)
238+
239+
expect(response.status).toBe(404)
240+
})
241+
242+
it('rejects a value-less personal write, which has no metadata field to update', async () => {
243+
const response = await PUT(
244+
request('PUT', { workspaceId: WORKSPACE_ID, scope: 'personal' }),
245+
context
246+
)
247+
248+
expect(response.status).toBe(400)
249+
expect(mocks.set).not.toHaveBeenCalled()
250+
})
251+
252+
it('rejects a workspace write carrying nothing to write', async () => {
253+
const response = await PUT(
254+
request('PUT', { workspaceId: WORKSPACE_ID, scope: 'workspace' }),
255+
context
256+
)
257+
258+
expect(response.status).toBe(400)
259+
expect(mocks.set).not.toHaveBeenCalled()
260+
})
261+
209262
it('deletes a secret through the semantic delete operation', async () => {
210263
const response = await DELETE(request('DELETE'), context)
211264

apps/sim/app/api/v2/secrets/[name]/route.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,10 @@ import { toV2Secret } from '@/app/api/v2/secrets/utils'
1212
export const dynamic = 'force-dynamic'
1313
export const revalidate = 0
1414

15-
/** PUT /api/v2/secrets/[name] — Create or replace a write-only secret value. */
15+
/**
16+
* PUT /api/v2/secrets/[name] — Create or replace a write-only secret value, or
17+
* update a workspace secret's metadata alone when the body carries no value.
18+
*/
1619
export const PUT = defineV2JsonRoute({
1720
contract: v2SetSecretContract,
1821
operation: secretOperations.set,

apps/sim/lib/api/contracts/v2/openapi/resources.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1368,11 +1368,14 @@ const declaredRoutes = [
13681368
resourceOperation('Secrets', {
13691369
operationId: 'setSecret',
13701370
summary: 'Set Secret',
1371-
description: `Create or replace a workspace or caller-owned personal secret. The value is encrypted at rest, is write-only, and is never included in the response. ${WORKSPACE_API_KEY_DENIED}`,
1371+
description: `Create or replace a workspace or caller-owned personal secret. The value is encrypted at rest, is write-only, and is never included in the response. Omit \`value\` on a workspace secret to update \`description\` and \`unredacted\` alone: the stored value is left untouched and is never re-encrypted, and because a metadata-only write cannot create a secret it answers \`404\` when the named secret does not exist. A personal secret always requires \`value\`, having no other writable field. ${WORKSPACE_API_KEY_DENIED}`,
13721372
errors: RESOURCE_ERRORS,
13731373
success: {
13741374
byStatus: {
1375-
200: { description: 'The existing secret value was replaced.' },
1375+
200: {
1376+
description:
1377+
'The existing secret value was replaced, or its metadata was updated in place.',
1378+
},
13761379
201: { description: 'The secret was created.' },
13771380
},
13781381
},
@@ -1389,13 +1392,18 @@ const declaredRoutes = [
13891392
v2SetSecretContract.body,
13901393
'SetSecretRequest',
13911394
'Set secret request',
1392-
'Ownership scope and write-only value for the secret.',
1395+
'Ownership scope and write-only value for the secret. A workspace secret may instead send description or unredacted alone, without a value.',
13931396
[
13941397
{
13951398
workspaceId: WORKSPACE_ID,
13961399
scope: SECRET_EXAMPLE.scope,
13971400
value: 'YOUR_SECRET_VALUE',
13981401
},
1402+
{
1403+
workspaceId: WORKSPACE_ID,
1404+
scope: 'workspace',
1405+
unredacted: false,
1406+
},
13991407
]
14001408
),
14011409
response: documentedSchema(

apps/sim/lib/api/contracts/v2/secrets.test.ts

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@
33
*/
44
import { describe, expect, it } from 'vitest'
55
import {
6+
v2DeleteSecretContract,
67
v2SecretSchema,
78
v2SecretWithValueSchema,
89
v2SetSecretBodySchema,
10+
v2SetSecretContract,
911
} from '@/lib/api/contracts/v2/secrets'
1012

1113
const secret = {
@@ -70,3 +72,95 @@ describe('v2SecretWithValueSchema value', () => {
7072
).toBe(true)
7173
})
7274
})
75+
76+
/** Reads the `name` field description off a contract's path-parameter schema. */
77+
function nameDescription(params: unknown): string | undefined {
78+
const shape = (params as { shape: Record<string, { description?: string }> }).shape
79+
return shape.name.description
80+
}
81+
82+
describe('secret path-parameter descriptions', () => {
83+
it('does not offer writes on the delete path parameter', () => {
84+
const description = nameDescription(v2DeleteSecretContract.params)
85+
86+
expect(description).toBe('Secret to delete.')
87+
expect(description).not.toMatch(/create|replace/i)
88+
})
89+
90+
it('does not offer deletion on the set path parameter', () => {
91+
expect(nameDescription(v2SetSecretContract.params)).not.toMatch(/delete/i)
92+
})
93+
94+
it('gives the two operations distinct path-parameter prose', () => {
95+
expect(nameDescription(v2SetSecretContract.params)).not.toBe(
96+
nameDescription(v2DeleteSecretContract.params)
97+
)
98+
})
99+
})
100+
101+
describe('v2SetSecretBodySchema metadata-only write', () => {
102+
it('accepts a workspace body carrying only unredacted, so restoring redaction costs no value', () => {
103+
const parsed = v2SetSecretBodySchema.safeParse({
104+
workspaceId: 'workspace-1',
105+
scope: 'workspace',
106+
unredacted: false,
107+
})
108+
109+
expect(parsed.success).toBe(true)
110+
if (parsed.success) expect(parsed.data.value).toBeUndefined()
111+
})
112+
113+
it('accepts a workspace body carrying only a description', () => {
114+
expect(
115+
v2SetSecretBodySchema.safeParse({
116+
workspaceId: 'workspace-1',
117+
scope: 'workspace',
118+
description: 'Prod billing key',
119+
}).success
120+
).toBe(true)
121+
})
122+
123+
it('rejects a workspace body with nothing to write rather than resolving to an empty update', () => {
124+
const parsed = v2SetSecretBodySchema.safeParse({
125+
workspaceId: 'workspace-1',
126+
scope: 'workspace',
127+
})
128+
129+
expect(parsed.success).toBe(false)
130+
if (!parsed.success) {
131+
expect(parsed.error.issues).toEqual([
132+
expect.objectContaining({
133+
path: ['value'],
134+
message: 'value, description, or unredacted is required',
135+
}),
136+
])
137+
}
138+
})
139+
140+
it('still requires a value for a personal secret, which has no metadata field to write', () => {
141+
const parsed = v2SetSecretBodySchema.safeParse({
142+
workspaceId: 'workspace-1',
143+
scope: 'personal',
144+
})
145+
146+
expect(parsed.success).toBe(false)
147+
if (!parsed.success) {
148+
expect(parsed.error.issues).toEqual([
149+
expect.objectContaining({
150+
path: ['value'],
151+
message: 'value is required for a personal secret',
152+
}),
153+
])
154+
}
155+
})
156+
157+
it('keeps rejecting an empty value, which is a write and not an omission', () => {
158+
expect(
159+
v2SetSecretBodySchema.safeParse({
160+
workspaceId: 'workspace-1',
161+
scope: 'workspace',
162+
value: '',
163+
}).success
164+
).toBe(false)
165+
})
166+
})

apps/sim/lib/api/contracts/v2/secrets.ts

Lines changed: 66 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,22 @@ export const v2ListSecretsQuerySchema = z
104104
.strict()
105105
export type V2ListSecretsQuery = z.output<typeof v2ListSecretsQuerySchema>
106106

107-
export const v2SecretParamsSchema = z.object({
108-
name: v2SecretNameSchema.describe('Secret to create, replace, or delete.'),
107+
/**
108+
* The secret a path addresses, named for what the route does to it.
109+
*
110+
* `PUT` and `DELETE` sit on the same path but are not the same operation, and the
111+
* OpenAPI document already publishes them as two components (`SetSecretParams`,
112+
* `DeleteSecretParams`). One shared `describe()` forced both to read "create,
113+
* replace, or delete", so `sim secrets delete` documented writes the route cannot
114+
* perform.
115+
*/
116+
export const v2SetSecretParamsSchema = z.object({
117+
name: v2SecretNameSchema.describe('Secret to create or replace.'),
118+
})
119+
120+
export const v2DeleteSecretParamsSchema = z.object({
121+
name: v2SecretNameSchema.describe('Secret to delete.'),
109122
})
110-
export type V2SecretParams = z.output<typeof v2SecretParamsSchema>
111123

112124
export const v2SetSecretBodySchema = z
113125
.object({
@@ -119,7 +131,10 @@ export const v2SetSecretBodySchema = z
119131
.string()
120132
.min(1, 'value is required')
121133
.max(65_536, 'value is too long')
122-
.describe('Write-only secret value. It is never returned.')
134+
.optional()
135+
.describe(
136+
'Write-only secret value. It is never returned. Omit it on a workspace secret to change description or unredacted alone, leaving the stored value untouched; the secret must already exist. Always required for a personal secret, which carries no other writable field.'
137+
)
123138
.meta({ writeOnly: true }),
124139
description: z
125140
.string()
@@ -137,19 +152,49 @@ export const v2SetSecretBodySchema = z
137152
),
138153
})
139154
.strict()
155+
/**
156+
* `value` is optional on the schema so a workspace secret's redaction policy can
157+
* be flipped back without re-transmitting the plaintext — restoring redaction is
158+
* the safe direction and must not cost more than leaving it off. Two refinements
159+
* keep that from over-relaxing the request: a personal secret has no metadata
160+
* field at all, so a value-less personal write would be a silent no-op rather
161+
* than an update; and a body carrying none of the three writable fields is
162+
* rejected outright instead of resolving to an empty write.
163+
*/
140164
.superRefine((data, ctx) => {
141-
if (data.scope === 'personal' && data.description !== undefined) {
142-
ctx.addIssue({
143-
code: 'custom',
144-
path: ['description'],
145-
message: 'description is only supported for a workspace secret',
146-
})
165+
if (data.scope === 'personal') {
166+
if (data.value === undefined) {
167+
ctx.addIssue({
168+
code: 'custom',
169+
path: ['value'],
170+
message: 'value is required for a personal secret',
171+
})
172+
}
173+
if (data.description !== undefined) {
174+
ctx.addIssue({
175+
code: 'custom',
176+
path: ['description'],
177+
message: 'description is only supported for a workspace secret',
178+
})
179+
}
180+
if (data.unredacted !== undefined) {
181+
ctx.addIssue({
182+
code: 'custom',
183+
path: ['unredacted'],
184+
message: 'unredacted is only supported for a workspace secret',
185+
})
186+
}
187+
return
147188
}
148-
if (data.scope === 'personal' && data.unredacted !== undefined) {
189+
if (
190+
data.value === undefined &&
191+
data.description === undefined &&
192+
data.unredacted === undefined
193+
) {
149194
ctx.addIssue({
150195
code: 'custom',
151-
path: ['unredacted'],
152-
message: 'unredacted is only supported for a workspace secret',
196+
path: ['value'],
197+
message: 'value, description, or unredacted is required',
153198
})
154199
}
155200
})
@@ -180,12 +225,17 @@ export const v2ListSecretsContract = defineRouteContract({
180225
},
181226
})
182227

183-
/** Creates or replaces a secret value without returning it. */
228+
/**
229+
* Creates or replaces a secret value without returning it, or — for a workspace
230+
* secret sent without a value — updates its description and redaction policy
231+
* alone. A value-less write never creates: it answers 404 when the secret is
232+
* absent.
233+
*/
184234
export const v2SetSecretContract = defineRouteContract({
185235
method: 'PUT',
186236
path: '/api/v2/secrets/[name]',
187237
query: noInputSchema,
188-
params: v2SecretParamsSchema,
238+
params: v2SetSecretParamsSchema,
189239
body: v2SetSecretBodySchema,
190240
response: {
191241
mode: 'json',
@@ -197,7 +247,7 @@ export const v2SetSecretContract = defineRouteContract({
197247
export const v2DeleteSecretContract = defineRouteContract({
198248
method: 'DELETE',
199249
path: '/api/v2/secrets/[name]',
200-
params: v2SecretParamsSchema,
250+
params: v2DeleteSecretParamsSchema,
201251
query: v2DeleteSecretQuerySchema,
202252
response: {
203253
mode: 'json',

0 commit comments

Comments
 (0)