Skip to content

Commit 2ab3211

Browse files
waleedlatif1waleedlatif1
authored
fix(mothership): show the copy control for API keys created in organization chat (#8861)
* fix(mothership): keep the workspace target on revealed workspace API keys in organization chat * fix(mothership): bind a revealed key to the workspace it was created in * test(mothership): exercise the copy action in the organization chat key regression * fix(mothership): render a saved masked key without a workspace target check
1 parent 1266679 commit 2ab3211

4 files changed

Lines changed: 197 additions & 48 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.test.tsx‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,12 @@ vi.mock('@/lib/browser-agent/transport', () => ({
107107

108108
import { toast } from '@sim/emcn'
109109
import type { GenericSecretSource } from '@/lib/api/contracts/organization-secrets'
110+
import {
111+
captureRevealedSimKeys,
112+
type RevealedSimKeysByMessage,
113+
redactSensitiveContent,
114+
restoreRevealedSimKeysForMessage,
115+
} from '@/lib/mothership/chat/sim-key-redaction'
110116
import type { CredentialItemData } from '@/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags'
111117
import { SpecialTags } from '@/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags'
112118
import { organizationSecretKeys } from '@/hooks/queries/organization-secrets'
@@ -379,6 +385,64 @@ describe('CredentialDisplay link tag', () => {
379385
act(() => root.unmount())
380386
})
381387

388+
it('reveals a created key in organization chat after the live fill and after persistence', async () => {
389+
mockParams.mockReturnValue({ organizationId: 'org' } as never)
390+
const tag = (item: Record<string, string>) => `<credential>${JSON.stringify(item)}</credential>`
391+
const tagData = (content: string) =>
392+
JSON.parse(
393+
content.slice('<credential>'.length, -'</credential>'.length)
394+
) as CredentialItemData
395+
const cache: RevealedSimKeysByMessage = new Map()
396+
captureRevealedSimKeys(cache, ['msg-1'], tag({ type: 'sim_key' }), [
397+
{
398+
toolCall: {
399+
name: 'generate_api_key',
400+
result: { success: true, output: { key: 'sk-sim-live', workspaceId: 'ws-1' } },
401+
},
402+
},
403+
])
404+
const live = restoreRevealedSimKeysForMessage(
405+
{ id: 'msg-1', role: 'assistant', content: tag({ type: 'sim_key' }) },
406+
cache
407+
).content
408+
409+
const writeText = vi.fn(async () => undefined)
410+
Object.defineProperty(navigator, 'clipboard', { configurable: true, value: { writeText } })
411+
const copyButton = (container: HTMLElement) =>
412+
Array.from(container.querySelectorAll('button')).find(
413+
(button) => button.textContent === 'Copy to clipboard'
414+
)
415+
416+
const liveView = renderCredentialLink(tagData(live))
417+
await act(async () => copyButton(liveView.container)?.click())
418+
expect(writeText).toHaveBeenCalledWith('sk-sim-live')
419+
act(() => liveView.root.unmount())
420+
421+
const persisted = redactSensitiveContent(
422+
tag({ type: 'sim_key', workspaceId: 'ws-1', value: 'sk-sim-live' })
423+
)
424+
const persistedView = renderCredentialLink(tagData(persisted))
425+
expect(persistedView.container.querySelector('code')).not.toBeNull()
426+
expect(copyButton(persistedView.container)).toBeUndefined()
427+
expect(persistedView.container.textContent).not.toContain('sk-sim-live')
428+
act(() => persistedView.root.unmount())
429+
})
430+
431+
it('renders a saved key masked in workspace chat even when its tag names another workspace', () => {
432+
const persisted = redactSensitiveContent(
433+
`<credential>${JSON.stringify({ type: 'sim_key', workspaceId: 'other-workspace', value: 'sk-sim-saved' })}</credential>`
434+
)
435+
const { container, root } = renderCredentialLink(
436+
JSON.parse(
437+
persisted.slice('<credential>'.length, -'</credential>'.length)
438+
) as CredentialItemData
439+
)
440+
expect(container.querySelector('[role="status"]')).toBeNull()
441+
expect(container.querySelector('code')).not.toBeNull()
442+
expect(container.querySelector('button')).toBeNull()
443+
act(() => root.unmount())
444+
})
445+
382446
it('does not mount credential inputs when the target host denies access', () => {
383447
mockParams.mockReturnValue({ organizationId: 'org' } as never)
384448
mockCredentialHost.mockReturnValue(null)

‎apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3327,7 +3327,8 @@ export function CredentialDisplay(props: Parameters<typeof CredentialDisplayCont
33273327
(item) =>
33283328
item.type === 'link' ||
33293329
item.type === 'service_account' ||
3330-
item.type === 'sim_key' ||
3330+
// A masked key has nothing to copy, so only a revealed one needs its workspace.
3331+
(item.type === 'sim_key' && item.value !== undefined) ||
33313332
(item.type === 'secret_input' && (!item.scope || item.scope === 'workspace'))
33323333
)
33333334
const targets = new Set(targeted.map((item) => item.workspaceId).filter(Boolean))

‎apps/sim/lib/mothership/chat/sim-key-redaction.test.ts‎

Lines changed: 84 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import {
44
captureRevealedSimKeys,
55
extractRevealedSimKeys,
66
extractRevealedSimKeysFromBlocks,
7+
type RevealedSimKey,
8+
type RevealedSimKeysByMessage,
79
redactSensitiveContent,
810
restoreRevealedSimKeysForMessage,
911
toolResultForModel,
@@ -16,6 +18,8 @@ const redacted = `<credential>${JSON.stringify({ type: 'sim_key', redacted: true
1618
const placeholder = `<credential>${JSON.stringify({ type: 'sim_key' })}</credential>`
1719
const credentialBatch = (items: unknown[]) => `<credential>${JSON.stringify(items)}</credential>`
1820

21+
const keys = (...values: string[]): RevealedSimKey[] => values.map((value) => ({ value }))
22+
1923
const apiKeyBlock = (key: string) => ({
2024
type: 'tool_call' as const,
2125
toolCall: { name: 'generate_api_key', result: { success: true, output: { id: 'k1', key } } },
@@ -25,13 +29,13 @@ describe('sim-key-redaction', () => {
2529
describe('extractRevealedSimKeys', () => {
2630
it('returns sim_key values in document order', () => {
2731
const text = `first ${credential('sk-sim-A')} mid ${credential('sk-sim-B')}`
28-
expect(extractRevealedSimKeys(text)).toEqual(['sk-sim-A', 'sk-sim-B'])
32+
expect(extractRevealedSimKeys(text)).toEqual(keys('sk-sim-A', 'sk-sim-B'))
2933
})
3034

3135
it('skips redacted entries and non-sim_key tags', () => {
3236
const link = `<credential>${JSON.stringify({ value: 'https://x', type: 'link', provider: 'slack' })}</credential>`
3337
const text = `${link} ${credential('sk-sim-A')} ${redacted}`
34-
expect(extractRevealedSimKeys(text)).toEqual(['sk-sim-A'])
38+
expect(extractRevealedSimKeys(text)).toEqual(keys('sk-sim-A'))
3539
})
3640

3741
it('returns sim_key values from a mixed credential-card array', () => {
@@ -40,7 +44,7 @@ describe('sim-key-redaction', () => {
4044
{ type: 'sim_key', value: 'sk-sim-A' },
4145
{ type: 'sim_key', value: 'sk-sim-B' },
4246
])
43-
expect(extractRevealedSimKeys(batch)).toEqual(['sk-sim-A', 'sk-sim-B'])
47+
expect(extractRevealedSimKeys(batch)).toEqual(keys('sk-sim-A', 'sk-sim-B'))
4448
})
4549
})
4650

@@ -50,6 +54,27 @@ describe('sim-key-redaction', () => {
5054
const batch = credentialBatch([link, { type: 'sim_key', value: 'sk-sim-secret' }])
5155
expect(redactSensitiveContent(batch)).toBe(credentialBatch([link, { type: 'sim_key' }]))
5256
})
57+
58+
it('keeps the workspace target an organization chat needs and strips only the value', () => {
59+
const single = `<credential>${JSON.stringify({ type: 'sim_key', workspaceId: 'ws-1', value: 'sk-sim-secret' })}</credential>`
60+
expect(redactSensitiveContent(single)).toBe(
61+
`<credential>${JSON.stringify({ type: 'sim_key', workspaceId: 'ws-1' })}</credential>`
62+
)
63+
const batch = credentialBatch([
64+
{ type: 'sim_key', workspaceId: 'ws-1', value: 'sk-sim-secret', extra: 'dropped' },
65+
])
66+
expect(redactSensitiveContent(batch)).toBe(
67+
credentialBatch([{ type: 'sim_key', workspaceId: 'ws-1' }])
68+
)
69+
})
70+
71+
it.each([' ws-1', 'w'.repeat(257)])(
72+
'saves no workspace target the renderer would reject: %j',
73+
(workspaceId) => {
74+
const tag = `<credential>${JSON.stringify({ type: 'sim_key', workspaceId, value: 'sk-sim-secret' })}</credential>`
75+
expect(redactSensitiveContent(tag)).toBe(placeholder)
76+
}
77+
)
5378
})
5479

5580
describe('toolResultForModel', () => {
@@ -80,7 +105,7 @@ describe('sim-key-redaction', () => {
80105
it('pulls generate_api_key output keys in block order', () => {
81106
expect(
82107
extractRevealedSimKeysFromBlocks([apiKeyBlock('sk-sim-A'), apiKeyBlock('sk-sim-B')])
83-
).toEqual(['sk-sim-A', 'sk-sim-B'])
108+
).toEqual(keys('sk-sim-A', 'sk-sim-B'))
84109
})
85110

86111
it('skips redacted markers and unrelated tools', () => {
@@ -92,7 +117,7 @@ describe('sim-key-redaction', () => {
92117
},
93118
apiKeyBlock('sk-sim-A'),
94119
]
95-
expect(extractRevealedSimKeysFromBlocks(blocks)).toEqual(['sk-sim-A'])
120+
expect(extractRevealedSimKeysFromBlocks(blocks)).toEqual(keys('sk-sim-A'))
96121
})
97122

98123
it('returns nothing for empty/undefined block lists', () => {
@@ -103,55 +128,55 @@ describe('sim-key-redaction', () => {
103128

104129
describe('captureRevealedSimKeys', () => {
105130
it('records new keys under each provided key', () => {
106-
const cache = new Map<string, string[]>()
131+
const cache = new Map<string, RevealedSimKey[]>()
107132
captureRevealedSimKeys(cache, ['msg-1', 'req-1'], credential('sk-sim-A'))
108-
expect(cache.get('msg-1')).toEqual(['sk-sim-A'])
109-
expect(cache.get('req-1')).toEqual(['sk-sim-A'])
133+
expect(cache.get('msg-1')).toEqual(keys('sk-sim-A'))
134+
expect(cache.get('req-1')).toEqual(keys('sk-sim-A'))
110135
})
111136

112137
it('extends but never shrinks the captured list across calls', () => {
113-
const cache = new Map<string, string[]>()
138+
const cache = new Map<string, RevealedSimKey[]>()
114139
captureRevealedSimKeys(
115140
cache,
116141
['msg-1'],
117142
`${credential('sk-sim-A')} ${credential('sk-sim-B')}`
118143
)
119144
captureRevealedSimKeys(cache, ['msg-1'], credential('sk-sim-A'))
120-
expect(cache.get('msg-1')).toEqual(['sk-sim-A', 'sk-sim-B'])
145+
expect(cache.get('msg-1')).toEqual(keys('sk-sim-A', 'sk-sim-B'))
121146
})
122147

123148
it('skips undefined keys without throwing', () => {
124-
const cache = new Map<string, string[]>()
149+
const cache = new Map<string, RevealedSimKey[]>()
125150
captureRevealedSimKeys(cache, ['msg-1', undefined], credential('sk-sim-A'))
126-
expect(cache.get('msg-1')).toEqual(['sk-sim-A'])
151+
expect(cache.get('msg-1')).toEqual(keys('sk-sim-A'))
127152
expect(cache.size).toBe(1)
128153
})
129154

130155
it('ignores content with no credential tag', () => {
131-
const cache = new Map<string, string[]>()
156+
const cache = new Map<string, RevealedSimKey[]>()
132157
captureRevealedSimKeys(cache, ['msg-1'], 'plain assistant text')
133158
expect(cache.has('msg-1')).toBe(false)
134159
})
135160

136161
it('sources the key from the generate_api_key tool result (model text is a redacted placeholder)', () => {
137-
const cache = new Map<string, string[]>()
162+
const cache = new Map<string, RevealedSimKey[]>()
138163
captureRevealedSimKeys(cache, ['msg-1', 'req-1'], `Here is your key: ${redacted}`, [
139164
apiKeyBlock('sk-sim-fromtool'),
140165
])
141-
expect(cache.get('msg-1')).toEqual(['sk-sim-fromtool'])
142-
expect(cache.get('req-1')).toEqual(['sk-sim-fromtool'])
166+
expect(cache.get('msg-1')).toEqual(keys('sk-sim-fromtool'))
167+
expect(cache.get('req-1')).toEqual(keys('sk-sim-fromtool'))
143168
})
144169

145170
it('prefers tool-result keys over any inline content values', () => {
146-
const cache = new Map<string, string[]>()
171+
const cache = new Map<string, RevealedSimKey[]>()
147172
captureRevealedSimKeys(cache, ['msg-1'], credential('sk-content'), [apiKeyBlock('sk-tool')])
148-
expect(cache.get('msg-1')).toEqual(['sk-tool'])
173+
expect(cache.get('msg-1')).toEqual(keys('sk-tool'))
149174
})
150175
})
151176

152177
describe('restoreRevealedSimKeysForMessage', () => {
153178
it('substitutes the live key back into a redacted message', () => {
154-
const cache = new Map<string, string[]>([['msg-1', ['sk-sim-A']]])
179+
const cache = new Map<string, RevealedSimKey[]>([['msg-1', keys('sk-sim-A')]])
155180
const msg: ChatMessage = {
156181
id: 'msg-1',
157182
role: 'assistant',
@@ -165,7 +190,7 @@ describe('sim-key-redaction', () => {
165190
})
166191

167192
it('fills a value-less {"type":"sim_key"} placeholder (no redacted flag needed)', () => {
168-
const cache = new Map<string, string[]>([['msg-1', ['sk-sim-A']]])
193+
const cache = new Map<string, RevealedSimKey[]>([['msg-1', keys('sk-sim-A')]])
169194
const msg: ChatMessage = {
170195
id: 'msg-1',
171196
role: 'assistant',
@@ -178,7 +203,7 @@ describe('sim-key-redaction', () => {
178203
})
179204

180205
it('fills value-less and redacted placeholders positionally in one message', () => {
181-
const cache = new Map<string, string[]>([['msg-1', ['sk-sim-A', 'sk-sim-B']]])
206+
const cache = new Map<string, RevealedSimKey[]>([['msg-1', keys('sk-sim-A', 'sk-sim-B')]])
182207
const msg: ChatMessage = {
183208
id: 'msg-1',
184209
role: 'assistant',
@@ -193,7 +218,7 @@ describe('sim-key-redaction', () => {
193218
it('fills sim_key rows positionally inside a mixed credential-card array', () => {
194219
const link = { type: 'link', value: 'https://x', provider: 'slack' }
195220
const batch = credentialBatch([link, { type: 'sim_key' }, { type: 'sim_key' }])
196-
const cache = new Map<string, string[]>([['msg-1', ['sk-sim-A', 'sk-sim-B']]])
221+
const cache = new Map<string, RevealedSimKey[]>([['msg-1', keys('sk-sim-A', 'sk-sim-B')]])
197222
const msg: ChatMessage = { id: 'msg-1', role: 'assistant', content: batch }
198223

199224
expect(restoreRevealedSimKeysForMessage(msg, cache).content).toBe(
@@ -205,8 +230,37 @@ describe('sim-key-redaction', () => {
205230
)
206231
})
207232

233+
it('binds the filled key to the workspace it was created in, not the tag target', () => {
234+
const cache: RevealedSimKeysByMessage = new Map()
235+
const tag = `<credential>${JSON.stringify({ type: 'sim_key', workspaceId: 'ws-other' })}</credential>`
236+
captureRevealedSimKeys(cache, ['msg-1'], tag, [
237+
{
238+
toolCall: {
239+
name: 'generate_api_key',
240+
result: { success: true, output: { key: 'sk-sim-A', workspaceId: 'ws-1' } },
241+
},
242+
},
243+
])
244+
const msg: ChatMessage = { id: 'msg-1', role: 'assistant', content: tag }
245+
246+
expect(restoreRevealedSimKeysForMessage(msg, cache).content).toBe(
247+
`<credential>${JSON.stringify({ value: 'sk-sim-A', type: 'sim_key', workspaceId: 'ws-1' })}</credential>`
248+
)
249+
})
250+
251+
it('keeps the tag workspace when the creation result carries none', () => {
252+
const cache: RevealedSimKeysByMessage = new Map()
253+
const tag = `<credential>${JSON.stringify({ type: 'sim_key', workspaceId: 'ws-tag' })}</credential>`
254+
captureRevealedSimKeys(cache, ['msg-1'], tag, [apiKeyBlock('sk-sim-A')])
255+
const msg: ChatMessage = { id: 'msg-1', role: 'assistant', content: tag }
256+
257+
expect(restoreRevealedSimKeysForMessage(msg, cache).content).toBe(
258+
`<credential>${JSON.stringify({ value: 'sk-sim-A', type: 'sim_key', workspaceId: 'ws-tag' })}</credential>`
259+
)
260+
})
261+
208262
it('substitutes multiple keys in stream order', () => {
209-
const cache = new Map<string, string[]>([['msg-1', ['sk-sim-A', 'sk-sim-B']]])
263+
const cache = new Map<string, RevealedSimKey[]>([['msg-1', keys('sk-sim-A', 'sk-sim-B')]])
210264
const msg: ChatMessage = {
211265
id: 'msg-1',
212266
role: 'assistant',
@@ -219,7 +273,7 @@ describe('sim-key-redaction', () => {
219273
})
220274

221275
it('leaves a redacted tag in place if no live value is captured for that slot', () => {
222-
const cache = new Map<string, string[]>([['msg-1', ['sk-sim-A']]])
276+
const cache = new Map<string, RevealedSimKey[]>([['msg-1', keys('sk-sim-A')]])
223277
const msg: ChatMessage = {
224278
id: 'msg-1',
225279
role: 'assistant',
@@ -230,7 +284,7 @@ describe('sim-key-redaction', () => {
230284
})
231285

232286
it('returns the same message reference when nothing to restore', () => {
233-
const cache = new Map<string, string[]>()
287+
const cache = new Map<string, RevealedSimKey[]>()
234288
const msg: ChatMessage = {
235289
id: 'msg-1',
236290
role: 'assistant',
@@ -240,7 +294,7 @@ describe('sim-key-redaction', () => {
240294
})
241295

242296
it('does nothing for user messages', () => {
243-
const cache = new Map<string, string[]>([['msg-1', ['sk-sim-A']]])
297+
const cache = new Map<string, RevealedSimKey[]>([['msg-1', keys('sk-sim-A')]])
244298
const msg: ChatMessage = {
245299
id: 'msg-1',
246300
role: 'user',
@@ -250,7 +304,7 @@ describe('sim-key-redaction', () => {
250304
})
251305

252306
it('threads the cursor across separate content blocks so each block gets its matching key', () => {
253-
const cache = new Map<string, string[]>([['msg-1', ['sk-sim-A', 'sk-sim-B']]])
307+
const cache = new Map<string, RevealedSimKey[]>([['msg-1', keys('sk-sim-A', 'sk-sim-B')]])
254308
const msg: ChatMessage = {
255309
id: 'msg-1',
256310
role: 'assistant',
@@ -269,9 +323,9 @@ describe('sim-key-redaction', () => {
269323
})
270324

271325
it('isolates revealed values by message id (multiple keys across messages)', () => {
272-
const cache = new Map<string, string[]>([
273-
['msg-1', ['sk-sim-A']],
274-
['msg-2', ['sk-sim-B']],
326+
const cache = new Map<string, RevealedSimKey[]>([
327+
['msg-1', keys('sk-sim-A')],
328+
['msg-2', keys('sk-sim-B')],
275329
])
276330
const msg1: ChatMessage = { id: 'msg-1', role: 'assistant', content: redacted }
277331
const msg2: ChatMessage = { id: 'msg-2', role: 'assistant', content: redacted }

0 commit comments

Comments
 (0)