Skip to content

Commit 27f0a83

Browse files
authored
fix(mothership): hold callerless runs' sim_cli to read API scopes (#9371)
* fix(mothership): hold callerless runs' sim_cli to read API scopes An unattributed inbox message runs as the workspace owner with its tool permission capped to read, but sim_cli has no catalog permission and its commands authorize the delegated subject's live role, so the CLI could create, run, and delete with the owner's authority. A run with no caller of its own now executes the CLI inside a delegated API-scope grant of api:read, enforced in requireOAuthOperationScope the same way an OAuth token's scopes are, so every authorization path the CLI reaches refuses api:write operations. * fix(mothership): refuse callerless service writes and organization-scoped CLI writes Mixed server tools reached through sim_cli services (settings, dashboards, tests, search_sources, workspaces) write through internal operations that declare no API scope, so the read grant never bound them. Each tool now declares which validated arguments write, and the router holds those to the current delegated grant. The grant also applies to callerless organization-scoped runs. * test(mothership): model attributed CLI runs with their secret actor
1 parent 4b8f3e4 commit 27f0a83

12 files changed

Lines changed: 252 additions & 28 deletions

File tree

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import { AsyncLocalStorage } from 'node:async_hooks'
2+
import type { OAuthApiScope } from '@/lib/auth/oauth-provider'
3+
4+
const grantedScopes = new AsyncLocalStorage<readonly OAuthApiScope[]>()
5+
6+
/**
7+
* Runs `execute` with every delegated principal it authorizes holding only `scopes`, the way an
8+
* OAuth token's grant bounds its user: `requireOAuthOperationScope` refuses an operation whose
9+
* declared scope they do not satisfy, whatever role the principal's subject holds.
10+
*/
11+
export function withDelegatedApiScopes<T>(scopes: readonly OAuthApiScope[], execute: () => T): T {
12+
return grantedScopes.run(Object.freeze([...scopes]), execute)
13+
}
14+
15+
/** The scopes the current invocation grants its delegated principals, or none when unbounded. */
16+
export function delegatedApiScopes(): readonly OAuthApiScope[] | undefined {
17+
return grantedScopes.getStore()
18+
}

‎apps/sim/lib/core/application/oauth-authorization.ts‎

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { Principal } from '@sim/auth/principal'
22
import { type OAuthApiScope, oauthScopeSatisfies } from '@/lib/auth/oauth-provider'
3+
import { delegatedApiScopes } from '@/lib/core/application/delegated-api-scopes'
34
import { ForbiddenOperationError } from '@/lib/core/application/forbidden'
45
import type { ApplicationOperation } from '@/lib/core/application/operation'
56
import { OrchestrationError } from '@/lib/core/orchestration/types'
@@ -18,17 +19,46 @@ export class OAuthAccessTokenExpiredError extends OrchestrationError {
1819
}
1920
}
2021

21-
/** Enforces the client's grant independently of workspace membership and HTTP method. */
22+
/**
23+
* Enforces the client's grant independently of workspace membership and HTTP method: an OAuth
24+
* token's scopes, or those {@link withDelegatedApiScopes} grants a delegated principal. A
25+
* delegated principal may still reach an operation that declares no scope, which is internal.
26+
*/
2227
export function requireOAuthOperationScope(
2328
principal: Principal,
2429
operation: ApplicationOperation
2530
): void {
26-
if (principal.kind !== 'oauth_access_token') return
27-
if (principal.expiresAt.getTime() <= Date.now()) throw new OAuthAccessTokenExpiredError()
28-
if (!operation.oauthScope) {
29-
throw new Error(`Operation ${operation.id} has no OAuth scope policy`)
31+
const granted = grantedApiScopes(principal)
32+
if (!granted) return
33+
if (principal.kind === 'oauth_access_token') {
34+
if (principal.expiresAt.getTime() <= Date.now()) throw new OAuthAccessTokenExpiredError()
35+
if (!operation.oauthScope) {
36+
throw new Error(`Operation ${operation.id} has no OAuth scope policy`)
37+
}
3038
}
31-
if (!oauthScopeSatisfies(principal.scopes, operation.oauthScope)) {
39+
if (operation.oauthScope && !oauthScopeSatisfies(granted, operation.oauthScope)) {
3240
throw new InsufficientScopeError(operation.oauthScope)
3341
}
3442
}
43+
44+
/**
45+
* Holds a change no scoped operation guards — an internal operation, or one authorized outside the
46+
* operation layer — to the current {@link withDelegatedApiScopes} grant. Outside a grant it allows.
47+
*/
48+
export function requireDelegatedApiScope(scope: OAuthApiScope): void {
49+
const granted = delegatedApiScopes()
50+
if (granted && !oauthScopeSatisfies(granted, scope)) throw new InsufficientScopeError(scope)
51+
}
52+
53+
function grantedApiScopes(principal: Principal): readonly string[] | undefined {
54+
switch (principal.kind) {
55+
case 'oauth_access_token':
56+
return principal.scopes
57+
case 'delegated':
58+
case 'resource_delegated':
59+
case 'organization_delegated':
60+
return delegatedApiScopes()
61+
default:
62+
return undefined
63+
}
64+
}

‎apps/sim/lib/mothership/agent-cli/index.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1+
import { OAUTH_API_READ_SCOPE } from '@/lib/auth/oauth-provider'
12
import { countWithheldContent } from '@/lib/core/application'
3+
import { withDelegatedApiScopes } from '@/lib/core/application/delegated-api-scopes'
24
import { executeFileCopyCliRequest } from '@/lib/mothership/agent-cli/file-copy'
35
import { isReadOnlyCliRequest } from '@/lib/mothership/agent-cli/read-only'
46
import { executeAgentCliService } from '@/lib/mothership/agent-cli/services'
@@ -10,6 +12,8 @@ import type { ServerToolContext } from '@/lib/mothership/tools/server/base-tool'
1012

1113
export interface AgentCliExecutionContext extends Omit<ServerToolContext, 'abortSignal'> {
1214
readOnly?: boolean
15+
/** Null when the run has no caller of its own, as for an unattributed inbox message. */
16+
secretActorUserId?: string | null
1317
signal?: AbortSignal
1418
}
1519

@@ -18,8 +22,12 @@ export async function executeAgentCliRequest(
1822
request: AgentCliRequest,
1923
context: AgentCliExecutionContext
2024
): Promise<AgentCliRawResult> {
25+
const execute = () => executeUncountedAgentCliRequest(request, context)
26+
// Commands authorize the subject's live role; a callerless inbox run's subject is the owner.
2127
const { result, withheld } = await countWithheldContent(() =>
22-
executeUncountedAgentCliRequest(request, context)
28+
context.secretActorUserId === null
29+
? withDelegatedApiScopes([OAUTH_API_READ_SCOPE], execute)
30+
: execute()
2331
)
2432
const note = describeWithheldContent(withheld)
2533
if (!note) return result

‎apps/sim/lib/mothership/agent-cli/project-file-read.integration.ts‎

Lines changed: 173 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
permissions,
88
user,
99
userStats,
10+
workflow,
1011
workspace,
1112
workspaceFileSearchBuild,
1213
workspaceFileSearchChunk,
@@ -21,7 +22,7 @@ import { setUploadDirServer, uploadsSetupMock } from '@sim/testing/mocks/uploads
2122
import { getErrorMessage } from '@sim/utils/errors'
2223
import { generateId } from '@sim/utils/id'
2324
import { Document, Packer, Paragraph } from 'docx'
24-
import { eq, inArray, or } from 'drizzle-orm'
25+
import { and, eq, inArray, or } from 'drizzle-orm'
2526
import sharp from 'sharp'
2627
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
2728

@@ -42,8 +43,13 @@ import { encryptSecret } from '@/lib/core/security/encryption'
4243
import { type AgentCliExecutionContext, executeAgentCliRequest } from '@/lib/mothership/agent-cli'
4344
import { createCopilotResourceAdmission } from '@/lib/mothership/auth/application-delegation'
4445
import { createRegistryContentGate } from '@/lib/mothership/content-delivery'
45-
import type { AgentCliAugmentationInvocation } from '@/lib/mothership/generated/agent-cli'
46+
import {
47+
type AgentCliAugmentationInvocation,
48+
AgentCliRawResult,
49+
} from '@/lib/mothership/generated/agent-cli'
4650
import { projectToolResultForCopilot } from '@/lib/mothership/request/tools/resolved-secret-result'
51+
import type { ToolExecutionContext } from '@/lib/mothership/tool-executor/types'
52+
import { executeSimCli } from '@/lib/mothership/tools/handlers/sim-cli'
4753
import { createProjectFile } from '@/lib/projects/files/application'
4854
import { storeCompiledDoc } from '@/lib/uploads/documents/compiled-store'
4955
import { fileDocumentInputIdentity } from '@/lib/uploads/documents/input-identity'
@@ -54,6 +60,7 @@ import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-tr
5460
const storageRoot = mkdtempSync(join(tmpdir(), 'sim-project-cli-reads-'))
5561
setUploadDirServer(storageRoot)
5662
const fixtures: { userId: string; workspaceId: string; projectId: string }[] = []
63+
const memberIds: string[] = []
5764
const checks: { name: string; status: 'passed' | 'failed'; durationMs: number; error?: string }[] =
5865
[]
5966

@@ -81,39 +88,32 @@ function check(name: string, run: () => Promise<void>, requiresRedis = false) {
8188
})
8289
}
8390

84-
async function fixture() {
91+
async function insertUser(name: string) {
8592
const userId = generateId()
86-
const workspaceId = generateId()
8793
await db.insert(user).values({
8894
id: userId,
8995
email: `${userId}@project-read.invalid`,
90-
name: 'File reader',
96+
name,
9197
emailVerified: true,
9298
createdAt: new Date(),
9399
updatedAt: new Date(),
94100
})
95101
await db.insert(userStats).values({ id: generateId(), userId })
96-
await insertWorkspaceFixture(db, {
97-
id: workspaceId,
98-
ownerId: userId,
99-
billedAccountUserId: userId,
100-
name: 'Read environment',
101-
})
102-
await db.insert(permissions).values({
102+
return userId
103+
}
104+
105+
function grantWorkspace(userId: string, workspaceId: string, permissionType: 'admin' | 'read') {
106+
return db.insert(permissions).values({
103107
id: generateId(),
104108
userId,
105109
entityType: 'workspace',
106110
entityId: workspaceId,
107-
permissionType: 'admin',
111+
permissionType,
108112
})
109-
const [binding] = await db
110-
.select({ projectId: workspace.projectId })
111-
.from(workspace)
112-
.where(eq(workspace.id, workspaceId))
113-
if (!binding?.projectId) throw new Error('Project fixture missing')
114-
const ids = { userId, workspaceId, projectId: binding.projectId }
115-
fixtures.push(ids)
116-
const context: AgentCliExecutionContext = {
113+
}
114+
115+
function cliContext(userId: string, workspaceId: string): AgentCliExecutionContext {
116+
return {
117117
userId,
118118
workspaceId,
119119
requestMode: 'agent',
@@ -125,6 +125,26 @@ async function fixture() {
125125
}),
126126
resolvedSecretTraceRegistry: new ResolvedSecretTraceRegistry([], { userId, workspaceId }),
127127
}
128+
}
129+
130+
async function fixture() {
131+
const userId = await insertUser('File reader')
132+
const workspaceId = generateId()
133+
await insertWorkspaceFixture(db, {
134+
id: workspaceId,
135+
ownerId: userId,
136+
billedAccountUserId: userId,
137+
name: 'Read environment',
138+
})
139+
await grantWorkspace(userId, workspaceId, 'admin')
140+
const [binding] = await db
141+
.select({ projectId: workspace.projectId })
142+
.from(workspace)
143+
.where(eq(workspace.id, workspaceId))
144+
if (!binding?.projectId) throw new Error('Project fixture missing')
145+
const ids = { userId, workspaceId, projectId: binding.projectId }
146+
fixtures.push(ids)
147+
const context = cliContext(userId, workspaceId)
128148
const principal = createSessionPrincipal({ userId })
129149
return { ...ids, context, principal }
130150
}
@@ -272,6 +292,137 @@ check('benchmark reference generation cannot create Project files', async () =>
272292
expect(await stored()).toHaveLength(1)
273293
})
274294

295+
function simCli(
296+
argv: string[],
297+
context: ToolExecutionContext,
298+
fileOwner?: { entityType: 'project'; entityId: string }
299+
) {
300+
return executeSimCli(
301+
{ request: { ...(fileOwner ? { fileOwner } : {}), invocation: { kind: 'cli', argv } } },
302+
context
303+
)
304+
}
305+
306+
check('sim_cli for a callerless run holds only the read API scopes', async () => {
307+
const f = await fixture()
308+
const name = `capped-${generateId()}`
309+
const ownerFallback = { ...f.context, workflowId: '', secretActorUserId: null }
310+
// In-band calls carry no `userPermission`, only the run's recorded callerless authority.
311+
const contexts: ToolExecutionContext[] = [
312+
{ ...ownerFallback, userPermission: 'read' },
313+
ownerFallback,
314+
]
315+
const project = { entityType: 'project', entityId: f.projectId } as const
316+
const writes: Array<[string[], typeof project | undefined]> = [
317+
[['workflows', 'create', '--name', name], undefined],
318+
[['projects', 'files', 'create', f.projectId, '--name', name, '--content', 'x'], project],
319+
[['secrets', 'set', 'CAPPED', '--scope', 'personal', '--value', 'v'], undefined],
320+
]
321+
const workflows = () =>
322+
db
323+
.select({ id: workflow.id })
324+
.from(workflow)
325+
.where(and(eq(workflow.workspaceId, f.workspaceId), eq(workflow.name, name)))
326+
const projectFiles = () =>
327+
db
328+
.select({ id: workspaceFiles.id })
329+
.from(workspaceFiles)
330+
.where(eq(workspaceFiles.originalName, name))
331+
332+
for (const context of contexts) {
333+
for (const [argv, owner] of writes) {
334+
const refused = await simCli(argv, context, owner)
335+
expect(refused.success).toBe(false)
336+
expect(AgentCliRawResult.parse(refused.output).stderr).toContain('api:write')
337+
}
338+
const read = await simCli(['workflows', 'list', '--output', 'json'], context)
339+
expect(read.success, read.error).toBe(true)
340+
}
341+
expect(await workflows()).toEqual([])
342+
expect(await projectFiles()).toEqual([])
343+
344+
const attributed: ToolExecutionContext = {
345+
...f.context,
346+
workflowId: '',
347+
userPermission: 'admin',
348+
secretActorUserId: f.userId,
349+
}
350+
for (const [argv, owner] of writes) {
351+
const written = await simCli(argv, attributed, owner)
352+
expect(written.success, written.error).toBe(true)
353+
}
354+
expect(await workflows()).toHaveLength(1)
355+
expect(await projectFiles()).toHaveLength(1)
356+
})
357+
358+
check('sim_cli services for a callerless run refuse the writes they route', async () => {
359+
const f = await fixture()
360+
const service = (name: string, input: Record<string, unknown>, context: ToolExecutionContext) =>
361+
executeSimCli({ request: { invocation: { kind: 'service', name, input } } }, context)
362+
const inboxSecretScope = async () =>
363+
(
364+
await db
365+
.select({ scope: workspace.inboxSecretScope })
366+
.from(workspace)
367+
.where(eq(workspace.id, f.workspaceId))
368+
)[0]?.scope
369+
const before = await inboxSecretScope()
370+
const widen = before === 'all' ? 'selected' : 'all'
371+
const writes: Array<[string, Record<string, unknown>]> = [
372+
[
373+
'settings',
374+
{ scope: 'workspace', action: 'update', section: 'inbox', changes: { secretScope: widen } },
375+
],
376+
]
377+
378+
const callerless: ToolExecutionContext = { ...f.context, workflowId: '', secretActorUserId: null }
379+
for (const [name, input] of writes) {
380+
const refused = await service(name, input, callerless)
381+
expect(refused.success).toBe(false)
382+
expect(refused.error).toContain('api:write')
383+
}
384+
expect(await inboxSecretScope()).toBe(before)
385+
const read = await service(
386+
'settings',
387+
{ scope: 'workspace', action: 'get', section: 'inbox' },
388+
callerless
389+
)
390+
expect(read.success, read.error).toBe(true)
391+
392+
const attributed: ToolExecutionContext = {
393+
...f.context,
394+
workflowId: '',
395+
userPermission: 'admin',
396+
secretActorUserId: f.userId,
397+
}
398+
for (const [name, input] of writes) {
399+
const written = await service(name, input, attributed)
400+
expect(written.success, `${name}: ${written.error}`).toBe(true)
401+
}
402+
expect(await inboxSecretScope()).toBe(widen)
403+
})
404+
405+
check('sim_cli for a read-only member keeps what that role allows', async () => {
406+
const f = await fixture()
407+
const memberId = await insertUser('Read member')
408+
memberIds.push(memberId)
409+
await grantWorkspace(memberId, f.workspaceId, 'read')
410+
const member: ToolExecutionContext = {
411+
...cliContext(memberId, f.workspaceId),
412+
workflowId: '',
413+
userPermission: 'read',
414+
}
415+
416+
const personalSecret = await simCli(
417+
['secrets', 'set', 'MEMBER', '--scope', 'personal', '--value', 'v'],
418+
member
419+
)
420+
expect(personalSecret.success, personalSecret.error).toBe(true)
421+
const workflowCreate = await simCli(['workflows', 'create', '--name', 'x'], member)
422+
expect(workflowCreate.success).toBe(false)
423+
expect(AgentCliRawResult.parse(workflowCreate.output).stderr).not.toContain('api:write')
424+
})
425+
275426
check('Project grep matches public bytes while excluding classified secret text', async () => {
276427
const f = await fixture()
277428
const secret = 'synthetic-project-grep-canary'
@@ -771,6 +922,7 @@ afterAll(async () => {
771922
await deleteWorkspaceFixture(db, eq(workspace.id, f.workspaceId))
772923
await db.delete(user).where(eq(user.id, f.userId))
773924
}
925+
if (memberIds.length) await db.delete(user).where(inArray(user.id, memberIds))
774926
await rm(storageRoot, { recursive: true, force: true })
775927
const report = process.env.PROJECT_FILE_READ_REPORT_PATH ?? 'test-results/project-file-read.json'
776928
await mkdir(dirname(report), { recursive: true })

‎apps/sim/lib/mothership/tools/handlers/sim-cli.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ export async function executeSimCli(
5353
messageId: context.messageId,
5454
parentToolCallId: context.parentToolCallId,
5555
userPermission: context.userPermission,
56+
secretActorUserId: context.secretActorUserId,
5657
resolvedSecretTraceRegistry: context.resolvedSecretTraceRegistry,
5758
readOnly: context.benchmark === 'distill',
5859
})

‎apps/sim/lib/mothership/tools/server/base-tool.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,4 +68,6 @@ export interface BaseServerTool<TArgs = unknown, TResult = unknown> {
6868
inputSchema?: z.ZodType<TArgs>
6969
/** Optional Zod schema for output validation */
7070
outputSchema?: z.ZodType<TResult>
71+
/** Whether these validated arguments change stored data; a delegated read grant refuses them. */
72+
writes?(args: TArgs): boolean
7173
}

‎apps/sim/lib/mothership/tools/server/dashboards.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ function workspaceFor(input: { workspaceId?: string }, context?: ServerToolConte
2424
export const dashboardsServerTool: BaseServerTool = {
2525
name: 'dashboards',
2626
inputSchema: mothershipDashboardsInputSchema,
27+
writes: (args) => mothershipDashboardsInputSchema.parse(args).action === 'set',
2728
async execute(raw, context) {
2829
const input = mothershipDashboardsInputSchema.parse(raw)
2930
const workspaceId = workspaceFor(input, context)

0 commit comments

Comments
 (0)